From mboxrd@z Thu Jan 1 00:00:00 1970 From: Christoph Hellwig Subject: Re: [PATCH 1/4] fs: do not drop inode_lock in dispose_list Date: Mon, 25 Oct 2010 12:07:58 +0200 Message-ID: <20101025100758.GA32680@lst.de> References: <20101024174024.GA2718@lst.de> <20101025053353.GA16131@dastard> <20101025054611.GB16131@dastard> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Christoph Hellwig , viro@zeniv.linux.org.uk, linux-fsdevel@vger.kernel.org To: Dave Chinner Return-path: Received: from verein.lst.de ([213.95.11.210]:53523 "EHLO verein.lst.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751574Ab0JYKIH (ORCPT ); Mon, 25 Oct 2010 06:08:07 -0400 Content-Disposition: inline In-Reply-To: <20101025054611.GB16131@dastard> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Mon, Oct 25, 2010 at 04:46:11PM +1100, Dave Chinner wrote: > Not sure whether it's my merge, something in Al's tree series or > something brought in from the post-2.6.36 tree, but it goes splat > pretty quickly (xfstest 013) with: That's because the version of the separate wb and lru lists patch that Al took from your tree was missing various deletions from the wb list. Below is the fix I sent him, not sure why it didn't make it to the list: Index: linux-2.6/fs/inode.c =================================================================== --- linux-2.6.orig/fs/inode.c 2010-10-24 16:30:07.592253652 +0200 +++ linux-2.6/fs/inode.c 2010-10-24 16:31:31.132003756 +0200 @@ -509,8 +509,14 @@ static int invalidate_list(struct list_h continue; } - list_move(&inode->i_lru, dispose); inode->i_state |= I_FREEING; + + /* + * Move the inode off the IO lists and LRU once I_FREEING is + * set so that it won't get moved back on there if it is dirty. + */ + list_move(&inode->i_lru, dispose); + list_del_init(&inode->i_wb_list); if (!(inode->i_state & (I_DIRTY | I_SYNC))) percpu_counter_dec(&nr_inodes_unused); } @@ -620,9 +626,15 @@ static void prune_icache(int nr_to_scan) if (!can_unuse(inode)) continue; } - list_move(&inode->i_lru, &freeable); WARN_ON(inode->i_state & I_NEW); inode->i_state |= I_FREEING; + + /* + * Move the inode off the IO lists and LRU once I_FREEING is + * set so that it won't get moved back on there if it is dirty. + */ + list_move(&inode->i_lru, &freeable); + list_del_init(&inode->i_wb_list); percpu_counter_dec(&nr_inodes_unused); } if (current_is_kswapd()) @@ -1343,16 +1355,16 @@ static void iput_final(struct inode *ino inode->i_state &= ~I_WILL_FREE; __remove_inode_hash(inode); } - list_del_init(&inode->i_wb_list); + WARN_ON(inode->i_state & I_NEW); inode->i_state |= I_FREEING; /* - * After we delete the inode from the LRU here, we avoid moving dirty - * inodes back onto the LRU now because I_FREEING is set and hence - * writeback_single_inode() won't move the inode around. + * Move the inode off the IO lists and LRU once I_FREEING is + * set so that it won't get moved back on there if it is dirty. */ inode_lru_list_del(inode); + list_del_init(&inode->i_wb_list); __inode_sb_list_del(inode); spin_unlock(&inode_lock);