[PATCH v5 00/18] nvme: refactoring and cleanups

Klaus Jensen posted 18 patches 3 years, 12 months ago
Test docker-mingw@fedora failed
Test checkpatch failed
Test asan failed
Test docker-quick@centos7 failed
Test FreeBSD failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20200505054840.186586-1-its@irrelevant.dk
Maintainers: Keith Busch <kbusch@kernel.org>, Kevin Wolf <kwolf@redhat.com>, Max Reitz <mreitz@redhat.com>
hw/block/nvme.c       | 543 ++++++++++++++++++++++++------------------
hw/block/nvme.h       |  31 ++-
hw/block/trace-events | 180 +++++++-------
include/block/nvme.h  |   8 +
4 files changed, 429 insertions(+), 333 deletions(-)
[PATCH v5 00/18] nvme: refactoring and cleanups
Posted by Klaus Jensen 3 years, 12 months ago
From: Klaus Jensen <k.jensen@samsung.com>

Changes since v5
~~~~~~~~~~~~~~~~
No functional changes, just updated Reviewed-by tags. Also, I screwed up
the CC list when sending v4.

Philippe and Keith, please add a Reviewed-by to

  * "nvme: factor out pmr setup" and
  * "do cmb/pmr init as part of pci init"

since the first one was added and the second one was changed in v4 when
rebasing on Kevins block-next tree which had the PMR work that was not
in master at the time.

With those in place, it should be ready for Kevin to merge.

Klaus Jensen (18):
  nvme: fix pci doorbell size calculation
  nvme: rename trace events to pci_nvme
  nvme: remove superfluous breaks
  nvme: move device parameters to separate struct
  nvme: use constants in identify
  nvme: refactor nvme_addr_read
  nvme: add max_ioqpairs device parameter
  nvme: remove redundant cmbloc/cmbsz members
  nvme: factor out property/constraint checks
  nvme: factor out device state setup
  nvme: factor out block backend setup
  nvme: add namespace helpers
  nvme: factor out namespace setup
  nvme: factor out pci setup
  nvme: factor out cmb setup
  nvme: factor out pmr setup
  nvme: do cmb/pmr init as part of pci init
  nvme: factor out controller identify setup

 hw/block/nvme.c       | 543 ++++++++++++++++++++++++------------------
 hw/block/nvme.h       |  31 ++-
 hw/block/trace-events | 180 +++++++-------
 include/block/nvme.h  |   8 +
 4 files changed, 429 insertions(+), 333 deletions(-)

-- 
2.26.2


Re: [PATCH v5 00/18] nvme: refactoring and cleanups
Posted by Philippe Mathieu-Daudé 3 years, 12 months ago
On 5/5/20 7:48 AM, Klaus Jensen wrote:
> From: Klaus Jensen <k.jensen@samsung.com>
> 
> Changes since v5
> ~~~~~~~~~~~~~~~~
> No functional changes, just updated Reviewed-by tags. Also, I screwed up
> the CC list when sending v4.
> 
> Philippe and Keith, please add a Reviewed-by to
> 
>    * "nvme: factor out pmr setup" and
>    * "do cmb/pmr init as part of pci init"
> 
> since the first one was added and the second one was changed in v4 when
> rebasing on Kevins block-next tree which had the PMR work that was not
> in master at the time.
> 
> With those in place, it should be ready for Kevin to merge.
> 
> Klaus Jensen (18):
>    nvme: fix pci doorbell size calculation
>    nvme: rename trace events to pci_nvme
>    nvme: remove superfluous breaks
>    nvme: move device parameters to separate struct
>    nvme: use constants in identify
>    nvme: refactor nvme_addr_read
>    nvme: add max_ioqpairs device parameter
>    nvme: remove redundant cmbloc/cmbsz members
>    nvme: factor out property/constraint checks
>    nvme: factor out device state setup
>    nvme: factor out block backend setup
>    nvme: add namespace helpers
>    nvme: factor out namespace setup
>    nvme: factor out pci setup
>    nvme: factor out cmb setup
>    nvme: factor out pmr setup
>    nvme: do cmb/pmr init as part of pci init
>    nvme: factor out controller identify setup

Thinking loudly, it would be easier to differentiate emulated device vs 
block driver using 's,^nvme,hw/nvme,' in patches (and series) title. 
Kevin, if you are motivated...

> 
>   hw/block/nvme.c       | 543 ++++++++++++++++++++++++------------------
>   hw/block/nvme.h       |  31 ++-
>   hw/block/trace-events | 180 +++++++-------
>   include/block/nvme.h  |   8 +
>   4 files changed, 429 insertions(+), 333 deletions(-)
> 


Re: [PATCH v5 00/18] nvme: refactoring and cleanups
Posted by Klaus Jensen 3 years, 11 months ago
On May  5 07:48, Klaus Jensen wrote:
> From: Klaus Jensen <k.jensen@samsung.com>
> 
> Changes since v5
> ~~~~~~~~~~~~~~~~
> No functional changes, just updated Reviewed-by tags. Also, I screwed up
> the CC list when sending v4.
> 
> Philippe and Keith, please add a Reviewed-by to
> 
>   * "nvme: factor out pmr setup" and
>   * "do cmb/pmr init as part of pci init"
> 
> since the first one was added and the second one was changed in v4 when
> rebasing on Kevins block-next tree which had the PMR work that was not
> in master at the time.
> 
> With those in place, it should be ready for Kevin to merge.
> 
 
Gentle ping on this.

Also, please see the two patches in "[PATCH 0/2] hw/block/nvme: fixes
for interrupt behavior". I think they should go in preparation to this
series.

Re: [PATCH v5 00/18] nvme: refactoring and cleanups
Posted by Philippe Mathieu-Daudé 3 years, 11 months ago
Hi Klaus,

On 5/11/20 8:25 AM, Klaus Jensen wrote:
> On May  5 07:48, Klaus Jensen wrote:
>> From: Klaus Jensen <k.jensen@samsung.com>
>>
>> Changes since v5
>> ~~~~~~~~~~~~~~~~
>> No functional changes, just updated Reviewed-by tags. Also, I screwed up
>> the CC list when sending v4.
>>
>> Philippe and Keith, please add a Reviewed-by to
>>
>>    * "nvme: factor out pmr setup" and
>>    * "do cmb/pmr init as part of pci init"
>>
>> since the first one was added and the second one was changed in v4 when
>> rebasing on Kevins block-next tree which had the PMR work that was not
>> in master at the time.
>>
>> With those in place, it should be ready for Kevin to merge.
>>
>   
> Gentle ping on this.
> 
> Also, please see the two patches in "[PATCH 0/2] hw/block/nvme: fixes
> for interrupt behavior". I think they should go in preparation to this
> series.

I was going to ping Kevin last week, but then read your comment on pach 
#7 "nvme: add max_ioqpairs device parameter", so I interpreted you would 
respin.
Now it is clearer, applying in the following order you don't need to 
respin, right?

- [PATCH 0/2] hw/block/nvme: fixes for interrupt behavior"
- [PATCH v5 00/18] nvme: refactoring and cleanups


