Subject: [nfsv4] proposed RFC5663 errata

From: Christoph Hellwig <[email-masked]>
Date: Wed, 10 Sep 2014 02:45:58 +0200

I'll reply with various proposed errata to RFC5663 to this mail.  They
are the result of bringing up the Linux block layout client to proper
quality standards and implementing a new server.

---

From: Christoph Hellwig <[email-masked]>
Date: Wed, 10 Sep 2014 02:46:08 +0200

Type: Technical

Section <> says:

It should say:

2.7.  Volatile write caches

   Many storage devices implement volatile write caches that require and
   explicit flush to persist the data from write write operations to
   stable storage.  When a volatile write cache is used the pNFS server
   must ensure the volatile write cache has been committed to stable
   storage before the LAYOUTCOMMIT operation returns.

Notes:

   RFC5663 currently doesn't acknowledge the existence of volatile
   write caches, but they are common in consumer or SMB storage
   systems.  Add a section that requires the server to take care
   of them.

---

From: Christoph Hellwig <[email-masked]>
Date: Wed, 10 Sep 2014 02:46:17 +0200

Type: Editorial

Section 2.3.5 says:

   Block/volume class storage devices are not required to perform read
   and write operations atomically.  Overlapping concurrent read and
   write operations to the same data may cause the read to return a
   mixture of before-write and after-write data.  Overlapping write
   operations can be worse, as the result could be a mixture of data
   from the two write operations; data corruption can occur if the
   underlying storage is striped and the operations complete in
   different orders on different stripes.  When there are multiple
   clients who wish to access the same data, a pNFS server can avoid
   these conflicts by implementing a concurrency control policy of
   single writer XOR multiple readers.  This policy MUST be implemented
   when storage devices do not provide atomicity for concurrent
   read/write and write/write operations to the same data.

It should say:
   
   Block/volume class storage devices do not provide byte granularity
   access and can only perform read and write operations atomically on a
   block granularity, and thus require read-modify-write cycles to write
   data smaller than the block size.  Overlapping concurrent read and
   write operations to the same data thus will cause the read to return
   a mixture of before-write and after-write data.  Additionally, data
   corruption can occur if the underlying storage is striped and the
   operations complete in different orders on different stripes.  When
   there are multiple clients who wish to access the same data, a pNFS
   server MUST avoid these conflicts by implementing a concurrency
   control policy of single writer XOR multiple readers for a given data
   region.

Notes:

   No device classified as block device can support concurrent writes
   at arbitrary byte granularity, so reword the section to not confuse
   the reader.  Also make it explicit that the reader XOR writer policy
   only applies to different clients, as existing client implementation
   require layouts not to be recalled due to their own LAYOUTGET
   operations.  Note that fixing this on the client also isn't feasible
   as the block layout unfortunately decided to introduce it's own
   extent concept instead of using layouts to describe individual
   I/O mappings.

---

From: Christoph Hellwig <[email-masked]>
Date: Wed, 10 Sep 2014 02:46:26 +0200

Type: Technical

Section 2.2.2 says:

It should say:

   The volume size of a PNFS_BLOCK_VOLUME_SIMPLE volume must be obtained
   by the client from the storage subsystem as no size is provided in the
   XDR.
   All volumes listed in bsv_volumes of a
   struct pnfs_block_stripe_volume_info4 must be the same size.  If
   the size of the volumes listed in a stripe set does not align
   to the bsv_stripe_unit, the last stripe should be treated as
   having a size of volume size modulo the stripe size.
   The volume size of a PNFS_BLOCK_VOLUME_SLICE volume is the sum
   of the volume sizes of each component listed in bsv_volumes.
   The volume size of a PNFS_BLOCK_VOLUME_CONCAT volume is the sum
   of the volume sizes of each component listed in bcv_volumes.

Notes:

   RFC5663 provides no explanation of the volume types except for
   a few sparse comments in the XDR.  Explain at least basic size
   related rules.

   XXX: Maybe we also need to explain the algorithms behind striping
   and concatenation, or can they be assumed to be generally available?

---

From: Christoph Hellwig <[email-masked]>
Date: Wed, 10 Sep 2014 02:46:36 +0200

Type: Technical

Section <> says:

It should say:
   
