linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH] mtd: tests: Fix eraseblock read speed miscalculation for lower partition sizes
@ 2022-02-03 13:21 Amit Kumar Mahapatra
  0 siblings, 0 replies; 4+ messages in thread
From: Amit Kumar Mahapatra @ 2022-02-03 13:21 UTC (permalink / raw)
  To: miquel.raynal, richard, vigneshr; +Cc: linux-mtd, linux-kernel, git

While calculating speed during  mtd_speedtest, the time interval
(i.e., start - finish) is rounded off to the nearest milliseconds by
ignoring the fractional part. This leads to miscalculation of speed.
The miscalculation is more visible while running speed test on small
partition sizes(i.e., when partition size is equal to eraseblock size or
twice the eraseblock size) at higher spi frequencies.

For e.g., while calculating eraseblock read speed for a mtd partition with
size equal to the eraseblock size(i.e., 64KiB) the eraseblock read time
interval comes out to be 966490 nanosecond. This is then converted to
millisecond(i.e., 0.966 msec.). The integer part (i.e., 0 msec) of the
value is considered and the fractional part (i.e., 0.966) is ignored,for
calculating the eraseblock read speed. So the reported eraseblock read
speed is 0 KiB/s, which is incorrect.

There are two approaches to fix this issue.

First approach will be to keep the time interval in millisecond. and round
up the integer value, with this approach the 0.966msec time interval in the
above example will be rounded up to 1msec and this value is used for
calculating the speed. Downside of this approach is that the reported speed
is still not accurate.

Second approach will be to convert the time interval to microseconds
instead of milliseconds, with this approach the 966490 nanosecond time
interval in the above example will be converted t0 966.490usec and this
value is used for calculating the speed. As compared to the current
implementation and the suggested First approach, this approach will report
a more accurate speed. Downside of this approach is that, in future if the
mtd size is too large then the u64 variable, that holds the number of
bytes, might overflow.

In this patch we have gone with the second approach as this reports a more
accurate speed. With this approach the eraseblock read speed in the above
example comes out to be 132505 KiB/s when the spi clock is configured at
150Mhz.

Signed-off-by: Amit Kumar Mahapatra <amit.kumar-mahapatra@xilinx.com>
---
 drivers/mtd/tests/speedtest.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/mtd/tests/speedtest.c b/drivers/mtd/tests/speedtest.c
index 93e76648f676..2b76e7750c68 100644
--- a/drivers/mtd/tests/speedtest.c
+++ b/drivers/mtd/tests/speedtest.c
@@ -161,13 +161,13 @@ static inline void stop_timing(void)
 static long calc_speed(void)
 {
 	uint64_t k;
-	long ms;
+	long us;
 
-	ms = ktime_ms_delta(finish, start);
-	if (ms == 0)
+	us = ktime_us_delta(finish, start);
+	if (us == 0)
 		return 0;
-	k = (uint64_t)goodebcnt * (mtd->erasesize / 1024) * 1000;
-	do_div(k, ms);
+	k = (uint64_t)goodebcnt * (mtd->erasesize / 1024) * 1000000;
+	do_div(k, us);
 	return k;
 }
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* RE: [RFC PATCH] mtd: tests: Fix eraseblock read speed miscalculation for lower partition sizes
  2022-02-03 16:56 ` Miquel Raynal
@ 2022-02-08  5:24   ` Amit Kumar Kumar Mahapatra
  0 siblings, 0 replies; 4+ messages in thread
From: Amit Kumar Kumar Mahapatra @ 2022-02-08  5:24 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: richard, vigneshr, linux-mtd, linux-kernel, git, David Oberhollenzer

Hello Miquel,


