Re: [PATCH] workqueue: Add WQ_SCHED_FIFO

From: Hillf Danton <hdanton@sina.com>
To: Nathan Huckleberry <nhuck@google.com>
Cc: Gao Xiang <hsiangkao@linux.alibaba.com>,
	Lai Jiangshan <jiangshanlai@gmail.com>, Tejun Heo <tj@kernel.org>,
	Sandeep Dhavale <dhavale@google.com>,
	Daeho Jeong <daehojeong@google.com>,
	Eric Biggers <ebiggers@kernel.org>,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org
Subject: Re: [PATCH] workqueue: Add WQ_SCHED_FIFO
Date: Sun, 15 Jan 2023 09:51:59 +0800	[thread overview]
Message-ID: <20230115015159.1417-1-hdanton@sina.com> (raw)
In-Reply-To: <CAJkfWY7duk+5tWpW3g1iMyV9Q5t5cGunC-dh3M0X25wNq0z-TA@mail.gmail.com>

On Sat, 14 Jan 2023 13:00:00 -0800 Nathan Huckleberry <nhuck@google.com>
> On Fri, Jan 13, 2023 at 6:20 PM Gao Xiang <hsiangkao@linux.alibaba.com> wrote:
> >
> > Hi Nathan!
> >
> > On 2023/1/14 05:07, Nathan Huckleberry wrote:
> > > Add a WQ flag that allows workqueues to use SCHED_FIFO with the least
> > > imporant RT priority.  This can reduce scheduler latency for IO
> > > post-processing when the CPU is under load without impacting other RT
> > > workloads.  This has been shown to improve app startup time on Android
> > > [1].
> >
> > Thank you all for your effort on this.  Unfortunately I have no time to
> > setup the test [1] until now.  If it can be addressed as a new workqueue
> > feature, that would be much helpful to me.  Otherwise, I still need to
> > find a way to resolve the latest Android + EROFS latency problem.
> >
> 
> The above patch and following diff should have equivalent performance
> to [1], but I have not tested it.
> 
> diff --git a/fs/erofs/zdata.c b/fs/erofs/zdata.c
> index ccf7c55d477f..a9c3893ad1d4 100644
> --- a/fs/erofs/zdata.c
> +++ b/fs/erofs/zdata.c
> @@ -201,7 +201,7 @@ static inline int z_erofs_init_workqueue(void)
>          * scheduling overhead, perhaps per-CPU threads should be better?
>          */
>         z_erofs_workqueue = alloc_workqueue("erofs_unzipd",
> -                                           WQ_UNBOUND | WQ_HIGHPRI,
> +                                           WQ_SCHED_FIFO,
>                                             onlinecpus + onlinecpus / 4);
>         return z_erofs_workqueue ? 0 : -ENOMEM;
> 

The chart in [1] shows the change in scheduling latency that is the latency
between when the task (workqueue kworker vs kthread_worker) became eligible
to run to when it actually started running. 

+-------------------------+-----------+----------------+---------+
|                         | workqueue | kthread_worker |  diff   |
+-------------------------+-----------+----------------+---------+
| Average (us)            |     15253 |           2914 | -80.89% |
| Median (us)             |     14001 |           2912 | -79.20% |
| Minimum (us)            |      3117 |           1027 | -67.05% |
| Maximum (us)            |     30170 |           3805 | -87.39% |
| Standard deviation (us) |      7166 |            359 |         |
+-------------------------+-----------+----------------+---------+

The change in scheduling latency after this one-line patch for
workqueue workers like kblockd_workqueue and rcu_gp_wq is welcome.

> > >
> > > [1]:
> > > https://lore.kernel.org/linux-erofs/20230106073502.4017276-1-dhavale@google.com/
> > > [2]:
> > > https://lore.kernel.org/linux-f2fs-devel/20220802192437.1895492-1-daeho43@gmail.com/
> > > [3]:
> > > https://lore.kernel.org/dm-devel/20220722093823.4158756-4-nhuck@google.com/
> > > [4]:
> > > https://lore.kernel.org/dm-crypt/20200706173731.3734-1-ignat@cloudflare.com/