On Fri, May 27, 2022 at 02:22:19PM +0300, Vasily Averin wrote:
> On 5/27/22 04:40, Shakeel Butt wrote:
> > On Thu, May 26, 2022 at 6:21 PM Matthew Wilcox <willy@xxxxxxxxxxxxx> wrote:
> >>
> >> On Wed, May 25, 2022 at 11:26:37AM +0300, Vasily Averin wrote:
> >>> Commit 7b785645e8f1 ("mm: fix page cache convergence regression")
> >>> added support of new XA_FLAGS_ACCOUNT flag into all Xarray allocation
> >>> functions. Later commit 8fc75643c5e1 ("XArray: add xas_split")
> >>> introduced xas_split_alloc() but missed about XA_FLAGS_ACCOUNT
> >>> processing.
> >>
> >> Thanks, Vasily.
> >>
> >> Johannes, Shakeel, is this right? I don't fully understand the accounting
> >> stuff.
> >>
> >
> > If called from __filemap_add_folio() then this is correct.
> >
> > However from split_huge_page_to_list(), we can not use the memcg from
> > current as that codepath is called from reclaim which can be triggered
> > by processes of other memcgs.
> Btw, Shakeel, Johannes,
> I would like to understand, when Xarray should use XA_FLAGS_ACCOUNT ?
>
> From my point of view, this should be useless:
> a) if Xarray stores some index (idr?) - his memory is quite small,
> and his accounting can be ignored.
> b) if Xarray stores some accounted - the size of the corresponding Xarray
> infrastructure is usually significantly smaller than the size of the stored object,
> sо his accounting can be skipped too.
> c) if Xarray stores some non-accounted objects - it makes no sense to account
> corresponding Xarray infrastructure. In case of necessary it makes much more sense
> to enable accounting for stored objects (and return to case b).
>
> Am I missed something important perhaps?
>
> I looked for the description of 7b785645e8f1, but o be honest I'm still not sure
> that I understand correctly why XA_FLAGS_ACCOUNT flag solved the described problem.
>
> Could you please explain this in more details?
>
> Was it because the non-accounted Xarray kept a reference to the stored object
> and thus prevents it from being reclaimed?
>
> If so, was it some special case, or should it affect all such cases,
> and my b) statement above is not correct?
It's all about shadow entries, which are small, so b) is not true for them.
There is a good description on how it works on top of mm/workingset.c
