Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Uladzislau Rezki <urezki@gmail.com>
Date:   Sat, 18 Jul 2020 00:07:35 +0200
To:     qianjun.kernel@gmail.com
Cc:     tglx@linutronix.de, peterz@infradead.org, will@kernel.org,
        luto@kernel.org, linux-kernel@vger.kernel.org,
        shaoyafang@didiglobal.com
Subject: Re: [RFC PATCH 1/1] Softirq:avoid large sched delay from the pending
 softirqs
Message-ID: <20200717220735.GA5965@pc636>
References: <1594967873-29522-1-git-send-email-qianjun.kernel@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1594967873-29522-1-git-send-email-qianjun.kernel@gmail.com>
User-Agent: Mutt/1.10.1 (2018-07-13)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

> From: jun qian <qianjun.kernel@gmail.com>
> 
> When get the pending softirqs, it need to process all the pending
> softirqs in the while loop. If the processing time of each pending
> softirq is need more than 2 msec in this loop, or one of the softirq
> will running a long time, according to the original code logic, it
> will process all the pending softirqs without wakeuping ksoftirqd,
> which will cause a relatively large scheduling delay on the
> corresponding CPU, which we do not wish to see. The patch will check
> the total time to process pending softirq, if the time exceeds 2 ms
> we need to wakeup the ksofirqd to aviod large sched delay.
> 
> Signed-off-by: jun qian <qianjun.kernel@gmail.com>
> ---
>  kernel/softirq.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/kernel/softirq.c b/kernel/softirq.c
> index c4201b7f..602d9fa 100644
> --- a/kernel/softirq.c
> +++ b/kernel/softirq.c
> @@ -299,6 +299,9 @@ asmlinkage __visible void __softirq_entry __do_softirq(void)
>  		}
>  		h++;
>  		pending >>= softirq_bit;
> +
> +		if (time_after(jiffies, end) && need_resched())
> +			break;
>  	}
>  
>  	if (__this_cpu_read(ksoftirqd) == current)
> 
I have a small concern about MAX_SOFTIRQ_TIME. The problem is that
an "end" time is based on jiffies/tick update, so it depends on CONFIG_HZ
value of your kernel.

For example if we have CONFIG_HZ=100, msecs_to_jiffies(2) will return 1.
For HZ=100 one jiffie is 10 milliseconds. So we can not rely on it,
because of low resolution.

Maybe it make sense to fix it first in order to be at least aligned with
"2 milliseconds time limit" documentation?

<snip>
 * We restart softirq processing for at most MAX_SOFTIRQ_RESTART times,
 * but break the loop if need_resched() is set or after 2 ms.
<snip>

ktime_get()/ktime_before()...?

--
Vlad Rezki