netdev - Re: [PATCHSET wq/for-6.9] workqueue: Implement BH workqueue and convert several tasklet users

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: <CAOMdWSJxQ4trkFeEtL1549kgnxmTN1ikWYH7r6bdD40FxbuNpw@mail.gmail.com>
Date: Tue, 30 Jan 2024 15:37:34 -0800
From: Allen <allen.lkml@...il.com>
To: Tejun Heo <tj@...nel.org>
Cc: torvalds@...ux-foundation.org, mpatocka@...hat.com, 
	linux-kernel@...r.kernel.org, dm-devel@...ts.linux.dev, msnitzer@...hat.com, 
	ignat@...udflare.com, damien.lemoal@....com, bob.liu@...cle.com, 
	houtao1@...wei.com, peterz@...radead.org, mingo@...nel.org, 
	netdev@...r.kernel.org, kernel-team@...a.com
Subject: Re: [PATCHSET wq/for-6.9] workqueue: Implement BH workqueue and
 convert several tasklet users

> The only generic interface to execute asynchronously in the BH context is
> tasklet; however, it's marked deprecated and has some design flaws such as
> the execution code accessing the tasklet item after the execution is
> complete which can lead to subtle use-after-free in certain usage scenarios
> and less-developed flush and cancel mechanisms.
>
> Mikulas Patocka recently reported that dm-crypt and dm-crypt are affected by
> the access-after-completion issue and suggested adding TASKLET_STATE_ONESHOT
> flag which selectively removes post-completion access while significantly
> limiting how the tasklet can be used in the following thread:
>
>  http://lkml.kernel.org/r/82b964f0-c2c8-a2c6-5b1f-f3145dc2c8e5@redhat.com
>
> Linus didn't like the approach and suggested extending workqueue to support
> execution from atomic context:
>
>  http://lkml.kernel.org/r/CAHk-=wjDW53w4-YcSmgKC5RruiRLHmJ1sXeYdp_ZgVoBw=5byA@mail.gmail.com
>
> As suggested, this patchset implements BH workqueues which are like regular
> workqueues but executes work items in the BH (softirq) context and converts
> several tasklet users.
>
> - The name bh is used instead of the suggested atomic as it's more in line
>   with widely used execution context interface - local_bh_enable/disable()
>   and friends.
>
> - The system default BH workqueues - system_bh_wq and system_bh_highpri_wq -
>   are provided. As queue-wide flushing doesn't exist in tasklet, all
>   existing tasklet users should be able to use the system BH workqueues
>   without creating their own.
>
> - BH workqueues currently use tasklet to run the work items to avoid
>   priority inversions involving tasklet_hi and WQ_BH | WQ_HIGHPRI. Once all
>   tasklet users are converted, tasklet code can be removed and BH workqueues
>   can take over its softirqs.
>
> This patchset is on top of wq/for-6.9 (aae17ebb53c ("workqueue: Avoid using
> isolated cpus' timers on queue_delayed_work")) and contains the following
> eight patches.
>
>  0001-workqueue-Update-lock-debugging-code.patch
>  0002-workqueue-Factor-out-init_cpu_worker_pool.patch
>  0003-workqueue-Implement-BH-workqueues-to-eventually-repl.patch
>  0004-backtracetest-Convert-from-tasklet-to-BH-workqueue.patch
>  0005-usb-core-hcd-Convert-from-tasklet-to-BH-workqueue.patch
>  0006-net-tcp-tsq-Convert-from-tasklet-to-BH-workqueue.patch
>  0007-dm-crypt-Convert-from-tasklet-to-BH-workqueue.patch
>  0008-dm-verity-Convert-from-tasklet-to-BH-workqueue.patch
>
> 0001-0003 prepare and implement BH workqueues.
>
> 0004-0008 convert some tasklet users to BH workqueue. The conversions are
> relatively straightforward but are in descending order of confidence.
>
> The patchset is also available in the following git branch.
>
>  git://git.kernel.org/pub/scm/linux/kernel/git/tj/wq.git wq-bh-v1
>

 Thank you for doing this work.

Tested the series, things look good. I am going to run LTP and Unixbench
on the kernel.

Tested-by: Allen Pais <allen.lkml@...il.com>

Thanks.

> diffstat follows. Thanks.
>
>  Documentation/core-api/workqueue.rst |   29 ++++-
>  drivers/md/dm-crypt.c                |   36 -------
>  drivers/md/dm-verity-target.c        |    8 -
>  drivers/md/dm-verity.h               |    2
>  drivers/usb/core/hcd.c               |   23 ++--
>  include/linux/usb/hcd.h              |    2
>  include/linux/workqueue.h            |    9 +
>  include/net/tcp.h                    |    2
>  kernel/backtracetest.c               |   18 +--
>  kernel/workqueue.c                   |  312 ++++++++++++++++++++++++++++++++++++++++++++++++--------------
>  kernel/workqueue_internal.h          |    3
>  net/ipv4/tcp.c                       |    2
>  net/ipv4/tcp_output.c                |   36 +++----
>  tools/workqueue/wq_dump.py           |   11 +-
>  14 files changed, 335 insertions(+), 158 deletions(-)
>
> --
> tejun



-- 
       - Allen