On Fri, 9 Apr 2021 20:40:59 +0100 Matthew Wilcox <willy@xxxxxxxxxxxxx> wrote: > Hi Claudio, > > I'm working on making the page cache manage memory in larger chunks > than PAGE_SIZE [1] [2]. In doing so, I came across this call that yes, I had read the LWN article with great interest > you added: > > @@ -2807,6 +2807,13 @@ int __test_set_page_writeback(struct page > *page, bool keep_write) inc_zone_page_state(page, > NR_ZONE_WRITE_PENDING); } > unlock_page_memcg(page); > + access_ret = arch_make_page_accessible(page); > > I'm going to change __test_set_page_writeback() to take a folio [3] > and now I'm wondering what interface you'd like to use. My > preference would be to rename arch_make_page_accessible() to > arch_make_folio_accessible() and pass a folio, at which time you > would make the entire folio (however many pages might be in it) > accessible. If you would rather, we can leave the interface as > arch_make_page_accessible(), in which case we'll just call it N times > in __test_set_page_writeback() (and I won't need to touch gup.c). For the rename case, how would you handle gup.c? Consider that arch_make_page_accessible deals (typically) with KVM guest pages. Once you bundle up the pages in folios, you can have different pages in the same folio with different properties. In case of failure, you could end up with a folio with some pages processed and some not processed. Would you stop at the first error? What would the state of the folio be? On s390x we use the PG_arch_1 bit to mark secure pages, how would that work with folios? and how are fault handlers affected by this folio conversion? would they still work on pages, or would that also work on folios? on s390x we use the arch_make_page_accessible function in some fault handlers. a possible approach maybe would be to keep the _page variant, and add a _folio wrapper around it for s390x the PG_arch_1 is very important to prevent protected pages from being fed to I/O, as in that case Very Bad Things™ would happen. sorry for the wall of questions, but I actually like your folio approach and I want to understand it better, so we can find a way to make everything work well together > Let me know what you want. > > [1] https://lwn.net/Articles/849538/ > [2] > https://lore.kernel.org/linux-mm/20210409185105.188284-1-willy@xxxxxxxxxxxxx/ > [3] > https://git.infradead.org/users/willy/pagecache.git/commitdiff/85297eb08f1b034b9652ea63dd053e3be4d7de7f > > PS: The prototype is in gfp.h. That's not really appropriate; gfp.h > is about allocating memory, and this call really has nothing to do > with memory allocation. I think mm.h is a better place for it, if > you can't find a better header file than that. I had put it there because arch_alloc_page and arch_free_page are also there, and the behaviour, from a kernel point of view, is similar (unaccessible/unallocated pages will trigger a fault). I actually do not have a preference regarding where the prototype lives, as long as everything works. If you think mm.h is more appropriate, go for it :)
