On Tue, Dec 18, 2018 at 09:50:44AM -0700, Parav Pandit wrote: > > > > From: Jason Gunthorpe > > Sent: Tuesday, December 18, 2018 10:35 AM > > To: Parav Pandit <parav@xxxxxxxxxxxx> > > Cc: Leon Romanovsky <leon@xxxxxxxxxx>; Doug Ledford > > <dledford@xxxxxxxxxx>; Leon Romanovsky <leonro@xxxxxxxxxxxx>; RDMA > > mailing list <linux-rdma@xxxxxxxxxxxxxxx>; Greg Kroah-Hartman > > <gregkh@xxxxxxx> > > Subject: Re: [PATCH rdma-next] IB/cm: Properly create sysfs layout of RDMA > > devices with InfiniBand link layer > > > > On Tue, Dec 18, 2018 at 09:32:16AM -0700, Parav Pandit wrote: > > > > > > > @@ -4368,13 +4371,12 @@ static void cm_add_one(struct ib_device > > > > *ib_device) > > > > > cm_dev->ib_device = ib_device; > > > > > cm_dev->ack_delay = ib_device->attrs.local_ca_ack_delay; > > > > > cm_dev->going_down = 0; > > > > > - cm_dev->device = device_create(&cm_class, &ib_device->dev, > > > > > + cm_dev->devnum = ida_alloc(&cm_ida, GFP_KERNEL); > > > > > + cm_dev->device = device_create(&cm_class, ib_device->dev.parent, > > > > > MKDEV(0, 0), NULL, > > > > > > > > Yikes, this is horrifying, why is it creating a struct device at all like this?? > > > > > > > > Probably the CM counter group debug sysfs should just be added > > > > directly to the actual ib_dev's sysfs?? > > > > > > Yes. anchoring to ibdev's sysfs looks right way but that breaks the > > > ABI/scripts who are accustomed to use it from infiniband_cm. > > > > But doesn't this patch rename those paths already? > > Yes, it breaks. I did raise this to Leon yesterday. But this patch > solves some other problem... Well, if we decide to break it then lets break it and end up in a sane spot.. > > What paths do you think people are using? > > > cat /sys/class/infiniband_cm/mlx5_0/1/cm_tx_retries/req Could we put a symlink here perhaps - one that tracks rename maybe? Jason
