[Cluster-devel] [GFS2 PATCH 0/4] fallocate quota fixes

[Cluster-devel] [GFS2 PATCH 0/4] fallocate quota fixes

[Abhi Das]

These patches are related to bz1174295 where fallocate could
exceed quota. I'm posting these for early feedback as these
patches are only compile-tested so far.

patch 1 - This is the patch that actually addresses the quota
	  exceed issue. Quota checks were not being performed
	  against the blocks about to be allocated.

patch 2 - Adds new variants of quota check functions that
	  return the number of allowed blocks if quotas are
	  violated by the number of requested blocks

patch 3 - Adds a new variant of gfs2_inplace_reserve that
	  returns the max number of available blocks if the
	  function returns -ENOSPC due to unavailability of
	  the requested number of blocks.

patch 4 - Allows fallocate to take advantage of patches 2 and
	  3 to efficiently max out quotas or fill up the fs
	  instead of returning -EDQUOT/-ENOSPC and leaving some
	  available blocks unallocated.

Abhi Das (4):
  gfs2: check quota for blocks we're about to allocate
  gfs2: add new quota check functions
  gfs2: add new function gfs2_inpl_rsrv_ret_max_avl
  gfs2: allow fallocate to max out quotas/fs efficiently

 fs/gfs2/aops.c  |  6 +++---
 fs/gfs2/bmap.c  |  2 +-
 fs/gfs2/file.c  | 30 ++++++++++++++++++------------
 fs/gfs2/inode.c | 14 ++++++++------
 fs/gfs2/quota.c | 26 +++++++++++++++++++++-----
 fs/gfs2/quota.h | 20 +++++++++++++++++---
 fs/gfs2/rgrp.c  | 13 +++++++++++--
 fs/gfs2/rgrp.h  | 10 +++++++++-
 fs/gfs2/xattr.c |  2 +-
 9 files changed, 89 insertions(+), 34 deletions(-)

-- 
1.8.1.4

[Cluster-devel] [GFS2 PATCH 1/4] gfs2: check quota for blocks we're about to allocate

[Abhi Das]

This patch allows gfs2_quota_check() to take an extra argument
called 'exp_change'. Prior to any allocation, gfs2_quota_check()
or gfs2_quota_lock_check() is called with exp_change containing
the number of blocks we expect to allocate in this operation.
gfs2_quota_check() will add this number to the current usage and
check the sum against the quota warns and limits and fail the
operation if necessary.

Resolves: rhbz#1174295
Signed-off-by: Abhi Das <adas@redhat.com>
---
 fs/gfs2/aops.c  |  6 +++---
 fs/gfs2/bmap.c  |  2 +-
 fs/gfs2/file.c  |  8 ++++----
 fs/gfs2/inode.c | 14 ++++++++------
 fs/gfs2/quota.c |  4 ++--
 fs/gfs2/quota.h |  6 +++---
 fs/gfs2/xattr.c |  2 +-
 7 files changed, 22 insertions(+), 20 deletions(-)

