From mboxrd@z Thu Jan  1 00:00:00 1970
From: Steven Whitehouse <swhiteho@redhat.com>
Date: Fri, 15 Feb 2019 11:56:53 +0000
Subject: [Cluster-devel] [GFS2 PATCH 5/9] gfs2: Keep transactions on
 ail1 list until after issuing revokes
In-Reply-To: <20190213152130.8047-6-rpeterso@redhat.com>
References: <20190213152130.8047-1-rpeterso@redhat.com>
	<20190213152130.8047-6-rpeterso@redhat.com>
Message-ID: <24111c35-dcdb-1671-9c17-544f45c0ad6d@redhat.com>
List-Id: <cluster-devel.redhat.com>
To: cluster-devel.redhat.com
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit

Hi,

On 13/02/2019 15:21, Bob Peterson wrote:
> Before this patch, function gfs2_write_revokes would call function
> gfs2_ail1_empty, then run the ail1 list, issuing revokes. But
> gfs2_ail1_empty can move transactions to the ail2 list, and thus,
> their revokes were never issued. This patch adds a new parameter to
> gfs2_ail1_empty that allows the transactions to remain on the ail1
> list until it can issue revokes for them. Then, if they have no more
> buffers, they're moved to the ail2 list after the revokes are added.
>
> Signed-off-by: Bob Peterson <rpeterso@redhat.com>

Why would we need to do this?

Steve.

> ---
>   fs/gfs2/log.c | 30 ++++++++++++++++++------------
>   1 file changed, 18 insertions(+), 12 deletions(-)
>
> diff --git a/fs/gfs2/log.c b/fs/gfs2/log.c
> index 81550038ace3..0d0dec3231c9 100644
> --- a/fs/gfs2/log.c
> +++ b/fs/gfs2/log.c
> @@ -217,11 +217,12 @@ static void gfs2_ail1_empty_one(struct gfs2_sbd *sdp, struct gfs2_trans *tr)
>   /**
>    * gfs2_ail1_empty - Try to empty the ail1 lists
>    * @sdp: The superblock
> + * @move_empty_to_ail2: 1 if transaction to be moved to ail2 when empty
>    *
>    * Tries to empty the ail1 lists, starting with the oldest first
>    */
>   
> -static int gfs2_ail1_empty(struct gfs2_sbd *sdp)
> +static int gfs2_ail1_empty(struct gfs2_sbd *sdp, bool move_empty_to_ail2)
>   {
>   	struct gfs2_trans *tr, *s;
>   	int oldest_tr = 1;
> @@ -230,10 +231,12 @@ static int gfs2_ail1_empty(struct gfs2_sbd *sdp)
>   	spin_lock(&sdp->sd_ail_lock);
>   	list_for_each_entry_safe_reverse(tr, s, &sdp->sd_ail1_list, tr_list) {
>   		gfs2_ail1_empty_one(sdp, tr);
> -		if (list_empty(&tr->tr_ail1_list) && oldest_tr)
> -			list_move(&tr->tr_list, &sdp->sd_ail2_list);
> -		else
> +		if (list_empty(&tr->tr_ail1_list) && oldest_tr) {
> +			if (move_empty_to_ail2)
> +				list_move(&tr->tr_list, &sdp->sd_ail2_list);
> +		} else {
>   			oldest_tr = 0;
> +		}
>   	}
>   	ret = list_empty(&sdp->sd_ail1_list);
>   	spin_unlock(&sdp->sd_ail_lock);
> @@ -609,12 +612,12 @@ void gfs2_add_revoke(struct gfs2_sbd *sdp, struct gfs2_bufdata *bd)
>   
>   void gfs2_write_revokes(struct gfs2_sbd *sdp)
>   {
> -	struct gfs2_trans *tr;
> +	struct gfs2_trans *tr, *s;
>   	struct gfs2_bufdata *bd, *tmp;
>   	int have_revokes = 0;
>   	int max_revokes = (sdp->sd_sb.sb_bsize - sizeof(struct gfs2_log_descriptor)) / sizeof(u64);
>   
> -	gfs2_ail1_empty(sdp);
> +	gfs2_ail1_empty(sdp, false);
>   	spin_lock(&sdp->sd_ail_lock);
>   	list_for_each_entry_reverse(tr, &sdp->sd_ail1_list, tr_list) {
>   		list_for_each_entry(bd, &tr->tr_ail2_list, bd_ail_st_list) {
> @@ -640,17 +643,20 @@ void gfs2_write_revokes(struct gfs2_sbd *sdp)
>   	}
>   	gfs2_log_lock(sdp);
>   	spin_lock(&sdp->sd_ail_lock);
> -	list_for_each_entry_reverse(tr, &sdp->sd_ail1_list, tr_list) {
> +	list_for_each_entry_safe_reverse(tr, s, &sdp->sd_ail1_list, tr_list) {
>   		list_for_each_entry_safe(bd, tmp, &tr->tr_ail2_list, bd_ail_st_list) {
>   			if (max_revokes == 0)
> -				goto out_of_blocks;
> +				break;
>   			if (!list_empty(&bd->bd_list))
>   				continue;
>   			gfs2_add_revoke(sdp, bd);
>   			max_revokes--;
>   		}
> +		if (list_empty(&tr->tr_ail1_list))
> +			list_move(&tr->tr_list, &sdp->sd_ail2_list);
> +		if (max_revokes == 0)
> +			break;
>   	}
> -out_of_blocks:
>   	spin_unlock(&sdp->sd_ail_lock);
>   	gfs2_log_unlock(sdp);
>   
> @@ -842,7 +848,7 @@ void gfs2_log_flush(struct gfs2_sbd *sdp, struct gfs2_glock *gl, u32 flags)
>   			for (;;) {
>   				gfs2_ail1_start(sdp);
>   				gfs2_ail1_wait(sdp);
> -				if (gfs2_ail1_empty(sdp))
> +				if (gfs2_ail1_empty(sdp, true))
>   					break;
>   			}
>   			atomic_dec(&sdp->sd_log_blks_free); /* Adjust for unreserved buffer */
> @@ -1008,7 +1014,7 @@ int gfs2_logd(void *data)
>   
>   		did_flush = false;
>   		if (gfs2_jrnl_flush_reqd(sdp) || t == 0) {
> -			gfs2_ail1_empty(sdp);
> +			gfs2_ail1_empty(sdp, true);
>   			if (test_bit(SDF_JOURNAL_LIVE, &sdp->sd_flags))
>   				gfs2_log_flush(sdp, NULL,
>   					       GFS2_LOG_HEAD_FLUSH_NORMAL |
> @@ -1019,7 +1025,7 @@ int gfs2_logd(void *data)
>   		if (gfs2_ail_flush_reqd(sdp)) {
>   			gfs2_ail1_start(sdp);
>   			gfs2_ail1_wait(sdp);
> -			gfs2_ail1_empty(sdp);
> +			gfs2_ail1_empty(sdp, true);
>   			if (test_bit(SDF_JOURNAL_LIVE, &sdp->sd_flags))
>   				gfs2_log_flush(sdp, NULL,
>   					       GFS2_LOG_HEAD_FLUSH_NORMAL |