Hi,
On Tue, 2003-04-15 at 16:12, Jan Kara wrote:
> I'm sending you a patch which fixes the deadlock when quota is used on
> ext3. The problem is that in sync_dquots() first dqio_sem is acquired
> and then transaction is started while in the other paths the order is
> inverse... The patch introduces needed framework into quota to allow
> ext3 start transaction before the semaphore is acquired. Please apply.
Please don't, there is at least one major and one minor problem to be
resolved.
> +#define EXT3_OLD_QFMT_BLOCKS 2
> +
> +static int ext3_sync_dquot(struct dquot *dquot)
> +{
> + int ret;
> + handle_t *handle;
> + struct inode *qinode;
> +
> + lock_kernel();
> + qinode = dquot->dq_sb->s_dquot.files[dquot->dq_type]->f_dentry->d_inode;
> + handle = ext3_journal_start(qinode, EXT3_OLD_QFMT_BLOCKS);
> + if (IS_ERR(handle)) {
> + unlock_kernel();
> + return PTR_ERR(handle);
> + }
> + unlock_kernel();
> + ret = old_sync_dquot(dquot);
> + lock_kernel();
> + ret = ext3_journal_stop(handle, qinode);
> + unlock_kernel();
> + return ret;
> +}
> +#endif
First of all, EXT3_OLD_QFMT_BLOCKS is wrong. If you "chown" a file to a
new user, you'll potentially allocate new space for a new quota file
entry. That requires a ton of preallocation. At the minimum, you have
the inode update; the superblock update for the free block counts; the
group descriptor update, likewise; the bitmap update; possibly one or
more indirect block updates; and (finally) the quota block itself.
You _will_ get buffer-credit assert failures with this value.
Secondly, you're now effectively calling generic_file_write with a
transaction already open. That's _another_ lock ranking violation.
generic_file_write() takes i_sem, and then opens a transaction. With
this new quota code, you can now open a transaction and then take
i_sem. Boom.
However, most users don't ever take i_sem on the quota inode in any
other way. Directly writing to, or truncating, the quota file is a
highly non-recommended activity. :-)
btw, I'm away all next week, so further responses will be slow...
Cheers,
Stephen
-
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/
This archive was generated by hypermail 2b29 : Wed Apr 23 2003 - 22:00:24 EST