Re: [PATCH 2/2] rpdfs: Set the inode creation time

On Wed, Mar 11, 2026 at 6:59=E2=80=AFPM Zach Brown <[email protected]> wrote:
>
> On Tue, Mar 10, 2026 at 06:31:40PM +0100, Valerie Aurora wrote:
> > +/*
> > + * Set the inode creation time.
> > + */
> > +void rpdfs_inode_crtime_set(struct rpdfs_fs_info *rfi, struct rpdfs_tr=
ansaction *txn,
> > +                         struct inode *inode, struct timespec64 ts)
> > +{
> > +     struct rpdfs_block_handle *hnd =3D NULL;
> > +     struct rpdfs_inode *rinode;
> > +     int ret;
> > +
> > +     ret =3D rpdfs_txn_use_prepared(rfi, txn, rpdfs_inode_bnr(inode), =
&hnd, RBAF_WRITE);
> > +     BUG_ON(ret < 0); /* caller must have prepared */
> > +
> > +     rinode =3D hnd->data;
> > +     rinode->crtime_nsec =3D cpu_ts64_to_le64_ns(ts);
> > +}
>
> The pattern is that the in-memory inode (struct rpdfs_inode_info) is a
> struct that has private fs stuff along with the storage for the vfs
> inode.  This is the FOO_I() pattern found all over fs/*.  Refreshing the
> inode updates the in-memory version from the current block, and the "txn
> update" of the inode copies from memory to the block again.
>
> So the way to do this is to have a local copy of the crtime in the
> rpdfs_inode_info.  Copy it back and forth in copy_vfs_inode_to_rinode()
> and vice versa.

The reason I didn't do this is that creation time is only set at inode
creation time and almost never read, so it seems like a waste of
memory to put it in the rpdfs_inode_info. It's only accessed through
getattr, so that needs to do its own separate block lookup anyway.

> Then in the apply phase that wants to update crtime it updates the
> in-memory copy with something like:
>
>         struct rpdfs_inode_info *ri =3D RPDFS_I(inode);
>         ri->crtime_nsec =3D inode_atime_to_u64_nsec_or_whatever(inode);
>
> This re-uses the current load from and store to the block and avoids
> another block hash lookup -- which is a bunch of rcu and spinlock
> barriers and will be a point of contention.

That's a good point. The other option I thought of is a
rpdfs_inode_txn_update_create() that takes a creation time arg and is
only used at inode creation.

But if you prefer, we can keep creation time in rpdfs_inode_info. Let
me know what you want.

Valerie