All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christoph Hellwig <hch@lst.de>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Jan Kara <jack@suse.cz>, Arnd Bergmann <arnd@arndb.de>,
	"linux-nvdimm@lists.01.org" <linux-nvdimm@lists.01.org>,
	Linux API <linux-api@vger.kernel.org>,
	Christoph Hellwig <hch@lst.de>,
	linux-xfs <linux-xfs@vger.kernel.org>,
	linux-mm <linux-mm@kvack.org>, Andy Lutomirski <luto@kernel.org>,
	linux-fsdevel <linux-fsdevel@vger.kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>
Subject: Re: [PATCH v9 1/6] mm: introduce MAP_SHARED_VALIDATE, a mechanism to safely define new mmap flags
Date: Mon, 16 Oct 2017 09:38:36 +0200	[thread overview]
Message-ID: <20171016073836.GB28778@lst.de> (raw)
In-Reply-To: <CA+55aFyy-nz99c6erFh=aeyCOzsk0td5wHaVLpwBNA-sWNDZkA@mail.gmail.com>

On Thu, Oct 12, 2017 at 09:32:17AM -0700, Linus Torvalds wrote:
> On Thu, Oct 12, 2017 at 6:51 AM, Jan Kara <jack@suse.cz> wrote:
> >
> > When thinking a bit more about this I've realized one problem: Currently
> > user can call mmap() with MAP_SHARED type and MAP_SYNC or MAP_DIRECT flags
> > and he will get the new semantics (if the kernel happens to support it).  I
> > think that is undesirable [..]
> 
> Why?
> 
> If you have a performance preference for MAP_DIRECT or something like
> that, but you don't want to *enforce* it, you'd use just plain
> MAP_SHARED with it.
> 
> Ie there may well be "I want this to work, possibly with downsides" issues.
> 
> So it seems to be a reasonable model, and disallowing it seems to
> limit people and not really help anything.

I don't think for MAP_DIRECT it matters (and I think we shouldn't have
MAP_DIRECT to start with, see the discussions later in the thread).

But for the main use case, MAP_SYNC you really want a hard error when you
don't get it.  And while we could tell people that they should only use
MAP_SYNC with MAP_SHARED_VALIDATE instead of MAP_SHARED chances that they
get it wrong are extremely high.  On the other hand if you really only
want a flag to optimize calling mmap twice is very little overhead, and
a very good documentation of you intent:

	addr = mmap(...., MAP_SHARED_VALIDATE | MAP_DIRECT, ...);
	if (!addr && errno = EOPNOTSUPP) {
		/* MAP_DIRECT didn't work, we'll just cope using blah, blah */
		addr = mmap(...., MAP_SHARED, ...);
	}
	if (!addr)
		goto handle_error;
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

WARNING: multiple messages have this Message-ID (diff)
From: Christoph Hellwig <hch@lst.de>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Jan Kara <jack@suse.cz>, Dan Williams <dan.j.williams@intel.com>,
	"linux-nvdimm@lists.01.org" <linux-nvdimm@lists.01.org>,
	Arnd Bergmann <arnd@arndb.de>,
	Linux API <linux-api@vger.kernel.org>,
	linux-xfs <linux-xfs@vger.kernel.org>,
	linux-mm <linux-mm@kvack.org>, Andy Lutomirski <luto@kernel.org>,
	linux-fsdevel <linux-fsdevel@vger.kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Christoph Hellwig <hch@lst.de>
Subject: Re: [PATCH v9 1/6] mm: introduce MAP_SHARED_VALIDATE, a mechanism to safely define new mmap flags
Date: Mon, 16 Oct 2017 09:38:36 +0200	[thread overview]
Message-ID: <20171016073836.GB28778@lst.de> (raw)
In-Reply-To: <CA+55aFyy-nz99c6erFh=aeyCOzsk0td5wHaVLpwBNA-sWNDZkA@mail.gmail.com>

