Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <1d8df8d2-f44a-dd66-8e01-7e50fa147660@gmail.com>
Date:   Mon, 29 May 2023 23:23:45 +0900
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.10.0
Subject: Re: [PATCH] PM / devfreq: Reorder fields in 'struct
 devfreq_dev_status'
To:     Christophe JAILLET <christophe.jaillet@wanadoo.fr>,
        MyungJoo Ham <myungjoo.ham@samsung.com>,
        Kyungmin Park <kyungmin.park@samsung.com>,
        Chanwoo Choi <cw00.choi@samsung.com>
Cc:     linux-kernel@vger.kernel.org, kernel-janitors@vger.kernel.org,
        linux-pm@vger.kernel.org
References: <6b1611f1440c8b73feb475723ad25f390cad3480.1683531691.git.christophe.jaillet@wanadoo.fr>
From:   Chanwoo Choi <cwchoi00@gmail.com>
Content-Language: en-US
In-Reply-To: <6b1611f1440c8b73feb475723ad25f390cad3480.1683531691.git.christophe.jaillet@wanadoo.fr>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

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@wanadoo.fr>
> ---
> 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