linux-kernel - Re: [PATCH] device model: Do a quickcheck for driver binding before doing an expensive check

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20080915070644.5503c613@infradead.org>
Date:	Mon, 15 Sep 2008 07:06:44 -0700
From:	Arjan van de Ven <arjan@...radead.org>
To:	Cornelia Huck <cornelia.huck@...ibm.com>
Cc:	linux-kernel@...r.kernel.org, linux-usb@...r.kernel.org,
	greg@...ah.com
Subject: Re: [PATCH] device model: Do a quickcheck for driver binding before
 doing an expensive check

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

> On Sun, 14 Sep 2008 08:32:06 -0700,
> Arjan van de Ven <arjan@...radead.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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/