All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tejun Heo <htejun@gmail.com>
To: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Cc: tomof@acm.org, jens.axboe@oracle.com,
	James.Bottomley@HansenPartnership.com, efault@gmx.de,
	akpm@linux-foundation.org, linux-kernel@vger.kernel.org,
	linux-ide@vger.kernel.org, linux-scsi@vger.kernel.org,
	jgarzik@pobox.com
Subject: [PATCH 1/2] block: fix residual byte count handling
Date: Mon, 03 Mar 2008 15:08:02 +0900	[thread overview]
Message-ID: <47CB95C2.40309@gmail.com> (raw)
In-Reply-To: <47CB79E9.8000505@gmail.com>

rq->raw_data_len introduced for block layer padding and draining
(commit 6b00769fe1502b4ad97bb327ef7ac971b208bfb5) broke residual byte
count handling.  Block drivers modify rq->data_len to notify residual
byte count to the block layer which blindly reported unmodified
rq->raw_data_len to userland.

To keep block drivers dealing only with rq->data_len, this should be
handled inside block layer.  However, how much extra buffer was
appened is lost after rq->data_len is modified.

This patch replaces rq->raw_data_len with rq->extra_len and add
blk_rq_raw_data_len() helper to calculate raw data size from
rq->data_len and rq->extra_len.  The helper returns correct raw
residual byte count when called on a rq whose data_len is modified to
carry residual byte count.

This problem was reported and diagnosed by Mike Galbraith.

Signed-off-by: Tejun Heo <htejun@gmail.com>
Cc: Mike Galbraith <efault@gmx.de>
---
Comments updated compared to the previous version.

 block/blk-core.c          |    3 +--
 block/blk-map.c           |    2 +-
 block/blk-merge.c         |    1 +
 block/blk-settings.c      |    4 ++++
 block/bsg.c               |    8 ++++----
 block/scsi_ioctl.c        |    4 ++--
 drivers/ata/libata-scsi.c |    3 ++-
 include/linux/blkdev.h    |    8 +++++++-
 8 files changed, 22 insertions(+), 11 deletions(-)

Index: work/block/blk-core.c
===================================================================
--- work.orig/block/blk-core.c
+++ work/block/blk-core.c
@@ -127,7 +127,7 @@ void rq_init(struct request_queue *q, st
 	rq->nr_hw_segments = 0;
 	rq->ioprio = 0;
 	rq->special = NULL;
-	rq->raw_data_len = 0;
+	rq->extra_len = 0;
 	rq->buffer = NULL;
 	rq->tag = -1;
 	rq->errors = 0;
@@ -2016,7 +2016,6 @@ void blk_rq_bio_prep(struct request_queu
 	rq->hard_cur_sectors = rq->current_nr_sectors;
 	rq->hard_nr_sectors = rq->nr_sectors = bio_sectors(bio);
 	rq->buffer = bio_data(bio);
-	rq->raw_data_len = bio->bi_size;
 	rq->data_len = bio->bi_size;
 
 	rq->bio = rq->biotail = bio;
Index: work/block/blk-map.c
===================================================================
--- work.orig/block/blk-map.c
+++ work/block/blk-map.c
@@ -19,7 +19,6 @@ int blk_rq_append_bio(struct request_que
 		rq->biotail->bi_next = bio;
 		rq->biotail = bio;
 
-		rq->raw_data_len += bio->bi_size;
 		rq->data_len += bio->bi_size;
 	}
 	return 0;
@@ -156,6 +155,7 @@ int blk_rq_map_user(struct request_queue
 		bio->bi_io_vec[bio->bi_vcnt - 1].bv_len += pad_len;
 		bio->bi_size += pad_len;
 		rq->data_len += pad_len;
+		rq->extra_len += pad_len;
 	}
 
 	rq->buffer = rq->data = NULL;
Index: work/block/blk-merge.c
===================================================================
--- work.orig/block/blk-merge.c
+++ work/block/blk-merge.c
@@ -232,6 +232,7 @@ new_segment:
 			    (PAGE_SIZE - 1));
 		nsegs++;
 		rq->data_len += q->dma_drain_size;
+		rq->extra_len += q->dma_drain_size;
 	}
 
 	if (sg)
