diff for duplicates of <1517337524.15224.0.camel@redhat.com> diff --git a/a/1.txt b/N1/1.txt index 096bf86..91e3a45 100644 --- a/a/1.txt +++ b/N1/1.txt @@ -1,69 +1,69 @@ -On Tue, 2018-01-30 at 09:52 -0800, Bart Van Assche wrote: +On Tue, 2018-01-30@09:52 -0800, Bart Van Assche wrote: > On 01/30/18 06:24, Mike Snitzer wrote: -> > + * -> > + * If driver returns BLK_STS_RESOURCE and +> > + ?* +> > + ?* If driver returns BLK_STS_RESOURCE and > > SCHED_RESTART -> > + * bit is set, run queue after a delay to avoid IO +> > + ?* bit is set, run queue after a delay to avoid IO > > stalls -> > + * that could otherwise occur if the queue is +> > + ?* that could otherwise occur if the queue is > > idle. -> > */ +> > ?? ?*/ > > - if (!blk_mq_sched_needs_restart(hctx) || > > + needs_restart = blk_mq_sched_needs_restart(hctx); > > + if (!needs_restart || -> > (no_tag && list_empty_careful(&hctx- +> > ?? ????(no_tag && list_empty_careful(&hctx- > > >dispatch_wait.entry))) -> > blk_mq_run_hw_queue(hctx, true); +> > ?? blk_mq_run_hw_queue(hctx, true); > > + else if (needs_restart && (ret == > > BLK_STS_RESOURCE)) > > + blk_mq_delay_run_hw_queue(hctx, > > BLK_MQ_QUEUE_DELAY); -> > } +> > ?? } > -> If a request completes concurrently with execution of the above code -> then the request completion will trigger a call of -> blk_mq_sched_restart_hctx() and that call will clear the +> If a request completes concurrently with execution of the above code? +> then the request completion will trigger a call of? +> blk_mq_sched_restart_hctx() and that call will clear the? > BLK_MQ_S_SCHED_RESTART bit. If that bit is cleared before the above -> code -> tests it then the above code will schedule an asynchronous call of +> code? +> tests it then the above code will schedule an asynchronous call of? > __blk_mq_run_hw_queue(). If the .queue_rq() call triggered by the -> new -> queue run returns again BLK_STS_RESOURCE then the above code will be +> new? +> queue run returns again BLK_STS_RESOURCE then the above code will be? > executed again. In other words, a loop occurs. That loop will repeat -> as -> long as the described race occurs. The current (kernel v4.15) block -> layer behavior is simpler: only block drivers call +> as? +> long as the described race occurs. The current (kernel v4.15) block? +> layer behavior is simpler: only block drivers call? > blk_mq_delay_run_hw_queue() and the block layer core never calls -> that +> that? > function. Hence that loop cannot occur with the v4.15 block layer -> core +> core? > and block drivers. A motivation of why that loop is preferred -> compared +> compared? > to the current behavior (no loop) is missing. Does this mean that -> that +> that? > loop is a needless complication of the block layer core? > -> Sorry but I still prefer the v4.15 block layer approach because this +> Sorry but I still prefer the v4.15 block layer approach because this? > patch has in my view the following disadvantages: > - It involves a blk-mq API change. API changes are always risky and > need -> some time to stabilize. +> ???some time to stabilize. > - The delay after which to rerun the queue is moved from block layer -> drivers into the block layer core. I think that's wrong because +> ???drivers into the block layer core. I think that's wrong because > only -> the block driver authors can make a good choice for this constant. +> ???the block driver authors can make a good choice for this constant. > - This patch makes block drivers harder to understand. Anyone who > sees -> return BLK_STS_RESOURCE / return BLK_STS_DEV_RESOURCE for the +> ???return BLK_STS_RESOURCE / return BLK_STS_DEV_RESOURCE for the > first -> time will have to look up the meaning of these constants. An +> ???time will have to look up the meaning of these constants. An > explicit -> blk_mq_delay_run_hw_queue() call is easier to understand. +> ???blk_mq_delay_run_hw_queue() call is easier to understand. > - This patch makes the blk-mq core harder to understand because of > the -> loop mentioned above. +> ???loop mentioned above. > - This patch does not fix any bugs nor makes block drivers easier to -> read or to implement. So why is this patch considered useful? +> ???read or to implement. So why is this patch considered useful? > > Thanks, > @@ -76,7 +76,7 @@ above. Is there more of a delay in your approach or in Ming's approach from your own testing. I guess it seems we will never get consensus on this so is it time to -take a vote. +take a vote.? I respect and trust your inputs, you know that, but are you perhaps prepared to accept the approach above and agree to it and if it turns diff --git a/a/content_digest b/N1/content_digest index 6811aa3..5525d3f 100644 --- a/a/content_digest +++ b/N1/content_digest @@ -1,83 +1,76 @@ "ref\020180130142459.52668-1-snitzer@redhat.com\0" "ref\043ac2314-c98d-bb76-0dfb-171d15cc5fd8@wdc.com\0" - "From\0Laurence Oberman <loberman@redhat.com>\0" - "Subject\0Re: [dm-devel] [PATCH v5] blk-mq: introduce BLK_STS_DEV_RESOURCE\0" + "From\0loberman@redhat.com (Laurence Oberman)\0" + "Subject\0[dm-devel] [PATCH v5] blk-mq: introduce BLK_STS_DEV_RESOURCE\0" "Date\0Tue, 30 Jan 2018 13:38:44 -0500\0" - "To\0Bart Van Assche <bart.vanassche@wdc.com>" - Mike Snitzer <snitzer@redhat.com> - " axboe@kernel.dk\0" - "Cc\0linux-block@vger.kernel.org" - dm-devel@redhat.com - linux-nvme@lists.infradead.org - " linux-scsi@vger.kernel.org\0" "\00:1\0" "b\0" - "On Tue, 2018-01-30 at 09:52 -0800, Bart Van Assche wrote:\n" + "On Tue, 2018-01-30@09:52 -0800, Bart Van Assche wrote:\n" "> On 01/30/18 06:24, Mike Snitzer wrote:\n" - "> > +\t\t\302\240*\n" - "> > +\t\t\302\240* If driver returns BLK_STS_RESOURCE and\n" + "> > +\t\t?*\n" + "> > +\t\t?* If driver returns BLK_STS_RESOURCE and\n" "> > SCHED_RESTART\n" - "> > +\t\t\302\240* bit is set, run queue after a delay to avoid IO\n" + "> > +\t\t?* bit is set, run queue after a delay to avoid IO\n" "> > stalls\n" - "> > +\t\t\302\240* that could otherwise occur if the queue is\n" + "> > +\t\t?* that could otherwise occur if the queue is\n" "> > idle.\n" - "> > \302\240\302\240\t\t\302\240*/\n" + "> > ??\t\t?*/\n" "> > -\t\tif (!blk_mq_sched_needs_restart(hctx) ||\n" "> > +\t\tneeds_restart = blk_mq_sched_needs_restart(hctx);\n" "> > +\t\tif (!needs_restart ||\n" - "> > \302\240\302\240\t\t\302\240\302\240\302\240\302\240(no_tag && list_empty_careful(&hctx-\n" + "> > ??\t\t????(no_tag && list_empty_careful(&hctx-\n" "> > >dispatch_wait.entry)))\n" - "> > \302\240\302\240\t\t\tblk_mq_run_hw_queue(hctx, true);\n" + "> > ??\t\t\tblk_mq_run_hw_queue(hctx, true);\n" "> > +\t\telse if (needs_restart && (ret ==\n" "> > BLK_STS_RESOURCE))\n" "> > +\t\t\tblk_mq_delay_run_hw_queue(hctx,\n" "> > BLK_MQ_QUEUE_DELAY);\n" - "> > \302\240\302\240\t}\n" + "> > ??\t}\n" "> \n" - "> If a request completes concurrently with execution of the above code\302\240\n" - "> then the request completion will trigger a call of\302\240\n" - "> blk_mq_sched_restart_hctx() and that call will clear the\302\240\n" + "> If a request completes concurrently with execution of the above code?\n" + "> then the request completion will trigger a call of?\n" + "> blk_mq_sched_restart_hctx() and that call will clear the?\n" "> BLK_MQ_S_SCHED_RESTART bit. If that bit is cleared before the above\n" - "> code\302\240\n" - "> tests it then the above code will schedule an asynchronous call of\302\240\n" + "> code?\n" + "> tests it then the above code will schedule an asynchronous call of?\n" "> __blk_mq_run_hw_queue(). If the .queue_rq() call triggered by the\n" - "> new\302\240\n" - "> queue run returns again BLK_STS_RESOURCE then the above code will be\302\240\n" + "> new?\n" + "> queue run returns again BLK_STS_RESOURCE then the above code will be?\n" "> executed again. In other words, a loop occurs. That loop will repeat\n" - "> as\302\240\n" - "> long as the described race occurs. The current (kernel v4.15) block\302\240\n" - "> layer behavior is simpler: only block drivers call\302\240\n" + "> as?\n" + "> long as the described race occurs. The current (kernel v4.15) block?\n" + "> layer behavior is simpler: only block drivers call?\n" "> blk_mq_delay_run_hw_queue() and the block layer core never calls\n" - "> that\302\240\n" + "> that?\n" "> function. Hence that loop cannot occur with the v4.15 block layer\n" - "> core\302\240\n" + "> core?\n" "> and block drivers. A motivation of why that loop is preferred\n" - "> compared\302\240\n" + "> compared?\n" "> to the current behavior (no loop) is missing. Does this mean that\n" - "> that\302\240\n" + "> that?\n" "> loop is a needless complication of the block layer core?\n" "> \n" - "> Sorry but I still prefer the v4.15 block layer approach because this\302\240\n" + "> Sorry but I still prefer the v4.15 block layer approach because this?\n" "> patch has in my view the following disadvantages:\n" "> - It involves a blk-mq API change. API changes are always risky and\n" "> need\n" - "> \302\240\302\240\302\240some time to stabilize.\n" + "> ???some time to stabilize.\n" "> - The delay after which to rerun the queue is moved from block layer\n" - "> \302\240\302\240\302\240drivers into the block layer core. I think that's wrong because\n" + "> ???drivers into the block layer core. I think that's wrong because\n" "> only\n" - "> \302\240\302\240\302\240the block driver authors can make a good choice for this constant.\n" + "> ???the block driver authors can make a good choice for this constant.\n" "> - This patch makes block drivers harder to understand. Anyone who\n" "> sees\n" - "> \302\240\302\240\302\240return BLK_STS_RESOURCE / return BLK_STS_DEV_RESOURCE for the\n" + "> ???return BLK_STS_RESOURCE / return BLK_STS_DEV_RESOURCE for the\n" "> first\n" - "> \302\240\302\240\302\240time will have to look up the meaning of these constants. An\n" + "> ???time will have to look up the meaning of these constants. An\n" "> explicit\n" - "> \302\240\302\240\302\240blk_mq_delay_run_hw_queue() call is easier to understand.\n" + "> ???blk_mq_delay_run_hw_queue() call is easier to understand.\n" "> - This patch makes the blk-mq core harder to understand because of\n" "> the\n" - "> \302\240\302\240\302\240loop mentioned above.\n" + "> ???loop mentioned above.\n" "> - This patch does not fix any bugs nor makes block drivers easier to\n" - "> \302\240\302\240\302\240read or to implement. So why is this patch considered useful?\n" + "> ???read or to implement. So why is this patch considered useful?\n" "> \n" "> Thanks,\n" "> \n" @@ -90,7 +83,7 @@ "Is there more of a delay in your approach or in Ming's approach from\n" "your own testing.\n" "I guess it seems we will never get consensus on this so is it time to\n" - "take a vote.\302\240\n" + "take a vote.?\n" "\n" "I respect and trust your inputs, you know that, but are you perhaps\n" "prepared to accept the approach above and agree to it and if it turns\n" @@ -102,4 +95,4 @@ "With much respect\n" Laurence -e31a047fe51fa67f4164952a3867a2c57650158ef2958b2b9657c4227bfa8c74 +65ab7df49529b609dfa00fa40cf6d52280ca519d3cd8da33a8106f276f4b3150
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.