lkml.org 
[lkml]   [2018]   [Mar]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [tip:locking/core 9/11] include/asm-generic/atomic-instrumented.h:288:24: sparse: cast truncates bits from constant value (100 becomes 0)
On Tue, Mar 13, 2018 at 02:08:13PM +0300, Dmitry Vyukov wrote:
> On Tue, Mar 13, 2018 at 1:46 PM, Peter Zijlstra <peterz@infradead.org> wrote:
> > On Tue, Mar 13, 2018 at 11:49:17AM +0300, Dmitry Vyukov wrote:
> >> On Mon, Mar 12, 2018 at 6:17 PM, Dmitry Vyukov <dvyukov@google.com> wrote:
> >> > On Mon, Mar 12, 2018 at 5:52 PM, kbuild test robot
> >> >> kernel/locking/qspinlock.c:418:22: sparse: incorrect type in assignment (different modifiers) @@ expected struct mcs_spinlock *prev @@ got struct struct mcs_spinlock *prev @@
> >> >> kernel/locking/qspinlock.c:418:22: expected struct mcs_spinlock *prev
> >> >> kernel/locking/qspinlock.c:418:22: got struct mcs_spinlock [pure] *
> >
> >> >> b06ed71a6 Dmitry Vyukov 2018-01-29 283 static __always_inline unsigned long
> >> >> b06ed71a6 Dmitry Vyukov 2018-01-29 284 cmpxchg_size(volatile void *ptr, unsigned long old, unsigned long new, int size)
> >> >> b06ed71a6 Dmitry Vyukov 2018-01-29 285 {
> >> >> b06ed71a6 Dmitry Vyukov 2018-01-29 286 switch (size) {
> >> >> b06ed71a6 Dmitry Vyukov 2018-01-29 287 case 1:
> >> >> b06ed71a6 Dmitry Vyukov 2018-01-29 @288 return arch_cmpxchg((u8 *)ptr, (u8)old, (u8)new);
> >> >> b06ed71a6 Dmitry Vyukov 2018-01-29 289 case 2:
> >> >> b06ed71a6 Dmitry Vyukov 2018-01-29 290 return arch_cmpxchg((u16 *)ptr, (u16)old, (u16)new);
> >> >> b06ed71a6 Dmitry Vyukov 2018-01-29 291 case 4:
> >> >> b06ed71a6 Dmitry Vyukov 2018-01-29 292 return arch_cmpxchg((u32 *)ptr, (u32)old, (u32)new);
> >> >> b06ed71a6 Dmitry Vyukov 2018-01-29 293 case 8:
> >> >> b06ed71a6 Dmitry Vyukov 2018-01-29 294 BUILD_BUG_ON(sizeof(unsigned long) != 8);
> >> >> b06ed71a6 Dmitry Vyukov 2018-01-29 295 return arch_cmpxchg((u64 *)ptr, (u64)old, (u64)new);
> >> >> b06ed71a6 Dmitry Vyukov 2018-01-29 296 }
> >> >> b06ed71a6 Dmitry Vyukov 2018-01-29 297 BUILD_BUG();
> >> >> b06ed71a6 Dmitry Vyukov 2018-01-29 298 return 0;
> >> >> b06ed71a6 Dmitry Vyukov 2018-01-29 299 }
> >> >> b06ed71a6 Dmitry Vyukov 2018-01-29 300
> >
> >> It seems that this is due to this guy:
> >>
> >> static __always_inline int trylock_clear_pending(struct qspinlock *lock)
> >> {
> >> struct __qspinlock *l = (void *)lock;
> >>
> >> return !READ_ONCE(l->locked) &&
> >> (cmpxchg_acquire(&l->locked_pending, _Q_PENDING_VAL,
> >> _Q_LOCKED_VAL) == _Q_PENDING_VAL);
> >> }
> >>
> >> _Q_PENDING_VAL is 0x100. However, locked_pending is 2 bytes. So it
> >> seems that compiler checks all switch cases, this inevitably will lead
> >> to such warnings.
> >>
> >> Any suggestion on how to resolve this? Leave as is?
> >
> > I'm not sure I understand what it thinks is wrong. Can't we fix sparse
> > to not be stupid? The actual compilers don't seem to a have a problem
> > with this.

The issue here is that sparse has a whole class of warnings that are
given very early (here at expansion of constant expressions), before
eliminating code from branches that are never taken (which, surprise,
need itself to have constant expressions already expanded).

It's often annoying like the case here.
OTOH, I don't think it's always a bad thing. Sometimes we want to
have warnings even from code we know will not be executed (in this
config but maybe it will in another one).

Several solution would be possible:
1) removing all those early warnings (but then we'll loose more legitimate ones)
2) add an option to not issues those early warnings (ame as above).
3) move those warnings (and thus the expansion of the concerned constants)
after trivial dead code elimination
4) add a very early pass to eliminate never taken branches
5) something else ?

1) & 2) are easy but not really acceptable.
3) can be good for some cases but not here, I think.
4) seems to way to go

-- Luc Van Oostenryck

\
 
 \ /
  Last update: 2018-03-13 19:01    [W:0.061 / U:1.088 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site