public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] perf/x86: Add cond_resched() when allocate/release DS buffers
@ 2025-10-27 21:27 Namhyung Kim
  2025-10-28 14:15 ` Peter Zijlstra
  0 siblings, 1 reply; 7+ messages in thread
From: Namhyung Kim @ 2025-10-27 21:27 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar
  Cc: Mark Rutland, Alexander Shishkin, Arnaldo Carvalho de Melo,
	Ian Rogers, Adrian Hunter, LKML, Eric Dumazet

On large systems, it's possible to trigger sched latency warning during
the DS buffer allocation or release.  Add cond_resched() to avoid it.

Reported-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 arch/x86/events/intel/ds.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/x86/events/intel/ds.c b/arch/x86/events/intel/ds.c
index c0b7ac1c7594ea92..c8e90c5a8d3390ab 100644
--- a/arch/x86/events/intel/ds.c
+++ b/arch/x86/events/intel/ds.c
@@ -754,6 +754,7 @@ void release_ds_buffers(void)
 		if (x86_pmu.ds_pebs)
 			release_pebs_buffer(cpu);
 		release_bts_buffer(cpu);
+		cond_resched();
 	}
 }
 
@@ -791,6 +792,8 @@ void reserve_ds_buffers(void)
 
 		if (bts_err && pebs_err)
 			break;
+
+		cond_resched();
 	}
 
 	if (bts_err) {
-- 
2.51.1.838.g19442a804e-goog


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

* Re: [PATCH] perf/x86: Add cond_resched() when allocate/release DS buffers
  2025-10-27 21:27 [PATCH] perf/x86: Add cond_resched() when allocate/release DS buffers Namhyung Kim
@ 2025-10-28 14:15 ` Peter Zijlstra
  2025-10-28 15:27   ` Peter Zijlstra
  0 siblings, 1 reply; 7+ messages in thread
From: Peter Zijlstra @ 2025-10-28 14:15 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Ingo Molnar, Mark Rutland, Alexander Shishkin,
	Arnaldo Carvalho de Melo, Ian Rogers, Adrian Hunter, LKML,
	Eric Dumazet

On Mon, Oct 27, 2025 at 02:27:24PM -0700, Namhyung Kim wrote:
> On large systems, it's possible to trigger sched latency warning during
> the DS buffer allocation or release.  Add cond_resched() to avoid it.

We're >.< close to deleting cond_resched(), it makes absolutely no sense
adding more.

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

* Re: [PATCH] perf/x86: Add cond_resched() when allocate/release DS buffers
  2025-10-28 14:15 ` Peter Zijlstra
@ 2025-10-28 15:27   ` Peter Zijlstra
  2025-10-28 19:02     ` Namhyung Kim
  0 siblings, 1 reply; 7+ messages in thread
From: Peter Zijlstra @ 2025-10-28 15:27 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Ingo Molnar, Mark Rutland, Alexander Shishkin,
	Arnaldo Carvalho de Melo, Ian Rogers, Adrian Hunter, LKML,
	Eric Dumazet, Thomas Gleixner, Sebastian Andrzej Siewior

On Tue, Oct 28, 2025 at 03:15:18PM +0100, Peter Zijlstra wrote:
> On Mon, Oct 27, 2025 at 02:27:24PM -0700, Namhyung Kim wrote:
> > On large systems, it's possible to trigger sched latency warning during
> > the DS buffer allocation or release.  Add cond_resched() to avoid it.
> 
> We're >.< close to deleting cond_resched(), it makes absolutely no sense
> adding more.

Specifically, IIRC the plan was to do something like the below after the
next LTS release, and then continue to remove VOLUNTARY in subsequent
releases, leaving NONE the only option for the legacy architectures that
do not support preemption.


diff --git a/kernel/Kconfig.preempt b/kernel/Kconfig.preempt
index da326800c1c9..db4ae53c1d49 100644
--- a/kernel/Kconfig.preempt
+++ b/kernel/Kconfig.preempt
@@ -16,11 +16,12 @@ config ARCH_HAS_PREEMPT_LAZY
 
 choice
 	prompt "Preemption Model"
-	default PREEMPT_NONE
+	default PREEMPT_LAZY
 
 config PREEMPT_NONE
 	bool "No Forced Preemption (Server)"
 	depends on !PREEMPT_RT
