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 <jack@xxxxxxxx>
SUSE Labs, CR
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
