Skip to content

Commit

Permalink
sr: implement ->free_disk to simplify refcounting
Browse files Browse the repository at this point in the history 
Simplify the refcounting and remove the need to clear disk->private_data
by implementing the ->free_disk method.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Reviewed-by: Ming Lei <ming.lei@redhat.com>
Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>
Link: https://lore.kernel.org/r/20220308055200.735835-9-hch@lst.de
Signed-off-by: Jens Axboe <axboe@kernel.dk>
  • Loading branch information
@axboe
Christoph Hellwig authored and axboe committed Mar 9, 2022
1 parent 9c63f7f commit 01d0c69
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 102 deletions.
124 changes: 26 additions & 98 deletions drivers/scsi/sr.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,11 +109,6 @@ static DEFINE_SPINLOCK(sr_index_lock);

static struct lock_class_key sr_bio_compl_lkclass;

/* This semaphore is used to mediate the 0->1 reference get in the
* face of object destruction (i.e. we can't allow a get on an
* object after last put) */
static DEFINE_MUTEX(sr_ref_mutex);

static int sr_open(struct cdrom_device_info *, int);
static void sr_release(struct cdrom_device_info *);

Expand Down Expand Up @@ -143,8 +138,6 @@ static const struct cdrom_device_ops sr_dops = {
.capability = SR_CAPABILITIES,
};

static void sr_kref_release(struct kref *kref);

static inline struct scsi_cd *scsi_cd(struct gendisk *disk)
{
return disk->private_data;
Expand All @@ -163,38 +156,6 @@ static int sr_runtime_suspend(struct device *dev)
return 0;
}

/*
* The get and put routines for the struct scsi_cd. Note this entity
* has a scsi_device pointer and owns a reference to this.
*/
static inline struct scsi_cd *scsi_cd_get(struct gendisk *disk)
{
struct scsi_cd *cd = NULL;

mutex_lock(&sr_ref_mutex);
if (disk->private_data == NULL)
goto out;
cd = scsi_cd(disk);
kref_get(&cd->kref);
if (scsi_device_get(cd->device)) {
kref_put(&cd->kref, sr_kref_release);
cd = NULL;
}
out:
mutex_unlock(&sr_ref_mutex);
return cd;
}

static void scsi_cd_put(struct scsi_cd *cd)
{
struct scsi_device *sdev = cd->device;

mutex_lock(&sr_ref_mutex);
kref_put(&cd->kref, sr_kref_release);
scsi_device_put(sdev);
mutex_unlock(&sr_ref_mutex);
}

static unsigned int sr_get_events(struct scsi_device *sdev)
{
u8 buf[8];
Expand Down Expand Up @@ -522,15 +483,13 @@ static void sr_revalidate_disk(struct scsi_cd *cd)

static int sr_block_open(struct block_device *bdev, fmode_t mode)
{
struct scsi_cd *cd;
struct scsi_device *sdev;
struct scsi_cd *cd = cd = scsi_cd(bdev->bd_disk);
struct scsi_device *sdev = cd->device;
int ret = -ENXIO;

cd = scsi_cd_get(bdev->bd_disk);
if (!cd)
goto out;
if (scsi_device_get(cd->device))
return -ENXIO;

sdev = cd->device;
scsi_autopm_get_device(sdev);
if (bdev_check_media_change(bdev))
sr_revalidate_disk(cd);
Expand All @@ -541,9 +500,7 @@ static int sr_block_open(struct block_device *bdev, fmode_t mode)

scsi_autopm_put_device(sdev);
if (ret)
scsi_cd_put(cd);

out:
scsi_device_put(cd->device);
return ret;
}

Expand All @@ -555,7 +512,7 @@ static void sr_block_release(struct gendisk *disk, fmode_t mode)
cdrom_release(&cd->cdi, mode);
mutex_unlock(&cd->lock);

scsi_cd_put(cd);
scsi_device_put(cd->device);
}

