mv sg features to the block layer v4

PATCH In review Mike Christie 1 participant 2 messages Feb 24, 2007

Subsystems (5)

VFS SCSI Block layer Uniform cdrom driver +1 more subsystems

lore

Share & tools

lore

Mike Christie

author

2007-02-24 11:51 UTC #0 lore raw

PATCH 1/2 mv sg features to the block layer v4

v4.

mv sg.c features to the block layer helper code, so that tape, scsi_tgt, and maybe bsg or block/scsi_ioctl.c can use them.

This patches moves the sg features and converts blk_rq_map_user and bio_map_user callers to the new API.

Previously, we did blk_rq_map_user() to map or copy date to a buffer. Then called blk_rq_unmap_user to unmap or copy back data. sg and st want finer control over when to use DIO vs indirect IO, and for sg mmap we want to use the code that sets up a bio buffer which is also used by indirect IO.

Now, if the caller does not care how we transfer data they can call blk_rq_init_transfer() to setup the buffers (this does what blk_rq_map_user() did before where it would try DIO first then fall back to indirect IO) and then call blk_rq_complete_transfer() when the IO is done (this does what blk_rq_unmap_user did before). block/scsi_ioctl.c, cdrom, and bsg use these functions.

If the callers wants to try to do DIO, then they can call blk_rq_map_user() to set up the buffer. When the IO is done you can then call blk_rq_destroy_buffer(). You could also call blk_rq_complete_transfer() is just a smart wrapper.

To do indirect IO, we now have blk_rq_copy_user_iov(). When that IO is done, you then call blk_rq_uncopy_user_iov().

For sg mmap, there are some helpers:
- blk_rq_mmap - does some checks to makre sure the reserved buf is large enough.
- blk_rq_vma_nopage - traverses the reserved buffer and does get_page()

To setup and teardown the request and bio reserved buffer mappings for the sg mmap operation you call blk_rq_setup_buffer() and blk_rq_destroy_buffer().

Finally, there is a bio_reserved_buf structure, which holds mutlple segments that can be mapped into BIOs. This replaces sg's reserved buffer code, and can be used for tape (I think we need some reserved buffer growing code for that, but that should not be too difficult to add). It can also be used for scsi_tgt, so we gaurantee a certain IO size will always be executable.

One interesting suggestion from Doug and I think Christoph mentioned it, is that if we implement common behavior (when to try DIO vs indirect and supporting the sg.c features in block/scsi_ioct.c) for block/scsi_ioctl SG_IO and sg.c SG_IO, we could merge up more code and sg.c could just call something like block/scsi_ioctl.c:sg_io_write() and sg_io_read(). The synchronous block/scsi_ioctl.c:sg_io() could then call those functions too. Not sure if we want to keep them different for some reason. And one other odd item is that block/scsi_ioctl.c does DIO for iovecs, but sg.c does indirect IO for iovecs and many apps do not aling the buffers correctly so they do not use the block/scsi_ioctl.c path.

The next patch coverts sg. I did not covert st. Christoph has some patches for this. I need to update them and test them.
I wanted to make sure this API is ok before tackling tape though. I was not sure where things should go. I think maybe a new file to hold all the pass through code might be niced. I am also not sure if the reserved buffer code should be living in bio.c

Signed-off-by Mike Christie <[email protected]>

 block/bsg.c                 |    5 
 block/ll_rw_blk.c           |  457 +++++++++++++++++++++++++++++++++-----------
 block/scsi_ioctl.c          |    4 
 drivers/cdrom/cdrom.c       |    4 
 drivers/scsi/scsi_tgt_lib.c |  104 ++--------
 fs/bio.c                    |  397 +++++++++++++++++++++++++++++---------
 include/linux/bio.h         |   28 ++
 include/linux/blkdev.h      |   23 ++
 include/scsi/scsi_cmnd.h    |    3 
 9 files changed, 731 insertions(+), 294 deletions(-)

