All of lore.kernel.org
 help / color / mirror / Atom feed
From: smoch@web.de (Soeren Moch)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] USB: EHCI: fix for leaking isochronous data
Date: Thu, 21 Mar 2013 22:45:16 +0100	[thread overview]
Message-ID: <514B7F6C.2060200@web.de> (raw)
In-Reply-To: <Pine.LNX.4.44L0.1303211708470.1899-100000@iolanthe.rowland.org>

On 03/21/13 22:12, Alan Stern wrote:
> On Thu, 21 Mar 2013, Alan Stern wrote:
>
>> On Thu, 21 Mar 2013, Soeren Moch wrote:
>>
>>> Now I found out what is going on here:
>>>
>>> In itd_urb_transaction() we allocate 9 iTDs for each URB with
>>> number_of_packets == 64 in my case. The iTDs are added to
>>> sched->td_list. For a frame-aligned scheduling we need 8 iTDs, the 9th
>>> one is released back to the front of the streams free_list in
>>> iso_sched_free(). This iTD was cleared after allocation and has a frame
>>> number of 0 now. So for each allocation when now_frame == 0 we allocate
>>> from the dma_pool, not from the free_list.
>>
>> Okay, that is a problem.  But it shouldn't be such a big problem,
>> because now_frame should not be equal to 0 very often.
>
> Oh, wait, now I get it.  We never reach a steady state, because the
> free list never shrinks, but occasionally it does increase when
> now_frame is equal to 0.  Even though that doesn't happen very often,
> the effects add up.

Correct. Approximately twice a second we allocate 9 iTDs from the 
dma_pool in my case (the frame numbers run from 0 to 511 - not up to 
2047, which I thought they should). But since we allocate iTDs every 8 
frames in my case (for each active stream), the pool allocations occur 
only when we hit the allocation sequence frame 0,8,16,... (not one of 
the other 7 opportunities frame 1,9,17,... - frame 7,15,23,...)

> Very good; tomorrow I will send your patch in.
>
> Alan Stern
>
Thank you. And also thanks to all who helped to track down this issue.

  Soeren

WARNING: multiple messages have this Message-ID (diff)
From: Soeren Moch <smoch@web.de>
To: Alan Stern <stern@rowland.harvard.edu>
Cc: Arnd Bergmann <arnd@arndb.de>,
	USB list <linux-usb@vger.kernel.org>,
	Jason Cooper <jason@lakedaemon.net>, Andrew Lunn <andrew@lunn.ch>,
	Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>,
	linux-mm@kvack.org,
	Kernel development list <linux-kernel@vger.kernel.org>,
	linux-arm-kernel@lists.infradead.org,
	michael@amarulasolutions.com
Subject: Re: [PATCH] USB: EHCI: fix for leaking isochronous data
Date: Thu, 21 Mar 2013 22:45:16 +0100	[thread overview]
Message-ID: <514B7F6C.2060200@web.de> (raw)
In-Reply-To: <Pine.LNX.4.44L0.1303211708470.1899-100000@iolanthe.rowland.org>

On 03/21/13 22:12, Alan Stern wrote:
> On Thu, 21 Mar 2013, Alan Stern wrote:
>
>> On Thu, 21 Mar 2013, Soeren Moch wrote:
>>
>>> Now I found out what is going on here:
>>>
>>> In itd_urb_transaction() we allocate 9 iTDs for each URB with
>>> number_of_packets == 64 in my case. The iTDs are added to
>>> sched->td_list. For a frame-aligned scheduling we need 8 iTDs, the 9th
>>> one is released back to the front of the streams free_list in
>>> iso_sched_free(). This iTD was cleared after allocation and has a frame
>>> number of 0 now. So for each allocation when now_frame == 0 we allocate
>>> from the dma_pool, not from the free_list.
>>
>> Okay, that is a problem.  But it shouldn't be such a big problem,
>> because now_frame should not be equal to 0 very often.
>
> Oh, wait, now I get it.  We never reach a steady state, because the
> free list never shrinks, but occasionally it does increase when
> now_frame is equal to 0.  Even though that doesn't happen very often,
> the effects add up.

Correct. Approximately twice a second we allocate 9 iTDs from the 
dma_pool in my case (the frame numbers run from 0 to 511 - not up to 
2047, which I thought they should). But since we allocate iTDs every 8 
frames in my case (for each active stream), the pool allocations occur 
only when we hit the allocation sequence frame 0,8,16,... (not one of 
the other 7 opportunities frame 1,9,17,... - frame 7,15,23,...)

> Very good; tomorrow I will send your patch in.
>
> Alan Stern
>
Thank you. And also thanks to all who helped to track down this issue.

  Soeren

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

WARNING: multiple messages have this Message-ID (diff)
From: Soeren Moch <smoch@web.de>
To: Alan Stern <stern@rowland.harvard.edu>
Cc: Arnd Bergmann <arnd@arndb.de>,
	USB list <linux-usb@vger.kernel.org>,
	Jason Cooper <jason@lakedaemon.net>, Andrew Lunn <andrew@lunn.ch>,
	Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>,
	linux-mm@kvack.org,
	Kernel development list <linux-kernel@vger.kernel.org>,
	linux-arm-kernel@lists.infradead.org,
	michael@amarulasolutions.com
Subject: Re: [PATCH] USB: EHCI: fix for leaking isochronous data
Date: Thu, 21 Mar 2013 22:45:16 +0100	[thread overview]
Message-ID: <514B7F6C.2060200@web.de> (raw)
In-Reply-To: <Pine.LNX.4.44L0.1303211708470.1899-100000@iolanthe.rowland.org>

