Re: [PATCH v2 2/2] fuse: remove tmp folio for writebacks and internal rb tree

[Joanne Koong <joannelkoong@gmail.com>]

On Wed, Oct 30, 2024 at 5:30 PM Shakeel Butt <shakeel.butt@linux.dev> wrote:
>
> On Wed, Oct 30, 2024 at 03:51:08PM GMT, Joanne Koong wrote:
> > On Wed, Oct 30, 2024 at 3:17 PM Bernd Schubert
> > <bernd.schubert@fastmail.fm> wrote:
> > >
> > >
> > >
> > > On 10/30/24 22:56, Shakeel Butt wrote:
> > > > On Wed, Oct 30, 2024 at 10:35:47AM GMT, Joanne Koong wrote:
> > > >> On Wed, Oct 30, 2024 at 10:27 AM Bernd Schubert
> > > >> <bernd.schubert@fastmail.fm> wrote:
> > > >>>
> > > >>>
> > > >>> Hmm, if tmp pages can be compacted, isn't that a problem for splice?
> > > >>> I.e. I don't understand what the difference between tmp page and
> > > >>> write-back page for migration.
> > > >>>
> > > >>
> > > >> That's a great question! I have no idea how compaction works for pages
> > > >> being used in splice. Shakeel, do you know the answer to this?
> > > >>
> > > >
> > > > Sorry for the late response. I still have to go through other unanswered
> > > > questions but let me answer this one quickly. From the way the tmp pages
> > > > are allocated, it does not seem like they are movable and thus are not
> > > > target for migration/compaction.
> > > >
> > > > The page with the writeback bit set is actually just a user memory page
> > > > cache which is moveable but due to, at the moment, under writeback it
> > > > temporarily becomes unmovable to not cause corruption.
> > >
> > > Thanks a lot for your quick reply Shakeel! (Actually very fast!).
> > >
> > > With that, it confirms what I wrote earlier - removing tmp and ignoring
> > > fuse writeback pages in migration should not make any difference
> > > regarding overall system performance. Unless I miss something,
> > > more on the contrary as additional memory pressure expensive page
> > > copying is being removed.
> > >
> >
> > Thanks for the information Shakeel, and thanks Bernd for bringing up
> > this point of discussion.
> >
> > Before I celebrate too prematurely, a few additional questions:
>
> You are asking hard questions, so CCed couple more folks to correct me
> if I am wrong.
>
> >
> > Are tmp pages (eg from folio_alloc(GFP_NOFS | __GFP_HIGHMEM, 0)) and
> > page cache pages allocated from the same memory pool? Or are tmp pages
> > allocated from a special memory pool that isn't meant to be
> > compacted/optimized?
>
> Memory pool is a bit confusing term here. Most probably you are asking
> about the migrate type of the page block from which tmp page is
> allocated from. In a normal system, tmp page would be allocated from page
> block with MIGRATE_UNMOVABLE migrate type while the page cache page, it
> depends on what gfp flag was used for its allocation. What does fuse fs
> use? GFP_HIGHUSER_MOVABLE or something else? Under low memory situation
> allocations can get mixed up with different migrate types.
>

I believe it's GFP_HIGHUSER_MOVABLE for the page cache pages since
fuse doesn't set any additional gfp masks on the inode mapping.

Could we just allocate the fuse writeback pages with GFP_HIGHUSER
instead of GFP_HIGHUSER_MOVABLE? That would be in fuse_write_begin()
where we pass in the gfp mask to __filemap_get_folio(). I think this
would give us the same behavior memory-wise as what the tmp pages
currently do, and would solve all our headaches regarding writeback
potentially blocking page migration/compaction.



Thanks,
Joanne

> >
> > If they are allocated from the same memory pool, then it seems like
> > there's no difference between tmp pages blocking a memory range from
> > being compacted vs. a page cache page blocking a memory range from
> > being compacted (by not clearing writeback). But if they are not
> > allocated from the same pool, then it seems like the page cache page
> > blocking migration could adversely affect general system performance
> > in a way that the tmp page doesn't?
>
> I think irrespective of where the page is coming from, the page under
> writeback is non-movable and can fragment the memory. The question that
> is that worse than a tmp page fragmenting the memory, I am not sure.
>