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.
