Hi Jiri, * Jiri Kosina <jkosina@xxxxxxx> [2013-05-28 10:45:26 +0200]: > On Thu, 23 May 2013, David Herrmann wrote: > > > While l2cap_user callbacks are running, the whole hci_dev is locked. Even > > if we would add more fine-grained locking to HCI core, it would still be > > called from the non-reentrant rx work-queue and thus block the event > > processing. > > > > However, if we want to perform synchronous I/O during HID device > > registration (eg., to perform device-detection), we need the HCI core > > to be able to dispatch incoming data. > > > > Therefore, we now move device-registration to a separate worker. The HCI > > core can continue running and we add devices asynchronously in another > > kernel thread. Device removal is synchronized and waits for the worker > > to exit before calling the usual device removal functions. > > > > If l2cap_user->remove is called before the thread registered the devices, > > we set "terminate" to true and the thread will skip it. If > > l2cap_user->remove is called after it, we notice this as the device > > is no longer in HIDP_SESSION_PREPARING state and simply unregister the > > device as we did before. > > There is no new deadlock as we now call hidp_session_add_dev() with > > one lock less held (the HCI lock) and it cannot itself call back into > > HCI as it was called with the HCI-lock held before. > > > > One might wonder whether this can block during device unregistration. > > But we set "terminate" to true and wake the HIDP thread up _before_ > > unregistering the HID/input devices. Therefore, all pending HID I/O > > operations are canceled. All further I/O attempts will fail with ENODEV > > or EIO. So all latency we can get are few context-switches, but no > > timeouts or blocking I/O waits! > > > > This change also prepares for a long standing HID bug. All HID devices > > that register power_supply devices need to be able to handle callbacks > > during registration (a power_supply oddity that cannot easily be fixed). > > So with this patch available, we can allow HID I/O during registration > > by calling the recently introduced hid_device_io_start/stop helpers, > > which currently are a no-op for bluetooth due to this locking. > > > > Note that we cannot do the same for input devices. input-core doesn't > > allow us to call input_event() asynchronously to input_register_device(), > > which HID-core kindly allows (for good reasons). > > Fixing input-core to allow this isn't as easy as it sounds and is, > > beside simplifying HIDP, not really an improvement. Hence, we still > > register input devices synchronously as we did before. Only HID devices > > are registered asynchronously. > > > > Signed-off-by: David Herrmann <dh.herrmann@xxxxxxxxx> > > Acked-by: Jiri Kosina <jkosina@xxxxxxx> > > Gustavo, I think I'd like to take this patch together with the ENODATA > change for hid-input, as they, in some sense, stick together. > > If you are OK with that, could you please provide me with your Acked-by, > and I'll take it through my tree? This looks good to me. Acked-by: Gustavo Padovan <gustavo.padovan@xxxxxxxxxxxxxxx> Gustavo -- To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