diff --git a/fs/gfs2/aops.c b/fs/gfs2/aops.c
index 805b37f..aa7700a 100644
--- a/fs/gfs2/aops.c
+++ b/fs/gfs2/aops.c
@@ -671,12 +671,12 @@ static int gfs2_write_begin(struct file *file, struct address_space *mapping,
 
 	if (alloc_required) {
 		struct gfs2_alloc_parms ap = { .aflags = 0, };
-		error = gfs2_quota_lock_check(ip);
+		requested = data_blocks + ind_blocks;
+		ap.target = requested;
+		error = gfs2_quota_lock_check(ip, ap.target);
 		if (error)
 			goto out_unlock;
 
-		requested = data_blocks + ind_blocks;
-		ap.target = requested;
 		error = gfs2_inplace_reserve(ip, &ap);
 		if (error)
 			goto out_qunlock;
diff --git a/fs/gfs2/bmap.c b/fs/gfs2/bmap.c
index f0b945a..86cc7b2 100644
--- a/fs/gfs2/bmap.c
+++ b/fs/gfs2/bmap.c
@@ -1224,7 +1224,7 @@ static int do_grow(struct inode *inode, u64 size)
 
 	if (gfs2_is_stuffed(ip) &&
 	    (size > (sdp->sd_sb.sb_bsize - sizeof(struct gfs2_dinode)))) {
-		error = gfs2_quota_lock_check(ip);
+		error = gfs2_quota_lock_check(ip, ap.target);
 		if (error)
 			return error;
 
diff --git a/fs/gfs2/file.c b/fs/gfs2/file.c
index 6e600ab..c9482ae 100644
--- a/fs/gfs2/file.c
+++ b/fs/gfs2/file.c
@@ -429,11 +429,11 @@ static int gfs2_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf)
 	if (ret)
 		goto out_unlock;
 
-	ret = gfs2_quota_lock_check(ip);
-	if (ret)
-		goto out_unlock;
 	gfs2_write_calc_reserv(ip, PAGE_CACHE_SIZE, &data_blocks, &ind_blocks);
 	ap.target = data_blocks + ind_blocks;
+	ret = gfs2_quota_lock_check(ip, ap.target);
+	if (ret)
+		goto out_unlock;
 	ret = gfs2_inplace_reserve(ip, &ap);
 	if (ret)
 		goto out_quota_unlock;
@@ -828,7 +828,7 @@ static long __gfs2_fallocate(struct file *file, int mode, loff_t offset, loff_t
 			offset += bytes;
 			continue;
 		}
-		error = gfs2_quota_lock_check(ip);
+		error = gfs2_quota_lock_check(ip, bytes >> sdp->sd_sb.sb_bsize_shift);
 		if (error)
 			return error;
 retry:
diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c
index 73c72253..67ffa07 100644
--- a/fs/gfs2/inode.c
+++ b/fs/gfs2/inode.c
@@ -382,7 +382,7 @@ static int alloc_dinode(struct gfs2_inode *ip, u32 flags, unsigned *dblocks)
 	struct gfs2_alloc_parms ap = { .target = *dblocks, .aflags = flags, };
 	int error;
 
-	error = gfs2_quota_lock_check(ip);
+	error = gfs2_quota_lock_check(ip, ap.target);
 	if (error)
 		goto out;
 
@@ -525,7 +525,7 @@ static int link_dinode(struct gfs2_inode *dip, const struct qstr *name,
 	int error;
 
 	if (da->nr_blocks) {
-		error = gfs2_quota_lock_check(dip);
+		error = gfs2_quota_lock_check(dip, ap.target);
 		if (error)
 			goto fail_quota_locks;
 
@@ -953,7 +953,7 @@ static int gfs2_link(struct dentry *old_dentry, struct inode *dir,
 
 	if (da.nr_blocks) {
 		struct gfs2_alloc_parms ap = { .target = da.nr_blocks, };
-		error = gfs2_quota_lock_check(dip);
+		error = gfs2_quota_lock_check(dip, ap.target);
 		if (error)
 			goto out_gunlock;
 
@@ -1470,7 +1470,7 @@ static int gfs2_rename(struct inode *odir, struct dentry *odentry,
 
 	if (da.nr_blocks) {
 		struct gfs2_alloc_parms ap = { .target = da.nr_blocks, };
-		error = gfs2_quota_lock_check(ndip);
+		error = gfs2_quota_lock_check(ndip, ap.target);
 		if (error)
 			goto out_gunlock;
 
@@ -1669,6 +1669,7 @@ static int setattr_chown(struct inode *inode, struct iattr *attr)
 	kuid_t ouid, nuid;
 	kgid_t ogid, ngid;
 	int error;
+	u64 blocks;
 
 	ouid = inode->i_uid;
 	ogid = inode->i_gid;
@@ -1696,9 +1697,11 @@ static int setattr_chown(struct inode *inode, struct iattr *attr)
 	if (error)
 		goto out;
 
+	blocks = gfs2_get_inode_blocks(&ip->i_inode);
+
 	if (!uid_eq(ouid, NO_UID_QUOTA_CHANGE) ||
 	    !gid_eq(ogid, NO_GID_QUOTA_CHANGE)) {
-		error = gfs2_quota_check(ip, nuid, ngid);
+		error = gfs2_quota_check(ip, nuid, ngid, blocks);
 		if (error)
 			goto out_gunlock_q;
 	}
@@ -1713,7 +1716,6 @@ static int setattr_chown(struct inode *inode, struct iattr *attr)
 
 	if (!uid_eq(ouid, NO_UID_QUOTA_CHANGE) ||
 	    !gid_eq(ogid, NO_GID_QUOTA_CHANGE)) {
-		u64 blocks = gfs2_get_inode_blocks(&ip->i_inode);
 		gfs2_quota_change(ip, -blocks, ouid, ogid);
 		gfs2_quota_change(ip, blocks, nuid, ngid);
 	}
diff --git a/fs/gfs2/quota.c b/fs/gfs2/quota.c
index c8b148b..e2f86ec 100644
--- a/fs/gfs2/quota.c
+++ b/fs/gfs2/quota.c
@@ -1093,7 +1093,7 @@ static int print_message(struct gfs2_quota_data *qd, char *type)
 	return 0;
 }
 
-int gfs2_quota_check(struct gfs2_inode *ip, kuid_t uid, kgid_t gid)
+int gfs2_quota_check(struct gfs2_inode *ip, kuid_t uid, kgid_t gid, s64 exp_change)
 {
 	struct gfs2_sbd *sdp = GFS2_SB(&ip->i_inode);
 	struct gfs2_quota_data *qd;
@@ -1116,7 +1116,7 @@ int gfs2_quota_check(struct gfs2_inode *ip, kuid_t uid, kgid_t gid)
 
 		value = (s64)be64_to_cpu(qd->qd_qb.qb_value);
 		spin_lock(&qd_lock);
-		value += qd->qd_change;
+		value += qd->qd_change + exp_change;
 		spin_unlock(&qd_lock);
 
 		if (be64_to_cpu(qd->qd_qb.qb_limit) && (s64)be64_to_cpu(qd->qd_qb.qb_limit) < value) {
diff --git a/fs/gfs2/quota.h b/fs/gfs2/quota.h
index 55d506e..1457c66 100644
--- a/fs/gfs2/quota.h
+++ b/fs/gfs2/quota.h
@@ -24,7 +24,7 @@ extern void gfs2_quota_unhold(struct gfs2_inode *ip);
 extern int gfs2_quota_lock(struct gfs2_inode *ip, kuid_t uid, kgid_t gid);
 extern void gfs2_quota_unlock(struct gfs2_inode *ip);
 
-extern int gfs2_quota_check(struct gfs2_inode *ip, kuid_t uid, kgid_t gid);
+extern int gfs2_quota_check(struct gfs2_inode *ip, kuid_t uid, kgid_t gid, s64 exp_change);
 extern void gfs2_quota_change(struct gfs2_inode *ip, s64 change,
 			      kuid_t uid, kgid_t gid);
 
@@ -37,7 +37,7 @@ extern int gfs2_quotad(void *data);
 
 extern void gfs2_wake_up_statfs(struct gfs2_sbd *sdp);
 
-static inline int gfs2_quota_lock_check(struct gfs2_inode *ip)
+static inline int gfs2_quota_lock_check(struct gfs2_inode *ip, s64 exp_change)
 {
 	struct gfs2_sbd *sdp = GFS2_SB(&ip->i_inode);
 	int ret;
@@ -48,7 +48,7 @@ static inline int gfs2_quota_lock_check(struct gfs2_inode *ip)
 		return ret;
 	if (sdp->sd_args.ar_quota != GFS2_QUOTA_ON)
 		return 0;
-	ret = gfs2_quota_check(ip, ip->i_inode.i_uid, ip->i_inode.i_gid);
+	ret = gfs2_quota_check(ip, ip->i_inode.i_uid, ip->i_inode.i_gid, exp_change);
 	if (ret)
 		gfs2_quota_unlock(ip);
 	return ret;
diff --git a/fs/gfs2/xattr.c b/fs/gfs2/xattr.c
index 0b81f78..479ce0a 100644
--- a/fs/gfs2/xattr.c
+++ b/fs/gfs2/xattr.c
@@ -732,7 +732,7 @@ static int ea_alloc_skeleton(struct gfs2_inode *ip, struct gfs2_ea_request *er,
 	if (error)
 		return error;
 
-	error = gfs2_quota_lock_check(ip);
+	error = gfs2_quota_lock_check(ip, ap.target);
 	if (error)
 		return error;
 
-- 
1.8.1.4

[Cluster-devel] [GFS2 PATCH 1/4] gfs2: check quota for blocks we're about to allocate

[Andrew Price]

Hi Abhi,

On 12/02/15 16:54, Abhi Das wrote:
> This patch allows gfs2_quota_check() to take an extra argument
> called 'exp_change'. Prior to any allocation, gfs2_quota_check()
> or gfs2_quota_lock_check() is called with exp_change containing
> the number of blocks we expect to allocate in this operation.
> gfs2_quota_check() will add this number to the current usage and
> check the sum against the quota warns and limits and fail the
> operation if necessary.
>
<snip>
> -int gfs2_quota_check(struct gfs2_inode *ip, kuid_t uid, kgid_t gid)
> +int gfs2_quota_check(struct gfs2_inode *ip, kuid_t uid, kgid_t gid, s64 exp_change)
>   {
>   	struct gfs2_sbd *sdp = GFS2_SB(&ip->i_inode);
>   	struct gfs2_quota_data *qd;
> @@ -1116,7 +1116,7 @@ int gfs2_quota_check(struct gfs2_inode *ip, kuid_t uid, kgid_t gid)
>
>   		value = (s64)be64_to_cpu(qd->qd_qb.qb_value);
>   		spin_lock(&qd_lock);
> -		value += qd->qd_change;
> +		value += qd->qd_change + exp_change;

So I guess I don't understand why we need both qd_change and exp_change 
now. What sets qd_change, and why can't it be accurate (i.e. equal to 
exp_change) when we need it to be?

Andy

[Cluster-devel] [GFS2 PATCH 1/4] gfs2: check quota for blocks we're about to allocate

[Abhijith Das]

----- Original Message -----
> From: "Andrew Price" <anprice@redhat.com>
> To: "Abhi Das" <adas@redhat.com>, cluster-devel at redhat.com
> Sent: Thursday, February 12, 2015 12:12:50 PM
> Subject: Re: [Cluster-devel] [GFS2 PATCH 1/4] gfs2: check quota for blocks we're about to allocate
> 
> Hi Abhi,
> 
> On 12/02/15 16:54, Abhi Das wrote:
> > This patch allows gfs2_quota_check() to take an extra argument
> > called 'exp_change'. Prior to any allocation, gfs2_quota_check()
> > or gfs2_quota_lock_check() is called with exp_change containing
> > the number of blocks we expect to allocate in this operation.
> > gfs2_quota_check() will add this number to the current usage and
> > check the sum against the quota warns and limits and fail the
> > operation if necessary.
> >
> <snip>
> > -int gfs2_quota_check(struct gfs2_inode *ip, kuid_t uid, kgid_t gid)
> > +int gfs2_quota_check(struct gfs2_inode *ip, kuid_t uid, kgid_t gid, s64
> > exp_change)
> >   {
> >   	struct gfs2_sbd *sdp = GFS2_SB(&ip->i_inode);
> >   	struct gfs2_quota_data *qd;
> > @@ -1116,7 +1116,7 @@ int gfs2_quota_check(struct gfs2_inode *ip, kuid_t
> > uid, kgid_t gid)
> >
> >   		value = (s64)be64_to_cpu(qd->qd_qb.qb_value);
> >   		spin_lock(&qd_lock);
> > -		value += qd->qd_change;
> > +		value += qd->qd_change + exp_change;
> 
> So I guess I don't understand why we need both qd_change and exp_change
> now. What sets qd_change, and why can't it be accurate (i.e. equal to
> exp_change) when we need it to be?
> 
> Andy
> 

qd_change gets set when gfs2_quota_change() is called... after the current operation has
completed to account for the block allocs/deallocs that have been done during that operation.
Once gfs2_quota_change() is called, that quota change is basically set in stone and do_qc()
goes ahead and syncs it to the local quota_change file and eventually it'll get synced to
the global quota file.

So, when quota_check() is called, the qd_change value (if not synced yet) will contain the
usage as of the previous operation. Does that make sense?

Cheers!
--Abhi

[Cluster-devel] [GFS2 PATCH 2/4] gfs2: add new quota check functions

[Abhi Das]

gfs2_quota_chk_ret_allow and gfs2_quota_lck_chk_ret_allow are
variants of gfs2_quota_check and gfs2_quota_lock_check respectively.

If an operation will not succeed due to a quota violation, these
functions will return the number of blocks that quota will actually
allow without failing in an extra parameter 'allow'

If acceptable to the caller logic, any of the quota_check functions
may be called again with the 'allow'ed blocks to try and avoid a
quota violation.

Signed-off-by: Abhi Das <adas@redhat.com>
---
 fs/gfs2/quota.c | 24 ++++++++++++++++++++----
 fs/gfs2/quota.h | 20 +++++++++++++++++---
 2 files changed, 37 insertions(+), 7 deletions(-)

diff --git a/fs/gfs2/quota.c b/fs/gfs2/quota.c
index e2f86ec..98cdf97 100644
--- a/fs/gfs2/quota.c
+++ b/fs/gfs2/quota.c
@@ -1092,8 +1092,20 @@ static int print_message(struct gfs2_quota_data *qd, char *type)
 
 	return 0;
 }
-
-int gfs2_quota_check(struct gfs2_inode *ip, kuid_t uid, kgid_t gid, s64 exp_change)
+/**
+ * gfs2_quota_chk_ret_allow - Checks if adding the specified number of 
+ * blocks will exceed usr/grp quotas
+ * @ip: The inode for which this check is being performed
+ * @uid: The uid to check against
+ * @gid: The gid to check against
+ * @exp_change: The expected change in blocks
+ * @allow: If non-NULL, we should return the number of blocks
+ *         quota will allow if exp_change exceeds limits
+ *
+ * Returns: 0 on success, error code otherwise.
+ */
+int gfs2_quota_chk_ret_allow(struct gfs2_inode *ip, kuid_t uid, kgid_t gid,
+			     s64 exp_change, u32 *allow)
 {
 	struct gfs2_sbd *sdp = GFS2_SB(&ip->i_inode);
 	struct gfs2_quota_data *qd;
@@ -1119,12 +1131,16 @@ int gfs2_quota_check(struct gfs2_inode *ip, kuid_t uid, kgid_t gid, s64 exp_chan
 		value += qd->qd_change + exp_change;
 		spin_unlock(&qd_lock);
 
-		if (be64_to_cpu(qd->qd_qb.qb_limit) && (s64)be64_to_cpu(qd->qd_qb.qb_limit) < value) {
+		if (be64_to_cpu(qd->qd_qb.qb_limit) &&
+		    (s64)be64_to_cpu(qd->qd_qb.qb_limit) < value) {
 			print_message(qd, "exceeded");
 			quota_send_warning(qd->qd_id,
 					   sdp->sd_vfs->s_dev, QUOTA_NL_BHARDWARN);
-
 			error = -EDQUOT;
+			if (allow) {
+				*allow = (s64)be64_to_cpu(qd->qd_qb.qb_limit) -
+					(value - exp_change);
+			}
 			break;
 		} else if (be64_to_cpu(qd->qd_qb.qb_warn) &&
 			   (s64)be64_to_cpu(qd->qd_qb.qb_warn) < value &&
diff --git a/fs/gfs2/quota.h b/fs/gfs2/quota.h
index 1457c66..49d1fd9 100644
--- a/fs/gfs2/quota.h
+++ b/fs/gfs2/quota.h
@@ -24,7 +24,14 @@ extern void gfs2_quota_unhold(struct gfs2_inode *ip);
 extern int gfs2_quota_lock(struct gfs2_inode *ip, kuid_t uid, kgid_t gid);
 extern void gfs2_quota_unlock(struct gfs2_inode *ip);
 
-extern int gfs2_quota_check(struct gfs2_inode *ip, kuid_t uid, kgid_t gid, s64 exp_change);
+extern int gfs2_quota_chk_ret_allow(struct gfs2_inode *ip, kuid_t uid,
+				    kgid_t gid, s64 exp_change, u32 *allow);
+static inline int gfs2_quota_check(struct gfs2_inode *ip, kuid_t uid, kgid_t gid,
+				   s64 exp_change)
+{
+	return gfs2_quota_chk_ret_allow(ip, uid, gid, exp_change, NULL);
+}
+
 extern void gfs2_quota_change(struct gfs2_inode *ip, s64 change,
 			      kuid_t uid, kgid_t gid);
 
@@ -37,7 +44,8 @@ extern int gfs2_quotad(void *data);
 
 extern void gfs2_wake_up_statfs(struct gfs2_sbd *sdp);
 
-static inline int gfs2_quota_lock_check(struct gfs2_inode *ip, s64 exp_change)
+static inline int gfs2_quota_lck_chk_ret_allow(struct gfs2_inode *ip,
+					       s64 exp_change, u32 *allow)
 {
 	struct gfs2_sbd *sdp = GFS2_SB(&ip->i_inode);
 	int ret;
@@ -48,12 +56,18 @@ static inline int gfs2_quota_lock_check(struct gfs2_inode *ip, s64 exp_change)
 		return ret;
 	if (sdp->sd_args.ar_quota != GFS2_QUOTA_ON)
 		return 0;
-	ret = gfs2_quota_check(ip, ip->i_inode.i_uid, ip->i_inode.i_gid, exp_change);
+	ret = gfs2_quota_chk_ret_allow(ip, ip->i_inode.i_uid, ip->i_inode.i_gid,
+				       exp_change, allow);
 	if (ret)
 		gfs2_quota_unlock(ip);
 	return ret;
 }
 
+static inline int gfs2_quota_lock_check(struct gfs2_inode *ip, s64 exp_change)
+{
+	return gfs2_quota_lck_chk_ret_allow(ip, exp_change, NULL);
+}
+
 extern const struct quotactl_ops gfs2_quotactl_ops;
 extern struct shrinker gfs2_qd_shrinker;
 extern struct list_lru gfs2_qd_lru;
-- 
1.8.1.4

[Cluster-devel] [GFS2 PATCH 2/4] gfs2: add new quota check functions

[Steven Whitehouse]

Hi,

Rather than changing the name of the function, it would be nicer just to 
add the extra variable in which to return the number of blocks that 
would be allowed in this case, and to update all the callers. It would 
also be worth considering whether it is worth passing the allocation 
parameters to this function rather than just the numbers of blocks - it 
might make it neater,

Steve.

On 12/02/15 16:54, Abhi Das wrote:
> gfs2_quota_chk_ret_allow and gfs2_quota_lck_chk_ret_allow are
> variants of gfs2_quota_check and gfs2_quota_lock_check respectively.
>
> If an operation will not succeed due to a quota violation, these
> functions will return the number of blocks that quota will actually
> allow without failing in an extra parameter 'allow'
>
> If acceptable to the caller logic, any of the quota_check functions
> may be called again with the 'allow'ed blocks to try and avoid a
> quota violation.
>
> Signed-off-by: Abhi Das <adas@redhat.com>
> ---
>   fs/gfs2/quota.c | 24 ++++++++++++++++++++----
>   fs/gfs2/quota.h | 20 +++++++++++++++++---
>   2 files changed, 37 insertions(+), 7 deletions(-)
>
> diff --git a/fs/gfs2/quota.c b/fs/gfs2/quota.c
> index e2f86ec..98cdf97 100644
> --- a/fs/gfs2/quota.c
> +++ b/fs/gfs2/quota.c
> @@ -1092,8 +1092,20 @@ static int print_message(struct gfs2_quota_data *qd, char *type)
>   
>   	return 0;
>   }
> -
> -int gfs2_quota_check(struct gfs2_inode *ip, kuid_t uid, kgid_t gid, s64 exp_change)
> +/**
> + * gfs2_quota_chk_ret_allow - Checks if adding the specified number of
> + * blocks will exceed usr/grp quotas
> + * @ip: The inode for which this check is being performed
> + * @uid: The uid to check against
> + * @gid: The gid to check against
> + * @exp_change: The expected change in blocks
> + * @allow: If non-NULL, we should return the number of blocks
> + *         quota will allow if exp_change exceeds limits
> + *
> + * Returns: 0 on success, error code otherwise.
> + */
> +int gfs2_quota_chk_ret_allow(struct gfs2_inode *ip, kuid_t uid, kgid_t gid,
> +			     s64 exp_change, u32 *allow)
>   {
>   	struct gfs2_sbd *sdp = GFS2_SB(&ip->i_inode);
>   	struct gfs2_quota_data *qd;
> @@ -1119,12 +1131,16 @@ int gfs2_quota_check(struct gfs2_inode *ip, kuid_t uid, kgid_t gid, s64 exp_chan
>   		value += qd->qd_change + exp_change;
>   		spin_unlock(&qd_lock);
>   
> -		if (be64_to_cpu(qd->qd_qb.qb_limit) && (s64)be64_to_cpu(qd->qd_qb.qb_limit) < value) {
> +		if (be64_to_cpu(qd->qd_qb.qb_limit) &&
> +		    (s64)be64_to_cpu(qd->qd_qb.qb_limit) < value) {
>   			print_message(qd, "exceeded");
>   			quota_send_warning(qd->qd_id,
>   					   sdp->sd_vfs->s_dev, QUOTA_NL_BHARDWARN);
> -
>   			error = -EDQUOT;
> +			if (allow) {
> +				*allow = (s64)be64_to_cpu(qd->qd_qb.qb_limit) -
> +					(value - exp_change);
> +			}
>   			break;
>   		} else if (be64_to_cpu(qd->qd_qb.qb_warn) &&
>   			   (s64)be64_to_cpu(qd->qd_qb.qb_warn) < value &&
> diff --git a/fs/gfs2/quota.h b/fs/gfs2/quota.h
> index 1457c66..49d1fd9 100644
> --- a/fs/gfs2/quota.h
> +++ b/fs/gfs2/quota.h
> @@ -24,7 +24,14 @@ extern void gfs2_quota_unhold(struct gfs2_inode *ip);
>   extern int gfs2_quota_lock(struct gfs2_inode *ip, kuid_t uid, kgid_t gid);
>   extern void gfs2_quota_unlock(struct gfs2_inode *ip);
>   
> -extern int gfs2_quota_check(struct gfs2_inode *ip, kuid_t uid, kgid_t gid, s64 exp_change);
> +extern int gfs2_quota_chk_ret_allow(struct gfs2_inode *ip, kuid_t uid,
> +				    kgid_t gid, s64 exp_change, u32 *allow);
> +static inline int gfs2_quota_check(struct gfs2_inode *ip, kuid_t uid, kgid_t gid,
> +				   s64 exp_change)
> +{
> +	return gfs2_quota_chk_ret_allow(ip, uid, gid, exp_change, NULL);
> +}
> +
>   extern void gfs2_quota_change(struct gfs2_inode *ip, s64 change,
>   			      kuid_t uid, kgid_t gid);
>   
> @@ -37,7 +44,8 @@ extern int gfs2_quotad(void *data);
>   
>   extern void gfs2_wake_up_statfs(struct gfs2_sbd *sdp);
>   
> -static inline int gfs2_quota_lock_check(struct gfs2_inode *ip, s64 exp_change)
> +static inline int gfs2_quota_lck_chk_ret_allow(struct gfs2_inode *ip,
> +					       s64 exp_change, u32 *allow)
>   {
>   	struct gfs2_sbd *sdp = GFS2_SB(&ip->i_inode);
>   	int ret;
> @@ -48,12 +56,18 @@ static inline int gfs2_quota_lock_check(struct gfs2_inode *ip, s64 exp_change)
>   		return ret;
>   	if (sdp->sd_args.ar_quota != GFS2_QUOTA_ON)
>   		return 0;
> -	ret = gfs2_quota_check(ip, ip->i_inode.i_uid, ip->i_inode.i_gid, exp_change);
> +	ret = gfs2_quota_chk_ret_allow(ip, ip->i_inode.i_uid, ip->i_inode.i_gid,
> +				       exp_change, allow);
>   	if (ret)
>   		gfs2_quota_unlock(ip);
>   	return ret;
>   }
>   
> +static inline int gfs2_quota_lock_check(struct gfs2_inode *ip, s64 exp_change)
> +{
> +	return gfs2_quota_lck_chk_ret_allow(ip, exp_change, NULL);
> +}
> +
>   extern const struct quotactl_ops gfs2_quotactl_ops;
>   extern struct shrinker gfs2_qd_shrinker;
>   extern struct list_lru gfs2_qd_lru;

[Cluster-devel] [GFS2 PATCH 3/4] gfs2: add new function gfs2_inpl_rsrv_ret_max_avl

[Abhi Das]

This is a variant of the existing gfs2_inplace_reserve() function.

If the requested number of blocks are not available to be reserved
from any of the rgrps, gfs2_inplace_reserve() return -ENOSPC.
gfs2_inpl_rsrv_ret_max_val() will also return the maximum blocks
available in an extra parameter 'max_avail'.

If acceptable to the caller logic, either of these inplace resreve
functions may be called again requesting 'max_avail' blocks to
avoid the -ENOSPC error.

Signed-off-by: Abhi Das <adas@redhat.com>
---
 fs/gfs2/rgrp.c | 13 +++++++++++--
 fs/gfs2/rgrp.h | 10 +++++++++-
 2 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/fs/gfs2/rgrp.c b/fs/gfs2/rgrp.c
index 9150207..0fa9ae3 100644
--- a/fs/gfs2/rgrp.c
+++ b/fs/gfs2/rgrp.c
@@ -1942,14 +1942,17 @@ static inline int fast_to_acquire(struct gfs2_rgrpd *rgd)
 }
 
 /**
- * gfs2_inplace_reserve - Reserve space in the filesystem
+ * gfs2_inpl_rsrv_ret_max_avl - Reserve space in the filesystem
  * @ip: the inode to reserve space for
  * @ap: the allocation parameters
+ * @max_avail: If non-NULL, return the max available extent if -ENOSPC
  *
  * Returns: errno
  */
 
-int gfs2_inplace_reserve(struct gfs2_inode *ip, const struct gfs2_alloc_parms *ap)
+int gfs2_inpl_rsrv_ret_max_avl(struct gfs2_inode *ip,
+			       const struct gfs2_alloc_parms *ap,
+			       u32 *max_avail)
 {
 	struct gfs2_sbd *sdp = GFS2_SB(&ip->i_inode);
 	struct gfs2_rgrpd *begin = NULL;
@@ -1958,6 +1961,7 @@ int gfs2_inplace_reserve(struct gfs2_inode *ip, const struct gfs2_alloc_parms *a
 	u64 last_unlinked = NO_BLOCK;
 	int loops = 0;
 	u32 skip = 0;
+	u32 max_avlbl = 0;
 
 	if (sdp->sd_args.ar_rgrplvb)
 		flags |= GL_SKIP;
@@ -2030,6 +2034,8 @@ int gfs2_inplace_reserve(struct gfs2_inode *ip, const struct gfs2_alloc_parms *a
 		if (rs->rs_rbm.rgd->rd_free_clone >= ap->target) {
 			ip->i_rgd = rs->rs_rbm.rgd;
 			return 0;
+		} else if (rs->rs_rbm.rgd->rd_free_clone > max_avlbl) {
+			max_avlbl = rs->rs_rbm.rgd->rd_free_clone;
 		}
 
 check_rgrp:
@@ -2068,6 +2074,9 @@ next_rgrp:
 			gfs2_log_flush(sdp, NULL, NORMAL_FLUSH);
 	}
 
+	if (max_avail)
+		*max_avail = max_avlbl;
+
 	return -ENOSPC;
 }
 
diff --git a/fs/gfs2/rgrp.h b/fs/gfs2/rgrp.h
index b104f4a..2adffca 100644
--- a/fs/gfs2/rgrp.h
+++ b/fs/gfs2/rgrp.h
@@ -41,7 +41,15 @@ extern void gfs2_rgrp_go_unlock(struct gfs2_holder *gh);
 extern struct gfs2_alloc *gfs2_alloc_get(struct gfs2_inode *ip);
 
 #define GFS2_AF_ORLOV 1
-extern int gfs2_inplace_reserve(struct gfs2_inode *ip, const struct gfs2_alloc_parms *ap);
+extern int gfs2_inpl_rsrv_ret_max_avl(struct gfs2_inode *ip,
+				      const struct gfs2_alloc_parms *ap,
+				      u32 *max_avail);
+static inline int gfs2_inplace_reserve(struct gfs2_inode *ip,
+				       const struct gfs2_alloc_parms *ap)
+{
+	return gfs2_inpl_rsrv_ret_max_avl(ip, ap, NULL);
+}
+
 extern void gfs2_inplace_release(struct gfs2_inode *ip);
 
 extern int gfs2_alloc_blocks(struct gfs2_inode *ip, u64 *bn, unsigned int *n,
-- 
1.8.1.4

[Cluster-devel] [GFS2 PATCH 3/4] gfs2: add new function gfs2_inpl_rsrv_ret_max_avl

[Steven Whitehouse]

Hi,

Likewise here, if the available blocks was added to the allocation 
parms, then this would help to neaten up the code a bit. Otherwise all 
four patches look good to me,

Steve.

On 12/02/15 16:54, Abhi Das wrote:
> This is a variant of the existing gfs2_inplace_reserve() function.
>
> If the requested number of blocks are not available to be reserved
> from any of the rgrps, gfs2_inplace_reserve() return -ENOSPC.
> gfs2_inpl_rsrv_ret_max_val() will also return the maximum blocks
> available in an extra parameter 'max_avail'.
>
> If acceptable to the caller logic, either of these inplace resreve
> functions may be called again requesting 'max_avail' blocks to
> avoid the -ENOSPC error.
>
> Signed-off-by: Abhi Das <adas@redhat.com>
> ---
>   fs/gfs2/rgrp.c | 13 +++++++++++--
>   fs/gfs2/rgrp.h | 10 +++++++++-
>   2 files changed, 20 insertions(+), 3 deletions(-)
>
> diff --git a/fs/gfs2/rgrp.c b/fs/gfs2/rgrp.c
> index 9150207..0fa9ae3 100644
> --- a/fs/gfs2/rgrp.c
> +++ b/fs/gfs2/rgrp.c
> @@ -1942,14 +1942,17 @@ static inline int fast_to_acquire(struct gfs2_rgrpd *rgd)
>   }
>   
>   /**
> - * gfs2_inplace_reserve - Reserve space in the filesystem
> + * gfs2_inpl_rsrv_ret_max_avl - Reserve space in the filesystem
>    * @ip: the inode to reserve space for
>    * @ap: the allocation parameters
> + * @max_avail: If non-NULL, return the max available extent if -ENOSPC
>    *
>    * Returns: errno
>    */
>   
> -int gfs2_inplace_reserve(struct gfs2_inode *ip, const struct gfs2_alloc_parms *ap)
> +int gfs2_inpl_rsrv_ret_max_avl(struct gfs2_inode *ip,
> +			       const struct gfs2_alloc_parms *ap,
> +			       u32 *max_avail)
>   {
>   	struct gfs2_sbd *sdp = GFS2_SB(&ip->i_inode);
>   	struct gfs2_rgrpd *begin = NULL;
> @@ -1958,6 +1961,7 @@ int gfs2_inplace_reserve(struct gfs2_inode *ip, const struct gfs2_alloc_parms *a
>   	u64 last_unlinked = NO_BLOCK;
>   	int loops = 0;
>   	u32 skip = 0;
> +	u32 max_avlbl = 0;
>   
>   	if (sdp->sd_args.ar_rgrplvb)
>   		flags |= GL_SKIP;
> @@ -2030,6 +2034,8 @@ int gfs2_inplace_reserve(struct gfs2_inode *ip, const struct gfs2_alloc_parms *a
>   		if (rs->rs_rbm.rgd->rd_free_clone >= ap->target) {
>   			ip->i_rgd = rs->rs_rbm.rgd;
>   			return 0;
> +		} else if (rs->rs_rbm.rgd->rd_free_clone > max_avlbl) {
> +			max_avlbl = rs->rs_rbm.rgd->rd_free_clone;
>   		}
>   
>   check_rgrp:
> @@ -2068,6 +2074,9 @@ next_rgrp:
>   			gfs2_log_flush(sdp, NULL, NORMAL_FLUSH);
>   	}
>   
> +	if (max_avail)
> +		*max_avail = max_avlbl;
> +
>   	return -ENOSPC;
>   }
>   
> diff --git a/fs/gfs2/rgrp.h b/fs/gfs2/rgrp.h
> index b104f4a..2adffca 100644
> --- a/fs/gfs2/rgrp.h
> +++ b/fs/gfs2/rgrp.h
> @@ -41,7 +41,15 @@ extern void gfs2_rgrp_go_unlock(struct gfs2_holder *gh);
>   extern struct gfs2_alloc *gfs2_alloc_get(struct gfs2_inode *ip);
>   
>   #define GFS2_AF_ORLOV 1
> -extern int gfs2_inplace_reserve(struct gfs2_inode *ip, const struct gfs2_alloc_parms *ap);
> +extern int gfs2_inpl_rsrv_ret_max_avl(struct gfs2_inode *ip,
> +				      const struct gfs2_alloc_parms *ap,
> +				      u32 *max_avail);
> +static inline int gfs2_inplace_reserve(struct gfs2_inode *ip,
> +				       const struct gfs2_alloc_parms *ap)
> +{
> +	return gfs2_inpl_rsrv_ret_max_avl(ip, ap, NULL);
> +}
> +
>   extern void gfs2_inplace_release(struct gfs2_inode *ip);
>   
>   extern int gfs2_alloc_blocks(struct gfs2_inode *ip, u64 *bn, unsigned int *n,

[Cluster-devel] [GFS2 PATCH 4/4] gfs2: allow fallocate to max out quotas/fs efficiently

[Abhi Das]

With the addition of gfs2_quota_lck_chk_ret_allow() and
gfs2_inpl_rsrv_ret_max_avl(), we can quickly get an estimate of
how many more blocks are available for allocation restricted by
quota and fs size respectively.
By trying to allocate what's available instead of guessing, we
can max out quotas or the filesystem efficiently.

Bear in mind that this applies only when the requested fallocate
operation would otherwise error out with -EDQUOT or -ENOSPC without
utilizing all the blocks that might still be available.

Signed-off-by: Abhi Das <adas@redhat.com>
---
 fs/gfs2/file.c | 24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/fs/gfs2/file.c b/fs/gfs2/file.c
index c9482ae..9d8e03a 100644
--- a/fs/gfs2/file.c
+++ b/fs/gfs2/file.c
@@ -805,6 +805,7 @@ static long __gfs2_fallocate(struct file *file, int mode, loff_t offset, loff_t
 	loff_t bsize_mask = ~((loff_t)sdp->sd_sb.sb_bsize - 1);
 	loff_t next = (offset + len - 1) >> sdp->sd_sb.sb_bsize_shift;
 	loff_t max_chunk_size = UINT_MAX & bsize_mask;
+	u32 allow;
 
 	next = (next + 1) << sdp->sd_sb.sb_bsize_shift;
 
@@ -828,20 +829,25 @@ static long __gfs2_fallocate(struct file *file, int mode, loff_t offset, loff_t
 			offset += bytes;
 			continue;
 		}
-		error = gfs2_quota_lock_check(ip, bytes >> sdp->sd_sb.sb_bsize_shift);
-		if (error)
+	quota_retry:
+		error = gfs2_quota_lck_chk_ret_allow(ip, 
+						     bytes >> sdp->sd_sb.sb_bsize_shift,
+						     &allow);
+		if (error) {
+			if (error == -EDQUOT && allow) {
+				bytes = allow << sdp->sd_sb.sb_bsize_shift;
+				goto quota_retry;
+			}
 			return error;
-retry:
+		}
+	retry:
 		gfs2_write_calc_reserv(ip, bytes, &data_blocks, &ind_blocks);
 
 		ap.target = data_blocks + ind_blocks;
-		error = gfs2_inplace_reserve(ip, &ap);
+		error = gfs2_inpl_rsrv_ret_max_avl(ip, &ap, &allow);
 		if (error) {
-			if (error == -ENOSPC && bytes > sdp->sd_sb.sb_bsize) {
-				bytes >>= 1;
-				bytes &= bsize_mask;
-				if (bytes == 0)
-					bytes = sdp->sd_sb.sb_bsize;
+			if (error == -ENOSPC && allow) {
+				bytes = allow << sdp->sd_sb.sb_bsize_shift;
 				goto retry;
 			}
 			goto out_qunlock;
-- 
1.8.1.4

[Cluster-devel] [GFS2 PATCH 0/4] fallocate quota fixes

[Bob Peterson]

----- Original Message -----
> These patches are related to bz1174295 where fallocate could
> exceed quota. I'm posting these for early feedback as these
> patches are only compile-tested so far.
> 
> patch 1 - This is the patch that actually addresses the quota
> 	  exceed issue. Quota checks were not being performed
> 	  against the blocks about to be allocated.
> 
> patch 2 - Adds new variants of quota check functions that
> 	  return the number of allowed blocks if quotas are
> 	  violated by the number of requested blocks
> 
> patch 3 - Adds a new variant of gfs2_inplace_reserve that
> 	  returns the max number of available blocks if the
> 	  function returns -ENOSPC due to unavailability of
> 	  the requested number of blocks.
> 
> patch 4 - Allows fallocate to take advantage of patches 2 and
> 	  3 to efficiently max out quotas or fill up the fs
> 	  instead of returning -EDQUOT/-ENOSPC and leaving some
> 	  available blocks unallocated.
> 
> Abhi Das (4):
>   gfs2: check quota for blocks we're about to allocate
>   gfs2: add new quota check functions
>   gfs2: add new function gfs2_inpl_rsrv_ret_max_avl
>   gfs2: allow fallocate to max out quotas/fs efficiently
> 
>  fs/gfs2/aops.c  |  6 +++---
>  fs/gfs2/bmap.c  |  2 +-
>  fs/gfs2/file.c  | 30 ++++++++++++++++++------------
>  fs/gfs2/inode.c | 14 ++++++++------
>  fs/gfs2/quota.c | 26 +++++++++++++++++++++-----
>  fs/gfs2/quota.h | 20 +++++++++++++++++---
>  fs/gfs2/rgrp.c  | 13 +++++++++++--
>  fs/gfs2/rgrp.h  | 10 +++++++++-
>  fs/gfs2/xattr.c |  2 +-
>  9 files changed, 89 insertions(+), 34 deletions(-)
> 
> --
> 1.8.1.4

Hi,

ACK. Looks okay to me.

Regards,

Bob Peterson
Red Hat File Systems