Re: [PATCH 00/29] nvme-fc/nvmet-fc: Add FC-NVME-2 disconnect association support

[Hannes Reinecke <hare@suse.de>]

On 2/5/20 7:37 PM, James Smart wrote:
> At the tail end of FC-NVME (1) standardization, the process for
> terminating an association was changed requiring interlock using FC-NVME
> Disconnect Assocation LS's between both the host port and target port.
> This was immediately relaxed with an ammendment to FC-NVME (1) and with
> wording put into FC-NVME-2. The interlock was removed, but it still
> required both the host port and target port to initiate Disconnect
> Association LS's and respond to LS's.
> 
> The linux nvme-fc and nvmet-fc implementations were interoperable with
> standards but the linux-driver api did not support all the functionality
> needed. It was missing:
> - nvme-fc: didn't support the reception of NVME LS's and the ability to
>   transmit responses to an LS.
> - nvmet-fc: didn't support the ability to send an NVME LS request. It
>   also did not support a method for the transport to specify a remote
>   port for an LS.
> 
> This patch adds the missing functionality. Specifically the patch set:
> - Updates the header with the FC-NVME-2 standard out for final approval.
> - Refactors data structure names that used to be dependent on role (ls
>   requests were specific to nvme; ls responses were specific to nvmet)
>   to generic names that can be used by both nvme-fc and nvmet-fc.
> - Modifies the nvme-fc transport template with interfaces to receive
>   NVME LS's and for the transport to then request LS responses to be
>   sent.
> - Modifies the nvmet-fc transport template with:
>   - The current NVME LS receive interface was modified to supply a
>     handle to indentify the remote port the LS as received from. If
>     the LS creates an association, the handle may be used to initiate
>     NVME LS requests to the remote port. An interface was put in place
>     to invalidate the handle on connectivity losses.
>   - Interfaces for the transport to request an NVME LS request to be
>     performed as well as to abort that LS in cases of error/teardown. 
> - The nvme-fc transport was modified to follow the standard:
>   - Disconnect association logic was revised to send Disconnect LS as
>     soon as all ABTS's were transmit rather than waiting for the ABTS
>     process to fully complete.
>   - Disconnect LS reception is supported, with reception initiating
>     controller reset and reconnect.
>   - Disconnect LS responses will not be transmit until association
>     termination has transmit the Disconnect LS.
> - The nvmet-fc transport was modified to follow the standard:
>   - Disconnect assocation logic was revised to transmit a Disconnect LS
>     request as soon as all ABTS's have been transmit. In the past, no
>     Disconnect LS had been transmit.
>   - Disconnect LS responses will not be sent until the Disconnect LS
>     request has been transmit.
> - nvme-fcloop: was updated with interfaces to allow testing of the
>   transports.
> - Along the way, cleanups and slight corrections were made to the
>   transports.
> - The lpfc driver was modified to support the new transport interfaces
>   for both the nvme and nvmet transports.  As much of the functionality
>   was already present, but specific to one side of the transport,
>   existing code was refactored to create common routines. Addition of
>   the new interfaces was able to slip in rather easily with the common
>   routines.
> 
> This code was cut against the for-5.6 branch.
> 
> I'll work with Martin to minimize any work to merge these lpfc mods 
> with lpfc changes in the scsi tree.
> 
> -- james
> 
> 
> 
> James Smart (29):
>   nvme-fc: Sync header to FC-NVME-2 rev 1.08
>   nvmet-fc: fix typo in comment
>   nvme-fc and nvmet-fc: revise LLDD api for LS reception and LS request
>   nvme-fc nvmet_fc nvme_fcloop: adapt code to changed names in api
>     header
>   lpfc: adapt code to changed names in api header
>   nvme-fcloop: Fix deallocation of working context
>   nvme-fc nvmet-fc: refactor for common LS definitions
>   nvmet-fc: Better size LS buffers
>   nvme-fc: Ensure private pointers are NULL if no data
>   nvmefc: Use common definitions for LS names, formatting, and
>     validation
>   nvme-fc: convert assoc_active flag to atomic
>   nvme-fc: Add Disconnect Association Rcv support
>   nvmet-fc: add LS failure messages
>   nvmet-fc: perform small cleanups on unneeded checks
>   nvmet-fc: track hostport handle for associations
>   nvmet-fc: rename ls_list to ls_rcv_list
>   nvmet-fc: Add Disconnect Association Xmt support
>   nvme-fcloop: refactor to enable target to host LS
>   nvme-fcloop: add target to host LS request support
>   lpfc: Refactor lpfc nvme headers
>   lpfc: Refactor nvmet_rcv_ctx to create lpfc_async_xchg_ctx
>   lpfc: Commonize lpfc_async_xchg_ctx state and flag definitions
>   lpfc: Refactor NVME LS receive handling
>   lpfc: Refactor Send LS Request support
>   lpfc: Refactor Send LS Abort support
>   lpfc: Refactor Send LS Response support
>   lpfc: nvme: Add Receive LS Request and Send LS Response support to
>     nvme
>   lpfc: nvmet: Add support for NVME LS request hosthandle
>   lpfc: nvmet: Add Send LS Request and Abort LS Request support
> 
>  drivers/nvme/host/fc.c             | 555 +++++++++++++++++++-----
>  drivers/nvme/host/fc.h             | 227 ++++++++++
>  drivers/nvme/target/fc.c           | 800 +++++++++++++++++++++++++----------
>  drivers/nvme/target/fcloop.c       | 228 ++++++++--
>  drivers/scsi/lpfc/lpfc.h           |   2 +-
>  drivers/scsi/lpfc/lpfc_attr.c      |   3 -
>  drivers/scsi/lpfc/lpfc_crtn.h      |   9 +-
>  drivers/scsi/lpfc/lpfc_ct.c        |   1 -
>  drivers/scsi/lpfc/lpfc_debugfs.c   |   5 +-
>  drivers/scsi/lpfc/lpfc_hbadisc.c   |   8 +-
>  drivers/scsi/lpfc/lpfc_init.c      |   7 +-
>  drivers/scsi/lpfc/lpfc_mem.c       |   4 -
>  drivers/scsi/lpfc/lpfc_nportdisc.c |  13 +-
>  drivers/scsi/lpfc/lpfc_nvme.c      | 550 ++++++++++++++++--------
>  drivers/scsi/lpfc/lpfc_nvme.h      | 198 +++++++++
>  drivers/scsi/lpfc/lpfc_nvmet.c     | 837 +++++++++++++++++++++++--------------
>  drivers/scsi/lpfc/lpfc_nvmet.h     | 158 -------
>  drivers/scsi/lpfc/lpfc_sli.c       | 126 +++++-
>  include/linux/nvme-fc-driver.h     | 368 +++++++++++-----
>  include/linux/nvme-fc.h            |   9 +-
>  20 files changed, 2970 insertions(+), 1138 deletions(-)
>  create mode 100644 drivers/nvme/host/fc.h
>  delete mode 100644 drivers/scsi/lpfc/lpfc_nvmet.h
> 
As mentioned, this patchset resolves one crash I've found when running
my nvme/034 blktest. However, we're still not quite there yet, as the
very same test now results in:

