All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] NVMe-CLI Fix command failures in regress script
@ 2018-02-02 19:50 Jeff Lien
  2018-02-02 20:29 ` Keith Busch
  0 siblings, 1 reply; 8+ messages in thread
From: Jeff Lien @ 2018-02-02 19:50 UTC (permalink / raw)


Signed-off-by: Jeff Lien <jeff.lien at wdc.com>
---
 regress | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/regress b/regress
index aa7820c..0278d1d 100755
--- a/regress
+++ b/regress
@@ -89,7 +89,7 @@ if $LIST ; then
     run_test nvme list
 fi
 run_test nvme id-ctrl ${DEVICE}
-run_test nvme id-ns -raw-binary ${DEVICE}
+run_test nvme id-ns -n 1 -raw-binary ${DEVICE}
 run_test nvme list-ns -n 1 ${DEVICE}
 run_test nvme get-ns-id ${DEVICE}
 run_test nvme get-log ${DEVICE}  --log-id=2 --log-len=512
@@ -98,13 +98,13 @@ run_test nvme fw-log ${DEVICE} -b
 run_test nvme smart-log ${DEVICE}
 run_test nvme error-log ${DEVICE}
 run_test nvme get-feature ${DEVICE} -f 7
-run_test nvme flush ${DEVICE}
+run_test nvme flush -n 1 ${DEVICE}
 
 if $WRITE ; then
     run_test dd if=/dev/urandom of=${RAND_WFILE} bs=${RAND_SIZE} count=1
-    run_test nvme write ${DEVICE} --start-block=0 --block-count=0 --data-size=${RAND_SIZE} --data ${RAND_WFILE}
+    run_test nvme write ${DEVICE}n1 --start-block=0 --block-count=0 --data-size=${RAND_SIZE} --data ${RAND_WFILE}
 fi
-run_test nvme read ${DEVICE} --start-block=0 --block-count=0 --data-size=${RAND_SIZE} --data ${RAND_RFILE} --latency
+run_test nvme read ${DEVICE}n1 --start-block=0 --block-count=0 --data-size=${RAND_SIZE} --data ${RAND_RFILE} --latency
 if $WRITE ; then
     run_test diff ${RAND_RFILE} ${RAND_WFILE}
     rm ${RAND_WFILE} > /dev/null
-- 
2.14.2.746.g8fb8a94

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

* [PATCH] NVMe-CLI Fix command failures in regress script
  2018-02-02 19:50 [PATCH] NVMe-CLI Fix command failures in regress script Jeff Lien
@ 2018-02-02 20:29 ` Keith Busch
  2018-02-02 21:33   ` Jeffrey Lien
  0 siblings, 1 reply; 8+ messages in thread
From: Keith Busch @ 2018-02-02 20:29 UTC (permalink / raw)


On Fri, Feb 02, 2018@01:50:20PM -0600, Jeff Lien wrote:
> Signed-off-by: Jeff Lien <jeff.lien at wdc.com>
> ---
>  regress | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/regress b/regress
> index aa7820c..0278d1d 100755
> --- a/regress
> +++ b/regress
> @@ -89,7 +89,7 @@ if $LIST ; then
>      run_test nvme list
>  fi
>  run_test nvme id-ctrl ${DEVICE}
> -run_test nvme id-ns -raw-binary ${DEVICE}
> +run_test nvme id-ns -n 1 -raw-binary ${DEVICE}
>  run_test nvme list-ns -n 1 ${DEVICE}
>  run_test nvme get-ns-id ${DEVICE}
>  run_test nvme get-log ${DEVICE}  --log-id=2 --log-len=512
> @@ -98,13 +98,13 @@ run_test nvme fw-log ${DEVICE} -b
>  run_test nvme smart-log ${DEVICE}
>  run_test nvme error-log ${DEVICE}
>  run_test nvme get-feature ${DEVICE} -f 7
> -run_test nvme flush ${DEVICE}
> +run_test nvme flush -n 1 ${DEVICE}
>  
>  if $WRITE ; then
>      run_test dd if=/dev/urandom of=${RAND_WFILE} bs=${RAND_SIZE} count=1
> -    run_test nvme write ${DEVICE} --start-block=0 --block-count=0 --data-size=${RAND_SIZE} --data ${RAND_WFILE}
> +    run_test nvme write ${DEVICE}n1 --start-block=0 --block-count=0 --data-size=${RAND_SIZE} --data ${RAND_WFILE}
>  fi
> -run_test nvme read ${DEVICE} --start-block=0 --block-count=0 --data-size=${RAND_SIZE} --data ${RAND_RFILE} --latency
> +run_test nvme read ${DEVICE}n1 --start-block=0 --block-count=0 --data-size=${RAND_SIZE} --data ${RAND_RFILE} --latency
>  if $WRITE ; then
>      run_test diff ${RAND_RFILE} ${RAND_WFILE}
>      rm ${RAND_WFILE} > /dev/null

This script is supposed to only run on an nvme block block device.

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

* [PATCH] NVMe-CLI Fix command failures in regress script
  2018-02-02 20:29 ` Keith Busch
