Message ID | 20221017105709.10830-1-jlayton@kernel.org |
---|---|
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a5d:4ac7:0:0:0:0:0 with SMTP id y7csp1380823wrs; Mon, 17 Oct 2022 03:59:23 -0700 (PDT) X-Google-Smtp-Source: AMsMyM7RY2euhuwXoHmEYO15j6ixm6DLVonhHv9WysueGGx2f5/o9bDqr1EQ2VuoHfThxejbqYI8 X-Received: by 2002:a63:513:0:b0:462:f15e:5efc with SMTP id 19-20020a630513000000b00462f15e5efcmr10459135pgf.113.1666004363430; Mon, 17 Oct 2022 03:59:23 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1666004363; cv=none; d=google.com; s=arc-20160816; b=Lt5IdFiMbEomLHhz9i87imW86PrzWU3yHQEjQAXcBTcpKJL5qcCBekOgkG5S6rHSTm bZvCAI5u/zCrt6MVp+FCWMHMtPig03o6I1IWVaa8JeWflQ8rRY1Fr4lDNJiSDJzxjeJy zmQLy/GDDRrZ3Pvh/ZSxfQYbeHDxOwCbx8YgNTRNKTuBK/Kwvyn5lbiSToblA1EkXylS TvlHkWWLHOQ5s2nN89awKukJkjkOpMJqKqnjYiBA+wduNb1FjvjkcKeUUSyIyVyHmtUI afPmK/Blwfzhh58r27jt3cM9lqsuMZVL0tazQm3xbLyTbh9ctz3Ipct0Xi5NFNo1+TxH ijrQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :message-id:date:subject:cc:to:from:dkim-signature; bh=v9EpXY1D3ASApAQdh3WuFU7SoX45jkWKNznfRKh8Ow0=; b=ViTM8XcywGQWclAX5pFXaIZ/CTVXebxEs8DF+k47XidWxhGdTTOasWaYCf+27ltGUO 5g6QuKLo1OuAGW+BXLdPx7usy6A/x2coGtyYiFfpZsB+cJ5zW7fvqoVQI4Leo/5RqxHs gISfWlndClePIzvaCiNMPAGHDK/UHHB+qZYXvckFMPpspr4mnKIF7//SunLmOO2GnghX IHjXNbBAgxL9siebvFhuDlHtckP4xJlTlFYwNjAjrvzxnyo2nYSmYhx4YRQOXvzzsgsh 2F254100w6Ibq92JLVHUwKqaFYFNL3UebmvNH3yST6IW0itrAWv+IRKrEMpKaL56Z4oM IfKA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=Gj7zxfDu; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id q135-20020a632a8d000000b0042b911ab690si10233297pgq.772.2022.10.17.03.59.06; Mon, 17 Oct 2022 03:59:23 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=Gj7zxfDu; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230301AbiJQK5U (ORCPT <rfc822;ouuuleilei@gmail.com> + 99 others); Mon, 17 Oct 2022 06:57:20 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:44222 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230144AbiJQK5P (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Mon, 17 Oct 2022 06:57:15 -0400 Received: from dfw.source.kernel.org (dfw.source.kernel.org [IPv6:2604:1380:4641:c500::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 7D1845C9D7; Mon, 17 Oct 2022 03:57:14 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 0A0D66103A; Mon, 17 Oct 2022 10:57:14 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 6F241C433C1; Mon, 17 Oct 2022 10:57:11 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1666004233; bh=6YT8a4bXoeXs8XCfVDx3M0Me+DkkbCjCyiu7tCiJL+s=; h=From:To:Cc:Subject:Date:From; b=Gj7zxfDuAX7XjOBrXuruhtxnquTTTafw/403MMHjD9isqBeJP2M9YniIOu5CZ1rHj urP61o2T+SVbkLdDx4Kd4xuw55ioynZlC4Lts975BLgtdZjhxrBrXpWc907xShB3RC 2ZtNUB7vJ3PGARKYGkDXHUC8mlqaFGHy/HCM1lXgkJYGBdj96W0XXcWMLrWc4m1lFy uriZS+qEWlw2JfLC10mtBf4dpwQp2BpxhMSFxWgOepFJiQxd3HvBBFvhpJSHu3/Osg XvQn8aLxzoFoN6LtpkL6PD5Y3unGQ4WVbpFaN7RMDEA+ldANE+GjmmbKMMjYkCMBnr 9I+ZFiT+pBJSA== From: Jeff Layton <jlayton@kernel.org> To: tytso@mit.edu, adilger.kernel@dilger.ca, djwong@kernel.org, david@fromorbit.com, trondmy@hammerspace.com, neilb@suse.de, viro@zeniv.linux.org.uk, zohar@linux.ibm.com, xiubli@redhat.com, chuck.lever@oracle.com, lczerner@redhat.com, jack@suse.cz, bfields@fieldses.org, brauner@kernel.org, fweimer@redhat.com Cc: linux-btrfs@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org, ceph-devel@vger.kernel.org, linux-ext4@vger.kernel.org, linux-nfs@vger.kernel.org, linux-xfs@vger.kernel.org Subject: [PATCH v7 0/9] fs: clean up handling of i_version counter Date: Mon, 17 Oct 2022 06:57:00 -0400 Message-Id: <20221017105709.10830-1-jlayton@kernel.org> X-Mailer: git-send-email 2.37.3 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-7.4 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_HI, SPF_HELO_NONE,SPF_PASS 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-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1746932191338931334?= X-GMAIL-MSGID: =?utf-8?q?1746932191338931334?= |
Series |
fs: clean up handling of i_version counter
|
|
Message
Jeff Layton
Oct. 17, 2022, 10:57 a.m. UTC
This patchset is intended to clean up the handling of the i_version counter by nfsd. Most of the changes are to internal interfaces. This set is not intended to address crash resilience, or the fact that the counter is bumped before a change and not after. I intend to tackle those in follow-on patchsets. My intention is to get this series included into linux-next soon, with an eye toward merging most of it during the v6.2 merge window. The last patch in the series is probably not suitable for merge as-is, at least until we sort out the semantics we want to present to userland for it. Jeff Layton (9): fs: uninline inode_query_iversion fs: clarify when the i_version counter must be updated vfs: plumb i_version handling into struct kstat nfs: report the inode version in getattr if requested ceph: report the inode version in getattr if requested nfsd: move nfsd4_change_attribute to nfsfh.c nfsd: use the getattr operation to fetch i_version nfsd: remove fetch_iversion export operation vfs: expose STATX_VERSION to userland fs/ceph/inode.c | 16 +++++++---- fs/libfs.c | 36 ++++++++++++++++++++++++ fs/nfs/export.c | 7 ----- fs/nfs/inode.c | 15 +++++++--- fs/nfsd/nfs4xdr.c | 4 ++- fs/nfsd/nfsfh.c | 42 ++++++++++++++++++++++++++++ fs/nfsd/nfsfh.h | 29 +------------------- fs/nfsd/vfs.h | 7 ++++- fs/stat.c | 7 +++++ include/linux/exportfs.h | 1 - include/linux/iversion.h | 58 ++++++++++++++------------------------- include/linux/stat.h | 2 +- include/uapi/linux/stat.h | 6 ++-- samples/vfs/test-statx.c | 8 ++++-- 14 files changed, 148 insertions(+), 90 deletions(-)
Comments
On Mon, Oct 17, 2022 at 06:57:00AM -0400, Jeff Layton wrote: > This patchset is intended to clean up the handling of the i_version > counter by nfsd. Most of the changes are to internal interfaces. > > This set is not intended to address crash resilience, or the fact that > the counter is bumped before a change and not after. I intend to tackle > those in follow-on patchsets. > > My intention is to get this series included into linux-next soon, with > an eye toward merging most of it during the v6.2 merge window. The last > patch in the series is probably not suitable for merge as-is, at least > until we sort out the semantics we want to present to userland for it. Over the course of the series I struggled a bit - and sorry for losing focus - with what i_version is supposed to represent for userspace. So I would support not exposing it to userspace before that. But that shouldn't affect your other changes iiuc.
On Wed, 2022-10-19 at 13:13 +0200, Christian Brauner wrote: > On Mon, Oct 17, 2022 at 06:57:00AM -0400, Jeff Layton wrote: > > This patchset is intended to clean up the handling of the i_version > > counter by nfsd. Most of the changes are to internal interfaces. > > > > This set is not intended to address crash resilience, or the fact that > > the counter is bumped before a change and not after. I intend to tackle > > those in follow-on patchsets. > > > > My intention is to get this series included into linux-next soon, with > > an eye toward merging most of it during the v6.2 merge window. The last > > patch in the series is probably not suitable for merge as-is, at least > > until we sort out the semantics we want to present to userland for it. > > Over the course of the series I struggled a bit - and sorry for losing > focus - with what i_version is supposed to represent for userspace. So I > would support not exposing it to userspace before that. But that > shouldn't affect your other changes iiuc. Thanks Christian, It has been a real struggle to nail this down, and yeah I too am not planning to expose this to userland until we have this much better defined. Patch #9 is just to give you an idea of what this would ultimately look like. I intend to re-post the first 8 patches with an eye toward merge in v6.2, once we've settled on the naming. On that note... I believe you had mentioned that you didn't like STATX_CHANGE_ATTR for the name, and suggested STATX_I_VERSION (or something similar), which I later shortened to STATX_VERSION. Dave C. objected to STATX_VERSION, as "version" fields in a struct usually refer to the version of the struct itself rather than the version of the thing it describes. It also sort of implies a monotonic counter, and I'm not ready to require that just yet. What about STATX_CHANGE for the name (with corresponding names for the field and other flags)? That drops the redundant "_ATTR" postfix, while being sufficiently vague to allow for alternative implementations in the future. Do you (or anyone else) have other suggestions for a name?
On Wed, Oct 19, 2022 at 08:18:15AM -0400, Jeff Layton wrote: > On Wed, 2022-10-19 at 13:13 +0200, Christian Brauner wrote: > > On Mon, Oct 17, 2022 at 06:57:00AM -0400, Jeff Layton wrote: > > > This patchset is intended to clean up the handling of the i_version > > > counter by nfsd. Most of the changes are to internal interfaces. > > > > > > This set is not intended to address crash resilience, or the fact that > > > the counter is bumped before a change and not after. I intend to tackle > > > those in follow-on patchsets. > > > > > > My intention is to get this series included into linux-next soon, with > > > an eye toward merging most of it during the v6.2 merge window. The last > > > patch in the series is probably not suitable for merge as-is, at least > > > until we sort out the semantics we want to present to userland for it. > > > > Over the course of the series I struggled a bit - and sorry for losing > > focus - with what i_version is supposed to represent for userspace. So I > > would support not exposing it to userspace before that. But that > > shouldn't affect your other changes iiuc. > > Thanks Christian, > > It has been a real struggle to nail this down, and yeah I too am not > planning to expose this to userland until we have this much better > defined. Patch #9 is just to give you an idea of what this would > ultimately look like. I intend to re-post the first 8 patches with an > eye toward merge in v6.2, once we've settled on the naming. On that > note... > > I believe you had mentioned that you didn't like STATX_CHANGE_ATTR for > the name, and suggested STATX_I_VERSION (or something similar), which I > later shortened to STATX_VERSION. > > Dave C. objected to STATX_VERSION, as "version" fields in a struct > usually refer to the version of the struct itself rather than the > version of the thing it describes. It also sort of implies a monotonic > counter, and I'm not ready to require that just yet. > > What about STATX_CHANGE for the name (with corresponding names for the > field and other flags)? That drops the redundant "_ATTR" postfix, while > being sufficiently vague to allow for alternative implementations in the > future. > > Do you (or anyone else) have other suggestions for a name? Welllll it's really a u32 whose value doesn't have any intrinsic meaning other than "if (value_now != value_before) flush_cache();" right? I think it really only tracks changes to file data, right? STATX_CHANGE_COOKIE (wait, does this cookie augment i_ctime?) STATX_MOD_COOKIE (...or just file modifications/i_mtime?) STATX_MONITOR_COOKIE (...what are we monitoring??) STATX_MON_COOKIE STATX_COOKIE_MON STATX_COOKIE_MONSTER There we go. ;) In seriousness, I'd probably go with one of the first two. I wouldn't be opposed to the last one, either, but others may disagree. ;) --D > -- > Jeff Layton <jlayton@kernel.org>
On Wed, 2022-10-19 at 08:45 -0700, Darrick J. Wong wrote: > On Wed, Oct 19, 2022 at 08:18:15AM -0400, Jeff Layton wrote: > > On Wed, 2022-10-19 at 13:13 +0200, Christian Brauner wrote: > > > On Mon, Oct 17, 2022 at 06:57:00AM -0400, Jeff Layton wrote: > > > > This patchset is intended to clean up the handling of the i_version > > > > counter by nfsd. Most of the changes are to internal interfaces. > > > > > > > > This set is not intended to address crash resilience, or the fact that > > > > the counter is bumped before a change and not after. I intend to tackle > > > > those in follow-on patchsets. > > > > > > > > My intention is to get this series included into linux-next soon, with > > > > an eye toward merging most of it during the v6.2 merge window. The last > > > > patch in the series is probably not suitable for merge as-is, at least > > > > until we sort out the semantics we want to present to userland for it. > > > > > > Over the course of the series I struggled a bit - and sorry for losing > > > focus - with what i_version is supposed to represent for userspace. So I > > > would support not exposing it to userspace before that. But that > > > shouldn't affect your other changes iiuc. > > > > Thanks Christian, > > > > It has been a real struggle to nail this down, and yeah I too am not > > planning to expose this to userland until we have this much better > > defined. Patch #9 is just to give you an idea of what this would > > ultimately look like. I intend to re-post the first 8 patches with an > > eye toward merge in v6.2, once we've settled on the naming. On that > > note... > > > > I believe you had mentioned that you didn't like STATX_CHANGE_ATTR for > > the name, and suggested STATX_I_VERSION (or something similar), which I > > later shortened to STATX_VERSION. > > > > Dave C. objected to STATX_VERSION, as "version" fields in a struct > > usually refer to the version of the struct itself rather than the > > version of the thing it describes. It also sort of implies a monotonic > > counter, and I'm not ready to require that just yet. > > > > What about STATX_CHANGE for the name (with corresponding names for the > > field and other flags)? That drops the redundant "_ATTR" postfix, while > > being sufficiently vague to allow for alternative implementations in the > > future. > > > > Do you (or anyone else) have other suggestions for a name? > > Welllll it's really a u32 whose value doesn't have any intrinsic meaning > other than "if (value_now != value_before) flush_cache();" right? > I think it really only tracks changes to file data, right? > It's a u64, but yeah, you're not supposed to assign any intrinsic meaning to the value itself. > STATX_CHANGE_COOKIE (wait, does this cookie augment i_ctime?) > > STATX_MOD_COOKIE (...or just file modifications/i_mtime?) > > STATX_MONITOR_COOKIE (...what are we monitoring??) > > STATX_MON_COOKIE > > STATX_COOKIE_MON > > STATX_COOKIE_MONSTER > > There we go. ;) > > In seriousness, I'd probably go with one of the first two. I wouldn't > be opposed to the last one, either, but others may disagree. ;) > > --D > > STATX_CHANGE_COOKIE is probably the best one. I'll plan to go with that unless someone has a better idea. Thanks for the suggestions! Cheers,
On Wed, Oct 19, 2022 at 04:36:47PM -0400, Jeff Layton wrote: > On Wed, 2022-10-19 at 08:45 -0700, Darrick J. Wong wrote: > > On Wed, Oct 19, 2022 at 08:18:15AM -0400, Jeff Layton wrote: > > > On Wed, 2022-10-19 at 13:13 +0200, Christian Brauner wrote: > > > > On Mon, Oct 17, 2022 at 06:57:00AM -0400, Jeff Layton wrote: > > > > > This patchset is intended to clean up the handling of the i_version > > > > > counter by nfsd. Most of the changes are to internal interfaces. > > > > > > > > > > This set is not intended to address crash resilience, or the fact that > > > > > the counter is bumped before a change and not after. I intend to tackle > > > > > those in follow-on patchsets. > > > > > > > > > > My intention is to get this series included into linux-next soon, with > > > > > an eye toward merging most of it during the v6.2 merge window. The last > > > > > patch in the series is probably not suitable for merge as-is, at least > > > > > until we sort out the semantics we want to present to userland for it. > > > > > > > > Over the course of the series I struggled a bit - and sorry for losing > > > > focus - with what i_version is supposed to represent for userspace. So I > > > > would support not exposing it to userspace before that. But that > > > > shouldn't affect your other changes iiuc. > > > > > > Thanks Christian, > > > > > > It has been a real struggle to nail this down, and yeah I too am not > > > planning to expose this to userland until we have this much better > > > defined. Patch #9 is just to give you an idea of what this would > > > ultimately look like. I intend to re-post the first 8 patches with an > > > eye toward merge in v6.2, once we've settled on the naming. On that > > > note... > > > > > > I believe you had mentioned that you didn't like STATX_CHANGE_ATTR for > > > the name, and suggested STATX_I_VERSION (or something similar), which I > > > later shortened to STATX_VERSION. > > > > > > Dave C. objected to STATX_VERSION, as "version" fields in a struct > > > usually refer to the version of the struct itself rather than the > > > version of the thing it describes. It also sort of implies a monotonic > > > counter, and I'm not ready to require that just yet. > > > > > > What about STATX_CHANGE for the name (with corresponding names for the > > > field and other flags)? That drops the redundant "_ATTR" postfix, while > > > being sufficiently vague to allow for alternative implementations in the > > > future. > > > > > > Do you (or anyone else) have other suggestions for a name? > > > > Welllll it's really a u32 whose value doesn't have any intrinsic meaning > > other than "if (value_now != value_before) flush_cache();" right? > > I think it really only tracks changes to file data, right? > > > > It's a u64, but yeah, you're not supposed to assign any intrinsic > meaning to the value itself. > > > STATX_CHANGE_COOKIE (wait, does this cookie augment i_ctime?) > > > > STATX_MOD_COOKIE (...or just file modifications/i_mtime?) > > > > STATX_MONITOR_COOKIE (...what are we monitoring??) > > > > STATX_MON_COOKIE > > > > STATX_COOKIE_MON > > > > STATX_COOKIE_MONSTER > > > > There we go. ;) > > > > In seriousness, I'd probably go with one of the first two. I wouldn't > > be opposed to the last one, either, but others may disagree. ;) > > > > --D > > > > > > STATX_CHANGE_COOKIE is probably the best one. I'll plan to go with that > unless someone has a better idea. Thanks for the suggestions! Sounds fine to me.