Index: work/block/bsg.c
===================================================================
--- work.orig/block/bsg.c
+++ work/block/bsg.c
@@ -437,14 +437,14 @@ static int blk_complete_sgv4_hdr_rq(stru
 	}
 
 	if (rq->next_rq) {
-		hdr->dout_resid = rq->raw_data_len;
-		hdr->din_resid = rq->next_rq->raw_data_len;
+		hdr->dout_resid = blk_rq_raw_data_len(rq);
+		hdr->din_resid = blk_rq_raw_data_len(rq->next_rq);
 		blk_rq_unmap_user(bidi_bio);
 		blk_put_request(rq->next_rq);
 	} else if (rq_data_dir(rq) == READ)
-		hdr->din_resid = rq->raw_data_len;
+		hdr->din_resid = blk_rq_raw_data_len(rq);
 	else
-		hdr->dout_resid = rq->raw_data_len;
+		hdr->dout_resid = blk_rq_raw_data_len(rq);
 
 	/*
 	 * If the request generated a negative error number, return it
Index: work/block/scsi_ioctl.c
===================================================================
--- work.orig/block/scsi_ioctl.c
+++ work/block/scsi_ioctl.c
@@ -266,7 +266,7 @@ static int blk_complete_sghdr_rq(struct 
 	hdr->info = 0;
 	if (hdr->masked_status || hdr->host_status || hdr->driver_status)
 		hdr->info |= SG_INFO_CHECK;
-	hdr->resid = rq->raw_data_len;
+	hdr->resid = blk_rq_raw_data_len(rq);
 	hdr->sb_len_wr = 0;
 
 	if (rq->sense_len && hdr->sbp) {
@@ -528,8 +528,8 @@ static int __blk_send_generic(struct req
 	rq = blk_get_request(q, WRITE, __GFP_WAIT);
 	rq->cmd_type = REQ_TYPE_BLOCK_PC;
 	rq->data = NULL;
-	rq->raw_data_len = 0;
 	rq->data_len = 0;
+	rq->extra_len = 0;
 	rq->timeout = BLK_DEFAULT_SG_TIMEOUT;
 	memset(rq->cmd, 0, sizeof(rq->cmd));
 	rq->cmd[0] = cmd;
Index: work/drivers/ata/libata-scsi.c
===================================================================
--- work.orig/drivers/ata/libata-scsi.c
+++ work/drivers/ata/libata-scsi.c
@@ -2549,7 +2549,8 @@ static unsigned int atapi_xlat(struct at
 	 * want to set it properly, and for DMA where it is
 	 * effectively meaningless.
 	 */
-	nbytes = min(scmd->request->raw_data_len, (unsigned int)63 * 1024);
+	nbytes = min(blk_rq_raw_data_len(scmd->request),
+		     (unsigned int)63 * 1024);
 
 	/* Most ATAPI devices which honor transfer chunk size don't
 	 * behave according to the spec when odd chunk size which
Index: work/include/linux/blkdev.h
===================================================================
--- work.orig/include/linux/blkdev.h
+++ work/include/linux/blkdev.h
@@ -216,8 +216,8 @@ struct request {
 	unsigned int cmd_len;
 	unsigned char cmd[BLK_MAX_CDB];
 
-	unsigned int raw_data_len;
 	unsigned int data_len;
+	unsigned int extra_len;	/* length of padding and draining buffers */
 	unsigned int sense_len;
 	void *data;
 	void *sense;
