From: Wang Yugui <wangyugui@e16-tech.com>
To: "NeilBrown" <neilb@suse.de>
Cc: "Christoph Hellwig" <hch@infradead.org>,
"Josef Bacik" <josef@toxicpanda.com>,
"J. Bruce Fields" <bfields@fieldses.org>,
"Chuck Lever" <chuck.lever@oracle.com>,
"Chris Mason" <clm@fb.com>, "David Sterba" <dsterba@suse.com>,
"Alexander Viro" <viro@zeniv.linux.org.uk>,
linux-fsdevel@vger.kernel.org, linux-nfs@vger.kernel.org,
linux-btrfs@vger.kernel.org
Subject: Re: [PATCH] VFS/BTRFS/NFSD: provide more unique inode number for btrfs export
Date: Wed, 18 Aug 2021 22:54:55 +0800 [thread overview]
Message-ID: <20210818225454.9558.409509F4@e16-tech.com> (raw)
In-Reply-To: <162881913686.1695.12479588032010502384@noble.neil.brown.name>
Hi,
We use 'swab64' to combinate 'subvol id' and 'inode' into 64bit in this
patch.
case1:
'subvol id': 16bit => 64K, a little small because the subvol id is
always increase?
'inode': 48bit * 4K per node, this is big enough.
case2:
'subvol id': 24bit => 16M, this is big enough.
'inode': 40bit * 4K per node => 4 PB. this is a little small?
Is there a way to 'bit-swap' the subvol id, rather the current byte-swap?
If not, maybe it is a better balance if we combinate 22bit subvol id and
42 bit inode?
Best Regards
Wang Yugui (wangyugui@e16-tech.com)
2021/08/18
>
> [[This patch is a minimal patch which addresses the current problems
> with nfsd and btrfs, in a way which I think is most supportable, least
> surprising, and least likely to impact any future attempts to more
> completely fix the btrfs file-identify problem]]
>
> BTRFS does not provide unique inode numbers across a filesystem.
> It *does* provide unique inode numbers with a subvolume and
> uses synthetic device numbers for different subvolumes to ensure
> uniqueness for device+inode.
>
> nfsd cannot use these varying device numbers. If nfsd were to
> synthesise different stable filesystem ids to give to the client, that
> would cause subvolumes to appear in the mount table on the client, even
> though they don't appear in the mount table on the server. Also, NFSv3
> doesn't support changing the filesystem id without a new explicit
> mount on the client (this is partially supported in practice, but
> violates the protocol specification).
>
> So currently, the roots of all subvolumes report the same inode number
> in the same filesystem to NFS clients and tools like 'find' notice that
> a directory has the same identity as an ancestor, and so refuse to
> enter that directory.
>
> This patch allows btrfs (or any filesystem) to provide a 64bit number
> that can be xored with the inode number to make the number more unique.
> Rather than the client being certain to see duplicates, with this patch
> it is possible but extremely rare.
>
> The number than btrfs provides is a swab64() version of the subvolume
> identifier. This has most entropy in the high bits (the low bits of the
> subvolume identifer), while the inoe has most entropy in the low bits.
> The result will always be unique within a subvolume, and will almost
> always be unique across the filesystem.
>
> Signed-off-by: NeilBrown <neilb@suse.de>
> ---
> fs/btrfs/inode.c | 4 ++++
> fs/nfsd/nfs3xdr.c | 17 ++++++++++++++++-
> fs/nfsd/nfs4xdr.c | 9 ++++++++-
> fs/nfsd/xdr3.h | 2 ++
> include/linux/stat.h | 17 +++++++++++++++++
> 5 files changed, 47 insertions(+), 2 deletions(-)
>
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index 0117d867ecf8..989fdf2032d5 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -9195,6 +9195,10 @@ static int btrfs_getattr(struct user_namespace *mnt_userns,
> generic_fillattr(&init_user_ns, inode, stat);
> stat->dev = BTRFS_I(inode)->root->anon_dev;
>
> + if (BTRFS_I(inode)->root->root_key.objectid != BTRFS_FS_TREE_OBJECTID)
> + stat->ino_uniquifier =
> + swab64(BTRFS_I(inode)->root->root_key.objectid);
> +
> spin_lock(&BTRFS_I(inode)->lock);
> delalloc_bytes = BTRFS_I(inode)->new_delalloc_bytes;
> inode_bytes = inode_get_bytes(inode);
> diff --git a/fs/nfsd/nfs3xdr.c b/fs/nfsd/nfs3xdr.c
> index 0a5ebc52e6a9..669e2437362a 100644
> --- a/fs/nfsd/nfs3xdr.c
> +++ b/fs/nfsd/nfs3xdr.c
> @@ -340,6 +340,7 @@ svcxdr_encode_fattr3(struct svc_rqst *rqstp, struct xdr_stream *xdr,
> {
> struct user_namespace *userns = nfsd_user_namespace(rqstp);
> __be32 *p;
> + u64 ino;
> u64 fsid;
>
> p = xdr_reserve_space(xdr, XDR_UNIT * 21);
> @@ -377,7 +378,10 @@ svcxdr_encode_fattr3(struct svc_rqst *rqstp, struct xdr_stream *xdr,
> p = xdr_encode_hyper(p, fsid);
>
> /* fileid */
> - p = xdr_encode_hyper(p, stat->ino);
> + ino = stat->ino;
> + if (stat->ino_uniquifier && stat->ino_uniquifier != ino)
> + ino ^= stat->ino_uniquifier;
> + p = xdr_encode_hyper(p, ino);
>
> p = encode_nfstime3(p, &stat->atime);
> p = encode_nfstime3(p, &stat->mtime);
> @@ -1151,6 +1155,17 @@ svcxdr_encode_entry3_common(struct nfsd3_readdirres *resp, const char *name,
> if (xdr_stream_encode_item_present(xdr) < 0)
> return false;
> /* fileid */
> + if (!resp->dir_have_uniquifier) {
> + struct kstat stat;
> + if (fh_getattr(&resp->fh, &stat) == nfs_ok)
> + resp->dir_ino_uniquifier = stat.ino_uniquifier;
> + else
> + resp->dir_ino_uniquifier = 0;
> + resp->dir_have_uniquifier = 1;
> + }
> + if (resp->dir_ino_uniquifier &&
> + resp->dir_ino_uniquifier != ino)
> + ino ^= resp->dir_ino_uniquifier;
> if (xdr_stream_encode_u64(xdr, ino) < 0)
> return false;
> /* name */
> diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
> index 7abeccb975b2..ddccf849c29c 100644
> --- a/fs/nfsd/nfs4xdr.c
> +++ b/fs/nfsd/nfs4xdr.c
> @@ -3114,10 +3114,14 @@ nfsd4_encode_fattr(struct xdr_stream *xdr, struct svc_fh *fhp,
> fhp->fh_handle.fh_size);
> }
> if (bmval0 & FATTR4_WORD0_FILEID) {
> + u64 ino = stat.ino;
> + if (stat.ino_uniquifier &&
> + stat.ino_uniquifier != stat.ino)
> + ino ^= stat.ino_uniquifier;
> p = xdr_reserve_space(xdr, 8);
> if (!p)
> goto out_resource;
> - p = xdr_encode_hyper(p, stat.ino);
> + p = xdr_encode_hyper(p, ino);
> }
> if (bmval0 & FATTR4_WORD0_FILES_AVAIL) {
> p = xdr_reserve_space(xdr, 8);
> @@ -3285,6 +3289,9 @@ nfsd4_encode_fattr(struct xdr_stream *xdr, struct svc_fh *fhp,
> if (err)
> goto out_nfserr;
> ino = parent_stat.ino;
> + if (parent_stat.ino_uniquifier &&
> + parent_stat.ino_uniquifier != ino)
> + ino ^= parent_stat.ino_uniquifier;
> }
> p = xdr_encode_hyper(p, ino);
> }
> diff --git a/fs/nfsd/xdr3.h b/fs/nfsd/xdr3.h
> index 933008382bbe..b4f9f3c71f72 100644
> --- a/fs/nfsd/xdr3.h
> +++ b/fs/nfsd/xdr3.h
> @@ -179,6 +179,8 @@ struct nfsd3_readdirres {
> struct xdr_buf dirlist;
> struct svc_fh scratch;
> struct readdir_cd common;
> + u64 dir_ino_uniquifier;
> + int dir_have_uniquifier;
> unsigned int cookie_offset;
> struct svc_rqst * rqstp;
>
> diff --git a/include/linux/stat.h b/include/linux/stat.h
> index fff27e603814..a5188f42ed81 100644
> --- a/include/linux/stat.h
> +++ b/include/linux/stat.h
> @@ -46,6 +46,23 @@ struct kstat {
> struct timespec64 btime; /* File creation time */
> u64 blocks;
> u64 mnt_id;
> + /*
> + * BTRFS does not provide unique inode numbers within a filesystem,
> + * depending on a synthetic 'dev' to provide uniqueness.
> + * NFSd cannot make use of this 'dev' number so clients often see
> + * duplicate inode numbers.
> + * For BTRFS, 'ino' is unlikely to use the high bits. It puts
> + * another number in ino_uniquifier which:
> + * - has most entropy in the high bits
> + * - is different precisely when 'dev' is different
> + * - is stable across unmount/remount
> + * NFSd can xor this with 'ino' to get a substantially more unique
> + * number for reporting to the client.
> + * The ino_uniquifier for a directory can reasonably be applied
> + * to inode numbers reported by the readdir filldir callback.
> + * It is NOT currently exported to user-space.
> + */
> + u64 ino_uniquifier;
> };
>
> #endif
> --
> 2.32.0
next prev parent reply other threads:[~2021-08-18 14:54 UTC|newest]
Thread overview: 122+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-07-27 22:37 [PATCH/RFC 00/11] expose btrfs subvols in mount table correctly NeilBrown
2021-07-27 22:37 ` [PATCH 07/11] exportfs: Allow filehandle lookup to cross internal mount points NeilBrown
2021-07-28 10:13 ` Amir Goldstein
2021-07-29 0:28 ` NeilBrown
2021-07-29 5:27 ` Amir Goldstein
2021-08-06 7:52 ` Miklos Szeredi
2021-08-06 8:08 ` Amir Goldstein
2021-08-06 8:18 ` Miklos Szeredi
2021-07-28 19:17 ` J. Bruce Fields
2021-07-28 22:25 ` NeilBrown
2021-07-27 22:37 ` [PATCH 04/11] VFS: export lookup_mnt() NeilBrown
2021-07-30 0:31 ` Al Viro
2021-07-30 5:33 ` NeilBrown
2021-07-27 22:37 ` [PATCH 01/11] VFS: show correct dev num in mountinfo NeilBrown
2021-07-30 0:25 ` Al Viro
2021-07-30 5:28 ` NeilBrown
2021-07-30 5:54 ` Miklos Szeredi
2021-07-30 6:13 ` NeilBrown
2021-07-30 7:18 ` Miklos Szeredi
2021-07-30 7:33 ` NeilBrown
2021-07-30 7:59 ` Miklos Szeredi
2021-08-02 4:18 ` A Third perspective on BTRFS nfsd subvol dev/inode number issues NeilBrown
2021-08-02 5:25 ` Al Viro
2021-08-02 5:40 ` NeilBrown
2021-08-02 7:54 ` Amir Goldstein
2021-08-02 13:53 ` Josef Bacik
2021-08-03 22:29 ` Qu Wenruo
2021-08-02 14:47 ` Frank Filz
2021-08-02 21:24 ` NeilBrown
2021-08-02 7:15 ` Martin Steigerwald
2021-08-02 21:40 ` NeilBrown
2021-08-02 12:39 ` J. Bruce Fields
2021-08-02 20:32 ` Patrick Goetz
2021-08-02 20:41 ` J. Bruce Fields
2021-08-02 21:10 ` NeilBrown
2021-08-02 21:50 ` J. Bruce Fields
2021-08-02 21:59 ` NeilBrown
2021-08-02 22:14 ` J. Bruce Fields
2021-08-02 22:36 ` NeilBrown
2021-08-03 0:15 ` J. Bruce Fields
2021-07-27 22:37 ` [PATCH 03/11] VFS: pass lookup_flags into follow_down() NeilBrown
2021-07-27 22:37 ` [PATCH 11/11] btrfs: use automount to bind-mount all subvol roots NeilBrown
2021-07-28 8:37 ` kernel test robot
2021-07-28 8:37 ` [RFC PATCH] btrfs: btrfs_mountpoint_expiry_timeout can be static kernel test robot
2021-07-28 13:12 ` [PATCH 11/11] btrfs: use automount to bind-mount all subvol roots Christian Brauner
2021-07-29 0:43 ` NeilBrown
2021-07-29 14:38 ` Christian Brauner
2021-07-31 6:25 ` [btrfs] 5874902268: xfstests.btrfs.202.fail kernel test robot
2021-07-27 22:37 ` [PATCH 06/11] nfsd: include a vfsmount in struct svc_fh NeilBrown
2021-07-27 22:37 ` [PATCH 10/11] btrfs: introduce mapping function from location to inum NeilBrown
2021-07-27 22:37 ` [PATCH 02/11] VFS: allow d_automount to create in-place bind-mount NeilBrown
2021-07-27 22:37 ` [PATCH 09/11] nfsd: Allow filehandle lookup to cross internal mount points NeilBrown
2021-07-28 19:15 ` J. Bruce Fields
2021-07-28 22:29 ` NeilBrown
2021-07-30 0:42 ` Al Viro
2021-07-30 5:43 ` NeilBrown
2021-07-27 22:37 ` [PATCH 08/11] nfsd: change get_parent_attributes() to nfsd_get_mounted_on() NeilBrown
2021-07-27 22:37 ` [PATCH 05/11] VFS: new function: mount_is_internal() NeilBrown
2021-07-28 2:16 ` Al Viro
2021-07-28 3:32 ` NeilBrown
2021-07-30 0:34 ` Al Viro
2021-07-28 2:19 ` [PATCH/RFC 00/11] expose btrfs subvols in mount table correctly Al Viro
2021-07-28 4:58 ` Wang Yugui
2021-07-28 6:04 ` Wang Yugui
2021-07-28 7:01 ` NeilBrown
2021-07-28 12:26 ` Neal Gompa
2021-07-28 19:14 ` J. Bruce Fields
2021-07-29 1:29 ` Zygo Blaxell
2021-07-29 1:43 ` NeilBrown
2021-07-29 23:20 ` Zygo Blaxell
2021-07-28 22:50 ` NeilBrown
2021-07-29 2:37 ` Zygo Blaxell
2021-07-29 3:36 ` NeilBrown
2021-07-29 23:20 ` Zygo Blaxell
2021-07-30 2:36 ` NeilBrown
2021-07-30 5:25 ` Qu Wenruo
2021-07-30 5:31 ` Qu Wenruo
2021-07-30 5:53 ` Amir Goldstein
2021-07-30 6:00 ` NeilBrown
2021-07-30 6:09 ` Qu Wenruo
2021-07-30 5:58 ` NeilBrown
2021-07-30 6:23 ` Qu Wenruo
2021-07-30 6:53 ` NeilBrown
2021-07-30 7:09 ` Qu Wenruo
2021-07-30 18:15 ` Zygo Blaxell
2021-07-30 15:17 ` J. Bruce Fields
2021-07-30 15:48 ` Josef Bacik
2021-07-30 16:25 ` Forza
2021-07-30 17:43 ` Zygo Blaxell
2021-07-30 5:28 ` Amir Goldstein
2021-07-28 13:43 ` g.btrfs
2021-07-29 1:39 ` NeilBrown
2021-07-29 9:28 ` Graham Cobb
2021-07-28 7:06 ` NeilBrown
2021-07-28 9:36 ` Wang Yugui
2021-07-28 19:35 ` J. Bruce Fields
2021-07-28 21:30 ` Josef Bacik
2021-07-30 0:13 ` Al Viro
2021-07-30 6:08 ` NeilBrown
2021-08-13 1:45 ` [PATCH] VFS/BTRFS/NFSD: provide more unique inode number for btrfs export NeilBrown
2021-08-13 14:55 ` Josef Bacik
2021-08-15 7:39 ` Goffredo Baroncelli
2021-08-15 19:35 ` Roman Mamedov
2021-08-15 21:03 ` Goffredo Baroncelli
2021-08-15 21:53 ` NeilBrown
2021-08-17 19:34 ` Goffredo Baroncelli
2021-08-17 21:39 ` NeilBrown
2021-08-18 17:24 ` Goffredo Baroncelli
2021-08-15 22:17 ` NeilBrown
2021-08-19 8:01 ` Amir Goldstein
2021-08-20 3:21 ` NeilBrown
2021-08-20 6:23 ` Amir Goldstein
2021-08-23 4:05 ` [PATCH v2] BTRFS/NFSD: " NeilBrown
2021-08-18 14:54 ` Wang Yugui [this message]
2021-08-18 21:46 ` [PATCH] VFS/BTRFS/NFSD: " NeilBrown
2021-08-19 2:19 ` Zygo Blaxell
2021-08-20 2:54 ` NeilBrown
2021-08-22 19:29 ` Zygo Blaxell
2021-08-23 5:51 ` NeilBrown
2021-08-23 23:22 ` NeilBrown
2021-08-25 2:06 ` Zygo Blaxell
2021-08-23 0:57 ` Wang Yugui
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=20210818225454.9558.409509F4@e16-tech.com \
--to=wangyugui@e16-tech.com \
--cc=bfields@fieldses.org \
--cc=chuck.lever@oracle.com \
--cc=clm@fb.com \
--cc=dsterba@suse.com \
--cc=hch@infradead.org \
--cc=josef@toxicpanda.com \
--cc=linux-btrfs@vger.kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-nfs@vger.kernel.org \
--cc=neilb@suse.de \
--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).