Skip to content

Commit

Permalink
Revert "scsi, block: fix duplicate bdi name registration crashes"
Browse files Browse the repository at this point in the history
This reverts commit 0dba131. It causes
leaking of device numbers for SCSI when SCSI registers multiple gendisks
for one request_queue in succession. It can be easily reproduced using
Omar's script [1] on kernel with CONFIG_DEBUG_TEST_DRIVER_REMOVE.
Furthermore the protection provided by this commit is not needed anymore
as the problem it was fixing got also fixed by commit 165a5e2
"block: Move bdi_unregister() to del_gendisk()".

[1]: http://marc.info/?l=linux-block&m=148554717109098&w=2

Signed-off-by: Jan Kara <jack@suse.cz>
Acked-by: Dan Williams <dan.j.williams@intel.com>
Tested-by: Omar Sandoval <osandov@fb.com>
Signed-off-by: Jens Axboe <axboe@fb.com>
  • Loading branch information
Jan Kara authored and Jens Axboe committed Mar 8, 2017
1 parent 90f16fd commit c01228d
Show file tree
Hide file tree
Showing 5 changed files with 8 additions and 65 deletions.
2 changes: 0 additions & 2 deletions block/blk-core.c
Original file line number Diff line number Diff line change
Expand Up @@ -578,8 +578,6 @@ void blk_cleanup_queue(struct request_queue *q)
q->queue_lock = &q->__queue_lock;
spin_unlock_irq(lock);

put_disk_devt(q->disk_devt);

/* @q is and will stay empty, shutdown and put */
blk_put_queue(q);
}
Expand Down
21 changes: 0 additions & 21 deletions block/genhd.c
Original file line number Diff line number Diff line change
Expand Up @@ -572,20 +572,6 @@ static void register_disk(struct device *parent, struct gendisk *disk)
disk_part_iter_exit(&piter);
}

void put_disk_devt(struct disk_devt *disk_devt)
{
if (disk_devt && atomic_dec_and_test(&disk_devt->count))
disk_devt->release(disk_devt);
}
EXPORT_SYMBOL(put_disk_devt);

void get_disk_devt(struct disk_devt *disk_devt)
{
if (disk_devt)
atomic_inc(&disk_devt->count);
}
EXPORT_SYMBOL(get_disk_devt);

/**
* device_add_disk - add partitioning information to kernel list
* @parent: parent device for the disk
Expand Down Expand Up @@ -626,13 +612,6 @@ void device_add_disk(struct device *parent, struct gendisk *disk)

disk_alloc_events(disk);

/*
* Take a reference on the devt and assign it to queue since it
* must not be reallocated while the bdi is registered
*/
disk->queue->disk_devt = disk->disk_devt;
get_disk_devt(disk->disk_devt);

/* Register BDI before referencing it from bdev */
bdi = disk->queue->backing_dev_info;
bdi_register_owner(bdi, disk_to_dev(disk));
Expand Down
41 changes: 8 additions & 33 deletions drivers/scsi/sd.c
Original file line number Diff line number Diff line change
Expand Up @@ -3075,23 +3075,6 @@ static void sd_probe_async(void *data, async_cookie_t cookie)
put_device(&sdkp->dev);
}

struct sd_devt {
int idx;
struct disk_devt disk_devt;
};

static void sd_devt_release(struct disk_devt *disk_devt)
{
struct sd_devt *sd_devt = container_of(disk_devt, struct sd_devt,
disk_devt);

spin_lock(&sd_index_lock);
ida_remove(&sd_index_ida, sd_devt->idx);
spin_unlock(&sd_index_lock);

kfree(sd_devt);
}

/**
* sd_probe - called during driver initialization and whenever a
* new scsi device is attached to the system. It is called once
Expand All @@ -3113,7 +3096,6 @@ static void sd_devt_release(struct disk_devt *disk_devt)
static int sd_probe(struct device *dev)
{
struct scsi_device *sdp = to_scsi_device(dev);
struct sd_devt *sd_devt;
struct scsi_disk *sdkp;
struct gendisk *gd;
int index;
Expand All @@ -3139,13 +3121,9 @@ static int sd_probe(struct device *dev)
if (!sdkp)
goto out;

sd_devt = kzalloc(sizeof(*sd_devt), GFP_KERNEL);
if (!sd_devt)
goto out_free;

gd = alloc_disk(SD_MINORS);
if (!gd)
goto out_free_devt;
goto out_free;

do {
if (!ida_pre_get(&sd_index_ida, GFP_KERNEL))
Expand All @@ -3161,11 +3139,6 @@ static int sd_probe(struct device *dev)
goto out_put;
}

atomic_set(&sd_devt->disk_devt.count, 1);
sd_devt->disk_devt.release = sd_devt_release;
sd_devt->idx = index;
gd->disk_devt = &sd_devt->disk_devt;

error = sd_format_disk_name("sd", index, gd->disk_name, DISK_NAME_LEN);
if (error) {
sdev_printk(KERN_WARNING, sdp, "SCSI disk (sd) name length exceeded.\n");
Expand Down Expand Up @@ -3205,12 +3178,11 @@ static int sd_probe(struct device *dev)
return 0;

out_free_index:
put_disk_devt(&sd_devt->disk_devt);
sd_devt = NULL;
spin_lock(&sd_index_lock);
ida_remove(&sd_index_ida, index);
spin_unlock(&sd_index_lock);
out_put:
put_disk(gd);
out_free_devt:
kfree(sd_devt);
out_free:
kfree(sdkp);
out:
Expand Down Expand Up @@ -3271,7 +3243,10 @@ static void scsi_disk_release(struct device *dev)
struct scsi_disk *sdkp = to_scsi_disk(dev);
struct gendisk *disk = sdkp->disk;

put_disk_devt(disk->disk_devt);
spin_lock(&sd_index_lock);
ida_remove(&sd_index_ida, sdkp->index);
spin_unlock(&sd_index_lock);

disk->private_data = NULL;
put_disk(disk);
put_device(&sdkp->device->sdev_gendev);
Expand Down
1 change: 0 additions & 1 deletion include/linux/blkdev.h
Original file line number Diff line number Diff line change
Expand Up @@ -435,7 +435,6 @@ struct request_queue {
struct delayed_work delay_work;

struct backing_dev_info *backing_dev_info;
struct disk_devt *disk_devt;

/*
* The queue owner gets to use this for whatever they like.
Expand Down
8 changes: 0 additions & 8 deletions include/linux/genhd.h
Original file line number Diff line number Diff line change
Expand Up @@ -167,13 +167,6 @@ struct blk_integrity {
};

#endif /* CONFIG_BLK_DEV_INTEGRITY */
struct disk_devt {
atomic_t count;
void (*release)(struct disk_devt *disk_devt);
};

void put_disk_devt(struct disk_devt *disk_devt);
void get_disk_devt(struct disk_devt *disk_devt);

struct gendisk {
/* major, first_minor and minors are input parameters only,
Expand All @@ -183,7 +176,6 @@ struct gendisk {
int first_minor;
int minors; /* maximum number of minors, =1 for
* disks that can't be partitioned. */
struct disk_devt *disk_devt;

char disk_name[DISK_NAME_LEN]; /* name of major driver */
char *(*devnode)(struct gendisk *gd, umode_t *mode);
Expand Down

0 comments on commit c01228d

Please sign in to comment.