Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755995AbcL0Cjz (ORCPT ); Mon, 26 Dec 2016 21:39:55 -0500 Received: from mga05.intel.com ([192.55.52.43]:61238 "EHLO mga05.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754345AbcL0Cjy (ORCPT ); Mon, 26 Dec 2016 21:39:54 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.33,414,1477983600"; d="scan'208";a="1104521093" Subject: Re: [PATCH] usb: dwc3: gadget: Avoid race between dwc3 interrupt handler and irq thread handler To: Baolin Wang , balbi@kernel.org, gregkh@linuxfoundation.org References: <0d79eb1f34e409749a136173b68f365b545c4789.1482738764.git.baolin.wang@linaro.org> Cc: linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, linaro-kernel@lists.linaro.org, broonie@kernel.org From: Lu Baolu Message-ID: <5861D477.7070407@linux.intel.com> Date: Tue, 27 Dec 2016 10:39:51 +0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.1 MIME-Version: 1.0 In-Reply-To: <0d79eb1f34e409749a136173b68f365b545c4789.1482738764.git.baolin.wang@linaro.org> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1651 Lines: 50 Hi, On 12/26/2016 04:01 PM, Baolin Wang wrote: > 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? Best regards, Lu Baolu > > Signed-off-by: Baolin Wang > --- > drivers/usb/dwc3/gadget.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c > index 6785595..1a1e1f4 100644 > --- a/drivers/usb/dwc3/gadget.c > +++ b/drivers/usb/dwc3/gadget.c > @@ -2894,10 +2894,13 @@ static irqreturn_t dwc3_check_event_buf(struct dwc3_event_buffer *evt) > return IRQ_HANDLED; > } > > + spin_lock(&dwc->lock); > count = dwc3_readl(dwc->regs, DWC3_GEVNTCOUNT(0)); > count &= DWC3_GEVNTCOUNT_MASK; > - if (!count) > + if (!count) { > + spin_unlock(&dwc->lock); > return IRQ_NONE; > + } > > evt->count = count; > evt->flags |= DWC3_EVENT_PENDING; > @@ -2914,6 +2917,7 @@ static irqreturn_t dwc3_check_event_buf(struct dwc3_event_buffer *evt) > memcpy(evt->cache, evt->buf, count - amount); > > dwc3_writel(dwc->regs, DWC3_GEVNTCOUNT(0), count); > + spin_unlock(&dwc->lock); > > return IRQ_WAKE_THREAD; > }