Commit 01d0c698 authored by Christoph Hellwig's avatar Christoph Hellwig Committed by Jens Axboe
Browse files

sr: implement ->free_disk to simplify refcounting 



Simplify the refcounting and remove the need to clear disk->private_data
by implementing the ->free_disk method.

Signed-off-by: default avatarChristoph Hellwig <hch@lst.de>
Reviewed-by: default avatarBart Van Assche <bvanassche@acm.org>
Reviewed-by: default avatarMing Lei <ming.lei@redhat.com>
Reviewed-by: default avatarMartin K. Petersen <martin.petersen@oracle.com>
Link: https://lore.kernel.org/r/20220308055200.735835-9-hch@lst.de


Signed-off-by: default avatarJens Axboe <axboe@kernel.dk>
parent 9c63f7f6

drivers/scsi/sr.c

+26 −98
Original line number Diff line number Diff line
@@ -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 *);
@@ -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;
@@ -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];
@@ -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);
@@ -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;

}
@@ -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,
@@ -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 =
@@ -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)
@@ -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)
@@ -727,10 +689,8 @@ 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);
@@ -738,6 +698,8 @@ static int sr_probe(struct device *dev) 


	return 0;



unregister_cdrom:

	unregister_cdrom(&cd->cdi);

fail_minor:

	spin_lock(&sr_index_lock);

	clear_bit(minor, sr_index_bits);
@@ -1009,36 +971,6 @@ 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);
@@ -1046,11 +978,7 @@ static int sr_remove(struct device *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;

}

drivers/scsi/sr.h

+0 −4
Original line number Diff line number Diff line
@@ -18,7 +18,6 @@ 
#ifndef _SR_H

#define _SR_H



#include <linux/kref.h>

#include <linux/mutex.h>



#define MAX_RETRIES	3
@@ -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;