Does this mean that the same page can be mapped multiple times in a VMA, at different offsets in the object?

The page is mapped in to different objects.
First time it is mapped in to vm_object created by shmem_file_setup(). That vm_object is associated with some GEM object in i915kms driver.
On the next step, the GEM object is mmap()-ed. It forces creation of another one vm_object with calling of cdev_pager_allocate(OBJT_MGTDEVICE).
And than execution of populate handler inserts into vm_object belonging mmap a page which is already inserted into vm_object created by shmem_file_setup()

And these are two different objects? So you must own two object' locks there?

BTW are the types of the objects different as well? I would expect that shmem_file_setup() to create something like a swap object.

So you must own two object' locks there?

Yes. That is why VM_OBJECT_ASSERT_WLOCKED() is placed twice in to this routine.

BTW are the types of the objects different as well? I would expect that shmem_file_setup() to create something like a swap object.

Yes. shmem_file_setup calls vm_pager_allocate(OBJT_DEFAULT

I.e. you establishing a lock order between two vm_objects. We do have order between a shadow object and its backing object, but there I do not think you have a shadowing. This is strange at least, and definitely should not be introduced on ad hoc manner.

When removing the page from the swap object, you at least need to clean the swap allocation, otherwise you get the swap space leak.

definitely should not be introduced on ad hoc manner.

Ok, I will rewrite that part with using vm_page_remove.

When removing the page from the swap object, you at least need to clean the swap allocation, otherwise you get the swap space leak.

Following code exists in vm_page_rename() execution path. Does it suits our needs?

/* Deferred free of swap space. */
if ((m->a.flags & PGA_SWAP_FREE) != 0)
        vm_pager_page_unswapped(m);

No, it should be something like vm_pager_page_unswapped(m) while m still belong to the shmem object.

I suggest to move the page renaming code into some helper that you would add into vm/.

A which point page leaves object?
As I see following steps in code starting from line 1620 in sys/vm/vm_page.c:

1. vm_pager_page_unswapped(m);
2. m->object = NULL;
3. vm_radix_remove(&object->rtree, m->pindex);
4. TAILQ_REMOVE(&object->memq, m, listq);

Does it not belong to shmem at step 1?

it should be something like vm_pager_page_unswapped(m) while m still belong to the shmem object.

The first thing vm_page_remove() does is calling of vm_pager_page_unswapped(m). Is last version satisfies above reqirement?

No, the call has to be unconditional. PGA_SWAP_FREE exists to support lazy deallocation of swap blocks, which helps reduce object lock contention. When removing a page from its object, we must complete the deallocation since the object contains swap block metadata. But if PGA_SWAP_FREE is not set, then vm_page_remove() will not deallocate the swap block.

So vm_page_page_unswapped() must be called directly.

Thank you for explanation.

Is it correct now?

Why page still belongs to shm_obj at this point?

Heh. That is result of "online coding".
MPASS moved to right place. Thanks!

I mean that you unbusied the page, and do not have shm_obj lock owned. What prevents other thread from e.g. handling the same access fault and removing page from shm_obj while all locks are dropped and page not busied?

From my point of view, execution of fault handlers is serialized at least on process level with mmap semaphore.
Following is what we have in sys/compat/linuxkpi/common/src/linux_compat.c (about line 550):

        down_write(&vmap->vm_mm->mmap_sem);
...
                err = vmap->vm_ops->fault(vmap, &vmf);
...
        up_write(&vmap->vm_mm->mmap_sem);

From what I see, mmap_sem only protects fault (or rather, the page instantiation during the fault), and mmap_single/close. Since shmem segment is just anon vm object, what prevents say pagedaemon from reclaiming the page while the object is unlocked/page is unbusied?

I am not even sure that taking semaphore in linux_cdev_pager_dtor() is enough to guarantee that other thread cannot unmap or remap the address meantime, if there more references on the cdev vm_object.

Do you mean something like this revision? Caller allowed to rename page from any object, not only from specified in parameter and callee checks that page->object persists across vm_object relocking?

Note that vm_page_busy_acquire() is not safe as you use it, for the same reason. The tmp_obj lock might be dropped, which would allow the page to be reclaimed. I thihk you need VM_ALLOC_WAITFAIL and retry on fail.

done

No, you need to retry same as for case page->object != tmp_obj simply because you might no longer own page->object lock.

done

Before the call to vm_page_busy_acquire(), you need to check that you own the lock for page->object, because tmp_obj might be already staled. [This does not mean that check after busy is not needed]

done

....
VM_OBJECT_WLOCK(tmp_obj);
if (page->object == tmp_obj &&
       vm_page_busy_acquire(page, VM_ALLOC_WAITFAIL)) {
           KASSERT(page->object == tmp_obj, ("XXX something about page changing identity"));
           KASSERT((page->oflags & VPO_UNMANAGED) == 0, ("XXX something about shmem"));
           vm_pager_page_unswapped(page);
           ....

BTW, is it possible to have other mappings for the page through the page->object mappings? If yes, this stuff would destroy almost all VM invariants. You should either add KASSERT(!pmap_page_is_mapped(page), ()); before vm_page_remove(page), or consider doing pmap_remove_all(page); but then existing mappings through shmem would loose content, getting zero pages into holes.

but then existing mappings through shmem would loose content, getting zero pages into holes.

At least in my case there are no mappings through shmem when populate handler is invoked. I was running i915kms with vm_object_deallocate(shm_obj) call inserted right before lkpi_vmf_insert_pfn_prot_locked() for some time and did not find any negative effects.

Ran it for some time. No "page is mapped" asserts were triggered

Should this printf() go away?

Is there a chance it may end up spamming the console?

Should this printf() go away?

Is there a chance it may end up spamming the console?

No, it should stay, and the spam is intended, if ever occurring.