Re: [Cluster-devel] [GFS2 PATCH 1/3] gfs2: perform quota checks against allocation parameters
- Original Message -
From: Steven Whitehouse swhit...@redhat.com
To: Abhi Das a...@redhat.com, cluster-devel@redhat.com
Sent: Tuesday, February 17, 2015 3:38:07 AM
Subject: Re: [Cluster-devel] [GFS2 PATCH 1/3] gfs2: perform quota checks
against allocation parameters
Hi,
On 16/02/15 17:59, Abhi Das wrote:
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 a...@redhat.com
---
fs/gfs2/aops.c | 6 +++---
fs/gfs2/bmap.c | 2 +-
fs/gfs2/file.c | 9 +
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, 29 insertions(+), 24 deletions(-)
diff --git a/fs/gfs2/aops.c b/fs/gfs2/aops.c
index 805b37f..0261126 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 6e600ab..2ea420a 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;
@@ -828,7 +828,8 @@ static long __gfs2_fallocate(struct file *file, int
mode, loff_t offset, loff_t
offset += bytes;
continue;
}
- error = gfs2_quota_lock_check(ip);
+ ap.target = bytes sdp-sd_sb.sb_bsize_shift;
+ error = gfs2_quota_lock_check(ip, ap);
if (error)
return error;
retry:
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;
Why does this need to be 64 bits in size? The max size of an rgrp is
2^32, so there should be no need to represent an extent larger than that,
Steve.
This has got to do with setattr_chown() which calls gfs2_get_inode_blocks() to
get the number of blocks used by the inode being chowned. This is a u64 value
and since we also use ap.target to check against quotas in gfs2_quota_check(),
I had to change its size.
Cheers!
--Abhi
Re: [Cluster-devel] [GFS2 PATCH 1/3] gfs2: perform quota checks against allocation parameters
Hi,
On 16/02/15 17:59, Abhi Das wrote:
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 a...@redhat.com
---
fs/gfs2/aops.c | 6 +++---
fs/gfs2/bmap.c | 2 +-
fs/gfs2/file.c | 9 +
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, 29 insertions(+), 24 deletions(-)
diff --git a/fs/gfs2/aops.c b/fs/gfs2/aops.c
index 805b37f..0261126 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 6e600ab..2ea420a 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;
@@ -828,7 +828,8 @@ static long __gfs2_fallocate(struct file *file, int mode,
loff_t offset, loff_t
offset += bytes;
continue;
}
- error = gfs2_quota_lock_check(ip);
+ ap.target = bytes sdp-sd_sb.sb_bsize_shift;
+ error = gfs2_quota_lock_check(ip, ap);
if (error)
return error;
retry:
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;
Why does this need to be 64 bits in size? The max size of an rgrp is
2^32, so there should be no need to represent an extent larger than that,
Steve.
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);
[Cluster-devel] [GFS2 PATCH 1/3] gfs2: perform quota checks against allocation parameters
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 a...@redhat.com
---
fs/gfs2/aops.c | 6 +++---
fs/gfs2/bmap.c | 2 +-
fs/gfs2/file.c | 9 +
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, 29 insertions(+), 24 deletions(-)
diff --git a/fs/gfs2/aops.c b/fs/gfs2/aops.c
index 805b37f..0261126 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 6e600ab..2ea420a 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;
@@ -828,7 +828,8 @@ static long __gfs2_fallocate(struct file *file, int mode,
loff_t offset, loff_t
offset += bytes;
continue;
}
- error = gfs2_quota_lock_check(ip);
+ ap.target = bytes sdp-sd_sb.sb_bsize_shift;
+ error = gfs2_quota_lock_check(ip, ap);
if (error)
return error;
retry:
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) {
