Message-ID: <TYAPR01MB6330B8C13D26A0B7BD3076878BC2A@TYAPR01MB6330.jpnprd01.prod.outlook.com>
Date:   Wed, 27 Sep 2023 07:38:02 +0000
From:   "Shaopeng Tan (Fujitsu)" <tan.shaopeng@...itsu.com>
To:     'James Morse' <james.morse@....com>,
        "x86@...nel.org" <x86@...nel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
CC:     Fenghua Yu <fenghua.yu@...el.com>,
        Reinette Chatre <reinette.chatre@...el.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        H Peter Anvin <hpa@...or.com>,
        Babu Moger <Babu.Moger@....com>,
        "shameerali.kolothum.thodi@...wei.com" 
        <shameerali.kolothum.thodi@...wei.com>,
        D Scott Phillips OS <scott@...amperecomputing.com>,
        "carl@...amperecomputing.com" <carl@...amperecomputing.com>,
        "lcherian@...vell.com" <lcherian@...vell.com>,
        "bobo.shaobowang@...wei.com" <bobo.shaobowang@...wei.com>,
        "xingxin.hx@...nanolis.org" <xingxin.hx@...nanolis.org>,
        "baolin.wang@...ux.alibaba.com" <baolin.wang@...ux.alibaba.com>,
        Jamie Iles <quic_jiles@...cinc.com>,
        Xin Hao <xhao@...ux.alibaba.com>,
        "peternewman@...gle.com" <peternewman@...gle.com>,
        "dfustini@...libre.com" <dfustini@...libre.com>,
        "amitsinght@...vell.com" <amitsinght@...vell.com>
Subject: RE: [PATCH v6 00/24] x86/resctrl: monitored closid+rmid together,
 separate arch/fs locking

Hello James,

I reviewed this patch series(v6) and ran resctrl selftest on 
Intel(R) Xeon(R) Gold 6254 CPU with nohz_full enabled/disabled, 
there is no problem.

<reviewed-by:tan.shaopeng@...fujitsu.com>
<tested-by:tan.shaopeng@...fujitsu.com>


