[PATCH] 2.5.67-ac2 direct-IO for IDE taskfile ioctl (0/4)

[PATCH] 2.5.67-ac2 direct-IO for IDE taskfile ioctl (0/4)

[Bartlomiej Zolnierkiewicz]

Hey,

Another bunch of patches:

(1) Enhance bio_(un)map_user() and add blk_rq_bio_prep().
(2) Pass bdev to IDE ioctl handlers.
(3) Add support for rq->bio based taskfile.
(4) Use direct-IO in ide_taskfile_ioctl() and in ide_cmd_ioctl().

[ more detailed changelogs inside patches ]

They are incremental to 2.5.67-ac1/2 and previously posted tf-ioctls patches,
you can find all patches at:
	http://home.elka.pw.edu.pl/~bzolnier/patches/2.5.67-ac2/


Now HDIO_DRIVE_TASKFILE and HDIO_DRIVE_CMD (taskfile version) ioctls
use direct-IO to user memory if it is possible (user memory buffer address
and transfer length must be both aligned to hardsector size = 512).

As a result ioctl generated IO request with aligned user buffer use
the same code path as fs generated IO request, which gives possibility
of testing IDE code used for fs-requests from user space.

These patches also make possible to use taskfile ioctl for up to 32 MB big
lba48 requests, since now we don't need to allocate kernel buffer for them.
[ There may be still some small glitches to fix. ]

Alignment of user buffer address is a limitation to removing code using
kernel buffer approach. If user buffer is not aligned it can can happen
that one hardware sector is mapped to diffirent bio-s.
[ However I have an idea how to deal with this issue. :-) ]


I have tested HDIO_DRIVE_TASKFILE ioctl after changes and both direct-IO
and normal transfers are working fine, here are results from DiskPerf:

# with direct-IO
./DiskPerf /dev/hda

Device: WDC WD800JB-00CRA1 Serial Number: WD-WMAxxxxxxxxx
LBA 0 DMA Read Test                      = 78.34 MB/Sec (3.19 Seconds)
Outer Diameter Sequential DMA Read Test  = 45.52 MB/Sec (5.49 Seconds)
Inner Diameter Sequential DMA Read Test  = 25.91 MB/Sec (9.65 Seconds)

# with kernel buffer
./DiskPerf /dev/hda

Device: WDC WD800JB-00CRA1 Serial Number: WD-WMAxxxxxxxxx
LBA 0 DMA Read Test                      = 69.81 MB/Sec (3.58 Seconds)
Outer Diameter Sequential DMA Read Test  = 44.83 MB/Sec (5.58 Seconds)
Inner Diameter Sequential DMA Read Test  = 25.94 MB/Sec (9.64 Seconds)


Example how to align user buffer for HDIO_DRIVE_TASKFILE and direct-IO:

with kernel buffer:
	ide_task_request_t reqtask;
	unsigned char task[sizeof(reqtask)+reqtask.out_size+reqtask.in_size];

	and &task were used as ioctl argument
direct-IO:
	#define HARDSECTOR_SIZE	512
	#define ALIGN(x,a)	(((x)+(a)-1)&~((a)-1))
	#define TASK_ALIGN(x)	(ALIGN((unsigned long)(x), HARDSECTOR_SIZE) \
				 +HARDSECTOR_SIZE-sizeof(ide_task_request_t))

	ide_task_request_t reqtask;
	unsigned char task[sizeof(reqtask)+reqtask.out_size+reqtask.in_size
			   +2*HARDSECTOR_SIZE];
	unsigned char *taskptr = (unsigned char *)TASK_ALIGN(task);

	and use taskptr as ioctl argument

	[ Yes, I know it is ugly ]

--
Bartlomiej Zolnierkiewicz

[PATCH] 2.5.67-ac2 direct-IO for IDE taskfile ioctl (1/4)

[Bartlomiej Zolnierkiewicz]

# Enhance bio_(un)map_user() and add blk_rq_bio_prep().
#
# Detailed changelog:
# - add blk_rq_bio_prep() helper for preparing non-fs bio based requests
# - bio_(un)map_user() -> __bio_(un)map_user()
# - add bio_(un)map_user() wrappers for __bio_(un)map_user(),
#   bio_map_user() checks size of bio returned by __bio_map_user()
#   and gets additional reference to bio (see comment inside)
# - update sg_io() to use new bio_(un)map_user() and blk_rq_bio_prep()
#
# Bartlomiej Zolnierkiewicz <bzolnier@elka.pw.edu.pl>

diff -uNr linux-2.5.67-ac2-tf4/drivers/block/ll_rw_blk.c linux/drivers/block/ll_rw_blk.c
--- linux-2.5.67-ac2-tf4/drivers/block/ll_rw_blk.c	Wed Apr 23 15:14:11 2003
+++ linux/drivers/block/ll_rw_blk.c	Wed Apr 23 15:19:27 2003
@@ -2274,6 +2274,21 @@
 	return 1;
 }

+void blk_rq_bio_prep(request_queue_t *q, struct request *rq, struct bio *bio)
+{
+	/* first three bits are identical in rq->flags and bio->bi_rw */
+	rq->flags |= (bio->bi_rw & 7);
+
+	rq->nr_phys_segments = bio_phys_segments(q, bio);
+	rq->nr_hw_segments = bio_hw_segments(q, bio);
+	rq->current_nr_sectors = bio_cur_sectors(bio);
+	rq->hard_cur_sectors = rq->current_nr_sectors;
+	rq->hard_nr_sectors = rq->nr_sectors = bio_sectors(bio);
+	rq->buffer = bio_data(bio);
+
+	rq->hard_bio = rq->bio = rq->biotail = bio;
+}
+
 int __init blk_dev_init(void)
 {
 	int total_ram = nr_free_pages() << (PAGE_SHIFT - 10);
@@ -2363,3 +2378,5 @@
 EXPORT_SYMBOL(__blk_stop_queue);
 EXPORT_SYMBOL(__blk_run_queue);
 EXPORT_SYMBOL(blk_run_queues);
+
+EXPORT_SYMBOL(blk_rq_bio_prep);
diff -uNr linux-2.5.67-ac2-tf4/drivers/block/scsi_ioctl.c linux/drivers/block/scsi_ioctl.c
--- linux-2.5.67-ac2-tf4/drivers/block/scsi_ioctl.c	Sun Apr 13 00:07:39 2003
+++ linux/drivers/block/scsi_ioctl.c	Tue Apr 22 17:57:22 2003
@@ -193,18 +193,8 @@
 		 * be a write to vm.
 		 */
 		bio = bio_map_user(bdev, uaddr, hdr.dxfer_len, reading);
-		if (bio) {
-			if (writing)
-				bio->bi_rw |= (1 << BIO_RW);
-
-			nr_sectors = (bio->bi_size + 511) >> 9;
-
-			if (bio->bi_size < hdr.dxfer_len) {
-				bio_endio(bio, bio->bi_size, 0);
-				bio_unmap_user(bio, 0);
-				bio = NULL;
-			}
-		}
+		if (bio && writing)
+			bio->bi_rw |= (1 << BIO_RW);

 		/*
 		 * if bio setup failed, fall back to slow approach
@@ -243,21 +233,10 @@
 	rq->hard_nr_sectors = rq->nr_sectors = nr_sectors;
 	rq->hard_cur_sectors = rq->current_nr_sectors = nr_sectors;

-	if (bio) {
-		/*
-		 * subtle -- if bio_map_user() ended up bouncing a bio, it
-		 * would normally disappear when its bi_end_io is run.
-		 * however, we need it for the unmap, so grab an extra
-		 * reference to it
-		 */
-		bio_get(bio);
+	rq->hard_bio = rq->bio = rq->biotail = bio;

-		rq->nr_phys_segments = bio_phys_segments(q, bio);
-		rq->nr_hw_segments = bio_hw_segments(q, bio);
-		rq->current_nr_sectors = bio_cur_sectors(bio);
-		rq->hard_cur_sectors = rq->current_nr_sectors;
-		rq->buffer = bio_data(bio);
-	}
+	if (bio)
+		blk_rq_bio_prep(q, rq, bio);

 	rq->data_len = hdr.dxfer_len;
 	rq->data = buffer;
@@ -268,8 +247,6 @@
 	if (!rq->timeout)
 		rq->timeout = BLK_DEFAULT_TIMEOUT;

