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

[PATCH v2 0/4] 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

---
Changes since V1:
- Address the comments from Masami, added some comments and changed the unmap type to void.
- Link: https://lore.kernel.org/lkml/20250212022718.1995504-1-xiaqinxin@huawei.com/

Qinxin Xia (4):
  dma-mapping: benchmark: Add padding to ensure uABI remained consistent
  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                 |  46 +++-
 kernel/dma/map_benchmark.c                    | 222 ++++++++++++++++--
 .../testing/selftests/dma/dma_map_benchmark.c |  16 +-
 3 files changed, 249 insertions(+), 35 deletions(-)

--
2.33.0

[PATCH v2 1/4] dma-mapping: benchmark: Add padding to ensure uABI remained consistent

[Qinxin Xia]

Fix a problem about commit (8ddde07a3d285a0f3cec, "dma-mapping:benchmark:
extract a common header file for map_benchmark definition") accidentally
removed that padding, which has completely broken the ABIs.

Signed-off-by: Qinxin Xia <xiaqinxin@huawei.com>
---
 include/linux/map_benchmark.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/linux/map_benchmark.h b/include/linux/map_benchmark.h
index 62674c83bde4..2ac2fe52f248 100644
--- a/include/linux/map_benchmark.h
+++ b/include/linux/map_benchmark.h
@@ -27,5 +27,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 expansion[76];     /* For future use */
 };
 #endif /* _KERNEL_DMA_BENCHMARK_H */
-- 
2.33.0

[PATCH v2 2/4] 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 |   8 ++-
 kernel/dma/map_benchmark.c    | 120 +++++++++++++++++++++++++++-------
 2 files changed, 104 insertions(+), 24 deletions(-)

diff --git a/include/linux/map_benchmark.h b/include/linux/map_benchmark.h
index 2ac2fe52f248..5294dfd1870f 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,6 +32,7 @@ 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 expansion[76];     /* For future use */
+	__u8  map_mode; /* the mode of dma map */
+	__u8 expansion[75];     /* For future use */
 };
 #endif /* _KERNEL_DMA_BENCHMARK_H */
diff --git a/kernel/dma/map_benchmark.c b/kernel/dma/map_benchmark.c
index cc19a3efea89..f04973eba1d8 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,97 @@ 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);
+	void (*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 void 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);
+}
+
+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 +130,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 +141,8 @@ 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);
+		dma_map_benchmark_ops[map_mode]->do_unmap(arg);
+
 		unmap_etime = ktime_get();
 		unmap_delta = ktime_sub(unmap_etime, unmap_stime);
 
@@ -108,7 +177,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 +278,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 v2 3/4] 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 |  43 ++++++++++----
 kernel/dma/map_benchmark.c    | 102 ++++++++++++++++++++++++++++++++++
 2 files changed, 133 insertions(+), 12 deletions(-)

diff --git a/include/linux/map_benchmark.h b/include/linux/map_benchmark.h
index 5294dfd1870f..bf8c9ade43fd 100644
--- a/include/linux/map_benchmark.h
+++ b/include/linux/map_benchmark.h
@@ -17,22 +17,41 @@
 
 enum {
 	DMA_MAP_SINGLE_MODE,
+	DMA_MAP_SG_MODE,
 	DMA_MAP_MODE_MAX
 };
 
