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 for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aK2C-Wuv7c_yIOJ0@hyeyoo>
Date: Tue, 26 Aug 2025 18:48:41 +0900
From: Harry Yoo <harry.yoo@...cle.com>
To: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
Cc: Jonathan Corbet <corbet@....net>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Suren Baghdasaryan <surenb@...gle.com>,
        "Liam R . Howlett" <Liam.Howlett@...cle.com>,
        David Hildenbrand <david@...hat.com>, Kees Cook <kees@...nel.org>,
        Vlastimil Babka <vbabka@...e.cz>,
        Shakeel Butt <shakeel.butt@...ux.dev>, Mike Rapoport <rppt@...nel.org>,
        Michal Hocko <mhocko@...e.com>, Jann Horn <jannh@...gle.com>,
        Pedro Falcato <pfalcato@...e.de>, Rik van Riel <riel@...riel.com>,
        linux-mm@...ck.org, linux-doc@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH V1 1/2] docs/mm: explain when and why rmap locks need to
 be taken during mremap()

On Tue, Aug 26, 2025 at 09:37:23AM +0100, Lorenzo Stoakes wrote:
> Harry - one brief very nitty note - could you do a cover letter even for 2 patch
> series?
> 
> This is a subjective thing and literally just my taste but I prefer it :P
> obviously this is optional as a result, but I feel it's neater.
>

No problem! will do cover letter from next time.

> On Tue, Aug 26, 2025 at 01:22:03AM -0600, Jonathan Corbet wrote:
> > Harry Yoo <harry.yoo@...cle.com> writes:
> >
> > > While move_ptes() has a comment explaining why rmap locks are needed,
> > > Documentation/mm/process_addrs.rst does not. Without being aware of that
> > > comment, I spent hours figuring out how things could go wrong and why,
> > > in some cases, rmap locks can be safely skipped.
> > >
> > > Add a more comprehensive explanation to the documentation to save time
> > > for others.
> > >
> > > Signed-off-by: Harry Yoo <harry.yoo@...cle.com>
> > > ---
> > >  Documentation/mm/process_addrs.rst | 32 ++++++++++++++++++++++++++++++
> > >  1 file changed, 32 insertions(+)
> > >
> > > diff --git a/Documentation/mm/process_addrs.rst b/Documentation/mm/process_addrs.rst
> > > index be49e2a269e4..ee7c0dba339e 100644
> > > --- a/Documentation/mm/process_addrs.rst
> > > +++ b/Documentation/mm/process_addrs.rst
> > > @@ -744,6 +744,38 @@ You can observe this in the :c:func:`!mremap` implementation in the functions
> > >  :c:func:`!take_rmap_locks` and :c:func:`!drop_rmap_locks` which perform the rmap
> > >  side of lock acquisition, invoked ultimately by :c:func:`!move_page_tables`.
> > >
> > > +.. note:: If :c:func:`!mremap()` -> :c:func:`!move_ptes()` does not take rmap
> > > +          locks, :c:func:`!rmap_walk()` may miss a pte for the folio.
> > > +
> > > +          The problematic sequence is as follows:
> >
> > Please don't use :c:func: - just write function() and all the right
> > things will happen.  (For extra credit, fix the existing usages :)

Hi Jonathan and Lorenzo,

**blaming myself for thinking**
"Hmmm it's already there, it should be fine to use it..."

> Yeah sorry Jon on latter bit, I did mean to get to that but workload
> been... well you can see on lore :P
> 
> I have a real backlog even more than usual right now too due to daring to take a
> day off on a national holiday here in the UK :))
> 
> Harry - more than happy for you to do the above as part of this series or
> separately, will sling you some tags accordingly.

Okay, I'll do as a part of the series (process_addrs.rst and memory-model.rst).

> If you're not already doing it (expect you are) you can generate docs via:
> 
> make SPHINXDIRS=mm htmldocs
> 
> Then get access to generated HTML in a browser locally in Documentation/output/

Thanks and yeah I'm doing it! :)

> >
> > Thanks,
> >
> > jon
> 
> Cheers, Lorenzo

-- 
Cheers,
Harry / Hyeonggon

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