-	rq->bio = rq->biotail = bio;
-
 	start_time = jiffies;

 	/* ignore return value. All information is passed back to caller
@@ -277,11 +254,9 @@
 	 * N.B. a non-zero SCSI status is _not_ necessarily an error.
 	 */
 	blk_do_rq(q, bdev, rq);
-
-	if (bio) {
+
+	if (bio)
 		bio_unmap_user(bio, reading);
-		bio_put(bio);
-	}

 	/* write to all output members */
 	hdr.status = rq->errors;
diff -uNr linux-2.5.67-ac2-tf4/fs/bio.c linux/fs/bio.c
--- linux-2.5.67-ac2-tf4/fs/bio.c	Thu Apr 17 17:46:02 2003
+++ linux/fs/bio.c	Tue Apr 22 17:55:03 2003
@@ -434,19 +434,9 @@
 	return len;
 }

-/**
- *	bio_map_user	-	map user address into bio
- *	@bdev: destination block device
- *	@uaddr: start of user address
- *	@len: length in bytes
- *	@write_to_vm: bool indicating writing to pages or not
- *
- *	Map the user space address into a bio suitable for io to a block
- *	device. Caller should check the size of the returned bio, we might
- *	not have mapped the entire range specified.
- */
-struct bio *bio_map_user(struct block_device *bdev, unsigned long uaddr,
-			 unsigned int len, int write_to_vm)
+static struct bio *__bio_map_user(struct block_device *bdev,
+				  unsigned long uaddr, unsigned int len,
+				  int write_to_vm)
 {
 	unsigned long end = (uaddr + len + PAGE_SIZE - 1) >> PAGE_SHIFT;
 	unsigned long start = uaddr >> PAGE_SHIFT;
@@ -521,17 +511,42 @@
 }

 /**
- *	bio_unmap_user	-	unmap a bio
- *	@bio:		the bio being unmapped
- *	@write_to_vm:	bool indicating whether pages were written to
- *
- *	Unmap a bio previously mapped by bio_map_user(). The @write_to_vm
- *	must be the same as passed into bio_map_user(). Must be called with
- *	a process context.
+ *	bio_map_user	-	map user address into bio
+ *	@bdev: destination block device
+ *	@uaddr: start of user address
+ *	@len: length in bytes
+ *	@write_to_vm: bool indicating writing to pages or not
  *
- *	bio_unmap_user() may sleep.
+ *	Map the user space address into a bio suitable for io to a block
+ *	device.
  */
-void bio_unmap_user(struct bio *bio, int write_to_vm)
+struct bio *bio_map_user(struct block_device *bdev, unsigned long uaddr,
+			 unsigned int len, int write_to_vm)
+{
+	struct bio *bio;
+
+	bio = __bio_map_user(bdev, uaddr, len, write_to_vm);
+
+	if (bio) {
+		if (bio->bi_size < len) {
+			bio_endio(bio, bio->bi_size, 0);
+			bio_unmap_user(bio, 0);
+			return NULL;
+		}
+
+		/*
+		 * subtle -- if __bio_map_user() ended up bouncing a bio,
+		 * it would normally disappear when its bi_end_io is run.
+		 * however, we need it for the unmap, so grab an extra
+		 * reference to it
+		 */
+		bio_get(bio);
+	}
+
+	return bio;
+}
+
+static void __bio_unmap_user(struct bio *bio, int write_to_vm)
 {
 	struct bio_vec *bvec;
 	int i;
@@ -561,6 +576,23 @@
 	bio_put(bio);
 }

+/**
+ *	bio_unmap_user	-	unmap a bio
+ *	@bio:		the bio being unmapped
+ *	@write_to_vm:	bool indicating whether pages were written to
+ *
+ *	Unmap a bio previously mapped by bio_map_user(). The @write_to_vm
+ *	must be the same as passed into bio_map_user(). Must be called with
+ *	a process context.
+ *
+ *	bio_unmap_user() may sleep.
+ */
+void bio_unmap_user(struct bio *bio, int write_to_vm)
+{
+	__bio_unmap_user(bio, write_to_vm);
+	bio_put(bio);
+}
+
 /*
  * bio_set_pages_dirty() and bio_check_pages_dirty() are support functions
  * for performing direct-IO in BIOs.
diff -uNr linux-2.5.67-ac2-tf4/include/linux/blkdev.h linux/include/linux/blkdev.h
--- linux-2.5.67-ac2-tf4/include/linux/blkdev.h	Fri Apr 18 02:03:38 2003
+++ linux/include/linux/blkdev.h	Tue Apr 22 17:48:52 2003
@@ -432,6 +432,8 @@
 extern void blk_queue_invalidate_tags(request_queue_t *);
 extern void blk_congestion_wait(int rw, long timeout);

+extern void blk_rq_bio_prep(request_queue_t *, struct request *, struct bio *);
+
 #define MAX_PHYS_SEGMENTS 128
 #define MAX_HW_SEGMENTS 128
 #define MAX_SECTORS 255

Re: [PATCH] 2.5.67-ac2 direct-IO for IDE taskfile ioctl (1/4)

[Jens Axboe]

On Wed, Apr 23 2003, Bartlomiej Zolnierkiewicz wrote:
>  		bio = bio_map_user(bdev, uaddr, hdr.dxfer_len, reading);
> -		if (bio) {
> -			if (writing)
> -				bio->bi_rw |= (1 << BIO_RW);
> -
> -			nr_sectors = (bio->bi_size + 511) >> 9;
> -
> -			if (bio->bi_size < hdr.dxfer_len) {
> -				bio_endio(bio, bio->bi_size, 0);
> -				bio_unmap_user(bio, 0);
> -				bio = NULL;
> -			}
> -		}
> +		if (bio && writing)
> +			bio->bi_rw |= (1 << BIO_RW);

I think it's cleaner to have bio_map_user() set the direction bit,
instead of having every user do it. It also uncovered an old bug where
blk_queue_bounce() is called without the direction bit set in the bio,
uh oh...

Here's my preferred version. You also had the incremental bio processing
as a prereq for this bio addition, I removed that for now as well.

# This is a BitKeeper generated patch for the following project:
# Project Name: Linux kernel tree
# This patch format is intended for GNU patch command version 2.5 or higher.
# This patch includes the following deltas:
#	           ChangeSet	1.1216  -> 1.1217 
#	drivers/block/ll_rw_blk.c	1.167   -> 1.168  
#	include/linux/blkdev.h	1.100   -> 1.101  
#	            fs/bio.c	1.41    -> 1.42   
#	drivers/block/scsi_ioctl.c	1.22    -> 1.23   
#
# The following is the BitKeeper ChangeSet Log
# --------------------------------------------
# 03/04/24	axboe@smithers.home.kernel.dk	1.1217
# - Abstract out bio request preparation
# - Have bio_map_user() set data direction (fixes bug where blk_queue_bounce()
#   is called without it set)
# - Split bio_map_user() in two
# --------------------------------------------
#
diff -Nru a/drivers/block/ll_rw_blk.c b/drivers/block/ll_rw_blk.c
--- a/drivers/block/ll_rw_blk.c	Thu Apr 24 10:23:09 2003
+++ b/drivers/block/ll_rw_blk.c	Thu Apr 24 10:23:09 2003
@@ -2196,6 +2196,21 @@
 	}
 }
 
+void blk_rq_bio_prep(request_queue_t *q, struct request *rq, struct bio *bio)
+{
+	/* first three bits are identical in rq->flags and bio->bi_rw */
+	rq->flags |= (bio->bi_rw & 7);
+
+	rq->nr_phys_segments = bio_phys_segments(q, bio);
+	rq->nr_hw_segments = bio_hw_segments(q, bio);
+	rq->current_nr_sectors = bio_cur_sectors(bio);
+	rq->hard_cur_sectors = rq->current_nr_sectors;
+	rq->hard_nr_sectors = rq->nr_sectors = bio_sectors(bio);
+	rq->buffer = bio_data(bio);
+
+	rq->bio = rq->biotail = bio;
+}
+
 int __init blk_dev_init(void)
 {
 	int total_ram = nr_free_pages() << (PAGE_SHIFT - 10);
@@ -2285,3 +2300,5 @@
 EXPORT_SYMBOL(__blk_stop_queue);
 EXPORT_SYMBOL(blk_run_queue);
 EXPORT_SYMBOL(blk_run_queues);
+
+EXPORT_SYMBOL(blk_rq_bio_prep);
diff -Nru a/drivers/block/scsi_ioctl.c b/drivers/block/scsi_ioctl.c
--- a/drivers/block/scsi_ioctl.c	Thu Apr 24 10:23:09 2003
+++ b/drivers/block/scsi_ioctl.c	Thu Apr 24 10:23:09 2003
@@ -193,18 +193,6 @@
 		 * be a write to vm.
 		 */
 		bio = bio_map_user(bdev, uaddr, hdr.dxfer_len, reading);
-		if (bio) {
-			if (writing)
-				bio->bi_rw |= (1 << BIO_RW);
-
-			nr_sectors = (bio->bi_size + 511) >> 9;
-
-			if (bio->bi_size < hdr.dxfer_len) {
-				bio_endio(bio, bio->bi_size, 0);
-				bio_unmap_user(bio, 0);
-				bio = NULL;
-			}
-		}
 
 		/*
 		 * if bio setup failed, fall back to slow approach
@@ -243,21 +231,10 @@
 	rq->hard_nr_sectors = rq->nr_sectors = nr_sectors;
 	rq->hard_cur_sectors = rq->current_nr_sectors = nr_sectors;
 
-	if (bio) {
-		/*
-		 * subtle -- if bio_map_user() ended up bouncing a bio, it
-		 * would normally disappear when its bi_end_io is run.
-		 * however, we need it for the unmap, so grab an extra
-		 * reference to it
-		 */
-		bio_get(bio);
+	rq->bio = rq->biotail = bio;
 
-		rq->nr_phys_segments = bio_phys_segments(q, bio);
-		rq->nr_hw_segments = bio_hw_segments(q, bio);
-		rq->current_nr_sectors = bio_cur_sectors(bio);
-		rq->hard_cur_sectors = rq->current_nr_sectors;
-		rq->buffer = bio_data(bio);
-	}
+	if (bio)
+		blk_rq_bio_prep(q, rq, bio);
 
 	rq->data_len = hdr.dxfer_len;
 	rq->data = buffer;
@@ -268,8 +245,6 @@
 	if (!rq->timeout)
 		rq->timeout = BLK_DEFAULT_TIMEOUT;
 
-	rq->bio = rq->biotail = bio;
-
 	start_time = jiffies;
 
 	/* ignore return value. All information is passed back to caller
@@ -277,11 +252,9 @@
 	 * N.B. a non-zero SCSI status is _not_ necessarily an error.
 	 */
 	blk_do_rq(q, bdev, rq);
-	
-	if (bio) {
+
+	if (bio)
 		bio_unmap_user(bio, reading);
-		bio_put(bio);
-	}
 
 	/* write to all output members */
 	hdr.status = rq->errors;	
diff -Nru a/fs/bio.c b/fs/bio.c
--- a/fs/bio.c	Thu Apr 24 10:23:09 2003
+++ b/fs/bio.c	Thu Apr 24 10:23:09 2003
@@ -434,19 +434,9 @@
 	return len;
 }
 
-/**
- *	bio_map_user	-	map user address into bio
- *	@bdev: destination block device
- *	@uaddr: start of user address
- *	@len: length in bytes
- *	@write_to_vm: bool indicating writing to pages or not
- *
- *	Map the user space address into a bio suitable for io to a block
- *	device. Caller should check the size of the returned bio, we might
- *	not have mapped the entire range specified.
- */
-struct bio *bio_map_user(struct block_device *bdev, unsigned long uaddr,
-			 unsigned int len, int write_to_vm)
+static struct bio *__bio_map_user(struct block_device *bdev,
+				  unsigned long uaddr, unsigned int len,
+				  int write_to_vm)
 {
 	unsigned long end = (uaddr + len + PAGE_SIZE - 1) >> PAGE_SHIFT;
 	unsigned long start = uaddr >> PAGE_SHIFT;
@@ -510,8 +500,11 @@
 	kfree(pages);
 
 	/*
-	 * check if the mapped pages need bouncing for an isa host.
+	 * set data direction, and check if mapped pages need bouncing
 	 */
+	if (!write_to_vm)
+		bio->bi_rw |= (1 << BIO_RW);
+
 	blk_queue_bounce(q, &bio);
 	return bio;
 out:
@@ -521,17 +514,42 @@
 }
 
 /**
- *	bio_unmap_user	-	unmap a bio
- *	@bio:		the bio being unmapped
- *	@write_to_vm:	bool indicating whether pages were written to
- *
- *	Unmap a bio previously mapped by bio_map_user(). The @write_to_vm
- *	must be the same as passed into bio_map_user(). Must be called with
- *	a process context.
+ *	bio_map_user	-	map user address into bio
+ *	@bdev: destination block device
+ *	@uaddr: start of user address
+ *	@len: length in bytes
+ *	@write_to_vm: bool indicating writing to pages or not
  *
- *	bio_unmap_user() may sleep.
+ *	Map the user space address into a bio suitable for io to a block
+ *	device.
  */
-void bio_unmap_user(struct bio *bio, int write_to_vm)
+struct bio *bio_map_user(struct block_device *bdev, unsigned long uaddr,
+			 unsigned int len, int write_to_vm)
+{
+	struct bio *bio;
+
+	bio = __bio_map_user(bdev, uaddr, len, write_to_vm);
+
+	if (bio) {
+		if (bio->bi_size < len) {
+			bio_endio(bio, bio->bi_size, 0);
+			bio_unmap_user(bio, 0);
+			return NULL;
+		}
+
+		/*
+		 * subtle -- if __bio_map_user() ended up bouncing a bio,
+		 * it would normally disappear when its bi_end_io is run.
+		 * however, we need it for the unmap, so grab an extra
+		 * reference to it
+		 */
+		bio_get(bio);
+	}
+
+	return bio;
+}
+
+static void __bio_unmap_user(struct bio *bio, int write_to_vm)
 {
 	struct bio_vec *bvec;
 	int i;
@@ -558,6 +576,23 @@
 		page_cache_release(bvec->bv_page);
 	}
 
+	bio_put(bio);
+}
+
+/**
+ *	bio_unmap_user	-	unmap a bio
+ *	@bio:		the bio being unmapped
+ *	@write_to_vm:	bool indicating whether pages were written to
+ *
+ *	Unmap a bio previously mapped by bio_map_user(). The @write_to_vm
+ *	must be the same as passed into bio_map_user(). Must be called with
+ *	a process context.
+ *
+ *	bio_unmap_user() may sleep.
+ */
+void bio_unmap_user(struct bio *bio, int write_to_vm)
+{
+	__bio_unmap_user(bio, write_to_vm);
 	bio_put(bio);
 }
 
diff -Nru a/include/linux/blkdev.h b/include/linux/blkdev.h
--- a/include/linux/blkdev.h	Thu Apr 24 10:23:09 2003
+++ b/include/linux/blkdev.h	Thu Apr 24 10:23:09 2003
@@ -391,6 +391,8 @@
 extern void blk_queue_invalidate_tags(request_queue_t *);
 extern void blk_congestion_wait(int rw, long timeout);
 
+extern void blk_rq_bio_prep(request_queue_t *, struct request *, struct bio *);
+
 #define MAX_PHYS_SEGMENTS 128
 #define MAX_HW_SEGMENTS 128
 #define MAX_SECTORS 255

-- 
Jens Axboe

Re: [PATCH] 2.5.67-ac2 direct-IO for IDE taskfile ioctl (1/4)

[Bartlomiej Zolnierkiewicz]

On Thu, 24 Apr 2003, Jens Axboe wrote:

> On Wed, Apr 23 2003, Bartlomiej Zolnierkiewicz wrote:
> >  		bio = bio_map_user(bdev, uaddr, hdr.dxfer_len, reading);
> > -		if (bio) {
> > -			if (writing)
> > -				bio->bi_rw |= (1 << BIO_RW);
> > -
> > -			nr_sectors = (bio->bi_size + 511) >> 9;
> > -
> > -			if (bio->bi_size < hdr.dxfer_len) {
> > -				bio_endio(bio, bio->bi_size, 0);
> > -				bio_unmap_user(bio, 0);
> > -				bio = NULL;
> > -			}
> > -		}
> > +		if (bio && writing)
> > +			bio->bi_rw |= (1 << BIO_RW);
>
> I think it's cleaner to have bio_map_user() set the direction bit,
> instead of having every user do it. It also uncovered an old bug where

You are right.

> blk_queue_bounce() is called without the direction bit set in the bio,
> uh oh...