> This series does two things, it changes resctrl to call resctrl_arch_rmid_read()
> in a way that works for MPAM, and it separates the locking so that the arch
> code and filesystem code don't have to share a mutex. I tried to split this as two
> series, but these touch similar call sites, so it would create more work.
> 
> (What's MPAM? See the cover letter of the first series. [1])
> 
> On x86 the RMID is an independent number. MPAMs equivalent is PMG, but
> this isn't an independent number - it extends the PARTID (same as CLOSID)
> space with bits that aren't used to select the configuration. The monitors can
> then be told to match specific PMG values, allowing monitor-groups to be
> created.
> 
> But, MPAM expects the monitors to always monitor by PARTID. The
> Cache-storage-utilisation counters can only work this way.
> (In the MPAM spec not setting the MATCH_PARTID bit is made
> CONSTRAINED UNPREDICTABLE - which is Arm's term to mean portable
> software can't rely on
> this)
> 
> It gets worse, as some SoCs may have very few PMG bits. I've seen the
> datasheet for one that has a single bit of PMG space.
> 
> To be usable, MPAM's counters always need the PARTID and the PMG.
> For resctrl, this means always making the CLOSID available when the RMID is
> used.
> 
> To ensure RMID are always unique, this series combines the CLOSID and
> RMID into an index, and manages RMID based on that. For x86, the index and
> RMID would always be the same.
> 
> 
> Currently the architecture specific code in the cpuhp callbacks takes the
> rdtgroup_mutex. This means the filesystem code would have to export this lock,
> resulting in an ill-defined interface between the two, and the possibility of
> cross-architecture lock-ordering head aches.
> 
> The second part of this series adds a domain_list_lock to protect writes to the
> domain list, and protects the domain list with RCU - or cpus_read_lock().
> 
> Use of RCU is to allow lockless readers of the domain list. To get MPAMs
> monitors working, its very likely they'll need to be plumbed up to perf. An
> uncore PMU driver would need to be a lockless reader of the domain list.
> 
> This series is based on v6.6-rc1, and can be retrieved from:
> https://git.kernel.org/pub/scm/linux/kernel/git/morse/linux.git
> mpam/monitors_and_locking/v6
> 
> Bugs welcome,
> 
> Thanks,
> 
> James
> 
> [1]
> https://lore.kernel.org/lkml/20210728170637.25610-1-james.morse@arm.com
> /
> [v1]
> https://lore.kernel.org/all/20221021131204.5581-1-james.morse@arm.com/
> [v2]
> https://lore.kernel.org/lkml/20230113175459.14825-1-james.morse@arm.com
> /
> [v3]
> https://lore.kernel.org/r/20230320172620.18254-1-james.morse@arm.com
> [v4]
> https://lore.kernel.org/r/20230525180209.19497-1-james.morse@arm.com
> [v6]
> https://lore.kernel.org/lkml/20230728164254.27562-1-james.morse@arm.com
> /
> 
> 
> James Morse (24):
>   tick/nohz: Move tick_nohz_full_mask declaration outside the #ifdef
>   x86/resctrl: kfree() rmid_ptrs from rdtgroup_exit()
>   x86/resctrl: Create helper for RMID allocation and mondata dir
>     creation
>   x86/resctrl: Move rmid allocation out of mkdir_rdt_prepare()
>   x86/resctrl: Track the closid with the rmid
>   x86/resctrl: Access per-rmid structures by index
>   x86/resctrl: Allow RMID allocation to be scoped by CLOSID
>   x86/resctrl: Track the number of dirty RMID a CLOSID has
>   x86/resctrl: Use set_bit()/clear_bit() instead of open coding
>   x86/resctrl: Allocate the cleanest CLOSID by searching
>     closid_num_dirty_rmid
>   x86/resctrl: Move CLOSID/RMID matching and setting to use helpers
>   x86/resctrl: Add cpumask_any_housekeeping() for limbo/overflow
>   x86/resctrl: Queue mon_event_read() instead of sending an IPI
>   x86/resctrl: Allow resctrl_arch_rmid_read() to sleep
>   x86/resctrl: Allow arch to allocate memory needed in
>     resctrl_arch_rmid_read()
>   x86/resctrl: Make resctrl_mounted checks explicit
>   x86/resctrl: Move alloc/mon static keys into helpers
>   x86/resctrl: Make rdt_enable_key the arch's decision to switch
>   x86/resctrl: Add helpers for system wide mon/alloc capable
>   x86/resctrl: Add CPU online callback for resctrl work
>   x86/resctrl: Allow overflow/limbo handlers to be scheduled on any-but
>     cpu
>   x86/resctrl: Add cpu offline callback for resctrl work
>   x86/resctrl: Move domain helper migration into resctrl_offline_cpu()
>   x86/resctrl: Separate arch and fs resctrl locks
> 
>  arch/x86/include/asm/resctrl.h            |  90 +++++
>  arch/x86/kernel/cpu/resctrl/core.c        |  78 ++--
>  arch/x86/kernel/cpu/resctrl/ctrlmondata.c |  47 ++-
>  arch/x86/kernel/cpu/resctrl/internal.h    |  56 ++-
>  arch/x86/kernel/cpu/resctrl/monitor.c     | 434
> +++++++++++++++++-----
>  arch/x86/kernel/cpu/resctrl/pseudo_lock.c |  15 +-
>  arch/x86/kernel/cpu/resctrl/rdtgroup.c    | 345 ++++++++++++-----
>  include/linux/resctrl.h                   |  43 ++-
>  include/linux/tick.h                      |   9 +-
>  9 files changed, 857 insertions(+), 260 deletions(-)
> 
> --
> 2.39.2

Powered by blists - more mailing lists