Re: [PATCH] PM / devfreq: Reorder fields in 'struct devfreq_dev_status'

[Chanwoo Choi]

On 23. 5. 8. 16:42, Christophe JAILLET wrote:
> Group some variables based on their sizes to reduce holes.
> On x86_64, this shrinks the size of 'struct devfreq_dev_status' from 72 to
> 64 bytes.
> 
> This structure is used both to allocate static variables or is embedded in
> some other structures. In both cases, reducing its size is nice to have.
> 
> Moreover, the whole structure now fits in a single cache line on x86_64.
> 
> Finally, it makes the order of code match the order of the above kernel
> doc.
> 
> Signed-off-by: Christophe JAILLET <christophe.jaillet@xxxxxxxxxx>
> ---
> Using pahole
> 
> Before:
> ======
> struct devfreq_dev_profile {
> 	long unsigned int          initial_freq;         /*     0     8 */
> 	unsigned int               polling_ms;           /*     8     4 */
> 	enum devfreq_timer         timer;                /*    12     4 */
> 	bool                       is_cooling_device;    /*    16     1 */
> 
> 	/* XXX 7 bytes hole, try to pack */
> 
> 	int                        (*target)(struct device *, long unsigned int *, u32); /*    24     8 */
> 	int                        (*get_dev_status)(struct device *, struct devfreq_dev_status *); /*    32     8 */
> 	int                        (*get_cur_freq)(struct device *, long unsigned int *); /*    40     8 */
> 	void                       (*exit)(struct device *); /*    48     8 */
> 	long unsigned int *        freq_table;           /*    56     8 */
> 	/* --- cacheline 1 boundary (64 bytes) --- */
> 	unsigned int               max_state;            /*    64     4 */
> 
> 	/* size: 72, cachelines: 2, members: 10 */
> 	/* sum members: 61, holes: 1, sum holes: 7 */
> 	/* padding: 4 */
> 	/* last cacheline: 8 bytes */
> };
> 
> 
> After:
> =====
> struct devfreq_dev_profile {
> 	long unsigned int          initial_freq;         /*     0     8 */
> 	unsigned int               polling_ms;           /*     8     4 */
> 	enum devfreq_timer         timer;                /*    12     4 */
> 	int                        (*target)(struct device *, long unsigned int *, u32); /*    16     8 */
> 	int                        (*get_dev_status)(struct device *, struct devfreq_dev_status *); /*    24     8 */
> 	int                        (*get_cur_freq)(struct device *, long unsigned int *); /*    32     8 */
> 	void                       (*exit)(struct device *); /*    40     8 */
> 	long unsigned int *        freq_table;           /*    48     8 */
> 	unsigned int               max_state;            /*    56     4 */
> 	bool                       is_cooling_device;    /*    60     1 */
> 
> 	/* size: 64, cachelines: 1, members: 10 */
> 	/* padding: 3 */
> };
> ---
>  include/linux/devfreq.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
> index 7fd704bb8f3d..d312ffbac4dd 100644
> --- a/include/linux/devfreq.h
> +++ b/include/linux/devfreq.h
> @@ -108,7 +108,6 @@ struct devfreq_dev_profile {
>  	unsigned long initial_freq;
>  	unsigned int polling_ms;
>  	enum devfreq_timer timer;
> -	bool is_cooling_device;
>  
>  	int (*target)(struct device *dev, unsigned long *freq, u32 flags);
>  	int (*get_dev_status)(struct device *dev,
> @@ -118,6 +117,8 @@ struct devfreq_dev_profile {
>  
>  	unsigned long *freq_table;
>  	unsigned int max_state;
> +
> +	bool is_cooling_device;
>  };
>  
>  /**


Applied it.

Thanks.

-- 
Best Regards,
Samsung Electronics
Chanwoo Choi