All of lore.kernel.org
 help / color / mirror / Atom feed
From: Li Lingfeng <lilingfeng3@huawei.com>
To: <stable@vger.kernel.org>, <gregkh@linuxfoundation.org>,
	<mpatocka@redhat.com>, <torvalds@linux-foundation.org>,
	<tglx@linutronix.de>
Cc: <linux-kernel@vger.kernel.org>, <dm-devel@lists.linux.dev>,
	<msnitzer@redhat.com>, <ignat@cloudflare.com>,
	<damien.lemoal@wdc.com>, <bob.liu@oracle.com>,
	<houtao1@huawei.com>, <nhuck@google.com>, <peterz@infradead.org>,
	<mingo@elte.hu>, <yukuai3@huawei.com>, <yangerkun@huawei.com>,
	<yi.zhang@huawei.com>, <lilingfeng3@huawei.com>,
	<lilingfeng@huaweicloud.com>
Subject: [PATCH 6.6] Revert "dm-crypt, dm-verity: disable tasklets"
Date: Thu, 11 Apr 2024 17:15:39 +0800	[thread overview]
Message-ID: <20240411091539.361470-1-lilingfeng3@huawei.com> (raw)

This reverts commit 5735a2671ffb70ea29ca83969fe01316ee2ed6fc which is
commit 0a9bab391e336489169b95cb0d4553d921302189 upstream.

Tasklet is thought to cause memory corruption [1], so it was disabled in
dm-crypt and dm-verity. However, memory corruption may not happen since
cc->io_queue is created without WQ_UNBOUND [2].
Revert commit 5735a2671ffb ("dm-crypt, dm-verity: disable tasklets") to
bring tasklet back.

[1] https://lore.kernel.org/all/d390d7ee-f142-44d3-822a-87949e14608b@suse.de/T/
[2] https://lore.kernel.org/all/4d331659-badd-749d-fba1-271543631a8a@huawei.com/

Signed-off-by: Li Lingfeng <lilingfeng3@huawei.com>
---
 drivers/md/dm-crypt.c         | 38 +++++++++++++++++++++++++++++++++--
 drivers/md/dm-verity-target.c | 26 ++++++++++++++++++++++--
 drivers/md/dm-verity.h        |  1 +
 3 files changed, 61 insertions(+), 4 deletions(-)

diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
index aa6bb5b4704b..a60d91d02e28 100644
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -75,8 +75,10 @@ struct dm_crypt_io {
 	struct bio *base_bio;
 	u8 *integrity_metadata;
 	bool integrity_metadata_from_pool:1;
+	bool in_tasklet:1;
 
 	struct work_struct work;
+	struct tasklet_struct tasklet;
 
 	struct convert_context ctx;
 
@@ -1775,6 +1777,7 @@ static void crypt_io_init(struct dm_crypt_io *io, struct crypt_config *cc,
 	io->ctx.r.req = NULL;
 	io->integrity_metadata = NULL;
 	io->integrity_metadata_from_pool = false;
+	io->in_tasklet = false;
 	atomic_set(&io->io_pending, 0);
 }
 
@@ -1785,6 +1788,13 @@ static void crypt_inc_pending(struct dm_crypt_io *io)
 
 static void kcryptd_queue_read(struct dm_crypt_io *io);
 
+static void kcryptd_io_bio_endio(struct work_struct *work)
+{
+	struct dm_crypt_io *io = container_of(work, struct dm_crypt_io, work);
+
+	bio_endio(io->base_bio);
+}
+
 /*
  * One of the bios was finished. Check for completion of
  * the whole request and correctly clean up the buffer.
@@ -1817,6 +1827,20 @@ static void crypt_dec_pending(struct dm_crypt_io *io)
 
 	base_bio->bi_status = error;
 
+	/*
+	 * If we are running this function from our tasklet,
+	 * we can't call bio_endio() here, because it will call
+	 * clone_endio() from dm.c, which in turn will
+	 * free the current struct dm_crypt_io structure with
+	 * our tasklet. In this case we need to delay bio_endio()
+	 * execution to after the tasklet is done and dequeued.
+	 */
+	if (io->in_tasklet) {
+		INIT_WORK(&io->work, kcryptd_io_bio_endio);
+		queue_work(cc->io_queue, &io->work);
+		return;
+	}
+
 	bio_endio(base_bio);
 }
 
@@ -2291,6 +2315,11 @@ static void kcryptd_crypt(struct work_struct *work)
 		kcryptd_crypt_write_convert(io);
 }
 
