[PATCH net-next 0/2] net: stmmac: multiple queue fixes

[PATCH net-next 0/2] net: stmmac: multiple queue fixes

[Joao Pinto]

This patch set contains two fixes for problems I detected when ran the driver
in single queue mode.

Please test in your setups to check if you get better results.

Joao Pinto (2):
  net: stmmac: fix netdev release
  net: stmmac: fix number of tx queues in stmmac_poll

 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

-- 
2.9.3

[PATCH net-next 1/2] net: stmmac: fix netdev release

[Joao Pinto]

This patch fixes the kernel crash problem that happens when the driver
exits. The dma resouces must be freed after netdev resouces get freed and
in driver removed, multiple napi's should be deleted before removing
netdev resouces or it will hang.

Signed-off-by: Joao Pinto <jpinto@synopsys.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 4b418d2..3827952 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -2659,9 +2659,6 @@ static int stmmac_release(struct net_device *dev)
 	/* Stop TX/RX DMA and clear the descriptors */
 	stmmac_stop_all_dma(priv);
 
-	/* Release and free the Rx/Tx resources */
-	free_dma_desc_resources(priv);
-
 	/* Disable the MAC Rx/Tx */
 	stmmac_set_mac(priv->ioaddr, false);
 
@@ -4080,7 +4077,7 @@ int stmmac_dvr_probe(struct device *device,
 	/* Init MAC and get the capabilities */
 	ret = stmmac_hw_init(priv);
 	if (ret)
-		goto error_hw_init;
+		return ret;
 
 	/* Configure real RX and TX queues */
 	ndev->real_num_rx_queues = priv->plat->rx_queues_to_use;
@@ -4207,9 +4204,8 @@ int stmmac_dvr_probe(struct device *device,
 		netif_napi_del(&rx_q->napi);
 	}
 init_dma_error:
-	free_dma_desc_resources(priv);
-error_hw_init:
 	free_netdev(ndev);
+	free_dma_desc_resources(priv);
 
 	return ret;
 }
@@ -4225,6 +4221,7 @@ int stmmac_dvr_remove(struct device *dev)
 {
 	struct net_device *ndev = dev_get_drvdata(dev);
 	struct stmmac_priv *priv = netdev_priv(ndev);
+	u32 queue = 0;
 
 	netdev_info(priv->dev, "%s: removing driver", __func__);
 
@@ -4241,7 +4238,15 @@ int stmmac_dvr_remove(struct device *dev)
 	    priv->hw->pcs != STMMAC_PCS_TBI &&
 	    priv->hw->pcs != STMMAC_PCS_RTBI)
 		stmmac_mdio_unregister(ndev);
+
+	for (queue = 0; queue < priv->plat->rx_queues_to_use; queue++) {
+		struct stmmac_rx_queue *rx_q = &priv->rx_queue[queue];
+
+		netif_napi_del(&rx_q->napi);
+	}
+
 	free_netdev(ndev);
+	free_dma_desc_resources(priv);
 
 	return 0;
 }
-- 
2.9.3

[PATCH net-next 2/2] net: stmmac: fix number of tx queues in stmmac_poll

[Joao Pinto]

For cores that have more than 1 TX queue configured, the kernel would crash,
since only one TX queue is permitted by default.

Signed-off-by: Joao Pinto <jpinto@synopsys.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 3827952..1eab084 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -3429,7 +3429,7 @@ static int stmmac_poll(struct napi_struct *napi, int budget)
 	struct stmmac_rx_queue *rx_q =
 		container_of(napi, struct stmmac_rx_queue, napi);
 	struct stmmac_priv *priv = rx_q->priv_data;
-	u32 tx_count = priv->dma_cap.number_tx_queues;
+	u32 tx_count = priv->plat->tx_queues_to_use;
 	u32 chan = rx_q->queue_index;
 	u32 work_done = 0;
 	u32 queue = 0;
-- 
2.9.3

Re: [PATCH net-next 2/2] net: stmmac: fix number of tx queues in stmmac_poll

[Corentin Labbe]

On Fri, Mar 24, 2017 at 05:16:45PM +0000, Joao Pinto wrote:
> For cores that have more than 1 TX queue configured, the kernel would crash,
> since only one TX queue is permitted by default.
> 
> Signed-off-by: Joao Pinto <jpinto@synopsys.com>
> ---
>  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 3827952..1eab084 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -3429,7 +3429,7 @@ static int stmmac_poll(struct napi_struct *napi, int budget)
>  	struct stmmac_rx_queue *rx_q =
>  		container_of(napi, struct stmmac_rx_queue, napi);
>  	struct stmmac_priv *priv = rx_q->priv_data;
> -	u32 tx_count = priv->dma_cap.number_tx_queues;
> +	u32 tx_count = priv->plat->tx_queues_to_use;
>  	u32 chan = rx_q->queue_index;
>  	u32 work_done = 0;
>  	u32 queue = 0;
> -- 
> 2.9.3
> 

This patch fix the performance issue on dwmac-sun8i only.
The dwmac-sunxi is still broken.

Re: [PATCH net-next 2/2] net: stmmac: fix number of tx queues in stmmac_poll

[Joao Pinto]

Às 7:26 AM de 3/25/2017, Corentin Labbe escreveu:
> On Fri, Mar 24, 2017 at 05:16:45PM +0000, Joao Pinto wrote:
>> For cores that have more than 1 TX queue configured, the kernel would crash,
>> since only one TX queue is permitted by default.
>>
>> Signed-off-by: Joao Pinto <jpinto@synopsys.com>
>> ---
>>  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>> index 3827952..1eab084 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>> @@ -3429,7 +3429,7 @@ static int stmmac_poll(struct napi_struct *napi, int budget)
>>  	struct stmmac_rx_queue *rx_q =
>>  		container_of(napi, struct stmmac_rx_queue, napi);
>>  	struct stmmac_priv *priv = rx_q->priv_data;
>> -	u32 tx_count = priv->dma_cap.number_tx_queues;
>> +	u32 tx_count = priv->plat->tx_queues_to_use;
>>  	u32 chan = rx_q->queue_index;
>>  	u32 work_done = 0;
>>  	u32 queue = 0;
>> -- 
>> 2.9.3
>>
> 
> This patch fix the performance issue on dwmac-sun8i only.

Ok, great!

> The dwmac-sunxi is still broken.
> 

What is the difference between the setups?

Thanks,
Joao

Re: [PATCH net-next 2/2] net: stmmac: fix number of tx queues in stmmac_poll

[Corentin Labbe]

On Mon, Mar 27, 2017 at 10:04:57AM +0100, Joao Pinto wrote:
> Às 7:26 AM de 3/25/2017, Corentin Labbe escreveu:
> > On Fri, Mar 24, 2017 at 05:16:45PM +0000, Joao Pinto wrote:
> >> For cores that have more than 1 TX queue configured, the kernel would crash,
> >> since only one TX queue is permitted by default.
> >>
> >> Signed-off-by: Joao Pinto <jpinto@synopsys.com>
> >> ---
> >>  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> >> index 3827952..1eab084 100644
> >> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> >> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> >> @@ -3429,7 +3429,7 @@ static int stmmac_poll(struct napi_struct *napi, int budget)
> >>  	struct stmmac_rx_queue *rx_q =
> >>  		container_of(napi, struct stmmac_rx_queue, napi);
> >>  	struct stmmac_priv *priv = rx_q->priv_data;
> >> -	u32 tx_count = priv->dma_cap.number_tx_queues;
> >> +	u32 tx_count = priv->plat->tx_queues_to_use;
> >>  	u32 chan = rx_q->queue_index;
> >>  	u32 work_done = 0;
> >>  	u32 queue = 0;
> >> -- 
> >> 2.9.3
> >>
> > 
> > This patch fix the performance issue on dwmac-sun8i only.
> 
> Ok, great!
> 

