> -----Original Message-----
> From: Jason Gunthorpe
> Sent: Friday, May 3, 2019 5:50 PM
> To: Leon Romanovsky <leon@xxxxxxxxxx>
> Cc: Doug Ledford <dledford@xxxxxxxxxx>; Parav Pandit
> <parav@xxxxxxxxxxxx>; RDMA mailing list <linux-rdma@xxxxxxxxxxxxxxx>;
> Mike Marciniszyn <mike.marciniszyn@xxxxxxxxx>; Leon Romanovsky
> <leonro@xxxxxxxxxxxx>
> Subject: Re: [PATCH rdma-next] RDMA/core: Do not invoke init_port on
> compat devices
>
> On Wed, May 01, 2019 at 08:46:55AM +0300, Leon Romanovsky wrote:
> > From: Parav Pandit <parav@xxxxxxxxxxxx>
> >
> > As compat devices in net namespaces may not need vendor specific
> > control attributes, avoid invoking init_port() on them.
> >
> > This avoids below reported call trace.
> >
> > Call Trace:
> > dump_stack+0x5a/0x73
> > kobject_init+0x74/0x80
> > kobject_init_and_add+0x35/0xb0
> > hfi1_create_port_files+0x6e/0x3c0 [hfi1]
> > ib_setup_port_attrs+0x43b/0x560 [ib_core]
> > add_one_compat_dev+0x16a/0x230 [ib_core]
> > rdma_dev_init_net+0x110/0x160 [ib_core]
> > ops_init+0x38/0xf0
> > setup_net+0xcf/0x1e0
> > copy_net_ns+0xb7/0x130
> > create_new_namespaces+0x11a/0x1b0
> > unshare_nsproxy_namespaces+0x55/0xa0
> > ksys_unshare+0x1a7/0x340
> > __x64_sys_unshare+0xe/0x20
> > do_syscall_64+0x5b/0x180
> > entry_SYSCALL_64_after_hwframe+0x44/0xa9
> >
> > Fixes: 5417783eabb2 ("RDMA/core: Support core port attributes in non
> > init_net")
> > Reported-by: Mike Marciniszyn <mike.marciniszyn@xxxxxxxxx>
> > Tested-by: Mike Marciniszyn <mike.marciniszyn@xxxxxxxxx>
> > Signed-off-by: Parav Pandit <parav@xxxxxxxxxxxx>
> > Signed-off-by: Leon Romanovsky <leonro@xxxxxxxxxxxx>
> > ---
> > drivers/infiniband/core/core_priv.h | 2 +-
> > drivers/infiniband/core/sysfs.c | 13 +++++++------
> > 2 files changed, 8 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/infiniband/core/core_priv.h
> > b/drivers/infiniband/core/core_priv.h
> > index 2764647056d8..77956c42358e 100644
> > --- a/drivers/infiniband/core/core_priv.h
> > +++ b/drivers/infiniband/core/core_priv.h
> > @@ -342,7 +342,7 @@ struct net_device
> > *rdma_read_gid_attr_ndev_rcu(const struct ib_gid_attr *attr);
> >
> > void ib_free_port_attrs(struct ib_core_device *coredev); int
> > ib_setup_port_attrs(struct ib_core_device *coredev,
> > - bool alloc_hw_stats);
> > + bool init_port_alloc_stats);
>
> Yuk this name is ugly. I reworked it a bit and put this in the wip branch,
> please check it:
>
Patch looks good.
I kept the flag to keep door open in case if user requires stats in ns.
But its fine, we can work through it at later point if required.
> commit eb15c78b05bd9fbac45ee5b56aaf29b2570b5238
> Author: Parav Pandit <parav@xxxxxxxxxxxx>
> Date: Wed May 1 08:46:55 2019 +0300
>
> RDMA/core: Do not invoke init_port on compat devices
>
> The driver interface cannot manipulate the sysfs of the compat device,
> only of the full device so we must avoid calling the driver sysfs APIs on
> compat devices.
>
> This prevents an oops:
>
> Call Trace:
> dump_stack+0x5a/0x73
> kobject_init+0x74/0x80
> kobject_init_and_add+0x35/0xb0
> hfi1_create_port_files+0x6e/0x3c0 [hfi1]
> ib_setup_port_attrs+0x43b/0x560 [ib_core]
> add_one_compat_dev+0x16a/0x230 [ib_core]
> rdma_dev_init_net+0x110/0x160 [ib_core]
> ops_init+0x38/0xf0
> setup_net+0xcf/0x1e0
> copy_net_ns+0xb7/0x130
> create_new_namespaces+0x11a/0x1b0
> unshare_nsproxy_namespaces+0x55/0xa0
> ksys_unshare+0x1a7/0x340
> __x64_sys_unshare+0xe/0x20
> do_syscall_64+0x5b/0x180
> entry_SYSCALL_64_after_hwframe+0x44/0xa9
>
> Fixes: 5417783eabb2 ("RDMA/core: Support core port attributes in non
> init_net")
> Reported-by: Mike Marciniszyn <mike.marciniszyn@xxxxxxxxx>
> Tested-by: Mike Marciniszyn <mike.marciniszyn@xxxxxxxxx>
> Signed-off-by: Parav Pandit <parav@xxxxxxxxxxxx>
> Signed-off-by: Leon Romanovsky <leonro@xxxxxxxxxxxx>
> Signed-off-by: Jason Gunthorpe <jgg@xxxxxxxxxxxx>
>
> diff --git a/drivers/infiniband/core/core_priv.h
> b/drivers/infiniband/core/core_priv.h
> index 2764647056d8fd..ff40a450b5d28e 100644
> --- a/drivers/infiniband/core/core_priv.h
> +++ b/drivers/infiniband/core/core_priv.h
> @@ -341,8 +341,7 @@ int roce_resolve_route_from_path(struct
> sa_path_rec *rec, struct net_device *rdma_read_gid_attr_ndev_rcu(const
> struct ib_gid_attr *attr);
>
> void ib_free_port_attrs(struct ib_core_device *coredev); -int
> ib_setup_port_attrs(struct ib_core_device *coredev,
> - bool alloc_hw_stats);
> +int ib_setup_port_attrs(struct ib_core_device *coredev);
>
> int rdma_compatdev_set(u8 enable);
>
> diff --git a/drivers/infiniband/core/device.c
> b/drivers/infiniband/core/device.c
> index 76088655f06ef7..2123cc693a2959 100644
> --- a/drivers/infiniband/core/device.c
> +++ b/drivers/infiniband/core/device.c
> @@ -870,7 +870,7 @@ static int add_one_compat_dev(struct ib_device
> *device,
> ret = device_add(&cdev->dev);
> if (ret)
> goto add_err;
> - ret = ib_setup_port_attrs(cdev, false);
> + ret = ib_setup_port_attrs(cdev);
> if (ret)
> goto port_err;
>
> diff --git a/drivers/infiniband/core/sysfs.c b/drivers/infiniband/core/sysfs.c
> index 2fe89754e59207..7a599c5e455fa9 100644
> --- a/drivers/infiniband/core/sysfs.c
> +++ b/drivers/infiniband/core/sysfs.c
> @@ -1015,10 +1015,10 @@ static void setup_hw_stats(struct ib_device
> *device, struct ib_port *port,
> return;
> }
>
> -static int add_port(struct ib_core_device *coredev,
> - int port_num, bool alloc_stats)
> +static int add_port(struct ib_core_device *coredev, int port_num)
> {
> struct ib_device *device = rdma_device_to_ibdev(&coredev->dev);
> + bool is_full_dev = &device->coredev == coredev;
> struct ib_port *p;
> struct ib_port_attr attr;
> int i;
> @@ -1057,7 +1057,7 @@ static int add_port(struct ib_core_device *coredev,
> goto err_put;
> }
>
> - if (device->ops.process_mad && alloc_stats) {
> + if (device->ops.process_mad && is_full_dev) {
> p->pma_table = get_counter_table(device, port_num);
> ret = sysfs_create_group(&p->kobj, p->pma_table);
> if (ret)
> @@ -1113,7 +1113,7 @@ static int add_port(struct ib_core_device *coredev,
> if (ret)
> goto err_free_pkey;
>
> - if (device->ops.init_port) {
> + if (device->ops.init_port && is_full_dev) {
> ret = device->ops.init_port(device, port_num, &p->kobj);
> if (ret)
> goto err_remove_pkey;
> @@ -1124,7 +1124,7 @@ static int add_port(struct ib_core_device *coredev,
> * port, so holder should be device. Therefore skip per port conunter
> * initialization.
> */
> - if (device->ops.alloc_hw_stats && port_num && alloc_stats)
> + if (device->ops.alloc_hw_stats && port_num && is_full_dev)
> setup_hw_stats(device, p, port_num);
>
> list_add_tail(&p->kobj.entry, &coredev->port_list); @@ -1308,7
> +1308,7 @@ void ib_free_port_attrs(struct ib_core_device *coredev)
> kobject_put(coredev->ports_kobj);
> }
>
> -int ib_setup_port_attrs(struct ib_core_device *coredev, bool alloc_stats)
> +int ib_setup_port_attrs(struct ib_core_device *coredev)
> {
> struct ib_device *device = rdma_device_to_ibdev(&coredev->dev);
> unsigned int port;
> @@ -1320,7 +1320,7 @@ int ib_setup_port_attrs(struct ib_core_device
> *coredev, bool alloc_stats)
> return -ENOMEM;
>
> rdma_for_each_port (device, port) {
> - ret = add_port(coredev, port, alloc_stats);
> + ret = add_port(coredev, port);
> if (ret)
> goto err_put;
> }
> @@ -1336,7 +1336,7 @@ int ib_device_register_sysfs(struct ib_device
> *device) {
> int ret;
>
> - ret = ib_setup_port_attrs(&device->coredev, true);
> + ret = ib_setup_port_attrs(&device->coredev);
> if (ret)
> return ret;
>