On Thu, Oct 12, 2017 at 09:32:17AM -0700, Linus Torvalds wrote:
> On Thu, Oct 12, 2017 at 6:51 AM, Jan Kara <jack@suse.cz> wrote:
> >
> > When thinking a bit more about this I've realized one problem: Currently
> > user can call mmap() with MAP_SHARED type and MAP_SYNC or MAP_DIRECT flags
> > and he will get the new semantics (if the kernel happens to support it).  I
> > think that is undesirable [..]
> 
> Why?
> 
> If you have a performance preference for MAP_DIRECT or something like
> that, but you don't want to *enforce* it, you'd use just plain
> MAP_SHARED with it.
> 
> Ie there may well be "I want this to work, possibly with downsides" issues.
> 
> So it seems to be a reasonable model, and disallowing it seems to
> limit people and not really help anything.

I don't think for MAP_DIRECT it matters (and I think we shouldn't have
MAP_DIRECT to start with, see the discussions later in the thread).

But for the main use case, MAP_SYNC you really want a hard error when you
don't get it.  And while we could tell people that they should only use
MAP_SYNC with MAP_SHARED_VALIDATE instead of MAP_SHARED chances that they
get it wrong are extremely high.  On the other hand if you really only
want a flag to optimize calling mmap twice is very little overhead, and
a very good documentation of you intent:

	addr = mmap(...., MAP_SHARED_VALIDATE | MAP_DIRECT, ...);
	if (!addr && errno = EOPNOTSUPP) {
		/* MAP_DIRECT didn't work, we'll just cope using blah, blah */
		addr = mmap(...., MAP_SHARED, ...);
	}
	if (!addr)
		goto handle_error;

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

WARNING: multiple messages have this Message-ID (diff)
From: Christoph Hellwig <hch@lst.de>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Jan Kara <jack@suse.cz>, Dan Williams <dan.j.williams@intel.com>,
	"linux-nvdimm@lists.01.org" <linux-nvdimm@lists.01.org>,
	Arnd Bergmann <arnd@arndb.de>,
	Linux API <linux-api@vger.kernel.org>,
	linux-xfs <linux-xfs@vger.kernel.org>,
	linux-mm <linux-mm@kvack.org>, Andy Lutomirski <luto@kernel.org>,
	linux-fsdevel <linux-fsdevel@vger.kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Christoph Hellwig <hch@lst.de>
Subject: Re: [PATCH v9 1/6] mm: introduce MAP_SHARED_VALIDATE, a mechanism to safely define new mmap flags
Date: Mon, 16 Oct 2017 09:38:36 +0200	[thread overview]
Message-ID: <20171016073836.GB28778@lst.de> (raw)
In-Reply-To: <CA+55aFyy-nz99c6erFh=aeyCOzsk0td5wHaVLpwBNA-sWNDZkA@mail.gmail.com>

On Thu, Oct 12, 2017 at 09:32:17AM -0700, Linus Torvalds wrote:
> On Thu, Oct 12, 2017 at 6:51 AM, Jan Kara <jack@suse.cz> wrote:
> >
> > When thinking a bit more about this I've realized one problem: Currently
> > user can call mmap() with MAP_SHARED type and MAP_SYNC or MAP_DIRECT flags
> > and he will get the new semantics (if the kernel happens to support it).  I
> > think that is undesirable [..]
> 
> Why?
> 
> If you have a performance preference for MAP_DIRECT or something like
> that, but you don't want to *enforce* it, you'd use just plain
> MAP_SHARED with it.
> 
> Ie there may well be "I want this to work, possibly with downsides" issues.
> 
> So it seems to be a reasonable model, and disallowing it seems to
> limit people and not really help anything.

I don't think for MAP_DIRECT it matters (and I think we shouldn't have
MAP_DIRECT to start with, see the discussions later in the thread).

