Re: [PATCHSET v2 wq/6.10] workqueue: Implement disable/enable_work()

[Lai Jiangshan]

On Thu, Feb 22, 2024 at 5:16 PM Tejun Heo <tj@xxxxxxxxxx> 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