diff mbox series

mm: anonymous shared memory naming

Message ID 20221105025342.3130038-1-pasha.tatashin@soleen.com (mailing list archive)
State New
Headers show
Series mm: anonymous shared memory naming | expand

Commit Message

Pasha Tatashin Nov. 5, 2022, 2:53 a.m. UTC
Since:
commit 9a10064f5625 ("mm: add a field to store names for private anonymous
memory")

We can set names for private anonymous memory but not for shared
anonymous memory. However, naming shared anonymous memory just as
useful for tracking purposes.

Extend the functionality to be able to set names for shared anon.

/ [anon_shmem:<name>]      an anonymous shared memory mapping that has
                           been named by userspace

Sample output:
        share = mmap(NULL, SIZE, PROT_READ | PROT_WRITE,
                     MAP_SHARED | MAP_ANONYMOUS, -1, 0);
        rv = prctl(PR_SET_VMA, PR_SET_VMA_ANON_NAME,
                   share, SIZE, "shared anon");

/proc/<pid>/maps (and smaps):
7fc8e2b4c000-7fc8f2b4c000 rw-s 00000000 00:01 1024
/dev/zero (deleted) [anon_shmem:shared anon]

pmap $(pgrep a.out)
254:   pub/a.out
000056093fab2000      4K r---- a.out
000056093fab3000      4K r-x-- a.out
000056093fab4000      4K r---- a.out
000056093fab5000      4K r---- a.out
000056093fab6000      4K rw--- a.out
000056093fdeb000    132K rw---   [ anon ]
00007fc8e2b4c000 262144K rw-s- zero (deleted) [anon_shmem:shared anon]

Signed-off-by: Pasha Tatashin <pasha.tatashin@soleen.com>
---
 Documentation/filesystems/proc.rst |  4 +++-
 fs/proc/task_mmu.c                 |  7 ++++---
 include/linux/mm.h                 |  2 ++
 include/linux/mm_types.h           | 27 +++++++++++++--------------
 mm/madvise.c                       |  7 ++-----
 mm/shmem.c                         | 13 +++++++++++--
 6 files changed, 35 insertions(+), 25 deletions(-)

Comments

kernel test robot Nov. 5, 2022, 8:27 a.m. UTC | #1
Hi Pasha,

I love your patch! Yet something to improve:

[auto build test ERROR on akpm-mm/mm-everything]

url:    https://github.com/intel-lab-lkp/linux/commits/Pasha-Tatashin/mm-anonymous-shared-memory-naming/20221105-105421
base:   https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything
patch link:    https://lore.kernel.org/r/20221105025342.3130038-1-pasha.tatashin%40soleen.com
patch subject: [PATCH] mm: anonymous shared memory naming
config: hexagon-randconfig-r022-20221104
compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project 2bbafe04fe785a9469bea5a3737f8d7d3ce4aca2)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/59f51f2cbc810732de1b8c350aaee7fb54b932e1
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Pasha-Tatashin/mm-anonymous-shared-memory-naming/20221105-105421
        git checkout 59f51f2cbc810732de1b8c350aaee7fb54b932e1
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   In file included from mm/shmem.c:29:
   In file included from include/linux/pagemap.h:11:
   In file included from include/linux/highmem.h:12:
   In file included from include/linux/hardirq.h:11:
   In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/hexagon/include/asm/io.h:334:
   include/asm-generic/io.h:547:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __raw_readb(PCI_IOBASE + addr);
                             ~~~~~~~~~~ ^
   include/asm-generic/io.h:560:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
                                                           ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu'
   #define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
                                                     ^
   In file included from mm/shmem.c:29:
   In file included from include/linux/pagemap.h:11:
   In file included from include/linux/highmem.h:12:
   In file included from include/linux/hardirq.h:11:
   In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/hexagon/include/asm/io.h:334:
   include/asm-generic/io.h:573:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
                                                           ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu'
   #define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
                                                     ^
   In file included from mm/shmem.c:29:
   In file included from include/linux/pagemap.h:11:
   In file included from include/linux/highmem.h:12:
   In file included from include/linux/hardirq.h:11:
   In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/hexagon/include/asm/io.h:334:
   include/asm-generic/io.h:584:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writeb(value, PCI_IOBASE + addr);
                               ~~~~~~~~~~ ^
   include/asm-generic/io.h:594:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
                                                         ~~~~~~~~~~ ^
   include/asm-generic/io.h:604:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
                                                         ~~~~~~~~~~ ^
