Linux SCSI driver development and usage

David C Somayajulu | 1 May 2008 02:36

Re: [PATCH]qla4xxx:Add support for Async Message PDUs [REPOST with fixes]

On Wed, 2008-04-30 at 14:36 -0500, Mike Christie wrote:
> Mike Christie wrote:
> > David C Somayajulu wrote:
> >> +        }
> >> +        if (sshdr.asc == 0x3f && sshdr.ascq == 0x0e) {
> >> +            /* induce rescan */
> >> +            iscsi_block_session(ddb_entry->sess);
> >> +            iscsi_unblock_session(ddb_entry->sess);
> >> +        }
> > 
> > I think I was wrong off list. We could actually just call 
> > iscsi_unblock_session, but we do not know the state of the session do 
> > we? If we got a event that indicated the session was failed could we 
> > have a block queued up and this would reverse it by accident?
> > 
> > I was thinking that because qla4xxx_process_aen is called before this we 
> > could just check the ddb state, but we can call iscsi_block_session from 
> > the interrupt handler to queue up a block so I think that could race 
> > with the unblock call here.
> 
> Maybe to handle the race we do not want to call qla4xxx_mark_device_busy 
> when we get SCS_TIMEOUT. If we get a ddb aen then could we just wait for 
> that and all those events would be serialized through the dpc thread?
> 
> What does SCS_TIMEOUT mean?
I think the driver on receiving the Async Message PDU of type
ISCSI_ASYNC_MSG_SCSI_EVENT, should first retrieve the DDB state. If the
session is still not active, it should block it. If the DDB state
indicates the session is active, then call iscsi_unblock_session() if
rescan is needed. What do you think of this solution ?

(Continue reading)

Pontus Lidman | 1 May 2008 14:54

Picon

Picon

Picon

Hack to not step on vendor commands for some devices

Hi all,

I'm trying to support an usb-storage device that uses vendor-specific
SCSI commands for some functions; I'm using the sg interface from user
space.

The problem is, the linux kernel will replace some bits in the
vendor-specific command with the LUN number, making it unintelligible
to the device. I wrote the included patch that works around this
problem.

cmd->device->scsi_level is SCSI_2 for this device.

Is there any other user-space solution to this problem? If not, what
are the next steps to have a solution appear in the mainline kernel?

Best Regards,

Pontus

--

-- 
Pontus Lidman, pontus <at> lysator.liu.se, Software Engineer

diff --recursive -u linux-2.6.24.5/drivers/scsi/scsi.c linux-2.6.24.5.pl/drivers/scsi/scsi.c
--- linux-2.6.24.5/drivers/scsi/scsi.c	2008-04-19 03:53:39.000000000 +0200
+++ linux-2.6.24.5.pl/drivers/scsi/scsi.c	2008-04-27 17:17:35.000000000 +0200
 <at>  <at>  -501,9 +501,12  <at>  <at> 

 	/* 
 	 * If SCSI-2 or lower, store the LUN value in cmnd.

(Continue reading)

Boaz Harrosh | 1 May 2008 15:47

[PATCH 0/4] Use of new scsi_allocate_command

Submitted are two drivers that should use the new scsi_allocate_command
instead of kmalloc their own.

But first we need some changes. Since the GFP_DMA support is going away
(Submitted next) I would like to change the API a bit before any users
use this code.

Also I would like to snick in a gdth change that is related to this effort.

[PATCH 1/4] scsi_free_command API change - Don't support GFP_DMA
  ISA support is going away and is not needed by any potential clients
  of this code. So change the API now before it has any users.

[PATCH 2/4] isd200: Use new scsi_allocate_command()
  Use scsi_allocate_command in isd200 driver

[PATCH 3/4] gdth: consolidate __gdth_execute && gdth_execute
  Cleanup and optimization. Please see patch for detailed explanations

[PATCH 4/4] gdth: Use scsi_allocate_command for private command allocation

Boaz
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo <at> vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Boaz Harrosh | 1 May 2008 15:56

[PATCH 1/4] scsi_free_command API change - Don't support GFP_DMA


Support of GFP_DMA for scsi_cmnd is going away. Don't let new
users get used to it. (None of the potential users need it)
Change that now before it has any users.

Signed-off-by: Boaz Harrosh <bharrosh <at> panasas.com>
---
 drivers/scsi/scsi.c      |   23 +++++++++--------------
 include/scsi/scsi_cmnd.h |    4 ++--
 2 files changed, 11 insertions(+), 16 deletions(-)

diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
index 749c9c7..a120c04 100644
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
 <at>  <at>  -386,13 +386,13  <at>  <at>  static void scsi_put_host_cmd_pool(gfp_t gfp_mask)

 /**
  * scsi_allocate_command - get a fully allocated SCSI command
- *  <at> gfp_mask:	allocation mask
+ *  <at> gfp_mask:	allocation mask (must not be __GFP_DMA)
  *
  * This function is for use outside of the normal host based pools.
- * It allocates the relevant command and takes an additional reference
- * on the pool it used.  This function *must* be paired with
- * scsi_free_command which also has the identical mask, otherwise the
- * free pool counts will eventually go wrong and you'll trigger a bug.
+ * It allocates a command and takes an additional reference on the
+ * pool. This function *must* be paired with scsi_free_command
+ * otherwise the free pool counts will eventually go wrong and will

(Continue reading)

Boaz Harrosh | 1 May 2008 15:58

[PATCH 2/4] isd200: Use new scsi_allocate_command()


Don't let isd200 struggle with scsi_cmnd allocation and construction.
Use the new scsi_allocate_command()/scsi_free_command() for that.

Signed-off-by: Boaz Harrosh <bharrosh <at> panasas.com>
CC: Alan Stern <stern <at> rowland.harvard.edu>
CC: Matthew Dharm <mdharm-usb <at> one-eyed-alien.net>
---
 drivers/usb/storage/isd200.c |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/storage/isd200.c b/drivers/usb/storage/isd200.c
index 971d13d..756d30f 100644
--- a/drivers/usb/storage/isd200.c
+++ b/drivers/usb/storage/isd200.c
 <at>  <at>  -292,7 +292,7  <at>  <at>  struct isd200_info {

 	/* maximum number of LUNs supported */
 	unsigned char MaxLUNs;
