mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: David Laight <David.Laight@ACULAB.COM>
To: 'Bo Ye' <bo.ye@mediatek.com>,
	"Rafael J. Wysocki" <rafael@kernel.org>,
	Daniel Lezcano <daniel.lezcano@linaro.org>,
	Matthias Brugger <matthias.bgg@gmail.com>,
	AngeloGioacchino Del Regno
	<angelogioacchino.delregno@collabora.com>
Cc: "yongdong.zhang@mediatek.com" <yongdong.zhang@mediatek.com>,
	C Cheng <C.Cheng@mediatek.com>,
	"linux-pm@vger.kernel.org" <linux-pm@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	"linux-mediatek@lists.infradead.org"
	<linux-mediatek@lists.infradead.org>
Subject: RE: [PATCH v2] fix overflow in idle exit_latency
Date: Tue, 19 Dec 2023 16:24:17 +0000	[thread overview]
Message-ID: <e807b180b074419ba4f1d1ff8a4c1a80@AcuMS.aculab.com> (raw)
In-Reply-To: <20231219031444.91752-1-bo.ye@mediatek.com>

From: Bo Ye 
> Sent: 19 December 2023 03:15
> 
> From: C Cheng <C.Cheng@mediatek.com>
> 
> In detail:
> 
> In C language, when you perform a multiplication operation, if
> both operands are of int type, the multiplication operation is
> performed on the int type, and then the result is converted to
> the target type. This means that if the product of int type
> multiplication exceeds the range that int type can represent,
>  an overflow will occur even if you store the result in a
> variable of int64_t type.
> 
> For a multiplication of two int values, it is better to use
> mul_u32_u32() rather than s->exit_latency_ns = s->exit_latency *
> NSEC_PER_USEC to avoid potential overflow happenning.
> 
> Signed-off-by: C Cheng <C.Cheng@mediatek.com>
> Signed-off-by: Bo Ye <bo.ye@mediatek.com>
> ---
> Change in v2:
> -remove Change-ID
> -correct patch author name
> -replace multiplication of two int values with mul_u32_u32
> -refine commit message
> ---
>  drivers/cpuidle/driver.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/cpuidle/driver.c b/drivers/cpuidle/driver.c
> index d9cda7f6ccb9..cf5873cc45dc 100644
> --- a/drivers/cpuidle/driver.c
> +++ b/drivers/cpuidle/driver.c
> @@ -16,6 +16,7 @@
>  #include <linux/cpumask.h>
>  #include <linux/tick.h>
>  #include <linux/cpu.h>
> +#include <linux/math64.h>
> 
>  #include "cpuidle.h"
> 
> @@ -187,7 +188,7 @@ static void __cpuidle_driver_init(struct cpuidle_driver *drv)
>  			s->target_residency = div_u64(s->target_residency_ns, NSEC_PER_USEC);
> 
>  		if (s->exit_latency > 0)
> -			s->exit_latency_ns = s->exit_latency * NSEC_PER_USEC;
> +			s->exit_latency_ns = mul_u32_u32(s->exit_latency, NSEC_PER_USEC);

Just force either side of the multiply to a 64bit unsigned type.
The compiler will then DTRT which is likely to be better code than
whatever mul_u32_u32() generates.

In any case is the 'exit_latency' ever going to be greater than 4 seconds?
In which case the 32bit multiply will never overflow.

	David

>  		else if (s->exit_latency_ns < 0)
>  			s->exit_latency_ns =  0;
>  		else
> --
> 2.18.0
> 

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)


  parent reply	other threads:[~2023-12-19 16:24 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-12-19  3:14 Bo Ye
2023-12-19 10:56 ` AngeloGioacchino Del Regno
2024-02-12 16:09   ` Rafael J. Wysocki
2023-12-19 16:24 ` David Laight [this message]
2023-12-19 16:30   ` Rafael J. Wysocki

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=e807b180b074419ba4f1d1ff8a4c1a80@AcuMS.aculab.com \
    --to=david.laight@aculab.com \
    --cc=C.Cheng@mediatek.com \
    --cc=angelogioacchino.delregno@collabora.com \
    --cc=bo.ye@mediatek.com \
    --cc=daniel.lezcano@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mediatek@lists.infradead.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=matthias.bgg@gmail.com \
    --cc=rafael@kernel.org \
    --cc=yongdong.zhang@mediatek.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®