* [PATCH 0/3] efct fixes & improvements
@ 2021-09-14 10:55 Dmitry Bogdanov
2021-09-14 10:55 ` [PATCH 1/3] scsi: efct: add state in nport sm trace printout Dmitry Bogdanov
` (4 more replies)
0 siblings, 5 replies; 9+ messages in thread
From: Dmitry Bogdanov @ 2021-09-14 10:55 UTC (permalink / raw)
To: Martin Petersen, target-devel
Cc: linux-scsi, linux, James Smart, Dmitry Bogdanov
This patchset contains fixes of some isues that were found during
evaluation of Emulex HBA as a target.
This patchset is intended for scsi-queue.
Dmitry Bogdanov (3):
scsi: efct: add state in nport sm trace printout
scsi: efct: fix nport free
scsi: efct: decrease area under spinlock
drivers/scsi/elx/efct/efct_scsi.c | 3 +--
drivers/scsi/elx/libefc/efc.h | 2 +-
drivers/scsi/elx/libefc/efc_cmds.c | 7 ++++++-
drivers/scsi/elx/libefc/efclib.h | 1 +
4 files changed, 9 insertions(+), 4 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH 1/3] scsi: efct: add state in nport sm trace printout 2021-09-14 10:55 [PATCH 0/3] efct fixes & improvements Dmitry Bogdanov @ 2021-09-14 10:55 ` Dmitry Bogdanov 2021-09-15 12:45 ` Ram Kishore Vegesna 2021-09-14 10:55 ` [PATCH 2/3] scsi: efct: fix nport free Dmitry Bogdanov ` (3 subsequent siblings) 4 siblings, 1 reply; 9+ messages in thread From: Dmitry Bogdanov @ 2021-09-14 10:55 UTC (permalink / raw) To: Martin Petersen, target-devel Cc: linux-scsi, linux, James Smart, Dmitry Bogdanov Similar to other state machine traces and to make debug easier add the state name to nport sm trace printout. Signed-off-by: Dmitry Bogdanov <d.bogdanov@yadro.com> --- drivers/scsi/elx/libefc/efc.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/scsi/elx/libefc/efc.h b/drivers/scsi/elx/libefc/efc.h index 927016283f41..468ff3cc9c00 100644 --- a/drivers/scsi/elx/libefc/efc.h +++ b/drivers/scsi/elx/libefc/efc.h @@ -47,6 +47,6 @@ enum efc_scsi_del_target_reason { #define nport_sm_trace(nport) \ efc_log_debug(nport->efc, \ - "[%s] %-20s\n", nport->display_name, efc_sm_event_name(evt)) \ + "[%s] %-20s %-20s\n", nport->display_name, __func__, efc_sm_event_name(evt)) \ #endif /* __EFC_H__ */ -- 2.25.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/3] scsi: efct: add state in nport sm trace printout 2021-09-14 10:55 ` [PATCH 1/3] scsi: efct: add state in nport sm trace printout Dmitry Bogdanov @ 2021-09-15 12:45 ` Ram Kishore Vegesna 0 siblings, 0 replies; 9+ messages in thread From: Ram Kishore Vegesna @ 2021-09-15 12:45 UTC (permalink / raw) To: Dmitry Bogdanov Cc: Martin Petersen, target-devel, linux-scsi, linux, James Smart [-- Attachment #1: Type: text/plain, Size: 1756 bytes --] Looks good. Thanks. Reviewed-by: Ram Vegesna <ram.vegesna@broadcom.com> On Tue, Sep 14, 2021 at 4:25 PM Dmitry Bogdanov <d.bogdanov@yadro.com> wrote: > > Similar to other state machine traces and to make debug easier add the > state name to nport sm trace printout. > > Signed-off-by: Dmitry Bogdanov <d.bogdanov@yadro.com> > --- > drivers/scsi/elx/libefc/efc.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/scsi/elx/libefc/efc.h b/drivers/scsi/elx/libefc/efc.h > index 927016283f41..468ff3cc9c00 100644 > --- a/drivers/scsi/elx/libefc/efc.h > +++ b/drivers/scsi/elx/libefc/efc.h > @@ -47,6 +47,6 @@ enum efc_scsi_del_target_reason { > > #define nport_sm_trace(nport) \ > efc_log_debug(nport->efc, \ > - "[%s] %-20s\n", nport->display_name, efc_sm_event_name(evt)) \ > + "[%s] %-20s %-20s\n", nport->display_name, __func__, efc_sm_event_name(evt)) \ > > #endif /* __EFC_H__ */ > -- > 2.25.1 > -- This electronic communication and the information and any files transmitted with it, or attached to it, are confidential and are intended solely for the use of the individual or entity to whom it is addressed and may contain information that is confidential, legally privileged, protected by privacy laws, or otherwise restricted from disclosure to anyone else. If you are not the intended recipient or the person responsible for delivering the e-mail to the intended recipient, you are hereby notified that any use, copying, distributing, dissemination, forwarding, printing, or copying of this e-mail is strictly prohibited. If you received this e-mail in error, please return the e-mail to the sender, delete it from your computer, and destroy any printed copy of it. [-- Attachment #2: S/MIME Cryptographic Signature --] [-- Type: application/pkcs7-signature, Size: 4214 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/3] scsi: efct: fix nport free 2021-09-14 10:55 [PATCH 0/3] efct fixes & improvements Dmitry Bogdanov 2021-09-14 10:55 ` [PATCH 1/3] scsi: efct: add state in nport sm trace printout Dmitry Bogdanov @ 2021-09-14 10:55 ` Dmitry Bogdanov 2021-09-15 12:51 ` Ram Kishore Vegesna 2021-09-14 10:55 ` [PATCH 3/3] scsi: efct: decrease area under spinlock Dmitry Bogdanov ` (2 subsequent siblings) 4 siblings, 1 reply; 9+ messages in thread From: Dmitry Bogdanov @ 2021-09-14 10:55 UTC (permalink / raw) To: Martin Petersen, target-devel Cc: linux-scsi, linux, James Smart, Dmitry Bogdanov nport_free for an empty nport hangs the state machine waiting for mbox completion if nport is not yet attached thinking that it is attaching right now. Add a check for nport attaching state and complete nport free. Signed-off-by: Dmitry Bogdanov <d.bogdanov@yadro.com> --- drivers/scsi/elx/libefc/efc_cmds.c | 7 ++++++- drivers/scsi/elx/libefc/efclib.h | 1 + 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/scsi/elx/libefc/efc_cmds.c b/drivers/scsi/elx/libefc/efc_cmds.c index 37e6697d86b8..f8665d48904a 100644 --- a/drivers/scsi/elx/libefc/efc_cmds.c +++ b/drivers/scsi/elx/libefc/efc_cmds.c @@ -249,6 +249,7 @@ efc_nport_attach_reg_vpi_cb(struct efc *efc, int status, u8 *mqe, { struct efc_nport *nport = arg; + nport->attaching = false; if (efc_nport_get_mbox_status(nport, mqe, status)) { efc_nport_free_resources(nport, EFC_EVT_NPORT_ATTACH_FAIL, mqe); return -EIO; @@ -286,6 +287,8 @@ efc_cmd_nport_attach(struct efc *efc, struct efc_nport *nport, u32 fc_id) if (rc) { efc_log_err(efc, "REG_VPI command failure\n"); efc_nport_free_resources(nport, EFC_EVT_NPORT_ATTACH_FAIL, buf); + } else { + nport->attaching = true; } return rc; @@ -302,8 +305,10 @@ efc_cmd_nport_free(struct efc *efc, struct efc_nport *nport) /* Issue the UNREG_VPI command to free the assigned VPI context */ if (nport->attached) efc_nport_free_unreg_vpi(nport); - else + else if (nport->attaching) nport->free_req_pending = true; + else + efc_sm_post_event(&nport->sm, EFC_EVT_NPORT_FREE_OK, NULL); return 0; } diff --git a/drivers/scsi/elx/libefc/efclib.h b/drivers/scsi/elx/libefc/efclib.h index ee291cabf7e0..dde20891c2dd 100644 --- a/drivers/scsi/elx/libefc/efclib.h +++ b/drivers/scsi/elx/libefc/efclib.h @@ -142,6 +142,7 @@ struct efc_nport { bool is_vport; bool free_req_pending; bool attached; + bool attaching; bool p2p_winner; struct efc_domain *domain; u64 wwpn; -- 2.25.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/3] scsi: efct: fix nport free 2021-09-14 10:55 ` [PATCH 2/3] scsi: efct: fix nport free Dmitry Bogdanov @ 2021-09-15 12:51 ` Ram Kishore Vegesna 0 siblings, 0 replies; 9+ messages in thread From: Ram Kishore Vegesna @ 2021-09-15 12:51 UTC (permalink / raw) To: Dmitry Bogdanov Cc: Martin Petersen, target-devel, linux-scsi, linux, James Smart [-- Attachment #1: Type: text/plain, Size: 3341 bytes --] On Tue, Sep 14, 2021 at 4:25 PM Dmitry Bogdanov <d.bogdanov@yadro.com> wrote: > > nport_free for an empty nport hangs the state machine waiting for mbox > completion if nport is not yet attached thinking that it is attaching > right now. > Add a check for nport attaching state and complete nport free. > > Signed-off-by: Dmitry Bogdanov <d.bogdanov@yadro.com> > --- > drivers/scsi/elx/libefc/efc_cmds.c | 7 ++++++- > drivers/scsi/elx/libefc/efclib.h | 1 + > 2 files changed, 7 insertions(+), 1 deletion(-) > > diff --git a/drivers/scsi/elx/libefc/efc_cmds.c b/drivers/scsi/elx/libefc/efc_cmds.c > index 37e6697d86b8..f8665d48904a 100644 > --- a/drivers/scsi/elx/libefc/efc_cmds.c > +++ b/drivers/scsi/elx/libefc/efc_cmds.c > @@ -249,6 +249,7 @@ efc_nport_attach_reg_vpi_cb(struct efc *efc, int status, u8 *mqe, > { > struct efc_nport *nport = arg; > > + nport->attaching = false; > if (efc_nport_get_mbox_status(nport, mqe, status)) { > efc_nport_free_resources(nport, EFC_EVT_NPORT_ATTACH_FAIL, mqe); > return -EIO; > @@ -286,6 +287,8 @@ efc_cmd_nport_attach(struct efc *efc, struct efc_nport *nport, u32 fc_id) > if (rc) { > efc_log_err(efc, "REG_VPI command failure\n"); > efc_nport_free_resources(nport, EFC_EVT_NPORT_ATTACH_FAIL, buf); > + } else { > + nport->attaching = true; > } > > return rc; > @@ -302,8 +305,10 @@ efc_cmd_nport_free(struct efc *efc, struct efc_nport *nport) > /* Issue the UNREG_VPI command to free the assigned VPI context */ > if (nport->attached) > efc_nport_free_unreg_vpi(nport); > - else > + else if (nport->attaching) > nport->free_req_pending = true; > + else > + efc_sm_post_event(&nport->sm, EFC_EVT_NPORT_FREE_OK, NULL); > > return 0; > } > diff --git a/drivers/scsi/elx/libefc/efclib.h b/drivers/scsi/elx/libefc/efclib.h > index ee291cabf7e0..dde20891c2dd 100644 > --- a/drivers/scsi/elx/libefc/efclib.h > +++ b/drivers/scsi/elx/libefc/efclib.h > @@ -142,6 +142,7 @@ struct efc_nport { > bool is_vport; > bool free_req_pending; > bool attached; > + bool attaching; > bool p2p_winner; > struct efc_domain *domain; > u64 wwpn; > -- > 2.25.1 > Looks good. Thanks. Reviewed-by: Ram Vegesna <ram.vegesna@broadcom.com> -- This electronic communication and the information and any files transmitted with it, or attached to it, are confidential and are intended solely for the use of the individual or entity to whom it is addressed and may contain information that is confidential, legally privileged, protected by privacy laws, or otherwise restricted from disclosure to anyone else. If you are not the intended recipient or the person responsible for delivering the e-mail to the intended recipient, you are hereby notified that any use, copying, distributing, dissemination, forwarding, printing, or copying of this e-mail is strictly prohibited. If you received this e-mail in error, please return the e-mail to the sender, delete it from your computer, and destroy any printed copy of it. [-- Attachment #2: S/MIME Cryptographic Signature --] [-- Type: application/pkcs7-signature, Size: 4214 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 3/3] scsi: efct: decrease area under spinlock 2021-09-14 10:55 [PATCH 0/3] efct fixes & improvements Dmitry Bogdanov 2021-09-14 10:55 ` [PATCH 1/3] scsi: efct: add state in nport sm trace printout Dmitry Bogdanov 2021-09-14 10:55 ` [PATCH 2/3] scsi: efct: fix nport free Dmitry Bogdanov @ 2021-09-14 10:55 ` Dmitry Bogdanov 2021-09-15 12:51 ` Ram Kishore Vegesna 2021-09-22 4:05 ` [PATCH 0/3] efct fixes & improvements Martin K. Petersen 2021-09-29 4:20 ` Martin K. Petersen 4 siblings, 1 reply; 9+ messages in thread From: Dmitry Bogdanov @ 2021-09-14 10:55 UTC (permalink / raw) To: Martin Petersen, target-devel Cc: linux-scsi, linux, James Smart, Dmitry Bogdanov, Roman Bolshakov Under session level spinlock node->active_ios_lock in efct_scsi_io_alloc() there is a getting other spinlock of port level. That lead to competition between sessions and even between IOs in the same session due too much instructions under spinlock. This change reduces spinlock area just to active_ios list for which active_ios_lock is intended. Spinlock CPU usage is decreased from 18% down to 13% in efct driver. IOPS are increased from 220 kIOPS upto 264 kIOPS for one lun on my setup. Reviewed-by: Roman Bolshakov <r.bolshakov@yadro.com> Signed-off-by: Dmitry Bogdanov <d.bogdanov@yadro.com> --- drivers/scsi/elx/efct/efct_scsi.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/scsi/elx/efct/efct_scsi.c b/drivers/scsi/elx/efct/efct_scsi.c index 40fb3a724c76..8535bb7eabd8 100644 --- a/drivers/scsi/elx/efct/efct_scsi.c +++ b/drivers/scsi/elx/efct/efct_scsi.c @@ -38,8 +38,6 @@ efct_scsi_io_alloc(struct efct_node *node) xport = efct->xport; - spin_lock_irqsave(&node->active_ios_lock, flags); - io = efct_io_pool_io_alloc(efct->xport->io_pool); if (!io) { efc_log_err(efct, "IO alloc Failed\n"); @@ -66,6 +64,7 @@ efct_scsi_io_alloc(struct efct_node *node) /* Add to node's active_ios list */ INIT_LIST_HEAD(&io->list_entry); + spin_lock_irqsave(&node->active_ios_lock, flags); list_add(&io->list_entry, &node->active_ios); spin_unlock_irqrestore(&node->active_ios_lock, flags); -- 2.25.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 3/3] scsi: efct: decrease area under spinlock 2021-09-14 10:55 ` [PATCH 3/3] scsi: efct: decrease area under spinlock Dmitry Bogdanov @ 2021-09-15 12:51 ` Ram Kishore Vegesna 0 siblings, 0 replies; 9+ messages in thread From: Ram Kishore Vegesna @ 2021-09-15 12:51 UTC (permalink / raw) To: Dmitry Bogdanov Cc: Martin Petersen, target-devel, linux-scsi, linux, James Smart, Roman Bolshakov [-- Attachment #1: Type: text/plain, Size: 2527 bytes --] On Tue, Sep 14, 2021 at 4:25 PM Dmitry Bogdanov <d.bogdanov@yadro.com> wrote: > > Under session level spinlock node->active_ios_lock in > efct_scsi_io_alloc() there is a getting other spinlock of port level. > That lead to competition between sessions and even between IOs in the > same session due too much instructions under spinlock. > > This change reduces spinlock area just to active_ios list for which > active_ios_lock is intended. > Spinlock CPU usage is decreased from 18% down to 13% in efct driver. > IOPS are increased from 220 kIOPS upto 264 kIOPS for one lun on my setup. > > Reviewed-by: Roman Bolshakov <r.bolshakov@yadro.com> > Signed-off-by: Dmitry Bogdanov <d.bogdanov@yadro.com> > --- > drivers/scsi/elx/efct/efct_scsi.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/scsi/elx/efct/efct_scsi.c b/drivers/scsi/elx/efct/efct_scsi.c > index 40fb3a724c76..8535bb7eabd8 100644 > --- a/drivers/scsi/elx/efct/efct_scsi.c > +++ b/drivers/scsi/elx/efct/efct_scsi.c > @@ -38,8 +38,6 @@ efct_scsi_io_alloc(struct efct_node *node) > > xport = efct->xport; > > - spin_lock_irqsave(&node->active_ios_lock, flags); > - > io = efct_io_pool_io_alloc(efct->xport->io_pool); > if (!io) { > efc_log_err(efct, "IO alloc Failed\n"); > @@ -66,6 +64,7 @@ efct_scsi_io_alloc(struct efct_node *node) > > /* Add to node's active_ios list */ > INIT_LIST_HEAD(&io->list_entry); > + spin_lock_irqsave(&node->active_ios_lock, flags); > list_add(&io->list_entry, &node->active_ios); > > spin_unlock_irqrestore(&node->active_ios_lock, flags); > -- > 2.25.1 > Looks good. Thanks. Reviewed-by: Ram Vegesna <ram.vegesna@broadcom.com> -- This electronic communication and the information and any files transmitted with it, or attached to it, are confidential and are intended solely for the use of the individual or entity to whom it is addressed and may contain information that is confidential, legally privileged, protected by privacy laws, or otherwise restricted from disclosure to anyone else. If you are not the intended recipient or the person responsible for delivering the e-mail to the intended recipient, you are hereby notified that any use, copying, distributing, dissemination, forwarding, printing, or copying of this e-mail is strictly prohibited. If you received this e-mail in error, please return the e-mail to the sender, delete it from your computer, and destroy any printed copy of it. [-- Attachment #2: S/MIME Cryptographic Signature --] [-- Type: application/pkcs7-signature, Size: 4214 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/3] efct fixes & improvements 2021-09-14 10:55 [PATCH 0/3] efct fixes & improvements Dmitry Bogdanov ` (2 preceding siblings ...) 2021-09-14 10:55 ` [PATCH 3/3] scsi: efct: decrease area under spinlock Dmitry Bogdanov @ 2021-09-22 4:05 ` Martin K. Petersen 2021-09-29 4:20 ` Martin K. Petersen 4 siblings, 0 replies; 9+ messages in thread From: Martin K. Petersen @ 2021-09-22 4:05 UTC (permalink / raw) To: Dmitry Bogdanov Cc: Martin Petersen, target-devel, linux-scsi, linux, James Smart Dmitry, > This patchset contains fixes of some isues that were found during > evaluation of Emulex HBA as a target. Applied to 5.16/scsi-staging, thanks! -- Martin K. Petersen Oracle Linux Engineering ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/3] efct fixes & improvements 2021-09-14 10:55 [PATCH 0/3] efct fixes & improvements Dmitry Bogdanov ` (3 preceding siblings ...) 2021-09-22 4:05 ` [PATCH 0/3] efct fixes & improvements Martin K. Petersen @ 2021-09-29 4:20 ` Martin K. Petersen 4 siblings, 0 replies; 9+ messages in thread From: Martin K. Petersen @ 2021-09-29 4:20 UTC (permalink / raw) To: Dmitry Bogdanov, target-devel Cc: Martin K . Petersen, linux-scsi, linux, James Smart On Tue, 14 Sep 2021 13:55:36 +0300, Dmitry Bogdanov wrote: > This patchset contains fixes of some isues that were found during > evaluation of Emulex HBA as a target. > > This patchset is intended for scsi-queue. > > Dmitry Bogdanov (3): > scsi: efct: add state in nport sm trace printout > scsi: efct: fix nport free > scsi: efct: decrease area under spinlock > > [...] Applied to 5.16/scsi-queue, thanks! [1/3] scsi: efct: add state in nport sm trace printout https://git.kernel.org/mkp/scsi/c/8d4efd0040e5 [2/3] scsi: efct: fix nport free https://git.kernel.org/mkp/scsi/c/ee3dce9f3842 [3/3] scsi: efct: decrease area under spinlock https://git.kernel.org/mkp/scsi/c/e76b7c5e25a1 -- Martin K. Petersen Oracle Linux Engineering ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2021-09-29 4:20 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-09-14 10:55 [PATCH 0/3] efct fixes & improvements Dmitry Bogdanov 2021-09-14 10:55 ` [PATCH 1/3] scsi: efct: add state in nport sm trace printout Dmitry Bogdanov 2021-09-15 12:45 ` Ram Kishore Vegesna 2021-09-14 10:55 ` [PATCH 2/3] scsi: efct: fix nport free Dmitry Bogdanov 2021-09-15 12:51 ` Ram Kishore Vegesna 2021-09-14 10:55 ` [PATCH 3/3] scsi: efct: decrease area under spinlock Dmitry Bogdanov 2021-09-15 12:51 ` Ram Kishore Vegesna 2021-09-22 4:05 ` [PATCH 0/3] efct fixes & improvements Martin K. Petersen 2021-09-29 4:20 ` Martin K. Petersen
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).