[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZMdu1YqUI7VIEq1y@alley>
Date: Mon, 31 Jul 2023 10:20:37 +0200
From: Petr Mladek <pmladek@...e.com>
To: Tejun Heo <tj@...nel.org>
Cc: Linus Torvalds <torvalds@...ux-foundation.org>,
Dave Airlie <airlied@...il.com>, Arnd Bergmann <arnd@...db.de>,
LKML <linux-kernel@...r.kernel.org>
Subject: Re: arm32 build warnings in workqueue.c
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
Powered by blists - more mailing lists