mbox series

[v6,00/42] nvme: support NVMe v1.3d, SGLs and multiple namespaces

Message ID 20200316142928.153431-1-its@irrelevant.dk
Headers show
Series nvme: support NVMe v1.3d, SGLs and multiple namespaces | expand

Message

Klaus Jensen March 16, 2020, 2:28 p.m. UTC 
From: Klaus Jensen <k.jensen@samsung.com>

Hi,

So this patchset kinda blew up in size (wrt. number of patches) after
Maxim's comments (26 -> 42), but Maxim's comments about splitting up a
bunch of the patches made a lot of sense.

v6 primarily splits up the big nasty patches into more digestible parts.
Specifically the 'nvme: refactor prp mapping' and 'nvme: allow multiple
aios per command' patches has been split up according to Maxim's
comments. Most additions to the shared include/block/nvme.h has also
been consolidated into a single patch (also according to Maxim's
comments). A lot of the patches still carries a 'Reviewed-By', but
git-backport-diff reports some changes due to changes/additions in some
of the early patches.

The only real "addition" is a new "max_ioqpairs" parameter for the
device. This is to fix some confusion about the current "num_queues"
parameter. See "nvme: add max_ioqpairs device parameter".

Maxim, I responded to your comments in the original thread and I believe
that all your comments has been adressed.

Also, I *did* change the line indentation style - I hope I caught 'em
all :)


Klaus Jensen (42):
  nvme: rename trace events to nvme_dev
  nvme: remove superfluous breaks
  nvme: move device parameters to separate struct
  nvme: bump spec data structures to v1.3
  nvme: use constant for identify data size
  nvme: add identify cns values in header
  nvme: refactor nvme_addr_read
  nvme: add support for the abort command
  nvme: add max_ioqpairs device parameter
  nvme: refactor device realization
  nvme: add temperature threshold feature
  nvme: add support for the get log page command
  nvme: add support for the asynchronous event request command
  nvme: add missing mandatory features
  nvme: additional tracing
  nvme: make sure ncqr and nsqr is valid
  nvme: add log specific field to trace events
  nvme: support identify namespace descriptor list
  nvme: enforce valid queue creation sequence
  nvme: provide the mandatory subnqn field
  nvme: bump supported version to v1.3
  nvme: memset preallocated requests structures
  nvme: add mapping helpers
  nvme: remove redundant has_sg member
  nvme: refactor dma read/write
  nvme: pass request along for tracing
  nvme: add request mapping helper
  nvme: verify validity of prp lists in the cmb
  nvme: refactor request bounds checking
  nvme: add check for mdts
  nvme: add check for prinfo
  nvme: allow multiple aios per command
  nvme: use preallocated qsg/iov in nvme_dma_prp
  pci: pass along the return value of dma_memory_rw
  nvme: handle dma errors
  nvme: add support for scatter gather lists
  nvme: refactor identify active namespace id list
  nvme: support multiple namespaces
  pci: allocate pci id for nvme
  nvme: change controller pci id
  nvme: remove redundant NvmeCmd pointer parameter
  nvme: make lba data size configurable

 MAINTAINERS            |    1 +
 block/nvme.c           |   18 +-
 docs/specs/nvme.txt    |   25 +
 docs/specs/pci-ids.txt |    1 +
 hw/block/Makefile.objs |    2 +-
 hw/block/nvme-ns.c     |  162 ++++
 hw/block/nvme-ns.h     |   62 ++
 hw/block/nvme.c        | 2041 ++++++++++++++++++++++++++++++++--------
 hw/block/nvme.h        |  205 +++-
 hw/block/trace-events  |  206 ++--
 hw/core/machine.c      |    1 +
 include/block/nvme.h   |  178 +++-
 include/hw/pci/pci.h   |    4 +-
 13 files changed, 2347 insertions(+), 559 deletions(-)
 create mode 100644 docs/specs/nvme.txt
 create mode 100644 hw/block/nvme-ns.c
 create mode 100644 hw/block/nvme-ns.h

Comments