Yep.

> Here's my preferred version. You also had the incremental bio processing
> as a prereq for this bio addition, I removed that for now as well.

Patch is okay.

> # The following is the BitKeeper ChangeSet Log
> # --------------------------------------------
> # 03/04/24	axboe@smithers.home.kernel.dk	1.1217
> # - Abstract out bio request preparation
> # - Have bio_map_user() set data direction (fixes bug where blk_queue_bounce()
> #   is called without it set)
> # - Split bio_map_user() in two
> # --------------------------------------------

Very minor issue, last comment is a bit misleading...

--
Bartlomiej Zolnierkiewicz

[PATCH] 2.5.68 fix mismatched access_ok() checks in sg_io()

[Bartlomiej Zolnierkiewicz]

I've found this while doing bio_map_user() changes,
please forward to Linus.

--
Bartlomiej

--- linux-2.5.68/drivers/block/scsi_ioctl.c	Sun Apr 20 04:51:16 2003
+++ linux/drivers/block/scsi_ioctl.c	Thu Apr 24 17:36:15 2003
@@ -183,9 +183,11 @@
 		}

 		uaddr = (unsigned long) hdr.dxferp;
-		if (writing && !access_ok(VERIFY_WRITE, uaddr, bytes))
+		/* writing to device -> reading from vm */
+		if (writing && !access_ok(VERIFY_READ, uaddr, bytes))
 			return -EFAULT;
-		else if (reading && !access_ok(VERIFY_READ, uaddr, bytes))
+		/* reading from device -> writing to vm */
+		else if (reading && !access_ok(VERIFY_WRITE, uaddr, bytes))
 			return -EFAULT;

 		/*

Re: [PATCH] 2.5.68 fix mismatched access_ok() checks in sg_io()

[Jens Axboe]

On Thu, Apr 24 2003, Bartlomiej Zolnierkiewicz wrote:
> 
> I've found this while doing bio_map_user() changes,
> please forward to Linus.

Oh yes you are right, I'll fold that in with the changes. Thanks!

-- 
Jens Axboe

[PATCH] 2.5.67-ac2 direct-IO for IDE taskfile ioctl (2/4)

[Bartlomiej Zolnierkiewicz]

# Pass bdev to IDE ioctl handlers.
#
# Pass bdev to ide_cmd_ioctl(), ide_task_ioctl() and ide_taskfile_ioctl()
# from generic_ide_ioctl().
#
# Bartlomiej Zolnierkiewicz <bzolnier@elka.pw.edu.pl>

diff -uNr linux-2.5.67-ac2-dtf1/drivers/ide/ide-taskfile.c linux/drivers/ide/ide-taskfile.c
--- linux-2.5.67-ac2-dtf1/drivers/ide/ide-taskfile.c	Fri Apr 18 02:04:28 2003
+++ linux/drivers/ide/ide-taskfile.c	Tue Apr 22 18:43:32 2003
@@ -1056,7 +1056,8 @@

 #define MAX_DMA		(256*SECTOR_WORDS)

-int ide_taskfile_ioctl (ide_drive_t *drive, unsigned int cmd, unsigned long arg)
+int ide_taskfile_ioctl (struct block_device *bdev, ide_drive_t *drive,
+			unsigned int cmd, unsigned long arg)
 {
 	ide_task_request_t	*req_task;
 	ide_task_t		args;
@@ -1256,7 +1257,8 @@
 /*
  * FIXME : this needs to map into at taskfile. <andre@linux-ide.org>
  */
