[Cluster-devel] [GFS2 PATCH v2] GFS2: Limit number of transaction blocks requested for truncates

[Steven Whitehouse <swhiteho@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);
>