linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Ross Zwisler <ross.zwisler@linux.intel.com>
To: Jan Kara <jack@suse.cz>
Cc: Dan Williams <dan.j.williams@intel.com>,
	Ross Zwisler <ross.zwisler@linux.intel.com>,
	Christoph Hellwig <hch@infradead.org>,
	linux-ext4@vger.kernel.org, linux-nvdimm@lists.01.org,
	linux-fsdevel@vger.kernel.org, linux-xfs@vger.kernel.org,
	linux-api@vger.kernel.org, linux-mm@kvack.org,
	Arnd Bergmann <arnd@arndb.de>, Andy Lutomirski <luto@kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>
Subject: Re: [PATCH 01/17] mm: introduce MAP_SHARED_VALIDATE, a mechanism to safely define new mmap flags
Date: Tue, 24 Oct 2017 15:21:19 -0600	[thread overview]
Message-ID: <20171024212119.GB1611@linux.intel.com> (raw)
In-Reply-To: <20171024152415.22864-2-jack@suse.cz>

On Tue, Oct 24, 2017 at 05:23:58PM +0200, Jan Kara wrote:
> From: Dan Williams <dan.j.williams@intel.com>
> 
> The mmap(2) syscall suffers from the ABI anti-pattern of not validating
> unknown flags. However, proposals like MAP_SYNC need a mechanism to
> define new behavior that is known to fail on older kernels without the
> support. Define a new MAP_SHARED_VALIDATE flag pattern that is
> guaranteed to fail on all legacy mmap implementations.
> 
> It is worth noting that the original proposal was for a standalone
> MAP_VALIDATE flag. However, when that  could not be supported by all
> archs Linus observed:
> 
>     I see why you *think* you want a bitmap. You think you want
>     a bitmap because you want to make MAP_VALIDATE be part of MAP_SYNC
>     etc, so that people can do
> 
>     ret = mmap(NULL, size, PROT_READ | PROT_WRITE, MAP_SHARED
> 		    | MAP_SYNC, fd, 0);
> 
>     and "know" that MAP_SYNC actually takes.
> 
>     And I'm saying that whole wish is bogus. You're fundamentally
>     depending on special semantics, just make it explicit. It's already
>     not portable, so don't try to make it so.
> 
>     Rename that MAP_VALIDATE as MAP_SHARED_VALIDATE, make it have a value
>     of 0x3, and make people do
> 
>     ret = mmap(NULL, size, PROT_READ | PROT_WRITE, MAP_SHARED_VALIDATE
> 		    | MAP_SYNC, fd, 0);
> 
>     and then the kernel side is easier too (none of that random garbage
>     playing games with looking at the "MAP_VALIDATE bit", but just another
>     case statement in that map type thing.
> 
>     Boom. Done.
> 
> Similar to ->fallocate() we also want the ability to validate the
> support for new flags on a per ->mmap() 'struct file_operations'
> instance basis.  Towards that end arrange for flags to be generically
> validated against a mmap_supported_flags exported by 'struct
> file_operations'. By default all existing flags are implicitly
> supported, but new flags require MAP_SHARED_VALIDATE and
> per-instance-opt-in.
> 
> Cc: Jan Kara <jack@suse.cz>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Andy Lutomirski <luto@kernel.org>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Suggested-by: Christoph Hellwig <hch@lst.de>
> Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> Signed-off-by: Jan Kara <jack@suse.cz>

Looks great.

Reviewed-by: Ross Zwisler <ross.zwisler@linux.intel.com>

--
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-10-24 21:21 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-10-24 15:23 [PATCH 0/17 v5] dax, ext4, xfs: Synchronous page faults Jan Kara
2017-10-24 15:23 ` [PATCH 01/17] mm: introduce MAP_SHARED_VALIDATE, a mechanism to safely define new mmap flags Jan Kara
2017-10-24 21:21   ` Ross Zwisler [this message]
2017-10-24 15:23 ` [PATCH 02/17] mm: Remove VM_FAULT_HWPOISON_LARGE_MASK Jan Kara
2017-10-24 15:24 ` [PATCH 03/17] dax: Simplify arguments of dax_insert_mapping() Jan Kara
2017-10-24 15:24 ` [PATCH 04/17] dax: Factor out getting of pfn out of iomap Jan Kara
2017-10-24 15:24 ` [PATCH 05/17] dax: Create local variable for VMA in dax_iomap_pte_fault() Jan Kara
2017-10-24 15:24 ` [PATCH 06/17] dax: Create local variable for vmf->flags & FAULT_FLAG_WRITE test Jan Kara
2017-10-24 15:24 ` [PATCH 07/17] dax: Inline dax_insert_mapping() into the callsite Jan Kara
2017-10-24 15:24 ` [PATCH 08/17] dax: Inline dax_pmd_insert_mapping() " Jan Kara
2017-10-24 15:24 ` [PATCH 09/17] dax: Fix comment describing dax_iomap_fault() Jan Kara
2017-10-24 15:24 ` [PATCH 10/17] dax: Allow dax_iomap_fault() to return pfn Jan Kara
2017-10-24 15:24 ` [PATCH 11/17] dax: Allow tuning whether dax_insert_mapping_entry() dirties entry Jan Kara
2017-10-24 15:24 ` [PATCH 12/17] mm: Define MAP_SYNC and VM_SYNC flags Jan Kara
2017-10-24 15:24 ` [PATCH 13/17] dax, iomap: Add support for synchronous faults Jan Kara
2017-10-24 15:24 ` [PATCH 14/17] dax: Implement dax_finish_sync_fault() Jan Kara
2017-10-24 15:24 ` [PATCH 15/17] ext4: Simplify error handling in ext4_dax_huge_fault() Jan Kara
2017-10-24 15:24 ` [PATCH 16/17] ext4: Support for synchronous DAX faults Jan Kara
2017-10-24 15:24 ` [PATCH 17/17] xfs: support " Jan Kara
2017-10-24 21:29   ` Ross Zwisler
2017-10-24 22:23   ` Dave Chinner
2017-10-26 15:48     ` Jan Kara
2017-10-26 21:16       ` Dave Chinner
2017-10-27 10:08         ` Jan Kara
2017-10-31 15:19           ` Jan Kara
2017-10-31 21:50             ` Dan Williams
2017-11-01  3:47               ` Ross Zwisler
2017-10-27  6:43       ` Christoph Hellwig
2017-10-27  9:13         ` Jan Kara
2017-10-24 15:24 ` [PATCH] mmap.2: Add description of MAP_SHARED_VALIDATE and MAP_SYNC Jan Kara
2017-10-24 21:10   ` Ross Zwisler
2017-10-26 13:24     ` Jan Kara
2017-10-26 18:22       ` Ross Zwisler

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=20171024212119.GB1611@linux.intel.com \
    --to=ross.zwisler@linux.intel.com \
    --cc=akpm@linux-foundation.org \
    --cc=arnd@arndb.de \
    --cc=dan.j.williams@intel.com \
    --cc=hch@infradead.org \
    --cc=jack@suse.cz \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux-nvdimm@lists.01.org \
    --cc=linux-xfs@vger.kernel.org \
    --cc=luto@kernel.org \
    /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).