Re: [PATCH 6/6] audit: Point to fsnotify mark instead of embedding it

On Thu, Jun 28, 2018 at 7:40 PM, Jan Kara <jack(a)suse.cz&gt; wrote: > Audit tree code currently embeds fsnotify mark in audit_chunk. As chunk > attached to an inode is replace when new tag is added / removed, we also > need to remove old fsnotify mark and add a new one on such occasion. > This is cumbersome and makes locking rules somewhat difficult to follow. > > Fix these problems by allocating fsnotify mark independently and keeping > it all the time while there is some chunk attached to an inode. Also add > documentation about the locking rules so that things are easier to > follow. > > Signed-off-by: Jan Kara <jack(a)suse.cz&gt; > --- Wow! this patch is a lot to take in at once. I wonder if it would be possible to split it to: - make mark not embedded and take chunk reference - replace_mark_chunk() and rid of cumbersome code

Or any other simplification that would help me survive this review. In the mean while just one nit below... [...] > + mutex_lock(&entry->group->mark_mutex); > + spin_lock(&hash_lock); > + chunk = AUDIT_M(entry)->chunk; > + replace_mark_chunk(entry, NULL); > + spin_unlock(&hash_lock); > + if (chunk) { > + mutex_unlock(&entry->group->mark_mutex); > + evict_chunk(chunk); > + audit_mark_put_chunk(chunk); > + } else { > + mutex_unlock(&entry->group->mark_mutex); > + } else case seems like a leftover or something.