Messages in this thread | | | Subject | Re: [PATCH 1/2] vga: implements VGA arbitration on Linux | From | Benjamin Herrenschmidt <> | Date | Fri, 17 Jul 2009 10:20:10 +1000 |
| |
On Thu, 2009-07-16 at 09:48 +0100, Alan Cox wrote: > > > >> + pdev = vga_default_device(); > > > > > > What if the BIOS provided device was hot unplugged ? > > > > we just use the pdev as a cookie, if it was hot unplugged we'll > > have gotten a callback to remove it from the VGA device list > > and the lookup which happens 5 lines later inside the spinlock > > will fail. > > What if I inserted a new device - the allocator might give me a new > device with the same pointer if its reusing the same slab entry for > that > size. Unlikely but given a zillion boxes this starts to occur 8(
The original proof-of concept draft code I wrote (and this is -very- close to it :-) didn't quite handle hotplug of the default device. That does need to be sorted.
But then, I have a hard time finding any useful locking to use vs. PCI hotplug, so it's a non trivial matter.
> Not commented on - but a serious question would be "do we actually > care enough or are there really devices with just I/O and just vga > memory access used ?"
I honestly cannot remember why I split the 4 resource types that way back then. I have the nagging feeling that I had a good reason to do so but it totally eludes me today :-)
The one main thing I wanted was to keep legacy vs. standard resources so that the all the portions of a driver (DDX, DRM, etc...) that use standard resources can continue to lock/unlock just these, without having to know whether one of the elements disable legacy decoding (or is trying to request it again).
But maybe somebody with a fresh new look on all this will rightfully say "fuck it, that's too complicated" and turn the whole thing into a single token to pass around.
> Your cookie validity is suspect I fear.
Yes, it is sadly.
> Also holding the device reference means you stop that exact set of > resources being reissued too early and > you (or clients) scribbling on them through unfortunate timing. So I > think you actually do need to grab references properly, Doesn't make > the > code much more complex that I can see. > Well, we are in the device addition/removal path, so the list of vga devices doesn't need to hold a reference here. However, we probably need to make sure we take one when we peek something from that list.
Cheers, Ben.
| |