Re: [GIT PULL] Fsnotify cleanups

[Amir Goldstein <amir73il@gmail.com>]

On Sat, Jun 9, 2018 at 9:46 PM, Amir Goldstein <amir73il@gmail.com> wrote:
> 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?
>>
>

Jan,

I reworked the cleanup patches to get rid of fsnotify_obj and pushed to:
https://github.com/amir73il/linux.git fsnotify-cleanup

Only last 5 patches from fsnotify_for_v4.18-rc1 have been modified
and I removed your S-O-B from the modified patches.

This leaves struct inode unchanged, in fact no changes to code outside
fsnotify/audit at all.

mask is now a member of connector for the purpose of generalizing
add/remove mark, but struct inode/mount still have a copy of the mask
for the purpose of the VFS optimizations.

I have a POC patch that removes mask from struct inode and
uses 2 bits in connector pointer for VFS optimizations, but that patch
requires more testing, so for another time.

Thanks,
Amir.