[PATCH v2] nvme: Add sibling to list after full initialization

[Sagi Grimberg]

>>> Thanks,
>>>
>>> applied to nvme-5.16.
>>
>> I thought this is not needed upstream. Daniel?
> 
> Why?

The correspondence on v1 of this patch concluded that it
is not needed for upstream.

--
On Sun, Nov 14, 2021 at 12:56:55PM +0200, Sagi Grimberg wrote:
 >> Adding the newly created namespace before the object is fully
 >> initialized is opening a race with nvme_mpath_revalidate_paths() which
 >> tries to access ns->disk. ns->disk can still be NULL when iterating
 >> over the sibling list.
 >
 > But ns->disk is assigned before nvme_init_ns_head is even called.

ns->disk is NULL, but we are missing 5f432cceb3e9 ("nvme: use
blk_mq_alloc_disk") in our kernels. The commit moves the ns->disk
assignment in front of nvme_init_ns_head.

I didn't spot the code change when I forward ported the patch.

Sorry for the noise and thanks for the feedback!
Daniel
--

However I do think we need the following:
--
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 3725b6a3791c..516eedeca044 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -3885,8 +3885,8 @@ static int nvme_init_ns_head(struct nvme_ns *ns, 
unsigned nsid,
                 }
         }

-       list_add_tail_rcu(&ns->siblings, &head->list);
         ns->head = head;
+       list_add_tail_rcu(&ns->siblings, &head->list);
         mutex_unlock(&ctrl->subsys->lock);
         return 0;
--