Skip to contentSkip to conversation

rt priority losing

RT In review Steven Rostedt 4 participants 7 messages Jul 24, 2006 – Jul 26, 2006

lore
Share & tools
lore
#0 lore raw

Ingo or Tglx,

It has come to my attention that the dynamic hrtimer softirq can lose a boosted priority. That is, if a softirq is running while a timeout happens, and the call back is of lower priority than the currently running hrtimer softirq, the timer interrupt will still lower the hrtimer softirq.

Here's the problem code:

static void wakeup_softirqd_prio(int softirq, int prio)

{
	/* Interrupts are disabled: no need to stop preemption */
	struct task_struct *tsk = __get_cpu_var(ksoftirqd[softirq].tsk);

	if (tsk) {
		if (tsk->normal_prio != prio) {
			struct sched_param param;

			param.sched_priority = MAX_RT_PRIO-1 - prio;
			setscheduler(tsk, -1, SCHED_FIFO, &param);
		}
		if(tsk->state != TASK_RUNNING)
			wake_up_process(tsk);
	}
}

So, tsk could be softirqd-hrmono and we lower the priority. (only normal_prio is checked versus prio).

So this can be a problem, if the softirq function holds a lock of a high priority task, and is running boosted. If another timer goes off with a lower priority, we can lower the priority of the softirqd and lose the inherited priority that it was running at.

-- Steve

#1 lore raw
Quoted 28 lines · Steven Rostedt

Ingo or Tglx,

It has come to my attention that the dynamic hrtimer softirq can lose a boosted priority. That is, if a softirq is running while a timeout happens, and the call back is of lower priority than the currently running hrtimer softirq, the timer interrupt will still lower the hrtimer softirq.

Here's the problem code:

static void wakeup_softirqd_prio(int softirq, int prio)

{
	/* Interrupts are disabled: no need to stop preemption */
	struct task_struct *tsk = __get_cpu_var(ksoftirqd[softirq].tsk);

	if (tsk) {
		if (tsk->normal_prio != prio) {
			struct sched_param param;

			param.sched_priority = MAX_RT_PRIO-1 - prio;
			setscheduler(tsk, -1, SCHED_FIFO, &param);
		}
		if(tsk->state != TASK_RUNNING)
			wake_up_process(tsk);
	}
}
Quoted 7 lines

So, tsk could be softirqd-hrmono and we lower the priority. (only normal_prio is checked versus prio).

So this can be a problem, if the softirq function holds a lock of a high priority task, and is running boosted. If another timer goes off with a lower priority, we can lower the priority of the softirqd and lose the inherited priority that it was running at.

There is a check for that inside setscheduler():

	p->prio = rt_mutex_getprio(p);

Esben

Quoted 9 lines

-- Steve

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

#2 lore raw
Quoted 33 lines · Esben Nielsen
Quoted 31 lines · Steven Rostedt

Ingo or Tglx,

It has come to my attention that the dynamic hrtimer softirq can lose a boosted priority. That is, if a softirq is running while a timeout happens, and the call back is of lower priority than the currently running hrtimer softirq, the timer interrupt will still lower the hrtimer softirq.

Here's the problem code:

static void wakeup_softirqd_prio(int softirq, int prio)

{
	/* Interrupts are disabled: no need to stop preemption */
	struct task_struct *tsk = __get_cpu_var(ksoftirqd[softirq].tsk);

	if (tsk) {
		if (tsk->normal_prio != prio) {
			struct sched_param param;

			param.sched_priority = MAX_RT_PRIO-1 - prio;
			setscheduler(tsk, -1, SCHED_FIFO, &param);
		}
		if(tsk->state != TASK_RUNNING)
			wake_up_process(tsk);
	}
}

So, tsk could be softirqd-hrmono and we lower the priority. (only normal_prio is checked versus prio).

Quoted 7 lines
Quoted 4 lines

So this can be a problem, if the softirq function holds a lock of a high priority task, and is running boosted. If another timer goes off with a lower priority, we can lower the priority of the softirqd and lose the inherited priority that it was running at.

There is a check for that inside setscheduler():
p->prio = rt_mutex_getprio(p);

OK, you are right about this. The PI chain should not be affected. But this could still be a problem if the softirq was running at a high prio for a task when a lower prio callback needs to be made. It looks like timer is removed from the base before the function runs. So when the interrupt looks at the base to determine the priority to set it at, it might actually lower the priority of a running hrtimer thread.

-- Steve

#3 lore raw

At Mon, 24 Jul 2006 12:37:22 -0400,

