Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1363041 > unrolled thread

[PATCH v2 0/5] mempool based chained scatterlist alloc/free api

Back to article view | Back to linux.kernel

Contents

  [PATCH v2 0/5] mempool based chained scatterlist alloc/free api Ming Lin <mlin@kernel.org> - 2016-03-22 23:10 +0100
    [PATCH v2 5/5] lib: scatterlist: move SG pool code from SCSI driver to  lib/sg_pool.c Ming Lin <mlin@kernel.org> - 2016-03-22 23:10 +0100
      [PATCH] lib: scatterlist: fix ifnullfree.cocci warnings kbuild test robot <lkp@intel.com> - 2016-03-23 03:40 +0100
      Re: [PATCH v2 5/5] lib: scatterlist: move SG pool code from SCSI  driver to lib/sg_pool.c kbuild test robot <lkp@intel.com> - 2016-03-23 03:40 +0100
    [PATCH v2 1/5] scsi: replace "scsi_data_buffer" with "sg_table" in SG  functions Ming Lin <mlin@kernel.org> - 2016-03-22 23:10 +0100
    Re: [PATCH v2 0/5] mempool based chained scatterlist alloc/free api Christoph Hellwig <hch@lst.de> - 2016-03-23 08:50 +0100
      Re: [PATCH v2 0/5] mempool based chained scatterlist alloc/free api Ming Lin <mlin@kernel.org> - 2016-03-24 16:20 +0100
        Re: [PATCH v2 0/5] mempool based chained scatterlist alloc/free api James Bottomley <James.Bottomley@HansenPartnership.com> - 2016-03-24 16:50 +0100
          Re: [PATCH v2 0/5] mempool based chained scatterlist alloc/free api Ming Lin <mlin@kernel.org> - 2016-03-28 17:00 +0200

#1363041 — [PATCH v2 0/5] mempool based chained scatterlist alloc/free api

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

[toc] | [next] | [standalone]

#1363045 — [PATCH v2 5/5] lib: scatterlist: move SG pool code from SCSI driver to lib/sg_pool.c

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

[toc] | [prev] | [next] | [standalone]

#1363179 — [PATCH] lib: scatterlist: fix ifnullfree.cocci warnings

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;

[toc] | [prev] | [next] | [standalone]

#1363180 — Re: [PATCH v2 5/5] lib: scatterlist: move SG pool code from SCSI driver to lib/sg_pool.c

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

[toc] | [prev] | [next] | [standalone]

#1363047 — [PATCH v2 1/5] scsi: replace "scsi_data_buffer" with "sg_table" in SG functions

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

[toc] | [prev] | [next] | [standalone]

#1363227

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>

[toc] | [prev] | [next] | [standalone]

#1364261

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.

[toc] | [prev] | [next] | [standalone]

#1364289

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

[toc] | [prev] | [next] | [standalone]

#1365534

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.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web