From 8677142710516d986d932d6f1fba7be8382c1fec Mon Sep 17 00:00:00 2001 From: FUJITA Tomonori Date: Mon, 13 Oct 2008 14:19:05 +0200 Subject: block: fix nr_phys_segments miscalculation bug This fixes the bug reported by Nikanth Karthikesan : http://lkml.org/lkml/2008/10/2/203 The root cause of the bug is that blk_phys_contig_segment miscalculates q->max_segment_size. blk_phys_contig_segment checks: req->biotail->bi_size + next_req->bio->bi_size > q->max_segment_size But blk_recalc_rq_segments might expect that req->biotail and the previous bio in the req are supposed be merged into one segment. blk_recalc_rq_segments might also expect that next_req->bio and the next bio in the next_req are supposed be merged into one segment. In such case, we merge two requests that can't be merged here. Later, blk_rq_map_sg gives more segments than it should. We need to keep track of segment size in blk_recalc_rq_segments and use it to see if two requests can be merged. This patch implements it in the similar way that we used to do for hw merging (virtual merging). Signed-off-by: FUJITA Tomonori Signed-off-by: Jens Axboe --- block/blk-merge.c | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) (limited to 'block') diff --git a/block/blk-merge.c b/block/blk-merge.c index 908d3e11ac52..8681cd6f9911 100644 --- a/block/blk-merge.c +++ b/block/blk-merge.c @@ -77,12 +77,20 @@ void blk_recalc_rq_segments(struct request *rq) continue; } new_segment: + if (nr_phys_segs == 1 && seg_size > rq->bio->bi_seg_front_size) + rq->bio->bi_seg_front_size = seg_size; + nr_phys_segs++; bvprv = bv; seg_size = bv->bv_len; highprv = high; } + if (nr_phys_segs == 1 && seg_size > rq->bio->bi_seg_front_size) + rq->bio->bi_seg_front_size = seg_size; + if (seg_size > rq->biotail->bi_seg_back_size) + rq->biotail->bi_seg_back_size = seg_size; + rq->nr_phys_segments = nr_phys_segs; } @@ -106,7 +114,8 @@ static int blk_phys_contig_segment(struct request_queue *q, struct bio *bio, if (!test_bit(QUEUE_FLAG_CLUSTER, &q->queue_flags)) return 0; - if (bio->bi_size + nxt->bi_size > q->max_segment_size) + if (bio->bi_seg_back_size + nxt->bi_seg_front_size > + q->max_segment_size) return 0; if (!bio_has_data(bio)) @@ -309,6 +318,8 @@ static int ll_merge_requests_fn(struct request_queue *q, struct request *req, struct request *next) { int total_phys_segments; + unsigned int seg_size = + req->biotail->bi_seg_back_size + next->bio->bi_seg_front_size; /* * First check if the either of the requests are re-queued @@ -324,8 +335,13 @@ static int ll_merge_requests_fn(struct request_queue *q, struct request *req, return 0; total_phys_segments = req->nr_phys_segments + next->nr_phys_segments; - if (blk_phys_contig_segment(q, req->biotail, next->bio)) + if (blk_phys_contig_segment(q, req->biotail, next->bio)) { + if (req->nr_phys_segments == 1) + req->bio->bi_seg_front_size = seg_size; + if (next->nr_phys_segments == 1) + next->biotail->bi_seg_back_size = seg_size; total_phys_segments--; + } if (total_phys_segments > q->max_phys_segments) return 0; -- cgit v1.2.3 From e6d63840ba55ffd3a79aea6792aac6f29f338083 Mon Sep 17 00:00:00 2001 From: Li Zefan Date: Tue, 14 Oct 2008 08:49:34 +0200 Subject: block: fix kernel-doc for blk_alloc_devt() No argument 'gfp_mask' for blk_alloc_devt(). Signed-off-by: Li Zefan Signed-off-by: Jens Axboe --- block/genhd.c | 1 - 1 file changed, 1 deletion(-) (limited to 'block') diff --git a/block/genhd.c b/block/genhd.c index 4cd3433c99ac..b8defae2ec03 100644 --- a/block/genhd.c +++ b/block/genhd.c @@ -358,7 +358,6 @@ static int blk_mangle_minor(int minor) /** * blk_alloc_devt - allocate a dev_t for a partition * @part: partition to allocate dev_t for - * @gfp_mask: memory allocation flag * @devt: out parameter for resulting dev_t * * Allocate a dev_t for block device. -- cgit v1.2.3 From ee2e992cc28553f6c4dd1ab5483c8733c393626b Mon Sep 17 00:00:00 2001 From: Li Zefan Date: Tue, 14 Oct 2008 08:49:56 +0200 Subject: block: simplify string handling in elv_iosched_store() strlcpy() guarantees the dest buffer is NULL teminated. Signed-off-by: Li Zefan Signed-off-by: Jens Axboe --- block/elevator.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) (limited to 'block') diff --git a/block/elevator.c b/block/elevator.c index 04518921db31..9482ffa1aae6 100644 --- a/block/elevator.c +++ b/block/elevator.c @@ -1166,15 +1166,10 @@ ssize_t elv_iosched_store(struct request_queue *q, const char *name, size_t count) { char elevator_name[ELV_NAME_MAX]; - size_t len; struct elevator_type *e; - elevator_name[sizeof(elevator_name) - 1] = '\0'; - strncpy(elevator_name, name, sizeof(elevator_name) - 1); - len = strlen(elevator_name); - - if (len && elevator_name[len - 1] == '\n') - elevator_name[len - 1] = '\0'; + strlcpy(elevator_name, name, sizeof(elevator_name)); + strstrip(elevator_name); e = elevator_get(elevator_name); if (!e) { -- cgit v1.2.3 From 80a4b58e36b63d7b0b592beb1bd6410aadeeb63c Mon Sep 17 00:00:00 2001 From: Jens Axboe Date: Tue, 14 Oct 2008 09:51:06 +0200 Subject: block: only call ->request_fn when the queue is not stopped Callers should use either blk_run_queue/__blk_run_queue, or blk_start_queueing() to invoke request handling instead of calling ->request_fn() directly as that does not take the queue stopped flag into account. Also add appropriate comments on the above functions to detail their usage. Signed-off-by: Jens Axboe --- block/blk-core.c | 19 +++++++++++++++++-- block/elevator.c | 7 +++---- 2 files changed, 20 insertions(+), 6 deletions(-) (limited to 'block') diff --git a/block/blk-core.c b/block/blk-core.c index 2d053b584410..91532f2d2fa7 100644 --- a/block/blk-core.c +++ b/block/blk-core.c @@ -325,6 +325,9 @@ EXPORT_SYMBOL(blk_unplug); static void blk_invoke_request_fn(struct request_queue *q) { + if (unlikely(blk_queue_stopped(q))) + return; + /* * one level of recursion is ok and is much faster than kicking * the unplug handling @@ -399,8 +402,13 @@ void blk_sync_queue(struct request_queue *q) EXPORT_SYMBOL(blk_sync_queue); /** - * blk_run_queue - run a single device queue + * __blk_run_queue - run a single device queue * @q: The queue to run + * + * Description: + * See @blk_run_queue. This variant must be called with the queue lock + * held and interrupts disabled. + * */ void __blk_run_queue(struct request_queue *q) { @@ -418,6 +426,12 @@ EXPORT_SYMBOL(__blk_run_queue); /** * blk_run_queue - run a single device queue * @q: The queue to run + * + * Description: + * Invoke request handling on this queue, if it has pending work to do. + * May be used to restart queueing when a request has completed. Also + * See @blk_start_queueing. + * */ void blk_run_queue(struct request_queue *q) { @@ -884,7 +898,8 @@ EXPORT_SYMBOL(blk_get_request); * * This is basically a helper to remove the need to know whether a queue * is plugged or not if someone just wants to initiate dispatch of requests - * for this queue. + * for this queue. Should be used to start queueing on a device outside + * of ->request_fn() context. Also see @blk_run_queue. * * The queue lock must be held with interrupts disabled. */ diff --git a/block/elevator.c b/block/elevator.c index 9482ffa1aae6..59173a69ebdf 100644 --- a/block/elevator.c +++ b/block/elevator.c @@ -612,7 +612,7 @@ void elv_insert(struct request_queue *q, struct request *rq, int where) * processing. */ blk_remove_plug(q); - q->request_fn(q); + blk_start_queueing(q); break; case ELEVATOR_INSERT_SORT: @@ -950,7 +950,7 @@ void elv_completed_request(struct request_queue *q, struct request *rq) blk_ordered_cur_seq(q) == QUEUE_ORDSEQ_DRAIN && blk_ordered_req_seq(first_rq) > QUEUE_ORDSEQ_DRAIN) { blk_ordered_complete_seq(q, QUEUE_ORDSEQ_DRAIN, 0); - q->request_fn(q); + blk_start_queueing(q); } } } @@ -1109,8 +1109,7 @@ static int elevator_switch(struct request_queue *q, struct elevator_type *new_e) elv_drain_elevator(q); while (q->rq.elvpriv) { - blk_remove_plug(q); - q->request_fn(q); + blk_start_queueing(q); spin_unlock_irq(q->queue_lock); msleep(10); spin_lock_irq(q->queue_lock); -- cgit v1.2.3 From 496aa8a98f5ab22ced46be5dc2087cdf3d029bd7 Mon Sep 17 00:00:00 2001 From: Randy Dunlap Date: Thu, 16 Oct 2008 07:46:23 +0200 Subject: block: fix current kernel-doc warnings Fix block kernel-doc warnings: Warning(linux-2.6.27-git4//fs/block_dev.c:1272): No description found for parameter 'path' Warning(linux-2.6.27-git4//block/blk-core.c:1021): No description found for parameter 'cpu' Warning(linux-2.6.27-git4//block/blk-core.c:1021): No description found for parameter 'part' Warning(/var/linsrc/linux-2.6.27-git4//block/genhd.c:544): No description found for parameter 'partno' Signed-off-by: Randy Dunlap Signed-off-by: Jens Axboe --- block/blk-core.c | 5 +++-- block/genhd.c | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) (limited to 'block') diff --git a/block/blk-core.c b/block/blk-core.c index 91532f2d2fa7..8517264eb71e 100644 --- a/block/blk-core.c +++ b/block/blk-core.c @@ -1018,8 +1018,9 @@ static void part_round_stats_single(int cpu, struct hd_struct *part, } /** - * part_round_stats() - Round off the performance stats on a struct - * disk_stats. + * part_round_stats() - Round off the performance stats on a struct disk_stats. + * @cpu: cpu number for stats access + * @part: target partition * * The average IO queue length and utilisation statistics are maintained * by observing the current state of the queue length and the amount of diff --git a/block/genhd.c b/block/genhd.c index b8defae2ec03..646e1d2507c7 100644 --- a/block/genhd.c +++ b/block/genhd.c @@ -534,7 +534,7 @@ void unlink_gendisk(struct gendisk *disk) /** * get_gendisk - get partitioning information for a given device * @devt: device to get partitioning information for - * @part: returned partition index + * @partno: returned partition index * * This function gets the structure containing partitioning * information for the given device @devt. -- cgit v1.2.3 From 713ada9ba94f2ad874cffd074b83e3dc681ca82f Mon Sep 17 00:00:00 2001 From: Peter Zijlstra Date: Thu, 16 Oct 2008 13:44:57 +0200 Subject: block: move q->unplug_work initialization modprobe loop; rmmod loop effectively creates a blk_queue and destroys it which results in q->unplug_work being canceled without it ever being initialized. Therefore, move the initialization of q->unplug_work from blk_queue_make_request() to blk_alloc_queue*(). Reported-by: Alexey Dobriyan Signed-off-by: Peter Zijlstra Signed-off-by: Jens Axboe --- block/blk-core.c | 1 + block/blk-settings.c | 2 -- 2 files changed, 1 insertion(+), 2 deletions(-) (limited to 'block') diff --git a/block/blk-core.c b/block/blk-core.c index 8517264eb71e..fcbd56dd41fa 100644 --- a/block/blk-core.c +++ b/block/blk-core.c @@ -515,6 +515,7 @@ struct request_queue *blk_alloc_queue_node(gfp_t gfp_mask, int node_id) init_timer(&q->unplug_timer); setup_timer(&q->timeout, blk_rq_timed_out_timer, (unsigned long) q); INIT_LIST_HEAD(&q->timeout_list); + INIT_WORK(&q->unplug_work, blk_unplug_work); kobject_init(&q->kobj, &blk_queue_ktype); diff --git a/block/blk-settings.c b/block/blk-settings.c index b21dcdb64151..41392fbe19ff 100644 --- a/block/blk-settings.c +++ b/block/blk-settings.c @@ -141,8 +141,6 @@ void blk_queue_make_request(struct request_queue *q, make_request_fn *mfn) if (q->unplug_delay == 0) q->unplug_delay = 1; - INIT_WORK(&q->unplug_work, blk_unplug_work); - q->unplug_timer.function = blk_unplug_timeout; q->unplug_timer.data = (unsigned long)q; -- cgit v1.2.3 From f73e2d13a16cc88c4faa4729967f92bfeec8a142 Mon Sep 17 00:00:00 2001 From: Jens Axboe Date: Fri, 17 Oct 2008 14:03:08 +0200 Subject: block: remove __generic_unplug_device() from exports The only out-of-core user is IDE, and that should be using blk_start_queueing() instead. Signed-off-by: Jens Axboe --- block/blk-core.c | 1 - block/blk.h | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) (limited to 'block') diff --git a/block/blk-core.c b/block/blk-core.c index fcbd56dd41fa..81a496004976 100644 --- a/block/blk-core.c +++ b/block/blk-core.c @@ -257,7 +257,6 @@ void __generic_unplug_device(struct request_queue *q) q->request_fn(q); } -EXPORT_SYMBOL(__generic_unplug_device); /** * generic_unplug_device - fire a request queue diff --git a/block/blk.h b/block/blk.h index e5c579769963..d2e49af90db5 100644 --- a/block/blk.h +++ b/block/blk.h @@ -20,6 +20,7 @@ void blk_unplug_timeout(unsigned long data); void blk_rq_timed_out_timer(unsigned long data); void blk_delete_timer(struct request *); void blk_add_timer(struct request *); +void __generic_unplug_device(struct request_queue *); /* * Internal atomic flags for request handling -- cgit v1.2.3