Sorry, in the we, I see that my gigabit dwmac-sun8i didnt work... (but didnt have time to find why).

> > The dwmac-sunxi is still broken.
> > 
> 
> What is the difference between the setups?

dwmac-sun8i use chain mode, dwmac-sunxi use ring mode

Re: [PATCH net-next 2/2] net: stmmac: fix number of tx queues in stmmac_poll

[Joao Pinto]

Às 10:09 AM de 3/27/2017, Corentin Labbe escreveu:
> On Mon, Mar 27, 2017 at 10:04:57AM +0100, Joao Pinto wrote:
>> Às 7:26 AM de 3/25/2017, Corentin Labbe escreveu:
>>> On Fri, Mar 24, 2017 at 05:16:45PM +0000, Joao Pinto wrote:
>>>> For cores that have more than 1 TX queue configured, the kernel would crash,
>>>> since only one TX queue is permitted by default.
>>>>
>>>> Signed-off-by: Joao Pinto <jpinto@synopsys.com>
>>>> ---
>>>>  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 +-
>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>>>> index 3827952..1eab084 100644
>>>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>>>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>>>> @@ -3429,7 +3429,7 @@ static int stmmac_poll(struct napi_struct *napi, int budget)
>>>>  	struct stmmac_rx_queue *rx_q =
>>>>  		container_of(napi, struct stmmac_rx_queue, napi);
>>>>  	struct stmmac_priv *priv = rx_q->priv_data;
>>>> -	u32 tx_count = priv->dma_cap.number_tx_queues;
>>>> +	u32 tx_count = priv->plat->tx_queues_to_use;
>>>>  	u32 chan = rx_q->queue_index;
>>>>  	u32 work_done = 0;
>>>>  	u32 queue = 0;
>>>> -- 
>>>> 2.9.3
>>>>
>>>
>>> This patch fix the performance issue on dwmac-sun8i only.
>>
>> Ok, great!
>>
> 
> Sorry, in the we, I see that my gigabit dwmac-sun8i didnt work... (but didnt have time to find why).
> 
>>> The dwmac-sunxi is still broken.

Ok, let's see what others report.

>>>
>>
>> What is the difference between the setups?
> 
> dwmac-sun8i use chain mode, dwmac-sunxi use ring mode
> 

@Thierry and Niklas: Hi! Your setup is working well with these 2 fix patches?

Thnaks.

Re: [PATCH net-next 2/2] net: stmmac: fix number of tx queues in stmmac_poll

[Niklas Cassel]

On 03/27/2017 11:12 AM, Joao Pinto wrote:
> Às 10:09 AM de 3/27/2017, Corentin Labbe escreveu:
>> On Mon, Mar 27, 2017 at 10:04:57AM +0100, Joao Pinto wrote:
>>> Às 7:26 AM de 3/25/2017, Corentin Labbe escreveu:
>>>> On Fri, Mar 24, 2017 at 05:16:45PM +0000, Joao Pinto wrote:
>>>>> For cores that have more than 1 TX queue configured, the kernel would crash,
>>>>> since only one TX queue is permitted by default.
>>>>>
>>>>> Signed-off-by: Joao Pinto <jpinto@synopsys.com>
>>>>> ---
>>>>>  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 +-
>>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>>>>> index 3827952..1eab084 100644
>>>>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>>>>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>>>>> @@ -3429,7 +3429,7 @@ static int stmmac_poll(struct napi_struct *napi, int budget)
>>>>>  	struct stmmac_rx_queue *rx_q =
>>>>>  		container_of(napi, struct stmmac_rx_queue, napi);
>>>>>  	struct stmmac_priv *priv = rx_q->priv_data;
>>>>> -	u32 tx_count = priv->dma_cap.number_tx_queues;
>>>>> +	u32 tx_count = priv->plat->tx_queues_to_use;
>>>>>  	u32 chan = rx_q->queue_index;
>>>>>  	u32 work_done = 0;
>>>>>  	u32 queue = 0;
>>>>> -- 
>>>>> 2.9.3
>>>>>
>>>>
>>>> This patch fix the performance issue on dwmac-sun8i only.
>>>
>>> Ok, great!
>>>
>>
>> Sorry, in the we, I see that my gigabit dwmac-sun8i didnt work... (but didnt have time to find why).
>>
>>>> The dwmac-sunxi is still broken.
> 
> Ok, let's see what others report.
> 
>>>>
>>>
>>> What is the difference between the setups?
>>
>> dwmac-sun8i use chain mode, dwmac-sunxi use ring mode
>>
> 
> @Thierry and Niklas: Hi! Your setup is working well with these 2 fix patches?
> 

Hello Joao,

I have not tested these 2 patches.

Previously, when I tested next-20170321, artpec-6 SoC got TX queue timeouts.
When testing today's linux-next, next-20170327, things are working again :)


So I guess one of the following patches fixed our problem:

270c7759fbbc net: stmmac: add set_mac to the stmmac_ops
b4f0a6615556 net: stmmac: fix dma operation mode config for older versions
33e85b8dd69e net: stmmac: Restore DT backwards-compatibility
f39768744fd6 net: stmmac: Always enable MAC RX queues
abe80fdc6ee6 net: stmmac: RX queue routing configuration
a8f5102af2a7 net: stmmac: TX and RX queue priority configuration
aff3d9eff843 net: stmmac: enable multiple buffers

Regards,
Niklas

Re: [PATCH net-next 2/2] net: stmmac: fix number of tx queues in stmmac_poll

[Joao Pinto]

Às 2:28 PM de 3/27/2017, Niklas Cassel escreveu:
> 
> 
> On 03/27/2017 11:12 AM, Joao Pinto wrote:
>> Às 10:09 AM de 3/27/2017, Corentin Labbe escreveu:
>>> On Mon, Mar 27, 2017 at 10:04:57AM +0100, Joao Pinto wrote:
>>>> Às 7:26 AM de 3/25/2017, Corentin Labbe escreveu:
>>>>> On Fri, Mar 24, 2017 at 05:16:45PM +0000, Joao Pinto wrote:
>>>>>> For cores that have more than 1 TX queue configured, the kernel would crash,
>>>>>> since only one TX queue is permitted by default.
>>>>>>
>>>>>> Signed-off-by: Joao Pinto <jpinto@synopsys.com>
>>>>>> ---
>>>>>>  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 +-
>>>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>>>>>> index 3827952..1eab084 100644
>>>>>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>>>>>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>>>>>> @@ -3429,7 +3429,7 @@ static int stmmac_poll(struct napi_struct *napi, int budget)
>>>>>>  	struct stmmac_rx_queue *rx_q =
>>>>>>  		container_of(napi, struct stmmac_rx_queue, napi);
>>>>>>  	struct stmmac_priv *priv = rx_q->priv_data;
>>>>>> -	u32 tx_count = priv->dma_cap.number_tx_queues;
>>>>>> +	u32 tx_count = priv->plat->tx_queues_to_use;
>>>>>>  	u32 chan = rx_q->queue_index;
>>>>>>  	u32 work_done = 0;
>>>>>>  	u32 queue = 0;
>>>>>> -- 
>>>>>> 2.9.3
>>>>>>
>>>>>
>>>>> This patch fix the performance issue on dwmac-sun8i only.
>>>>
>>>> Ok, great!
>>>>
>>>
>>> Sorry, in the we, I see that my gigabit dwmac-sun8i didnt work... (but didnt have time to find why).
>>>
>>>>> The dwmac-sunxi is still broken.
>>
>> Ok, let's see what others report.
>>
>>>>>
>>>>
>>>> What is the difference between the setups?
>>>
>>> dwmac-sun8i use chain mode, dwmac-sunxi use ring mode
>>>
>>
>> @Thierry and Niklas: Hi! Your setup is working well with these 2 fix patches?
>>
> 
> Hello Joao,
> 
> I have not tested these 2 patches.
> 
> Previously, when I tested next-20170321, artpec-6 SoC got TX queue timeouts.
> When testing today's linux-next, next-20170327, things are working again :)
> 
> 
> So I guess one of the following patches fixed our problem:
> 
> 270c7759fbbc net: stmmac: add set_mac to the stmmac_ops
> b4f0a6615556 net: stmmac: fix dma operation mode config for older versions
> 33e85b8dd69e net: stmmac: Restore DT backwards-compatibility
> f39768744fd6 net: stmmac: Always enable MAC RX queues
> abe80fdc6ee6 net: stmmac: RX queue routing configuration
> a8f5102af2a7 net: stmmac: TX and RX queue priority configuration
> aff3d9eff843 net: stmmac: enable multiple buffers

