| Message ID | 20190213145443.26836-3-jack@suse.cz (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | fanotify: Make wait for permission event response interruptible | expand |
On Wed, Feb 13, 2019 at 4:54 PM Jan Kara <jack@suse.cz> wrote: > > get_one_event() has a single caller and that just locks > notification_lock around the call. Move locking inside get_one_event() > as that will make using ->response field for permission event state > easier. > > Signed-off-by: Jan Kara <jack@suse.cz> Reviewed-by: Amir Goldstein <amir73il@gmail.com> > --- > fs/notify/fanotify/fanotify_user.c | 25 +++++++++++-------------- > 1 file changed, 11 insertions(+), 14 deletions(-) > > diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c > index 908ebc421d15..4e36f5642797 100644 > --- a/fs/notify/fanotify/fanotify_user.c > +++ b/fs/notify/fanotify/fanotify_user.c > @@ -51,25 +51,25 @@ struct kmem_cache *fanotify_perm_event_cachep __read_mostly; > * Get an fsnotify notification event if one exists and is small > * enough to fit in "count". Return an error pointer if the count > * is not large enough. > - * > - * Called with the group->notification_lock held. > */ > static struct fsnotify_event *get_one_event(struct fsnotify_group *group, > size_t count) > { > - assert_spin_locked(&group->notification_lock); > + struct fsnotify_event *event = NULL; > > pr_debug("%s: group=%p count=%zd\n", __func__, group, count); > - > + spin_lock(&group->notification_lock); > if (fsnotify_notify_queue_is_empty(group)) > - return NULL; > + goto out; > > - if (FAN_EVENT_METADATA_LEN > count) > - return ERR_PTR(-EINVAL); > - > - /* held the notification_lock the whole time, so this is the > - * same event we peeked above */ > - return fsnotify_remove_first_event(group); > + if (FAN_EVENT_METADATA_LEN > count) { > + event = ERR_PTR(-EINVAL); > + goto out; > + } > + event = fsnotify_remove_first_event(group); > +out: > + spin_unlock(&group->notification_lock); > + return event; > } > > static int create_fd(struct fsnotify_group *group, > @@ -261,10 +261,7 @@ static ssize_t fanotify_read(struct file *file, char __user *buf, > > add_wait_queue(&group->notification_waitq, &wait); > while (1) { > - spin_lock(&group->notification_lock); > kevent = get_one_event(group, count); > - spin_unlock(&group->notification_lock); > - > if (IS_ERR(kevent)) { > ret = PTR_ERR(kevent); > break; > -- > 2.16.4 >
diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c index 908ebc421d15..4e36f5642797 100644 --- a/fs/notify/fanotify/fanotify_user.c +++ b/fs/notify/fanotify/fanotify_user.c @@ -51,25 +51,25 @@ struct kmem_cache *fanotify_perm_event_cachep __read_mostly; * Get an fsnotify notification event if one exists and is small * enough to fit in "count". Return an error pointer if the count * is not large enough. - * - * Called with the group->notification_lock held. */ static struct fsnotify_event *get_one_event(struct fsnotify_group *group, size_t count) { - assert_spin_locked(&group->notification_lock); + struct fsnotify_event *event = NULL; pr_debug("%s: group=%p count=%zd\n", __func__, group, count); - + spin_lock(&group->notification_lock); if (fsnotify_notify_queue_is_empty(group)) - return NULL; + goto out; - if (FAN_EVENT_METADATA_LEN > count) - return ERR_PTR(-EINVAL); - - /* held the notification_lock the whole time, so this is the - * same event we peeked above */ - return fsnotify_remove_first_event(group); + if (FAN_EVENT_METADATA_LEN > count) { + event = ERR_PTR(-EINVAL); + goto out; + } + event = fsnotify_remove_first_event(group); +out: + spin_unlock(&group->notification_lock); + return event; } static int create_fd(struct fsnotify_group *group, @@ -261,10 +261,7 @@ static ssize_t fanotify_read(struct file *file, char __user *buf, add_wait_queue(&group->notification_waitq, &wait); while (1) { - spin_lock(&group->notification_lock); kevent = get_one_event(group, count); - spin_unlock(&group->notification_lock); - if (IS_ERR(kevent)) { ret = PTR_ERR(kevent); break;
get_one_event() has a single caller and that just locks notification_lock around the call. Move locking inside get_one_event() as that will make using ->response field for permission event state easier. Signed-off-by: Jan Kara <jack@suse.cz> --- fs/notify/fanotify/fanotify_user.c | 25 +++++++++++-------------- 1 file changed, 11 insertions(+), 14 deletions(-)