Re: [PATCH] docs/mm: expand vma doc to highlight pte freeing, non-vma traversal

[Lorenzo Stoakes]

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@xxxxxxxxxx> 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