From patchwork Fri May 29 00:29:06 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Junichi Nomura X-Patchwork-Id: 6503201 X-Patchwork-Delegate: snitzer@redhat.com Return-Path: X-Original-To: patchwork-dm-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 5C63E9F1CC for ; Fri, 29 May 2015 00:36:59 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 5B7C620704 for ; Fri, 29 May 2015 00:36:58 +0000 (UTC) Received: from mx3-phx2.redhat.com (mx3-phx2.redhat.com [209.132.183.24]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 1A0FD206FE for ; Fri, 29 May 2015 00:36:57 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by mx3-phx2.redhat.com (8.13.8/8.13.8) with ESMTP id t4T0UTW8012016; Thu, 28 May 2015 20:30:30 -0400 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id t4T0URX9023287 for ; Thu, 28 May 2015 20:30:27 -0400 Received: from mx1.redhat.com (ext-mx03.extmail.prod.ext.phx2.redhat.com [10.5.110.27]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t4T0UR8b016606 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Thu, 28 May 2015 20:30:27 -0400 Received: from tyo202.gate.nec.co.jp (TYO202.gate.nec.co.jp [210.143.35.52]) by mx1.redhat.com (Postfix) with ESMTPS id A6342B7A8E; Fri, 29 May 2015 00:30:25 +0000 (UTC) Received: from mailgate3.nec.co.jp ([10.7.69.160]) by tyo202.gate.nec.co.jp (8.13.8/8.13.4) with ESMTP id t4T0UOuh029215; Fri, 29 May 2015 09:30:24 +0900 (JST) Received: from mailsv.nec.co.jp (imss61.nec.co.jp [10.7.69.156]) by mailgate3.nec.co.jp (8.11.7/3.7W-MAILGATE-NEC) with ESMTP id t4T0UN707950; Fri, 29 May 2015 09:30:23 +0900 (JST) Received: from mail02.kamome.nec.co.jp (mail02.kamome.nec.co.jp [10.25.43.5]) by mailsv.nec.co.jp (8.13.8/8.13.4) with ESMTP id t4T0UM9C029414; Fri, 29 May 2015 09:30:23 +0900 (JST) Received: from bpxc99gp.gisp.nec.co.jp ([10.38.151.136] [10.38.151.136]) by mail02.kamome.nec.co.jp with ESMTP id BT-MMP-272965; Fri, 29 May 2015 09:29:08 +0900 Received: from BPXM12GP.gisp.nec.co.jp ([169.254.2.218]) by BPXC08GP.gisp.nec.co.jp ([10.38.151.136]) with mapi id 14.03.0174.002; Fri, 29 May 2015 09:29:07 +0900 From: Junichi Nomura To: Mike Snitzer Thread-Topic: dm: fix false alarm in free_rq_clone() for non blk-mq target Thread-Index: AQHQmXqZw1RvQfjEsEesp/4qSHMHUZ2RgxwA Date: Fri, 29 May 2015 00:29:06 +0000 Message-ID: <5567B2D2.5080907@ce.jp.nec.com> References: <5566CAE8.4070404@ce.jp.nec.com> <20150528191457.GA26218@redhat.com> In-Reply-To: <20150528191457.GA26218@redhat.com> Accept-Language: ja-JP, en-US Content-Language: ja-JP X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.34.125.85] Content-ID: MIME-Version: 1.0 X-RedHat-Spam-Score: -4.601 (BAYES_00, DCC_REPUT_00_12, RCVD_IN_DNSWL_MED, SPF_HELO_PASS, SPF_PASS, URIBL_BLOCKED) 210.143.35.52 TYO202.gate.nec.co.jp 210.143.35.52 TYO202.gate.nec.co.jp X-Scanned-By: MIMEDefang 2.68 on 10.5.11.26 X-Scanned-By: MIMEDefang 2.76 on 10.5.110.27 X-MIME-Autoconverted: from quoted-printable to 8bit by lists01.pubmisc.prod.ext.phx2.redhat.com id t4T0URX9023287 X-loop: dm-devel@redhat.com Cc: device-mapper development Subject: Re: [dm-devel] dm: fix false alarm in free_rq_clone() for non blk-mq target X-BeenThere: dm-devel@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk Reply-To: device-mapper development List-Id: device-mapper development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dm-devel-bounces@redhat.com Errors-To: dm-devel-bounces@redhat.com X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, T_RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable 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 05/29/15 04:14, Mike Snitzer wrote: > On Thu, May 28 2015 at 3:59am -0400, > Junichi Nomura wrote: > >> When stacking request-based dm device on non blk-mq device >> and device-mapper target could not map the request >> (error target is used, multipath target with all paths down, etc), >> following warning will show up once: >> >> WARNING: CPU: 7 PID: 0 at drivers/md/dm.c:1090 free_rq_clone+0x7a/0xf0 >> CPU: 7 PID: 0 Comm: swapper/7 Not tainted 4.1.0-rc5+ #1 >> .. >> Call Trace: >> [] dump_stack+0x45/0x57 >> [] warn_slowpath_common+0x8a/0xc0 >> [] warn_slowpath_null+0x1a/0x20 >> [] free_rq_clone+0x7a/0xf0 [dm_mod] >> [] dm_softirq_done+0x13c/0x250 [dm_mod] >> [] blk_done_softirq+0x80/0xa0 >> [] __do_softirq+0xf4/0x2d0 >> [] irq_exit+0x125/0x130 >> [] smp_call_function_single_interrupt+0x35/0x40 >> [] call_function_single_interrupt+0x6e/0x80 >> [] ? native_safe_halt+0x6/0x10 >> [] ? rcu_eqs_enter+0x68/0x90 >> [] default_idle+0x1e/0xc0 >> [] arch_cpu_idle+0xf/0x20 >> [] cpu_startup_entry+0x2fc/0x3a0 >> [] start_secondary+0x182/0x1b0 >> >> The warning was added by commit aa6df8dd28c0 ("dm: fix free_rq_clone() >> NULL pointer when requeueing unmapped request"). >> >> But free_rq_clone() with clone->q == NULL is valid usage for non blk-mq >> underlying device. Such a call can happen via dm_kill_unmapped_request(). > > dm_kill_unmapped_request() is called from the blk-mq error path too Yes, but: > (e.g. if clone_and_map_rq fails with an error). So this isn't non > blk-mq specific. unmapped request does not have clone in the case of blk-mq? (as the clone_and_map_rq() API suggests) Then dm_softirq_done(orig) for dm_kill_unmapped_request() will fall into 'if (!clone)' path. Thus neither of dm_done(clone), dm_end_request(clone) nor free_rq_clone() will be called. > dm_kill_unmapped_request() sets REQ_FAILED, which dm_softirq_done() > checks for and will set 'mapped' to false. I think a proper fix is to > pass 'mapped' into dm_end_request() and then pass it to free_rq_clone(). > Like so, what do you think? So I don't think it's necessary to extend dm_end_request() with 'mapped' parameter. For blk-mq underlying device, we can (and should be able to) assume unmapped request as not having clone. It might be better to clear tio->clone in free_rq_clone(). Currently it doesn't seem matter because tio is not reused for non-mq DM and always re-initialized for mq DM, though. diff --git a/drivers/md/dm.c b/drivers/md/dm.c index 1c62ed8..f252adc 100644 --- a/drivers/md/dm.c +++ b/drivers/md/dm.c @@ -1087,14 +1087,14 @@ static void free_rq_clone(struct request *clone, bool must_be_mapped) struct dm_rq_target_io *tio = clone->end_io_data; struct mapped_device *md = tio->md; - WARN_ON_ONCE(must_be_mapped && !clone->q); - blk_rq_unprep_clone(clone); - if (md->type == DM_TYPE_MQ_REQUEST_BASED) + if (md->type == DM_TYPE_MQ_REQUEST_BASED) { /* stacked on blk-mq queue(s) */ + WARN_ON_ONCE(must_be_mapped && !clone->q); tio->ti->type->release_clone_rq(clone); + tio->clone = NULL; - else if (!md->queue->mq_ops) + } else if (!md->queue->mq_ops) /* request_fn queue stacked on request_fn queue(s) */ free_clone_request(md, clone); /*