Re: [PATCH v11 2/8] mm: rust: add vm_area_struct methods that require read access

[Andreas Hindborg <a.hindborg@kernel.org>]

"Lorenzo Stoakes" <lorenzo.stoakes@oracle.com> writes:

> On Tue, Jan 14, 2025 at 10:50:01AM +0100, Alice Ryhl wrote:
>> On Mon, Jan 13, 2025 at 3:45 PM Lorenzo Stoakes
>> <lorenzo.stoakes@oracle.com> wrote:
>> > > >> > For a series at v11 where there is broad agreement with maintainers within
>> > > >> > the subsystem which it wraps, perhaps the priority should be to try to have
>> > > >> > the series merged unless there is significant technical objection from the
>> > > >> > rust side?
>> > > >> >
>> > > >> >>
>> > > >> >> How about this:
>> > > >> >>
>> > > >> >> This clears the virtual memory map for the range given by `start` and
>> > > >> >> `size`, dropping refcounts to memory held by the mappings in this range. That
>> > > >> >> is, anonymous memory is completely freed, file-backed memory has its
>> > > >> >> reference count on page cache folio's dropped, any dirty data will still
>> > > >> >> be written back to disk as usual.
>> > > >> >
>> > > >> > Sorry I object to this, 'clears the virtual memory map' is really
>> > > >> > vague. What is already there is better.
>> > > >>
>> > > >> Would you like the proposed paragraph if we replaced "virtual memory
>> > > >> map" with "page table mappings", or do you object to the entirety of the
>> > > >> new suggestion?
>> > > >
>> > > > I object to the suggestion in general. The description is fine as it is.
>> > >
>> > > Ok. I'm raising a flag because I had more questions after reading the
>> > > docstring than before.
>> >
>> > Sure and so I think this is valuable information, and indicates it's
>> > probably worthwhile adding a little extra information on mentioning page
>> > tables.
>>
>> Sorry, I'm a bit lost. What would you like me to add? Perhaps there's
>> an existing file in Documentation/ that I can link to?
>
> Sure no problem, I propose expanding:
>
> /// This clears page table mappings for the range at the leaf level, leaving all other page
> /// tables intact,
> /// anonymous memory is completely freed, file-backed memory has its reference count on page
> /// cache folio's dropped, any dirty data will still be written back to disk as usual.
>
> To include information on page tables. I suggest something like:
>
> /// It may seem odd that we clear at the leaf level, this is however a product
> /// of the page table structure used to map physical memory into a virtual
> /// address space - each virtual address actually consists of a bitmap of array
> /// indices into page tables, which form a hierarchical page table level
> /// structure.
> ///
> /// As a result, each page table level maps a multiple of page table levels
> /// below, and thus span ever increasing ranges of pages. At the leaf or PTE
> /// level, we map the actual physical memory.
> ///
> /// It is here where a zap operates, as it the only place we can be certain of
> /// clearing without impacting any other virtual mappings. It is an
> /// implementation detail as to whether the kernel goes further in freeing
> /// unused page tables, but for the purposes of this operation we must only
> /// assume that the leaf level is cleared.
>
> Alice, Andreas - please let me know if this makes sense/is clear or needs
> further clarification.

Sounds good to me - thanks.

@Alice - can we add PTE, PTE entry, PMD, PUD to the vocabulary at the
top? Not sure if it should go here in virt.rs or in mm.rs. If you have
no cycles I can try to add it down the road.


Best regards,
Andreas Hindborg