Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752168AbcL2Bjm convert rfc822-to-8bit (ORCPT ); Wed, 28 Dec 2016 20:39:42 -0500 Received: from smtprelay.synopsys.com ([198.182.60.111]:50235 "EHLO smtprelay.synopsys.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752092AbcL2Bjl (ORCPT ); Wed, 28 Dec 2016 20:39:41 -0500 From: John Youn To: Felipe Balbi , Janusz Dziedzic CC: Lu Baolu , Baolin Wang , Greg KH , USB , LKML , Linaro Kernel Mailman List , Mark Brown Subject: RE: [PATCH] usb: dwc3: gadget: Avoid race between dwc3 interrupt handler and irq thread handler Thread-Topic: [PATCH] usb: dwc3: gadget: Avoid race between dwc3 interrupt handler and irq thread handler Thread-Index: AQHSX05ayuG5wEita0Wh8mrXEu1voqEbnH6AgACNZgCAAdt3AIAADmWAgAAS4FA= Date: Thu, 29 Dec 2016 01:29:49 +0000 Message-ID: <2B3535C5ECE8B5419E3ECBE300772909021B3DECCC@US01WEMBX2.internal.synopsys.com> References: <0d79eb1f34e409749a136173b68f365b545c4789.1482738764.git.baolin.wang@linaro.org> <5861D477.7070407@linux.intel.com> <874m1p3a3v.fsf@linux.intel.com> <87o9zwauwu.fsf@linux.intel.com> In-Reply-To: <87o9zwauwu.fsf@linux.intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.13.184.20] Content-Type: text/plain; charset="Windows-1252" Content-Transfer-Encoding: 8BIT MIME-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1997 Lines: 56 > -----Original Message----- > From: linux-usb-owner@vger.kernel.org [mailto:linux-usb- > owner@vger.kernel.org] On Behalf Of Felipe Balbi > Sent: Wednesday, December 28, 2016 8:19 AM > To: Janusz Dziedzic > Cc: Lu Baolu ; Baolin Wang > ; Greg KH ; USB > ; LKML ; Linaro > Kernel Mailman List ; Mark Brown > > Subject: Re: [PATCH] usb: dwc3: gadget: Avoid race between dwc3 interrupt > handler and irq thread handler > > > Hi, > > 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