Re: arm32 build warnings in workqueue.c

[Petr Mladek]

On Fri 2023-06-23 09:47:56, Tejun Heo wrote:
> Hello,
> 
> On Fri, Jun 23, 2023 at 11:24:16AM -0700, Linus Torvalds wrote:
> ...
> >   enum {
> >         [..]
> >         WORK_STRUCT_FLAG_BITS   = WORK_STRUCT_COLOR_SHIFT +
> >                 WORK_STRUCT_COLOR_BITS,
> >         [..]
> >         WORK_STRUCT_FLAG_MASK   = (1UL << WORK_STRUCT_FLAG_BITS) - 1,
> >         WORK_STRUCT_WQ_DATA_MASK = ~WORK_STRUCT_FLAG_MASK,
> >   }
> > 
> > Anyway, that code absolutely has to be fixed. Using enums for types is
> > wrong, wrong, wrong. You should consider an enum to be a weakly typed
> > integer expression with some specific advantages (the automatic
> > incrementing at definition time, and the compiler being able to limit
> > ranges in switch() statements and maybe doing limited type warnings
> > elsewhere)
> 
> The only benefit I care about is it being visible in the type system and
> thus in debug info whether that's the usual one or BTF. This makes a huge
> difference in tracing, monitoring and debugging. For one off's, it's fine to
> track down the define values. For something more sophisticated (e.g.
> non-trivial drgn, bcc and other BPF programs), the macro constants are a
> source of sadness and a really dumb and insidious one.

Would it help to use const variables?

> > +/* Convenience constants - of type 'unsigned long', not 'enum'! */
> > +#define WORK_OFFQ_CANCELING	(1ul << __WORK_OFFQ_CANCELING)

static const WORK_OFFQ_CANCELING = 1ul << __WORK_OFFQ_CANCELING;

It is kind of nasty as well. But it defines the right type and should
be visible in debuginfo. Well, it is usable only for local variables.

Best Regards,
Petr