Re: [PATCH 1/7] workqueue: Preserve OFFQ bits in cancel[_sync] paths

[Tejun Heo]

On Thu, Feb 22, 2024 at 12:35:31PM +0800, Lai Jiangshan wrote:
> Hello,
> 
> On Thu, Feb 22, 2024 at 1:43 AM Tejun Heo <tj@xxxxxxxxxx> wrote:
> 
> >
> >  /* Convenience constants - of type 'unsigned long', not 'enum'! */
> >  #define WORK_OFFQ_CANCELING    (1ul << WORK_OFFQ_CANCELING_BIT)
> > +#define WORK_OFFQ_FLAG_MASK    (((1ul << WORK_OFFQ_FLAG_BITS) - 1) << WORK_OFFQ_FLAG_SHIFT)
> 
> It can use GENMASK.

So, that'd be

 GENMASK(WORK_OFFQ_FLAG_SHIFT + WORK_OFFQ_FLAG_BITS - 1, WORK_OFFQ_FLAG_SHIFT)

I'm not quite sure that's an improvement. It's longer and more indirect -
the open coded one says "BITS number of 1's shifted by SHIFT", which is
shorter and easier to comprehend.

Another thing is that none of the other masks fits GENMASK either. The only
other candidate is WORK_STRUCT_PWQ_MASK but the number of bits in that mask
depends on the size of ulong, so it can't use GENMASK.

I don't think using GENMASK would be an improvement here.

Thanks.

-- 
tejun