[PATCH 0/3] scsi: bnx2fc: simplify allocation

[PATCH 0/3] scsi: bnx2fc: simplify allocation

[Rosen Penev]

Use flexible array members to combine allocations and simplify freeing.

Rosen Penev (3):
  scsi: bnx2fc: simplify allocation of cmgr
  scsi: bnx2fc: no double pointer for io_bdt_pool
  scsi: bnx2fc: tgt_ofld_list to FAM

 drivers/scsi/bnx2fc/bnx2fc.h      | 10 ++---
 drivers/scsi/bnx2fc/bnx2fc_fcoe.c | 14 +-----
 drivers/scsi/bnx2fc/bnx2fc_io.c   | 73 +++++--------------------------
 3 files changed, 18 insertions(+), 79 deletions(-)

--
2.54.0

[PATCH 1/3] scsi: bnx2fc: simplify allocation of cmgr

[Rosen Penev]

Switch to flexible array member for cmds so that all members can be
referenced properly instead of using + 1 hacks.

Allocate io_bdt_pool, free_list, and free_list_lock with the struct to
avoid separate kfrees.

Remove mem_size variable. It's just open coding kzalloc_objs.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/scsi/bnx2fc/bnx2fc.h    |  2 +-
 drivers/scsi/bnx2fc/bnx2fc_io.c | 52 +++++----------------------------
 2 files changed, 9 insertions(+), 45 deletions(-)

diff --git a/drivers/scsi/bnx2fc/bnx2fc.h b/drivers/scsi/bnx2fc/bnx2fc.h
index 8c8968ec8cb4..30d8b563db0c 100644
--- a/drivers/scsi/bnx2fc/bnx2fc.h
+++ b/drivers/scsi/bnx2fc/bnx2fc.h
@@ -281,7 +281,7 @@ struct bnx2fc_cmd_mgr {
 	struct list_head *free_list;
 	spinlock_t *free_list_lock;
 	struct io_bdt **io_bdt_pool;
-	struct bnx2fc_cmd **cmds;
+	struct bnx2fc_cmd *cmds[];
 };
 
 struct bnx2fc_rport {
diff --git a/drivers/scsi/bnx2fc/bnx2fc_io.c b/drivers/scsi/bnx2fc/bnx2fc_io.c
index 9c7a541a4523..4810999976b6 100644
--- a/drivers/scsi/bnx2fc/bnx2fc_io.c
+++ b/drivers/scsi/bnx2fc/bnx2fc_io.c
@@ -214,7 +214,6 @@ struct bnx2fc_cmd_mgr *bnx2fc_cmd_mgr_alloc(struct bnx2fc_hba *hba)
 	struct io_bdt *bdt_info;
 	struct bnx2fc_cmd *io_req;
 	size_t len;
-	u32 mem_size;
 	u16 xid;
 	int i;
 	int num_ios, num_pri_ios;
@@ -231,8 +230,10 @@ struct bnx2fc_cmd_mgr *bnx2fc_cmd_mgr_alloc(struct bnx2fc_hba *hba)
 	BNX2FC_MISC_DBG("min xid 0x%x, max xid 0x%x\n", min_xid, max_xid);
 
 	num_ios = max_xid - min_xid + 1;
-	len = (num_ios * (sizeof(struct bnx2fc_cmd *)));
-	len += sizeof(struct bnx2fc_cmd_mgr);
+	len = struct_size(cmgr, cmds, num_ios);
+	len += sizeof(*cmgr->io_bdt_pool) * num_ios;
+	len += sizeof(*cmgr->free_list) * arr_sz;
+	len += sizeof(*cmgr->free_list_lock) * arr_sz;
 
 	cmgr = kzalloc(len, GFP_KERNEL);
 	if (!cmgr) {
@@ -241,21 +242,9 @@ struct bnx2fc_cmd_mgr *bnx2fc_cmd_mgr_alloc(struct bnx2fc_hba *hba)
 	}
 
 	cmgr->hba = hba;
-	cmgr->free_list = kzalloc_objs(*cmgr->free_list, arr_sz);
-	if (!cmgr->free_list) {
-		printk(KERN_ERR PFX "failed to alloc free_list\n");
-		goto mem_err;
-	}
-
-	cmgr->free_list_lock = kzalloc_objs(*cmgr->free_list_lock, arr_sz);
-	if (!cmgr->free_list_lock) {
-		printk(KERN_ERR PFX "failed to alloc free_list_lock\n");
-		kfree(cmgr->free_list);
-		cmgr->free_list = NULL;
-		goto mem_err;
-	}
-
-	cmgr->cmds = (struct bnx2fc_cmd **)(cmgr + 1);
+	cmgr->io_bdt_pool = (struct io_bdt **)(cmgr->cmds + num_ios);
+	cmgr->free_list = (struct list_head *)(cmgr->io_bdt_pool + num_ios);
+	cmgr->free_list_lock = (spinlock_t *)(cmgr->free_list + arr_sz);
 
 	for (i = 0; i < arr_sz; i++)  {
 		INIT_LIST_HEAD(&cmgr->free_list[i]);
@@ -291,17 +280,8 @@ struct bnx2fc_cmd_mgr *bnx2fc_cmd_mgr_alloc(struct bnx2fc_hba *hba)
 		io_req++;
 	}
 
-	/* Allocate pool of io_bdts - one for each bnx2fc_cmd */
-	mem_size = num_ios * sizeof(struct io_bdt *);
-	cmgr->io_bdt_pool = kzalloc(mem_size, GFP_KERNEL);
-	if (!cmgr->io_bdt_pool) {
-		printk(KERN_ERR PFX "failed to alloc io_bdt_pool\n");
-		goto mem_err;
-	}
-
-	mem_size = sizeof(struct io_bdt);
 	for (i = 0; i < num_ios; i++) {
-		cmgr->io_bdt_pool[i] = kmalloc(mem_size, GFP_KERNEL);
+		cmgr->io_bdt_pool[i] = kmalloc_obj(struct io_bdt);
 		if (!cmgr->io_bdt_pool[i]) {
 			printk(KERN_ERR PFX "failed to alloc "
 				"io_bdt_pool[%d]\n", i);
@@ -343,10 +323,6 @@ void bnx2fc_cmd_mgr_free(struct bnx2fc_cmd_mgr *cmgr)
 
 	num_ios = max_xid - min_xid + 1;
 
-	/* Free fcoe_bdt_ctx structures */
-	if (!cmgr->io_bdt_pool)
-		goto free_cmd_pool;
-
 	bd_tbl_sz = BNX2FC_MAX_BDS_PER_CMD * sizeof(struct fcoe_bd_ctx);
 	for (i = 0; i < num_ios; i++) {
 		bdt_info = cmgr->io_bdt_pool[i];
@@ -364,16 +340,6 @@ void bnx2fc_cmd_mgr_free(struct bnx2fc_cmd_mgr *cmgr)
 		cmgr->io_bdt_pool[i] = NULL;
 	}
 
-	kfree(cmgr->io_bdt_pool);
-	cmgr->io_bdt_pool = NULL;
-
-free_cmd_pool:
-	kfree(cmgr->free_list_lock);
-
-	/* Destroy cmd pool */
-	if (!cmgr->free_list)
-		goto free_cmgr;
-
 	for (i = 0; i < num_possible_cpus() + 1; i++)  {
 		struct bnx2fc_cmd *tmp, *io_req;
 
@@ -383,8 +349,6 @@ void bnx2fc_cmd_mgr_free(struct bnx2fc_cmd_mgr *cmgr)
 			kfree(io_req);
 		}
 	}
-	kfree(cmgr->free_list);
-free_cmgr:
 	/* Free command manager itself */
 	kfree(cmgr);
 }
-- 
2.54.0

[PATCH 2/3] scsi: bnx2fc: no double pointer for io_bdt_pool

[Rosen Penev]

Instead of allocating a double pointer, allocate a single and avoid
extra malloc and free. It's not needed anyway.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/scsi/bnx2fc/bnx2fc.h    |  2 +-
 drivers/scsi/bnx2fc/bnx2fc_io.c | 25 +++++--------------------
 2 files changed, 6 insertions(+), 21 deletions(-)

diff --git a/drivers/scsi/bnx2fc/bnx2fc.h b/drivers/scsi/bnx2fc/bnx2fc.h
index 30d8b563db0c..9a5de1048542 100644
--- a/drivers/scsi/bnx2fc/bnx2fc.h
+++ b/drivers/scsi/bnx2fc/bnx2fc.h
@@ -280,7 +280,7 @@ struct bnx2fc_cmd_mgr {
 	u16 next_idx;
 	struct list_head *free_list;
 	spinlock_t *free_list_lock;
-	struct io_bdt **io_bdt_pool;
+	struct io_bdt *io_bdt_pool;
 	struct bnx2fc_cmd *cmds[];
 };
 
diff --git a/drivers/scsi/bnx2fc/bnx2fc_io.c b/drivers/scsi/bnx2fc/bnx2fc_io.c
index 4810999976b6..6773f8d5d84e 100644
--- a/drivers/scsi/bnx2fc/bnx2fc_io.c
+++ b/drivers/scsi/bnx2fc/bnx2fc_io.c
@@ -242,7 +242,7 @@ struct bnx2fc_cmd_mgr *bnx2fc_cmd_mgr_alloc(struct bnx2fc_hba *hba)
 	}
 
 	cmgr->hba = hba;
-	cmgr->io_bdt_pool = (struct io_bdt **)(cmgr->cmds + num_ios);
+	cmgr->io_bdt_pool = (struct io_bdt *)(cmgr->cmds + num_ios);
 	cmgr->free_list = (struct list_head *)(cmgr->io_bdt_pool + num_ios);
 	cmgr->free_list_lock = (spinlock_t *)(cmgr->free_list + arr_sz);
 
@@ -280,19 +280,10 @@ struct bnx2fc_cmd_mgr *bnx2fc_cmd_mgr_alloc(struct bnx2fc_hba *hba)
 		io_req++;
 	}
 
-	for (i = 0; i < num_ios; i++) {
-		cmgr->io_bdt_pool[i] = kmalloc_obj(struct io_bdt);
-		if (!cmgr->io_bdt_pool[i]) {
-			printk(KERN_ERR PFX "failed to alloc "
-				"io_bdt_pool[%d]\n", i);
-			goto mem_err;
-		}
-	}
-
 	/* Allocate an map fcoe_bdt_ctx structures */
 	bd_tbl_sz = BNX2FC_MAX_BDS_PER_CMD * sizeof(struct fcoe_bd_ctx);
 	for (i = 0; i < num_ios; i++) {
-		bdt_info = cmgr->io_bdt_pool[i];
+		bdt_info = &cmgr->io_bdt_pool[i];
 		bdt_info->bd_tbl = dma_alloc_coherent(&hba->pcidev->dev,
 						      bd_tbl_sz,
 						      &bdt_info->bd_tbl_dma,
@@ -325,7 +316,7 @@ void bnx2fc_cmd_mgr_free(struct bnx2fc_cmd_mgr *cmgr)
 
 	bd_tbl_sz = BNX2FC_MAX_BDS_PER_CMD * sizeof(struct fcoe_bd_ctx);
 	for (i = 0; i < num_ios; i++) {
-		bdt_info = cmgr->io_bdt_pool[i];
+		bdt_info = &cmgr->io_bdt_pool[i];
 		if (bdt_info->bd_tbl) {
 			dma_free_coherent(&hba->pcidev->dev, bd_tbl_sz,
 					    bdt_info->bd_tbl,
@@ -334,12 +325,6 @@ void bnx2fc_cmd_mgr_free(struct bnx2fc_cmd_mgr *cmgr)
 		}
 	}
 
-	/* Destroy io_bdt pool */
-	for (i = 0; i < num_ios; i++) {
-		kfree(cmgr->io_bdt_pool[i]);
-		cmgr->io_bdt_pool[i] = NULL;
-	}
-
 	for (i = 0; i < num_possible_cpus() + 1; i++)  {
 		struct bnx2fc_cmd *tmp, *io_req;
 
@@ -415,7 +400,7 @@ struct bnx2fc_cmd *bnx2fc_elstm_alloc(struct bnx2fc_rport *tgt, int type)
 
 	/* Bind io_bdt for this io_req */
 	/* Have a static link between io_req and io_bdt_pool */
-	bd_tbl = io_req->bd_tbl = cmd_mgr->io_bdt_pool[xid];
+	bd_tbl = io_req->bd_tbl = &cmd_mgr->io_bdt_pool[xid];
 	bd_tbl->io_req = io_req;
 
 	/* Hold the io_req  against deletion */
@@ -468,7 +453,7 @@ struct bnx2fc_cmd *bnx2fc_cmd_alloc(struct bnx2fc_rport *tgt)
 
 	/* Bind io_bdt for this io_req */
 	/* Have a static link between io_req and io_bdt_pool */
-	bd_tbl = io_req->bd_tbl = cmd_mgr->io_bdt_pool[xid];
+	bd_tbl = io_req->bd_tbl = &cmd_mgr->io_bdt_pool[xid];
 	bd_tbl->io_req = io_req;
 
 	/* Hold the io_req  against deletion */
-- 
2.54.0

[PATCH 3/3] scsi: bnx2fc: tgt_ofld_list to FAM

[Rosen Penev]

Simplify allocation slightly by allocating tgt_ofld_list with hba. No
need to kfree separately.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/scsi/bnx2fc/bnx2fc.h      |  6 +++---
 drivers/scsi/bnx2fc/bnx2fc_fcoe.c | 14 ++------------
 2 files changed, 5 insertions(+), 15 deletions(-)

diff --git a/drivers/scsi/bnx2fc/bnx2fc.h b/drivers/scsi/bnx2fc/bnx2fc.h
index 9a5de1048542..31bb701cc5ea 100644
--- a/drivers/scsi/bnx2fc/bnx2fc.h
+++ b/drivers/scsi/bnx2fc/bnx2fc.h
@@ -224,9 +224,6 @@ struct bnx2fc_hba {
 	char *dummy_buffer;
 	dma_addr_t dummy_buf_dma;
 
-	/* Active list of offloaded sessions */
-	struct bnx2fc_rport **tgt_ofld_list;
-
 	/* statistics */
 	struct bnx2fc_fw_stats bfw_stats;
 	struct fcoe_statistics_params prev_stats;
@@ -246,6 +243,9 @@ struct bnx2fc_hba {
 	struct list_head vports;
 
 	char chip_num[BCM_CHIP_LEN];
+
+	/* Active list of offloaded sessions */
+	struct bnx2fc_rport *tgt_ofld_list[];
 };
 
 struct bnx2fc_interface {
diff --git a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
index 26e0ff380860..7e740ac6bf1b 100644
--- a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
+++ b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
@@ -1337,7 +1337,6 @@ static void bnx2fc_hba_destroy(struct bnx2fc_hba *hba)
 		bnx2fc_cmd_mgr_free(hba->cmd_mgr);
 		hba->cmd_mgr = NULL;
 	}
-	kfree(hba->tgt_ofld_list);
 	bnx2fc_unbind_pcidev(hba);
 	kfree(hba);
 }
@@ -1356,7 +1355,7 @@ static struct bnx2fc_hba *bnx2fc_hba_create(struct cnic_dev *cnic)
 	struct fcoe_capabilities *fcoe_cap;
 	int rc;
 
-	hba = kzalloc_obj(*hba);
+	hba = kzalloc_flex(*hba, tgt_ofld_list, BNX2FC_NUM_MAX_SESS);
 	if (!hba) {
 		printk(KERN_ERR PFX "Unable to allocate hba structure\n");
 		return NULL;
@@ -1380,19 +1379,12 @@ static struct bnx2fc_hba *bnx2fc_hba_create(struct cnic_dev *cnic)
 	hba->phys_dev = cnic->netdev;
 	hba->next_conn_id = 0;
 
-	hba->tgt_ofld_list =
-		kzalloc_objs(struct bnx2fc_rport *, BNX2FC_NUM_MAX_SESS);
-	if (!hba->tgt_ofld_list) {
-		printk(KERN_ERR PFX "Unable to allocate tgt offload list\n");
-		goto tgtofld_err;
-	}
-
 	hba->num_ofld_sess = 0;
 
 	hba->cmd_mgr = bnx2fc_cmd_mgr_alloc(hba);
 	if (!hba->cmd_mgr) {
 		printk(KERN_ERR PFX "em_config:bnx2fc_cmd_mgr_alloc failed\n");
-		goto cmgr_err;
+		goto tgtofld_err;
 	}
 	fcoe_cap = &hba->fcoe_cap;
 
@@ -1416,8 +1408,6 @@ static struct bnx2fc_hba *bnx2fc_hba_create(struct cnic_dev *cnic)
 
 	return hba;
 
-cmgr_err:
-	kfree(hba->tgt_ofld_list);
 tgtofld_err:
 	bnx2fc_unbind_pcidev(hba);
 bind_err:
-- 
2.54.0