Re: [PATCH v3 0/6] add mTHP support for anonymous shmem

[Daniel Gomez]

On Tue, Jun 04, 2024 at 05:45:20PM +0800, Baolin Wang wrote:
> 
> 
> On 2024/6/4 16:18, Daniel Gomez wrote:
> > On Fri, May 31, 2024 at 01:13:48PM +0200, David Hildenbrand wrote:
> > > > > 
> > > > > As a default, we should not be using large folios / mTHP for any shmem,
> > > > > just like we did with THP via shmem_enabled. This is what this series
> > > > > currently does, and is aprt of the whole mTHP user-space interface design.
> > > > > 
> > > > > Further, the mTHP controls should control all of shmem, not only
> > > > > "anonymous shmem".
> > > > 
> > > > Yes, that's what I thought and in my TODO list.
> > > 
> > > Good, it would be helpful to coordinate with Daniel and Pankaj.
> > 
> > I've integrated patches 11 and 12 from the lsf RFC thread [1] on top of Baolin's
> > v3 patches. You may find a version in my integration branch here [2]. I can
> > attach them here if it's preferred.
> > 
> > [1] https://lore.kernel.org/all/20240515055719.32577-1-da.gomez@xxxxxxxxxxx/
> > [2] https://protect2.fireeye.com/v1/url?k=a23e7c06-c3b56926-a23ff749-74fe485fb347-371ca2bfd5d9869f&q=1&e=6974304e-a786-4255-93a7-57498540241c&u=https%3A%2F%2Fgitlab.com%2Fdkruces%2Flinux-next%2F-%2Fcommits%2Fnext-20240604-shmem-mthp
> > 
> > The point here is to combine the large folios strategy I proposed with mTHP
> > user controls. Would it make sense to limit the orders to the mapping order
> > calculated based on the size and index?
> 
> IMO, for !anon shmem, this change makes sense to me. We should respect the
> size and mTHP should act as a order filter.

What about respecing the size when within_size flag is enabled? Then, 'always'
would allocate mTHP enabled folios, regardless of the size. And 'never'
would ignore mTHP and size. So, 'never' can be used for this 'safe' boot case
mentioned in the discussion.

> 
> For anon shmem, we should ignore the length, which you always set it to
> PAGE_SIZE in patch [1].
> 
> [1] https://protect2.fireeye.com/v1/url?k=0d75a0c6-6cfeb5e6-0d742b89-74fe485fb347-904fa75c8efebdc2&q=1&e=6974304e-a786-4255-93a7-57498540241c&u=https%3A%2F%2Fgitlab.com%2Fdkruces%2Flinux-next%2F-%2Fcommit%2Fedf02311fd6d86b355d3aeb74e67c8da6de3c569

Since we are ignoring the length, we should ignore any value being passed.

> 
> > @@ -1765,6 +1798,10 @@ static struct folio *shmem_alloc_and_add_folio(struct vm_fault *vmf,
> > 
> >                  order = highest_order(suitable_orders);
> >                  while (suitable_orders) {
> > +                       if (order > mapping_order) {
> > +                               order = next_order(&suitable_orders, order);
> > +                               continue;
> > +                       }
> >                          pages = 1UL << order;
> >                          index = round_down(index, pages);
> >                          folio = shmem_alloc_folio(gfp, order, info, index);
> > 
> > Note: The branch still need to be adapted to include !anon mm.