On Mon, Feb 22, 2016 at 12:46:09PM -0800, Jesse Barnes wrote: > On 02/22/2016 05:49 AM, Alan Cox wrote: > >> we have some good alternatives in the form of bus and platform > >> drivers that > >> can manage the appropriate serialization and keep things from > >> stomping > >> on one another. > > > > It's not used much, especially nowdays. The use case is basically multi > > I/O chips on the ISA/LPC bus with magic shared config register ports. > > > > We have sufficiently few of those we could give muxed the boot and > > special case them if preferred. > > Ah that's right, now I remember the context. So where should we go from here > then? Just leave the ugly fix in or hack on old stuff and hope not to break > it?
Hi Jesse,
The fix is not ugly but only incomplete. And I have to say that the
whole IORESOURCE_MUXED thing is not shiny either :)
The main problem in __request_region() is that we are dropping the
spinlock of the resource list while holding a reference on a resource,
waiting for a muxed resource to become available.
From here, I can see two bugs:
1 - At wake-up, the next __request_resource() iteration will not detect
a remaining conflict. To work properly, __request_resource() needs
to be called with a parent of the conflicting resource. Instead we
are passing the conflicting resource itself...
2 - At wake-up the resource pointer we are holding could have been
freed. Since we are just about to use this pointer to insert a new
resource in the linked list, it is broken...
My patch fixes 1 and makes things better for 2.
But I think Linus is right. If at wake-up we use the original parent
resource to check again for a conflict, then we are safe.
If you want, I can propose a such patch.
Note that IORESOURCE_MUXED is mostly used by Super-I/Os drivers. A
Super-I/O is a legacy I/O controller embedded on x86 motherboards. It is
used to connect the low-bandwidth devices such as parallel ports, serial
ports, keyboard controllers, hardware monitoring controllers, GPIO
controllers, etc. While each logical device have its own memory region,
a shared memory region is used for some configuration purpose.
IORESOURCE_MUXED is a convenient way to deal with that. For some code
examples you can look at the superio_* functions in the IT87 drivers:
gpio/gpio-it87.c, hwmon/it87.c and watchdog/it87_wdt.c.
I am not aware of any other users for IORESOURCE_MUXED.
Let me know how you want to go and if you need my help.
Simon
signature.asc
Description: PGP signature
