block: fix accounting bug on cross partition merges

/proc/diskstats would display a strange output as follows.

$ cat /proc/diskstats |grep sda
   8       0 sda 90524 7579 102154 20464 0 0 0 0 0 14096 20089
   8       1 sda1 19085 1352 21841 4209 0 0 0 0 4294967064 15689 4293424691
                                                ~~~~~~~~~~
   8       2 sda2 71252 3624 74891 15950 0 0 0 0 232 23995 1562390
   8       3 sda3 54 487 2188 92 0 0 0 0 0 88 92
   8       4 sda4 4 0 8 0 0 0 0 0 0 0 0
   8       5 sda5 81 2027 2130 138 0 0 0 0 0 87 137

Its reason is the wrong way of accounting hd_struct->in_flight. When a bio is
merged into a request belongs to different partition by ELEVATOR_FRONT_MERGE.

The detailed root cause is as follows.

Assuming that there are two partition, sda1 and sda2.

1. A request for sda2 is in request_queue. Hence sda1's hd_struct->in_flight
   is 0 and sda2's one is 1.

        | hd_struct->in_flight
   ---------------------------
   sda1 |          0
   sda2 |          1
   ---------------------------

2. A bio belongs to sda1 is issued and is merged into the request mentioned on
   step1 by ELEVATOR_BACK_MERGE. The first sector of the request is changed
   from sda2 region to sda1 region. However the two partition's
   hd_struct->in_flight are not changed.

        | hd_struct->in_flight
   ---------------------------
   sda1 |          0
   sda2 |          1
   ---------------------------

3. The request is finished and blk_account_io_done() is called. In this case,
   sda2's hd_struct->in_flight, not a sda1's one, is decremented.

        | hd_struct->in_flight
   ---------------------------
   sda1 |         -1
   sda2 |          1
   ---------------------------

The patch fixes the problem by caching the partition lookup
inside the request structure, hence making sure that the increment
and decrement will always happen on the same partition struct. This
also speeds up IO with accounting enabled, since it cuts down on
the number of lookups we have to do.

When reloading partition tables, quiesce IO to ensure that no
request references to the partition struct exists. When it is safe
to free the partition table, the IO for that device is restarted
again.

Signed-off-by: Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com>
Cc: stable@kernel.org
Signed-off-by: Jens Axboe <jaxboe@fusionio.com>

diff --git a/block/blk-core.c b/block/blk-core.c
index 797d5095eb83fa5cada4be1712858fe6dc391145..ddc68332d65568a0438b36007e4ff868ca55cf84 100644 (file)
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -64,13 +64,15 @@ static void drive_stat_acct(struct request *rq, int new_io)
                return;
 
        cpu = part_stat_lock();
-       part = disk_map_sector_rcu(rq->rq_disk, blk_rq_pos(rq));
 
-       if (!new_io)
+       if (!new_io) {
+               part = rq->part;
                part_stat_inc(cpu, part, merges[rw]);
-       else {
+       } else {
+               part = disk_map_sector_rcu(rq->rq_disk, blk_rq_pos(rq));
                part_round_stats(cpu, part);
                part_inc_in_flight(part, rw);
+               rq->part = part;
        }
 
        part_stat_unlock();
@@ -128,6 +130,7 @@ void blk_rq_init(struct request_queue *q, struct request *rq)
        rq->ref_count = 1;
        rq->start_time = jiffies;
        set_start_time_ns(rq);
+       rq->part = NULL;
 }
 EXPORT_SYMBOL(blk_rq_init);
 
@@ -804,11 +807,16 @@ static struct request *get_request(struct request_queue *q, int rw_flags,
        rl->starved[is_sync] = 0;
 
        priv = !test_bit(QUEUE_FLAG_ELVSWITCH, &q->queue_flags);
-       if (priv)
+       if (priv) {
                rl->elvpriv++;
 
-       if (blk_queue_io_stat(q))
-               rw_flags |= REQ_IO_STAT;
+               /*
+                * Don't do stats for non-priv requests
+                */
+               if (blk_queue_io_stat(q))
+                       rw_flags |= REQ_IO_STAT;
+       }
+
        spin_unlock_irq(q->queue_lock);
 
        rq = blk_alloc_request(q, rw_flags, priv, gfp_mask);
@@ -1777,7 +1785,7 @@ static void blk_account_io_completion(struct request *req, unsigned int bytes)
                int cpu;
 
                cpu = part_stat_lock();
-               part = disk_map_sector_rcu(req->rq_disk, blk_rq_pos(req));
+               part = req->part;
                part_stat_add(cpu, part, sectors[rw], bytes >> 9);
                part_stat_unlock();
        }
@@ -1797,7 +1805,7 @@ static void blk_account_io_done(struct request *req)
                int cpu;
 
                cpu = part_stat_lock();
-               part = disk_map_sector_rcu(req->rq_disk, blk_rq_pos(req));
+               part = req->part;
 
                part_stat_inc(cpu, part, ios[rw]);
                part_stat_add(cpu, part, ticks[rw], duration);

diff --git a/block/blk-merge.c b/block/blk-merge.c
index 6a725461654d86302c508faba05012f827d65780..38ff234012a4617e42a5950f362600219d04634b 100644 (file)
--- a/block/blk-merge.c
+++ b/block/blk-merge.c
@@ -351,7 +351,7 @@ static void blk_account_io_merge(struct request *req)
                int cpu;
 
                cpu = part_stat_lock();
-               part = disk_map_sector_rcu(req->rq_disk, blk_rq_pos(req));
+               part = req->part;
 
                part_round_stats(cpu, part);
                part_dec_in_flight(part, rq_data_dir(req));

