linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Mike Kravetz <mike.kravetz@oracle.com>
To: Michal Hocko <mhocko@kernel.org>
Cc: Vlastimil Babka <vbabka@suse.cz>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	Andrew Morton <akpm@linux-foundation.org>,
	Andrea Arcangeli <aarcange@redhat.com>,
	Aaron Lu <aaron.lu@intel.com>,
	"Kirill A . Shutemov" <kirill.shutemov@linux.intel.com>,
	Anshuman Khandual <khandual@linux.vnet.ibm.com>,
	Linux API <linux-api@vger.kernel.org>
Subject: Re: [PATCH] mm/mremap: Fail map duplication attempts for private mappings
Date: Fri, 14 Jul 2017 10:29:01 -0700	[thread overview]
Message-ID: <146116f3-c318-efc0-de40-f67655cbbf94@oracle.com> (raw)
In-Reply-To: <20170714082629.GA2618@dhcp22.suse.cz>

On 07/14/2017 01:26 AM, Michal Hocko wrote:
> On Thu 13-07-17 15:33:47, Mike Kravetz wrote:
>> On 07/13/2017 12:11 PM, Vlastimil Babka wrote:
>>> [+CC linux-api]
>>>
>>> On 07/13/2017 05:58 PM, Mike Kravetz wrote:
>>>> mremap will create a 'duplicate' mapping if old_size == 0 is
>>>> specified.  Such duplicate mappings make no sense for private
>>>> mappings.  If duplication is attempted for a private mapping,
>>>> mremap creates a separate private mapping unrelated to the
>>>> original mapping and makes no modifications to the original.
>>>> This is contrary to the purpose of mremap which should return
>>>> a mapping which is in some way related to the original.
>>>>
>>>> Therefore, return EINVAL in the case where if an attempt is
>>>> made to duplicate a private mapping.
>>>>
>>>> Signed-off-by: Mike Kravetz <mike.kravetz@oracle.com>
>>>
>>> Acked-by: Vlastimil Babka <vbabka@suse.cz>
>>>
>>
>> In another e-mail thread, Andrea makes the case that mremap(old_size == 0)
>> of private file backed mappings could possibly be used for something useful.
>> For example to create a private COW mapping.
> 
> What does this mean exactly? I do not see it would force CoW so again
> the new mapping could fail with the basic invariant that the content
> of the new mapping should match the old one (e.g. old mapping already
> CoWed some pages the new mapping would still contain the origin content
> unless I am missing something).

I do not think you are missing anything.  You are correct in saying that
the new mapping would be COW of the original file contents.  It is NOT
based on any private pages of the old private mapping.  Sorry, my wording
above was not quite clear.

As previously discussed, the more straight forward to way to accomplish
the same thing would be a simple call to mmap with the fd.

After thinking about this some more, perhaps the original patch to return
EINVAL for all private mappings makes more sense.  Even in the case of a
file backed private mapping, the new mapping will be based on the file and
not the old mapping.  The purpose of mremap is to create a new mapping
based on the old mapping.  So, this is not strictly in line with the purpose
of mremap.

Actually, the more I think about this, the more I wish there was some way
to deprecate and eventually eliminate the old_size == 0 behavior.

> [...]
>> +	/*
>> +	 * !old_len  is a special case where a mapping is 'duplicated'.
>> +	 * Do not allow this for private anon mappings.
>> +	 */
>> +	if (!old_len && vma_is_anonymous(vma) &&
>> +	    !(vma->vm_flags & (VM_SHARED | VM_MAYSHARE)))
>> +		return ERR_PTR(-EINVAL);
> 
> Why is vma_is_anonymous() without VM_*SHARE* check insufficient?

Are you asking,
why is if (!old_len && vma_is_anonymous(vma)) insufficient?

If so, you are correct that the additional check for VM_*SHARE* is not
necessary.  Shared mappings are technically not anonymous as they must
contain a common backing object.

The !(vma->vm_flags & (VM_SHARED | VM_MAYSHARE) check was there in the first
patch to catch all private mappings.  When adding vma_is_anonymous(vma), I
missed the fact that it was redundant.  But, based on your comments above
I think the first patch is more correct.

-- 
Mike Kravetz

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

  reply	other threads:[~2017-07-14 17:29 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-07-13 15:58 [PATCH] mm/mremap: Fail map duplication attempts for private mappings Mike Kravetz
2017-07-13 19:11 ` Vlastimil Babka
2017-07-13 22:33   ` Mike Kravetz
2017-07-14  4:51     ` Anshuman Khandual
2017-07-14  8:26     ` Michal Hocko
2017-07-14 17:29       ` Mike Kravetz [this message]
2017-07-17  6:44         ` Michal Hocko
2017-07-19 16:39   ` Mike Kravetz
2017-07-20  8:20     ` Michal Hocko
2017-07-20 20:37       ` [PATCH v2] " Mike Kravetz
2017-07-21 14:36         ` Michal Hocko
2017-07-21 21:18           ` Mike Kravetz
2017-07-24  8:50             ` Michal Hocko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=146116f3-c318-efc0-de40-f67655cbbf94@oracle.com \
    --to=mike.kravetz@oracle.com \
    --cc=aarcange@redhat.com \
    --cc=aaron.lu@intel.com \
    --cc=akpm@linux-foundation.org \
    --cc=khandual@linux.vnet.ibm.com \
    --cc=kirill.shutemov@linux.intel.com \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mhocko@kernel.org \
    --cc=vbabka@suse.cz \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).