Skip to contentSkip to conversation

remove bio_map_user

PATCH In review Christoph Hellwig 2 participants 6 messages Jun 19, 2005 – Jun 20, 2005

Subsystems (3)
VFS SCSI Block layer
lore
Share & tools
lore

Christoph Hellwig

author
#0 lore raw

PATCH 1/1 remove bio_map_user

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.

drivers/block/ll_rw_blk.c +4 -3
Index: linux-2.6/drivers/block/ll_rw_blk.c
===================================================================
@@ -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)) {
fs/bio.c +0 -23
Index: linux-2.6/fs/bio.c
===================================================================
@@ -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);
include/linux/bio.h +0 -2
Index: linux-2.6/include/linux/bio.h
===================================================================
@@ -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 *,
#1 lore raw
Quoted 1 line · Christoph Hellwig

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.

Quoted 1 line

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.

Signature · 1 line
Jens Axboe
author
#2 lore raw
Quoted 4 lines · Jens Axboe
Quoted 1 line · Christoph Hellwig

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?

Quoted 5 lines
Quoted 2 lines

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?

#3 lore raw
Quoted 7 lines · Christoph Hellwig
Quoted 4 lines · Jens Axboe
Quoted 1 line · Christoph Hellwig

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.

Quoted 7 lines
Quoted 5 lines
Quoted 2 lines

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.

Signature · 1 line
Jens Axboe
author
#4 lore raw
Quoted 3 lines · Jens Axboe
Quoted 1 line

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

Quoted 6 lines
Quoted 4 lines
Quoted 2 lines

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

#5 lore raw
Quoted 7 lines · Christoph Hellwig
Quoted 3 lines · Jens Axboe
Quoted 1 line

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.

Quoted 1 line

fs/direct-io.c

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

Quoted 8 lines
Quoted 6 lines
Quoted 4 lines
Quoted 2 lines

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.

Signature · 1 line
Jens Axboe