Re: [PATCH] Bluetooth: hidp: register HID devices async

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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?

Thanks.

-- 
Jiri Kosina
SUSE Labs
--
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




[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux