On 10/19/2014 05:59 PM, Christoph Hellwig wrote: > We need to grab a reference to the module before calling the attach > routines to avoid a small race vs module removal. It also cleans up > the code significantly as a side effect. > > Signed-off-by: Christoph Hellwig <[email protected]> > --- > drivers/scsi/device_handler/scsi_dh.c | 31 > ++++++++++++++++++++--------- > drivers/scsi/device_handler/scsi_dh_alua.c | 4 ---- > drivers/scsi/device_handler/scsi_dh_emc.c | 4 ---- > drivers/scsi/device_handler/scsi_dh_hp_sw.c | 4 ---- > drivers/scsi/device_handler/scsi_dh_rdac.c | 4 ---- > 5 files changed, 22 insertions(+), 25 deletions(-) > > diff --git a/drivers/scsi/device_handler/scsi_dh.c > b/drivers/scsi/device_handler/scsi_dh.c > index 33e422e..1a8dbf3 100644 > --- a/drivers/scsi/device_handler/scsi_dh.c > +++ b/drivers/scsi/device_handler/scsi_dh.c > @@ -102,23 +102,36 @@ static int scsi_dh_handler_attach(struct scsi_device > *sdev, > > if (sdev->scsi_dh_data) { > if (sdev->scsi_dh_data->scsi_dh != scsi_dh) > - err = -EBUSY; > - else > - kref_get(&sdev->scsi_dh_data->kref); > - } else if (scsi_dh->attach) { > + return -EBUSY; > + > + kref_get(&sdev->scsi_dh_data->kref); > + return 0; > + } > + > + if (scsi_dh->attach) { > + if (!try_module_get(scsi_dh->module)) > + return -EINVAL; > + > err = scsi_dh->attach(sdev); > - if (!err) { > - kref_init(&sdev->scsi_dh_data->kref); > - sdev->scsi_dh_data->sdev = sdev; > + if (err) { > + module_put(scsi_dh->module); > + return err; > } > + > + kref_init(&sdev->scsi_dh_data->kref); > + sdev->scsi_dh_data->sdev = sdev; > } > return err; > } > > static void __detach_handler (struct kref *kref) > { > - struct scsi_dh_data *scsi_dh_data = container_of(kref, struct > scsi_dh_data, kref); > - scsi_dh_data->scsi_dh->detach(scsi_dh_data->sdev); > + struct scsi_dh_data *scsi_dh_data = > + container_of(kref, struct scsi_dh_data, kref); > + struct scsi_device_handler *scsi_dh = scsi_dh_data->scsi_dh; > + > + scsi_dh->detach(scsi_dh_data->sdev); > + module_put(scsi_dh->module); > } > > /* Is it guaranteed that you cannot call ->attach() for devices which already have a device_handler attached? You've skipped the case
scsi_dh != sdev->scsi_dh_data->scsi_dh IE someone called 'attach()' on a device which already has a different device_handler attached to it. Cheers, Hannes -- Dr. Hannes Reinecke zSeries & Storage [email protected] +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg) -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
