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 ...
Best regards,
Oliver
Thanks and regards,
Dario
Do you take care on this one?
https://lore.kernel.org/linux-can/20220717025703.pcrf6bseaigsq22r@revolver/T/#m54028de8868a0f653294f6a272a9094378a1cdd7
Best regards,
Oliver