+	depends on ARCH_NO_PREEMPT
 	select PREEMPT_NONE_BUILD if !PREEMPT_DYNAMIC
 	help
 	  This is the traditional Linux preemption model, geared towards
@@ -35,8 +36,8 @@ config PREEMPT_NONE
 
 config PREEMPT_VOLUNTARY
 	bool "Voluntary Kernel Preemption (Desktop)"
-	depends on !ARCH_NO_PREEMPT
 	depends on !PREEMPT_RT
+	depends on ARCH_NO_PREEMPT
 	select PREEMPT_VOLUNTARY_BUILD if !PREEMPT_DYNAMIC
 	help
 	  This option reduces the latency of the kernel by adding more

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

* Re: [PATCH] perf/x86: Add cond_resched() when allocate/release DS buffers
  2025-10-28 15:27   ` Peter Zijlstra
@ 2025-10-28 19:02     ` Namhyung Kim
  2025-10-28 19:04       ` Peter Zijlstra
  0 siblings, 1 reply; 7+ messages in thread
From: Namhyung Kim @ 2025-10-28 19:02 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Ingo Molnar, Mark Rutland, Alexander Shishkin,
	Arnaldo Carvalho de Melo, Ian Rogers, Adrian Hunter, LKML,
	Eric Dumazet, Thomas Gleixner, Sebastian Andrzej Siewior

Hi Peter,

On Tue, Oct 28, 2025 at 04:27:47PM +0100, Peter Zijlstra wrote:
> On Tue, Oct 28, 2025 at 03:15:18PM +0100, Peter Zijlstra wrote:
> > On Mon, Oct 27, 2025 at 02:27:24PM -0700, Namhyung Kim wrote:
> > > On large systems, it's possible to trigger sched latency warning during
> > > the DS buffer allocation or release.  Add cond_resched() to avoid it.
> > 
> > We're >.< close to deleting cond_resched(), it makes absolutely no sense
> > adding more.
> 
> Specifically, IIRC the plan was to do something like the below after the
> next LTS release, and then continue to remove VOLUNTARY in subsequent
> releases, leaving NONE the only option for the legacy architectures that
> do not support preemption.

Thanks for your review!

I haven't followed the work in this area so was not aware of the
PREEMPT_LAZY.  Looks great!  I hope it'll work well on server platforms
with many batch jobs and interactive tasks.

I will drop my patch then.

Thanks,
Namhyung

> 
> 
> diff --git a/kernel/Kconfig.preempt b/kernel/Kconfig.preempt
> index da326800c1c9..db4ae53c1d49 100644
> --- a/kernel/Kconfig.preempt
> +++ b/kernel/Kconfig.preempt
> @@ -16,11 +16,12 @@ config ARCH_HAS_PREEMPT_LAZY
>  
>  choice
>  	prompt "Preemption Model"
> -	default PREEMPT_NONE
> +	default PREEMPT_LAZY
>  
>  config PREEMPT_NONE
>  	bool "No Forced Preemption (Server)"
>  	depends on !PREEMPT_RT
> +	depends on ARCH_NO_PREEMPT
>  	select PREEMPT_NONE_BUILD if !PREEMPT_DYNAMIC
>  	help
>  	  This is the traditional Linux preemption model, geared towards
> @@ -35,8 +36,8 @@ config PREEMPT_NONE
>  
>  config PREEMPT_VOLUNTARY
>  	bool "Voluntary Kernel Preemption (Desktop)"
> -	depends on !ARCH_NO_PREEMPT
>  	depends on !PREEMPT_RT
> +	depends on ARCH_NO_PREEMPT
>  	select PREEMPT_VOLUNTARY_BUILD if !PREEMPT_DYNAMIC
>  	help
>  	  This option reduces the latency of the kernel by adding more

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

* Re: [PATCH] perf/x86: Add cond_resched() when allocate/release DS buffers
  2025-10-28 19:02     ` Namhyung Kim
@ 2025-10-28 19:04       ` Peter Zijlstra
  2025-10-28 20:13         ` Sebastian Andrzej Siewior
  0 siblings, 1 reply; 7+ messages in thread
From: Peter Zijlstra @ 2025-10-28 19:04 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Ingo Molnar, Mark Rutland, Alexander Shishkin,
	Arnaldo Carvalho de Melo, Ian Rogers, Adrian Hunter, LKML,
	Eric Dumazet, Thomas Gleixner, Sebastian Andrzej Siewior

