lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<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

Powered by Openwall GNU/*/Linux Powered by OpenVZ