linux-kernel - Re: [PATCH RT 2/2] net: gianfar: do not try to cleanup TX packets if they are not done

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Thu, 10 Apr 2014 09:20:25 +0200
From:	Sebastian Andrzej Siewior <bigeasy@...utronix.de>
To:	Scott Wood <scottwood@...escale.com>
CC:	linux-rt-users@...r.kernel.org, linux-kernel@...r.kernel.org,
	Claudiu Manoil <claudiu.manoil@...escale.com>
Subject: Re: [PATCH RT 2/2] net: gianfar: do not try to cleanup TX packets
 if they are not done

On 04/10/2014 02:48 AM, Scott Wood wrote:
> Why is this only being sent to RT and not to netdev for mainline?

I tried. And complained about how that problem was fixed by a duct tape
solution (by dropping the outer loop) instead of understanding the
problem and fixing it properly. Even Eric tied to point out that there
might be something else going on. Look at netdev for "gianfar: Simplify
MQ polling to avoid soft lockup".
The threaded ended ended up with the fact that I took this for -RT only
because netdev had already code for v3.15 and Claudiu managed to
rewrite that part (again) and added napi for TX. So that Patch as-is
does not apply anymore.
With NAPI for TX I had may no longer persists so I will probably drop
this patch in -RT >= 3.15

>> --- a/drivers/net/ethernet/freescale/gianfar.c
>> +++ b/drivers/net/ethernet/freescale/gianfar.c
>> @@ -2516,7 +2515,7 @@ static void gfar_align_skb(struct sk_buff *skb)
>>  }
>>  
>>  /* Interrupt Handler for Transmit complete */
>> -static void gfar_clean_tx_ring(struct gfar_priv_tx_q *tx_queue)
>> +static int gfar_clean_tx_ring(struct gfar_priv_tx_q *tx_queue)
>>  {
>>  	struct net_device *dev = tx_queue->dev;
>>  	struct netdev_queue *txq;
>> @@ -2939,10 +2938,14 @@ static int gfar_poll(struct napi_struct *napi, int budget)
> 
> You changed the return from void to int, but you never added any return
> statement.  GCC should have warned you about this...

Interesting. I remember that I added "howmany" as return value. I
remember testing it. And yet there is evidence that I did not such a
thing. I will add it. Thanks for pointing out.

> 
> -Scott
> 

Sebastian
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/