9:30 a.m.
On Mon, Apr 24, 2017 at 03:56:50PM -0700, Dan Williams wrote:
On Mon, Apr 24, 2017 at 3:36 PM, Jerry Hoemann
<jerry.hoemann(a)hpe.com> wrote:
> nd_cmd_out_size is called by __nd_ioctl to size the buffer passed to
> acpi_nfit_ctl. If the DSM function being called has a variable
> sized output, nd_cmd_out_size will look at the return field to
> determine buffer size. However, the DSM call hasn't been made yet,
> so output size is core residue.
>
> Have nd_cmd_out_size be bimodal with version "early" to be called
> before the DSM call is made. For variable sized output fields
> have it return ND_IOCTL_MAX_BUFLEN. __nd_ioctl sees new return
> values and adjust buffer size accordingly.
>
> The downside to this approach are:
> 1) Requires user buffer input size to be ND_IOCTL_MAX_BUFLEN (4 MB) for
> calls to DSM with variable return.
> 2) The needless copyin of 4MB
>
> An alternative approach (not yet prototyped) would move the call
> to nd_cmd_out_size until after the return from acpi_nfit_ctl. Here,
> the call has been made and return size would be known.
>
> The size of the buffer allocated in would always be ND_IOCTL_MAX_BUFLEN.
>
> Signed-off-by: Jerry Hoemann <jerry.hoemann(a)hpe.com>
> ---
> drivers/nvdimm/bus.c | 30 ++++++++++++++++++++++++------
> 1 file changed, 24 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/nvdimm/bus.c b/drivers/nvdimm/bus.c
> index 23d4a17..50f8cc6 100644
> --- a/drivers/nvdimm/bus.c
> +++ b/drivers/nvdimm/bus.c
> @@ -713,9 +713,9 @@ u32 nd_cmd_in_size(struct nvdimm *nvdimm, int cmd,
> }
> EXPORT_SYMBOL_GPL(nd_cmd_in_size);
>
> -u32 nd_cmd_out_size(struct nvdimm *nvdimm, int cmd,
> +u32 nd_cmd_out_size_early(struct nvdimm *nvdimm, int cmd,
> const struct nd_cmd_desc *desc, int idx, const u32 *in_field,
> - const u32 *out_field, unsigned long remainder)
> + const u32 *out_field, unsigned long remainder, int early)
> {
> if (idx >= desc->out_num)
> return UINT_MAX;
> @@ -725,9 +725,13 @@ u32 nd_cmd_out_size(struct nvdimm *nvdimm, int cmd,
>
> if (nvdimm && cmd == ND_CMD_GET_CONFIG_DATA && idx == 1)
> return in_field[1];
> - else if (nvdimm && cmd == ND_CMD_VENDOR && idx == 2)
> + else if (nvdimm && cmd == ND_CMD_VENDOR && idx == 2) {
> + if (early)
> + return ND_IOCTL_MAX_BUFLEN;
> return out_field[1];
Are you using ndctl_dimm_cmd_new_vendor_specific() to submit a vendor
specific command? It already pre-populates out_field[1] with the
initial output size.
I was actually concerned w/ calling the DSM for nd_cmd_ars_status and changed
ND_CMD_VENDOR on spec as it looked like it had the same pattern.
Hit this using a stand alone test tool I have.
For nd_cmd_ars_status, does the caller want to set the output field
"num_records" before making the IOCTL call? I.e. setting the maximum number
of records the caller is prepared to accept?
I took a quick look at the ndctl source and its not obvious that
is what the tool is doing. But I might have missed it.
--
-----------------------------------------------------------------------------
Jerry Hoemann Software Engineer Hewlett Packard Enterprise
-----------------------------------------------------------------------------