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. -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
