From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Price Date: Thu, 16 Mar 2017 12:18:03 +0000 Subject: [Cluster-devel] [PATCH] gfs2: Don't pack struct lm_lockname In-Reply-To: <1489603738-27603-1-git-send-email-agruenba@redhat.com> References: <1489603738-27603-1-git-send-email-agruenba@redhat.com> Message-ID: List-Id: To: cluster-devel.redhat.com MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit 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 Tested-by: Andrew Price 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) && \ >