Re: [PATCH 4/4] usb: libusual: locking cleanup

Daniel Walker
2007-12-22T17:02:43Z

On Fri, 2007-12-21 at 22:24 -0800, Pete Zaitcev wrote:

> When I tried it, usb-storage would not load with unresolved symbols.
> It happens if child (usu_probe_thread) runs ahead of its parent
> (usb_usual_init -> usb_register -> usu_probe). It's entirely possible,
> depending on your scheduler.
>
> I hate this down-up trick too, so if you have a better idea, I'm all ears.

This is what you originally had,

static int usu_probe_thread(void *arg)
{

/* A completion does not work here because it's counted. */
down(&usu_init_notify);
up(&usu_init_notify);
...
}

static int __init usb_usual_init(void)
{
sema_init(&usu_init_notify, 0); <-- Locked init

rc = usb_register(&usu_driver);
up(&usu_init_notify);
return rc;
}

The locked init can easily be an unlocked init combined with a down() ..
So your protecting usb_register() from something else.

Then in usu_probe_thread() your basically stopping it at the start of
the function with a down(), and the up() is just ancillary .. So you
could easily move the up() further down in the function and still have
the same level of exclusion..

static int usu_probe_thread(void *arg)
{

down(&usu_init_notify);
...
up(&usu_init_notify);
}

static int __init usb_usual_init(void)
{
sema_init(&usu_init_notify, 1); <-- Unlocked init

down(&usu_init_notify);
rc = usb_register(&usu_driver);
up(&usu_init_notify);
return rc;
}

The above protects the same way that your original code did, with the
added benefit of conforming to mutex style usage. The next step is to
convert to the mutex API..

What I've done is all suppose to be mathematical translations, I wasn't
trying to improve the code just make it use a different API..

Daniel