* [Cluster-devel] [GFS2 1/3] gfs2: perform quota checks against allocation parameters
2015-03-18 7:36 [Cluster-devel] [GFS2 0/3] fallocate and quota fixes Abhi Das
@ 2015-03-18 7:36 ` Abhi Das
2015-03-18 7:36 ` [Cluster-devel] [GFS2 2/3] gfs2: allow quota_check and inplace_reserve to return available blocks Abhi Das
` (3 subsequent siblings)
4 siblings, 0 replies; 9+ messages in thread
From: Abhi Das @ 2015-03-18 7:36 UTC (permalink / raw)
To: cluster-devel.redhat.com
Use struct gfs2_alloc_parms as an argument to gfs2_quota_check()
and gfs2_quota_lock_check() to check for quota violations while
accounting for the new blocks requested by the current operation
in ap->target.
Previously, the number of new blocks requested during an operation
were not accounted for during quota_check and would allow these
operations to exceed quota. This was not very apparent since most
operations allocated only 1 block at a time and quotas would get
violated in the next operation. i.e. quota excess would only be by
1 block or so. With fallocate, (where we allocate a bunch of blocks
at once) the quota excess is non-trivial and is addressed by this
patch.
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 | 15 ++++++++-------
fs/gfs2/incore.h | 2 +-
fs/gfs2/inode.c | 18 ++++++++++--------
fs/gfs2/quota.c | 6 +++---
fs/gfs2/quota.h | 8 +++++---
fs/gfs2/xattr.c | 2 +-
8 files changed, 32 insertions(+), 27 deletions(-)
diff --git a/fs/gfs2/aops.c b/fs/gfs2/aops.c
index 4ad4f94..7bc5c82 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);
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..61296ec 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);
if (error)
return error;
diff --git a/fs/gfs2/file.c b/fs/gfs2/file.c
index 7353c0a..c569adb 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);
+ if (ret)
+ goto out_unlock;
ret = gfs2_inplace_reserve(ip, &ap);
if (ret)
goto out_quota_unlock;
@@ -827,13 +827,13 @@ static long __gfs2_fallocate(struct file *file, int mode, loff_t offset, loff_t
offset += bytes;
continue;
}
- error = gfs2_quota_lock_check(ip);
- if (error)
- return error;
retry:
gfs2_write_calc_reserv(ip, bytes, &data_blocks, &ind_blocks);
-
ap.target = data_blocks + ind_blocks;
+
+ error = gfs2_quota_lock_check(ip, &ap);
+ if (error)
+ return error;
error = gfs2_inplace_reserve(ip, &ap);
if (error) {
if (error == -ENOSPC && bytes > sdp->sd_sb.sb_bsize) {
@@ -841,6 +841,7 @@ retry:
bytes &= bsize_mask;
if (bytes == 0)
bytes = sdp->sd_sb.sb_bsize;
+ gfs2_quota_unlock(ip);
goto retry;
}
goto out_qunlock;
diff --git a/fs/gfs2/incore.h b/fs/gfs2/incore.h
index 7a2dbbc..3a4ea50 100644
--- a/fs/gfs2/incore.h
+++ b/fs/gfs2/incore.h
@@ -301,7 +301,7 @@ struct gfs2_blkreserv {
* to the allocation code.
*/
struct gfs2_alloc_parms {
- u32 target;
+ u64 target;
u32 aflags;
};
diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c
index 73c72253..08bc84d 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);
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);
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);
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);
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;
+ struct gfs2_alloc_parms ap;
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;
+ ap.target = 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, &ap);
if (error)
goto out_gunlock_q;
}
@@ -1713,9 +1716,8 @@ 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);
+ gfs2_quota_change(ip, -ap.target, ouid, ogid);
+ gfs2_quota_change(ip, ap.target, nuid, ngid);
}
out_end_trans:
diff --git a/fs/gfs2/quota.c b/fs/gfs2/quota.c
index 3aa17d4..964a769 100644
--- a/fs/gfs2/quota.c
+++ b/fs/gfs2/quota.c
@@ -1094,7 +1094,8 @@ 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,
+ struct gfs2_alloc_parms *ap)
{
struct gfs2_sbd *sdp = GFS2_SB(&ip->i_inode);
struct gfs2_quota_data *qd;
@@ -1117,14 +1118,13 @@ 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 + ap->target;
spin_unlock(&qd_lock);
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;
break;
} else if (be64_to_cpu(qd->qd_qb.qb_warn) &&
diff --git a/fs/gfs2/quota.h b/fs/gfs2/quota.h
index 55d506e..ad04b3a 100644
--- a/fs/gfs2/quota.h
+++ b/fs/gfs2/quota.h
@@ -24,7 +24,8 @@ 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,
+ struct gfs2_alloc_parms *ap);
extern void gfs2_quota_change(struct gfs2_inode *ip, s64 change,
kuid_t uid, kgid_t gid);
@@ -37,7 +38,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)
+static inline int gfs2_quota_lock_check(struct gfs2_inode *ip,
+ struct gfs2_alloc_parms *ap)
{
struct gfs2_sbd *sdp = GFS2_SB(&ip->i_inode);
int ret;
@@ -48,7 +50,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, ap);
if (ret)
gfs2_quota_unlock(ip);
return ret;
diff --git a/fs/gfs2/xattr.c b/fs/gfs2/xattr.c
index 0b81f78..fd260ce 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);
if (error)
return error;
--
1.8.1.4
^ permalink raw reply related [flat|nested] 9+ messages in thread* [Cluster-devel] [GFS2 2/3] gfs2: allow quota_check and inplace_reserve to return available blocks
2015-03-18 7:36 [Cluster-devel] [GFS2 0/3] fallocate and quota fixes Abhi Das
2015-03-18 7:36 ` [Cluster-devel] [GFS2 1/3] gfs2: perform quota checks against allocation parameters Abhi Das
@ 2015-03-18 7:36 ` Abhi Das
2015-03-18 7:36 ` [Cluster-devel] [GFS2 3/3] gfs2: allow fallocate to max out quotas/fs efficiently Abhi Das
` (2 subsequent siblings)
4 siblings, 0 replies; 9+ messages in thread
From: Abhi Das @ 2015-03-18 7:36 UTC (permalink / raw)
To: cluster-devel.redhat.com
struct gfs2_alloc_parms is passed to gfs2_quota_check() and
gfs2_inplace_reserve() with ap->target containing the number of
blocks being requested for allocation in the current operation.
We add a new field to struct gfs2_alloc_parms called 'allowed'.
gfs2_quota_check() and gfs2_inplace_reserve() return the max
blocks allowed by quota and the max blocks allowed by the chosen
rgrp respectively in 'allowed'.
A new field 'min_target', when non-zero, tells gfs2_quota_check()
and gfs2_inplace_reserve() to not return -EDQUOT/-ENOSPC when
there are atleast 'min_target' blocks allowable/available. The
assumption is that the caller is ok with just 'min_target' blocks
and will likely proceed with allocating them.
Signed-off-by: Abhi Das <adas@redhat.com>
---
fs/gfs2/incore.h | 2 ++
fs/gfs2/quota.c | 52 +++++++++++++++++++++++++++++++++++++++-------------
fs/gfs2/rgrp.c | 20 +++++++++++++++-----
fs/gfs2/rgrp.h | 3 ++-
4 files changed, 58 insertions(+), 19 deletions(-)
diff --git a/fs/gfs2/incore.h b/fs/gfs2/incore.h
index 3a4ea50..58b75ab 100644
--- a/fs/gfs2/incore.h
+++ b/fs/gfs2/incore.h
@@ -302,7 +302,9 @@ struct gfs2_blkreserv {
*/
struct gfs2_alloc_parms {
u64 target;
+ u32 min_target;
u32 aflags;
+ u64 allowed;
};
enum {
diff --git a/fs/gfs2/quota.c b/fs/gfs2/quota.c
index 964a769..5561468 100644
--- a/fs/gfs2/quota.c
+++ b/fs/gfs2/quota.c
@@ -1094,15 +1094,33 @@ static int print_message(struct gfs2_quota_data *qd, char *type)
return 0;
}
+/**
+ * gfs2_quota_check - check if allocating new blocks will exceed quota
+ * @ip: The inode for which this check is being performed
+ * @uid: The uid to check against
+ * @gid: The gid to check against
+ * @ap: The allocation parameters. ap->target contains the requested
+ * blocks. ap->min_target, if set, contains the minimum blks
+ * requested.
+ *
+ * Returns: 0 on success.
+ * min_req = ap->min_target ? ap->min_target : ap->target;
+ * quota must allow atleast min_req blks for success and
+ * ap->allowed is set to the number of blocks allowed
+ *
+ * -EDQUOT otherwise, quota violation. ap->allowed is set to number
+ * of blocks available.
+ */
int gfs2_quota_check(struct gfs2_inode *ip, kuid_t uid, kgid_t gid,
struct gfs2_alloc_parms *ap)
{
struct gfs2_sbd *sdp = GFS2_SB(&ip->i_inode);
struct gfs2_quota_data *qd;
- s64 value;
+ s64 value, warn, limit;
unsigned int x;
int error = 0;
+ ap->allowed = UINT_MAX; /* Assume we are permitted a whole lot */
if (!test_bit(GIF_QD_LOCKED, &ip->i_flags))
return 0;
@@ -1116,29 +1134,37 @@ int gfs2_quota_check(struct gfs2_inode *ip, kuid_t uid, kgid_t gid,
qid_eq(qd->qd_id, make_kqid_gid(gid))))
continue;
+ warn = (s64)be64_to_cpu(qd->qd_qb.qb_warn);
+ limit = (s64)be64_to_cpu(qd->qd_qb.qb_limit);
value = (s64)be64_to_cpu(qd->qd_qb.qb_value);
spin_lock(&qd_lock);
- value += qd->qd_change + ap->target;
+ value += qd->qd_change;
spin_unlock(&qd_lock);
- 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;
- break;
- } else if (be64_to_cpu(qd->qd_qb.qb_warn) &&
- (s64)be64_to_cpu(qd->qd_qb.qb_warn) < value &&
+ if (limit > 0 && (limit - value) < ap->allowed)
+ ap->allowed = limit - value;
+ /* If we can't meet the target */
+ if (limit && limit < (value + (s64)ap->target)) {
+ /* If no min_target specified or we don't meet
+ * min_target, return -EDQUOT */
+ if (!ap->min_target || ap->min_target > ap->allowed) {
+ print_message(qd, "exceeded");
+ quota_send_warning(qd->qd_id,
+ sdp->sd_vfs->s_dev,
+ QUOTA_NL_BHARDWARN);
+ error = -EDQUOT;
+ break;
+ }
+ } else if (warn && warn < value &&
time_after_eq(jiffies, qd->qd_last_warn +
- gfs2_tune_get(sdp,
- gt_quota_warn_period) * HZ)) {
+ gfs2_tune_get(sdp, gt_quota_warn_period)
+ * HZ)) {
quota_send_warning(qd->qd_id,
sdp->sd_vfs->s_dev, QUOTA_NL_BSOFTWARN);
error = print_message(qd, "warning");
qd->qd_last_warn = jiffies;
}
}
-
return error;
}
diff --git a/fs/gfs2/rgrp.c b/fs/gfs2/rgrp.c
index 9150207..6af2396 100644
--- a/fs/gfs2/rgrp.c
+++ b/fs/gfs2/rgrp.c
@@ -1946,10 +1946,18 @@ static inline int fast_to_acquire(struct gfs2_rgrpd *rgd)
* @ip: the inode to reserve space for
* @ap: the allocation parameters
*
- * Returns: errno
+ * We try our best to find an rgrp that has at least ap->target blocks
+ * available. After a couple of passes (loops == 2), the prospects of finding
+ * such an rgrp diminish. At this stage, we return the first rgrp that has
+ * atleast ap->min_target blocks available. Either way, we set ap->allowed to
+ * the number of blocks available in the chosen rgrp.
+ *
+ * Returns: 0 on success,
+ * -ENOMEM if a suitable rgrp can't be found
+ * errno otherwise
*/
-int gfs2_inplace_reserve(struct gfs2_inode *ip, const struct gfs2_alloc_parms *ap)
+int gfs2_inplace_reserve(struct gfs2_inode *ip, struct gfs2_alloc_parms *ap)
{
struct gfs2_sbd *sdp = GFS2_SB(&ip->i_inode);
struct gfs2_rgrpd *begin = NULL;
@@ -2012,7 +2020,7 @@ int gfs2_inplace_reserve(struct gfs2_inode *ip, const struct gfs2_alloc_parms *a
/* Skip unuseable resource groups */
if ((rs->rs_rbm.rgd->rd_flags & (GFS2_RGF_NOALLOC |
GFS2_RDF_ERROR)) ||
- (ap->target > rs->rs_rbm.rgd->rd_extfail_pt))
+ (loops == 0 && ap->target > rs->rs_rbm.rgd->rd_extfail_pt))
goto skip_rgrp;
if (sdp->sd_args.ar_rgrplvb)
@@ -2027,11 +2035,13 @@ int gfs2_inplace_reserve(struct gfs2_inode *ip, const struct gfs2_alloc_parms *a
goto check_rgrp;
/* If rgrp has enough free space, use it */
- if (rs->rs_rbm.rgd->rd_free_clone >= ap->target) {
+ if (rs->rs_rbm.rgd->rd_free_clone >= ap->target ||
+ (loops == 2 && ap->min_target &&
+ rs->rs_rbm.rgd->rd_free_clone >= ap->min_target)) {
ip->i_rgd = rs->rs_rbm.rgd;
+ ap->allowed = ip->i_rgd->rd_free_clone;
return 0;
}
-
check_rgrp:
/* Check for unlinked inodes which can be reclaimed */
if (rs->rs_rbm.rgd->rd_flags & GFS2_RDF_CHECK)
diff --git a/fs/gfs2/rgrp.h b/fs/gfs2/rgrp.h
index b104f4a..68972ec 100644
--- a/fs/gfs2/rgrp.h
+++ b/fs/gfs2/rgrp.h
@@ -41,7 +41,8 @@ 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_inplace_reserve(struct gfs2_inode *ip,
+ struct gfs2_alloc_parms *ap);
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
^ permalink raw reply related [flat|nested] 9+ messages in thread* [Cluster-devel] [GFS2 3/3] gfs2: allow fallocate to max out quotas/fs efficiently
2015-03-18 7:36 [Cluster-devel] [GFS2 0/3] fallocate and quota fixes Abhi Das
2015-03-18 7:36 ` [Cluster-devel] [GFS2 1/3] gfs2: perform quota checks against allocation parameters Abhi Das
2015-03-18 7:36 ` [Cluster-devel] [GFS2 2/3] gfs2: allow quota_check and inplace_reserve to return available blocks Abhi Das
@ 2015-03-18 7:36 ` Abhi Das
2015-03-18 13:40 ` [Cluster-devel] [GFS2 0/3] fallocate and quota fixes Steven Whitehouse
2015-03-18 17:09 ` Bob Peterson
4 siblings, 0 replies; 9+ messages in thread
From: Abhi Das @ 2015-03-18 7:36 UTC (permalink / raw)
To: cluster-devel.redhat.com
We can quickly get an estimate of how many blocks are available
for allocation restricted by quota and fs size respectively, using
the ap->allowed field in the gfs2_alloc_parms structure.
gfs2_quota_check() and gfs2_inplace_reserve() provide these values.
Once we have the total number of blocks available to us, we can
compute how many bytes of data can be written using those blocks
instead of guessing inefficiently.
Signed-off-by: Abhi Das <adas@redhat.com>
---
fs/gfs2/file.c | 70 +++++++++++++++++++++++++++++++++++++++-------------------
1 file changed, 47 insertions(+), 23 deletions(-)
diff --git a/fs/gfs2/file.c b/fs/gfs2/file.c
index c569adb..4d31087 100644
--- a/fs/gfs2/file.c
+++ b/fs/gfs2/file.c
@@ -765,22 +765,30 @@ out:
brelse(dibh);
return error;
}
-
-static void calc_max_reserv(struct gfs2_inode *ip, loff_t max, loff_t *len,
- unsigned int *data_blocks, unsigned int *ind_blocks)
+/**
+ * calc_max_reserv() - Reverse of write_calc_reserv. Given a number of
+ * blocks, determine how many bytes can be written.
+ * @ip: The inode in question.
+ * @len: Max cap of bytes. What we return in *len must be <= this.
+ * @data_blocks: Compute and return the number of data blocks needed
+ * @ind_blocks: Compute and return the number of indirect blocks needed
+ * @max_blocks: The total blocks available to work with.
+ *
+ * Returns: void, but @len, @data_blocks and @ind_blocks are filled in.
+ */
+static void calc_max_reserv(struct gfs2_inode *ip, loff_t *len,
+ unsigned int *data_blocks, unsigned int *ind_blocks,
+ unsigned int max_blocks)
{
+ loff_t max = *len;
const struct gfs2_sbd *sdp = GFS2_SB(&ip->i_inode);
- unsigned int max_blocks = ip->i_rgd->rd_free_clone;
unsigned int tmp, max_data = max_blocks - 3 * (sdp->sd_max_height - 1);
for (tmp = max_data; tmp > sdp->sd_diptrs;) {
tmp = DIV_ROUND_UP(tmp, sdp->sd_inptrs);
max_data -= tmp;
}
- /* This calculation isn't the exact reverse of gfs2_write_calc_reserve,
- so it might end up with fewer data blocks */
- if (max_data <= *data_blocks)
- return;
+
*data_blocks = max_data;
*ind_blocks = max_blocks - max_data;
*len = ((loff_t)max_data - 3) << sdp->sd_sb.sb_bsize_shift;
@@ -797,7 +805,7 @@ static long __gfs2_fallocate(struct file *file, int mode, loff_t offset, loff_t
struct gfs2_inode *ip = GFS2_I(inode);
struct gfs2_alloc_parms ap = { .aflags = 0, };
unsigned int data_blocks = 0, ind_blocks = 0, rblocks;
- loff_t bytes, max_bytes;
+ loff_t bytes, max_bytes, max_blks = UINT_MAX;
int error;
const loff_t pos = offset;
const loff_t count = len;
@@ -819,6 +827,9 @@ static long __gfs2_fallocate(struct file *file, int mode, loff_t offset, loff_t
gfs2_size_hint(file, offset, len);
+ gfs2_write_calc_reserv(ip, PAGE_SIZE, &data_blocks, &ind_blocks);
+ ap.min_target = data_blocks + ind_blocks;
+
while (len > 0) {
if (len < bytes)
bytes = len;
@@ -827,28 +838,41 @@ static long __gfs2_fallocate(struct file *file, int mode, loff_t offset, loff_t
offset += bytes;
continue;
}
-retry:
+
+ /* We need to determine how many bytes we can actually
+ * fallocate without exceeding quota or going over the
+ * end of the fs. We start off optimistically by assuming
+ * we can write max_bytes */
+ max_bytes = (len > max_chunk_size) ? max_chunk_size : len;
+
+ /* Since max_bytes is most likely a theoretical max, we
+ * calculate a more realistic 'bytes' to serve as a good
+ * starting point for the number of bytes we may be able
+ * to write */
gfs2_write_calc_reserv(ip, bytes, &data_blocks, &ind_blocks);
ap.target = data_blocks + ind_blocks;
error = gfs2_quota_lock_check(ip, &ap);
if (error)
return error;
+ /* ap.allowed tells us how many blocks quota will allow
+ * us to write. Check if this reduces max_blks */
+ if (ap.allowed && ap.allowed < max_blks)
+ max_blks = ap.allowed;
+
error = gfs2_inplace_reserve(ip, &ap);
- 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;
- gfs2_quota_unlock(ip);
- goto retry;
- }
+ if (error)
goto out_qunlock;
- }
- max_bytes = bytes;
- calc_max_reserv(ip, (len > max_chunk_size)? max_chunk_size: len,
- &max_bytes, &data_blocks, &ind_blocks);
+
+ /* check if the selected rgrp limits our max_blks further */
+ if (ap.allowed && ap.allowed < max_blks)
+ max_blks = ap.allowed;
+
+ /* Almost done. Calculate bytes that can be written using
+ * max_blks. We also recompute max_bytes, data_blocks and
+ * ind_blocks */
+ calc_max_reserv(ip, &max_bytes, &data_blocks,
+ &ind_blocks, max_blks);
rblocks = RES_DINODE + ind_blocks + RES_STATFS + RES_QUOTA +
RES_RG_HDR + gfs2_rg_blocks(ip, data_blocks + ind_blocks);
--
1.8.1.4
^ permalink raw reply related [flat|nested] 9+ messages in thread* [Cluster-devel] [GFS2 0/3] fallocate and quota fixes
2015-03-18 7:36 [Cluster-devel] [GFS2 0/3] fallocate and quota fixes Abhi Das
` (2 preceding siblings ...)
2015-03-18 7:36 ` [Cluster-devel] [GFS2 3/3] gfs2: allow fallocate to max out quotas/fs efficiently Abhi Das
@ 2015-03-18 13:40 ` Steven Whitehouse
2015-03-18 17:09 ` Bob Peterson
4 siblings, 0 replies; 9+ messages in thread
From: Steven Whitehouse @ 2015-03-18 13:40 UTC (permalink / raw)
To: cluster-devel.redhat.com
Hi,
Acked-by: Steven Whitehouse <swhiteho@redhat.com>
Steve.
On 18/03/15 07:36, Abhi Das wrote:
> This is a revised version of the patches required to properly fix the
> fallocate quota issue described in bz1174295
>
> patch1: This patch supplies gfs2_quota_check() with the number of blocks
> the caller intends to allocate in the current operation, resulting
> in a more accurate quota check.
>
> patch2: gfs2_quota_check() and gfs2_inplace_reserve() return the number
> of blocks available subject to quota limits and rgrp size
> respectively. These functions don't return error if atleast
> ap.min_target (if set) blocks are allocatable.
>
> patch3: The fallocate() function uses the features of patch2 to determine
> how many total blocks are available for allocation and uses them
> right away, instead of guessing/retrying inefficiently.
>
> The behavior of quota enforcement is altered by this patchset.
> i. Quotas are exceeded (a warning message is also issued to syslog) before
> the actual usage blocks exceed the imposed limit. In fact, the actual
> usage can never exceed the limit. Whenever it is determined that the
> completion of an operation will cause a quota to exceed its limit, such
> an operation is aborted with -EDQUOT and a 'quota exceeded' message is
> dispatched. Note: When min_target is set and allowed blocks are >=
> min_target, we don't issue an error. It is assumed that the caller will
> only allocate the allowed blocks.
> ii. The gfs2_write_calc_reserv()/calc_max_reserv() functions are used to
> map between available blocks and the data bytes that can be written
> using them. Typically, for large files, some blocks are used up for
> metadata and only the remaining blocks can be used for data. Example:
> To write only a handful of bytes that would easily fit in one block, we
> might have to allocate an extra bunch of intermediate metadata blocks.
> If we had only 1 block left in our allotted quota, this operation would
> likely fail.
>
> The functions mentioned in ii. are not very efficient. They always compute
> the worst case number of extra blocks required and it is often the case that
> not all those extra blocks are used. We need to find a better algorithm to
> get a tighter estimate on the blocks needed for a given number of bytes.
>
> I've run some basic tests and things seem to be holding up. The failing case
> in bz1174295 is fixed using this patchset. I'll do test build and pass it on
> to Nate to test with.
>
> Abhi Das (3):
> gfs2: perform quota checks against allocation parameters
> gfs2: allow quota_check and inplace_reserve to return available blocks
> gfs2: allow fallocate to max out quotas/fs efficiently
>
> fs/gfs2/aops.c | 6 ++---
> fs/gfs2/bmap.c | 2 +-
> fs/gfs2/file.c | 81 ++++++++++++++++++++++++++++++++++++--------------------
> fs/gfs2/incore.h | 4 ++-
> fs/gfs2/inode.c | 18 +++++++------
> fs/gfs2/quota.c | 54 +++++++++++++++++++++++++++----------
> fs/gfs2/quota.h | 8 +++---
> fs/gfs2/rgrp.c | 20 ++++++++++----
> fs/gfs2/rgrp.h | 3 ++-
> fs/gfs2/xattr.c | 2 +-
> 10 files changed, 133 insertions(+), 65 deletions(-)
>
^ permalink raw reply [flat|nested] 9+ messages in thread* [Cluster-devel] [GFS2 0/3] fallocate and quota fixes
2015-03-18 7:36 [Cluster-devel] [GFS2 0/3] fallocate and quota fixes Abhi Das
` (3 preceding siblings ...)
2015-03-18 13:40 ` [Cluster-devel] [GFS2 0/3] fallocate and quota fixes Steven Whitehouse
@ 2015-03-18 17:09 ` Bob Peterson
4 siblings, 0 replies; 9+ messages in thread
From: Bob Peterson @ 2015-03-18 17:09 UTC (permalink / raw)
To: cluster-devel.redhat.com
----- Original Message -----
> This is a revised version of the patches required to properly fix the
> fallocate quota issue described in bz1174295
>
> patch1: This patch supplies gfs2_quota_check() with the number of blocks
> the caller intends to allocate in the current operation, resulting
> in a more accurate quota check.
>
> patch2: gfs2_quota_check() and gfs2_inplace_reserve() return the number
> of blocks available subject to quota limits and rgrp size
> respectively. These functions don't return error if atleast
> ap.min_target (if set) blocks are allocatable.
>
> patch3: The fallocate() function uses the features of patch2 to determine
> how many total blocks are available for allocation and uses them
> right away, instead of guessing/retrying inefficiently.
>
> The behavior of quota enforcement is altered by this patchset.
> i. Quotas are exceeded (a warning message is also issued to syslog) before
> the actual usage blocks exceed the imposed limit. In fact, the actual
> usage can never exceed the limit. Whenever it is determined that the
> completion of an operation will cause a quota to exceed its limit, such
> an operation is aborted with -EDQUOT and a 'quota exceeded' message is
> dispatched. Note: When min_target is set and allowed blocks are >=
> min_target, we don't issue an error. It is assumed that the caller will
> only allocate the allowed blocks.
> ii. The gfs2_write_calc_reserv()/calc_max_reserv() functions are used to
> map between available blocks and the data bytes that can be written
> using them. Typically, for large files, some blocks are used up for
> metadata and only the remaining blocks can be used for data. Example:
> To write only a handful of bytes that would easily fit in one block, we
> might have to allocate an extra bunch of intermediate metadata blocks.
> If we had only 1 block left in our allotted quota, this operation would
> likely fail.
>
> The functions mentioned in ii. are not very efficient. They always compute
> the worst case number of extra blocks required and it is often the case that
> not all those extra blocks are used. We need to find a better algorithm to
> get a tighter estimate on the blocks needed for a given number of bytes.
>
> I've run some basic tests and things seem to be holding up. The failing case
> in bz1174295 is fixed using this patchset. I'll do test build and pass it on
> to Nate to test with.
>
> Abhi Das (3):
> gfs2: perform quota checks against allocation parameters
> gfs2: allow quota_check and inplace_reserve to return available blocks
> gfs2: allow fallocate to max out quotas/fs efficiently
>
> fs/gfs2/aops.c | 6 ++---
> fs/gfs2/bmap.c | 2 +-
> fs/gfs2/file.c | 81
> ++++++++++++++++++++++++++++++++++++--------------------
> fs/gfs2/incore.h | 4 ++-
> fs/gfs2/inode.c | 18 +++++++------
> fs/gfs2/quota.c | 54 +++++++++++++++++++++++++++----------
> fs/gfs2/quota.h | 8 +++---
> fs/gfs2/rgrp.c | 20 ++++++++++----
> fs/gfs2/rgrp.h | 3 ++-
> fs/gfs2/xattr.c | 2 +-
> 10 files changed, 133 insertions(+), 65 deletions(-)
>
> --
> 1.8.1.4
Hi,
ACK to all three.
Now pushed to the for-next branch of the linux-gfs2.git git tree.
Regards,
Bob Peterson
^ permalink raw reply [flat|nested] 9+ messages in thread