Re: [f2fs-dev] [PATCH v6 1/7] fs: pass the request_mask to generic_fillattr
On Tue, Aug 29, 2023 at 08:43:31PM -0400, Jeff Layton wrote: > On Wed, 2023-08-30 at 01:02 +0100, Al Viro wrote: > > On Tue, Aug 29, 2023 at 06:58:47PM -0400, Jeff Layton wrote: > > > On Tue, 2023-08-29 at 23:44 +0100, Al Viro wrote: > > > > On Tue, Jul 25, 2023 at 10:58:14AM -0400, Jeff Layton wrote: > > > > > generic_fillattr just fills in the entire stat struct indiscriminately > > > > > today, copying data from the inode. There is at least one attribute > > > > > (STATX_CHANGE_COOKIE) that can have side effects when it is reported, > > > > > and we're looking at adding more with the addition of multigrain > > > > > timestamps. > > > > > > > > > > Add a request_mask argument to generic_fillattr and have most callers > > > > > just pass in the value that is passed to getattr. Have other callers > > > > > (e.g. ksmbd) just pass in STATX_BASIC_STATS. Also move the setting of > > > > > STATX_CHANGE_COOKIE into generic_fillattr. > > > > > > > > Out of curiosity - how much PITA would it be to put request_mask into > > > > kstat? Set it in vfs_getattr_nosec() (and those get_file_..._info() > > > > on smbd side) and don't bother with that kind of propagation boilerplate > > > > - just have generic_fillattr() pick it there... > > > > > > > > Reduces the patchset size quite a bit... > > > > > > It could be done. To do that right, I think we'd want to drop > > > request_mask from the ->getattr prototype as well and just have > > > everything use the mask in the kstat. > > > > > > I don't think it'd reduce the size of the patchset in any meaningful > > > way, but it might make for a more sensible API over the long haul. > > > > ->getattr() prototype change would be decoupled from that - for your > > patchset you'd only need the field addition + setting in vfs_getattr_nosec() > > (and possibly in ksmbd), with the remainders of both series being > > independent from each other. > > > > What I suggest is > > > > branchpoint -> field addition (trivial commit) -> argument removal > > | > > V > > your series, starting with "use stat->request_mask in generic_fillattr()" > > > > Total size would be about the same, but it would be easier to follow > > the less trivial part of that. Nothing in your branch downstream of > > that touches any ->getattr() instances, so it should have no > > conflicts with the argument removal side of things. > > The only problem with this plan is that Linus has already merged this. > I've no issue with adding the request_mask to the kstat and removing it > as a separate parameter elsewhere, but I think we'll need to do it on > top of what's already been merged. D'oh... My apologies; I'll do a branch on top of that (and rebase on top of -rc1 once the window closes). ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH v6 1/7] fs: pass the request_mask to generic_fillattr
On Wed, 2023-08-30 at 01:02 +0100, Al Viro wrote: > On Tue, Aug 29, 2023 at 06:58:47PM -0400, Jeff Layton wrote: > > On Tue, 2023-08-29 at 23:44 +0100, Al Viro wrote: > > > On Tue, Jul 25, 2023 at 10:58:14AM -0400, Jeff Layton wrote: > > > > generic_fillattr just fills in the entire stat struct indiscriminately > > > > today, copying data from the inode. There is at least one attribute > > > > (STATX_CHANGE_COOKIE) that can have side effects when it is reported, > > > > and we're looking at adding more with the addition of multigrain > > > > timestamps. > > > > > > > > Add a request_mask argument to generic_fillattr and have most callers > > > > just pass in the value that is passed to getattr. Have other callers > > > > (e.g. ksmbd) just pass in STATX_BASIC_STATS. Also move the setting of > > > > STATX_CHANGE_COOKIE into generic_fillattr. > > > > > > Out of curiosity - how much PITA would it be to put request_mask into > > > kstat? Set it in vfs_getattr_nosec() (and those get_file_..._info() > > > on smbd side) and don't bother with that kind of propagation boilerplate > > > - just have generic_fillattr() pick it there... > > > > > > Reduces the patchset size quite a bit... > > > > It could be done. To do that right, I think we'd want to drop > > request_mask from the ->getattr prototype as well and just have > > everything use the mask in the kstat. > > > > I don't think it'd reduce the size of the patchset in any meaningful > > way, but it might make for a more sensible API over the long haul. > > ->getattr() prototype change would be decoupled from that - for your > patchset you'd only need the field addition + setting in vfs_getattr_nosec() > (and possibly in ksmbd), with the remainders of both series being > independent from each other. > > What I suggest is > > branchpoint -> field addition (trivial commit) -> argument removal > | > V > your series, starting with "use stat->request_mask in generic_fillattr()" > > Total size would be about the same, but it would be easier to follow > the less trivial part of that. Nothing in your branch downstream of > that touches any ->getattr() instances, so it should have no > conflicts with the argument removal side of things. The only problem with this plan is that Linus has already merged this. I've no issue with adding the request_mask to the kstat and removing it as a separate parameter elsewhere, but I think we'll need to do it on top of what's already been merged. -- Jeff Layton ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH v6 1/7] fs: pass the request_mask to generic_fillattr
On Tue, Aug 29, 2023 at 06:58:47PM -0400, Jeff Layton wrote: > On Tue, 2023-08-29 at 23:44 +0100, Al Viro wrote: > > On Tue, Jul 25, 2023 at 10:58:14AM -0400, Jeff Layton wrote: > > > generic_fillattr just fills in the entire stat struct indiscriminately > > > today, copying data from the inode. There is at least one attribute > > > (STATX_CHANGE_COOKIE) that can have side effects when it is reported, > > > and we're looking at adding more with the addition of multigrain > > > timestamps. > > > > > > Add a request_mask argument to generic_fillattr and have most callers > > > just pass in the value that is passed to getattr. Have other callers > > > (e.g. ksmbd) just pass in STATX_BASIC_STATS. Also move the setting of > > > STATX_CHANGE_COOKIE into generic_fillattr. > > > > Out of curiosity - how much PITA would it be to put request_mask into > > kstat? Set it in vfs_getattr_nosec() (and those get_file_..._info() > > on smbd side) and don't bother with that kind of propagation boilerplate > > - just have generic_fillattr() pick it there... > > > > Reduces the patchset size quite a bit... > > It could be done. To do that right, I think we'd want to drop > request_mask from the ->getattr prototype as well and just have > everything use the mask in the kstat. > > I don't think it'd reduce the size of the patchset in any meaningful > way, but it might make for a more sensible API over the long haul. ->getattr() prototype change would be decoupled from that - for your patchset you'd only need the field addition + setting in vfs_getattr_nosec() (and possibly in ksmbd), with the remainders of both series being independent from each other. What I suggest is branchpoint -> field addition (trivial commit) -> argument removal | V your series, starting with "use stat->request_mask in generic_fillattr()" Total size would be about the same, but it would be easier to follow the less trivial part of that. Nothing in your branch downstream of that touches any ->getattr() instances, so it should have no conflicts with the argument removal side of things. ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH v6 1/7] fs: pass the request_mask to generic_fillattr
On Tue, 2023-08-29 at 23:44 +0100, Al Viro wrote: > On Tue, Jul 25, 2023 at 10:58:14AM -0400, Jeff Layton wrote: > > generic_fillattr just fills in the entire stat struct indiscriminately > > today, copying data from the inode. There is at least one attribute > > (STATX_CHANGE_COOKIE) that can have side effects when it is reported, > > and we're looking at adding more with the addition of multigrain > > timestamps. > > > > Add a request_mask argument to generic_fillattr and have most callers > > just pass in the value that is passed to getattr. Have other callers > > (e.g. ksmbd) just pass in STATX_BASIC_STATS. Also move the setting of > > STATX_CHANGE_COOKIE into generic_fillattr. > > Out of curiosity - how much PITA would it be to put request_mask into > kstat? Set it in vfs_getattr_nosec() (and those get_file_..._info() > on smbd side) and don't bother with that kind of propagation boilerplate > - just have generic_fillattr() pick it there... > > Reduces the patchset size quite a bit... It could be done. To do that right, I think we'd want to drop request_mask from the ->getattr prototype as well and just have everything use the mask in the kstat. I don't think it'd reduce the size of the patchset in any meaningful way, but it might make for a more sensible API over the long haul. -- Jeff Layton ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH v6 1/7] fs: pass the request_mask to generic_fillattr
On Tue, Jul 25, 2023 at 10:58:14AM -0400, Jeff Layton wrote: > generic_fillattr just fills in the entire stat struct indiscriminately > today, copying data from the inode. There is at least one attribute > (STATX_CHANGE_COOKIE) that can have side effects when it is reported, > and we're looking at adding more with the addition of multigrain > timestamps. > > Add a request_mask argument to generic_fillattr and have most callers > just pass in the value that is passed to getattr. Have other callers > (e.g. ksmbd) just pass in STATX_BASIC_STATS. Also move the setting of > STATX_CHANGE_COOKIE into generic_fillattr. Out of curiosity - how much PITA would it be to put request_mask into kstat? Set it in vfs_getattr_nosec() (and those get_file_..._info() on smbd side) and don't bother with that kind of propagation boilerplate - just have generic_fillattr() pick it there... Reduces the patchset size quite a bit... ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH v6 1/7] fs: pass the request_mask to generic_fillattr
Jeff Layton writes: > generic_fillattr just fills in the entire stat struct indiscriminately > today, copying data from the inode. There is at least one attribute > (STATX_CHANGE_COOKIE) that can have side effects when it is reported, > and we're looking at adding more with the addition of multigrain > timestamps. > > Add a request_mask argument to generic_fillattr and have most callers > just pass in the value that is passed to getattr. Have other callers > (e.g. ksmbd) just pass in STATX_BASIC_STATS. Also move the setting of > STATX_CHANGE_COOKIE into generic_fillattr. > > Signed-off-by: Jeff Layton > --- > fs/9p/vfs_inode.c | 4 ++-- > fs/9p/vfs_inode_dotl.c | 4 ++-- > fs/afs/inode.c | 2 +- > fs/btrfs/inode.c| 2 +- > fs/ceph/inode.c | 2 +- > fs/coda/inode.c | 3 ++- > fs/ecryptfs/inode.c | 5 +++-- > fs/erofs/inode.c| 2 +- > fs/exfat/file.c | 2 +- > fs/ext2/inode.c | 2 +- > fs/ext4/inode.c | 2 +- > fs/f2fs/file.c | 2 +- > fs/fat/file.c | 2 +- > fs/fuse/dir.c | 2 +- > fs/gfs2/inode.c | 2 +- > fs/hfsplus/inode.c | 2 +- > fs/kernfs/inode.c | 2 +- > fs/libfs.c | 4 ++-- > fs/minix/inode.c| 2 +- > fs/nfs/inode.c | 2 +- > fs/nfs/namespace.c | 3 ++- > fs/ntfs3/file.c | 2 +- > fs/ocfs2/file.c | 2 +- > fs/orangefs/inode.c | 2 +- > fs/proc/base.c | 4 ++-- > fs/proc/fd.c| 2 +- > fs/proc/generic.c | 2 +- > fs/proc/proc_net.c | 2 +- > fs/proc/proc_sysctl.c | 2 +- > fs/proc/root.c | 3 ++- > fs/smb/client/inode.c | 2 +- > fs/smb/server/smb2pdu.c | 22 +++--- > fs/smb/server/vfs.c | 3 ++- > fs/stat.c | 18 ++ > fs/sysv/itree.c | 3 ++- > fs/ubifs/dir.c | 2 +- > fs/udf/symlink.c| 2 +- > fs/vboxsf/utils.c | 2 +- > include/linux/fs.h | 2 +- > mm/shmem.c | 2 +- > 40 files changed, 70 insertions(+), 62 deletions(-) > > [...] > > diff --git a/fs/smb/client/inode.c b/fs/smb/client/inode.c > index 218f03dd3f52..93fe43789d7a 100644 > --- a/fs/smb/client/inode.c > +++ b/fs/smb/client/inode.c > @@ -2540,7 +2540,7 @@ int cifs_getattr(struct mnt_idmap *idmap, const struct > path *path, > return rc; > } > > - generic_fillattr(_mnt_idmap, inode, stat); > + generic_fillattr(_mnt_idmap, request_mask, inode, stat); > stat->blksize = cifs_sb->ctx->bsize; > stat->ino = CIFS_I(inode)->uniqueid; Reviewed-by: Paulo Alcantara (SUSE) ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH v6 1/7] fs: pass the request_mask to generic_fillattr
On Tue 25-07-23 10:58:14, Jeff Layton wrote:
> generic_fillattr just fills in the entire stat struct indiscriminately
> today, copying data from the inode. There is at least one attribute
> (STATX_CHANGE_COOKIE) that can have side effects when it is reported,
> and we're looking at adding more with the addition of multigrain
> timestamps.
>
> Add a request_mask argument to generic_fillattr and have most callers
> just pass in the value that is passed to getattr. Have other callers
> (e.g. ksmbd) just pass in STATX_BASIC_STATS. Also move the setting of
> STATX_CHANGE_COOKIE into generic_fillattr.
>
> Signed-off-by: Jeff Layton
Looks good. Feel free to add:
Reviewed-by: Jan Kara
Honza
> ---
> fs/9p/vfs_inode.c | 4 ++--
> fs/9p/vfs_inode_dotl.c | 4 ++--
> fs/afs/inode.c | 2 +-
> fs/btrfs/inode.c| 2 +-
> fs/ceph/inode.c | 2 +-
> fs/coda/inode.c | 3 ++-
> fs/ecryptfs/inode.c | 5 +++--
> fs/erofs/inode.c| 2 +-
> fs/exfat/file.c | 2 +-
> fs/ext2/inode.c | 2 +-
> fs/ext4/inode.c | 2 +-
> fs/f2fs/file.c | 2 +-
> fs/fat/file.c | 2 +-
> fs/fuse/dir.c | 2 +-
> fs/gfs2/inode.c | 2 +-
> fs/hfsplus/inode.c | 2 +-
> fs/kernfs/inode.c | 2 +-
> fs/libfs.c | 4 ++--
> fs/minix/inode.c| 2 +-
> fs/nfs/inode.c | 2 +-
> fs/nfs/namespace.c | 3 ++-
> fs/ntfs3/file.c | 2 +-
> fs/ocfs2/file.c | 2 +-
> fs/orangefs/inode.c | 2 +-
> fs/proc/base.c | 4 ++--
> fs/proc/fd.c| 2 +-
> fs/proc/generic.c | 2 +-
> fs/proc/proc_net.c | 2 +-
> fs/proc/proc_sysctl.c | 2 +-
> fs/proc/root.c | 3 ++-
> fs/smb/client/inode.c | 2 +-
> fs/smb/server/smb2pdu.c | 22 +++---
> fs/smb/server/vfs.c | 3 ++-
> fs/stat.c | 18 ++
> fs/sysv/itree.c | 3 ++-
> fs/ubifs/dir.c | 2 +-
> fs/udf/symlink.c| 2 +-
> fs/vboxsf/utils.c | 2 +-
> include/linux/fs.h | 2 +-
> mm/shmem.c | 2 +-
> 40 files changed, 70 insertions(+), 62 deletions(-)
>
> diff --git a/fs/9p/vfs_inode.c b/fs/9p/vfs_inode.c
> index 16d85e6033a3..d24d1f20e922 100644
> --- a/fs/9p/vfs_inode.c
> +++ b/fs/9p/vfs_inode.c
> @@ -1016,7 +1016,7 @@ v9fs_vfs_getattr(struct mnt_idmap *idmap, const struct
> path *path,
> p9_debug(P9_DEBUG_VFS, "dentry: %p\n", dentry);
> v9ses = v9fs_dentry2v9ses(dentry);
> if (v9ses->cache & (CACHE_META|CACHE_LOOSE)) {
> - generic_fillattr(_mnt_idmap, inode, stat);
> + generic_fillattr(_mnt_idmap, request_mask, inode, stat);
> return 0;
> } else if (v9ses->cache & CACHE_WRITEBACK) {
> if (S_ISREG(inode->i_mode)) {
> @@ -1037,7 +1037,7 @@ v9fs_vfs_getattr(struct mnt_idmap *idmap, const struct
> path *path,
> return PTR_ERR(st);
>
> v9fs_stat2inode(st, d_inode(dentry), dentry->d_sb, 0);
> - generic_fillattr(_mnt_idmap, d_inode(dentry), stat);
> + generic_fillattr(_mnt_idmap, request_mask, d_inode(dentry), stat);
>
> p9stat_free(st);
> kfree(st);
> diff --git a/fs/9p/vfs_inode_dotl.c b/fs/9p/vfs_inode_dotl.c
> index 464ea73d1bf8..8e8d5d2a13d8 100644
> --- a/fs/9p/vfs_inode_dotl.c
> +++ b/fs/9p/vfs_inode_dotl.c
> @@ -451,7 +451,7 @@ v9fs_vfs_getattr_dotl(struct mnt_idmap *idmap,
> p9_debug(P9_DEBUG_VFS, "dentry: %p\n", dentry);
> v9ses = v9fs_dentry2v9ses(dentry);
> if (v9ses->cache & (CACHE_META|CACHE_LOOSE)) {
> - generic_fillattr(_mnt_idmap, inode, stat);
> + generic_fillattr(_mnt_idmap, request_mask, inode, stat);
> return 0;
> } else if (v9ses->cache) {
> if (S_ISREG(inode->i_mode)) {
> @@ -476,7 +476,7 @@ v9fs_vfs_getattr_dotl(struct mnt_idmap *idmap,
> return PTR_ERR(st);
>
> v9fs_stat2inode_dotl(st, d_inode(dentry), 0);
> - generic_fillattr(_mnt_idmap, d_inode(dentry), stat);
> + generic_fillattr(_mnt_idmap, request_mask, d_inode(dentry), stat);
> /* Change block size to what the server returned */
> stat->blksize = st->st_blksize;
>
> diff --git a/fs/afs/inode.c b/fs/afs/inode.c
> index 6b636f43f548..1c794a1896aa 100644
> --- a/fs/afs/inode.c
> +++ b/fs/afs/inode.c
> @@ -773,7 +773,7 @@ int afs_getattr(struct mnt_idmap *idmap, const struct
> path *path,
>
> do {
> read_seqbegin_or_lock(>cb_lock, );
> - generic_fillattr(_mnt_idmap, inode, stat);
> + generic_fillattr(_mnt_idmap, request_mask, inode, stat);
> if (test_bit(AFS_VNODE_SILLY_DELETED, >flags) &&
> stat->nlink > 0)
> stat->nlink -= 1;
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index
Re: [f2fs-dev] [PATCH v6 1/7] fs: pass the request_mask to generic_fillattr
On Wed, 2023-07-26 at 17:40 +0800, Joseph Qi wrote:
>
> On 7/25/23 10:58 PM, Jeff Layton wrote:
> > generic_fillattr just fills in the entire stat struct indiscriminately
> > today, copying data from the inode. There is at least one attribute
> > (STATX_CHANGE_COOKIE) that can have side effects when it is reported,
> > and we're looking at adding more with the addition of multigrain
> > timestamps.
> >
> > Add a request_mask argument to generic_fillattr and have most callers
> > just pass in the value that is passed to getattr. Have other callers
> > (e.g. ksmbd) just pass in STATX_BASIC_STATS. Also move the setting of
> > STATX_CHANGE_COOKIE into generic_fillattr.
> >
> > Signed-off-by: Jeff Layton
> > ---
> > fs/9p/vfs_inode.c | 4 ++--
> > fs/9p/vfs_inode_dotl.c | 4 ++--
> > fs/afs/inode.c | 2 +-
> > fs/btrfs/inode.c| 2 +-
> > fs/ceph/inode.c | 2 +-
> > fs/coda/inode.c | 3 ++-
> > fs/ecryptfs/inode.c | 5 +++--
> > fs/erofs/inode.c| 2 +-
> > fs/exfat/file.c | 2 +-
> > fs/ext2/inode.c | 2 +-
> > fs/ext4/inode.c | 2 +-
> > fs/f2fs/file.c | 2 +-
> > fs/fat/file.c | 2 +-
> > fs/fuse/dir.c | 2 +-
> > fs/gfs2/inode.c | 2 +-
> > fs/hfsplus/inode.c | 2 +-
> > fs/kernfs/inode.c | 2 +-
> > fs/libfs.c | 4 ++--
> > fs/minix/inode.c| 2 +-
> > fs/nfs/inode.c | 2 +-
> > fs/nfs/namespace.c | 3 ++-
> > fs/ntfs3/file.c | 2 +-
> > fs/ocfs2/file.c | 2 +-
> > fs/orangefs/inode.c | 2 +-
> > fs/proc/base.c | 4 ++--
> > fs/proc/fd.c| 2 +-
> > fs/proc/generic.c | 2 +-
> > fs/proc/proc_net.c | 2 +-
> > fs/proc/proc_sysctl.c | 2 +-
> > fs/proc/root.c | 3 ++-
> > fs/smb/client/inode.c | 2 +-
> > fs/smb/server/smb2pdu.c | 22 +++---
> > fs/smb/server/vfs.c | 3 ++-
> > fs/stat.c | 18 ++
> > fs/sysv/itree.c | 3 ++-
> > fs/ubifs/dir.c | 2 +-
> > fs/udf/symlink.c| 2 +-
> > fs/vboxsf/utils.c | 2 +-
> > include/linux/fs.h | 2 +-
> > mm/shmem.c | 2 +-
> > 40 files changed, 70 insertions(+), 62 deletions(-)
> >
>
> ...
>
> > diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
> > index 1b337ebce4df..8184499ae7a5 100644
> > --- a/fs/ocfs2/file.c
> > +++ b/fs/ocfs2/file.c
> > @@ -1319,7 +1319,7 @@ int ocfs2_getattr(struct mnt_idmap *idmap, const
> > struct path *path,
> > goto bail;
> > }
> >
> > - generic_fillattr(_mnt_idmap, inode, stat);
> > + generic_fillattr(_mnt_idmap, request_mask, inode, stat);
>
> For ocfs2 part, looks fine to me.
>
> Acked-by: Joseph Qi
>
> > /*
> > * If there is inline data in the inode, the inode will normally not
> > * have data blocks allocated (it may have an external xattr block).
>
> ...
>
> > diff --git a/fs/stat.c b/fs/stat.c
> > index 8c2b30af19f5..062f311b5386 100644
> > --- a/fs/stat.c
> > +++ b/fs/stat.c
> > @@ -29,6 +29,7 @@
> > /**
> > * generic_fillattr - Fill in the basic attributes from the inode struct
> > * @idmap: idmap of the mount the inode was found from
> > + * @req_mask statx request_mask
>
> s/req_mask/request_mask
>
Thanks. Fixed in my tree.
> > * @inode: Inode to use as the source
> > * @stat: Where to fill in the attributes
> > *
> > @@ -42,8 +43,8 @@
> > * uid and gid filds. On non-idmapped mounts or if permission checking is
> > to be
> > * performed on the raw inode simply passs @nop_mnt_idmap.
> > */
> > -void generic_fillattr(struct mnt_idmap *idmap, struct inode *inode,
> > - struct kstat *stat)
> > +void generic_fillattr(struct mnt_idmap *idmap, u32 request_mask,
> > + struct inode *inode, struct kstat *stat)
> > {
> > vfsuid_t vfsuid = i_uid_into_vfsuid(idmap, inode);
> > vfsgid_t vfsgid = i_gid_into_vfsgid(idmap, inode);
> > @@ -61,6 +62,12 @@ void generic_fillattr(struct mnt_idmap *idmap, struct
> > inode *inode,
> > stat->ctime = inode_get_ctime(inode);
> > stat->blksize = i_blocksize(inode);
> > stat->blocks = inode->i_blocks;
> > +
> > + if ((request_mask & STATX_CHANGE_COOKIE) && IS_I_VERSION(inode)) {
> > + stat->result_mask |= STATX_CHANGE_COOKIE;
> > + stat->change_cookie = inode_query_iversion(inode);
> > + }
> > +
> > }
> > EXPORT_SYMBOL(generic_fillattr);
> >
> > @@ -123,17 +130,12 @@ int vfs_getattr_nosec(const struct path *path, struct
> > kstat *stat,
> > stat->attributes_mask |= (STATX_ATTR_AUTOMOUNT |
> > STATX_ATTR_DAX);
> >
> > - if ((request_mask & STATX_CHANGE_COOKIE) && IS_I_VERSION(inode)) {
> > - stat->result_mask |= STATX_CHANGE_COOKIE;
> > - stat->change_cookie = inode_query_iversion(inode);
> > - }
> > -
> > idmap = mnt_idmap(path->mnt);
> >
Re: [f2fs-dev] [PATCH v6 1/7] fs: pass the request_mask to generic_fillattr
On 7/25/23 10:58 PM, Jeff Layton wrote:
> generic_fillattr just fills in the entire stat struct indiscriminately
> today, copying data from the inode. There is at least one attribute
> (STATX_CHANGE_COOKIE) that can have side effects when it is reported,
> and we're looking at adding more with the addition of multigrain
> timestamps.
>
> Add a request_mask argument to generic_fillattr and have most callers
> just pass in the value that is passed to getattr. Have other callers
> (e.g. ksmbd) just pass in STATX_BASIC_STATS. Also move the setting of
> STATX_CHANGE_COOKIE into generic_fillattr.
>
> Signed-off-by: Jeff Layton
> ---
> fs/9p/vfs_inode.c | 4 ++--
> fs/9p/vfs_inode_dotl.c | 4 ++--
> fs/afs/inode.c | 2 +-
> fs/btrfs/inode.c| 2 +-
> fs/ceph/inode.c | 2 +-
> fs/coda/inode.c | 3 ++-
> fs/ecryptfs/inode.c | 5 +++--
> fs/erofs/inode.c| 2 +-
> fs/exfat/file.c | 2 +-
> fs/ext2/inode.c | 2 +-
> fs/ext4/inode.c | 2 +-
> fs/f2fs/file.c | 2 +-
> fs/fat/file.c | 2 +-
> fs/fuse/dir.c | 2 +-
> fs/gfs2/inode.c | 2 +-
> fs/hfsplus/inode.c | 2 +-
> fs/kernfs/inode.c | 2 +-
> fs/libfs.c | 4 ++--
> fs/minix/inode.c| 2 +-
> fs/nfs/inode.c | 2 +-
> fs/nfs/namespace.c | 3 ++-
> fs/ntfs3/file.c | 2 +-
> fs/ocfs2/file.c | 2 +-
> fs/orangefs/inode.c | 2 +-
> fs/proc/base.c | 4 ++--
> fs/proc/fd.c| 2 +-
> fs/proc/generic.c | 2 +-
> fs/proc/proc_net.c | 2 +-
> fs/proc/proc_sysctl.c | 2 +-
> fs/proc/root.c | 3 ++-
> fs/smb/client/inode.c | 2 +-
> fs/smb/server/smb2pdu.c | 22 +++---
> fs/smb/server/vfs.c | 3 ++-
> fs/stat.c | 18 ++
> fs/sysv/itree.c | 3 ++-
> fs/ubifs/dir.c | 2 +-
> fs/udf/symlink.c| 2 +-
> fs/vboxsf/utils.c | 2 +-
> include/linux/fs.h | 2 +-
> mm/shmem.c | 2 +-
> 40 files changed, 70 insertions(+), 62 deletions(-)
>
...
> diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
> index 1b337ebce4df..8184499ae7a5 100644
> --- a/fs/ocfs2/file.c
> +++ b/fs/ocfs2/file.c
> @@ -1319,7 +1319,7 @@ int ocfs2_getattr(struct mnt_idmap *idmap, const struct
> path *path,
> goto bail;
> }
>
> - generic_fillattr(_mnt_idmap, inode, stat);
> + generic_fillattr(_mnt_idmap, request_mask, inode, stat);
For ocfs2 part, looks fine to me.
Acked-by: Joseph Qi
> /*
>* If there is inline data in the inode, the inode will normally not
>* have data blocks allocated (it may have an external xattr block).
...
> diff --git a/fs/stat.c b/fs/stat.c
> index 8c2b30af19f5..062f311b5386 100644
> --- a/fs/stat.c
> +++ b/fs/stat.c
> @@ -29,6 +29,7 @@
> /**
> * generic_fillattr - Fill in the basic attributes from the inode struct
> * @idmap: idmap of the mount the inode was found from
> + * @req_mask statx request_mask
s/req_mask/request_mask
> * @inode: Inode to use as the source
> * @stat:Where to fill in the attributes
> *
> @@ -42,8 +43,8 @@
> * uid and gid filds. On non-idmapped mounts or if permission checking is to
> be
> * performed on the raw inode simply passs @nop_mnt_idmap.
> */
> -void generic_fillattr(struct mnt_idmap *idmap, struct inode *inode,
> - struct kstat *stat)
> +void generic_fillattr(struct mnt_idmap *idmap, u32 request_mask,
> + struct inode *inode, struct kstat *stat)
> {
> vfsuid_t vfsuid = i_uid_into_vfsuid(idmap, inode);
> vfsgid_t vfsgid = i_gid_into_vfsgid(idmap, inode);
> @@ -61,6 +62,12 @@ void generic_fillattr(struct mnt_idmap *idmap, struct
> inode *inode,
> stat->ctime = inode_get_ctime(inode);
> stat->blksize = i_blocksize(inode);
> stat->blocks = inode->i_blocks;
> +
> + if ((request_mask & STATX_CHANGE_COOKIE) && IS_I_VERSION(inode)) {
> + stat->result_mask |= STATX_CHANGE_COOKIE;
> + stat->change_cookie = inode_query_iversion(inode);
> + }
> +
> }
> EXPORT_SYMBOL(generic_fillattr);
>
> @@ -123,17 +130,12 @@ int vfs_getattr_nosec(const struct path *path, struct
> kstat *stat,
> stat->attributes_mask |= (STATX_ATTR_AUTOMOUNT |
> STATX_ATTR_DAX);
>
> - if ((request_mask & STATX_CHANGE_COOKIE) && IS_I_VERSION(inode)) {
> - stat->result_mask |= STATX_CHANGE_COOKIE;
> - stat->change_cookie = inode_query_iversion(inode);
> - }
> -
> idmap = mnt_idmap(path->mnt);
> if (inode->i_op->getattr)
> return inode->i_op->getattr(idmap, path, stat,
> request_mask, query_flags);
>
> - generic_fillattr(idmap, inode, stat);
> + generic_fillattr(idmap, request_mask, inode, stat);
> return 0;
>
Re: [f2fs-dev] [PATCH v6 1/7] fs: pass the request_mask to generic_fillattr
On 7/25/23 22:58, Jeff Layton wrote:
generic_fillattr just fills in the entire stat struct indiscriminately
today, copying data from the inode. There is at least one attribute
(STATX_CHANGE_COOKIE) that can have side effects when it is reported,
and we're looking at adding more with the addition of multigrain
timestamps.
Add a request_mask argument to generic_fillattr and have most callers
just pass in the value that is passed to getattr. Have other callers
(e.g. ksmbd) just pass in STATX_BASIC_STATS. Also move the setting of
STATX_CHANGE_COOKIE into generic_fillattr.
Signed-off-by: Jeff Layton
---
fs/9p/vfs_inode.c | 4 ++--
fs/9p/vfs_inode_dotl.c | 4 ++--
fs/afs/inode.c | 2 +-
fs/btrfs/inode.c| 2 +-
fs/ceph/inode.c | 2 +-
The ceph change looks good to me.
Reviewed-by: Xiubo Li
fs/coda/inode.c | 3 ++-
fs/ecryptfs/inode.c | 5 +++--
fs/erofs/inode.c| 2 +-
fs/exfat/file.c | 2 +-
fs/ext2/inode.c | 2 +-
fs/ext4/inode.c | 2 +-
fs/f2fs/file.c | 2 +-
fs/fat/file.c | 2 +-
fs/fuse/dir.c | 2 +-
fs/gfs2/inode.c | 2 +-
fs/hfsplus/inode.c | 2 +-
fs/kernfs/inode.c | 2 +-
fs/libfs.c | 4 ++--
fs/minix/inode.c| 2 +-
fs/nfs/inode.c | 2 +-
fs/nfs/namespace.c | 3 ++-
fs/ntfs3/file.c | 2 +-
fs/ocfs2/file.c | 2 +-
fs/orangefs/inode.c | 2 +-
fs/proc/base.c | 4 ++--
fs/proc/fd.c| 2 +-
fs/proc/generic.c | 2 +-
fs/proc/proc_net.c | 2 +-
fs/proc/proc_sysctl.c | 2 +-
fs/proc/root.c | 3 ++-
fs/smb/client/inode.c | 2 +-
fs/smb/server/smb2pdu.c | 22 +++---
fs/smb/server/vfs.c | 3 ++-
fs/stat.c | 18 ++
fs/sysv/itree.c | 3 ++-
fs/ubifs/dir.c | 2 +-
fs/udf/symlink.c| 2 +-
fs/vboxsf/utils.c | 2 +-
include/linux/fs.h | 2 +-
mm/shmem.c | 2 +-
40 files changed, 70 insertions(+), 62 deletions(-)
diff --git a/fs/9p/vfs_inode.c b/fs/9p/vfs_inode.c
index 16d85e6033a3..d24d1f20e922 100644
--- a/fs/9p/vfs_inode.c
+++ b/fs/9p/vfs_inode.c
@@ -1016,7 +1016,7 @@ v9fs_vfs_getattr(struct mnt_idmap *idmap, const struct
path *path,
p9_debug(P9_DEBUG_VFS, "dentry: %p\n", dentry);
v9ses = v9fs_dentry2v9ses(dentry);
if (v9ses->cache & (CACHE_META|CACHE_LOOSE)) {
- generic_fillattr(_mnt_idmap, inode, stat);
+ generic_fillattr(_mnt_idmap, request_mask, inode, stat);
return 0;
} else if (v9ses->cache & CACHE_WRITEBACK) {
if (S_ISREG(inode->i_mode)) {
@@ -1037,7 +1037,7 @@ v9fs_vfs_getattr(struct mnt_idmap *idmap, const struct
path *path,
return PTR_ERR(st);
v9fs_stat2inode(st, d_inode(dentry), dentry->d_sb, 0);
- generic_fillattr(_mnt_idmap, d_inode(dentry), stat);
+ generic_fillattr(_mnt_idmap, request_mask, d_inode(dentry), stat);
p9stat_free(st);
kfree(st);
diff --git a/fs/9p/vfs_inode_dotl.c b/fs/9p/vfs_inode_dotl.c
index 464ea73d1bf8..8e8d5d2a13d8 100644
--- a/fs/9p/vfs_inode_dotl.c
+++ b/fs/9p/vfs_inode_dotl.c
@@ -451,7 +451,7 @@ v9fs_vfs_getattr_dotl(struct mnt_idmap *idmap,
p9_debug(P9_DEBUG_VFS, "dentry: %p\n", dentry);
v9ses = v9fs_dentry2v9ses(dentry);
if (v9ses->cache & (CACHE_META|CACHE_LOOSE)) {
- generic_fillattr(_mnt_idmap, inode, stat);
+ generic_fillattr(_mnt_idmap, request_mask, inode, stat);
return 0;
} else if (v9ses->cache) {
if (S_ISREG(inode->i_mode)) {
@@ -476,7 +476,7 @@ v9fs_vfs_getattr_dotl(struct mnt_idmap *idmap,
return PTR_ERR(st);
v9fs_stat2inode_dotl(st, d_inode(dentry), 0);
- generic_fillattr(_mnt_idmap, d_inode(dentry), stat);
+ generic_fillattr(_mnt_idmap, request_mask, d_inode(dentry), stat);
/* Change block size to what the server returned */
stat->blksize = st->st_blksize;
diff --git a/fs/afs/inode.c b/fs/afs/inode.c
index 6b636f43f548..1c794a1896aa 100644
--- a/fs/afs/inode.c
+++ b/fs/afs/inode.c
@@ -773,7 +773,7 @@ int afs_getattr(struct mnt_idmap *idmap, const struct path
*path,
do {
read_seqbegin_or_lock(>cb_lock, );
- generic_fillattr(_mnt_idmap, inode, stat);
+ generic_fillattr(_mnt_idmap, request_mask, inode, stat);
if (test_bit(AFS_VNODE_SILLY_DELETED, >flags) &&
stat->nlink > 0)
stat->nlink -= 1;
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index bcccd551f547..7346059209aa 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -8773,7 +8773,7 @@ static int btrfs_getattr(struct mnt_idmap *idmap,
STATX_ATTR_IMMUTABLE |
[f2fs-dev] [PATCH v6 1/7] fs: pass the request_mask to generic_fillattr
generic_fillattr just fills in the entire stat struct indiscriminately
today, copying data from the inode. There is at least one attribute
(STATX_CHANGE_COOKIE) that can have side effects when it is reported,
and we're looking at adding more with the addition of multigrain
timestamps.
Add a request_mask argument to generic_fillattr and have most callers
just pass in the value that is passed to getattr. Have other callers
(e.g. ksmbd) just pass in STATX_BASIC_STATS. Also move the setting of
STATX_CHANGE_COOKIE into generic_fillattr.
Signed-off-by: Jeff Layton
---
fs/9p/vfs_inode.c | 4 ++--
fs/9p/vfs_inode_dotl.c | 4 ++--
fs/afs/inode.c | 2 +-
fs/btrfs/inode.c| 2 +-
fs/ceph/inode.c | 2 +-
fs/coda/inode.c | 3 ++-
fs/ecryptfs/inode.c | 5 +++--
fs/erofs/inode.c| 2 +-
fs/exfat/file.c | 2 +-
fs/ext2/inode.c | 2 +-
fs/ext4/inode.c | 2 +-
fs/f2fs/file.c | 2 +-
fs/fat/file.c | 2 +-
fs/fuse/dir.c | 2 +-
fs/gfs2/inode.c | 2 +-
fs/hfsplus/inode.c | 2 +-
fs/kernfs/inode.c | 2 +-
fs/libfs.c | 4 ++--
fs/minix/inode.c| 2 +-
fs/nfs/inode.c | 2 +-
fs/nfs/namespace.c | 3 ++-
fs/ntfs3/file.c | 2 +-
fs/ocfs2/file.c | 2 +-
fs/orangefs/inode.c | 2 +-
fs/proc/base.c | 4 ++--
fs/proc/fd.c| 2 +-
fs/proc/generic.c | 2 +-
fs/proc/proc_net.c | 2 +-
fs/proc/proc_sysctl.c | 2 +-
fs/proc/root.c | 3 ++-
fs/smb/client/inode.c | 2 +-
fs/smb/server/smb2pdu.c | 22 +++---
fs/smb/server/vfs.c | 3 ++-
fs/stat.c | 18 ++
fs/sysv/itree.c | 3 ++-
fs/ubifs/dir.c | 2 +-
fs/udf/symlink.c| 2 +-
fs/vboxsf/utils.c | 2 +-
include/linux/fs.h | 2 +-
mm/shmem.c | 2 +-
40 files changed, 70 insertions(+), 62 deletions(-)
diff --git a/fs/9p/vfs_inode.c b/fs/9p/vfs_inode.c
index 16d85e6033a3..d24d1f20e922 100644
--- a/fs/9p/vfs_inode.c
+++ b/fs/9p/vfs_inode.c
@@ -1016,7 +1016,7 @@ v9fs_vfs_getattr(struct mnt_idmap *idmap, const struct
path *path,
p9_debug(P9_DEBUG_VFS, "dentry: %p\n", dentry);
v9ses = v9fs_dentry2v9ses(dentry);
if (v9ses->cache & (CACHE_META|CACHE_LOOSE)) {
- generic_fillattr(_mnt_idmap, inode, stat);
+ generic_fillattr(_mnt_idmap, request_mask, inode, stat);
return 0;
} else if (v9ses->cache & CACHE_WRITEBACK) {
if (S_ISREG(inode->i_mode)) {
@@ -1037,7 +1037,7 @@ v9fs_vfs_getattr(struct mnt_idmap *idmap, const struct
path *path,
return PTR_ERR(st);
v9fs_stat2inode(st, d_inode(dentry), dentry->d_sb, 0);
- generic_fillattr(_mnt_idmap, d_inode(dentry), stat);
+ generic_fillattr(_mnt_idmap, request_mask, d_inode(dentry), stat);
p9stat_free(st);
kfree(st);
diff --git a/fs/9p/vfs_inode_dotl.c b/fs/9p/vfs_inode_dotl.c
index 464ea73d1bf8..8e8d5d2a13d8 100644
--- a/fs/9p/vfs_inode_dotl.c
+++ b/fs/9p/vfs_inode_dotl.c
@@ -451,7 +451,7 @@ v9fs_vfs_getattr_dotl(struct mnt_idmap *idmap,
p9_debug(P9_DEBUG_VFS, "dentry: %p\n", dentry);
v9ses = v9fs_dentry2v9ses(dentry);
if (v9ses->cache & (CACHE_META|CACHE_LOOSE)) {
- generic_fillattr(_mnt_idmap, inode, stat);
+ generic_fillattr(_mnt_idmap, request_mask, inode, stat);
return 0;
} else if (v9ses->cache) {
if (S_ISREG(inode->i_mode)) {
@@ -476,7 +476,7 @@ v9fs_vfs_getattr_dotl(struct mnt_idmap *idmap,
return PTR_ERR(st);
v9fs_stat2inode_dotl(st, d_inode(dentry), 0);
- generic_fillattr(_mnt_idmap, d_inode(dentry), stat);
+ generic_fillattr(_mnt_idmap, request_mask, d_inode(dentry), stat);
/* Change block size to what the server returned */
stat->blksize = st->st_blksize;
diff --git a/fs/afs/inode.c b/fs/afs/inode.c
index 6b636f43f548..1c794a1896aa 100644
--- a/fs/afs/inode.c
+++ b/fs/afs/inode.c
@@ -773,7 +773,7 @@ int afs_getattr(struct mnt_idmap *idmap, const struct path
*path,
do {
read_seqbegin_or_lock(>cb_lock, );
- generic_fillattr(_mnt_idmap, inode, stat);
+ generic_fillattr(_mnt_idmap, request_mask, inode, stat);
if (test_bit(AFS_VNODE_SILLY_DELETED, >flags) &&
stat->nlink > 0)
stat->nlink -= 1;
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index bcccd551f547..7346059209aa 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -8773,7 +8773,7 @@ static int btrfs_getattr(struct mnt_idmap *idmap,
STATX_ATTR_IMMUTABLE |
STATX_ATTR_NODUMP);
- generic_fillattr(idmap, inode, stat);
+ generic_fillattr(idmap, request_mask,