static int sr_block_ioctl(struct block_device *bdev, fmode_t mode, unsigned cmd,
Expand Down Expand Up @@ -595,18 +552,24 @@ static int sr_block_ioctl(struct block_device *bdev, fmode_t mode, unsigned cmd,
static unsigned int sr_block_check_events(struct gendisk *disk,
unsigned int clearing)
{
unsigned int ret = 0;
struct scsi_cd *cd;
struct scsi_cd *cd = disk->private_data;

cd = scsi_cd_get(disk);
if (!cd)
if (atomic_read(&cd->device->disk_events_disable_depth))
return 0;
return cdrom_check_events(&cd->cdi, clearing);
}

if (!atomic_read(&cd->device->disk_events_disable_depth))
ret = cdrom_check_events(&cd->cdi, clearing);
static void sr_free_disk(struct gendisk *disk)
{
struct scsi_cd *cd = disk->private_data;

scsi_cd_put(cd);
return ret;
spin_lock(&sr_index_lock);
clear_bit(MINOR(disk_devt(disk)), sr_index_bits);
spin_unlock(&sr_index_lock);

unregister_cdrom(&cd->cdi);
mutex_destroy(&cd->lock);
kfree(cd);
}

static const struct block_device_operations sr_bdops =
Expand All @@ -617,6 +580,7 @@ static const struct block_device_operations sr_bdops =
.ioctl = sr_block_ioctl,
.compat_ioctl = blkdev_compat_ptr_ioctl,
.check_events = sr_block_check_events,
.free_disk = sr_free_disk,
};

static int sr_open(struct cdrom_device_info *cdi, int purpose)
Expand Down Expand Up @@ -660,8 +624,6 @@ static int sr_probe(struct device *dev)
if (!cd)
goto fail;

kref_init(&cd->kref);

disk = __alloc_disk_node(sdev->request_queue, NUMA_NO_NODE,
&sr_bio_compl_lkclass);
if (!disk)
Expand Down Expand Up @@ -727,17 +689,17 @@ static int sr_probe(struct device *dev)
sr_revalidate_disk(cd);

error = device_add_disk(&sdev->sdev_gendev, disk, NULL);
if (error) {
kref_put(&cd->kref, sr_kref_release);
goto fail;
}
if (error)
goto unregister_cdrom;

sdev_printk(KERN_DEBUG, sdev,
"Attached scsi CD-ROM %s\n", cd->cdi.name);
scsi_autopm_put_device(cd->device);

return 0;

unregister_cdrom:
unregister_cdrom(&cd->cdi);
fail_minor:
spin_lock(&sr_index_lock);
clear_bit(minor, sr_index_bits);
Expand Down Expand Up @@ -1009,48 +971,14 @@ static int sr_read_cdda_bpc(struct cdrom_device_info *cdi, void __user *ubuf,
return ret;
}


/**
* sr_kref_release - Called to free the scsi_cd structure
* @kref: pointer to embedded kref
*
* sr_ref_mutex must be held entering this routine. Because it is
* called on last put, you should always use the scsi_cd_get()
* scsi_cd_put() helpers which manipulate the semaphore directly
* and never do a direct kref_put().
**/
static void sr_kref_release(struct kref *kref)
{
struct scsi_cd *cd = container_of(kref, struct scsi_cd, kref);
struct gendisk *disk = cd->disk;

spin_lock(&sr_index_lock);
clear_bit(MINOR(disk_devt(disk)), sr_index_bits);
spin_unlock(&sr_index_lock);

unregister_cdrom(&cd->cdi);

disk->private_data = NULL;

put_disk(disk);

mutex_destroy(&cd->lock);

kfree(cd);
}

static int sr_remove(struct device *dev)
{
struct scsi_cd *cd = dev_get_drvdata(dev);

scsi_autopm_get_device(cd->device);

del_gendisk(cd->disk);
dev_set_drvdata(dev, NULL);

mutex_lock(&sr_ref_mutex);
kref_put(&cd->kref, sr_kref_release);
mutex_unlock(&sr_ref_mutex);
put_disk(cd->disk);

return 0;
}
Expand Down
4 changes: 0 additions & 4 deletions drivers/scsi/sr.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
#ifndef _SR_H
#define _SR_H

#include <linux/kref.h>
#include <linux/mutex.h>

#define MAX_RETRIES 3
Expand Down Expand Up @@ -51,9 +50,6 @@ typedef struct scsi_cd {

struct cdrom_device_info cdi;
struct mutex lock;
/* We hold gendisk and scsi_device references on probe and use
* the refs on this kref to decide when to release them */
struct kref kref;
struct gendisk *disk;
} Scsi_CD;

Expand Down

0 comments on commit 01d0c69

Please sign in to comment.