Skip to content

Commit

Permalink
nvmet: Remove the data_len field from the nvmet_req struct
Browse files Browse the repository at this point in the history 
Instead of storing the expected length and checking it when it's
executed, just check the length inside the command themselves.

A new helper, nvmet_check_data_len() is created to help with this
check.

Signed-off-by: Christoph Hellwig <hch@lst.de>
[split patch, udpate changelog]
Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
Signed-off-by: Keith Busch <kbusch@kernel.org>
Signed-off-by: Jens Axboe <axboe@kernel.dk>
  • Loading branch information
Christoph Hellwig authored and Jens Axboe committed Nov 4, 2019
1 parent 59ef0ea commit e9061c3
Show file tree
Hide file tree
Showing 7 changed files with 86 additions and 39 deletions.
35 changes: 24 additions & 11 deletions drivers/nvme/target/admin-cmd.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ u64 nvmet_get_log_page_offset(struct nvme_command *cmd)

static void nvmet_execute_get_log_page_noop(struct nvmet_req *req)
{
nvmet_req_complete(req, nvmet_zero_sgl(req, 0, req->data_len));
nvmet_req_complete(req, nvmet_zero_sgl(req, 0, req->transfer_len));
}

static void nvmet_execute_get_log_page_error(struct nvmet_req *req)
Expand Down Expand Up @@ -134,7 +134,7 @@ static void nvmet_execute_get_log_page_smart(struct nvmet_req *req)
u16 status = NVME_SC_INTERNAL;
unsigned long flags;

if (req->data_len != sizeof(*log))
if (req->transfer_len != sizeof(*log))
goto out;

log = kzalloc(sizeof(*log), GFP_KERNEL);
Expand Down Expand Up @@ -196,7 +196,7 @@ static void nvmet_execute_get_log_changed_ns(struct nvmet_req *req)
u16 status = NVME_SC_INTERNAL;
size_t len;

if (req->data_len != NVME_MAX_CHANGED_NAMESPACES * sizeof(__le32))
if (req->transfer_len != NVME_MAX_CHANGED_NAMESPACES * sizeof(__le32))
goto out;

mutex_lock(&ctrl->lock);
Expand All @@ -206,7 +206,7 @@ static void nvmet_execute_get_log_changed_ns(struct nvmet_req *req)
len = ctrl->nr_changed_ns * sizeof(__le32);
status = nvmet_copy_to_sgl(req, 0, ctrl->changed_ns_list, len);
if (!status)
status = nvmet_zero_sgl(req, len, req->data_len - len);
status = nvmet_zero_sgl(req, len, req->transfer_len - len);
ctrl->nr_changed_ns = 0;
nvmet_clear_aen_bit(req, NVME_AEN_BIT_NS_ATTR);
mutex_unlock(&ctrl->lock);
Expand Down Expand Up @@ -284,6 +284,9 @@ static void nvmet_execute_get_log_page_ana(struct nvmet_req *req)