>> mm/shmem.c:3997:62: error: initializer element is not a compile-time constant
   static const struct vm_operations_struct shmem_anon_vm_ops = shmem_vm_ops;
                                                                ^~~~~~~~~~~~
   6 warnings and 1 error generated.


vim +3997 mm/shmem.c

  3996	
> 3997	static const struct vm_operations_struct shmem_anon_vm_ops = shmem_vm_ops;
  3998
Bagas Sanjaya Nov. 5, 2022, 9:34 a.m. UTC | #2
On Sat, Nov 05, 2022 at 02:53:42AM +0000, Pasha Tatashin wrote:
> Since:
> commit 9a10064f5625 ("mm: add a field to store names for private anonymous
> memory")
> 
> We can set names for private anonymous memory but not for shared
> anonymous memory. However, naming shared anonymous memory just as
> useful for tracking purposes.
> 

Who are "we"?

Instead, say "Since commit <commit>, name for private anonymous memory,
but not shared anonymous, can be set".

> @@ -431,8 +431,10 @@ is not associated with a file:
>   [stack]                    the stack of the main process
>   [vdso]                     the "virtual dynamic shared object",
>                              the kernel system call handler
> - [anon:<name>]              an anonymous mapping that has been
> + [anon:<name>]              a private anonymous mapping that has been
>                              named by userspace
> + path [anon_shmem:<name>]   an anonymous shared memory mapping that has
> +                            been named by userspace
>   =============              ====================================
>  

The table above triggers Sphinx warning:

Documentation/filesystems/proc.rst:436: WARNING: Malformed table.
Text in column margin in table line 8.

=============              ====================================
[heap]                     the heap of the program
[stack]                    the stack of the main process
[vdso]                     the "virtual dynamic shared object",
                           the kernel system call handler
[anon:<name>]              a private anonymous mapping that has been
                           named by userspace
path [anon_shmem:<name>]   an anonymous shared memory mapping that has
                           been named by userspace
=============              ====================================

I have applied the fixup:

---- >8 ----

diff --git a/Documentation/filesystems/proc.rst b/Documentation/filesystems/proc.rst
index 8f1e68460da5cd..3f17b4ef307fe4 100644
--- a/Documentation/filesystems/proc.rst
+++ b/Documentation/filesystems/proc.rst
@@ -426,7 +426,7 @@ with the memory region, as the case would be with BSS (uninitialized data).
 The "pathname" shows the name associated file for this mapping.  If the mapping
 is not associated with a file:
 
- =============              ====================================
+ ========================   ===========================================
  [heap]                     the heap of the program
  [stack]                    the stack of the main process
  [vdso]                     the "virtual dynamic shared object",
@@ -435,7 +435,7 @@ is not associated with a file:
                             named by userspace
  path [anon_shmem:<name>]   an anonymous shared memory mapping that has
                             been named by userspace
- =============              ====================================
+ ========================   ===========================================
 
  or if empty, the mapping is anonymous.
 

Thanks.
Kirill A . Shutemov Nov. 6, 2022, 1:33 p.m. UTC | #3
On Sat, Nov 05, 2022 at 02:53:42AM +0000, Pasha Tatashin wrote:
> Since:
> commit 9a10064f5625 ("mm: add a field to store names for private anonymous
> memory")
> 
> We can set names for private anonymous memory but not for shared
> anonymous memory. However, naming shared anonymous memory just as
> useful for tracking purposes.
> 
> Extend the functionality to be able to set names for shared anon.
> 
> / [anon_shmem:<name>]      an anonymous shared memory mapping that has
>                            been named by userspace
> 
> Sample output:
>         share = mmap(NULL, SIZE, PROT_READ | PROT_WRITE,
>                      MAP_SHARED | MAP_ANONYMOUS, -1, 0);
>         rv = prctl(PR_SET_VMA, PR_SET_VMA_ANON_NAME,
>                    share, SIZE, "shared anon");
> 
> /proc/<pid>/maps (and smaps):
> 7fc8e2b4c000-7fc8f2b4c000 rw-s 00000000 00:01 1024
> /dev/zero (deleted) [anon_shmem:shared anon]
> 
> pmap $(pgrep a.out)
> 254:   pub/a.out
> 000056093fab2000      4K r---- a.out
> 000056093fab3000      4K r-x-- a.out
> 000056093fab4000      4K r---- a.out
> 000056093fab5000      4K r---- a.out
> 000056093fab6000      4K rw--- a.out
> 000056093fdeb000    132K rw---   [ anon ]
> 00007fc8e2b4c000 262144K rw-s- zero (deleted) [anon_shmem:shared anon]
> 
> Signed-off-by: Pasha Tatashin <pasha.tatashin@soleen.com>
> ---
>  Documentation/filesystems/proc.rst |  4 +++-
>  fs/proc/task_mmu.c                 |  7 ++++---
>  include/linux/mm.h                 |  2 ++
>  include/linux/mm_types.h           | 27 +++++++++++++--------------
>  mm/madvise.c                       |  7 ++-----
>  mm/shmem.c                         | 13 +++++++++++--
>  6 files changed, 35 insertions(+), 25 deletions(-)
> 
> diff --git a/Documentation/filesystems/proc.rst b/Documentation/filesystems/proc.rst
> index 898c99eae8e4..8f1e68460da5 100644
> --- a/Documentation/filesystems/proc.rst
> +++ b/Documentation/filesystems/proc.rst
> @@ -431,8 +431,10 @@ is not associated with a file:
>   [stack]                    the stack of the main process
>   [vdso]                     the "virtual dynamic shared object",
>                              the kernel system call handler
> - [anon:<name>]              an anonymous mapping that has been
> + [anon:<name>]              a private anonymous mapping that has been
>                              named by userspace
> + path [anon_shmem:<name>]   an anonymous shared memory mapping that has
> +                            been named by userspace

I expect it to break existing parsers. If the field starts with '/' it is
reasonable to assume the rest of the string to be a path, but it is not
the case now.
Pasha Tatashin Nov. 6, 2022, 1:45 p.m. UTC | #4
On Sun, Nov 6, 2022 at 8:34 AM Kirill A. Shutemov <kirill@shutemov.name> wrote:
>
> On Sat, Nov 05, 2022 at 02:53:42AM +0000, Pasha Tatashin wrote:
> > Since:
> > commit 9a10064f5625 ("mm: add a field to store names for private anonymous
> > memory")
> >
> > We can set names for private anonymous memory but not for shared
> > anonymous memory. However, naming shared anonymous memory just as
> > useful for tracking purposes.
> >
> > Extend the functionality to be able to set names for shared anon.
> >
> > / [anon_shmem:<name>]      an anonymous shared memory mapping that has
> >                            been named by userspace
> >
> > Sample output:
> >         share = mmap(NULL, SIZE, PROT_READ | PROT_WRITE,
> >                      MAP_SHARED | MAP_ANONYMOUS, -1, 0);
> >         rv = prctl(PR_SET_VMA, PR_SET_VMA_ANON_NAME,
> >                    share, SIZE, "shared anon");
> >
> > /proc/<pid>/maps (and smaps):
> > 7fc8e2b4c000-7fc8f2b4c000 rw-s 00000000 00:01 1024
> > /dev/zero (deleted) [anon_shmem:shared anon]
> >
> > pmap $(pgrep a.out)
> > 254:   pub/a.out
> > 000056093fab2000      4K r---- a.out
> > 000056093fab3000      4K r-x-- a.out
> > 000056093fab4000      4K r---- a.out
> > 000056093fab5000      4K r---- a.out
> > 000056093fab6000      4K rw--- a.out
> > 000056093fdeb000    132K rw---   [ anon ]
> > 00007fc8e2b4c000 262144K rw-s- zero (deleted) [anon_shmem:shared anon]
> >
> > Signed-off-by: Pasha Tatashin <pasha.tatashin@soleen.com>
> > ---
> >  Documentation/filesystems/proc.rst |  4 +++-
> >  fs/proc/task_mmu.c                 |  7 ++++---
> >  include/linux/mm.h                 |  2 ++
> >  include/linux/mm_types.h           | 27 +++++++++++++--------------
> >  mm/madvise.c                       |  7 ++-----
> >  mm/shmem.c                         | 13 +++++++++++--
> >  6 files changed, 35 insertions(+), 25 deletions(-)
> >
> > diff --git a/Documentation/filesystems/proc.rst b/Documentation/filesystems/proc.rst
> > index 898c99eae8e4..8f1e68460da5 100644
> > --- a/Documentation/filesystems/proc.rst
> > +++ b/Documentation/filesystems/proc.rst
> > @@ -431,8 +431,10 @@ is not associated with a file:
> >   [stack]                    the stack of the main process
> >   [vdso]                     the "virtual dynamic shared object",
> >                              the kernel system call handler
> > - [anon:<name>]              an anonymous mapping that has been
> > + [anon:<name>]              a private anonymous mapping that has been
> >                              named by userspace
> > + path [anon_shmem:<name>]   an anonymous shared memory mapping that has
> > +                            been named by userspace
>
> I expect it to break existing parsers. If the field starts with '/' it is
> reasonable to assume the rest of the string to be a path, but it is not
> the case now.

This is actually exactly why I kept the "path" part. It stays the same
as today for  anon-shared memory, but prevents pmap to change
anon-shared memory from showing it as simply [anon].

Here is what we have today in /proc/<pid>/maps (and smaps):
7fc8e2b4c000-7fc8f2b4c000 rw-s 00000000 00:01 1024  /dev/zero (deleted)

So, the path points to /dev/zero but appended with (deleted) mark. The
pmap shows the same thing, as it is looking for leading '/' to
determine that this is a path.

With my change the above changes only when user specifically changed
the name like this:

7fc8e2b4c000-7fc8f2b4c000 rw-s 00000000 00:01 1024  /dev/zero
(deleted) [USER-SPECIFIED-NAME]

So, the path stays, the (deleted) mark stays, and a name is added.

Since this is anon memory, we can get rid of the /dev/zero path part
entirely and only print the name when the user specified one, but this
would change the output in pmap command to show all anonymous shared
memory as simply [anon].

Pasha

>
> --
>   Kiryl Shutsemau / Kirill A. Shutemov
Pasha Tatashin Nov. 6, 2022, 1:46 p.m. UTC | #5
On Sat, Nov 5, 2022 at 5:34 AM Bagas Sanjaya <bagasdotme@gmail.com> wrote:
>
> On Sat, Nov 05, 2022 at 02:53:42AM +0000, Pasha Tatashin wrote:
> > Since:
> > commit 9a10064f5625 ("mm: add a field to store names for private anonymous
> > memory")
> >
> > We can set names for private anonymous memory but not for shared
> > anonymous memory. However, naming shared anonymous memory just as
> > useful for tracking purposes.
> >
>
> Who are "we"?
>
> Instead, say "Since commit <commit>, name for private anonymous memory,
> but not shared anonymous, can be set".

Thanks will update.

>
> > @@ -431,8 +431,10 @@ is not associated with a file:
> >   [stack]                    the stack of the main process
> >   [vdso]                     the "virtual dynamic shared object",
> >                              the kernel system call handler
> > - [anon:<name>]              an anonymous mapping that has been
> > + [anon:<name>]              a private anonymous mapping that has been
> >                              named by userspace
> > + path [anon_shmem:<name>]   an anonymous shared memory mapping that has
> > +                            been named by userspace
> >   =============              ====================================
> >
>
> The table above triggers Sphinx warning:
>
> Documentation/filesystems/proc.rst:436: WARNING: Malformed table.
> Text in column margin in table line 8.
>
> =============              ====================================
> [heap]                     the heap of the program
> [stack]                    the stack of the main process
> [vdso]                     the "virtual dynamic shared object",
>                            the kernel system call handler
> [anon:<name>]              a private anonymous mapping that has been
>                            named by userspace
> path [anon_shmem:<name>]   an anonymous shared memory mapping that has
>                            been named by userspace
> =============              ====================================
>
> I have applied the fixup:
>
> ---- >8 ----
>
> diff --git a/Documentation/filesystems/proc.rst b/Documentation/filesystems/proc.rst
> index 8f1e68460da5cd..3f17b4ef307fe4 100644
> --- a/Documentation/filesystems/proc.rst
> +++ b/Documentation/filesystems/proc.rst
> @@ -426,7 +426,7 @@ with the memory region, as the case would be with BSS (uninitialized data).
>  The "pathname" shows the name associated file for this mapping.  If the mapping
>  is not associated with a file:
>
> - =============              ====================================
> + ========================   ===========================================
>   [heap]                     the heap of the program
>   [stack]                    the stack of the main process
>   [vdso]                     the "virtual dynamic shared object",
> @@ -435,7 +435,7 @@ is not associated with a file:
>                              named by userspace
>   path [anon_shmem:<name>]   an anonymous shared memory mapping that has
>                              been named by userspace
> - =============              ====================================
> + ========================   ===========================================
>
>   or if empty, the mapping is anonymous.
>
>
> Thanks.

Great, thank you.

Pasha

>
> --
> An old man doll... just what I always wanted! - Clara
Kirill A . Shutemov Nov. 6, 2022, 4:52 p.m. UTC | #6
On Sun, Nov 06, 2022 at 08:45:44AM -0500, Pasha Tatashin wrote:
> On Sun, Nov 6, 2022 at 8:34 AM Kirill A. Shutemov <kirill@shutemov.name> wrote:
> >
> > On Sat, Nov 05, 2022 at 02:53:42AM +0000, Pasha Tatashin wrote:
> > > Since:
> > > commit 9a10064f5625 ("mm: add a field to store names for private anonymous
> > > memory")
> > >
> > > We can set names for private anonymous memory but not for shared
> > > anonymous memory. However, naming shared anonymous memory just as
> > > useful for tracking purposes.
> > >
> > > Extend the functionality to be able to set names for shared anon.
> > >
> > > / [anon_shmem:<name>]      an anonymous shared memory mapping that has
> > >                            been named by userspace
> > >
> > > Sample output:
> > >         share = mmap(NULL, SIZE, PROT_READ | PROT_WRITE,
> > >                      MAP_SHARED | MAP_ANONYMOUS, -1, 0);
> > >         rv = prctl(PR_SET_VMA, PR_SET_VMA_ANON_NAME,
> > >                    share, SIZE, "shared anon");
> > >
> > > /proc/<pid>/maps (and smaps):
> > > 7fc8e2b4c000-7fc8f2b4c000 rw-s 00000000 00:01 1024
> > > /dev/zero (deleted) [anon_shmem:shared anon]
> > >
> > > pmap $(pgrep a.out)
> > > 254:   pub/a.out
> > > 000056093fab2000      4K r---- a.out
> > > 000056093fab3000      4K r-x-- a.out
> > > 000056093fab4000      4K r---- a.out
> > > 000056093fab5000      4K r---- a.out
> > > 000056093fab6000      4K rw--- a.out
> > > 000056093fdeb000    132K rw---   [ anon ]
> > > 00007fc8e2b4c000 262144K rw-s- zero (deleted) [anon_shmem:shared anon]
> > >
> > > Signed-off-by: Pasha Tatashin <pasha.tatashin@soleen.com>
> > > ---
> > >  Documentation/filesystems/proc.rst |  4 +++-
> > >  fs/proc/task_mmu.c                 |  7 ++++---
> > >  include/linux/mm.h                 |  2 ++
> > >  include/linux/mm_types.h           | 27 +++++++++++++--------------
> > >  mm/madvise.c                       |  7 ++-----
> > >  mm/shmem.c                         | 13 +++++++++++--
> > >  6 files changed, 35 insertions(+), 25 deletions(-)
> > >
> > > diff --git a/Documentation/filesystems/proc.rst b/Documentation/filesystems/proc.rst
> > > index 898c99eae8e4..8f1e68460da5 100644
> > > --- a/Documentation/filesystems/proc.rst
> > > +++ b/Documentation/filesystems/proc.rst
> > > @@ -431,8 +431,10 @@ is not associated with a file:
> > >   [stack]                    the stack of the main process
> > >   [vdso]                     the "virtual dynamic shared object",
> > >                              the kernel system call handler
> > > - [anon:<name>]              an anonymous mapping that has been
> > > + [anon:<name>]              a private anonymous mapping that has been
> > >                              named by userspace
> > > + path [anon_shmem:<name>]   an anonymous shared memory mapping that has
> > > +                            been named by userspace
> >
> > I expect it to break existing parsers. If the field starts with '/' it is
> > reasonable to assume the rest of the string to be a path, but it is not
> > the case now.
> 
> This is actually exactly why I kept the "path" part. It stays the same
> as today for  anon-shared memory, but prevents pmap to change
> anon-shared memory from showing it as simply [anon].
> 
> Here is what we have today in /proc/<pid>/maps (and smaps):
> 7fc8e2b4c000-7fc8f2b4c000 rw-s 00000000 00:01 1024  /dev/zero (deleted)
> 
> So, the path points to /dev/zero but appended with (deleted) mark. The
> pmap shows the same thing, as it is looking for leading '/' to
> determine that this is a path.
> 
> With my change the above changes only when user specifically changed
> the name like this:
> 
> 7fc8e2b4c000-7fc8f2b4c000 rw-s 00000000 00:01 1024  /dev/zero
> (deleted) [USER-SPECIFIED-NAME]
> 
> So, the path stays, the (deleted) mark stays, and a name is added.

Okay, fair enough.
Pasha Tatashin Nov. 7, 2022, 3:59 p.m. UTC | #7
On Sun, Nov 6, 2022 at 11:52 AM Kirill A. Shutemov <kirill@shutemov.name> wrote:
>
> On Sun, Nov 06, 2022 at 08:45:44AM -0500, Pasha Tatashin wrote:
> > On Sun, Nov 6, 2022 at 8:34 AM Kirill A. Shutemov <kirill@shutemov.name> wrote:
> > >
> > > On Sat, Nov 05, 2022 at 02:53:42AM +0000, Pasha Tatashin wrote:
> > > > Since:
> > > > commit 9a10064f5625 ("mm: add a field to store names for private anonymous
> > > > memory")
> > > >
> > > > We can set names for private anonymous memory but not for shared
> > > > anonymous memory. However, naming shared anonymous memory just as
> > > > useful for tracking purposes.
> > > >
> > > > Extend the functionality to be able to set names for shared anon.
> > > >
> > > > / [anon_shmem:<name>]      an anonymous shared memory mapping that has
> > > >                            been named by userspace
> > > >
> > > > Sample output:
> > > >         share = mmap(NULL, SIZE, PROT_READ | PROT_WRITE,
> > > >                      MAP_SHARED | MAP_ANONYMOUS, -1, 0);
> > > >         rv = prctl(PR_SET_VMA, PR_SET_VMA_ANON_NAME,
> > > >                    share, SIZE, "shared anon");
> > > >
> > > > /proc/<pid>/maps (and smaps):
> > > > 7fc8e2b4c000-7fc8f2b4c000 rw-s 00000000 00:01 1024
> > > > /dev/zero (deleted) [anon_shmem:shared anon]
> > > >
> > > > pmap $(pgrep a.out)
> > > > 254:   pub/a.out
> > > > 000056093fab2000      4K r---- a.out
> > > > 000056093fab3000      4K r-x-- a.out
> > > > 000056093fab4000      4K r---- a.out
> > > > 000056093fab5000      4K r---- a.out
> > > > 000056093fab6000      4K rw--- a.out
> > > > 000056093fdeb000    132K rw---   [ anon ]
> > > > 00007fc8e2b4c000 262144K rw-s- zero (deleted) [anon_shmem:shared anon]
> > > >
> > > > Signed-off-by: Pasha Tatashin <pasha.tatashin@soleen.com>
> > > > ---
> > > >  Documentation/filesystems/proc.rst |  4 +++-
> > > >  fs/proc/task_mmu.c                 |  7 ++++---
> > > >  include/linux/mm.h                 |  2 ++
> > > >  include/linux/mm_types.h           | 27 +++++++++++++--------------
> > > >  mm/madvise.c                       |  7 ++-----
> > > >  mm/shmem.c                         | 13 +++++++++++--
> > > >  6 files changed, 35 insertions(+), 25 deletions(-)
> > > >
> > > > diff --git a/Documentation/filesystems/proc.rst b/Documentation/filesystems/proc.rst
> > > > index 898c99eae8e4..8f1e68460da5 100644
> > > > --- a/Documentation/filesystems/proc.rst
> > > > +++ b/Documentation/filesystems/proc.rst
> > > > @@ -431,8 +431,10 @@ is not associated with a file:
> > > >   [stack]                    the stack of the main process
> > > >   [vdso]                     the "virtual dynamic shared object",
> > > >                              the kernel system call handler
> > > > - [anon:<name>]              an anonymous mapping that has been
> > > > + [anon:<name>]              a private anonymous mapping that has been
> > > >                              named by userspace
> > > > + path [anon_shmem:<name>]   an anonymous shared memory mapping that has
> > > > +                            been named by userspace
> > >
> > > I expect it to break existing parsers. If the field starts with '/' it is
> > > reasonable to assume the rest of the string to be a path, but it is not
> > > the case now.
> >
> > This is actually exactly why I kept the "path" part. It stays the same
> > as today for  anon-shared memory, but prevents pmap to change
> > anon-shared memory from showing it as simply [anon].
> >
> > Here is what we have today in /proc/<pid>/maps (and smaps):
> > 7fc8e2b4c000-7fc8f2b4c000 rw-s 00000000 00:01 1024  /dev/zero (deleted)
> >
> > So, the path points to /dev/zero but appended with (deleted) mark. The
> > pmap shows the same thing, as it is looking for leading '/' to
> > determine that this is a path.
> >
> > With my change the above changes only when user specifically changed
> > the name like this:
> >
> > 7fc8e2b4c000-7fc8f2b4c000 rw-s 00000000 00:01 1024  /dev/zero
> > (deleted) [USER-SPECIFIED-NAME]
> >
> > So, the path stays, the (deleted) mark stays, and a name is added.
>
> Okay, fair enough.

After thinking about this, it makes sense to remove "path" entirely.
The pmap without arguments will show the user named segment as
"[anon]", but with -X argument it will show the full name:

pmap -X
7fa84fcef000 rw-s 00000000  00:01      1024 262144    0   0         0
        0         0        0              0             0
0               0    0       0      0           0 [anon_shmem:named
shared anon]
7fa85fcef000 rw-p 00000000  00:00         0 262144    0   0         0
        0         0        0              0             0
0               0    0       0      0           0 [anon:named anon]

In my opinion this is better to stay consistent with regular anon
memory, and also to minimize the chance to surprise the existing
scripts.

Pasha

>
> --
>   Kiryl Shutsemau / Kirill A. Shutemov
diff mbox series

Patch

diff --git a/Documentation/filesystems/proc.rst b/Documentation/filesystems/proc.rst
index 898c99eae8e4..8f1e68460da5 100644
--- a/Documentation/filesystems/proc.rst
+++ b/Documentation/filesystems/proc.rst
@@ -431,8 +431,10 @@  is not associated with a file:
  [stack]                    the stack of the main process
  [vdso]                     the "virtual dynamic shared object",
                             the kernel system call handler
- [anon:<name>]              an anonymous mapping that has been
+ [anon:<name>]              a private anonymous mapping that has been
                             named by userspace
+ path [anon_shmem:<name>]   an anonymous shared memory mapping that has
+                            been named by userspace
  =============              ====================================
 
  or if empty, the mapping is anonymous.
diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
index 8a74cdcc9af0..d6ae75ed81ca 100644
--- a/fs/proc/task_mmu.c
+++ b/fs/proc/task_mmu.c
@@ -277,6 +277,7 @@  show_map_vma(struct seq_file *m, struct vm_area_struct *vma)
 	struct mm_struct *mm = vma->vm_mm;
 	struct file *file = vma->vm_file;
 	vm_flags_t flags = vma->vm_flags;
+	struct anon_vma_name *anon_name;
 	unsigned long ino = 0;
 	unsigned long long pgoff = 0;
 	unsigned long start, end;
@@ -293,6 +294,7 @@  show_map_vma(struct seq_file *m, struct vm_area_struct *vma)
 	start = vma->vm_start;
 	end = vma->vm_end;
 	show_vma_header_prefix(m, start, end, flags, pgoff, dev, ino);
+	anon_name = anon_vma_name(vma);
 
 	/*
 	 * Print the dentry name for named mappings, and a
@@ -301,6 +303,8 @@  show_map_vma(struct seq_file *m, struct vm_area_struct *vma)
 	if (file) {
 		seq_pad(m, ' ');
 		seq_file_path(m, file, "\n");
+		if (anon_name)
+			seq_printf(m, " [anon_shmem:%s]", anon_name->name);
 		goto done;
 	}
 
@@ -312,8 +316,6 @@  show_map_vma(struct seq_file *m, struct vm_area_struct *vma)
 
 	name = arch_vma_name(vma);
 	if (!name) {
-		struct anon_vma_name *anon_name;
-
 		if (!mm) {
 			name = "[vdso]";
 			goto done;
@@ -330,7 +332,6 @@  show_map_vma(struct seq_file *m, struct vm_area_struct *vma)
 			goto done;
 		}
 
-		anon_name = anon_vma_name(vma);
 		if (anon_name) {
 			seq_pad(m, ' ');
 			seq_printf(m, "[anon:%s]", anon_name->name);
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 8bbcccbc5565..06b6fb3277ab 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -699,8 +699,10 @@  static inline unsigned long vma_iter_addr(struct vma_iterator *vmi)
  * paths in userfault.
  */
 bool vma_is_shmem(struct vm_area_struct *vma);
+bool vma_is_anon_shmem(struct vm_area_struct *vma);
 #else
 static inline bool vma_is_shmem(struct vm_area_struct *vma) { return false; }
+static inline bool vma_is_anon_shmem(struct vm_area_struct *vma) { return false; }
 #endif
 
 int vma_is_stack_for_current(struct vm_area_struct *vma);
diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
index 500e536796ca..08d8b973fb60 100644
--- a/include/linux/mm_types.h
+++ b/include/linux/mm_types.h
@@ -461,21 +461,11 @@  struct vm_area_struct {
 	 * For areas with an address space and backing store,
 	 * linkage into the address_space->i_mmap interval tree.
 	 *
-	 * For private anonymous mappings, a pointer to a null terminated string
-	 * containing the name given to the vma, or NULL if unnamed.
 	 */
-
-	union {
-		struct {
-			struct rb_node rb;
-			unsigned long rb_subtree_last;
-		} shared;
-		/*
-		 * Serialized by mmap_sem. Never use directly because it is
-		 * valid only when vm_file is NULL. Use anon_vma_name instead.
-		 */
-		struct anon_vma_name *anon_name;
-	};
+	struct {
+		struct rb_node rb;
+		unsigned long rb_subtree_last;
+	} shared;
 
 	/*
 	 * A file's MAP_PRIVATE vma can be in both i_mmap tree and anon_vma
@@ -485,6 +475,7 @@  struct vm_area_struct {
 	 */
 	struct list_head anon_vma_chain; /* Serialized by mmap_lock &
 					  * page_table_lock */
+
 	struct anon_vma *anon_vma;	/* Serialized by page_table_lock */
 
 	/* Function pointers to deal with this struct. */
@@ -496,6 +487,14 @@  struct vm_area_struct {
 	struct file * vm_file;		/* File we map to (can be NULL). */
 	void * vm_private_data;		/* was vm_pte (shared mem) */
 
+#ifdef CONFIG_ANON_VMA_NAME
+	/*
+	 * For private and shared anonymous mappings, a pointer to a null
+	 * terminated string containing the name given to the vma, or NULL if
+	 * unnamed. Serialized by mmap_sem. Use anon_vma_name to access.
+	 */
+	struct anon_vma_name *anon_name;
+#endif
 #ifdef CONFIG_SWAP
 	atomic_long_t swap_readahead_info;
 #endif
diff --git a/mm/madvise.c b/mm/madvise.c
index c7105ec6d08c..255d5b485432 100644
--- a/mm/madvise.c
+++ b/mm/madvise.c
@@ -95,9 +95,6 @@  struct anon_vma_name *anon_vma_name(struct vm_area_struct *vma)
 {
 	mmap_assert_locked(vma->vm_mm);
 
-	if (vma->vm_file)
-		return NULL;
-
 	return vma->anon_name;
 }
 
@@ -183,7 +180,7 @@  static int madvise_update_vma(struct vm_area_struct *vma,
 	 * vm_flags is protected by the mmap_lock held in write mode.
 	 */
 	vma->vm_flags = new_flags;
-	if (!vma->vm_file) {
+	if (!vma->vm_file || vma_is_anon_shmem(vma)) {
 		error = replace_anon_vma_name(vma, anon_name);
 		if (error)
 			return error;
@@ -1273,7 +1270,7 @@  static int madvise_vma_anon_name(struct vm_area_struct *vma,
 	int error;
 
 	/* Only anonymous mappings can be named */
-	if (vma->vm_file)
+	if (vma->vm_file && !vma_is_anon_shmem(vma))
 		return -EBADF;
 
 	error = madvise_update_vma(vma, prev, start, end, vma->vm_flags,
diff --git a/mm/shmem.c b/mm/shmem.c
index c1d8b8a1aa3b..638bcb3d26bd 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -237,11 +237,17 @@  static const struct inode_operations shmem_inode_operations;
 static const struct inode_operations shmem_dir_inode_operations;
 static const struct inode_operations shmem_special_inode_operations;
 static const struct vm_operations_struct shmem_vm_ops;
+static const struct vm_operations_struct shmem_anon_vm_ops;
 static struct file_system_type shmem_fs_type;
 
+bool vma_is_anon_shmem(struct vm_area_struct *vma)
+{
+	return vma->vm_ops == &shmem_anon_vm_ops;
+}
+
 bool vma_is_shmem(struct vm_area_struct *vma)
 {
-	return vma->vm_ops == &shmem_vm_ops;
+	return vma_is_anon_shmem(vma) || vma->vm_ops == &shmem_vm_ops;
 }
 
 static LIST_HEAD(shmem_swaplist);
@@ -3995,6 +4001,8 @@  static const struct vm_operations_struct shmem_vm_ops = {
 #endif
 };
 
+static const struct vm_operations_struct shmem_anon_vm_ops = shmem_vm_ops;
+
 int shmem_init_fs_context(struct fs_context *fc)
 {
 	struct shmem_options *ctx;
@@ -4170,6 +4178,7 @@  void shmem_truncate_range(struct inode *inode, loff_t lstart, loff_t lend)
 EXPORT_SYMBOL_GPL(shmem_truncate_range);
 
 #define shmem_vm_ops				generic_file_vm_ops
+#define shmem_anon_vm_ops			generic_file_vm_ops
 #define shmem_file_operations			ramfs_file_operations
 #define shmem_get_inode(sb, dir, mode, dev, flags)	ramfs_get_inode(sb, dir, mode, dev)
 #define shmem_acct_size(flags, size)		0
@@ -4275,7 +4284,7 @@  int shmem_zero_setup(struct vm_area_struct *vma)
 	if (vma->vm_file)
 		fput(vma->vm_file);
 	vma->vm_file = file;
-	vma->vm_ops = &shmem_vm_ops;
+	vma->vm_ops = &shmem_anon_vm_ops;
 
 	return 0;
 }