Re: [PATCH] Newbus locking

On Saturday 01 August 2009 04:17:42 Giovanni Trematerra wrote:
> > On Fri, Jul 31, 2009 at 7:19 PM, Hans Petter Selasky<hselasky_at_c2i.net>
> > wrote:
> >
> >
> > I'm not saying that your approach will not work or that it is wrong. I'm
> > saying that it is not fast enough. Your patch affects the boottime, in a
> > negative way.
>
> I tested a patch for a while. I didn't notice any slow down in boot time.
> Well, I haven't measured it but I can't see any noticeable difference
> even booting from an USB key.

Hi,

We are talking about some seconds. Store the "ticks" varible in "usb_attach()" 
in sys/dev/usb/controller/usb_controller.c and print out the difference every 
time "usb_bus_explore()" is called having "if (bus->bus_roothold != NULL)".

Different motherboards have different number of ports. Some have just two 
ports others have seven or more. The boot time is after the proposed newbus 
lock patch, proportional to the total number of ports including HUBs.

I'm gone for the weekend and back on Sunday evening. No e-mails will be 
answered until then.

Attilio: Your newbus_lock() must be moved into usb_probe_and_attach(), and 
maybe in usb_suspend_resume(). newbus_lock() should be locked always after 
"udev->default_sx + 1" in usb_device.c. "udev->default_sx + 1" is the lock 
protecting enumeration on a per-device level. Try on a usb device:

usbconfig -u XXX -a YYY set_config 255

Then:

usbconfig -u XXX -a YYY set_config 0

And I think you will have a prompt panic, because the newbus lock is not 
locked.

>
> I have to be honest, I don't understand the patch but any comments on
> performance without an evident profiling seems to me a
> "premature optimization" that is known to be the root of all devils (D.
> Knuth)

BTW: Why do none of the device_get_xxx() functions not have newbus lock 
assertions in them?

--HPS