Re: [linux-next:master 6618/6917] kernel/sched/psi.c:1230:13: sparse: error: incompatible types in comparison expression (different address spaces)

["Paul E. McKenney" <paulmck@linux.ibm.com>]

On Mon, Feb 11, 2019 at 05:36:06PM -0800, Matthew Wilcox wrote:
> On Fri, Feb 08, 2019 at 11:44:07PM -0800, Paul E. McKenney wrote:
> > On Fri, Feb 08, 2019 at 03:14:41PM -0800, Andrew Morton wrote:
> > > On Fri, 8 Feb 2019 02:29:33 +0800 kbuild test robot <lkp@intel.com> wrote:
> > > 
> > > > tree:   https://urldefense.proofpoint.com/v2/url?u=https-3A__git.kernel.org_pub_scm_linux_kernel_git_next_linux-2Dnext.git&d=DwICAg&c=jf_iaSHvJObTbx-siA1ZOg&r=q4hkQkeaNH3IlTsPvEwkaUALMqf7y6jCMwT5b6lVQbQ&m=myIJaLgovNwHx7SqCW_p1sQx2YvRlmVbShFnuZEFqxY&s=0Y32d-tVCGOq6Vu_VAGgVgbEplhfvOSJ5evHbXTtyBI&e= master
> > > > head:   1bd831d68d5521c01d783af0275439ac645f5027
> > > > commit: e7acbba0d6f7a24c8d24280089030eb9a0eb7522 [6618/6917] psi: introduce psi monitor
> > > > reproduce:
> > > >         # apt-get install sparse
> > > >         git checkout e7acbba0d6f7a24c8d24280089030eb9a0eb7522
> > > >         make ARCH=x86_64 allmodconfig
> > > >         make C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'
> > > > 
> > > > All errors (new ones prefixed by >>):
> > > > 
> > > >    kernel/sched/psi.c:151:6: sparse: warning: symbol 'psi_enable' was not declared. Should it be static?
> > > > >> kernel/sched/psi.c:1230:13: sparse: error: incompatible types in comparison expression (different address spaces)
> > > >    kernel/sched/psi.c:774:30: sparse: warning: dereference of noderef expression
> > > > 
> > > > vim +1230 kernel/sched/psi.c
> > > > 
> > > >   1222	
> > > >   1223	static __poll_t psi_fop_poll(struct file *file, poll_table *wait)
> > > >   1224	{
> > > >   1225		struct seq_file *seq = file->private_data;
> > > >   1226		struct psi_trigger *t;
> > > >   1227		__poll_t ret;
> > > >   1228	
> > > >   1229		rcu_read_lock();
> > > > > 1230		t = rcu_dereference(seq->private);
> 
> So the problem here is the opposite of what we think it is -- seq->private
> is not marked as being RCU protected.

Glad to have helped, then.  ;-)

> > If you wish to opt into this checking, you need to mark the pointer
> > definitions (in this case ->private) with __rcu.  It may also
> > be necessary to mark function parameters as well, as is done for
> > radix_tree_iter_resume().  If you do not wish to use this checking,
> > you should ignore these sparse warnings.
> 
> radix_tree_iter_resume is, happily, gone from my xarray-conv tree.
> __radix_tree_lookup, __radix_tree_replace, radix_tree_iter_replace and
> radix_tree_iter_init are still present, but hopefully not for too much
> longer.  For example, __radix_tree_replace() is (now) called only from
> idr_replace(), and there are only 12 remaining callers of idr_replace().

Will this reduce the number of uses of rcu_dereference_raw()?  Or do they
simply migrate into Xarray?

							Thanx, Paul