From: Logan Gunthorpe <logang@deltatee.com>
To: Christoph Hellwig <hch@lst.de>
Cc: linux-kernel@vger.kernel.org, linux-nvme@lists.infradead.org,
linux-block@vger.kernel.org, linux-fsdevel@vger.kernel.org,
Sagi Grimberg <sagi@grimberg.me>, Keith Busch <kbusch@kernel.org>,
Jens Axboe <axboe@fb.com>,
Chaitanya Kulkarni <Chaitanya.Kulkarni@wdc.com>,
Max Gurtovoy <maxg@mellanox.com>,
Stephen Bates <sbates@raithlin.com>
Subject: Re: [PATCH v9 10/12] block: don't check blk_rq_is_passthrough() in blk_do_io_stat()
Date: Thu, 10 Oct 2019 11:56:48 -0600 [thread overview]
Message-ID: <d0b7e064-2e28-83b0-1593-405ab75d5bd2@deltatee.com> (raw)
In-Reply-To: <20191010100526.GA27209@lst.de>
Hey,
Thanks for the thorough review, lots here to go through. I'll address it
all as I have time and try to get the prep work done first, as soon as I
can.
On 2019-10-10 4:05 a.m., Christoph Hellwig wrote:
>> @@ -319,7 +319,7 @@ static struct request *blk_mq_rq_ctx_init(struct blk_mq_alloc_data *data,
>> rq->cmd_flags = op;
>> if (data->flags & BLK_MQ_REQ_PREEMPT)
>> rq->rq_flags |= RQF_PREEMPT;
>> - if (blk_queue_io_stat(data->q))
>> + if (blk_queue_io_stat(data->q) && !blk_rq_is_passthrough(rq))
>> rq->rq_flags |= RQF_IO_STAT;
>
> This needs a comment why we don't account passthrough requests by
> default. And I'm really curious about the answer, because I don't
> know it myself.
Yes, sadly, I don't know this answer either but the comment made it
appear that someone did it deliberately. Digging into git blame suggests
that it just evolved that way. The check was originally added in 2005
here with blk_fs_request():
commit d72d904a5367 ("[BLOCK] Update read/write block io statistics at
completion time")
blk_fs_request() evolved to become blk_rq_is_passthrough() but I suspect
no one ever considered whether we want to account the passthru requests.
So, I'll leave this restriction out and see if anyone complains.
Logan
>> * a) it's attached to a gendisk, and
>> * b) the queue had IO stats enabled when this request was started, and
>> - * c) it's a file system request
>> + * c) it's a file system request (RQF_IO_STAT will not be set otherwise)
>
> c) should just go away now based on your changes.
>
>> static inline bool blk_do_io_stat(struct request *rq)
>> {
>> return rq->rq_disk &&
>> - (rq->rq_flags & RQF_IO_STAT) &&
>> - !blk_rq_is_passthrough(rq);
>> + (rq->rq_flags & RQF_IO_STAT);
>
> The check can be collapsed onto a single line now.
>
next prev parent reply other threads:[~2019-10-10 17:57 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-10-09 19:25 [PATCH v9 00/12] nvmet: add target passthru commands support Logan Gunthorpe
2019-10-09 19:25 ` [PATCH v9 01/12] nvme-core: introduce nvme_ctrl_get_by_path() Logan Gunthorpe
2019-10-09 22:13 ` Keith Busch
2019-10-10 11:37 ` Christoph Hellwig
2019-10-09 19:25 ` [PATCH v9 02/12] nvme-core: export existing ctrl and ns interfaces Logan Gunthorpe
2019-10-09 22:14 ` Keith Busch
2019-10-09 19:25 ` [PATCH v9 03/12] nvmet: add return value to nvmet_add_async_event() Logan Gunthorpe
2019-10-09 22:17 ` Keith Busch
2019-10-09 19:25 ` [PATCH v9 04/12] nvmet: make nvmet_copy_ns_identifier() non-static Logan Gunthorpe
2019-10-09 22:18 ` Keith Busch
2019-10-10 11:50 ` Christoph Hellwig
2019-10-09 19:25 ` [PATCH v9 05/12] Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com> [logang@deltatee.com: fixed some of the wording in the help message] Signed-off-by: Logan Gunthorpe <logang@deltatee.com> Reviewed-by: Max Gurtovoy <maxg@mellanox.com> Logan Gunthorpe
2019-10-10 12:34 ` Christoph Hellwig
2019-10-25 23:09 ` [PATCH v9 05/12] Logan Gunthorpe
2019-10-09 19:25 ` [PATCH v9 05/12] nvmet-passthru: add passthru code to process commands Logan Gunthorpe
2019-10-09 19:25 ` [PATCH v9 06/12] nvmet-passthru: add enable/disable helpers Logan Gunthorpe
2019-10-09 19:25 ` [PATCH v9 07/12] nvmet-core: don't check the data len for pt-ctrl Logan Gunthorpe
2019-10-10 11:04 ` Christoph Hellwig
2019-10-09 19:25 ` [PATCH v9 08/12] nvmet-tcp: don't check data_len in nvmet_tcp_map_data() Logan Gunthorpe
2019-10-10 11:07 ` Christoph Hellwig
2019-10-09 19:25 ` [PATCH v9 09/12] nvmet-configfs: introduce passthru configfs interface Logan Gunthorpe
2019-10-09 19:25 ` [PATCH v9 10/12] block: don't check blk_rq_is_passthrough() in blk_do_io_stat() Logan Gunthorpe
2019-10-10 10:05 ` Christoph Hellwig
2019-10-10 17:56 ` Logan Gunthorpe [this message]
2019-10-09 19:25 ` [PATCH v9 11/12] block: call blk_account_io_start() in blk_execute_rq_nowait() Logan Gunthorpe
2019-10-10 10:06 ` Christoph Hellwig
2019-10-09 19:25 ` [PATCH v9 12/12] nvmet-passthru: support block accounting Logan Gunthorpe
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=d0b7e064-2e28-83b0-1593-405ab75d5bd2@deltatee.com \
--to=logang@deltatee.com \
--cc=Chaitanya.Kulkarni@wdc.com \
--cc=axboe@fb.com \
--cc=hch@lst.de \
--cc=kbusch@kernel.org \
--cc=linux-block@vger.kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-nvme@lists.infradead.org \
--cc=maxg@mellanox.com \
--cc=sagi@grimberg.me \
--cc=sbates@raithlin.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).