Re: [PATCH v2 1/5] mm: Implement folio_remove_rmap_range()

[David Hildenbrand]

On 30.08.23 17:42, Ryan Roberts wrote:
> On 30/08/2023 15:51, Matthew Wilcox wrote:
> > On Wed, Aug 30, 2023 at 10:50:07AM +0100, Ryan Roberts wrote:
> > > Like page_remove_rmap() but batch-removes the rmap for a range of pages
> > > belonging to a folio. This can provide a small speedup due to less
> > > manipuation of the various counters. But more crucially, if removing the
> > > rmap for all pages of a folio in a batch, there is no need to
> > > (spuriously) add it to the deferred split list, which saves significant
> > > cost when there is contention for the split queue lock.
> > >
> > > All contained pages are accounted using the order-0 folio (or base page)
> > > scheme.
> > >
> > > page_remove_rmap() is refactored so that it forwards to
> > > folio_remove_rmap_range() for !compound cases, and both functions now
> > > share a common epilogue function. The intention here is to avoid
> > > duplication of code.
> >
> > What would you think to doing it like this instead?  This probably doesn't
> > even compile and it's definitely not sanity checked; just trying to get
> > across an idea of the shape of this code.  I think this is more like
> > what DavidH was asking for (but he's on holiday this week so won't be
> > able to confirm).
>
> I think it was actually Yu Zhou who was arguing for something more like this?

I think so, not me.

... but the second variant is certainly shorter.

--
Cheers,

David / dhildenb