Quoted 8 lines · Steven Rostedt
Quoted 7 lines
Quoted 4 lines

So this can be a problem, if the softirq function holds a lock of a high priority task, and is running boosted. If another timer goes off with a lower priority, we can lower the priority of the softirqd and lose the inherited priority that it was running at.

There is a check for that inside setscheduler():
p->prio = rt_mutex_getprio(p);

Quoted 6 lines

OK, you are right about this. The PI chain should not be affected. But this could still be a problem if the softirq was running at a high prio for a task when a lower prio callback needs to be made. It looks like timer is removed from the base before the function runs. So when the interrupt looks at the base to determine the priority to set it at, it might actually lower the priority of a running hrtimer thread.

The correct solution is to run the callback in the context of the thread which will receive it and get rid of the softirq.

I experimented with this already, but its more than a saturday afternoon project.

tglx

#4 lore raw
In reply to Steven Rostedt
Quoted 42 lines · Steven Rostedt
Quoted 33 lines · Esben Nielsen
Quoted 31 lines · Steven Rostedt

Ingo or Tglx,

It has come to my attention that the dynamic hrtimer softirq can lose a boosted priority. That is, if a softirq is running while a timeout happens, and the call back is of lower priority than the currently running hrtimer softirq, the timer interrupt will still lower the hrtimer softirq.

Here's the problem code:

static void wakeup_softirqd_prio(int softirq, int prio)

{
	/* Interrupts are disabled: no need to stop preemption */
	struct task_struct *tsk = __get_cpu_var(ksoftirqd[softirq].tsk);

	if (tsk) {
		if (tsk->normal_prio != prio) {
			struct sched_param param;

			param.sched_priority = MAX_RT_PRIO-1 - prio;
			setscheduler(tsk, -1, SCHED_FIFO, &param);
		}
		if(tsk->state != TASK_RUNNING)
			wake_up_process(tsk);
	}
}

So, tsk could be softirqd-hrmono and we lower the priority. (only normal_prio is checked versus prio).

Quoted 7 lines
Quoted 4 lines

So this can be a problem, if the softirq function holds a lock of a high priority task, and is running boosted. If another timer goes off with a lower priority, we can lower the priority of the softirqd and lose the inherited priority that it was running at.

There is a check for that inside setscheduler():
p->prio = rt_mutex_getprio(p);

Quoted 7 lines

OK, you are right about this. The PI chain should not be affected. But this could still be a problem if the softirq was running at a high prio for a task when a lower prio callback needs to be made. It looks like timer is removed from the base before the function runs. So when the interrupt looks at the base to determine the priority to set it at, it might actually lower the priority of a running hrtimer thread.

That is a simple bug which ought to be simple fixable.

Esben

Quoted 3 lines

-- Steve

#5 lore raw

PATCH 1/1 Don't let raise_softirq_prio lower the prio (was: [RT] rt priority losing)

Quoted 10 lines · Esben Nielsen
Quoted 8 lines

OK, you are right about this. The PI chain should not be affected. But this could still be a problem if the softirq was running at a high prio for a task when a lower prio callback needs to be made. It looks like timer is removed from the base before the function runs. So when the interrupt looks at the base to determine the priority to set it at, it might actually lower the priority of a running hrtimer thread.

That is a simple bug which ought to be simple fixable.

I guess the simple fix is not to allow the interrupt to lower the priority. But simple fixes do not always handle all the cases.

Thomas G., see any side effects with this patch?

-- Steve

kernel/softirq.c +6 -1
Index: linux-2.6.17-rt7/kernel/softirq.c
===================================================================
@@ -108,7 +108,12 @@ static void wakeup_softirqd_prio(int sof
 	struct task_struct *tsk = __get_cpu_var(ksoftirqd[softirq].tsk);
 
 	if (tsk) {
-		if (tsk->normal_prio != prio) {
+		/*
+		 * The lower the prio, the higher the priority.
+		 * This can only raise the priority but it can
+		 * not lower it.
+		 */
+		if (tsk->normal_prio > prio) {
 			struct sched_param param;
 
 			param.sched_priority = MAX_RT_PRIO-1 - prio;
#6 lore raw

Re: [PATCH -rt] Don't let raise_softirq_prio lower the prio (was: [RT] rt priority losing)

Quoted 8 lines · Steven Rostedt

I guess the simple fix is not to allow the interrupt to lower the priority. But simple fixes do not always handle all the cases.

Thomas G., see any side effects with this patch?

-- Steve

thanks, applied. Thomas, fine with you too?

Ingo