[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZMFNgmlT1JpI0z5+@casper.infradead.org>
Date: Wed, 26 Jul 2023 17:44:50 +0100
From: Matthew Wilcox <willy@...radead.org>
To: Yu Zhao <yuzhao@...gle.com>
Cc: Ryan Roberts <ryan.roberts@....com>,
Andrew Morton <akpm@...ux-foundation.org>,
Yin Fengwei <fengwei.yin@...el.com>,
David Hildenbrand <david@...hat.com>,
Yang Shi <shy828301@...il.com>,
"Huang, Ying" <ying.huang@...el.com>, Zi Yan <ziy@...dia.com>,
linux-kernel@...r.kernel.org, linux-mm@...ck.org
Subject: Re: [PATCH v3 2/3] mm: Implement folio_remove_rmap_range()
On Tue, Jul 25, 2023 at 11:53:26PM -0600, Yu Zhao wrote:
> > +void folio_remove_rmap_range(struct folio *folio, struct page *page,
> > + int nr, struct vm_area_struct *vma);
>
> I prefer folio_remove_rmap_range(page, nr, vma). Passing both the
> folio and the starting page seems redundant to me.
>
> Matthew, is there a convention (function names, parameters, etc.) for
> operations on a range of pages within a folio?
We've been establishing that convention recently, yes. It seems
pointless to re-derive the folio from the page when the caller already
has the folio. I also like Ryan's point that it reinforces that all
pages must be from the same folio.
> And regarding the refactor, what I have in mind is that
> folio_remove_rmap_range() is the core API and page_remove_rmap() is
> just a wrapper around it, i.e., folio_remove_rmap_range(page, 1, vma).
>
> Let me post a diff later and see if it makes sense to you.
I think that can make sense. Because we limit to a single page table,
specifying 'nr = 1 << PMD_ORDER' is the same as 'compound = true'.
Just make it folio, page, nr, vma. I'd actually prefer it as (vma,
folio, page, nr), but that isn't the convention we've had in rmap up
until now.
Powered by blists - more mailing lists