On Thu 29-11-18 13:42:00, Amir Goldstein wrote:
> On Thu, Nov 29, 2018 at 1:12 PM Jan Kara <jack@xxxxxxx> wrote:
> > > Suggested-by: Jan Kara <jack@xxxxxxx>
> > > Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx>
> > > ---
> > > fs/notify/fanotify/fanotify.c | 42 ++++++++++------
> > > fs/notify/fanotify/fanotify.h | 5 +-
> > > fs/notify/fanotify/fanotify_user.c | 62 ++++++++++++++----------
> > > fs/notify/mark.c | 77 ++++++++++++++++++++++++++----
> > > include/linux/fsnotify_backend.h | 24 ++++++++--
> > > 5 files changed, 154 insertions(+), 56 deletions(-)
> > >
> > > diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c
> > > index 844b748f0b74..90bac98dd8c7 100644
> > > --- a/fs/notify/fanotify/fanotify.c
> > > +++ b/fs/notify/fanotify/fanotify.c
> > > @@ -103,13 +103,13 @@ static int fanotify_get_response(struct fsnotify_group *group,
> > > * requested by the user, will not be present in the returned mask.
> > > */
> > > static u32 fanotify_group_event_mask(struct fsnotify_iter_info *iter_info,
> > > - u32 event_mask, const void *data,
> > > - int data_type)
> > > + u32 event_mask, const void *data,
> > > + int data_type, __kernel_fsid_t *fsid)
> > > {
> > > __u32 marks_mask = 0, marks_ignored_mask = 0;
> > > const struct path *path = data;
> > > struct fsnotify_mark *mark;
> > > - int type;
> > > + int type, err;
> > >
> > > pr_debug("%s: report_mask=%x mask=%x data=%p data_type=%d\n",
> > > __func__, iter_info->report_mask, event_mask, data, data_type);
> > > @@ -136,6 +136,13 @@ static u32 fanotify_group_event_mask(struct fsnotify_iter_info *iter_info,
> > > !(mark->mask & FS_EVENT_ON_CHILD)))
> > > continue;
> > >
> > > + if (fsid) {
> > > + err = fsnotify_get_conn_fsid(mark->connector, fsid);
> > > + /* Should we skip mark with null or conflicting fsid? */
> > > + pr_debug("%s: fsid=%x.%x type=%u err=%d\n", __func__,
> > > + fsid->val[0], fsid->val[1], type, err);
> > > + }
> > > +
> > > marks_mask |= mark->mask;
> > > marks_ignored_mask |= mark->ignored_mask;
> > > }
> >
> > I would just move getting of fsid into a separate function. I does not seem
> > to fit well into fanotify_group_event_mask() except for the fact that we
> > iterate through marks there... Also is the pr_debug() long-term useful
> > there?
>
> Don't mind to get rid of it. How about
> fsnotify_get_event_fsid(iter_info) will just get the first fsid it finds
> from any connector or via prioirty and not enforce conflicts here at all?
> Conflicts are supposed to be enforces when adding the mark anyway.
Sounds good to me.
> > > +/*
> > > + * Check consistent fsid for all marks on the same object.
> > > + * It is expected to always get the same fsid (or no fsid) for all
> > > + * marks added to object.
> > > + */
> > > +static int fsnotify_test_conn_fsid(const struct fsnotify_mark_connector *conn,
> > > + const __kernel_fsid_t *fsid,
> > > + const char *where)
> > > +
> >
> > Call this fsnotify_conn_fsid_consistent()?
> >
>
> As written above, I am considering dropping the call to this function from
> get_fsid path and use it only from set_fsid, so I might as well just open code
> this test in set_fsid.
OK.
Honza
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
