Patchwork RDMA/nes: Fix mis-merge

login
register
mail settings
Submitter Roland Dreier
Date March 26, 2009, 11:25 p.m.
Message ID <adahc1fuarm.fsf_-_@cisco.com>
Download mbox | patch
Permalink /patch/25183/
State Accepted
Delegated to: David Miller
Headers show

Comments

Roland Dreier - March 26, 2009, 11:25 p.m.
When net-next and infiniband were merged upstream, each branch deleted
one of a pair of adjacent lines from nes_nic.c, but when Linus fixed the
conflict up, he brought back both of the lines.  Fix up to the intended
final tree state.
---
 > Well, I merged infiniband, and that caused more conflicts. I fixed them 
 > up, but people need to double-check my fixes.

Looks like you mismerged one file -- each side deleted one line right
next to each other, but your merge left both lines in.  I think the
patch below is needed.  (The cxgb3 fix looks correct)

 drivers/infiniband/hw/nes/nes_nic.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

--
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
David Miller - March 26, 2009, 11:31 p.m.
From: Roland Dreier <rdreier@cisco.com>
Date: Thu, 26 Mar 2009 16:25:49 -0700

> When net-next and infiniband were merged upstream, each branch deleted
> one of a pair of adjacent lines from nes_nic.c, but when Linus fixed the
> conflict up, he brought back both of the lines.  Fix up to the intended
> final tree state.
> ---
>  > Well, I merged infiniband, and that caused more conflicts. I fixed them 
>  > up, but people need to double-check my fixes.
> 
> Looks like you mismerged one file -- each side deleted one line right
> next to each other, but your merge left both lines in.  I think the
> patch below is needed.  (The cxgb3 fix looks correct)
> 
>  drivers/infiniband/hw/nes/nes_nic.c |    2 --
>  1 files changed, 0 insertions(+), 2 deletions(-)

Looks good, Linus please apply:

Acked-by: David S. Miller <davem@davemloft.net>

> diff --git a/drivers/infiniband/hw/nes/nes_nic.c b/drivers/infiniband/hw/nes/nes_nic.c
> index 8d3e4c6..ecb1f6f 100644
> --- a/drivers/infiniband/hw/nes/nes_nic.c
> +++ b/drivers/infiniband/hw/nes/nes_nic.c
> @@ -1602,8 +1602,6 @@ struct net_device *nes_netdev_init(struct nes_device *nesdev,
>  	netif_napi_add(netdev, &nesvnic->napi, nes_netdev_poll, 128);
>  	nes_debug(NES_DBG_INIT, "Enabling VLAN Insert/Delete.\n");
>  	netdev->features |= NETIF_F_HW_VLAN_TX | NETIF_F_HW_VLAN_RX;
> -	netdev->vlan_rx_register = nes_netdev_vlan_rx_register;
> -	netdev->features |= NETIF_F_LLTX;
>  
>  	/* Fill in the port structure */
>  	nesvnic->netdev = netdev;
--
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
Linus Torvalds - March 26, 2009, 11:38 p.m.
On Thu, 26 Mar 2009, David Miller wrote:
> 
> Acked-by: David S. Miller <davem@davemloft.net>

Can I get a sign-off from Roland too?

		Linus
--
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
Linus Torvalds - March 26, 2009, 11:41 p.m.
On Thu, 26 Mar 2009, Linus Torvalds wrote:

> 
> 
> On Thu, 26 Mar 2009, David Miller wrote:
> > 
> > Acked-by: David S. Miller <davem@davemloft.net>
> 
> Can I get a sign-off from Roland too?

Btw, as far as I can tell, that function had never been tested with any 
polling. It had a line like

	netif_napi_add(netdev, &nesvnic->napi, nes_netdev_poll, 128);

but that line was _before_ 'nesvnic' was actually initialized (well, it 
had been initialized to NULL, so it passed in some NULL-pointer-offset to 
that function).

I moved that initialization up in the merge, but I obviously didn't test 
anything. So I'd suggest somebody actually test this case, because it 
clearly must have  gotten no testing at all.

			Linus
--
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
Roland Dreier - March 26, 2009, 11:48 p.m.
> Can I get a sign-off from Roland too?

Duh, not my day...

Signed-off-by: Roland Dreier <rolandd@cisco.com>
--
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
Roland Dreier - March 26, 2009, 11:53 p.m.
> Btw, as far as I can tell, that function had never been tested with any 
 > polling. It had a line like
 > 
 > 	netif_napi_add(netdev, &nesvnic->napi, nes_netdev_poll, 128);
 > 
 > but that line was _before_ 'nesvnic' was actually initialized (well, it 
 > had been initialized to NULL, so it passed in some NULL-pointer-offset to 
 > that function).

Yes, looks like the bug was introduced by d0929553 ("infiniband: convert
nes driver to net_device_ops").  And unfortunately the nesvnic=NULL
initialization stopped gcc from warning about used-uninitialized I
guess.  I'll test the final merge out...

 - R.
--
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
David Miller - March 27, 2009, 7:29 a.m.
From: Roland Dreier <rdreier@cisco.com>
Date: Thu, 26 Mar 2009 16:53:25 -0700

>  > Btw, as far as I can tell, that function had never been tested with any 
>  > polling. It had a line like
>  > 
>  > 	netif_napi_add(netdev, &nesvnic->napi, nes_netdev_poll, 128);
>  > 
>  > but that line was _before_ 'nesvnic' was actually initialized (well, it 
>  > had been initialized to NULL, so it passed in some NULL-pointer-offset to 
>  > that function).
> 
> Yes, looks like the bug was introduced by d0929553 ("infiniband: convert
> nes driver to net_device_ops").  And unfortunately the nesvnic=NULL
> initialization stopped gcc from warning about used-uninitialized I
> guess.  I'll test the final merge out...

Sorry about that, thanks for working this out Roland.
--
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

Patch

diff --git a/drivers/infiniband/hw/nes/nes_nic.c b/drivers/infiniband/hw/nes/nes_nic.c
index 8d3e4c6..ecb1f6f 100644
--- a/drivers/infiniband/hw/nes/nes_nic.c
+++ b/drivers/infiniband/hw/nes/nes_nic.c
@@ -1602,8 +1602,6 @@  struct net_device *nes_netdev_init(struct nes_device *nesdev,
 	netif_napi_add(netdev, &nesvnic->napi, nes_netdev_poll, 128);
 	nes_debug(NES_DBG_INIT, "Enabling VLAN Insert/Delete.\n");
 	netdev->features |= NETIF_F_HW_VLAN_TX | NETIF_F_HW_VLAN_RX;
-	netdev->vlan_rx_register = nes_netdev_vlan_rx_register;
-	netdev->features |= NETIF_F_LLTX;
 
 	/* Fill in the port structure */
 	nesvnic->netdev = netdev;