From: Jan Kara <jack@suse.cz>
To: Amir Goldstein <amir73il@gmail.com>
Cc: Jan Kara <jack@suse.cz>, Linux Audit <linux-audit@redhat.com>,
Paul Moore <paul@paul-moore.com>,
linux-fsdevel <linux-fsdevel@vger.kernel.org>,
Al Viro <viro@zeniv.linux.org.uk>,
Richard Guy Briggs <rgb@redhat.com>
Subject: Re: [PATCH 06/10] audit: Factor out chunk replacement code
Date: Wed, 11 Jul 2018 10:26:38 +0200 [thread overview]
Message-ID: <20180711082638.lpi6nnrfkwmm6is4@quack2.suse.cz> (raw)
In-Reply-To: <CAOQ4uxhn3Wc_J+LEKTV6Y3N4_3rMOQee-dSKjhNHc+4CW0B46A@mail.gmail.com>
On Wed 11-07-18 10:58:24, Amir Goldstein wrote:
> On Tue, Jul 10, 2018 at 1:02 PM, Jan Kara <jack@suse.cz> wrote:
> > Chunk replacement code is very similar for the cases where we grow or
> > shrink chunk. Factor the code out into a common helper function.
> >
> > Signed-off-by: Jan Kara <jack@suse.cz>
>
> Ack. Nice re-factoring.
Thanks. BTW, I didn't add your Acked-by tags because it wasn't obvious to
me whether you meant them as an official tag-worthy statement. So do want
me to add the tags or do you prefer to stay "anonymous reviewer" ;)?
Honza
>
> Thanks,
> Amir.
>
> > ---
> > kernel/audit_tree.c | 86 +++++++++++++++++++++++++----------------------------
> > 1 file changed, 40 insertions(+), 46 deletions(-)
> >
> > diff --git a/kernel/audit_tree.c b/kernel/audit_tree.c
> > index 6a01738d1ac2..f419fdfc25b4 100644
> > --- a/kernel/audit_tree.c
> > +++ b/kernel/audit_tree.c
> > @@ -235,6 +235,38 @@ static struct audit_chunk *find_chunk(struct node *p)
> > return container_of(p, struct audit_chunk, owners[0]);
> > }
> >
> > +static void replace_chunk(struct audit_chunk *new, struct audit_chunk *old,
> > + struct node *skip)
> > +{
> > + struct audit_tree *owner;
> > + int i, j;
> > +
> > + new->key = old->key;
> > + list_replace_init(&old->trees, &new->trees);
> > + list_for_each_entry(owner, &new->trees, same_root)
> > + owner->root = new;
> > + for (i = j = 0; j < old->count; i++, j++) {
> > + if (&old->owners[j] == skip) {
> > + i--;
> > + continue;
> > + }
> > + owner = old->owners[j].owner;
> > + new->owners[i].owner = owner;
> > + new->owners[i].index = old->owners[j].index - j + i;
> > + if (!owner) /* result of earlier fallback */
> > + continue;
> > + get_tree(owner);
> > + list_replace_init(&old->owners[j].list, &new->owners[i].list);
> > + }
> > + /*
> > + * Make sure chunk is fully initialized before making it visible in the
> > + * hash. Pairs with a data dependency barrier in READ_ONCE() in
> > + * audit_tree_lookup().
> > + */
> > + smp_wmb();
> > + list_replace_rcu(&old->hash, &new->hash);
> > +}
> > +
> > static void untag_chunk(struct node *p)
> > {
> > struct audit_chunk *chunk = find_chunk(p);
> > @@ -242,7 +274,6 @@ static void untag_chunk(struct node *p)
> > struct audit_chunk *new = NULL;
> > struct audit_tree *owner;
> > int size = chunk->count - 1;
> > - int i, j;
> >
> > fsnotify_get_mark(entry);
> >
> > @@ -291,38 +322,16 @@ static void untag_chunk(struct node *p)
> >
> > chunk->dead = 1;
> > spin_lock(&hash_lock);
> > - new->key = chunk->key;
> > - list_replace_init(&chunk->trees, &new->trees);
> > if (owner->root == chunk) {
> > list_del_init(&owner->same_root);
> > owner->root = NULL;
> > }
> > -
> > - for (i = j = 0; j <= size; i++, j++) {
> > - struct audit_tree *s;
> > - if (&chunk->owners[j] == p) {
> > - list_del_init(&p->list);
> > - i--;
> > - continue;
> > - }
> > - s = chunk->owners[j].owner;
> > - new->owners[i].owner = s;
> > - new->owners[i].index = chunk->owners[j].index - j + i;
> > - if (!s) /* result of earlier fallback */
> > - continue;
> > - get_tree(s);
> > - list_replace_init(&chunk->owners[j].list, &new->owners[i].list);
> > - }
> > -
> > - list_for_each_entry(owner, &new->trees, same_root)
> > - owner->root = new;
> > + list_del_init(&p->list);
> > /*
> > - * Make sure chunk is fully initialized before making it visible in the
> > - * hash. Pairs with a data dependency barrier in READ_ONCE() in
> > - * audit_tree_lookup().
> > + * This has to go last when updating chunk as once replace_chunk() is
> > + * called, new RCU readers can see the new chunk.
> > */
> > - smp_wmb();
> > - list_replace_rcu(&chunk->hash, &new->hash);
> > + replace_chunk(new, chunk, p);
> > spin_unlock(&hash_lock);
> > fsnotify_detach_mark(entry);
> > mutex_unlock(&entry->group->mark_mutex);
> > @@ -399,7 +408,6 @@ static int create_chunk(struct inode *inode, struct audit_tree *tree)
> > static int tag_chunk(struct inode *inode, struct audit_tree *tree)
> > {
> > struct fsnotify_mark *old_entry, *chunk_entry;
> > - struct audit_tree *owner;
> > struct audit_chunk *chunk, *old;
> > struct node *p;
> > int n;
> > @@ -464,35 +472,21 @@ static int tag_chunk(struct inode *inode, struct audit_tree *tree)
> > fsnotify_put_mark(old_entry);
> > return 0;
> > }
> > - chunk->key = old->key;
> > - list_replace_init(&old->trees, &chunk->trees);
> > - for (n = 0, p = chunk->owners; n < old->count; n++, p++) {
> > - struct audit_tree *s = old->owners[n].owner;
> > - p->owner = s;
> > - p->index = old->owners[n].index;
> > - if (!s) /* result of fallback in untag */
> > - continue;
> > - get_tree(s);
> > - list_replace_init(&old->owners[n].list, &p->list);
> > - }
> > + p = &chunk->owners[chunk->count - 1];
> > p->index = (chunk->count - 1) | (1U<<31);
> > p->owner = tree;
> > get_tree(tree);
> > list_add(&p->list, &tree->chunks);
> > - list_for_each_entry(owner, &chunk->trees, same_root)
> > - owner->root = chunk;
> > old->dead = 1;
> > if (!tree->root) {
> > tree->root = chunk;
> > list_add(&tree->same_root, &chunk->trees);
> > }
> > /*
> > - * Make sure chunk is fully initialized before making it visible in the
> > - * hash. Pairs with a data dependency barrier in READ_ONCE() in
> > - * audit_tree_lookup().
> > + * This has to go last when updating chunk as once replace_chunk() is
> > + * called, new RCU readers can see the new chunk.
> > */
> > - smp_wmb();
> > - list_replace_rcu(&old->hash, &chunk->hash);
> > + replace_chunk(chunk, old, NULL);
> > spin_unlock(&hash_lock);
> > fsnotify_detach_mark(old_entry);
> > mutex_unlock(&audit_tree_group->mark_mutex);
> > --
> > 2.16.4
> >
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
next prev parent reply other threads:[~2018-07-11 8:29 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-07-10 10:02 [PATCH 0/10 v2] audit: Fix various races when tagging and untagging mounts Jan Kara
2018-07-10 10:02 ` [PATCH 01/10] audit_tree: Remove mark->lock locking Jan Kara
2018-07-27 4:47 ` Paul Moore
2018-09-04 9:53 ` Jan Kara
2018-07-10 10:02 ` [PATCH 02/10] audit: Fix possible spurious -ENOSPC error Jan Kara
2018-07-27 4:47 ` Paul Moore
2018-09-04 10:00 ` Jan Kara
2018-07-10 10:02 ` [PATCH 03/10] audit: Fix possible tagging failures Jan Kara
2018-07-10 10:02 ` [PATCH 04/10] audit: Embed key into chunk Jan Kara
2018-07-27 4:47 ` Paul Moore
2018-07-10 10:02 ` [PATCH 05/10] audit: Make hash table insertion safe against concurrent lookups Jan Kara
2018-07-10 10:02 ` [PATCH 06/10] audit: Factor out chunk replacement code Jan Kara
2018-07-11 7:58 ` Amir Goldstein
2018-07-11 8:26 ` Jan Kara [this message]
2018-07-11 9:01 ` Amir Goldstein
2018-07-11 9:23 ` Jan Kara
2018-07-27 4:47 ` Paul Moore
2018-07-10 10:02 ` [PATCH 07/10] audit: Remove pointless check in insert_hash() Jan Kara
2018-07-27 4:47 ` Paul Moore
2018-07-10 10:02 ` [PATCH 08/10] audit: Provide helper for dropping mark's chunk reference Jan Kara
2018-07-10 10:02 ` [PATCH 09/10] audit: Allocate fsnotify mark independently of chunk Jan Kara
2018-07-11 8:57 ` Amir Goldstein
2018-07-11 10:48 ` Amir Goldstein
2018-07-16 15:13 ` Jan Kara
2018-07-27 4:47 ` Paul Moore
2018-09-04 14:03 ` Jan Kara
2018-09-04 14:07 ` Jan Kara
2018-07-10 10:02 ` [PATCH 10/10] audit: Replace chunk attached to mark instead of replacing mark Jan Kara
2018-07-11 14:17 ` Amir Goldstein
2018-07-27 4:47 ` Paul Moore
2018-09-04 14:11 ` Jan Kara
2018-07-10 10:02 ` [PATCH 11/10 TESTSUITE] audit_testsuite: Add stress test for tree watches Jan Kara
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180711082638.lpi6nnrfkwmm6is4@quack2.suse.cz \
--to=jack@suse.cz \
--cc=amir73il@gmail.com \
--cc=linux-audit@redhat.com \
--cc=linux-fsdevel@vger.kernel.org \
--cc=paul@paul-moore.com \
--cc=rgb@redhat.com \
--cc=viro@zeniv.linux.org.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).