diff mbox series

[4/5] fuse: implement statx

Message ID 20230810105501.1418427-5-mszeredi@redhat.com (mailing list archive)
State New, archived
Headers show
Series fuse: support birth time | expand

Commit Message

Miklos Szeredi Aug. 10, 2023, 10:55 a.m. UTC
Allow querying btime.  When btime is requested in mask, then FUSE_STATX
request is sent.  Otherwise keep using FUSE_GETATTR.

The userspace interface for statx matches that of the statx(2) API.
However there are limitations on how this interface is used:

 - returned basic stats and btime are used, stx_attributes, etc. are
   ignored

 - always query basic stats and btime, regardless of what was requested

 - requested sync type is ignored, the default is passed to the server

 - if server returns with some attributes missing from the result_mask,
   then no attributes will be cached

 - btime is not cached yet (next patch will fix that)

For new inodes initialize fi->inval_mask to "all invalid", instead of "all
valid" as previously.  Also only clear basic stats from inval_mask when
caching attributes.  This will result in the caching logic not thinking
that btime is cached.

Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
---
 fs/fuse/dir.c    | 106 ++++++++++++++++++++++++++++++++++++++++++++---
 fs/fuse/fuse_i.h |   3 ++
 fs/fuse/inode.c  |   5 ++-
 3 files changed, 107 insertions(+), 7 deletions(-)

Comments

kernel test robot Aug. 10, 2023, 1:34 p.m. UTC | #1
Hi Miklos,

kernel test robot noticed the following build warnings:

[auto build test WARNING on mszeredi-fuse/for-next]
[also build test WARNING on linus/master v6.5-rc5 next-20230809]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Miklos-Szeredi/fuse-handle-empty-request_mask-in-statx/20230810-185708
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/fuse.git for-next
patch link:    https://lore.kernel.org/r/20230810105501.1418427-5-mszeredi%40redhat.com
patch subject: [PATCH 4/5] fuse: implement statx
config: alpha-allyesconfig (https://download.01.org/0day-ci/archive/20230810/202308102130.EEqF5GG3-lkp@intel.com/config)
compiler: alpha-linux-gcc (GCC) 12.3.0
reproduce: (https://download.01.org/0day-ci/archive/20230810/202308102130.EEqF5GG3-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202308102130.EEqF5GG3-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> fs/fuse/dir.c:353:6: warning: no previous prototype for 'fuse_valid_size' [-Wmissing-prototypes]
     353 | bool fuse_valid_size(u64 size)
         |      ^~~~~~~~~~~~~~~


vim +/fuse_valid_size +353 fs/fuse/dir.c

   352	
 > 353	bool fuse_valid_size(u64 size)
   354	{
   355		return size <= LLONG_MAX;
   356	}
   357
Miklos Szeredi Aug. 10, 2023, 2:19 p.m. UTC | #2
On Thu, Aug 10, 2023 at 09:34:59PM +0800, kernel test robot wrote:
> Hi Miklos,
> 
> kernel test robot noticed the following build warnings:
> 
> [auto build test WARNING on mszeredi-fuse/for-next]


Thanks.

Updated patch:

---
From: Miklos Szeredi <mszeredi@redhat.com>
Subject: fuse: implement statx

Allow querying btime.  When btime is requested in mask, then FUSE_STATX
request is sent.  Otherwise keep using FUSE_GETATTR.

The userspace interface for statx matches that of the statx(2) API.
However there are limitations on how this interface is used:

 - returned basic stats and btime are used, stx_attributes, etc. are
   ignored

 - always query basic stats and btime, regardless of what was requested

 - requested sync type is ignored, the default is passed to the server

 - if server returns with some attributes missing from the result_mask,
   then no attributes will be cached

 - btime is not cached yet (next patch will fix that)

For new inodes initialize fi->inval_mask to "all invalid", instead of "all
valid" as previously.  Also only clear basic stats from inval_mask when
caching attributes.  This will result in the caching logic not thinking
that btime is cached.

Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
---
 fs/fuse/dir.c    |  106 ++++++++++++++++++++++++++++++++++++++++++++++++++++---
 fs/fuse/fuse_i.h |    3 +
 fs/fuse/inode.c  |    5 +-
 3 files changed, 107 insertions(+), 7 deletions(-)

--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -350,10 +350,14 @@ int fuse_valid_type(int m)
 		S_ISBLK(m) || S_ISFIFO(m) || S_ISSOCK(m);
 }
 
+static bool fuse_valid_size(u64 size)
+{
+	return size <= LLONG_MAX;
+}
+
 bool fuse_invalid_attr(struct fuse_attr *attr)
 {
-	return !fuse_valid_type(attr->mode) ||
-		attr->size > LLONG_MAX;
+	return !fuse_valid_type(attr->mode) || !fuse_valid_size(attr->size);
 }
 
 int fuse_lookup_name(struct super_block *sb, u64 nodeid, const struct qstr *name,
@@ -1143,6 +1147,84 @@ static void fuse_fillattr(struct inode *
 	stat->blksize = 1 << blkbits;
 }
 
+static void fuse_statx_to_attr(struct fuse_statx *sx, struct fuse_attr *attr)
+{
+	memset(attr, 0, sizeof(*attr));
+	attr->ino = sx->ino;
+	attr->size = sx->size;
+	attr->blocks = sx->blocks;
+	attr->atime = sx->atime.tv_sec;
+	attr->mtime = sx->mtime.tv_sec;
+	attr->ctime = sx->ctime.tv_sec;
+	attr->atimensec = sx->atime.tv_nsec;
+	attr->mtimensec = sx->mtime.tv_nsec;
+	attr->ctimensec = sx->ctime.tv_nsec;
+	attr->mode = sx->mode;
+	attr->nlink = sx->nlink;
+	attr->uid = sx->uid;
+	attr->gid = sx->gid;
+	attr->rdev = new_encode_dev(MKDEV(sx->rdev_major, sx->rdev_minor));
+	attr->blksize = sx->blksize;
+}
+
+static int fuse_do_statx(struct inode *inode, struct file *file,
+			 struct kstat *stat)
+{
+	int err;
+	struct fuse_attr attr;
+	struct fuse_statx *sx;
+	struct fuse_statx_in inarg;
+	struct fuse_statx_out outarg;
+	struct fuse_mount *fm = get_fuse_mount(inode);
+	u64 attr_version = fuse_get_attr_version(fm->fc);
+	FUSE_ARGS(args);
+
+	memset(&inarg, 0, sizeof(inarg));
+	memset(&outarg, 0, sizeof(outarg));
+	/* Directories have separate file-handle space */
+	if (file && S_ISREG(inode->i_mode)) {
+		struct fuse_file *ff = file->private_data;
+
+		inarg.getattr_flags |= FUSE_GETATTR_FH;
+		inarg.fh = ff->fh;
+	}
+	/* For now leave sync hints as the default, request all stats. */
+	inarg.sx_flags = 0;
+	inarg.sx_mask = STATX_BASIC_STATS | STATX_BTIME;
+	args.opcode = FUSE_STATX;
+	args.nodeid = get_node_id(inode);
+	args.in_numargs = 1;
+	args.in_args[0].size = sizeof(inarg);
+	args.in_args[0].value = &inarg;
+	args.out_numargs = 1;
+	args.out_args[0].size = sizeof(outarg);
+	args.out_args[0].value = &outarg;
+	err = fuse_simple_request(fm, &args);
+	if (err)
+		return err;
+
+	sx = &outarg.stat;
+	if (((sx->mask & STATX_SIZE) && !fuse_valid_size(sx->size)) ||
+	    ((sx->mask & STATX_TYPE) && (!fuse_valid_type(sx->mode) ||
+					 inode_wrong_type(inode, sx->mode)))) {
+		make_bad_inode(inode);
+		return -EIO;
+	}
+
+	fuse_statx_to_attr(&outarg.stat, &attr);
+	if ((sx->mask & STATX_BASIC_STATS) == STATX_BASIC_STATS) {
+		fuse_change_attributes(inode, &attr, ATTR_TIMEOUT(&outarg),
+				       attr_version);
+	}
+	stat->result_mask = sx->mask & (STATX_BASIC_STATS | STATX_BTIME);
+	stat->btime.tv_sec = sx->btime.tv_sec;
+	stat->btime.tv_nsec = min_t(u32, sx->btime.tv_nsec, NSEC_PER_SEC - 1);
+	fuse_fillattr(inode, &attr, stat);
+	stat->result_mask |= STATX_TYPE;
+
+	return 0;
+}
+
 static int fuse_do_getattr(struct inode *inode, struct kstat *stat,
 			   struct file *file)
 {
@@ -1194,13 +1276,18 @@ static int fuse_update_get_attr(struct i
 				unsigned int flags)
 {
 	struct fuse_inode *fi = get_fuse_inode(inode);
+	struct fuse_conn *fc = get_fuse_conn(inode);
 	int err = 0;
 	bool sync;
 	u32 inval_mask = READ_ONCE(fi->inval_mask);
 	u32 cache_mask = fuse_get_cache_mask(inode);
 
-	/* FUSE only supports basic stats */
-	request_mask &= STATX_BASIC_STATS;
+
+	/* FUSE only supports basic stats and possibly btime */
+	request_mask &= STATX_BASIC_STATS | STATX_BTIME;
+retry:
+	if (fc->no_statx)
+		request_mask &= STATX_BASIC_STATS;
 
 	if (!request_mask)
 		sync = false;
@@ -1215,7 +1302,16 @@ static int fuse_update_get_attr(struct i
 
 	if (sync) {
 		forget_all_cached_acls(inode);
-		err = fuse_do_getattr(inode, stat, file);
+		/* Try statx if BTIME is requested */
+		if (!fc->no_statx && (request_mask & ~STATX_BASIC_STATS)) {
+			err = fuse_do_statx(inode, file, stat);
+			if (err == -ENOSYS) {
+				fc->no_statx = 1;
+				goto retry;
+			}
+		} else {
+			err = fuse_do_getattr(inode, stat, file);
+		}
 	} else if (stat) {
 		generic_fillattr(&nop_mnt_idmap, inode, stat);
 		stat->mode = fi->orig_i_mode;
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -792,6 +792,9 @@ struct fuse_conn {
 	/* Is tmpfile not implemented by fs? */
 	unsigned int no_tmpfile:1;
 
+	/* Is statx not implemented by fs? */
+	unsigned int no_statx:1;
+
 	/** The number of requests waiting for completion */
 	atomic_t num_waiting;
 
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -77,7 +77,7 @@ static struct inode *fuse_alloc_inode(st
 		return NULL;
 
 	fi->i_time = 0;
-	fi->inval_mask = 0;
+	fi->inval_mask = ~0;
 	fi->nodeid = 0;
 	fi->nlookup = 0;
 	fi->attr_version = 0;
@@ -172,7 +172,8 @@ void fuse_change_attributes_common(struc
 
 	fi->attr_version = atomic64_inc_return(&fc->attr_version);
 	fi->i_time = attr_valid;
-	WRITE_ONCE(fi->inval_mask, 0);
+	/* Clear basic stats from invalid mask */
+	set_mask_bits(&fi->inval_mask, STATX_BASIC_STATS, 0);
 
 	inode->i_ino     = fuse_squash_ino(attr->ino);
 	inode->i_mode    = (inode->i_mode & S_IFMT) | (attr->mode & 07777);
Bernd Schubert Aug. 22, 2023, 3:20 p.m. UTC | #3
Hi Miklos,

sorry for late review.

On 8/10/23 12:55, Miklos Szeredi wrote:
[...]
> +static int fuse_do_statx(struct inode *inode, struct file *file,
> +			 struct kstat *stat)
> +{
> +	int err;
> +	struct fuse_attr attr;
> +	struct fuse_statx *sx;
> +	struct fuse_statx_in inarg;
> +	struct fuse_statx_out outarg;
> +	struct fuse_mount *fm = get_fuse_mount(inode);
> +	u64 attr_version = fuse_get_attr_version(fm->fc);
> +	FUSE_ARGS(args);
> +
> +	memset(&inarg, 0, sizeof(inarg));
> +	memset(&outarg, 0, sizeof(outarg));
> +	/* Directories have separate file-handle space */
> +	if (file && S_ISREG(inode->i_mode)) {
> +		struct fuse_file *ff = file->private_data;
> +
> +		inarg.getattr_flags |= FUSE_GETATTR_FH;
> +		inarg.fh = ff->fh;
> +	}
> +	/* For now leave sync hints as the default, request all stats. */
> +	inarg.sx_flags = 0;
> +	inarg.sx_mask = STATX_BASIC_STATS | STATX_BTIME;



What is actually the reason not to pass through flags from 
fuse_update_get_attr()? Wouldn't it make sense to request the minimal 
required mask and then server side can decide if it wants to fill in more?


Thanks,
Bernd
Miklos Szeredi Aug. 22, 2023, 3:33 p.m. UTC | #4
On Tue, 22 Aug 2023 at 17:20, Bernd Schubert <bernd.schubert@fastmail.fm> wrote:
>
> Hi Miklos,
>
> sorry for late review.
>
> On 8/10/23 12:55, Miklos Szeredi wrote:
> [...]
> > +static int fuse_do_statx(struct inode *inode, struct file *file,
> > +                      struct kstat *stat)
> > +{
> > +     int err;
> > +     struct fuse_attr attr;
> > +     struct fuse_statx *sx;
> > +     struct fuse_statx_in inarg;
> > +     struct fuse_statx_out outarg;
> > +     struct fuse_mount *fm = get_fuse_mount(inode);
> > +     u64 attr_version = fuse_get_attr_version(fm->fc);
> > +     FUSE_ARGS(args);
> > +
> > +     memset(&inarg, 0, sizeof(inarg));
> > +     memset(&outarg, 0, sizeof(outarg));
> > +     /* Directories have separate file-handle space */
> > +     if (file && S_ISREG(inode->i_mode)) {
> > +             struct fuse_file *ff = file->private_data;
> > +
> > +             inarg.getattr_flags |= FUSE_GETATTR_FH;
> > +             inarg.fh = ff->fh;
> > +     }
> > +     /* For now leave sync hints as the default, request all stats. */
> > +     inarg.sx_flags = 0;
> > +     inarg.sx_mask = STATX_BASIC_STATS | STATX_BTIME;
>
>
>
> What is actually the reason not to pass through flags from
> fuse_update_get_attr()? Wouldn't it make sense to request the minimal
> required mask and then server side can decide if it wants to fill in more?

This and following commit is about btime and btime only.  It's about
adding just this single attribute, otherwise the logic is unchanged.

But the flexibility is there in the interface definition, and
functionality can be added later.

Thanks,
Miklos
Bernd Schubert Aug. 22, 2023, 4:39 p.m. UTC | #5
On 8/10/23 12:55, Miklos Szeredi wrote:
> Allow querying btime.  When btime is requested in mask, then FUSE_STATX
> request is sent.  Otherwise keep using FUSE_GETATTR.
> 
> The userspace interface for statx matches that of the statx(2) API.
> However there are limitations on how this interface is used:
> 
>   - returned basic stats and btime are used, stx_attributes, etc. are
>     ignored
> 
>   - always query basic stats and btime, regardless of what was requested
> 
>   - requested sync type is ignored, the default is passed to the server
> 
>   - if server returns with some attributes missing from the result_mask,
>     then no attributes will be cached
> 
>   - btime is not cached yet (next patch will fix that)
> 
> For new inodes initialize fi->inval_mask to "all invalid", instead of "all
> valid" as previously.  Also only clear basic stats from inval_mask when
> caching attributes.  This will result in the caching logic not thinking
> that btime is cached.
> 
> Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
> ---
>   fs/fuse/dir.c    | 106 ++++++++++++++++++++++++++++++++++++++++++++---
>   fs/fuse/fuse_i.h |   3 ++
>   fs/fuse/inode.c  |   5 ++-
>   3 files changed, 107 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
> index 04006db6e173..552157bd6a4d 100644
> --- a/fs/fuse/dir.c
> +++ b/fs/fuse/dir.c
> @@ -350,10 +350,14 @@ int fuse_valid_type(int m)
>   		S_ISBLK(m) || S_ISFIFO(m) || S_ISSOCK(m);
>   }
>   
> +bool fuse_valid_size(u64 size)
> +{
> +	return size <= LLONG_MAX;
> +}
> +
>   bool fuse_invalid_attr(struct fuse_attr *attr)
>   {
> -	return !fuse_valid_type(attr->mode) ||
> -		attr->size > LLONG_MAX;
> +	return !fuse_valid_type(attr->mode) || !fuse_valid_size(attr->size);
>   }
>   
>   int fuse_lookup_name(struct super_block *sb, u64 nodeid, const struct qstr *name,
> @@ -1143,6 +1147,84 @@ static void fuse_fillattr(struct inode *inode, struct fuse_attr *attr,
>   	stat->blksize = 1 << blkbits;
>   }
>   
> +static void fuse_statx_to_attr(struct fuse_statx *sx, struct fuse_attr *attr)
> +{
> +	memset(attr, 0, sizeof(*attr));
> +	attr->ino = sx->ino;
> +	attr->size = sx->size;
> +	attr->blocks = sx->blocks;
> +	attr->atime = sx->atime.tv_sec;
> +	attr->mtime = sx->mtime.tv_sec;
> +	attr->ctime = sx->ctime.tv_sec;
> +	attr->atimensec = sx->atime.tv_nsec;
> +	attr->mtimensec = sx->mtime.tv_nsec;
> +	attr->ctimensec = sx->ctime.tv_nsec;
> +	attr->mode = sx->mode;
> +	attr->nlink = sx->nlink;
> +	attr->uid = sx->uid;
> +	attr->gid = sx->gid;
> +	attr->rdev = new_encode_dev(MKDEV(sx->rdev_major, sx->rdev_minor));
> +	attr->blksize = sx->blksize;
> +}
> +
> +static int fuse_do_statx(struct inode *inode, struct file *file,
> +			 struct kstat *stat)
> +{
> +	int err;
> +	struct fuse_attr attr;
> +	struct fuse_statx *sx;
> +	struct fuse_statx_in inarg;
> +	struct fuse_statx_out outarg;
> +	struct fuse_mount *fm = get_fuse_mount(inode);
> +	u64 attr_version = fuse_get_attr_version(fm->fc);
> +	FUSE_ARGS(args);
> +
> +	memset(&inarg, 0, sizeof(inarg));
> +	memset(&outarg, 0, sizeof(outarg));
> +	/* Directories have separate file-handle space */
> +	if (file && S_ISREG(inode->i_mode)) {
> +		struct fuse_file *ff = file->private_data;
> +
> +		inarg.getattr_flags |= FUSE_GETATTR_FH;
> +		inarg.fh = ff->fh;
> +	}
> +	/* For now leave sync hints as the default, request all stats. */
> +	inarg.sx_flags = 0;
> +	inarg.sx_mask = STATX_BASIC_STATS | STATX_BTIME;
> +	args.opcode = FUSE_STATX;
> +	args.nodeid = get_node_id(inode);
> +	args.in_numargs = 1;
> +	args.in_args[0].size = sizeof(inarg);
> +	args.in_args[0].value = &inarg;
> +	args.out_numargs = 1;
> +	args.out_args[0].size = sizeof(outarg);
> +	args.out_args[0].value = &outarg;
> +	err = fuse_simple_request(fm, &args);
> +	if (err)
> +		return err;
> +
> +	sx = &outarg.stat;
> +	if (((sx->mask & STATX_SIZE) && !fuse_valid_size(sx->size)) ||
> +	    ((sx->mask & STATX_TYPE) && (!fuse_valid_type(sx->mode) ||
> +					 inode_wrong_type(inode, sx->mode)))) {
> +		make_bad_inode(inode);
> +		return -EIO;
> +	}
> +
> +	fuse_statx_to_attr(&outarg.stat, &attr);
> +	if ((sx->mask & STATX_BASIC_STATS) == STATX_BASIC_STATS) {
> +		fuse_change_attributes(inode, &attr, ATTR_TIMEOUT(&outarg),
> +				       attr_version);
> +	}
> +	stat->result_mask = sx->mask & (STATX_BASIC_STATS | STATX_BTIME);
> +	stat->btime.tv_sec = sx->btime.tv_sec;
> +	stat->btime.tv_nsec = min_t(u32, sx->btime.tv_nsec, NSEC_PER_SEC - 1);
> +	fuse_fillattr(inode, &attr, stat);
> +	stat->result_mask |= STATX_TYPE;
> +
> +	return 0;
> +}


Hmm, unconditionally using stat is potentially a NULL ptr with future 
updates. I think not right now, as fuse_update_get_attr() has the 
(request_mask & ~STATX_BASIC_STATS) condition and no caller
that passes 'stat = NULL' requests anything beyond STATX_BASIC_STATS, 
but wouldn't it be more safe to access stat only conditionally?


Thanks,
Bernd
Bernd Schubert Aug. 22, 2023, 4:55 p.m. UTC | #6
On 8/22/23 17:33, Miklos Szeredi wrote:
> On Tue, 22 Aug 2023 at 17:20, Bernd Schubert <bernd.schubert@fastmail.fm> wrote:
>>
>> Hi Miklos,
>>
>> sorry for late review.
>>
>> On 8/10/23 12:55, Miklos Szeredi wrote:
>> [...]
>>> +static int fuse_do_statx(struct inode *inode, struct file *file,
>>> +                      struct kstat *stat)
>>> +{
>>> +     int err;
>>> +     struct fuse_attr attr;
>>> +     struct fuse_statx *sx;
>>> +     struct fuse_statx_in inarg;
>>> +     struct fuse_statx_out outarg;
>>> +     struct fuse_mount *fm = get_fuse_mount(inode);
>>> +     u64 attr_version = fuse_get_attr_version(fm->fc);
>>> +     FUSE_ARGS(args);
>>> +
>>> +     memset(&inarg, 0, sizeof(inarg));
>>> +     memset(&outarg, 0, sizeof(outarg));
>>> +     /* Directories have separate file-handle space */
>>> +     if (file && S_ISREG(inode->i_mode)) {
>>> +             struct fuse_file *ff = file->private_data;
>>> +
>>> +             inarg.getattr_flags |= FUSE_GETATTR_FH;
>>> +             inarg.fh = ff->fh;
>>> +     }
>>> +     /* For now leave sync hints as the default, request all stats. */
>>> +     inarg.sx_flags = 0;
>>> +     inarg.sx_mask = STATX_BASIC_STATS | STATX_BTIME;
>>
>>
>>
>> What is actually the reason not to pass through flags from
>> fuse_update_get_attr()? Wouldn't it make sense to request the minimal
>> required mask and then server side can decide if it wants to fill in more?
> 
> This and following commit is about btime and btime only.  It's about
> adding just this single attribute, otherwise the logic is unchanged.
> 
> But the flexibility is there in the interface definition, and
> functionality can be added later.

Sure, though what speaks against setting (limiting the mask) right away?

diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index 42f49fe6e770..de1d991757a5 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -1168,7 +1168,8 @@ static void fuse_statx_to_attr(struct fuse_statx *sx, struct fuse_attr *attr)
  }
  
  static int fuse_do_statx(struct inode *inode, struct file *file,
-                        struct kstat *stat)
+                        struct kstat *stat, u32 request_mask,
+                        unsigned int flags)
  {
         int err;
         struct fuse_attr attr;
@@ -1188,9 +1189,10 @@ static int fuse_do_statx(struct inode *inode, struct file *file,
                 inarg.getattr_flags |= FUSE_GETATTR_FH;
                 inarg.fh = ff->fh;
         }
-       /* For now leave sync hints as the default, request all stats. */
-       inarg.sx_flags = 0;
-       inarg.sx_mask = STATX_BASIC_STATS | STATX_BTIME;
+
+       /* request the given mask, server side is free to return more */
+       inarg.sx_flags = flags;
+       inarg.sx_mask = request_mask;
         args.opcode = FUSE_STATX;
         args.nodeid = get_node_id(inode);
         args.in_numargs = 1;
@@ -1304,7 +1306,8 @@ static int fuse_update_get_attr(struct inode *inode, struct file *file,
                 forget_all_cached_acls(inode);
                 /* Try statx if BTIME is requested */
                 if (!fc->no_statx && (request_mask & ~STATX_BASIC_STATS)) {
-                       err = fuse_do_statx(inode, file, stat);
+                       err = fuse_do_statx(inode, file, stat, request_mask,
+                                           flags);
                         if (err == -ENOSYS) {
                                 fc->no_statx = 1;
                                 goto retry;



Thanks,
Bernd
Miklos Szeredi Aug. 23, 2023, 6:15 a.m. UTC | #7
On Tue, 22 Aug 2023 at 18:39, Bernd Schubert <bernd.schubert@fastmail.fm> wrote:
>
>
>
> On 8/10/23 12:55, Miklos Szeredi wrote:
> > Allow querying btime.  When btime is requested in mask, then FUSE_STATX
> > request is sent.  Otherwise keep using FUSE_GETATTR.
> >
> > The userspace interface for statx matches that of the statx(2) API.
> > However there are limitations on how this interface is used:
> >
> >   - returned basic stats and btime are used, stx_attributes, etc. are
> >     ignored
> >
> >   - always query basic stats and btime, regardless of what was requested
> >
> >   - requested sync type is ignored, the default is passed to the server
> >
> >   - if server returns with some attributes missing from the result_mask,
> >     then no attributes will be cached
> >
> >   - btime is not cached yet (next patch will fix that)
> >
> > For new inodes initialize fi->inval_mask to "all invalid", instead of "all
> > valid" as previously.  Also only clear basic stats from inval_mask when
> > caching attributes.  This will result in the caching logic not thinking
> > that btime is cached.
> >
> > Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
> > ---
> >   fs/fuse/dir.c    | 106 ++++++++++++++++++++++++++++++++++++++++++++---
> >   fs/fuse/fuse_i.h |   3 ++
> >   fs/fuse/inode.c  |   5 ++-
> >   3 files changed, 107 insertions(+), 7 deletions(-)
> >
> > diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
> > index 04006db6e173..552157bd6a4d 100644
> > --- a/fs/fuse/dir.c
> > +++ b/fs/fuse/dir.c
> > @@ -350,10 +350,14 @@ int fuse_valid_type(int m)
> >               S_ISBLK(m) || S_ISFIFO(m) || S_ISSOCK(m);
> >   }
> >
> > +bool fuse_valid_size(u64 size)
> > +{
> > +     return size <= LLONG_MAX;
> > +}
> > +
> >   bool fuse_invalid_attr(struct fuse_attr *attr)
> >   {
> > -     return !fuse_valid_type(attr->mode) ||
> > -             attr->size > LLONG_MAX;
> > +     return !fuse_valid_type(attr->mode) || !fuse_valid_size(attr->size);
> >   }
> >
> >   int fuse_lookup_name(struct super_block *sb, u64 nodeid, const struct qstr *name,
> > @@ -1143,6 +1147,84 @@ static void fuse_fillattr(struct inode *inode, struct fuse_attr *attr,
> >       stat->blksize = 1 << blkbits;
> >   }
> >
> > +static void fuse_statx_to_attr(struct fuse_statx *sx, struct fuse_attr *attr)
> > +{
> > +     memset(attr, 0, sizeof(*attr));
> > +     attr->ino = sx->ino;
> > +     attr->size = sx->size;
> > +     attr->blocks = sx->blocks;
> > +     attr->atime = sx->atime.tv_sec;
> > +     attr->mtime = sx->mtime.tv_sec;
> > +     attr->ctime = sx->ctime.tv_sec;
> > +     attr->atimensec = sx->atime.tv_nsec;
> > +     attr->mtimensec = sx->mtime.tv_nsec;
> > +     attr->ctimensec = sx->ctime.tv_nsec;
> > +     attr->mode = sx->mode;
> > +     attr->nlink = sx->nlink;
> > +     attr->uid = sx->uid;
> > +     attr->gid = sx->gid;
> > +     attr->rdev = new_encode_dev(MKDEV(sx->rdev_major, sx->rdev_minor));
> > +     attr->blksize = sx->blksize;
> > +}
> > +
> > +static int fuse_do_statx(struct inode *inode, struct file *file,
> > +                      struct kstat *stat)
> > +{
> > +     int err;
> > +     struct fuse_attr attr;
> > +     struct fuse_statx *sx;
> > +     struct fuse_statx_in inarg;
> > +     struct fuse_statx_out outarg;
> > +     struct fuse_mount *fm = get_fuse_mount(inode);
> > +     u64 attr_version = fuse_get_attr_version(fm->fc);
> > +     FUSE_ARGS(args);
> > +
> > +     memset(&inarg, 0, sizeof(inarg));
> > +     memset(&outarg, 0, sizeof(outarg));
> > +     /* Directories have separate file-handle space */
> > +     if (file && S_ISREG(inode->i_mode)) {
> > +             struct fuse_file *ff = file->private_data;
> > +
> > +             inarg.getattr_flags |= FUSE_GETATTR_FH;
> > +             inarg.fh = ff->fh;
> > +     }
> > +     /* For now leave sync hints as the default, request all stats. */
> > +     inarg.sx_flags = 0;
> > +     inarg.sx_mask = STATX_BASIC_STATS | STATX_BTIME;
> > +     args.opcode = FUSE_STATX;
> > +     args.nodeid = get_node_id(inode);
> > +     args.in_numargs = 1;
> > +     args.in_args[0].size = sizeof(inarg);
> > +     args.in_args[0].value = &inarg;
> > +     args.out_numargs = 1;
> > +     args.out_args[0].size = sizeof(outarg);
> > +     args.out_args[0].value = &outarg;
> > +     err = fuse_simple_request(fm, &args);
> > +     if (err)
> > +             return err;
> > +
> > +     sx = &outarg.stat;
> > +     if (((sx->mask & STATX_SIZE) && !fuse_valid_size(sx->size)) ||
> > +         ((sx->mask & STATX_TYPE) && (!fuse_valid_type(sx->mode) ||
> > +                                      inode_wrong_type(inode, sx->mode)))) {
> > +             make_bad_inode(inode);
> > +             return -EIO;
> > +     }
> > +
> > +     fuse_statx_to_attr(&outarg.stat, &attr);
> > +     if ((sx->mask & STATX_BASIC_STATS) == STATX_BASIC_STATS) {
> > +             fuse_change_attributes(inode, &attr, ATTR_TIMEOUT(&outarg),
> > +                                    attr_version);
> > +     }
> > +     stat->result_mask = sx->mask & (STATX_BASIC_STATS | STATX_BTIME);
> > +     stat->btime.tv_sec = sx->btime.tv_sec;
> > +     stat->btime.tv_nsec = min_t(u32, sx->btime.tv_nsec, NSEC_PER_SEC - 1);
> > +     fuse_fillattr(inode, &attr, stat);
> > +     stat->result_mask |= STATX_TYPE;
> > +
> > +     return 0;
> > +}
>
>
> Hmm, unconditionally using stat is potentially a NULL ptr with future
> updates. I think not right now, as fuse_update_get_attr() has the
> (request_mask & ~STATX_BASIC_STATS) condition and no caller
> that passes 'stat = NULL' requests anything beyond STATX_BASIC_STATS,
> but wouldn't it be more safe to access stat only conditionally?

Yes, makes sense.

Thanks,
Miklos
Miklos Szeredi Aug. 23, 2023, 6:18 a.m. UTC | #8
On Tue, 22 Aug 2023 at 18:55, Bernd Schubert <bernd.schubert@fastmail.fm> wrote:
>
>
>
> On 8/22/23 17:33, Miklos Szeredi wrote:
> > On Tue, 22 Aug 2023 at 17:20, Bernd Schubert <bernd.schubert@fastmail.fm> wrote:
> >>
> >> Hi Miklos,
> >>
> >> sorry for late review.
> >>
> >> On 8/10/23 12:55, Miklos Szeredi wrote:
> >> [...]
> >>> +static int fuse_do_statx(struct inode *inode, struct file *file,
> >>> +                      struct kstat *stat)
> >>> +{
> >>> +     int err;
> >>> +     struct fuse_attr attr;
> >>> +     struct fuse_statx *sx;
> >>> +     struct fuse_statx_in inarg;
> >>> +     struct fuse_statx_out outarg;
> >>> +     struct fuse_mount *fm = get_fuse_mount(inode);
> >>> +     u64 attr_version = fuse_get_attr_version(fm->fc);
> >>> +     FUSE_ARGS(args);
> >>> +
> >>> +     memset(&inarg, 0, sizeof(inarg));
> >>> +     memset(&outarg, 0, sizeof(outarg));
> >>> +     /* Directories have separate file-handle space */
> >>> +     if (file && S_ISREG(inode->i_mode)) {
> >>> +             struct fuse_file *ff = file->private_data;
> >>> +
> >>> +             inarg.getattr_flags |= FUSE_GETATTR_FH;
> >>> +             inarg.fh = ff->fh;
> >>> +     }
> >>> +     /* For now leave sync hints as the default, request all stats. */
> >>> +     inarg.sx_flags = 0;
> >>> +     inarg.sx_mask = STATX_BASIC_STATS | STATX_BTIME;
> >>
> >>
> >>
> >> What is actually the reason not to pass through flags from
> >> fuse_update_get_attr()? Wouldn't it make sense to request the minimal
> >> required mask and then server side can decide if it wants to fill in more?
> >
> > This and following commit is about btime and btime only.  It's about
> > adding just this single attribute, otherwise the logic is unchanged.
> >
> > But the flexibility is there in the interface definition, and
> > functionality can be added later.
>
> Sure, though what speaks against setting (limiting the mask) right away?

But then the result is basically uncacheable, until we have separate
validity timeouts for each attribute.  Maybe we need that, maybe not,
but it does definitely have side effects.

Thanks,
Miklos
Bernd Schubert Aug. 23, 2023, 2:51 p.m. UTC | #9
On 8/23/23 08:18, Miklos Szeredi wrote:
> On Tue, 22 Aug 2023 at 18:55, Bernd Schubert <bernd.schubert@fastmail.fm> wrote:
>>
>>
>>
>> On 8/22/23 17:33, Miklos Szeredi wrote:
>>> On Tue, 22 Aug 2023 at 17:20, Bernd Schubert <bernd.schubert@fastmail.fm> wrote:
>>>>
>>>> Hi Miklos,
>>>>
>>>> sorry for late review.
>>>>
>>>> On 8/10/23 12:55, Miklos Szeredi wrote:
>>>> [...]
>>>>> +static int fuse_do_statx(struct inode *inode, struct file *file,
>>>>> +                      struct kstat *stat)
>>>>> +{
>>>>> +     int err;
>>>>> +     struct fuse_attr attr;
>>>>> +     struct fuse_statx *sx;
>>>>> +     struct fuse_statx_in inarg;
>>>>> +     struct fuse_statx_out outarg;
>>>>> +     struct fuse_mount *fm = get_fuse_mount(inode);
>>>>> +     u64 attr_version = fuse_get_attr_version(fm->fc);
>>>>> +     FUSE_ARGS(args);
>>>>> +
>>>>> +     memset(&inarg, 0, sizeof(inarg));
>>>>> +     memset(&outarg, 0, sizeof(outarg));
>>>>> +     /* Directories have separate file-handle space */
>>>>> +     if (file && S_ISREG(inode->i_mode)) {
>>>>> +             struct fuse_file *ff = file->private_data;
>>>>> +
>>>>> +             inarg.getattr_flags |= FUSE_GETATTR_FH;
>>>>> +             inarg.fh = ff->fh;
>>>>> +     }
>>>>> +     /* For now leave sync hints as the default, request all stats. */
>>>>> +     inarg.sx_flags = 0;
>>>>> +     inarg.sx_mask = STATX_BASIC_STATS | STATX_BTIME;
>>>>
>>>>
>>>>
>>>> What is actually the reason not to pass through flags from
>>>> fuse_update_get_attr()? Wouldn't it make sense to request the minimal
>>>> required mask and then server side can decide if it wants to fill in more?
>>>
>>> This and following commit is about btime and btime only.  It's about
>>> adding just this single attribute, otherwise the logic is unchanged.
>>>
>>> But the flexibility is there in the interface definition, and
>>> functionality can be added later.
>>
>> Sure, though what speaks against setting (limiting the mask) right away?
> 
> But then the result is basically uncacheable, until we have separate
> validity timeouts for each attribute.  Maybe we need that, maybe not,
> but it does definitely have side effects.

Ah right, updating the cache timeout shouldn't be done unless the reply 
contains all attributes. Although you already handle that in fuse_do_statx


	if ((sx->mask & STATX_BASIC_STATS) == STATX_BASIC_STATS) {
		fuse_change_attributes(inode, &attr, &outarg.stat,
				       ATTR_TIMEOUT(&outarg), attr_version);
	}



Thanks,
Bernd
Miklos Szeredi Aug. 23, 2023, 2:58 p.m. UTC | #10
On Wed, 23 Aug 2023 at 16:51, Bernd Schubert <bernd.schubert@fastmail.fm> wrote:
>
>
>
> On 8/23/23 08:18, Miklos Szeredi wrote:
> > On Tue, 22 Aug 2023 at 18:55, Bernd Schubert <bernd.schubert@fastmail.fm> wrote:
> >>
> >>
> >>
> >> On 8/22/23 17:33, Miklos Szeredi wrote:
> >>> On Tue, 22 Aug 2023 at 17:20, Bernd Schubert <bernd.schubert@fastmail.fm> wrote:
> >>>>
> >>>> Hi Miklos,
> >>>>
> >>>> sorry for late review.
> >>>>
> >>>> On 8/10/23 12:55, Miklos Szeredi wrote:
> >>>> [...]
> >>>>> +static int fuse_do_statx(struct inode *inode, struct file *file,
> >>>>> +                      struct kstat *stat)
> >>>>> +{
> >>>>> +     int err;
> >>>>> +     struct fuse_attr attr;
> >>>>> +     struct fuse_statx *sx;
> >>>>> +     struct fuse_statx_in inarg;
> >>>>> +     struct fuse_statx_out outarg;
> >>>>> +     struct fuse_mount *fm = get_fuse_mount(inode);
> >>>>> +     u64 attr_version = fuse_get_attr_version(fm->fc);
> >>>>> +     FUSE_ARGS(args);
> >>>>> +
> >>>>> +     memset(&inarg, 0, sizeof(inarg));
> >>>>> +     memset(&outarg, 0, sizeof(outarg));
> >>>>> +     /* Directories have separate file-handle space */
> >>>>> +     if (file && S_ISREG(inode->i_mode)) {
> >>>>> +             struct fuse_file *ff = file->private_data;
> >>>>> +
> >>>>> +             inarg.getattr_flags |= FUSE_GETATTR_FH;
> >>>>> +             inarg.fh = ff->fh;
> >>>>> +     }
> >>>>> +     /* For now leave sync hints as the default, request all stats. */
> >>>>> +     inarg.sx_flags = 0;
> >>>>> +     inarg.sx_mask = STATX_BASIC_STATS | STATX_BTIME;
> >>>>
> >>>>
> >>>>
> >>>> What is actually the reason not to pass through flags from
> >>>> fuse_update_get_attr()? Wouldn't it make sense to request the minimal
> >>>> required mask and then server side can decide if it wants to fill in more?
> >>>
> >>> This and following commit is about btime and btime only.  It's about
> >>> adding just this single attribute, otherwise the logic is unchanged.
> >>>
> >>> But the flexibility is there in the interface definition, and
> >>> functionality can be added later.
> >>
> >> Sure, though what speaks against setting (limiting the mask) right away?
> >
> > But then the result is basically uncacheable, until we have separate
> > validity timeouts for each attribute.  Maybe we need that, maybe not,
> > but it does definitely have side effects.
>
> Ah right, updating the cache timeout shouldn't be done unless the reply
> contains all attributes. Although you already handle that in fuse_do_statx

Yes, correctness is guaranteed.

However not setting the full mask might easily result in a performance
regression. At this point just avoid such issues by not allowing
partial masks to reach the server.

Thanks,
Miklos


>
>
>         if ((sx->mask & STATX_BASIC_STATS) == STATX_BASIC_STATS) {
>                 fuse_change_attributes(inode, &attr, &outarg.stat,
>                                        ATTR_TIMEOUT(&outarg), attr_version);
>         }
>
>
>
> Thanks,
> Bernd
Bernd Schubert Aug. 23, 2023, 3:24 p.m. UTC | #11
On 8/23/23 16:58, Miklos Szeredi wrote:
> On Wed, 23 Aug 2023 at 16:51, Bernd Schubert <bernd.schubert@fastmail.fm> wrote:
>>
>>
>>
>> On 8/23/23 08:18, Miklos Szeredi wrote:
>>> On Tue, 22 Aug 2023 at 18:55, Bernd Schubert <bernd.schubert@fastmail.fm> wrote:
>>>>
>>>>
>>>>
>>>> On 8/22/23 17:33, Miklos Szeredi wrote:
>>>>> On Tue, 22 Aug 2023 at 17:20, Bernd Schubert <bernd.schubert@fastmail.fm> wrote:
>>>>>>
>>>>>> Hi Miklos,
>>>>>>
>>>>>> sorry for late review.
>>>>>>
>>>>>> On 8/10/23 12:55, Miklos Szeredi wrote:
>>>>>> [...]
>>>>>>> +static int fuse_do_statx(struct inode *inode, struct file *file,
>>>>>>> +                      struct kstat *stat)
>>>>>>> +{
>>>>>>> +     int err;
>>>>>>> +     struct fuse_attr attr;
>>>>>>> +     struct fuse_statx *sx;
>>>>>>> +     struct fuse_statx_in inarg;
>>>>>>> +     struct fuse_statx_out outarg;
>>>>>>> +     struct fuse_mount *fm = get_fuse_mount(inode);
>>>>>>> +     u64 attr_version = fuse_get_attr_version(fm->fc);
>>>>>>> +     FUSE_ARGS(args);
>>>>>>> +
>>>>>>> +     memset(&inarg, 0, sizeof(inarg));
>>>>>>> +     memset(&outarg, 0, sizeof(outarg));
>>>>>>> +     /* Directories have separate file-handle space */
>>>>>>> +     if (file && S_ISREG(inode->i_mode)) {
>>>>>>> +             struct fuse_file *ff = file->private_data;
>>>>>>> +
>>>>>>> +             inarg.getattr_flags |= FUSE_GETATTR_FH;
>>>>>>> +             inarg.fh = ff->fh;
>>>>>>> +     }
>>>>>>> +     /* For now leave sync hints as the default, request all stats. */
>>>>>>> +     inarg.sx_flags = 0;
>>>>>>> +     inarg.sx_mask = STATX_BASIC_STATS | STATX_BTIME;
>>>>>>
>>>>>>
>>>>>>
>>>>>> What is actually the reason not to pass through flags from
>>>>>> fuse_update_get_attr()? Wouldn't it make sense to request the minimal
>>>>>> required mask and then server side can decide if it wants to fill in more?
>>>>>
>>>>> This and following commit is about btime and btime only.  It's about
>>>>> adding just this single attribute, otherwise the logic is unchanged.
>>>>>
>>>>> But the flexibility is there in the interface definition, and
>>>>> functionality can be added later.
>>>>
>>>> Sure, though what speaks against setting (limiting the mask) right away?
>>>
>>> But then the result is basically uncacheable, until we have separate
>>> validity timeouts for each attribute.  Maybe we need that, maybe not,
>>> but it does definitely have side effects.
>>
>> Ah right, updating the cache timeout shouldn't be done unless the reply
>> contains all attributes. Although you already handle that in fuse_do_statx
> 
> Yes, correctness is guaranteed.
> 
> However not setting the full mask might easily result in a performance
> regression. At this point just avoid such issues by not allowing
> partial masks to reach the server.


Hmm ok, I see a bit more like "these flags are absolutely needed, if you 
(daemon/server) can provide more/all, I can update my cache timeout".
Bernd Schubert Aug. 23, 2023, 3:24 p.m. UTC | #12
On 8/23/23 16:58, Miklos Szeredi wrote:
> On Wed, 23 Aug 2023 at 16:51, Bernd Schubert <bernd.schubert@fastmail.fm> wrote:
>>
>>
>>
>> On 8/23/23 08:18, Miklos Szeredi wrote:
>>> On Tue, 22 Aug 2023 at 18:55, Bernd Schubert <bernd.schubert@fastmail.fm> wrote:
>>>>
>>>>
>>>>
>>>> On 8/22/23 17:33, Miklos Szeredi wrote:
>>>>> On Tue, 22 Aug 2023 at 17:20, Bernd Schubert <bernd.schubert@fastmail.fm> wrote:
>>>>>>
>>>>>> Hi Miklos,
>>>>>>
>>>>>> sorry for late review.
>>>>>>
>>>>>> On 8/10/23 12:55, Miklos Szeredi wrote:
>>>>>> [...]
>>>>>>> +static int fuse_do_statx(struct inode *inode, struct file *file,
>>>>>>> +                      struct kstat *stat)
>>>>>>> +{
>>>>>>> +     int err;
>>>>>>> +     struct fuse_attr attr;
>>>>>>> +     struct fuse_statx *sx;
>>>>>>> +     struct fuse_statx_in inarg;
>>>>>>> +     struct fuse_statx_out outarg;
>>>>>>> +     struct fuse_mount *fm = get_fuse_mount(inode);
>>>>>>> +     u64 attr_version = fuse_get_attr_version(fm->fc);
>>>>>>> +     FUSE_ARGS(args);
>>>>>>> +
>>>>>>> +     memset(&inarg, 0, sizeof(inarg));
>>>>>>> +     memset(&outarg, 0, sizeof(outarg));
>>>>>>> +     /* Directories have separate file-handle space */
>>>>>>> +     if (file && S_ISREG(inode->i_mode)) {
>>>>>>> +             struct fuse_file *ff = file->private_data;
>>>>>>> +
>>>>>>> +             inarg.getattr_flags |= FUSE_GETATTR_FH;
>>>>>>> +             inarg.fh = ff->fh;
>>>>>>> +     }
>>>>>>> +     /* For now leave sync hints as the default, request all stats. */
>>>>>>> +     inarg.sx_flags = 0;
>>>>>>> +     inarg.sx_mask = STATX_BASIC_STATS | STATX_BTIME;
>>>>>>
>>>>>>
>>>>>>
>>>>>> What is actually the reason not to pass through flags from
>>>>>> fuse_update_get_attr()? Wouldn't it make sense to request the minimal
>>>>>> required mask and then server side can decide if it wants to fill in more?
>>>>>
>>>>> This and following commit is about btime and btime only.  It's about
>>>>> adding just this single attribute, otherwise the logic is unchanged.
>>>>>
>>>>> But the flexibility is there in the interface definition, and
>>>>> functionality can be added later.
>>>>
>>>> Sure, though what speaks against setting (limiting the mask) right away?
>>>
>>> But then the result is basically uncacheable, until we have separate
>>> validity timeouts for each attribute.  Maybe we need that, maybe not,
>>> but it does definitely have side effects.
>>
>> Ah right, updating the cache timeout shouldn't be done unless the reply
>> contains all attributes. Although you already handle that in fuse_do_statx
> 
> Yes, correctness is guaranteed.
> 
> However not setting the full mask might easily result in a performance
> regression. At this point just avoid such issues by not allowing
> partial masks to reach the server.


Hmm ok, I see a bit more like "these flags are absolutely needed, if you 
(daemon/server) can provide more/all, I can update my cache timeout".


Thanks,
Bernd
diff mbox series

Patch

diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index 04006db6e173..552157bd6a4d 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -350,10 +350,14 @@  int fuse_valid_type(int m)
 		S_ISBLK(m) || S_ISFIFO(m) || S_ISSOCK(m);
 }
 
+bool fuse_valid_size(u64 size)
+{
+	return size <= LLONG_MAX;
+}
+
 bool fuse_invalid_attr(struct fuse_attr *attr)
 {
-	return !fuse_valid_type(attr->mode) ||
-		attr->size > LLONG_MAX;
+	return !fuse_valid_type(attr->mode) || !fuse_valid_size(attr->size);
 }
 
 int fuse_lookup_name(struct super_block *sb, u64 nodeid, const struct qstr *name,
@@ -1143,6 +1147,84 @@  static void fuse_fillattr(struct inode *inode, struct fuse_attr *attr,
 	stat->blksize = 1 << blkbits;
 }
 
+static void fuse_statx_to_attr(struct fuse_statx *sx, struct fuse_attr *attr)
+{
+	memset(attr, 0, sizeof(*attr));
+	attr->ino = sx->ino;
+	attr->size = sx->size;
+	attr->blocks = sx->blocks;
+	attr->atime = sx->atime.tv_sec;
+	attr->mtime = sx->mtime.tv_sec;
+	attr->ctime = sx->ctime.tv_sec;
+	attr->atimensec = sx->atime.tv_nsec;
+	attr->mtimensec = sx->mtime.tv_nsec;
+	attr->ctimensec = sx->ctime.tv_nsec;
+	attr->mode = sx->mode;
+	attr->nlink = sx->nlink;
+	attr->uid = sx->uid;
+	attr->gid = sx->gid;
+	attr->rdev = new_encode_dev(MKDEV(sx->rdev_major, sx->rdev_minor));
+	attr->blksize = sx->blksize;
+}
+
+static int fuse_do_statx(struct inode *inode, struct file *file,
+			 struct kstat *stat)
+{
+	int err;
+	struct fuse_attr attr;
+	struct fuse_statx *sx;
+	struct fuse_statx_in inarg;
+	struct fuse_statx_out outarg;
+	struct fuse_mount *fm = get_fuse_mount(inode);
+	u64 attr_version = fuse_get_attr_version(fm->fc);
+	FUSE_ARGS(args);
+
+	memset(&inarg, 0, sizeof(inarg));
+	memset(&outarg, 0, sizeof(outarg));
+	/* Directories have separate file-handle space */
+	if (file && S_ISREG(inode->i_mode)) {
+		struct fuse_file *ff = file->private_data;
+
+		inarg.getattr_flags |= FUSE_GETATTR_FH;
+		inarg.fh = ff->fh;
+	}
+	/* For now leave sync hints as the default, request all stats. */
+	inarg.sx_flags = 0;
+	inarg.sx_mask = STATX_BASIC_STATS | STATX_BTIME;
+	args.opcode = FUSE_STATX;
+	args.nodeid = get_node_id(inode);
+	args.in_numargs = 1;
+	args.in_args[0].size = sizeof(inarg);
+	args.in_args[0].value = &inarg;
+	args.out_numargs = 1;
+	args.out_args[0].size = sizeof(outarg);
+	args.out_args[0].value = &outarg;
+	err = fuse_simple_request(fm, &args);
+	if (err)
+		return err;
+
+	sx = &outarg.stat;
+	if (((sx->mask & STATX_SIZE) && !fuse_valid_size(sx->size)) ||
+	    ((sx->mask & STATX_TYPE) && (!fuse_valid_type(sx->mode) ||
+					 inode_wrong_type(inode, sx->mode)))) {
+		make_bad_inode(inode);
+		return -EIO;
+	}
+
+	fuse_statx_to_attr(&outarg.stat, &attr);
+	if ((sx->mask & STATX_BASIC_STATS) == STATX_BASIC_STATS) {
+		fuse_change_attributes(inode, &attr, ATTR_TIMEOUT(&outarg),
+				       attr_version);
+	}
+	stat->result_mask = sx->mask & (STATX_BASIC_STATS | STATX_BTIME);
+	stat->btime.tv_sec = sx->btime.tv_sec;
+	stat->btime.tv_nsec = min_t(u32, sx->btime.tv_nsec, NSEC_PER_SEC - 1);
+	fuse_fillattr(inode, &attr, stat);
+	stat->result_mask |= STATX_TYPE;
+
+	return 0;
+}
+
 static int fuse_do_getattr(struct inode *inode, struct kstat *stat,
 			   struct file *file)
 {
@@ -1194,13 +1276,18 @@  static int fuse_update_get_attr(struct inode *inode, struct file *file,
 				unsigned int flags)
 {
 	struct fuse_inode *fi = get_fuse_inode(inode);
+	struct fuse_conn *fc = get_fuse_conn(inode);
 	int err = 0;
 	bool sync;
 	u32 inval_mask = READ_ONCE(fi->inval_mask);
 	u32 cache_mask = fuse_get_cache_mask(inode);
 
-	/* FUSE only supports basic stats */
-	request_mask &= STATX_BASIC_STATS;
+
+	/* FUSE only supports basic stats and possibly btime */
+	request_mask &= STATX_BASIC_STATS | STATX_BTIME;
+retry:
+	if (fc->no_statx)
+		request_mask &= STATX_BASIC_STATS;
 
 	if (!request_mask)
 		sync = false;
@@ -1215,7 +1302,16 @@  static int fuse_update_get_attr(struct inode *inode, struct file *file,
 
 	if (sync) {
 		forget_all_cached_acls(inode);
-		err = fuse_do_getattr(inode, stat, file);
+		/* Try statx if BTIME is requested */
+		if (!fc->no_statx && (request_mask & ~STATX_BASIC_STATS)) {
+			err = fuse_do_statx(inode, file, stat);
+			if (err == -ENOSYS) {
+				fc->no_statx = 1;
+				goto retry;
+			}
+		} else {
+			err = fuse_do_getattr(inode, stat, file);
+		}
 	} else if (stat) {
 		generic_fillattr(&nop_mnt_idmap, inode, stat);
 		stat->mode = fi->orig_i_mode;
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index fd55c09514cd..daae31c58754 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -792,6 +792,9 @@  struct fuse_conn {
 	/* Is tmpfile not implemented by fs? */
 	unsigned int no_tmpfile:1;
 
+	/* Is statx not implemented by fs? */
+	unsigned int no_statx:1;
+
 	/** The number of requests waiting for completion */
 	atomic_t num_waiting;
 
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index f19d748890f0..a6cc102e66bc 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -77,7 +77,7 @@  static struct inode *fuse_alloc_inode(struct super_block *sb)
 		return NULL;
 
 	fi->i_time = 0;
-	fi->inval_mask = 0;
+	fi->inval_mask = ~0;
 	fi->nodeid = 0;
 	fi->nlookup = 0;
 	fi->attr_version = 0;
@@ -172,7 +172,8 @@  void fuse_change_attributes_common(struct inode *inode, struct fuse_attr *attr,
 
 	fi->attr_version = atomic64_inc_return(&fc->attr_version);
 	fi->i_time = attr_valid;
-	WRITE_ONCE(fi->inval_mask, 0);
+	/* Clear basic stats from invalid mask */
+	set_mask_bits(&fi->inval_mask, STATX_BASIC_STATS, 0);
 
 	inode->i_ino     = fuse_squash_ino(attr->ino);
 	inode->i_mode    = (inode->i_mode & S_IFMT) | (attr->mode & 07777);