On 23/01/17 12:04, Johan Hovold wrote:

> +static void gb_operation_timeout(unsigned long arg)
> +{
> +     struct gb_operation *operation = (void *)arg;
> +
> +     if (gb_operation_result_set(operation, -ETIMEDOUT)) {
> +             /*
> +              * A stuck request message will be cancelled from the
> +              * workqueue.
> +              */
> +             queue_work(gb_operation_completion_wq, &operation->work);
> +     }
> +}
> +


If queue_work fails, you will never wake up the waiter.

> -
> -     ret = wait_for_completion_interruptible_timeout(&operation->completion,
> -                                                     timeout_jiffies);
> +     ret = wait_for_completion_interruptible(&operation->completion);

Note, that's a case I explicitly handle (failure to queue the work) in
the async set I sent out.

On gbsim at any rate, with a sufficient number of outstanding operations
in-flight it's possible to cause queue_work() to fail. Since you're in a
timer callback you're atomic so you can't call gb_operation_cancel()
here directly, those two reasons are why I did it in a work-queue as
opposed to a timer.

That's why I trapped that error at the send() stage of the operation -
because testing it out on gbsim showed messages getting lost @ the
queue_work stage.

---
bod

_______________________________________________
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Reply via email to