From: Ming Lei <ming.lei@canonical.com>
To: Jens Axboe <axboe@fb.com>, linux-kernel@vger.kernel.org
Cc: linux-block@vger.kernel.org,
Christoph Hellwig <hch@infradead.org>,
linux-btrfs@vger.kernel.org, Ming Lei <ming.lei@canonical.com>,
Shaun Tancheff <shaun.tancheff@seagate.com>,
Mikulas Patocka <mpatocka@redhat.com>,
Alan Cox <alan@linux.intel.com>, Neil Brown <neilb@suse.de>,
Liu Bo <bo.li.liu@oracle.com>, Jens Axboe <axboe@kernel.dk>
Subject: [PATCH v3 3/3] block: avoid to call .bi_end_io() recursively
Date: Thu, 28 Apr 2016 09:09:49 +0800 [thread overview]
Message-ID: <1461805789-3632-3-git-send-email-ming.lei@canonical.com> (raw)
In-Reply-To: <1461805743-3559-1-git-send-email-ming.lei@canonical.com>
There were reports about heavy stack use by recursive calling
.bi_end_io()([1][2][3]). For example, more than 16K stack is
consumed in a single bio complete path[3], and in [2] stack
overflow can be triggered if 20 nested dm-crypt is used.
Also patches[1] [2] [3] were posted for addressing the issue,
but never be merged. And the idea in these patches is basically
similar, all serializes the recursive calling of .bi_end_io() by
percpu list.
This patch still takes the same idea, but uses bio_list to
implement it, which turns out more simple and the code becomes
more readable meantime.
One corner case which wasn't covered before is that
bi_endio() may be scheduled to run in process context(such
as btrfs), and this patch just bypasses the optimizing for
that case because one new context should have enough stack space,
and this approach isn't capable of optimizing it too because
there isn't easy way to get a per-task linked list head.
xfstests(-g auto) is run with this patch and no regression is
found on ext4, xfs and btrfs.
[1] http://marc.info/?t=121428502000004&r=1&w=2
[2] http://marc.info/?l=dm-devel&m=139595190620008&w=2
[3] http://marc.info/?t=145974644100001&r=1&w=2
Cc: Shaun Tancheff <shaun.tancheff@seagate.com>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Mikulas Patocka <mpatocka@redhat.com>
Cc: Alan Cox <alan@linux.intel.com>
Cc: Neil Brown <neilb@suse.de>
Cc: Liu Bo <bo.li.liu@oracle.com>
Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
block/bio.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 53 insertions(+), 2 deletions(-)
diff --git a/block/bio.c b/block/bio.c
index 807d25e..e20a83c 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -68,6 +68,8 @@ static DEFINE_MUTEX(bio_slab_lock);
static struct bio_slab *bio_slabs;
static unsigned int bio_slab_nr, bio_slab_max;
+static DEFINE_PER_CPU(struct bio_list *, bio_end_list) = { NULL };
+
static struct kmem_cache *bio_find_or_create_slab(unsigned int extra_size)
{
unsigned int sz = sizeof(struct bio) + extra_size;
@@ -1737,6 +1739,56 @@ static inline bool bio_remaining_done(struct bio *bio)
return false;
}
+static void __bio_endio(struct bio *bio)
+{
+ if (bio->bi_end_io)
+ bio->bi_end_io(bio);
+}
+
+/* disable local irq when manipulating the percpu bio_list */
+static void unwind_bio_endio(struct bio *bio)
+{
+ struct bio_list *bl;
+ unsigned long flags;
+
+ /*
+ * We can't optimize if bi_endio() is scheduled to run from
+ * process context because there isn't easy way to get a
+ * per-task bio list head or allocate a per-task variable.
+ */
+ if (!in_interrupt()) {
+ /*
+ * It has to be a top calling when it is run from
+ * process context.
+ */
+ WARN_ON(this_cpu_read(bio_end_list));
+ __bio_endio(bio);
+ return;
+ }
+
+ local_irq_save(flags);
+ bl = __this_cpu_read(bio_end_list);
+ if (bl) {
+ bio_list_add(bl, bio);
+ } else {
+ struct bio_list bl_in_stack;
+
+ bl = &bl_in_stack;
+ bio_list_init(bl);
+
+ __this_cpu_write(bio_end_list, bl);
+ while (bio) {
+ local_irq_restore(flags);
+ __bio_endio(bio);
+ local_irq_save(flags);
+
+ bio = bio_list_pop(bl);
+ }
+ __this_cpu_write(bio_end_list, NULL);
+ }
+ local_irq_restore(flags);
+}
+
/**
* bio_endio - end I/O on a bio
* @bio: bio
@@ -1765,8 +1817,7 @@ again:
goto again;
}
- if (bio->bi_end_io)
- bio->bi_end_io(bio);
+ unwind_bio_endio(bio);
}
EXPORT_SYMBOL(bio_endio);
--
1.9.1
next prev parent reply other threads:[~2016-04-28 1:10 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-04-28 1:09 [PATCH v3 0/3] block: avoid to call .bi_end_io() recursively Ming Lei
2016-04-28 1:09 ` [PATCH v3 1/3] fs: direct-io: handle error in dio_end_io() Ming Lei
2016-04-28 1:09 ` [PATCH v3 2/3] fs: direct-io: call .bi_end_io via bio_endio() Ming Lei
2016-05-02 14:50 ` Christoph Hellwig
2016-05-03 1:26 ` Ming Lei
2016-04-28 1:09 ` Ming Lei [this message]
2016-04-28 15:29 ` [PATCH v3 3/3] block: avoid to call .bi_end_io() recursively Mikulas Patocka
2016-04-28 15:52 ` Ming Lei
2016-04-28 15:58 ` Mikulas Patocka
2016-04-28 16:12 ` Ming Lei
2016-04-28 16:59 ` Mikulas Patocka
2016-04-29 5:50 ` Ming Lei
2016-04-29 8:39 ` Mikulas Patocka
2016-04-29 9:33 ` Ming Lei
2016-04-29 9:59 ` Mikulas Patocka
2016-05-02 16:22 ` Jens Axboe
2016-05-02 14:52 ` [PATCH v3 0/3] " Christoph Hellwig
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=1461805789-3632-3-git-send-email-ming.lei@canonical.com \
--to=ming.lei@canonical.com \
--cc=alan@linux.intel.com \
--cc=axboe@fb.com \
--cc=axboe@kernel.dk \
--cc=bo.li.liu@oracle.com \
--cc=hch@infradead.org \
--cc=linux-block@vger.kernel.org \
--cc=linux-btrfs@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mpatocka@redhat.com \
--cc=neilb@suse.de \
--cc=shaun.tancheff@seagate.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).