linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Dave Chinner <david@fromorbit.com>
To: Christoph Hellwig <hch@lst.de>
Cc: Dan Williams <dan.j.williams@intel.com>,
	darrick.wong@oracle.com, Jan Kara <jack@suse.cz>,
	linux-nvdimm@lists.01.org, linux-kernel@vger.kernel.org,
	linux-xfs@vger.kernel.org, Jeff Moyer <jmoyer@redhat.com>,
	Alexander Viro <viro@zeniv.linux.org.uk>,
	luto@kernel.org, linux-fsdevel@vger.kernel.org,
	Ross Zwisler <ross.zwisler@linux.intel.com>
Subject: Re: [PATCH v2 1/5] fs, xfs: introduce S_IOMAP_IMMUTABLE
Date: Mon, 7 Aug 2017 10:25:02 +1000	[thread overview]
Message-ID: <20170807002502.GI21024@dastard> (raw)
In-Reply-To: <20170805094708.GB14930@lst.de>

On Sat, Aug 05, 2017 at 11:47:08AM +0200, Christoph Hellwig wrote:
> NAK^4.
> 
> We should not allow users to create immutable files.  We have
> proper ways to synchronize I/O, and this is just an invitation
> for horrible abuses that should not be allowed, and which we've
> always people told not to do.

We've always told people not to do those "horrible abuses" because
of the TOCTOU race conditions inherent in getting accurate
BMAP/FIEMAP information to userspace. However, immutable extent maps
solve the TOCTOU problem and so removes the only *technical* barrier
in the way of using extent maps to implement functionality such as
userspace pNFS servers.

The core requirement for a userspace pNFS block server to be able to
safely export the block map of a file to remote clients is that the
extent map is allocated and will not change while the client has
been granted access to it. Immutable extent maps provide that
functionality to userspace.  However, for this to work, us
filesystem developers have to give up the idea that only the
filesystem can access the storage underlying the filesystem.

I'm not writing this for your benefit, Christoph, but for everyone
else who doesn't know about existing direct remote storage access
protocols and implementations. That is, I'm letting everyone know
we've already had to give up the exclusive storage device access
model...

.... when you implemented the kernel pNFS server code that provides
unknown third parties with the *remote direct access* to the storage
underlying the XFS filesystem.

Yup, we already allow third parties to arbitrate and directly access
to the XFS block device map. That "horrible abuse" was allowed
because it could be done safely via NFSv4 delegations and a new API
that provided a "blocks will always be allocated before a write and
won't change while the remote client has access" guarantee from XFS
to the kernel pNFS server (i.e. ->map_blocks()/->commit_blocks()
export ops and the break_layouts() API).

Immutable extent maps provide userspace with this same guarantee, so
what used to be considered a "horrible abuse" can now be done safely
and without risking data and/or filesystem corruption.  So, really,
calling this an "invitation to horrible abuses that should not be
allowed" ignores the reality that you were the architect that
introduced this "safe remote direct access" model to convert a
"horrible abuse" into a set of safe, supportable operations.

In the end, all I care about is that everyone understands the
technical merits of the proposals being considered rather than
discussion and review being shut down because "Christoph shouted
nasty words at me but I still don't understand why?".....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

  reply	other threads:[~2017-08-07  0:25 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-08-04  2:28 [PATCH v2 0/5] fs, xfs: block map immutable files for dax, dma-to-storage, and swap Dan Williams
2017-08-04  2:28 ` [PATCH v2 1/5] fs, xfs: introduce S_IOMAP_IMMUTABLE Dan Williams
2017-08-04 20:00   ` Darrick J. Wong
2017-08-04 20:31     ` Dan Williams
2017-08-05  9:47   ` Christoph Hellwig
2017-08-07  0:25     ` Dave Chinner [this message]
2017-08-11 10:34       ` Christoph Hellwig
2017-08-04  2:28 ` [PATCH v2 2/5] fs, xfs: introduce FALLOC_FL_SEAL_BLOCK_MAP Dan Williams
2017-08-04 19:46   ` Darrick J. Wong
2017-08-04 19:52     ` Dan Williams
2017-08-04 23:31   ` Dave Chinner
2017-08-04 23:43     ` Dan Williams
2017-08-05  0:04       ` Dave Chinner
2017-08-04  2:28 ` [PATCH v2 3/5] fs, xfs: introduce FALLOC_FL_UNSEAL_BLOCK_MAP Dan Williams
2017-08-04 20:04   ` Darrick J. Wong
2017-08-04 20:36     ` Dan Williams
2017-08-04  2:28 ` [PATCH v2 4/5] xfs: introduce XFS_DIFLAG2_IOMAP_IMMUTABLE Dan Williams
2017-08-04 20:33   ` Darrick J. Wong
2017-08-04 20:45     ` Dan Williams
2017-08-04 23:46     ` Dave Chinner
2017-08-04 23:57       ` Darrick J. Wong
2017-08-04  2:28 ` [PATCH v2 5/5] xfs: toggle XFS_DIFLAG2_IOMAP_IMMUTABLE in response to fallocate Dan Williams
2017-08-04 20:14   ` Darrick J. Wong
2017-08-04 20:47     ` Dan Williams
2017-08-04 20:53       ` Darrick J. Wong
2017-08-04 20:55         ` Dan Williams
2017-08-04  2:38 ` [PATCH v2 0/5] fs, xfs: block map immutable files for dax, dma-to-storage, and swap Dan Williams
2017-08-05  9:50   ` Christoph Hellwig
2017-08-06 18:51     ` Dan Williams
2017-08-11 10:44       ` Christoph Hellwig
2017-08-11 22:26         ` Dan Williams
2017-08-12  3:57           ` Andy Lutomirski
2017-08-12  4:44             ` Dan Williams
2017-08-12  7:34             ` Christoph Hellwig
2017-08-12  7:33           ` Christoph Hellwig
2017-08-12 19:19             ` Dan Williams
2017-08-13  9:24               ` Christoph Hellwig
2017-08-13 20:31                 ` Dan Williams
2017-08-14 12:40                   ` Jan Kara
2017-08-14 16:14                     ` Dan Williams
2017-08-15  8:37                       ` Jan Kara
2017-08-15 23:50                         ` Dan Williams
2017-08-16 13:57                           ` Jan Kara
2017-08-21  9:16                     ` Peter Zijlstra
2017-08-14 21:46                   ` Darrick J. Wong
2017-08-13 23:46                 ` Dave Chinner

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=20170807002502.GI21024@dastard \
    --to=david@fromorbit.com \
    --cc=dan.j.williams@intel.com \
    --cc=darrick.wong@oracle.com \
    --cc=hch@lst.de \
    --cc=jack@suse.cz \
    --cc=jmoyer@redhat.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-nvdimm@lists.01.org \
    --cc=linux-xfs@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=ross.zwisler@linux.intel.com \
    --cc=viro@zeniv.linux.org.uk \
    /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).