Yes for sure :) Great! Thanks.

> 
> Regards,
> Niklas
> 

Re: [PATCH net-next 2/2] net: stmmac: fix number of tx queues in stmmac_poll

[Joao Pinto]

Hin Thierry,

Às 2:34 PM de 3/27/2017, Joao Pinto escreveu:
> Às 2:28 PM de 3/27/2017, Niklas Cassel escreveu:
>>
>>
>> On 03/27/2017 11:12 AM, Joao Pinto wrote:
>>> Às 10:09 AM de 3/27/2017, Corentin Labbe escreveu:
>>>> On Mon, Mar 27, 2017 at 10:04:57AM +0100, Joao Pinto wrote:
>>>>> Às 7:26 AM de 3/25/2017, Corentin Labbe escreveu:
>>>>>> On Fri, Mar 24, 2017 at 05:16:45PM +0000, Joao Pinto wrote:
>>>>>>> For cores that have more than 1 TX queue configured, the kernel would crash,
>>>>>>> since only one TX queue is permitted by default.
>>>>>>>
>>>>>>> Signed-off-by: Joao Pinto <jpinto@synopsys.com>
>>>>>>> ---
>>>>>>>  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 +-
>>>>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>>
>>>>>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>>>>>>> index 3827952..1eab084 100644
>>>>>>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>>>>>>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>>>>>>> @@ -3429,7 +3429,7 @@ static int stmmac_poll(struct napi_struct *napi, int budget)
>>>>>>>  	struct stmmac_rx_queue *rx_q =
>>>>>>>  		container_of(napi, struct stmmac_rx_queue, napi);
>>>>>>>  	struct stmmac_priv *priv = rx_q->priv_data;
>>>>>>> -	u32 tx_count = priv->dma_cap.number_tx_queues;
>>>>>>> +	u32 tx_count = priv->plat->tx_queues_to_use;
>>>>>>>  	u32 chan = rx_q->queue_index;
>>>>>>>  	u32 work_done = 0;
>>>>>>>  	u32 queue = 0;
>>>>>>> -- 
>>>>>>> 2.9.3
>>>>>>>
>>>>>>
>>>>>> This patch fix the performance issue on dwmac-sun8i only.
>>>>>
>>>>> Ok, great!
>>>>>
>>>>
>>>> Sorry, in the we, I see that my gigabit dwmac-sun8i didnt work... (but didnt have time to find why).
>>>>
>>>>>> The dwmac-sunxi is still broken.
>>>
>>> Ok, let's see what others report.
>>>
>>>>>>
>>>>>
>>>>> What is the difference between the setups?
>>>>
>>>> dwmac-sun8i use chain mode, dwmac-sunxi use ring mode
>>>>
>>>
>>> @Thierry and Niklas: Hi! Your setup is working well with these 2 fix patches?
>>>
>>
>> Hello Joao,
>>
>> I have not tested these 2 patches.
>>
>> Previously, when I tested next-20170321, artpec-6 SoC got TX queue timeouts.
>> When testing today's linux-next, next-20170327, things are working again :)
>>
>>
>> So I guess one of the following patches fixed our problem:
>>
>> 270c7759fbbc net: stmmac: add set_mac to the stmmac_ops
>> b4f0a6615556 net: stmmac: fix dma operation mode config for older versions
>> 33e85b8dd69e net: stmmac: Restore DT backwards-compatibility
>> f39768744fd6 net: stmmac: Always enable MAC RX queues
>> abe80fdc6ee6 net: stmmac: RX queue routing configuration
>> a8f5102af2a7 net: stmmac: TX and RX queue priority configuration
>> aff3d9eff843 net: stmmac: enable multiple buffers
> 
> Yes for sure :) Great! Thanks.

Could you please confirm your setup works with these final 2 patches?

Thanks.

> 
>>
>> Regards,
>> Niklas
>>
> 

Re: [PATCH net-next 2/2] net: stmmac: fix number of tx queues in stmmac_poll

[Joao Pinto]

Hi David,

Às 7:26 AM de 3/25/2017, Corentin Labbe escreveu:
> On Fri, Mar 24, 2017 at 05:16:45PM +0000, Joao Pinto wrote:
>> For cores that have more than 1 TX queue configured, the kernel would crash,
>> since only one TX queue is permitted by default.
>>
>> Signed-off-by: Joao Pinto <jpinto@synopsys.com>
>> ---
>>  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>> index 3827952..1eab084 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>> @@ -3429,7 +3429,7 @@ static int stmmac_poll(struct napi_struct *napi, int budget)
>>  	struct stmmac_rx_queue *rx_q =
>>  		container_of(napi, struct stmmac_rx_queue, napi);
>>  	struct stmmac_priv *priv = rx_q->priv_data;
>> -	u32 tx_count = priv->dma_cap.number_tx_queues;
>> +	u32 tx_count = priv->plat->tx_queues_to_use;
>>  	u32 chan = rx_q->queue_index;
>>  	u32 work_done = 0;
>>  	u32 queue = 0;
>> -- 
>> 2.9.3
>>
> 
> This patch fix the performance issue on dwmac-sun8i only.
> The dwmac-sunxi is still broken.
> 

This patch series can be upstreamed please, since they make 2 fixes, one of them
solving the problem in dwmac-sun8i.

Thanks.

Re: [PATCH net-next 2/2] net: stmmac: fix number of tx queues in stmmac_poll

[Corentin Labbe]

On Mon, Mar 27, 2017 at 04:26:48PM +0100, Joao Pinto wrote:
> Hi David,
> 
> Às 7:26 AM de 3/25/2017, Corentin Labbe escreveu:
> > On Fri, Mar 24, 2017 at 05:16:45PM +0000, Joao Pinto wrote:
> >> For cores that have more than 1 TX queue configured, the kernel would crash,
> >> since only one TX queue is permitted by default.
> >>
> >> Signed-off-by: Joao Pinto <jpinto@synopsys.com>
> >> ---
> >>  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> >> index 3827952..1eab084 100644
> >> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> >> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> >> @@ -3429,7 +3429,7 @@ static int stmmac_poll(struct napi_struct *napi, int budget)
> >>  	struct stmmac_rx_queue *rx_q =
> >>  		container_of(napi, struct stmmac_rx_queue, napi);
> >>  	struct stmmac_priv *priv = rx_q->priv_data;
> >> -	u32 tx_count = priv->dma_cap.number_tx_queues;
> >> +	u32 tx_count = priv->plat->tx_queues_to_use;
> >>  	u32 chan = rx_q->queue_index;
> >>  	u32 work_done = 0;
> >>  	u32 queue = 0;
> >> -- 
> >> 2.9.3
> >>
> > 
> > This patch fix the performance issue on dwmac-sun8i only.
> > The dwmac-sunxi is still broken.
> > 
> 
> This patch series can be upstreamed please, since they make 2 fixes, one of them
> solving the problem in dwmac-sun8i.
> 
> Thanks.

