Skip to content

Commit 72ea7fe

Browse files
tyreldmartinkpetersen
tyreld
authored and
martinkpetersen
committed
scsi: ibmvfc: Allocate/free queue resource only during probe/remove
Currently, the sub-queues and event pool resources are allocated/freed for every CRQ connection event such as reset and LPM. This exposes the driver to a couple issues. First the inefficiency of freeing and reallocating memory that can simply be resued after being sanitized. Further, a system under memory pressue runs the risk of allocation failures that could result in a crippled driver. Finally, there is a race window where command submission/compeletion can try to pull/return elements from/to an event pool that is being deleted or already has been deleted due to the lack of host state around freeing/allocating resources. The following is an example of list corruption following a live partition migration (LPM): Oops: Exception in kernel mode, sig: 5 [#1] LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=2048 NUMA pSeries Modules linked in: vfat fat isofs cdrom ext4 mbcache jbd2 nft_counter nft_compat nf_tables nfnetlink rpadlpar_io rpaphp xsk_diag nfsv3 nfs_acl nfs lockd grace fscache netfs rfkill bonding tls sunrpc pseries_rng drm drm_panel_orientation_quirks xfs libcrc32c dm_service_time sd_mod t10_pi sg ibmvfc scsi_transport_fc ibmveth vmx_crypto dm_multipath dm_mirror dm_region_hash dm_log dm_mod ipmi_devintf ipmi_msghandler fuse CPU: 0 PID: 2108 Comm: ibmvfc_0 Kdump: loaded Not tainted 5.14.0-70.9.1.el9_0.ppc64le #1 NIP: c0000000007c4bb0 LR: c0000000007c4bac CTR: 00000000005b9a10 REGS: c00000025c10b760 TRAP: 0700 Not tainted (5.14.0-70.9.1.el9_0.ppc64le) MSR: 800000000282b033 <SF,VEC,VSX,EE,FP,ME,IR,DR,RI,LE> CR: 2800028f XER: 0000000f CFAR: c0000000001f55bc IRQMASK: 0 GPR00: c0000000007c4bac c00000025c10ba00 c000000002a47c00 000000000000004e GPR04: c0000031e3006f88 c0000031e308bd00 c00000025c10b768 0000000000000027 GPR08: 0000000000000000 c0000031e3009dc0 00000031e0eb0000 0000000000000000 GPR12: c0000031e2ffffa8 c000000002dd0000 c000000000187108 c00000020fcee2c0 GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000 GPR20: 0000000000000000 0000000000000000 0000000000000000 c008000002f81300 GPR24: 5deadbeef0000100 5deadbeef0000122 c000000263ba6910 c00000024cc88000 GPR28: 000000000000003c c0000002430a0000 c0000002430ac300 000000000000c300 NIP [c0000000007c4bb0] __list_del_entry_valid+0x90/0x100 LR [c0000000007c4bac] __list_del_entry_valid+0x8c/0x100 Call Trace: [c00000025c10ba00] [c0000000007c4bac] __list_del_entry_valid+0x8c/0x100 (unreliable) [c00000025c10ba60] [c008000002f42284] ibmvfc_free_queue+0xec/0x210 [ibmvfc] [c00000025c10bb10] [c008000002f4246c] ibmvfc_deregister_scsi_channel+0xc4/0x160 [ibmvfc] [c00000025c10bba0] [c008000002f42580] ibmvfc_release_sub_crqs+0x78/0x130 [ibmvfc] [c00000025c10bc20] [c008000002f4f6cc] ibmvfc_do_work+0x5c4/0xc70 [ibmvfc] [c00000025c10bce0] [c008000002f4fdec] ibmvfc_work+0x74/0x1e8 [ibmvfc] [c00000025c10bda0] [c0000000001872b8] kthread+0x1b8/0x1c0 [c00000025c10be10] [c00000000000cd64] ret_from_kernel_thread+0x5c/0x64 Instruction dump: 40820034 38600001 38210060 4e800020 7c0802a6 7c641b78 3c62fe7a 7d254b78 3863b590 f8010070 4ba309cd 60000000 <0fe00000> 7c0802a6 3c62fe7a 3863b640 ---[ end trace 11a2b65a92f8b66c ]--- ibmvfc 30000003: Send warning. Receive queue closed, will retry. Add registration/deregistration helpers that are called instead during connection resets to sanitize and reconfigure the queues. Link: https://lore.kernel.org/r/20220616191126.1281259-3-tyreld@linux.ibm.com Fixes: 3034ebe ("scsi: ibmvfc: Add alloc/dealloc routines for SCSI Sub-CRQ Channels") Cc: stable@vger.kernel.org Reviewed-by: Brian King <brking@linux.vnet.ibm.com> Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com> Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
1 parent 1d3e098 commit 72ea7fe

File tree

1 file changed

+62
-17
lines changed

1 file changed

+62
-17
lines changed

‎drivers/scsi/ibmvscsi/ibmvfc.c

Lines changed: 62 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -160,8 +160,8 @@ static void ibmvfc_npiv_logout(struct ibmvfc_host *);
160160
static void ibmvfc_tgt_implicit_logout_and_del(struct ibmvfc_target *);
161161
static void ibmvfc_tgt_move_login(struct ibmvfc_target *);
162162

163-
static void ibmvfc_release_sub_crqs(struct ibmvfc_host *);
164-
static void ibmvfc_init_sub_crqs(struct ibmvfc_host *);
163+
static void ibmvfc_dereg_sub_crqs(struct ibmvfc_host *);
164+
static void ibmvfc_reg_sub_crqs(struct ibmvfc_host *);
165165

166166
static const char *unknown_error = "unknown error";
167167

@@ -917,7 +917,7 @@ static int ibmvfc_reenable_crq_queue(struct ibmvfc_host *vhost)
917917
struct vio_dev *vdev = to_vio_dev(vhost->dev);
918918
unsigned long flags;
919919

920-
ibmvfc_release_sub_crqs(vhost);
920+
ibmvfc_dereg_sub_crqs(vhost);
921921

922922
/* Re-enable the CRQ */
923923
do {
@@ -936,7 +936,7 @@ static int ibmvfc_reenable_crq_queue(struct ibmvfc_host *vhost)
936936
spin_unlock(vhost->crq.q_lock);
937937
spin_unlock_irqrestore(vhost->host->host_lock, flags);
938938

939-
ibmvfc_init_sub_crqs(vhost);
939+
ibmvfc_reg_sub_crqs(vhost);
940940

941941
return rc;
942942
}
@@ -955,7 +955,7 @@ static int ibmvfc_reset_crq(struct ibmvfc_host *vhost)
955955
struct vio_dev *vdev = to_vio_dev(vhost->dev);
956956
struct ibmvfc_queue *crq = &vhost->crq;
957957

958-
ibmvfc_release_sub_crqs(vhost);
958+
ibmvfc_dereg_sub_crqs(vhost);
959959

960960
/* Close the CRQ */
961961
do {
@@ -988,7 +988,7 @@ static int ibmvfc_reset_crq(struct ibmvfc_host *vhost)
988988
spin_unlock(vhost->crq.q_lock);
989989
spin_unlock_irqrestore(vhost->host->host_lock, flags);
990990

991-
ibmvfc_init_sub_crqs(vhost);
991+
ibmvfc_reg_sub_crqs(vhost);
992992

993993
return rc;
994994
}
@@ -5757,9 +5757,6 @@ static int ibmvfc_register_scsi_channel(struct ibmvfc_host *vhost,
57575757

57585758
ENTER;
57595759

5760-
if (ibmvfc_alloc_queue(vhost, scrq, IBMVFC_SUB_CRQ_FMT))
5761-
return -ENOMEM;
5762-
57635760
rc = h_reg_sub_crq(vdev->unit_address, scrq->msg_token, PAGE_SIZE,
57645761
&scrq->cookie, &scrq->hw_irq);
57655762

@@ -5800,7 +5797,6 @@ static int ibmvfc_register_scsi_channel(struct ibmvfc_host *vhost,
58005797
rc = plpar_hcall_norets(H_FREE_SUB_CRQ, vdev->unit_address, scrq->cookie);
58015798
} while (rtas_busy_delay(rc));
58025799
reg_failed:
5803-
ibmvfc_free_queue(vhost, scrq);
58045800
LEAVE;
58055801
return rc;
58065802
}
@@ -5826,12 +5822,50 @@ static void ibmvfc_deregister_scsi_channel(struct ibmvfc_host *vhost, int index)
58265822
if (rc)
58275823
dev_err(dev, "Failed to free sub-crq[%d]: rc=%ld\n", index, rc);
58285824

5829-
ibmvfc_free_queue(vhost, scrq);
5825+
/* Clean out the queue */
5826+
memset(scrq->msgs.crq, 0, PAGE_SIZE);
5827+
scrq->cur = 0;
5828+
5829+
LEAVE;
5830+
}
5831+
5832+
static void ibmvfc_reg_sub_crqs(struct ibmvfc_host *vhost)
5833+
{
5834+
int i, j;
5835+
5836+
ENTER;
5837+
if (!vhost->mq_enabled || !vhost->scsi_scrqs.scrqs)
5838+
return;
5839+
5840+
for (i = 0; i < nr_scsi_hw_queues; i++) {
5841+
if (ibmvfc_register_scsi_channel(vhost, i)) {
5842+
for (j = i; j > 0; j--)
5843+
ibmvfc_deregister_scsi_channel(vhost, j - 1);
5844+
vhost->do_enquiry = 0;
5845+
return;
5846+
}
5847+
}
5848+
5849+
LEAVE;
5850+
}
5851+
5852+
static void ibmvfc_dereg_sub_crqs(struct ibmvfc_host *vhost)
5853+
{
5854+
int i;
5855+
5856+
ENTER;
5857+
if (!vhost->mq_enabled || !vhost->scsi_scrqs.scrqs)
5858+
return;
5859+
5860+
for (i = 0; i < nr_scsi_hw_queues; i++)
5861+
ibmvfc_deregister_scsi_channel(vhost, i);
5862+
58305863
LEAVE;
58315864
}
58325865

58335866
static void ibmvfc_init_sub_crqs(struct ibmvfc_host *vhost)
58345867
{
5868+
struct ibmvfc_queue *scrq;
58355869
int i, j;
58365870

58375871
ENTER;
@@ -5847,30 +5881,41 @@ static void ibmvfc_init_sub_crqs(struct ibmvfc_host *vhost)
58475881
}
58485882

58495883
for (i = 0; i < nr_scsi_hw_queues; i++) {
5850-
if (ibmvfc_register_scsi_channel(vhost, i)) {
5851-
for (j = i; j > 0; j--)
5852-
ibmvfc_deregister_scsi_channel(vhost, j - 1);
5884+
scrq = &vhost->scsi_scrqs.scrqs[i];
5885+
if (ibmvfc_alloc_queue(vhost, scrq, IBMVFC_SUB_CRQ_FMT)) {
5886+
for (j = i; j > 0; j--) {
5887+
scrq = &vhost->scsi_scrqs.scrqs[j - 1];
5888+
ibmvfc_free_queue(vhost, scrq);
5889+
}
58535890
kfree(vhost->scsi_scrqs.scrqs);
58545891
vhost->scsi_scrqs.scrqs = NULL;
58555892
vhost->scsi_scrqs.active_queues = 0;
58565893
vhost->do_enquiry = 0;
5857-
break;
5894+
vhost->mq_enabled = 0;
5895+
return;
58585896
}
58595897
}
58605898

5899+
ibmvfc_reg_sub_crqs(vhost);
5900+
58615901
LEAVE;
58625902
}
58635903

58645904
static void ibmvfc_release_sub_crqs(struct ibmvfc_host *vhost)
58655905
{
5906+
struct ibmvfc_queue *scrq;
58665907
int i;
58675908

58685909
ENTER;
58695910
if (!vhost->scsi_scrqs.scrqs)
58705911
return;
58715912

5872-
for (i = 0; i < nr_scsi_hw_queues; i++)
5873-
ibmvfc_deregister_scsi_channel(vhost, i);
5913+
ibmvfc_dereg_sub_crqs(vhost);
5914+
5915+
for (i = 0; i < nr_scsi_hw_queues; i++) {
5916+
scrq = &vhost->scsi_scrqs.scrqs[i];
5917+
ibmvfc_free_queue(vhost, scrq);
5918+
}
58745919

58755920
kfree(vhost->scsi_scrqs.scrqs);
58765921
vhost->scsi_scrqs.scrqs = NULL;

0 commit comments

Comments
 (0)