[patch added to the 3.12 stable tree] scsi: Fix error handling in SCSI_IOCTL_SEND_COMMAND
From: Jan Kara
This patch has been added to the 3.12 stable tree. If you have any
objections, please let us know.
===
commit 84ce0f0e94ac97217398b3b69c21c7a62ebeed05 upstream.
When sg_scsi_ioctl() fails to prepare request to submit in
blk_rq_map_kern() we jump to a label where we just end up copying
(luckily zeroed-out) kernel buffer to userspace instead of reporting
error. Fix the problem by jumping to the right label.
CC: Jens Axboe
CC: linux-scsi@vger.kernel.org
Coverity-id: 1226871
Signed-off-by: Jan Kara
Signed-off-by: Jiri Slaby
Fixed up the, now unused, out label.
Signed-off-by: Jens Axboe
---
block/scsi_ioctl.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/block/scsi_ioctl.c b/block/scsi_ioctl.c
index a5ffcc988f0b..1b4988b4bc11 100644
--- a/block/scsi_ioctl.c
+++ b/block/scsi_ioctl.c
@@ -506,7 +506,7 @@ int sg_scsi_ioctl(struct request_queue *q, struct gendisk
*disk, fmode_t mode,
if (bytes && blk_rq_map_kern(q, rq, buffer, bytes, __GFP_WAIT)) {
err = DRIVER_ERROR << 24;
- goto out;
+ goto error;
}
memset(sense, 0, sizeof(sense));
@@ -516,7 +516,6 @@ int sg_scsi_ioctl(struct request_queue *q, struct gendisk
*disk, fmode_t mode,
blk_execute_rq(q, disk, rq, 0);
-out:
err = rq->errors & 0xff;/* only 8 bit SCSI status */
if (err) {
if (rq->sense_len && rq->sense) {
--
2.1.3
--
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] bnx2fc: do not add shared skbs to the fcoe_rx_list
Hi Chad,
On Fri, 2014-08-22 at 16:02 -0700, Eddie Wai wrote:
> On Fri, 2014-07-25 at 10:12 +0200, Maurizio Lombardi wrote:
> >
> > On 07/25/2014 10:02 AM, Maurizio Lombardi wrote:
> > > In some cases, the fcoe_rx_list may contains multiple instances
> > > of the same skb (the so called "shared skbs").
> > >
> > > the bnx2fc_l2_rcv thread is a loop that extracts a skb from the list,
> > > modifies (and destroys) its content and the proceed to the next one.
> > > The problem is that if the skb is shared, the remaining instances will
> > > be corrupted.
> > >
> > > The solution is to use skb_share_check() before adding the skb to the
> > > fcoe_rx_list.
> > >
> > > [ 6286.808725] [ cut here ]
> > > [ 6286.808729] WARNING: at include/scsi/fc_frame.h:173
> > > bnx2fc_l2_rcv_thread+0x425/0x450 [bnx2fc]()
> > > [ 6286.808748] Modules linked in: bnx2x(-) mdio dm_service_time bnx2fc
> > > cnic uio fcoe libfcoe 8021q garp stp mrp libfc llc scsi_transport_fc
> > > scsi_tgt sg iTCO_wdt iTCO_vendor_support coretemp kvm_intel kvm
> > > crct10dif_pclmul crc32_pclmul crc32c_intel e1000e ghash_clmulni_intel
> > > aesni_intel lrw gf128mul glue_helper ablk_helper ptp cryptd hpilo
> > > serio_raw hpwdt lpc_ich pps_core ipmi_si pcspkr mfd_core ipmi_msghandler
> > > shpchp pcc_cpufreq mperf nfsd auth_rpcgss nfs_acl lockd sunrpc
> > > dm_multipath xfs libcrc32c ata_generic pata_acpi sd_mod crc_t10dif
> > > crct10dif_common mgag200 syscopyarea sysfillrect sysimgblt i2c_algo_bit
> > > ata_piix drm_kms_helper ttm drm libata i2c_core hpsa dm_mirror
> > > dm_region_hash dm_log dm_mod [last unloaded: mdio]
> > > [ 6286.808750] CPU: 3 PID: 1304 Comm: bnx2fc_l2_threa Not tainted
> > > 3.10.0-121.el7.x86_64 #1
> > > [ 6286.808750] Hardware name: HP ProLiant DL120 G7, BIOS J01 07/01/2013
> > > [ 6286.808752] 0b36e715 8800deba1e00
> > > 815ec0ba
> > > [ 6286.808753] 8800deba1e38 8105dee1 a05618c0
> > > 8801e4c81888
> > > [ 6286.808754] e8663868 8801f402b180 8801f56bc000
> > > 8800deba1e48
> > > [ 6286.808754] Call Trace:
> > > [ 6286.808759] [] dump_stack+0x19/0x1b
> > > [ 6286.808762] [] warn_slowpath_common+0x61/0x80
> > > [ 6286.808763] [] warn_slowpath_null+0x1a/0x20
> > > [ 6286.808765] [] bnx2fc_l2_rcv_thread+0x425/0x450
> > > [bnx2fc]
> > > [ 6286.808767] [] ? bnx2fc_disable+0x90/0x90 [bnx2fc]
> > > [ 6286.808769] [] kthread+0xcf/0xe0
> > > [ 6286.808770] [] ? kthread_create_on_node+0x140/0x140
> > > [ 6286.808772] [] ret_from_fork+0x7c/0xb0
> > > [ 6286.808773] [] ? kthread_create_on_node+0x140/0x140
> > > [ 6286.808774] ---[ end trace c6cdb939184ccb4e ]---
> > >
> > > Signed-off-by: Maurizio Lombardi
> > > ---
> > > drivers/scsi/bnx2fc/bnx2fc_fcoe.c | 7 +++
> > > 1 file changed, 7 insertions(+)
> > >
> > > diff --git a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
> > > b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
> > > index 785d0d7..a190ab6 100644
> > > --- a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
> > > +++ b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
> > > @@ -411,6 +411,7 @@ static int bnx2fc_rcv(struct sk_buff *skb, struct
> > > net_device *dev,
> > > struct fc_frame_header *fh;
> > > struct fcoe_rcv_info *fr;
> > > struct fcoe_percpu_s *bg;
> > > + struct sk_buff *tmp_skb;
> > > unsigned short oxid;
> > >
> > > interface = container_of(ptype, struct bnx2fc_interface,
> > > @@ -423,6 +424,12 @@ static int bnx2fc_rcv(struct sk_buff *skb, struct
> > > net_device *dev,
> > > goto err;
> > > }
> > >
> > > + tmp_skb = skb_share_check(skb, GFP_ATOMIC);
> > > + if (!tmp_skb)
> > > + goto err;
> > > +
> > > + skb = tmp_skb;
> > > +
> > > if (unlikely(eth_hdr(skb)->h_proto != htons(ETH_P_FCOE))) {
> > > printk(KERN_ERR PFX "bnx2fc_rcv: Wrong FC type frame\n");
> > > goto err;
> > >
> Seems logical, but this patch requires some testing which ought to be
> verified by the Qlogic team. Thanks.
Any update about this?
Is the QLogic team still reviewing this patch?
Thanks,
Maurizio Lombardi
--
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 06/10] libata: use __scsi_format_command()
> -Original Message-
> From: Hannes Reinecke [mailto:h...@suse.de]
> Sent: Thursday, 06 November, 2014 2:31 AM
> To: James Bottomley
> Cc: Christoph Hellwig; Ewan Milne; Elliott, Robert (Server Storage);
> linux-scsi@vger.kernel.org; Hannes Reinecke; linux-
> i...@vger.kernel.org; LKML
> Subject: [PATCH 06/10] libata: use __scsi_format_command()
>
> libata already uses an internal buffer, so we should be using
> __scsi_format_command() here.
>
> Cc: linux-...@vger.kernel.org
> Cc: LKML
> Reviewed-by: Christoph Hellwig
> Acked-by: Tejun Heo
> Signed-off-by: Hannes Reinecke
> ---
> drivers/ata/libata-eh.c | 13 +
> 1 file changed, 5 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
> index dad83df..6550a9a 100644
> --- a/drivers/ata/libata-eh.c
> +++ b/drivers/ata/libata-eh.c
> @@ -2509,15 +2509,12 @@ static void ata_eh_link_report(struct
> ata_link *link)
>
> if (ata_is_atapi(qc->tf.protocol)) {
> if (qc->scsicmd)
> - scsi_print_command(qc->scsicmd);
> + __scsi_format_command(cdb_buf,
> sizeof(cdb_buf),
> + qc->scsicmd->cmnd,
> + qc->scsicmd->cmd_len);
> else
> - snprintf(cdb_buf, sizeof(cdb_buf),
> - "cdb %02x %02x %02x %02x %02x %02x %02x
> %02x "
> - "%02x %02x %02x %02x %02x %02x %02x %02x\n
> ",
> - cdb[0], cdb[1], cdb[2], cdb[3],
> - cdb[4], cdb[5], cdb[6], cdb[7],
> - cdb[8], cdb[9], cdb[10], cdb[11],
> - cdb[12], cdb[13], cdb[14], cdb[15]);
> + __scsi_format_command(cdb_buf,
> sizeof(cdb_buf),
> + cdb, qc->dev->cdb_len);
Since results in just one "cdb" variable usage, you could change
"cdb" to qc->cdb" to get rid of the variable declaration and
slightly simplify the code.
const u8 *cdb = qc->cdb;
> } else {
> const char *descr = ata_get_cmd_descript(cmd-
> >command);
> if (descr)
> --
> 1.8.5.2
Reviewed-by: Robert Elliott
--
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 01/10] scsi: Rename SERVICE_ACTION_IN to SERVICE_ACTION_IN_16
> -Original Message- > From: Hannes Reinecke [mailto:h...@suse.de] > Sent: Thursday, 06 November, 2014 2:31 AM > To: James Bottomley > Cc: Christoph Hellwig; Ewan Milne; Elliott, Robert (Server Storage); > linux-scsi@vger.kernel.org; Hannes Reinecke > Subject: [PATCH 01/10] scsi: Rename SERVICE_ACTION_IN to > SERVICE_ACTION_IN_16 > > SPC-3 defines SERVICE ACTION IN(12) and SERVICE ACTION IN(16). > So rename SERVICE_ACTION_IN to SERVICE_ACTION_IN_16 to be > consistent with SPC and to allow for better distinction. > > Reviewed-by: Christoph Hellwig > Signed-off-by: Hannes Reinecke Reviewed-by: Robert Elliott -- 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: BUG in scsi_lib.c due to a bad commit
Hello Christoph, I did the new bisect, the first bad commit is : 74665016086615bbaa3fa6f83af410a0a4e029ee scsi: convert host_busy to atomic_t you can find the bisect log here : https://bugzilla.kernel.org/attachment.cgi?id=157621 Le 14/11/2014 08:32, Christoph Hellwig a écrit : > On Thu, Nov 13, 2014 at 11:55:38PM +0100, Barto wrote: >> it's interesting, with this commit >> 74665016086615bbaa3fa6f83af410a0a4e029ee I have the bug : >> >> scsi: convert host_busy to atomic_t : > > At this point we'll need a bisction between v3.16 as the last good > point, and 74665016086615bbaa3fa6f83af410a0a4e029ee as the known bad > point. > -- 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 07/10] scsi: use per-cpu buffer for formatting sense
> -Original Message-
> From: Hannes Reinecke [mailto:h...@suse.de]
> Sent: Thursday, 06 November, 2014 2:31 AM
...
> diff --git a/drivers/scsi/scsi_logging.c
...
> @@ -249,3 +255,146 @@ void scsi_print_command(struct scsi_cmnd *cmd)
> }
> }
> EXPORT_SYMBOL(scsi_print_command);
> +
> +static size_t
> +scsi_format_extd_sense(char *buffer, size_t buf_len,
> +unsigned char asc, unsigned char ascq)
> +{
> + size_t off = 0;
> + const char *extd_sense_fmt = NULL;
> + const char *extd_sense_str = scsi_extd_sense_format(asc, ascq,
> + &extd_sense_fmt);
As Dan Carpenter noted, there's a missing {} in
scsi_extd_sense_format (from the previous logging series)
that causes it to return incorrectly.
...
> +static void
> +scsi_log_print_sense_hdr(const struct scsi_device *sdev, const char
> *name,
> + int tag, const struct scsi_sense_hdr *sshdr)
> +{
> + char *logbuf;
> + size_t off, logbuf_len;
> +
> + logbuf = scsi_log_reserve_buffer(&logbuf_len);
> + if (!logbuf)
> + return;
> + off = sdev_format_header(logbuf, logbuf_len, name, tag);
> + off += scsi_format_sense_hdr(logbuf + off, logbuf_len - off,
> sshdr);
> + dev_printk(KERN_INFO, &sdev->sdev_gendev, logbuf);
> + scsi_log_release_buffer(logbuf);
> +
> + logbuf = scsi_log_reserve_buffer(&logbuf_len);
> + if (!logbuf)
> + return;
> + off = sdev_format_header(logbuf, logbuf_len, name, tag);
> + off += scsi_format_extd_sense(logbuf + off, logbuf_len - off,
> + sshdr->asc, sshdr->ascq);
> + dev_printk(KERN_INFO, &sdev->sdev_gendev, logbuf);
> + scsi_log_release_buffer(logbuf);
> +}
This releases the buffer, but reserves it on the next line.
Should the buffer just be held between these two portions?
The subfunctions being called aren't functions that will cause
delays and thus delay other functions waiting on a buffer.
Although the tag on each line helps tremendously, the serial
log will be even more readable if the prints are back-to-back.
The same question applies to scsi_print_command in patch 05/10.
That function prints several lines in a for loop (for long CDBs):
+ for (k = 0; k < cmd->cmd_len; k += 16) {
+ size_t linelen = min(cmd->cmd_len - k, 16);
+
+ logbuf = scsi_log_reserve_buffer(&logbuf_len);
...
+ scsi_log_release_buffer(logbuf);
+ }
Perhaps the reserve/release should be outside the for loop
so they all reuse one buffer and are printed adjacently.
...
> +/* Normalize and print sense buffer in SCSI command */
> +void scsi_print_sense(const struct scsi_cmnd *cmd)
> +{
> + struct gendisk *disk = cmd->request->rq_disk;
> + const char *disk_name = disk ? disk->disk_name : NULL;
> + scsi_log_print_sense(cmd->device, disk_name, cmd->request->tag,
> + cmd->sense_buffer, SCSI_SENSE_BUFFERSIZE);
> +}
> +EXPORT_SYMBOL(scsi_print_sense);
This function should use the new scmd_name function, which does:
return scmd->request->rq_disk ?
scmd->request->rq_disk->disk_name : NULL;
Reviewed-by: Robert Elliott
---
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: Another (ESP?) scsi blk-mq problem on sparc64
Paul, what's the best way to figure out these CPU stalls?
The second oops is in blk_mq_map_queue() which is a trivial
two level cpu lookup. I wonder if there's something odd about
cpu numbers on these big old sparc systems?
Something like the debug patch below might shed some light on where the
index goes wrong, but it'll be horribly verbose.
diff --git a/block/blk-mq.c b/block/blk-mq.c
index b5896d4..ef4b35b 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -1270,7 +1270,12 @@ run_queue:
*/
struct blk_mq_hw_ctx *blk_mq_map_queue(struct request_queue *q, const int cpu)
{
- return q->queue_hw_ctx[q->mq_map[cpu]];
+ int idx;
+
+ printk("cpu: %d\n", cpu);
+ idx = q->mq_map[cpu];
+ printk("queue: %d\n", idx);
+ return q->queue_hw_ctx[idx];
}
EXPORT_SYMBOL(blk_mq_map_queue);
--
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: Another (ESP?) scsi blk-mq problem on sparc64
On 11/14/2014 09:58 AM, Christoph Hellwig wrote:
> Paul, what's the best way to figure out these CPU stalls?
>
> The second oops is in blk_mq_map_queue() which is a trivial
> two level cpu lookup. I wonder if there's something odd about
> cpu numbers on these big old sparc systems?
>
> Something like the debug patch below might shed some light on where the
> index goes wrong, but it'll be horribly verbose.
>
>
> diff --git a/block/blk-mq.c b/block/blk-mq.c
> index b5896d4..ef4b35b 100644
> --- a/block/blk-mq.c
> +++ b/block/blk-mq.c
> @@ -1270,7 +1270,12 @@ run_queue:
> */
> struct blk_mq_hw_ctx *blk_mq_map_queue(struct request_queue *q, const int
> cpu)
> {
> - return q->queue_hw_ctx[q->mq_map[cpu]];
> + int idx;
> +
> + printk("cpu: %d\n", cpu);
> + idx = q->mq_map[cpu];
> + printk("queue: %d\n", idx);
> + return q->queue_hw_ctx[idx];
> }
> EXPORT_SYMBOL(blk_mq_map_queue);
It'd probably be better to shove this debug stuff into the map building
code instead, ala attached.
--
Jens Axboe
diff --git a/block/blk-mq-cpumap.c b/block/blk-mq-cpumap.c
index 1065d7c65fa1..9200e2aee746 100644
--- a/block/blk-mq-cpumap.c
+++ b/block/blk-mq-cpumap.c
@@ -81,6 +81,9 @@ int blk_mq_update_queue_map(unsigned int *map, unsigned int nr_queues)
map[i] = map[first_sibling];
}
+ for (i = 0; i < queue; i++)
+ printk(KERN_ERR "cpumap %d -> %d\n", i, map[i]);
+
free_cpumask_var(cpus);
return 0;
}
[PATCH V4 linux-next RESEND 2] bfa: replace 2 kzalloc/copy_from_user by memdup_user
This patch also removes unnecessary printk(KERN_INFO
Signed-off-by: Fabian Frederick
---
V4: remove blank line after memdup_user
V3: memdup_user first argument is already void __user * (thanks to Joe Perches)
typo in title
V2: Remove printk(KERN_INFO (suggested by Alan)
drivers/scsi/bfa/bfad_debugfs.c | 30 ++
1 file changed, 6 insertions(+), 24 deletions(-)
diff --git a/drivers/scsi/bfa/bfad_debugfs.c b/drivers/scsi/bfa/bfad_debugfs.c
index 8e83d04..74a307c 100644
--- a/drivers/scsi/bfa/bfad_debugfs.c
+++ b/drivers/scsi/bfa/bfad_debugfs.c
@@ -260,18 +260,9 @@ bfad_debugfs_write_regrd(struct file *file, const char
__user *buf,
unsigned long flags;
void *kern_buf;
- kern_buf = kzalloc(nbytes, GFP_KERNEL);
-
- if (!kern_buf) {
- printk(KERN_INFO "bfad[%d]: Failed to allocate buffer\n",
- bfad->inst_no);
- return -ENOMEM;
- }
-
- if (copy_from_user(kern_buf, (void __user *)buf, nbytes)) {
- kfree(kern_buf);
- return -ENOMEM;
- }
+ kern_buf = memdup_user(buf, nbytes);
+ if (IS_ERR(kern_buf))
+ return PTR_ERR(kern_buf);
rc = sscanf(kern_buf, "%x:%x", &addr, &len);
if (rc < 2) {
@@ -336,18 +327,9 @@ bfad_debugfs_write_regwr(struct file *file, const char
__user *buf,
unsigned long flags;
void *kern_buf;
- kern_buf = kzalloc(nbytes, GFP_KERNEL);
-
- if (!kern_buf) {
- printk(KERN_INFO "bfad[%d]: Failed to allocate buffer\n",
- bfad->inst_no);
- return -ENOMEM;
- }
-
- if (copy_from_user(kern_buf, (void __user *)buf, nbytes)) {
- kfree(kern_buf);
- return -ENOMEM;
- }
+ kern_buf = memdup_user(buf, nbytes);
+ if (IS_ERR(kern_buf))
+ return PTR_ERR(kern_buf);
rc = sscanf(kern_buf, "%x:%x", &addr, &val);
if (rc < 2) {
--
1.9.3
--
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
Backport of a fix for a race condition in HPSA driver
Hello stable maintainers, Can you please backport commitid 2cc5bfaf854463d9d1aa52091f60110fbf102a9 ([SCSI] hpsa: fix a race in cmd_free/scsi_done) to 3.10 stable (and earlier, if applicable)? It seems to fix an issue that we have run into a couple of times. Many thanks, Masoud Sharbiani -- 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
Backport of a fix for a race condition in HPSA driver (3rd time is the charm)
Dear stable maintainers, Can you please backport commitid 2cc5bfaf854463d9d1aa52091f60110fbf102a9 ([SCSI] hpsa: fix a race in cmd_free/scsi_done) to 3.10 stable (and earlier, if applicable)? It seems to fix an issue that we have run into a couple of times. Many thanks, Masoud Sharbiani PS: Apologies for duplicate message that may have been captured in your spam folder. -- 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: Another (ESP?) scsi blk-mq problem on sparc64
> Paul, what's the best way to figure out these CPU stalls? > > The second oops is in blk_mq_map_queue() which is a trivial > two level cpu lookup. I wonder if there's something odd about > cpu numbers on these big old sparc systems? CPU numbers are sparse - they are determined by hardware slot number and some models only fill every other mainboard slot, and first slots can be free. I have first board offline and currently have CPUs numbered 10,11,14,15 online. Here is debug with Jens's patch: screen not found. Can't open input device. Keyboard not present. Using ttya for input and output. 4-slot Sun Enterprise 3000, No Keyboard OpenBoot 3.2.29, 4096 MB memory installed, Serial #51540060. Copyright 2001 Sun Microsystems, Inc. All rights reserved Ethernet address 0:3:ba:12:70:5c, Host ID: 8312705c. Rebooting with command: boot Boot device: disk File and args: SILO Version 1.4.14 boot: Allocated 64 Megs of memory at 0x4000 for kernel \Uncompressing image... |/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/- \|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|Loaded kernel version 3.18.0 [0.00] PROMLIB: Sun IEEE Boot Prom 'OBP 3.2.29 2001/06/18 17:28' [0.00] PROMLIB: Root node compatible: [0.00] Linux version 3.18.0-rc4-00052-g04689e7-dirty (mroos@mandel) (gcc version 4.9.2 (Debian 4.9.2-1) ) #26 SMP Fri Nov 14 21:22:28 EET 2014 [0.00] debug: ignoring loglevel setting. [0.00] bootconsole [earlyprom0] enabled [0.00] ARCH: SUN4U [0.00] Ethernet address: 00:03:ba:12:70:5c [0.00] MM: PAGE_OFFSET is 0xf800 (max_phys_bits == 40) [0.00] MM: VMALLOC [0x0001 --> 0x0600] [0.00] MM: VMEMMAP [0x0600 --> 0x0c00] [0.00] Kernel: Using 2 locked TLB entries for main kernel image. [0.00] Remapping the kernel... done. [0.00] OF stdout device is: /central@1f,0/fhc@0,f880/zs@0,902000:a [0.00] PROM: Built device tree with 92727 bytes of memory. [0.00] Top of RAM: 0xffd16000, Total RAM: 0xff954000 [0.00] Memory hole size: 3MB [0.00] Allocated 16384 bytes for kernel page tables. [0.00] Zone ranges: [0.00] Normal [mem 0x-0xffd15fff] [0.00] Movable zone start for each node [0.00] Early memory node ranges [0.00] node 0: [mem 0x-0xff84dfff] [0.00] node 0: [mem 0xffc0-0xffce] [0.00] node 0: [mem 0xffd0-0xffd15fff] [0.00] Initmem setup node 0 [mem 0x-0xffd15fff] [0.00] On node 0 totalpages: 523434 [0.00] Normal zone: 4094 pages used for memmap [0.00] Normal zone: 0 pages reserved [0.00] Normal zone: 523434 pages, LIFO batch:15 [0.00] Booting Linux... [0.00] CPU CAPS: [flush,stbar,swap,muldiv,v9,mul32,div32,v8plus] [0.00] CPU CAPS: [vis] [0.00] PERCPU: Embedded 7 pages/cpu @f800ff40 s13248 r8192 d35904 u1048576 [0.00] pcpu-alloc: s13248 r8192 d35904 u1048576 alloc=1*4194304 [0.00] pcpu-alloc: [0] 10 11 14 15 [0.00] Built 1 zonelists in Zone order, mobility grouping on. Total pages: 519340 [0.00] Kernel command line: root=/dev/sda2 ro ignore_loglevel [0.00] PID hash table entries: 4096 (order: 2, 32768 bytes) [0.00] Dentry cache hash table entries: 524288 (order: 9, 4194304 bytes) [0.00] Inode-cache hash table entries: 262144 (order: 8, 2097152 bytes) [0.00] Sorting __ex_table... [0.00] Memory: 4142488K/4187472K available (3653K kernel code, 246K rwdata, 800K rodata, 184K init, 413K bss, 44984K reserved) [0.00] SLUB: HWalign=32, Order=0-3, MinObjects=0, CPUs=16, Nodes=1 [0.00] Hierarchical RCU implementation. [0.00] Additional per-CPU info printed with stalls. [0.00] NR_IRQS:2048 nr_irq
Re: Another (ESP?) scsi blk-mq problem on sparc64
On 11/14/2014 12:35 PM, Meelis Roos wrote: >> Paul, what's the best way to figure out these CPU stalls? >> >> The second oops is in blk_mq_map_queue() which is a trivial >> two level cpu lookup. I wonder if there's something odd about >> cpu numbers on these big old sparc systems? > > CPU numbers are sparse - they are determined by hardware slot number and > some models only fill every other mainboard slot, and first slots can be > free. I have first board offline and currently have CPUs numbered > 10,11,14,15 online. > > Here is debug with Jens's patch: > [ 133.971050] CPU 11: synchronized TICK with master CPU (last diff -1 > cycles, maxerr 516 cycles) > [ 133.975491] CPU 14: synchronized TICK with master CPU (last diff -3 > cycles, maxerr 531 cycles) > [ 133.979943] CPU 15: synchronized TICK with master CPU (last diff -3 > cycles, maxerr 531 cycles) > [ 133.980146] Brought up 4 CPUs So this looks like this might be the issue. On a scsi-mq disabled boot, you have 4 CPUs, but how are they numbered? We might need Christophs debug patch on top this to fully know... -- Jens Axboe -- 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 3/3] block: Introduce blkdev_issue_zeroout_discard() function
> "Martin" == Martin K Petersen writes:
Martin> What would you prefer as the default for the ext4 use case? To
Martin> allocate or to discard?
I didn't get a preference for whether sb_issue_zeroout() should discard
or allocate.
But here's an updated patch 3...
commit eb23c9e71e08b7f467cbc36990a1a01a94a7b959
Author: Martin K. Petersen
Date: Thu Nov 6 14:36:05 2014 -0500
block: Add discard flag to blkdev_issue_zeroout() function
blkdev_issue_discard() will zero a given block range. This is done by
way of explicit writing, thus provisioning or allocating the blocks on
disk.
There are use cases where the desired behavior is to zero the blocks but
unprovision them if possible. The blocks must deterministically contain
zeroes when they are subsequently read back.
This patch adds a flag to blkdev_issue_zeroout() that provides this
variant. If the discard flag is set and a block device guarantees
discard_zeroes_data we will use REQ_DISCARD to clear the block range. If
the device does not support discard_zeroes_data or if the discard
request fails we will fall back to first REQ_WRITE_SAME and then a
regular REQ_WRITE.
Also update the callers of blkdev_issue_zero() to reflect the new flag
and make sb_issue_zeroout() prefer the discard approach.
Signed-off-by: Martin K. Petersen
diff --git a/block/blk-lib.c b/block/blk-lib.c
index 8411be3c19d3..715e948f58a4 100644
--- a/block/blk-lib.c
+++ b/block/blk-lib.c
@@ -283,23 +283,45 @@ static int __blkdev_issue_zeroout(struct block_device
*bdev, sector_t sector,
* @sector:start sector
* @nr_sects: number of sectors to write
* @gfp_mask: memory allocation flags (for bio_alloc)
+ * @discard: whether to discard the block range
*
* Description:
- * Generate and issue number of bios with zerofiled pages.
+
+ * Zero-fill a block range. If the discard flag is set and the block
+ * device guarantees that subsequent READ operations to the block range
+ * in question will return zeroes, the blocks will be discarded. Should
+ * the discard request fail, if the discard flag is not set, or if
+ * discard_zeroes_data is not supported, this function will resort to
+ * zeroing the blocks manually, thus provisioning (allocating,
+ * anchoring) them. If the block device supports the WRITE SAME command
+ * blkdev_issue_zeroout() will use it to optimize the process of
+ * clearing the block range. Otherwise the zeroing will be performed
+ * using regular WRITE calls.
*/
int blkdev_issue_zeroout(struct block_device *bdev, sector_t sector,
-sector_t nr_sects, gfp_t gfp_mask)
+sector_t nr_sects, gfp_t gfp_mask, bool discard)
{
+ struct request_queue *q = bdev_get_queue(bdev);
+ unsigned char bdn[BDEVNAME_SIZE];
+
+ if (discard && blk_queue_discard(q) && q->limits.discard_zeroes_data) {
+
+ if (!blkdev_issue_discard(bdev, sector, nr_sects, gfp_mask, 0))
+ return 0;
+
+ bdevname(bdev, bdn);
+ pr_warn("%s: DISCARD failed. Manually zeroing.\n", bdn);
+ }
+
if (bdev_write_same(bdev)) {
- unsigned char bdn[BDEVNAME_SIZE];
if (!blkdev_issue_write_same(bdev, sector, nr_sects, gfp_mask,
ZERO_PAGE(0)))
return 0;
bdevname(bdev, bdn);
- pr_err("%s: WRITE SAME failed. Manually zeroing.\n", bdn);
+ pr_warn("%s: WRITE SAME failed. Manually zeroing.\n", bdn);
}
return __blkdev_issue_zeroout(bdev, sector, nr_sects, gfp_mask);
diff --git a/block/ioctl.c b/block/ioctl.c
index 6c7bf903742f..7d8befde2aca 100644
--- a/block/ioctl.c
+++ b/block/ioctl.c
@@ -198,7 +198,7 @@ static int blk_ioctl_zeroout(struct block_device *bdev,
uint64_t start,
if (start + len > (i_size_read(bdev->bd_inode) >> 9))
return -EINVAL;
- return blkdev_issue_zeroout(bdev, start, len, GFP_KERNEL);
+ return blkdev_issue_zeroout(bdev, start, len, GFP_KERNEL, false);
}
static int put_ushort(unsigned long arg, unsigned short val)
diff --git a/drivers/block/drbd/drbd_receiver.c
b/drivers/block/drbd/drbd_receiver.c
index 6960fb064731..ee5b9611c51c 100644
--- a/drivers/block/drbd/drbd_receiver.c
+++ b/drivers/block/drbd/drbd_receiver.c
@@ -1388,7 +1388,7 @@ int drbd_submit_peer_request(struct drbd_device *device,
list_add_tail(&peer_req->w.list, &device->active_ee);
spin_unlock_irq(&device->resource->req_lock);
if (blkdev_issue_zeroout(device->ldev->backing_bdev,
- sector, data_size >> 9, GFP_NOIO))
+ sector, data_size >> 9, GFP_NOIO, false))
peer_req->flags |= EE_WAS_ERROR;
drbd_endio_write_sec_final(peer_req);
return 0;
Re: [PATCH] bnx2fc: do not add shared skbs to the fcoe_rx_list
Maurizio, we've been running this for a little while with no issues so
it's good to go from our perspective.
Acked-by: Chad Dupuis
On Fri, 14 Nov 2014, Maurizio Lombardi wrote:
Hi Chad,
On Fri, 2014-08-22 at 16:02 -0700, Eddie Wai wrote:
On Fri, 2014-07-25 at 10:12 +0200, Maurizio Lombardi wrote:
On 07/25/2014 10:02 AM, Maurizio Lombardi wrote:
In some cases, the fcoe_rx_list may contains multiple instances
of the same skb (the so called "shared skbs").
the bnx2fc_l2_rcv thread is a loop that extracts a skb from the list,
modifies (and destroys) its content and the proceed to the next one.
The problem is that if the skb is shared, the remaining instances will
be corrupted.
The solution is to use skb_share_check() before adding the skb to the
fcoe_rx_list.
[ 6286.808725] [ cut here ]
[ 6286.808729] WARNING: at include/scsi/fc_frame.h:173
bnx2fc_l2_rcv_thread+0x425/0x450 [bnx2fc]()
[ 6286.808748] Modules linked in: bnx2x(-) mdio dm_service_time bnx2fc cnic uio
fcoe libfcoe 8021q garp stp mrp libfc llc scsi_transport_fc scsi_tgt sg
iTCO_wdt iTCO_vendor_support coretemp kvm_intel kvm crct10dif_pclmul
crc32_pclmul crc32c_intel e1000e ghash_clmulni_intel aesni_intel lrw gf128mul
glue_helper ablk_helper ptp cryptd hpilo serio_raw hpwdt lpc_ich pps_core
ipmi_si pcspkr mfd_core ipmi_msghandler shpchp pcc_cpufreq mperf nfsd
auth_rpcgss nfs_acl lockd sunrpc dm_multipath xfs libcrc32c ata_generic
pata_acpi sd_mod crc_t10dif crct10dif_common mgag200 syscopyarea sysfillrect
sysimgblt i2c_algo_bit ata_piix drm_kms_helper ttm drm libata i2c_core hpsa
dm_mirror dm_region_hash dm_log dm_mod [last unloaded: mdio]
[ 6286.808750] CPU: 3 PID: 1304 Comm: bnx2fc_l2_threa Not tainted
3.10.0-121.el7.x86_64 #1
[ 6286.808750] Hardware name: HP ProLiant DL120 G7, BIOS J01 07/01/2013
[ 6286.808752] 0b36e715 8800deba1e00
815ec0ba
[ 6286.808753] 8800deba1e38 8105dee1 a05618c0
8801e4c81888
[ 6286.808754] e8663868 8801f402b180 8801f56bc000
8800deba1e48
[ 6286.808754] Call Trace:
[ 6286.808759] [] dump_stack+0x19/0x1b
[ 6286.808762] [] warn_slowpath_common+0x61/0x80
[ 6286.808763] [] warn_slowpath_null+0x1a/0x20
[ 6286.808765] [] bnx2fc_l2_rcv_thread+0x425/0x450 [bnx2fc]
[ 6286.808767] [] ? bnx2fc_disable+0x90/0x90 [bnx2fc]
[ 6286.808769] [] kthread+0xcf/0xe0
[ 6286.808770] [] ? kthread_create_on_node+0x140/0x140
[ 6286.808772] [] ret_from_fork+0x7c/0xb0
[ 6286.808773] [] ? kthread_create_on_node+0x140/0x140
[ 6286.808774] ---[ end trace c6cdb939184ccb4e ]---
Signed-off-by: Maurizio Lombardi
---
drivers/scsi/bnx2fc/bnx2fc_fcoe.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
index 785d0d7..a190ab6 100644
--- a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
+++ b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
@@ -411,6 +411,7 @@ static int bnx2fc_rcv(struct sk_buff *skb, struct
net_device *dev,
struct fc_frame_header *fh;
struct fcoe_rcv_info *fr;
struct fcoe_percpu_s *bg;
+ struct sk_buff *tmp_skb;
unsigned short oxid;
interface = container_of(ptype, struct bnx2fc_interface,
@@ -423,6 +424,12 @@ static int bnx2fc_rcv(struct sk_buff *skb, struct
net_device *dev,
goto err;
}
+ tmp_skb = skb_share_check(skb, GFP_ATOMIC);
+ if (!tmp_skb)
+ goto err;
+
+ skb = tmp_skb;
+
if (unlikely(eth_hdr(skb)->h_proto != htons(ETH_P_FCOE))) {
printk(KERN_ERR PFX "bnx2fc_rcv: Wrong FC type frame\n");
goto err;
Seems logical, but this patch requires some testing which ought to be
verified by the Qlogic team. Thanks.
Any update about this?
Is the QLogic team still reviewing this patch?
Thanks,
Maurizio Lombardi
--
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 08/10] scsi: use per-cpu buffer for formatting scsi_print_result()
> -Original Message-
> From: Hannes Reinecke [mailto:h...@suse.de]
> Sent: Thursday, 06 November, 2014 2:31 AM
...
> diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
...
> diff --git a/drivers/scsi/scsi_logging.c
> b/drivers/scsi/scsi_logging.c
> index 065792a3..e7e7cab 100644
> --- a/drivers/scsi/scsi_logging.c
> +++ b/drivers/scsi/scsi_logging.c
> @@ -398,3 +398,47 @@ void scsi_print_sense(const struct scsi_cmnd
> *cmd)
>cmd->sense_buffer, SCSI_SENSE_BUFFERSIZE);
> }
> EXPORT_SYMBOL(scsi_print_sense);
> +
> +void scsi_print_result(struct scsi_cmnd *cmd, const char *msg, int
> disposition)
Since this function does not modify anything pointed to by cmd,
consider adding const to that argument.
> +{
> + char *logbuf;
> + size_t off, logbuf_len;
> + const char *mlret_string = scsi_mlreturn_string(disposition);
As mentioned in the last series, it might be good to
change the midlayer string from SUCCESS to COMPLETE,
since that is printed even for commands that fail with
errors.
(This patch series doesn't touch that function, so mentioning
the issue here at the only caller)
>
> + const char *hb_string = scsi_hostbyte_string(cmd->result);
> + const char *db_string = scsi_driverbyte_string(cmd->result);
> +
> + logbuf = scsi_log_reserve_buffer(&logbuf_len);
> + if (!logbuf)
> + return;
> +
> + off = sdev_format_header(logbuf, logbuf_len,
> + scmd_name(cmd), cmd->request->tag);
> +
> + if (msg)
> + off += scnprintf(logbuf + off, logbuf_len - off,
> + "%s: ", msg);
> + if (mlret_string)
> + off += scnprintf(logbuf + off, logbuf_len - off,
> + "%s ", mlret_string);
> + else
> + off += scnprintf(logbuf + off, logbuf_len - off,
> + "UNKNOWN(0x%02x) ", disposition);
> + off += scnprintf(logbuf + off, logbuf_len - off, "Result: ");
...
Consider printing "Result: " first. That would make
the messages more consistent since "Done:" is not always there.
Current:
[491784.462209] sd 1:0:0:0: [sdq] tag#0 Done: SUCCESS Result: hostbyte=DID_OK
driverbyte=DRIVER_OK
[491784.464962] sd 1:0:0:0: [sdq] tag#0 CDB: Write(10) 2a 00 39 8c fa 80 00 00
08 00
[ 259.667383] sd 2:0:0:0: [sdr] tag#334 FAILED Result: hostbyte=DID_OK
driverbyte=DRIVER_SENSE
[ 259.667387] sd 2:0:0:0: [sdr] tag#334 Sense Key : Hardware Error [current]
[ 259.667391] sd 2:0:0:0: [sdr] tag#334 Add. Sense: Logical unit failure
[ 259.667395] sd 2:0:0:0: [sdr] tag#334 CDB: Read(10) 28 00 1b 85 9d 30 00 00
08 00
[27907.647377] sd 1:0:0:0: [sdq] tag#768 Done: TIMEOUT_ERROR Result:
hostbyte=DID_OK driverbyte=DRIVER_OK
[27907.647380] sd 1:0:0:0: [sdq] tag#768 CDB: Write(10) 2a 00 39 8a f4 e0 00
00 08 00
[27907.647382] sd 1:0:0:0: [sdq] tag#768 scmd 880424761470 abort scheduled
Proposed:
[491784.462209] sd 1:0:0:0: [sdq] tag#0 Result: Done: COMPLETE hostbyte=DID_OK
driverbyte=DRIVER_OK
[491784.464962] sd 1:0:0:0: [sdq] tag#0 CDB: Write(10) 2a 00 39 8c fa 80 00 00
08 00
[ 259.667383] sd 2:0:0:0: [sdr] tag#334 Result: FAILED hostbyte=DID_OK
driverbyte=DRIVER_SENSE
[ 259.667387] sd 2:0:0:0: [sdr] tag#334 Sense Key : Hardware Error [current]
[ 259.667391] sd 2:0:0:0: [sdr] tag#334 Add. Sense: Logical unit failure
[ 259.667395] sd 2:0:0:0: [sdr] tag#334 CDB: Read(10) 28 00 1b 85 9d 30 00 00
08 00
[27907.647377] sd 1:0:0:0: [sdq] tag#768 Result: Done: TIMEOUT_ERROR
hostbyte=DID_OK driverbyte=DRIVER_OK
[27907.647380] sd 1:0:0:0: [sdq] tag#768 CDB: Write(10) 2a 00 39 8a f4 e0 00
00 08 00
[27907.647382] sd 1:0:0:0: [sdq] tag#768 scmd 880424761470 abort scheduled
Furthermore, consider dropping "Done" (the only msg argument
ever used) altogether. "Done" means scsi_log_completion is
printing this. No "Done" means scsi_io_completion's ACTION_FAIL
case is printing this.
The disposition (COMPLETE, FAILED, TIMEOUT_ERROR, etc.) seems
to convey that same information, in more detail.
Reviewed-by: Robert Elliott
---
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: [PATCH 09/10] scsi: Conditionally compile in constants.c
> -Original Message- > From: Hannes Reinecke [mailto:h...@suse.de] > Sent: Thursday, 06 November, 2014 2:31 AM > To: James Bottomley > Cc: Christoph Hellwig; Ewan Milne; Elliott, Robert (Server Storage); > linux-scsi@vger.kernel.org; Hannes Reinecke > Subject: [PATCH 09/10] scsi: Conditionally compile in constants.c > > Instead of having constants.c littered with ifdef statements > we should be moving dummy functions into the header and > condintionally compile in constants.c if selected. > > Suggested-by: Christoph Hellwig > Reviewed-by: Christoph Hellwig > Signed-off-by: Hannes Reinecke > --- > drivers/scsi/Makefile | 4 +-- > drivers/scsi/constants.c | 42 > drivers/xen/xen-scsiback.c | 1 + > include/scsi/scsi_dbg.h| 68 > +++--- > 4 files changed, 67 insertions(+), 48 deletions(-) > > diff --git a/drivers/scsi/Makefile b/drivers/scsi/Makefile > index 4991b62..76f8932 100644 > --- a/drivers/scsi/Makefile > +++ b/drivers/scsi/Makefile > @@ -158,9 +158,9 @@ obj-$(CONFIG_SCSI_OSD_INITIATOR) += osd/ > > # This goes last, so that "real" scsi devices probe earlier > obj-$(CONFIG_SCSI_DEBUG) += scsi_debug.o > - > -scsi_mod-y += scsi.o hosts.o scsi_ioctl.o constants.o \ > +scsi_mod-y += scsi.o hosts.o scsi_ioctl.o \ > scsicam.o scsi_error.o scsi_lib.o > +scsi_mod-$(CONFIG_SCSI_CONSTANTS) += constants.o > scsi_mod-$(CONFIG_SCSI_DMA) += scsi_lib_dma.o > scsi_mod-y += scsi_scan.o scsi_sysfs.o scsi_devinfo.o > scsi_mod-$(CONFIG_SCSI_NETLINK) += scsi_netlink.o ... I has no compile issues with and without CONFIG_SCSI_CONSTANTS. make menuconfig says SCSI_CONSTANTS causes "(kernel size +=12K)". That appears low now (although other config settings might be exaggerating that on my machine). Without CONFIG_SCSI_CONSTANTS: -rwxrwxr-x 1 relliott relliott 133057434 Nov 14 16:27 vmlinux -rw-rw-r-- 1 relliott relliott 313306350 Nov 14 16:27 vmlinux.o With CONFIG_SCSI_CONSTANTS: -rwxrwxr-x 1 relliott relliott 133132674 Nov 14 16:33 vmlinux -rw-rw-r-- 1 relliott relliott 313575486 Nov 14 16:33 vmlinux.o vmlinux is 75,240 bytes different. So, you might want to modify the Kconfig description. Reviewed-by: Robert Elliott --- 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: [PATCH 10/10] scsi: Do not display buffer pointers in scsi_log_send()
> -Original Message-
> From: Hannes Reinecke [mailto:h...@suse.de]
> Sent: Thursday, 06 November, 2014 2:31 AM
> To: James Bottomley
> Cc: Christoph Hellwig; Ewan Milne; Elliott, Robert (Server Storage);
> linux-scsi@vger.kernel.org; Hannes Reinecke
> Subject: [PATCH 10/10] scsi: Do not display buffer pointers in
> scsi_log_send()
>
> scsi_log_send() would display buffer pointer for higher
> logging levels. This is not only of questionable value
> but also exposes kernel pointer to userspace, which is
> discouraged in some setups. So drop this message
> altogether.
>
> Signed-off-by: Hannes Reinecke
> ---
> drivers/scsi/scsi.c | 9 +
> 1 file changed, 1 insertion(+), 8 deletions(-)
>
> diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
> index 92d5912..9ec576d 100644
> --- a/drivers/scsi/scsi.c
> +++ b/drivers/scsi/scsi.c
> @@ -531,7 +531,7 @@ void scsi_log_send(struct scsi_cmnd *cmd)
>*
>* 3: same as 2
>*
> - * 4: same as 3 plus dump extra junk
> + * 4: same as 3
>*/
> if (unlikely(scsi_logging_level)) {
> level = SCSI_LOG_LEVEL(SCSI_LOG_MLQUEUE_SHIFT,
> @@ -540,13 +540,6 @@ void scsi_log_send(struct scsi_cmnd *cmd)
> scmd_printk(KERN_INFO, cmd,
> "Send: scmd 0x%p\n", cmd);
> scsi_print_command(cmd);
> - if (level > 3) {
> - printk(KERN_INFO "buffer = 0x%p, bufflen =
> %d,"
> -" queuecommand 0x%p\n",
> - scsi_sglist(cmd), scsi_bufflen(cmd),
> - cmd->device->host->hostt-
> >queuecommand);
> -
> - }
> }
> }
> }
Reviewed-by: Robert Elliott
One more comment on the series:
After the tags, there are still a few scmd %p prints left
including the one above. Is this series supposed to get
rid of the rest of them?
drivers/scsi/scsi.c:"Send: scmd 0x%p\n", cmd);
drivers/scsi/scsi_error.c: "scmd %p eh
timeout, not aborting\n",
drivers/scsi/scsi_error.c: "aborting command
%p\n", scmd));
drivers/scsi/scsi_error.c:
"scmd %p eh timeout, "
drivers/scsi/scsi_error.c:
"scmd %p retry "
drivers/scsi/scsi_error.c:
"scmd %p finish "
drivers/scsi/scsi_error.c: "scmd %p
abort %s\n", scmd,
drivers/scsi/scsi_error.c: "scmd %p terminate "
drivers/scsi/scsi_error.c: "scmd %p previous
abort failed\n", scmd));
drivers/scsi/scsi_error.c: "scmd %p not
aborting, host in recovery\n",
drivers/scsi/scsi_error.c: "scmd %p abort
scheduled\n", scmd));
drivers/scsi/scsi_error.c: "%s scmd: %p result: %x\n",
drivers/scsi/scsi_error.c: "%s: scmd: %p, timeleft: %ld\n",
drivers/scsi/scsi_error.c: "sense requested for %p result
%x\n",
drivers/scsi/scsi_error.c: "%s: scmd %p rtn %x\n", __func__, scmd,
rtn));
drivers/scsi/scsi_error.c: "%s: flush
retry cmd: %p\n",
drivers/scsi/scsi_error.c: "%s: flush
finish cmd: %p\n",
drivers/scsi/scsi_lib.c:"Inserting command %p into mlqueue\n",
cmd));
drivers/scsi/scsi_scan.c: printk(KERN_WARNING "%s no slave_alloc
function in hostt %p\n", __func__, shost->hostt);
drivers/scsi/scsi_scan.c: printk(KERN_WARNING "sdev %p scmd %p
sending INQUIRY\n", sdev, scsi_cmd);
drivers/scsi/scsi_scan.c: printk(KERN_WARNING "%s sdev %p from
lookup_by_target\n", __func__, sdev);
drivers/scsi/scsi_scan.c: printk(KERN_WARNING "%s sdev %p from
alloc_sdev\n", __func__, sdev);
drivers/scsi/scsi_scan.c: printk(KERN_WARNING "%s sdev %p from
alloc_sdev\n", __func__, sdev);
drivers/scsi/scsi_scan.c: printk(KERN_WARNING "%s sdev %p from
lookup_by_target\n", __func__, sdev);
drivers/scsi/scsi_scan.c: printk(KERN_WARNING "%s sdev %p from
alloc_sdev\n", __func__, sdev);
--
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: Another (ESP?) scsi blk-mq problem on sparc64
> >> The second oops is in blk_mq_map_queue() which is a trivial > >> two level cpu lookup. I wonder if there's something odd about > >> cpu numbers on these big old sparc systems? > > > > CPU numbers are sparse - they are determined by hardware slot number and > > some models only fill every other mainboard slot, and first slots can be > > free. I have first board offline and currently have CPUs numbered > > 10,11,14,15 online. > > > > Here is debug with Jens's patch: > > > [ 133.971050] CPU 11: synchronized TICK with master CPU (last diff -1 > > cycles, maxerr 516 cycles) > > [ 133.975491] CPU 14: synchronized TICK with master CPU (last diff -3 > > cycles, maxerr 531 cycles) > > [ 133.979943] CPU 15: synchronized TICK with master CPU (last diff -3 > > cycles, maxerr 531 cycles) > > [ 133.980146] Brought up 4 CPUs > > So this looks like this might be the issue. On a scsi-mq disabled boot, > you have 4 CPUs, but how are they numbered? The numbers are always the same. But everything seems to be mapped to queue 0? > We might need Christophs debug patch on top this to fully know... Applied it too, dmesg is below. Yes it does spam the log a lot, and over 9600bps console its' somewhat slow :) There is another detail to note -this server contains a faulty disk as sdc that times out spinup. I left it in the server because it helped to pinpoint and fix a previous error in esp scsi driver. This can be a factor here too - the error handling details. screen not found. Can't open input device. Keyboard not present. Using ttya for input and output. 4-slot Sun Enterprise 3000, No Keyboard OpenBoot 3.2.29, 4096 MB memory installed, Serial #51540060. Copyright 2001 Sun Microsystems, Inc. All rights reserved Ethernet address 0:3:ba:12:70:5c, Host ID: 8312705c. Rebooting with command: boot Boot device: disk File and args: SILO Version 1.4.14 boot: Allocated 64 Megs of memory at 0x4000 for kernel \Uncompressing image... |/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/- \|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|/-\|Loaded kernel version 3.18.0 [0.00] PROMLIB: Sun IEEE Boot Prom 'OBP 3.2.29 2001/06/18 17:28' [0.00] PROMLIB: Root node compatible: [0.00] Linux version 3.18.0-rc4-00052-g04689e7-dirty (mroos@mandel) (gcc version 4.9.2 (Debian 4.9.2-1) ) #27 SMP Sat Nov 15 00:34:49 EET 2014 [0.00] debug: ignoring loglevel setting. [0.00] bootconsole [earlyprom0] enabled [0.00] ARCH: SUN4U [0.00] Ethernet address: 00:03:ba:12:70:5c [0.00] MM: PAGE_OFFSET is 0xf800 (max_phys_bits == 40) [0.00] MM: VMALLOC [0x0001 --> 0x0600] [0.00] MM: VMEMMAP [0x0600 --> 0x0c00] [0.00] Kernel: Using 2 locked TLB entries for main kernel image. [0.00] Remapping the kernel... done. [0.00] OF stdout device is: /central@1f,0/fhc@0,f880/zs@0,902000:a [0.00] PROM: Built device tree with 92727 bytes of memory. [0.00] Top of RAM: 0xffd16000, Total RAM: 0xff954000 [0.00] Memory hole size: 3MB [0.00] Allocated 16384 bytes for kernel page tables. [0.00] Zone ranges: [0.00] Normal [mem 0x-0xffd15fff] [0.00] Movable zone start for each node [0.00] Early memory node ranges [0.00] node 0: [mem 0x-0xff84dfff] [0.00] node 0: [mem 0xffc0-0xffce] [0.00] node 0: [mem 0xffd0-0xffd15fff] [0.00] Initmem setup node 0 [mem 0x-0xffd15fff] [0.00] On node 0 totalpages: 523434 [0.00] Normal zone: 4094 pages used for memmap [0.00] Normal zone: 0 pages reserved [0.00] Normal zone: 523434 pages, LIFO batch:15 [0.00] Booting Linux... [0.00] CPU CAPS: [flush,stbar,swap,muldiv,v9,mul32,div32,v8plus] [0.00] CPU CAPS: [vis] [0.
RE: [PATCHv2 00/10] scsi logging update: the real thing
> -Original Message- > From: Hannes Reinecke [mailto:h...@suse.de] > Sent: Thursday, 06 November, 2014 2:30 AM > To: James Bottomley > Cc: Christoph Hellwig; Ewan Milne; Elliott, Robert (Server Storage); > linux-scsi@vger.kernel.org; Hannes Reinecke > Subject: [PATCHv2 00/10] scsi logging update: the real thing > > Hi all, > > this is the second part of my scsi logging update, covering > CDB and sense code printing. With this patchset CDBs and > sense code bytes will be formatted on a single line > (where possible), and prefixed with the device and command > tag. > > To ensure CDBs and sense code bytes are not broken up during > printk I've implemented a per-cpu buffer for formatting > messages. One can allocate a chunk from the buffer using > scsi_log_reserve_buffer() and return it via > scsi_log_release_buffer(). > Both function do an implicit preempt_disable() / preempt_enable() > to ensure we're not scheduled away from that cpu whilst writing > into the buffer. > > With that I've been able to clean up constants.c to remove all the > #ifdef CONFIG_SCSI_CONSTANTS statements. It'll now be compiled in > conditionally if CONFIG_SCSI_CONSTANTS is set. > > Additionally I've updated the SCSI command definitions to SPC-3. > > Thanks go to Stephen Rostedt who suffered my annoying questions > during LPC. > I've run with v2 for a while. Tag prints look good; CDBs are coming out on one line, etc. I haven't tested every line of every patch, but I think I exercised patches 3, 5, 7, and 8 pretty thoroughly, so you may add this to those: Tested-by: Robert Elliott --- 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
[PATCH 02/13] hpsa: remove dev_warn prints from RAID-1ADM
From: Robert Elliott RAID-1ADM is unusable with dev_warn called on every command. Signed-off-by: Robert Elliott Signed-off-by: Don Brace Reviewed-by: Stephen M. Cameron Reviewed-by: Webb Scales --- drivers/scsi/hpsa.c |5 - 1 file changed, 5 deletions(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index f345cc5..e24b304 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -3811,11 +3811,6 @@ static int hpsa_scsi_ioaccel_raid_map(struct ctlr_info *h, offload_to_mirror = (offload_to_mirror >= map->layout_map_count - 1) ? 0 : offload_to_mirror + 1; - /* FIXME: remove after debug/dev */ - BUG_ON(offload_to_mirror >= map->layout_map_count); - dev_warn(&h->pdev->dev, - "DEBUG: Using physical disk map index %d from mirror group %d\n", - map_index, offload_to_mirror); dev->offload_to_mirror = offload_to_mirror; /* Avoid direct use of dev->offload_to_mirror within this * function since multiple threads might simultaneously -- 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 01/13] hpsa: Clean up warnings from sparse.
Clean up issues reported when running sparse.
Signed-off-by: Don Brace
Reviewed-by: Webb Scales
---
drivers/scsi/hpsa.c | 29 -
drivers/scsi/hpsa.h |6 +++---
2 files changed, 19 insertions(+), 16 deletions(-)
diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index cef5d49..f345cc5 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -48,6 +48,7 @@
#include
#include
#include
+#include
#include
#include
#include "hpsa_cmd.h"
@@ -193,12 +194,13 @@ static int number_of_controllers;
static irqreturn_t do_hpsa_intr_intx(int irq, void *dev_id);
static irqreturn_t do_hpsa_intr_msi(int irq, void *dev_id);
-static int hpsa_ioctl(struct scsi_device *dev, int cmd, void *arg);
+static int hpsa_ioctl(struct scsi_device *dev, int cmd, void __user *arg);
static void lock_and_start_io(struct ctlr_info *h);
static void start_io(struct ctlr_info *h, unsigned long *flags);
#ifdef CONFIG_COMPAT
-static int hpsa_compat_ioctl(struct scsi_device *dev, int cmd, void *arg);
+static int hpsa_compat_ioctl(struct scsi_device *dev, int cmd,
+ void __user *arg);
#endif
static void cmd_free(struct ctlr_info *h, struct CommandList *c);
@@ -2941,8 +2943,8 @@ static int hpsa_gather_lun_info(struct ctlr_info *h,
return 0;
}
-u8 *figure_lunaddrbytes(struct ctlr_info *h, int raid_ctlr_position, int i,
- int nphysicals, int nlogicals,
+static u8 *figure_lunaddrbytes(struct ctlr_info *h, int raid_ctlr_position,
+ int i, int nphysicals, int nlogicals,
struct ReportExtendedLUNdata *physdev_list,
struct ReportLUNdata *logdev_list)
{
@@ -4410,7 +4412,7 @@ static struct CommandList *hpsa_find_cmd_in_queue(struct
ctlr_info *h,
struct CommandList *c = NULL; /* ptr into cmpQ */
if (!find)
- return 0;
+ return NULL;
spin_lock_irqsave(&h->lock, flags);
list_for_each_entry(c, queue_head, list) {
if (c->scsi_cmd == NULL) /* e.g.: passthru ioctl */
@@ -4785,7 +4787,8 @@ static void cmd_special_free(struct ctlr_info *h, struct
CommandList *c)
#ifdef CONFIG_COMPAT
-static int hpsa_ioctl32_passthru(struct scsi_device *dev, int cmd, void *arg)
+static int hpsa_ioctl32_passthru(struct scsi_device *dev, int cmd,
+ void __user *arg)
{
IOCTL32_Command_struct __user *arg32 =
(IOCTL32_Command_struct __user *) arg;
@@ -4810,7 +4813,7 @@ static int hpsa_ioctl32_passthru(struct scsi_device *dev,
int cmd, void *arg)
if (err)
return -EFAULT;
- err = hpsa_ioctl(dev, CCISS_PASSTHRU, (void *)p);
+ err = hpsa_ioctl(dev, CCISS_PASSTHRU, p);
if (err)
return err;
err |= copy_in_user(&arg32->error_info, &p->error_info,
@@ -4821,7 +4824,7 @@ static int hpsa_ioctl32_passthru(struct scsi_device *dev,
int cmd, void *arg)
}
static int hpsa_ioctl32_big_passthru(struct scsi_device *dev,
- int cmd, void *arg)
+ int cmd, void __user *arg)
{
BIG_IOCTL32_Command_struct __user *arg32 =
(BIG_IOCTL32_Command_struct __user *) arg;
@@ -4848,7 +4851,7 @@ static int hpsa_ioctl32_big_passthru(struct scsi_device
*dev,
if (err)
return -EFAULT;
- err = hpsa_ioctl(dev, CCISS_BIG_PASSTHRU, (void *)p);
+ err = hpsa_ioctl(dev, CCISS_BIG_PASSTHRU, p);
if (err)
return err;
err |= copy_in_user(&arg32->error_info, &p->error_info,
@@ -4858,7 +4861,7 @@ static int hpsa_ioctl32_big_passthru(struct scsi_device
*dev,
return err;
}
-static int hpsa_compat_ioctl(struct scsi_device *dev, int cmd, void *arg)
+static int hpsa_compat_ioctl(struct scsi_device *dev, int cmd, void __user
*arg)
{
switch (cmd) {
case CCISS_GETPCIINFO:
@@ -5206,7 +5209,7 @@ static void decrement_passthru_count(struct ctlr_info *h)
/*
* ioctl
*/
-static int hpsa_ioctl(struct scsi_device *dev, int cmd, void *arg)
+static int hpsa_ioctl(struct scsi_device *dev, int cmd, void __user *arg)
{
struct ctlr_info *h;
void __user *argp = (void __user *)arg;
@@ -5818,7 +5821,7 @@ static int hpsa_message(struct pci_dev *pdev, unsigned
char opcode,
#define hpsa_noop(p) hpsa_message(p, 3, 0)
static int hpsa_controller_hard_reset(struct pci_dev *pdev,
- void * __iomem vaddr, u32 use_doorbell)
+ void __iomem *vaddr, u32 use_doorbell)
{
u16 pmcsr;
int pos;
@@ -6056,7 +6059,7 @@ unmap_vaddr:
* the io functions.
* This is for debug only.
*/
-static void print_cfg_table(struct device *dev, struct CfgTable *tb)
+static void print_cfg_table(struct device *dev, struct CfgTable __iomem *tb)
{
#ifdef HPSA_DEBUG
int i;
diff --git a/drivers/scsi/hpsa.h b/drivers/scsi/hpsa.h
index 24472ce..80fa9a9 100644
--- a/drivers/scsi/hpsa.h
+++ b/drivers/scsi/hpsa.h
@@ -164,7 +164,7 @@ struct ctlr_info {
*/
u32 trans_support;
u32 trans_offset;
[PATCH 00/13] hpsa update
This patch set is based on Linus's tree. The changes are: - correct warnings from sparse - updates for some error handling issues - general code cleanup - performance enhancements based on removing spin_locks --- Don Brace (1): hpsa: Clean up warnings from sparse. Nicholas Bellinger (1): hpsa: Convert SCSI LLD ->queuecommand() for host_lock less operation Robert Elliott (2): hpsa: remove dev_warn prints from RAID-1ADM hpsa: always call pci_set_master after pci_enable_device Stephen M. Cameron (8): hpsa: fix a couple pci id table mistakes hpsa: remove 'action required' phrasing hpsa: fix allocation sizes for CISS_REPORT_LUNs commands hpsa: fix endianness issue with scatter gather elements hpsa: get rid of type/attribute/direction bit field where possible hpsa: use atomics for commands_outstanding hpsa: do not be so noisy about check conditions hpsa: remove spin lock around command allocation Webb Scales (1): hpsa: correct off-by-one sizing of chained SG block drivers/scsi/hpsa.c | 488 --- drivers/scsi/hpsa.h | 33 +-- drivers/scsi/hpsa_cmd.h | 34 ++- 3 files changed, 234 insertions(+), 321 deletions(-) -- Don Brace don.br...@pmcs.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
[PATCH 08/13] hpsa: get rid of type/attribute/direction bit field where possible
From: Stephen M. Cameron
Using bit fields for hardware command fields isn't portable and
relies on assumptions about how the compiler lays out the bits.
We can fix this in the driver's internal command structure, but the
ioctl interface we can't change because it is part of the
userland ABI.
Signed-off-by: Don Brace
Reviewed-by: Webb Scales
---
drivers/scsi/hpsa.c | 58 ---
drivers/scsi/hpsa_cmd.h | 18 +++
2 files changed, 42 insertions(+), 34 deletions(-)
diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index fe1b589..95c34a5 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -4019,17 +4019,18 @@ static int hpsa_scsi_queue_command_lck(struct scsi_cmnd
*cmd,
BUG_ON(cmd->cmd_len > sizeof(c->Request.CDB));
c->Request.CDBLen = cmd->cmd_len;
memcpy(c->Request.CDB, cmd->cmnd, cmd->cmd_len);
- c->Request.Type.Type = TYPE_CMD;
- c->Request.Type.Attribute = ATTR_SIMPLE;
switch (cmd->sc_data_direction) {
case DMA_TO_DEVICE:
- c->Request.Type.Direction = XFER_WRITE;
+ c->Request.type_attr_dir =
+ TYPE_ATTR_DIR(TYPE_CMD, ATTR_SIMPLE, XFER_WRITE);
break;
case DMA_FROM_DEVICE:
- c->Request.Type.Direction = XFER_READ;
+ c->Request.type_attr_dir =
+ TYPE_ATTR_DIR(TYPE_CMD, ATTR_SIMPLE, XFER_READ);
break;
case DMA_NONE:
- c->Request.Type.Direction = XFER_NONE;
+ c->Request.type_attr_dir =
+ TYPE_ATTR_DIR(TYPE_CMD, ATTR_SIMPLE, XFER_NONE);
break;
case DMA_BIDIRECTIONAL:
/* This can happen if a buggy application does a scsi passthru
@@ -4037,7 +4038,8 @@ static int hpsa_scsi_queue_command_lck(struct scsi_cmnd
*cmd,
* ../scsi/scsi_ioctl.c:scsi_ioctl_send_command() )
*/
- c->Request.Type.Direction = XFER_RSVD;
+ c->Request.type_attr_dir =
+ TYPE_ATTR_DIR(TYPE_CMD, ATTR_SIMPLE, XFER_RSVD);
/* This is technically wrong, and hpsa controllers should
* reject it with CMD_INVALID, which is the most correct
* response, but non-fibre backends appear to let it
@@ -5257,7 +5259,6 @@ static int fill_cmd(struct CommandList *c, u8 cmd, struct
ctlr_info *h,
c->Header.tag = c->busaddr;
memcpy(c->Header.LUN.LunAddrBytes, scsi3addr, 8);
- c->Request.Type.Type = cmd_type;
if (cmd_type == TYPE_CMD) {
switch (cmd) {
case HPSA_INQUIRY:
@@ -5267,8 +5268,8 @@ static int fill_cmd(struct CommandList *c, u8 cmd, struct
ctlr_info *h,
c->Request.CDB[2] = (page_code & 0xff);
}
c->Request.CDBLen = 6;
- c->Request.Type.Attribute = ATTR_SIMPLE;
- c->Request.Type.Direction = XFER_READ;
+ c->Request.type_attr_dir =
+ TYPE_ATTR_DIR(cmd_type, ATTR_SIMPLE, XFER_READ);
c->Request.Timeout = 0;
c->Request.CDB[0] = HPSA_INQUIRY;
c->Request.CDB[4] = size & 0xFF;
@@ -5279,8 +5280,8 @@ static int fill_cmd(struct CommandList *c, u8 cmd, struct
ctlr_info *h,
mode = 00 target = 0. Nothing to write.
*/
c->Request.CDBLen = 12;
- c->Request.Type.Attribute = ATTR_SIMPLE;
- c->Request.Type.Direction = XFER_READ;
+ c->Request.type_attr_dir =
+ TYPE_ATTR_DIR(cmd_type, ATTR_SIMPLE, XFER_READ);
c->Request.Timeout = 0;
c->Request.CDB[0] = cmd;
c->Request.CDB[6] = (size >> 24) & 0xFF; /* MSB */
@@ -5290,8 +5291,9 @@ static int fill_cmd(struct CommandList *c, u8 cmd, struct
ctlr_info *h,
break;
case HPSA_CACHE_FLUSH:
c->Request.CDBLen = 12;
- c->Request.Type.Attribute = ATTR_SIMPLE;
- c->Request.Type.Direction = XFER_WRITE;
+ c->Request.type_attr_dir =
+ TYPE_ATTR_DIR(cmd_type,
+ ATTR_SIMPLE, XFER_WRITE);
c->Request.Timeout = 0;
c->Request.CDB[0] = BMIC_WRITE;
c->Request.CDB[6] = BMIC_CACHE_FLUSH;
@@ -5300,14 +5302,14 @@ static int fill_cmd(struct CommandList *c, u8 cmd,
struct ctlr_info *h,
break;
case TEST_UNIT_READY:
c->Request.CDBLen = 6;
- c->Request.Type.
[PATCH 11/13] hpsa: Convert SCSI LLD ->queuecommand() for host_lock less operation
From: Nicholas Bellinger
There isn't anything in hpsa that requires the host lock to be held
during queuecommand.
Signed-off-by: Don Brace
Signed-off-by: Nicholas Bellinger
Reviewed-by: Stephen M. Cameron
---
drivers/scsi/hpsa.c | 16 +++-
1 file changed, 7 insertions(+), 9 deletions(-)
diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index 58f6847..761612d 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -3881,8 +3881,11 @@ static int hpsa_scsi_ioaccel_raid_map(struct ctlr_info
*h,
dev->scsi3addr);
}
-static int hpsa_scsi_queue_command_lck(struct scsi_cmnd *cmd,
- void (*done)(struct scsi_cmnd *))
+/*
+ * Running in struct Scsi_Host->host_lock less mode using LLD internal
+ * struct ctlr_info *h->lock w/ spin_lock_irqsave() protection.
+ */
+static int hpsa_scsi_queue_command(struct Scsi_Host *sh, struct scsi_cmnd *cmd)
{
struct ctlr_info *h;
struct hpsa_scsi_dev_t *dev;
@@ -3895,14 +3898,14 @@ static int hpsa_scsi_queue_command_lck(struct scsi_cmnd
*cmd,
dev = cmd->device->hostdata;
if (!dev) {
cmd->result = DID_NO_CONNECT << 16;
- done(cmd);
+ cmd->scsi_done(cmd);
return 0;
}
memcpy(scsi3addr, dev->scsi3addr, sizeof(scsi3addr));
if (unlikely(lockup_detected(h))) {
cmd->result = DID_ERROR << 16;
- done(cmd);
+ cmd->scsi_done(cmd);
return 0;
}
c = cmd_alloc(h);
@@ -3912,9 +3915,6 @@ static int hpsa_scsi_queue_command_lck(struct scsi_cmnd
*cmd,
}
/* Fill in the command list header */
-
- cmd->scsi_done = done;/* save this for use by completion code */
-
/* save c in case we have to abort it */
cmd->host_scribble = (unsigned char *) c;
@@ -4005,8 +4005,6 @@ static int hpsa_scsi_queue_command_lck(struct scsi_cmnd
*cmd,
return 0;
}
-static DEF_SCSI_QCMD(hpsa_scsi_queue_command)
-
static int do_not_scan_if_controller_locked_up(struct ctlr_info *h)
{
unsigned long flags;
--
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 07/13] hpsa: fix endianness issue with scatter gather elements
From: Stephen M. Cameron
The hardware needs little endian scatter gather addresses and
lengths but we were not bothering to convert from cpu byte
order as we should have been. On Intel, this is all just
a bunch of no-ops macros, but it makes the code endian-clean(er).
Signed-off-by: Don Brace
Signed-off-by: Robert Elliott
Reviewed-by: Webb Scales
---
drivers/scsi/hpsa.c | 223 +--
drivers/scsi/hpsa_cmd.h | 14 +--
2 files changed, 107 insertions(+), 130 deletions(-)
diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index a221f7b..fe1b589 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -1502,22 +1502,22 @@ static int hpsa_map_sg_chain_block(struct ctlr_info *h,
{
struct SGDescriptor *chain_sg, *chain_block;
u64 temp64;
+ u32 chain_len;
chain_sg = &c->SG[h->max_cmd_sg_entries - 1];
chain_block = h->cmd_sg_list[c->cmdindex];
- chain_sg->Ext = HPSA_SG_CHAIN;
- chain_sg->Len = sizeof(*chain_sg) *
+ chain_sg->Ext = cpu_to_le32(HPSA_SG_CHAIN);
+ chain_len = sizeof(*chain_sg) *
(c->Header.SGTotal - h->max_cmd_sg_entries);
- temp64 = pci_map_single(h->pdev, chain_block, chain_sg->Len,
+ chain_sg->Len = cpu_to_le32(chain_len);
+ temp64 = pci_map_single(h->pdev, chain_block, chain_len,
PCI_DMA_TODEVICE);
if (dma_mapping_error(&h->pdev->dev, temp64)) {
/* prevent subsequent unmapping */
- chain_sg->Addr.lower = 0;
- chain_sg->Addr.upper = 0;
+ chain_sg->Addr = cpu_to_le64(0);
return -1;
}
- chain_sg->Addr.lower = (u32) (temp64 & 0x0ULL);
- chain_sg->Addr.upper = (u32) ((temp64 >> 32) & 0x0ULL);
+ chain_sg->Addr = cpu_to_le64(temp64);
return 0;
}
@@ -1525,15 +1525,13 @@ static void hpsa_unmap_sg_chain_block(struct ctlr_info
*h,
struct CommandList *c)
{
struct SGDescriptor *chain_sg;
- union u64bit temp64;
- if (c->Header.SGTotal <= h->max_cmd_sg_entries)
+ if (le16_to_cpu(c->Header.SGTotal) <= h->max_cmd_sg_entries)
return;
chain_sg = &c->SG[h->max_cmd_sg_entries - 1];
- temp64.val32.lower = chain_sg->Addr.lower;
- temp64.val32.upper = chain_sg->Addr.upper;
- pci_unmap_single(h->pdev, temp64.val, chain_sg->Len, PCI_DMA_TODEVICE);
+ pci_unmap_single(h->pdev, le64_to_cpu(chain_sg->Addr),
+ le32_to_cpu(chain_sg->Len), PCI_DMA_TODEVICE);
}
@@ -1734,8 +1732,7 @@ static void complete_scsi_command(struct CommandList *cp)
struct io_accel1_cmd *c = &h->ioaccel_cmd_pool[cp->cmdindex];
cp->Header.SGList = cp->Header.SGTotal = scsi_sg_count(cmd);
cp->Request.CDBLen = c->io_flags & IOACCEL1_IOFLAGS_CDBLEN_MASK;
- cp->Header.Tag.lower = c->Tag.lower;
- cp->Header.Tag.upper = c->Tag.upper;
+ cp->Header.tag = c->tag;
memcpy(cp->Header.LUN.LunAddrBytes, c->CISS_LUN, 8);
memcpy(cp->Request.CDB, c->CDB, cp->Request.CDBLen);
@@ -1936,14 +1933,11 @@ static void hpsa_pci_unmap(struct pci_dev *pdev,
struct CommandList *c, int sg_used, int data_direction)
{
int i;
- union u64bit addr64;
- for (i = 0; i < sg_used; i++) {
- addr64.val32.lower = c->SG[i].Addr.lower;
- addr64.val32.upper = c->SG[i].Addr.upper;
- pci_unmap_single(pdev, (dma_addr_t) addr64.val, c->SG[i].Len,
- data_direction);
- }
+ for (i = 0; i < sg_used; i++)
+ pci_unmap_single(pdev, (dma_addr_t) le64_to_cpu(c->SG[i].Addr),
+ le32_to_cpu(c->SG[i].Len),
+ data_direction);
}
static int hpsa_map_one(struct pci_dev *pdev,
@@ -1956,25 +1950,22 @@ static int hpsa_map_one(struct pci_dev *pdev,
if (buflen == 0 || data_direction == PCI_DMA_NONE) {
cp->Header.SGList = 0;
- cp->Header.SGTotal = 0;
+ cp->Header.SGTotal = cpu_to_le16(0);
return 0;
}
- addr64 = (u64) pci_map_single(pdev, buf, buflen, data_direction);
+ addr64 = pci_map_single(pdev, buf, buflen, data_direction);
if (dma_mapping_error(&pdev->dev, addr64)) {
/* Prevent subsequent unmap of something never mapped */
cp->Header.SGList = 0;
- cp->Header.SGTotal = 0;
+ cp->Header.SGTotal = cpu_to_le16(0);
return -1;
}
- cp->SG[0].Addr.lower =
- (u32) (addr64 & (u64) 0x);
- cp->SG[0].Addr.upper =
- (u32) ((addr64 >> 32) & (u64) 0x);
- cp->SG[0].Len = buflen;
- cp->SG[0].Ext = HPSA_SG_LAST; /* we are not chaining */
- cp->He
[PATCH 04/13] hpsa: correct off-by-one sizing of chained SG block
From: Webb Scales
Correct the size calculation of the chained SG block
Signed-off-by: Don Brace
Signed-off-by: Webb Scales
Reviewed-by: Stephen M. Cameron
Reviewed-by: Don Brace
---
drivers/scsi/hpsa.c |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index ed92e9e..f57081c 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -6321,11 +6321,11 @@ static void hpsa_find_board_params(struct ctlr_info *h)
h->max_cmd_sg_entries = 31;
if (h->maxsgentries > 512) {
h->max_cmd_sg_entries = 32;
- h->chainsize = h->maxsgentries - h->max_cmd_sg_entries + 1;
+ h->chainsize = h->maxsgentries - h->max_cmd_sg_entries;
h->maxsgentries--; /* save one for chain pointer */
} else {
- h->maxsgentries = 31; /* default to traditional values */
h->chainsize = 0;
+ h->maxsgentries = 31; /* default to traditional values */
}
/* Find out what task management functions are supported and cache */
--
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 06/13] hpsa: fix allocation sizes for CISS_REPORT_LUNs commands
From: Stephen M. Cameron
We were allocating roughly double the amount of memory
we should be due to ReportLUNdata and ExtendedReportLUNdata
containing a non-zero sized array but adding extra memory
to allocate as if the array were zero sized.
Track the logical and physical sizes separately.
Allocate the memory based on the specific data
structure sizes.
Signed-off-by: Don Brace
Reviewed-by: Webb Scales
---
drivers/scsi/hpsa.c | 14 +++---
drivers/scsi/hpsa_cmd.h |2 +-
2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index 538fdea..a221f7b 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -2893,7 +2893,7 @@ static int hpsa_get_pdisk_of_ioaccel2(struct ctlr_info *h,
* Returns 0 on success, -1 otherwise.
*/
static int hpsa_gather_lun_info(struct ctlr_info *h,
- int reportlunsize,
+ int reportphyslunsize, int reportloglunsize,
struct ReportLUNdata *physdev, u32 *nphysicals, int *physical_mode,
struct ReportLUNdata *logdev, u32 *nlogicals)
{
@@ -2907,7 +2907,7 @@ static int hpsa_gather_lun_info(struct ctlr_info *h,
*physical_mode = HPSA_REPORT_PHYS_EXTENDED;
physical_entry_size = 24;
}
- if (hpsa_scsi_do_report_phys_luns(h, physdev, reportlunsize,
+ if (hpsa_scsi_do_report_phys_luns(h, physdev, reportphyslunsize,
*physical_mode)) {
dev_err(&h->pdev->dev, "report physical LUNs failed.\n");
return -1;
@@ -2920,7 +2920,7 @@ static int hpsa_gather_lun_info(struct ctlr_info *h,
*nphysicals - HPSA_MAX_PHYS_LUN);
*nphysicals = HPSA_MAX_PHYS_LUN;
}
- if (hpsa_scsi_do_report_log_luns(h, logdev, reportlunsize)) {
+ if (hpsa_scsi_do_report_log_luns(h, logdev, reportloglunsize)) {
dev_err(&h->pdev->dev, "report logical LUNs failed.\n");
return -1;
}
@@ -3013,15 +3013,14 @@ static void hpsa_update_scsi_devices(struct ctlr_info
*h, int hostno)
u32 ndev_allocated = 0;
struct hpsa_scsi_dev_t **currentsd, *this_device, *tmpdevice;
int ncurrent = 0;
- int reportlunsize = sizeof(*physdev_list) + HPSA_MAX_PHYS_LUN * 24;
int i, n_ext_target_devs, ndevs_to_allocate;
int raid_ctlr_position;
int rescan_hba_mode;
DECLARE_BITMAP(lunzerobits, MAX_EXT_TARGETS);
currentsd = kzalloc(sizeof(*currentsd) * HPSA_MAX_DEVICES, GFP_KERNEL);
- physdev_list = kzalloc(reportlunsize, GFP_KERNEL);
- logdev_list = kzalloc(reportlunsize, GFP_KERNEL);
+ physdev_list = kzalloc(sizeof(*physdev_list), GFP_KERNEL);
+ logdev_list = kzalloc(sizeof(*logdev_list), GFP_KERNEL);
tmpdevice = kzalloc(sizeof(*tmpdevice), GFP_KERNEL);
if (!currentsd || !physdev_list || !logdev_list || !tmpdevice) {
@@ -3041,7 +3040,8 @@ static void hpsa_update_scsi_devices(struct ctlr_info *h,
int hostno)
h->hba_mode_enabled = rescan_hba_mode;
- if (hpsa_gather_lun_info(h, reportlunsize,
+ if (hpsa_gather_lun_info(h,
+ sizeof(*physdev_list), sizeof(*logdev_list),
(struct ReportLUNdata *) physdev_list, &nphysicals,
&physical_mode, logdev_list, &nlogicals))
goto out;
diff --git a/drivers/scsi/hpsa_cmd.h b/drivers/scsi/hpsa_cmd.h
index b5125dc..9b19042f 100644
--- a/drivers/scsi/hpsa_cmd.h
+++ b/drivers/scsi/hpsa_cmd.h
@@ -252,7 +252,7 @@ struct ReportExtendedLUNdata {
u8 LUNListLength[4];
u8 extended_response_flag;
u8 reserved[3];
- struct ext_report_lun_entry LUN[HPSA_MAX_LUN];
+ struct ext_report_lun_entry LUN[HPSA_MAX_PHYS_LUN];
};
struct SenseSubsystem_info {
--
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 10/13] hpsa: do not be so noisy about check conditions
From: Stephen M. Cameron
We were printing a lot of useless information before ultimately
just passing things up to the SCSI mid layer. Just let the
midlayer handle it without LLD chatter.
Signed-off-by: Don Brace
Signed-off-by: Stephen M. Cameron
Reviewed-by: Joe Handzik
Reviewed-by: Scott Teel
---
drivers/scsi/hpsa.c | 59 ---
1 file changed, 59 deletions(-)
diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index c9554e6..58f6847 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -1760,72 +1760,13 @@ static void complete_scsi_command(struct CommandList
*cp)
/* Get addition sense code qualifier */
ascq = ei->SenseInfo[13];
}
-
if (ei->ScsiStatus == SAM_STAT_CHECK_CONDITION) {
- if (check_for_unit_attention(h, cp))
- break;
- if (sense_key == ILLEGAL_REQUEST) {
- /*
-* SCSI REPORT_LUNS is commonly unsupported on
-* Smart Array. Suppress noisy complaint.
-*/
- if (cp->Request.CDB[0] == REPORT_LUNS)
- break;
-
- /* If ASC/ASCQ indicate Logical Unit
-* Not Supported condition,
-*/
- if ((asc == 0x25) && (ascq == 0x0)) {
- dev_warn(&h->pdev->dev, "cp %p "
- "has check condition\n", cp);
- break;
- }
- }
-
- if (sense_key == NOT_READY) {
- /* If Sense is Not Ready, Logical Unit
-* Not ready, Manual Intervention
-* required
-*/
- if ((asc == 0x04) && (ascq == 0x03)) {
- dev_warn(&h->pdev->dev, "cp %p "
- "has check condition: unit "
- "not ready, manual "
- "intervention required\n", cp);
- break;
- }
- }
if (sense_key == ABORTED_COMMAND) {
- /* Aborted command is retryable */
- dev_warn(&h->pdev->dev, "cp %p "
- "has check condition: aborted command: "
- "ASC: 0x%x, ASCQ: 0x%x\n",
- cp, asc, ascq);
cmd->result |= DID_SOFT_ERROR << 16;
break;
}
- /* Must be some other type of check condition */
- dev_dbg(&h->pdev->dev, "cp %p has check condition: "
- "unknown type: "
- "Sense: 0x%x, ASC: 0x%x, ASCQ: 0x%x, "
- "Returning result: 0x%x, "
- "cmd=[%02x %02x %02x %02x %02x "
- "%02x %02x %02x %02x %02x %02x "
- "%02x %02x %02x %02x %02x]\n",
- cp, sense_key, asc, ascq,
- cmd->result,
- cmd->cmnd[0], cmd->cmnd[1],
- cmd->cmnd[2], cmd->cmnd[3],
- cmd->cmnd[4], cmd->cmnd[5],
- cmd->cmnd[6], cmd->cmnd[7],
- cmd->cmnd[8], cmd->cmnd[9],
- cmd->cmnd[10], cmd->cmnd[11],
- cmd->cmnd[12], cmd->cmnd[13],
- cmd->cmnd[14], cmd->cmnd[15]);
break;
}
-
-
/* Problem was not a check condition
* Pass it up to the upper layers...
*/
--
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 12/13] hpsa: always call pci_set_master after pci_enable_device
From: Robert Elliott
If the kernel is booted with the reset_device parameter, which
is done for kdump, then the driver needs to call pci_set_master
after pci_enable_device to reenable bus mastering (since
the preceding pci_disable_device call disables bus mastering).
Also, place that after pci_request_regions both in the
kdump code and the normal pci_init code.
Remove the comment summarizing what pci_set_master
does, with the incomplete commentary on the impact of
pci_disable_device.
Signed-off-by: Robert Elliott
Signed-off-by: Don Brace
Reviewed-by: Don Brace
---
drivers/scsi/hpsa.c |8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index 761612d..72daca9 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -6363,15 +6363,15 @@ static int hpsa_pci_init(struct ctlr_info *h)
return err;
}
- /* Enable bus mastering (pci_disable_device may disable this) */
- pci_set_master(h->pdev);
-
err = pci_request_regions(h->pdev, HPSA);
if (err) {
dev_err(&h->pdev->dev,
"cannot obtain PCI resources, aborting\n");
return err;
}
+
+ pci_set_master(h->pdev);
+
hpsa_interrupt_mode(h);
err = hpsa_pci_find_memory_BAR(h->pdev, &h->paddr);
if (err)
@@ -6451,7 +6451,9 @@ static int hpsa_init_reset_devices(struct pci_dev *pdev)
dev_warn(&pdev->dev, "failed to enable device.\n");
return -ENODEV;
}
+
pci_set_master(pdev);
+
/* Reset the controller with a PCI power-cycle or via doorbell */
rc = hpsa_kdump_hard_reset_controller(pdev);
--
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 09/13] hpsa: use atomics for commands_outstanding
From: Stephen M. Cameron
Use atomics for commands_outstanding instead of protecting with spin locks.
Signed-off-by: Don Brace
Signed-off-by: Stephen M. Cameron
Reviewed-by: Joe Handzik
---
drivers/scsi/hpsa.c | 26 +-
drivers/scsi/hpsa.h | 27 +++
2 files changed, 16 insertions(+), 37 deletions(-)
diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index 95c34a5..c9554e6 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -394,7 +394,8 @@ static ssize_t host_show_commands_outstanding(struct device
*dev,
struct Scsi_Host *shost = class_to_shost(dev);
struct ctlr_info *h = shost_to_hba(shost);
- return snprintf(buf, 20, "%d\n", h->commands_outstanding);
+ return snprintf(buf, 20, "%d\n",
+ atomic_read(&h->commands_outstanding));
}
static ssize_t host_show_transport_mode(struct device *dev,
@@ -700,7 +701,6 @@ static inline u32 next_command(struct ctlr_info *h, u8 q)
{
u32 a;
struct reply_queue_buffer *rq = &h->reply_queue[q];
- unsigned long flags;
if (h->transMethod & CFGTBL_Trans_io_accel1)
return h->access.command_completed(h, q);
@@ -711,9 +711,7 @@ static inline u32 next_command(struct ctlr_info *h, u8 q)
if ((rq->head[rq->current_entry] & 1) == rq->wraparound) {
a = rq->head[rq->current_entry];
rq->current_entry++;
- spin_lock_irqsave(&h->lock, flags);
- h->commands_outstanding--;
- spin_unlock_irqrestore(&h->lock, flags);
+ atomic_dec(&h->commands_outstanding);
} else {
a = FIFO_EMPTY;
}
@@ -5445,15 +5443,9 @@ static void start_io(struct ctlr_info *h, unsigned long
*flags)
/* Put job onto the completed Q */
addQ(&h->cmpQ, c);
-
- /* Must increment commands_outstanding before unlocking
-* and submitting to avoid race checking for fifo full
-* condition.
-*/
- h->commands_outstanding++;
-
- /* Tell the controller execute command */
+ atomic_inc(&h->commands_outstanding);
spin_unlock_irqrestore(&h->lock, *flags);
+ /* Tell the controller execute command */
h->access.submit_command(h, c);
spin_lock_irqsave(&h->lock, *flags);
}
@@ -5499,6 +5491,7 @@ static inline void finish_cmd(struct CommandList *c)
unsigned long flags;
int io_may_be_stalled = 0;
struct ctlr_info *h = c->h;
+ int count;
spin_lock_irqsave(&h->lock, flags);
removeQ(c);
@@ -5519,11 +5512,10 @@ static inline void finish_cmd(struct CommandList *c)
* want to get in a cycle where we call start_io every time
* through here.
*/
- if (unlikely(h->fifo_recently_full) &&
- h->commands_outstanding < 5)
- io_may_be_stalled = 1;
-
+ count = atomic_read(&h->commands_outstanding);
spin_unlock_irqrestore(&h->lock, flags);
+ if (unlikely(h->fifo_recently_full) && count < 5)
+ io_may_be_stalled = 1;
dial_up_lockup_detection_on_fw_flash_complete(c->h, c);
if (likely(c->cmd_type == CMD_IOACCEL1 || c->cmd_type == CMD_SCSI
diff --git a/drivers/scsi/hpsa.h b/drivers/scsi/hpsa.h
index 80fa9a9..8e06d9e 100644
--- a/drivers/scsi/hpsa.h
+++ b/drivers/scsi/hpsa.h
@@ -118,7 +118,7 @@ struct ctlr_info {
struct CfgTable __iomem *cfgtable;
int interrupts_enabled;
int max_commands;
- int commands_outstanding;
+ atomic_t commands_outstanding;
# define PERF_MODE_INT0
# define DOORBELL_INT 1
# define SIMPLE_MODE_INT 2
@@ -395,7 +395,7 @@ static void SA5_performant_intr_mask(struct ctlr_info *h,
unsigned long val)
static unsigned long SA5_performant_completed(struct ctlr_info *h, u8 q)
{
struct reply_queue_buffer *rq = &h->reply_queue[q];
- unsigned long flags, register_value = FIFO_EMPTY;
+ unsigned long register_value = FIFO_EMPTY;
/* msi auto clears the interrupt pending bit. */
if (!(h->msi_vector || h->msix_vector)) {
@@ -413,9 +413,7 @@ static unsigned long SA5_performant_completed(struct
ctlr_info *h, u8 q)
if ((rq->head[rq->current_entry] & 1) == rq->wraparound) {
register_value = rq->head[rq->current_entry];
rq->current_entry++;
- spin_lock_irqsave(&h->lock, flags);
- h->commands_outstanding--;
- spin_unlock_irqrestore(&h->lock, flags);
+ atomic_dec(&h->commands_outstanding);
} else {
register_value = FIFO_EMPTY;
}
@@ -433,11 +431,7 @@ static unsigned long SA5_performant_completed(struct
ctlr_info *h, u8 q)
*/
static unsigned long SA5_fifo_full(struct ctlr_in
[PATCH 05/13] hpsa: remove 'action required' phrasing
From: Stephen M. Cameron In the case of LUN data changing, the driver will auto rescan and so it's not even true that "action" is "required". Remove "action required" phrases from warning messages and replace with description phrases. Signed-off-by: Don Brace Reviewed-by: Stephen M. Cameron Reviewed-by: Joe Handzik Reviewed-by: Webb Scales --- drivers/scsi/hpsa.c |8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index f57081c..538fdea 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -276,12 +276,12 @@ static int check_for_unit_attention(struct ctlr_info *h, "detected, command retried\n", h->ctlr); break; case LUN_FAILED: - dev_warn(&h->pdev->dev, HPSA "%d: LUN failure " - "detected, action required\n", h->ctlr); + dev_warn(&h->pdev->dev, + HPSA "%d: LUN failure detected\n", h->ctlr); break; case REPORT_LUNS_CHANGED: - dev_warn(&h->pdev->dev, HPSA "%d: report LUN data " - "changed, action required\n", h->ctlr); + dev_warn(&h->pdev->dev, + HPSA "%d: report LUN data changed\n", h->ctlr); /* * Note: this REPORT_LUNS_CHANGED condition only occurs on the external * target (array) devices. -- 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 13/13] hpsa: remove spin lock around command allocation
From: Stephen M. Cameron
It is already using atomic test_and_set_bit to do the
allocation.
There is some microscopic chance of starvation, but it is
so microscopic that it should never happen in reality.
Signed-off-by: Don Brace
Reviewed-by: Webb Scales
---
drivers/scsi/hpsa.c | 36 +++-
1 file changed, 23 insertions(+), 13 deletions(-)
diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index 72daca9..8a3d0de 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -4607,19 +4607,32 @@ static struct CommandList *cmd_alloc(struct ctlr_info
*h)
int i;
union u64bit temp64;
dma_addr_t cmd_dma_handle, err_dma_handle;
- unsigned long flags;
+ int loopcount;
+
+ /* There is some *extremely* small but non-zero chance that that
+* multiple threads could get in here, and one thread could
+* be scanning through the list of bits looking for a free
+* one, but the free ones are always behind him, and other
+* threads sneak in behind him and eat them before he can
+* get to them, so that while there is always a free one, a
+* very unlucky thread might be starved anyway, never able to
+* beat the other threads. In reality, this happens so
+* infrequently as to be indistinguishable from never.
+*/
- spin_lock_irqsave(&h->lock, flags);
+ loopcount = 0;
do {
i = find_first_zero_bit(h->cmd_pool_bits, h->nr_cmds);
- if (i == h->nr_cmds) {
- spin_unlock_irqrestore(&h->lock, flags);
- return NULL;
- }
- } while (test_and_set_bit
-(i & (BITS_PER_LONG - 1),
- h->cmd_pool_bits + (i / BITS_PER_LONG)) != 0);
- spin_unlock_irqrestore(&h->lock, flags);
+ if (i == h->nr_cmds)
+ i = 0;
+ loopcount++;
+ } while (test_and_set_bit(i & (BITS_PER_LONG - 1),
+ h->cmd_pool_bits + (i / BITS_PER_LONG)) != 0 &&
+ loopcount < 10);
+
+ /* Thread got starved? We do not expect this to ever happen. */
+ if (loopcount >= 10)
+ return NULL;
c = h->cmd_pool + i;
memset(c, 0, sizeof(*c));
@@ -4679,13 +4692,10 @@ static struct CommandList *cmd_special_alloc(struct
ctlr_info *h)
static void cmd_free(struct ctlr_info *h, struct CommandList *c)
{
int i;
- unsigned long flags;
i = c - h->cmd_pool;
- spin_lock_irqsave(&h->lock, flags);
clear_bit(i & (BITS_PER_LONG - 1),
h->cmd_pool_bits + (i / BITS_PER_LONG));
- spin_unlock_irqrestore(&h->lock, flags);
}
static void cmd_special_free(struct ctlr_info *h, struct CommandList *c)
--
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 03/13] hpsa: fix a couple pci id table mistakes
From: Stephen M. Cameron
Fix a couple of pci id table mistakes:
Subdevice ID 0x3323 missing from product[] table
(another name for HP Smart Storage 1210m)
Bogus 0x1925 subdevice id removed from hpsa_pci_device_id[] (no such thing.)
Signed-off-by: Don Brace
Reviewed-by: Webb Scales
---
drivers/scsi/hpsa.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index e24b304..ed92e9e 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -104,7 +104,6 @@ static const struct pci_device_id hpsa_pci_device_id[] = {
{PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSH, 0x103C, 0x1922},
{PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSH, 0x103C, 0x1923},
{PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSH, 0x103C, 0x1924},
- {PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSH, 0x103C, 0x1925},
{PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSH, 0x103C, 0x1926},
{PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSH, 0x103C, 0x1928},
{PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_CISSH, 0x103C, 0x1929},
@@ -150,6 +149,7 @@ static struct board_type products[] = {
{0x3249103C, "Smart Array P812", &SA5_access},
{0x324A103C, "Smart Array P712m", &SA5_access},
{0x324B103C, "Smart Array P711m", &SA5_access},
+ {0x3233103C, "HP StorageWorks 1210m", &SA5_access}, /* alias of 333f */
{0x3350103C, "Smart Array P222", &SA5_access},
{0x3351103C, "Smart Array P420", &SA5_access},
{0x3352103C, "Smart Array P421", &SA5_access},
--
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: Another (ESP?) scsi blk-mq problem on sparc64
On 2014-11-14 15:59, Meelis Roos wrote:
The second oops is in blk_mq_map_queue() which is a trivial
two level cpu lookup. I wonder if there's something odd about
cpu numbers on these big old sparc systems?
CPU numbers are sparse - they are determined by hardware slot number and
some models only fill every other mainboard slot, and first slots can be
free. I have first board offline and currently have CPUs numbered
10,11,14,15 online.
Here is debug with Jens's patch:
[ 133.971050] CPU 11: synchronized TICK with master CPU (last diff -1 cycles,
maxerr 516 cycles)
[ 133.975491] CPU 14: synchronized TICK with master CPU (last diff -3 cycles,
maxerr 531 cycles)
[ 133.979943] CPU 15: synchronized TICK with master CPU (last diff -3 cycles,
maxerr 531 cycles)
[ 133.980146] Brought up 4 CPUs
So this looks like this might be the issue. On a scsi-mq disabled boot,
you have 4 CPUs, but how are they numbered?
The numbers are always the same.
I would hope so, my question was really on what CPU numbers you see. But
I guess that 10, 11, 14, and 15?
But everything seems to be mapped to queue 0?
As it should, scsi-mq only supports a single hw queue for now.
We might need Christophs debug patch on top this to fully know...
Applied it too, dmesg is below. Yes it does spam the log a lot, and over
9600bps console its' somewhat slow :)
There is another detail to note -this server contains a faulty disk as
sdc that times out spinup. I left it in the server because it helped to
pinpoint and fix a previous error in esp scsi driver. This can be a
factor here too - the error handling details.
It could be. So we have tons of mappings from CPU10 to queue 0, but then
we see this:
[ 256.236742] cpu: 10
[ 256.236749] queue: 809119744
and it turns to crap. This is pretty weird. Try with this debug patch -
get rid of the other ones first. It should reduce your noise level too.
--
Jens Axboe
diff --git a/block/blk-mq-cpumap.c b/block/blk-mq-cpumap.c
index 1065d7c65fa1..9200e2aee746 100644
--- a/block/blk-mq-cpumap.c
+++ b/block/blk-mq-cpumap.c
@@ -81,6 +81,9 @@ int blk_mq_update_queue_map(unsigned int *map, unsigned int nr_queues)
map[i] = map[first_sibling];
}
+ for (i = 0; i < queue; i++)
+ printk(KERN_ERR "cpumap %d -> %d\n", i, map[i]);
+
free_cpumask_var(cpus);
return 0;
}
diff --git a/block/blk-mq.c b/block/blk-mq.c
index 68929bad9a6a..1678da3505ea 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -1265,12 +1265,25 @@ run_queue:
blk_mq_put_ctx(data.ctx);
}
+static int did_warn;
+
/*
* Default mapping to a software queue, since we use one per CPU.
*/
struct blk_mq_hw_ctx *blk_mq_map_queue(struct request_queue *q, const int cpu)
{
- return q->queue_hw_ctx[q->mq_map[cpu]];
+ int i;
+
+ i = q->mq_map[cpu];
+ if (!i || did_warn)
+ return q->queue_hw_ctx[0];
+
+ printk(KERN_ERR "blk-mq: cpu %u got queue %u\n", cpu, i);
+ for_each_online_cpu(i)
+ printk(KERN_ERR " cpu%d -> queue index %u\n", i, q->mq_map[i]);
+
+ did_warn = 1;
+ return q->queue_hw_ctx[0];
}
EXPORT_SYMBOL(blk_mq_map_queue);
Re: [V2 PATCH 3/4] scsi:stex.c Add reboot support
On Wed, Nov 12, 2014 at 09:27:50AM -0800, Christoph Hellwig wrote:
> > +static int stex_reboot_callback(struct notifier_block *self,
> > + unsigned long val,
> > + void *data)
> > +{
> > + if (val == SYS_RESTART)
> > + isRestart = 1;
> > + return NOTIFY_OK;
> > +}
> >
> > @@ -1832,7 +1859,14 @@ static void stex_shutdown(struct pci_dev *pdev)
> > {
> > struct st_hba *hba = pci_get_drvdata(pdev);
> >
> > - stex_hba_stop(hba);
> > + if (hba->yellowstone == 1)
> > + stex_hba_stop(hba, ST_IGNORED);
> > + else {
> > + if (isRestart)
> > + stex_hba_stop(hba, ST_S6);
> > + else
> > + stex_hba_stop(hba, ST_S5);
> > + }
>
> This sort of check for reboot vs restart isn't really something
> we want in drivers. I don't really know how we could find this
> out assuming we even want drivers to behave differently.
>
> Maybe Greg or someone on lkml has an idea how to best handle this case.
What is "this case"?
And yes, I agree, we shouldn't care, in drivers, about reboot vs.
restart, as they should both be the same thing, along with "disconnect",
right?
thanks,
greg k-h
--
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 V1] virtio_scsi: support multi hw queue of blk-mq
Since virtio_scsi has supported multi virtqueue already,
it is natural to map the virtque to hw-queue of blk-mq.
Cc: Paolo Bonzini
Signed-off-by: Ming Lei
---
V1:
- support non-mq too
drivers/scsi/virtio_scsi.c | 17 -
1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
index b83846f..d3af70e 100644
--- a/drivers/scsi/virtio_scsi.c
+++ b/drivers/scsi/virtio_scsi.c
@@ -561,6 +561,15 @@ static int virtscsi_queuecommand_single(struct Scsi_Host
*sh,
return virtscsi_queuecommand(vscsi, &vscsi->req_vqs[0], sc);
}
+static struct virtio_scsi_vq *virtscsi_pick_vq_mq(struct virtio_scsi *vscsi,
+ struct scsi_cmnd *sc)
+{
+ u32 tag = blk_mq_unique_tag(sc->request);
+ u16 hwq = blk_mq_unique_tag_to_hwq(tag);
+
+ return &vscsi->req_vqs[hwq];
+}
+
static struct virtio_scsi_vq *virtscsi_pick_vq(struct virtio_scsi *vscsi,
struct virtio_scsi_target_state
*tgt)
{
@@ -604,7 +613,12 @@ static int virtscsi_queuecommand_multi(struct Scsi_Host
*sh,
struct virtio_scsi *vscsi = shost_priv(sh);
struct virtio_scsi_target_state *tgt =
scsi_target(sc->device)->hostdata;
- struct virtio_scsi_vq *req_vq = virtscsi_pick_vq(vscsi, tgt);
+ struct virtio_scsi_vq *req_vq;
+
+ if (shost_use_blk_mq(sh))
+ req_vq = virtscsi_pick_vq_mq(vscsi, sc);
+ else
+ req_vq = virtscsi_pick_vq(vscsi, tgt);
return virtscsi_queuecommand(vscsi, req_vq, sc);
}
@@ -983,6 +997,7 @@ static int virtscsi_probe(struct virtio_device *vdev)
shost->max_id = num_targets;
shost->max_channel = 0;
shost->max_cmd_len = VIRTIO_SCSI_CDB_SIZE;
+ shost->nr_hw_queues = num_queues;
if (virtio_has_feature(vdev, VIRTIO_SCSI_F_T10_PI)) {
host_prot = SHOST_DIF_TYPE1_PROTECTION |
SHOST_DIF_TYPE2_PROTECTION |
--
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: Another (ESP?) scsi blk-mq problem on sparc64
> On 2014-11-14 15:59, Meelis Roos wrote: > > > > > The second oops is in blk_mq_map_queue() which is a trivial > > > > > two level cpu lookup. I wonder if there's something odd about > > > > > cpu numbers on these big old sparc systems? > > > > > > > > CPU numbers are sparse - they are determined by hardware slot number and > > > > some models only fill every other mainboard slot, and first slots can be > > > > free. I have first board offline and currently have CPUs numbered > > > > 10,11,14,15 online. > > > > > > > > Here is debug with Jens's patch: > > > > > > > [ 133.971050] CPU 11: synchronized TICK with master CPU (last diff -1 > > > > cycles, maxerr 516 cycles) > > > > [ 133.975491] CPU 14: synchronized TICK with master CPU (last diff -3 > > > > cycles, maxerr 531 cycles) > > > > [ 133.979943] CPU 15: synchronized TICK with master CPU (last diff -3 > > > > cycles, maxerr 531 cycles) > > > > [ 133.980146] Brought up 4 CPUs > > > > > > So this looks like this might be the issue. On a scsi-mq disabled boot, > > > you have 4 CPUs, but how are they numbered? > > > > The numbers are always the same. > > I would hope so, my question was really on what CPU numbers you see. But I > guess that 10, 11, 14, and 15? Yes, I am always seeing these CPU number, sorry if I was unclear. > > But everything seems to be mapped to queue 0? > > As it should, scsi-mq only supports a single hw queue for now. > > > > We might need Christophs debug patch on top this to fully know... > > > > Applied it too, dmesg is below. Yes it does spam the log a lot, and over > > 9600bps console its' somewhat slow :) > > > > There is another detail to note -this server contains a faulty disk as > > sdc that times out spinup. I left it in the server because it helped to > > pinpoint and fix a previous error in esp scsi driver. This can be a > > factor here too - the error handling details. > > It could be. So we have tons of mappings from CPU10 to queue 0, but then we > see this: > > > [ 256.236742] cpu: 10 > > [ 256.236749] queue: 809119744 > > and it turns to crap. This is pretty weird. Try with this debug patch - get > rid of the other ones first. It should reduce your noise level too. Here it is. This time it booted up: [0.00] PROMLIB: Sun IEEE Boot Prom 'OBP 3.2.29 2001/06/18 17:28' [0.00] PROMLIB: Root node compatible: [0.00] Linux version 3.18.0-rc4-00052-g04689e7-dirty (mroos@mandel) (gcc version 4.9.2 (Debian 4.9.2-1) ) #28 SMP Sat Nov 15 08:35:23 EET 2014 [0.00] debug: ignoring loglevel setting. [0.00] bootconsole [earlyprom0] enabled [0.00] ARCH: SUN4U [0.00] Ethernet address: 00:03:ba:12:70:5c [0.00] MM: PAGE_OFFSET is 0xf800 (max_phys_bits == 40) [0.00] MM: VMALLOC [0x0001 --> 0x0600] [0.00] MM: VMEMMAP [0x0600 --> 0x0c00] [0.00] Kernel: Using 2 locked TLB entries for main kernel image. [0.00] Remapping the kernel... done. [0.00] OF stdout device is: /central@1f,0/fhc@0,f880/zs@0,902000:a [0.00] PROM: Built device tree with 92727 bytes of memory. [0.00] Top of RAM: 0xffd16000, Total RAM: 0xff954000 [0.00] Memory hole size: 3MB [0.00] Allocated 16384 bytes for kernel page tables. [0.00] Zone ranges: [0.00] Normal [mem 0x-0xffd15fff] [0.00] Movable zone start for each node [0.00] Early memory node ranges [0.00] node 0: [mem 0x-0xff84dfff] [0.00] node 0: [mem 0xffc0-0xffce] [0.00] node 0: [mem 0xffd0-0xffd15fff] [0.00] Initmem setup node 0 [mem 0x-0xffd15fff] [0.00] On node 0 totalpages: 523434 [0.00] Normal zone: 4094 pages used for memmap [0.00] Normal zone: 0 pages reserved [0.00] Normal zone: 523434 pages, LIFO batch:15 [0.00] Booting Linux... [0.00] CPU CAPS: [flush,stbar,swap,muldiv,v9,mul32,div32,v8plus] [0.00] CPU CAPS: [vis] [0.00] PERCPU: Embedded 7 pages/cpu @f800ff40 s13248 r8192 d35904 u1048576 [0.00] pcpu-alloc: s13248 r8192 d35904 u1048576 alloc=1*4194304 [0.00] pcpu-alloc: [0] 10 11 14 15 [0.00] Built 1 zonelists in Zone order, mobility grouping on. Total pages: 519340 [0.00] Kernel command line: root=/dev/sda2 ro ignore_loglevel [0.00] PID hash table entries: 4096 (order: 2, 32768 bytes) [0.00] Dentry cache hash table entries: 524288 (order: 9, 4194304 bytes) [0.00] Inode-cache hash table entries: 262144 (order: 8, 2097152 bytes) [0.00] Sorting __ex_table... [0.00] Memory: 4142488K/4187472K available (3653K kernel code, 246K rwdata, 800K rodata, 184K init, 413K bss, 44984K reserved) [0.00] SLUB: HWalign=32, Order=0-3, MinObjects=0, CPUs=16, Nodes=1 [0.00] Hierarchical RCU implementation. [0.00] Additional pe
