* [Cluster-devel] [GFS2 PATCH v2] GFS2: Limit number of transaction blocks requested for truncates
[not found] <556393196.14180052.1482159137998.JavaMail.zimbra@redhat.com>
@ 2016-12-19 14:52 ` Bob Peterson
2016-12-20 10:14 ` Steven Whitehouse
0 siblings, 1 reply; 2+ messages in thread
From: Bob Peterson @ 2016-12-19 14:52 UTC (permalink / raw)
To: cluster-devel.redhat.com
Hi,
On 16 December, I posted a predecessor patch for this problem.
Steve Whitehouse had suggested we base the number of allowable
blocks on the size of the journal rather than an arbitrary number.
This version 2 patch implements that suggestion:
---
GFS2: Limit number of transaction blocks requested for truncates
This patch limits the number of transaction blocks requested during
file truncates. If we have very large multi-terabyte files, and want
to delete or truncate them, they might span so many resource groups
that we overflow the journal blocks, and cause an assert failure.
By limiting the number of blocks in the transaction, we prevent this
overflow and give other running processes time to do transactions.
Signed-off-by: Bob Peterson <rpeterso@redhat.com>
---
diff --git a/fs/gfs2/bmap.c b/fs/gfs2/bmap.c
index 645721f..07d6ef1 100644
--- a/fs/gfs2/bmap.c
+++ b/fs/gfs2/bmap.c
@@ -720,6 +720,7 @@ static int do_strip(struct gfs2_inode *ip, struct buffer_head *dibh,
{
struct gfs2_sbd *sdp = GFS2_SB(&ip->i_inode);
struct gfs2_rgrp_list rlist;
+ struct gfs2_trans *tr;
u64 bn, bstart;
u32 blen, btotal;
__be64 *p;
@@ -728,6 +729,7 @@ static int do_strip(struct gfs2_inode *ip, struct buffer_head *dibh,
unsigned int revokes = 0;
int x;
int error;
+ int jblocks_rqsted;
error = gfs2_rindex_update(sdp);
if (error)
@@ -791,12 +793,17 @@ static int do_strip(struct gfs2_inode *ip, struct buffer_head *dibh,
if (gfs2_rs_active(&ip->i_res)) /* needs to be done with the rgrp glock held */
gfs2_rs_deltree(&ip->i_res);
- error = gfs2_trans_begin(sdp, rg_blocks + RES_DINODE +
- RES_INDIRECT + RES_STATFS + RES_QUOTA,
- revokes);
+restart:
+ jblocks_rqsted = rg_blocks + RES_DINODE +
+ RES_INDIRECT + RES_STATFS + RES_QUOTA +
+ gfs2_struct2blk(sdp, revokes, sizeof(u64));
+ if (jblocks_rqsted > sdp->sd_log_rsrv_max)
+ jblocks_rqsted = sdp->sd_log_rsrv_max;
+ error = gfs2_trans_begin(sdp, jblocks_rqsted, revokes);
if (error)
goto out_rg_gunlock;
+ tr = current->journal_info;
down_write(&ip->i_rw_mutex);
gfs2_trans_add_meta(ip->i_gl, dibh);
@@ -810,6 +817,16 @@ static int do_strip(struct gfs2_inode *ip, struct buffer_head *dibh,
if (!*p)
continue;
+ /* check for max reasonable journal transaction blocks */
+ if (tr->tr_num_buf_new + RES_STATFS +
+ RES_QUOTA >= sdp->sd_log_rsrv_max) {
+ if (rg_blocks >= tr->tr_num_buf_new)
+ rg_blocks -= tr->tr_num_buf_new;
+ else
+ rg_blocks = 0;
+ break;
+ }
+
bn = be64_to_cpu(*p);
if (bstart + blen == bn)
@@ -827,6 +844,9 @@ static int do_strip(struct gfs2_inode *ip, struct buffer_head *dibh,
*p = 0;
gfs2_add_inode_blocks(&ip->i_inode, -1);
}
+ if (p == bottom)
+ rg_blocks = 0;
+
if (bstart) {
__gfs2_free_blocks(ip, bstart, blen, metadata);
btotal += blen;
@@ -844,6 +864,9 @@ static int do_strip(struct gfs2_inode *ip, struct buffer_head *dibh,
gfs2_trans_end(sdp);
+ if (rg_blocks)
+ goto restart;
+
out_rg_gunlock:
gfs2_glock_dq_m(rlist.rl_rgrps, rlist.rl_ghs);
out_rlist:
diff --git a/fs/gfs2/incore.h b/fs/gfs2/incore.h
index a6a3389..55b076a 100644
--- a/fs/gfs2/incore.h
+++ b/fs/gfs2/incore.h
@@ -818,6 +818,8 @@ struct gfs2_sbd {
struct list_head sd_ail1_list;
struct list_head sd_ail2_list;
+ unsigned int sd_log_rsrv_max;
+
/* For quiescing the filesystem */
struct gfs2_holder sd_freeze_gh;
atomic_t sd_freeze_state;
diff --git a/fs/gfs2/super.c b/fs/gfs2/super.c
index e3ee387..4db5423 100644
--- a/fs/gfs2/super.c
+++ b/fs/gfs2/super.c
@@ -363,6 +363,8 @@ int gfs2_jdesc_check(struct gfs2_jdesc *jd)
return -EIO;
jd->jd_blocks = size >> sdp->sd_sb.sb_bsize_shift;
+ /* Allow transactions to reserve 3/4 of the journal at most */
+ sdp->sd_log_rsrv_max = (jd->jd_blocks >> 1) + (jd->jd_blocks >> 2);
if (gfs2_write_alloc_required(ip, 0, size)) {
gfs2_consist_inode(ip);
^ permalink raw reply related [flat|nested] 2+ messages in thread
* [Cluster-devel] [GFS2 PATCH v2] GFS2: Limit number of transaction blocks requested for truncates
2016-12-19 14:52 ` [Cluster-devel] [GFS2 PATCH v2] GFS2: Limit number of transaction blocks requested for truncates Bob Peterson
@ 2016-12-20 10:14 ` Steven Whitehouse
0 siblings, 0 replies; 2+ messages in thread
From: Steven Whitehouse @ 2016-12-20 10:14 UTC (permalink / raw)
To: cluster-devel.redhat.com
Hi,
That looks better. Thanks for fixing that up. I guess the next stage is
to move ahead with the longer term fix by sorting out the non-recursive
deletion which should then fix the issues here properly,
Steve.
On 19/12/16 14:52, Bob Peterson wrote:
> Hi,
>
> On 16 December, I posted a predecessor patch for this problem.
> Steve Whitehouse had suggested we base the number of allowable
> blocks on the size of the journal rather than an arbitrary number.
> This version 2 patch implements that suggestion:
> ---
> GFS2: Limit number of transaction blocks requested for truncates
>
> This patch limits the number of transaction blocks requested during
> file truncates. If we have very large multi-terabyte files, and want
> to delete or truncate them, they might span so many resource groups
> that we overflow the journal blocks, and cause an assert failure.
> By limiting the number of blocks in the transaction, we prevent this
> overflow and give other running processes time to do transactions.
>
> Signed-off-by: Bob Peterson <rpeterso@redhat.com>
> ---
> diff --git a/fs/gfs2/bmap.c b/fs/gfs2/bmap.c
> index 645721f..07d6ef1 100644
> --- a/fs/gfs2/bmap.c
> +++ b/fs/gfs2/bmap.c
> @@ -720,6 +720,7 @@ static int do_strip(struct gfs2_inode *ip, struct buffer_head *dibh,
> {
> struct gfs2_sbd *sdp = GFS2_SB(&ip->i_inode);
> struct gfs2_rgrp_list rlist;
> + struct gfs2_trans *tr;
> u64 bn, bstart;
> u32 blen, btotal;
> __be64 *p;
> @@ -728,6 +729,7 @@ static int do_strip(struct gfs2_inode *ip, struct buffer_head *dibh,
> unsigned int revokes = 0;
> int x;
> int error;
> + int jblocks_rqsted;
>
> error = gfs2_rindex_update(sdp);
> if (error)
> @@ -791,12 +793,17 @@ static int do_strip(struct gfs2_inode *ip, struct buffer_head *dibh,
> if (gfs2_rs_active(&ip->i_res)) /* needs to be done with the rgrp glock held */
> gfs2_rs_deltree(&ip->i_res);
>
> - error = gfs2_trans_begin(sdp, rg_blocks + RES_DINODE +
> - RES_INDIRECT + RES_STATFS + RES_QUOTA,
> - revokes);
> +restart:
> + jblocks_rqsted = rg_blocks + RES_DINODE +
> + RES_INDIRECT + RES_STATFS + RES_QUOTA +
> + gfs2_struct2blk(sdp, revokes, sizeof(u64));
> + if (jblocks_rqsted > sdp->sd_log_rsrv_max)
> + jblocks_rqsted = sdp->sd_log_rsrv_max;
> + error = gfs2_trans_begin(sdp, jblocks_rqsted, revokes);
> if (error)
> goto out_rg_gunlock;
>
> + tr = current->journal_info;
> down_write(&ip->i_rw_mutex);
>
> gfs2_trans_add_meta(ip->i_gl, dibh);
> @@ -810,6 +817,16 @@ static int do_strip(struct gfs2_inode *ip, struct buffer_head *dibh,
> if (!*p)
> continue;
>
> + /* check for max reasonable journal transaction blocks */
> + if (tr->tr_num_buf_new + RES_STATFS +
> + RES_QUOTA >= sdp->sd_log_rsrv_max) {
> + if (rg_blocks >= tr->tr_num_buf_new)
> + rg_blocks -= tr->tr_num_buf_new;
> + else
> + rg_blocks = 0;
> + break;
> + }
> +
> bn = be64_to_cpu(*p);
>
> if (bstart + blen == bn)
> @@ -827,6 +844,9 @@ static int do_strip(struct gfs2_inode *ip, struct buffer_head *dibh,
> *p = 0;
> gfs2_add_inode_blocks(&ip->i_inode, -1);
> }
> + if (p == bottom)
> + rg_blocks = 0;
> +
> if (bstart) {
> __gfs2_free_blocks(ip, bstart, blen, metadata);
> btotal += blen;
> @@ -844,6 +864,9 @@ static int do_strip(struct gfs2_inode *ip, struct buffer_head *dibh,
>
> gfs2_trans_end(sdp);
>
> + if (rg_blocks)
> + goto restart;
> +
> out_rg_gunlock:
> gfs2_glock_dq_m(rlist.rl_rgrps, rlist.rl_ghs);
> out_rlist:
> diff --git a/fs/gfs2/incore.h b/fs/gfs2/incore.h
> index a6a3389..55b076a 100644
> --- a/fs/gfs2/incore.h
> +++ b/fs/gfs2/incore.h
> @@ -818,6 +818,8 @@ struct gfs2_sbd {
> struct list_head sd_ail1_list;
> struct list_head sd_ail2_list;
>
> + unsigned int sd_log_rsrv_max;
> +
> /* For quiescing the filesystem */
> struct gfs2_holder sd_freeze_gh;
> atomic_t sd_freeze_state;
> diff --git a/fs/gfs2/super.c b/fs/gfs2/super.c
> index e3ee387..4db5423 100644
> --- a/fs/gfs2/super.c
> +++ b/fs/gfs2/super.c
> @@ -363,6 +363,8 @@ int gfs2_jdesc_check(struct gfs2_jdesc *jd)
> return -EIO;
>
> jd->jd_blocks = size >> sdp->sd_sb.sb_bsize_shift;
> + /* Allow transactions to reserve 3/4 of the journal at most */
> + sdp->sd_log_rsrv_max = (jd->jd_blocks >> 1) + (jd->jd_blocks >> 2);
>
> if (gfs2_write_alloc_required(ip, 0, size)) {
> gfs2_consist_inode(ip);
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2016-12-20 10:14 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <556393196.14180052.1482159137998.JavaMail.zimbra@redhat.com>
2016-12-19 14:52 ` [Cluster-devel] [GFS2 PATCH v2] GFS2: Limit number of transaction blocks requested for truncates Bob Peterson
2016-12-20 10:14 ` Steven Whitehouse
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.