From: Richard Guy Briggs <rgb@redhat.com>
To: Jan Kara <jack@suse.cz>
Cc: Paul Moore <paul@paul-moore.com>,
Al Viro <viro@ZenIV.linux.org.uk>,
linux-audit@redhat.com, linux-fsdevel@vger.kernel.org,
Amir Goldstein <amir73il@gmail.com>
Subject: Re: [PATCH 11/11] audit: Use 'mark' name for fsnotify_mark variables
Date: Fri, 14 Sep 2018 14:29:45 -0400 [thread overview]
Message-ID: <20180914182945.oxlmb6gednsza4wg@madcap2.tricolour.ca> (raw)
In-Reply-To: <20180904160632.21210-12-jack@suse.cz>
On 2018-09-04 18:06, Jan Kara wrote:
> Variables pointing to fsnotify_mark are sometimes called 'entry' and
> sometimes 'mark'. Use 'mark' in all places.
Thank you, thank you, thank you! This would have been great at the
beginning of the patchset!
> Signed-off-by: Jan Kara <jack@suse.cz>
> ---
> kernel/audit_tree.c | 95 +++++++++++++++++++++++++++--------------------------
> 1 file changed, 48 insertions(+), 47 deletions(-)
>
> diff --git a/kernel/audit_tree.c b/kernel/audit_tree.c
> index ef109000ed01..9c53f7c37bdf 100644
> --- a/kernel/audit_tree.c
> +++ b/kernel/audit_tree.c
> @@ -158,9 +158,9 @@ static void audit_mark_put_chunk(struct audit_chunk *chunk)
> call_rcu(&chunk->head, __put_chunk);
> }
>
> -static inline struct audit_tree_mark *audit_mark(struct fsnotify_mark *entry)
> +static inline struct audit_tree_mark *audit_mark(struct fsnotify_mark *mark)
> {
> - return container_of(entry, struct audit_tree_mark, mark);
> + return container_of(mark, struct audit_tree_mark, mark);
> }
>
> static struct audit_chunk *mark_chunk(struct fsnotify_mark *mark)
> @@ -168,9 +168,9 @@ 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)
> +static void audit_tree_destroy_watch(struct fsnotify_mark *mark)
> {
> - kmem_cache_free(audit_tree_mark_cachep, audit_mark(entry));
> + kmem_cache_free(audit_tree_mark_cachep, audit_mark(mark));
> }
>
> static struct fsnotify_mark *alloc_mark(void)
> @@ -224,7 +224,7 @@ static inline struct list_head *chunk_hash(unsigned long key)
> return chunk_hash_heads + n % HASH_SIZE;
> }
>
> -/* hash_lock & entry->group->mark_mutex is held by caller */
> +/* hash_lock & mark->group->mark_mutex is held by caller */
> static void insert_hash(struct audit_chunk *chunk)
> {
> struct list_head *list;
> @@ -278,16 +278,16 @@ static struct audit_chunk *find_chunk(struct node *p)
> return container_of(p, struct audit_chunk, owners[0]);
> }
>
> -static void replace_mark_chunk(struct fsnotify_mark *entry,
> +static void replace_mark_chunk(struct fsnotify_mark *mark,
> struct audit_chunk *chunk)
> {
> struct audit_chunk *old;
>
> assert_spin_locked(&hash_lock);
> - old = mark_chunk(entry);
> - audit_mark(entry)->chunk = chunk;
> + old = mark_chunk(mark);
> + audit_mark(mark)->chunk = chunk;
> if (chunk)
> - chunk->mark = entry;
> + chunk->mark = mark;
> if (old)
> old->mark = NULL;
> }
> @@ -328,30 +328,30 @@ static void replace_chunk(struct audit_chunk *new, struct audit_chunk *old,
> static void untag_chunk(struct node *p)
> {
> struct audit_chunk *chunk = find_chunk(p);
> - struct fsnotify_mark *entry = chunk->mark;
> + struct fsnotify_mark *mark = chunk->mark;
> struct audit_chunk *new = NULL;
> struct audit_tree *owner;
> int size = chunk->count - 1;
>
> /* Racing with audit_tree_freeing_mark()? */
> - if (!entry)
> + if (!mark)
> return;
>
> - fsnotify_get_mark(entry);
> + fsnotify_get_mark(mark);
>
> spin_unlock(&hash_lock);
>
> if (size)
> new = alloc_chunk(size);
>
> - mutex_lock(&entry->group->mark_mutex);
> + mutex_lock(&mark->group->mark_mutex);
> /*
> * mark_mutex stabilizes chunk attached to the mark so we can check
> * whether it didn't change while we've dropped hash_lock.
> */
> - if (!(entry->flags & FSNOTIFY_MARK_FLAG_ATTACHED) ||
> - mark_chunk(entry) != chunk) {
> - mutex_unlock(&entry->group->mark_mutex);
> + if (!(mark->flags & FSNOTIFY_MARK_FLAG_ATTACHED) ||
> + mark_chunk(mark) != chunk) {
> + mutex_unlock(&mark->group->mark_mutex);
> kfree(new);
> goto out;
> }
> @@ -365,12 +365,12 @@ static void untag_chunk(struct node *p)
> owner->root = NULL;
> list_del_init(&p->list);
> list_del_rcu(&chunk->hash);
> - replace_mark_chunk(entry, NULL);
> + replace_mark_chunk(mark, NULL);
> spin_unlock(&hash_lock);
> - fsnotify_detach_mark(entry);
> - mutex_unlock(&entry->group->mark_mutex);
> + fsnotify_detach_mark(mark);
> + mutex_unlock(&mark->group->mark_mutex);
> audit_mark_put_chunk(chunk);
> - fsnotify_free_mark(entry);
> + fsnotify_free_mark(mark);
> goto out;
> }
>
> @@ -389,7 +389,7 @@ static void untag_chunk(struct node *p)
> */
> replace_chunk(new, chunk, p);
> spin_unlock(&hash_lock);
> - mutex_unlock(&entry->group->mark_mutex);
> + mutex_unlock(&mark->group->mark_mutex);
> audit_mark_put_chunk(chunk);
> goto out;
>
> @@ -404,16 +404,16 @@ static void untag_chunk(struct node *p)
> p->owner = NULL;
> put_tree(owner);
> spin_unlock(&hash_lock);
> - mutex_unlock(&entry->group->mark_mutex);
> + mutex_unlock(&mark->group->mark_mutex);
> out:
> - fsnotify_put_mark(entry);
> + fsnotify_put_mark(mark);
> spin_lock(&hash_lock);
> }
>
> /* Call with group->mark_mutex held, releases it */
> static int create_chunk(struct inode *inode, struct audit_tree *tree)
> {
> - struct fsnotify_mark *entry;
> + struct fsnotify_mark *mark;
> struct audit_chunk *chunk = alloc_chunk(1);
>
> if (!chunk) {
> @@ -421,16 +421,16 @@ static int create_chunk(struct inode *inode, struct audit_tree *tree)
> return -ENOMEM;
> }
>
> - entry = alloc_mark();
> - if (!entry) {
> + mark = alloc_mark();
> + if (!mark) {
> mutex_unlock(&audit_tree_group->mark_mutex);
> kfree(chunk);
> return -ENOMEM;
> }
>
> - if (fsnotify_add_inode_mark_locked(entry, inode, 0)) {
> + if (fsnotify_add_inode_mark_locked(mark, inode, 0)) {
> mutex_unlock(&audit_tree_group->mark_mutex);
> - fsnotify_put_mark(entry);
> + fsnotify_put_mark(mark);
> kfree(chunk);
> return -ENOSPC;
> }
> @@ -438,14 +438,14 @@ static int create_chunk(struct inode *inode, struct audit_tree *tree)
> spin_lock(&hash_lock);
> if (tree->goner) {
> spin_unlock(&hash_lock);
> - fsnotify_detach_mark(entry);
> + fsnotify_detach_mark(mark);
> mutex_unlock(&audit_tree_group->mark_mutex);
> - fsnotify_free_mark(entry);
> - fsnotify_put_mark(entry);
> + fsnotify_free_mark(mark);
> + fsnotify_put_mark(mark);
> kfree(chunk);
> return 0;
> }
> - replace_mark_chunk(entry, chunk);
> + replace_mark_chunk(mark, chunk);
> chunk->owners[0].index = (1U << 31);
> chunk->owners[0].owner = tree;
> get_tree(tree);
> @@ -467,21 +467,21 @@ static int create_chunk(struct inode *inode, struct audit_tree *tree)
> * we get notification through ->freeing_mark callback and cleanup
> * chunk pointing to this mark.
> */
> - fsnotify_put_mark(entry);
> + fsnotify_put_mark(mark);
> return 0;
> }
>
> /* the first tagged inode becomes root of tree */
> static int tag_chunk(struct inode *inode, struct audit_tree *tree)
> {
> - struct fsnotify_mark *entry;
> + struct fsnotify_mark *mark;
> struct audit_chunk *chunk, *old;
> struct node *p;
> int n;
>
> mutex_lock(&audit_tree_group->mark_mutex);
> - entry = fsnotify_find_mark(&inode->i_fsnotify_marks, audit_tree_group);
> - if (!entry)
> + mark = fsnotify_find_mark(&inode->i_fsnotify_marks, audit_tree_group);
> + if (!mark)
> return create_chunk(inode, tree);
>
> /*
> @@ -491,12 +491,12 @@ static int tag_chunk(struct inode *inode, struct audit_tree *tree)
> */
> /* are we already there? */
> spin_lock(&hash_lock);
> - old = mark_chunk(entry);
> + old = mark_chunk(mark);
> for (n = 0; n < old->count; n++) {
> if (old->owners[n].owner == tree) {
> spin_unlock(&hash_lock);
> mutex_unlock(&audit_tree_group->mark_mutex);
> - fsnotify_put_mark(entry);
> + fsnotify_put_mark(mark);
> return 0;
> }
> }
> @@ -505,7 +505,7 @@ static int tag_chunk(struct inode *inode, struct audit_tree *tree)
> chunk = alloc_chunk(old->count + 1);
> if (!chunk) {
> mutex_unlock(&audit_tree_group->mark_mutex);
> - fsnotify_put_mark(entry);
> + fsnotify_put_mark(mark);
> return -ENOMEM;
> }
>
> @@ -513,7 +513,7 @@ static int tag_chunk(struct inode *inode, struct audit_tree *tree)
> if (tree->goner) {
> spin_unlock(&hash_lock);
> mutex_unlock(&audit_tree_group->mark_mutex);
> - fsnotify_put_mark(entry);
> + fsnotify_put_mark(mark);
> kfree(chunk);
> return 0;
> }
> @@ -533,7 +533,7 @@ static int tag_chunk(struct inode *inode, struct audit_tree *tree)
> replace_chunk(chunk, old, NULL);
> spin_unlock(&hash_lock);
> mutex_unlock(&audit_tree_group->mark_mutex);
> - fsnotify_put_mark(entry); /* pair to fsnotify_find mark_entry */
> + fsnotify_put_mark(mark); /* pair to fsnotify_find_mark */
> audit_mark_put_chunk(old);
>
> return 0;
> @@ -1040,16 +1040,17 @@ static int audit_tree_handle_event(struct fsnotify_group *group,
> return 0;
> }
>
> -static void audit_tree_freeing_mark(struct fsnotify_mark *entry, struct fsnotify_group *group)
> +static void audit_tree_freeing_mark(struct fsnotify_mark *mark,
> + struct fsnotify_group *group)
> {
> struct audit_chunk *chunk;
>
> - mutex_lock(&entry->group->mark_mutex);
> + mutex_lock(&mark->group->mark_mutex);
> spin_lock(&hash_lock);
> - chunk = mark_chunk(entry);
> - replace_mark_chunk(entry, NULL);
> + chunk = mark_chunk(mark);
> + replace_mark_chunk(mark, NULL);
> spin_unlock(&hash_lock);
> - mutex_unlock(&entry->group->mark_mutex);
> + mutex_unlock(&mark->group->mark_mutex);
> if (chunk) {
> evict_chunk(chunk);
> audit_mark_put_chunk(chunk);
> @@ -1059,7 +1060,7 @@ static void audit_tree_freeing_mark(struct fsnotify_mark *entry, struct fsnotify
> * We are guaranteed to have at least one reference to the mark from
> * either the inode or the caller of fsnotify_destroy_mark().
> */
> - BUG_ON(refcount_read(&entry->refcnt) < 1);
> + BUG_ON(refcount_read(&mark->refcnt) < 1);
> }
>
> static const struct fsnotify_ops audit_tree_ops = {
> --
> 2.16.4
>
- RGB
--
Richard Guy Briggs <rgb@redhat.com>
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635
next prev parent reply other threads:[~2018-09-14 23:50 UTC|newest]
Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-09-04 16:06 [PATCH 0/11 v3] audit: Fix various races when tagging and untagging mounts Jan Kara
2018-09-04 16:06 ` [PATCH 01/11] audit_tree: Remove mark->lock locking Jan Kara
2018-09-04 16:06 ` [PATCH 02/11] audit: Fix possible spurious -ENOSPC error Jan Kara
2018-09-04 16:06 ` [PATCH 03/11] audit: Fix possible tagging failures Jan Kara
2018-09-04 16:06 ` [PATCH 04/11] audit: Embed key into chunk Jan Kara
2018-09-13 20:06 ` Richard Guy Briggs
2018-09-04 16:06 ` [PATCH 05/11] audit: Make hash table insertion safe against concurrent lookups Jan Kara
2018-09-04 16:06 ` [PATCH 06/11] audit: Factor out chunk replacement code Jan Kara
2018-09-04 16:06 ` [PATCH 07/11] audit: Remove pointless check in insert_hash() Jan Kara
2018-09-04 16:06 ` [PATCH 08/11] audit: Provide helper for dropping mark's chunk reference Jan Kara
2018-09-04 16:06 ` [PATCH 09/11] audit: Allocate fsnotify mark independently of chunk Jan Kara
2018-09-14 14:09 ` Richard Guy Briggs
2018-09-17 16:46 ` Jan Kara
2018-10-03 22:11 ` Paul Moore
2018-10-03 22:08 ` Paul Moore
2018-10-03 22:39 ` Richard Guy Briggs
2018-10-04 6:57 ` Jan Kara
2018-09-04 16:06 ` [PATCH 10/11] audit: Replace chunk attached to mark instead of replacing mark Jan Kara
2018-09-04 16:06 ` [PATCH 11/11] audit: Use 'mark' name for fsnotify_mark variables Jan Kara
2018-09-14 18:29 ` Richard Guy Briggs [this message]
2018-09-17 16:44 ` Jan Kara
2018-09-17 18:13 ` Richard Guy Briggs
2018-09-04 16:06 ` [PATCH 12/11 TESTSUITE] audit_testsuite: Add stress test for tree watches Jan Kara
2018-09-14 18:21 ` Richard Guy Briggs
2018-09-17 16:56 ` Jan Kara
2018-10-05 21:06 ` Paul Moore
2018-10-09 7:40 ` Jan Kara
2018-10-10 6:43 ` Paul Moore
2018-10-11 11:39 ` Jan Kara
2018-10-11 23:03 ` Paul Moore
2018-10-15 10:04 ` Jan Kara
2018-10-15 15:39 ` Paul Moore
2018-10-17 10:09 ` Jan Kara
2018-11-14 0:34 ` Paul Moore
2018-11-14 12:16 ` Jan Kara
2018-11-19 15:19 ` Paul Moore
2018-09-14 19:13 ` [PATCH 0/11 v3] audit: Fix various races when tagging and untagging mounts Richard Guy Briggs
2018-09-17 16:57 ` Jan Kara
2018-10-04 1:20 ` Paul Moore
2018-10-04 6:59 ` 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=20180914182945.oxlmb6gednsza4wg@madcap2.tricolour.ca \
--to=rgb@redhat.com \
--cc=amir73il@gmail.com \
--cc=jack@suse.cz \
--cc=linux-audit@redhat.com \
--cc=linux-fsdevel@vger.kernel.org \
--cc=paul@paul-moore.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).