Skip to content

Commit 9bb3677

Browse files
Justin Teemartinkpetersen
Justin Tee
authored and
martinkpetersen
committed
scsi: lpfc: Protect vport fc_nodes list with an explicit spin lock
In attempt to reduce the amount of unnecessary shost_lock acquisitions in the lpfc driver, replace shost_lock with an explicit fc_nodes_list_lock spinlock when accessing vport->fc_nodes lists. Although vport memory region is owned by shost->hostdata, it is driver private memory and an explicit fc_nodes list lock for fc_nodes list mutations is more appropriate than locking the entire shost. Signed-off-by: Justin Tee <justin.tee@broadcom.com> Link: https://lore.kernel.org/r/20240131185112.149731-14-justintee8345@gmail.com Reviewed-by: Himanshu Madhani <himanshu.madhani@oracle.com> Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
1 parent 0dfd9cb commit 9bb3677

File tree

6 files changed

+53
-54
lines changed

6 files changed

+53
-54
lines changed

‎drivers/scsi/lpfc/lpfc.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -587,6 +587,7 @@ struct lpfc_vport {
587587
#define FC_CT_RPRT_DEFER 0x20 /* Defer issuing FDMI RPRT */
588588

589589
struct list_head fc_nodes;
590+
spinlock_t fc_nodes_list_lock; /* spinlock for fc_nodes list */
590591

591592
/* Keep counters for the number of entries in each list. */
592593
atomic_t fc_plogi_cnt;

‎drivers/scsi/lpfc/lpfc_attr.c

Lines changed: 18 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -344,6 +344,7 @@ lpfc_nvme_info_show(struct device *dev, struct device_attribute *attr,
344344
struct lpfc_fc4_ctrl_stat *cstat;
345345
uint64_t data1, data2, data3;
346346
uint64_t totin, totout, tot;
347+
unsigned long iflags;
347348
char *statep;
348349
int i;
349350
int len = 0;
@@ -543,7 +544,7 @@ lpfc_nvme_info_show(struct device *dev, struct device_attribute *attr,
543544
if (strlcat(buf, tmp, PAGE_SIZE) >= PAGE_SIZE)
544545
goto buffer_done;
545546

546-
spin_lock_irq(shost->host_lock);
547+
spin_lock_irqsave(&vport->fc_nodes_list_lock, iflags);
547548

548549
list_for_each_entry(ndlp, &vport->fc_nodes, nlp_listp) {
549550
nrport = NULL;
@@ -617,7 +618,7 @@ lpfc_nvme_info_show(struct device *dev, struct device_attribute *attr,
617618
if (strlcat(buf, tmp, PAGE_SIZE) >= PAGE_SIZE)
618619
goto unlock_buf_done;
619620
}
620-
spin_unlock_irq(shost->host_lock);
621+
spin_unlock_irqrestore(&vport->fc_nodes_list_lock, iflags);
621622

622623
if (!lport)
623624
goto buffer_done;
@@ -681,7 +682,7 @@ lpfc_nvme_info_show(struct device *dev, struct device_attribute *attr,
681682
goto buffer_done;
682683

683684
unlock_buf_done:
684-
spin_unlock_irq(shost->host_lock);
685+
spin_unlock_irqrestore(&vport->fc_nodes_list_lock, iflags);
685686

686687
buffer_done:
687688
len = strnlen(buf, PAGE_SIZE);
@@ -3765,15 +3766,14 @@ lpfc_nodev_tmo_init(struct lpfc_vport *vport, int val)
37653766
static void
37663767
lpfc_update_rport_devloss_tmo(struct lpfc_vport *vport)
37673768
{
3768-
struct Scsi_Host *shost;
37693769
struct lpfc_nodelist *ndlp;
3770+
unsigned long iflags;
37703771
#if (IS_ENABLED(CONFIG_NVME_FC))
37713772
struct lpfc_nvme_rport *rport;
37723773
struct nvme_fc_remote_port *remoteport = NULL;
37733774
#endif
37743775

3775-
shost = lpfc_shost_from_vport(vport);
3776-
spin_lock_irq(shost->host_lock);
3776+
spin_lock_irqsave(&vport->fc_nodes_list_lock, iflags);
37773777
list_for_each_entry(ndlp, &vport->fc_nodes, nlp_listp) {
37783778
if (ndlp->rport)
37793779
ndlp->rport->dev_loss_tmo = vport->cfg_devloss_tmo;
@@ -3788,7 +3788,7 @@ lpfc_update_rport_devloss_tmo(struct lpfc_vport *vport)
37883788
vport->cfg_devloss_tmo);
37893789
#endif
37903790
}
3791-
spin_unlock_irq(shost->host_lock);
3791+
spin_unlock_irqrestore(&vport->fc_nodes_list_lock, iflags);
37923792
}
37933793

37943794
/**
@@ -3974,23 +3974,22 @@ lpfc_vport_param_init(tgt_queue_depth, LPFC_MAX_TGT_QDEPTH,
39743974
static int
39753975
lpfc_tgt_queue_depth_set(struct lpfc_vport *vport, uint val)
39763976
{
3977-
struct Scsi_Host *shost = lpfc_shost_from_vport(vport);
39783977
struct lpfc_nodelist *ndlp;
3978+
unsigned long iflags;
39793979

39803980
if (!lpfc_rangecheck(val, LPFC_MIN_TGT_QDEPTH, LPFC_MAX_TGT_QDEPTH))
39813981
return -EINVAL;
39823982

39833983
if (val == vport->cfg_tgt_queue_depth)
39843984
return 0;
39853985

3986-
spin_lock_irq(shost->host_lock);
39873986
vport->cfg_tgt_queue_depth = val;
39883987

39893988
/* Next loop thru nodelist and change cmd_qdepth */
3989+
spin_lock_irqsave(&vport->fc_nodes_list_lock, iflags);
39903990
list_for_each_entry(ndlp, &vport->fc_nodes, nlp_listp)
39913991
ndlp->cmd_qdepth = vport->cfg_tgt_queue_depth;
3992-
3993-
spin_unlock_irq(shost->host_lock);
3992+
spin_unlock_irqrestore(&vport->fc_nodes_list_lock, iflags);
39943993
return 0;
39953994
}
39963995

@@ -5236,22 +5235,22 @@ lpfc_vport_param_show(max_scsicmpl_time);
52365235
static int
52375236
lpfc_max_scsicmpl_time_set(struct lpfc_vport *vport, int val)
52385237
{
5239-
struct Scsi_Host *shost = lpfc_shost_from_vport(vport);
52405238
struct lpfc_nodelist *ndlp, *next_ndlp;
5239+
unsigned long iflags;
52415240

52425241
if (val == vport->cfg_max_scsicmpl_time)
52435242
return 0;
52445243
if ((val < 0) || (val > 60000))
52455244
return -EINVAL;
52465245
vport->cfg_max_scsicmpl_time = val;
52475246

5248-
spin_lock_irq(shost->host_lock);
5247+
spin_lock_irqsave(&vport->fc_nodes_list_lock, iflags);
52495248
list_for_each_entry_safe(ndlp, next_ndlp, &vport->fc_nodes, nlp_listp) {
52505249
if (ndlp->nlp_state == NLP_STE_UNUSED_NODE)
52515250
continue;
52525251
ndlp->cmd_qdepth = vport->cfg_tgt_queue_depth;
52535252
}
5254-
spin_unlock_irq(shost->host_lock);
5253+
spin_unlock_irqrestore(&vport->fc_nodes_list_lock, iflags);
52555254
return 0;
52565255
}
52575256
lpfc_vport_param_store(max_scsicmpl_time);
@@ -6853,17 +6852,19 @@ lpfc_get_node_by_target(struct scsi_target *starget)
68536852
struct Scsi_Host *shost = dev_to_shost(starget->dev.parent);
68546853
struct lpfc_vport *vport = (struct lpfc_vport *) shost->hostdata;
68556854
struct lpfc_nodelist *ndlp;
6855+
unsigned long iflags;
68566856

6857-
spin_lock_irq(shost->host_lock);
6857+
spin_lock_irqsave(&vport->fc_nodes_list_lock, iflags);
68586858
/* Search for this, mapped, target ID */
68596859
list_for_each_entry(ndlp, &vport->fc_nodes, nlp_listp) {
68606860
if (ndlp->nlp_state == NLP_STE_MAPPED_NODE &&
68616861
starget->id == ndlp->nlp_sid) {
6862-
spin_unlock_irq(shost->host_lock);
6862+
spin_unlock_irqrestore(&vport->fc_nodes_list_lock,
6863+
iflags);
68636864
return ndlp;
68646865
}
68656866
}
6866-
spin_unlock_irq(shost->host_lock);
6867+
spin_unlock_irqrestore(&vport->fc_nodes_list_lock, iflags);
68676868
return NULL;
68686869
}
68696870

‎drivers/scsi/lpfc/lpfc_ct.c

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1853,19 +1853,18 @@ static uint32_t
18531853
lpfc_find_map_node(struct lpfc_vport *vport)
18541854
{
18551855
struct lpfc_nodelist *ndlp, *next_ndlp;
1856-
struct Scsi_Host *shost;
1856+
unsigned long iflags;
18571857
uint32_t cnt = 0;
18581858

1859-
shost = lpfc_shost_from_vport(vport);
1860-
spin_lock_irq(shost->host_lock);
1859+
spin_lock_irqsave(&vport->fc_nodes_list_lock, iflags);
18611860
list_for_each_entry_safe(ndlp, next_ndlp, &vport->fc_nodes, nlp_listp) {
18621861
if (ndlp->nlp_type & NLP_FABRIC)
18631862
continue;
18641863
if ((ndlp->nlp_state == NLP_STE_MAPPED_NODE) ||
18651864
(ndlp->nlp_state == NLP_STE_UNMAPPED_NODE))
18661865
cnt++;
18671866
}
1868-
spin_unlock_irq(shost->host_lock);
1867+
spin_unlock_irqrestore(&vport->fc_nodes_list_lock, iflags);
18691868
return cnt;
18701869
}
18711870

‎drivers/scsi/lpfc/lpfc_debugfs.c

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -806,10 +806,10 @@ lpfc_debugfs_nodelist_data(struct lpfc_vport *vport, char *buf, int size)
806806
{
807807
int len = 0;
808808
int i, iocnt, outio, cnt;
809-
struct Scsi_Host *shost = lpfc_shost_from_vport(vport);
810809
struct lpfc_hba *phba = vport->phba;
811810
struct lpfc_nodelist *ndlp;
812811
unsigned char *statep;
812+
unsigned long iflags;
813813
struct nvme_fc_local_port *localport;
814814
struct nvme_fc_remote_port *nrport = NULL;
815815
struct lpfc_nvme_rport *rport;
@@ -818,7 +818,7 @@ lpfc_debugfs_nodelist_data(struct lpfc_vport *vport, char *buf, int size)
818818
outio = 0;
819819

820820
len += scnprintf(buf+len, size-len, "\nFCP Nodelist Entries ...\n");
821-
spin_lock_irq(shost->host_lock);
821+
spin_lock_irqsave(&vport->fc_nodes_list_lock, iflags);
822822
list_for_each_entry(ndlp, &vport->fc_nodes, nlp_listp) {
823823
iocnt = 0;
824824
if (!cnt) {
@@ -908,7 +908,7 @@ lpfc_debugfs_nodelist_data(struct lpfc_vport *vport, char *buf, int size)
908908
ndlp->nlp_defer_did);
909909
len += scnprintf(buf+len, size-len, "\n");
910910
}
911-
spin_unlock_irq(shost->host_lock);
911+
spin_unlock_irqrestore(&vport->fc_nodes_list_lock, iflags);
912912

913913
len += scnprintf(buf + len, size - len,
914914
"\nOutstanding IO x%x\n", outio);
@@ -940,8 +940,6 @@ lpfc_debugfs_nodelist_data(struct lpfc_vport *vport, char *buf, int size)
940940
if (!localport)
941941
goto out_exit;
942942

943-
spin_lock_irq(shost->host_lock);
944-
945943
/* Port state is only one of two values for now. */
946944
if (localport->port_id)
947945
statep = "ONLINE";
@@ -953,6 +951,7 @@ lpfc_debugfs_nodelist_data(struct lpfc_vport *vport, char *buf, int size)
953951
localport->port_id, statep);
954952

955953
len += scnprintf(buf + len, size - len, "\tRport List:\n");
954+
spin_lock_irqsave(&vport->fc_nodes_list_lock, iflags);
956955
list_for_each_entry(ndlp, &vport->fc_nodes, nlp_listp) {
957956
/* local short-hand pointer. */
958957
spin_lock(&ndlp->lock);
@@ -1006,8 +1005,7 @@ lpfc_debugfs_nodelist_data(struct lpfc_vport *vport, char *buf, int size)
10061005
/* Terminate the string. */
10071006
len += scnprintf(buf + len, size - len, "\n");
10081007
}
1009-
1010-
spin_unlock_irq(shost->host_lock);
1008+
spin_unlock_irqrestore(&vport->fc_nodes_list_lock, iflags);
10111009
out_exit:
10121010
return len;
10131011
}

‎drivers/scsi/lpfc/lpfc_hbadisc.c

Lines changed: 25 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -4860,10 +4860,10 @@ void
48604860
lpfc_nlp_set_state(struct lpfc_vport *vport, struct lpfc_nodelist *ndlp,
48614861
int state)
48624862
{
4863-
struct Scsi_Host *shost = lpfc_shost_from_vport(vport);
48644863
int old_state = ndlp->nlp_state;
48654864
int node_dropped = ndlp->nlp_flag & NLP_DROPPED;
48664865
char name1[16], name2[16];
4866+
unsigned long iflags;
48674867

48684868
lpfc_printf_vlog(vport, KERN_INFO, LOG_NODE,
48694869
"0904 NPort state transition x%06x, %s -> %s\n",
@@ -4890,9 +4890,9 @@ lpfc_nlp_set_state(struct lpfc_vport *vport, struct lpfc_nodelist *ndlp,
48904890
}
48914891

48924892
if (list_empty(&ndlp->nlp_listp)) {
4893-
spin_lock_irq(shost->host_lock);
4893+
spin_lock_irqsave(&vport->fc_nodes_list_lock, iflags);
48944894
list_add_tail(&ndlp->nlp_listp, &vport->fc_nodes);
4895-
spin_unlock_irq(shost->host_lock);
4895+
spin_unlock_irqrestore(&vport->fc_nodes_list_lock, iflags);
48964896
} else if (old_state)
48974897
lpfc_nlp_counters(vport, old_state, -1);
48984898

@@ -4904,26 +4904,26 @@ lpfc_nlp_set_state(struct lpfc_vport *vport, struct lpfc_nodelist *ndlp,
49044904
void
49054905
lpfc_enqueue_node(struct lpfc_vport *vport, struct lpfc_nodelist *ndlp)
49064906
{
4907-
struct Scsi_Host *shost = lpfc_shost_from_vport(vport);
4907+
unsigned long iflags;
49084908

49094909
if (list_empty(&ndlp->nlp_listp)) {
4910-
spin_lock_irq(shost->host_lock);
4910+
spin_lock_irqsave(&vport->fc_nodes_list_lock, iflags);
49114911
list_add_tail(&ndlp->nlp_listp, &vport->fc_nodes);
4912-
spin_unlock_irq(shost->host_lock);
4912+
spin_unlock_irqrestore(&vport->fc_nodes_list_lock, iflags);
49134913
}
49144914
}
49154915

49164916
void
49174917
lpfc_dequeue_node(struct lpfc_vport *vport, struct lpfc_nodelist *ndlp)
49184918
{
4919-
struct Scsi_Host *shost = lpfc_shost_from_vport(vport);
4919+
unsigned long iflags;
49204920

49214921
lpfc_cancel_retry_delay_tmo(vport, ndlp);
49224922
if (ndlp->nlp_state && !list_empty(&ndlp->nlp_listp))
49234923
lpfc_nlp_counters(vport, ndlp->nlp_state, -1);
4924-
spin_lock_irq(shost->host_lock);
4924+
spin_lock_irqsave(&vport->fc_nodes_list_lock, iflags);
49254925
list_del_init(&ndlp->nlp_listp);
4926-
spin_unlock_irq(shost->host_lock);
4926+
spin_unlock_irqrestore(&vport->fc_nodes_list_lock, iflags);
49274927
lpfc_nlp_state_cleanup(vport, ndlp, ndlp->nlp_state,
49284928
NLP_STE_UNUSED_NODE);
49294929
}
@@ -5421,8 +5421,8 @@ lpfc_unreg_hba_rpis(struct lpfc_hba *phba)
54215421
{
54225422
struct lpfc_vport **vports;
54235423
struct lpfc_nodelist *ndlp;
5424-
struct Scsi_Host *shost;
54255424
int i;
5425+
unsigned long iflags;
54265426

54275427
vports = lpfc_create_vport_work_array(phba);
54285428
if (!vports) {
@@ -5431,17 +5431,18 @@ lpfc_unreg_hba_rpis(struct lpfc_hba *phba)
54315431
return;
54325432
}
54335433
for (i = 0; i <= phba->max_vports && vports[i] != NULL; i++) {
5434-
shost = lpfc_shost_from_vport(vports[i]);
5435-
spin_lock_irq(shost->host_lock);
5434+
spin_lock_irqsave(&vports[i]->fc_nodes_list_lock, iflags);
54365435
list_for_each_entry(ndlp, &vports[i]->fc_nodes, nlp_listp) {
54375436
if (ndlp->nlp_flag & NLP_RPI_REGISTERED) {
54385437
/* The mempool_alloc might sleep */
5439-
spin_unlock_irq(shost->host_lock);
5438+
spin_unlock_irqrestore(&vports[i]->fc_nodes_list_lock,
5439+
iflags);
54405440
lpfc_unreg_rpi(vports[i], ndlp);
5441-
spin_lock_irq(shost->host_lock);
5441+
spin_lock_irqsave(&vports[i]->fc_nodes_list_lock,
5442+
iflags);
54425443
}
54435444
}
5444-
spin_unlock_irq(shost->host_lock);
5445+
spin_unlock_irqrestore(&vports[i]->fc_nodes_list_lock, iflags);
54455446
}
54465447
lpfc_destroy_vport_work_array(phba, vports);
54475448
}
@@ -5683,12 +5684,11 @@ lpfc_findnode_did(struct lpfc_vport *vport, uint32_t did)
56835684
struct lpfc_nodelist *
56845685
lpfc_findnode_mapped(struct lpfc_vport *vport)
56855686
{
5686-
struct Scsi_Host *shost = lpfc_shost_from_vport(vport);
56875687
struct lpfc_nodelist *ndlp;
56885688
uint32_t data1;
56895689
unsigned long iflags;
56905690

5691-
spin_lock_irqsave(shost->host_lock, iflags);
5691+
spin_lock_irqsave(&vport->fc_nodes_list_lock, iflags);
56925692

56935693
list_for_each_entry(ndlp, &vport->fc_nodes, nlp_listp) {
56945694
if (ndlp->nlp_state == NLP_STE_UNMAPPED_NODE ||
@@ -5697,7 +5697,8 @@ lpfc_findnode_mapped(struct lpfc_vport *vport)
56975697
((uint32_t)ndlp->nlp_xri << 16) |
56985698
((uint32_t)ndlp->nlp_type << 8) |
56995699
((uint32_t)ndlp->nlp_rpi & 0xff));
5700-
spin_unlock_irqrestore(shost->host_lock, iflags);
5700+
spin_unlock_irqrestore(&vport->fc_nodes_list_lock,
5701+
iflags);
57015702
lpfc_printf_vlog(vport, KERN_INFO, LOG_NODE_VERBOSE,
57025703
"2025 FIND node DID MAPPED "
57035704
"Data: x%px x%x x%x x%x x%px\n",
@@ -5707,7 +5708,7 @@ lpfc_findnode_mapped(struct lpfc_vport *vport)
57075708
return ndlp;
57085709
}
57095710
}
5710-
spin_unlock_irqrestore(shost->host_lock, iflags);
5711+
spin_unlock_irqrestore(&vport->fc_nodes_list_lock, iflags);
57115712

57125713
/* FIND node did <did> NOT FOUND */
57135714
lpfc_printf_vlog(vport, KERN_INFO, LOG_NODE,
@@ -6742,7 +6743,7 @@ lpfc_fcf_inuse(struct lpfc_hba *phba)
67426743
struct lpfc_vport **vports;
67436744
int i, ret = 0;
67446745
struct lpfc_nodelist *ndlp;
6745-
struct Scsi_Host *shost;
6746+
unsigned long iflags;
67466747

67476748
vports = lpfc_create_vport_work_array(phba);
67486749

@@ -6751,24 +6752,23 @@ lpfc_fcf_inuse(struct lpfc_hba *phba)
67516752
return 1;
67526753

67536754
for (i = 0; i <= phba->max_vports && vports[i] != NULL; i++) {
6754-
shost = lpfc_shost_from_vport(vports[i]);
6755-
spin_lock_irq(shost->host_lock);
67566755
/*
67576756
* IF the CVL_RCVD bit is not set then we have sent the
67586757
* flogi.
67596758
* If dev_loss fires while we are waiting we do not want to
67606759
* unreg the fcf.
67616760
*/
67626761
if (!(vports[i]->fc_flag & FC_VPORT_CVL_RCVD)) {
6763-
spin_unlock_irq(shost->host_lock);
67646762
ret = 1;
67656763
goto out;
67666764
}
6765+
spin_lock_irqsave(&vports[i]->fc_nodes_list_lock, iflags);
67676766
list_for_each_entry(ndlp, &vports[i]->fc_nodes, nlp_listp) {
67686767
if (ndlp->rport &&
67696768
(ndlp->rport->roles & FC_RPORT_ROLE_FCP_TARGET)) {
67706769
ret = 1;
6771-
spin_unlock_irq(shost->host_lock);
6770+
spin_unlock_irqrestore(&vports[i]->fc_nodes_list_lock,
6771+
iflags);
67726772
goto out;
67736773
} else if (ndlp->nlp_flag & NLP_RPI_REGISTERED) {
67746774
ret = 1;
@@ -6780,7 +6780,7 @@ lpfc_fcf_inuse(struct lpfc_hba *phba)
67806780
ndlp->nlp_flag);
67816781
}
67826782
}
6783-
spin_unlock_irq(shost->host_lock);
6783+
spin_unlock_irqrestore(&vports[i]->fc_nodes_list_lock, iflags);
67846784
}
67856785
out:
67866786
lpfc_destroy_vport_work_array(phba, vports);

0 commit comments

Comments
 (0)