-	struct scsi_cmnd srb;
+	struct scsi_cmnd *srb;
 	struct scatterlist sg;
 };

 <at>  <at>  -414,7 +414,7  <at>  <at>  static void isd200_build_sense(struct us_data *us, struct scsi_cmnd *srb)
 static void isd200_set_srb(struct isd200_info *info,
 	enum dma_data_direction dir, void* buff, unsigned bufflen)
 {
-	struct scsi_cmnd *srb = &info->srb;
+	struct scsi_cmnd *srb = info->srb;

(Continue reading)

Boaz Harrosh | 1 May 2008 16:02

[PATCH 3/3] gdth: consolidate __gdth_execute && gdth_execute


ioctl into gdth's char device would allocate an host-device at file-open
and would cache that device until the removal of the host, not at file
close. On the other hand a /proc access to gdth, and also flush() call at
host removal, would allocate a new device on every call and destroy it. Even
if one was allocated by file-open.

All this optimization is just a mess. But my primary concern is the construction
of the device just before destruction at flush(), looks extra pointless to me.

So allocate an host-device for every host, and remove it at host removal.
Now the existence of __gdth_execute is pointless and is removed. Also
gdth_execute can operate directly on gdth_ha_str.

Signed-off-by: Boaz Harrosh <bharrosh <at> panasas.com>
---
 drivers/scsi/gdth.c      |   45 +++++++++++++++------------------------------
 drivers/scsi/gdth_proc.c |   16 ++++++++--------
 drivers/scsi/gdth_proc.h |    2 +-
 3 files changed, 24 insertions(+), 39 deletions(-)

diff --git a/drivers/scsi/gdth.c b/drivers/scsi/gdth.c
index c6d6e7c..35ad3bf 100644
--- a/drivers/scsi/gdth.c
+++ b/drivers/scsi/gdth.c
 <at>  <at>  -439,10 +439,9  <at>  <at>  static void gdth_scsi_done(struct scsi_cmnd *scp)
 		scp->scsi_done(scp);
 }

-int __gdth_execute(struct scsi_device *sdev, gdth_cmd_str *gdtcmd, char *cmnd,

(Continue reading)

Boaz Harrosh | 1 May 2008 16:06

[PATCH 4/4] gdth: Use scsi_allocate_command for private command allocation


In gdth_execute() use scsi_allocate_command for allocation of
a command. To be insulated from future scsi_cmnd construction
considerations.

Signed-off-by: Boaz Harrosh <bharrosh <at> panasas.com>
---
 drivers/scsi/gdth.c |   12 +++---------
 1 files changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/scsi/gdth.c b/drivers/scsi/gdth.c
index 35ad3bf..7778373 100644
--- a/drivers/scsi/gdth.c
+++ b/drivers/scsi/gdth.c
 <at>  <at>  -447,16 +447,10  <at>  <at>  int gdth_execute(gdth_ha_str *ha, gdth_cmd_str *gdtcmd, char *cmnd,
     DECLARE_COMPLETION_ONSTACK(wait);
     int rval;

-    scp = kzalloc(sizeof(*scp), GFP_KERNEL);
+    scp = scsi_allocate_command(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 = ha->sdev;

(Continue reading)

James Bottomley | 1 May 2008 16:24

Re: [PATCH 0/4] Use of new scsi_allocate_command

On Thu, 2008-05-01 at 16:47 +0300, Boaz Harrosh wrote:
> [PATCH 1/4] scsi_free_command API change - Don't support GFP_DMA
>   ISA support is going away and is not needed by any potential clients
>   of this code. So change the API now before it has any users.

What makes you think this?  We still have lots of ISA installations out
there (and some broken PCI ones requiring ISA DMA masks) that we have to
support.

For gdth you have an ISA board that *you* need to support, so the isa
board has to have this flag.  The reason no-one's run into a bug here is
that the problematic allocation is the sense buffer not the command.
Unfortunately, when the fix was added to the gdth driver for the sense
buffer separation:

commit 1b96f8955aaeeb05f7fb7ff548aa12415fbf3904
Author: Sven Schnelle <svens <at> bitebene.org>
Date:   Mon Mar 10 22:50:04 2008 +0100

    [SCSI] gdth: Allocate sense_buffer to prevent NULL pointer dereference

We actually forgot about this case.  Right at the moment, any ioctl that
uses gdth_execute on an ISA device will probably fail if the executed
command resturns sense and there look to be paths down gdth_execute even
for special commands where the OpCode flips to a regular command (-1)
and so DMAs into the sense buffer.

James

--

(Continue reading)

Boaz Harrosh | 1 May 2008 16:56

Re: [PATCH 0/4] Use of new scsi_allocate_command

On Thu, May 01 2008 at 17:24 +0300, James Bottomley
<James.Bottomley@...> wrote:
> On Thu, 2008-05-01 at 16:47 +0300, Boaz Harrosh wrote:
>> [PATCH 1/4] scsi_free_command API change - Don't support GFP_DMA
>>   ISA support is going away and is not needed by any potential clients
>>   of this code. So change the API now before it has any users.
> 
> What makes you think this?  We still have lots of ISA installations out
> there (and some broken PCI ones requiring ISA DMA masks) that we have to
> support.
> 

Believe me I'm an expert. I'm working on the complete removal of GFP_DMA dma
support, initiated by Andi Kleen. And will submit the complete work on Sunday
once reviewed by Andi and some preliminary test pass.

> For gdth you have an ISA board that *you* need to support, so the isa
> board has to have this flag.  The reason no-one's run into a bug here is
> that the problematic allocation is the sense buffer not the command.
> Unfortunately, when the fix was added to the gdth driver for the sense
> buffer separation:
> 
> commit 1b96f8955aaeeb05f7fb7ff548aa12415fbf3904
> Author: Sven Schnelle <svens@...>
> Date:   Mon Mar 10 22:50:04 2008 +0100
> 
>     [SCSI] gdth: Allocate sense_buffer to prevent NULL pointer dereference
> 
> We actually forgot about this case.  Right at the moment, any ioctl that
> uses gdth_execute on an ISA device will probably fail if the executed

(Continue reading)

Mike Christie | 1 May 2008 16:59

Picon

Re: [PATCH]qla4xxx:Add support for Async Message PDUs [REPOST with fixes]

David C Somayajulu wrote:
> On Wed, 2008-04-30 at 14:36 -0500, Mike Christie wrote:
>> Mike Christie wrote:
>>> David C Somayajulu wrote:
>>>> +        }
>>>> +        if (sshdr.asc == 0x3f && sshdr.ascq == 0x0e) {
>>>> +            /* induce rescan */
>>>> +            iscsi_block_session(ddb_entry->sess);
>>>> +            iscsi_unblock_session(ddb_entry->sess);
>>>> +        }
>>> I think I was wrong off list. We could actually just call 
>>> iscsi_unblock_session, but we do not know the state of the session do 
>>> we? If we got a event that indicated the session was failed could we 
>>> have a block queued up and this would reverse it by accident?
>>>
>>> I was thinking that because qla4xxx_process_aen is called before this we 
>>> could just check the ddb state, but we can call iscsi_block_session from 
>>> the interrupt handler to queue up a block so I think that could race 
>>> with the unblock call here.
>> Maybe to handle the race we do not want to call qla4xxx_mark_device_busy 
>> when we get SCS_TIMEOUT. If we get a ddb aen then could we just wait for 
>> that and all those events would be serialized through the dpc thread?
>>
>> What does SCS_TIMEOUT mean?
> I think the driver on receiving the Async Message PDU of type
> ISCSI_ASYNC_MSG_SCSI_EVENT, should first retrieve the DDB state. If the
> session is still not active, it should block it. If the DDB state

You should not need to block it though. qla4xxx_process_aen will have 
done this already when it set the ddb state, wouldn't it? I am saying if

(Continue reading)

Group

gmane.linux.scsi.

Search Archive

Page

1 | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 9 | 10 | 11 | 79

Articles in period: 782

May 2008

Language

Change language

Options

Current view: Threads only / Showing only 20 lines / Not hiding cited text.
Change to All messages, whole messages, or hide cited text.

Post a message
NNTP Newsgroup
Classic Gmane web interface
XML

XML

RSS Feed
List Information

About Gmane

Recent entries

[PATCH 0/1] ipr: possible irq lock inversion dependency detected (24 May 2013)
Question: eh_abort_handler() and terminate commands (24 May 2013)
[PATCH 0/4] New FC timeout handler (24 May 2013)
re :SCSI error handling -- one error blocks the whole SCSI host (23 May 2013)
SCSI error handling -- one error blocks the whole SCSI host (23 May 2013)
[PATCH] scsi: use platform_{get,set}_drvdata() (23 May 2013)
Hello (23 May 2013)
[PATCH 12/12] megaraid_sas : Changelog and driver version update (22 May 2013)
[PATCH 11/12] megaraid_sas : Add support to differentiate between iMR (22 May 2013)
[PATCH 10/12] megaraid_sas : Add support for Uneven Span PRL11 (22 May 2013)
[PATCH 09/12] megaraid_sas : Add support for Extended MSI-x vectors fo (22 May 2013)
[PATCH 08/12] megaraid_sas : Set IoFlags to enable Fast Path for JBODs (22 May 2013)
[PATCH 07/12] megaraid_sas : Add support to display Customer branding (22 May 2013)
[PATCH 06/12] megaraid_sas : Add support for MegaRAID Fury (device ID- (22 May 2013)
[PATCH 05/12] megaraid_sas : Set IO request timeout value provided by (22 May 2013)
[PATCH 04/12] megaraid_sas : Free event detail memory without device I (22 May 2013)
[PATCH 03/12] megaraid_sas : Update balance count in driver to be in s (22 May 2013)
[PATCH 02/12] megaraid_sas : Fix the interrupt mask for Gen2 controlle (22 May 2013)
[PATCH 01/12] megaraid_sas : Return DID_ERROR for SCSI IO, when contro (22 May 2013)
[PATCH 00/12] megaraid_sas : Updates for scsi for-next (22 May 2013)
[PATCH 1/6] scsi/a100u2w: Use module_pci_driver to register driver (21 May 2013)

Archives

369	May 2013
366	April 2013
352	March 2013
387	February 2013
287	January 2013
323	December 2012
294	November 2012
333	October 2012
445	September 2012
393	August 2012
266	July 2012
340	June 2012
558	May 2012
356	April 2012
505	March 2012
423	February 2012
441	January 2012
347	December 2011
255	November 2011
300	October 2011
262	September 2011
467	August 2011
643	July 2011
392	June 2011
430	May 2011
255	April 2011
578	March 2011
514	February 2011
513	January 2011
479	December 2010
642	November 2010
502	October 2010
600	September 2010
441	August 2010
581	July 2010
480	June 2010
481	May 2010
363	April 2010
530	March 2010
429	February 2010
340	January 2010
400	December 2009
635	November 2009
387	October 2009
848	September 2009
930	August 2009
620	July 2009
769	June 2009
985	May 2009
1000	April 2009
744	March 2009
656	February 2009
824	January 2009
896	December 2008
669	November 2008
757	October 2008
586	September 2008
711	August 2008
874	July 2008
614	June 2008
782	May 2008
802	April 2008
1115	March 2008
1200	February 2008
1057	January 2008
712	December 2007
555	November 2007
1117	October 2007
805	September 2007
562	August 2007
1027	July 2007
445	June 2007
769	May 2007
485	April 2007
507	March 2007
544	February 2007
598	January 2007
419	December 2006
508	November 2006
612	October 2006
490	September 2006
629	August 2006
471	July 2006
727	June 2006
569	May 2006
519	April 2006
812	March 2006
796	February 2006
711	January 2006
539	December 2005
647	November 2005
743	October 2005
1018	September 2005
825	August 2005
484	July 2005
552	June 2005
559	May 2005
593	April 2005
614	March 2005
492	February 2005
499	January 2005
297	December 2004
441	November 2004
729	October 2004
734	September 2004
615	August 2004
294	July 2004
642	June 2004
460	May 2004
552	April 2004
472	March 2004
592	February 2004
653	January 2004
325	December 2003
541	November 2003
426	October 2003
395	September 2003
400	August 2003
304	July 2003
361	June 2003
438	May 2003
349	April 2003
362	March 2003
302	February 2003
403	January 2003
250	December 2002
417	November 2002
465	October 2002
290	September 2002
217	August 2002
226	July 2002
257	June 2002
133	May 2002
142	April 2002

Design

Zawodny | Right Menu | Left Menu

Your Own Design

Paste the URL of your CSS below. Download CSS template