> -----Original Message-----
> From: Heiner Kallweit [mailto:[email protected]]
> Sent: Monday, December 14, 2020 5:33 AM
> To: Christoph Hellwig <[email protected]>; Marek Szyprowski
> <[email protected]>; Robin Murphy <[email protected]>; Song Bao Hua
> (Barry Song) <[email protected]>
> Cc: open list:AMD IOMMU (AMD-VI) <[email protected]>; Linux
> Kernel Mailing List <[email protected]>
> Subject: [PATCH v2] dma-mapping: add unlikely hint for error path in
> dma_mapping_error
>
> Zillions of drivers use the unlikely() hint when checking the result of
> dma_mapping_error(). This is an inline function anyway, so we can move
> the hint into this function and remove it from drivers.
>
> Signed-off-by: Heiner Kallweit <[email protected]>
not sure if this is really necessary. It seems the original code
is more readable. Readers can more easily understand we are
predicting the branch based on the return value of
dma_mapping_error().
Anyway, I don't object to this one. if other people like it, I am
also ok with it.
> ---
> v2:
> Split the big patch into the change for dma-mapping.h and follow-up
> patches per subsystem that will go through the trees of the respective
> maintainers.
> ---
> include/linux/dma-mapping.h | 2 +-
> kernel/dma/map_benchmark.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
> index 2e49996a8..6177e20b5 100644
> --- a/include/linux/dma-mapping.h
> +++ b/include/linux/dma-mapping.h
> @@ -95,7 +95,7 @@ static inline int dma_mapping_error(struct device *dev,
> dma_addr_t dma_addr)
> {
> debug_dma_mapping_error(dev, dma_addr);
>
> - if (dma_addr == DMA_MAPPING_ERROR)
> + if (unlikely(dma_addr == DMA_MAPPING_ERROR))
> return -ENOMEM;
> return 0;
> }
> diff --git a/kernel/dma/map_benchmark.c b/kernel/dma/map_benchmark.c
> index b1496e744..901420a5d 100644
> --- a/kernel/dma/map_benchmark.c
> +++ b/kernel/dma/map_benchmark.c
> @@ -78,7 +78,7 @@ static int map_benchmark_thread(void *data)
>
> map_stime = ktime_get();
> dma_addr = dma_map_single(map->dev, buf, PAGE_SIZE, map->dir);
> - if (unlikely(dma_mapping_error(map->dev, dma_addr))) {
> + if (dma_mapping_error(map->dev, dma_addr)) {
> pr_err("dma_map_single failed on %s\n",
> dev_name(map->dev));
> ret = -ENOMEM;
> --
> 2.29.2
Thanks
Barry