no-reply@patchew.org March 16, 2020, 7:30 p.m. UTC | #1 
Patchew URL: https://patchew.org/QEMU/20200316142928.153431-1-its@irrelevant.dk/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Subject: [PATCH v6 00/42] nvme: support NVMe v1.3d, SGLs and multiple namespaces
Message-id: 20200316142928.153431-1-its@irrelevant.dk
Type: series

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
   509f617..a98135f  master     -> master
 - [tag update]      patchew/20200315144653.22660-1-armbru@redhat.com -> patchew/20200315144653.22660-1-armbru@redhat.com
 - [tag update]      patchew/20200315235716.28448-1-philmd@redhat.com -> patchew/20200315235716.28448-1-philmd@redhat.com
 - [tag update]      patchew/20200316001111.31004-1-philmd@redhat.com -> patchew/20200316001111.31004-1-philmd@redhat.com
 - [tag update]      patchew/20200316060631.30052-1-vsementsov@virtuozzo.com -> patchew/20200316060631.30052-1-vsementsov@virtuozzo.com
 - [tag update]      patchew/20200316103203.10046-1-ovoshcha@redhat.com -> patchew/20200316103203.10046-1-ovoshcha@redhat.com
 - [tag update]      patchew/20200316120049.11225-1-philmd@redhat.com -> patchew/20200316120049.11225-1-philmd@redhat.com
 - [tag update]      patchew/20200316160702.478964-1-stefanha@redhat.com -> patchew/20200316160702.478964-1-stefanha@redhat.com
 * [new tag]         patchew/20200316172155.971-1-alex.bennee@linaro.org -> patchew/20200316172155.971-1-alex.bennee@linaro.org
 * [new tag]         patchew/20200316174610.115820-1-jandryuk@gmail.com -> patchew/20200316174610.115820-1-jandryuk@gmail.com
Switched to a new branch 'test'
29ae701 nvme: make lba data size configurable
18cddc9 nvme: remove redundant NvmeCmd pointer parameter
526882e nvme: change controller pci id
98c330b pci: allocate pci id for nvme
0e37aa0 nvme: support multiple namespaces
6983597 nvme: refactor identify active namespace id list
d918ef5 nvme: add support for scatter gather lists
30f6663 nvme: handle dma errors
0cdbf87 pci: pass along the return value of dma_memory_rw
3a6a832 nvme: use preallocated qsg/iov in nvme_dma_prp
50022cc nvme: allow multiple aios per command
0720b5d nvme: add check for prinfo
fec3f89 nvme: add check for mdts
13ce0f5 nvme: refactor request bounds checking
9e8d597 nvme: verify validity of prp lists in the cmb
52c3589 nvme: add request mapping helper
5cbee0c nvme: pass request along for tracing
85e635e nvme: refactor dma read/write
91c10c5 nvme: remove redundant has_sg member
8c73469 nvme: add mapping helpers
26c1cba nvme: memset preallocated requests structures
5f27206 nvme: bump supported version to v1.3
6301b23 nvme: provide the mandatory subnqn field
e42793c nvme: enforce valid queue creation sequence
6cb67c5 nvme: support identify namespace descriptor list
2fd6cd0 nvme: add log specific field to trace events
28646e0 nvme: make sure ncqr and nsqr is valid
f093fa6 nvme: additional tracing
e01e2c3 nvme: add missing mandatory features
acc8277 nvme: add support for the asynchronous event request command
751053c nvme: add support for the get log page command
a75d78af nvme: add temperature threshold feature
90c3b3a nvme: refactor device realization
c6909e3 nvme: add max_ioqpairs device parameter
cf80062 nvme: add support for the abort command
c150866 nvme: refactor nvme_addr_read
153786f nvme: add identify cns values in header
7fb5521 nvme: use constant for identify data size
a350897 nvme: bump spec data structures to v1.3
b3abe79 nvme: move device parameters to separate struct
5a50ee9 nvme: remove superfluous breaks
79122a7 nvme: rename trace events to nvme_dev

=== OUTPUT BEGIN ===
1/42 Checking commit 79122a7973d8 (nvme: rename trace events to nvme_dev)
2/42 Checking commit 5a50ee90c197 (nvme: remove superfluous breaks)
3/42 Checking commit b3abe7986742 (nvme: move device parameters to separate struct)
ERROR: Macros with complex values should be enclosed in parenthesis
#179: FILE: hw/block/nvme.h:6:
+#define DEFINE_NVME_PROPERTIES(_state, _props) \
+    DEFINE_PROP_STRING("serial", _state, _props.serial), \
+    DEFINE_PROP_UINT32("cmb_size_mb", _state, _props.cmb_size_mb, 0), \
+    DEFINE_PROP_UINT32("num_queues", _state, _props.num_queues, 64)

