Just some improvement suggestions. On 10/6/2017 9:31 AM, Jean-Philippe Brucker wrote: > + spin_lock(&iommu_process_lock); > + idr_for_each_entry(&iommu_process_idr, process, i) { > + if (process->pid != pid) > + continue; if you see this construct a lot, this could be a for_each_iommu_process. > + > + if (!iommu_process_get_locked(process)) { > + /* Process is defunct, create a new one */ > + process = NULL; > + break; > + } > + > + /* Great, is it also bound to this domain? */ > + list_for_each_entry(cur_context, &process->domains, > + process_head) { > + if (cur_context->domain != domain) > + continue; if you see this construct a lot, this could be a for_each_iommu_process_domain. > + > + context = cur_context; > + *pasid = process->pasid; > + > + /* Splendid, tell the driver and increase the ref */ > + err = iommu_process_attach_locked(context, dev); > + if (err) > + iommu_process_put_locked(process); > + > + break; > + } > + break; > + } > + spin_unlock(&iommu_process_lock); > + put_pid(pid); > + > + if (context) > + return err; I think you should make the section above a independent function and return here when the context is found. -- Sinan Kaya Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project. -- 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