RDMA/srp: Rename SRP sysfs name after IB device rename trigger
authorLeon Romanovsky <leonro@mellanox.com>
Fri, 17 May 2019 12:43:10 +0000 (15:43 +0300)
committerJason Gunthorpe <jgg@mellanox.com>
Tue, 21 May 2019 18:06:45 +0000 (15:06 -0300)
SRP logic used device name and port index as symlink to relevant
kobject. If the IB device is renamed then the prior name will be re-used
by the next device plugged in and sysfs will panic as SRP will try to
re-use the same name.

 mlx5_ib: Mellanox Connect-IB Infiniband driver v5.0-0
 sysfs: cannot create duplicate filename '/class/infiniband_srp/srp-mlx5_0-1'
 CPU: 3 PID: 1107 Comm: modprobe Not tainted 5.1.0-for-upstream-perf-2019-05-12_15-09-52-87 #1
 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1ubuntu1 04/01/2014
 Call Trace:
  dump_stack+0x5a/0x73
  sysfs_warn_dup+0x58/0x70
  sysfs_do_create_link_sd.isra.2+0xa3/0xb0
  device_add+0x33f/0x660
  srp_add_one+0x301/0x4f0 [ib_srp]
  add_client_context+0x99/0xe0 [ib_core]
  enable_device_and_get+0xd1/0x1b0 [ib_core]
  ib_register_device+0x533/0x710 [ib_core]
  ? mutex_lock+0xe/0x30
  __mlx5_ib_add+0x23/0x70 [mlx5_ib]
  mlx5_add_device+0x4e/0xd0 [mlx5_core]
  mlx5_register_interface+0x85/0xc0 [mlx5_core]
  ? 0xffffffffa0791000
  do_one_initcall+0x4b/0x1cb
  ? kmem_cache_alloc_trace+0xc6/0x1d0
  ? do_init_module+0x22/0x21f
  do_init_module+0x5a/0x21f
  load_module+0x17f2/0x1ca0
  ? m_show+0x1c0/0x1c0
  __do_sys_finit_module+0x94/0xe0
  do_syscall_64+0x48/0x120
  entry_SYSCALL_64_after_hwframe+0x44/0xa9
 RIP: 0033:0x7f157cce10d9

The module load/unload sequence was used to trigger such kernel panic:
 sudo modprobe ib_srp
 sudo modprobe -r mlx5_ib
 sudo modprobe -r mlx5_core
 sudo modprobe mlx5_core

Have SRP track the name of the core device so that it can't have a name
collision.

Fixes: d21943dd19b5 ("RDMA/core: Implement IB device rename function")
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Jason Gunthorpe <jgg@mellanox.com>
drivers/infiniband/core/device.c
drivers/infiniband/ulp/srp/ib_srp.c
include/rdma/ib_verbs.h

index 78dc07c6ac4b24448a4e8c9bcb3d41906ab16398..cd6b679badfe497981baca896ee9234719230ede 100644 (file)
@@ -409,27 +409,44 @@ static int rename_compat_devs(struct ib_device *device)
 
 int ib_device_rename(struct ib_device *ibdev, const char *name)
 {
+       unsigned long index;
+       void *client_data;
        int ret;
 
        down_write(&devices_rwsem);
        if (!strcmp(name, dev_name(&ibdev->dev))) {
-               ret = 0;
-               goto out;
+               up_write(&devices_rwsem);
+               return 0;
        }
 
        if (__ib_device_get_by_name(name)) {
-               ret = -EEXIST;
-               goto out;
+               up_write(&devices_rwsem);
+               return -EEXIST;
        }
 
        ret = device_rename(&ibdev->dev, name);
-       if (ret)
-               goto out;
+       if (ret) {
+               up_write(&devices_rwsem);
+               return ret;
+       }
+
        strlcpy(ibdev->name, name, IB_DEVICE_NAME_MAX);
        ret = rename_compat_devs(ibdev);
-out:
-       up_write(&devices_rwsem);
-       return ret;
+
+       downgrade_write(&devices_rwsem);
+       down_read(&ibdev->client_data_rwsem);
+       xan_for_each_marked(&ibdev->client_data, index, client_data,
+                           CLIENT_DATA_REGISTERED) {
+               struct ib_client *client = xa_load(&clients, index);
+
+               if (!client || !client->rename)
+                       continue;
+
+               client->rename(ibdev, client_data);
+       }
+       up_read(&ibdev->client_data_rwsem);
+       up_read(&devices_rwsem);
+       return 0;
 }
 
 static int alloc_name(struct ib_device *ibdev, const char *name)
index be9ddcad8f28745843edf273391319efc817fb5d..4305da2c9037f0185e8adc48db247c6b4fb5771b 100644 (file)
@@ -148,6 +148,7 @@ MODULE_PARM_DESC(ch_count,
 
 static void srp_add_one(struct ib_device *device);
 static void srp_remove_one(struct ib_device *device, void *client_data);
+static void srp_rename_dev(struct ib_device *device, void *client_data);
 static void srp_recv_done(struct ib_cq *cq, struct ib_wc *wc);
 static void srp_handle_qp_err(struct ib_cq *cq, struct ib_wc *wc,
                const char *opname);
@@ -162,7 +163,8 @@ static struct workqueue_struct *srp_remove_wq;
 static struct ib_client srp_client = {
        .name   = "srp",
        .add    = srp_add_one,
-       .remove = srp_remove_one
+       .remove = srp_remove_one,
+       .rename = srp_rename_dev
 };
 
 static struct ib_sa_client srp_sa_client;
@@ -4112,6 +4114,20 @@ free_host:
        return NULL;
 }
 
+static void srp_rename_dev(struct ib_device *device, void *client_data)
+{
+       struct srp_device *srp_dev = client_data;
+       struct srp_host *host, *tmp_host;
+
+       list_for_each_entry_safe(host, tmp_host, &srp_dev->dev_list, list) {
+               char name[IB_DEVICE_NAME_MAX + 8];
+
+               snprintf(name, sizeof(name), "srp-%s-%d",
+                        dev_name(&device->dev), host->port);
+               device_rename(&host->dev, name);
+       }
+}
+
 static void srp_add_one(struct ib_device *device)
 {
        struct srp_device *srp_dev;
index 0742095355f28c7bbf994ab137cc113f0aa76760..54873085f2dab21257680f63c2e8db1e83416b8c 100644 (file)
@@ -2698,6 +2698,7 @@ struct ib_client {
        const char *name;
        void (*add)   (struct ib_device *);
        void (*remove)(struct ib_device *, void *client_data);
+       void (*rename)(struct ib_device *dev, void *client_data);
 
        /* Returns the net_dev belonging to this ib_client and matching the
         * given parameters.