[PATCH] clk: Reorder fields in 'struct clk_fractional_divider'

[PATCH] clk: Reorder fields in 'struct clk_fractional_divider'

[Christophe JAILLET]

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(-)

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);
-- 
2.34.1

Re: [PATCH] clk: Reorder fields in 'struct clk_fractional_divider'

[Stephen Boyd]

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);

Re: [PATCH] clk: Reorder fields in 'struct clk_fractional_divider'

[Christophe JAILLET]

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);
>