Re: [PATCH 2.6.23] ibmebus: Prevent bus_id collisions

[Joachim Fenkes]

Nathan Lynch <ntl@xxxxxxxxx> wrote on 29.08.2007 20:12:32:

> > Previously, ibmebus derived a device's bus_id from its location code. 
The
> > location code is not guaranteed to be unique, so we might get bus_id
> > collisions if two devices share the same location code. The OFDT 
full_name,
> > however, is unique, so we use that instead.
> 
> This is a userspace-visible change, but I guess it's unavoidable.
> Will anything break?

Nope. Userspace programs should not depend on ibmebus' way of naming the 
devices; especially since some overly long loc_codes tended to be 
truncated and thus rendered useless. I have tested IBM's DLPAR tools 
against the changed kernel, and they didn't break.
 
> Also, I dislike this approach of duplicating the firmware device tree
> path in sysfs.

Why? Any specific reasons for your dislike?

> Are GX/ibmebus devices guaranteed to be children of
> the same node in the OF device tree?  If so, their unit addresses will
> be unique, and therefore suitable values for bus_id.  I believe this
> is what the powerpc vio bus code does.

While there's no such guarantee (as in "officially signed document"), yes, 
I expect future GX devices to also appear beneath the OFDT root node. For 
the existing devices, the unit addresses are already part of the device 
name, so I save the need to use sprintf() again. Plus, I rather like using 
the full_name since it also contains a descriptive name as opposed to 
being just nondescript numbers, helping the layman (ie user) to make sense 
out of a dev_id.

jschopp <jschopp@xxxxxxxxxxxxxx> wrote on 29.08.2007 20:33:30:

> > +   len = strlen(dn->full_name + 1);
> > +   bus_len = min(len, BUS_ID_SIZE - 1);
> > +   memcpy(dev->ofdev.dev.bus_id, dn->full_name + 1
> > +          + (len - bus_len), bus_len);
> > +   for (i = 0; i < bus_len; i++)
> > +      if (dev->ofdev.dev.bus_id[i] == '/')
> > +         dev->ofdev.dev.bus_id[i] = '_';
> > 
> >     /* Register with generic device framework. */
> >     if (ibmebus_register_device_common(dev, dn->name) != 0) {
> 
> What happens when the full name is > 31 characters?  It looks to me that 
it 
> will be truncated, which takes away the uniqueness guarantee.

There are currently two GX devices, eHCA and eHEA, which both reside 
beneath the root node - this is required by architecture for those 
devices. Unless they invent a device called 
"supercalifragilisticexpialidocious", devices in the root note will have a 
full_name of less than 31 chars. Even in that case, the truncation occurs 
at the beginning, so the @xxx part that makes the nodes unique will stay 
in place.

If any more GX devices appear on the scene, I expect them to appear in the 
root node as well. The substitution of "/" by "_" is a safeguard so 
possible weird OFDT setups don't break the kernel.
 
> There must be an individual property that is guaranteed to be unique and 

> less than 32 characters.  How about "ibm,my-drc-index"?  That looks like 
a 
> good candidate.

On first glance, it does, however the attribute might not be present in 
all cases. Architecture states it only needs to be present on systems with 
dynamic reconfiguration enabled.

All things considered, I still like the idea of using the full_name most. 
No offense.

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