[  686.272596] (NULL device *): {1:0} Association deleted
[  686.318243] (NULL device *): {1:0} Association freed
[ 1309.673673] kmemleak: 2 new suspected memory leaks (see
/sys/kernel/debug/kmemleak)
[ 1488.024222] INFO: task kworker/4:2:656 blocked for more than 491 seconds.
[ 1488.085196]       Tainted: G            E     5.6.0-rc1-default+ #518
[ 1488.142347] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs"
disables this message.
[ 1488.211323] kworker/4:2     D    0   656      2 0x80004000
[ 1488.211329] Workqueue: events fcloop_tport_lsrqst_work [nvme_fcloop]
[ 1488.211331] Call Trace:
[ 1488.211337]  ? __schedule+0x28b/0x720
[ 1488.369706]  schedule+0x40/0xb0
[ 1488.369708]  schedule_timeout+0x1dd/0x300
[ 1488.369713]  ? free_pcp_prepare+0x59/0x1d0
[ 1488.469700]  wait_for_completion+0xba/0x140
[ 1488.469702]  ? wake_up_q+0xa0/0xa0
[ 1488.469705]  __flush_work+0x177/0x1b0
[ 1488.469708]  ? worker_detach_from_pool+0xa0/0xa0
[ 1488.610280]  fcloop_targetport_delete+0x13/0x20 [nvme_fcloop]
[ 1488.610284]  nvmet_fc_tgtport_put+0x150/0x190 [nvmet_fc]
[ 1488.706952]  nvmet_fc_disconnect_assoc_done+0x9a/0xe0 [nvmet_fc]
[ 1488.706954]  fcloop_tport_lsrqst_work+0x7a/0xa0 [nvme_fcloop]
[ 1488.706957]  process_one_work+0x208/0x400
[ 1488.706959]  worker_thread+0x2d/0x3e0
[ 1488.706961]  ? process_one_work+0x400/0x400
[ 1488.706963]  kthread+0x117/0x130
[ 1488.706966]  ? kthread_create_worker_on_cpu+0x70/0x70
[ 1488.985004]  ret_from_fork+0x35/0x40

So we do miss a completion somewhere ...

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		           Kernel Storage Architect
hare@suse.de			                  +49 911 74053 688
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), GF: Felix Imendörffer

_______________________________________________
linux-nvme mailing list
linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme