[Cluster-devel] [PATCH] gfs2: Don't pack struct lm_lockname

[Andrew Price <anprice@redhat.com>]

On 15/03/17 18:48, Andreas Gruenbacher wrote:
> As per a suggestion by Linus, don't pack struct lm_lockname: we did that
> because the struct used as a rhashtable key, but packing tells the
> compiler that the 64-bit fields in the struct may be unaligned, causing
> it to generate worse code on some architectures.  Instead, rearrange the
> fields in the struct so that there is no padding between fields, and
> exclude any tail padding from the hash key size.
>
> Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>

Tested-by: Andrew Price <anprice@redhat.com>

Andy

> ---
>  fs/gfs2/glock.c  | 2 +-
>  fs/gfs2/incore.h | 8 ++++++--
>  2 files changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/fs/gfs2/glock.c b/fs/gfs2/glock.c
> index ec0848f..903c853 100644
> --- a/fs/gfs2/glock.c
> +++ b/fs/gfs2/glock.c
> @@ -73,7 +73,7 @@ static DEFINE_SPINLOCK(lru_lock);
>
>  static struct rhashtable_params ht_parms = {
>  	.nelem_hint = GFS2_GL_HASH_SIZE * 3 / 4,
> -	.key_len = sizeof(struct lm_lockname),
> +	.key_len = offsetofend(struct lm_lockname, ln_type),
>  	.key_offset = offsetof(struct gfs2_glock, gl_name),
>  	.head_offset = offsetof(struct gfs2_glock, gl_node),
>  };
> diff --git a/fs/gfs2/incore.h b/fs/gfs2/incore.h
> index 511e1ed..b7cf65d 100644
> --- a/fs/gfs2/incore.h
> +++ b/fs/gfs2/incore.h
> @@ -203,11 +203,15 @@ enum {
>  	DFL_DLM_RECOVERY	= 6,
>  };
>
> +/*
> + * We are using struct lm_lockname as an rhashtable key.  Avoid holes within
> + * the struct; padding at the end is fine.
> + */
>  struct lm_lockname {
> -	struct gfs2_sbd *ln_sbd;
>  	u64 ln_number;
> +	struct gfs2_sbd *ln_sbd;
>  	unsigned int ln_type;
> -} __packed __aligned(sizeof(int));
> +};
>
>  #define lm_name_equal(name1, name2) \
>          (((name1)->ln_number == (name2)->ln_number) &&	\
>