Hi Song, hi Kuai,
ping, Any comments?
Thx!
On Thu, Mar 7, 2024 at 1:08 PM Jack Wang <jinpu.wang@xxxxxxxxx> wrote:
>
> From: Florian-Ewald Mueller <florian-ewald.mueller@xxxxxxxxx>
>
> Replace md_thread's wait_event()/wake_up() related calls with their
> simple swait~ variants to improve performance as well as memory and
> stack footprint.
>
> Use the IDLE state for the worker thread put to sleep instead of
> misusing the INTERRUPTIBLE state combined with flushing signals
> just for not contributing to system's cpu load-average stats.
>
> Also check for sleeping thread before attempting its wake_up in
> md_wakeup_thread() for avoiding unnecessary spinlock contention.
>
> With this patch (backported) on a kernel 6.1, the IOPS improved
> by around 4% with raid1 and/or raid5, in high IO load scenarios.
>
> Fixes: 93588e2284b6 ("[PATCH] md: make md threads interruptible again")
> Signed-off-by: Florian-Ewald Mueller <florian-ewald.mueller@xxxxxxxxx>
> Signed-off-by: Jack Wang <jinpu.wang@xxxxxxxxx>
> Reviewed-by: Paul Menzel <pmenzel@xxxxxxxxxxxxx>
>
> ---
> v3: add reviewed by from Paul, add fixes tag.
> v2: fix a type error for thread
> drivers/md/md.c | 23 +++++++----------------
> drivers/md/md.h | 2 +-
> 2 files changed, 8 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 48ae2b1cb57a..14d12ee4b06b 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -7970,22 +7970,12 @@ static int md_thread(void *arg)
> * many dirty RAID5 blocks.
> */
>
> - allow_signal(SIGKILL);
> while (!kthread_should_stop()) {
>
> - /* We need to wait INTERRUPTIBLE so that
> - * we don't add to the load-average.
> - * That means we need to be sure no signals are
> - * pending
> - */
> - if (signal_pending(current))
> - flush_signals(current);
> -
> - wait_event_interruptible_timeout
> - (thread->wqueue,
> - test_bit(THREAD_WAKEUP, &thread->flags)
> - || kthread_should_stop() || kthread_should_park(),
> - thread->timeout);
> + swait_event_idle_timeout_exclusive(thread->wqueue,
> + test_bit(THREAD_WAKEUP, &thread->flags) ||
> + kthread_should_stop() || kthread_should_park(),
> + thread->timeout);
>
> clear_bit(THREAD_WAKEUP, &thread->flags);
> if (kthread_should_park())
> @@ -8017,7 +8007,8 @@ void md_wakeup_thread(struct md_thread __rcu *thread)
> if (t) {
> pr_debug("md: waking up MD thread %s.\n", t->tsk->comm);
> set_bit(THREAD_WAKEUP, &t->flags);
> - wake_up(&t->wqueue);
> + if (swq_has_sleeper(&t->wqueue))
> + swake_up_one(&t->wqueue);
> }
> rcu_read_unlock();
> }
> @@ -8032,7 +8023,7 @@ struct md_thread *md_register_thread(void (*run) (struct md_thread *),
> if (!thread)
> return NULL;
>
> - init_waitqueue_head(&thread->wqueue);
> + init_swait_queue_head(&thread->wqueue);
>
> thread->run = run;
> thread->mddev = mddev;
> diff --git a/drivers/md/md.h b/drivers/md/md.h
> index b2076a165c10..1dc01bc5c038 100644
> --- a/drivers/md/md.h
> +++ b/drivers/md/md.h
> @@ -716,7 +716,7 @@ static inline void sysfs_unlink_rdev(struct mddev *mddev, struct md_rdev *rdev)
> struct md_thread {
> void (*run) (struct md_thread *thread);
> struct mddev *mddev;
> - wait_queue_head_t wqueue;
> + struct swait_queue_head wqueue;
> unsigned long flags;
> struct task_struct *tsk;
> unsigned long timeout;
> --
> 2.34.1
>
