netdev - Re: [net-next v3 05/15] net: cavium/liquidio: Convert tasklet API to new bottom half workqueue mechanism

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: <CAOMdWSKRFXFdi4SF20LH528KcXtxD+OL=HzSh9Gzqy9HCqkUGw@mail.gmail.com>
Date: Mon, 5 Aug 2024 10:23:41 -0700
From: Allen <allen.lkml@...il.com>
To: Jakub Kicinski <kuba@...nel.org>
Cc: "David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, 
	Paolo Abeni <pabeni@...hat.com>, jes@...ined-monkey.org, kda@...ux-powerpc.org, 
	cai.huoqing@...ux.dev, dougmill@...ux.ibm.com, npiggin@...il.com, 
	christophe.leroy@...roup.eu, aneesh.kumar@...nel.org, 
	naveen.n.rao@...ux.ibm.com, nnac123@...ux.ibm.com, tlfalcon@...ux.ibm.com, 
	cooldavid@...ldavid.org, marcin.s.wojtas@...il.com, mlindner@...vell.com, 
	stephen@...workplumber.org, nbd@....name, sean.wang@...iatek.com, 
	Mark-MC.Lee@...iatek.com, lorenzo@...nel.org, matthias.bgg@...il.com, 
	angelogioacchino.delregno@...labora.com, borisp@...dia.com, 
	bryan.whitehead@...rochip.com, UNGLinuxDriver@...rochip.com, 
	louis.peens@...igine.com, richardcochran@...il.com, 
	linux-rdma@...r.kernel.org, linux-kernel@...r.kernel.org, 
	linux-acenic@...site.dk, linux-net-drivers@....com, netdev@...r.kernel.org, 
	Sunil Goutham <sgoutham@...vell.com>
Subject: Re: [net-next v3 05/15] net: cavium/liquidio: Convert tasklet API to
 new bottom half workqueue mechanism

> > > Could you shed some light in the cover letter or this patch why
> > > tasklet_enable() is converted to enable_and_queue_work() at
> > > the face of it those two do not appear to do the same thing?
> >
> > With the transition to workqueues, the implementation on the workqueue side is:
> >
> > tasklet_enable() -> enable_work() + queue_work()
> >
> > Ref: https://lore.kernel.org/all/20240227172852.2386358-7-tj@kernel.org/
> >
> > enable_and_queue_work() is a helper which combines the two calls.
> >
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=474a549ff4c989427a14fdab851e562c8a63fe24
> >
> > Hope this answers your question.
>
> To an extent. tj says "unconditionally scheduling the work item after
> enable_work() returns %true should work for most users."
> You need to include the explanation of the conversion not being 1:1
> in the commit message, and provide some analysis why it's fine for this
> user.

Sure, please review the explanation below and let me
know if it is clear enough:

tasklet_enable() is used to enable a tasklet, which defers
work to be executed in an interrupt context. It relies on the
tasklet mechanism for deferred execution.

enable_and_queue_work() combines enabling the work with
scheduling it on a workqueue. This approach not only enables
the work but also schedules it for execution by the workqueue
system, which is more flexible and suitable for tasks needing
process context rather than interrupt context.

enable_and_queue_work() internally calls enable_work() to enable
the work item and then uses queue_work() to add it to the workqueue.
This ensures that the work item is both enabled and explicitly
scheduled for execution within the workqueue system's context.

As mentioned, "unconditionally scheduling the work item after
enable_work() returns true should work for most users." This
ensures that the work is consistently scheduled for execution,
aligning with the typical workqueue usage pattern. Most users
expect that enabling a work item implies it will be scheduled for
execution without additional conditional logic.

Thanks,
- Allen