Received-SPF: pass (google.com: domain of linux-kernel+bounces-76059-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Sender: Tejun Heo <htejun@gmail.com>
Date: Wed, 21 Feb 2024 22:05:16 -1000
From: Tejun Heo <tj@kernel.org>
To: Lai Jiangshan <jiangshanlai@gmail.com>
Cc: torvalds@linux-foundation.org, linux-kernel@vger.kernel.org,
	allen.lkml@gmail.com, kernel-team@meta.com
Subject: Re: [PATCH 1/7] workqueue: Preserve OFFQ bits in cancel[_sync] paths
Message-ID: <ZdcAPKtxgw2ZUVFn@slm.duckdns.org>
References: <20240221174333.700197-1-tj@kernel.org>
 <20240221174333.700197-2-tj@kernel.org>
 <CAJhGHyBgjMWtob2VRWhxCRyNuUVMzSEQufza5TRn7Cw-hBaQ3Q@mail.gmail.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <CAJhGHyBgjMWtob2VRWhxCRyNuUVMzSEQufza5TRn7Cw-hBaQ3Q@mail.gmail.com>

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@kernel.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