[PATCH 0/2] Measure read while write latency

[PATCH 0/2] Measure read while write latency

[Miquel Raynal]

Macronix SPI-NOR flashes have Read-While-Write (RWW) capabilities. When programming/erasing big chunks of data, a subsequent read would take ages before being executed. If the area accessed by the read is not on the same bank of the program or erase, the read may just happen during the programming step (the bus being the bottleneck). Let's add a way to measure the latency gain with an additional flash_speed test.

By providing '-b <x> -k <y> -c <z> --destructive', reads will be attempted on blocks [y; y+z] just after starting writes on blocks [x; x+z]. The test tool gives the total time of the operation (close to making a write then a read, serially) as well as the time when the read was over. If the device does not support the RWW feature, or if the reads and writes target the same bank, the read will end last at the end of the full operation. Otherwise, when the RWW feature is leveraged, the read ends much earlier.

Example with a Macronix MX25UW51245G with bank and RWW support:

     // Testing the two accesses in the same bank
     $ flash_speed -b0 -k0 -c10 -d /dev/mtd0
     [...]
     testing read while write latency
     read while write took 51ms, read ended after 51ms

     // Testing the two accesses within different banks
     $ flash_speed -b0 -k4096 -c10 -d /dev/mtd0
     [...]
     testing read while write latency
     read while write took 51ms, read ended after 20ms

Thanks,
Miquèl

Miquel Raynal (2):
  mtd-utils: flash_speed: Rework the time counting helpers
  mtd-utils: flash_speed: Measure read while write latency

 tests/mtd-tests/Makemodule.am |   5 +-
 tests/mtd-tests/flash_speed.c | 165 ++++++++++++++++++++++++++++++----
 2 files changed, 150 insertions(+), 20 deletions(-)

-- 
2.34.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH 1/2] mtd-utils: flash_speed: Rework the time counting helpers

[Miquel Raynal]

In order to be able to have interleaved measures, let's not use the
start and finish global variables from the time helpers directly,
provide parameters for these variables so that we can provide either the
global entries, or more specific ones when relevant.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 tests/mtd-tests/flash_speed.c | 39 +++++++++++++++++++++--------------
 1 file changed, 23 insertions(+), 16 deletions(-)

diff --git a/tests/mtd-tests/flash_speed.c b/tests/mtd-tests/flash_speed.c
index 2fc70a1..035768b 100644
--- a/tests/mtd-tests/flash_speed.c
+++ b/tests/mtd-tests/flash_speed.c
@@ -258,22 +258,29 @@ static int read_eraseblock_by_2pages(int ebnum)
 	return err;
 }
 
-static void start_timing(void)
+static void start_timing(struct timespec *start)
 {
-	clock_gettime(CLOCK_MONOTONIC_RAW, &start);
+	clock_gettime(CLOCK_MONOTONIC_RAW, start);
 }
 
-static void stop_timing(void)
+static void stop_timing(struct timespec *finish)
 {
-	clock_gettime(CLOCK_MONOTONIC_RAW, &finish);
+	clock_gettime(CLOCK_MONOTONIC_RAW, finish);
 }
 
-static long calc_speed(void)
+static long calc_duration(struct timespec *start, struct timespec *finish)
 {
 	long ms;
 
-	ms = (finish.tv_sec - start.tv_sec) * 1000L;
-	ms += (finish.tv_nsec - start.tv_nsec) / 1000000L;
+	ms = (finish->tv_sec - start->tv_sec) * 1000L;
+	ms += (finish->tv_nsec - start->tv_nsec) / 1000000L;
+
+	return ms;
+}
+
+static long calc_speed(struct timespec *start, struct timespec *finish)
+{
+	long ms = calc_duration(start, finish);
 
 	if (ms <= 0)
 		return 0;
@@ -314,7 +321,7 @@ static int erase_good_eraseblocks(unsigned int eb, int ebcnt, int ebskip)
 }
 
 #define TIME_OP_PER_PEB( op )\
-		start_timing();\
+		start_timing(&start);\
 		for (i = 0; i < count; ++i) {\
 			if (bbt[i])\
 				continue;\
@@ -322,8 +329,8 @@ static int erase_good_eraseblocks(unsigned int eb, int ebcnt, int ebskip)
 			if (err)\
 				goto out;\
 		}\
