| Message ID | 1461571293-953-1-git-send-email-hare@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, 2016-04-25 at 10:01 +0200, Hannes Reinecke wrote: > We cannot use an embedded mutex in a structure with reference > counting, as mutex unlock might be delayed, and the waiters > might then access an already freed memory area. > So convert it to a spinlock. > > For details cf https://lkml.org/lkml/2015/2/11/245 > > Signed-off-by: Hannes Reinecke <hare@suse.de> There are comments at the beginning of fc_rport.c in the section RPORT LOCKING that still refer to the rport mutex. (I assume this is really the fc_rport_priv mutex since fc_rport does not have a mutex.) These comments should be updated if the locking is changed. Also see the comment about the mutex being held near fc_rport_enter_delete(). We may have a reproducible test case for our FCoE issues, I will see if we can tell if this fixes our problem. -Ewan > --- > drivers/scsi/libfc/fc_rport.c | 90 +++++++++++++++++++++---------------------- > include/scsi/libfc.h | 4 +- > 2 files changed, 47 insertions(+), 47 deletions(-) > > diff --git a/drivers/scsi/libfc/fc_rport.c b/drivers/scsi/libfc/fc_rport.c > index 589ff9a..4520b5a 100644 > --- a/drivers/scsi/libfc/fc_rport.c > +++ b/drivers/scsi/libfc/fc_rport.c > @@ -136,7 +136,7 @@ static struct fc_rport_priv *fc_rport_create(struct fc_lport *lport, > rdata->ids.roles = FC_RPORT_ROLE_UNKNOWN; > > kref_init(&rdata->kref); > - mutex_init(&rdata->rp_mutex); > + spin_lock_init(&rdata->rp_lock); > rdata->local_port = lport; > rdata->rp_state = RPORT_ST_INIT; > rdata->event = RPORT_EV_NONE; > @@ -251,7 +251,7 @@ static void fc_rport_work(struct work_struct *work) > struct fc4_prov *prov; > u8 type; > > - mutex_lock(&rdata->rp_mutex); > + spin_lock(&rdata->rp_lock); > event = rdata->event; > rport_ops = rdata->ops; > rport = rdata->rport; > @@ -264,7 +264,7 @@ static void fc_rport_work(struct work_struct *work) > rdata->event = RPORT_EV_NONE; > rdata->major_retries = 0; > kref_get(&rdata->kref); > - mutex_unlock(&rdata->rp_mutex); > + spin_unlock(&rdata->rp_lock); > > if (!rport) > rport = fc_remote_port_add(lport->host, 0, &ids); > @@ -274,7 +274,7 @@ static void fc_rport_work(struct work_struct *work) > kref_put(&rdata->kref, lport->tt.rport_destroy); > return; > } > - mutex_lock(&rdata->rp_mutex); > + spin_lock(&rdata->rp_lock); > if (rdata->rport) > FC_RPORT_DBG(rdata, "rport already allocated\n"); > rdata->rport = rport; > @@ -287,7 +287,7 @@ static void fc_rport_work(struct work_struct *work) > rpriv->flags = rdata->flags; > rpriv->e_d_tov = rdata->e_d_tov; > rpriv->r_a_tov = rdata->r_a_tov; > - mutex_unlock(&rdata->rp_mutex); > + spin_unlock(&rdata->rp_lock); > > if (rport_ops && rport_ops->event_callback) { > FC_RPORT_DBG(rdata, "callback ev %d\n", event); > @@ -313,7 +313,7 @@ static void fc_rport_work(struct work_struct *work) > mutex_unlock(&fc_prov_mutex); > } > port_id = rdata->ids.port_id; > - mutex_unlock(&rdata->rp_mutex); > + spin_unlock(&rdata->rp_lock); > > if (rport_ops && rport_ops->event_callback) { > FC_RPORT_DBG(rdata, "callback ev %d\n", event); > @@ -334,18 +334,18 @@ static void fc_rport_work(struct work_struct *work) > if (rport) { > rpriv = rport->dd_data; > rpriv->rp_state = RPORT_ST_DELETE; > - mutex_lock(&rdata->rp_mutex); > + spin_lock(&rdata->rp_lock); > rdata->rport = NULL; > - mutex_unlock(&rdata->rp_mutex); > + spin_unlock(&rdata->rp_lock); > fc_remote_port_delete(rport); > } > > mutex_lock(&lport->disc.disc_mutex); > - mutex_lock(&rdata->rp_mutex); > + spin_lock(&rdata->rp_lock); > if (rdata->rp_state == RPORT_ST_DELETE) { > if (port_id == FC_FID_DIR_SERV) { > rdata->event = RPORT_EV_NONE; > - mutex_unlock(&rdata->rp_mutex); > + spin_unlock(&rdata->rp_lock); > kref_put(&rdata->kref, lport->tt.rport_destroy); > } else if ((rdata->flags & FC_RP_STARTED) && > rdata->major_retries < > @@ -354,11 +354,11 @@ static void fc_rport_work(struct work_struct *work) > rdata->event = RPORT_EV_NONE; > FC_RPORT_DBG(rdata, "work restart\n"); > fc_rport_enter_flogi(rdata); > - mutex_unlock(&rdata->rp_mutex); > + spin_unlock(&rdata->rp_lock); > } else { > FC_RPORT_DBG(rdata, "work delete\n"); > list_del_rcu(&rdata->peers); > - mutex_unlock(&rdata->rp_mutex); > + spin_unlock(&rdata->rp_lock); > kref_put(&rdata->kref, lport->tt.rport_destroy); > } > } else { > @@ -368,13 +368,13 @@ static void fc_rport_work(struct work_struct *work) > rdata->event = RPORT_EV_NONE; > if (rdata->rp_state == RPORT_ST_READY) > fc_rport_enter_ready(rdata); > - mutex_unlock(&rdata->rp_mutex); > + spin_unlock(&rdata->rp_lock); > } > mutex_unlock(&lport->disc.disc_mutex); > break; > > default: > - mutex_unlock(&rdata->rp_mutex); > + spin_unlock(&rdata->rp_lock); > break; > } > } > @@ -393,7 +393,7 @@ static void fc_rport_work(struct work_struct *work) > */ > static int fc_rport_login(struct fc_rport_priv *rdata) > { > - mutex_lock(&rdata->rp_mutex); > + spin_lock(&rdata->rp_lock); > > rdata->flags |= FC_RP_STARTED; > switch (rdata->rp_state) { > @@ -409,7 +409,7 @@ static int fc_rport_login(struct fc_rport_priv *rdata) > fc_rport_enter_flogi(rdata); > break; > } > - mutex_unlock(&rdata->rp_mutex); > + spin_unlock(&rdata->rp_lock); > > return 0; > } > @@ -453,7 +453,7 @@ static void fc_rport_enter_delete(struct fc_rport_priv *rdata, > */ > static int fc_rport_logoff(struct fc_rport_priv *rdata) > { > - mutex_lock(&rdata->rp_mutex); > + spin_lock(&rdata->rp_lock); > > FC_RPORT_DBG(rdata, "Remove port\n"); > > @@ -470,7 +470,7 @@ static int fc_rport_logoff(struct fc_rport_priv *rdata) > */ > fc_rport_enter_delete(rdata, RPORT_EV_STOP); > out: > - mutex_unlock(&rdata->rp_mutex); > + spin_unlock(&rdata->rp_lock); > return 0; > } > > @@ -505,7 +505,7 @@ static void fc_rport_timeout(struct work_struct *work) > struct fc_rport_priv *rdata = > container_of(work, struct fc_rport_priv, retry_work.work); > > - mutex_lock(&rdata->rp_mutex); > + spin_lock(&rdata->rp_lock); > > switch (rdata->rp_state) { > case RPORT_ST_FLOGI: > @@ -530,7 +530,7 @@ static void fc_rport_timeout(struct work_struct *work) > break; > } > > - mutex_unlock(&rdata->rp_mutex); > + spin_unlock(&rdata->rp_lock); > } > > /** > @@ -666,7 +666,7 @@ static void fc_rport_flogi_resp(struct fc_seq *sp, struct fc_frame *fp, > if (fp == ERR_PTR(-FC_EX_CLOSED)) > goto put; > > - mutex_lock(&rdata->rp_mutex); > + spin_lock(&rdata->rp_lock); > > if (rdata->rp_state != RPORT_ST_FLOGI) { > FC_RPORT_DBG(rdata, "Received a FLOGI response, but in state " > @@ -700,7 +700,7 @@ static void fc_rport_flogi_resp(struct fc_seq *sp, struct fc_frame *fp, > out: > fc_frame_free(fp); > err: > - mutex_unlock(&rdata->rp_mutex); > + spin_unlock(&rdata->rp_lock); > put: > kref_put(&rdata->kref, rdata->local_port->tt.rport_destroy); > return; > @@ -783,7 +783,7 @@ static void fc_rport_recv_flogi_req(struct fc_lport *lport, > rjt_data.explan = ELS_EXPL_NOT_NEIGHBOR; > goto reject; > } > - mutex_lock(&rdata->rp_mutex); > + spin_lock(&rdata->rp_lock); > > FC_RPORT_DBG(rdata, "Received FLOGI in %s state\n", > fc_rport_state(rdata)); > @@ -805,7 +805,7 @@ static void fc_rport_recv_flogi_req(struct fc_lport *lport, > if (lport->point_to_multipoint) > break; > case RPORT_ST_DELETE: > - mutex_unlock(&rdata->rp_mutex); > + spin_unlock(&rdata->rp_lock); > rjt_data.reason = ELS_RJT_FIP; > rjt_data.explan = ELS_EXPL_NOT_NEIGHBOR; > goto reject; > @@ -822,13 +822,13 @@ static void fc_rport_recv_flogi_req(struct fc_lport *lport, > * This queues work to be sure exchanges are reset. > */ > fc_rport_enter_delete(rdata, RPORT_EV_LOGO); > - mutex_unlock(&rdata->rp_mutex); > + spin_unlock(&rdata->rp_lock); > rjt_data.reason = ELS_RJT_BUSY; > rjt_data.explan = ELS_EXPL_NONE; > goto reject; > } > if (fc_rport_login_complete(rdata, fp)) { > - mutex_unlock(&rdata->rp_mutex); > + spin_unlock(&rdata->rp_lock); > rjt_data.reason = ELS_RJT_LOGIC; > rjt_data.explan = ELS_EXPL_NONE; > goto reject; > @@ -850,7 +850,7 @@ static void fc_rport_recv_flogi_req(struct fc_lport *lport, > else > fc_rport_state_enter(rdata, RPORT_ST_PLOGI_WAIT); > out: > - mutex_unlock(&rdata->rp_mutex); > + spin_unlock(&rdata->rp_lock); > mutex_unlock(&disc->disc_mutex); > fc_frame_free(rx_fp); > return; > @@ -881,7 +881,7 @@ static void fc_rport_plogi_resp(struct fc_seq *sp, struct fc_frame *fp, > u16 cssp_seq; > u8 op; > > - mutex_lock(&rdata->rp_mutex); > + spin_lock(&rdata->rp_lock); > > FC_RPORT_DBG(rdata, "Received a PLOGI %s\n", fc_els_resp_type(fp)); > > @@ -922,7 +922,7 @@ static void fc_rport_plogi_resp(struct fc_seq *sp, struct fc_frame *fp, > out: > fc_frame_free(fp); > err: > - mutex_unlock(&rdata->rp_mutex); > + spin_unlock(&rdata->rp_lock); > kref_put(&rdata->kref, rdata->local_port->tt.rport_destroy); > } > > @@ -1005,7 +1005,7 @@ static void fc_rport_prli_resp(struct fc_seq *sp, struct fc_frame *fp, > u8 op; > u8 resp_code = 0; > > - mutex_lock(&rdata->rp_mutex); > + spin_lock(&rdata->rp_lock); > > FC_RPORT_DBG(rdata, "Received a PRLI %s\n", fc_els_resp_type(fp)); > > @@ -1075,7 +1075,7 @@ static void fc_rport_prli_resp(struct fc_seq *sp, struct fc_frame *fp, > out: > fc_frame_free(fp); > err: > - mutex_unlock(&rdata->rp_mutex); > + spin_unlock(&rdata->rp_lock); > kref_put(&rdata->kref, rdata->local_port->tt.rport_destroy); > } > > @@ -1153,7 +1153,7 @@ static void fc_rport_rtv_resp(struct fc_seq *sp, struct fc_frame *fp, > struct fc_rport_priv *rdata = rdata_arg; > u8 op; > > - mutex_lock(&rdata->rp_mutex); > + spin_lock(&rdata->rp_lock); > > FC_RPORT_DBG(rdata, "Received a RTV %s\n", fc_els_resp_type(fp)); > > @@ -1197,7 +1197,7 @@ static void fc_rport_rtv_resp(struct fc_seq *sp, struct fc_frame *fp, > out: > fc_frame_free(fp); > err: > - mutex_unlock(&rdata->rp_mutex); > + spin_unlock(&rdata->rp_lock); > kref_put(&rdata->kref, rdata->local_port->tt.rport_destroy); > } > > @@ -1289,7 +1289,7 @@ static void fc_rport_adisc_resp(struct fc_seq *sp, struct fc_frame *fp, > struct fc_els_adisc *adisc; > u8 op; > > - mutex_lock(&rdata->rp_mutex); > + spin_lock(&rdata->rp_lock); > > FC_RPORT_DBG(rdata, "Received a ADISC response\n"); > > @@ -1326,7 +1326,7 @@ static void fc_rport_adisc_resp(struct fc_seq *sp, struct fc_frame *fp, > out: > fc_frame_free(fp); > err: > - mutex_unlock(&rdata->rp_mutex); > + spin_unlock(&rdata->rp_lock); > kref_put(&rdata->kref, rdata->local_port->tt.rport_destroy); > } > > @@ -1483,7 +1483,7 @@ static void fc_rport_recv_els_req(struct fc_lport *lport, struct fc_frame *fp) > mutex_unlock(&lport->disc.disc_mutex); > goto reject; > } > - mutex_lock(&rdata->rp_mutex); > + spin_lock(&rdata->rp_lock); > mutex_unlock(&lport->disc.disc_mutex); > > switch (rdata->rp_state) { > @@ -1493,7 +1493,7 @@ static void fc_rport_recv_els_req(struct fc_lport *lport, struct fc_frame *fp) > case RPORT_ST_ADISC: > break; > default: > - mutex_unlock(&rdata->rp_mutex); > + spin_unlock(&rdata->rp_lock); > goto reject; > } > > @@ -1523,7 +1523,7 @@ static void fc_rport_recv_els_req(struct fc_lport *lport, struct fc_frame *fp) > break; > } > > - mutex_unlock(&rdata->rp_mutex); > + spin_unlock(&rdata->rp_lock); > return; > > reject: > @@ -1616,7 +1616,7 @@ static void fc_rport_recv_plogi_req(struct fc_lport *lport, > goto reject; > } > > - mutex_lock(&rdata->rp_mutex); > + spin_lock(&rdata->rp_lock); > mutex_unlock(&disc->disc_mutex); > > rdata->ids.port_name = get_unaligned_be64(&pl->fl_wwpn); > @@ -1643,7 +1643,7 @@ static void fc_rport_recv_plogi_req(struct fc_lport *lport, > case RPORT_ST_PLOGI: > FC_RPORT_DBG(rdata, "Received PLOGI in PLOGI state\n"); > if (rdata->ids.port_name < lport->wwpn) { > - mutex_unlock(&rdata->rp_mutex); > + spin_unlock(&rdata->rp_lock); > rjt_data.reason = ELS_RJT_INPROG; > rjt_data.explan = ELS_EXPL_NONE; > goto reject; > @@ -1661,14 +1661,14 @@ static void fc_rport_recv_plogi_req(struct fc_lport *lport, > case RPORT_ST_DELETE: > FC_RPORT_DBG(rdata, "Received PLOGI in state %s - send busy\n", > fc_rport_state(rdata)); > - mutex_unlock(&rdata->rp_mutex); > + spin_unlock(&rdata->rp_lock); > rjt_data.reason = ELS_RJT_BUSY; > rjt_data.explan = ELS_EXPL_NONE; > goto reject; > } > if (!fc_rport_compatible_roles(lport, rdata)) { > FC_RPORT_DBG(rdata, "Received PLOGI for incompatible role\n"); > - mutex_unlock(&rdata->rp_mutex); > + spin_unlock(&rdata->rp_lock); > rjt_data.reason = ELS_RJT_LOGIC; > rjt_data.explan = ELS_EXPL_NONE; > goto reject; > @@ -1691,7 +1691,7 @@ static void fc_rport_recv_plogi_req(struct fc_lport *lport, > lport->tt.frame_send(lport, fp); > fc_rport_enter_prli(rdata); > out: > - mutex_unlock(&rdata->rp_mutex); > + spin_unlock(&rdata->rp_lock); > fc_frame_free(rx_fp); > return; > > @@ -1910,12 +1910,12 @@ static void fc_rport_recv_logo_req(struct fc_lport *lport, struct fc_frame *fp) > mutex_lock(&lport->disc.disc_mutex); > rdata = lport->tt.rport_lookup(lport, sid); > if (rdata) { > - mutex_lock(&rdata->rp_mutex); > + spin_lock(&rdata->rp_lock); > FC_RPORT_DBG(rdata, "Received LOGO request while in state %s\n", > fc_rport_state(rdata)); > > fc_rport_enter_delete(rdata, RPORT_EV_LOGO); > - mutex_unlock(&rdata->rp_mutex); > + spin_unlock(&rdata->rp_lock); > } else > FC_RPORT_ID_DBG(lport, sid, > "Received LOGO from non-logged-in port\n"); > diff --git a/include/scsi/libfc.h b/include/scsi/libfc.h > index 93d14da..7d63d23 100644 > --- a/include/scsi/libfc.h > +++ b/include/scsi/libfc.h > @@ -187,7 +187,7 @@ struct fc_rport_libfc_priv { > * @major_retries: The retry count for the entire PLOGI/PRLI state machine > * @e_d_tov: Error detect timeout value (in msec) > * @r_a_tov: Resource allocation timeout value (in msec) > - * @rp_mutex: The mutex that protects the remote port > + * @rp_lock: The lock that protects the remote port > * @retry_work: Handle for retries > * @event_callback: Callback when READY, FAILED or LOGO states complete > * @prli_count: Count of open PRLI sessions in providers > @@ -207,7 +207,7 @@ struct fc_rport_priv { > unsigned int major_retries; > unsigned int e_d_tov; > unsigned int r_a_tov; > - struct mutex rp_mutex; > + spinlock_t rp_lock; > struct delayed_work retry_work; > enum fc_rport_event event; > struct fc_rport_operations *ops; -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Apr 25, 2016 at 10:01:33AM +0200, Hannes Reinecke wrote: > We cannot use an embedded mutex in a structure with reference > counting, as mutex unlock might be delayed, and the waiters > might then access an already freed memory area. > So convert it to a spinlock. > > For details cf https://lkml.org/lkml/2015/2/11/245 > > Signed-off-by: Hannes Reinecke <hare@suse.de> Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
On 04/25/2016 01:01 AM, Hannes Reinecke wrote: > We cannot use an embedded mutex in a structure with reference > counting, as mutex unlock might be delayed, and the waiters > might then access an already freed memory area. > So convert it to a spinlock. > > For details cf https://lkml.org/lkml/2015/2/11/245 Hello Hannes, Is what you describe a theoretical concern or have you observed any issues that could have been caused by the rport mutex? I'm asking this because my interpretation of the thread you refer to is different. My conclusion is that it is safe to embed a mutex in a structure that uses reference counting but that the mutex_unlock() call may trigger a spurious wakeup. I think that the conclusion of that thread was that glibc and kernel code should tolerate such spurious wakeups. Bart. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
>>>>> "Bart" == Bart Van Assche <bart.vanassche@sandisk.com> writes:
Bart> Is what you describe a theoretical concern or have you observed
Bart> any issues that could have been caused by the rport mutex?
I believe Ewan had some data. Ewan?
On 05/10/2016 08:33 PM, Bart Van Assche wrote: > On 04/25/2016 01:01 AM, Hannes Reinecke wrote: >> We cannot use an embedded mutex in a structure with reference >> counting, as mutex unlock might be delayed, and the waiters >> might then access an already freed memory area. >> So convert it to a spinlock. >> >> For details cf https://lkml.org/lkml/2015/2/11/245 > > Hello Hannes, > > Is what you describe a theoretical concern or have you observed any > issues that could have been caused by the rport mutex? I'm asking > this because my interpretation of the thread you refer to is > different. My conclusion is that it is safe to embed a mutex in a > structure that uses reference counting but that the mutex_unlock() > call may trigger a spurious wakeup. I think that the conclusion of > that thread was that glibc and kernel code should tolerate such > spurious wakeups. > We have several bugzillas referring to that specific code. Most notably triggered when removing target ports with and open-fcoe HBA. And this patch seems to resolve it. Read: with this patch the issue doesn't occur anymore. Cheers, Hannes
On 05/11/2016 07:49 AM, Hannes Reinecke wrote: > On 05/10/2016 08:33 PM, Bart Van Assche wrote: >> On 04/25/2016 01:01 AM, Hannes Reinecke wrote: >>> We cannot use an embedded mutex in a structure with reference >>> counting, as mutex unlock might be delayed, and the waiters >>> might then access an already freed memory area. >>> So convert it to a spinlock. >>> >>> For details cf https://lkml.org/lkml/2015/2/11/245 >> >> Hello Hannes, >> >> Is what you describe a theoretical concern or have you observed any >> issues that could have been caused by the rport mutex? I'm asking >> this because my interpretation of the thread you refer to is >> different. My conclusion is that it is safe to embed a mutex in a >> structure that uses reference counting but that the mutex_unlock() >> call may trigger a spurious wakeup. I think that the conclusion of >> that thread was that glibc and kernel code should tolerate such >> spurious wakeups. >> > We have several bugzillas referring to that specific code. > Most notably triggered when removing target ports with and open-fcoe > HBA. > > And this patch seems to resolve it. > Read: with this patch the issue doesn't occur anymore. > And for the unbelievers here's the crash: general protection fault: 0000 [#1] SMP Modules linked in: raw rpcsec_gss_krb5 auth_rpcgss nfsv4 dns_reso lver nfs lockd sunrpc fscache iscsi_ibft iscsi_boot_sysfs af_packet xfs ext4 libcrc32c crc16 mbcache jbd2 joydev coretemp kvm_intel kvm crct10dif_pclmul crc32_pclmul iTCO_wdt iTCO_vendor_support aesni_intel aes_x86_64 lrw gf128mul glue_helper ablk_helper cryptd pcspkr lpc_ich mfd_core enic ipmi_si ipmi_msghandler wmi shpchp acpi_power_meter processor button ac hid_generic usbhid btrfs xor raid6_pq mgag200 syscopyarea ehci_pci sysfillrect sysimgblt i2c_algo_bit ehci_hcd drm_kms_helper ttm usbcore drm crc32c_intel usb_common megaraid_sas dm_service_time sd_mod scsi_dh_rdac scsi_dh_emc scsi_dh_alua fnic(OEX) libfcoe libfc scsi_transpor t_fc scsi_tgt dm_multipath scsi_dh sg dm_mod scsi_mod autofs4 Supported: Yes, External CPU: 0 PID: 16868 Comm: kworker/u25:2 Tainted: G W OE X 3 .12.55-52.42.1.10435.0.PTF.962846-default #1 Workqueue: fnic_event_wq fnic_handle_frame [fnic] task: ffff88080021e140 ti: ffff8807cf076000 task.ti: ffff8807cf076000 RIP: 0010:[<ffffffffa00b2adb>] [<ffffffffa00b2adb>] fc_rport_lookup+0x4b/0x70 [libfc] RSP: 0018:ffff8807cf077d20 EFLAGS: 00010202 RAX: 8500a090df03ff00 RBX: ffff8808560386f0 RCX: ffff880846351400 RDX: 8500a090df040000 RSI: 0000000000610c00 RDI: ffff880856038738 RBP: ffff8808560386f0 R08: 0000000000000001 R09: 0000000000000000 R10: ffff8808402f8e40 R11: ffff880856127600 R12: ffff8807cf077d78 R13: 0000000000610c00 R14: ffff880846351400 R15: 0000000000000000 FS: 0000000000000000(0000) GS:ffff88087fc00000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 00007ff6a540f000 CR3: 0000000846831000 CR4: 00000000001407f0 Stack: 8500a090df040000 ffffffffa00b2e17 ffff8808042fe0c0 ffff8808560386f0 ffff8807cf077d78 ffff880856038750 ffffffffa00a9f81 ffff880800007530 ffff8808402f8e40 ffff880856127600 0000000000000001 ffff880846351408 Call Trace: [<ffffffffa00b2e17>] fc_rport_create+0x17/0x1b0 [libfc] [<ffffffffa00a9f81>] fc_disc_recv_req+0x261/0x480 [libfc] [<ffffffffa00b1008>] fc_lport_recv_els_req+0x68/0x130 [libfc] [<ffffffffa00afd5a>] fc_lport_recv_req+0x9a/0xf0 [libfc] [<ffffffffa00e8333>] fnic_handle_frame+0x63/0xd0 [fnic] [<ffffffff8106fd52>] process_one_work+0x172/0x420 [<ffffffff810709ca>] worker_thread+0x11a/0x3c0 [<ffffffff81077344>] kthread+0xb4/0xc0 [<ffffffff81521318>] ret_from_fork+0x58/0x90 Code: ff ff ff 75 26 eb 39 66 0f 1f 84 00 00 00 00 00 48 8b 80 00 01 00 00 48 89 04 24 48 8b 14 24 48 39 d7 48 8d 82 00 ff ff ff 74 15 <39> b2 28 ff ff ff 75 dd 48 83 c4 08 c3 0f 1f 84 00 00 00 00 00 RIP [<ffffffffa00b2adb>] fc_rport_lookup+0x4b/0x70 [libfc] So no, it's not theoretical. Cheers, Hannes
On 05/10/16 23:07, Hannes Reinecke wrote: > On 05/11/2016 07:49 AM, Hannes Reinecke wrote: > RIP: 0010:[<ffffffffa00b2adb>] [<ffffffffa00b2adb>] > fc_rport_lookup+0x4b/0x70 [libfc] > Call Trace: > [<ffffffffa00b2e17>] fc_rport_create+0x17/0x1b0 [libfc] > [<ffffffffa00a9f81>] fc_disc_recv_req+0x261/0x480 [libfc] > [<ffffffffa00b1008>] fc_lport_recv_els_req+0x68/0x130 [libfc] > [<ffffffffa00afd5a>] fc_lport_recv_req+0x9a/0xf0 [libfc] > [<ffffffffa00e8333>] fnic_handle_frame+0x63/0xd0 [fnic] > [<ffffffff8106fd52>] process_one_work+0x172/0x420 > [<ffffffff810709ca>] worker_thread+0x11a/0x3c0 > [<ffffffff81077344>] kthread+0xb4/0xc0 > [<ffffffff81521318>] ret_from_fork+0x58/0x90 Hello Hannes, Thanks for sharing this information. fc_disc_recv_req() protects the fc_rport_create() call via a mutex (disc_mutex). Since a mutex_lock() call may sleep it can trigger the start of an RCU grace period. I think this may result in freeing of an rport while fc_rport_lookup() is examining it. Have you already considered to add a rcu_read_lock()/rcu_read_unlock() pair in fc_rport_lookup()? Thanks, Bart. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 05/11/2016 04:44 PM, Bart Van Assche wrote: > On 05/10/16 23:07, Hannes Reinecke wrote: >> On 05/11/2016 07:49 AM, Hannes Reinecke wrote: >> RIP: 0010:[<ffffffffa00b2adb>] [<ffffffffa00b2adb>] >> fc_rport_lookup+0x4b/0x70 [libfc] >> Call Trace: >> [<ffffffffa00b2e17>] fc_rport_create+0x17/0x1b0 [libfc] >> [<ffffffffa00a9f81>] fc_disc_recv_req+0x261/0x480 [libfc] >> [<ffffffffa00b1008>] fc_lport_recv_els_req+0x68/0x130 [libfc] >> [<ffffffffa00afd5a>] fc_lport_recv_req+0x9a/0xf0 [libfc] >> [<ffffffffa00e8333>] fnic_handle_frame+0x63/0xd0 [fnic] >> [<ffffffff8106fd52>] process_one_work+0x172/0x420 >> [<ffffffff810709ca>] worker_thread+0x11a/0x3c0 >> [<ffffffff81077344>] kthread+0xb4/0xc0 >> [<ffffffff81521318>] ret_from_fork+0x58/0x90 > > Hello Hannes, > > Thanks for sharing this information. fc_disc_recv_req() protects the > fc_rport_create() call via a mutex (disc_mutex). Since a mutex_lock() > call may sleep it can trigger the start of an RCU grace period. I think > this may result in freeing of an rport while fc_rport_lookup() is > examining it. Have you already considered to add a > rcu_read_lock()/rcu_read_unlock() pair in fc_rport_lookup()? > No, I haven't so far. This issue is hard to trigger, and I've only got the customer report to go by. Also, when using an rcu_read_lock() here one probably needs to revisit the entire locking structure in libfc: rport list is an RCU-proctected list, so in principle one only needs to hold the rport mutex when _assigning_ new rports, and _could_ drop the mutex usage for a simple lookup. But this really needs some more thought and testing, so I haven't attempted it. Also, with your suggestion I would need to take the mutex_lock _and_ call rcu_read_lock(), which just looks wrong. Hence I prefer to use the spin_lock method (which, incidentally, is also suggested in the RCU documentation). Cheers, Hannes
On 05/16/2016 11:46 PM, Hannes Reinecke wrote: > On 05/11/2016 04:44 PM, Bart Van Assche wrote: >> On 05/10/16 23:07, Hannes Reinecke wrote: >>> On 05/11/2016 07:49 AM, Hannes Reinecke wrote: >>> RIP: 0010:[<ffffffffa00b2adb>] [<ffffffffa00b2adb>] >>> fc_rport_lookup+0x4b/0x70 [libfc] >>> Call Trace: >>> [<ffffffffa00b2e17>] fc_rport_create+0x17/0x1b0 [libfc] >>> [<ffffffffa00a9f81>] fc_disc_recv_req+0x261/0x480 [libfc] >>> [<ffffffffa00b1008>] fc_lport_recv_els_req+0x68/0x130 [libfc] >>> [<ffffffffa00afd5a>] fc_lport_recv_req+0x9a/0xf0 [libfc] >>> [<ffffffffa00e8333>] fnic_handle_frame+0x63/0xd0 [fnic] >>> [<ffffffff8106fd52>] process_one_work+0x172/0x420 >>> [<ffffffff810709ca>] worker_thread+0x11a/0x3c0 >>> [<ffffffff81077344>] kthread+0xb4/0xc0 >>> [<ffffffff81521318>] ret_from_fork+0x58/0x90 >> >> Hello Hannes, >> >> Thanks for sharing this information. fc_disc_recv_req() protects the >> fc_rport_create() call via a mutex (disc_mutex). Since a mutex_lock() >> call may sleep it can trigger the start of an RCU grace period. I think >> this may result in freeing of an rport while fc_rport_lookup() is >> examining it. Have you already considered to add a >> rcu_read_lock()/rcu_read_unlock() pair in fc_rport_lookup()? >> > No, I haven't so far. > This issue is hard to trigger, and I've only got the customer report to > go by. > > Also, when using an rcu_read_lock() here one probably needs to revisit > the entire locking structure in libfc: > rport list is an RCU-proctected list, so in principle one only needs to > hold the rport mutex when _assigning_ new rports, and _could_ drop the > mutex usage for a simple lookup. > But this really needs some more thought and testing, so I haven't > attempted it. > > Also, with your suggestion I would need to take the mutex_lock _and_ > call rcu_read_lock(), which just looks wrong. > Hence I prefer to use the spin_lock method (which, incidentally, is also > suggested in the RCU documentation). Hello Hannes, I think the following section from Documentation/RCU/checklist.txt applies to the code in fc_rport_lookup(): "Do the RCU read-side critical sections make proper use of rcu_read_lock() and friends? These primitives are needed to prevent grace periods from ending prematurely, which could result in data being unceremoniously freed out from under your read-side code, which can greatly increase the actuarial risk of your kernel." Thanks, Bart. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/scsi/libfc/fc_rport.c b/drivers/scsi/libfc/fc_rport.c index 589ff9a..4520b5a 100644 --- a/drivers/scsi/libfc/fc_rport.c +++ b/drivers/scsi/libfc/fc_rport.c @@ -136,7 +136,7 @@ static struct fc_rport_priv *fc_rport_create(struct fc_lport *lport, rdata->ids.roles = FC_RPORT_ROLE_UNKNOWN; kref_init(&rdata->kref); - mutex_init(&rdata->rp_mutex); + spin_lock_init(&rdata->rp_lock); rdata->local_port = lport; rdata->rp_state = RPORT_ST_INIT; rdata->event = RPORT_EV_NONE; @@ -251,7 +251,7 @@ static void fc_rport_work(struct work_struct *work) struct fc4_prov *prov; u8 type; - mutex_lock(&rdata->rp_mutex); + spin_lock(&rdata->rp_lock); event = rdata->event; rport_ops = rdata->ops; rport = rdata->rport; @@ -264,7 +264,7 @@ static void fc_rport_work(struct work_struct *work) rdata->event = RPORT_EV_NONE; rdata->major_retries = 0; kref_get(&rdata->kref); - mutex_unlock(&rdata->rp_mutex); + spin_unlock(&rdata->rp_lock); if (!rport) rport = fc_remote_port_add(lport->host, 0, &ids); @@ -274,7 +274,7 @@ static void fc_rport_work(struct work_struct *work) kref_put(&rdata->kref, lport->tt.rport_destroy); return; } - mutex_lock(&rdata->rp_mutex); + spin_lock(&rdata->rp_lock); if (rdata->rport) FC_RPORT_DBG(rdata, "rport already allocated\n"); rdata->rport = rport; @@ -287,7 +287,7 @@ static void fc_rport_work(struct work_struct *work) rpriv->flags = rdata->flags; rpriv->e_d_tov = rdata->e_d_tov; rpriv->r_a_tov = rdata->r_a_tov; - mutex_unlock(&rdata->rp_mutex); + spin_unlock(&rdata->rp_lock); if (rport_ops && rport_ops->event_callback) { FC_RPORT_DBG(rdata, "callback ev %d\n", event); @@ -313,7 +313,7 @@ static void fc_rport_work(struct work_struct *work) mutex_unlock(&fc_prov_mutex); } port_id = rdata->ids.port_id; - mutex_unlock(&rdata->rp_mutex); + spin_unlock(&rdata->rp_lock); if (rport_ops && rport_ops->event_callback) { FC_RPORT_DBG(rdata, "callback ev %d\n", event); @@ -334,18 +334,18 @@ static void fc_rport_work(struct work_struct *work) if (rport) { rpriv = rport->dd_data; rpriv->rp_state = RPORT_ST_DELETE; - mutex_lock(&rdata->rp_mutex); + spin_lock(&rdata->rp_lock); rdata->rport = NULL; - mutex_unlock(&rdata->rp_mutex); + spin_unlock(&rdata->rp_lock); fc_remote_port_delete(rport); } mutex_lock(&lport->disc.disc_mutex); - mutex_lock(&rdata->rp_mutex); + spin_lock(&rdata->rp_lock); if (rdata->rp_state == RPORT_ST_DELETE) { if (port_id == FC_FID_DIR_SERV) { rdata->event = RPORT_EV_NONE; - mutex_unlock(&rdata->rp_mutex); + spin_unlock(&rdata->rp_lock); kref_put(&rdata->kref, lport->tt.rport_destroy); } else if ((rdata->flags & FC_RP_STARTED) && rdata->major_retries < @@ -354,11 +354,11 @@ static void fc_rport_work(struct work_struct *work) rdata->event = RPORT_EV_NONE; FC_RPORT_DBG(rdata, "work restart\n"); fc_rport_enter_flogi(rdata); - mutex_unlock(&rdata->rp_mutex); + spin_unlock(&rdata->rp_lock); } else { FC_RPORT_DBG(rdata, "work delete\n"); list_del_rcu(&rdata->peers); - mutex_unlock(&rdata->rp_mutex); + spin_unlock(&rdata->rp_lock); kref_put(&rdata->kref, lport->tt.rport_destroy); } } else { @@ -368,13 +368,13 @@ static void fc_rport_work(struct work_struct *work) rdata->event = RPORT_EV_NONE; if (rdata->rp_state == RPORT_ST_READY) fc_rport_enter_ready(rdata); - mutex_unlock(&rdata->rp_mutex); + spin_unlock(&rdata->rp_lock); } mutex_unlock(&lport->disc.disc_mutex); break; default: - mutex_unlock(&rdata->rp_mutex); + spin_unlock(&rdata->rp_lock); break; } } @@ -393,7 +393,7 @@ static void fc_rport_work(struct work_struct *work) */ static int fc_rport_login(struct fc_rport_priv *rdata) { - mutex_lock(&rdata->rp_mutex); + spin_lock(&rdata->rp_lock); rdata->flags |= FC_RP_STARTED; switch (rdata->rp_state) { @@ -409,7 +409,7 @@ static int fc_rport_login(struct fc_rport_priv *rdata) fc_rport_enter_flogi(rdata); break; } - mutex_unlock(&rdata->rp_mutex); + spin_unlock(&rdata->rp_lock); return 0; } @@ -453,7 +453,7 @@ static void fc_rport_enter_delete(struct fc_rport_priv *rdata, */ static int fc_rport_logoff(struct fc_rport_priv *rdata) { - mutex_lock(&rdata->rp_mutex); + spin_lock(&rdata->rp_lock); FC_RPORT_DBG(rdata, "Remove port\n"); @@ -470,7 +470,7 @@ static int fc_rport_logoff(struct fc_rport_priv *rdata) */ fc_rport_enter_delete(rdata, RPORT_EV_STOP); out: - mutex_unlock(&rdata->rp_mutex); + spin_unlock(&rdata->rp_lock); return 0; } @@ -505,7 +505,7 @@ static void fc_rport_timeout(struct work_struct *work) struct fc_rport_priv *rdata = container_of(work, struct fc_rport_priv, retry_work.work); - mutex_lock(&rdata->rp_mutex); + spin_lock(&rdata->rp_lock); switch (rdata->rp_state) { case RPORT_ST_FLOGI: @@ -530,7 +530,7 @@ static void fc_rport_timeout(struct work_struct *work) break; } - mutex_unlock(&rdata->rp_mutex); + spin_unlock(&rdata->rp_lock); } /** @@ -666,7 +666,7 @@ static void fc_rport_flogi_resp(struct fc_seq *sp, struct fc_frame *fp, if (fp == ERR_PTR(-FC_EX_CLOSED)) goto put; - mutex_lock(&rdata->rp_mutex); + spin_lock(&rdata->rp_lock); if (rdata->rp_state != RPORT_ST_FLOGI) { FC_RPORT_DBG(rdata, "Received a FLOGI response, but in state " @@ -700,7 +700,7 @@ static void fc_rport_flogi_resp(struct fc_seq *sp, struct fc_frame *fp, out: fc_frame_free(fp); err: - mutex_unlock(&rdata->rp_mutex); + spin_unlock(&rdata->rp_lock); put: kref_put(&rdata->kref, rdata->local_port->tt.rport_destroy); return; @@ -783,7 +783,7 @@ static void fc_rport_recv_flogi_req(struct fc_lport *lport, rjt_data.explan = ELS_EXPL_NOT_NEIGHBOR; goto reject; } - mutex_lock(&rdata->rp_mutex); + spin_lock(&rdata->rp_lock); FC_RPORT_DBG(rdata, "Received FLOGI in %s state\n", fc_rport_state(rdata)); @@ -805,7 +805,7 @@ static void fc_rport_recv_flogi_req(struct fc_lport *lport, if (lport->point_to_multipoint) break; case RPORT_ST_DELETE: - mutex_unlock(&rdata->rp_mutex); + spin_unlock(&rdata->rp_lock); rjt_data.reason = ELS_RJT_FIP; rjt_data.explan = ELS_EXPL_NOT_NEIGHBOR; goto reject; @@ -822,13 +822,13 @@ static void fc_rport_recv_flogi_req(struct fc_lport *lport, * This queues work to be sure exchanges are reset. */ fc_rport_enter_delete(rdata, RPORT_EV_LOGO); - mutex_unlock(&rdata->rp_mutex); + spin_unlock(&rdata->rp_lock); rjt_data.reason = ELS_RJT_BUSY; rjt_data.explan = ELS_EXPL_NONE; goto reject; } if (fc_rport_login_complete(rdata, fp)) { - mutex_unlock(&rdata->rp_mutex); + spin_unlock(&rdata->rp_lock); rjt_data.reason = ELS_RJT_LOGIC; rjt_data.explan = ELS_EXPL_NONE; goto reject; @@ -850,7 +850,7 @@ static void fc_rport_recv_flogi_req(struct fc_lport *lport, else fc_rport_state_enter(rdata, RPORT_ST_PLOGI_WAIT); out: - mutex_unlock(&rdata->rp_mutex); + spin_unlock(&rdata->rp_lock); mutex_unlock(&disc->disc_mutex); fc_frame_free(rx_fp); return; @@ -881,7 +881,7 @@ static void fc_rport_plogi_resp(struct fc_seq *sp, struct fc_frame *fp, u16 cssp_seq; u8 op; - mutex_lock(&rdata->rp_mutex); + spin_lock(&rdata->rp_lock); FC_RPORT_DBG(rdata, "Received a PLOGI %s\n", fc_els_resp_type(fp)); @@ -922,7 +922,7 @@ static void fc_rport_plogi_resp(struct fc_seq *sp, struct fc_frame *fp, out: fc_frame_free(fp); err: - mutex_unlock(&rdata->rp_mutex); + spin_unlock(&rdata->rp_lock); kref_put(&rdata->kref, rdata->local_port->tt.rport_destroy); } @@ -1005,7 +1005,7 @@ static void fc_rport_prli_resp(struct fc_seq *sp, struct fc_frame *fp, u8 op; u8 resp_code = 0; - mutex_lock(&rdata->rp_mutex); + spin_lock(&rdata->rp_lock); FC_RPORT_DBG(rdata, "Received a PRLI %s\n", fc_els_resp_type(fp)); @@ -1075,7 +1075,7 @@ static void fc_rport_prli_resp(struct fc_seq *sp, struct fc_frame *fp, out: fc_frame_free(fp); err: - mutex_unlock(&rdata->rp_mutex); + spin_unlock(&rdata->rp_lock); kref_put(&rdata->kref, rdata->local_port->tt.rport_destroy); } @@ -1153,7 +1153,7 @@ static void fc_rport_rtv_resp(struct fc_seq *sp, struct fc_frame *fp, struct fc_rport_priv *rdata = rdata_arg; u8 op; - mutex_lock(&rdata->rp_mutex); + spin_lock(&rdata->rp_lock); FC_RPORT_DBG(rdata, "Received a RTV %s\n", fc_els_resp_type(fp)); @@ -1197,7 +1197,7 @@ static void fc_rport_rtv_resp(struct fc_seq *sp, struct fc_frame *fp, out: fc_frame_free(fp); err: - mutex_unlock(&rdata->rp_mutex); + spin_unlock(&rdata->rp_lock); kref_put(&rdata->kref, rdata->local_port->tt.rport_destroy); } @@ -1289,7 +1289,7 @@ static void fc_rport_adisc_resp(struct fc_seq *sp, struct fc_frame *fp, struct fc_els_adisc *adisc; u8 op; - mutex_lock(&rdata->rp_mutex); + spin_lock(&rdata->rp_lock); FC_RPORT_DBG(rdata, "Received a ADISC response\n"); @@ -1326,7 +1326,7 @@ static void fc_rport_adisc_resp(struct fc_seq *sp, struct fc_frame *fp, out: fc_frame_free(fp); err: - mutex_unlock(&rdata->rp_mutex); + spin_unlock(&rdata->rp_lock); kref_put(&rdata->kref, rdata->local_port->tt.rport_destroy); } @@ -1483,7 +1483,7 @@ static void fc_rport_recv_els_req(struct fc_lport *lport, struct fc_frame *fp) mutex_unlock(&lport->disc.disc_mutex); goto reject; } - mutex_lock(&rdata->rp_mutex); + spin_lock(&rdata->rp_lock); mutex_unlock(&lport->disc.disc_mutex); switch (rdata->rp_state) { @@ -1493,7 +1493,7 @@ static void fc_rport_recv_els_req(struct fc_lport *lport, struct fc_frame *fp) case RPORT_ST_ADISC: break; default: - mutex_unlock(&rdata->rp_mutex); + spin_unlock(&rdata->rp_lock); goto reject; } @@ -1523,7 +1523,7 @@ static void fc_rport_recv_els_req(struct fc_lport *lport, struct fc_frame *fp) break; } - mutex_unlock(&rdata->rp_mutex); + spin_unlock(&rdata->rp_lock); return; reject: @@ -1616,7 +1616,7 @@ static void fc_rport_recv_plogi_req(struct fc_lport *lport, goto reject; } - mutex_lock(&rdata->rp_mutex); + spin_lock(&rdata->rp_lock); mutex_unlock(&disc->disc_mutex); rdata->ids.port_name = get_unaligned_be64(&pl->fl_wwpn); @@ -1643,7 +1643,7 @@ static void fc_rport_recv_plogi_req(struct fc_lport *lport, case RPORT_ST_PLOGI: FC_RPORT_DBG(rdata, "Received PLOGI in PLOGI state\n"); if (rdata->ids.port_name < lport->wwpn) { - mutex_unlock(&rdata->rp_mutex); + spin_unlock(&rdata->rp_lock); rjt_data.reason = ELS_RJT_INPROG; rjt_data.explan = ELS_EXPL_NONE; goto reject; @@ -1661,14 +1661,14 @@ static void fc_rport_recv_plogi_req(struct fc_lport *lport, case RPORT_ST_DELETE: FC_RPORT_DBG(rdata, "Received PLOGI in state %s - send busy\n", fc_rport_state(rdata)); - mutex_unlock(&rdata->rp_mutex); + spin_unlock(&rdata->rp_lock); rjt_data.reason = ELS_RJT_BUSY; rjt_data.explan = ELS_EXPL_NONE; goto reject; } if (!fc_rport_compatible_roles(lport, rdata)) { FC_RPORT_DBG(rdata, "Received PLOGI for incompatible role\n"); - mutex_unlock(&rdata->rp_mutex); + spin_unlock(&rdata->rp_lock); rjt_data.reason = ELS_RJT_LOGIC; rjt_data.explan = ELS_EXPL_NONE; goto reject; @@ -1691,7 +1691,7 @@ static void fc_rport_recv_plogi_req(struct fc_lport *lport, lport->tt.frame_send(lport, fp); fc_rport_enter_prli(rdata); out: - mutex_unlock(&rdata->rp_mutex); + spin_unlock(&rdata->rp_lock); fc_frame_free(rx_fp); return; @@ -1910,12 +1910,12 @@ static void fc_rport_recv_logo_req(struct fc_lport *lport, struct fc_frame *fp) mutex_lock(&lport->disc.disc_mutex); rdata = lport->tt.rport_lookup(lport, sid); if (rdata) { - mutex_lock(&rdata->rp_mutex); + spin_lock(&rdata->rp_lock); FC_RPORT_DBG(rdata, "Received LOGO request while in state %s\n", fc_rport_state(rdata)); fc_rport_enter_delete(rdata, RPORT_EV_LOGO); - mutex_unlock(&rdata->rp_mutex); + spin_unlock(&rdata->rp_lock); } else FC_RPORT_ID_DBG(lport, sid, "Received LOGO from non-logged-in port\n"); diff --git a/include/scsi/libfc.h b/include/scsi/libfc.h index 93d14da..7d63d23 100644 --- a/include/scsi/libfc.h +++ b/include/scsi/libfc.h @@ -187,7 +187,7 @@ struct fc_rport_libfc_priv { * @major_retries: The retry count for the entire PLOGI/PRLI state machine * @e_d_tov: Error detect timeout value (in msec) * @r_a_tov: Resource allocation timeout value (in msec) - * @rp_mutex: The mutex that protects the remote port + * @rp_lock: The lock that protects the remote port * @retry_work: Handle for retries * @event_callback: Callback when READY, FAILED or LOGO states complete * @prli_count: Count of open PRLI sessions in providers @@ -207,7 +207,7 @@ struct fc_rport_priv { unsigned int major_retries; unsigned int e_d_tov; unsigned int r_a_tov; - struct mutex rp_mutex; + spinlock_t rp_lock; struct delayed_work retry_work; enum fc_rport_event event; struct fc_rport_operations *ops;
We cannot use an embedded mutex in a structure with reference counting, as mutex unlock might be delayed, and the waiters might then access an already freed memory area. So convert it to a spinlock. For details cf https://lkml.org/lkml/2015/2/11/245 Signed-off-by: Hannes Reinecke <hare@suse.de> --- drivers/scsi/libfc/fc_rport.c | 90 +++++++++++++++++++++---------------------- include/scsi/libfc.h | 4 +- 2 files changed, 47 insertions(+), 47 deletions(-)