| Message ID | 20200318204407.793899611@linutronix.de (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Lock ordering documentation and annotation for lockdep | expand |
On Wed, Mar 18, 2020 at 09:43:06PM +0100, Thomas Gleixner wrote: > From: Thomas Gleixner <tglx@linutronix.de> > > 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> > Cc: Kalle Valo <kvalo@codeaurora.org> > Cc: "David S. Miller" <davem@davemloft.net> > Cc: linux-wireless@vger.kernel.org > Cc: netdev@vger.kernel.org > --- > V2: New patch to avoid conversion to swait functions later. > --- > drivers/net/wireless/intersil/orinoco/orinoco_usb.c | 21 ++++---------------- > 1 file changed, 5 insertions(+), 16 deletions(-) > > --- a/drivers/net/wireless/intersil/orinoco/orinoco_usb.c > +++ b/drivers/net/wireless/intersil/orinoco/orinoco_usb.c > @@ -365,17 +365,6 @@ static struct request_context *ezusb_all > 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); > -} That's so funny... :( Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
--- a/drivers/net/wireless/intersil/orinoco/orinoco_usb.c +++ b/drivers/net/wireless/intersil/orinoco/orinoco_usb.c @@ -365,17 +365,6 @@ static struct request_context *ezusb_all 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 re 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 re /* 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 ez * 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: