Re: Inaccessible pages & folios

[Claudio Imbrenda <imbrenda@linux.ibm.com>]

On Fri, 9 Apr 2021 20:40:59 +0100
Matthew Wilcox <willy@infradead.org> 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@infradead.org/
> [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 :)