[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <38bcf562-86dc-42b2-9ffc-53cbc5d8ac22@lucifer.local>
Date: Tue, 3 Jun 2025 12:24:12 +0100
From: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
To: Jonathan Corbet <corbet@....net>
Cc: Andrew Morton <akpm@...ux-foundation.org>,
Suren Baghdasaryan <surenb@...gle.com>,
"Liam R . Howlett" <Liam.Howlett@...cle.com>,
Vlastimil Babka <vbabka@...e.cz>,
Shakeel Butt <shakeel.butt@...ux.dev>, Jann Horn <jannh@...gle.com>,
Qi Zheng <zhengqi.arch@...edance.com>, linux-mm@...ck.org,
linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] docs/mm: expand vma doc to highlight pte freeing,
non-vma traversal
On Tue, Jun 03, 2025 at 11:56:37AM +0100, Lorenzo Stoakes wrote:
> On Mon, Jun 02, 2025 at 03:38:55PM -0600, Jonathan Corbet wrote:
> > Lorenzo Stoakes <lorenzo.stoakes@...cle.com> writes:
> >
> > > --- a/Documentation/mm/process_addrs.rst
> > > +++ b/Documentation/mm/process_addrs.rst
> > > @@ -303,7 +303,9 @@ There are four key operations typically performed on page tables:
> > > 1. **Traversing** page tables - Simply reading page tables in order to traverse
> > > them. This only requires that the VMA is kept stable, so a lock which
> > > establishes this suffices for traversal (there are also lockless variants
> > > - which eliminate even this requirement, such as :c:func:`!gup_fast`).
> > > + which eliminate even this requirement, such as :c:func:`!gup_fast`). There is
> > > + also a special case of page table traversal for non-VMA regions which we
> >
> > The "!gup_fast" caught my attention - I was unaware that Sphinx had such
> > a thing. Its purpose would be to appear to suppress the generation of the
> > link that turns the cross reference into a cross reference.
> >
> > The MM docs are full of these, do we know why?
>
> Removing it from the struct vm_area_struct struct immediately give:
>
> /home/lorenzo/kerndev/kernels/mm/Documentation/mm/process_addrs.rst:11: WARNING: Unparseable C cross-reference: 'struct vm_area_struct'
> Invalid C declaration: Expected identifier in nested name, got keyword: struct [error at 6]
> struct vm_area_struct
>
> And given C's weirdness with typing I really prefer to be explicit in
> referencing a struct vs. e.g. a typedef.
>
> At any rate I'm not sure it's all that useful to cross-reference these?
>
> Any such change would need to be a separate patch anyway or otherwise this
> becomes a 'add additional documentation and drop cross-refs'.
>
> >
> > I would recommend removing them unless there's some reason I don't see
> > for doing this. Also get rid of the :c:func: noise entirely - just
> > saying gup_fast() will do the right thing.
>
> Re: the c:func: stuff -
>
> Well, the right thing is making function + type names clearly discernable, and
> it just putting in the function name like that absolutely does not do the right
> thing in that respect.
>
> I feel strongly on this, as I've tried it both ways and it's a _really_ big
> difference in how readable the document is.
>
> I spent a lot of time trying to make it as readable as possible (given the
> complexity) so would really rather not do anything that would hurt that.
>
Somebody told me that in _other_ .rst's, seemingly, it does figure out xxx() ->
function and highlights it like this.
But for me, it does not... :)
In case that's something you assumed would happen here.
This is against me building locally with:
make SPHINXDIRS=mm htmldocs
> >
> > > +.. note:: Since v6.14 and commit 6375e95f381e ("mm: pgtable: reclaim empty
> > > PTE + page in madvise (MADV_DONTNEED)"), we now also free empty PTE tables
> > > + on zap. This does not change zapping locking requirements.
> >
> > As a general rule, the docs should represent the current state of
> > affairs; people wanting documentation for older kernels are best advised
> > to look at those kernels. Or so it seems to me, anyway. So I'm not
> > sure we need the "since..." stuff.
>
> Sure, I will drop this.
>
> >
> > Thanks,
> >
> > jon
Powered by blists - more mailing lists