linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Logan Gunthorpe <logang@deltatee.com>
To: Sagi Grimberg <sagi@grimberg.me>,
	linux-kernel@vger.kernel.org, linux-nvme@lists.infradead.org,
	linux-block@vger.kernel.org, linux-fsdevel@vger.kernel.org
Cc: Chaitanya Kulkarni <Chaitanya.Kulkarni@wdc.com>,
	Stephen Bates <sbates@raithlin.com>, Jens Axboe <axboe@fb.com>,
	Keith Busch <kbusch@kernel.org>, Max Gurtovoy <maxg@mellanox.com>,
	Christoph Hellwig <hch@lst.de>
Subject: Re: [PATCH v8 13/13] nvmet-passthru: support block accounting
Date: Wed, 11 Sep 2019 10:53:30 -0600	[thread overview]
Message-ID: <19da6ca1-3cde-3f29-59ee-923c562e6487@deltatee.com> (raw)
In-Reply-To: <ca811aea-c4ae-10ee-15a5-2332d5a9e29a@grimberg.me>



On 2019-09-09 5:15 p.m., Sagi Grimberg wrote:
> 
>>>>>> Support block disk accounting by setting the RQF_IO_STAT flag
>>>>>> and gendisk in the request.
>>>>>>
>>>>>> After this change, IO counts will be reflected correctly in
>>>>>> /proc/diskstats for drives being used by passthru.
>>>>>>
>>>>>> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
>>>>>> ---
>>>>>>     drivers/nvme/target/io-cmd-passthru.c | 5 ++++-
>>>>>>     1 file changed, 4 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/drivers/nvme/target/io-cmd-passthru.c
>>>>>> b/drivers/nvme/target/io-cmd-passthru.c
>>>>>> index 7557927a3451..63f12750a80d 100644
>>>>>> --- a/drivers/nvme/target/io-cmd-passthru.c
>>>>>> +++ b/drivers/nvme/target/io-cmd-passthru.c
>>>>>> @@ -410,6 +410,9 @@ static struct request
>>>>>> *nvmet_passthru_blk_make_request(struct nvmet_req *req,
>>>>>>         if (unlikely(IS_ERR(rq)))
>>>>>>             return rq;
>>>>>>     +    if (blk_queue_io_stat(q) && cmd->common.opcode !=
>>>>>> nvme_cmd_flush)
>>>>>> +        rq->rq_flags |= RQF_IO_STAT;
>>>>
>>>> Thanks for the review!
>>>>
>>>>> Does flush has data bytes in the request? Why the special casing?
>>>>
>>>> Well it was special cased in the vanilla blk account flow... But I
>>>> think
>>>> it's required to be special cased so the IO and in_flight counts don't
>>>> count flushes (as they do not for regular block device traffic).
>>>
>>> I think that the accounting exclude I/O that is yielded from the flush
>>> sequence. Don't think its relevant here...
>>
>> What? Per blk_account_io_done(), RQF_FLUSH_SEQ will not be set by us for
>> passthru commands and I don't think it's appropriate to do so. Thus, if
>> we set RQF_IO_STAT for passthru flush commands, they will be counted
>> which we do not want.
> 
> Have you considered to have nvmet_passthru_blk_make_request set RQF_FUA
> for nvme_cmd_flush? this way blk_insert_flush will be called and
> RQF_FLUSH_SEQ will be set and you don't need to worry about this
> special casing...

Well, I haven't done that mostly because I'm not sure of all the side
effects of doing so. However, I've tried it and everything seems to
still work, so I'll make that change for v9 which I'll probably send out
after the merge window.

Logan

      reply	other threads:[~2019-09-11 16:53 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-28 21:54 [PATCH v8 00/13] nvmet: add target passthru commands support Logan Gunthorpe
2019-08-28 21:54 ` [PATCH v8 01/13] nvme-core: introduce nvme_ctrl_get_by_path() Logan Gunthorpe
2019-09-06 23:33   ` Sagi Grimberg
2019-08-28 21:54 ` [PATCH v8 02/13] nvme-core: export existing ctrl and ns interfaces Logan Gunthorpe
2019-09-06 23:33   ` Sagi Grimberg
2019-08-28 21:54 ` [PATCH v8 03/13] nvmet: add return value to nvmet_add_async_event() Logan Gunthorpe
2019-09-06 23:34   ` Sagi Grimberg
2019-08-28 21:54 ` [PATCH v8 04/13] nvmet: make nvmet_copy_ns_identifier() non-static Logan Gunthorpe
2019-09-06 23:34   ` Sagi Grimberg
2019-08-28 21:54 ` [PATCH v8 05/13] nvmet-passthru: update KConfig with config passthru option Logan Gunthorpe
2019-09-06 23:35   ` Sagi Grimberg
2019-08-28 21:54 ` [PATCH v8 06/13] nvmet-passthru: add passthru code to process commands Logan Gunthorpe
2019-08-28 21:54 ` [PATCH v8 07/13] nvmet-passthru: add enable/disable helpers Logan Gunthorpe
2019-08-28 21:54 ` [PATCH v8 08/13] nvmet-core: don't check the data len for pt-ctrl Logan Gunthorpe
2019-09-06 23:38   ` Sagi Grimberg
2019-08-28 21:54 ` [PATCH v8 09/13] nvmet-tcp: don't check data_len in nvmet_tcp_map_data() Logan Gunthorpe
2019-09-06 23:39   ` Sagi Grimberg
2019-08-28 21:54 ` [PATCH v8 10/13] nvmet-configfs: introduce passthru configfs interface Logan Gunthorpe
2019-09-06 23:39   ` Sagi Grimberg
2019-08-28 21:54 ` [PATCH v8 11/13] block: don't check blk_rq_is_passthrough() in blk_do_io_stat() Logan Gunthorpe
2019-09-06 23:40   ` Sagi Grimberg
2019-08-28 21:54 ` [PATCH v8 12/13] block: call blk_account_io_start() in blk_execute_rq_nowait() Logan Gunthorpe
2019-09-06 23:41   ` Sagi Grimberg
2019-08-28 21:54 ` [PATCH v8 13/13] nvmet-passthru: support block accounting Logan Gunthorpe
2019-09-07  0:00   ` Sagi Grimberg
2019-09-09 16:06     ` Logan Gunthorpe
2019-09-09 19:26       ` Sagi Grimberg
2019-09-09 19:32         ` Logan Gunthorpe
2019-09-09 23:15           ` Sagi Grimberg
2019-09-11 16:53             ` Logan Gunthorpe [this message]

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=19da6ca1-3cde-3f29-59ee-923c562e6487@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).