> -----Original Message-----
> From: Miquel Raynal <miquel.raynal@bootlin.com>
> Sent: Thursday, February 3, 2022 10:26 PM
> To: Amit Kumar Kumar Mahapatra <akumarma@xilinx.com>
> Cc: richard@nod.at; vigneshr@ti.com; linux-mtd@lists.infradead.org; linux-
> kernel@vger.kernel.org; git <git@xilinx.com>; David Oberhollenzer
> <david.oberhollenzer@sigma-star.at>
> Subject: Re: [RFC PATCH] mtd: tests: Fix eraseblock read speed miscalculation
> for lower partition sizes
> 
> Hi Amit,
> 
> +Cc: David, who's maintaining the tools. Please keep him in the
> recipients list!
> 
> amit.kumar-mahapatra@xilinx.com wrote on Thu, 3 Feb 2022 18:54:34
> +0530:
> 
> > While calculating speed during  mtd_speedtest, the time interval
> > (i.e., start - finish) is rounded off to the nearest milliseconds by
> > ignoring the fractional part. This leads to miscalculation of speed.
> > The miscalculation is more visible while running speed test on small
> > partition sizes(i.e., when partition size is equal to eraseblock size
> > or twice the eraseblock size) at higher spi frequencies.
> >
> > For e.g., while calculating eraseblock read speed for a mtd partition
> > with size equal to the eraseblock size(i.e., 64KiB) the eraseblock
> > read time interval comes out to be 966490 nanosecond. This is then
> > converted to millisecond(i.e., 0.966 msec.). The integer part (i.e., 0
> > msec) of the value is considered and the fractional part (i.e., 0.966)
> > is ignored,for calculating the eraseblock read speed. So the reported
> > eraseblock read speed is 0 KiB/s, which is incorrect.
> >
> > There are two approaches to fix this issue.
> >
> > First approach will be to keep the time interval in millisecond. and
> > round up the integer value, with this approach the 0.966msec time
> > interval in the above example will be rounded up to 1msec and this
> > value is used for calculating the speed. Downside of this approach is
> > that the reported speed is still not accurate.
> >
> > Second approach will be to convert the time interval to microseconds
> > instead of milliseconds, with this approach the 966490 nanosecond time
> > interval in the above example will be converted t0 966.490usec and
> > this value is used for calculating the speed. As compared to the
> > current implementation and the suggested First approach, this approach
> > will report a more accurate speed. Downside of this approach is that,
> > in future if the mtd size is too large then the u64 variable, that
> > holds the number of bytes, might overflow.
> >
> > In this patch we have gone with the second approach as this reports a
> > more accurate speed. With this approach the eraseblock read speed in
> > the above example comes out to be 132505 KiB/s when the spi clock is
> > configured at 150Mhz.
> >
> > Signed-off-by: Amit Kumar Mahapatra <amit.kumar-
> mahapatra@xilinx.com>
> > ---
> > BRANCH: mtd/next
> > ---
> >  drivers/mtd/tests/speedtest.c | 10 +++++-----
> >  1 file changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/mtd/tests/speedtest.c
> > b/drivers/mtd/tests/speedtest.c index 93e76648f676..2b76e7750c68
> > 100644
> > --- a/drivers/mtd/tests/speedtest.c
> > +++ b/drivers/mtd/tests/speedtest.c
> > @@ -161,13 +161,13 @@ static inline void stop_timing(void)  static
> > long calc_speed(void)  {
> >  	uint64_t k;
> > -	long ms;
> > +	long us;
> 
> Should this be an explicit 64-bit value? And unsigned?
> unsigned long long int or uint64_t? I believe we are now 1000x closer
> to the 4GiB limit so we might need to enlarge this variable.

I will change it to uint64_t.

Regards,
Amit
> 
> >
> > -	ms = ktime_ms_delta(finish, start);
> > -	if (ms == 0)
> > +	us = ktime_us_delta(finish, start);
> > +	if (us == 0)
> >  		return 0;
> > -	k = (uint64_t)goodebcnt * (mtd->erasesize / 1024) * 1000;
> > -	do_div(k, ms);
> > +	k = (uint64_t)goodebcnt * (mtd->erasesize / 1024) * 1000000;
> > +	do_div(k, us);
> >  	return k;
> >  }
> >
> 
> Otherwise lgtm!
> 
> Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
> 
> 
> Thanks,
> Miquèl

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [RFC PATCH] mtd: tests: Fix eraseblock read speed miscalculation for lower partition sizes
  2022-02-03 13:24 Amit Kumar Mahapatra
