On Mon, Aug 18, 2025 at 01:01:55PM -0700, Darrick J. Wong wrote:
> On Mon, Aug 18, 2025 at 05:11:07PM +0200, Miklos Szeredi wrote:
> > On Fri, 18 Jul 2025 at 01:27, Darrick J. Wong <djwong@xxxxxxxxxx> wrote:
> > >
> > > From: Darrick J. Wong <djwong@xxxxxxxxxx>
> > >
> > > Actually copy the attributes/attributes_mask from userspace.
> > >
> > > Signed-off-by: "Darrick J. Wong" <djwong@xxxxxxxxxx>
> > > ---
> > > fs/fuse/dir.c | 2 ++
> > > 1 file changed, 2 insertions(+)
> > >
> > >
> > > diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
> > > index 45b4c3cc1396af..4d841869ba3d0a 100644
> > > --- a/fs/fuse/dir.c
> > > +++ b/fs/fuse/dir.c
> > > @@ -1285,6 +1285,8 @@ static int fuse_do_statx(struct mnt_idmap *idmap, struct inode *inode,
> > > 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);
> > > + stat->attributes = sx->attributes;
> > > + stat->attributes_mask = sx->attributes_mask;
> >
> > fuse_update_get_attr() has a cached and an uncached branch and these
> > fields are only getting set in the uncached case.
>
> Hrmm, do you want to cache all the various statx attributes in struct
> fuse_inode? Or would you rather that the kernel always call the fuse
> server if any of the statx flags outside of (BASIC_STATS|BTIME) are set?
I should have said explicitly that attributes/attributes_mask need to be
cached because there's no separate STATX_ request flag for the bitfield.
However, the *new* fields that have been added since BASIC_STATS are the
subject of my ramblings below.
--D
> Right now the full version of kstat_from_fuse_statx contains:
>
> if (sx->mask & 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);
> }
>
> if (sx->mask & STATX_DIOALIGN) {
> stat->dio_mem_align = sx->dio_mem_align;
> stat->dio_offset_align = sx->dio_offset_align;
> }
>
> if (sx->mask & STATX_SUBVOL)
> stat->subvol = sx->subvol;
>
> if (sx->mask & STATX_WRITE_ATOMIC) {
> stat->atomic_write_unit_min = sx->atomic_write_unit_min;
> stat->atomic_write_unit_max = sx->atomic_write_unit_max;
> stat->atomic_write_unit_max_opt = sx->atomic_write_unit_max_opt;
> stat->atomic_write_segments_max = sx->atomic_write_segments_max;
> }
>
> if (sx->mask & STATX_DIO_READ_ALIGN)
> stat->dio_read_offset_align = sx->dio_read_offset_align;
>
> In theory only specialty programs are going to be interested in directio
> or atomic writes, and only userspace nfs servers and backup programs are
> going to care about subvolumes, so I don't know if it's really worth the
> trouble to cache all that.
>
> The dio/atomic fields are 7x u32, and the subvol id is u64. That's 40
> bytes per inode, which is kind of a lot.
>
> --D
>
> > Thanks,
> > Miklos
> >
>
