diff mbox

net: ethernet: cpsw: fix usage of cpdma_check_free_tx_desc()

Message ID 1363105879-26921-1-git-send-email-zonque@gmail.com
State Accepted, archived
Delegated to: David Miller
Headers show

Commit Message

Daniel Mack March 12, 2013, 4:31 p.m. UTC
Commit fae50823d0 ("net: ethernet: davinci_cpdma: Add boundary for rx
and tx descriptors") introduced a function to check the current
allocation state of tx packets. The return value is taken into account
to stop the netqork queue on the adapter in case there are no free
slots.

However, cpdma_check_free_tx_desc() returns 'true' if there is room in
the bitmap, not 'false', so the usage of the function is wrong.

Signed-off-by: Daniel Mack <zonque@gmail.com>
Cc: Mugunthan V N <mugunthanvnm@ti.com>
Reported-by: Sven Neumann <s.neumann@raumfeld.com>
Reported-by: Andreas Fenkart <andreas.fenkart@streamunlimited.com>
---
 drivers/net/ethernet/ti/cpsw.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Mugunthan V N March 12, 2013, 7:47 p.m. UTC | #1
On 3/12/2013 10:01 PM, Daniel Mack wrote:
> Commit fae50823d0 ("net: ethernet: davinci_cpdma: Add boundary for rx
> and tx descriptors") introduced a function to check the current
> allocation state of tx packets. The return value is taken into account
> to stop the netqork queue on the adapter in case there are no free
> slots.
>
> However, cpdma_check_free_tx_desc() returns 'true' if there is room in
> the bitmap, not 'false', so the usage of the function is wrong.
>
> Signed-off-by: Daniel Mack <zonque@gmail.com>
> Cc: Mugunthan V N <mugunthanvnm@ti.com>
> Reported-by: Sven Neumann <s.neumann@raumfeld.com>
> Reported-by: Andreas Fenkart <andreas.fenkart@streamunlimited.com>
> ---
>   drivers/net/ethernet/ti/cpsw.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
> index 01ffbc4..75c4855 100644
> --- a/drivers/net/ethernet/ti/cpsw.c
> +++ b/drivers/net/ethernet/ti/cpsw.c
> @@ -905,7 +905,7 @@ static netdev_tx_t cpsw_ndo_start_xmit(struct sk_buff *skb,
>   	/* If there is no more tx desc left free then we need to
>   	 * tell the kernel to stop sending us tx frames.
>   	 */
> -	if (unlikely(cpdma_check_free_tx_desc(priv->txch)))
> +	if (unlikely(!cpdma_check_free_tx_desc(priv->txch)))
>   		netif_stop_queue(ndev);
>   
>   	return NETDEV_TX_OK;
Tested-by: Mugunthan V N <mugunthanvnm@ti.com>
Acked-by: Mugunthan V N <mugunthanvnm@ti.com>

Regards
Mugunthan V N
--
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
Andreas Fenkart March 12, 2013, 8:20 p.m. UTC | #2
On Wed, Mar 13, 2013 at 01:17:44AM +0530, Mugunthan V N wrote:
> On 3/12/2013 10:01 PM, Daniel Mack wrote:
> >Commit fae50823d0 ("net: ethernet: davinci_cpdma: Add boundary for rx
> >and tx descriptors") introduced a function to check the current
> >allocation state of tx packets. The return value is taken into account
> >to stop the netqork queue on the adapter in case there are no free
> >slots.
> >
> >However, cpdma_check_free_tx_desc() returns 'true' if there is room in
> >the bitmap, not 'false', so the usage of the function is wrong.
> >
> >Signed-off-by: Daniel Mack <zonque@gmail.com>
> >Cc: Mugunthan V N <mugunthanvnm@ti.com>
> >Reported-by: Sven Neumann <s.neumann@raumfeld.com>
> >Reported-by: Andreas Fenkart <andreas.fenkart@streamunlimited.com>
> >---
> >  drivers/net/ethernet/ti/cpsw.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> >diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
> >index 01ffbc4..75c4855 100644
> >--- a/drivers/net/ethernet/ti/cpsw.c
> >+++ b/drivers/net/ethernet/ti/cpsw.c
> >@@ -905,7 +905,7 @@ static netdev_tx_t cpsw_ndo_start_xmit(struct sk_buff *skb,
> >  	/* If there is no more tx desc left free then we need to
> >  	 * tell the kernel to stop sending us tx frames.
> >  	 */
> >-	if (unlikely(cpdma_check_free_tx_desc(priv->txch)))
> >+	if (unlikely(!cpdma_check_free_tx_desc(priv->txch)))
> >  		netif_stop_queue(ndev);
> >  	return NETDEV_TX_OK;
> Tested-by: Mugunthan V N <mugunthanvnm@ti.com>
> Acked-by: Mugunthan V N <mugunthanvnm@ti.com>
> 
> Regards
> Mugunthan V N

This restores throughput to the level of 3.7.0

Tested-by: Andreas Fenkart <andreas.fenkart@streamunlimited.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
David Miller March 13, 2013, 8:51 a.m. UTC | #3
From: Andreas Fenkart <andreas.fenkart@streamunlimited.com>
Date: Tue, 12 Mar 2013 21:20:27 +0100

> On Wed, Mar 13, 2013 at 01:17:44AM +0530, Mugunthan V N wrote:
>> On 3/12/2013 10:01 PM, Daniel Mack wrote:
>> >Commit fae50823d0 ("net: ethernet: davinci_cpdma: Add boundary for rx
>> >and tx descriptors") introduced a function to check the current
>> >allocation state of tx packets. The return value is taken into account
>> >to stop the netqork queue on the adapter in case there are no free
>> >slots.
>> >
>> >However, cpdma_check_free_tx_desc() returns 'true' if there is room in
>> >the bitmap, not 'false', so the usage of the function is wrong.
>> >
>> >Signed-off-by: Daniel Mack <zonque@gmail.com>
>> >Cc: Mugunthan V N <mugunthanvnm@ti.com>
>> >Reported-by: Sven Neumann <s.neumann@raumfeld.com>
>> >Reported-by: Andreas Fenkart <andreas.fenkart@streamunlimited.com>
...
>> Tested-by: Mugunthan V N <mugunthanvnm@ti.com>
>> Acked-by: Mugunthan V N <mugunthanvnm@ti.com>
 ...
> This restores throughput to the level of 3.7.0
> 
> Tested-by: Andreas Fenkart <andreas.fenkart@streamunlimited.com>

Applied.
--
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
diff mbox

Patch

diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index 01ffbc4..75c4855 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -905,7 +905,7 @@  static netdev_tx_t cpsw_ndo_start_xmit(struct sk_buff *skb,
 	/* If there is no more tx desc left free then we need to
 	 * tell the kernel to stop sending us tx frames.
 	 */
-	if (unlikely(cpdma_check_free_tx_desc(priv->txch)))
+	if (unlikely(!cpdma_check_free_tx_desc(priv->txch)))
 		netif_stop_queue(ndev);
 
 	return NETDEV_TX_OK;