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

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

On 1/14/25 9:46 AM, David Hildenbrand wrote:
> On 14.01.25 18:38, Yang Shi wrote:
>>
>>
>>
>> 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.
>
> Note that MAP_PRIVATE of /dev/zero will also make it appear in the 
> same way right now (I just tried).

Yes, I will add this in the commit log as another user visible change.

>
> The example is about MAP_FILE in general, not just MAP_SHARED IIUC.

MAP_FILE is actually ignored on Linux per 
https://man7.org/linux/man-pages/man2/mmap.2.html. It also says 
"(regions created with the MAP_ANON | MAP_SHARED flags)". Anyway it 
looks like this man page may be a little bit outdated. We can clean it 
up later.