Re: [PATCH 5/9] usb: xhci: use list_for_each_entry

From: Julia Lawall
Date: Fri Dec 18 2015 - 13:30:38 EST

Geliang,

Please check whether it is acceptable that last_unlinked_td point to the
dummy entry at th beginning of the list, in the case where the
list_for_each_entry loop runs out normally.

It seems that you have sent a bunch of these patches. Please recheck them
all to see if they really follow the semantics of list_for_each_entry
properly. If particular, you should not normally use the index variable
after leaving the loop, unless it is guaranteed that the exit from the
loop was via a break.

julia

On Sat, 19 Dec 2015, kbuild test robot wrote:

> CC: kbuild-all@xxxxxx
> In-Reply-To: <0fbea26fdbcb76e24188fc0800d425f927f45b6f.1450455485.git.geliangtang@xxxxxxx>
> TO: Geliang Tang <geliangtang@xxxxxxx>
> CC: Mathias Nyman <mathias.nyman@xxxxxxxxx>, Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> CC: Geliang Tang <geliangtang@xxxxxxx>, linux-usb@xxxxxxxxxxxxxxx, linux-kernel@xxxxxxxxxxxxxxx
>
> Hi Geliang,
>
> [auto build test WARNING on usb/usb-testing]
> [also build test WARNING on v4.4-rc5 next-20151218]
>
> url: https://github.com/0day-ci/linux/commits/Geliang-Tang/usb-host-fotg210-use-list_for_each_entry_safe/20151219-003955
> base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
> :::::: branch date: 2 hours ago
> :::::: commit date: 2 hours ago
>
> >> drivers/usb/host/xhci-ring.c:714:20-26: ERROR: invalid reference to the index variable of the iterator on line 672
>
> git remote add linux-review https://github.com/0day-ci/linux
> git remote update linux-review
> git checkout 0af06cc5bab47032f7fc8e2e6a3df0fb29513b52
> vim +714 drivers/usb/host/xhci-ring.c
>
> ae6367471 Sarah Sharp 2009-04-29 666
> ae6367471 Sarah Sharp 2009-04-29 667 /* Fix up the ep ring first, so HW stops executing cancelled TDs.
> ae6367471 Sarah Sharp 2009-04-29 668 * We have the xHCI lock, so nothing can modify this list until we drop
> ae6367471 Sarah Sharp 2009-04-29 669 * it. We're also in the event handler, so we can't get re-interrupted
> ae6367471 Sarah Sharp 2009-04-29 670 * if another Stop Endpoint command completes
> ae6367471 Sarah Sharp 2009-04-29 671 */
> 0af06cc5b Geliang Tang 2015-12-19 @672 list_for_each_entry(cur_td, &ep->cancelled_td_list, cancelled_td_list) {
> aa50b2906 Xenia Ragiadakou 2013-08-14 673 xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb,
> aa50b2906 Xenia Ragiadakou 2013-08-14 674 "Removing canceled TD starting at 0x%llx (dma).",
> 79688acfb Sarah Sharp 2011-12-19 675 (unsigned long long)xhci_trb_virt_to_dma(
> 79688acfb Sarah Sharp 2011-12-19 676 cur_td->start_seg, cur_td->first_trb));
> e9df17eb1 Sarah Sharp 2010-04-02 677 ep_ring = xhci_urb_to_transfer_ring(xhci, cur_td->urb);
> e9df17eb1 Sarah Sharp 2010-04-02 678 if (!ep_ring) {
> e9df17eb1 Sarah Sharp 2010-04-02 679 /* This shouldn't happen unless a driver is mucking
> e9df17eb1 Sarah Sharp 2010-04-02 680 * with the stream ID after submission. This will
> e9df17eb1 Sarah Sharp 2010-04-02 681 * leave the TD on the hardware ring, and the hardware
> e9df17eb1 Sarah Sharp 2010-04-02 682 * will try to execute it, and may access a buffer
> e9df17eb1 Sarah Sharp 2010-04-02 683 * that has already been freed. In the best case, the
> e9df17eb1 Sarah Sharp 2010-04-02 684 * hardware will execute it, and the event handler will
> e9df17eb1 Sarah Sharp 2010-04-02 685 * ignore the completion event for that TD, since it was
> e9df17eb1 Sarah Sharp 2010-04-02 686 * removed from the td_list for that endpoint. In
> e9df17eb1 Sarah Sharp 2010-04-02 687 * short, don't muck with the stream ID after
> e9df17eb1 Sarah Sharp 2010-04-02 688 * submission.
> e9df17eb1 Sarah Sharp 2010-04-02 689 */
> e9df17eb1 Sarah Sharp 2010-04-02 690 xhci_warn(xhci, "WARN Cancelled URB %p "
> e9df17eb1 Sarah Sharp 2010-04-02 691 "has invalid stream ID %u.\n",
> e9df17eb1 Sarah Sharp 2010-04-02 692 cur_td->urb,
> e9df17eb1 Sarah Sharp 2010-04-02 693 cur_td->urb->stream_id);
> e9df17eb1 Sarah Sharp 2010-04-02 694 goto remove_finished_td;
> e9df17eb1 Sarah Sharp 2010-04-02 695 }
> ae6367471 Sarah Sharp 2009-04-29 696 /*
> ae6367471 Sarah Sharp 2009-04-29 697 * If we stopped on the TD we need to cancel, then we have to
> ae6367471 Sarah Sharp 2009-04-29 698 * move the xHC endpoint ring dequeue pointer past this TD.
> ae6367471 Sarah Sharp 2009-04-29 699 */
> 63a0d9abd Sarah Sharp 2009-09-04 700 if (cur_td == ep->stopped_td)
> e9df17eb1 Sarah Sharp 2010-04-02 701 xhci_find_new_dequeue_state(xhci, slot_id, ep_index,
> e9df17eb1 Sarah Sharp 2010-04-02 702 cur_td->urb->stream_id,
> e9df17eb1 Sarah Sharp 2010-04-02 703 cur_td, &deq_state);
> ae6367471 Sarah Sharp 2009-04-29 704 else
> 522989a27 Sarah Sharp 2011-07-29 705 td_to_noop(xhci, ep_ring, cur_td, false);
> e9df17eb1 Sarah Sharp 2010-04-02 706 remove_finished_td:
> ae6367471 Sarah Sharp 2009-04-29 707 /*
> ae6367471 Sarah Sharp 2009-04-29 708 * The event handler won't see a completion for this TD anymore,
> ae6367471 Sarah Sharp 2009-04-29 709 * so remove it from the endpoint ring's TD list. Keep it in
> ae6367471 Sarah Sharp 2009-04-29 710 * the cancelled TD list for URB completion later.
> ae6367471 Sarah Sharp 2009-04-29 711 */
> 585df1d90 Sarah Sharp 2011-08-02 712 list_del_init(&cur_td->td_list);
> ae6367471 Sarah Sharp 2009-04-29 713 }
> ae6367471 Sarah Sharp 2009-04-29 @714 last_unlinked_td = cur_td;
> 6f5165cf9 Sarah Sharp 2009-10-27 715 xhci_stop_watchdog_timer_in_irq(xhci, ep);
> ae6367471 Sarah Sharp 2009-04-29 716
> ae6367471 Sarah Sharp 2009-04-29 717 /* If necessary, queue a Set Transfer Ring Dequeue Pointer command */
>
> :::::: The code at line 714 was first introduced by commit
> :::::: ae636747146ea97efa18e04576acd3416e2514f5 USB: xhci: URB cancellation support.
>
> :::::: TO: Sarah Sharp <sarah.a.sharp@xxxxxxxxxxxxxxx>
> :::::: CC: Greg Kroah-Hartman <gregkh@xxxxxxx>
>
> ---
> 0-DAY kernel test infrastructure Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all Intel Corporation
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/