From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S938644AbcKLHfH (ORCPT ); Sat, 12 Nov 2016 02:35:07 -0500 Received: from mga03.intel.com ([134.134.136.65]:11136 "EHLO mga03.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932841AbcKLHfF (ORCPT ); Sat, 12 Nov 2016 02:35:05 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.31,625,1473145200"; d="scan'208";a="1058560963" Subject: Re: [PATCH v4 1/4] usb: dbc: early driver for xhci debug capability To: Peter Zijlstra References: <1477976354-13291-1-git-send-email-baolu.lu@linux.intel.com> <1477976354-13291-2-git-send-email-baolu.lu@linux.intel.com> <5823CB63.7090603@linux.intel.com> <20161110114445.GW3142@twins.programming.kicks-ass.net> <58254A19.8030003@linux.intel.com> <20161111122824.GH3117@twins.programming.kicks-ass.net> Cc: Thomas Gleixner , Greg Kroah-Hartman , Mathias Nyman , Ingo Molnar , linux-usb@vger.kernel.org, x86@kernel.org, LKML From: Lu Baolu Message-ID: <5826C626.4010600@linux.intel.com> Date: Sat, 12 Nov 2016 15:35:02 +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: <20161111122824.GH3117@twins.programming.kicks-ass.net> 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 Hi Peter, On 11/11/2016 08:28 PM, Peter Zijlstra wrote: > On Fri, Nov 11, 2016 at 12:33:29PM +0800, Lu Baolu wrote: > >> Things become complicated when it comes to USB debug port. >> But it's still addressable. >> >> At this time, we can do it like this. >> >> write() >> { >> if (in_nmi() && raw_spin_is_locked(&lock)) >> return; >> >> raw_spinlock_irqsave(&lock, flags); >> .... >> > Please use raw_spin_trlock_irqsave() instead, spin_is_locked() is fairly > icky. Sure. > > Also, there's a bunch of exception contexts that do not set in_nmi(). > That is in_nmi() is really only set for #NM. #MC and #DB and > others do not set this. That's worth another fix patch. Let me look into it later. > >> This will filter some messages from NMI handler in case that >> another thread is holding the spinlock. I have no idea about >> how much chance could a debug user faces this. But it might >> further be fixed with below enhancement. >> >> write() >> { >> if (in_nmi() && raw_spin_is_locked(&lock)) { >> produce_a_pending_item_in_ring(); >> return; >> } >> >> raw_spinlock_irqsave(&lock, flags); >> >> while (!pending_item_ring_is_empty) >> consume_a_pending_item_in_ring(); >> >> .... >> >> >> We can design the pending item ring in a producer-consumer >> model. It's easy to avoid race between the producer and >> consumer. > Problem is that the consumer might never happen, those are the fun most > bugs. > > Not being able to deal with random nested exception context really > reduces the utility of this thing. > > Again, a UART rules. Make a virtual UART in hardware, that'd be totally > awesome. This thing, I'm not convinced its worth having. This is the initial work. It helps at least in cases where people need to dump kernel messages but lacking of a console. It's also a cheap way, people don't need to buy any third-party devices. With more and more people trying and enhancing it, it will become more robust and helpful. Best regards, Lu Baolu