Make the SCSI mempool allocations variable

Based on Jeremy's request to increase max_phys_segments, this is the way to do it (SCSI_MAX_PHYS_SEGMENTS) is the value that gets set as the queue's max_phys_segments.

I have to say that when I tried raising it to 256 and hammering a 10GB ext2 filesystem, I still didn't generate any >128 segment requests, so I'm dubious that raising it has any benefit at all, but feel free to try it and publish the figures.

I did wonder if lowering it might help improve the memory footprint for some embedded systems, so I set it up to be lowered as far as 32.

James

===== drivers/scsi/scsi_lib.c 1.120 vs edited =====

@@ -24,7 +24,7 @@
 #include "scsi_logging.h"
 
 
-#define SG_MEMPOOL_NR		5
+#define SG_MEMPOOL_NR		(sizeof(scsi_sg_pools)/sizeof(struct scsi_host_sg_pool))
 #define SG_MEMPOOL_SIZE		32
 
 struct scsi_host_sg_pool {

@@ -34,9 +34,27 @@
 	mempool_t	*pool;
 };
 
+#if (SCSI_MAX_PHYS_SEGMENTS < 32)
+#error SCSI_MAX_PHYS_SEGMENTS is too small
+#endif
+
 #define SP(x) { x, "sgpool-" #x } 
-struct scsi_host_sg_pool scsi_sg_pools[SG_MEMPOOL_NR] = { 
-	SP(8), SP(16), SP(32), SP(64), SP(MAX_PHYS_SEGMENTS)
+struct scsi_host_sg_pool scsi_sg_pools[] = { 
+	SP(8),
+	SP(16),
+	SP(32),
+#if (SCSI_MAX_PHYS_SEGMENTS > 32)
+	SP(64),
+#if (SCSI_MAX_PHYS_SEGMENTS > 64)
+	SP(128),
+#if (SCSI_MAX_PHYS_SEGMENTS > 128)
+	SP(256),
+#if (SCSI_MAX_PHYS_SEGMENTS > 256)
+#error SCSI_MAX_PHYS_SEGMENTS is too large
+#endif
+#endif
+#endif
+#endif
 }; 	
 #undef SP
 

@@ -558,12 +576,21 @@
 	case 17 ... 32:
 		cmd->sglist_len = 2;
 		break;
+#if (SCSI_MAX_PHYS_SEGMENTS > 32)
 	case 33 ... 64:
 		cmd->sglist_len = 3;
 		break;
-	case 65 ... MAX_PHYS_SEGMENTS:
+#if (SCSI_MAX_PHYS_SEGMENTS > 64)
+	case 65 ... 128:
 		cmd->sglist_len = 4;
 		break;
+#if (SCSI_MAX_PHYS_SEGMENTS  > 128)
+	case 129 ... 256:
+		cmd->sglist_len = 5;
+		break;
+#endif
+#endif
+#endif
 	default:
 		return NULL;
 	}

@@ -1285,7 +1312,7 @@
 	blk_queue_prep_rq(q, scsi_prep_fn);
 
 	blk_queue_max_hw_segments(q, shost->sg_tablesize);
-	blk_queue_max_phys_segments(q, MAX_PHYS_SEGMENTS);
+	blk_queue_max_phys_segments(q, SCSI_MAX_PHYS_SEGMENTS);
 	blk_queue_max_sectors(q, shost->max_sectors);
 	blk_queue_bounce_limit(q, scsi_calculate_bounce_limit(shost));
 	blk_queue_segment_boundary(q, shost->dma_boundary);

===== include/scsi/scsi.h 1.17 vs edited =====

@@ -10,6 +10,12 @@
 
 #include <linux/types.h>
 
+/*
+ *	The maximum sg list length SCSI can cope with
+ *	(currently must be a power of 2 between 32 and 256)
+ */
+#define SCSI_MAX_PHYS_SEGMENTS	MAX_PHYS_SEGMENTS
+
 
 /*
  *	SCSI command lengths

Thanks, James!

I promise that we will take advantage of this, but it might take a couple of months. As you point out, the filesystem code also has to deliver the larger requests.

I'll try a couple of tests with XFS and block device direct I/O.

jeremy

I got a chance to try this tonight and can report excellent results. I modified the qla2xxx driver to have an sg_tablesize of 256 (default is SG_ALL, which is 0xFF). I also modified SCSI_MAX_PHYS_SEGMENTS to 256.

I first tried direct I/O to the block device and found that the max I/O size increased to 4MB from 2MB. I then built an XFS filesystem and tried direct I/O to it, and found the same results.

Believe it or not, there are some h/w RAIDs in which you can get a significant performance benefit by increasing I/O size to 4MB from 2MB. I don't have one immediately available to post MB/s numbers, but you can trust me ('cause I can trust those who told me) :-)

So I heartily approve of this change.

Are you thinking of making SCSI_MAX_PHYS_SEGMENTS a config variable? Or would you increase it to 256 by default?

Thanks for doing this,

jeremy

Well, I'm glad someone saw a benefit.

Depending on the arch size of struct scatterlist, the 256 enty sglist is usually around 6k (on 64 bits). That leads the slab to use order 3 allocations and with a default mempool size of 32, that's eight order 3 allocations that get eaten.

Since my tests showed this pool never to be used on ext3, I certainly don't think it should be the default if all it does is waste memory for the majority.

The best course of action is probably to put the patch in (since it has its current default at 128, it will change nothing) so that people who wish to play with increasing the allocation can.

To address the question of how you set this, I'd like more information about which filesystems and workloads find this useful, perhaps with a view to tying it to an indirect config option (CONFIG_ENTERPRISE or something).

James

Most of our users that would benefit from this would fall into the "HPC" segment. HPC meaning High Performance Computing. That's probably not very descriptive. There is typically a lot of floating point and large data sets.

There may be a database involved, but it's not traditional commercial transaction processing, which typically has very small I/O requests.

Other users would be Broadcast, where they stream A/V streams to/from a RAID device.

There are also sites that store large amounts of data keeping track of things (people/places/etc.), looking for things, etc.

Maybe CONFIG_HPC or CONFIG_HIGH_BANDWIDTH_IO. The longer and clumsier the name, the more descriptive. :-)

Filesystems? Is there anything other than XFS? :-) :-)

jeremy