Linux filesystem development
 help / color / mirror / Atom feed
* [PATCH v2] fuse: Wake requests on the same cpu
@ 2025-10-14  9:49 Bernd Schubert
  2025-10-14 23:11 ` Joanne Koong
  0 siblings, 1 reply; 13+ messages in thread
From: Bernd Schubert @ 2025-10-14  9:49 UTC (permalink / raw)
  To: Miklos Szeredi, Ingo Molnar, Peter Zijlstra, Juri Lelli,
	Vincent Guittot, Dietmar Eggemann, Steven Rostedt, Ben Segall,
	Mel Gorman, Valentin Schneider
  Cc: Johannes Thumshirn, Joanne Koong, Luis Henriques, linux-fsdevel,
	Bernd Schubert

For io-uring it makes sense to wake the waiting application (synchronous
IO) on the same core.

With queue-per-pore

fio --directory=/tmp/dest --name=iops.\$jobnum --rw=randread --bs=4k \
    --size=1G --numjobs=1 --iodepth=1 --time_based --runtime=30s
    \ --group_reporting --ioengine=psync --direct=1

no-io-uring
   READ: bw=116MiB/s (122MB/s), 116MiB/s-116MiB/s
no-io-uring wake on the same core (not part of this patch)
   READ: bw=115MiB/s (120MB/s), 115MiB/s-115MiB/s
unpatched
   READ: bw=260MiB/s (273MB/s), 260MiB/s-260MiB/s
patched
   READ: bw=345MiB/s (362MB/s), 345MiB/s-345MiB/s

Without io-uring and core bound fuse-server queues there is almost
not difference. In fact, fio results are very fluctuating, in
between 85MB/s and 205MB/s during the run.

With --numjobs=8

unpatched
   READ: bw=2378MiB/s (2493MB/s), 2378MiB/s-2378MiB/s
patched
   READ: bw=2402MiB/s (2518MB/s), 2402MiB/s-2402MiB/s
(differences within the confidence interval)

'-o io_uring_q_mask=0-3:8-11' (16 core / 32 SMT core system) and

unpatched
   READ: bw=1286MiB/s (1348MB/s), 1286MiB/s-1286MiB/s
patched
   READ: bw=1561MiB/s (1637MB/s), 1561MiB/s-1561MiB/s

I.e. no differences with many application threads and queue-per-core,
but perf gain with overloaded queues - a bit surprising.

Signed-off-by: Bernd Schubert <bschubert@ddn.com>
---
This was already part of the RFC series and was then removed on
request to keep out optimizations from the main fuse-io-uring
series.
Later I was hesitating to add it back, as I was working on reducing the
required number of queues/rings and initially thought
wake-on-current-cpu needs to be a conditional if queue-per-core or
a reduced number of queues is used.
After testing with reduced number of queues, there is still a measurable
benefit with reduced number of queues - no condition on that needed
and the patch can be handled independently of queue size reduction.
---
Changes in v2:
- Fix the doxygen comment for __wake_up_on_current_cpu
- Move up the ' Wake up waiter sleeping in
  request_wait_answer()' comment in fuse_request_end()
- Link to v1: https://lore.kernel.org/r/20251013-wake-same-cpu-v1-1-45d8059adde7@ddn.com
---
 fs/fuse/dev.c        |  5 ++++-
 include/linux/wait.h |  6 +++---
 kernel/sched/wait.c  | 16 +++++++++++++++-
 3 files changed, 22 insertions(+), 5 deletions(-)

diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
index 132f38619d70720ce74eedc002a7b8f31e760a61..3a3d88e60e48df3ac57cff3be8df12c4f20ace9a 100644
--- a/fs/fuse/dev.c
+++ b/fs/fuse/dev.c
@@ -500,7 +500,10 @@ void fuse_request_end(struct fuse_req *req)
 		spin_unlock(&fc->bg_lock);
 	} else {
 		/* Wake up waiter sleeping in request_wait_answer() */
-		wake_up(&req->waitq);
+		if (test_bit(FR_URING, &req->flags))
+			wake_up_on_current_cpu(&req->waitq);
+		else
+			wake_up(&req->waitq);
 	}
 
 	if (test_bit(FR_ASYNC, &req->flags))
diff --git a/include/linux/wait.h b/include/linux/wait.h
index f648044466d5f55f2d65a3aa153b4dfe39f0b6dc..831a187b3f68f0707c75ceee919fec338db410b3 100644
--- a/include/linux/wait.h
+++ b/include/linux/wait.h
@@ -219,6 +219,7 @@ void __wake_up_sync(struct wait_queue_head *wq_head, unsigned int mode);
 void __wake_up_pollfree(struct wait_queue_head *wq_head);
 
 #define wake_up(x)			__wake_up(x, TASK_NORMAL, 1, NULL)
+#define wake_up_on_current_cpu(x)	__wake_up_on_current_cpu(x, TASK_NORMAL, NULL)
 #define wake_up_nr(x, nr)		__wake_up(x, TASK_NORMAL, nr, NULL)
 #define wake_up_all(x)			__wake_up(x, TASK_NORMAL, 0, NULL)
 #define wake_up_locked(x)		__wake_up_locked((x), TASK_NORMAL, 1)
@@ -479,9 +480,8 @@ do {										\
 	__wait_event_cmd(wq_head, condition, cmd1, cmd2);			\
 } while (0)
 
-#define __wait_event_interruptible(wq_head, condition)				\
-	___wait_event(wq_head, condition, TASK_INTERRUPTIBLE, 0, 0,		\
-		      schedule())
+#define __wait_event_interruptible(wq_head, condition) \
+	___wait_event(wq_head, condition, TASK_INTERRUPTIBLE, 0, 0, schedule())
 
 /**
  * wait_event_interruptible - sleep until a condition gets true
diff --git a/kernel/sched/wait.c b/kernel/sched/wait.c
index 20f27e2cf7aec691af040fcf2236a20374ec66bf..94120076bc1ae465735843cc5821ca532d9c398a 100644
--- a/kernel/sched/wait.c
+++ b/kernel/sched/wait.c
@@ -147,10 +147,24 @@ int __wake_up(struct wait_queue_head *wq_head, unsigned int mode,
 }
 EXPORT_SYMBOL(__wake_up);
 
-void __wake_up_on_current_cpu(struct wait_queue_head *wq_head, unsigned int mode, void *key)
+/**
+ * __wake_up_on_current_cpu - wake up threads blocked on a waitqueue, on the
+ * current cpu
+ * @wq_head: the waitqueue
+ * @mode: which threads
+ * @nr_exclusive: how many wake-one or wake-many threads to wake up
+ * @key: is directly passed to the wakeup function
+ *
+ * If this function wakes up a task, it executes a full memory barrier
+ * before accessing the task state.  Returns the number of exclusive
+ * tasks that were awaken.
+ */
+void __wake_up_on_current_cpu(struct wait_queue_head *wq_head,
+			      unsigned int mode, void *key)
 {
 	__wake_up_common_lock(wq_head, mode, 1, WF_CURRENT_CPU, key);
 }
+EXPORT_SYMBOL_GPL(__wake_up_on_current_cpu);
 
 /*
  * Same as __wake_up but called with the spinlock in wait_queue_head_t held.

---
base-commit: ec714e371f22f716a04e6ecb2a24988c92b26911
change-id: 20251013-wake-same-cpu-b7ddb0b0688e

Best regards,
-- 
Bernd Schubert <bschubert@ddn.com>


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH v2] fuse: Wake requests on the same cpu
  2025-10-14  9:49 [PATCH v2] fuse: Wake requests on the same cpu Bernd Schubert
@ 2025-10-14 23:11 ` Joanne Koong
  2025-10-15 15:30   ` Bernd Schubert
  0 siblings, 1 reply; 13+ messages in thread
From: Joanne Koong @ 2025-10-14 23:11 UTC (permalink / raw)
  To: Bernd Schubert
  Cc: Miklos Szeredi, Ingo Molnar, Peter Zijlstra, Juri Lelli,
	Vincent Guittot, Dietmar Eggemann, Steven Rostedt, Ben Segall,
	Mel Gorman, Valentin Schneider, Johannes Thumshirn,
	Luis Henriques, linux-fsdevel

On Tue, Oct 14, 2025 at 2:50 AM Bernd Schubert <bschubert@ddn.com> wrote:
>
> For io-uring it makes sense to wake the waiting application (synchronous
> IO) on the same core.
>
> With queue-per-pore

nit typo: core, not pore

>
> fio --directory=/tmp/dest --name=iops.\$jobnum --rw=randread --bs=4k \
>     --size=1G --numjobs=1 --iodepth=1 --time_based --runtime=30s
>     \ --group_reporting --ioengine=psync --direct=1
>

Which server are you using for these benchmarks? passthrough_hp?

> no-io-uring
>    READ: bw=116MiB/s (122MB/s), 116MiB/s-116MiB/s
> no-io-uring wake on the same core (not part of this patch)
>    READ: bw=115MiB/s (120MB/s), 115MiB/s-115MiB/s
> unpatched
>    READ: bw=260MiB/s (273MB/s), 260MiB/s-260MiB/s
> patched
>    READ: bw=345MiB/s (362MB/s), 345MiB/s-345MiB/s
>
> Without io-uring and core bound fuse-server queues there is almost
> not difference. In fact, fio results are very fluctuating, in
> between 85MB/s and 205MB/s during the run.
>
> With --numjobs=8
>
> unpatched
>    READ: bw=2378MiB/s (2493MB/s), 2378MiB/s-2378MiB/s
> patched
>    READ: bw=2402MiB/s (2518MB/s), 2402MiB/s-2402MiB/s
> (differences within the confidence interval)
>
> '-o io_uring_q_mask=0-3:8-11' (16 core / 32 SMT core system) and
>
> unpatched
>    READ: bw=1286MiB/s (1348MB/s), 1286MiB/s-1286MiB/s
> patched
>    READ: bw=1561MiB/s (1637MB/s), 1561MiB/s-1561MiB/s
>
> I.e. no differences with many application threads and queue-per-core,
> but perf gain with overloaded queues - a bit surprising.
>
> Signed-off-by: Bernd Schubert <bschubert@ddn.com>
> ---
> This was already part of the RFC series and was then removed on
> request to keep out optimizations from the main fuse-io-uring
> series.
> Later I was hesitating to add it back, as I was working on reducing the
> required number of queues/rings and initially thought
> wake-on-current-cpu needs to be a conditional if queue-per-core or
> a reduced number of queues is used.
> After testing with reduced number of queues, there is still a measurable
> benefit with reduced number of queues - no condition on that needed
> and the patch can be handled independently of queue size reduction.
> ---
> Changes in v2:
> - Fix the doxygen comment for __wake_up_on_current_cpu
> - Move up the ' Wake up waiter sleeping in
>   request_wait_answer()' comment in fuse_request_end()
> - Link to v1: https://lore.kernel.org/r/20251013-wake-same-cpu-v1-1-45d8059adde7@ddn.com
> ---
>  fs/fuse/dev.c        |  5 ++++-
>  include/linux/wait.h |  6 +++---
>  kernel/sched/wait.c  | 16 +++++++++++++++-
>  3 files changed, 22 insertions(+), 5 deletions(-)
>
> diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
> index 132f38619d70720ce74eedc002a7b8f31e760a61..3a3d88e60e48df3ac57cff3be8df12c4f20ace9a 100644
> --- a/fs/fuse/dev.c
> +++ b/fs/fuse/dev.c
> @@ -500,7 +500,10 @@ void fuse_request_end(struct fuse_req *req)
>                 spin_unlock(&fc->bg_lock);
>         } else {
>                 /* Wake up waiter sleeping in request_wait_answer() */
> -               wake_up(&req->waitq);
> +               if (test_bit(FR_URING, &req->flags))

might be worth having a separate helper for this since this is also
called in request_wait_answer()

> +                       wake_up_on_current_cpu(&req->waitq);

Won't this lose cache locality for all the other data that is in the
client thread's cache on the previous CPU? It seems to me like on
average this would be a costlier miss overall? What are your thoughts
on this?

> +               else
> +                       wake_up(&req->waitq);
>         }
>
>         if (test_bit(FR_ASYNC, &req->flags))
> diff --git a/include/linux/wait.h b/include/linux/wait.h
> index f648044466d5f55f2d65a3aa153b4dfe39f0b6dc..831a187b3f68f0707c75ceee919fec338db410b3 100644
> --- a/include/linux/wait.h
> +++ b/include/linux/wait.h
> @@ -219,6 +219,7 @@ void __wake_up_sync(struct wait_queue_head *wq_head, unsigned int mode);
>  void __wake_up_pollfree(struct wait_queue_head *wq_head);
>
>  #define wake_up(x)                     __wake_up(x, TASK_NORMAL, 1, NULL)
> +#define wake_up_on_current_cpu(x)      __wake_up_on_current_cpu(x, TASK_NORMAL, NULL)
>  #define wake_up_nr(x, nr)              __wake_up(x, TASK_NORMAL, nr, NULL)
>  #define wake_up_all(x)                 __wake_up(x, TASK_NORMAL, 0, NULL)
>  #define wake_up_locked(x)              __wake_up_locked((x), TASK_NORMAL, 1)
> @@ -479,9 +480,8 @@ do {                                                                                \
>         __wait_event_cmd(wq_head, condition, cmd1, cmd2);                       \
>  } while (0)
>
> -#define __wait_event_interruptible(wq_head, condition)                         \
> -       ___wait_event(wq_head, condition, TASK_INTERRUPTIBLE, 0, 0,             \
> -                     schedule())
> +#define __wait_event_interruptible(wq_head, condition) \
> +       ___wait_event(wq_head, condition, TASK_INTERRUPTIBLE, 0, 0, schedule())
>
>  /**
>   * wait_event_interruptible - sleep until a condition gets true
> diff --git a/kernel/sched/wait.c b/kernel/sched/wait.c
> index 20f27e2cf7aec691af040fcf2236a20374ec66bf..94120076bc1ae465735843cc5821ca532d9c398a 100644
> --- a/kernel/sched/wait.c
> +++ b/kernel/sched/wait.c
> @@ -147,10 +147,24 @@ int __wake_up(struct wait_queue_head *wq_head, unsigned int mode,
>  }
>  EXPORT_SYMBOL(__wake_up);
>
> -void __wake_up_on_current_cpu(struct wait_queue_head *wq_head, unsigned int mode, void *key)
> +/**
> + * __wake_up_on_current_cpu - wake up threads blocked on a waitqueue, on the
> + * current cpu
> + * @wq_head: the waitqueue
> + * @mode: which threads
> + * @nr_exclusive: how many wake-one or wake-many threads to wake up

I don't think you meant to include this line?

> + * @key: is directly passed to the wakeup function
> + *
> + * If this function wakes up a task, it executes a full memory barrier
> + * before accessing the task state.  Returns the number of exclusive
> + * tasks that were awaken.

Doesn't this return a void?

Thanks,
Joanne

> + */
> +void __wake_up_on_current_cpu(struct wait_queue_head *wq_head,
> +                             unsigned int mode, void *key)
>  {
>         __wake_up_common_lock(wq_head, mode, 1, WF_CURRENT_CPU, key);
>  }
> +EXPORT_SYMBOL_GPL(__wake_up_on_current_cpu);
>
>  /*
>   * Same as __wake_up but called with the spinlock in wait_queue_head_t held.
>
> ---
> base-commit: ec714e371f22f716a04e6ecb2a24988c92b26911
> change-id: 20251013-wake-same-cpu-b7ddb0b0688e
>
> Best regards,
> --
> Bernd Schubert <bschubert@ddn.com>
>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2] fuse: Wake requests on the same cpu
  2025-10-14 23:11 ` Joanne Koong
