netdev - Re: [PATCH 7/8] dm-crypt: Convert from tasklet to BH workqueue

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]

Date: Tue, 30 Jan 2024 05:53:26 -1000
From: Tejun Heo <tj@...nel.org>
To: Sebastian Andrzej Siewior <bigeasy@...utronix.de>
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, allen.lkml@...il.com,
	kernel-team@...a.com, Alasdair Kergon <agk@...hat.com>,
	Mike Snitzer <snitzer@...nel.org>, tglx@...utronix.de
Subject: Re: [PATCH 7/8] dm-crypt: Convert from tasklet to BH workqueue

Hello,

On Tue, Jan 30, 2024 at 11:46:45AM +0100, Sebastian Andrzej Siewior wrote:
> On 2024-01-29 23:11:54 [-1000], Tejun Heo wrote:
> >  		if (in_hardirq() || irqs_disabled()) {
> > -			io->in_tasklet = true;
> > -			tasklet_init(&io->tasklet, kcryptd_crypt_tasklet, (unsigned long)&io->work);
> > -			tasklet_schedule(&io->tasklet);
> > +			INIT_WORK(&io->work, kcryptd_crypt);
> > +			queue_work(system_bh_wq, &io->work);
> 
> Why do we need the tasklet here in the first place? Couldn't we use
> workqueue? As per comment, the request originates in hardirq and then it
> is moved to tasklet. Couldn't it be moved to workqueue regardless?

Yes, you can and if you replace that system_bh_wq with system_wq, or
system_highpri_wq, everything should be fine in terms of correctness.
However, that would mean that the work item now would always incur a
scheduling latency which can be lengthy in certain circumstances. Whether
that's an actual problem for the subsystem at hand, I have no idea.

Thanks.

-- 
tejun