mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
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
Subject: Re: [PATCH 4/4] usb: libusual: locking cleanup
Date: Sat, 22 Dec 2007 09:01:50 -0800	[thread overview]
Message-ID: <1198342910.2742.14.camel@imap.mvista.com> (raw)
In-Reply-To: <20071221222428.a75a5a34.zaitcev@redhat.com>

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


  parent reply	other threads:[~2007-12-22 17:02 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20071221205848.989157559@mvista.com>
2007-12-21  8:00 ` [PATCH 1/4] usb microtek: remove unused semaphore Daniel Walker
2007-12-21  8:00 ` [PATCH 2/4] prism54: remove questionable down_interruptible usage Daniel Walker
2007-12-21  8:00 ` [PATCH 3/4] docs: convert kref semaphore to mutex Daniel Walker
2007-12-21 21:33   ` Corey Minyard
2007-12-21  8:00 ` [PATCH 4/4] usb: libusual: locking cleanup Daniel Walker
2007-12-22  4:22   ` Andrew Morton
2007-12-22  6:24   ` Pete Zaitcev
2007-12-22  6:31     ` Andrew Morton
2007-12-22 17:01     ` Daniel Walker [this message]
2007-12-23  7:37       ` Pete Zaitcev
2007-12-23 16:46         ` Daniel Walker
2007-12-24 14:12           ` Pete Zaitcev
2007-12-24 16:04             ` Daniel Walker

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1198342910.2742.14.camel@imap.mvista.com \
    --to=dwalker@mvista.com \
    --cc=akpm@linux-foundation.org \
    --cc=jonathan@jonmasters.org \
    --cc=kjwinchester@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@bohmer.net \
    --cc=matthias.kaehlcke@gmail.com \
    --cc=mingo@elte.hu \
    --cc=zaitcev@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®