Re: [PATCH 09/11] audit: Allocate fsnotify mark independently of chunk

[Paul Moore <paul@paul-moore.com>]

On Mon, Sep 17, 2018 at 12:46 PM Jan Kara <jack@suse.cz> 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 <jack@suse.cz>
> > > ---
> ...
> > > +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