Re: [GIT PULL] Fsnotify cleanups

[Amir Goldstein <amir73il@gmail.com>]

On Sat, Jun 9, 2018 at 8:30 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Fri, Jun 8, 2018 at 11:57 PM Amir Goldstein <amir73il@gmail.com> 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.