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

[Philippe Mathieu-Daudé]

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'?

Phil.