Skip to content

Commit 9c63f7f

Browse files
Christoph Hellwigaxboe
authored andcommitted
sd: implement ->free_disk to simplify refcounting
Implement the ->free_disk method to to put struct scsi_disk when the last gendisk reference count goes away. This removes the need to clear ->private_data and thus freeze the queue on unbind. Signed-off-by: Christoph Hellwig <hch@lst.de> 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-8-hch@lst.de Signed-off-by: Jens Axboe <axboe@kernel.dk>
1 parent 534cf52 commit 9c63f7f

File tree

1 file changed

+16
-74
lines changed

1 file changed

+16
-74
lines changed

drivers/scsi/sd.c

Lines changed: 16 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -121,11 +121,6 @@ static void scsi_disk_release(struct device *cdev);
121121

122122
static DEFINE_IDA(sd_index_ida);
123123

124-
/* This semaphore is used to mediate the 0->1 reference get in the
125-
* face of object destruction (i.e. we can't allow a get on an
126-
* object after last put) */
127-
static DEFINE_MUTEX(sd_ref_mutex);
128-
129124
static struct kmem_cache *sd_cdb_cache;
130125
static mempool_t *sd_cdb_pool;
131126
static mempool_t *sd_page_pool;
@@ -663,33 +658,6 @@ static int sd_major(int major_idx)
663658
}
664659
}
665660

