Hi Frederic
On 12/02/20 12:57, Frederic Weisbecker wrote:
> #endif /* _LINUX_KERNEL_VTIME_H */
> diff --git a/kernel/sched/cputime.c b/kernel/sched/cputime.c
> index 02163d4260d7..5f611658eeab 100644
> --- a/kernel/sched/cputime.c
> +++ b/kernel/sched/cputime.c
> @@ -44,12 +44,13 @@ static void irqtime_account_delta(struct irqtime
> *irqtime, u64 delta,
> }
>
> /*
> - * Called before incrementing preempt_count on {soft,}irq_enter
> + * Called after incrementing preempt_count on {soft,}irq_enter
> * and before decrementing preempt_count on {soft,}irq_exit.
> */
> -void irqtime_account_irq(struct task_struct *curr)
> +void irqtime_account_irq(struct task_struct *curr, unsigned int offset)
> {
> struct irqtime *irqtime = this_cpu_ptr(&cpu_irqtime);
> + unsigned int pc;
> s64 delta;
> int cpu;
>
> @@ -59,6 +60,7 @@ void irqtime_account_irq(struct task_struct *curr)
> cpu = smp_processor_id();
> delta = sched_clock_cpu(cpu) - irqtime->irq_start_time;
> irqtime->irq_start_time += delta;
> + pc = preempt_count() - offset;
>
> /*
> * We do not account for softirq time from ksoftirqd here.
> @@ -66,9 +68,9 @@ void irqtime_account_irq(struct task_struct *curr)
> * in that case, so as not to confuse scheduler with a special task
> * that do not consume any time, but still wants to run.
> */
> - if (hardirq_count())
> + if (pc & HARDIRQ_MASK)
> irqtime_account_delta(irqtime, delta, CPUTIME_IRQ);
> - else if (in_serving_softirq() && curr != this_cpu_ksoftirqd())
> + else if ((pc & SOFTIRQ_OFFSET) && curr != this_cpu_ksoftirqd())
Noob question. Why for SOFTIRQs we do sofirq_count() & *SOFTIRQ_OFFSET*? It
seems we're in-softirq only if the count is odd numbered.
/me tries to dig more
Hmm could it be because the softirq count is actually 1 bit and the rest is
for SOFTIRQ_DISABLE_OFFSET (BH disabled)?
IOW, 1 bit is for we're in softirq context, and the remaining 7 bits are to
count BH disable nesting, right?
I guess this would make sense; we don't nest softirqs processing AFAIK. But
I could be misreading the code too :-)
> irqtime_account_delta(irqtime, delta, CPUTIME_SOFTIRQ);
> }
>
> @@ -417,11 +419,13 @@ void vtime_task_switch(struct task_struct *prev)
> }
> # endif
>
> -void vtime_account_irq(struct task_struct *tsk)
> +void vtime_account_irq(struct task_struct *tsk, unsigned int offset)
> {
> - if (hardirq_count()) {
> + unsigned int pc = preempt_count() - offset;
> +
> + if (pc & HARDIRQ_OFFSET) {
Shouldn't this be HARDIRQ_MASK like above?
> vtime_account_hardirq(tsk);
> - } else if (in_serving_softirq()) {
> + } else if (pc & SOFTIRQ_OFFSET) {
> vtime_account_softirq(tsk);
> } else if (!IS_ENABLED(CONFIG_HAVE_VIRT_CPU_ACCOUNTING_IDLE) &&
> is_idle_task(tsk)) {
Thanks
--
Qais Yousef