Re: [PATCH v5 00/18] nvme: refactoring and cleanups
Posted by Klaus Jensen 3 years, 11 months ago
On May 11 09:00, Philippe Mathieu-Daudé wrote:
> Hi Klaus,
> 
> On 5/11/20 8:25 AM, Klaus Jensen wrote:
> > On May  5 07:48, Klaus Jensen wrote:
> > > From: Klaus Jensen <k.jensen@samsung.com>
> > > 
> > > Changes since v5
> > > ~~~~~~~~~~~~~~~~
> > > No functional changes, just updated Reviewed-by tags. Also, I screwed up
> > > the CC list when sending v4.
> > > 
> > > Philippe and Keith, please add a Reviewed-by to
> > > 
> > >    * "nvme: factor out pmr setup" and
> > >    * "do cmb/pmr init as part of pci init"
> > > 
> > > since the first one was added and the second one was changed in v4 when
> > > rebasing on Kevins block-next tree which had the PMR work that was not
> > > in master at the time.
> > > 
> > > With those in place, it should be ready for Kevin to merge.
> > > 
> > Gentle ping on this.
> > 
> > Also, please see the two patches in "[PATCH 0/2] hw/block/nvme: fixes
> > for interrupt behavior". I think they should go in preparation to this
> > series.
> 
> I was going to ping Kevin last week, but then read your comment on pach #7
> "nvme: add max_ioqpairs device parameter", so I interpreted you would
> respin.
> Now it is clearer, applying in the following order you don't need to respin,
> right?
> 
> - [PATCH 0/2] hw/block/nvme: fixes for interrupt behavior"
> - [PATCH v5 00/18] nvme: refactoring and cleanups
> 

Ugh. "[PATCH v5 00/18] nvme: refactoring and cleanups" doesn't apply
completely cleanly.

"[PATCH 0/2] hw/block/nvme: fixes for interrupt behavior" was intented
to go into master because it fixes a bug, that is why I split them up.

But looks like it is better to just roll it into this series. I'll
respin a v6 with the two interrupt fixes.

Re: [PATCH v5 00/18] nvme: refactoring and cleanups
Posted by Kevin Wolf 3 years, 11 months ago
Am 11.05.2020 um 09:09 hat Klaus Jensen geschrieben:
> On May 11 09:00, Philippe Mathieu-Daudé wrote:
> > Hi Klaus,
> > 
> > On 5/11/20 8:25 AM, Klaus Jensen wrote:
> > > On May  5 07:48, Klaus Jensen wrote:
> > > > From: Klaus Jensen <k.jensen@samsung.com>
> > > > 
> > > > Changes since v5
> > > > ~~~~~~~~~~~~~~~~
> > > > No functional changes, just updated Reviewed-by tags. Also, I screwed up
> > > > the CC list when sending v4.
> > > > 
> > > > Philippe and Keith, please add a Reviewed-by to
> > > > 
> > > >    * "nvme: factor out pmr setup" and
> > > >    * "do cmb/pmr init as part of pci init"
> > > > 
> > > > since the first one was added and the second one was changed in v4 when
> > > > rebasing on Kevins block-next tree which had the PMR work that was not
> > > > in master at the time.
> > > > 
> > > > With those in place, it should be ready for Kevin to merge.
> > > > 
> > > Gentle ping on this.
> > > 
> > > Also, please see the two patches in "[PATCH 0/2] hw/block/nvme: fixes
> > > for interrupt behavior". I think they should go in preparation to this
> > > series.
> > 
> > I was going to ping Kevin last week, but then read your comment on pach #7
> > "nvme: add max_ioqpairs device parameter", so I interpreted you would
> > respin.
> > Now it is clearer, applying in the following order you don't need to respin,
> > right?
> > 
> > - [PATCH 0/2] hw/block/nvme: fixes for interrupt behavior"
> > - [PATCH v5 00/18] nvme: refactoring and cleanups

I was waiting for the review Klaus asked for. You had a comment about
renaming patches, but I didn't see any comments about the patches in
question.

> Ugh. "[PATCH v5 00/18] nvme: refactoring and cleanups" doesn't apply
> completely cleanly.
> 
> "[PATCH 0/2] hw/block/nvme: fixes for interrupt behavior" was intented
> to go into master because it fixes a bug, that is why I split them up.
> 
> But looks like it is better to just roll it into this series. I'll
> respin a v6 with the two interrupt fixes.

Ok, I'll wait for that one then. I'm still not sure, though, whether I
should then wait for additional review or just apply the patches.

Kevin