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: <20221107184715.3950621-1-pasha.tatashin@soleen.com> <e94ac231-7137-010c-2f2b-6a309c941759@redhat.com>
In-Reply-To: <e94ac231-7137-010c-2f2b-6a309c941759@redhat.com>
From:   Pasha Tatashin <pasha.tatashin@soleen.com>
Date:   Tue, 8 Nov 2022 13:44:58 -0500
Message-ID: <CA+CK2bAbKMj8-crNCtmQ=DB5uRvQBJtFTLf5TH9=RWRGjfOGew@mail.gmail.com>
Subject: Re: [PATCH v2] mm: anonymous shared memory naming
To:     David Hildenbrand <david@redhat.com>
Cc:     corbet@lwn.net, akpm@linux-foundation.org, hughd@google.com,
        hannes@cmpxchg.org, 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,
        bagasdotme@gmail.com, kirill@shutemov.name
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hi David,

Thank you for taking a look at this change:

On Tue, Nov 8, 2022 at 12:56 PM David Hildenbrand <david@redhat.com> wrote:
>
> On 07.11.22 19:47, Pasha Tatashin wrote:
> > Since: commit 9a10064f5625 ("mm: add a field to store names for private
> > anonymous memory"), name for private anonymous memory, but not shared
> > anonymous, can be set. However, naming shared anonymous memory just as
>
>                                                                   ^ is
>

OK

> > useful for tracking purposes.
> >
> > Extend the functionality to be able to set names for shared anon.
> >
> > Sample output:
> >    /* Create shared anonymous segmenet */
>
> s/segmenet/segment/

Ok

>
> >    anon_shmem = mmap(NULL, SIZE, PROT_READ | PROT_WRITE,
> >                      MAP_SHARED | MAP_ANONYMOUS, -1, 0);
> >    /* Name the segment: "MY-NAME" */
> >    rv = prctl(PR_SET_VMA, PR_SET_VMA_ANON_NAME,
> >               anon_shmem, SIZE, "MY-NAME");
> >
> > cat /proc/<pid>/maps (and smaps):
> > 7fc8e2b4c000-7fc8f2b4c000 rw-s 00000000 00:01 1024 [anon_shmem:MY-NAME]
>
> What would it have looked like before? Just no additional information?

Before:

7fc8e2b4c000-7fc8f2b4c000 rw-s 00000000 00:01 1024 /dev/zero (deleted)

Pasha

>
> >
> > Signed-off-by: Pasha Tatashin <pasha.tatashin@soleen.com>
> > ---
>
>
> [...]
>
> > 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;
> >
>
> So that effectively grows the size of vm_area_struct. Hm. I'd really
> prefer to keep this specific to actual anonymous memory, not extending
> it to anonymous files.

It grows only when CONFIG_ANON_VMA_NAME=y, otherwise it stays the same
as before. Are you suggesting adding another config specifically for
shared memory? I wonder if we could add a union for some other part of
vm_area_struct where anon and file cannot be used together.

> Do we have any *actual* users where we don't have an alternative? I
> doubt that this is really required.
>
> The simplest approach seems to be to use memfd instead of MAP_SHARED |
> MAP_ANONYMOUS. __NR_memfd_create can be passed a name and you get what
> you propose here effectively already. Or does anything speak against it?

For our use case the above does not work. We are working on highly
paravirtualized virtual machines. The VMM maps VM memory as anonymous
shared memory (not private because VMM is sandboxed and drivers are
running in their own processes). However, the VM tells back to the VMM
how parts of the memory are actually used by the guest, how each of
the segments should be backed (i.e. 4K pages, 2M pages), and some
other information about the segments. The naming allows us to monitor
the effective memory footprint for each of these segments from the
host without looking inside the guest.

Pasha