Re: [PATCH 1/1] char: hpet: Remove unused local variable 'm' in hpet_interrupt()

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

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@huawei.com>
> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.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