Received-SPF: pass (google.com: domain of linux-kernel+bounces-80039-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Precedence: bulk
MIME-Version: 1.0
References: <20240221174333.700197-1-tj@kernel.org> <CAJhGHyCJS7Pb_5dwTQtcZ25yOVzxFULJEYT4o3id_3xdj32EYA@mail.gmail.com>
 <ZdcQ1jfOKNTlbB6C@slm.duckdns.org>
In-Reply-To: <ZdcQ1jfOKNTlbB6C@slm.duckdns.org>
From: Lai Jiangshan <jiangshanlai@gmail.com>
Date: Sun, 25 Feb 2024 18:55:56 +0800
Message-ID: <CAJhGHyAic563hZMLvbQ2X7RsZNmHHEYfh5Lr48cE1LGTUm_K_w@mail.gmail.com>
Subject: Re: [PATCHSET v2 wq/6.10] workqueue: Implement disable/enable_work()
To: Tejun Heo <tj@kernel.org>
Cc: torvalds@linux-foundation.org, linux-kernel@vger.kernel.org, 
	allen.lkml@gmail.com, kernel-team@meta.com, 
	Thomas Gleixner <tglx@linutronix.de>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Thu, Feb 22, 2024 at 5:16=E2=80=AFPM Tejun Heo <tj@kernel.org> wrote:
>
> Hello,
>
> On Thu, Feb 22, 2024 at 11:33:24AM +0800, Lai Jiangshan wrote:
> > From the last patch:
> > > - tasklet_disable_nosync()      -> disable_work()
> > > - tasklet_disable[_in_atomic]() -> disable_work_sync()
> >
> > I think it is a misuse-prone conversion.
> >
> > A developer familiar with tasklet_disable() might happily use disable_w=
ork()
> > and, to her/his surprise, leave the running works unsynced.
> >
> > And tasklet_disable_nosync() is used at only 3 places while tasklet_dis=
able()
> > is used a lot.  I think the shorter name for the most common cases is b=
etter.
>
> While I agree that this can be argued either way, keeping the interface
> congruent with the existing cancel_work_sync() and friends seems a lot mo=
re
> important to me. It can be a bit more confusing for users who are used to
> tasklet interface but then again we aren't gonna rename cancel_work_sync(=
)
> to kill_work() and the conversion overhead isn't all that significant or
> lasting. However, if we break the consnistency within workqueue API, that=
's
> a source of lasting confusion.
>

Hello, Tejun

I don't want to object to any names. But I'm still thinking of just providi=
ng
disable_work_nosync() rather than disable work().  It will be used
at only places at most.

Thanks
Lai