-----Original Message----- From: John Garry Sent: Wednesday, November 11, 2020 10:37 PM To: Song Bao Hua (Barry Song) song.bao.hua@hisilicon.com; iommu@lists.linux-foundation.org; hch@lst.de; robin.murphy@arm.com; m.szyprowski@samsung.com Cc: linux-kselftest@vger.kernel.org; Will Deacon will@kernel.org; Joerg Roedel joro@8bytes.org; Linuxarm linuxarm@huawei.com; xuwei (O) xuwei5@huawei.com; Shuah Khan shuah@kernel.org Subject: Re: [PATCH v3 1/2] dma-mapping: add benchmark support for streaming DMA APIs

On 11/11/2020 01:29, Song Bao Hua (Barry Song) wrote:

...

I'd like to think checking this here would be overdesign. We just give users the freedom to bind any device they care about to the benchmark driver. Usually that means a real hardware either behind an IOMMU or through a direct mapping.

if for any reason users put a wrong "device", that is the choice of users.

Right, but if the device simply has no DMA ops supported, it could be better to fail the probe rather than let them try the test at all.

Anyhow,

...

the below code will still handle it properly and users will get a report in which everything is zero.

+static int map_benchmark_thread(void *data) +{ ...

• dma_addr = dma_map_single(map->dev, buf, PAGE_SIZE,
  

DMA_BIDIRECTIONAL);

...

• if (unlikely(dma_mapping_error(map->dev, dma_addr))) {
  

Doing this is proper, but I am not sure if this tells the user the real problem.

Telling users the real problem isn't the design intention of this test benchmark. It is never the purpose of this benchmark.

...

• 	pr_err("dma_map_single failed on %s\n",
  

dev_name(map->dev));

Not sure why use pr_err() over dev_err().

We are reporting errors in dma-benchmark driver rather than reporting errors in the driver of the specific device. I think we should have "dma-benchmark" as the prefix while printing the name of the device by dev_name().

...

• 	ret = -ENOMEM;
  

• 	goto out;
  

• }
  

Thanks, John

Thanks Barry