* [PATCH] net, mellanox mlx4 Fix compile warnings
@ 2013-09-13 12:30 Prarit Bhargava
2013-09-14 19:10 ` Or Gerlitz
2013-09-17 1:25 ` David Miller
0 siblings, 2 replies; 12+ messages in thread
From: Prarit Bhargava @ 2013-09-13 12:30 UTC (permalink / raw
To: netdev; +Cc: Prarit Bhargava, dledford, Amir Vadai, Or Gerlitz
Fix unitialized variable warnings.
drivers/net/ethernet/mellanox/mlx4/resource_tracker.c: In function ‘mlx4_HW2SW_CQ_wrapper’:
drivers/net/ethernet/mellanox/mlx4/resource_tracker.c:2551:16: error: ‘cq’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
atomic_dec(&cq->mtt->ref_count);
^
drivers/net/ethernet/mellanox/mlx4/resource_tracker.c: In function ‘mlx4_HW2SW_SRQ_wrapper’:
drivers/net/ethernet/mellanox/mlx4/resource_tracker.c:2734:17: error: ‘srq’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
atomic_dec(&srq->mtt->ref_count);
Signed-off-by: Prarit Bhargava <prarit@redhat.com>
Cc: dledford@redhat.com
Cc: Amir Vadai <amirv@mellanox.com>
Cc: Or Gerlitz <ogerlitz@mellanox.com>
---
.../net/ethernet/mellanox/mlx4/resource_tracker.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c b/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c
index dd68763..d703838 100644
--- a/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c
+++ b/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c
@@ -2563,7 +2563,7 @@ int mlx4_HW2SW_CQ_wrapper(struct mlx4_dev *dev, int slave,
{
int err;
int cqn = vhcr->in_modifier;
- struct res_cq *cq;
+ struct res_cq *uninitialized_var(cq);
err = cq_res_start_move_to(dev, slave, cqn, RES_CQ_ALLOCATED, &cq);
if (err)
@@ -2746,7 +2746,7 @@ int mlx4_HW2SW_SRQ_wrapper(struct mlx4_dev *dev, int slave,
{
int err;
int srqn = vhcr->in_modifier;
- struct res_srq *srq;
+ struct res_srq *uninitialized_var(srq);
err = srq_res_start_move_to(dev, slave, srqn, RES_SRQ_ALLOCATED, &srq);
if (err)
--
1.7.9.3
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] net, mellanox mlx4 Fix compile warnings
2013-09-13 12:30 [PATCH] net, mellanox mlx4 Fix compile warnings Prarit Bhargava
@ 2013-09-14 19:10 ` Or Gerlitz
2013-09-15 6:14 ` Jack Morgenstein
2013-09-17 1:25 ` David Miller
1 sibling, 1 reply; 12+ messages in thread
From: Or Gerlitz @ 2013-09-14 19:10 UTC (permalink / raw
To: Prarit Bhargava, Jack Morgenstein
Cc: netdev@vger.kernel.org, Doug Ledford, Amir Vadai, Or Gerlitz
On Fri, Sep 13, 2013 at 3:30 PM, Prarit Bhargava <prarit@redhat.com> wrote:
> Fix unitialized variable warnings.
Hi,
I'd like Jack, who is in charge on this code to take a look, added him.
Or.
>
> drivers/net/ethernet/mellanox/mlx4/resource_tracker.c: In function ‘mlx4_HW2SW_CQ_wrapper’:
> drivers/net/ethernet/mellanox/mlx4/resource_tracker.c:2551:16: error: ‘cq’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
> atomic_dec(&cq->mtt->ref_count);
> ^
> drivers/net/ethernet/mellanox/mlx4/resource_tracker.c: In function ‘mlx4_HW2SW_SRQ_wrapper’:
> drivers/net/ethernet/mellanox/mlx4/resource_tracker.c:2734:17: error: ‘srq’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
> atomic_dec(&srq->mtt->ref_count);
>
> Signed-off-by: Prarit Bhargava <prarit@redhat.com>
> Cc: dledford@redhat.com
> Cc: Amir Vadai <amirv@mellanox.com>
> Cc: Or Gerlitz <ogerlitz@mellanox.com>
> ---
> .../net/ethernet/mellanox/mlx4/resource_tracker.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c b/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c
> index dd68763..d703838 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c
> @@ -2563,7 +2563,7 @@ int mlx4_HW2SW_CQ_wrapper(struct mlx4_dev *dev, int slave,
> {
> int err;
> int cqn = vhcr->in_modifier;
> - struct res_cq *cq;
> + struct res_cq *uninitialized_var(cq);
>
> err = cq_res_start_move_to(dev, slave, cqn, RES_CQ_ALLOCATED, &cq);
> if (err)
> @@ -2746,7 +2746,7 @@ int mlx4_HW2SW_SRQ_wrapper(struct mlx4_dev *dev, int slave,
> {
> int err;
> int srqn = vhcr->in_modifier;
> - struct res_srq *srq;
> + struct res_srq *uninitialized_var(srq);
>
> err = srq_res_start_move_to(dev, slave, srqn, RES_SRQ_ALLOCATED, &srq);
> if (err)
> --
> 1.7.9.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] net, mellanox mlx4 Fix compile warnings
2013-09-14 19:10 ` Or Gerlitz
@ 2013-09-15 6:14 ` Jack Morgenstein
2013-09-15 6:22 ` ZHAO Gang
2013-09-16 8:59 ` David Laight
0 siblings, 2 replies; 12+ messages in thread
From: Jack Morgenstein @ 2013-09-15 6:14 UTC (permalink / raw
To: Or Gerlitz
Cc: Prarit Bhargava, netdev@vger.kernel.org, Doug Ledford, Amir Vadai,
Or Gerlitz
On Sat, 14 Sep 2013 22:10:19 +0300
Or Gerlitz <or.gerlitz@gmail.com> wrote:
> > + struct res_cq *uninitialized_var(cq);
> >
> > err = cq_res_start_move_to(dev, slave, cqn,
> > RES_CQ_ALLOCATED, &cq);
I have no objection. However, I don't know if the compiler is being too
clever here or too stupid. The cq variable is initialized in
cq_res_start_move_to(), but the compiler is ignoring this -- or maybe
it is simply not assuming that cq will in fact be
initialized by the called procedure?
In any event, this change cannot hurt.
-Jack
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] net, mellanox mlx4 Fix compile warnings
2013-09-15 6:14 ` Jack Morgenstein
@ 2013-09-15 6:22 ` ZHAO Gang
2013-09-15 6:27 ` ZHAO Gang
2013-09-16 8:59 ` David Laight
1 sibling, 1 reply; 12+ messages in thread
From: ZHAO Gang @ 2013-09-15 6:22 UTC (permalink / raw
To: gamerh2o
Cc: Or Gerlitz, Prarit Bhargava, netdev@vger.kernel.org, Doug Ledford,
Amir Vadai, Or Gerlitz
hell what!
On Sun, Sep 15, 2013 at 09:14:00AM +0300, Jack Morgenstein wrote:
> On Sat, 14 Sep 2013 22:10:19 +0300
> Or Gerlitz <or.gerlitz@gmail.com> wrote:
>
> > > + struct res_cq *uninitialized_var(cq);
> > >
> > > err = cq_res_start_move_to(dev, slave, cqn,
> > > RES_CQ_ALLOCATED, &cq);
>
> I have no objection. However, I don't know if the compiler is being too
> clever here or too stupid. The cq variable is initialized in
> cq_res_start_move_to(), but the compiler is ignoring this -- or maybe
> it is simply not assuming that cq will in fact be
> initialized by the called procedure?
>
> In any event, this change cannot hurt.
>
> -Jack
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] net, mellanox mlx4 Fix compile warnings
2013-09-15 6:22 ` ZHAO Gang
@ 2013-09-15 6:27 ` ZHAO Gang
0 siblings, 0 replies; 12+ messages in thread
From: ZHAO Gang @ 2013-09-15 6:27 UTC (permalink / raw
To: ZHAO Gang
Cc: Or Gerlitz, Prarit Bhargava, netdev@vger.kernel.org, Doug Ledford,
Amir Vadai, Or Gerlitz
sorry for the inconvenince, i'm just practise the use of mutt, sorry again.
On Sun, Sep 15, 2013 at 2:22 PM, ZHAO Gang <gamerh2o@gmail.com> wrote:
> hell what!
>
> On Sun, Sep 15, 2013 at 09:14:00AM +0300, Jack Morgenstein wrote:
>> On Sat, 14 Sep 2013 22:10:19 +0300
>> Or Gerlitz <or.gerlitz@gmail.com> wrote:
>>
>> > > + struct res_cq *uninitialized_var(cq);
>> > >
>> > > err = cq_res_start_move_to(dev, slave, cqn,
>> > > RES_CQ_ALLOCATED, &cq);
>>
>> I have no objection. However, I don't know if the compiler is being too
>> clever here or too stupid. The cq variable is initialized in
>> cq_res_start_move_to(), but the compiler is ignoring this -- or maybe
>> it is simply not assuming that cq will in fact be
>> initialized by the called procedure?
>>
>> In any event, this change cannot hurt.
>>
>> -Jack
>> --
>> To unsubscribe from this list: send the line "unsubscribe netdev" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [PATCH] net, mellanox mlx4 Fix compile warnings
2013-09-15 6:14 ` Jack Morgenstein
2013-09-15 6:22 ` ZHAO Gang
@ 2013-09-16 8:59 ` David Laight
2013-09-16 12:14 ` Prarit Bhargava
2013-09-16 13:52 ` Jack Morgenstein
1 sibling, 2 replies; 12+ messages in thread
From: David Laight @ 2013-09-16 8:59 UTC (permalink / raw
To: Jack Morgenstein, Or Gerlitz
Cc: Prarit Bhargava, netdev, Doug Ledford, Amir Vadai, Or Gerlitz
> On Sat, 14 Sep 2013 22:10:19 +0300
> Or Gerlitz <or.gerlitz@gmail.com> wrote:
>
> > > + struct res_cq *uninitialized_var(cq);
> > >
> > > err = cq_res_start_move_to(dev, slave, cqn,
> > > RES_CQ_ALLOCATED, &cq);
>
> I have no objection. However, I don't know if the compiler is being too
> clever here or too stupid. The cq variable is initialized in
> cq_res_start_move_to(), but the compiler is ignoring this -- or maybe
> it is simply not assuming that cq will in fact be
> initialized by the called procedure?
Possibly cq_res_start_move_to() got inlined - and the analysis
of the inlined code found a path where it wasn't initialised.
Most likely one of the paths where the return value is non-zero.
> In any event, this change cannot hurt.
It could hide a real 'used but not initialised' error later on...
David
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] net, mellanox mlx4 Fix compile warnings
2013-09-16 8:59 ` David Laight
@ 2013-09-16 12:14 ` Prarit Bhargava
2013-09-16 13:52 ` Jack Morgenstein
1 sibling, 0 replies; 12+ messages in thread
From: Prarit Bhargava @ 2013-09-16 12:14 UTC (permalink / raw
To: David Laight
Cc: Jack Morgenstein, Or Gerlitz, netdev, Doug Ledford, Amir Vadai,
Or Gerlitz
On 09/16/2013 04:59 AM, David Laight wrote:
>> On Sat, 14 Sep 2013 22:10:19 +0300
>> Or Gerlitz <or.gerlitz@gmail.com> wrote:
>>
>>>> + struct res_cq *uninitialized_var(cq);
>>>>
>>>> err = cq_res_start_move_to(dev, slave, cqn,
>>>> RES_CQ_ALLOCATED, &cq);
>>
>> I have no objection. However, I don't know if the compiler is being too
>> clever here or too stupid. The cq variable is initialized in
>> cq_res_start_move_to(), but the compiler is ignoring this -- or maybe
>> it is simply not assuming that cq will in fact be
>> initialized by the called procedure?
>
> Possibly cq_res_start_move_to() got inlined - and the analysis
> of the inlined code found a path where it wasn't initialised.
>
> Most likely one of the paths where the return value is non-zero.
>
>> In any event, this change cannot hurt.
>
> It could hide a real 'used but not initialised' error later on...
Hi David,
I certainly don't profess to know the code better than you do of course, but in
terms of risk we're no better in than the original code here, and it seems that
other places in the kernel the resolution has been to either init to NULL or
wrap with uninitialized_var()...
P.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] net, mellanox mlx4 Fix compile warnings
2013-09-16 8:59 ` David Laight
2013-09-16 12:14 ` Prarit Bhargava
@ 2013-09-16 13:52 ` Jack Morgenstein
1 sibling, 0 replies; 12+ messages in thread
From: Jack Morgenstein @ 2013-09-16 13:52 UTC (permalink / raw
To: David Laight
Cc: Or Gerlitz, Prarit Bhargava, netdev, Doug Ledford, Amir Vadai,
Or Gerlitz
On Mon, 16 Sep 2013 09:59:41 +0100
"David Laight" <David.Laight@ACULAB.COM> wrote:
> > In any event, this change cannot hurt.
>
> It could hide a real 'used but not initialised' error later on...
>
> David
In this case, it doesn't. If cq_res_start_move_to returns 0, the "cq"
pointer points to the desired cq object. In all instances where the cq
object is not found, you get an error return, and the cq pointer in the
caller is never de-referenced.
Therefore, the compiler warning is a false positive, and the
"uninitialized_var" is just a workaround to satisfy the compiler.
Same for the SRQ case.
-Jack
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] net, mellanox mlx4 Fix compile warnings
2013-09-13 12:30 [PATCH] net, mellanox mlx4 Fix compile warnings Prarit Bhargava
2013-09-14 19:10 ` Or Gerlitz
@ 2013-09-17 1:25 ` David Miller
2013-09-17 19:13 ` [PATCH 2/2] " Prarit Bhargava
1 sibling, 1 reply; 12+ messages in thread
From: David Miller @ 2013-09-17 1:25 UTC (permalink / raw
To: prarit; +Cc: netdev, dledford, amirv, ogerlitz
From: Prarit Bhargava <prarit@redhat.com>
Date: Fri, 13 Sep 2013 08:30:38 -0400
> @@ -2563,7 +2563,7 @@ int mlx4_HW2SW_CQ_wrapper(struct mlx4_dev *dev, int slave,
> {
> int err;
> int cqn = vhcr->in_modifier;
> - struct res_cq *cq;
> + struct res_cq *uninitialized_var(cq);
>
> err = cq_res_start_move_to(dev, slave, cqn, RES_CQ_ALLOCATED, &cq);
> if (err)
This gross annotation is not necessary.
Make cq_res_start_move_to() return 'cq' as an error pointer instead of
setting 'cq' by reference.
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 2/2] Re: net, mellanox mlx4 Fix compile warnings
2013-09-17 1:25 ` David Miller
@ 2013-09-17 19:13 ` Prarit Bhargava
2013-09-17 19:25 ` Or Gerlitz
0 siblings, 1 reply; 12+ messages in thread
From: Prarit Bhargava @ 2013-09-17 19:13 UTC (permalink / raw
To: netdev; +Cc: Prarit Bhargava, dledford, amirv, davem, ogerlitz
Fix unitialized variable warnings.
drivers/net/ethernet/mellanox/mlx4/resource_tracker.c: In function ‘mlx4_HW2SW_CQ_wrapper’:
drivers/net/ethernet/mellanox/mlx4/resource_tracker.c:2551:16: error: ‘cq’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
atomic_dec(&cq->mtt->ref_count);
^
drivers/net/ethernet/mellanox/mlx4/resource_tracker.c: In function ‘mlx4_HW2SW_SRQ_wrapper’:
drivers/net/ethernet/mellanox/mlx4/resource_tracker.c:2734:17: error: ‘srq’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
atomic_dec(&srq->mtt->ref_count);
[v2]: davem suggested making cq_res_start_move_to() return 'cq' as an error
pointer instead of setting 'cq' by reference. I also did the same for
srq.
Signed-off-by: Prarit Bhargava <prarit@redhat.com>
Cc: dledford@redhat.com
Cc: amirv@mellanox.com
Cc: davem@davemloft.net
Cc: ogerlitz@mellanox.com
---
.../net/ethernet/mellanox/mlx4/resource_tracker.c | 46 ++++++++++----------
1 file changed, 24 insertions(+), 22 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c b/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c
index dd68763..343206b 100644
--- a/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c
+++ b/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c
@@ -1073,8 +1073,9 @@ static int eq_res_start_move_to(struct mlx4_dev *dev, int slave, int index,
return err;
}
-static int cq_res_start_move_to(struct mlx4_dev *dev, int slave, int cqn,
- enum res_cq_states state, struct res_cq **cq)
+static struct res_cq *cq_res_start_move_to(struct mlx4_dev *dev,
+ int slave, int cqn,
+ enum res_cq_states state)
{
struct mlx4_priv *priv = mlx4_priv(dev);
struct mlx4_resource_tracker *tracker = &priv->mfunc.master.res_tracker;
@@ -1117,18 +1118,19 @@ static int cq_res_start_move_to(struct mlx4_dev *dev, int slave, int cqn,
r->com.from_state = r->com.state;
r->com.to_state = state;
r->com.state = RES_CQ_BUSY;
- if (cq)
- *cq = r;
+ } else {
+ r = ERR_PTR(err);
}
}
spin_unlock_irq(mlx4_tlock(dev));
- return err;
+ return r;
}
-static int srq_res_start_move_to(struct mlx4_dev *dev, int slave, int index,
- enum res_cq_states state, struct res_srq **srq)
+static struct res_srq *srq_res_start_move_to(struct mlx4_dev *dev, int slave,
+ int index,
+ enum res_cq_states state)
{
struct mlx4_priv *priv = mlx4_priv(dev);
struct mlx4_resource_tracker *tracker = &priv->mfunc.master.res_tracker;
@@ -1167,14 +1169,14 @@ static int srq_res_start_move_to(struct mlx4_dev *dev, int slave, int index,
r->com.from_state = r->com.state;
r->com.to_state = state;
r->com.state = RES_SRQ_BUSY;
- if (srq)
- *srq = r;
+ } else {
+ r = ERR_PTR(err);
}
}
spin_unlock_irq(mlx4_tlock(dev));
- return err;
+ return r;
}
static void res_abort_move(struct mlx4_dev *dev, int slave,
@@ -2530,9 +2532,9 @@ int mlx4_SW2HW_CQ_wrapper(struct mlx4_dev *dev, int slave,
struct res_cq *cq;
struct res_mtt *mtt;
- err = cq_res_start_move_to(dev, slave, cqn, RES_CQ_HW, &cq);
- if (err)
- return err;
+ cq = cq_res_start_move_to(dev, slave, cqn, RES_CQ_HW);
+ if (IS_ERR(cq))
+ return PTR_ERR(cq);
err = get_res(dev, slave, mtt_base, RES_MTT, &mtt);
if (err)
goto out_move;
@@ -2565,9 +2567,9 @@ int mlx4_HW2SW_CQ_wrapper(struct mlx4_dev *dev, int slave,
int cqn = vhcr->in_modifier;
struct res_cq *cq;
- err = cq_res_start_move_to(dev, slave, cqn, RES_CQ_ALLOCATED, &cq);
- if (err)
- return err;
+ cq = cq_res_start_move_to(dev, slave, cqn, RES_CQ_ALLOCATED);
+ if (IS_ERR(cq))
+ return PTR_ERR(cq);
err = mlx4_DMA_wrapper(dev, slave, vhcr, inbox, outbox, cmd);
if (err)
goto out_move;
@@ -2709,9 +2711,9 @@ int mlx4_SW2HW_SRQ_wrapper(struct mlx4_dev *dev, int slave,
if (srqn != (be32_to_cpu(srqc->state_logsize_srqn) & 0xffffff))
return -EINVAL;
- err = srq_res_start_move_to(dev, slave, srqn, RES_SRQ_HW, &srq);
- if (err)
- return err;
+ srq = srq_res_start_move_to(dev, slave, srqn, RES_SRQ_ALLOCATED);
+ if (IS_ERR(srq))
+ return PTR_ERR(srq);
err = get_res(dev, slave, mtt_base, RES_MTT, &mtt);
if (err)
goto ex_abort;
@@ -2748,9 +2750,9 @@ int mlx4_HW2SW_SRQ_wrapper(struct mlx4_dev *dev, int slave,
int srqn = vhcr->in_modifier;
struct res_srq *srq;
- err = srq_res_start_move_to(dev, slave, srqn, RES_SRQ_ALLOCATED, &srq);
- if (err)
- return err;
+ srq = srq_res_start_move_to(dev, slave, srqn, RES_SRQ_ALLOCATED);
+ if (IS_ERR(srq))
+ return PTR_ERR(srq);
err = mlx4_DMA_wrapper(dev, slave, vhcr, inbox, outbox, cmd);
if (err)
goto ex_abort;
--
1.7.9.3
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] Re: net, mellanox mlx4 Fix compile warnings
2013-09-17 19:13 ` [PATCH 2/2] " Prarit Bhargava
@ 2013-09-17 19:25 ` Or Gerlitz
2013-09-17 19:28 ` David Miller
0 siblings, 1 reply; 12+ messages in thread
From: Or Gerlitz @ 2013-09-17 19:25 UTC (permalink / raw
To: Prarit Bhargava, Jack Morgenstein
Cc: netdev@vger.kernel.org, Doug Ledford, Amir Vadai, David Miller,
Or Gerlitz
On Tue, Sep 17, 2013 at 10:13 PM, Prarit Bhargava <prarit@redhat.com> wrote:
> Fix unitialized variable warnings.
>
> drivers/net/ethernet/mellanox/mlx4/resource_tracker.c: In function ‘mlx4_HW2SW_CQ_wrapper’:
> drivers/net/ethernet/mellanox/mlx4/resource_tracker.c:2551:16: error: ‘cq’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
> atomic_dec(&cq->mtt->ref_count);
> ^
> drivers/net/ethernet/mellanox/mlx4/resource_tracker.c: In function ‘mlx4_HW2SW_SRQ_wrapper’:
> drivers/net/ethernet/mellanox/mlx4/resource_tracker.c:2734:17: error: ‘srq’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
> atomic_dec(&srq->mtt->ref_count);
>
> [v2]: davem suggested making cq_res_start_move_to() return 'cq' as an error
> pointer instead of setting 'cq' by reference. I also did the same for srq.
Pravit, as I wrote you earlier on this thread, Jack from our team
maintains this piece (SRIOV resource tracker) of the mlx4 core driver
code, so I am adding him. Dave, as many of us here he might be OOO for
the coming ten days for holiday vacation, so would ask you to please
wait patiently for his ack/nak...
Or.
> Signed-off-by: Prarit Bhargava <prarit@redhat.com>
> Cc: dledford@redhat.com
> Cc: amirv@mellanox.com
> Cc: davem@davemloft.net
> Cc: ogerlitz@mellanox.com
> ---
> .../net/ethernet/mellanox/mlx4/resource_tracker.c | 46 ++++++++++----------
> 1 file changed, 24 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c b/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c
> index dd68763..343206b 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c
> @@ -1073,8 +1073,9 @@ static int eq_res_start_move_to(struct mlx4_dev *dev, int slave, int index,
> return err;
> }
>
> -static int cq_res_start_move_to(struct mlx4_dev *dev, int slave, int cqn,
> - enum res_cq_states state, struct res_cq **cq)
> +static struct res_cq *cq_res_start_move_to(struct mlx4_dev *dev,
> + int slave, int cqn,
> + enum res_cq_states state)
> {
> struct mlx4_priv *priv = mlx4_priv(dev);
> struct mlx4_resource_tracker *tracker = &priv->mfunc.master.res_tracker;
> @@ -1117,18 +1118,19 @@ static int cq_res_start_move_to(struct mlx4_dev *dev, int slave, int cqn,
> r->com.from_state = r->com.state;
> r->com.to_state = state;
> r->com.state = RES_CQ_BUSY;
> - if (cq)
> - *cq = r;
> + } else {
> + r = ERR_PTR(err);
> }
> }
>
> spin_unlock_irq(mlx4_tlock(dev));
>
> - return err;
> + return r;
> }
>
> -static int srq_res_start_move_to(struct mlx4_dev *dev, int slave, int index,
> - enum res_cq_states state, struct res_srq **srq)
> +static struct res_srq *srq_res_start_move_to(struct mlx4_dev *dev, int slave,
> + int index,
> + enum res_cq_states state)
> {
> struct mlx4_priv *priv = mlx4_priv(dev);
> struct mlx4_resource_tracker *tracker = &priv->mfunc.master.res_tracker;
> @@ -1167,14 +1169,14 @@ static int srq_res_start_move_to(struct mlx4_dev *dev, int slave, int index,
> r->com.from_state = r->com.state;
> r->com.to_state = state;
> r->com.state = RES_SRQ_BUSY;
> - if (srq)
> - *srq = r;
> + } else {
> + r = ERR_PTR(err);
> }
> }
>
> spin_unlock_irq(mlx4_tlock(dev));
>
> - return err;
> + return r;
> }
>
> static void res_abort_move(struct mlx4_dev *dev, int slave,
> @@ -2530,9 +2532,9 @@ int mlx4_SW2HW_CQ_wrapper(struct mlx4_dev *dev, int slave,
> struct res_cq *cq;
> struct res_mtt *mtt;
>
> - err = cq_res_start_move_to(dev, slave, cqn, RES_CQ_HW, &cq);
> - if (err)
> - return err;
> + cq = cq_res_start_move_to(dev, slave, cqn, RES_CQ_HW);
> + if (IS_ERR(cq))
> + return PTR_ERR(cq);
> err = get_res(dev, slave, mtt_base, RES_MTT, &mtt);
> if (err)
> goto out_move;
> @@ -2565,9 +2567,9 @@ int mlx4_HW2SW_CQ_wrapper(struct mlx4_dev *dev, int slave,
> int cqn = vhcr->in_modifier;
> struct res_cq *cq;
>
> - err = cq_res_start_move_to(dev, slave, cqn, RES_CQ_ALLOCATED, &cq);
> - if (err)
> - return err;
> + cq = cq_res_start_move_to(dev, slave, cqn, RES_CQ_ALLOCATED);
> + if (IS_ERR(cq))
> + return PTR_ERR(cq);
> err = mlx4_DMA_wrapper(dev, slave, vhcr, inbox, outbox, cmd);
> if (err)
> goto out_move;
> @@ -2709,9 +2711,9 @@ int mlx4_SW2HW_SRQ_wrapper(struct mlx4_dev *dev, int slave,
> if (srqn != (be32_to_cpu(srqc->state_logsize_srqn) & 0xffffff))
> return -EINVAL;
>
> - err = srq_res_start_move_to(dev, slave, srqn, RES_SRQ_HW, &srq);
> - if (err)
> - return err;
> + srq = srq_res_start_move_to(dev, slave, srqn, RES_SRQ_ALLOCATED);
> + if (IS_ERR(srq))
> + return PTR_ERR(srq);
> err = get_res(dev, slave, mtt_base, RES_MTT, &mtt);
> if (err)
> goto ex_abort;
> @@ -2748,9 +2750,9 @@ int mlx4_HW2SW_SRQ_wrapper(struct mlx4_dev *dev, int slave,
> int srqn = vhcr->in_modifier;
> struct res_srq *srq;
>
> - err = srq_res_start_move_to(dev, slave, srqn, RES_SRQ_ALLOCATED, &srq);
> - if (err)
> - return err;
> + srq = srq_res_start_move_to(dev, slave, srqn, RES_SRQ_ALLOCATED);
> + if (IS_ERR(srq))
> + return PTR_ERR(srq);
> err = mlx4_DMA_wrapper(dev, slave, vhcr, inbox, outbox, cmd);
> if (err)
> goto ex_abort;
> --
> 1.7.9.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] Re: net, mellanox mlx4 Fix compile warnings
2013-09-17 19:25 ` Or Gerlitz
@ 2013-09-17 19:28 ` David Miller
0 siblings, 0 replies; 12+ messages in thread
From: David Miller @ 2013-09-17 19:28 UTC (permalink / raw
To: or.gerlitz; +Cc: prarit, jackm, netdev, dledford, amirv, ogerlitz
From: Or Gerlitz <or.gerlitz@gmail.com>
Date: Tue, 17 Sep 2013 22:25:57 +0300
> On Tue, Sep 17, 2013 at 10:13 PM, Prarit Bhargava <prarit@redhat.com> wrote:
>> Fix unitialized variable warnings.
>>
>> drivers/net/ethernet/mellanox/mlx4/resource_tracker.c: In function ‘mlx4_HW2SW_CQ_wrapper’:
>> drivers/net/ethernet/mellanox/mlx4/resource_tracker.c:2551:16: error: ‘cq’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
>> atomic_dec(&cq->mtt->ref_count);
>> ^
>> drivers/net/ethernet/mellanox/mlx4/resource_tracker.c: In function ‘mlx4_HW2SW_SRQ_wrapper’:
>> drivers/net/ethernet/mellanox/mlx4/resource_tracker.c:2734:17: error: ‘srq’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
>> atomic_dec(&srq->mtt->ref_count);
>>
>> [v2]: davem suggested making cq_res_start_move_to() return 'cq' as an error
>> pointer instead of setting 'cq' by reference. I also did the same for srq.
>
> Pravit, as I wrote you earlier on this thread, Jack from our team
> maintains this piece (SRIOV resource tracker) of the mlx4 core driver
> code, so I am adding him. Dave, as many of us here he might be OOO for
> the coming ten days for holiday vacation, so would ask you to please
> wait patiently for his ack/nak...
I already stated that I wanted this issue fixed differently.
Specifically I said that these functions should return error pointers,
instead of trying to return an integer error whilst setting the cq
pointer by reference.
This is a method which is idiomatic and common across the kernel, and
is the preferred way to handle this kind of situation.
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2013-09-17 19:28 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-09-13 12:30 [PATCH] net, mellanox mlx4 Fix compile warnings Prarit Bhargava
2013-09-14 19:10 ` Or Gerlitz
2013-09-15 6:14 ` Jack Morgenstein
2013-09-15 6:22 ` ZHAO Gang
2013-09-15 6:27 ` ZHAO Gang
2013-09-16 8:59 ` David Laight
2013-09-16 12:14 ` Prarit Bhargava
2013-09-16 13:52 ` Jack Morgenstein
2013-09-17 1:25 ` David Miller
2013-09-17 19:13 ` [PATCH 2/2] " Prarit Bhargava
2013-09-17 19:25 ` Or Gerlitz
2013-09-17 19:28 ` David Miller
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.