All of lore.kernel.org
 help / color / mirror / Atom feed
From: Shakeel Butt <shakeel.butt@linux.dev>
To: Reinette Chatre <reinette.chatre@intel.com>
Cc: Tony Luck <tony.luck@intel.com>,
	Dave Martin <Dave.Martin@arm.com>,
	 James Morse <james.morse@arm.com>,
	Babu Moger <babu.moger@amd.com>,
	 Thomas Gleixner <tglx@kernel.org>,
	Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
	 Dave Hansen <dave.hansen@linux.intel.com>,
	Peter Newman <peternewman@google.com>,
	 Meta kernel team <kernel-team@meta.com>,
	x86@kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [RFC PATCH] x86,resctrl: Read event counters with an IPI when the read cannot block
Date: Mon, 26 Oct 2026 20:15:35 -0700	[thread overview]
Message-ID: <auAOCnqJFTMQ3ZdD@linux.dev> (raw)
In-Reply-To: <c96fb51f-9ab2-42ac-81a3-a5e49c87c2b4@intel.com>

Hi Reinette,

Thanks for the detailed review and feedback and sorry for late response.

On Wed, Oct 21, 2026 at 03:58:38PM -0700, Reinette Chatre wrote:
> Hi Shakeel,
> 
> On 10/20/26 8:13 AM, Shakeel Butt wrote:
> > In the Meta fleet, readers of the resctrl mon_data files often sleep
> > while holding rdtgroup_mutex. Over several hours, a lock profiler on
> > several thousand hosts (almost all AMD) caught:
> > 
> >   - 96,368 reads that held rdtgroup_mutex and cpu_hotplug_lock for
> >     17 ms on average and up to several seconds, mostly sleeping.
> >   - Three different userspace applications [1, 2, 3] doing these reads.
> 
> This is very demanding usage of an interface that cannot and has never 
> intended to be fast.

I think I agree here and I should have been more clear about my motivation. The
actual issue I am trying to resolve is that one of the reader of this interface
(actually below) is very critical for reliability of the system (or actually
observability) and we are seeing multi-second stalls in that daemon on this
interface. This causes gaps in our observability into the issues.

Also I think the read rates are modest. Below reads the whole tree every 5
seconds and scx_layered reads one root-group file per domain every 100ms.

[...]
> > smp_call_function_any() fallback. MPAM and other architectures keep
> > the old path. Peter Newman posted a similar change in 2024 [4].
> 
> Did you consider the thread that followed that proposal to learn why it did
> not land? 
> 

Yes and I think the problem I am tring to solve is a bit different. I am ok with
spending more cpu cycles in order to avoid critical daemon getting stuck behind
lower priority readers and may get impacted by scheduling delays or priority
inversion.

> > 
> > Results on a 316-CPU AMD EPYC with busy target CPUs, and readers that
> > copy the fleet agents, per 60 s run:
> > 
> >                                        before    after
> >   hold time per read, average          350 us    4 us
> >   hold time per read, worst            3.4 ms    109 us
> >   time off CPU while holding           9 s       0
> >   one pass over 750 files, average     360 ms    7.4 ms
> 
> The numbers make the contention clear. If I got the math right the "after"
> is still about 10us per file and with 4us spent holding rdtgroup_mutex there
> is still about 6us per file spent outside resctrl. I expect this is
> taken up by the syscalls, the various kernfs allocations to manage the read
> cycle, also a seq_buf allocation of a page (just to return a few bytes of data).
> 
> I do not think the 750 file example accurately captures how "bad" things
> can get. From what I can tell this proposal only *slightly* adjusts the limit
> of how many monitor groups this deployment can support.
> 
> My guess is that the 750 file example looks like possibly about 25 monitor
> groups with three events each monitored across 10 domains.

Yes you are right. The patch does not change how a full sweep scales. It is only
changing the potential hold time of the sleeping locks and today it depends on
scheduling on another CPU which introduced a lot of uncertainty.

> 
> The "after" value indicates that this deployment can now support more monitoring
> groups. For example, 250 monitor groups with a pass over the increased number
> of files taking about 74ms.
> 
> AMD can support 4096 monitor groups though so while this change is undeniably
> an improvement, it does not address the foundational limitations of this
> interface and does not enable this deployment to use full capability of
> the systems.
> 
> If the goal is indeed to obtain accurate monitoring data at regular
> intervals when using the stated hardware at its full capacity then
> I would suggest that you consider the per-group rate files that Peter
> settled on per
> https://lore.kernel.org/all/CALPaoCgpnVORZfbKVLXDFUZvv8jhpShHPzB3cwdLTZQH1o9ULw@mail.gmail.com/
> 

I will think more about rate file interface. It seems like Peter has offered his
prototype, I will take a look.

> I actually cannot remember seeing an update on that work after he posted that
> message and my assumption has been that that is what they have been using since
> then.
> 
> Essentially the solution builds on the monitoring data already collected by the
> per-domain overflow handler, it turns that data into a rate that user space can
> read on demand without any IPI.
>  
> > The cost: interrupts are off on the reading CPU for about 0.5 us per
> > counter, and up to 131 us for a root group read with 128 MON groups.
> 
> 128 MON groups sound low - it looks like AMD has 4096 possible MON groups.
> 
> Interrupting a monitored application with interrupts off for so long
> is significant for a fleet whose product is the monitored application.
> 
> > A reader that runs every 100 ms used up to 29% more system time,
> 
> Is it really needed to get the data every 100ms? 

Most probably not. I have to check with scx_layered folks if they really need 100ms.
However below and dynolog reads less often and are impacted by stalls on this
path most of the time.

> ...
> 
> As I understand this patch slightly moves the needle on how many monitoring
> groups this deployment can support. I do not believe this patch enables this
> deployment to support all monitoring groups possible. 
> 
> Please do consider per-group rate files as discussed in the thread around [4].
> 
> Even so, surely this patch improves performance under some scenarios. We could continue
> to consider it if you still believe it useful.

I will and I still feel the rate file are complementary as the existing tools
use the mon_data files and this patch helps them.

I will address all of your suggestions/concerns on the code in the next version.

Again, thanks a lot of the detailed and helpful feedback.

      parent reply	other threads:[~2026-10-27  3:15 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-20 15:13 [RFC PATCH] x86,resctrl: Read event counters with an IPI when the read cannot block Shakeel Butt
2026-10-21 22:58 ` Reinette Chatre
2026-10-22  9:05   ` Peter Newman
2026-10-27  3:17     ` Shakeel Butt
2026-10-27  3:15   ` Shakeel Butt [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=auAOCnqJFTMQ3ZdD@linux.dev \
    --to=shakeel.butt@linux.dev \
    --cc=Dave.Martin@arm.com \
    --cc=babu.moger@amd.com \
    --cc=bp@alien8.de \
    --cc=dave.hansen@linux.intel.com \
    --cc=james.morse@arm.com \
    --cc=kernel-team@meta.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=peternewman@google.com \
    --cc=reinette.chatre@intel.com \
    --cc=tglx@kernel.org \
    --cc=tony.luck@intel.com \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.