block: export __make_request
Subsystems (12)
PATCH 1/2 block: export __make_request
Avoid the hacks need for request based device mappers currently by simply exporting the symbol instead of trying to get it through the back door.
block/blk-core.c +2 -3
@@ -38,8 +38,6 @@ EXPORT_TRACEPOINT_SYMBOL_GPL(block_bio_rEXPORT_TRACEPOINT_SYMBOL_GPL (block_rq_remap);EXPORT_TRACEPOINT_SYMBOL_GPL (block_bio_complete); -static int __make_request (struct request_queue *q,struct bio *bio); -/* * For the allocated request tables */
@@ -1213,7 +1211,7 @@ void init_request_from_bio(struct requesblk_rq_bio_prep (req->q, req, bio); } -static int __make_request (struct request_queue *q,struct bio *bio) +int __make_request (struct request_queue *q,struct bio *bio) {const bool sync = !!(bio->bi_rw &REQ_SYNC );struct blk_plug *plug;
@@ -1317,6 +1315,7 @@ out_unlock: out:return 0 ; } +EXPORT_SYMBOL_GPL (__make_request);/* for device mapper only */ /* * If bio->bi_dev is a partition, remap the location
drivers/md/dm.c +1 -12
@@ -180,9 +180,6 @@ struct mapped_device {/* forced geometry settings */ struct hd_geometry geometry; -/* For saving the address of __make_request for request based dm */ -make_request_fn *saved_make_request_fn; -/* sysfs handle */ struct kobject kobj;
@@ -1420,13 +1417,6 @@ static int _dm_request(struct request_qureturn 0 ; } -static int dm_make_request (struct request_queue *q,struct bio *bio) -{ -struct mapped_device *md = q->queuedata; - -return md->saved_make_request_fn (q, bio);/* call __make_request() */ -} -static int dm_request_based (struct mapped_device *md) {return blk_queue_stackable (md->queue);
@@ -1437,7 +1427,7 @@ static int dm_request(struct request_questruct mapped_device *md = q->queuedata;if (dm_request_based (md)) -return dm_make_request (q, bio); +return __make_request (q, bio);return _dm_request (q, bio); }
@@ -2172,7 +2162,6 @@ static int dm_init_request_based_queue(sreturn 0 ; md->queue = q; - md->saved_make_request_fn = md->queue->make_request_fn;dm_init_md_queue (md);blk_queue_softirq_done (md->queue, dm_softirq_done);blk_queue_prep_rq (md->queue, dm_prep_fn);
include/linux/blkdev.h +2 -0
@@ -680,6 +680,8 @@ extern int scsi_cmd_ioctl(struct requestextern int sg_scsi_ioctl (struct request_queue *,struct gendisk *,fmode_t ,struct scsi_ioctl_command __user *); +extern int __make_request (struct request_queue *q,struct bio *bio); +/* * A queue has just exitted congestion. Note this in the global counter of * congested queues, and wake up anyone who was waiting for requests to be
PATCH 2/2 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.
arch/m68k/emu/nfblock.c +1 -2
@@ -59,7 +59,7 @@ struct nfhd_device {struct gendisk *disk; }; -static int nfhd_make_request (struct request_queue *queue,struct bio *bio) +static void nfhd_make_request (struct request_queue *queue,struct bio *bio) {struct nfhd_device *dev = queue->queuedata;struct bio_vec *bvec;
@@ -76,7 +76,6 @@ static int nfhd_make_request(struct requ sec += len; }bio_endio (bio,0 ); -return 0 ; }static int nfhd_getgeo (struct block_device *bdev,struct hd_geometry *geo)
arch/powerpc/sysdev/axonram.c +2 -6
@@ -104,7 +104,7 @@ axon_ram_irq_handler(int irq, void *dev)* axon_ram_make_request - make_request() method for block device * @queue, @bio: see blk_queue_make_request() */ -static int +static void axon_ram_make_request (struct request_queue *queue,struct bio *bio) {struct axon_ram_bank *bank = bio->bi_bdev->bd_disk->private_data;
@@ -113,7 +113,6 @@ axon_ram_make_request(struct request_questruct bio_vec *vec;unsigned int transfered;unsigned short idx; -int rc =0 ; phys_mem = bank->io_addr + (bio->bi_sector <<AXON_RAM_SECTOR_SHIFT ); phys_end = bank->io_addr + bank->size;
@@ -121,8 +120,7 @@ axon_ram_make_request(struct request_quebio_for_each_segment (vec, bio, idx) {if (unlikely (phys_mem + vec->bv_len > phys_end)) {bio_io_error (bio); - rc = -ERANGE ; -break ; +return ; } user_mem =page_address (vec->bv_page) + vec->bv_offset;
@@ -135,8 +133,6 @@ axon_ram_make_request(struct request_que transfered += vec->bv_len; }bio_endio (bio,0 ); - -return rc; }/**
drivers/block/aoe/aoeblk.c +6 -8
@@ -159,7 +159,7 @@ aoeblk_release(struct gendisk *disk, fmoreturn 0 ; } -static int +static void aoeblk_make_request (struct request_queue *q,struct bio *bio) {struct sk_buff_head queue;
@@ -172,25 +172,25 @@ aoeblk_make_request(struct request_queueif (bio ==NULL ) {printk (KERN_ERR "aoe: bio is NULL\n" );BUG (); -return 0 ; +return ; } d = bio->bi_bdev->bd_disk->private_data;if (d ==NULL ) {printk (KERN_ERR "aoe: bd_disk->private_data is NULL\n" );BUG ();bio_endio (bio, -ENXIO ); -return 0 ; +return ; }else if (bio->bi_io_vec ==NULL ) {printk (KERN_ERR "aoe: bi_io_vec is NULL\n" );BUG ();bio_endio (bio, -ENXIO ); -return 0 ; +return ; } buf =mempool_alloc (d->bufpool,GFP_NOIO );if (buf ==NULL ) {printk (KERN_INFO "aoe: buf allocation failure\n" );bio_endio (bio, -ENOMEM ); -return 0 ; +return ; }memset (buf,0 ,sizeof (*buf));INIT_LIST_HEAD (&buf->bufs);
@@ -211,7 +211,7 @@ aoeblk_make_request(struct request_queuespin_unlock_irqrestore (&d->lock, flags);mempool_free (buf, d->bufpool);bio_endio (bio, -ENXIO ); -return 0 ; +return ; }list_add_tail (&buf->bufs, &d->bufq);
@@ -222,8 +222,6 @@ aoeblk_make_request(struct request_queuespin_unlock_irqrestore (&d->lock, flags);aoenet_xmit (&queue); - -return 0 ; } static int
drivers/block/brd.c +1 -3
@@ -323,7 +323,7 @@ out:return err; } -static int brd_make_request (struct request_queue *q,struct bio *bio) +static void brd_make_request (struct request_queue *q,struct bio *bio) {struct block_device *bdev = bio->bi_bdev;struct brd_device *brd = bdev->bd_disk->private_data;
@@ -359,8 +359,6 @@ static int brd_make_request(struct reque out:bio_endio (bio, err); - -return 0 ; }#ifdef CONFIG_BLK_DEV_XIP
drivers/block/drbd/drbd_int.h +1 -1
@@ -1507,7 +1507,7 @@ extern void drbd_free_mdev(struct drbd_cextern int proc_details;/* drbd_req */ -extern int drbd_make_request (struct request_queue *q,struct bio *bio); +extern void drbd_make_request (struct request_queue *q,struct bio *bio);extern int drbd_read_remote (struct drbd_conf *mdev,struct drbd_request *req);extern int drbd_merge_bvec (struct request_queue *q,struct bvec_merge_data *bvm,struct bio_vec *bvec);extern int is_valid_ar_handle (struct drbd_request *,sector_t );
drivers/block/drbd/drbd_req.c +4 -4
@@ -1073,7 +1077,7 @@ static int drbd_fail_request_early(strucreturn 0 ; } -int drbd_make_request (struct request_queue *q,struct bio *bio) +void drbd_make_request (struct request_queue *q,struct bio *bio) {unsigned int s_enr, e_enr;struct drbd_conf *mdev = (struct drbd_conf *) q->queuedata;
@@ -1081,7 +1085,7 @@ int drbd_make_request(struct request_queif (drbd_fail_request_early (mdev,bio_data_dir (bio) &WRITE )) {bio_endio (bio, -EPERM ); -return 0 ; +return ; } start_time = jiffies;
@@ -1100,7 +1104,8 @@ int drbd_make_request(struct request_queif (likely (s_enr == e_enr)) {inc_ap_bio (mdev,1 ); -return drbd_make_request_common (mdev, bio, start_time); +drbd_make_request_common (mdev, bio, start_time); +return ; }/* can this bio be split generically?
@@ -1148,7 +1153,6 @@ int drbd_make_request(struct request_quebio_pair_release (bp); } -return 0 ; }/* This is called by bio_add_page(). With this function we reduce
drivers/block/loop.c +2 -3
@@ -514,7 +514,7 @@ static struct bio *loop_get_bio(struct lreturn 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 requloop_add_bio (lo, old_bio);wake_up (&lo->lo_event);spin_unlock_irq (&lo->lo_lock); -return 0 ; +return ; out:spin_unlock_irq (&lo->lo_lock);bio_io_error (old_bio); -return 0 ; }struct switch_request {
drivers/block/pktcdvd.c +5 -6
@@ -2444,7 +2444,7 @@ static void pkt_end_io_read_cloned(strucpkt_bio_finished (pd); } -static int pkt_make_request (struct request_queue *q,struct bio *bio) +static void pkt_make_request (struct request_queue *q,struct bio *bio) {struct pktcdvd_device *pd;char b[BDEVNAME_SIZE ];
@@ -2473,7 +2473,7 @@ static int pkt_make_request(struct reque cloned_bio->bi_end_io = pkt_end_io_read_cloned; pd->stats.secs_r += bio->bi_size >>9 ;pkt_queue_bio (pd, cloned_bio); -return 0 ; +return ; }if (!test_bit (PACKET_WRITABLE , &pd->flags)) {
@@ -2509,7 +2509,7 @@ static int pkt_make_request(struct requepkt_make_request (q, &bp->bio1);pkt_make_request (q, &bp->bio2);bio_pair_release (bp); -return 0 ; +return ; } }
@@ -2533,7 +2533,7 @@ static int pkt_make_request(struct reque }spin_unlock (&pkt->lock);spin_unlock (&pd->cdrw.active_list_lock); -return 0 ; +return ; }else { blocked_bio =1 ; }
@@ -2584,10 +2584,9 @@ static int pkt_make_request(struct reque*/ wake_up (&pd->wqueue); } -return 0 ; +return ; end_io:bio_io_error (bio); -return 0 ; }
drivers/block/ps3vram.c +2 -4
@@ -596,7 +596,7 @@ out:return next; } -static int ps3vram_make_request (struct request_queue *q,struct bio *bio) +static void ps3vram_make_request (struct request_queue *q,struct bio *bio) {struct ps3_system_bus_device *dev = q->queuedata;struct ps3vram_priv *priv =ps3_system_bus_get_drvdata (dev);
@@ -610,13 +610,11 @@ static int ps3vram_make_request(struct rspin_unlock_irq (&priv->lock);if (busy) -return 0 ; +return ;do { bio =ps3vram_do_bio (dev, bio); }while (bio); - -return 0 ; }static int __devinit ps3vram_probe (struct ps3_system_bus_device *dev)
drivers/block/umem.c +2 -2
@@ -513,7 +513,7 @@ static void process_page(unsigned long d } } -static int mm_make_request (struct request_queue *q,struct bio *bio) +static void mm_make_request (struct request_queue *q,struct bio *bio) {struct cardinfo *card = q->queuedata; pr_debug("mm_make_request %llu %u\n",
@@ -525,7 +525,7 @@ static int mm_make_request(struct reques card->biotail = &bio->bi_next;spin_unlock_irq (&card->lock); -return 0 ; +return ; }static irqreturn_t mm_interrupt (int irq,void *__card)
drivers/md/dm.c +7 -7
@@ -1388,7 +1388,7 @@ out:* The request function that just remaps the bio built up by * dm_merge_bvec. */ -static int _dm_request (struct request_queue *q,struct bio *bio) +static void _dm_request (struct request_queue *q,struct bio *bio) {int rw =bio_data_dir (bio);struct mapped_device *md = q->queuedata;
@@ -1409,12 +1409,12 @@ static int _dm_request(struct request_ququeue_io (md, bio); elsebio_io_error (bio ); -return 0 ; +return ; }__split_and_process_bio (md, bio);up_read (&md->io_lock); -return 0 ; +return ; }static int dm_request_based (struct mapped_device *md)
@@ -1422,14 +1422,14 @@ static int dm_request_based(struct mappereturn blk_queue_stackable (md->queue); } -static int dm_request (struct request_queue *q,struct bio *bio) +static void dm_request (struct request_queue *q,struct bio *bio) {struct mapped_device *md = q->queuedata;if (dm_request_based (md)) -return __make_request (q, bio); - -return _dm_request (q, bio); +__make_request (q, bio); +else +_dm_request (q, bio); }void dm_dispatch_request (struct request *rq)
drivers/s390/block/dcssblk.c +3 -4
@@ -27,7 +27,7 @@static int dcssblk_open (struct block_device *bdev,fmode_t mode);static int dcssblk_release (struct gendisk *disk,fmode_t mode); -static int dcssblk_make_request (struct request_queue *q,struct bio *bio); +static void dcssblk_make_request (struct request_queue *q,struct bio *bio);static int dcssblk_direct_access (struct block_device *bdev,sector_t secnum,void **kaddr,unsigned long *pfn);
@@ -814,7 +814,7 @@ out:return rc; } -static int +static void dcssblk_make_request (struct request_queue *q,struct bio *bio) {struct dcssblk_dev_info *dev_info;
@@ -871,10 +871,9 @@ dcssblk_make_request(struct request_queu bytes_done += bvec->bv_len; }bio_endio (bio,0 ); -return 0 ; +return ; fail:bio_io_error (bio); -return 0 ; } static int
drivers/s390/block/xpram.c +2 -3
@@ -181,7 +181,7 @@ static unsigned long xpram_highest_page_/* * Block device make request function. */ -static int xpram_make_request (struct request_queue *q,struct bio *bio) +static void xpram_make_request (struct request_queue *q,struct bio *bio) {xpram_device_t *xdev = bio->bi_bdev->bd_disk->private_data;struct bio_vec *bvec;
@@ -221,10 +221,9 @@ static int xpram_make_request(struct req }set_bit (BIO_UPTODATE , &bio->bi_flags);bio_endio (bio,0 ); -return 0 ; +return ; fail:bio_io_error (bio); -return 0 ; }static int xpram_getgeo (struct block_device *bdev,struct hd_geometry *geo)
drivers/staging/zram/zram_drv.c +3 -5
@@ -556,24 +556,22 @@ static inline int valid_io_request(struc/* * Handler function for all zram I/O requests. */ -static int zram_make_request (struct request_queue *queue,struct bio *bio) +static void zram_make_request (struct request_queue *queue,struct bio *bio) {struct zram *zram = queue->queuedata;if (!valid_io_request (zram, bio)) {zram_stat64_inc (zram, &zram->stats.invalid_io);bio_io_error (bio); -return 0 ; +return ; }if (unlikely (!zram->init_done) &&zram_init_device (zram)) {bio_io_error (bio); -return 0 ; +return ; }__zram_make_request (zram, bio,bio_data_dir (bio)); - -return 0 ; }void zram_reset_device (struct zram *zram)
include/linux/blkdev.h +2 -2
@@ -195,7 +195,7 @@ struct request_pm_state#include <linux/elevator.h> typedef void (request_fn_proc ) (struct request_queue *q); -typedef int (make_request_fn ) (struct request_queue *q,struct bio *bio); +typedef void (make_request_fn ) (struct request_queue *q,struct bio *bio);typedef int (prep_rq_fn ) (struct request_queue *,struct request *);typedef void (unprep_rq_fn ) (struct request_queue *,struct request *);
@@ -680,7 +680,7 @@ extern int scsi_cmd_ioctl(struct requestextern int sg_scsi_ioctl (struct request_queue *,struct gendisk *,fmode_t ,struct scsi_ioctl_command __user *); -extern int __make_request (struct request_queue *q,struct bio *bio); +extern void __make_request (struct request_queue *q,struct bio *bio);/* * A queue has just exitted congestion. Note this in the global counter of
drivers/md/faulty.c +4 -5
@@ -169,7 +169,7 @@ static void add_sector(conf_t *conf, sec conf->nfaults = n+1 ; } -static int make_request (mddev_t *mddev,struct bio *bio) +static void make_request (mddev_t *mddev,struct bio *bio) {conf_t *conf = mddev->private;int failit =0 ;
@@ -181,7 +181,7 @@ static int make_request(mddev_t *mddev,* just fail immediately */ bio_endio (bio, -EIO ); -return 0 ; +return ; } if (check_sector(conf, bio->bi_sector, bio->bi_sector+(bio->bi_size>>9),
@@ -214,12 +214,11 @@ static int make_request(mddev_t *mddev, b->bi_bdev = conf->rdev->bdev; b->bi_private = bio; b->bi_end_io = faulty_fail; -generic_make_request (b); -return 0 ; }else { bio->bi_bdev = conf->rdev->bdev; -return 1 ; } + +generic_make_request (b); }static void status (struct seq_file *seq,mddev_t *mddev)
drivers/md/linear.c +7 -10
@@ -264,14 +264,14 @@ static int linear_stop (mddev_t *mddev)return 0 ; } -static int linear_make_request (mddev_t *mddev,struct bio *bio) +static void linear_make_request (mddev_t *mddev,struct bio *bio) {dev_info_t *tmp_dev;sector_t start_sector;if (unlikely (bio->bi_rw &REQ_FLUSH )) {md_flush_request (mddev, bio); -return 0 ; +return ; }rcu_read_lock ();
@@ -293,7 +293,7 @@ static int linear_make_request (mddev_t (unsigned long long)start_sector);rcu_read_unlock ();bio_io_error (bio); -return 0 ; +return ; }if (unlikely (bio->bi_sector + (bio->bi_size >>9 ) > tmp_dev->end_sector)) {
@@ -307,20 +307,17 @@ static int linear_make_request (mddev_t bp =bio_split (bio, end_sector - bio->bi_sector); -if (linear_make_request (mddev, &bp->bio1)) -generic_make_request (&bp->bio1); -if (linear_make_request (mddev, &bp->bio2)) -generic_make_request (&bp->bio2); +linear_make_request (mddev, &bp->bio1); +linear_make_request (mddev, &bp->bio2);bio_pair_release (bp); -return 0 ; +return ; } bio->bi_bdev = tmp_dev->rdev->bdev; bio->bi_sector = bio->bi_sector - start_sector + tmp_dev->rdev->data_offset;rcu_read_unlock (); - -return 1 ; +generic_make_request (bio); }static void linear_status (struct seq_file *seq,mddev_t *mddev)
drivers/md/multipath.c +4 -4
@@ -106,7 +106,7 @@ static void multipath_end_request(structrdev_dec_pending (rdev, conf->mddev); } -static int multipath_make_request (mddev_t *mddev,struct bio * bio) +static void multipath_make_request (mddev_t *mddev,struct bio * bio) {multipath_conf_t *conf = mddev->private;struct multipath_bh * mp_bh;
@@ -114,7 +114,7 @@ static int multipath_make_request(mddev_if (unlikely (bio->bi_rw &REQ_FLUSH )) {md_flush_request (mddev, bio); -return 0 ; +return ; } mp_bh =mempool_alloc (conf->pool,GFP_NOIO );
@@ -126,7 +126,7 @@ static int multipath_make_request(mddev_if (mp_bh->path <0 ) {bio_endio (bio, -EIO );mempool_free (mp_bh, conf->pool); -return 0 ; +return ; } multipath = conf->multipaths + mp_bh->path;
@@ -137,7 +137,7 @@ static int multipath_make_request(mddev_ mp_bh->bio.bi_end_io = multipath_end_request; mp_bh->bio.bi_private = mp_bh;generic_make_request (&mp_bh->bio); -return 0 ; +return ; }static void multipath_status (struct seq_file *seq,mddev_t *mddev)
drivers/md/raid0.c +9 -13
@@ -466,7 +466,7 @@ static inline int is_io_in_chunk_boundar } } -static int raid0_make_request (mddev_t *mddev,struct bio *bio) +static void raid0_make_request (mddev_t *mddev,struct bio *bio) {unsigned int chunk_sects;sector_t sector_offset;
@@ -475,7 +475,7 @@ static int raid0_make_request(mddev_t *mif (unlikely (bio->bi_rw &REQ_FLUSH )) {md_flush_request (mddev, bio); -return 0 ; +return ; } chunk_sects = mddev->chunk_sectors;
@@ -495,13 +495,10 @@ static int raid0_make_request(mddev_t *melse bp =bio_split (bio, chunk_sects -sector_div (sector, chunk_sects)); -if (raid0_make_request (mddev, &bp->bio1)) -generic_make_request (&bp->bio1); -if (raid0_make_request (mddev, &bp->bio2)) -generic_make_request (&bp->bio2); - +raid0_make_request (mddev, &bp->bio1); +raid0_make_request (mddev, &bp->bio2);bio_pair_release (bp); -return 0 ; +return ; } sector_offset = bio->bi_sector;
@@ -511,10 +508,9 @@ static int raid0_make_request(mddev_t *m bio->bi_bdev = tmp_dev->bdev; bio->bi_sector = sector_offset + zone->dev_start + tmp_dev->data_offset; -/* -* Let the main block layer submit the IO and resolve recursion: -*/ -return 1 ; + +generic_make_request (bio); +return ; bad_map: printk("md/raid0:%s: make_request bug: can't convert block across chunks"
@@ -523,7 +519,7 @@ bad_map: (unsigned long long)bio->bi_sector, bio->bi_size >> 10);bio_io_error (bio); -return 0 ; +return ; }static void raid0_status (struct seq_file *seq,mddev_t *mddev)
drivers/md/raid1.c +3 -5
@@ -785,7 +785,7 @@ do_sync_io:PRINTK ("%dB behind alloc failed, doing sync I/O\n" , bio->bi_size); } -static int make_request (mddev_t *mddev,struct bio * bio) +static void make_request (mddev_t *mddev,struct bio * bio) {conf_t *conf = mddev->private;mirror_info_t *mirror;
@@ -870,7 +870,7 @@ read_again:if (rdisk <0 ) {/* couldn't find anywhere to read from */ raid_end_bio_io (r1_bio); -return 0 ; +return ; } mirror = conf->mirrors + rdisk;
@@ -928,7 +928,7 @@ read_again: goto read_again; }else generic_make_request (read_bio); -return 0 ; +return ; }/*
@@ -1119,8 +1119,6 @@ read_again:if (do_sync || !bitmap || !plugged)md_wakeup_thread (mddev->thread); - -return 0 ; }static void status (struct seq_file *seq,mddev_t *mddev)
drivers/md/raid10.c +8 -11
@@ -825,7 +825,7 @@ static void unfreeze_array(conf_t *conf)spin_unlock_irq (&conf->resync_lock); } -static int make_request (mddev_t *mddev,struct bio * bio) +static void make_request (mddev_t *mddev,struct bio * bio) {conf_t *conf = mddev->private;mirror_info_t *mirror;
@@ -844,7 +844,7 @@ static int make_request(mddev_t *mddev,if (unlikely (bio->bi_rw &REQ_FLUSH )) {md_flush_request (mddev, bio); -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);spin_lock_irq (&conf->resync_lock); conf->nr_waiting--;
@@ -887,14 +885,14 @@ static int make_request(mddev_t *mddev,spin_unlock_irq (&conf->resync_lock);bio_pair_release (bp); -return 0 ; +return ; bad_map:printk ("md/raid10:%s: make_request bug: can't convert block across chunks" " or bigger than %dk %llu %d\n" ,mdname (mddev), chunk_sects/2 , (unsigned long long )bio->bi_sector, bio->bi_size >>10 );bio_io_error (bio); -return 0 ; +return ; }md_write_start (mddev, bio);
@@ -937,7 +935,7 @@ read_again: slot = r10_bio->read_slot;if (disk <0 ) {raid_end_bio_io (r10_bio); -return 0 ; +return ; } mirror = conf->mirrors + disk;
@@ -985,7 +983,7 @@ read_again: goto read_again; }else generic_make_request (read_bio); -return 0 ; +return ; }/*
@@ -1157,7 +1155,6 @@ retry_write:if (do_sync || !mddev->bitmap || !plugged)md_wakeup_thread (mddev->thread); -return 0 ; }static void status (struct seq_file *seq,mddev_t *mddev)
drivers/md/raid5.c +3 -5
@@ -3695,7 +3695,7 @@ static struct stripe_head *__get_prioritreturn sh; } -static int make_request (mddev_t *mddev,struct bio * bi) +static void make_request (mddev_t *mddev,struct bio * bi) {raid5_conf_t *conf = mddev->private;int dd_idx;
@@ -3708,7 +3708,7 @@ static int make_request(mddev_t *mddev,if (unlikely (bi->bi_rw &REQ_FLUSH )) {md_flush_request (mddev, bi); -return 0 ; +return ; }md_write_start (mddev, bi);
@@ -3716,7 +3716,7 @@ static int make_request(mddev_t *mddev,if (rw ==READ && mddev->reshape_position == MaxSector &&chunk_aligned_read (mddev,bi)) -return 0 ; +return ; logical_sector = bi->bi_sector & ~((sector_t )STRIPE_SECTORS -1 ); last_sector = bi->bi_sector + (bi->bi_size>>9 );
@@ -3851,8 +3851,6 @@ static int make_request(mddev_t *mddev,bio_endio (bi,0 ); } - -return 0 ; }static sector_t raid5_size (mddev_t *mddev,sector_t sectors,int raid_disks);
block/blk-core.c +62 -91
196 lines not on this page · raw message
drivers/md/md.h +1 -1
@@ -424,7 +424,7 @@ struct mdk_personalityint 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);
drivers/md/md.c +4 -8
@@ -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;if (mddev ==NULL || mddev->pers ==NULL || !mddev->ready) {bio_io_error (bio); -return 0 ; +return ; }smp_rmb ();/* Ensure implications of 'active' are visible */ rcu_read_lock ();
@@ -366,7 +365,7 @@ static int md_make_request(struct reques* go away inside make_request */ sectors =bio_sectors (bio); - rv = mddev->pers->make_request (mddev, bio); + mddev->pers->make_request (mddev, bio); cpu =part_stat_lock ();part_stat_inc (cpu, &mddev->gendisk->part0, ios[rw]);
@@ -375,8 +374,6 @@ static int md_make_request(struct requesif (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(structbio_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 ;
Re: [PATCH 2/2] block: remove support for bio remapping from ->make_request
Quoted 4 lines · On Sun, 11 Sep 2011 10:51:29 -0400 Christoph Hellwig <[email protected]> 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_requestwhen we can archive the same through callinggeneric_make_requestfrom the driver and letting the loop ingeneric_make_requesthandle it.
Completely agree - I like this change!
Quoted 3 lines
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.
Quoted 25 lines
Index:
linux-2.6/drivers/md/faulty.c
===================================================================drivers/md/faulty.c +2
-2@@ -169,7 +169,7 @@ static void add_sector(conf_t *conf, sec conf->nfaults = n+1 ; } -static int make_request (mddev_t *mddev,struct bio *bio) +static void make_request (mddev_t *mddev,struct bio *bio) {conf_t *conf = mddev->private;int failit =0 ;@@ -181,7 +181,7 @@ static int make_request(mddev_t *mddev,* just fail immediately */ bio_endio (bio, -EIO ); -return 0 ; +return ; } if (check_sector(conf, bio->bi_sector, bio->bi_sector+(bio->bi_size>>9),@@ -214,12 +214,11 @@ static int make_request(mddev_t *mddev, b->bi_bdev = conf->rdev->bdev; b->bi_private = bio;
drivers/md/faulty.c static int make_request(mddev_t *mddev, · line 222
Quoted 10 lines
drivers/md/faulty.c +2
-3b->bi_end_io = faulty_fail; -generic_make_request (b); -return 0 ; }else { bio->bi_bdev = conf->rdev->bdev; -return 1 ; } + +generic_make_request (b); }
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);Changes to these files:
Quoted 8 lines
Index:
linux-2.6/drivers/md/linear.c
Index:linux-2.6/drivers/md/multipath.c
Index:linux-2.6/drivers/md/raid0.c
Index:linux-2.6/drivers/md/raid1.c
Index:linux-2.6/drivers/md/raid10.c
Index:linux-2.6/drivers/md/raid5.c
Index:linux-2.6/drivers/md/md.h
Index:linux-2.6/drivers/md/md.c
Thanks,
NeilBrown
Jens Axboe
maintainerRe: [PATCH 2/2] block: remove support for bio remapping from ->make_request
In reply to Christoph HellwigQuoted 5 lines · Christoph Hellwig
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_requestwhen we can archive the same through callinggeneric_make_requestfrom the driver and letting the loop ingeneric_make_requesthandle it.
Agree, this looks good! Will apply for 3.2.
Quoted 2 lines
Note that various drivers got the return value from ->make_request and returned non-zero values for errors.
Hadn't noticed that, I guess the return values are easy to get wrong in that case. Never a good sign.
Signature · 1 line
Jens Axboe
Re: [PATCH 2/2] block: remove support for bio remapping from ->make_request
Quoted 8 lines · Jens Axboe
Quoted 5 lines · Christoph Hellwig
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_requestwhen we can archive the same through callinggeneric_make_requestfrom the driver and letting the loop ingeneric_make_requesthandle it.Agree, this looks good! Will apply for 3.2.
Are you going to fix the issue Neil pointed out beforehand?
Jens Axboe
maintainerRe: [PATCH 2/2] block: remove support for bio remapping from ->make_request
Quoted 11 lines · Christoph Hellwig
Quoted 8 lines · Jens Axboe
Quoted 5 lines · Christoph Hellwig
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_requestwhen we can archive the same through callinggeneric_make_requestfrom the driver and letting the loop ingeneric_make_requesthandle it.Agree, this looks good! Will apply for 3.2.
Are you going to fix the issue Neil pointed out beforehand?
Yep, I merged that one in with the 2/2 patch and added his Acked-by as well.
Signature · 1 line
Jens Axboe
Jens Axboe
maintainerQuoted 2 lines · Christoph Hellwig
Avoid the hacks need for request based device mappers currently by simply exporting the symbol instead of trying to get it through the back door.
Good idea, lets rename it to blk_make_request() at the same time though. I'll merge it as such.
Signature · 1 line
Jens Axboe
Quoted 6 lines · Jens Axboe
Quoted 2 lines · Christoph Hellwig
Avoid the hacks need for request based device mappers currently by simply exporting the symbol instead of trying to get it through the back door.
Good idea, lets rename it to
blk_make_request()at the same time though. I'll merge it as such.
What happens to the existing blk_make_request?
Jens Axboe
maintainerQuoted 9 lines · Christoph Hellwig
Quoted 6 lines · Jens Axboe
Quoted 2 lines · Christoph Hellwig
Avoid the hacks need for request based device mappers currently by simply exporting the symbol instead of trying to get it through the back door.
Good idea, lets rename it to
blk_make_request()at the same time though. I'll merge it as such.What happens to the existing
blk_make_request?
I ended up naming __make_request() blk_queue_bio() instead.
Signature · 1 line
Jens Axboe
Quoted 3 lines · Jens Axboe
Quoted 1 line
What happens to the existing
blk_make_request?I ended up naming
__make_request()blk_queue_bio()instead.
I really hate naming things different from the method they are implementing. I've tried to figure out what the point of the old blk_make_request is - why would we not go through generic_make_request for this?
Boaz, any idea?
Jens Axboe
maintainerQuoted 11 lines · Christoph Hellwig
Quoted 3 lines · Jens Axboe
Quoted 1 line
What happens to the existing
blk_make_request?I ended up naming
__make_request()blk_queue_bio()instead.I really hate naming things different from the method they are implementing. I've tried to figure out what the point of the old
blk_make_requestis - why would we not go throughgeneric_make_requestfor this?Boaz, any idea?
I tend to agree, we could rename the existing blk_make_request(). It could be blk_make_request_from_bio() or something like that, since that's what it does.
Signature · 1 line
Jens Axboe
Quoted 10 lines · Jens Axboe
Quoted 6 lines
I really hate naming things different from the method they are implementing. I've tried to figure out what the point of the old
blk_make_requestis - why would we not go throughgeneric_make_requestfor this?Boaz, any idea?
I tend to agree, we could rename the existing
blk_make_request(). It could beblk_make_request_from_bio()or something like that, since that's what it does.
It should at very least be renamed. But I still can't figure out what it is for exactly.
There are three users:
(1) virtio_blk::virtblk_get_id():
This looks like it really should just use blk_rq_map_kern.
(2) osd_initiator::_make_request():
This one looks like it should just use the same scheme as sg_io(), as it's doing the same thing.
(3) target_core_pscsi::__pscsi_map_SG():
Same as (2).
Quoted 15 lines · Christoph Hellwig
Quoted 10 lines · Jens Axboe
Quoted 6 lines
I really hate naming things different from the method they are implementing. I've tried to figure out what the point of the old
blk_make_requestis - why would we not go throughgeneric_make_requestfor this?Boaz, any idea?
I tend to agree, we could rename the existing
blk_make_request(). It could beblk_make_request_from_bio()or something like that, since that's what it does.It should at very least be renamed. But I still can't figure out what it is for exactly.
Quoted 7 lines
There are three users:
(1)
virtio_blk::virtblk_get_id():
This looks like it really should just useblk_rq_map_kern.
(2)osd_initiator::_make_request():
This one looks like it should just use the same scheme assg_io(), as it's doing the same thing.
Good god what sg_io? That broken pointr+length from user-mode that sg.c
and bsg.c are using? no can do it's not user-mode pointers, and it's not
pointer+length it's pages pointers of a bio. The only other structure
that could carry the same information is struct sg, but we work very hard to get rid of this contraption. (scsi_execute_async or something that it was)
blk_make_request() was made to be the parallel of __make_request, to be used from filesystem level users. But with two differences.
1. Mainly support for none-FS BLOCK_PC requests
2. Also support chained bios. (was added later)
Quoted 3 lines
(3)
target_core_pscsi::__pscsi_map_SG():Same as (2).
There is no better suitable structure in current Kernel to carry a list of pages, with optional offset and length, then bio struct. Given a bio at hand. how do you make a block request out of it? (If it's not an FS_PC type IO?)
As I remember target_core had their own pages-linked-list structure, and how do you make a request out of that? again best at hand is bio.
bio is for a long time a page-pointers-carrier-structure and is out of private block-level use. The filesystem level is full of it.
Thanks
Boaz
It would be great if this function were exported, but I would encourage everyone to consider exporting it generally, not just for GPL usage.
The kernel seems to be pretty clean that some subsystems are GPL only (things like sysfs), and some are open (things like the device-mapper interfaces). When a subsystem is part GPL and part open, it gets very confusing for developers of commercial modules to know what to use and what not to use.
The goal of GPL versus commercial exports should be to encourage GPL code, not to punish commercial code by requiring stupid work arounds that only waste time and hurt performance.
Again, most of the block IO layer is plain exports. Adding random GPL exports seems somewhat arbitrary.
Then again, this is your code, and If you believe it should be GPL linkable only, then that is fine. Just consider that there are projects out there that are strange enough to be impacted and some degree of symmetry is desirable.
As a side node, I would vote for longer, more descriptive names. "__anything" should never be exported.
Doug Dumitru
EasyCo LLC
Quoted 60 lines · Boaz Harrosh
Quoted 17 lines · Christoph Hellwig
Quoted 11 lines · Jens Axboe
Quoted 7 lines
I really hate naming things different from the method they are implementing. I've tried to figure out what the point of the old
blk_make_requestis - why would we not go throughgeneric_make_requestfor this?Boaz, any idea?
I tend to agree, we could rename the existing
blk_make_request(). It could beblk_make_request_from_bio()or something like that, since that's what it does.It should at very least be renamed. But I still can't figure out what it is for exactly.
Quoted 7 lines
There are three users:
(1)
virtio_blk::virtblk_get_id():
This looks like it really should just useblk_rq_map_kern.
(2)osd_initiator::_make_request():
This one looks like it should just use the same scheme assg_io(), as it's doing the same thing.Good god what
sg_io? That broken pointr+length from user-mode thatsg.candbsg.care using? no can do it's not user-mode pointers, and it's not pointer+length it's pages pointers of a bio. The only other structure that could carry the same information is struct sg, but we work very hard to get rid of this contraption. (scsi_execute_asyncor something that it was)
blk_make_request()was made to be the parallel of__make_request, to be used from filesystem level users. But with two differences.
1. Mainly support for none-FSBLOCK_PCrequests
2. Also support chained bios. (was added later)Quoted 3 lines
(3)
target_core_pscsi::__pscsi_map_SG():
Same as (2).There is no better suitable structure in current Kernel to carry a list of pages, with optional offset and length, then bio struct. Given a bio at hand. how do you make a block request out of it? (If it's not an
FS_PCtype IO?)As I remember
target_corehad their own pages-linked-list structure, and how do you make a request out of that? again best at hand is bio.bio is for a long time a page-pointers-carrier-structure and is out of private block-level use. The filesystem level is full of it.
Thanks
Boaz--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Signature · 6 lines
Doug Dumitru EasyCo LLC -- To unsubscribe from this list: send the line "unsubscribe linux-raid" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 3 lines · On Wed, 14 Sep 2011 10:53:22 -0700 Doug Dumitru <[email protected]> wrote:
It would be great if this function were exported, but I would encourage everyone to consider exporting it generally, not just for GPL usage.
That would require a licensing change for the kernel. The kernel is a GPL work. Some of us have not licensed it in any other way.
Quoted 3 lines
The kernel seems to be pretty clean that some subsystems are GPL only (things like sysfs), and some are open
The _GPL is meant to make it clear they are internal symbols. The lack of an _GPL does not make them usable by non GPL code, it merely indicates that its perhaps more likely that you could in some cases shown something was not a derivative work.
The licensing is determined by the license document, and that very clearly says the boundary is derivative works. The rest is just vague guidance.
In addition btw please be careful when trying to do this even with non derivative code your lawyer is happy with. The kernel contains algorithms which are subject to various US software patent mess where GPL use has been granted, that may impact you if you use such code indirectly in non GPL code.
Alan
Quoted 9 lines · Alan Cox
Quoted 3 lines · On Wed, 14 Sep 2011 10:53:22 -0700 Doug Dumitru <[email protected]> wrote:
It would be great if this function were exported, but I would encourage everyone to consider exporting it generally, not just for GPL usage.
That would require a licensing change for the kernel. The kernel is a GPL work. Some of us have not licensed it in any other way.
Alan,
I agree with you, in part. What you are saying implies that no new EXPORT can ever happen. If you wrote this function, then obviously, you can choose to export it GPL only. Likewise, if you wrote this function, considering it's location in the block IO layer, I would ask that you consider exporting it generally.
This does not change the GPL nature of the kernel. It does change how non GPL modules can interact with the kernel. There are some (and I am not sure if this includes you), that believe that all commercial license kernel modules violate the GPL. Other believe that they are legal, but perhaps not desirable. Others believe there is no issue.
Again, my request was just a polite request to consider the sub-system when deciding on whether an EXPORT or EXPORT_GPL was more appropriate.
Quoted 7 lines
Quoted 2 lines
The kernel seems to be pretty clean that some subsystems are GPL only (things like sysfs), and some are open
The
_GPLis meant to make it clear they are internal symbols. The lack of an_GPLdoes not make them usable by non GPL code, it merely indicates that its perhaps more likely that you could in some cases shown something was not a derivative work.
I don't think this is true, at least from a practical point. If you develop a commercial module, you can link to the kernel through public symbols. These symbols, and the kernels header files, define a public API that is usable from commercial modules. Provided that the techniques used inside of the commercial module were not "derived from" the rest of the kernel, using the API is acceptable.
When a symbol is not exported publicly, then, pretty much by definition, it is not a part of the public API. This creates kernel functionality that is unreachable from a commercial module, regardless of whether the commercial module is derived or not. As such, the EXPORT_GPL has the effect of being even more stringent than the GPL license itself. With an EXPORT_GPL, the author is saying that this API is not available to commercial modules even if the commercial software is not a derivative work.
Quoted 3 lines
The licensing is determined by the license document, and that very clearly says the boundary is derivative works. The rest is just vague guidance.
I am very careful about what is a derivative work. I am also careful about honoring author's "desires" as to how stuff gets exported. Right now, the block IO layer has a pretty clean, published, API with adequate public symbols to get real work done, either from GPL or commercial licensed modules. If useful functions are walled off, then this does not change the derivative nature, but just degrades the usefulness of the kernel. This would be like exporting kmalloc() as an open symbol, but exporting kfree() as GPL only.
I guess another way of saying this is that if you are creating a new entry into an existing, published API, and the API keeps caller/called at "arms length" (which the block IO layer does), then it is possible to create a work that uses the API but is not a derivative, and hopefully the new symbol will be declared accessible to everyone.
Quoted 5 lines
In addition btw please be careful when trying to do this even with non derivative code your lawyer is happy with. The kernel contains algorithms which are subject to various US software patent mess where GPL use has been granted, that may impact you if you use such code indirectly in non GPL code.
Understood, but, in all fairness, the US patent mess is far worse and I suspect that breathing violates someones claim.
Quoted 1 line
Alan
Again, I was not trying to create a contentious issue. I was just asking that new exports consider what is around them, and try to maintain some consistency of GPL vs open exports. A function that submits a block IO request seems to be a good candidate for an open API which implies an open export.
Signature · 2 lines
Doug Dumitru EasyCo LLC
Quoted 2 lines
I agree with you, in part. What you are saying implies that no new EXPORT can ever happen. If you wrote this function, then obviously,
No.
Quoted 3 lines
you can choose to export it GPL only. Likewise, if you wrote this function, considering it's location in the block IO layer, I would ask that you consider exporting it generally.
The kernel is GPL, any work that is derivative is subject to the licence, any work which is not is generally not subject to the licence.
Quoted 3 lines
I don't think this is true, at least from a practical point. If you develop a commercial module, you can link to the kernel through public symbols. These symbols, and the kernels header files, define a public
You can create commercial modules but their licence must be GPL compatible including source etc if they are derivative in any way. At least that is what my lawyer advises me.
Possibly you mean "proprietary" here. The GPL welcomes people making money out of things, it just rejects theft of freedoms given by other parties.
Quoted 3 lines
API that is usable from commercial modules. Provided that the techniques used inside of the commercial module were not "derived from" the rest of the kernel, using the API is acceptable.
I'm not sure derivative works law is quite so clear cut, but then 'provide a clear concise definition of derivative works' appears to be the legal version of The Goldbach Conjecture.
Quoted 2 lines
When a symbol is not exported publicly, then, pretty much by definition, it is not a part of the public API. This creates kernel
Not the view of the legal advice I have
Quoted 5 lines
Again, I was not trying to create a contentious issue. I was just asking that new exports consider what is around them, and try to maintain some consistency of GPL vs open exports. A function that submits a block IO request seems to be a good candidate for an open API which implies an open export.
It's a GPL kernel, so to me the question is really moot.
I'm not trying to create contention. The reason I reply now and then to these kinds of discussions is that I've been advised by my lawyer to do so, because anyone who then produces a violating module who has seen this message either as a recipient or if it turns up in a trawl can them be subject to triple damages for wilful infringement - should it turn out that the work is violating and they do not in turn have a good legal assessment with which to show they did in fact take good care (and which of course might well happen because derivative works are such a grey fuzzy mess)
Whether it says EXPORT_SYMBOL, EXPORT_SYMBOL_GPL or RUBBERIZED_BANANA is to my understanding irrelevant. If it does turn out to be relevant however you are correct I'd make all my symbols _GPL....
Alan
Alan,
I appreciate your position and thank you for your continuing work on the Kernel.
Doug Dumitru
EasyCo LLC
Hi!
On Mit, 2011-09-14 at 14:34 -0700, Doug Dumitru wrote:
[....]
Quoted 4 lines
This does not change the GPL nature of the kernel. It does change how non GPL modules can interact with the kernel. There are some (and I am not sure if this includes you), that believe that all commercial license kernel modules violate the GPL. Other believe that they are
You wrote "commercial" but actually meant "proprietary".
And yes, there are fully GPLv2 modules out there which are "commercial" (whatever that actually means) - even in the sense that people and companies live (also) directly from it.
You may rewrite the whole mail with s/commercial/proprietary/. It is then (more) correct and IMHO very probably also more clear where you are heading.
And please don't mix license issues (what can I do with it and what not) with commercial issues (how and how much money can I make out of it) - these are two totally different and independent things.
[...]
Quoted 1 line
entry into an existing, published API, and the API keeps caller/called
Is there a "published API" apart from the pragmatic one: look into the .h (and .c) files and find it but beware, it may change with the next release?
I don't think so. And no, generated (or other) documentation does not change that.
Quoted 3 lines
at "arms length" (which the block IO layer does), then it is possible to create a work that uses the API but is not a derivative, and hopefully the new symbol will be declared accessible to everyone.
That's the big wish of the proprietary folks - have an in-kernel API which allows proprietary kernel modules.
Google for it to find lots of discussions about this topic and opinions on what's OK and what's not OK.
[...]
Quoted 3 lines
maintain some consistency of GPL vs open exports. A function that submits a block IO request seems to be a good candidate for an open API which implies an open export.
That has IMHO next to nothing to do if the module is a derivative work of the kernel or not if you are a lawyer.
And there are several companies which play this game so you might want to look how they try to handle that.
Bernd
Signature · 2 lines
Bernd Petrovitsch Email : [email protected] LUGA : http://www.luga.at
Quoted 40 lines · Boaz Harrosh
Quoted 15 lines · Christoph Hellwig
Quoted 10 lines · Jens Axboe
Quoted 6 lines
I really hate naming things different from the method they are implementing. I've tried to figure out what the point of the old
blk_make_requestis - why would we not go throughgeneric_make_requestfor this?Boaz, any idea?
I tend to agree, we could rename the existing
blk_make_request(). It could beblk_make_request_from_bio()or something like that, since that's what it does.It should at very least be renamed. But I still can't figure out what it is for exactly.
Quoted 7 lines
There are three users:
(1)
virtio_blk::virtblk_get_id():
This looks like it really should just useblk_rq_map_kern.
(2)osd_initiator::_make_request():
This one looks like it should just use the same scheme assg_io(), as it's doing the same thing.Good god what
sg_io? That broken pointr+length from user-mode thatsg.candbsg.care using? no can do it's not user-mode pointers, and it's not pointer+length it's pages pointers of a bio. The only other structure that could carry the same information is struct sg, but we work very hard to get rid of this contraption. (scsi_execute_asyncor something that it was)
blk_make_request()was made to be the parallel of__make_request, to be used from filesystem level users. But with two differences.
1. Mainly support for none-FSBLOCK_PCrequests
2. Also support chained bios. (was added later)Quoted 3 lines
(3)
target_core_pscsi::__pscsi_map_SG():Same as (2).
Quoted 8 lines
There is no better suitable structure in current Kernel to carry a list of pages, with optional offset and length, then bio struct. Given a bio at hand. how do you make a block request out of it? (If it's not an
FS_PCtype IO?)As I remember
target_corehad their own pages-linked-list structure, and how do you make a request out of that? again best at hand is bio.
FYI, for v3.1-rc code, target core has been converted to use native struct scatterlist for everything, and the legacy struct se_mem that existed before the advent of modern >= .24 SGL linking, et al. has been completely removed.
--nab
[PATCH 3/2] block: refactor generic_make_request
In reply to Christoph HellwigMove all the checks performed on a bio into a new helper, and call it as soon as bio is submitted even if it is a re-submission from ->make_request.
We explicitly mark the new helper as beeing non-inlined as the stack usage for printing the block device name in the failure case is quite high and this a patch where we have to be extremely conservative about stack usage.
block/blk-core.c +47 -44
@@ -1412,31 +1412,8 @@ static inline int bio_check_eod(struct breturn 0 ; } -/** -* generic_make_request - hand a buffer to its device driver for I/O -* @bio: The bio describing the location in memory and on the device. -* -* generic_make_request() is used to make I/O requests of block -* devices. It is passed a &struct bio, which describes the I/O that needs -* to be done. -* -* generic_make_request() does not return any status. The -* success/failure status of the request, along with notification of -* completion, is delivered asynchronously through the bio->bi_end_io -* function described (one day) else where. -* -* The caller of generic_make_request must make sure that bi_io_vec -* are set to describe the memory buffer, and that bi_dev and bi_sector are -* set to describe the device address, and the -* bi_end_io and optionally bi_private are set to describe how -* completion notification should be signaled. -* -* generic_make_request and the drivers it calls may use bi_next if this -* bio happens to be merged with someone else, and may change bi_dev and -* bi_sector for remaps as it sees fit. So the values of these fields -* should NOT be depended on after the call to generic_make_request. -*/ -static inline void __generic_make_request (struct bio *bio) +static noinline_for_stack bool +generic_make_request_checks (struct bio *bio) {struct request_queue *q;int nr_sectors =bio_sectors (bio);
@@ -1515,35 +1492,62 @@ static inline void __generic_make_reques/* if bio = NULL, bio has been throttled and will be submitted later. */ if (!bio) -return ; +return false; +trace_block_bio_queue (q, bio); - q->make_request_fn (q, bio); -return ; +return true; end_io:bio_endio (bio, err); +return false; } -/* -* We only want one ->make_request_fn to be active at a time, -* else stack usage with stacked devices could be a problem. -* So use current->bio_list to keep a list of requests -* submited by a make_request_fn function. -* current->bio_list is also used as a flag to say if -* generic_make_request is currently active in this task or not. -* If it is NULL, then no make_request is active. If it is non-NULL, -* then a make_request is active, and new requests should be added -* at the tail +/** +* generic_make_request - hand a buffer to its device driver for I/O +* @bio: The bio describing the location in memory and on the device. +* +* generic_make_request() is used to make I/O requests of block +* devices. It is passed a &struct bio, which describes the I/O that needs +* to be done. +* +* generic_make_request() does not return any status. The +* success/failure status of the request, along with notification of +* completion, is delivered asynchronously through the bio->bi_end_io +* function described (one day) else where. +* +* The caller of generic_make_request must make sure that bi_io_vec +* are set to describe the memory buffer, and that bi_dev and bi_sector are +* set to describe the device address, and the +* bi_end_io and optionally bi_private are set to describe how +* completion notification should be signaled. +* +* generic_make_request and the drivers it calls may use bi_next if this +* bio happens to be merged with someone else, and may resubmit the bio to +* a lower device by calling into generic_make_request recursively, which +* means the bio should NOT be touched after the call to ->make_request_fn. */ void generic_make_request (struct bio *bio) {struct bio_list bio_list_on_stack; +if (!generic_make_request_checks (bio)) +return ; + +/* +* We only want one ->make_request_fn to be active at a time, else +* stack usage with stacked devices could be a problem. So use +* current->bio_list to keep a list of requests submited by a +* make_request_fn function. current->bio_list is also used as a +* flag to say if generic_make_request is currently active in this +* task or not. If it is NULL, then no make_request is active. If +* it is non-NULL, then a make_request is active, and new requests +* should be added at the tail +*/ if (current->bio_list) { -/* make_request is active */ bio_list_add (current->bio_list, bio);return ; } +/* following loop may be a bit non-obvious, and so deserves some * explanation. * Before entering the loop, bio->bi_next is NULL (as all callers
@@ -1557,16 +1561,15 @@ void generic_make_request(struct bio *bi* from the top. In this case we really did just take the bio * of the top of the list (no pretending) and so remove it from * bio_list, and call into __generic_make_request again. -* -* The loop was structured like this to make only one call to -* __generic_make_request (which is important as it is large and -* inlined) and to keep the structure simple. */ BUG_ON (bio->bi_next);bio_list_init (&bio_list_on_stack); current->bio_list = &bio_list_on_stack;do { -__generic_make_request (bio); +struct request_queue *q =bdev_get_queue (bio->bi_bdev); + + q->make_request_fn (q, bio); + bio =bio_list_pop (current->bio_list); }while (bio); current->bio_list =NULL ;/* deactivate */
Re: [PATCH 3/2] block: refactor generic_make_request
On Mon, Sep 12, 2011 at 09:42:02AM -0400, Christoph Hellwig wrote:
[..]
block/blk-core.c void generic_make_request(struct bio *bi · line 1563
Quoted 8 lines
block/blk-core.c +1
-0+/* following loop may be a bit non-obvious, and so deserves some * explanation. * Before entering the loop, bio->bi_next is NULL (as all callers @@ -1557,16 +1561,15 @@ void generic_make_request(struct bio *bi* from the top. In this case we really did just take the bio * of the top of the list (no pretending) and so remove it from * bio_list, and call into __generic_make_request again.
^^^^^^^^^^
You got rid of __generic_make_request(). Now above comment has stale reference to it.
Thanks
Vivek
Re: [PATCH 3/2] block: refactor generic_make_request
Quoted 12 lines · Vivek Goyal
On Mon, Sep 12, 2011 at 09:42:02AM -0400, Christoph Hellwig wrote:
[..]
block/blk-core.c void generic_make_request(struct bio *bi · line 1563
Quoted 8 lines
block/blk-core.c +1
-0+/* following loop may be a bit non-obvious, and so deserves some * explanation. * Before entering the loop, bio->bi_next is NULL (as all callers @@ -1557,16 +1561,15 @@ void generic_make_request(struct bio *bi* from the top. In this case we really did just take the bio * of the top of the list (no pretending) and so remove it from * bio_list, and call into __generic_make_request again. ^^^^^^^^^^
Quoted 2 lines
You got rid of
__generic_make_request(). Now above comment has stale reference to it.
Yeah, this should be ->make_request now.
Re: [PATCH 3/2] block: refactor generic_make_request
Quoted 9 lines · Christoph Hellwig
Quoted 7 lines
block/blk-core.c void generic_make_request(struct bio *bi · line 1563
Quoted 4 lines
block/blk-core.c +0
-0@@ -1557,16 +1561,15 @@ void generic_make_request(struct bio *bi* from the top. In this case we really did just take the bio * of the top of the list (no pretending) and so remove it from * bio_list, and call into __generic_make_request again. ^^^^^^^^^^You got rid of
__generic_make_request(). Now above comment has stale reference to it.Yeah, this should be ->make_request now.
Jens, do you want me to resend with that fix or can you fix it up when applying?
Jens Axboe
maintainerRe: [PATCH 3/2] block: refactor generic_make_request
Quoted 9 lines · Christoph Hellwig
Quoted 8 lines · Christoph Hellwig
Quoted 7 lines
block/blk-core.c void generic_make_request(struct bio *bi · line 1563
Quoted 4 lines
block/blk-core.c +0
-0@@ -1557,16 +1561,15 @@ void generic_make_request(struct bio *bi* from the top. In this case we really did just take the bio * of the top of the list (no pretending) and so remove it from * bio_list, and call into __generic_make_request again. ^^^^^^^^^^You got rid of
__generic_make_request(). Now above comment has stale reference to it.
Quoted 4 lines
Quoted 1 line
Yeah, this should be ->make_request now.
Jens, do you want me to resend with that fix or can you fix it up when applying?
I'll just fix it up, applying it now.
Signature · 1 line
Jens Axboe