Hello, On 29/11/17 06:15, Yisheng Xie wrote: > Hi Jean, > > On 2017/10/6 21:31, Jean-Philippe Brucker wrote: >> - if (domain->ext_handler) { >> + if (domain->handler_flags & IOMMU_FAULT_HANDLER_ATOMIC) { >> + fault->flags |= IOMMU_FAULT_ATOMIC; > > Why remove the condition of domain->ext_handler? should it be much better like: > if ((domain->handler_flags & IOMMU_FAULT_HANDLER_ATOMIC) && domain->ext_handler) > > If domain->ext_handler is NULL, and (domain->handler_flags & IOMMU_FAULT_HANDLER_ATOMIC) > is true. It will oops, right? I removed the check because ext_handler shouldn't be NULL if handler_flags has a bit set (as per iommu_set_ext_fault_handler). But you're right that this is fragile, and I overlooked the case where users could call set_ext_fault_handler to clear the fault handler. (Note that this ext_handler will most likely be replaced by the fault infrastructure that Jacob is working on: https://patchwork.kernel.org/patch/10063385/ to which we should add the atomic/blocking flags) Thanks, Jean -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html