[<prev] [next>] [day] [month] [year] [list]
Message-ID: <AANLkTikDeSLkUCRgnEQlyTbLzorCXTgVD7CIr4mN-xYQ@mail.gmail.com>
Date: Thu, 20 May 2010 18:23:30 +0800
From: Sonic Zhang <sonic.adi@...il.com>
To: David Miller <davem@...emloft.net>
Cc: netdev@...r.kernel.org
Subject: Re: [PATCH 06/11] netdev: bfin_mac: avoid tx skb overflows in the tx
DMA ring
On Thu, May 20, 2010 at 6:08 PM, David Miller <davem@...emloft.net> wrote:
> From: Sonic Zhang <sonic.adi@...il.com>
> Date: Thu, 20 May 2010 17:38:07 +0800
>
>> On Thu, May 20, 2010 at 4:12 AM, David Miller <davem@...emloft.net> wrote:
>>> From: Sonic Zhang <sonic.adi@...il.com>
>>> Date: Wed, 19 May 2010 17:23:16 +0800
>>>
>>>> No, this doesn't happen, because before ndo_start_xmit() returns, the
>>>> old TX buffers and skbs in the ring, which finished DMA operation, are
>>>> freed. The only difference is that the free operation of a skb is done
>>>> in next tx transfer.
>>>
>>> This is still illegal.
>>>
>>> What if TX activity stops right then, and there is no "next tx
>>> transfer"?
>>>
>> The skb remain in the TX ring will be freed finally when ndo_stop() is
>> called to shutdown the network. So, this is not a problem.
>
> You really don't understand me, and I'm starting to get really
> frustrated. You must free all packets in your TX ring in a very
> small, finite, amount of time. This is not optional. And this
> must happen regardless of what TX traffic which occurs in the future,
> that means it must happen even if TX traffic suddenly stops.
>
> Your driver's behavior is absolutely not acceptable.
>
> Leaving the SKB In the TX ring like that means that potentially there
> is a socket in the system or other major resource that cannot be released
> and freed up.
>
> Please stop your driver from keeping packets in the TX ring indefinitely.
>
Forgot to CC netdev mailing list in my last reply.
Try again.
Sonic
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists