From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx2.suse.de ([195.135.220.15]:56948 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1728108AbeK3AQc (ORCPT ); Thu, 29 Nov 2018 19:16:32 -0500 Date: Thu, 29 Nov 2018 14:11:11 +0100 From: Jan Kara To: Amir Goldstein Cc: Jan Kara , Matthew Bobrowski , linux-fsdevel Subject: Re: [PATCH v3 09/13] fanotify: cache fsid in fsnotify_mark_connector Message-ID: <20181129131111.GO31087@quack2.suse.cz> References: <20181125134352.21499-1-amir73il@gmail.com> <20181125134352.21499-10-amir73il@gmail.com> <20181129104835.GJ31087@quack2.suse.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Thu 29-11-18 13:42:00, Amir Goldstein wrote: > On Thu, Nov 29, 2018 at 1:12 PM Jan Kara wrote: > > > Suggested-by: Jan Kara > > > Signed-off-by: Amir Goldstein > > > --- > > > 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 SUSE Labs, CR