On Sat, Jun 9, 2018 at 8:30 PM, Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote: > On Fri, Jun 8, 2018 at 11:57 PM Amir Goldstein <amir73il@xxxxxxxxx> wrote: >> >> We embed fsnotify_obj in struct inode and fsnotify_obj_mask in struct mount. > > So I'd *really* like to see just a pointer, not an embedded struct. > > Yes, if you get rid of the mask from the embedded struct (so that it > only contains a pointer), you do get rid of the odd alignment issues > and the need for "packed". > > But from previous experience, once you embed a structure, that > structure tends to grow. Because it can, and it's so convenient. > Suddently it has a spinlock in it too etc. > Fair enough. > So if you can make do with just the pointer, it would actually be > nicer to expose it as such. Then you can also avoid the header file > dependency chain, because you can just pre-declare the structure (like > it does now) and > > struct fsnotify_mark_connector; > .. > struct fsnotify_mark_connector __rcu *i_fsnotify_marks; > > in the inode. That way the core header files don't need to worry about > the fsnotify details, and don't need to include fsnotify headers. > > And we can do inode packing without having to know (not that it > happens all that often - everybody would *love* to shrink the inode > structure, but it's just hard. Because everybody also wants to put > their own data into the inode ..) > > Can't the generalization code just take a pointer to a __rcu pointer > to fsnotify_mark_connector, obviating the need for the fsnotify_obj > structure definition? > Well, if Jan agrees with me that using 2 bits for FS_ACCESS and FS_EVENT_ON_CHILD on pointer is enough for the purpose of optimizing pointer dereference, then we can have generalized code with just the pointer in struct inode/mount. Thanks, Amir.