-int ide_cmd_ioctl (ide_drive_t *drive, unsigned int cmd, unsigned long arg)
+int ide_cmd_ioctl (struct block_device *bdev, ide_drive_t *drive,
+		   unsigned int cmd, unsigned long arg)
 {
 #ifndef CONFIG_IDE_TASKFILE_IO
 	int err = 0;
@@ -1400,7 +1402,8 @@
 /*
  * FIXME : this needs to map into at taskfile. <andre@linux-ide.org>
  */
-int ide_task_ioctl (ide_drive_t *drive, unsigned int cmd, unsigned long arg)
+int ide_task_ioctl (struct block_device *bdev, ide_drive_t *drive,
+		    unsigned int cmd, unsigned long arg)
 {
 	int err = 0;
 	u8 args[7], *argbuf = args;
@@ -1544,7 +1547,8 @@

 #ifdef CONFIG_PKT_TASK_IOCTL

-int pkt_taskfile_ioctl (ide_drive_t *drive, unsigned int cmd, unsigned long arg)
+int pkt_taskfile_ioctl (struct block_device *bdev, ide_drive_t *drive,
+			unsigned int cmd, unsigned long arg)
 {
 #if 0
 	switch(req_task->data_phase) {
diff -uNr linux-2.5.67-ac2-dtf1/drivers/ide/ide.c linux/drivers/ide/ide.c
--- linux-2.5.67-ac2-dtf1/drivers/ide/ide.c	Wed Apr 23 15:14:10 2003
+++ linux/drivers/ide/ide.c	Wed Apr 23 15:25:13 2003
@@ -1533,29 +1533,47 @@
 		case HDIO_DRIVE_TASKFILE:
 		        if (!capable(CAP_SYS_ADMIN) || !capable(CAP_SYS_RAWIO))
 				return -EACCES;
+			err = bd_claim(bdev, current);
+			if (err)
+				return err;
 			switch(drive->media) {
 				case ide_disk:
-					return ide_taskfile_ioctl(drive, cmd, arg);
+					err = ide_taskfile_ioctl(bdev, drive, cmd, arg);
+					break;
 #ifdef CONFIG_PKT_TASK_IOCTL
 				case ide_cdrom:
 				case ide_tape:
 				case ide_floppy:
-					return pkt_taskfile_ioctl(drive, cmd, arg);
+					err = pkt_taskfile_ioctl(bdev, drive, cmd, arg);
+					break;
 #endif /* CONFIG_PKT_TASK_IOCTL */
 				default:
-					return -ENOMSG;
+					err = -ENOMSG;
+					break;
 			}
+			bd_release(bdev);
+			return err;
 #endif /* CONFIG_IDE_TASK_IOCTL */

 		case HDIO_DRIVE_CMD:
 			if (!capable(CAP_SYS_ADMIN) || !capable(CAP_SYS_RAWIO))
 				return -EACCES;
-			return ide_cmd_ioctl(drive, cmd, arg);
+			err = bd_claim(bdev, current);
+			if (err)
+				return err;
+			err = ide_cmd_ioctl(bdev, drive, cmd, arg);
+			bd_release(bdev);
+			return err;

 		case HDIO_DRIVE_TASK:
 			if (!capable(CAP_SYS_ADMIN) || !capable(CAP_SYS_RAWIO))
 				return -EACCES;
-			return ide_task_ioctl(drive, cmd, arg);
+			err = bd_claim(bdev, current);
+			if (err)
+				return err;
+			err = ide_task_ioctl(bdev, drive, cmd, arg);
+			bd_release(bdev);
+			return err;

 		case HDIO_SCAN_HWIF:
 		{
diff -uNr linux-2.5.67-ac2-dtf1/include/linux/ide.h linux/include/linux/ide.h
--- linux-2.5.67-ac2-dtf1/include/linux/ide.h	Fri Apr 18 02:04:12 2003
+++ linux/include/linux/ide.h	Tue Apr 22 18:45:56 2003
@@ -1477,9 +1477,9 @@
 /* Expects args is a full set of TF registers and parses the command type */
 extern int ide_cmd_type_parser(ide_task_t *);

-int ide_taskfile_ioctl(ide_drive_t *, unsigned int, unsigned long);
-int ide_cmd_ioctl(ide_drive_t *, unsigned int, unsigned long);
-int ide_task_ioctl(ide_drive_t *, unsigned int, unsigned long);
+int ide_taskfile_ioctl(struct block_device *, ide_drive_t *, unsigned int, unsigned long);
+int ide_cmd_ioctl(struct block_device *, ide_drive_t *, unsigned int, unsigned long);
+int ide_task_ioctl(struct block_device *, ide_drive_t *, unsigned int, unsigned long);

 #if 0

[PATCH] 2.5.67-ac2 direct-IO for IDE taskfile ioctl (3/4)

[Bartlomiej Zolnierkiewicz]

# Add support for rq->bio based taskfile.
#
# Detailed changelog:
# - use ide_build_sglist() also for rq->bio based REQ_DRIVE_TASKFILE
#   in ide_build_dmatable(), plus similar changes for ARM and PPC
# - add support for REQ_DRIVE_TASKFILE to ide_end_request() and PIO handlers
# - add support for REQ_DRIVE_TASKFILE to ide_dma_intr() and icside_dmaintr()
#
# Bartlomiej Zolnierkiewicz <bzolnier@elka.pw.edu.pl>

diff -uNr linux-2.5.67-ac2-dtf2/drivers/ide/arm/icside.c linux/drivers/ide/arm/icside.c
--- linux-2.5.67-ac2-dtf2/drivers/ide/arm/icside.c	Wed Apr 23 15:14:07 2003
+++ linux/drivers/ide/arm/icside.c	Wed Apr 23 15:32:56 2003
@@ -276,7 +276,8 @@

 	BUG_ON(hwif->sg_dma_active);

-	if (rq->flags & REQ_DRIVE_TASKFILE) {
+	/* rq->buffer based taskfile */
+	if ((rq->flags & REQ_DRIVE_TASKFILE) && !rq->bio) {
 		ide_task_t *args = rq->special;

 		if (args->command_type == IDE_DRIVE_TASK_RAW_WRITE)
@@ -573,13 +574,10 @@
 	if (OK_STAT(stat, DRIVE_READY, drive->bad_wstat | DRQ_STAT)) {
 		if (!dma_stat) {
 			struct request *rq = HWGROUP(drive)->rq;
-			int i;
-
-			for (i = rq->nr_sectors; i > 0; ) {
-				i -= rq->current_nr_sectors;
-				DRIVER(drive)->end_request(drive, 1, rq->nr_sectors);
-			}

+			DRIVER(drive)->end_request(drive, 1, rq->nr_sectors);
+			if (rq->flags & REQ_DRIVE_TASKFILE)
+				ide_end_drive_cmd(drive, stat, HWIF(drive)->INB(IDE_ERROR_REG));
 			return ide_stopped;
 		}
 		printk(KERN_ERR "%s: bad DMA status (dma_stat=%x)\n",
diff -uNr linux-2.5.67-ac2-dtf2/drivers/ide/ppc/pmac.c linux/drivers/ide/ppc/pmac.c
--- linux-2.5.67-ac2-dtf2/drivers/ide/ppc/pmac.c	Wed Apr 23 15:14:07 2003
+++ linux/drivers/ide/ppc/pmac.c	Wed Apr 23 15:39:28 2003
@@ -1010,7 +1010,9 @@
 		udelay(1);

 	/* Build sglist */
-	if (HWGROUP(drive)->rq->flags & REQ_DRIVE_TASKFILE)
+
+	/* rq->buffer based taskfile */
+	if ((HWGROUP(drive)->rq->flags & REQ_DRIVE_TASKFILE) && !rq->bio)
 		pmif->sg_nents = i = pmac_ide_raw_build_sglist(drive, rq);
 	else
 		pmif->sg_nents = i = pmac_ide_build_sglist(drive, rq);
diff -uNr linux-2.5.67-ac2-dtf2/drivers/ide/ide-dma.c linux/drivers/ide/ide-dma.c
--- linux-2.5.67-ac2-dtf2/drivers/ide/ide-dma.c	Wed Apr 23 15:14:07 2003
+++ linux/drivers/ide/ide-dma.c	Wed Apr 23 15:35:27 2003
@@ -180,6 +180,8 @@
 			struct request *rq = HWGROUP(drive)->rq;

 			DRIVER(drive)->end_request(drive, 1, rq->nr_sectors);
+			if (rq->flags & REQ_DRIVE_TASKFILE)
+				ide_end_drive_cmd(drive, stat, HWIF(drive)->INB(IDE_ERROR_REG));
 			return ide_stopped;
 		}
 		printk(KERN_ERR "%s: dma_intr: bad DMA status (dma_stat=%x)\n",
@@ -303,7 +305,8 @@
 	int i;
 	struct scatterlist *sg;

-	if (HWGROUP(drive)->rq->flags & REQ_DRIVE_TASKFILE)
+	/* rq->buffer based taskfile */
+	if ((HWGROUP(drive)->rq->flags & REQ_DRIVE_TASKFILE) && !rq->bio)
 		hwif->sg_nents = i = ide_raw_build_sglist(drive, rq);
 	else
 		hwif->sg_nents = i = ide_build_sglist(drive, rq);
diff -uNr linux-2.5.67-ac2-dtf2/drivers/ide/ide-io.c linux/drivers/ide/ide-io.c
--- linux-2.5.67-ac2-dtf2/drivers/ide/ide-io.c	Tue Apr 22 22:34:34 2003
+++ linux/drivers/ide/ide-io.c	Wed Apr 23 00:27:43 2003
@@ -124,6 +124,12 @@
 	}

 	if (!end_that_request_first(rq, uptodate, nr_sectors)) {
+
+		if (rq->flags & REQ_DRIVE_TASKFILE) {
+			spin_unlock_irqrestore(&ide_lock, flags);
+			return 0;
+		}
+
 		add_disk_randomness(rq->rq_disk);
 		if (!blk_rq_tagged(rq))
 			blkdev_dequeue_request(rq);
diff -uNr linux-2.5.67-ac2-dtf2/drivers/ide/ide-taskfile.c linux/drivers/ide/ide-taskfile.c
--- linux-2.5.67-ac2-dtf2/drivers/ide/ide-taskfile.c	Wed Apr 23 00:19:19 2003
+++ linux/drivers/ide/ide-taskfile.c	Wed Apr 23 00:21:34 2003
@@ -419,8 +419,11 @@
 	 * Status was already verifyied.
 	 */
 	while (rq->hard_bio != rq->bio)
-		if (!DRIVER(drive)->end_request(drive, 1, bio_sectors(rq->hard_bio)))
+		if (!DRIVER(drive)->end_request(drive, 1, bio_sectors(rq->hard_bio))) {
+			if (rq->flags & REQ_DRIVE_TASKFILE)
+				ide_end_drive_cmd(drive, stat, HWIF(drive)->INB(IDE_ERROR_REG));
 			return ide_stopped;
+		}
 	/* Complete rq->buffer based request (ioctls). */
 	if (!rq->bio && !rq->nr_sectors) {
 		ide_end_drive_cmd(drive, stat, HWIF(drive)->INB(IDE_ERROR_REG));
@@ -476,8 +479,11 @@
 	 * Status was already verifyied.
 	 */
 	while (rq->hard_bio != rq->bio)
-		if (!DRIVER(drive)->end_request(drive, 1, bio_sectors(rq->hard_bio)))
+		if (!DRIVER(drive)->end_request(drive, 1, bio_sectors(rq->hard_bio))) {
+			if (rq->flags & REQ_DRIVE_TASKFILE)
+				ide_end_drive_cmd(drive, stat, HWIF(drive)->INB(IDE_ERROR_REG));
 			return ide_stopped;
+		}
 	/* Complete rq->buffer based request (ioctls). */
 	if (!rq->bio && !rq->nr_sectors) {
 		ide_end_drive_cmd(drive, stat, HWIF(drive)->INB(IDE_ERROR_REG));
@@ -549,8 +555,11 @@
 	 * Status was already verifyied.
 	 */
 	while (rq->hard_bio != rq->bio)
-		if (!DRIVER(drive)->end_request(drive, 1, bio_sectors(rq->hard_bio)))
+		if (!DRIVER(drive)->end_request(drive, 1, bio_sectors(rq->hard_bio))) {
+			if (rq->flags & REQ_DRIVE_TASKFILE)
+				ide_end_drive_cmd(drive, stat, HWIF(drive)->INB(IDE_ERROR_REG));
 			return ide_stopped;
+		}
 	/* Complete rq->buffer based request (ioctls). */
 	if (!rq->bio && !rq->nr_sectors) {
 		ide_end_drive_cmd(drive, stat, HWIF(drive)->INB(IDE_ERROR_REG));
@@ -618,8 +627,11 @@
 	 * Status was already verifyied.
 	 */
 	while (rq->hard_bio != rq->bio)
-		if (!DRIVER(drive)->end_request(drive, 1, bio_sectors(rq->hard_bio)))
+		if (!DRIVER(drive)->end_request(drive, 1, bio_sectors(rq->hard_bio))) {
+			if (rq->flags & REQ_DRIVE_TASKFILE)
+				ide_end_drive_cmd(drive, stat, HWIF(drive)->INB(IDE_ERROR_REG));
 			return ide_stopped;
+		}
 	/* Complete rq->buffer based request (ioctls). */
 	if (!rq->bio && !rq->nr_sectors) {
 		ide_end_drive_cmd(drive, stat, HWIF(drive)->INB(IDE_ERROR_REG));

[PATCH] 2.5.67-ac2 direct-IO for IDE taskfile ioctl (4/4)

[Bartlomiej Zolnierkiewicz]

# Use direct-IO in ide_taskfile_ioctl() and in ide_cmd_ioctl().
#
# Detailed changelog:
# - add ide_bio_taskfile() which is equivalent to ide_diag_taskfile()
#   for non-fs rq->bio based requests
# - use direct-IO to user memory if possible in ide_taskfile_ioctl()
#   and in taskfile version of ide_cmd_ioctl()
#
# NOTE: user memory out/in buffers have to be hardsector (=512) aligned
#	to take advantage of direct-IO
#
# Bartlomiej Zolnierkiewicz <bzolnier@elka.pw.edu.pl>

diff -uNr linux-2.5.67-ac2-dtf3/drivers/ide/ide-taskfile.c linux/drivers/ide/ide-taskfile.c
--- linux-2.5.67-ac2-dtf3/drivers/ide/ide-taskfile.c	Wed Apr 23 15:44:36 2003
+++ linux/drivers/ide/ide-taskfile.c	Wed Apr 23 16:22:12 2003
@@ -1054,6 +1054,25 @@

 EXPORT_SYMBOL(ide_raw_taskfile);

+/*
+ * Comments to ide_diag_taskfile() apply here as well.
+ */
+static int ide_bio_taskfile (ide_drive_t *drive, ide_task_t *args,
+			     request_queue_t *q, struct bio *bio)
+{
+	struct request rq;
+
+	ide_init_drive_taskfile(&rq);
+	blk_rq_bio_prep(q, &rq, bio);
+
+	if (args->tf_out_flags.all == 0)
+		args->posthandler = ide_post_handler_parser(
+				(struct hd_drive_task_hdr *) args->tfRegister,
+				(struct hd_drive_hob_hdr *) args->hobRegister);
+	rq.special = args;
+	return ide_do_drive_cmd(drive, &rq, ide_wait);
+}
+
 #ifdef CONFIG_IDE_TASK_IOCTL_DEBUG
 char * ide_ioctl_verbose (unsigned int cmd)
 {
@@ -1081,10 +1100,19 @@
 	int tasksize		= sizeof(struct ide_task_request_s);
 	int taskin		= 0;
 	int taskout		= 0;
+
+	unsigned long outaddr, inaddr;
+	request_queue_t *q;
+	struct bio *outbio = NULL, *inbio = NULL;
+
 	u8 io_32bit		= drive->io_32bit;

 //	printk("IDE Taskfile ...\n");

+	q = bdev_get_queue(bdev);
+	if (!q)
+		return -ENXIO;
+
 	req_task = kmalloc(tasksize, GFP_KERNEL);
 	if (req_task == NULL) return -ENOMEM;
 	memset(req_task, 0, tasksize);
@@ -1097,7 +1125,21 @@
 	taskin  = (int) req_task->in_size;

 	if (taskout) {
+		outaddr = arg + tasksize;
+		/* writing to device -> reading from vm */
+		if (!access_ok(VERIFY_READ, (void *)outaddr, taskout)) {
+			kfree(req_task);
+			return -EFAULT;
+		}
+		outbio = bio_map_user(bdev, outaddr, taskout, 0);
+		if (outbio)
+			outbio->bi_rw |= (1 << BIO_RW);
+	}
+
+	if (taskout && !outbio) {
 		int outtotal = tasksize;
+		printk(KERN_INFO "%s: %s: taking OUT slow-path\n",
+				 drive->name, __FUNCTION__);
 		outbuf = kmalloc(taskout, GFP_KERNEL);
 		if (outbuf == NULL) {
 			err = -ENOMEM;
@@ -1111,7 +1153,19 @@
 	}

 	if (taskin) {
+		inaddr = arg + tasksize + taskout;
+		/* reading from device -> writing to vm */
+		if (!access_ok(VERIFY_WRITE, (void *)inaddr, taskin)) {
+			kfree(req_task);
+			return -EFAULT;
+		}
+		inbio = bio_map_user(bdev, inaddr, taskin, 1);
+	}
+
+	if (taskin && !inbio) {
 		int intotal = tasksize + taskout;
+		printk(KERN_INFO "%s: %s: taking IN slow-path\n",
+				 drive->name, __FUNCTION__);
 		inbuf = kmalloc(taskin, GFP_KERNEL);
 		if (inbuf == NULL) {
 			err = -ENOMEM;
@@ -1144,22 +1198,34 @@
 	switch(req_task->data_phase) {
 		case TASKFILE_OUT_DMAQ:
 		case TASKFILE_OUT_DMA:
-			err = ide_diag_taskfile(drive, &args, taskout, outbuf);
+			if (outbio)
+				err = ide_bio_taskfile(drive, &args, q, outbio);
+			else
+				err = ide_diag_taskfile(drive, &args, taskout, outbuf);
 			break;
 		case TASKFILE_IN_DMAQ:
 		case TASKFILE_IN_DMA:
-			err = ide_diag_taskfile(drive, &args, taskin, inbuf);
+			if (inbio)
+				err = ide_bio_taskfile(drive, &args, q, inbio);
+			else
+				err = ide_diag_taskfile(drive, &args, taskin, inbuf);
 			break;
 		case TASKFILE_IN_OUT:
 #if 0
 			args.prehandler = &pre_task_out_intr;
 			args.handler = &task_out_intr;
 			args.posthandler = NULL;
-			err = ide_diag_taskfile(drive, &args, taskout, outbuf);
+			if (outbio)
+				err = ide_bio_taskfile(drive, &args, q, outbio);
+			else
+				err = ide_diag_taskfile(drive, &args, taskout, outbuf);
 			args.prehandler = NULL;
 			args.handler = &task_in_intr;
 			args.posthandler = NULL;
-			err = ide_diag_taskfile(drive, &args, taskin, inbuf);
+			if (inbio)
+				err = ide_bio_taskfile(drive, &args, q, inbio);
+			else
+				err = ide_diag_taskfile(drive, &args, taskin, inbuf);
 			break;
 #else
 			err = -EFAULT;
@@ -1176,12 +1242,18 @@
 			}
 			args.prehandler = &pre_task_mulout_intr;
 			args.handler = &task_mulout_intr;
-			err = ide_diag_taskfile(drive, &args, taskout, outbuf);
+			if (outbio)
+				err = ide_bio_taskfile(drive, &args, q, outbio);
+			else
+				err = ide_diag_taskfile(drive, &args, taskout, outbuf);
 			break;
 		case TASKFILE_OUT:
 			args.prehandler = &pre_task_out_intr;
 			args.handler = &task_out_intr;
-			err = ide_diag_taskfile(drive, &args, taskout, outbuf);
+			if (outbio)
+				err = ide_bio_taskfile(drive, &args, q, outbio);
+			else
+				err = ide_diag_taskfile(drive, &args, taskout, outbuf);
 			break;
 		case TASKFILE_MULTI_IN:
 			if (!drive->mult_count) {
@@ -1193,11 +1265,17 @@
 				goto abort;
 			}
 			args.handler = &task_mulin_intr;
-			err = ide_diag_taskfile(drive, &args, taskin, inbuf);
+			if (inbio)
+				err = ide_bio_taskfile(drive, &args, q, inbio);
+			else
+				err = ide_diag_taskfile(drive, &args, taskin, inbuf);
 			break;
 		case TASKFILE_IN:
 			args.handler = &task_in_intr;
-			err = ide_diag_taskfile(drive, &args, taskin, inbuf);
+			if (inbio)
+				err = ide_bio_taskfile(drive, &args, q, inbio);
+			else
+				err = ide_diag_taskfile(drive, &args, taskin, inbuf);
 			break;
 		case TASKFILE_NO_DATA:
 			args.handler = &task_no_data_intr;
@@ -1208,6 +1286,11 @@
 			goto abort;
 	}

+	if (outbio)
+		bio_unmap_user(outbio, 0);
+	if (inbio)
+		bio_unmap_user(inbio, 1);
+
 	memcpy(req_task->io_ports, &(args.tfRegister), HDIO_DRIVE_TASK_HDR_SIZE);
 	memcpy(req_task->hob_ports, &(args.hobRegister), HDIO_DRIVE_HOB_HDR_SIZE);
 	req_task->in_flags  = args.tf_in_flags;
@@ -1217,14 +1300,14 @@
 		err = -EFAULT;
 		goto abort;
 	}
-	if (taskout) {
+	if (outbuf) {
 		int outtotal = tasksize;
 		if (copy_to_user((void *)arg+outtotal, outbuf, taskout)) {
 			err = -EFAULT;
 			goto abort;
 		}
 	}
-	if (taskin) {
+	if (inbuf) {
 		int intotal = tasksize + taskout;
 		if (copy_to_user((void *)arg+intotal, inbuf, taskin)) {
 			err = -EFAULT;
@@ -1331,6 +1414,9 @@
 	u8 xfer_rate = 0;
 	int argsize = 0;
 	ide_task_t tfargs;
+	int reading = 0, writing = 0;
+	request_queue_t *q;
+	struct bio *bio = NULL;

 	if (NULL == (void *) arg) {
 		struct request rq;
@@ -1338,6 +1424,10 @@
 		return ide_do_drive_cmd(drive, &rq, ide_wait);
 	}

+	q = bdev_get_queue(bdev);
+	if (!q)
+		return -ENXIO;
+
 	if (copy_from_user(args, (void *)arg, 4))
 		return -EFAULT;

@@ -1357,21 +1447,51 @@
 	if (drive->media != ide_disk && args[0] == WIN_IDENTIFY)
 		tfargs.tfRegister[IDE_COMMAND_OFFSET] = WIN_PIDENTIFY;

+	if (set_transfer(drive, &tfargs)) {
+		xfer_rate = args[1];
+		if (ide_ata66_check(drive, &tfargs))
+			goto abort;
+	}
+
+	tfargs.command_type = ide_cmd_type_parser(&tfargs);
+
 	if (args[3]) {
 		argsize = (SECTOR_WORDS * 4 * args[3]);
+
+		if (tfargs.command_type == IDE_DRIVE_TASK_OUT ||
+		    tfargs.command_type == IDE_DRIVE_TASK_RAW_WRITE)
+			writing = 1;
+		else if (tfargs.command_type == IDE_DRIVE_TASK_IN)
+			reading = 1;
+		else
+			return -EPERM;
+
+		if (writing && !access_ok(VERIFY_READ, arg+4, argsize))
+			return -EFAULT;
+		else if (reading && !access_ok(VERIFY_WRITE, arg+4, argsize))
+			return -EFAULT;
+
+		bio = bio_map_user(bdev, arg+4, argsize, reading);
+		if (bio && writing)
+			bio->bi_rw |= (1 << BIO_RW);
+	}
+
+	if (args[3] && !bio) {
+		printk(KERN_INFO "%s: %s: taking slow-path\n",
+				 drive->name, __FUNCTION__);
+		argsize = (SECTOR_WORDS * 4 * args[3]);
 		argbuf = kmalloc(argsize, GFP_KERNEL);
 		if (argbuf == NULL)
 			return -ENOMEM;
 	}

-	if (set_transfer(drive, &tfargs)) {
-		xfer_rate = args[1];
-		if (ide_ata66_check(drive, &tfargs))
-			goto abort;
-	}
+	if (bio)
+		err = ide_bio_taskfile(drive, &tfargs, q, bio);
+	else
+		err = ide_raw_taskfile(drive, &tfargs, argbuf);

-	tfargs.command_type = ide_cmd_type_parser(&tfargs);
-	err = ide_raw_taskfile(drive, &tfargs, argbuf);
+	if (bio)
+		bio_unmap_user(bio, reading);

 	if (!err && xfer_rate) {
 		/* active-retuning-calls future */

Re: [PATCH] 2.5.67-ac2 direct-IO for IDE taskfile ioctl (4/4)

[Jens Axboe]

On Wed, Apr 23 2003, Bartlomiej Zolnierkiewicz wrote:
> 
> diff -uNr linux-2.5.67-ac2-dtf3/drivers/ide/ide-taskfile.c linux/drivers/ide/ide-taskfile.c
> --- linux-2.5.67-ac2-dtf3/drivers/ide/ide-taskfile.c	Wed Apr 23 15:44:36 2003
> +++ linux/drivers/ide/ide-taskfile.c	Wed Apr 23 16:22:12 2003
> @@ -1054,6 +1054,25 @@
> 
>  EXPORT_SYMBOL(ide_raw_taskfile);
> 
> +/*
> + * Comments to ide_diag_taskfile() apply here as well.
> + */
> +static int ide_bio_taskfile (ide_drive_t *drive, ide_task_t *args,
> +			     request_queue_t *q, struct bio *bio)
> +{

any reason you are passing q seperately instead of using &drive->queue?

-- 
Jens Axboe

[PATCH] 2.5.67-ac2 direct-IO for IDE taskfile ioctl (0/4) [resend]

[Bartlomiej Zolnierkiewicz]

[ hmmm, it din't make through? ]

Hey,

Another bunch of patches:

(1) Enhance bio_(un)map_user() and add blk_rq_bio_prep().
(2) Pass bdev to IDE ioctl handlers.
(3) Add support for rq->bio based taskfile.
(4) Use direct-IO in ide_taskfile_ioctl() and in ide_cmd_ioctl().

[ more detailed changelogs inside patches ]

They are incremental to 2.5.67-ac1/2 and previously posted tf-ioctls patches,
you can find all patches at:
	http://home.elka.pw.edu.pl/~bzolnier/patches/2.5.67-ac2/


Now HDIO_DRIVE_TASKFILE and HDIO_DRIVE_CMD (taskfile version) ioctls
use direct-IO to user memory if it is possible (user memory buffer address
and transfer length must be both aligned to hardsector size = 512).

As a result ioctl generated IO request with aligned user buffer use
the same code path as fs generated IO request, which gives possibility
of testing IDE code used for fs-requests from user space.

These patches also make possible to use taskfile ioctl for up to 32 MB big
lba48 requests, since now we don't need to allocate kernel buffer for them.
[ There may be still some small glitches to fix. ]

Alignment of user buffer address is a limitation to removing code using
kernel buffer approach. If user buffer is not aligned it can can happen
that one hardware sector is mapped to diffirent bio-s.
[ However I have an idea how to deal with this issue. :-) ]


I have tested HDIO_DRIVE_TASKFILE ioctl after changes and both direct-IO
and normal transfers are working fine, here are results from DiskPerf:

# with direct-IO
./DiskPerf /dev/hda

Device: WDC WD800JB-00CRA1 Serial Number: WD-WMAxxxxxxxxx
LBA 0 DMA Read Test                      = 78.34 MB/Sec (3.19 Seconds)
Outer Diameter Sequential DMA Read Test  = 45.52 MB/Sec (5.49 Seconds)
Inner Diameter Sequential DMA Read Test  = 25.91 MB/Sec (9.65 Seconds)

# with kernel buffer
./DiskPerf /dev/hda

Device: WDC WD800JB-00CRA1 Serial Number: WD-WMAxxxxxxxxx
LBA 0 DMA Read Test                      = 69.81 MB/Sec (3.58 Seconds)
Outer Diameter Sequential DMA Read Test  = 44.83 MB/Sec (5.58 Seconds)
Inner Diameter Sequential DMA Read Test  = 25.94 MB/Sec (9.64 Seconds)


Example how to align user buffer for HDIO_DRIVE_TASKFILE and direct-IO:

with kernel buffer:
	ide_task_request_t reqtask;
	unsigned char task[sizeof(reqtask)+reqtask.out_size+reqtask.in_size];

	and &task were used as ioctl argument
direct-IO:
	#define HARDSECTOR_SIZE	512
	#define ALIGN(x,a)	(((x)+(a)-1)&~((a)-1))
	#define TASK_ALIGN(x)	(ALIGN((unsigned long)(x), HARDSECTOR_SIZE) \
				 +HARDSECTOR_SIZE-sizeof(ide_task_request_t))

	ide_task_request_t reqtask;
	unsigned char task[sizeof(reqtask)+reqtask.out_size+reqtask.in_size
			   +2*HARDSECTOR_SIZE];
	unsigned char *taskptr = (unsigned char *)TASK_ALIGN(task);

	and use taskptr as ioctl argument

	[ Yes, I know it is ugly ]

--
Bartlomiej Zolnierkiewicz

Re: [PATCH] 2.5.67-ac2 direct-IO for IDE taskfile ioctl (0/4)

[Andrew Morton]

Bartlomiej Zolnierkiewicz <B.Zolnierkiewicz@elka.pw.edu.pl> wrote:
>
> 
> Hey,
> 
> Another bunch of patches:
> 
> (1) Enhance bio_(un)map_user() and add blk_rq_bio_prep().
> (2) Pass bdev to IDE ioctl handlers.
> (3) Add support for rq->bio based taskfile.
> (4) Use direct-IO in ide_taskfile_ioctl() and in ide_cmd_ioctl().

Dumb question: what does all this code actually do?

It appears to be implementing an IDE-specific ioctl() which performs bulk
IO direct to/from userspace.  But that seems to be equivalent to O_DIRECT
access to /dev/hda.

What is special about the IDE ioctl approach?

Thanks.

Re: [PATCH] 2.5.67-ac2 direct-IO for IDE taskfile ioctl (0/4)

[Alan Cox]

On Mer, 2003-04-23 at 23:35, Andrew Morton wrote:
> Dumb question: what does all this code actually do?
> 
> It appears to be implementing an IDE-specific ioctl() which performs bulk
> IO direct to/from userspace.  But that seems to be equivalent to O_DIRECT
> access to /dev/hda.
> 
> What is special about the IDE ioctl approach?

Think /dev/sg at the ATA level

Re: [PATCH] 2.5.67-ac2 direct-IO for IDE taskfile ioctl (0/4)

[Bartlomiej Zolnierkiewicz]

On Wed, 23 Apr 2003, Andrew Morton wrote:

> Bartlomiej Zolnierkiewicz <B.Zolnierkiewicz@elka.pw.edu.pl> wrote:
> >
> >
> > Hey,
> >
> > Another bunch of patches:
> >
> > (1) Enhance bio_(un)map_user() and add blk_rq_bio_prep().
> > (2) Pass bdev to IDE ioctl handlers.
> > (3) Add support for rq->bio based taskfile.
> > (4) Use direct-IO in ide_taskfile_ioctl() and in ide_cmd_ioctl().
>
> Dumb question: what does all this code actually do?

Code actually implements 'subject' so direct IO to user memory
for IDE specific taskfile ioctl. As a side effect it also cleans
scsi_ioctl.c a little.

>
> It appears to be implementing an IDE-specific ioctl() which performs bulk
> IO direct to/from userspace.  But that seems to be equivalent to O_DIRECT
> access to /dev/hda.

No, it is not equivalent, it does much, much more then O_DIRECT.
And this ioctl has been here for ages, only now it gets direct IO.

> What is special about the IDE ioctl approach?
>
> Thanks.

It can be used for _any_ access to IDE drive, even for some diagnostic
or vendor specific commands. User decides on transfer protocol
(PIO/DMA/queued DMA), addressing mode (CHS/LBA28/LBA48) etc.
User also gets drive status after command completion etc.

The main goal of the patch is to bring internal kernel handling
of this ioctl closer to kernel handling of normal fs IO, so only
_one_ common code will be used in future (benefits desribed in annonce).

Hope this helps...
--
Bartlomiej

Re: [PATCH] 2.5.67-ac2 direct-IO for IDE taskfile ioctl (0/4)

[Andries Brouwer]

On Wed, Apr 23, 2003 at 03:35:00PM -0700, Andrew Morton wrote:

> What is special about the IDE ioctl approach?

Usually one wants to use the standard commands for I/O.
But if the purpose is to talk to the drive (set password,
set native max, eject, change ZIP drive from big floppy
mode to removable disk mode, etc. etc.) then one needs
a means to execute IDE commands "by hand".

Also SCSI has an ioctl for "do command by hand".

Andries

Re: [PATCH] 2.5.67-ac2 direct-IO for IDE taskfile ioctl (0/4)

[Andrew Morton]

Andries Brouwer <aebr@win.tue.nl> wrote:
>
> On Wed, Apr 23, 2003 at 03:35:00PM -0700, Andrew Morton wrote:
> 
> > What is special about the IDE ioctl approach?
> 
> Usually one wants to use the standard commands for I/O.
> But if the purpose is to talk to the drive (set password,
> set native max, eject, change ZIP drive from big floppy
> mode to removable disk mode, etc. etc.) then one needs
> a means to execute IDE commands "by hand".

Yes, but none of these are performance-critical and they don't involve
large amnounts of data.  A copy is OK.

If all the rework against bio_map_user() and friends is needed for other
reasons then fine.  But it doesn't seem to be needed for the IDE taskfile
ioctl.

Re: [PATCH] 2.5.67-ac2 direct-IO for IDE taskfile ioctl (0/4)

[Bartlomiej Zolnierkiewicz]

On Wed, 23 Apr 2003, Andrew Morton wrote:

> Andries Brouwer <aebr@win.tue.nl> wrote:
> >
> > On Wed, Apr 23, 2003 at 03:35:00PM -0700, Andrew Morton wrote:
> >
> > > What is special about the IDE ioctl approach?
> >
> > Usually one wants to use the standard commands for I/O.
> > But if the purpose is to talk to the drive (set password,
> > set native max, eject, change ZIP drive from big floppy
> > mode to removable disk mode, etc. etc.) then one needs
> > a means to execute IDE commands "by hand".
>
> Yes, but none of these are performance-critical and they don't involve
> large amnounts of data.  A copy is OK.

low-level benchmarking is somehow performance critical :-)

note that direct-IO here is _only_ bonus, main goal is to
use the same code for _all_ ioctl and fs generated IDE IO
(remember that all fs IDE IO is rq->bio based)

and this is work-in-progress...

> If all the rework against bio_map_user() and friends is needed for other
> reasons then fine.  But it doesn't seem to be needed for the IDE taskfile
> ioctl.

Rework of bio_map_user() and co. is minimal and not needed but otherwise
I will have to duplicate same code in 4 places. bio_map_user() now does
allocated bio checking and grabs extra reference to bio which all users of
old bio_map_user() have to do anyway (and they will probably forgot to).

Re: [PATCH] 2.5.67-ac2 direct-IO for IDE taskfile ioctl (0/4)

[Jens Axboe]

On Thu, Apr 24 2003, Bartlomiej Zolnierkiewicz wrote:
> > If all the rework against bio_map_user() and friends is needed for other
> > reasons then fine.  But it doesn't seem to be needed for the IDE taskfile
> > ioctl.
> 
> Rework of bio_map_user() and co. is minimal and not needed but otherwise
> I will have to duplicate same code in 4 places. bio_map_user() now does
> allocated bio checking and grabs extra reference to bio which all users of
> old bio_map_user() have to do anyway (and they will probably forgot to).

The bio_map_user() changes look perfectly fine, I'd like to commit those
separately.

>From the performance point of view, I agree that the whole thing is
rather silly (I mean, who cares?). But the non-allocating nature of
HDIO_DRIVE_TASK is a step in the right direction.

-- 
Jens Axboe

Re: [PATCH] 2.5.67-ac2 direct-IO for IDE taskfile ioctl (0/4)

[Andre Hedrick]

Does Task Management Command set help?
For those who speak FibreChannel, this has meaning.

I can be used as a means to test the depth of the protocol support for a
family of drives.  This is what www.linuxdiskcert.org was to be about, and
maybe Jens will be able to make it happen.  As I will be transfering the
domain th Jens this summer.

On Wed, 23 Apr 2003, Andrew Morton wrote:

> Andries Brouwer <aebr@win.tue.nl> wrote:
> >
> > On Wed, Apr 23, 2003 at 03:35:00PM -0700, Andrew Morton wrote:
> > 
> > > What is special about the IDE ioctl approach?
> > 
> > Usually one wants to use the standard commands for I/O.
> > But if the purpose is to talk to the drive (set password,
> > set native max, eject, change ZIP drive from big floppy
> > mode to removable disk mode, etc. etc.) then one needs
> > a means to execute IDE commands "by hand".
> 
> Yes, but none of these are performance-critical and they don't involve
> large amnounts of data.  A copy is OK.
> 
> If all the rework against bio_map_user() and friends is needed for other
> reasons then fine.  But it doesn't seem to be needed for the IDE taskfile
> ioctl.
> 

Andre Hedrick
LAD Storage Consulting Group