@ 2018-02-02 21:33   ` Jeffrey Lien
  2018-02-02 21:38     ` Keith Busch
  0 siblings, 1 reply; 8+ messages in thread
From: Jeffrey Lien @ 2018-02-02 21:33 UTC (permalink / raw)


Keith, 
Then should we remove the commands that require a namespace id (ie id-ns, flush, write, and read)?  


Jeff Lien

-----Original Message-----
From: Keith Busch [mailto:keith.busch@intel.com] 
Sent: Friday, February 2, 2018 2:30 PM
To: Jeffrey Lien
Cc: linux-nvme at lists.infradead.org; David Darrington
Subject: Re: [PATCH] NVMe-CLI Fix command failures in regress script

On Fri, Feb 02, 2018@01:50:20PM -0600, Jeff Lien wrote:
> Signed-off-by: Jeff Lien <jeff.lien at wdc.com>
> ---
>  regress | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/regress b/regress
> index aa7820c..0278d1d 100755
> --- a/regress
> +++ b/regress
> @@ -89,7 +89,7 @@ if $LIST ; then
>      run_test nvme list
>  fi
>  run_test nvme id-ctrl ${DEVICE}
> -run_test nvme id-ns -raw-binary ${DEVICE}
> +run_test nvme id-ns -n 1 -raw-binary ${DEVICE}
>  run_test nvme list-ns -n 1 ${DEVICE}
>  run_test nvme get-ns-id ${DEVICE}
>  run_test nvme get-log ${DEVICE}  --log-id=2 --log-len=512 @@ -98,13 
> +98,13 @@ run_test nvme fw-log ${DEVICE} -b  run_test nvme smart-log 
> ${DEVICE}  run_test nvme error-log ${DEVICE}  run_test nvme 
> get-feature ${DEVICE} -f 7 -run_test nvme flush ${DEVICE}
> +run_test nvme flush -n 1 ${DEVICE}
>  
>  if $WRITE ; then
>      run_test dd if=/dev/urandom of=${RAND_WFILE} bs=${RAND_SIZE} count=1
> -    run_test nvme write ${DEVICE} --start-block=0 --block-count=0 --data-size=${RAND_SIZE} --data ${RAND_WFILE}
> +    run_test nvme write ${DEVICE}n1 --start-block=0 --block-count=0 
> + --data-size=${RAND_SIZE} --data ${RAND_WFILE}
>  fi
> -run_test nvme read ${DEVICE} --start-block=0 --block-count=0 
> --data-size=${RAND_SIZE} --data ${RAND_RFILE} --latency
> +run_test nvme read ${DEVICE}n1 --start-block=0 --block-count=0 
> +--data-size=${RAND_SIZE} --data ${RAND_RFILE} --latency
>  if $WRITE ; then
>      run_test diff ${RAND_RFILE} ${RAND_WFILE}
>      rm ${RAND_WFILE} > /dev/null

This script is supposed to only run on an nvme block block device.

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

