Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1713061 > unrolled thread

[PATCH] completion: Document that reinit_completion() must be called after complete_all()

Started bySteven Rostedt <rostedt@goodmis.org>
First post2017-08-16 17:30 +0200
Last post2017-08-16 19:00 +0200
Articles 3 — 2 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH] completion: Document that reinit_completion() must be  called after complete_all() Steven Rostedt <rostedt@goodmis.org> - 2017-08-16 17:30 +0200
    Re: [PATCH] completion: Document that reinit_completion() must be  called after complete_all() Linus Torvalds <torvalds@linux-foundation.org> - 2017-08-16 18:50 +0200
      Re: [PATCH] completion: Document that reinit_completion() must be  called after complete_all() Steven Rostedt <rostedt@goodmis.org> - 2017-08-16 19:00 +0200

#1713061 — [PATCH] completion: Document that reinit_completion() must be called after complete_all()

FromSteven Rostedt <rostedt@goodmis.org>
Date2017-08-16 17:30 +0200
Subject[PATCH] completion: Document that reinit_completion() must be called after complete_all()
Message-ID<uf8qC-ju-21@gated-at.bofh.it>
The function complete_all() modifies the completion "done" variable to
UINT_MAX, and no other caller (wait_for_completion(), etc) will modify
it back to zero. That means that any call to complete_all() must have a
reinit_completion() before that completion can be used again.

Document this fact by the complete_all() function.

Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
diff --git a/kernel/sched/completion.c b/kernel/sched/completion.c
index 13fc5ae..cc9d926 100644
--- a/kernel/sched/completion.c
+++ b/kernel/sched/completion.c
@@ -47,6 +47,9 @@ EXPORT_SYMBOL(complete);
  *
  * It may be assumed that this function implies a write memory barrier before
  * changing the task state if and only if any tasks are woken up.
+ *
+ * A call to reinit_completion() must be used on @x if it is to be used
+ * again after this call.
  */
 void complete_all(struct completion *x)
 {

[toc] | [next] | [standalone]


#1713119

FromLinus Torvalds <torvalds@linux-foundation.org>
Date2017-08-16 18:50 +0200
Message-ID<uf9G2-11c-21@gated-at.bofh.it>
In reply to#1713061
On Wed, Aug 16, 2017 at 8:27 AM, Steven Rostedt <rostedt@goodmis.org> wrote:
> The function complete_all() modifies the completion "done" variable to
> UINT_MAX, and no other caller (wait_for_completion(), etc) will modify
> it back to zero. That means that any call to complete_all() must have a
> reinit_completion() before that completion can be used again.
>
> Document this fact by the complete_all() function.

I think this is misleading.

People reading that comment will just say "why doesn't complete_all()
just reinit the thing then?"

So the comment should probably say that it needs to be reinited after
all the existing completion users have actually woken up, so that it
explains why the reinit isn't just done by complete_all().

                    Linus

[toc] | [prev] | [next] | [standalone]


#1713130

FromSteven Rostedt <rostedt@goodmis.org>
Date2017-08-16 19:00 +0200
Message-ID<uf9PI-14p-21@gated-at.bofh.it>
In reply to#1713119
On Wed, 16 Aug 2017 09:47:38 -0700
Linus Torvalds <torvalds@linux-foundation.org> wrote:

> On Wed, Aug 16, 2017 at 8:27 AM, Steven Rostedt <rostedt@goodmis.org> wrote:
> > The function complete_all() modifies the completion "done" variable to
> > UINT_MAX, and no other caller (wait_for_completion(), etc) will modify
> > it back to zero. That means that any call to complete_all() must have a
> > reinit_completion() before that completion can be used again.
> >
> > Document this fact by the complete_all() function.  
> 
> I think this is misleading.
> 
> People reading that comment will just say "why doesn't complete_all()
> just reinit the thing then?"
> 
> So the comment should probably say that it needs to be reinited after
> all the existing completion users have actually woken up, so that it
> explains why the reinit isn't just done by complete_all().
> 

Agreed. I'll send a v2.

Thanks,

-- Steve

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web