On 03/21/13 22:12, Alan Stern wrote:
> On Thu, 21 Mar 2013, Alan Stern wrote:
>
>> On Thu, 21 Mar 2013, Soeren Moch wrote:
>>
>>> Now I found out what is going on here:
>>>
>>> In itd_urb_transaction() we allocate 9 iTDs for each URB with
>>> number_of_packets == 64 in my case. The iTDs are added to
>>> sched->td_list. For a frame-aligned scheduling we need 8 iTDs, the 9th
>>> one is released back to the front of the streams free_list in
>>> iso_sched_free(). This iTD was cleared after allocation and has a frame
>>> number of 0 now. So for each allocation when now_frame == 0 we allocate
>>> from the dma_pool, not from the free_list.
>>
>> Okay, that is a problem.  But it shouldn't be such a big problem,
>> because now_frame should not be equal to 0 very often.
>
> Oh, wait, now I get it.  We never reach a steady state, because the
> free list never shrinks, but occasionally it does increase when
> now_frame is equal to 0.  Even though that doesn't happen very often,
> the effects add up.

Correct. Approximately twice a second we allocate 9 iTDs from the 
dma_pool in my case (the frame numbers run from 0 to 511 - not up to 
2047, which I thought they should). But since we allocate iTDs every 8 
frames in my case (for each active stream), the pool allocations occur 
only when we hit the allocation sequence frame 0,8,16,... (not one of 
the other 7 opportunities frame 1,9,17,... - frame 7,15,23,...)

> Very good; tomorrow I will send your patch in.
>
> Alan Stern
>
Thank you. And also thanks to all who helped to track down this issue.

  Soeren

  parent reply	other threads:[~2013-03-21 21:45 UTC|newest]

Thread overview: 73+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <Pine.LNX.4.44L0.1302211337580.1529-100000@iolanthe.rowland.org>
2013-03-10 18:45 ` [PATCH] USB: EHCI: fix for leaking isochronous data Soeren Moch
2013-03-10 18:45   ` Soeren Moch
2013-03-10 18:45   ` Soeren Moch
2013-03-10 20:59   ` Alan Stern
2013-03-10 20:59     ` Alan Stern
2013-03-10 20:59     ` Alan Stern
2013-03-14 18:48     ` Soeren Moch
2013-03-14 18:48       ` Soeren Moch
2013-03-14 18:48       ` Soeren Moch
2013-03-14 20:22       ` Soeren Moch
2013-03-14 20:22         ` Soeren Moch
2013-03-14 20:22         ` Soeren Moch
2013-03-14 20:32       ` Alan Stern
2013-03-14 20:32         ` Alan Stern
2013-03-14 20:32         ` Alan Stern
2013-03-14 20:51         ` Soeren Moch
2013-03-14 20:51           ` Soeren Moch
2013-03-14 21:33           ` Alan Stern
2013-03-14 21:33             ` Alan Stern
2013-03-14 21:33             ` Alan Stern
2013-03-15  0:00             ` Soeren Moch
2013-03-15  0:00               ` Soeren Moch
2013-03-15  0:00               ` Soeren Moch
2013-03-15 14:30               ` Alan Stern
2013-03-15 14:30                 ` Alan Stern
2013-03-15 14:30                 ` Alan Stern
2013-03-16  2:10             ` Soeren Moch
2013-03-16  2:10               ` Soeren Moch
2013-03-16  2:10               ` Soeren Moch
2013-03-16 17:39               ` Alan Stern
2013-03-16 17:39                 ` Alan Stern
2013-03-16 17:39                 ` Alan Stern
2013-03-17 16:56                 ` Soeren Moch
2013-03-17 16:56                   ` Soeren Moch
2013-03-17 16:56                   ` Soeren Moch
2013-03-17 17:36                   ` Alan Stern
2013-03-17 17:36                     ` Alan Stern
2013-03-17 17:36                     ` Alan Stern
2013-03-17 17:39                     ` Alan Stern
2013-03-17 17:39                       ` Alan Stern
2013-03-17 17:39                       ` Alan Stern
2013-03-21 17:04                     ` Soeren Moch
2013-03-21 17:04                       ` Soeren Moch
2013-03-21 17:33                       ` Jason Cooper
2013-03-21 17:33                         ` Jason Cooper
2013-03-21 17:33                         ` Jason Cooper
2013-03-21 19:10                         ` Arnd Bergmann
2013-03-21 19:10                           ` Arnd Bergmann
2013-03-21 19:10                           ` Arnd Bergmann
2013-03-21 19:34                           ` Michael Trimarchi
2013-03-21 19:34                             ` Michael Trimarchi
2013-03-21 19:34                             ` Michael Trimarchi
2013-03-21 21:52                           ` Soeren Moch
2013-03-21 21:52                             ` Soeren Moch
2013-03-21 21:52                             ` Soeren Moch
2013-03-21 21:06                       ` Alan Stern
2013-03-21 21:06                         ` Alan Stern
2013-03-21 21:06                         ` Alan Stern
2013-03-21 21:12                         ` Alan Stern
2013-03-21 21:12                           ` Alan Stern
2013-03-21 21:12                           ` Alan Stern
2013-03-21 21:20                           ` Andrew Lunn
2013-03-21 21:20                             ` Andrew Lunn
2013-03-21 21:20                             ` Andrew Lunn
2013-03-21 22:16                             ` Soeren Moch
2013-03-21 22:16                               ` Soeren Moch
2013-03-21 22:16                               ` Soeren Moch
2013-03-22 14:24                               ` Alan Stern
2013-03-22 14:24                                 ` Alan Stern
2013-03-22 14:24                                 ` Alan Stern
2013-03-21 21:45                           ` Soeren Moch [this message]
2013-03-21 21:45                             ` Soeren Moch
2013-03-21 21:45                             ` Soeren Moch

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=514B7F6C.2060200@web.de \
    --to=smoch@web.de \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.