Re: [PATCH v4 01/10] mm: vmscan: add validation before spliting shmem large folio

[Matthew Wilcox]

On Thu, Aug 08, 2024 at 10:36:23AM +0800, Baolin Wang wrote:
> On 2024/8/7 23:53, David Hildenbrand wrote:
> > But now I am wondering under which circumstances we end up calling
> > shmem_writepage() with a large folio. And I think the answer is the
> > comment of
> > folio_test_large(): via drivers/gpu/drm/i915/gem/i915_gem_shmem.c.
> > 
> > 
> > ... so if shmem_writepage() handles+checks that, could we do
> > 
> > diff --git a/mm/vmscan.c b/mm/vmscan.c
> > index a332cb80e928..7dfa3d6e8ba7 100644
> > --- a/mm/vmscan.c
> > +++ b/mm/vmscan.c
> > @@ -1257,11 +1257,6 @@ static unsigned int shrink_folio_list(struct
> > list_head *folio_list,
> >                                                  goto
> > activate_locked_split;
> >                                  }
> >                          }
> > -               } else if (folio_test_swapbacked(folio) &&
> > -                          folio_test_large(folio)) {
> > -                       /* Split shmem folio */
> > -                       if (split_folio_to_list(folio, folio_list))
> > -                               goto keep_locked;
> >                  }
> > 
> >                  /*
> > 
> > instead?
> 
> Seems reasonable to me. But we should pass the 'folio_list' to
> shmem_writepage() to list the subpages of the large folio. Let me try.
> Thanks.

We should be trying to remove shmem_writepage(), not make it more
complex.  We're making good progress removing instances of ->writepage;
just ceph, ecryptfs, f2fs, gfs2, hostfs, nilfs2, orangefs, vboxsf, shmem
& swap are left.  gfs2 patches are out for review.

As you can see from previous patches, the approach is to use
->writepages instead of ->writepage.  There should be no need to
handle a folio split list as splitting a folio leaves the folios in the
page cache and they'll naturally be found by subsequent iterations.