+/**
+ * struct map_benchmark - Benchmarking data for DMA mapping operations.
+ * @avg_map_100ns: Average map latency in 100ns.
+ * @map_stddev: Standard deviation of map latency.
+ * @avg_unmap_100ns: Average unmap latency in 100ns.
+ * @unmap_stddev: Standard deviation of unmap latency.
+ * @threads: Number of threads performing map/unmap operations in parallel.
+ * @seconds: Duration of the test in seconds.
+ * @node: NUMA node on which this benchmark will run.
+ * @dma_bits: DMA addressing capability.
+ * @dma_dir: DMA data direction.
+ * @dma_trans_ns: Time for DMA transmission in ns.
+ * @granule: Number of PAGE_SIZE units to map/unmap at once.
+	     In SG mode, this represents the number of scatterlist entries.
+	     In single mode, this represents the total size of the mapping.
+ * @map_mode: Mode of DMA mapping.
+ * @expansion: Reserved for future use.
+ */
 struct map_benchmark {
-	__u64 avg_map_100ns; /* average map latency in 100ns */
-	__u64 map_stddev; /* standard deviation of map latency */
-	__u64 avg_unmap_100ns; /* as above */
+	__u64 avg_map_100ns;
+	__u64 map_stddev;
+	__u64 avg_unmap_100ns;
 	__u64 unmap_stddev;
-	__u32 threads; /* how many threads will do map/unmap in parallel */
-	__u32 seconds; /* how long the test will last */
-	__s32 node; /* which numa node this benchmark will run on */
-	__u32 dma_bits; /* DMA addressing capability */
-	__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 */
-	__u8 expansion[75];     /* For future use */
+	__u32 threads;
+	__u32 seconds;
+	__s32 node;
+	__u32 dma_bits;
+	__u32 dma_dir;
+	__u32 dma_trans_ns;
+	__u32 granule;
+	__u8  map_mode;
+	__u8 expansion[75];
 };
 #endif /* _KERNEL_DMA_BENCHMARK_H */
diff --git a/kernel/dma/map_benchmark.c b/kernel/dma/map_benchmark.c
index f04973eba1d8..72cd2c5f839e 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>
 
@@ -110,8 +111,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;
+
+	/*
+	 * Set the number of scatterlist entries based on the granule.
+	 * In SG mode, 'granule' represents the number of scatterlist entries.
+	 * Each scatterlist entry corresponds to a single page.
+	 */
+	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 out;
+
+	if (sg_alloc_table(&mparam->sgt, mparam->npages, GFP_KERNEL))
+		goto free_buf;
+
+	for_each_sgtable_sg(&mparam->sgt, sg, i) {
+		mparam->buf[i] = (void *)__get_free_page(GFP_KERNEL);
+		if (!mparam->buf[i])
+			goto free_page;
+
+		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);
+
+free_page:
+	while (i-- > 0)
+		free_page((unsigned long)mparam->buf[i]);
+
+	sg_free_table(&mparam->sgt);
+free_buf:
+	kfree(mparam->buf);
+out:
+	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 void 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);
+}
+
+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 v2 4/4] 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 v2 0/4] dma mapping benchmark: add support for dma_map_sg

[ALOK TIWARI]

On 06-05-2025 08:30, Qinxin Xia wrote:
> 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
> 
> ---
> Changes since V1:
> - Address the comments from Masami, added some comments and changed the unmap type to void.
> - Link: https://urldefense.com/v3/__https://lore.kernel.org/lkml/20250212022718.1995504-1-xiaqinxin@huawei.com/__;!!ACWV5N9M2RV99hQ!PZwCK7yspP9AzXLsodEKNp6_pWU8ir6IKfsJQJIpvfLQdOrNgjLpwNySORdXdT1JJFAbo82SMmIlnnYI3tSpGQ$
> 
> Qinxin Xia (4):
>    dma-mapping: benchmark: Add padding to ensure uABI remained consistent
>    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

I noticed the subject lines use both "dma-mapping" and "dma mapping" ?
Could we make them consistent?
> 
>   include/linux/map_benchmark.h                 |  46 +++-
>   kernel/dma/map_benchmark.c                    | 222 ++++++++++++++++--
>   .../testing/selftests/dma/dma_map_benchmark.c |  16 +-
>   3 files changed, 249 insertions(+), 35 deletions(-)
> 
> --
> 2.33.0
> 
> 

Thanks,
Alok

Re: [PATCH v2 0/4] dma mapping benchmark: add support for dma_map_sg

[Qinxin Xia]

Okay, I'll fix this in the next version.

在 2025/5/6 20:20, ALOK TIWARI 写道:
> Could we make them consistent

Re: [PATCH v2 1/4] dma-mapping: benchmark: Add padding to ensure uABI remained consistent

[Barry Song]

On Tue, May 6, 2025 at 3:01 PM Qinxin Xia <xiaqinxin@huawei.com> wrote:
>
> Fix a problem about commit (8ddde07a3d285a0f3cec, "dma-mapping:benchmark:
> extract a common header file for map_benchmark definition") accidentally
> removed that padding, which has completely broken the ABIs.
>
> Signed-off-by: Qinxin Xia <xiaqinxin@huawei.com>

Please add Fixes tags and cc stable.

> ---
>  include/linux/map_benchmark.h | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/include/linux/map_benchmark.h b/include/linux/map_benchmark.h
> index 62674c83bde4..2ac2fe52f248 100644
> --- a/include/linux/map_benchmark.h
> +++ b/include/linux/map_benchmark.h
> @@ -27,5 +27,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 expansion[76];     /* For future use */
>  };
>  #endif /* _KERNEL_DMA_BENCHMARK_H */
> --
> 2.33.0
>

Thanks
Barry

Re: [PATCH v2 1/4] dma-mapping: benchmark: Add padding to ensure uABI remained consistent

[Barry Song]

On Wed, May 7, 2025 at 1:52 PM Barry Song <21cnbao@gmail.com> wrote:
>
> On Tue, May 6, 2025 at 3:01 PM Qinxin Xia <xiaqinxin@huawei.com> wrote:
> >
> > Fix a problem about commit (8ddde07a3d285a0f3cec, "dma-mapping:benchmark:
> > extract a common header file for map_benchmark definition") accidentally
> > removed that padding, which has completely broken the ABIs.
> >
> > Signed-off-by: Qinxin Xia <xiaqinxin@huawei.com>
>
> Please add Fixes tags and cc stable.

We are *NOT* adding any field.

Also, the subject and changelog are not appropriate. They should be something
like the following:

Previously, we had a padding field in this structure to allow for future
extension without breaking compatibility with user-space shared data
structures. In other words, the padding was reserved to maintain a stable
interface for potential new fields.

However, in one of the commits, tiantao accidentally removed this padding,
which could lead to incompatibility issues between user space and the
kernel.

This patch restores the padding to bring back the original structure layout
and ensure compatibility is preserved.

>
> > ---
> >  include/linux/map_benchmark.h | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/include/linux/map_benchmark.h b/include/linux/map_benchmark.h
> > index 62674c83bde4..2ac2fe52f248 100644
> > --- a/include/linux/map_benchmark.h
> > +++ b/include/linux/map_benchmark.h
> > @@ -27,5 +27,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 expansion[76];     /* For future use */
> >  };
> >  #endif /* _KERNEL_DMA_BENCHMARK_H */
> > --
> > 2.33.0
> >
>
Thanks
Barry

Re: [PATCH v2 2/4] dma mapping benchmark: modify the framework to adapt to more map modes

[Barry Song]

On Tue, May 6, 2025 at 3:01 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.

There are too many irrelevant details.

We're more interested in the reasoning behind this change than just the change
itself. There should be some explanation of the purpose.

>
> Signed-off-by: Qinxin Xia <xiaqinxin@huawei.com>
> ---
>  include/linux/map_benchmark.h |   8 ++-
>  kernel/dma/map_benchmark.c    | 120 +++++++++++++++++++++++++++-------
>  2 files changed, 104 insertions(+), 24 deletions(-)
>
> diff --git a/include/linux/map_benchmark.h b/include/linux/map_benchmark.h
> index 2ac2fe52f248..5294dfd1870f 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,6 +32,7 @@ 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 expansion[76];     /* For future use */
> +       __u8  map_mode; /* the mode of dma map */
> +       __u8 expansion[75];     /* For future use */
>  };
>  #endif /* _KERNEL_DMA_BENCHMARK_H */
> diff --git a/kernel/dma/map_benchmark.c b/kernel/dma/map_benchmark.c
> index cc19a3efea89..f04973eba1d8 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,97 @@ 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);
> +       void (*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 void 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);
> +}
> +
> +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);

It's a bit awkward. Let's try something like this instead:

struct map_benchmark_ops *mb_ops = &dma_map_benchmark_ops[map_mode];

then
mb_ops->prepare()/map/unmap/unprepare etc.

And can you find a better name than "arg" which is meaningless, mparam?

> +
> +       if (!arg)
>                 return -ENOMEM;
>
>         while (!kthread_should_stop())  {
> @@ -49,23 +130,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 +141,8 @@ 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);
> +               dma_map_benchmark_ops[map_mode]->do_unmap(arg);
> +
>                 unmap_etime = ktime_get();
>                 unmap_delta = ktime_sub(unmap_etime, unmap_stime);
>
> @@ -108,7 +177,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 +278,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 v2 3/4] dma-mapping: benchmark: add support for dma_map_sg

[Barry Song]

On Tue, May 6, 2025 at 3:01 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 |  43 ++++++++++----
>  kernel/dma/map_benchmark.c    | 102 ++++++++++++++++++++++++++++++++++
>  2 files changed, 133 insertions(+), 12 deletions(-)
>
> diff --git a/include/linux/map_benchmark.h b/include/linux/map_benchmark.h
> index 5294dfd1870f..bf8c9ade43fd 100644
> --- a/include/linux/map_benchmark.h
> +++ b/include/linux/map_benchmark.h
> @@ -17,22 +17,41 @@
>
>  enum {
>         DMA_MAP_SINGLE_MODE,
> +       DMA_MAP_SG_MODE,
>         DMA_MAP_MODE_MAX
>  };
>
> +/**
> + * struct map_benchmark - Benchmarking data for DMA mapping operations.
> + * @avg_map_100ns: Average map latency in 100ns.
> + * @map_stddev: Standard deviation of map latency.
> + * @avg_unmap_100ns: Average unmap latency in 100ns.
> + * @unmap_stddev: Standard deviation of unmap latency.
> + * @threads: Number of threads performing map/unmap operations in parallel.
> + * @seconds: Duration of the test in seconds.
> + * @node: NUMA node on which this benchmark will run.
> + * @dma_bits: DMA addressing capability.
> + * @dma_dir: DMA data direction.
> + * @dma_trans_ns: Time for DMA transmission in ns.
> + * @granule: Number of PAGE_SIZE units to map/unmap at once.
> +            In SG mode, this represents the number of scatterlist entries.
> +            In single mode, this represents the total size of the mapping.
> + * @map_mode: Mode of DMA mapping.
> + * @expansion: Reserved for future use.
> + */
>  struct map_benchmark {
> -       __u64 avg_map_100ns; /* average map latency in 100ns */
> -       __u64 map_stddev; /* standard deviation of map latency */
> -       __u64 avg_unmap_100ns; /* as above */
> +       __u64 avg_map_100ns;
> +       __u64 map_stddev;
> +       __u64 avg_unmap_100ns;
>         __u64 unmap_stddev;
> -       __u32 threads; /* how many threads will do map/unmap in parallel */
> -       __u32 seconds; /* how long the test will last */
> -       __s32 node; /* which numa node this benchmark will run on */
> -       __u32 dma_bits; /* DMA addressing capability */
> -       __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 */
> -       __u8 expansion[75];     /* For future use */
> +       __u32 threads;
> +       __u32 seconds;
> +       __s32 node;
> +       __u32 dma_bits;
> +       __u32 dma_dir;
> +       __u32 dma_trans_ns;
> +       __u32 granule;
> +       __u8  map_mode;
> +       __u8 expansion[75];
>  };
>  #endif /* _KERNEL_DMA_BENCHMARK_H */
> diff --git a/kernel/dma/map_benchmark.c b/kernel/dma/map_benchmark.c
> index f04973eba1d8..72cd2c5f839e 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>
>
> @@ -110,8 +111,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;

I'm not sure we need `i = 0`; perhaps just `int i` would be sufficient.

> +
> +       struct dma_sg_map_param *mparam __free(kfree) = kzalloc(sizeof(*mparam), GFP_KERNEL);
> +       if (!mparam)
> +               return NULL;
> +
> +       /*
> +        * Set the number of scatterlist entries based on the granule.
> +        * In SG mode, 'granule' represents the number of scatterlist entries.
> +        * Each scatterlist entry corresponds to a single page.
> +        */
> +       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 out;
> +
> +       if (sg_alloc_table(&mparam->sgt, mparam->npages, GFP_KERNEL))
> +               goto free_buf;
> +
> +       for_each_sgtable_sg(&mparam->sgt, sg, i) {
> +               mparam->buf[i] = (void *)__get_free_page(GFP_KERNEL);
> +               if (!mparam->buf[i])
> +                       goto free_page;
> +
> +               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);
> +
> +free_page:
> +       while (i-- > 0)
> +               free_page((unsigned long)mparam->buf[i]);
> +
> +       sg_free_table(&mparam->sgt);
> +free_buf:
> +       kfree(mparam->buf);
> +out:
> +       return NULL;
> +}
> +
> +static void dma_sg_map_benchmark_unprepare(void *arg)

