public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Tejun Heo <tj@kernel.org>
To: axboe@kernel.dk, bharrosh@panasas.com, linux-kernel@vger.kernel.org
Cc: Tejun Heo <tj@kernel.org>, Tejun Heo <tj@kenel.org>
Subject: [PATCH 8/8] blk-map: reimplement blk_rq_map_user() using blk_rq_map_user_iov()
Date: Wed,  1 Apr 2009 20:04:44 +0900	[thread overview]
Message-ID: <1238583884-13517-9-git-send-email-tj@kernel.org> (raw)
In-Reply-To: <1238583884-13517-1-git-send-email-tj@kernel.org>

Impact: subtle bugs fixed, cleanup

blk_rq_map_user() supported multi-bio mapping by calling
bio_map/copy_user() multiple times and linking the resulting bios into
rq; however, this had subtle bugs.

* Because each call to bio_map/copy_user() is independent, segment
  limit check was done only per each bio, so it was possible to create
  requests which are larger than the driver and hardware limits, which
  could lead to disastrous outcome.

* Layers under FS may call blk_rq_map*() functions during request
  processing.  Under severe memory pressure and with enough bad luck,
  this can lead to deadlock.  As fs bvec pool is quite small, the
  possibility isn't completely theoretical.

This patch reimplement blk_rq_map_user() in terms of
blk_rq_map_user_iov() which doesn't support multi-bio mappping and
drop multi bio handling from blk_rq_unmap_user().  Note that with the
previous patch to remove bio max size limit and to add null mapping
support to blk_rq_map_user_iov(), this change doesn't remove any
functionality.

Signed-off-by: Tejun Heo <tj@kenel.org>
---
 block/blk-map.c |  118 +++++--------------------------------------------------
 1 files changed, 10 insertions(+), 108 deletions(-)

diff --git a/block/blk-map.c b/block/blk-map.c
index ac1961d..a43c93c 100644
--- a/block/blk-map.c
+++ b/block/blk-map.c
@@ -40,49 +40,6 @@ static int __blk_rq_unmap_user(struct bio *bio)
 	return ret;
 }
 
-static int __blk_rq_map_user(struct request_queue *q, struct request *rq,
-			     struct rq_map_data *map_data, void __user *ubuf,
-			     unsigned int len, gfp_t gfp_mask)
-{
-	unsigned long uaddr;
-	struct bio *bio, *orig_bio;
-	int reading, ret;
-
-	reading = rq_data_dir(rq) == READ;
-
-	/*
-	 * if alignment requirement is satisfied, map in user pages for
-	 * direct dma. else, set up kernel bounce buffers
-	 */
-	uaddr = (unsigned long) ubuf;
-	if (blk_rq_aligned(q, ubuf, len) && !map_data)
-		bio = bio_map_user(q, NULL, uaddr, len, reading, gfp_mask);
-	else
-		bio = bio_copy_user(q, map_data, uaddr, len, reading, gfp_mask);
-
-	if (IS_ERR(bio))
-		return PTR_ERR(bio);
-
-	orig_bio = bio;
-	blk_queue_bounce(q, &bio);
-
-	/*
-	 * We link the bounce buffer in and could have to traverse it
-	 * later so we have to get a ref to prevent it from being freed
-	 */
-	bio_get(bio);
-
-	ret = blk_rq_append_bio(q, rq, bio);
-	if (!ret)
-		return bio->bi_size;
-
-	/* if it was boucned we must call the end io function */
-	bio_endio(bio, 0);
-	__blk_rq_unmap_user(orig_bio);
-	bio_put(bio);
-	return ret;
-}
-
 /**
  * blk_rq_map_user - map user data to a request, for REQ_TYPE_BLOCK_PC usage
  * @q:		request queue where request should be inserted
@@ -109,58 +66,12 @@ int blk_rq_map_user(struct request_queue *q, struct request *rq,
 		    struct rq_map_data *map_data, void __user *ubuf,
 		    unsigned long len, gfp_t gfp_mask)
 {
-	unsigned long bytes_read = 0;
-	struct bio *bio = NULL;
-	int ret;
+	struct sg_iovec iov;
 
-	if (len > (q->max_hw_sectors << 9))
-		return -EINVAL;
-	if (!len)
-		return -EINVAL;
+	iov.iov_base = ubuf;
+	iov.iov_len = len;
 
-	if (!ubuf && (!map_data || !map_data->null_mapped))
-		return -EINVAL;
-
-	while (bytes_read != len) {
-		unsigned long map_len, end, start;
-
-		map_len = min_t(unsigned long, len - bytes_read,
-				BIO_GUARANTEED_SIZE);
-		end = ((unsigned long)ubuf + map_len + PAGE_SIZE - 1)
-								>> PAGE_SHIFT;
-		start = (unsigned long)ubuf >> PAGE_SHIFT;
-
-		/*
-		 * A bad offset could cause us to require
-		 * BIO_GUARANTEED_PAGES + 1 pages. If this happens we
-		 * just lower the requested mapping len by a page so
-		 * that we can fit
-		 */
-		if (end - start > BIO_GUARANTEED_PAGES)
-			map_len -= PAGE_SIZE;
-
-		ret = __blk_rq_map_user(q, rq, map_data, ubuf, map_len,
-					gfp_mask);
-		if (ret < 0)
-			goto unmap_rq;
-		if (!bio)
-			bio = rq->bio;
-		bytes_read += ret;
-		ubuf += ret;
-
-		if (map_data)
-			map_data->offset += ret;
-	}
-
-	if (!bio_flagged(bio, BIO_USER_MAPPED))
-		rq->cmd_flags |= REQ_COPY_USER;
-
-	rq->buffer = rq->data = NULL;
-	return 0;
-unmap_rq:
-	blk_rq_unmap_user(bio);
-	rq->bio = NULL;
-	return ret;
+	return blk_rq_map_user_iov(q, rq, map_data, &iov, 1, len, gfp_mask);
 }
 EXPORT_SYMBOL(blk_rq_map_user);
 
