hi jean, On 2017/11/29 23:01, Jean-Philippe Brucker wrote: > 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) > Get it, thanks for your explanation. Thanks Yisheng Xie > Thanks, > Jean > > . > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html