[RESEND,v2,3/3] block: hide back blk_freeze_queue_start and export its blk-mq alias
Commit Message
blk_freeze_queue_start is used internally for universal queue draining and
externally for blk-mq specific queue freezing. Keep the non-blk-mq name
private and export a blk-mq alias to users.
Signed-off-by: Jinlong Chen <nickyc975@zju.edu.cn>
---
block/blk-core.c | 13 +++++++++++++
block/blk-mq.c | 27 ++++++++++++++-------------
block/blk-pm.c | 2 +-
block/blk.h | 1 +
drivers/nvme/host/core.c | 2 +-
drivers/nvme/host/multipath.c | 2 +-
include/linux/blk-mq.h | 2 +-
7 files changed, 32 insertions(+), 17 deletions(-)
Comments
On Sun, Oct 30, 2022 at 01:26:46PM +0800, Jinlong Chen wrote:
> blk_freeze_queue_start is used internally for universal queue draining and
> externally for blk-mq specific queue freezing. Keep the non-blk-mq name
> private and export a blk-mq alias to users.
I really don't see the point here. Eventually all of the freezing
should move out of the mq namespace. But that given that we have
actual technical work pending here I'd suggest to just leave it alone
for now, and just respin a version of patch 1 without the pointless
comment.
> On Sun, Oct 30, 2022 at 01:26:46PM +0800, Jinlong Chen wrote:
> > blk_freeze_queue_start is used internally for universal queue draining and
> > externally for blk-mq specific queue freezing. Keep the non-blk-mq name
> > private and export a blk-mq alias to users.
>
> I really don't see the point here. Eventually all of the freezing
> should move out of the mq namespace. But that given that we have
> actual technical work pending here I'd suggest to just leave it alone
> for now, and just respin a version of patch 1 without the pointless
> comment.
I agree that the freezing stuff (maybe also the quiescing stuff) should
move out of the mq namespace. If now is not the proper time, I'll leave
them alone. I'll resend patch 1 alone without the comment.
Thanks!
Jinlong Chen
On Sun, Oct 30, 2022 at 04:19:49PM +0800, Jinlong Chen wrote:
> I agree that the freezing stuff (maybe also the quiescing stuff) should
> move out of the mq namespace. If now is not the proper time, I'll leave
> them alone. I'll resend patch 1 alone without the comment.
The quiesce actually is entirely blk-mq specific, which just have some
careless callers.
> > I agree that the freezing stuff (maybe also the quiescing stuff) should
> > move out of the mq namespace. If now is not the proper time, I'll leave
> > them alone. I'll resend patch 1 alone without the comment.
>
> The quiesce actually is entirely blk-mq specific, which just have some
> careless callers.
Got it, thank you!
Thanks!
Jinlong Chen
Greeting,
FYI, we noticed WARNING:at_block/blk-mq.c:#blk_mq_freeze_queue due to commit (built with gcc-11):
commit: c39dd1922bb12913c74552b8685fa08c818c0f0b ("[RESEND PATCH v2 3/3] block: hide back blk_freeze_queue_start and export its blk-mq alias")
url: https://github.com/intel-lab-lkp/linux/commits/Jinlong-Chen/queue-freezing-improvement-and-cleanups/20221030-132846
base: https://git.kernel.org/cgit/linux/kernel/git/axboe/linux-block.git for-next
patch link: https://lore.kernel.org/linux-block/3f2b51cc7f5c21e49bfa089e594cb203a4015183.1667107410.git.nickyc975@zju.edu.cn
patch subject: [RESEND PATCH v2 3/3] block: hide back blk_freeze_queue_start and export its blk-mq alias
in testcase: boot
on test machine: qemu-system-x86_64 -enable-kvm -cpu SandyBridge -smp 2 -m 16G
caused below changes (please refer to attached dmesg/kmsg for entire log/backtrace):
[ 51.738147][ T1] ------------[ cut here ]------------
[ 51.739393][ T1] WARNING: CPU: 0 PID: 1 at block/blk-mq.c:169 blk_mq_freeze_queue (??:?)
[ 51.741212][ T1] Modules linked in:
[ 51.742143][ T1] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 6.1.0-rc2-00131-gc39dd1922bb1 #7
[ 51.744035][ T1] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.0-debian-1.16.0-4 04/01/2014
[ 51.746130][ T1] RIP: 0010:blk_mq_freeze_queue (??:?)
[ 51.747380][ T1] Code: fd 48 83 c7 30 48 89 fa 48 c1 ea 03 80 3c 02 00 75 1c 48 83 7d 30 00 74 11 48 89 ef e8 7d bc fd ff 48 89 ef 5d e9 d4 a9 ff ff <0f> 0b eb eb e8 4b 87 8d ff eb dd 66 0f 1f 84 00 00 00 00 00 0f 1f
All code
========
0: fd std
1: 48 83 c7 30 add $0x30,%rdi
5: 48 89 fa mov %rdi,%rdx
8: 48 c1 ea 03 shr $0x3,%rdx
c: 80 3c 02 00 cmpb $0x0,(%rdx,%rax,1)
10: 75 1c jne 0x2e
12: 48 83 7d 30 00 cmpq $0x0,0x30(%rbp)
17: 74 11 je 0x2a
19: 48 89 ef mov %rbp,%rdi
1c: e8 7d bc fd ff callq 0xfffffffffffdbc9e
21: 48 89 ef mov %rbp,%rdi
24: 5d pop %rbp
25: e9 d4 a9 ff ff jmpq 0xffffffffffffa9fe
2a:* 0f 0b ud2 <-- trapping instruction
2c: eb eb jmp 0x19
2e: e8 4b 87 8d ff callq 0xffffffffff8d877e
33: eb dd jmp 0x12
35: 66 0f 1f 84 00 00 00 nopw 0x0(%rax,%rax,1)
3c: 00 00
3e: 0f .byte 0xf
3f: 1f (bad)
Code starting with the faulting instruction
===========================================
0: 0f 0b ud2
2: eb eb jmp 0xffffffffffffffef
4: e8 4b 87 8d ff callq 0xffffffffff8d8754
9: eb dd jmp 0xffffffffffffffe8
b: 66 0f 1f 84 00 00 00 nopw 0x0(%rax,%rax,1)
12: 00 00
14: 0f .byte 0xf
15: 1f (bad)
[ 51.751305][ T1] RSP: 0000:ffffc9000001fb48 EFLAGS: 00010246
[ 51.752597][ T1] RAX: dffffc0000000000 RBX: ffff888151d69000 RCX: ffff8881b0e6cee0
[ 51.754303][ T1] RDX: 1ffff1102a3b6a8d RSI: ffffffff8409eb00 RDI: ffff888151db5468
[ 51.756001][ T1] RBP: ffff888151db5438 R08: 0000000000000001 R09: ffffffff85d3f903
[ 51.761843][ T1] R10: fffffbfff0ba7f20 R11: 0000000000000001 R12: ffff88814d39f210
[ 51.763631][ T1] R13: ffff888151db5438 R14: ffff888151db54f0 R15: ffff888151db54f0
[ 51.765781][ T1] FS: 0000000000000000(0000) GS:ffff88839d300000(0000) knlGS:0000000000000000
[ 51.767816][ T1] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 51.769216][ T1] CR2: 0000000000000000 CR3: 000000000502a000 CR4: 00000000000406e0
[ 51.770953][ T1] Call Trace:
[ 51.771779][ T1] <TASK>
[ 51.772539][ T1] blk_iolatency_init (??:?)
[ 51.773716][ T1] ? blk_throtl_init (??:?)
[ 51.774886][ T1] blkcg_init_disk (??:?)
[ 51.775995][ T1] __alloc_disk_node (??:?)
[ 51.777114][ T1] __blk_alloc_disk (??:?)
[ 51.778193][ T1] brd_alloc (brd.c:?)
[ 51.779308][ T1] ? brd_lookup_page (brd.c:?)
[ 51.780458][ T1] ? preempt_count_add (??:?)
[ 51.793870][ T1] ? __up_write (rwsem.c:?)
[ 51.794957][ T1] ? debugfs_create_dir (??:?)
[ 51.796180][ T1] ? ramdisk_size (brd.c:?)
[ 51.797244][ T1] brd_init (brd.c:?)
[ 51.798304][ T1] ? ramdisk_size (brd.c:?)
[ 51.799393][ T1] do_one_initcall (??:?)
[ 51.800434][ T1] ? trace_event_raw_event_initcall_level (??:?)
[ 51.801841][ T1] ? parse_one (??:?)
[ 51.802859][ T1] ? lock_is_held_type (??:?)
[ 51.803992][ T1] ? lock_is_held_type (??:?)
[ 51.805144][ T1] do_initcalls (main.c:?)
[ 51.806214][ T1] kernel_init_freeable (main.c:?)
[ 51.807402][ T1] ? console_on_rootfs (main.c:?)
[ 51.808492][ T1] ? usleep_range_state (??:?)
[ 51.809692][ T1] ? rest_init (main.c:?)
[ 51.810700][ T1] ? rest_init (main.c:?)
[ 51.811697][ T1] kernel_init (main.c:?)
[ 51.812690][ T1] ret_from_fork (??:?)
[ 51.813692][ T1] </TASK>
[ 51.814465][ T1] irq event stamp: 167753
[ 51.815453][ T1] hardirqs last enabled at (167765): __up_console_sem (printk.c:?)
[ 51.817435][ T1] hardirqs last disabled at (167778): __up_console_sem (printk.c:?)
[ 51.819412][ T1] softirqs last enabled at (167580): __do_softirq (??:?)
[ 51.825500][ T1] softirqs last disabled at (167575): __irq_exit_rcu (softirq.c:?)
[ 51.827549][ T1] ---[ end trace 0000000000000000 ]---
If you fix the issue, kindly add following tag
| Reported-by: kernel test robot <yujie.liu@intel.com>
| Link: https://lore.kernel.org/oe-lkp/202210311456.2d2c7899-yujie.liu@intel.com
To reproduce:
# build kernel
cd linux
cp config-6.1.0-rc2-00131-gc39dd1922bb1 .config
make HOSTCC=gcc-11 CC=gcc-11 ARCH=x86_64 olddefconfig prepare modules_prepare bzImage modules
make HOSTCC=gcc-11 CC=gcc-11 ARCH=x86_64 INSTALL_MOD_PATH=<mod-install-dir> modules_install
cd <mod-install-dir>
find lib/ | cpio -o -H newc --quiet | gzip > modules.cgz
git clone https://github.com/intel/lkp-tests.git
cd lkp-tests
bin/lkp qemu -k <bzImage> -m modules.cgz job-script # job-script is attached in this email
# if come across any failure that blocks the test,
# please remove ~/.lkp and /lkp dir to run from a clean state.
>
> Greeting,
>
> FYI, we noticed WARNING:at_block/blk-mq.c:#blk_mq_freeze_queue due to commit (built with gcc-11):
>
Sorry for the disturbance.
The patch series is deprecated. Please ignore this.
Thanks!
Jinlong Chen
@@ -269,6 +269,19 @@ void blk_put_queue(struct request_queue *q)
}
EXPORT_SYMBOL(blk_put_queue);
+void blk_freeze_queue_start(struct request_queue *q)
+{
+ mutex_lock(&q->mq_freeze_lock);
+ if (++q->mq_freeze_depth == 1) {
+ percpu_ref_kill(&q->q_usage_counter);
+ mutex_unlock(&q->mq_freeze_lock);
+ if (queue_is_mq(q))
+ blk_mq_run_hw_queues(q, false);
+ } else {
+ mutex_unlock(&q->mq_freeze_lock);
+ }
+}
+
void blk_queue_start_drain(struct request_queue *q)
{
/*
@@ -161,19 +161,20 @@ void blk_mq_in_flight_rw(struct request_queue *q, struct block_device *part,
inflight[1] = mi.inflight[1];
}
-void blk_freeze_queue_start(struct request_queue *q)
+void blk_mq_freeze_queue_start(struct request_queue *q)
{
- mutex_lock(&q->mq_freeze_lock);
- if (++q->mq_freeze_depth == 1) {
- percpu_ref_kill(&q->q_usage_counter);
- mutex_unlock(&q->mq_freeze_lock);
- if (queue_is_mq(q))
- blk_mq_run_hw_queues(q, false);
- } else {
- mutex_unlock(&q->mq_freeze_lock);
- }
+ /*
+ * Warn on non-blk-mq usages.
+ */
+ WARN_ON_ONCE(!queue_is_mq(q));
+
+ /*
+ * Just an alias of blk_freeze_queue_start to keep the consistency of the
+ * blk_mq_* namespace.
+ */
+ blk_freeze_queue_start(q);
}
-EXPORT_SYMBOL_GPL(blk_freeze_queue_start);
+EXPORT_SYMBOL_GPL(blk_mq_freeze_queue_start);
void blk_mq_freeze_queue_wait(struct request_queue *q)
{
@@ -196,7 +197,7 @@ EXPORT_SYMBOL_GPL(blk_mq_freeze_queue_wait_timeout);
*/
void blk_mq_freeze_queue(struct request_queue *q)
{
- blk_freeze_queue_start(q);
+ blk_mq_freeze_queue_start(q);
blk_mq_freeze_queue_wait(q);
}
EXPORT_SYMBOL_GPL(blk_mq_freeze_queue);
@@ -1570,7 +1571,7 @@ static void blk_mq_timeout_work(struct work_struct *work)
* percpu_ref_tryget directly, because we need to be able to
* obtain a reference even in the short window between the queue
* starting to freeze, by dropping the first reference in
- * blk_freeze_queue_start, and the moment the last request is
+ * blk_mq_freeze_queue_start, and the moment the last request is
* consumed, marked by the instant q_usage_counter reaches
* zero.
*/
@@ -80,7 +80,7 @@ int blk_pre_runtime_suspend(struct request_queue *q)
blk_set_pm_only(q);
ret = -EBUSY;
/* Switch q_usage_counter from per-cpu to atomic mode. */
- blk_freeze_queue_start(q);
+ blk_mq_freeze_queue_start(q);
/*
* Wait until atomic mode has been reached. Since that
* involves calling call_rcu(), it is guaranteed that later
@@ -37,6 +37,7 @@ struct blk_flush_queue *blk_alloc_flush_queue(int node, int cmd_size,
gfp_t flags);
void blk_free_flush_queue(struct blk_flush_queue *q);
+void blk_freeze_queue_start(struct request_queue *q);
void __blk_mq_unfreeze_queue(struct request_queue *q, bool force_atomic);
void blk_queue_start_drain(struct request_queue *q);
int __bio_queue_enter(struct request_queue *q, struct bio *bio);
@@ -5199,7 +5199,7 @@ void nvme_start_freeze(struct nvme_ctrl *ctrl)
down_read(&ctrl->namespaces_rwsem);
list_for_each_entry(ns, &ctrl->namespaces, list)
- blk_freeze_queue_start(ns->queue);
+ blk_mq_freeze_queue_start(ns->queue);
up_read(&ctrl->namespaces_rwsem);
}
EXPORT_SYMBOL_GPL(nvme_start_freeze);
@@ -77,7 +77,7 @@ void nvme_mpath_start_freeze(struct nvme_subsystem *subsys)
lockdep_assert_held(&subsys->lock);
list_for_each_entry(h, &subsys->nsheads, entry)
if (h->disk)
- blk_freeze_queue_start(h->disk->queue);
+ blk_mq_freeze_queue_start(h->disk->queue);
}
void nvme_failover_req(struct request *req)
@@ -887,7 +887,7 @@ void blk_mq_tagset_busy_iter(struct blk_mq_tag_set *tagset,
void blk_mq_tagset_wait_completed_request(struct blk_mq_tag_set *tagset);
void blk_mq_freeze_queue(struct request_queue *q);
void blk_mq_unfreeze_queue(struct request_queue *q);
-void blk_freeze_queue_start(struct request_queue *q);
+void blk_mq_freeze_queue_start(struct request_queue *q);
void blk_mq_freeze_queue_wait(struct request_queue *q);
int blk_mq_freeze_queue_wait_timeout(struct request_queue *q,
unsigned long timeout);