From patchwork Wed Feb 21 19:31:18 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Luck, Tony" X-Patchwork-Id: 204373 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:a05:693c:2685:b0:108:e6aa:91d0 with SMTP id mn5csp1252603dyc; Wed, 21 Feb 2024 11:32:30 -0800 (PST) X-Forwarded-Encrypted: i=3; AJvYcCWj9XG6V0O78mN4g3SEBeJVmttrbZ4Kt0P1jYfxADv9HLyqzSrETP7Tp6WMognSd1i4104sOK0hn4uLBsahzi53IocnJw== X-Google-Smtp-Source: AGHT+IHU09wVMA7eqa9+bo8W5L1BZB1hCLrMI0RgrqE7w8U5pca0dA9otcSUvRjSuNLTaNWtd/M0 X-Received: by 2002:a17:90a:ea05:b0:29a:15c0:44bd with SMTP id w5-20020a17090aea0500b0029a15c044bdmr2494626pjy.23.1708543949960; Wed, 21 Feb 2024 11:32:29 -0800 (PST) ARC-Seal: i=2; a=rsa-sha256; t=1708543949; cv=pass; d=google.com; s=arc-20160816; b=S/lA40EMOBAQHZHXBE+/EM2b8IRMKeHr126LQLLo+/rT8gWpLC5nsF8Xz80QxWGWLA YgtdlZYCWXPS3fMRBl3aVOZ95ih5GJu1LaQMcKmTiEhwTAfWK29lDJPZba05M01lRlD1 Oq9pZYS12GxlIhxlB1iN1moHay0PdyfF0D4iMH9gnzFvvTDQia/xyf9SSRtfvnW1JAv6 EpuPq1uyUS2Ty65fp7vNsQ4d7V36qS6nzx+rZAK8GctMaug4pTjEBDjk2BF7wcY5KupO eSDDF2H5jFmNqJZih2mjJpPwPs2hWjO6i9SjZqKGnbcx8Iuay4+sHQBvok0afGWTeDzu qxzA== ARC-Message-Signature: i=2; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=in-reply-to:content-disposition:mime-version:list-unsubscribe :list-subscribe:list-id:precedence:references:message-id:subject:cc :to:from:date:dkim-signature; bh=DahXwEKpjbxLruFC92AzXEc/R6WfkjQNZwIeAtVgijg=; fh=5jCiTq7HhAqR+PIc3xWAvzl+sHpYYjU7s1H4a0b/Tzk=; b=vzHX+iqMdEKVCi6V+nr3zYH0iUpmBgCWCs+TBAw24CdblMB22qUDoCLn0LV+0ERf/4 YRyYUn7BR9RF1hmh9TDbKacuMFat1fwL3uA8xfYOidLmOHXa5unf5m7WeMNHbyivioYj OzFiFDiSmWhmcVoEViLLIXyVXzNl9ur9744G5t3lwhCurICDoAD2R0y6s4sSLOiqmIT/ hwAo9N2aK19gahfy4cxeuT/0bICTu2eTxwCor0eRNjxTvQ9A2v58jxtqaiffOjqGvaB1 IX8pG6+eS5TS/RiEbff9DKlkJyquoUTsvOgcOEAFZJ3JbMr55ixSGdNfA3W5m3hOl7rm PYIg==; dara=google.com ARC-Authentication-Results: i=2; mx.google.com; dkim=pass header.i=@intel.com header.s=Intel header.b=j1pYQ1WG; arc=pass (i=1 spf=pass spfdomain=intel.com dkim=pass dkdomain=intel.com dmarc=pass fromdomain=intel.com); spf=pass (google.com: domain of linux-kernel+bounces-75369-ouuuleilei=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) smtp.mailfrom="linux-kernel+bounces-75369-ouuuleilei=gmail.com@vger.kernel.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=intel.com Received: from sv.mirrors.kernel.org (sv.mirrors.kernel.org. [139.178.88.99]) by mx.google.com with ESMTPS id o16-20020a17090ac71000b002990da768c0si1987873pjt.174.2024.02.21.11.32.29 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 21 Feb 2024 11:32:29 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel+bounces-75369-ouuuleilei=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99; Authentication-Results: mx.google.com; dkim=pass header.i=@intel.com header.s=Intel header.b=j1pYQ1WG; arc=pass (i=1 spf=pass spfdomain=intel.com dkim=pass dkdomain=intel.com dmarc=pass fromdomain=intel.com); spf=pass (google.com: domain of linux-kernel+bounces-75369-ouuuleilei=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) smtp.mailfrom="linux-kernel+bounces-75369-ouuuleilei=gmail.com@vger.kernel.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=intel.com Received: from smtp.subspace.kernel.org (wormhole.subspace.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by sv.mirrors.kernel.org (Postfix) with ESMTPS id 8E8AF28937C for ; Wed, 21 Feb 2024 19:31:36 +0000 (UTC) Received: from localhost.localdomain (localhost.localdomain [127.0.0.1]) by smtp.subspace.kernel.org (Postfix) with ESMTP id CF9538615F; Wed, 21 Feb 2024 19:31:26 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=intel.com header.i=@intel.com header.b="j1pYQ1WG" Received: from mgamail.intel.com (mgamail.intel.com [198.175.65.13]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id AC43583CB6 for ; Wed, 21 Feb 2024 19:31:20 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=198.175.65.13 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1708543882; cv=none; b=ZEa8lnpSkRfyD+VxqF49gDOTwkrOMNL52fE5xK2nYQaJ5gvfYPuyiXH6jLb0f0ikfGuoqoZJFbqiHeB27lW8+SrhHw2nnmePiQNfegxy8fAMlVJ4AYesNr1bSaCKfQkHJnKpeo2hMQzEsI0zE2Xc0P5Ju3BVk3rahJceSQcXVuo= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1708543882; c=relaxed/simple; bh=h7rP+mzEg4pomwZyzq/B3UltLYapvjHuIFCRs+4UPaQ=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=Pgh/RcaobYFLB2HTTm9I8kW1WwMlBFI87XzvG/3q6v4+GcdpX35ZIXW4NUXGm2Ls+r6v1asYCHe5uT5K8QT0jaGhjQDTKtcKH3nun2mZli+KK9c64yGzcCoreEEhi4PEwTkHb4kRnvVP6IcMSf2shoo3lVmYrmwT6mSyK6N56cU= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=intel.com; spf=pass smtp.mailfrom=intel.com; dkim=pass (2048-bit key) header.d=intel.com header.i=@intel.com header.b=j1pYQ1WG; arc=none smtp.client-ip=198.175.65.13 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=intel.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=intel.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1708543880; x=1740079880; h=date:from:to:cc:subject:message-id:references: mime-version:in-reply-to; bh=h7rP+mzEg4pomwZyzq/B3UltLYapvjHuIFCRs+4UPaQ=; b=j1pYQ1WGRJzbX52U7UOpyTml9rzH63sFqW+5nFMdb2qYDQHBYQZaveTM 2iDNw0LzMxcgGz4TDMUvOkeuR7jasvmRRHy0ssa3MJQxXiv+u8T+bU/uv m7n0emrgbhVzG1/VV5dRiwM9WuQS5Hcr6Ix34Z62XEeh4S82hGjetOUTp 8xNJPuxyoduhdkRJ+cH3iojmlxgeA0pa4omdH/k32AF7Z5FMU3Akp03L4 3X/88Gm22U25JhSQm7/F/QKLD4QhiedaCZIg4ipeWbhlscJxQomtyGcBs VOArVAf9pPG3SkV7s/9mWQvOSyDDJIs/OioYdvyH1Z0fbd5og37A119iN w==; X-IronPort-AV: E=McAfee;i="6600,9927,10991"; a="13855483" X-IronPort-AV: E=Sophos;i="6.06,176,1705392000"; d="scan'208";a="13855483" Received: from fmviesa008.fm.intel.com ([10.60.135.148]) by orvoesa105.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 21 Feb 2024 11:31:20 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.06,176,1705392000"; d="scan'208";a="5395198" Received: from agluck-desk3.sc.intel.com (HELO agluck-desk3) ([172.25.222.74]) by fmviesa008-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 21 Feb 2024 11:31:19 -0800 Date: Wed, 21 Feb 2024 11:31:18 -0800 From: Tony Luck To: "Chatre, Reinette" , Thomas Gleixner Cc: Borislav Petkov , James Morse , "x86@kernel.org" , "linux-kernel@vger.kernel.org" , "Yu, Fenghua" , Ingo Molnar , H Peter Anvin , Babu Moger , "shameerali.kolothum.thodi@huawei.com" , D Scott Phillips OS , "carl@os.amperecomputing.com" , "lcherian@marvell.com" , "bobo.shaobowang@huawei.com" , "tan.shaopeng@fujitsu.com" , "baolin.wang@linux.alibaba.com" , Jamie Iles , Xin Hao , "peternewman@google.com" , "dfustini@baylibre.com" , "amitsinght@marvell.com" , David Hildenbrand Subject: [PATCH v2] x86/resctrl: Fix WARN in get_domain_from_cpu() Message-ID: References: <20240213184438.16675-1-james.morse@arm.com> <20240217105543.GAZdCQr_nosDP4tGuO@fat_crate.local> <87zfvwieli.ffs@tglx> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1791466668631806912 X-GMAIL-MSGID: 1791538181009842847 reset_all_ctrls() and resctrl_arch_update_domains() use on_each_cpu_mask() to call rdt_ctrl_update() on potentially one CPU from each domain. But this means rdt_ctrl_update() needs to figure out which domain to apply changes to. Doing so requires a search of all domains in a resource, which can only be done safely if cpus_lock is held. Both callers do hold this lock, but there isn't a way for a function called on another CPU via IPI to verify this. Fix by adding the target domain to the msr_param structure and passing an array with CDP_NUM_TYPES entries. Then calling for each domain separately using smp_call_function_single() Change the low level cat_wrmsr(), mba_wrmsr_intel(), and mba_wrmsr_amd() functions to just take a msr_param structure since it contains the rdt_resource and rdt_domain information. Signed-off-by: Tony Luck --- Changes since v1: * Avoid double IPI to the same core when CDP is enabled. * Don't pass the resource and domain to functions that can get these from the msr_param structure. * Clean up some fir tree issues in functions that I changed. arch/x86/kernel/cpu/resctrl/internal.h | 4 +- arch/x86/kernel/cpu/resctrl/core.c | 56 +++++++++------------ arch/x86/kernel/cpu/resctrl/ctrlmondata.c | 59 +++++++---------------- arch/x86/kernel/cpu/resctrl/rdtgroup.c | 23 ++++----- 4 files changed, 50 insertions(+), 92 deletions(-) diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h index c99f26ebe7a6..2f21358b9621 100644 --- a/arch/x86/kernel/cpu/resctrl/internal.h +++ b/arch/x86/kernel/cpu/resctrl/internal.h @@ -383,6 +383,7 @@ static inline struct rdt_hw_domain *resctrl_to_arch_dom(struct rdt_domain *r) */ struct msr_param { struct rdt_resource *res; + struct rdt_domain *dom; u32 low; u32 high; }; @@ -442,8 +443,7 @@ struct rdt_hw_resource { struct rdt_resource r_resctrl; u32 num_closid; unsigned int msr_base; - void (*msr_update) (struct rdt_domain *d, struct msr_param *m, - struct rdt_resource *r); + void (*msr_update)(struct msr_param *m); unsigned int mon_scale; unsigned int mbm_width; unsigned int mbm_cfg_mask; diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c index 8a4ef4f5bddc..643bf64bf1be 100644 --- a/arch/x86/kernel/cpu/resctrl/core.c +++ b/arch/x86/kernel/cpu/resctrl/core.c @@ -56,14 +56,9 @@ int max_name_width, max_data_width; */ bool rdt_alloc_capable; -static void -mba_wrmsr_intel(struct rdt_domain *d, struct msr_param *m, - struct rdt_resource *r); -static void -cat_wrmsr(struct rdt_domain *d, struct msr_param *m, struct rdt_resource *r); -static void -mba_wrmsr_amd(struct rdt_domain *d, struct msr_param *m, - struct rdt_resource *r); +static void mba_wrmsr_intel(struct msr_param *m); +static void cat_wrmsr(struct msr_param *m); +static void mba_wrmsr_amd(struct msr_param *m); #define domain_init(id) LIST_HEAD_INIT(rdt_resources_all[id].r_resctrl.domains) @@ -309,12 +304,11 @@ static void rdt_get_cdp_l2_config(void) rdt_get_cdp_config(RDT_RESOURCE_L2); } -static void -mba_wrmsr_amd(struct rdt_domain *d, struct msr_param *m, struct rdt_resource *r) +static void mba_wrmsr_amd(struct msr_param *m) { + struct rdt_hw_resource *hw_res = resctrl_to_arch_res(m->res); + struct rdt_hw_domain *hw_dom = resctrl_to_arch_dom(m->dom); unsigned int i; - struct rdt_hw_domain *hw_dom = resctrl_to_arch_dom(d); - struct rdt_hw_resource *hw_res = resctrl_to_arch_res(r); for (i = m->low; i < m->high; i++) wrmsrl(hw_res->msr_base + i, hw_dom->ctrl_val[i]); @@ -334,25 +328,22 @@ static u32 delay_bw_map(unsigned long bw, struct rdt_resource *r) return r->default_ctrl; } -static void -mba_wrmsr_intel(struct rdt_domain *d, struct msr_param *m, - struct rdt_resource *r) +static void mba_wrmsr_intel(struct msr_param *m) { + struct rdt_hw_resource *hw_res = resctrl_to_arch_res(m->res); + struct rdt_hw_domain *hw_dom = resctrl_to_arch_dom(m->dom); unsigned int i; - struct rdt_hw_domain *hw_dom = resctrl_to_arch_dom(d); - struct rdt_hw_resource *hw_res = resctrl_to_arch_res(r); /* Write the delay values for mba. */ for (i = m->low; i < m->high; i++) - wrmsrl(hw_res->msr_base + i, delay_bw_map(hw_dom->ctrl_val[i], r)); + wrmsrl(hw_res->msr_base + i, delay_bw_map(hw_dom->ctrl_val[i], m->res)); } -static void -cat_wrmsr(struct rdt_domain *d, struct msr_param *m, struct rdt_resource *r) +static void cat_wrmsr(struct msr_param *m) { + struct rdt_hw_resource *hw_res = resctrl_to_arch_res(m->res); + struct rdt_hw_domain *hw_dom = resctrl_to_arch_dom(m->dom); unsigned int i; - struct rdt_hw_domain *hw_dom = resctrl_to_arch_dom(d); - struct rdt_hw_resource *hw_res = resctrl_to_arch_res(r); for (i = m->low; i < m->high; i++) wrmsrl(hw_res->msr_base + i, hw_dom->ctrl_val[i]); @@ -387,19 +378,14 @@ u32 resctrl_arch_get_num_closid(struct rdt_resource *r) void rdt_ctrl_update(void *arg) { + struct rdt_hw_resource *hw_res; struct msr_param *m = arg; - struct rdt_hw_resource *hw_res = resctrl_to_arch_res(m->res); - struct rdt_resource *r = m->res; - int cpu = smp_processor_id(); - struct rdt_domain *d; + int t; - d = get_domain_from_cpu(cpu, r); - if (d) { - hw_res->msr_update(d, m, r); - return; - } - pr_warn_once("cpu %d not found in any domain for resource %s\n", - cpu, r->name); + hw_res = resctrl_to_arch_res(m->res); + for (t = 0; t < CDP_NUM_TYPES; t++) + if (m[t].dom) + hw_res->msr_update(m + t); } /* @@ -472,9 +458,11 @@ static int domain_setup_ctrlval(struct rdt_resource *r, struct rdt_domain *d) hw_dom->ctrl_val = dc; setup_default_ctrlval(r, dc); + m.res = r; + m.dom = d; m.low = 0; m.high = hw_res->num_closid; - hw_res->msr_update(d, &m, r); + hw_res->msr_update(&m); return 0; } diff --git a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c index 7997b47743a2..09f6e624f1bb 100644 --- a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c +++ b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c @@ -272,22 +272,6 @@ static u32 get_config_index(u32 closid, enum resctrl_conf_type type) } } -static bool apply_config(struct rdt_hw_domain *hw_dom, - struct resctrl_staged_config *cfg, u32 idx, - cpumask_var_t cpu_mask) -{ - struct rdt_domain *dom = &hw_dom->d_resctrl; - - if (cfg->new_ctrl != hw_dom->ctrl_val[idx]) { - cpumask_set_cpu(cpumask_any(&dom->cpu_mask), cpu_mask); - hw_dom->ctrl_val[idx] = cfg->new_ctrl; - - return true; - } - - return false; -} - int resctrl_arch_update_one(struct rdt_resource *r, struct rdt_domain *d, u32 closid, enum resctrl_conf_type t, u32 cfg_val) { @@ -304,59 +288,50 @@ int resctrl_arch_update_one(struct rdt_resource *r, struct rdt_domain *d, msr_param.res = r; msr_param.low = idx; msr_param.high = idx + 1; - hw_res->msr_update(d, &msr_param, r); + hw_res->msr_update(&msr_param); return 0; } int resctrl_arch_update_domains(struct rdt_resource *r, u32 closid) { + struct msr_param msr_param[CDP_NUM_TYPES]; struct resctrl_staged_config *cfg; struct rdt_hw_domain *hw_dom; - struct msr_param msr_param; enum resctrl_conf_type t; - cpumask_var_t cpu_mask; struct rdt_domain *d; + bool need_update; + int cpu; u32 idx; /* Walking r->domains, ensure it can't race with cpuhp */ lockdep_assert_cpus_held(); - if (!zalloc_cpumask_var(&cpu_mask, GFP_KERNEL)) - return -ENOMEM; - - msr_param.res = NULL; + memset(msr_param, 0, sizeof(msr_param)); list_for_each_entry(d, &r->domains, list) { hw_dom = resctrl_to_arch_dom(d); + need_update = false; for (t = 0; t < CDP_NUM_TYPES; t++) { cfg = &hw_dom->d_resctrl.staged_config[t]; if (!cfg->have_new_ctrl) continue; idx = get_config_index(closid, t); - if (!apply_config(hw_dom, cfg, idx, cpu_mask)) + if (cfg->new_ctrl == hw_dom->ctrl_val[idx]) continue; - - if (!msr_param.res) { - msr_param.low = idx; - msr_param.high = msr_param.low + 1; - msr_param.res = r; - } else { - msr_param.low = min(msr_param.low, idx); - msr_param.high = max(msr_param.high, idx + 1); - } + hw_dom->ctrl_val[idx] = cfg->new_ctrl; + cpu = cpumask_any(&d->cpu_mask); + + msr_param[t].low = idx; + msr_param[t].high = msr_param[t].low + 1; + msr_param[t].res = r; + msr_param[t].dom = d; + need_update = true; } + if (need_update) + smp_call_function_single(cpu, rdt_ctrl_update, &msr_param, 1); } - if (cpumask_empty(cpu_mask)) - goto done; - - /* Update resource control msr on all the CPUs. */ - on_each_cpu_mask(cpu_mask, rdt_ctrl_update, &msr_param, 1); - -done: - free_cpumask_var(cpu_mask); - return 0; } diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c index 011e17efb1a6..5d9ff8883c60 100644 --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c @@ -2811,21 +2811,19 @@ static int rdt_init_fs_context(struct fs_context *fc) static int reset_all_ctrls(struct rdt_resource *r) { struct rdt_hw_resource *hw_res = resctrl_to_arch_res(r); + struct msr_param msr_param[CDP_NUM_TYPES]; struct rdt_hw_domain *hw_dom; - struct msr_param msr_param; - cpumask_var_t cpu_mask; struct rdt_domain *d; + int cpu; int i; /* Walking r->domains, ensure it can't race with cpuhp */ lockdep_assert_cpus_held(); - if (!zalloc_cpumask_var(&cpu_mask, GFP_KERNEL)) - return -ENOMEM; - - msr_param.res = r; - msr_param.low = 0; - msr_param.high = hw_res->num_closid; + memset(msr_param, 0, sizeof(msr_param)); + msr_param[0].res = r; + msr_param[0].low = 0; + msr_param[0].high = hw_res->num_closid; /* * Disable resource control for this resource by setting all @@ -2834,17 +2832,14 @@ static int reset_all_ctrls(struct rdt_resource *r) */ list_for_each_entry(d, &r->domains, list) { hw_dom = resctrl_to_arch_dom(d); - cpumask_set_cpu(cpumask_any(&d->cpu_mask), cpu_mask); + cpu = cpumask_any(&d->cpu_mask); for (i = 0; i < hw_res->num_closid; i++) hw_dom->ctrl_val[i] = r->default_ctrl; + msr_param[0].dom = d; + smp_call_function_single(cpu, rdt_ctrl_update, &msr_param, 1); } - /* Update CBM on all the CPUs in cpu_mask */ - on_each_cpu_mask(cpu_mask, rdt_ctrl_update, &msr_param, 1); - - free_cpumask_var(cpu_mask); - return 0; }