Re: NVMe over Fabrics target implementation

2016-06-07 Thread Ming Lin 
On Tue, Jun 7, 2016 at 2:02 PM, Andy Grover  wrote:
> On 06/06/2016 11:23 PM, Nicholas A. Bellinger wrote:
>>
>> Hi HCH & Co,
>>
>> On Mon, 2016-06-06 at 23:22 +0200, Christoph Hellwig wrote:
>>>
>>> This patch set adds a generic NVMe over Fabrics target. The
>>> implementation conforms to the NVMe 1.2b specification (which
>>> includes Fabrics) and provides the NVMe over Fabrics access
>>> to Linux block devices.
>>>
>>
>> Thanks for all of the development work by the fabric_linux_driver team
>> (HCH, Sagi, Ming, James F., James S., and Dave M.) over the last year.
>>
>> Very excited to see this code get a public release now that NVMf
>> specification is out.  Now that it's in the wild, it's a good
>> opportunity to discuss some of the more interesting implementation
>> details, beyond the new NVMf wire-protocol itself.
>
>
> I'm sorry but I missed it, can you repeat the link to the NVMe spec(s)?

http://www.nvmexpress.org/wp-content/uploads/NVM_Express_1_2_1_Gold_20160603.pdf
http://www.nvmexpress.org/wp-content/uploads/NVMe_over_Fabrics_1_0_Gold_20160605.pdf
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 4/5] scsi: rename SCSI_MAX_{SG, SG_CHAIN}_SEGMENTS

2016-04-11 Thread Ming Lin 
On Mon, Apr 11, 2016 at 2:34 PM, Martin K. Petersen
<martin.peter...@oracle.com> wrote:
>>>>>> "Ming" == Ming Lin <m...@kernel.org> writes:
>
> Ming> Are we ready to merge it?
>
> We're still missing an ack from Sagi.

Thought we already had a ack from Bart.
OK, let's get one more from Sagi.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 4/5] scsi: rename SCSI_MAX_{SG, SG_CHAIN}_SEGMENTS

2016-04-10 Thread Ming Lin 
On Tue, Apr 5, 2016 at 7:55 AM, Tejun Heo <t...@kernel.org> wrote:
> On Mon, Apr 04, 2016 at 02:48:10PM -0700, Ming Lin wrote:
>> From: Ming Lin <min...@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.
>>
>> Reviewed-by: Christoph Hellwig <h...@lst.de>
>> Acked-by: Bart Van Assche <bart.vanass...@sandisk.com> (for ib_srp changes)
>> Signed-off-by: Ming Lin <min...@ssi.samsung.com>
>
> For libata,
>
> Acked-by: Tejun Heo <t...@kernel.org>

Hi James,

Are we ready to merge it?

Thanks,
Ming
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH RFC 1/2] scatterlist: add mempool based chained SG alloc/free api

2016-04-07 Thread Ming Lin 
On Thu, Apr 7, 2016 at 9:43 AM, Ming Lin <m...@kernel.org> wrote:
> On Thu, Apr 7, 2016 at 7:56 AM, Bart Van Assche
> <bart.vanass...@sandisk.com> wrote:
>> On 03/15/16 15:39, Ming Lin wrote:
>>>
>>> +static void sg_mempoll_free(struct scatterlist *sgl, unsigned int nents)
>>
>>
>> Please change mempoll into mempool.
>
> Good catch. Thanks Bart!

Hi Bart,

This is actually the previous old RFC patch.
The v2 and v3 patch series doesn't have this typo.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH RFC 1/2] scatterlist: add mempool based chained SG alloc/free api

2016-04-07 Thread Ming Lin 
On Thu, Apr 7, 2016 at 7:56 AM, Bart Van Assche
<bart.vanass...@sandisk.com> wrote:
> On 03/15/16 15:39, Ming Lin wrote:
>>
>> +static void sg_mempoll_free(struct scatterlist *sgl, unsigned int nents)
>
>
> Please change mempoll into mempool.

Good catch. Thanks Bart!
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 1/5] scsi: replace "scsi_data_buffer" with "sg_table" in SG functions

2016-04-04 Thread Ming Lin 
From: Ming Lin <min...@ssi.samsung.com>

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

Reviewed-by: Christoph Hellwig <h...@lst.de>
Signed-off-by: Ming Lin <min...@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 8106515..4229c18 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(>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(>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(>sdb, true);
-   if (cmd->request->next_rq && cmd->request->next_rq->special)
-   scsi_free_sgtable(cmd->request->next_rq->special, true);
+   scsi_free_sgtable(>sdb.table, true);
+   if (cmd->request->next_rq) {
+   sdb = cmd->request->next_rq->special;
+   if (sdb)
+   scsi_free_sgtable(>table, true);
+   }
if (scsi_prot_sg_count(cmd))
-   scsi_free_sgtable(cmd->prot_sdb, true);
+   scsi_free_sgtable(>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(>sdb, false);
+   scsi_free_sgtable(>sdb.table, false);
 
memset(>sdb, 0, sizeof(cmd->sdb));
 
if (scsi_prot_sg_count(cmd))
-   scsi_free_sgtable(cmd->prot_sdb, false);
+   scsi_free_sgtable(>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(_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(>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(_sdb->table, ivecs, is_mq)) {
error = BLKPREP_DEFER;
goto err_exit;
}
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 4/5] scsi: rename SCSI_MAX_{SG, SG_CHAIN}_SEGMENTS

2016-04-04 Thread Ming Lin 
From: Ming Lin <min...@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.

Reviewed-by: Christoph Hellwig <h...@lst.de>
Acked-by: Bart Van Assche <bart.vanass...@sandisk.com> (for ib_srp changes)
Signed-off-by: Ming Lin <min...@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 ff21597..369a75e 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,
@@ -3097,7 +3097,7 @@ static int srp_parse_options(const char *buf, struct 
srp_target_port *target)
 
case SRP_OPT_SG_TABLESIZE:
if (match_int(args, ) || 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,
dif

[PATCH v3 2/5] scsi: replace "mq" with "first_chunk" in SG functions

2016-04-04 Thread Ming Lin 
From: Ming Lin <min...@ssi.samsung.com>

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

Reviewed-by: Christoph Hellwig <h...@lst.de>
Signed-off-by: Ming Lin <min...@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 4229c18..9675353 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(>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(_sdb->table, ivecs, is_mq)) {
+   if (scsi_alloc_sgtable(_sdb->table, ivecs,
+   prot_sdb->table.sgl)) {
error = BLKPREP_DEFER;
goto err_exit;
}
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 5/5] lib: scatterlist: move SG pool code from SCSI driver to lib/sg_pool.c

2016-04-04 Thread Ming Lin 
From: Ming Lin <min...@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.

Reviewed-by: Christoph Hellwig <h...@lst.de>
Signed-off-by: Ming Lin <min...@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 0950567..98e5d51 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 8f776f1..b920c5d 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -14,8 +14,6 @@
 #include 
 #include 
 #include 
-#include 
-#include 
 #include 
 #include 
 #include 
@@ -40,39 +38,6 @@
 #include "scsi_logging.h"
 
 
-#define SG_MEMPOOL_NR  ARRAY_SIZE(sg_pools)
-#define SG_MEMPOOL_SIZE2
-
-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,
-

[PATCH v3 3/5] scsi: rename SG related struct and functions

2016-04-04 Thread Ming Lin 
From: Ming Lin <min...@ssi.samsung.com>

Rename SCSI specific struct and functions to more genenic names.

Reviewed-by: Christoph Hellwig <h...@lst.de>
Signed-off-by: Ming Lin <min...@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 9675353..08134f6 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_SIZE2
 
-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(>sdb.table, true);
+   sg_free_table_chained(>sdb.table, true);
if (cmd->request->next_rq) {
sdb = cmd->request->next_rq->special;
if (sdb)
-   scsi_free_sgtable(>table, true);
+   sg_free_table_chained(>table, true);
}
if (scsi_prot_sg_count(cmd))
-   scsi_free_sgtable(>prot_sdb->table, true);
+   sg_free_table_chained(>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(>sdb.table, false);
+   sg_free_table_chained(>sdb.table, false);
 
memset(>sdb, 0, sizeof(cmd->sdb));
 
if (scsi_prot_sg_count(cmd))
-   scsi_free_sgtable(>prot_sdb->table, false);
+   sg_free_table_chained(>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(_sdb->table, false);
+   sg_free_table_ch

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

2016-04-04 Thread Ming Lin 
From: Ming Lin <min...@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.

v3:
  - Resend for Tejun to review. No code change since v2.
  - Add review/ack tags

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

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2016-04-04 Thread Ming Lin 
On Mon, Apr 4, 2016 at 1:17 PM, Christoph Hellwig <h...@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.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 4/5] scsi: rename SCSI_MAX_{SG, SG_CHAIN}_SEGMENTS

2016-04-04 Thread Ming Lin 
On Tue, Mar 22, 2016 at 3:03 PM, Ming Lin <m...@kernel.org> wrote:
> From: Ming Lin <min...@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 <min...@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_SEGMENTS2048
>  #else
> -#define SCSI_MAX_SG_CHAIN_SEGMENTS SCSI_MAX_SG_SEGMENTS
> +#define SG_MAX_SEGMENTSSG_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
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2016-04-04 Thread Ming Lin 
On Tue, Mar 22, 2016 at 7:38 PM, kbuild test robot <l...@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.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2016-03-28 Thread Ming Lin 
On Thu, Mar 24, 2016 at 8:46 AM, James Bottomley
<james.bottom...@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 <h...@lst.de>
>> wrote:
>> > On Tue, Mar 22, 2016 at 03:03:11PM -0700, Ming Lin wrote:
>> > > From: Ming Lin <min...@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 <h...@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.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2016-03-24 Thread Ming Lin 
On Wed, Mar 23, 2016 at 12:40 AM, Christoph Hellwig <h...@lst.de> wrote:
> On Tue, Mar 22, 2016 at 03:03:11PM -0700, Ming Lin wrote:
>> From: Ming Lin <min...@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 <h...@lst.de>

Hi James,

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

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 2/5] scsi: replace "mq" with "first_chunk" in SG functions

2016-03-22 Thread Ming Lin 
From: Ming Lin <min...@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 <min...@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(>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(_sdb->table, ivecs, is_mq)) {
+   if (scsi_alloc_sgtable(_sdb->table, ivecs,
+   prot_sdb->table.sgl)) {
error = BLKPREP_DEFER;
goto err_exit;
}
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2016-03-22 Thread Ming Lin 
From: Ming Lin <min...@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

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2016-03-22 Thread Ming Lin 
From: Ming Lin <min...@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 <min...@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(>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(>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(>sdb, true);
-   if (cmd->request->next_rq && cmd->request->next_rq->special)
-   scsi_free_sgtable(cmd->request->next_rq->special, true);
+   scsi_free_sgtable(>sdb.table, true);
+   if (cmd->request->next_rq) {
+   sdb = cmd->request->next_rq->special;
+   if (sdb)
+   scsi_free_sgtable(>table, true);
+   }
if (scsi_prot_sg_count(cmd))
-   scsi_free_sgtable(cmd->prot_sdb, true);
+   scsi_free_sgtable(>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(>sdb, false);
+   scsi_free_sgtable(>sdb.table, false);
 
memset(>sdb, 0, sizeof(cmd->sdb));
 
if (scsi_prot_sg_count(cmd))
-   scsi_free_sgtable(cmd->prot_sdb, false);
+   scsi_free_sgtable(>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(_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(>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(_sdb->table, ivecs, is_mq)) {
error = BLKPREP_DEFER;
goto err_exit;
}
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 4/5] scsi: rename SCSI_MAX_{SG, SG_CHAIN}_SEGMENTS

2016-03-22 Thread Ming Lin 
From: Ming Lin <min...@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 <min...@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 < 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/esa

[PATCH v2 3/5] scsi: rename SG related struct and functions

2016-03-22 Thread Ming Lin 
From: Ming Lin <min...@ssi.samsung.com>

Rename SCSI specific struct and functions to more genenic names.

Signed-off-by: Ming Lin <min...@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_SIZE2
 
-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(>sdb.table, true);
+   sg_free_table_chained(>sdb.table, true);
if (cmd->request->next_rq) {
sdb = cmd->request->next_rq->special;
if (sdb)
-   scsi_free_sgtable(>table, true);
+   sg_free_table_chained(>table, true);
}
if (scsi_prot_sg_count(cmd))
-   scsi_free_sgtable(>prot_sdb->table, true);
+   sg_free_table_chained(>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(>sdb.table, false);
+   sg_free_table_chained(>sdb.table, false);
 
memset(>sdb, 0, sizeof(cmd->sdb));
 
if (scsi_prot_sg_count(cmd))
-   scsi_free_sgtable(>prot_sdb->table, false);
+   sg_free_table_chained(>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(_sdb->table, false);
+   sg_free_table_chained(_sdb->table, false);
kmem_cache_free

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

2016-03-22 Thread Ming Lin 
From: Ming Lin <min...@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 <min...@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 
 #include 
 #include 
-#include 
-#include 
 #include 
 #include 
 #include 
@@ -40,39 +38,6 @@
 #include "scsi_logging.h"
 
 
-#define SG_MEMPOOL_NR  ARRAY_SIZE(sg_pools)
-#define SG_MEMPOOL_SIZE2
-
-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_

Re: [PATCH RFC 1/2] scatterlist: add mempool based chained SG alloc/free api

2016-03-21 Thread Ming Lin 
On Wed, 2016-03-16 at 09:23 +0100, Christoph Hellwig wrote:
> > 
> We can defintively kill this one.

We want to support different size of pools.
How can we kill this one?

Or did you mean we just create a single pool with size SG_CHUNK_SIZE?

> 
> > +static __init int sg_mempool_init(void)
> > +{
> > +   int i;
> > +
> > +   for (i = 0; i < SG_MEMPOOL_NR; i++) {
> > +   struct sg_mempool *sgp = sg_pools + i;
> > +   int size = sgp->size * sizeof(struct scatterlist);
> > +
> > +   sgp->slab = kmem_cache_create(sgp->name, size, 0,
> > +   SLAB_HWCACHE_ALIGN, NULL);
> 
> Having these mempoools around in every kernel will make some embedded
> developers rather unhappy.  We could either not create them at
> runtime, which would require either a check in the fast path, or
> an init call in every driver, or just move the functions you
> added into a separe file, which will be compiled only based on a
> Kconfig
> symbol, and could even be potentially modular.  I think that
> second option might be easier.

I created lib/sg_pool.c with CONFIG_SG_POOL.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH RFC 0/2] mempool based chained scatterlist alloc/free api api

2016-03-15 Thread Ming Lin 
On Tue, 2016-03-15 at 16:12 -0700, James Bottomley wrote:
> On Tue, 2016-03-15 at 15:39 -0700, Ming Lin wrote:
> > From: Ming Lin <min...@ssi.samsung.com>
> > 
> > Hi list,
> > 
> > This moves the mempool based chained scatterlist alloc/free code
> > from
> > scsi_lib.c to lib/scatterlist.c.
> > 
> > So other drivers(for example, the under development NVMe over
> > fabric 
> > drivers) can also use it.
> > 
> > Ming Lin (2):
> >   scatterlist: add mempool based chained SG alloc/free api
> >   scsi: use the new chained SG api
> > 
> >  drivers/scsi/scsi_lib.c | 129 ++
> > --
> >  include/linux/scatterlist.h |  12 
> >  lib/scatterlist.c   | 156
> > 
> >  3 files changed, 175 insertions(+), 122 deletions(-)
> 
> I'd really rather this were a single patch so git can tell us the
> code
> motion.  If you add in one patch and remove in another the code
> motion
> trackers don't see it.
> 
> Secondly, you said "This copied code from scsi_lib.c to scatterlist.c
> and modified it a bit" could you move in one patch and modify in
> another, so we can see exactly what you're changing.

The modification is mostly about structure names and function names
changes.

I can do it in a single patch.

> 
> Thirdly, are you sure the pool structure for NVMe should be the same
> as
> for SCSI?  We don't do buddy pools for 1,2 or 4 entry transactions in
> SCSI just basically because of heuristics, but the packetised io
> characteristics of NVMe make single entry lists more likely for it,
> don't they?

Not sure about this, but the nvme-pci driver may not use this api,
because it also has a PRP lists except for the SG lists.

But nvme-over-rdma/nvme-over-fiber-channel driver is good to use this
api.

> 
> James
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH RFC 1/2] scatterlist: add mempool based chained SG alloc/free api

2016-03-15 Thread Ming Lin 
From: Ming Lin <min...@ssi.samsung.com>

This copied code from scsi_lib.c to scatterlist.c and
modified it a bit.

Signed-off-by: Ming Lin <min...@ssi.samsung.com>
---
 include/linux/scatterlist.h |  12 
 lib/scatterlist.c   | 156 
 2 files changed, 168 insertions(+)

diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h
index 556ec1e..888f2c3 100644
--- a/include/linux/scatterlist.h
+++ b/include/linux/scatterlist.h
@@ -266,6 +266,10 @@ int sg_alloc_table_from_pages(struct sg_table *sgt,
unsigned long offset, unsigned long size,
gfp_t gfp_mask);
 
+void sg_free_chained(struct sg_table *table, bool first_chunk);
+int sg_alloc_chained(struct sg_table *table, int nents,
+   struct scatterlist *first_chunk, gfp_t gfp);
+
 size_t sg_copy_buffer(struct scatterlist *sgl, unsigned int nents, void *buf,
  size_t buflen, off_t skip, bool to_buffer);
 
@@ -286,6 +290,14 @@ 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.
+ *
+ * XXX: what's the best number?
+ */
+#define SG_MAX_SEGMENTS128
+
+/*
  * sg page iterator
  *
  * Iterates over sg entries page-by-page.  On each successful iteration,
diff --git a/lib/scatterlist.c b/lib/scatterlist.c
index 004fc70..f97831e 100644
--- a/lib/scatterlist.c
+++ b/lib/scatterlist.c
@@ -11,6 +11,7 @@
 #include 
 #include 
 #include 
+#include 
 
 /**
  * sg_next - return the next scatterlist entry in a list
@@ -755,3 +756,158 @@ size_t sg_pcopy_to_buffer(struct scatterlist *sgl, 
unsigned int nents,
return sg_copy_buffer(sgl, nents, buf, buflen, skip, true);
 }
 EXPORT_SYMBOL(sg_pcopy_to_buffer);
+
+#define SG_MEMPOOL_NR  ARRAY_SIZE(sg_pools)
+#define SG_MEMPOOL_SIZE2
+
+struct sg_mempool {
+   size_t  size;
+   char*name;
+   struct kmem_cache *slab;
+   mempool_t   *pool;
+};
+
+#define SP(x) { .size = x, "sgpool-" __stringify(x) }
+#if (SG_MAX_SEGMENTS < 32)
+#error SG_MAX_SEGMENTS is too small (must be 32 or greater)
+#endif
+static struct sg_mempool sg_pools[] = {
+   SP(8),
+   SP(16),
+#if (SG_MAX_SEGMENTS > 32)
+   SP(32),
+#if (SG_MAX_SEGMENTS > 64)
+   SP(64),
+#if (SG_MAX_SEGMENTS > 128)
+   SP(128),
+#if (SG_MAX_SEGMENTS > 256)
+#error SG_MAX_SEGMENTS is too large (256 MAX)
+#endif
+#endif
+#endif
+#endif
+   SP(SG_MAX_SEGMENTS)
+};
+#undef SP
+
+static inline unsigned int sg_pool_index(unsigned short nents)
+{
+   unsigned int index;
+
+   BUG_ON(nents > SG_MAX_SEGMENTS);
+
+   if (nents <= 8)
+   index = 0;
+   else
+   index = get_count_order(nents) - 3;
+
+   return index;
+}
+
+static void sg_mempoll_free(struct scatterlist *sgl, unsigned int nents)
+{
+   struct sg_mempool *sgp;
+
+   sgp = sg_pools + sg_pool_index(nents);
+   mempool_free(sgl, sgp->pool);
+}
+
+static struct scatterlist *sg_mempool_alloc(unsigned int nents, gfp_t gfp)
+{
+   struct sg_mempool *sgp;
+
+   sgp = sg_pools + sg_pool_index(nents);
+   return mempool_alloc(sgp->pool, gfp);
+}
+
+/**
+ * sg_free_chained - Free a previously mapped sg table
+ * @table: The sg table header to use
+ * @first_chunk: was first_chunk not NULL in sg_alloc_chained?
+ *
+ *  Description:
+ *Free an sg table previously allocated and setup with
+ *sg_alloc_chained().
+ *
+ **/
+void sg_free_chained(struct sg_table *table, bool first_chunk)
+{
+   if (first_chunk && table->orig_nents <= SG_MAX_SEGMENTS)
+   return;
+   __sg_free_table(table, SG_MAX_SEGMENTS, 1, sg_mempoll_free);
+}
+EXPORT_SYMBOL_GPL(sg_free_chained);
+
+/**
+ * sg_alloc_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
+ * @gfp:   GFP allocation mask
+ *
+ *  Description:
+ *Allocate and chain SGLs in an sg table. If @nents@ is larger than
+ *SG_MAX_SEGMENTS a chained sg table will be setup.
+ *
+ **/
+int sg_alloc_chained(struct sg_table *table, int nents,
+   struct scatterlist *first_chunk, gfp_t gfp)
+{
+   int ret;
+
+   BUG_ON(!nents);
+
+   if (first_chunk && nents <= SG_MAX_SEGMENTS) {
+   table->nents = table->orig_nents = nents;
+   sg_init_table(first_chunk, nents);
+   return 0;
+   }
+
+   ret = __sg_alloc_table(table, nents, SG_MAX_SEGMENTS,
+   first_chunk, gfp, sg_mempool_alloc);
+   if (unlikely(ret))
+   sg_free_chained(table, (bool)first_chunk);
+
+   return ret;
+}
+EXPORT_SYMBOL_GP

[PATCH RFC 0/2] mempool based chained scatterlist alloc/free api api

2016-03-15 Thread Ming Lin 
From: Ming Lin <min...@ssi.samsung.com>

Hi list,

This moves the mempool based chained scatterlist alloc/free code from
scsi_lib.c to lib/scatterlist.c.

So other drivers(for example, the under development NVMe over fabric drivers)
can also use it.

Ming Lin (2):
  scatterlist: add mempool based chained SG alloc/free api
  scsi: use the new chained SG api

 drivers/scsi/scsi_lib.c | 129 ++--
 include/linux/scatterlist.h |  12 
 lib/scatterlist.c   | 156 
 3 files changed, 175 insertions(+), 122 deletions(-)

-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH RFC 2/2] scsi: use the new chained SG api

2016-03-15 Thread Ming Lin 
From: Ming Lin <min...@ssi.samsung.com>

This removes the old code and uses the new chained SG alloc/free api.

Signed-off-by: Ming Lin <min...@ssi.samsung.com>
---
 drivers/scsi/scsi_lib.c | 129 +++-
 1 file changed, 7 insertions(+), 122 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 8c6e318..97e283c 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -39,40 +39,6 @@
 #include "scsi_priv.h"
 #include "scsi_logging.h"
 
-
-#define SG_MEMPOOL_NR  ARRAY_SIZE(scsi_sg_pools)
-#define SG_MEMPOOL_SIZE2
-
-struct scsi_host_sg_pool {
-   size_t  size;
-   char*name;
-   struct kmem_cache   *slab;
-   mempool_t   *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)
-#endif
-static struct scsi_host_sg_pool scsi_sg_pools[] = {
-   SP(8),
-   SP(16),
-#if (SCSI_MAX_SG_SEGMENTS > 32)
-   SP(32),
-#if (SCSI_MAX_SG_SEGMENTS > 64)
-   SP(64),
-#if (SCSI_MAX_SG_SEGMENTS > 128)
-   SP(128),
-#if (SCSI_MAX_SG_SEGMENTS > 256)
-#error SCSI_MAX_SG_SEGMENTS is too large (256 MAX)
-#endif
-#endif
-#endif
-#endif
-   SP(SCSI_MAX_SG_SEGMENTS)
-};
-#undef SP
-
 struct kmem_cache *scsi_sdb_cache;
 
 /*
@@ -553,61 +519,23 @@ 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)
-{
-   unsigned int index;
-
-   BUG_ON(nents > SCSI_MAX_SG_SEGMENTS);
-
-   if (nents <= 8)
-   index = 0;
-   else
-   index = get_count_order(nents) - 3;
-
-   return index;
-}
-
-static void scsi_sg_free(struct scatterlist *sgl, unsigned int nents)
-{
-   struct scsi_host_sg_pool *sgp;
-
-   sgp = scsi_sg_pools + scsi_sgtable_index(nents);
-   mempool_free(sgl, sgp->pool);
-}
-
-static struct scatterlist *scsi_sg_alloc(unsigned int nents, gfp_t gfp_mask)
-{
-   struct scsi_host_sg_pool *sgp;
-
-   sgp = scsi_sg_pools + scsi_sgtable_index(nents);
-   return mempool_alloc(sgp->pool, gfp_mask);
-}
-
 static void scsi_free_sgtable(struct scsi_data_buffer *sdb, bool mq)
 {
-   if (mq && sdb->table.orig_nents <= SCSI_MAX_SG_SEGMENTS)
-   return;
-   __sg_free_table(>table, SCSI_MAX_SG_SEGMENTS, mq, scsi_sg_free);
+   sg_free_chained(>table, mq);
 }
 
 static int scsi_alloc_sgtable(struct scsi_data_buffer *sdb, int nents, bool mq)
 {
-   struct scatterlist *first_chunk = NULL;
+   struct scatterlist *first_chunk;
int ret;
 
-   BUG_ON(!nents);
-
-   if (mq) {
-   if (nents <= SCSI_MAX_SG_SEGMENTS) {
-   sdb->table.nents = sdb->table.orig_nents = nents;
-   sg_init_table(sdb->table.sgl, nents);
-   return 0;
-   }
+   if (mq)
first_chunk = sdb->table.sgl;
-   }
+   else
+   first_chunk = NULL;
+
+   ret = sg_alloc_chained(>table, nents, first_chunk, GFP_ATOMIC);
 
-   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);
return ret;
@@ -2264,8 +2192,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);
@@ -2274,53 +2200,12 @@ int __init scsi_init_queue(void)
return -ENOMEM;
}
 
-   for (i = 0; i < SG_MEMPOOL_NR; i++) {
-   struct scsi_host_sg_pool *sgp = scsi_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;