Discussion:
two more fixes for block/for-linus
Christoph Hellwig
2014-09-22 13:59:29 UTC
Permalink
Fix two regressions with the timer/blk_mq_start_request changes found
when using scsi-mq on an ATA device.

Note that given how late we are in the cycle I wonder if we really should
push the changes in block/for-linus to Linus in this cycle. The race
part of the timer fixes is extremly narrow, and the reserved request
part of it requires an out of tree driver update to trigger it, so it
doesn't sound super criticial.
Christoph Hellwig
2014-09-22 13:59:30 UTC
Permalink
Signed-off-by: Christoph Hellwig <***@lst.de>
---
block/blk-mq.c | 2 +-
block/blk-timeout.c | 5 ++++-
block/blk.h | 2 ++
3 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index 966c920..aeac4b7 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -531,7 +531,7 @@ struct blk_mq_timeout_data {
unsigned int next_set;
};

-static void blk_mq_rq_timed_out(struct request *req, bool reserved)
+void blk_mq_rq_timed_out(struct request *req, bool reserved)
{
struct blk_mq_ops *ops = req->q->mq_ops;
enum blk_eh_timer_return ret = BLK_EH_RESET_TIMER;
diff --git a/block/blk-timeout.c b/block/blk-timeout.c
index 8bae410..c3c14ce 100644
--- a/block/blk-timeout.c
+++ b/block/blk-timeout.c
@@ -158,7 +158,10 @@ void blk_abort_request(struct request *req)
if (blk_mark_rq_complete(req))
return;
blk_delete_timer(req);
- blk_rq_timed_out(req);
+ if (req->q->mq_ops)
+ blk_mq_rq_timed_out(req, false);
+ else
+ blk_rq_timed_out(req);
}
EXPORT_SYMBOL_GPL(blk_abort_request);

diff --git a/block/blk.h b/block/blk.h
index e515a28..159e973 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -250,4 +250,6 @@ static inline int blk_throtl_init(struct request_queue *q) { return 0; }
static inline void blk_throtl_exit(struct request_queue *q) { }
#endif /* CONFIG_BLK_DEV_THROTTLING */

+extern void blk_mq_rq_timed_out(struct request *req, bool reserved);
+
#endif /* BLK_INTERNAL_H */
--
1.9.1
Jens Axboe
2014-09-22 14:08:19 UTC
Permalink
Post by Christoph Hellwig
---
block/blk-mq.c | 2 +-
block/blk-timeout.c | 5 ++++-
block/blk.h | 2 ++
3 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/block/blk.h b/block/blk.h
index e515a28..159e973 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -250,4 +250,6 @@ static inline int blk_throtl_init(struct request_queue *q) { return 0; }
static inline void blk_throtl_exit(struct request_queue *q) { }
#endif /* CONFIG_BLK_DEV_THROTTLING */
+extern void blk_mq_rq_timed_out(struct request *req, bool reserved);
Moving this to blk-mq.h.

Applied.
--
Jens Axboe

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Christoph Hellwig
2014-09-22 13:59:31 UTC
Permalink
Some ATA drivers need the dma drain size workaround, and thus need to
call blk_mq_start_request before the S/G mapping.

Signed-off-by: Christoph Hellwig <***@lst.de>
---
drivers/scsi/scsi_lib.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index dc01ab2..56cc4b4 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1846,6 +1846,8 @@ static int scsi_mq_prep_fn(struct request *req)
next_rq->special = bidi_sdb;
}

+ blk_mq_start_request(req);
+
return scsi_setup_cmnd(sdev, req);
}

@@ -1880,17 +1882,19 @@ static int scsi_queue_rq(struct blk_mq_hw_ctx *hctx, struct request *req,
if (!scsi_host_queue_ready(q, shost, sdev))
goto out_dec_target_busy;

+
if (!(req->cmd_flags & REQ_DONTPREP)) {
ret = prep_to_mq(scsi_mq_prep_fn(req));
if (ret)
goto out_dec_host_busy;
req->cmd_flags |= REQ_DONTPREP;
+ } else {
+ blk_mq_start_request(req);
}

scsi_init_cmd_errh(cmd);
cmd->scsi_done = scsi_mq_done;

- blk_mq_start_request(req);
reason = scsi_dispatch_cmd(cmd);
if (reason) {
scsi_set_blocked(cmd, reason);
--
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Jens Axboe
2014-09-22 14:06:49 UTC
Permalink
Post by Christoph Hellwig
Fix two regressions with the timer/blk_mq_start_request changes found
when using scsi-mq on an ATA device.
Note that given how late we are in the cycle I wonder if we really should
push the changes in block/for-linus to Linus in this cycle. The race
part of the timer fixes is extremly narrow, and the reserved request
part of it requires an out of tree driver update to trigger it, so it
doesn't sound super criticial.
I was planning on pushing it out today, I've run a lot of testing on it
internally last week. Ho hum, let me think about it...
--
Jens Axboe
Douglas Gilbert
2014-09-22 18:40:15 UTC
Permalink
Post by Jens Axboe
Post by Christoph Hellwig
Fix two regressions with the timer/blk_mq_start_request changes found
when using scsi-mq on an ATA device.
Note that given how late we are in the cycle I wonder if we really should
push the changes in block/for-linus to Linus in this cycle. The race
part of the timer fixes is extremly narrow, and the reserved request
part of it requires an out of tree driver update to trigger it, so it
doesn't sound super criticial.
I was planning on pushing it out today, I've run a lot of testing on it
internally last week. Ho hum, let me think about it...
With these patches applied (actually a resync an hour
ago with the for-linus tree which includes them), the
freeze-during-boot-up problem that I have been seeing
with an old SATA boot disk (perhaps 1.5 Gbps) for
the last two weeks, has gone away.

That SATA disk is connected to the motherboard (Gigabyte
Z97M-D3H/Z97M-D3H, BIOS F5 05/30/2014) and has a standard
AHCI interface as far as I can tell. dmesg confirms that.


Please add "Tested-by: Douglas Gilbert <***@interlog.com>"
to Christoph's "two more fixes for block/for-linus" series.

Doug Gilbert
Christoph Hellwig
2014-09-23 05:56:54 UTC
Permalink
Post by Douglas Gilbert
With these patches applied (actually a resync an hour
ago with the for-linus tree which includes them), the
freeze-during-boot-up problem that I have been seeing
with an old SATA boot disk (perhaps 1.5 Gbps) for
the last two weeks, has gone away.
That SATA disk is connected to the motherboard (Gigabyte
Z97M-D3H/Z97M-D3H, BIOS F5 05/30/2014) and has a standard
AHCI interface as far as I can tell. dmesg confirms that.
Should have thought of the weird ATA error handling earlier. Sorry Doug!
Continue reading on narkive:
Loading...