gdth: Use scsi_get_command for gdth internal commands
James Bottomley
maintainerQuoted 46 lines · Boaz Harrosh
This is for for the next kernel window (v2.6.26). It is based on top of both scsi-misc and scsi-rc-fixes. Once scsi-misc rebases to scsi-rc-fixes this patch can be committed.
Boaz
---
From: Boaz Harrosh <[email protected]>
Date: Tue, 11 Mar 2008 17:42:06 +0200
Subject: [PATCH] gdth: Usescsi_get_commandfor gdth internal commandsuse
scsi_get_command()in__gdth_execute()of internal commands for two reasons.
- To be insulated from future scsi-ml changes and the wayscsi_cmndis structured / allocated.
- Hold onto thescsi_devicewhile executing since execution can come from user-mode management SW through the gdth char device.Signed-off-by Boaz Harrosh <[email protected]>Tested-by Sven Schnelle <[email protected]>--- drivers/scsi/gdth.c | 12 ++---------- 1 files changed, 2 insertions(+), 10 deletions(-)drivers/scsi/gdth.c +1
-8@@ -448,17 +448,10 @@ int __gdth_execute(struct scsi_device *sdev, gdth_cmd_str *gdtcmd, char *cmnd,DECLARE_COMPLETION_ONSTACK (wait);int rval; - scp =kzalloc (sizeof (*scp),GFP_KERNEL ); + scp =scsi_get_command (sdev,GFP_KERNEL );if (!scp)return -ENOMEM ; - scp->sense_buffer =kzalloc (SCSI_SENSE_BUFFERSIZE ,GFP_KERNEL ); -if (!scp->sense_buffer) { -kfree (scp); -return -ENOMEM ; - } - - scp->device = sdev;memset (&cmndinfo,0 ,sizeof (cmndinfo));/* use request field to save the ptr. to completion struct. */
drivers/scsi/gdth.c int __gdth_execute(struct scsi_device *sdev, gdth_cmd_str *gdtcmd, char *cmnd, · line 475
Quoted 7 lines
drivers/scsi/gdth.c +1
-2@@ -478,8 +471,7 @@ int __gdth_execute(struct scsi_device *sdev, gdth_cmd_str *gdtcmd, char *cmnd, rval = cmndinfo.status;if (info) *info = cmndinfo.info; -kfree (scp->sense_buffer); -kfree (scp); +scsi_put_command (scp);return rval;
This needs to be the scsi_allocate_command/scsi_free_command interface. Other than that, it looks fine.
James
James Bottomley
maintainerQuoted 58 lines · Boaz Harrosh
Quoted 57 lines · James Bottomley
Quoted 46 lines · Boaz Harrosh
This is for for the next kernel window (v2.6.26). It is based on top of both scsi-misc and scsi-rc-fixes. Once scsi-misc rebases to scsi-rc-fixes this patch can be committed.
Boaz
---
From: Boaz Harrosh <[email protected]>
Date: Tue, 11 Mar 2008 17:42:06 +0200
Subject: [PATCH] gdth: Usescsi_get_commandfor gdth internal commandsuse
scsi_get_command()in__gdth_execute()of internal commands for two reasons.
- To be insulated from future scsi-ml changes and the wayscsi_cmndis structured / allocated.
- Hold onto thescsi_devicewhile executing since execution can come from user-mode management SW through the gdth char device.Signed-off-by Boaz Harrosh <[email protected]>Tested-by Sven Schnelle <[email protected]>--- drivers/scsi/gdth.c | 12 ++---------- 1 files changed, 2 insertions(+), 10 deletions(-)drivers/scsi/gdth.c +1
-8@@ -448,17 +448,10 @@ int __gdth_execute(struct scsi_device *sdev, gdth_cmd_str *gdtcmd, char *cmnd,DECLARE_COMPLETION_ONSTACK (wait);int rval; - scp =kzalloc (sizeof (*scp),GFP_KERNEL ); + scp =scsi_get_command (sdev,GFP_KERNEL );if (!scp)return -ENOMEM ; - scp->sense_buffer =kzalloc (SCSI_SENSE_BUFFERSIZE ,GFP_KERNEL ); -if (!scp->sense_buffer) { -kfree (scp); -return -ENOMEM ; - } - - scp->device = sdev;memset (&cmndinfo,0 ,sizeof (cmndinfo));/* use request field to save the ptr. to completion struct. */ drivers/scsi/gdth.c int __gdth_execute(struct scsi_device *sdev, gdth_cmd_str *gdtcmd, char *cmnd, · line 475
Quoted 7 lines
drivers/scsi/gdth.c +1
-2@@ -478,8 +471,7 @@ int __gdth_execute(struct scsi_device *sdev, gdth_cmd_str *gdtcmd, char *cmnd, rval = cmndinfo.status;if (info) *info = cmndinfo.info; -kfree (scp->sense_buffer); -kfree (scp); +scsi_put_command (scp);return rval;This needs to be the scsi_allocate_command/scsi_free_command interface. Other than that, it looks fine.
Quoted 5 lines
Quoted 3 lines
James
No, I disagree, and I said that in the commit log above.
You can't use scsi_get_command() because it will violate the forward progress guarantees that the writeout deadlock avoidance relies on.
Quoted 2 lines
I like how we take the reference to the device by doing it through the
scsi_device.
It's executing on the host device. The whole difference between the gdth_execute() and __gdth_execute() paths is whether you need a reference to the host device, so another reference is redundant.
Quoted 6 lines
__gdth_executein
principle should have been using simplescsi_execute(). It does not because it sendsnone_standardcommands that would choke the midlayer. Actually once my varlen patches go through we could easily convert it.(In the USB case you are right)
James
Quoted 57 lines · James Bottomley
Quoted 46 lines · Boaz Harrosh
This is for for the next kernel window (v2.6.26). It is based on top of both scsi-misc and scsi-rc-fixes. Once scsi-misc rebases to scsi-rc-fixes this patch can be committed.
Boaz
---
From: Boaz Harrosh <[email protected]>
Date: Tue, 11 Mar 2008 17:42:06 +0200
Subject: [PATCH] gdth: Usescsi_get_commandfor gdth internal commandsuse
scsi_get_command()in__gdth_execute()of internal commands for two reasons.
- To be insulated from future scsi-ml changes and the wayscsi_cmndis structured / allocated.
- Hold onto thescsi_devicewhile executing since execution can come from user-mode management SW through the gdth char device.Signed-off-by Boaz Harrosh <[email protected]>Tested-by Sven Schnelle <[email protected]>--- drivers/scsi/gdth.c | 12 ++---------- 1 files changed, 2 insertions(+), 10 deletions(-)drivers/scsi/gdth.c +1
-8@@ -448,17 +448,10 @@ int __gdth_execute(struct scsi_device *sdev, gdth_cmd_str *gdtcmd, char *cmnd,DECLARE_COMPLETION_ONSTACK (wait);int rval; - scp =kzalloc (sizeof (*scp),GFP_KERNEL ); + scp =scsi_get_command (sdev,GFP_KERNEL );if (!scp)return -ENOMEM ; - scp->sense_buffer =kzalloc (SCSI_SENSE_BUFFERSIZE ,GFP_KERNEL ); -if (!scp->sense_buffer) { -kfree (scp); -return -ENOMEM ; - } - - scp->device = sdev;memset (&cmndinfo,0 ,sizeof (cmndinfo));/* use request field to save the ptr. to completion struct. */ drivers/scsi/gdth.c int __gdth_execute(struct scsi_device *sdev, gdth_cmd_str *gdtcmd, char *cmnd, · line 475
Quoted 7 lines
drivers/scsi/gdth.c +1
-2@@ -478,8 +471,7 @@ int __gdth_execute(struct scsi_device *sdev, gdth_cmd_str *gdtcmd, char *cmnd, rval = cmndinfo.status;if (info) *info = cmndinfo.info; -kfree (scp->sense_buffer); -kfree (scp); +scsi_put_command (scp);return rval;This needs to be the scsi_allocate_command/scsi_free_command interface. Other than that, it looks fine.
Quoted 3 lines
James
No, I disagree, and I said that in the commit log above. I like how we take the reference to the device by doing it through the scsi_device. __gdth_execute in principle should have been using simple scsi_execute(). It does not because it sends none_standard commands that would choke the midlayer. Actually once my varlen patches go through we could easily convert it.
(In the USB case you are right)
Boaz
PATCH 1/1 gdth: Use scsi_get_command for gdth internal commands
This is for for the next kernel window (v2.6.26). It is based on top of both scsi-misc and scsi-rc-fixes. Once scsi-misc rebases to scsi-rc-fixes this patch can be committed.
Boaz
---
From: Boaz Harrosh <[email protected]>
Date: Tue, 11 Mar 2008 17:42:06 +0200
Subject: [PATCH] gdth: Use scsi_get_command for gdth internal commands
use scsi_get_command() in __gdth_execute() of internal commands for two reasons.
- To be insulated from future scsi-ml changes and the way scsi_cmnd is structured / allocated.
- Hold onto the scsi_device while executing since execution can come from user-mode management SW through the gdth char device.
--- drivers/scsi/gdth.c | 12 ++---------- 1 files changed, 2 insertions(+), 10 deletions(-)
drivers/scsi/gdth.c +2 -10
@@ -448,17 +448,10 @@ int __gdth_execute(struct scsi_device *sdev, gdth_cmd_str *gdtcmd, char *cmnd,DECLARE_COMPLETION_ONSTACK (wait);int rval; - scp =kzalloc (sizeof (*scp),GFP_KERNEL ); + scp =scsi_get_command (sdev,GFP_KERNEL );if (!scp)return -ENOMEM ; - scp->sense_buffer =kzalloc (SCSI_SENSE_BUFFERSIZE ,GFP_KERNEL ); -if (!scp->sense_buffer) { -kfree (scp); -return -ENOMEM ; - } - - scp->device = sdev;memset (&cmndinfo,0 ,sizeof (cmndinfo));/* use request field to save the ptr. to completion struct. */
@@ -478,8 +471,7 @@ int __gdth_execute(struct scsi_device *sdev, gdth_cmd_str *gdtcmd, char *cmnd, rval = cmndinfo.status;if (info) *info = cmndinfo.info; -kfree (scp->sense_buffer); -kfree (scp); +scsi_put_command (scp);return rval; }
Signature · 1 line
1.5.3.3