@ 2025-10-15 15:30   ` Bernd Schubert
  2025-10-15 16:36     ` Bernd Schubert
  2025-10-15 22:19     ` Joanne Koong
  0 siblings, 2 replies; 13+ messages in thread
From: Bernd Schubert @ 2025-10-15 15:30 UTC (permalink / raw)
  To: Joanne Koong, Bernd Schubert
  Cc: Miklos Szeredi, Ingo Molnar, Peter Zijlstra, Juri Lelli,
	Vincent Guittot, Dietmar Eggemann, Steven Rostedt, Ben Segall,
	Mel Gorman, Valentin Schneider, Johannes Thumshirn,
	Luis Henriques, linux-fsdevel



On 10/15/25 01:11, Joanne Koong wrote:
> On Tue, Oct 14, 2025 at 2:50 AM Bernd Schubert <bschubert@ddn.com> wrote:
>>
>> For io-uring it makes sense to wake the waiting application (synchronous
>> IO) on the same core.
>>
>> With queue-per-pore
> 
> nit typo: core, not pore

:) Thanks, dunno how I managed to get that.

> 
>>
>> fio --directory=/tmp/dest --name=iops.\$jobnum --rw=randread --bs=4k \
>>      --size=1G --numjobs=1 --iodepth=1 --time_based --runtime=30s
>>      \ --group_reporting --ioengine=psync --direct=1
>>
> 
> Which server are you using for these benchmarks? passthrough_hp?

passthrough_hp on tmpfs, system has 256GB RAM - enough for these benchmarks, with 16 (32 HT) cores.

> 
>> no-io-uring
>>     READ: bw=116MiB/s (122MB/s), 116MiB/s-116MiB/s
>> no-io-uring wake on the same core (not part of this patch)
>>     READ: bw=115MiB/s (120MB/s), 115MiB/s-115MiB/s
>> unpatched
>>     READ: bw=260MiB/s (273MB/s), 260MiB/s-260MiB/s
>> patched
>>     READ: bw=345MiB/s (362MB/s), 345MiB/s-345MiB/s
>>
>> Without io-uring and core bound fuse-server queues there is almost
>> not difference. In fact, fio results are very fluctuating, in
>> between 85MB/s and 205MB/s during the run.
>>
>> With --numjobs=8
>>
>> unpatched
>>     READ: bw=2378MiB/s (2493MB/s), 2378MiB/s-2378MiB/s
>> patched
>>     READ: bw=2402MiB/s (2518MB/s), 2402MiB/s-2402MiB/s
>> (differences within the confidence interval)
>>
>> '-o io_uring_q_mask=0-3:8-11' (16 core / 32 SMT core system) and
>>
>> unpatched
>>     READ: bw=1286MiB/s (1348MB/s), 1286MiB/s-1286MiB/s
>> patched
>>     READ: bw=1561MiB/s (1637MB/s), 1561MiB/s-1561MiB/s
>>
>> I.e. no differences with many application threads and queue-per-core,
>> but perf gain with overloaded queues - a bit surprising.
>>
>> Signed-off-by: Bernd Schubert <bschubert@ddn.com>
>> ---
>> This was already part of the RFC series and was then removed on
>> request to keep out optimizations from the main fuse-io-uring
>> series.
>> Later I was hesitating to add it back, as I was working on reducing the
>> required number of queues/rings and initially thought
>> wake-on-current-cpu needs to be a conditional if queue-per-core or
>> a reduced number of queues is used.
>> After testing with reduced number of queues, there is still a measurable
>> benefit with reduced number of queues - no condition on that needed
>> and the patch can be handled independently of queue size reduction.
>> ---
>> Changes in v2:
>> - Fix the doxygen comment for __wake_up_on_current_cpu
>> - Move up the ' Wake up waiter sleeping in
>>    request_wait_answer()' comment in fuse_request_end()
>> - Link to v1: https://lore.kernel.org/r/20251013-wake-same-cpu-v1-1-45d8059adde7@ddn.com
>> ---
>>   fs/fuse/dev.c        |  5 ++++-
>>   include/linux/wait.h |  6 +++---
>>   kernel/sched/wait.c  | 16 +++++++++++++++-
>>   3 files changed, 22 insertions(+), 5 deletions(-)
>>
>> diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
>> index 132f38619d70720ce74eedc002a7b8f31e760a61..3a3d88e60e48df3ac57cff3be8df12c4f20ace9a 100644
>> --- a/fs/fuse/dev.c
>> +++ b/fs/fuse/dev.c
>> @@ -500,7 +500,10 @@ void fuse_request_end(struct fuse_req *req)
>>                  spin_unlock(&fc->bg_lock);
>>          } else {
>>                  /* Wake up waiter sleeping in request_wait_answer() */
>> -               wake_up(&req->waitq);
>> +               if (test_bit(FR_URING, &req->flags))
> 
> might be worth having a separate helper for this since this is also
> called in request_wait_answer()

Ok, I can do that in v3

> 
>> +                       wake_up_on_current_cpu(&req->waitq);
> 
> Won't this lose cache locality for all the other data that is in the
> client thread's cache on the previous CPU? It seems to me like on
> average this would be a costlier miss overall? What are your thoughts
> on this?

So as in the introduction, which b4 made a '---' comment below,
initially I thought this should be a conditional on queue-per-core.
With queue-per-core it should be easy to explain, I think.

App submits request on core-X, waits/sleeps, request gets handle on
core-X by queue-X.
If there are more applications running on this core, they
get likely re-scheduled to another core, as the libfuse queue thread is
core bound. If other applications don't get re-scheduled either the
entire system is overloaded or someone sets manual application core
affinity - we can't do much about that in either case. With
queue-per-core there is also no debate about "previous CPU".
Worse is actually scheduler behavior here, although the ring thread
itself goes to sleep soon enough. Application gets still quite often
re-scheduled to another core. Without wake-on-same core behavior is
even worse and it jumps across all the time. Not good for CPU cache...

With reduced queues we can assume that it to jump between cores, I
have no problem to make it a conditional on that, just results are
encouraging to apply it unconditionally - see the results above for
"-o io_uring_q_mask=0-3:8-11' (16 core / 32 SMT core system)"





> 
>> +               else
>> +                       wake_up(&req->waitq);
>>          }
>>
>>          if (test_bit(FR_ASYNC, &req->flags))
>> diff --git a/include/linux/wait.h b/include/linux/wait.h
>> index f648044466d5f55f2d65a3aa153b4dfe39f0b6dc..831a187b3f68f0707c75ceee919fec338db410b3 100644
>> --- a/include/linux/wait.h
>> +++ b/include/linux/wait.h
>> @@ -219,6 +219,7 @@ void __wake_up_sync(struct wait_queue_head *wq_head, unsigned int mode);
>>   void __wake_up_pollfree(struct wait_queue_head *wq_head);
>>
>>   #define wake_up(x)                     __wake_up(x, TASK_NORMAL, 1, NULL)
>> +#define wake_up_on_current_cpu(x)      __wake_up_on_current_cpu(x, TASK_NORMAL, NULL)
>>   #define wake_up_nr(x, nr)              __wake_up(x, TASK_NORMAL, nr, NULL)
>>   #define wake_up_all(x)                 __wake_up(x, TASK_NORMAL, 0, NULL)
>>   #define wake_up_locked(x)              __wake_up_locked((x), TASK_NORMAL, 1)
>> @@ -479,9 +480,8 @@ do {                                                                                \
>>          __wait_event_cmd(wq_head, condition, cmd1, cmd2);                       \
>>   } while (0)
>>
>> -#define __wait_event_interruptible(wq_head, condition)                         \
>> -       ___wait_event(wq_head, condition, TASK_INTERRUPTIBLE, 0, 0,             \
>> -                     schedule())
>> +#define __wait_event_interruptible(wq_head, condition) \
>> +       ___wait_event(wq_head, condition, TASK_INTERRUPTIBLE, 0, 0, schedule())
>>
>>   /**
>>    * wait_event_interruptible - sleep until a condition gets true
>> diff --git a/kernel/sched/wait.c b/kernel/sched/wait.c
>> index 20f27e2cf7aec691af040fcf2236a20374ec66bf..94120076bc1ae465735843cc5821ca532d9c398a 100644
>> --- a/kernel/sched/wait.c
>> +++ b/kernel/sched/wait.c
>> @@ -147,10 +147,24 @@ int __wake_up(struct wait_queue_head *wq_head, unsigned int mode,
>>   }
>>   EXPORT_SYMBOL(__wake_up);
>>
>> -void __wake_up_on_current_cpu(struct wait_queue_head *wq_head, unsigned int mode, void *key)
>> +/**
>> + * __wake_up_on_current_cpu - wake up threads blocked on a waitqueue, on the
>> + * current cpu
>> + * @wq_head: the waitqueue
>> + * @mode: which threads
>> + * @nr_exclusive: how many wake-one or wake-many threads to wake up
> 
> I don't think you meant to include this line?

Yeah, the entire comment is broken :( Sorry about that.

> 
>> + * @key: is directly passed to the wakeup function
>> + *
>> + * If this function wakes up a task, it executes a full memory barrier
>> + * before accessing the task state.  Returns the number of exclusive
>> + * tasks that were awaken.
> 
> Doesn't this return a void?
> 

Yeah, I promise I triple check next time when I copy and paste comments.


Thanks,
Bernd

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2] fuse: Wake requests on the same cpu
  2025-10-15 15:30   ` Bernd Schubert
