Re: [PATCH for-6.0 v2 6/8] hw/block/nvme: update dmsrl limit on namespace detachment

[Klaus Jensen <its@irrelevant.dk>]

[-- Attachment #1: Type: text/plain, Size: 3882 bytes --]

On Apr  9 19:39, Thomas Huth wrote:
>On 06/04/2021 09.24, Klaus Jensen wrote:
>>On Apr  6 09:10, Philippe Mathieu-Daudé wrote:
>>>On 4/5/21 7:54 PM, Klaus Jensen wrote:
>>>>From: Klaus Jensen <k.jensen@samsung.com>
>>>>
>>>>The Non-MDTS DMSRL limit must be recomputed when namespaces are
>>>>detached.
>>>>
>>>>Fixes: 645ce1a70cb6 ("hw/block/nvme: support namespace attachment command")
>>>>Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
>>>>Reviewed-by: Gollu Appalanaidu <anaidu.gollu@samsung.com>
>>>>---
>>>>  hw/block/nvme.c | 17 +++++++++++++++++
>>>>  1 file changed, 17 insertions(+)
>>>>
>>>>diff --git a/hw/block/nvme.c b/hw/block/nvme.c
>>>>index de0e726dfdd8..3dc51f407671 100644
>>>>--- a/hw/block/nvme.c
>>>>+++ b/hw/block/nvme.c
>>>>@@ -4876,6 +4876,21 @@ static uint16_t nvme_aer(NvmeCtrl *n, NvmeRequest *req)
>>>>      return NVME_NO_COMPLETE;
>>>>  }
>>>>+static void __nvme_update_dmrsl(NvmeCtrl *n)
>>>>+{
>>>>+    int nsid;
>>>>+
>>>>+    for (nsid = 1; nsid <= NVME_MAX_NAMESPACES; nsid++) {
>>>>+        NvmeNamespace *ns = nvme_ns(n, nsid);
>>>>+        if (!ns) {
>>>>+            continue;
>>>>+        }
>>>>+
>>>>+        n->dmrsl = MIN_NON_ZERO(n->dmrsl,
>>>>+                                BDRV_REQUEST_MAX_BYTES / nvme_l2b(ns, 1));
>>>>+    }
>>>>+}
>>>>+
>>>>  static void __nvme_select_ns_iocs(NvmeCtrl *n, NvmeNamespace *ns);
>>>>  static uint16_t nvme_ns_attachment(NvmeCtrl *n, NvmeRequest *req)
>>>>  {
>>>>@@ -4925,6 +4940,8 @@ static uint16_t nvme_ns_attachment(NvmeCtrl *n, NvmeRequest *req)
>>>>              }
>>>>              nvme_ns_detach(ctrl, ns);
>>>>+
>>>>+            __nvme_update_dmrsl(ctrl);
>>>>          }
>>>
>>>Why the '__' prefix? It doesn't seem clearer (I'm not sure there is
>>>a convention, it makes me think of a internal macro expansion use
>>>for preprocessor).
>>>
>>>There are very few uses of this prefix:
>>>
>>>hw/9pfs/cofs.c:21:static ssize_t __readlink(V9fsState *s, V9fsPath
>>>*path, V9fsString *buf)
>>>hw/block/nvme.c:1683:static uint16_t __nvme_zrm_open(NvmeNamespace *ns,
>>>NvmeZone *zone,
>>>hw/block/nvme.c:1742:static void __nvme_advance_zone_wp(NvmeNamespace
>>>*ns, NvmeZone *zone,
>>>hw/block/nvme.c:5213:static void __nvme_select_ns_iocs(NvmeCtrl *n,
>>>NvmeNamespace *ns)
>>>hw/i386/amd_iommu.c:1160:static int __amdvi_int_remap_msi(AMDVIState *iommu,
>>>hw/intc/s390_flic_kvm.c:255:static int __get_all_irqs(KVMS390FLICState
>>>*flic,
>>>hw/net/rocker/rocker_desc.c:199:static bool
>>>__desc_ring_post_desc(DescRing *ring, int err)
>>>hw/net/sungem.c:766:static uint16_t __sungem_mii_read(SunGEMState *s,
>>>uint8_t phy_addr,
>>>hw/ppc/ppc.c:867:static void __cpu_ppc_store_decr(PowerPCCPU *cpu,
>>>uint64_t *nextp,
>>>hw/s390x/pv.c:25:static int __s390_pv_cmd(uint32_t cmd, const char
>>>*cmdname, void *data)
>>>pc-bios/s390-ccw/cio.c:315:static int __do_cio(SubChannelId schid,
>>>uint32_t ccw_addr, int fmt, Irb *irb)
>>>target/ppc/mmu-hash64.c:170:static void __helper_slbie(CPUPPCState *env,
>>>target_ulong addr,
>>>
>>>Thomas, Eric, is it worth cleaning these and updating the
>>>'CODESTYLE.rst'?
>>>
>>
>>Yeah ok, I think you are right that there is no clear convention on when
>>to use this or not. I typically just use it for functions that are
>>normally not supposed to be called directly.
>>
>>But I don't even think its consistent in the nvme device. For my sake,
>>we can clean it up, I'll drop it in this case since there is no good
>>reason for it other than my own idea of "style".
>
>IIRC all identifiers that start with two underscores are reserved by 
>the C standard:
>
> https://busybox.net/~landley/c99-draft.html#7.1.3
>
>Thus you should not use two underscores at the beginning here at all.
>

I'll clean up the remaining double underscores in the next cycle!

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]