From patchwork Fri Aug 5 16:06:38 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mike Snitzer X-Patchwork-Id: 9265537 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 3700160754 for ; Fri, 5 Aug 2016 16:06:42 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 28DFE28306 for ; Fri, 5 Aug 2016 16:06:42 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 1D75628451; Fri, 5 Aug 2016 16:06:42 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 797A428306 for ; Fri, 5 Aug 2016 16:06:41 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1759910AbcHEQGl (ORCPT ); Fri, 5 Aug 2016 12:06:41 -0400 Received: from mx1.redhat.com ([209.132.183.28]:34980 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1759807AbcHEQGk (ORCPT ); Fri, 5 Aug 2016 12:06:40 -0400 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id D2C5DC04B31E; Fri, 5 Aug 2016 16:06:39 +0000 (UTC) Received: from localhost (dhcp-25-149.bos.redhat.com [10.18.25.149]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u75G6cRK008305 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA256 bits=256 verify=NO); Fri, 5 Aug 2016 12:06:39 -0400 Date: Fri, 5 Aug 2016 12:06:38 -0400 From: Mike Snitzer To: Jens Axboe , Benjamin Block Cc: dm-devel@redhat.com, linux-kernel@vger.kernel.org, hch@lst.de, linux-block@vger.kernel.org Subject: Re: bug: using smp_processor_id() in preemptible code in rr_select_path() Message-ID: <20160805160638.GA17651@redhat.com> References: <20160803153545.GE20370@bblock-ThinkPad-W530> <20160805152705.GA17480@redhat.com> <497f02e5-4f3d-8590-3096-fb5e2422bff4@kernel.dk> <20160805154223.GA2340@redhat.com> <107dc81f-675d-8ae4-0ac2-99c248a23c60@kernel.dk> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <107dc81f-675d-8ae4-0ac2-99c248a23c60@kernel.dk> User-Agent: Mutt/1.5.21 (2010-09-15) X-Scanned-By: MIMEDefang 2.68 on 10.5.11.23 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.31]); Fri, 05 Aug 2016 16:06:39 +0000 (UTC) Sender: linux-block-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-block@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Fri, Aug 05 2016 at 11:54am -0400, Jens Axboe wrote: > On 08/05/2016 09:42 AM, Mike Snitzer wrote: > >On Fri, Aug 05 2016 at 11:33P -0400, > >Jens Axboe wrote: > > > >>On 08/05/2016 09:27 AM, Mike Snitzer wrote: > >>>On Wed, Aug 03 2016 at 11:35am -0400, > >>>Benjamin Block wrote: > >>> > >>>>Hej Mike, > >>>> > >>>>when running a debug-kernel today with several multipath-devices using > >>>>the round-robin path selector I noticed that the kernel throws these > >>>>warnings here: > >>>> > >>>>BUG: using smp_processor_id() in preemptible [00000000] code: kdmwork-252:0/881 > >>>>caller is rr_select_path+0x36/0x108 [dm_round_robin] > >>>>CPU: 1 PID: 881 Comm: kdmwork-252:0 Not tainted 4.7.0-debug #4 > >>>> 00000000617679b8 0000000061767a48 0000000000000002 0000000000000000 > >>>> 0000000061767ae8 0000000061767a60 0000000061767a60 00000000001145d0 > >>>> 0000000000000000 0000000000b962ae 0000000000bb291e 000000000000000b > >>>> 0000000061767aa8 0000000061767a48 0000000000000000 0000000000000000 > >>>> 0700000000b962ae 00000000001145d0 0000000061767a48 0000000061767aa8 > >>>>Call Trace: > >>>>([<00000000001144a2>] show_trace+0x8a/0xe0) > >>>>([<0000000000114586>] show_stack+0x8e/0xf0) > >>>>([<00000000006c7fdc>] dump_stack+0x9c/0xe0) > >>>>([<00000000006fbbc0>] check_preemption_disabled+0x108/0x130) > >>>>([<000003ff80268646>] rr_select_path+0x36/0x108 [dm_round_robin]) > >>>>([<000003ff80259a42>] choose_path_in_pg+0x42/0xc8 [dm_multipath]) > >>>>([<000003ff80259b62>] choose_pgpath+0x9a/0x1a0 [dm_multipath]) > >>>>([<000003ff8025b51a>] __multipath_map.isra.5+0x72/0x228 [dm_multipath]) > >>>>([<000003ff8025b75e>] multipath_map+0x3e/0x50 [dm_multipath]) > >>>>([<000003ff80225eb6>] map_request+0x66/0x458 [dm_mod]) > >>>>([<000003ff802262ec>] map_tio_request+0x44/0x70 [dm_mod]) > >>>>([<000000000016835a>] kthread_worker_fn+0xf2/0x1d8) > >>>>([<00000000001681da>] kthread+0x112/0x120) > >>>>([<000000000098378a>] kernel_thread_starter+0x6/0xc) > >>>>([<0000000000983784>] kernel_thread_starter+0x0/0xc) > >>>>no locks held by kdmwork-252:0/881. > >>>> > >>>> > >>>>There are several more of these warnings, but all have the same > >>>>stack-trace (this is on s390x, but this looks like its only common code) > >>>>- sometimes the process-context is multipath. > >>>> > >>>>Looking at the changes in this function, it rather looks like this is > >>>>caused by changes made in commit > >>>>b0b477c7e0dd93f8916d106018ded1331b81bf61 (dm round robin: use percpu > >>>>'repeat_count' and 'current_path'). > >>>> > >>>>The kernel is a stock v4.7 with some debug options enabled (prominently > >>>>DEBUG_PREEMPT). Need any more info? > >>> > >>>As you can see from commit b0b477c7e0dd9 the round-robin path selector > >>>is now using percpu data (pointer) and a percpu_counter. > >>> > >>>I'm really not sure how else to access this percpu data. > >>> > >>>Cc'ing LKML, linux-block, Jens and hch to cast a wider net in the hopes > >>>of getting an answer to how to fix this. > >> > >>From a quick look, looks like you are using this_cpu_ptr() without > >>having preemption disabled. > > > >Right, that is what it looked like to me too. > > > >I'm just not sure on what the proper pattern is to fix this. > > > >I'll look closer though. > > I always forget the details (if this confuses lockdep or not), but you > could potentially turn it into: > > local_irq_save(flags); > x = this_cpu_ptr(); > [...] > > spin_lock(&s->lock); > [...] > > instead. Cool, I've coded up the patch (compile tested only). Benjamin, any chance you could test this against your v4.7 kernel and report back? Thanks, Mike --- To unsubscribe from this list: send the line "unsubscribe linux-block" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/drivers/md/dm-round-robin.c b/drivers/md/dm-round-robin.c index 4ace1da..ed446f8 100644 --- a/drivers/md/dm-round-robin.c +++ b/drivers/md/dm-round-robin.c @@ -210,14 +210,17 @@ static struct dm_path *rr_select_path(struct path_selector *ps, size_t nr_bytes) struct path_info *pi = NULL; struct dm_path *current_path = NULL; + local_irq_save(flags); current_path = *this_cpu_ptr(s->current_path); if (current_path) { percpu_counter_dec(&s->repeat_count); - if (percpu_counter_read_positive(&s->repeat_count) > 0) + if (percpu_counter_read_positive(&s->repeat_count) > 0) { + local_irq_restore(flags); return current_path; + } } - spin_lock_irqsave(&s->lock, flags); + spin_lock(&s->lock); if (!list_empty(&s->valid_paths)) { pi = list_entry(s->valid_paths.next, struct path_info, list); list_move_tail(&pi->list, &s->valid_paths); @@ -225,7 +228,8 @@ static struct dm_path *rr_select_path(struct path_selector *ps, size_t nr_bytes) set_percpu_current_path(s, pi->path); current_path = pi->path; } - spin_unlock_irqrestore(&s->lock, flags); + spin_unlock(&s->lock); + local_irq_restore(flags); return current_path; }