mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Arjan van de Ven <arjan@infradead.org>
To: Cornelia Huck <cornelia.huck@de.ibm.com>
Cc: linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org, greg@kroah.com
Subject: Re: [PATCH] device model: Do a quickcheck for driver binding before doing an expensive check
Date: Mon, 15 Sep 2008 07:06:44 -0700	[thread overview]
Message-ID: <20080915070644.5503c613@infradead.org> (raw)
In-Reply-To: <20080915133226.7e433379@gondolin.boeblingen.de.ibm.com>

On Mon, 15 Sep 2008 13:32:26 +0200
Cornelia Huck <cornelia.huck@de.ibm.com> wrote:

> On Sun, 14 Sep 2008 08:32:06 -0700,
> Arjan van de Ven <arjan@infradead.org> wrote:
> 
> > This patch adds a quick check for the driver<->device match before
> > taking the locks and doin gthe expensive checks. Taking the lock
> > hurts in asynchronous boot context where the device lock gets hit;
> > one of the init functions takes the lock and goes to do an
> > expensive hardware init; the other init functions walk the same PCI
> > list and get stuck on the lock as a result.
> 
> Hm, you call bus->match twice now; once without dev->sem held and once
> with it. For the busses I'm familiar with that shouldn't be a problem,
> but are you sure there aren't busses which want dev->sem held?
> (Although I think not relying on dev->sem would be the sane thing...)

As far as I can see it's ok, but if not I obviously like to hear about
it SOON :)


> > 
> > For the common case, we can know there's no chance whatsoever of a
> > match if the device isn't in the drivers ID table... so this patch
> > does that check as a best-effort-avoid-the-lock approach.
> 
> I've always thought of ->match being a quick check which just looks at
> the IDs with ->probe doing the heavier stuff, so this should be
> reasonable (if all busses play nicely). But driver_probe_device()
> still calls ->match a second time, and device_attach() will thus
> always call ->match under the lock. Should it be moved out of the
> lock there as well?

having a second check is actually not a bad thing per se; in terms of
programming pattern, doing the quick checks before the lock, but doing
the final check inside the lock makes sense to me. If there's real
objections to doing the match the second time (it's cheap!) I'll remove
it, but this way, you can call the "heavy" function always and from
anywhere, and it'll just do the right thing no matter what. I kinda like
that as concept ;)



-- 
Arjan van de Ven 	Intel Open Source Technology Centre
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

  reply	other threads:[~2008-09-15 14:07 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-09-14 15:32 Arjan van de Ven
2008-09-15 11:32 ` Cornelia Huck
2008-09-15 14:06   ` Arjan van de Ven [this message]
2008-09-15 14:53     ` Cornelia Huck

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=20080915070644.5503c613@infradead.org \
    --to=arjan@infradead.org \
    --cc=cornelia.huck@de.ibm.com \
    --cc=greg@kroah.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    /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®