On Thu, Feb 06, 2020 at 12:51:58PM -0500, Dennis Dalessandro wrote:
> On 2/6/2020 8:52 AM, Jason Gunthorpe wrote:
> > On Wed, Feb 05, 2020 at 04:59:11PM -0500, Dennis Dalessandro wrote:
> > > > I would actively block an attempt to try and do an end-run around
> > > > upstream like this. rdma-core is supposed to be the defacto
> > > > configuration, not be modified randomly by distros as before.
> > >
> > > No but users should be free to name their devices how they want should they
> > > not?
> >
> > Isn't that exactly why PSM is broken?
> >
> > These days I can do
> >
> > $ rdma link add hfi1_0 type siw netdev eth0
> >
> > and PSM will become very confused.
> >
> > This is why keying off the device name was *never* OK.
> >
> > > > Why isn't psm keying off it's own chardev anyhow? There should be back
> > > > links to the RDMA device in sysfs from there.
> > >
> > > No arguments here. No sense in going down this road though at this point in
> > > the game.
> >
> > I'm not sure what these means? Are you saying you won't be fixing PSM? Why?
> >
>
> It's not worth going through the same to have a cdev or not argument over
> again.
What do you mean? You already have the extra cdev. It has a stable
name - I see hfi1_%d used as the pattern.
It even creates a class called hfi1
If you want to enumerate devices of this class readdir on
/sys/class/hfi1. This is yours, it will have only your devices. This
is what should have been done from the beginning.
If you want to find the RDMA parent then
readlink("/sys/class/hfi1/hfi1_xx/") + "/../.." should give it to you
Assuming the driver doesn't screw up the usage of sysfs.. Oh, right,
it does, and that never did get fixed.
The struct hfi1_devdata can not have both a struct kobj and a struct
ib_device as inline members. It *should not* have a kobject at all, it
should be using cdev_device_add() and specifying the ib_device as the
parent. Due to these mistakes I suppose the hfi1_x chardev is placed
incorrectly in sysfs.
So you fix it all up. Fix the kernel, fix the psm, etc. You insistend
on this extra char dev, you need to take responsibility to make sure
it is done properly and doesn't interfere with the rest of the system.
Jason
