From mboxrd@z Thu Jan 1 00:00:00 1970 From: Christoph Hellwig Subject: Re: [PATCH, RFC] prune back iprune_sem Date: Tue, 15 Feb 2011 11:29:16 +0100 Message-ID: <20110215102916.GA959@lst.de> References: <20101102184536.GA22941@lst.de> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: linux-fsdevel@vger.kernel.org To: viro@ZenIV.linux.org.uk, akpm@linux-foundation.org Return-path: Received: from verein.lst.de ([213.95.11.210]:41391 "EHLO verein.lst.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752960Ab1BOK3v (ORCPT ); Tue, 15 Feb 2011 05:29:51 -0500 Content-Disposition: inline In-Reply-To: <20101102184536.GA22941@lst.de> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: ping? On Tue, Nov 02, 2010 at 07:45:36PM +0100, Christoph Hellwig wrote: > iprune_sem is continously giving us lockdep warnings because we do take it in > read mode in the reclaim path, but we're also doing non-NOFS allocations under > it taken in write mode. > > Taking a bit deeper look at it I think it's fixable quite trivially: > > - for invalidate_inodes we do not need iprune_sem at all. We have an active > reference on the superblock, so the filesystem is not going away until it > has finished. > - for evict_inodes we do need it, to make sure prune_icache has done it's > work before we tear down the superblock. But there is no reason to > hold it over the actual reclaim operation - it's enough to cycle through > it after the actual reclaim to make sure we wait for any pending > prune_icache to complete. > > Signed-off-by: Christoph Hellwig > > diff --git a/fs/inode.c b/fs/inode.c > index ae2727a..cfa7722 100644 > --- a/fs/inode.c > +++ b/fs/inode.c > @@ -492,8 +492,6 @@ void evict_inodes(struct super_block *sb) > struct inode *inode, *next; > LIST_HEAD(dispose); > > - down_write(&iprune_sem); > - > spin_lock(&inode_lock); > list_for_each_entry_safe(inode, next, &sb->s_inodes, i_sb_list) { > if (atomic_read(&inode->i_count)) > @@ -518,6 +516,13 @@ void evict_inodes(struct super_block *sb) > spin_unlock(&inode_lock); > > dispose_list(&dispose); > + > + /* > + * Cycle through iprune_sem to make sure any inode that prune_icache > + * moved off the list before we took the lock has been fully torn > + * down. > + */ > + down_write(&iprune_sem); > up_write(&iprune_sem); > } > > @@ -534,8 +539,6 @@ int invalidate_inodes(struct super_block *sb) > struct inode *inode, *next; > LIST_HEAD(dispose); > > - down_write(&iprune_sem); > - > spin_lock(&inode_lock); > list_for_each_entry_safe(inode, next, &sb->s_inodes, i_sb_list) { > if (inode->i_state & (I_NEW | I_FREEING | I_WILL_FREE)) > @@ -559,7 +562,6 @@ int invalidate_inodes(struct super_block *sb) > spin_unlock(&inode_lock); > > dispose_list(&dispose); > - up_write(&iprune_sem); > > return busy; > } ---end quoted text---