From patchwork Fri Nov 20 21:30:43 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jens Axboe X-Patchwork-Id: 7671371 Return-Path: X-Original-To: patchwork-linux-btrfs@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 47169BF90C for ; Fri, 20 Nov 2015 21:31:00 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 6D03E20411 for ; Fri, 20 Nov 2015 21:30:58 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id B5497203C3 for ; Fri, 20 Nov 2015 21:30:57 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1760390AbbKTVat (ORCPT ); Fri, 20 Nov 2015 16:30:49 -0500 Received: from mx0b-00082601.pphosted.com ([67.231.153.30]:22480 "EHLO mx0b-00082601.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1758669AbbKTVat (ORCPT ); Fri, 20 Nov 2015 16:30:49 -0500 Received: from pps.filterd (m0001255.ppops.net [127.0.0.1]) by mx0b-00082601.pphosted.com (8.15.0.59/8.15.0.59) with SMTP id tAKLMMqf004064; Fri, 20 Nov 2015 13:30:46 -0800 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=fb.com; h=subject : to : references : from : message-id : date : mime-version : in-reply-to : content-type; s=facebook; bh=Aw3iRatTANTko5w/CB9DmpM45wa8DSZ3fB7k61DmpXo=; b=SPJd1f5kMoH1ti85EldLcJBj2F2AUfnXLYkPhzau890cJ51FdjsqlGgO3aN7YIF6GKkl 5Z25qjRd10OmLN/YAt2E1cJ35QFXHnBjU7vL8gR8SrhrKyQqe5dB3xk7z1vMow56Bb+s cFV3UaxLZNJ0cqnmRXHFcJ2aTTVyoT7AQRc= Received: from mail.thefacebook.com ([199.201.64.23]) by mx0b-00082601.pphosted.com with ESMTP id 1y9cas6hx4-1 (version=TLSv1/SSLv3 cipher=AES128-SHA bits=128 verify=NOT); Fri, 20 Nov 2015 13:30:46 -0800 Received: from [192.168.1.124] (192.168.54.13) by mail.thefacebook.com (192.168.16.12) with Microsoft SMTP Server (TLS) id 14.3.248.2; Fri, 20 Nov 2015 13:30:43 -0800 Subject: Re: [PATCH] Btrfs: fix a bug of sleeping in atomic context To: Chris Mason , Liu Bo , References: <1447984177-26795-1-git-send-email-bo.li.liu@oracle.com> <20151120131358.GC9887@ret.masoncoding.com> From: Jens Axboe Message-ID: <564F9103.7060301@fb.com> Date: Fri, 20 Nov 2015 14:30:43 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 MIME-Version: 1.0 In-Reply-To: <20151120131358.GC9887@ret.masoncoding.com> X-Originating-IP: [192.168.54.13] X-Proofpoint-Spam-Reason: safe X-FB-Internal: Safe X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:, , definitions=2015-11-20_12:, , signatures=0 Sender: linux-btrfs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-btrfs@vger.kernel.org X-Spam-Status: No, score=-7.3 required=5.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI,RP_MATCHES_RCVD,T_DKIM_INVALID,T_TVD_MIME_EPI, UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP 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. 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; }