Skip to contentSkip to conversation

linux-next: Signed-off-by missing for commit in the rcu tree

DISCUSSION Stephen Rothwell 3 participants 6 messages Sep 17, 2020 – Nov 18, 2020

Subsystems (2)
Linux-next tree Scheduler
lore
Share & tools
lore

Stephen Rothwell

author
#0 lore raw

Hi all,

Commit

903c5302fa2d ("sched/core: Allow try_invoke_on_locked_down_task() with irqs disabled")

is missing a Signed-off-by from its author and committer.

I didn't complain about this when it was first present because I figured it was just a debugging commit that would be removed quickly. However, there are now quite a few follow up commits ...

Signature · 2 lines
Cheers,
Stephen Rothwell
#1 lore raw
Quoted 11 lines · Stephen Rothwell

Hi all,

Commit

903c5302fa2d ("sched/core: Allow try_invoke_on_locked_down_task() with irqs disabled")

is missing a Signed-off-by from its author and committer.

I didn't complain about this when it was first present because I figured it was just a debugging commit that would be removed quickly. However, there are now quite a few follow up commits ...

Without Peter's Signed-off-by, I clearly won't be submitting it to the upcoming merge window.

Peter, this is now quite close to your original patch. May I please add your Signed-off-by?

Thanx, Paul

author
#2 lore raw

Hi Paul,

Quoted 13 lines · On Thu, 17 Sep 2020 11:00:05 -0700 "Paul E. McKenney" <[email protected]>…
Quoted 10 lines · Stephen Rothwell

Commit

903c5302fa2d ("sched/core: Allow try_invoke_on_locked_down_task() with irqs disabled")

is missing a Signed-off-by from its author and committer.

I didn't complain about this when it was first present because I figured it was just a debugging commit that would be removed quickly. However, there are now quite a few follow up commits ...

Quoted 5 lines

Without Peter's Signed-off-by, I clearly won't be submitting it to the upcoming merge window.

Peter, this is now quite close to your original patch. May I please add your Signed-off-by?

Rebased today, but still no SOB lines.

Signature · 2 lines
Cheers,
Stephen Rothwell
maintainer
#3 lore raw
In reply to Paul E. McKenney
Quoted 13 lines · Paul E. McKenney
Quoted 11 lines · Stephen Rothwell

Hi all,

Commit

903c5302fa2d ("sched/core: Allow try_invoke_on_locked_down_task() with irqs disabled")

is missing a Signed-off-by from its author and committer.

I didn't complain about this when it was first present because I figured it was just a debugging commit that would be removed quickly. However, there are now quite a few follow up commits ...

Quoted 5 lines

Without Peter's Signed-off-by, I clearly won't be submitting it to the upcoming merge window.

Peter, this is now quite close to your original patch. May I please add your Signed-off-by?

Sure!

#4 lore raw
Quoted 14 lines · Peter Zijlstra
Quoted 13 lines · Paul E. McKenney
Quoted 11 lines · Stephen Rothwell

Hi all,

Commit

903c5302fa2d ("sched/core: Allow try_invoke_on_locked_down_task() with irqs disabled")

is missing a Signed-off-by from its author and committer.

I didn't complain about this when it was first present because I figured it was just a debugging commit that would be removed quickly. However, there are now quite a few follow up commits ...

Quoted 7 lines
Quoted 5 lines

Without Peter's Signed-off-by, I clearly won't be submitting it to the upcoming merge window.

Peter, this is now quite close to your original patch. May I please add your Signed-off-by?

Sure!

Much appreciated!!!

But I already replaced that commit with one that moves the call to try_invoke_on_locked_down_task() an interrupts-enabled region of code. Which is probably what I should have done to start with... :-/

Thanx, Paul

#5 lore raw
Quoted 21 lines · Paul E. McKenney
Quoted 20 lines · Peter Zijlstra
Quoted 18 lines · Paul E. McKenney
Quoted 11 lines · Stephen Rothwell

Hi all,

Commit

903c5302fa2d ("sched/core: Allow try_invoke_on_locked_down_task() with irqs disabled")

is missing a Signed-off-by from its author and committer.

I didn't complain about this when it was first present because I figured it was just a debugging commit that would be removed quickly. However, there are now quite a few follow up commits ...

Without Peter's Signed-off-by, I clearly won't be submitting it to the upcoming merge window.

Peter, this is now quite close to your original patch. May I please add your Signed-off-by?

Quoted 7 lines
Quoted 1 line

Sure!

Much appreciated!!!

But I already replaced that commit with one that moves the call to try_invoke_on_locked_down_task() an interrupts-enabled region of code. Which is probably what I should have done to start with... :-/

Except that Marco Elver is making that fail, so I am resuscitating your patch with your Signed-off-by, as shown below.

So once again, much appreciated!!!

Thanx, Paul

------------------------------------------------------------------------

commit 444ef3bbd0f243b912fdfd51f326704f8ee872bf

Author: Peter Zijlstra <[email protected]>
Date:   Sat Aug 29 10:22:24 2020 -0700

sched/core: Allow try_invoke_on_locked_down_task() with irqs disabled

The try_invoke_on_locked_down_task() function currently requires that interrupts be enabled, but it is called with interrupts disabled from rcu_print_task_stall(), resulting in an "IRQs not enabled as expected" diagnostic. This commit therefore updates try_invoke_on_locked_down_task() to use raw_spin_lock_irqsave() instead of raw_spin_lock_irq(), thus allowing use from either context.

Link https://lore.kernel.org/lkml/[email protected]/ open here
Link https://lore.kernel.org/lkml/[email protected]/ open here
Reported-by [email protected]
Signed-off-by Peter Zijlstra <[email protected]>
Signed-off-by Paul E. McKenney <[email protected]>
kernel/sched/core.c +4 -5
index e172f2d..09ef5cf 100644
@@ -2984,7 +2984,7 @@ try_to_wake_up(struct task_struct *p, unsigned int state, int wake_flags)
 
 /**
  * try_invoke_on_locked_down_task - Invoke a function on task in fixed state
- * @p: Process for which the function is to be invoked.
+ * @p: Process for which the function is to be invoked, can be @current.
  * @func: Function to invoke.
  * @arg: Argument to function.
  *

@@ -3002,12 +3002,11 @@ try_to_wake_up(struct task_struct *p, unsigned int state, int wake_flags)
  */
 bool try_invoke_on_locked_down_task(struct task_struct *p, bool (*func)(struct task_struct *t, void *arg), void *arg)
 {
-	bool ret = false;
 	struct rq_flags rf;
+	bool ret = false;
 	struct rq *rq;
 
-	lockdep_assert_irqs_enabled();
-	raw_spin_lock_irq(&p->pi_lock);
+	raw_spin_lock_irqsave(&p->pi_lock, rf.flags);
 	if (p->on_rq) {
 		rq = __task_rq_lock(p, &rf);
 		if (task_rq(p) == rq)

@@ -3024,7 +3023,7 @@ bool try_invoke_on_locked_down_task(struct task_struct *p, bool (*func)(struct t
 				ret = func(p, arg);
 		}
 	}
-	raw_spin_unlock_irq(&p->pi_lock);
+	raw_spin_unlock_irqrestore(&p->pi_lock, rf.flags);
 	return ret;
 }