From: Logan Gunthorpe <logang@deltatee.com>
To: Keith Busch <kbusch@kernel.org>
Cc: linux-kernel@vger.kernel.org, linux-nvme@lists.infradead.org,
Christoph Hellwig <hch@lst.de>, Sagi Grimberg <sagi@grimberg.me>,
Jens Axboe <axboe@fb.com>,
Chaitanya Kulkarni <Chaitanya.Kulkarni@wdc.com>,
Max Gurtovoy <maxg@mellanox.com>,
Stephen Bates <sbates@raithlin.com>
Subject: Re: [PATCH v12 1/9] nvme-core: Clear any SGL flags in passthru commands
Date: Mon, 20 Apr 2020 16:34:14 -0600 [thread overview]
Message-ID: <ce687c5d-eb19-e9b5-e03a-97dd2a04b4ea@deltatee.com> (raw)
In-Reply-To: <20200420222646.GA11115@redsun51.ssa.fujisawa.hgst.com>
On 2020-04-20 4:26 p.m., Keith Busch wrote:
> On Mon, Apr 20, 2020 at 10:46:52AM -0600, Logan Gunthorpe wrote:
>> The host driver should decide whether to use SGLs or PRPs and they
>> currently assume the flags are cleared after the call to
>> nvme_setup_cmd(). However, passed-through commands may erroneously
>> set these bits; so clear them for all cases.
>>
>> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
>> Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
>> ---
>> drivers/nvme/host/core.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
>> index 91c1bd659947..f5283b300e87 100644
>> --- a/drivers/nvme/host/core.c
>> +++ b/drivers/nvme/host/core.c
>> @@ -756,6 +756,8 @@ blk_status_t nvme_setup_cmd(struct nvme_ns *ns, struct request *req,
>> case REQ_OP_DRV_IN:
>> case REQ_OP_DRV_OUT:
>> memcpy(cmd, nvme_req(req)->cmd, sizeof(*cmd));
>> + /* passthru commands should let the driver set the SGL flags */
>> + cmd->common.flags &= ~NVME_CMD_SGL_ALL;
>> break;
>
> Is this really necessary? The passthrough handler currently rejects user
> commands that set command flags:
Yes, the flags coming from the passthru target's host (in subsequent
patches in this series) may have these set and we need to clear them
somewhere. The passthru code submits the requests directly using
blk_execute_rq_nowait() and thus the check in nvme_user_cmd() doesn't apply.
If I recall correctly, we had originally cleared the flags in the target
code, but Christoph had suggested it should be done more generally in
nvme_setup_cmd().
Logan
next prev parent reply other threads:[~2020-04-20 22:34 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-04-20 16:46 [PATCH v12 0/9] nvmet: add target passthru commands support Logan Gunthorpe
2020-04-20 16:46 ` [PATCH v12 1/9] nvme-core: Clear any SGL flags in passthru commands Logan Gunthorpe
2020-04-20 22:26 ` Keith Busch
2020-04-20 22:34 ` Logan Gunthorpe [this message]
2020-04-20 16:46 ` [PATCH v12 2/9] nvme: Create helper function to obtain command effects Logan Gunthorpe
2020-04-20 16:46 ` [PATCH v12 3/9] nvme: Move nvme_passthru_[start|end]() calls to common helper Logan Gunthorpe
2020-04-20 16:46 ` [PATCH v12 4/9] nvmet-passthru: Introduce NVMet passthru Kconfig option Logan Gunthorpe
2020-04-21 6:48 ` Sagi Grimberg
2020-04-20 16:46 ` [PATCH v12 5/9] nvme-core: Introduce nvme_ctrl_get_by_path() Logan Gunthorpe
2020-04-20 16:46 ` [PATCH v12 6/9] nvme: Export existing nvme core functions Logan Gunthorpe
2020-04-20 16:46 ` [PATCH v12 7/9] nvmet-passthru: Add passthru code to process commands Logan Gunthorpe
2020-04-21 6:50 ` Sagi Grimberg
2020-04-20 16:46 ` [PATCH v12 8/9] nvmet-passthru: Add enable/disable helpers Logan Gunthorpe
2020-04-21 6:48 ` Sagi Grimberg
2020-04-20 16:47 ` [PATCH v12 9/9] nvmet-configfs: Introduce passthru configfs interface 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=ce687c5d-eb19-e9b5-e03a-97dd2a04b4ea@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-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).