@ 2025-10-15 16:36     ` Bernd Schubert
  2025-10-15 22:19     ` Joanne Koong
  1 sibling, 0 replies; 13+ messages in thread
From: Bernd Schubert @ 2025-10-15 16:36 UTC (permalink / raw)
  To: Bernd Schubert, Joanne Koong
  Cc: Miklos Szeredi, Ingo Molnar, Peter Zijlstra, Juri Lelli,
	Vincent Guittot, Dietmar Eggemann, Steven Rostedt, Ben Segall,
	Mel Gorman, Valentin Schneider, Johannes Thumshirn,
	Luis Henriques, linux-fsdevel@vger.kernel.org

On 10/15/25 17:30, Bernd Schubert wrote:
> 
> 
> On 10/15/25 01:11, Joanne Koong wrote:
>> On Tue, Oct 14, 2025 at 2:50 AM Bernd Schubert <bschubert@ddn.com> wrote:
>>>
>>> For io-uring it makes sense to wake the waiting application (synchronous
>>> IO) on the same core.
>>>
>>> With queue-per-pore
>>
>> nit typo: core, not pore
> 
> :) Thanks, dunno how I managed to get that.
> 
>>
>>>
>>> fio --directory=/tmp/dest --name=iops.\$jobnum --rw=randread --bs=4k \
>>>      --size=1G --numjobs=1 --iodepth=1 --time_based --runtime=30s
>>>      \ --group_reporting --ioengine=psync --direct=1
>>>
>>
>> Which server are you using for these benchmarks? passthrough_hp?
> 
> passthrough_hp on tmpfs, system has 256GB RAM - enough for these 
> benchmarks, with 16 (32 HT) cores.
> 
>>
>>> no-io-uring
>>>     READ: bw=116MiB/s (122MB/s), 116MiB/s-116MiB/s
>>> no-io-uring wake on the same core (not part of this patch)
>>>     READ: bw=115MiB/s (120MB/s), 115MiB/s-115MiB/s
>>> unpatched
>>>     READ: bw=260MiB/s (273MB/s), 260MiB/s-260MiB/s
>>> patched
>>>     READ: bw=345MiB/s (362MB/s), 345MiB/s-345MiB/s
>>>
>>> Without io-uring and core bound fuse-server queues there is almost
>>> not difference. In fact, fio results are very fluctuating, in
>>> between 85MB/s and 205MB/s during the run.
>>>
>>> With --numjobs=8
>>>
>>> unpatched
>>>     READ: bw=2378MiB/s (2493MB/s), 2378MiB/s-2378MiB/s
>>> patched
>>>     READ: bw=2402MiB/s (2518MB/s), 2402MiB/s-2402MiB/s
>>> (differences within the confidence interval)
>>>
>>> '-o io_uring_q_mask=0-3:8-11' (16 core / 32 SMT core system) and
>>>
>>> unpatched
>>>     READ: bw=1286MiB/s (1348MB/s), 1286MiB/s-1286MiB/s
>>> patched
>>>     READ: bw=1561MiB/s (1637MB/s), 1561MiB/s-1561MiB/s
>>>
>>> I.e. no differences with many application threads and queue-per-core,
>>> but perf gain with overloaded queues - a bit surprising.
>>>
>>> Signed-off-by: Bernd Schubert <bschubert@ddn.com>
>>> ---
>>> This was already part of the RFC series and was then removed on
>>> request to keep out optimizations from the main fuse-io-uring
>>> series.
>>> Later I was hesitating to add it back, as I was working on reducing the
>>> required number of queues/rings and initially thought
>>> wake-on-current-cpu needs to be a conditional if queue-per-core or
>>> a reduced number of queues is used.
>>> After testing with reduced number of queues, there is still a measurable
>>> benefit with reduced number of queues - no condition on that needed
>>> and the patch can be handled independently of queue size reduction.
>>> ---
>>> Changes in v2:
>>> - Fix the doxygen comment for __wake_up_on_current_cpu
>>> - Move up the ' Wake up waiter sleeping in
>>>    request_wait_answer()' comment in fuse_request_end()
>>> - Link to v1: https://lore.kernel.org/r/20251013-wake-same-cpu- 
>>> v1-1-45d8059adde7@ddn.com
>>> ---
>>>   fs/fuse/dev.c        |  5 ++++-
>>>   include/linux/wait.h |  6 +++---
>>>   kernel/sched/wait.c  | 16 +++++++++++++++-
>>>   3 files changed, 22 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
>>> index 
>>> 132f38619d70720ce74eedc002a7b8f31e760a61..3a3d88e60e48df3ac57cff3be8df12c4f20ace9a 100644
>>> --- a/fs/fuse/dev.c
>>> +++ b/fs/fuse/dev.c
>>> @@ -500,7 +500,10 @@ void fuse_request_end(struct fuse_req *req)
>>>                  spin_unlock(&fc->bg_lock);
>>>          } else {
>>>                  /* Wake up waiter sleeping in request_wait_answer() */
>>> -               wake_up(&req->waitq);
>>> +               if (test_bit(FR_URING, &req->flags))
>>
>> might be worth having a separate helper for this since this is also
>> called in request_wait_answer()
> 
> Ok, I can do that in v3
> 
>>
>>> +                       wake_up_on_current_cpu(&req->waitq);
>>
>> Won't this lose cache locality for all the other data that is in the
>> client thread's cache on the previous CPU? It seems to me like on
>> average this would be a costlier miss overall? What are your thoughts
>> on this?
> 
> So as in the introduction, which b4 made a '---' comment below,
> initially I thought this should be a conditional on queue-per-core.
> With queue-per-core it should be easy to explain, I think.
> 
> App submits request on core-X, waits/sleeps, request gets handle on
> core-X by queue-X.
> If there are more applications running on this core, they
> get likely re-scheduled to another core, as the libfuse queue thread is
> core bound. If other applications don't get re-scheduled either the
> entire system is overloaded or someone sets manual application core
> affinity - we can't do much about that in either case. With
> queue-per-core there is also no debate about "previous CPU".
> Worse is actually scheduler behavior here, although the ring thread
> itself goes to sleep soon enough. Application gets still quite often
> re-scheduled to another core. Without wake-on-same core behavior is
> even worse and it jumps across all the time. Not good for CPU cache...
> 
> With reduced queues we can assume that it to jump between cores, I
> have no problem to make it a conditional on that, just results are
> encouraging to apply it unconditionally - see the results above for
> "-o io_uring_q_mask=0-3:8-11' (16 core / 32 SMT core system)"
> 

Isn't what you are suggesting a function called "wake_on_prev_cpu"?

Thanks,
Bernd

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2] fuse: Wake requests on the same cpu
  2025-10-15 15:30   ` Bernd Schubert
  2025-10-15 16:36     ` Bernd Schubert
@ 2025-10-15 22:19     ` Joanne Koong
  2025-10-16  8:58       ` Peter Zijlstra
  1 sibling, 1 reply; 13+ messages in thread
From: Joanne Koong @ 2025-10-15 22:19 UTC (permalink / raw)
  To: Bernd Schubert
  Cc: Bernd Schubert, Miklos Szeredi, Ingo Molnar, Peter Zijlstra,
	Juri Lelli, Vincent Guittot, Dietmar Eggemann, Steven Rostedt,
	Ben Segall, Mel Gorman, Valentin Schneider, Johannes Thumshirn,
	Luis Henriques, linux-fsdevel

On Wed, Oct 15, 2025 at 8:30 AM Bernd Schubert <bernd@bsbernd.com> wrote:
>
> On 10/15/25 01:11, Joanne Koong wrote:
> > On Tue, Oct 14, 2025 at 2:50 AM Bernd Schubert <bschubert@ddn.com> wrote:
> >>
> >> For io-uring it makes sense to wake the waiting application (synchronous
> >> IO) on the same core.
> >>
> >> With queue-per-pore
> >
> > nit typo: core, not pore
>
> :) Thanks, dunno how I managed to get that.
>
> >
> >>
> >> fio --directory=/tmp/dest --name=iops.\$jobnum --rw=randread --bs=4k \
> >>      --size=1G --numjobs=1 --iodepth=1 --time_based --runtime=30s
> >>      \ --group_reporting --ioengine=psync --direct=1
> >>
> >
> > Which server are you using for these benchmarks? passthrough_hp?
>
> passthrough_hp on tmpfs, system has 256GB RAM - enough for these benchmarks, with 16 (32 HT) cores.
>

Thanks!