The term "arg" is meaningless for a function argument, as it's already implied
that it is an argument.

> +{
> +       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);

int ret.

> +       if (!sg_mapped) {
> +               pr_err("dma_map_sg failed on %s\n", dev_name(mparam->dev));
> +               return -ENOMEM;
> +       }
> +
> +       return 0;
> +}
> +
> +static void 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);
> +}
> +
> +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 v2 4/4] dma mapping benchmark:add support for dma_map_sg

[Barry Song]

On Tue, May 6, 2025 at 3:01 PM Qinxin Xia <xiaqinxin@huawei.com> wrote:
>
> 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>

Either merge this into the previous patch or use the correct prefix in the
subject to differentiate patch 3 from patch 4. For example: selftests/dma

> ---
>  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
>

Thanks
Barry

Re: [PATCH v2 1/4] dma-mapping: benchmark: Add padding to ensure uABI remained consistent

[Qinxin Xia]

在 2025/5/7 9:52, Barry Song 写道:
> On Tue, May 6, 2025 at 3:01 PM Qinxin Xia <xiaqinxin@huawei.com> wrote:
>> Fix a problem about commit (8ddde07a3d285a0f3cec, "dma-mapping:benchmark:
>> extract a common header file for map_benchmark definition") accidentally
>> removed that padding, which has completely broken the ABIs.
>>
>> Signed-off-by: Qinxin Xia <xiaqinxin@huawei.com>
> Please add Fixes tags and cc stable.

Okay, I'll fix it in the next version.

Thanks!

>> ---
>>   include/linux/map_benchmark.h | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/include/linux/map_benchmark.h b/include/linux/map_benchmark.h
>> index 62674c83bde4..2ac2fe52f248 100644
>> --- a/include/linux/map_benchmark.h
>> +++ b/include/linux/map_benchmark.h
>> @@ -27,5 +27,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 expansion[76];     /* For future use */
>>   };
>>   #endif /* _KERNEL_DMA_BENCHMARK_H */
>> --
>> 2.33.0
>>
> Thanks
> Barry

Re: [PATCH v2 1/4] dma-mapping: benchmark: Add padding to ensure uABI remained consistent

[Qinxin Xia]

在 2025/5/7 10:08, Barry Song 写道:
> On Wed, May 7, 2025 at 1:52 PM Barry Song <21cnbao@gmail.com> wrote:
>> On Tue, May 6, 2025 at 3:01 PM Qinxin Xia <xiaqinxin@huawei.com> wrote:
>>> Fix a problem about commit (8ddde07a3d285a0f3cec, "dma-mapping:benchmark:
>>> extract a common header file for map_benchmark definition") accidentally
>>> removed that padding, which has completely broken the ABIs.
>>>
>>> Signed-off-by: Qinxin Xia <xiaqinxin@huawei.com>
>> Please add Fixes tags and cc stable.
> We are *NOT* adding any field.
>
> Also, the subject and changelog are not appropriate. They should be something
> like the following:
>
> Previously, we had a padding field in this structure to allow for future
> extension without breaking compatibility with user-space shared data
> structures. In other words, the padding was reserved to maintain a stable
> interface for potential new fields.
>
> However, in one of the commits, tiantao accidentally removed this padding,
> which could lead to incompatibility issues between user space and the
> kernel.
>
> This patch restores the padding to bring back the original structure layout
> and ensure compatibility is preserved.
Okay, thank you for your advice.
>>> ---
>>>   include/linux/map_benchmark.h | 1 +
>>>   1 file changed, 1 insertion(+)
>>>
>>> diff --git a/include/linux/map_benchmark.h b/include/linux/map_benchmark.h
>>> index 62674c83bde4..2ac2fe52f248 100644
>>> --- a/include/linux/map_benchmark.h
>>> +++ b/include/linux/map_benchmark.h
>>> @@ -27,5 +27,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 expansion[76];     /* For future use */
>>>   };
>>>   #endif /* _KERNEL_DMA_BENCHMARK_H */
>>> --
>>> 2.33.0
>>>
> Thanks
> Barry

Re: [PATCH v2 2/4] dma mapping benchmark: modify the framework to adapt to more map modes

[Qinxin Xia]

在 2025/5/7 10:30, Barry Song 写道:
> On Tue, May 6, 2025 at 3:01 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.
> There are too many irrelevant details.
>
> We're more interested in the reasoning behind this change than just the change
> itself. There should be some explanation of the purpose.
Okay, I'm going to add some purposeful commit information.
>> Signed-off-by: Qinxin Xia <xiaqinxin@huawei.com>
>> ---
>>   include/linux/map_benchmark.h |   8 ++-
>>   kernel/dma/map_benchmark.c    | 120 +++++++++++++++++++++++++++-------
>>   2 files changed, 104 insertions(+), 24 deletions(-)
>>
>> diff --git a/include/linux/map_benchmark.h b/include/linux/map_benchmark.h
>> index 2ac2fe52f248..5294dfd1870f 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,6 +32,7 @@ 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 expansion[76];     /* For future use */
>> +       __u8  map_mode; /* the mode of dma map */
>> +       __u8 expansion[75];     /* For future use */
>>   };
>>   #endif /* _KERNEL_DMA_BENCHMARK_H */
>> diff --git a/kernel/dma/map_benchmark.c b/kernel/dma/map_benchmark.c
>> index cc19a3efea89..f04973eba1d8 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,97 @@ 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);
>> +       void (*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 void 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);
>> +}
>> +
>> +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);
> It's a bit awkward. Let's try something like this instead:
>
> struct map_benchmark_ops *mb_ops = &dma_map_benchmark_ops[map_mode];
>
> then
> mb_ops->prepare()/map/unmap/unprepare etc.
>
> And can you find a better name than "arg" which is meaningless, mparam?

OK, I'll change it in the next version according to your suggestion.

Thanks!

>> +
>> +       if (!arg)
>>                  return -ENOMEM;
>>
>>          while (!kthread_should_stop())  {
>> @@ -49,23 +130,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 +141,8 @@ 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);
>> +               dma_map_benchmark_ops[map_mode]->do_unmap(arg);
>> +
>>                  unmap_etime = ktime_get();
>>                  unmap_delta = ktime_sub(unmap_etime, unmap_stime);
>>
>> @@ -108,7 +177,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 +278,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 v2 3/4] dma-mapping: benchmark: add support for dma_map_sg

[Qinxin Xia]

