From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx2.suse.de ([195.135.220.15]:47611 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751592AbcAERIu (ORCPT ); Tue, 5 Jan 2016 12:08:50 -0500 Date: Tue, 5 Jan 2016 18:08:58 +0100 From: Jan Kara To: Al Viro Cc: Zhihui Zhang , Chris Mason , Theodore Ts'o , swhiteho@redhat.com, mfasheh@suse.com, linux-fsdevel@vger.kernel.org, Andrew Morton Subject: Re: [PATCH] Remove I_WILL_FREE Message-ID: <20160105170858.GC18604@quack.suse.cz> References: <1451699526-28608-1-git-send-email-zzhsuny@gmail.com> <20160102023105.GH9938@ZenIV.linux.org.uk> <20160102051810.GI9938@ZenIV.linux.org.uk> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160102051810.GI9938@ZenIV.linux.org.uk> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Sat 02-01-16 05:18:11, Al Viro wrote: > [akpm Cc'd] > On Fri, Jan 01, 2016 at 10:12:54PM -0500, Zhihui Zhang wrote: > > You are right, I was thinking from the perspective of I_WILL_FREE. > > > > However, for the examples in fs-writeback.c and a few in > > ext4/btrfs/inode.c, we can argue that they really should check > > I_WILL_FREE as well. In theory, bad things can happen if they don't > > because as soon as I_WILL_FREE is set, the inode is going to be > > evicted. For example, in fs-writeback.c: > > > > 471 spin_lock(&inode->i_lock); > > 472 if (inode->i_state & (I_WB_SWITCH | I_FREEING) || > > > > <-- Assume I_WILL_FREE is set > > at this point. > > > > 473 inode_to_wb(inode) == isw->new_wb) { > > 474 spin_unlock(&inode->i_lock); > > 475 goto out_free; > > 476 } > > 477 inode->i_state |= I_WB_SWITCH; > > 478 spin_unlock(&inode->i_lock); > > 479 > > 480 ihold(inode); <-- This will cause a warning because of i_count. > > Hmm... That ihold() is actually a lot more recent than original > introduction of I_WILL_FREE, but looking at the state of the tree > back when it was originally introduced... I'm trying to recall > what made us go for a separate flag, but so far I've got nothing > definite. Hell knows - it had been 10 years ago, and I have a gap > from late 2004 to September 2005 in mailboxes, so those are no help > either... I _think_ it got discussed with akpm, maybe he would be > able to help reconstructing what happened. > > It looks like you are right regarding the current state of the tree, but > I really wonder if there's something subtle that got missed during > one of rewrites in those ten years... OTOH, it's quite possible that there > had been no good reason for using a separate flag from the very beginning. Just for record, I don't see a reason for distinguishing between I_FREEING and I_WILL_FREE either. There could have been some difference back then when pdflush was still grabbing inode references to writeback inodes and there was no other explicit writeback barrier when evicting inodes (these days we have inode_wait_for_writeback() in evict()). But it's all just guessing. Honza -- Jan Kara SUSE Labs, CR