As I said in a previous answer, finaly dwmac-sun8i is still broken.
Adding thoses 2 patch will just made the revert harder.

Regards

Re: [PATCH net-next 2/2] net: stmmac: fix number of tx queues in stmmac_poll

[Joao Pinto]

Às 6:00 PM de 3/27/2017, Corentin Labbe escreveu:
> On Mon, Mar 27, 2017 at 04:26:48PM +0100, Joao Pinto wrote:
>> Hi David,
>>
>> Às 7:26 AM de 3/25/2017, Corentin Labbe escreveu:
>>> On Fri, Mar 24, 2017 at 05:16:45PM +0000, Joao Pinto wrote:
>>>> For cores that have more than 1 TX queue configured, the kernel would crash,
>>>> since only one TX queue is permitted by default.
>>>>
>>>> Signed-off-by: Joao Pinto <jpinto@synopsys.com>
>>>> ---
>>>>  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 +-
>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>>>> index 3827952..1eab084 100644
>>>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>>>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>>>> @@ -3429,7 +3429,7 @@ static int stmmac_poll(struct napi_struct *napi, int budget)
>>>>  	struct stmmac_rx_queue *rx_q =
>>>>  		container_of(napi, struct stmmac_rx_queue, napi);
>>>>  	struct stmmac_priv *priv = rx_q->priv_data;
>>>> -	u32 tx_count = priv->dma_cap.number_tx_queues;
>>>> +	u32 tx_count = priv->plat->tx_queues_to_use;
>>>>  	u32 chan = rx_q->queue_index;
>>>>  	u32 work_done = 0;
>>>>  	u32 queue = 0;
>>>> -- 
>>>> 2.9.3
>>>>
>>>
>>> This patch fix the performance issue on dwmac-sun8i only.
>>> The dwmac-sunxi is still broken.
>>>
>>
>> This patch series can be upstreamed please, since they make 2 fixes, one of them
>> solving the problem in dwmac-sun8i.
>>
>> Thanks.
> 
> As I said in a previous answer, finaly dwmac-sun8i is still broken.
> Adding thoses 2 patch will just made the revert harder.

Oh, ok. You are using version >=4.00 or older versions?

> 
> Regards
> 

Re: [PATCH net-next 2/2] net: stmmac: fix number of tx queues in stmmac_poll

[David Miller]

From: Corentin Labbe <clabbe.montjoie@gmail.com>
Date: Mon, 27 Mar 2017 19:00:58 +0200

> On Mon, Mar 27, 2017 at 04:26:48PM +0100, Joao Pinto wrote:
>> Hi David,
>> 
>> Às 7:26 AM de 3/25/2017, Corentin Labbe escreveu:
>> > On Fri, Mar 24, 2017 at 05:16:45PM +0000, Joao Pinto wrote:
>> >> For cores that have more than 1 TX queue configured, the kernel would crash,
>> >> since only one TX queue is permitted by default.
>> >>
>> >> Signed-off-by: Joao Pinto <jpinto@synopsys.com>
>> >> ---
>> >>  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 +-
>> >>  1 file changed, 1 insertion(+), 1 deletion(-)
>> >>
>> >> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>> >> index 3827952..1eab084 100644
>> >> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>> >> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>> >> @@ -3429,7 +3429,7 @@ static int stmmac_poll(struct napi_struct *napi, int budget)
>> >>  	struct stmmac_rx_queue *rx_q =
>> >>  		container_of(napi, struct stmmac_rx_queue, napi);
>> >>  	struct stmmac_priv *priv = rx_q->priv_data;
>> >> -	u32 tx_count = priv->dma_cap.number_tx_queues;
>> >> +	u32 tx_count = priv->plat->tx_queues_to_use;
>> >>  	u32 chan = rx_q->queue_index;
>> >>  	u32 work_done = 0;
>> >>  	u32 queue = 0;
>> >> -- 
>> >> 2.9.3
>> >>
>> > 
>> > This patch fix the performance issue on dwmac-sun8i only.
>> > The dwmac-sunxi is still broken.
>> > 
>> 
>> This patch series can be upstreamed please, since they make 2 fixes, one of them
>> solving the problem in dwmac-sun8i.
>> 
>> Thanks.
> 
> As I said in a previous answer, finaly dwmac-sun8i is still broken.
> Adding thoses 2 patch will just made the revert harder.

I agree.

Re: [PATCH net-next 2/2] net: stmmac: fix number of tx queues in stmmac_poll

[Joao Pinto]

Às 6:28 PM de 3/27/2017, David Miller escreveu:
> From: Corentin Labbe <clabbe.montjoie@gmail.com>
> Date: Mon, 27 Mar 2017 19:00:58 +0200
> 
>> On Mon, Mar 27, 2017 at 04:26:48PM +0100, Joao Pinto wrote:
>>> Hi David,
>>>
>>> Às 7:26 AM de 3/25/2017, Corentin Labbe escreveu:
>>>> On Fri, Mar 24, 2017 at 05:16:45PM +0000, Joao Pinto wrote:
>>>>> For cores that have more than 1 TX queue configured, the kernel would crash,
>>>>> since only one TX queue is permitted by default.
>>>>>
>>>>> Signed-off-by: Joao Pinto <jpinto@synopsys.com>
>>>>> ---
>>>>>  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 +-
>>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>>>>> index 3827952..1eab084 100644
>>>>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>>>>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>>>>> @@ -3429,7 +3429,7 @@ static int stmmac_poll(struct napi_struct *napi, int budget)
>>>>>  	struct stmmac_rx_queue *rx_q =
>>>>>  		container_of(napi, struct stmmac_rx_queue, napi);
>>>>>  	struct stmmac_priv *priv = rx_q->priv_data;
>>>>> -	u32 tx_count = priv->dma_cap.number_tx_queues;
>>>>> +	u32 tx_count = priv->plat->tx_queues_to_use;
>>>>>  	u32 chan = rx_q->queue_index;
>>>>>  	u32 work_done = 0;
>>>>>  	u32 queue = 0;
>>>>> -- 
>>>>> 2.9.3
>>>>>
>>>>
>>>> This patch fix the performance issue on dwmac-sun8i only.
>>>> The dwmac-sunxi is still broken.
>>>>
>>>
>>> This patch series can be upstreamed please, since they make 2 fixes, one of them
>>> solving the problem in dwmac-sun8i.
>>>
>>> Thanks.
>>
>> As I said in a previous answer, finaly dwmac-sun8i is still broken.
>> Adding thoses 2 patch will just made the revert harder.
> 
> I agree.

For what I am understanding, SoCs base on Core versions >= 4.00 are working
properly and for some reason SoCs based on older versions are not working.

This fix is necessary, since if you have a diferent configured tx_queues_to_use
in the driver and priv->dma_cap.number_tx_queues in the core, this can lead to
kernel crashes.

The other fix (netdev resources release) is also necessary, since when you
release the driver its crashes, because the rx queue struct is freed before
releasing the netdevs.

We can revert, but I think it might not solve the issue. We can break the
"multiple buffers" patch into "rx multilple buffers" and "tx multiple buffers",
but will that actually work? We can give it a try, I don't mind making a new
multiple buffers patch broken into 2, that can be tested by new cores and older
cores.

Joao

Re: [PATCH net-next 2/2] net: stmmac: fix number of tx queues in stmmac_poll

[Corentin Labbe]

