From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lf1-f67.google.com ([209.85.167.67]:45462 "EHLO mail-lf1-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725723AbeJDFBa (ORCPT ); Thu, 4 Oct 2018 01:01:30 -0400 Received: by mail-lf1-f67.google.com with SMTP id m80-v6so5289982lfi.12 for ; Wed, 03 Oct 2018 15:11:14 -0700 (PDT) MIME-Version: 1.0 References: <20180904160632.21210-1-jack@suse.cz> <20180904160632.21210-10-jack@suse.cz> <20180914140909.4q77jy2nuqes2azt@madcap2.tricolour.ca> <20180917164623.GC10257@quack2.suse.cz> In-Reply-To: <20180917164623.GC10257@quack2.suse.cz> From: Paul Moore Date: Wed, 3 Oct 2018 18:11:02 -0400 Message-ID: Subject: Re: [PATCH 09/11] audit: Allocate fsnotify mark independently of chunk To: jack@suse.cz Cc: rgb@redhat.com, viro@zeniv.linux.org.uk, linux-audit@redhat.com, linux-fsdevel@vger.kernel.org, amir73il@gmail.com Content-Type: text/plain; charset="UTF-8" Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Mon, Sep 17, 2018 at 12:46 PM Jan Kara wrote: > On Fri 14-09-18 10:09:09, Richard Guy Briggs wrote: > > On 2018-09-04 18:06, Jan Kara wrote: > > > Allocate fsnotify mark independently instead of embedding it inside > > > chunk. This will allow us to just replace chunk attached to mark when > > > growing / shrinking chunk instead of replacing mark attached to inode > > > which is a more complex operation. > > > > > > Signed-off-by: Jan Kara > > > --- > ... > > > +static struct audit_chunk *mark_chunk(struct fsnotify_mark *mark) > > > +{ > > > + return audit_mark(mark)->chunk; > > > +} > > > + > > > static void audit_tree_destroy_watch(struct fsnotify_mark *entry) > > > { > > > - struct audit_chunk *chunk = container_of(entry, struct audit_chunk, mark); > > > + struct audit_chunk *chunk = mark_chunk(entry); > > > audit_mark_put_chunk(chunk); > > > + kmem_cache_free(audit_tree_mark_cachep, audit_mark(entry)); > > > +} > > > + > > > +static struct fsnotify_mark *alloc_mark(void) > > > +{ > > > + struct audit_tree_mark *mark; > > > > Would it make sense to call this local variable "amark" to indicate it > > isn't a struct fsnotify_mark, but in fact an audit helper variant? > > > > > + > > > + mark = kmem_cache_zalloc(audit_tree_mark_cachep, GFP_KERNEL); > > > + if (!mark) > > > + return NULL; > > > + fsnotify_init_mark(&mark->mark, audit_tree_group); > > > + mark->mark.mask = FS_IN_IGNORED; > > > + return &mark->mark; > > > > There are no other places where it is used in this patch to name a > > variable, but this one I found a bit confusing to follow the > > "mark->mark" > > Yeah, makes sense. I can do the change. Unless you have to respin this patchset for some other reason I wouldn't worry about it. I've been working my way through the patchset this week (currently on 09/11) and I expect to finish it up today. Assuming everything looks good, I'm going to merge this into a working branch, include it in my weekly -rc test builds, and beat on it for a couple of weeks. If all is good I'll merge it into audit/next after the upcoming merge window. Thanks for your patience. -- paul moore www.paul-moore.com From mboxrd@z Thu Jan 1 00:00:00 1970 From: Paul Moore Subject: Re: [PATCH 09/11] audit: Allocate fsnotify mark independently of chunk Date: Wed, 3 Oct 2018 18:11:02 -0400 Message-ID: References: <20180904160632.21210-1-jack@suse.cz> <20180904160632.21210-10-jack@suse.cz> <20180914140909.4q77jy2nuqes2azt@madcap2.tricolour.ca> <20180917164623.GC10257@quack2.suse.cz> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mx1.redhat.com (ext-mx01.extmail.prod.ext.phx2.redhat.com [10.5.110.25]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 3BA7D60F97 for ; Wed, 3 Oct 2018 22:11:16 +0000 (UTC) Received: from mail-lf1-f66.google.com (mail-lf1-f66.google.com [209.85.167.66]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 5EB7B85363 for ; Wed, 3 Oct 2018 22:11:15 +0000 (UTC) Received: by mail-lf1-f66.google.com with SMTP id o21-v6so5324085lfe.0 for ; Wed, 03 Oct 2018 15:11:15 -0700 (PDT) In-Reply-To: <20180917164623.GC10257@quack2.suse.cz> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-audit-bounces@redhat.com Errors-To: linux-audit-bounces@redhat.com To: jack@suse.cz Cc: rgb@redhat.com, linux-fsdevel@vger.kernel.org, linux-audit@redhat.com, amir73il@gmail.com, viro@zeniv.linux.org.uk List-Id: linux-audit@redhat.com On Mon, Sep 17, 2018 at 12:46 PM Jan Kara wrote: > On Fri 14-09-18 10:09:09, Richard Guy Briggs wrote: > > On 2018-09-04 18:06, Jan Kara wrote: > > > Allocate fsnotify mark independently instead of embedding it inside > > > chunk. This will allow us to just replace chunk attached to mark when > > > growing / shrinking chunk instead of replacing mark attached to inode > > > which is a more complex operation. > > > > > > Signed-off-by: Jan Kara > > > --- > ... > > > +static struct audit_chunk *mark_chunk(struct fsnotify_mark *mark) > > > +{ > > > + return audit_mark(mark)->chunk; > > > +} > > > + > > > static void audit_tree_destroy_watch(struct fsnotify_mark *entry) > > > { > > > - struct audit_chunk *chunk = container_of(entry, struct audit_chunk, mark); > > > + struct audit_chunk *chunk = mark_chunk(entry); > > > audit_mark_put_chunk(chunk); > > > + kmem_cache_free(audit_tree_mark_cachep, audit_mark(entry)); > > > +} > > > + > > > +static struct fsnotify_mark *alloc_mark(void) > > > +{ > > > + struct audit_tree_mark *mark; > > > > Would it make sense to call this local variable "amark" to indicate it > > isn't a struct fsnotify_mark, but in fact an audit helper variant? > > > > > + > > > + mark = kmem_cache_zalloc(audit_tree_mark_cachep, GFP_KERNEL); > > > + if (!mark) > > > + return NULL; > > > + fsnotify_init_mark(&mark->mark, audit_tree_group); > > > + mark->mark.mask = FS_IN_IGNORED; > > > + return &mark->mark; > > > > There are no other places where it is used in this patch to name a > > variable, but this one I found a bit confusing to follow the > > "mark->mark" > > Yeah, makes sense. I can do the change. Unless you have to respin this patchset for some other reason I wouldn't worry about it. I've been working my way through the patchset this week (currently on 09/11) and I expect to finish it up today. Assuming everything looks good, I'm going to merge this into a working branch, include it in my weekly -rc test builds, and beat on it for a couple of weeks. If all is good I'll merge it into audit/next after the upcoming merge window. Thanks for your patience. -- paul moore www.paul-moore.com