> >
> >> no-io-uring
> >>     READ: bw=116MiB/s (122MB/s), 116MiB/s-116MiB/s
> >> no-io-uring wake on the same core (not part of this patch)
> >>     READ: bw=115MiB/s (120MB/s), 115MiB/s-115MiB/s
> >> unpatched
> >>     READ: bw=260MiB/s (273MB/s), 260MiB/s-260MiB/s
> >> patched
> >>     READ: bw=345MiB/s (362MB/s), 345MiB/s-345MiB/s
> >>
> >> Without io-uring and core bound fuse-server queues there is almost
> >> not difference. In fact, fio results are very fluctuating, in
> >> between 85MB/s and 205MB/s during the run.
> >>
> >> With --numjobs=8
> >>
> >> unpatched
> >>     READ: bw=2378MiB/s (2493MB/s), 2378MiB/s-2378MiB/s
> >> patched
> >>     READ: bw=2402MiB/s (2518MB/s), 2402MiB/s-2402MiB/s
> >> (differences within the confidence interval)
> >>
> >> '-o io_uring_q_mask=0-3:8-11' (16 core / 32 SMT core system) and
> >>
> >> unpatched
> >>     READ: bw=1286MiB/s (1348MB/s), 1286MiB/s-1286MiB/s
> >> patched
> >>     READ: bw=1561MiB/s (1637MB/s), 1561MiB/s-1561MiB/s
> >>
> >> I.e. no differences with many application threads and queue-per-core,
> >> but perf gain with overloaded queues - a bit surprising.
> >>
> >> Signed-off-by: Bernd Schubert <bschubert@ddn.com>
> >> ---
> >> This was already part of the RFC series and was then removed on
> >> request to keep out optimizations from the main fuse-io-uring
> >> series.
> >> Later I was hesitating to add it back, as I was working on reducing the
> >> required number of queues/rings and initially thought
> >> wake-on-current-cpu needs to be a conditional if queue-per-core or
> >> a reduced number of queues is used.
> >> After testing with reduced number of queues, there is still a measurable
> >> benefit with reduced number of queues - no condition on that needed
> >> and the patch can be handled independently of queue size reduction.
> >> ---
> >> Changes in v2:
> >> - Fix the doxygen comment for __wake_up_on_current_cpu
> >> - Move up the ' Wake up waiter sleeping in
> >>    request_wait_answer()' comment in fuse_request_end()
> >> - Link to v1: https://lore.kernel.org/r/20251013-wake-same-cpu-v1-1-45d8059adde7@ddn.com
> >> ---
> >>   fs/fuse/dev.c        |  5 ++++-
> >>   include/linux/wait.h |  6 +++---
> >>   kernel/sched/wait.c  | 16 +++++++++++++++-
> >>   3 files changed, 22 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
> >> index 132f38619d70720ce74eedc002a7b8f31e760a61..3a3d88e60e48df3ac57cff3be8df12c4f20ace9a 100644
> >> --- a/fs/fuse/dev.c
> >> +++ b/fs/fuse/dev.c
> >> @@ -500,7 +500,10 @@ void fuse_request_end(struct fuse_req *req)
> >>                  spin_unlock(&fc->bg_lock);
> >>          } else {
> >>                  /* Wake up waiter sleeping in request_wait_answer() */
> >> -               wake_up(&req->waitq);
> >> +               if (test_bit(FR_URING, &req->flags))
> >
> > might be worth having a separate helper for this since this is also
> > called in request_wait_answer()
>
> Ok, I can do that in v3

That could also be part of a different patchset. Sorry, didn't mean to
imply that it should be necessary for this one.
>
> >
> >> +                       wake_up_on_current_cpu(&req->waitq);
> >
> > Won't this lose cache locality for all the other data that is in the
> > client thread's cache on the previous CPU? It seems to me like on
> > average this would be a costlier miss overall? What are your thoughts
> > on this?
>
> So as in the introduction, which b4 made a '---' comment below,
> initially I thought this should be a conditional on queue-per-core.
> With queue-per-core it should be easy to explain, I think.
>
> App submits request on core-X, waits/sleeps, request gets handle on
> core-X by queue-X.
> If there are more applications running on this core, they
> get likely re-scheduled to another core, as the libfuse queue thread is
> core bound. If other applications don't get re-scheduled either the
> entire system is overloaded or someone sets manual application core
> affinity - we can't do much about that in either case. With
> queue-per-core there is also no debate about "previous CPU".
> Worse is actually scheduler behavior here, although the ring thread
> itself goes to sleep soon enough. Application gets still quite often
> re-scheduled to another core. Without wake-on-same core behavior is
> even worse and it jumps across all the time. Not good for CPU cache...

Maybe this is a lack of my understanding of scheduler internals,  but
I'm having a hard time seeing what the benefit of
wake_up_on_current_cpu() is over wake_up() for the queue-per-core
case.

As I understand it, with wake_up() the scheduler already will try to
wake up the thread and put it back on the same core to maintain cache
locality, which in this case is the same core
"wake_up_on_current_cpu()" is trying to put it on. If there's too much
load imbalance then regardless of whether you call wake_up() or
wake_up_on_current_cpu(), the scheduler will migrate the task to
whatever other core is better for it.

So I guess the main benefit of calling wake_up_on_current_cpu() over
wake_up() is that for situations where there is only some but not too
much load imbalance we force the application to run on the current
core even despite the scheduler thinking it's better for overall
system health to distribute the load? I don't see an issue if the
application thread runs very briefly but it seems more likely that the
application thread could be work intensive in which case it seems like
the thread would get migrated anyways or lead to more latency in the
long term with trying to compete on an overloaded core?

>
> With reduced queues we can assume that it to jump between cores, I
> have no problem to make it a conditional on that, just results are
> encouraging to apply it unconditionally - see the results above for
> "-o io_uring_q_mask=0-3:8-11' (16 core / 32 SMT core system)"

I'm wondering if that's because the application workloads being run
through fio aren't workloads where there is other locally cached data
the app is using.

> >
> >> +               else
> >> +                       wake_up(&req->waitq);
> >>          }
> >>
> >>          if (test_bit(FR_ASYNC, &req->flags))
> >> diff --git a/include/linux/wait.h b/include/linux/wait.h
> >> index f648044466d5f55f2d65a3aa153b4dfe39f0b6dc..831a187b3f68f0707c75ceee919fec338db410b3 100644
> >> --- a/include/linux/wait.h
> >> +++ b/include/linux/wait.h
> >> @@ -219,6 +219,7 @@ void __wake_up_sync(struct wait_queue_head *wq_head, unsigned int mode);
> >>   void __wake_up_pollfree(struct wait_queue_head *wq_head);
> >>
> >>   #define wake_up(x)                     __wake_up(x, TASK_NORMAL, 1, NULL)
> >> +#define wake_up_on_current_cpu(x)      __wake_up_on_current_cpu(x, TASK_NORMAL, NULL)
> >>   #define wake_up_nr(x, nr)              __wake_up(x, TASK_NORMAL, nr, NULL)
> >>   #define wake_up_all(x)                 __wake_up(x, TASK_NORMAL, 0, NULL)
> >>   #define wake_up_locked(x)              __wake_up_locked((x), TASK_NORMAL, 1)
> >> @@ -479,9 +480,8 @@ do {                                                                                \
> >>          __wait_event_cmd(wq_head, condition, cmd1, cmd2);                       \
> >>   } while (0)
> >>
> >> -#define __wait_event_interruptible(wq_head, condition)                         \
> >> -       ___wait_event(wq_head, condition, TASK_INTERRUPTIBLE, 0, 0,             \
> >> -                     schedule())
> >> +#define __wait_event_interruptible(wq_head, condition) \
> >> +       ___wait_event(wq_head, condition, TASK_INTERRUPTIBLE, 0, 0, schedule())
> >>
> >>   /**
> >>    * wait_event_interruptible - sleep until a condition gets true
> >> diff --git a/kernel/sched/wait.c b/kernel/sched/wait.c
> >> index 20f27e2cf7aec691af040fcf2236a20374ec66bf..94120076bc1ae465735843cc5821ca532d9c398a 100644
> >> --- a/kernel/sched/wait.c
> >> +++ b/kernel/sched/wait.c
> >> @@ -147,10 +147,24 @@ int __wake_up(struct wait_queue_head *wq_head, unsigned int mode,
> >>   }
> >>   EXPORT_SYMBOL(__wake_up);
> >>
> >> -void __wake_up_on_current_cpu(struct wait_queue_head *wq_head, unsigned int mode, void *key)
> >> +/**
> >> + * __wake_up_on_current_cpu - wake up threads blocked on a waitqueue, on the
> >> + * current cpu
> >> + * @wq_head: the waitqueue
> >> + * @mode: which threads
> >> + * @nr_exclusive: how many wake-one or wake-many threads to wake up
> >
> > I don't think you meant to include this line?
>
> Yeah, the entire comment is broken :( Sorry about that.
>
> >
> >> + * @key: is directly passed to the wakeup function
> >> + *
> >> + * If this function wakes up a task, it executes a full memory barrier
> >> + * before accessing the task state.  Returns the number of exclusive
> >> + * tasks that were awaken.
> >
> > Doesn't this return a void?
> >
>
> Yeah, I promise I triple check next time when I copy and paste comments.

No worries at all!

Thanks,
Joanne

>
>
> Thanks,
> Bernd

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2] fuse: Wake requests on the same cpu
  2025-10-15 22:19     ` Joanne Koong
@ 2025-10-16  8:58       ` Peter Zijlstra
  2025-10-16  9:00         ` Peter Zijlstra
  0 siblings, 1 reply; 13+ messages in thread
From: Peter Zijlstra @ 2025-10-16  8:58 UTC (permalink / raw)
  To: Joanne Koong
  Cc: Bernd Schubert, Bernd Schubert, Miklos Szeredi, Ingo Molnar,
	Juri Lelli, Vincent Guittot, Dietmar Eggemann, Steven Rostedt,
	Ben Segall, Mel Gorman, Valentin Schneider, Johannes Thumshirn,
	Luis Henriques, linux-fsdevel

On Wed, Oct 15, 2025 at 03:19:31PM -0700, Joanne Koong wrote:

> > > Won't this lose cache locality for all the other data that is in the
> > > client thread's cache on the previous CPU? It seems to me like on
> > > average this would be a costlier miss overall? What are your thoughts
> > > on this?
> >
> > So as in the introduction, which b4 made a '---' comment below,
> > initially I thought this should be a conditional on queue-per-core.
> > With queue-per-core it should be easy to explain, I think.
> >
> > App submits request on core-X, waits/sleeps, request gets handle on
> > core-X by queue-X.
> > If there are more applications running on this core, they
> > get likely re-scheduled to another core, as the libfuse queue thread is
> > core bound. If other applications don't get re-scheduled either the
> > entire system is overloaded or someone sets manual application core
> > affinity - we can't do much about that in either case. With
> > queue-per-core there is also no debate about "previous CPU".
> > Worse is actually scheduler behavior here, although the ring thread
> > itself goes to sleep soon enough. Application gets still quite often
> > re-scheduled to another core. Without wake-on-same core behavior is
> > even worse and it jumps across all the time. Not good for CPU cache...
> 
> Maybe this is a lack of my understanding of scheduler internals,  but
> I'm having a hard time seeing what the benefit of
> wake_up_on_current_cpu() is over wake_up() for the queue-per-core
> case.
> 
> As I understand it, with wake_up() the scheduler already will try to
> wake up the thread and put it back on the same core to maintain cache
> locality, which in this case is the same core
> "wake_up_on_current_cpu()" is trying to put it on. If there's too much
> load imbalance then regardless of whether you call wake_up() or
> wake_up_on_current_cpu(), the scheduler will migrate the task to
> whatever other core is better for it.
> 
> So I guess the main benefit of calling wake_up_on_current_cpu() over
> wake_up() is that for situations where there is only some but not too
> much load imbalance we force the application to run on the current
> core even despite the scheduler thinking it's better for overall
> system health to distribute the load? I don't see an issue if the
> application thread runs very briefly but it seems more likely that the
> application thread could be work intensive in which case it seems like
> the thread would get migrated anyways or lead to more latency in the
> long term with trying to compete on an overloaded core?

So the scheduler will try and wake on the previous CPU, but if that CPU
is not idle it will look for any non-idle CPU in the same L3 and very
aggressively move tasks around.

Notably if Task-A is waking Task-B, and Task-A is running on CPU-1 and
Task-B was previously running on CPU-1, then the wakeup will see CPU-1
is not idle (it is running Task-A) and it will try and find another CPU
in the same L3.

This is fine if Task-A continues running; however in the case where
Task-A is going to sleep right after doing the wakeup, this is perhaps
sub-optimal, CPU-1 will end up idle.

We have the WF_SYNC (wake-flag) that tries to indicate this latter case;
trouble is, it often gets used where it should not be, it is unreliable.
Therefore it not a strong hint.

Then we 'recently' grew WF_CURRENT_CPU, that forces the wakeup to the
same CPU. If you abuse, you keep pieces :-)

So it all depends a bit on the workload, machine and situation.

Some machines L3 is fine, some machines L3 has exclusive L2 and it hurts
more to move tasks. Some workloads don't fit L2 so it doesn't matter
anyway. TL;DR is we need this damn crystal ball instruction :-)

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2] fuse: Wake requests on the same cpu
  2025-10-16  8:58       ` Peter Zijlstra
@ 2025-10-16  9:00         ` Peter Zijlstra
  2025-10-16 20:13           ` Joanne Koong
  0 siblings, 1 reply; 13+ messages in thread
From: Peter Zijlstra @ 2025-10-16  9:00 UTC (permalink / raw)
  To: Joanne Koong
  Cc: Bernd Schubert, Bernd Schubert, Miklos Szeredi, Ingo Molnar,
	Juri Lelli, Vincent Guittot, Dietmar Eggemann, Steven Rostedt,
	Ben Segall, Mel Gorman, Valentin Schneider, Johannes Thumshirn,
	Luis Henriques, linux-fsdevel

On Thu, Oct 16, 2025 at 10:58:14AM +0200, Peter Zijlstra wrote:
> On Wed, Oct 15, 2025 at 03:19:31PM -0700, Joanne Koong wrote:
> 
> > > > Won't this lose cache locality for all the other data that is in the
> > > > client thread's cache on the previous CPU? It seems to me like on
> > > > average this would be a costlier miss overall? What are your thoughts
> > > > on this?
> > >
> > > So as in the introduction, which b4 made a '---' comment below,
> > > initially I thought this should be a conditional on queue-per-core.
> > > With queue-per-core it should be easy to explain, I think.
> > >
> > > App submits request on core-X, waits/sleeps, request gets handle on
> > > core-X by queue-X.
> > > If there are more applications running on this core, they
> > > get likely re-scheduled to another core, as the libfuse queue thread is
> > > core bound. If other applications don't get re-scheduled either the
> > > entire system is overloaded or someone sets manual application core
> > > affinity - we can't do much about that in either case. With
> > > queue-per-core there is also no debate about "previous CPU".
> > > Worse is actually scheduler behavior here, although the ring thread
> > > itself goes to sleep soon enough. Application gets still quite often
> > > re-scheduled to another core. Without wake-on-same core behavior is
> > > even worse and it jumps across all the time. Not good for CPU cache...
> > 
> > Maybe this is a lack of my understanding of scheduler internals,  but
> > I'm having a hard time seeing what the benefit of
> > wake_up_on_current_cpu() is over wake_up() for the queue-per-core
> > case.
> > 
> > As I understand it, with wake_up() the scheduler already will try to
> > wake up the thread and put it back on the same core to maintain cache
> > locality, which in this case is the same core
> > "wake_up_on_current_cpu()" is trying to put it on. If there's too much
> > load imbalance then regardless of whether you call wake_up() or
> > wake_up_on_current_cpu(), the scheduler will migrate the task to
> > whatever other core is better for it.
> > 
> > So I guess the main benefit of calling wake_up_on_current_cpu() over
> > wake_up() is that for situations where there is only some but not too
> > much load imbalance we force the application to run on the current
> > core even despite the scheduler thinking it's better for overall
> > system health to distribute the load? I don't see an issue if the
> > application thread runs very briefly but it seems more likely that the
> > application thread could be work intensive in which case it seems like
> > the thread would get migrated anyways or lead to more latency in the
> > long term with trying to compete on an overloaded core?
> 
> So the scheduler will try and wake on the previous CPU, but if that CPU
> is not idle it will look for any non-idle CPU in the same L3 and very

Typing hard: s/non-//

> aggressively move tasks around.
> 
> Notably if Task-A is waking Task-B, and Task-A is running on CPU-1 and
> Task-B was previously running on CPU-1, then the wakeup will see CPU-1
> is not idle (it is running Task-A) and it will try and find another CPU
> in the same L3.
> 
> This is fine if Task-A continues running; however in the case where
> Task-A is going to sleep right after doing the wakeup, this is perhaps
> sub-optimal, CPU-1 will end up idle.
> 
> We have the WF_SYNC (wake-flag) that tries to indicate this latter case;
> trouble is, it often gets used where it should not be, it is unreliable.
> Therefore it not a strong hint.
> 
> Then we 'recently' grew WF_CURRENT_CPU, that forces the wakeup to the
> same CPU. If you abuse, you keep pieces :-)
> 
> So it all depends a bit on the workload, machine and situation.
> 
> Some machines L3 is fine, some machines L3 has exclusive L2 and it hurts
> more to move tasks. Some workloads don't fit L2 so it doesn't matter
> anyway. TL;DR is we need this damn crystal ball instruction :-)

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2] fuse: Wake requests on the same cpu
  2025-10-16  9:00         ` Peter Zijlstra
@ 2025-10-16 20:13           ` Joanne Koong
  2025-10-16 21:53             ` Bernd Schubert
  0 siblings, 1 reply; 13+ messages in thread
From: Joanne Koong @ 2025-10-16 20:13 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Bernd Schubert, Bernd Schubert, Miklos Szeredi, Ingo Molnar,
	Juri Lelli, Vincent Guittot, Dietmar Eggemann, Steven Rostedt,
	Ben Segall, Mel Gorman, Valentin Schneider, Johannes Thumshirn,
	Luis Henriques, linux-fsdevel

On Thu, Oct 16, 2025 at 2:00 AM Peter Zijlstra <peterz@infradead.org> wrote:
>
> On Thu, Oct 16, 2025 at 10:58:14AM +0200, Peter Zijlstra wrote:
> > On Wed, Oct 15, 2025 at 03:19:31PM -0700, Joanne Koong wrote:
> >
> > > > > Won't this lose cache locality for all the other data that is in the
> > > > > client thread's cache on the previous CPU? It seems to me like on
> > > > > average this would be a costlier miss overall? What are your thoughts
> > > > > on this?
> > > >
> > > > So as in the introduction, which b4 made a '---' comment below,
> > > > initially I thought this should be a conditional on queue-per-core.
> > > > With queue-per-core it should be easy to explain, I think.
> > > >
> > > > App submits request on core-X, waits/sleeps, request gets handle on
> > > > core-X by queue-X.
> > > > If there are more applications running on this core, they
> > > > get likely re-scheduled to another core, as the libfuse queue thread is
> > > > core bound. If other applications don't get re-scheduled either the
> > > > entire system is overloaded or someone sets manual application core
> > > > affinity - we can't do much about that in either case. With
> > > > queue-per-core there is also no debate about "previous CPU".
> > > > Worse is actually scheduler behavior here, although the ring thread
> > > > itself goes to sleep soon enough. Application gets still quite often
> > > > re-scheduled to another core. Without wake-on-same core behavior is
> > > > even worse and it jumps across all the time. Not good for CPU cache...
> > >
> > > Maybe this is a lack of my understanding of scheduler internals,  but
> > > I'm having a hard time seeing what the benefit of
> > > wake_up_on_current_cpu() is over wake_up() for the queue-per-core
> > > case.
> > >
> > > As I understand it, with wake_up() the scheduler already will try to
> > > wake up the thread and put it back on the same core to maintain cache
> > > locality, which in this case is the same core
> > > "wake_up_on_current_cpu()" is trying to put it on. If there's too much
> > > load imbalance then regardless of whether you call wake_up() or
> > > wake_up_on_current_cpu(), the scheduler will migrate the task to
> > > whatever other core is better for it.
> > >
> > > So I guess the main benefit of calling wake_up_on_current_cpu() over
> > > wake_up() is that for situations where there is only some but not too
> > > much load imbalance we force the application to run on the current
> > > core even despite the scheduler thinking it's better for overall
> > > system health to distribute the load? I don't see an issue if the
> > > application thread runs very briefly but it seems more likely that the
> > > application thread could be work intensive in which case it seems like
> > > the thread would get migrated anyways or lead to more latency in the
> > > long term with trying to compete on an overloaded core?
> >
> > So the scheduler will try and wake on the previous CPU, but if that CPU
> > is not idle it will look for any non-idle CPU in the same L3 and very
>
> Typing hard: s/non-//
>
> > aggressively move tasks around.
> >
> > Notably if Task-A is waking Task-B, and Task-A is running on CPU-1 and
> > Task-B was previously running on CPU-1, then the wakeup will see CPU-1
> > is not idle (it is running Task-A) and it will try and find another CPU
> > in the same L3.
> >
> > This is fine if Task-A continues running; however in the case where
> > Task-A is going to sleep right after doing the wakeup, this is perhaps
> > sub-optimal, CPU-1 will end up idle.
> >
> > We have the WF_SYNC (wake-flag) that tries to indicate this latter case;
> > trouble is, it often gets used where it should not be, it is unreliable.
> > Therefore it not a strong hint.
> >
> > Then we 'recently' grew WF_CURRENT_CPU, that forces the wakeup to the
> > same CPU. If you abuse, you keep pieces :-)
> >
> > So it all depends a bit on the workload, machine and situation.
> >
> > Some machines L3 is fine, some machines L3 has exclusive L2 and it hurts
> > more to move tasks. Some workloads don't fit L2 so it doesn't matter
> > anyway. TL;DR is we need this damn crystal ball instruction :-)

Thanks for the explanation! I found it very helpful.

In light of that information, it seems to me that the original
wake_up() would be more optimal here than wake_up_on_current_cpu()
then. After fuse_request_end(), the thread still has work to do with
fetching and servicing the next requests. If it wakes up the
application on its cpu, then with queue-per-core the thread would be
forced to sleep since on the libfuse side during setup the thread is
pinned to the core, which would prevent any migration while the
application task runs. Or am I misassuming something in this analysis,
Bernd?

Thanks,
Joanne

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2] fuse: Wake requests on the same cpu
  2025-10-16 20:13           ` Joanne Koong
@ 2025-10-16 21:53             ` Bernd Schubert
  2025-10-16 22:30               ` Bernd Schubert
                                 ` (2 more replies)
  0 siblings, 3 replies; 13+ messages in thread
From: Bernd Schubert @ 2025-10-16 21:53 UTC (permalink / raw)
  To: Joanne Koong, Peter Zijlstra
  Cc: Bernd Schubert, Miklos Szeredi, Ingo Molnar, Juri Lelli,
	Vincent Guittot, Dietmar Eggemann, Steven Rostedt, Ben Segall,
	Mel Gorman, Valentin Schneider, Johannes Thumshirn,
	Luis Henriques, linux-fsdevel@vger.kernel.org

On 10/16/25 22:13, Joanne Koong wrote:
> On Thu, Oct 16, 2025 at 2:00 AM Peter Zijlstra <peterz@infradead.org> wrote:
>>
>> On Thu, Oct 16, 2025 at 10:58:14AM +0200, Peter Zijlstra wrote:
>>> On Wed, Oct 15, 2025 at 03:19:31PM -0700, Joanne Koong wrote:
>>>
>>>>>> Won't this lose cache locality for all the other data that is in the
>>>>>> client thread's cache on the previous CPU? It seems to me like on
>>>>>> average this would be a costlier miss overall? What are your thoughts
>>>>>> on this?
>>>>>
>>>>> So as in the introduction, which b4 made a '---' comment below,
>>>>> initially I thought this should be a conditional on queue-per-core.
>>>>> With queue-per-core it should be easy to explain, I think.
>>>>>
>>>>> App submits request on core-X, waits/sleeps, request gets handle on
>>>>> core-X by queue-X.
>>>>> If there are more applications running on this core, they
>>>>> get likely re-scheduled to another core, as the libfuse queue thread is
>>>>> core bound. If other applications don't get re-scheduled either the
>>>>> entire system is overloaded or someone sets manual application core
>>>>> affinity - we can't do much about that in either case. With
>>>>> queue-per-core there is also no debate about "previous CPU".
>>>>> Worse is actually scheduler behavior here, although the ring thread
>>>>> itself goes to sleep soon enough. Application gets still quite often
>>>>> re-scheduled to another core. Without wake-on-same core behavior is
>>>>> even worse and it jumps across all the time. Not good for CPU cache...
>>>>
>>>> Maybe this is a lack of my understanding of scheduler internals,  but
>>>> I'm having a hard time seeing what the benefit of
>>>> wake_up_on_current_cpu() is over wake_up() for the queue-per-core
>>>> case.
>>>>
>>>> As I understand it, with wake_up() the scheduler already will try to
>>>> wake up the thread and put it back on the same core to maintain cache
>>>> locality, which in this case is the same core
>>>> "wake_up_on_current_cpu()" is trying to put it on. If there's too much
>>>> load imbalance then regardless of whether you call wake_up() or
>>>> wake_up_on_current_cpu(), the scheduler will migrate the task to
>>>> whatever other core is better for it.
>>>>
>>>> So I guess the main benefit of calling wake_up_on_current_cpu() over
>>>> wake_up() is that for situations where there is only some but not too
>>>> much load imbalance we force the application to run on the current
>>>> core even despite the scheduler thinking it's better for overall
>>>> system health to distribute the load? I don't see an issue if the
>>>> application thread runs very briefly but it seems more likely that the
>>>> application thread could be work intensive in which case it seems like
>>>> the thread would get migrated anyways or lead to more latency in the
>>>> long term with trying to compete on an overloaded core?
>>>
>>> So the scheduler will try and wake on the previous CPU, but if that CPU
>>> is not idle it will look for any non-idle CPU in the same L3 and very
>>
>> Typing hard: s/non-//
>>
>>> aggressively move tasks around.
>>>
>>> Notably if Task-A is waking Task-B, and Task-A is running on CPU-1 and
>>> Task-B was previously running on CPU-1, then the wakeup will see CPU-1
>>> is not idle (it is running Task-A) and it will try and find another CPU
>>> in the same L3.
>>>
>>> This is fine if Task-A continues running; however in the case where
>>> Task-A is going to sleep right after doing the wakeup, this is perhaps
>>> sub-optimal, CPU-1 will end up idle.
>>>
>>> We have the WF_SYNC (wake-flag) that tries to indicate this latter case;
>>> trouble is, it often gets used where it should not be, it is unreliable.
>>> Therefore it not a strong hint.
>>>
>>> Then we 'recently' grew WF_CURRENT_CPU, that forces the wakeup to the
>>> same CPU. If you abuse, you keep pieces :-)
>>>
>>> So it all depends a bit on the workload, machine and situation.
>>>
>>> Some machines L3 is fine, some machines L3 has exclusive L2 and it hurts
>>> more to move tasks. Some workloads don't fit L2 so it doesn't matter
>>> anyway. TL;DR is we need this damn crystal ball instruction :-)
> 
> Thanks for the explanation! I found it very helpful.
> 
> In light of that information, it seems to me that the original
> wake_up() would be more optimal here than wake_up_on_current_cpu()
> then. After fuse_request_end(), the thread still has work to do with
> fetching and servicing the next requests. If it wakes up the
> application on its cpu, then with queue-per-core the thread would be
> forced to sleep since on the libfuse side during setup the thread is
> pinned to the core, which would prevent any migration while the
> application task runs. Or am I misassuming something in this analysis,
> Bernd?


Well, the numbers speak a different language. And I still don't see
why wouldn't want to take on the current CPU at least if we have
queue-per-core.
Thanks a lot @Peter for the explanation. To my understanding WF_SYNC
should do the trick, i.e. wake on the same core, when the current task
is going to sleep anyway and there is nothing else running on that core.
For a blocking IO with queue-per-core this is exactly what we have.

Example

+ echo 'Running: example/passthrough_hp' -o allow_other --foreground --nopassthrough -o io_uring -o io_uring_nr_qs=2 /tmp/source /tmp/dest


And then 

bschubert2@imesrv3 ~>fio --directory=/tmp/dest --name=iops.\$jobnum --rw=randread --bs=4k --size=1G --numjobs=1 --iodepth=1 --time_based --runtime=30s --group_reporting --ioengine=psync --direct=1 


With WF_CURRENT_CPU: READ: bw=269MiB/s
With WF_SYNC:        READ: bw=214MiB/s
With plain wake_up:  READ: bw=217MiB/s

With WF_SYNC and plain wake_up I see a persistent core switching
of the fio process between two cores on one numa node - so much 
about L1/L2 cpu cache.
With more fuse-io-uring queues there also would be additional
switching for that and even lower perf, but with only one queue
per numa that gets a bit restricted.

My guess is that WF_SYNC doesn't detect that the current libfuse
ring thread will go to sleep in the io_uring_enter() system call
rather quickly.
I can try to get some time and figure out why the fio process
bounces between two cores. I had already started to ftrace
things, because even WF_CURRENT_CPU isn't ideal. Although this
discussion here goes the other direction.


Thanks,
Bernd



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2] fuse: Wake requests on the same cpu
  2025-10-16 21:53             ` Bernd Schubert
@ 2025-10-16 22:30               ` Bernd Schubert
  2025-10-17  0:01               ` Joanne Koong
  2025-10-19 18:15               ` Bernd Schubert
  2 siblings, 0 replies; 13+ messages in thread
From: Bernd Schubert @ 2025-10-16 22:30 UTC (permalink / raw)
  To: Bernd Schubert, Joanne Koong, Peter Zijlstra
  Cc: Miklos Szeredi, Ingo Molnar, Juri Lelli, Vincent Guittot,
	Dietmar Eggemann, Steven Rostedt, Ben Segall, Mel Gorman,
	Valentin Schneider, Johannes Thumshirn, Luis Henriques,
	linux-fsdevel@vger.kernel.org



On 10/16/25 23:53, Bernd Schubert wrote:
> On 10/16/25 22:13, Joanne Koong wrote:
>> On Thu, Oct 16, 2025 at 2:00 AM Peter Zijlstra <peterz@infradead.org> wrote:
>>>
>>> On Thu, Oct 16, 2025 at 10:58:14AM +0200, Peter Zijlstra wrote:
>>>> On Wed, Oct 15, 2025 at 03:19:31PM -0700, Joanne Koong wrote:
>>>>
>>>>>>> Won't this lose cache locality for all the other data that is in the
>>>>>>> client thread's cache on the previous CPU? It seems to me like on
>>>>>>> average this would be a costlier miss overall? What are your thoughts
>>>>>>> on this?
>>>>>>
>>>>>> So as in the introduction, which b4 made a '---' comment below,
>>>>>> initially I thought this should be a conditional on queue-per-core.
>>>>>> With queue-per-core it should be easy to explain, I think.
>>>>>>
>>>>>> App submits request on core-X, waits/sleeps, request gets handle on
>>>>>> core-X by queue-X.
>>>>>> If there are more applications running on this core, they
>>>>>> get likely re-scheduled to another core, as the libfuse queue thread is
>>>>>> core bound. If other applications don't get re-scheduled either the
>>>>>> entire system is overloaded or someone sets manual application core
>>>>>> affinity - we can't do much about that in either case. With
>>>>>> queue-per-core there is also no debate about "previous CPU".
>>>>>> Worse is actually scheduler behavior here, although the ring thread
>>>>>> itself goes to sleep soon enough. Application gets still quite often
>>>>>> re-scheduled to another core. Without wake-on-same core behavior is
>>>>>> even worse and it jumps across all the time. Not good for CPU cache...
>>>>>
>>>>> Maybe this is a lack of my understanding of scheduler internals,  but
>>>>> I'm having a hard time seeing what the benefit of
>>>>> wake_up_on_current_cpu() is over wake_up() for the queue-per-core
>>>>> case.
>>>>>
>>>>> As I understand it, with wake_up() the scheduler already will try to
>>>>> wake up the thread and put it back on the same core to maintain cache
>>>>> locality, which in this case is the same core
>>>>> "wake_up_on_current_cpu()" is trying to put it on. If there's too much
>>>>> load imbalance then regardless of whether you call wake_up() or
>>>>> wake_up_on_current_cpu(), the scheduler will migrate the task to
>>>>> whatever other core is better for it.
>>>>>
>>>>> So I guess the main benefit of calling wake_up_on_current_cpu() over
>>>>> wake_up() is that for situations where there is only some but not too
>>>>> much load imbalance we force the application to run on the current
>>>>> core even despite the scheduler thinking it's better for overall
>>>>> system health to distribute the load? I don't see an issue if the
>>>>> application thread runs very briefly but it seems more likely that the
>>>>> application thread could be work intensive in which case it seems like
>>>>> the thread would get migrated anyways or lead to more latency in the
>>>>> long term with trying to compete on an overloaded core?
>>>>
>>>> So the scheduler will try and wake on the previous CPU, but if that CPU
>>>> is not idle it will look for any non-idle CPU in the same L3 and very
>>>
>>> Typing hard: s/non-//
>>>
>>>> aggressively move tasks around.
>>>>
>>>> Notably if Task-A is waking Task-B, and Task-A is running on CPU-1 and
>>>> Task-B was previously running on CPU-1, then the wakeup will see CPU-1
>>>> is not idle (it is running Task-A) and it will try and find another CPU
>>>> in the same L3.
>>>>
>>>> This is fine if Task-A continues running; however in the case where
>>>> Task-A is going to sleep right after doing the wakeup, this is perhaps
>>>> sub-optimal, CPU-1 will end up idle.
>>>>
>>>> We have the WF_SYNC (wake-flag) that tries to indicate this latter case;
>>>> trouble is, it often gets used where it should not be, it is unreliable.
>>>> Therefore it not a strong hint.
>>>>
>>>> Then we 'recently' grew WF_CURRENT_CPU, that forces the wakeup to the
>>>> same CPU. If you abuse, you keep pieces :-)
>>>>
>>>> So it all depends a bit on the workload, machine and situation.
>>>>
>>>> Some machines L3 is fine, some machines L3 has exclusive L2 and it hurts
>>>> more to move tasks. Some workloads don't fit L2 so it doesn't matter
>>>> anyway. TL;DR is we need this damn crystal ball instruction :-)
>>
>> Thanks for the explanation! I found it very helpful.
>>
>> In light of that information, it seems to me that the original
>> wake_up() would be more optimal here than wake_up_on_current_cpu()
>> then. After fuse_request_end(), the thread still has work to do with
>> fetching and servicing the next requests. If it wakes up the
>> application on its cpu, then with queue-per-core the thread would be
>> forced to sleep since on the libfuse side during setup the thread is
>> pinned to the core, which would prevent any migration while the
>> application task runs. Or am I misassuming something in this analysis,
>> Bernd?
> 
> 
> Well, the numbers speak a different language. And I still don't see
> why wouldn't want to take on the current CPU at least if we have
> queue-per-core.
> Thanks a lot @Peter for the explanation. To my understanding WF_SYNC
> should do the trick, i.e. wake on the same core, when the current task
> is going to sleep anyway and there is nothing else running on that core.
> For a blocking IO with queue-per-core this is exactly what we have.
> 
> Example
> 
> + echo 'Running: example/passthrough_hp' -o allow_other --foreground --nopassthrough -o io_uring -o io_uring_nr_qs=2 /tmp/source /tmp/dest
> 
> 
> And then 
> 
> bschubert2@imesrv3 ~>fio --directory=/tmp/dest --name=iops.\$jobnum --rw=randread --bs=4k --size=1G --numjobs=1 --iodepth=1 --time_based --runtime=30s --group_reporting --ioengine=psync --direct=1 
> 
> 
> With WF_CURRENT_CPU: READ: bw=269MiB/s
> With WF_SYNC:        READ: bw=214MiB/s
> With plain wake_up:  READ: bw=217MiB/s
> 
> With WF_SYNC and plain wake_up I see a persistent core switching
> of the fio process between two cores on one numa node - so much 
> about L1/L2 cpu cache.
> With more fuse-io-uring queues there also would be additional
> switching for that and even lower perf, but with only one queue
> per numa that gets a bit restricted.
> 
> My guess is that WF_SYNC doesn't detect that the current libfuse
> ring thread will go to sleep in the io_uring_enter() system call
> rather quickly.
> I can try to get some time and figure out why the fio process
> bounces between two cores. I had already started to ftrace
> things, because even WF_CURRENT_CPU isn't ideal. Although this
> discussion here goes the other direction.


A bit more data points, with WF_CURRENT_CPU it sometimes jumps to 
330MB/s - both process then run on the same core and cpu-freq goes
up to max.

With 'numactl --localalloc --physcpubind=8', i.e. binding to the 
core of one of the ring threads, it 330MB/s.

With 'numactl --localalloc --physcpubind=10' or any other non-ring
thread core, it is about 270MB/s - this 'ideal' wake_up() should
result in this.

(fuse-io-uring ring threads are running on core-0 and core-8).


Thanks,
Bernd

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2] fuse: Wake requests on the same cpu
  2025-10-16 21:53             ` Bernd Schubert
  2025-10-16 22:30               ` Bernd Schubert
@ 2025-10-17  0:01               ` Joanne Koong
  2025-10-19 18:15               ` Bernd Schubert
  2 siblings, 0 replies; 13+ messages in thread
From: Joanne Koong @ 2025-10-17  0:01 UTC (permalink / raw)
  To: Bernd Schubert
  Cc: Peter Zijlstra, Bernd Schubert, Miklos Szeredi, Ingo Molnar,
	Juri Lelli, Vincent Guittot, Dietmar Eggemann, Steven Rostedt,
	Ben Segall, Mel Gorman, Valentin Schneider, Johannes Thumshirn,
	Luis Henriques, linux-fsdevel@vger.kernel.org

On Thu, Oct 16, 2025 at 2:54 PM Bernd Schubert <bschubert@ddn.com> wrote:
>
> On 10/16/25 22:13, Joanne Koong wrote:
> > On Thu, Oct 16, 2025 at 2:00 AM Peter Zijlstra <peterz@infradead.org> wrote:
> >>
> >> On Thu, Oct 16, 2025 at 10:58:14AM +0200, Peter Zijlstra wrote:
> >>> On Wed, Oct 15, 2025 at 03:19:31PM -0700, Joanne Koong wrote:
> >>>
> >>>>>> Won't this lose cache locality for all the other data that is in the
> >>>>>> client thread's cache on the previous CPU? It seems to me like on
> >>>>>> average this would be a costlier miss overall? What are your thoughts
> >>>>>> on this?
> >>>>>
> >>>>> So as in the introduction, which b4 made a '---' comment below,
> >>>>> initially I thought this should be a conditional on queue-per-core.
> >>>>> With queue-per-core it should be easy to explain, I think.
> >>>>>
> >>>>> App submits request on core-X, waits/sleeps, request gets handle on
> >>>>> core-X by queue-X.
> >>>>> If there are more applications running on this core, they
> >>>>> get likely re-scheduled to another core, as the libfuse queue thread is
> >>>>> core bound. If other applications don't get re-scheduled either the
> >>>>> entire system is overloaded or someone sets manual application core
> >>>>> affinity - we can't do much about that in either case. With
> >>>>> queue-per-core there is also no debate about "previous CPU".
> >>>>> Worse is actually scheduler behavior here, although the ring thread
> >>>>> itself goes to sleep soon enough. Application gets still quite often
> >>>>> re-scheduled to another core. Without wake-on-same core behavior is
> >>>>> even worse and it jumps across all the time. Not good for CPU cache...
> >>>>
> >>>> Maybe this is a lack of my understanding of scheduler internals,  but
> >>>> I'm having a hard time seeing what the benefit of
> >>>> wake_up_on_current_cpu() is over wake_up() for the queue-per-core
> >>>> case.
> >>>>
> >>>> As I understand it, with wake_up() the scheduler already will try to
> >>>> wake up the thread and put it back on the same core to maintain cache
> >>>> locality, which in this case is the same core
> >>>> "wake_up_on_current_cpu()" is trying to put it on. If there's too much
> >>>> load imbalance then regardless of whether you call wake_up() or
> >>>> wake_up_on_current_cpu(), the scheduler will migrate the task to
> >>>> whatever other core is better for it.
> >>>>
> >>>> So I guess the main benefit of calling wake_up_on_current_cpu() over
> >>>> wake_up() is that for situations where there is only some but not too
> >>>> much load imbalance we force the application to run on the current
> >>>> core even despite the scheduler thinking it's better for overall
> >>>> system health to distribute the load? I don't see an issue if the
> >>>> application thread runs very briefly but it seems more likely that the
> >>>> application thread could be work intensive in which case it seems like
> >>>> the thread would get migrated anyways or lead to more latency in the
> >>>> long term with trying to compete on an overloaded core?
> >>>
> >>> So the scheduler will try and wake on the previous CPU, but if that CPU
> >>> is not idle it will look for any non-idle CPU in the same L3 and very
> >>
> >> Typing hard: s/non-//
> >>
> >>> aggressively move tasks around.
> >>>
> >>> Notably if Task-A is waking Task-B, and Task-A is running on CPU-1 and
> >>> Task-B was previously running on CPU-1, then the wakeup will see CPU-1
> >>> is not idle (it is running Task-A) and it will try and find another CPU
> >>> in the same L3.
> >>>
> >>> This is fine if Task-A continues running; however in the case where
> >>> Task-A is going to sleep right after doing the wakeup, this is perhaps
> >>> sub-optimal, CPU-1 will end up idle.
> >>>
> >>> We have the WF_SYNC (wake-flag) that tries to indicate this latter case;
> >>> trouble is, it often gets used where it should not be, it is unreliable.
> >>> Therefore it not a strong hint.
> >>>
> >>> Then we 'recently' grew WF_CURRENT_CPU, that forces the wakeup to the
> >>> same CPU. If you abuse, you keep pieces :-)
> >>>
> >>> So it all depends a bit on the workload, machine and situation.
> >>>
> >>> Some machines L3 is fine, some machines L3 has exclusive L2 and it hurts
> >>> more to move tasks. Some workloads don't fit L2 so it doesn't matter
> >>> anyway. TL;DR is we need this damn crystal ball instruction :-)
> >
> > Thanks for the explanation! I found it very helpful.
> >
> > In light of that information, it seems to me that the original
> > wake_up() would be more optimal here than wake_up_on_current_cpu()
> > then. After fuse_request_end(), the thread still has work to do with
> > fetching and servicing the next requests. If it wakes up the
> > application on its cpu, then with queue-per-core the thread would be
> > forced to sleep since on the libfuse side during setup the thread is
> > pinned to the core, which would prevent any migration while the
> > application task runs. Or am I misassuming something in this analysis,
> > Bernd?
>
>
> Well, the numbers speak a different language. And I still don't see

Thanks for all the data points and your work on this.

> why wouldn't want to take on the current CPU at least if we have
> queue-per-core.

In my understanding, it's because if there are other requests on that
io-uring queue, then they would get delayed in being relayed /
serviced by the server thread since the application thread is now
running on that core, whereas if it was scheduled onto another core,
they could run in parallel.

> Thanks a lot @Peter for the explanation. To my understanding WF_SYNC
> should do the trick, i.e. wake on the same core, when the current task
> is going to sleep anyway and there is nothing else running on that core.
> For a blocking IO with queue-per-core this is exactly what we have.
>
> Example
>
> + echo 'Running: example/passthrough_hp' -o allow_other --foreground --nopassthrough -o io_uring -o io_uring_nr_qs=2 /tmp/source /tmp/dest
>
>
> And then
>
> bschubert2@imesrv3 ~>fio --directory=/tmp/dest --name=iops.\$jobnum --rw=randread --bs=4k --size=1G --numjobs=1 --iodepth=1 --time_based --runtime=30s --group_reporting --ioengine=psync --direct=1
>
>
> With WF_CURRENT_CPU: READ: bw=269MiB/s
> With WF_SYNC:        READ: bw=214MiB/s
> With plain wake_up:  READ: bw=217MiB/s

If you have time, could you run the fio job with something like
--ioengine=libaio --numjobs=8 --iodepth=8? I'm wondering how that
would compare.

With psync, numjobs=1 and iodepth=1, I think this is a single-threaded
synchronous i/o environment where it does make sense to wake up the
client task on the current core because there are no pending requests
for the thread to fetch and execute in the meantime (since additional
requests are only enqueued after the application's read/write call has
returned). I'm curious how this looks with libaio and multiple threads
where there would be other pending requests on the queue when waking
up the client. I'm also happy to run this on my end tomorrow if you're
pressed for time and busy with other stuff.

>
> With WF_SYNC and plain wake_up I see a persistent core switching
> of the fio process between two cores on one numa node - so much
> about L1/L2 cpu cache.
> With more fuse-io-uring queues there also would be additional
> switching for that and even lower perf, but with only one queue
> per numa that gets a bit restricted.
>
> My guess is that WF_SYNC doesn't detect that the current libfuse
> ring thread will go to sleep in the io_uring_enter() system call
> rather quickly.

This is the part I'm confused about. Does the ring thread go to sleep
in the io_uring_enter() system call? My understanding is that it
doesn't unless IOSQE_ASYNC is set on the sqes which to my knowledge
libfuse doesn't do.

Thanks,
Joanne

> I can try to get some time and figure out why the fio process
> bounces between two cores. I had already started to ftrace
> things, because even WF_CURRENT_CPU isn't ideal. Although this
> discussion here goes the other direction.
>
>
> Thanks,
> Bernd
>
>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2] fuse: Wake requests on the same cpu
  2025-10-16 21:53             ` Bernd Schubert
  2025-10-16 22:30               ` Bernd Schubert
  2025-10-17  0:01               ` Joanne Koong
@ 2025-10-19 18:15               ` Bernd Schubert
  2025-10-19 22:22                 ` Bernd Schubert
  2 siblings, 1 reply; 13+ messages in thread
From: Bernd Schubert @ 2025-10-19 18:15 UTC (permalink / raw)
  To: Bernd Schubert, Joanne Koong, Peter Zijlstra
  Cc: Miklos Szeredi, Ingo Molnar, Juri Lelli, Vincent Guittot,
	Dietmar Eggemann, Steven Rostedt, Ben Segall, Mel Gorman,
	Valentin Schneider, Johannes Thumshirn, Luis Henriques,
	linux-fsdevel@vger.kernel.org



On 10/16/25 23:53, Bernd Schubert wrote:
> On 10/16/25 22:13, Joanne Koong wrote:
> And then 
> 
> bschubert2@imesrv3 ~>fio --directory=/tmp/dest --name=iops.\$jobnum --rw=randread --bs=4k --size=1G --numjobs=1 --iodepth=1 --time_based --runtime=30s --group_reporting --ioengine=psync --direct=1 
> 
> 
> With WF_CURRENT_CPU: READ: bw=269MiB/s
> With WF_SYNC:        READ: bw=214MiB/s
> With plain wake_up:  READ: bw=217MiB/s
> 

The culprit with plain wake might be here

             fio-7669    [011] d..2. 13916.272607: sched_switch: prev_comm=fio prev_pid=7669 prev_prio=120 prev_state=S ==> next_comm=swapper/11 next_pid=
...
          <idle>-0       [011] d.s4. 13916.281747: sched_waking: comm=kworker/11:1 pid=297 prio=120 target_cpu=011
          <idle>-0       [011] d.s4. 13916.281749: sched_waking_extended: comm=kworker/11:1 pid=297 prio=120 prev_cpu=011 current_cpu=011 target_cpu=011
          <idle>-0       [011] dNs5. 13916.281769: sched_wakeup: comm=kworker/11:1 pid=297 prio=120 target_cpu=011
          <idle>-0       [011] dNs5. 13916.281769: sched_wakeup_extended: comm=kworker/11:1 pid=297 prio=120 prev_cpu=011 current_cpu=011 target_cpu=011
          <idle>-0       [011] d..2. 13916.281779: sched_switch: prev_comm=swapper/11 prev_pid=0 prev_prio=120 prev_state=R ==> next_comm=kworker/11:1 nex
t_pid=297 next_prio=120
    kworker/11:1-297     [011] d.h3. 13916.282654: sched_waking: comm=fio pid=7669 prio=120 target_cpu=011
    kworker/11:1-297     [011] d.h3. 13916.282654: sched_waking_extended: comm=fio pid=7669 prio=120 prev_cpu=011 current_cpu=011 target_cpu=011
    kworker/11:1-297     [011] d.h3. 13916.282654: select_task_rq_fair <-select_task_rq
    kworker/11:1-297     [011] d.h3. 13916.282655: sched_migrate_task: comm=fio pid=7669 prio=120 orig_cpu=11 dest_cpu=26


==> migration from cpu=11 to cpu=26

    kworker/11:1-297     [011] d.h3. 13916.282657: sched_wake_idle_without_ipi: cpu=26
          <idle>-0       [026] dN.2. 13916.282686: sched_wakeup: comm=fio pid=7669 prio=120 target_cpu=026
          <idle>-0       [026] dN.2. 13916.282687: sched_wakeup_extended: comm=fio pid=7669 prio=120 prev_cpu=026 current_cpu=026 target_cpu=026
          <idle>-0       [026] d..2. 13916.282690: sched_switch: prev_comm=swapper/26 prev_pid=0 prev_prio=120 prev_state=R ==> next_comm=fio next_pid=766
9 next_prio=120
             fio-7669    [026] d..2. 13916.282717: sched_switch: prev_comm=fio prev_pid=7669 prev_prio=120 prev_state=S ==> next_comm=swapper/26 next_pid=


This is with added in sched_wakeup_extended() but prev_cpu is not
correctly assigned yet. Also not needed anymore as we can see
that migration is in in select_task_rq_fair/sched_migrate_task.




^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2] fuse: Wake requests on the same cpu
  2025-10-19 18:15               ` Bernd Schubert
@ 2025-10-19 22:22                 ` Bernd Schubert
  0 siblings, 0 replies; 13+ messages in thread
From: Bernd Schubert @ 2025-10-19 22:22 UTC (permalink / raw)
  To: Bernd Schubert, Joanne Koong, Peter Zijlstra
  Cc: Miklos Szeredi, Ingo Molnar, Juri Lelli, Vincent Guittot,
	Dietmar Eggemann, Steven Rostedt, Ben Segall, Mel Gorman,
	Valentin Schneider, Johannes Thumshirn, Luis Henriques,
	linux-fsdevel@vger.kernel.org



On 10/19/25 20:15, Bernd Schubert wrote:
> 
> 
> On 10/16/25 23:53, Bernd Schubert wrote:
>> On 10/16/25 22:13, Joanne Koong wrote:
>> And then 
>>
>> bschubert2@imesrv3 ~>fio --directory=/tmp/dest --name=iops.\$jobnum --rw=randread --bs=4k --size=1G --numjobs=1 --iodepth=1 --time_based --runtime=30s --group_reporting --ioengine=psync --direct=1 
>>
>>
>> With WF_CURRENT_CPU: READ: bw=269MiB/s
>> With WF_SYNC:        READ: bw=214MiB/s
>> With plain wake_up:  READ: bw=217MiB/s
>>
> 
> The culprit with plain wake might be here
> 
>              fio-7669    [011] d..2. 13916.272607: sched_switch: prev_comm=fio prev_pid=7669 prev_prio=120 prev_state=S ==> next_comm=swapper/11 next_pid=
> ...
>           <idle>-0       [011] d.s4. 13916.281747: sched_waking: comm=kworker/11:1 pid=297 prio=120 target_cpu=011
>           <idle>-0       [011] d.s4. 13916.281749: sched_waking_extended: comm=kworker/11:1 pid=297 prio=120 prev_cpu=011 current_cpu=011 target_cpu=011
>           <idle>-0       [011] dNs5. 13916.281769: sched_wakeup: comm=kworker/11:1 pid=297 prio=120 target_cpu=011
>           <idle>-0       [011] dNs5. 13916.281769: sched_wakeup_extended: comm=kworker/11:1 pid=297 prio=120 prev_cpu=011 current_cpu=011 target_cpu=011
>           <idle>-0       [011] d..2. 13916.281779: sched_switch: prev_comm=swapper/11 prev_pid=0 prev_prio=120 prev_state=R ==> next_comm=kworker/11:1 nex
> t_pid=297 next_prio=120
>     kworker/11:1-297     [011] d.h3. 13916.282654: sched_waking: comm=fio pid=7669 prio=120 target_cpu=011
>     kworker/11:1-297     [011] d.h3. 13916.282654: sched_waking_extended: comm=fio pid=7669 prio=120 prev_cpu=011 current_cpu=011 target_cpu=011
>     kworker/11:1-297     [011] d.h3. 13916.282654: select_task_rq_fair <-select_task_rq
>     kworker/11:1-297     [011] d.h3. 13916.282655: sched_migrate_task: comm=fio pid=7669 prio=120 orig_cpu=11 dest_cpu=26
> 
> 
> ==> migration from cpu=11 to cpu=26
> 
>     kworker/11:1-297     [011] d.h3. 13916.282657: sched_wake_idle_without_ipi: cpu=26
>           <idle>-0       [026] dN.2. 13916.282686: sched_wakeup: comm=fio pid=7669 prio=120 target_cpu=026
>           <idle>-0       [026] dN.2. 13916.282687: sched_wakeup_extended: comm=fio pid=7669 prio=120 prev_cpu=026 current_cpu=026 target_cpu=026
>           <idle>-0       [026] d..2. 13916.282690: sched_switch: prev_comm=swapper/26 prev_pid=0 prev_prio=120 prev_state=R ==> next_comm=fio next_pid=766
> 9 next_prio=120
>              fio-7669    [026] d..2. 13916.282717: sched_switch: prev_comm=fio prev_pid=7669 prev_prio=120 prev_state=S ==> next_comm=swapper/26 next_pid=
> 
> 
> This is with added in sched_wakeup_extended() but prev_cpu is not
> correctly assigned yet. Also not needed anymore as we can see
> that migration is in in select_task_rq_fair/sched_migrate_task.
> 

The persistent core switches seem to come from select_idle_sibling(). 
Hmm, that kind of means we should create one queue and ring-thread per
sibling only.



^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2025-10-19 22:22 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-14  9:49 [PATCH v2] fuse: Wake requests on the same cpu Bernd Schubert
2025-10-14 23:11 ` Joanne Koong
2025-10-15 15:30   ` Bernd Schubert
2025-10-15 16:36     ` Bernd Schubert
2025-10-15 22:19     ` Joanne Koong
2025-10-16  8:58       ` Peter Zijlstra
2025-10-16  9:00         ` Peter Zijlstra
2025-10-16 20:13           ` Joanne Koong
2025-10-16 21:53             ` Bernd Schubert
2025-10-16 22:30               ` Bernd Schubert
2025-10-17  0:01               ` Joanne Koong
2025-10-19 18:15               ` Bernd Schubert
2025-10-19 22:22                 ` Bernd Schubert

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox