[PATCH net-next 0/2] net/mlx4_core: Deprecate module parameter use_prio

[PATCH net-next 0/2] net/mlx4_core: Deprecate module parameter use_prio

[Amir Vadai]

Hi,

This small patchset deprecates the mlx4_core module paramater 'use_prio', as
suggested by Carol Soto from IBM in [1].
Also, replaced some calls to the prefered pr_warn/info/devel macro's.

Patchset was applied and tested on commit b6052af: "Merge tag
'batman-adv-for-davem' of git://git.open-mesh.org/linux-merge"

[1] - http://marc.info/?l=linux-netdev&m=139871350103432&w=2

Thanks,
Amir


Amir Vadai (2):
  net/mlx4_core: Deprecate use_prio module parameter
  net/mlx4_core: Replace pr_warning() with pr_warn()

 drivers/net/ethernet/mellanox/mlx4/cmd.c            |  6 +++---
 drivers/net/ethernet/mellanox/mlx4/main.c           | 21 +++++++++++----------
 drivers/net/ethernet/mellanox/mlx4/mr.c             |  6 ++----
 .../net/ethernet/mellanox/mlx4/resource_tracker.c   | 10 +++++-----
 include/linux/mlx4/device.h                         |  1 -
 5 files changed, 21 insertions(+), 23 deletions(-)

-- 
1.8.3.4

[PATCH net-next 1/2] net/mlx4_core: Deprecate use_prio module parameter

[Amir Vadai]

It was used for steering by user priority for A0 steering. A0 mode is
not supported anymore. Printing a message and ignoring the parameter.

CC: Carol Soto <clsoto@linux.vnet.ibm.com>
Signed-off-by: Amir Vadai <amirv@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx4/main.c | 8 ++++----
 include/linux/mlx4/device.h               | 1 -
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/main.c b/drivers/net/ethernet/mellanox/mlx4/main.c
index a56f601..08ff5dd 100644
--- a/drivers/net/ethernet/mellanox/mlx4/main.c
+++ b/drivers/net/ethernet/mellanox/mlx4/main.c
@@ -132,8 +132,7 @@ MODULE_PARM_DESC(log_num_vlan, "Log2 max number of VLANs per ETH port (0-7)");
 
 static bool use_prio;
 module_param_named(use_prio, use_prio, bool, 0444);
-MODULE_PARM_DESC(use_prio, "Enable steering by VLAN priority on ETH ports "
-		  "(0/1, default 0)");
+MODULE_PARM_DESC(use_prio, "Enable steering by VLAN priority on ETH ports (deprecated)");
 
 int log_mtts_per_seg = ilog2(MLX4_MTT_ENTRY_PER_SEG);
 module_param_named(log_mtts_per_seg, log_mtts_per_seg, int, 0444);
@@ -290,7 +289,6 @@ static int mlx4_dev_cap(struct mlx4_dev *dev, struct mlx4_dev_cap *dev_cap)
 
 	dev->caps.log_num_macs  = log_num_mac;
 	dev->caps.log_num_vlans = MLX4_LOG_NUM_VLANS;