-		stop_timing();\
-		speed = calc_speed()
+		stop_timing(&finish);\
+		speed = calc_speed(&start, &finish)
 
 int main(int argc, char **argv)
 {
@@ -428,12 +435,12 @@ int main(int argc, char **argv)
 	/* Erase all eraseblocks */
 	if (flags & DESTRUCTIVE) {
 		puts("Testing erase speed");
-		start_timing();
+		start_timing(&start);
 		err = erase_good_eraseblocks(peb, count, skip);
 		if (err)
 			goto out;
-		stop_timing();
-		speed = calc_speed();
+		stop_timing(&finish);
+		speed = calc_speed(&start, &finish);
 		printf("erase speed is %ld KiB/s\n", speed);
 	}
 
@@ -442,7 +449,7 @@ int main(int argc, char **argv)
 		for (k = 1; k < 7; ++k) {
 			blocks = 1 << k;
 			printf("Testing %dx multi-block erase speed\n", blocks);
-			start_timing();
+			start_timing(&start);
 			for (i = 0; i < count; ) {
 				for (j = 0; j < blocks && (i + j) < count; ++j)
 					if (bbt[i + j])
@@ -456,8 +463,8 @@ int main(int argc, char **argv)
 					goto out;
 				i += j;
 			}
-			stop_timing();
-			speed = calc_speed();
+			stop_timing(&finish);
+			speed = calc_speed(&start, &finish);
 			printf("%dx multi-block erase speed is %ld KiB/s\n",
 					blocks, speed);
 		}
-- 
2.34.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH 2/2] mtd-utils: flash_speed: Measure read while write latency

[Miquel Raynal]

The Read While Write (RWW) feature allows to perform reads from the
flash array into cache while a program (from cache) or an erase
operation happens, provided that the two areas are located on different
banks.

The main benefit is the possible reduced latency when requesting to read
a page while a much longer operation is ongoing, like a write or an
erase.

We can try to compare the positive impact of such a feature by enhancing
the flash_speed test tool with the following test:
- Measure the time taken by an eraseblock write in parallel with an
  eraseblock read.
- Measure when the read operation ends.
- Compare the two to get the latency saved with the RWW feature.

To be sure the mtd_write actually starts (and acquires the necessary
locks) before the mtd_read does, we use SCHED_FIFO at rather high
(arbitrary) priorities, respectively 42 and 41.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 tests/mtd-tests/Makemodule.am |   5 +-
 tests/mtd-tests/flash_speed.c | 126 +++++++++++++++++++++++++++++++++-
 2 files changed, 127 insertions(+), 4 deletions(-)

diff --git a/tests/mtd-tests/Makemodule.am b/tests/mtd-tests/Makemodule.am
index d849e3c..d02e9e4 100644
--- a/tests/mtd-tests/Makemodule.am
+++ b/tests/mtd-tests/Makemodule.am
@@ -7,9 +7,12 @@ flash_stress_LDADD = libmtd.a
 flash_stress_CPPFLAGS = $(AM_CPPFLAGS)
 
 flash_speed_SOURCES = tests/mtd-tests/flash_speed.c
-flash_speed_LDADD = libmtd.a
+flash_speed_LDADD = libmtd.a $(PTHREAD_LIBS)
 flash_speed_CPPFLAGS = $(AM_CPPFLAGS)
 
+flash_speed_LDADD += $(PTHREAD_CFLAGS)
+flash_speed_CPPFLAGS += $(PTHREAD_CFLAGS)
+
 nandbiterrs_SOURCES = tests/mtd-tests/nandbiterrs.c
 nandbiterrs_LDADD = libmtd.a
 nandbiterrs_CPPFLAGS = $(AM_CPPFLAGS)
diff --git a/tests/mtd-tests/flash_speed.c b/tests/mtd-tests/flash_speed.c
index 035768b..0f82047 100644
--- a/tests/mtd-tests/flash_speed.c
+++ b/tests/mtd-tests/flash_speed.c
@@ -33,6 +33,7 @@
 #include <stdlib.h>
 #include <libmtd.h>
 #include <getopt.h>
+#include <pthread.h>
 #include <stdio.h>
 #include <fcntl.h>
 #include <time.h>
@@ -46,7 +47,7 @@ static const char *mtddev;
 static libmtd_t mtd_desc;
 static int fd;
 
