From: NeilBrown <neilb@suse.com>
To: Jeff Layton <jlayton@kernel.org>, linux-fsdevel@vger.kernel.org
Date: Thu, 14 Dec 2017 09:04:33 +1100
Cc: linux-kernel@vger.kernel.org, hch@lst.de, neilb@suse.de,
        bfields@fieldses.org, amir73il@gmail.com, jack@suse.de,
        viro@zeniv.linux.org.uk
Subject: Re: [PATCH 01/19] fs: new API for handling inode->i_version
In-Reply-To: <20171213142017.23653-2-jlayton@kernel.org>
References: <20171213142017.23653-1-jlayton@kernel.org> <20171213142017.23653-2-jlayton@kernel.org>
Message-ID: <87indanv2m.fsf@notabene.neil.brown.name>
MIME-Version: 1.0
Content-Type: multipart/signed; boundary="=-=-=";
        micalg=pgp-sha256; protocol="application/pgp-signature"
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 3593
Lines: 88

--=-=-=
Content-Type: text/plain
Content-Transfer-Encoding: quoted-printable

On Wed, Dec 13 2017, Jeff Layton wrote:

> +/*
> + * The change attribute (i_version) is mandated by NFSv4 and is mostly f=
or
> + * knfsd, but is also used for other purposes (e.g. IMA). The i_version =
must
> + * appear different to observers if there was a change to the inode's da=
ta or
> + * metadata since it was last queried.
> + *
> + * It should be considered an opaque value by observers. If it remains t=
he same
> + * since it was last checked, then nothing has changed in the inode. If =
it's
> + * different then something has changed. Observers cannot infer anything=
 about
> + * the nature or magnitude of the changes from the value, only that the =
inode
> + * has changed in some fashion.

I agree that it "should be" considered opaque, but I have a suspicion
that NFSv4 doesn't consider it opaque.
There is something about write delegations and the server performing a
GETATTR callback to the delegated client so that it can answer GETATTR
from other clients without recalling the delegation.

Specifically section "10.4.3 Handling of CB_GETATTR" of RFC5661 contains
the text:

   o  The client will create a value greater than c that will be used
      for communicating that modified data is held at the client.  Let
      this value be represented by d.

"c" here is a 'change' attribute.

Then:

   While the change attribute is opaque to the client in the sense that
   it has no idea what units of time, if any, the server is counting
   change with, it is not opaque in that the client has to treat it as
   an unsigned integer, and the server has to be able to see the results
   of the client's changes to that integer.  Therefore, the server MUST
   encode the change attribute in network order when sending it to the
   client.  The client MUST decode it from network order to its native
   order when receiving it, and the client MUST encode it in network
   order when sending it to the server.  For this reason, change is
   defined as an unsigned integer rather than an opaque array of bytes.

This all suggests that nfsd needs to be certain that "incrementing" the
change id will produce a new changeid, which has not been used before,
and also suggests that nfsd needs to be able to control the changeid
stored after writes that result from a delegation being returned.

I'd just like to say that this is one of the most annoying dumb features
of NFSv4, because it is trivial to fix and I suggested a fix before
NFSv4.0 was finalized.  Grumble.

Otherwise the patch set looks good.  I haven't gone over the code
closely, the but approach is spot-on.

NeilBrown


--=-=-=
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAEBCAAdFiEEG8Yp69OQ2HB7X0l6Oeye3VZigbkFAloxo/MACgkQOeye3VZi
gblB+BAAxL74L2lQJSAF8ffpgFIJ+/w8IGJCSqLtKIpwTWuUcKizdIG+gdE0UU4x
N+TLb5bgeTWaSzAYeaYIjT8DeMJBfdwGT18fLnmZ3Ptpkc9puA4oS8IhYLuub3Ub
mtXujsRMOJ9JYsYdrmyPfZuHpBQxtqkJ5dP04VKish7QWCvR/R+nrC72c6JXG6G/
i47ibmSZ3A36RBlJ3EGP420Dp8mKpUL2scARaVP3HHtT9IDmumuXfszvb6tioVrp
dRVX87wcYn3ZGc1zU9f6+vwwXmcEZIcFH1TwMiXFTZzQEhj65YF4rKnd8xhFsxYA
pyZTS+i+RLL/+1wXYrDCUmaIQEbtL00x7PsDgPoW+HhnStyHUHjc4NI5y842AsZ0
O0zPrhGfNA+I9HUw6VAJGl36MKaDIdd4Gyh7n0O/sz3muWAkHpaIEtGz8Gz4JKU8
1UVV+wL22leUvogC2/7PpGkPzJcnFFWvcEHhWtCWjTSvqH9GGGdeGZDgGIp5Oz4V
ut4UwdhbEUlffTtCruoOzrhGvdFp4Fw1HWqI5Ue4nq/gXBXTZds4/y/F43ZZO/z5
aJWWUyUGJjJguIyNA5zFkETOp3ku5RSjQEXICe6PAJsO34cbAELmFMxy9/BKZ34X
f6KeyjZzUv9Z5UIHT8URc0WkVE8KFxoUYlx0Z3ox6kCYNuroJRM=
=EGsI
-----END PGP SIGNATURE-----
--=-=-=--