Skip to contentSkip to conversation

bio_copy_user

PATCH New Jens Axboe 1 participant 1 message Jul 7, 2004

Subsystems (3)
lore
Share & tools
lore

Jens Axboe

author
#0 lore raw

PATCH 1/1 bio_copy_user

Hi,

blk_rq_map_user() is a bit of a hack currently, since it drops back to kmalloc() of bio_map_user() fails. This is unfortunate since it means we do no real segment or size checking (and the request segment counts contain crap, already found one bug in a scsi lld). It's also pretty nasty for > PAGE_SIZE requests, as we attempt to do higher order page allocations. Even worse still, ide-cd will drop back to pio for non-sg/bio requests. All in all, very suboptimal.

This patch adds bio_copy_user() which simply sets up a bio with kernel pages and copies data as needed for reads and writes. It also changes bio_map_user() to return an error pointer like bio_copy_user(), so we can return something sane to the user instead of always -ENOMEM.

Against 2.6.7-mm6.

drivers/block/ll_rw_blk.c renamed from opt/kernel/linux-2.6.7-mm6/drivers/block/ll_rw_blk.c +34 -44
diff -urp -X /home/axboe/exclude /opt/kernel/linux-2.6.7-mm6/drivers/block/ll_rw_blk.c linux-2.6.7-mm6/drivers/block/ll_rw_blk.c
@@ -1857,50 +1857,43 @@ EXPORT_SYMBOL(blk_insert_request);
 struct request *blk_rq_map_user(request_queue_t *q, int rw, void __user *ubuf,
 				unsigned int len)
 {
-	struct request *rq = NULL;
-	char *buf = NULL;
+	unsigned long uaddr;
+	struct request *rq;
 	struct bio *bio;
-	int ret;
+
+	if (len > (q->max_sectors << 9))
+		return ERR_PTR(-EINVAL);
+	if ((!len && ubuf) || (len && !ubuf))
+		return ERR_PTR(-EINVAL);
 
 	rq = blk_get_request(q, rw, __GFP_WAIT);
 	if (!rq)
 		return ERR_PTR(-ENOMEM);
 
-	bio = bio_map_user(q, NULL, (unsigned long) ubuf, len, rw == READ);
-	if (!bio) {
-		int bytes = (len + 511) & ~511;
-
-		buf = kmalloc(bytes, q->bounce_gfp | GFP_USER);
-		if (!buf) {
-			ret = -ENOMEM;
-			goto fault;
-		}
-
-		if (rw == WRITE) {
-			if (copy_from_user(buf, ubuf, len)) {
-				ret = -EFAULT;
-				goto fault;
-			}
-		} else
-			memset(buf, 0, len);
-	}
+	/*
+	 * if alignment requirement is satisfied, map in user pages for
+	 * direct dma. else, set up kernel bounce buffers
+	 */
+	uaddr = (unsigned long) ubuf;
+	if (!(uaddr & queue_dma_alignment(q)) && !(len & queue_dma_alignment(q)))
+		bio = bio_map_user(q, NULL, uaddr, len, rw == READ);
+	else
+		bio = bio_copy_user(q, uaddr, len, rw == READ);
 
-	rq->bio = rq->biotail = bio;
-	if (rq->bio)
+	if (!IS_ERR(bio)) {
+		rq->bio = rq->biotail = bio;
 		blk_rq_bio_prep(q, rq, bio);
 
-	rq->buffer = rq->data = buf;
-	rq->data_len = len;
-	return rq;
-fault:
-	if (buf)
-		kfree(buf);
-	if (bio)
-		bio_unmap_user(bio, 1);
-	if (rq)
-		blk_put_request(rq);
-
-	return ERR_PTR(ret);
+		rq->buffer = rq->data = NULL;
+		rq->data_len = len;
+		return rq;
+	}
+	
+	/*
+	 * bio is the err-ptr
+	 */
+	blk_put_request(rq);
+	return (struct request *) bio;
 }
 
 EXPORT_SYMBOL(blk_rq_map_user);
@@ -1914,18 +1907,15 @@ EXPORT_SYMBOL(blk_rq_map_user);
  * Description:
  *    Unmap a request previously mapped by blk_rq_map_user().
  */
-int blk_rq_unmap_user(struct request *rq, void __user *ubuf, struct bio *bio,
-		      unsigned int ulen)
+int blk_rq_unmap_user(struct request *rq, struct bio *bio, unsigned int ulen)
 {
-	const int read = rq_data_dir(rq) == READ;
 	int ret = 0;
 
-	if (bio)
-		bio_unmap_user(bio, read);
-	if (rq->buffer) {
-		if (read && copy_to_user(ubuf, rq->buffer, ulen))
-			ret = -EFAULT;
-		kfree(rq->buffer);
+	if (bio) {
+		if (bio_flagged(bio, BIO_USER_MAPPED))
+			bio_unmap_user(bio);
+		else
+			ret = bio_uncopy_user(bio);
 	}
 
 	blk_put_request(rq);
drivers/block/scsi_ioctl.c renamed from opt/kernel/linux-2.6.7-mm6/drivers/block/scsi_ioctl.c +1 -1
diff -urp -X /home/axboe/exclude /opt/kernel/linux-2.6.7-mm6/drivers/block/scsi_ioctl.c linux-2.6.7-mm6/drivers/block/scsi_ioctl.c
@@ -204,7 +204,7 @@ static int sg_io(request_queue_t *q, str
 			hdr->sb_len_wr = len;
 	}
 
-	if (blk_rq_unmap_user(rq, hdr->dxferp, bio, hdr->dxfer_len))
+	if (blk_rq_unmap_user(rq, bio, hdr->dxfer_len))
 		return -EFAULT;
 
 	/* may not have succeeded, but output values written to control
drivers/cdrom/cdrom.c renamed from opt/kernel/linux-2.6.7-mm6/drivers/cdrom/cdrom.c +1 -1
diff -urp -X /home/axboe/exclude /opt/kernel/linux-2.6.7-mm6/drivers/cdrom/cdrom.c linux-2.6.7-mm6/drivers/cdrom/cdrom.c
@@ -2088,7 +2088,7 @@ static int cdrom_read_cdda_bpc(struct cd
 			cdi->last_sense = s->sense_key;
 		}
 
-		if (blk_rq_unmap_user(rq, ubuf, bio, len))
+		if (blk_rq_unmap_user(rq, bio, len))
 			ret = -EFAULT;
 
 		if (ret)
drivers/ide/ide-cd.c renamed from opt/kernel/linux-2.6.7-mm6/drivers/ide/ide-cd.c +1 -6
diff -urp -X /home/axboe/exclude /opt/kernel/linux-2.6.7-mm6/drivers/ide/ide-cd.c linux-2.6.7-mm6/drivers/ide/ide-cd.c
@@ -1969,11 +1969,6 @@ static ide_startstop_t cdrom_do_block_pc
 	 * sg request
 	 */
 	if (rq->bio) {
-		if (rq->data_len & 3) {
-			printk("%s: block pc not aligned, len=%d\n", drive->name, rq->data_len);
-			cdrom_end_request(drive, 0);
-			return ide_stopped;
-		}
 		info->dma = drive->using_dma;
 		info->cmd = rq_data_dir(rq);
 	}
@@ -3145,7 +3140,7 @@ int ide_cdrom_setup (ide_drive_t *drive)
 	int nslots;
 
 	blk_queue_prep_rq(drive->queue, ide_cdrom_prep_fn);
-	blk_queue_dma_alignment(drive->queue, 3);
+	blk_queue_dma_alignment(drive->queue, 15);
 	drive->queue->unplug_delay = (1 * HZ) / 1000;
 	if (!drive->queue->unplug_delay)
 		drive->queue->unplug_delay = 1;
fs/bio.c renamed from opt/kernel/linux-2.6.7-mm6/fs/bio.c +140 -26

230 lines not on this page · raw message

include/linux/bio.h renamed from opt/kernel/linux-2.6.7-mm6/include/linux/bio.h +4 -1
diff -urp -X /home/axboe/exclude /opt/kernel/linux-2.6.7-mm6/include/linux/bio.h linux-2.6.7-mm6/include/linux/bio.h
@@ -121,6 +121,7 @@ struct bio {
 #define BIO_CLONED	4	/* doesn't own data */
 #define BIO_BOUNCED	5	/* bio is a bounce bio */
 #define BIO_EOPNOTSUPP	6	/* not supported */
+#define BIO_USER_MAPPED 7	/* contains user pages */
 #define bio_flagged(bio, flag)	((bio)->bi_flags & (1 << (flag)))
 
 /*
@@ -267,9 +268,11 @@ extern int bio_add_page(struct bio *, st
 extern int bio_get_nr_vecs(struct block_device *);
 extern struct bio *bio_map_user(struct request_queue *, struct block_device *,
 				unsigned long, unsigned int, int);
-extern void bio_unmap_user(struct bio *, int);
+extern void bio_unmap_user(struct bio *);
 extern void bio_set_pages_dirty(struct bio *bio);
 extern void bio_check_pages_dirty(struct bio *bio);
+extern struct bio *bio_copy_user(struct request_queue *, unsigned long, unsigned int, int);
+extern int bio_uncopy_user(struct bio *);
 
 #ifdef CONFIG_HIGHMEM
 /*
include/linux/blkdev.h renamed from opt/kernel/linux-2.6.7-mm6/include/linux/blkdev.h +1 -1
diff -urp -X /home/axboe/exclude /opt/kernel/linux-2.6.7-mm6/include/linux/blkdev.h linux-2.6.7-mm6/include/linux/blkdev.h
@@ -535,7 +535,7 @@ extern void __blk_stop_queue(request_que
 extern void blk_run_queue(request_queue_t *);
 extern void blk_queue_activity_fn(request_queue_t *, activity_fn *, void *);
 extern struct request *blk_rq_map_user(request_queue_t *, int, void __user *, unsigned int);
-extern int blk_rq_unmap_user(struct request *, void __user *, struct bio *, unsigned int);
+extern int blk_rq_unmap_user(struct request *, struct bio *, unsigned int);
 extern int blk_execute_rq(request_queue_t *, struct gendisk *, struct request *);
 
 static inline request_queue_t *bdev_get_queue(struct block_device *bdev)
Signature · 1 line
Jens Axboe