Thread (20 messages) 20 messages, 6 authors, 2020-05-26

Re: [RESEND PATCH v7 4/5] ndctl/papr_scm, uapi: Add support for PAPR nvdimm specific methods

From: Michael Ellerman <mpe@ellerman.id.au>
Date: 2020-05-26 12:14:20
Also in: lkml, nvdimm

Vaibhav Jain [off-list ref] writes:
Hi Ira, Mpe and Aneesh,

Vaibhav Jain [off-list ref] writes:
quoted
Michael Ellerman [off-list ref] writes:
quoted
Ira Weiny [off-list ref] writes:
quoted
On Wed, May 20, 2020 at 12:30:57AM +0530, Vaibhav Jain wrote:
quoted
Introduce support for Papr nvDimm Specific Methods (PDSM) in papr_scm
modules and add the command family to the white list of NVDIMM command
sets. Also advertise support for ND_CMD_CALL for the dimm
command mask and implement necessary scaffolding in the module to
handle ND_CMD_CALL ioctl and PDSM requests that we receive.
...
quoted
quoted
+ *
+ * Payload Version:
+ *
+ * A 'payload_version' field is present in PDSM header that indicates a specific
+ * version of the structure present in PDSM Payload for a given PDSM command.
+ * This provides backward compatibility in case the PDSM Payload structure
+ * evolves and different structures are supported by 'papr_scm' and 'libndctl'.
+ *
+ * When sending a PDSM Payload to 'papr_scm', 'libndctl' should send the version
+ * of the payload struct it supports via 'payload_version' field. The 'papr_scm'
+ * module when servicing the PDSM envelope checks the 'payload_version' and then
+ * uses 'payload struct version' == MIN('payload_version field',
+ * 'max payload-struct-version supported by papr_scm') to service the PDSM.
+ * After servicing the PDSM, 'papr_scm' put the negotiated version of payload
+ * struct in returned 'payload_version' field.
FWIW many people believe using a size rather than version is more sustainable.
It is expected that new payload structures are larger (more features) than the
previous payload structure.

I can't find references at the moment through.
I think clone_args is a good modern example:

  https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/uapi/linux/sched.h#n88
Thank Ira and Mpe for pointing this out. I looked into how clone3 sycall
handles clone_args and few differences came out:

* Unlike clone_args that are always transferred in one direction from
  user-space to kernel, payload contents of pdsms are transferred in both
  directions. Having a single version number makes it easier for
  user-space and kernel to determine what data will be exchanged.

* For PDSMs, the version number is negotiated between libndctl and
  kernel. For example in case kernel only supports an older version of
  a structure, its free to send a lower version number back to
  libndctl. Such negotiations doesnt happen with clone3 syscall.
If you are ok with the explaination above please let me know. I will
quickly spin off a v8 addressing your review comments.
I don't have strong opinions about the user API, it's really up to the
nvdimm folks.

cheers
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help