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

[Amir Goldstein <amir73il@gmail.com>]

On Thu, Feb 18, 2021 at 1:11 PM Jan Kara <jack@suse.cz> wrote:
>
> 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?
>

Makes sense.

Thanks,
Amir.