On Tue, 22 Aug 2023 at 18:55, Bernd Schubert <bernd.schubert@xxxxxxxxxxx> wrote:
>
>
>
> On 8/22/23 17:33, Miklos Szeredi wrote:
> > On Tue, 22 Aug 2023 at 17:20, Bernd Schubert <bernd.schubert@xxxxxxxxxxx> 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
