Thursday 19 February 2009 17:49:52 Bartlomiej Zolnierkiewicz napisał(a): > > > Seems like ide_device_put() needs the same module_refcount() check that > > > is present in scsi_device_put() so removal of device driver won't trigger > > > a spurious module_put() on a host driver? > > > > I little surprise about scsi code (linux-scsi ML CC). Is comment inside > > scsi_device_put() function correct? Why scsi_device_get() not check > > try_module_get() return value? And most importand: there is reference > > counter check before put, so it can be 0, but data does it protect is in > > use ? Any comments? > Uh... we will need some more intrusive changes to the reference counting > to fix it -- like to replace idkp->kref by idkp->dev and make drive->gendev > a parent of it (so only after the final put on ->dev ->gendev can go away). > > [ IOW we need to have some changes similar to those done in sd.c by: > commit 6bdaa1f17dd32ec62345c7b57842f53e6278a2fa > and later by: > commit ee959b00c335d7780136c5abda37809191fe52c3 ] > > > There is no oops with my workaround, when I just remove ide_disk_put() from > > I suppose that after ide_disk_put() removal ide_disk_release() is simply > never called... ;) > > > ide_gd_remove(). It's strange why there is lack of symmetrical _put/_get calls, > > ide_gd_probe() has no call to ide_disk_get(). > > We have kref_init() in ide_disk_probe(), so there is no need for it > and we also don't want to hold an extra reference on host driver... Looks that using ->dev insted of ->kref will do the work. But perhaps less intrusive fix, like check kref in ide_disk_put() would be better solution. I tested below patch and everythings is fine. diff --git a/drivers/ide/ide-gd.c b/drivers/ide/ide-gd.c index 7857b20..598f21b 100644 --- a/drivers/ide/ide-gd.c +++ b/drivers/ide/ide-gd.c @@ -48,8 +48,8 @@ static void ide_disk_put(struct ide_disk_obj *idkp) ide_drive_t *drive = idkp->drive; mutex_lock(&ide_disk_ref_mutex); - kref_put(&idkp->kref, ide_disk_release); - ide_device_put(drive); + if (!kref_put(&idkp->kref, ide_disk_release)) + ide_device_put(drive); mutex_unlock(&ide_disk_ref_mutex); } If this patch is ok and dropping kref to dev is not planed currently, maybe I'll send "official" patch with ide-gd fix and for other devices types. Regards Stanislaw Gruszka -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
