From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Kara Subject: Re: [PATCH 01/10] audit_tree: Remove mark->lock locking Date: Tue, 4 Sep 2018 11:53:07 +0200 Message-ID: <20180904095307.GE9444@quack2.suse.cz> References: <20180710100217.12866-1-jack@suse.cz> <20180710100217.12866-2-jack@suse.cz> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-audit-bounces@redhat.com Errors-To: linux-audit-bounces@redhat.com To: Paul Moore Cc: jack@suse.cz, rgb@redhat.com, amir73il@gmail.com, linux-audit@redhat.com, viro@zeniv.linux.org.uk, linux-fsdevel@vger.kernel.org List-Id: linux-audit@redhat.com Hi, sorry for getting to this so late but I was catching up after vacation and your replies got burried in my inbox. On Fri 27-07-18 00:47:04, Paul Moore wrote: > On Tue, Jul 10, 2018 at 6:02 AM Jan Kara wrote: > > Currently, audit_tree code uses mark->lock to protect against detaching > > of mark from an inode. In most places it however also uses > > mark->group->mark_mutex (as we need to atomically replace attached > > marks) and this provides protection against mark detaching as well. So > > just remove protection with mark->lock from audit tree code and replace > > it with mark->group->mark_mutex protection in all the places. It > > simplifies the code and gets rid of some ugly catches like calling > > fsnotify_add_mark_locked() with mark->lock held (which cannot sleep only > > because we hold a reference to another mark attached to the same inode). > > > > Signed-off-by: Jan Kara > > --- > > kernel/audit_tree.c | 24 ++++-------------------- > > 1 file changed, 4 insertions(+), 20 deletions(-) > > ... > > > diff --git a/kernel/audit_tree.c b/kernel/audit_tree.c > > index 02feef939560..1c82eb6674c4 100644 > > --- a/kernel/audit_tree.c > > +++ b/kernel/audit_tree.c > > @@ -360,12 +355,12 @@ static int create_chunk(struct inode *inode, struct audit_tree *tree) > > return -ENOSPC; > > } > > > > - spin_lock(&entry->lock); > > + mutex_lock(&entry->group->mark_mutex); > > I wonder if we could move the lock up above the > fsnotify_add_inode_mark() earlier in create_chunk() and use > fsnotify_add_mark_locked()? Possibly, but I didn't want to do this in this patch as that's a separate change. Also this is what in fact happens in later patches. Honza -- Jan Kara SUSE Labs, CR