Timothy Shimmin wrote:
Lachlan McIlroy wrote:
Lock debugging reported the ilock was being destroyed
without being unlocked.
--- a/fs/xfs/xfs_iget.c 2008-09-19 13:03:57.000000000 +1000
+++ b/fs/xfs/xfs_iget.c 2008-09-19 13:12:38.000000000 +1000
@@ -214,6 +214,7 @@ finish_inode:
xfs_ilock(ip, lock_flags);
if ((ip->i_d.di_mode == 0) && !(flags & XFS_IGET_CREATE)) {
+ xfs_iunlock(ip, lock_flags);
xfs_idestroy(ip);
xfs_put_perag(mp, pag);
return ENOENT;
@@ -224,6 +225,7 @@ finish_inode:
* write spinlock.
*/
if (radix_tree_preload(GFP_KERNEL)) {
+ xfs_iunlock(ip, lock_flags);
xfs_idestroy(ip);
delay(1);
goto again;
@@ -239,6 +241,7 @@ finish_inode:
BUG_ON(error != -EEXIST);
write_unlock(&pag->pag_ici_lock);
radix_tree_preload_end();
+ xfs_iunlock(ip, lock_flags);
xfs_idestroy(ip);
XFS_STATS_INC(xs_ig_dup);
goto again;
I'm just wondering about the case where lock_flags==0
and the inode is not locked.
I think it would fail an assert in xfs_iunlock().
Good catch Tim, thanks.
--Tim
if (lock_flags)
xfs_ilock(ip, lock_flags);
if ((ip->i_d.di_mode == 0) && !(flags & XFS_IGET_CREATE)) {
xfs_iunlock(ip, lock_flags);
xfs_idestroy(ip);
xfs_put_perag(mp, pag);
return ENOENT;
}
void
xfs_iunlock(
xfs_inode_t *ip,
uint lock_flags)
{
/*
* You can't set both SHARED and EXCL for the same lock,
* and only XFS_IOLOCK_SHARED, XFS_IOLOCK_EXCL, XFS_ILOCK_SHARED,
* and XFS_ILOCK_EXCL are valid values to set in lock_flags.
*/
ASSERT((lock_flags & (XFS_IOLOCK_SHARED | XFS_IOLOCK_EXCL)) !=
(XFS_IOLOCK_SHARED | XFS_IOLOCK_EXCL));
ASSERT((lock_flags & (XFS_ILOCK_SHARED | XFS_ILOCK_EXCL)) !=
(XFS_ILOCK_SHARED | XFS_ILOCK_EXCL));
ASSERT((lock_flags & ~(XFS_LOCK_MASK | XFS_IUNLOCK_NONOTIFY |
XFS_LOCK_DEP_MASK)) == 0);
ASSERT(lock_flags != 0);
|