On 4/21/2020 6:30 PM, Christoph Hellwig wrote:
+ /*
+ * Max command capsule size is sqe + single page of in-capsule data.
+ * Disable inline data for Metadata capable controllers.
+ */
id->ioccsz = cpu_to_le32((sizeof(struct nvme_command) +
- req->port->inline_data_size) / 16);
+ req->port->inline_data_size *
+ !ctrl->pi_support) / 16);
Can we de-obsfucated this a little?
cmd_capsule_size = sizeof(struct nvme_command);
if (!ctrl->pi_support)
cmd_capsule_size += req->port->inline_data_size;
id->ioccsz = cpu_to_le32(cmd_capsule_size / 16);
Yes good idea.
+ if (ctrl->subsys->pi_support && ctrl->port->pi_enable) {
+ if (ctrl->port->pi_capable) {
+ ctrl->pi_support = true;
+ pr_info("controller %d T10-PI enabled\n", ctrl->cntlid);
+ } else {
+ ctrl->pi_support = false;
+ pr_warn("T10-PI is not supported on controller %d\n",
+ ctrl->cntlid);
+ }
I think the printks are a little verbose. Also why can we set
ctrl->port->pi_enable if ctrl->port->pi_capable is false? Shoudn't
we reject that earlier? In that case this could simply become:
ctrl->pi_support = ctrl->subsys->pi_support && ctrl->port->pi_enable;
for that we'll need to check pi_capable during add_port process and
disable pi_enable bit if user set it.
User should set it before enable the port (this will always succeed).
I'll make this change as well.
re the verbosity, sometimes I get many requests from users to get
indication for some features.
We can remove this as well if needed.
+#ifdef CONFIG_BLK_DEV_INTEGRITY
+static inline u32 nvmet_rw_md_len(struct nvmet_req *req)
+{
+ return ((u32)le16_to_cpu(req->cmd->rw.length) + 1) * req->ns->ms;
+}
+
+static inline bool nvmet_ns_has_pi(struct nvmet_ns *ns)
+{
+ return ns->md_type && ns->ms == sizeof(struct t10_pi_tuple);
+}
+#else
+static inline u32 nvmet_rw_md_len(struct nvmet_req *req)
+{
+ return 0;
Do we really need a stub for nvmet_rw_md_len? Also for nvmet_ns_has_pi
we could probably reword it as:
static inline bool nvmet_ns_has_pi(struct nvmet_ns *ns)
{
if (!IS_ENABLED(CONFIG_BLK_DEV_INTEGRITY))
return false;
return ns->pi_type && ns->metadata_size == sizeof(struct t10_pi_tuple);
}
and avoid the need for a stub as well.
yup.
