Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753725AbcL0LNH (ORCPT ); Tue, 27 Dec 2016 06:13:07 -0500 Received: from mga14.intel.com ([192.55.52.115]:59532 "EHLO mga14.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752679AbcL0LM7 (ORCPT ); Tue, 27 Dec 2016 06:12:59 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.33,416,1477983600"; d="asc'?scan'208";a="23186371" From: Felipe Balbi To: Baolin Wang , Janusz Dziedzic Cc: 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 In-Reply-To: References: <0d79eb1f34e409749a136173b68f365b545c4789.1482738764.git.baolin.wang@linaro.org> Date: Tue, 27 Dec 2016 13:11:23 +0200 Message-ID: <87wpel1vac.fsf@linux.intel.com> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha256; protocol="application/pgp-signature" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3562 Lines: 117 --=-=-= Content-Type: text/plain Content-Transfer-Encoding: quoted-printable Hi, Baolin Wang writes: > Hi, > > On 27 December 2016 at 18:52, Janusz Dziedzic = wrote: >> 2016-12-26 9:01 GMT+01:00 Baolin Wang : >>> On some platfroms(like x86 platform), when one core is running the USB = gadget >>> irq thread handler by dwc3_thread_interrupt(), meanwhile another core a= lso can >>> respond other interrupts from dwc3 controller and modify the event buff= er by >>> dwc3_interrupt() function, that will cause getting the wrong event coun= t in >>> irq thread handler to make the USB function abnormal. >>> >>> We should add spin_lock/unlock() in dwc3_check_event_buf() to avoid thi= s race. >>> >> Interesting, I always think we mask interrupt in dwc3_interrupt() by set= ting >> DWC3_GEVNTSIZ_INTMASK >> And unmask interrupt when we end dwc3_thread_interrupt(). >> >> So, we shouldn't get any IRQ from HW during dwc3_thread_interrupt(), >> or I miss something? >> Do you have some traces that indicate this masking will not work correct= ly? > > Yes, but we just masked the interrupts described in DEVTEN register, > and we did not mask all the interrupts, like the endpoint command > complete event, transfer complete event and so on, so we can still get > interrupts. not true, we masked interrupts for the entire event buffer: > static irqreturn_t dwc3_check_event_buf(struct dwc3_event_buffer *evt) > { > struct dwc3 *dwc =3D evt->dwc; > u32 count; > u32 reg; > > if (pm_runtime_suspended(dwc->dev)) { > pm_runtime_get(dwc->dev); > disable_irq_nosync(dwc->irq_gadget); > dwc->pending_events =3D true; > return IRQ_HANDLED; > } > > count =3D dwc3_readl(dwc->regs, DWC3_GEVNTCOUNT(0)); > count &=3D DWC3_GEVNTCOUNT_MASK; > if (!count) > return IRQ_NONE; > > evt->count =3D count; > evt->flags |=3D DWC3_EVENT_PENDING; > > /* Mask interrupt */ > reg =3D dwc3_readl(dwc->regs, DWC3_GEVNTSIZ(0)); > reg |=3D DWC3_GEVNTSIZ_INTMASK; See here ?!? > dwc3_writel(dwc->regs, DWC3_GEVNTSIZ(0), reg); > > return IRQ_WAKE_THREAD; > } >> BTW, what value you get when problem occured, 0xFFFC? > > Yes, something like this, the event count become huge. please send us tracepoint data. You probably need to compress it. Something like 256k of trace data is probably enough, so: # mkdir -p /t # mount -t tracefs none /t # cd /t # echo 256 > buffer_size_kb # echo 1 > events/dwc3/enable # echo 0 > events/dwc3/dwc3_readl/enable # echo 0 > events/dwc3/dwc3_writel/enable (reproduce) # cp /t/trace /path/to/non-volatile/media/trace.txt =2D-=20 balbi --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEElLzh7wn96CXwjh2IzL64meEamQYFAlhiTFsACgkQzL64meEa mQZx1w/9H3GBZDlkEvim4Axz6sAOXIhzKDNxVvqnVftP0HJfvvpoBWctrWL6WdsM Dr7nj2S3orW4SHkLEDX517uI3yuZStTt1WMEiiTk3PmVuIxXXy4acGUMwrFs0zLl xayJyvlmNXKESmyFaElCyo8sA6jrpFU714Ul981zpECN276bRJmGvu0+goxei4YM uJcjOlPDA0RdopR4QTDfNztkQrHZokI7VIwoxVtI0SWr2m+PKVcM3VjNNb2XWlnn 6Ghd1MmwQ4sKK37i2jP7qyonvGlIaVHkAkkmJe0We54iz/+fUk+QnJVLpkWPX4Vm VxPey1idzfwJeQMfjGMgatZVF3+dCRLesCSMz40Rs64DZM8h6bagzlbRjawmOuVH H6jt5DGAzAxkxUL1JVp4c+41VhIbHGdYX4AjJ7BEMlL530a0odaDnUR1Ngb3xlZg wxBZ4aURN/NB+Amh29mA8WBOJRit7Md9nYUvnR5TpS7EKQLAlnEFggqBkzya01IU 4bAJK4OUZ8smV7NIXCHjZjnAGUxKJzS5iKaPmjT578vTRQENK9tWo93rF/nA5gxF kx9k9pTarbFgCHQafeBEAgF7MKeHEFu8LW5CCxT/uzPIkLK9W0qfu/mHtPtZUj/X st1FWTwVEZbKhGtm7zDA3iRBTRQSW45zdan2+lvh1qWxhF9Ie4U= =A+jo -----END PGP SIGNATURE----- --=-=-=--