Hi,
On Tue 15-02-11 11:29:16, Christoph Hellwig wrote:
> 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.
I just wonder: So with this change, evict_inodes() can start seeing
inodes, that are just being freed by prune_icache(). Thus we can trigger
WARN_ON() in evict_inodes():
if (inode->i_state & (I_NEW | I_FREEING | I_WILL_FREE)) {
WARN_ON(1);
continue;
}
Otherwise, the change looks safe to me. BTW, the iprune_sem is now used
only so that evict_inodes() can wait for prune_icache() to finish so maybe
we could have something simpler for that?
Honza
> >
> > Signed-off-by: Christoph Hellwig <hch@xxxxxx>
> >
> > 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---
> --
> 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
--
Jan Kara <jack@xxxxxxx>
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