@@ -477,6 +477,12 @@ enum {
 
 #define rq_data_dir(rq)		((rq)->cmd_flags & 1)
 
+/* data_len of the request sans extra stuff for padding and draining */
+static inline unsigned int blk_rq_raw_data_len(struct request *rq)
+{
+	return rq->data_len - min(rq->extra_len, rq->data_len);
+}
+
 /*
  * We regard a request as sync, if it's a READ or a SYNC write.
  */
Index: work/block/blk-settings.c
===================================================================
--- work.orig/block/blk-settings.c
+++ work/block/blk-settings.c
@@ -309,6 +309,10 @@ EXPORT_SYMBOL(blk_queue_stack_limits);
  * does is adjust the queue so that the buf is always appended
  * silently to the scatterlist.
  *
+ * Appending draining buffer to a request modifies ->data_len such
+ * that it includes the drain buffer.  The original requested data
+ * length can be obtained using blk_rq_raw_data_len().
+ *
  * Note: This routine adjusts max_hw_segments to make room for
  * appending the drain buffer.  If you call
  * blk_queue_max_hw_segments() or blk_queue_max_phys_segments() after

  reply	other threads:[~2008-03-03  6:08 UTC|newest]

Thread overview: 116+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-02-21  8:42 regression: CD burning (k3b) went broke Mike Galbraith
2008-02-22  7:32 ` Jens Axboe
2008-02-23  7:42   ` Mike Galbraith
2008-02-24  7:54     ` Mike Galbraith
2008-02-26  9:48       ` Mike Galbraith
2008-02-26 13:36         ` Mike Galbraith
2008-02-26 23:08           ` Andrew Morton
2008-02-27  0:46             ` Jeff Garzik
2008-02-27  2:58               ` Mike Galbraith
2008-02-27  2:24             ` Mike Galbraith
2008-02-27  6:00               ` Mike Galbraith
2008-02-27  7:07                 ` Mike Galbraith
2008-02-28  7:43                   ` Tejun Heo
2008-02-28  8:20                     ` Mike Galbraith
2008-02-28  8:50                       ` [PATCH] block: fix residual byte count handling Tejun Heo
2008-02-28 15:35                         ` Jens Axboe
2008-02-28 15:46                           ` Tejun Heo
2008-02-29 16:47                             ` James Bottomley
2008-02-29 20:11                               ` Jens Axboe
2008-03-01  6:17                                 ` Tejun Heo
2008-03-01 15:19                                   ` James Bottomley
2008-03-02 14:52                                   ` FUJITA Tomonori
2008-03-02 14:52                                     ` FUJITA Tomonori
2008-03-02 18:46                                     ` Mike Christie
2008-03-03  3:27                                       ` Mike Galbraith
2008-03-03  2:40                                     ` Tejun Heo
2008-03-03  3:59                                       ` FUJITA Tomonori
2008-03-03  4:09                                         ` Tejun Heo
2008-03-03  6:08                                           ` Tejun Heo [this message]
2008-03-03  6:10                                             ` [PATCH] block: separate out padding from alignment Tejun Heo
2008-03-03 18:27                                               ` James Bottomley
2008-03-03  8:26                                           ` [PATCH] block: fix residual byte count handling FUJITA Tomonori
2008-03-03  9:21                                             ` Tejun Heo
2008-03-03 12:17                                               ` FUJITA Tomonori
2008-03-03 12:17                                                 ` FUJITA Tomonori
2008-03-03 13:38                                                 ` Tejun Heo
2008-03-03 13:50                                                   ` FUJITA Tomonori
2008-03-03 13:50                                                     ` FUJITA Tomonori
2008-03-03 13:55                                                     ` Tejun Heo
2008-03-03 14:01                                                       ` FUJITA Tomonori
2008-03-03 14:01                                                         ` FUJITA Tomonori
2008-03-03 14:22                                                         ` Tejun Heo
2008-03-03 14:52                                                           ` FUJITA Tomonori
2008-03-03 22:44                                                             ` Tejun Heo
2008-03-04  2:11                                                               ` FUJITA Tomonori
2008-03-04  2:32                                                                 ` Tejun Heo
2008-03-04  8:53                                                                   ` FUJITA Tomonori
2008-03-04  8:59                                                                     ` Jens Axboe
2008-03-04  9:06                                                                       ` FUJITA Tomonori
2008-03-04  9:22                                                                         ` FUJITA Tomonori
2008-03-04  9:30                                                                           ` Tejun Heo
2008-03-04  9:35                                                                           ` Jens Axboe
2008-03-04  9:40                                                                             ` Tejun Heo
2008-03-04  9:46                                                                               ` Jens Axboe
2008-03-04 12:37                                                                             ` Mike Galbraith
2008-03-04 12:39                                                                               ` Jens Axboe
2008-03-04 12:43                                                                                 ` Mike Galbraith
2008-03-04 12:58                                                                                   ` Mike Galbraith
2008-03-04 13:03                                                                                     ` Jens Axboe
2008-03-04 14:25                                                                                       ` Mike Galbraith
2008-03-04 18:17                                                                                         ` Jens Axboe
2008-03-04 18:29                                                                                           ` Jens Axboe
2008-03-04 18:35                                                                                           ` Mike Galbraith
2008-03-04 18:45                                                                                             ` Jens Axboe
2008-03-04 18:49                                                                                               ` Mike Galbraith
2008-03-04 18:54                                                                                                 ` Jens Axboe
2008-03-04 19:26                                                                                                   ` Mike Galbraith
2008-03-04 19:28                                                                                                     ` Jens Axboe
2008-03-04 16:04                                                                                 ` James Bottomley
2008-03-04 18:46                                                                                   ` Jens Axboe
2008-03-04 17:34                                                                                 ` walt
2008-03-04 17:34                                                                                   ` walt
2008-03-04 17:59                                                                                   ` Tejun Heo
2008-03-04 19:18                                                                                     ` walt
2008-03-04 19:42                                                                                   ` Kiyoshi Ueda
2008-03-04 12:40                                                                               ` Tejun Heo
2008-03-04 12:45                                                                                 ` Mike Galbraith
2008-03-04 13:30                                                                                 ` FUJITA Tomonori
2008-03-04 13:50                                                                                   ` Tejun Heo
2008-03-04 16:17                                                                                     ` Tejun Heo
2008-03-04 16:42                                                                                       ` Tejun Heo
2008-03-04 18:26                                                                                         ` Boaz Harrosh
2008-03-04 18:35                                                                                           ` Tejun Heo
2008-03-04 18:27                                                                                         ` James Bottomley
2008-03-04 18:33                                                                                           ` Tejun Heo
2008-03-04 18:45                                                                                         ` Mike Galbraith
2008-03-04 19:25                                                                                           ` Jens Axboe
2008-03-04 19:33                                                                                             ` Mike Galbraith
2008-03-04 19:34                                                                                               ` Jens Axboe
2008-03-04 19:19                                                                                         ` FUJITA Tomonori
2008-03-04 19:19                                                                                           ` FUJITA Tomonori
2008-03-04 23:33                                                                                           ` Tejun Heo
2008-03-04 23:54                                                                                             ` Tejun Heo
2008-03-05  0:26                                                                                             ` FUJITA Tomonori
2008-03-05  0:44                                                                                               ` Tejun Heo
2008-03-06  4:56                                                                                                 ` FUJITA Tomonori
2008-03-06  5:02                                                                                                   ` Tejun Heo
2008-03-05 10:16                                                                                               ` [PATCH] blk: missing add of padded bytes to io completion byte count Boaz Harrosh
2008-03-05 12:28                                                                                                 ` Mike Galbraith
2008-03-05 12:33                                                                                                 ` Jens Axboe
2008-03-05 12:46                                                                                                   ` Boaz Harrosh
2008-03-05 12:48                                                                                                     ` Jens Axboe
2008-03-05 13:45                                                                                                       ` Tejun Heo
2008-03-05 13:51                                                                                                         ` Jens Axboe
2008-03-05 14:08                                                                                                           ` Tejun Heo
2008-03-05 15:21                                                                                                           ` James Bottomley
2008-03-06  4:41                                                                                                             ` FUJITA Tomonori
2008-03-06 13:41                                                                                                               ` Jens Axboe
2008-03-07  0:07                                                                                                                 ` Tejun Heo
2008-03-07 15:07                                                                                                                   ` FUJITA Tomonori
2008-03-08  1:06                                                                                                                     ` Tejun Heo
2008-03-20 12:54                                                                                                                 ` FUJITA Tomonori
2008-03-05 14:46                                                                                                         ` Boaz Harrosh
2008-03-05 15:11                                                                                                           ` Tejun Heo
2008-03-06  5:02                                                                                                 ` FUJITA Tomonori
2008-03-04  9:29                                                                       ` [PATCH] block: fix residual byte count handling 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=47CB95C2.40309@gmail.com \
    --to=htejun@gmail.com \
    --cc=James.Bottomley@HansenPartnership.com \
    --cc=akpm@linux-foundation.org \
    --cc=efault@gmx.de \
    --cc=fujita.tomonori@lab.ntt.co.jp \
    --cc=jens.axboe@oracle.com \
    --cc=jgarzik@pobox.com \
    --cc=linux-ide@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=tomof@acm.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.