2.8.  Device-ID-to-device-address

   A pNFS block volume layout server MAY signal device-ID-to-device-address
   changes to the client using the CB_NOTIFY_DEVICEID callback operation.

   If the change is compatible and does not require outstanding layouts
   to be called back the server can issue a notification of type
   NOTIFY_DEVICEID4_CHANGE.

   A device-ID-to-device-address mapping change signaled by
   NOTIFY_DEVICEID4_CHANGE must not change the storage system specific
   addressing of the volume, and can only add new storage to the existing
   device.  In particular the following changes are allowed:

     o increasing the size of the underlying block device of a
       PNFS_BLOCK_VOLUME_SIMPLE volume.
     o increase the size of a PNFS_BLOCK_VOLUME_SLICE volume if the
       underlying block device of the PNFS_BLOCK_VOLUME_SIMPLE volume
       it refers to is big enough to fit the new size.
     o increase the size of each volume in a bsv_volumes of a
       PNFS_BLOCK_VOLUME_SLICE volume by the same amount.
     o increase the size of the last volume in bcv_volumes of a
       PNFS_BLOCK_VOLUME_CONCAT volume.
     o add new members to the end of bcv_volumes of a
       PNFS_BLOCK_VOLUME_CONCAT volume.

Notes:

   Specify what device configuration changes can be supported without
   recalling layouts.

---

From: Christoph Hellwig <[email-masked]>
Date: Wed, 10 Sep 2014 09:20:01 +0200

On Tue, Sep 09, 2014 at 10:23:26PM -0400, Jason Glasgow wrote:
> Perhaps I am confused, but if a client issues a LAYOUTCOMMIT operation I
> think this would generally imply that the client has not used the WRITE
> operation to write data to the disk via the server volatile write cache.
>  If the client as used the WRITE operation, then the explanation offered
> below is already covered by RFC5561 in section 18.3 Operation 5: COMMIT.

I think we are talking about different caches here, and that probably suggested
I need to tighten up the wording.  The volatile caches I'm concerned about
are in the storage device.  For SCSI we're talking about a device with the WCE
bit set to 1 in the INQUIRY data, which requires a SYNCHRONIZE_CACHE
to flush the volatile storage device cache to stable storage.

---

From: Benjamin Kaduk <[email-masked]>
Date: Wed, 10 Sep 2014 09:59:35 -0400 (EDT)

>    Many storage devices implement volatile write caches that require and

That should probably be "an" instead of "and"?

-Ben

> _______________________________________________
> nfsv4 mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/nfsv4
>

---

From: Jason Glasgow <[email-masked]>
Date: Tue, 9 Sep 2014 22:10:41 -0400

Comments in line

>    to be called back the server can issue a notification of type

replaced "called backed" with "recalled"

> In the following bullets similar language should be used.  Replace
"increase" with "increasing", "add" with "adding"

---

From: Jason Glasgow <[email-masked]>
Date: Tue, 9 Sep 2014 22:23:26 -0400

If you think that this still belongs in the errata, then I have some
specific comments below.

>    Many storage devices implement volatile write caches that require and
replace "and" with "an"

>    explicit flush to persist the data from write write operations to
replace "write write" with "WRITE"

>    storage before the LAYOUTCOMMIT operation returns.

I am not sure that this is clear enough.  The statement, "the pNFS server
must ensure the volatile write cache has been committed" seems overly
broad.  The entire volatile write cache?  Perhaps this should be qualified
to clarify that the parts of the cache associated with the layouts
specified in the LAYOUTCOMMIT operation have been committed to stable
storage before the LAYOUTCOMMIT operation returns.  Though really, I think
the LAYOUTCOMMIT is only responsible for committing the server's metadata
changes that are possibly in the volatile write cache.

---

From: Jason Glasgow <[email-masked]>
Date: Tue, 9 Sep 2014 22:24:41 -0400

Chris,
Thank you for sending the proposed errata and for moving the pNFS block
layout client forward.  I am replying to each proposal individually.
Jason

---

From: Jason Glasgow <[email-masked]>
Date: Tue, 9 Sep 2014 22:27:16 -0400

>    access and can only perform read and write operations atomically on a
replace "on a" with "at"

>    write operations to the same data thus will cause the read to return
replace "will cause" with "may cause"

---

From: Jason Glasgow <[email-masked]>
Date: Wed, 10 Sep 2014 06:24:00 -0400

Yes I was confused.  Thanks for the clarification.

