On Thu, Mar 5, 2020 at 5:49 PM Jan Kara <jack@xxxxxxx> wrote: > > Hi Amir! > > On Sun 01-03-20 18:26:25, Amir Goldstein wrote: > > > > I'd rather do the fanotity_fh padding optimization I outlined in another > > > > email. That would save one long without any packing and the following u8 > > > > name_len would get packed tightly after the fanotify_fh by the compiler. > > > > > > > > > > OK. I will try that and the non-inherited variant of perm/name event struct > > > and see how it looks like. > > > > > > > Pushed sample code to branch fanotify_name-wip: > > > > b5e56d3e1358 fanotify: fanotify_perm_event inherits from fanotify_path_event > > 55041285b3b7 fanotify: divorce fanotify_path_event and fanotify_fid_event > > Thanks for the work! > > > I opted for fanotify_name_event inherits from fanotify_fid_event, > > because it felt better this way. > > I've commented on github in the patches - I'm not sure the inheritance > really brings a significant benefit and it costs 6 bytes per name event. > Maybe there can be more simplifications gained from the inheritance (but I > think the move of fsid out of fanotify_fid mostly precludes that) but at > this point it doesn't seem to be worth it to me. > As agreed on github discussion, the padding is a non issue. To see what the benefit of inherit fanotify_fid_event is, I did a test patch to get rid of it and pushed the result to fanotify_name-wip: * b7eb8314c61b - fanotify: do not inherit fanotify_name_event from fanotify_fid_event IMO, the removal of inheritance in this struct is artificial and brings no benefit. There is not a single line of code that improved IMO vs. several added helpers which abstract something that is pretty obvious. That said, I don't mind going with this variant. Let me you what your final call is. Thanks, Amir.
