linux-kernel - Re: [PATCH v2 89/89] fs: move i_generation into new hole created after timestamp conversion

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 05 Oct 2023 08:45:53 -0400
From:   Jeff Layton <jlayton@...nel.org>
To:     Amir Goldstein <amir73il@...il.com>
Cc:     Christian Brauner <brauner@...nel.org>,
        Al Viro <viro@...iv.linux.org.uk>, Jan Kara <jack@...e.cz>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        linux-fsdevel <linux-fsdevel@...r.kernel.org>
Subject: Re: [PATCH v2 89/89] fs: move i_generation into new hole created
 after timestamp conversion

On Thu, 2023-10-05 at 08:08 +0300, Amir Goldstein wrote:
> On Wed, Oct 4, 2023 at 9:56 PM Jeff Layton <jlayton@...nel.org> wrote:
> > 
> > The recent change to use discrete integers instead of struct timespec64
> > shaved 8 bytes off of struct inode, but it also moves the i_lock
> > into the previous cacheline, away from the fields that it protects.
> > 
> > Move i_generation above the i_lock, which moves the new 4 byte hole to
> > just after the i_fsnotify_mask in my setup.
> 
> Might be good to mention that this hole has a purpose...
> 
> > 
> > Suggested-by: Amir Goldstein <amir73il@...il.com>
> > Signed-off-by: Jeff Layton <jlayton@...nel.org>
> > ---
> >  include/linux/fs.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/include/linux/fs.h b/include/linux/fs.h
> > index 485b5e21c8e5..686c9f33e725 100644
> > --- a/include/linux/fs.h
> > +++ b/include/linux/fs.h
> > @@ -677,6 +677,7 @@ struct inode {
> >         u32                     i_atime_nsec;
> >         u32                     i_mtime_nsec;
> >         u32                     i_ctime_nsec;
> > +       u32                     i_generation;
> >         spinlock_t              i_lock; /* i_blocks, i_bytes, maybe i_size */
> >         unsigned short          i_bytes;
> >         u8                      i_blkbits;
> > @@ -733,7 +734,6 @@ struct inode {
> >                 unsigned                i_dir_seq;
> >         };
> > 
> > -       __u32                   i_generation;
> > 
> >  #ifdef CONFIG_FSNOTIFY
> >         __u32                   i_fsnotify_mask; /* all events this inode cares about */
> 
> If you post another version, please leave a comment here
> 
> +         /* 32bit hole reserved for expanding i_fsnotify_mask to 64bit */
> 

Sure.

I suppose we could create a union there too if you really want to
reserve it:

	union {
		__u32		i_fsnotify_mask;
		__u64		__i_fsnotify_mask_ext;
	};

-- 
Jeff Layton <jlayton@...nel.org>