2021-11-11 23:06:06

by kernel test robot

[permalink] [raw]
Subject: drivers/clocksource/timer-pistachio.c:74:22: warning: variable 'overflow' set but not used

Hi Drew,

FYI, the error/warning still remains.

tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
head: debe436e77c72fcee804fb867f275e6d31aa999c
commit: a47d7ef4550d08fb428ea4c3f1a9c71674212208 clocksource/drivers/pistachio: Fix trivial typo
date: 7 months ago
config: mips-randconfig-r012-20210927 (attached as .config)
compiler: mips-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=a47d7ef4550d08fb428ea4c3f1a9c71674212208
git remote add linus https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
git fetch --no-tags linus master
git checkout a47d7ef4550d08fb428ea4c3f1a9c71674212208
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross ARCH=mips

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>

All warnings (new ones prefixed by >>):

drivers/clocksource/timer-pistachio.c: In function 'pistachio_clocksource_read_cycles':
>> drivers/clocksource/timer-pistachio.c:74:22: warning: variable 'overflow' set but not used [-Wunused-but-set-variable]
74 | u32 counter, overflow;
| ^~~~~~~~


vim +/overflow +74 drivers/clocksource/timer-pistachio.c

69
70 static u64 notrace
71 pistachio_clocksource_read_cycles(struct clocksource *cs)
72 {
73 struct pistachio_clocksource *pcs = to_pistachio_clocksource(cs);
> 74 u32 counter, overflow;
75 unsigned long flags;
76
77 /*
78 * The counter value is only refreshed after the overflow value is read.
79 * And they must be read in strict order, hence raw spin lock added.
80 */
81
82 raw_spin_lock_irqsave(&pcs->lock, flags);
83 overflow = gpt_readl(pcs->base, TIMER_CURRENT_OVERFLOW_VALUE, 0);
84 counter = gpt_readl(pcs->base, TIMER_CURRENT_VALUE, 0);
85 raw_spin_unlock_irqrestore(&pcs->lock, flags);
86
87 return (u64)~counter;
88 }
89

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]


Attachments:
(No filename) (2.34 kB)
.config.gz (28.20 kB)
Download all attachments

2021-11-14 21:30:13

by Drew Fustini

[permalink] [raw]
Subject: Re: drivers/clocksource/timer-pistachio.c:74:22: warning: variable 'overflow' set but not used

On Fri, Nov 12, 2021 at 07:05:48AM +0800, kernel test robot wrote:
> Hi Drew,
>
> FYI, the error/warning still remains.
>
> tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> head: debe436e77c72fcee804fb867f275e6d31aa999c
> commit: a47d7ef4550d08fb428ea4c3f1a9c71674212208 clocksource/drivers/pistachio: Fix trivial typo
> date: 7 months ago
> config: mips-randconfig-r012-20210927 (attached as .config)
> compiler: mips-linux-gcc (GCC) 11.2.0
> reproduce (this is a W=1 build):
> wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=a47d7ef4550d08fb428ea4c3f1a9c71674212208
> git remote add linus https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
> git fetch --no-tags linus master
> git checkout a47d7ef4550d08fb428ea4c3f1a9c71674212208
> # save the attached .config to linux build tree
> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross ARCH=mips
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <[email protected]>
>
> All warnings (new ones prefixed by >>):
>
> drivers/clocksource/timer-pistachio.c: In function 'pistachio_clocksource_read_cycles':
> >> drivers/clocksource/timer-pistachio.c:74:22: warning: variable 'overflow' set but not used [-Wunused-but-set-variable]
> 74 | u32 counter, overflow;
> | ^~~~~~~~
>
>
> vim +/overflow +74 drivers/clocksource/timer-pistachio.c
>
> 69
> 70 static u64 notrace
> 71 pistachio_clocksource_read_cycles(struct clocksource *cs)
> 72 {
> 73 struct pistachio_clocksource *pcs = to_pistachio_clocksource(cs);
> > 74 u32 counter, overflow;
> 75 unsigned long flags;
> 76
> 77 /*
> 78 * The counter value is only refreshed after the overflow value is read.
> 79 * And they must be read in strict order, hence raw spin lock added.
> 80 */
> 81
> 82 raw_spin_lock_irqsave(&pcs->lock, flags);
> 83 overflow = gpt_readl(pcs->base, TIMER_CURRENT_OVERFLOW_VALUE, 0);

I fail to see how variable 'overflow' set but not used as the return
value of gpt_readl() is unconditionally assigned to 'overflow'.

Am I missing something?

Thanks,
Drew

2021-11-14 22:24:29

by Daniel Lezcano

[permalink] [raw]
Subject: Re: drivers/clocksource/timer-pistachio.c:74:22: warning: variable 'overflow' set but not used

On 14/11/2021 22:29, Drew Fustini wrote:
> On Fri, Nov 12, 2021 at 07:05:48AM +0800, kernel test robot wrote:
>> Hi Drew,
>>
>> FYI, the error/warning still remains.
>>
>> tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
>> head: debe436e77c72fcee804fb867f275e6d31aa999c
>> commit: a47d7ef4550d08fb428ea4c3f1a9c71674212208 clocksource/drivers/pistachio: Fix trivial typo
>> date: 7 months ago
>> config: mips-randconfig-r012-20210927 (attached as .config)
>> compiler: mips-linux-gcc (GCC) 11.2.0
>> reproduce (this is a W=1 build):
>> wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>> chmod +x ~/bin/make.cross
>> # https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=a47d7ef4550d08fb428ea4c3f1a9c71674212208
>> git remote add linus https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
>> git fetch --no-tags linus master
>> git checkout a47d7ef4550d08fb428ea4c3f1a9c71674212208
>> # save the attached .config to linux build tree
>> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross ARCH=mips
>>
>> If you fix the issue, kindly add following tag as appropriate
>> Reported-by: kernel test robot <[email protected]>
>>
>> All warnings (new ones prefixed by >>):
>>
>> drivers/clocksource/timer-pistachio.c: In function 'pistachio_clocksource_read_cycles':
>>>> drivers/clocksource/timer-pistachio.c:74:22: warning: variable 'overflow' set but not used [-Wunused-but-set-variable]
>> 74 | u32 counter, overflow;
>> | ^~~~~~~~
>>
>>
>> vim +/overflow +74 drivers/clocksource/timer-pistachio.c
>>
>> 69
>> 70 static u64 notrace
>> 71 pistachio_clocksource_read_cycles(struct clocksource *cs)
>> 72 {
>> 73 struct pistachio_clocksource *pcs = to_pistachio_clocksource(cs);
>> > 74 u32 counter, overflow;
>> 75 unsigned long flags;
>> 76
>> 77 /*
>> 78 * The counter value is only refreshed after the overflow value is read.
>> 79 * And they must be read in strict order, hence raw spin lock added.
>> 80 */
>> 81
>> 82 raw_spin_lock_irqsave(&pcs->lock, flags);
>> 83 overflow = gpt_readl(pcs->base, TIMER_CURRENT_OVERFLOW_VALUE, 0);

overflow is set here but then never reused in the function. So a call to
gpt_readl without getting the return value is fine.

The warning is different from 'variable is unused'

> I fail to see how variable 'overflow' set but not used as the return
> value of gpt_readl() is unconditionally assigned to 'overflow'.




--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

2021-11-15 01:03:08

by Drew Fustini

[permalink] [raw]
Subject: Re: drivers/clocksource/timer-pistachio.c:74:22: warning: variable 'overflow' set but not used

On Sun, Nov 14, 2021 at 11:17:38PM +0100, Daniel Lezcano wrote:
> On 14/11/2021 22:29, Drew Fustini wrote:
> > On Fri, Nov 12, 2021 at 07:05:48AM +0800, kernel test robot wrote:
> >> Hi Drew,
> >>
> >> FYI, the error/warning still remains.
> >>
> >> tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> >> head: debe436e77c72fcee804fb867f275e6d31aa999c
> >> commit: a47d7ef4550d08fb428ea4c3f1a9c71674212208 clocksource/drivers/pistachio: Fix trivial typo
> >> date: 7 months ago
> >> config: mips-randconfig-r012-20210927 (attached as .config)
> >> compiler: mips-linux-gcc (GCC) 11.2.0
> >> reproduce (this is a W=1 build):
> >> wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> >> chmod +x ~/bin/make.cross
> >> # https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=a47d7ef4550d08fb428ea4c3f1a9c71674212208
> >> git remote add linus https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
> >> git fetch --no-tags linus master
> >> git checkout a47d7ef4550d08fb428ea4c3f1a9c71674212208
> >> # save the attached .config to linux build tree
> >> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross ARCH=mips
> >>
> >> If you fix the issue, kindly add following tag as appropriate
> >> Reported-by: kernel test robot <[email protected]>
> >>
> >> All warnings (new ones prefixed by >>):
> >>
> >> drivers/clocksource/timer-pistachio.c: In function 'pistachio_clocksource_read_cycles':
> >>>> drivers/clocksource/timer-pistachio.c:74:22: warning: variable 'overflow' set but not used [-Wunused-but-set-variable]
> >> 74 | u32 counter, overflow;
> >> | ^~~~~~~~
> >>
> >>
> >> vim +/overflow +74 drivers/clocksource/timer-pistachio.c
> >>
> >> 69
> >> 70 static u64 notrace
> >> 71 pistachio_clocksource_read_cycles(struct clocksource *cs)
> >> 72 {
> >> 73 struct pistachio_clocksource *pcs = to_pistachio_clocksource(cs);
> >> > 74 u32 counter, overflow;
> >> 75 unsigned long flags;
> >> 76
> >> 77 /*
> >> 78 * The counter value is only refreshed after the overflow value is read.
> >> 79 * And they must be read in strict order, hence raw spin lock added.
> >> 80 */
> >> 81
> >> 82 raw_spin_lock_irqsave(&pcs->lock, flags);
> >> 83 overflow = gpt_readl(pcs->base, TIMER_CURRENT_OVERFLOW_VALUE, 0);
>
> overflow is set here but then never reused in the function. So a call to
> gpt_readl without getting the return value is fine.
>
> The warning is different from 'variable is unused'

Thank you for pointing that out. I was reading it incorrectly.

As for this function, it seems that warning is a false positive as
the act of reading TIMER_CURRENT_OVERFLOW_VALUE causes the hardware
to refresh TIMER_CURRENT_VALUE.

Thanks,
Drew

2021-11-15 12:22:18

by Daniel Lezcano

[permalink] [raw]
Subject: Re: drivers/clocksource/timer-pistachio.c:74:22: warning: variable 'overflow' set but not used

On 15/11/2021 02:02, Drew Fustini wrote:
> On Sun, Nov 14, 2021 at 11:17:38PM +0100, Daniel Lezcano wrote:
>> On 14/11/2021 22:29, Drew Fustini wrote:
>>> On Fri, Nov 12, 2021 at 07:05:48AM +0800, kernel test robot wrote:
>>>> Hi Drew,
>>>>
>>>> FYI, the error/warning still remains.
>>>>
>>>> tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
>>>> head: debe436e77c72fcee804fb867f275e6d31aa999c
>>>> commit: a47d7ef4550d08fb428ea4c3f1a9c71674212208 clocksource/drivers/pistachio: Fix trivial typo
>>>> date: 7 months ago
>>>> config: mips-randconfig-r012-20210927 (attached as .config)
>>>> compiler: mips-linux-gcc (GCC) 11.2.0
>>>> reproduce (this is a W=1 build):
>>>> wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>>>> chmod +x ~/bin/make.cross
>>>> # https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=a47d7ef4550d08fb428ea4c3f1a9c71674212208
>>>> git remote add linus https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
>>>> git fetch --no-tags linus master
>>>> git checkout a47d7ef4550d08fb428ea4c3f1a9c71674212208
>>>> # save the attached .config to linux build tree
>>>> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross ARCH=mips
>>>>
>>>> If you fix the issue, kindly add following tag as appropriate
>>>> Reported-by: kernel test robot <[email protected]>
>>>>
>>>> All warnings (new ones prefixed by >>):
>>>>
>>>> drivers/clocksource/timer-pistachio.c: In function 'pistachio_clocksource_read_cycles':
>>>>>> drivers/clocksource/timer-pistachio.c:74:22: warning: variable 'overflow' set but not used [-Wunused-but-set-variable]
>>>> 74 | u32 counter, overflow;
>>>> | ^~~~~~~~
>>>>
>>>>
>>>> vim +/overflow +74 drivers/clocksource/timer-pistachio.c
>>>>
>>>> 69
>>>> 70 static u64 notrace
>>>> 71 pistachio_clocksource_read_cycles(struct clocksource *cs)
>>>> 72 {
>>>> 73 struct pistachio_clocksource *pcs = to_pistachio_clocksource(cs);
>>>> > 74 u32 counter, overflow;
>>>> 75 unsigned long flags;
>>>> 76
>>>> 77 /*
>>>> 78 * The counter value is only refreshed after the overflow value is read.
>>>> 79 * And they must be read in strict order, hence raw spin lock added.
>>>> 80 */
>>>> 81
>>>> 82 raw_spin_lock_irqsave(&pcs->lock, flags);
>>>> 83 overflow = gpt_readl(pcs->base, TIMER_CURRENT_OVERFLOW_VALUE, 0);
>>
>> overflow is set here but then never reused in the function. So a call to
>> gpt_readl without getting the return value is fine.
>>
>> The warning is different from 'variable is unused'
>
> Thank you for pointing that out. I was reading it incorrectly.
>
> As for this function, it seems that warning is a false positive as
> the act of reading TIMER_CURRENT_OVERFLOW_VALUE causes the hardware
> to refresh TIMER_CURRENT_VALUE.

The warning is actually not a false positive. It just state the variable
is unused after setting it which is actually correct.

The indirect warning is about the behavior of the hardware where the
counter is refreshed only when the overflow is read.

May be adding the may_be_unused annotation to the overflow variable can
silent the warning?


--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog