Re: [PATCH v4 8/8] iommu/vt-d: Misc macro clean up for SVM

From: Joe Perches
Date: Thu Nov 21 2019 - 16:37:38 EST


On Thu, 2019-11-21 at 13:26 -0800, Jacob Pan wrote:
> Use combined macros for_each_svm_dev() to simplify SVM device iteration
> and error checking.
[]
> diff --git a/drivers/iommu/intel-svm.c b/drivers/iommu/intel-svm.c
[]
> +#define for_each_svm_dev(sdev, svm, d) \
> + list_for_each_entry((sdev), &(svm)->devs, list) \
> + if ((d) != (sdev)->dev) {} else
> +
> int intel_svm_bind_mm(struct device *dev, int *pasid, int flags, struct svm_dev_ops *ops)
> {
> struct intel_iommu *iommu = intel_svm_device_to_iommu(dev);
> @@ -274,15 +278,13 @@ int intel_svm_bind_mm(struct device *dev, int *pasid, int flags, struct svm_dev_
> goto out;
> }
>
> - list_for_each_entry(sdev, &svm->devs, list) {
> - if (dev == sdev->dev) {
> - if (sdev->ops != ops) {
> - ret = -EBUSY;
> - goto out;
> - }
> - sdev->users++;
> - goto success;
> + for_each_svm_dev(sdev, svm, dev) {
> + if (sdev->ops != ops) {
> + ret = -EBUSY;
> + goto out;
> }
> + sdev->users++;
> + goto success;
> }

I think this does not read better as this is now a
for_each loop that exits the loop on the first match.

>
> break;
> @@ -427,43 +429,36 @@ int intel_svm_unbind_mm(struct device *dev, int pasid)
> goto out;
> }
>
> - if (!svm)
> - goto out;
> -
> - list_for_each_entry(sdev, &svm->devs, list) {
[]
> + for_each_svm_dev(sdev, svm, dev) {

I think this should not remove the !svm test above.