From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Kara Subject: Re: [PATCH 06/14] audit: Factor out chunk replacement code Date: Mon, 12 Nov 2018 16:25:05 +0100 Message-ID: <20181112152505.GH7175@quack2.suse.cz> References: <20181017101505.25881-1-jack@suse.cz> <20181017101505.25881-7-jack@suse.cz> <20181018192701.w57xqyzzzqyx2og4@madcap2.tricolour.ca> <20181107095512.GA25261@quack2.suse.cz> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-audit-bounces@redhat.com Errors-To: linux-audit-bounces@redhat.com To: Paul Moore Cc: linux-audit@redhat.com, jack@suse.cz, amir73il@gmail.com, viro@zeniv.linux.org.uk List-Id: linux-audit@redhat.com On Mon 12-11-18 10:15:42, Paul Moore wrote: > On Fri, Nov 9, 2018 at 9:45 AM Paul Moore wrote: > > On Wed, Nov 7, 2018 at 4:55 AM Jan Kara wrote: > > > On Tue 06-11-18 08:58:36, Paul Moore wrote: > > > > On Thu, Oct 18, 2018 at 3:27 PM Richard Guy Briggs wrote: > > > > > On 2018-10-17 12:14, Jan Kara 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. > > > > > > > > > > Noting just the switch from list_replace_init() to list_splice_init(). > > > > > > > > Yeah, I wasn't expecting to see that, maybe it will make sense as I > > > > work through the rest of the patchset. > > > > > > > > Jan, can you explain the reason behind the change? I'm a little > > > > nervous that this is simply hiding a problem (e.g. the list_empty() > > > > check in splice). > > > > > > The reason is very simple: replace_chunk() gets called from tag_chunk() > > > *after* we have possibly done: > > > > > > if (!tree->root) { > > > tree->root = chunk; > > > list_add(&tree->same_root, &chunk->trees); > > > } > > > > > > So new->trees is possibly non-empty and we need to preserve its contents. > > > That's why we need list_splice() and not plain list_replace(). > > > > After revisiting this a couple of time this week, I found my problem - > > I had reversed list_splice_init() in my mind and was worried that if > > the old->trees list was not empty we would end up not adding > > new->trees. Sorry about the noise. > > > > I've updated the audit/working-fsnotify_fixes with the latest patches, > > including the revised 12/14 sent as an attachment, and I'm going to > > test it over the weekend. If the kernel is still standing on Monday > > morning I'll merge it into audit/next. > > > > Thanks again. > > My test machine was still standing this morning so I went ahead and > merged all of the patches into audit/next - Thanks Jan. Cool, thanks! Honza -- Jan Kara SUSE Labs, CR