| Message ID | 20190801234514.7941-8-logang@deltatee.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | nvmet: add target passthru commands support | expand |
On 8/2/2019 2:45 AM, Logan Gunthorpe wrote: > This patch adds helper functions which are used in the NVMeOF configfs > when the user is configuring the passthru subsystem. Here we ensure > that only one subsys is assigned to each nvme_ctrl by using an xarray > on the cntlid. > > [chaitanya.kulkarni@wdc.com: this patch is very roughly based > on a similar one by Chaitanya] > Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com> > Signed-off-by: Logan Gunthorpe <logang@deltatee.com> > --- > drivers/nvme/target/core.c | 8 +++ > drivers/nvme/target/io-cmd-passthru.c | 77 +++++++++++++++++++++++++++ > drivers/nvme/target/nvmet.h | 10 ++++ > 3 files changed, 95 insertions(+) > > diff --git a/drivers/nvme/target/core.c b/drivers/nvme/target/core.c > index 50c01b2da568..2e75968af7f4 100644 > --- a/drivers/nvme/target/core.c > +++ b/drivers/nvme/target/core.c > @@ -519,6 +519,12 @@ int nvmet_ns_enable(struct nvmet_ns *ns) > > mutex_lock(&subsys->lock); > ret = 0; > + > + if (nvmet_passthru_ctrl(subsys)) { > + pr_info("cannot enable both passthru and regular namespaces for a single subsystem"); > + goto out_unlock; > + } > + > if (ns->enabled) > goto out_unlock; > > @@ -1439,6 +1445,8 @@ static void nvmet_subsys_free(struct kref *ref) > > WARN_ON_ONCE(!list_empty(&subsys->namespaces)); > > + nvmet_passthru_subsys_free(subsys); > + > kfree(subsys->subsysnqn); > kfree(subsys); > } > diff --git a/drivers/nvme/target/io-cmd-passthru.c b/drivers/nvme/target/io-cmd-passthru.c > index 46c58fec5608..b199785500ad 100644 > --- a/drivers/nvme/target/io-cmd-passthru.c > +++ b/drivers/nvme/target/io-cmd-passthru.c > @@ -11,6 +11,11 @@ > #include "../host/nvme.h" > #include "nvmet.h" > > +/* > + * xarray to maintain one passthru subsystem per nvme controller. > + */ > +static DEFINE_XARRAY(passthru_subsystems); > + > static struct workqueue_struct *passthru_wq; > > int nvmet_passthru_init(void) > @@ -27,6 +32,78 @@ void nvmet_passthru_destroy(void) > destroy_workqueue(passthru_wq); > } > > +int nvmet_passthru_ctrl_enable(struct nvmet_subsys *subsys) > +{ > + struct nvme_ctrl *ctrl; > + int ret = -EINVAL; > + void *old; > + > + mutex_lock(&subsys->lock); > + if (!subsys->passthru_ctrl_path) > + goto out_unlock; > + if (subsys->passthru_ctrl) > + goto out_unlock; > + > + if (subsys->nr_namespaces) { > + pr_info("cannot enable both passthru and regular namespaces for a single subsystem"); > + goto out_unlock; > + } > + > + ctrl = nvme_ctrl_get_by_path(subsys->passthru_ctrl_path); > + if (IS_ERR(ctrl)) { > + ret = PTR_ERR(ctrl); > + pr_err("failed to open nvme controller %s\n", > + subsys->passthru_ctrl_path); > + > + goto out_unlock; > + } > + > + old = xa_cmpxchg(&passthru_subsystems, ctrl->cntlid, NULL, > + subsys, GFP_KERNEL); > + if (xa_is_err(old)) { > + ret = xa_err(old); > + goto out_put_ctrl; > + } > + > + if (old) > + goto out_put_ctrl; > + > + subsys->passthru_ctrl = ctrl; > + ret = 0; > + > + goto out_unlock; can we re-arrange the code here ? it's not so common to see goto in a good flow. maybe have 1 good flow the goto's will go bellow it as we usually do in this subsystem. > + > +out_put_ctrl: > + nvme_put_ctrl(ctrl); > +out_unlock: > + mutex_unlock(&subsys->lock); > + return ret; > +} > + > +static void __nvmet_passthru_ctrl_disable(struct nvmet_subsys *subsys) > +{ > + if (subsys->passthru_ctrl) { > + xa_erase(&passthru_subsystems, subsys->passthru_ctrl->cntlid); > + nvme_put_ctrl(subsys->passthru_ctrl); > + } > + subsys->passthru_ctrl = NULL; > +} > + > +void nvmet_passthru_ctrl_disable(struct nvmet_subsys *subsys) > +{ > + mutex_lock(&subsys->lock); > + __nvmet_passthru_ctrl_disable(subsys); > + mutex_unlock(&subsys->lock); > +} > + > +void nvmet_passthru_subsys_free(struct nvmet_subsys *subsys) > +{ > + mutex_lock(&subsys->lock); > + __nvmet_passthru_ctrl_disable(subsys); > + kfree(subsys->passthru_ctrl_path); > + mutex_unlock(&subsys->lock); > +} > + > static void nvmet_passthru_req_complete(struct nvmet_req *req, > struct request *rq, u16 status) > { > diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h > index bd11114ebbb9..aff4db03269d 100644 > --- a/drivers/nvme/target/nvmet.h > +++ b/drivers/nvme/target/nvmet.h > @@ -230,6 +230,7 @@ struct nvmet_subsys { > > #ifdef CONFIG_NVME_TARGET_PASSTHRU > struct nvme_ctrl *passthru_ctrl; > + char *passthru_ctrl_path; > #endif /* CONFIG_NVME_TARGET_PASSTHRU */ > }; > > @@ -509,6 +510,9 @@ static inline u32 nvmet_rw_len(struct nvmet_req *req) > > int nvmet_passthru_init(void); > void nvmet_passthru_destroy(void); > +void nvmet_passthru_subsys_free(struct nvmet_subsys *subsys); > +int nvmet_passthru_ctrl_enable(struct nvmet_subsys *subsys); > +void nvmet_passthru_ctrl_disable(struct nvmet_subsys *subsys); > u16 nvmet_parse_passthru_cmd(struct nvmet_req *req); > > static inline > @@ -526,6 +530,12 @@ static inline int nvmet_passthru_init(void) > static inline void nvmet_passthru_destroy(void) > { > } > +static inline void nvmet_passthru_subsys_free(struct nvmet_subsys *subsys) > +{ > +} > +static inline void nvmet_passthru_ctrl_disable(struct nvmet_subsys *subsys) > +{ > +} > static inline u16 nvmet_parse_passthru_cmd(struct nvmet_req *req) > { > return 0;
On 2019-08-15 6:20 a.m., Max Gurtovoy wrote: > > On 8/2/2019 2:45 AM, Logan Gunthorpe wrote: >> This patch adds helper functions which are used in the NVMeOF configfs >> when the user is configuring the passthru subsystem. Here we ensure >> that only one subsys is assigned to each nvme_ctrl by using an xarray >> on the cntlid. >> >> [chaitanya.kulkarni@wdc.com: this patch is very roughly based >> on a similar one by Chaitanya] >> Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com> >> Signed-off-by: Logan Gunthorpe <logang@deltatee.com> >> --- >> drivers/nvme/target/core.c | 8 +++ >> drivers/nvme/target/io-cmd-passthru.c | 77 +++++++++++++++++++++++++++ >> drivers/nvme/target/nvmet.h | 10 ++++ >> 3 files changed, 95 insertions(+) >> >> diff --git a/drivers/nvme/target/core.c b/drivers/nvme/target/core.c >> index 50c01b2da568..2e75968af7f4 100644 >> --- a/drivers/nvme/target/core.c >> +++ b/drivers/nvme/target/core.c >> @@ -519,6 +519,12 @@ int nvmet_ns_enable(struct nvmet_ns *ns) >> mutex_lock(&subsys->lock); >> ret = 0; >> + >> + if (nvmet_passthru_ctrl(subsys)) { >> + pr_info("cannot enable both passthru and regular namespaces >> for a single subsystem"); >> + goto out_unlock; >> + } >> + >> if (ns->enabled) >> goto out_unlock; >> @@ -1439,6 +1445,8 @@ static void nvmet_subsys_free(struct kref *ref) >> WARN_ON_ONCE(!list_empty(&subsys->namespaces)); >> + nvmet_passthru_subsys_free(subsys); >> + >> kfree(subsys->subsysnqn); >> kfree(subsys); >> } >> diff --git a/drivers/nvme/target/io-cmd-passthru.c >> b/drivers/nvme/target/io-cmd-passthru.c >> index 46c58fec5608..b199785500ad 100644 >> --- a/drivers/nvme/target/io-cmd-passthru.c >> +++ b/drivers/nvme/target/io-cmd-passthru.c >> @@ -11,6 +11,11 @@ >> #include "../host/nvme.h" >> #include "nvmet.h" >> +/* >> + * xarray to maintain one passthru subsystem per nvme controller. >> + */ >> +static DEFINE_XARRAY(passthru_subsystems); >> + >> static struct workqueue_struct *passthru_wq; >> int nvmet_passthru_init(void) >> @@ -27,6 +32,78 @@ void nvmet_passthru_destroy(void) >> destroy_workqueue(passthru_wq); >> } >> +int nvmet_passthru_ctrl_enable(struct nvmet_subsys *subsys) >> +{ >> + struct nvme_ctrl *ctrl; >> + int ret = -EINVAL; >> + void *old; >> + >> + mutex_lock(&subsys->lock); >> + if (!subsys->passthru_ctrl_path) >> + goto out_unlock; >> + if (subsys->passthru_ctrl) >> + goto out_unlock; >> + >> + if (subsys->nr_namespaces) { >> + pr_info("cannot enable both passthru and regular namespaces >> for a single subsystem"); >> + goto out_unlock; >> + } >> + >> + ctrl = nvme_ctrl_get_by_path(subsys->passthru_ctrl_path); >> + if (IS_ERR(ctrl)) { >> + ret = PTR_ERR(ctrl); >> + pr_err("failed to open nvme controller %s\n", >> + subsys->passthru_ctrl_path); >> + >> + goto out_unlock; >> + } >> + >> + old = xa_cmpxchg(&passthru_subsystems, ctrl->cntlid, NULL, >> + subsys, GFP_KERNEL); >> + if (xa_is_err(old)) { >> + ret = xa_err(old); >> + goto out_put_ctrl; >> + } >> + >> + if (old) >> + goto out_put_ctrl; >> + >> + subsys->passthru_ctrl = ctrl; >> + ret = 0; >> + >> + goto out_unlock; > > can we re-arrange the code here ? > > it's not so common to see goto in a good flow. > > maybe have 1 good flow the goto's will go bellow it as we usually do in > this subsystem. OK, queued up for v8. Thanks, Logan
diff --git a/drivers/nvme/target/core.c b/drivers/nvme/target/core.c index 50c01b2da568..2e75968af7f4 100644 --- a/drivers/nvme/target/core.c +++ b/drivers/nvme/target/core.c @@ -519,6 +519,12 @@ int nvmet_ns_enable(struct nvmet_ns *ns) mutex_lock(&subsys->lock); ret = 0; + + if (nvmet_passthru_ctrl(subsys)) { + pr_info("cannot enable both passthru and regular namespaces for a single subsystem"); + goto out_unlock; + } + if (ns->enabled) goto out_unlock; @@ -1439,6 +1445,8 @@ static void nvmet_subsys_free(struct kref *ref) WARN_ON_ONCE(!list_empty(&subsys->namespaces)); + nvmet_passthru_subsys_free(subsys); + kfree(subsys->subsysnqn); kfree(subsys); } diff --git a/drivers/nvme/target/io-cmd-passthru.c b/drivers/nvme/target/io-cmd-passthru.c index 46c58fec5608..b199785500ad 100644 --- a/drivers/nvme/target/io-cmd-passthru.c +++ b/drivers/nvme/target/io-cmd-passthru.c @@ -11,6 +11,11 @@ #include "../host/nvme.h" #include "nvmet.h" +/* + * xarray to maintain one passthru subsystem per nvme controller. + */ +static DEFINE_XARRAY(passthru_subsystems); + static struct workqueue_struct *passthru_wq; int nvmet_passthru_init(void) @@ -27,6 +32,78 @@ void nvmet_passthru_destroy(void) destroy_workqueue(passthru_wq); } +int nvmet_passthru_ctrl_enable(struct nvmet_subsys *subsys) +{ + struct nvme_ctrl *ctrl; + int ret = -EINVAL; + void *old; + + mutex_lock(&subsys->lock); + if (!subsys->passthru_ctrl_path) + goto out_unlock; + if (subsys->passthru_ctrl) + goto out_unlock; + + if (subsys->nr_namespaces) { + pr_info("cannot enable both passthru and regular namespaces for a single subsystem"); + goto out_unlock; + } + + ctrl = nvme_ctrl_get_by_path(subsys->passthru_ctrl_path); + if (IS_ERR(ctrl)) { + ret = PTR_ERR(ctrl); + pr_err("failed to open nvme controller %s\n", + subsys->passthru_ctrl_path); + + goto out_unlock; + } + + old = xa_cmpxchg(&passthru_subsystems, ctrl->cntlid, NULL, + subsys, GFP_KERNEL); + if (xa_is_err(old)) { + ret = xa_err(old); + goto out_put_ctrl; + } + + if (old) + goto out_put_ctrl; + + subsys->passthru_ctrl = ctrl; + ret = 0; + + goto out_unlock; + +out_put_ctrl: + nvme_put_ctrl(ctrl); +out_unlock: + mutex_unlock(&subsys->lock); + return ret; +} + +static void __nvmet_passthru_ctrl_disable(struct nvmet_subsys *subsys) +{ + if (subsys->passthru_ctrl) { + xa_erase(&passthru_subsystems, subsys->passthru_ctrl->cntlid); + nvme_put_ctrl(subsys->passthru_ctrl); + } + subsys->passthru_ctrl = NULL; +} + +void nvmet_passthru_ctrl_disable(struct nvmet_subsys *subsys) +{ + mutex_lock(&subsys->lock); + __nvmet_passthru_ctrl_disable(subsys); + mutex_unlock(&subsys->lock); +} + +void nvmet_passthru_subsys_free(struct nvmet_subsys *subsys) +{ + mutex_lock(&subsys->lock); + __nvmet_passthru_ctrl_disable(subsys); + kfree(subsys->passthru_ctrl_path); + mutex_unlock(&subsys->lock); +} + static void nvmet_passthru_req_complete(struct nvmet_req *req, struct request *rq, u16 status) { diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h index bd11114ebbb9..aff4db03269d 100644 --- a/drivers/nvme/target/nvmet.h +++ b/drivers/nvme/target/nvmet.h @@ -230,6 +230,7 @@ struct nvmet_subsys { #ifdef CONFIG_NVME_TARGET_PASSTHRU struct nvme_ctrl *passthru_ctrl; + char *passthru_ctrl_path; #endif /* CONFIG_NVME_TARGET_PASSTHRU */ }; @@ -509,6 +510,9 @@ static inline u32 nvmet_rw_len(struct nvmet_req *req) int nvmet_passthru_init(void); void nvmet_passthru_destroy(void); +void nvmet_passthru_subsys_free(struct nvmet_subsys *subsys); +int nvmet_passthru_ctrl_enable(struct nvmet_subsys *subsys); +void nvmet_passthru_ctrl_disable(struct nvmet_subsys *subsys); u16 nvmet_parse_passthru_cmd(struct nvmet_req *req); static inline @@ -526,6 +530,12 @@ static inline int nvmet_passthru_init(void) static inline void nvmet_passthru_destroy(void) { } +static inline void nvmet_passthru_subsys_free(struct nvmet_subsys *subsys) +{ +} +static inline void nvmet_passthru_ctrl_disable(struct nvmet_subsys *subsys) +{ +} static inline u16 nvmet_parse_passthru_cmd(struct nvmet_req *req) { return 0;