On 6/5/14, 9:53 PM, KY Srinivasan wrote:
-----Original Message-----
From: Mike Christie [mailto:michaelc@xxxxxxxxxxx]
Sent: Thursday, June 5, 2014 6:33 PM
To: KY Srinivasan
Cc: James Bottomley; linux-kernel@xxxxxxxxxxxxxxx; apw@xxxxxxxxxxxxx;
devel@xxxxxxxxxxxxxxxxxxxxxx; hch@xxxxxxxxxxxxx; linux-
scsi@xxxxxxxxxxxxxxx; ohering@xxxxxxxx; gregkh@xxxxxxxxxxxxxxxxxxx;
jasowang@xxxxxxxxxx
Subject: Re: [PATCH 1/1] [SCSI] Fix a bug in deriving the FLUSH_TIMEOUT
from the basic I/O timeout
On 06/04/2014 12:15 PM, KY Srinivasan wrote:
-----Original Message-----
From: James Bottomley [mailto:jbottomley@xxxxxxxxxxxxx]
Sent: Wednesday, June 4, 2014 10:02 AM
To: KY Srinivasan
Cc: linux-kernel@xxxxxxxxxxxxxxx; apw@xxxxxxxxxxxxx;
devel@xxxxxxxxxxxxxxxxxxxxxx; hch@xxxxxxxxxxxxx; linux-
scsi@xxxxxxxxxxxxxxx; ohering@xxxxxxxx; gregkh@xxxxxxxxxxxxxxxxxxx;
jasowang@xxxxxxxxxx
Subject: Re: [PATCH 1/1] [SCSI] Fix a bug in deriving the
FLUSH_TIMEOUT from the basic I/O timeout
On Wed, 2014-06-04 at 09:33 -0700, K. Y. Srinivasan wrote:
Commit ID: 7e660100d85af860e7ad763202fff717adcdaacd added code to
derive the FLUSH_TIMEOUT from the basic I/O timeout. However, this
patch did not use the basic I/O timeout of the device. Fix this bug.
Signed-off-by: K. Y. Srinivasan <kys@xxxxxxxxxxxxx>
---
drivers/scsi/sd.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c index
e9689d5..54150b1 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -832,7 +832,9 @@ static int sd_setup_write_same_cmnd(struct
scsi_device *sdp, struct request *rq)
static int scsi_setup_flush_cmnd(struct scsi_device *sdp, struct
request *rq) {
- rq->timeout *= SD_FLUSH_TIMEOUT_MULTIPLIER;
+ int timeout = sdp->request_queue->rq_timeout;
+
+ rq->timeout = (timeout * SD_FLUSH_TIMEOUT_MULTIPLIER);
Could you share where you found this to be a problem? It looks like
a bug in block because all inbound requests being prepared should
have a timeout set, so block would be the place to fix it.
Perhaps; what I found was that the value in rq->timeout was 0 coming
into this function and thus multiplying obviously has no effect.
I think you are right. We hit this problem because we are doing:
scsi_request_fn -> blk_peek_request -> sd_prep_fn ->
scsi_setup_flush_cmnd.
At this time request->timeout is zero so the multiplication does nothing. See
how sd_setup_write_same_cmnd will set the request->timeout at this time.
Then in scsi_request_fn we do:
scsi_request_fn -> blk_start_request -> blk_add_timer.
At this time it will set the request->timeout if something like req block pc
users (like scsi_execute() or block/scsi_ioctl.c) or the write same code
mentioned above have not set the timeout.
I don't think this is a recent change. Prior to this commit, we were setting the timeout
value in this function; it just happened to be a different constant unrelated to the I/O
timeout.
Yeah, it looks like when 7e660100d85af860e7ad763202fff717adcdaacd was
merged we were supposed to initialize it like in your patch in this thread.
I guess we could do your patch in this thread, or if we want the block
layer to initialize the timeout before the prep_fn callout is called
then we would need to have the blk-flush.c code to that when it sets up
the request. If we do the latter, do we want the discard and write same
code to initialize the request's timeout before the prep_fn callout is
called too?
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