@ 2022-02-03 16:56 ` Miquel Raynal
  2022-02-08  5:24   ` Amit Kumar Kumar Mahapatra
  0 siblings, 1 reply; 4+ messages in thread
From: Miquel Raynal @ 2022-02-03 16:56 UTC (permalink / raw)
  To: Amit Kumar Mahapatra
  Cc: richard, vigneshr, linux-mtd, linux-kernel, git, David Oberhollenzer

Hi Amit,

+Cc: David, who's maintaining the tools. Please keep him in the
recipients list!

amit.kumar-mahapatra@xilinx.com wrote on Thu, 3 Feb 2022 18:54:34 +0530:

> While calculating speed during  mtd_speedtest, the time interval
> (i.e., start - finish) is rounded off to the nearest milliseconds by
> ignoring the fractional part. This leads to miscalculation of speed.
> The miscalculation is more visible while running speed test on small
> partition sizes(i.e., when partition size is equal to eraseblock size or
> twice the eraseblock size) at higher spi frequencies.
> 
> For e.g., while calculating eraseblock read speed for a mtd partition with
> size equal to the eraseblock size(i.e., 64KiB) the eraseblock read time
> interval comes out to be 966490 nanosecond. This is then converted to
> millisecond(i.e., 0.966 msec.). The integer part (i.e., 0 msec) of the
> value is considered and the fractional part (i.e., 0.966) is ignored,for
> calculating the eraseblock read speed. So the reported eraseblock read
> speed is 0 KiB/s, which is incorrect.
> 
> There are two approaches to fix this issue.
> 
> First approach will be to keep the time interval in millisecond. and round
> up the integer value, with this approach the 0.966msec time interval in the
> above example will be rounded up to 1msec and this value is used for
> calculating the speed. Downside of this approach is that the reported speed
> is still not accurate.
> 
> Second approach will be to convert the time interval to microseconds
> instead of milliseconds, with this approach the 966490 nanosecond time
> interval in the above example will be converted t0 966.490usec and this
> value is used for calculating the speed. As compared to the current
> implementation and the suggested First approach, this approach will report
> a more accurate speed. Downside of this approach is that, in future if the
> mtd size is too large then the u64 variable, that holds the number of
> bytes, might overflow.
> 
> In this patch we have gone with the second approach as this reports a more
> accurate speed. With this approach the eraseblock read speed in the above
> example comes out to be 132505 KiB/s when the spi clock is configured at
> 150Mhz.
> 
> Signed-off-by: Amit Kumar Mahapatra <amit.kumar-mahapatra@xilinx.com>
> ---
> BRANCH: mtd/next
> ---
>  drivers/mtd/tests/speedtest.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/mtd/tests/speedtest.c b/drivers/mtd/tests/speedtest.c
> index 93e76648f676..2b76e7750c68 100644
> --- a/drivers/mtd/tests/speedtest.c
> +++ b/drivers/mtd/tests/speedtest.c
> @@ -161,13 +161,13 @@ static inline void stop_timing(void)
>  static long calc_speed(void)
>  {
>  	uint64_t k;
> -	long ms;
> +	long us;

Should this be an explicit 64-bit value? And unsigned?
unsigned long long int or uint64_t? I believe we are now 1000x closer
to the 4GiB limit so we might need to enlarge this variable.

>  
> -	ms = ktime_ms_delta(finish, start);
> -	if (ms == 0)
> +	us = ktime_us_delta(finish, start);
> +	if (us == 0)
>  		return 0;
> -	k = (uint64_t)goodebcnt * (mtd->erasesize / 1024) * 1000;
> -	do_div(k, ms);
> +	k = (uint64_t)goodebcnt * (mtd->erasesize / 1024) * 1000000;
> +	do_div(k, us);
>  	return k;
>  }
>  

