linux-kernel - Re: [PATCH 1/7] workqueue: Preserve OFFQ bits in cancel[

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZdcAPKtxgw2ZUVFn@slm.duckdns.org>
Date: Wed, 21 Feb 2024 22:05:16 -1000
From: Tejun Heo <tj@...nel.org>
To: Lai Jiangshan <jiangshanlai@...il.com>
Cc: torvalds@...ux-foundation.org, linux-kernel@...r.kernel.org,
	allen.lkml@...il.com, kernel-team@...a.com
Subject: Re: [PATCH 1/7] workqueue: Preserve OFFQ bits in cancel[_sync] paths

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@...nel.org> 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