Re: [PATCH] nvme: initialize identify ns data to NULL

[Tokunori Ikegami <ikegami.t@gmail.com>]

On 2024/03/26 17:20, Sagi Grimberg wrote:
>
>
> On 25/03/2024 17:45, Tokunori Ikegami wrote:
>> Currently nvme_identify_ns() sets the data to NULL if failed.
>
> Where? Nothing sets id to NULL in nvme_identify_n(), what am I missing?

It is set as below.

https://github.com/torvalds/linux/blame/master/drivers/nvme/host/core.c#L1542

>> Also the data is not freed if the function returned failure.
>
> Where? I think you may be looking at a different code base?

The ns_update_nuse() returns without the free if the ns_update_nuse() 
failure as below and the ns_head_update_nuse() also same.

https://github.com/torvalds/linux/blame/master/drivers/nvme/host/sysfs.c#L224
>
> -- 
> int nvme_identify_ns(struct nvme_ctrl *ctrl, unsigned nsid,
>                         struct nvme_id_ns **id)
> {
>         ....
>         error = nvme_submit_sync_cmd(ctrl->admin_q, &c, *id, 
> sizeof(**id));
>         if (error) {
>                 dev_warn(ctrl->device, "Identify namespace failed 
> (%d)\n", error);
>                 kfree(*id);
>         }
>         return error;
> }
>
> I don't see how this patch makes sense...

I mean the id pointer should be initialized to NULL before the 
nvme_identify_ns() call. Currently above both the NULL setting and the 
failure return without the free the double error issue resolved but 
looks ideally initialize the data to NULL.