| Message ID | 1457458494-10550-2-git-send-email-jonathan.derrick@intel.com (mailing list archive) |
|---|---|
| State | Rejected, archived |
| Delegated to: | Jens Axboe |
| Headers | show |
> Extends iostats to encompass polling statistics to save a few cycles > when disabled. > > Signed-off-by: Jon Derrick <jonathan.derrick@intel.com> > --- > block/blk-core.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/block/blk-core.c b/block/blk-core.c > index ab51685..354d03b 100644 > --- a/block/blk-core.c > +++ b/block/blk-core.c > @@ -3350,13 +3350,16 @@ bool blk_poll(struct request_queue *q, blk_qc_t cookie) > while (!need_resched()) { > unsigned int queue_num = blk_qc_t_to_queue_num(cookie); > struct blk_mq_hw_ctx *hctx = q->queue_hw_ctx[queue_num]; > + int io_stat = blk_queue_io_stat(q); > int ret; > > - hctx->poll_invoked++; > + if (io_stat) > + hctx->poll_invoked++; > > ret = q->mq_ops->poll(hctx, blk_qc_t_to_tag(cookie)); > if (ret > 0) { > - hctx->poll_success++; > + if (io_stat) > + hctx->poll_success++; > set_current_state(TASK_RUNNING); > return true; > } I fail to see how replacing incrementation with a branch statement helps performance or even not making it worse... -- 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
On 03/08/2016 10:34 AM, Jon Derrick wrote: > Extends iostats to encompass polling statistics to save a few cycles > when disabled. > > Signed-off-by: Jon Derrick <jonathan.derrick@intel.com> > --- > block/blk-core.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/block/blk-core.c b/block/blk-core.c > index ab51685..354d03b 100644 > --- a/block/blk-core.c > +++ b/block/blk-core.c > @@ -3350,13 +3350,16 @@ bool blk_poll(struct request_queue *q, blk_qc_t cookie) > while (!need_resched()) { > unsigned int queue_num = blk_qc_t_to_queue_num(cookie); > struct blk_mq_hw_ctx *hctx = q->queue_hw_ctx[queue_num]; > + int io_stat = blk_queue_io_stat(q); > int ret; > > - hctx->poll_invoked++; > + if (io_stat) > + hctx->poll_invoked++; > > ret = q->mq_ops->poll(hctx, blk_qc_t_to_tag(cookie)); > if (ret > 0) { > - hctx->poll_success++; > + if (io_stat) > + hctx->poll_success++; > set_current_state(TASK_RUNNING); > return true; > } > Not sure this is a great idea. First of all, the poll stats are per hardware queue. How many submission queues and CPUs do you have in your setup? For most cases, I'd assume there'd be a 1:1 mapping between the two, which makes the stats essentially free. And secondly, even for a less optimal mapping, the poll stats are a lot cheaper than the io stats. So bundling them together might not make a ton of sense.
> > I fail to see how replacing incrementation with a branch statement helps > performance or even not making it worse... It may have been coincidental in my setup - I had assumed the test on the local io_stat was cheaper than the cost of following hctx to then increment (or not) poll_invoked. -- 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
> Not sure this is a great idea. First of all, the poll stats are per hardware > queue. How many submission queues and CPUs do you have in your setup? For 31 hq, 64 cpus (so actually just 31 sq)- but the test I ran was: taskset -c 1 fio --name=global --gtod_reduce=1 --filename=/dev/nvme0n1 --bs=4k --rw=read --ioengine=sync --iodepth=1 --numjobs=1 --direct=1 --name=job I was a bit surprised at the delta I saw, but it may have been coincidental > most cases, I'd assume there'd be a 1:1 mapping between the two, which makes > the stats essentially free. And secondly, even for a less optimal mapping, > the poll stats are a lot cheaper than the io stats. So bundling them > together might not make a ton of sense. Fair enough :) -- 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/block/blk-core.c b/block/blk-core.c index ab51685..354d03b 100644 --- a/block/blk-core.c +++ b/block/blk-core.c @@ -3350,13 +3350,16 @@ bool blk_poll(struct request_queue *q, blk_qc_t cookie) while (!need_resched()) { unsigned int queue_num = blk_qc_t_to_queue_num(cookie); struct blk_mq_hw_ctx *hctx = q->queue_hw_ctx[queue_num]; + int io_stat = blk_queue_io_stat(q); int ret; - hctx->poll_invoked++; + if (io_stat) + hctx->poll_invoked++; ret = q->mq_ops->poll(hctx, blk_qc_t_to_tag(cookie)); if (ret > 0) { - hctx->poll_success++; + if (io_stat) + hctx->poll_success++; set_current_state(TASK_RUNNING); return true; }
Extends iostats to encompass polling statistics to save a few cycles when disabled. Signed-off-by: Jon Derrick <jonathan.derrick@intel.com> --- block/blk-core.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)