[PATCH 0/3] dma mapping benchmark: add support for dma_map_sg

[PATCH 0/3] dma mapping benchmark: add support for dma_map_sg

[Qinxin Xia]

Modify the framework to adapt to more map modes, add benchmark
support for dma_map_sg, and add support sg map mode in ioctl.

The result:
[root@localhost]# ./dma_map_benchmark -m 1 -g 8 -t 8 -s 30 -d 2
dma mapping mode: DMA_MAP_SG_MODE
dma mapping benchmark: threads:8 seconds:30 node:-1 dir:FROM_DEVICE granule/sg_nents: 8
average map latency(us):1.4 standard deviation:0.3
average unmap latency(us):1.3 standard deviation:0.3
[root@localhost]# ./dma_map_benchmark -m 0 -g 8 -t 8 -s 30 -d 2
dma mapping mode: DMA_MAP_SINGLE_MODE
dma mapping benchmark: threads:8 seconds:30 node:-1 dir:FROM_DEVICE granule/sg_nents: 8
average map latency(us):1.0 standard deviation:0.3
average unmap latency(us):1.3 standard deviation:0.5

---
Qinxin Xia (3):
  dma-mapping: benchmark: modify the framework to adapt to more map
    modes
  dma-mapping: benchmark: add support for dma_map_sg
  dma-mapping benchmark:add support for dma_map_sg

 include/linux/map_benchmark.h                 |   7 +
 kernel/dma/map_benchmark.c                    | 224 ++++++++++++++++--
 .../testing/selftests/dma/dma_map_benchmark.c |  16 +-
 3 files changed, 222 insertions(+), 25 deletions(-)

--
2.33.0

[PATCH 1/3] dma mapping benchmark: modify the framework to adapt to more map modes

[Qinxin Xia]

In this patch map_benchmark abstract in four interface: prepare, unprepare,
do_map, do_unmap. When there's a new mode to add, need four steps:
1) Add the mode in map_benchmark.h

2) Defines the mode param, like struct dma_xxx_map_param, and this object
   will be return in prepare and as input parameter in other ops;

3) Defines the ops functions:prepare, unprepare, do_map, do_unmap.

4) Add the new mode in dma_map_benchmark_ops.

Signed-off-by: Qinxin Xia <xiaqinxin@huawei.com>
---
 include/linux/map_benchmark.h |   6 ++
 kernel/dma/map_benchmark.c    | 122 +++++++++++++++++++++++++++-------
 2 files changed, 105 insertions(+), 23 deletions(-)

diff --git a/include/linux/map_benchmark.h b/include/linux/map_benchmark.h
index 62674c83bde4..054db02a03a7 100644
--- a/include/linux/map_benchmark.h
+++ b/include/linux/map_benchmark.h
@@ -15,6 +15,11 @@
 #define DMA_MAP_TO_DEVICE       1
 #define DMA_MAP_FROM_DEVICE     2
 