+static void kcryptd_crypt_tasklet(unsigned long work)
+{
+	kcryptd_crypt((struct work_struct *)work);
+}
+
 static void kcryptd_queue_crypt(struct dm_crypt_io *io)
 {
 	struct crypt_config *cc = io->cc;
@@ -2302,10 +2331,15 @@ static void kcryptd_queue_crypt(struct dm_crypt_io *io)
 		 * irqs_disabled(): the kernel may run some IO completion from the idle thread, but
 		 * it is being executed with irqs disabled.
 		 */
-		if (!(in_hardirq() || irqs_disabled())) {
-			kcryptd_crypt(&io->work);
+		if (in_hardirq() || irqs_disabled()) {
+			io->in_tasklet = true;
+			tasklet_init(&io->tasklet, kcryptd_crypt_tasklet, (unsigned long)&io->work);
+			tasklet_schedule(&io->tasklet);
 			return;
 		}
+
+		kcryptd_crypt(&io->work);
+		return;
 	}
 
 	INIT_WORK(&io->work, kcryptd_crypt);
diff --git a/drivers/md/dm-verity-target.c b/drivers/md/dm-verity-target.c
index 49e4a35d7019..0bb126eadc0d 100644
--- a/drivers/md/dm-verity-target.c
+++ b/drivers/md/dm-verity-target.c
@@ -701,6 +701,23 @@ static void verity_work(struct work_struct *w)
 	verity_finish_io(io, errno_to_blk_status(verity_verify_io(io)));
 }
 
+static void verity_tasklet(unsigned long data)
+{
+	struct dm_verity_io *io = (struct dm_verity_io *)data;
+	int err;
+
+	io->in_tasklet = true;
+	err = verity_verify_io(io);
+	if (err == -EAGAIN || err == -ENOMEM) {
+		/* fallback to retrying with work-queue */
+		INIT_WORK(&io->work, verity_work);
+		queue_work(io->v->verify_wq, &io->work);
+		return;
+	}
+
+	verity_finish_io(io, errno_to_blk_status(err));
+}
+
 static void verity_end_io(struct bio *bio)
 {
 	struct dm_verity_io *io = bio->bi_private;
@@ -713,8 +730,13 @@ static void verity_end_io(struct bio *bio)
 		return;
 	}
 
-	INIT_WORK(&io->work, verity_work);
-	queue_work(io->v->verify_wq, &io->work);
+	if (static_branch_unlikely(&use_tasklet_enabled) && io->v->use_tasklet) {
+		tasklet_init(&io->tasklet, verity_tasklet, (unsigned long)io);
+		tasklet_schedule(&io->tasklet);
+	} else {
+		INIT_WORK(&io->work, verity_work);
+		queue_work(io->v->verify_wq, &io->work);
+	}
 }
 
 /*
diff --git a/drivers/md/dm-verity.h b/drivers/md/dm-verity.h
index db93a91169d5..7e495cc375b0 100644
--- a/drivers/md/dm-verity.h
+++ b/drivers/md/dm-verity.h
@@ -87,6 +87,7 @@ struct dm_verity_io {
 	bool in_tasklet;
 
 	struct work_struct work;
+	struct tasklet_struct tasklet;
 
 	char *recheck_buffer;
 
-- 
2.31.1


             reply	other threads:[~2024-04-11  9:14 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-11  9:15 Li Lingfeng [this message]
2024-04-11  9:52 ` [PATCH 6.6] Revert "dm-crypt, dm-verity: disable tasklets" Greg KH
2024-04-11 12:17   ` Li Lingfeng
2024-04-11 12:37 ` Mikulas Patocka
2024-04-12  2:06   ` Li Lingfeng
2024-04-15 15:04     ` Mikulas Patocka

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240411091539.361470-1-lilingfeng3@huawei.com \
    --to=lilingfeng3@huawei.com \
    --cc=bob.liu@oracle.com \
    --cc=damien.lemoal@wdc.com \
    --cc=dm-devel@lists.linux.dev \
    --cc=gregkh@linuxfoundation.org \
    --cc=houtao1@huawei.com \
    --cc=ignat@cloudflare.com \
    --cc=lilingfeng@huaweicloud.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=mpatocka@redhat.com \
    --cc=msnitzer@redhat.com \
    --cc=nhuck@google.com \
    --cc=peterz@infradead.org \
    --cc=stable@vger.kernel.org \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    --cc=yangerkun@huawei.com \
    --cc=yi.zhang@huawei.com \
    --cc=yukuai3@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.