Received-SPF: pass (google.com: domain of linux-ext4-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Chuck Lever III <chuck.lever@oracle.com>
To:     Bruce Fields <bfields@fieldses.org>, Theodore Ts'o <tytso@mit.edu>
CC:     Jeff Layton <jlayton@kernel.org>, Jan Kara <jack@suse.cz>,
        Neil Brown <neilb@suse.de>,
        "adilger.kernel@dilger.ca" <adilger.kernel@dilger.ca>,
        "Darrick J. Wong" <djwong@kernel.org>,
        Dave Chinner <david@fromorbit.com>,
        Trond Myklebust <trondmy@hammerspace.com>,
        Al Viro <viro@zeniv.linux.org.uk>,
        "zohar@linux.ibm.com" <zohar@linux.ibm.com>,
        "xiubli@redhat.com" <xiubli@redhat.com>,
        Lukas Czerner <lczerner@redhat.com>,
        Christian Brauner <brauner@kernel.org>,
        "fweimer@redhat.com" <fweimer@redhat.com>,
        "linux-man@vger.kernel.org" <linux-man@vger.kernel.org>,
        "linux-api@vger.kernel.org" <linux-api@vger.kernel.org>,
        "linux-btrfs@vger.kernel.org" <linux-btrfs@vger.kernel.org>,
        linux-fsdevel <linux-fsdevel@vger.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        "ceph-devel@vger.kernel.org" <ceph-devel@vger.kernel.org>,
        "linux-ext4@vger.kernel.org" <linux-ext4@vger.kernel.org>,
        Linux NFS Mailing List <linux-nfs@vger.kernel.org>,
        Linux-XFS <linux-xfs@vger.kernel.org>
Subject: Re: [man-pages RFC PATCH v4] statx, inode: document the new
 STATX_INO_VERSION field
Thread-Topic: [man-pages RFC PATCH v4] statx, inode: document the new
 STATX_INO_VERSION field
Thread-Index: AQHYwqtBEl18RH0NrEO0UKb7ApfWq63T1xmAgAATgACAAAFbgIAABbEAgAAK/YCAALZNAIAAgw4AgAByGoCAAAZagIAAAzmAgAAFVoA=
Date:   Thu, 8 Sep 2022 16:15:12 +0000
Message-ID: <03CEBF7E-649D-4BE3-B759-EA2E561B4110@oracle.com>
References: <20220907111606.18831-1-jlayton@kernel.org>
 <166255065346.30452.6121947305075322036@noble.neil.brown.name>
 <79aaf122743a295ddab9525d9847ac767a3942aa.camel@kernel.org>
 <20220907125211.GB17729@fieldses.org>
 <771650a814ab1ff4dc5473d679936b747d9b6cf5.camel@kernel.org>
 <20220907135153.qvgibskeuz427abw@quack3>
 <166259786233.30452.5417306132987966849@noble.neil.brown.name>
 <20220908083326.3xsanzk7hy3ff4qs@quack3> <YxoIjV50xXKiLdL9@mit.edu>
 <02928a8c5718590bea5739b13d6b6ebe66cac577.camel@kernel.org>
 <20220908155605.GD8951@fieldses.org>
In-Reply-To: <20220908155605.GD8951@fieldses.org>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?6qd5Mcvazkpu5NkURJjXj0krVJBFSKUVhPy282rhmZ5NSCENBzD6wFJfS+S5?=
 =?us-ascii?Q?0SU+f1lDDDWI3uFwTnARmKuuKU1kjt+2UArIhWuSPGlqCuFaoTxo8yY/Pw19?=
 =?us-ascii?Q?Ge3yUHxTTpyOIMy4sxjeyyxBhnWsHTrlK6Wgt+YzsrUKbc+Iv9sHoyyo8mBV?=
 =?us-ascii?Q?tGE1EZVSbzxFdHSXv0Mky1cX8j5HXUG0uEVlmBQ10O8satWxHHSF+D+iLvr+?=
 =?us-ascii?Q?yZRX7sLUPyODFbDj24MZ7k78JJaYmOBfF0XHOfGazq3dujYun5zMZb1vB6Ut?=
 =?us-ascii?Q?Dr36SmIeIAPML/uI7hwVCncEZ7sinrXRnK44z7Iy7XUclSCHY0wWxc8ugvbI?=
 =?us-ascii?Q?bdL3HllDRoAd2P3HdIDywNpni+yeN377iUf7iwiaodb6HZeo5FfeisNo8KfP?=
 =?us-ascii?Q?kvyHZegcBSEWLUBXXfndUXgYDSuRhJgW7eFHsZbkQKsXilP0dw4y+pkO/U1k?=
 =?us-ascii?Q?8XBOvwOj8yTQCS2huYlJ34/4h64hUk+ZRc6sab2LNyYaI88XWT6lWv4eOFNL?=
 =?us-ascii?Q?75qpCyWBc09VnuVsyKaBDFyYVMsQiueRhf94TyNUxD3K9a5cN6s3PY7/Qyiv?=
 =?us-ascii?Q?8DwZRhyixP7kiaiAEuuyV4j/titLZbrPM18Lu4XtD3gQOoxtuVcGXw2tEsXR?=
 =?us-ascii?Q?RLdlNTwNBepQKp1PauPDoWgiW08n/CbroMlur8O0KZYdkvS7RY6sy60jWMgw?=
 =?us-ascii?Q?JbuHxEj9TvKbM4MBJVuY+BsLFDf9svOVWL2SecrFF1R8VcVZZUNrF41ZV06z?=
 =?us-ascii?Q?6SI5rbRUSZ8fY6O5OYepElo2mVAXwqrfZiYTpWi4Bd0qxAkPxMoNn9SH50AA?=
 =?us-ascii?Q?+z97m5vk9pdfxepxmx2HcHC9JKu79DWLrdGDRaMeIVQyWuPLhIa5Zb8yTy7J?=
 =?us-ascii?Q?z6FBYQcro8tDPxRhpDScmuBpvsFzdcpupIqtCoY+u5p85QwiS18nmbBFvWX3?=
 =?us-ascii?Q?/X9HPxYx0FqXaKGxp+EMe1zPN1y3ZRAVNSjgt9zk4kvo9YkOeKJlMjLhrzpp?=
 =?us-ascii?Q?0hhSi+OMev+7aybrGZzrap9tQCuXymxuF941aCeDoWZFYeZE6h3QEI8MNWdu?=
 =?us-ascii?Q?HihkYdLXPsCVCJgjf9lmQ4dKMZFdwoJQWqQz9yKXaQekxmRjZtVy9jpOxGei?=
 =?us-ascii?Q?bs+Mx2nkdZfREiF70IcnQeDzSL1PkDko9Qowq5qfeDnZad8PDk7EOuf8Dmw0?=
 =?us-ascii?Q?UnLV3bzbwYVS8juM84hRB7JpiPh1Uq1pig2EIZsUYoho8+ZPvhkTVvatD+O7?=
 =?us-ascii?Q?+b9ANbilNad+PM7VaVVrRZWdjTejeW931IEXGDkO0qQRcBjdk0ew7Ga4nKL0?=
 =?us-ascii?Q?/YDo4abz9ZqS+oy1sx0MfMLadB6AX05rESyRbh5K+d6qDsQbXbcbSKCjRGnl?=
 =?us-ascii?Q?ULCdmYk8QUAIwe9fwgwk0QXyKyZZZX4NwZB8dZruWVlddh1lxUTUEQFGrpVt?=
 =?us-ascii?Q?DZ11LReQZQzqg6ySE64APSF7LSU9ayP0dQt1CjxrOpCwTvE0nGtjW2Whv5zm?=
 =?us-ascii?Q?uzUuXDdP9P+9abvoIhurwxaUtcfbeoyZsFIRrH/p6ZCF8uONxys+JQgmUNNo?=
 =?us-ascii?Q?TTow6HvNCOk35DlxloWabsKkqQpC7pEISF4/UTPYqeJNFxo3JB/HceVEhwf4?=
 =?us-ascii?Q?vg=3D=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-ID: <B74C339A860EEC448CBF1DBA16F06BA9@namprd10.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: BN0PR10MB5128.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 1c16f76d-b91d-4a1b-3736-08da91b54ec9
X-MS-Exchange-CrossTenant-originalarrivaltime: 08 Sep 2022 16:15:12.2795
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: fEYtarKVjFgNSCniNmY91rVdOZDS0vfVjPmUbNqZD6WT7uF8lU2UNSJKZdwA3urFhsOLglTD201SccVhs84xzQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH8PR10MB6338
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.205,Aquarius:18.0.895,Hydra:6.0.528,FMLib:17.11.122.1
 definitions=2022-09-08_10,2022-09-08_01,2022-06-22_01
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 adultscore=0 phishscore=0 spamscore=0
 mlxlogscore=999 mlxscore=0 bulkscore=0 malwarescore=0 suspectscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2207270000
 definitions=main-2209080059
X-Proofpoint-ORIG-GUID: x5f_wc6upijpf1H68PoXLpKc8TDRXWR4
X-Proofpoint-GUID: x5f_wc6upijpf1H68PoXLpKc8TDRXWR4
X-Spam-Status: No, score=-2.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_LOW,
        RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE,T_SCC_BODY_TEXT_LINE
        autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-ext4.vger.kernel.org>
X-Mailing-List: linux-ext4@vger.kernel.org


> On Sep 8, 2022, at 11:56 AM, J. Bruce Fields <bfields@fieldses.org> wrote=
:
>=20
> On Thu, Sep 08, 2022 at 11:44:33AM -0400, Jeff Layton wrote:
>> On Thu, 2022-09-08 at 11:21 -0400, Theodore Ts'o wrote:
>>> On Thu, Sep 08, 2022 at 10:33:26AM +0200, Jan Kara wrote:
>>>> It boils down to the fact that we don't want to call mark_inode_dirty(=
)
>>>> from IOCB_NOWAIT path because for lots of filesystems that means journ=
al
>>>> operation and there are high chances that may block.
>>>>=20
>>>> Presumably we could treat inode dirtying after i_version change simila=
rly
>>>> to how we handle timestamp updates with lazytime mount option (i.e., n=
ot
>>>> dirty the inode immediately but only with a delay) but then the time w=
indow
>>>> for i_version inconsistencies due to a crash would be much larger.
>>>=20
>>> Perhaps this is a radical suggestion, but there seems to be a lot of
>>> the problems which are due to the concern "what if the file system
>>> crashes" (and so we need to worry about making sure that any
>>> increments to i_version MUST be persisted after it is incremented).
>>>=20
>>> Well, if we assume that unclean shutdowns are rare, then perhaps we
>>> shouldn't be optimizing for that case.  So.... what if a file system
>>> had a counter which got incremented each time its journal is replayed
>>> representing an unclean shutdown.  That shouldn't happen often, but if
>>> it does, there might be any number of i_version updates that may have
>>> gotten lost.  So in that case, the NFS client should invalidate all of
>>> its caches.
>>>=20
>>> If the i_version field was large enough, we could just prefix the
>>> "unclean shutdown counter" with the existing i_version number when it
>>> is sent over the NFS protocol to the client.  But if that field is too
>>> small, and if (as I understand things) NFS just needs to know when
>>> i_version is different, we could just simply hash the "unclean
>>> shtudown counter" with the inode's "i_version counter", and let that
>>> be the version which is sent from the NFS client to the server.
>>>=20
>>> If we could do that, then it doesn't become critical that every single
>>> i_version bump has to be persisted to disk, and we could treat it like
>>> a lazytime update; it's guaranteed to updated when we do an clean
>>> unmount of the file system (and when the file system is frozen), but
>>> on a crash, there is no guaranteee that all i_version bumps will be
>>> persisted, but we do have this "unclean shutdown" counter to deal with
>>> that case.
>>>=20
>>> Would this make life easier for folks?
>>>=20
>>> 						- Ted
>>=20
>> Thanks for chiming in, Ted. That's part of the problem, but we're
>> actually not too worried about that case:
>>=20
>> nfsd mixes the ctime in with i_version, so you'd have to crash+clock
>> jump backward by juuuust enough to allow you to get the i_version and
>> ctime into a state it was before the crash, but with different data.
>> We're assuming that that is difficult to achieve in practice.
>=20
> But a change in the clock could still cause our returned change
> attribute to go backwards (even without a crash).  Not sure how to
> evaluate the risk, but it was enough that Trond hasn't been comfortable
> with nfsd advertising NFS4_CHANGE_TYPE_IS_MONOTONIC.
>=20
> Ted's idea would be sufficient to allow us to turn that flag on, which I
> think allows some client-side optimizations.
>=20
>> The issue with a reboot counter (or similar) is that on an unclean crash
>> the NFS client would end up invalidating every inode in the cache, as
>> all of the i_versions would change. That's probably excessive.
>=20
> But if we use the crash counter on write instead of read, we don't
> invalidate caches unnecessarily.  And I think the monotonicity would
> still be close enough for our purposes?
>=20
>> The bigger issue (at the moment) is atomicity: when we fetch an
>> i_version, the natural inclination is to associate that with the state
>> of the inode at some point in time, so we need this to be updated
>> atomically with certain other attributes of the inode. That's the part
>> I'm trying to sort through at the moment.
>=20
> That may be, but I still suspect the crash counter would help.

Fwiw, I like the crash counter idea too.

--
Chuck Lever