Re: [PATCH] x86/tsc: Replace do_div() with div64_u64()/div64_ul()

[David Laight <david.laight.linux@gmail.com>]

On Thu, 24 Jul 2025 16:32:43 -0600
Shuah Khan <skhan@linuxfoundation.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@gmail.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
>