On Wed, Jan 08, 2025 at 10:12:36PM +0100, David Hildenbrand wrote: > On 08.01.25 21:54, Matthew Wilcox wrote: > > On Wed, Jan 08, 2025 at 09:14:53PM +0100, David Hildenbrand wrote: > > > On 08.01.25 18:32, Matthew Wilcox wrote: > > > > On Wed, Jan 08, 2025 at 04:18:42PM +0000, Lorenzo Stoakes wrote: > > > > > @@ -280,7 +269,10 @@ static void fb_deferred_io_work(struct work_struct *work) > > > > > struct folio *folio = page_folio(pageref->page); > > > > > folio_lock(folio); > > > > > - folio_mkclean(folio); > > > > > + rmap_wrprotect_file_page(fbdefio->mapping, > > > > > + pageref->offset >> PAGE_SHIFT, > > > > > + compound_nr(pageref->page), > > > > > + page_to_pfn(pageref->page)); > > > > > folio_unlock(folio); > > > > > > > > Why do we need to lock the folio? (since this isn't necessarily a > > > > folio) > > > > > > Can you clarify the "since this isn't necessarily a folio" part ? Do you > > > mean in the future, when we split "struct page" and "struct folio"? > > > > Right. I need to finish the email that explains where I think we're > > going in 2025 ... > > > > > Doing an rmap walk on something that won't be a folio is ... sounds odd > > > (->wrong :) ) > > > > Not necessarily! We already do that (since 2022) for DAX (see > > 6a8e0596f004). rmap lets you find every place that a given range > > of a file is mapped into user address spaces; but that file might be a > > device file, and so it's not just pagecache but also (in this case) > > fb memory, and whatever else device drivers decide to mmap. > > Yes, that part I remember. > > I thought we would be passing in a page into rmap_wrprotect_file_page(), and > was wondering what we would do to "struct page" that won't be a folio in > there. The reason I provide a PFN is that we internally use a PFN for the walk, and everything else is folio-fied for stuff that isn't necessarily a folio. However it does seem silly to have to page_to_pfn() a page that we pass in, so I will update to accept a page and do this bit in the function itself. > > Probably, because the "_page" in rmap_wrprotect_file_page() is misleading :) > > ... should it be "file_range" ? (but we also pass the pfn ... ) > > -- > Cheers, > > David / dhildenb >
