From ea6949b66d084a197dd7f243b72e216a71d9f2ca Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Thu, 21 Apr 2011 20:54:44 +0200 Subject: cdrom: always check_disk_change() on open cdrom_open() called check_disk_change() after the rest of open path succeeded which leads to the following bizarre behavior. * After media change, if the device opened without O_NONBLOCK, open_for_data() naturally fails with -ENOMEDIA and check_disk_change() is never called. The media is known to be gone and the open failure makes it obvious to the userland but device invalidation never happens. * But if the device is opened with O_NONBLOCK, all the checks are bypassed and cdrom_open() doesn't notice that the media is not there and check_disk_change() is called and invalidation happens. There's nothing to be gained by avoiding calling check_disk_change() on open failure. Common cases end up calling check_disk_change() anyway. All we get is inconsistent behavior. Fix it by moving check_disk_change() invocation to the top of cdrom_open() so that it always gets called regardless of how the rest of open proceeds. Note for stable: 2.6.38 and later only Cc: stable@kernel.org Signed-off-by: Tejun Heo Reported-by: Amit Shah Tested-by: Amit Shah Signed-off-by: Jens Axboe --- drivers/cdrom/cdrom.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/cdrom/cdrom.c b/drivers/cdrom/cdrom.c index 514dd8efaf73..75fb965b8f72 100644 --- a/drivers/cdrom/cdrom.c +++ b/drivers/cdrom/cdrom.c @@ -986,6 +986,9 @@ int cdrom_open(struct cdrom_device_info *cdi, struct block_device *bdev, fmode_t cdinfo(CD_OPEN, "entering cdrom_open\n"); + /* open is event synchronization point, check events first */ + check_disk_change(bdev); + /* if this was a O_NONBLOCK open and we should honor the flags, * do a quick open without drive/disc integrity checks. */ cdi->use_count++; @@ -1012,9 +1015,6 @@ int cdrom_open(struct cdrom_device_info *cdi, struct block_device *bdev, fmode_t cdinfo(CD_OPEN, "Use count for \"/dev/%s\" now %d\n", cdi->name, cdi->use_count); - /* Do this on open. Don't wait for mount, because they might - not be mounting, but opening with O_NONBLOCK */ - check_disk_change(bdev); return 0; err_release: if (CDROM_CAN(CDC_LOCK) && cdi->options & CDO_LOCK) { -- cgit v1.2.1 From 1196f8b814f32cd04df334abf47648c2a9fd8324 Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Thu, 21 Apr 2011 20:54:45 +0200 Subject: block: rescan partitions on invalidated devices on -ENOMEDIA too __blkdev_get() doesn't rescan partitions if disk->fops->open() fails, which leads to ghost partition devices lingering after medimum removal is known to both the kernel and userland. The behavior also creates a subtle inconsistency where O_NONBLOCK open, which doesn't fail even if there's no medium, clears the ghots partitions, which is exploited to work around the problem from userland. Fix it by updating __blkdev_get() to issue partition rescan after -ENOMEDIA too. This was reported in the following bz. https://bugzilla.kernel.org/show_bug.cgi?id=13029 Note for stable: 2.6.38 and later only Cc: stable@kernel.org Signed-off-by: Tejun Heo Reported-by: David Zeuthen Reported-by: Martin Pitt Reported-by: Kay Sievers Tested-by: Kay Sievers Cc: Alan Cox Signed-off-by: Jens Axboe --- fs/block_dev.c | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/fs/block_dev.c b/fs/block_dev.c index 5147bdd3b8e1..257b00e98428 100644 --- a/fs/block_dev.c +++ b/fs/block_dev.c @@ -1102,6 +1102,7 @@ static int __blkdev_get(struct block_device *bdev, fmode_t mode, int for_part) if (!bdev->bd_part) goto out_clear; + ret = 0; if (disk->fops->open) { ret = disk->fops->open(bdev, mode); if (ret == -ERESTARTSYS) { @@ -1118,9 +1119,18 @@ static int __blkdev_get(struct block_device *bdev, fmode_t mode, int for_part) put_disk(disk); goto restart; } - if (ret) - goto out_clear; } + /* + * If the device is invalidated, rescan partition + * if open succeeded or failed with -ENOMEDIUM. + * The latter is necessary to prevent ghost + * partitions on a removed medium. + */ + if (bdev->bd_invalidated && (!ret || ret == -ENOMEDIUM)) + rescan_partitions(disk, bdev); + if (ret) + goto out_clear; + if (!bdev->bd_openers) { bd_set_size(bdev,(loff_t)get_capacity(disk)<<9); bdi = blk_get_backing_dev_info(bdev); @@ -1128,8 +1138,6 @@ static int __blkdev_get(struct block_device *bdev, fmode_t mode, int for_part) bdi = &default_backing_dev_info; bdev_inode_switch_bdi(bdev->bd_inode, bdi); } - if (bdev->bd_invalidated) - rescan_partitions(disk, bdev); } else { struct block_device *whole; whole = bdget_disk(disk, 0); @@ -1153,13 +1161,14 @@ static int __blkdev_get(struct block_device *bdev, fmode_t mode, int for_part) } } else { if (bdev->bd_contains == bdev) { - if (bdev->bd_disk->fops->open) { + ret = 0; + if (bdev->bd_disk->fops->open) ret = bdev->bd_disk->fops->open(bdev, mode); - if (ret) - goto out_unlock_bdev; - } - if (bdev->bd_invalidated) + /* the same as first opener case, read comment there */ + if (bdev->bd_invalidated && (!ret || ret == -ENOMEDIUM)) rescan_partitions(bdev->bd_disk, bdev); + if (ret) + goto out_unlock_bdev; } /* only one opener holds refs to the module and disk */ module_put(disk->fops->owner); -- cgit v1.2.1 From d4dc210f69bcb0b4bef5a83b1c323817be89bad1 Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Thu, 21 Apr 2011 20:54:46 +0200 Subject: block: don't block events on excl write for non-optical devices Disk event code automatically blocks events on excl write. This is primarily to avoid issuing polling commands while burning is in progress. This behavior doesn't fit other types of devices with removeable media where polling commands don't have adverse side effects and door locking usually doesn't exist. This patch introduces new genhd flag which controls the auto-blocking behavior and uses it to enable auto-blocking only on optical devices. Note for stable: 2.6.38 and later only Cc: stable@kernel.org Signed-off-by: Tejun Heo Reported-by: Kay Sievers Signed-off-by: Jens Axboe --- drivers/block/paride/pcd.c | 1 + drivers/cdrom/viocd.c | 3 ++- drivers/ide/ide-cd.c | 2 +- drivers/scsi/sr.c | 2 +- fs/block_dev.c | 17 ++++++++++------- include/linux/genhd.h | 1 + 6 files changed, 16 insertions(+), 10 deletions(-) diff --git a/drivers/block/paride/pcd.c b/drivers/block/paride/pcd.c index 2f2ccf686251..a0aabd904a51 100644 --- a/drivers/block/paride/pcd.c +++ b/drivers/block/paride/pcd.c @@ -320,6 +320,7 @@ static void pcd_init_units(void) disk->first_minor = unit; strcpy(disk->disk_name, cd->name); /* umm... */ disk->fops = &pcd_bdops; + disk->flags = GENHD_FL_BLOCK_EVENTS_ON_EXCL_WRITE; disk->events = DISK_EVENT_MEDIA_CHANGE; } } diff --git a/drivers/cdrom/viocd.c b/drivers/cdrom/viocd.c index 4e874c5fa605..ae15a4ddaa9b 100644 --- a/drivers/cdrom/viocd.c +++ b/drivers/cdrom/viocd.c @@ -625,7 +625,8 @@ static int viocd_probe(struct vio_dev *vdev, const struct vio_device_id *id) blk_queue_max_hw_sectors(q, 4096 / 512); gendisk->queue = q; gendisk->fops = &viocd_fops; - gendisk->flags = GENHD_FL_CD|GENHD_FL_REMOVABLE; + gendisk->flags = GENHD_FL_CD | GENHD_FL_REMOVABLE | + GENHD_FL_BLOCK_EVENTS_ON_EXCL_WRITE; gendisk->events = DISK_EVENT_MEDIA_CHANGE; set_capacity(gendisk, 0); gendisk->private_data = d; diff --git a/drivers/ide/ide-cd.c b/drivers/ide/ide-cd.c index fd1e11799137..6e5123b1d341 100644 --- a/drivers/ide/ide-cd.c +++ b/drivers/ide/ide-cd.c @@ -1781,7 +1781,7 @@ static int ide_cd_probe(ide_drive_t *drive) ide_cd_read_toc(drive, &sense); g->fops = &idecd_ops; - g->flags |= GENHD_FL_REMOVABLE; + g->flags |= GENHD_FL_REMOVABLE | GENHD_FL_BLOCK_EVENTS_ON_EXCL_WRITE; g->events = DISK_EVENT_MEDIA_CHANGE; add_disk(g); return 0; diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c index 95019c747cc1..4778e2707168 100644 --- a/drivers/scsi/sr.c +++ b/drivers/scsi/sr.c @@ -636,7 +636,7 @@ static int sr_probe(struct device *dev) disk->first_minor = minor; sprintf(disk->disk_name, "sr%d", minor); disk->fops = &sr_bdops; - disk->flags = GENHD_FL_CD; + disk->flags = GENHD_FL_CD | GENHD_FL_BLOCK_EVENTS_ON_EXCL_WRITE; disk->events = DISK_EVENT_MEDIA_CHANGE | DISK_EVENT_EJECT_REQUEST; blk_queue_rq_timeout(sdev->request_queue, SR_TIMEOUT); diff --git a/fs/block_dev.c b/fs/block_dev.c index 257b00e98428..d7c2e0fddc6f 100644 --- a/fs/block_dev.c +++ b/fs/block_dev.c @@ -1237,6 +1237,8 @@ int blkdev_get(struct block_device *bdev, fmode_t mode, void *holder) res = __blkdev_get(bdev, mode, 0); if (whole) { + struct gendisk *disk = whole->bd_disk; + /* finish claiming */ mutex_lock(&bdev->bd_mutex); spin_lock(&bdev_lock); @@ -1263,15 +1265,16 @@ int blkdev_get(struct block_device *bdev, fmode_t mode, void *holder) spin_unlock(&bdev_lock); /* - * Block event polling for write claims. Any write - * holder makes the write_holder state stick until all - * are released. This is good enough and tracking - * individual writeable reference is too fragile given - * the way @mode is used in blkdev_get/put(). + * Block event polling for write claims if requested. Any + * write holder makes the write_holder state stick until + * all are released. This is good enough and tracking + * individual writeable reference is too fragile given the + * way @mode is used in blkdev_get/put(). */ - if (!res && (mode & FMODE_WRITE) && !bdev->bd_write_holder) { + if ((disk->flags & GENHD_FL_BLOCK_EVENTS_ON_EXCL_WRITE) && + !res && (mode & FMODE_WRITE) && !bdev->bd_write_holder) { bdev->bd_write_holder = true; - disk_block_events(bdev->bd_disk); + disk_block_events(disk); } mutex_unlock(&bdev->bd_mutex); diff --git a/include/linux/genhd.h b/include/linux/genhd.h index d764a426e9fd..300d7582006e 100644 --- a/include/linux/genhd.h +++ b/include/linux/genhd.h @@ -127,6 +127,7 @@ struct hd_struct { #define GENHD_FL_SUPPRESS_PARTITION_INFO 32 #define GENHD_FL_EXT_DEVT 64 /* allow extended devt */ #define GENHD_FL_NATIVE_CAPACITY 128 +#define GENHD_FL_BLOCK_EVENTS_ON_EXCL_WRITE 256 enum { DISK_EVENT_MEDIA_CHANGE = 1 << 0, /* media changed */ -- cgit v1.2.1 From addd0a09fc06179f2e02b4221775d9ab265c9fc7 Mon Sep 17 00:00:00 2001 From: Tao Ma Date: Thu, 5 May 2011 15:10:05 -0600 Subject: block: Remove 'plug/unplug' comment in blk_execute_rq_nowait unplug is replaced with blk_run_queue now in blk_execute_rq_nowait, so change the comment accordingly. Signed-off-by: Tao Ma Signed-off-by: Jens Axboe --- block/blk-exec.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/block/blk-exec.c b/block/blk-exec.c index 81e31819a597..8a0e7ec056e7 100644 --- a/block/blk-exec.c +++ b/block/blk-exec.c @@ -56,7 +56,7 @@ void blk_execute_rq_nowait(struct request_queue *q, struct gendisk *bd_disk, spin_lock_irq(q->queue_lock); __elv_add_request(q, rq, where); __blk_run_queue(q); - /* the queue is stopped so it won't be plugged+unplugged */ + /* the queue is stopped so it won't be run */ if (rq->cmd_type == REQ_TYPE_PM_RESUME) q->request_fn(q); spin_unlock_irq(q->queue_lock); -- cgit v1.2.1 From 490b94be0282c3b67f56453628ff0aaae827a670 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Thu, 5 May 2011 18:02:12 -0600 Subject: iosched: remove redundant sprintf After the anticipatory scheduler was dropped, there was no need to special-case the request_module string. As such, drop the redundant sprintf and stack variable. Signed-off-by: Kees Cook Signed-off-by: Jens Axboe --- block/elevator.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/block/elevator.c b/block/elevator.c index 6f6abc08bb56..3cd0d8c84902 100644 --- a/block/elevator.c +++ b/block/elevator.c @@ -155,13 +155,8 @@ static struct elevator_type *elevator_get(const char *name) e = elevator_find(name); if (!e) { - char elv[ELV_NAME_MAX + strlen("-iosched")]; - spin_unlock(&elv_list_lock); - - snprintf(elv, sizeof(elv), "%s-iosched", name); - - request_module("%s", elv); + request_module("%s-iosched", name); spin_lock(&elv_list_lock); e = elevator_find(name); } -- cgit v1.2.1 From f3876930952390a31c3a7fd68dd621464a36eb80 Mon Sep 17 00:00:00 2001 From: "shaohua.li@intel.com" Date: Fri, 6 May 2011 11:34:32 -0600 Subject: block: add a non-queueable flush flag flush request isn't queueable in some drives. Add a flag to let driver notify block layer about this. We can optimize flush performance with the knowledge. Stable: 2.6.39 only Cc: stable@kernel.org Signed-off-by: Shaohua Li Acked-by: Tejun Heo Signed-off-by: Jens Axboe --- block/blk-settings.c | 6 ++++++ include/linux/blkdev.h | 7 +++++++ 2 files changed, 13 insertions(+) diff --git a/block/blk-settings.c b/block/blk-settings.c index 1fa769293597..cd3c428e194f 100644 --- a/block/blk-settings.c +++ b/block/blk-settings.c @@ -790,6 +790,12 @@ void blk_queue_flush(struct request_queue *q, unsigned int flush) } EXPORT_SYMBOL_GPL(blk_queue_flush); +void blk_queue_flush_queueable(struct request_queue *q, bool queueable) +{ + q->flush_not_queueable = !queueable; +} +EXPORT_SYMBOL_GPL(blk_queue_flush_queueable); + static int __init blk_settings_init(void) { blk_max_low_pfn = max_low_pfn - 1; diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h index cbbfd98ad4a3..8bd2a271b2d8 100644 --- a/include/linux/blkdev.h +++ b/include/linux/blkdev.h @@ -364,6 +364,7 @@ struct request_queue * for flush operations */ unsigned int flush_flags; + unsigned int flush_not_queueable:1; unsigned int flush_pending_idx:1; unsigned int flush_running_idx:1; unsigned long flush_pending_since; @@ -843,6 +844,7 @@ extern void blk_queue_softirq_done(struct request_queue *, softirq_done_fn *); extern void blk_queue_rq_timed_out(struct request_queue *, rq_timed_out_fn *); extern void blk_queue_rq_timeout(struct request_queue *, unsigned int); extern void blk_queue_flush(struct request_queue *q, unsigned int flush); +extern void blk_queue_flush_queueable(struct request_queue *q, bool queueable); extern struct backing_dev_info *blk_get_backing_dev_info(struct block_device *bdev); extern int blk_rq_map_sg(struct request_queue *, struct request *, struct scatterlist *); @@ -1111,6 +1113,11 @@ static inline unsigned int block_size(struct block_device *bdev) return bdev->bd_block_size; } +static inline bool queue_flush_queueable(struct request_queue *q) +{ + return !q->flush_not_queueable; +} + typedef struct {struct page *v;} Sector; unsigned char *read_dev_sector(struct block_device *, sector_t, Sector *); -- cgit v1.2.1 From 3ac0cc4508709d42ec9aa351086c7d38bfc0660c Mon Sep 17 00:00:00 2001 From: "shaohua.li@intel.com" Date: Fri, 6 May 2011 11:34:41 -0600 Subject: block: hold queue if flush is running for non-queueable flush drive In some drives, flush requests are non-queueable. When flush request is running, normal read/write requests can't run. If block layer dispatches such request, driver can't handle it and requeue it. Tejun suggested we can hold the queue when flush is running. This can avoid unnecessary requeue. Also this can improve performance. For example, we have request flush1, write1, flush 2. flush1 is dispatched, then queue is hold, write1 isn't inserted to queue. After flush1 is finished, flush2 will be dispatched. Since disk cache is already clean, flush2 will be finished very soon, so looks like flush2 is folded to flush1. In my test, the queue holding completely solves a regression introduced by commit 53d63e6b0dfb95882ec0219ba6bbd50cde423794: block: make the flush insertion use the tail of the dispatch list It's not a preempt type request, in fact we have to insert it behind requests that do specify INSERT_FRONT. which causes about 20% regression running a sysbench fileio workload. Stable: 2.6.39 only Cc: stable@kernel.org Signed-off-by: Shaohua Li Acked-by: Tejun Heo Signed-off-by: Jens Axboe --- block/blk-flush.c | 16 +++++++++++----- block/blk.h | 21 ++++++++++++++++++++- include/linux/blkdev.h | 1 + 3 files changed, 32 insertions(+), 6 deletions(-) diff --git a/block/blk-flush.c b/block/blk-flush.c index 6c9b5e189e62..bb21e4c36f70 100644 --- a/block/blk-flush.c +++ b/block/blk-flush.c @@ -212,13 +212,19 @@ static void flush_end_io(struct request *flush_rq, int error) } /* - * Moving a request silently to empty queue_head may stall the - * queue. Kick the queue in those cases. This function is called - * from request completion path and calling directly into - * request_fn may confuse the driver. Always use kblockd. + * Kick the queue to avoid stall for two cases: + * 1. Moving a request silently to empty queue_head may stall the + * queue. + * 2. When flush request is running in non-queueable queue, the + * queue is hold. Restart the queue after flush request is finished + * to avoid stall. + * This function is called from request completion path and calling + * directly into request_fn may confuse the driver. Always use + * kblockd. */ - if (queued) + if (queued || q->flush_queue_delayed) blk_run_queue_async(q); + q->flush_queue_delayed = 0; } /** diff --git a/block/blk.h b/block/blk.h index c9df8fc3c999..83e4bff36201 100644 --- a/block/blk.h +++ b/block/blk.h @@ -62,7 +62,26 @@ static inline struct request *__elv_next_request(struct request_queue *q) rq = list_entry_rq(q->queue_head.next); return rq; } - + /* + * Flush request is running and flush request isn't queueable + * in the drive, we can hold the queue till flush request is + * finished. Even we don't do this, driver can't dispatch next + * requests and will requeue them. And this can improve + * throughput too. For example, we have request flush1, write1, + * flush 2. flush1 is dispatched, then queue is hold, write1 + * isn't inserted to queue. After flush1 is finished, flush2 + * will be dispatched. Since disk cache is already clean, + * flush2 will be finished very soon, so looks like flush2 is + * folded to flush1. + * Since the queue is hold, a flag is set to indicate the queue + * should be restarted later. Please see flush_end_io() for + * details. + */ + if (q->flush_pending_idx != q->flush_running_idx && + !queue_flush_queueable(q)) { + q->flush_queue_delayed = 1; + return NULL; + } if (!q->elevator->ops->elevator_dispatch_fn(q, 0)) return NULL; } diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h index 8bd2a271b2d8..9f921bf4bf8c 100644 --- a/include/linux/blkdev.h +++ b/include/linux/blkdev.h @@ -365,6 +365,7 @@ struct request_queue */ unsigned int flush_flags; unsigned int flush_not_queueable:1; + unsigned int flush_queue_delayed:1; unsigned int flush_pending_idx:1; unsigned int flush_running_idx:1; unsigned long flush_pending_since; -- cgit v1.2.1 From 900e599eb0c16390ff671652a44e0ea90532220e Mon Sep 17 00:00:00 2001 From: "shaohua.li@intel.com" Date: Fri, 6 May 2011 11:35:31 -0600 Subject: SATA: enable non-queueable flush flag Enable non-queueable flush flag for SATA. Stable: 2.6.39 only Cc: stable@kernel.org Signed-off-by: Shaohua Li Acked-by: Tejun Heo Acked-by: Jeff Garzik Signed-off-by: Jens Axboe --- drivers/ata/libata-scsi.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c index e2f57e9e12f0..dad2c952e0d2 100644 --- a/drivers/ata/libata-scsi.c +++ b/drivers/ata/libata-scsi.c @@ -1089,21 +1089,21 @@ static int atapi_drain_needed(struct request *rq) static int ata_scsi_dev_config(struct scsi_device *sdev, struct ata_device *dev) { + struct request_queue *q = sdev->request_queue; + if (!ata_id_has_unload(dev->id)) dev->flags |= ATA_DFLAG_NO_UNLOAD; /* configure max sectors */ - blk_queue_max_hw_sectors(sdev->request_queue, dev->max_sectors); + blk_queue_max_hw_sectors(q, dev->max_sectors); if (dev->class == ATA_DEV_ATAPI) { - struct request_queue *q = sdev->request_queue; void *buf; sdev->sector_size = ATA_SECT_SIZE; /* set DMA padding */ - blk_queue_update_dma_pad(sdev->request_queue, - ATA_DMA_PAD_SZ - 1); + blk_queue_update_dma_pad(q, ATA_DMA_PAD_SZ - 1); /* configure draining */ buf = kmalloc(ATAPI_MAX_DRAIN, q->bounce_gfp | GFP_KERNEL); @@ -1131,8 +1131,7 @@ static int ata_scsi_dev_config(struct scsi_device *sdev, "sector_size=%u > PAGE_SIZE, PIO may malfunction\n", sdev->sector_size); - blk_queue_update_dma_alignment(sdev->request_queue, - sdev->sector_size - 1); + blk_queue_update_dma_alignment(q, sdev->sector_size - 1); if (dev->flags & ATA_DFLAG_AN) set_bit(SDEV_EVT_MEDIA_CHANGE, sdev->supported_events); @@ -1145,6 +1144,8 @@ static int ata_scsi_dev_config(struct scsi_device *sdev, scsi_adjust_queue_depth(sdev, MSG_SIMPLE_TAG, depth); } + blk_queue_flush_queueable(q, false); + dev->sdev = sdev; return 0; } -- cgit v1.2.1 From 5dba3089ed03f84b84c6c739df8330112f04a15d Mon Sep 17 00:00:00 2001 From: Lukas Czerner Date: Fri, 6 May 2011 19:26:27 -0600 Subject: blkdev: Submit discard bio in batches in blkdev_issue_discard() Currently we are waiting for every submitted REQ_DISCARD bio separately, but it can have unwanted consequences of repeatedly flushing the queue, so we rather submit bios in batches and wait for the entire batch, hence narrowing the window of other ios going in. Use bio_batch_end_io() and struct bio_batch for that purpose, the same is used by blkdev_issue_zeroout(). Also change bio_batch_end_io() so we always set !BIO_UPTODATE in the case of error and remove the check for bb, since we are the only user of this function and we always set this. Remove bio_get()/bio_put() from the blkdev_issue_discard() since bio_alloc() and bio_batch_end_io() is doing the same thing, hence it is not needed anymore. I have done simple dd testing with surprising results. The script I have used is: for i in $(seq 10); do echo $i dd if=/dev/sdb1 of=/dev/sdc1 bs=4k & sleep 5 done /usr/bin/time -f %e ./blkdiscard /dev/sdc1 Running time of BLKDISCARD on the whole device: with patch without patch 0.95 15.58 So we can see that in this artificial test the kernel with the patch applied is approx 16x faster in discarding the device. Signed-off-by: Lukas Czerner CC: Dmitry Monakhov CC: Jens Axboe CC: Jeff Moyer Signed-off-by: Jens Axboe --- block/blk-lib.c | 69 ++++++++++++++++++++++++--------------------------------- 1 file changed, 29 insertions(+), 40 deletions(-) diff --git a/block/blk-lib.c b/block/blk-lib.c index 25de73e4759b..9260cb0b209b 100644 --- a/block/blk-lib.c +++ b/block/blk-lib.c @@ -9,17 +9,23 @@ #include "blk.h" -static void blkdev_discard_end_io(struct bio *bio, int err) +struct bio_batch { + atomic_t done; + unsigned long flags; + struct completion *wait; +}; + +static void bio_batch_end_io(struct bio *bio, int err) { + struct bio_batch *bb = bio->bi_private; + if (err) { if (err == -EOPNOTSUPP) - set_bit(BIO_EOPNOTSUPP, &bio->bi_flags); - clear_bit(BIO_UPTODATE, &bio->bi_flags); + set_bit(BIO_EOPNOTSUPP, &bb->flags); + clear_bit(BIO_UPTODATE, &bb->flags); } - - if (bio->bi_private) - complete(bio->bi_private); - + if (atomic_dec_and_test(&bb->done)) + complete(bb->wait); bio_put(bio); } @@ -41,6 +47,7 @@ int blkdev_issue_discard(struct block_device *bdev, sector_t sector, struct request_queue *q = bdev_get_queue(bdev); int type = REQ_WRITE | REQ_DISCARD; unsigned int max_discard_sectors; + struct bio_batch bb; struct bio *bio; int ret = 0; @@ -67,7 +74,11 @@ int blkdev_issue_discard(struct block_device *bdev, sector_t sector, type |= REQ_SECURE; } - while (nr_sects && !ret) { + atomic_set(&bb.done, 1); + bb.flags = 1 << BIO_UPTODATE; + bb.wait = &wait; + + while (nr_sects) { bio = bio_alloc(gfp_mask, 1); if (!bio) { ret = -ENOMEM; @@ -75,9 +86,9 @@ int blkdev_issue_discard(struct block_device *bdev, sector_t sector, } bio->bi_sector = sector; - bio->bi_end_io = blkdev_discard_end_io; + bio->bi_end_io = bio_batch_end_io; bio->bi_bdev = bdev; - bio->bi_private = &wait; + bio->bi_private = &bb; if (nr_sects > max_discard_sectors) { bio->bi_size = max_discard_sectors << 9; @@ -88,45 +99,23 @@ int blkdev_issue_discard(struct block_device *bdev, sector_t sector, nr_sects = 0; } - bio_get(bio); + atomic_inc(&bb.done); submit_bio(type, bio); + } + /* Wait for bios in-flight */ + if (!atomic_dec_and_test(&bb.done)) wait_for_completion(&wait); - if (bio_flagged(bio, BIO_EOPNOTSUPP)) - ret = -EOPNOTSUPP; - else if (!bio_flagged(bio, BIO_UPTODATE)) - ret = -EIO; - bio_put(bio); - } + if (test_bit(BIO_EOPNOTSUPP, &bb.flags)) + ret = -EOPNOTSUPP; + else if (!test_bit(BIO_UPTODATE, &bb.flags)) + ret = -EIO; return ret; } EXPORT_SYMBOL(blkdev_issue_discard); -struct bio_batch -{ - atomic_t done; - unsigned long flags; - struct completion *wait; -}; - -static void bio_batch_end_io(struct bio *bio, int err) -{ - struct bio_batch *bb = bio->bi_private; - - if (err) { - if (err == -EOPNOTSUPP) - set_bit(BIO_EOPNOTSUPP, &bb->flags); - else - clear_bit(BIO_UPTODATE, &bb->flags); - } - if (bb) - if (atomic_dec_and_test(&bb->done)) - complete(bb->wait); - bio_put(bio); -} - /** * blkdev_issue_zeroout - generate number of zero filed write bios * @bdev: blockdev to issue -- cgit v1.2.1 From 5baebe5c86acd6100536a466905880529f79cf1a Mon Sep 17 00:00:00 2001 From: Lukas Czerner Date: Fri, 6 May 2011 19:26:28 -0600 Subject: blkdev: Simple cleanup in blkdev_issue_zeroout() In blkdev_issue_zeroout() we are submitting regular WRITE bios, so we do not need to check for -EOPNOTSUPP specifically in case of error. Also there is no need to have label submit: because there is no way to jump out from the while cycle without an error and we really want to exit, rather than try again. And also remove the check for (sz == 0) since at that point sz can never be zero. Signed-off-by: Lukas Czerner Reviewed-by: Jeff Moyer CC: Dmitry Monakhov CC: Jens Axboe Signed-off-by: Jens Axboe --- block/blk-lib.c | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/block/blk-lib.c b/block/blk-lib.c index 9260cb0b209b..d7a98d3ed4aa 100644 --- a/block/blk-lib.c +++ b/block/blk-lib.c @@ -140,7 +140,6 @@ int blkdev_issue_zeroout(struct block_device *bdev, sector_t sector, bb.flags = 1 << BIO_UPTODATE; bb.wait = &wait; -submit: ret = 0; while (nr_sects != 0) { bio = bio_alloc(gfp_mask, @@ -157,9 +156,6 @@ submit: while (nr_sects != 0) { sz = min((sector_t) PAGE_SIZE >> 9 , nr_sects); - if (sz == 0) - /* bio has maximum size possible */ - break; ret = bio_add_page(bio, ZERO_PAGE(0), sz << 9, 0); nr_sects -= ret >> 9; sector += ret >> 9; @@ -179,16 +175,6 @@ submit: /* One of bios in the batch was completed with error.*/ ret = -EIO; - if (ret) - goto out; - - if (test_bit(BIO_EOPNOTSUPP, &bb.flags)) { - ret = -EOPNOTSUPP; - goto out; - } - if (nr_sects != 0) - goto submit; -out: return ret; } EXPORT_SYMBOL(blkdev_issue_zeroout); -- cgit v1.2.1 From 8af1954d172a46a63e5e79dae523a6d74715e458 Mon Sep 17 00:00:00 2001 From: Lukas Czerner Date: Fri, 6 May 2011 19:30:01 -0600 Subject: blkdev: Do not return -EOPNOTSUPP if discard is supported Currently we return -EOPNOTSUPP in blkdev_issue_discard() if any of the bio fails due to underlying device not supporting discard request. However, if the device is for example dm device composed of devices which some of them support discard and some of them does not, it is ok for some bios to fail with EOPNOTSUPP, but it does not mean that discard is not supported at all. This commit removes the check for bios failed with EOPNOTSUPP and change blkdev_issue_discard() to return operation not supported if and only if the device does not actually supports it, not just part of the device as some bios might indicate. This change also fixes problem with BLKDISCARD ioctl() which now works correctly on such dm devices. Signed-off-by: Lukas Czerner CC: Jens Axboe CC: Jeff Moyer Signed-off-by: Jens Axboe --- block/blk-lib.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/block/blk-lib.c b/block/blk-lib.c index d7a98d3ed4aa..78e627e2581d 100644 --- a/block/blk-lib.c +++ b/block/blk-lib.c @@ -19,11 +19,8 @@ static void bio_batch_end_io(struct bio *bio, int err) { struct bio_batch *bb = bio->bi_private; - if (err) { - if (err == -EOPNOTSUPP) - set_bit(BIO_EOPNOTSUPP, &bb->flags); + if (err && (err != -EOPNOTSUPP)) clear_bit(BIO_UPTODATE, &bb->flags); - } if (atomic_dec_and_test(&bb->done)) complete(bb->wait); bio_put(bio); @@ -107,9 +104,7 @@ int blkdev_issue_discard(struct block_device *bdev, sector_t sector, if (!atomic_dec_and_test(&bb.done)) wait_for_completion(&wait); - if (test_bit(BIO_EOPNOTSUPP, &bb.flags)) - ret = -EOPNOTSUPP; - else if (!test_bit(BIO_UPTODATE, &bb.flags)) + if (!test_bit(BIO_UPTODATE, &bb.flags)) ret = -EIO; return ret; -- cgit v1.2.1 From 23ceb5b7719e9276d4fa72a3ecf94dd396755276 Mon Sep 17 00:00:00 2001 From: Tao Ma Date: Fri, 6 May 2011 19:30:02 -0600 Subject: block: Remove extra discard_alignment from hd_struct. Currently, hd_struct.discard_alignment is only used when we show /sys/block/sdx/sdx/discard_alignment. So remove it and calculate when it is asked to show. Signed-off-by: Tao Ma Signed-off-by: Jens Axboe --- fs/partitions/check.c | 9 ++++++--- include/linux/genhd.h | 1 - 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/fs/partitions/check.c b/fs/partitions/check.c index d545e97d99c3..b7e16bccd5e5 100644 --- a/fs/partitions/check.c +++ b/fs/partitions/check.c @@ -255,7 +255,12 @@ ssize_t part_discard_alignment_show(struct device *dev, struct device_attribute *attr, char *buf) { struct hd_struct *p = dev_to_part(dev); - return sprintf(buf, "%u\n", p->discard_alignment); + struct gendisk *disk = dev_to_disk(dev); + + return sprintf(buf, "%u\n", + (unsigned long long)queue_limit_discard_alignment( + &disk->queue->limits, + p->start_sect)); } ssize_t part_stat_show(struct device *dev, @@ -449,8 +454,6 @@ struct hd_struct *add_partition(struct gendisk *disk, int partno, p->start_sect = start; p->alignment_offset = queue_limit_alignment_offset(&disk->queue->limits, start); - p->discard_alignment = - queue_limit_discard_alignment(&disk->queue->limits, start); p->nr_sects = len; p->partno = partno; p->policy = get_disk_ro(disk); diff --git a/include/linux/genhd.h b/include/linux/genhd.h index 300d7582006e..b78956b3c2e7 100644 --- a/include/linux/genhd.h +++ b/include/linux/genhd.h @@ -100,7 +100,6 @@ struct hd_struct { sector_t start_sect; sector_t nr_sects; sector_t alignment_offset; - unsigned int discard_alignment; struct device __dev; struct kobject *holder_dir; int policy, partno; -- cgit v1.2.1 From bbdd304cf66fbf2b4b2d28418dc619d443635e83 Mon Sep 17 00:00:00 2001 From: Jens Axboe Date: Mon, 9 May 2011 08:28:13 +0200 Subject: fs: fixup warning part_discard_alignment_show() Stephen reports: ----- After merging the block tree, today's linux-next build (x86_64 allmodconfig) produced this warning: fs/partitions/check.c: In function 'part_discard_alignment_show': fs/partitions/check.c:263: warning: format '%u' expects type 'unsigned int', but argument 3 has type 'long long unsigned int' Introduced by commit ("block: Remove extra discard_alignment from hd_struct") ----- Fix it up by just removing the cast, we return an int already. Reported-by: Stephen Rothwell Signed-off-by: Jens Axboe --- fs/partitions/check.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/fs/partitions/check.c b/fs/partitions/check.c index b7e16bccd5e5..8ed4d3433199 100644 --- a/fs/partitions/check.c +++ b/fs/partitions/check.c @@ -258,8 +258,7 @@ ssize_t part_discard_alignment_show(struct device *dev, struct gendisk *disk = dev_to_disk(dev); return sprintf(buf, "%u\n", - (unsigned long long)queue_limit_discard_alignment( - &disk->queue->limits, + queue_limit_discard_alignment(&disk->queue->limits, p->start_sect)); } -- cgit v1.2.1 From a934a00a69e940b126b9bdbf83e630ef5fe43523 Mon Sep 17 00:00:00 2001 From: "Martin K. Petersen" Date: Wed, 18 May 2011 10:37:35 +0200 Subject: block: Fix discard topology stacking and reporting In some cases we would end up stacking discard_zeroes_data incorrectly. Fix this by enabling the feature by default for stacking drivers and clearing it for low-level drivers. Incorporating a device that does not support dzd will then cause the feature to be disabled in the stacking driver. Also ensure that the maximum discard value does not overflow when exported in sysfs and return 0 in the alignment and dzd fields for devices that don't support discard. Reported-by: Lukas Czerner Signed-off-by: Martin K. Petersen Acked-by: Mike Snitzer Cc: stable@kernel.org Signed-off-by: Jens Axboe --- block/blk-settings.c | 3 ++- block/blk-sysfs.c | 3 ++- include/linux/blkdev.h | 7 +++++-- 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/block/blk-settings.c b/block/blk-settings.c index cd3c428e194f..fa1eb0449a05 100644 --- a/block/blk-settings.c +++ b/block/blk-settings.c @@ -120,7 +120,7 @@ void blk_set_default_limits(struct queue_limits *lim) lim->discard_granularity = 0; lim->discard_alignment = 0; lim->discard_misaligned = 0; - lim->discard_zeroes_data = -1; + lim->discard_zeroes_data = 1; lim->logical_block_size = lim->physical_block_size = lim->io_min = 512; lim->bounce_pfn = (unsigned long)(BLK_BOUNCE_ANY >> PAGE_SHIFT); lim->alignment_offset = 0; @@ -166,6 +166,7 @@ void blk_queue_make_request(struct request_queue *q, make_request_fn *mfn) blk_set_default_limits(&q->limits); blk_queue_max_hw_sectors(q, BLK_SAFE_MAX_SECTORS); + q->limits.discard_zeroes_data = 0; /* * by default assume old behaviour and bounce for any highmem page diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c index 6d735122bc59..53bd0c77bfda 100644 --- a/block/blk-sysfs.c +++ b/block/blk-sysfs.c @@ -152,7 +152,8 @@ static ssize_t queue_discard_granularity_show(struct request_queue *q, char *pag static ssize_t queue_discard_max_show(struct request_queue *q, char *page) { - return queue_var_show(q->limits.max_discard_sectors << 9, page); + return sprintf(page, "%llu\n", + (unsigned long long)q->limits.max_discard_sectors << 9); } static ssize_t queue_discard_zeroes_data_show(struct request_queue *q, char *page) diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h index 9f921bf4bf8c..520d8618ed76 100644 --- a/include/linux/blkdev.h +++ b/include/linux/blkdev.h @@ -257,7 +257,7 @@ struct queue_limits { unsigned char misaligned; unsigned char discard_misaligned; unsigned char cluster; - signed char discard_zeroes_data; + unsigned char discard_zeroes_data; }; struct request_queue @@ -1069,13 +1069,16 @@ static inline int queue_limit_discard_alignment(struct queue_limits *lim, sector { unsigned int alignment = (sector << 9) & (lim->discard_granularity - 1); + if (!lim->max_discard_sectors) + return 0; + return (lim->discard_granularity + lim->discard_alignment - alignment) & (lim->discard_granularity - 1); } static inline unsigned int queue_discard_zeroes_data(struct request_queue *q) { - if (q->limits.discard_zeroes_data == 1) + if (q->limits.max_discard_sectors && q->limits.discard_zeroes_data == 1) return 1; return 0; -- cgit v1.2.1 From d70d0711edd8076ec2ce0ed109106e2df950681b Mon Sep 17 00:00:00 2001 From: "Martin K. Petersen" Date: Wed, 18 May 2011 10:37:39 +0200 Subject: block: Add sysfs documentation for the discard topology parameters Add documentation for the discard I/O topology parameters exported in sysfs. Signed-off-by: Martin K. Petersen Signed-off-by: Jens Axboe --- Documentation/ABI/testing/sysfs-block | 64 +++++++++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/Documentation/ABI/testing/sysfs-block b/Documentation/ABI/testing/sysfs-block index 4873c759d535..c1eb41cb9876 100644 --- a/Documentation/ABI/testing/sysfs-block +++ b/Documentation/ABI/testing/sysfs-block @@ -142,3 +142,67 @@ Description: with the previous I/O request are enabled. When set to 2, all merge tries are disabled. The default value is 0 - which enables all types of merge tries. + +What: /sys/block//discard_alignment +Date: May 2011 +Contact: Martin K. Petersen +Description: + Devices that support discard functionality may + internally allocate space in units that are bigger than + the exported logical block size. The discard_alignment + parameter indicates how many bytes the beginning of the + device is offset from the internal allocation unit's + natural alignment. + +What: /sys/block///discard_alignment +Date: May 2011 +Contact: Martin K. Petersen +Description: + Devices that support discard functionality may + internally allocate space in units that are bigger than + the exported logical block size. The discard_alignment + parameter indicates how many bytes the beginning of the + partition is offset from the internal allocation unit's + natural alignment. + +What: /sys/block//queue/discard_granularity +Date: May 2011 +Contact: Martin K. Petersen +Description: + Devices that support discard functionality may + internally allocate space using units that are bigger + than the logical block size. The discard_granularity + parameter indicates the size of the internal allocation + unit in bytes if reported by the device. Otherwise the + discard_granularity will be set to match the device's + physical block size. A discard_granularity of 0 means + that the device does not support discard functionality. + +What: /sys/block//queue/discard_max_bytes +Date: May 2011 +Contact: Martin K. Petersen +Description: + Devices that support discard functionality may have + internal limits on the number of bytes that can be + trimmed or unmapped in a single operation. Some storage + protocols also have inherent limits on the number of + blocks that can be described in a single command. The + discard_max_bytes parameter is set by the device driver + to the maximum number of bytes that can be discarded in + a single operation. Discard requests issued to the + device must not exceed this limit. A discard_max_bytes + value of 0 means that the device does not support + discard functionality. + +What: /sys/block//queue/discard_zeroes_data +Date: May 2011 +Contact: Martin K. Petersen +Description: + Devices that support discard functionality may return + stale or random data when a previously discarded block + is read back. This can cause problems if the filesystem + expects discarded blocks to be explicitly cleared. If a + device reports that it deterministically returns zeroes + when a discarded area is read the discard_zeroes_data + parameter will be set to one. Otherwise it will be 0 and + the result of reading a discarded area is undefined. -- cgit v1.2.1 From 0a58e077eb600d1efd7e54ad9926a75a39d7f8ae Mon Sep 17 00:00:00 2001 From: James Bottomley Date: Wed, 18 May 2011 16:20:10 +0200 Subject: block: add proper state guards to __elv_next_request blk_cleanup_queue() calls elevator_exit() and after this, we can't touch the elevator without oopsing. __elv_next_request() must check for this state because in the refcounted queue model, we can still call it after blk_cleanup_queue() has been called. This was reported as causing an oops attributable to scsi. Signed-off-by: James Bottomley Cc: stable@kernel.org Signed-off-by: Jens Axboe --- block/blk.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/block/blk.h b/block/blk.h index 61263463e38e..4df474d37e6d 100644 --- a/block/blk.h +++ b/block/blk.h @@ -62,7 +62,8 @@ static inline struct request *__elv_next_request(struct request_queue *q) return rq; } - if (!q->elevator->ops->elevator_dispatch_fn(q, 0)) + if (test_bit(QUEUE_FLAG_DEAD, &q->queue_flags) || + !q->elevator->ops->elevator_dispatch_fn(q, 0)) return NULL; } } -- cgit v1.2.1 From a29a171e7c46c60842b85729280e2f5690372683 Mon Sep 17 00:00:00 2001 From: Vivek Goyal Date: Thu, 19 May 2011 15:38:19 -0400 Subject: blk-throttle: Do the new group initialization with the help of a function Group initialization code seems to be at two places. root group initialization in blk_throtl_init() and dynamically allocated group in throtl_find_alloc_tg(). Create a common function and use at both the places. Signed-off-by: Vivek Goyal Signed-off-by: Jens Axboe --- block/blk-throttle.c | 64 ++++++++++++++++++++++++++++------------------------ 1 file changed, 35 insertions(+), 29 deletions(-) diff --git a/block/blk-throttle.c b/block/blk-throttle.c index 252a81a306f7..fa9a900c1254 100644 --- a/block/blk-throttle.c +++ b/block/blk-throttle.c @@ -159,6 +159,35 @@ static void throtl_put_tg(struct throtl_grp *tg) kfree(tg); } +static void throtl_init_group(struct throtl_grp *tg) +{ + INIT_HLIST_NODE(&tg->tg_node); + RB_CLEAR_NODE(&tg->rb_node); + bio_list_init(&tg->bio_lists[0]); + bio_list_init(&tg->bio_lists[1]); + tg->limits_changed = false; + + /* Practically unlimited BW */ + tg->bps[0] = tg->bps[1] = -1; + tg->iops[0] = tg->iops[1] = -1; + + /* + * Take the initial reference that will be released on destroy + * This can be thought of a joint reference by cgroup and + * request queue which will be dropped by either request queue + * exit or cgroup deletion path depending on who is exiting first. + */ + atomic_set(&tg->ref, 1); +} + +/* Should be called with rcu read lock held (needed for blkcg) */ +static void +throtl_add_group_to_td_list(struct throtl_data *td, struct throtl_grp *tg) +{ + hlist_add_head(&tg->tg_node, &td->tg_list); + td->nr_undestroyed_grps++; +} + static struct throtl_grp * throtl_find_alloc_tg(struct throtl_data *td, struct blkio_cgroup *blkcg) { @@ -196,19 +225,7 @@ static struct throtl_grp * throtl_find_alloc_tg(struct throtl_data *td, if (!tg) goto done; - INIT_HLIST_NODE(&tg->tg_node); - RB_CLEAR_NODE(&tg->rb_node); - bio_list_init(&tg->bio_lists[0]); - bio_list_init(&tg->bio_lists[1]); - td->limits_changed = false; - - /* - * Take the initial reference that will be released on destroy - * This can be thought of a joint reference by cgroup and - * request queue which will be dropped by either request queue - * exit or cgroup deletion path depending on who is exiting first. - */ - atomic_set(&tg->ref, 1); + throtl_init_group(tg); /* Add group onto cgroup list */ sscanf(dev_name(bdi->dev), "%u:%u", &major, &minor); @@ -220,8 +237,7 @@ static struct throtl_grp * throtl_find_alloc_tg(struct throtl_data *td, tg->iops[READ] = blkcg_get_read_iops(blkcg, tg->blkg.dev); tg->iops[WRITE] = blkcg_get_write_iops(blkcg, tg->blkg.dev); - hlist_add_head(&tg->tg_node, &td->tg_list); - td->nr_undestroyed_grps++; + throtl_add_group_to_td_list(td, tg); done: return tg; } @@ -1060,18 +1076,11 @@ int blk_throtl_init(struct request_queue *q) INIT_HLIST_HEAD(&td->tg_list); td->tg_service_tree = THROTL_RB_ROOT; td->limits_changed = false; + INIT_DELAYED_WORK(&td->throtl_work, blk_throtl_work); /* Init root group */ tg = &td->root_tg; - INIT_HLIST_NODE(&tg->tg_node); - RB_CLEAR_NODE(&tg->rb_node); - bio_list_init(&tg->bio_lists[0]); - bio_list_init(&tg->bio_lists[1]); - - /* Practically unlimited BW */ - tg->bps[0] = tg->bps[1] = -1; - tg->iops[0] = tg->iops[1] = -1; - td->limits_changed = false; + throtl_init_group(tg); /* * Set root group reference to 2. One reference will be dropped when @@ -1080,16 +1089,13 @@ int blk_throtl_init(struct request_queue *q) * as it is statically allocated and gets destroyed when throtl_data * goes away. */ - atomic_set(&tg->ref, 2); - hlist_add_head(&tg->tg_node, &td->tg_list); - td->nr_undestroyed_grps++; - - INIT_DELAYED_WORK(&td->throtl_work, blk_throtl_work); + atomic_inc(&tg->ref); rcu_read_lock(); blkiocg_add_blkio_group(&blkio_root_cgroup, &tg->blkg, (void *)td, 0, BLKIO_POLICY_THROTL); rcu_read_unlock(); + throtl_add_group_to_td_list(td, tg); /* Attach throtl data to request queue */ td->queue = q; -- cgit v1.2.1 From a23e68695593d00b35a6cddf8e9c9ec03505ecb9 Mon Sep 17 00:00:00 2001 From: Vivek Goyal Date: Thu, 19 May 2011 15:38:20 -0400 Subject: blk-cgroup: move some fields of unaccounted_time file under right config option cgroup unaccounted_time file is created only if CONFIG_DEBUG_BLK_CGROUP=y. there are some fields which are out side this config option. Fix that. Signed-off-by: Vivek Goyal Signed-off-by: Jens Axboe --- block/blk-cgroup.c | 2 ++ block/blk-cgroup.h | 7 ++++--- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c index 471fdcc5df85..b0592bca6970 100644 --- a/block/blk-cgroup.c +++ b/block/blk-cgroup.c @@ -385,7 +385,9 @@ void blkiocg_update_timeslice_used(struct blkio_group *blkg, unsigned long time, spin_lock_irqsave(&blkg->stats_lock, flags); blkg->stats.time += time; +#ifdef CONFIG_DEBUG_BLK_CGROUP blkg->stats.unaccounted_time += unaccounted_time; +#endif spin_unlock_irqrestore(&blkg->stats_lock, flags); } EXPORT_SYMBOL_GPL(blkiocg_update_timeslice_used); diff --git a/block/blk-cgroup.h b/block/blk-cgroup.h index c774930cc206..63f1ef4450d7 100644 --- a/block/blk-cgroup.h +++ b/block/blk-cgroup.h @@ -49,9 +49,9 @@ enum stat_type { /* All the single valued stats go below this */ BLKIO_STAT_TIME, BLKIO_STAT_SECTORS, +#ifdef CONFIG_DEBUG_BLK_CGROUP /* Time not charged to this cgroup */ BLKIO_STAT_UNACCOUNTED_TIME, -#ifdef CONFIG_DEBUG_BLK_CGROUP BLKIO_STAT_AVG_QUEUE_SIZE, BLKIO_STAT_IDLE_TIME, BLKIO_STAT_EMPTY_TIME, @@ -117,10 +117,11 @@ struct blkio_group_stats { /* total disk time and nr sectors dispatched by this group */ uint64_t time; uint64_t sectors; - /* Time not charged to this cgroup */ - uint64_t unaccounted_time; uint64_t stat_arr[BLKIO_STAT_QUEUED + 1][BLKIO_STAT_TOTAL]; #ifdef CONFIG_DEBUG_BLK_CGROUP + /* Time not charged to this cgroup */ + uint64_t unaccounted_time; + /* Sum of number of IOs queued across all samples */ uint64_t avg_queue_size_sum; /* Count of samples taken for average */ -- cgit v1.2.1 From 3e59cf9d66a87763fef6c232a4a8dc664461ca50 Mon Sep 17 00:00:00 2001 From: Vivek Goyal Date: Thu, 19 May 2011 15:38:21 -0400 Subject: cfq-iosched: Get rid of redundant function parameter "create" Nobody seems to be using cfq_find_alloc_cfqg() function parameter "create". Get rid of that. Signed-off-by: Vivek Goyal Signed-off-by: Jens Axboe --- block/cfq-iosched.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c index ab7a9e6a9b1c..a905b5519ab6 100644 --- a/block/cfq-iosched.c +++ b/block/cfq-iosched.c @@ -1015,7 +1015,7 @@ void cfq_update_blkio_group_weight(void *key, struct blkio_group *blkg, } static struct cfq_group * cfq_find_alloc_cfqg(struct cfq_data *cfqd, - struct blkio_cgroup *blkcg, int create) + struct blkio_cgroup *blkcg) { struct cfq_group *cfqg = NULL; void *key = cfqd; @@ -1030,7 +1030,7 @@ static struct cfq_group * cfq_find_alloc_cfqg(struct cfq_data *cfqd, cfqg->blkg.dev = MKDEV(major, minor); goto done; } - if (cfqg || !create) + if (cfqg) goto done; cfqg = kzalloc_node(sizeof(*cfqg), GFP_ATOMIC, cfqd->queue->node); @@ -1073,18 +1073,18 @@ done: } /* - * Search for the cfq group current task belongs to. If create = 1, then also - * create the cfq group if it does not exist. request_queue lock must be held. + * Search for the cfq group current task belongs to. request_queue lock must + * be held. */ -static struct cfq_group *cfq_get_cfqg(struct cfq_data *cfqd, int create) +static struct cfq_group *cfq_get_cfqg(struct cfq_data *cfqd) { struct blkio_cgroup *blkcg; struct cfq_group *cfqg = NULL; rcu_read_lock(); blkcg = task_blkio_cgroup(current); - cfqg = cfq_find_alloc_cfqg(cfqd, blkcg, create); - if (!cfqg && create) + cfqg = cfq_find_alloc_cfqg(cfqd, blkcg); + if (!cfqg) cfqg = &cfqd->root_group; rcu_read_unlock(); return cfqg; @@ -1176,7 +1176,7 @@ void cfq_unlink_blkio_group(void *key, struct blkio_group *blkg) } #else /* GROUP_IOSCHED */ -static struct cfq_group *cfq_get_cfqg(struct cfq_data *cfqd, int create) +static struct cfq_group *cfq_get_cfqg(struct cfq_data *cfqd) { return &cfqd->root_group; } @@ -2911,7 +2911,7 @@ cfq_find_alloc_queue(struct cfq_data *cfqd, bool is_sync, struct cfq_group *cfqg; retry: - cfqg = cfq_get_cfqg(cfqd, 1); + cfqg = cfq_get_cfqg(cfqd); cic = cfq_cic_lookup(cfqd, ioc); /* cic always exists here */ cfqq = cic_to_cfqq(cic, is_sync); -- cgit v1.2.1 From 56edf7d75db5b14d628b46623c414ffbeed68d7f Mon Sep 17 00:00:00 2001 From: Vivek Goyal Date: Thu, 19 May 2011 15:38:22 -0400 Subject: cfq-iosched: Fix a possible race with cfq cgroup removal code blkg->key = cfqd is an rcu protected pointer and hence we used to do call_rcu(cfqd->rcu_head) to free up cfqd after one rcu grace period. The problem here is that even though cfqd is around, there are no gurantees that associated request queue (td->queue) or q->queue_lock is still around. A driver might have called blk_cleanup_queue() and release the lock. It might happen that after freeing up the lock we call blkg->key->queue->queue_ock and crash. This is possible in following path. blkiocg_destroy() blkio_unlink_group_fn() cfq_unlink_blkio_group() Hence, wait for an rcu peirod if there are groups which have not been unlinked from blkcg->blkg_list. That way, if there are any groups which are taking cfq_unlink_blkio_group() path, can safely take queue lock. This is how we have taken care of race in throttling logic also. Signed-off-by: Vivek Goyal Signed-off-by: Jens Axboe --- block/cfq-iosched.c | 48 ++++++++++++++++++++++++++++++++++++------------ 1 file changed, 36 insertions(+), 12 deletions(-) diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c index a905b5519ab6..e2e6719832e1 100644 --- a/block/cfq-iosched.c +++ b/block/cfq-iosched.c @@ -300,7 +300,9 @@ struct cfq_data { /* List of cfq groups being managed on this device*/ struct hlist_head cfqg_list; - struct rcu_head rcu; + + /* Number of groups which are on blkcg->blkg_list */ + unsigned int nr_blkcg_linked_grps; }; static struct cfq_group *cfq_get_next_cfqg(struct cfq_data *cfqd); @@ -1063,6 +1065,7 @@ static struct cfq_group * cfq_find_alloc_cfqg(struct cfq_data *cfqd, cfq_blkiocg_add_blkio_group(blkcg, &cfqg->blkg, (void *)cfqd, 0); + cfqd->nr_blkcg_linked_grps++; cfqg->weight = blkcg_get_weight(blkcg, cfqg->blkg.dev); /* Add group on cfqd list */ @@ -3815,15 +3818,11 @@ static void cfq_put_async_queues(struct cfq_data *cfqd) cfq_put_queue(cfqd->async_idle_cfqq); } -static void cfq_cfqd_free(struct rcu_head *head) -{ - kfree(container_of(head, struct cfq_data, rcu)); -} - static void cfq_exit_queue(struct elevator_queue *e) { struct cfq_data *cfqd = e->elevator_data; struct request_queue *q = cfqd->queue; + bool wait = false; cfq_shutdown_timer_wq(cfqd); @@ -3842,7 +3841,13 @@ static void cfq_exit_queue(struct elevator_queue *e) cfq_put_async_queues(cfqd); cfq_release_cfq_groups(cfqd); - cfq_blkiocg_del_blkio_group(&cfqd->root_group.blkg); + + /* + * If there are groups which we could not unlink from blkcg list, + * wait for a rcu period for them to be freed. + */ + if (cfqd->nr_blkcg_linked_grps) + wait = true; spin_unlock_irq(q->queue_lock); @@ -3852,8 +3857,20 @@ static void cfq_exit_queue(struct elevator_queue *e) ida_remove(&cic_index_ida, cfqd->cic_index); spin_unlock(&cic_index_lock); - /* Wait for cfqg->blkg->key accessors to exit their grace periods. */ - call_rcu(&cfqd->rcu, cfq_cfqd_free); + /* + * Wait for cfqg->blkg->key accessors to exit their grace periods. + * Do this wait only if there are other unlinked groups out + * there. This can happen if cgroup deletion path claimed the + * responsibility of cleaning up a group before queue cleanup code + * get to the group. + * + * Do not call synchronize_rcu() unconditionally as there are drivers + * which create/delete request queue hundreds of times during scan/boot + * and synchronize_rcu() can take significant time and slow down boot. + */ + if (wait) + synchronize_rcu(); + kfree(cfqd); } static int cfq_alloc_cic_index(void) @@ -3909,14 +3926,21 @@ static void *cfq_init_queue(struct request_queue *q) #ifdef CONFIG_CFQ_GROUP_IOSCHED /* - * Take a reference to root group which we never drop. This is just - * to make sure that cfq_put_cfqg() does not try to kfree root group + * Set root group reference to 2. One reference will be dropped when + * all groups on cfqd->cfqg_list are being deleted during queue exit. + * Other reference will remain there as we don't want to delete this + * group as it is statically allocated and gets destroyed when + * throtl_data goes away. */ - cfqg->ref = 1; + cfqg->ref = 2; rcu_read_lock(); cfq_blkiocg_add_blkio_group(&blkio_root_cgroup, &cfqg->blkg, (void *)cfqd, 0); rcu_read_unlock(); + cfqd->nr_blkcg_linked_grps++; + + /* Add group on cfqd->cfqg_list */ + hlist_add_head(&cfqg->cfqd_node, &cfqd->cfqg_list); #endif /* * Not strictly needed (since RB_ROOT just clears the node and we -- cgit v1.2.1 From f469a7b4d5b1d1d053200a9015fd25d59c057f49 Mon Sep 17 00:00:00 2001 From: Vivek Goyal Date: Thu, 19 May 2011 15:38:23 -0400 Subject: blk-cgroup: Allow sleeping while dynamically allocating a group Currently, all the cfq_group or throtl_group allocations happen while we are holding ->queue_lock and sleeping is not allowed. Soon, we will move to per cpu stats and also need to allocate the per group stats. As one can not call alloc_percpu() from atomic context as it can sleep, we need to drop ->queue_lock, allocate the group, retake the lock and continue processing. In throttling code, I check the queue DEAD flag again to make sure that driver did not call blk_cleanup_queue() in the mean time. Signed-off-by: Vivek Goyal Signed-off-by: Jens Axboe --- block/blk-core.c | 3 +- block/blk-throttle.c | 141 +++++++++++++++++++++++++++++++++++++++------------ block/cfq-iosched.c | 128 ++++++++++++++++++++++++++++++++++------------ 3 files changed, 205 insertions(+), 67 deletions(-) diff --git a/block/blk-core.c b/block/blk-core.c index 3fe00a14822a..9e8e297374b9 100644 --- a/block/blk-core.c +++ b/block/blk-core.c @@ -1550,7 +1550,8 @@ static inline void __generic_make_request(struct bio *bio) goto end_io; } - blk_throtl_bio(q, &bio); + if (blk_throtl_bio(q, &bio)) + goto end_io; /* * If bio = NULL, bio has been throttled and will be submitted diff --git a/block/blk-throttle.c b/block/blk-throttle.c index fa9a900c1254..c201967b33cd 100644 --- a/block/blk-throttle.c +++ b/block/blk-throttle.c @@ -188,20 +188,46 @@ throtl_add_group_to_td_list(struct throtl_data *td, struct throtl_grp *tg) td->nr_undestroyed_grps++; } -static struct throtl_grp * throtl_find_alloc_tg(struct throtl_data *td, - struct blkio_cgroup *blkcg) +static void throtl_init_add_tg_lists(struct throtl_data *td, + struct throtl_grp *tg, struct blkio_cgroup *blkcg) +{ + struct backing_dev_info *bdi = &td->queue->backing_dev_info; + unsigned int major, minor; + + /* Add group onto cgroup list */ + sscanf(dev_name(bdi->dev), "%u:%u", &major, &minor); + blkiocg_add_blkio_group(blkcg, &tg->blkg, (void *)td, + MKDEV(major, minor), BLKIO_POLICY_THROTL); + + tg->bps[READ] = blkcg_get_read_bps(blkcg, tg->blkg.dev); + tg->bps[WRITE] = blkcg_get_write_bps(blkcg, tg->blkg.dev); + tg->iops[READ] = blkcg_get_read_iops(blkcg, tg->blkg.dev); + tg->iops[WRITE] = blkcg_get_write_iops(blkcg, tg->blkg.dev); + + throtl_add_group_to_td_list(td, tg); +} + +/* Should be called without queue lock and outside of rcu period */ +static struct throtl_grp *throtl_alloc_tg(struct throtl_data *td) +{ + struct throtl_grp *tg = NULL; + + tg = kzalloc_node(sizeof(*tg), GFP_ATOMIC, td->queue->node); + if (!tg) + return NULL; + + throtl_init_group(tg); + return tg; +} + +static struct +throtl_grp *throtl_find_tg(struct throtl_data *td, struct blkio_cgroup *blkcg) { struct throtl_grp *tg = NULL; void *key = td; struct backing_dev_info *bdi = &td->queue->backing_dev_info; unsigned int major, minor; - /* - * TODO: Speed up blkiocg_lookup_group() by maintaining a radix - * tree of blkg (instead of traversing through hash list all - * the time. - */ - /* * This is the common case when there are no blkio cgroups. * Avoid lookup in this case @@ -215,43 +241,83 @@ static struct throtl_grp * throtl_find_alloc_tg(struct throtl_data *td, if (tg && !tg->blkg.dev && bdi->dev && dev_name(bdi->dev)) { sscanf(dev_name(bdi->dev), "%u:%u", &major, &minor); tg->blkg.dev = MKDEV(major, minor); - goto done; } - if (tg) - goto done; - - tg = kzalloc_node(sizeof(*tg), GFP_ATOMIC, td->queue->node); - if (!tg) - goto done; - - throtl_init_group(tg); - - /* Add group onto cgroup list */ - sscanf(dev_name(bdi->dev), "%u:%u", &major, &minor); - blkiocg_add_blkio_group(blkcg, &tg->blkg, (void *)td, - MKDEV(major, minor), BLKIO_POLICY_THROTL); - - tg->bps[READ] = blkcg_get_read_bps(blkcg, tg->blkg.dev); - tg->bps[WRITE] = blkcg_get_write_bps(blkcg, tg->blkg.dev); - tg->iops[READ] = blkcg_get_read_iops(blkcg, tg->blkg.dev); - tg->iops[WRITE] = blkcg_get_write_iops(blkcg, tg->blkg.dev); - - throtl_add_group_to_td_list(td, tg); -done: return tg; } +/* + * This function returns with queue lock unlocked in case of error, like + * request queue is no more + */ static struct throtl_grp * throtl_get_tg(struct throtl_data *td) { - struct throtl_grp *tg = NULL; + struct throtl_grp *tg = NULL, *__tg = NULL; struct blkio_cgroup *blkcg; + struct request_queue *q = td->queue; rcu_read_lock(); blkcg = task_blkio_cgroup(current); - tg = throtl_find_alloc_tg(td, blkcg); - if (!tg) + tg = throtl_find_tg(td, blkcg); + if (tg) { + rcu_read_unlock(); + return tg; + } + + /* + * Need to allocate a group. Allocation of group also needs allocation + * of per cpu stats which in-turn takes a mutex() and can block. Hence + * we need to drop rcu lock and queue_lock before we call alloc + * + * Take the request queue reference to make sure queue does not + * go away once we return from allocation. + */ + blk_get_queue(q); + rcu_read_unlock(); + spin_unlock_irq(q->queue_lock); + + tg = throtl_alloc_tg(td); + /* + * We might have slept in group allocation. Make sure queue is not + * dead + */ + if (unlikely(test_bit(QUEUE_FLAG_DEAD, &q->queue_flags))) { + blk_put_queue(q); + if (tg) + kfree(tg); + + return ERR_PTR(-ENODEV); + } + blk_put_queue(q); + + /* Group allocated and queue is still alive. take the lock */ + spin_lock_irq(q->queue_lock); + + /* + * Initialize the new group. After sleeping, read the blkcg again. + */ + rcu_read_lock(); + blkcg = task_blkio_cgroup(current); + + /* + * If some other thread already allocated the group while we were + * not holding queue lock, free up the group + */ + __tg = throtl_find_tg(td, blkcg); + + if (__tg) { + kfree(tg); + rcu_read_unlock(); + return __tg; + } + + /* Group allocation failed. Account the IO to root group */ + if (!tg) { tg = &td->root_tg; + return tg; + } + + throtl_init_add_tg_lists(td, tg, blkcg); rcu_read_unlock(); return tg; } @@ -1014,6 +1080,15 @@ int blk_throtl_bio(struct request_queue *q, struct bio **biop) spin_lock_irq(q->queue_lock); tg = throtl_get_tg(td); + if (IS_ERR(tg)) { + if (PTR_ERR(tg) == -ENODEV) { + /* + * Queue is gone. No queue lock held here. + */ + return -ENODEV; + } + } + if (tg->nr_queued[rw]) { /* * There is already another bio queued in same dir. No diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c index e2e6719832e1..606020fe93f3 100644 --- a/block/cfq-iosched.c +++ b/block/cfq-iosched.c @@ -1016,28 +1016,47 @@ void cfq_update_blkio_group_weight(void *key, struct blkio_group *blkg, cfqg->needs_update = true; } -static struct cfq_group * cfq_find_alloc_cfqg(struct cfq_data *cfqd, - struct blkio_cgroup *blkcg) +static void cfq_init_add_cfqg_lists(struct cfq_data *cfqd, + struct cfq_group *cfqg, struct blkio_cgroup *blkcg) { - struct cfq_group *cfqg = NULL; - void *key = cfqd; - int i, j; - struct cfq_rb_root *st; struct backing_dev_info *bdi = &cfqd->queue->backing_dev_info; unsigned int major, minor; - cfqg = cfqg_of_blkg(blkiocg_lookup_group(blkcg, key)); - if (cfqg && !cfqg->blkg.dev && bdi->dev && dev_name(bdi->dev)) { + /* + * Add group onto cgroup list. It might happen that bdi->dev is + * not initialized yet. Initialize this new group without major + * and minor info and this info will be filled in once a new thread + * comes for IO. + */ + if (bdi->dev) { sscanf(dev_name(bdi->dev), "%u:%u", &major, &minor); - cfqg->blkg.dev = MKDEV(major, minor); - goto done; - } - if (cfqg) - goto done; + cfq_blkiocg_add_blkio_group(blkcg, &cfqg->blkg, + (void *)cfqd, MKDEV(major, minor)); + } else + cfq_blkiocg_add_blkio_group(blkcg, &cfqg->blkg, + (void *)cfqd, 0); + + cfqd->nr_blkcg_linked_grps++; + cfqg->weight = blkcg_get_weight(blkcg, cfqg->blkg.dev); + + /* Add group on cfqd list */ + hlist_add_head(&cfqg->cfqd_node, &cfqd->cfqg_list); +} + +/* + * Should be called from sleepable context. No request queue lock as per + * cpu stats are allocated dynamically and alloc_percpu needs to be called + * from sleepable context. + */ +static struct cfq_group * cfq_alloc_cfqg(struct cfq_data *cfqd) +{ + struct cfq_group *cfqg = NULL; + int i, j; + struct cfq_rb_root *st; cfqg = kzalloc_node(sizeof(*cfqg), GFP_ATOMIC, cfqd->queue->node); if (!cfqg) - goto done; + return NULL; for_each_cfqg_st(cfqg, i, j, st) *st = CFQ_RB_ROOT; @@ -1050,28 +1069,31 @@ static struct cfq_group * cfq_find_alloc_cfqg(struct cfq_data *cfqd, * or cgroup deletion path depending on who is exiting first. */ cfqg->ref = 1; + return cfqg; +} + +static struct cfq_group * +cfq_find_cfqg(struct cfq_data *cfqd, struct blkio_cgroup *blkcg) +{ + struct cfq_group *cfqg = NULL; + void *key = cfqd; + struct backing_dev_info *bdi = &cfqd->queue->backing_dev_info; + unsigned int major, minor; /* - * Add group onto cgroup list. It might happen that bdi->dev is - * not initialized yet. Initialize this new group without major - * and minor info and this info will be filled in once a new thread - * comes for IO. See code above. + * This is the common case when there are no blkio cgroups. + * Avoid lookup in this case */ - if (bdi->dev) { - sscanf(dev_name(bdi->dev), "%u:%u", &major, &minor); - cfq_blkiocg_add_blkio_group(blkcg, &cfqg->blkg, (void *)cfqd, - MKDEV(major, minor)); - } else - cfq_blkiocg_add_blkio_group(blkcg, &cfqg->blkg, (void *)cfqd, - 0); - - cfqd->nr_blkcg_linked_grps++; - cfqg->weight = blkcg_get_weight(blkcg, cfqg->blkg.dev); + if (blkcg == &blkio_root_cgroup) + cfqg = &cfqd->root_group; + else + cfqg = cfqg_of_blkg(blkiocg_lookup_group(blkcg, key)); - /* Add group on cfqd list */ - hlist_add_head(&cfqg->cfqd_node, &cfqd->cfqg_list); + if (cfqg && !cfqg->blkg.dev && bdi->dev && dev_name(bdi->dev)) { + sscanf(dev_name(bdi->dev), "%u:%u", &major, &minor); + cfqg->blkg.dev = MKDEV(major, minor); + } -done: return cfqg; } @@ -1082,13 +1104,53 @@ done: static struct cfq_group *cfq_get_cfqg(struct cfq_data *cfqd) { struct blkio_cgroup *blkcg; - struct cfq_group *cfqg = NULL; + struct cfq_group *cfqg = NULL, *__cfqg = NULL; + struct request_queue *q = cfqd->queue; + + rcu_read_lock(); + blkcg = task_blkio_cgroup(current); + cfqg = cfq_find_cfqg(cfqd, blkcg); + if (cfqg) { + rcu_read_unlock(); + return cfqg; + } + + /* + * Need to allocate a group. Allocation of group also needs allocation + * of per cpu stats which in-turn takes a mutex() and can block. Hence + * we need to drop rcu lock and queue_lock before we call alloc. + * + * Not taking any queue reference here and assuming that queue is + * around by the time we return. CFQ queue allocation code does + * the same. It might be racy though. + */ + + rcu_read_unlock(); + spin_unlock_irq(q->queue_lock); + + cfqg = cfq_alloc_cfqg(cfqd); + + spin_lock_irq(q->queue_lock); rcu_read_lock(); blkcg = task_blkio_cgroup(current); - cfqg = cfq_find_alloc_cfqg(cfqd, blkcg); + + /* + * If some other thread already allocated the group while we were + * not holding queue lock, free up the group + */ + __cfqg = cfq_find_cfqg(cfqd, blkcg); + + if (__cfqg) { + kfree(cfqg); + rcu_read_unlock(); + return __cfqg; + } + if (!cfqg) cfqg = &cfqd->root_group; + + cfq_init_add_cfqg_lists(cfqd, cfqg, blkcg); rcu_read_unlock(); return cfqg; } -- cgit v1.2.1 From 29b125892f3317ada86b662e0b6ebc0f79be9037 Mon Sep 17 00:00:00 2001 From: Vivek Goyal Date: Thu, 19 May 2011 15:38:24 -0400 Subject: blk-throttle: Dynamically allocate root group Currently, we allocate root throtl_grp statically. But as we will be introducing per cpu stat pointers and that will be allocated dynamically even for root group, we might as well make whole root throtl_grp allocation dynamic and treat it in same manner as other groups. Signed-off-by: Vivek Goyal Signed-off-by: Jens Axboe --- block/blk-throttle.c | 27 ++++++++++++--------------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/block/blk-throttle.c b/block/blk-throttle.c index c201967b33cd..68f2ac3f3b07 100644 --- a/block/blk-throttle.c +++ b/block/blk-throttle.c @@ -88,7 +88,7 @@ struct throtl_data /* service tree for active throtl groups */ struct throtl_rb_root tg_service_tree; - struct throtl_grp root_tg; + struct throtl_grp *root_tg; struct request_queue *queue; /* Total Number of queued bios on READ and WRITE lists */ @@ -233,7 +233,7 @@ throtl_grp *throtl_find_tg(struct throtl_data *td, struct blkio_cgroup *blkcg) * Avoid lookup in this case */ if (blkcg == &blkio_root_cgroup) - tg = &td->root_tg; + tg = td->root_tg; else tg = tg_of_blkg(blkiocg_lookup_group(blkcg, key)); @@ -313,7 +313,7 @@ static struct throtl_grp * throtl_get_tg(struct throtl_data *td) /* Group allocation failed. Account the IO to root group */ if (!tg) { - tg = &td->root_tg; + tg = td->root_tg; return tg; } @@ -1153,18 +1153,16 @@ int blk_throtl_init(struct request_queue *q) td->limits_changed = false; INIT_DELAYED_WORK(&td->throtl_work, blk_throtl_work); - /* Init root group */ - tg = &td->root_tg; - throtl_init_group(tg); + /* alloc and Init root group. */ + td->queue = q; + tg = throtl_alloc_tg(td); - /* - * Set root group reference to 2. One reference will be dropped when - * all groups on tg_list are being deleted during queue exit. Other - * reference will remain there as we don't want to delete this group - * as it is statically allocated and gets destroyed when throtl_data - * goes away. - */ - atomic_inc(&tg->ref); + if (!tg) { + kfree(td); + return -ENOMEM; + } + + td->root_tg = tg; rcu_read_lock(); blkiocg_add_blkio_group(&blkio_root_cgroup, &tg->blkg, (void *)td, @@ -1173,7 +1171,6 @@ int blk_throtl_init(struct request_queue *q) throtl_add_group_to_td_list(td, tg); /* Attach throtl data to request queue */ - td->queue = q; q->td = td; return 0; } -- cgit v1.2.1 From 269f541555d8f5da322d592fb3e13e23ed62d80c Mon Sep 17 00:00:00 2001 From: Vivek Goyal Date: Thu, 19 May 2011 15:38:25 -0400 Subject: blk-throttle: Introduce a helper function to fill in device details A helper function for the code which is used at 2-3 places. Makes reading code little easier. Signed-off-by: Vivek Goyal Signed-off-by: Jens Axboe --- block/blk-throttle.c | 35 +++++++++++++++++++++++------------ 1 file changed, 23 insertions(+), 12 deletions(-) diff --git a/block/blk-throttle.c b/block/blk-throttle.c index 68f2ac3f3b07..97ea7f82477d 100644 --- a/block/blk-throttle.c +++ b/block/blk-throttle.c @@ -188,16 +188,34 @@ throtl_add_group_to_td_list(struct throtl_data *td, struct throtl_grp *tg) td->nr_undestroyed_grps++; } -static void throtl_init_add_tg_lists(struct throtl_data *td, - struct throtl_grp *tg, struct blkio_cgroup *blkcg) +static void +__throtl_tg_fill_dev_details(struct throtl_data *td, struct throtl_grp *tg) { struct backing_dev_info *bdi = &td->queue->backing_dev_info; unsigned int major, minor; + if (!tg || tg->blkg.dev) + return; + + /* + * Fill in device details for a group which might not have been + * filled at group creation time as queue was being instantiated + * and driver had not attached a device yet + */ + if (bdi->dev && dev_name(bdi->dev)) { + sscanf(dev_name(bdi->dev), "%u:%u", &major, &minor); + tg->blkg.dev = MKDEV(major, minor); + } +} + +static void throtl_init_add_tg_lists(struct throtl_data *td, + struct throtl_grp *tg, struct blkio_cgroup *blkcg) +{ + __throtl_tg_fill_dev_details(td, tg); + /* Add group onto cgroup list */ - sscanf(dev_name(bdi->dev), "%u:%u", &major, &minor); blkiocg_add_blkio_group(blkcg, &tg->blkg, (void *)td, - MKDEV(major, minor), BLKIO_POLICY_THROTL); + tg->blkg.dev, BLKIO_POLICY_THROTL); tg->bps[READ] = blkcg_get_read_bps(blkcg, tg->blkg.dev); tg->bps[WRITE] = blkcg_get_write_bps(blkcg, tg->blkg.dev); @@ -225,8 +243,6 @@ throtl_grp *throtl_find_tg(struct throtl_data *td, struct blkio_cgroup *blkcg) { struct throtl_grp *tg = NULL; void *key = td; - struct backing_dev_info *bdi = &td->queue->backing_dev_info; - unsigned int major, minor; /* * This is the common case when there are no blkio cgroups. @@ -237,12 +253,7 @@ throtl_grp *throtl_find_tg(struct throtl_data *td, struct blkio_cgroup *blkcg) else tg = tg_of_blkg(blkiocg_lookup_group(blkcg, key)); - /* Fill in device details for root group */ - if (tg && !tg->blkg.dev && bdi->dev && dev_name(bdi->dev)) { - sscanf(dev_name(bdi->dev), "%u:%u", &major, &minor); - tg->blkg.dev = MKDEV(major, minor); - } - + __throtl_tg_fill_dev_details(td, tg); return tg; } -- cgit v1.2.1 From 5617cbef7723952cbdff28c7a10ff8a254945f4f Mon Sep 17 00:00:00 2001 From: Vivek Goyal Date: Thu, 19 May 2011 15:38:26 -0400 Subject: blk-throttle: Use helper function to add root throtl group to lists Use same helper function for root group as we use with dynamically allocated groups to add it to various lists. Signed-off-by: Vivek Goyal Signed-off-by: Jens Axboe --- block/blk-throttle.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/block/blk-throttle.c b/block/blk-throttle.c index 97ea7f82477d..b9412d1cea9e 100644 --- a/block/blk-throttle.c +++ b/block/blk-throttle.c @@ -1176,10 +1176,8 @@ int blk_throtl_init(struct request_queue *q) td->root_tg = tg; rcu_read_lock(); - blkiocg_add_blkio_group(&blkio_root_cgroup, &tg->blkg, (void *)td, - 0, BLKIO_POLICY_THROTL); + throtl_init_add_tg_lists(td, tg, &blkio_root_cgroup); rcu_read_unlock(); - throtl_add_group_to_td_list(td, tg); /* Attach throtl data to request queue */ q->td = td; -- cgit v1.2.1 From 4843c69d496a8d2e4caab6182fe016b9a79136e0 Mon Sep 17 00:00:00 2001 From: Vivek Goyal Date: Thu, 19 May 2011 15:38:27 -0400 Subject: blk-throttle: Free up a group only after one rcu grace period Soon we will allow accessing a throtl_grp under rcu_read_lock(). Hence start freeing up throtl_grp after one rcu grace period. Signed-off-by: Vivek Goyal Signed-off-by: Jens Axboe --- block/blk-throttle.c | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/block/blk-throttle.c b/block/blk-throttle.c index b9412d1cea9e..90ad40735f73 100644 --- a/block/blk-throttle.c +++ b/block/blk-throttle.c @@ -78,6 +78,8 @@ struct throtl_grp { /* Some throttle limits got updated for the group */ int limits_changed; + + struct rcu_head rcu_head; }; struct throtl_data @@ -151,12 +153,30 @@ static inline struct throtl_grp *throtl_ref_get_tg(struct throtl_grp *tg) return tg; } +static void throtl_free_tg(struct rcu_head *head) +{ + struct throtl_grp *tg; + + tg = container_of(head, struct throtl_grp, rcu_head); + kfree(tg); +} + static void throtl_put_tg(struct throtl_grp *tg) { BUG_ON(atomic_read(&tg->ref) <= 0); if (!atomic_dec_and_test(&tg->ref)) return; - kfree(tg); + + /* + * A group is freed in rcu manner. But having an rcu lock does not + * mean that one can access all the fields of blkg and assume these + * are valid. For example, don't try to follow throtl_data and + * request queue links. + * + * Having a reference to blkg under an rcu allows acess to only + * values local to groups like group stats and group rate limits + */ + call_rcu(&tg->rcu_head, throtl_free_tg); } static void throtl_init_group(struct throtl_grp *tg) -- cgit v1.2.1 From 5624a4e445e2ec27582984b068d7bf7f127cee10 Mon Sep 17 00:00:00 2001 From: Vivek Goyal Date: Thu, 19 May 2011 15:38:28 -0400 Subject: blk-throttle: Make dispatch stats per cpu Currently we take blkg_stat lock for even updating the stats. So even if a group has no throttling rules (common case for root group), we end up taking blkg_lock, for updating the stats. Make dispatch stats per cpu so that these can be updated without taking blkg lock. If cpu goes offline, these stats simply disappear. No protection has been provided for that yet. Do we really need anything for that? Signed-off-by: Vivek Goyal Signed-off-by: Jens Axboe --- block/blk-cgroup.c | 135 ++++++++++++++++++++++++++++++++++++++------------- block/blk-cgroup.h | 27 ++++++++--- block/blk-throttle.c | 9 ++++ block/cfq-iosched.c | 18 ++++++- 4 files changed, 147 insertions(+), 42 deletions(-) diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c index b0592bca6970..34bfcefdd924 100644 --- a/block/blk-cgroup.c +++ b/block/blk-cgroup.c @@ -392,20 +392,22 @@ void blkiocg_update_timeslice_used(struct blkio_group *blkg, unsigned long time, } EXPORT_SYMBOL_GPL(blkiocg_update_timeslice_used); +/* + * should be called under rcu read lock or queue lock to make sure blkg pointer + * is valid. + */ void blkiocg_update_dispatch_stats(struct blkio_group *blkg, uint64_t bytes, bool direction, bool sync) { - struct blkio_group_stats *stats; - unsigned long flags; + struct blkio_group_stats_cpu *stats_cpu; - spin_lock_irqsave(&blkg->stats_lock, flags); - stats = &blkg->stats; - stats->sectors += bytes >> 9; - blkio_add_stat(stats->stat_arr[BLKIO_STAT_SERVICED], 1, direction, - sync); - blkio_add_stat(stats->stat_arr[BLKIO_STAT_SERVICE_BYTES], bytes, - direction, sync); - spin_unlock_irqrestore(&blkg->stats_lock, flags); + stats_cpu = this_cpu_ptr(blkg->stats_cpu); + + stats_cpu->sectors += bytes >> 9; + blkio_add_stat(stats_cpu->stat_arr_cpu[BLKIO_STAT_CPU_SERVICED], + 1, direction, sync); + blkio_add_stat(stats_cpu->stat_arr_cpu[BLKIO_STAT_CPU_SERVICE_BYTES], + bytes, direction, sync); } EXPORT_SYMBOL_GPL(blkiocg_update_dispatch_stats); @@ -440,6 +442,20 @@ void blkiocg_update_io_merged_stats(struct blkio_group *blkg, bool direction, } EXPORT_SYMBOL_GPL(blkiocg_update_io_merged_stats); +/* + * This function allocates the per cpu stats for blkio_group. Should be called + * from sleepable context as alloc_per_cpu() requires that. + */ +int blkio_alloc_blkg_stats(struct blkio_group *blkg) +{ + /* Allocate memory for per cpu stats */ + blkg->stats_cpu = alloc_percpu(struct blkio_group_stats_cpu); + if (!blkg->stats_cpu) + return -ENOMEM; + return 0; +} +EXPORT_SYMBOL_GPL(blkio_alloc_blkg_stats); + void blkiocg_add_blkio_group(struct blkio_cgroup *blkcg, struct blkio_group *blkg, void *key, dev_t dev, enum blkio_policy_id plid) @@ -600,6 +616,53 @@ static uint64_t blkio_fill_stat(char *str, int chars_left, uint64_t val, return val; } + +static uint64_t blkio_read_stat_cpu(struct blkio_group *blkg, + enum stat_type_cpu type, enum stat_sub_type sub_type) +{ + int cpu; + struct blkio_group_stats_cpu *stats_cpu; + uint64_t val = 0; + + for_each_possible_cpu(cpu) { + stats_cpu = per_cpu_ptr(blkg->stats_cpu, cpu); + + if (type == BLKIO_STAT_CPU_SECTORS) + val += stats_cpu->sectors; + else + val += stats_cpu->stat_arr_cpu[type][sub_type]; + } + + return val; +} + +static uint64_t blkio_get_stat_cpu(struct blkio_group *blkg, + struct cgroup_map_cb *cb, dev_t dev, enum stat_type_cpu type) +{ + uint64_t disk_total, val; + char key_str[MAX_KEY_LEN]; + enum stat_sub_type sub_type; + + if (type == BLKIO_STAT_CPU_SECTORS) { + val = blkio_read_stat_cpu(blkg, type, 0); + return blkio_fill_stat(key_str, MAX_KEY_LEN - 1, val, cb, dev); + } + + for (sub_type = BLKIO_STAT_READ; sub_type < BLKIO_STAT_TOTAL; + sub_type++) { + blkio_get_key_name(sub_type, dev, key_str, MAX_KEY_LEN, false); + val = blkio_read_stat_cpu(blkg, type, sub_type); + cb->fill(cb, key_str, val); + } + + disk_total = blkio_read_stat_cpu(blkg, type, BLKIO_STAT_READ) + + blkio_read_stat_cpu(blkg, type, BLKIO_STAT_WRITE); + + blkio_get_key_name(BLKIO_STAT_TOTAL, dev, key_str, MAX_KEY_LEN, false); + cb->fill(cb, key_str, disk_total); + return disk_total; +} + /* This should be called with blkg->stats_lock held */ static uint64_t blkio_get_stat(struct blkio_group *blkg, struct cgroup_map_cb *cb, dev_t dev, enum stat_type type) @@ -611,9 +674,6 @@ static uint64_t blkio_get_stat(struct blkio_group *blkg, if (type == BLKIO_STAT_TIME) return blkio_fill_stat(key_str, MAX_KEY_LEN - 1, blkg->stats.time, cb, dev); - if (type == BLKIO_STAT_SECTORS) - return blkio_fill_stat(key_str, MAX_KEY_LEN - 1, - blkg->stats.sectors, cb, dev); #ifdef CONFIG_DEBUG_BLK_CGROUP if (type == BLKIO_STAT_UNACCOUNTED_TIME) return blkio_fill_stat(key_str, MAX_KEY_LEN - 1, @@ -1077,8 +1137,8 @@ static int blkiocg_file_read(struct cgroup *cgrp, struct cftype *cft, } static int blkio_read_blkg_stats(struct blkio_cgroup *blkcg, - struct cftype *cft, struct cgroup_map_cb *cb, enum stat_type type, - bool show_total) + struct cftype *cft, struct cgroup_map_cb *cb, + enum stat_type type, bool show_total, bool pcpu) { struct blkio_group *blkg; struct hlist_node *n; @@ -1089,10 +1149,15 @@ static int blkio_read_blkg_stats(struct blkio_cgroup *blkcg, if (blkg->dev) { if (!cftype_blkg_same_policy(cft, blkg)) continue; - spin_lock_irq(&blkg->stats_lock); - cgroup_total += blkio_get_stat(blkg, cb, blkg->dev, - type); - spin_unlock_irq(&blkg->stats_lock); + if (pcpu) + cgroup_total += blkio_get_stat_cpu(blkg, cb, + blkg->dev, type); + else { + spin_lock_irq(&blkg->stats_lock); + cgroup_total += blkio_get_stat(blkg, cb, + blkg->dev, type); + spin_unlock_irq(&blkg->stats_lock); + } } } if (show_total) @@ -1116,47 +1181,47 @@ static int blkiocg_file_read_map(struct cgroup *cgrp, struct cftype *cft, switch(name) { case BLKIO_PROP_time: return blkio_read_blkg_stats(blkcg, cft, cb, - BLKIO_STAT_TIME, 0); + BLKIO_STAT_TIME, 0, 0); case BLKIO_PROP_sectors: return blkio_read_blkg_stats(blkcg, cft, cb, - BLKIO_STAT_SECTORS, 0); + BLKIO_STAT_CPU_SECTORS, 0, 1); case BLKIO_PROP_io_service_bytes: return blkio_read_blkg_stats(blkcg, cft, cb, - BLKIO_STAT_SERVICE_BYTES, 1); + BLKIO_STAT_CPU_SERVICE_BYTES, 1, 1); case BLKIO_PROP_io_serviced: return blkio_read_blkg_stats(blkcg, cft, cb, - BLKIO_STAT_SERVICED, 1); + BLKIO_STAT_CPU_SERVICED, 1, 1); case BLKIO_PROP_io_service_time: return blkio_read_blkg_stats(blkcg, cft, cb, - BLKIO_STAT_SERVICE_TIME, 1); + BLKIO_STAT_SERVICE_TIME, 1, 0); case BLKIO_PROP_io_wait_time: return blkio_read_blkg_stats(blkcg, cft, cb, - BLKIO_STAT_WAIT_TIME, 1); + BLKIO_STAT_WAIT_TIME, 1, 0); case BLKIO_PROP_io_merged: return blkio_read_blkg_stats(blkcg, cft, cb, - BLKIO_STAT_MERGED, 1); + BLKIO_STAT_MERGED, 1, 0); case BLKIO_PROP_io_queued: return blkio_read_blkg_stats(blkcg, cft, cb, - BLKIO_STAT_QUEUED, 1); + BLKIO_STAT_QUEUED, 1, 0); #ifdef CONFIG_DEBUG_BLK_CGROUP case BLKIO_PROP_unaccounted_time: return blkio_read_blkg_stats(blkcg, cft, cb, - BLKIO_STAT_UNACCOUNTED_TIME, 0); + BLKIO_STAT_UNACCOUNTED_TIME, 0, 0); case BLKIO_PROP_dequeue: return blkio_read_blkg_stats(blkcg, cft, cb, - BLKIO_STAT_DEQUEUE, 0); + BLKIO_STAT_DEQUEUE, 0, 0); case BLKIO_PROP_avg_queue_size: return blkio_read_blkg_stats(blkcg, cft, cb, - BLKIO_STAT_AVG_QUEUE_SIZE, 0); + BLKIO_STAT_AVG_QUEUE_SIZE, 0, 0); case BLKIO_PROP_group_wait_time: return blkio_read_blkg_stats(blkcg, cft, cb, - BLKIO_STAT_GROUP_WAIT_TIME, 0); + BLKIO_STAT_GROUP_WAIT_TIME, 0, 0); case BLKIO_PROP_idle_time: return blkio_read_blkg_stats(blkcg, cft, cb, - BLKIO_STAT_IDLE_TIME, 0); + BLKIO_STAT_IDLE_TIME, 0, 0); case BLKIO_PROP_empty_time: return blkio_read_blkg_stats(blkcg, cft, cb, - BLKIO_STAT_EMPTY_TIME, 0); + BLKIO_STAT_EMPTY_TIME, 0, 0); #endif default: BUG(); @@ -1166,10 +1231,10 @@ static int blkiocg_file_read_map(struct cgroup *cgrp, struct cftype *cft, switch(name){ case BLKIO_THROTL_io_service_bytes: return blkio_read_blkg_stats(blkcg, cft, cb, - BLKIO_STAT_SERVICE_BYTES, 1); + BLKIO_STAT_CPU_SERVICE_BYTES, 1, 1); case BLKIO_THROTL_io_serviced: return blkio_read_blkg_stats(blkcg, cft, cb, - BLKIO_STAT_SERVICED, 1); + BLKIO_STAT_CPU_SERVICED, 1, 1); default: BUG(); } diff --git a/block/blk-cgroup.h b/block/blk-cgroup.h index 63f1ef4450d7..fd730a24b491 100644 --- a/block/blk-cgroup.h +++ b/block/blk-cgroup.h @@ -36,10 +36,6 @@ enum stat_type { * request completion for IOs doen by this cgroup. This may not be * accurate when NCQ is turned on. */ BLKIO_STAT_SERVICE_TIME = 0, - /* Total bytes transferred */ - BLKIO_STAT_SERVICE_BYTES, - /* Total IOs serviced, post merge */ - BLKIO_STAT_SERVICED, /* Total time spent waiting in scheduler queue in ns */ BLKIO_STAT_WAIT_TIME, /* Number of IOs merged */ @@ -48,7 +44,6 @@ enum stat_type { BLKIO_STAT_QUEUED, /* All the single valued stats go below this */ BLKIO_STAT_TIME, - BLKIO_STAT_SECTORS, #ifdef CONFIG_DEBUG_BLK_CGROUP /* Time not charged to this cgroup */ BLKIO_STAT_UNACCOUNTED_TIME, @@ -60,6 +55,16 @@ enum stat_type { #endif }; +/* Per cpu stats */ +enum stat_type_cpu { + BLKIO_STAT_CPU_SECTORS, + /* Total bytes transferred */ + BLKIO_STAT_CPU_SERVICE_BYTES, + /* Total IOs serviced, post merge */ + BLKIO_STAT_CPU_SERVICED, + BLKIO_STAT_CPU_NR +}; + enum stat_sub_type { BLKIO_STAT_READ = 0, BLKIO_STAT_WRITE, @@ -116,7 +121,6 @@ struct blkio_cgroup { struct blkio_group_stats { /* total disk time and nr sectors dispatched by this group */ uint64_t time; - uint64_t sectors; uint64_t stat_arr[BLKIO_STAT_QUEUED + 1][BLKIO_STAT_TOTAL]; #ifdef CONFIG_DEBUG_BLK_CGROUP /* Time not charged to this cgroup */ @@ -146,6 +150,12 @@ struct blkio_group_stats { #endif }; +/* Per cpu blkio group stats */ +struct blkio_group_stats_cpu { + uint64_t sectors; + uint64_t stat_arr_cpu[BLKIO_STAT_CPU_NR][BLKIO_STAT_TOTAL]; +}; + struct blkio_group { /* An rcu protected unique identifier for the group */ void *key; @@ -161,6 +171,8 @@ struct blkio_group { /* Need to serialize the stats in the case of reset/update */ spinlock_t stats_lock; struct blkio_group_stats stats; + /* Per cpu stats pointer */ + struct blkio_group_stats_cpu __percpu *stats_cpu; }; struct blkio_policy_node { @@ -296,6 +308,7 @@ extern struct blkio_cgroup *task_blkio_cgroup(struct task_struct *tsk); extern void blkiocg_add_blkio_group(struct blkio_cgroup *blkcg, struct blkio_group *blkg, void *key, dev_t dev, enum blkio_policy_id plid); +extern int blkio_alloc_blkg_stats(struct blkio_group *blkg); extern int blkiocg_del_blkio_group(struct blkio_group *blkg); extern struct blkio_group *blkiocg_lookup_group(struct blkio_cgroup *blkcg, void *key); @@ -323,6 +336,8 @@ static inline void blkiocg_add_blkio_group(struct blkio_cgroup *blkcg, struct blkio_group *blkg, void *key, dev_t dev, enum blkio_policy_id plid) {} +static inline int blkio_alloc_blkg_stats(struct blkio_group *blkg) { return 0; } + static inline int blkiocg_del_blkio_group(struct blkio_group *blkg) { return 0; } diff --git a/block/blk-throttle.c b/block/blk-throttle.c index 90ad40735f73..c29a5a8cc18c 100644 --- a/block/blk-throttle.c +++ b/block/blk-throttle.c @@ -158,6 +158,7 @@ static void throtl_free_tg(struct rcu_head *head) struct throtl_grp *tg; tg = container_of(head, struct throtl_grp, rcu_head); + free_percpu(tg->blkg.stats_cpu); kfree(tg); } @@ -249,11 +250,19 @@ static void throtl_init_add_tg_lists(struct throtl_data *td, static struct throtl_grp *throtl_alloc_tg(struct throtl_data *td) { struct throtl_grp *tg = NULL; + int ret; tg = kzalloc_node(sizeof(*tg), GFP_ATOMIC, td->queue->node); if (!tg) return NULL; + ret = blkio_alloc_blkg_stats(&tg->blkg); + + if (ret) { + kfree(tg); + return NULL; + } + throtl_init_group(tg); return tg; } diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c index 606020fe93f3..d646b279c8bb 100644 --- a/block/cfq-iosched.c +++ b/block/cfq-iosched.c @@ -1051,7 +1051,7 @@ static void cfq_init_add_cfqg_lists(struct cfq_data *cfqd, static struct cfq_group * cfq_alloc_cfqg(struct cfq_data *cfqd) { struct cfq_group *cfqg = NULL; - int i, j; + int i, j, ret; struct cfq_rb_root *st; cfqg = kzalloc_node(sizeof(*cfqg), GFP_ATOMIC, cfqd->queue->node); @@ -1069,6 +1069,13 @@ static struct cfq_group * cfq_alloc_cfqg(struct cfq_data *cfqd) * or cgroup deletion path depending on who is exiting first. */ cfqg->ref = 1; + + ret = blkio_alloc_blkg_stats(&cfqg->blkg); + if (ret) { + kfree(cfqg); + return NULL; + } + return cfqg; } @@ -1183,6 +1190,7 @@ static void cfq_put_cfqg(struct cfq_group *cfqg) return; for_each_cfqg_st(cfqg, i, j, st) BUG_ON(!RB_EMPTY_ROOT(&st->rb)); + free_percpu(cfqg->blkg.stats_cpu); kfree(cfqg); } @@ -3995,7 +4003,15 @@ static void *cfq_init_queue(struct request_queue *q) * throtl_data goes away. */ cfqg->ref = 2; + + if (blkio_alloc_blkg_stats(&cfqg->blkg)) { + kfree(cfqg); + kfree(cfqd); + return NULL; + } + rcu_read_lock(); + cfq_blkiocg_add_blkio_group(&blkio_root_cgroup, &cfqg->blkg, (void *)cfqd, 0); rcu_read_unlock(); -- cgit v1.2.1 From 575969a0dd3fe65c6556bcb8f87c42303326ea55 Mon Sep 17 00:00:00 2001 From: Vivek Goyal Date: Thu, 19 May 2011 15:38:29 -0400 Subject: blk-cgroup: Make 64bit per cpu stats safe on 32bit arch Some of the stats are 64bit and updation will be non atomic on 32bit architecture. Use sequence counters on 32bit arch to make reading of stats safe. Signed-off-by: Vivek Goyal Signed-off-by: Jens Axboe --- block/blk-cgroup.c | 27 ++++++++++++++++++++++----- block/blk-cgroup.h | 2 ++ 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c index 34bfcefdd924..3622518e1c23 100644 --- a/block/blk-cgroup.c +++ b/block/blk-cgroup.c @@ -400,14 +400,25 @@ void blkiocg_update_dispatch_stats(struct blkio_group *blkg, uint64_t bytes, bool direction, bool sync) { struct blkio_group_stats_cpu *stats_cpu; + unsigned long flags; + + /* + * Disabling interrupts to provide mutual exclusion between two + * writes on same cpu. It probably is not needed for 64bit. Not + * optimizing that case yet. + */ + local_irq_save(flags); stats_cpu = this_cpu_ptr(blkg->stats_cpu); + u64_stats_update_begin(&stats_cpu->syncp); stats_cpu->sectors += bytes >> 9; blkio_add_stat(stats_cpu->stat_arr_cpu[BLKIO_STAT_CPU_SERVICED], 1, direction, sync); blkio_add_stat(stats_cpu->stat_arr_cpu[BLKIO_STAT_CPU_SERVICE_BYTES], bytes, direction, sync); + u64_stats_update_end(&stats_cpu->syncp); + local_irq_restore(flags); } EXPORT_SYMBOL_GPL(blkiocg_update_dispatch_stats); @@ -622,15 +633,21 @@ static uint64_t blkio_read_stat_cpu(struct blkio_group *blkg, { int cpu; struct blkio_group_stats_cpu *stats_cpu; - uint64_t val = 0; + u64 val = 0, tval; for_each_possible_cpu(cpu) { + unsigned int start; stats_cpu = per_cpu_ptr(blkg->stats_cpu, cpu); - if (type == BLKIO_STAT_CPU_SECTORS) - val += stats_cpu->sectors; - else - val += stats_cpu->stat_arr_cpu[type][sub_type]; + do { + start = u64_stats_fetch_begin(&stats_cpu->syncp); + if (type == BLKIO_STAT_CPU_SECTORS) + tval = stats_cpu->sectors; + else + tval = stats_cpu->stat_arr_cpu[type][sub_type]; + } while(u64_stats_fetch_retry(&stats_cpu->syncp, start)); + + val += tval; } return val; diff --git a/block/blk-cgroup.h b/block/blk-cgroup.h index fd730a24b491..262226798093 100644 --- a/block/blk-cgroup.h +++ b/block/blk-cgroup.h @@ -14,6 +14,7 @@ */ #include +#include enum blkio_policy_id { BLKIO_POLICY_PROP = 0, /* Proportional Bandwidth division */ @@ -154,6 +155,7 @@ struct blkio_group_stats { struct blkio_group_stats_cpu { uint64_t sectors; uint64_t stat_arr_cpu[BLKIO_STAT_CPU_NR][BLKIO_STAT_TOTAL]; + struct u64_stats_sync syncp; }; struct blkio_group { -- cgit v1.2.1 From f0bdc8cdd9a2bcc2c84ae2a1fdbff4188b354d8d Mon Sep 17 00:00:00 2001 From: Vivek Goyal Date: Thu, 19 May 2011 15:38:30 -0400 Subject: blk-cgroup: Make cgroup stat reset path blkg->lock free for dispatch stats Now dispatch stats update is lock free. But reset of these stats still takes blkg->stats_lock and is dependent on that. As stats are per cpu, we should be able to just reset the stats on each cpu without any locks. (Atleast for 64bit arch). On 32bit arch there is a small race where 64bit updates are not atomic. The result of this race can be that in the presence of other writers, one might not get 0 value after reset of a stat and might see something intermediate One can write more complicated code to cover this race like sending IPI to other cpus to reset stats and for offline cpus, reset these directly. Right not I am not taking that path because reset_update is more of a debug feature and it can happen only on 32bit arch and possibility of it happening is small. Will fix it if it becomes a real problem. For the time being going for code simplicity. Signed-off-by: Vivek Goyal Signed-off-by: Jens Axboe --- block/blk-cgroup.c | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c index 3622518e1c23..e41cc6f2ccc1 100644 --- a/block/blk-cgroup.c +++ b/block/blk-cgroup.c @@ -537,6 +537,30 @@ struct blkio_group *blkiocg_lookup_group(struct blkio_cgroup *blkcg, void *key) } EXPORT_SYMBOL_GPL(blkiocg_lookup_group); +static void blkio_reset_stats_cpu(struct blkio_group *blkg) +{ + struct blkio_group_stats_cpu *stats_cpu; + int i, j, k; + /* + * Note: On 64 bit arch this should not be an issue. This has the + * possibility of returning some inconsistent value on 32bit arch + * as 64bit update on 32bit is non atomic. Taking care of this + * corner case makes code very complicated, like sending IPIs to + * cpus, taking care of stats of offline cpus etc. + * + * reset stats is anyway more of a debug feature and this sounds a + * corner case. So I am not complicating the code yet until and + * unless this becomes a real issue. + */ + for_each_possible_cpu(i) { + stats_cpu = per_cpu_ptr(blkg->stats_cpu, i); + stats_cpu->sectors = 0; + for(j = 0; j < BLKIO_STAT_CPU_NR; j++) + for (k = 0; k < BLKIO_STAT_TOTAL; k++) + stats_cpu->stat_arr_cpu[j][k] = 0; + } +} + static int blkiocg_reset_stats(struct cgroup *cgroup, struct cftype *cftype, u64 val) { @@ -581,7 +605,11 @@ blkiocg_reset_stats(struct cgroup *cgroup, struct cftype *cftype, u64 val) } #endif spin_unlock(&blkg->stats_lock); + + /* Reset Per cpu stats which don't take blkg->stats_lock */ + blkio_reset_stats_cpu(blkg); } + spin_unlock_irq(&blkcg->lock); return 0; } -- cgit v1.2.1 From af75cd3c67845ebe31d2df9a780889a5ebecef11 Mon Sep 17 00:00:00 2001 From: Vivek Goyal Date: Thu, 19 May 2011 15:38:31 -0400 Subject: blk-throttle: Make no throttling rule group processing lockless Currently we take a queue lock on each bio to check if there are any throttling rules associated with the group and also update the stats. Now access the group under rcu and update the stats without taking the queue lock. Queue lock is taken only if there are throttling rules associated with the group. So the common case of root group when there are no rules, save unnecessary pounding of request queue lock. Signed-off-by: Vivek Goyal Signed-off-by: Jens Axboe --- block/blk-throttle.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 49 insertions(+), 4 deletions(-) diff --git a/block/blk-throttle.c b/block/blk-throttle.c index c29a5a8cc18c..a62be8d0dc1b 100644 --- a/block/blk-throttle.c +++ b/block/blk-throttle.c @@ -229,6 +229,22 @@ __throtl_tg_fill_dev_details(struct throtl_data *td, struct throtl_grp *tg) } } +/* + * Should be called with without queue lock held. Here queue lock will be + * taken rarely. It will be taken only once during life time of a group + * if need be + */ +static void +throtl_tg_fill_dev_details(struct throtl_data *td, struct throtl_grp *tg) +{ + if (!tg || tg->blkg.dev) + return; + + spin_lock_irq(td->queue->queue_lock); + __throtl_tg_fill_dev_details(td, tg); + spin_unlock_irq(td->queue->queue_lock); +} + static void throtl_init_add_tg_lists(struct throtl_data *td, struct throtl_grp *tg, struct blkio_cgroup *blkcg) { @@ -666,6 +682,12 @@ static bool tg_with_in_bps_limit(struct throtl_data *td, struct throtl_grp *tg, return 0; } +static bool tg_no_rule_group(struct throtl_grp *tg, bool rw) { + if (tg->bps[rw] == -1 && tg->iops[rw] == -1) + return 1; + return 0; +} + /* * Returns whether one can dispatch a bio or not. Also returns approx number * of jiffies to wait before this bio is with-in IO rate and can be dispatched @@ -730,10 +752,6 @@ static void throtl_charge_bio(struct throtl_grp *tg, struct bio *bio) tg->bytes_disp[rw] += bio->bi_size; tg->io_disp[rw]++; - /* - * TODO: This will take blkg->stats_lock. Figure out a way - * to avoid this cost. - */ blkiocg_update_dispatch_stats(&tg->blkg, bio->bi_size, rw, sync); } @@ -1111,12 +1129,39 @@ int blk_throtl_bio(struct request_queue *q, struct bio **biop) struct throtl_grp *tg; struct bio *bio = *biop; bool rw = bio_data_dir(bio), update_disptime = true; + struct blkio_cgroup *blkcg; if (bio->bi_rw & REQ_THROTTLED) { bio->bi_rw &= ~REQ_THROTTLED; return 0; } + /* + * A throtl_grp pointer retrieved under rcu can be used to access + * basic fields like stats and io rates. If a group has no rules, + * just update the dispatch stats in lockless manner and return. + */ + + rcu_read_lock(); + blkcg = task_blkio_cgroup(current); + tg = throtl_find_tg(td, blkcg); + if (tg) { + throtl_tg_fill_dev_details(td, tg); + + if (tg_no_rule_group(tg, rw)) { + blkiocg_update_dispatch_stats(&tg->blkg, bio->bi_size, + rw, bio->bi_rw & REQ_SYNC); + rcu_read_unlock(); + return 0; + } + } + rcu_read_unlock(); + + /* + * Either group has not been allocated yet or it is not an unlimited + * IO group + */ + spin_lock_irq(q->queue_lock); tg = throtl_get_tg(td); -- cgit v1.2.1 From 771949d03b4f5295f648f09141325fd478f6c7ce Mon Sep 17 00:00:00 2001 From: Jens Axboe Date: Fri, 20 May 2011 20:52:16 +0200 Subject: block: get rid of on-stack plugging debug checks We don't need them anymore, so kill: - REQ_ON_PLUG checks in various places - !rq_mergeable() check in plug merging Signed-off-by: Jens Axboe --- block/blk-core.c | 27 --------------------------- block/elevator.c | 4 ---- include/linux/blk_types.h | 2 -- 3 files changed, 33 deletions(-) diff --git a/block/blk-core.c b/block/blk-core.c index 9e8e297374b9..7369eeeafe23 100644 --- a/block/blk-core.c +++ b/block/blk-core.c @@ -569,8 +569,6 @@ int blk_get_queue(struct request_queue *q) static inline void blk_free_request(struct request_queue *q, struct request *rq) { - BUG_ON(rq->cmd_flags & REQ_ON_PLUG); - if (rq->cmd_flags & REQ_ELVPRIV) elv_put_request(q, rq); mempool_free(rq, q->rq.rq_pool); @@ -1110,14 +1108,6 @@ static bool bio_attempt_back_merge(struct request_queue *q, struct request *req, { const int ff = bio->bi_rw & REQ_FAILFAST_MASK; - /* - * Debug stuff, kill later - */ - if (!rq_mergeable(req)) { - blk_dump_rq_flags(req, "back"); - return false; - } - if (!ll_back_merge_fn(q, req, bio)) return false; @@ -1141,14 +1131,6 @@ static bool bio_attempt_front_merge(struct request_queue *q, const int ff = bio->bi_rw & REQ_FAILFAST_MASK; sector_t sector; - /* - * Debug stuff, kill later - */ - if (!rq_mergeable(req)) { - blk_dump_rq_flags(req, "front"); - return false; - } - if (!ll_front_merge_fn(q, req, bio)) return false; @@ -1258,14 +1240,12 @@ static int __make_request(struct request_queue *q, struct bio *bio) el_ret = elv_merge(q, &req, bio); if (el_ret == ELEVATOR_BACK_MERGE) { - BUG_ON(req->cmd_flags & REQ_ON_PLUG); if (bio_attempt_back_merge(q, req, bio)) { if (!attempt_back_merge(q, req)) elv_merged_request(q, req, el_ret); goto out_unlock; } } else if (el_ret == ELEVATOR_FRONT_MERGE) { - BUG_ON(req->cmd_flags & REQ_ON_PLUG); if (bio_attempt_front_merge(q, req, bio)) { if (!attempt_front_merge(q, req)) elv_merged_request(q, req, el_ret); @@ -1320,10 +1300,6 @@ get_rq: if (__rq->q != q) plug->should_sort = 1; } - /* - * Debug flag, kill later - */ - req->cmd_flags |= REQ_ON_PLUG; list_add_tail(&req->queuelist, &plug->list); drive_stat_acct(req, 1); } else { @@ -2749,7 +2725,6 @@ void blk_flush_plug_list(struct blk_plug *plug, bool from_schedule) while (!list_empty(&list)) { rq = list_entry_rq(list.next); list_del_init(&rq->queuelist); - BUG_ON(!(rq->cmd_flags & REQ_ON_PLUG)); BUG_ON(!rq->q); if (rq->q != q) { /* @@ -2761,8 +2736,6 @@ void blk_flush_plug_list(struct blk_plug *plug, bool from_schedule) depth = 0; spin_lock(q->queue_lock); } - rq->cmd_flags &= ~REQ_ON_PLUG; - /* * rq is already accounted, so use raw insert */ diff --git a/block/elevator.c b/block/elevator.c index 2a0b653c90fd..b0b38ce0dcb6 100644 --- a/block/elevator.c +++ b/block/elevator.c @@ -416,8 +416,6 @@ void elv_dispatch_sort(struct request_queue *q, struct request *rq) struct list_head *entry; int stop_flags; - BUG_ON(rq->cmd_flags & REQ_ON_PLUG); - if (q->last_merge == rq) q->last_merge = NULL; @@ -656,8 +654,6 @@ void __elv_add_request(struct request_queue *q, struct request *rq, int where) rq->q = q; - BUG_ON(rq->cmd_flags & REQ_ON_PLUG); - if (rq->cmd_flags & REQ_SOFTBARRIER) { /* barriers are scheduling boundary, update end_sector */ if (rq->cmd_type == REQ_TYPE_FS || diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h index be50d9e70a7d..2a7cea53ca0d 100644 --- a/include/linux/blk_types.h +++ b/include/linux/blk_types.h @@ -151,7 +151,6 @@ enum rq_flag_bits { __REQ_IO_STAT, /* account I/O stat */ __REQ_MIXED_MERGE, /* merge of different types, fail separately */ __REQ_SECURE, /* secure discard (used with __REQ_DISCARD) */ - __REQ_ON_PLUG, /* on plug list */ __REQ_NR_BITS, /* stops here */ }; @@ -192,6 +191,5 @@ enum rq_flag_bits { #define REQ_IO_STAT (1 << __REQ_IO_STAT) #define REQ_MIXED_MERGE (1 << __REQ_MIXED_MERGE) #define REQ_SECURE (1 << __REQ_SECURE) -#define REQ_ON_PLUG (1 << __REQ_ON_PLUG) #endif /* __LINUX_BLK_TYPES_H */ -- cgit v1.2.1 From 345227d705f2318e9bc088e79fe71a38bb5fe82b Mon Sep 17 00:00:00 2001 From: "Gustavo F. Padovan" Date: Fri, 20 May 2011 21:23:37 +0200 Subject: backing-dev: Kill set but not used var in bdi_debug_stats_show() Signed-off-by: Gustavo F. Padovan Signed-off-by: Jens Axboe --- mm/backing-dev.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mm/backing-dev.c b/mm/backing-dev.c index befc87531e4f..f032e6e1e09a 100644 --- a/mm/backing-dev.c +++ b/mm/backing-dev.c @@ -63,10 +63,10 @@ static int bdi_debug_stats_show(struct seq_file *m, void *v) unsigned long background_thresh; unsigned long dirty_thresh; unsigned long bdi_thresh; - unsigned long nr_dirty, nr_io, nr_more_io, nr_wb; + unsigned long nr_dirty, nr_io, nr_more_io; struct inode *inode; - nr_wb = nr_dirty = nr_io = nr_more_io = 0; + nr_dirty = nr_io = nr_more_io = 0; spin_lock(&inode_wb_list_lock); list_for_each_entry(inode, &wb->b_dirty, i_wb_list) nr_dirty++; -- cgit v1.2.1 From 2abae55f5a41d29f21b5b259c33d218c950d5c90 Mon Sep 17 00:00:00 2001 From: Vivek Goyal Date: Mon, 23 May 2011 10:02:19 +0200 Subject: cfq-iosched: Fix a memory leak of per cpu stats for root group We allocated per cpu stats struct for root group but did not free it. Fix it. Signed-off-by: Vivek Goyal Signed-off-by: Jens Axboe --- block/cfq-iosched.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c index d646b279c8bb..0316f6eece03 100644 --- a/block/cfq-iosched.c +++ b/block/cfq-iosched.c @@ -3940,6 +3940,11 @@ static void cfq_exit_queue(struct elevator_queue *e) */ if (wait) synchronize_rcu(); + +#ifdef CONFIG_CFQ_GROUP_IOSCHED + /* Free up per cpu stats for root group */ + free_percpu(cfqd->root_group.blkg.stats_cpu); +#endif kfree(cfqd); } -- cgit v1.2.1 From 317389a7739675aa990b7e0d750a7c435f1d25d7 Mon Sep 17 00:00:00 2001 From: Vivek Goyal Date: Mon, 23 May 2011 10:02:19 +0200 Subject: cfq-iosched: Make IO merge related stats per cpu Make BLKIO_STAT_MERGED per cpu hence gettring rid of need of taking blkg->stats_lock. Signed-off-by: Vivek Goyal Signed-off-by: Jens Axboe --- block/blk-cgroup.c | 22 +++++++++++++++++----- block/blk-cgroup.h | 4 ++-- 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c index e41cc6f2ccc1..07371cfdfae6 100644 --- a/block/blk-cgroup.c +++ b/block/blk-cgroup.c @@ -441,15 +441,27 @@ void blkiocg_update_completion_stats(struct blkio_group *blkg, } EXPORT_SYMBOL_GPL(blkiocg_update_completion_stats); +/* Merged stats are per cpu. */ void blkiocg_update_io_merged_stats(struct blkio_group *blkg, bool direction, bool sync) { + struct blkio_group_stats_cpu *stats_cpu; unsigned long flags; - spin_lock_irqsave(&blkg->stats_lock, flags); - blkio_add_stat(blkg->stats.stat_arr[BLKIO_STAT_MERGED], 1, direction, - sync); - spin_unlock_irqrestore(&blkg->stats_lock, flags); + /* + * Disabling interrupts to provide mutual exclusion between two + * writes on same cpu. It probably is not needed for 64bit. Not + * optimizing that case yet. + */ + local_irq_save(flags); + + stats_cpu = this_cpu_ptr(blkg->stats_cpu); + + u64_stats_update_begin(&stats_cpu->syncp); + blkio_add_stat(stats_cpu->stat_arr_cpu[BLKIO_STAT_CPU_MERGED], 1, + direction, sync); + u64_stats_update_end(&stats_cpu->syncp); + local_irq_restore(flags); } EXPORT_SYMBOL_GPL(blkiocg_update_io_merged_stats); @@ -1244,7 +1256,7 @@ static int blkiocg_file_read_map(struct cgroup *cgrp, struct cftype *cft, BLKIO_STAT_WAIT_TIME, 1, 0); case BLKIO_PROP_io_merged: return blkio_read_blkg_stats(blkcg, cft, cb, - BLKIO_STAT_MERGED, 1, 0); + BLKIO_STAT_CPU_MERGED, 1, 1); case BLKIO_PROP_io_queued: return blkio_read_blkg_stats(blkcg, cft, cb, BLKIO_STAT_QUEUED, 1, 0); diff --git a/block/blk-cgroup.h b/block/blk-cgroup.h index 262226798093..a71d2904ffb9 100644 --- a/block/blk-cgroup.h +++ b/block/blk-cgroup.h @@ -39,8 +39,6 @@ enum stat_type { BLKIO_STAT_SERVICE_TIME = 0, /* Total time spent waiting in scheduler queue in ns */ BLKIO_STAT_WAIT_TIME, - /* Number of IOs merged */ - BLKIO_STAT_MERGED, /* Number of IOs queued up */ BLKIO_STAT_QUEUED, /* All the single valued stats go below this */ @@ -63,6 +61,8 @@ enum stat_type_cpu { BLKIO_STAT_CPU_SERVICE_BYTES, /* Total IOs serviced, post merge */ BLKIO_STAT_CPU_SERVICED, + /* Number of IOs merged */ + BLKIO_STAT_CPU_MERGED, BLKIO_STAT_CPU_NR }; -- cgit v1.2.1 From 95cf3dd9dbe6883a0328724e2110e3fc6465630b Mon Sep 17 00:00:00 2001 From: Vivek Goyal Date: Mon, 23 May 2011 10:02:19 +0200 Subject: block: call elv_bio_merged() when merged Commit 73c101011926 ("block: initial patch for on-stack per-task plugging") removed calls to elv_bio_merged() when @bio merged with @req. Re-add them. This in turn will update merged stats in associated group. That should be safe as long as request has got reference to the blkio_group. Signed-off-by: Namhyung Kim Cc: Divyesh Shah Signed-off-by: Jens Axboe --- block/blk-core.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/block/blk-core.c b/block/blk-core.c index 7369eeeafe23..c8303e9d919d 100644 --- a/block/blk-core.c +++ b/block/blk-core.c @@ -1122,6 +1122,7 @@ static bool bio_attempt_back_merge(struct request_queue *q, struct request *req, req->ioprio = ioprio_best(req->ioprio, bio_prio(bio)); drive_stat_acct(req, 0); + elv_bio_merged(q, req, bio); return true; } @@ -1155,6 +1156,7 @@ static bool bio_attempt_front_merge(struct request_queue *q, req->ioprio = ioprio_best(req->ioprio, bio_prio(bio)); drive_stat_acct(req, 0); + elv_bio_merged(q, req, bio); return true; } -- cgit v1.2.1 From 7e69723fef8771a9d57bd27d36281d756130b4b5 Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Mon, 23 May 2011 13:26:07 +0200 Subject: block: move bd_set_size() above rescan_partitions() in __blkdev_get() 02e352287a4 (block: rescan partitions on invalidated devices on -ENOMEDIA too) relocated partition rescan above explicit bd_set_size() to simplify condition check. As rescan_partitions() does its own bdev size setting, this doesn't break anything; however, rescan_partitions() prints out the following messages when adjusting bdev size, which can be confusing. sda: detected capacity change from 0 to 146815737856 sdb: detected capacity change from 0 to 146815737856 This patch restores the original order and remove the warning messages. stable: Please apply together with 02e352287a4 (block: rescan partitions on invalidated devices on -ENOMEDIA too). Signed-off-by: Tejun Heo Reported-by: Tony Luck Tested-by: Tony Luck Cc: stable@kernel.org Stable note: 2.6.39 only. Signed-off-by: Jens Axboe --- fs/block_dev.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/fs/block_dev.c b/fs/block_dev.c index d7c2e0fddc6f..1f2b19978333 100644 --- a/fs/block_dev.c +++ b/fs/block_dev.c @@ -1120,6 +1120,15 @@ static int __blkdev_get(struct block_device *bdev, fmode_t mode, int for_part) goto restart; } } + + if (!ret && !bdev->bd_openers) { + bd_set_size(bdev,(loff_t)get_capacity(disk)<<9); + bdi = blk_get_backing_dev_info(bdev); + if (bdi == NULL) + bdi = &default_backing_dev_info; + bdev_inode_switch_bdi(bdev->bd_inode, bdi); + } + /* * If the device is invalidated, rescan partition * if open succeeded or failed with -ENOMEDIUM. @@ -1130,14 +1139,6 @@ static int __blkdev_get(struct block_device *bdev, fmode_t mode, int for_part) rescan_partitions(disk, bdev); if (ret) goto out_clear; - - if (!bdev->bd_openers) { - bd_set_size(bdev,(loff_t)get_capacity(disk)<<9); - bdi = blk_get_backing_dev_info(bdev); - if (bdi == NULL) - bdi = &default_backing_dev_info; - bdev_inode_switch_bdi(bdev->bd_inode, bdi); - } } else { struct block_device *whole; whole = bdget_disk(disk, 0); -- cgit v1.2.1 From 4cbadbd16e2fb727f6926597e0a580829e6222f1 Mon Sep 17 00:00:00 2001 From: Vivek Goyal Date: Mon, 23 May 2011 19:35:04 +0200 Subject: blk-cgroup: Initialize ioc->cgroup_changed at ioc creation time If we don't explicitly initialize it to zero, CFQ might think that cgroup of ioc has changed and it generates lots of unnecessary calls to call_for_each_cic(changed_cgroup). Fix it. cfq_get_io_context() cfq_ioc_set_cgroup() call_for_each_cic(ioc, changed_cgroup) Signed-off-by: Vivek Goyal Signed-off-by: Jens Axboe --- block/blk-ioc.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/block/blk-ioc.c b/block/blk-ioc.c index b791022beef3..c898049dafd5 100644 --- a/block/blk-ioc.c +++ b/block/blk-ioc.c @@ -96,6 +96,9 @@ struct io_context *alloc_io_context(gfp_t gfp_flags, int node) INIT_RADIX_TREE(&ret->radix_root, GFP_ATOMIC | __GFP_HIGH); INIT_HLIST_HEAD(&ret->cic_list); ret->ioc_data = NULL; +#if defined(CONFIG_BLK_CGROUP) || defined(CONFIG_BLK_CGROUP_MODULE) + ret->cgroup_changed = 0; +#endif } return ret; -- cgit v1.2.1 From b9f8ce059940064a732da49b5d6e618381dad956 Mon Sep 17 00:00:00 2001 From: Namhyung Kim Date: Tue, 24 May 2011 10:23:21 +0200 Subject: cfq-iosched: algebraic simplification in cfq_prio_to_maxrq() Simplify the calculation in cfq_prio_to_maxrq(), plus replace CFQ_PRIO_LISTS to IOPRIO_BE_NR since they are the same and IOPRIO_BE_NR looks more reasonable in this context IMHO. Signed-off-by: Namhyung Kim Signed-off-by: Jens Axboe --- block/cfq-iosched.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c index 0316f6eece03..00763e510228 100644 --- a/block/cfq-iosched.c +++ b/block/cfq-iosched.c @@ -2102,7 +2102,7 @@ cfq_prio_to_maxrq(struct cfq_data *cfqd, struct cfq_queue *cfqq) WARN_ON(cfqq->ioprio >= IOPRIO_BE_NR); - return 2 * (base_rq + base_rq * (CFQ_PRIO_LISTS - 1 - cfqq->ioprio)); + return 2 * base_rq * (IOPRIO_BE_NR - cfqq->ioprio); } /* -- cgit v1.2.1 From 229836bd63c5413a4f8eed93d610b432be141e9b Mon Sep 17 00:00:00 2001 From: Namhyung Kim Date: Tue, 24 May 2011 10:23:21 +0200 Subject: cfq-iosched: reduce bit operations in cfq_choose_req() Reduce the number of bit operations in cfq_choose_req() on average (and worst) cases. Signed-off-by: Namhyung Kim Signed-off-by: Jens Axboe --- block/cfq-iosched.c | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c index 00763e510228..7ffceddee420 100644 --- a/block/cfq-iosched.c +++ b/block/cfq-iosched.c @@ -667,15 +667,11 @@ cfq_choose_req(struct cfq_data *cfqd, struct request *rq1, struct request *rq2, if (rq2 == NULL) return rq1; - if (rq_is_sync(rq1) && !rq_is_sync(rq2)) - return rq1; - else if (rq_is_sync(rq2) && !rq_is_sync(rq1)) - return rq2; - if ((rq1->cmd_flags & REQ_META) && !(rq2->cmd_flags & REQ_META)) - return rq1; - else if ((rq2->cmd_flags & REQ_META) && - !(rq1->cmd_flags & REQ_META)) - return rq2; + if (rq_is_sync(rq1) != rq_is_sync(rq2)) + return rq_is_sync(rq1) ? rq1 : rq2; + + if ((rq1->cmd_flags ^ rq2->cmd_flags) & REQ_META) + return rq1->cmd_flags & REQ_META ? rq1 : rq2; s1 = blk_rq_pos(rq1); s2 = blk_rq_pos(rq2); -- cgit v1.2.1 From 20359f27e8ff115f7cddf3da5b3a6cdcca2e650d Mon Sep 17 00:00:00 2001 From: Namhyung Kim Date: Tue, 24 May 2011 10:23:22 +0200 Subject: cfq-iosched: remove unused 'group_changed' in cfq_service_tree_add() The 'group_changed' variable is initialized to 0 and never changed, so checking the variable is meaningless. It is a leftover from 0bbfeb832042 ("cfq-iosched: Always provide group iosolation."). Let's get rid of it. Signed-off-by: Namhyung Kim Cc: Justin TerAvest Signed-off-by: Jens Axboe --- block/cfq-iosched.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c index 7ffceddee420..6dd2179cf1a4 100644 --- a/block/cfq-iosched.c +++ b/block/cfq-iosched.c @@ -1279,7 +1279,6 @@ static void cfq_service_tree_add(struct cfq_data *cfqd, struct cfq_queue *cfqq, struct cfq_rb_root *service_tree; int left; int new_cfqq = 1; - int group_changed = 0; service_tree = service_tree_for(cfqq->cfqg, cfqq_prio(cfqq), cfqq_type(cfqq)); @@ -1350,7 +1349,7 @@ static void cfq_service_tree_add(struct cfq_data *cfqd, struct cfq_queue *cfqq, rb_link_node(&cfqq->rb_node, parent, p); rb_insert_color(&cfqq->rb_node, &service_tree->rb); service_tree->count++; - if ((add_front || !new_cfqq) && !group_changed) + if (add_front || !new_cfqq) return; cfq_group_notify_queue_add(cfqd, cfqq->cfqg); } -- cgit v1.2.1 From 1547010e6e15a3f44f49381246421a1e19de526e Mon Sep 17 00:00:00 2001 From: Namhyung Kim Date: Tue, 24 May 2011 10:23:22 +0200 Subject: cfq-iosched: free cic_index if cfqd allocation fails When struct cfq_data allocation fails, cic_index need to be freed. Signed-off-by: Namhyung Kim Signed-off-by: Jens Axboe --- block/cfq-iosched.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c index 6dd2179cf1a4..7c52d6888924 100644 --- a/block/cfq-iosched.c +++ b/block/cfq-iosched.c @@ -3973,8 +3973,12 @@ static void *cfq_init_queue(struct request_queue *q) return NULL; cfqd = kmalloc_node(sizeof(*cfqd), GFP_KERNEL | __GFP_ZERO, q->node); - if (!cfqd) + if (!cfqd) { + spin_lock(&cic_index_lock); + ida_remove(&cic_index_ida, i); + spin_unlock(&cic_index_lock); return NULL; + } /* * Don't need take queue_lock in the routine, since we are -- cgit v1.2.1