Re: [PATCH 86/87] fs: switch timespec64 fields in inode to discrete integers
Amir Goldstein <[email protected]>
| Newsgroups | gmane.linux.file-systems.union,gmane.linux.file-systems,gmane.linux.kernel,gmane.linux.kernel.mm,gmane.linux.ports.ppc64.devel,gmane.linux.drivers.platform.x86.devel,gmane.linux.drivers.rdma,gmane.linux.serial,gmane.linux.usb.general,gmane.linux.kernel.autofs,gmane.comp.file-systems.btrfs,gmane.comp.file-systems.ceph.devel,gmane.comp.file-systems.coda.general,gmane.linux.kernel.efi,gmane.comp.file-systems.ext4,gmane.linux.file-systems.f2fs,gmane.linux.uml.devel,gmane.linux.drivers.mtd,gmane.linux.nfs,gmane.comp.file-systems.nilfs.user,gmane.linux.file-systems.ntfs.devel,gmane.linux.hardware.karma.devel,gmane.comp.file-systems.reiserfs.general,gmane.linux.kernel.cifs,gmane.network.samba.internals,gmane.linux.kernel.bpf,gmane.linux.network,gmane.comp.security.apparmor,gmane.linux.kernel.lsm |
|---|---|
| Message-ID | <CAOQ4uxjTpPPUa3VXW+DWKy72JABOZBCXD6pjNk-FhJZWnqvNPA@mail.gmail.com> |
On Thu, Sep 28, 2023 at 8:19 PM Darrick J. Wong <[email protected]> wrote: > > 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 <[email protected]> > > > > > > FWIW, this is similar to the approach that Deepa suggested > > > back in 2016: > > > > > > https://lore.kernel.org/lkml/[email protected]/ > > > > > > 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.