But for the main use case, MAP_SYNC you really want a hard error when you
don't get it.  And while we could tell people that they should only use
MAP_SYNC with MAP_SHARED_VALIDATE instead of MAP_SHARED chances that they
get it wrong are extremely high.  On the other hand if you really only
want a flag to optimize calling mmap twice is very little overhead, and
a very good documentation of you intent:

	addr = mmap(...., MAP_SHARED_VALIDATE | MAP_DIRECT, ...);
	if (!addr && errno = EOPNOTSUPP) {
		/* MAP_DIRECT didn't work, we'll just cope using blah, blah */
		addr = mmap(...., MAP_SHARED, ...);
	}
	if (!addr)
		goto handle_error;

  reply	other threads:[~2017-10-16  7:35 UTC|newest]

Thread overview: 116+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-10-12  0:47 [PATCH v9 0/6] MAP_DIRECT for DAX userspace flush Dan Williams
2017-10-12  0:47 ` Dan Williams
2017-10-12  0:47 ` Dan Williams
2017-10-12  0:47 ` [PATCH v9 1/6] mm: introduce MAP_SHARED_VALIDATE, a mechanism to safely define new mmap flags Dan Williams
2017-10-12  0:47   ` Dan Williams
2017-10-12  0:47   ` Dan Williams
2017-10-12  0:47   ` Dan Williams
2017-10-12 13:51   ` Jan Kara
2017-10-12 13:51     ` Jan Kara
2017-10-12 13:51     ` Jan Kara
2017-10-12 13:51     ` Jan Kara
2017-10-12 16:32     ` Linus Torvalds
2017-10-12 16:32       ` Linus Torvalds
2017-10-12 16:32       ` Linus Torvalds
2017-10-16  7:38       ` Christoph Hellwig [this message]
2017-10-16  7:38         ` Christoph Hellwig
2017-10-16  7:38         ` Christoph Hellwig
2017-10-16  7:56       ` Jan Kara
2017-10-16  7:56         ` Jan Kara
2017-10-16  7:56         ` Jan Kara
2017-10-12  0:47 ` [PATCH v9 2/6] fs, mm: pass fd to ->mmap_validate() Dan Williams
2017-10-12  0:47   ` Dan Williams
2017-10-12  0:47   ` Dan Williams
2017-10-12  1:21   ` Al Viro
2017-10-12  1:21     ` Al Viro
2017-10-12  1:21     ` Al Viro
2017-10-12  1:21     ` Al Viro
2017-10-12  1:28     ` Dan Williams
2017-10-12  1:28       ` Dan Williams
2017-10-12  1:28       ` Dan Williams
2017-10-12  1:28       ` Dan Williams
2017-10-12  2:17       ` Dan Williams
2017-10-12  2:17         ` Dan Williams
2017-10-12  2:17         ` Dan Williams
2017-10-12  2:17         ` Dan Williams
2017-10-12  3:44         ` Dan Williams
2017-10-12  3:44           ` Dan Williams
2017-10-12  3:44           ` Dan Williams
2017-10-12  3:44           ` Dan Williams
2017-10-12  0:47 ` [PATCH v9 3/6] fs: MAP_DIRECT core Dan Williams
2017-10-12  0:47   ` Dan Williams
2017-10-12  0:47   ` Dan Williams
2017-10-12  0:47   ` Dan Williams
2017-10-12  0:47 ` [PATCH v9 4/6] xfs: prepare xfs_break_layouts() for reuse with MAP_DIRECT Dan Williams
2017-10-12  0:47   ` Dan Williams
2017-10-12  0:47   ` Dan Williams
2017-10-12  0:47   ` Dan Williams
2017-10-12  0:47 ` [PATCH v9 5/6] fs, xfs, iomap: introduce break_layout_nowait() Dan Williams
2017-10-12  0:47   ` Dan Williams
2017-10-12  0:47   ` Dan Williams
2017-10-12  0:47   ` Dan Williams
2017-10-12  0:47 ` [PATCH v9 6/6] xfs: wire up MAP_DIRECT Dan Williams
2017-10-12  0:47   ` Dan Williams
2017-10-12  0:47   ` Dan Williams
2017-10-12  0:47   ` Dan Williams
2017-10-12 14:23 ` [PATCH v9 0/6] MAP_DIRECT for DAX userspace flush Christoph Hellwig
2017-10-12 14:23   ` Christoph Hellwig
2017-10-12 14:23   ` Christoph Hellwig
2017-10-12 17:41   ` Dan Williams
2017-10-12 17:41     ` Dan Williams
2017-10-12 17:41     ` Dan Williams
2017-10-13  6:57     ` Christoph Hellwig
2017-10-13  6:57       ` Christoph Hellwig
2017-10-13  6:57       ` Christoph Hellwig
2017-10-13 15:14       ` Dan Williams
2017-10-13 15:14         ` Dan Williams
2017-10-13 15:14         ` Dan Williams
2017-10-13 16:38         ` Jason Gunthorpe
2017-10-13 16:38           ` Jason Gunthorpe
2017-10-13 16:38           ` Jason Gunthorpe
2017-10-13 16:38           ` Jason Gunthorpe
2017-10-13 17:01           ` Dan Williams
2017-10-13 17:01             ` Dan Williams
2017-10-13 17:01             ` Dan Williams
2017-10-13 17:01             ` Dan Williams
2017-10-13 17:31             ` Jason Gunthorpe
2017-10-13 17:31               ` Jason Gunthorpe
2017-10-13 17:31               ` Jason Gunthorpe
2017-10-13 17:31               ` Jason Gunthorpe
2017-10-13 18:22               ` Dan Williams
2017-10-13 18:22                 ` Dan Williams
2017-10-13 18:22                 ` Dan Williams
2017-10-13 18:22                 ` Dan Williams
2017-10-14  1:57                 ` Jason Gunthorpe
2017-10-14  1:57                   ` Jason Gunthorpe
2017-10-14  1:57                   ` Jason Gunthorpe
2017-10-14  1:57                   ` Jason Gunthorpe
2017-10-16 12:02                   ` Sagi Grimberg
2017-10-16 12:02                     ` Sagi Grimberg
2017-10-19  6:02                     ` Jason Gunthorpe
2017-10-19  6:02                       ` Jason Gunthorpe
2017-10-19  6:02                       ` Jason Gunthorpe
2017-10-19  6:02                       ` Jason Gunthorpe
2017-10-16  7:30                 ` Christoph Hellwig
2017-10-16  7:30                   ` Christoph Hellwig
2017-10-16  7:30                   ` Christoph Hellwig
2017-10-16  7:26               ` Christoph Hellwig
2017-10-16  7:26                 ` Christoph Hellwig
2017-10-16  7:26                 ` Christoph Hellwig
2017-10-16 12:07                 ` Sagi Grimberg
2017-10-16 12:07                   ` Sagi Grimberg
2017-10-16 12:07                   ` Sagi Grimberg
2017-10-16 17:43                 ` Dan Williams
2017-10-16 17:43                   ` Dan Williams
2017-10-16 17:43                   ` Dan Williams
2017-10-16 19:44                   ` Dan Williams
2017-10-16 19:44                     ` Dan Williams
2017-10-16 19:44                     ` Dan Williams
2017-10-17  6:46                     ` Christoph Hellwig
2017-10-17  6:46                       ` Christoph Hellwig
2017-10-17  6:46                       ` Christoph Hellwig
2017-10-17  6:46                       ` Christoph Hellwig
2017-10-16  7:22           ` Christoph Hellwig
2017-10-16  7:22             ` Christoph Hellwig
2017-10-16  7:22             ` Christoph Hellwig
2017-10-16  7:22             ` Christoph Hellwig

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=20171016073836.GB28778@lst.de \
    --to=hch@lst.de \
    --cc=akpm@linux-foundation.org \
    --cc=arnd@arndb.de \
    --cc=jack@suse.cz \
    --cc=linux-api@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 \
    --cc=torvalds@linux-foundation.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.