linux-kernel - Re: [PATCH 1/1] char: hpet: Remove unused local variable 'm' in hpet

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <YHhTVXS4u5jXG5Fc@kroah.com>
Date:   Thu, 15 Apr 2021 16:53:09 +0200
From:   Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To:     Zhen Lei <thunder.leizhen@...wei.com>
Cc:     Clemens Ladisch <clemens@...isch.de>,
        Arnd Bergmann <arnd@...db.de>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        Nils Carlson <nils.carlson@...csson.com>,
        Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH 1/1] char: hpet: Remove unused local variable 'm' in
 hpet_interrupt()

On Thu, Apr 15, 2021 at 10:24:04PM +0800, Zhen Lei wrote:
> Commit 273ef9509b79 ("drivers/char/hpet.c: fix periodic-emulation for
> delayed interrupt") removed the reference to local variable 'm', but
> forgot to remove the definition and assignment of it. Due to
> read_counter() indirectly calls "read barrier", the performance is
> slightly degraded.
> 
> Since the following comments give some description based on 'm', so move
> the assignment of 'm' into it.
> 
> Fixes: 273ef9509b79 ("drivers/char/hpet.c: fix periodic-emulation for delayed interrupt")
> Reported-by: Hulk Robot <hulkci@...wei.com>
> Signed-off-by: Zhen Lei <thunder.leizhen@...wei.com>
> ---
>  drivers/char/hpet.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/char/hpet.c b/drivers/char/hpet.c
> index ed3b7dab678dbd1..46950a0cda181a1 100644
> --- a/drivers/char/hpet.c
> +++ b/drivers/char/hpet.c
> @@ -156,14 +156,16 @@ static irqreturn_t hpet_interrupt(int irq, void *data)
>  	 * This has the effect of treating non-periodic like periodic.
>  	 */
>  	if ((devp->hd_flags & (HPET_IE | HPET_PERIODIC)) == HPET_IE) {
> -		unsigned long m, t, mc, base, k;
> +		unsigned long t, mc, base, k;
>  		struct hpet __iomem *hpet = devp->hd_hpet;
>  		struct hpets *hpetp = devp->hd_hpets;
>  
>  		t = devp->hd_ireqfreq;
> -		m = read_counter(&devp->hd_timer->hpet_compare);
>  		mc = read_counter(&hpet->hpet_mc);
> -		/* The time for the next interrupt would logically be t + m,
> +		/*
> +		 * m = read_counter(&devp->hd_timer->hpet_compare);

Why did you comment this out?

And are you sure that yuou are not required to actually read that
counter, even if you do not do anything with the value?  Lots of
hardware works in odd ways...

Have you tested and verified that this still works properly?

thanks,

greg k-h