666-
static struct scsi_disk *scsi_disk_get(struct gendisk *disk)
667-
{
668-
struct scsi_disk *sdkp = NULL;
669-
670-
mutex_lock(&sd_ref_mutex);
671-
672-
if (disk->private_data) {
673-
sdkp = scsi_disk(disk);
674-
if (scsi_device_get(sdkp->device) == 0)
675-
get_device(&sdkp->disk_dev);
676-
else
677-
sdkp = NULL;
678-
}
679-
mutex_unlock(&sd_ref_mutex);
680-
return sdkp;
681-
}
682-
683-
static void scsi_disk_put(struct scsi_disk *sdkp)
684-
{
685-
struct scsi_device *sdev = sdkp->device;
686-
687-
mutex_lock(&sd_ref_mutex);
688-
put_device(&sdkp->disk_dev);
689-
scsi_device_put(sdev);
690-
mutex_unlock(&sd_ref_mutex);
691-
}
692-
693661
#ifdef CONFIG_BLK_SED_OPAL
694662
static int sd_sec_submit(void *data, u16 spsp, u8 secp, void *buffer,
695663
size_t len, bool send)
@@ -1418,17 +1386,15 @@ static bool sd_need_revalidate(struct block_device *bdev,
14181386
**/
14191387
static int sd_open(struct block_device *bdev, fmode_t mode)
14201388
{
1421-
struct scsi_disk *sdkp = scsi_disk_get(bdev->bd_disk);
1422-
struct scsi_device *sdev;
1389+
struct scsi_disk *sdkp = scsi_disk(bdev->bd_disk);
1390+
struct scsi_device *sdev = sdkp->device;
14231391
int retval;
14241392

1425-
if (!sdkp)
1393+
if (scsi_device_get(sdev))
14261394
return -ENXIO;
14271395

14281396
SCSI_LOG_HLQUEUE(3, sd_printk(KERN_INFO, sdkp, "sd_open\n"));
14291397

1430-
sdev = sdkp->device;
1431-
14321398
/*
14331399
* If the device is in error recovery, wait until it is done.
14341400
* If the device is offline, then disallow any access to it.
@@ -1473,7 +1439,7 @@ static int sd_open(struct block_device *bdev, fmode_t mode)
14731439
return 0;
14741440

14751441
error_out:
1476-
scsi_disk_put(sdkp);
1442+
scsi_device_put(sdev);
14771443
return retval;
14781444
}
14791445

@@ -1502,7 +1468,7 @@ static void sd_release(struct gendisk *disk, fmode_t mode)
15021468
scsi_set_medium_removal(sdev, SCSI_REMOVAL_ALLOW);
15031469
}
15041470

1505-
scsi_disk_put(sdkp);
1471+
scsi_device_put(sdev);
15061472
}
15071473

15081474
static int sd_getgeo(struct block_device *bdev, struct hd_geometry *geo)
@@ -1616,7 +1582,7 @@ static int media_not_present(struct scsi_disk *sdkp,
16161582
**/
16171583
static unsigned int sd_check_events(struct gendisk *disk, unsigned int clearing)
16181584
{
1619-
struct scsi_disk *sdkp = scsi_disk_get(disk);
1585+
struct scsi_disk *sdkp = disk->private_data;
16201586
struct scsi_device *sdp;
16211587
int retval;
16221588
bool disk_changed;
@@ -1679,7 +1645,6 @@ static unsigned int sd_check_events(struct gendisk *disk, unsigned int clearing)
16791645
*/
16801646
disk_changed = sdp->changed;
16811647
sdp->changed = 0;
1682-
scsi_disk_put(sdkp);
16831648
return disk_changed ? DISK_EVENT_MEDIA_CHANGE : 0;
16841649
}
16851650

@@ -1887,6 +1852,13 @@ static const struct pr_ops sd_pr_ops = {
18871852
.pr_clear = sd_pr_clear,
18881853
};
18891854

1855+
static void scsi_disk_free_disk(struct gendisk *disk)
1856+
{
1857+
struct scsi_disk *sdkp = scsi_disk(disk);
1858+
1859+
put_device(&sdkp->disk_dev);
1860+
}
1861+
18901862
static const struct block_device_operations sd_fops = {
18911863
.owner = THIS_MODULE,
18921864
.open = sd_open,
@@ -1898,6 +1870,7 @@ static const struct block_device_operations sd_fops = {
18981870
.unlock_native_capacity = sd_unlock_native_capacity,
18991871
.report_zones = sd_zbc_report_zones,
19001872
.get_unique_id = sd_get_unique_id,
1873+
.free_disk = scsi_disk_free_disk,
19011874
.pr_ops = &sd_pr_ops,
19021875
};
19031876

@@ -3623,54 +3596,23 @@ static int sd_probe(struct device *dev)
36233596
**/
36243597
static int sd_remove(struct device *dev)
36253598
{
3626-
struct scsi_disk *sdkp;
3599+
struct scsi_disk *sdkp = dev_get_drvdata(dev);
36273600

3628-
sdkp = dev_get_drvdata(dev);
36293601
scsi_autopm_get_device(sdkp->device);
36303602

36313603
device_del(&sdkp->disk_dev);
36323604
del_gendisk(sdkp->disk);
36333605
sd_shutdown(dev);
36343606

3635-
mutex_lock(&sd_ref_mutex);
3636-
dev_set_drvdata(dev, NULL);
3637-
put_device(&sdkp->disk_dev);
3638-
mutex_unlock(&sd_ref_mutex);
3639-
3607+
put_disk(sdkp->disk);
36403608
return 0;
36413609
}
36423610

3643-
/**
3644-
* scsi_disk_release - Called to free the scsi_disk structure
3645-
* @dev: pointer to embedded class device
3646-
*
3647-
* sd_ref_mutex must be held entering this routine. Because it is
3648-
* called on last put, you should always use the scsi_disk_get()
3649-
* scsi_disk_put() helpers which manipulate the semaphore directly
3650-
* and never do a direct put_device.
3651-
**/
36523611
static void scsi_disk_release(struct device *dev)
36533612
{
36543613
struct scsi_disk *sdkp = to_scsi_disk(dev);
3655-
struct gendisk *disk = sdkp->disk;
3656-
struct request_queue *q = disk->queue;
36573614

36583615
ida_free(&sd_index_ida, sdkp->index);
3659-
3660-
/*
3661-
* Wait until all requests that are in progress have completed.
3662-
* This is necessary to avoid that e.g. scsi_end_request() crashes
3663-
* due to clearing the disk->private_data pointer. Wait from inside
3664-
* scsi_disk_release() instead of from sd_release() to avoid that
3665-
* freezing and unfreezing the request queue affects user space I/O
3666-
* in case multiple processes open a /dev/sd... node concurrently.
3667-
*/
3668-
blk_mq_freeze_queue(q);
3669-
blk_mq_unfreeze_queue(q);
3670-
3671-
disk->private_data = NULL;
3672-
put_disk(disk);
3673-
36743616
sd_zbc_release_disk(sdkp);
36753617
put_device(&sdkp->device->sdev_gendev);
36763618
free_opal_dev(sdkp->opal_dev);

0 commit comments

Comments
 (0)