Re: [PATCH 3/6] nvme: register ns_id attributes as default sysfs groups

2018-08-14 Thread Hannes Reinecke 
On 08/13/2018 09:51 PM, Bart Van Assche wrote:
> On Mon, 2018-07-30 at 09:12 +0200, Hannes Reinecke wrote:
>> @@ -3061,11 +3066,7 @@ static void nvme_alloc_ns(struct nvme_ctrl *ctrl, 
>> unsigned nsid)
>>  
>>  nvme_get_ctrl(ctrl);
>>  
>> -device_add_disk(ctrl->device, ns->disk, NULL);
>> -if (sysfs_create_group(_to_dev(ns->disk)->kobj,
>> -_ns_id_attr_group))
>> -pr_warn("%s: failed to create sysfs group for identification\n",
>> -ns->disk->disk_name);
>> +device_add_disk(ctrl->device, ns->disk, nvme_ns_id_attr_groups);
>>  if (ns->ndev && nvme_nvm_register_sysfs(ns))
>>  pr_warn("%s: failed to register lightnvm sysfs group for 
>> identification\n",
>>  ns->disk->disk_name);
> 
> Hello Hannes,
> 
> Have you noticed that nvme_nvm_register_sysfs() also registers sysfs 
> attributes
> and hence that the attributes registered by that function should be merged 
> into
> the nvme_ns_id_attr_groups array?
> 
Actually, no :-)
I'll be looking into it and reposting the series.

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

Re: [PATCH 3/6] nvme: register ns_id attributes as default sysfs groups

2018-08-13 Thread Bart Van Assche 
On Mon, 2018-07-30 at 09:12 +0200, Hannes Reinecke wrote:
> @@ -3061,11 +3066,7 @@ static void nvme_alloc_ns(struct nvme_ctrl *ctrl, 
> unsigned nsid)
>  
>   nvme_get_ctrl(ctrl);
>  
> - device_add_disk(ctrl->device, ns->disk, NULL);
> - if (sysfs_create_group(_to_dev(ns->disk)->kobj,
> - _ns_id_attr_group))
> - pr_warn("%s: failed to create sysfs group for identification\n",
> - ns->disk->disk_name);
> + device_add_disk(ctrl->device, ns->disk, nvme_ns_id_attr_groups);
>   if (ns->ndev && nvme_nvm_register_sysfs(ns))
>   pr_warn("%s: failed to register lightnvm sysfs group for 
> identification\n",
>   ns->disk->disk_name);

Hello Hannes,

Have you noticed that nvme_nvm_register_sysfs() also registers sysfs attributes
and hence that the attributes registered by that function should be merged into
the nvme_ns_id_attr_groups array?

Thanks,

Bart.

Re: [PATCH 3/6] nvme: register ns_id attributes as default sysfs groups

2018-07-30 Thread Christoph Hellwig 
On Mon, Jul 30, 2018 at 09:12:24AM +0200, Hannes Reinecke wrote:
> We should be registering the ns_id attribute as default sysfs
> attribute groups, otherwise we have a race condition between
> the uevent and the attributes appearing in sysfs.
> 
> Signed-off-by: Hannes Reinecke 
> Cc: Sagi Grimberg 
> Cc: Keith Busch 

Looks good,

Reviewed-by: Christoph Hellwig