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

[Yang Shi]

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.