[06/10] audit: Factor out chunk replacement code
diff mbox

Message ID 20180710100217.12866-7-jack@suse.cz
State New
Headers show

Commit Message

Jan Kara July 10, 2018, 10:02 a.m. UTC
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>
---
 kernel/audit_tree.c | 86 +++++++++++++++++++++++++----------------------------
 1 file changed, 40 insertions(+), 46 deletions(-)

Comments

Amir Goldstein July 11, 2018, 7:58 a.m. UTC | #1
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,
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 July 11, 2018, 8:26 a.m. UTC | #2
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
> >
Amir Goldstein July 11, 2018, 9:01 a.m. UTC | #3
On Wed, Jul 11, 2018 at 11:26 AM, Jan Kara <jack@suse.cz> wrote:
> 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" ;)?
>

Don't feel confident enough with Reviewed-by/Acked-by on this series,
just letting you know that I eyeballed your patches FWIW ;-)

I'm down to the last one, which is agonizing to make sure nothing is missed
from the diff. At this point [10/10] I'd rather review the final code.

Can you push the work to a branch.

Thanks,
Amir.
Jan Kara July 11, 2018, 9:23 a.m. UTC | #4
On Wed 11-07-18 12:01:35, Amir Goldstein wrote:
> On Wed, Jul 11, 2018 at 11:26 AM, Jan Kara <jack@suse.cz> wrote:
> > 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" ;)?
> >
> 
> Don't feel confident enough with Reviewed-by/Acked-by on this series,
> just letting you know that I eyeballed your patches FWIW ;-)
> 
> I'm down to the last one, which is agonizing to make sure nothing is missed
> from the diff. At this point [10/10] I'd rather review the final code.

Yes, changing lifetime rules is almost always painful, especially in the
code you don't understand too well. It took me a day to understand audit
well enough to be confident to make that change...

> Can you push the work to a branch.

Sure, audit_cleanup branch pushed out to
  git://git.kernel.org/pub/scm/linux/kernel/git/jack/linux-fs.git

Thanks for your review!

								Honza
Paul Moore July 27, 2018, 4:47 a.m. UTC | #5
On Tue, Jul 10, 2018 at 6:02 AM 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>
> ---
>  kernel/audit_tree.c | 86 +++++++++++++++++++++++++----------------------------
>  1 file changed, 40 insertions(+), 46 deletions(-)

Ooof.  That code sucks.  Not your code Jan, rather the code you are
replacing.  Thanks for doing this.

I believe this looks right, although see my previous comment/question
about RCU/barriers/etc. that affects some of this code (inherited from
a previous patch in the patchset).

> 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

--
paul moore
www.paul-moore.com

Patch
diff mbox

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);