Linux Btrfs filesystem development
 help / color / mirror / Atom feed
From: Jens Axboe <axboe@fb.com>
To: Chris Mason <clm@fb.com>, Liu Bo <bo.li.liu@oracle.com>,
	<linux-btrfs@vger.kernel.org>
Subject: Re: [PATCH] Btrfs: fix a bug of sleeping in atomic context
Date: Fri, 20 Nov 2015 14:30:43 -0700	[thread overview]
Message-ID: <564F9103.7060301@fb.com> (raw)
In-Reply-To: <20151120131358.GC9887@ret.masoncoding.com>

[-- Attachment #1: Type: text/plain, Size: 1015 bytes --]

On 11/20/2015 06:13 AM, Chris Mason wrote:
> On Thu, Nov 19, 2015 at 05:49:37PM -0800, Liu Bo wrote:
>> while xfstesting, this bug[1] is spotted by both btrfs/061 and btrfs/063,
>> so those sub-stripe writes are gatherred into plug callback list and
>> hopefully we can have a full stripe writes.
>>
>> However, while processing these plugged callbacks, it's within an atomic
>> context which is provided by blk_sq_make_request() because of a get_cpu()
>> in blk_mq_get_ctx().
>>
>> This changes to always use btrfs_rmw_helper to complete the pending writes.
>>
>
> Thanks Liu, but MD raid has the same troubles, we're not atomic in our unplugs.
>
> Jens?

Yeah, blk-mq does have preemption disabled when it flushes, for the 
single queue setup. That's a bug. Attached is an untested patch that 
should fix it, can you try it?

I'll rework this to be a proper patch, not convinced we want to add the 
new request before flush, that might destroy merging opportunities. I'll 
unify the mq/sq parts.

-- 
Jens Axboe


[-- Attachment #2: blk-mq-preempt-plug-flush.patch --]
[-- Type: text/x-patch, Size: 676 bytes --]

diff --git a/block/blk-mq.c b/block/blk-mq.c
index 3ae09de62f19..0325dcec8c74 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -1380,12 +1391,15 @@ static blk_qc_t blk_sq_make_request(struct request_queue *q, struct bio *bio)
 		blk_mq_bio_to_request(rq, bio);
 		if (!request_count)
 			trace_block_plug(q);
-		else if (request_count >= BLK_MAX_REQUEST_COUNT) {
+
+		list_add_tail(&rq->queuelist, &plug->mq_list);
+		blk_mq_put_ctx(data.ctx);
+
+		if (request_count + 1 >= BLK_MAX_REQUEST_COUNT) {
 			blk_flush_plug_list(plug, false);
 			trace_block_plug(q);
 		}
-		list_add_tail(&rq->queuelist, &plug->mq_list);
-		blk_mq_put_ctx(data.ctx);
+
 		return cookie;
 	}
 


  parent reply	other threads:[~2015-11-20 21:30 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-11-20  1:49 [PATCH] Btrfs: fix a bug of sleeping in atomic context Liu Bo
2015-11-20 13:13 ` Chris Mason
2015-11-20 17:57   ` Liu Bo
2015-11-20 20:06     ` Liu Bo
2015-11-20 20:09       ` Chris Mason
2015-11-20 21:30   ` Jens Axboe [this message]
2015-11-20 23:08     ` Liu Bo
2015-11-21  2:26       ` Jens Axboe
2015-11-21  3:14         ` Liu Bo
2015-11-21  3:29           ` Jens Axboe
2015-11-21  6:05             ` Liu Bo

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=564F9103.7060301@fb.com \
    --to=axboe@fb.com \
    --cc=bo.li.liu@oracle.com \
    --cc=clm@fb.com \
    --cc=linux-btrfs@vger.kernel.org \
    /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