From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx2.suse.de ([195.135.220.15]:60015 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752208AbdAaNgJ (ORCPT ); Tue, 31 Jan 2017 08:36:09 -0500 Date: Tue, 31 Jan 2017 14:28:49 +0100 From: Jan Kara To: Miklos Szeredi Cc: Jan Kara , linux-fsdevel@vger.kernel.org, Amir Goldstein , Paul Moore Subject: Re: [PATCH 13/22] fanotify: Release SRCU lock when waiting for userspace response Message-ID: <20170131132849.GA14556@quack2.suse.cz> References: <20170120132123.9670-1-jack@suse.cz> <20170120132123.9670-14-jack@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 Wed 25-01-17 16:22:12, Miklos Szeredi wrote: > On Fri, Jan 20, 2017 at 2:21 PM, Jan Kara wrote: > > When userspace task processing fanotify permission events screws up and > > does not respond, fsnotify_mark_srcu SRCU is held indefinitely which > > causes further hangs in the whole notification subsystem. Although we > > cannot easily solve the problem of operations blocked waiting for > > response from userspace, we can at least somewhat localize the damage by > > dropping SRCU lock before waiting for userspace response and reacquiring > > it when userspace responds. > > > > Reviewed-by: Amir Goldstein > > Signed-off-by: Jan Kara > > --- > > fs/notify/fanotify/fanotify.c | 19 +++++++++++++++++-- > > 1 file changed, 17 insertions(+), 2 deletions(-) > > > > diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c > > index 2e8ca885fb3e..98d7dc94d34c 100644 > > --- a/fs/notify/fanotify/fanotify.c > > +++ b/fs/notify/fanotify/fanotify.c > > @@ -61,14 +61,28 @@ static int fanotify_merge(struct list_head *list, struct fsnotify_event *event) > > > > #ifdef CONFIG_FANOTIFY_ACCESS_PERMISSIONS > > static int fanotify_get_response(struct fsnotify_group *group, > > - struct fanotify_perm_event_info *event) > > + struct fsnotify_mark *inode_mark, > > + struct fsnotify_mark *vfsmount_mark, > > + struct fanotify_perm_event_info *event, > > + int *srcu_idx) > > Should these (inode_mark, vfsmount_mark, srcu_idx) be passed in an > opaque struct to simplify the interface? After some thought, having an opaque struct containing also mark pointers and pass it around instead of srcu_idx looks like a neat idea. We will have mark pointers in two places (explicit arguments of ->handle_event and the opaque struct) but that should not be a problem as notification frameworks cannot play any tricks with them anyway as it would break iteration in the generic notification core. I'll do this, thanks for the idea. Honza -- Jan Kara SUSE Labs, CR