Re: [PATCH v4 1/2] nvme: add per-queue sysfs directories

[Nilay Shroff <nilay@linux.ibm.com>]

> diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
> index bac25b287d25..20410f0d6118 100644
> --- a/drivers/nvme/host/nvme.h
> +++ b/drivers/nvme/host/nvme.h
> @@ -339,6 +339,28 @@ enum nvme_ctrl_flags {
>   	NVME_CTRL_FROZEN		= 6,
>   };
>   
> +enum nvme_queue_info_flags {
> +	NVME_QUEUE_INFO_REGISTERED	= 0,
> +	NVME_QUEUE_INFO_IO_CPU_USER	= 1,
> +};

NVME_QUEUE_INFO_IO_CPU_USER appears to be a transport-specific flag,
so I don't think it belongs in the common NVMe core code. It would
be better to keep this state in the transport-specific queue structure.

[...]

> +/*
> + * Per-queue sysfs directories under /sys/class/nvme/nvmeX/queues/. The
> + * transport backing the controller registers a directory for each of its
> + * queues and provides the attributes through the ktype. Registration is
> + * expected once per controller lifetime, so the directories persist while
> + * the transport's queues cycle across reconnects. The queues directory
> + * itself is released with the controller device.
> + */
> +int nvme_register_queue_info(struct nvme_ctrl *ctrl,
> +			     struct nvme_queue_info *qinfo, unsigned int qid,
> +			     const struct kobj_type *ktype)
> +{
> +	int ret;
> +
> +	if (!ctrl->queues_kobj) {
> +		ctrl->queues_kobj = kobject_create_and_add("queues",
> +							   &ctrl->device->kobj);
> +		if (!ctrl->queues_kobj)
> +			return -ENOMEM;
> +	}
> +
If this function can be invoked concurrently, how is ctrl->queues_kobj creation
serialized? For example, if two callers concurrently pass the !ctrl->queues_kobj
check, both could call kobject_create_and_add(), which is not what we want.

I understand that the current user of this API in patch 2/2 invokes it serially.
If serialization is an API requirement, could we document that in the function
comment? Alternatively, if this API is expected to be safe for concurrent callers,
the creation of queues_kobj should be serialized within this function.

In particular, I'd prefer the API contract to make the expected serialization
explicit rather than relying on the current caller to invoke it serially.

Thanks,
--Nilay