Re: [PATCH v2 10/12] IB/srp: Use block layer tags
On 10/23/14 19:43, Webb Scales wrote:
On 10/23/14 3:16 AM, Bart Van Assche wrote:
All my tests with use_blk_mq=n were run with a WARN_ON_ONCE(req->tag <
0) statement present in srp_queuecommand(). I haven't seen any kernel
warning being triggered during the tests I ran.
Bart, what's the data type of "req->tag", here? (E.g., if it
"unsigned", it will never be less than zero, right?)
Hello Webb,
This is what I found in "struct request" in :
struct request {
[ ... ]
int tag;
[ ... ]
};
Bart.
--
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 v2 10/12] IB/srp: Use block layer tags
On Fri, Oct 24, 2014 at 04:43:15AM +, Elliott, Robert (Server Storage) wrote: > However, it was looking at scmd->tag, which is always 0xff (at > least in those early discovery commands). scmd->request->tag > looks like it is the field that has the correct values. > > Also, I noticed that scmd->tag is just an 8 bit field, so > it could never represent a large number of tags. Yes, we need to get rid of scmd->tag. Hannes had a patchset to get started on it, and I hope either he or someone else will have time to get back to it ASAP. > Just to confirm: After calling scsi_init_shared_tag_map() > in non-mq mode, will scmd->request->tag be based on > controller-wide tag allocation (never using the same > value at the same time for the request queues of multiple > devices in that controller)? Yes. -- 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 v2 10/12] IB/srp: Use block layer tags
> -Original Message- > From: Christoph Hellwig [mailto:h...@infradead.org] > Sent: Thursday, October 23, 2014 3:48 AM > To: Elliott, Robert (Server Storage) > Cc: Bart Van Assche; Jens Axboe; Sagi Grimberg; Sebastian Parschauer; > Ming Lei; linux-scsi@vger.kernel.org; linux-rdma; Scales, Webb; Don > Brace (PMC) > Subject: Re: [PATCH v2 10/12] IB/srp: Use block layer tags > > On Wed, Oct 22, 2014 at 10:03:24PM +, Elliott, Robert (Server > Storage) wrote: > > Have you tested this with scsi_mod.use_blk_mq=n? > > > > Trying similar changes in hpsa, we still receive some INQUIRY commands > > submitted through queuecommand with tag -1. They are for devices for > > which slave_alloc has not yet been run, implying this work needs to > > be done even earlier. Maybe the midlayer is missing a slave_alloc > > call somewhere? > > Did that version of hpsa really enable tagging in slave_alloc > or just in slave_configure? The latter would cause INQUIRY to be > sent untagged. Yes, it is slave_alloc, not slave_configure. However, it was looking at scmd->tag, which is always 0xff (at least in those early discovery commands). scmd->request->tag looks like it is the field that has the correct values. Also, I noticed that scmd->tag is just an 8 bit field, so it could never represent a large number of tags. Just to confirm: After calling scsi_init_shared_tag_map() in non-mq mode, will scmd->request->tag be based on controller-wide tag allocation (never using the same value at the same time for the request queues of multiple devices in that controller)? -- 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] scsi: Remove unnecesary label
Since commit "scsi: Fix error handling in SCSI_IOCTL_SEND_COMMAND",
the 'out' label is not used anymore. Thus, remove the unnecesary
label.
Cc: Jan Kara
Signed-off-by: Jingoo Han
---
block/scsi_ioctl.c |1 -
1 file changed, 1 deletion(-)
diff --git a/block/scsi_ioctl.c b/block/scsi_ioctl.c
index cc7927a..1e053d9 100644
--- a/block/scsi_ioctl.c
+++ b/block/scsi_ioctl.c
@@ -517,7 +517,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) {
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] scsi_debug: switch to table based parser
Changing a frequently hacked, big switch parser to being table
based is, of necessity, not a small patch. Testing showed up
some breakages which required extra code and re-factoring.
Since supporting the REPORT SUPPORTED OPERATION CODES command,
checking cdbs for non-zero values in reserved locations, and
the table based parser are closely related; implementing them
at the same time seemed to be practical. But some additions,
such as the COMPARE AND WRITE command, should really be in
their own patches (but adding new bugs was a useful technique
for finding existing ones).
The first attachment is large and against Christoph's
drivers-for-3.19 branch. It will also apply to his
drivers-for-3.18 branch. The second, smaller attachment is
for anyone who wants to look at (or try) this patch on
lk 3.17.1; first apply the second patch, then the first.
Almost all of my testing has been on lk 3.17.0 and .1 .
Perhaps contributors to this driver, such as Martin Petersen
who has written large parts of this driver (e.g. logical block
provisioning, DIF and error injection), might run any test
cases they have to determine what I have broken.
Speed improvements at this stage are marginal at best.
ChangeLog:
- remove big switch statement in queuecommand() and replace
with a table based parser
- implement REPORT SUPPORTED OPERATION CODES command which
reads that table
- add 'strict' option which when set will cause each incoming
cdb to be checked against the cdb usage mask held in the
table based parser
- add logic for ILLEGAL REQUEST sense key specific field
pointers, use for most ILLEGAL REQUESTs
- add 'capacity data has changed' unit attention since the
virtual_gb option can be changed on-the-fly
- implement REPORT SUPPORTED TASK MANAGEMENT FUNCTIONS command
- implement COMPARE AND WRITE command
- implement NDOB (no data-out buffer) in WRITE SAME(16)
- make GET LBA STATUS work when no logical block provisioning
Todo:
- re-order teardown in scsi_debug_exit()
- make sdebug_dev_info::stopped atomic (add to end of uas_bm ?)
- review Rob Elliott's suggestions; look at speed ups
- remove host_lock logic and make the host_lock option a dummy
- update some mode page and VPD data to reflect more recent
devices
- changing remaining >> and << byte handling over to
get/put_unaligned_be*()
- set INFO field for COMPARE AND WRITE command MISCOMPAREs
Signed-off-by: Douglas Gilbert
diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 2a215ee..a315d91 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -63,8 +63,8 @@
#include "sd.h"
#include "scsi_logging.h"
-#define SCSI_DEBUG_VERSION "1.84"
-static const char *scsi_debug_version_date = "20140706";
+#define SCSI_DEBUG_VERSION "1.85"
+static const char *scsi_debug_version_date = "20141022";
#define MY_NAME "scsi_debug"
@@ -75,19 +75,22 @@ static const char *scsi_debug_version_date = "20140706";
#define UNRECOVERED_READ_ERR 0x11
#define PARAMETER_LIST_LENGTH_ERR 0x1a
#define INVALID_OPCODE 0x20
-#define ADDR_OUT_OF_RANGE 0x21
-#define INVALID_COMMAND_OPCODE 0x20
+#define LBA_OUT_OF_RANGE 0x21
#define INVALID_FIELD_IN_CDB 0x24
#define INVALID_FIELD_IN_PARAM_LIST 0x26
#define UA_RESET_ASC 0x29
#define UA_CHANGED_ASC 0x2a
+#define INSUFF_RES_ASC 0x55
+#define INSUFF_RES_ASCQ 0x3
#define POWER_ON_RESET_ASCQ 0x0
#define BUS_RESET_ASCQ 0x2 /* scsi bus reset occurred */
#define MODE_CHANGED_ASCQ 0x1 /* mode parameters changed */
+#define CAPACITY_CHANGED_ASCQ 0x9
#define SAVING_PARAMS_UNSUP 0x39
#define TRANSPORT_PROBLEM 0x4b
#define THRESHOLD_EXCEEDED 0x5d
#define LOW_POWER_COND_ON 0x5e
+#define MISCOMPARE_VERIFY_ASC 0x1d
/* Additional Sense Code Qualifier (ASCQ) */
#define ACK_NAK_TO 0x3
@@ -133,6 +136,7 @@ static const char *scsi_debug_version_date = "20140706";
#define DEF_VIRTUAL_GB 0
#define DEF_VPD_USE_HOSTNO 1
#define DEF_WRITESAME_LENGTH 0x
+#define DEF_STRICT 0
#define DELAY_OVERRIDDEN -
/* bit mask values for scsi_debug_opts */
@@ -176,11 +180,12 @@ static const char *scsi_debug_version_date = "20140706";
#define SDEBUG_UA_POR 0 /* Power on, reset, or bus device reset */
#define SDEBUG_UA_BUS_RESET 1
#define SDEBUG_UA_MODE_CHANGED 2
-#define SDEBUG_NUM_UAS 3
+#define SDEBUG_UA_CAPACITY_CHANGED 3
+#define SDEBUG_NUM_UAS 4
/* for check_readiness() */
-#define UAS_ONLY 1
-#define UAS_TUR 0
+#define UAS_ONLY 1 /* check for UAs only */
+#define UAS_TUR 0 /* if no UAs then check if media access possible */
/* when 1==SCSI_DEBUG_OPT_MEDIUM_ERR, a medium error is simulated at this
* sector on read commands: */
@@ -206,6 +211,301 @@ static const char *scsi_debug_version_date = "20140706";
#warning "Expect DEF_CMD_PER_LUN <= SCSI_DEBUG_CANQUEUE"
#endif
+/* SCSI opcodes (first byte of cdb) mapped onto these indexes */
+enum sdeb_opcode_index {
+ SDEB_I_INVALID_OPCODE = 0,
+ SDEB_I_INQUIRY = 1,
+ SDEB_I_
[PATCH] scsi: Fix more error handling in SCSI_IOCTL_SEND_COMMAND
Fix an error path in SCSI_IOCTL_SEND_COMMAND that calls
blk_put_request(rq) on an invalid IS_ERR(rq) pointer.
Fixes: a492f075450f ("block,scsi: fixup blk_get_request dead queue
scenarios")
Signed-off-by: Tony Battersby
---
For inclusion in 3.18 only.
This does not conflict with the other recent fix to sg_scsi_ioctl() from
Jan Kara.
--- a/block/scsi_ioctl.c
+++ b/block/scsi_ioctl.c
@@ -458,7 +458,7 @@ int sg_scsi_ioctl(struct request_queue *q, struct gendisk
*disk, fmode_t mode,
rq = blk_get_request(q, in_len ? WRITE : READ, __GFP_WAIT);
if (IS_ERR(rq)) {
err = PTR_ERR(rq);
- goto error;
+ goto error_free_buffer;
}
blk_rq_set_block_pc(rq);
@@ -532,9 +532,11 @@ out:
}
error:
+ blk_put_request(rq);
+
+error_free_buffer:
kfree(buffer);
- if (rq)
- blk_put_request(rq);
+
return err;
}
EXPORT_SYMBOL_GPL(sg_scsi_ioctl);
--
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] lib/scatterlist: fix memory leak with scsi-mq
Fix a memory leak with scsi-mq triggered by commands with large data
transfer length.
Fixes: c53c6d6a68b1 ("scatterlist: allow chaining to preallocated chunks")
Cc: # 3.17.x
Signed-off-by: Tony Battersby
---
For inclusion in 3.18 and 3.17.x.
--- a/lib/scatterlist.c 2014-10-23 13:32:27.0 -0400
+++ b/lib/scatterlist.c 2014-10-23 13:32:36.0 -0400
@@ -203,10 +203,10 @@ void __sg_free_table(struct sg_table *ta
}
table->orig_nents -= sg_size;
- if (!skip_first_chunk) {
- free_fn(sgl, alloc_size);
+ if (skip_first_chunk)
skip_first_chunk = false;
- }
+ else
+ free_fn(sgl, alloc_size);
sgl = next;
}
--
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 v2 10/12] IB/srp: Use block layer tags
On 10/23/14 3:16 AM, Bart Van Assche wrote: All my tests with use_blk_mq=n were run with a WARN_ON_ONCE(req->tag < 0) statement present in srp_queuecommand(). I haven't seen any kernel warning being triggered during the tests I ran. Bart, what's the data type of "req->tag", here? (E.g., if it "unsigned", it will never be less than zero, right?) Thanks, Webb -- 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: fix blk-mq for SPI hosts
On 10/23/2014 11:14 AM, Christoph Hellwig wrote: > On Thu, Oct 23, 2014 at 01:49:07PM +0300, Meelis Roos wrote: >>> ping? >> >> Sorry, forgot to reply. Yes, it worked fine, on the initial Ultra 1 and >> additionally on Ultra 2 too. > > Thanks! > > Can I get a review from Jens and some SCSI developers, too? Yes, I did read them, you can add my reviewed/acked. > Jens, are you fine taking the blkdev.h revert through the SCSI tree? Sure, that seems to be the easiest way to do it. -- 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: fix blk-mq for SPI hosts
On Thu, Oct 23, 2014 at 01:49:07PM +0300, Meelis Roos wrote: > > ping? > > Sorry, forgot to reply. Yes, it worked fine, on the initial Ultra 1 and > additionally on Ultra 2 too. Thanks! Can I get a review from Jens and some SCSI developers, too? Jens, are you fine taking the blkdev.h revert through the SCSI tree? -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/6] various fixes for UFS-PM series
On Thu, Oct 23, 2014 at 01:25:11PM +0300, Dolev Raviv wrote: > Contains a couple of bug fixes reported by Akinobu Mita, In adition to > "static checker" warnings reported by Dan Carpenter. Any reason you didn't add a signoff to the first patch? I'll happily apply these after I get a second review for them, preferably from the ufs community. -- 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] tmscim: Remove unused SCSI_IRQ_NONE macro definition
Thanks, applied to the drivers-for-3.19 branch. -- 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] scsi_debug: Error message should say scsi_host_alloc not scsi_register
Thanks, applied to the drivers-for-3.19 branch. -- 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] Fix trivial typos in scsi_scan.c comment
Thanks, applied to the core-for-3.19 branch. -- 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] Fix off-by-one LUN check in scsi_scan_host_selected()
Thanks, applied to the core-for-3.19 branch. -- 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] scsi: Resolve some missing-field-initializers warnings
Thanks, applied to the core-for-3.19 branch. -- 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 v3] SG_SCSI_RESET ioctl: add no_escalate values
Thanks, applied to the core-for-3.19 branch. -- 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]: add debug flag parameter for SCSI tape driver - 2nd request
Thanks, I've applied this patch to the core-for-3.19 branch after fixing a few whitespace issues. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/1 linux-next] bsg: fix shadow warning in bsg_ioctl()
directly call scsi_cmd_ioctl() with (void __user *)arg instead
of uarg redeclaration.
Signed-off-by: Fabian Frederick
---
block/bsg.c | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/block/bsg.c b/block/bsg.c
index 276e869..c2f90bd 100644
--- a/block/bsg.c
+++ b/block/bsg.c
@@ -921,10 +921,9 @@ static long bsg_ioctl(struct file *file, unsigned int cmd,
unsigned long arg)
case SG_GET_RESERVED_SIZE:
case SG_SET_RESERVED_SIZE:
case SG_EMULATED_HOST:
- case SCSI_IOCTL_SEND_COMMAND: {
- void __user *uarg = (void __user *) arg;
- return scsi_cmd_ioctl(bd->queue, NULL, file->f_mode, cmd, uarg);
- }
+ case SCSI_IOCTL_SEND_COMMAND:
+ return scsi_cmd_ioctl(bd->queue, NULL, file->f_mode, cmd,
+ (void __user *)arg);
case SG_IO: {
struct request *rq;
struct bio *bio, *bidi_bio = NULL;
--
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
[PATCH] uas: Add US_FL_NO_ATA_1X quirk for 2 more Seagate models
These drives hang when receiving ATA12 commands, so set the US_FL_NO_ATA_1X quirk to filter these out. Cc: sta...@vger.kernel.org # 3.16 Signed-off-by: Hans de Goede --- drivers/usb/storage/unusual_uas.h | 14 ++ 1 file changed, 14 insertions(+) diff --git a/drivers/usb/storage/unusual_uas.h b/drivers/usb/storage/unusual_uas.h index a801850..44ab3289 100644 --- a/drivers/usb/storage/unusual_uas.h +++ b/drivers/usb/storage/unusual_uas.h @@ -54,6 +54,20 @@ UNUSUAL_DEV(0x0bc2, 0x3312, 0x, 0x, USB_SC_DEVICE, USB_PR_DEVICE, NULL, US_FL_NO_ATA_1X), +/* Reported-by: Hans de Goede */ +UNUSUAL_DEV(0x0bc2, 0x3320, 0x, 0x, + "Seagate", + "Expansion Desk", + USB_SC_DEVICE, USB_PR_DEVICE, NULL, + US_FL_NO_ATA_1X), + +/* Reported-by: Bogdan Mihalcea */ +UNUSUAL_DEV(0x0bc2, 0xa003, 0x, 0x, + "Seagate", + "Backup Plus", + USB_SC_DEVICE, USB_PR_DEVICE, NULL, + US_FL_NO_ATA_1X), + /* https://bbs.archlinux.org/viewtopic.php?id=183190 */ UNUSUAL_DEV(0x0bc2, 0xab20, 0x, 0x, "Seagate", -- 2.1.0 -- 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: fix blk-mq for SPI hosts
> ping? Sorry, forgot to reply. Yes, it worked fine, on the initial Ultra 1 and additionally on Ultra 2 too. > On Sun, Oct 19, 2014 at 05:13:56PM +0200, Christoph Hellwig wrote: > > Fix the assumption that we can treat all blk-mq requests as tagged. For > > traditional SCSI that's wrong, as being tagged has a very explicit meaning > > on the wire. > > > > This is a little bit different from the version Meelis tested earlier, but > > the concept is the same. I didn't want to add a Tested-by tag as it's > > different enough, though. > > > > -- > > 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 > ---end quoted text--- > -- Meelis Roos (mr...@linux.ee) -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 6/6] scsi: ufs: fix static checker warning in ufshcd_parse_clock_info
This patch fixes newly introduced static checker warning in
ufshcd_parse_clock_info, introduced by UFS power management series.
Warning:
drivers/scsi/ufs/ufshcd-pltfrm.c:138 ufshcd_parse_clock_info()
warn: passing devm_ allocated variable to kfree. 'clkfreq'
To fix it we remove the kfree(clkfreq) statement.
In addition we removed the redundant goto label.
Signed-off-by: Dolev Raviv
diff --git a/drivers/scsi/ufs/ufshcd-pltfrm.c b/drivers/scsi/ufs/ufshcd-pltfrm.c
index 2cdec78..1c3467b 100644
--- a/drivers/scsi/ufs/ufshcd-pltfrm.c
+++ b/drivers/scsi/ufs/ufshcd-pltfrm.c
@@ -102,7 +102,6 @@ static int ufshcd_parse_clock_info(struct ufs_hba *hba)
clkfreq = devm_kzalloc(dev, sz * sizeof(*clkfreq),
GFP_KERNEL);
if (!clkfreq) {
- dev_err(dev, "%s: no memory\n", "freq-table-hz");
ret = -ENOMEM;
goto out;
}
@@ -112,19 +111,19 @@ static int ufshcd_parse_clock_info(struct ufs_hba *hba)
if (ret && (ret != -EINVAL)) {
dev_err(dev, "%s: error reading array %d\n",
"freq-table-hz", ret);
- goto free_clkfreq;
+ return ret;
}
for (i = 0; i < sz; i += 2) {
ret = of_property_read_string_index(np,
"clock-names", i/2, (const char **)&name);
if (ret)
- goto free_clkfreq;
+ goto out;
clki = devm_kzalloc(dev, sizeof(*clki), GFP_KERNEL);
if (!clki) {
ret = -ENOMEM;
- goto free_clkfreq;
+ goto out;
}
clki->min_freq = clkfreq[i];
@@ -134,8 +133,6 @@ static int ufshcd_parse_clock_info(struct ufs_hba *hba)
clki->min_freq, clki->max_freq, clki->name);
list_add_tail(&clki->list, &hba->clk_list_head);
}
-free_clkfreq:
- kfree(clkfreq);
out:
return ret;
}
--
1.8.5.2
--
Qualcomm Israel, on behalf of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux
Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/6] scsi: ufs: fix static checker errors in ufshcd_system_suspend
This patch fixes newly introduced sparse warning in
ufshcd_system_suspend, introduced by UFS power management series.
Sparse warning:
drivers/scsi/ufs/ufshcd.c:5118 ufshcd_system_suspend()
error: we previously assumed 'hba' could be null (see line 5089)
To fix it, we return 0 in case HBA is not initialized or is
not powered.
Signed-off-by: Dolev Raviv
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 77a4e38..d3f6ddb 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -5104,7 +5104,7 @@ int ufshcd_system_suspend(struct ufs_hba *hba)
int ret = 0;
if (!hba || !hba->is_powered)
- goto out;
+ return 0;
if (pm_runtime_suspended(hba->dev)) {
if (hba->rpm_lvl == hba->spm_lvl)
--
1.8.5.2
--
Qualcomm Israel, on behalf of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux
Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/6] scsi: ufs: fix static checker warning in ufshcd_populate_vreg
This patch fixes newly introduced static checker warning in
ufshcd_populate_vreg, introduced by UFS power management series.
Warning:
drivers/scsi/ufs/ufshcd-pltfrm.c:167 ufshcd_populate_vreg()
warn: missing error code here? 'devm_kzalloc()' failed. 'ret' = '0'
To fix it we return -ENOMEM and skip the message print.
Signed-off-by: Dolev Raviv
diff --git a/drivers/scsi/ufs/ufshcd-pltfrm.c b/drivers/scsi/ufs/ufshcd-pltfrm.c
index 8adf067..2cdec78 100644
--- a/drivers/scsi/ufs/ufshcd-pltfrm.c
+++ b/drivers/scsi/ufs/ufshcd-pltfrm.c
@@ -162,10 +162,8 @@ static int ufshcd_populate_vreg(struct device *dev, const
char *name,
}
vreg = devm_kzalloc(dev, sizeof(*vreg), GFP_KERNEL);
- if (!vreg) {
- dev_err(dev, "No memory for %s regulator\n", name);
- goto out;
- }
+ if (!vreg)
+ return -ENOMEM;
vreg->name = kstrdup(name, GFP_KERNEL);
--
1.8.5.2
--
Qualcomm Israel, on behalf of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux
Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/6] scsi: ufs: fix reference counting of W-LUs
From: Akinobu Mita
UFS driver adds three well known LUs in the initialization, but those
reference counts are not decremented, so it makes ufshcd module
impossible to unload.
This fixes it by putting scsi_device_put() in the initalization, and in
order to protect concurrent access to hba->sdev_ufs_device (UFS Device
W-LU) from manual delete, increment the reference count while requesting
device power mode setting.
The rest of W-LUs (hba->sdev_boot and hba->sdev_rpmb) are not directly
used from driver, so these references in struct ufs_hba are removed.
Signed-off-by: Akinobu Mita
Cc: Vinayak Holikatti
Cc: Santosh Y
Cc: Dolev Raviv
Cc: Subhash Jadavani
Cc: Yaniv Gardi
Cc: Christoph Hellwig
Cc: "James E.J. Bottomley"
Cc: linux-scsi@vger.kernel.org
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 497c38a..59b6544 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -2844,8 +2844,13 @@ static void ufshcd_slave_destroy(struct scsi_device
*sdev)
hba = shost_priv(sdev->host);
scsi_deactivate_tcq(sdev, hba->nutrs);
/* Drop the reference as it won't be needed anymore */
- if (ufshcd_scsi_to_upiu_lun(sdev->lun) == UFS_UPIU_UFS_DEVICE_WLUN)
+ if (ufshcd_scsi_to_upiu_lun(sdev->lun) == UFS_UPIU_UFS_DEVICE_WLUN) {
+ unsigned long flags;
+
+ spin_lock_irqsave(hba->host->host_lock, flags);
hba->sdev_ufs_device = NULL;
+ spin_unlock_irqrestore(hba->host->host_lock, flags);
+ }
}
/**
@@ -4062,6 +4067,8 @@ static void ufshcd_init_icc_levels(struct ufs_hba *hba)
static int ufshcd_scsi_add_wlus(struct ufs_hba *hba)
{
int ret = 0;
+ struct scsi_device *sdev_rpmb;
+ struct scsi_device *sdev_boot;
hba->sdev_ufs_device = __scsi_add_device(hba->host, 0, 0,
ufshcd_upiu_wlun_to_scsi_wlun(UFS_UPIU_UFS_DEVICE_WLUN), NULL);
@@ -4070,26 +4077,27 @@ static int ufshcd_scsi_add_wlus(struct ufs_hba *hba)
hba->sdev_ufs_device = NULL;
goto out;
}
+ scsi_device_put(hba->sdev_ufs_device);
- hba->sdev_boot = __scsi_add_device(hba->host, 0, 0,
+ sdev_boot = __scsi_add_device(hba->host, 0, 0,
ufshcd_upiu_wlun_to_scsi_wlun(UFS_UPIU_BOOT_WLUN), NULL);
- if (IS_ERR(hba->sdev_boot)) {
- ret = PTR_ERR(hba->sdev_boot);
- hba->sdev_boot = NULL;
+ if (IS_ERR(sdev_boot)) {
+ ret = PTR_ERR(sdev_boot);
goto remove_sdev_ufs_device;
}
+ scsi_device_put(sdev_boot);
- hba->sdev_rpmb = __scsi_add_device(hba->host, 0, 0,
+ sdev_rpmb = __scsi_add_device(hba->host, 0, 0,
ufshcd_upiu_wlun_to_scsi_wlun(UFS_UPIU_RPMB_WLUN), NULL);
- if (IS_ERR(hba->sdev_rpmb)) {
- ret = PTR_ERR(hba->sdev_rpmb);
- hba->sdev_rpmb = NULL;
+ if (IS_ERR(sdev_rpmb)) {
+ ret = PTR_ERR(sdev_rpmb);
goto remove_sdev_boot;
}
+ scsi_device_put(sdev_rpmb);
goto out;
remove_sdev_boot:
- scsi_remove_device(hba->sdev_boot);
+ scsi_remove_device(sdev_boot);
remove_sdev_ufs_device:
scsi_remove_device(hba->sdev_ufs_device);
out:
@@ -4097,30 +4105,6 @@ out:
}
/**
- * ufshcd_scsi_remove_wlus - Removes the W-LUs which were added by
- * ufshcd_scsi_add_wlus()
- * @hba: per-adapter instance
- *
- */
-static void ufshcd_scsi_remove_wlus(struct ufs_hba *hba)
-{
- if (hba->sdev_ufs_device) {
- scsi_remove_device(hba->sdev_ufs_device);
- hba->sdev_ufs_device = NULL;
- }
-
- if (hba->sdev_boot) {
- scsi_remove_device(hba->sdev_boot);
- hba->sdev_boot = NULL;
- }
-
- if (hba->sdev_rpmb) {
- scsi_remove_device(hba->sdev_rpmb);
- hba->sdev_rpmb = NULL;
- }
-}
-
-/**
* ufshcd_probe_hba - probe hba to detect device and initialize
* @hba: per-adapter instance
*
@@ -4675,11 +4659,25 @@ static int ufshcd_set_dev_pwr_mode(struct ufs_hba *hba,
{
unsigned char cmd[6] = { START_STOP };
struct scsi_sense_hdr sshdr;
- struct scsi_device *sdp = hba->sdev_ufs_device;
+ struct scsi_device *sdp;
+ unsigned long flags;
int ret;
- if (!sdp || !scsi_device_online(sdp))
- return -ENODEV;
+ spin_lock_irqsave(hba->host->host_lock, flags);
+ sdp = hba->sdev_ufs_device;
+ if (sdp) {
+ ret = scsi_device_get(sdp);
+ if (!ret && !scsi_device_online(sdp)) {
+ ret = -ENODEV;
+ scsi_device_put(sdp);
+ }
+ } else {
+ ret = -ENODEV;
+ }
+ spin_unlock_irqrestore(hba->host->host_lock, flags);
+
+ if (ret)
+ return ret;
/*
* If scsi commands fail, the scsi
[PATCH 2/6] scsi: ufs: fix power info after link start-up
From: Yaniv Gardi
After link start-up power mode will always be PWM G1. This is not
reflected in the pwr_info struct which will keep the previous values.
Since ufshcd_change_power_mode() tries to avoid unnecessary power mode
change if the requested power mode and current power mode are same,
power mode change won't execute again after driver initialization.
This patch solves the problem by setting pwr_info to PWM G1 after link
start-up.
Signed-off-by: Yaniv Gardi
Signed-off-by: Dolev Raviv
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 59b6544..77a4e38 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -2246,6 +2246,22 @@ static int ufshcd_uic_hibern8_exit(struct ufs_hba *hba)
return ret;
}
+ /**
+ * ufshcd_init_pwr_info - setting the POR (power on reset)
+ * values in hba power info
+ * @hba: per-adapter instance
+ */
+static void ufshcd_init_pwr_info(struct ufs_hba *hba)
+{
+ hba->pwr_info.gear_rx = UFS_PWM_G1;
+ hba->pwr_info.gear_tx = UFS_PWM_G1;
+ hba->pwr_info.lane_rx = 1;
+ hba->pwr_info.lane_tx = 1;
+ hba->pwr_info.pwr_rx = SLOWAUTO_MODE;
+ hba->pwr_info.pwr_tx = SLOWAUTO_MODE;
+ hba->pwr_info.hs_rate = 0;
+}
+
/**
* ufshcd_get_max_pwr_mode - reads the max power mode negotiated with device
* @hba: per-adapter instance
@@ -4118,6 +4134,8 @@ static int ufshcd_probe_hba(struct ufs_hba *hba)
if (ret)
goto out;
+ ufshcd_init_pwr_info(hba);
+
/* UniPro link is active now */
ufshcd_set_link_active(hba);
--
1.8.5.2
--
Qualcomm Israel, on behalf of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux
Foundation Collaborative Project
--
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 0/6] various fixes for UFS-PM series
Contains a couple of bug fixes reported by Akinobu Mita, In adition to "static checker" warnings reported by Dan Carpenter. Akinobu Mita (1): scsi: ufs: fix reference counting of W-LUs Dolev Raviv (4): scsi: ufs: fix static checker errors in ufshcd_system_suspend scsi: ufs: fix static checker warning in ufshcd_populate_vreg scsi: ufs: fix static checker warning in __ufshcd_setup_clocks scsi: ufs: fix static checker warning in ufshcd_parse_clock_info Yaniv Gardi (1): scsi: ufs: fix power info after link start-up drivers/scsi/ufs/ufshcd-pltfrm.c | 15 +++ drivers/scsi/ufs/ufshcd.c| 96 +++- drivers/scsi/ufs/ufshcd.h| 2 - 3 files changed, 61 insertions(+), 52 deletions(-) -- 1.8.5.2 -- Qualcomm Israel, on behalf of Qualcomm Innovation Center, Inc. Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/6] scsi: ufs: fix static checker warning in __ufshcd_setup_clocks
This patch fixes newly introduced static checker warning in
__ufshcd_setup_clocks, introduced by UFS power management series.
Warning:
drivers/scsi/ufs/ufshcd.c:4474 __ufshcd_setup_clocks()
warn: we tested 'ret' before and it was 'false'
To fix it we remove the (!ret) from the condition.
Signed-off-by: Dolev Raviv
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index d3f6ddb..b9da446 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -4473,7 +4473,7 @@ out:
if (!IS_ERR_OR_NULL(clki->clk) && clki->enabled)
clk_disable_unprepare(clki->clk);
}
- } else if (!ret && on) {
+ } else if (on) {
spin_lock_irqsave(hba->host->host_lock, flags);
hba->clk_gating.state = CLKS_ON;
spin_unlock_irqrestore(hba->host->host_lock, flags);
--
1.8.5.2
--
Qualcomm Israel, on behalf of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux
Foundation Collaborative Project
--
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: ufs: add UFS power management support
On Thu, Oct 23, 2014 at 10:28:52AM +0300, Dolev Raviv wrote:
> Hi Dan,
> This seem like a false alarm, please let me know if it requires a fix.
>
It may not indicate a bug, but it is correct in that the code could be
re-written:
Old:
else if ((req_link_state == UIC_LINK_OFF_STATE) &&
(!check_for_bkops || (check_for_bkops &&
!hba->auto_bkops_enabled))) {
Proposed:
else if ((req_link_state == UIC_LINK_OFF_STATE) &&
(!check_for_bkops || !hba->auto_bkops_enabled)) {
Those two are the same.
regards,
dan carpenter
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] scsi: Add Hyper-V logical block provisioning quirks
On 23 October 2014 02:50, Martin K. Petersen wrote:
>> "Sitsofe" == Sitsofe Wheeler writes:
>
> Sitsofe> 2. On top of the above, when a disk is "small" (has less than
> Sitsofe>2^32 sectors which is typically < 2 TBytes in size) READ
> Sitsofe>CAPACITY(16) won't be triggered.
>
> static int sd_try_rc16_first(struct scsi_device *sdp)
> {
> if (sdp->host->max_cmd_len < 16)
> return 0;
> if (sdp->try_rc_10_first)
> return 0;
> if (sdp->scsi_level > SCSI_SPC_2)
> return 1;
> if (scsi_device_protection(sdp))
> return 1;
> return 0;
> }
Yes but since SCSI_SPC_2 was being advertised by the Hyper-V virtual
disk (and presumably device protection isn't advertised either) this
function was returning 0. That's why the patch in
https://lkml.org/lkml/2014/10/10/49 added yet another quirk. If if
SPC_3 were correctly advertised on Hyper-V virtual disks would
TRY_VPD_PAGES still be needed to cope correctly with passthrough
disks?
What I didn't realise was that the TRY_VPD_PAGES quirk was purely to
fix WRITE_SAME Hyper-V issues and not to address anything thin
provisioning related. Having said that, why was the TRY_VPD_PAGES
quirk back-ported 3.14 without any users -
https://lkml.org/lkml/2014/9/15/733 ?
--
Sitsofe | http://sucs.org/~sits/
--
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 v2 10/12] IB/srp: Use block layer tags
On Wed, Oct 22, 2014 at 10:03:24PM +, Elliott, Robert (Server Storage) wrote: > Have you tested this with scsi_mod.use_blk_mq=n? > > Trying similar changes in hpsa, we still receive some INQUIRY commands > submitted through queuecommand with tag -1. They are for devices for > which slave_alloc has not yet been run, implying this work needs to > be done even earlier. Maybe the midlayer is missing a slave_alloc > call somewhere? Did that version of hpsa really enable tagging in slave_alloc or just in slave_configure? The latter would cause INQUIRY to be sent untagged. -- 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: fix blk-mq for SPI hosts
ping? On Sun, Oct 19, 2014 at 05:13:56PM +0200, Christoph Hellwig wrote: > Fix the assumption that we can treat all blk-mq requests as tagged. For > traditional SCSI that's wrong, as being tagged has a very explicit meaning > on the wire. > > This is a little bit different from the version Meelis tested earlier, but > the concept is the same. I didn't want to add a Tested-by tag as it's > different enough, though. > > -- > 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 ---end quoted text--- -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/5] Feature enhancements for ses module
On Wed, Oct 22, 2014 at 01:12:01PM -0600, Jens Axboe wrote: > Guys, where are we with these? Seemed like they got reviewed (and > updates/fixes posted), then nothing else happened. Would be nice to get > these upstream, so we don't have to carry them at FB. Nothign happened, I guess mostly like due to the odd way it was posted, seemingly in reply to something else that wasn't sent in mutt. I'll way for Song and Doug to agree on the reserved field handling and will apply it once all remaining issues there are sorted out. -- 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: ufs: add UFS power management support
Hi Dan,
This seem like a false alarm, please let me know if it requires a fix.
Thanks,
Dolev
--
Qualcomm Israel, on behalf of Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
-Original Message-
From: Dan Carpenter [mailto:dan.carpen...@oracle.com]
Sent: Thursday, October 02, 2014 6:31 PM
To: subha...@codeaurora.org
Cc: linux-scsi@vger.kernel.org
Subject: re: ufs: add UFS power management support
Hello Subhash Jadavani,
The patch 57d104c153d3: "ufs: add UFS power management support" from Sep 25,
2014, leads to the following static checker warning:
drivers/scsi/ufs/ufshcd.c:4746 ufshcd_link_state_transition()
warn: we tested 'check_for_bkops' before and it was 'true'
drivers/scsi/ufs/ufshcd.c
4734 if (req_link_state == UIC_LINK_HIBERN8_STATE) {
4735 ret = ufshcd_uic_hibern8_enter(hba);
4736 if (!ret)
4737 ufshcd_set_link_hibern8(hba);
4738 else
4739 goto out;
4740 }
4741 /*
4742 * If autobkops is enabled, link can't be turned off because
4743 * turning off the link would also turn off the device.
4744 */
4745 else if ((req_link_state == UIC_LINK_OFF_STATE) &&
4746 (!check_for_bkops || (check_for_bkops &&
^^^ Not needed.
4747 !hba->auto_bkops_enabled))) {
4748 /*
4749 * Change controller state to "reset state" which
4750 * should also put the link in off/reset state
4751 */
4752 ufshcd_hba_stop(hba);
4753 /*
4754 * TODO: Check if we need any delay to make sure
that
4755 * controller is reset
4756 */
4757 ufshcd_set_link_off(hba);
4758 }
4759
4760 out:
4761 return ret;
4762 }
regards,
dan carpenter
--
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 v2 10/12] IB/srp: Use block layer tags
On 10/23/14 00:03, Elliott, Robert (Server Storage) wrote:
-Original Message-
From: Bart Van Assche [mailto:bvanass...@acm.org]
Sent: Tuesday, 07 October, 2014 8:07 AM
...
@@ -1927,7 +1931,7 @@ static int srp_queuecommand(struct Scsi_Host
*shost, struct scsi_cmnd *scmnd)
cmd->opcode = SRP_CMD;
cmd->lun= cpu_to_be64((u64) scmnd->device->lun << 48);
- cmd->tag= req->index;
+ cmd->tag= tag;
memcpy(cmd->cdb, scmnd->cmnd, scmnd->cmd_len);
req->scmnd= scmnd;
...
+static int srp_slave_alloc(struct scsi_device *sdev)
+{
+ sdev->tagged_supported = 1;
+
+ scsi_activate_tcq(sdev, sdev->queue_depth);
+
+ return 0;
+}
+
Have you tested this with scsi_mod.use_blk_mq=n?
Trying similar changes in hpsa, we still receive some INQUIRY commands
submitted through queuecommand with tag -1. They are for devices for
which slave_alloc has not yet been run, implying this work needs to
be done even earlier. Maybe the midlayer is missing a slave_alloc
call somewhere?
Hello Rob,
All my tests with use_blk_mq=n were run with a WARN_ON_ONCE(req->tag <
0) statement present in srp_queuecommand(). I haven't seen any kernel
warning being triggered during the tests I ran.
I also had a look at scsi_alloc_sdev() in drivers/scsi/scsi_scan.c. The
number of statements between queue allocation and the slave_alloc() call
is limited. The only scenario I can think of which could cause
queuecommand() to be invoked before slave_alloc() is a LUN scan
initiated from user space via sysfs due to
scsi_sysfs_device_initialize() being invoked before slave_alloc(). Does
that make sense to you ?
Bart.
--
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
