| Message ID | 20200122155304.120733-1-vaibhav@linux.ibm.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | libnvdimm/bus: return the outvar 'cmd_rc' error code in __nd_ioctl() | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | Successfully applied on branch powerpc/merge (20862247a368dbb75d6e97d82345999adaacf3cc) |
| snowpatch_ozlabs/build-ppc64le | success | Build succeeded |
| snowpatch_ozlabs/build-ppc64be | success | Build succeeded |
| snowpatch_ozlabs/build-ppc64e | success | Build succeeded |
| snowpatch_ozlabs/build-pmac32 | success | Build succeeded |
| snowpatch_ozlabs/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 11 lines checked |
| snowpatch_ozlabs/needsstable | success | Patch has no Fixes tags |
Vaibhav Jain <vaibhav@linux.ibm.com> writes: > Presently the error code returned via out variable 'cmd_rc' from the > nvdimm-bus controller function is ignored when called from > __nd_ioctl() and never communicated back to user-space code that called > an ioctl on dimm/bus. > > This minor patch updates __nd_ioctl() to propagate the value of out > variable 'cmd_rc' back to user-space in case it reports an error. > > Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com> > --- > drivers/nvdimm/bus.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/drivers/nvdimm/bus.c b/drivers/nvdimm/bus.c > index a8b515968569..5b687a27fdf2 100644 > --- a/drivers/nvdimm/bus.c > +++ b/drivers/nvdimm/bus.c > @@ -1153,6 +1153,11 @@ static int __nd_ioctl(struct nvdimm_bus *nvdimm_bus, struct nvdimm *nvdimm, > if (rc < 0) > goto out_unlock; > > + if (cmd_rc < 0) { > + rc = cmd_rc; > + goto out_unlock; > + } > + > if (!nvdimm && cmd == ND_CMD_CLEAR_ERROR && cmd_rc >= 0) { > struct nd_cmd_clear_error *clear_err = buf; Looks good to me. Reviewed-by: Jeff Moyer <jmoyer@redhat.com>
On Tue, Feb 18, 2020 at 1:00 PM Jeff Moyer <jmoyer@redhat.com> wrote: > > Vaibhav Jain <vaibhav@linux.ibm.com> writes: > > > Presently the error code returned via out variable 'cmd_rc' from the > > nvdimm-bus controller function is ignored when called from > > __nd_ioctl() and never communicated back to user-space code that called > > an ioctl on dimm/bus. > > > > This minor patch updates __nd_ioctl() to propagate the value of out > > variable 'cmd_rc' back to user-space in case it reports an error. > > > > Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com> > > --- > > drivers/nvdimm/bus.c | 5 +++++ > > 1 file changed, 5 insertions(+) > > > > diff --git a/drivers/nvdimm/bus.c b/drivers/nvdimm/bus.c > > index a8b515968569..5b687a27fdf2 100644 > > --- a/drivers/nvdimm/bus.c > > +++ b/drivers/nvdimm/bus.c > > @@ -1153,6 +1153,11 @@ static int __nd_ioctl(struct nvdimm_bus *nvdimm_bus, struct nvdimm *nvdimm, > > if (rc < 0) > > goto out_unlock; > > > > + if (cmd_rc < 0) { > > + rc = cmd_rc; > > + goto out_unlock; > > + } > > + > > if (!nvdimm && cmd == ND_CMD_CLEAR_ERROR && cmd_rc >= 0) { > > struct nd_cmd_clear_error *clear_err = buf; > > Looks good to me. > > Reviewed-by: Jeff Moyer <jmoyer@redhat.com> Applied.
On Tue, Feb 18, 2020 at 1:03 PM Dan Williams <dan.j.williams@intel.com> wrote: > > On Tue, Feb 18, 2020 at 1:00 PM Jeff Moyer <jmoyer@redhat.com> wrote: > > > > Vaibhav Jain <vaibhav@linux.ibm.com> writes: > > > > > Presently the error code returned via out variable 'cmd_rc' from the > > > nvdimm-bus controller function is ignored when called from > > > __nd_ioctl() and never communicated back to user-space code that called > > > an ioctl on dimm/bus. > > > > > > This minor patch updates __nd_ioctl() to propagate the value of out > > > variable 'cmd_rc' back to user-space in case it reports an error. > > > > > > Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com> > > > --- > > > drivers/nvdimm/bus.c | 5 +++++ > > > 1 file changed, 5 insertions(+) > > > > > > diff --git a/drivers/nvdimm/bus.c b/drivers/nvdimm/bus.c > > > index a8b515968569..5b687a27fdf2 100644 > > > --- a/drivers/nvdimm/bus.c > > > +++ b/drivers/nvdimm/bus.c > > > @@ -1153,6 +1153,11 @@ static int __nd_ioctl(struct nvdimm_bus *nvdimm_bus, struct nvdimm *nvdimm, > > > if (rc < 0) > > > goto out_unlock; > > > > > > + if (cmd_rc < 0) { > > > + rc = cmd_rc; > > > + goto out_unlock; > > > + } > > > + > > > if (!nvdimm && cmd == ND_CMD_CLEAR_ERROR && cmd_rc >= 0) { > > > struct nd_cmd_clear_error *clear_err = buf; > > > > Looks good to me. > > > > Reviewed-by: Jeff Moyer <jmoyer@redhat.com> > > Applied. Unapplied. This breaks the NVDIMM unit test, and now that I look closer you are likely overlooking the fact that cmd_rc is a translation of the firmware status, while the ioctl rc is whether the command was successfully submitted. If you want the equivalent of cmd_rc in userspace you need to translate the firmware status. See ndctl_cmd_submit_xlat() in libndctl as an example of how the equivalent of cmd_rc is generated from the firmware status.
diff --git a/drivers/nvdimm/bus.c b/drivers/nvdimm/bus.c index a8b515968569..5b687a27fdf2 100644 --- a/drivers/nvdimm/bus.c +++ b/drivers/nvdimm/bus.c @@ -1153,6 +1153,11 @@ static int __nd_ioctl(struct nvdimm_bus *nvdimm_bus, struct nvdimm *nvdimm, if (rc < 0) goto out_unlock; + if (cmd_rc < 0) { + rc = cmd_rc; + goto out_unlock; + } + if (!nvdimm && cmd == ND_CMD_CLEAR_ERROR && cmd_rc >= 0) { struct nd_cmd_clear_error *clear_err = buf;
Presently the error code returned via out variable 'cmd_rc' from the nvdimm-bus controller function is ignored when called from __nd_ioctl() and never communicated back to user-space code that called an ioctl on dimm/bus. This minor patch updates __nd_ioctl() to propagate the value of out variable 'cmd_rc' back to user-space in case it reports an error. Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com> --- drivers/nvdimm/bus.c | 5 +++++ 1 file changed, 5 insertions(+)