Re: [PATCH 03/10] nvme-multipath: add error handling support for add_disk()

2021-08-30 Thread Luis Chamberlain 
On Fri, Aug 27, 2021 at 01:29:32PM -0700, Keith Busch wrote:
> On Fri, Aug 27, 2021 at 12:18:02PM -0700, Luis Chamberlain wrote:
> > @@ -479,13 +479,17 @@ int nvme_mpath_alloc_disk(struct nvme_ctrl *ctrl, 
> > struct nvme_ns_head *head)
> >  static void nvme_mpath_set_live(struct nvme_ns *ns)
> >  {
> > struct nvme_ns_head *head = ns->head;
> > +   int rc;
> >  
> > if (!head->disk)
> > return;
> >  
> > -   if (!test_and_set_bit(NVME_NSHEAD_DISK_LIVE, &head->flags)) {
> > -   device_add_disk(&head->subsys->dev, head->disk,
> > -   nvme_ns_id_attr_groups);
> > +   if (!test_bit(NVME_NSHEAD_DISK_LIVE, &head->flags)) {
> 
> This should still be test_and_set_bit() because it is protecting against
> two nvme paths simultaneously calling device_add_disk() on the same
> namespace head.

Interesting, I'll add a comment as well, as this was not clear with the drive
by effort.

  Luis

Re: [PATCH 03/10] nvme-multipath: add error handling support for add_disk()

2021-08-27 Thread Keith Busch 
On Fri, Aug 27, 2021 at 12:18:02PM -0700, Luis Chamberlain wrote:
> @@ -479,13 +479,17 @@ int nvme_mpath_alloc_disk(struct nvme_ctrl *ctrl, 
> struct nvme_ns_head *head)
>  static void nvme_mpath_set_live(struct nvme_ns *ns)
>  {
>   struct nvme_ns_head *head = ns->head;
> + int rc;
>  
>   if (!head->disk)
>   return;
>  
> - if (!test_and_set_bit(NVME_NSHEAD_DISK_LIVE, &head->flags)) {
> - device_add_disk(&head->subsys->dev, head->disk,
> - nvme_ns_id_attr_groups);
> + if (!test_bit(NVME_NSHEAD_DISK_LIVE, &head->flags)) {

This should still be test_and_set_bit() because it is protecting against
two nvme paths simultaneously calling device_add_disk() on the same
namespace head.

> + rc = device_add_disk(&head->subsys->dev, head->disk,
> +  nvme_ns_id_attr_groups);
> + if (rc)
> + return;
> + set_bit(NVME_NSHEAD_DISK_LIVE, &head->flags);
>   nvme_add_ns_head_cdev(head);
>   }

[PATCH 03/10] nvme-multipath: add error handling support for add_disk()

2021-08-27 Thread Luis Chamberlain 
We never checked for errors on add_disk() as this function
returned void. Now that this is fixed, use the shiny new
error handling.

Since we now can tell for sure when a disk was added, move
setting the bit NVME_NSHEAD_DISK_LIVE only when we did
add the disk successfully.

Nothing to do here as the cleanup is done elsewhere.

Signed-off-by: Luis Chamberlain 
---
 drivers/nvme/host/multipath.c | 10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
index 37ce3e8b1db2..f95643629fdb 100644
--- a/drivers/nvme/host/multipath.c
+++ b/drivers/nvme/host/multipath.c
@@ -479,13 +479,17 @@ int nvme_mpath_alloc_disk(struct nvme_ctrl *ctrl, struct 
nvme_ns_head *head)
 static void nvme_mpath_set_live(struct nvme_ns *ns)
 {
struct nvme_ns_head *head = ns->head;
+   int rc;
 
if (!head->disk)
return;
 
-   if (!test_and_set_bit(NVME_NSHEAD_DISK_LIVE, &head->flags)) {
-   device_add_disk(&head->subsys->dev, head->disk,
-   nvme_ns_id_attr_groups);
+   if (!test_bit(NVME_NSHEAD_DISK_LIVE, &head->flags)) {
+   rc = device_add_disk(&head->subsys->dev, head->disk,
+nvme_ns_id_attr_groups);
+   if (rc)
+   return;
+   set_bit(NVME_NSHEAD_DISK_LIVE, &head->flags);
nvme_add_ns_head_cdev(head);
}
 
-- 
2.30.2