Re: [GIT PULL] Fsnotify cleanups

[Jan Kara <jack@suse.cz>]

On Thu 14-06-18 01:17:38, Amir Goldstein wrote:
> On Wed, Jun 13, 2018 at 4:56 PM, Amir Goldstein <amir73il@gmail.com> wrote:
> > On Wed, Jun 13, 2018 at 4:21 PM, Jan Kara <jack@suse.cz> wrote:
> 
> >> Going through patches:
> >>
> >> Regarding "fsnotify: use abstract fsnotify_obj_t * instead of **connp
> >> argument" - I agree "struct fsnotify_mark_connector __rcu *" is quite
> >> verbose but given how things evolved I don't think "fsnotify_obj_t" is a
> >> great name. How about "fsnotify_connp_t" and keep parameter names as
> >> "connp" instead of renaming them to "obj"? Because abstraction (like
> >> pretending this is some kind of object when it is actually just a pointer)
> >> that does not actually abstract anything is just obfuscation... So let's be
> >> direct and admit this is just a shortcut name for connector pointer.
> >>
> >
> > I though you'd say that and I agree.
> > will rework after pull request.
> 
> Two places I couldn't resist keeping 'obj':
> 1. connector->obj
> 2. fsnotify_obj_{inode,mount}
> 
> The first one because conn->connp is horrible.
> The second one because most call sites pass conn->obj as argument.
> 
> Force pushed result to fsnotify-cleanup.
> 
> See if you find it acceptable.

Thanks. I like the patches. Just one suggestion for improvement:
Maybe we could call fsnotify_obj_inode() fsnotify_conn_inode()
and let it take connector as an argument. Everybody does conn->obj anyway
unnecessarily. Similarly for mount. And then fsnotify_connector_mask()
could be fsnotify_conn_mask() for consistency?

fsnotify_connector_inode() (or mount) is already a bit too long for my
taste ;).

If you update this, please post the patches to fsdevel with me on CC so
that we have an official posting and I'll pick them up to my tree.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR