On Fri, Dec 20, 2013 at 11:55:53AM -0600, Thomas Pugliese wrote:
> Only acquire rc->uwb_dev.mutex in the error case in
> uwbd_evt_handle_rc_bp_slot_change. This fixes a bug where establishing
> a reservation on a new channel will fail if we were unable to establish
> a reservation on the previous channel due to DRP conflict.
>
> If rc->uwb_dev.mutex is acquired in the non-error case when the uwb
> system is attempting to start beaconing, it will block because the start
> beaconing code is holding this mutex. This prevents any other
> notifications from the URC from being processed. In particular, the
> DRP_AVAILABILITY notification will not be processed during the start
> beaconing process which can result in a failure to establish a
> reservation. It is safe to not hold the mutex in the non-error
> case since the only other place rc->uwb_dev.beacon_slot is accessed is
> in the same worker thread that uwbd_evt_handle_rc_bp_slot_change
> executes in.
>
> Signed-off-by: Thomas Pugliese <thomas.pugliese@xxxxxxxxx>
> ---
> drivers/uwb/beacon.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/uwb/beacon.c b/drivers/uwb/beacon.c
> index 4190f29..92407e8 100644
> --- a/drivers/uwb/beacon.c
> +++ b/drivers/uwb/beacon.c
> @@ -516,13 +516,13 @@ int uwbd_evt_handle_rc_bp_slot_change(struct uwb_event *evt)
> }
> bpsc = container_of(evt->notif.rceb, struct uwb_rc_evt_bp_slot_change, rceb);
>
> - mutex_lock(&rc->uwb_dev.mutex);
> if (uwb_rc_evt_bp_slot_change_no_slot(bpsc)) {
> + mutex_lock(&rc->uwb_dev.mutex);
> dev_err(dev, "stopped beaconing: No free slots in BP\n");
It could be just nitpicking, but I think it's a waste of resource to
unnecessarily print to console while holding a lock.
If you move this dev_err() to after mutex_unlock() you'd achieve the
same result with a cheaper lock.
> rc->beaconing = -1;
Although I'm not sure how useful it is to protect this single line of
code.
Br, David Cohen
> + mutex_unlock(&rc->uwb_dev.mutex);
> } else
> rc->uwb_dev.beacon_slot = uwb_rc_evt_bp_slot_change_slot_num(bpsc);
> - mutex_unlock(&rc->uwb_dev.mutex);
>
> return 0;
> }
> --
> 1.7.10.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
