From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Paul E. McKenney" Subject: Re: [PATCH 2/2] dm: stay in blk_queue_bypass until queue becomes initialized Date: Mon, 29 Oct 2012 09:55:16 -0700 Message-ID: <20121029165516.GI3027@linux.vnet.ibm.com> References: <50890937.7010809@ce.jp.nec.com> <20121026202105.GF24687@redhat.com> <508E572C.6000707@ce.jp.nec.com> <20121029163845.GB7709@redhat.com> Reply-To: paulmck@linux.vnet.ibm.com Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Content-Disposition: inline In-Reply-To: <20121029163845.GB7709@redhat.com> Sender: linux-kernel-owner@vger.kernel.org To: Vivek Goyal Cc: Jun'ichi Nomura , "linux-kernel@vger.kernel.org" , device-mapper development , Tejun Heo , Jens Axboe , Alasdair G Kergon , Peter Zijlstra List-Id: dm-devel.ids On Mon, Oct 29, 2012 at 12:38:45PM -0400, Vivek Goyal wrote: > On Mon, Oct 29, 2012 at 07:15:08PM +0900, Jun'ichi Nomura wrote: > > On 10/27/12 05:21, Vivek Goyal wrote: > > > On Thu, Oct 25, 2012 at 06:41:11PM +0900, Jun'ichi Nomura wrote: > > >> [PATCH] dm: stay in blk_queue_bypass until queue becomes initialized > > >> > > >> With 749fefe677 ("block: lift the initial queue bypass mode on > > >> blk_register_queue() instead of blk_init_allocated_queue()"), > > >> add_disk() eventually calls blk_queue_bypass_end(). > > >> This change invokes the following warning when multipath is used. > > >> > > >> BUG: scheduling while atomic: multipath/2460/0x00000002 Looks like preemption has been disabled, nested two deep, based on the /0x00000002, which dumps out preempt_count(). So is someone invoking this with preemption disabled? > > >> 1 lock held by multipath/2460: > > >> #0: (&md->type_lock){......}, at: [] dm_lock_md_type+0x17/0x19 [dm_mod] > > >> Modules linked in: ... > > >> Pid: 2460, comm: multipath Tainted: G W 3.7.0-rc2 #1 > > >> Call Trace: > > >> [] __schedule_bug+0x6a/0x78 > > >> [] __schedule+0xb4/0x5e0 > > >> [] schedule+0x64/0x66 > > >> [] schedule_timeout+0x39/0xf8 > > >> [] ? put_lock_stats+0xe/0x29 > > >> [] ? lock_release_holdtime+0xb6/0xbb > > >> [] wait_for_common+0x9d/0xee > > >> [] ? try_to_wake_up+0x206/0x206 > > >> [] ? kfree_call_rcu+0x1c/0x1c > > >> [] wait_for_completion+0x1d/0x1f > > >> [] wait_rcu_gp+0x5d/0x7a > > >> [] ? wait_rcu_gp+0x7a/0x7a > > >> [] ? complete+0x21/0x53 > > >> [] synchronize_rcu+0x1e/0x20 > > >> [] blk_queue_bypass_start+0x5d/0x62 > > >> [] blkcg_activate_policy+0x73/0x270 > > >> [] ? kmem_cache_alloc_node_trace+0xc7/0x108 > > >> [] cfq_init_queue+0x80/0x28e > > >> [] ? dm_blk_ioctl+0xa7/0xa7 [dm_mod] > > >> [] elevator_init+0xe1/0x115 > > >> [] ? blk_queue_make_request+0x54/0x59 > > >> [] blk_init_allocated_queue+0x8c/0x9e > > >> [] dm_setup_md_queue+0x36/0xaa [dm_mod] > > >> [] table_load+0x1bd/0x2c8 [dm_mod] > > >> [] ctl_ioctl+0x1d6/0x236 [dm_mod] > > >> [] ? table_clear+0xaa/0xaa [dm_mod] > > >> [] dm_ctl_ioctl+0x13/0x17 [dm_mod] > > >> [] do_vfs_ioctl+0x3fb/0x441 > > >> [] ? file_has_perm+0x8a/0x99 > > >> [] sys_ioctl+0x5e/0x82 > > >> [] ? trace_hardirqs_on_thunk+0x3a/0x3f > > >> [] system_call_fastpath+0x16/0x1b > > >> > > >> The warning means during queue initialization blk_queue_bypass_start() > > >> calls sleeping function (synchronize_rcu) while dm holds md->type_lock. > > > > > > md->type_lock is a mutex, isn't it? I thought we are allowed to block > > > and schedule out under mutex? > > > > Hm, you are right. It's a mutex. > > The warning occurs only if I turned on CONFIG_PREEMPT=y. > > Ok, so the question is what's wrong with calling synchronize_rcu() inside > a mutex with CONFIG_PREEMPT=y. I don't know. Ccing paul mckenney and > peterz. Nothing is wrong with calling synchronize_rcu() inside a mutex, this has been used and does work. Last I tried it, anyway. ;-) > > > add_disk() also calls disk_alloc_events() which does kzalloc(GFP_KERNEL). > > > So we already have code which can block/wait under md->type_lock. I am > > > not sure why should we get this warning under a mutex. > > > > add_disk() is called without md->type_lock. > > > > Call flow is like this: > > > > dm_create > > alloc_dev > > blk_alloc_queue > > alloc_disk > > add_disk > > blk_queue_bypass_end [with 3.7-rc2] > > > > table_load > > dm_lock_md_type [takes md->type_lock] > > dm_setup_md_queue > > blk_init_allocated_queue [when DM_TYPE_REQUEST_BASED] > > elevator_init > > blkcg_activate_policy > > blk_queue_bypass_start <-- THIS triggers the warning > > blk_queue_bypass_end > > blk_queue_bypass_end [with 3.6] > > dm_unlock_md_type > > > > blk_queue_bypass_start() in blkcg_activate_policy was nested call, > > that did nothing, with 3.6. > > With 3.7-rc2, it becomes the initial call and does > > actual draining stuff. > > Ok. Once we know what's wrong, we should be able to figure out the > right solution. Artificially putting queue one level deep in bypass > to avoid calling synchronize_rcu() sounds bad. Also should be unnecessary. I suggest finding out where preemption is disabled. Scattering checks for preempt_count()!=0 might help locate it. Thanx, Paul