Re: [PATCH resend] introduce I_SYNC

From: Dave Kleikamp
Date: Thu May 31 2007 - 14:06:22 EST


On Thu, 2007-05-31 at 16:25 +0200, Jörn Engel wrote:
> --- linux-2.6.21logfs/fs/jfs/jfs_txnmgr.c~I_LOCK 2007-05-07
> 10:28:55.000000000 +0200
> +++ linux-2.6.21logfs/fs/jfs/jfs_txnmgr.c 2007-05-29
> 13:10:32.000000000 +0200
> @@ -1286,7 +1286,14 @@ int txCommit(tid_t tid, /*
> transaction
> * commit the transaction synchronously, so the last
> iput
> * will be done by the calling thread (or later)
> */
> - if (tblk->u.ip->i_state & I_LOCK)
> + /*
> + * I believe this code is no longer needed. Splitting
> I_LOCK
> + * into two bits, I_LOCK and I_SYNC should prevent
> this
> + * deadlock as well. But since I don't have a JFS
> testload
> + * to verify this, only a trivial s/I_LOCK/I_SYNC/ was
> done.
> + * Joern
> + */
> + if (tblk->u.ip->i_state & I_SYNC)
> tblk->xflag &= ~COMMIT_LAZY;
> }

I think the code is still needed, and I think this change is correct.
The deadlock that this code is avoiding is caused by clear_inode()
calling wait_on_inode(). Since clear_inode() now calls
inode_sync_wait(inode), we want to avoid the lazily committing this
transaction when the I_SYNC flag is set.

Unfortunately, recreating the deadlock is hard, and I haven't been able
to recreate it with this code commented out.

Thanks,
Shaggy
--
David Kleikamp
IBM Linux Technology Center

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/