[PATCH] kernel: refactor: shorten has_pending_signals

[PATCH] kernel: refactor: shorten has_pending_signals

[Andrea Calabrese]

In has_pending_signals there was a switch/case pattern that did not need
to be there, as the default for loop also covers those cases.
put i inside the for declaration so we do not risk its escape from the
scope. Moreover, i starts now from 0 and counts up, as it is a more
usual pattern.

Signed-off-by: Andrea Calabrese <andrea.calabrese@amarulasolutions.com>
---

This patch does not change the binary output of the file
kernel/signal.o. A diff between the files gives shows (old -> new):
2072c2072
<       case 1: ready  = signal->sig[0] &~ blocked->sig[0];
---
>               ready |= signal->sig[i] & ~blocked->sig[i];
2149c2149
<       case 1: ready  = signal->sig[0] &~ blocked->sig[0];
---
>               ready |= signal->sig[i] & ~blocked->sig[i];
2155c2155
<       case 1: ready  = signal->sig[0] &~ blocked->sig[0];
---
>               ready |= signal->sig[i] & ~blocked->sig[i];
9799c9799
<       case 1: ready  = signal->sig[0] &~ blocked->sig[0];
---
>               ready |= signal->sig[i] & ~blocked->sig[i];
9805c9805
<       case 1: ready  = signal->sig[0] &~ blocked->sig[0];
---
>               ready |= signal->sig[i] & ~blocked->sig[i];


 kernel/signal.c | 26 ++++----------------------
 1 file changed, 4 insertions(+), 22 deletions(-)

diff --git a/kernel/signal.c b/kernel/signal.c
index 2d102e025883..799ee98cf03e 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -130,28 +130,10 @@ static bool sig_ignored(struct task_struct *t, int sig, bool force)
  */
 static inline bool has_pending_signals(sigset_t *signal, sigset_t *blocked)
 {
-	unsigned long ready;
-	long i;
-
-	switch (_NSIG_WORDS) {
-	default:
-		for (i = _NSIG_WORDS, ready = 0; --i >= 0 ;)
-			ready |= signal->sig[i] &~ blocked->sig[i];
-		break;
-
-	case 4: ready  = signal->sig[3] &~ blocked->sig[3];
-		ready |= signal->sig[2] &~ blocked->sig[2];
-		ready |= signal->sig[1] &~ blocked->sig[1];
-		ready |= signal->sig[0] &~ blocked->sig[0];
-		break;
-
-	case 2: ready  = signal->sig[1] &~ blocked->sig[1];
-		ready |= signal->sig[0] &~ blocked->sig[0];
-		break;
-
-	case 1: ready  = signal->sig[0] &~ blocked->sig[0];
-	}
-	return ready !=	0;
+	unsigned long ready = 0;
+	for (long i = 0; i < _NSIG_WORDS; i++)
+		ready |= signal->sig[i] & ~blocked->sig[i];
+	return ready != 0;
 }
 
 #define PENDING(p,b) has_pending_signals(&(p)->signal, (b))
-- 
2.54.0

Re: [PATCH] kernel: refactor: shorten has_pending_signals

[Oleg Nesterov]

Hi Andrea,

thanks, but perhaps you can cleanup the changelog?

On 05/19, Andrea Calabrese wrote:
>
> In has_pending_signals there was a switch/case pattern that did not need
> to be there, as the default for loop also covers those cases.

Yes, but this code is very, very old. I agree, this optimization makes no
sense with the modern compilers, but your description doesn't look accurate.
The switch/case code was added on purpose, even if the default loop covers
those cases.

> This patch does not change the binary output of the file
> kernel/signal.o.

I guess you tested x86. Good.

> A diff between the files gives shows (old -> new):
> 2072c2072
> <       case 1: ready  = signal->sig[0] &~ blocked->sig[0];
> ---
> >               ready |= signal->sig[i] & ~blocked->sig[i];
> 2149c2149
> <       case 1: ready  = signal->sig[0] &~ blocked->sig[0];
> ---
> >               ready |= signal->sig[i] & ~blocked->sig[i];
> 2155c2155
> <       case 1: ready  = signal->sig[0] &~ blocked->sig[0];
> ---
> >               ready |= signal->sig[i] & ~blocked->sig[i];
> 9799c9799
> <       case 1: ready  = signal->sig[0] &~ blocked->sig[0];
> ---
> >               ready |= signal->sig[i] & ~blocked->sig[i];
> 9805c9805
> <       case 1: ready  = signal->sig[0] &~ blocked->sig[0];
> ---
> >               ready |= signal->sig[i] & ~blocked->sig[i];

I don't understand this part of your changelog, but I think it should
be removed anyway ;)

Oleg.