[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20250728200644.3bcc1f3e@pumpkin>
Date: Mon, 28 Jul 2025 20:06:44 +0100
From: David Laight <david.laight.linux@...il.com>
To: Shuah Khan <skhan@...uxfoundation.org>
Cc: Niko Nikolov <nikolay.niko.nikolov@...il.com>, shuah@...nel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH] x86/tsc: Replace do_div() with div64_u64()/div64_ul()
On Thu, 24 Jul 2025 16:32:43 -0600
Shuah Khan <skhan@...uxfoundation.org> wrote:
> On 7/24/25 15:53, Niko Nikolov wrote:
> > Replace do_div() with the recommended division helpers to avoid
> > truncation and follow kernel best practices, as flagged by static
> > analysis.
>
> You are making more changes than what change log indicates.
>
> >
> > ./arch/x86/kernel/tsc.c:409:1-7:
> > WARNING: do_div() does a 64-by-32 division, please consider using div64_u64 instead.
> > ./arch/x86/kernel/tsc.c:492:1-7:
> > WARNING: do_div() does a 64-by-32 division, please consider using div64_ul instead.
> > ./arch/x86/kernel/tsc.c:831:2-8:
> > WARNING: do_div() does a 64-by-32 division, please consider using div64_ul instead.
>
> It is hard to see where you are addressing the above warnings with all
> the other changes.
>
> Also you don't have the right reviewers cc'ed
And the change is entirely broken...
David
>
> >
> > Signed-off-by: Niko Nikolov <nikolay.niko.nikolov@...il.com>
> > ---
> > arch/x86/kernel/tsc.c | 185 +++++++++++++++++++++---------------------
> > 1 file changed, 91 insertions(+), 94 deletions(-)
> >
> > diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
> > index 87e749106dda..96f40759340e 100644
> > --- a/arch/x86/kernel/tsc.c
> > +++ b/arch/x86/kernel/tsc.c
> > @@ -34,13 +34,13 @@
> > #include <asm/uv/uv.h>
> > #include <asm/sev.h>
> >
> > -unsigned int __read_mostly cpu_khz; /* TSC clocks / usec, not used here */
> > +unsigned int __read_mostly cpu_khz; /* TSC clocks / usec, not used here */
> > EXPORT_SYMBOL(cpu_khz);
> >
> > unsigned int __read_mostly tsc_khz;
> > EXPORT_SYMBOL(tsc_khz);
> >
> > -#define KHZ 1000
> > +#define KHZ 1000
>
> What changed here?
>
> >
> > /*
> > * TSC can be unstable due to cpufreq or due to unsynced TSCs
> > @@ -55,13 +55,13 @@ int tsc_clocksource_reliable;
> > static int __read_mostly tsc_force_recalibrate;
> >
> > static struct clocksource_base art_base_clk = {
> > - .id = CSID_X86_ART,
> > + .id = CSID_X86_ART,
>
> Tabs removed? Why?
>
> > };
> > static bool have_art;
> >
>
> Same questions for the rest of the format changes.
>
> thanks,
> -- Shuah
>
Powered by blists - more mailing lists