[PATCH, scsi-block-2.6] remove bio_map_user

[PATCH, scsi-block-2.6] remove bio_map_user

[Christoph Hellwig]

It's only used by blk_rq_map_user and that's the only sensible caller.
So do the iov wrapping there and kill bio_map_user.


Index: linux-2.6/drivers/block/ll_rw_blk.c
===================================================================
--- linux-2.6.orig/drivers/block/ll_rw_blk.c	2005-06-18 17:15:17.000000000 +0200
+++ linux-2.6/drivers/block/ll_rw_blk.c	2005-06-18 17:26:55.000000000 +0200
@@ -2126,9 +2126,10 @@
 	 * 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, reading);
-	else
+	if (!(uaddr & queue_dma_alignment(q)) && !(len & queue_dma_alignment(q))) {
+		struct sg_iovec iov = { .iov_base = ubuf, .iov_len = len };
+		bio = bio_map_user_iov(q, NULL, &iov, 1, reading);
+	} else
 		bio = bio_copy_user(q, uaddr, len, reading);
 
 	if (!IS_ERR(bio)) {
Index: linux-2.6/fs/bio.c
===================================================================
--- linux-2.6.orig/fs/bio.c	2005-06-18 17:15:21.000000000 +0200
+++ linux-2.6/fs/bio.c	2005-06-18 17:26:42.000000000 +0200
@@ -662,28 +662,6 @@
 }
 
 /**
- *	bio_map_user	-	map user address into bio
- *	@q: the request_queue_t for the 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. Returns an error pointer in case of error.
- */
-struct bio *bio_map_user(request_queue_t *q, struct block_device *bdev,
-			 unsigned long uaddr, unsigned int len, int write_to_vm)
-{
-	struct sg_iovec iov;
-
-	iov.iov_base = (__user void *)uaddr;
-	iov.iov_len = len;
-
-	return bio_map_user_iov(q, bdev, &iov, 1, write_to_vm);
-}
-
-/**
  *	bio_map_user_iov - map user sg_iovec table into bio
  *	@q: the request_queue_t for the bio
  *	@bdev: destination block device
@@ -1222,7 +1200,6 @@
 EXPORT_SYMBOL(bio_hw_segments);
 EXPORT_SYMBOL(bio_add_page);
 EXPORT_SYMBOL(bio_get_nr_vecs);
-EXPORT_SYMBOL(bio_map_user);
 EXPORT_SYMBOL(bio_unmap_user);
 EXPORT_SYMBOL(bio_map_kern);
 EXPORT_SYMBOL(bio_pair_release);
Index: linux-2.6/include/linux/bio.h
===================================================================
--- linux-2.6.orig/include/linux/bio.h	2005-06-18 17:15:21.000000000 +0200
+++ linux-2.6/include/linux/bio.h	2005-06-18 17:26:16.000000000 +0200
@@ -279,8 +279,6 @@
 
 extern int bio_add_page(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);
 struct sg_iovec;
 extern struct bio *bio_map_user_iov(struct request_queue *,
 				    struct block_device *,

Re: [PATCH, scsi-block-2.6] remove bio_map_user

[Jens Axboe]

On Sun, Jun 19 2005, Christoph Hellwig wrote:
> It's only used by blk_rq_map_user and that's the only sensible caller.

That's not really true, there is/are valid uses of bio_map_user() alone.

> So do the iov wrapping there and kill bio_map_user.

While I do agree with the change, I know of external users of
bio_map_user(). It's a really trivial piece of code, so I would rather
just let bio_map_user() stay as-is.


-- 
Jens Axboe

Re: [PATCH, scsi-block-2.6] remove bio_map_user

[Christoph Hellwig]

On Mon, Jun 20, 2005 at 12:37:13PM +0200, Jens Axboe wrote:
> On Sun, Jun 19 2005, Christoph Hellwig wrote:
> > It's only used by blk_rq_map_user and that's the only sensible caller.
> 
> That's not really true, there is/are valid uses of bio_map_user() alone.

That would be?

> 
> > So do the iov wrapping there and kill bio_map_user.
> 
> While I do agree with the change, I know of external users of
> bio_map_user().

Who's that?

Re: [PATCH, scsi-block-2.6] remove bio_map_user

[Jens Axboe]

On Mon, Jun 20 2005, Christoph Hellwig wrote:
> On Mon, Jun 20, 2005 at 12:37:13PM +0200, Jens Axboe wrote:
> > On Sun, Jun 19 2005, Christoph Hellwig wrote:
> > > It's only used by blk_rq_map_user and that's the only sensible caller.
> > 
> > That's not really true, there is/are valid uses of bio_map_user() alone.
> 
> That would be?

Stacking drivers or file systems.

> > 
> > > So do the iov wrapping there and kill bio_map_user.
> > 
> > While I do agree with the change, I know of external users of
> > bio_map_user().
> 
> Who's that?

ocfs2, iirc.

-- 
Jens Axboe

Re: [PATCH, scsi-block-2.6] remove bio_map_user

[Christoph Hellwig]

On Mon, Jun 20, 2005 at 12:41:14PM +0200, Jens Axboe wrote:
> > That would be?
> 
> Stacking drivers or file systems.

Doesn't make sense, a stacking driver must not deal with locking down
user memory at all, and for filesystem there's better helpers in
fs/direct-io.c

> > > While I do agree with the change, I know of external users of
> > > bio_map_user().
> > 
> > Who's that?
> 
> ocfs2, iirc.

ocfs2 doesn't use bio_map_user

Re: [PATCH, scsi-block-2.6] remove bio_map_user

[Jens Axboe]

On Mon, Jun 20 2005, Christoph Hellwig wrote:
> On Mon, Jun 20, 2005 at 12:41:14PM +0200, Jens Axboe wrote:
> > > That would be?
> > 
> > Stacking drivers or file systems.
> 
> Doesn't make sense, a stacking driver must not deal with locking down
> user memory at all, and for filesystem there's better helpers in

That's a very generic statement to make, there could be valid uses of
that. But the point of the matter is that I don't think we should just
remove bio_map_user() and intentionally break an exported API to save a
few bytes. We are not talking a lot of code, it's just a wrapper.

> fs/direct-io.c

That's true, I'll give you that.

> > > > While I do agree with the change, I know of external users of
> > > > bio_map_user().
> > > 
> > > Who's that?
> > 
> > ocfs2, iirc.
> 
> ocfs2 doesn't use bio_map_user

It did at some point in time, I don't track the tree so apparently they
stopped doing so at some point.

-- 
Jens Axboe