All of lore.kernel.org
 help / color / mirror / Atom feed
From: Boaz Harrosh <bharrosh@panasas.com>
To: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>, jens.axboe@oracle.com
Cc: James.Bottomley@HansenPartnership.com,
	linux-scsi@vger.kernel.org,
	linux-kernel <linux-kernel@vger.kernel.org>
Subject: [PATCH] [RFC] block: Don't let blk_put_request leak BIOs
Date: Thu, 12 Feb 2009 14:19:39 +0200	[thread overview]
Message-ID: <499413DB.2020208@panasas.com> (raw)
In-Reply-To: <20090212184208X.fujita.tomonori@lab.ntt.co.jp>


If a block ULD had allocated a request and mapped some memory into it,
using one of blk_rq_map_xxx routines, but then for some reason failed to
execute the request through one of the blk_execute_request routines.
Then the associated BIO would leak, unless ULD resorts to low-level loops
intimate of block internals.

[RFC]
  This code will also catch situations where LLD failed to complete
  the request before aborting it. Such situations are a BUG. Should we
  use WARN_ON_ONCE() in that case. The situation above is possible and
  can happen normally in memory pressure situations so maybe we should
  devise a bit-flag that ULD denotes that the request was aborted and
  only WARN_ON if flag was not set.

  I'm sending this before any-tests so people can comment on possible
  pitfalls.

Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>
---
 block/blk-core.c                 |   16 ++++++++++++++++
 drivers/scsi/osd/osd_initiator.c |   17 +----------------
 2 files changed, 17 insertions(+), 16 deletions(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index a824e49..eac96c2 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -1055,6 +1055,21 @@ void part_round_stats(int cpu, struct hd_struct *part)
 EXPORT_SYMBOL_GPL(part_round_stats);
 
 /*
+ * If one of the blk_rq_map_xxx() was called but the request was not
+ * executed by the block layer, then we must release BIOs. Otherwise they
+ * will leak.
+ */
+static void _abort_unexecuted_bios(struct request *req)
+{
+	struct bio *bio;
+
+	while ((bio = req->bio) != NULL) {
+		req->bio = bio->bi_next;
+		bio_endio(bio, 0);
+	}
+}
+
+/*
  * queue lock must be held
  */
 void __blk_put_request(struct request_queue *q, struct request *req)
@@ -1066,6 +1081,7 @@ void __blk_put_request(struct request_queue *q, struct request *req)
 
 	elv_completed_request(q, req);
 
+	_abort_unexecuted_bios(req);
 	/*
 	 * Request may not have originated from ll_rw_blk. if not,
 	 * it didn't come out of our reserved rq pools
diff --git a/drivers/scsi/osd/osd_initiator.c b/drivers/scsi/osd/osd_initiator.c
index 0bbbf27..8b1cf72 100644
--- a/drivers/scsi/osd/osd_initiator.c
+++ b/drivers/scsi/osd/osd_initiator.c
@@ -335,20 +335,6 @@ struct osd_request *osd_start_request(struct osd_dev *dev, gfp_t gfp)
 }
 EXPORT_SYMBOL(osd_start_request);
 
-/*
- * If osd_finalize_request() was called but the request was not executed through
- * the block layer, then we must release BIOs.
- */
-static void _abort_unexecuted_bios(struct request *rq)
-{
-	struct bio *bio;
-
-	while ((bio = rq->bio) != NULL) {
-		rq->bio = bio->bi_next;
-		bio_endio(bio, 0);
-	}
-}
-
 static void _osd_free_seg(struct osd_request *or __unused,
 	struct _osd_req_data_segment *seg)
 {
@@ -370,11 +356,10 @@ void osd_end_request(struct osd_request *or)
 
 	if (rq) {
 		if (rq->next_rq) {
-			_abort_unexecuted_bios(rq->next_rq);
 			blk_put_request(rq->next_rq);
+			rq->next_rq = NULL;
 		}
 
-		_abort_unexecuted_bios(rq);
 		blk_put_request(rq);
 	}
 	_osd_request_free(or);
-- 
1.6.0.1



  reply	other threads:[~2009-02-12 12:19 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-12-13 16:23 [PATCH 0/3] remove scsi_req_map_sg FUJITA Tomonori
2008-12-13 16:23 ` [PATCH 1/3] " FUJITA Tomonori
2008-12-13 16:23   ` [PATCH 2/3] block: unexport blk_rq_append_bio FUJITA Tomonori
2008-12-13 16:23     ` [PATCH 3/3] block: unexport bio_add_pc_page FUJITA Tomonori
2009-02-10 17:43     ` [PATCH 2/3] block: unexport blk_rq_append_bio James Bottomley
2009-02-10 18:19       ` James Bottomley
2009-02-11  0:21         ` FUJITA Tomonori
2009-02-11  8:17         ` Boaz Harrosh
2009-02-11  8:19           ` Boaz Harrosh
2009-02-11  9:15           ` Boaz Harrosh
2009-02-11 14:32             ` FUJITA Tomonori
2009-02-11 14:52               ` Boaz Harrosh
2009-02-11 15:01                 ` FUJITA Tomonori
2009-02-11 15:07                   ` Boaz Harrosh
2009-02-11 15:21                     ` FUJITA Tomonori
2009-02-11 15:41                       ` Boaz Harrosh
2009-02-11 16:04                         ` FUJITA Tomonori
2009-02-11 16:30                           ` James Bottomley
2009-02-11 17:55                             ` Boaz Harrosh
2009-02-12  1:30                               ` FUJITA Tomonori
2009-02-12  8:24                                 ` Boaz Harrosh
2009-02-12  8:28                                   ` [RFD] blk_rq_map_pages new API Boaz Harrosh
2009-02-12  9:19                                     ` Boaz Harrosh
2009-02-12  9:50                                       ` FUJITA Tomonori
2009-02-12 10:20                                         ` FUJITA Tomonori
2009-02-12 11:34                                           ` Boaz Harrosh
2009-02-12  8:41                                   ` [PATCH 2/3] block: unexport blk_rq_append_bio FUJITA Tomonori
2009-02-12  9:14                                     ` Boaz Harrosh
2009-02-12  9:50                                       ` FUJITA Tomonori
2009-02-12 12:19                                         ` Boaz Harrosh [this message]
2009-02-12 13:49                                           ` [PATCH] [RFC] block: Don't let blk_put_request leak BIOs Boaz Harrosh
2009-02-12 13:56                                             ` Boaz Harrosh
2009-02-12 17:27                                               ` [PATCH 1/2] " Boaz Harrosh
2009-02-12 17:30                                                 ` [PATCH 2/2] [RFC] libosd: Don't let osd abuse block internals, now that it's fixed Boaz Harrosh
2009-02-12 14:48                               ` [PATCH 2/3] block: unexport blk_rq_append_bio James Bottomley
2009-02-12 16:51                                 ` Boaz Harrosh
2008-12-15  7:28 ` [PATCH 0/3] remove scsi_req_map_sg Jens Axboe
2008-12-18  8:14   ` FUJITA Tomonori

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=499413DB.2020208@panasas.com \
    --to=bharrosh@panasas.com \
    --cc=James.Bottomley@HansenPartnership.com \
    --cc=fujita.tomonori@lab.ntt.co.jp \
    --cc=jens.axboe@oracle.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.