-static int peb=-1, count=-1, skip=-1, flags=0;
+static int peb=-1, count=-1, skip=-1, flags=0, speb=-1;
 static struct timespec start, finish;
 static int pgsize, pgcnt;
 static int goodebcnt;
@@ -57,6 +58,7 @@ static const struct option options[] = {
 	{ "peb", required_argument, NULL, 'b' },
 	{ "count", required_argument, NULL, 'c' },
 	{ "skip", required_argument, NULL, 's' },
+	{ "sec-peb", required_argument, NULL, 'k' },
 	{ NULL, 0, NULL, 0 },
 };
 
@@ -69,7 +71,8 @@ static NORETURN void usage(int status)
 	"  -b, --peb <num>     Start from this physical erase block\n"
 	"  -c, --count <num>   Number of erase blocks to use (default: all)\n"
 	"  -s, --skip <num>    Number of blocks to skip\n"
-	"  -d, --destructive   Run destructive (erase and write speed) tests\n",
+	"  -d, --destructive   Run destructive (erase and write speed) tests\n"
+	"  -k, --sec-peb <num> Start of secondary block to measure RWW latency (requires -d)\n",
 	status==EXIT_SUCCESS ? stdout : stderr);
 	exit(status);
 }
@@ -93,7 +96,7 @@ static void process_options(int argc, char **argv)
 	int c;
 
 	while (1) {
-		c = getopt_long(argc, argv, "hb:c:s:d", options, NULL);
+		c = getopt_long(argc, argv, "hb:c:s:dk:", options, NULL);
 		if (c == -1)
 			break;
 
@@ -126,6 +129,13 @@ static void process_options(int argc, char **argv)
 				goto failmulti;
 			flags |= DESTRUCTIVE;
 			break;
+		case 'k':
+			if (speb >= 0)
+				goto failmulti;
+			speb = read_num(c, optarg);
+			if (speb < 0)
+				goto failarg;
+			break;
 		default:
 			exit(EXIT_FAILURE);
 		}
@@ -144,11 +154,15 @@ static void process_options(int argc, char **argv)
 		skip = 0;
 	if (count < 0)
 		count = 1;
+	if (speb >= 0 && !(flags & DESTRUCTIVE))
+		goto faildestr;
 	return;
 failmulti:
 	errmsg_die("'-%c' specified more than once!\n", c);
 failarg:
 	errmsg_die("Invalid argument for '-%c'!\n", c);
+faildestr:
+	errmsg_die("'-k' specified, -d is missing!\n");
 }
 
 static int write_eraseblock(int ebnum)
@@ -320,6 +334,32 @@ static int erase_good_eraseblocks(unsigned int eb, int ebcnt, int ebskip)
 	return err;
 }
 
+struct thread_arg {
+	int (*op)(int peb);
+	int peb;
+	struct timespec start;
+	struct timespec finish;
+};
+
+static void *op_thread(void *ptr)
+{
+	struct thread_arg *args = ptr;
+	unsigned long err = 0;
+	int i;
+
+	start_timing(&args->start);
+	for (i = 0; i < count; ++i) {
+		if (bbt[i])
+			continue;
+		err = args->op(args->peb + i * (skip + 1));
+		if (err)
+			break;
+	}
+	stop_timing(&args->finish);
+
+	return (void *)err;
+}
+
 #define TIME_OP_PER_PEB( op )\
 		start_timing(&start);\
 		for (i = 0; i < count; ++i) {\
@@ -470,6 +510,86 @@ int main(int argc, char **argv)
 		}
 	}
 
