diff for duplicates of <1489771915.2826.4.camel@sandisk.com> diff --git a/a/1.txt b/N1/1.txt index a256415..09adfef 100644 --- a/a/1.txt +++ b/N1/1.txt @@ -4,22 +4,22 @@ On Fri, 2017-03-17 at 17:57 +0800, Ming Lei wrote: > entering queue, and Tejun converts the checking into .mq_freeze_depth, > and assumes the counter is increased just after dying flag > is set. Unfortunately we doesn't do that in blk_set_queue_dying(). ->=20 +> > This patch calls blk_mq_freeze_queue_start() for blk-mq in > blk_set_queue_dying(), so that we can block new I/O coming > once the queue is set as dying. ->=20 +> > Given blk_set_queue_dying() is always called in remove path > of block device, and queue will be cleaned up later, we don't > need to worry about undoing the counter. ->=20 +> > Cc: Bart Van Assche <bart.vanassche@sandisk.com> > Cc: Tejun Heo <tj@kernel.org> > Signed-off-by: Ming Lei <tom.leiming@gmail.com> > --- > block/blk-core.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) ->=20 +> > diff --git a/block/blk-core.c b/block/blk-core.c > index d772c221cc17..62d4967c369f 100644 > --- a/block/blk-core.c @@ -27,7 +27,7 @@ On Fri, 2017-03-17 at 17:57 +0800, Ming Lei wrote: > @@ -500,9 +500,12 @@ void blk_set_queue_dying(struct request_queue *q) > queue_flag_set(QUEUE_FLAG_DYING, q); > spin_unlock_irq(q->queue_lock); -> =20 +> > - if (q->mq_ops) > + if (q->mq_ops) { > blk_mq_wake_waiters(q); @@ -37,7 +37,7 @@ On Fri, 2017-03-17 at 17:57 +0800, Ming Lei wrote: > + blk_mq_freeze_queue_start(q); > + } else { > struct request_list *rl; -> =20 +> > spin_lock_irq(q->queue_lock); Hello Ming, @@ -45,12 +45,11 @@ Hello Ming, I think we need the queue freezing not only for blk-mq but also for blk-sq. Since the queue flags and the mq_freeze_depth are stored in different variables we need to prevent that the CPU reorders the stores to these -variables. The comment about=A0blk_mq_freeze_queue_start() should be more +variables. The comment about blk_mq_freeze_queue_start() should be more clear. How about something like the patch below? -[PATCH] blk-mq: Force block layer users to check the "dying" flag=A0after i= -t has been set +[PATCH] blk-mq: Force block layer users to check the "dying" flag after it has been set Commit 780db2071ac4 removed the blk_queue_dying() check from the hot path of blk_mq_queue_enter() although that check is necessary @@ -60,36 +59,35 @@ and blk_mq_queue_enter() check the dying flag after it has been set. Because blk_set_queue_dying() is only called from the remove path of a block device we don't need to worry about unfreezing the queue. -Fixes: commit 780db2071ac4 ("blk-mq: decouble blk-mq freezing from generic = -bypassing") +Fixes: commit 780db2071ac4 ("blk-mq: decouble blk-mq freezing from generic bypassing") --- -=A0block/blk-core.c | 13 +++++++++++++ -=A01 file changed, 13 insertions(+) + block/blk-core.c | 13 +++++++++++++ + 1 file changed, 13 insertions(+) diff --git a/block/blk-core.c b/block/blk-core.c index d772c221cc17..730f715b72ff 100644 --- a/block/blk-core.c +++ b/block/blk-core.c @@ -500,6 +500,19 @@ void blk_set_queue_dying(struct request_queue *q) -=A0 queue_flag_set(QUEUE_FLAG_DYING, q); -=A0 spin_unlock_irq(q->queue_lock); -=A0 + queue_flag_set(QUEUE_FLAG_DYING, q); + spin_unlock_irq(q->queue_lock); + + /* -+ =A0* Avoid that the updates of the queue flags and q_usage_counter -+ =A0* are reordered. -+ =A0*/ ++ * Avoid that the updates of the queue flags and q_usage_counter ++ * are reordered. ++ */ + smp_wmb(); + + /* -+ =A0* Force blk_queue_enter() and blk_mq_queue_enter() to check the -+ =A0* "dying" flag. Despite its name, blk_mq_freeze_queue_start() -+ =A0* affects blk-sq and blk-mq queues. -+ =A0*/ ++ * Force blk_queue_enter() and blk_mq_queue_enter() to check the ++ * "dying" flag. Despite its name, blk_mq_freeze_queue_start() ++ * affects blk-sq and blk-mq queues. ++ */ + blk_mq_freeze_queue_start(q); + -=A0 if (q->mq_ops) -=A0 blk_mq_wake_waiters(q); -=A0 else { + if (q->mq_ops) + blk_mq_wake_waiters(q); + else { Thanks, diff --git a/a/content_digest b/N1/content_digest index fe6fb20..f75284a 100644 --- a/a/content_digest +++ b/N1/content_digest @@ -18,22 +18,22 @@ "> entering queue, and Tejun converts the checking into .mq_freeze_depth,\n" "> and assumes the counter is increased just after dying flag\n" "> is set. Unfortunately we doesn't do that in blk_set_queue_dying().\n" - ">=20\n" + "> \n" "> This patch calls blk_mq_freeze_queue_start() for blk-mq in\n" "> blk_set_queue_dying(), so that we can block new I/O coming\n" "> once the queue is set as dying.\n" - ">=20\n" + "> \n" "> Given blk_set_queue_dying() is always called in remove path\n" "> of block device, and queue will be cleaned up later, we don't\n" "> need to worry about undoing the counter.\n" - ">=20\n" + "> \n" "> Cc: Bart Van Assche <bart.vanassche@sandisk.com>\n" "> Cc: Tejun Heo <tj@kernel.org>\n" "> Signed-off-by: Ming Lei <tom.leiming@gmail.com>\n" "> ---\n" "> block/blk-core.c | 7 +++++--\n" "> 1 file changed, 5 insertions(+), 2 deletions(-)\n" - ">=20\n" + "> \n" "> diff --git a/block/blk-core.c b/block/blk-core.c\n" "> index d772c221cc17..62d4967c369f 100644\n" "> --- a/block/blk-core.c\n" @@ -41,7 +41,7 @@ "> @@ -500,9 +500,12 @@ void blk_set_queue_dying(struct request_queue *q)\n" "> \tqueue_flag_set(QUEUE_FLAG_DYING, q);\n" "> \tspin_unlock_irq(q->queue_lock);\n" - "> =20\n" + "> \n" "> -\tif (q->mq_ops)\n" "> +\tif (q->mq_ops) {\n" "> \t\tblk_mq_wake_waiters(q);\n" @@ -51,7 +51,7 @@ "> +\t\tblk_mq_freeze_queue_start(q);\n" "> +\t} else {\n" "> \t\tstruct request_list *rl;\n" - "> =20\n" + "> \n" "> \t\tspin_lock_irq(q->queue_lock);\n" "\n" "Hello Ming,\n" @@ -59,12 +59,11 @@ "I think we need the queue freezing not only for blk-mq but also for blk-sq.\n" "Since the queue flags and the mq_freeze_depth are stored in different\n" "variables we need to prevent that the CPU reorders the stores to these\n" - "variables. The comment about=A0blk_mq_freeze_queue_start() should be more\n" + "variables. The comment about\302\240blk_mq_freeze_queue_start() should be more\n" "clear. How about something like the patch below?\n" "\n" "\n" - "[PATCH] blk-mq: Force block layer users to check the \"dying\" flag=A0after i=\n" - "t has been set\n" + "[PATCH] blk-mq: Force block layer users to check the \"dying\" flag\302\240after it has been set\n" "\n" "Commit 780db2071ac4 removed the blk_queue_dying() check from the\n" "hot path of blk_mq_queue_enter() although that check is necessary\n" @@ -74,40 +73,39 @@ "Because blk_set_queue_dying() is only called from the remove path\n" "of a block device we don't need to worry about unfreezing the queue.\n" "\n" - "Fixes: commit 780db2071ac4 (\"blk-mq: decouble blk-mq freezing from generic =\n" - "bypassing\")\n" + "Fixes: commit 780db2071ac4 (\"blk-mq: decouble blk-mq freezing from generic bypassing\")\n" "---\n" - "=A0block/blk-core.c | 13 +++++++++++++\n" - "=A01 file changed, 13 insertions(+)\n" + "\302\240block/blk-core.c | 13 +++++++++++++\n" + "\302\2401 file changed, 13 insertions(+)\n" "\n" "diff --git a/block/blk-core.c b/block/blk-core.c\n" "index d772c221cc17..730f715b72ff 100644\n" "--- a/block/blk-core.c\n" "+++ b/block/blk-core.c\n" "@@ -500,6 +500,19 @@ void blk_set_queue_dying(struct request_queue *q)\n" - "=A0\tqueue_flag_set(QUEUE_FLAG_DYING, q);\n" - "=A0\tspin_unlock_irq(q->queue_lock);\n" - "=A0\n" + "\302\240\tqueue_flag_set(QUEUE_FLAG_DYING, q);\n" + "\302\240\tspin_unlock_irq(q->queue_lock);\n" + "\302\240\n" "+\t/*\n" - "+\t=A0* Avoid that the updates of the queue flags and q_usage_counter\n" - "+\t=A0* are reordered.\n" - "+\t=A0*/\n" + "+\t\302\240* Avoid that the updates of the queue flags and q_usage_counter\n" + "+\t\302\240* are reordered.\n" + "+\t\302\240*/\n" "+\tsmp_wmb();\n" "+\n" "+\t/*\n" - "+\t=A0* Force blk_queue_enter() and blk_mq_queue_enter() to check the\n" - "+\t=A0* \"dying\" flag. Despite its name, blk_mq_freeze_queue_start()\n" - "+\t=A0* affects blk-sq and blk-mq queues.\n" - "+\t=A0*/\n" + "+\t\302\240* Force blk_queue_enter() and blk_mq_queue_enter() to check the\n" + "+\t\302\240* \"dying\" flag. Despite its name, blk_mq_freeze_queue_start()\n" + "+\t\302\240* affects blk-sq and blk-mq queues.\n" + "+\t\302\240*/\n" "+\tblk_mq_freeze_queue_start(q);\n" "+\n" - "=A0\tif (q->mq_ops)\n" - "=A0\t\tblk_mq_wake_waiters(q);\n" - "=A0\telse {\n" + "\302\240\tif (q->mq_ops)\n" + "\302\240\t\tblk_mq_wake_waiters(q);\n" + "\302\240\telse {\n" "\n" "\n" "Thanks,\n" "\n" Bart. -d0292a51a7bdaf4f41da572822804ce15dd27f19a430c79be2b84e85895e7396 +154b6af740fabc22aa6be92d3b8d851998db1171583b9215787b5ba7285b31ed
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.