total: 1 errors, 0 warnings, 181 lines checked

Patch 3/42 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

4/42 Checking commit a350897dab5c (nvme: bump spec data structures to v1.3)
5/42 Checking commit 7fb5521cf3df (nvme: use constant for identify data size)
6/42 Checking commit 153786f01b2c (nvme: add identify cns values in header)
7/42 Checking commit c15086697656 (nvme: refactor nvme_addr_read)
8/42 Checking commit cf8006250540 (nvme: add support for the abort command)
9/42 Checking commit c6909e377ba7 (nvme: add max_ioqpairs device parameter)
10/42 Checking commit 90c3b3a96d0e (nvme: refactor device realization)
11/42 Checking commit a75d78afec33 (nvme: add temperature threshold feature)
12/42 Checking commit 751053c2eebe (nvme: add support for the get log page command)
13/42 Checking commit acc82772cb85 (nvme: add support for the asynchronous event request command)
14/42 Checking commit e01e2c38efb6 (nvme: add missing mandatory features)
15/42 Checking commit f093fa63dd1d (nvme: additional tracing)
16/42 Checking commit 28646e0cb43e (nvme: make sure ncqr and nsqr is valid)
17/42 Checking commit 2fd6cd0b68b4 (nvme: add log specific field to trace events)
18/42 Checking commit 6cb67c52235e (nvme: support identify namespace descriptor list)
19/42 Checking commit e42793ca050c (nvme: enforce valid queue creation sequence)
20/42 Checking commit 6301b23c832c (nvme: provide the mandatory subnqn field)
21/42 Checking commit 5f272069c824 (nvme: bump supported version to v1.3)
22/42 Checking commit 26c1cba5e438 (nvme: memset preallocated requests structures)
23/42 Checking commit 8c73469d1135 (nvme: add mapping helpers)
24/42 Checking commit 91c10c5b8a10 (nvme: remove redundant has_sg member)
25/42 Checking commit 85e635e6490b (nvme: refactor dma read/write)
26/42 Checking commit 5cbee0c45413 (nvme: pass request along for tracing)
27/42 Checking commit 52c35897ba1d (nvme: add request mapping helper)
28/42 Checking commit 9e8d597376f9 (nvme: verify validity of prp lists in the cmb)
29/42 Checking commit 13ce0f521731 (nvme: refactor request bounds checking)
30/42 Checking commit fec3f89de690 (nvme: add check for mdts)
31/42 Checking commit 0720b5d83eef (nvme: add check for prinfo)
32/42 Checking commit 50022cc39bd7 (nvme: allow multiple aios per command)
33/42 Checking commit 3a6a83209c32 (nvme: use preallocated qsg/iov in nvme_dma_prp)
34/42 Checking commit 0cdbf87e80ff (pci: pass along the return value of dma_memory_rw)
35/42 Checking commit 30f6663d58dd (nvme: handle dma errors)
36/42 Checking commit d918ef503f4c (nvme: add support for scatter gather lists)
37/42 Checking commit 6983597bfc2c (nvme: refactor identify active namespace id list)
38/42 Checking commit 0e37aa02f23a (nvme: support multiple namespaces)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#43: 
new file mode 100644

ERROR: Macros with complex values should be enclosed in parenthesis
#218: FILE: hw/block/nvme-ns.h:8:
+#define DEFINE_NVME_NS_PROPERTIES(_state, _props) \
+    DEFINE_PROP_DRIVE("drive", _state, blk), \
+    DEFINE_PROP_UINT32("nsid", _state, _props.nsid, 0)

total: 1 errors, 1 warnings, 822 lines checked

Patch 38/42 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

39/42 Checking commit 98c330b4dd11 (pci: allocate pci id for nvme)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#31: 
new file mode 100644

total: 0 errors, 1 warnings, 46 lines checked

