Issue with AWUPF when using multiple controllers in a subsystem

[alan.adamson at oracle.com]

While testing atomic write parameters with multiple controllers within a 
subsystem, I found an issue:

QEMU-NVME Configuartion
=================
-device nvme-subsys,id=subsys0 \
-device 
nvme,serial=deadbeef,id=nvme0,subsys=subsys0,atomic.dn=off,atomic.awun=31,atomic.awupf=15 
\
-device 
nvme,serial=deadbeef,id=nvme1,subsys=subsys0,atomic.dn=off,atomic.awun=127,atomic.awupf=63 
\
-drive id=ns1,file=/dev/nullb3,if=none \
-drive id=ns2,file=/dev/nullb2,if=none \
-device nvme-ns,drive=ns1,bus=nvme0,nsid=1,shared=false \
-device nvme-ns,drive=ns2,bus=nvme1,nsid=2,shared=false \

[root at localhost ~]# nvme id-ctrl /dev/nvme1n2 | grep awupf
awupf     : 63
[root at localhost ~]# cat /sys/block/nvme1n2/queue/atomic_write_max_bytes
32768
[root at localhost ~]# nvme id-ctrl /dev/nvme1n1 | grep awupf
awupf     : 15
[root at localhost ~]# cat /sys/block/nvme1n1/queue/atomic_write_max_bytes
32768
[root at localhost ~]#

When dumping the awupf value with nvme-cli, the values match what was 
setup in qemu, but when each devices atomic queue limits was displayed, 
it didn't match its awupf. Currently the awupf is saved in the 
nvme_subsystem, but the awupf is specific to the controller, not the 
subsystem. Two controllers in a subsystem can have different atomic 
parameters.

The commit 81adb8633491 ("nvme: set physical block size and optimal I/O 
size") added subsys->awupf.

I think the awupf belongs in the nvme_ctrl structure.

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index a8259e3334ff..2483651817fb 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -2059,7 +2059,7 @@ static bool nvme_update_disk_info(struct nvme_ns 
*ns, struct nvme_id_ns *id,
                 if (id->nsfeat & NVME_NS_FEAT_ATOMICS && id->nawupf)
                         atomic_bs = (1 + le16_to_cpu(id->nawupf)) * bs;
                 else
-                       atomic_bs = (1 + ns->ctrl->subsys->awupf) * bs;
+                       atomic_bs = (1 + ns->ctrl->awupf) * bs;

                 nvme_update_atomic_write_disk_info(ns, id, lim, bs, 
atomic_bs);
         }
@@ -3032,7 +3032,6 @@ static int nvme_init_subsystem(struct nvme_ctrl 
*ctrl, struct nvme_id_ctrl *id)
                 kfree(subsys);
                 return -EINVAL;
         }
-       subsys->awupf = le16_to_cpu(id->awupf);
         nvme_mpath_default_iopolicy(subsys);

         subsys->dev.class = &nvme_subsys_class;
@@ -3443,6 +3442,7 @@ static int nvme_init_identify(struct nvme_ctrl *ctrl)
         else if (!ctrl->apst_enabled && prev_apst_enabled)
dev_pm_qos_hide_latency_tolerance(ctrl->device);

+       ctrl->awupf = le16_to_cpu(id->awupf);
  out_free:
         kfree(id);
         return ret;
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index 7be92d07430e..fddcd2de218c 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -410,6 +410,7 @@ struct nvme_ctrl {

         enum nvme_ctrl_type cntrltype;
         enum nvme_dctype dctype;
+       u16 awupf;      /* 0's based awupf value. */
  };

  static inline enum nvme_ctrl_state nvme_ctrl_state(struct nvme_ctrl *ctrl)
@@ -442,7 +443,6 @@ struct nvme_subsystem {
         u8                      cmic;
         enum nvme_subsys_type   subtype;
         u16                     vendor_id;
-       u16                     awupf;  /* 0's based awupf value. */
         struct ida              ns_ida;
  #ifdef CONFIG_NVME_MULTIPATH
         enum nvme_iopolicy      iopolicy;


When I apply this fix, the problem is resolved.

[root at localhost ~]# nvme id-ctrl /dev/nvme1n2 | grep awupf
awupf     : 63
[root at localhost ~]# cat /sys/block/nvme1n2/queue/atomic_write_max_bytes
32768
[root at localhost ~]# nvme id-ctrl /dev/nvme1n1 | grep awupf
awupf     : 15
[root at localhost ~]#  cat /sys/block/nvme1n1/queue/atomic_write_max_bytes
8192
[root at localhost ~]#


Does this fix make sense?

Alan Adamson