+enum {
+	DMA_MAP_SINGLE_MODE,
+	DMA_MAP_MODE_MAX
+};
+
 struct map_benchmark {
 	__u64 avg_map_100ns; /* average map latency in 100ns */
 	__u64 map_stddev; /* standard deviation of map latency */
@@ -27,5 +32,6 @@ struct map_benchmark {
 	__u32 dma_dir; /* DMA data direction */
 	__u32 dma_trans_ns; /* time for DMA transmission in ns */
 	__u32 granule;  /* how many PAGE_SIZE will do map/unmap once a time */
+	__u8  map_mode; /* the mode of dma map */
 };
 #endif /* _KERNEL_DMA_BENCHMARK_H */
diff --git a/kernel/dma/map_benchmark.c b/kernel/dma/map_benchmark.c
index cc19a3efea89..d8ec0ce058d8 100644
--- a/kernel/dma/map_benchmark.c
+++ b/kernel/dma/map_benchmark.c
@@ -5,6 +5,7 @@
 
 #define pr_fmt(fmt)	KBUILD_MODNAME ": " fmt
 
+#include <linux/cleanup.h>
 #include <linux/debugfs.h>
 #include <linux/delay.h>
 #include <linux/device.h>
@@ -31,17 +32,98 @@ struct map_benchmark_data {
 	atomic64_t loops;
 };
 
+struct map_benchmark_ops {
+	void *(*prepare)(struct map_benchmark_data *map);
+	void (*unprepare)(void *arg);
+	int (*do_map)(void *arg);
+	int (*do_unmap)(void *arg);
+};
+
+struct dma_single_map_param {
+	struct device *dev;
+	dma_addr_t addr;
+	void *xbuf;
+	u32 npages;
+	u32 dma_dir;
+};
+
+static void *dma_single_map_benchmark_prepare(struct map_benchmark_data *map)
+{
+	struct dma_single_map_param *mparam __free(kfree) = kzalloc(sizeof(*mparam),
+								    GFP_KERNEL);
+	if (!mparam)
+		return NULL;
+
+	mparam->npages = map->bparam.granule;
+	mparam->dma_dir = map->bparam.dma_dir;
+	mparam->dev = map->dev;
+	mparam->xbuf = alloc_pages_exact(mparam->npages * PAGE_SIZE, GFP_KERNEL);
+	if (!mparam->xbuf)
+		return NULL;
+
+	/*
+	 * for a non-coherent device, if we don't stain them in the
+	 * cache, this will give an underestimate of the real-world
+	 * overhead of BIDIRECTIONAL or TO_DEVICE mappings;
+	 * 66 means evertything goes well! 66 is lucky.
+	 */
+	if (mparam->dma_dir != DMA_FROM_DEVICE)
+		memset(mparam->xbuf, 0x66, mparam->npages * PAGE_SIZE);
+
+	return_ptr(mparam);
+}
+
+static void dma_single_map_benchmark_unprepare(void *arg)
+{
+	struct dma_single_map_param *mparam = arg;
+
+	free_pages_exact(mparam->xbuf, mparam->npages * PAGE_SIZE);
+	kfree(mparam);
+}
+
+static int dma_single_map_benchmark_do_map(void *arg)
+{
+	struct dma_single_map_param *mparam = arg;
+
+	mparam->addr = dma_map_single(mparam->dev, mparam->xbuf,
+				      mparam->npages * PAGE_SIZE, mparam->dma_dir);
+	if (unlikely(dma_mapping_error(mparam->dev, mparam->addr))) {
+		pr_err("dma_map_single failed on %s\n", dev_name(mparam->dev));
+		return -ENOMEM;
+	}
+
+	return 0;
+}
+
+static int dma_single_map_benchmark_do_unmap(void *arg)
+{
+	struct dma_single_map_param *mparam = arg;
+
+	dma_unmap_single(mparam->dev, mparam->addr,
+			 mparam->npages * PAGE_SIZE, mparam->dma_dir);
+	return 0;
+}
+
+static struct map_benchmark_ops dma_single_map_benchmark_ops = {
+	.prepare = dma_single_map_benchmark_prepare,
+	.unprepare = dma_single_map_benchmark_unprepare,
+	.do_map = dma_single_map_benchmark_do_map,
+	.do_unmap = dma_single_map_benchmark_do_unmap,
+};
+
+static struct map_benchmark_ops *dma_map_benchmark_ops[DMA_MAP_MODE_MAX] = {
+	[DMA_MAP_SINGLE_MODE] = &dma_single_map_benchmark_ops,
+};
+
 static int map_benchmark_thread(void *data)
 {
-	void *buf;
-	dma_addr_t dma_addr;
 	struct map_benchmark_data *map = data;
-	int npages = map->bparam.granule;
-	u64 size = npages * PAGE_SIZE;
+	__u8 map_mode = map->bparam.map_mode;
 	int ret = 0;
 
-	buf = alloc_pages_exact(size, GFP_KERNEL);
-	if (!buf)
+	void *arg = dma_map_benchmark_ops[map_mode]->prepare(map);
+
+	if (!arg)
 		return -ENOMEM;
 
 	while (!kthread_should_stop())  {
@@ -49,23 +131,10 @@ static int map_benchmark_thread(void *data)
 		ktime_t map_stime, map_etime, unmap_stime, unmap_etime;
 		ktime_t map_delta, unmap_delta;
 
-		/*
-		 * for a non-coherent device, if we don't stain them in the
-		 * cache, this will give an underestimate of the real-world
-		 * overhead of BIDIRECTIONAL or TO_DEVICE mappings;
-		 * 66 means evertything goes well! 66 is lucky.
-		 */
-		if (map->dir != DMA_FROM_DEVICE)
-			memset(buf, 0x66, size);
-
 		map_stime = ktime_get();
-		dma_addr = dma_map_single(map->dev, buf, size, map->dir);
-		if (unlikely(dma_mapping_error(map->dev, dma_addr))) {
-			pr_err("dma_map_single failed on %s\n",
-				dev_name(map->dev));
-			ret = -ENOMEM;
+		ret = dma_map_benchmark_ops[map_mode]->do_map(arg);
+		if (ret)
 			goto out;
-		}
 		map_etime = ktime_get();
 		map_delta = ktime_sub(map_etime, map_stime);
 
@@ -73,7 +142,9 @@ static int map_benchmark_thread(void *data)
 		ndelay(map->bparam.dma_trans_ns);
 
 		unmap_stime = ktime_get();
-		dma_unmap_single(map->dev, dma_addr, size, map->dir);
+		ret = dma_map_benchmark_ops[map_mode]->do_unmap(arg);
+		if (ret)
+			goto out;
 		unmap_etime = ktime_get();
 		unmap_delta = ktime_sub(unmap_etime, unmap_stime);
 
@@ -108,7 +179,7 @@ static int map_benchmark_thread(void *data)
 	}
 
 out:
-	free_pages_exact(buf, size);
+	dma_map_benchmark_ops[map_mode]->unprepare(arg);
 	return ret;
 }
 
@@ -209,6 +280,11 @@ static long map_benchmark_ioctl(struct file *file, unsigned int cmd,
 
 	switch (cmd) {
 	case DMA_MAP_BENCHMARK:
+		if (map->bparam.map_mode >= DMA_MAP_MODE_MAX) {
+			pr_err("invalid map mode\n");
+			return -EINVAL;
+		}
+
 		if (map->bparam.threads == 0 ||
 		    map->bparam.threads > DMA_MAP_MAX_THREADS) {
 			pr_err("invalid thread number\n");
-- 
2.33.0

[PATCH 2/3] dma-mapping: benchmark: add support for dma_map_sg

[Qinxin Xia]

Support for dma scatter-gather mapping and is intended for testing
mapping performance. It achieves by introducing the dma_sg_map_param
structure and related functions, which enable the implementation of
scatter-gather mapping preparation, mapping, and unmapping operations.
Additionally, the dma_map_benchmark_ops array is updated to include
operations for scatter-gather mapping. This commit aims to provide
a wider range of mapping performance test  to cater to different scenarios.

Signed-off-by: Qinxin Xia <xiaqinxin@huawei.com>
---
 include/linux/map_benchmark.h |   1 +
 kernel/dma/map_benchmark.c    | 102 ++++++++++++++++++++++++++++++++++
 2 files changed, 103 insertions(+)

diff --git a/include/linux/map_benchmark.h b/include/linux/map_benchmark.h
index 054db02a03a7..a9c1a104ba4f 100644
--- a/include/linux/map_benchmark.h
+++ b/include/linux/map_benchmark.h
@@ -17,6 +17,7 @@
 
 enum {
 	DMA_MAP_SINGLE_MODE,
+	DMA_MAP_SG_MODE,
 	DMA_MAP_MODE_MAX
 };
 
diff --git a/kernel/dma/map_benchmark.c b/kernel/dma/map_benchmark.c
index d8ec0ce058d8..b5828eeb3db7 100644
--- a/kernel/dma/map_benchmark.c
+++ b/kernel/dma/map_benchmark.c
@@ -17,6 +17,7 @@
 #include <linux/module.h>
 #include <linux/pci.h>
 #include <linux/platform_device.h>
+#include <linux/scatterlist.h>
 #include <linux/slab.h>
 #include <linux/timekeeping.h>
 
@@ -111,8 +112,109 @@ static struct map_benchmark_ops dma_single_map_benchmark_ops = {
 	.do_unmap = dma_single_map_benchmark_do_unmap,
 };
 
+struct dma_sg_map_param {
+	struct sg_table sgt;
+	struct device *dev;
+	void **buf;
+	u32 npages;
+	u32 dma_dir;
+};
+
+static void *dma_sg_map_benchmark_prepare(struct map_benchmark_data *map)
+{
+	struct scatterlist *sg;
+	int i = 0;
+
+	struct dma_sg_map_param *mparam __free(kfree) = kzalloc(sizeof(*mparam), GFP_KERNEL);
+	if (!mparam)
+		return NULL;
+
+	mparam->npages = map->bparam.granule;
+	mparam->dma_dir = map->bparam.dma_dir;
+	mparam->dev = map->dev;
+	mparam->buf = kmalloc_array(mparam->npages, sizeof(*mparam->buf),
+				    GFP_KERNEL);
+	if (!mparam->buf)
+		goto err1;
+
+	if (sg_alloc_table(&mparam->sgt, mparam->npages, GFP_KERNEL))
+		goto err2;
+
+	for_each_sgtable_sg(&mparam->sgt, sg, i) {
+		mparam->buf[i] = (void *)__get_free_page(GFP_KERNEL);
+		if (!mparam->buf[i])
+			goto err3;
+
+		if (mparam->dma_dir != DMA_FROM_DEVICE)
+			memset(mparam->buf[i], 0x66, PAGE_SIZE);
+
+		sg_set_buf(sg, mparam->buf[i], PAGE_SIZE);
+	}
+
+	return_ptr(mparam);
+
+err3:
+	while (i-- > 0)
+		free_page((unsigned long)mparam->buf[i]);
+
+	pr_err("dma_map_sg failed get free page on %s\n", dev_name(mparam->dev));
+	sg_free_table(&mparam->sgt);
+err2:
+	pr_err("dma_map_sg failed alloc sg table on %s\n", dev_name(mparam->dev));
+	kfree(mparam->buf);
+err1:
+	pr_err("dma_map_sg failed alloc mparam buf on %s\n", dev_name(mparam->dev));
+	return NULL;
+}
+
+static void dma_sg_map_benchmark_unprepare(void *arg)
+{
+	struct dma_sg_map_param *mparam = arg;
+	int i;
+
+	for (i = 0; i < mparam->npages; i++)
+		free_page((unsigned long)mparam->buf[i]);
+
+	sg_free_table(&mparam->sgt);
+
+	kfree(mparam->buf);
+	kfree(mparam);
+}
+
+static int dma_sg_map_benchmark_do_map(void *arg)
+{
+	struct dma_sg_map_param *mparam = arg;
+
+	int sg_mapped = dma_map_sg(mparam->dev, mparam->sgt.sgl,
+				   mparam->npages, mparam->dma_dir);
+	if (!sg_mapped) {
+		pr_err("dma_map_sg failed on %s\n", dev_name(mparam->dev));
+		return -ENOMEM;
+	}
+
+	return 0;
+}
+
+static int dma_sg_map_benchmark_do_unmap(void *arg)
+{
+	struct dma_sg_map_param *mparam = arg;
+
+	dma_unmap_sg(mparam->dev, mparam->sgt.sgl, mparam->npages,
+		     mparam->dma_dir);
+
+	return 0;
+}
+
+static struct map_benchmark_ops dma_sg_map_benchmark_ops = {
+	.prepare = dma_sg_map_benchmark_prepare,
+	.unprepare = dma_sg_map_benchmark_unprepare,
+	.do_map = dma_sg_map_benchmark_do_map,
+	.do_unmap = dma_sg_map_benchmark_do_unmap,
+};
+
 static struct map_benchmark_ops *dma_map_benchmark_ops[DMA_MAP_MODE_MAX] = {
 	[DMA_MAP_SINGLE_MODE] = &dma_single_map_benchmark_ops,
+	[DMA_MAP_SG_MODE] = &dma_sg_map_benchmark_ops,
 };
 
 static int map_benchmark_thread(void *data)
-- 
2.33.0

[PATCH 3/3] dma mapping benchmark:add support for dma_map_sg

[Qinxin Xia]

Support for dma_map_sg, add option '-m' to distinguish mode.

i) Users can set option '-m' to select mode:
   DMA_MAP_SINGLE_MODE=0, DMA_MAP_SG_MODE:=1
   (The mode is also show in the test result).
ii) Users can set option '-g' to set sg_nents
    (total count of entries in scatterlist)
    the maximum number is 1024. Each of sg buf size is PAGE_SIZE.
    e.g
    [root@localhost]# ./dma_map_benchmark -m 1 -g 8 -t 8 -s 30 -d 2
    dma mapping mode: DMA_MAP_SG_MODE
    dma mapping benchmark: threads:8 seconds:30 node:-1
    dir:FROM_DEVICE granule/sg_nents: 8
    average map latency(us):1.4 standard deviation:0.3
    average unmap latency(us):1.3 standard deviation:0.3
    [root@localhost]# ./dma_map_benchmark -m 0 -g 8 -t 8 -s 30 -d 2
    dma mapping mode: DMA_MAP_SINGLE_MODE
    dma mapping benchmark: threads:8 seconds:30 node:-1
    dir:FROM_DEVICE granule/sg_nents: 8
    average map latency(us):1.0 standard deviation:0.3
    average unmap latency(us):1.3 standard deviation:0.5

Signed-off-by: Qinxin Xia <xiaqinxin@huawei.com>
---
 tools/testing/selftests/dma/dma_map_benchmark.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/dma/dma_map_benchmark.c b/tools/testing/selftests/dma/dma_map_benchmark.c
index b12f1f9babf8..036ddb5ac862 100644
--- a/tools/testing/selftests/dma/dma_map_benchmark.c
+++ b/tools/testing/selftests/dma/dma_map_benchmark.c
@@ -27,6 +27,7 @@ int main(int argc, char **argv)
 	int fd, opt;
 	/* default single thread, run 20 seconds on NUMA_NO_NODE */
 	int threads = 1, seconds = 20, node = -1;
+	int map_mode = DMA_MAP_SINGLE_MODE;
 	/* default dma mask 32bit, bidirectional DMA */
 	int bits = 32, xdelay = 0, dir = DMA_MAP_BIDIRECTIONAL;
 	/* default granule 1 PAGESIZE */
@@ -34,7 +35,7 @@ int main(int argc, char **argv)
 
 	int cmd = DMA_MAP_BENCHMARK;
 
-	while ((opt = getopt(argc, argv, "t:s:n:b:d:x:g:")) != -1) {
+	while ((opt = getopt(argc, argv, "t:s:n:b:d:x:g:m:")) != -1) {
 		switch (opt) {
 		case 't':
 			threads = atoi(optarg);
@@ -57,11 +58,20 @@ int main(int argc, char **argv)
 		case 'g':
 			granule = atoi(optarg);
 			break;
+		case 'm':
+			map_mode = atoi(optarg);
+			break;
 		default:
 			return -1;
 		}
 	}
 
+	if (map_mode >= DMA_MAP_MODE_MAX) {
+		fprintf(stderr, "invalid map mode, DMA_MAP_SINGLE_MODE:%d, DMA_MAP_SG_MODE:%d\n",
+			DMA_MAP_SINGLE_MODE, DMA_MAP_SG_MODE);
+		exit(1);
+	}
+
 	if (threads <= 0 || threads > DMA_MAP_MAX_THREADS) {
 		fprintf(stderr, "invalid number of threads, must be in 1-%d\n",
 			DMA_MAP_MAX_THREADS);
@@ -111,13 +121,15 @@ int main(int argc, char **argv)
 	map.dma_dir = dir;
 	map.dma_trans_ns = xdelay;
 	map.granule = granule;
+	map.map_mode = map_mode;
 
 	if (ioctl(fd, cmd, &map)) {
 		perror("ioctl");
 		exit(1);
 	}
 
-	printf("dma mapping benchmark: threads:%d seconds:%d node:%d dir:%s granule: %d\n",
+	printf("dma mapping mode: %d\n", map_mode);
+	printf("dma mapping benchmark: threads:%d seconds:%d node:%d dir:%s granule/sg_nents: %d\n",
 			threads, seconds, node, dir[directions], granule);
 	printf("average map latency(us):%.1f standard deviation:%.1f\n",
 			map.avg_map_100ns/10.0, map.map_stddev/10.0);
-- 
2.33.0

Re: [PATCH 2/3] dma-mapping: benchmark: add support for dma_map_sg

[Barry Song]

On Wed, Feb 12, 2025 at 3:27 PM Qinxin Xia <xiaqinxin@huawei.com> wrote:
>
> Support for dma scatter-gather mapping and is intended for testing
> mapping performance. It achieves by introducing the dma_sg_map_param
> structure and related functions, which enable the implementation of
> scatter-gather mapping preparation, mapping, and unmapping operations.
> Additionally, the dma_map_benchmark_ops array is updated to include
> operations for scatter-gather mapping. This commit aims to provide
> a wider range of mapping performance test  to cater to different scenarios.

This benchmark is mainly designed to debug contention issues, such as IOMMU
TLB flushes or IOMMU driver bottlenecks. I don't fully understand how SG or
single will impact the evaluation of the IOMMU driver, making it unclear if the
added complexity is justified.

Can you add some explanation on why single mode is not sufficient for profiling
and improving IOMMU drivers?

>
> Signed-off-by: Qinxin Xia <xiaqinxin@huawei.com>
> ---
>  include/linux/map_benchmark.h |   1 +
>  kernel/dma/map_benchmark.c    | 102 ++++++++++++++++++++++++++++++++++
>  2 files changed, 103 insertions(+)
>
> diff --git a/include/linux/map_benchmark.h b/include/linux/map_benchmark.h
> index 054db02a03a7..a9c1a104ba4f 100644
> --- a/include/linux/map_benchmark.h
> +++ b/include/linux/map_benchmark.h
> @@ -17,6 +17,7 @@
>
>  enum {
>         DMA_MAP_SINGLE_MODE,
> +       DMA_MAP_SG_MODE,
>         DMA_MAP_MODE_MAX
>  };
>
> diff --git a/kernel/dma/map_benchmark.c b/kernel/dma/map_benchmark.c
> index d8ec0ce058d8..b5828eeb3db7 100644
> --- a/kernel/dma/map_benchmark.c
> +++ b/kernel/dma/map_benchmark.c
> @@ -17,6 +17,7 @@
>  #include <linux/module.h>
>  #include <linux/pci.h>
>  #include <linux/platform_device.h>
> +#include <linux/scatterlist.h>
>  #include <linux/slab.h>
>  #include <linux/timekeeping.h>
>
> @@ -111,8 +112,109 @@ static struct map_benchmark_ops dma_single_map_benchmark_ops = {
>         .do_unmap = dma_single_map_benchmark_do_unmap,
>  };
>
> +struct dma_sg_map_param {
> +       struct sg_table sgt;
> +       struct device *dev;
> +       void **buf;
> +       u32 npages;
> +       u32 dma_dir;
> +};
> +
> +static void *dma_sg_map_benchmark_prepare(struct map_benchmark_data *map)
> +{
> +       struct scatterlist *sg;
> +       int i = 0;
> +
> +       struct dma_sg_map_param *mparam __free(kfree) = kzalloc(sizeof(*mparam), GFP_KERNEL);
> +       if (!mparam)
> +               return NULL;
> +
> +       mparam->npages = map->bparam.granule;
> +       mparam->dma_dir = map->bparam.dma_dir;
> +       mparam->dev = map->dev;
> +       mparam->buf = kmalloc_array(mparam->npages, sizeof(*mparam->buf),
> +                                   GFP_KERNEL);
> +       if (!mparam->buf)
> +               goto err1;
> +
> +       if (sg_alloc_table(&mparam->sgt, mparam->npages, GFP_KERNEL))
> +               goto err2;
> +
> +       for_each_sgtable_sg(&mparam->sgt, sg, i) {
> +               mparam->buf[i] = (void *)__get_free_page(GFP_KERNEL);
> +               if (!mparam->buf[i])
> +                       goto err3;
> +
> +               if (mparam->dma_dir != DMA_FROM_DEVICE)
> +                       memset(mparam->buf[i], 0x66, PAGE_SIZE);
> +
> +               sg_set_buf(sg, mparam->buf[i], PAGE_SIZE);
> +       }
> +
> +       return_ptr(mparam);
> +
> +err3:
> +       while (i-- > 0)
> +               free_page((unsigned long)mparam->buf[i]);
> +
> +       pr_err("dma_map_sg failed get free page on %s\n", dev_name(mparam->dev));
> +       sg_free_table(&mparam->sgt);
> +err2:
> +       pr_err("dma_map_sg failed alloc sg table on %s\n", dev_name(mparam->dev));
> +       kfree(mparam->buf);
> +err1:
> +       pr_err("dma_map_sg failed alloc mparam buf on %s\n", dev_name(mparam->dev));
> +       return NULL;
> +}
> +
> +static void dma_sg_map_benchmark_unprepare(void *arg)
> +{
> +       struct dma_sg_map_param *mparam = arg;
> +       int i;
> +
> +       for (i = 0; i < mparam->npages; i++)
> +               free_page((unsigned long)mparam->buf[i]);
> +
> +       sg_free_table(&mparam->sgt);
> +
> +       kfree(mparam->buf);
> +       kfree(mparam);
> +}
> +
> +static int dma_sg_map_benchmark_do_map(void *arg)
> +{
> +       struct dma_sg_map_param *mparam = arg;
> +
> +       int sg_mapped = dma_map_sg(mparam->dev, mparam->sgt.sgl,
> +                                  mparam->npages, mparam->dma_dir);
> +       if (!sg_mapped) {
> +               pr_err("dma_map_sg failed on %s\n", dev_name(mparam->dev));
> +               return -ENOMEM;
> +       }
> +
> +       return 0;
> +}
> +
> +static int dma_sg_map_benchmark_do_unmap(void *arg)
> +{
> +       struct dma_sg_map_param *mparam = arg;
> +
> +       dma_unmap_sg(mparam->dev, mparam->sgt.sgl, mparam->npages,
> +                    mparam->dma_dir);
> +
> +       return 0;
> +}
> +
> +static struct map_benchmark_ops dma_sg_map_benchmark_ops = {
> +       .prepare = dma_sg_map_benchmark_prepare,
> +       .unprepare = dma_sg_map_benchmark_unprepare,
> +       .do_map = dma_sg_map_benchmark_do_map,
> +       .do_unmap = dma_sg_map_benchmark_do_unmap,
> +};
> +
>  static struct map_benchmark_ops *dma_map_benchmark_ops[DMA_MAP_MODE_MAX] = {
>         [DMA_MAP_SINGLE_MODE] = &dma_single_map_benchmark_ops,
> +       [DMA_MAP_SG_MODE] = &dma_sg_map_benchmark_ops,
>  };
>
>  static int map_benchmark_thread(void *data)
> --
> 2.33.0
>

Thanks
Barry

回复: [PATCH 2/3] dma-mapping: benchmark: add support for dma_map_sg

[xiaqinxin]

-----邮件原件-----
发件人: Barry Song <21cnbao@gmail.com> 
发送时间: 2025年2月18日 4:59
收件人: xiaqinxin <xiaqinxin@huawei.com>
抄送: chenxiang66@hisilicon.com; yangyicong <yangyicong@huawei.com>; hch@lst.de; iommu@lists.linux.dev; Jonathan Cameron <jonathan.cameron@huawei.com>; Zengtao (B) <prime.zeng@hisilicon.com>; fanghao (A) <fanghao11@huawei.com>; linux-kernel@vger.kernel.org
主题: Re: [PATCH 2/3] dma-mapping: benchmark: add support for dma_map_sg

On Wed, Feb 12, 2025 at 3:27 PM Qinxin Xia <xiaqinxin@huawei.com> wrote:
>
> Support for dma scatter-gather mapping and is intended for testing 
> mapping performance. It achieves by introducing the dma_sg_map_param 
> structure and related functions, which enable the implementation of 
> scatter-gather mapping preparation, mapping, and unmapping operations.
> Additionally, the dma_map_benchmark_ops array is updated to include 
> operations for scatter-gather mapping. This commit aims to provide a 
> wider range of mapping performance test  to cater to different scenarios.

This benchmark is mainly designed to debug contention issues, such as IOMMU TLB flushes or IOMMU driver bottlenecks. I don't fully understand how SG or single will impact the evaluation of the IOMMU driver, making it unclear if the added complexity is justified.

Can you add some explanation on why single mode is not sufficient for profiling and improving IOMMU drivers?

Hello Barry ! 😊
Currently, the HiSilicon accelerator service uses the dma_map_sg interface. We want to evaluate the performance of the entire DMA map process. (including not only the iommu, but also the map framework). In addition, for scatterlist, __iommu_map is executed for each nent. This increases the complexity and time overhead of mapping. The effect of this fragmentation is not obvious in dma_map_single, which only handles a single contiguous block of memory.

>
> Signed-off-by: Qinxin Xia <xiaqinxin@huawei.com>
> ---
>  include/linux/map_benchmark.h |   1 +
>  kernel/dma/map_benchmark.c    | 102 ++++++++++++++++++++++++++++++++++
>  2 files changed, 103 insertions(+)
>
> diff --git a/include/linux/map_benchmark.h 
> b/include/linux/map_benchmark.h index 054db02a03a7..a9c1a104ba4f 
> 100644
> --- a/include/linux/map_benchmark.h
> +++ b/include/linux/map_benchmark.h
> @@ -17,6 +17,7 @@
>
>  enum {
>         DMA_MAP_SINGLE_MODE,
> +       DMA_MAP_SG_MODE,
>         DMA_MAP_MODE_MAX
>  };
>
> diff --git a/kernel/dma/map_benchmark.c b/kernel/dma/map_benchmark.c 
> index d8ec0ce058d8..b5828eeb3db7 100644
> --- a/kernel/dma/map_benchmark.c
> +++ b/kernel/dma/map_benchmark.c
> @@ -17,6 +17,7 @@
>  #include <linux/module.h>
>  #include <linux/pci.h>
>  #include <linux/platform_device.h>
> +#include <linux/scatterlist.h>
>  #include <linux/slab.h>
>  #include <linux/timekeeping.h>
>
> @@ -111,8 +112,109 @@ static struct map_benchmark_ops dma_single_map_benchmark_ops = {
>         .do_unmap = dma_single_map_benchmark_do_unmap,
>  };
>
> +struct dma_sg_map_param {
> +       struct sg_table sgt;
> +       struct device *dev;
> +       void **buf;
> +       u32 npages;
> +       u32 dma_dir;
> +};
> +
> +static void *dma_sg_map_benchmark_prepare(struct map_benchmark_data 
> +*map) {
> +       struct scatterlist *sg;
> +       int i = 0;
> +
> +       struct dma_sg_map_param *mparam __free(kfree) = kzalloc(sizeof(*mparam), GFP_KERNEL);
> +       if (!mparam)
> +               return NULL;
> +
> +       mparam->npages = map->bparam.granule;
> +       mparam->dma_dir = map->bparam.dma_dir;
> +       mparam->dev = map->dev;
> +       mparam->buf = kmalloc_array(mparam->npages, sizeof(*mparam->buf),
> +                                   GFP_KERNEL);
> +       if (!mparam->buf)
> +               goto err1;
> +
> +       if (sg_alloc_table(&mparam->sgt, mparam->npages, GFP_KERNEL))
> +               goto err2;
> +
> +       for_each_sgtable_sg(&mparam->sgt, sg, i) {
> +               mparam->buf[i] = (void *)__get_free_page(GFP_KERNEL);
> +               if (!mparam->buf[i])
> +                       goto err3;
> +
> +               if (mparam->dma_dir != DMA_FROM_DEVICE)
> +                       memset(mparam->buf[i], 0x66, PAGE_SIZE);
> +
> +               sg_set_buf(sg, mparam->buf[i], PAGE_SIZE);
> +       }
> +
> +       return_ptr(mparam);
> +
> +err3:
> +       while (i-- > 0)
> +               free_page((unsigned long)mparam->buf[i]);
> +
> +       pr_err("dma_map_sg failed get free page on %s\n", dev_name(mparam->dev));
> +       sg_free_table(&mparam->sgt);
> +err2:
> +       pr_err("dma_map_sg failed alloc sg table on %s\n", dev_name(mparam->dev));
> +       kfree(mparam->buf);
> +err1:
> +       pr_err("dma_map_sg failed alloc mparam buf on %s\n", dev_name(mparam->dev));
> +       return NULL;
> +}
> +
> +static void dma_sg_map_benchmark_unprepare(void *arg) {
> +       struct dma_sg_map_param *mparam = arg;
> +       int i;
> +
> +       for (i = 0; i < mparam->npages; i++)
> +               free_page((unsigned long)mparam->buf[i]);
> +
> +       sg_free_table(&mparam->sgt);
> +
> +       kfree(mparam->buf);
> +       kfree(mparam);
> +}
> +
> +static int dma_sg_map_benchmark_do_map(void *arg) {
> +       struct dma_sg_map_param *mparam = arg;
> +
> +       int sg_mapped = dma_map_sg(mparam->dev, mparam->sgt.sgl,
> +                                  mparam->npages, mparam->dma_dir);
> +       if (!sg_mapped) {
> +               pr_err("dma_map_sg failed on %s\n", dev_name(mparam->dev));
> +               return -ENOMEM;
> +       }
> +
> +       return 0;
> +}
> +
> +static int dma_sg_map_benchmark_do_unmap(void *arg) {
> +       struct dma_sg_map_param *mparam = arg;
> +
> +       dma_unmap_sg(mparam->dev, mparam->sgt.sgl, mparam->npages,
> +                    mparam->dma_dir);
> +
> +       return 0;
> +}
> +
> +static struct map_benchmark_ops dma_sg_map_benchmark_ops = {
> +       .prepare = dma_sg_map_benchmark_prepare,
> +       .unprepare = dma_sg_map_benchmark_unprepare,
> +       .do_map = dma_sg_map_benchmark_do_map,
> +       .do_unmap = dma_sg_map_benchmark_do_unmap, };
> +
>  static struct map_benchmark_ops *dma_map_benchmark_ops[DMA_MAP_MODE_MAX] = {
>         [DMA_MAP_SINGLE_MODE] = &dma_single_map_benchmark_ops,
> +       [DMA_MAP_SG_MODE] = &dma_sg_map_benchmark_ops,
>  };
>
>  static int map_benchmark_thread(void *data)
> --
> 2.33.0
>

Thanks
Barry

Re: [PATCH 2/3] dma-mapping: benchmark: add support for dma_map_sg

[Barry Song]

On Fri, Feb 21, 2025 at 4:16 PM xiaqinxin <xiaqinxin@huawei.com> wrote:
>
>
>
> -----邮件原件-----
> 发件人: Barry Song <21cnbao@gmail.com>
> 发送时间: 2025年2月18日 4:59
> 收件人: xiaqinxin <xiaqinxin@huawei.com>
> 抄送: chenxiang66@hisilicon.com; yangyicong <yangyicong@huawei.com>; hch@lst.de; iommu@lists.linux.dev; Jonathan Cameron <jonathan.cameron@huawei.com>; Zengtao (B) <prime.zeng@hisilicon.com>; fanghao (A) <fanghao11@huawei.com>; linux-kernel@vger.kernel.org
> 主题: Re: [PATCH 2/3] dma-mapping: benchmark: add support for dma_map_sg
>
> On Wed, Feb 12, 2025 at 3:27 PM Qinxin Xia <xiaqinxin@huawei.com> wrote:
> >
> > Support for dma scatter-gather mapping and is intended for testing
> > mapping performance. It achieves by introducing the dma_sg_map_param
> > structure and related functions, which enable the implementation of
> > scatter-gather mapping preparation, mapping, and unmapping operations.
> > Additionally, the dma_map_benchmark_ops array is updated to include
> > operations for scatter-gather mapping. This commit aims to provide a
> > wider range of mapping performance test  to cater to different scenarios.
>
> This benchmark is mainly designed to debug contention issues, such as IOMMU TLB flushes or IOMMU driver bottlenecks. I don't fully understand how SG or single will impact the evaluation of the IOMMU driver, making it unclear if the added complexity is justified.
>
> Can you add some explanation on why single mode is not sufficient for profiling and improving IOMMU drivers?
>
> Hello Barry ! 😊
> Currently, the HiSilicon accelerator service uses the dma_map_sg interface. We want to evaluate the performance of the entire DMA map process. (including not only the iommu, but also the map framework). In addition, for scatterlist, __iommu_map is executed for each nent. This increases the complexity and time overhead of mapping. The effect of this fragmentation is not obvious in dma_map_single, which only handles a single contiguous block of memory.
>

Thanks!
Please update your editor to ensure it doesn't respond with such long sentences
without line breaks in the future :-)

Can you provide concrete examples or data showing how the newly added mode
improves profiling of the entire DMA map process? For instance, what limitations
exist without this mode? What performance issues cannot be identified without
it?

> >
> > Signed-off-by: Qinxin Xia <xiaqinxin@huawei.com>
> > ---
> >  include/linux/map_benchmark.h |   1 +
> >  kernel/dma/map_benchmark.c    | 102 ++++++++++++++++++++++++++++++++++
> >  2 files changed, 103 insertions(+)
> >
> > diff --git a/include/linux/map_benchmark.h
> > b/include/linux/map_benchmark.h index 054db02a03a7..a9c1a104ba4f
> > 100644
> > --- a/include/linux/map_benchmark.h
> > +++ b/include/linux/map_benchmark.h
> > @@ -17,6 +17,7 @@
> >
> >  enum {
> >         DMA_MAP_SINGLE_MODE,
> > +       DMA_MAP_SG_MODE,
> >         DMA_MAP_MODE_MAX
> >  };
> >
> > diff --git a/kernel/dma/map_benchmark.c b/kernel/dma/map_benchmark.c
> > index d8ec0ce058d8..b5828eeb3db7 100644
> > --- a/kernel/dma/map_benchmark.c
> > +++ b/kernel/dma/map_benchmark.c
> > @@ -17,6 +17,7 @@
> >  #include <linux/module.h>
> >  #include <linux/pci.h>
> >  #include <linux/platform_device.h>
> > +#include <linux/scatterlist.h>
> >  #include <linux/slab.h>
> >  #include <linux/timekeeping.h>
> >
> > @@ -111,8 +112,109 @@ static struct map_benchmark_ops dma_single_map_benchmark_ops = {
> >         .do_unmap = dma_single_map_benchmark_do_unmap,
> >  };
> >
> > +struct dma_sg_map_param {
> > +       struct sg_table sgt;
> > +       struct device *dev;
> > +       void **buf;
> > +       u32 npages;
> > +       u32 dma_dir;
> > +};
> > +
> > +static void *dma_sg_map_benchmark_prepare(struct map_benchmark_data
> > +*map) {
> > +       struct scatterlist *sg;
> > +       int i = 0;
> > +
> > +       struct dma_sg_map_param *mparam __free(kfree) = kzalloc(sizeof(*mparam), GFP_KERNEL);
> > +       if (!mparam)
> > +               return NULL;
> > +
> > +       mparam->npages = map->bparam.granule;
> > +       mparam->dma_dir = map->bparam.dma_dir;
> > +       mparam->dev = map->dev;
> > +       mparam->buf = kmalloc_array(mparam->npages, sizeof(*mparam->buf),
> > +                                   GFP_KERNEL);
> > +       if (!mparam->buf)
> > +               goto err1;
> > +
> > +       if (sg_alloc_table(&mparam->sgt, mparam->npages, GFP_KERNEL))
> > +               goto err2;
> > +
> > +       for_each_sgtable_sg(&mparam->sgt, sg, i) {
> > +               mparam->buf[i] = (void *)__get_free_page(GFP_KERNEL);
> > +               if (!mparam->buf[i])
> > +                       goto err3;
> > +
> > +               if (mparam->dma_dir != DMA_FROM_DEVICE)
> > +                       memset(mparam->buf[i], 0x66, PAGE_SIZE);
> > +
> > +               sg_set_buf(sg, mparam->buf[i], PAGE_SIZE);
> > +       }
> > +
> > +       return_ptr(mparam);
> > +
> > +err3:
> > +       while (i-- > 0)
> > +               free_page((unsigned long)mparam->buf[i]);
> > +
> > +       pr_err("dma_map_sg failed get free page on %s\n", dev_name(mparam->dev));
> > +       sg_free_table(&mparam->sgt);
> > +err2:
> > +       pr_err("dma_map_sg failed alloc sg table on %s\n", dev_name(mparam->dev));
> > +       kfree(mparam->buf);
> > +err1:
> > +       pr_err("dma_map_sg failed alloc mparam buf on %s\n", dev_name(mparam->dev));
> > +       return NULL;
> > +}
> > +
> > +static void dma_sg_map_benchmark_unprepare(void *arg) {
> > +       struct dma_sg_map_param *mparam = arg;
> > +       int i;
> > +
> > +       for (i = 0; i < mparam->npages; i++)
> > +               free_page((unsigned long)mparam->buf[i]);
> > +
> > +       sg_free_table(&mparam->sgt);
> > +
> > +       kfree(mparam->buf);
> > +       kfree(mparam);
> > +}
> > +
> > +static int dma_sg_map_benchmark_do_map(void *arg) {
> > +       struct dma_sg_map_param *mparam = arg;
> > +
> > +       int sg_mapped = dma_map_sg(mparam->dev, mparam->sgt.sgl,
> > +                                  mparam->npages, mparam->dma_dir);
> > +       if (!sg_mapped) {
> > +               pr_err("dma_map_sg failed on %s\n", dev_name(mparam->dev));
> > +               return -ENOMEM;
> > +       }
> > +
> > +       return 0;
> > +}
> > +
> > +static int dma_sg_map_benchmark_do_unmap(void *arg) {
> > +       struct dma_sg_map_param *mparam = arg;
> > +
> > +       dma_unmap_sg(mparam->dev, mparam->sgt.sgl, mparam->npages,
> > +                    mparam->dma_dir);
> > +
> > +       return 0;
> > +}
> > +
> > +static struct map_benchmark_ops dma_sg_map_benchmark_ops = {
> > +       .prepare = dma_sg_map_benchmark_prepare,
> > +       .unprepare = dma_sg_map_benchmark_unprepare,
> > +       .do_map = dma_sg_map_benchmark_do_map,
> > +       .do_unmap = dma_sg_map_benchmark_do_unmap, };
> > +
> >  static struct map_benchmark_ops *dma_map_benchmark_ops[DMA_MAP_MODE_MAX] = {
> >         [DMA_MAP_SINGLE_MODE] = &dma_single_map_benchmark_ops,
> > +       [DMA_MAP_SG_MODE] = &dma_sg_map_benchmark_ops,
> >  };
> >
> >  static int map_benchmark_thread(void *data)
> > --
> > 2.33.0
> >
>

Thanks
Barry

Re: [PATCH 2/3] dma-mapping: benchmark: add support for dma_map_sg

[Qinxin Xia]

在 2025/2/22 14:36, Barry Song 写道:
> On Fri, Feb 21, 2025 at 4:16 PM xiaqinxin <xiaqinxin@huawei.com> wrote:
>>
>>
>> -----邮件原件-----
>> 发件人: Barry Song <21cnbao@gmail.com>
>> 发送时间: 2025年2月18日 4:59
>> 收件人: xiaqinxin <xiaqinxin@huawei.com>
>> 抄送: chenxiang66@hisilicon.com; yangyicong <yangyicong@huawei.com>; hch@lst.de; iommu@lists.linux.dev; Jonathan Cameron <jonathan.cameron@huawei.com>; Zengtao (B) <prime.zeng@hisilicon.com>; fanghao (A) <fanghao11@huawei.com>; linux-kernel@vger.kernel.org
>> 主题: Re: [PATCH 2/3] dma-mapping: benchmark: add support for dma_map_sg
>>
>> On Wed, Feb 12, 2025 at 3:27 PM Qinxin Xia <xiaqinxin@huawei.com> wrote:
>>> Support for dma scatter-gather mapping and is intended for testing
>>> mapping performance. It achieves by introducing the dma_sg_map_param
>>> structure and related functions, which enable the implementation of
>>> scatter-gather mapping preparation, mapping, and unmapping operations.
>>> Additionally, the dma_map_benchmark_ops array is updated to include
>>> operations for scatter-gather mapping. This commit aims to provide a
>>> wider range of mapping performance test  to cater to different scenarios.
>> This benchmark is mainly designed to debug contention issues, such as IOMMU TLB flushes or IOMMU driver bottlenecks. I don't fully understand how SG or single will impact the evaluation of the IOMMU driver, making it unclear if the added complexity is justified.
>>
>> Can you add some explanation on why single mode is not sufficient for profiling and improving IOMMU drivers?
>>
>> Hello Barry ! 😊
>> Currently, the HiSilicon accelerator service uses the dma_map_sg interface. We want to evaluate the performance of the entire DMA map process. (including not only the iommu, but also the map framework). In addition, for scatterlist, __iommu_map is executed for each nent. This increases the complexity and time overhead of mapping. The effect of this fragmentation is not obvious in dma_map_single, which only handles a single contiguous block of memory.
>>
> Thanks!
> Please update your editor to ensure it doesn't respond with such long sentences
> without line breaks in the future :-)

Hello Barry !

Thank you for your advice, I will I'll pay attention. Leon

> Can you provide concrete examples or data showing how the newly added mode
> improves profiling of the entire DMA map process? For instance, what limitations
> exist without this mode? What performance issues cannot be identified without
> it?

You can see this patch 
:https://lore.kernel.org/all/cover.1738765879.git.leonro@nvidia.com/

Leon provides new interface for scatterlist scenarios to improve 
performance and gives some

application instance in rdma and vfio. Users can use dma_map_sg 
benchmark to measure

the performance improvement of the optimized interface compared with the 
previous one.

Thanks!

>
>>> Signed-off-by: Qinxin Xia <xiaqinxin@huawei.com>
>>> ---
>>>   include/linux/map_benchmark.h |   1 +
>>>   kernel/dma/map_benchmark.c    | 102 ++++++++++++++++++++++++++++++++++
>>>   2 files changed, 103 insertions(+)
>>>
>>> diff --git a/include/linux/map_benchmark.h
>>> b/include/linux/map_benchmark.h index 054db02a03a7..a9c1a104ba4f
>>> 100644
>>> --- a/include/linux/map_benchmark.h
>>> +++ b/include/linux/map_benchmark.h
>>> @@ -17,6 +17,7 @@
>>>
>>>   enum {
>>>          DMA_MAP_SINGLE_MODE,
>>> +       DMA_MAP_SG_MODE,
>>>          DMA_MAP_MODE_MAX
>>>   };
>>>
>>> diff --git a/kernel/dma/map_benchmark.c b/kernel/dma/map_benchmark.c
>>> index d8ec0ce058d8..b5828eeb3db7 100644
>>> --- a/kernel/dma/map_benchmark.c
>>> +++ b/kernel/dma/map_benchmark.c
>>> @@ -17,6 +17,7 @@
>>>   #include <linux/module.h>
>>>   #include <linux/pci.h>
>>>   #include <linux/platform_device.h>
>>> +#include <linux/scatterlist.h>
>>>   #include <linux/slab.h>
>>>   #include <linux/timekeeping.h>
>>>
>>> @@ -111,8 +112,109 @@ static struct map_benchmark_ops dma_single_map_benchmark_ops = {
>>>          .do_unmap = dma_single_map_benchmark_do_unmap,
>>>   };
>>>
>>> +struct dma_sg_map_param {
>>> +       struct sg_table sgt;
>>> +       struct device *dev;
>>> +       void **buf;
>>> +       u32 npages;
>>> +       u32 dma_dir;
>>> +};
>>> +
>>> +static void *dma_sg_map_benchmark_prepare(struct map_benchmark_data
>>> +*map) {
>>> +       struct scatterlist *sg;
>>> +       int i = 0;
>>> +
>>> +       struct dma_sg_map_param *mparam __free(kfree) = kzalloc(sizeof(*mparam), GFP_KERNEL);
>>> +       if (!mparam)
>>> +               return NULL;
>>> +
>>> +       mparam->npages = map->bparam.granule;
>>> +       mparam->dma_dir = map->bparam.dma_dir;
>>> +       mparam->dev = map->dev;
>>> +       mparam->buf = kmalloc_array(mparam->npages, sizeof(*mparam->buf),
>>> +                                   GFP_KERNEL);
>>> +       if (!mparam->buf)
>>> +               goto err1;
>>> +
>>> +       if (sg_alloc_table(&mparam->sgt, mparam->npages, GFP_KERNEL))
>>> +               goto err2;
>>> +
>>> +       for_each_sgtable_sg(&mparam->sgt, sg, i) {
>>> +               mparam->buf[i] = (void *)__get_free_page(GFP_KERNEL);
>>> +               if (!mparam->buf[i])
>>> +                       goto err3;
>>> +
>>> +               if (mparam->dma_dir != DMA_FROM_DEVICE)
>>> +                       memset(mparam->buf[i], 0x66, PAGE_SIZE);
>>> +
>>> +               sg_set_buf(sg, mparam->buf[i], PAGE_SIZE);
>>> +       }
>>> +
>>> +       return_ptr(mparam);
>>> +
>>> +err3:
>>> +       while (i-- > 0)
>>> +               free_page((unsigned long)mparam->buf[i]);
>>> +
>>> +       pr_err("dma_map_sg failed get free page on %s\n", dev_name(mparam->dev));
>>> +       sg_free_table(&mparam->sgt);
>>> +err2:
>>> +       pr_err("dma_map_sg failed alloc sg table on %s\n", dev_name(mparam->dev));
>>> +       kfree(mparam->buf);
>>> +err1:
>>> +       pr_err("dma_map_sg failed alloc mparam buf on %s\n", dev_name(mparam->dev));
>>> +       return NULL;
>>> +}
>>> +
>>> +static void dma_sg_map_benchmark_unprepare(void *arg) {
>>> +       struct dma_sg_map_param *mparam = arg;
>>> +       int i;
>>> +
>>> +       for (i = 0; i < mparam->npages; i++)
>>> +               free_page((unsigned long)mparam->buf[i]);
>>> +
>>> +       sg_free_table(&mparam->sgt);
>>> +
>>> +       kfree(mparam->buf);
>>> +       kfree(mparam);
>>> +}
>>> +
>>> +static int dma_sg_map_benchmark_do_map(void *arg) {
>>> +       struct dma_sg_map_param *mparam = arg;
>>> +
>>> +       int sg_mapped = dma_map_sg(mparam->dev, mparam->sgt.sgl,
>>> +                                  mparam->npages, mparam->dma_dir);
>>> +       if (!sg_mapped) {
>>> +               pr_err("dma_map_sg failed on %s\n", dev_name(mparam->dev));
>>> +               return -ENOMEM;
>>> +       }
>>> +
>>> +       return 0;
>>> +}
>>> +
>>> +static int dma_sg_map_benchmark_do_unmap(void *arg) {
>>> +       struct dma_sg_map_param *mparam = arg;
>>> +
>>> +       dma_unmap_sg(mparam->dev, mparam->sgt.sgl, mparam->npages,
>>> +                    mparam->dma_dir);
>>> +
>>> +       return 0;
>>> +}
>>> +
>>> +static struct map_benchmark_ops dma_sg_map_benchmark_ops = {
>>> +       .prepare = dma_sg_map_benchmark_prepare,
>>> +       .unprepare = dma_sg_map_benchmark_unprepare,
>>> +       .do_map = dma_sg_map_benchmark_do_map,
>>> +       .do_unmap = dma_sg_map_benchmark_do_unmap, };
>>> +
>>>   static struct map_benchmark_ops *dma_map_benchmark_ops[DMA_MAP_MODE_MAX] = {
>>>          [DMA_MAP_SINGLE_MODE] = &dma_single_map_benchmark_ops,
>>> +       [DMA_MAP_SG_MODE] = &dma_sg_map_benchmark_ops,
>>>   };
>>>
>>>   static int map_benchmark_thread(void *data)
>>> --
>>> 2.33.0
>>>
> Thanks
> Barry

Re: [PATCH 2/3] dma-mapping: benchmark: add support for dma_map_sg

[Qinxin Xia]

在 2025/3/4 21:49, Qinxin Xia 写道:
>
> 在 2025/2/22 14:36, Barry Song 写道:
>> On Fri, Feb 21, 2025 at 4:16 PM xiaqinxin <xiaqinxin@huawei.com> wrote:
>>>
>>>
>>> -----邮件原件-----
>>> 发件人: Barry Song <21cnbao@gmail.com>
>>> 发送时间: 2025年2月18日 4:59
>>> 收件人: xiaqinxin <xiaqinxin@huawei.com>
>>> 抄送: chenxiang66@hisilicon.com; yangyicong <yangyicong@huawei.com>; 
>>> hch@lst.de; iommu@lists.linux.dev; Jonathan Cameron 
>>> <jonathan.cameron@huawei.com>; Zengtao (B) 
>>> <prime.zeng@hisilicon.com>; fanghao (A) <fanghao11@huawei.com>; 
>>> linux-kernel@vger.kernel.org
>>> 主题: Re: [PATCH 2/3] dma-mapping: benchmark: add support for 
>>> dma_map_sg
>>>
>>> On Wed, Feb 12, 2025 at 3:27 PM Qinxin Xia <xiaqinxin@huawei.com> 
>>> wrote:
>>>> Support for dma scatter-gather mapping and is intended for testing
>>>> mapping performance. It achieves by introducing the dma_sg_map_param
>>>> structure and related functions, which enable the implementation of
>>>> scatter-gather mapping preparation, mapping, and unmapping operations.
>>>> Additionally, the dma_map_benchmark_ops array is updated to include
>>>> operations for scatter-gather mapping. This commit aims to provide a
>>>> wider range of mapping performance test  to cater to different 
>>>> scenarios.
>>> This benchmark is mainly designed to debug contention issues, such 
>>> as IOMMU TLB flushes or IOMMU driver bottlenecks. I don't fully 
>>> understand how SG or single will impact the evaluation of the IOMMU 
>>> driver, making it unclear if the added complexity is justified.
>>>
>>> Can you add some explanation on why single mode is not sufficient 
>>> for profiling and improving IOMMU drivers?
>>>
>>> Hello Barry ! 😊
>>> Currently, the HiSilicon accelerator service uses the dma_map_sg 
>>> interface. We want to evaluate the performance of the entire DMA map 
>>> process. (including not only the iommu, but also the map framework). 
>>> In addition, for scatterlist, __iommu_map is executed for each nent. 
>>> This increases the complexity and time overhead of mapping. The 
>>> effect of this fragmentation is not obvious in dma_map_single, which 
>>> only handles a single contiguous block of memory.
>>>
>> Thanks!
>> Please update your editor to ensure it doesn't respond with such long 
>> sentences
>> without line breaks in the future :-)
>
> Hello Barry !
>
> Thank you for your advice, I will I'll pay attention. Leon

A little mistake, sorry for it...

I will I'll pay attention. Leon  ==> I will pay attention.

>
>> Can you provide concrete examples or data showing how the newly added 
>> mode
>> improves profiling of the entire DMA map process? For instance, what 
>> limitations
>> exist without this mode? What performance issues cannot be identified 
>> without
>> it?
>
> You can see this patch 
> :https://lore.kernel.org/all/cover.1738765879.git.leonro@nvidia.com/
>
> Leon provides new interface for scatterlist scenarios to improve 
> performance and gives some
>
> application instance in rdma and vfio. Users can use dma_map_sg 
> benchmark to measure
>
> the performance improvement of the optimized interface compared with 
> the previous one.
>
> Thanks!
>
>>
>>>> Signed-off-by: Qinxin Xia <xiaqinxin@huawei.com>
>>>> ---
>>>>   include/linux/map_benchmark.h |   1 +
>>>>   kernel/dma/map_benchmark.c    | 102 
>>>> ++++++++++++++++++++++++++++++++++
>>>>   2 files changed, 103 insertions(+)
>>>>
>>>> diff --git a/include/linux/map_benchmark.h
>>>> b/include/linux/map_benchmark.h index 054db02a03a7..a9c1a104ba4f
>>>> 100644
>>>> --- a/include/linux/map_benchmark.h
>>>> +++ b/include/linux/map_benchmark.h
>>>> @@ -17,6 +17,7 @@
>>>>
>>>>   enum {
>>>>          DMA_MAP_SINGLE_MODE,
>>>> +       DMA_MAP_SG_MODE,
>>>>          DMA_MAP_MODE_MAX
>>>>   };
>>>>
>>>> diff --git a/kernel/dma/map_benchmark.c b/kernel/dma/map_benchmark.c
>>>> index d8ec0ce058d8..b5828eeb3db7 100644
>>>> --- a/kernel/dma/map_benchmark.c
>>>> +++ b/kernel/dma/map_benchmark.c
>>>> @@ -17,6 +17,7 @@
>>>>   #include <linux/module.h>
>>>>   #include <linux/pci.h>
>>>>   #include <linux/platform_device.h>
>>>> +#include <linux/scatterlist.h>
>>>>   #include <linux/slab.h>
>>>>   #include <linux/timekeeping.h>
>>>>
>>>> @@ -111,8 +112,109 @@ static struct map_benchmark_ops 
>>>> dma_single_map_benchmark_ops = {
>>>>          .do_unmap = dma_single_map_benchmark_do_unmap,
>>>>   };
>>>>
>>>> +struct dma_sg_map_param {
>>>> +       struct sg_table sgt;
>>>> +       struct device *dev;
>>>> +       void **buf;
>>>> +       u32 npages;
>>>> +       u32 dma_dir;
>>>> +};
>>>> +
>>>> +static void *dma_sg_map_benchmark_prepare(struct map_benchmark_data
>>>> +*map) {
>>>> +       struct scatterlist *sg;
>>>> +       int i = 0;
>>>> +
>>>> +       struct dma_sg_map_param *mparam __free(kfree) = 
>>>> kzalloc(sizeof(*mparam), GFP_KERNEL);
>>>> +       if (!mparam)
>>>> +               return NULL;
>>>> +
>>>> +       mparam->npages = map->bparam.granule;
>>>> +       mparam->dma_dir = map->bparam.dma_dir;
>>>> +       mparam->dev = map->dev;
>>>> +       mparam->buf = kmalloc_array(mparam->npages, 
>>>> sizeof(*mparam->buf),
>>>> +                                   GFP_KERNEL);
>>>> +       if (!mparam->buf)
>>>> +               goto err1;
>>>> +
>>>> +       if (sg_alloc_table(&mparam->sgt, mparam->npages, GFP_KERNEL))
>>>> +               goto err2;
>>>> +
>>>> +       for_each_sgtable_sg(&mparam->sgt, sg, i) {
>>>> +               mparam->buf[i] = (void *)__get_free_page(GFP_KERNEL);
>>>> +               if (!mparam->buf[i])
>>>> +                       goto err3;
>>>> +
>>>> +               if (mparam->dma_dir != DMA_FROM_DEVICE)
>>>> +                       memset(mparam->buf[i], 0x66, PAGE_SIZE);
>>>> +
>>>> +               sg_set_buf(sg, mparam->buf[i], PAGE_SIZE);
>>>> +       }
>>>> +
>>>> +       return_ptr(mparam);
>>>> +
>>>> +err3:
>>>> +       while (i-- > 0)
>>>> +               free_page((unsigned long)mparam->buf[i]);
>>>> +
>>>> +       pr_err("dma_map_sg failed get free page on %s\n", 
>>>> dev_name(mparam->dev));
>>>> +       sg_free_table(&mparam->sgt);
>>>> +err2:
>>>> +       pr_err("dma_map_sg failed alloc sg table on %s\n", 
>>>> dev_name(mparam->dev));
>>>> +       kfree(mparam->buf);
>>>> +err1:
>>>> +       pr_err("dma_map_sg failed alloc mparam buf on %s\n", 
>>>> dev_name(mparam->dev));
>>>> +       return NULL;
>>>> +}
>>>> +
>>>> +static void dma_sg_map_benchmark_unprepare(void *arg) {
>>>> +       struct dma_sg_map_param *mparam = arg;
>>>> +       int i;
>>>> +
>>>> +       for (i = 0; i < mparam->npages; i++)
>>>> +               free_page((unsigned long)mparam->buf[i]);
>>>> +
>>>> +       sg_free_table(&mparam->sgt);
>>>> +
>>>> +       kfree(mparam->buf);
>>>> +       kfree(mparam);
>>>> +}
>>>> +
>>>> +static int dma_sg_map_benchmark_do_map(void *arg) {
>>>> +       struct dma_sg_map_param *mparam = arg;
>>>> +
>>>> +       int sg_mapped = dma_map_sg(mparam->dev, mparam->sgt.sgl,
>>>> +                                  mparam->npages, mparam->dma_dir);
>>>> +       if (!sg_mapped) {
>>>> +               pr_err("dma_map_sg failed on %s\n", 
>>>> dev_name(mparam->dev));
>>>> +               return -ENOMEM;
>>>> +       }
>>>> +
>>>> +       return 0;
>>>> +}
>>>> +
>>>> +static int dma_sg_map_benchmark_do_unmap(void *arg) {
>>>> +       struct dma_sg_map_param *mparam = arg;
>>>> +
>>>> +       dma_unmap_sg(mparam->dev, mparam->sgt.sgl, mparam->npages,
>>>> +                    mparam->dma_dir);
>>>> +
>>>> +       return 0;
>>>> +}
>>>> +
>>>> +static struct map_benchmark_ops dma_sg_map_benchmark_ops = {
>>>> +       .prepare = dma_sg_map_benchmark_prepare,
>>>> +       .unprepare = dma_sg_map_benchmark_unprepare,
>>>> +       .do_map = dma_sg_map_benchmark_do_map,
>>>> +       .do_unmap = dma_sg_map_benchmark_do_unmap, };
>>>> +
>>>>   static struct map_benchmark_ops 
>>>> *dma_map_benchmark_ops[DMA_MAP_MODE_MAX] = {
>>>>          [DMA_MAP_SINGLE_MODE] = &dma_single_map_benchmark_ops,
>>>> +       [DMA_MAP_SG_MODE] = &dma_sg_map_benchmark_ops,
>>>>   };
>>>>
>>>>   static int map_benchmark_thread(void *data)
>>>> -- 
>>>> 2.33.0
>>>>
>> Thanks
>> Barry

Re: [PATCH 2/3] dma-mapping: benchmark: add support for dma_map_sg

[Barry Song]

On Wed, Mar 5, 2025 at 2:49 AM Qinxin Xia <xiaqinxin@huawei.com> wrote:
>
>
> 在 2025/2/22 14:36, Barry Song 写道:
> > On Fri, Feb 21, 2025 at 4:16 PM xiaqinxin <xiaqinxin@huawei.com> wrote:
> >>
> >>
> >> -----邮件原件-----
> >> 发件人: Barry Song <21cnbao@gmail.com>
> >> 发送时间: 2025年2月18日 4:59
> >> 收件人: xiaqinxin <xiaqinxin@huawei.com>
> >> 抄送: chenxiang66@hisilicon.com; yangyicong <yangyicong@huawei.com>; hch@lst.de; iommu@lists.linux.dev; Jonathan Cameron <jonathan.cameron@huawei.com>; Zengtao (B) <prime.zeng@hisilicon.com>; fanghao (A) <fanghao11@huawei.com>; linux-kernel@vger.kernel.org
> >> 主题: Re: [PATCH 2/3] dma-mapping: benchmark: add support for dma_map_sg
> >>
> >> On Wed, Feb 12, 2025 at 3:27 PM Qinxin Xia <xiaqinxin@huawei.com> wrote:
> >>> Support for dma scatter-gather mapping and is intended for testing
> >>> mapping performance. It achieves by introducing the dma_sg_map_param
> >>> structure and related functions, which enable the implementation of
> >>> scatter-gather mapping preparation, mapping, and unmapping operations.
> >>> Additionally, the dma_map_benchmark_ops array is updated to include
> >>> operations for scatter-gather mapping. This commit aims to provide a
> >>> wider range of mapping performance test  to cater to different scenarios.
> >> This benchmark is mainly designed to debug contention issues, such as IOMMU TLB flushes or IOMMU driver bottlenecks. I don't fully understand how SG or single will impact the evaluation of the IOMMU driver, making it unclear if the added complexity is justified.
> >>
> >> Can you add some explanation on why single mode is not sufficient for profiling and improving IOMMU drivers?
> >>
> >> Hello Barry ! 😊
> >> Currently, the HiSilicon accelerator service uses the dma_map_sg interface. We want to evaluate the performance of the entire DMA map process. (including not only the iommu, but also the map framework). In addition, for scatterlist, __iommu_map is executed for each nent. This increases the complexity and time overhead of mapping. The effect of this fragmentation is not obvious in dma_map_single, which only handles a single contiguous block of memory.
> >>
> > Thanks!
> > Please update your editor to ensure it doesn't respond with such long sentences
> > without line breaks in the future :-)
>
> Hello Barry !
>
> Thank you for your advice, I will I'll pay attention. Leon
>
> > Can you provide concrete examples or data showing how the newly added mode
> > improves profiling of the entire DMA map process? For instance, what limitations
> > exist without this mode? What performance issues cannot be identified without
> > it?
>
> You can see this patch
> :https://lore.kernel.org/all/cover.1738765879.git.leonro@nvidia.com/
>
> Leon provides new interface for scatterlist scenarios to improve
> performance and gives some
>
> application instance in rdma and vfio. Users can use dma_map_sg
> benchmark to measure
>
> the performance improvement of the optimized interface compared with the
> previous one.

I’m not quite sure how this patchset helps compare the new interfaces—
dma_iova_try_alloc(), dma_iova_link(), dma_iova_destroy()—with
dma_map_sg(), dma_unmap_sg(), etc. Does this mean you also plan
to include these new interfaces in the benchmark?

However, I agree that this patchset could be useful for evaluating cases
where we are optimizing dma_map_sg itself for A/B side testing. Do we
have such a case?

>
> Thanks!
>
> >
> >>> Signed-off-by: Qinxin Xia <xiaqinxin@huawei.com>
> >>> ---
> >>>   include/linux/map_benchmark.h |   1 +
> >>>   kernel/dma/map_benchmark.c    | 102 ++++++++++++++++++++++++++++++++++
> >>>   2 files changed, 103 insertions(+)
> >>>
> >>> diff --git a/include/linux/map_benchmark.h
> >>> b/include/linux/map_benchmark.h index 054db02a03a7..a9c1a104ba4f
> >>> 100644
> >>> --- a/include/linux/map_benchmark.h
> >>> +++ b/include/linux/map_benchmark.h
> >>> @@ -17,6 +17,7 @@
> >>>
> >>>   enum {
> >>>          DMA_MAP_SINGLE_MODE,
> >>> +       DMA_MAP_SG_MODE,
> >>>          DMA_MAP_MODE_MAX
> >>>   };
> >>>
> >>> diff --git a/kernel/dma/map_benchmark.c b/kernel/dma/map_benchmark.c
> >>> index d8ec0ce058d8..b5828eeb3db7 100644
> >>> --- a/kernel/dma/map_benchmark.c
> >>> +++ b/kernel/dma/map_benchmark.c
> >>> @@ -17,6 +17,7 @@
> >>>   #include <linux/module.h>
> >>>   #include <linux/pci.h>
> >>>   #include <linux/platform_device.h>
> >>> +#include <linux/scatterlist.h>
> >>>   #include <linux/slab.h>
> >>>   #include <linux/timekeeping.h>
> >>>
> >>> @@ -111,8 +112,109 @@ static struct map_benchmark_ops dma_single_map_benchmark_ops = {
> >>>          .do_unmap = dma_single_map_benchmark_do_unmap,
> >>>   };
> >>>
> >>> +struct dma_sg_map_param {
> >>> +       struct sg_table sgt;
> >>> +       struct device *dev;
> >>> +       void **buf;
> >>> +       u32 npages;
> >>> +       u32 dma_dir;
> >>> +};
> >>> +
> >>> +static void *dma_sg_map_benchmark_prepare(struct map_benchmark_data
> >>> +*map) {
> >>> +       struct scatterlist *sg;
> >>> +       int i = 0;
> >>> +
> >>> +       struct dma_sg_map_param *mparam __free(kfree) = kzalloc(sizeof(*mparam), GFP_KERNEL);
> >>> +       if (!mparam)
> >>> +               return NULL;
> >>> +
> >>> +       mparam->npages = map->bparam.granule;
> >>> +       mparam->dma_dir = map->bparam.dma_dir;
> >>> +       mparam->dev = map->dev;
> >>> +       mparam->buf = kmalloc_array(mparam->npages, sizeof(*mparam->buf),
> >>> +                                   GFP_KERNEL);
> >>> +       if (!mparam->buf)
> >>> +               goto err1;
> >>> +
> >>> +       if (sg_alloc_table(&mparam->sgt, mparam->npages, GFP_KERNEL))
> >>> +               goto err2;
> >>> +
> >>> +       for_each_sgtable_sg(&mparam->sgt, sg, i) {
> >>> +               mparam->buf[i] = (void *)__get_free_page(GFP_KERNEL);
> >>> +               if (!mparam->buf[i])
> >>> +                       goto err3;
> >>> +
> >>> +               if (mparam->dma_dir != DMA_FROM_DEVICE)
> >>> +                       memset(mparam->buf[i], 0x66, PAGE_SIZE);
> >>> +
> >>> +               sg_set_buf(sg, mparam->buf[i], PAGE_SIZE);
> >>> +       }
> >>> +
> >>> +       return_ptr(mparam);
> >>> +
> >>> +err3:
> >>> +       while (i-- > 0)
> >>> +               free_page((unsigned long)mparam->buf[i]);
> >>> +
> >>> +       pr_err("dma_map_sg failed get free page on %s\n", dev_name(mparam->dev));
> >>> +       sg_free_table(&mparam->sgt);
> >>> +err2:
> >>> +       pr_err("dma_map_sg failed alloc sg table on %s\n", dev_name(mparam->dev));
> >>> +       kfree(mparam->buf);
> >>> +err1:
> >>> +       pr_err("dma_map_sg failed alloc mparam buf on %s\n", dev_name(mparam->dev));
> >>> +       return NULL;
> >>> +}
> >>> +
> >>> +static void dma_sg_map_benchmark_unprepare(void *arg) {
> >>> +       struct dma_sg_map_param *mparam = arg;
> >>> +       int i;
> >>> +
> >>> +       for (i = 0; i < mparam->npages; i++)
> >>> +               free_page((unsigned long)mparam->buf[i]);
> >>> +
> >>> +       sg_free_table(&mparam->sgt);
> >>> +
> >>> +       kfree(mparam->buf);
> >>> +       kfree(mparam);
> >>> +}
> >>> +
> >>> +static int dma_sg_map_benchmark_do_map(void *arg) {
> >>> +       struct dma_sg_map_param *mparam = arg;
> >>> +
> >>> +       int sg_mapped = dma_map_sg(mparam->dev, mparam->sgt.sgl,
> >>> +                                  mparam->npages, mparam->dma_dir);
> >>> +       if (!sg_mapped) {
> >>> +               pr_err("dma_map_sg failed on %s\n", dev_name(mparam->dev));
> >>> +               return -ENOMEM;
> >>> +       }
> >>> +
> >>> +       return 0;
> >>> +}
> >>> +
> >>> +static int dma_sg_map_benchmark_do_unmap(void *arg) {
> >>> +       struct dma_sg_map_param *mparam = arg;
> >>> +
> >>> +       dma_unmap_sg(mparam->dev, mparam->sgt.sgl, mparam->npages,
> >>> +                    mparam->dma_dir);
> >>> +
> >>> +       return 0;
> >>> +}
> >>> +
> >>> +static struct map_benchmark_ops dma_sg_map_benchmark_ops = {
> >>> +       .prepare = dma_sg_map_benchmark_prepare,
> >>> +       .unprepare = dma_sg_map_benchmark_unprepare,
> >>> +       .do_map = dma_sg_map_benchmark_do_map,
> >>> +       .do_unmap = dma_sg_map_benchmark_do_unmap, };
> >>> +
> >>>   static struct map_benchmark_ops *dma_map_benchmark_ops[DMA_MAP_MODE_MAX] = {
> >>>          [DMA_MAP_SINGLE_MODE] = &dma_single_map_benchmark_ops,
> >>> +       [DMA_MAP_SG_MODE] = &dma_sg_map_benchmark_ops,
> >>>   };
> >>>
> >>>   static int map_benchmark_thread(void *data)
> >>> --
> >>> 2.33.0
> >>>

Thanks
Barry

Re: [PATCH 2/3] dma-mapping: benchmark: add support for dma_map_sg

[Qinxin Xia]

在 2025/3/6 17:28, Barry Song 写道:
> On Wed, Mar 5, 2025 at 2:49 AM Qinxin Xia <xiaqinxin@huawei.com> wrote:
>>
>> 在 2025/2/22 14:36, Barry Song 写道:
>>> On Fri, Feb 21, 2025 at 4:16 PM xiaqinxin <xiaqinxin@huawei.com> wrote:
>>>>
>>>> -----邮件原件-----
>>>> 发件人: Barry Song <21cnbao@gmail.com>
>>>> 发送时间: 2025年2月18日 4:59
>>>> 收件人: xiaqinxin <xiaqinxin@huawei.com>
>>>> 抄送: chenxiang66@hisilicon.com; yangyicong <yangyicong@huawei.com>; hch@lst.de; iommu@lists.linux.dev; Jonathan Cameron <jonathan.cameron@huawei.com>; Zengtao (B) <prime.zeng@hisilicon.com>; fanghao (A) <fanghao11@huawei.com>; linux-kernel@vger.kernel.org
>>>> 主题: Re: [PATCH 2/3] dma-mapping: benchmark: add support for dma_map_sg
>>>>
>>>> On Wed, Feb 12, 2025 at 3:27 PM Qinxin Xia <xiaqinxin@huawei.com> wrote:
>>>>> Support for dma scatter-gather mapping and is intended for testing
>>>>> mapping performance. It achieves by introducing the dma_sg_map_param
>>>>> structure and related functions, which enable the implementation of
>>>>> scatter-gather mapping preparation, mapping, and unmapping operations.
>>>>> Additionally, the dma_map_benchmark_ops array is updated to include
>>>>> operations for scatter-gather mapping. This commit aims to provide a
>>>>> wider range of mapping performance test  to cater to different scenarios.
>>>> This benchmark is mainly designed to debug contention issues, such as IOMMU TLB flushes or IOMMU driver bottlenecks. I don't fully understand how SG or single will impact the evaluation of the IOMMU driver, making it unclear if the added complexity is justified.
>>>>
>>>> Can you add some explanation on why single mode is not sufficient for profiling and improving IOMMU drivers?
>>>>
>>>> Hello Barry ! 😊
>>>> Currently, the HiSilicon accelerator service uses the dma_map_sg interface. We want to evaluate the performance of the entire DMA map process. (including not only the iommu, but also the map framework). In addition, for scatterlist, __iommu_map is executed for each nent. This increases the complexity and time overhead of mapping. The effect of this fragmentation is not obvious in dma_map_single, which only handles a single contiguous block of memory.
>>>>
>>> Thanks!
>>> Please update your editor to ensure it doesn't respond with such long sentences
>>> without line breaks in the future :-)
>> Hello Barry !
>>
>> Thank you for your advice, I will I'll pay attention. Leon
>>
>>> Can you provide concrete examples or data showing how the newly added mode
>>> improves profiling of the entire DMA map process? For instance, what limitations
>>> exist without this mode? What performance issues cannot be identified without
>>> it?
>> You can see this patch
>> :https://lore.kernel.org/all/cover.1738765879.git.leonro@nvidia.com/
>>
>> Leon provides new interface for scatterlist scenarios to improve
>> performance and gives some
>>
>> application instance in rdma and vfio. Users can use dma_map_sg
>> benchmark to measure
>>
>> the performance improvement of the optimized interface compared with the
>> previous one.
> I’m not quite sure how this patchset helps compare the new interfaces—
> dma_iova_try_alloc(), dma_iova_link(), dma_iova_destroy()—with
> dma_map_sg(), dma_unmap_sg(), etc. Does this mean you also plan
> to include these new interfaces in the benchmark?
>
> However, I agree that this patchset could be useful for evaluating cases
> where we are optimizing dma_map_sg itself for A/B side testing. Do we
> have such a case?
Hello Barry !

I'm sorry, I haven't responded for so long, something urgent took up my 
time,
and we've had some discussion about the requirement scenario.
CCA precision computing calls dma_map_sg and reuses the swiotlb process.
Our team and HUAWEI CLOUD are trying to optimize this process, such as 
this patch:
https://lwn.net/ml/linux-kernel/cover.1690871004.git.petr.tesarik.ext@huawei.com/, 


dma_map_sg benchmark provides a test method to evaluate performance results.

Thank you !
>> Thanks!
>>
>>>>> Signed-off-by: Qinxin Xia <xiaqinxin@huawei.com>
>>>>> ---
>>>>>    include/linux/map_benchmark.h |   1 +
>>>>>    kernel/dma/map_benchmark.c    | 102 ++++++++++++++++++++++++++++++++++
>>>>>    2 files changed, 103 insertions(+)
>>>>>
>>>>> diff --git a/include/linux/map_benchmark.h
>>>>> b/include/linux/map_benchmark.h index 054db02a03a7..a9c1a104ba4f
>>>>> 100644
>>>>> --- a/include/linux/map_benchmark.h
>>>>> +++ b/include/linux/map_benchmark.h
>>>>> @@ -17,6 +17,7 @@
>>>>>
>>>>>    enum {
>>>>>           DMA_MAP_SINGLE_MODE,
>>>>> +       DMA_MAP_SG_MODE,
>>>>>           DMA_MAP_MODE_MAX
>>>>>    };
>>>>>
>>>>> diff --git a/kernel/dma/map_benchmark.c b/kernel/dma/map_benchmark.c
>>>>> index d8ec0ce058d8..b5828eeb3db7 100644
>>>>> --- a/kernel/dma/map_benchmark.c
>>>>> +++ b/kernel/dma/map_benchmark.c
>>>>> @@ -17,6 +17,7 @@
>>>>>    #include <linux/module.h>
>>>>>    #include <linux/pci.h>
>>>>>    #include <linux/platform_device.h>
>>>>> +#include <linux/scatterlist.h>
>>>>>    #include <linux/slab.h>
>>>>>    #include <linux/timekeeping.h>
>>>>>
>>>>> @@ -111,8 +112,109 @@ static struct map_benchmark_ops dma_single_map_benchmark_ops = {
>>>>>           .do_unmap = dma_single_map_benchmark_do_unmap,
>>>>>    };
>>>>>
>>>>> +struct dma_sg_map_param {
>>>>> +       struct sg_table sgt;
>>>>> +       struct device *dev;
>>>>> +       void **buf;
>>>>> +       u32 npages;
>>>>> +       u32 dma_dir;
>>>>> +};
>>>>> +
>>>>> +static void *dma_sg_map_benchmark_prepare(struct map_benchmark_data
>>>>> +*map) {
>>>>> +       struct scatterlist *sg;
>>>>> +       int i = 0;
>>>>> +
>>>>> +       struct dma_sg_map_param *mparam __free(kfree) = kzalloc(sizeof(*mparam), GFP_KERNEL);
>>>>> +       if (!mparam)
>>>>> +               return NULL;
>>>>> +
>>>>> +       mparam->npages = map->bparam.granule;
>>>>> +       mparam->dma_dir = map->bparam.dma_dir;
>>>>> +       mparam->dev = map->dev;
>>>>> +       mparam->buf = kmalloc_array(mparam->npages, sizeof(*mparam->buf),
>>>>> +                                   GFP_KERNEL);
>>>>> +       if (!mparam->buf)
>>>>> +               goto err1;
>>>>> +
>>>>> +       if (sg_alloc_table(&mparam->sgt, mparam->npages, GFP_KERNEL))
>>>>> +               goto err2;
>>>>> +
>>>>> +       for_each_sgtable_sg(&mparam->sgt, sg, i) {
>>>>> +               mparam->buf[i] = (void *)__get_free_page(GFP_KERNEL);
>>>>> +               if (!mparam->buf[i])
>>>>> +                       goto err3;
>>>>> +
>>>>> +               if (mparam->dma_dir != DMA_FROM_DEVICE)
>>>>> +                       memset(mparam->buf[i], 0x66, PAGE_SIZE);
>>>>> +
>>>>> +               sg_set_buf(sg, mparam->buf[i], PAGE_SIZE);
>>>>> +       }
>>>>> +
>>>>> +       return_ptr(mparam);
>>>>> +
>>>>> +err3:
>>>>> +       while (i-- > 0)
>>>>> +               free_page((unsigned long)mparam->buf[i]);
>>>>> +
>>>>> +       pr_err("dma_map_sg failed get free page on %s\n", dev_name(mparam->dev));
>>>>> +       sg_free_table(&mparam->sgt);
>>>>> +err2:
>>>>> +       pr_err("dma_map_sg failed alloc sg table on %s\n", dev_name(mparam->dev));
>>>>> +       kfree(mparam->buf);
>>>>> +err1:
>>>>> +       pr_err("dma_map_sg failed alloc mparam buf on %s\n", dev_name(mparam->dev));
>>>>> +       return NULL;
>>>>> +}
>>>>> +
>>>>> +static void dma_sg_map_benchmark_unprepare(void *arg) {
>>>>> +       struct dma_sg_map_param *mparam = arg;
>>>>> +       int i;
>>>>> +
>>>>> +       for (i = 0; i < mparam->npages; i++)
>>>>> +               free_page((unsigned long)mparam->buf[i]);
>>>>> +
>>>>> +       sg_free_table(&mparam->sgt);
>>>>> +
>>>>> +       kfree(mparam->buf);
>>>>> +       kfree(mparam);
>>>>> +}
>>>>> +
>>>>> +static int dma_sg_map_benchmark_do_map(void *arg) {
>>>>> +       struct dma_sg_map_param *mparam = arg;
>>>>> +
>>>>> +       int sg_mapped = dma_map_sg(mparam->dev, mparam->sgt.sgl,
>>>>> +                                  mparam->npages, mparam->dma_dir);
>>>>> +       if (!sg_mapped) {
>>>>> +               pr_err("dma_map_sg failed on %s\n", dev_name(mparam->dev));
>>>>> +               return -ENOMEM;
>>>>> +       }
>>>>> +
>>>>> +       return 0;
>>>>> +}
>>>>> +
>>>>> +static int dma_sg_map_benchmark_do_unmap(void *arg) {
>>>>> +       struct dma_sg_map_param *mparam = arg;
>>>>> +
>>>>> +       dma_unmap_sg(mparam->dev, mparam->sgt.sgl, mparam->npages,
>>>>> +                    mparam->dma_dir);
>>>>> +
>>>>> +       return 0;
>>>>> +}
>>>>> +
>>>>> +static struct map_benchmark_ops dma_sg_map_benchmark_ops = {
>>>>> +       .prepare = dma_sg_map_benchmark_prepare,
>>>>> +       .unprepare = dma_sg_map_benchmark_unprepare,
>>>>> +       .do_map = dma_sg_map_benchmark_do_map,
>>>>> +       .do_unmap = dma_sg_map_benchmark_do_unmap, };
>>>>> +
>>>>>    static struct map_benchmark_ops *dma_map_benchmark_ops[DMA_MAP_MODE_MAX] = {
>>>>>           [DMA_MAP_SINGLE_MODE] = &dma_single_map_benchmark_ops,
>>>>> +       [DMA_MAP_SG_MODE] = &dma_sg_map_benchmark_ops,
>>>>>    };
>>>>>
>>>>>    static int map_benchmark_thread(void *data)
>>>>> --
>>>>> 2.33.0
>>>>>
> Thanks
> Barry

