Change SG_ALL to mean "any size"

PATCH In review Boaz Harrosh 4 participants 23 messages Jan 17, 2008 – Feb 10, 2008

Subsystems (14)

SCSI PowerPC QLogic QLA2XXX FC-SCSI driver Firewire +10 more subsystems

lore

Share & tools

lore

Boaz Harrosh

author

2008-01-17 16:37 UTC #0 lore raw

cover Change SG_ALL to mean "any size"

I have scanned through the scsi tree and checked all places that used SG_ALL. Those drivers that where not ready for SG_ALL to mean "any size" where changed to what ever is appropriate for this driver. Mainly 3 groups.

Those that meant SG_ALL == 255:

[PATCH 1/8] firewire: ieee1394: Move away from SG_ALL [PATCH 2/8] scsi-drivers Don't use SG_ALL as allocation size

Those who are not sg-chaining ready:

[PATCH 3/8] NCR5380: Not sg-chain ready [PATCH 4/8] wd33c93: Not sg-chain ready [PATCH 5/8] arm/scsi: Not sg-chain ready [PATCH 6/8] scsi: Drivers not ready for sg-chaining

And those that just got confused:
[PATCH 7/8] a100u2w: advansys: initio: Wrong use of SG_ALL

And finally the switch of SG_ALL to ~0, that is MAX_ what ever type it is.
[PATCH 8/8] Change SG_ALL to mean "any size"

Please request if you want any patches squashed or split.

These are based on the scsi-bidi git-tree and mainly on top of the: "[SCSI] remove use_sg_chaining" patch. James do you want that I reverse the dependency so your patch will be on top of these patches? (For bisect-ability)

Boaz

Boaz Harrosh

author

2008-01-17 16:42 UTC #1 lore raw

PATCH 1/8 firewire: ieee1394: Move away from SG_ALL

SG_ALL wants to be ~0 meaning "any future size". Below group of drivers preallocate a scatter list buffer of max_size, so set that size to be 255 (Like before).

A better schema can be advised with a more dynamic allocation. Perhaps from a kmem_cache.

List of drivers/files:

	drivers/firewire/fw-sbp2.c
	drivers/ieee1394/sbp2.[ch]

Signed-off-by Boaz Harrosh <[email protected]>

---
 drivers/firewire/fw-sbp2.c |    6 ++++--
 drivers/ieee1394/sbp2.c    |    2 +-
 drivers/ieee1394/sbp2.h    |    4 +++-
 3 files changed, 8 insertions(+), 4 deletions(-)

drivers/firewire/fw-sbp2.c +4 -2

index c2169d2..078501c 100644
@@ -151,6 +151,7 @@ struct sbp2_target {
 };
 
 #define SBP2_MAX_SG_ELEMENT_LENGTH	0xf000
+#define SBP2_MAX_SG_COUNT		255
 #define SBP2_MAX_SECTORS		255	/* Max sectors supported */
 #define SBP2_ORB_TIMEOUT		2000	/* Timeout in ms */

@@ -272,7 +273,8 @@ struct sbp2_command_orb {
 	scsi_done_fn_t done;
 	struct sbp2_logical_unit *lu;
 
-	struct sbp2_pointer page_table[SG_ALL] __attribute__((aligned(8)));
+	struct sbp2_pointer page_table[SBP2_MAX_SG_COUNT]
+						__attribute__((aligned(8)));
 	dma_addr_t page_table_bus;
 };

@@ -1329,7 +1331,7 @@ static struct scsi_host_template scsi_driver_template = {
 	.slave_configure	= sbp2_scsi_slave_configure,
 	.eh_abort_handler	= sbp2_scsi_abort,
 	.this_id		= -1,
-	.sg_tablesize		= SG_ALL,
+	.sg_tablesize		= SBP2_MAX_SG_COUNT,
 	.use_clustering		= ENABLE_CLUSTERING,
 	.cmd_per_lun		= 1,
 	.can_queue		= 1,

drivers/ieee1394/sbp2.c +1 -1

index 1eda11a..070763a 100644
@@ -324,7 +324,7 @@ static struct scsi_host_template sbp2_shost_template = {
 	.slave_configure	 = sbp2scsi_slave_configure,
 	.slave_destroy		 = sbp2scsi_slave_destroy,
 	.this_id		 = -1,
-	.sg_tablesize		 = SG_ALL,
+	.sg_tablesize		 = SBP2_MAX_SG_COUNT,
 	.use_clustering		 = ENABLE_CLUSTERING,
 	.cmd_per_lun		 = SBP2_MAX_CMDS,
 	.can_queue		 = SBP2_MAX_CMDS,

drivers/ieee1394/sbp2.h +3 -1

index 333a4bb..ae31788 100644
@@ -222,6 +222,7 @@ struct sbp2_status_block {
  */
 
 #define SBP2_MAX_SG_ELEMENT_LENGTH		0xf000
+#define SBP2_MAX_SG_COUNT			255
 #define SBP2_MAX_SECTORS			255
 /* There is no real limitation of the queue depth (i.e. length of the linked
  * list of command ORBs) at the target. The chosen depth is merely an

@@ -257,7 +258,8 @@ struct sbp2_command_info {
 
 	/* Also need s/g structure for each sbp2 command */
 	struct sbp2_unrestricted_page_table
-		scatter_gather_element[SG_ALL] __attribute__((aligned(8)));
+		scatter_gather_element[SBP2_MAX_SG_COUNT]
+						__attribute__((aligned(8)));
 	dma_addr_t sge_dma;
 	void *sge_buffer;
 	dma_addr_t cmd_dma;

Signature · 1 line

1.5.3.3

Stefan Richter

Acked-by on v1 1/8

2008-01-17 17:52 UTC #2 lore raw

Re: [PATCH 1/8] firewire: ieee1394: Move away from SG_ALL

Quoted 4 lines · Boaz Harrosh

SG_ALL wants to be ~0 meaning "any future size". Below group of drivers preallocate a scatter list buffer of max_size, so set that size to be 255 (Like before).

Quoted 8 lines

A better schema can be advised with a more dynamic allocation. Perhaps from a kmem_cache.
List of drivers/files:
	drivers/firewire/fw-sbp2.c
	drivers/ieee1394/sbp2.[ch]
Signed-off-by Boaz Harrosh <[email protected]>

Acked-by Stefan Richter <[email protected]>

Quoted 5 lines

---
 drivers/firewire/fw-sbp2.c |    6 ++++--
 drivers/ieee1394/sbp2.c    |    2 +-
 drivers/ieee1394/sbp2.h    |    4 +++-
 3 files changed, 8 insertions(+), 4 deletions(-)

Since I wasn't Cc'd, I suppose you don't want me to take it upstream via linux1394-2.6.git?

My guess is that there won't be conflicts of this with linux1394-2.6.git anytime soon, so taking it into any other tree should be fine.

Signature · 3 lines

Stefan Richter
-=====-==--- ---= =---=
http://arcgraph.de/sr/

Stefan Richter

2008-01-19 07:59 UTC #3 lore raw

Re: [PATCH 1/8] firewire: ieee1394: Move away from SG_ALL

Quoted 2 lines · Stefan Richter

My guess is that there won't be conflicts of this with linux1394-2.6.git anytime soon, so taking it into any other tree should be fine.

Wrong, there should already be a (trivial) conflict due to a patch from December. Another (trivially) conflicting patch was sent to linux1394-devel a few hours ago. Both patches will land in Linus' tree early during the next merge window. So either refresh the patch when it's time, or give it to me. (Whatever, try not to add to akpm's pain.)

Signature · 3 lines

Stefan Richter
-=====-==--- ---= =--==
http://arcgraph.de/sr/

James Bottomley

maintainer

2008-01-19 15:01 UTC #4 lore raw

Re: [PATCH 1/8] firewire: ieee1394: Move away from SG_ALL

Quoted 9 lines · Stefan Richter

Quoted 2 lines · Stefan Richter
My guess is that there won't be conflicts of this with linux1394-2.6.git anytime soon, so taking it into any other tree should be fine.
Wrong, there should already be a (trivial) conflict due to a patch from December. Another (trivially) conflicting patch was sent to linux1394-devel a few hours ago. Both patches will land in Linus' tree early during the next merge window. So either refresh the patch when it's time, or give it to me. (Whatever, try not to add to akpm's pain.)

Actually, I'm already tracking potential conflicts. The linux1394-2.6.git is part of the merge candidate tree I'm building nightly. These patches will go through the (now slightly misnamed) scsi-bidi-2.6.git tree. That one's built on top of all the currently conflicting trees, and is scheduled to be the last thing merged in the merge window (or at least to wait until the others its built on have gone in).

James

Stefan Richter

2008-01-19 15:18 UTC #5 lore raw

Re: [PATCH 1/8] firewire: ieee1394: Move away from SG_ALL

Quoted 3 lines · James Bottomley

Actually, I'm already tracking potential conflicts. The linux1394-2.6.git is part of the merge candidate tree I'm building nightly.

Right, I saw your announcement of this tree.

Quoted 5 lines

These patches will go through the (now slightly misnamed) scsi-bidi-2.6.git tree. That one's built on top of all the currently conflicting trees, and is scheduled to be the last thing merged in the merge window (or at least to wait until the others its built on have gone in).

Good, everything should come together nicely then.

Signature · 3 lines

Stefan Richter
-=====-==--- ---= =--==
http://arcgraph.de/sr/

Boaz Harrosh

author

2008-01-17 16:44 UTC #6 lore raw

PATCH 2/8 scsi-drivers Don't use SG_ALL as allocation size

In reply to Boaz Harrosh

below list of drivers have used SG_ALL as a size to preallocate maximum possible command's sg-count. This is no longer possible since the maximum is not set at compile time but as a run time configuration.

A better schema can be advised with a more dynamic allocation. Perhaps from a kmem_cache.

Affected drivers/files:

	drivers/scsi/atari_scsi.[ch]
	drivers/scsi/eata_pio.c
	drivers/scsi/ibmvscsi/ibmvscsi.[ch]
	drivers/scsi/mac53c94.c
	drivers/scsi/mesh.c
	drivers/scsi/nsp32.h
	drivers/scsi/qla1280.c
	drivers/scsi/qla2xxx/qla_os.c
	drivers/scsi/qla4xxx/ql4_def.h

Signed-off-by Boaz Harrosh <[email protected]>

---
 drivers/scsi/atari_scsi.c        |    8 ++++----
 drivers/scsi/atari_scsi.h        |    4 ++--
 drivers/scsi/eata_pio.c          |    5 ++++-
 drivers/scsi/ibmvscsi/ibmvscsi.c |    8 +++++---
 drivers/scsi/ibmvscsi/ibmvscsi.h |    1 +
 drivers/scsi/mac53c94.c          |    8 ++++++--
 drivers/scsi/mesh.c              |    5 ++++-
 drivers/scsi/nsp32.h             |    2 +-
 drivers/scsi/qla1280.c           |    3 ++-
 drivers/scsi/qla2xxx/qla_os.c    |    2 ++
 drivers/scsi/qla4xxx/ql4_def.h   |    1 +
 11 files changed, 32 insertions(+), 15 deletions(-)

drivers/scsi/atari_scsi.c +4 -4

index f5732d8..b47b61d 100644
@@ -620,7 +620,8 @@ int atari_scsi_detect(struct scsi_host_template *host)
 	/* Force sg_tablesize to 0 on a Falcon! */
 	host->sg_tablesize =
 		!IS_A_TT() ? ATARI_FALCON_SG_TABLESIZE :
-		(setup_sg_tablesize >= 0) ? setup_sg_tablesize : ATARI_TT_SG_TABLESIZE;
+		(setup_sg_tablesize > 0) ? setup_sg_tablesize :
+							ATARI_TT_SG_TABLESIZE;
 
 	if (setup_hostid >= 0)
 		host->this_id = setup_hostid;

@@ -792,9 +793,8 @@ void __init atari_scsi_setup(char *str, int *ints)
 	if (ints[0] >= 3) {
 		if (ints[3] >= 0) {
 			setup_sg_tablesize = ints[3];
-			/* Must be <= SG_ALL (255) */
-			if (setup_sg_tablesize > SG_ALL)
-				setup_sg_tablesize = SG_ALL;
+			if (setup_sg_tablesize > ATARI_TT_SG_TABLESIZE)
+				setup_sg_tablesize = ATARI_TT_SG_TABLESIZE;
 		}
 	}
 	if (ints[0] >= 4) {

drivers/scsi/atari_scsi.h +2 -2

index efadb8d..21b8c91 100644
@@ -35,11 +35,11 @@ int atari_scsi_release (struct Scsi_Host *);
 
 #define ATARI_TT_CAN_QUEUE		16
 #define ATARI_TT_CMD_PER_LUN		8
-#define ATARI_TT_SG_TABLESIZE		SG_ALL
+#define ATARI_TT_SG_TABLESIZE		255
 
 #define ATARI_FALCON_CAN_QUEUE		8
 #define ATARI_FALCON_CMD_PER_LUN	1
-#define ATARI_FALCON_SG_TABLESIZE	SG_NONE
+#define ATARI_FALCON_SG_TABLESIZE	1
 
 #define	DEFAULT_USE_TAGGED_QUEUING	0

drivers/scsi/eata_pio.c +4 -1

index b5a6092..8913d9d 100644
@@ -691,6 +691,9 @@ static unsigned int print_selftest(unsigned int base)
 	return (!(inb(base + HA_RSTATUS) & HA_SERROR));
 }
 
+/* FIXME: is this driver sg-chainning ready? */
+#define EATA_MAX_SG 255
+
 static int register_pio_HBA(long base, struct get_conf *gc, struct pci_dev *pdev)
 {
 	unsigned long size = 0;

@@ -801,7 +804,7 @@ static int register_pio_HBA(long base, struct get_conf *gc, struct pci_dev *pdev
 	sh->this_id = gc->scsi_id[3];
 	sh->can_queue = 1;
 	sh->cmd_per_lun = 1;
-	sh->sg_tablesize = SG_ALL;
+	sh->sg_tablesize = EATA_MAX_SG;
 
 	hd->channel = 0;

drivers/scsi/ibmvscsi/ibmvscsi.c +5 -3

index 78d46a9..9b1fc94 100644
@@ -173,7 +173,8 @@ static void release_event_pool(struct event_pool *pool,
 			++in_use;
 		if (pool->events[i].ext_list) {
 			dma_free_coherent(hostdata->dev,
-				  SG_ALL * sizeof(struct srp_direct_buf),
+				  IBMVSCSI_MAX_SG *
+						sizeof(struct srp_direct_buf),
 				  pool->events[i].ext_list,
 				  pool->events[i].ext_list_token);
 		}

@@ -423,7 +424,8 @@ static int map_sg_data(struct scsi_cmnd *cmd,
 	if (!evt_struct->ext_list) {
 		evt_struct->ext_list = (struct srp_direct_buf *)
 			dma_alloc_coherent(dev,
-					   SG_ALL * sizeof(struct srp_direct_buf),
+					   IBMVSCSI_MAX_SG *
+						sizeof(struct srp_direct_buf),
 					   &evt_struct->ext_list_token, 0);
 		if (!evt_struct->ext_list) {
 			sdev_printk(KERN_ERR, cmd->device,

@@ -1598,7 +1600,7 @@ static struct scsi_host_template driver_template = {
 	.cmd_per_lun = 16,
 	.can_queue = IBMVSCSI_MAX_REQUESTS_DEFAULT,
 	.this_id = -1,
-	.sg_tablesize = SG_ALL,
+	.sg_tablesize = IBMVSCSI_MAX_SG,
 	.use_clustering = ENABLE_CLUSTERING,
 	.shost_attrs = ibmvscsi_attrs,
 };

drivers/scsi/ibmvscsi/ibmvscsi.h +1 -0

index 46e850e..e3e3a2e 100644
@@ -46,6 +46,7 @@ struct Scsi_Host;
 
 #define IBMVSCSI_MAX_REQUESTS_DEFAULT 100
 #define IBMVSCSI_MAX_CMDS_PER_LUN 64
+#define IBMVSCSI_MAX_SG 255
 
 /* ------------------------------------------------------------
  * Data Structures

drivers/scsi/mac53c94.c +6 -2

index b12ad7c..9ef0272 100644
@@ -392,6 +392,9 @@ static void set_dma_cmds(struct fsc_state *state, struct scsi_cmnd *cmd)
 	cmd->SCp.this_residual = total;
 }
 
+/* Dynamicaly allocated at init */
+#define MAC53c94_MAX_SG 255
+
 static struct scsi_host_template mac53c94_template = {
 	.proc_name	= "53c94",
 	.name		= "53C94",

@@ -399,7 +402,7 @@ static struct scsi_host_template mac53c94_template = {
 	.eh_host_reset_handler = mac53c94_host_reset,
 	.can_queue	= 1,
 	.this_id	= 7,
-	.sg_tablesize	= SG_ALL,
+	.sg_tablesize	= MAC53c94_MAX_SG,
 	.cmd_per_lun	= 1,
 	.use_clustering	= DISABLE_CLUSTERING,
 };

@@ -464,7 +467,8 @@ static int mac53c94_probe(struct macio_dev *mdev, const struct of_device_id *mat
 	 * XXX FIXME: Use DMA consistent routines
 	 */
        	dma_cmd_space = kmalloc((host->sg_tablesize + 2) *
-       				sizeof(struct dbdma_cmd), GFP_KERNEL);
+				sizeof(struct dbdma_cmd),
+				GFP_KERNEL | GFP_DMA);
        	if (dma_cmd_space == 0) {
        		printk(KERN_ERR "mac53c94: couldn't allocate dma "
        		       "command space for %s\n", node->full_name);

drivers/scsi/mesh.c +4 -1

index 651d09b..e4e151d 100644
@@ -1832,6 +1832,9 @@ static int mesh_shutdown(struct macio_dev *mdev)
 	return 0;
 }
 
+/* allocated at probe time */
+#define MESH_MAX_SG 255 
+
 static struct scsi_host_template mesh_template = {
 	.proc_name			= "mesh",
 	.name				= "MESH",

@@ -1840,7 +1843,7 @@ static struct scsi_host_template mesh_template = {
 	.eh_host_reset_handler		= mesh_host_reset,
 	.can_queue			= 20,
 	.this_id			= 7,
-	.sg_tablesize			= SG_ALL,
+	.sg_tablesize			= MESH_MAX_SG,
 	.cmd_per_lun			= 2,
 	.use_clustering			= DISABLE_CLUSTERING,
 };

drivers/scsi/nsp32.h +1 -1

index 6715ecb..ace52f5 100644
@@ -444,7 +444,7 @@ typedef u16 u16_le;
 /************************************************************************
  * structure for DMA/Scatter Gather list
  */
-#define NSP32_SG_SIZE		SG_ALL
+#define NSP32_SG_SIZE		255
 
 typedef struct _nsp32_sgtable {
 	/* values must be little endian */

drivers/scsi/qla1280.c +2 -1

index 68c0d09..c0319da 100644
@@ -422,6 +422,7 @@
 #define IS_ISP1x160(ha)        (ha->pdev->device == PCI_DEVICE_ID_QLOGIC_ISP10160 || \
 				ha->pdev->device == PCI_DEVICE_ID_QLOGIC_ISP12160)
 
+#define QLA_MAX_SG 255
 
 static int qla1280_probe_one(struct pci_dev *, const struct pci_device_id *);
 static void qla1280_remove_one(struct pci_dev *);

@@ -4201,7 +4202,7 @@ static struct scsi_host_template qla1280_driver_template = {
 	.bios_param		= qla1280_biosparam,
 	.can_queue		= 0xfffff,
 	.this_id		= -1,
-	.sg_tablesize		= SG_ALL,
+	.sg_tablesize		= QLA_MAX_SG,
 	.cmd_per_lun		= 1,
 	.use_clustering		= ENABLE_CLUSTERING,
 };

drivers/scsi/qla2xxx/qla_os.c +2 -0

index 78d1103..e368f38 100644
@@ -111,6 +111,8 @@ static int qla2x00_device_reset(scsi_qla_host_t *, fc_port_t *);
 static int qla2x00_change_queue_depth(struct scsi_device *, int);
 static int qla2x00_change_queue_type(struct scsi_device *, int);
 
+#define QLA2XXX_MAX_SG 255
+
 struct scsi_host_template qla2x00_driver_template = {
 	.module			= THIS_MODULE,
 	.name			= QLA2XXX_DRIVER_NAME,

drivers/scsi/qla4xxx/ql4_def.h +1 -0

index accaf69..64cd43b 100644
@@ -101,6 +101,7 @@
 #define MBOX_AEN_REG_COUNT	5
 #define MAX_INIT_RETRIES	5
 #define IOCB_HIWAT_CUSHION	16
+#define QLA_MAX_SG		255
 
 /*
  * Buffer sizes

Signature · 1 line

1.5.3.3

Andrew Vasquez

2008-01-17 17:47 UTC #7 lore raw

Re: [PATCH 2/8] scsi-drivers Don't use SG_ALL as allocation size

Quoted 9 lines · Boaz Harrosh

below list of drivers have used SG_ALL as a size to preallocate maximum possible command's sg-count. This is no longer possible since the maximum is not set at compile time but as a run time configuration.
A better schema can be advised with a more dynamic allocation. Perhaps from a kmem_cache.
Affected drivers/files:

Quoted 9 lines

	drivers/scsi/atari_scsi.[ch]
	drivers/scsi/eata_pio.c
	drivers/scsi/ibmvscsi/ibmvscsi.[ch]
	drivers/scsi/mac53c94.c
	drivers/scsi/mesh.c
	drivers/scsi/nsp32.h
	drivers/scsi/qla1280.c
	drivers/scsi/qla2xxx/qla_os.c
	drivers/scsi/qla4xxx/ql4_def.h

...

There's no functional change in your patches to qla2xxx and qla4xxx. Perhaps a cut-n-paste typo from qla1280.c (which should be capped):

Quoted 18 lines

drivers/scsi/qla2xxx/qla_os.c +2 -0

index 78d1103..e368f38 100644
@@ -111,6 +111,8 @@ static int qla2x00_device_reset(scsi_qla_host_t *, fc_port_t *);
 static int qla2x00_change_queue_depth(struct scsi_device *, int);
 static int qla2x00_change_queue_type(struct scsi_device *, int);
 
+#define QLA2XXX_MAX_SG 255
+
 struct scsi_host_template qla2x00_driver_template = {
 	.module			= THIS_MODULE,
 	.name			= QLA2XXX_DRIVER_NAME,

drivers/scsi/qla4xxx/ql4_def.h +0 -0

index accaf69..64cd43b 100644

drivers/scsi/qla4xxx/ql4_def.h line 104

Quoted 4 lines

drivers/scsi/qla4xxx/ql4_def.h +1 -0

@@ -101,6 +101,7 @@
 #define MBOX_AEN_REG_COUNT	5
 #define MAX_INIT_RETRIES	5
 #define IOCB_HIWAT_CUSHION	16
+#define QLA_MAX_SG		255

I don't forsee any issues with maintaining SG_ALL (~0) usage within qla2xxx and qla4xxx, as the number of unused entries on the HBA's request-queue shall ultimately be the rate-limiting factor.

--
av

Boaz Harrosh

author

2008-01-17 18:12 UTC #8 lore raw

Re: [PATCH 2/8] scsi-drivers Don't use SG_ALL as allocation size

Quoted 53 lines · Andrew Vasquez

Quoted 9 lines · Boaz Harrosh
below list of drivers have used SG_ALL as a size to preallocate maximum possible command's sg-count. This is no longer possible since the maximum is not set at compile time but as a run time configuration.
A better schema can be advised with a more dynamic allocation. Perhaps from a kmem_cache.
Affected drivers/files:
Quoted 9 lines
	drivers/scsi/atari_scsi.[ch]
	drivers/scsi/eata_pio.c
	drivers/scsi/ibmvscsi/ibmvscsi.[ch]
	drivers/scsi/mac53c94.c
	drivers/scsi/mesh.c
	drivers/scsi/nsp32.h
	drivers/scsi/qla1280.c
	drivers/scsi/qla2xxx/qla_os.c
	drivers/scsi/qla4xxx/ql4_def.h
...
There's no functional change in your patches to qla2xxx and qla4xxx. Perhaps a cut-n-paste typo from qla1280.c (which should be capped):
Quoted 18 lines
drivers/scsi/qla2xxx/qla_os.c +2 -0
index 78d1103..e368f38 100644
@@ -111,6 +111,8 @@ static int qla2x00_device_reset(scsi_qla_host_t *, fc_port_t *);
 static int qla2x00_change_queue_depth(struct scsi_device *, int);
 static int qla2x00_change_queue_type(struct scsi_device *, int);
 
+#define QLA2XXX_MAX_SG 255
+
 struct scsi_host_template qla2x00_driver_template = {
 	.module			= THIS_MODULE,
 	.name			= QLA2XXX_DRIVER_NAME,
drivers/scsi/qla4xxx/ql4_def.h +0 -0
index accaf69..64cd43b 100644
drivers/scsi/qla4xxx/ql4_def.h line 104
Quoted 4 lines
drivers/scsi/qla4xxx/ql4_def.h +1 -0
@@ -101,6 +101,7 @@
 #define MBOX_AEN_REG_COUNT	5
 #define MAX_INIT_RETRIES	5
 #define IOCB_HIWAT_CUSHION	16
+#define QLA_MAX_SG		255
I don't forsee any issues with maintaining SG_ALL (~0) usage within qla2xxx and qla4xxx, as the number of unused entries on the HBA's request-queue shall ultimately be the rate-limiting factor.

Quoted 1 line

--

Please forgive me I have forgot to put the use of this constants at body, see patch below.

I have seen the use of a device memory at the scsi_for_each_sg loop and was not at all sure what is the size limit for that. So I played it safe, actually I changed nothing. Please give me the OK to remove ql[24]xxx from these patches, and have them change to SG_ALL==~0 in the last patch. Or should I resend the patch with below added.

Thanks for catching this
Boaz

drivers/scsi/qla2xxx/qla_os.c +2 -2

index e368f38..b0999b6 100644
@@ -134,7 +134,7 @@ struct scsi_host_template qla2x00_driver_template = {
 	.this_id		= -1,
 	.cmd_per_lun		= 3,
 	.use_clustering		= ENABLE_CLUSTERING,
-	.sg_tablesize		= SG_ALL,
+	.sg_tablesize		= QLA2XXX_MAX_SG,
 
 	/*
 	 * The RISC allows for each command to transfer (2^32-1) bytes of data,

@@ -165,7 +165,7 @@ struct scsi_host_template qla24xx_driver_template = {
 	.this_id		= -1,
 	.cmd_per_lun		= 3,
 	.use_clustering		= ENABLE_CLUSTERING,
-	.sg_tablesize		= SG_ALL,
+	.sg_tablesize		= QLA2XXX_MAX_SG,
 
 	.max_sectors		= 0xFFFF,
 	.shost_attrs		= qla2x00_host_attrs,

drivers/scsi/qla4xxx/ql4_os.c +1 -1

index b128da5..0b8de79 100644
@@ -94,7 +94,7 @@ static struct scsi_host_template qla4xxx_driver_template = {
 	.this_id		= -1,
 	.cmd_per_lun		= 3,
 	.use_clustering		= ENABLE_CLUSTERING,
-	.sg_tablesize		= SG_ALL,
+	.sg_tablesize		= QLA_MAX_SG,
 
 	.max_sectors		= 0xFFFF,
 };

Andrew Vasquez

2008-01-17 18:31 UTC #9 lore raw

Re: [PATCH 2/8] scsi-drivers Don't use SG_ALL as allocation size

Quoted 4 lines · Boaz Harrosh

Quoted 4 lines
I don't forsee any issues with maintaining SG_ALL (~0) usage within qla2xxx and qla4xxx, as the number of unused entries on the HBA's request-queue shall ultimately be the rate-limiting factor.

Quoted 8 lines

Quoted 1 line
--
Please forgive me I have forgot to put the use of this constants at body, see patch below.
I have seen the use of a device memory at the scsi_for_each_sg loop and was not at all sure what is the size limit for that. So I played it safe, actually I changed nothing. Please give me the OK to remove ql[24]xxx from these patches,

I believe that's best, simply remove the qla2xxx/qla4xxx component from your patchset.

Thanks, AV

Boaz Harrosh

author

2008-01-17 18:57 UTC #10 lore raw

Re: [PATCH 2/8 ver2] scsi-drivers Don't use SG_ALL as allocation size

In reply to Boaz Harrosh

Subject: [PATCH] scsi-drivers Don't use SG_ALL as allocation size

A better schema can be advised with a more dynamic allocation. Perhaps from a kmem_cache.

Affected drivers/files:

	drivers/scsi/atari_scsi.[ch]
	drivers/scsi/eata_pio.c
	drivers/scsi/ibmvscsi/ibmvscsi.[ch]
	drivers/scsi/mac53c94.c
	drivers/scsi/mesh.c
	drivers/scsi/nsp32.h
	drivers/scsi/qla1280.c

Signed-off-by Boaz Harrosh <[email protected]>

---
 drivers/scsi/atari_scsi.c        |    8 ++++----
 drivers/scsi/atari_scsi.h        |    4 ++--
 drivers/scsi/eata_pio.c          |    5 ++++-
 drivers/scsi/ibmvscsi/ibmvscsi.c |    8 +++++---
 drivers/scsi/ibmvscsi/ibmvscsi.h |    1 +
 drivers/scsi/mac53c94.c          |    8 ++++++--
 drivers/scsi/mesh.c              |    5 ++++-
 drivers/scsi/nsp32.h             |    2 +-
 drivers/scsi/qla1280.c           |    3 ++-
 9 files changed, 29 insertions(+), 15 deletions(-)

drivers/scsi/atari_scsi.c +4 -4

index f5732d8..b47b61d 100644
@@ -620,7 +620,8 @@ int atari_scsi_detect(struct scsi_host_template *host)
 	/* Force sg_tablesize to 0 on a Falcon! */
 	host->sg_tablesize =
 		!IS_A_TT() ? ATARI_FALCON_SG_TABLESIZE :
-		(setup_sg_tablesize >= 0) ? setup_sg_tablesize : ATARI_TT_SG_TABLESIZE;
+		(setup_sg_tablesize > 0) ? setup_sg_tablesize :
+							ATARI_TT_SG_TABLESIZE;
 
 	if (setup_hostid >= 0)
 		host->this_id = setup_hostid;

@@ -792,9 +793,8 @@ void __init atari_scsi_setup(char *str, int *ints)
 	if (ints[0] >= 3) {
 		if (ints[3] >= 0) {
 			setup_sg_tablesize = ints[3];
-			/* Must be <= SG_ALL (255) */
-			if (setup_sg_tablesize > SG_ALL)
-				setup_sg_tablesize = SG_ALL;
+			if (setup_sg_tablesize > ATARI_TT_SG_TABLESIZE)
+				setup_sg_tablesize = ATARI_TT_SG_TABLESIZE;
 		}
 	}
 	if (ints[0] >= 4) {

drivers/scsi/atari_scsi.h +2 -2

index efadb8d..21b8c91 100644
@@ -35,11 +35,11 @@ int atari_scsi_release (struct Scsi_Host *);
 
 #define ATARI_TT_CAN_QUEUE		16
 #define ATARI_TT_CMD_PER_LUN		8
-#define ATARI_TT_SG_TABLESIZE		SG_ALL
+#define ATARI_TT_SG_TABLESIZE		255
 
 #define ATARI_FALCON_CAN_QUEUE		8
 #define ATARI_FALCON_CMD_PER_LUN	1
-#define ATARI_FALCON_SG_TABLESIZE	SG_NONE
+#define ATARI_FALCON_SG_TABLESIZE	1
 
 #define	DEFAULT_USE_TAGGED_QUEUING	0

drivers/scsi/eata_pio.c +4 -1

index b5a6092..8913d9d 100644
@@ -691,6 +691,9 @@ static unsigned int print_selftest(unsigned int base)
 	return (!(inb(base + HA_RSTATUS) & HA_SERROR));
 }
 
+/* FIXME: is this driver sg-chainning ready? */
+#define EATA_MAX_SG 255
+
 static int register_pio_HBA(long base, struct get_conf *gc, struct pci_dev *pdev)
 {
 	unsigned long size = 0;

@@ -801,7 +804,7 @@ static int register_pio_HBA(long base, struct get_conf *gc, struct pci_dev *pdev
 	sh->this_id = gc->scsi_id[3];
 	sh->can_queue = 1;
 	sh->cmd_per_lun = 1;
-	sh->sg_tablesize = SG_ALL;
+	sh->sg_tablesize = EATA_MAX_SG;
 
 	hd->channel = 0;

drivers/scsi/ibmvscsi/ibmvscsi.c +5 -3

index 78d46a9..9b1fc94 100644
@@ -173,7 +173,8 @@ static void release_event_pool(struct event_pool *pool,
 			++in_use;
 		if (pool->events[i].ext_list) {
 			dma_free_coherent(hostdata->dev,
-				  SG_ALL * sizeof(struct srp_direct_buf),
+				  IBMVSCSI_MAX_SG *
+						sizeof(struct srp_direct_buf),
 				  pool->events[i].ext_list,
 				  pool->events[i].ext_list_token);
 		}

@@ -423,7 +424,8 @@ static int map_sg_data(struct scsi_cmnd *cmd,
 	if (!evt_struct->ext_list) {
 		evt_struct->ext_list = (struct srp_direct_buf *)
 			dma_alloc_coherent(dev,
-					   SG_ALL * sizeof(struct srp_direct_buf),
+					   IBMVSCSI_MAX_SG *
+						sizeof(struct srp_direct_buf),
 					   &evt_struct->ext_list_token, 0);
 		if (!evt_struct->ext_list) {
 			sdev_printk(KERN_ERR, cmd->device,

@@ -1598,7 +1600,7 @@ static struct scsi_host_template driver_template = {
 	.cmd_per_lun = 16,
 	.can_queue = IBMVSCSI_MAX_REQUESTS_DEFAULT,
 	.this_id = -1,
-	.sg_tablesize = SG_ALL,
+	.sg_tablesize = IBMVSCSI_MAX_SG,
 	.use_clustering = ENABLE_CLUSTERING,
 	.shost_attrs = ibmvscsi_attrs,
 };

drivers/scsi/ibmvscsi/ibmvscsi.h +1 -0

index 46e850e..e3e3a2e 100644
@@ -46,6 +46,7 @@ struct Scsi_Host;
 
 #define IBMVSCSI_MAX_REQUESTS_DEFAULT 100
 #define IBMVSCSI_MAX_CMDS_PER_LUN 64
+#define IBMVSCSI_MAX_SG 255
 
 /* ------------------------------------------------------------
  * Data Structures

drivers/scsi/mac53c94.c +6 -2

index b12ad7c..9ef0272 100644
@@ -392,6 +392,9 @@ static void set_dma_cmds(struct fsc_state *state, struct scsi_cmnd *cmd)
 	cmd->SCp.this_residual = total;
 }
 
+/* Dynamicaly allocated at init */
+#define MAC53c94_MAX_SG 255
+
 static struct scsi_host_template mac53c94_template = {
 	.proc_name	= "53c94",
 	.name		= "53C94",

@@ -399,7 +402,7 @@ static struct scsi_host_template mac53c94_template = {
 	.eh_host_reset_handler = mac53c94_host_reset,
 	.can_queue	= 1,
 	.this_id	= 7,
-	.sg_tablesize	= SG_ALL,
+	.sg_tablesize	= MAC53c94_MAX_SG,
 	.cmd_per_lun	= 1,
 	.use_clustering	= DISABLE_CLUSTERING,
 };

@@ -464,7 +467,8 @@ static int mac53c94_probe(struct macio_dev *mdev, const struct of_device_id *mat
 	 * XXX FIXME: Use DMA consistent routines
 	 */
        	dma_cmd_space = kmalloc((host->sg_tablesize + 2) *
-       				sizeof(struct dbdma_cmd), GFP_KERNEL);
+				sizeof(struct dbdma_cmd),
+				GFP_KERNEL | GFP_DMA);
        	if (dma_cmd_space == 0) {
        		printk(KERN_ERR "mac53c94: couldn't allocate dma "
        		       "command space for %s\n", node->full_name);

drivers/scsi/mesh.c +4 -1

index 651d09b..2a8654a 100644
@@ -1832,6 +1832,9 @@ static int mesh_shutdown(struct macio_dev *mdev)
 	return 0;
 }
 
+/* allocated at probe time */
+#define MESH_MAX_SG 255
+
 static struct scsi_host_template mesh_template = {
 	.proc_name			= "mesh",
 	.name				= "MESH",

@@ -1840,7 +1843,7 @@ static struct scsi_host_template mesh_template = {
 	.eh_host_reset_handler		= mesh_host_reset,
 	.can_queue			= 20,
 	.this_id			= 7,
-	.sg_tablesize			= SG_ALL,
+	.sg_tablesize			= MESH_MAX_SG,
 	.cmd_per_lun			= 2,
 	.use_clustering			= DISABLE_CLUSTERING,
 };

drivers/scsi/nsp32.h +1 -1

index 6715ecb..ace52f5 100644
@@ -444,7 +444,7 @@ typedef u16 u16_le;
 /************************************************************************
  * structure for DMA/Scatter Gather list
  */
-#define NSP32_SG_SIZE		SG_ALL
+#define NSP32_SG_SIZE		255
 
 typedef struct _nsp32_sgtable {
 	/* values must be little endian */

drivers/scsi/qla1280.c +2 -1

index 68c0d09..c0319da 100644
@@ -422,6 +422,7 @@
 #define IS_ISP1x160(ha)        (ha->pdev->device == PCI_DEVICE_ID_QLOGIC_ISP10160 || \
 				ha->pdev->device == PCI_DEVICE_ID_QLOGIC_ISP12160)
 
+#define QLA_MAX_SG 255
 
 static int qla1280_probe_one(struct pci_dev *, const struct pci_device_id *);
 static void qla1280_remove_one(struct pci_dev *);

@@ -4201,7 +4202,7 @@ static struct scsi_host_template qla1280_driver_template = {
 	.bios_param		= qla1280_biosparam,
 	.can_queue		= 0xfffff,
 	.this_id		= -1,
-	.sg_tablesize		= SG_ALL,
+	.sg_tablesize		= QLA_MAX_SG,
 	.cmd_per_lun		= 1,
 	.use_clustering		= ENABLE_CLUSTERING,
 };

Signature · 1 line

1.5.3.3

Boaz Harrosh

author

2008-01-17 16:46 UTC #11 lore raw

PATCH 3/8 NCR5380: Not sg-chain ready

In reply to Boaz Harrosh

NCR5380 core is not sg-chain ready, so maximum scatterlist supported is SCSI_MAX_SG_SEGMENTS. Define it so, in core, and let all drivers use it for the sg_tablesize in host templates.

Signed-off-by Boaz Harrosh <[email protected]>

---
 drivers/scsi/NCR5380.h       |    3 +++
 drivers/scsi/arm/cumana_1.c  |    2 +-
 drivers/scsi/arm/oak.c       |    2 +-
 drivers/scsi/dmx3191d.c      |    2 +-
 drivers/scsi/dtc.c           |    2 +-
 drivers/scsi/g_NCR5380.c     |    2 +-
 drivers/scsi/mac_scsi.c      |    9 ++++-----
 drivers/scsi/mac_scsi.h      |    4 ----
 drivers/scsi/sun3_scsi.c     |    4 ++--
 drivers/scsi/sun3_scsi.h     |    4 ----
 drivers/scsi/sun3_scsi_vme.c |    4 ++--
 11 files changed, 16 insertions(+), 22 deletions(-)

drivers/scsi/NCR5380.h +3 -0

index bdc468c..cd7e258 100644
@@ -37,6 +37,9 @@
 #define NCR5380_PUBLIC_RELEASE 7
 #define NCR53C400_PUBLIC_RELEASE 2
 
+/* FIXME: Not sg-chain ready */
+#define NCR5380_MAX_SG SCSI_MAX_SG_SEGMENTS
+
 #define NDEBUG_ARBITRATION	0x1
 #define NDEBUG_AUTOSENSE	0x2
 #define NDEBUG_DMA		0x4

drivers/scsi/arm/cumana_1.c +1 -1

index 49d838e..6315730 100644
@@ -220,7 +220,7 @@ static struct scsi_host_template cumanascsi_template = {
 	.eh_bus_reset_handler	= NCR5380_bus_reset,
 	.can_queue		= 16,
 	.this_id		= 7,
-	.sg_tablesize		= SG_ALL,
+	.sg_tablesize		= NCR5380_MAX_SG,
 	.cmd_per_lun		= 2,
 	.unchecked_isa_dma	= 0,
 	.use_clustering		= DISABLE_CLUSTERING,

drivers/scsi/arm/oak.c +1 -1

index 849cdf8..69d9afd 100644
@@ -123,7 +123,7 @@ static struct scsi_host_template oakscsi_template = {
 	.eh_bus_reset_handler	= NCR5380_bus_reset,
 	.can_queue		= 16,
 	.this_id		= 7,
-	.sg_tablesize		= SG_ALL,
+	.sg_tablesize		= NCR5380_MAX_SG,
 	.cmd_per_lun		= 2,
 	.use_clustering		= DISABLE_CLUSTERING,
 	.proc_name		= "oakscsi",

drivers/scsi/dmx3191d.c +1 -1

index fa738ec..92e89b8 100644
@@ -63,7 +63,7 @@ static struct scsi_host_template dmx3191d_driver_template = {
 	.eh_bus_reset_handler	= NCR5380_bus_reset,
 	.can_queue		= 32,
 	.this_id		= 7,
-	.sg_tablesize		= SG_ALL,
+	.sg_tablesize		= NCR5380_MAX_SG,
 	.cmd_per_lun		= 2,
 	.use_clustering		= DISABLE_CLUSTERING,
 };

drivers/scsi/dtc.c +1 -1

index c2677ba..f8e5dd9 100644
@@ -479,7 +479,7 @@ static struct scsi_host_template driver_template = {
 	.bios_param     		= dtc_biosparam,
 	.can_queue      		= CAN_QUEUE,
 	.this_id        		= 7,
-	.sg_tablesize   		= SG_ALL,
+	.sg_tablesize   		= NCR5380_MAX_SG,
 	.cmd_per_lun    		= CMD_PER_LUN,
 	.use_clustering 		= DISABLE_CLUSTERING,
 };

drivers/scsi/g_NCR5380.c +1 -1

index 75585a5..aded3be 100644
@@ -922,7 +922,7 @@ static struct scsi_host_template driver_template = {
 	.bios_param     	= NCR5380_BIOSPARAM,
 	.can_queue      	= CAN_QUEUE,
         .this_id        	= 7,
-        .sg_tablesize   	= SG_ALL,
+        .sg_tablesize   	= NCR5380_MAX_SG,
 	.cmd_per_lun    	= CMD_PER_LUN,
         .use_clustering		= DISABLE_CLUSTERING,
 };

drivers/scsi/mac_scsi.c +4 -5

index 3b09ab2..d8a27c0 100644
@@ -180,9 +180,8 @@ static int __init mac_scsi_setup(char *str) {
 	if (ints[0] >= 3) {
 		if (ints[3] >= 0) {
 			setup_sg_tablesize = ints[3];
-			/* Must be <= SG_ALL (255) */
-			if (setup_sg_tablesize > SG_ALL)
-				setup_sg_tablesize = SG_ALL;
+			if (setup_sg_tablesize > NCR5380_MAX_SG)
+				setup_sg_tablesize = NCR5380_MAX_SG;
 		}
 	}
 	if (ints[0] >= 4) {

@@ -252,7 +251,7 @@ int macscsi_detect(struct scsi_host_template * tpnt)
     tpnt->cmd_per_lun =
 	(setup_cmd_per_lun > 0) ? setup_cmd_per_lun : CMD_PER_LUN;
     tpnt->sg_tablesize = 
-	(setup_sg_tablesize >= 0) ? setup_sg_tablesize : SG_TABLESIZE;
+	(setup_sg_tablesize > 0) ? setup_sg_tablesize : 1;
 
     if (setup_hostid >= 0)
 	tpnt->this_id = setup_hostid;

@@ -590,7 +589,7 @@ static struct scsi_host_template driver_template = {
 	.eh_bus_reset_handler		= macscsi_bus_reset,
 	.can_queue			= CAN_QUEUE,
 	.this_id			= 7,
-	.sg_tablesize			= SG_ALL,
+	.sg_tablesize			= NCR5380_MAX_SG,
 	.cmd_per_lun			= CMD_PER_LUN,
 	.unchecked_isa_dma		= 0,
 	.use_clustering			= DISABLE_CLUSTERING

drivers/scsi/mac_scsi.h +0 -4

index d26e331..3bc9919 100644
@@ -41,10 +41,6 @@
 #define CAN_QUEUE 16
 #endif
 
-#ifndef SG_TABLESIZE
-#define SG_TABLESIZE SG_NONE
-#endif
-
 #ifndef USE_TAGGED_QUEUING
 #define	USE_TAGGED_QUEUING 0
 #endif

drivers/scsi/sun3_scsi.c +2 -2

index e606cf0..ff06208 100644
@@ -222,7 +222,7 @@ int sun3scsi_detect(struct scsi_host_template * tpnt)
 	tpnt->cmd_per_lun =
 		(setup_cmd_per_lun > 0) ? setup_cmd_per_lun : CMD_PER_LUN;
 	tpnt->sg_tablesize = 
-		(setup_sg_tablesize >= 0) ? setup_sg_tablesize : SG_TABLESIZE;
+		(setup_sg_tablesize > 0) ? setup_sg_tablesize : 1;
 
 	if (setup_hostid >= 0)
 		tpnt->this_id = setup_hostid;

@@ -631,7 +631,7 @@ static struct scsi_host_template driver_template = {
 	.eh_bus_reset_handler  	= sun3scsi_bus_reset,
 	.can_queue		= CAN_QUEUE,
 	.this_id		= 7,
-	.sg_tablesize		= SG_TABLESIZE,
+	.sg_tablesize		= NCR5380_MAX_SG,
 	.cmd_per_lun		= CMD_PER_LUN,
 	.use_clustering		= DISABLE_CLUSTERING
 };

drivers/scsi/sun3_scsi.h +0 -4

index b29a9d6..b44e7b4 100644
@@ -63,10 +63,6 @@ static int sun3scsi_release (struct Scsi_Host *);
 #define CAN_QUEUE 16
 #endif
 
-#ifndef SG_TABLESIZE
-#define SG_TABLESIZE SG_NONE
-#endif
-
 #ifndef MAX_TAGS
 #define MAX_TAGS 32
 #endif

drivers/scsi/sun3_scsi_vme.c +2 -2

index 02d9727..e9c3632 100644
@@ -170,7 +170,7 @@ static int sun3scsi_detect(struct scsi_host_template * tpnt)
 	tpnt->cmd_per_lun =
 		(setup_cmd_per_lun > 0) ? setup_cmd_per_lun : CMD_PER_LUN;
 	tpnt->sg_tablesize = 
-		(setup_sg_tablesize >= 0) ? setup_sg_tablesize : SG_TABLESIZE;
+		(setup_sg_tablesize > 0) ? setup_sg_tablesize : 1;
 	
 	if (setup_hostid >= 0)
 		tpnt->this_id = setup_hostid;

@@ -574,7 +574,7 @@ static struct scsi_host_template driver_template = {
 	.eh_bus_reset_handler  	= sun3scsi_bus_reset,
 	.can_queue		= CAN_QUEUE,
 	.this_id		= 7,
-	.sg_tablesize		= SG_TABLESIZE,
+	.sg_tablesize		= NCR5380_MAX_SG,
 	.cmd_per_lun		= CMD_PER_LUN,
 	.use_clustering		= DISABLE_CLUSTERING
 };

Signature · 1 line

1.5.3.3

Boaz Harrosh

author

2008-01-17 16:48 UTC #12 lore raw

PATCH 4/8 wd33c93: Not sg-chain ready

In reply to Boaz Harrosh

wd33c93 core is not sg-chain ready, so maximum scatterlist supported is SCSI_MAX_SG_SEGMENTS. Define it so, in core, and let all drivers use it for the sg_tablesize in host templates.

Signed-off-by Boaz Harrosh <[email protected]>

---
 drivers/scsi/a2091.c   |    2 +-
 drivers/scsi/a3000.c   |    2 +-
 drivers/scsi/gvp11.c   |    2 +-
 drivers/scsi/mvme147.c |    2 +-
 drivers/scsi/sgiwd93.c |    2 +-
 drivers/scsi/wd33c93.h |    2 ++
 6 files changed, 7 insertions(+), 5 deletions(-)

drivers/scsi/a2091.c +1 -1

index 23f27c9..a5a511f 100644
@@ -217,7 +217,7 @@ static struct scsi_host_template driver_template = {
 	.eh_host_reset_handler	= wd33c93_host_reset,
 	.can_queue		= CAN_QUEUE,
 	.this_id		= 7,
-	.sg_tablesize		= SG_ALL,
+	.sg_tablesize		= WD_MAX_SG,
 	.cmd_per_lun		= CMD_PER_LUN,
 	.use_clustering		= DISABLE_CLUSTERING
 };

drivers/scsi/a3000.c +1 -1

index d7255c8..aaadf73 100644
@@ -222,7 +222,7 @@ static struct scsi_host_template driver_template = {
 	.eh_host_reset_handler	= wd33c93_host_reset,
 	.can_queue		= CAN_QUEUE,
 	.this_id		= 7,
-	.sg_tablesize		= SG_ALL,
+	.sg_tablesize		= WD_MAX_SG,
 	.cmd_per_lun		= CMD_PER_LUN,
 	.use_clustering		= ENABLE_CLUSTERING
 };

drivers/scsi/gvp11.c +1 -1

index 37741e9..e56c257 100644
@@ -371,7 +371,7 @@ static struct scsi_host_template driver_template = {
 	.eh_host_reset_handler	= wd33c93_host_reset,
 	.can_queue		= CAN_QUEUE,
 	.this_id		= 7,
-	.sg_tablesize		= SG_ALL,
+	.sg_tablesize		= WD_MAX_SG,
 	.cmd_per_lun		= CMD_PER_LUN,
 	.use_clustering		= DISABLE_CLUSTERING
 };

drivers/scsi/mvme147.c +1 -1

index be41aad..e69c108 100644
@@ -140,7 +140,7 @@ static struct scsi_host_template driver_template = {
 	.eh_host_reset_handler	= wd33c93_host_reset,
 	.can_queue		= CAN_QUEUE,
 	.this_id		= 7,
-	.sg_tablesize		= SG_ALL,
+	.sg_tablesize		= WD_MAX_SG,
 	.cmd_per_lun		= CMD_PER_LUN,
 	.use_clustering		= ENABLE_CLUSTERING
 };

drivers/scsi/sgiwd93.c +1 -1

index d4ebe8c..60cc3d1 100644
@@ -208,7 +208,7 @@ static struct scsi_host_template sgiwd93_template = {
 	.eh_host_reset_handler	= wd33c93_host_reset,
 	.can_queue		= 16,
 	.this_id		= 7,
-	.sg_tablesize		= SG_ALL,
+	.sg_tablesize		= WD_MAX_SG,
 	.cmd_per_lun		= 8,
 	.use_clustering		= DISABLE_CLUSTERING,
 };

drivers/scsi/wd33c93.h +2 -0

index 00123f2..e54d7f4 100644
@@ -339,6 +339,8 @@ struct WD33C93_hostdata {
 #define PR_TEST      1<<6
 #define PR_STOP      1<<7
 
+/* FIXME: Not sg-chain ready */
+#define WD_MAX_SG SCSI_MAX_SG_SEGMENTS
 
 void wd33c93_init (struct Scsi_Host *instance, const wd33c93_regs regs,
          dma_setup_t setup, dma_stop_t stop, int clock_freq);

Signature · 1 line

1.5.3.3

Boaz Harrosh

author

2008-01-17 16:49 UTC #13 lore raw

PATCH 5/8 arm/scsi: Not sg-chain ready

In reply to Boaz Harrosh

arm-scsi core is not sg-chain ready, so maximum scatterlist supported is SCSI_MAX_SG_SEGMENTS. Define it so, in core, and let all drivers use it for the sg_tablesize in host templates.

Signed-off-by Boaz Harrosh <[email protected]>

---
 drivers/scsi/arm/acornscsi.c |    2 +-
 drivers/scsi/arm/arxescsi.c  |    2 +-
 drivers/scsi/arm/cumana_2.c  |    2 +-
 drivers/scsi/arm/eesox.c     |    2 +-
 drivers/scsi/arm/powertec.c  |    2 +-
 drivers/scsi/arm/scsi.h      |    2 ++
 6 files changed, 7 insertions(+), 5 deletions(-)

drivers/scsi/arm/acornscsi.c +1 -1

index 3bedf24..58a88a8 100644
@@ -2981,7 +2981,7 @@ static struct scsi_host_template acornscsi_template = {
 	.reset			= acornscsi_reset,
 	.can_queue		= 16,
 	.this_id		= 7,
-	.sg_tablesize		= SG_ALL,
+	.sg_tablesize		= ARM_SCSI_MAX_SG,
 	.cmd_per_lun		= 2,
 	.unchecked_isa_dma	= 0,
 	.use_clustering		= DISABLE_CLUSTERING,

drivers/scsi/arm/arxescsi.c +1 -1

index 2836fe2..a35e7c1 100644
@@ -270,7 +270,7 @@ static struct scsi_host_template arxescsi_template = {
 	.eh_abort_handler		= fas216_eh_abort,
 	.can_queue			= 0,
 	.this_id			= 7,
-	.sg_tablesize			= SG_ALL,
+	.sg_tablesize			= ARM_SCSI_MAX_SG,
 	.cmd_per_lun			= 1,
 	.use_clustering			= DISABLE_CLUSTERING,
 	.proc_name			= "arxescsi",

drivers/scsi/arm/cumana_2.c +1 -1

index 68a6412..3fe690c 100644
@@ -390,7 +390,7 @@ static struct scsi_host_template cumanascsi2_template = {
 	.eh_abort_handler		= fas216_eh_abort,
 	.can_queue			= 1,
 	.this_id			= 7,
-	.sg_tablesize			= SG_ALL,
+	.sg_tablesize			= ARM_SCSI_MAX_SG,
 	.cmd_per_lun			= 1,
 	.use_clustering			= DISABLE_CLUSTERING,
 	.proc_name			= "cumanascsi2",

drivers/scsi/arm/eesox.c +1 -1

index bb2477b..e0a1cf6 100644
@@ -508,7 +508,7 @@ static struct scsi_host_template eesox_template = {
 	.eh_abort_handler		= fas216_eh_abort,
 	.can_queue			= 1,
 	.this_id			= 7,
-	.sg_tablesize			= SG_ALL,
+	.sg_tablesize			= ARM_SCSI_MAX_SG,
 	.cmd_per_lun			= 1,
 	.use_clustering			= DISABLE_CLUSTERING,
 	.proc_name			= "eesox",

drivers/scsi/arm/powertec.c +1 -1

index d9a546d..ca27047 100644
@@ -302,7 +302,7 @@ static struct scsi_host_template powertecscsi_template = {
 
 	.can_queue			= 8,
 	.this_id			= 7,
-	.sg_tablesize			= SG_ALL,
+	.sg_tablesize			= ARM_SCSI_MAX_SG,
 	.cmd_per_lun			= 2,
 	.use_clustering			= ENABLE_CLUSTERING,
 	.proc_name			= "powertec",

drivers/scsi/arm/scsi.h +2 -0

index c7415fc..dffd073 100644
@@ -14,6 +14,8 @@
 
 #define BELT_AND_BRACES
 
+/* FIXME: all drivers that depend on this are not chain-safe */
+#define ARM_SCSI_MAX_SG SCSI_MAX_SG_SEGMENTS
 /*
  * The scatter-gather list handling.  This contains all
  * the yucky stuff that needs to be fixed properly.

Signature · 1 line

1.5.3.3

Boaz Harrosh

author

2008-01-17 16:51 UTC #14 lore raw

scsi: Drivers not ready for sg-chaining

In reply to Boaz Harrosh

All below drivers are not sg-chain ready do to incomplete software. Once fixed they can move back to SG_ALL. For now they are stuck on SCSI_MAX_SG_SEGMENTS.

Affected drivers/files:

	drivers/scsi/aha152x.c
	drivers/scsi/esp_scsi.[ch]
	drivers/scsi/imm.c
	drivers/scsi/in2000.h
	drivers/scsi/pcmcia/nsp_cs.c
	drivers/scsi/ppa.c
	drivers/scsi/tmscsim.c

Signed-off-by Boaz Harrosh <[email protected]>

---
 drivers/scsi/aha152x.c       |    5 ++++-
 drivers/scsi/esp_scsi.c      |    2 +-
 drivers/scsi/esp_scsi.h      |    3 +++
 drivers/scsi/imm.c           |    5 ++++-
 drivers/scsi/in2000.h        |    2 +-
 drivers/scsi/pcmcia/nsp_cs.c |    5 ++++-
 drivers/scsi/ppa.c           |    4 +++-
 drivers/scsi/tmscsim.c       |    5 ++++-
 8 files changed, 24 insertions(+), 7 deletions(-)

drivers/scsi/aha152x.c +4 -1

index 6ccdc96..6be6008 100644
@@ -3460,6 +3460,9 @@ static int aha152x_adjust_queue(struct scsi_device *device)
 	return 0;
 }
 
+/* FIXME: Not sg-chain ready */
+#define AHA152X_MAX_SG SCSI_MAX_SG_SEGMENTS
+
 static struct scsi_host_template aha152x_driver_template = {
 	.module				= THIS_MODULE,
 	.name				= AHA152X_REVID,

@@ -3473,7 +3476,7 @@ static struct scsi_host_template aha152x_driver_template = {
 	.bios_param			= aha152x_biosparam,
 	.can_queue			= 1,
 	.this_id			= 7,
-	.sg_tablesize			= SG_ALL,
+	.sg_tablesize			= AHA152X_MAX_SG,
 	.cmd_per_lun			= 1,
 	.use_clustering			= DISABLE_CLUSTERING,
 	.slave_alloc			= aha152x_adjust_queue,

drivers/scsi/esp_scsi.c +1 -1

index bfdee59..83165f2 100644
@@ -2603,7 +2603,7 @@ struct scsi_host_template scsi_esp_template = {
 	.eh_host_reset_handler	= esp_eh_host_reset_handler,
 	.can_queue		= 7,
 	.this_id		= 7,
-	.sg_tablesize		= SG_ALL,
+	.sg_tablesize		= ESP_MAX_SG,
 	.use_clustering		= ENABLE_CLUSTERING,
 	.max_sectors		= 0xffff,
 	.skip_settle_delay	= 1,

drivers/scsi/esp_scsi.h +3 -0

index d5576d5..0f86a90 100644
@@ -289,6 +289,9 @@ struct esp_cmd_entry {
 #define ESP_MAX_LUN		8
 #define ESP_MAX_TAG		256
 
+/* FIXME: not sg-chain ready */
+#define ESP_MAX_SG SCSI_MAX_SG_SEGMENTS
+
 struct esp_lun_data {
 	struct esp_cmd_entry	*non_tagged_cmd;
 	int			num_tagged;

drivers/scsi/imm.c +4 -1

index f97d172..6b81692 100644
@@ -1112,6 +1112,9 @@ static int imm_adjust_queue(struct scsi_device *device)
 	return 0;
 }
 
+/* FIXME: not sg-chain ready */
+#define IMM_MAX_SG SCSI_MAX_SG_SEGMENTS
+
 static struct scsi_host_template imm_template = {
 	.module			= THIS_MODULE,
 	.proc_name		= "imm",

@@ -1123,7 +1126,7 @@ static struct scsi_host_template imm_template = {
 	.eh_host_reset_handler	= imm_reset,
 	.bios_param		= imm_biosparam,
 	.this_id		= 7,
-	.sg_tablesize		= SG_ALL,
+	.sg_tablesize		= IMM_MAX_SG,
 	.cmd_per_lun		= 1,
 	.use_clustering		= ENABLE_CLUSTERING,
 	.can_queue		= 1,

drivers/scsi/in2000.h +1 -1

index 0fb8b06..388e16f 100644
@@ -405,7 +405,7 @@ static int in2000_bus_reset(Scsi_Cmnd *);
 
 
 #define IN2000_CAN_Q    16
-#define IN2000_SG       SG_ALL
+#define IN2000_SG       SCSI_MAX_SG_SEGMENTS /* FIXME: not sg-chain ready */
 #define IN2000_CPL      2
 #define IN2000_HOST_ID  7

drivers/scsi/pcmcia/nsp_cs.c +4 -1

index 5082ca3..a8f6d24 100644
@@ -77,6 +77,9 @@ static int       free_ports = 0;
 module_param(free_ports, bool, 0);
 MODULE_PARM_DESC(free_ports, "Release IO ports after configuration? (default: 0 (=no))");
 
+/* FIXME: not sg-chain ready */
+#define NSP_MAX_SG SCSI_MAX_SG_SEGMENTS
+
 static struct scsi_host_template nsp_driver_template = {
 	.proc_name	         = "nsp_cs",
 	.proc_info		 = nsp_proc_info,

@@ -88,7 +91,7 @@ static struct scsi_host_template nsp_driver_template = {
 	.eh_host_reset_handler	 = nsp_eh_host_reset,
 	.can_queue		 = 1,
 	.this_id		 = NSP_INITIATOR_ID,
-	.sg_tablesize		 = SG_ALL,
+	.sg_tablesize		 = NSP_MAX_SG,
 	.cmd_per_lun		 = 1,
 	.use_clustering		 = DISABLE_CLUSTERING,
 };

drivers/scsi/ppa.c +3 -1

index f655ae3..a47309d 100644
@@ -23,6 +23,8 @@
 #include <scsi/scsi_device.h>
 #include <scsi/scsi_host.h>
 
+/* FIXME: Not chainning safe */
+#define PPA_MAX_SG SCSI_MAX_SG_SEGMENTS
 
 static void ppa_reset_pulse(unsigned int base);

@@ -986,7 +988,7 @@ static struct scsi_host_template ppa_template = {
 	.eh_host_reset_handler	= ppa_reset,
 	.bios_param		= ppa_biosparam,
 	.this_id		= -1,
-	.sg_tablesize		= SG_ALL,
+	.sg_tablesize		= PPA_MAX_SG,
 	.cmd_per_lun		= 1,
 	.use_clustering		= ENABLE_CLUSTERING,
 	.can_queue		= 1,

drivers/scsi/tmscsim.c +4 -1

index 5b04ddf..6a7824a 100644
@@ -2194,6 +2194,9 @@ static int dc390_slave_configure(struct scsi_device *sdev)
 	return 0;
 }
 
+/* FIXME: not sg-chain ready */
+#define DC390_MAX_SG SCSI_MAX_SG_SEGMENTS
+
 static struct scsi_host_template driver_template = {
 	.module			= THIS_MODULE,
 	.proc_name		= "tmscsim",

@@ -2206,7 +2209,7 @@ static struct scsi_host_template driver_template = {
 	.eh_bus_reset_handler	= DC390_bus_reset,
 	.can_queue		= 1,
 	.this_id		= 7,
-	.sg_tablesize		= SG_ALL,
+	.sg_tablesize		= DC390_MAX_SG,
 	.cmd_per_lun		= 1,
 	.use_clustering		= ENABLE_CLUSTERING,
 	.max_sectors		= 0x4000, /* 8MiB = 16 * 1024 * 512 */

Signature · 1 line

1.5.3.3

James Bottomley

maintainer

2008-02-10 15:42 UTC #15 lore raw

Re: scsi: Drivers not ready for sg-chaining

Quoted 6 lines · Boaz Harrosh

All below drivers are not sg-chain ready do to incomplete software.
Once fixed they can move back to SG_ALL. For now they are stuck on SCSI_MAX_SG_SEGMENTS.
Affected drivers/files:
	drivers/scsi/aha152x.c

This seems to process an element at a time and should be fixed by sg_next()

Quoted 1 line

drivers/scsi/esp_scsi.[ch]

As does this.

Quoted 1 line

drivers/scsi/imm.c

And this.

Quoted 1 line

	drivers/scsi/in2000.h

And this (.c not .h)

Quoted 1 line

drivers/scsi/pcmcia/nsp_cs.c

This uses a software table, so should be fine.

Quoted 1 line

drivers/scsi/ppa.c

This does element at a time, so should be fixed by sg_next()

Quoted 1 line

drivers/scsi/tmscsim.c

as does this.

Given where we are in the cycle, it's probably best just to set SG_ALL to 128 so there's no possibility of problems. Individual drivers can make the tradeoff on how they handle larger lists going beyond this.

James

---

include/scsi/scsi_host.h +6 -2

index d1299e9..530ff4c 100644
@@ -6,6 +6,7 @@
 #include <linux/types.h>
 #include <linux/workqueue.h>
 #include <linux/mutex.h>
+#include <scsi/scsi.h>
 
 struct request_queue;
 struct block_device;

@@ -25,12 +26,15 @@ struct blk_queue_tags;
  * NONE: Self evident.	Host adapter is not capable of scatter-gather.
  * ALL:	 Means that the host adapter module can do scatter-gather,
  *	 and that there is no limit to the size of the table to which
- *	 we scatter/gather data.
+ *	 we scatter/gather data.  The value we set here is the maximum
+ *	 single element sglist.  To use chained sglists, the adapter
+ *	 has to set a value beyond ALL (and correctly use the chain
+ *	 handling API.
  * Anything else:  Indicates the maximum number of chains that can be
  *	 used in one scatter-gather request.
  */
 #define SG_NONE 0
-#define SG_ALL 0xff
+#define SG_ALL	SCSI_MAX_SG_SEGMENTS
 
 #define MODE_UNKNOWN 0x00
 #define MODE_INITIATOR 0x01

James

Boaz Harrosh

author

2008-02-10 16:08 UTC #16 lore raw

Re: scsi: Drivers not ready for sg-chaining

Quoted 75 lines · James Bottomley

Quoted 6 lines · Boaz Harrosh
All below drivers are not sg-chain ready do to incomplete software.
Once fixed they can move back to SG_ALL. For now they are stuck on SCSI_MAX_SG_SEGMENTS.
Affected drivers/files:
	drivers/scsi/aha152x.c
This seems to process an element at a time and should be fixed by sg_next()
Quoted 1 line
drivers/scsi/esp_scsi.[ch]
As does this.
Quoted 1 line
drivers/scsi/imm.c
And this.
Quoted 1 line
	drivers/scsi/in2000.h
And this (.c not .h)
Quoted 1 line
drivers/scsi/pcmcia/nsp_cs.c
This uses a software table, so should be fine.
Quoted 1 line
drivers/scsi/ppa.c
This does element at a time, so should be fixed by sg_next()
Quoted 1 line
drivers/scsi/tmscsim.c
as does this.
Given where we are in the cycle, it's probably best just to set SG_ALL to 128 so there's no possibility of problems. Individual drivers can make the tradeoff on how they handle larger lists going beyond this.
James
---
include/scsi/scsi_host.h +6 -2
index d1299e9..530ff4c 100644
@@ -6,6 +6,7 @@
 #include <linux/types.h>
 #include <linux/workqueue.h>
 #include <linux/mutex.h>
+#include <scsi/scsi.h>
 
 struct request_queue;
 struct block_device;
@@ -25,12 +26,15 @@ struct blk_queue_tags;
  * NONE: Self evident.	Host adapter is not capable of scatter-gather.
  * ALL:	 Means that the host adapter module can do scatter-gather,
  *	 and that there is no limit to the size of the table to which
- *	 we scatter/gather data.
+ *	 we scatter/gather data.  The value we set here is the maximum
+ *	 single element sglist.  To use chained sglists, the adapter
+ *	 has to set a value beyond ALL (and correctly use the chain
+ *	 handling API.
  * Anything else:  Indicates the maximum number of chains that can be
  *	 used in one scatter-gather request.
  */
 #define SG_NONE 0
-#define SG_ALL 0xff
+#define SG_ALL	SCSI_MAX_SG_SEGMENTS
 
 #define MODE_UNKNOWN 0x00
 #define MODE_INITIATOR 0x01

Quoted 3 lines

James

My patches *do not* attempt to fix the sg_chaining support. They only make all the drivers that use SG_ALL to use SCSI_MAX_SG_SEGMENTS. One by One, and not globally as your suggestion.

This is for two reasons.
1. So drivers can be individually fixed and in the patch that fixes them they can go back to SG_ALL.

2. Those drivers that have been using SG_ALL correctly and were converted to support sg-chaining are not penalized because of bad/old drivers

3. Some drivers in this patchset are converted to use a real internal driver limit. That does not necessarily match SCSI_MAX_SG_SEGMENTS. In the event that SCSI_MAX_SG_SEGMENTS wants to change.

The bulk of the patchset is very much mechanical and is not dangerous and was ACKed by the more important maintainers. (That is where the changes are more then trivial). So I don't see why they cannot get a proper review and be accepted. Instead of doing the safe but the wrong thing, cross tree.

Boaz

James Bottomley

maintainer

2008-02-10 16:16 UTC #17 lore raw

Re: scsi: Drivers not ready for sg-chaining

Quoted 3 lines · Boaz Harrosh

My patches *do not* attempt to fix the sg_chaining support. They only make all the drivers that use SG_ALL to use SCSI_MAX_SG_SEGMENTS. One by One, and not globally as your suggestion.

Yes, I know ... but it does need fixing for the listed drivers.

Quoted 3 lines

This is for two reasons.
1. So drivers can be individually fixed and in the patch that fixes them they can go back to SG_ALL.

No, it's so SG_ALL can mean use chaining ... I'm not sure that's desirable for the default value. Particularly for devices that key internal sglist arrays off SG_ALL

Quoted 2 lines

2. Those drivers that have been using SG_ALL correctly and were converted to support sg-chaining are not penalized because of bad/old drivers

I don't see they're penalised this way either ... they just have to set a higher value in their host template.

Quoted 3 lines

3. Some drivers in this patchset are converted to use a real internal driver limit. That does not necessarily match SCSI_MAX_SG_SEGMENTS. In the event that SCSI_MAX_SG_SEGMENTS wants to change.

Yes, I looked at those they're all either safe or currently (eventually) do the right thing.

Quoted 5 lines

The bulk of the patchset is very much mechanical and is not dangerous and was ACKed by the more important maintainers. (That is where the changes are more then trivial). So I don't see why they cannot get a proper review and be accepted. Instead of doing the safe but the wrong thing, cross tree.

What's wrong about this?

James

Boaz Harrosh

author

2008-02-10 16:36 UTC #18 lore raw

Re: scsi: Drivers not ready for sg-chaining

Quoted 6 lines · James Bottomley

Quoted 3 lines · Boaz Harrosh
My patches *do not* attempt to fix the sg_chaining support. They only make all the drivers that use SG_ALL to use SCSI_MAX_SG_SEGMENTS. One by One, and not globally as your suggestion.
Yes, I know ... but it does need fixing for the listed drivers.

That is another patchset that its time as not yet come. Once every thing settles I will send these too.
That's why I put the list as a reminder for me to check upon later.

Quoted 9 lines

Quoted 4 lines
This is for two reasons.
1. So drivers can be individually fixed and in the patch that fixes them they can go back to SG_ALL.
No, it's so SG_ALL can mean use chaining ... I'm not sure that's desirable for the default value. Particularly for devices that key internal sglist arrays off SG_ALL

I have fixed *all* these drivers that are based off SG_ALL for table sizes, and all other reasons. exactly so the SG_ALL will continue to mean what it means in English. (And this was triggered by your request)

Quoted 6 lines

Quoted 2 lines
2. Those drivers that have been using SG_ALL correctly and were converted to support sg-chaining are not penalized because of bad/old drivers
I don't see they're penalised this way either ... they just have to set a higher value in their host template.

It was you who wanted that to be SG_ALL. I wanted just an hard coded = ~0.

Quoted 7 lines

Quoted 3 lines
3. Some drivers in this patchset are converted to use a real internal driver limit. That does not necessarily match SCSI_MAX_SG_SEGMENTS. In the event that SCSI_MAX_SG_SEGMENTS wants to change.
Yes, I looked at those they're all either safe or currently (eventually) do the right thing.

What ??

Quoted 8 lines

Quoted 5 lines
The bulk of the patchset is very much mechanical and is not dangerous and was ACKed by the more important maintainers. (That is where the changes are more then trivial). So I don't see why they cannot get a proper review and be accepted. Instead of doing the safe but the wrong thing, cross tree.
What's wrong about this?

I don't want to repeat myself. If it's fine with you, I trust your final judgment. You are welcome to submit a patch that fixes all the good drivers that are regressed by your suggestion.

Quoted 3 lines

James

Boaz

James Bottomley

maintainer

2008-02-10 16:53 UTC #19 lore raw

Re: scsi: Drivers not ready for sg-chaining

Quoted 7 lines · Boaz Harrosh

Quoted 5 lines
Quoted 2 lines
2. Those drivers that have been using SG_ALL correctly and were converted to support sg-chaining are not penalized because of bad/old drivers
I don't see they're penalised this way either ... they just have to set a higher value in their host template.
It was you who wanted that to be SG_ALL. I wanted just an hard coded = ~0.

Yes, I've changed my mind. I think the best value for SG_ALL is the max single table and we'll do a dynamic increase for drivers that really have considered all the chaining implications. That doesn't mean we don't make the others chain ready ... we do ... we just don't force it on them.

Quoted 3 lines

I don't want to repeat myself. If it's fine with you, I trust your final judgment. You are welcome to submit a patch that fixes all the good drivers that are regressed by your suggestion.

Lowering the figure from current 255 to 128 isn't really going to cause any regressions. It will actually save a few unnecessary allocations for some drivers.

James

Boaz Harrosh

author

2008-01-17 16:53 UTC #20 lore raw

PATCH 7/8 a100u2w: advansys: initio: Wrong use of SG_ALL

In reply to Boaz Harrosh

These drivers used the SG_ALL constant, But actually meant some other define.

Signed-off-by Boaz Harrosh <[email protected]>

---
 drivers/scsi/a100u2w.c  |    3 +--
 drivers/scsi/advansys.c |    8 +++-----
 drivers/scsi/initio.c   |    2 +-
 3 files changed, 5 insertions(+), 8 deletions(-)

drivers/scsi/a100u2w.c +1 -2

index f608d4a..82d5210 100644
@@ -1068,7 +1068,7 @@ static struct scsi_host_template inia100_template = {
 	.eh_device_reset_handler = inia100_device_reset,
 	.can_queue		= 1,
 	.this_id		= 1,
-	.sg_tablesize		= SG_ALL,
+	.sg_tablesize		= TOTAL_SG_ENTRY,
 	.cmd_per_lun 		= 1,
 	.use_clustering		= ENABLE_CLUSTERING,
 };

@@ -1150,7 +1150,6 @@ static int __devinit inia100_probe_one(struct pci_dev *pdev,
 	shost->max_lun = 16;
 	shost->irq = pdev->irq;
 	shost->this_id = host->scsi_id;	/* Assign HCS index */
-	shost->sg_tablesize = TOTAL_SG_ENTRY;
 
 	/* Initial orc chip           */
 	error = request_irq(pdev->irq, inia100_intr, IRQF_SHARED,

drivers/scsi/advansys.c +3 -5

index 492702b..45d58d3 100644
@@ -13735,13 +13735,11 @@ static int __devinit advansys_board_found(struct Scsi_Host *shost,
 	}
 
 	/*
-	 * The value of 'sg_tablesize' can not exceed the SCSI
-	 * mid-level driver definition of SG_ALL. SG_ALL also
-	 * must not be exceeded, because it is used to define the
+	 * The value of 'sg_tablesize' can not exceed the
 	 * size of the scatter-gather table in 'struct asc_sg_head'.
 	 */
-	if (shost->sg_tablesize > SG_ALL) {
-		shost->sg_tablesize = SG_ALL;
+	if (shost->sg_tablesize > ASC_MAX_SG_LIST) {
+		shost->sg_tablesize = ASC_MAX_SG_LIST;
 	}
 
 	ASC_DBG(1, "sg_tablesize: %d\n", shost->sg_tablesize);

drivers/scsi/initio.c +1 -1

index 52238ee..02cb133 100644
@@ -2829,7 +2829,7 @@ static struct scsi_host_template initio_template = {
 	.bios_param		= i91u_biosparam,
 	.can_queue		= MAX_TARGETS * i91u_MAXQUEUE,
 	.this_id		= 1,
-	.sg_tablesize		= SG_ALL,
+	.sg_tablesize		= TOTAL_SG_ENTRY,
 	.cmd_per_lun		= 1,
 	.use_clustering		= ENABLE_CLUSTERING,
 };

Signature · 1 line

1.5.3.3

Boaz Harrosh

author

2008-01-17 16:55 UTC #21 lore raw

PATCH 8/8 Change SG_ALL to mean "any size"

In reply to Boaz Harrosh

Change the definition of SG_ALL from 255, which was the biggest possible scatter list array in 32 bit ARCHs (minus one who knows why), to be ~0. Which means that LLD has no size limit on number of scatter-list array and the actual size will be determined by upper layers.
Also SG_NONE is removed because it is no longer supported.
Note: SG_ALL can not be used as an allocation size anymore.

FIXME: Important/capable drivers like iscsi have moved away from
SG_ALL to hard coded numbers like 4096. They should reuse SG_ALL now.

Signed-off-by Boaz Harrosh <[email protected]>

---
 include/scsi/scsi_host.h |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

include/scsi/scsi_host.h +2 -4

index 5c58d59..6032d07 100644
@@ -21,16 +21,14 @@ struct blk_queue_tags;
 
 
 /*
- * The various choices mean:
- * NONE: Self evident.	Host adapter is not capable of scatter-gather.
+ * values to use with .sg_tablesize:
  * ALL:	 Means that the host adapter module can do scatter-gather,
  *	 and that there is no limit to the size of the table to which
  *	 we scatter/gather data.
  * Anything else:  Indicates the maximum number of chains that can be
  *	 used in one scatter-gather request.
  */
-#define SG_NONE 0
-#define SG_ALL 0xff
+#define SG_ALL (~0)
 
 #define MODE_UNKNOWN 0x00
 #define MODE_INITIATOR 0x01

Signature · 1 line

1.5.3.3

Stefan Richter

2008-01-17 17:54 UTC #22 lore raw

In reply to Boaz Harrosh

Quoted 3 lines · Boaz Harrosh

Those that meant SG_ALL == 255:
[PATCH 1/8] firewire: ieee1394: Move away from SG_ALL

They simply mean SG_ALL == something, as long as it isn't unreasonably small or large.

Signature · 3 lines

Stefan Richter
-=====-==--- ---= =---=
http://arcgraph.de/sr/