sched: Fix softirq time accounting
Subsystems (3)
PATCH 13/28 sched: Fix softirq time accounting
Commit: 75e1056f5c57050415b64cb761a3acc35d91f013 upstream
Author: Venkatesh Pallipadi <[email protected]>
AuthorDate: Mon Oct 4 17:03:16 2010 -0700
Peter Zijlstra found a bug in the way softirq time is accounted in VIRT_CPU_ACCOUNTING on this thread:
http://lkml.indiana.edu/hypermail//linux/kernel/1009.2/01366.html
The problem is, softirq processing uses local_bh_disable internally. There is no way, later in the flow, to differentiate between whether softirq is being processed or is it just that bh has been disabled. So, a hardirq when bh is disabled results in time being wrongly accounted as softirq.
Looking at the code a bit more, the problem exists in !VIRT_CPU_ACCOUNTING as well. As account_system_time() in normal tick based accouting also uses softirq_count, which will be set even when not in softirq with bh disabled.
Peter also suggested solution of using 2*SOFTIRQ_OFFSET as irq count for local_bh_{disable,enable} and using just SOFTIRQ_OFFSET while softirq processing. The patch below does that and adds API in_serving_softirq() which returns whether we are currently processing softirq or not.
Also changes one of the usages of softirq_count in net/sched/cls_cgroup.c to in_serving_softirq.
Looks like many usages of in_softirq really want in_serving_softirq. Those changes can be made individually on a case by case basis.
LKML-Reference: <[email protected]>
--- include/linux/hardirq.h | 5 ++++ include/linux/sched.h | 6 ++-- kernel/sched.c | 2 +- kernel/softirq.c | 51 +++++++++++++++++++++++++++++++--------------- net/sched/cls_cgroup.c | 2 +- 5 files changed, 44 insertions(+), 22 deletions(-)
include/linux/hardirq.h +5 -0
@@ -64,6 +64,8 @@#define HARDIRQ_OFFSET (1UL << HARDIRQ_SHIFT)#define NMI_OFFSET (1UL << NMI_SHIFT) +#define SOFTIRQ_DISABLE_OFFSET (2 * SOFTIRQ_OFFSET) +#ifndef PREEMPT_ACTIVE #define PREEMPT_ACTIVE_BITS 1#define PREEMPT_ACTIVE_SHIFT (NMI_SHIFT + NMI_BITS)
@@ -82,10 +84,13 @@/* * Are we doing bottom half or hardware interrupt processing? * Are we in a softirq context? Interrupt context? +* in_softirq - Are we currently processing softirq or have bh disabled? +* in_serving_softirq - Are we currently processing softirq? */ #define in_irq () (hardirq_count())#define in_softirq () (softirq_count())#define in_interrupt () (irq_count()) +#define in_serving_softirq () (softirq_count() & SOFTIRQ_OFFSET)/* * Are we in NMI context?
include/linux/sched.h +3 -3
@@ -2401,9 +2401,9 @@ extern int __cond_resched_lock(spinlock_t *lock);extern int __cond_resched_softirq (void ); -#define cond_resched_softirq () ({ \ - __might_sleep(__FILE__, __LINE__, SOFTIRQ_OFFSET); \ - __cond_resched_softirq(); \ +#define cond_resched_softirq () ({ \ + __might_sleep(__FILE__, __LINE__, SOFTIRQ_DISABLE_OFFSET); \ + __cond_resched_softirq(); \ })/*
kernel/sched.c +1 -1
@@ -5178,7 +5178,7 @@ void account_system_time(struct task_struct *p, int hardirq_offset, tmp =cputime_to_cputime64 (cputime);if (hardirq_count () - hardirq_offset) cpustat->irq =cputime64_add (cpustat->irq, tmp); -else if (softirq_count ()) +else if (in_serving_softirq ()) cpustat->softirq =cputime64_add (cpustat->softirq, tmp);else cpustat->system =cputime64_add (cpustat->system, tmp);
kernel/softirq.c +34 -17
116 lines not on this page · raw message
net/sched/cls_cgroup.c +1 -1
@@ -110,7 +110,7 @@ static int cls_cgroup_classify(struct sk_buff *skb, struct tcf_proto *tp,* calls by looking at the number of nested bh disable calls because * softirqs always disables bh. */ -if (softirq_count () !=SOFTIRQ_OFFSET ) +if (in_serving_softirq ())return -1 ;rcu_read_lock ();
Signature · 1 line
1.7.4