在 2025/5/7 10:39, Barry Song 写道:
> On Tue, May 6, 2025 at 3:01 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 |  43 ++++++++++----
>>   kernel/dma/map_benchmark.c    | 102 ++++++++++++++++++++++++++++++++++
>>   2 files changed, 133 insertions(+), 12 deletions(-)
>>
>> diff --git a/include/linux/map_benchmark.h b/include/linux/map_benchmark.h
>> index 5294dfd1870f..bf8c9ade43fd 100644
>> --- a/include/linux/map_benchmark.h
>> +++ b/include/linux/map_benchmark.h
>> @@ -17,22 +17,41 @@
>>
>>   enum {
>>          DMA_MAP_SINGLE_MODE,
>> +       DMA_MAP_SG_MODE,
>>          DMA_MAP_MODE_MAX
>>   };
>>
>> +/**
>> + * struct map_benchmark - Benchmarking data for DMA mapping operations.
>> + * @avg_map_100ns: Average map latency in 100ns.
>> + * @map_stddev: Standard deviation of map latency.
>> + * @avg_unmap_100ns: Average unmap latency in 100ns.
>> + * @unmap_stddev: Standard deviation of unmap latency.
>> + * @threads: Number of threads performing map/unmap operations in parallel.
>> + * @seconds: Duration of the test in seconds.
>> + * @node: NUMA node on which this benchmark will run.
>> + * @dma_bits: DMA addressing capability.
>> + * @dma_dir: DMA data direction.
>> + * @dma_trans_ns: Time for DMA transmission in ns.
>> + * @granule: Number of PAGE_SIZE units to map/unmap at once.
>> +            In SG mode, this represents the number of scatterlist entries.
>> +            In single mode, this represents the total size of the mapping.
>> + * @map_mode: Mode of DMA mapping.
>> + * @expansion: Reserved for future use.
>> + */
>>   struct map_benchmark {
>> -       __u64 avg_map_100ns; /* average map latency in 100ns */
>> -       __u64 map_stddev; /* standard deviation of map latency */
>> -       __u64 avg_unmap_100ns; /* as above */
>> +       __u64 avg_map_100ns;
>> +       __u64 map_stddev;
>> +       __u64 avg_unmap_100ns;
>>          __u64 unmap_stddev;
>> -       __u32 threads; /* how many threads will do map/unmap in parallel */
>> -       __u32 seconds; /* how long the test will last */
>> -       __s32 node; /* which numa node this benchmark will run on */
>> -       __u32 dma_bits; /* DMA addressing capability */
>> -       __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 */
>> -       __u8 expansion[75];     /* For future use */
>> +       __u32 threads;
>> +       __u32 seconds;
>> +       __s32 node;
>> +       __u32 dma_bits;
>> +       __u32 dma_dir;
>> +       __u32 dma_trans_ns;
>> +       __u32 granule;
>> +       __u8  map_mode;
>> +       __u8 expansion[75];
>>   };
>>   #endif /* _KERNEL_DMA_BENCHMARK_H */
>> diff --git a/kernel/dma/map_benchmark.c b/kernel/dma/map_benchmark.c
>> index f04973eba1d8..72cd2c5f839e 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>
>>
>> @@ -110,8 +111,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;
> I'm not sure we need `i = 0`; perhaps just `int i` would be sufficient.
You're right, 'int i' is more appropriate.
>
>> +
>> +       struct dma_sg_map_param *mparam __free(kfree) = kzalloc(sizeof(*mparam), GFP_KERNEL);
>> +       if (!mparam)
>> +               return NULL;
>> +
>> +       /*
>> +        * Set the number of scatterlist entries based on the granule.
>> +        * In SG mode, 'granule' represents the number of scatterlist entries.
>> +        * Each scatterlist entry corresponds to a single page.
>> +        */
>> +       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 out;
>> +
>> +       if (sg_alloc_table(&mparam->sgt, mparam->npages, GFP_KERNEL))
>> +               goto free_buf;
>> +
>> +       for_each_sgtable_sg(&mparam->sgt, sg, i) {
>> +               mparam->buf[i] = (void *)__get_free_page(GFP_KERNEL);
>> +               if (!mparam->buf[i])
>> +                       goto free_page;
>> +
>> +               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);
>> +
>> +free_page:
>> +       while (i-- > 0)
>> +               free_page((unsigned long)mparam->buf[i]);
>> +
>> +       sg_free_table(&mparam->sgt);
>> +free_buf:
>> +       kfree(mparam->buf);
>> +out:
>> +       return NULL;
>> +}
>> +
>> +static void dma_sg_map_benchmark_unprepare(void *arg)
> The term "arg" is meaningless for a function argument, as it's already implied
> that it is an argument.
Okay, I'm going to modify this in the next version.
>> +{
>> +       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);
> int ret.

Okay, I'm going to modify this in the next version.

Thanks!

>> +       if (!sg_mapped) {
>> +               pr_err("dma_map_sg failed on %s\n", dev_name(mparam->dev));
>> +               return -ENOMEM;
>> +       }
>> +
>> +       return 0;
>> +}
>> +
>> +static void 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);
>> +}
>> +
>> +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 v2 4/4] dma mapping benchmark:add support for dma_map_sg

[Qinxin Xia]

在 2025/5/7 10:44, Barry Song 写道:
> On Tue, May 6, 2025 at 3:01 PM Qinxin Xia <xiaqinxin@huawei.com> wrote:
>> 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>
> Either merge this into the previous patch or use the correct prefix in the
> subject to differentiate patch 3 from patch 4. For example: selftests/dma

Okay, I'll fix this in the next version.

Thanks!

>> ---
>>   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
>>
> Thanks
> Barry