+	/* Write a page and immediately after try to read another page. Report
+	 * the latency difference when performed on different banks (NOR only).
+	 */
+	if (speb >= 0 && mtd.subpage_size == 1) {
+		long duration_w, duration_r, rww_duration_w, rww_latency_end;
+		long rww_duration_rnw, rww_duration_r_end;
+		bool rww_r_end_first;
+		struct thread_arg write_args_peb = {
+			.op = write_eraseblock,
+			.peb = peb,
+		};
+		struct thread_arg read_args_speb = {
+			.op = read_eraseblock,
+			.peb = speb,
+		};
+		struct sched_param param_write, param_read;
+		pthread_attr_t attr_write, attr_read;
+		pthread_t write_thread, read_thread;
+		void *retval;
+
+		puts("testing read while write latency");
+
+		/* Change scheduling priorities so that the write thread gets
+		 *scheduled more aggressively than the read thread.
+		 */
+		pthread_attr_init(&attr_write);
+		pthread_attr_setinheritsched(&attr_write, PTHREAD_EXPLICIT_SCHED);
+		pthread_attr_setschedpolicy(&attr_write, SCHED_FIFO);
+		param_write.sched_priority = 42;
+		pthread_attr_setschedparam(&attr_write, &param_write);
+
+		pthread_attr_init(&attr_read);
+		pthread_attr_setinheritsched(&attr_read, PTHREAD_EXPLICIT_SCHED);
+		pthread_attr_setschedpolicy(&attr_read, SCHED_FIFO);
+		param_read.sched_priority = 41;
+		pthread_attr_setschedparam(&attr_read, &param_read);
+
+		err = pthread_create(&write_thread, &attr_write,
+				     (void *)op_thread, &write_args_peb);
+		if (err) {
+			errmsg("parallel write pthread create failed");
+			goto out;
+		}
+
+		err = pthread_create(&read_thread, &attr_read,
+				     (void *)op_thread, &read_args_speb);
+		if (err) {
+			errmsg("parallel read pthread create failed");
+			goto out;
+		}
+
+		pthread_join(read_thread, &retval);
+		if ((long)retval) {
+			errmsg("parallel read pthread failed");
+			goto out;
+		}
+
+		pthread_join(write_thread, &retval);
+		if ((long)retval) {
+			errmsg("parallel write pthread failed");
+			goto out;
+		}
+
+		rww_duration_w = calc_duration(&write_args_peb.start,
+					       &write_args_peb.finish);
+		rww_latency_end = calc_duration(&write_args_peb.finish,
+						&read_args_speb.finish);
+		rww_r_end_first = rww_latency_end < 0;
+		if (rww_r_end_first)
+			rww_duration_rnw = rww_duration_w;
+		else
+			rww_duration_rnw = calc_duration(&write_args_peb.start,
+							 &read_args_speb.finish);
+
+		rww_duration_r_end = calc_duration(&write_args_peb.start,
+						   &read_args_speb.finish);
+		printf("read while write took %ldms, read ended after %ldms\n",
+		       rww_duration_rnw, rww_duration_r_end);
+	}
+
 	puts("finished");
 	status = EXIT_SUCCESS;
 out:
-- 
2.34.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 0/2] Measure read while write latency

[David Oberhollenzer]

Applied to mtd-utils.git master.

Thanks,

David


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 0/2] Measure read while write latency

[Pratyush Yadav]

On 21/11/22 08:37AM, David Oberhollenzer wrote:
> Applied to mtd-utils.git master.

But SPI NOR does not yet support read-while-write. The patches are still 
under review [0]. So does this even work? Should mtd-utils test a 
feature that has not been merged into the driver yet? I think it should 
not.

[0] https://lore.kernel.org/linux-mtd/20221110155513.819798-1-miquel.raynal@bootlin.com/T/#mf4fdf32e77b1b21bd30ca01eed999f0efc3db6ee

> 
> Thanks,
> 
> David

-- 
Regards,
Pratyush Yadav

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 0/2] Measure read while write latency

[Miquel Raynal]

Hi Pratyush, David

pratyush@kernel.org wrote on Mon, 21 Nov 2022 10:40:20 +0100:

> On 21/11/22 08:37AM, David Oberhollenzer wrote:
> > Applied to mtd-utils.git master.  
> 
> But SPI NOR does not yet support read-while-write. The patches are still 
> under review [0]. So does this even work? Should mtd-utils test a 
> feature that has not been merged into the driver yet? I think it should 
> not.

I don't mind if the tool gets in first, it won't hurt anyway as:
- no changes are expected if people do not use the new -k option
- if they do, the gained latency will just be 0 (which is a use case of
  the tool: measuring whether or not RWW works/is supported).

> [0] https://lore.kernel.org/linux-mtd/20221110155513.819798-1-miquel.raynal@bootlin.com/T/#mf4fdf32e77b1b21bd30ca01eed999f0efc3db6ee

The only thing is: if the series does not get in for any reason, we'll
have to revert it.

Either ways, I'm fine with it.

Thanks,
Miquèl

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/