From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Thu, 21 Jun 2018 11:34:15 -0400 (EDT) From: Alan Stern To: Sebastian Andrzej Siewior cc: Marcel Holtmann , Greg Kroah-Hartman , "open list:BLUETOOTH DRIVERS" , , Thomas Gleixner , Johan Hedberg Subject: Re: [PATCH] Bluetooth: btusb: use irqsave() in URB's complete callback In-Reply-To: <20180621125207.7xtz2jpnp2vdvaau@linutronix.de> Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII List-ID: On Thu, 21 Jun 2018, Sebastian Andrzej Siewior wrote: > On 2018-06-21 14:43:41 [+0200], Marcel Holtmann wrote: > > Hi Sebastian, > Hi Marcel, > > > > The USB completion callback does not disable interrupts while acquiring > > > the ->lock. We want to remove the local_irq_disable() invocation from > > > __usb_hcd_giveback_urb() and therefore it is required for the callback > > > handler to disable the interrupts while acquiring the lock. > > > The callback may be invoked either in IRQ or BH context depending on the > > > USB host controller. > > > Use the _irqsave variant of the locking primitives. > > > > > > Cc: Marcel Holtmann > > > Cc: Johan Hedberg > > > Cc: linux-bluetooth@vger.kernel.org > > > Signed-off-by: Sebastian Andrzej Siewior > > > --- > > > drivers/bluetooth/btusb.c | 20 ++++++++++++-------- > > > 1 file changed, 12 insertions(+), 8 deletions(-) > > > > can I get an ACK from someone ensuring that this is the direction we are going with the USB host controllers? > +Alan. > > EHCI completes in BH since v3.12-rc1. In order to get rid of that > local_irq_save() in USB core code I need to make sure that the USB > device driver(s) use irqsave primitives. See > https://lkml.kernel.org/r/Pine.LNX.4.44L0.1806011629140.1404-100000@iolanthe.rowland.org Hi, Marcel! Yes, Sebastian is right. We are aiming to make it possible for the USB core to invoke URB completion handlers with interrupts enabled, in order to reduce latency (since USB interrupt processing can take a fairly long time). And of course, this means completion handlers have to work correctly regardless of whether interrupts are enabled or disabled. Currently ehci-hcd supports this possibility. Other host controller drivers may follow along; I'd like to see xhci-hcd do this too. Alan Stern From mboxrd@z Thu Jan 1 00:00:00 1970 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Subject: Bluetooth: btusb: use irqsave() in URB's complete callback From: Alan Stern Message-Id: Date: Thu, 21 Jun 2018 11:34:15 -0400 (EDT) To: Sebastian Andrzej Siewior Cc: Marcel Holtmann , Greg Kroah-Hartman , "open list:BLUETOOTH DRIVERS" , linux-usb@vger.kernel.org, Thomas Gleixner , Johan Hedberg List-ID: T24gVGh1LCAyMSBKdW4gMjAxOCwgU2ViYXN0aWFuIEFuZHJ6ZWogU2lld2lvciB3cm90ZToKCj4g T24gMjAxOC0wNi0yMSAxNDo0Mzo0MSBbKzAyMDBdLCBNYXJjZWwgSG9sdG1hbm4gd3JvdGU6Cj4g PiBIaSBTZWJhc3RpYW4sCj4gSGkgTWFyY2VsLAo+IAo+ID4gPiBUaGUgVVNCIGNvbXBsZXRpb24g Y2FsbGJhY2sgZG9lcyBub3QgZGlzYWJsZSBpbnRlcnJ1cHRzIHdoaWxlIGFjcXVpcmluZwo+ID4g PiB0aGUgLT5sb2NrLiBXZSB3YW50IHRvIHJlbW92ZSB0aGUgbG9jYWxfaXJxX2Rpc2FibGUoKSBp bnZvY2F0aW9uIGZyb20KPiA+ID4gX191c2JfaGNkX2dpdmViYWNrX3VyYigpIGFuZCB0aGVyZWZv cmUgaXQgaXMgcmVxdWlyZWQgZm9yIHRoZSBjYWxsYmFjawo+ID4gPiBoYW5kbGVyIHRvIGRpc2Fi bGUgdGhlIGludGVycnVwdHMgd2hpbGUgYWNxdWlyaW5nIHRoZSBsb2NrLgo+ID4gPiBUaGUgY2Fs bGJhY2sgbWF5IGJlIGludm9rZWQgZWl0aGVyIGluIElSUSBvciBCSCBjb250ZXh0IGRlcGVuZGlu ZyBvbiB0aGUKPiA+ID4gVVNCIGhvc3QgY29udHJvbGxlci4KPiA+ID4gVXNlIHRoZSBfaXJxc2F2 ZSB2YXJpYW50IG9mIHRoZSBsb2NraW5nIHByaW1pdGl2ZXMuCj4gPiA+IAo+ID4gPiBDYzogTWFy Y2VsIEhvbHRtYW5uIDxtYXJjZWxAaG9sdG1hbm4ub3JnPgo+ID4gPiBDYzogSm9oYW4gSGVkYmVy ZyA8am9oYW4uaGVkYmVyZ0BnbWFpbC5jb20+Cj4gPiA+IENjOiBsaW51eC1ibHVldG9vdGhAdmdl ci5rZXJuZWwub3JnCj4gPiA+IFNpZ25lZC1vZmYtYnk6IFNlYmFzdGlhbiBBbmRyemVqIFNpZXdp b3IgPGJpZ2Vhc3lAbGludXRyb25peC5kZT4KPiA+ID4gLS0tCj4gPiA+IGRyaXZlcnMvYmx1ZXRv b3RoL2J0dXNiLmMgfCAyMCArKysrKysrKysrKystLS0tLS0tLQo+ID4gPiAxIGZpbGUgY2hhbmdl ZCwgMTIgaW5zZXJ0aW9ucygrKSwgOCBkZWxldGlvbnMoLSkKPiA+IAo+ID4gY2FuIEkgZ2V0IGFu IEFDSyBmcm9tIHNvbWVvbmUgZW5zdXJpbmcgdGhhdCB0aGlzIGlzIHRoZSBkaXJlY3Rpb24gd2Ug YXJlIGdvaW5nIHdpdGggdGhlIFVTQiBob3N0IGNvbnRyb2xsZXJzPwo+ICtBbGFuLgo+IAo+IEVI Q0kgY29tcGxldGVzIGluIEJIIHNpbmNlIHYzLjEyLXJjMS4gSW4gb3JkZXIgdG8gZ2V0IHJpZCBv ZiB0aGF0Cj4gbG9jYWxfaXJxX3NhdmUoKSBpbiBVU0IgY29yZSBjb2RlIEkgbmVlZCB0byBtYWtl IHN1cmUgdGhhdCB0aGUgVVNCCj4gZGV2aWNlIGRyaXZlcihzKSB1c2UgaXJxc2F2ZSBwcmltaXRp dmVzLiBTZWUKPiAgIGh0dHBzOi8vbGttbC5rZXJuZWwub3JnL3IvUGluZS5MTlguNC40NEwwLjE4 MDYwMTE2MjkxNDAuMTQwNC0xMDAwMDBAaW9sYW50aGUucm93bGFuZC5vcmcKCkhpLCBNYXJjZWwh CgpZZXMsIFNlYmFzdGlhbiBpcyByaWdodC4gIFdlIGFyZSBhaW1pbmcgdG8gbWFrZSBpdCBwb3Nz aWJsZSBmb3IgdGhlIFVTQiAKY29yZSB0byBpbnZva2UgVVJCIGNvbXBsZXRpb24gaGFuZGxlcnMg d2l0aCBpbnRlcnJ1cHRzIGVuYWJsZWQsIGluIApvcmRlciB0byByZWR1Y2UgbGF0ZW5jeSAoc2lu Y2UgVVNCIGludGVycnVwdCBwcm9jZXNzaW5nIGNhbiB0YWtlIGEgCmZhaXJseSBsb25nIHRpbWUp LiAgQW5kIG9mIGNvdXJzZSwgdGhpcyBtZWFucyBjb21wbGV0aW9uIGhhbmRsZXJzIGhhdmUgCnRv IHdvcmsgY29ycmVjdGx5IHJlZ2FyZGxlc3Mgb2Ygd2hldGhlciBpbnRlcnJ1cHRzIGFyZSBlbmFi bGVkIG9yIApkaXNhYmxlZC4KCkN1cnJlbnRseSBlaGNpLWhjZCBzdXBwb3J0cyB0aGlzIHBvc3Np YmlsaXR5LiAgT3RoZXIgaG9zdCBjb250cm9sbGVyIApkcml2ZXJzIG1heSBmb2xsb3cgYWxvbmc7 IEknZCBsaWtlIHRvIHNlZSB4aGNpLWhjZCBkbyB0aGlzIHRvby4KCkFsYW4gU3Rlcm4KLS0tClRv IHVuc3Vic2NyaWJlIGZyb20gdGhpcyBsaXN0OiBzZW5kIHRoZSBsaW5lICJ1bnN1YnNjcmliZSBs aW51eC11c2IiIGluCnRoZSBib2R5IG9mIGEgbWVzc2FnZSB0byBtYWpvcmRvbW9Admdlci5rZXJu ZWwub3JnCk1vcmUgbWFqb3Jkb21vIGluZm8gYXQgIGh0dHA6Ly92Z2VyLmtlcm5lbC5vcmcvbWFq b3Jkb21vLWluZm8uaHRtbAo=