Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933017AbbLRSa0 (ORCPT ); Fri, 18 Dec 2015 13:30:26 -0500 Received: from mail2-relais-roc.national.inria.fr ([192.134.164.83]:12744 "EHLO mail2-relais-roc.national.inria.fr" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932499AbbLRSaZ (ORCPT ); Fri, 18 Dec 2015 13:30:25 -0500 X-IronPort-AV: E=Sophos;i="5.20,447,1444687200"; d="scan'208";a="193033157" Date: Fri, 18 Dec 2015 19:30:20 +0100 (CET) From: Julia Lawall X-X-Sender: jll@hadrien To: Geliang Tang cc: Mathias Nyman , Greg Kroah-Hartman , linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 5/9] usb: xhci: use list_for_each_entry In-Reply-To: <201512190233.xPC4M6l4%fengguang.wu@intel.com> Message-ID: References: <201512190233.xPC4M6l4%fengguang.wu@intel.com> User-Agent: Alpine 2.10 (DEB 1266 2009-07-14) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 6713 Lines: 107 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@01.org > In-Reply-To: <0fbea26fdbcb76e24188fc0800d425f927f45b6f.1450455485.git.geliangtang@163.com> > TO: Geliang Tang > CC: Mathias Nyman , Greg Kroah-Hartman > CC: Geliang Tang , linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org > > 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 > :::::: CC: Greg Kroah-Hartman > > --- > 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/