On Mon, Mar 27, 2017 at 06:06:05PM +0100, Joao Pinto wrote:
> Às 6:00 PM de 3/27/2017, Corentin Labbe escreveu:
> > On Mon, Mar 27, 2017 at 04:26:48PM +0100, Joao Pinto wrote:
> >> Hi David,
> >>
> >> Às 7:26 AM de 3/25/2017, Corentin Labbe escreveu:
> >>> On Fri, Mar 24, 2017 at 05:16:45PM +0000, Joao Pinto wrote:
> >>>> For cores that have more than 1 TX queue configured, the kernel would crash,
> >>>> since only one TX queue is permitted by default.
> >>>>
> >>>> Signed-off-by: Joao Pinto <jpinto@synopsys.com>
> >>>> ---
> >>>>  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 +-
> >>>>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> >>>> index 3827952..1eab084 100644
> >>>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> >>>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> >>>> @@ -3429,7 +3429,7 @@ static int stmmac_poll(struct napi_struct *napi, int budget)
> >>>>  	struct stmmac_rx_queue *rx_q =
> >>>>  		container_of(napi, struct stmmac_rx_queue, napi);
> >>>>  	struct stmmac_priv *priv = rx_q->priv_data;
> >>>> -	u32 tx_count = priv->dma_cap.number_tx_queues;
> >>>> +	u32 tx_count = priv->plat->tx_queues_to_use;
> >>>>  	u32 chan = rx_q->queue_index;
> >>>>  	u32 work_done = 0;
> >>>>  	u32 queue = 0;
> >>>> -- 
> >>>> 2.9.3
> >>>>
> >>>
> >>> This patch fix the performance issue on dwmac-sun8i only.
> >>> The dwmac-sunxi is still broken.
> >>>
> >>
> >> This patch series can be upstreamed please, since they make 2 fixes, one of them
> >> solving the problem in dwmac-sun8i.
> >>
> >> Thanks.
> > 
> > As I said in a previous answer, finaly dwmac-sun8i is still broken.
> > Adding thoses 2 patch will just made the revert harder.
> 
> Oh, ok. You are using version >=4.00 or older versions?
> 

older for both

Re: [PATCH net-next 2/2] net: stmmac: fix number of tx queues in stmmac_poll

[Corentin Labbe]

On Mon, Mar 27, 2017 at 06:44:22PM +0100, Joao Pinto wrote:
> Às 6:28 PM de 3/27/2017, David Miller escreveu:
> > From: Corentin Labbe <clabbe.montjoie@gmail.com>
> > Date: Mon, 27 Mar 2017 19:00:58 +0200
> > 
> >> On Mon, Mar 27, 2017 at 04:26:48PM +0100, Joao Pinto wrote:
> >>> Hi David,
> >>>
> >>> Às 7:26 AM de 3/25/2017, Corentin Labbe escreveu:
> >>>> On Fri, Mar 24, 2017 at 05:16:45PM +0000, Joao Pinto wrote:
> >>>>> For cores that have more than 1 TX queue configured, the kernel would crash,
> >>>>> since only one TX queue is permitted by default.
> >>>>>
> >>>>> Signed-off-by: Joao Pinto <jpinto@synopsys.com>
> >>>>> ---
> >>>>>  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 +-
> >>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>>>>
> >>>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> >>>>> index 3827952..1eab084 100644
> >>>>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> >>>>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> >>>>> @@ -3429,7 +3429,7 @@ static int stmmac_poll(struct napi_struct *napi, int budget)
> >>>>>  	struct stmmac_rx_queue *rx_q =
> >>>>>  		container_of(napi, struct stmmac_rx_queue, napi);
> >>>>>  	struct stmmac_priv *priv = rx_q->priv_data;
> >>>>> -	u32 tx_count = priv->dma_cap.number_tx_queues;
> >>>>> +	u32 tx_count = priv->plat->tx_queues_to_use;
> >>>>>  	u32 chan = rx_q->queue_index;
> >>>>>  	u32 work_done = 0;
> >>>>>  	u32 queue = 0;
> >>>>> -- 
> >>>>> 2.9.3
> >>>>>
> >>>>
> >>>> This patch fix the performance issue on dwmac-sun8i only.
> >>>> The dwmac-sunxi is still broken.
> >>>>
> >>>
> >>> This patch series can be upstreamed please, since they make 2 fixes, one of them
> >>> solving the problem in dwmac-sun8i.
> >>>
> >>> Thanks.
> >>
> >> As I said in a previous answer, finaly dwmac-sun8i is still broken.
> >> Adding thoses 2 patch will just made the revert harder.
> > 
> > I agree.
> 
> For what I am understanding, SoCs base on Core versions >= 4.00 are working
> properly and for some reason SoCs based on older versions are not working.
> 
> This fix is necessary, since if you have a diferent configured tx_queues_to_use
> in the driver and priv->dma_cap.number_tx_queues in the core, this can lead to
> kernel crashes.
> 
> The other fix (netdev resources release) is also necessary, since when you
> release the driver its crashes, because the rx queue struct is freed before
> releasing the netdevs.
> 
> We can revert, but I think it might not solve the issue. We can break the
> "multiple buffers" patch into "rx multilple buffers" and "tx multiple buffers",
> but will that actually work? We can give it a try, I don't mind making a new
> multiple buffers patch broken into 2, that can be tested by new cores and older
> cores.
> 

Reverting at least will bring back my archs to good status:)
Spliting will not solve magically the issue, but will permit to easily detect which part is faulty.
And I am sure that it is possible to split more than in 2.
The more small the patch will be, the easier it will.

Regards

Re: [PATCH net-next 2/2] net: stmmac: fix number of tx queues in stmmac_poll

[David Miller]

From: Joao Pinto <Joao.Pinto@synopsys.com>
Date: Mon, 27 Mar 2017 18:44:22 +0100

> For what I am understanding, SoCs base on Core versions >= 4.00 are working
> properly and for some reason SoCs based on older versions are not working.

Please send me a revert, and work offline with these people to resolve
all of the regressions you introduced.

Once you resolved all of the regressions, we can put the changes back
in.

Thank you.

Re: [PATCH net-next 0/2] net: stmmac: multiple queue fixes

[Niklas Cassel]

On 03/24/2017 06:16 PM, Joao Pinto wrote:
> This patch set contains two fixes for problems I detected when ran the driver
> in single queue mode.
> 
> Please test in your setups to check if you get better results.
> 
> Joao Pinto (2):
>   net: stmmac: fix netdev release
>   net: stmmac: fix number of tx queues in stmmac_poll
> 
>  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 18 +++++++++++-------
>  1 file changed, 11 insertions(+), 7 deletions(-)
> 

Hello Joao,

I think that it would be helpful if these commit messages were a
little more elaborate.
Both of these commit messages just say "fixes kernel crash".
Please include the relevant part of the stack trace so that others
will know if the bug you are fixing is the same as theirs.

Regards,
Niklas

Re: [PATCH net-next 2/2] net: stmmac: fix number of tx queues in stmmac_poll

[Niklas Cassel]

