Messages in this thread | | | From | (Eric W. Biederman) | Subject | Re: [PATCH 2.6.21-rc5] MSI: read-flush MSI-X table | Date | Tue, 27 Mar 2007 19:14:37 -0600 |
| |
"Williams, Mitch A" <mitch.a.williams@intel.com> writes:
> Doh! I was reading the code wrong. We only mask if we're still > handling a previous interrupt on the same vector. My bad. > > However, I can't really see where mask() is used outside of that > instance. Which then leads us back to the question: do we need > a read flush on mask/unmask or just enable/disable?
I'm not even certain we need the read flush in the enable. However having it in there makes the code easier to reason about. Which is a big plus.
Generally if the interrupt controller hardware is sane mask/unmask and enable/disable should be the same function.
If we need to work around something in the hardware enable/disable should do that and mask/unmask should poke the hardware.
Since MSI is specified as properly handle pending interrupts I would put the write flush in mask. It makes the code easier to understand and comprehend.
The practical question in my book is do we set the enable/disable methods to the same functions as the mask/unmask methods or do we let them default to the crazy delayed disable scenario.
Given that we do have a tiny race where we need to ensure the MSI is disabled before we unregister it, we don't know of any MSI implementation problems that will result in a screaming IRQ. I would say set enable/disable to the mask/unmask methods.
This will fix the tiny freeing bug mentioned above, and not play games with drivers that are using MSI irqs.
If at some point we need a lesser form someone can change the msi enable/disable methods to something else.
Eric - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |