On Wed, Sep 19, 2012 at 04:52:27PM +0200, Joerg Roedel wrote: > Hi Sebastian, Hi Joerg,
> > 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. > > Okay, I am usually a bit conservative when it comes to checking function > pointers and conditions. One reason is that the Linux kernel had severe > security bugs in the past with NULL functions pointers being called. > Especially when future changes modify the code path these function > pointers it is easy to forget re-adding the checks. But I will look > again at the patches to check which checks can be removed and which > should better stay. But if you forget to set the pointer and you simply return doing nothing it is kind of bad. So if you do a setup, make check at the beginning and then you *can* expect that pointers are set. And you can never prepare against people touching code and not thinking much. If you look at the IRQ code or x86'ops code for instance, there are a few places where function pointers are checked / set at the beginning and then are called. And sometimes the functions are simple "void / return 0" code if this is really not implemented / required but you don't have to check for it. > Well, when something goes wrong on resume and irq-remapping doesn't work > anymore the system is usually doomed (with and without this patch-set). > When irq-remapping does not work the dma-remapping will likely also not > work anymore which is even worse. understood. > > Regards, > > 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/
