[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <f64483ac-31d1-4f80-8fb0-fcf15867c6c5@lucifer.local>
Date: Tue, 26 Aug 2025 09:37:23 +0100
From: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
To: Jonathan Corbet <corbet@....net>
Cc: Harry Yoo <harry.yoo@...cle.com>,
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()
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.
Thanks :)
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 :)
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.
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,
>
> jon
Cheers, Lorenzo
Powered by blists - more mailing lists