-	dev->caps.log_num_prios = use_prio ? 3 : 0;
 
 	for (i = 1; i <= dev->caps.num_ports; ++i) {
 		dev->caps.port_type[i] = MLX4_PORT_TYPE_NONE;
@@ -358,7 +356,6 @@ static int mlx4_dev_cap(struct mlx4_dev *dev, struct mlx4_dev_cap *dev_cap)
 		dev->caps.reserved_qps_cnt[MLX4_QP_REGION_FC_ADDR] =
 		(1 << dev->caps.log_num_macs) *
 		(1 << dev->caps.log_num_vlans) *
-		(1 << dev->caps.log_num_prios) *
 		dev->caps.num_ports;
 	dev->caps.reserved_qps_cnt[MLX4_QP_REGION_FC_EXCH] = MLX4_NUM_FEXCH;
 
@@ -2775,6 +2772,9 @@ static int __init mlx4_verify_params(void)
 		pr_warning("mlx4_core: log_num_vlan - obsolete module param, using %d\n",
 			   MLX4_LOG_NUM_VLANS);
 
+	if (use_prio != 0)
+		pr_warn("mlx4_core: use_prio - obsolete module param, ignored\n");
+
 	if ((log_mtts_per_seg < 1) || (log_mtts_per_seg > 7)) {
 		pr_warning("mlx4_core: bad log_mtts_per_seg: %d\n", log_mtts_per_seg);
 		return -1;
diff --git a/include/linux/mlx4/device.h b/include/linux/mlx4/device.h
index c0468e6..ca38871 100644
--- a/include/linux/mlx4/device.h
+++ b/include/linux/mlx4/device.h
@@ -449,7 +449,6 @@ struct mlx4_caps {
 	int                     reserved_qps_base[MLX4_NUM_QP_REGION];
 	int                     log_num_macs;
 	int                     log_num_vlans;
-	int                     log_num_prios;
 	enum mlx4_port_type	port_type[MLX4_MAX_PORTS + 1];
 	u8			supported_type[MLX4_MAX_PORTS + 1];
 	u8                      suggested_type[MLX4_MAX_PORTS + 1];
-- 
1.8.3.4

[PATCH net-next 2/2] net/mlx4_core: Replace pr_warning() with pr_warn()

[Amir Vadai]

As checkpatch suggests. Also changed some printk's into pr_*

Signed-off-by: Amir Vadai <amirv@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx4/cmd.c              |  6 +++---
 drivers/net/ethernet/mellanox/mlx4/main.c             | 13 +++++++------
 drivers/net/ethernet/mellanox/mlx4/mr.c               |  6 ++----
 drivers/net/ethernet/mellanox/mlx4/resource_tracker.c | 10 +++++-----
 4 files changed, 17 insertions(+), 18 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/cmd.c b/drivers/net/ethernet/mellanox/mlx4/cmd.c
index 357dcb0..59c7fd4 100644
--- a/drivers/net/ethernet/mellanox/mlx4/cmd.c
+++ b/drivers/net/ethernet/mellanox/mlx4/cmd.c
@@ -1887,9 +1887,9 @@ void mlx4_master_comm_channel(struct work_struct *work)
 			if (toggle != slt) {
 				if (master->slave_state[slave].comm_toggle
 				    != slt) {
-					printk(KERN_INFO "slave %d out of sync. read toggle %d, state toggle %d. Resynching.\n",
-					       slave, slt,
-					       master->slave_state[slave].comm_toggle);
+					pr_info("slave %d out of sync. read toggle %d, state toggle %d. Resynching.\n",
+						slave, slt,
+						master->slave_state[slave].comm_toggle);
 					master->slave_state[slave].comm_toggle =
 						slt;
 				}
diff --git a/drivers/net/ethernet/mellanox/mlx4/main.c b/drivers/net/ethernet/mellanox/mlx4/main.c
index 08ff5dd..38e9a4c 100644
--- a/drivers/net/ethernet/mellanox/mlx4/main.c
+++ b/drivers/net/ethernet/mellanox/mlx4/main.c
@@ -2593,7 +2593,7 @@ static void __mlx4_remove_one(struct pci_dev *pdev)
 	/* in SRIOV it is not allowed to unload the pf's
 	 * driver while there are alive vf's */
 	if (mlx4_is_master(dev) && mlx4_how_many_lives_vf(dev))
-		printk(KERN_ERR "Removing PF when there are assigned VF's !!!\n");
+		pr_warn("Removing PF when there are assigned VF's !!!\n");
 	mlx4_stop_sense(dev);
 	mlx4_unregister_device(dev);
 
@@ -2764,25 +2764,26 @@ static struct pci_driver mlx4_driver = {
 static int __init mlx4_verify_params(void)
 {
 	if ((log_num_mac < 0) || (log_num_mac > 7)) {
-		pr_warning("mlx4_core: bad num_mac: %d\n", log_num_mac);
+		pr_warn("mlx4_core: bad num_mac: %d\n", log_num_mac);
 		return -1;
 	}
 
 	if (log_num_vlan != 0)
-		pr_warning("mlx4_core: log_num_vlan - obsolete module param, using %d\n",
-			   MLX4_LOG_NUM_VLANS);
+		pr_warn("mlx4_core: log_num_vlan - obsolete module param, using %d\n",
+			MLX4_LOG_NUM_VLANS);
 
 	if (use_prio != 0)
 		pr_warn("mlx4_core: use_prio - obsolete module param, ignored\n");
 
 	if ((log_mtts_per_seg < 1) || (log_mtts_per_seg > 7)) {
-		pr_warning("mlx4_core: bad log_mtts_per_seg: %d\n", log_mtts_per_seg);
+		pr_warn("mlx4_core: bad log_mtts_per_seg: %d\n",
+			log_mtts_per_seg);
 		return -1;
 	}
 
 	/* Check if module param for ports type has legal combination */
 	if (port_type_array[0] == false && port_type_array[1] == true) {
-		printk(KERN_WARNING "Module parameter configuration ETH/IB is not supported. Switching to default configuration IB/IB\n");
+		pr_warn("Module parameter configuration ETH/IB is not supported. Switching to default configuration IB/IB\n");
 		port_type_array[0] = true;
 	}
 
diff --git a/drivers/net/ethernet/mellanox/mlx4/mr.c b/drivers/net/ethernet/mellanox/mlx4/mr.c
index 64fb3e6..3e04ea1 100644
--- a/drivers/net/ethernet/mellanox/mlx4/mr.c
+++ b/drivers/net/ethernet/mellanox/mlx4/mr.c
@@ -954,8 +954,7 @@ void mlx4_fmr_unmap(struct mlx4_dev *dev, struct mlx4_fmr *fmr,
 	mailbox = mlx4_alloc_cmd_mailbox(dev);
 	if (IS_ERR(mailbox)) {
 		err = PTR_ERR(mailbox);
-		printk(KERN_WARNING "mlx4_ib: mlx4_alloc_cmd_mailbox failed (%d)\n",
-		       err);
+		pr_warn("mlx4_ib: mlx4_alloc_cmd_mailbox failed (%d)\n", err);
 		return;
 	}
 
@@ -964,8 +963,7 @@ void mlx4_fmr_unmap(struct mlx4_dev *dev, struct mlx4_fmr *fmr,
 			     (dev->caps.num_mpts - 1));
 	mlx4_free_cmd_mailbox(dev, mailbox);
 	if (err) {
-		printk(KERN_WARNING "mlx4_ib: mlx4_HW2SW_MPT failed (%d)\n",
-		       err);
+		pr_warn("mlx4_ib: mlx4_HW2SW_MPT failed (%d)\n", err);
 		return;
 	}
 	fmr->mr.enabled = MLX4_MPT_EN_SW;
diff --git a/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c b/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c
index a95df9d..4094e11 100644
--- a/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c
+++ b/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c
@@ -962,7 +962,7 @@ static struct res_common *alloc_tr(u64 id, enum mlx4_resource type, int slave,
 		ret = alloc_srq_tr(id);
 		break;
 	case RES_MAC:
-		printk(KERN_ERR "implementation missing\n");
+		pr_err("implementation missing\n");
 		return NULL;
 	case RES_COUNTER:
 		ret = alloc_counter_tr(id);
@@ -1056,10 +1056,10 @@ static int remove_mtt_ok(struct res_mtt *res, int order)
 {
 	if (res->com.state == RES_MTT_BUSY ||
 	    atomic_read(&res->ref_count)) {
-		printk(KERN_DEBUG "%s-%d: state %s, ref_count %d\n",
-		       __func__, __LINE__,
-		       mtt_states_str(res->com.state),
-		       atomic_read(&res->ref_count));
+		pr_devel("%s-%d: state %s, ref_count %d\n",
+			 __func__, __LINE__,
+			 mtt_states_str(res->com.state),
+			 atomic_read(&res->ref_count));
 		return -EBUSY;
 	} else if (res->com.state != RES_MTT_ALLOCATED)
 		return -EPERM;
-- 
1.8.3.4

Re: [PATCH net-next 1/2] net/mlx4_core: Deprecate use_prio module parameter

[David Miller]

From: Amir Vadai <amirv@mellanox.com>
Date: Mon, 19 May 2014 11:37:41 +0300

> It was used for steering by user priority for A0 steering. A0 mode is
> not supported anymore. Printing a message and ignoring the parameter.
> 
> CC: Carol Soto <clsoto@linux.vnet.ibm.com>
> Signed-off-by: Amir Vadai <amirv@mellanox.com>

So you completely took away a feature that was publicly exposed and
available to the user?

Sorry this is still not acceptable, you have to bring back A0
steering, what if someone was using it?

I sense you really don't care, and that I find really disturbing.

Re: [PATCH net-next 1/2] net/mlx4_core: Deprecate use_prio module parameter

[Amir Vadai]

On 5/21/2014 10:49 PM, David Miller wrote:
> From: Amir Vadai <amirv@mellanox.com>
> Date: Mon, 19 May 2014 11:37:41 +0300
>
>> It was used for steering by user priority for A0 steering. A0 mode is
>> not supported anymore. Printing a message and ignoring the parameter.
>>
>> CC: Carol Soto <clsoto@linux.vnet.ibm.com>
>> Signed-off-by: Amir Vadai <amirv@mellanox.com>
>
> So you completely took away a feature that was publicly exposed and
> available to the user?
>
> Sorry this is still not acceptable, you have to bring back A0
> steering, what if someone was using it?
>
> I sense you really don't care, and that I find really disturbing.
>

My description wasn't accurate. I resubmitted the patch set with a 
better explanation about the purpose of this module param, and why we 
don't expect that anything will break for anyone because we deprecated it.

Amir

Re: [PATCH net-next 1/2] net/mlx4_core: Deprecate use_prio module parameter

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 3241 bytes --]

On Mon, 2014-05-19 at 11:37 +0300, Amir Vadai wrote:
> It was used for steering by user priority for A0 steering. A0 mode is
> not supported anymore. Printing a message and ignoring the parameter.
> 
> CC: Carol Soto <clsoto@linux.vnet.ibm.com>
> Signed-off-by: Amir Vadai <amirv@mellanox.com>
> ---
>  drivers/net/ethernet/mellanox/mlx4/main.c | 8 ++++----
>  include/linux/mlx4/device.h               | 1 -
>  2 files changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx4/main.c b/drivers/net/ethernet/mellanox/mlx4/main.c
> index a56f601..08ff5dd 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/main.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/main.c
> @@ -132,8 +132,7 @@ MODULE_PARM_DESC(log_num_vlan, "Log2 max number of VLANs per ETH port (0-7)");
>  
>  static bool use_prio;
>  module_param_named(use_prio, use_prio, bool, 0444);

Perhaps the sysfs permissions should be set to 0 (i.e. not visible in
sysfs)?

> -MODULE_PARM_DESC(use_prio, "Enable steering by VLAN priority on ETH ports "
> -		  "(0/1, default 0)");
> +MODULE_PARM_DESC(use_prio, "Enable steering by VLAN priority on ETH ports (deprecated)");
>
>  int log_mtts_per_seg = ilog2(MLX4_MTT_ENTRY_PER_SEG);
>  module_param_named(log_mtts_per_seg, log_mtts_per_seg, int, 0444);
> @@ -290,7 +289,6 @@ static int mlx4_dev_cap(struct mlx4_dev *dev, struct mlx4_dev_cap *dev_cap)
>  
>  	dev->caps.log_num_macs  = log_num_mac;
>  	dev->caps.log_num_vlans = MLX4_LOG_NUM_VLANS;
> -	dev->caps.log_num_prios = use_prio ? 3 : 0;
>  
>  	for (i = 1; i <= dev->caps.num_ports; ++i) {
>  		dev->caps.port_type[i] = MLX4_PORT_TYPE_NONE;
> @@ -358,7 +356,6 @@ static int mlx4_dev_cap(struct mlx4_dev *dev, struct mlx4_dev_cap *dev_cap)
>  		dev->caps.reserved_qps_cnt[MLX4_QP_REGION_FC_ADDR] =
>  		(1 << dev->caps.log_num_macs) *
>  		(1 << dev->caps.log_num_vlans) *
> -		(1 << dev->caps.log_num_prios) *
>  		dev->caps.num_ports;
>  	dev->caps.reserved_qps_cnt[MLX4_QP_REGION_FC_EXCH] = MLX4_NUM_FEXCH;
>  
> @@ -2775,6 +2772,9 @@ static int __init mlx4_verify_params(void)
>  		pr_warning("mlx4_core: log_num_vlan - obsolete module param, using %d\n",
>  			   MLX4_LOG_NUM_VLANS);
>  
> +	if (use_prio != 0)
> +		pr_warn("mlx4_core: use_prio - obsolete module param, ignored\n");

You spell it pr_warning() in the adjacent logging statements.

Ben.

>  	if ((log_mtts_per_seg < 1) || (log_mtts_per_seg > 7)) {
>  		pr_warning("mlx4_core: bad log_mtts_per_seg: %d\n", log_mtts_per_seg);
>  		return -1;
> diff --git a/include/linux/mlx4/device.h b/include/linux/mlx4/device.h
> index c0468e6..ca38871 100644
> --- a/include/linux/mlx4/device.h
> +++ b/include/linux/mlx4/device.h
> @@ -449,7 +449,6 @@ struct mlx4_caps {
>  	int                     reserved_qps_base[MLX4_NUM_QP_REGION];
>  	int                     log_num_macs;
>  	int                     log_num_vlans;
> -	int                     log_num_prios;
>  	enum mlx4_port_type	port_type[MLX4_MAX_PORTS + 1];
>  	u8			supported_type[MLX4_MAX_PORTS + 1];
>  	u8                      suggested_type[MLX4_MAX_PORTS + 1];

-- 
Ben Hutchings
Humans are not rational beings; they are rationalising beings.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: [PATCH net-next 1/2] net/mlx4_core: Deprecate use_prio module parameter

[Amir Vadai]

On 5/26/2014 10:22 PM, Ben Hutchings wrote:
> On Mon, 2014-05-19 at 11:37 +0300, Amir Vadai wrote:
>> It was used for steering by user priority for A0 steering. A0 mode is
>> not supported anymore. Printing a message and ignoring the parameter.
>>
>> CC: Carol Soto <clsoto@linux.vnet.ibm.com>
>> Signed-off-by: Amir Vadai <amirv@mellanox.com>
>> ---
>>   drivers/net/ethernet/mellanox/mlx4/main.c | 8 ++++----
>>   include/linux/mlx4/device.h               | 1 -
>>   2 files changed, 4 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/mellanox/mlx4/main.c b/drivers/net/ethernet/mellanox/mlx4/main.c
>> index a56f601..08ff5dd 100644
>> --- a/drivers/net/ethernet/mellanox/mlx4/main.c
>> +++ b/drivers/net/ethernet/mellanox/mlx4/main.c
>> @@ -132,8 +132,7 @@ MODULE_PARM_DESC(log_num_vlan, "Log2 max number of VLANs per ETH port (0-7)");
>>
>>   static bool use_prio;
>>   module_param_named(use_prio, use_prio, bool, 0444);
>
> Perhaps the sysfs permissions should be set to 0 (i.e. not visible in
> sysfs)?
>
Good idea - since the series already applied, will send another patch.

[...]

>> +	if (use_prio != 0)
>> +		pr_warn("mlx4_core: use_prio - obsolete module param, ignored\n");
>
> You spell it pr_warning() in the adjacent logging statements.
Patch 2/2 "net/mlx4_core: Replace pr_warning() with pr_warn()" in this 
patch series will take care to the other pr_warning's

Thanks,
Amir

>
> Ben.
>