static void nvmet_execute_get_log_page(struct nvmet_req *req)
{
if (!nvmet_check_data_len(req, nvmet_get_log_page_len(req->cmd)))
return;

switch (req->cmd->get_log_page.lid) {
case NVME_LOG_ERROR:
return nvmet_execute_get_log_page_error(req);
Expand Down Expand Up @@ -594,6 +597,9 @@ static void nvmet_execute_identify_desclist(struct nvmet_req *req)

static void nvmet_execute_identify(struct nvmet_req *req)
{
if (!nvmet_check_data_len(req, NVME_IDENTIFY_DATA_SIZE))
return;

switch (req->cmd->identify.cns) {
case NVME_ID_CNS_NS:
return nvmet_execute_identify_ns(req);
Expand All @@ -620,6 +626,8 @@ static void nvmet_execute_identify(struct nvmet_req *req)
*/
static void nvmet_execute_abort(struct nvmet_req *req)
{
if (!nvmet_check_data_len(req, 0))
return;
nvmet_set_result(req, 1);
nvmet_req_complete(req, 0);
}
Expand Down Expand Up @@ -704,6 +712,9 @@ static void nvmet_execute_set_features(struct nvmet_req *req)
u32 cdw10 = le32_to_cpu(req->cmd->common.cdw10);
u16 status = 0;

if (!nvmet_check_data_len(req, 0))
return;

switch (cdw10 & 0xff) {
case NVME_FEAT_NUM_QUEUES:
nvmet_set_result(req,
Expand Down Expand Up @@ -767,6 +778,9 @@ static void nvmet_execute_get_features(struct nvmet_req *req)
u32 cdw10 = le32_to_cpu(req->cmd->common.cdw10);
u16 status = 0;

if (!nvmet_check_data_len(req, 0))
return;

switch (cdw10 & 0xff) {
/*
* These features are mandatory in the spec, but we don't
Expand Down Expand Up @@ -831,6 +845,9 @@ void nvmet_execute_async_event(struct nvmet_req *req)
{
struct nvmet_ctrl *ctrl = req->sq->ctrl;

if (!nvmet_check_data_len(req, 0))
return;

mutex_lock(&ctrl->lock);
if (ctrl->nr_async_event_cmds >= NVMET_ASYNC_EVENTS) {
mutex_unlock(&ctrl->lock);
Expand All @@ -847,6 +864,9 @@ void nvmet_execute_keep_alive(struct nvmet_req *req)
{
struct nvmet_ctrl *ctrl = req->sq->ctrl;

if (!nvmet_check_data_len(req, 0))
return;

pr_debug("ctrl %d update keep-alive timer for %d secs\n",
ctrl->cntlid, ctrl->kato);

Expand All @@ -866,31 +886,24 @@ u16 nvmet_parse_admin_cmd(struct nvmet_req *req)
switch (cmd->common.opcode) {
case nvme_admin_get_log_page:
req->execute = nvmet_execute_get_log_page;
req->data_len = nvmet_get_log_page_len(cmd);
return 0;
case nvme_admin_identify:
req->execute = nvmet_execute_identify;
req->data_len = NVME_IDENTIFY_DATA_SIZE;
return 0;
case nvme_admin_abort_cmd:
req->execute = nvmet_execute_abort;
req->data_len = 0;
return 0;
case nvme_admin_set_features:
req->execute = nvmet_execute_set_features;
req->data_len = 0;
return 0;
case nvme_admin_get_features:
req->execute = nvmet_execute_get_features;
req->data_len = 0;
return 0;
case nvme_admin_async_event:
req->execute = nvmet_execute_async_event;
req->data_len = 0;
return 0;
case nvme_admin_keep_alive:
req->execute = nvmet_execute_keep_alive;
req->data_len = 0;
return 0;
}

Expand Down
16 changes: 12 additions & 4 deletions drivers/nvme/target/core.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -930,13 +930,21 @@ void nvmet_req_uninit(struct nvmet_req *req)
}
EXPORT_SYMBOL_GPL(nvmet_req_uninit);

void nvmet_req_execute(struct nvmet_req *req)
bool nvmet_check_data_len(struct nvmet_req *req, size_t data_len)
{
if (unlikely(req->data_len != req->transfer_len)) {
if (unlikely(data_len != req->transfer_len)) {
req->error_loc = offsetof(struct nvme_common_command, dptr);
nvmet_req_complete(req, NVME_SC_SGL_INVALID_DATA | NVME_SC_DNR);
} else
req->execute(req);
return false;
}

return true;
}
EXPORT_SYMBOL_GPL(nvmet_check_data_len);

void nvmet_req_execute(struct nvmet_req *req)
{
req->execute(req);
}
EXPORT_SYMBOL_GPL(nvmet_req_execute);

Expand Down
18 changes: 12 additions & 6 deletions drivers/nvme/target/discovery.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,9 @@ static void nvmet_execute_disc_get_log_page(struct nvmet_req *req)
u16 status = 0;
void *buffer;

if (!nvmet_check_data_len(req, data_len))
return;

if (req->cmd->get_log_page.lid != NVME_LOG_DISC) {
req->error_loc =
offsetof(struct nvme_get_log_page_command, lid);
Expand Down Expand Up @@ -240,6 +243,9 @@ static void nvmet_execute_disc_identify(struct nvmet_req *req)
struct nvme_id_ctrl *id;
u16 status = 0;

if (!nvmet_check_data_len(req, NVME_IDENTIFY_DATA_SIZE))
return;

if (req->cmd->identify.cns != NVME_ID_CNS_CTRL) {
req->error_loc = offsetof(struct nvme_identify, cns);
status = NVME_SC_INVALID_OPCODE | NVME_SC_DNR;
Expand Down Expand Up @@ -286,6 +292,9 @@ static void nvmet_execute_disc_set_features(struct nvmet_req *req)
u32 cdw10 = le32_to_cpu(req->cmd->common.cdw10);
u16 stat;

if (!nvmet_check_data_len(req, 0))
return;

switch (cdw10 & 0xff) {
case NVME_FEAT_KATO:
stat = nvmet_set_feat_kato(req);
Expand All @@ -309,6 +318,9 @@ static void nvmet_execute_disc_get_features(struct nvmet_req *req)
u32 cdw10 = le32_to_cpu(req->cmd->common.cdw10);
u16 stat = 0;

if (!nvmet_check_data_len(req, 0))
return;

switch (cdw10 & 0xff) {
case NVME_FEAT_KATO:
nvmet_get_feat_kato(req);
Expand Down Expand Up @@ -341,26 +353,20 @@ u16 nvmet_parse_discovery_cmd(struct nvmet_req *req)
switch (cmd->common.opcode) {
case nvme_admin_set_features:
req->execute = nvmet_execute_disc_set_features;
req->data_len = 0;
return 0;
case nvme_admin_get_features:
req->execute = nvmet_execute_disc_get_features;
req->data_len = 0;
return 0;
case nvme_admin_async_event:
req->execute = nvmet_execute_async_event;
req->data_len = 0;
return 0;
case nvme_admin_keep_alive:
req->execute = nvmet_execute_keep_alive;
req->data_len = 0;
return 0;
case nvme_admin_get_log_page:
req->data_len = nvmet_get_log_page_len(cmd);
req->execute = nvmet_execute_disc_get_log_page;
return 0;
case nvme_admin_identify:
req->data_len = NVME_IDENTIFY_DATA_SIZE;
req->execute = nvmet_execute_disc_identify;
return 0;
default:
Expand Down
15 changes: 12 additions & 3 deletions drivers/nvme/target/fabrics-cmd.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ static void nvmet_execute_prop_set(struct nvmet_req *req)
u64 val = le64_to_cpu(req->cmd->prop_set.value);
u16 status = 0;

if (!nvmet_check_data_len(req, 0))
return;

if (req->cmd->prop_set.attrib & 1) {
req->error_loc =
offsetof(struct nvmf_property_set_command, attrib);
Expand All @@ -38,6 +41,9 @@ static void nvmet_execute_prop_get(struct nvmet_req *req)
u16 status = 0;
u64 val = 0;

if (!nvmet_check_data_len(req, 0))
return;

if (req->cmd->prop_get.attrib & 1) {
switch (le32_to_cpu(req->cmd->prop_get.offset)) {
case NVME_REG_CAP:
Expand Down Expand Up @@ -82,11 +88,9 @@ u16 nvmet_parse_fabrics_cmd(struct nvmet_req *req)

switch (cmd->fabrics.fctype) {
case nvme_fabrics_type_property_set:
req->data_len = 0;
req->execute = nvmet_execute_prop_set;
break;
case nvme_fabrics_type_property_get:
req->data_len = 0;
req->execute = nvmet_execute_prop_get;
break;
default:
Expand Down Expand Up @@ -147,6 +151,9 @@ static void nvmet_execute_admin_connect(struct nvmet_req *req)
struct nvmet_ctrl *ctrl = NULL;
u16 status = 0;

if (!nvmet_check_data_len(req, sizeof(struct nvmf_connect_data)))
return;

d = kmalloc(sizeof(*d), GFP_KERNEL);
if (!d) {
status = NVME_SC_INTERNAL;
Expand Down Expand Up @@ -211,6 +218,9 @@ static void nvmet_execute_io_connect(struct nvmet_req *req)
u16 qid = le16_to_cpu(c->qid);
u16 status = 0;

if (!nvmet_check_data_len(req, sizeof(struct nvmf_connect_data)))
return;

d = kmalloc(sizeof(*d), GFP_KERNEL);
if (!d) {
status = NVME_SC_INTERNAL;
Expand Down Expand Up @@ -281,7 +291,6 @@ u16 nvmet_parse_connect_cmd(struct nvmet_req *req)
return NVME_SC_INVALID_OPCODE | NVME_SC_DNR;
}

req->data_len = sizeof(struct nvmf_connect_data);
if (cmd->connect.qid == 0)
req->execute = nvmet_execute_admin_connect;
else
Expand Down
19 changes: 13 additions & 6 deletions drivers/nvme/target/io-cmd-bdev.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,9 @@ static void nvmet_bdev_execute_rw(struct nvmet_req *req)
sector_t sector;
int op, op_flags = 0, i;

if (!nvmet_check_data_len(req, nvmet_rw_len(req)))
return;

if (!req->sg_cnt) {
nvmet_req_complete(req, 0);
return;
Expand All @@ -170,7 +173,7 @@ static void nvmet_bdev_execute_rw(struct nvmet_req *req)
sector = le64_to_cpu(req->cmd->rw.slba);
sector <<= (req->ns->blksize_shift - 9);

if (req->data_len <= NVMET_MAX_INLINE_DATA_LEN) {
if (req->transfer_len <= NVMET_MAX_INLINE_DATA_LEN) {
bio = &req->b.inline_bio;
bio_init(bio, req->inline_bvec, ARRAY_SIZE(req->inline_bvec));
} else {
Expand Down Expand Up @@ -207,6 +210,9 @@ static void nvmet_bdev_execute_flush(struct nvmet_req *req)
{
struct bio *bio = &req->b.inline_bio;

if (!nvmet_check_data_len(req, 0))
return;

bio_init(bio, req->inline_bvec, ARRAY_SIZE(req->inline_bvec));
bio_set_dev(bio, req->ns->bdev);
bio->bi_private = req;
Expand Down Expand Up @@ -272,6 +278,9 @@ static void nvmet_bdev_execute_discard(struct nvmet_req *req)

static void nvmet_bdev_execute_dsm(struct nvmet_req *req)
{
if (!nvmet_check_data_len(req, nvmet_dsm_len(req)))
return;

switch (le32_to_cpu(req->cmd->dsm.attributes)) {
case NVME_DSMGMT_AD:
nvmet_bdev_execute_discard(req);
Expand All @@ -293,6 +302,9 @@ static void nvmet_bdev_execute_write_zeroes(struct nvmet_req *req)
sector_t nr_sector;
int ret;

if (!nvmet_check_data_len(req, 0))
return;

sector = le64_to_cpu(write_zeroes->slba) <<
(req->ns->blksize_shift - 9);
nr_sector = (((sector_t)le16_to_cpu(write_zeroes->length) + 1) <<
Expand All @@ -317,20 +329,15 @@ u16 nvmet_bdev_parse_io_cmd(struct nvmet_req *req)
case nvme_cmd_read:
case nvme_cmd_write:
req->execute = nvmet_bdev_execute_rw;
req->data_len = nvmet_rw_len(req);
return 0;
case nvme_cmd_flush:
req->execute = nvmet_bdev_execute_flush;
req->data_len = 0;
return 0;
case nvme_cmd_dsm:
req->execute = nvmet_bdev_execute_dsm;
req->data_len = (le32_to_cpu(cmd->dsm.nr) + 1) *
sizeof(struct nvme_dsm_range);
return 0;
case nvme_cmd_write_zeroes:
req->execute = nvmet_bdev_execute_write_zeroes;
req->data_len = 0;
return 0;
default:
pr_err("unhandled cmd %d on qid %d\n", cmd->common.opcode,
Expand Down
Loading

0 comments on commit e9061c3

Please sign in to comment.