Re: [PATCH] mm: Fix __wp_page_copy_user fallback path for remote mm

[David Hildenbrand]

On 12.11.24 10:48, Asahi Lina wrote:
> On 11/11/24 8:24 AM, Alistair Popple wrote:
> > David Hildenbrand <david@xxxxxxxxxx> writes:
> >
> > > On 07.11.24 18:32, Asahi Lina wrote:
> > > > On 11/8/24 2:14 AM, David Hildenbrand wrote:
> > > > > I recall that there is still a problem with false-positives on
> > > > > folio_test_anon() with ZONE_DEVICE pages, so it's maybe not that
> > > > > easy ... and the whole get_dev_pagemap() stuff is nasty.
> >
> > Specifically FS DAX reuses PAGE_MAPPING_ANON in
> > include/linux/page-flags.h
> >
> >      /*
> >       * Different with flags above, this flag is used only for fsdax mode.  It
> >       * indicates that this page->mapping is now under reflink case.
> >       */
> >      #define PAGE_MAPPING_DAX_SHARED	((void *)0x1)
> >
> > FS DAX pages are never anon though, so you could probably test for
> > !vma_is_dax() and/or add an implementation of is_fsdax_page().
> >
> > > > > Likely we would have to do what GUP does, and temporarily grab a pgmap
> > > > > reference. Gah.
> > > > >
> > > > >
> > > > > So if we sort out the pagemap stuff and the possibly wrong
> > > > > folio_test_anon() on some ZONE_DEVICE pages (but not all, because IIRC
> > > > > DEVICE_PRIVATE can be anon ...), it might be doable.
> >
> > Correct, DEVICE_PRIVATE and DEVICE_COHERENT pages are always anon (at
> > least for now).
> >
> > > > > But it sounds ugly, especially because that code might change soon and
> > > > > not require messing with ZONE_DEVICE pages on that level.
> >
> > Yes, I'm hopoing to get the next version of that series posted this
> > week. I found a couple of other FS DAX bugs that slowed me down.
> >
> >   - Alistair
> >
> > > > > And then, we'd not be able to handle VM_PFNMAP cleanly ...
>
> If this is all going to be fixed another way soon then I think there's
> no rush to get a workaround in earlier than that, I just don't want it
> to fall by the wayside.
>
> We have my original patch downstream in libkrunfw (which despite the
> lockdep complaints does work in practice)

I assume it's sufficient to deadlock when a writer pops up after you 
succeeded with the first read-locking, and before you start the second 
read-locking. IIRC, rwsem is a fair lock, so read-locking when 
already-read-locked is not guaranteed to work.

That's why lockdep complains.

--
Cheers,

David / dhildenb