[PATCH] net: mlxsw: avoid unused variable warnings

[PATCH] net: mlxsw: avoid unused variable warnings

[Arnd Bergmann]

dev_dbg_ratelimited() is a macro that ignores its arguments when DEBUG is
not set, which can lead to unused variable warnings:

ethernet/mellanox/mlxsw/pci.c: In function 'mlxsw_pci_cqe_sdq_handle':
ethernet/mellanox/mlxsw/pci.c:646:18: warning: unused variable 'pdev' [-Wunused-variable]
ethernet/mellanox/mlxsw/pci.c: In function 'mlxsw_pci_cqe_rdq_handle':
ethernet/mellanox/mlxsw/pci.c:671:18: warning: unused variable 'pdev' [-Wunused-variable]

This changes the mlxsw driver to remove the local variables we get
warnings for and instead pass the device directly into the API.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/net/ethernet/mellanox/mlxsw/pci.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/pci.c b/drivers/net/ethernet/mellanox/mlxsw/pci.c
index 7f4173c8eda3..fdcce03b028d 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/pci.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/pci.c
@@ -643,7 +643,6 @@ static void mlxsw_pci_cqe_sdq_handle(struct mlxsw_pci *mlxsw_pci,
 				     u16 consumer_counter_limit,
 				     char *cqe)
 {
-	struct pci_dev *pdev = mlxsw_pci->pdev;
 	struct mlxsw_pci_queue_elem_info *elem_info;
 	char *wqe;
 	struct sk_buff *skb;
@@ -659,7 +658,7 @@ static void mlxsw_pci_cqe_sdq_handle(struct mlxsw_pci *mlxsw_pci,
 	elem_info->u.sdq.skb = NULL;
 
 	if (q->consumer_counter++ != consumer_counter_limit)
-		dev_dbg_ratelimited(&pdev->dev, "Consumer counter does not match limit in SDQ\n");
+		dev_dbg_ratelimited(&mlxsw_pci->pdev->dev, "Consumer counter does not match limit in SDQ\n");
 	spin_unlock(&q->lock);
 }
 
@@ -668,7 +667,6 @@ static void mlxsw_pci_cqe_rdq_handle(struct mlxsw_pci *mlxsw_pci,
 				     u16 consumer_counter_limit,
 				     char *cqe)
 {
-	struct pci_dev *pdev = mlxsw_pci->pdev;
 	struct mlxsw_pci_queue_elem_info *elem_info;
 	char *wqe;
 	struct sk_buff *skb;
@@ -684,7 +682,7 @@ static void mlxsw_pci_cqe_rdq_handle(struct mlxsw_pci *mlxsw_pci,
 	mlxsw_pci_wqe_frag_unmap(mlxsw_pci, wqe, 0, DMA_FROM_DEVICE);
 
 	if (q->consumer_counter++ != consumer_counter_limit)
-		dev_dbg_ratelimited(&pdev->dev, "Consumer counter does not match limit in RDQ\n");
+		dev_dbg_ratelimited(&mlxsw_pci->pdev->dev, "Consumer counter does not match limit in RDQ\n");
 
 	if (mlxsw_pci_cqe_lag_get(cqe)) {
 		rx_info.is_lag = true;
@@ -706,7 +704,7 @@ static void mlxsw_pci_cqe_rdq_handle(struct mlxsw_pci *mlxsw_pci,
 	memset(wqe, 0, q->elem_size);
 	err = mlxsw_pci_rdq_skb_alloc(mlxsw_pci, elem_info);
 	if (err)
-		dev_dbg_ratelimited(&pdev->dev, "Failed to alloc skb for RDQ\n");
+		dev_dbg_ratelimited(&mlxsw_pci->pdev->dev, "Failed to alloc skb for RDQ\n");
 	/* Everything is set up, ring doorbell to pass elem to HW */
 	q->producer_counter++;
 	mlxsw_pci_queue_doorbell_producer_ring(mlxsw_pci, q);
-- 
2.7.0

Re: [PATCH] net: mlxsw: avoid unused variable warnings

[Andrew Lunn]

On Wed, Mar 23, 2016 at 05:37:38PM +0100, Arnd Bergmann wrote:
> dev_dbg_ratelimited() is a macro that ignores its arguments when DEBUG is
> not set, which can lead to unused variable warnings:
> 
> ethernet/mellanox/mlxsw/pci.c: In function 'mlxsw_pci_cqe_sdq_handle':
> ethernet/mellanox/mlxsw/pci.c:646:18: warning: unused variable 'pdev' [-Wunused-variable]
> ethernet/mellanox/mlxsw/pci.c: In function 'mlxsw_pci_cqe_rdq_handle':
> ethernet/mellanox/mlxsw/pci.c:671:18: warning: unused variable 'pdev' [-Wunused-variable]
> 
> This changes the mlxsw driver to remove the local variables we get
> warnings for and instead pass the device directly into the API.

Hi Arnd

Would it not be better to fix the macro?

I think the issue is that dev_dbg_ratelimited calls no_printk(),
without making use of dev. So how about:

#define dev_dbg_ratelimited(dev, fmt, ...)                              \
({                                                                      \
        if (0)                                                          \
                dev_printk(KERN_DEBUG, dev, fmt, ##__VA_ARGS__);        \
})

This follows the pattern for other macros for when DEBUG is not defined.

      Andrew

Re: [PATCH] net: mlxsw: avoid unused variable warnings

[David Miller]

From: Andrew Lunn <andrew@lunn.ch>
Date: Wed, 23 Mar 2016 17:51:11 +0100

> On Wed, Mar 23, 2016 at 05:37:38PM +0100, Arnd Bergmann wrote:
>> dev_dbg_ratelimited() is a macro that ignores its arguments when DEBUG is
>> not set, which can lead to unused variable warnings:
>> 
>> ethernet/mellanox/mlxsw/pci.c: In function 'mlxsw_pci_cqe_sdq_handle':
>> ethernet/mellanox/mlxsw/pci.c:646:18: warning: unused variable 'pdev' [-Wunused-variable]
>> ethernet/mellanox/mlxsw/pci.c: In function 'mlxsw_pci_cqe_rdq_handle':
>> ethernet/mellanox/mlxsw/pci.c:671:18: warning: unused variable 'pdev' [-Wunused-variable]
>> 
>> This changes the mlxsw driver to remove the local variables we get
>> warnings for and instead pass the device directly into the API.
> 
> Hi Arnd
> 
> Would it not be better to fix the macro?
> 
> I think the issue is that dev_dbg_ratelimited calls no_printk(),
> without making use of dev. So how about:
> 
> #define dev_dbg_ratelimited(dev, fmt, ...)                              \
> ({                                                                      \
>         if (0)                                                          \
>                 dev_printk(KERN_DEBUG, dev, fmt, ##__VA_ARGS__);        \
> })
> 
> This follows the pattern for other macros for when DEBUG is not defined.

Yeah, this is probably a better way to fix this problem.

Re: [PATCH] net: mlxsw: avoid unused variable warnings

[Arnd Bergmann]

On Wednesday 23 March 2016 14:38:05 David Miller wrote:
> From: Andrew Lunn <andrew@lunn.ch>
> Date: Wed, 23 Mar 2016 17:51:11 +0100
> 
> > On Wed, Mar 23, 2016 at 05:37:38PM +0100, Arnd Bergmann wrote:
> >> dev_dbg_ratelimited() is a macro that ignores its arguments when DEBUG is
> >> not set, which can lead to unused variable warnings:
> >> 
> >> ethernet/mellanox/mlxsw/pci.c: In function 'mlxsw_pci_cqe_sdq_handle':
> >> ethernet/mellanox/mlxsw/pci.c:646:18: warning: unused variable 'pdev' [-Wunused-variable]
> >> ethernet/mellanox/mlxsw/pci.c: In function 'mlxsw_pci_cqe_rdq_handle':
> >> ethernet/mellanox/mlxsw/pci.c:671:18: warning: unused variable 'pdev' [-Wunused-variable]
> >> 
> >> This changes the mlxsw driver to remove the local variables we get
> >> warnings for and instead pass the device directly into the API.
> > 
> > Hi Arnd
> > 
> > Would it not be better to fix the macro?
> > 
> > I think the issue is that dev_dbg_ratelimited calls no_printk(),
> > without making use of dev. So how about:
> > 
> > #define dev_dbg_ratelimited(dev, fmt, ...)                              \
> > ({                                                                      \
> >         if (0)                                                          \
> >                 dev_printk(KERN_DEBUG, dev, fmt, ##__VA_ARGS__);        \
> > })
> > 
> > This follows the pattern for other macros for when DEBUG is not defined.
> 
> Yeah, this is probably a better way to fix this problem.

Makes sense. I was thrown off by how a related patch recently
modified the no_printk() definition in Fixes: fe22cd9b7c98 ("printk:
help pr_debug and pr_devel to optimize out arguments") around the
same time I ran into the problem in the mlxsw driver.

I'll test build the patch below for a while and submit that if it doesn't
cause any other problems.

diff --git a/include/linux/device.h b/include/linux/device.h
index 002c59728dbe..07f74c246cac 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -1293,8 +1293,11 @@ do {									\
 		dev_printk(KERN_DEBUG, dev, fmt, ##__VA_ARGS__);	\
 } while (0)
 #else
-#define dev_dbg_ratelimited(dev, fmt, ...)			\
-	no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
+#define dev_dbg_ratelimited(dev, fmt, ...)				\
+do {									\
+	if (0)								\
+		dev_printk(KERN_DEBUG, dev, fmt, ##__VA_ARGS__);	\
+} while (0)
 #endif
 
 #ifdef VERBOSE_DEBUG

Thanks,

	Arnd