Patch 39/42 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
40/42 Checking commit 526882e1651a (nvme: change controller pci id)
41/42 Checking commit 18cddc952a86 (nvme: remove redundant NvmeCmd pointer parameter)
42/42 Checking commit 29ae701c9d3a (nvme: make lba data size configurable)
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20200316142928.153431-1-its@irrelevant.dk/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
Maxim Levitsky March 25, 2020, 10:35 a.m. UTC | #2 
On Mon, 2020-03-16 at 07:28 -0700, Klaus Jensen wrote:
> From: Klaus Jensen <k.jensen@samsung.com>
> 
> Hi,
> 
> So this patchset kinda blew up in size (wrt. number of patches) after
> Maxim's comments (26 -> 42), but Maxim's comments about splitting up a
> bunch of the patches made a lot of sense.
I don't think this is bad. 
You might actually found the ultimate question of life the universe and everything.
;-)

Best regards,
	Maxim Levitsky

> 
> v6 primarily splits up the big nasty patches into more digestible parts.
> Specifically the 'nvme: refactor prp mapping' and 'nvme: allow multiple
> aios per command' patches has been split up according to Maxim's
> comments. Most additions to the shared include/block/nvme.h has also
> been consolidated into a single patch (also according to Maxim's
> comments). A lot of the patches still carries a 'Reviewed-By', but
> git-backport-diff reports some changes due to changes/additions in some
> of the early patches.
> 
> The only real "addition" is a new "max_ioqpairs" parameter for the
> device. This is to fix some confusion about the current "num_queues"
> parameter. See "nvme: add max_ioqpairs device parameter".
> 
> Maxim, I responded to your comments in the original thread and I believe
> that all your comments has been adressed.
> 
> Also, I *did* change the line indentation style - I hope I caught 'em
> all :)
> 
> 
> Klaus Jensen (42):
>   nvme: rename trace events to nvme_dev
>   nvme: remove superfluous breaks
>   nvme: move device parameters to separate struct
>   nvme: bump spec data structures to v1.3
>   nvme: use constant for identify data size
>   nvme: add identify cns values in header
>   nvme: refactor nvme_addr_read
>   nvme: add support for the abort command
>   nvme: add max_ioqpairs device parameter
>   nvme: refactor device realization
>   nvme: add temperature threshold feature
>   nvme: add support for the get log page command
>   nvme: add support for the asynchronous event request command
>   nvme: add missing mandatory features
>   nvme: additional tracing
>   nvme: make sure ncqr and nsqr is valid
>   nvme: add log specific field to trace events
>   nvme: support identify namespace descriptor list
>   nvme: enforce valid queue creation sequence
>   nvme: provide the mandatory subnqn field
>   nvme: bump supported version to v1.3
>   nvme: memset preallocated requests structures
>   nvme: add mapping helpers
>   nvme: remove redundant has_sg member
>   nvme: refactor dma read/write
>   nvme: pass request along for tracing
>   nvme: add request mapping helper
>   nvme: verify validity of prp lists in the cmb
>   nvme: refactor request bounds checking
>   nvme: add check for mdts
>   nvme: add check for prinfo
>   nvme: allow multiple aios per command
>   nvme: use preallocated qsg/iov in nvme_dma_prp
>   pci: pass along the return value of dma_memory_rw
>   nvme: handle dma errors
>   nvme: add support for scatter gather lists
>   nvme: refactor identify active namespace id list
>   nvme: support multiple namespaces
>   pci: allocate pci id for nvme
>   nvme: change controller pci id
>   nvme: remove redundant NvmeCmd pointer parameter
>   nvme: make lba data size configurable
> 
>  MAINTAINERS            |    1 +
>  block/nvme.c           |   18 +-
>  docs/specs/nvme.txt    |   25 +
>  docs/specs/pci-ids.txt |    1 +
>  hw/block/Makefile.objs |    2 +-
>  hw/block/nvme-ns.c     |  162 ++++
>  hw/block/nvme-ns.h     |   62 ++
>  hw/block/nvme.c        | 2041 ++++++++++++++++++++++++++++++++--------
>  hw/block/nvme.h        |  205 +++-
>  hw/block/trace-events  |  206 ++--
>  hw/core/machine.c      |    1 +
>  include/block/nvme.h   |  178 +++-
>  include/hw/pci/pci.h   |    4 +-
>  13 files changed, 2347 insertions(+), 559 deletions(-)
>  create mode 100644 docs/specs/nvme.txt
>  create mode 100644 hw/block/nvme-ns.c
>  create mode 100644 hw/block/nvme-ns.h
>