Otherwise lgtm!

Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>


Thanks,
Miquèl

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [RFC PATCH] mtd: tests: Fix eraseblock read speed miscalculation for lower partition sizes
@ 2022-02-03 13:24 Amit Kumar Mahapatra
  2022-02-03 16:56 ` Miquel Raynal
  0 siblings, 1 reply; 4+ messages in thread
From: Amit Kumar Mahapatra @ 2022-02-03 13:24 UTC (permalink / raw)
  To: miquel.raynal, richard, vigneshr
  Cc: linux-mtd, linux-kernel, git, Amit Kumar Mahapatra

While calculating speed during  mtd_speedtest, the time interval
(i.e., start - finish) is rounded off to the nearest milliseconds by
ignoring the fractional part. This leads to miscalculation of speed.
The miscalculation is more visible while running speed test on small
partition sizes(i.e., when partition size is equal to eraseblock size or
twice the eraseblock size) at higher spi frequencies.

For e.g., while calculating eraseblock read speed for a mtd partition with
size equal to the eraseblock size(i.e., 64KiB) the eraseblock read time
interval comes out to be 966490 nanosecond. This is then converted to
millisecond(i.e., 0.966 msec.). The integer part (i.e., 0 msec) of the
value is considered and the fractional part (i.e., 0.966) is ignored,for
calculating the eraseblock read speed. So the reported eraseblock read
speed is 0 KiB/s, which is incorrect.

There are two approaches to fix this issue.

First approach will be to keep the time interval in millisecond. and round
up the integer value, with this approach the 0.966msec time interval in the
above example will be rounded up to 1msec and this value is used for
calculating the speed. Downside of this approach is that the reported speed
is still not accurate.

Second approach will be to convert the time interval to microseconds
instead of milliseconds, with this approach the 966490 nanosecond time
interval in the above example will be converted t0 966.490usec and this
value is used for calculating the speed. As compared to the current
implementation and the suggested First approach, this approach will report
a more accurate speed. Downside of this approach is that, in future if the
mtd size is too large then the u64 variable, that holds the number of
bytes, might overflow.

In this patch we have gone with the second approach as this reports a more
accurate speed. With this approach the eraseblock read speed in the above
example comes out to be 132505 KiB/s when the spi clock is configured at
150Mhz.

Signed-off-by: Amit Kumar Mahapatra <amit.kumar-mahapatra@xilinx.com>
---
BRANCH: mtd/next
---
 drivers/mtd/tests/speedtest.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/mtd/tests/speedtest.c b/drivers/mtd/tests/speedtest.c
index 93e76648f676..2b76e7750c68 100644
--- a/drivers/mtd/tests/speedtest.c
+++ b/drivers/mtd/tests/speedtest.c
@@ -161,13 +161,13 @@ static inline void stop_timing(void)
 static long calc_speed(void)
 {
 	uint64_t k;
-	long ms;
+	long us;
 
-	ms = ktime_ms_delta(finish, start);
-	if (ms == 0)
+	us = ktime_us_delta(finish, start);
+	if (us == 0)
 		return 0;
-	k = (uint64_t)goodebcnt * (mtd->erasesize / 1024) * 1000;
-	do_div(k, ms);
+	k = (uint64_t)goodebcnt * (mtd->erasesize / 1024) * 1000000;
+	do_div(k, us);
 	return k;
 }
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2022-02-08  5:33 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-03 13:21 [RFC PATCH] mtd: tests: Fix eraseblock read speed miscalculation for lower partition sizes Amit Kumar Mahapatra
2022-02-03 13:24 Amit Kumar Mahapatra
2022-02-03 16:56 ` Miquel Raynal
2022-02-08  5:24   ` Amit Kumar Kumar Mahapatra

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).