---

From: Benny Halevy <[email-masked]>
Date: Tue, 16 Sep 2014 14:39:23 +0300

> to flush the volatile storage device cache to stable storage.

Christoph, I would consider adding that example above for clarification.

Also, how about adding another possible strategy like setting the FUA
(Force Unit Access)
bit in the SCSI CDB?

Benny

---

From: Benny Halevy <[email-masked]>
Date: Tue, 16 Sep 2014 14:50:07 +0300

>    extent concept instead of using layouts to describe individual

That is a pretty harsh requirement on the server.  Atomic
read-modify-write issues
are also present for normal NFS clients reading and writing via the MDS, no?
The "text book" solution is to use byte range locking to serialize access to the
shared blocks / pages.  Best practice for the client is to
flush/invalidate its page cache
respectively.

Besides client-side synchronization, for a server that implements provisional
block allocation clients may (read-modify-)write into their own provisionally
allocated space and synchronization happens at LAYOUTCOMMIT time.

Benny

---

From: Christoph Hellwig <[email-masked]>
Date: Fri, 19 Sep 2014 01:26:29 +0200

> Christoph, I would consider adding that example above for clarification.

Will do for the next iteration.

> Also, how about adding another possible strategy like setting the FUA

If we decided the client and not the server must enforce the cache writeback
that would be a possible implementation.  Although in general doing multiple
I/Os with the FUA bit set is slower than batching it up for a cache flush.

---

From: Christoph Hellwig <[email-masked]>
Date: Fri, 19 Sep 2014 01:35:22 +0200

> are also present for normal NFS clients reading and writing via the MDS, no?

Normal NFS clients should not implement any read-modify-write cycles
that could corrupt data on the server, that they may read in the whole
page, but they must only write out the actual dirty data to the server.

For block layouts on the other hand we have the fundamental problem that
we cannot write smaller than the server block size, so any two clients
writing different regions in the same block WILL corrupt data.

Note that my change in this errata proposal does not introduce this language,
it just does a better job at wording it (I hope).

> allocated space and synchronization happens at LAYOUTCOMMIT time.

With provisionally allocated space you mean an out of place update using
COW read extents?  Either way that does not change the fundamental behavior
of block storage that does not allow sub-blocksize (usually 512 byte or 4k)
writes.

---

From: Benny Halevy <[email-masked]>
Date: Mon, 22 Sep 2014 14:47:55 +0300

True.  but at write time, the client and consequently the layout driver
know whether writes
are required to be stable or not (e.g. O_SYNC) and for unstable writes,
taking the linux client
layout driver API for example, there is a commit method to sync unstable
writes to stable storage.

The client chooses the strategy of writing unstable writes after which
committing, say when the writes
are backed by the page cache, or sending stable writes in cases like
O_DIRECT|O_SYNC or
in retry code paths.  The point is that there are cases that the client
should be controlling target cache
writeback timing.

Also, a simple layout driver on linux that does not support the commit
method and that can determine that the
target has volatile write cache can choose to send all writes (DATA OUT)
commands with FUA set.

Benny

---

From: Christoph Hellwig <[email-masked]>
Date: Mon, 22 Sep 2014 16:55:11 +0200

Just trying to understand what point you're trying to make here.

If we let the client handle device writeback cache it can indeed either
use cache flushes or fua.  if we let the server handle it it can
only use cache flushes.

The first one allows the client to do choices like yours below, although
I don't know about any local filesystem bothering with the FUA bit
for data, if we let the server handle the flush that option falls away,
but the server can possible batch the cache flushes more intelligently
as part of it's transactions.

Given my preference for server side flushing you may assume I've actually
implemented the latter, but don't think the former is overly useful.

That being said both options are valid, although I think implementing
it on the client is useful for two reasons:

 a) dramatically keeps the client complexity down.  A properly working
    pnfs block client already needs to implement a stable similar
    to NFS unstable writes for block layout I/O to deal with data
    that has been written but for which they layout commit hasn't
    been sent yet to allow for resend via MDS in case the layoutcommit
    fails, so we'd need to introduce another state in the state machine
    to support it.
 b) having implemented both client and server side I see a lot more
    optimization potential on the server side.

And in practice I suspect few people will implement pNFS over devices
with a volatile writeback cache at all, so keeping the complexity down
seems to be the biggest priority to me.