linux-kernel - Re: [PATCHSET v2 wq/6.10] workqueue: Implement disable/enable

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: <CAJhGHyAic563hZMLvbQ2X7RsZNmHHEYfh5Lr48cE1LGTUm_K_w@mail.gmail.com>
Date: Sun, 25 Feb 2024 18:55:56 +0800
From: Lai Jiangshan <jiangshanlai@...il.com>
To: Tejun Heo <tj@...nel.org>
Cc: torvalds@...ux-foundation.org, linux-kernel@...r.kernel.org, 
	allen.lkml@...il.com, kernel-team@...a.com, 
	Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [PATCHSET v2 wq/6.10] workqueue: Implement disable/enable_work()

On Thu, Feb 22, 2024 at 5:16 PM Tejun Heo <tj@...nel.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_work()
> > and, to her/his surprise, leave the running works unsynced.
> >
> > And tasklet_disable_nosync() is used at only 3 places while tasklet_disable()
> > is used a lot.  I think the shorter name for the most common cases is better.
>
> 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 more
> 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 providing
disable_work_nosync() rather than disable work().  It will be used
at only places at most.

Thanks
Lai