orinoco_usb: Use the regular completion interfaces (9fe114ce) · Commits · 戴 / test

The completion usage in this driver is interesting: - it uses a magic complete function which according to the comment was implemented by invoking complete() four times in a row because complete_all() was not exported at that time. - it uses an open coded wait/poll which checks completion:done. Only one wait side (device removal) uses the regular wait_for_completion() interface. The rationale behind this is to prevent that wait_for_completion() consumes completion::done which would prevent that all waiters are woken. This is not necessary with complete_all() as that sets completion::done to UINT_MAX which is left unmodified by the woken waiters. Replace the magic complete function with complete_all() and convert the open coded wait/poll to regular completion interfaces. This changes the wait to exclusive wait mode. But that does not make any difference because the wakers use complete_all() which ignores the exclusive mode. Reported-by:

Sebastian Andrzej Siewior <bigeasy@linutronix.de> Signed-off-by:

Thomas Gleixner <tglx@linutronix.de> Signed-off-by:

Peter Zijlstra (Intel) <peterz@infradead.org> Reviewed-by:

Greg Kroah-Hartman <gregkh@linuxfoundation.org> Link: https://lkml.kernel.org/r/20200321113241.150783464@linutronix.de

drivers/net/wireless/intersil/orinoco/orinoco_usb.c

+5 −16

Original line number	Diff line number	Diff line
		@@ -365,17 +365,6 @@ static struct request_context ezusb_alloc_ctx(struct ezusb_priv upriv,
		return ctx;
		}


		/* Hopefully the real complete_all will soon be exported, in the mean
		* while this should work. */
		static inline void ezusb_complete_all(struct completion *comp)
		{
		complete(comp);
		complete(comp);
		complete(comp);
		complete(comp);
		}

		static void ezusb_ctx_complete(struct request_context *ctx)
		{
		struct ezusb_priv *upriv = ctx->upriv;
		@@ -409,7 +398,7 @@ static void ezusb_ctx_complete(struct request_context *ctx)

		netif_wake_queue(dev);
		}
		ezusb_complete_all(&ctx->done);
		complete_all(&ctx->done);
		ezusb_request_context_put(ctx);
		break;

		@@ -419,7 +408,7 @@ static void ezusb_ctx_complete(struct request_context *ctx)
		/* This is normal, as all request contexts get flushed
		* when the device is disconnected */
		err("Called, CTX not terminating, but device gone");
		ezusb_complete_all(&ctx->done);
		complete_all(&ctx->done);
		ezusb_request_context_put(ctx);
		break;
		}
		@@ -690,11 +679,11 @@ static void ezusb_req_ctx_wait(struct ezusb_priv *upriv,
		* get the chance to run themselves. So we make sure
		* that we don't sleep for ever */
		int msecs = DEF_TIMEOUT * (1000 / HZ);
		while (!ctx->done.done && msecs--)

		while (!try_wait_for_completion(&ctx->done) && msecs--)
		udelay(1000);
		} else {
		wait_event_interruptible(ctx->done.wait,
		ctx->done.done);
		wait_for_completion(&ctx->done);
		}
		break;
		default:

Admin message