[PATCH 0/6] blk-mq: initialize pdu of flush req explicitly
Hi, This patchset introduces init_flush_rq and its pair callback to initialize pdu of flush request explicitly, instead of using copying from request which is inefficient and buggy, and implements them in virtio-blk and scsi-lib. block/blk-flush.c | 22 +- block/blk-mq.c | 23 --- block/blk-mq.h |3 ++- drivers/block/virtio_blk.c | 15 ++- drivers/scsi/scsi_lib.c| 32 +++- include/linux/blk-mq.h |9 + 6 files changed, 81 insertions(+), 23 deletions(-) Thanks, -- Ming Lei -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 6/6] blk-mq: don't copy pdu any more for flush req
The in-tree drivers which need to handle flush request
have implemented init_flush_rq already, so don't
copy pdu any more for flush req.
Signed-off-by: Ming Lei ming@canonical.com
---
block/blk-mq.c |7 ---
1 file changed, 7 deletions(-)
diff --git a/block/blk-mq.c b/block/blk-mq.c
index 113d58d..16f595f 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -287,15 +287,8 @@ void blk_mq_free_request(struct request *rq)
void blk_mq_clone_flush_request(struct request *flush_rq,
struct request *orig_rq)
{
- struct blk_mq_hw_ctx *hctx =
- orig_rq-q-mq_ops-map_queue(orig_rq-q, orig_rq-mq_ctx-cpu);
-
flush_rq-mq_ctx = orig_rq-mq_ctx;
flush_rq-tag = orig_rq-tag;
-
- if (!orig_rq-q-mq_ops-init_flush_rq)
- memcpy(blk_mq_rq_to_pdu(flush_rq),
- blk_mq_rq_to_pdu(orig_rq), hctx-cmd_size);
}
inline void __blk_mq_end_io(struct request *rq, int error)
--
1.7.9.5
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/6] blk-mq: allocate flush_rq in blk_mq_init_flush()
It is reasonable to allocate flush req in blk_mq_init_flush().
Signed-off-by: Ming Lei ming@canonical.com
---
block/blk-flush.c | 11 ++-
block/blk-mq.c| 16 ++--
block/blk-mq.h|2 +-
3 files changed, 17 insertions(+), 12 deletions(-)
diff --git a/block/blk-flush.c b/block/blk-flush.c
index 3cb5e9e..75ca6cd0 100644
--- a/block/blk-flush.c
+++ b/block/blk-flush.c
@@ -474,7 +474,16 @@ int blkdev_issue_flush(struct block_device *bdev, gfp_t
gfp_mask,
}
EXPORT_SYMBOL(blkdev_issue_flush);
-void blk_mq_init_flush(struct request_queue *q)
+int blk_mq_init_flush(struct request_queue *q)
{
+ struct blk_mq_tag_set *set = q-tag_set;
+
spin_lock_init(q-mq_flush_lock);
+
+ q-flush_rq = kzalloc(round_up(sizeof(struct request) +
+ set-cmd_size, cache_line_size()),
+ GFP_KERNEL);
+ if (!q-flush_rq)
+ return -ENOMEM;
+ return 0;
}
diff --git a/block/blk-mq.c b/block/blk-mq.c
index 4aac826..23386c0 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -1826,17 +1826,10 @@ struct request_queue *blk_mq_init_queue(struct
blk_mq_tag_set *set)
if (set-ops-complete)
blk_queue_softirq_done(q, set-ops-complete);
- blk_mq_init_flush(q);
blk_mq_init_cpu_queues(q, set-nr_hw_queues);
- q-flush_rq = kzalloc(round_up(sizeof(struct request) +
- set-cmd_size, cache_line_size()),
- GFP_KERNEL);
- if (!q-flush_rq)
- goto err_hw;
-
if (blk_mq_init_hw_queues(q, set))
- goto err_flush_rq;
+ goto err_hw;
mutex_lock(all_q_mutex);
list_add_tail(q-all_q_node, all_q_list);
@@ -1844,12 +1837,15 @@ struct request_queue *blk_mq_init_queue(struct
blk_mq_tag_set *set)
blk_mq_add_queue_tag_set(set, q);
+ if (blk_mq_init_flush(q))
+ goto err_hw_queues;
+
blk_mq_map_swqueue(q);
return q;
-err_flush_rq:
- kfree(q-flush_rq);
+err_hw_queues:
+ blk_mq_exit_hw_queues(q, set, set-nr_hw_queues);
err_hw:
blk_cleanup_queue(q);
err_hctxs:
diff --git a/block/blk-mq.h b/block/blk-mq.h
index ca4964a..b0bd9bc 100644
--- a/block/blk-mq.h
+++ b/block/blk-mq.h
@@ -27,7 +27,7 @@ struct blk_mq_ctx {
void __blk_mq_complete_request(struct request *rq);
void blk_mq_run_hw_queue(struct blk_mq_hw_ctx *hctx, bool async);
-void blk_mq_init_flush(struct request_queue *q);
+int blk_mq_init_flush(struct request_queue *q);
void blk_mq_freeze_queue(struct request_queue *q);
void blk_mq_free_queue(struct request_queue *q);
void blk_mq_clone_flush_request(struct request *flush_rq,
--
1.7.9.5
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/6] scsi-lib: implement init_flush_rq and its pair
Now implement init_flush_rq callback to avoid the unnecessary
pdu copying done in blk_mq_clone_flush_request().
The sense buffer is introduced to flush req, but it won't be
a deal since there is only one flush request per queue. It still
may be borrowed from the sence buffer of the request cloned from,
but looks a bit ugly.
Signed-off-by: Ming Lei ming@canonical.com
---
drivers/scsi/scsi_lib.c | 32 +++-
1 file changed, 27 insertions(+), 5 deletions(-)
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index b9a8ddd..9090418 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1930,9 +1930,7 @@ out:
return ret;
}
-static int scsi_init_request(void *data, struct request *rq,
- unsigned int hctx_idx, unsigned int request_idx,
- unsigned int numa_node)
+static int __scsi_init_request(struct request *rq, int numa_node)
{
struct scsi_cmnd *cmd = blk_mq_rq_to_pdu(rq);
@@ -1943,14 +1941,36 @@ static int scsi_init_request(void *data, struct request
*rq,
return 0;
}
-static void scsi_exit_request(void *data, struct request *rq,
- unsigned int hctx_idx, unsigned int request_idx)
+static void __scsi_exit_request(struct request *rq)
{
struct scsi_cmnd *cmd = blk_mq_rq_to_pdu(rq);
kfree(cmd-sense_buffer);
}
+static int scsi_init_request(void *data, struct request *rq,
+ unsigned int hctx_idx, unsigned int request_idx,
+ unsigned int numa_node)
+{
+ return __scsi_init_request(rq, numa_node);
+}
+
+static void scsi_exit_request(void *data, struct request *rq,
+ unsigned int hctx_idx, unsigned int request_idx)
+{
+ __scsi_exit_request(rq);
+}
+
+static int scsi_init_flush_rq(struct request_queue *q, struct request *rq)
+{
+ return __scsi_init_request(rq, NUMA_NO_NODE);
+}
+
+static void scsi_exit_flush_rq(struct request_queue *q, struct request *rq)
+{
+ __scsi_exit_request(rq);
+}
+
static u64 scsi_calculate_bounce_limit(struct Scsi_Host *shost)
{
struct device *host_dev;
@@ -2044,6 +2064,8 @@ static struct blk_mq_ops scsi_mq_ops = {
.timeout= scsi_times_out,
.init_request = scsi_init_request,
.exit_request = scsi_exit_request,
+ .init_flush_rq = scsi_init_flush_rq,
+ .exit_flush_rq = scsi_exit_flush_rq,
};
struct request_queue *scsi_mq_alloc_queue(struct scsi_device *sdev)
--
1.7.9.5
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/6] blk-mq: don't copy pdu if init_flush_rq is implemented
Current copying serves purpose of initializing flush req's pdu, so don't do that if init_flush_rq is implemented. Signed-off-by: Ming Lei ming@canonical.com --- block/blk-mq.c |6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/block/blk-mq.c b/block/blk-mq.c index 1daef32..113d58d 100644 --- a/block/blk-mq.c +++ b/block/blk-mq.c @@ -292,8 +292,10 @@ void blk_mq_clone_flush_request(struct request *flush_rq, flush_rq-mq_ctx = orig_rq-mq_ctx; flush_rq-tag = orig_rq-tag; - memcpy(blk_mq_rq_to_pdu(flush_rq), blk_mq_rq_to_pdu(orig_rq), - hctx-cmd_size); + + if (!orig_rq-q-mq_ops-init_flush_rq) + memcpy(blk_mq_rq_to_pdu(flush_rq), + blk_mq_rq_to_pdu(orig_rq), hctx-cmd_size); } inline void __blk_mq_end_io(struct request *rq, int error) -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/6] blk-mq: introduce init_flush_rq callback and its pair
Currently pdu of the flush rq is simlpy copied from another rq,
turns out it isn't a good approach:
- it isn't enough to initialize pointer field well, and
easy to cause bugs if some pointers filed are included
in pdu
- the copy isn't necessary, because the pdu should just
serve for submitting flush req purpose, like non-flush
case, and flush case of legacy block driver
- actually init_flush_rq/exit_flush_rq is needed for
seting up flush request.
So introduce these two callbacks for driver to handle the case
easily.
Signed-off-by: Ming Lei ming@canonical.com
---
block/blk-flush.c | 11 +++
block/blk-mq.c |2 ++
block/blk-mq.h |1 +
include/linux/blk-mq.h |9 +
4 files changed, 23 insertions(+)
diff --git a/block/blk-flush.c b/block/blk-flush.c
index 75ca6cd0..14654e1 100644
--- a/block/blk-flush.c
+++ b/block/blk-flush.c
@@ -485,5 +485,16 @@ int blk_mq_init_flush(struct request_queue *q)
GFP_KERNEL);
if (!q-flush_rq)
return -ENOMEM;
+
+ if (q-mq_ops-init_flush_rq)
+ return q-mq_ops-init_flush_rq(q, q-flush_rq);
+
return 0;
}
+
+void blk_mq_exit_flush(struct request_queue *q)
+{
+ /* flush_rq is freed centrally for both mq and legacy queue */
+ if (q-mq_ops-exit_flush_rq)
+ q-mq_ops-exit_flush_rq(q, q-flush_rq);
+}
diff --git a/block/blk-mq.c b/block/blk-mq.c
index 23386c0..1daef32 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -1868,6 +1868,8 @@ void blk_mq_free_queue(struct request_queue *q)
{
struct blk_mq_tag_set *set = q-tag_set;
+ blk_mq_exit_flush(q);
+
blk_mq_del_queue_tag_set(q);
blk_mq_exit_hw_queues(q, set, set-nr_hw_queues);
diff --git a/block/blk-mq.h b/block/blk-mq.h
index b0bd9bc..ea9974f 100644
--- a/block/blk-mq.h
+++ b/block/blk-mq.h
@@ -28,6 +28,7 @@ struct blk_mq_ctx {
void __blk_mq_complete_request(struct request *rq);
void blk_mq_run_hw_queue(struct blk_mq_hw_ctx *hctx, bool async);
int blk_mq_init_flush(struct request_queue *q);
+void blk_mq_exit_flush(struct request_queue *q);
void blk_mq_freeze_queue(struct request_queue *q);
void blk_mq_free_queue(struct request_queue *q);
void blk_mq_clone_flush_request(struct request *flush_rq,
diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
index a1e31f2..d24c13e 100644
--- a/include/linux/blk-mq.h
+++ b/include/linux/blk-mq.h
@@ -85,6 +85,8 @@ typedef int (init_request_fn)(void *, struct request *,
unsigned int,
unsigned int, unsigned int);
typedef void (exit_request_fn)(void *, struct request *, unsigned int,
unsigned int);
+typedef int (init_flush_rq_fn)(struct request_queue *, struct request *);
+typedef void (exit_flush_rq_fn)(struct request_queue *, struct request *);
struct blk_mq_ops {
/*
@@ -119,6 +121,13 @@ struct blk_mq_ops {
*/
init_request_fn *init_request;
exit_request_fn *exit_request;
+
+ /*
+* Called after flush request allocated by the block layer to
+* allow the driver to set up driver specific data.
+*/
+ init_flush_rq_fn*init_flush_rq;
+ exit_flush_rq_fn*exit_flush_rq;
};
enum {
--
1.7.9.5
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: WARNING in block layer triggered in 3.17-rc3
On Sat, Sep 6, 2014 at 1:45 AM, Alan Stern st...@rowland.harvard.edu wrote: James and Jens: I got a WARNING when unbinding the sd driver from a USB flash drive and then binding it back again. Here's where the flash drive gets probed initially: [ 143.300886] usb-storage 4-8:1.0: usb_probe_interface [ 143.300911] usb-storage 4-8:1.0: usb_probe_interface - got id [ 143.300930] usb-storage 4-8:1.0: USB Mass Storage device detected [ 143.318239] scsi host0: usb-storage 4-8:1.0 [ 143.359979] scsi 0:0:0:0: Direct-Access Ut165USB2FlashStorage 0.00 PQ: 0 ANSI: 2 [ 143.376366] usbcore: registered new interface driver usb-storage [ 143.468464] sd 0:0:0:0: [sda] 7892040 512-byte logical blocks: (4.04 GB/3.76 GiB) [ 143.481725] sd 0:0:0:0: [sda] Write Protect is off [ 143.485712] sd 0:0:0:0: [sda] Mode Sense: 00 00 00 00 [ 143.487064] sd 0:0:0:0: [sda] Asking for cache data failed [ 143.498428] sd 0:0:0:0: [sda] Assuming drive cache: write through [ 143.656797] sda: sda1 [ 143.676922] sd 0:0:0:0: [sda] Attached SCSI removable disk Then I did echo 0:0:0:0 /sys/bus/scsi/drivers/sd_mod/unbind followed by echo 0:0:0:0 /sys/bus/scsi/drivers/sd_mod/bind I can reproduce it on virtio-blk too with scsi-mq, and can't duplicate it on non-scsi devices, so looks there are refcounts leak in scsi subsystem? Thanks, -- Ming Lei -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] libata: consolidate ata_dev_classify()
On 09/06/2014 02:52 PM, Tejun Heo wrote: Hello, On Sat, Sep 06, 2014 at 10:21:51AM +0200, Hannes Reinecke wrote: Well, yes, in principle. I was looking into that, too. But then I figured that moving to ata_taskfile would be a major overhaul for libsas, which would be quite beyond scope here. And all for a puny little patch. Hmm? Just allocate a ata_taskfile stuct on stack when invoking the function. The change would be a lot smaller actually. Which was actually my first attempt, but then I figured I'd be increasing the stacksize in doing so. But sure, if you're okay with it I'll be redoing the patch. Cheers, Hannes -- Dr. Hannes Reinecke zSeries Storage h...@suse.de +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg) -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] libiscsi: fix potential buffer overrun in __iscsi_conn_send_pdu
On 9/3/2014 8:00 AM, micha...@cs.wisc.edu wrote:
From: Mike Christie micha...@cs.wisc.edu
This patches fixes a potential buffer overrun in __iscsi_conn_send_pdu.
This function is used by iscsi drivers and userspace to send iscsi PDUs/
commands. For login commands, we have a set buffer size. For all other
commands we do not support data buffers.
This was reported by Dan Carpenter here:
http://www.spinics.net/lists/linux-scsi/msg66838.html
Reported-by: Dan Carpenter dan.carpen...@oracle.com
Signed-off-by: Mike Christie micha...@cs.wisc.edu
---
drivers/scsi/libiscsi.c | 10 ++
1 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c
index ea025e4..191b597 100644
--- a/drivers/scsi/libiscsi.c
+++ b/drivers/scsi/libiscsi.c
@@ -717,11 +717,21 @@ __iscsi_conn_send_pdu(struct iscsi_conn *conn, struct
iscsi_hdr *hdr,
return NULL;
}
+ if (data_size ISCSI_DEF_MAX_RECV_SEG_LEN) {
+ iscsi_conn_printk(KERN_ERR, conn, Invalid buffer len of %u
for login task. Max len is %u\n, data_size, ISCSI_DEF_MAX_RECV_SEG_LEN);
+ return NULL;
+ }
+
task = conn-login_task;
} else {
if (session-state != ISCSI_STATE_LOGGED_IN)
return NULL;
+ if (data_size != 0) {
+ iscsi_conn_printk(KERN_ERR, conn, Can not send data buffer
of len %u for op 0x%x\n, data_size, opcode);
+ return NULL;
+ }
+
BUG_ON(conn-c_stage == ISCSI_CONN_INITIAL_STAGE);
BUG_ON(conn-c_stage == ISCSI_CONN_STOPPED);
Looks good to me too,
Reviewed-by: Sagi Grimberg sa...@mellanox.com
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] libata: consolidate ata_dev_classify()
On Sun, Sep 07, 2014 at 01:24:47PM +0200, Hannes Reinecke wrote: Which was actually my first attempt, but then I figured I'd be increasing the stacksize in doing so. But sure, if you're okay with it I'll be redoing the patch. The struct is only 32 bytes. I don't think it's gonna make any meaningful difference. Thanks. -- tejun -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 14/20] scsi: use local buffer for printing CDB
On Wed, Sep 03, 2014 at 12:06:09PM +0200, Hannes Reinecke wrote:
The CDB needs to be printed in one line (ie with one printk
statement) to avoid the individual bytes to be broken up
under high load.
As using individual printk() statements here would lead to
unnecessary complicated code and needs the stack space to
hold the format string we should be using a local buffer
here. If the CDB is longer than the provided buffer
it will be printed in several lines.
Some general comments:
- as pointed out by YUNOMAE-san we need a constant for the buffer. And
I'd also like to see a comment why exactly you're chosing 80 for it.
- I'd really like to see some worst case stack usage analysis of the
old vs the new case.
@@ -189,6 +189,7 @@ ch_do_scsi(scsi_changer *ch, unsigned char *cmd,
{
int errno, retries = 0, timeout, result;
struct scsi_sense_hdr sshdr;
+ char buf[80];
timeout = (cmd[0] == INITIALIZE_ELEMENT_STATUS)
? timeout_init : timeout_move;
@@ -196,8 +197,8 @@ ch_do_scsi(scsi_changer *ch, unsigned char *cmd,
retry:
errno = 0;
if (debug) {
- DPRINTK(command: );
- __scsi_print_command(cmd);
+ __scsi_print_command(cmd, buf, 80);
+ DPRINTK(command: %s, buf);
The buffer variable should be inside the if (debug) here.
+/* attempt to guess cdb length if cdb_len==0 */
cdb_len == 0 is only passed in from __scsi_print_command. But as far
as I can tell all of it's caller have it easily available, so we should
just pass it in and get rid of this special case.
+static int print_opcode_name(unsigned char * cdbp, int cdb_len,
+ char *buf, int buf_len)
This doesn't print anything now, but just formats the CDB, so it
should be called format_opcode_name. Also as a convention pass
buf and buf_len as the first two arguments similar to s*printf,
and fix up the formatting for the cdbp argument. Can you please
just run your next series through checkpatch?
-void __scsi_print_command(unsigned char *cdb)
+void __scsi_print_command(unsigned char *cdb, char *buf, int buf_len)
Rename this to scsi_format_command, pass buf and buf_len as first two
argument, and as mention earlier pass in the cdb length as well.
{
+ int len, off;
+ off = print_opcode_name(cdb, 0, buf, buf_len);
len = scsi_command_size(cdb);
+ /* Variable length CDBs are not supported */
Why?
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 16/20] scsi: separate out scsi_retval_string()
On Wed, Sep 03, 2014 at 12:06:11PM +0200, Hannes Reinecke wrote: Implement scsi_retval_string() to simplify logging. Needs a way better patch description. What exactly is this going to be useful for? -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 18/20] scsi: remove scsi_show_result()
On Wed, Sep 03, 2014 at 12:06:13PM +0200, Hannes Reinecke wrote:
scsi_show_result() was only ever used in one place in sd.c.
So open-code scsi_show_result() in sd.c and remove it from
constants.c.
This does a lot more refactoring in sd.c, which needs a proper
explanation and probably splitting into a separate patch.
I also think we're much better having a different prototype
for scsi_print_result and use it here, similar to your
sdev_prefix_printk:
void scsi_print_result(struct scsi_device *sdev, const char *prefix)
{
...
}
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 19/20] sd: Reduce logging output
On Wed, Sep 03, 2014 at 12:06:14PM +0200, Hannes Reinecke wrote: There is no need to print out the command result verbatim; that will be done by the scsi stack if required. Here we just should log the result in short if requested. Looks good, Reviewed-by: Christoph Hellwig h...@lst.de -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/7] Use 'Scsi_Host' as argument for host reset
Did you plan to get back to this series and revisit it per the comments? I'd love to at least get the first preparatory patches in ASAP. -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] scsi_scan: Send TEST UNIT READY to the LUN before scanning
Looks like you put a version into the SuSE tree with a blacklist entry just for the Fujitsu array that requires the TEST UNIT READY. Can you send that version upstream as well? -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
I/O path cleanup
This series cleans up a couple of lose ends I noticed during the scsi-mq work, but which weren't important enough to address during the last cycle. -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/8] scsi: don't use scsi_next_command in scsi_reset_provider
scsi_reset_provider already manually runs all queues for the given host,
so it doesn't need the scsi_run_queues call from it, and it doesn't need
a reference on the device because it's synchronous.
So let's just call scsi_put_command directly and avoid the device reference
dance to simplify the code.
Signed-off-by: Christoph Hellwig h...@lst.de
---
drivers/scsi/scsi_error.c | 9 +
1 file changed, 1 insertion(+), 8 deletions(-)
diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
index 6b20ef3..14cece9 100644
--- a/drivers/scsi/scsi_error.c
+++ b/drivers/scsi/scsi_error.c
@@ -2317,15 +2317,9 @@ scsi_reset_provider(struct scsi_device *dev, int flag)
if (scsi_autopm_get_host(shost) 0)
return FAILED;
- if (!get_device(dev-sdev_gendev)) {
- rtn = FAILED;
- goto out_put_autopm_host;
- }
-
scmd = scsi_get_command(dev, GFP_KERNEL);
if (!scmd) {
rtn = FAILED;
- put_device(dev-sdev_gendev);
goto out_put_autopm_host;
}
@@ -2381,10 +2375,9 @@ scsi_reset_provider(struct scsi_device *dev, int flag)
waking up host to restart after TMF\n));
wake_up(shost-host_wait);
-
scsi_run_host_queues(shost);
- scsi_next_command(scmd);
+ scsi_put_command(scmd);
out_put_autopm_host:
scsi_autopm_put_host(shost);
return rtn;
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/8] scsi: remove scsi_next_command
There's only one caller left, so inline it and reduce the blk-mq vs !blk-mq
diff a litte bit.
Signed-off-by: Christoph Hellwig h...@lst.de
---
drivers/scsi/scsi_lib.c | 18 --
drivers/scsi/scsi_priv.h | 1 -
2 files changed, 4 insertions(+), 15 deletions(-)
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 8b76231..f21e661 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -542,17 +542,6 @@ static void scsi_requeue_command(struct request_queue *q,
struct scsi_cmnd *cmd)
put_device(sdev-sdev_gendev);
}
-void scsi_next_command(struct scsi_cmnd *cmd)
-{
- struct scsi_device *sdev = cmd-device;
- struct request_queue *q = sdev-request_queue;
-
- scsi_put_command(cmd);
- scsi_run_queue(q);
-
- put_device(sdev-sdev_gendev);
-}
-
void scsi_run_host_queues(struct Scsi_Host *shost)
{
struct scsi_device *sdev;
@@ -730,8 +719,6 @@ static bool scsi_end_request(struct request *req, int error,
kblockd_schedule_work(sdev-requeue_work);
else
blk_mq_start_stopped_hw_queues(q, true);
-
- put_device(sdev-sdev_gendev);
} else {
unsigned long flags;
@@ -742,9 +729,12 @@ static bool scsi_end_request(struct request *req, int
error,
if (bidi_bytes)
scsi_release_bidi_buffers(cmd);
scsi_release_buffers(cmd);
- scsi_next_command(cmd);
+
+ scsi_put_command(cmd);
+ scsi_run_queue(q);
}
+ put_device(sdev-sdev_gendev);
return false;
}
diff --git a/drivers/scsi/scsi_priv.h b/drivers/scsi/scsi_priv.h
index 12b8e1b..2a382c1 100644
--- a/drivers/scsi/scsi_priv.h
+++ b/drivers/scsi/scsi_priv.h
@@ -84,7 +84,6 @@ int scsi_noretry_cmd(struct scsi_cmnd *scmd);
extern int scsi_maybe_unblock_host(struct scsi_device *sdev);
extern void scsi_device_unbusy(struct scsi_device *sdev);
extern void scsi_queue_insert(struct scsi_cmnd *cmd, int reason);
-extern void scsi_next_command(struct scsi_cmnd *cmd);
extern void scsi_io_completion(struct scsi_cmnd *, unsigned int);
extern void scsi_run_host_queues(struct Scsi_Host *shost);
extern struct request_queue *scsi_alloc_queue(struct scsi_device *sdev);
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 6/8] scsi: move more requeue handling into scsi_requeue_command
Move a bit code out of scsi_io_completion and into scsi_requeue_command
in preparation for further refactoring.
Signed-off-by: Christoph Hellwig h...@lst.de
---
drivers/scsi/scsi_lib.c | 85 ++---
1 file changed, 38 insertions(+), 47 deletions(-)
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index c398343..2221bf1 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -506,42 +506,6 @@ void scsi_requeue_run_queue(struct work_struct *work)
scsi_run_queue(q);
}
-/*
- * Function: scsi_requeue_command()
- *
- * Purpose:Handle post-processing of completed commands.
- *
- * Arguments: q - queue to operate on
- * cmd - command that may need to be requeued.
- *
- * Returns:Nothing
- *
- * Notes: After command completion, there may be blocks left
- * over which weren't finished by the previous command
- * this can be for a number of reasons - the main one is
- * I/O errors in the middle of the request, in which case
- * we need to request the blocks that come after the bad
- * sector.
- * Notes: Upon return, cmd is a stale pointer.
- */
-static void scsi_requeue_command(struct request_queue *q, struct scsi_cmnd
*cmd)
-{
- struct scsi_device *sdev = cmd-device;
- struct request *req = cmd-request;
- unsigned long flags;
-
- spin_lock_irqsave(q-queue_lock, flags);
- blk_unprep_request(req);
- req-special = NULL;
- scsi_put_command(cmd);
- blk_requeue_request(q, req);
- spin_unlock_irqrestore(q-queue_lock, flags);
-
- scsi_run_queue(q);
-
- put_device(sdev-sdev_gendev);
-}
-
void scsi_run_host_queues(struct Scsi_Host *shost)
{
struct scsi_device *sdev;
@@ -647,6 +611,43 @@ static void scsi_mq_uninit_cmd(struct scsi_cmnd *cmd)
}
}
+/**
+ * scsi_requeue_command - requeue a command from the I/O completion path.
+ * @cmd: command that needs to be requeued
+ *
+ * After command completion, there may be blocks left over which weren't
+ * finished by the previous command this can be for a number of reasons -
+ * the main one is I/O errors in the middle of the request, in which case
+ * we need to request the blocks that come after the bad sector.
+ */
+static void scsi_requeue_command(struct scsi_cmnd *cmd)
+{
+ struct request *req = cmd-request;
+ struct request_queue *q = req-q;
+
+ scsi_free_sgtables(cmd);
+
+ if (q-mq_ops) {
+ cmd-request-cmd_flags = ~REQ_DONTPREP;
+ scsi_mq_uninit_cmd(cmd);
+ scsi_mq_requeue_cmd(cmd);
+ } else {
+ struct scsi_device *sdev = cmd-device;
+ unsigned long flags;
+
+ spin_lock_irqsave(q-queue_lock, flags);
+ blk_unprep_request(req);
+ req-special = NULL;
+ scsi_put_command(cmd);
+ blk_requeue_request(q, req);
+ spin_unlock_irqrestore(q-queue_lock, flags);
+
+ scsi_run_queue(q);
+
+ put_device(sdev-sdev_gendev);
+ }
+}
+
static bool scsi_end_request(struct request *req, int error,
unsigned int bytes, unsigned int bidi_bytes)
{
@@ -773,7 +774,6 @@ static int __scsi_error_from_host_byte(struct scsi_cmnd
*cmd, int result)
void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes)
{
int result = cmd-result;
- struct request_queue *q = cmd-device-request_queue;
struct request *req = cmd-request;
int error = 0;
struct scsi_sense_hdr sshdr;
@@ -995,16 +995,7 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned
int good_bytes)
/*FALLTHRU*/
case ACTION_REPREP:
requeue:
- /* Unprep the request and put it back at the head of the queue.
-* A new command will be prepared and issued.
-*/
- scsi_free_sgtables(cmd);
- if (q-mq_ops) {
- cmd-request-cmd_flags = ~REQ_DONTPREP;
- scsi_mq_uninit_cmd(cmd);
- scsi_mq_requeue_cmd(cmd);
- } else
- scsi_requeue_command(q, cmd);
+ scsi_requeue_command(cmd);
break;
case ACTION_RETRY:
/* Retry the same command immediately */
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/8] scsi: stop passing a gfp_mask argument down the command setup path
There is no reason for ULDs to pass in a flag on how to allocate the S/G
lists. While we don't need GFP_ATOMIC for the blk-mq case because we
don't hold locks, that decision can be made way down the chain without
having to pass a pointless gfp_mask argument.
Signed-off-by: Christoph Hellwig h...@lst.de
---
drivers/scsi/scsi_lib.c | 20 +---
drivers/scsi/sd.c| 6 +++---
drivers/scsi/sr.c| 2 +-
include/scsi/scsi_cmnd.h | 2 +-
4 files changed, 14 insertions(+), 16 deletions(-)
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 0062865..877ea3d 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -587,10 +587,10 @@ static void scsi_free_sgtable(struct scsi_data_buffer
*sdb, bool mq)
__sg_free_table(sdb-table, SCSI_MAX_SG_SEGMENTS, mq, scsi_sg_free);
}
-static int scsi_alloc_sgtable(struct scsi_data_buffer *sdb, int nents,
- gfp_t gfp_mask, bool mq)
+static int scsi_alloc_sgtable(struct scsi_data_buffer *sdb, int nents, bool mq)
{
struct scatterlist *first_chunk = NULL;
+ gfp_t gfp_mask = mq ? GFP_NOIO : GFP_ATOMIC;
int ret;
BUG_ON(!nents);
@@ -1017,8 +1017,7 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned
int good_bytes)
}
}
-static int scsi_init_sgtable(struct request *req, struct scsi_data_buffer *sdb,
-gfp_t gfp_mask)
+static int scsi_init_sgtable(struct request *req, struct scsi_data_buffer *sdb)
{
int count;
@@ -1026,7 +1025,7 @@ static int scsi_init_sgtable(struct request *req, struct
scsi_data_buffer *sdb,
* If sg table allocation fails, requeue request later.
*/
if (unlikely(scsi_alloc_sgtable(sdb, req-nr_phys_segments,
- gfp_mask, req-mq_ctx != NULL)))
+ req-mq_ctx != NULL)))
return BLKPREP_DEFER;
/*
@@ -1051,7 +1050,7 @@ static int scsi_init_sgtable(struct request *req, struct
scsi_data_buffer *sdb,
* BLKPREP_DEFER if the failure is retryable
* BLKPREP_KILL if the failure is fatal
*/
-int scsi_init_io(struct scsi_cmnd *cmd, gfp_t gfp_mask)
+int scsi_init_io(struct scsi_cmnd *cmd)
{
struct scsi_device *sdev = cmd-device;
struct request *rq = cmd-request;
@@ -1060,7 +1059,7 @@ int scsi_init_io(struct scsi_cmnd *cmd, gfp_t gfp_mask)
BUG_ON(!rq-nr_phys_segments);
- error = scsi_init_sgtable(rq, cmd-sdb, gfp_mask);
+ error = scsi_init_sgtable(rq, cmd-sdb);
if (error)
goto err_exit;
@@ -1076,8 +1075,7 @@ int scsi_init_io(struct scsi_cmnd *cmd, gfp_t gfp_mask)
rq-next_rq-special = bidi_sdb;
}
- error = scsi_init_sgtable(rq-next_rq, rq-next_rq-special,
- GFP_ATOMIC);
+ error = scsi_init_sgtable(rq-next_rq, rq-next_rq-special);
if (error)
goto err_exit;
}
@@ -1089,7 +1087,7 @@ int scsi_init_io(struct scsi_cmnd *cmd, gfp_t gfp_mask)
BUG_ON(prot_sdb == NULL);
ivecs = blk_rq_count_integrity_sg(rq-q, rq-bio);
- if (scsi_alloc_sgtable(prot_sdb, ivecs, gfp_mask, is_mq)) {
+ if (scsi_alloc_sgtable(prot_sdb, ivecs, is_mq)) {
error = BLKPREP_DEFER;
goto err_exit;
}
@@ -1156,7 +1154,7 @@ static int scsi_setup_blk_pc_cmnd(struct scsi_device
*sdev, struct request *req)
* submit a request without an attached bio.
*/
if (req-bio) {
- int ret = scsi_init_io(cmd, GFP_ATOMIC);
+ int ret = scsi_init_io(cmd);
if (unlikely(ret))
return ret;
} else {
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index aa43496..6f7588d 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -769,7 +769,7 @@ static int sd_setup_discard_cmnd(struct scsi_cmnd *cmd)
* amount of blocks described by the request.
*/
blk_add_request_payload(rq, page, len);
- ret = scsi_init_io(cmd, GFP_ATOMIC);
+ ret = scsi_init_io(cmd);
rq-__data_len = nr_bytes;
out:
@@ -863,7 +863,7 @@ static int sd_setup_write_same_cmnd(struct scsi_cmnd *cmd)
* knows how much to actually write.
*/
rq-__data_len = sdp-sector_size;
- ret = scsi_init_io(cmd, GFP_ATOMIC);
+ ret = scsi_init_io(cmd);
rq-__data_len = nr_bytes;
return ret;
}
@@ -896,7 +896,7 @@ static int sd_setup_read_write_cmnd(struct scsi_cmnd *SCpnt)
int ret, host_dif;
unsigned char protect;
- ret = scsi_init_io(SCpnt, GFP_ATOMIC);
+ ret = scsi_init_io(SCpnt);
if (ret != BLKPREP_OK)
goto out;
SCpnt = rq-special;
diff --git
[PATCH 5/8] scsi: move scsi_dispatch_cmd to scsi_lib.c
scsi_lib.c is where the rest of the I/O submission path lives, so move
scsi_dispatch_cmd there and mark it static.
Signed-off-by: Christoph Hellwig h...@lst.de
---
drivers/scsi/scsi.c | 81
drivers/scsi/scsi_lib.c | 81
drivers/scsi/scsi_priv.h | 1 -
3 files changed, 81 insertions(+), 82 deletions(-)
diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
index 534bf26..a86ccb7 100644
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -626,87 +626,6 @@ void scsi_cmd_get_serial(struct Scsi_Host *host, struct
scsi_cmnd *cmd)
EXPORT_SYMBOL(scsi_cmd_get_serial);
/**
- * scsi_dispatch_command - Dispatch a command to the low-level driver.
- * @cmd: command block we are dispatching.
- *
- * Return: nonzero return request was rejected and device's queue needs to be
- * plugged.
- */
-int scsi_dispatch_cmd(struct scsi_cmnd *cmd)
-{
- struct Scsi_Host *host = cmd-device-host;
- int rtn = 0;
-
- atomic_inc(cmd-device-iorequest_cnt);
-
- /* check if the device is still usable */
- if (unlikely(cmd-device-sdev_state == SDEV_DEL)) {
- /* in SDEV_DEL we error all commands. DID_NO_CONNECT
-* returns an immediate error upwards, and signals
-* that the device is no longer present */
- cmd-result = DID_NO_CONNECT 16;
- goto done;
- }
-
- /* Check to see if the scsi lld made this device blocked. */
- if (unlikely(scsi_device_blocked(cmd-device))) {
- /*
-* in blocked state, the command is just put back on
-* the device queue. The suspend state has already
-* blocked the queue so future requests should not
-* occur until the device transitions out of the
-* suspend state.
-*/
- SCSI_LOG_MLQUEUE(3, scmd_printk(KERN_INFO, cmd,
- queuecommand : device blocked\n));
- return SCSI_MLQUEUE_DEVICE_BUSY;
- }
-
- /* Store the LUN value in cmnd, if needed. */
- if (cmd-device-lun_in_cdb)
- cmd-cmnd[1] = (cmd-cmnd[1] 0x1f) |
- (cmd-device-lun 5 0xe0);
-
- scsi_log_send(cmd);
-
- /*
-* Before we queue this command, check if the command
-* length exceeds what the host adapter can handle.
-*/
- if (cmd-cmd_len cmd-device-host-max_cmd_len) {
- SCSI_LOG_MLQUEUE(3, scmd_printk(KERN_INFO, cmd,
- queuecommand : command too long.
- cdb_size=%d host-max_cmd_len=%d\n,
- cmd-cmd_len, cmd-device-host-max_cmd_len));
- cmd-result = (DID_ABORT 16);
- goto done;
- }
-
- if (unlikely(host-shost_state == SHOST_DEL)) {
- cmd-result = (DID_NO_CONNECT 16);
- goto done;
-
- }
-
- trace_scsi_dispatch_cmd_start(cmd);
- rtn = host-hostt-queuecommand(host, cmd);
- if (rtn) {
- trace_scsi_dispatch_cmd_error(cmd, rtn);
- if (rtn != SCSI_MLQUEUE_DEVICE_BUSY
- rtn != SCSI_MLQUEUE_TARGET_BUSY)
- rtn = SCSI_MLQUEUE_HOST_BUSY;
-
- SCSI_LOG_MLQUEUE(3, scmd_printk(KERN_INFO, cmd,
- queuecommand : request rejected\n));
- }
-
- return rtn;
- done:
- cmd-scsi_done(cmd);
- return 0;
-}
-
-/**
* scsi_finish_command - cleanup and pass command back to upper layer
* @cmd: the command
*
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 877ea3d..c398343 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1581,6 +1581,87 @@ static void scsi_softirq_done(struct request *rq)
}
/**
+ * scsi_dispatch_command - Dispatch a command to the low-level driver.
+ * @cmd: command block we are dispatching.
+ *
+ * Return: nonzero return request was rejected and device's queue needs to be
+ * plugged.
+ */
+static int scsi_dispatch_cmd(struct scsi_cmnd *cmd)
+{
+ struct Scsi_Host *host = cmd-device-host;
+ int rtn = 0;
+
+ atomic_inc(cmd-device-iorequest_cnt);
+
+ /* check if the device is still usable */
+ if (unlikely(cmd-device-sdev_state == SDEV_DEL)) {
+ /* in SDEV_DEL we error all commands. DID_NO_CONNECT
+* returns an immediate error upwards, and signals
+* that the device is no longer present */
+ cmd-result = DID_NO_CONNECT 16;
+ goto done;
+ }
+
+ /* Check to see if the scsi lld made this device blocked. */
+ if (unlikely(scsi_device_blocked(cmd-device))) {
+ /*
+* in blocked state, the command is just put back on
+* the device queue. The suspend state has
[PATCH 3/8] scsi: clean up S/G table freeing
Now that we are using the split completion model for the legacy request
path as well we can use scsi_mq_free_sgtables unconditionally.
Rename it to scsi_free_sgtables, use it for the legacy path and remove
scsi_release_(bidi_)buffers.
Signed-off-by: Christoph Hellwig h...@lst.de
---
drivers/scsi/scsi_lib.c | 76 +++--
1 file changed, 17 insertions(+), 59 deletions(-)
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index f21e661..0062865 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -621,7 +621,7 @@ static void scsi_uninit_cmd(struct scsi_cmnd *cmd)
}
}
-static void scsi_mq_free_sgtables(struct scsi_cmnd *cmd)
+static void scsi_free_sgtables(struct scsi_cmnd *cmd)
{
if (cmd-sdb.table.nents)
scsi_free_sgtable(cmd-sdb, true);
@@ -637,7 +637,6 @@ static void scsi_mq_uninit_cmd(struct scsi_cmnd *cmd)
struct Scsi_Host *shost = sdev-host;
unsigned long flags;
- scsi_mq_free_sgtables(cmd);
scsi_uninit_cmd(cmd);
if (shost-use_cmd_list) {
@@ -648,42 +647,6 @@ static void scsi_mq_uninit_cmd(struct scsi_cmnd *cmd)
}
}
-/*
- * Function:scsi_release_buffers()
- *
- * Purpose: Free resources allocate for a scsi_command.
- *
- * Arguments: cmd- command that we are bailing.
- *
- * Lock status: Assumed that no lock is held upon entry.
- *
- * Returns: Nothing
- *
- * Notes: In the event that an upper level driver rejects a
- * command, we must release resources allocated during
- * the __init_io() function. Primarily this would involve
- * the scatter-gather table.
- */
-static void scsi_release_buffers(struct scsi_cmnd *cmd)
-{
- if (cmd-sdb.table.nents)
- scsi_free_sgtable(cmd-sdb, false);
-
- memset(cmd-sdb, 0, sizeof(cmd-sdb));
-
- if (scsi_prot_sg_count(cmd))
- scsi_free_sgtable(cmd-prot_sdb, false);
-}
-
-static void scsi_release_bidi_buffers(struct scsi_cmnd *cmd)
-{
- struct scsi_data_buffer *bidi_sdb = cmd-request-next_rq-special;
-
- scsi_free_sgtable(bidi_sdb, false);
- kmem_cache_free(scsi_sdb_cache, bidi_sdb);
- cmd-request-next_rq-special = NULL;
-}
-
static bool scsi_end_request(struct request *req, int error,
unsigned int bytes, unsigned int bidi_bytes)
{
@@ -702,16 +665,14 @@ static bool scsi_end_request(struct request *req, int
error,
if (blk_queue_add_random(q))
add_disk_randomness(req-rq_disk);
+ /*
+* In the MQ case the command gets freed by __blk_mq_end_io, so we have
+* to do all cleanup that depends on the command before that.
+*/
+ scsi_free_sgtables(cmd);
+
if (req-mq_ctx) {
- /*
-* In the MQ case the command gets freed by __blk_mq_end_io,
-* so we have to do all cleanup that depends on it earlier.
-*
-* We also can't kick the queues from irq context, so we
-* will have to defer it to a workqueue.
-*/
scsi_mq_uninit_cmd(cmd);
-
__blk_mq_end_io(req, error);
if (scsi_target(sdev)-single_lun ||
@@ -726,10 +687,6 @@ static bool scsi_end_request(struct request *req, int
error,
blk_finish_request(req, error);
spin_unlock_irqrestore(q-queue_lock, flags);
- if (bidi_bytes)
- scsi_release_bidi_buffers(cmd);
- scsi_release_buffers(cmd);
-
scsi_put_command(cmd);
scsi_run_queue(q);
}
@@ -1041,14 +998,13 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned
int good_bytes)
/* Unprep the request and put it back at the head of the queue.
* A new command will be prepared and issued.
*/
+ scsi_free_sgtables(cmd);
if (q-mq_ops) {
cmd-request-cmd_flags = ~REQ_DONTPREP;
scsi_mq_uninit_cmd(cmd);
scsi_mq_requeue_cmd(cmd);
- } else {
- scsi_release_buffers(cmd);
+ } else
scsi_requeue_command(q, cmd);
- }
break;
case ACTION_RETRY:
/* Retry the same command immediately */
@@ -1149,10 +1105,8 @@ int scsi_init_io(struct scsi_cmnd *cmd, gfp_t gfp_mask)
return BLKPREP_OK;
err_exit:
- if (is_mq) {
- scsi_mq_free_sgtables(cmd);
- } else {
- scsi_release_buffers(cmd);
+ scsi_free_sgtables(cmd);
+ if (!is_mq) {
cmd-request-special = NULL;
scsi_put_command(cmd);
put_device(sdev-sdev_gendev);
@@ -1322,7 +1276,9 @@ scsi_prep_return(struct request_queue *q, struct request
[PATCH 7/8] scsi: split error handling slow path out of scsi_io_completion
Move the error handling path out of scsi_io_completion and into an
out of line helper.
Signed-off-by: Christoph Hellwig h...@lst.de
---
drivers/scsi/scsi_lib.c | 263 +---
1 file changed, 136 insertions(+), 127 deletions(-)
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 2221bf1..cc5d404 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -742,6 +742,132 @@ static int __scsi_error_from_host_byte(struct scsi_cmnd
*cmd, int result)
return error;
}
+static noinline bool
+scsi_handle_ioerror(struct scsi_cmnd *cmd, int result,
+ struct scsi_sense_hdr *sshdr)
+{
+ struct request *req = cmd-request;
+ unsigned long wait_for = (cmd-allowed + 1) * req-timeout;
+ int error = 0;
+ enum {
+ ACTION_FAIL,
+ ACTION_REPREP,
+ ACTION_RETRY,
+ ACTION_DELAYED_RETRY,
+ } action = ACTION_FAIL;
+
+ error = __scsi_error_from_host_byte(cmd, result);
+
+ if (host_byte(result) == DID_RESET) {
+ /* Third party bus reset or reset for error recovery
+* reasons. Just retry the command and see what
+* happens.
+*/
+ action = ACTION_RETRY;
+ } else if (sshdr) {
+ switch (sshdr-sense_key) {
+ case UNIT_ATTENTION:
+ if (cmd-device-removable) {
+ /* Detected disc change. Set a bit
+* and quietly refuse further access.
+*/
+ cmd-device-changed = 1;
+ } else {
+ /* Must have been a power glitch, or a
+* bus reset. Could not have been a
+* media change, so we just retry the
+* command and see what happens.
+*/
+ action = ACTION_RETRY;
+ }
+ break;
+ case ILLEGAL_REQUEST:
+ /* If we had an ILLEGAL REQUEST returned, then
+* we may have performed an unsupported
+* command. The only thing this should be
+* would be a ten byte read where only a six
+* byte read was supported. Also, on a system
+* where READ CAPACITY failed, we may have
+* read past the end of the disk.
+*/
+ if ((cmd-device-use_10_for_rw
+ sshdr-asc == 0x20 sshdr-ascq == 0x00)
+ (cmd-cmnd[0] == READ_10 ||
+cmd-cmnd[0] == WRITE_10)) {
+ /* This will issue a new 6-byte command. */
+ cmd-device-use_10_for_rw = 0;
+ action = ACTION_REPREP;
+ } else if (sshdr-asc == 0x10) /* DIX */ {
+ error = -EILSEQ;
+ /* INVALID COMMAND OPCODE or INVALID FIELD IN CDB */
+ } else if (sshdr-asc == 0x20 || sshdr-asc == 0x24) {
+ error = -EREMOTEIO;
+ }
+ break;
+ case ABORTED_COMMAND:
+ if (sshdr-asc == 0x10) /* DIF */
+ error = -EILSEQ;
+ break;
+ case NOT_READY:
+ /* If the device is in the process of becoming
+* ready, or has a temporary blockage, retry.
+*/
+ if (sshdr-asc == 0x04) {
+ switch (sshdr-ascq) {
+ case 0x01: /* becoming ready */
+ case 0x04: /* format in progress */
+ case 0x05: /* rebuild in progress */
+ case 0x06: /* recalculation in progress */
+ case 0x07: /* operation in progress */
+ case 0x08: /* Long write in progress */
+ case 0x09: /* self test in progress */
+ case 0x14: /* space allocation in progress */
+ action = ACTION_DELAYED_RETRY;
+ break;
+ default:
+ break;
+ }
+ }
+ break;
+ case VOLUME_OVERFLOW:
+ /* See SSC3rXX or current. */
+ break;
+ default:
+
[PATCH 8/8] scsi: merge scsi_finish_command and scsi_io_completion
Signed-off-by: Christoph Hellwig h...@lst.de
---
Documentation/scsi/scsi_eh.txt | 12 +++---
drivers/scsi/scsi.c| 58 -
drivers/scsi/scsi_lib.c| 84 +++---
drivers/scsi/scsi_priv.h | 1 -
4 files changed, 59 insertions(+), 96 deletions(-)
diff --git a/Documentation/scsi/scsi_eh.txt b/Documentation/scsi/scsi_eh.txt
index a0c8511..8a7ac48 100644
--- a/Documentation/scsi/scsi_eh.txt
+++ b/Documentation/scsi/scsi_eh.txt
@@ -57,13 +57,11 @@ looks at the scmd-result value and sense data to determine
what to do
with the command.
- SUCCESS
- scsi_finish_command() is invoked for the command. The
- function does some maintenance chores and then calls
- scsi_io_completion() to finish the I/O.
- scsi_io_completion() then notifies the block layer on
- the completed request by calling blk_end_request and
- friends or figures out what to do with the remainder
- of the data in case of an error.
+ scsi_finish_command() is invoked for the command. The function
+ does some maintenance chores and then notifies the block layer on
+ the completed request by calling blk_end_request / __blk_mq_end_io
+ or figures out what to do with the remainder of the data in case
+ of an error.
- NEEDS_RETRY
- ADD_TO_MLQUEUE
diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
index a86ccb7..cdc0686 100644
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -626,64 +626,6 @@ void scsi_cmd_get_serial(struct Scsi_Host *host, struct
scsi_cmnd *cmd)
EXPORT_SYMBOL(scsi_cmd_get_serial);
/**
- * scsi_finish_command - cleanup and pass command back to upper layer
- * @cmd: the command
- *
- * Description: Pass command off to upper layer for finishing of I/O
- * request, waking processes that are waiting on results,
- * etc.
- */
-void scsi_finish_command(struct scsi_cmnd *cmd)
-{
- struct scsi_device *sdev = cmd-device;
- struct scsi_target *starget = scsi_target(sdev);
- struct Scsi_Host *shost = sdev-host;
- struct scsi_driver *drv;
- unsigned int good_bytes;
-
- scsi_device_unbusy(sdev);
-
- /*
-* Clear the flags that say that the device/target/host is no longer
-* capable of accepting new commands.
-*/
- if (atomic_read(shost-host_blocked))
- atomic_set(shost-host_blocked, 0);
- if (atomic_read(starget-target_blocked))
- atomic_set(starget-target_blocked, 0);
- if (atomic_read(sdev-device_blocked))
- atomic_set(sdev-device_blocked, 0);
-
- /*
-* If we have valid sense information, then some kind of recovery
-* must have taken place. Make a note of this.
-*/
- if (SCSI_SENSE_VALID(cmd))
- cmd-result |= (DRIVER_SENSE 24);
-
- SCSI_LOG_MLCOMPLETE(4, sdev_printk(KERN_INFO, sdev,
- Notifying upper driver of completion
- (result %x)\n, cmd-result));
-
- good_bytes = scsi_bufflen(cmd);
-if (cmd-request-cmd_type != REQ_TYPE_BLOCK_PC) {
- int old_good_bytes = good_bytes;
- drv = scsi_cmd_to_driver(cmd);
- if (drv-done)
- good_bytes = drv-done(cmd);
- /*
-* USB may not give sense identifying bad sector and
-* simply return a residue instead, so subtract off the
-* residue if drv-done() error processing indicates no
-* change to the completion length.
-*/
- if (good_bytes == old_good_bytes)
- good_bytes -= scsi_get_resid(cmd);
- }
- scsi_io_completion(cmd, good_bytes);
-}
-
-/**
* scsi_adjust_queue_depth - Let low level drivers change a device's queue
depth
* @sdev: SCSI Device in question
* @tagged: Do we use tagged queueing (non-0) or do we treat
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index cc5d404..e0eb809 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -868,44 +868,68 @@ scsi_handle_ioerror(struct scsi_cmnd *cmd, int result,
return true;
}
-/*
- * Function:scsi_io_completion()
- *
- * Purpose: Completion processing for block device I/O requests.
- *
- * Arguments: cmd - command that is finished.
- *
- * Lock status: Assumed that no lock is held upon entry.
- *
- * Returns: Nothing
- *
- * Notes: We will finish off the specified number of sectors. If we
- * are done, the command block will be released and the queue
- * function will be goosed. If we are not done then we have to
- * figure out what to do next:
- *
- * a) We can call scsi_requeue_command(). The request
- *will be unprepared and put back on the queue. Then
- *
Re: Block/SCSI data integrity update v3
On Thu, Aug 28, 2014 at 02:10:49PM -0600, Jens Axboe wrote: On 08/28/2014 01:31 PM, Martin K. Petersen wrote: This is the data integrity patch series originally submitted for 3.16 and 3.17. It has been rebased on top of block/for-3.18/core. Other than that there are no changes from v2. Thanks, picked up. I did not apply 14/14 since that should go through the SCSI tree. I can carry it if need be, but it'd be nice to have a SCSI signoff on it then. While Sagi found some minor nitpicks I think we could address them later if needed, so Acked-by: Christoph Hellwig h...@lst.de from me for picking it up through the block tree. -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
3.16.2: 2TiB Seagate Expansion Desk apparently still broken with both USB mass storage *and* UAS: some debugging output
I have a brand new Seagate Expansion Desk drive attached to my x86-64 desktop. (I also have a 4TiB model of the same drive, but I haven't even unboxed it: there seems little point as long as the 2TiB version doesn't work.) I am seeing apparently the same problem as Alexandre Oliva reported in https://bugzilla.kernel.org/show_bug.cgi?id=79511. The drive is USB ID 0bc2:3321, so probably a slightly different model than Alexandre's 0bc2:3320, but similar enough to be broken it seems. I've tried it with both the usb-storage driver on xhci and with UAS, with verbose USB mass storage debugging turned on. Both fail with different log messages: see below. I haven't tried Alexandre's quirk yet. (I have USB debugging for the UAS case only, alas, because the system helpfully autoloaded and used that module when I was trying to replicate the usb-storage-only failure, sigh. Autoloading is annoying sometimes! :) ) It's attached via an 8-port powered USB-3 hub marked only with the logo 'U-speed', possibly USB id 2109:0811. Note: this system has strange boot messages at startup which may relate to this no-name USB hub: Sep 7 18:06:05 mutilate info: : [ 33.960925] usb 6-2: reset SuperSpeed USB device number 3 using xhci_hcd Sep 7 18:06:05 mutilate warning: : [ 33.974576] xhci_hcd :04:00.0: xHCI xhci_drop_endpoint called with disabled ep 88041b418cc0 Sep 7 18:06:05 mutilate warning: : [ 33.976995] xhci_hcd :04:00.0: xHCI xhci_drop_endpoint called with disabled ep 88041b418d08 While the system is running with mass storage debugging turned on, I see a constant pulse of Sep 7 17:45:27 mutilate warning: : [ 152.048083] 00 00 00 00 00 00 Sep 7 17:45:27 mutilate warning: : [ 152.049890] (unknown ASC/ASCQ) Sep 7 17:45:27 mutilate warning: : [ 152.051075] 00 00 00 00 00 00 Sep 7 17:45:27 mutilate warning: : [ 152.053181] (unknown ASC/ASCQ) Sep 7 17:45:27 mutilate warning: : [ 152.054378] 00 00 00 00 00 00 Sep 7 17:45:27 mutilate warning: : [ 152.056299] (unknown ASC/ASCQ) Sep 7 17:45:27 mutilate warning: : [ 152.057485] 00 00 00 00 00 00 Sep 7 17:45:27 mutilate warning: : [ 152.059434] (unknown ASC/ASCQ) Sep 7 17:45:28 mutilate warning: : [ 152.559807] 4a 01 00 00 10 00 00 00 08 00 which may or may not relate to this device: it's definitely not directly associated with the failing device though, since it's emitted even when that device is off and unplugged. (I *do* have an always-mounted but generally idle USB mass storage device here, a 2TiB WD My Book Essential drive, USB ID 1058:1140: it may relate to that.) Anyway: to the problem. When mounting the Seagate Expansion Desk drive with usb-storage, I see: Sep 6 10:47:39 mutilate info: : [474931.354699] usb 6-1.1.2: new SuperSpeed USB device number 9 using xhci_hcd Sep 6 10:47:39 mutilate info: : [474931.367533] usb-storage 6-1.1.2:1.0: USB Mass Storage device detected Sep 6 10:47:39 mutilate info: : [474931.367717] scsi13 : usb-storage 6-1.1.2:1.0 Sep 6 10:47:40 mutilate notice: : [474932.368828] scsi 13:0:0:0: Direct-Access Seagate Expansion Desk 0604 PQ: 0 ANSI: 6 Sep 6 10:47:40 mutilate notice: : [474932.371900] sd 13:0:0:0: [sdh] Spinning up disk... Sep 6 10:47:44 mutilate warning: : [474933.371782] ... Sep 6 10:47:44 mutilate info: : [474935.805121] usb 6-1.1.2: reset SuperSpeed USB device number 9 using xhci_hcd Sep 6 10:47:44 mutilate warning: : [474935.816264] xhci_hcd :04:00.0: xHCI xhci_drop_endpoint called with disabled ep 8800c259ac00 Sep 6 10:47:44 mutilate warning: : [474935.816274] xhci_hcd :04:00.0: xHCI xhci_drop_endpoint called with disabled ep 8800c259ac48 Sep 6 10:47:44 mutilate warning: : [474935.823611] ready Sep 6 10:47:44 mutilate notice: : [474935.823835] sd 13:0:0:0: [sdh] 488378645 4096-byte logical blocks: (2.00 TB/1.81 TiB) Sep 6 10:47:44 mutilate notice: : [474935.843934] sd 13:0:0:0: [sdh] Write Protect is off Sep 6 10:47:44 mutilate notice: : [474935.844924] sd 13:0:0:0: [sdh] Write cache: enabled, read cache: enabled, doesn't support DPO or FUA Sep 6 10:47:44 mutilate notice: : [474935.845268] sd 13:0:0:0: [sdh] 488378645 4096-byte logical blocks: (2.00 TB/1.81 TiB) Sep 6 10:47:44 mutilate info: : [474936.190245] usb 6-1.1.2: reset SuperSpeed USB device number 9 using xhci_hcd Sep 6 10:47:44 mutilate warning: : [474936.201858] xhci_hcd :04:00.0: xHCI xhci_drop_endpoint called with disabled ep 8800c259ac00 Sep 6 10:47:44 mutilate warning: : [474936.201867] xhci_hcd :04:00.0: xHCI xhci_drop_endpoint called with disabled ep 8800c259ac48 Sep 6 10:47:44 mutilate info: : [474936.312082] usb 6-1.1.2: reset SuperSpeed USB device number 9 using xhci_hcd Sep 6 10:47:44 mutilate warning: : [474936.322885] xhci_hcd :04:00.0: xHCI xhci_drop_endpoint called with disabled ep 8800c259ac00 Sep 6 10:47:44 mutilate warning: : [474936.322895] xhci_hcd :04:00.0: xHCI xhci_drop_endpoint called with disabled ep
Re: [PATCH 0/6] blk-mq: initialize pdu of flush req explicitly
This works fine for me, although I still don't really like it very much. If you really want to go down the path of major surgery in this area we should probably allocate a flush request per hw_ctx, and initialize it using the normal init/exit functions. If we want to have proper multiqueue performance on devices needing flushes we'll need something like that anyway. -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/6] virtio-blk: implement init_flush_rq
A couple comments not directly related to this patch, it's just a convenient vehicle for my rants :) @@ -556,6 +555,19 @@ static int virtblk_init_request(void *data, struct request *rq, struct virtio_blk *vblk = data; struct virtblk_req *vbr = blk_mq_rq_to_pdu(rq); + vbr-req = rq; I really hate how we need these backpointers in most drivers. Given that struct request and the driver privata data are allocated together we should be able to do this with simple pointer arithmetics. + sg_init_table(vbr-sg, vblk-sg_elems); Jens, what do you think about moving of the SG list handling to the core block layer? I'd really like to have the S/G list in struct request, and if we do that we could also take the scsi-mq code that allows small S/G lists preallocated in blk-mq and allocating larger ones at runtime there, avoiding the need to duplicate that code and the whole mempool magic it requires in drivers that want to make use of it. -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/6] scsi-lib: implement init_flush_rq and its pair
+static int __scsi_init_request(struct request *rq, int numa_node) Nitpick: Please use a sane name here, e.g. scsi_mq_alloc/free_sense_buffer. -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: 3.16.2: 2TiB Seagate Expansion Desk apparently still broken with both USB mass storage *and* UAS: some debugging output
On Sun, 7 Sep 2014, Nix wrote: I have a brand new Seagate Expansion Desk drive attached to my x86-64 desktop. (I also have a 4TiB model of the same drive, but I haven't even unboxed it: there seems little point as long as the 2TiB version doesn't work.) I am seeing apparently the same problem as Alexandre Oliva reported in https://bugzilla.kernel.org/show_bug.cgi?id=79511. The drive is USB ID 0bc2:3321, so probably a slightly different model than Alexandre's 0bc2:3320, but similar enough to be broken it seems. I've tried it with both the usb-storage driver on xhci and with UAS, with verbose USB mass storage debugging turned on. Both fail with different log messages: see below. I haven't tried Alexandre's quirk yet. (I have USB debugging for the UAS case only, alas, because the system helpfully autoloaded and used that module when I was trying to replicate the usb-storage-only failure, sigh. Autoloading is annoying sometimes! :) ) ... I'm happy to do whatever further debugging people may deem necessary: this system has up-to-date backups and is easy to reboot and try new kernels on, and the drive is brand new and completely empty so it's pretty much impossible to mess up! Please post a usbmon trace showing what happens when the drive binds to usb-storage. To prevent extraneous data from cluttering the trace, you should unplug as many of the other USB devices on the same bus as possible before starting the trace. Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Looks like a broken hub? (was Re: 3.16.2: 2TiB Seagate Expansion Desk apparently still broken with both USB mass storage *and* UAS: some debugging output)
On 7 Sep 2014, Alan Stern spake thusly: On Sun, 7 Sep 2014, Nix wrote: I have a brand new Seagate Expansion Desk drive attached to my x86-64 desktop. (I also have a 4TiB model of the same drive, but I haven't even unboxed it: there seems little point as long as the 2TiB version doesn't work.) I am seeing apparently the same problem as Alexandre Oliva reported in https://bugzilla.kernel.org/show_bug.cgi?id=79511. The drive is USB ID 0bc2:3321, so probably a slightly different model than Alexandre's 0bc2:3320, but similar enough to be broken it seems. I've tried it with both the usb-storage driver on xhci and with UAS, with verbose USB mass storage debugging turned on. Both fail with different log messages: see below. I haven't tried Alexandre's quirk yet. (I have USB debugging for the UAS case only, alas, because the system helpfully autoloaded and used that module when I was trying to replicate the usb-storage-only failure, sigh. Autoloading is annoying sometimes! :) ) ... I'm happy to do whatever further debugging people may deem necessary: this system has up-to-date backups and is easy to reboot and try new kernels on, and the drive is brand new and completely empty so it's pretty much impossible to mess up! Please post a usbmon trace showing what happens when the drive binds to usb-storage. To prevent extraneous data from cluttering the trace, you should unplug as many of the other USB devices on the same bus as possible before starting the trace. Done. I dropped the no-name hub out of the equation by taking over my existing backup device's USB link (which also meant I could figure out the bus number, since it doesn't change while the system is running :) ). And... now it works, at least well enough to get a device file. So it's not the disk that's at fault: it's the no-name hub! (Which is, I think, USB ID 2109:0811 -- at least two instances of this disappear when I unplug the hub.) Attached, usbmon output from a successful negotiation, and a failed one via the hub. I am more than slightly annoyed. This machine only *has* two physical USB3 ports, both on the same bus, and they're almost totally inaccessible round the back, because of course you'd want your fastest ports to be hardest to reach. The whole point of this hub was to fix that stupid case-design decision. So... anyone got any suggestions for a USB-3 hub I might buy that doesn't mess things up for devices plugged into it? Do I just need one new enough that it supports UAS? (And... how on earth can I tell before buying?) Sigh. Hardware sucks. (But maybe you can quirk around this...) 88041c19a3c0 1246876544 C Ii:6:001:1 0:2048 1 = 04 88041c19a3c0 1246876553 S Ii:6:001:1 -115:2048 4 880405a47480 1246876588 S Ci:6:001:0 s a3 00 0002 0004 4 880405a47480 1246876613 C Ci:6:001:0 0 4 = 03020100 8804058ced80 1246876646 S Co:6:001:0 s 23 01 0010 0002 0 8804058ced80 124687 C Co:6:001:0 0 0 8803508b8600 1246876690 S Ci:6:001:0 s a3 00 0002 0004 4 8803508b8600 1246876712 C Ci:6:001:0 0 4 = 0302 880405a470c0 1246902105 S Ci:6:001:0 s a3 00 0002 0004 4 880405a470c0 1246902117 C Ci:6:001:0 0 4 = 0302 88009bacd900 1246928126 S Ci:6:001:0 s a3 00 0002 0004 4 88009bacd900 1246928143 C Ci:6:001:0 0 4 = 0302 8803508b83c0 1246954116 S Ci:6:001:0 s a3 00 0002 0004 4 8803508b83c0 1246954127 C Ci:6:001:0 0 4 = 0302 8803508b8480 1246980134 S Ci:6:001:0 s a3 00 0002 0004 4 8803508b8480 1246980146 C Ci:6:001:0 0 4 = 0302 8803508b8e40 1246980371 S Ci:6:001:0 s a3 00 0002 0004 4 8803508b8e40 1246980383 C Ci:6:001:0 0 4 = 0302 880405a479c0 1246980425 S Co:6:001:0 s 23 03 0004 0002 0 880405a479c0 1246980440 C Co:6:001:0 0 0 8804058cef00 1247031101 S Ci:6:001:0 s a3 00 0002 0004 4 8804058cef00 1247031121 C Ci:6:001:0 0 4 = 03021000 8803508b8e40 1247082123 S Co:6:001:0 s 23 01 0014 0002 0 8803508b8e40 1247082146 C Co:6:001:0 0 0 880405a47d80 1247082177 S Co:6:001:0 s 23 01 001d 0002 0 880405a47d80 1247082201 C Co:6:001:0 0 0 8804058cef00 1247082228 S Co:6:001:0 s 23 01 0019 0002 0 8804058cef00 1247082241 C Co:6:001:0 0 0 8803508b8480 1247082284 S Co:6:001:0 s 23 01 0010 0002 0 8803508b8480 1247082312 C Co:6:001:0 0 0 880405a47d80 1247082345 S Ci:6:001:0 s a3 00 0002 0004 4 880405a47d80 1247082356 C Ci:6:001:0 0 4 = 0302 8803508b8600 1247096118 S Ci:6:010:0 s 80 06 0100 0008 8 8803508b8600 1247096214 C Ci:6:010:0 0 8 = 12010003 0009 8800b85e1840 1247096248 S Ci:6:010:0 s 80 06 0100 0012 18 8800b85e1840 1247096339 C Ci:6:010:0 0 18 = 12010003 0009 c20b2133 00010203 0101 8800b85e1840 1247096382 S Ci:6:010:0 s 80 06 0f00 0005 5 8800b85e1840 1247096492 C Ci:6:010:0 0 5 = 050f1600 02 8800b85e1840 1247096534 S Ci:6:010:0 s 80 06 0f00 0016 22
RE: scsi-mq and 3.17rc1
-Original Message- From: Christoph Hellwig [mailto:h...@infradead.org] Sent: Monday, 25 August, 2014 9:51 AM To: Elliott, Robert (Server Storage) Cc: linux-scsi@vger.kernel.org Subject: Re: scsi-mq and 3.17rc1 On Mon, Aug 25, 2014 at 02:31:58PM +, Elliott, Robert (Server Storage) wrote: Two scsi-mq tips: 1. Several people have been wondering how to enable scsi-mq. Add this to your kernel command line (e.g., in /boot/grub/grub.conf if using grub-1): scsi_mod.use_blk_mq=Y Btw, I heard a few complaints about this, and one suggestion would be to add a config option to enable it by default. Would you or others like this? Yes. It shouldn't become a command line parameter that clutters up user boot configuration files for years. A config option will ease the transition from defaulting to off to defaulting to on. The command line parameter could override both ways: config default=N, scsi_mod.use_blk_mq=Y: on config default=N, scsi_mod.use_blk_mq=N: off config default=N, scsi_mod.use_blk_mq not specified: off config default=Y, scsi_mod.use_blk_mq=Y: on config default=Y, scsi_mod.use_blk_mq=N: off config default=Y, scsi_mod.use_blk_mq not specified: on Right now, shost-hostt-disable_blk_mq also allows the LLD to override to off if needed. Perhaps that should move to shost-disable_blk_mq so the LLD could have per-scsi_host control (e.g., let the user specify they want it off for for an HBA attached mostly to HDDs and on for an HBA attached mostly to SSDs). I also suggest that scsi_add_host_with_dma use shost_printk to indicate what was selected for each scsi_host. --- Rob ElliottHP Server Storage -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: WARNING in block layer triggered in 3.17-rc3
I think the problem is, when a gendisk is detached, its request queue
is not put in bypass mode
cause when it is re-attached, code tries to put it out of bypass mode,
hence the warning.
So either of these should work, I have not tested it, just coded it up.
diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
index 4db5abf..f94c7ba 100644
--- a/block/blk-sysfs.c
+++ b/block/blk-sysfs.c
@@ -600,6 +600,8 @@ void blk_unregister_queue(struct gendisk *disk)
if (q-request_fn)
elv_unregister_queue(q);
+ blk_queue_bypss_start(q);
+
kobject_uevent(q-kobj, KOBJ_REMOVE);
kobject_del(q-kobj);
blk_trace_remove_sysfs(disk_to_dev(disk));
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 2c2041c..1c7ef55f 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -3054,15 +3054,26 @@ static int sd_probe(struct device *dev)
static int sd_remove(struct device *dev)
{
struct scsi_disk *sdkp;
+ struct request_queue *q;
+ struct scsi_device *sdp;
dev_t devt;
sdkp = dev_get_drvdata(dev);
+ q = sdkp-disk-queue;
devt = disk_devt(sdkp-disk);
scsi_autopm_get_device(sdkp-device);
async_synchronize_full_domain(scsi_sd_pm_domain);
async_synchronize_full_domain(scsi_sd_probe_domain);
device_del(sdkp-dev);
+
+ if (q) {
+ spin_lock_irq(q-queue_lock);
+ q-bypass_depth++;
+ queue_flag_set(QUEUE_FLAG_BYPASS, q);
+ spin_unlock_irq(q-queue_lock);
+ }
+
del_gendisk(sdkp-disk);
sd_shutdown(dev);
On Fri, Sep 5, 2014 at 12:45 PM, Alan Stern st...@rowland.harvard.edu wrote:
James and Jens:
I got a WARNING when unbinding the sd driver from a USB flash drive and
then binding it back again. Here's where the flash drive gets probed
initially:
[ 143.300886] usb-storage 4-8:1.0: usb_probe_interface
[ 143.300911] usb-storage 4-8:1.0: usb_probe_interface - got id
[ 143.300930] usb-storage 4-8:1.0: USB Mass Storage device detected
[ 143.318239] scsi host0: usb-storage 4-8:1.0
[ 143.359979] scsi 0:0:0:0: Direct-Access Ut165USB2FlashStorage 0.00
PQ: 0 ANSI: 2
[ 143.376366] usbcore: registered new interface driver usb-storage
[ 143.468464] sd 0:0:0:0: [sda] 7892040 512-byte logical blocks: (4.04
GB/3.76 GiB)
[ 143.481725] sd 0:0:0:0: [sda] Write Protect is off
[ 143.485712] sd 0:0:0:0: [sda] Mode Sense: 00 00 00 00
[ 143.487064] sd 0:0:0:0: [sda] Asking for cache data failed
[ 143.498428] sd 0:0:0:0: [sda] Assuming drive cache: write through
[ 143.656797] sda: sda1
[ 143.676922] sd 0:0:0:0: [sda] Attached SCSI removable disk
Then I did
echo 0:0:0:0 /sys/bus/scsi/drivers/sd_mod/unbind
followed by
echo 0:0:0:0 /sys/bus/scsi/drivers/sd_mod/bind
which resulted in:
[ 165.079557] sd 0:0:0:0: [sda] 7892040 512-byte logical blocks: (4.04
GB/3.76 GiB)
[ 165.093510] sd 0:0:0:0: [sda] Write Protect is off
[ 165.104388] sd 0:0:0:0: [sda] Mode Sense: 00 00 00 00
[ 165.105632] sd 0:0:0:0: [sda] Asking for cache data failed
[ 165.115136] sd 0:0:0:0: [sda] Assuming drive cache: write through
[ 165.142950] sda: sda1
[ 165.156480] [ cut here ]
[ 165.159912] WARNING: CPU: 0 PID: 29 at block/blk-core.c:473
blk_queue_bypass_end+0x4d/0x62()
[ 165.160030] Modules linked in: sd_mod usb_storage scsi_mod hid_generic
usbhid hid pcspkr evdev i915 cfbfillrect cfbimgblt i2c_algo_bit cfbcopyarea
video fbcon backlight bitblit softcursor font ehci_pci drm_kms_helper
uhci_hcd ehci_hcd ohci_pci ohci_hcd drm i2ccore usbcore e100 mii usb_common
fb fbdev fan processor button thermal_sys
[ 165.160030] CPU: 0 PID: 29 Comm: kworker/u4:1 Not tainted
3.17.0-rc3AS-dirty #12
[ 165.160030] Hardware name: Hewlett-Packard HP dx2000 MT (EE004AA)/08FCh,
BIOS 1.1711/24/2005
[ 165.160030] Workqueue: events_unbound async_run_entry_fn
[ 165.160030] c105d2ef ea569e10 c12771c0 ea569e28
c102c5fb c114907d
[ 165.160030] ecc18000 ea619c20 ecc18000 ea569e38 c102c676 0009
ea569e44
[ 165.160030] c114907d ea619c20 ea569e5c c114b97d ea569e5c ea619c20
ed74e400 ea619c2c
[ 165.160030] Call Trace:
[ 165.160030] [c105d2ef] ? console_unlock+0x37e/0x3ab
[ 165.160030] [c12771c0] dump_stack+0x49/0x73
[ 165.160030] [c102c5fb] warn_slowpath_common+0x5c/0x73
[ 165.160030] [c114907d] ? blk_queue_bypass_end+0x4d/0x62
[ 165.160030] [c102c676] warn_slowpath_null+0xf/0x13
[ 165.160030] [c114907d] blk_queue_bypass_end+0x4d/0x62
[ 165.160030] [c114b97d] blk_register_queue+0x8f/0xc4
[ 165.160030] [c11548a6] add_disk+0x2bc/0x3a8
[ 165.160030] [efff40a5] sd_probe_async+0xf5/0x17b [sd_mod]
[ 165.160030] [c103fc08] async_run_entry_fn+0x59/0xf9
[ 165.160030] [c103ab22] process_one_work+0x187/0x2ac
[ 165.160030] [c103aac4] ? process_one_work+0x129/0x2ac
[ 165.160030] [c103ae19] worker_thread+0x1b1/0x26b
[ 165.160030] [c103ac68] ? process_scheduled_works+0x21/0x21
[ 165.160030] [c103e4ee] kthread+0x82/0x87
[ 165.160030] [c127b074] ? _raw_spin_unlock_irq+0x22/0x3f
[
