Re: [PATCH] /dev/zero: make private mapping full anonymous mapping

[Yang Shi <yang@os.amperecomputing.com>]

On 1/14/25 9:23 AM, David Hildenbrand wrote:
> On 14.01.25 18:01, Yang Shi wrote:
>>
>>
>>
>> On 1/14/25 7:06 AM, David Hildenbrand wrote:
>>> On 14.01.25 15:52, Lorenzo Stoakes wrote:
>>>> On Tue, Jan 14, 2025 at 02:01:32PM +0100, David Hildenbrand wrote:
>>>>> On 13.01.25 23:30, Yang Shi wrote:
>>>>>> When creating private mapping for /dev/zero, the driver makes it an
>>>>>> anonymous mapping by calling set_vma_anonymous().  But it just sets
>>>>>> vm_ops to NULL, vm_file is still valid and vm_pgoff is also file
>>>>>> offset.
>>>>>>
>>>>>> This is a special case and the VMA doesn't look like either
>>>>>> anonymous VMA
>>>>>> or file VMA.  It confused other kernel subsystem, for example,
>>>>>> khugepaged [1].
>>>>>>
>>>>>> It seems pointless to keep such special case.  Making private
>>>>>> /dev/zero>
>>>>> mapping a full anonymous mapping doesn't change the semantic of
>>>>>> /dev/zero either.
>>>>>>
>>>>>> The user visible effect is the mapping entry shown in
>>>>>> /proc/<PID>/smaps
>>>>>> and /proc/<PID>/maps.
>>>>>>
>>>>>> Before the change:
>>>>>> ffffb7190000-ffffb7590000 rw-p 00001000 00:06
>>>>>> 8                          /dev/zero
>>>>>>
>>>>>> After the change:
>>>>>> ffffb6130000-ffffb6530000 rw-p 00000000 00:00 0
>>>>>>
>>>>>
>>>>> Hm, not sure about this. It's actually quite consistent to have that
>>>>> output
>>>>> in smaps the way it is. You mapped a file at an offset, and it
>>>>> behaves like
>>>>> an anonymous mapping apart from that.
>>>>>
>>>>> Not sure if the buggy khugepaged thing is a good indicator to
>>>>> warrant this
>>>>> change.
>>
>> I admit this may be a concern, but I doubt who really care about it...
>>
>
> There is an example in the man page [1] about /proc/self/map_files/.
>
> I assume that will also change here.

IIUC, that example is specific to "anonymous shared memory" created by 
shared mapping of /dev/zero.

>
> It's always hard to tell who that could affect, but I'm not convinced 
> this is worth it to find it out :)
>
>>>>
>>>> Yeah, this is a user-facing fundamental change that hides information
>>>> and
>>>> defies expectation so I mean - it's a no go really isn't it?
>>>>
>>>> I'd rather we _not_ make this anon though, because isn't life 
>>>> confusing
>>>> enough David? I thought it was bad enough with 'anon, file and lol
>>>> shmem'
>>>> but 'lol lol also /dev/zero' is enough to make me want to frolick 
>>>> in the
>>>> fields...
>>>
>>> I recall there are users that rely on this memory to get the shared
>>> zeropage on reads etc (in comparison to shmem!), so I better not ...
>>> mess with this *at all* :)
>>
>> The behavior won't be changed.
>
> Yes, I know. And that's good ;)
>
>
> [1] https://man7.org/linux/man-pages/man5/proc_pid_map_files.5.html
>