[Cluster-devel] [GFS2 PATCH 07/12] gfs2: make gfs2_ail1_empty_one return the count of active items

[Andreas Gruenbacher]

Hi Bob,

On Fri, Aug 21, 2020 at 7:33 PM Bob Peterson <rpeterso at redhat.com> wrote:
> This patch is one baby step toward simplifying the journal management.
> It simply changes function gfs2_ail1_empty_one from a void to an int and
> makes it return a count of active items. This allows the caller to check
> the return code rather than list_empty on the tr_ail1_list. This way
> we can, in a later patch, combine transaction ail1 and ail2 lists.

what does "we can combine transaction ail1 and ail2 lists" mean?

> Signed-off-by: Bob Peterson <rpeterso at redhat.com>
> ---
>  fs/gfs2/log.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/fs/gfs2/log.c b/fs/gfs2/log.c
> index 3e24e8733950..4fb1a96b8124 100644
> --- a/fs/gfs2/log.c
> +++ b/fs/gfs2/log.c
> @@ -244,13 +244,15 @@ static void gfs2_ail1_start(struct gfs2_sbd *sdp)
>   * @tr: the transaction
>   * @max_revokes: If nonzero, issue revokes for the bd items for written buffers
>   *
> + * returns: the transaction's count of remaining active items
>   */
>
> -static void gfs2_ail1_empty_one(struct gfs2_sbd *sdp, struct gfs2_trans *tr,
> +static int gfs2_ail1_empty_one(struct gfs2_sbd *sdp, struct gfs2_trans *tr,
>                                 int *max_revokes)
>  {
>         struct gfs2_bufdata *bd, *s;
>         struct buffer_head *bh;
> +       int active_count = 0;
>
>         list_for_each_entry_safe_reverse(bd, s, &tr->tr_ail1_list,
>                                          bd_ail_st_list) {
> @@ -265,8 +267,10 @@ static void gfs2_ail1_empty_one(struct gfs2_sbd *sdp, struct gfs2_trans *tr,
>                  * If the ail buffer is not busy and caught an error, flag it
>                  * for others.
>                  */
> -               if (!sdp->sd_log_error && buffer_busy(bh))
> +               if (!sdp->sd_log_error && buffer_busy(bh)) {
> +                       active_count++;
>                         continue;
> +               }
>                 if (!buffer_uptodate(bh) &&
>                     !cmpxchg(&sdp->sd_log_error, 0, -EIO)) {
>                         gfs2_io_error_bh(sdp, bh);
> @@ -285,6 +289,7 @@ static void gfs2_ail1_empty_one(struct gfs2_sbd *sdp, struct gfs2_trans *tr,
>                 }
>                 list_move(&bd->bd_ail_st_list, &tr->tr_ail2_list);
>         }
> +       return active_count;
>  }
>
>  /**
> @@ -303,8 +308,7 @@ static int gfs2_ail1_empty(struct gfs2_sbd *sdp, int max_revokes)
>
>         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, &max_revokes);
> -               if (list_empty(&tr->tr_ail1_list) && oldest_tr)
> +               if (!gfs2_ail1_empty_one(sdp, tr, &max_revokes) && oldest_tr)
>                         list_move(&tr->tr_list, &sdp->sd_ail2_list);
>                 else
>                         oldest_tr = 0;
> --
> 2.26.2
>

Thanks,
Andreas