[Cluster-devel] [PATCH 10/12] gfs2: Generalize truncate code

[Andreas Gruenbacher]

On 9 January 2018 at 16:45, Bob Peterson <rpeterso at redhat.com> wrote:
> Hi,
>
> ----- Original Message -----
> | Pull the code for computing the range of metapointers to iterate out of
> | gfs2_metapath_ra (for readahead), sweep_bh_for_rgrps (for deallocating
> | metapointers within a block), and trunc_dealloc (for walking the
> | metadata tree).
> |
> | In sweep_bh_for_rgrps, move the code for looking up the resource group
> | descriptor of the current resource group out of the inner loop.  The
> | metatype check moves to trunc_dealloc.
> |
> | Signed-off-by: Andreas Gruenbacher <agruenba at redhat.com>
> | ---
> (snip)
> | @@ -1229,7 +1216,11 @@ static int sweep_bh_for_rgrps(struct gfs2_inode *ip,
> | struct gfs2_holder *rd_gh,
> |                                           outside the rgrp we just processed,
> |                                           do it all over again. */
> |               if (current->journal_info) {
> | -                     struct buffer_head *dibh = mp->mp_bh[0];
> | +                     struct buffer_head *dibh;
> | +
> | +                     ret = gfs2_meta_inode_buffer(ip, &dibh);
> | +                     if (ret)
> | +                             goto out;
> (snip)...
> | +                     brelse(dibh);
>
> I don't understand why you prefer to use gfs2_meta_inode_buffer here
> instead of just using the copy we already have in the metapath.
> It seems to me this method would be a lot less efficient.
> Can you explain?

This patch removes the metapath parameter of sweep_bh_for_rgrps as
well. We could pass in the dibh in a separate parameter, but we only
need it when an indirect block spans multiple resource groups, which
should be relatively rare. Even then, we'll also be switching resource
group glocks, and so an additional bh lookup won't have a measurable
impact.

Thanks,
Andreas