From: Mingming Cao <cmm@us.ibm.com>
Subject: Re: [PATCH V5 2/5] quota: Add quota claim and release reserved
	quota blocks operations
Date: Mon, 12 Jan 2009 16:24:47 -0800
Message-ID: <1231806287.6752.25.camel@mingming-laptop>
References: <1231216837.9267.24.camel@mingming-laptop>
	 <20090106095252.GG10705@duck.suse.cz>
Mime-Version: 1.0
Content-Type: multipart/mixed; boundary="=-pfbnQ2oB7v2ivalSHPWc"
Cc: Andrew Morton <akpm@linux-foundation.org>, tytso <tytso@mit.edu>,
	linux-ext4 <linux-ext4@vger.kernel.org>,
	linux-fsdevel <linux-fsdevel@vger.kernel.org>
To: Jan Kara <jack@suse.cz>
Return-path: <linux-ext4-owner@vger.kernel.org>
Received: from e32.co.us.ibm.com ([32.97.110.150]:54978 "EHLO
	e32.co.us.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1750826AbZAMAYt (ORCPT
	<rfc822;linux-ext4@vger.kernel.org>); Mon, 12 Jan 2009 19:24:49 -0500
In-Reply-To: <20090106095252.GG10705@duck.suse.cz>
Sender: linux-ext4-owner@vger.kernel.org
List-ID: <linux-ext4.vger.kernel.org>


--=-pfbnQ2oB7v2ivalSHPWc
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Hi Jan,

在 2009-01-06二的 10:52 +0100，Jan Kara写道：
> Hi,
> 
> On Mon 05-01-09 20:40:37, Mingming Cao wrote:
> > quota: Add quota reservation claim and released operations
> > 
> > Reserved quota will be claimed at the block allocation time. Over-booked
> > quota could be returned back with the release callback function.
> > 
> > Signed-off-by: Mingming Cao <cmm@us.ibm.com>
>   Just a few minor comments below.
> 
> > ---
> >  fs/dquot.c               |  113 +++++++++++++++++++++++++++++++++++++++++++++--
> >  include/linux/quota.h    |    6 ++
> >  include/linux/quotaops.h |   53 ++++++++++++++++++++++
> >  3 files changed, 168 insertions(+), 4 deletions(-)
> > 
> > Index: linux-2.6.28-git7/include/linux/quota.h
> > ===================================================================
> > --- linux-2.6.28-git7.orig/include/linux/quota.h	2009-01-05 17:26:47.000000000 -0800
> > +++ linux-2.6.28-git7/include/linux/quota.h	2009-01-05 17:26:48.000000000 -0800
> > @@ -311,6 +311,12 @@ struct dquot_operations {
> >  	int (*write_info) (struct super_block *, int);	/* Write of quota "superblock" */
> >  	/* reserve quota for delayed block allocation */
> >  	int (*reserve_space) (struct inode *, qsize_t, int);
> > +	/* claim reserved quota for delayed alloc */
> > +	int (*claim_space) (struct inode *, qsize_t);
> > +	/* release rsved quota for delayed alloc */
> > +	void (*release_rsv) (struct inode *, qsize_t);
> > +	/* get reserved quota for delayed alloc */
> > +	unsigned long long (*get_reserved_space) (struct inode *);
>         ^^^ qsize_t would be more appropriate here IMO.
> 
> 
> >  };
> >  
> >  /* Operations handling requests from userspace */
>   <snip>
> 
> > Index: linux-2.6.28-git7/fs/dquot.c
> > ===================================================================
> > --- linux-2.6.28-git7.orig/fs/dquot.c	2009-01-05 17:26:47.000000000 -0800
> > +++ linux-2.6.28-git7/fs/dquot.c	2009-01-05 20:06:23.000000000 -0800
> > @@ -903,6 +903,23 @@ static inline void dquot_resv_space(stru
> >  	dquot->dq_dqb.dqb_rsvspace += number;
> >  }
> >  
> > +/*
> > + * Claim reserved quota space
> > + */
> > +static void dquot_claim_reserved_space(struct dquot *dquot,
> > +						qsize_t number)
> > +{
> > +	WARN_ON(dquot->dq_dqb.dqb_rsvspace < number);
> > +	dquot->dq_dqb.dqb_curspace += number;
> > +	dquot->dq_dqb.dqb_rsvspace -= number;
> > +}
> > +
> > +static inline
> > +void dquot_free_reserved_space(struct dquot *dquot, qsize_t number)
> > +{
> > +	dquot->dq_dqb.dqb_rsvspace -= number;
> > +}
> > +
> >  static inline void dquot_decr_inodes(struct dquot *dquot, qsize_t number)
> >  {
> >  	if (sb_dqopt(dquot->dq_sb)->flags & DQUOT_NEGATIVE_USAGE ||
> > @@ -1432,6 +1449,75 @@ warn_put_all:
> >  	return ret;
> >  }
> >  
> > +int dquot_claim_space(struct inode *inode, qsize_t number)
> > +{
> > +	int cnt;
> > +	int ret = QUOTA_OK;
> > +
> > +	if (IS_NOQUOTA(inode)) {
> > +		inode_add_bytes(inode, number);
> > +		goto out;
> > +	}
> > +
> > +	down_read(&sb_dqopt(inode->i_sb)->dqptr_sem);
> > +	if (IS_NOQUOTA(inode))	{
> > +		up_read(&sb_dqopt(inode->i_sb)->dqptr_sem);
> > +		inode_add_bytes(inode, number);
> > +		goto out;
> > +	}
> > +
> > +	spin_lock(&dq_data_lock);
> > +	/* Claim reserved quotas to allocated quotas */
> > +	for (cnt = 0; cnt < MAXQUOTAS; cnt++) {
> > +		if (inode->i_dquot[cnt] != NODQUOT)
> > +			dquot_claim_reserved_space(inode->i_dquot[cnt],
> > +							number);
> > +	}
> > +	/* Update inode bytes */
> > +	inode_add_bytes(inode, number);
> > +	spin_unlock(&dq_data_lock);
> > +	/* Dirtify all the dquots - this can block when journalling */
> > +	for (cnt = 0; cnt < MAXQUOTAS; cnt++)
> > +		if (inode->i_dquot[cnt])
> > +			mark_dquot_dirty(inode->i_dquot[cnt]);
> > +	up_read(&sb_dqopt(inode->i_sb)->dqptr_sem);
> > +out:
> > +	return ret;
> > +}
> > +EXPORT_SYMBOL(dquot_claim_space);
> > +
> > +/*
> > + * Release reserved quota space
> > + */
> > +void dquot_release_reserved_space(struct inode *inode, qsize_t number)
> > +{
> > +	int cnt;
> > +	struct dquot *dquot;
> > +
> > +	if (IS_NOQUOTA(inode))
> > +		goto out;
> > +
> > +	down_read(&sb_dqopt(inode->i_sb)->dqptr_sem);
> > +	if (IS_NOQUOTA(inode))
> > +		goto out_unlock;
> > +
> > +	spin_lock(&dq_data_lock);
> > +	/* Release reserved dquots */
> > +	for (cnt = 0; cnt < MAXQUOTAS; cnt++) {
> > +		if (inode->i_dquot[cnt] != NODQUOT) {
> > +			dquot = inode->i_dquot[cnt];
> > +			dquot_free_reserved_space(dquot, number);
> > +		}
> > +	}
> > +	spin_unlock(&dq_data_lock);
>   Variable dquot seems to be quite useless here...
> 
Yeah, it was probably due to the rewrite of this part. I removed this
varible .

> > +
> > +out_unlock:
> > +	up_read(&sb_dqopt(inode->i_sb)->dqptr_sem);
> > +out:
> > +	return;
> > +}
> > +EXPORT_SYMBOL(dquot_release_reserved_space);
> > +
> >  /*
> >   * This operation can block, but only after everything is updated
> >   */
> > @@ -1507,6 +1593,19 @@ int dquot_free_inode(const struct inode 
> >  	up_read(&sb_dqopt(inode->i_sb)->dqptr_sem);
> >  	return QUOTA_OK;
> >  }
>   Empty line here?
> 
Removed.
> > +/*
> > + * call back function, get reserved quota space from underlying fs
> > + */
> > +qsize_t dquot_get_reserved_space(struct inode *inode)
> > +{
> > +	qsize_t reserved_space = 0;
> > +
> > +	if (sb_any_quota_active(inode->i_sb) &&
> > +	    inode->i_sb->dq_op->get_reserved_space)
> > +		reserved_space = inode->i_sb->dq_op->get_reserved_space(inode)
> > +					<< inode->i_blkbits;
> > +	return reserved_space;
> > +}
>   The function should really return number of bytes as its name suggests.
> In principle there's no reason why inode could not have 5 bytes reserved
> :).
>   Hmm, sorry for my comment to ext4 patch. That is correct given what is
> written here. I was just confused by the name of the callback.
> 

No problem.  I am fine with either way.  To ext4, the number of bytes
reserved should be always aligned with the fs block size. But it
probably nicer to keep the interface consistant with other quota
operations.

I switched to the method you suggested ,having underlying fs return
number of bytes reserved. Please take a look, thanks.


--=-pfbnQ2oB7v2ivalSHPWc
Content-Disposition: attachment; filename=quota-claim-reservation-vfs.patch
Content-Type: text/x-patch; name=quota-claim-reservation-vfs.patch; charset=UTF-8
Content-Transfer-Encoding: 7bit

quota: Add quota reservation claim and released operations

Reserved quota will be claimed at the block allocation time. Over-booked
quota could be returned back with the release callback function.

Signed-off-by: Mingming Cao <cmm@us.ibm.com>
---
 fs/dquot.c               |  110 +++++++++++++++++++++++++++++++++++++++++++++--
 include/linux/quota.h    |    6 ++
 include/linux/quotaops.h |   53 ++++++++++++++++++++++
 3 files changed, 165 insertions(+), 4 deletions(-)

Index: linux-2.6.28-git7/include/linux/quota.h
===================================================================
--- linux-2.6.28-git7.orig/include/linux/quota.h	2009-01-06 17:35:38.000000000 -0800
+++ linux-2.6.28-git7/include/linux/quota.h	2009-01-08 17:22:56.000000000 -0800
@@ -311,6 +311,12 @@ struct dquot_operations {
 	int (*write_info) (struct super_block *, int);	/* Write of quota "superblock" */
 	/* reserve quota for delayed block allocation */
 	int (*reserve_space) (struct inode *, qsize_t, int);
+	/* claim reserved quota for delayed alloc */
+	int (*claim_space) (struct inode *, qsize_t);
+	/* release rsved quota for delayed alloc */
+	void (*release_rsv) (struct inode *, qsize_t);
+	/* get reserved quota for delayed alloc */
+	qsize_t (*get_reserved_space) (struct inode *);
 };
 
 /* Operations handling requests from userspace */
Index: linux-2.6.28-git7/include/linux/quotaops.h
===================================================================
--- linux-2.6.28-git7.orig/include/linux/quotaops.h	2009-01-06 17:35:38.000000000 -0800
+++ linux-2.6.28-git7/include/linux/quotaops.h	2009-01-08 17:18:40.000000000 -0800
@@ -37,6 +37,11 @@ void dquot_destroy(struct dquot *dquot);
 int dquot_alloc_space(struct inode *inode, qsize_t number, int prealloc);
 int dquot_alloc_inode(const struct inode *inode, qsize_t number);
 
+int dquot_reserve_space(struct inode *inode, qsize_t number, int prealloc);
+int dquot_claim_space(struct inode *inode, qsize_t number);
+void dquot_release_reserved_space(struct inode *inode, qsize_t number);
+qsize_t dquot_get_reserved_space(struct inode *inode);
+
 int dquot_free_space(struct inode *inode, qsize_t number);
 int dquot_free_inode(const struct inode *inode, qsize_t number);
 
@@ -205,6 +210,31 @@ static inline int vfs_dq_alloc_inode(str
 	return 0;
 }
 
+/*
+ * Convert in-memory reserved quotas to real consumed quotas
+ */
+static inline int vfs_dq_claim_space(struct inode *inode, qsize_t nr)
+{
+	if (sb_any_quota_active(inode->i_sb)) {
+		if (inode->i_sb->dq_op->claim_space(inode, nr) == NO_QUOTA)
+			return 1;
+	} else
+		inode_add_bytes(inode, nr);
+
+	mark_inode_dirty(inode);
+	return 0;
+}
+
+/*
+ * Release reserved (in-memory) quotas
+ */
+static inline
+void vfs_dq_release_reservation_space(struct inode *inode, qsize_t nr)
+{
+	if (sb_any_quota_active(inode->i_sb))
+		inode->i_sb->dq_op->release_rsv(inode, nr);
+}
+
 static inline void vfs_dq_free_space_nodirty(struct inode *inode, qsize_t nr)
 {
 	if (sb_any_quota_active(inode->i_sb))
@@ -356,6 +386,17 @@ static inline int vfs_dq_reserve_space(s
 	return 0;
 }
 
+static inline int vfs_dq_claim_space(struct inode *inode, qsize_t nr)
+{
+	return vfs_dq_alloc_space(inode, nr);
+}
+
+static inline
+int vfs_dq_release_reservation_space(struct inode *inode, qsize_t nr)
+{
+	return 0;
+}
+
 static inline void vfs_dq_free_space_nodirty(struct inode *inode, qsize_t nr)
 {
 	inode_sub_bytes(inode, nr);
@@ -399,6 +440,18 @@ static inline int vfs_dq_reserve_block(s
 			nr << inode->i_blkbits);
 }
 
+static inline int vfs_dq_claim_block(struct inode *inode, qsize_t nr)
+{
+	return vfs_dq_claim_space(inode,
+			nr << inode->i_blkbits);
+}
+
+static inline
+void vfs_dq_release_reservation_block(struct inode *inode, qsize_t nr)
+{
+	vfs_dq_release_reservation_space(inode, nr << inode->i_blkbits);
+}
+
 static inline void vfs_dq_free_block_nodirty(struct inode *inode, qsize_t nr)
 {
 	vfs_dq_free_space_nodirty(inode, nr << inode->i_sb->s_blocksize_bits);
Index: linux-2.6.28-git7/fs/dquot.c
===================================================================
--- linux-2.6.28-git7.orig/fs/dquot.c	2009-01-06 17:35:38.000000000 -0800
+++ linux-2.6.28-git7/fs/dquot.c	2009-01-08 17:22:56.000000000 -0800
@@ -903,6 +903,23 @@ static inline void dquot_resv_space(stru
 	dquot->dq_dqb.dqb_rsvspace += number;
 }
 
+/*
+ * Claim reserved quota space
+ */
+static void dquot_claim_reserved_space(struct dquot *dquot,
+						qsize_t number)
+{
+	WARN_ON(dquot->dq_dqb.dqb_rsvspace < number);
+	dquot->dq_dqb.dqb_curspace += number;
+	dquot->dq_dqb.dqb_rsvspace -= number;
+}
+
+static inline
+void dquot_free_reserved_space(struct dquot *dquot, qsize_t number)
+{
+	dquot->dq_dqb.dqb_rsvspace -= number;
+}
+
 static inline void dquot_decr_inodes(struct dquot *dquot, qsize_t number)
 {
 	if (sb_dqopt(dquot->dq_sb)->flags & DQUOT_NEGATIVE_USAGE ||
@@ -1438,6 +1455,72 @@ warn_put_all:
 	return ret;
 }
 
+int dquot_claim_space(struct inode *inode, qsize_t number)
+{
+	int cnt;
+	int ret = QUOTA_OK;
+
+	if (IS_NOQUOTA(inode)) {
+		inode_add_bytes(inode, number);
+		goto out;
+	}
+
+	down_read(&sb_dqopt(inode->i_sb)->dqptr_sem);
+	if (IS_NOQUOTA(inode))	{
+		up_read(&sb_dqopt(inode->i_sb)->dqptr_sem);
+		inode_add_bytes(inode, number);
+		goto out;
+	}
+
+	spin_lock(&dq_data_lock);
+	/* Claim reserved quotas to allocated quotas */
+	for (cnt = 0; cnt < MAXQUOTAS; cnt++) {
+		if (inode->i_dquot[cnt] != NODQUOT)
+			dquot_claim_reserved_space(inode->i_dquot[cnt],
+							number);
+	}
+	/* Update inode bytes */
+	inode_add_bytes(inode, number);
+	spin_unlock(&dq_data_lock);
+	/* Dirtify all the dquots - this can block when journalling */
+	for (cnt = 0; cnt < MAXQUOTAS; cnt++)
+		if (inode->i_dquot[cnt])
+			mark_dquot_dirty(inode->i_dquot[cnt]);
+	up_read(&sb_dqopt(inode->i_sb)->dqptr_sem);
+out:
+	return ret;
+}
+EXPORT_SYMBOL(dquot_claim_space);
+
+/*
+ * Release reserved quota space
+ */
+void dquot_release_reserved_space(struct inode *inode, qsize_t number)
+{
+	int cnt;
+
+	if (IS_NOQUOTA(inode))
+		goto out;
+
+	down_read(&sb_dqopt(inode->i_sb)->dqptr_sem);
+	if (IS_NOQUOTA(inode))
+		goto out_unlock;
+
+	spin_lock(&dq_data_lock);
+	/* Release reserved dquots */
+	for (cnt = 0; cnt < MAXQUOTAS; cnt++) {
+		if (inode->i_dquot[cnt] != NODQUOT)
+			dquot_free_reserved_space(inode->i_dquot[cnt], number);
+	}
+	spin_unlock(&dq_data_lock);
+
+out_unlock:
+	up_read(&sb_dqopt(inode->i_sb)->dqptr_sem);
+out:
+	return;
+}
+EXPORT_SYMBOL(dquot_release_reserved_space);
+
 /*
  * This operation can block, but only after everything is updated
  */
@@ -1515,6 +1598,19 @@ int dquot_free_inode(const struct inode 
 }
 
 /*
+ * call back function, get reserved quota space from underlying fs
+ */
+qsize_t dquot_get_reserved_space(struct inode *inode)
+{
+	qsize_t reserved_space = 0;
+
+	if (sb_any_quota_active(inode->i_sb) &&
+	    inode->i_sb->dq_op->get_reserved_space)
+		reserved_space = inode->i_sb->dq_op->get_reserved_space(inode);
+	return reserved_space;
+}
+
+/*
  * Transfer the number of inode and blocks from one diskquota to an other.
  *
  * This operation can block, but only after everything is updated
@@ -1522,7 +1618,8 @@ int dquot_free_inode(const struct inode 
  */
 int dquot_transfer(struct inode *inode, struct iattr *iattr)
 {
-	qsize_t space;
+	qsize_t space, cur_space;
+	qsize_t rsv_space = 0;
 	struct dquot *transfer_from[MAXQUOTAS];
 	struct dquot *transfer_to[MAXQUOTAS];
 	int cnt, ret = NO_QUOTA, chuid = (iattr->ia_valid & ATTR_UID) && inode->i_uid != iattr->ia_uid,
@@ -1563,7 +1660,9 @@ int dquot_transfer(struct inode *inode, 
 		}
 	}
 	spin_lock(&dq_data_lock);
-	space = inode_get_bytes(inode);
+	cur_space = inode_get_bytes(inode);
+	rsv_space = dquot_get_reserved_space(inode);
+	space = cur_space + rsv_space;
 	/* Build the transfer_from list and check the limits */
 	for (cnt = 0; cnt < MAXQUOTAS; cnt++) {
 		if (transfer_to[cnt] == NODQUOT)
@@ -1592,11 +1691,14 @@ int dquot_transfer(struct inode *inode, 
 			warntype_from_space[cnt] =
 				info_bdq_free(transfer_from[cnt], space);
 			dquot_decr_inodes(transfer_from[cnt], 1);
-			dquot_decr_space(transfer_from[cnt], space);
+			dquot_decr_space(transfer_from[cnt], cur_space);
+			dquot_free_reserved_space(transfer_from[cnt],
+						  rsv_space);
 		}
 
 		dquot_incr_inodes(transfer_to[cnt], 1);
-		dquot_incr_space(transfer_to[cnt], space);
+		dquot_incr_space(transfer_to[cnt], cur_space);
+		dquot_resv_space(transfer_to[cnt], rsv_space);
 
 		inode->i_dquot[cnt] = transfer_to[cnt];
 	}

--=-pfbnQ2oB7v2ivalSHPWc--