* [PATCH] SG_SCSI_RESET ioctl: add no_escalate values
@ 2013-02-15 19:39 Douglas Gilbert
2013-02-15 19:46 ` Jeremy Linton
2013-02-15 21:48 ` Mike Christie
0 siblings, 2 replies; 6+ messages in thread
From: Douglas Gilbert @ 2013-02-15 19:39 UTC (permalink / raw)
To: SCSI development list; +Cc: Jeremy Linton
[-- Attachment #1: Type: text/plain, Size: 721 bytes --]
Further to the thread titled: "[PATCH] SG_SCSI_RESET ioctl should
only perform requested operation" by Jeremy Linton a patch
is presented that adds "no_escalate" versions to the existing
ioctl. This should not break any existing code.
This patches applies to lk 3.7.7 and lk 3.8.0-rc7 . I will extend
sg_reset in the sg3_utils package to use it.
ChangeLog:
- modify SG_SCSI_RESET ioctl so the SG_SCSI_RESET_NO_ESCALATE
value may be added to the existing values. If so the existing
device->target->bus->host escalation does not occur.
- modify scsi_reset_provider() in the scsi_error.c file in a
similar way to support this new functionality.
Signed-off-by: Douglas Gilbert <dgilbert@interlog.com>
[-- Attachment #2: scsi_reset_no_escal1.patch --]
[-- Type: text/x-patch, Size: 4015 bytes --]
diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
index c1b05a8..4ba79c1 100644
--- a/drivers/scsi/scsi_error.c
+++ b/drivers/scsi/scsi_error.c
@@ -2013,8 +2013,18 @@ scsi_reset_provider(struct scsi_device *dev, int flag)
break;
/* FALLTHROUGH */
case SCSI_TRY_RESET_HOST:
+ case SCSI_TRY_RESET_HOST + SCSI_TRY_RESET_NO_ESCALATE:
rtn = scsi_try_host_reset(scmd);
break;
+ case SCSI_TRY_RESET_DEVICE + SCSI_TRY_RESET_NO_ESCALATE:
+ rtn = scsi_try_bus_device_reset(scmd);
+ break;
+ case SCSI_TRY_RESET_TARGET + SCSI_TRY_RESET_NO_ESCALATE:
+ rtn = scsi_try_target_reset(scmd);
+ break;
+ case SCSI_TRY_RESET_BUS + SCSI_TRY_RESET_NO_ESCALATE:
+ rtn = scsi_try_bus_reset(scmd);
+ break;
default:
rtn = FAILED;
}
diff --git a/include/scsi/scsi_eh.h b/include/scsi/scsi_eh.h
index 06a8790..49d07e9 100644
--- a/include/scsi/scsi_eh.h
+++ b/include/scsi/scsi_eh.h
@@ -62,11 +62,16 @@ extern void scsi_build_sense_buffer(int desc, u8 *buf, u8 key, u8 asc, u8 ascq);
/*
* Reset request from external source
+ * Note: if SCSI_TRY_RESET_DEVICE fails then it will escalate to
+ * SCSI_TRY_RESET_TARGET which if it fails will escalate to
+ * SCSI_TRY_RESET_BUS which if it fails will escalate to SCSI_TRY_RESET_HOST.
+ * To prevent escalation add SCSI_TRY_RESET_NO_ESCALATE.
*/
#define SCSI_TRY_RESET_DEVICE 1
#define SCSI_TRY_RESET_BUS 2
#define SCSI_TRY_RESET_HOST 3
#define SCSI_TRY_RESET_TARGET 4
+#define SCSI_TRY_RESET_NO_ESCALATE 0x100 /* may add to prior defines */
extern int scsi_reset_provider(struct scsi_device *, int);
diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index be2c9a6..d9226ec 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -810,7 +810,7 @@ sg_ioctl(struct file *filp, unsigned int cmd_in, unsigned long arg)
{
void __user *p = (void __user *)arg;
int __user *ip = p;
- int result, val, read_only;
+ int result, val, val2, read_only;
Sg_device *sdp;
Sg_fd *sfp;
Sg_request *srp;
@@ -1045,27 +1045,32 @@ sg_ioctl(struct file *filp, unsigned int cmd_in, unsigned long arg)
result = get_user(val, ip);
if (result)
return result;
+ if (val >= SG_SCSI_RESET_NO_ESCALATE) {
+ val -= SG_SCSI_RESET_NO_ESCALATE;
+ val2 = SCSI_TRY_RESET_NO_ESCALATE;
+ } else
+ val2 = 0;
if (SG_SCSI_RESET_NOTHING == val)
return 0;
switch (val) {
case SG_SCSI_RESET_DEVICE:
- val = SCSI_TRY_RESET_DEVICE;
+ val2 += SCSI_TRY_RESET_DEVICE;
break;
case SG_SCSI_RESET_TARGET:
- val = SCSI_TRY_RESET_TARGET;
+ val2 += SCSI_TRY_RESET_TARGET;
break;
case SG_SCSI_RESET_BUS:
- val = SCSI_TRY_RESET_BUS;
+ val2 += SCSI_TRY_RESET_BUS;
break;
case SG_SCSI_RESET_HOST:
- val = SCSI_TRY_RESET_HOST;
+ val2 += SCSI_TRY_RESET_HOST;
break;
default:
return -EINVAL;
}
if (!capable(CAP_SYS_ADMIN) || !capable(CAP_SYS_RAWIO))
return -EACCES;
- return (scsi_reset_provider(sdp->device, val) ==
+ return (scsi_reset_provider(sdp->device, val2) ==
SUCCESS) ? 0 : -EIO;
case SCSI_IOCTL_SEND_COMMAND:
if (sdp->detached)
diff --git a/include/scsi/sg.h b/include/scsi/sg.h
index a9f3c6f..8207171 100644
--- a/include/scsi/sg.h
+++ b/include/scsi/sg.h
@@ -204,12 +204,15 @@ typedef struct sg_req_info { /* used by SG_GET_REQUEST_TABLE ioctl() */
/* Returns -EBUSY if occupied. 3rd argument pointer to int (see next) */
#define SG_SCSI_RESET 0x2284
-/* Associated values that can be given to SG_SCSI_RESET follow */
+/* Associated values that can be given to SG_SCSI_RESET follow.
+ * SG_SCSI_RESET_NO_ESCALATE may be added to the _DEVICE, _TARGET, _BUS
+ * or _HOST reset value so only that action is attempted. */
#define SG_SCSI_RESET_NOTHING 0
#define SG_SCSI_RESET_DEVICE 1
#define SG_SCSI_RESET_BUS 2
#define SG_SCSI_RESET_HOST 3
#define SG_SCSI_RESET_TARGET 4
+#define SG_SCSI_RESET_NO_ESCALATE 0x100
/* synchronous SCSI command ioctl, (only in version 3 interface) */
#define SG_IO 0x2285 /* similar effect as write() followed by read() */
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] SG_SCSI_RESET ioctl: add no_escalate values
2013-02-15 19:39 [PATCH] SG_SCSI_RESET ioctl: add no_escalate values Douglas Gilbert
@ 2013-02-15 19:46 ` Jeremy Linton
2013-02-15 21:48 ` Mike Christie
1 sibling, 0 replies; 6+ messages in thread
From: Jeremy Linton @ 2013-02-15 19:46 UTC (permalink / raw)
To: dgilbert@interlog.com; +Cc: SCSI development list
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On 2/15/2013 1:39 PM, Douglas Gilbert wrote:
> Further to the thread titled: "[PATCH] SG_SCSI_RESET ioctl should only
> perform requested operation" by Jeremy Linton a patch is presented that
> adds "no_escalate" versions to the existing ioctl. This should not break
> any existing code.
Looks good, I will apply it here and try it out.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (MingW32)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
iQEcBAEBAgAGBQJRHpCDAAoJEL5i86xrzcy7lvoIAK5pxe8rdx8WinxG/NqhLkRb
+537Ln3asshkWcH385gaa1JzizoWhiS1+H+tGcLdEwPDwvSSGGqtUAn/qtxB64cM
F5X0XqjS/XVup7GrkcoQ4ZzXG2rdI6Lb5gwbpb98QeCGzaVKfRMvJCCtXSIFuPXA
S7gL0Xl5d5KapPCWVRpucE05XVaAZq2vrxC3/8hDwB8+3HYUW4gUTcwSAM0pvcUb
d7w984jlcFHVxT3Yk+ZHuQ3QOKifbqWBE9WlsgqkPnGjlAK8Q19NQsD6/C9Q9wTg
OhMjj9HKHOlcBE65e/xKlcncTXjaa1qcLfa94hOukjSQBkf0vqJH3XeWuACESE0=
=Oqr2
-----END PGP SIGNATURE-----
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] SG_SCSI_RESET ioctl: add no_escalate values
2013-02-15 19:39 [PATCH] SG_SCSI_RESET ioctl: add no_escalate values Douglas Gilbert
2013-02-15 19:46 ` Jeremy Linton
@ 2013-02-15 21:48 ` Mike Christie
2013-02-15 23:32 ` Douglas Gilbert
1 sibling, 1 reply; 6+ messages in thread
From: Mike Christie @ 2013-02-15 21:48 UTC (permalink / raw)
To: dgilbert; +Cc: SCSI development list, Jeremy Linton
On 02/15/2013 01:39 PM, Douglas Gilbert wrote:
> Further to the thread titled: "[PATCH] SG_SCSI_RESET ioctl should
> only perform requested operation" by Jeremy Linton a patch
> is presented that adds "no_escalate" versions to the existing
> ioctl. This should not break any existing code.
>
> This patches applies to lk 3.7.7 and lk 3.8.0-rc7 . I will extend
> sg_reset in the sg3_utils package to use it.
>
> ChangeLog:
> - modify SG_SCSI_RESET ioctl so the SG_SCSI_RESET_NO_ESCALATE
> value may be added to the existing values. If so the existing
> device->target->bus->host escalation does not occur.
> - modify scsi_reset_provider() in the scsi_error.c file in a
> similar way to support this new functionality.
>
> Signed-off-by: Douglas Gilbert <dgilbert@interlog.com>
Some drivers rely on more invasive eh callbacks to be called if they
return FAILED in a eh callbacks. Is there a way for drivers to tell
scsi-ml it needs the old behavior?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] SG_SCSI_RESET ioctl: add no_escalate values
2013-02-15 21:48 ` Mike Christie
@ 2013-02-15 23:32 ` Douglas Gilbert
2013-02-16 5:45 ` Michael Christie
0 siblings, 1 reply; 6+ messages in thread
From: Douglas Gilbert @ 2013-02-15 23:32 UTC (permalink / raw)
To: Mike Christie; +Cc: SCSI development list, Jeremy Linton
On 13-02-15 04:48 PM, Mike Christie wrote:
> On 02/15/2013 01:39 PM, Douglas Gilbert wrote:
>> Further to the thread titled: "[PATCH] SG_SCSI_RESET ioctl should
>> only perform requested operation" by Jeremy Linton a patch
>> is presented that adds "no_escalate" versions to the existing
>> ioctl. This should not break any existing code.
>>
>> This patches applies to lk 3.7.7 and lk 3.8.0-rc7 . I will extend
>> sg_reset in the sg3_utils package to use it.
>>
>> ChangeLog:
>> - modify SG_SCSI_RESET ioctl so the SG_SCSI_RESET_NO_ESCALATE
>> value may be added to the existing values. If so the existing
>> device->target->bus->host escalation does not occur.
>> - modify scsi_reset_provider() in the scsi_error.c file in a
>> similar way to support this new functionality.
>>
>> Signed-off-by: Douglas Gilbert <dgilbert@interlog.com>
>
> Some drivers rely on more invasive eh callbacks to be called if they
> return FAILED in a eh callbacks. Is there a way for drivers to tell
> scsi-ml it needs the old behavior?
Mike,
The old behaviour hasn't changed both at the
ioctl(SG_SCSI_RESET) level and the underlying kernel
scsi_reset_provider() function. In both cases extra
values have been added: to third argument of the ioctl
and the second argument ('flag') of the
scsi_reset_provider() function. The existing values
will do exactly the same thing (i.e. escalate) with
the same return values.
The only way it is different is that values that were
previously errors (precisely 0x101 to 0x104) now cause a
non-escalating device(LU)/target/bus reset.
Doug Gilbert
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] SG_SCSI_RESET ioctl: add no_escalate values
2013-02-15 23:32 ` Douglas Gilbert
@ 2013-02-16 5:45 ` Michael Christie
2013-02-16 6:44 ` Michael Christie
0 siblings, 1 reply; 6+ messages in thread
From: Michael Christie @ 2013-02-16 5:45 UTC (permalink / raw)
To: dgilbert; +Cc: SCSI development list, Jeremy Linton
On Feb 15, 2013, at 5:32 PM, Douglas Gilbert <dgilbert@interlog.com> wrote:
> On 13-02-15 04:48 PM, Mike Christie wrote:
>> On 02/15/2013 01:39 PM, Douglas Gilbert wrote:
>>> Further to the thread titled: "[PATCH] SG_SCSI_RESET ioctl should
>>> only perform requested operation" by Jeremy Linton a patch
>>> is presented that adds "no_escalate" versions to the existing
>>> ioctl. This should not break any existing code.
>>>
>>> This patches applies to lk 3.7.7 and lk 3.8.0-rc7 . I will extend
>>> sg_reset in the sg3_utils package to use it.
>>>
>>> ChangeLog:
>>> - modify SG_SCSI_RESET ioctl so the SG_SCSI_RESET_NO_ESCALATE
>>> value may be added to the existing values. If so the existing
>>> device->target->bus->host escalation does not occur.
>>> - modify scsi_reset_provider() in the scsi_error.c file in a
>>> similar way to support this new functionality.
>>>
>>> Signed-off-by: Douglas Gilbert <dgilbert@interlog.com>
>>
>> Some drivers rely on more invasive eh callbacks to be called if they
>> return FAILED in a eh callbacks. Is there a way for drivers to tell
>> scsi-ml it needs the old behavior?
>
> Mike,
> The old behaviour hasn't changed both at the
> ioctl(SG_SCSI_RESET) level and the underlying kernel
> scsi_reset_provider() function. In both cases extra
> values have been added: to third argument of the ioctl
> and the second argument ('flag') of the
> scsi_reset_provider() function. The existing values
> will do exactly the same thing (i.e. escalate) with
> the same return values.
>
> The only way it is different is that values that were
> previously errors (precisely 0x101 to 0x104) now cause a
> non-escalating device(LU)/target/bus reset.
Sorry for the confusion. I should have written is there a way for drivers to tell scsi-ml it only supports the old behavior and does not support the new calls? If not then I am guessing this is mandatory to support and we need to fix the drivers right?
If userspace sends a command that performs the non-escalting *reset to some drivers, it is going to leave them in a state where they may not process IO or will crash.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] SG_SCSI_RESET ioctl: add no_escalate values
2013-02-16 5:45 ` Michael Christie
@ 2013-02-16 6:44 ` Michael Christie
0 siblings, 0 replies; 6+ messages in thread
From: Michael Christie @ 2013-02-16 6:44 UTC (permalink / raw)
To: dgilbert; +Cc: SCSI development list, Jeremy Linton
On Feb 15, 2013, at 11:45 PM, Michael Christie <michaelc@cs.wisc.edu> wrote:
>>> Some drivers rely on more invasive eh callbacks to be called if they
>>> return FAILED in a eh callbacks.
> If userspace sends a command that performs the non-escalting *reset to some drivers, it is going to leave them in a state where they may not process IO or will crash.
Actually, I take those 2 comments back. I think it is just some drivers abort callbacks that might be issues, so we should be safe. I did not look at all drivers though.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-02-16 6:45 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-02-15 19:39 [PATCH] SG_SCSI_RESET ioctl: add no_escalate values Douglas Gilbert
2013-02-15 19:46 ` Jeremy Linton
2013-02-15 21:48 ` Mike Christie
2013-02-15 23:32 ` Douglas Gilbert
2013-02-16 5:45 ` Michael Christie
2013-02-16 6:44 ` Michael Christie
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox