block: remove support for bio remapping from ->make_request
There is very little benefit in allowing to let a ->make_request instance update
the bios device and sector and loop around it in __generic_make_request when
we can archive the same through calling generic_make_request from the driver
and letting the loop in generic_make_request handle it.
Note that various drivers got the return value from ->make_request and returned
non-zero values for errors.
/* can this bio be split generically?
@@ -1148,7 +1153,6 @@ int drbd_make_request(struct request_que
bio_pair_release(bp);
}
- return 0;
}
/* This is called by bio_add_page(). With this function we reduce
Index: linux-2.6/drivers/block/loop.c
================================================== =================
--- linux-2.6.orig/drivers/block/loop.c 2011-09-08 12:01:53.000000000 +0200
+++ linux-2.6/drivers/block/loop.c 2011-09-08 13:46:08.037866440 +0200
@@ -514,7 +514,7 @@ static struct bio *loop_get_bio(struct l
return bio_list_pop(&lo->lo_bio_list);
}
-static int loop_make_request(struct request_queue *q, struct bio *old_bio)
+static void loop_make_request(struct request_queue *q, struct bio *old_bio)
{
struct loop_device *lo = q->queuedata;
int rw = bio_rw(old_bio);
@@ -532,12 +532,11 @@ static int loop_make_request(struct requ
loop_add_bio(lo, old_bio);
wake_up(&lo->lo_event);
spin_unlock_irq(&lo->lo_lock);
- return 0;
+ return;
/* If this request crosses a chunk boundary, we need to
@@ -876,10 +876,8 @@ static int make_request(mddev_t *mddev,
conf->nr_waiting++;
spin_unlock_irq(&conf->resync_lock);
- if (make_request(mddev, &bp->bio1))
- generic_make_request(&bp->bio1);
- if (make_request(mddev, &bp->bio2))
- generic_make_request(&bp->bio2);
+ make_request(mddev, &bp->bio1);
+ make_request(mddev, &bp->bio2);
-int __make_request(struct request_queue *q, struct bio *bio)
+void __make_request(struct request_queue *q, struct bio *bio)
{
const bool sync = !!(bio->bi_rw & REQ_SYNC);
struct blk_plug *plug;
@@ -1236,7 +1236,7 @@ int __make_request(struct request_queue
* any locks.
*/
if (attempt_plug_merge(current, q, bio))
- goto out;
+ return;
spin_lock_irq(q->queue_lock);
@@ -1312,8 +1312,6 @@ get_rq:
out_unlock:
spin_unlock_irq(q->queue_lock);
}
-out:
- return 0;
}
EXPORT_SYMBOL_GPL(__make_request); /* for device mapper only */
@@ -1441,112 +1439,85 @@ static inline int bio_check_eod(struct b
static inline void __generic_make_request(struct bio *bio)
{
struct request_queue *q;
- sector_t old_sector;
- int ret, nr_sectors = bio_sectors(bio);
- dev_t old_dev;
+ int nr_sectors = bio_sectors(bio);
int err = -EIO;
+ char b[BDEVNAME_SIZE];
+ struct hd_struct *part;
might_sleep();
if (bio_check_eod(bio, nr_sectors))
goto end_io;
- /*
- * Resolve the mapping until finished. (drivers are
- * still free to implement/resolve their own stacking
- * by explicitly returning 0)
- *
- * NOTE: we don't repeat the blk_size check for each new device.
- * Stacking drivers are expected to know what they are doing.
- */
- old_sector = -1;
- old_dev = 0;
- do {
- char b[BDEVNAME_SIZE];
- struct hd_struct *part;
-
- q = bdev_get_queue(bio->bi_bdev);
- if (unlikely(!q)) {
- printk(KERN_ERR
- "generic_make_request: Trying to access "
- "nonexistent block-device %s (%Lu)
",
- bdevname(bio->bi_bdev, b),
- (long long) bio->bi_sector);
- goto end_io;
- }
-
- if (unlikely(!(bio->bi_rw & REQ_DISCARD) &&
- nr_sectors > queue_max_hw_sectors(q))) {
- printk(KERN_ERR "bio too big device %s (%u > %u)
",
- bdevname(bio->bi_bdev, b),
- bio_sectors(bio),
- queue_max_hw_sectors(q));
- goto end_io;
- }
-
- if (unlikely(test_bit(QUEUE_FLAG_DEAD, &q->queue_flags)))
- goto end_io;
-
- part = bio->bi_bdev->bd_part;
- if (should_fail_request(part, bio->bi_size) ||
- should_fail_request(&part_to_disk(part)->part0,
- bio->bi_size))
- goto end_io;
+ q = bdev_get_queue(bio->bi_bdev);
+ if (unlikely(!q)) {
+ printk(KERN_ERR
+ "generic_make_request: Trying to access "
+ "nonexistent block-device %s (%Lu)
",
+ bdevname(bio->bi_bdev, b),
+ (long long) bio->bi_sector);
+ goto end_io;
+ }
- /*
- * If this device has partitions, remap block n
- * of partition p to block n+start(p) of the disk.
- */
- blk_partition_remap(bio);
+ if (unlikely(!(bio->bi_rw & REQ_DISCARD) &&
+ nr_sectors > queue_max_hw_sectors(q))) {
+ printk(KERN_ERR "bio too big device %s (%u > %u)
",
+ bdevname(bio->bi_bdev, b),
+ bio_sectors(bio),
+ queue_max_hw_sectors(q));
+ goto end_io;
+ }
- if (bio_integrity_enabled(bio) && bio_integrity_prep(bio))
- goto end_io;
+ if (unlikely(test_bit(QUEUE_FLAG_DEAD, &q->queue_flags)))
+ goto end_io;
- if (old_sector != -1)
- trace_block_bio_remap(q, bio, old_dev, old_sector);
+ part = bio->bi_bdev->bd_part;
+ if (should_fail_request(part, bio->bi_size) ||
+ should_fail_request(&part_to_disk(part)->part0,
+ bio->bi_size))
+ goto end_io;
- old_sector = bio->bi_sector;
- old_dev = bio->bi_bdev->bd_dev;
+ /*
+ * If this device has partitions, remap block n
+ * of partition p to block n+start(p) of the disk.
+ */
+ blk_partition_remap(bio);
- if (bio_check_eod(bio, nr_sectors))
- goto end_io;
+ if (bio_integrity_enabled(bio) && bio_integrity_prep(bio))
+ goto end_io;
- /*
- * Filter flush bio's early so that make_request based
- * drivers without flush support don't have to worry
- * about them.
- */
- if ((bio->bi_rw & (REQ_FLUSH | REQ_FUA)) && !q->flush_flags) {
- bio->bi_rw &= ~(REQ_FLUSH | REQ_FUA);
- if (!nr_sectors) {
- err = 0;
- goto end_io;
- }
- }
+ if (bio_check_eod(bio, nr_sectors))
+ goto end_io;
- if ((bio->bi_rw & REQ_DISCARD) &&
- (!blk_queue_discard(q) ||
- ((bio->bi_rw & REQ_SECURE) &&
- !blk_queue_secdiscard(q)))) {
- err = -EOPNOTSUPP;
+ /*
+ * Filter flush bio's early so that make_request based
+ * drivers without flush support don't have to worry
+ * about them.
+ */
+ if ((bio->bi_rw & (REQ_FLUSH | REQ_FUA)) && !q->flush_flags) {
+ bio->bi_rw &= ~(REQ_FLUSH | REQ_FUA);
+ if (!nr_sectors) {
+ err = 0;
goto end_io;
}
+ }
- if (blk_throtl_bio(q, &bio))
- goto end_io;
-
- /*
- * If bio = NULL, bio has been throttled and will be submitted
- * later.
- */
- if (!bio)
- break;
-
- trace_block_bio_queue(q, bio);
+ if ((bio->bi_rw & REQ_DISCARD) &&
+ (!blk_queue_discard(q) ||
+ ((bio->bi_rw & REQ_SECURE) &&
+ !blk_queue_secdiscard(q)))) {
+ err = -EOPNOTSUPP;
+ goto end_io;
+ }
- ret = q->make_request_fn(q, bio);
- } while (ret);
+ if (blk_throtl_bio(q, &bio))
+ goto end_io;
+ /* if bio = NULL, bio has been throttled and will be submitted later. */
+ if (!bio)
+ return;
+ trace_block_bio_queue(q, bio);
+ q->make_request_fn(q, bio);
return;
end_io:
Index: linux-2.6/drivers/md/md.h
================================================== =================
--- linux-2.6.orig/drivers/md/md.h 2011-09-08 12:01:53.000000000 +0200
+++ linux-2.6/drivers/md/md.h 2011-09-08 12:05:58.630773689 +0200
@@ -424,7 +424,7 @@ struct mdk_personality
int level;
struct list_head list;
struct module *owner;
- int (*make_request)(mddev_t *mddev, struct bio *bio);
+ void (*make_request)(mddev_t *mddev, struct bio *bio);
int (*run)(mddev_t *mddev);
int (*stop)(mddev_t *mddev);
void (*status)(struct seq_file *seq, mddev_t *mddev);
Index: linux-2.6/drivers/md/md.c
================================================== =================
--- linux-2.6.orig/drivers/md/md.c 2011-09-08 12:01:53.000000000 +0200
+++ linux-2.6/drivers/md/md.c 2011-09-08 13:46:08.089865876 +0200
@@ -330,18 +330,17 @@ static DEFINE_SPINLOCK(all_mddevs_lock);
* call has finished, the bio has been linked into some internal structure
* and so is visible to ->quiesce(), so we don't need the refcount any more.
*/
-static int md_make_request(struct request_queue *q, struct bio *bio)
+static void md_make_request(struct request_queue *q, struct bio *bio)
{
const int rw = bio_data_dir(bio);
mddev_t *mddev = q->queuedata;
- int rv;
int cpu;
unsigned int sectors;
cpu = part_stat_lock();
part_stat_inc(cpu, &mddev->gendisk->part0, ios[rw]);
@@ -375,8 +374,6 @@ static int md_make_request(struct reques
if (atomic_dec_and_test(&mddev->active_io) && mddev->suspended)
wake_up(&mddev->sb_wait);
-
- return rv;
}
/* mddev_suspend makes sure no new requests are submitted
@@ -475,8 +472,7 @@ static void md_submit_flush_data(struct
bio_endio(bio, 0);
else {
bio->bi_rw &= ~REQ_FLUSH;
- if (mddev->pers->make_request(mddev, bio))
- generic_make_request(bio);
+ mddev->pers->make_request(mddev, bio);
}
mddev->flush_bio = NULL;
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
09-12-2011, 03:25 AM
NeilBrown
block: remove support for bio remapping from ->make_request
On Sun, 11 Sep 2011 10:51:29 -0400 Christoph Hellwig <hch@infradead.org>
wrote:
> There is very little benefit in allowing to let a ->make_request instance update
> the bios device and sector and loop around it in __generic_make_request when
> we can archive the same through calling generic_make_request from the driver
> and letting the loop in generic_make_request handle it.
Completely agree - I like this change!
>
> Note that various drivers got the return value from ->make_request and returned
> non-zero values for errors.
:-(
A diff-stat would have been nice... I think I've found all 'my' files though.
If the 'else' branch was taken, we need to generic_make_request(bio).
And 'b' isn't even declared at this level.
So at the end of the 'then' branch, add
bio = b;
and make the final call
generic_make_request(bio);
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
09-12-2011, 12:25 PM
Christoph Hellwig
block: remove support for bio remapping from ->make_request
On Mon, Sep 12, 2011 at 11:59:52AM +0200, Jens Axboe wrote:
> On 2011-09-11 16:51, Christoph Hellwig wrote:
> > There is very little benefit in allowing to let a ->make_request
> > instance update the bios device and sector and loop around it in
> > __generic_make_request when we can archive the same through calling
> > generic_make_request from the driver and letting the loop in
> > generic_make_request handle it.
>
> Agree, this looks good! Will apply for 3.2.
Are you going to fix the issue Neil pointed out beforehand?
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
block: remove support for bio remapping from ->make_request
