On 11/12/24 7:00 PM, David Hildenbrand wrote: > 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. > That's fair, I just mean that "works most of the time" is probably good enough for the time being considering that this codepath is only invoked by debuggers in practice anyway. ~~ Lina
