On Tue, May 02, 2023 at 02:47:22PM +0200, David Hildenbrand wrote: > On 02.05.23 14:40, Peter Zijlstra wrote: > > On Tue, May 02, 2023 at 02:08:10PM +0200, Peter Zijlstra wrote: > > > > > > > > > > > > > > > > > if (folio_test_anon(folio)) > > > > > return true; > > > > > > > > This relies on the mapping so belongs below the lockdep assert imo. > > > > > > Oh, right you are. > > > > > > > > > > > > > /* > > > > > * Having IRQs disabled (as per GUP-fast) also inhibits RCU > > > > > * grace periods from making progress, IOW. they imply > > > > > * rcu_read_lock(). > > > > > */ > > > > > lockdep_assert_irqs_disabled(); > > > > > > > > > > /* > > > > > * Inodes and thus address_space are RCU freed and thus safe to > > > > > * access at this point. > > > > > */ > > > > > mapping = folio_mapping(folio); > > > > > if (mapping && shmem_mapping(mapping)) > > > > > return true; > > > > > > > > > > return false; > > > > > > > > > > > +} > > > > So arguably you should do *one* READ_ONCE() load of mapping and > > consistently use that, this means open-coding both folio_test_anon() and > > folio_mapping(). > > Open-coding folio_test_anon() should not be required. We only care about > PAGE_MAPPING_FLAGS stored alongside folio->mapping, that will stick around > until the anon page was freed. > Ack, good point! > @Lorenzo, you might also want to special-case hugetlb directly using > folio_test_hugetlb(). > I already am :) I guess you mean when I respin along these lines? Will port that across to. > -- > Thanks, > > David / dhildenb >
