On Mon, Aug 20, 2012 at 03:55:46PM +0200, Joerg Roedel wrote: > Please review.
Finally. Usually you don't add/change code but you just move common irq remapping pieces out of geric io-apic code and put them in once place. I think it would be good, if you would note this in the description of your patch. Altogether it makes a good impression. After browsing through the new functions in irq_remapping_modify_x86_ops() I see that some of them test for "remap_ops" which is pointless because you don't call irq_remapping_modify_x86_ops() if it is not the case. This also goes mostly for irq_remapping_enabled. The only reason when you can disable (or say irq_remapping_disable() is called) is in the suspend path. And the remap is enabled again in via irq_remapping_reenable() in resume. Now if this goes wrong what is next? You don't even return an error if the callback is missing. The variable irq_remapping_enabled does not save your ass here because some function behave now different. But back to the realisitic world: If something goes wrong in resume and you can't re-enable irq remapping, the system is not really useable or is it (even before your series)? > Thanks, > > Joerg > Sebastian -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/