Re: [RFC 02/10] IB/hfi-vnic: Virtual Network Interface Controller (VNIC) Bus driver

On Tue, Nov 22, 2016 at 10:04:07AM -0700, Jason Gunthorpe wrote:

On Mon, Nov 21, 2016 at 05:53:04PM -0800, Vishwanathapura, Niranjana wrote:

quoted

There are many example drivers in kernel which are using bus_register() in
an initcall.

There really are not, certainly not in major subsystems.

I see 2 drivers in the Block subsystem which do this:


19   5354  /nfs/site/home/iweiny/linux-stable/drivers/block/cciss.c <<cciss_init>>
	err = bus_register(&cciss_bus_type);
20   6447 /nfs/site/home/iweiny/linux-stable/drivers/block/rbd.c <<rbd_sysfs_init>>
	ret = bus_register(&rbd_bus_type);

2 drivers in the drm subsystem which do this:


29   1155  /nfs/site/home/iweiny/linux-stable/drivers/gpu/drm/drm_mipi_dsi.c <<mipi_dsi_bus_init>>
	return bus_register(&mipi_dsi_bus_type);
30    242 /nfs/site/home/iweiny/linux-stable/drivers/gpu/host1x/dev.c <<tegra_host1x_init>>
	err = bus_register(&host1x_bus_type);

And I think there are a couple others.

I'm not sure what these devices/buses do but they are registering their own bus
while being in another major subsystem.  Is what we are doing really so
crazy/wrong?

quoted

We could add a custom Interface between HFI1 driver and hfi_vnic drivers
without involving a bus.

hfi is already registering on the infiniband class, just use that.

I don't understand what you mean here?

The bus_register provides a really clean way for the hfi1 driver and hfi_vnic
driver to find each other.  This includes being able to support hfi1 with or
without hfi_vnic being loaded.  Note that without configuration from the "EM"
Ethernet Manager the hfi_vnic does not export a net device.

Why wouldn't we use this core kernel support?[*]

quoted

But using the existing bus model gave a lot of in-built flexibility in
decoupling devices from the drivers.

If you want to have your own bus then you need your own hfi
subsystem. drivers/infiniband is not a dumping ground..

We don't consider drivers/infiniband a "dumping ground".  There is a
requirement on ib_mad from the hfi_vnic driver.

Ira

[*] As an aside why does the ib_core not use this methodology?  It dawned on
me that this may be a better way to fix our module load problems.  However, I
have not looked into details.

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Tue, Nov 22, 2016 at 07:05:05PM -0500, ira.weiny wrote:

On Tue, Nov 22, 2016 at 10:04:07AM -0700, Jason Gunthorpe wrote:

quoted

On Mon, Nov 21, 2016 at 05:53:04PM -0800, Vishwanathapura, Niranjana wrote:

quoted

There are many example drivers in kernel which are using bus_register() in
an initcall.

There really are not, certainly not in major subsystems.

I see 2 drivers in the Block subsystem which do this:


19   5354  /nfs/site/home/iweiny/linux-stable/drivers/block/cciss.c <<cciss_init>>
	err = bus_register(&cciss_bus_type);
20   6447 /nfs/site/home/iweiny/linux-stable/drivers/block/rbd.c <<rbd_sysfs_init>>
	ret = bus_register(&rbd_bus_type);

2 drivers in the drm subsystem which do this:


29   1155  /nfs/site/home/iweiny/linux-stable/drivers/gpu/drm/drm_mipi_dsi.c <<mipi_dsi_bus_init>>
	return bus_register(&mipi_dsi_bus_type);
30    242 /nfs/site/home/iweiny/linux-stable/drivers/gpu/host1x/dev.c <<tegra_host1x_init>>
	err = bus_register(&host1x_bus_type);

IMHO this is all obscure or legacy stuff (eg ccsiss lost its bus when
it was reworked into hpsa). Who knows about that SOC stuff, maybe
there really is a special on-chip bus under those drivers.

The point is using a bus as a generic interconnect between two driver
modules seems very rare, and is not what we have historically ever
done in drivers/infiniband - all our split drivers use a trivial
register scheme. eg see cxgb4_register_uld/mlx4_register_interface/etc.

Should a multi-function driver use a bus or class to connect its
parts? Who knows. Maybe Greg KH/etc has an opinion. But that is not
what we have been doing, it doesn't seem very simplifying, and
this series doesn't even make module auto-loading work...

Since doing this creates a bunch of uapis (again, from a driver, ugh) it
seems like a bad idea without more support as 'the right way'

.. and yes, it would be nice to have a lightweight mechanism to
replace those register functions that could handle module auto loading
too, and maybe that is a 'multi part driver bus/class' or somesuch
... This is really a topic for the device core maintainers, IMHO.

quoted

quoted

We could add a custom Interface between HFI1 driver and hfi_vnic drivers
without involving a bus.

hfi is already registering on the infiniband class, just use that.

I don't understand what you mean here?

Get the struct ib_device for the hfi and then do something to get hfi
specific function calls.

Or work it backwards with a _register function..

[*] As an aside why does the ib_core not use this methodology?  It dawned on
me that this may be a better way to fix our module load problems.  However, I
have not looked into details.

ib_core is a class, which is appropriate. RDMA devices are not busses.

Jason