Re: [PATCH v1 0/7] Centrelize common fabrics code to core drivers

[James Smart <jsmart2021@gmail.com>]

On 10/18/2021 6:40 AM, Max Gurtovoy wrote:
> Hi Christoph, Sagi, Keith and Co,
> This series is intended to move common reconnection, device removal and
> error recovery logic to common fabrics and core drivers. In this stage,
> it mainly deals with the RDMA and TCP drivers which had the same logic
> for the above tasks. FC driver can start using this framework in the
> future after some preparation work that should be done.
> 
> This will ease on code maintenance and reduce the code duplication
> between the RDMA and TCP drivers.
> 
> Since there are some attributes that were moved to generic nvme_ctrl
> structure, I've measured perf impact.
> 
> NVMe PCI Perf numbers using NVIDIA's NVMe SNAP Bluefield-2 device
> (Fio 16 jobs, 128 iodepth, libaio):
> 
> IO size   rand READ (before/after)  rand WRITE (before/after)
> -------- -------------------------  --------------------------
>   512B          2189k/2197k                2031k/2092k
>   1KB           2188k/2201k                2027k/2088k
>   2KB           2166k/2181k                2023k/2078k
>   4KB           2153k/2169k                2025k/2076k
>   8KB           1381k/1381k                1378k/1375k
>   16KB          669k/676k                  660k/664k
>   32KB          320k/322k                  305k/305k
>   64KB          157k/157k                  151k/151k
> 
> As we can see, there is no impact on the performance.
> 
> Max Gurtovoy (7):
>    nvme: add connect_work attribute to nvme ctrl
>    nvme-fabrics: introduce nvmf_reconnect_or_remove API
>    nvme: add err_work attribute to nvme ctrl
>    nvme-fabrics: introduce nvmf_error_recovery API
>    nvme/nvme-fabrics: introduce nvmf_error_recovery_work API
>    nvme/nvme-fabrics: introduce nvmf_reconnect_ctrl_work API
>    nvme-fabrics: add nvmf_init_ctrl/nvmf_teardown_ctrl API
> 
>   drivers/nvme/host/fabrics.c |  90 ++++++++++++++++++++++++
>   drivers/nvme/host/fabrics.h |   4 ++
>   drivers/nvme/host/fc.c      |  23 ++++---
>   drivers/nvme/host/nvme.h    |   7 ++
>   drivers/nvme/host/rdma.c    | 134 ++++++++++--------------------------
>   drivers/nvme/host/tcp.c     | 124 ++++++++-------------------------
>   6 files changed, 180 insertions(+), 202 deletions(-)
> 

Thanks Max.

I've been thinking we needed to do this for a while as well.  FC should 
be part of this, and up front, so the abstraction is tested properly. 
I'd rather merge now than spin things again.  I'll go through this over 
the next day or so.

-- james