@@ -250,23 +161,14 @@ EXPORT_SYMBOL(blk_rq_map_user_iov);
  */
 int blk_rq_unmap_user(struct bio *bio)
 {
-	struct bio *mapped_bio;
-	int ret = 0, ret2;
-
-	while (bio) {
-		mapped_bio = bio;
-		if (unlikely(bio_flagged(bio, BIO_BOUNCED)))
-			mapped_bio = bio->bi_private;
+	struct bio *mapped_bio = bio;
+	int ret;
 
-		ret2 = __blk_rq_unmap_user(mapped_bio);
-		if (ret2 && !ret)
-			ret = ret2;
-
-		mapped_bio = bio;
-		bio = bio->bi_next;
-		bio_put(mapped_bio);
-	}
+	if (unlikely(bio_flagged(bio, BIO_BOUNCED)))
+		mapped_bio = bio->bi_private;
 
+	ret = __blk_rq_unmap_user(mapped_bio);
+	bio_put(bio);
 	return ret;
 }
 EXPORT_SYMBOL(blk_rq_unmap_user);
-- 
1.6.0.2


  parent reply	other threads:[~2009-04-01 11:08 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-04-01 11:04 [GIT PATCHSET block#for-linus] block: blk-map related fixes Tejun Heo
2009-04-01 11:04 ` [PATCH 1/8] scatterlist: make sure sg_miter_next() doesn't return 0 sized mappings Tejun Heo
2009-04-01 11:04 ` [PATCH 2/8] block: fix SG_IO vector request data length handling Tejun Heo
2009-04-01 11:47   ` FUJITA Tomonori
2009-04-01 11:50     ` Jens Axboe
2009-04-01 12:18       ` FUJITA Tomonori
2009-04-01 12:24         ` Jens Axboe
2009-04-01 12:27         ` Tejun Heo
2009-04-01 12:29           ` Jens Axboe
2009-04-01 11:04 ` [PATCH 3/8] block: fix queue bounce limit setting Tejun Heo
2009-04-01 11:04 ` [PATCH 4/8] bio: actually inline inline bvecs into bio Tejun Heo
2009-04-01 12:47   ` Jens Axboe
2009-04-01 12:55     ` Tejun Heo
2009-04-01 12:59       ` Jens Axboe
2009-04-01 11:04 ` [PATCH 5/8] bio: fix bio_kmalloc() Tejun Heo
2009-04-01 11:04 ` [PATCH 6/8] bio: remove size/segments limit on bio_{copy|map}_{user|kern}*() Tejun Heo
2009-04-01 11:04 ` [PATCH 7/8] blk-map: let blk_rq_map_user_iov() support null mapping Tejun Heo
2009-04-01 11:54   ` FUJITA Tomonori
2009-04-01 12:03     ` Tejun Heo
2009-04-01 11:04 ` Tejun Heo [this message]
2009-04-01 12:44   ` [PATCH 8/8] blk-map: reimplement blk_rq_map_user() using blk_rq_map_user_iov() FUJITA Tomonori
2009-04-01 12:50     ` Tejun Heo
2009-04-01 12:59       ` FUJITA Tomonori
2009-04-01 13:03         ` Tejun Heo
2009-04-01 13:10           ` FUJITA Tomonori
2009-04-01 13:17             ` Tejun Heo
2009-04-01 13:18               ` Tejun Heo
2009-04-01 13:22               ` FUJITA Tomonori
2009-04-01 13:28                 ` Tejun Heo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1238583884-13517-9-git-send-email-tj@kernel.org \
    --to=tj@kernel.org \
    --cc=axboe@kernel.dk \
    --cc=bharrosh@panasas.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tj@kenel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox