On Fri, Sep 1, 2023 at 3:28 AM Chao Peng <chao.p.peng@xxxxxxxxxxxxxxx> wrote:
> > FYI, I jumped the gun, sounds like Paolo got far enough along to form a strong
> > opinion[*].
Yeah, I still have some crashes here and there :) so I didn't post
anything, but here are some note from the experiment.
The only benefit is that gmem does not need the splitting logic of
__filemap_add_folio, because (I think) there shouldn't be conflicts
with existing entries. Otherwise it's just a bunch of duplicate code
with mm/filemap.c, about 150 lines of code.
One question is whether to use our own xarray (e.g. in the private
inode data) or use i_pages, and likewise for the invalidation lock.
In my patches I did the former for the sake of safety; I skimmed
filemap.c enough to think it would work to use i_pages, but I wasn't
very convinced of which idea is better.
Initially I was most nervous about memory failure, because of the path
identify_page_state -> page_action -> me_pagecache_{clean,dirty} ->
truncate_error_page -> filemap_release_folio. In the end
filemap_release_folio is doing nothing that is filemap-dependent, in
particular it doesn't do anything on the i_pages and the filemap
locks. So a plan could be to just identify filemap functions that
don't use i_pages, and rename them, for example filemap_release_folio
could become folio_release_from_mapping.
Crashes aside, I actually don't have any objection to *not* using the
filemap long term, but right now I don't really see a reason to do it.
We don't know if hugetlbfs support will be easier or harder with the
filemap for example, and given Vlastimil's reply I think the main
objection to using the filemap is gone. If we switch, we should invest
the time in making the filemap and mapping APIs a bit more separate.
Paolo
> Yeah, I see that, that is a good news actually, then we can go ahead with
> the current filemap one. I personally think these mm touchpoints are not
> a big deal when compared to previous versions, most part we are just using
> the APIs.
>
> >
> > Thanks for volunteering though, much appreciated!
>
> NP, any collaboration is to make this lasting years series merge earlier.
>
> Chao
> >
> > [*] https://lore.kernel.org/all/CABgObfay4FKV=foWLZzAWaC2kVHRnF1ib+6NC058QVZVFhGeyA@xxxxxxxxxxxxxx
>
