| Message ID | d1874eb8848d5f97f87337011188640a1463a666.1676649335.git.christophe.jaillet@wanadoo.fr (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | clk: Reorder fields in 'struct clk_fractional_divider' | expand |
Quoting Christophe JAILLET (2023-02-17 07:55:55) > diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h > index cf1adfeaf257..8cec844f5dde 100644 > --- a/include/linux/clk-provider.h > +++ b/include/linux/clk-provider.h > @@ -1134,13 +1134,13 @@ struct clk_hw *clk_hw_register_fixed_factor_parent_hw(struct device *dev, > struct clk_fractional_divider { > struct clk_hw hw; > void __iomem *reg; > + u8 flags; > u8 mshift; > u8 mwidth; > u32 mmask; This member ... > u8 nshift; > u8 nwidth; > u32 nmask; and this member can be calculated. Doing that would save more bytes when combined with this patch. Can you make this into a patch series? > - u8 flags; > void (*approximation)(struct clk_hw *hw, > unsigned long rate, unsigned long *parent_rate, > unsigned long *m, unsigned long *n);
Le 29/03/2023 à 20:46, Stephen Boyd a écrit : > Quoting Christophe JAILLET (2023-02-17 07:55:55) >> diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h >> index cf1adfeaf257..8cec844f5dde 100644 >> --- a/include/linux/clk-provider.h >> +++ b/include/linux/clk-provider.h >> @@ -1134,13 +1134,13 @@ struct clk_hw *clk_hw_register_fixed_factor_parent_hw(struct device *dev, >> struct clk_fractional_divider { >> struct clk_hw hw; >> void __iomem *reg; >> + u8 flags; >> u8 mshift; >> u8 mwidth; >> u32 mmask; > > This member ... > >> u8 nshift; >> u8 nwidth; >> u32 nmask; > > and this member can be calculated. Doing that would save more bytes when > combined with this patch. Can you make this into a patch series? Hi, with your proposed change, there is no more need to move flags. All u8 are grouped and the struct is optimal. I'll send a v2, but not sure it really worth it. When dynamically allocated, even when the struct is 56 bytes, 64 are still allocated. And there are only 2 users of struct clk_fractional_divider embedded in another struct that will be shrinked with your change. On the other side, having theses masks pre-computed could save a few cycles, at nearly no memory usage impact. I'll let you decide if v1 or v2 is the preferred way to go. CJ > >> - u8 flags; >> void (*approximation)(struct clk_hw *hw, >> unsigned long rate, unsigned long *parent_rate, >> unsigned long *m, unsigned long *n); >
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h index cf1adfeaf257..8cec844f5dde 100644 --- a/include/linux/clk-provider.h +++ b/include/linux/clk-provider.h @@ -1134,13 +1134,13 @@ struct clk_hw *clk_hw_register_fixed_factor_parent_hw(struct device *dev, struct clk_fractional_divider { struct clk_hw hw; void __iomem *reg; + u8 flags; u8 mshift; u8 mwidth; u32 mmask; u8 nshift; u8 nwidth; u32 nmask; - u8 flags; void (*approximation)(struct clk_hw *hw, unsigned long rate, unsigned long *parent_rate, unsigned long *m, unsigned long *n);
Group some variables based on their sizes to reduce hole and avoid padding. On x86_64, this shrinks the size of 'struct clk_fractional_divider' from 72 to 64 bytes. It saves a few bytes of memory when the structure is kzalloc()'ed. Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> --- Using pahole Before: ====== struct clk_fractional_divider { struct clk_hw hw; /* 0 24 */ void * reg; /* 24 8 */ u8 mshift; /* 32 1 */ u8 mwidth; /* 33 1 */ /* XXX 2 bytes hole, try to pack */ u32 mmask; /* 36 4 */ u8 nshift; /* 40 1 */ u8 nwidth; /* 41 1 */ /* XXX 2 bytes hole, try to pack */ u32 nmask; /* 44 4 */ u8 flags; /* 48 1 */ /* XXX 7 bytes hole, try to pack */ void (*approximation)(struct clk_hw *, long unsigned int, long unsigned int *, long unsigned int *, long unsigned int *); /* 56 8 */ /* --- cacheline 1 boundary (64 bytes) --- */ spinlock_t * lock; /* 64 8 */ /* size: 72, cachelines: 2, members: 11 */ /* sum members: 61, holes: 3, sum holes: 11 */ /* last cacheline: 8 bytes */ }; After: ===== struct clk_fractional_divider { struct clk_hw hw; /* 0 24 */ void * reg; /* 24 8 */ u8 flags; /* 32 1 */ u8 mshift; /* 33 1 */ u8 mwidth; /* 34 1 */ /* XXX 1 byte hole, try to pack */ u32 mmask; /* 36 4 */ u8 nshift; /* 40 1 */ u8 nwidth; /* 41 1 */ /* XXX 2 bytes hole, try to pack */ u32 nmask; /* 44 4 */ void (*approximation)(struct clk_hw *, long unsigned int, long unsigned int *, long unsigned int *, long unsigned int *); /* 48 8 */ spinlock_t * lock; /* 56 8 */ /* size: 64, cachelines: 1, members: 11 */ /* sum members: 61, holes: 2, sum holes: 3 */ }; --- include/linux/clk-provider.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)