From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mout.gmx.net ([212.227.15.15]:65322 "EHLO mout.gmx.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1761088AbcINRO0 (ORCPT ); Wed, 14 Sep 2016 13:14:26 -0400 Subject: Re: [PATCH 5/6] fanotify: Use notification_lock instead of access_lock To: Jan Kara , Andrew Morton References: <1473797711-14111-1-git-send-email-jack@suse.cz> <1473797711-14111-6-git-send-email-jack@suse.cz> Cc: linux-fsdevel@vger.kernel.org, Miklos Szeredi , Eric Paris , Al Viro From: Lino Sanfilippo Message-ID: Date: Wed, 14 Sep 2016 19:14:11 +0200 MIME-Version: 1.0 In-Reply-To: <1473797711-14111-6-git-send-email-jack@suse.cz> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On 13.09.2016 22:15, Jan Kara wrote: > Fanotify code has an own lock (access_lock) to protect a list of events > waiting for a response from userspace. However this is somewhat awkward > as the same list_head in the event is protected by notification_lock > if it is part of the notification queue and by access_lock if it is part > of the fanotify private queue which makes it difficult for any reliable > checks in the generic code. So make fanotify use the same lock - > notification_lock - for protecting its private event list. > > Signed-off-by: Jan Kara > --- > fs/notify/fanotify/fanotify_user.c | 13 +++++-------- > include/linux/fsnotify_backend.h | 1 - > 2 files changed, 5 insertions(+), 9 deletions(-) > > diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c > index e6f3fe9bb2ed..aed06fe2533d 100644 > --- a/fs/notify/fanotify/fanotify_user.c > +++ b/fs/notify/fanotify/fanotify_user.c > @@ -147,7 +147,7 @@ static struct fanotify_perm_event_info *dequeue_event( > { > struct fanotify_perm_event_info *event, *return_e = NULL; > > - spin_lock(&group->fanotify_data.access_lock); > + spin_lock(&group->notification_lock); > list_for_each_entry(event, &group->fanotify_data.access_list, > fae.fse.list) { > if (event->fd != fd) > @@ -157,7 +157,7 @@ static struct fanotify_perm_event_info *dequeue_event( > return_e = event; > break; > } > - spin_unlock(&group->fanotify_data.access_lock); > + spin_unlock(&group->notification_lock); > > pr_debug("%s: found return_re=%p\n", __func__, return_e); > > @@ -309,10 +309,10 @@ static ssize_t fanotify_read(struct file *file, char __user *buf, > wake_up(&group->fanotify_data.access_waitq); > break; > } > - spin_lock(&group->fanotify_data.access_lock); > + spin_lock(&group->notification_lock); > list_add_tail(&kevent->list, > &group->fanotify_data.access_list); > - spin_unlock(&group->fanotify_data.access_lock); > + spin_unlock(&group->notification_lock); > #endif > } > buf += ret; > @@ -371,7 +371,7 @@ static int fanotify_release(struct inode *ignored, struct file *file) > * Process all permission events on access_list and notification queue > * and simulate reply from userspace. > */ > - spin_lock(&group->fanotify_data.access_lock); > + spin_lock(&group->notification_lock); > list_for_each_entry_safe(event, next, &group->fanotify_data.access_list, > fae.fse.list) { > pr_debug("%s: found group=%p event=%p\n", __func__, group, > @@ -380,14 +380,12 @@ static int fanotify_release(struct inode *ignored, struct file *file) > list_del_init(&event->fae.fse.list); > event->response = FAN_ALLOW; > } > - spin_unlock(&group->fanotify_data.access_lock); > > /* > * Destroy all non-permission events. For permission events just > * dequeue them and set the response. They will be freed once the > * response is consumed and fanotify_get_response() returns. > */ > - spin_lock(&group->notification_lock); > while (!fsnotify_notify_queue_is_empty(group)) { > fsn_event = fsnotify_remove_first_event(group); > if (!(fsn_event->mask & FAN_ALL_PERM_EVENTS)) { > @@ -767,7 +765,6 @@ SYSCALL_DEFINE2(fanotify_init, unsigned int, flags, unsigned int, event_f_flags) > event_f_flags |= O_LARGEFILE; > group->fanotify_data.f_flags = event_f_flags; > #ifdef CONFIG_FANOTIFY_ACCESS_PERMISSIONS > - spin_lock_init(&group->fanotify_data.access_lock); > init_waitqueue_head(&group->fanotify_data.access_waitq); > INIT_LIST_HEAD(&group->fanotify_data.access_list); > #endif > diff --git a/include/linux/fsnotify_backend.h b/include/linux/fsnotify_backend.h > index 0713e873b1c9..79467b239fcf 100644 > --- a/include/linux/fsnotify_backend.h > +++ b/include/linux/fsnotify_backend.h > @@ -177,7 +177,6 @@ struct fsnotify_group { > struct fanotify_group_private_data { > #ifdef CONFIG_FANOTIFY_ACCESS_PERMISSIONS > /* allows a group to block waiting for a userspace response */ > - spinlock_t access_lock; > struct list_head access_list; > wait_queue_head_t access_waitq; > #endif /* CONFIG_FANOTIFY_ACCESS_PERMISSIONS */ > Reviewed-by: Lino Sanfilippo