diff --git a/block/blk.h b/block/blk.h
index 6738831ba4477ff67350dac3b0e3c3d5c43a445a..1340cce5721a755ee39e7c975a4813a3100537e6 100644 (file)
--- a/block/blk.h
+++ b/block/blk.h
@@ -110,10 +110,6 @@ void blk_queue_congestion_threshold(struct request_queue *q);
 
 int blk_dev_init(void);
 
-void elv_quiesce_start(struct request_queue *q);
-void elv_quiesce_end(struct request_queue *q);
-
-
 /*
  * Return the threshold (number of used requests) at which the queue is
  * considered to be congested.  It include a little hysteresis to keep the

diff --git a/block/genhd.c b/block/genhd.c
index 7923e720ddf52f1626cf5422ef5271fd9c7395d6..8313834596db660993cee5564a6729f21f6a7f6a 100644 (file)
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -932,8 +932,15 @@ static void disk_free_ptbl_rcu_cb(struct rcu_head *head)
 {
        struct disk_part_tbl *ptbl =
                container_of(head, struct disk_part_tbl, rcu_head);
+       struct gendisk *disk = ptbl->disk;
+       struct request_queue *q = disk->queue;
+       unsigned long flags;
 
        kfree(ptbl);
+
+       spin_lock_irqsave(q->queue_lock, flags);
+       elv_quiesce_end(q);
+       spin_unlock_irqrestore(q->queue_lock, flags);
 }
 
 /**
@@ -951,11 +958,17 @@ static void disk_replace_part_tbl(struct gendisk *disk,
                                  struct disk_part_tbl *new_ptbl)
 {
        struct disk_part_tbl *old_ptbl = disk->part_tbl;
+       struct request_queue *q = disk->queue;
 
        rcu_assign_pointer(disk->part_tbl, new_ptbl);
 
        if (old_ptbl) {
                rcu_assign_pointer(old_ptbl->last_lookup, NULL);
+
+               spin_lock_irq(q->queue_lock);
+               elv_quiesce_start(q);
+               spin_unlock_irq(q->queue_lock);
+
                call_rcu(&old_ptbl->rcu_head, disk_free_ptbl_rcu_cb);
        }
 }
@@ -996,6 +1009,7 @@ int disk_expand_part_tbl(struct gendisk *disk, int partno)
                return -ENOMEM;
 
        new_ptbl->len = target;
+       new_ptbl->disk = disk;
 
        for (i = 0; i < len; i++)
                rcu_assign_pointer(new_ptbl->part[i], old_ptbl->part[i]);

diff --git a/fs/partitions/check.c b/fs/partitions/check.c
index 6dfbee03ccc67337d8cc0b1f6cc834182be0867c..30f46c2cb9d5dfb4ae8691aefde7df6aebca47e6 100644 (file)
--- a/fs/partitions/check.c
+++ b/fs/partitions/check.c
@@ -365,17 +365,25 @@ struct device_type part_type = {
 static void delete_partition_rcu_cb(struct rcu_head *head)
 {
        struct hd_struct *part = container_of(head, struct hd_struct, rcu_head);
+       struct gendisk *disk = part_to_disk(part);
+       struct request_queue *q = disk->queue;
+       unsigned long flags;
 
        part->start_sect = 0;
        part->nr_sects = 0;
        part_stat_set_all(part, 0);
        put_device(part_to_dev(part));
+
+       spin_lock_irqsave(q->queue_lock, flags);
+       elv_quiesce_end(q);
+       spin_unlock_irqrestore(q->queue_lock, flags);
 }
 
 void delete_partition(struct gendisk *disk, int partno)
 {
        struct disk_part_tbl *ptbl = disk->part_tbl;
        struct hd_struct *part;
+       struct request_queue *q = disk->queue;
 
        if (partno >= ptbl->len)
                return;
@@ -390,6 +398,10 @@ void delete_partition(struct gendisk *disk, int partno)
        kobject_put(part->holder_dir);
        device_del(part_to_dev(part));
 
+       spin_lock_irq(q->queue_lock);
+       elv_quiesce_start(q);
+       spin_unlock_irq(q->queue_lock);
+
        call_rcu(&part->rcu_head, delete_partition_rcu_cb);
 }
 

diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 8f3dd981b973900944ab82625b8462a5f3f8ccec..16f7f1be1acf2d88569955dba40e11d9308cce4b 100644 (file)
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -115,6 +115,7 @@ struct request {
        void *elevator_private3;
 
        struct gendisk *rq_disk;
+       struct hd_struct *part;
        unsigned long start_time;
 #ifdef CONFIG_BLK_CGROUP
        unsigned long long start_time_ns;

diff --git a/include/linux/elevator.h b/include/linux/elevator.h
index 2c958f4fce1ed6f1d6f4a8c3fc865155cd1eac81..df1ee866d71567dfe522bd8cfbe53c3a7f2ce2e7 100644 (file)
--- a/include/linux/elevator.h
+++ b/include/linux/elevator.h
@@ -121,6 +121,8 @@ extern void elv_completed_request(struct request_queue *, struct request *);
 extern int elv_set_request(struct request_queue *, struct request *, gfp_t);
 extern void elv_put_request(struct request_queue *, struct request *);
 extern void elv_drain_elevator(struct request_queue *);
+extern void elv_quiesce_start(struct request_queue *);
+extern void elv_quiesce_end(struct request_queue *);
 
 /*
  * io scheduler registration

diff --git a/include/linux/genhd.h b/include/linux/genhd.h
index 66e26b5a1537af5a8d357ec84b7e530556f3af12..57647ecfc1bd634da5f7dfede07f6958256db8e3 100644 (file)
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -140,6 +140,7 @@ struct disk_part_tbl {
        struct rcu_head rcu_head;
        int len;
        struct hd_struct *last_lookup;
+       struct gendisk *disk;
        struct hd_struct *part[];
 };