From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1030996AbdAETJO (ORCPT ); Thu, 5 Jan 2017 14:09:14 -0500 Received: from us01smtprelay-2.synopsys.com ([198.182.60.111]:54173 "EHLO smtprelay.synopsys.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1030688AbdAETJD (ORCPT ); Thu, 5 Jan 2017 14:09:03 -0500 Subject: Re: [PATCH] usb: dwc3: gadget: Avoid race between dwc3 interrupt handler and irq thread handler To: Felipe Balbi , Janusz Dziedzic References: <0d79eb1f34e409749a136173b68f365b545c4789.1482738764.git.baolin.wang@linaro.org> <5861D477.7070407@linux.intel.com> <874m1p3a3v.fsf@linux.intel.com> <87o9zwauwu.fsf@linux.intel.com> <2B3535C5ECE8B5419E3ECBE300772909021B3DECCC@US01WEMBX2.internal.synopsys.com> From: John Youn CC: Lu Baolu , Baolin Wang , Greg KH , USB , LKML , Linaro Kernel Mailman List , Mark Brown Message-ID: <0ac76b9b-c12b-866b-f76e-99380b7126fd@synopsys.com> Date: Thu, 5 Jan 2017 11:08:32 -0800 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.6.0 MIME-Version: 1.0 In-Reply-To: <2B3535C5ECE8B5419E3ECBE300772909021B3DECCC@US01WEMBX2.internal.synopsys.com> Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit X-Originating-IP: [10.9.138.175] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 12/28/2016 5:29 PM, John Youn wrote: > > >> Janusz Dziedzic writes: >>>>>> On some platfroms(like x86 platform), when one core is running the >> USB gadget >>>>>> irq thread handler by dwc3_thread_interrupt(), meanwhile another >> core also can >>>>>> respond other interrupts from dwc3 controller and modify the event >> buffer by >>>>>> dwc3_interrupt() function, that will cause getting the wrong event >> count in >>>>>> irq thread handler to make the USB function abnormal. >>>>>> >>>>>> We should add spin_lock/unlock() in dwc3_check_event_buf() to avoid >> this race. >>>>> >>>>> Why not spin_lock_irq ones? This lock seems to be used in both >>>>> normal and interrupt threads. Or, I missed anything? >>>> >>>> this is top half handler. Interrupts are already disabled. >>>> >>> BTW, >>> We don't use spin_lock in top half handler. >>> Maybe we should/can switch all spin_lock_irqsave() to simple >>> spin_lock() in the thread/callbacks? >> >> in theory, yes we've masked all interrupts from this controller for the >> duration of the thread handler. However this breaks networking >> gadgets. I can only guess network stack has a hard requirement to run >> with IRQs disabled. >> > > Hi, > > Is this version 3.00a of the core? > > That version has a STAR where the interrupts cannot be masked. That > results in similar symptoms to what you're seeing here. > Regards, > John > Didn't see any response to this. Just want to make sure this possibility is addressed as there is a workaround for it on mainline. See the following commits for details: d9fa4c63f766 ("usb: dwc3: core: add a event buffer cache") ebbb2d59398f ("usb: dwc3: gadget: use evt->cache for processing events") 65aca3205046 ("usb: dwc3: gadget: clear events in top-half handler") cf40b86b6ef6 ("usb: dwc3: Implement interrupt moderation") 28632b44d129 ("usb: dwc3: Workaround for irq mask issue") Regards, John