Re: -- END OF BLOCK -- (fwd)

Re: -- END OF BLOCK -- (fwd)

[Christoph Hellwig]

On Mon, Jul 14, 2003 at 10:08:02AM -0300, Marcelo Tosatti wrote:
> The quota code you have in -ac is new Jan Kara's stuff (which supports
> both formats, etc) plus the ext3 deadlock avoidance and the compat stuff ?
> 
> Christoph, for what reason have you removed the ext3 deadlock avoidance
> patches? And also, what else have you changed wrt originals Jan code ?

Umm, I didn't not remove anything.  I backported Jan's code from 2.5
long ago and resynched with 2.5 from time to time.  I sent this code
to Alan and Andrea for -aa and -ac and now sent it to you with the
changes you requested (make the old quota format and userland ABI
compiled in unconditionally.)

Now it seems Alan merged other stuff without telling the person
who send him the original patch, thus -ac seems to have code the
XFS tree and -aa don't have.

I've now extracted the ext3 quota changes from -ac for you.  Note
that their only relation of them to the new quota code is that
the patch is ontop of it and thus takes it into account, the problem
is an old one.

Here's the patch, probably the last 2.4 patch from me if we really
want to play the hot potatoe game.

> Lets have -pre6 soon to fix up this mess (which is causing DEADLOCKS), ok?

s/cause/not fix existing/


p.s. yes, the patch is more ugly than anything I'd usually submit.
but it's not mine and I don't have any interest in it.  merge it asis
or clean it up yourself - I don't care anymore.

--- 1.19/fs/dquot.c	Mon Jul  7 12:44:36 2003
+++ edited/fs/dquot.c	Mon Jul 14 05:52:09 2003
@@ -395,7 +395,7 @@
 		if (dquot->dq_flags & DQ_LOCKED)
 			wait_on_dquot(dquot);
 		if (dquot_dirty(dquot))
-			commit_dqblk(dquot);
+			sb->dq_op->sync_dquot(dquot);
 		dqput(dquot);
 		goto restart;
 	}
@@ -1219,9 +1219,16 @@
 	alloc_inode:	dquot_alloc_inode,
 	free_space:	dquot_free_space,
 	free_inode:	dquot_free_inode,
-	transfer:	dquot_transfer
+	transfer:	dquot_transfer,
+	sync_dquot:	commit_dqblk
 };
 
