linux-kernel - Re: lib/flex_proportions.c:73:9: sparse: sparse: incorrect type in argument 1 (different address spaces)

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] [day] [month] [year] [list]
Date:   Sat, 11 Nov 2023 17:42:13 +0800
From:   Philip Li <philip.li@...el.com>
To:     Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
        Huacai Chen <chenhuacai@...ngson.cn>,
        Bibo Mao <maobibo@...ngson.cn>
CC:     kernel test robot <lkp@...el.com>, <oe-kbuild-all@...ts.linux.dev>,
        <linux-kernel@...r.kernel.org>,
        Thomas Gleixner <tglx@...utronix.de>
Subject: Re: lib/flex_proportions.c:73:9: sparse: sparse: incorrect type in
 argument 1 (different address spaces)

+Huacai and Bibo for support.

On Sat, Nov 11, 2023 at 09:54:33AM +0800, Philip Li wrote:
> On Fri, Nov 10, 2023 at 04:42:08PM +0100, Sebastian Andrzej Siewior wrote:
> > On 2023-11-08 09:04:40 [+0800], kernel test robot wrote:
> > …
> > Could you please explain what is wrong here? The code in line 73 has no
> > percpu reference at all. Or expects an argument.
> 
> Apologize for the confusing report, we will investigate this in earliest
> time to understand what goes wrong. Kindly ignore this and sorry for the noise.

Hi Sebastian, I do a further check and realize this is same as [1], that Steven mentioned

	> > 2430		lockdep_assert_preemption_disabled();  

	The above is a generic lockdep utility. Sounds to me that this is a bug in
	the loongarch code that doesn't handle this properly.

So the commit reported is not the actual cause of the issue, which just calls the
preempt_disable_nested and exposes the issue of loongarch.

As for the question "line 73 has nopercpu reference", probably the preempt_disable_nested
is expanded to call lockdep_assert_preemption_disabled, which in turn further calls into
this_cpu_read (because CONFIG_PREEMPT_RT is not set and CONFIG_PROVE_LOCKING=y for this
randconfig).

For now, I will update bot logic to avoid sending similar reports and look for fix
from loongarch side.

Hi Huacai and Bibo, sorry to bother, could you help do a check of [1] and [2], or point
me to the right contact for this sparse warning?

[1] https://lore.kernel.org/oe-kbuild-all/20231108094847.236d04b2@gandalf.local.home/
[2] https://lore.kernel.org/oe-kbuild-all/202311080409.LlOfTR3m-lkp@intel.com/

Thanks

> 
> > 
> > > sparse warnings: (new ones prefixed by >>)
> > > >> lib/flex_proportions.c:73:9: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected void *ptr @@     got unsigned int [noderef] __percpu * @@
> > >    lib/flex_proportions.c:73:9: sparse:     expected void *ptr
> > >    lib/flex_proportions.c:73:9: sparse:     got unsigned int [noderef] __percpu *
> > > >> lib/flex_proportions.c:73:9: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected void *ptr @@     got unsigned int [noderef] __percpu * @@
> > >    lib/flex_proportions.c:73:9: sparse:     expected void *ptr
> > >    lib/flex_proportions.c:73:9: sparse:     got unsigned int [noderef] __percpu *
> > > >> lib/flex_proportions.c:73:9: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected void *ptr @@     got unsigned int [noderef] __percpu * @@
> > >    lib/flex_proportions.c:73:9: sparse:     expected void *ptr
> > >    lib/flex_proportions.c:73:9: sparse:     got unsigned int [noderef] __percpu *
> > > >> lib/flex_proportions.c:73:9: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected void *ptr @@     got unsigned int [noderef] __percpu * @@
> > >    lib/flex_proportions.c:73:9: sparse:     expected void *ptr
> > >    lib/flex_proportions.c:73:9: sparse:     got unsigned int [noderef] __percpu *
> > > >> lib/flex_proportions.c:73:9: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected void *ptr @@     got int [noderef] __percpu * @@
> > >    lib/flex_proportions.c:73:9: sparse:     expected void *ptr
> > >    lib/flex_proportions.c:73:9: sparse:     got int [noderef] __percpu *
> > > >> lib/flex_proportions.c:73:9: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected void *ptr @@     got int [noderef] __percpu * @@
> > >    lib/flex_proportions.c:73:9: sparse:     expected void *ptr
> > >    lib/flex_proportions.c:73:9: sparse:     got int [noderef] __percpu *
> > > >> lib/flex_proportions.c:73:9: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected void *ptr @@     got int [noderef] __percpu * @@
> > >    lib/flex_proportions.c:73:9: sparse:     expected void *ptr
> > >    lib/flex_proportions.c:73:9: sparse:     got int [noderef] __percpu *
> > > >> lib/flex_proportions.c:73:9: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected void *ptr @@     got int [noderef] __percpu * @@
> > >    lib/flex_proportions.c:73:9: sparse:     expected void *ptr
> > >    lib/flex_proportions.c:73:9: sparse:     got int [noderef] __percpu *
> > >    lib/flex_proportions.c: note: in included file (through include/linux/mmzone.h, include/linux/gfp.h, include/linux/percpu_counter.h, ...):
> > > 
> > > vim +73 lib/flex_proportions.c
> > > 
> > >     55	
> > >     56	/*
> > >     57	 * Declare @periods new periods. It is upto the caller to make sure period
> > >     58	 * transitions cannot happen in parallel.
> > >     59	 *
> > >     60	 * The function returns true if the proportions are still defined and false
> > >     61	 * if aging zeroed out all events. This can be used to detect whether declaring
> > >     62	 * further periods has any effect.
> > >     63	 */
> > >     64	bool fprop_new_period(struct fprop_global *p, int periods)
> > >     65	{
> > >     66		s64 events = percpu_counter_sum(&p->events);
> > >     67	
> > >     68		/*
> > >     69		 * Don't do anything if there are no events.
> > >     70		 */
> > >     71		if (events <= 1)
> > >     72			return false;
> > >   > 73		preempt_disable_nested();
> > >     74		write_seqcount_begin(&p->sequence);
> > >     75		if (periods < 64)
> > >     76			events -= events >> periods;
> > >     77		/* Use addition to avoid losing events happening between sum and set */
> > >     78		percpu_counter_add(&p->events, -events);
> > >     79		p->period += periods;
> > >     80		write_seqcount_end(&p->sequence);
> > >     81		preempt_enable_nested();
> > >     82	
> > >     83		return true;
> > >     84	}
> > >     85	
> > > 
> > Sebastian
> >