[PATCH net-next 0/3] bna: remove useless global variables

[PATCH net-next 0/3] bna: remove useless global variables

[Ivan Vecera]

The set removes useless global bnad_list as well as bnad->entry that track
a list of driver instances but it is not used anywhere. The associated
bnad_list_mutex is removed as well but as it is also used to protect
bna_id increment it is necessary to convert bna_id to atomic_t.

Signed-off-by: Ivan Vecera <ivecera@redhat.com>

 drivers/net/ethernet/brocade/bna/bnad.c | 27 ++-------------------------
 drivers/net/ethernet/brocade/bna/bnad.h |  1 -
 2 files changed, 2 insertions(+), 26 deletions(-)

-- 
2.7.3

[PATCH net-next 1/3] bna: remove useless linked list

[Ivan Vecera]

Remove global variable bnad_list and bnad->list_entry that are used
as list of bna driver instances. It is not necessary and useless.

Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
 drivers/net/ethernet/brocade/bna/bnad.c | 3 ---
 drivers/net/ethernet/brocade/bna/bnad.h | 1 -
 2 files changed, 4 deletions(-)

diff --git a/drivers/net/ethernet/brocade/bna/bnad.c b/drivers/net/ethernet/brocade/bna/bnad.c
index 771cc26..696bbae 100644
--- a/drivers/net/ethernet/brocade/bna/bnad.c
+++ b/drivers/net/ethernet/brocade/bna/bnad.c
@@ -56,7 +56,6 @@ MODULE_PARM_DESC(bna_debugfs_enable, "Enables debugfs feature, default=1,"
 static u32 bnad_rxqs_per_cq = 2;
 static u32 bna_id;
 static struct mutex bnad_list_mutex;
-static LIST_HEAD(bnad_list);
 static const u8 bnad_bcast_addr[] __aligned(2) =
 	{ 0xff, 0xff, 0xff, 0xff, 0xff, 0xff };
 
@@ -80,7 +79,6 @@ static void
 bnad_add_to_list(struct bnad *bnad)
 {
 	mutex_lock(&bnad_list_mutex);
-	list_add_tail(&bnad->list_entry, &bnad_list);
 	bnad->id = bna_id++;
 	mutex_unlock(&bnad_list_mutex);
 }
@@ -89,7 +87,6 @@ static void
 bnad_remove_from_list(struct bnad *bnad)
 {
 	mutex_lock(&bnad_list_mutex);
-	list_del(&bnad->list_entry);
 	mutex_unlock(&bnad_list_mutex);
 }
 
diff --git a/drivers/net/ethernet/brocade/bna/bnad.h b/drivers/net/ethernet/brocade/bna/bnad.h
index f4ed816..46f7b84 100644
--- a/drivers/net/ethernet/brocade/bna/bnad.h
+++ b/drivers/net/ethernet/brocade/bna/bnad.h
@@ -288,7 +288,6 @@ struct bnad_rx_unmap_q {
 struct bnad {
 	struct net_device	*netdev;
 	u32			id;
-	struct list_head	list_entry;
 
 	/* Data path */
 	struct bnad_tx_info tx_info[BNAD_MAX_TX];
-- 
2.7.3

[PATCH net-next 2/3] bna: change type of bna_id to atomic_t

[Ivan Vecera]

Change type of bna_id to atomic_t. The bnad_list_mutex is used to prevent
a race when bna_id is incremented. After the change the mutex can be
removed in the next step.

Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
 drivers/net/ethernet/brocade/bna/bnad.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/brocade/bna/bnad.c b/drivers/net/ethernet/brocade/bna/bnad.c
index 696bbae..2bed050 100644
--- a/drivers/net/ethernet/brocade/bna/bnad.c
+++ b/drivers/net/ethernet/brocade/bna/bnad.c
@@ -54,7 +54,7 @@ MODULE_PARM_DESC(bna_debugfs_enable, "Enables debugfs feature, default=1,"
  * Global variables
  */
 static u32 bnad_rxqs_per_cq = 2;
-static u32 bna_id;
+static atomic_t bna_id;
 static struct mutex bnad_list_mutex;
 static const u8 bnad_bcast_addr[] __aligned(2) =
 	{ 0xff, 0xff, 0xff, 0xff, 0xff, 0xff };
@@ -79,7 +79,6 @@ static void
 bnad_add_to_list(struct bnad *bnad)
 {
 	mutex_lock(&bnad_list_mutex);
-	bnad->id = bna_id++;
 	mutex_unlock(&bnad_list_mutex);
 }
 
@@ -3651,6 +3650,7 @@ bnad_pci_probe(struct pci_dev *pdev,
 	bnad = netdev_priv(netdev);
 	bnad_lock_init(bnad);
 	bnad_add_to_list(bnad);
+	bnad->id = atomic_inc_return(&bna_id) - 1;
 
 	mutex_lock(&bnad->conf_mutex);
 	/*
-- 
2.7.3

RE: [PATCH net-next 2/3] bna: change type of bna_id to atomic_t

[David Laight]

From: Ivan Vecera
> Sent: 29 July 2016 18:53
> Change type of bna_id to atomic_t. The bnad_list_mutex is used to prevent
> a race when bna_id is incremented. After the change the mutex can be
> removed in the next step.
...
> -static u32 bna_id;
> +static atomic_t bna_id;
...
>  	bnad_lock_init(bnad);
>  	bnad_add_to_list(bnad);
> +	bnad->id = atomic_inc_return(&bna_id) - 1;
...

Is the 'id' expected to be unique?
Looks like they get reallocated after 2^32 allocates.

	David

Re: [PATCH net-next 2/3] bna: change type of bna_id to atomic_t

[Ivan Vecera]

On 1.8.2016 16:20, David Laight wrote:
> From: Ivan Vecera
>> Sent: 29 July 2016 18:53
>> Change type of bna_id to atomic_t. The bnad_list_mutex is used to prevent
>> a race when bna_id is incremented. After the change the mutex can be
>> removed in the next step.
> ...
>> -static u32 bna_id;
>> +static atomic_t bna_id;
> ...
>>  	bnad_lock_init(bnad);
>>  	bnad_add_to_list(bnad);
>> +	bnad->id = atomic_inc_return(&bna_id) - 1;
> ...
>
> Is the 'id' expected to be unique?
> Looks like they get reallocated after 2^32 allocates.
>
> 	David
>
>
I have just converted it to atomic type to eliminate mutex. The id is 
only used as parameter 'instance' for bfa_nw_flash_{update,read}_part(). 
There is no documentation in source code regarding this so this is a 
question to Qlogic.

Ivan

[PATCH net-next 3/3] bna: remove global bnad_list_mutex

[Ivan Vecera]

Remove global bnad_list_mutex as it is not used anymore. This makes
bnad_add_to_list() and bnad_remove_from_list() empty so remove them too.

Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
 drivers/net/ethernet/brocade/bna/bnad.c | 20 --------------------
 1 file changed, 20 deletions(-)

diff --git a/drivers/net/ethernet/brocade/bna/bnad.c b/drivers/net/ethernet/brocade/bna/bnad.c
index 2bed050..f9df4b5 100644
--- a/drivers/net/ethernet/brocade/bna/bnad.c
+++ b/drivers/net/ethernet/brocade/bna/bnad.c
@@ -55,7 +55,6 @@ MODULE_PARM_DESC(bna_debugfs_enable, "Enables debugfs feature, default=1,"
  */
 static u32 bnad_rxqs_per_cq = 2;
 static atomic_t bna_id;
-static struct mutex bnad_list_mutex;
 static const u8 bnad_bcast_addr[] __aligned(2) =
 	{ 0xff, 0xff, 0xff, 0xff, 0xff, 0xff };
 
@@ -75,20 +74,6 @@ do {								\
 	(_res_info)->res_u.mem_info.len = (_size);		\
 } while (0)
 
-static void
-bnad_add_to_list(struct bnad *bnad)
-{
-	mutex_lock(&bnad_list_mutex);
-	mutex_unlock(&bnad_list_mutex);
-}
-
-static void
-bnad_remove_from_list(struct bnad *bnad)
-{
-	mutex_lock(&bnad_list_mutex);
-	mutex_unlock(&bnad_list_mutex);
-}
-
 /*
  * Reinitialize completions in CQ, once Rx is taken down
  */
@@ -3569,14 +3554,12 @@ bnad_lock_init(struct bnad *bnad)
 {
 	spin_lock_init(&bnad->bna_lock);
 	mutex_init(&bnad->conf_mutex);
-	mutex_init(&bnad_list_mutex);
 }
 
 static void
 bnad_lock_uninit(struct bnad *bnad)
 {
 	mutex_destroy(&bnad->conf_mutex);
-	mutex_destroy(&bnad_list_mutex);
 }
 
 /* PCI Initialization */
@@ -3649,7 +3632,6 @@ bnad_pci_probe(struct pci_dev *pdev,
 	}
 	bnad = netdev_priv(netdev);
 	bnad_lock_init(bnad);
-	bnad_add_to_list(bnad);
 	bnad->id = atomic_inc_return(&bna_id) - 1;
 
 	mutex_lock(&bnad->conf_mutex);
@@ -3804,7 +3786,6 @@ pci_uninit:
 	bnad_pci_uninit(pdev);
 unlock_mutex:
 	mutex_unlock(&bnad->conf_mutex);
-	bnad_remove_from_list(bnad);
 	bnad_lock_uninit(bnad);
 	free_netdev(netdev);
 	return err;
@@ -3842,7 +3823,6 @@ bnad_pci_remove(struct pci_dev *pdev)
 	bnad_disable_msix(bnad);
 	bnad_pci_uninit(pdev);
 	mutex_unlock(&bnad->conf_mutex);
-	bnad_remove_from_list(bnad);
 	bnad_lock_uninit(bnad);
 	/* Remove the debugfs node for this bnad */
 	kfree(bnad->regdata);
-- 
2.7.3