> -----Original Message-----
> From: John Garry
> Sent: Wednesday, November 11, 2020 10:37 PM
> To: Song Bao Hua (Barry Song) <song.bao.hua@xxxxxxxxxxxxx>;
> iommu@xxxxxxxxxxxxxxxxxxxxxxxxxx; hch@xxxxxx; robin.murphy@xxxxxxx;
> m.szyprowski@xxxxxxxxxxx
> Cc: linux-kselftest@xxxxxxxxxxxxxxx; Will Deacon <will@xxxxxxxxxx>; Joerg
> Roedel <joro@xxxxxxxxxx>; Linuxarm <linuxarm@xxxxxxxxxx>; xuwei (O)
> <xuwei5@xxxxxxxxxx>; Shuah Khan <shuah@xxxxxxxxxx>
> 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
