mirror of https://lore.kernel.org/lkml/
 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 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®