Subject: Re: [PATCH 4/4] usb: libusual: locking cleanup
From: Daniel Walker <dwalker@mvista.com>
To: Pete Zaitcev <zaitcev@redhat.com>
Cc: akpm@linux-foundation.org, mingo@elte.hu, linux-kernel@vger.kernel.org,
       linux@bohmer.net, jonathan@jonmasters.org, matthias.kaehlcke@gmail.com,
       kjwinchester@gmail.com
In-Reply-To: <20071221222428.a75a5a34.zaitcev@redhat.com>
References: <20071221205854.408865412@mvista.com>
	 <20071221205859.316759032@mvista.com>
	 <20071221222428.a75a5a34.zaitcev@redhat.com>
Content-Type: text/plain
Date: Sat, 22 Dec 2007 09:01:50 -0800
Message-Id: <1198342910.2742.14.camel@imap.mvista.com>
Mime-Version: 1.0
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2037
Lines: 69

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

--
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/