Re: [PATCH 9 of 20] ipath - char devices for diagnostics and lightweight subnet management

[Roland Dreier <rdreier@cisco.com>]

    Bryan> I *assumed* that there was something more that we would
    Bryan> need to do in order to support real hotplug of actual
    Bryan> physical cards, but now that I look more closely, it
    Bryan> doesn't appear that there is.  At least, there's nothing in
    Bryan> Documentation/pci.txt or LDD3 that indicates to me that we
    Bryan> ought to be doing more.

    Bryan> Am I missing something?

No, the only problems are with the way the various pieces of your
drivers refer to devices by index.  There are obvious races such as

 > +int __init ipath_verbs_init(void)
 > +{
 > +	int i;
 > +
 > +	number_of_devices = ipath_layer_get_num_of_dev();
 > +	i = number_of_devices * sizeof(struct ipath_ibdev *);
 > +	ipath_devices = kmalloc(i, GFP_ATOMIC);
 > +	if (ipath_devices == NULL)
 > +		return -ENOMEM;
 > +
 > +	for (i = 0; i < number_of_devices; i++) {
 > +		struct ipath_devdata *dd;
 > +		int ret = ipath_verbs_register(i, ipath_ib_piobufavail,
 > +					       ipath_ib_rcv, ipath_ib_timer,
 > +					       &dd);

suppose number_of_devices gets set to 5 but by the time you call
ipath_verbs_register(5,...), the 5th device has been unplugged?

Also you only do this when the module is loaded, so you won't handle
devices that are hot-plugged later.  And I don't see anything that
would handle hot unplug either.

Pretty much any use of ipath_max is probably broken by hot plug.

 - R.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/