+/* Function used by filesystems for initializing the dquot_operations structure */
+void init_dquot_operations(struct dquot_operations *fsdqops)
+{
+	memcpy(fsdqops, &dquot_operations, sizeof(dquot_operations));
+}
+
 static inline void set_enable_flags(struct quota_info *dqopt, int type)
 {
 	switch (type) {
@@ -1517,3 +1524,4 @@
 EXPORT_SYMBOL(register_quota_format);
 EXPORT_SYMBOL(unregister_quota_format);
 EXPORT_SYMBOL(dqstats);
+EXPORT_SYMBOL(init_dquot_operations);
--- 1.9/fs/ext3/super.c	Tue Apr 22 03:16:23 2003
+++ edited/fs/ext3/super.c	Mon Jul 14 06:01:49 2003
@@ -448,6 +448,9 @@
 	return;
 }
 
+static struct dquot_operations ext3_qops;
+static int (*old_sync_dquot)(struct dquot *dquot);
+
 static struct super_operations ext3_sops = {
 	read_inode:	ext3_read_inode,	/* BKL held */
 	write_inode:	ext3_write_inode,	/* BKL not held.  Don't need */
@@ -1128,6 +1131,7 @@
 	 * set up enough so that it can read an inode
 	 */
 	sb->s_op = &ext3_sops;
+	sb->dq_op = &ext3_qops;
 	INIT_LIST_HEAD(&sbi->s_orphan); /* unlinked but open files */
 
 	sb->s_root = 0;
@@ -1758,10 +1762,65 @@
 	return 0;
 }
 
+/* Helper function for writing quotas on sync - we need to start transaction before quota file
+ * is locked for write. Otherwise the are possible deadlocks:
+ * Process 1                         Process 2
+ * ext3_create()                     quota_sync()
+ *   journal_start()                   write_dquot()
+ *   DQUOT_INIT()                        down(dqio_sem)
+ *     down(dqio_sem)                    journal_start()
+ *
+ */
+
+#ifdef CONFIG_QUOTA
+
+/* Blocks: (2 data blocks) * (3 indirect + 1 descriptor + 1 bitmap) + superblock */
+#define EXT3_OLD_QFMT_BLOCKS 11
+/* Blocks: quota info + (4 pointer blocks + 1 entry block) * (3 indirect + 1 descriptor + 1 bitmap) + superblock */
+#define EXT3_V0_QFMT_BLOCKS 27
+
+static int ext3_sync_dquot(struct dquot *dquot)
+{
+	int nblocks, ret;
+	handle_t *handle;
+	struct quota_info *dqops = sb_dqopt(dquot->dq_sb);
+	struct inode *qinode;
+
+	switch (dqops->info[dquot->dq_type].dqi_format->qf_fmt_id) {
+		case QFMT_VFS_OLD:
+			nblocks = EXT3_OLD_QFMT_BLOCKS;
+			break;
+		case QFMT_VFS_V0:
+			nblocks = EXT3_V0_QFMT_BLOCKS;
+			break;
+		default:
+			nblocks = EXT3_MAX_TRANS_DATA;
+	}
+	lock_kernel();
+	qinode = dqops->files[dquot->dq_type]->f_dentry->d_inode;
+	handle = ext3_journal_start(qinode, nblocks);
+	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
+
 static DECLARE_FSTYPE_DEV(ext3_fs_type, "ext3", ext3_read_super);
 
 static int __init init_ext3_fs(void)
 {
+#ifdef CONFIG_QUOTA
+	init_dquot_operations(&ext3_qops);
+	old_sync_dquot = ext3_qops.sync_dquot;
+	ext3_qops.sync_dquot = ext3_sync_dquot;
+#endif
         return register_filesystem(&ext3_fs_type);
 }
 
===== include/linux/quota.h 1.4 vs edited =====
--- 1.4/include/linux/quota.h	Tue Jun 24 23:11:29 2003
+++ edited/include/linux/quota.h	Mon Jul 14 06:03:04 2003
@@ -251,6 +251,7 @@
 	void (*free_space) (struct inode *, qsize_t);
 	void (*free_inode) (const struct inode *, unsigned long);
 	int (*transfer) (struct inode *, struct iattr *);
+	int (*sync_dquot) (struct dquot *);
 };
 
 /* Operations handling requests from userspace */
@@ -312,6 +313,7 @@
 
 int register_quota_format(struct quota_format_type *fmt);
 void unregister_quota_format(struct quota_format_type *fmt);
+void init_dquot_operations(struct dquot_operations *fsdqops);
 
 #else
 

Re: -- END OF BLOCK -- (fwd)

[Alan Cox]

On Llu, 2003-07-14 at 15:36, Christoph Hellwig wrote:
> Now it seems Alan merged other stuff without telling the person
> who send him the original patch, thus -ac seems to have code the
> XFS tree and -aa don't have.

Yes I merged the vendor fixes that stopped it hanging machines. I was
suprised you didn't work from that base since that had extensive vendor
tree testing and while it had some uglies which you cleaned up in the
other stuff it would have been a saner base point

Re: -- END OF BLOCK -- (fwd)

[Marcelo Tosatti]

On Mon, 14 Jul 2003, Christoph Hellwig wrote:

> On Mon, Jul 14, 2003 at 10:08:02AM -0300, Marcelo Tosatti wrote:
> > The quota code you have in -ac is new Jan Kara's stuff (which supports
> > both formats, etc) plus the ext3 deadlock avoidance and the compat stuff ?
> >
> > Christoph, for what reason have you removed the ext3 deadlock avoidance
> > patches? And also, what else have you changed wrt originals Jan code ?
>
> Umm, I didn't not remove anything.  I backported Jan's code from 2.5
> long ago and resynched with 2.5 from time to time.  I sent this code
> to Alan and Andrea for -aa and -ac and now sent it to you with the
> changes you requested (make the old quota format and userland ABI
> compiled in unconditionally.)
>
> Now it seems Alan merged other stuff without telling the person
> who send him the original patch, thus -ac seems to have code the
> XFS tree and -aa don't have.
>
> I've now extracted the ext3 quota changes from -ac for you.  Note
> that their only relation of them to the new quota code is that
> the patch is ontop of it and thus takes it into account, the problem
> is an old one.
>
> Here's the patch, probably the last 2.4 patch from me if we really
> want to play the hot potatoe game.
>
> > Lets have -pre6 soon to fix up this mess (which is causing DEADLOCKS), ok?
>
> s/cause/not fix existing/
>
>
> p.s. yes, the patch is more ugly than anything I'd usually submit.
> but it's not mine and I don't have any interest in it.  merge it asis
> or clean it up yourself - I don't care anymore.

Thanks a lot for the patch. Its applied.