* [PATCH 01/10] block: misc updates
2008-07-14 7:12 [PATCHSET 2.6.26] block: implement extended devt, take #2 Tejun Heo
@ 2008-07-14 7:12 ` Tejun Heo
2008-07-14 7:12 ` [PATCH 02/10] block: make variable and argument names more consistent Tejun Heo
` (10 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Tejun Heo @ 2008-07-14 7:12 UTC (permalink / raw)
To: jens.axboe, James.Bottomley, bharrosh, greg.freemyer, linux-scsi,
brking, liml, viro
Cc: Tejun Heo
This patch makes the following misc updates in preparation for
disk->part dereference fix and extended block devt support.
* implment part_to_disk()
* fix comment about gendisk->part indexing
* rename get_part() to disk_map_sector()
* don't use n which is always zero while printing disk information in
diskstats_show()
Signed-off-by: Tejun Heo <tj@kernel.org>
---
block/blk-core.c | 7 ++++---
block/blk-merge.c | 4 ++--
block/genhd.c | 4 ++--
drivers/block/aoe/aoecmd.c | 2 +-
include/linux/genhd.h | 13 ++++++++++---
5 files changed, 19 insertions(+), 11 deletions(-)
diff --git a/block/blk-core.c b/block/blk-core.c
index 1905aab..bc832aa 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -60,7 +60,7 @@ static void drive_stat_acct(struct request *rq, int new_io)
if (!blk_fs_request(rq) || !rq->rq_disk)
return;
- part = get_part(rq->rq_disk, rq->sector);
+ part = disk_map_sector(rq->rq_disk, rq->sector);
if (!new_io)
__all_stat_inc(rq->rq_disk, part, merges[rw], rq->sector);
else {
@@ -1535,7 +1535,8 @@ static int __end_that_request_first(struct request *req, int error,
}
if (blk_fs_request(req) && req->rq_disk) {
- struct hd_struct *part = get_part(req->rq_disk, req->sector);
+ struct hd_struct *part =
+ disk_map_sector(req->rq_disk, req->sector);
const int rw = rq_data_dir(req);
all_stat_add(req->rq_disk, part, sectors[rw],
@@ -1723,7 +1724,7 @@ static void end_that_request_last(struct request *req, int error)
if (disk && blk_fs_request(req) && req != &req->q->bar_rq) {
unsigned long duration = jiffies - req->start_time;
const int rw = rq_data_dir(req);
- struct hd_struct *part = get_part(disk, req->sector);
+ struct hd_struct *part = disk_map_sector(disk, req->sector);
__all_stat_inc(disk, part, ios[rw], req->sector);
__all_stat_add(disk, part, ticks[rw], duration, req->sector);
diff --git a/block/blk-merge.c b/block/blk-merge.c
index 651136a..e96d87d 100644
--- a/block/blk-merge.c
+++ b/block/blk-merge.c
@@ -467,8 +467,8 @@ static int attempt_merge(struct request_queue *q, struct request *req,
elv_merge_requests(q, req, next);
if (req->rq_disk) {
- struct hd_struct *part
- = get_part(req->rq_disk, req->sector);
+ struct hd_struct *part =
+ disk_map_sector(req->rq_disk, req->sector);
disk_round_stats(req->rq_disk);
req->rq_disk->in_flight--;
if (part) {
diff --git a/block/genhd.c b/block/genhd.c
index b646e7e..432c358 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -567,7 +567,7 @@ static int diskstats_show(struct seq_file *s, void *v)
{
struct gendisk *gp = v;
char buf[BDEVNAME_SIZE];
- int n = 0;
+ int n;
/*
if (&gp->dev.kobj.entry == block_class.devices.next)
@@ -581,7 +581,7 @@ static int diskstats_show(struct seq_file *s, void *v)
disk_round_stats(gp);
preempt_enable();
seq_printf(s, "%4d %4d %s %lu %lu %llu %u %lu %lu %llu %u %u %u %u\n",
- gp->major, n + gp->first_minor, disk_name(gp, n, buf),
+ gp->major, gp->first_minor, disk_name(gp, 0, buf),
disk_stat_read(gp, ios[0]), disk_stat_read(gp, merges[0]),
(unsigned long long)disk_stat_read(gp, sectors[0]),
jiffies_to_msecs(disk_stat_read(gp, ticks[0])),
diff --git a/drivers/block/aoe/aoecmd.c b/drivers/block/aoe/aoecmd.c
index 41f818b..ba40473 100644
--- a/drivers/block/aoe/aoecmd.c
+++ b/drivers/block/aoe/aoecmd.c
@@ -757,7 +757,7 @@ diskstats(struct gendisk *disk, struct bio *bio, ulong duration, sector_t sector
const int rw = bio_data_dir(bio);
struct hd_struct *part;
- part = get_part(disk, sector);
+ part = disk_map_sector(disk, sector);
all_stat_inc(disk, part, ios[rw], sector);
all_stat_add(disk, part, ticks[rw], duration, sector);
all_stat_add(disk, part, sectors[rw], n_sect, sector);
diff --git a/include/linux/genhd.h b/include/linux/genhd.h
index e485cf0..68d50a2 100644
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -117,7 +117,7 @@ struct gendisk {
int minors; /* maximum number of minors, =1 for
* disks that can't be partitioned. */
char disk_name[32]; /* name of major driver */
- struct hd_struct **part; /* [indexed by minor] */
+ struct hd_struct **part; /* [indexed by minor - 1] */
struct block_device_operations *fops;
struct request_queue *queue;
void *private_data;
@@ -143,14 +143,21 @@ struct gendisk {
struct work_struct async_notify;
};
+static inline struct gendisk *part_to_disk(struct hd_struct *part)
+{
+ if (likely(part))
+ return dev_to_disk((part)->dev.parent);
+ return NULL;
+}
+
/*
* Macros to operate on percpu disk statistics:
*
* The __ variants should only be called in critical sections. The full
* variants disable/enable preemption.
*/
-static inline struct hd_struct *get_part(struct gendisk *gendiskp,
- sector_t sector)
+static inline struct hd_struct *disk_map_sector(struct gendisk *gendiskp,
+ sector_t sector)
{
struct hd_struct *part;
int i;
--
1.5.4.5
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 02/10] block: make variable and argument names more consistent
2008-07-14 7:12 [PATCHSET 2.6.26] block: implement extended devt, take #2 Tejun Heo
2008-07-14 7:12 ` [PATCH 01/10] block: misc updates Tejun Heo
@ 2008-07-14 7:12 ` Tejun Heo
2008-07-14 7:12 ` [PATCH 03/10] block: don't depend on consecutive minor space Tejun Heo
` (9 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Tejun Heo @ 2008-07-14 7:12 UTC (permalink / raw)
To: jens.axboe, James.Bottomley, bharrosh, greg.freemyer, linux-scsi,
brking, liml, viro
Cc: Tejun Heo
In hd_struct, @partno is used to denote partition number and a number
of other places use @part to denote hd_struct. Functions use @part
and @index instead. This causes confusion and makes it difficult to
use consistent variable names for hd_struct. Always use @partno if a
variable represents partition number.
Also, print out functions use @f or @part for seq_file argument. Use
@seqf instead.
Signed-off-by: Tejun Heo <tj@kernel.org>
---
block/genhd.c | 50 ++++++++++++++++++++++++------------------------
block/ioctl.c | 14 ++++++------
fs/block_dev.c | 8 +++---
fs/partitions/check.c | 32 +++++++++++++++---------------
include/linux/genhd.h | 12 +++++-----
5 files changed, 58 insertions(+), 58 deletions(-)
diff --git a/block/genhd.c b/block/genhd.c
index 432c358..94c4203 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -43,14 +43,14 @@ static inline int major_to_index(int major)
}
#ifdef CONFIG_PROC_FS
-void blkdev_show(struct seq_file *f, off_t offset)
+void blkdev_show(struct seq_file *seqf, off_t offset)
{
struct blk_major_name *dp;
if (offset < BLKDEV_MAJOR_HASH_SIZE) {
mutex_lock(&block_class_lock);
for (dp = major_names[offset]; dp; dp = dp->next)
- seq_printf(f, "%3d %s\n", dp->major, dp->name);
+ seq_printf(seqf, "%3d %s\n", dp->major, dp->name);
mutex_unlock(&block_class_lock);
}
}
@@ -157,7 +157,7 @@ void blk_unregister_region(dev_t devt, unsigned long range)
EXPORT_SYMBOL(blk_unregister_region);
-static struct kobject *exact_match(dev_t devt, int *part, void *data)
+static struct kobject *exact_match(dev_t devt, int *partno, void *data)
{
struct gendisk *p = data;
@@ -214,9 +214,9 @@ void unlink_gendisk(struct gendisk *disk)
* This function gets the structure containing partitioning
* information for the given device @dev.
*/
-struct gendisk *get_gendisk(dev_t devt, int *part)
+struct gendisk *get_gendisk(dev_t devt, int *partno)
{
- struct kobject *kobj = kobj_lookup(bdev_map, devt, part);
+ struct kobject *kobj = kobj_lookup(bdev_map, devt, partno);
struct device *dev = kobj_to_dev(kobj);
return kobj ? dev_to_disk(dev) : NULL;
@@ -280,7 +280,7 @@ void __init printk_all_partitions(void)
#ifdef CONFIG_PROC_FS
/* iterator */
-static void *part_start(struct seq_file *part, loff_t *pos)
+static void *part_start(struct seq_file *seqf, loff_t *pos)
{
loff_t k = *pos;
struct device *dev;
@@ -295,7 +295,7 @@ static void *part_start(struct seq_file *part, loff_t *pos)
return NULL;
}
-static void *part_next(struct seq_file *part, void *v, loff_t *pos)
+static void *part_next(struct seq_file *seqf, void *v, loff_t *pos)
{
struct gendisk *gp = v;
struct device *dev;
@@ -309,19 +309,19 @@ static void *part_next(struct seq_file *part, void *v, loff_t *pos)
return NULL;
}
-static void part_stop(struct seq_file *part, void *v)
+static void part_stop(struct seq_file *seqf, void *v)
{
up(&block_class.sem);
}
-static int show_partition(struct seq_file *part, void *v)
+static int show_partition(struct seq_file *seqf, void *v)
{
struct gendisk *sgp = v;
int n;
char buf[BDEVNAME_SIZE];
if (&sgp->dev.node == block_class.devices.next)
- seq_puts(part, "major minor #blocks name\n\n");
+ seq_puts(seqf, "major minor #blocks name\n\n");
/* Don't show non-partitionable removeable devices or empty devices */
if (!get_capacity(sgp) ||
@@ -331,7 +331,7 @@ static int show_partition(struct seq_file *part, void *v)
return 0;
/* show the full disk and all non-0 size partitions of it */
- seq_printf(part, "%4d %4d %10llu %s\n",
+ seq_printf(seqf, "%4d %4d %10llu %s\n",
sgp->major, sgp->first_minor,
(unsigned long long)get_capacity(sgp) >> 1,
disk_name(sgp, 0, buf));
@@ -340,7 +340,7 @@ static int show_partition(struct seq_file *part, void *v)
continue;
if (sgp->part[n]->nr_sects == 0)
continue;
- seq_printf(part, "%4d %4d %10llu %s\n",
+ seq_printf(seqf, "%4d %4d %10llu %s\n",
sgp->major, n + 1 + sgp->first_minor,
(unsigned long long)sgp->part[n]->nr_sects >> 1 ,
disk_name(sgp, n + 1, buf));
@@ -358,7 +358,7 @@ const struct seq_operations partitions_op = {
#endif
-static struct kobject *base_probe(dev_t devt, int *part, void *data)
+static struct kobject *base_probe(dev_t devt, int *partno, void *data)
{
if (request_module("block-major-%d-%d", MAJOR(devt), MINOR(devt)) > 0)
/* Make old-style 2.4 aliases work */
@@ -528,7 +528,7 @@ static struct device_type disk_type = {
* extra fields.
*/
-static void *diskstats_start(struct seq_file *part, loff_t *pos)
+static void *diskstats_start(struct seq_file *seqf, loff_t *pos)
{
loff_t k = *pos;
struct device *dev;
@@ -543,7 +543,7 @@ static void *diskstats_start(struct seq_file *part, loff_t *pos)
return NULL;
}
-static void *diskstats_next(struct seq_file *part, void *v, loff_t *pos)
+static void *diskstats_next(struct seq_file *seqf, void *v, loff_t *pos)
{
struct gendisk *gp = v;
struct device *dev;
@@ -558,12 +558,12 @@ static void *diskstats_next(struct seq_file *part, void *v, loff_t *pos)
return NULL;
}
-static void diskstats_stop(struct seq_file *part, void *v)
+static void diskstats_stop(struct seq_file *seqf, void *v)
{
up(&block_class.sem);
}
-static int diskstats_show(struct seq_file *s, void *v)
+static int diskstats_show(struct seq_file *seqf, void *v)
{
struct gendisk *gp = v;
char buf[BDEVNAME_SIZE];
@@ -571,7 +571,7 @@ static int diskstats_show(struct seq_file *s, void *v)
/*
if (&gp->dev.kobj.entry == block_class.devices.next)
- seq_puts(s, "major minor name"
+ seq_puts(seqf, "major minor name"
" rio rmerge rsect ruse wio wmerge "
"wsect wuse running use aveq"
"\n\n");
@@ -580,7 +580,7 @@ static int diskstats_show(struct seq_file *s, void *v)
preempt_disable();
disk_round_stats(gp);
preempt_enable();
- seq_printf(s, "%4d %4d %s %lu %lu %llu %u %lu %lu %llu %u %u %u %u\n",
+ seq_printf(seqf, "%4d %4d %s %lu %lu %llu %u %lu %lu %llu %u %u %u %u\n",
gp->major, gp->first_minor, disk_name(gp, 0, buf),
disk_stat_read(gp, ios[0]), disk_stat_read(gp, merges[0]),
(unsigned long long)disk_stat_read(gp, sectors[0]),
@@ -602,7 +602,7 @@ static int diskstats_show(struct seq_file *s, void *v)
preempt_disable();
part_round_stats(hd);
preempt_enable();
- seq_printf(s, "%4d %4d %s %lu %lu %llu "
+ seq_printf(seqf, "%4d %4d %s %lu %lu %llu "
"%u %lu %lu %llu %u %u %u %u\n",
gp->major, n + gp->first_minor + 1,
disk_name(gp, n + 1, buf),
@@ -653,7 +653,7 @@ void genhd_media_change_notify(struct gendisk *disk)
EXPORT_SYMBOL_GPL(genhd_media_change_notify);
#endif /* 0 */
-dev_t blk_lookup_devt(const char *name, int part)
+dev_t blk_lookup_devt(const char *name, int partno)
{
struct device *dev;
dev_t devt = MKDEV(0, 0);
@@ -665,9 +665,9 @@ dev_t blk_lookup_devt(const char *name, int part)
if (strcmp(dev->bus_id, name) == 0) {
struct gendisk *disk = dev_to_disk(dev);
- if (part < disk->minors)
+ if (partno < disk->minors)
devt = MKDEV(MAJOR(dev->devt),
- MINOR(dev->devt) + part);
+ MINOR(dev->devt) + partno);
break;
}
}
@@ -778,10 +778,10 @@ int bdev_read_only(struct block_device *bdev)
EXPORT_SYMBOL(bdev_read_only);
-int invalidate_partition(struct gendisk *disk, int index)
+int invalidate_partition(struct gendisk *disk, int partno)
{
int res = 0;
- struct block_device *bdev = bdget_disk(disk, index);
+ struct block_device *bdev = bdget_disk(disk, partno);
if (bdev) {
fsync_bdev(bdev);
res = __invalidate_device(bdev);
diff --git a/block/ioctl.c b/block/ioctl.c
index ecbdd74..54a468d 100644
--- a/block/ioctl.c
+++ b/block/ioctl.c
@@ -15,7 +15,7 @@ static int blkpg_ioctl(struct block_device *bdev, struct blkpg_ioctl_arg __user
struct blkpg_ioctl_arg a;
struct blkpg_partition p;
long long start, length;
- int part;
+ int partno;
int i, rc;
if (!capable(CAP_SYS_ADMIN))
@@ -27,8 +27,8 @@ static int blkpg_ioctl(struct block_device *bdev, struct blkpg_ioctl_arg __user
disk = bdev->bd_disk;
if (bdev != bdev->bd_contains)
return -EINVAL;
- part = p.pno;
- if (part <= 0 || part >= disk->minors)
+ partno = p.pno;
+ if (partno <= 0 || partno >= disk->minors)
return -EINVAL;
switch (a.op) {
case BLKPG_ADD_PARTITION:
@@ -59,16 +59,16 @@ static int blkpg_ioctl(struct block_device *bdev, struct blkpg_ioctl_arg __user
}
/* all seems OK */
- rc = add_partition(disk, part, start, length,
+ rc = add_partition(disk, partno, start, length,
ADDPART_FLAG_NONE);
mutex_unlock(&bdev->bd_mutex);
return rc;
case BLKPG_DEL_PARTITION:
- if (!disk->part[part-1])
+ if (!disk->part[partno - 1])
return -ENXIO;
- bdevp = bdget_disk(disk, part);
+ bdevp = bdget_disk(disk, partno);
if (!bdevp)
return -ENOMEM;
mutex_lock(&bdevp->bd_mutex);
@@ -82,7 +82,7 @@ static int blkpg_ioctl(struct block_device *bdev, struct blkpg_ioctl_arg __user
invalidate_bdev(bdevp);
mutex_lock_nested(&bdev->bd_mutex, 1);
- delete_partition(disk, part);
+ delete_partition(disk, partno);
mutex_unlock(&bdev->bd_mutex);
mutex_unlock(&bdevp->bd_mutex);
bdput(bdevp);
diff --git a/fs/block_dev.c b/fs/block_dev.c
index 10d8a0a..6451120 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -930,7 +930,7 @@ static int do_open(struct block_device *bdev, struct file *file, int for_part)
struct module *owner = NULL;
struct gendisk *disk;
int ret;
- int part;
+ int partno;
int perm = 0;
if (file->f_mode & FMODE_READ)
@@ -947,7 +947,7 @@ static int do_open(struct block_device *bdev, struct file *file, int for_part)
ret = -ENXIO;
file->f_mapping = bdev->bd_inode->i_mapping;
lock_kernel();
- disk = get_gendisk(bdev->bd_dev, &part);
+ disk = get_gendisk(bdev->bd_dev, &partno);
if (!disk) {
unlock_kernel();
bdput(bdev);
@@ -959,7 +959,7 @@ static int do_open(struct block_device *bdev, struct file *file, int for_part)
if (!bdev->bd_openers) {
bdev->bd_disk = disk;
bdev->bd_contains = bdev;
- if (!part) {
+ if (!partno) {
struct backing_dev_info *bdi;
if (disk->fops->open) {
ret = disk->fops->open(bdev->bd_inode, file);
@@ -987,7 +987,7 @@ static int do_open(struct block_device *bdev, struct file *file, int for_part)
if (ret)
goto out_first;
bdev->bd_contains = whole;
- p = disk->part[part - 1];
+ p = disk->part[partno - 1];
bdev->bd_inode->i_data.backing_dev_info =
whole->bd_inode->i_data.backing_dev_info;
if (!(disk->flags & GENHD_FL_UP) || !p || !p->nr_sects) {
diff --git a/fs/partitions/check.c b/fs/partitions/check.c
index b75ca08..fa27fdb 100644
--- a/fs/partitions/check.c
+++ b/fs/partitions/check.c
@@ -120,22 +120,22 @@ static int (*check_part[])(struct parsed_partitions *, struct block_device *) =
* a pointer to that same buffer (for convenience).
*/
-char *disk_name(struct gendisk *hd, int part, char *buf)
+char *disk_name(struct gendisk *hd, int partno, char *buf)
{
- if (!part)
+ if (!partno)
snprintf(buf, BDEVNAME_SIZE, "%s", hd->disk_name);
else if (isdigit(hd->disk_name[strlen(hd->disk_name)-1]))
- snprintf(buf, BDEVNAME_SIZE, "%sp%d", hd->disk_name, part);
+ snprintf(buf, BDEVNAME_SIZE, "%sp%d", hd->disk_name, partno);
else
- snprintf(buf, BDEVNAME_SIZE, "%s%d", hd->disk_name, part);
+ snprintf(buf, BDEVNAME_SIZE, "%s%d", hd->disk_name, partno);
return buf;
}
const char *bdevname(struct block_device *bdev, char *buf)
{
- int part = MINOR(bdev->bd_dev) - bdev->bd_disk->first_minor;
- return disk_name(bdev->bd_disk, part, buf);
+ int partno = MINOR(bdev->bd_dev) - bdev->bd_disk->first_minor;
+ return disk_name(bdev->bd_disk, partno, buf);
}
EXPORT_SYMBOL(bdevname);
@@ -319,13 +319,13 @@ static inline void disk_sysfs_add_subdirs(struct gendisk *disk)
kobject_put(k);
}
-void delete_partition(struct gendisk *disk, int part)
+void delete_partition(struct gendisk *disk, int partno)
{
- struct hd_struct *p = disk->part[part-1];
+ struct hd_struct *p = disk->part[partno-1];
if (!p)
return;
- disk->part[part-1] = NULL;
+ disk->part[partno-1] = NULL;
p->start_sect = 0;
p->nr_sects = 0;
part_stat_set_all(p, 0);
@@ -342,13 +342,13 @@ static ssize_t whole_disk_show(struct device *dev,
static DEVICE_ATTR(whole_disk, S_IRUSR | S_IRGRP | S_IROTH,
whole_disk_show, NULL);
-int add_partition(struct gendisk *disk, int part,
+int add_partition(struct gendisk *disk, int partno,
sector_t start, sector_t len, int flags)
{
struct hd_struct *p = NULL;
int err;
- if (disk->part[part - 1]) {
+ if (disk->part[partno - 1]) {
err = -EBUSY;
goto fail;
}
@@ -362,22 +362,22 @@ int add_partition(struct gendisk *disk, int part,
p->start_sect = start;
p->nr_sects = len;
- p->partno = part;
+ p->partno = partno;
p->policy = disk->policy;
if (isdigit(disk->dev.bus_id[strlen(disk->dev.bus_id)-1]))
snprintf(p->dev.bus_id, BUS_ID_SIZE,
- "%sp%d", disk->dev.bus_id, part);
+ "%sp%d", disk->dev.bus_id, partno);
else
snprintf(p->dev.bus_id, BUS_ID_SIZE,
- "%s%d", disk->dev.bus_id, part);
+ "%s%d", disk->dev.bus_id, partno);
device_initialize(&p->dev);
- p->dev.devt = MKDEV(disk->major, disk->first_minor + part);
+ p->dev.devt = MKDEV(disk->major, disk->first_minor + partno);
p->dev.class = &block_class;
p->dev.type = &part_type;
p->dev.parent = &disk->dev;
- disk->part[part-1] = p;
+ disk->part[partno-1] = p;
/* delay uevent until 'holders' subdir is created */
p->dev.uevent_suppress = 1;
diff --git a/include/linux/genhd.h b/include/linux/genhd.h
index 68d50a2..f8a54bb 100644
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -363,7 +363,7 @@ extern int get_blkdev_list(char *, int);
extern void add_disk(struct gendisk *disk);
extern void del_gendisk(struct gendisk *gp);
extern void unlink_gendisk(struct gendisk *gp);
-extern struct gendisk *get_gendisk(dev_t dev, int *part);
+extern struct gendisk *get_gendisk(dev_t dev, int *partno);
extern void set_device_ro(struct block_device *bdev, int flag);
extern void set_disk_ro(struct gendisk *disk, int flag);
@@ -532,8 +532,8 @@ struct unixware_disklabel {
#define ADDPART_FLAG_RAID 1
#define ADDPART_FLAG_WHOLEDISK 2
-extern dev_t blk_lookup_devt(const char *name, int part);
-extern char *disk_name (struct gendisk *hd, int part, char *buf);
+extern dev_t blk_lookup_devt(const char *name, int partno);
+extern char *disk_name (struct gendisk *hd, int partno, char *buf);
extern int rescan_partitions(struct gendisk *disk, struct block_device *bdev);
extern int add_partition(struct gendisk *, int, sector_t, sector_t, int);
@@ -551,16 +551,16 @@ extern void blk_register_region(dev_t devt, unsigned long range,
void *data);
extern void blk_unregister_region(dev_t devt, unsigned long range);
-static inline struct block_device *bdget_disk(struct gendisk *disk, int index)
+static inline struct block_device *bdget_disk(struct gendisk *disk, int partno)
{
- return bdget(MKDEV(disk->major, disk->first_minor) + index);
+ return bdget(MKDEV(disk->major, disk->first_minor) + partno);
}
#else /* CONFIG_BLOCK */
static inline void printk_all_partitions(void) { }
-static inline dev_t blk_lookup_devt(const char *name, int part)
+static inline dev_t blk_lookup_devt(const char *name, int partno)
{
dev_t devt = MKDEV(0, 0);
return devt;
--
1.5.4.5
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 03/10] block: don't depend on consecutive minor space
2008-07-14 7:12 [PATCHSET 2.6.26] block: implement extended devt, take #2 Tejun Heo
2008-07-14 7:12 ` [PATCH 01/10] block: misc updates Tejun Heo
2008-07-14 7:12 ` [PATCH 02/10] block: make variable and argument names more consistent Tejun Heo
@ 2008-07-14 7:12 ` Tejun Heo
2008-07-14 7:12 ` [PATCH 04/10] block: fix disk->part[] dereferencing race Tejun Heo
` (8 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Tejun Heo @ 2008-07-14 7:12 UTC (permalink / raw)
To: jens.axboe, James.Bottomley, bharrosh, greg.freemyer, linux-scsi,
brking, liml, viro
Cc: Tejun Heo
* Implement disk_devt() and part_devt() and use them to directly
access devt instead of computing it from ->major and ->first_minor.
Note that all references to ->major and ->first_minor outside of
block layer is used to determine devt of the disk (the part0) and as
->major and ->first_minor will continue to represent devt for the
disk, converting these users aren't strictly necessary. However,
convert them for consistency.
* Implement disk_max_parts() to avoid directly deferencing
genhd->minors.
* Update bdget_disk() such that it doesn't assume consecutive minor
space.
* Move devt computation from register_disk() to add_disk() and make it
the only one (all other usages use the initially determined value).
These changes clean up the code and will help disk->part dereference
fix and extended block device numbers.
Signed-off-by: Tejun Heo <tj@kernel.org>
---
block/genhd.c | 111 ++++++++++++++++++++++++----------
block/ioctl.c | 6 +-
drivers/block/pktcdvd.c | 2 +-
drivers/block/ps3disk.c | 2 +-
drivers/char/random.c | 6 +-
drivers/md/dm-ioctl.c | 4 +-
drivers/md/dm-stripe.c | 4 +-
drivers/md/dm.c | 7 +-
drivers/memstick/core/mspro_block.c | 2 +-
drivers/mmc/card/block.c | 2 +-
drivers/s390/block/dasd_proc.c | 3 +-
drivers/s390/block/dcssblk.c | 4 +-
drivers/scsi/sr.c | 2 +-
fs/block_dev.c | 2 +-
fs/partitions/check.c | 19 ++++---
include/linux/genhd.h | 27 +++++++--
16 files changed, 134 insertions(+), 69 deletions(-)
diff --git a/block/genhd.c b/block/genhd.c
index 94c4203..1212acd 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -185,13 +185,14 @@ void add_disk(struct gendisk *disk)
struct backing_dev_info *bdi;
disk->flags |= GENHD_FL_UP;
- blk_register_region(MKDEV(disk->major, disk->first_minor),
- disk->minors, NULL, exact_match, exact_lock, disk);
+ disk->dev.devt = MKDEV(disk->major, disk->first_minor);
+ blk_register_region(disk_devt(disk), disk->minors, NULL,
+ exact_match, exact_lock, disk);
register_disk(disk);
blk_register_queue(disk);
bdi = &disk->queue->backing_dev_info;
- bdi_register_dev(bdi, MKDEV(disk->major, disk->first_minor));
+ bdi_register_dev(bdi, disk_devt(disk));
sysfs_create_link(&disk->dev.kobj, &bdi->dev->kobj, "bdi");
}
@@ -203,8 +204,7 @@ void unlink_gendisk(struct gendisk *disk)
sysfs_remove_link(&disk->dev.kobj, "bdi");
bdi_unregister(&disk->queue->backing_dev_info);
blk_unregister_queue(disk);
- blk_unregister_region(MKDEV(disk->major, disk->first_minor),
- disk->minors);
+ blk_unregister_region(disk_devt(disk), disk->minors);
}
/**
@@ -222,6 +222,38 @@ struct gendisk *get_gendisk(dev_t devt, int *partno)
return kobj ? dev_to_disk(dev) : NULL;
}
+/**
+ * bdget_disk - do bdget() by gendisk and partition number
+ * @disk: gendisk of interest
+ * @partno: partition number
+ *
+ * Find partition @partno from @disk, do bdget() on it.
+ *
+ * CONTEXT:
+ * Don't care.
+ *
+ * RETURNS:
+ * Resulting block_device on success, NULL on failure.
+ */
+extern struct block_device *bdget_disk(struct gendisk *disk, int partno)
+{
+ dev_t devt = MKDEV(0, 0);
+
+ if (partno == 0)
+ devt = disk_devt(disk);
+ else {
+ struct hd_struct *part = disk->part[partno - 1];
+
+ if (part && part->nr_sects)
+ devt = part_devt(part);
+ }
+
+ if (likely(devt != MKDEV(0, 0)))
+ return bdget(devt);
+ return NULL;
+}
+EXPORT_SYMBOL(bdget_disk);
+
/*
* print a full list of all partitions - intended for places where the root
* filesystem can't be mounted and thus to give the victim some idea of what
@@ -252,7 +284,7 @@ void __init printk_all_partitions(void)
* hex - the same format as the root= option takes.
*/
printk("%02x%02x %10llu %s",
- sgp->major, sgp->first_minor,
+ MAJOR(disk_devt(sgp)), MINOR(disk_devt(sgp)),
(unsigned long long)get_capacity(sgp) >> 1,
disk_name(sgp, 0, buf));
if (sgp->driverfs_dev != NULL &&
@@ -263,15 +295,16 @@ void __init printk_all_partitions(void)
printk(" (driver?)\n");
/* now show the partitions */
- for (n = 0; n < sgp->minors - 1; ++n) {
- if (sgp->part[n] == NULL)
- continue;
- if (sgp->part[n]->nr_sects == 0)
+ for (n = 0; n < disk_max_parts(sgp); ++n) {
+ struct hd_struct *part = sgp->part[n];
+
+ if (!part || !part->nr_sects)
continue;
+
printk(" %02x%02x %10llu %s\n",
- sgp->major, n + 1 + sgp->first_minor,
- (unsigned long long)sgp->part[n]->nr_sects >> 1,
- disk_name(sgp, n + 1, buf));
+ MAJOR(part_devt(part)), MINOR(part_devt(part)),
+ (unsigned long long)part->nr_sects >> 1,
+ disk_name(sgp, part->partno, buf));
}
}
@@ -324,26 +357,27 @@ static int show_partition(struct seq_file *seqf, void *v)
seq_puts(seqf, "major minor #blocks name\n\n");
/* Don't show non-partitionable removeable devices or empty devices */
- if (!get_capacity(sgp) ||
- (sgp->minors == 1 && (sgp->flags & GENHD_FL_REMOVABLE)))
+ if (!get_capacity(sgp) || (!disk_max_parts(sgp) &&
+ (sgp->flags & GENHD_FL_REMOVABLE)))
return 0;
if (sgp->flags & GENHD_FL_SUPPRESS_PARTITION_INFO)
return 0;
/* show the full disk and all non-0 size partitions of it */
seq_printf(seqf, "%4d %4d %10llu %s\n",
- sgp->major, sgp->first_minor,
+ MAJOR(disk_devt(sgp)), MINOR(disk_devt(sgp)),
(unsigned long long)get_capacity(sgp) >> 1,
disk_name(sgp, 0, buf));
- for (n = 0; n < sgp->minors - 1; n++) {
- if (!sgp->part[n])
+ for (n = 0; n < disk_max_parts(sgp); n++) {
+ struct hd_struct *part = sgp->part[n];
+ if (!part)
continue;
- if (sgp->part[n]->nr_sects == 0)
+ if (part->nr_sects == 0)
continue;
seq_printf(seqf, "%4d %4d %10llu %s\n",
- sgp->major, n + 1 + sgp->first_minor,
- (unsigned long long)sgp->part[n]->nr_sects >> 1 ,
- disk_name(sgp, n + 1, buf));
+ MAJOR(part_devt(part)), MINOR(part_devt(part)),
+ (unsigned long long)part->nr_sects >> 1,
+ disk_name(sgp, part->partno, buf));
}
return 0;
@@ -581,7 +615,8 @@ static int diskstats_show(struct seq_file *seqf, void *v)
disk_round_stats(gp);
preempt_enable();
seq_printf(seqf, "%4d %4d %s %lu %lu %llu %u %lu %lu %llu %u %u %u %u\n",
- gp->major, gp->first_minor, disk_name(gp, 0, buf),
+ MAJOR(disk_devt(gp)), MINOR(disk_devt(gp)),
+ disk_name(gp, 0, buf),
disk_stat_read(gp, ios[0]), disk_stat_read(gp, merges[0]),
(unsigned long long)disk_stat_read(gp, sectors[0]),
jiffies_to_msecs(disk_stat_read(gp, ticks[0])),
@@ -593,7 +628,7 @@ static int diskstats_show(struct seq_file *seqf, void *v)
jiffies_to_msecs(disk_stat_read(gp, time_in_queue)));
/* now show all non-0 size partitions of it */
- for (n = 0; n < gp->minors - 1; n++) {
+ for (n = 0; n < disk_max_parts(gp); n++) {
struct hd_struct *hd = gp->part[n];
if (!hd || !hd->nr_sects)
@@ -604,8 +639,8 @@ static int diskstats_show(struct seq_file *seqf, void *v)
preempt_enable();
seq_printf(seqf, "%4d %4d %s %lu %lu %llu "
"%u %lu %lu %llu %u %u %u %u\n",
- gp->major, n + gp->first_minor + 1,
- disk_name(gp, n + 1, buf),
+ MAJOR(part_devt(hd)), MINOR(part_devt(hd)),
+ disk_name(gp, hd->partno, buf),
part_stat_read(hd, ios[0]),
part_stat_read(hd, merges[0]),
(unsigned long long)part_stat_read(hd, sectors[0]),
@@ -660,16 +695,26 @@ dev_t blk_lookup_devt(const char *name, int partno)
down(&block_class.sem);
list_for_each_entry(dev, &block_class.devices, node) {
+ struct gendisk *disk = dev_to_disk(dev);
+
if (dev->type != &disk_type)
continue;
- if (strcmp(dev->bus_id, name) == 0) {
- struct gendisk *disk = dev_to_disk(dev);
+ if (strcmp(dev->bus_id, name))
+ continue;
+ if (partno < 0 || partno > disk_max_parts(disk))
+ continue;
- if (partno < disk->minors)
- devt = MKDEV(MAJOR(dev->devt),
- MINOR(dev->devt) + partno);
- break;
+ if (partno == 0)
+ devt = disk_devt(disk);
+ else {
+ struct hd_struct *part = disk->part[partno - 1];
+
+ if (!part || !part->nr_sects)
+ continue;
+
+ devt = part_devt(part);
}
+ break;
}
up(&block_class.sem);
@@ -760,7 +805,7 @@ void set_disk_ro(struct gendisk *disk, int flag)
{
int i;
disk->policy = flag;
- for (i = 0; i < disk->minors - 1; i++)
+ for (i = 0; i < disk_max_parts(disk); i++)
if (disk->part[i]) disk->part[i]->policy = flag;
}
diff --git a/block/ioctl.c b/block/ioctl.c
index 54a468d..e922895 100644
--- a/block/ioctl.c
+++ b/block/ioctl.c
@@ -28,7 +28,7 @@ static int blkpg_ioctl(struct block_device *bdev, struct blkpg_ioctl_arg __user
if (bdev != bdev->bd_contains)
return -EINVAL;
partno = p.pno;
- if (partno <= 0 || partno >= disk->minors)
+ if (partno <= 0 || partno > disk_max_parts(disk))
return -EINVAL;
switch (a.op) {
case BLKPG_ADD_PARTITION:
@@ -46,7 +46,7 @@ static int blkpg_ioctl(struct block_device *bdev, struct blkpg_ioctl_arg __user
mutex_lock(&bdev->bd_mutex);
/* overlap? */
- for (i = 0; i < disk->minors - 1; i++) {
+ for (i = 0; i < disk_max_parts(disk); i++) {
struct hd_struct *s = disk->part[i];
if (!s)
@@ -98,7 +98,7 @@ static int blkdev_reread_part(struct block_device *bdev)
struct gendisk *disk = bdev->bd_disk;
int res;
- if (disk->minors == 1 || bdev != bdev->bd_contains)
+ if (!disk_max_parts(disk) || bdev != bdev->bd_contains)
return -EINVAL;
if (!capable(CAP_SYS_ADMIN))
return -EACCES;
diff --git a/drivers/block/pktcdvd.c b/drivers/block/pktcdvd.c
index 3ba1df9..9eb9a71 100644
--- a/drivers/block/pktcdvd.c
+++ b/drivers/block/pktcdvd.c
@@ -2910,7 +2910,7 @@ static int pkt_setup_dev(dev_t dev, dev_t* pkt_dev)
if (!disk->queue)
goto out_mem2;
- pd->pkt_dev = MKDEV(disk->major, disk->first_minor);
+ pd->pkt_dev = MKDEV(pktdev_major, idx);
ret = pkt_new_dev(pd, dev);
if (ret)
goto out_new_dev;
diff --git a/drivers/block/ps3disk.c b/drivers/block/ps3disk.c
index d797e20..733504d 100644
--- a/drivers/block/ps3disk.c
+++ b/drivers/block/ps3disk.c
@@ -538,7 +538,7 @@ static int ps3disk_remove(struct ps3_system_bus_device *_dev)
struct ps3disk_private *priv = dev->sbd.core.driver_data;
mutex_lock(&ps3disk_mask_mutex);
- __clear_bit(priv->gendisk->first_minor / PS3DISK_MINORS,
+ __clear_bit(MINOR(disk_devt(priv->gendisk)) / PS3DISK_MINORS,
&ps3disk_mask);
mutex_unlock(&ps3disk_mask_mutex);
del_gendisk(priv->gendisk);
diff --git a/drivers/char/random.c b/drivers/char/random.c
index 0cf98bd..0565c8d 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -659,10 +659,10 @@ void add_disk_randomness(struct gendisk *disk)
if (!disk || !disk->random)
return;
/* first major is 1, so we get >= 0x200 here */
- DEBUG_ENT("disk event %d:%d\n", disk->major, disk->first_minor);
+ DEBUG_ENT("disk event %d:%d\n",
+ MAJOR(disk_devt(disk)), MINOR(disk_devt(disk)));
- add_timer_randomness(disk->random,
- 0x100 + MKDEV(disk->major, disk->first_minor));
+ add_timer_randomness(disk->random, 0x100 + disk_devt(disk));
}
#endif
diff --git a/drivers/md/dm-ioctl.c b/drivers/md/dm-ioctl.c
index b262c00..c3de311 100644
--- a/drivers/md/dm-ioctl.c
+++ b/drivers/md/dm-ioctl.c
@@ -426,7 +426,7 @@ static int list_devices(struct dm_ioctl *param, size_t param_size)
old_nl->next = (uint32_t) ((void *) nl -
(void *) old_nl);
disk = dm_disk(hc->md);
- nl->dev = huge_encode_dev(MKDEV(disk->major, disk->first_minor));
+ nl->dev = huge_encode_dev(disk_devt(disk));
nl->next = 0;
strcpy(nl->name, hc->name);
@@ -539,7 +539,7 @@ static int __dev_status(struct mapped_device *md, struct dm_ioctl *param)
if (dm_suspended(md))
param->flags |= DM_SUSPEND_FLAG;
- param->dev = huge_encode_dev(MKDEV(disk->major, disk->first_minor));
+ param->dev = huge_encode_dev(disk_devt(disk));
/*
* Yes, this will be out of date by the time it gets back
diff --git a/drivers/md/dm-stripe.c b/drivers/md/dm-stripe.c
index 4de90ab..b745d8a 100644
--- a/drivers/md/dm-stripe.c
+++ b/drivers/md/dm-stripe.c
@@ -284,8 +284,8 @@ static int stripe_end_io(struct dm_target *ti, struct bio *bio,
memset(major_minor, 0, sizeof(major_minor));
sprintf(major_minor, "%d:%d",
- bio->bi_bdev->bd_disk->major,
- bio->bi_bdev->bd_disk->first_minor);
+ MAJOR(disk_devt(bio->bi_bdev->bd_disk)),
+ MINOR(disk_devt(bio->bi_bdev->bd_disk)));
/*
* Test to see which stripe drive triggered the event
diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index 372369b..923fea4 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -1098,7 +1098,7 @@ static void unlock_fs(struct mapped_device *md);
static void free_dev(struct mapped_device *md)
{
- int minor = md->disk->first_minor;
+ int minor = MINOR(disk_devt(md->disk));
if (md->suspended_bdev) {
unlock_fs(md);
@@ -1219,7 +1219,7 @@ static struct mapped_device *dm_find_md(dev_t dev)
md = idr_find(&_minor_idr, minor);
if (md && (md == MINOR_ALLOCED ||
- (dm_disk(md)->first_minor != minor) ||
+ (MINOR(disk_devt(dm_disk(md))) != minor) ||
test_bit(DMF_FREEING, &md->flags))) {
md = NULL;
goto out;
@@ -1270,7 +1270,8 @@ void dm_put(struct mapped_device *md)
if (atomic_dec_and_lock(&md->holders, &_minor_lock)) {
map = dm_get_table(md);
- idr_replace(&_minor_idr, MINOR_ALLOCED, dm_disk(md)->first_minor);
+ idr_replace(&_minor_idr, MINOR_ALLOCED,
+ MINOR(disk_devt(dm_disk(md))));
set_bit(DMF_FREEING, &md->flags);
spin_unlock(&_minor_lock);
if (!dm_suspended(md)) {
diff --git a/drivers/memstick/core/mspro_block.c b/drivers/memstick/core/mspro_block.c
index 477d0fb..31309ae 100644
--- a/drivers/memstick/core/mspro_block.c
+++ b/drivers/memstick/core/mspro_block.c
@@ -193,7 +193,7 @@ static int mspro_block_bd_open(struct inode *inode, struct file *filp)
static int mspro_block_disk_release(struct gendisk *disk)
{
struct mspro_block_data *msb = disk->private_data;
- int disk_id = disk->first_minor >> MEMSTICK_PART_SHIFT;
+ int disk_id = MINOR(disk_devt(disk)) >> MEMSTICK_PART_SHIFT;
mutex_lock(&mspro_block_disk_lock);
diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
index f9ad960..b1484c0 100644
--- a/drivers/mmc/card/block.c
+++ b/drivers/mmc/card/block.c
@@ -83,7 +83,7 @@ static void mmc_blk_put(struct mmc_blk_data *md)
mutex_lock(&open_lock);
md->usage--;
if (md->usage == 0) {
- int devidx = md->disk->first_minor >> MMC_SHIFT;
+ int devidx = MINOR(disk_devt(md->disk)) >> MMC_SHIFT;
__clear_bit(devidx, dev_use);
put_disk(md->disk);
diff --git a/drivers/s390/block/dasd_proc.c b/drivers/s390/block/dasd_proc.c
index 03c0e40..e3b5c4d 100644
--- a/drivers/s390/block/dasd_proc.c
+++ b/drivers/s390/block/dasd_proc.c
@@ -76,7 +76,8 @@ dasd_devices_show(struct seq_file *m, void *v)
/* Print kdev. */
if (block->gdp)
seq_printf(m, " at (%3d:%6d)",
- block->gdp->major, block->gdp->first_minor);
+ MAJOR(disk_devt(block->gdp)),
+ MINOR(disk_devt(block->gdp)));
else
seq_printf(m, " at (???:??????)");
/* Print device name. */
diff --git a/drivers/s390/block/dcssblk.c b/drivers/s390/block/dcssblk.c
index bb52d2f..7d8222a 100644
--- a/drivers/s390/block/dcssblk.c
+++ b/drivers/s390/block/dcssblk.c
@@ -114,7 +114,7 @@ dcssblk_assign_free_minor(struct dcssblk_dev_info *dev_info)
found = 0;
// test if minor available
list_for_each_entry(entry, &dcssblk_devices, lh)
- if (minor == entry->gd->first_minor)
+ if (minor == MINOR(disk_devt(entry->gd)))
found++;
if (!found) break; // got unused minor
}
@@ -398,7 +398,7 @@ dcssblk_add_store(struct device *dev, struct device_attribute *attr, const char
goto unload_seg;
}
sprintf(dev_info->gd->disk_name, "dcssblk%d",
- dev_info->gd->first_minor);
+ MINOR(disk_devt(dev_info->gd)));
list_add_tail(&dev_info->lh, &dcssblk_devices);
if (!try_module_get(THIS_MODULE)) {
diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
index c82df8b..6580d22 100644
--- a/drivers/scsi/sr.c
+++ b/drivers/scsi/sr.c
@@ -888,7 +888,7 @@ static void sr_kref_release(struct kref *kref)
struct gendisk *disk = cd->disk;
spin_lock(&sr_index_lock);
- clear_bit(disk->first_minor, sr_index_bits);
+ clear_bit(MINOR(disk_devt(disk)), sr_index_bits);
spin_unlock(&sr_index_lock);
unregister_cdrom(&cd->cdi);
diff --git a/fs/block_dev.c b/fs/block_dev.c
index 6451120..7d00b22 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -892,7 +892,7 @@ int check_disk_change(struct block_device *bdev)
if (bdops->revalidate_disk)
bdops->revalidate_disk(bdev->bd_disk);
- if (bdev->bd_disk->minors > 1)
+ if (disk_max_parts(bdev->bd_disk))
bdev->bd_invalidated = 1;
return 1;
}
diff --git a/fs/partitions/check.c b/fs/partitions/check.c
index fa27fdb..32024f7 100644
--- a/fs/partitions/check.c
+++ b/fs/partitions/check.c
@@ -134,7 +134,11 @@ char *disk_name(struct gendisk *hd, int partno, char *buf)
const char *bdevname(struct block_device *bdev, char *buf)
{
- int partno = MINOR(bdev->bd_dev) - bdev->bd_disk->first_minor;
+ int partno = 0;
+
+ if (bdev->bd_part)
+ partno = bdev->bd_part->partno;
+
return disk_name(bdev->bd_disk, partno, buf);
}
@@ -169,7 +173,7 @@ check_partition(struct gendisk *hd, struct block_device *bdev)
if (isdigit(state->name[strlen(state->name)-1]))
sprintf(state->name, "p");
- state->limit = hd->minors;
+ state->limit = disk_max_parts(hd) + 1;
i = res = err = 0;
while (!res && check_part[i]) {
memset(&state->parts, 0, sizeof(state->parts));
@@ -416,7 +420,6 @@ void register_disk(struct gendisk *disk)
int err;
disk->dev.parent = disk->driverfs_dev;
- disk->dev.devt = MKDEV(disk->major, disk->first_minor);
strlcpy(disk->dev.bus_id, disk->disk_name, KOBJ_NAME_LEN);
/* ewww... some of these buggers have / in the name... */
@@ -440,7 +443,7 @@ void register_disk(struct gendisk *disk)
disk_sysfs_add_subdirs(disk);
/* No minors to use for partitions */
- if (disk->minors == 1)
+ if (!disk_max_parts(disk))
goto exit;
/* No such device (e.g., media were just removed) */
@@ -463,8 +466,8 @@ exit:
kobject_uevent(&disk->dev.kobj, KOBJ_ADD);
/* announce possible partitions */
- for (i = 1; i < disk->minors; i++) {
- p = disk->part[i-1];
+ for (i = 0; i < disk_max_parts(disk); i++) {
+ p = disk->part[i];
if (!p || !p->nr_sects)
continue;
kobject_uevent(&p->dev.kobj, KOBJ_ADD);
@@ -482,7 +485,7 @@ int rescan_partitions(struct gendisk *disk, struct block_device *bdev)
if (res)
return res;
bdev->bd_invalidated = 0;
- for (p = 1; p < disk->minors; p++)
+ for (p = 1; p <= disk_max_parts(disk); p++)
delete_partition(disk, p);
if (disk->fops->revalidate_disk)
disk->fops->revalidate_disk(disk);
@@ -544,7 +547,7 @@ void del_gendisk(struct gendisk *disk)
int p;
/* invalidate stuff */
- for (p = disk->minors - 1; p > 0; p--) {
+ for (p = disk_max_parts(disk); p > 0; p--) {
invalidate_partition(disk, p);
delete_partition(disk, p);
}
diff --git a/include/linux/genhd.h b/include/linux/genhd.h
index f8a54bb..0b102c1 100644
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -112,10 +112,14 @@ struct hd_struct {
struct gendisk {
+ /* major, first_minor and minors are input parameters only,
+ * don't use directly. Use disk_devt() and disk_max_parts().
+ */
int major; /* major number of driver */
int first_minor;
int minors; /* maximum number of minors, =1 for
* disks that can't be partitioned. */
+
char disk_name[32]; /* name of major driver */
struct hd_struct **part; /* [indexed by minor - 1] */
struct block_device_operations *fops;
@@ -150,6 +154,21 @@ static inline struct gendisk *part_to_disk(struct hd_struct *part)
return NULL;
}
+static inline int disk_max_parts(struct gendisk *disk)
+{
+ return disk->minors - 1;
+}
+
+static inline dev_t disk_devt(struct gendisk *disk)
+{
+ return disk->dev.devt;
+}
+
+static inline dev_t part_devt(struct hd_struct *part)
+{
+ return part->dev.devt;
+}
+
/*
* Macros to operate on percpu disk statistics:
*
@@ -161,7 +180,7 @@ static inline struct hd_struct *disk_map_sector(struct gendisk *gendiskp,
{
struct hd_struct *part;
int i;
- for (i = 0; i < gendiskp->minors - 1; i++) {
+ for (i = 0; i < disk_max_parts(gendiskp); i++) {
part = gendiskp->part[i];
if (part && part->start_sect <= sector
&& sector < part->start_sect + part->nr_sects)
@@ -364,6 +383,7 @@ extern void add_disk(struct gendisk *disk);
extern void del_gendisk(struct gendisk *gp);
extern void unlink_gendisk(struct gendisk *gp);
extern struct gendisk *get_gendisk(dev_t dev, int *partno);
+extern struct block_device *bdget_disk(struct gendisk *disk, int partno);
extern void set_device_ro(struct block_device *bdev, int flag);
extern void set_disk_ro(struct gendisk *disk, int flag);
@@ -551,11 +571,6 @@ extern void blk_register_region(dev_t devt, unsigned long range,
void *data);
extern void blk_unregister_region(dev_t devt, unsigned long range);
-static inline struct block_device *bdget_disk(struct gendisk *disk, int partno)
-{
- return bdget(MKDEV(disk->major, disk->first_minor) + partno);
-}
-
#else /* CONFIG_BLOCK */
static inline void printk_all_partitions(void) { }
--
1.5.4.5
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 04/10] block: fix disk->part[] dereferencing race
2008-07-14 7:12 [PATCHSET 2.6.26] block: implement extended devt, take #2 Tejun Heo
` (2 preceding siblings ...)
2008-07-14 7:12 ` [PATCH 03/10] block: don't depend on consecutive minor space Tejun Heo
@ 2008-07-14 7:12 ` Tejun Heo
2008-07-14 7:12 ` [PATCH 05/10] block: always use __{disk|part|all}_stat_*() and kill non-underbarred versions Tejun Heo
` (7 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Tejun Heo @ 2008-07-14 7:12 UTC (permalink / raw)
To: jens.axboe, James.Bottomley, bharrosh, greg.freemyer, linux-scsi,
brking, liml, viro
Cc: Tejun Heo
disk->part[] is protected by its matching bdev's lock. However,
non-critical accesses like collecting stats and printing out sysfs and
proc information used to be performed without any locking. As
partitions can come and go dynamically, partitions can go away
underneath those non-critical accesses. As some of those accesses are
writes, this theoretically can lead to silent corruption.
This patch fixes the race by using RCU for the partition array and dev
reference counter to hold partitions.
* Rename disk->part[] to disk->__part[] to make sure no one outside
genhd layer proper accesses it directly.
* Use RCU for disk->__part[] dereferencing.
* Implement disk_{get|put}_part() which can be used to get and put
partitions from gendisk respectively.
* Iterators are implemented to help iterate through all partitions
safely.
* Functions which require RCU readlock are marked with _rcu suffix.
* Use disk_put_part() in __blkdev_put() instead of directly putting
the contained kobject.
Signed-off-by: Tejun Heo <tj@kernel.org>
---
block/blk-core.c | 20 +++-
block/blk-merge.c | 9 ++-
block/genhd.c | 219 +++++++++++++++++++++++++++++++++++++-------
block/ioctl.c | 26 +++--
drivers/block/aoe/aoecmd.c | 6 +-
fs/block_dev.c | 15 ++--
fs/partitions/check.c | 69 +++++++++-----
include/linux/genhd.h | 53 ++++++++---
8 files changed, 322 insertions(+), 95 deletions(-)
diff --git a/block/blk-core.c b/block/blk-core.c
index bc832aa..84292e1 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -60,7 +60,9 @@ static void drive_stat_acct(struct request *rq, int new_io)
if (!blk_fs_request(rq) || !rq->rq_disk)
return;
- part = disk_map_sector(rq->rq_disk, rq->sector);
+ rcu_read_lock();
+
+ part = disk_map_sector_rcu(rq->rq_disk, rq->sector);
if (!new_io)
__all_stat_inc(rq->rq_disk, part, merges[rw], rq->sector);
else {
@@ -71,6 +73,8 @@ static void drive_stat_acct(struct request *rq, int new_io)
part->in_flight++;
}
}
+
+ rcu_read_unlock();
}
void blk_queue_congestion_threshold(struct request_queue *q)
@@ -1535,12 +1539,14 @@ static int __end_that_request_first(struct request *req, int error,
}
if (blk_fs_request(req) && req->rq_disk) {
- struct hd_struct *part =
- disk_map_sector(req->rq_disk, req->sector);
const int rw = rq_data_dir(req);
+ struct hd_struct *part;
+ rcu_read_lock();
+ part = disk_map_sector_rcu(req->rq_disk, req->sector);
all_stat_add(req->rq_disk, part, sectors[rw],
nr_bytes >> 9, req->sector);
+ rcu_read_unlock();
}
total_bytes = bio_nbytes = 0;
@@ -1724,7 +1730,11 @@ static void end_that_request_last(struct request *req, int error)
if (disk && blk_fs_request(req) && req != &req->q->bar_rq) {
unsigned long duration = jiffies - req->start_time;
const int rw = rq_data_dir(req);
- struct hd_struct *part = disk_map_sector(disk, req->sector);
+ struct hd_struct *part;
+
+ rcu_read_lock();
+
+ part = disk_map_sector_rcu(disk, req->sector);
__all_stat_inc(disk, part, ios[rw], req->sector);
__all_stat_add(disk, part, ticks[rw], duration, req->sector);
@@ -1734,6 +1744,8 @@ static void end_that_request_last(struct request *req, int error)
part_round_stats(part);
part->in_flight--;
}
+
+ rcu_read_unlock();
}
if (req->end_io)
diff --git a/block/blk-merge.c b/block/blk-merge.c
index e96d87d..9be9b2f 100644
--- a/block/blk-merge.c
+++ b/block/blk-merge.c
@@ -467,14 +467,19 @@ static int attempt_merge(struct request_queue *q, struct request *req,
elv_merge_requests(q, req, next);
if (req->rq_disk) {
- struct hd_struct *part =
- disk_map_sector(req->rq_disk, req->sector);
+ struct hd_struct *part;
+
+ rcu_read_lock();
+
+ part = disk_map_sector_rcu(req->rq_disk, req->sector);
disk_round_stats(req->rq_disk);
req->rq_disk->in_flight--;
if (part) {
part_round_stats(part);
part->in_flight--;
}
+
+ rcu_read_unlock();
}
req->ioprio = ioprio_best(req->ioprio, next->ioprio);
diff --git a/block/genhd.c b/block/genhd.c
index 1212acd..76961be 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -26,6 +26,158 @@ struct kobject *block_depr;
static struct device_type disk_type;
+/**
+ * disk_get_part - get partition
+ * @disk: disk to look partition from
+ * @partno: partition number
+ *
+ * Look for partition @partno from @disk. If found, increment
+ * reference count and return it.
+ *
+ * CONTEXT:
+ * Don't care.
+ *
+ * RETURNS:
+ * Pointer to the found partition on success, NULL if not found.
+ */
+struct hd_struct *disk_get_part(struct gendisk *disk, int partno)
+{
+ struct hd_struct *part;
+
+ if (unlikely(partno < 1 || partno > disk_max_parts(disk)))
+ return NULL;
+ rcu_read_lock();
+ part = rcu_dereference(disk->__part[partno - 1]);
+ if (part)
+ get_device(&part->dev);
+ rcu_read_unlock();
+
+ return part;
+}
+EXPORT_SYMBOL_GPL(disk_get_part);
+
+/**
+ * disk_part_iter_start - initialize partition iterator
+ * @piter: iterator to initialize
+ * @disk: disk to iterate over
+ * @flags: DISK_PITER_* flags
+ *
+ * Initialize @piter so that it iterates over partitions of @disk.
+ *
+ * CONTEXT:
+ * Don't care.
+ */
+void disk_part_iter_start(struct disk_part_iter *piter, struct gendisk *disk,
+ unsigned int flags)
+{
+ piter->disk = disk;
+ piter->part = NULL;
+
+ if (flags & DISK_PITER_REVERSE)
+ piter->idx = disk_max_parts(piter->disk) - 1;
+ else
+ piter->idx = 0;
+
+ piter->flags = flags;
+}
+EXPORT_SYMBOL_GPL(disk_part_iter_start);
+
+/**
+ * disk_part_iter_next - proceed iterator to the next partition and return it
+ * @piter: iterator of interest
+ *
+ * Proceed @piter to the next partition and return it.
+ *
+ * CONTEXT:
+ * Don't care.
+ */
+struct hd_struct *disk_part_iter_next(struct disk_part_iter *piter)
+{
+ int inc, end;
+
+ /* put the last partition */
+ disk_put_part(piter->part);
+ piter->part = NULL;
+
+ rcu_read_lock();
+
+ /* determine iteration parameters */
+ if (piter->flags & DISK_PITER_REVERSE) {
+ inc = -1;
+ end = -1;
+ } else {
+ inc = 1;
+ end = disk_max_parts(piter->disk);
+ }
+
+ /* iterate to the next partition */
+ for (; piter->idx != end; piter->idx += inc) {
+ struct hd_struct *part;
+
+ part = rcu_dereference(piter->disk->__part[piter->idx]);
+ if (!part)
+ continue;
+ if (!(piter->flags & DISK_PITER_INCL_EMPTY) && !part->nr_sects)
+ continue;
+
+ get_device(&part->dev);
+ piter->part = part;
+ piter->idx++;
+ break;
+ }
+
+ rcu_read_unlock();
+
+ return piter->part;
+}
+EXPORT_SYMBOL_GPL(disk_part_iter_next);
+
+/**
+ * disk_part_iter_stop - finish up partition iteration
+ * @piter: iter of interest
+ *
+ * Called when iteration is over. Cleans up @piter.
+ *
+ * CONTEXT:
+ * Don't care.
+ */
+void disk_part_iter_stop(struct disk_part_iter *piter)
+{
+ disk_put_part(piter->part);
+ piter->part = NULL;
+}
+EXPORT_SYMBOL_GPL(disk_part_iter_stop);
+
+/**
+ * disk_map_sector_rcu - map sector to partition
+ * @disk: gendisk of interest
+ * @sector: sector to map
+ *
+ * Find out which partition @sector maps to on @disk. This is
+ * primarily used for stats accounting.
+ *
+ * CONTEXT:
+ * RCU read locked. The returned partition pointer is valid only
+ * while preemption is disabled.
+ *
+ * RETURNS:
+ * Found partition on success, NULL if there's no matching partition.
+ */
+struct hd_struct *disk_map_sector_rcu(struct gendisk *disk, sector_t sector)
+{
+ int i;
+
+ for (i = 0; i < disk_max_parts(disk); i++) {
+ struct hd_struct *part = rcu_dereference(disk->__part[i]);
+
+ if (part && part->start_sect <= sector &&
+ sector < part->start_sect + part->nr_sects)
+ return part;
+ }
+ return NULL;
+}
+EXPORT_SYMBOL_GPL(disk_map_sector_rcu);
+
/*
* Can be deleted altogether. Later.
*
@@ -242,10 +394,12 @@ extern struct block_device *bdget_disk(struct gendisk *disk, int partno)
if (partno == 0)
devt = disk_devt(disk);
else {
- struct hd_struct *part = disk->part[partno - 1];
+ struct hd_struct *part;
+ part = disk_get_part(disk, partno);
if (part && part->nr_sects)
devt = part_devt(part);
+ disk_put_part(part);
}
if (likely(devt != MKDEV(0, 0)))
@@ -263,8 +417,9 @@ void __init printk_all_partitions(void)
{
struct device *dev;
struct gendisk *sgp;
+ struct disk_part_iter piter;
+ struct hd_struct *part;
char buf[BDEVNAME_SIZE];
- int n;
down(&block_class.sem);
/* For each block device... */
@@ -295,17 +450,13 @@ void __init printk_all_partitions(void)
printk(" (driver?)\n");
/* now show the partitions */
- for (n = 0; n < disk_max_parts(sgp); ++n) {
- struct hd_struct *part = sgp->part[n];
-
- if (!part || !part->nr_sects)
- continue;
-
+ disk_part_iter_start(&piter, sgp, 0);
+ while ((part = disk_part_iter_next(&piter)))
printk(" %02x%02x %10llu %s\n",
MAJOR(part_devt(part)), MINOR(part_devt(part)),
(unsigned long long)part->nr_sects >> 1,
disk_name(sgp, part->partno, buf));
- }
+ disk_part_iter_stop(&piter);
}
up(&block_class.sem);
@@ -350,7 +501,8 @@ static void part_stop(struct seq_file *seqf, void *v)
static int show_partition(struct seq_file *seqf, void *v)
{
struct gendisk *sgp = v;
- int n;
+ struct disk_part_iter piter;
+ struct hd_struct *part;
char buf[BDEVNAME_SIZE];
if (&sgp->dev.node == block_class.devices.next)
@@ -368,17 +520,14 @@ static int show_partition(struct seq_file *seqf, void *v)
MAJOR(disk_devt(sgp)), MINOR(disk_devt(sgp)),
(unsigned long long)get_capacity(sgp) >> 1,
disk_name(sgp, 0, buf));
- for (n = 0; n < disk_max_parts(sgp); n++) {
- struct hd_struct *part = sgp->part[n];
- if (!part)
- continue;
- if (part->nr_sects == 0)
- continue;
+
+ disk_part_iter_start(&piter, sgp, 0);
+ while ((part = disk_part_iter_next(&piter)))
seq_printf(seqf, "%4d %4d %10llu %s\n",
MAJOR(part_devt(part)), MINOR(part_devt(part)),
(unsigned long long)part->nr_sects >> 1,
disk_name(sgp, part->partno, buf));
- }
+ disk_part_iter_stop(&piter);
return 0;
}
@@ -540,7 +689,7 @@ static void disk_release(struct device *dev)
struct gendisk *disk = dev_to_disk(dev);
kfree(disk->random);
- kfree(disk->part);
+ kfree(disk->__part);
free_disk_stats(disk);
kfree(disk);
}
@@ -600,8 +749,9 @@ static void diskstats_stop(struct seq_file *seqf, void *v)
static int diskstats_show(struct seq_file *seqf, void *v)
{
struct gendisk *gp = v;
+ struct disk_part_iter piter;
+ struct hd_struct *hd;
char buf[BDEVNAME_SIZE];
- int n;
/*
if (&gp->dev.kobj.entry == block_class.devices.next)
@@ -628,12 +778,8 @@ static int diskstats_show(struct seq_file *seqf, void *v)
jiffies_to_msecs(disk_stat_read(gp, time_in_queue)));
/* now show all non-0 size partitions of it */
- for (n = 0; n < disk_max_parts(gp); n++) {
- struct hd_struct *hd = gp->part[n];
-
- if (!hd || !hd->nr_sects)
- continue;
-
+ disk_part_iter_start(&piter, gp, 0);
+ while ((hd = disk_part_iter_next(&piter))) {
preempt_disable();
part_round_stats(hd);
preempt_enable();
@@ -654,6 +800,7 @@ static int diskstats_show(struct seq_file *seqf, void *v)
jiffies_to_msecs(part_stat_read(hd, time_in_queue))
);
}
+ disk_part_iter_stop(&piter);
return 0;
}
@@ -707,12 +854,16 @@ dev_t blk_lookup_devt(const char *name, int partno)
if (partno == 0)
devt = disk_devt(disk);
else {
- struct hd_struct *part = disk->part[partno - 1];
+ struct hd_struct *part;
- if (!part || !part->nr_sects)
+ part = disk_get_part(disk, partno);
+ if (!part || !part->nr_sects) {
+ disk_put_part(part);
continue;
+ }
devt = part_devt(part);
+ disk_put_part(part);
}
break;
}
@@ -740,9 +891,9 @@ struct gendisk *alloc_disk_node(int minors, int node_id)
}
if (minors > 1) {
int size = (minors - 1) * sizeof(struct hd_struct *);
- disk->part = kmalloc_node(size,
+ disk->__part = kmalloc_node(size,
GFP_KERNEL | __GFP_ZERO, node_id);
- if (!disk->part) {
+ if (!disk->__part) {
free_disk_stats(disk);
kfree(disk);
return NULL;
@@ -803,10 +954,14 @@ EXPORT_SYMBOL(set_device_ro);
void set_disk_ro(struct gendisk *disk, int flag)
{
- int i;
+ struct disk_part_iter piter;
+ struct hd_struct *part;
+
disk->policy = flag;
- for (i = 0; i < disk_max_parts(disk); i++)
- if (disk->part[i]) disk->part[i]->policy = flag;
+ disk_part_iter_start(&piter, disk, DISK_PITER_INCL_EMPTY);
+ while ((part = disk_part_iter_next(&piter)))
+ part->policy = flag;
+ disk_part_iter_stop(&piter);
}
EXPORT_SYMBOL(set_disk_ro);
diff --git a/block/ioctl.c b/block/ioctl.c
index e922895..75aeead 100644
--- a/block/ioctl.c
+++ b/block/ioctl.c
@@ -12,11 +12,13 @@ static int blkpg_ioctl(struct block_device *bdev, struct blkpg_ioctl_arg __user
{
struct block_device *bdevp;
struct gendisk *disk;
+ struct hd_struct *part;
struct blkpg_ioctl_arg a;
struct blkpg_partition p;
+ struct disk_part_iter piter;
long long start, length;
int partno;
- int i, rc;
+ int rc;
if (!capable(CAP_SYS_ADMIN))
return -EACCES;
@@ -46,17 +48,17 @@ static int blkpg_ioctl(struct block_device *bdev, struct blkpg_ioctl_arg __user
mutex_lock(&bdev->bd_mutex);
/* overlap? */
- for (i = 0; i < disk_max_parts(disk); i++) {
- struct hd_struct *s = disk->part[i];
-
- if (!s)
- continue;
- if (!(start+length <= s->start_sect ||
- start >= s->start_sect + s->nr_sects)) {
+ disk_part_iter_start(&piter, disk,
+ DISK_PITER_INCL_EMPTY);
+ while ((part = disk_part_iter_next(&piter))) {
+ if (!(start + length <= part->start_sect ||
+ start >= part->start_sect + part->nr_sects)) {
+ disk_part_iter_stop(&piter);
mutex_unlock(&bdev->bd_mutex);
return -EBUSY;
}
}
+ disk_part_iter_stop(&piter);
/* all seems OK */
rc = add_partition(disk, partno, start, length,
@@ -66,11 +68,15 @@ static int blkpg_ioctl(struct block_device *bdev, struct blkpg_ioctl_arg __user
return rc;
case BLKPG_DEL_PARTITION:
- if (!disk->part[partno - 1])
+ part = disk_get_part(disk, partno);
+ if (!part)
return -ENXIO;
- bdevp = bdget_disk(disk, partno);
+
+ bdevp = bdget(part_devt(part));
+ disk_put_part(part);
if (!bdevp)
return -ENOMEM;
+
mutex_lock(&bdevp->bd_mutex);
if (bdevp->bd_openers) {
mutex_unlock(&bdevp->bd_mutex);
diff --git a/drivers/block/aoe/aoecmd.c b/drivers/block/aoe/aoecmd.c
index ba40473..50ef9ea 100644
--- a/drivers/block/aoe/aoecmd.c
+++ b/drivers/block/aoe/aoecmd.c
@@ -757,11 +757,15 @@ diskstats(struct gendisk *disk, struct bio *bio, ulong duration, sector_t sector
const int rw = bio_data_dir(bio);
struct hd_struct *part;
- part = disk_map_sector(disk, sector);
+ rcu_read_lock();
+
+ part = disk_map_sector_rcu(disk, sector);
all_stat_inc(disk, part, ios[rw], sector);
all_stat_add(disk, part, ticks[rw], duration, sector);
all_stat_add(disk, part, sectors[rw], n_sect, sector);
all_stat_add(disk, part, io_ticks, duration, sector);
+
+ rcu_read_unlock();
}
void
diff --git a/fs/block_dev.c b/fs/block_dev.c
index 7d00b22..5d38203 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -929,6 +929,7 @@ static int do_open(struct block_device *bdev, struct file *file, int for_part)
{
struct module *owner = NULL;
struct gendisk *disk;
+ struct hd_struct *part = NULL;
int ret;
int partno;
int perm = 0;
@@ -976,7 +977,6 @@ static int do_open(struct block_device *bdev, struct file *file, int for_part)
if (bdev->bd_invalidated)
rescan_partitions(disk, bdev);
} else {
- struct hd_struct *p;
struct block_device *whole;
whole = bdget_disk(disk, 0);
ret = -ENOMEM;
@@ -987,16 +987,16 @@ static int do_open(struct block_device *bdev, struct file *file, int for_part)
if (ret)
goto out_first;
bdev->bd_contains = whole;
- p = disk->part[partno - 1];
+ part = disk_get_part(disk, partno);
bdev->bd_inode->i_data.backing_dev_info =
whole->bd_inode->i_data.backing_dev_info;
- if (!(disk->flags & GENHD_FL_UP) || !p || !p->nr_sects) {
+ if (!(disk->flags & GENHD_FL_UP) ||
+ !part || !part->nr_sects) {
ret = -ENXIO;
goto out_first;
}
- kobject_get(&p->dev.kobj);
- bdev->bd_part = p;
- bd_set_size(bdev, (loff_t) p->nr_sects << 9);
+ bdev->bd_part = part;
+ bd_set_size(bdev, (loff_t)part->nr_sects << 9);
}
} else {
put_disk(disk);
@@ -1025,6 +1025,7 @@ out_first:
__blkdev_put(bdev->bd_contains, 1);
bdev->bd_contains = NULL;
put_disk(disk);
+ disk_put_part(part);
module_put(owner);
out:
mutex_unlock(&bdev->bd_mutex);
@@ -1117,7 +1118,7 @@ static int __blkdev_put(struct block_device *bdev, int for_part)
module_put(owner);
if (bdev->bd_contains != bdev) {
- kobject_put(&bdev->bd_part->dev.kobj);
+ disk_put_part(bdev->bd_part);
bdev->bd_part = NULL;
}
bdev->bd_disk = NULL;
diff --git a/fs/partitions/check.c b/fs/partitions/check.c
index 32024f7..87c23e3 100644
--- a/fs/partitions/check.c
+++ b/fs/partitions/check.c
@@ -323,19 +323,30 @@ static inline void disk_sysfs_add_subdirs(struct gendisk *disk)
kobject_put(k);
}
+static void delete_partition_rcu_cb(struct rcu_head *head)
+{
+ struct hd_struct *part = container_of(head, struct hd_struct, rcu_head);
+
+ part->start_sect = 0;
+ part->nr_sects = 0;
+ part_stat_set_all(part, 0);
+ put_device(&part->dev);
+}
+
+
void delete_partition(struct gendisk *disk, int partno)
{
- struct hd_struct *p = disk->part[partno-1];
+ struct hd_struct *part;
- if (!p)
+ part = disk->__part[partno-1];
+ if (!part)
return;
- disk->part[partno-1] = NULL;
- p->start_sect = 0;
- p->nr_sects = 0;
- part_stat_set_all(p, 0);
- kobject_put(p->holder_dir);
- device_del(&p->dev);
- put_device(&p->dev);
+
+ rcu_assign_pointer(disk->__part[partno-1], NULL);
+ kobject_put(part->holder_dir);
+ device_del(&part->dev);
+
+ call_rcu(&part->rcu_head, delete_partition_rcu_cb);
}
static ssize_t whole_disk_show(struct device *dev,
@@ -352,7 +363,7 @@ int add_partition(struct gendisk *disk, int partno,
struct hd_struct *p = NULL;
int err;
- if (disk->part[partno - 1]) {
+ if (disk->__part[partno - 1]) {
err = -EBUSY;
goto fail;
}
@@ -381,7 +392,8 @@ int add_partition(struct gendisk *disk, int partno,
p->dev.class = &block_class;
p->dev.type = &part_type;
p->dev.parent = &disk->dev;
- disk->part[partno-1] = p;
+ INIT_RCU_HEAD(&p->rcu_head);
+ rcu_assign_pointer(disk->__part[partno-1], p);
/* delay uevent until 'holders' subdir is created */
p->dev.uevent_suppress = 1;
@@ -414,9 +426,9 @@ int add_partition(struct gendisk *disk, int partno,
void register_disk(struct gendisk *disk)
{
struct block_device *bdev;
+ struct disk_part_iter piter;
+ struct hd_struct *part;
char *s;
- int i;
- struct hd_struct *p;
int err;
disk->dev.parent = disk->driverfs_dev;
@@ -466,16 +478,16 @@ exit:
kobject_uevent(&disk->dev.kobj, KOBJ_ADD);
/* announce possible partitions */
- for (i = 0; i < disk_max_parts(disk); i++) {
- p = disk->part[i];
- if (!p || !p->nr_sects)
- continue;
- kobject_uevent(&p->dev.kobj, KOBJ_ADD);
- }
+ disk_part_iter_start(&piter, disk, 0);
+ while ((part = disk_part_iter_next(&piter)))
+ kobject_uevent(&part->dev.kobj, KOBJ_ADD);
+ disk_part_iter_stop(&piter);
}
int rescan_partitions(struct gendisk *disk, struct block_device *bdev)
{
+ struct disk_part_iter piter;
+ struct hd_struct *part;
struct parsed_partitions *state;
int p, res;
@@ -485,8 +497,12 @@ int rescan_partitions(struct gendisk *disk, struct block_device *bdev)
if (res)
return res;
bdev->bd_invalidated = 0;
- for (p = 1; p <= disk_max_parts(disk); p++)
- delete_partition(disk, p);
+
+ disk_part_iter_start(&piter, disk, DISK_PITER_INCL_EMPTY);
+ while ((part = disk_part_iter_next(&piter)))
+ delete_partition(disk, part->partno);
+ disk_part_iter_stop(&piter);
+
if (disk->fops->revalidate_disk)
disk->fops->revalidate_disk(disk);
if (!get_capacity(disk) || !(state = check_partition(disk, bdev)))
@@ -544,12 +560,15 @@ EXPORT_SYMBOL(read_dev_sector);
void del_gendisk(struct gendisk *disk)
{
- int p;
+ struct disk_part_iter piter;
+ struct hd_struct *part;
/* invalidate stuff */
- for (p = disk_max_parts(disk); p > 0; p--) {
- invalidate_partition(disk, p);
- delete_partition(disk, p);
+ disk_part_iter_start(&piter, disk,
+ DISK_PITER_INCL_EMPTY | DISK_PITER_REVERSE);
+ while ((part = disk_part_iter_next(&piter))) {
+ invalidate_partition(disk, part->partno);
+ delete_partition(disk, part->partno);
}
invalidate_partition(disk, 0);
disk->capacity = 0;
diff --git a/include/linux/genhd.h b/include/linux/genhd.h
index 0b102c1..3e35945 100644
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -11,6 +11,7 @@
#include <linux/types.h>
#include <linux/kdev_t.h>
+#include <linux/rcupdate.h>
#ifdef CONFIG_BLOCK
@@ -100,6 +101,7 @@ struct hd_struct {
#else
struct disk_stats dkstats;
#endif
+ struct rcu_head rcu_head;
};
#define GENHD_FL_REMOVABLE 1
@@ -121,7 +123,14 @@ struct gendisk {
* disks that can't be partitioned. */
char disk_name[32]; /* name of major driver */
- struct hd_struct **part; /* [indexed by minor - 1] */
+
+ /* Array of pointers to partitions indexed by partno - 1.
+ * Protected with matching bdev lock but stat and other
+ * non-critical accesses use RCU. Always access through
+ * helpers.
+ */
+ struct hd_struct **__part;
+
struct block_device_operations *fops;
struct request_queue *queue;
void *private_data;
@@ -169,25 +178,41 @@ static inline dev_t part_devt(struct hd_struct *part)
return part->dev.devt;
}
+extern struct hd_struct *disk_get_part(struct gendisk *disk, int partno);
+
+static inline void disk_put_part(struct hd_struct *part)
+{
+ if (likely(part))
+ put_device(&part->dev);
+}
+
+/*
+ * Smarter partition iterator without context limits.
+ */
+#define DISK_PITER_REVERSE (1 << 0) /* iterate in the reverse direction */
+#define DISK_PITER_INCL_EMPTY (1 << 1) /* include 0-sized parts */
+
+struct disk_part_iter {
+ struct gendisk *disk;
+ struct hd_struct *part;
+ int idx;
+ unsigned int flags;
+};
+
+extern void disk_part_iter_start(struct disk_part_iter *piter,
+ struct gendisk *disk, unsigned int flags);
+extern struct hd_struct *disk_part_iter_next(struct disk_part_iter *piter);
+extern void disk_part_iter_stop(struct disk_part_iter *piter);
+
+extern struct hd_struct *disk_map_sector_rcu(struct gendisk *disk,
+ sector_t sector);
+
/*
* Macros to operate on percpu disk statistics:
*
* The __ variants should only be called in critical sections. The full
* variants disable/enable preemption.
*/
-static inline struct hd_struct *disk_map_sector(struct gendisk *gendiskp,
- sector_t sector)
-{
- struct hd_struct *part;
- int i;
- for (i = 0; i < disk_max_parts(gendiskp); i++) {
- part = gendiskp->part[i];
- if (part && part->start_sect <= sector
- && sector < part->start_sect + part->nr_sects)
- return part;
- }
- return NULL;
-}
#ifdef CONFIG_SMP
#define __disk_stat_add(gendiskp, field, addnd) \
--
1.5.4.5
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 05/10] block: always use __{disk|part|all}_stat_*() and kill non-underbarred versions
2008-07-14 7:12 [PATCHSET 2.6.26] block: implement extended devt, take #2 Tejun Heo
` (3 preceding siblings ...)
2008-07-14 7:12 ` [PATCH 04/10] block: fix disk->part[] dereferencing race Tejun Heo
@ 2008-07-14 7:12 ` Tejun Heo
2008-07-14 7:12 ` [PATCH 06/10] block: drop underbars from __{disk|part|all}_stat_*() Tejun Heo
` (6 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Tejun Heo @ 2008-07-14 7:12 UTC (permalink / raw)
To: jens.axboe, James.Bottomley, bharrosh, greg.freemyer, linux-scsi,
brking, liml, viro
Cc: Tejun Heo
There are two variants of stat functions - ones prefixed with double
underbars which don't care about preemption and ones without which
disable preemption before manipulating per-cpu counters. It's unclear
whether the underbarred ones assume that preemtion is disabled on
entry as some callers don't do that. In any case, stats are not
critical data and errors don't lead to critical failures.
However, consistently using the underbarred version and ensuring that
they are called with preemption disabled doesn't incur noticeable
overhead or even reduces overhead in some cases.
* part stat manipulations need disk_map_sector_rcu() which involves
read locking RCU anyway. Using rcu_read_lock_preempt() instead
solves the problem nicely. On rcuclassic, this means no extra
overhead.
* Calls to the non-underbarred versions are converted to explicit
preemtion disable and calls to respective underbarrded versions. As
all such users had more than one consecutive stat ops, this reduces
extra preemption disable/enables.
* In drivers/md/dm.c::end_io_acct(), __disk_stat_add() call is moved
into neighboring preemption disabled block.
The conversion makes the stats usage more consistent and IMHO the
added few preemption calls are well justified for that.
As this change makes non-underbarred versions useless, non-underbarred
stat functions and macros are killed. The next patch will drop
underbars from the underbarred versions as it's superflous now. This
is done as a separate step so that compile fails between this and the
next patch if someone's left behind.
Signed-off-by: Tejun Heo <tj@kernel.org>
---
block/blk-core.c | 16 +++++++-------
block/blk-merge.c | 4 +-
drivers/block/aoe/aoecmd.c | 12 +++++-----
drivers/md/dm.c | 10 +++++---
drivers/md/linear.c | 6 +++-
drivers/md/multipath.c | 6 +++-
drivers/md/raid0.c | 6 +++-
drivers/md/raid1.c | 6 +++-
drivers/md/raid10.c | 6 +++-
drivers/md/raid5.c | 6 +++-
include/linux/genhd.h | 48 --------------------------------------------
11 files changed, 46 insertions(+), 80 deletions(-)
diff --git a/block/blk-core.c b/block/blk-core.c
index 84292e1..3238ffe 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -60,7 +60,7 @@ static void drive_stat_acct(struct request *rq, int new_io)
if (!blk_fs_request(rq) || !rq->rq_disk)
return;
- rcu_read_lock();
+ rcu_read_lock_preempt();
part = disk_map_sector_rcu(rq->rq_disk, rq->sector);
if (!new_io)
@@ -74,7 +74,7 @@ static void drive_stat_acct(struct request *rq, int new_io)
}
}
- rcu_read_unlock();
+ rcu_read_unlock_preempt();
}
void blk_queue_congestion_threshold(struct request_queue *q)
@@ -1542,11 +1542,11 @@ static int __end_that_request_first(struct request *req, int error,
const int rw = rq_data_dir(req);
struct hd_struct *part;
- rcu_read_lock();
+ rcu_read_lock_preempt();
part = disk_map_sector_rcu(req->rq_disk, req->sector);
- all_stat_add(req->rq_disk, part, sectors[rw],
- nr_bytes >> 9, req->sector);
- rcu_read_unlock();
+ __all_stat_add(req->rq_disk, part, sectors[rw],
+ nr_bytes >> 9, req->sector);
+ rcu_read_unlock_preempt();
}
total_bytes = bio_nbytes = 0;
@@ -1732,7 +1732,7 @@ static void end_that_request_last(struct request *req, int error)
const int rw = rq_data_dir(req);
struct hd_struct *part;
- rcu_read_lock();
+ rcu_read_lock_preempt();
part = disk_map_sector_rcu(disk, req->sector);
@@ -1745,7 +1745,7 @@ static void end_that_request_last(struct request *req, int error)
part->in_flight--;
}
- rcu_read_unlock();
+ rcu_read_unlock_preempt();
}
if (req->end_io)
diff --git a/block/blk-merge.c b/block/blk-merge.c
index 9be9b2f..55456c8 100644
--- a/block/blk-merge.c
+++ b/block/blk-merge.c
@@ -469,7 +469,7 @@ static int attempt_merge(struct request_queue *q, struct request *req,
if (req->rq_disk) {
struct hd_struct *part;
- rcu_read_lock();
+ rcu_read_lock_preempt();
part = disk_map_sector_rcu(req->rq_disk, req->sector);
disk_round_stats(req->rq_disk);
@@ -479,7 +479,7 @@ static int attempt_merge(struct request_queue *q, struct request *req,
part->in_flight--;
}
- rcu_read_unlock();
+ rcu_read_unlock_preempt();
}
req->ioprio = ioprio_best(req->ioprio, next->ioprio);
diff --git a/drivers/block/aoe/aoecmd.c b/drivers/block/aoe/aoecmd.c
index 50ef9ea..7bd8c98 100644
--- a/drivers/block/aoe/aoecmd.c
+++ b/drivers/block/aoe/aoecmd.c
@@ -757,15 +757,15 @@ diskstats(struct gendisk *disk, struct bio *bio, ulong duration, sector_t sector
const int rw = bio_data_dir(bio);
struct hd_struct *part;
- rcu_read_lock();
+ rcu_read_lock_preempt();
part = disk_map_sector_rcu(disk, sector);
- all_stat_inc(disk, part, ios[rw], sector);
- all_stat_add(disk, part, ticks[rw], duration, sector);
- all_stat_add(disk, part, sectors[rw], n_sect, sector);
- all_stat_add(disk, part, io_ticks, duration, sector);
+ __all_stat_inc(disk, part, ios[rw], sector);
+ __all_stat_add(disk, part, ticks[rw], duration, sector);
+ __all_stat_add(disk, part, sectors[rw], n_sect, sector);
+ __all_stat_add(disk, part, io_ticks, duration, sector);
- rcu_read_unlock();
+ rcu_read_unlock_preempt();
}
void
diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index 923fea4..6918bb7 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -396,10 +396,10 @@ static int end_io_acct(struct dm_io *io)
preempt_disable();
disk_round_stats(dm_disk(md));
+ __disk_stat_add(dm_disk(md), ticks[rw], duration);
preempt_enable();
- dm_disk(md)->in_flight = pending = atomic_dec_return(&md->pending);
- disk_stat_add(dm_disk(md), ticks[rw], duration);
+ dm_disk(md)->in_flight = pending = atomic_dec_return(&md->pending);
return !pending;
}
@@ -850,8 +850,10 @@ static int dm_request(struct request_queue *q, struct bio *bio)
down_read(&md->io_lock);
- disk_stat_inc(dm_disk(md), ios[rw]);
- disk_stat_add(dm_disk(md), sectors[rw], bio_sectors(bio));
+ preempt_disable();
+ __disk_stat_inc(dm_disk(md), ios[rw]);
+ __disk_stat_add(dm_disk(md), sectors[rw], bio_sectors(bio));
+ preempt_enable();
/*
* If we're suspended we have to queue
diff --git a/drivers/md/linear.c b/drivers/md/linear.c
index 1074824..ec35b8b 100644
--- a/drivers/md/linear.c
+++ b/drivers/md/linear.c
@@ -322,8 +322,10 @@ static int linear_make_request (struct request_queue *q, struct bio *bio)
return 0;
}
- disk_stat_inc(mddev->gendisk, ios[rw]);
- disk_stat_add(mddev->gendisk, sectors[rw], bio_sectors(bio));
+ preempt_disable();
+ __disk_stat_inc(mddev->gendisk, ios[rw]);
+ __disk_stat_add(mddev->gendisk, sectors[rw], bio_sectors(bio));
+ preempt_enable();
tmp_dev = which_dev(mddev, bio->bi_sector);
block = bio->bi_sector >> 1;
diff --git a/drivers/md/multipath.c b/drivers/md/multipath.c
index e968116..aed8ea9 100644
--- a/drivers/md/multipath.c
+++ b/drivers/md/multipath.c
@@ -158,8 +158,10 @@ static int multipath_make_request (struct request_queue *q, struct bio * bio)
mp_bh->master_bio = bio;
mp_bh->mddev = mddev;
- disk_stat_inc(mddev->gendisk, ios[rw]);
- disk_stat_add(mddev->gendisk, sectors[rw], bio_sectors(bio));
+ preempt_disable();
+ __disk_stat_inc(mddev->gendisk, ios[rw]);
+ __disk_stat_add(mddev->gendisk, sectors[rw], bio_sectors(bio));
+ preempt_enable();
mp_bh->path = multipath_map(conf);
if (mp_bh->path < 0) {
diff --git a/drivers/md/raid0.c b/drivers/md/raid0.c
index 914c04d..d0cdfe1 100644
--- a/drivers/md/raid0.c
+++ b/drivers/md/raid0.c
@@ -403,8 +403,10 @@ static int raid0_make_request (struct request_queue *q, struct bio *bio)
return 0;
}
- disk_stat_inc(mddev->gendisk, ios[rw]);
- disk_stat_add(mddev->gendisk, sectors[rw], bio_sectors(bio));
+ preempt_disable();
+ __disk_stat_inc(mddev->gendisk, ios[rw]);
+ __disk_stat_add(mddev->gendisk, sectors[rw], bio_sectors(bio));
+ preempt_enable();
chunk_size = mddev->chunk_size >> 10;
chunk_sects = mddev->chunk_size >> 9;
diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index c610b94..6687ffe 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -804,8 +804,10 @@ static int make_request(struct request_queue *q, struct bio * bio)
bitmap = mddev->bitmap;
- disk_stat_inc(mddev->gendisk, ios[rw]);
- disk_stat_add(mddev->gendisk, sectors[rw], bio_sectors(bio));
+ preempt_disable();
+ __disk_stat_inc(mddev->gendisk, ios[rw]);
+ __disk_stat_add(mddev->gendisk, sectors[rw], bio_sectors(bio));
+ preempt_enable();
/*
* make_request() can abort the operation when READA is being
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index a71277b..1d644dc 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -838,8 +838,10 @@ static int make_request(struct request_queue *q, struct bio * bio)
*/
wait_barrier(conf);
- disk_stat_inc(mddev->gendisk, ios[rw]);
- disk_stat_add(mddev->gendisk, sectors[rw], bio_sectors(bio));
+ preempt_disable();
+ __disk_stat_inc(mddev->gendisk, ios[rw]);
+ __disk_stat_add(mddev->gendisk, sectors[rw], bio_sectors(bio));
+ preempt_enable();
r10_bio = mempool_alloc(conf->r10bio_pool, GFP_NOIO);
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index 3b27df5..a869e58 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -3580,8 +3580,10 @@ static int make_request(struct request_queue *q, struct bio * bi)
md_write_start(mddev, bi);
- disk_stat_inc(mddev->gendisk, ios[rw]);
- disk_stat_add(mddev->gendisk, sectors[rw], bio_sectors(bi));
+ preempt_disable();
+ __disk_stat_inc(mddev->gendisk, ios[rw]);
+ __disk_stat_add(mddev->gendisk, sectors[rw], bio_sectors(bi));
+ preempt_enable();
if (rw == READ &&
mddev->reshape_position == MaxSector &&
diff --git a/include/linux/genhd.h b/include/linux/genhd.h
index 3e35945..87a338b 100644
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -207,13 +207,6 @@ extern void disk_part_iter_stop(struct disk_part_iter *piter);
extern struct hd_struct *disk_map_sector_rcu(struct gendisk *disk,
sector_t sector);
-/*
- * Macros to operate on percpu disk statistics:
- *
- * The __ variants should only be called in critical sections. The full
- * variants disable/enable preemption.
- */
-
#ifdef CONFIG_SMP
#define __disk_stat_add(gendiskp, field, addnd) \
(per_cpu_ptr(gendiskp->dkstats, smp_processor_id())->field += addnd)
@@ -292,63 +285,22 @@ static inline void part_stat_set_all(struct hd_struct *part, int value)
#endif /* CONFIG_SMP */
-#define disk_stat_add(gendiskp, field, addnd) \
- do { \
- preempt_disable(); \
- __disk_stat_add(gendiskp, field, addnd); \
- preempt_enable(); \
- } while (0)
-
#define __disk_stat_dec(gendiskp, field) __disk_stat_add(gendiskp, field, -1)
-#define disk_stat_dec(gendiskp, field) disk_stat_add(gendiskp, field, -1)
-
#define __disk_stat_inc(gendiskp, field) __disk_stat_add(gendiskp, field, 1)
-#define disk_stat_inc(gendiskp, field) disk_stat_add(gendiskp, field, 1)
-
#define __disk_stat_sub(gendiskp, field, subnd) \
__disk_stat_add(gendiskp, field, -subnd)
-#define disk_stat_sub(gendiskp, field, subnd) \
- disk_stat_add(gendiskp, field, -subnd)
-
-#define part_stat_add(gendiskp, field, addnd) \
- do { \
- preempt_disable(); \
- __part_stat_add(gendiskp, field, addnd);\
- preempt_enable(); \
- } while (0)
#define __part_stat_dec(gendiskp, field) __part_stat_add(gendiskp, field, -1)
-#define part_stat_dec(gendiskp, field) part_stat_add(gendiskp, field, -1)
-
#define __part_stat_inc(gendiskp, field) __part_stat_add(gendiskp, field, 1)
-#define part_stat_inc(gendiskp, field) part_stat_add(gendiskp, field, 1)
-
#define __part_stat_sub(gendiskp, field, subnd) \
__part_stat_add(gendiskp, field, -subnd)
-#define part_stat_sub(gendiskp, field, subnd) \
- part_stat_add(gendiskp, field, -subnd)
-
-#define all_stat_add(gendiskp, part, field, addnd, sector) \
- do { \
- preempt_disable(); \
- __all_stat_add(gendiskp, part, field, addnd, sector); \
- preempt_enable(); \
- } while (0)
#define __all_stat_dec(gendiskp, field, sector) \
__all_stat_add(gendiskp, field, -1, sector)
-#define all_stat_dec(gendiskp, field, sector) \
- all_stat_add(gendiskp, field, -1, sector)
-
#define __all_stat_inc(gendiskp, part, field, sector) \
__all_stat_add(gendiskp, part, field, 1, sector)
-#define all_stat_inc(gendiskp, part, field, sector) \
- all_stat_add(gendiskp, part, field, 1, sector)
-
#define __all_stat_sub(gendiskp, part, field, subnd, sector) \
__all_stat_add(gendiskp, part, field, -subnd, sector)
-#define all_stat_sub(gendiskp, part, field, subnd, sector) \
- all_stat_add(gendiskp, part, field, -subnd, sector)
/* Inlines to alloc and free disk stats in struct gendisk */
#ifdef CONFIG_SMP
--
1.5.4.5
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 06/10] block: drop underbars from __{disk|part|all}_stat_*()
2008-07-14 7:12 [PATCHSET 2.6.26] block: implement extended devt, take #2 Tejun Heo
` (4 preceding siblings ...)
2008-07-14 7:12 ` [PATCH 05/10] block: always use __{disk|part|all}_stat_*() and kill non-underbarred versions Tejun Heo
@ 2008-07-14 7:12 ` Tejun Heo
2008-07-14 7:12 ` [PATCH 07/10] block: implement extended dev numbers Tejun Heo
` (5 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Tejun Heo @ 2008-07-14 7:12 UTC (permalink / raw)
To: jens.axboe, James.Bottomley, bharrosh, greg.freemyer, linux-scsi,
brking, liml, viro
Cc: Tejun Heo
Now that the not-underbarred versions are gone, there's no reason to
keep the ugly underbars. Drop and add comments explaining that users
of these macros and funcitons need to turn off preemption.
While at it, add comments for stat macros and functions.
Signed-off-by: Tejun Heo <tj@kernel.org>
---
block/blk-core.c | 36 ++++++++++++++++--------
drivers/block/aoe/aoecmd.c | 8 +++---
drivers/md/dm.c | 6 ++--
drivers/md/linear.c | 4 +-
drivers/md/multipath.c | 4 +-
drivers/md/raid0.c | 4 +-
drivers/md/raid1.c | 4 +-
drivers/md/raid10.c | 4 +-
drivers/md/raid5.c | 4 +-
include/linux/genhd.h | 63 +++++++++++++++++++++++++------------------
10 files changed, 80 insertions(+), 57 deletions(-)
diff --git a/block/blk-core.c b/block/blk-core.c
index 3238ffe..1da936f 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -64,7 +64,7 @@ static void drive_stat_acct(struct request *rq, int new_io)
part = disk_map_sector_rcu(rq->rq_disk, rq->sector);
if (!new_io)
- __all_stat_inc(rq->rq_disk, part, merges[rw], rq->sector);
+ all_stat_inc(rq->rq_disk, part, merges[rw], rq->sector);
else {
disk_round_stats(rq->rq_disk);
rq->rq_disk->in_flight++;
@@ -966,7 +966,7 @@ static inline void add_request(struct request_queue *q, struct request *req)
__elv_add_request(q, req, ELEVATOR_INSERT_SORT, 0);
}
-/*
+/**
* disk_round_stats() - Round off the performance stats on a struct
* disk_stats.
*
@@ -980,6 +980,9 @@ static inline void add_request(struct request_queue *q, struct request *req)
* function to do a round-off before returning the results when reading
* /proc/diskstats. This accounts immediately for all queue usage up to
* the current jiffies and restarts the counters again.
+ *
+ * CONTEXT:
+ * Preemption disabled.
*/
void disk_round_stats(struct gendisk *disk)
{
@@ -989,14 +992,23 @@ void disk_round_stats(struct gendisk *disk)
return;
if (disk->in_flight) {
- __disk_stat_add(disk, time_in_queue,
- disk->in_flight * (now - disk->stamp));
- __disk_stat_add(disk, io_ticks, (now - disk->stamp));
+ disk_stat_add(disk, time_in_queue,
+ disk->in_flight * (now - disk->stamp));
+ disk_stat_add(disk, io_ticks, (now - disk->stamp));
}
disk->stamp = now;
}
EXPORT_SYMBOL_GPL(disk_round_stats);
+/**
+ * part_round_stats() - Round off the performance stats on a struct
+ * disk_stats.
+ *
+ * Please see disk_round_stats() for more information.
+ *
+ * CONTEXT:
+ * Preemption disabled.
+ */
void part_round_stats(struct hd_struct *part)
{
unsigned long now = jiffies;
@@ -1005,9 +1017,9 @@ void part_round_stats(struct hd_struct *part)
return;
if (part->in_flight) {
- __part_stat_add(part, time_in_queue,
- part->in_flight * (now - part->stamp));
- __part_stat_add(part, io_ticks, (now - part->stamp));
+ part_stat_add(part, time_in_queue,
+ part->in_flight * (now - part->stamp));
+ part_stat_add(part, io_ticks, (now - part->stamp));
}
part->stamp = now;
}
@@ -1544,8 +1556,8 @@ static int __end_that_request_first(struct request *req, int error,
rcu_read_lock_preempt();
part = disk_map_sector_rcu(req->rq_disk, req->sector);
- __all_stat_add(req->rq_disk, part, sectors[rw],
- nr_bytes >> 9, req->sector);
+ all_stat_add(req->rq_disk, part, sectors[rw],
+ nr_bytes >> 9, req->sector);
rcu_read_unlock_preempt();
}
@@ -1736,8 +1748,8 @@ static void end_that_request_last(struct request *req, int error)
part = disk_map_sector_rcu(disk, req->sector);
- __all_stat_inc(disk, part, ios[rw], req->sector);
- __all_stat_add(disk, part, ticks[rw], duration, req->sector);
+ all_stat_inc(disk, part, ios[rw], req->sector);
+ all_stat_add(disk, part, ticks[rw], duration, req->sector);
disk_round_stats(disk);
disk->in_flight--;
if (part) {
diff --git a/drivers/block/aoe/aoecmd.c b/drivers/block/aoe/aoecmd.c
index 7bd8c98..6710df3 100644
--- a/drivers/block/aoe/aoecmd.c
+++ b/drivers/block/aoe/aoecmd.c
@@ -760,10 +760,10 @@ diskstats(struct gendisk *disk, struct bio *bio, ulong duration, sector_t sector
rcu_read_lock_preempt();
part = disk_map_sector_rcu(disk, sector);
- __all_stat_inc(disk, part, ios[rw], sector);
- __all_stat_add(disk, part, ticks[rw], duration, sector);
- __all_stat_add(disk, part, sectors[rw], n_sect, sector);
- __all_stat_add(disk, part, io_ticks, duration, sector);
+ all_stat_inc(disk, part, ios[rw], sector);
+ all_stat_add(disk, part, ticks[rw], duration, sector);
+ all_stat_add(disk, part, sectors[rw], n_sect, sector);
+ all_stat_add(disk, part, io_ticks, duration, sector);
rcu_read_unlock_preempt();
}
diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index 6918bb7..f76fb06 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -396,7 +396,7 @@ static int end_io_acct(struct dm_io *io)
preempt_disable();
disk_round_stats(dm_disk(md));
- __disk_stat_add(dm_disk(md), ticks[rw], duration);
+ disk_stat_add(dm_disk(md), ticks[rw], duration);
preempt_enable();
dm_disk(md)->in_flight = pending = atomic_dec_return(&md->pending);
@@ -851,8 +851,8 @@ static int dm_request(struct request_queue *q, struct bio *bio)
down_read(&md->io_lock);
preempt_disable();
- __disk_stat_inc(dm_disk(md), ios[rw]);
- __disk_stat_add(dm_disk(md), sectors[rw], bio_sectors(bio));
+ disk_stat_inc(dm_disk(md), ios[rw]);
+ disk_stat_add(dm_disk(md), sectors[rw], bio_sectors(bio));
preempt_enable();
/*
diff --git a/drivers/md/linear.c b/drivers/md/linear.c
index ec35b8b..7341196 100644
--- a/drivers/md/linear.c
+++ b/drivers/md/linear.c
@@ -323,8 +323,8 @@ static int linear_make_request (struct request_queue *q, struct bio *bio)
}
preempt_disable();
- __disk_stat_inc(mddev->gendisk, ios[rw]);
- __disk_stat_add(mddev->gendisk, sectors[rw], bio_sectors(bio));
+ disk_stat_inc(mddev->gendisk, ios[rw]);
+ disk_stat_add(mddev->gendisk, sectors[rw], bio_sectors(bio));
preempt_enable();
tmp_dev = which_dev(mddev, bio->bi_sector);
diff --git a/drivers/md/multipath.c b/drivers/md/multipath.c
index aed8ea9..5db0a45 100644
--- a/drivers/md/multipath.c
+++ b/drivers/md/multipath.c
@@ -159,8 +159,8 @@ static int multipath_make_request (struct request_queue *q, struct bio * bio)
mp_bh->mddev = mddev;
preempt_disable();
- __disk_stat_inc(mddev->gendisk, ios[rw]);
- __disk_stat_add(mddev->gendisk, sectors[rw], bio_sectors(bio));
+ disk_stat_inc(mddev->gendisk, ios[rw]);
+ disk_stat_add(mddev->gendisk, sectors[rw], bio_sectors(bio));
preempt_enable();
mp_bh->path = multipath_map(conf);
diff --git a/drivers/md/raid0.c b/drivers/md/raid0.c
index d0cdfe1..2602b05 100644
--- a/drivers/md/raid0.c
+++ b/drivers/md/raid0.c
@@ -404,8 +404,8 @@ static int raid0_make_request (struct request_queue *q, struct bio *bio)
}
preempt_disable();
- __disk_stat_inc(mddev->gendisk, ios[rw]);
- __disk_stat_add(mddev->gendisk, sectors[rw], bio_sectors(bio));
+ disk_stat_inc(mddev->gendisk, ios[rw]);
+ disk_stat_add(mddev->gendisk, sectors[rw], bio_sectors(bio));
preempt_enable();
chunk_size = mddev->chunk_size >> 10;
diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index 6687ffe..1fd2c3d 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -805,8 +805,8 @@ static int make_request(struct request_queue *q, struct bio * bio)
bitmap = mddev->bitmap;
preempt_disable();
- __disk_stat_inc(mddev->gendisk, ios[rw]);
- __disk_stat_add(mddev->gendisk, sectors[rw], bio_sectors(bio));
+ disk_stat_inc(mddev->gendisk, ios[rw]);
+ disk_stat_add(mddev->gendisk, sectors[rw], bio_sectors(bio));
preempt_enable();
/*
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index 1d644dc..2521b73 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -839,8 +839,8 @@ static int make_request(struct request_queue *q, struct bio * bio)
wait_barrier(conf);
preempt_disable();
- __disk_stat_inc(mddev->gendisk, ios[rw]);
- __disk_stat_add(mddev->gendisk, sectors[rw], bio_sectors(bio));
+ disk_stat_inc(mddev->gendisk, ios[rw]);
+ disk_stat_add(mddev->gendisk, sectors[rw], bio_sectors(bio));
preempt_enable();
r10_bio = mempool_alloc(conf->r10bio_pool, GFP_NOIO);
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index a869e58..bfd8b6c 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -3581,8 +3581,8 @@ static int make_request(struct request_queue *q, struct bio * bi)
md_write_start(mddev, bi);
preempt_disable();
- __disk_stat_inc(mddev->gendisk, ios[rw]);
- __disk_stat_add(mddev->gendisk, sectors[rw], bio_sectors(bi));
+ disk_stat_inc(mddev->gendisk, ios[rw]);
+ disk_stat_add(mddev->gendisk, sectors[rw], bio_sectors(bi));
preempt_enable();
if (rw == READ &&
diff --git a/include/linux/genhd.h b/include/linux/genhd.h
index 87a338b..7737468 100644
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -207,8 +207,18 @@ extern void disk_part_iter_stop(struct disk_part_iter *piter);
extern struct hd_struct *disk_map_sector_rcu(struct gendisk *disk,
sector_t sector);
+/*
+ * Macros to to operate on disk statistics:
+ *
+ * {disk|part|all}_stat_{add|sub|inc|dec}() modify the stat counters
+ * and should be called with preemption disabled.
+ *
+ * {part|disk}_stat_read() can be called with preemption enabled.
+ *
+ * {part|disk}_stat_set_all() are for internal use only.
+ */
#ifdef CONFIG_SMP
-#define __disk_stat_add(gendiskp, field, addnd) \
+#define disk_stat_add(gendiskp, field, addnd) \
(per_cpu_ptr(gendiskp->dkstats, smp_processor_id())->field += addnd)
#define disk_stat_read(gendiskp, field) \
@@ -220,7 +230,8 @@ extern struct hd_struct *disk_map_sector_rcu(struct gendisk *disk,
res; \
})
-static inline void disk_stat_set_all(struct gendisk *gendiskp, int value) {
+static inline void disk_stat_set_all(struct gendisk *gendiskp, int value)
+{
int i;
for_each_possible_cpu(i)
@@ -228,14 +239,14 @@ static inline void disk_stat_set_all(struct gendisk *gendiskp, int value) {
sizeof(struct disk_stats));
}
-#define __part_stat_add(part, field, addnd) \
+#define part_stat_add(part, field, addnd) \
(per_cpu_ptr(part->dkstats, smp_processor_id())->field += addnd)
-#define __all_stat_add(gendiskp, part, field, addnd, sector) \
+#define all_stat_add(gendiskp, part, field, addnd, sector) \
({ \
if (part) \
- __part_stat_add(part, field, addnd); \
- __disk_stat_add(gendiskp, field, addnd); \
+ part_stat_add(part, field, addnd); \
+ disk_stat_add(gendiskp, field, addnd); \
})
#define part_stat_read(part, field) \
@@ -257,7 +268,7 @@ static inline void part_stat_set_all(struct hd_struct *part, int value)
}
#else /* !CONFIG_SMP */
-#define __disk_stat_add(gendiskp, field, addnd) \
+#define disk_stat_add(gendiskp, field, addnd) \
(gendiskp->dkstats.field += addnd)
#define disk_stat_read(gendiskp, field) (gendiskp->dkstats.field)
@@ -266,14 +277,14 @@ static inline void disk_stat_set_all(struct gendisk *gendiskp, int value)
memset(&gendiskp->dkstats, value, sizeof (struct disk_stats));
}
-#define __part_stat_add(part, field, addnd) \
+#define part_stat_add(part, field, addnd) \
(part->dkstats.field += addnd)
-#define __all_stat_add(gendiskp, part, field, addnd, sector) \
+#define all_stat_add(gendiskp, part, field, addnd, sector) \
({ \
if (part) \
part->dkstats.field += addnd; \
- __disk_stat_add(gendiskp, field, addnd); \
+ disk_stat_add(gendiskp, field, addnd); \
})
#define part_stat_read(part, field) (part->dkstats.field)
@@ -285,22 +296,22 @@ static inline void part_stat_set_all(struct hd_struct *part, int value)
#endif /* CONFIG_SMP */
-#define __disk_stat_dec(gendiskp, field) __disk_stat_add(gendiskp, field, -1)
-#define __disk_stat_inc(gendiskp, field) __disk_stat_add(gendiskp, field, 1)
-#define __disk_stat_sub(gendiskp, field, subnd) \
- __disk_stat_add(gendiskp, field, -subnd)
-
-#define __part_stat_dec(gendiskp, field) __part_stat_add(gendiskp, field, -1)
-#define __part_stat_inc(gendiskp, field) __part_stat_add(gendiskp, field, 1)
-#define __part_stat_sub(gendiskp, field, subnd) \
- __part_stat_add(gendiskp, field, -subnd)
-
-#define __all_stat_dec(gendiskp, field, sector) \
- __all_stat_add(gendiskp, field, -1, sector)
-#define __all_stat_inc(gendiskp, part, field, sector) \
- __all_stat_add(gendiskp, part, field, 1, sector)
-#define __all_stat_sub(gendiskp, part, field, subnd, sector) \
- __all_stat_add(gendiskp, part, field, -subnd, sector)
+#define disk_stat_dec(gendiskp, field) disk_stat_add(gendiskp, field, -1)
+#define disk_stat_inc(gendiskp, field) disk_stat_add(gendiskp, field, 1)
+#define disk_stat_sub(gendiskp, field, subnd) \
+ disk_stat_add(gendiskp, field, -subnd)
+
+#define part_stat_dec(gendiskp, field) part_stat_add(gendiskp, field, -1)
+#define part_stat_inc(gendiskp, field) part_stat_add(gendiskp, field, 1)
+#define part_stat_sub(gendiskp, field, subnd) \
+ part_stat_add(gendiskp, field, -subnd)
+
+#define all_stat_dec(gendiskp, field, sector) \
+ all_stat_add(gendiskp, field, -1, sector)
+#define all_stat_inc(gendiskp, part, field, sector) \
+ all_stat_add(gendiskp, part, field, 1, sector)
+#define all_stat_sub(gendiskp, part, field, subnd, sector) \
+ all_stat_add(gendiskp, part, field, -subnd, sector)
/* Inlines to alloc and free disk stats in struct gendisk */
#ifdef CONFIG_SMP
--
1.5.4.5
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 07/10] block: implement extended dev numbers
2008-07-14 7:12 [PATCHSET 2.6.26] block: implement extended devt, take #2 Tejun Heo
` (5 preceding siblings ...)
2008-07-14 7:12 ` [PATCH 06/10] block: drop underbars from __{disk|part|all}_stat_*() Tejun Heo
@ 2008-07-14 7:12 ` Tejun Heo
2008-07-14 7:12 ` [PATCH 08/10] block: adjust formatting for large minors and add ext_range sysfs attr Tejun Heo
` (4 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Tejun Heo @ 2008-07-14 7:12 UTC (permalink / raw)
To: jens.axboe, James.Bottomley, bharrosh, greg.freemyer, linux-scsi,
brking, liml, viro
Cc: Tejun Heo
Implement extended device numbers. A block driver can tell block
layer that it wants to use extended device numbers. After the usual
minor space is used up, block layer automatically allocates devt's
from EXT_BLOCK_MAJOR.
Currently only one major number is allocated for this but as the
allocation is strictly on-demand, ~1mil minor space under it should
suffice unless the system actually has more than ~1mil partitions and
if that ever happens adding more majors to the extended devt area is
easy.
Due to internal implementation issues, the first partition can't be
allocated on the extended area. In other words, genhd->minors should
at least be 1. This limitation will be lifted by later changes.
Signed-off-by: Tejun Heo <tj@kernel.org>
---
block/genhd.c | 119 ++++++++++++++++++++++++++++++++++++++++++++++--
fs/partitions/check.c | 10 ++++-
include/linux/genhd.h | 13 ++++-
include/linux/major.h | 2 +
4 files changed, 135 insertions(+), 9 deletions(-)
diff --git a/block/genhd.c b/block/genhd.c
index 76961be..2511694 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -16,6 +16,7 @@
#include <linux/kobj_map.h>
#include <linux/buffer_head.h>
#include <linux/mutex.h>
+#include <linux/idr.h>
#include "blk.h"
@@ -24,6 +25,15 @@ static DEFINE_MUTEX(block_class_lock);
struct kobject *block_depr;
#endif
+/* for extended dynamic devt allocation, currently only one major is used */
+#define MAX_EXT_DEVT (1 << MINORBITS)
+
+/* For extended devt allocation. ext_devt_mutex prevents look up
+ * results from going away underneath its user.
+ */
+static DEFINE_MUTEX(ext_devt_mutex);
+static DEFINE_IDR(ext_devt_idr);
+
static struct device_type disk_type;
/**
@@ -288,6 +298,73 @@ EXPORT_SYMBOL(unregister_blkdev);
static struct kobj_map *bdev_map;
+/**
+ * blk_alloc_devt - allocate a dev_t for a partition
+ * @part: partition to allocate dev_t for
+ * @gfp_mask: memory allocation flag
+ * @devt: out parameter for resulting dev_t
+ *
+ * Allocate a dev_t for block device.
+ *
+ * RETURNS:
+ * 0 on success, allocated dev_t is returned in *@devt. -errno on
+ * failure.
+ *
+ * CONTEXT:
+ * Might sleep.
+ */
+int blk_alloc_devt(struct hd_struct *part, dev_t *devt)
+{
+ int idx, rc;
+
+ /* in consecutive minor range? */
+ if (part->partno < part_to_disk(part)->minors) {
+ *devt = MKDEV(part_to_disk(part)->major, part->partno);
+ return 0;
+ }
+
+ /* allocate ext devt */
+ do {
+ if (!idr_pre_get(&ext_devt_idr, GFP_KERNEL))
+ return -ENOMEM;
+ rc = idr_get_new(&ext_devt_idr, part, &idx);
+ } while (rc == -EAGAIN);
+
+ if (rc)
+ return rc;
+
+ if (idx > MAX_EXT_DEVT) {
+ idr_remove(&ext_devt_idr, idx);
+ return -EBUSY;
+ }
+
+ *devt = MKDEV(BLOCK_EXT_MAJOR, idx);
+ return 0;
+}
+
+/**
+ * blk_free_devt - free a dev_t
+ * @devt: dev_t to free
+ *
+ * Free @devt which was allocated using blk_alloc_devt().
+ *
+ * CONTEXT:
+ * Might sleep.
+ */
+void blk_free_devt(dev_t devt)
+{
+ might_sleep();
+
+ if (devt == MKDEV(0, 0))
+ return;
+
+ if (MAJOR(devt) == BLOCK_EXT_MAJOR) {
+ mutex_lock(&ext_devt_mutex);
+ idr_remove(&ext_devt_idr, MINOR(devt));
+ mutex_unlock(&ext_devt_mutex);
+ }
+}
+
/*
* Register device numbers dev..(dev+range-1)
* range must be nonzero
@@ -368,10 +445,27 @@ void unlink_gendisk(struct gendisk *disk)
*/
struct gendisk *get_gendisk(dev_t devt, int *partno)
{
- struct kobject *kobj = kobj_lookup(bdev_map, devt, partno);
- struct device *dev = kobj_to_dev(kobj);
+ struct gendisk *disk = NULL;
+
+ if (MAJOR(devt) != BLOCK_EXT_MAJOR) {
+ struct kobject *kobj;
+
+ kobj = kobj_lookup(bdev_map, devt, partno);
+ if (kobj)
+ disk = dev_to_disk(kobj_to_dev(kobj));
+ } else {
+ struct hd_struct *part;
+
+ mutex_lock(&ext_devt_mutex);
+ part = idr_find(&ext_devt_idr, MINOR(devt));
+ if (part && get_disk(part_to_disk(part))) {
+ *partno = part->partno;
+ disk = part_to_disk(part);
+ }
+ mutex_unlock(&ext_devt_mutex);
+ }
- return kobj ? dev_to_disk(dev) : NULL;
+ return disk;
}
/**
@@ -880,17 +974,29 @@ struct gendisk *alloc_disk(int minors)
struct gendisk *alloc_disk_node(int minors, int node_id)
{
+ return alloc_disk_ext_node(minors, 0, node_id);
+}
+
+struct gendisk *alloc_disk_ext(int minors, int ext_minors)
+{
+ return alloc_disk_ext_node(minors, ext_minors, -1);
+}
+
+struct gendisk *alloc_disk_ext_node(int minors, int ext_minors, int node_id)
+{
struct gendisk *disk;
disk = kmalloc_node(sizeof(struct gendisk),
GFP_KERNEL | __GFP_ZERO, node_id);
if (disk) {
+ int tot_minors = minors + ext_minors;
+
if (!init_disk_stats(disk)) {
kfree(disk);
return NULL;
}
- if (minors > 1) {
- int size = (minors - 1) * sizeof(struct hd_struct *);
+ if (tot_minors > 1) {
+ int size = (tot_minors - 1) * sizeof(struct hd_struct *);
disk->__part = kmalloc_node(size,
GFP_KERNEL | __GFP_ZERO, node_id);
if (!disk->__part) {
@@ -900,6 +1006,7 @@ struct gendisk *alloc_disk_node(int minors, int node_id)
}
}
disk->minors = minors;
+ disk->ext_minors = ext_minors;
rand_initialize_disk(disk);
disk->dev.class = &block_class;
disk->dev.type = &disk_type;
@@ -912,6 +1019,8 @@ struct gendisk *alloc_disk_node(int minors, int node_id)
EXPORT_SYMBOL(alloc_disk);
EXPORT_SYMBOL(alloc_disk_node);
+EXPORT_SYMBOL(alloc_disk_ext);
+EXPORT_SYMBOL(alloc_disk_ext_node);
struct kobject *get_disk(struct gendisk *disk)
{
diff --git a/fs/partitions/check.c b/fs/partitions/check.c
index 87c23e3..78be967 100644
--- a/fs/partitions/check.c
+++ b/fs/partitions/check.c
@@ -342,6 +342,7 @@ void delete_partition(struct gendisk *disk, int partno)
if (!part)
return;
+ blk_free_devt(part_devt(part));
rcu_assign_pointer(disk->__part[partno-1], NULL);
kobject_put(part->holder_dir);
device_del(&part->dev);
@@ -361,6 +362,7 @@ int add_partition(struct gendisk *disk, int partno,
sector_t start, sector_t len, int flags)
{
struct hd_struct *p = NULL;
+ dev_t devt = MKDEV(0, 0);
int err;
if (disk->__part[partno - 1]) {
@@ -388,10 +390,15 @@ int add_partition(struct gendisk *disk, int partno,
"%s%d", disk->dev.bus_id, partno);
device_initialize(&p->dev);
- p->dev.devt = MKDEV(disk->major, disk->first_minor + partno);
p->dev.class = &block_class;
p->dev.type = &part_type;
p->dev.parent = &disk->dev;
+
+ err = blk_alloc_devt(p, &devt);
+ if (err)
+ goto fail;
+ p->dev.devt = devt;
+
INIT_RCU_HEAD(&p->rcu_head);
rcu_assign_pointer(disk->__part[partno-1], p);
@@ -416,6 +423,7 @@ int add_partition(struct gendisk *disk, int partno,
return 0;
fail:
+ blk_free_devt(devt);
if (p)
free_part_stats(p);
kfree(p);
diff --git a/include/linux/genhd.h b/include/linux/genhd.h
index 7737468..cb6d9a7 100644
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -114,13 +114,15 @@ struct hd_struct {
struct gendisk {
- /* major, first_minor and minors are input parameters only,
- * don't use directly. Use disk_devt() and disk_max_parts().
+ /* major, first_minor, minors and ext_minors are input
+ * parameters only, don't use directly. Use disk_devt() and
+ * disk_max_parts().
*/
int major; /* major number of driver */
int first_minor;
int minors; /* maximum number of minors, =1 for
* disks that can't be partitioned. */
+ int ext_minors; /* number of extended dynamic minors */
char disk_name[32]; /* name of major driver */
@@ -165,7 +167,7 @@ static inline struct gendisk *part_to_disk(struct hd_struct *part)
static inline int disk_max_parts(struct gendisk *disk)
{
- return disk->minors - 1;
+ return disk->minors + disk->ext_minors - 1;
}
static inline dev_t disk_devt(struct gendisk *disk)
@@ -540,6 +542,8 @@ struct unixware_disklabel {
#define ADDPART_FLAG_RAID 1
#define ADDPART_FLAG_WHOLEDISK 2
+extern int blk_alloc_devt(struct hd_struct *part, dev_t *devt);
+extern void blk_free_devt(dev_t devt);
extern dev_t blk_lookup_devt(const char *name, int partno);
extern char *disk_name (struct gendisk *hd, int partno, char *buf);
@@ -550,6 +554,9 @@ extern void printk_all_partitions(void);
extern struct gendisk *alloc_disk_node(int minors, int node_id);
extern struct gendisk *alloc_disk(int minors);
+extern struct gendisk *alloc_disk_ext_node(int minors, int ext_minrs,
+ int node_id);
+extern struct gendisk *alloc_disk_ext(int minors, int ext_minors);
extern struct kobject *get_disk(struct gendisk *disk);
extern void put_disk(struct gendisk *disk);
extern void blk_register_region(dev_t devt, unsigned long range,
diff --git a/include/linux/major.h b/include/linux/major.h
index 0cb9805..d62fd67 100644
--- a/include/linux/major.h
+++ b/include/linux/major.h
@@ -170,4 +170,6 @@
#define VIOTAPE_MAJOR 230
+#define BLOCK_EXT_MAJOR 259
+
#endif
--
1.5.4.5
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 08/10] block: adjust formatting for large minors and add ext_range sysfs attr
2008-07-14 7:12 [PATCHSET 2.6.26] block: implement extended devt, take #2 Tejun Heo
` (6 preceding siblings ...)
2008-07-14 7:12 ` [PATCH 07/10] block: implement extended dev numbers Tejun Heo
@ 2008-07-14 7:12 ` Tejun Heo
2008-07-14 7:12 ` [PATCH 09/10] sd/ide-disk: apply extended minors to sd and ide Tejun Heo
` (3 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Tejun Heo @ 2008-07-14 7:12 UTC (permalink / raw)
To: jens.axboe, James.Bottomley, bharrosh, greg.freemyer, linux-scsi,
brking, liml, viro
Cc: Tejun Heo
With extended minors and the soon-to-follow debug feature, large minor
numbers for block devices will be common. This patch does the
followings to make printouts pretty.
* Adapt print formats such that large minors don't break the
formatting.
* For extended MAJ:MIN, %02x%02x for MAJ:MIN used in
printk_all_partitions() doesn't cut it anymore. Update it such that
%03x:%05x is used if either MAJ or MIN doesn't fit in %02x.
* Implement ext_range sysfs attribute which shows total minors the
device can use including both conventional minor space and the
extended one.
Signed-off-by: Tejun Heo <tj@kernel.org>
---
block/genhd.c | 47 +++++++++++++++++++++++++++++++++++------------
include/linux/fs.h | 1 +
2 files changed, 36 insertions(+), 12 deletions(-)
diff --git a/block/genhd.c b/block/genhd.c
index 2511694..1b8ca16 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -365,6 +365,18 @@ void blk_free_devt(dev_t devt)
}
}
+static char *bdevt_str(dev_t devt, char *buf)
+{
+ if (MAJOR(devt) <= 0xff && MINOR(devt) <= 0xff) {
+ char tbuf[BDEVT_SIZE];
+ snprintf(tbuf, BDEVT_SIZE, "%02x%02x", MAJOR(devt), MINOR(devt));
+ snprintf(buf, BDEVT_SIZE, "%-9s", tbuf);
+ } else
+ snprintf(buf, BDEVT_SIZE, "%03x:%05x", MAJOR(devt), MINOR(devt));
+
+ return buf;
+}
+
/*
* Register device numbers dev..(dev+range-1)
* range must be nonzero
@@ -513,7 +525,8 @@ void __init printk_all_partitions(void)
struct gendisk *sgp;
struct disk_part_iter piter;
struct hd_struct *part;
- char buf[BDEVNAME_SIZE];
+ char name_buf[BDEVNAME_SIZE];
+ char devt_buf[BDEVT_SIZE];
down(&block_class.sem);
/* For each block device... */
@@ -532,10 +545,10 @@ void __init printk_all_partitions(void)
* Note, unlike /proc/partitions, I am showing the numbers in
* hex - the same format as the root= option takes.
*/
- printk("%02x%02x %10llu %s",
- MAJOR(disk_devt(sgp)), MINOR(disk_devt(sgp)),
+ printk("%s %10llu %s",
+ bdevt_str(disk_devt(sgp), devt_buf),
(unsigned long long)get_capacity(sgp) >> 1,
- disk_name(sgp, 0, buf));
+ disk_name(sgp, 0, name_buf));
if (sgp->driverfs_dev != NULL &&
sgp->driverfs_dev->driver != NULL)
printk(" driver: %s\n",
@@ -546,10 +559,10 @@ void __init printk_all_partitions(void)
/* now show the partitions */
disk_part_iter_start(&piter, sgp, 0);
while ((part = disk_part_iter_next(&piter)))
- printk(" %02x%02x %10llu %s\n",
- MAJOR(part_devt(part)), MINOR(part_devt(part)),
+ printk(" %s %10llu %s\n",
+ bdevt_str(part_devt(part), devt_buf),
(unsigned long long)part->nr_sects >> 1,
- disk_name(sgp, part->partno, buf));
+ disk_name(sgp, part->partno, name_buf));
disk_part_iter_stop(&piter);
}
@@ -600,7 +613,7 @@ static int show_partition(struct seq_file *seqf, void *v)
char buf[BDEVNAME_SIZE];
if (&sgp->dev.node == block_class.devices.next)
- seq_puts(seqf, "major minor #blocks name\n\n");
+ seq_puts(seqf, "major minor #blocks name\n\n");
/* Don't show non-partitionable removeable devices or empty devices */
if (!get_capacity(sgp) || (!disk_max_parts(sgp) &&
@@ -610,14 +623,14 @@ static int show_partition(struct seq_file *seqf, void *v)
return 0;
/* show the full disk and all non-0 size partitions of it */
- seq_printf(seqf, "%4d %4d %10llu %s\n",
+ seq_printf(seqf, "%4d %7d %10llu %s\n",
MAJOR(disk_devt(sgp)), MINOR(disk_devt(sgp)),
(unsigned long long)get_capacity(sgp) >> 1,
disk_name(sgp, 0, buf));
disk_part_iter_start(&piter, sgp, 0);
while ((part = disk_part_iter_next(&piter)))
- seq_printf(seqf, "%4d %4d %10llu %s\n",
+ seq_printf(seqf, "%4d %7d %10llu %s\n",
MAJOR(part_devt(part)), MINOR(part_devt(part)),
(unsigned long long)part->nr_sects >> 1,
disk_name(sgp, part->partno, buf));
@@ -668,6 +681,14 @@ static ssize_t disk_range_show(struct device *dev,
return sprintf(buf, "%d\n", disk->minors);
}
+static ssize_t disk_ext_range_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct gendisk *disk = dev_to_disk(dev);
+
+ return sprintf(buf, "%d\n", disk_max_parts(disk) + 1);
+}
+
static ssize_t disk_removable_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
@@ -748,6 +769,7 @@ static ssize_t disk_fail_store(struct device *dev,
#endif
static DEVICE_ATTR(range, S_IRUGO, disk_range_show, NULL);
+static DEVICE_ATTR(ext_range, S_IRUGO, disk_ext_range_show, NULL);
static DEVICE_ATTR(removable, S_IRUGO, disk_removable_show, NULL);
static DEVICE_ATTR(size, S_IRUGO, disk_size_show, NULL);
static DEVICE_ATTR(capability, S_IRUGO, disk_capability_show, NULL);
@@ -759,6 +781,7 @@ static struct device_attribute dev_attr_fail =
static struct attribute *disk_attrs[] = {
&dev_attr_range.attr,
+ &dev_attr_ext_range.attr,
&dev_attr_removable.attr,
&dev_attr_size.attr,
&dev_attr_capability.attr,
@@ -858,7 +881,7 @@ static int diskstats_show(struct seq_file *seqf, void *v)
preempt_disable();
disk_round_stats(gp);
preempt_enable();
- seq_printf(seqf, "%4d %4d %s %lu %lu %llu %u %lu %lu %llu %u %u %u %u\n",
+ seq_printf(seqf, "%4d %7d %s %lu %lu %llu %u %lu %lu %llu %u %u %u %u\n",
MAJOR(disk_devt(gp)), MINOR(disk_devt(gp)),
disk_name(gp, 0, buf),
disk_stat_read(gp, ios[0]), disk_stat_read(gp, merges[0]),
@@ -877,7 +900,7 @@ static int diskstats_show(struct seq_file *seqf, void *v)
preempt_disable();
part_round_stats(hd);
preempt_enable();
- seq_printf(seqf, "%4d %4d %s %lu %lu %llu "
+ seq_printf(seqf, "%4d %7d %s %lu %lu %llu "
"%u %lu %lu %llu %u %u %u %u\n",
MAJOR(part_devt(hd)), MINOR(part_devt(hd)),
disk_name(gp, hd->partno, buf),
diff --git a/include/linux/fs.h b/include/linux/fs.h
index d8e2762..8337243 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1667,6 +1667,7 @@ extern void chrdev_show(struct seq_file *,off_t);
/* fs/block_dev.c */
#define BDEVNAME_SIZE 32 /* Largest string for a blockdev identifier */
+#define BDEVT_SIZE 10 /* Largest string for MAJ:MIN for blkdev */
#ifdef CONFIG_BLOCK
#define BLKDEV_MAJOR_HASH_SIZE 255
--
1.5.4.5
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 09/10] sd/ide-disk: apply extended minors to sd and ide
2008-07-14 7:12 [PATCHSET 2.6.26] block: implement extended devt, take #2 Tejun Heo
` (7 preceding siblings ...)
2008-07-14 7:12 ` [PATCH 08/10] block: adjust formatting for large minors and add ext_range sysfs attr Tejun Heo
@ 2008-07-14 7:12 ` Tejun Heo
2008-07-14 7:12 ` [PATCH 10/10] block: implement CONFIG_DEBUG_BLOCK_EXT_DEVT Tejun Heo
` (2 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Tejun Heo @ 2008-07-14 7:12 UTC (permalink / raw)
To: jens.axboe, James.Bottomley, bharrosh, greg.freemyer, linux-scsi,
brking, liml, viro
Cc: Tejun Heo
Update sd and ide-disk such that they can take advantage of extended
minors.
ide-disk already has 64 minors per device and currently doesn't use
extended minors although after this patch it can be turned on by
simply tweaking constants.
sd only had 16 minors per device causing problems on certain peculiar
configurations. This patch lifts the restriction and enables it to
use upto 64 minors.
Signed-off-by: Tejun Heo <tj@kernel.org>
---
drivers/ide/ide-disk.c | 11 ++++++++---
drivers/scsi/sd.c | 9 +++++++--
2 files changed, 15 insertions(+), 5 deletions(-)
diff --git a/drivers/ide/ide-disk.c b/drivers/ide/ide-disk.c
index 8e08d08..f8b091a 100644
--- a/drivers/ide/ide-disk.c
+++ b/drivers/ide/ide-disk.c
@@ -41,6 +41,10 @@
#include <asm/io.h>
#include <asm/div64.h>
+#define IDE_DISK_PARTS (1 << PARTN_BITS)
+#define IDE_DISK_MINORS IDE_DISK_PARTS
+#define IDE_DISK_EXT_MINORS (IDE_DISK_PARTS - IDE_DISK_MINORS)
+
struct ide_disk_obj {
ide_drive_t *drive;
ide_driver_t *driver;
@@ -1158,8 +1162,8 @@ static int ide_disk_probe(ide_drive_t *drive)
if (!idkp)
goto failed;
- g = alloc_disk_node(1 << PARTN_BITS,
- hwif_to_node(drive->hwif));
+ g = alloc_disk_ext_node(IDE_DISK_MINORS, IDE_DISK_EXT_MINORS,
+ hwif_to_node(drive->hwif));
if (!g)
goto out_free_idkp;
@@ -1185,7 +1189,8 @@ static int ide_disk_probe(ide_drive_t *drive)
} else
drive->attach = 1;
- g->minors = 1 << PARTN_BITS;
+ g->minors = IDE_DISK_MINORS;
+ g->ext_minors = IDE_DISK_EXT_MINORS;
g->driverfs_dev = &drive->gendev;
g->flags = drive->removable ? GENHD_FL_REMOVABLE : 0;
set_capacity(g, idedisk_capacity(drive));
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index c330f16..0a8b559 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -86,6 +86,10 @@ MODULE_ALIAS_SCSI_DEVICE(TYPE_DISK);
MODULE_ALIAS_SCSI_DEVICE(TYPE_MOD);
MODULE_ALIAS_SCSI_DEVICE(TYPE_RBC);
+#define SD_PARTS 64
+#define SD_MINORS 16
+#define SD_EXT_MINORS (SD_PARTS - SD_MINORS)
+
static int sd_revalidate_disk(struct gendisk *);
static int sd_probe(struct device *);
static int sd_remove(struct device *);
@@ -1641,7 +1645,7 @@ static int sd_probe(struct device *dev)
if (!sdkp)
goto out;
- gd = alloc_disk(16);
+ gd = alloc_disk_ext(SD_MINORS, SD_EXT_MINORS);
if (!gd)
goto out_free;
@@ -1685,7 +1689,8 @@ static int sd_probe(struct device *dev)
gd->major = sd_major((index & 0xf0) >> 4);
gd->first_minor = ((index & 0xf) << 4) | (index & 0xfff00);
- gd->minors = 16;
+ gd->minors = SD_MINORS;
+ gd->ext_minors = SD_EXT_MINORS;
gd->fops = &sd_fops;
if (index < 26) {
--
1.5.4.5
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 10/10] block: implement CONFIG_DEBUG_BLOCK_EXT_DEVT
2008-07-14 7:12 [PATCHSET 2.6.26] block: implement extended devt, take #2 Tejun Heo
` (8 preceding siblings ...)
2008-07-14 7:12 ` [PATCH 09/10] sd/ide-disk: apply extended minors to sd and ide Tejun Heo
@ 2008-07-14 7:12 ` Tejun Heo
2008-07-14 7:15 ` [PATCHSET 2.6.26] block: implement extended devt, take #2 Tejun Heo
[not found] ` <1216019534-29977-6-git-send-email-tj@kernel.org>
11 siblings, 0 replies; 13+ messages in thread
From: Tejun Heo @ 2008-07-14 7:12 UTC (permalink / raw)
To: jens.axboe, James.Bottomley, bharrosh, greg.freemyer, linux-scsi,
brking, liml, viro
Cc: Tejun Heo
Extended devt introduces non-contiguos device numbers. This patch
implements a debug option which forces most devt allocations to be
from the extended area and spreads them out. This is enabled by
default if DEBUG_KERNEL is set and achieves...
1. Detects code paths in kernel or userland which expect predetermined
consecutive device numbers.
2. When something goes wrong, avoid corruption as adding to the minor
of earlier partition won't lead to the wrong but valid device.
Signed-off-by: Tejun Heo <tj@kernel.org>
---
block/genhd.c | 38 +++++++++++++++++++++++++++++++++++---
drivers/ide/ide-disk.c | 6 ++++++
drivers/scsi/sd.c | 6 ++++++
lib/Kconfig.debug | 16 ++++++++++++++++
4 files changed, 63 insertions(+), 3 deletions(-)
diff --git a/block/genhd.c b/block/genhd.c
index 1b8ca16..e2deec2 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -299,6 +299,38 @@ EXPORT_SYMBOL(unregister_blkdev);
static struct kobj_map *bdev_map;
/**
+ * blk_mangle_minor - scatter minor numbers apart
+ * @minor: minor number to mangle
+ *
+ * Scatter consecutively allocated @minor number apart if MANGLE_DEVT
+ * is enabled. Mangling twice gives the original value.
+ *
+ * RETURNS:
+ * Mangled value.
+ *
+ * CONTEXT:
+ * Don't care.
+ */
+static int blk_mangle_minor(int minor)
+{
+#ifdef CONFIG_DEBUG_BLOCK_EXT_DEVT
+ int i;
+
+ for (i = 0; i < MINORBITS / 2; i++) {
+ int low = minor & (1 << i);
+ int high = minor & (1 << (MINORBITS - 1 - i));
+ int distance = MINORBITS - 1 - 2 * i;
+
+ minor ^= low | high; /* clear both bits */
+ low <<= distance; /* swap the positions */
+ high >>= distance;
+ minor |= low | high; /* and set */
+ }
+#endif
+ return minor;
+}
+
+/**
* blk_alloc_devt - allocate a dev_t for a partition
* @part: partition to allocate dev_t for
* @gfp_mask: memory allocation flag
@@ -338,7 +370,7 @@ int blk_alloc_devt(struct hd_struct *part, dev_t *devt)
return -EBUSY;
}
- *devt = MKDEV(BLOCK_EXT_MAJOR, idx);
+ *devt = MKDEV(BLOCK_EXT_MAJOR, blk_mangle_minor(idx));
return 0;
}
@@ -360,7 +392,7 @@ void blk_free_devt(dev_t devt)
if (MAJOR(devt) == BLOCK_EXT_MAJOR) {
mutex_lock(&ext_devt_mutex);
- idr_remove(&ext_devt_idr, MINOR(devt));
+ idr_remove(&ext_devt_idr, blk_mangle_minor(MINOR(devt)));
mutex_unlock(&ext_devt_mutex);
}
}
@@ -469,7 +501,7 @@ struct gendisk *get_gendisk(dev_t devt, int *partno)
struct hd_struct *part;
mutex_lock(&ext_devt_mutex);
- part = idr_find(&ext_devt_idr, MINOR(devt));
+ part = idr_find(&ext_devt_idr, blk_mangle_minor(MINOR(devt)));
if (part && get_disk(part_to_disk(part))) {
*partno = part->partno;
disk = part_to_disk(part);
diff --git a/drivers/ide/ide-disk.c b/drivers/ide/ide-disk.c
index f8b091a..3c6f0fa 100644
--- a/drivers/ide/ide-disk.c
+++ b/drivers/ide/ide-disk.c
@@ -42,7 +42,13 @@
#include <asm/div64.h>
#define IDE_DISK_PARTS (1 << PARTN_BITS)
+
+#if !defined(CONFIG_DEBUG_BLOCK_EXT_DEVT)
#define IDE_DISK_MINORS IDE_DISK_PARTS
+#else
+#define IDE_DISK_MINORS 1
+#endif
+
#define IDE_DISK_EXT_MINORS (IDE_DISK_PARTS - IDE_DISK_MINORS)
struct ide_disk_obj {
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 0a8b559..f355a39 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -87,7 +87,13 @@ MODULE_ALIAS_SCSI_DEVICE(TYPE_MOD);
MODULE_ALIAS_SCSI_DEVICE(TYPE_RBC);
#define SD_PARTS 64
+
+#if !defined(CONFIG_DEBUG_BLOCK_EXT_DEVT)
#define SD_MINORS 16
+#else
+#define SD_MINORS 1
+#endif
+
#define SD_EXT_MINORS (SD_PARTS - SD_MINORS)
static int sd_revalidate_disk(struct gendisk *);
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index d2099f4..46bc380 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -565,6 +565,22 @@ config BACKTRACE_SELF_TEST
Say N if you are unsure.
+config DEBUG_BLOCK_EXT_DEVT
+ bool "Force extended block device numbers and spread them"
+ depends on DEBUG_KERNEL
+ depends on BLOCK
+ default y
+ help
+ Conventionally, block device numbers are allocated from
+ predetermined contiguous area. However, extended block area
+ may introduce non-contiguous block device numbers. This
+ option forces most block device numbers to be allocated from
+ the extended space and spreads them to discover kernel or
+ userland code paths which assume predetermined contiguous
+ device number allocation.
+
+ Say N if you are unsure.
+
config LKDTM
tristate "Linux Kernel Dump Test Tool Module"
depends on DEBUG_KERNEL
--
1.5.4.5
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCHSET 2.6.26] block: implement extended devt, take #2
2008-07-14 7:12 [PATCHSET 2.6.26] block: implement extended devt, take #2 Tejun Heo
` (9 preceding siblings ...)
2008-07-14 7:12 ` [PATCH 10/10] block: implement CONFIG_DEBUG_BLOCK_EXT_DEVT Tejun Heo
@ 2008-07-14 7:15 ` Tejun Heo
[not found] ` <1216019534-29977-6-git-send-email-tj@kernel.org>
11 siblings, 0 replies; 13+ messages in thread
From: Tejun Heo @ 2008-07-14 7:15 UTC (permalink / raw)
To: jens.axboe, James.Bottomley, bharrosh, greg.freemyer, linux-scsi,
brking, liml, viro
Oops, missed one prerequisite patch.
Tejun Heo wrote:
> v2.6.26 (bce7f793daec3e65ec5c5705d2457b81fe7b5725)
> + [1] RCU-implement-rcu_read_[un]lock_preempt
> + [2] block-fix-block_class-iteration-locking
> + [3] block-allow-deleting-zero-lenght-partition
> + [4] block-update-add_partition-error-handling
+ [5] sd-update-index-allocation-and-use-ida
> [L] http://thread.gmane.org/gmane.linux.kernel/701825
> [1] http://article.gmane.org/gmane.linux.kernel/706149
> [2] http://article.gmane.org/gmane.linux.kernel/706152
> [3] http://article.gmane.org/gmane.linux.kernel/706153
> [4] http://article.gmane.org/gmane.linux.kernel/706154
[5] http://article.gmane.org/gmane.linux.scsi/43109
The git tree has everything included.
Thanks.
--
tejun
^ permalink raw reply [flat|nested] 13+ messages in thread[parent not found: <1216019534-29977-6-git-send-email-tj@kernel.org>]
* Re: [PATCH 05/10] block: always use __{disk|part|all}_stat_*() and kill non-underbarred versions
[not found] ` <1216019534-29977-6-git-send-email-tj@kernel.org>
@ 2008-07-16 6:19 ` Peter Zijlstra
0 siblings, 0 replies; 13+ messages in thread
From: Peter Zijlstra @ 2008-07-16 6:19 UTC (permalink / raw)
To: Tejun Heo
Cc: jens.axboe, James.Bottomley, bharrosh, greg.freemyer, linux-scsi,
brking, liml, viro, linux-kernel, linux-ide, Paul E McKenney,
Ingo Molnar
On Mon, 2008-07-14 at 16:12 +0900, Tejun Heo wrote:
> There are two variants of stat functions - ones prefixed with double
> underbars which don't care about preemption and ones without which
> disable preemption before manipulating per-cpu counters. It's unclear
> whether the underbarred ones assume that preemtion is disabled on
> entry as some callers don't do that. In any case, stats are not
> critical data and errors don't lead to critical failures.
>
> However, consistently using the underbarred version and ensuring that
> they are called with preemption disabled doesn't incur noticeable
> overhead or even reduces overhead in some cases.
>
> * part stat manipulations need disk_map_sector_rcu() which involves
> read locking RCU anyway. Using rcu_read_lock_preempt() instead
> solves the problem nicely. On rcuclassic, this means no extra
> overhead.
>
> * Calls to the non-underbarred versions are converted to explicit
> preemtion disable and calls to respective underbarrded versions. As
> all such users had more than one consecutive stat ops, this reduces
> extra preemption disable/enables.
>
> * In drivers/md/dm.c::end_io_acct(), __disk_stat_add() call is moved
> into neighboring preemption disabled block.
>
> The conversion makes the stats usage more consistent and IMHO the
> added few preemption calls are well justified for that.
>
> As this change makes non-underbarred versions useless, non-underbarred
> stat functions and macros are killed. The next patch will drop
> underbars from the underbarred versions as it's superflous now. This
> is done as a separate step so that compile fails between this and the
> next patch if someone's left behind.
Aah, found what you use it for...
See, you need the preempt-off for something different than the RCU
usage, hence it doesn't make sense to mix that in. Use the regular
get_cpu/put_cpu stuff to fiddle with the percpu counters already.
So NAK on this one too.
> Signed-off-by: Tejun Heo <tj@kernel.org>
> ---
> block/blk-core.c | 16 +++++++-------
> block/blk-merge.c | 4 +-
> drivers/block/aoe/aoecmd.c | 12 +++++-----
> drivers/md/dm.c | 10 +++++---
> drivers/md/linear.c | 6 +++-
> drivers/md/multipath.c | 6 +++-
> drivers/md/raid0.c | 6 +++-
> drivers/md/raid1.c | 6 +++-
> drivers/md/raid10.c | 6 +++-
> drivers/md/raid5.c | 6 +++-
> include/linux/genhd.h | 48 --------------------------------------------
> 11 files changed, 46 insertions(+), 80 deletions(-)
>
> diff --git a/block/blk-core.c b/block/blk-core.c
> index 84292e1..3238ffe 100644
> --- a/block/blk-core.c
> +++ b/block/blk-core.c
> @@ -60,7 +60,7 @@ static void drive_stat_acct(struct request *rq, int new_io)
> if (!blk_fs_request(rq) || !rq->rq_disk)
> return;
>
> - rcu_read_lock();
> + rcu_read_lock_preempt();
>
> part = disk_map_sector_rcu(rq->rq_disk, rq->sector);
> if (!new_io)
> @@ -74,7 +74,7 @@ static void drive_stat_acct(struct request *rq, int new_io)
> }
> }
>
> - rcu_read_unlock();
> + rcu_read_unlock_preempt();
> }
>
> void blk_queue_congestion_threshold(struct request_queue *q)
> @@ -1542,11 +1542,11 @@ static int __end_that_request_first(struct request *req, int error,
> const int rw = rq_data_dir(req);
> struct hd_struct *part;
>
> - rcu_read_lock();
> + rcu_read_lock_preempt();
> part = disk_map_sector_rcu(req->rq_disk, req->sector);
> - all_stat_add(req->rq_disk, part, sectors[rw],
> - nr_bytes >> 9, req->sector);
> - rcu_read_unlock();
> + __all_stat_add(req->rq_disk, part, sectors[rw],
> + nr_bytes >> 9, req->sector);
> + rcu_read_unlock_preempt();
> }
>
> total_bytes = bio_nbytes = 0;
> @@ -1732,7 +1732,7 @@ static void end_that_request_last(struct request *req, int error)
> const int rw = rq_data_dir(req);
> struct hd_struct *part;
>
> - rcu_read_lock();
> + rcu_read_lock_preempt();
>
> part = disk_map_sector_rcu(disk, req->sector);
>
> @@ -1745,7 +1745,7 @@ static void end_that_request_last(struct request *req, int error)
> part->in_flight--;
> }
>
> - rcu_read_unlock();
> + rcu_read_unlock_preempt();
> }
>
> if (req->end_io)
> diff --git a/block/blk-merge.c b/block/blk-merge.c
> index 9be9b2f..55456c8 100644
> --- a/block/blk-merge.c
> +++ b/block/blk-merge.c
> @@ -469,7 +469,7 @@ static int attempt_merge(struct request_queue *q, struct request *req,
> if (req->rq_disk) {
> struct hd_struct *part;
>
> - rcu_read_lock();
> + rcu_read_lock_preempt();
>
> part = disk_map_sector_rcu(req->rq_disk, req->sector);
> disk_round_stats(req->rq_disk);
> @@ -479,7 +479,7 @@ static int attempt_merge(struct request_queue *q, struct request *req,
> part->in_flight--;
> }
>
> - rcu_read_unlock();
> + rcu_read_unlock_preempt();
> }
>
> req->ioprio = ioprio_best(req->ioprio, next->ioprio);
> diff --git a/drivers/block/aoe/aoecmd.c b/drivers/block/aoe/aoecmd.c
> index 50ef9ea..7bd8c98 100644
> --- a/drivers/block/aoe/aoecmd.c
> +++ b/drivers/block/aoe/aoecmd.c
> @@ -757,15 +757,15 @@ diskstats(struct gendisk *disk, struct bio *bio, ulong duration, sector_t sector
> const int rw = bio_data_dir(bio);
> struct hd_struct *part;
>
> - rcu_read_lock();
> + rcu_read_lock_preempt();
>
> part = disk_map_sector_rcu(disk, sector);
> - all_stat_inc(disk, part, ios[rw], sector);
> - all_stat_add(disk, part, ticks[rw], duration, sector);
> - all_stat_add(disk, part, sectors[rw], n_sect, sector);
> - all_stat_add(disk, part, io_ticks, duration, sector);
> + __all_stat_inc(disk, part, ios[rw], sector);
> + __all_stat_add(disk, part, ticks[rw], duration, sector);
> + __all_stat_add(disk, part, sectors[rw], n_sect, sector);
> + __all_stat_add(disk, part, io_ticks, duration, sector);
>
> - rcu_read_unlock();
> + rcu_read_unlock_preempt();
> }
>
> void
> diff --git a/drivers/md/dm.c b/drivers/md/dm.c
> index 923fea4..6918bb7 100644
> --- a/drivers/md/dm.c
> +++ b/drivers/md/dm.c
> @@ -396,10 +396,10 @@ static int end_io_acct(struct dm_io *io)
>
> preempt_disable();
> disk_round_stats(dm_disk(md));
> + __disk_stat_add(dm_disk(md), ticks[rw], duration);
> preempt_enable();
> - dm_disk(md)->in_flight = pending = atomic_dec_return(&md->pending);
>
> - disk_stat_add(dm_disk(md), ticks[rw], duration);
> + dm_disk(md)->in_flight = pending = atomic_dec_return(&md->pending);
>
> return !pending;
> }
> @@ -850,8 +850,10 @@ static int dm_request(struct request_queue *q, struct bio *bio)
>
> down_read(&md->io_lock);
>
> - disk_stat_inc(dm_disk(md), ios[rw]);
> - disk_stat_add(dm_disk(md), sectors[rw], bio_sectors(bio));
> + preempt_disable();
> + __disk_stat_inc(dm_disk(md), ios[rw]);
> + __disk_stat_add(dm_disk(md), sectors[rw], bio_sectors(bio));
> + preempt_enable();
>
> /*
> * If we're suspended we have to queue
> diff --git a/drivers/md/linear.c b/drivers/md/linear.c
> index 1074824..ec35b8b 100644
> --- a/drivers/md/linear.c
> +++ b/drivers/md/linear.c
> @@ -322,8 +322,10 @@ static int linear_make_request (struct request_queue *q, struct bio *bio)
> return 0;
> }
>
> - disk_stat_inc(mddev->gendisk, ios[rw]);
> - disk_stat_add(mddev->gendisk, sectors[rw], bio_sectors(bio));
> + preempt_disable();
> + __disk_stat_inc(mddev->gendisk, ios[rw]);
> + __disk_stat_add(mddev->gendisk, sectors[rw], bio_sectors(bio));
> + preempt_enable();
>
> tmp_dev = which_dev(mddev, bio->bi_sector);
> block = bio->bi_sector >> 1;
> diff --git a/drivers/md/multipath.c b/drivers/md/multipath.c
> index e968116..aed8ea9 100644
> --- a/drivers/md/multipath.c
> +++ b/drivers/md/multipath.c
> @@ -158,8 +158,10 @@ static int multipath_make_request (struct request_queue *q, struct bio * bio)
> mp_bh->master_bio = bio;
> mp_bh->mddev = mddev;
>
> - disk_stat_inc(mddev->gendisk, ios[rw]);
> - disk_stat_add(mddev->gendisk, sectors[rw], bio_sectors(bio));
> + preempt_disable();
> + __disk_stat_inc(mddev->gendisk, ios[rw]);
> + __disk_stat_add(mddev->gendisk, sectors[rw], bio_sectors(bio));
> + preempt_enable();
>
> mp_bh->path = multipath_map(conf);
> if (mp_bh->path < 0) {
> diff --git a/drivers/md/raid0.c b/drivers/md/raid0.c
> index 914c04d..d0cdfe1 100644
> --- a/drivers/md/raid0.c
> +++ b/drivers/md/raid0.c
> @@ -403,8 +403,10 @@ static int raid0_make_request (struct request_queue *q, struct bio *bio)
> return 0;
> }
>
> - disk_stat_inc(mddev->gendisk, ios[rw]);
> - disk_stat_add(mddev->gendisk, sectors[rw], bio_sectors(bio));
> + preempt_disable();
> + __disk_stat_inc(mddev->gendisk, ios[rw]);
> + __disk_stat_add(mddev->gendisk, sectors[rw], bio_sectors(bio));
> + preempt_enable();
>
> chunk_size = mddev->chunk_size >> 10;
> chunk_sects = mddev->chunk_size >> 9;
> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
> index c610b94..6687ffe 100644
> --- a/drivers/md/raid1.c
> +++ b/drivers/md/raid1.c
> @@ -804,8 +804,10 @@ static int make_request(struct request_queue *q, struct bio * bio)
>
> bitmap = mddev->bitmap;
>
> - disk_stat_inc(mddev->gendisk, ios[rw]);
> - disk_stat_add(mddev->gendisk, sectors[rw], bio_sectors(bio));
> + preempt_disable();
> + __disk_stat_inc(mddev->gendisk, ios[rw]);
> + __disk_stat_add(mddev->gendisk, sectors[rw], bio_sectors(bio));
> + preempt_enable();
>
> /*
> * make_request() can abort the operation when READA is being
> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
> index a71277b..1d644dc 100644
> --- a/drivers/md/raid10.c
> +++ b/drivers/md/raid10.c
> @@ -838,8 +838,10 @@ static int make_request(struct request_queue *q, struct bio * bio)
> */
> wait_barrier(conf);
>
> - disk_stat_inc(mddev->gendisk, ios[rw]);
> - disk_stat_add(mddev->gendisk, sectors[rw], bio_sectors(bio));
> + preempt_disable();
> + __disk_stat_inc(mddev->gendisk, ios[rw]);
> + __disk_stat_add(mddev->gendisk, sectors[rw], bio_sectors(bio));
> + preempt_enable();
>
> r10_bio = mempool_alloc(conf->r10bio_pool, GFP_NOIO);
>
> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index 3b27df5..a869e58 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
> @@ -3580,8 +3580,10 @@ static int make_request(struct request_queue *q, struct bio * bi)
>
> md_write_start(mddev, bi);
>
> - disk_stat_inc(mddev->gendisk, ios[rw]);
> - disk_stat_add(mddev->gendisk, sectors[rw], bio_sectors(bi));
> + preempt_disable();
> + __disk_stat_inc(mddev->gendisk, ios[rw]);
> + __disk_stat_add(mddev->gendisk, sectors[rw], bio_sectors(bi));
> + preempt_enable();
>
> if (rw == READ &&
> mddev->reshape_position == MaxSector &&
> diff --git a/include/linux/genhd.h b/include/linux/genhd.h
> index 3e35945..87a338b 100644
> --- a/include/linux/genhd.h
> +++ b/include/linux/genhd.h
> @@ -207,13 +207,6 @@ extern void disk_part_iter_stop(struct disk_part_iter *piter);
> extern struct hd_struct *disk_map_sector_rcu(struct gendisk *disk,
> sector_t sector);
>
> -/*
> - * Macros to operate on percpu disk statistics:
> - *
> - * The __ variants should only be called in critical sections. The full
> - * variants disable/enable preemption.
> - */
> -
> #ifdef CONFIG_SMP
> #define __disk_stat_add(gendiskp, field, addnd) \
> (per_cpu_ptr(gendiskp->dkstats, smp_processor_id())->field += addnd)
> @@ -292,63 +285,22 @@ static inline void part_stat_set_all(struct hd_struct *part, int value)
>
> #endif /* CONFIG_SMP */
>
> -#define disk_stat_add(gendiskp, field, addnd) \
> - do { \
> - preempt_disable(); \
> - __disk_stat_add(gendiskp, field, addnd); \
> - preempt_enable(); \
> - } while (0)
> -
> #define __disk_stat_dec(gendiskp, field) __disk_stat_add(gendiskp, field, -1)
> -#define disk_stat_dec(gendiskp, field) disk_stat_add(gendiskp, field, -1)
> -
> #define __disk_stat_inc(gendiskp, field) __disk_stat_add(gendiskp, field, 1)
> -#define disk_stat_inc(gendiskp, field) disk_stat_add(gendiskp, field, 1)
> -
> #define __disk_stat_sub(gendiskp, field, subnd) \
> __disk_stat_add(gendiskp, field, -subnd)
> -#define disk_stat_sub(gendiskp, field, subnd) \
> - disk_stat_add(gendiskp, field, -subnd)
> -
> -#define part_stat_add(gendiskp, field, addnd) \
> - do { \
> - preempt_disable(); \
> - __part_stat_add(gendiskp, field, addnd);\
> - preempt_enable(); \
> - } while (0)
>
> #define __part_stat_dec(gendiskp, field) __part_stat_add(gendiskp, field, -1)
> -#define part_stat_dec(gendiskp, field) part_stat_add(gendiskp, field, -1)
> -
> #define __part_stat_inc(gendiskp, field) __part_stat_add(gendiskp, field, 1)
> -#define part_stat_inc(gendiskp, field) part_stat_add(gendiskp, field, 1)
> -
> #define __part_stat_sub(gendiskp, field, subnd) \
> __part_stat_add(gendiskp, field, -subnd)
> -#define part_stat_sub(gendiskp, field, subnd) \
> - part_stat_add(gendiskp, field, -subnd)
> -
> -#define all_stat_add(gendiskp, part, field, addnd, sector) \
> - do { \
> - preempt_disable(); \
> - __all_stat_add(gendiskp, part, field, addnd, sector); \
> - preempt_enable(); \
> - } while (0)
>
> #define __all_stat_dec(gendiskp, field, sector) \
> __all_stat_add(gendiskp, field, -1, sector)
> -#define all_stat_dec(gendiskp, field, sector) \
> - all_stat_add(gendiskp, field, -1, sector)
> -
> #define __all_stat_inc(gendiskp, part, field, sector) \
> __all_stat_add(gendiskp, part, field, 1, sector)
> -#define all_stat_inc(gendiskp, part, field, sector) \
> - all_stat_add(gendiskp, part, field, 1, sector)
> -
> #define __all_stat_sub(gendiskp, part, field, subnd, sector) \
> __all_stat_add(gendiskp, part, field, -subnd, sector)
> -#define all_stat_sub(gendiskp, part, field, subnd, sector) \
> - all_stat_add(gendiskp, part, field, -subnd, sector)
>
> /* Inlines to alloc and free disk stats in struct gendisk */
> #ifdef CONFIG_SMP
^ permalink raw reply [flat|nested] 13+ messages in thread