Re: [linux-next:master] BUILD REGRESSION 4662b7adea50bb62e993a67f611f3be625d3df0d

[Oliver Hartkopp <socketcan@hartkopp.net>]

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@hartkopp.net> 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
> 
> 
>