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;
Message-ID: <e94ac231-7137-010c-2f2b-6a309c941759@redhat.com>
Date:   Tue, 8 Nov 2022 18:56:48 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.4.0
Content-Language: en-US
To:     Pasha Tatashin <pasha.tatashin@soleen.com>, 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
References: <20221107184715.3950621-1-pasha.tatashin@soleen.com>
From:   David Hildenbrand <david@redhat.com>
Organization: Red Hat
Subject: Re: [PATCH v2] mm: anonymous shared memory naming
In-Reply-To: <20221107184715.3950621-1-pasha.tatashin@soleen.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

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

> 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/

>    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?

> 
> 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.

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?

-- 
Thanks,

David / dhildenb