On Mon, May 12, 2025 at 5:22 AM Ilya Leoshkevich <iii@xxxxxxxxxxxxx> wrote:
>
> On Fri, 2025-05-09 at 09:51 -0700, Alexei Starovoitov wrote:
> > On Thu, May 8, 2025 at 12:21 PM Ilya Leoshkevich <iii@xxxxxxxxxxxxx>
> > wrote:
> > >
> > > On Thu, 2025-05-08 at 11:38 -0700, Alexei Starovoitov wrote:
> > > > On Thu, May 8, 2025 at 4:38 AM Ilya Leoshkevich
> > > > <iii@xxxxxxxxxxxxx>
> > > > wrote:
> > > > >
> > > > > clang-21 complains about unused expressions in a few progs.
> > > > > Fix by explicitly casting the respective expressions to void.
> > > >
> > > > ...
> > > > > if (val & _Q_LOCKED_MASK)
> > > > > - smp_cond_load_acquire_label(&lock->locked,
> > > > > !VAL,
> > > > > release_err);
> > > > > + (void)smp_cond_load_acquire_label(&lock-
> > > > > >locked,
> > > > > !VAL, release_err);
> > > >
> > > > Hmm. I'm on clang-21 too and I don't see them.
> > > > What warnings do you see ?
> > >
> > > In file included from progs/arena_spin_lock.c:7:
> > > progs/bpf_arena_spin_lock.h:305:1756: error: expression result
> > > unused
> > > [-Werror,-Wunused-value]
> > > 305 | ({ typeof(_Generic((*&lock->locked), char: (char)0,
> > > unsigned
> > > char : (unsigned char)0, signed char : (signed char)0, unsigned
> > > short :
> > > (unsigned short)0, signed short : (signed short)0, unsigned int :
> > > (unsigned int)0, signed int : (signed int)0, unsigned long :
> > > (unsigned
> > > long)0, signed long : (signed long)0, unsigned long long :
> > > (unsigned
> > > long long)0, signed long long : (signed long long)0, default:
> > > (typeof(*&lock->locked))0)) __val = ({ typeof(&lock->locked) __ptr
> > > =
> > > (&lock->locked); typeof(_Generic((*(&lock->locked)), char: (char)0,
> > > unsigned char : (unsigned char)0, signed char : (signed char)0,
> > > unsigned short : (unsigned short)0, signed short : (signed short)0,
> > > unsigned int : (unsigned int)0, signed int : (signed int)0,
> > > unsigned
> > > long : (unsigned long)0, signed long : (signed long)0, unsigned
> > > long
> > > long : (unsigned long long)0, signed long long : (signed long
> > > long)0,
> > > default: (typeof(*(&lock->locked)))0)) VAL; for (;;) { VAL =
> > > (typeof(_Generic((*(&lock->locked)), char: (char)0, unsigned char :
> > > (unsigned char)0, signed char : (signed char)0, unsigned short :
> > > (unsigned short)0, signed short : (signed short)0, unsigned int :
> > > (unsigned int)0, signed int : (signed int)0, unsigned long :
> > > (unsigned
> > > long)0, signed long : (signed long)0, unsigned long long :
> > > (unsigned
> > > long long)0, signed long long : (signed long long)0, default:
> > > (typeof(*(&lock->locked)))0)))(*(volatile typeof(*__ptr)
> > > *)&(*__ptr));
> > > if (!VAL) break; ({ __label__ l_break, l_continue; asm volatile
> > > goto("may_goto %l[l_break]" :::: l_break); goto l_continue;
> > > l_break:
> > > goto release_err; l_continue:; }); ({}); } (typeof(*(&lock-
> > > > locked)))VAL; }); ({ ({ if (!CONFIG_X86_64) ({ unsigned long
> > > > __val;
> > > __sync_fetch_and_add(&__val, 0); }); else asm volatile("" :::
> > > "memory"); }); }); (typeof(*(&lock->locked)))__val; });
> > > |
> > > ^ ~~~~~
> > > 1 error generated.
> >
> > hmm. The error is impossible to read.
> >
> > Kumar,
> >
> > Do you see a way to silence it differently ?
> >
> > Without adding (void)...
> >
> > Things like:
> > - bpf_obj_new(..
> > + (void)bpf_obj_new(..
> >
> > are good to fix, and if we could annotate
> > bpf_obj_new_impl kfunc with __must_check we would have done it,
> >
> > but
> > - arch_mcs_spin_lock...
> > + (void)arch_mcs_spin_lock...
> >
> > is odd.
>
> What do you think about moving (void) to the definition of
> arch_mcs_spin_lock_contended_label()? I can send a v2 if this is
> better.
Kumar,
thoughts?