* [PATCH] NVMe-CLI Fix command failures in regress script
  2018-02-02 21:33   ` Jeffrey Lien
@ 2018-02-02 21:38     ` Keith Busch
  2018-02-05 16:49       ` Jeffrey Lien
  0 siblings, 1 reply; 8+ messages in thread
From: Keith Busch @ 2018-02-02 21:38 UTC (permalink / raw)


On Fri, Feb 02, 2018@09:33:12PM +0000, Jeffrey Lien wrote:
> Keith, 
> Then should we remove the commands that require a namespace id (ie id-ns, flush, write, and read)?  

If you provide a block device, nvme-cli will automatically figure out
the nsid if you didn't provide one.

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

* [PATCH] NVMe-CLI Fix command failures in regress script
  2018-02-02 21:38     ` Keith Busch
@ 2018-02-05 16:49       ` Jeffrey Lien
  2018-02-05 16:57         ` Keith Busch
  0 siblings, 1 reply; 8+ messages in thread
From: Jeffrey Lien @ 2018-02-05 16:49 UTC (permalink / raw)


Keith,
That doesn't seem to happen with our nvme device.  Below is what I get when I run the regress script as is.  Am I missing something on the invocation?

./regress -d /dev/nvme0 -l -w
WARNING: Write mode enabled, this might trash your drive!
NVME_VERSION = 1.5.79.ga75e.dirty
  RUN   nvme list                                                            : PASSED!
  RUN   nvme id-ctrl /dev/nvme0                                              : PASSED!
  RUN   nvme id-ns -raw-binary /dev/nvme0                                    : FAILED!
Failed running command: 
   nvme id-ns -raw-binary /dev/nvme0
  RUN   nvme list-ns -n 1 /dev/nvme0                                         : PASSED!
  RUN   nvme get-ns-id /dev/nvme0                                            : PASSED!
  RUN   nvme get-log /dev/nvme0 --log-id=2 --log-len=512                     : PASSED!
  RUN   nvme fw-log /dev/nvme0                                               : PASSED!
  RUN   nvme fw-log /dev/nvme0 -b                                            : PASSED!
  RUN   nvme smart-log /dev/nvme0                                            : PASSED!
  RUN   nvme error-log /dev/nvme0                                            : PASSED!
  RUN   nvme get-feature /dev/nvme0 -f 7                                     : PASSED!
  RUN   nvme flush /dev/nvme0                                                : FAILED!
Failed running command: 
   nvme flush /dev/nvme0
  RUN   dd if=/dev/urandom of=temp.rand.write bs=4k count=1                  : PASSED!
  RUN   nvme write /dev/nvme0 --start-block=0 --block-count=0 --data-size=4  : FAILED!
Failed running command: 
   nvme write /dev/nvme0 --start-block=0 --block-count=0 --data-size=4k --data temp.rand.write
  RUN   nvme read /dev/nvme0 --start-block=0 --block-count=0 --data-size=4k  : FAILED!
Failed running command: 
   nvme read /dev/nvme0 --start-block=0 --block-count=0 --data-size=4k --data temp.rand.read --latency
  RUN   diff temp.rand.read temp.rand.write                                  : FAILED!
Failed running command: 
   diff temp.rand.read temp.rand.write


Jeff Lien

-----Original Message-----
From: Keith Busch [mailto:keith.busch@intel.com] 
Sent: Friday, February 2, 2018 3:38 PM
To: Jeffrey Lien
Cc: linux-nvme at lists.infradead.org; David Darrington
Subject: Re: [PATCH] NVMe-CLI Fix command failures in regress script

On Fri, Feb 02, 2018@09:33:12PM +0000, Jeffrey Lien wrote:
> Keith,
> Then should we remove the commands that require a namespace id (ie id-ns, flush, write, and read)?  

If you provide a block device, nvme-cli will automatically figure out the nsid if you didn't provide one.

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

