On Thu, Sep 28, 2023 at 8:19 PM Darrick J. Wong [off-list ref] wrote:

On Thu, Sep 28, 2023 at 01:06:03PM -0400, Jeff Layton wrote:

quoted

On Thu, 2023-09-28 at 11:48 -0400, Arnd Bergmann wrote:

quoted

On Thu, Sep 28, 2023, at 07:05, Jeff Layton wrote:

quoted

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/ (local)

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

Sure we can.
That's what btrfs_inode is for.
vfs inode also does not store i_otime (birth time) and there is even a
precedent of vfs/btrfs variable size mismatch:

        /* full 64 bit generation number, struct vfs_inode doesn't have a big
         * enough field for this.
         */
        u64 generation;

If we decide that vfs should use "bigtime", btrfs pre-historic
timestamps are not a show stopper.

Thanks,
Amir.