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

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: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <9fc9ac50-abce-48bd-979f-2e00b26917b5@lucifer.local>
Date: Tue, 3 Jun 2025 11:56:37 +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 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.

>
> > +.. 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