2008-03-09 21:54:19

by Oleg Nesterov

[permalink] [raw]
Subject: [PATCH] signals: document CLD_CONTINUED notification mechanics

A couple of small comments about how CLD_CONTINUED notification works.

Signed-off-by: Oleg Nesterov <[email protected]>

--- 25/kernel/signal.c~DOC_NOTIFY 2008-03-10 00:11:50.000000000 +0300
+++ 25/kernel/signal.c 2008-03-10 00:47:21.000000000 +0300
@@ -632,6 +632,10 @@ static int prepare_signal(int sig, struc
why |= SIGNAL_CLD_STOPPED;

if (why) {
+ /* The first thread which returns from finish_stop()
+ * will take ->siglock and notice SIGNAL_CLD_MASK,
+ * see get_signal_to_deliver().
+ */
signal->flags = why | SIGNAL_STOP_CONTINUED;
signal->group_stop_count = 0;
signal->group_exit_code = 0;
@@ -1641,7 +1645,7 @@ int get_signal_to_deliver(siginfo_t *inf

relock:
spin_lock_irq(&sighand->siglock);
-
+ /* see prepare_signal(SIGCONT) */
if (unlikely(signal->flags & SIGNAL_CLD_MASK)) {
int why = (signal->flags & SIGNAL_STOP_CONTINUED)
? CLD_CONTINUED : CLD_STOPPED;


2008-03-11 02:18:24

by Roland McGrath

[permalink] [raw]
Subject: Re: [PATCH] signals: document CLD_CONTINUED notification mechanics

> A couple of small comments about how CLD_CONTINUED notification works.

I'm always in favor of more good comments.

> if (why) {
> + /* The first thread which returns from finish_stop()

Canonical style is:

/*
* The first thread that returns from finish_stop()


Thanks,
Roland

2008-03-11 18:32:56

by Oleg Nesterov

[permalink] [raw]
Subject: Re: [PATCH] signals: document CLD_CONTINUED notification mechanics

(surprisingly, this is rather important issue for me ;)

On 03/10, Roland McGrath wrote:
>
> > if (why) {
> > + /* The first thread which returns from finish_stop()
>
> Canonical style is:
>
> /*
> * The first thread that returns from finish_stop()

Yes, I know. But the latter needs an extra line! Don't get me wrong, I am
not worrying about the result of `wc ...`, and I often add the blank lines
to improve the readability. But since the comment block itself adds the
"space", why should we lessen the number of the code lines visible to the
reader?

Oleg.

2008-03-11 20:28:33

by Roland McGrath

[permalink] [raw]
Subject: Re: [PATCH] signals: document CLD_CONTINUED notification mechanics

> Yes, I know. But the latter needs an extra line! [...]

It doesn't matter what your taste is, or mine. That's why we have
conventions. We just follow them and don't spend time grousing about
trivia. The worst thing for a reader is to have the flow broken up by
inconsistent style. New code follows the conventions. Old code
(eventually) gets fixed up to follow the conventions. End of story
(and a deathly dull one it is at that).


Thanks,
Roland