* [PATCH] NVMe-CLI Fix command failures in regress script
  2018-02-05 16:49       ` Jeffrey Lien
@ 2018-02-05 16:57         ` Keith Busch
  2018-02-05 19:00           ` Jeffrey Lien
  0 siblings, 1 reply; 8+ messages in thread
From: Keith Busch @ 2018-02-05 16:57 UTC (permalink / raw)


On Mon, Feb 05, 2018@04:49:11PM +0000, Jeffrey Lien wrote:
> Keith,
> That doesn't seem to happen with our nvme device.  Below is what I get when I run the regress script as is.  Am I missing something on the invocation?
> 
> ./regress -d /dev/nvme0 -l -w
                    ^^^^^

That's a character device. You need to use a block device, like
/dev/nvme0n1.

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

* [PATCH] NVMe-CLI Fix command failures in regress script
  2018-02-05 16:57         ` Keith Busch
@ 2018-02-05 19:00           ` Jeffrey Lien
  2018-02-05 19:34             ` Keith Busch
  0 siblings, 1 reply; 8+ messages in thread
From: Jeffrey Lien @ 2018-02-05 19:00 UTC (permalink / raw)


Ok, that makes sense.  I should have thought to try that; sorry for the trouble.   

I did find 2 other problems/failures when using the block device though:
1)  The flush command has a default namespace id of All ID's (0xffffffff) which causes it to fail.  So to fix that either:
    * The default ns id has to change for the flush command or 
    * The "-n 1" parm needs to be added to the flush command in the regress script 
2) The --block-count=0 setting in the read/write commands fails when the nvme drive is formatted with a lba data size of 512 bytes.  Setting it to 8 seems to work for both  512 and 4k bytes lba data size.  
    
I can recreate the patch with these fixes, just not sure which option to take with the flush command. 

Jeff Lien

-----Original Message-----
From: Keith Busch [mailto:keith.busch@intel.com] 
Sent: Monday, February 5, 2018 10:57 AM
To: Jeffrey Lien
Cc: linux-nvme at lists.infradead.org; David Darrington
Subject: Re: [PATCH] NVMe-CLI Fix command failures in regress script

On Mon, Feb 05, 2018@04:49:11PM +0000, Jeffrey Lien wrote:
> Keith,
> That doesn't seem to happen with our nvme device.  Below is what I get when I run the regress script as is.  Am I missing something on the invocation?
> 
> ./regress -d /dev/nvme0 -l -w
                    ^^^^^

That's a character device. You need to use a block device, like /dev/nvme0n1.

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

* [PATCH] NVMe-CLI Fix command failures in regress script
  2018-02-05 19:00           ` Jeffrey Lien
@ 2018-02-05 19:34             ` Keith Busch
  0 siblings, 0 replies; 8+ messages in thread
From: Keith Busch @ 2018-02-05 19:34 UTC (permalink / raw)


On Mon, Feb 05, 2018@07:00:58PM +0000, Jeffrey Lien wrote:
> Ok, that makes sense.  I should have thought to try that; sorry for the trouble.   
> 
> I did find 2 other problems/failures when using the block device though:
> 1)  The flush command has a default namespace id of All ID's (0xffffffff) which causes it to fail.  So to fix that either:
>     * The default ns id has to change for the flush command or 
>     * The "-n 1" parm needs to be added to the flush command in the regress script 

I will change nvme-cli to use the block handle NSID no problem, but
isn't Flush the only IO command that is required to support the
broadcast NSID? I know the wording was a bit vague on using this value
and there was discussion on the technical reflector about it, but I
haven't had time to read into where they're going with it.

> 2) The --block-count=0 setting in the read/write commands fails when the nvme drive is formatted with a lba data size of 512 bytes.  Setting it to 8 seems to work for both  512 and 4k bytes lba data size.  

Oops, the regress script has hard-coded an expected 4k LBA format. We need
it to discover and be re-active to the format instead.

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

end of thread, other threads:[~2018-02-05 19:34 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-02 19:50 [PATCH] NVMe-CLI Fix command failures in regress script Jeff Lien
2018-02-02 20:29 ` Keith Busch
2018-02-02 21:33   ` Jeffrey Lien
2018-02-02 21:38     ` Keith Busch
2018-02-05 16:49       ` Jeffrey Lien
2018-02-05 16:57         ` Keith Busch
2018-02-05 19:00           ` Jeffrey Lien
2018-02-05 19:34             ` Keith Busch

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.