> On Wed, Nov 6, 2013 at 12:53 AM, <hemantk@xxxxxxxxxxxxxx> wrote: >> Hi >> >> By performing iterative port suspend and resume (which results in >> function >> suspend and resume), ring expansion failure is observed. Attached device >> has multiple interfaces for which interface host drivers are unlinking >> the >> urbs during function suspend and submitting urbs during resume. >> >> For the cases when dequeue ptr is close to the end of deq segment, value >> of num_trbs_in_deq_seg (= ring->dequeue - ring->deq_seg->trbs) becomes >> high when function suspend is over. At the time of resume this high >> value >> is causing to trigger ring expansion in the middle of submitting >> urbs(ring->num_trbs_free < num_trbs + num_trbs_in_deq_seg). All the >> interface host drivers only request one TRB per urb (num_trb =1). Ring >> expansion logic is doubling the num of previously allocated segments >> every >> time. This is causing the num of segments to grow at fast pace (at the >> time of failure, one of the ep rings num_seg was 128), even though it is >> also increasing num_trbs_free. >> > > Sounds like a bug. Why num_trbs_in_deq_seg becomes high? How many urbs > are submitted during resume? Normally only isoc transfer triggers ring > expansion with multiple trbs per urb. Let me explain it little further what is being observed in terms of enq , deq ptr:- For example 1) Let?s say First seg of the ring starts with 0xed0d3000 which means enq ptr and deq ptr are pointing to this address. 2) Interface driver queuing 10 urbs (one urb represents one trb) enq ptr goes to ed0d30a0. 3) Interface suspend happened. All 10 urbs are going to get unlinked. For the first urb to be unlinked, it is added to ep->cancelled_td_list and Stop ep cmd was called (rest of the urbs added to ep->cancelled_td_list before stop ep cmd completed). 4) When stop ep cmd is completed, for 9 urbs td_to_noop() is called. For the first urb (for which stop ep cmd was called), xhci_find_new_dequeue_state() gets called which updates deq ptr to next trb (0x ed0d3010) so set tr deq ptr cmd is issued to xhci to set its deq ptr to next trb(0xed0d3010). 5) In set tr deq cmd completion handler update_ring_for_set_deq_completion() gets called which will just increments num_trbs_free to only one, but we unlinked all the urbs. So when interface suspend finished we have enq ptr pointing to 0x ed0d30a0 and dep ptr pointing to 0x ed0d3010. With num_trbs_free only incremented by one. 6) At the time of interface resume we re-queue 10 urbs back with num_trbs_free (just incremented 1 during suspend). This means num_trbs_free is incrementing very slowly every time interface suspend happens. And it is very easy to get into a situation where room_on_ring() returning 0 as deq ptr will be reaching to the end of the seg and causing num_trbs_in_deq_seg to go high enough that ring->num_trbs_free < num_trbs + num_trbs_in_deq_seg condition becomes true. Thanks, Hemant >> Even for the cases where we observe data transfer as well as suspend & >> resume (which are normal scenarios) ring expansion failure is easily >> observed due to large number of segments being allocated for ever. This >> means triggering of ring expansion will result in failure of further >> expansion at some point of time. >> >> I was trying to think of some options to reduce ring expansion with >> respect to suspend resume scenario: - >> 1) Can we reset enq and deq ptr to point to ep_ring first seg once >> all the >> urbs are unlinked (function suspend)? >> 2) Can we free the segments at the time of suspend and allocate >> segments >> back when resume happens? >> > > These does not point to the root cause of ring expansion failure. > > Thanks, > Andiry > >> In general, as per the spec can we also implement shrinking of transfer >> ring (4.9.2.4). >> >> Please help to provide your comments on the options above (which can >> easily fit in to the current design) also feel free to add better >> options >> to consider to avoid ring expansion failures which can be added easily >> to >> the current implementation. >> >> Currently I am using 3.10 kernel and back ported recent fixes on xhci. >> >> Thanks, >> Hemant >> >> >> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-usb" in >> the body of a message to majordomo@xxxxxxxxxxxxxxx >> More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