On 03/27/2017 07:44 PM, Joao Pinto wrote:
> Às 6:28 PM de 3/27/2017, David Miller escreveu:
>> From: Corentin Labbe <clabbe.montjoie@gmail.com>
>> Date: Mon, 27 Mar 2017 19:00:58 +0200
>>
>>> On Mon, Mar 27, 2017 at 04:26:48PM +0100, Joao Pinto wrote:
>>>> Hi David,
>>>>
>>>> Às 7:26 AM de 3/25/2017, Corentin Labbe escreveu:
>>>>> On Fri, Mar 24, 2017 at 05:16:45PM +0000, Joao Pinto wrote:
>>>>>> For cores that have more than 1 TX queue configured, the kernel would crash,
>>>>>> since only one TX queue is permitted by default.
>>>>>>
>>>>>> Signed-off-by: Joao Pinto <jpinto@synopsys.com>
>>>>>> ---
>>>>>>  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 +-
>>>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>>>>>> index 3827952..1eab084 100644
>>>>>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>>>>>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>>>>>> @@ -3429,7 +3429,7 @@ static int stmmac_poll(struct napi_struct *napi, int budget)
>>>>>>  	struct stmmac_rx_queue *rx_q =
>>>>>>  		container_of(napi, struct stmmac_rx_queue, napi);
>>>>>>  	struct stmmac_priv *priv = rx_q->priv_data;
>>>>>> -	u32 tx_count = priv->dma_cap.number_tx_queues;
>>>>>> +	u32 tx_count = priv->plat->tx_queues_to_use;
>>>>>>  	u32 chan = rx_q->queue_index;
>>>>>>  	u32 work_done = 0;
>>>>>>  	u32 queue = 0;
>>>>>> -- 
>>>>>> 2.9.3
>>>>>>
>>>>>
>>>>> This patch fix the performance issue on dwmac-sun8i only.
>>>>> The dwmac-sunxi is still broken.
>>>>>
>>>>
>>>> This patch series can be upstreamed please, since they make 2 fixes, one of them
>>>> solving the problem in dwmac-sun8i.
>>>>
>>>> Thanks.
>>>
>>> As I said in a previous answer, finaly dwmac-sun8i is still broken.
>>> Adding thoses 2 patch will just made the revert harder.
>>
>> I agree.
> 
> For what I am understanding, SoCs base on Core versions >= 4.00 are working
> properly and for some reason SoCs based on older versions are not working.
> 
> This fix is necessary, since if you have a diferent configured tx_queues_to_use
> in the driver and priv->dma_cap.number_tx_queues in the core, this can lead to
> kernel crashes.
> 
> The other fix (netdev resources release) is also necessary, since when you
> release the driver its crashes, because the rx queue struct is freed before
> releasing the netdevs.
> 
> We can revert, but I think it might not solve the issue. We can break the
> "multiple buffers" patch into "rx multilple buffers" and "tx multiple buffers",
> but will that actually work? We can give it a try, I don't mind making a new
> multiple buffers patch broken into 2, that can be tested by new cores and older
> cores.

I've hit a bug on stmmac where RX is broken after boot.
Sometimes it works, and sometimes it doesn't.
I usually notice that DHCP never receives an offer,
but it's possible to reproduce the problem without DHCP,
where a simple ping will not work after setting an address manually.

I've bisected it to commit aff3d9eff843 ("net: stmmac: enable multiple buffers").
(Note that I had to cherry-pick 22446ad8e118 ("net: stmmac: Restore DT backwards-compatibility")
to avoid TX queue timeouts, and the patch included in the beginning of this
mail thread "net: stmmac: fix number of tx queues in stmmac_poll" to avoid
random crashes in stmmac_tx_clean.

Looking at wireshark I can see that we send out a DHCP discover,
so TX seems to be working.
A DHCP offer is sent out from the server, but it is never received.

Our setup has 1 RX queue and 2 TX queues.
According to the databook, the field Receive Queue Size in register
MTL_RxQ0_Operation_Mode is read-only when number of RX queues == 1,
so I guess the problem is not related to RX queue size.
It's quite annoying since it does not trigger every single boot.

Has anyone else noticed broken RX after boot since commit aff3d9eff843 and newer?

Re: [PATCH net-next 2/2] net: stmmac: fix number of tx queues in stmmac_poll

[Thierry Reding]

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

On Tue, Mar 28, 2017 at 03:34:58PM +0200, Niklas Cassel wrote:
> 
> 
> On 03/27/2017 07:44 PM, Joao Pinto wrote:
> > Às 6:28 PM de 3/27/2017, David Miller escreveu:
> >> From: Corentin Labbe <clabbe.montjoie@gmail.com>
> >> Date: Mon, 27 Mar 2017 19:00:58 +0200
> >>
> >>> On Mon, Mar 27, 2017 at 04:26:48PM +0100, Joao Pinto wrote:
> >>>> Hi David,
> >>>>
> >>>> Às 7:26 AM de 3/25/2017, Corentin Labbe escreveu:
> >>>>> On Fri, Mar 24, 2017 at 05:16:45PM +0000, Joao Pinto wrote:
> >>>>>> For cores that have more than 1 TX queue configured, the kernel would crash,
> >>>>>> since only one TX queue is permitted by default.
> >>>>>>
> >>>>>> Signed-off-by: Joao Pinto <jpinto@synopsys.com>
> >>>>>> ---
> >>>>>>  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 +-
> >>>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>>>>>
> >>>>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> >>>>>> index 3827952..1eab084 100644
> >>>>>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> >>>>>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> >>>>>> @@ -3429,7 +3429,7 @@ static int stmmac_poll(struct napi_struct *napi, int budget)
> >>>>>>  	struct stmmac_rx_queue *rx_q =
> >>>>>>  		container_of(napi, struct stmmac_rx_queue, napi);
> >>>>>>  	struct stmmac_priv *priv = rx_q->priv_data;
> >>>>>> -	u32 tx_count = priv->dma_cap.number_tx_queues;
> >>>>>> +	u32 tx_count = priv->plat->tx_queues_to_use;
> >>>>>>  	u32 chan = rx_q->queue_index;
> >>>>>>  	u32 work_done = 0;
> >>>>>>  	u32 queue = 0;
> >>>>>> -- 
> >>>>>> 2.9.3
> >>>>>>
> >>>>>
> >>>>> This patch fix the performance issue on dwmac-sun8i only.
> >>>>> The dwmac-sunxi is still broken.
> >>>>>
> >>>>
> >>>> This patch series can be upstreamed please, since they make 2 fixes, one of them
> >>>> solving the problem in dwmac-sun8i.
> >>>>
> >>>> Thanks.
> >>>
> >>> As I said in a previous answer, finaly dwmac-sun8i is still broken.
> >>> Adding thoses 2 patch will just made the revert harder.
> >>
> >> I agree.
> > 
> > For what I am understanding, SoCs base on Core versions >= 4.00 are working
> > properly and for some reason SoCs based on older versions are not working.
> > 
> > This fix is necessary, since if you have a diferent configured tx_queues_to_use
> > in the driver and priv->dma_cap.number_tx_queues in the core, this can lead to
> > kernel crashes.
> > 
> > The other fix (netdev resources release) is also necessary, since when you
> > release the driver its crashes, because the rx queue struct is freed before
> > releasing the netdevs.
> > 
> > We can revert, but I think it might not solve the issue. We can break the
> > "multiple buffers" patch into "rx multilple buffers" and "tx multiple buffers",
> > but will that actually work? We can give it a try, I don't mind making a new
> > multiple buffers patch broken into 2, that can be tested by new cores and older
> > cores.
> 
> I've hit a bug on stmmac where RX is broken after boot.
> Sometimes it works, and sometimes it doesn't.
> I usually notice that DHCP never receives an offer,
> but it's possible to reproduce the problem without DHCP,
> where a simple ping will not work after setting an address manually.
> 
> I've bisected it to commit aff3d9eff843 ("net: stmmac: enable multiple buffers").
> (Note that I had to cherry-pick 22446ad8e118 ("net: stmmac: Restore DT backwards-compatibility")
> to avoid TX queue timeouts, and the patch included in the beginning of this
> mail thread "net: stmmac: fix number of tx queues in stmmac_poll" to avoid
> random crashes in stmmac_tx_clean.
> 
> Looking at wireshark I can see that we send out a DHCP discover,
> so TX seems to be working.
> A DHCP offer is sent out from the server, but it is never received.
> 
> Our setup has 1 RX queue and 2 TX queues.
> According to the databook, the field Receive Queue Size in register
> MTL_RxQ0_Operation_Mode is read-only when number of RX queues == 1,
> so I guess the problem is not related to RX queue size.
> It's quite annoying since it does not trigger every single boot.
> 
> Has anyone else noticed broken RX after boot since commit aff3d9eff843 and newer?

Yeah, I've been hitting the same, or at least very similar, issue. I'm
going to attach my local stack of three patches, of which the first is
only cleanup, the second is the same as the one that Joao had sent out
earlier and patch three seems to be the one that gets things back on
track for me. I haven't quite figured out why yet, but it'd be
interesting to see if it fixes things for others.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH 1/3] net: stmmac: Remove unneeded checks for NULL pointer

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

Taking the address of an element within a non-NULL array will never be
zero. This condition isn't checked anywhere else, so drop it in these
two instances as well.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index c78f444ad423..106ace3781b3 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -1332,9 +1332,6 @@ static void free_rx_dma_desc_resources(struct stmmac_priv *priv)
 	for (queue = 0; queue < rx_count; queue++) {
 		struct stmmac_rx_queue *rx_q = &priv->rx_queue[queue];
 
-		if (!rx_q)
-			break;
-
 		/* Release the DMA RX socket buffers */
 		dma_free_rx_skbufs(priv, queue);
 
@@ -1373,9 +1370,6 @@ static void free_tx_dma_desc_resources(struct stmmac_priv *priv)
 	for (queue = 0; queue < tx_count; queue++) {
 		struct stmmac_tx_queue *tx_q = &priv->tx_queue[queue];
 
-		if (!tx_q)
-			break;
-
 		/* Release the DMA TX socket buffers */
 		dma_free_tx_skbufs(priv, queue);
 
-- 
2.12.0

[PATCH 2/3] net: stmmac: Always use the number of configured TX queues

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

Even if hardware supports multiple queues, software can choose to only
use a subset of them. Make sure we never try to access uninitialized
queues.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 106ace3781b3..ec5bba85c529 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -3423,7 +3423,7 @@ static int stmmac_poll(struct napi_struct *napi, int budget)
 	struct stmmac_rx_queue *rx_q =
 		container_of(napi, struct stmmac_rx_queue, napi);
 	struct stmmac_priv *priv = rx_q->priv_data;
-	u32 tx_count = priv->dma_cap.number_tx_queues;
+	u32 tx_count = priv->plat->tx_queues_to_use;
 	u32 chan = rx_q->queue_index;
 	u32 work_done = 0;
 	u32 queue = 0;
-- 
2.12.0

[PATCH 3/3] net: stmmac: Prefer kcalloc() over kmalloc_array()

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

Some of the data in the new queue structures seems to not be properly
initialized, causing undefined behaviour (networking will work about 2
out of 10 tries). kcalloc() will zero the allocated memory and results
in 10 out of 10 successful boots.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 37 +++++++++++------------
 1 file changed, 17 insertions(+), 20 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index ec5bba85c529..845320bc3333 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -1412,13 +1412,12 @@ static void free_dma_desc_resources(struct stmmac_priv *priv)
 static int alloc_rx_dma_desc_resources(struct stmmac_priv *priv)
 {
 	u32 rx_count = priv->plat->rx_queues_to_use;
+	struct stmmac_rx_queue *rx_q;
 	int ret = -ENOMEM;
 	u32 queue = 0;
 
 	/* Allocate RX queues array */
-	priv->rx_queue = kmalloc_array(rx_count,
-				       sizeof(struct stmmac_rx_queue),
-				       GFP_KERNEL);
+	priv->rx_queue = kcalloc(rx_count, sizeof(*rx_q), GFP_KERNEL);
 	if (!priv->rx_queue) {
 		kfree(priv->rx_queue);
 		return -ENOMEM;
@@ -1426,20 +1425,19 @@ static int alloc_rx_dma_desc_resources(struct stmmac_priv *priv)
 
 	/* RX queues buffers and DMA */
 	for (queue = 0; queue < rx_count; queue++) {
-		struct stmmac_rx_queue *rx_q = &priv->rx_queue[queue];
+		rx_q = &priv->rx_queue[queue];
 
 		rx_q->queue_index = queue;
 		rx_q->priv_data = priv;
 
-		rx_q->rx_skbuff_dma = kmalloc_array(DMA_RX_SIZE,
-							sizeof(dma_addr_t),
-							GFP_KERNEL);
+		rx_q->rx_skbuff_dma = kcalloc(DMA_RX_SIZE, sizeof(dma_addr_t),
+					      GFP_KERNEL);
 		if (!rx_q->rx_skbuff_dma)
 			goto err_dma_buffers;
 
-		rx_q->rx_skbuff = kmalloc_array(DMA_RX_SIZE,
-						    sizeof(struct sk_buff *),
-						    GFP_KERNEL);
+		rx_q->rx_skbuff = kcalloc(DMA_RX_SIZE,
+					  sizeof(struct sk_buff *),
+					  GFP_KERNEL);
 		if (!rx_q->rx_skbuff)
 			goto err_dma_buffers;
 
@@ -1477,33 +1475,32 @@ static int alloc_rx_dma_desc_resources(struct stmmac_priv *priv)
 static int alloc_tx_dma_desc_resources(struct stmmac_priv *priv)
 {
 	u32 tx_count = priv->plat->tx_queues_to_use;
+	struct stmmac_tx_queue *tx_q;
 	int ret = -ENOMEM;
 	u32 queue = 0;
 
 	/* Allocate TX queues array */
-	priv->tx_queue = kmalloc_array(tx_count,
-				       sizeof(struct stmmac_tx_queue),
-				       GFP_KERNEL);
+	priv->tx_queue = kcalloc(tx_count, sizeof(*tx_q), GFP_KERNEL);
 	if (!priv->tx_queue)
 		return -ENOMEM;
 
 	/* TX queues buffers and DMA */
 	for (queue = 0; queue < tx_count; queue++) {
-		struct stmmac_tx_queue *tx_q = &priv->tx_queue[queue];
+		tx_q = &priv->tx_queue[queue];
 
 		tx_q->queue_index = queue;
 		tx_q->priv_data = priv;
 
-		tx_q->tx_skbuff_dma = kmalloc_array(DMA_TX_SIZE,
-					  sizeof(struct stmmac_tx_info),
-					  GFP_KERNEL);
+		tx_q->tx_skbuff_dma = kcalloc(DMA_TX_SIZE,
+					      sizeof(struct stmmac_tx_info),
+					      GFP_KERNEL);
 
 		if (!tx_q->tx_skbuff_dma)
 			goto err_dma_buffers;
 
-		tx_q->tx_skbuff = kmalloc_array(DMA_TX_SIZE,
-						    sizeof(struct sk_buff *),
-						    GFP_KERNEL);
+		tx_q->tx_skbuff = kcalloc(DMA_TX_SIZE,
+					  sizeof(struct sk_buff *),
+					  GFP_KERNEL);
 		if (!tx_q->tx_skbuff)
 			goto err_dma_buffers;
 
-- 
2.12.0

Re: [PATCH 2/3] net: stmmac: Always use the number of configured TX queues

[Niklas Cassel]

On 03/28/2017 03:57 PM, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
> 
> Even if hardware supports multiple queues, software can choose to only
> use a subset of them. Make sure we never try to access uninitialized
> queues.
> 
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
>  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 106ace3781b3..ec5bba85c529 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -3423,7 +3423,7 @@ static int stmmac_poll(struct napi_struct *napi, int budget)
>  	struct stmmac_rx_queue *rx_q =
>  		container_of(napi, struct stmmac_rx_queue, napi);
>  	struct stmmac_priv *priv = rx_q->priv_data;
> -	u32 tx_count = priv->dma_cap.number_tx_queues;
> +	u32 tx_count = priv->plat->tx_queues_to_use;
>  	u32 chan = rx_q->queue_index;
>  	u32 work_done = 0;
>  	u32 queue = 0;
> 

Thanks Thierry,
but this look like an equivalent patch to Joao's
"net: stmmac: fix number of tx queues in stmmac_poll"
which I already used during bisecting to avoid random
crashes in stmmac_tx_clean.

Have you noticed random RX brokenness after boot in
recent linux-next tags?

It feels like it might trigger more often after a
cold boot, but that could just be my brain seeing
patterns that aren't really there :)

Re: [PATCH 2/3] net: stmmac: Always use the number of configured TX queues

[Thierry Reding]

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

On Tue, Mar 28, 2017 at 04:10:43PM +0200, Niklas Cassel wrote:
> 
> 
> On 03/28/2017 03:57 PM, Thierry Reding wrote:
> > From: Thierry Reding <treding@nvidia.com>
> > 
> > Even if hardware supports multiple queues, software can choose to only
> > use a subset of them. Make sure we never try to access uninitialized
> > queues.
> > 
> > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > ---
> >  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > index 106ace3781b3..ec5bba85c529 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > @@ -3423,7 +3423,7 @@ static int stmmac_poll(struct napi_struct *napi, int budget)
> >  	struct stmmac_rx_queue *rx_q =
> >  		container_of(napi, struct stmmac_rx_queue, napi);
> >  	struct stmmac_priv *priv = rx_q->priv_data;
> > -	u32 tx_count = priv->dma_cap.number_tx_queues;
> > +	u32 tx_count = priv->plat->tx_queues_to_use;
> >  	u32 chan = rx_q->queue_index;
> >  	u32 work_done = 0;
> >  	u32 queue = 0;
> > 
> 
> Thanks Thierry,
> but this look like an equivalent patch to Joao's
> "net: stmmac: fix number of tx queues in stmmac_poll"
> which I already used during bisecting to avoid random
> crashes in stmmac_tx_clean.

Yeah, patch 3/3 is the important one in this batch. I just sent out the
complete set of 3 because it's what I use on top of linux-next to make
stmmac work again on my setup.

> Have you noticed random RX brokenness after boot in
> recent linux-next tags?

Yes, I was seeing about 8 in 10 bad boots and 2 in 10 good ones.

> It feels like it might trigger more often after a
> cold boot, but that could just be my brain seeing
> patterns that aren't really there :)

It'd be good if you could try applying patch 3/3 since that got me back
to 10/10 successful boots. It's somewhat surprising because I had gone
through the code looking for uninitialized data that would explain why
kcalloc() instead of kmalloc_array() would fix things but didn't find
anything.

Still, that patch works for me, so maybe I was just overlooking
something.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 3/3] net: stmmac: Prefer kcalloc() over kmalloc_array()

[Niklas Cassel]

(resending mail without SPAM header)

Hi Thierry

Sorry that I missed your previous email,
for some reason it got stuck in the spam filter.

Really good catch. This patch fixes the random
RX brokenness for me. Thanks!

It would be nice with a Fixes tag though,
but lately there's been so many changes,
so it might be hard to point to a single commit.

Nevertheless:

Tested-by: Niklas Cassel <niklas.cassel@axis.com>

On 03/28/2017 03:57 PM, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
> 
> Some of the data in the new queue structures seems to not be properly
> initialized, causing undefined behaviour (networking will work about 2
> out of 10 tries). kcalloc() will zero the allocated memory and results
> in 10 out of 10 successful boots.
> 
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
>  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 37 +++++++++++------------
>  1 file changed, 17 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index ec5bba85c529..845320bc3333 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -1412,13 +1412,12 @@ static void free_dma_desc_resources(struct stmmac_priv *priv)
>  static int alloc_rx_dma_desc_resources(struct stmmac_priv *priv)
>  {
>  	u32 rx_count = priv->plat->rx_queues_to_use;
> +	struct stmmac_rx_queue *rx_q;
>  	int ret = -ENOMEM;
>  	u32 queue = 0;
>  
>  	/* Allocate RX queues array */
> -	priv->rx_queue = kmalloc_array(rx_count,
> -				       sizeof(struct stmmac_rx_queue),
> -				       GFP_KERNEL);
> +	priv->rx_queue = kcalloc(rx_count, sizeof(*rx_q), GFP_KERNEL);
>  	if (!priv->rx_queue) {
>  		kfree(priv->rx_queue);
>  		return -ENOMEM;
> @@ -1426,20 +1425,19 @@ static int alloc_rx_dma_desc_resources(struct stmmac_priv *priv)
>  
>  	/* RX queues buffers and DMA */
>  	for (queue = 0; queue < rx_count; queue++) {
> -		struct stmmac_rx_queue *rx_q = &priv->rx_queue[queue];
> +		rx_q = &priv->rx_queue[queue];
>  
>  		rx_q->queue_index = queue;
>  		rx_q->priv_data = priv;
>  
> -		rx_q->rx_skbuff_dma = kmalloc_array(DMA_RX_SIZE,
> -							sizeof(dma_addr_t),
> -							GFP_KERNEL);
> +		rx_q->rx_skbuff_dma = kcalloc(DMA_RX_SIZE, sizeof(dma_addr_t),
> +					      GFP_KERNEL);
>  		if (!rx_q->rx_skbuff_dma)
>  			goto err_dma_buffers;
>  
> -		rx_q->rx_skbuff = kmalloc_array(DMA_RX_SIZE,
> -						    sizeof(struct sk_buff *),
> -						    GFP_KERNEL);
> +		rx_q->rx_skbuff = kcalloc(DMA_RX_SIZE,
> +					  sizeof(struct sk_buff *),
> +					  GFP_KERNEL);
>  		if (!rx_q->rx_skbuff)
>  			goto err_dma_buffers;
>  
> @@ -1477,33 +1475,32 @@ static int alloc_rx_dma_desc_resources(struct stmmac_priv *priv)
>  static int alloc_tx_dma_desc_resources(struct stmmac_priv *priv)
>  {
>  	u32 tx_count = priv->plat->tx_queues_to_use;
> +	struct stmmac_tx_queue *tx_q;
>  	int ret = -ENOMEM;
>  	u32 queue = 0;
>  
>  	/* Allocate TX queues array */
> -	priv->tx_queue = kmalloc_array(tx_count,
> -				       sizeof(struct stmmac_tx_queue),
> -				       GFP_KERNEL);
> +	priv->tx_queue = kcalloc(tx_count, sizeof(*tx_q), GFP_KERNEL);
>  	if (!priv->tx_queue)
>  		return -ENOMEM;
>  
>  	/* TX queues buffers and DMA */
>  	for (queue = 0; queue < tx_count; queue++) {
> -		struct stmmac_tx_queue *tx_q = &priv->tx_queue[queue];
> +		tx_q = &priv->tx_queue[queue];
>  
>  		tx_q->queue_index = queue;
>  		tx_q->priv_data = priv;
>  
> -		tx_q->tx_skbuff_dma = kmalloc_array(DMA_TX_SIZE,
> -					  sizeof(struct stmmac_tx_info),
> -					  GFP_KERNEL);
> +		tx_q->tx_skbuff_dma = kcalloc(DMA_TX_SIZE,
> +					      sizeof(struct stmmac_tx_info),
> +					      GFP_KERNEL);
>  
>  		if (!tx_q->tx_skbuff_dma)
>  			goto err_dma_buffers;
>  
> -		tx_q->tx_skbuff = kmalloc_array(DMA_TX_SIZE,
> -						    sizeof(struct sk_buff *),
> -						    GFP_KERNEL);
> +		tx_q->tx_skbuff = kcalloc(DMA_TX_SIZE,
> +					  sizeof(struct sk_buff *),
> +					  GFP_KERNEL);
>  		if (!tx_q->tx_skbuff)
>  			goto err_dma_buffers;
>  
>