Re: [PATCH] mm: add sys_madvise2 and MADV_NAME to name vmas

[Oleg Nesterov <oleg@redhat.com>]

On 07/03, Colin Cross wrote:
>
> The names of named anonymous vmas are shown in /proc/pid/maps
> as [anon:<name>].  The name of all named vmas are shown in
> /proc/pid/smaps in a new "Name" field that is only present
> for named vmas.

And this is the only purpose, yes?

>  static long madvise_behavior(struct vm_area_struct * vma,
>  		     struct vm_area_struct **prev,
> -		     unsigned long start, unsigned long end, int behavior)
> +		     unsigned long start, unsigned long end, int behavior,
> +		     void *arg, size_t arg_len)
>  {
>  	struct mm_struct * mm = vma->vm_mm;
>  	int error = 0;
>  	pgoff_t pgoff;
>  	unsigned long new_flags = vma->vm_flags;
> +	struct vma_name *new_name = vma->vm_name;
>  
>  	switch (behavior) {
>  	case MADV_NORMAL:
> @@ -93,16 +97,28 @@ static long madvise_behavior(struct vm_area_struct * vma,
>  		if (error)
>  			goto out;
>  		break;
> +	case MADV_NAME:
> +		if (arg) {
> +			new_name = vma_name_get_from_str(arg, arg_len);
> +			if (!new_name) {
> +				error = -ENOMEM;
> +				goto out;
> +			}
> +		} else {
> +			new_name = NULL;
> +		}
> +		break;
>  	}
>  
> -	if (new_flags == vma->vm_flags) {
> +	if (new_flags == vma->vm_flags && new_name == vma->vm_name) {
>  		*prev = vma;
>  		goto out;
>  	}
>  
>  	pgoff = vma->vm_pgoff + ((start - vma->vm_start) >> PAGE_SHIFT);
>  	*prev = vma_merge(mm, *prev, start, end, new_flags, vma->anon_vma,
> -				vma->vm_file, pgoff, vma_policy(vma));
> +				vma->vm_file, pgoff, vma_policy(vma),
> +				new_name);
>  	if (*prev) {
>  		vma = *prev;
>  		goto success;
> @@ -127,8 +143,17 @@ success:
>  	 * vm_flags is protected by the mmap_sem held in write mode.
>  	 */
>  	vma->vm_flags = new_flags;
> +	if (vma->vm_name != new_name) {
> +		if (vma->vm_name)
> +			vma_name_put(vma->vm_name);
> +		if (new_name)
> +			vma_name_get(new_name);
> +		vma->vm_name = new_name;
> +	}

So we change vma->vm_name after vma_merge(). But given that is_mergeable_vma()
checks vma->vm_name with this patch, this means that we can have 2 vma's with
the same ->vm_name which should be merged?

IOW. Suppose that we have vma with vm_start = START, end = START + 2 * PAGE_SIZE.
Suppose that an application does

	MADV_NAME(START, PAGE_SIZE, "MY_NAME");
	MADV_NAME(START + PAGE_SIZE, PAGE_SIZE, "MY_NAME");

The 1st MADV_NAME will split this vma, the 2nd won't merge. Not that I think
this is buggy, just a bit inconsistent imho.

And I guess vma_name_get(new_name) is not needed, you can simply nullify it
after changing ->vm_name to avoid vma_name_put() below.

Oleg.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>