Changes since v1 - v3.
- sg iovec support. sg.c supported indirect IO iovecs, but for some reason the block layer SG IO code support direct IO iovec support.
- Move reserved buffer code to bio.c and seperate from q, so that sg and others can support mutiple reserved buffers.

block/bsg.c +3 -2

index c85d961..b4a9b14 100644
@@ -320,7 +320,7 @@ bsg_map_hdr(struct bsg_device *bd, struc
 		dxfer_len = 0;
 
 	if (dxfer_len) {
-		ret = blk_rq_map_user(q, rq, dxferp, dxfer_len);
+		ret = blk_rq_init_transfer(q, rq, dxferp, dxfer_len);
 		if (ret) {
 			dprintk("failed map at %d\n", ret);
 			blk_put_request(rq);

@@ -459,7 +459,8 @@ static int blk_complete_sgv4_hdr_rq(stru
 			ret = -EFAULT;
 	}
 
-	blk_rq_unmap_user(bio);
+	blk_rq_complete_transfer(bio, (void __user *)hdr->din_xferp,
+				 hdr->din_xfer_len);
 	blk_put_request(rq);
 
 	return ret;

block/ll_rw_blk.c +351 ~~-106~~

553 lines not on this page · raw message

block/scsi_ioctl.c +2 -2

index 18e935f..c947647 100644
@@ -244,7 +244,7 @@ EXPORT_SYMBOL_GPL(blk_fill_sghdr_rq);
  */
 int blk_unmap_sghdr_rq(struct request *rq, struct sg_io_hdr *hdr)
 {
-	blk_rq_unmap_user(rq->bio);
+	blk_rq_complete_transfer(rq->bio, hdr->dxferp, hdr->dxfer_len);
 	blk_put_request(rq);
 	return 0;
 }

@@ -348,7 +348,7 @@ static int sg_io(struct file *file, requ
 					  hdr->dxfer_len);
 		kfree(iov);
 	} else if (hdr->dxfer_len)
-		ret = blk_rq_map_user(q, rq, hdr->dxferp, hdr->dxfer_len);
+		ret = blk_rq_init_transfer(q, rq, hdr->dxferp, hdr->dxfer_len);
 
 	if (ret)
 		goto out;

drivers/cdrom/cdrom.c +2 -2

index 3105ddd..e2ec75a 100644
@@ -2118,7 +2118,7 @@ static int cdrom_read_cdda_bpc(struct cd
 
 		len = nr * CD_FRAMESIZE_RAW;
 
-		ret = blk_rq_map_user(q, rq, ubuf, len);
+		ret = blk_rq_init_transfer(q, rq, ubuf, len);
 		if (ret)
 			break;

@@ -2145,7 +2145,7 @@ static int cdrom_read_cdda_bpc(struct cd
 			cdi->last_sense = s->sense_key;
 		}
 
-		if (blk_rq_unmap_user(bio))
+		if (blk_rq_complete_transfer(bio, ubuf, len))
 			ret = -EFAULT;
 
 		if (ret)

drivers/scsi/scsi_tgt_lib.c +22 ~~-82~~

162 lines not on this page · raw message

fs/bio.c +307 ~~-90~~

508 lines not on this page · raw message

include/linux/bio.h +23 -5

index 08daf32..a14f72b 100644
@@ -51,6 +51,18 @@ #define BIO_MAX_PAGES		256
 #define BIO_MAX_SIZE		(BIO_MAX_PAGES << PAGE_CACHE_SHIFT)
 #define BIO_MAX_SECTORS		(BIO_MAX_SIZE >> 9)
 
+struct scatterlist;
+
+#define BIO_RESERVE_BUF_IN_USE 0
+
+struct bio_reserve_buf {
+	unsigned long flags;		/* state bits */
+	struct scatterlist *sg;		/* sg to hold pages */
+	unsigned buf_size;		/* size of reserve buffer */
+	int sg_count;			/* number of sg entries in use */
+	int sg_index;			/* index of sg in list */
+};
+
 /*
  * was unsigned short, but we might as well be ready for > 64kB I/O pages
  */

@@ -125,6 +137,7 @@ #define BIO_CLONED	4	/* doesn't own data
 #define BIO_BOUNCED	5	/* bio is a bounce bio */
 #define BIO_USER_MAPPED 6	/* contains user pages */
 #define BIO_EOPNOTSUPP	7	/* not supported */
+#define BIO_USED_RESERVE 8	/* using reserve buffer */
 #define bio_flagged(bio, flag)	((bio)->bi_flags & (1 << (flag)))
 
 /*

@@ -298,11 +311,15 @@ extern int bio_add_page(struct bio *, st
 extern int bio_add_pc_page(struct request_queue *, struct bio *, struct page *,
 			   unsigned int, unsigned int);
 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 int bio_free_reserve_buf(struct bio_reserve_buf *);
+extern struct bio_reserve_buf *bio_alloc_reserve_buf(struct request_queue *,
+						    unsigned long);
+extern int bio_claim_reserve_buf(struct bio_reserve_buf *, unsigned long);
+extern void bio_release_reserve_buf(struct bio_reserve_buf *);
+extern struct bio *bio_map_user(struct request_queue *, unsigned long,
+				unsigned int, int);
 struct sg_iovec;
 extern struct bio *bio_map_user_iov(struct request_queue *,
-				    struct block_device *,
 				    struct sg_iovec *, int, int);
 extern void bio_unmap_user(struct bio *);
 extern struct bio *bio_map_kern(struct request_queue *, void *, unsigned int,

@@ -310,8 +327,9 @@ extern struct bio *bio_map_kern(struct r
 extern void bio_set_pages_dirty(struct bio *bio);
 extern void bio_check_pages_dirty(struct bio *bio);
 extern void bio_release_pages(struct bio *bio);
-extern struct bio *bio_copy_user(struct request_queue *, unsigned long, unsigned int, int);
-extern int bio_uncopy_user(struct bio *);
+extern struct bio *bio_setup_user_buffer(struct request_queue *, unsigned int,
+					 int, struct bio_reserve_buf *);
+extern void bio_destroy_user_buffer(struct bio *bio);
 void zero_fill_bio(struct bio *bio);
 
 #ifdef CONFIG_HIGHMEM

include/linux/blkdev.h +21 -2

index aa000d2..a916b55 100644
@@ -358,6 +358,13 @@ struct blk_queue_tag {
 	atomic_t refcnt;		/* map can be shared */
 };
 
+struct blk_reserve_buf {
+	struct scatterlist *sg;		/* sg to hold pages */
+	unsigned buf_size;		/* size of reserve buffer */
+	int sg_count;			/* number of sg entries in use */
+	int sg_index;			/* index pf sg in list */
+};
+
 struct request_queue
 {
 	/*

@@ -677,8 +684,20 @@ extern void blk_sync_queue(struct reques
 extern void __blk_stop_queue(request_queue_t *q);
 extern void blk_run_queue(request_queue_t *);
 extern void blk_start_queueing(request_queue_t *);
-extern int blk_rq_map_user(request_queue_t *, struct request *, void __user *, unsigned long);
-extern int blk_rq_unmap_user(struct bio *);
+extern struct page *blk_rq_vma_nopage(struct bio_reserve_buf *,
+				      struct vm_area_struct *, unsigned long,
+				      int *);
+extern int blk_rq_mmap(struct bio_reserve_buf *, struct vm_area_struct *);
+extern int blk_rq_init_transfer(request_queue_t *, struct request *, void __user *, unsigned long);
+extern int blk_rq_map_user(request_queue_t *, struct request *,
+			   void __user *, unsigned long, int);
+extern int blk_rq_setup_buffer(struct request *, void __user *, unsigned long,
+				int, struct bio_reserve_buf *);
+extern void blk_rq_destroy_buffer(struct bio *);
+extern int blk_rq_copy_user_iov(struct request *, struct sg_iovec *,
+				int, unsigned long, struct bio_reserve_buf *);
+extern int blk_rq_uncopy_user_iov(struct bio *, struct sg_iovec *, int);
+extern int blk_rq_complete_transfer(struct bio *, void __user *, unsigned long);
 extern int blk_rq_map_kern(request_queue_t *, struct request *, void *, unsigned int, gfp_t);
 extern int blk_rq_map_user_iov(request_queue_t *, struct request *,
 			       struct sg_iovec *, int, unsigned int);

include/scsi/scsi_cmnd.h +0 -3

index d6948d0..a2e0c10 100644
@@ -73,9 +73,6 @@ #define MAX_COMMAND_SIZE	16
 	unsigned short use_sg;	/* Number of pieces of scatter-gather */
 	unsigned short sglist_len;	/* size of malloc'd scatter-gather list */
 
-	/* offset in cmd we are at (for multi-transfer tgt cmds) */
-	unsigned offset;
-
 	unsigned underflow;	/* Return error if less than
 				   this amount is transferred */

Mike Christie

author

2007-02-24 18:05 UTC #1 lore raw

Quoted 55 lines · Mike Christie

v4.
mv sg.c features to the block layer helper code, so that tape, scsi_tgt, and maybe bsg or block/scsi_ioctl.c can use them.
This patches moves the sg features and converts blk_rq_map_user and bio_map_user callers to the new API.
Previously, we did blk_rq_map_user() to map or copy date to a buffer. Then called blk_rq_unmap_user to unmap or copy back data. sg and st want finer control over when to use DIO vs indirect IO, and for sg mmap we want to use the code that sets up a bio buffer which is also used by indirect IO.
Now, if the caller does not care how we transfer data they can call blk_rq_init_transfer() to setup the buffers (this does what blk_rq_map_user() did before where it would try DIO first then fall back to indirect IO) and then call blk_rq_complete_transfer() when the IO is done (this does what blk_rq_unmap_user did before). block/scsi_ioctl.c, cdrom, and bsg use these functions.
If the callers wants to try to do DIO, then they can call blk_rq_map_user() to set up the buffer. When the IO is done you can then call blk_rq_destroy_buffer(). You could also call blk_rq_complete_transfer() is just a smart wrapper.
To do indirect IO, we now have blk_rq_copy_user_iov(). When that IO is done, you then call blk_rq_uncopy_user_iov().
For sg mmap, there are some helpers:
- blk_rq_mmap - does some checks to makre sure the reserved buf is large enough.
- blk_rq_vma_nopage - traverses the reserved buffer and does get_page()
To setup and teardown the request and bio reserved buffer mappings for the sg mmap operation you call blk_rq_setup_buffer() and blk_rq_destroy_buffer().
Finally, there is a bio_reserved_buf structure, which holds mutlple segments that can be mapped into BIOs. This replaces sg's reserved buffer code, and can be used for tape (I think we need some reserved buffer growing code for that, but that should not be too difficult to add). It can also be used for scsi_tgt, so we gaurantee a certain IO size will always be executable.
One interesting suggestion from Doug and I think Christoph mentioned it, is that if we implement common behavior (when to try DIO vs indirect and supporting the sg.c features in block/scsi_ioct.c) for block/scsi_ioctl SG_IO and sg.c SG_IO, we could merge up more code and sg.c could just call something like block/scsi_ioctl.c:sg_io_write() and sg_io_read(). The synchronous block/scsi_ioctl.c:sg_io() could then call those functions too. Not sure if we want to keep them different for some reason. And one other odd item is that block/scsi_ioctl.c does DIO for iovecs, but sg.c does indirect IO for iovecs and many apps do not aling the buffers correctly so they do not use the block/scsi_ioctl.c path.

Quoted 9 lines

The next patch coverts sg. I did not covert st. Christoph has some patches for this. I need to update them and test them.
I wanted to make sure this API is ok before tackling tape though. I was not sure where things should go. I think maybe a new file to hold all the pass through code might be niced. I am also not sure if the reserved buffer code should be living in bio.c
Signed-off-by Mike Christie <[email protected]>

Here is a patch that compiles. Sorry about that. I goofed and merged two patches into one and retested the wrong patch before sending.