Re: [PATCH 6/7] fanotify: mix event info into merge key hash

[Jan Kara <jack@suse.cz>]

On Thu 18-02-21 12:46:48, Amir Goldstein wrote:
> On Wed, Feb 17, 2021 at 12:13 PM Amir Goldstein <amir73il@gmail.com> wrote:
> > > > @@ -154,7 +162,10 @@ static inline void fanotify_init_event(struct fanotify_event *event,
> > > >
> > > >  struct fanotify_fid_event {
> > > >       struct fanotify_event fae;
> > > > -     __kernel_fsid_t fsid;
> > > > +     union {
> > > > +             __kernel_fsid_t fsid;
> > > > +             void *fskey;    /* 64 or 32 bits of fsid used for salt */
> > > > +     };
> > > >       struct fanotify_fh object_fh;
> > > >       /* Reserve space in object_fh.buf[] - access with fanotify_fh_buf() */
> > > >       unsigned char _inline_fh_buf[FANOTIFY_INLINE_FH_LEN];
> > > > @@ -168,7 +179,10 @@ FANOTIFY_FE(struct fanotify_event *event)
> > > >
> > > >  struct fanotify_name_event {
> > > >       struct fanotify_event fae;
> > > > -     __kernel_fsid_t fsid;
> > > > +     union {
> > > > +             __kernel_fsid_t fsid;
> > > > +             void *fskey;    /* 64 or 32 bits of fsid used for salt */
> > > > +     };
> > > >       struct fanotify_info info;
> > > >  };
> > >
> > > What games are you playing here with the unions? I presume you can remove
> > > these 'fskey' unions and just use (void *)(event->fsid) at appropriate
> > > places? IMO much more comprehensible...
> >
> 
> FYI, this is what the open coded conversion looks like:
> 
> (void *)*(long *)event->fsid.val

Not great but at least fairly localized. I'd just note that this doesn't quite
work on 32-bit archs (sizeof(long) != sizeof(__kernel_fsid_t) there). Maybe
we could just use

hash_32(event->fsid.val[0]) ^ hash_32(event->fsid.val[1])

for mixing into the 'key' value and thus avoid all these games?

								Honza

-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR