[PATCH 0/2] scsi: lpfc: Cleanup some bitmap handling.

[PATCH 0/2] scsi: lpfc: Cleanup some bitmap handling.

[Christophe JAILLET]

The 1st patch of this serie uses bitmap_zalloc() to simplify code.
The 2nd patch makes sure that the whole bitmap is cleared, should it need more
than 1 long.

However, this 'fcf_rr_bmask' is small (LPFC_SLI4_FCF_TBL_INDX_MAX = 32 bits), so
another option could be to use DECLARE_BITMAP instead of dynamic allocation.

This would simplify code (no more allocation and associated error handling) and
be slighly more efficient (1 less indirection when accessing the bitmap).
In the case the 2nd patch could also be removed (using bitmap_zero() would still
be cleaner (IMHO), but the actual memset+sizeof would work as expected)

Let me know if I should send a v2 with this other approach.

Christophe JAILLET (2):
  scsi: lpfc: Use bitmap_zalloc() when applicable
  scsi: lpfc: Make sure to completely clear some bitmaps

 drivers/scsi/lpfc/lpfc_hbadisc.c |  7 ++++---
 drivers/scsi/lpfc/lpfc_init.c    | 10 ++++------
 drivers/scsi/lpfc/lpfc_sli.c     |  3 +--
 3 files changed, 9 insertions(+), 11 deletions(-)

-- 
2.32.0

[PATCH 1/2] scsi: lpfc: Use bitmap_zalloc() when applicable

[Christophe JAILLET]

'phba->fcf.fcf_rr_bmask' is a bitmap. So use bitmap_zalloc() to simplify
code, improve the semantic and avoid some open-coded arithmetic in
allocator arguments.

Also change the corresponding kfree() into bitmap_free() to keep
consistency.

Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
 drivers/scsi/lpfc/lpfc_init.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/lpfc/lpfc_init.c b/drivers/scsi/lpfc/lpfc_init.c
index a56f01f659f8..39e6e53773ee 100644
--- a/drivers/scsi/lpfc/lpfc_init.c
+++ b/drivers/scsi/lpfc/lpfc_init.c
@@ -7858,7 +7858,6 @@ lpfc_sli4_driver_resource_setup(struct lpfc_hba *phba)
 	LPFC_MBOXQ_t *mboxq;
 	MAILBOX_t *mb;
 	int rc, i, max_buf_size;
-	int longs;
 	int extra;
 	uint64_t wwn;
 	u32 if_type;
@@ -8286,9 +8285,8 @@ lpfc_sli4_driver_resource_setup(struct lpfc_hba *phba)
 	}
 
 	/* Allocate eligible FCF bmask memory for FCF roundrobin failover */
-	longs = (LPFC_SLI4_FCF_TBL_INDX_MAX + BITS_PER_LONG - 1)/BITS_PER_LONG;
-	phba->fcf.fcf_rr_bmask = kcalloc(longs, sizeof(unsigned long),
-					 GFP_KERNEL);
+	phba->fcf.fcf_rr_bmask = bitmap_zalloc(LPFC_SLI4_FCF_TBL_INDX_MAX,
+					       GFP_KERNEL);
 	if (!phba->fcf.fcf_rr_bmask) {
 		lpfc_printf_log(phba, KERN_ERR, LOG_TRACE_EVENT,
 				"2759 Failed allocate memory for FCF round "
@@ -8387,7 +8385,7 @@ lpfc_sli4_driver_resource_setup(struct lpfc_hba *phba)
 out_free_hba_eq_hdl:
 	kfree(phba->sli4_hba.hba_eq_hdl);
 out_free_fcf_rr_bmask:
-	kfree(phba->fcf.fcf_rr_bmask);
+	bitmap_free(phba->fcf.fcf_rr_bmask);
 out_remove_rpi_hdrs:
 	lpfc_sli4_remove_rpi_hdrs(phba);
 out_free_active_sgl:
@@ -8441,7 +8439,7 @@ lpfc_sli4_driver_resource_unset(struct lpfc_hba *phba)
 	lpfc_sli4_remove_rpis(phba);
 
 	/* Free eligible FCF index bmask */
-	kfree(phba->fcf.fcf_rr_bmask);
+	bitmap_free(phba->fcf.fcf_rr_bmask);
 
 	/* Free the ELS sgl list */
 	lpfc_free_active_sgl(phba);
-- 
2.32.0

[PATCH 2/2] scsi: lpfc: Make sure to completely clear some bitmaps

[Christophe JAILLET]

When the 'phba->fcf.fcf_rr_bmask' bitmap is cleared, the code makes the
assumption that it fits in only 1 long.
Depending on the value of LPFC_SLI4_FCF_TBL_INDX_MAX, this may be wrong
and only part of the bitmap would be cleared.

Up to now, LPFC_SLI4_FCF_TBL_INDX_MAX is 32, so it is not an issue. But
using bitmap_zero() is more future proof.

Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
 drivers/scsi/lpfc/lpfc_hbadisc.c | 7 ++++---
 drivers/scsi/lpfc/lpfc_sli.c     | 3 +--
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/scsi/lpfc/lpfc_hbadisc.c b/drivers/scsi/lpfc/lpfc_hbadisc.c
index 816fc406135b..5f142ee639ad 100644
--- a/drivers/scsi/lpfc/lpfc_hbadisc.c
+++ b/drivers/scsi/lpfc/lpfc_hbadisc.c
@@ -1517,7 +1517,8 @@ lpfc_sli4_clear_fcf_rr_bmask(struct lpfc_hba *phba)
 {
 	struct lpfc_fcf_pri *fcf_pri;
 	struct lpfc_fcf_pri *next_fcf_pri;
-	memset(phba->fcf.fcf_rr_bmask, 0, sizeof(*phba->fcf.fcf_rr_bmask));
+
+	bitmap_zero(phba->fcf.fcf_rr_bmask, LPFC_SLI4_FCF_TBL_INDX_MAX);
 	spin_lock_irq(&phba->hbalock);
 	list_for_each_entry_safe(fcf_pri, next_fcf_pri,
 				&phba->fcf.fcf_pri_list, list) {
@@ -2476,8 +2477,8 @@ static int lpfc_sli4_fcf_pri_list_add(struct lpfc_hba *phba,
 	if (new_fcf_pri->fcf_rec.priority <=  current_fcf_pri) {
 		list_add(&new_fcf_pri->list, &phba->fcf.fcf_pri_list);
 		if (new_fcf_pri->fcf_rec.priority <  current_fcf_pri) {
-			memset(phba->fcf.fcf_rr_bmask, 0,
-				sizeof(*phba->fcf.fcf_rr_bmask));
+			bitmap_zero(phba->fcf.fcf_rr_bmask,
+				    LPFC_SLI4_FCF_TBL_INDX_MAX);
 			/* fcfs_at_this_priority_level = 1; */
 			phba->fcf.eligible_fcf_cnt = 1;
 		} else
diff --git a/drivers/scsi/lpfc/lpfc_sli.c b/drivers/scsi/lpfc/lpfc_sli.c
index 1bc0db572d9e..9ea1b0db42a6 100644
--- a/drivers/scsi/lpfc/lpfc_sli.c
+++ b/drivers/scsi/lpfc/lpfc_sli.c
@@ -20220,8 +20220,7 @@ lpfc_check_next_fcf_pri_level(struct lpfc_hba *phba)
 	 * Clear the rr_bmask and set all of the bits that are at this
 	 * priority.
 	 */
-	memset(phba->fcf.fcf_rr_bmask, 0,
-			sizeof(*phba->fcf.fcf_rr_bmask));
+	bitmap_zero(phba->fcf.fcf_rr_bmask, LPFC_SLI4_FCF_TBL_INDX_MAX);
 	spin_lock_irq(&phba->hbalock);
 	list_for_each_entry(fcf_pri, &phba->fcf.fcf_pri_list, list) {
 		if (fcf_pri->fcf_rec.flag & LPFC_FCF_FLOGI_FAILED)
-- 
2.32.0