Re: [GIT PULL] Fsnotify cleanups

[Amir Goldstein <amir73il@gmail.com>]

On Mon, Jun 11, 2018 at 7:12 PM, Jan Kara <jack@suse.cz> wrote:
> On Sat 09-06-18 11:00:17, Amir Goldstein wrote:
>> On Sat, Jun 9, 2018 at 9:57 AM, Amir Goldstein <amir73il@gmail.com> wrote:
>> > The only code that needs to test the shadow mask before following into
>> > connector is the optimization code in start of fsnotify() which was not
>> > generalized anyway and where the mask optimization is more important
>> > for mount watches.
>> >
>>
>> No. That's not true, there is also fsnotify_inode_watches_children() and
>> __fsnotify_parent(). But I think the only bits in the mask that really matter
>> for optimizing inode watches are FS_ACCESS and FS_EVENT_ON_CHILD
>> and those could be encoded in lower bits of connector pointer.
>
> Actually for children events, we have a dentry flag
> DCACHE_FSNOTIFY_PARENT_WATCHED and that should catch 99% of situation.
> FS_EVENT_ON_CHILD is used only to verify event really should be delivered
> since the DCACHE_FSNOTIFY_PARENT_WATCHED flag is cleared lazily. Or where
> do you think the performance of FS_EVENT_ON_CHILD checking matters?
>

I thought that dereferencing the connector from
fsnotify_inode_watches_children()
is not a good idea, so we can set that bit  in connector pointer during
fsnotify_recalc_mask().

> Also I'm not 100% sure just having FS_ACCESS checked quickly is going to
> buy us that much. We have quite a few event types and if someone is
> watching for a different event type than the current event (just not
> FS_ACCESS), we'll just have to pay the cost of srcu_read_lock and
> indirection. So it just depends whether we consider such loads worthwhile
> or not...

I just though user could generate FS_ACCESS callbacks on an inode
at higher rate than any other event types, so for other event types
optimization is going to be less noticeable. Not sure that is true.

Thanks,
Amir.