Re: [PATCH 1/3] dma mapping benchmark: modify the framework to adapt to more map modes

[Barry Song]

On Wed, Feb 12, 2025 at 3:27 PM Qinxin Xia <xiaqinxin@huawei.com> wrote:
>
> In this patch map_benchmark abstract in four interface: prepare, unprepare,
> do_map, do_unmap. When there's a new mode to add, need four steps:
> 1) Add the mode in map_benchmark.h
>
> 2) Defines the mode param, like struct dma_xxx_map_param, and this object
>    will be return in prepare and as input parameter in other ops;
>
> 3) Defines the ops functions:prepare, unprepare, do_map, do_unmap.
>
> 4) Add the new mode in dma_map_benchmark_ops.
>
> Signed-off-by: Qinxin Xia <xiaqinxin@huawei.com>
> ---
>  include/linux/map_benchmark.h |   6 ++
>  kernel/dma/map_benchmark.c    | 122 +++++++++++++++++++++++++++-------
>  2 files changed, 105 insertions(+), 23 deletions(-)
>
> diff --git a/include/linux/map_benchmark.h b/include/linux/map_benchmark.h
> index 62674c83bde4..054db02a03a7 100644
> --- a/include/linux/map_benchmark.h
> +++ b/include/linux/map_benchmark.h
> @@ -15,6 +15,11 @@
>  #define DMA_MAP_TO_DEVICE       1
>  #define DMA_MAP_FROM_DEVICE     2
>
> +enum {
> +       DMA_MAP_SINGLE_MODE,
> +       DMA_MAP_MODE_MAX
> +};
> +
>  struct map_benchmark {
>         __u64 avg_map_100ns; /* average map latency in 100ns */
>         __u64 map_stddev; /* standard deviation of map latency */
> @@ -27,5 +32,6 @@ struct map_benchmark {
>         __u32 dma_dir; /* DMA data direction */
>         __u32 dma_trans_ns; /* time for DMA transmission in ns */
>         __u32 granule;  /* how many PAGE_SIZE will do map/unmap once a time */
> +       __u8  map_mode; /* the mode of dma map */

We previously added some padding to ensure the uABI remained consistent.
I just noticed that Tiantao’s commit (8ddde07a3d285a0f3cec, "dma-mapping:
benchmark: extract a common header file for map_benchmark definition")
accidentally removed that padding, which has completely broken the ABIs.

Could you send a patch to fix this regression, and CC it to
stable@vger.kernel.org before adding the new field which should
use the expansion[] instead.


>  };
>  #endif /* _KERNEL_DMA_BENCHMARK_H */
> diff --git a/kernel/dma/map_benchmark.c b/kernel/dma/map_benchmark.c
> index cc19a3efea89..d8ec0ce058d8 100644
> --- a/kernel/dma/map_benchmark.c
> +++ b/kernel/dma/map_benchmark.c
> @@ -5,6 +5,7 @@
>
>  #define pr_fmt(fmt)    KBUILD_MODNAME ": " fmt
>
> +#include <linux/cleanup.h>
>  #include <linux/debugfs.h>
>  #include <linux/delay.h>
>  #include <linux/device.h>
> @@ -31,17 +32,98 @@ struct map_benchmark_data {
>         atomic64_t loops;
>  };
>
> +struct map_benchmark_ops {
> +       void *(*prepare)(struct map_benchmark_data *map);
> +       void (*unprepare)(void *arg);
> +       int (*do_map)(void *arg);
> +       int (*do_unmap)(void *arg);
> +};
> +
> +struct dma_single_map_param {
> +       struct device *dev;
> +       dma_addr_t addr;
> +       void *xbuf;
> +       u32 npages;
> +       u32 dma_dir;
> +};
> +
> +static void *dma_single_map_benchmark_prepare(struct map_benchmark_data *map)
> +{
> +       struct dma_single_map_param *mparam __free(kfree) = kzalloc(sizeof(*mparam),
> +                                                                   GFP_KERNEL);
> +       if (!mparam)
> +               return NULL;
> +
> +       mparam->npages = map->bparam.granule;
> +       mparam->dma_dir = map->bparam.dma_dir;
> +       mparam->dev = map->dev;
> +       mparam->xbuf = alloc_pages_exact(mparam->npages * PAGE_SIZE, GFP_KERNEL);
> +       if (!mparam->xbuf)
> +               return NULL;
> +
> +       /*
> +        * for a non-coherent device, if we don't stain them in the
> +        * cache, this will give an underestimate of the real-world
> +        * overhead of BIDIRECTIONAL or TO_DEVICE mappings;
> +        * 66 means evertything goes well! 66 is lucky.
> +        */
> +       if (mparam->dma_dir != DMA_FROM_DEVICE)
> +               memset(mparam->xbuf, 0x66, mparam->npages * PAGE_SIZE);
> +
> +       return_ptr(mparam);
> +}
> +
> +static void dma_single_map_benchmark_unprepare(void *arg)
> +{
> +       struct dma_single_map_param *mparam = arg;
> +
> +       free_pages_exact(mparam->xbuf, mparam->npages * PAGE_SIZE);
> +       kfree(mparam);
> +}
> +
> +static int dma_single_map_benchmark_do_map(void *arg)
> +{
> +       struct dma_single_map_param *mparam = arg;
> +
> +       mparam->addr = dma_map_single(mparam->dev, mparam->xbuf,
> +                                     mparam->npages * PAGE_SIZE, mparam->dma_dir);
> +       if (unlikely(dma_mapping_error(mparam->dev, mparam->addr))) {
> +               pr_err("dma_map_single failed on %s\n", dev_name(mparam->dev));
> +               return -ENOMEM;
> +       }
> +
> +       return 0;
> +}
> +
> +static int dma_single_map_benchmark_do_unmap(void *arg)
> +{
> +       struct dma_single_map_param *mparam = arg;
> +
> +       dma_unmap_single(mparam->dev, mparam->addr,
> +                        mparam->npages * PAGE_SIZE, mparam->dma_dir);
> +       return 0;
> +}
> +
> +static struct map_benchmark_ops dma_single_map_benchmark_ops = {
> +       .prepare = dma_single_map_benchmark_prepare,
> +       .unprepare = dma_single_map_benchmark_unprepare,
> +       .do_map = dma_single_map_benchmark_do_map,
> +       .do_unmap = dma_single_map_benchmark_do_unmap,
> +};
> +
> +static struct map_benchmark_ops *dma_map_benchmark_ops[DMA_MAP_MODE_MAX] = {
> +       [DMA_MAP_SINGLE_MODE] = &dma_single_map_benchmark_ops,
> +};
> +
>  static int map_benchmark_thread(void *data)
>  {
> -       void *buf;
> -       dma_addr_t dma_addr;
>         struct map_benchmark_data *map = data;
> -       int npages = map->bparam.granule;
> -       u64 size = npages * PAGE_SIZE;
> +       __u8 map_mode = map->bparam.map_mode;
>         int ret = 0;
>
> -       buf = alloc_pages_exact(size, GFP_KERNEL);
> -       if (!buf)
> +       void *arg = dma_map_benchmark_ops[map_mode]->prepare(map);
> +
> +       if (!arg)
>                 return -ENOMEM;
>
>         while (!kthread_should_stop())  {
> @@ -49,23 +131,10 @@ static int map_benchmark_thread(void *data)
>                 ktime_t map_stime, map_etime, unmap_stime, unmap_etime;
>                 ktime_t map_delta, unmap_delta;
>
> -               /*
> -                * for a non-coherent device, if we don't stain them in the
> -                * cache, this will give an underestimate of the real-world
> -                * overhead of BIDIRECTIONAL or TO_DEVICE mappings;
> -                * 66 means evertything goes well! 66 is lucky.
> -                */
> -               if (map->dir != DMA_FROM_DEVICE)
> -                       memset(buf, 0x66, size);
> -
>                 map_stime = ktime_get();
> -               dma_addr = dma_map_single(map->dev, buf, size, map->dir);
> -               if (unlikely(dma_mapping_error(map->dev, dma_addr))) {
> -                       pr_err("dma_map_single failed on %s\n",
> -                               dev_name(map->dev));
> -                       ret = -ENOMEM;
> +               ret = dma_map_benchmark_ops[map_mode]->do_map(arg);
> +               if (ret)
>                         goto out;
> -               }
>                 map_etime = ktime_get();
>                 map_delta = ktime_sub(map_etime, map_stime);
>
> @@ -73,7 +142,9 @@ static int map_benchmark_thread(void *data)
>                 ndelay(map->bparam.dma_trans_ns);
>
>                 unmap_stime = ktime_get();
> -               dma_unmap_single(map->dev, dma_addr, size, map->dir);
> +               ret = dma_map_benchmark_ops[map_mode]->do_unmap(arg);
> +               if (ret)
> +                       goto out;
>                 unmap_etime = ktime_get();
>                 unmap_delta = ktime_sub(unmap_etime, unmap_stime);
>
> @@ -108,7 +179,7 @@ static int map_benchmark_thread(void *data)
>         }
>
>  out:
> -       free_pages_exact(buf, size);
> +       dma_map_benchmark_ops[map_mode]->unprepare(arg);
>         return ret;
>  }
>
> @@ -209,6 +280,11 @@ static long map_benchmark_ioctl(struct file *file, unsigned int cmd,
>
>         switch (cmd) {
>         case DMA_MAP_BENCHMARK:
> +               if (map->bparam.map_mode >= DMA_MAP_MODE_MAX) {
> +                       pr_err("invalid map mode\n");
> +                       return -EINVAL;
> +               }
> +
>                 if (map->bparam.threads == 0 ||
>                     map->bparam.threads > DMA_MAP_MAX_THREADS) {
>                         pr_err("invalid thread number\n");
> --
> 2.33.0
>

Thanks
Barry

Re: [PATCH 2/3] dma-mapping: benchmark: add support for dma_map_sg

[Barry Song]

On Wed, Feb 12, 2025 at 3:27 PM Qinxin Xia <xiaqinxin@huawei.com> wrote:
>
> Support for dma scatter-gather mapping and is intended for testing
> mapping performance. It achieves by introducing the dma_sg_map_param
> structure and related functions, which enable the implementation of
> scatter-gather mapping preparation, mapping, and unmapping operations.
> Additionally, the dma_map_benchmark_ops array is updated to include
> operations for scatter-gather mapping. This commit aims to provide
> a wider range of mapping performance test  to cater to different scenarios.
>
> Signed-off-by: Qinxin Xia <xiaqinxin@huawei.com>
> ---
>  include/linux/map_benchmark.h |   1 +
>  kernel/dma/map_benchmark.c    | 102 ++++++++++++++++++++++++++++++++++
>  2 files changed, 103 insertions(+)
>
> diff --git a/include/linux/map_benchmark.h b/include/linux/map_benchmark.h
> index 054db02a03a7..a9c1a104ba4f 100644
> --- a/include/linux/map_benchmark.h
> +++ b/include/linux/map_benchmark.h
> @@ -17,6 +17,7 @@
>
>  enum {
>         DMA_MAP_SINGLE_MODE,
> +       DMA_MAP_SG_MODE,
>         DMA_MAP_MODE_MAX
>  };
>
> diff --git a/kernel/dma/map_benchmark.c b/kernel/dma/map_benchmark.c
> index d8ec0ce058d8..b5828eeb3db7 100644
> --- a/kernel/dma/map_benchmark.c
> +++ b/kernel/dma/map_benchmark.c
> @@ -17,6 +17,7 @@
>  #include <linux/module.h>
>  #include <linux/pci.h>
>  #include <linux/platform_device.h>
> +#include <linux/scatterlist.h>
>  #include <linux/slab.h>
>  #include <linux/timekeeping.h>
>
> @@ -111,8 +112,109 @@ static struct map_benchmark_ops dma_single_map_benchmark_ops = {
>         .do_unmap = dma_single_map_benchmark_do_unmap,
>  };
>
> +struct dma_sg_map_param {
> +       struct sg_table sgt;
> +       struct device *dev;
> +       void **buf;
> +       u32 npages;
> +       u32 dma_dir;
> +};
> +
> +static void *dma_sg_map_benchmark_prepare(struct map_benchmark_data *map)
> +{
> +       struct scatterlist *sg;
> +       int i = 0;
> +
> +       struct dma_sg_map_param *mparam __free(kfree) = kzalloc(sizeof(*mparam), GFP_KERNEL);
> +       if (!mparam)
> +               return NULL;
> +
> +       mparam->npages = map->bparam.granule;

Please add comments explaining that "granule" serves as  nents in SG
mode, and that each SG entry corresponds to a single page.
Otherwise, in single mode, the granule represents what we map and
unmap as a whole in a single operation.
I mean, make the code below clearly express what you are doing:

__u32 granule;  /* how many PAGE_SIZE will do map/unmap once a time */


> +       mparam->dma_dir = map->bparam.dma_dir;
> +       mparam->dev = map->dev;
> +       mparam->buf = kmalloc_array(mparam->npages, sizeof(*mparam->buf),
> +                                   GFP_KERNEL);
> +       if (!mparam->buf)
> +               goto err1;
> +
> +       if (sg_alloc_table(&mparam->sgt, mparam->npages, GFP_KERNEL))
> +               goto err2;
> +
> +       for_each_sgtable_sg(&mparam->sgt, sg, i) {
> +               mparam->buf[i] = (void *)__get_free_page(GFP_KERNEL);
> +               if (!mparam->buf[i])
> +                       goto err3;
> +
> +               if (mparam->dma_dir != DMA_FROM_DEVICE)
> +                       memset(mparam->buf[i], 0x66, PAGE_SIZE);
> +
> +               sg_set_buf(sg, mparam->buf[i], PAGE_SIZE);
> +       }
> +
> +       return_ptr(mparam);
> +
> +err3:
> +       while (i-- > 0)
> +               free_page((unsigned long)mparam->buf[i]);
> +
> +       pr_err("dma_map_sg failed get free page on %s\n", dev_name(mparam->dev));
> +       sg_free_table(&mparam->sgt);
> +err2:
> +       pr_err("dma_map_sg failed alloc sg table on %s\n", dev_name(mparam->dev));
> +       kfree(mparam->buf);
> +err1:
> +       pr_err("dma_map_sg failed alloc mparam buf on %s\n", dev_name(mparam->dev));
> +       return NULL;

I assume allocation failures will already trigger their own warnings, so your
pr_err isn't necessary. BTW, please replace err1, err2, err3 with something
meaningful.

> +}
> +
> +static void dma_sg_map_benchmark_unprepare(void *arg)
> +{
> +       struct dma_sg_map_param *mparam = arg;
> +       int i;
> +
> +       for (i = 0; i < mparam->npages; i++)
> +               free_page((unsigned long)mparam->buf[i]);
> +
> +       sg_free_table(&mparam->sgt);
> +
> +       kfree(mparam->buf);
> +       kfree(mparam);
> +}
> +
> +static int dma_sg_map_benchmark_do_map(void *arg)
> +{
> +       struct dma_sg_map_param *mparam = arg;
> +
> +       int sg_mapped = dma_map_sg(mparam->dev, mparam->sgt.sgl,
> +                                  mparam->npages, mparam->dma_dir);
> +       if (!sg_mapped) {
> +               pr_err("dma_map_sg failed on %s\n", dev_name(mparam->dev));
> +               return -ENOMEM;
> +       }
> +
> +       return 0;
> +}
> +
> +static int dma_sg_map_benchmark_do_unmap(void *arg)

void

> +{
> +       struct dma_sg_map_param *mparam = arg;
> +
> +       dma_unmap_sg(mparam->dev, mparam->sgt.sgl, mparam->npages,
> +                    mparam->dma_dir);
> +
> +       return 0;

drop it.

> +}
> +
> +static struct map_benchmark_ops dma_sg_map_benchmark_ops = {
> +       .prepare = dma_sg_map_benchmark_prepare,
> +       .unprepare = dma_sg_map_benchmark_unprepare,
> +       .do_map = dma_sg_map_benchmark_do_map,
> +       .do_unmap = dma_sg_map_benchmark_do_unmap,
> +};
> +
>  static struct map_benchmark_ops *dma_map_benchmark_ops[DMA_MAP_MODE_MAX] = {
>         [DMA_MAP_SINGLE_MODE] = &dma_single_map_benchmark_ops,
> +       [DMA_MAP_SG_MODE] = &dma_sg_map_benchmark_ops,
>  };
>
>  static int map_benchmark_thread(void *data)
> --
> 2.33.0
>

Thanks
Barry

Re: [PATCH 1/3] dma mapping benchmark: modify the framework to adapt to more map modes

[Qinxin Xia]

在 2025/4/7 13:28, Barry Song 写道:
> On Wed, Feb 12, 2025 at 3:27 PM Qinxin Xia <xiaqinxin@huawei.com> wrote:
>> In this patch map_benchmark abstract in four interface: prepare, unprepare,
>> do_map, do_unmap. When there's a new mode to add, need four steps:
>> 1) Add the mode in map_benchmark.h
>>
>> 2) Defines the mode param, like struct dma_xxx_map_param, and this object
>>     will be return in prepare and as input parameter in other ops;
>>
>> 3) Defines the ops functions:prepare, unprepare, do_map, do_unmap.
>>
>> 4) Add the new mode in dma_map_benchmark_ops.
>>
>> Signed-off-by: Qinxin Xia <xiaqinxin@huawei.com>
>> ---
>>   include/linux/map_benchmark.h |   6 ++
>>   kernel/dma/map_benchmark.c    | 122 +++++++++++++++++++++++++++-------
>>   2 files changed, 105 insertions(+), 23 deletions(-)
>>
>> diff --git a/include/linux/map_benchmark.h b/include/linux/map_benchmark.h
>> index 62674c83bde4..054db02a03a7 100644
>> --- a/include/linux/map_benchmark.h
>> +++ b/include/linux/map_benchmark.h
>> @@ -15,6 +15,11 @@
>>   #define DMA_MAP_TO_DEVICE       1
>>   #define DMA_MAP_FROM_DEVICE     2
>>
>> +enum {
>> +       DMA_MAP_SINGLE_MODE,
>> +       DMA_MAP_MODE_MAX
>> +};
>> +
>>   struct map_benchmark {
>>          __u64 avg_map_100ns; /* average map latency in 100ns */
>>          __u64 map_stddev; /* standard deviation of map latency */
>> @@ -27,5 +32,6 @@ struct map_benchmark {
>>          __u32 dma_dir; /* DMA data direction */
>>          __u32 dma_trans_ns; /* time for DMA transmission in ns */
>>          __u32 granule;  /* how many PAGE_SIZE will do map/unmap once a time */
>> +       __u8  map_mode; /* the mode of dma map */
> We previously added some padding to ensure the uABI remained consistent.
> I just noticed that Tiantao’s commit (8ddde07a3d285a0f3cec, "dma-mapping:
> benchmark: extract a common header file for map_benchmark definition")
> accidentally removed that padding, which has completely broken the ABIs.
>
> Could you send a patch to fix this regression, and CC it to
> stable@vger.kernel.org before adding the new field which should
> use the expansion[] instead.
OK, I will send a patch to fix this regression in next version.
>
>>   };
>>   #endif /* _KERNEL_DMA_BENCHMARK_H */
>> diff --git a/kernel/dma/map_benchmark.c b/kernel/dma/map_benchmark.c
>> index cc19a3efea89..d8ec0ce058d8 100644
>> --- a/kernel/dma/map_benchmark.c
>> +++ b/kernel/dma/map_benchmark.c
>> @@ -5,6 +5,7 @@
>>
>>   #define pr_fmt(fmt)    KBUILD_MODNAME ": " fmt
>>
>> +#include <linux/cleanup.h>
>>   #include <linux/debugfs.h>
>>   #include <linux/delay.h>
>>   #include <linux/device.h>
>> @@ -31,17 +32,98 @@ struct map_benchmark_data {
>>          atomic64_t loops;
>>   };
>>
>> +struct map_benchmark_ops {
>> +       void *(*prepare)(struct map_benchmark_data *map);
>> +       void (*unprepare)(void *arg);
>> +       int (*do_map)(void *arg);
>> +       int (*do_unmap)(void *arg);
>> +};
>> +
>> +struct dma_single_map_param {
>> +       struct device *dev;
>> +       dma_addr_t addr;
>> +       void *xbuf;
>> +       u32 npages;
>> +       u32 dma_dir;
>> +};
>> +
>> +static void *dma_single_map_benchmark_prepare(struct map_benchmark_data *map)
>> +{
>> +       struct dma_single_map_param *mparam __free(kfree) = kzalloc(sizeof(*mparam),
>> +                                                                   GFP_KERNEL);
>> +       if (!mparam)
>> +               return NULL;
>> +
>> +       mparam->npages = map->bparam.granule;
>> +       mparam->dma_dir = map->bparam.dma_dir;
>> +       mparam->dev = map->dev;
>> +       mparam->xbuf = alloc_pages_exact(mparam->npages * PAGE_SIZE, GFP_KERNEL);
>> +       if (!mparam->xbuf)
>> +               return NULL;
>> +
>> +       /*
>> +        * for a non-coherent device, if we don't stain them in the
>> +        * cache, this will give an underestimate of the real-world
>> +        * overhead of BIDIRECTIONAL or TO_DEVICE mappings;
>> +        * 66 means evertything goes well! 66 is lucky.
>> +        */
>> +       if (mparam->dma_dir != DMA_FROM_DEVICE)
>> +               memset(mparam->xbuf, 0x66, mparam->npages * PAGE_SIZE);
>> +
>> +       return_ptr(mparam);
>> +}
>> +
>> +static void dma_single_map_benchmark_unprepare(void *arg)
>> +{
>> +       struct dma_single_map_param *mparam = arg;
>> +
>> +       free_pages_exact(mparam->xbuf, mparam->npages * PAGE_SIZE);
>> +       kfree(mparam);
>> +}
>> +
>> +static int dma_single_map_benchmark_do_map(void *arg)
>> +{
>> +       struct dma_single_map_param *mparam = arg;
>> +
>> +       mparam->addr = dma_map_single(mparam->dev, mparam->xbuf,
>> +                                     mparam->npages * PAGE_SIZE, mparam->dma_dir);
>> +       if (unlikely(dma_mapping_error(mparam->dev, mparam->addr))) {
>> +               pr_err("dma_map_single failed on %s\n", dev_name(mparam->dev));
>> +               return -ENOMEM;
>> +       }
>> +
>> +       return 0;
>> +}
>> +
>> +static int dma_single_map_benchmark_do_unmap(void *arg)
>> +{
>> +       struct dma_single_map_param *mparam = arg;
>> +
>> +       dma_unmap_single(mparam->dev, mparam->addr,
>> +                        mparam->npages * PAGE_SIZE, mparam->dma_dir);
>> +       return 0;
>> +}
>> +
>> +static struct map_benchmark_ops dma_single_map_benchmark_ops = {
>> +       .prepare = dma_single_map_benchmark_prepare,
>> +       .unprepare = dma_single_map_benchmark_unprepare,
>> +       .do_map = dma_single_map_benchmark_do_map,
>> +       .do_unmap = dma_single_map_benchmark_do_unmap,
>> +};
>> +
>> +static struct map_benchmark_ops *dma_map_benchmark_ops[DMA_MAP_MODE_MAX] = {
>> +       [DMA_MAP_SINGLE_MODE] = &dma_single_map_benchmark_ops,
>> +};
>> +
>>   static int map_benchmark_thread(void *data)
>>   {
>> -       void *buf;
>> -       dma_addr_t dma_addr;
>>          struct map_benchmark_data *map = data;
>> -       int npages = map->bparam.granule;
>> -       u64 size = npages * PAGE_SIZE;
>> +       __u8 map_mode = map->bparam.map_mode;
>>          int ret = 0;
>>
>> -       buf = alloc_pages_exact(size, GFP_KERNEL);
>> -       if (!buf)
>> +       void *arg = dma_map_benchmark_ops[map_mode]->prepare(map);
>> +
>> +       if (!arg)
>>                  return -ENOMEM;
>>
>>          while (!kthread_should_stop())  {
>> @@ -49,23 +131,10 @@ static int map_benchmark_thread(void *data)
>>                  ktime_t map_stime, map_etime, unmap_stime, unmap_etime;
>>                  ktime_t map_delta, unmap_delta;
>>
>> -               /*
>> -                * for a non-coherent device, if we don't stain them in the
>> -                * cache, this will give an underestimate of the real-world
>> -                * overhead of BIDIRECTIONAL or TO_DEVICE mappings;
>> -                * 66 means evertything goes well! 66 is lucky.
>> -                */
>> -               if (map->dir != DMA_FROM_DEVICE)
>> -                       memset(buf, 0x66, size);
>> -
>>                  map_stime = ktime_get();
>> -               dma_addr = dma_map_single(map->dev, buf, size, map->dir);
>> -               if (unlikely(dma_mapping_error(map->dev, dma_addr))) {
>> -                       pr_err("dma_map_single failed on %s\n",
>> -                               dev_name(map->dev));
>> -                       ret = -ENOMEM;
>> +               ret = dma_map_benchmark_ops[map_mode]->do_map(arg);
>> +               if (ret)
>>                          goto out;
>> -               }
>>                  map_etime = ktime_get();
>>                  map_delta = ktime_sub(map_etime, map_stime);
>>
>> @@ -73,7 +142,9 @@ static int map_benchmark_thread(void *data)
>>                  ndelay(map->bparam.dma_trans_ns);
>>
>>                  unmap_stime = ktime_get();
>> -               dma_unmap_single(map->dev, dma_addr, size, map->dir);
>> +               ret = dma_map_benchmark_ops[map_mode]->do_unmap(arg);
>> +               if (ret)
>> +                       goto out;
>>                  unmap_etime = ktime_get();
>>                  unmap_delta = ktime_sub(unmap_etime, unmap_stime);
>>
>> @@ -108,7 +179,7 @@ static int map_benchmark_thread(void *data)
>>          }
>>
>>   out:
>> -       free_pages_exact(buf, size);
>> +       dma_map_benchmark_ops[map_mode]->unprepare(arg);
>>          return ret;
>>   }
>>
>> @@ -209,6 +280,11 @@ static long map_benchmark_ioctl(struct file *file, unsigned int cmd,
>>
>>          switch (cmd) {
>>          case DMA_MAP_BENCHMARK:
>> +               if (map->bparam.map_mode >= DMA_MAP_MODE_MAX) {
>> +                       pr_err("invalid map mode\n");
>> +                       return -EINVAL;
>> +               }
>> +
>>                  if (map->bparam.threads == 0 ||
>>                      map->bparam.threads > DMA_MAP_MAX_THREADS) {
>>                          pr_err("invalid thread number\n");
>> --
>> 2.33.0
>>
> Thanks
> Barry

Re: [PATCH 2/3] dma-mapping: benchmark: add support for dma_map_sg

[Qinxin Xia]

在 2025/4/7 13:50, Barry Song 写道:
> On Wed, Feb 12, 2025 at 3:27 PM Qinxin Xia <xiaqinxin@huawei.com> wrote:
>> Support for dma scatter-gather mapping and is intended for testing
>> mapping performance. It achieves by introducing the dma_sg_map_param
>> structure and related functions, which enable the implementation of
>> scatter-gather mapping preparation, mapping, and unmapping operations.
>> Additionally, the dma_map_benchmark_ops array is updated to include
>> operations for scatter-gather mapping. This commit aims to provide
>> a wider range of mapping performance test  to cater to different scenarios.
>>
>> Signed-off-by: Qinxin Xia <xiaqinxin@huawei.com>
>> ---
>>   include/linux/map_benchmark.h |   1 +
>>   kernel/dma/map_benchmark.c    | 102 ++++++++++++++++++++++++++++++++++
>>   2 files changed, 103 insertions(+)
>>
>> diff --git a/include/linux/map_benchmark.h b/include/linux/map_benchmark.h
>> index 054db02a03a7..a9c1a104ba4f 100644
>> --- a/include/linux/map_benchmark.h
>> +++ b/include/linux/map_benchmark.h
>> @@ -17,6 +17,7 @@
>>
>>   enum {
>>          DMA_MAP_SINGLE_MODE,
>> +       DMA_MAP_SG_MODE,
>>          DMA_MAP_MODE_MAX
>>   };
>>
>> diff --git a/kernel/dma/map_benchmark.c b/kernel/dma/map_benchmark.c
>> index d8ec0ce058d8..b5828eeb3db7 100644
>> --- a/kernel/dma/map_benchmark.c
>> +++ b/kernel/dma/map_benchmark.c
>> @@ -17,6 +17,7 @@
>>   #include <linux/module.h>
>>   #include <linux/pci.h>
>>   #include <linux/platform_device.h>
>> +#include <linux/scatterlist.h>
>>   #include <linux/slab.h>
>>   #include <linux/timekeeping.h>
>>
>> @@ -111,8 +112,109 @@ static struct map_benchmark_ops dma_single_map_benchmark_ops = {
>>          .do_unmap = dma_single_map_benchmark_do_unmap,
>>   };
>>
>> +struct dma_sg_map_param {
>> +       struct sg_table sgt;
>> +       struct device *dev;
>> +       void **buf;
>> +       u32 npages;
>> +       u32 dma_dir;
>> +};
>> +
>> +static void *dma_sg_map_benchmark_prepare(struct map_benchmark_data *map)
>> +{
>> +       struct scatterlist *sg;
>> +       int i = 0;
>> +
>> +       struct dma_sg_map_param *mparam __free(kfree) = kzalloc(sizeof(*mparam), GFP_KERNEL);
>> +       if (!mparam)
>> +               return NULL;
>> +
>> +       mparam->npages = map->bparam.granule;
> Please add comments explaining that "granule" serves as  nents in SG
> mode, and that each SG entry corresponds to a single page.
> Otherwise, in single mode, the granule represents what we map and
> unmap as a whole in a single operation.
> I mean, make the code below clearly express what you are doing:
>
> __u32 granule;  /* how many PAGE_SIZE will do map/unmap once a time */
>
Okay, I'll add comments here in next version, thank you for your advice!
>> +       mparam->dma_dir = map->bparam.dma_dir;
>> +       mparam->dev = map->dev;
>> +       mparam->buf = kmalloc_array(mparam->npages, sizeof(*mparam->buf),
>> +                                   GFP_KERNEL);
>> +       if (!mparam->buf)
>> +               goto err1;
>> +
>> +       if (sg_alloc_table(&mparam->sgt, mparam->npages, GFP_KERNEL))
>> +               goto err2;
>> +
>> +       for_each_sgtable_sg(&mparam->sgt, sg, i) {
>> +               mparam->buf[i] = (void *)__get_free_page(GFP_KERNEL);
>> +               if (!mparam->buf[i])
>> +                       goto err3;
>> +
>> +               if (mparam->dma_dir != DMA_FROM_DEVICE)
>> +                       memset(mparam->buf[i], 0x66, PAGE_SIZE);
>> +
>> +               sg_set_buf(sg, mparam->buf[i], PAGE_SIZE);
>> +       }
>> +
>> +       return_ptr(mparam);
>> +
>> +err3:
>> +       while (i-- > 0)
>> +               free_page((unsigned long)mparam->buf[i]);
>> +
>> +       pr_err("dma_map_sg failed get free page on %s\n", dev_name(mparam->dev));
>> +       sg_free_table(&mparam->sgt);
>> +err2:
>> +       pr_err("dma_map_sg failed alloc sg table on %s\n", dev_name(mparam->dev));
>> +       kfree(mparam->buf);
>> +err1:
>> +       pr_err("dma_map_sg failed alloc mparam buf on %s\n", dev_name(mparam->dev));
>> +       return NULL;
> I assume allocation failures will already trigger their own warnings, so your
> pr_err isn't necessary. BTW, please replace err1, err2, err3 with something
> meaningful.
Okay, I'll delete these unnecessary prints and make the label changes 
more meaningful in the next version.
>> +}
>> +
>> +static void dma_sg_map_benchmark_unprepare(void *arg)
>> +{
>> +       struct dma_sg_map_param *mparam = arg;
>> +       int i;
>> +
>> +       for (i = 0; i < mparam->npages; i++)
>> +               free_page((unsigned long)mparam->buf[i]);
>> +
>> +       sg_free_table(&mparam->sgt);
>> +
>> +       kfree(mparam->buf);
>> +       kfree(mparam);
>> +}
>> +
>> +static int dma_sg_map_benchmark_do_map(void *arg)
>> +{
>> +       struct dma_sg_map_param *mparam = arg;
>> +
>> +       int sg_mapped = dma_map_sg(mparam->dev, mparam->sgt.sgl,
>> +                                  mparam->npages, mparam->dma_dir);
>> +       if (!sg_mapped) {
>> +               pr_err("dma_map_sg failed on %s\n", dev_name(mparam->dev));
>> +               return -ENOMEM;
>> +       }
>> +
>> +       return 0;
>> +}
>> +
>> +static int dma_sg_map_benchmark_do_unmap(void *arg)
> void
Initially for future expansion considerations, but I think, as you 
suggested, we could make it 'void' first.
>> +{
>> +       struct dma_sg_map_param *mparam = arg;
>> +
>> +       dma_unmap_sg(mparam->dev, mparam->sgt.sgl, mparam->npages,
>> +                    mparam->dma_dir);
>> +
>> +       return 0;
> drop it.
Okay, I'll fix it in the next version.
>> +}
>> +
>> +static struct map_benchmark_ops dma_sg_map_benchmark_ops = {
>> +       .prepare = dma_sg_map_benchmark_prepare,
>> +       .unprepare = dma_sg_map_benchmark_unprepare,
>> +       .do_map = dma_sg_map_benchmark_do_map,
>> +       .do_unmap = dma_sg_map_benchmark_do_unmap,
>> +};
>> +
>>   static struct map_benchmark_ops *dma_map_benchmark_ops[DMA_MAP_MODE_MAX] = {
>>          [DMA_MAP_SINGLE_MODE] = &dma_single_map_benchmark_ops,
>> +       [DMA_MAP_SG_MODE] = &dma_sg_map_benchmark_ops,
>>   };
>>
>>   static int map_benchmark_thread(void *data)
>> --
>> 2.33.0
>>
> Thanks
> Barry