Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20221105025342.3130038-1-pasha.tatashin@soleen.com>
 <20221106133351.ukb5quoizkkzyrge@box.shutemov.name> <CA+CK2bDK=oUYM-HZsYfZoq_n5BQMGpysMq395WK78r+SwYk99A@mail.gmail.com>
 <20221106165204.odb7febmnykhna2h@box.shutemov.name>
In-Reply-To: <20221106165204.odb7febmnykhna2h@box.shutemov.name>
From:   Pasha Tatashin <pasha.tatashin@soleen.com>
Date:   Mon, 7 Nov 2022 10:59:08 -0500
Message-ID: <CA+CK2bBQik_=a5vuaXs+=Zrnod5bn8XcKHdz=aqsudhcS=PijA@mail.gmail.com>
Subject: Re: [PATCH] mm: anonymous shared memory naming
To:     "Kirill A. Shutemov" <kirill@shutemov.name>
Cc:     corbet@lwn.net, akpm@linux-foundation.org, hughd@google.com,
        hannes@cmpxchg.org, david@redhat.com, vincent.whitchurch@axis.com,
        seanjc@google.com, rppt@kernel.org, shy828301@gmail.com,
        paul.gortmaker@windriver.com, peterx@redhat.com, vbabka@suse.cz,
        Liam.Howlett@oracle.com, ccross@google.com, willy@infradead.org,
        arnd@arndb.de, cgel.zte@gmail.com, yuzhao@google.com,
        linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
        linux-doc@vger.kernel.org, linux-mm@kvack.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

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