linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jens Axboe <axboe@kernel.dk>
To: Ming Lei <ming.lei@canonical.com>, 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,
	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>
Subject: Re: [PATCH v3 3/3] block: avoid to call .bi_end_io() recursively
Date: Mon, 2 May 2016 10:22:08 -0600	[thread overview]
Message-ID: <57277EB0.5060408@kernel.dk> (raw)
In-Reply-To: <1461805789-3632-3-git-send-email-ming.lei@canonical.com>

On 04/27/2016 07:09 PM, Ming Lei wrote:
> 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.

I don't like this at all, it's going about fixing this the completely 
wrong way. For the fast path, single device, we don't have an issue. Yet 
this patch slows down that part unnecessarily.

Fix this for the fat dm/md stacks, and don't add unwarranted fat in the 
normal fast path. The dm/md stacks won't notice a bit of extra overhead, 
whereas the core path is pretty well optimized.

-- 
Jens Axboe

      parent reply	other threads:[~2016-05-02 16:22 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 ` [PATCH v3 3/3] block: avoid to call .bi_end_io() recursively Ming Lei
2016-04-28 15:29   ` 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 [this message]

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=57277EB0.5060408@kernel.dk \
    --to=axboe@kernel.dk \
    --cc=alan@linux.intel.com \
    --cc=axboe@fb.com \
    --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=ming.lei@canonical.com \
    --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).