linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/5] mempool based chained scatterlist alloc/free api
@ 2016-03-22 22:03 Ming Lin
  2016-03-22 22:03 ` [PATCH v2 1/5] scsi: replace "scsi_data_buffer" with "sg_table" in SG functions Ming Lin
                   ` (5 more replies)
  0 siblings, 6 replies; 20+ messages in thread
From: Ming Lin @ 2016-03-22 22:03 UTC (permalink / raw)
  To: linux-kernel, linux-scsi; +Cc: Christoph Hellwig

From: Ming Lin <ming.l@ssi.samsung.com>

The fist 4 patches make the SG related definitions/structs/functions
in SCSI code generic and the last patch move it to lib/sg_pool.c.

I still keep the macro "SG_MEMPOOL_NR" since it's used in 3 places.

v2:
  - do modification in scsi code first then move to lib/sg_pool.c
  - address Christoph's comments

Ming Lin (5):
  scsi: replace "scsi_data_buffer" with "sg_table" in SG functions
  scsi: replace "mq" with "first_chunk" in SG functions
  scsi: rename SG related struct and functions
  scsi: rename SCSI_MAX_{SG, SG_CHAIN}_SEGMENTS
  lib: scatterlist: move SG pool code from SCSI driver to lib/sg_pool.c

 drivers/ata/pata_icside.c           |   2 +-
 drivers/infiniband/ulp/srp/ib_srp.c |   4 +-
 drivers/scsi/Kconfig                |   1 +
 drivers/scsi/arm/cumana_2.c         |   2 +-
 drivers/scsi/arm/eesox.c            |   2 +-
 drivers/scsi/arm/powertec.c         |   2 +-
 drivers/scsi/esas2r/esas2r_main.c   |   4 +-
 drivers/scsi/hisi_sas/hisi_sas.h    |   2 +-
 drivers/scsi/mpt3sas/mpt3sas_base.c |   4 +-
 drivers/scsi/mpt3sas/mpt3sas_base.h |   2 +-
 drivers/scsi/scsi_debug.c           |   2 +-
 drivers/scsi/scsi_lib.c             | 172 +++++-------------------------------
 drivers/usb/storage/scsiglue.c      |   2 +-
 include/linux/scatterlist.h         |  25 ++++++
 include/scsi/scsi.h                 |  19 ----
 include/scsi/scsi_host.h            |   2 +-
 lib/Kconfig                         |   7 ++
 lib/Makefile                        |   1 +
 lib/sg_pool.c                       | 172 ++++++++++++++++++++++++++++++++++++
 19 files changed, 241 insertions(+), 186 deletions(-)
 create mode 100644 lib/sg_pool.c

-- 
1.9.1

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v2 1/5] scsi: replace "scsi_data_buffer" with "sg_table" in SG functions
  2016-03-22 22:03 [PATCH v2 0/5] mempool based chained scatterlist alloc/free api Ming Lin
@ 2016-03-22 22:03 ` Ming Lin
  2016-03-22 22:03 ` [PATCH v2 2/5] scsi: replace "mq" with "first_chunk" " Ming Lin
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 20+ messages in thread
From: Ming Lin @ 2016-03-22 22:03 UTC (permalink / raw)
  To: linux-kernel, linux-scsi; +Cc: Christoph Hellwig

From: Ming Lin <ming.l@ssi.samsung.com>

Replace parameter "struct scsi_data_buffer" with "struct sg_table" in
SG alloc/free functions to make them generic.

Signed-off-by: Ming Lin <ming.l@ssi.samsung.com>
---
 drivers/scsi/scsi_lib.c | 41 +++++++++++++++++++++++------------------
 1 file changed, 23 insertions(+), 18 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 8c6e318..5eaddc7 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -583,14 +583,14 @@ static struct scatterlist *scsi_sg_alloc(unsigned int nents, gfp_t gfp_mask)
 	return mempool_alloc(sgp->pool, gfp_mask);
 }
 
-static void scsi_free_sgtable(struct scsi_data_buffer *sdb, bool mq)
+static void scsi_free_sgtable(struct sg_table *table, bool mq)
 {
-	if (mq && sdb->table.orig_nents <= SCSI_MAX_SG_SEGMENTS)
+	if (mq && table->orig_nents <= SCSI_MAX_SG_SEGMENTS)
 		return;
-	__sg_free_table(&sdb->table, SCSI_MAX_SG_SEGMENTS, mq, scsi_sg_free);
+	__sg_free_table(table, SCSI_MAX_SG_SEGMENTS, mq, scsi_sg_free);
 }
 
-static int scsi_alloc_sgtable(struct scsi_data_buffer *sdb, int nents, bool mq)
+static int scsi_alloc_sgtable(struct sg_table *table, int nents, bool mq)
 {
 	struct scatterlist *first_chunk = NULL;
 	int ret;
@@ -599,17 +599,17 @@ static int scsi_alloc_sgtable(struct scsi_data_buffer *sdb, int nents, bool mq)
 
 	if (mq) {
 		if (nents <= SCSI_MAX_SG_SEGMENTS) {
-			sdb->table.nents = sdb->table.orig_nents = nents;
-			sg_init_table(sdb->table.sgl, nents);
+			table->nents = table->orig_nents = nents;
+			sg_init_table(table->sgl, nents);
 			return 0;
 		}
-		first_chunk = sdb->table.sgl;
+		first_chunk = table->sgl;
 	}
 
-	ret = __sg_alloc_table(&sdb->table, nents, SCSI_MAX_SG_SEGMENTS,
+	ret = __sg_alloc_table(table, nents, SCSI_MAX_SG_SEGMENTS,
 			       first_chunk, GFP_ATOMIC, scsi_sg_alloc);
 	if (unlikely(ret))
-		scsi_free_sgtable(sdb, mq);
+		scsi_free_sgtable(table, mq);
 	return ret;
 }
 
@@ -625,12 +625,17 @@ static void scsi_uninit_cmd(struct scsi_cmnd *cmd)
 
 static void scsi_mq_free_sgtables(struct scsi_cmnd *cmd)
 {
+	struct scsi_data_buffer *sdb;
+
 	if (cmd->sdb.table.nents)
-		scsi_free_sgtable(&cmd->sdb, true);
-	if (cmd->request->next_rq && cmd->request->next_rq->special)
-		scsi_free_sgtable(cmd->request->next_rq->special, true);
+		scsi_free_sgtable(&cmd->sdb.table, true);
+	if (cmd->request->next_rq) {
+		sdb = cmd->request->next_rq->special;
+		if (sdb)
+			scsi_free_sgtable(&sdb->table, true);
+	}
 	if (scsi_prot_sg_count(cmd))
-		scsi_free_sgtable(cmd->prot_sdb, true);
+		scsi_free_sgtable(&cmd->prot_sdb->table, true);
 }
 
 static void scsi_mq_uninit_cmd(struct scsi_cmnd *cmd)
@@ -669,19 +674,19 @@ static void scsi_mq_uninit_cmd(struct scsi_cmnd *cmd)
 static void scsi_release_buffers(struct scsi_cmnd *cmd)
 {
 	if (cmd->sdb.table.nents)
-		scsi_free_sgtable(&cmd->sdb, false);
+		scsi_free_sgtable(&cmd->sdb.table, false);
 
 	memset(&cmd->sdb, 0, sizeof(cmd->sdb));
 
 	if (scsi_prot_sg_count(cmd))
-		scsi_free_sgtable(cmd->prot_sdb, false);
+		scsi_free_sgtable(&cmd->prot_sdb->table, false);
 }
 
 static void scsi_release_bidi_buffers(struct scsi_cmnd *cmd)
 {
 	struct scsi_data_buffer *bidi_sdb = cmd->request->next_rq->special;
 
-	scsi_free_sgtable(bidi_sdb, false);
+	scsi_free_sgtable(&bidi_sdb->table, false);
 	kmem_cache_free(scsi_sdb_cache, bidi_sdb);
 	cmd->request->next_rq->special = NULL;
 }
@@ -1085,7 +1090,7 @@ static int scsi_init_sgtable(struct request *req, struct scsi_data_buffer *sdb)
 	/*
 	 * If sg table allocation fails, requeue request later.
 	 */
-	if (unlikely(scsi_alloc_sgtable(sdb, req->nr_phys_segments,
+	if (unlikely(scsi_alloc_sgtable(&sdb->table, req->nr_phys_segments,
 					req->mq_ctx != NULL)))
 		return BLKPREP_DEFER;
 
@@ -1158,7 +1163,7 @@ int scsi_init_io(struct scsi_cmnd *cmd)
 
 		ivecs = blk_rq_count_integrity_sg(rq->q, rq->bio);
 
-		if (scsi_alloc_sgtable(prot_sdb, ivecs, is_mq)) {
+		if (scsi_alloc_sgtable(&prot_sdb->table, ivecs, is_mq)) {
 			error = BLKPREP_DEFER;
 			goto err_exit;
 		}
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 2/5] scsi: replace "mq" with "first_chunk" in SG functions
  2016-03-22 22:03 [PATCH v2 0/5] mempool based chained scatterlist alloc/free api Ming Lin
  2016-03-22 22:03 ` [PATCH v2 1/5] scsi: replace "scsi_data_buffer" with "sg_table" in SG functions Ming Lin
@ 2016-03-22 22:03 ` Ming Lin
  2016-03-22 22:03 ` [PATCH v2 3/5] scsi: rename SG related struct and functions Ming Lin
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 20+ messages in thread
From: Ming Lin @ 2016-03-22 22:03 UTC (permalink / raw)
  To: linux-kernel, linux-scsi; +Cc: Christoph Hellwig

From: Ming Lin <ming.l@ssi.samsung.com>

Parameter "bool mq" is block driver specific.
Change it to "first_chunk" to make it more generic.

Signed-off-by: Ming Lin <ming.l@ssi.samsung.com>
---
 drivers/scsi/scsi_lib.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 5eaddc7..ab3dd09 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -583,33 +583,32 @@ static struct scatterlist *scsi_sg_alloc(unsigned int nents, gfp_t gfp_mask)
 	return mempool_alloc(sgp->pool, gfp_mask);
 }
 
-static void scsi_free_sgtable(struct sg_table *table, bool mq)
+static void scsi_free_sgtable(struct sg_table *table, bool first_chunk)
 {
-	if (mq && table->orig_nents <= SCSI_MAX_SG_SEGMENTS)
+	if (first_chunk && table->orig_nents <= SCSI_MAX_SG_SEGMENTS)
 		return;
-	__sg_free_table(table, SCSI_MAX_SG_SEGMENTS, mq, scsi_sg_free);
+	__sg_free_table(table, SCSI_MAX_SG_SEGMENTS, first_chunk, scsi_sg_free);
 }
 
-static int scsi_alloc_sgtable(struct sg_table *table, int nents, bool mq)
+static int scsi_alloc_sgtable(struct sg_table *table, int nents,
+		struct scatterlist *first_chunk)
 {
-	struct scatterlist *first_chunk = NULL;
 	int ret;
 
 	BUG_ON(!nents);
 
-	if (mq) {
+	if (first_chunk) {
 		if (nents <= SCSI_MAX_SG_SEGMENTS) {
 			table->nents = table->orig_nents = nents;
 			sg_init_table(table->sgl, nents);
 			return 0;
 		}
-		first_chunk = table->sgl;
 	}
 
 	ret = __sg_alloc_table(table, nents, SCSI_MAX_SG_SEGMENTS,
 			       first_chunk, GFP_ATOMIC, scsi_sg_alloc);
 	if (unlikely(ret))
-		scsi_free_sgtable(table, mq);
+		scsi_free_sgtable(table, (bool)first_chunk);
 	return ret;
 }
 
@@ -1091,7 +1090,7 @@ static int scsi_init_sgtable(struct request *req, struct scsi_data_buffer *sdb)
 	 * If sg table allocation fails, requeue request later.
 	 */
 	if (unlikely(scsi_alloc_sgtable(&sdb->table, req->nr_phys_segments,
-					req->mq_ctx != NULL)))
+					sdb->table.sgl)))
 		return BLKPREP_DEFER;
 
 	/* 
@@ -1163,7 +1162,8 @@ int scsi_init_io(struct scsi_cmnd *cmd)
 
 		ivecs = blk_rq_count_integrity_sg(rq->q, rq->bio);
 
-		if (scsi_alloc_sgtable(&prot_sdb->table, ivecs, is_mq)) {
+		if (scsi_alloc_sgtable(&prot_sdb->table, ivecs,
+				prot_sdb->table.sgl)) {
 			error = BLKPREP_DEFER;
 			goto err_exit;
 		}
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 3/5] scsi: rename SG related struct and functions
  2016-03-22 22:03 [PATCH v2 0/5] mempool based chained scatterlist alloc/free api Ming Lin
  2016-03-22 22:03 ` [PATCH v2 1/5] scsi: replace "scsi_data_buffer" with "sg_table" in SG functions Ming Lin
  2016-03-22 22:03 ` [PATCH v2 2/5] scsi: replace "mq" with "first_chunk" " Ming Lin
@ 2016-03-22 22:03 ` Ming Lin
  2016-03-22 22:03 ` [PATCH v2 4/5] scsi: rename SCSI_MAX_{SG, SG_CHAIN}_SEGMENTS Ming Lin
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 20+ messages in thread
From: Ming Lin @ 2016-03-22 22:03 UTC (permalink / raw)
  To: linux-kernel, linux-scsi; +Cc: Christoph Hellwig

From: Ming Lin <ming.l@ssi.samsung.com>

Rename SCSI specific struct and functions to more genenic names.

Signed-off-by: Ming Lin <ming.l@ssi.samsung.com>
---
 drivers/scsi/scsi_lib.c | 52 ++++++++++++++++++++++++-------------------------
 1 file changed, 26 insertions(+), 26 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index ab3dd09..5fd9dd7 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -40,10 +40,10 @@
 #include "scsi_logging.h"
 
 
-#define SG_MEMPOOL_NR		ARRAY_SIZE(scsi_sg_pools)
+#define SG_MEMPOOL_NR		ARRAY_SIZE(sg_pools)
 #define SG_MEMPOOL_SIZE		2
 
-struct scsi_host_sg_pool {
+struct sg_pool {
 	size_t		size;
 	char		*name;
 	struct kmem_cache	*slab;
@@ -54,7 +54,7 @@ struct scsi_host_sg_pool {
 #if (SCSI_MAX_SG_SEGMENTS < 32)
 #error SCSI_MAX_SG_SEGMENTS is too small (must be 32 or greater)
 #endif
-static struct scsi_host_sg_pool scsi_sg_pools[] = {
+static struct sg_pool sg_pools[] = {
 	SP(8),
 	SP(16),
 #if (SCSI_MAX_SG_SEGMENTS > 32)
@@ -553,7 +553,7 @@ void scsi_run_host_queues(struct Scsi_Host *shost)
 		scsi_run_queue(sdev->request_queue);
 }
 
-static inline unsigned int scsi_sgtable_index(unsigned short nents)
+static inline unsigned int sg_pool_index(unsigned short nents)
 {
 	unsigned int index;
 
@@ -567,30 +567,30 @@ static inline unsigned int scsi_sgtable_index(unsigned short nents)
 	return index;
 }
 
-static void scsi_sg_free(struct scatterlist *sgl, unsigned int nents)
+static void sg_pool_free(struct scatterlist *sgl, unsigned int nents)
 {
-	struct scsi_host_sg_pool *sgp;
+	struct sg_pool *sgp;
 
-	sgp = scsi_sg_pools + scsi_sgtable_index(nents);
+	sgp = sg_pools + sg_pool_index(nents);
 	mempool_free(sgl, sgp->pool);
 }
 
-static struct scatterlist *scsi_sg_alloc(unsigned int nents, gfp_t gfp_mask)
+static struct scatterlist *sg_pool_alloc(unsigned int nents, gfp_t gfp_mask)
 {
-	struct scsi_host_sg_pool *sgp;
+	struct sg_pool *sgp;
 
-	sgp = scsi_sg_pools + scsi_sgtable_index(nents);
+	sgp = sg_pools + sg_pool_index(nents);
 	return mempool_alloc(sgp->pool, gfp_mask);
 }
 
-static void scsi_free_sgtable(struct sg_table *table, bool first_chunk)
+static void sg_free_table_chained(struct sg_table *table, bool first_chunk)
 {
 	if (first_chunk && table->orig_nents <= SCSI_MAX_SG_SEGMENTS)
 		return;
-	__sg_free_table(table, SCSI_MAX_SG_SEGMENTS, first_chunk, scsi_sg_free);
+	__sg_free_table(table, SCSI_MAX_SG_SEGMENTS, first_chunk, sg_pool_free);
 }
 
-static int scsi_alloc_sgtable(struct sg_table *table, int nents,
+static int sg_alloc_table_chained(struct sg_table *table, int nents,
 		struct scatterlist *first_chunk)
 {
 	int ret;
@@ -606,9 +606,9 @@ static int scsi_alloc_sgtable(struct sg_table *table, int nents,
 	}
 
 	ret = __sg_alloc_table(table, nents, SCSI_MAX_SG_SEGMENTS,
-			       first_chunk, GFP_ATOMIC, scsi_sg_alloc);
+			       first_chunk, GFP_ATOMIC, sg_pool_alloc);
 	if (unlikely(ret))
-		scsi_free_sgtable(table, (bool)first_chunk);
+		sg_free_table_chained(table, (bool)first_chunk);
 	return ret;
 }
 
@@ -627,14 +627,14 @@ static void scsi_mq_free_sgtables(struct scsi_cmnd *cmd)
 	struct scsi_data_buffer *sdb;
 
 	if (cmd->sdb.table.nents)
-		scsi_free_sgtable(&cmd->sdb.table, true);
+		sg_free_table_chained(&cmd->sdb.table, true);
 	if (cmd->request->next_rq) {
 		sdb = cmd->request->next_rq->special;
 		if (sdb)
-			scsi_free_sgtable(&sdb->table, true);
+			sg_free_table_chained(&sdb->table, true);
 	}
 	if (scsi_prot_sg_count(cmd))
-		scsi_free_sgtable(&cmd->prot_sdb->table, true);
+		sg_free_table_chained(&cmd->prot_sdb->table, true);
 }
 
 static void scsi_mq_uninit_cmd(struct scsi_cmnd *cmd)
@@ -673,19 +673,19 @@ static void scsi_mq_uninit_cmd(struct scsi_cmnd *cmd)
 static void scsi_release_buffers(struct scsi_cmnd *cmd)
 {
 	if (cmd->sdb.table.nents)
-		scsi_free_sgtable(&cmd->sdb.table, false);
+		sg_free_table_chained(&cmd->sdb.table, false);
 
 	memset(&cmd->sdb, 0, sizeof(cmd->sdb));
 
 	if (scsi_prot_sg_count(cmd))
-		scsi_free_sgtable(&cmd->prot_sdb->table, false);
+		sg_free_table_chained(&cmd->prot_sdb->table, false);
 }
 
 static void scsi_release_bidi_buffers(struct scsi_cmnd *cmd)
 {
 	struct scsi_data_buffer *bidi_sdb = cmd->request->next_rq->special;
 
-	scsi_free_sgtable(&bidi_sdb->table, false);
+	sg_free_table_chained(&bidi_sdb->table, false);
 	kmem_cache_free(scsi_sdb_cache, bidi_sdb);
 	cmd->request->next_rq->special = NULL;
 }
@@ -1089,7 +1089,7 @@ static int scsi_init_sgtable(struct request *req, struct scsi_data_buffer *sdb)
 	/*
 	 * If sg table allocation fails, requeue request later.
 	 */
-	if (unlikely(scsi_alloc_sgtable(&sdb->table, req->nr_phys_segments,
+	if (unlikely(sg_alloc_table_chained(&sdb->table, req->nr_phys_segments,
 					sdb->table.sgl)))
 		return BLKPREP_DEFER;
 
@@ -1162,7 +1162,7 @@ int scsi_init_io(struct scsi_cmnd *cmd)
 
 		ivecs = blk_rq_count_integrity_sg(rq->q, rq->bio);
 
-		if (scsi_alloc_sgtable(&prot_sdb->table, ivecs,
+		if (sg_alloc_table_chained(&prot_sdb->table, ivecs,
 				prot_sdb->table.sgl)) {
 			error = BLKPREP_DEFER;
 			goto err_exit;
@@ -2280,7 +2280,7 @@ int __init scsi_init_queue(void)
 	}
 
 	for (i = 0; i < SG_MEMPOOL_NR; i++) {
-		struct scsi_host_sg_pool *sgp = scsi_sg_pools + i;
+		struct sg_pool *sgp = sg_pools + i;
 		int size = sgp->size * sizeof(struct scatterlist);
 
 		sgp->slab = kmem_cache_create(sgp->name, size, 0,
@@ -2304,7 +2304,7 @@ int __init scsi_init_queue(void)
 
 cleanup_sdb:
 	for (i = 0; i < SG_MEMPOOL_NR; i++) {
-		struct scsi_host_sg_pool *sgp = scsi_sg_pools + i;
+		struct sg_pool *sgp = sg_pools + i;
 		if (sgp->pool)
 			mempool_destroy(sgp->pool);
 		if (sgp->slab)
@@ -2322,7 +2322,7 @@ void scsi_exit_queue(void)
 	kmem_cache_destroy(scsi_sdb_cache);
 
 	for (i = 0; i < SG_MEMPOOL_NR; i++) {
-		struct scsi_host_sg_pool *sgp = scsi_sg_pools + i;
+		struct sg_pool *sgp = sg_pools + i;
 		mempool_destroy(sgp->pool);
 		kmem_cache_destroy(sgp->slab);
 	}
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 4/5] scsi: rename SCSI_MAX_{SG, SG_CHAIN}_SEGMENTS
  2016-03-22 22:03 [PATCH v2 0/5] mempool based chained scatterlist alloc/free api Ming Lin
                   ` (2 preceding siblings ...)
  2016-03-22 22:03 ` [PATCH v2 3/5] scsi: rename SG related struct and functions Ming Lin
@ 2016-03-22 22:03 ` Ming Lin
  2016-04-04 14:22   ` Bart Van Assche
  2016-04-04 20:24   ` Ming Lin
  2016-03-22 22:03 ` [PATCH v2 5/5] lib: scatterlist: move SG pool code from SCSI driver to lib/sg_pool.c Ming Lin
  2016-03-23  7:40 ` [PATCH v2 0/5] mempool based chained scatterlist alloc/free api Christoph Hellwig
  5 siblings, 2 replies; 20+ messages in thread
From: Ming Lin @ 2016-03-22 22:03 UTC (permalink / raw)
  To: linux-kernel, linux-scsi; +Cc: Christoph Hellwig

From: Ming Lin <ming.l@ssi.samsung.com>

Rename SCSI_MAX_SG_SEGMENTS to SG_CHUNK_SIZE, which means the amount
we fit into a single scatterlist chunk.

Rename SCSI_MAX_SG_CHAIN_SEGMENTS to SG_MAX_SEGMENTS.

Will move these 2 generic definitions to scatterlist.h later.

Signed-off-by: Ming Lin <ming.l@ssi.samsung.com>
---
 drivers/ata/pata_icside.c           |  2 +-
 drivers/infiniband/ulp/srp/ib_srp.c |  4 ++--
 drivers/scsi/arm/cumana_2.c         |  2 +-
 drivers/scsi/arm/eesox.c            |  2 +-
 drivers/scsi/arm/powertec.c         |  2 +-
 drivers/scsi/esas2r/esas2r_main.c   |  4 ++--
 drivers/scsi/hisi_sas/hisi_sas.h    |  2 +-
 drivers/scsi/mpt3sas/mpt3sas_base.c |  4 ++--
 drivers/scsi/mpt3sas/mpt3sas_base.h |  2 +-
 drivers/scsi/scsi_debug.c           |  2 +-
 drivers/scsi/scsi_lib.c             | 34 +++++++++++++++++-----------------
 drivers/usb/storage/scsiglue.c      |  2 +-
 include/scsi/scsi.h                 |  8 ++++----
 include/scsi/scsi_host.h            |  2 +-
 14 files changed, 36 insertions(+), 36 deletions(-)

diff --git a/drivers/ata/pata_icside.c b/drivers/ata/pata_icside.c
index d7c7320..188f2f2 100644
--- a/drivers/ata/pata_icside.c
+++ b/drivers/ata/pata_icside.c
@@ -294,7 +294,7 @@ static int icside_dma_init(struct pata_icside_info *info)
 
 static struct scsi_host_template pata_icside_sht = {
 	ATA_BASE_SHT(DRV_NAME),
-	.sg_tablesize		= SCSI_MAX_SG_CHAIN_SEGMENTS,
+	.sg_tablesize		= SG_MAX_SEGMENTS,
 	.dma_boundary		= IOMD_DMA_BOUNDARY,
 };
 
diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
index 60b169a..b428b48 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -81,7 +81,7 @@ MODULE_PARM_DESC(cmd_sg_entries,
 
 module_param(indirect_sg_entries, uint, 0444);
 MODULE_PARM_DESC(indirect_sg_entries,
-		 "Default max number of gather/scatter entries (default is 12, max is " __stringify(SCSI_MAX_SG_CHAIN_SEGMENTS) ")");
+		 "Default max number of gather/scatter entries (default is 12, max is " __stringify(SG_MAX_SEGMENTS) ")");
 
 module_param(allow_ext_sg, bool, 0444);
 MODULE_PARM_DESC(allow_ext_sg,
@@ -3129,7 +3129,7 @@ static int srp_parse_options(const char *buf, struct srp_target_port *target)
 
 		case SRP_OPT_SG_TABLESIZE:
 			if (match_int(args, &token) || token < 1 ||
-					token > SCSI_MAX_SG_CHAIN_SEGMENTS) {
+					token > SG_MAX_SEGMENTS) {
 				pr_warn("bad max sg_tablesize parameter '%s'\n",
 					p);
 				goto out;
diff --git a/drivers/scsi/arm/cumana_2.c b/drivers/scsi/arm/cumana_2.c
index faa1bee..edce5f3 100644
--- a/drivers/scsi/arm/cumana_2.c
+++ b/drivers/scsi/arm/cumana_2.c
@@ -365,7 +365,7 @@ static struct scsi_host_template cumanascsi2_template = {
 	.eh_abort_handler		= fas216_eh_abort,
 	.can_queue			= 1,
 	.this_id			= 7,
-	.sg_tablesize			= SCSI_MAX_SG_CHAIN_SEGMENTS,
+	.sg_tablesize			= SG_MAX_SEGMENTS,
 	.dma_boundary			= IOMD_DMA_BOUNDARY,
 	.use_clustering			= DISABLE_CLUSTERING,
 	.proc_name			= "cumanascsi2",
diff --git a/drivers/scsi/arm/eesox.c b/drivers/scsi/arm/eesox.c
index a8ad688..e93e047 100644
--- a/drivers/scsi/arm/eesox.c
+++ b/drivers/scsi/arm/eesox.c
@@ -484,7 +484,7 @@ static struct scsi_host_template eesox_template = {
 	.eh_abort_handler		= fas216_eh_abort,
 	.can_queue			= 1,
 	.this_id			= 7,
-	.sg_tablesize			= SCSI_MAX_SG_CHAIN_SEGMENTS,
+	.sg_tablesize			= SG_MAX_SEGMENTS,
 	.dma_boundary			= IOMD_DMA_BOUNDARY,
 	.use_clustering			= DISABLE_CLUSTERING,
 	.proc_name			= "eesox",
diff --git a/drivers/scsi/arm/powertec.c b/drivers/scsi/arm/powertec.c
index 5e1b73e..79aa889 100644
--- a/drivers/scsi/arm/powertec.c
+++ b/drivers/scsi/arm/powertec.c
@@ -291,7 +291,7 @@ static struct scsi_host_template powertecscsi_template = {
 
 	.can_queue			= 8,
 	.this_id			= 7,
-	.sg_tablesize			= SCSI_MAX_SG_CHAIN_SEGMENTS,
+	.sg_tablesize			= SG_MAX_SEGMENTS,
 	.dma_boundary			= IOMD_DMA_BOUNDARY,
 	.cmd_per_lun			= 2,
 	.use_clustering			= ENABLE_CLUSTERING,
diff --git a/drivers/scsi/esas2r/esas2r_main.c b/drivers/scsi/esas2r/esas2r_main.c
index 33581ba..2aca4d1 100644
--- a/drivers/scsi/esas2r/esas2r_main.c
+++ b/drivers/scsi/esas2r/esas2r_main.c
@@ -246,7 +246,7 @@ static struct scsi_host_template driver_template = {
 	.eh_target_reset_handler	= esas2r_target_reset,
 	.can_queue			= 128,
 	.this_id			= -1,
-	.sg_tablesize			= SCSI_MAX_SG_SEGMENTS,
+	.sg_tablesize			= SG_CHUNK_SIZE,
 	.cmd_per_lun			=
 		ESAS2R_DEFAULT_CMD_PER_LUN,
 	.present			= 0,
@@ -271,7 +271,7 @@ module_param(num_sg_lists, int, 0);
 MODULE_PARM_DESC(num_sg_lists,
 		 "Number of scatter/gather lists.  Default 1024.");
 
-int sg_tablesize = SCSI_MAX_SG_SEGMENTS;
+int sg_tablesize = SG_CHUNK_SIZE;
 module_param(sg_tablesize, int, 0);
 MODULE_PARM_DESC(sg_tablesize,
 		 "Maximum number of entries in a scatter/gather table.");
diff --git a/drivers/scsi/hisi_sas/hisi_sas.h b/drivers/scsi/hisi_sas/hisi_sas.h
index 5af2e41..788a136 100644
--- a/drivers/scsi/hisi_sas/hisi_sas.h
+++ b/drivers/scsi/hisi_sas/hisi_sas.h
@@ -307,7 +307,7 @@ struct hisi_sas_command_table_stp {
 	u8	atapi_cdb[ATAPI_CDB_LEN];
 };
 
-#define HISI_SAS_SGE_PAGE_CNT SCSI_MAX_SG_SEGMENTS
+#define HISI_SAS_SGE_PAGE_CNT SG_CHUNK_SIZE
 struct hisi_sas_sge_page {
 	struct hisi_sas_sge sge[HISI_SAS_SGE_PAGE_CNT];
 };
diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c
index 83658ac..f4cf9df 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -3177,10 +3177,10 @@ _base_allocate_memory_pools(struct MPT3SAS_ADAPTER *ioc,  int sleep_flag)
 		sg_tablesize = MPT_MIN_PHYS_SEGMENTS;
 	else if (sg_tablesize > MPT_MAX_PHYS_SEGMENTS) {
 		sg_tablesize = min_t(unsigned short, sg_tablesize,
-				      SCSI_MAX_SG_CHAIN_SEGMENTS);
+				      SG_MAX_SEGMENTS);
 		pr_warn(MPT3SAS_FMT
 		 "sg_tablesize(%u) is bigger than kernel"
-		 " defined SCSI_MAX_SG_SEGMENTS(%u)\n", ioc->name,
+		 " defined SG_CHUNK_SIZE(%u)\n", ioc->name,
 		 sg_tablesize, MPT_MAX_PHYS_SEGMENTS);
 	}
 	ioc->shost->sg_tablesize = sg_tablesize;
diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.h b/drivers/scsi/mpt3sas/mpt3sas_base.h
index 5ad271e..5925ce3 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.h
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.h
@@ -90,7 +90,7 @@
 /*
  * Set MPT3SAS_SG_DEPTH value based on user input.
  */
-#define MPT_MAX_PHYS_SEGMENTS	SCSI_MAX_SG_SEGMENTS
+#define MPT_MAX_PHYS_SEGMENTS	SG_CHUNK_SIZE
 #define MPT_MIN_PHYS_SEGMENTS	16
 
 #ifdef CONFIG_SCSI_MPT3SAS_MAX_SGE
diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index f3d69a98..06b1517 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -5305,7 +5305,7 @@ static struct scsi_host_template sdebug_driver_template = {
 	.eh_host_reset_handler = scsi_debug_host_reset,
 	.can_queue =		SCSI_DEBUG_CANQUEUE,
 	.this_id =		7,
-	.sg_tablesize =		SCSI_MAX_SG_CHAIN_SEGMENTS,
+	.sg_tablesize =		SG_MAX_SEGMENTS,
 	.cmd_per_lun =		DEF_CMD_PER_LUN,
 	.max_sectors =		-1U,
 	.use_clustering = 	DISABLE_CLUSTERING,
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 5fd9dd7..478b0e8 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -51,25 +51,25 @@ struct sg_pool {
 };
 
 #define SP(x) { .size = x, "sgpool-" __stringify(x) }
-#if (SCSI_MAX_SG_SEGMENTS < 32)
-#error SCSI_MAX_SG_SEGMENTS is too small (must be 32 or greater)
+#if (SG_CHUNK_SIZE < 32)
+#error SG_CHUNK_SIZE is too small (must be 32 or greater)
 #endif
 static struct sg_pool sg_pools[] = {
 	SP(8),
 	SP(16),
-#if (SCSI_MAX_SG_SEGMENTS > 32)
+#if (SG_CHUNK_SIZE > 32)
 	SP(32),
-#if (SCSI_MAX_SG_SEGMENTS > 64)
+#if (SG_CHUNK_SIZE > 64)
 	SP(64),
-#if (SCSI_MAX_SG_SEGMENTS > 128)
+#if (SG_CHUNK_SIZE > 128)
 	SP(128),
-#if (SCSI_MAX_SG_SEGMENTS > 256)
-#error SCSI_MAX_SG_SEGMENTS is too large (256 MAX)
+#if (SG_CHUNK_SIZE > 256)
+#error SG_CHUNK_SIZE is too large (256 MAX)
 #endif
 #endif
 #endif
 #endif
-	SP(SCSI_MAX_SG_SEGMENTS)
+	SP(SG_CHUNK_SIZE)
 };
 #undef SP
 
@@ -557,7 +557,7 @@ static inline unsigned int sg_pool_index(unsigned short nents)
 {
 	unsigned int index;
 
-	BUG_ON(nents > SCSI_MAX_SG_SEGMENTS);
+	BUG_ON(nents > SG_CHUNK_SIZE);
 
 	if (nents <= 8)
 		index = 0;
@@ -585,9 +585,9 @@ static struct scatterlist *sg_pool_alloc(unsigned int nents, gfp_t gfp_mask)
 
 static void sg_free_table_chained(struct sg_table *table, bool first_chunk)
 {
-	if (first_chunk && table->orig_nents <= SCSI_MAX_SG_SEGMENTS)
+	if (first_chunk && table->orig_nents <= SG_CHUNK_SIZE)
 		return;
-	__sg_free_table(table, SCSI_MAX_SG_SEGMENTS, first_chunk, sg_pool_free);
+	__sg_free_table(table, SG_CHUNK_SIZE, first_chunk, sg_pool_free);
 }
 
 static int sg_alloc_table_chained(struct sg_table *table, int nents,
@@ -598,14 +598,14 @@ static int sg_alloc_table_chained(struct sg_table *table, int nents,
 	BUG_ON(!nents);
 
 	if (first_chunk) {
-		if (nents <= SCSI_MAX_SG_SEGMENTS) {
+		if (nents <= SG_CHUNK_SIZE) {
 			table->nents = table->orig_nents = nents;
 			sg_init_table(table->sgl, nents);
 			return 0;
 		}
 	}
 
-	ret = __sg_alloc_table(table, nents, SCSI_MAX_SG_SEGMENTS,
+	ret = __sg_alloc_table(table, nents, SG_CHUNK_SIZE,
 			       first_chunk, GFP_ATOMIC, sg_pool_alloc);
 	if (unlikely(ret))
 		sg_free_table_chained(table, (bool)first_chunk);
@@ -1937,7 +1937,7 @@ static int scsi_mq_prep_fn(struct request *req)
 	if (scsi_host_get_prot(shost)) {
 		cmd->prot_sdb = (void *)sg +
 			min_t(unsigned int,
-			      shost->sg_tablesize, SCSI_MAX_SG_SEGMENTS) *
+			      shost->sg_tablesize, SG_CHUNK_SIZE) *
 			sizeof(struct scatterlist);
 		memset(cmd->prot_sdb, 0, sizeof(struct scsi_data_buffer));
 
@@ -2110,7 +2110,7 @@ static void __scsi_init_queue(struct Scsi_Host *shost, struct request_queue *q)
 	 * this limit is imposed by hardware restrictions
 	 */
 	blk_queue_max_segments(q, min_t(unsigned short, shost->sg_tablesize,
-					SCSI_MAX_SG_CHAIN_SEGMENTS));
+					SG_MAX_SEGMENTS));
 
 	if (scsi_host_prot_dma(shost)) {
 		shost->sg_prot_tablesize =
@@ -2192,8 +2192,8 @@ int scsi_mq_setup_tags(struct Scsi_Host *shost)
 	unsigned int cmd_size, sgl_size, tbl_size;
 
 	tbl_size = shost->sg_tablesize;
-	if (tbl_size > SCSI_MAX_SG_SEGMENTS)
-		tbl_size = SCSI_MAX_SG_SEGMENTS;
+	if (tbl_size > SG_CHUNK_SIZE)
+		tbl_size = SG_CHUNK_SIZE;
 	sgl_size = tbl_size * sizeof(struct scatterlist);
 	cmd_size = sizeof(struct scsi_cmnd) + shost->hostt->cmd_size + sgl_size;
 	if (scsi_host_get_prot(shost))
diff --git a/drivers/usb/storage/scsiglue.c b/drivers/usb/storage/scsiglue.c
index dba5136..82e5eddc 100644
--- a/drivers/usb/storage/scsiglue.c
+++ b/drivers/usb/storage/scsiglue.c
@@ -563,7 +563,7 @@ static const struct scsi_host_template usb_stor_host_template = {
 	.target_alloc =			target_alloc,
 
 	/* lots of sg segments can be handled */
-	.sg_tablesize =			SCSI_MAX_SG_CHAIN_SEGMENTS,
+	.sg_tablesize =			SG_MAX_SEGMENTS,
 
 	/* limit the total size of a transfer to 120 KB */
 	.max_sectors =                  240,
diff --git a/include/scsi/scsi.h b/include/scsi/scsi.h
index e0a3398..74dafa7 100644
--- a/include/scsi/scsi.h
+++ b/include/scsi/scsi.h
@@ -24,16 +24,16 @@ enum scsi_timeouts {
  * to SG_MAX_SINGLE_ALLOC to pack correctly at the highest order.  The
  * minimum value is 32
  */
-#define SCSI_MAX_SG_SEGMENTS	128
+#define SG_CHUNK_SIZE	128
 
 /*
- * Like SCSI_MAX_SG_SEGMENTS, but for archs that have sg chaining. This limit
+ * Like SG_CHUNK_SIZE, but for archs that have sg chaining. This limit
  * is totally arbitrary, a setting of 2048 will get you at least 8mb ios.
  */
 #ifdef CONFIG_ARCH_HAS_SG_CHAIN
-#define SCSI_MAX_SG_CHAIN_SEGMENTS	2048
+#define SG_MAX_SEGMENTS	2048
 #else
-#define SCSI_MAX_SG_CHAIN_SEGMENTS	SCSI_MAX_SG_SEGMENTS
+#define SG_MAX_SEGMENTS	SG_CHUNK_SIZE
 #endif
 
 /*
diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h
index fcfa3d7..76e9d27 100644
--- a/include/scsi/scsi_host.h
+++ b/include/scsi/scsi_host.h
@@ -37,7 +37,7 @@ struct blk_queue_tags;
  *	 used in one scatter-gather request.
  */
 #define SG_NONE 0
-#define SG_ALL	SCSI_MAX_SG_SEGMENTS
+#define SG_ALL	SG_CHUNK_SIZE
 
 #define MODE_UNKNOWN 0x00
 #define MODE_INITIATOR 0x01
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 5/5] lib: scatterlist: move SG pool code from SCSI driver to lib/sg_pool.c
  2016-03-22 22:03 [PATCH v2 0/5] mempool based chained scatterlist alloc/free api Ming Lin
                   ` (3 preceding siblings ...)
  2016-03-22 22:03 ` [PATCH v2 4/5] scsi: rename SCSI_MAX_{SG, SG_CHAIN}_SEGMENTS Ming Lin
@ 2016-03-22 22:03 ` Ming Lin
  2016-03-23  2:38   ` [PATCH] lib: scatterlist: fix ifnullfree.cocci warnings kbuild test robot
  2016-03-23  2:38   ` [PATCH v2 5/5] lib: scatterlist: move SG pool code from SCSI driver to lib/sg_pool.c kbuild test robot
  2016-03-23  7:40 ` [PATCH v2 0/5] mempool based chained scatterlist alloc/free api Christoph Hellwig
  5 siblings, 2 replies; 20+ messages in thread
From: Ming Lin @ 2016-03-22 22:03 UTC (permalink / raw)
  To: linux-kernel, linux-scsi; +Cc: Christoph Hellwig

From: Ming Lin <ming.l@ssi.samsung.com>

Now it's ready to move the mempool based SG chained allocator code from
SCSI driver to lib/sg_pool.c, which will be compiled only based on a Kconfig
symbol CONFIG_SG_POOL.

SCSI selects CONFIG_SG_POOL.

Signed-off-by: Ming Lin <ming.l@ssi.samsung.com>
---
 drivers/scsi/Kconfig        |   1 +
 drivers/scsi/scsi_lib.c     | 137 -----------------------------------
 include/linux/scatterlist.h |  25 +++++++
 include/scsi/scsi.h         |  19 -----
 lib/Kconfig                 |   7 ++
 lib/Makefile                |   1 +
 lib/sg_pool.c               | 172 ++++++++++++++++++++++++++++++++++++++++++++
 7 files changed, 206 insertions(+), 156 deletions(-)
 create mode 100644 lib/sg_pool.c

diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
index e2f31c9..ee9cf41 100644
--- a/drivers/scsi/Kconfig
+++ b/drivers/scsi/Kconfig
@@ -17,6 +17,7 @@ config SCSI
 	tristate "SCSI device support"
 	depends on BLOCK
 	select SCSI_DMA if HAS_DMA
+	select SG_POOL
 	---help---
 	  If you want to use a SCSI hard disk, SCSI tape drive, SCSI CD-ROM or
 	  any other SCSI device under Linux, say Y and make sure that you know
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 478b0e8..6d818e8 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -14,8 +14,6 @@
 #include <linux/completion.h>
 #include <linux/kernel.h>
 #include <linux/export.h>
-#include <linux/mempool.h>
-#include <linux/slab.h>
 #include <linux/init.h>
 #include <linux/pci.h>
 #include <linux/delay.h>
@@ -40,39 +38,6 @@
 #include "scsi_logging.h"
 
 
-#define SG_MEMPOOL_NR		ARRAY_SIZE(sg_pools)
-#define SG_MEMPOOL_SIZE		2
-
-struct sg_pool {
-	size_t		size;
-	char		*name;
-	struct kmem_cache	*slab;
-	mempool_t	*pool;
-};
-
-#define SP(x) { .size = x, "sgpool-" __stringify(x) }
-#if (SG_CHUNK_SIZE < 32)
-#error SG_CHUNK_SIZE is too small (must be 32 or greater)
-#endif
-static struct sg_pool sg_pools[] = {
-	SP(8),
-	SP(16),
-#if (SG_CHUNK_SIZE > 32)
-	SP(32),
-#if (SG_CHUNK_SIZE > 64)
-	SP(64),
-#if (SG_CHUNK_SIZE > 128)
-	SP(128),
-#if (SG_CHUNK_SIZE > 256)
-#error SG_CHUNK_SIZE is too large (256 MAX)
-#endif
-#endif
-#endif
-#endif
-	SP(SG_CHUNK_SIZE)
-};
-#undef SP
-
 struct kmem_cache *scsi_sdb_cache;
 
 /*
@@ -553,65 +518,6 @@ void scsi_run_host_queues(struct Scsi_Host *shost)
 		scsi_run_queue(sdev->request_queue);
 }
 
-static inline unsigned int sg_pool_index(unsigned short nents)
-{
-	unsigned int index;
-
-	BUG_ON(nents > SG_CHUNK_SIZE);
-
-	if (nents <= 8)
-		index = 0;
-	else
-		index = get_count_order(nents) - 3;
-
-	return index;
-}
-
-static void sg_pool_free(struct scatterlist *sgl, unsigned int nents)
-{
-	struct sg_pool *sgp;
-
-	sgp = sg_pools + sg_pool_index(nents);
-	mempool_free(sgl, sgp->pool);
-}
-
-static struct scatterlist *sg_pool_alloc(unsigned int nents, gfp_t gfp_mask)
-{
-	struct sg_pool *sgp;
-
-	sgp = sg_pools + sg_pool_index(nents);
-	return mempool_alloc(sgp->pool, gfp_mask);
-}
-
-static void sg_free_table_chained(struct sg_table *table, bool first_chunk)
-{
-	if (first_chunk && table->orig_nents <= SG_CHUNK_SIZE)
-		return;
-	__sg_free_table(table, SG_CHUNK_SIZE, first_chunk, sg_pool_free);
-}
-
-static int sg_alloc_table_chained(struct sg_table *table, int nents,
-		struct scatterlist *first_chunk)
-{
-	int ret;
-
-	BUG_ON(!nents);
-
-	if (first_chunk) {
-		if (nents <= SG_CHUNK_SIZE) {
-			table->nents = table->orig_nents = nents;
-			sg_init_table(table->sgl, nents);
-			return 0;
-		}
-	}
-
-	ret = __sg_alloc_table(table, nents, SG_CHUNK_SIZE,
-			       first_chunk, GFP_ATOMIC, sg_pool_alloc);
-	if (unlikely(ret))
-		sg_free_table_chained(table, (bool)first_chunk);
-	return ret;
-}
-
 static void scsi_uninit_cmd(struct scsi_cmnd *cmd)
 {
 	if (cmd->request->cmd_type == REQ_TYPE_FS) {
@@ -2269,8 +2175,6 @@ EXPORT_SYMBOL(scsi_unblock_requests);
 
 int __init scsi_init_queue(void)
 {
-	int i;
-
 	scsi_sdb_cache = kmem_cache_create("scsi_data_buffer",
 					   sizeof(struct scsi_data_buffer),
 					   0, 0, NULL);
@@ -2279,53 +2183,12 @@ int __init scsi_init_queue(void)
 		return -ENOMEM;
 	}
 
-	for (i = 0; i < SG_MEMPOOL_NR; i++) {
-		struct sg_pool *sgp = sg_pools + i;
-		int size = sgp->size * sizeof(struct scatterlist);
-
-		sgp->slab = kmem_cache_create(sgp->name, size, 0,
-				SLAB_HWCACHE_ALIGN, NULL);
-		if (!sgp->slab) {
-			printk(KERN_ERR "SCSI: can't init sg slab %s\n",
-					sgp->name);
-			goto cleanup_sdb;
-		}
-
-		sgp->pool = mempool_create_slab_pool(SG_MEMPOOL_SIZE,
-						     sgp->slab);
-		if (!sgp->pool) {
-			printk(KERN_ERR "SCSI: can't init sg mempool %s\n",
-					sgp->name);
-			goto cleanup_sdb;
-		}
-	}
-
 	return 0;
-
-cleanup_sdb:
-	for (i = 0; i < SG_MEMPOOL_NR; i++) {
-		struct sg_pool *sgp = sg_pools + i;
-		if (sgp->pool)
-			mempool_destroy(sgp->pool);
-		if (sgp->slab)
-			kmem_cache_destroy(sgp->slab);
-	}
-	kmem_cache_destroy(scsi_sdb_cache);
-
-	return -ENOMEM;
 }
 
 void scsi_exit_queue(void)
 {
-	int i;
-
 	kmem_cache_destroy(scsi_sdb_cache);
-
-	for (i = 0; i < SG_MEMPOOL_NR; i++) {
-		struct sg_pool *sgp = sg_pools + i;
-		mempool_destroy(sgp->pool);
-		kmem_cache_destroy(sgp->slab);
-	}
 }
 
 /**
diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h
index 556ec1e..cb3c8fe 100644
--- a/include/linux/scatterlist.h
+++ b/include/linux/scatterlist.h
@@ -286,6 +286,31 @@ size_t sg_pcopy_to_buffer(struct scatterlist *sgl, unsigned int nents,
 #define SG_MAX_SINGLE_ALLOC		(PAGE_SIZE / sizeof(struct scatterlist))
 
 /*
+ * The maximum number of SG segments that we will put inside a
+ * scatterlist (unless chaining is used). Should ideally fit inside a
+ * single page, to avoid a higher order allocation.  We could define this
+ * to SG_MAX_SINGLE_ALLOC to pack correctly at the highest order.  The
+ * minimum value is 32
+ */
+#define SG_CHUNK_SIZE	128
+
+/*
+ * Like SG_CHUNK_SIZE, but for archs that have sg chaining. This limit
+ * is totally arbitrary, a setting of 2048 will get you at least 8mb ios.
+ */
+#ifdef CONFIG_ARCH_HAS_SG_CHAIN
+#define SG_MAX_SEGMENTS	2048
+#else
+#define SG_MAX_SEGMENTS	SG_CHUNK_SIZE
+#endif
+
+#ifdef CONFIG_SG_POOL
+void sg_free_table_chained(struct sg_table *table, bool first_chunk);
+int sg_alloc_table_chained(struct sg_table *table, int nents,
+			   struct scatterlist *first_chunk);
+#endif
+
+/*
  * sg page iterator
  *
  * Iterates over sg entries page-by-page.  On each successful iteration,
diff --git a/include/scsi/scsi.h b/include/scsi/scsi.h
index 74dafa7..8ec7c30 100644
--- a/include/scsi/scsi.h
+++ b/include/scsi/scsi.h
@@ -18,25 +18,6 @@ enum scsi_timeouts {
 };
 
 /*
- * The maximum number of SG segments that we will put inside a
- * scatterlist (unless chaining is used). Should ideally fit inside a
- * single page, to avoid a higher order allocation.  We could define this
- * to SG_MAX_SINGLE_ALLOC to pack correctly at the highest order.  The
- * minimum value is 32
- */
-#define SG_CHUNK_SIZE	128
-
-/*
- * Like SG_CHUNK_SIZE, but for archs that have sg chaining. This limit
- * is totally arbitrary, a setting of 2048 will get you at least 8mb ios.
- */
-#ifdef CONFIG_ARCH_HAS_SG_CHAIN
-#define SG_MAX_SEGMENTS	2048
-#else
-#define SG_MAX_SEGMENTS	SG_CHUNK_SIZE
-#endif
-
-/*
  * DIX-capable adapters effectively support infinite chaining for the
  * protection information scatterlist
  */
diff --git a/lib/Kconfig b/lib/Kconfig
index 133ebc0..3ee40b2 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -523,6 +523,13 @@ config SG_SPLIT
 	 a scatterlist. This should be selected by a driver or an API which
 	 whishes to split a scatterlist amongst multiple DMA channels.
 
+config SG_POOL
+	def_bool n
+	help
+	 Provides a helper to allocate chained scatterlists. This should be
+	 selected by a driver or an API which whishes to allocate chained
+	 scatterlist.
+
 #
 # sg chaining option
 #
diff --git a/lib/Makefile b/lib/Makefile
index a7c26a4..377876e 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -164,6 +164,7 @@ obj-$(CONFIG_GENERIC_STRNLEN_USER) += strnlen_user.o
 obj-$(CONFIG_GENERIC_NET_UTILS) += net_utils.o
 
 obj-$(CONFIG_SG_SPLIT) += sg_split.o
+obj-$(CONFIG_SG_POOL) += sg_pool.o
 obj-$(CONFIG_STMP_DEVICE) += stmp_device.o
 obj-$(CONFIG_IRQ_POLL) += irq_poll.o
 
diff --git a/lib/sg_pool.c b/lib/sg_pool.c
new file mode 100644
index 0000000..6dd3061
--- /dev/null
+++ b/lib/sg_pool.c
@@ -0,0 +1,172 @@
+#include <linux/module.h>
+#include <linux/scatterlist.h>
+#include <linux/mempool.h>
+#include <linux/slab.h>
+
+#define SG_MEMPOOL_NR		ARRAY_SIZE(sg_pools)
+#define SG_MEMPOOL_SIZE		2
+
+struct sg_pool {
+	size_t		size;
+	char		*name;
+	struct kmem_cache	*slab;
+	mempool_t	*pool;
+};
+
+#define SP(x) { .size = x, "sgpool-" __stringify(x) }
+#if (SG_CHUNK_SIZE < 32)
+#error SG_CHUNK_SIZE is too small (must be 32 or greater)
+#endif
+static struct sg_pool sg_pools[] = {
+	SP(8),
+	SP(16),
+#if (SG_CHUNK_SIZE > 32)
+	SP(32),
+#if (SG_CHUNK_SIZE > 64)
+	SP(64),
+#if (SG_CHUNK_SIZE > 128)
+	SP(128),
+#if (SG_CHUNK_SIZE > 256)
+#error SG_CHUNK_SIZE is too large (256 MAX)
+#endif
+#endif
+#endif
+#endif
+	SP(SG_CHUNK_SIZE)
+};
+#undef SP
+
+static inline unsigned int sg_pool_index(unsigned short nents)
+{
+	unsigned int index;
+
+	BUG_ON(nents > SG_CHUNK_SIZE);
+
+	if (nents <= 8)
+		index = 0;
+	else
+		index = get_count_order(nents) - 3;
+
+	return index;
+}
+
+static void sg_pool_free(struct scatterlist *sgl, unsigned int nents)
+{
+	struct sg_pool *sgp;
+
+	sgp = sg_pools + sg_pool_index(nents);
+	mempool_free(sgl, sgp->pool);
+}
+
+static struct scatterlist *sg_pool_alloc(unsigned int nents, gfp_t gfp_mask)
+{
+	struct sg_pool *sgp;
+
+	sgp = sg_pools + sg_pool_index(nents);
+	return mempool_alloc(sgp->pool, 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?
+ *
+ *  Description:
+ *    Free an sg table previously allocated and setup with
+ *    sg_alloc_table_chained().
+ *
+ **/
+void sg_free_table_chained(struct sg_table *table, bool first_chunk)
+{
+	if (first_chunk && table->orig_nents <= SG_CHUNK_SIZE)
+		return;
+	__sg_free_table(table, SG_CHUNK_SIZE, first_chunk, sg_pool_free);
+}
+EXPORT_SYMBOL_GPL(sg_free_table_chained);
+
+/**
+ * sg_alloc_table_chained - Allocate and chain SGLs in an sg table
+ * @table:	The sg table header to use
+ * @nents:	Number of entries in sg list
+ * @first_chunk: first SGL
+ *
+ *  Description:
+ *    Allocate and chain SGLs in an sg table. If @nents@ is larger than
+ *    SG_CHUNK_SIZE a chained sg table will be setup.
+ *
+ **/
+int sg_alloc_table_chained(struct sg_table *table, int nents,
+		struct scatterlist *first_chunk)
+{
+	int ret;
+
+	BUG_ON(!nents);
+
+	if (first_chunk) {
+		if (nents <= SG_CHUNK_SIZE) {
+			table->nents = table->orig_nents = nents;
+			sg_init_table(table->sgl, nents);
+			return 0;
+		}
+	}
+
+	ret = __sg_alloc_table(table, nents, SG_CHUNK_SIZE,
+			       first_chunk, GFP_ATOMIC, sg_pool_alloc);
+	if (unlikely(ret))
+		sg_free_table_chained(table, (bool)first_chunk);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(sg_alloc_table_chained);
+
+static __init int sg_pool_init(void)
+{
+	int i;
+
+	for (i = 0; i < SG_MEMPOOL_NR; i++) {
+		struct sg_pool *sgp = sg_pools + i;
+		int size = sgp->size * sizeof(struct scatterlist);
+
+		sgp->slab = kmem_cache_create(sgp->name, size, 0,
+				SLAB_HWCACHE_ALIGN, NULL);
+		if (!sgp->slab) {
+			printk(KERN_ERR "SG_POOL: can't init sg slab %s\n",
+					sgp->name);
+			goto cleanup_sdb;
+		}
+
+		sgp->pool = mempool_create_slab_pool(SG_MEMPOOL_SIZE,
+						     sgp->slab);
+		if (!sgp->pool) {
+			printk(KERN_ERR "SG_POOL: can't init sg mempool %s\n",
+					sgp->name);
+			goto cleanup_sdb;
+		}
+	}
+
+	return 0;
+
+cleanup_sdb:
+	for (i = 0; i < SG_MEMPOOL_NR; i++) {
+		struct sg_pool *sgp = sg_pools + i;
+		if (sgp->pool)
+			mempool_destroy(sgp->pool);
+		if (sgp->slab)
+			kmem_cache_destroy(sgp->slab);
+	}
+
+	return -ENOMEM;
+}
+
+static __exit void sg_pool_exit(void)
+{
+	int i;
+
+	for (i = 0; i < SG_MEMPOOL_NR; i++) {
+		struct sg_pool *sgp = sg_pools + i;
+		mempool_destroy(sgp->pool);
+		kmem_cache_destroy(sgp->slab);
+	}
+}
+
+module_init(sg_pool_init);
+module_exit(sg_pool_exit);
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 5/5] lib: scatterlist: move SG pool code from SCSI driver to lib/sg_pool.c
  2016-03-22 22:03 ` [PATCH v2 5/5] lib: scatterlist: move SG pool code from SCSI driver to lib/sg_pool.c Ming Lin
  2016-03-23  2:38   ` [PATCH] lib: scatterlist: fix ifnullfree.cocci warnings kbuild test robot
@ 2016-03-23  2:38   ` kbuild test robot
  2016-04-04 20:15     ` Ming Lin
  1 sibling, 1 reply; 20+ messages in thread
From: kbuild test robot @ 2016-03-23  2:38 UTC (permalink / raw)
  To: Ming Lin; +Cc: kbuild-all, linux-kernel, linux-scsi, Christoph Hellwig

Hi Ming,

[auto build test WARNING on scsi/for-next]
[also build test WARNING on v4.5 next-20160322]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]

url:    https://github.com/0day-ci/linux/commits/Ming-Lin/mempool-based-chained-scatterlist-alloc-free-api/20160323-060710
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git for-next


coccinelle warnings: (new ones prefixed by >>)

>> lib/sg_pool.c:152:3-18: WARNING: NULL check before freeing functions like kfree, debugfs_remove, debugfs_remove_recursive or usb_free_urb is not needed. Maybe consider reorganizing relevant code to avoid passing NULL values.
   lib/sg_pool.c:154:3-21: WARNING: NULL check before freeing functions like kfree, debugfs_remove, debugfs_remove_recursive or usb_free_urb is not needed. Maybe consider reorganizing relevant code to avoid passing NULL values.

Please review and possibly fold the followup patch.

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH] lib: scatterlist: fix ifnullfree.cocci warnings
  2016-03-22 22:03 ` [PATCH v2 5/5] lib: scatterlist: move SG pool code from SCSI driver to lib/sg_pool.c Ming Lin
@ 2016-03-23  2:38   ` kbuild test robot
  2016-03-23  2:38   ` [PATCH v2 5/5] lib: scatterlist: move SG pool code from SCSI driver to lib/sg_pool.c kbuild test robot
  1 sibling, 0 replies; 20+ messages in thread
From: kbuild test robot @ 2016-03-23  2:38 UTC (permalink / raw)
  To: Ming Lin; +Cc: kbuild-all, linux-kernel, linux-scsi, Christoph Hellwig

lib/sg_pool.c:152:3-18: WARNING: NULL check before freeing functions like kfree, debugfs_remove, debugfs_remove_recursive or usb_free_urb is not needed. Maybe consider reorganizing relevant code to avoid passing NULL values.
lib/sg_pool.c:154:3-21: WARNING: NULL check before freeing functions like kfree, debugfs_remove, debugfs_remove_recursive or usb_free_urb is not needed. Maybe consider reorganizing relevant code to avoid passing NULL values.

 NULL check before some freeing functions is not needed.

 Based on checkpatch warning
 "kfree(NULL) is safe this check is probably not required"
 and kfreeaddr.cocci by Julia Lawall.

Generated by: scripts/coccinelle/free/ifnullfree.cocci

CC: Ming Lin <ming.l@ssi.samsung.com>
Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
---

 sg_pool.c |    6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

--- a/lib/sg_pool.c
+++ b/lib/sg_pool.c
@@ -148,10 +148,8 @@ static __init int sg_pool_init(void)
 cleanup_sdb:
 	for (i = 0; i < SG_MEMPOOL_NR; i++) {
 		struct sg_pool *sgp = sg_pools + i;
-		if (sgp->pool)
-			mempool_destroy(sgp->pool);
-		if (sgp->slab)
-			kmem_cache_destroy(sgp->slab);
+		mempool_destroy(sgp->pool);
+		kmem_cache_destroy(sgp->slab);
 	}
 
 	return -ENOMEM;

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 0/5] mempool based chained scatterlist alloc/free api
  2016-03-22 22:03 [PATCH v2 0/5] mempool based chained scatterlist alloc/free api Ming Lin
                   ` (4 preceding siblings ...)
  2016-03-22 22:03 ` [PATCH v2 5/5] lib: scatterlist: move SG pool code from SCSI driver to lib/sg_pool.c Ming Lin
@ 2016-03-23  7:40 ` Christoph Hellwig
  2016-03-24 15:09   ` Ming Lin
  5 siblings, 1 reply; 20+ messages in thread
From: Christoph Hellwig @ 2016-03-23  7:40 UTC (permalink / raw)
  To: Ming Lin; +Cc: linux-kernel, linux-scsi, Christoph Hellwig

On Tue, Mar 22, 2016 at 03:03:11PM -0700, Ming Lin wrote:
> From: Ming Lin <ming.l@ssi.samsung.com>
> 
> The fist 4 patches make the SG related definitions/structs/functions
> in SCSI code generic and the last patch move it to lib/sg_pool.c.
> 
> I still keep the macro "SG_MEMPOOL_NR" since it's used in 3 places.

I don't ѕee the point, but I'm not going to block the series over
it either.

The new series looks really nice to me!

Reviewed-by: Christoph Hellwig <hch@lst.de>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 0/5] mempool based chained scatterlist alloc/free api
  2016-03-23  7:40 ` [PATCH v2 0/5] mempool based chained scatterlist alloc/free api Christoph Hellwig
@ 2016-03-24 15:09   ` Ming Lin
  2016-03-24 15:46     ` James Bottomley
  0 siblings, 1 reply; 20+ messages in thread
From: Ming Lin @ 2016-03-24 15:09 UTC (permalink / raw)
  To: Christoph Hellwig, James Bottomley; +Cc: lkml, linux-scsi

On Wed, Mar 23, 2016 at 12:40 AM, Christoph Hellwig <hch@lst.de> wrote:
> On Tue, Mar 22, 2016 at 03:03:11PM -0700, Ming Lin wrote:
>> From: Ming Lin <ming.l@ssi.samsung.com>
>>
>> The fist 4 patches make the SG related definitions/structs/functions
>> in SCSI code generic and the last patch move it to lib/sg_pool.c.
>>
>> I still keep the macro "SG_MEMPOOL_NR" since it's used in 3 places.
>
> I don't ѕee the point, but I'm not going to block the series over
> it either.
>
> The new series looks really nice to me!
>
> Reviewed-by: Christoph Hellwig <hch@lst.de>

Hi James,

This series touches several sub-systems.
What's the best way to merge it?

Thanks.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 0/5] mempool based chained scatterlist alloc/free api
  2016-03-24 15:09   ` Ming Lin
@ 2016-03-24 15:46     ` James Bottomley
  2016-03-28 14:48       ` Ming Lin
  0 siblings, 1 reply; 20+ messages in thread
From: James Bottomley @ 2016-03-24 15:46 UTC (permalink / raw)
  To: Ming Lin, Christoph Hellwig; +Cc: lkml, linux-scsi

On Thu, 2016-03-24 at 08:09 -0700, Ming Lin wrote:
> On Wed, Mar 23, 2016 at 12:40 AM, Christoph Hellwig <hch@lst.de>
> wrote:
> > On Tue, Mar 22, 2016 at 03:03:11PM -0700, Ming Lin wrote:
> > > From: Ming Lin <ming.l@ssi.samsung.com>
> > > 
> > > The fist 4 patches make the SG related
> > > definitions/structs/functions
> > > in SCSI code generic and the last patch move it to lib/sg_pool.c.
> > > 
> > > I still keep the macro "SG_MEMPOOL_NR" since it's used in 3
> > > places.
> > 
> > I don't ѕee the point, but I'm not going to block the series over
> > it either.
> > 
> > The new series looks really nice to me!
> > 
> > Reviewed-by: Christoph Hellwig <hch@lst.de>
> 
> Hi James,
> 
> This series touches several sub-systems.
> What's the best way to merge it?

It has a minor intrusion into 

 drivers/ata/pata_icside.c           |   2 +-
 drivers/infiniband/ulp/srp/ib_srp.c |   4 +-
 drivers/usb/storage/scsiglue.c      |   2 +-

Apart from that, it's all SCSI, so the SCSI tree would seem to be the
best one.

James

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 0/5] mempool based chained scatterlist alloc/free api
  2016-03-24 15:46     ` James Bottomley
@ 2016-03-28 14:48       ` Ming Lin
  2016-04-04  6:07         ` Ming Lin
  0 siblings, 1 reply; 20+ messages in thread
From: Ming Lin @ 2016-03-28 14:48 UTC (permalink / raw)
  To: James Bottomley; +Cc: Christoph Hellwig, lkml, linux-scsi

On Thu, Mar 24, 2016 at 8:46 AM, James Bottomley
<James.Bottomley@hansenpartnership.com> wrote:
> On Thu, 2016-03-24 at 08:09 -0700, Ming Lin wrote:
>> On Wed, Mar 23, 2016 at 12:40 AM, Christoph Hellwig <hch@lst.de>
>> wrote:
>> > On Tue, Mar 22, 2016 at 03:03:11PM -0700, Ming Lin wrote:
>> > > From: Ming Lin <ming.l@ssi.samsung.com>
>> > >
>> > > The fist 4 patches make the SG related
>> > > definitions/structs/functions
>> > > in SCSI code generic and the last patch move it to lib/sg_pool.c.
>> > >
>> > > I still keep the macro "SG_MEMPOOL_NR" since it's used in 3
>> > > places.
>> >
>> > I don't ѕee the point, but I'm not going to block the series over
>> > it either.
>> >
>> > The new series looks really nice to me!
>> >
>> > Reviewed-by: Christoph Hellwig <hch@lst.de>
>>
>> Hi James,
>>
>> This series touches several sub-systems.
>> What's the best way to merge it?
>
> It has a minor intrusion into
>
>  drivers/ata/pata_icside.c           |   2 +-
>  drivers/infiniband/ulp/srp/ib_srp.c |   4 +-
>  drivers/usb/storage/scsiglue.c      |   2 +-
>
> Apart from that, it's all SCSI, so the SCSI tree would seem to be the
> best one.

Are you OK to merge it?

Thanks.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 0/5] mempool based chained scatterlist alloc/free api
  2016-03-28 14:48       ` Ming Lin
@ 2016-04-04  6:07         ` Ming Lin
  2016-04-04 14:01           ` James Bottomley
  0 siblings, 1 reply; 20+ messages in thread
From: Ming Lin @ 2016-04-04  6:07 UTC (permalink / raw)
  To: James Bottomley; +Cc: Christoph Hellwig, lkml, linux-scsi

On Mon, Mar 28, 2016 at 7:48 AM, Ming Lin <mlin@kernel.org> wrote:
> On Thu, Mar 24, 2016 at 8:46 AM, James Bottomley
> <James.Bottomley@hansenpartnership.com> wrote:
>> On Thu, 2016-03-24 at 08:09 -0700, Ming Lin wrote:
>>> On Wed, Mar 23, 2016 at 12:40 AM, Christoph Hellwig <hch@lst.de>
>>> wrote:
>>> > On Tue, Mar 22, 2016 at 03:03:11PM -0700, Ming Lin wrote:
>>> > > From: Ming Lin <ming.l@ssi.samsung.com>
>>> > >
>>> > > The fist 4 patches make the SG related
>>> > > definitions/structs/functions
>>> > > in SCSI code generic and the last patch move it to lib/sg_pool.c.
>>> > >
>>> > > I still keep the macro "SG_MEMPOOL_NR" since it's used in 3
>>> > > places.
>>> >
>>> > I don't ѕee the point, but I'm not going to block the series over
>>> > it either.
>>> >
>>> > The new series looks really nice to me!
>>> >
>>> > Reviewed-by: Christoph Hellwig <hch@lst.de>
>>>
>>> Hi James,
>>>
>>> This series touches several sub-systems.
>>> What's the best way to merge it?
>>
>> It has a minor intrusion into
>>
>>  drivers/ata/pata_icside.c           |   2 +-
>>  drivers/infiniband/ulp/srp/ib_srp.c |   4 +-
>>  drivers/usb/storage/scsiglue.c      |   2 +-
>>
>> Apart from that, it's all SCSI, so the SCSI tree would seem to be the
>> best one.
>
> Are you OK to merge it?

Hi James,

Ping ...

Are you OK to apply this series?
Or any changes needed?

Thanks.

>
> Thanks.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 0/5] mempool based chained scatterlist alloc/free api
  2016-04-04  6:07         ` Ming Lin
@ 2016-04-04 14:01           ` James Bottomley
  0 siblings, 0 replies; 20+ messages in thread
From: James Bottomley @ 2016-04-04 14:01 UTC (permalink / raw)
  To: Ming Lin; +Cc: Christoph Hellwig, lkml, linux-scsi

On Sun, 2016-04-03 at 23:07 -0700, Ming Lin wrote:
> On Mon, Mar 28, 2016 at 7:48 AM, Ming Lin <mlin@kernel.org> wrote:
> > On Thu, Mar 24, 2016 at 8:46 AM, James Bottomley
> > <James.Bottomley@hansenpartnership.com> wrote:
> > > On Thu, 2016-03-24 at 08:09 -0700, Ming Lin wrote:
> > > > On Wed, Mar 23, 2016 at 12:40 AM, Christoph Hellwig <hch@lst.de
> > > > >
> > > > wrote:
> > > > > On Tue, Mar 22, 2016 at 03:03:11PM -0700, Ming Lin wrote:
> > > > > > From: Ming Lin <ming.l@ssi.samsung.com>
> > > > > > 
> > > > > > The fist 4 patches make the SG related
> > > > > > definitions/structs/functions
> > > > > > in SCSI code generic and the last patch move it to
> > > > > > lib/sg_pool.c.
> > > > > > 
> > > > > > I still keep the macro "SG_MEMPOOL_NR" since it's used in 3
> > > > > > places.
> > > > > 
> > > > > I don't ѕee the point, but I'm not going to block the series
> > > > > over
> > > > > it either.
> > > > > 
> > > > > The new series looks really nice to me!
> > > > > 
> > > > > Reviewed-by: Christoph Hellwig <hch@lst.de>
> > > > 
> > > > Hi James,
> > > > 
> > > > This series touches several sub-systems.
> > > > What's the best way to merge it?
> > > 
> > > It has a minor intrusion into
> > > 
> > >  drivers/ata/pata_icside.c           |   2 +-
> > >  drivers/infiniband/ulp/srp/ib_srp.c |   4 +-
> > >  drivers/usb/storage/scsiglue.c      |   2 +-
> > > 
> > > Apart from that, it's all SCSI, so the SCSI tree would seem to be
> > > the
> > > best one.
> > 
> > Are you OK to merge it?
> 
> Hi James,
> 
> Ping ...
> 
> Are you OK to apply this series?
> Or any changes needed?

It's got the needed reviews, but we need acks from tejun (ATA) and Sagi
(IB) and a repost fixing the test robot objection to patch 5.

James

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 4/5] scsi: rename SCSI_MAX_{SG, SG_CHAIN}_SEGMENTS
  2016-03-22 22:03 ` [PATCH v2 4/5] scsi: rename SCSI_MAX_{SG, SG_CHAIN}_SEGMENTS Ming Lin
@ 2016-04-04 14:22   ` Bart Van Assche
  2016-04-04 20:24   ` Ming Lin
  1 sibling, 0 replies; 20+ messages in thread
From: Bart Van Assche @ 2016-04-04 14:22 UTC (permalink / raw)
  To: Ming Lin, linux-kernel, linux-scsi; +Cc: Christoph Hellwig

On 03/22/16 15:03, Ming Lin wrote:
> From: Ming Lin <ming.l@ssi.samsung.com>
>
> Rename SCSI_MAX_SG_SEGMENTS to SG_CHUNK_SIZE, which means the amount
> we fit into a single scatterlist chunk.
>
> Rename SCSI_MAX_SG_CHAIN_SEGMENTS to SG_MAX_SEGMENTS.
>
> Will move these 2 generic definitions to scatterlist.h later.
>
> Signed-off-by: Ming Lin <ming.l@ssi.samsung.com>
> ---
>   drivers/ata/pata_icside.c           |  2 +-
>   drivers/infiniband/ulp/srp/ib_srp.c |  4 ++--
>   drivers/scsi/arm/cumana_2.c         |  2 +-
>   drivers/scsi/arm/eesox.c            |  2 +-
>   drivers/scsi/arm/powertec.c         |  2 +-
>   drivers/scsi/esas2r/esas2r_main.c   |  4 ++--
>   drivers/scsi/hisi_sas/hisi_sas.h    |  2 +-
>   drivers/scsi/mpt3sas/mpt3sas_base.c |  4 ++--
>   drivers/scsi/mpt3sas/mpt3sas_base.h |  2 +-
>   drivers/scsi/scsi_debug.c           |  2 +-
>   drivers/scsi/scsi_lib.c             | 34 +++++++++++++++++-----------------
>   drivers/usb/storage/scsiglue.c      |  2 +-
>   include/scsi/scsi.h                 |  8 ++++----
>   include/scsi/scsi_host.h            |  2 +-
>   14 files changed, 36 insertions(+), 36 deletions(-)

Hello Ming,

The ib_srp changes in this patch look fine to me.

Bart.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 5/5] lib: scatterlist: move SG pool code from SCSI driver to lib/sg_pool.c
  2016-03-23  2:38   ` [PATCH v2 5/5] lib: scatterlist: move SG pool code from SCSI driver to lib/sg_pool.c kbuild test robot
@ 2016-04-04 20:15     ` Ming Lin
  2016-04-04 20:17       ` Christoph Hellwig
  0 siblings, 1 reply; 20+ messages in thread
From: Ming Lin @ 2016-04-04 20:15 UTC (permalink / raw)
  To: kbuild test robot
  Cc: Ming Lin, kbuild-all, lkml, linux-scsi, Christoph Hellwig

On Tue, Mar 22, 2016 at 7:38 PM, kbuild test robot <lkp@intel.com> wrote:
> Hi Ming,
>
> [auto build test WARNING on scsi/for-next]
> [also build test WARNING on v4.5 next-20160322]
> [if your patch is applied to the wrong git tree, please drop us a note to help improving the system]
>
> url:    https://github.com/0day-ci/linux/commits/Ming-Lin/mempool-based-chained-scatterlist-alloc-free-api/20160323-060710
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git for-next
>
>
> coccinelle warnings: (new ones prefixed by >>)
>
>>> lib/sg_pool.c:152:3-18: WARNING: NULL check before freeing functions like kfree, debugfs_remove, debugfs_remove_recursive or usb_free_urb is not needed. Maybe consider reorganizing relevant code to avoid passing NULL values.
>    lib/sg_pool.c:154:3-21: WARNING: NULL check before freeing functions like kfree, debugfs_remove, debugfs_remove_recursive or usb_free_urb is not needed. Maybe consider reorganizing relevant code to avoid passing NULL values.

mempool_destroy()/kmem_cache_destroy() is OK to accept NULL pointer.
But the logic is more readable that we do NULL check when cleanup due to error.

cleanup_sdb:
        for (i = 0; i < SG_MEMPOOL_NR; i++) {
                struct sg_pool *sgp = sg_pools + i;
                if (sgp->pool)
                        mempool_destroy(sgp->pool);
                if (sgp->slab)
                        kmem_cache_destroy(sgp->slab);
        }

I'll keep the NULL check if no objection.

Thanks.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 5/5] lib: scatterlist: move SG pool code from SCSI driver to lib/sg_pool.c
  2016-04-04 20:15     ` Ming Lin
@ 2016-04-04 20:17       ` Christoph Hellwig
  2016-04-04 20:29         ` Ming Lin
  0 siblings, 1 reply; 20+ messages in thread
From: Christoph Hellwig @ 2016-04-04 20:17 UTC (permalink / raw)
  To: Ming Lin
  Cc: kbuild test robot, kbuild-all, lkml, linux-scsi, Christoph Hellwig

On Mon, Apr 04, 2016 at 01:15:45PM -0700, Ming Lin wrote:
> cleanup_sdb:
>         for (i = 0; i < SG_MEMPOOL_NR; i++) {
>                 struct sg_pool *sgp = sg_pools + i;
>                 if (sgp->pool)
>                         mempool_destroy(sgp->pool);
>                 if (sgp->slab)
>                         kmem_cache_destroy(sgp->slab);
>         }
> 
> I'll keep the NULL check if no objection.

I don't necessarily, but given that this is a code move I'd prefer
to keep the code as similar as possible in the actual move patch..

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 4/5] scsi: rename SCSI_MAX_{SG, SG_CHAIN}_SEGMENTS
  2016-03-22 22:03 ` [PATCH v2 4/5] scsi: rename SCSI_MAX_{SG, SG_CHAIN}_SEGMENTS Ming Lin
  2016-04-04 14:22   ` Bart Van Assche
@ 2016-04-04 20:24   ` Ming Lin
  2016-04-04 21:22     ` Tejun Heo
  1 sibling, 1 reply; 20+ messages in thread
From: Ming Lin @ 2016-04-04 20:24 UTC (permalink / raw)
  To: Tejun Heo; +Cc: lkml, linux-scsi, Christoph Hellwig

On Tue, Mar 22, 2016 at 3:03 PM, Ming Lin <mlin@kernel.org> wrote:
> From: Ming Lin <ming.l@ssi.samsung.com>
>
> Rename SCSI_MAX_SG_SEGMENTS to SG_CHUNK_SIZE, which means the amount
> we fit into a single scatterlist chunk.
>
> Rename SCSI_MAX_SG_CHAIN_SEGMENTS to SG_MAX_SEGMENTS.
>
> Will move these 2 generic definitions to scatterlist.h later.
>
> Signed-off-by: Ming Lin <ming.l@ssi.samsung.com>
> ---
>  drivers/ata/pata_icside.c           |  2 +-
>  drivers/infiniband/ulp/srp/ib_srp.c |  4 ++--
>  drivers/scsi/arm/cumana_2.c         |  2 +-
>  drivers/scsi/arm/eesox.c            |  2 +-
>  drivers/scsi/arm/powertec.c         |  2 +-
>  drivers/scsi/esas2r/esas2r_main.c   |  4 ++--
>  drivers/scsi/hisi_sas/hisi_sas.h    |  2 +-
>  drivers/scsi/mpt3sas/mpt3sas_base.c |  4 ++--
>  drivers/scsi/mpt3sas/mpt3sas_base.h |  2 +-
>  drivers/scsi/scsi_debug.c           |  2 +-
>  drivers/scsi/scsi_lib.c             | 34 +++++++++++++++++-----------------
>  drivers/usb/storage/scsiglue.c      |  2 +-
>  include/scsi/scsi.h                 |  8 ++++----
>  include/scsi/scsi_host.h            |  2 +-
>  14 files changed, 36 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/ata/pata_icside.c b/drivers/ata/pata_icside.c
> index d7c7320..188f2f2 100644
> --- a/drivers/ata/pata_icside.c
> +++ b/drivers/ata/pata_icside.c
> @@ -294,7 +294,7 @@ static int icside_dma_init(struct pata_icside_info *info)
>
>  static struct scsi_host_template pata_icside_sht = {
>         ATA_BASE_SHT(DRV_NAME),
> -       .sg_tablesize           = SCSI_MAX_SG_CHAIN_SEGMENTS,
> +       .sg_tablesize           = SG_MAX_SEGMENTS,
>         .dma_boundary           = IOMD_DMA_BOUNDARY,
>  };

Hi Tejun,

Could you help to review/ack this ATA part?
Thanks.

> diff --git a/include/scsi/scsi.h b/include/scsi/scsi.h
> index e0a3398..74dafa7 100644
> --- a/include/scsi/scsi.h
> +++ b/include/scsi/scsi.h
> @@ -24,16 +24,16 @@ enum scsi_timeouts {
>   * to SG_MAX_SINGLE_ALLOC to pack correctly at the highest order.  The
>   * minimum value is 32
>   */
> -#define SCSI_MAX_SG_SEGMENTS   128
> +#define SG_CHUNK_SIZE  128
>
>  /*
> - * Like SCSI_MAX_SG_SEGMENTS, but for archs that have sg chaining. This limit
> + * Like SG_CHUNK_SIZE, but for archs that have sg chaining. This limit
>   * is totally arbitrary, a setting of 2048 will get you at least 8mb ios.
>   */
>  #ifdef CONFIG_ARCH_HAS_SG_CHAIN
> -#define SCSI_MAX_SG_CHAIN_SEGMENTS     2048
> +#define SG_MAX_SEGMENTS        2048
>  #else
> -#define SCSI_MAX_SG_CHAIN_SEGMENTS     SCSI_MAX_SG_SEGMENTS
> +#define SG_MAX_SEGMENTS        SG_CHUNK_SIZE
>  #endif
>
>  /*
> diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h
> index fcfa3d7..76e9d27 100644
> --- a/include/scsi/scsi_host.h
> +++ b/include/scsi/scsi_host.h
> @@ -37,7 +37,7 @@ struct blk_queue_tags;
>   *      used in one scatter-gather request.
>   */
>  #define SG_NONE 0
> -#define SG_ALL SCSI_MAX_SG_SEGMENTS
> +#define SG_ALL SG_CHUNK_SIZE
>
>  #define MODE_UNKNOWN 0x00
>  #define MODE_INITIATOR 0x01
> --
> 1.9.1

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 5/5] lib: scatterlist: move SG pool code from SCSI driver to lib/sg_pool.c
  2016-04-04 20:17       ` Christoph Hellwig
@ 2016-04-04 20:29         ` Ming Lin
  0 siblings, 0 replies; 20+ messages in thread
From: Ming Lin @ 2016-04-04 20:29 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: kbuild test robot, kbuild-all, lkml, linux-scsi

On Mon, Apr 4, 2016 at 1:17 PM, Christoph Hellwig <hch@lst.de> wrote:
> On Mon, Apr 04, 2016 at 01:15:45PM -0700, Ming Lin wrote:
>> cleanup_sdb:
>>         for (i = 0; i < SG_MEMPOOL_NR; i++) {
>>                 struct sg_pool *sgp = sg_pools + i;
>>                 if (sgp->pool)
>>                         mempool_destroy(sgp->pool);
>>                 if (sgp->slab)
>>                         kmem_cache_destroy(sgp->slab);
>>         }
>>
>> I'll keep the NULL check if no objection.
>
> I don't necessarily, but given that this is a code move I'd prefer
> to keep the code as similar as possible in the actual move patch..

So I'll just keep it.
And I can send a cleanup patch after this series applied.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 4/5] scsi: rename SCSI_MAX_{SG, SG_CHAIN}_SEGMENTS
  2016-04-04 20:24   ` Ming Lin
@ 2016-04-04 21:22     ` Tejun Heo
  0 siblings, 0 replies; 20+ messages in thread
From: Tejun Heo @ 2016-04-04 21:22 UTC (permalink / raw)
  To: Ming Lin; +Cc: lkml, linux-scsi, Christoph Hellwig

On Mon, Apr 04, 2016 at 01:24:52PM -0700, Ming Lin wrote:
> Could you help to review/ack this ATA part?
> Thanks.

Can you please resend me the patches?

Thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2016-04-04 21:22 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-03-22 22:03 [PATCH v2 0/5] mempool based chained scatterlist alloc/free api Ming Lin
2016-03-22 22:03 ` [PATCH v2 1/5] scsi: replace "scsi_data_buffer" with "sg_table" in SG functions Ming Lin
2016-03-22 22:03 ` [PATCH v2 2/5] scsi: replace "mq" with "first_chunk" " Ming Lin
2016-03-22 22:03 ` [PATCH v2 3/5] scsi: rename SG related struct and functions Ming Lin
2016-03-22 22:03 ` [PATCH v2 4/5] scsi: rename SCSI_MAX_{SG, SG_CHAIN}_SEGMENTS Ming Lin
2016-04-04 14:22   ` Bart Van Assche
2016-04-04 20:24   ` Ming Lin
2016-04-04 21:22     ` Tejun Heo
2016-03-22 22:03 ` [PATCH v2 5/5] lib: scatterlist: move SG pool code from SCSI driver to lib/sg_pool.c Ming Lin
2016-03-23  2:38   ` [PATCH] lib: scatterlist: fix ifnullfree.cocci warnings kbuild test robot
2016-03-23  2:38   ` [PATCH v2 5/5] lib: scatterlist: move SG pool code from SCSI driver to lib/sg_pool.c kbuild test robot
2016-04-04 20:15     ` Ming Lin
2016-04-04 20:17       ` Christoph Hellwig
2016-04-04 20:29         ` Ming Lin
2016-03-23  7:40 ` [PATCH v2 0/5] mempool based chained scatterlist alloc/free api Christoph Hellwig
2016-03-24 15:09   ` Ming Lin
2016-03-24 15:46     ` James Bottomley
2016-03-28 14:48       ` Ming Lin
2016-04-04  6:07         ` Ming Lin
2016-04-04 14:01           ` James Bottomley

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).