From 4635873c561ac57b66adfcc2487c38106b1c916c Mon Sep 17 00:00:00 2001 From: Ming Lei Date: Sun, 28 Apr 2019 15:39:30 +0800 Subject: scsi: lib/sg_pool.c: improve APIs for allocating sg pool sg_alloc_table_chained() currently allows the caller to provide one preallocated SGL and returns if the requested number isn't bigger than size of that SGL. This is used to inline an SGL for an IO request. However, scattergather code only allows that size of the 1st preallocated SGL to be SG_CHUNK_SIZE(128). This means a substantial amount of memory (4KB) is claimed for the SGL for each IO request. If the I/O is small, it would be prudent to allocate a smaller SGL. Introduce an extra parameter to sg_alloc_table_chained() and sg_free_table_chained() for specifying size of the preallocated SGL. Both __sg_free_table() and __sg_alloc_table() assume that each SGL has the same size except for the last one. Change the code to allow both functions to accept a variable size for the 1st preallocated SGL. [mkp: attempted to clarify commit desc] Cc: Christoph Hellwig Cc: Bart Van Assche Cc: Ewan D. Milne Cc: Hannes Reinecke Cc: Sagi Grimberg Cc: Chuck Lever Cc: netdev@vger.kernel.org Cc: linux-nvme@lists.infradead.org Suggested-by: Christoph Hellwig Signed-off-by: Ming Lei Reviewed-by: Christoph Hellwig Signed-off-by: Martin K. Petersen --- lib/sg_pool.c | 37 +++++++++++++++++++++++++++---------- 1 file changed, 27 insertions(+), 10 deletions(-) (limited to 'lib/sg_pool.c') diff --git a/lib/sg_pool.c b/lib/sg_pool.c index d1c1e6388eaa..b3b8cf62ff49 100644 --- a/lib/sg_pool.c +++ b/lib/sg_pool.c @@ -69,18 +69,27 @@ static struct scatterlist *sg_pool_alloc(unsigned int nents, gfp_t gfp_mask) /** * sg_free_table_chained - Free a previously mapped sg table * @table: The sg table header to use - * @first_chunk: was first_chunk not NULL in sg_alloc_table_chained? + * @nents_first_chunk: size of the first_chunk SGL passed to + * sg_alloc_table_chained * * Description: * Free an sg table previously allocated and setup with * sg_alloc_table_chained(). * + * @nents_first_chunk has to be same with that same parameter passed + * to sg_alloc_table_chained(). + * **/ -void sg_free_table_chained(struct sg_table *table, bool first_chunk) +void sg_free_table_chained(struct sg_table *table, + unsigned nents_first_chunk) { - if (first_chunk && table->orig_nents <= SG_CHUNK_SIZE) + if (table->orig_nents <= nents_first_chunk) return; - __sg_free_table(table, SG_CHUNK_SIZE, first_chunk, sg_pool_free); + + if (nents_first_chunk == 1) + nents_first_chunk = 0; + + __sg_free_table(table, SG_CHUNK_SIZE, nents_first_chunk, sg_pool_free); } EXPORT_SYMBOL_GPL(sg_free_table_chained); @@ -89,31 +98,39 @@ EXPORT_SYMBOL_GPL(sg_free_table_chained); * @table: The sg table header to use * @nents: Number of entries in sg list * @first_chunk: first SGL + * @nents_first_chunk: number of the SGL of @first_chunk * * Description: * Allocate and chain SGLs in an sg table. If @nents@ is larger than - * SG_CHUNK_SIZE a chained sg table will be setup. + * @nents_first_chunk a chained sg table will be setup. * **/ int sg_alloc_table_chained(struct sg_table *table, int nents, - struct scatterlist *first_chunk) + struct scatterlist *first_chunk, unsigned nents_first_chunk) { int ret; BUG_ON(!nents); - if (first_chunk) { - if (nents <= SG_CHUNK_SIZE) { + if (first_chunk && nents_first_chunk) { + if (nents <= nents_first_chunk) { table->nents = table->orig_nents = nents; sg_init_table(table->sgl, nents); return 0; } } + /* User supposes that the 1st SGL includes real entry */ + if (nents_first_chunk == 1) { + first_chunk = NULL; + nents_first_chunk = 0; + } + ret = __sg_alloc_table(table, nents, SG_CHUNK_SIZE, - first_chunk, GFP_ATOMIC, sg_pool_alloc); + first_chunk, nents_first_chunk, + GFP_ATOMIC, sg_pool_alloc); if (unlikely(ret)) - sg_free_table_chained(table, (bool)first_chunk); + sg_free_table_chained(table, nents_first_chunk); return ret; } EXPORT_SYMBOL_GPL(sg_alloc_table_chained); -- cgit From b79d9a09ae23c7047bdce3a15e284398334198ea Mon Sep 17 00:00:00 2001 From: Ming Lei Date: Thu, 6 Jun 2019 16:34:08 +0800 Subject: scsi: lib/sg_pool.c: clear 'first_chunk' in case of no preallocation If user doesn't ask to preallocate by passing zero 'nents_first_chunk' to sg_alloc_table_chained, we need to make sure that 'first_chunk' is cleared. Otherwise, __sg_alloc_table() still may think that the 1st SGL should be from the preallocation. Fixes the issue by clearing 'first_chunk' in sg_alloc_table_chained() if 'nents_first_chunk' is zero. Cc: Christoph Hellwig Cc: Bart Van Assche Cc: Ewan D. Milne Cc: Hannes Reinecke Cc: Guenter Roeck Reported-by: Guenter Roeck Tested-by: Guenter Roeck Signed-off-by: Ming Lei Signed-off-by: Martin K. Petersen --- lib/sg_pool.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'lib/sg_pool.c') diff --git a/lib/sg_pool.c b/lib/sg_pool.c index b3b8cf62ff49..f1cc8372df67 100644 --- a/lib/sg_pool.c +++ b/lib/sg_pool.c @@ -102,7 +102,9 @@ EXPORT_SYMBOL_GPL(sg_free_table_chained); * * Description: * Allocate and chain SGLs in an sg table. If @nents@ is larger than - * @nents_first_chunk a chained sg table will be setup. + * @nents_first_chunk a chained sg table will be setup. @first_chunk is + * ignored if nents_first_chunk <= 1 because user expects the SGL points + * non-chain SGL. * **/ int sg_alloc_table_chained(struct sg_table *table, int nents, @@ -121,7 +123,7 @@ int sg_alloc_table_chained(struct sg_table *table, int nents, } /* User supposes that the 1st SGL includes real entry */ - if (nents_first_chunk == 1) { + if (nents_first_chunk <= 1) { first_chunk = NULL; nents_first_chunk = 0; } -- cgit