On Wed, 2016-04-20 at 15:24 +0200, Hannes Reinecke wrote: > When pushing items on a workqueue we cannot take reference > when the workqueue item is executed, as the structure might > already been freed at that time. > So instead we need to take a reference before adding it > to the workqueue, thereby ensuring that the workqueue item > will always be valid.
Have you actually seen this happen? The rdata structure is fully ref counted, so if it's done a final put, then something should see unreferenced memory. It looks like the model is that the final put is done from the queue, so I don't quite see how you can lose the final reference in either of the places you alter. Plus, kref_get_unless_zero() should not be used. At that point, the structure would be freed, so there's no point looking for it. kref_get_unless_zero is for refcounts that don't necessarily free the structure (embedded ones). James > Signed-off-by: Hannes Reinecke <[email protected]> > --- > drivers/scsi/libfc/fc_rport.c | 25 +++++++++++++++++++------ > 1 file changed, 19 insertions(+), 6 deletions(-) > > diff --git a/drivers/scsi/libfc/fc_rport.c > b/drivers/scsi/libfc/fc_rport.c > index 589ff9a..8b08263f 100644 > --- a/drivers/scsi/libfc/fc_rport.c > +++ b/drivers/scsi/libfc/fc_rport.c > @@ -263,7 +263,6 @@ static void fc_rport_work(struct work_struct > *work) > ids = rdata->ids; > rdata->event = RPORT_EV_NONE; > rdata->major_retries = 0; > - kref_get(&rdata->kref); > mutex_unlock(&rdata->rp_mutex); > > if (!rport) > @@ -297,7 +296,6 @@ static void fc_rport_work(struct work_struct > *work) > FC_RPORT_DBG(rdata, "lld callback ev %d\n", > event); > rdata->lld_event_callback(lport, rdata, > event); > } > - kref_put(&rdata->kref, lport->tt.rport_destroy); > break; > > case RPORT_EV_FAILED: > @@ -377,6 +375,7 @@ static void fc_rport_work(struct work_struct > *work) > mutex_unlock(&rdata->rp_mutex); > break; > } > + kref_put(&rdata->kref, lport->tt.rport_destroy); > } > > /** > @@ -438,8 +437,15 @@ static void fc_rport_enter_delete(struct > fc_rport_priv *rdata, > > fc_rport_state_enter(rdata, RPORT_ST_DELETE); > > - if (rdata->event == RPORT_EV_NONE) > - queue_work(rport_event_queue, &rdata->event_work); > + if (rdata->event == RPORT_EV_NONE) { > + if (!kref_get_unless_zero(&rdata->kref)) { > + FC_RPORT_DBG(rdata, "port already > deleted\n"); > + return; > + } > + if (!queue_work(rport_event_queue, &rdata > ->event_work)) > + kref_put(&rdata->kref, > + rdata->local_port > ->tt.rport_destroy); > + } > rdata->event = event; > } > > @@ -487,8 +493,15 @@ static void fc_rport_enter_ready(struct > fc_rport_priv *rdata) > > FC_RPORT_DBG(rdata, "Port is Ready\n"); > > - if (rdata->event == RPORT_EV_NONE) > - queue_work(rport_event_queue, &rdata->event_work); > + if (rdata->event == RPORT_EV_NONE) { > + if (!kref_get_unless_zero(&rdata->kref)) { > + FC_RPORT_DBG(rdata, "port already > deleted\n"); > + return; > + } > + if (!queue_work(rport_event_queue, &rdata > ->event_work)) > + kref_put(&rdata->kref, > + rdata->local_port > ->tt.rport_destroy); > + } > rdata->event = RPORT_EV_READY; > } > -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
