Messages in this thread | | | Date | Sun, 20 Jun 2004 21:45:35 -0300 | From | Marcelo Tosatti <> | Subject | Re: inode_unused list corruption in 2.4.26 - spin_lock problem? |
| |
On Sat, Jun 19, 2004 at 11:33:55PM -0400, Trond Myklebust wrote: > På lau , 19/06/2004 klokka 20:15, skreiv Marcelo Tosatti: > > > The changes between 2.4.25->2.4.26 (which introduce __refile_inode() and > > the unused_pagecache list) must have something to do with this. > > Here's one question: > > Given the fact that in iput(), the inode remains hashed and the > inode->i_state does not change until after we've dropped the inode_lock, > called write_inode_now(), and then retaken the inode_lock, exactly what > is preventing a third party task from grabbing that inode? > > (Better still: write_inode_now() itself actually calls __iget(), which > could cause that inode to be plonked right back onto the "inode_in_use" > list if ever refile_inode() gets called.)
Lets see if I get this right, while we drop the lock in iput to call write_inode_now() an iget happens, possibly from write_inode_now itself (sync_one->__iget) causing the inode->i_list to be added to to inode_in_use.
But then the call returns, locks inode_lock, decreases inodes_stat.nr_unused-- and deletes the inode from the inode_in_use and adds to inode_unused.
AFAICS its an inode with i_count==1 in the unused list, which does not mean "list corruption", right? Am I missing something here?
If you are indeed right all 2.4.x versions contain this bug.
Thanks for helping!
> > So does the following patch help? > > +++ linux-2.4.27-pre3/fs/inode.c 2004-06-19 23:22:29.000000000 -0400 > @@ -1200,6 +1200,7 @@ void iput(struct inode *inode) > struct super_block *sb = inode->i_sb; > struct super_operations *op = NULL; > > +again: > if (inode->i_state == I_CLEAR) > BUG(); > > @@ -1241,11 +1242,16 @@ void iput(struct inode *inode) > if (!(inode->i_state & (I_DIRTY|I_LOCK))) > __refile_inode(inode); > inodes_stat.nr_unused++; > - spin_unlock(&inode_lock); > - if (!sb || (sb->s_flags & MS_ACTIVE)) > + if (!sb || (sb->s_flags & MS_ACTIVE)) { > + spin_unlock(&inode_lock); > return; > - write_inode_now(inode, 1); > - spin_lock(&inode_lock); > + } > + if (inode->i_state & I_DIRTY) { > + __iget(inode); > + spin_unlock(&inode_lock); > + write_inode_now(inode, 1); > + goto again; > + } > inodes_stat.nr_unused--; > list_del_init(&inode->i_hash); > } > > - > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |