Re: Bad module reference counter

[Stanislaw Gruszka <stf_xl@wp.pl>]

Monday 23 February 2009 23:36:35 Bartlomiej Zolnierkiewicz napisał(a):
> > 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);
> 
> I worry that this just masks the problem as according to your previous
> mail drive still can be already gone before ->flush in ide_gd_remove().

I checked my patch against all mentioned problems, but it doesn't matter now.
 
> >  	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.
> 
> Lets fix it fully.  The below patch together with previous ide_device_put()
> fix and drive_release_dev() one (from another mail) should make all problems
> go away...
> 
> From: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> Subject: [PATCH] ide: fix refcounting in device drivers
> 
> During host driver module removal del_gendisk() results in a final
> put on drive->gendev and freeing the drive by drive_release_dev().
> 
> Convert device drivers from using struct kref to use struct device
> so device driver's object holds reference on ->gendev and prevents
> drive from prematurely going away.
> 
> Also fix ->remove methods to not erroneously drop reference on a
> host driver by using only put_device() instead of ide*_put().
> 
> Reported-by: Stanislaw Gruszka <stf_xl@wp.pl>
> Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>

Tested-by: Stanislaw Gruszka <stf_xl@wp.pl>