Re: [PATCH 86/87] fs: switch timespec64 fields in inode to discrete integers

Darrick J. Wong
2023-09-28T17:19:50Z

On Thu, Sep 28, 2023 at 01:06:03PM -0400, Jeff Layton wrote:
> On Thu, 2023-09-28 at 11:48 -0400, Arnd Bergmann wrote:
> > On Thu, Sep 28, 2023, at 07:05, Jeff Layton wrote:
> > > This shaves 8 bytes off struct inode, according to pahole.
> > >
> > > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> >
> > FWIW, this is similar to the approach that Deepa suggested
> > back in 2016:
> >
> > https://lore.kernel.org/lkml/1452144972-15802-3-git-send-email-deepa.kernel@gmail.com/
> >
> > It was NaKed at the time because of the added complexity,
> > though it would have been much easier to do it then,
> > as we had to touch all the timespec references anyway.
> >
> > The approach still seems ok to me, but I'm not sure it's worth
> > doing it now if we didn't do it then.
> >
>
> I remember seeing those patches go by. I don't remember that change
> being NaK'ed, but I wasn't paying close attention at the time
>
> Looking at it objectively now, I think it's worth it to recover 8 bytes
> per inode and open a 4 byte hole that Amir can use to grow the
> i_fsnotify_mask. We might even able to shave off another 12 bytes
> eventually if we can move to a single 64-bit word per timestamp.

I don't think you can, since btrfs timestamps utilize s64 seconds
counting in both directions from the Unix epoch. They also support ns
resolution:

struct btrfs_timespec {
__le64 sec;
__le32 nsec;
} __attribute__ ((__packed__));

--D

> It is a lot of churn though.
> --
> Jeff Layton <jlayton@kernel.org>