Re: [PATCH 1/2] don't wait on disk to start on resume
On 02/03/2013 02:23 PM, Aaron Lu wrote:
No, the modification is actually for disk.
With v8 of block layer runtime PM, it is no longer the case runtime
suspend is the same as system suspend for hard disk that utilize block
layer runtime PM: we quiesce the device and run its suspend callback for
the device during system suspend but we didn't touch the queue's
rpm_status as we do in runtime_suspend callback. So I did some
modifications to scsi_pm.c to make runtime suspend and system suspend do
exactly the same thing for disk type scsi device, no matter if they are
using block layer runtime PM or not.
Probably I had better post code here, this is a replacement for the
patch 4 of v8 block layer runtime PM patchset(I omit the sd part, since
it is irrevelant), please kindly review, see if you like it :-)
diff --git a/drivers/scsi/scsi_pm.c b/drivers/scsi/scsi_pm.c
index 8f6b12c..d9956b6 100644
--- a/drivers/scsi/scsi_pm.c
+++ b/drivers/scsi/scsi_pm.c
@@ -16,17 +16,44 @@
#include scsi_priv.h
+static int sdev_blk_suspend(struct scsi_device *sdev)
+{
+ int err;
+
+ err = blk_pre_runtime_suspend(sdev-request_queue);
+ if (err)
+ return err;
+ err = pm_generic_runtime_suspend(sdev-sdev_gendev);
+ blk_post_runtime_suspend(sdev-request_queue, err);
+
+ return err;
+}
+
static int scsi_dev_type_suspend(struct device *dev, int (*cb)(struct device
*))
{
+ struct scsi_device *sdev = to_scsi_device(dev);
int err;
- err = scsi_device_quiesce(to_scsi_device(dev));
+ err = scsi_device_quiesce(sdev);
if (err == 0) {
- if (cb) {
+ if (sdev-request_queue-dev)
+ err = sdev_blk_suspend(sdev);
+ else if (cb)
err = cb(dev);
- if (err)
- scsi_device_resume(to_scsi_device(dev));
- }
+
+ if (err)
+ scsi_device_resume(sdev);
}
dev_dbg(dev, scsi suspend: %d\n, err);
return err;
I just realized that this will not work, as we can't request that there
is no request pending in the device's request queue when doing system
suspend. And if we can't request this, we will not be able to runtime
resume the device due to nr_pending may not be zero when new request
comes after system resumed, though this can be solved if we modify the
runtime resume policy.
During my test, I'm not doing any IO when suspend, so the nr_pending is
zero and it worked...
Please ignore my suggestion for this thread, sorry for the noise.
-Aaron
--
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] [SCSI] fnic: Remove unneeded version.h header file include
version.h header file inclusion is not necessary. Signed-off-by: Sachin Kamat sachin.ka...@linaro.org --- drivers/scsi/fnic/fnic_trace.c |1 - 1 files changed, 0 insertions(+), 1 deletions(-) diff --git a/drivers/scsi/fnic/fnic_trace.c b/drivers/scsi/fnic/fnic_trace.c index 0b18bf4..63f6f5b 100644 --- a/drivers/scsi/fnic/fnic_trace.c +++ b/drivers/scsi/fnic/fnic_trace.c @@ -19,7 +19,6 @@ #include linux/mempool.h #include linux/errno.h #include linux/spinlock.h -#include linux/version.h #include linux/kallsyms.h #include fnic_io.h #include fnic.h -- 1.7.4.1 -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] don't wait on disk to start on resume
On 02/04/2013 08:07 AM, dbasehore . wrote: On the topic that we do a fast return for both scsi and ata. Now I don't remember everything about this (and correct me if I'm wrong) since I figured this out a few months ago. There are some dependencies that scsi has on the resume path of ata. I think it's that before we can send the command to spin up the disk, we need to wait for the ata host controller to come up. As Aaron Lu pointed out, it takes seconds for the ata port to resume. On the hand, I just did some more recording, the result is: host controller takes 1ms or less to resume; port reset takes the most time, almost the same as the whole port resume callback, 2-4 seconds; sd resume callback takes 16ms. So for ata disks, the most time consuming part is in port's reset routine, which is executed in port's resume callback. -Aaron -- 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 2/2] Configure reported luns
Thanks for looking Mike.
I agree on changing the GFP_ATOMIC to something less
restrictive but wonder if NOIO is the right choice or not.
See below.
02/01/2013 02:44 AM, Mike Christie wrote:
On 01/28/2013 02:27 PM, Rob Evers wrote:
Change default value of max_report_luns to 16k-1.
Use data returned from max report luns command to configure the number
of logical units present if previous default of 511 isn't enough.
Signed-off-by: Rob Eversrev...@redhat.com
---
drivers/scsi/scsi_scan.c | 79 +++-
1 file changed, 58 insertions(+), 21 deletions(-)
diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index b2abf22..0f7ce45 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -109,12 +109,13 @@ MODULE_PARM_DESC(scan, sync, async or none);
* in practice, the maximum number of LUNs suppored by any device
* is about 16k.
*/
-static unsigned int max_scsi_report_luns = 511;
+static unsigned int max_scsi_report_luns = 16383;
module_param_named(max_report_luns, max_scsi_report_luns, uint,
S_IRUGO|S_IWUSR);
MODULE_PARM_DESC(max_report_luns,
REPORT LUNS maximum number of LUNS received (should be
- between 1 and 16384));
+ between 1 and 16383));
+#define INITIAL_MAX_REPORT_LUNS 511
static unsigned int scsi_inq_timeout = SCSI_TIMEOUT/HZ + 18;
@@ -1366,9 +1367,10 @@ static int scsi_report_lun_scan(struct scsi_target
*starget, int bflags,
char devname[64];
unsigned int length;
unsigned int lun;
- unsigned int num_luns;
+ unsigned int num_luns, num_luns_reported;
int result;
struct scsi_lun *lunp, *lun_data;
+ struct scsi_lun *first_lun_data, *second_lun_data;
u8 *data;
struct scsi_device *sdev;
struct Scsi_Host *shost = dev_to_shost(starget-dev);
@@ -1409,45 +1411,80 @@ static int scsi_report_lun_scan(struct scsi_target
*starget, int bflags,
/*
* Allocate enough to hold the header (the same size as one scsi_lun)
* plus the max number of luns we are requesting.
-*
-* Reallocating and trying again (with the exact amount we need)
-* would be nice, but then we need to somehow limit the size
-* allocated based on the available memory and the limits of
-* kmalloc - we don't want a kmalloc() failure of a huge value to
-* prevent us from finding any LUNs on this target.
*/
- length = (max_scsi_report_luns + 1) * sizeof(struct scsi_lun);
- lun_data = kmalloc(length, GFP_ATOMIC |
- (sdev-host-unchecked_isa_dma ? __GFP_DMA : 0));
- if (!lun_data) {
+ if (max_scsi_report_luns INITIAL_MAX_REPORT_LUNS)
+ length = (INITIAL_MAX_REPORT_LUNS + 1) *
+ sizeof(struct scsi_lun);
+ else
+ length = (max_scsi_report_luns + 1) *
+ sizeof(struct scsi_lun);
+
+ first_lun_data = kmalloc(length, GFP_ATOMIC |
+(sdev-host-unchecked_isa_dma ?
+__GFP_DMA : 0));
+ if (!first_lun_data) {
printk(ALLOC_FAILURE_MSG, __func__);
goto out;
I think it seems like a good idea from the user perspective. They do not
have to worry about setting the modparam. Maybe this is not done already
because of bad targets? I do not know. Maybe someone else does.
One question about the memory allocation here. I know it was already
like this, but why are we using GFP_ATOMIC here and in some of the other
places in this code path? We sleep in some of these paths and some
functions we call use GFP_KERNEL,
As you say...
Followed this call chain and noticed blk_alloc_queue_node(GFP_KERNEL, ...)
scsi_report_lun_scan-
scsi_probe_and_add_lun-scsi_alloc_sdev-
scsi_alloc_queue-
__scsi_alloc_queue-
blk_init_queue-blk_init_queue_node-
blk_alloc_queue_node(GFP_KERNEL...)-
kmem_cache_alloc_node
so it seems like we could use
something that is a little safer from allocation failures like GFP_NOIO?
Seems GFP_NOIO would be better than GFP_ATOMIC for
scsi_report_lun_scan() and scsi_probe_and_add_lun()
The question in my mind is whether it is ok to use GFP_KERNEL
in scsi_report_lun_scan() and scsi_probe_and_add_lun() or
should the block_alloc_queue_node() be changed to use GFP_NOIO?
If we change it, that could be another patch that modifies all the scan
code since you are are just carrying over the existing gfp flag in this
patch and this patch is just doing the one change to the report luns code.
The kmalloc in scsi_complete_async_scans() uses GFP_KERNEL
which looks ok to me. Agree?
--
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] SG_SCSI_RESET ioctl should only perform requested operation
From all the documentation I've found, it is not clear that users of the
SG_SCSI_RESET ioctl may have their requests progress up the hierarchy of reset
operations.
Basically, requests for a SCSI_TRY_RESET_DEVICE may eventually result in a
TARGET, BUS, or HOST reset. The sg_reset utility hints at the error handling,
but actually leads the user to believe that they should reissue the sg_reset
command themselves to enact more aggressive reset functions. It also says:
Note that a host reset and a bus reset may cause collateral damage.
In the interest of error isolation, I have attached a patch which changes the
behavior. The existing behavior is obviously intentional, but I don't believe
its the best choice.
There are other alternatives to this patch. For one, to avoid the possibility of
breaking an existing application, maybe SG_SCSI_RESET needs some new values like
SG_SCSI_RESET_DEVICE_ONLY. While leaving the existing operations alone.
Just in case...
Signed-off-by: Jeremy Linton jlin...@tributary.com
---
diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
index c1b05a8..6f05bc2 100644
--- a/drivers/scsi/scsi_error.c
+++ b/drivers/scsi/scsi_error.c
@@ -1999,19 +1999,13 @@ scsi_reset_provider(struct scsi_device *dev, int flag)
switch (flag) {
case SCSI_TRY_RESET_DEVICE:
rtn = scsi_try_bus_device_reset(scmd);
- if (rtn == SUCCESS)
- break;
- /* FALLTHROUGH */
+ break;
case SCSI_TRY_RESET_TARGET:
rtn = scsi_try_target_reset(scmd);
- if (rtn == SUCCESS)
- break;
- /* FALLTHROUGH */
+ break;
case SCSI_TRY_RESET_BUS:
rtn = scsi_try_bus_reset(scmd);
- if (rtn == SUCCESS)
- break;
- /* FALLTHROUGH */
+ break;
case SCSI_TRY_RESET_HOST:
rtn = scsi_try_host_reset(scmd);
break;
--
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] SG_SCSI_RESET ioctl should only perform requested operation
On 13-02-04 03:17 PM, Jeremy Linton wrote:
From all the documentation I've found, it is not clear that users of the
SG_SCSI_RESET ioctl may have their requests progress up the hierarchy of reset
operations.
Basically, requests for a SCSI_TRY_RESET_DEVICE may eventually result in a
TARGET, BUS, or HOST reset. The sg_reset utility hints at the error handling,
but actually leads the user to believe that they should reissue the sg_reset
command themselves to enact more aggressive reset functions. It also says:
Note that a host reset and a bus reset may cause collateral damage.
In the interest of error isolation, I have attached a patch which changes the
behavior. The existing behavior is obviously intentional, but I don't believe
its the best choice.
There are other alternatives to this patch. For one, to avoid the possibility of
breaking an existing application, maybe SG_SCSI_RESET needs some new values like
SG_SCSI_RESET_DEVICE_ONLY. While leaving the existing operations alone.
Just in case...
Signed-off-by: Jeremy Linton jlin...@tributary.com
---
diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
index c1b05a8..6f05bc2 100644
--- a/drivers/scsi/scsi_error.c
+++ b/drivers/scsi/scsi_error.c
@@ -1999,19 +1999,13 @@ scsi_reset_provider(struct scsi_device *dev, int flag)
switch (flag) {
case SCSI_TRY_RESET_DEVICE:
rtn = scsi_try_bus_device_reset(scmd);
- if (rtn == SUCCESS)
- break;
- /* FALLTHROUGH */
+ break;
case SCSI_TRY_RESET_TARGET:
rtn = scsi_try_target_reset(scmd);
- if (rtn == SUCCESS)
- break;
- /* FALLTHROUGH */
+ break;
case SCSI_TRY_RESET_BUS:
rtn = scsi_try_bus_reset(scmd);
- if (rtn == SUCCESS)
- break;
- /* FALLTHROUGH */
+ break;
case SCSI_TRY_RESET_HOST:
rtn = scsi_try_host_reset(scmd);
break;
--
Jeremy,
That is way too sensible to be approved as it will most likely
break existing code that assumes the escalation property.
Perhaps some new constants could be introduced (e.g.
SCSI_TRY_RESET_TARGET_ONLY) that could be passed to
the SG_SCSI_RESET ioctl. And perhaps that new set of
constants could more accurately reflect how such things
are done in modern SCSI.
Doug Gilbert
--
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: [LSF/MM TOPIC] Thin provisioning SOFT_THRESHOLD error handling
On Tue, Jan 29, 2013 at 12:14 AM, Hannes Reinecke h...@suse.de wrote: I would like to discuss at LSF the possible implementations and handling mechanism for this kind of failure scenarios. I'd be interested in that discussion. With my Pure hat on, our array can generate these thin provisioning threshold unit attentions, and it would be nice if Linux did something intelligent with them. - R. -- 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: [LSF/MM TOPIC] I_T Nexus loss SCSI error handling
On Mon, Jan 21, 2013 at 3:18 AM, Hannes Reinecke h...@suse.de wrote: The current error handler still uses a 'target reset' (or, rather, bus reset) strategy, although the respective TMF has been obsoleted since SAM-3. SAM-5 defines an I_T nexus loss event instead, which so far has only been implemented in libsas. There has been some discussions on the mailing list, but so far there hasn't been any conclusion. So I would like to discuss a possible implementation of a I_T Nexus loss strategy and how to keep compability with SAM-2 targets. Would this handle the case of a SAS fabric with lots of target devices, where we start to get errors from a single device while IO is proceeding fine to everything else on the fabric? It would be really nice if the Linux stack handled this without escalating to a target reset and host reset, since that disrupts all the IO that is proceeding fine (and which won't do much to fix a target drive that might really be dead). - R. -- 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: [LSF/MM TOPIC] Making sure soft SCSI Targets are Valid
On Tue, 2013-01-29 at 11:13 -0800, Lee Duncan wrote: Hi: I'm not sure if there is much interest in this, but I've recently realized that there is no good free software to validate iSCSI targets, not to mention FCOE targets, IB soft targets, etc. There's just no way to know if any change you make is legal short of learning to speak SCSI geek spec (or waiting to see what fails when you make a subtle change). So I have been working with the (user-space) libiscsi creator and maintainer, Ronnie Sahlberg, to enhance his test suite. But this only addresses iSCSI targets. Some of his tests have already shown problems like kernel panics when an incorrect bit is injected, showing the need for such testing. It occurs to me it would be most valuable if we had more generic SCSI tests, not even limited to soft targets, available to developers and manufacturers. How best to support such tests with our SCSI layer, and what tests are needed now and in the future may be a good topic for discussion. +1. Please count me in for this discussion. There are a number of CDB level tests that need to be added to scsi-testsuite.git, and I'd really love to get those interested working on a single codebase for SCSI target verification. --nab -- 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
