Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751723AbcL1QUs (ORCPT ); Wed, 28 Dec 2016 11:20:48 -0500 Received: from mga05.intel.com ([192.55.52.43]:61099 "EHLO mga05.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751117AbcL1QUr (ORCPT ); Wed, 28 Dec 2016 11:20:47 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.33,422,1477983600"; d="scan'208";a="47999850" From: Felipe Balbi 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 In-Reply-To: References: <0d79eb1f34e409749a136173b68f365b545c4789.1482738764.git.baolin.wang@linaro.org> <5861D477.7070407@linux.intel.com> <874m1p3a3v.fsf@linux.intel.com> Date: Wed, 28 Dec 2016 18:19:13 +0200 Message-ID: <87o9zwauwu.fsf@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1189 Lines: 33 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. > Or there is a reason to use irqsave() version? see above :-) -- balbi