On Tue, Oct 28, 2025 at 12:02:58PM -0700, Namhyung Kim wrote:
> Hi Peter,
> 
> On Tue, Oct 28, 2025 at 04:27:47PM +0100, Peter Zijlstra wrote:
> > On Tue, Oct 28, 2025 at 03:15:18PM +0100, Peter Zijlstra wrote:
> > > On Mon, Oct 27, 2025 at 02:27:24PM -0700, Namhyung Kim wrote:
> > > > On large systems, it's possible to trigger sched latency warning during
> > > > the DS buffer allocation or release.  Add cond_resched() to avoid it.
> > > 
> > > We're >.< close to deleting cond_resched(), it makes absolutely no sense
> > > adding more.
> > 
> > Specifically, IIRC the plan was to do something like the below after the
> > next LTS release, and then continue to remove VOLUNTARY in subsequent
> > releases, leaving NONE the only option for the legacy architectures that
> > do not support preemption.
> 
> Thanks for your review!
> 
> I haven't followed the work in this area so was not aware of the
> PREEMPT_LAZY.  Looks great!  I hope it'll work well on server platforms
> with many batch jobs and interactive tasks.
> 
> I will drop my patch then.

Well, we've been trying to get people to test things... But
realistically people will only test once you force them. So we'll see.

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

* Re: [PATCH] perf/x86: Add cond_resched() when allocate/release DS buffers
  2025-10-28 19:04       ` Peter Zijlstra
@ 2025-10-28 20:13         ` Sebastian Andrzej Siewior
  2025-10-29  9:07           ` Peter Zijlstra
  0 siblings, 1 reply; 7+ messages in thread
From: Sebastian Andrzej Siewior @ 2025-10-28 20:13 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Namhyung Kim, Ingo Molnar, Mark Rutland, Alexander Shishkin,
	Arnaldo Carvalho de Melo, Ian Rogers, Adrian Hunter, LKML,
	Eric Dumazet, Thomas Gleixner

On 2025-10-28 20:04:22 [+0100], Peter Zijlstra wrote:
> Well, we've been trying to get people to test things... But
> realistically people will only test once you force them. So we'll see.

| grep CONFIG_PREEMPT_LAZY /boot/config-6.16.12+deb14+1-amd64
| CONFIG_PREEMPT_LAZY=y

Debian has it enabled by default in current testing/ unstable distro.
\o/

Sebastian


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

* Re: [PATCH] perf/x86: Add cond_resched() when allocate/release DS buffers
  2025-10-28 20:13         ` Sebastian Andrzej Siewior
@ 2025-10-29  9:07           ` Peter Zijlstra
  0 siblings, 0 replies; 7+ messages in thread
From: Peter Zijlstra @ 2025-10-29  9:07 UTC (permalink / raw)
  To: Sebastian Andrzej Siewior
  Cc: Namhyung Kim, Ingo Molnar, Mark Rutland, Alexander Shishkin,
	Arnaldo Carvalho de Melo, Ian Rogers, Adrian Hunter, LKML,
	Eric Dumazet, Thomas Gleixner

On Tue, Oct 28, 2025 at 09:13:54PM +0100, Sebastian Andrzej Siewior wrote:
> On 2025-10-28 20:04:22 [+0100], Peter Zijlstra wrote:
> > Well, we've been trying to get people to test things... But
> > realistically people will only test once you force them. So we'll see.
> 
> | grep CONFIG_PREEMPT_LAZY /boot/config-6.16.12+deb14+1-amd64
> | CONFIG_PREEMPT_LAZY=y
> 
> Debian has it enabled by default in current testing/ unstable distro.
> \o/

Yes, I noticed, thanks!

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

end of thread, other threads:[~2025-10-29  9:07 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-27 21:27 [PATCH] perf/x86: Add cond_resched() when allocate/release DS buffers Namhyung Kim
2025-10-28 14:15 ` Peter Zijlstra
2025-10-28 15:27   ` Peter Zijlstra
2025-10-28 19:02     ` Namhyung Kim
2025-10-28 19:04       ` Peter Zijlstra
2025-10-28 20:13         ` Sebastian Andrzej Siewior
2025-10-29  9:07           ` Peter Zijlstra

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