On 18.07.2022 09:05:35, Oliver Hartkopp wrote: > Hi Dario, > > On 18.07.22 08:52, Dario Binacchi wrote: > > Hi Oliver, > > > > On Sun, Jul 17, 2022 at 3:58 PM Oliver Hartkopp <socketcan@xxxxxxxxxxxx> wrote: > > > > > > Hi Dario, > > > > > > did you see this build regression too? > > > > > > On 14.07.22 03:56, kernel test robot wrote: > > > > tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master > > > > branch HEAD: 4662b7adea50bb62e993a67f611f3be625d3df0d Add linux-next specific files for 20220713 > > > > > > > > Error/Warning reports: > > > > > > (..) > > > > > > > drivers/net/can/slcan/slcan-core.c:601:14: sparse: void * > > > > drivers/net/can/slcan/slcan-core.c:601:14: sparse: void [noderef] __rcu * > > > > drivers/net/can/slcan/slcan-core.c:601:14: sparse: sparse: incompatible types in comparison expression (different address spaces): > > > > > > > IMHO I think that adding '__rcu' annotation would remove the warning: > > > > diff --git a/include/linux/tty.h b/include/linux/tty.h > > index 7b0a5d478ef6..278b84f04d20 100644 > > --- a/include/linux/tty.h > > +++ b/include/linux/tty.h > > @@ -239,7 +239,7 @@ struct tty_struct { > > wait_queue_head_t write_wait; > > wait_queue_head_t read_wait; > > struct work_struct hangup_work; > > - void *disc_data; > > + void __rcu *disc_data; > > void *driver_data; > > spinlock_t files_lock; > > struct list_head tty_files; > > > > But in the paragraph "SPARSE CHECKING OF RCU-PROTECTED POINTERS" of > > Documentation/RCU/rcu_dereference.rst > > we read: > > ... > > Unfortunately, these sorts of bugs can be extremely hard to spot during > > review. This is where the sparse tool comes into play, along with the > > "__rcu" marker. If you mark a pointer declaration, whether in a structure > > or as a formal parameter, with "__rcu", which tells sparse to complain if > > this pointer is accessed directly. It will also cause sparse to complain > > if a pointer not marked with "__rcu" is accessed using rcu_dereference() > > and friends. > > ... > > Use of "__rcu" is opt-in. If you choose not to use it, then you should > > ignore the sparse warnings. > > ... > > > > So, I think that by adding the '__rcu' annotation we would have new > > warnings in all those points > > where disc_data is accessed directly (which are many more than those > > where rcu_dereference() is used). > > > > If I'm not mistaken, the warning also refers to code that my series > > hasn't touched. Also, in the 'BUILD REGRESSION' > > Oh, I didn't check that myself. > > So some old code just came into focus :-/ > > > report, the slcan warning is found under the 'Unverified Error / > > Warning (likely false positive, please contact us if interested)' > > section. > > > > So, can it be okay to think about leaving everything as it is, and > > then not apply any patches to remove this warning? > > Yes. With this this background leaving the code as-is seems to be > appropriate too. Thanks for the explanation! > > Maybe Marc has another opinion. So let's wait for his feedback ... This is not a regression, so leave as is for now. regards, Marc -- Pengutronix e.K. | Marc Kleine-Budde | Embedded Linux | https://www.pengutronix.de | Vertretung West/Dortmund | Phone: +49-231-2826-924 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
Attachment:
signature.asc
Description: PGP signature
