lkml.org 
[lkml]   [2022]   [Nov]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH] iommu/arm-smmu-v3: Optimize checking for invalid values
From
On 2022-11-13 13:38, Harry Song wrote:
> Move the check of invalid value (iotlb_gather->pgsize = 0) from
> arm_smmu_iotlb_sync() to __arm_smmu_tlb_inv_range() for iotlb sync
> to make the code clearer.

How is this an optimisation? It adds a redundant check on paths that
don't need it, and even the unmap error paths now have to do *more* work
to figure out when there's nothing to sync. Furthermore, to me "clear
code" means making decisions at the appropriate level of abstraction,
not deliberately passing invalid arguments down through 3 more levels of
callstack to eventually do nothing.

If you think that the gather->pgsize logic is hard to follow and needs
more explanation then by all means add a comment to call out how
iommu_iotlb_gather_init() and iommu_iotlb_gather_add_page() join it up,
but I don't agree with this patch as it is, sorry.

Thanks,
Robin.

> Signed-off-by: Harry Song <jundongsong1@gmail.com>
> ---
> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 5 +----
> 1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> index 6d5df91c5..e51b9f506 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> @@ -1873,7 +1873,7 @@ static void __arm_smmu_tlb_inv_range(struct arm_smmu_cmdq_ent *cmd,
> size_t inv_range = granule;
> struct arm_smmu_cmdq_batch cmds;
>
> - if (!size)
> + if (!size || !inv_range)
> return;
>
> if (smmu->features & ARM_SMMU_FEAT_RANGE_INV) {
> @@ -2507,9 +2507,6 @@ static void arm_smmu_iotlb_sync(struct iommu_domain *domain,
> {
> struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
>
> - if (!gather->pgsize)
> - return;
> -
> arm_smmu_tlb_inv_range_domain(gather->start,
> gather->end - gather->start + 1,
> gather->pgsize, true, smmu_domain);

\
 
 \ /
  Last update: 2022-11-14 11:47    [W:0.381 / U:0.076 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site