gfs2: Don't pack struct lm_lockname
Andreas Gruenbacher
maintainerPATCH 1/1 gfs2: Don't pack struct lm_lockname
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.
--- fs/gfs2/glock.c | 2 +- fs/gfs2/incore.h | 8 ++++++-- 2 files changed, 7 insertions(+), 3 deletions(-)
fs/gfs2/glock.c +1 -1
@@ -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), };
fs/gfs2/incore.h +6 -2
@@ -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) && \
Signature · 1 line
2.7.4
PATCH 1/1 gfs2: Don't pack struct lm_lockname
Quoted 8 lines · Andreas Gruenbacher
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 <[email protected]>
Andy
Quoted 41 lines
--- fs/gfs2/glock.c | 2 +- fs/gfs2/incore.h | 8 ++++++-- 2 files changed, 7 insertions(+), 3 deletions(-)fs/gfs2/glock.c +1
-1@@ -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), };fs/gfs2/incore.h +6
-2@@ -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) && \
PATCH 1/1 gfs2: Don't pack struct lm_lockname
In reply to Andreas Gruenbacher----- Original Message -----
| 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 <[email protected]> | ---
Hi,
Thanks. This is now applied to the for-next branch of the linux-gfs2 tree:
https://git.kernel.org/pub/scm/linux/kernel/git/gfs2/linux-gfs2.git/commit/?h=for-next&id=972b044eeca582ef173d063773e292bd9e8633d6
Regards,
Bob Peterson
Red Hat File Systems