[<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