Re: [PATCH 2/3] libfc: fix lun reset failure bugs in fc_fcp_resp handling of FCP_RSP_INFO
On 09/24/12 20:52, Robert Love wrote: In LUN RESET testing involving NetApp targets, it is observed that LUN RESET is failing. The fc_fcp_resp() is not completing the completiong It looks like there is a typo in the patch description (in the last word of the second line) ? 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 v7 2/6] scsi: sr: support runtime pm
On Mon, Sep 24, 2012 at 11:40:18PM +0200, Rafael J. Wysocki wrote: On Monday, September 24, 2012, Aaron Lu wrote: On Mon, Sep 24, 2012 at 02:55:31PM +0200, Rafael J. Wysocki wrote: And I'd add a comment about the next poll. This appears somewhat racy, though, because in theory a media may be inserted while we are removing power from the device. Isn't that a problem? Yes, this is a problem. To avoid this race, I think the following needs to be done: - For slot type ODD, make it such that user can't insert a disc; - For tray type ODD, make it such that when user presses the eject button, the tray doesn't open. I'll see if this is possible, thanks for the remind. OK Looks like this is not the right thing to do, if I lock the door user will be confused. The poll runs periodically, until the device is powered off(reflected by the powered_off flag), in which case, the poll will simply return 0 without touching this device. Is it useful to poll the device after it has been suspended, but not powered off? Yes, it is necessary to poll the device when it has been suspended with power left. The reason is, we need to check if there is a media change event happened, and the way to check this is to issue a GET_EVENT_STATUS_NOTIFICATION comand. Please note that when the drive is not powered off, it will not send us a notification when its eject button is pressed. I'm not sure about that, actually. If it doesn't notify us, that whole thing is inherently racy pretty much beyond fixing, because it is always possible that the user will press the eject button right after we've checked the status last time and right before power removal. We're going to lose that event, so the user will have to push the button once again in that case. I just checked the spec again and tested, when the ODD has power, it will also send out notifications on pressing the eject button/inserting a disc. So we should be able to capture such a event. That's correct. AFAIK, user space does not care how often the device is polled, it cares only one thing: when there is a media change event, please let me know(through uevent). So that's why we do the polling, right? Yes. Well, that's difficult. If the remote wakeup is signaled through a GPE, it should be possible to enable it before we actually put the device into D3cold. That may allow us to eliminate the races. Thanks for the suggestion, I'll update the code. I'm thinking of enabling this GPE in sr_suspend once we decided that it is ready to be powered off, so the time frame between sr_suspend and when the power is actually removed in libata should be taken care of by the GPE. If GPE fires, the notification function will request a runtime resume of the device. Does this sound OK? Thanks, 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 v7 0/6] ZPODD patches
On Mon, Sep 24, 2012 at 11:46:03PM +0200, Rafael J. Wysocki wrote:
On Monday, September 24, 2012, Aaron Lu wrote:
On Mon, Sep 24, 2012 at 03:06:11PM +0200, Rafael J. Wysocki wrote:
On Monday, September 24, 2012, Aaron Lu wrote:
need_eject:
First consider how the device will be runtime resumed:
1 Some program opens the block device;
2 Events checking poll when it's not powered off yet;
3 User presses the eject button or inserts a disc into the slot when the
device is in powered off state.
And the need_eject flag is for case 3, when the device is in powered off
state and user presses the eject button, it will be powered on(through
acpi wake notification function) and runtime resumed. In its runtime
resume callback, its tray needs to be ejected since user just presses
the eject button. The whole process of ZPODD is opaque to the user,
he/she doesn't know the ODD lost power so the ODD has to behave exactly
like it doesn't lose power.
Do you think it can be useful for other types of devices, not necessarily
handled through ACPI?
I can only say that it is useful for ZPODD, if ZPODD someday is used on
another platform that does not use ACPI, the need_eject flag should
still be needed.
As for other scsi devices, I'm not sure.
I see. This means we don't really have good arguments for putting that flag
into struct scsi_device ...
OK.
I'm thinking of moving the acpi wake notification code from ata to scsi,
so that the notification function lives in sr module and then I do not
need to set this need_eject flag in scsi_device and scsi_cd structure
needs to host this flag.
A example patch would be something like the following, I didn't seperate
these ACPI calls in sr.c as this is just a concept proof, if this is the
right thing to do, I will separate them into another file sr-acpi.c and
make empty stubs for them in sr.h for systems do not have ACPI configured.
diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
index ef72682..94d17f1 100644
--- a/drivers/scsi/sr.c
+++ b/drivers/scsi/sr.c
@@ -46,6 +46,7 @@
#include linux/mutex.h
#include linux/slab.h
#include linux/pm_runtime.h
+#include linux/acpi.h
#include asm/uaccess.h
#include scsi/scsi.h
@@ -57,6 +58,8 @@
#include scsi/scsi_host.h
#include scsi/scsi_ioctl.h /* For the door lock/unlock commands */
+#include acpi/acpi_bus.h
+
#include scsi_logging.h
#include sr.h
@@ -212,8 +220,8 @@ static int sr_resume(struct device *dev)
scsi_test_unit_ready(cd-device, SR_TIMEOUT, MAX_RETRIES, sshdr);
/* If user wakes up the ODD, eject the tray */
- if (cd-device-need_eject) {
- cd-device-need_eject = 0;
+ if (cd-need_eject) {
+ cd-need_eject = false;
/* But only for tray type ODD when door is not locked */
if (!(cd-cdi.mask CDC_CLOSE_TRAY) !cd-door_locked)
sr_tray_move(cd-cdi, 1);
@@ -704,6 +711,58 @@ static void sr_release(struct cdrom_device_info *cdi)
}
+static void sr_acpi_wake_dev(acpi_handle handle, u32 event, void *context)
+{
+ struct device *dev = context;
+ struct scsi_cd *cd = dev_get_drvdata(dev);
+
+ if (event == ACPI_NOTIFY_DEVICE_WAKE pm_runtime_suspended(dev)) {
+ cd-need_eject = true;
+ pm_runtime_resume(dev);
+ }
+}
+
+static void sr_acpi_add_pm_notifier(struct device *dev)
+{
+ struct acpi_device *acpi_dev;
+ acpi_handle handle;
+ acpi_status status;
+
+ handle = dev-archdata.acpi_handle;
+ if (!handle)
+ return;
+
+ status = acpi_bus_get_device(handle, acpi_dev);
+ if (ACPI_FAILURE(status))
+ return;
+
+ acpi_power_resource_register_device(dev, handle);
+ acpi_install_notify_handler(handle, ACPI_SYSTEM_NOTIFY,
+ sr_acpi_wake_dev, dev);
+ device_set_run_wake(dev, true);
+}
+
+static void sr_acpi_remove_pm_notifier(struct device *dev)
+{
+ struct acpi_device *acpi_dev;
+ acpi_handle handle;
+ acpi_status status;
+
+ handle = dev-archdata.acpi_handle;
+ if (!handle)
+ return;
+
+ status = acpi_bus_get_device(handle, acpi_dev);
+ if (ACPI_FAILURE(status))
+ return;
+
+ acpi_power_resource_unregister_device(dev, handle);
+ device_set_run_wake(dev, false);
+ acpi_remove_notify_handler(handle, ACPI_SYSTEM_NOTIFY,
sr_acpi_wake_dev);
+}
+
static int sr_probe(struct device *dev)
{
struct scsi_device *sdev = to_scsi_device(dev);
@@ -786,7 +845,9 @@ static int sr_probe(struct device *dev)
sdev_printk(KERN_DEBUG, sdev,
Attached scsi CD-ROM %s\n, cd-cdi.name);
- /* enable runtime pm */
+ if (sdev-can_power_off)
+ sr_acpi_add_pm_notifier(dev);
+
scsi_autopm_put_device(cd-device);
return 0;
@@ -1036,8
Re: [SCSI PATCH] sd: max-retries becomes configurable
On Tue, 2012-09-25 at 01:21 -0400, Jeff Garzik wrote: On 09/25/2012 12:06 AM, James Bottomley wrote: On Mon, 2012-09-24 at 17:00 -0400, Jeff Garzik wrote: drivers/scsi/sd.c |4 drivers/scsi/sd.h |2 +- 2 files changed, 5 insertions(+), 1 deletion(-) I'm not opposed in principle to doing this (except that it should be a sysfs parameter like all our other controls), but what's the reasoning behind needing it changed? vendor hat on Periodically turns up as a useful field sledgehammer for solving problems, until the real problem is found and fixed. Got tired of a very similar patch manually bouncing around the hey, pssst, this worked for me backchannel IT network. /red hat I'm asking because the general consensus from the device guys is that we should never retry unless the device or the transport tells us to (and then we shouldn't count the retries). A long time ago we used to get spurious command failures from retry exhaustion on QUEUE_FULL or BUSY, but since we switched those to being purely timeout based, I thought the problem had gone away and I'm curious to know what guise it resurfaced in. Can you be more specific about sysfs location? A runtime-writable (via sysfs!) module parameter for a module-wide default seemed appropriate. Well, if it's really important, the same thing should happen with retries as happened with timeout (it became a request_queue property), but it could be hacked as a struct scsi_disk one with a corresponding entry in sd_dis_attrs. James -- 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 v7 0/6] ZPODD patches
On Tue, 2012-09-25 at 16:18 +0800, Aaron Lu wrote:
A example patch would be something like the following, I didn't seperate
these ACPI calls in sr.c as this is just a concept proof, if this is the
right thing to do, I will separate them into another file sr-acpi.c and
make empty stubs for them in sr.h for systems do not have ACPI configured.
Apart from the needed separation to compile in the !ACPI case
diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
index ef72682..94d17f1 100644
--- a/drivers/scsi/sr.c
+++ b/drivers/scsi/sr.c
@@ -46,6 +46,7 @@
#include linux/mutex.h
#include linux/slab.h
#include linux/pm_runtime.h
+#include linux/acpi.h
#include asm/uaccess.h
#include scsi/scsi.h
@@ -57,6 +58,8 @@
#include scsi/scsi_host.h
#include scsi/scsi_ioctl.h /* For the door lock/unlock commands */
+#include acpi/acpi_bus.h
+
#include scsi_logging.h
#include sr.h
@@ -212,8 +220,8 @@ static int sr_resume(struct device *dev)
scsi_test_unit_ready(cd-device, SR_TIMEOUT, MAX_RETRIES, sshdr);
/* If user wakes up the ODD, eject the tray */
- if (cd-device-need_eject) {
- cd-device-need_eject = 0;
+ if (cd-need_eject) {
+ cd-need_eject = false;
/* But only for tray type ODD when door is not locked */
if (!(cd-cdi.mask CDC_CLOSE_TRAY) !cd-door_locked)
sr_tray_move(cd-cdi, 1);
@@ -704,6 +711,58 @@ static void sr_release(struct cdrom_device_info *cdi)
}
+static void sr_acpi_wake_dev(acpi_handle handle, u32 event, void *context)
+{
+ struct device *dev = context;
+ struct scsi_cd *cd = dev_get_drvdata(dev);
+
+ if (event == ACPI_NOTIFY_DEVICE_WAKE pm_runtime_suspended(dev)) {
+ cd-need_eject = true;
+ pm_runtime_resume(dev);
+ }
+}
+
+static void sr_acpi_add_pm_notifier(struct device *dev)
+{
+ struct acpi_device *acpi_dev;
+ acpi_handle handle;
+ acpi_status status;
+
+ handle = dev-archdata.acpi_handle;
This is a complete no-no. archdata is defined to be specific to the
architecture it's supposed to be opaque to non-arch code. You'll find
that only x86 and ia64 defines an acpi_handle there. This will
instantly fail to compile on non intel. If you need the handle, it
should be obtained via some accessor like dev_to_acpi_handle() which
will allow this to continue to function when, say, arm acquires ACPI.
I've got to say, this looks like a fault in ACPI itself. If the handles
are 1:1 with struct device, then why not have all the functions taking
handles take the device instead and leave the embedded handle safely in
the architecture specific code?
James
--
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 v7 2/6] scsi: sr: support runtime pm
On Tuesday, September 25, 2012, Aaron Lu wrote: On Mon, Sep 24, 2012 at 11:40:18PM +0200, Rafael J. Wysocki wrote: On Monday, September 24, 2012, Aaron Lu wrote: On Mon, Sep 24, 2012 at 02:55:31PM +0200, Rafael J. Wysocki wrote: And I'd add a comment about the next poll. This appears somewhat racy, though, because in theory a media may be inserted while we are removing power from the device. Isn't that a problem? Yes, this is a problem. To avoid this race, I think the following needs to be done: - For slot type ODD, make it such that user can't insert a disc; - For tray type ODD, make it such that when user presses the eject button, the tray doesn't open. I'll see if this is possible, thanks for the remind. OK Looks like this is not the right thing to do, if I lock the door user will be confused. The poll runs periodically, until the device is powered off(reflected by the powered_off flag), in which case, the poll will simply return 0 without touching this device. Is it useful to poll the device after it has been suspended, but not powered off? Yes, it is necessary to poll the device when it has been suspended with power left. The reason is, we need to check if there is a media change event happened, and the way to check this is to issue a GET_EVENT_STATUS_NOTIFICATION comand. Please note that when the drive is not powered off, it will not send us a notification when its eject button is pressed. I'm not sure about that, actually. If it doesn't notify us, that whole thing is inherently racy pretty much beyond fixing, because it is always possible that the user will press the eject button right after we've checked the status last time and right before power removal. We're going to lose that event, so the user will have to push the button once again in that case. I just checked the spec again and tested, when the ODD has power, it will also send out notifications on pressing the eject button/inserting a disc. So we should be able to capture such a event. Good! That's correct. AFAIK, user space does not care how often the device is polled, it cares only one thing: when there is a media change event, please let me know(through uevent). So that's why we do the polling, right? Yes. Well, that's difficult. If the remote wakeup is signaled through a GPE, it should be possible to enable it before we actually put the device into D3cold. That may allow us to eliminate the races. Thanks for the suggestion, I'll update the code. I'm thinking of enabling this GPE in sr_suspend once we decided that it is ready to be powered off, so the time frame between sr_suspend and when the power is actually removed in libata should be taken care of by the GPE. If GPE fires, the notification function will request a runtime resume of the device. Does this sound OK? Well, depending on the implementation. sr_suspend() should be rather generic, but the ACPI association (including the GPE thing) is specific to ATA. Thanks, Rafael -- 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] aic94xx: fix handling of default CTRL-A settings
Compiling aic94xx_sds.o (part of the aic94xx driver) triggers this GCC
warning:
drivers/scsi/aic94xx/aic94xx_sds.c: In function 'asd_read_flash':
drivers/scsi/aic94xx/aic94xx_sds.c:597:21: warning: 'offs' may be used
uninitialized in this function [-Wmaybe-uninitialized]
drivers/scsi/aic94xx/aic94xx_sds.c:985:6: note: 'offs' was declared here
This warning can be traced back to asd_find_flash_de(). If it fails to
find a FLASH_DE_CTRL_A_USER entry it will return without setting 'offs'.
And that will leave 'offs' uninitialized when asd_read_flash_seg() is
called a little later.
But that call of asd_read_flash_seg() isn't needed in that case.
Everything this code cares about can already be found in the default
CTRL-A user settings section that was created in the error path. So
let's just jump over that call (and all other unneeded code) after
creating that section.
Signed-off-by: Paul Bolle pebo...@tiscali.nl
---
0) I noticed this warning while building v3.6-rc7 on current Fedora
17, using Fedora's default config.
1) Compile tested only. It might be best to run test this too, if only
to test whether the non-error path is unaffected.
2) This piece of code has not been touched ever since it was added in
v2.6.19, with commit 2908d778ab3e244900c310974e1fc1c69066e450 ([SCSI]
aic94xx: new driver). So, given the current code, chances are that a
CTRL-A user settings section is always present and the code to create
a default section might as well be dropped. So perhaps a more drastic
patch is preferable.
drivers/scsi/aic94xx/aic94xx_sds.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/aic94xx/aic94xx_sds.c
b/drivers/scsi/aic94xx/aic94xx_sds.c
index edb43fd..ecb4a1c 100644
--- a/drivers/scsi/aic94xx/aic94xx_sds.c
+++ b/drivers/scsi/aic94xx/aic94xx_sds.c
@@ -983,7 +983,7 @@ static int asd_process_ctrl_a_user(struct asd_ha_struct
*asd_ha,
{
int err, i;
u32 offs, size;
- struct asd_ll_el *el;
+ struct asd_ll_el *el = NULL;
struct asd_ctrla_phy_settings *ps;
struct asd_ctrla_phy_settings dflt_ps;
@@ -1004,6 +1004,7 @@ static int asd_process_ctrl_a_user(struct asd_ha_struct
*asd_ha,
size = sizeof(struct asd_ctrla_phy_settings);
ps = dflt_ps;
+ goto ctrla_phy_settings;
}
if (size == 0)
@@ -1029,6 +1030,7 @@ static int asd_process_ctrl_a_user(struct asd_ha_struct
*asd_ha,
goto out2;
}
+ctrla_phy_settings:
err = asd_process_ctrla_phy_settings(asd_ha, ps);
if (err) {
ASD_DPRINTK(couldn't process ctrla phy settings\n);
--
1.7.11.4
--
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 v7 0/6] ZPODD patches
On Tue, Sep 25, 2012 at 03:02:29PM +0400, James Bottomley wrote:
On Tue, 2012-09-25 at 16:18 +0800, Aaron Lu wrote:
A example patch would be something like the following, I didn't seperate
these ACPI calls in sr.c as this is just a concept proof, if this is the
right thing to do, I will separate them into another file sr-acpi.c and
make empty stubs for them in sr.h for systems do not have ACPI configured.
Apart from the needed separation to compile in the !ACPI case
diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
index ef72682..94d17f1 100644
--- a/drivers/scsi/sr.c
+++ b/drivers/scsi/sr.c
@@ -46,6 +46,7 @@
#include linux/mutex.h
#include linux/slab.h
#include linux/pm_runtime.h
+#include linux/acpi.h
#include asm/uaccess.h
#include scsi/scsi.h
@@ -57,6 +58,8 @@
#include scsi/scsi_host.h
#include scsi/scsi_ioctl.h /* For the door lock/unlock commands */
+#include acpi/acpi_bus.h
+
#include scsi_logging.h
#include sr.h
@@ -212,8 +220,8 @@ static int sr_resume(struct device *dev)
scsi_test_unit_ready(cd-device, SR_TIMEOUT, MAX_RETRIES, sshdr);
/* If user wakes up the ODD, eject the tray */
- if (cd-device-need_eject) {
- cd-device-need_eject = 0;
+ if (cd-need_eject) {
+ cd-need_eject = false;
/* But only for tray type ODD when door is not locked */
if (!(cd-cdi.mask CDC_CLOSE_TRAY) !cd-door_locked)
sr_tray_move(cd-cdi, 1);
@@ -704,6 +711,58 @@ static void sr_release(struct cdrom_device_info *cdi)
}
+static void sr_acpi_wake_dev(acpi_handle handle, u32 event, void *context)
+{
+ struct device *dev = context;
+ struct scsi_cd *cd = dev_get_drvdata(dev);
+
+ if (event == ACPI_NOTIFY_DEVICE_WAKE pm_runtime_suspended(dev)) {
+ cd-need_eject = true;
+ pm_runtime_resume(dev);
+ }
+}
+
+static void sr_acpi_add_pm_notifier(struct device *dev)
+{
+ struct acpi_device *acpi_dev;
+ acpi_handle handle;
+ acpi_status status;
+
+ handle = dev-archdata.acpi_handle;
This is a complete no-no. archdata is defined to be specific to the
architecture it's supposed to be opaque to non-arch code. You'll find
that only x86 and ia64 defines an acpi_handle there. This will
instantly fail to compile on non intel. If you need the handle, it
If you are OK with this change to solve the need_eject flag, I'll prepare
a formal patch, in which, all of the newly added function will be within
the range of
#ifdef CONFIG_ACPI
... ...
#endif
And for the CONFIG_ACPI not defined case, they will be static inline
empty functions. Then there should be no compile errors.
should be obtained via some accessor like dev_to_acpi_handle() which
will allow this to continue to function when, say, arm acquires ACPI.
There is a DEVICE_ACPI_HANDLE macro that I'll use when preparing the
formal patch. I'm rushing out these code to show the idea.
Sorry for not considering these things.
Thanks,
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 v7 2/6] scsi: sr: support runtime pm
On Tue, Sep 25, 2012 at 01:47:52PM +0200, Rafael J. Wysocki wrote: On Tuesday, September 25, 2012, Aaron Lu wrote: I'm thinking of enabling this GPE in sr_suspend once we decided that it is ready to be powered off, so the time frame between sr_suspend and when the power is actually removed in libata should be taken care of by the GPE. If GPE fires, the notification function will request a runtime resume of the device. Does this sound OK? Well, depending on the implementation. sr_suspend() should be rather generic, but the ACPI association (including the GPE thing) is specific to ATA. Sorry, but don't quite understand this. We have ACPI bindings for scsi devices, isn't that for us to use ACPI when needed in scsi? BTW, if sr_suspend should be generic, that would suggest I shouldn't write any ZPODD related code there, right? Any suggestion where these code should go then? Thanks, 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 v7 2/6] scsi: sr: support runtime pm
On Tuesday 25 September 2012 22:20:21 Aaron Lu wrote: On Tue, Sep 25, 2012 at 01:47:52PM +0200, Rafael J. Wysocki wrote: On Tuesday, September 25, 2012, Aaron Lu wrote: I'm thinking of enabling this GPE in sr_suspend once we decided that it is ready to be powered off, so the time frame between sr_suspend and when the power is actually removed in libata should be taken care of by the GPE. If GPE fires, the notification function will request a runtime resume of the device. Does this sound OK? Well, depending on the implementation. sr_suspend() should be rather generic, but the ACPI association (including the GPE thing) is specific to ATA. Sorry, but don't quite understand this. We have ACPI bindings for scsi devices, isn't that for us to use ACPI when needed in scsi? We don't have ACPI bindings for generic SCSI devices. We have such bindings for SATA drives. You can put such things only in sr if it applies to all (maybe most) types of drives. BTW, if sr_suspend should be generic, that would suggest I shouldn't write any ZPODD related code there, right? Any suggestion where these code should go then? libata. Maybe some generic hooks can be called in sr_suspend(). Regards Oliver PS: Are you sure sr_suspend() handles DVD-RAMs correctly? -- 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 v7 2/6] scsi: sr: support runtime pm
On 09/25/2012 10:23 PM, Oliver Neukum wrote: On Tuesday 25 September 2012 22:20:21 Aaron Lu wrote: On Tue, Sep 25, 2012 at 01:47:52PM +0200, Rafael J. Wysocki wrote: On Tuesday, September 25, 2012, Aaron Lu wrote: I'm thinking of enabling this GPE in sr_suspend once we decided that it is ready to be powered off, so the time frame between sr_suspend and when the power is actually removed in libata should be taken care of by the GPE. If GPE fires, the notification function will request a runtime resume of the device. Does this sound OK? Well, depending on the implementation. sr_suspend() should be rather generic, but the ACPI association (including the GPE thing) is specific to ATA. Sorry, but don't quite understand this. We have ACPI bindings for scsi devices, isn't that for us to use ACPI when needed in scsi? We don't have ACPI bindings for generic SCSI devices. We have such bindings for SATA drives. You can put such things only in sr if it applies to all (maybe most) types of drives. OK. Then these scsi bindings for sata drives will be pretty much of no use I think. BTW, if sr_suspend should be generic, that would suggest I shouldn't write any ZPODD related code there, right? Any suggestion where these code should go then? libata. Maybe some generic hooks can be called in sr_suspend(). Thanks for the suggestion. The problem is, I need to know whether the door is closed and if there is a medium inside. I've no way of getting such information in libata. PS: Are you sure sr_suspend() handles DVD-RAMs correctly? No. Is there a spec for it? Considering there are many different drives sr handle, is it possible to write a generic sr_suspend? Maybe your suggestion of callback is the way to go. What about this idea, if we find this is a ZPODD capable drive, we enable runtime suspend for it and write a suspend callback according to ZPODD spec. For other drives that does not have a suspend callback, we do not enable runtime suspend. Does this sound reasonable? Thanks, 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 00/20, v4] Make ib_srp better suited for H.A. purposes
On 08/09/12 17:41, Bart Van Assche wrote: [ ... ] Hello Dave, More than six weeks have elapsed since I posted version four of this patch series. It would be appreciated if you could tell me when review comments for this patch series will be posted. I'd also like to remind you that some time ago you asked other people to wait with posting more ib_srp patches until this patch series is upstream [1, 2]. Thanks, Bart. [1] David Dillow, Re: [PATCH 1/1] ib_srp: Infiniband srp fast failover patch, May 29, 2012 (http://www.mail-archive.com/linux-rdma@vger.kernel.org/msg11801.html). [2] David Dillow, Re: [PATCH] srp: convert SRP_RQ_SHIFT into a module parameter, May 29, 2012 (http://www.mail-archive.com/linux-rdma@vger.kernel.org/msg11802.html). -- 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 v2 2/3] scsi: create an all-zero filter for scanners
Using /dev/sg for scanners is blocked from unprivileged users. Reimplement
this using customizable command filters, so that the sysfs knobs will work
in this case too.
Cc: linux-scsi@vger.kernel.org
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
v1-v2: OOM check [Alan Cox]
use GFP_ATOMIC, not GFP_KERNEL
drivers/scsi/scsi_scan.c |8 +++-
drivers/scsi/sg.c|3 ---
2 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index 56a9379..81b1579 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -773,13 +773,19 @@ static int scsi_add_lun(struct scsi_device *sdev,
unsigned char *inq_result,
}
switch (sdev-type) {
+ case TYPE_SCANNER:
+ sdev-request_queue-cmd_filter =
+ kzalloc(sizeof(struct blk_cmd_filter), GFP_ATOMIC);
+ if (sdev-request_queue-cmd_filter == NULL)
+ return SCSI_SCAN_NO_RESPONSE;
+ /* fallthrough */
+
case TYPE_RBC:
case TYPE_TAPE:
case TYPE_DISK:
case TYPE_PRINTER:
case TYPE_MOD:
case TYPE_PROCESSOR:
- case TYPE_SCANNER:
case TYPE_MEDIUM_CHANGER:
case TYPE_ENCLOSURE:
case TYPE_COMM:
diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index 2ba7c82..c7474f5 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -219,9 +219,6 @@ static int sg_allow_access(struct file *filp, unsigned char
*cmd)
struct sg_fd *sfp = filp-private_data;
struct request_queue *q = sfp-parentdp-device-request_queue;
- if (sfp-parentdp-device-type == TYPE_SCANNER)
- return 0;
-
return blk_verify_command(q-cmd_filter,
cmd, filp-f_mode FMODE_WRITE);
}
--
1.7.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: usb3 fails to write when using usb3 hub in usb3 port
On Tue, Sep 25, 2012 at 09:26:00AM +0300, Adrian Sandu wrote:
On Tue, Sep 25, 2012 at 12:38 AM, Sarah Sharp
sarah.a.sh...@linux.intel.com wrote:
Ok, so 3.4.11 doesn't work, and the log file was from 3.5.
If you want I can provide a 3.4 log...
Hmm, does a 3.3 stable kernel work for you? I have a hypothesis.
Alan, I'm wondering if the xHCI ring expansion is causing issues with
USB hard drives under xHCI. Testing with a Buffalo USB 3.0 hard drive
with an NEC uPD720200 xHCI host, I see that the usb-storage and SCSI
initialization produces I/O errors on random sectors in 3.4.0, 3.4.6,
and 3.5.0. I can't get those errors to be reproduced in 3.3.1.
The xHCI ring expansion was added in 3.4, and we changed the xHCI's
sg_tablesize:
int xhci_gen_setup(struct usb_hcd *hcd, xhci_get_quirks_t get_quirks)
{
...
/* Accept arbitrarily long scatter-gather lists */
hcd-self.sg_tablesize = ~0;
The usb-storage driver sets the tablesize thus:
static unsigned int usb_stor_sg_tablesize(struct usb_interface *intf)
{
struct usb_device *usb_dev = interface_to_usbdev(intf);
if (usb_dev-bus-sg_tablesize) {
return usb_dev-bus-sg_tablesize;
}
return SG_ALL;
}
I notice that SG_ALL is set to SCSI_MAX_SG_SEGMENTS, which is only 128.
Should we be passing an arbitrarily large number to the SCSI core?
There's some wording in include/scsi/scsi.h about also limiting the
number of chained sgs to 2048. I'm wondering if we're hitting some bugs
in the SCSI layer because we're setting the sg_tablesize so high.
Alternately, we could be hitting bugs in the USB 3.0 firmware when we
attempt to issue a read or write that's too big. The read on Adrian's
hard drive failed on a bigger read request (122880 bytes). It would be
interesting to see if it works fine if the xHCI sg_tablesize is limited.
I'm going to try that with my own drive on 3.5.4 and see if it helps.
Sarah Sharp
--
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 v7 2/6] scsi: sr: support runtime pm
On Tuesday, September 25, 2012, Aaron Lu wrote: On 09/25/2012 10:23 PM, Oliver Neukum wrote: On Tuesday 25 September 2012 22:20:21 Aaron Lu wrote: On Tue, Sep 25, 2012 at 01:47:52PM +0200, Rafael J. Wysocki wrote: On Tuesday, September 25, 2012, Aaron Lu wrote: I'm thinking of enabling this GPE in sr_suspend once we decided that it is ready to be powered off, so the time frame between sr_suspend and when the power is actually removed in libata should be taken care of by the GPE. If GPE fires, the notification function will request a runtime resume of the device. Does this sound OK? Well, depending on the implementation. sr_suspend() should be rather generic, but the ACPI association (including the GPE thing) is specific to ATA. Sorry, but don't quite understand this. We have ACPI bindings for scsi devices, isn't that for us to use ACPI when needed in scsi? We don't have ACPI bindings for generic SCSI devices. We have such bindings for SATA drives. You can put such things only in sr if it applies to all (maybe most) types of drives. OK. Then these scsi bindings for sata drives will be pretty much of no use I think. BTW, if sr_suspend should be generic, that would suggest I shouldn't write any ZPODD related code there, right? Any suggestion where these code should go then? libata. Maybe some generic hooks can be called in sr_suspend(). Thanks for the suggestion. The problem is, I need to know whether the door is closed and if there is a medium inside. I've no way of getting such information in libata. How does sr get to know it in the libata case? PS: Are you sure sr_suspend() handles DVD-RAMs correctly? No. Is there a spec for it? Considering there are many different drives sr handle, is it possible to write a generic sr_suspend? Maybe your suggestion of callback is the way to go. What about this idea, if we find this is a ZPODD capable drive, we enable runtime suspend for it and write a suspend callback according to ZPODD spec. For other drives that does not have a suspend callback, we do not enable runtime suspend. You can enable runtime PM for all kinds of drives, but make the suspend and resume callbacks only do something for ZPODD ones. This may allow their parents to use runtime PM (as Alan said earlier in this thread), even if the drives themseleves are not really physically suspended. Does this sound reasonable? First, we need to know when the drive is not in use. That information we can get from the sr's runtime PM and it looks like we need to notify libata about that somehow. I'm not sure what mechanism is the best for that at the moment. Second, when the device is resumed by remote wakeup, we need to notify sr about that. A resume alone is not sufficient, though, because it may be necessary to open the tray. Perhaps in that case we can use the same mechanism by which user events are processed by libata and delivered to sr? Rafael -- 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] MAINTAINERS: update Intel C600 SAS driver maintainers
Cc: Lukasz Dorau lukasz.do...@intel.com Cc: Maciej Patelczyk maciej.patelc...@intel.com Signed-off-by: Dave Jiang dave.ji...@intel.com --- MAINTAINERS |7 --- 1 files changed, 4 insertions(+), 3 deletions(-) diff --git a/MAINTAINERS b/MAINTAINERS index b17587d..162f602 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -3552,11 +3552,12 @@ K: \b(ABS|SYN)_MT_ INTEL C600 SERIES SAS CONTROLLER DRIVER M: Intel SCU Linux support intel-linux-...@intel.com +M: Lukasz Dorau lukasz.do...@intel.com +M: Maciej Patelczyk maciej.patelc...@intel.com M: Dave Jiang dave.ji...@intel.com -M: Ed Nadolski edmund.nadol...@intel.com L: linux-scsi@vger.kernel.org -T: git git://git.kernel.org/pub/scm/linux/kernel/git/djbw/isci.git -S: Maintained +T: git git://git.code.sf.net/p/intel-sas/isci +S: Supported F: drivers/scsi/isci/ F: firmware/isci/ -- 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: next-20120925: BUG at drivers/scsi/scsi_lib.c:640!
(cc's added)
On Tue, 25 Sep 2012 22:06:37 +0400
Dmitry Monakhov dmonak...@openvz.org wrote:
Seems like barriers are broken again
kernel BUG at drivers/scsi/scsi_lib.c:1180!
invalid opcode: [#1] SMP
Modules linked in: coretemp kvm_intel kvm crc32c_intel ghash_clmulni_intel
microcode sg xhci_hcd button ext3 jbd mbcache sd_mod crc_t10dif\
elper cryptd lrw aes_x86_64 xts gf128mul ahci libahci pata_acpi ata_generic
dm_mirror dm_region_hash dm_log dm_mod
CPU 0
Pid: 753, comm: fsck.ext3 Not tainted 3.6.0-rc7-next-20120925+ #4
/DQ67SW
RIP: 0010:[81470dbc] [81470dbc]
scsi_setup_fs_cmnd+0xec/0x180
RSP: 0018:880233aff9f8 EFLAGS: 00010002
RAX: 0003 RBX: 88022a741000 RCX: 0002
RDX: RSI: 0001 RDI: 81f32b48
RBP: 880233affa18 R08: 0001 R09:
R10: 88022a26c800 R11: R12: 880229369968
R13: 0001 R14: 88022a741000 R15:
FS: 7f1348632760() GS:88023e20() knlGS:
CS: 0010 DS: ES: CR0: 80050033
CR2: 003a3dc0e550 CR3: 0002338cf000 CR4: 000407f0
DR0: DR1: DR2:
DR3: DR6: 0ff0 DR7: 0400
Process fsck.ext3 (pid: 753, threadinfo 880233afe000, task
880233f48240)
Stack:
880233affa48 880229369968 0001 880229bdb550
880233affaa8 a00a8860 880233affab8 0082
8107d696 8802 817410d8
Call Trace:
[a00a8860] sd_prep_fn+0x140/0xfe0 [sd_mod]
[8107d696] ? lock_timer_base+0x76/0xf0
[817410d8] ? _raw_spin_unlock_irq+0x48/0x80
[8130023c] blk_peek_request+0x23c/0x450
[8146fad0] scsi_request_fn+0x70/0x820
[812f54e5] __blk_run_queue+0x55/0x70
[8132a065] cfq_rq_enqueued+0x155/0x1c0
[8132a386] cfq_insert_request+0x2b6/0x2f0
[8132a11d] ? cfq_insert_request+0x4d/0x2f0
[812f002f] ? md5_final+0x9f/0x130
[810e5463] ? __lock_release+0xc3/0xe0
[812fe074] ? drive_stat_acct+0x334/0x3b0
[812f4be6] __elv_add_request+0x2a6/0x350
[813010fb] blk_queue_bio+0x52b/0x570
[812fd8f5] generic_make_request+0x125/0x1c0
[812fdb68] submit_bio+0x1d8/0x240
[81250c63] ? bio_alloc_bioset+0x103/0x1e0
[813039e7] blkdev_issue_flush+0x177/0x200
[81253afa] blkdev_fsync+0x4a/0x70
[81245af6] vfs_fsync_range+0x36/0x60
[81245b3c] vfs_fsync+0x1c/0x20
[81245ea8] do_fsync+0x58/0x90
[81246100] sys_fsync+0x10/0x20
[8174e539] system_call_fastpath+0x16/0x1b
Code: 00 48 c7 c7 48 2b f3 81 41 0f 94 c5 31 d2 44 89 ee e8 d9 e4 cd ff 49
63 c5 48 83 c0 02 48 83 04 c5 b0 a5 13 82 01 45 85 ed 74 04 0f\
48 89 df 31 db e8 a3 f6 ff ff 48 85 c0 48
RIP [81470dbc] scsi_setup_fs_cmnd+0xec/0x180
RSP 880233aff9f8
[ cut here ]
kernel BUG at drivers/scsi/scsi_lib.c:640!
invalid opcode: [#1] SMP
Modules linked in: coretemp kvm_intel kvm crc32c_intel ghash_clmulni_intel
microcode sg xhci_hcd button ext3 jbd mbcache sd_mod crc_t10dif\
elper cryptd lrw aes_x86_64 xts gf128mul ahci libahci pata_acpi ata_generic
dm_mirror dm_region_hash dm_log dm_mod
CPU 0
Pid: 727, comm: fsck.ext3 Not tainted 3.6.0-rc7-next-20120925+ #5
/DQ67SW
RIP: 0010:[81470585] [81470585]
scsi_alloc_sgtable+0x55/0xe0
RSP: 0018:880228215aa8 EFLAGS: 00010002
RAX: 0003 RBX: 880228111a18 RCX: 0001
RDX: RSI: 0001 RDI: 81f32a08
RBP: 880228215ac8 R08: 0001 R09:
R10: 0002 R11: R12:
R13: 0020 R14: 0001 R15:
FS: 7fb605f35760() GS:88023e20() knlGS:
CS: 0010 DS: ES: CR0: 80050033
CR2: 003a3dc0e550 CR3: 000233e83000 CR4: 000407f0
DR0: DR1: DR2:
DR3: DR6: 0ff0 DR7: 0400
Process fsck.ext3 (pid: 727, threadinfo 880228214000, task
880233af8c80)
Stack:
880228111a18 88022a0a0638 88022a679000
880228215b08 81470641 8802281119c0 88022a679000
880228215b28 8802281119c0 88022a0a0638 0020
Call Trace:
[81470641] scsi_init_sgtable+0x31/0xe0
[81470a2d] scsi_init_io+0x3d/0x2e0
[81470e23] scsi_setup_fs_cmnd+0x153/0x180
[a00a8860] sd_prep_fn+0x140/0xfe0 [sd_mod]
[8135afec
Re: [PATCH v7 2/6] scsi: sr: support runtime pm
On Tue, Sep 25, 2012 at 11:45:34PM +0200, Rafael J. Wysocki wrote: On Tuesday, September 25, 2012, Aaron Lu wrote: On 09/25/2012 10:23 PM, Oliver Neukum wrote: On Tuesday 25 September 2012 22:20:21 Aaron Lu wrote: On Tue, Sep 25, 2012 at 01:47:52PM +0200, Rafael J. Wysocki wrote: On Tuesday, September 25, 2012, Aaron Lu wrote: I'm thinking of enabling this GPE in sr_suspend once we decided that it is ready to be powered off, so the time frame between sr_suspend and when the power is actually removed in libata should be taken care of by the GPE. If GPE fires, the notification function will request a runtime resume of the device. Does this sound OK? Well, depending on the implementation. sr_suspend() should be rather generic, but the ACPI association (including the GPE thing) is specific to ATA. Sorry, but don't quite understand this. We have ACPI bindings for scsi devices, isn't that for us to use ACPI when needed in scsi? We don't have ACPI bindings for generic SCSI devices. We have such bindings for SATA drives. You can put such things only in sr if it applies to all (maybe most) types of drives. OK. Then these scsi bindings for sata drives will be pretty much of no use I think. BTW, if sr_suspend should be generic, that would suggest I shouldn't write any ZPODD related code there, right? Any suggestion where these code should go then? libata. Maybe some generic hooks can be called in sr_suspend(). Thanks for the suggestion. The problem is, I need to know whether the door is closed and if there is a medium inside. I've no way of getting such information in libata. How does sr get to know it in the libata case? By executing a test_unit_ready command. Libata does/should not have any routine to do this, it is one of the transport of SCSI devices and it relies on SCSI driver to manage the device(both disk and ODD). PS: Are you sure sr_suspend() handles DVD-RAMs correctly? No. Is there a spec for it? Considering there are many different drives sr handle, is it possible to write a generic sr_suspend? Maybe your suggestion of callback is the way to go. What about this idea, if we find this is a ZPODD capable drive, we enable runtime suspend for it and write a suspend callback according to ZPODD spec. For other drives that does not have a suspend callback, we do not enable runtime suspend. You can enable runtime PM for all kinds of drives, but make the suspend and resume callbacks only do something for ZPODD ones. This may allow their parents to use runtime PM (as Alan said earlier in this thread), even if the drives themseleves are not really physically suspended. Sounds good. Does this sound reasonable? First, we need to know when the drive is not in use. That information we can get from the sr's runtime PM and it looks like we need to notify libata about that somehow. I'm not sure what mechanism is the best for that at the moment. The current mechanism to notify libata is by rumtime suspend. When scsi device is runtime suspended, its parent device will be suspended. And ata_port is one of the ancestor devices of scsi device, and we will remove its power in ata_port's runtime suspend callback. Second, when the device is resumed by remote wakeup, we need to notify sr about that. A resume alone is not sufficient, though, because it may be necessary to open the tray. Perhaps in that case we can use the same mechanism by which user events are processed by libata and delivered to sr? Thanks for the suggestion. I'm not aware of any user events processed by libata. Do you mean the events_checking poll? I'm not sure about this events passing thing, as in that case, I will need to add code to listen to a socket in sr. Thanks, 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
