From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) (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 BF00B43F8A3 for ; Wed, 15 Jul 2026 10:22:46 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=170.10.129.124 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1784110968; cv=none; b=h4mHSINUmWTeWddJlRtF7vQCaDfCsoI4injYTD1baPt58utqeJeT4VF3aIQ52HgUedBF56/S4G6le/WPGOD5G2GsFRZ4vhANO8En1w3e4WBXQGp3dW8uGGKK4eo4/HG5MiSoQc+FDXSW8Nxn3BjyH5QdqnzOim5KTUGQPqZpzM8= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1784110968; c=relaxed/simple; bh=kvT1v3/3/xTnEODdzRicvmrgkte4P6mzXO32yHYah6Q=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=g6iu/wmHpPzB1HCuolrfkJ0qAd3h7OHzcc9rh5B4dUaioGQvWtDSLd2SegyRZUN87mnCcau+fbrAVE8PiBuihQhH9yx4ueq5YMvevjodZkdZCBNEfq5UApq2tX0YN88zbCSekiylTGVvouThr1byphFZ8GQuNiGjUutZkyOeDeE= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=redhat.com; spf=pass smtp.mailfrom=redhat.com; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b=QTNOwnW1; dkim=pass (2048-bit key) header.d=redhat.com header.i=@redhat.com header.b=OFAk26OT; arc=none smtp.client-ip=170.10.129.124 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=redhat.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=redhat.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="QTNOwnW1"; dkim=pass (2048-bit key) header.d=redhat.com header.i=@redhat.com header.b="OFAk26OT" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1784110965; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=vgKQijptlsuzxuAX5DklXoNvmIiuH/6P/5aISzNMB0c=; b=QTNOwnW1WJsDdqnq6ZPn7FaYPtR4IUbGq/WiIMnXwqW9A1N8ijB9EX3eiAbWq5J+a7zCtI 1Uh8aIXgb/NpkcIzX6XH8/GHBgsqicjjJloC2qslQzchNt2CP87EWi+qXGk8JpLjRZ4ome HCT2/fgPYOXGPjjD6jpFEM5xOz8t++A= Received: from mail-pf1-f199.google.com (mail-pf1-f199.google.com [209.85.210.199]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-557-xsG55bZxNXm3qtfP5rxtlg-1; Wed, 15 Jul 2026 06:22:43 -0400 X-MC-Unique: xsG55bZxNXm3qtfP5rxtlg-1 X-Mimecast-MFC-AGG-ID: xsG55bZxNXm3qtfP5rxtlg_1784110963 Received: by mail-pf1-f199.google.com with SMTP id d2e1a72fcca58-84a4d8fd6deso2744795b3a.3 for ; Wed, 15 Jul 2026 03:22:43 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=google; t=1784110963; x=1784715763; darn=vger.kernel.org; h=content-transfer-encoding:content-type:in-reply-to:from :content-language:references:cc:to:subject:user-agent:mime-version :date:message-id:from:to:cc:subject:date:message-id:reply-to :content-type; bh=vgKQijptlsuzxuAX5DklXoNvmIiuH/6P/5aISzNMB0c=; b=OFAk26OTzMDJZEbKyvH5dRYOd2Zv6z9RxY1c6BheGfc5hmXhtqPpJa+O8tF3FBQ+ff A/uT1Rx+ks2eHHNvkZjmX634620+jWUtWvSzBrrzPWtxIUHxAIlCZX4ocmJYCVCo1ZIv acqjuvL5OcYvwI7JkWt4olJz/qjXblPEMLCNZdTMblSdQkwhiD0y+o6hwf7Cd5bzzSdk Uz8F5XkE0MlBQWOnReTuKxQWd/p12AwpTGmFomcebAOX1P4p0I9IJKMUwXB9No1pJs7Q CE+Afj0esNKudh/6+dTOExItbhZTs8AuNjzW8WRxrqXAht1Ina38fkRjWMtFE+vL+Tbo 2hIg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1784110963; x=1784715763; h=content-transfer-encoding:content-type:in-reply-to:from :content-language:references:cc:to:subject:user-agent:mime-version :date:message-id:x-gm-gg:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to:content-type; bh=vgKQijptlsuzxuAX5DklXoNvmIiuH/6P/5aISzNMB0c=; b=geXGE7cVysL04cvBswWyK/7rpMl9lXMIMDT1iVwSgGUmePEMneNJWvrMMg0t/n77tX Wsx1owRep4cscNy15vfYFcg50gSuwg4iHyC/vUVSckbTt0QwVQrAVcqaernYUmdKtrJe SRJ4kt7RuMHRCyQIgvv6NynMFULIJ3KiSJa6c7LOKKAs244dHVz7d+on2lcU52XhUJPL 934EsSj9SzCBiaOjX/LORhxfeKQZCtyJ37zCFH7cz8uJMjwdLiYIloXPE8oXmU/OmWRX YQg0EjMKbz6mmi8wM7NUrmcmktTaEpJxpUZOr9B48RxJ8mWksma8PP4+24U2yY/hjSX9 dNMg== X-Forwarded-Encrypted: i=1; AHgh+RpJu48XcluePUGUiglM0kNFSJKcNY77nszy4dFkqIGBCeitN10rsACCQvnAgVhU0BSdod8/WsOJTvTspXA=@vger.kernel.org X-Gm-Message-State: AOJu0YwqImYBgVNgRNQHyW4m/x6qhBGoMBBQ5Uc34pChyj97/eq2NzS2 to52Dea5gBXGG0bGeseidEpBq4b2GyWJdLB1wVTQboKNVIPgnPZqgFM/mPFzMuRAWNshlU5dhtj lQiNvvA3QQ958O5Esy4qgnNGvSngE1RAaIfOArcm4rseFS7hxIFJAn+6d4N96UxepHg== X-Gm-Gg: AfdE7cnAB2atCGvyMQW+c48Gd9oxUklTWvWdYI5XC8btYyYM8QNpvgXahGt4wdiGaVa 2kJtvRimZAyvm0jMy0To4C79dQFAhu5Cz281lduNjrexLa1QVf+Q3xeUi6FIkcPmYr0guu7TBvp r7gXltJZZGZaoFpZ8Hw78MMFOYGnaz3YTWWVCyCx5A8ESrgppxEJWY7p3nwoJqJ0v9r7V2Qyk/M 7/RqZL6JMvcsSjdLBgyw0mTEh79AUqZ21VOPg7SWSfgJQSuL0KpVsXaupUDSlkIT0DEOYg0sPof GzHnugdhradX9dW4HaUsAMoUnWqaCFmHuqCgHyToGEQW0SDl62fz06jPP7wPt1PsPlwZAh6TwC6 aWmqlYmofZFcIoP0U7bcGZ8YwQPzgsCHscUWJcyh6vglJ3XEXyTTHCg== X-Received: by 2002:a05:6a00:3698:b0:845:a435:a2eb with SMTP id d2e1a72fcca58-848897166aamr15053714b3a.31.1784110962686; Wed, 15 Jul 2026 03:22:42 -0700 (PDT) X-Received: by 2002:a05:6a00:3698:b0:845:a435:a2eb with SMTP id d2e1a72fcca58-848897166aamr15053690b3a.31.1784110962194; Wed, 15 Jul 2026 03:22:42 -0700 (PDT) Received: from [192.168.68.51] (n175-34-8-244.mrk21.qld.optusnet.com.au. [175.34.8.244]) by smtp.gmail.com with ESMTPSA id d2e1a72fcca58-84a4f818eaesm2925654b3a.58.2026.07.15.03.22.28 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 15 Jul 2026 03:22:39 -0700 (PDT) Message-ID: Date: Wed, 15 Jul 2026 20:22:26 +1000 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v5 2/5] arm_mpam: resctrl: Pre-allocate assignable monitors To: Ben Horgan Cc: amitsinght@marvell.com, baisheng.gao@unisoc.com, baolin.wang@linux.alibaba.com, carl@os.amperecomputing.com, dave.martin@arm.com, david@kernel.org, dfustini@baylibre.com, fenghuay@nvidia.com, james.morse@arm.com, jic23@kernel.org, kobak@nvidia.com, lcherian@marvell.com, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, peternewman@google.com, punit.agrawal@oss.qualcomm.com, quic_jiles@quicinc.com, reinette.chatre@intel.com, rohit.mathew@arm.com, scott@os.amperecomputing.com, sdonthineni@nvidia.com, tan.shaopeng@fujitsu.com, xhao@linux.alibaba.com, zengheng4@huawei.com, x86@kernel.org, leitao@kernel.org, kas@kernel.org, puranjay@kernel.org, usama.arif@linux.dev, kernel-team@meta.com, Shaopeng Tan References: <20260707162440.12132-1-ben.horgan@arm.com> <20260707162440.12132-3-ben.horgan@arm.com> Content-Language: en-US From: Gavin Shan In-Reply-To: <20260707162440.12132-3-ben.horgan@arm.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit On 7/8/26 2:24 AM, Ben Horgan wrote: > MPAM is able to emulate ABMC, i.e. mbm_event mode, by making memory > bandwidth monitors assignable. Rather than supporting the 'default' > mbm_assign_mode always use 'mbm_event' mode even if there are sufficient > memory bandwidth monitors. The per monitor event configuration is only > provided by resctrl when in 'mbm_event' mode and so only allowing > 'mbm_event' mode will make it easier to support per-monitor event > configuration for MPAM. For the moment, the only event supported is > mbm_total_event with no bandwidth type configuration. The 'mbm_assign_mode' > file will still show 'default' when there is no support for memory > bandwidth monitoring. > > The monitors need to be allocated from the driver, and mapped to whichever > control/monitor group resctrl wants to use them with. > > Add a second array to hold the monitor values indexed by resctrl's cntr_id. > > When CDP is in use, two monitors are needed so the available number of > counters halves. Platforms with one monitor will have zero monitors when > CDP is in use. > > Co-developed-by: James Morse > Signed-off-by: James Morse > Signed-off-by: Ben Horgan > Tested-by: Shaopeng Tan > Tested-by: Fenghua Yu > Reviewed-by: Shaopeng Tan > Reviewed-by: Fenghua Yu > --- > Changes since rfc v1: > abmc enabled even if enough counters > Helpers from dropped free running commits > carry on with zero counters if using cdp > set config bits > use kmalloc_objs > drop tags for rework > Configure mbm_cntr_configurable, mbm_cntr_assign_fixed > > Changes since rfc v2: > Don't set mon->assigned_counters to an error pointer > Fix mpam_resctrl_teardown_mon() > Remove free running check > Separate cleanup allocations, e.g. __free(), from the rest > Restrict scope on err in mpam_resctrl_monitor_init() > > Changes since v3: > Correct NULL check in mpam_resctrl_teardown_mon() (Shaopeng) > variable allocation ordering in mpam_resctrl_pick_domain_id() (Shaopeng) > Move mon.* assignments from mpam_resctrl_monitor_sync_abmc_vals() > to mpam_resctrl_monitor_init_abmc() counters (Sashiko) > use kvmalloc_obj() for allocations that may be big on some > platforms (Sashiko) > > Changes since v4: > Remove l3_num_allocated_mbwu (Koba) > indexed by idx -> indexed by resctrl's cntr_id (Fenghua) > --- > drivers/resctrl/mpam_internal.h | 6 +- > drivers/resctrl/mpam_resctrl.c | 143 +++++++++++++++++++++++++++++++- > 2 files changed, 145 insertions(+), 4 deletions(-) > Some nitpicks below, this looks good in either way: Reviewed-by: Gavin Shan > diff --git a/drivers/resctrl/mpam_internal.h b/drivers/resctrl/mpam_internal.h > index 04d1a59f02af..def0e3a65c23 100644 > --- a/drivers/resctrl/mpam_internal.h > +++ b/drivers/resctrl/mpam_internal.h > @@ -409,7 +409,11 @@ struct mpam_resctrl_res { > struct mpam_resctrl_mon { > struct mpam_class *class; > > - /* per-class data that resctrl needs will live here */ > + /* Array of allocated MBWU monitors, indexed by (closid, rmid). */ > + int *mbwu_idx_to_mon; > + > + /* Array of assigned MBWU monitors, indexed by resctrl's cntr_id. */ > + int *assigned_counters; > }; > > static inline int mpam_alloc_csu_mon(struct mpam_class *class) > diff --git a/drivers/resctrl/mpam_resctrl.c b/drivers/resctrl/mpam_resctrl.c > index 0fce9703b869..91b589b110a2 100644 > --- a/drivers/resctrl/mpam_resctrl.c > +++ b/drivers/resctrl/mpam_resctrl.c > @@ -140,7 +140,7 @@ int resctrl_arch_cntr_read(struct rdt_resource *r, struct rdt_l3_mon_domain *d, > > bool resctrl_arch_mbm_cntr_assign_enabled(struct rdt_resource *r) > { > - return false; > + return (r == &mpam_resctrl_controls[RDT_RESOURCE_L3].resctrl_res); > } > > int resctrl_arch_mbm_cntr_assign_set(struct rdt_resource *r, bool enable) > @@ -185,6 +185,26 @@ static void resctrl_reset_task_closids(void) > read_unlock(&tasklist_lock); > } > > +static void mpam_resctrl_monitor_sync_abmc_vals(struct rdt_resource *l3) > +{ > + struct mpam_resctrl_mon *mon = &mpam_resctrl_counters[QOS_L3_MBM_TOTAL_EVENT_ID]; > + > + if (!mon->class) > + return; > + > + if (!mon->assigned_counters) > + return; > + > + l3->mon.num_mbm_cntrs = mon->class->props.num_mbwu_mon; > + if (cdp_enabled) > + l3->mon.num_mbm_cntrs /= 2; > + > + /* > + * Continue as normal even if enabling cdp causes there to be > + * zero counters. This avoids giving resctrl mixed messages. > + */ > +} > + > int resctrl_arch_set_cdp_enabled(enum resctrl_res_level rid, bool enable) > { > u32 partid_i = RESCTRL_RESERVED_CLOSID, partid_d = RESCTRL_RESERVED_CLOSID; > @@ -244,6 +264,7 @@ int resctrl_arch_set_cdp_enabled(enum resctrl_res_level rid, bool enable) > WRITE_ONCE(arm64_mpam_global_default, mpam_get_regval(current)); > > resctrl_reset_task_closids(); > + mpam_resctrl_monitor_sync_abmc_vals(l3); > > for_each_possible_cpu(cpu) > mpam_set_cpu_defaults(cpu, partid_d, partid_i, 0, 0); > @@ -613,6 +634,9 @@ static bool class_has_usable_mbwu(struct mpam_class *class) > if (!mpam_has_feature(mpam_feat_msmon_mbwu, cprops)) > return false; > > + if (!cprops->num_mbwu_mon) > + return false; > + > return true; > } > > @@ -935,6 +959,50 @@ static void mpam_resctrl_pick_mba(void) > } > } > > +static void __free_mbwu_mon(struct mpam_class *class, int *array, > + u16 num_mbwu_mon) > +{ > + for (int i = 0; i < num_mbwu_mon; i++) { > + if (array[i] < 0) > + continue; > + > + mpam_free_mbwu_mon(class, array[i]); > + array[i] = ~0; s/~0/-1. In long run, we may need a macro for this: #define MPAM_MBWU_MON_INVALID_ID (-1) > + } > +} > + > +static int __alloc_mbwu_mon(struct mpam_class *class, int *array, > + u16 num_mbwu_mon) > +{ > + for (int i = 0; i < num_mbwu_mon; i++) { > + int mbwu_mon = mpam_alloc_mbwu_mon(class); > + > + if (mbwu_mon < 0) { > + __free_mbwu_mon(class, array, num_mbwu_mon); > + return mbwu_mon; > + } > + array[i] = mbwu_mon; > + } > + > + return 0; > +} > + > +static int *__alloc_mbwu_array(struct mpam_class *class, u16 num_mbwu_mon) > +{ > + int err; > + > + int *array __free(kvfree) = kvmalloc_objs(*array, num_mbwu_mon); > + if (!array) > + return ERR_PTR(-ENOMEM); > + > + memset(array, -1, num_mbwu_mon * sizeof(*array)); This looks a bit subtle. array[i] is split into 4-bytes and each of them is filled with -1 (0xff), the combined output is correct that array[i] == -1. > + > + err = __alloc_mbwu_mon(class, array, num_mbwu_mon); > + if (err) > + return ERR_PTR(err); > + return_ptr(array); > +} > + > static void counter_update_class(enum resctrl_event_id evt_id, > struct mpam_class *class) > { > @@ -1091,6 +1159,43 @@ static int mpam_resctrl_pick_domain_id(int cpu, struct mpam_component *comp) > return comp->comp_id; > } > > +/* > + * This must run after all event counters have been picked so that any free > + * running counters have already been allocated. > + */ > +static int mpam_resctrl_monitor_init_abmc(struct mpam_resctrl_mon *mon) > +{ > + struct mpam_resctrl_res *res = &mpam_resctrl_controls[RDT_RESOURCE_L3]; > + size_t num_rmid = resctrl_arch_system_num_rmid_idx(); > + struct rdt_resource *l3 = &res->resctrl_res; > + struct mpam_class *class = mon->class; > + u16 num_mbwu_mon; > + int *cntrs; > + > + int *rmid_array __free(kvfree) = kvmalloc_objs(*rmid_array, num_rmid); > + if (!rmid_array) { > + pr_debug("Failed to allocate RMID array\n"); > + return -ENOMEM; > + } > + memset(rmid_array, -1, num_rmid * sizeof(*rmid_array)); > + Same as above. > + num_mbwu_mon = class->props.num_mbwu_mon; > + cntrs = __alloc_mbwu_array(mon->class, num_mbwu_mon); > + if (IS_ERR(cntrs)) > + return PTR_ERR(cntrs); > + mon->assigned_counters = cntrs; > + mon->mbwu_idx_to_mon = no_free_ptr(rmid_array); > + > + l3->mon.mbm_cntr_assignable = true; > + l3->mon.mbm_assign_on_mkdir = true; > + l3->mon.mbm_cntr_configurable = false; > + l3->mon.mbm_cntr_assign_fixed = true; > + > + mpam_resctrl_monitor_sync_abmc_vals(l3); > + > + return 0; > +} > + > static int mpam_resctrl_monitor_init(struct mpam_resctrl_mon *mon, > enum resctrl_event_id type) > { > @@ -1135,8 +1240,21 @@ static int mpam_resctrl_monitor_init(struct mpam_resctrl_mon *mon, > */ > l3->mon.num_rmid = resctrl_arch_system_num_rmid_idx(); > > - if (resctrl_enable_mon_event(type, false, 0, NULL)) > - l3->mon_capable = true; > + if (type == QOS_L3_MBM_TOTAL_EVENT_ID) { > + int err; > + > + err = mpam_resctrl_monitor_init_abmc(mon); > + if (err) > + return err; > + > + static_assert(MAX_EVT_CONFIG_BITS == 0x7f); > + l3->mon.mbm_cfg_mask = MAX_EVT_CONFIG_BITS; > + } > + > + if (!resctrl_enable_mon_event(type, false, 0, NULL)) > + return -EINVAL; I don't see wher the memory allocated by mpam_resctrl_monitor_init_abmc() is release on errors. We may be tolerant to small memory leakage in this case, I guess. > + > + l3->mon_capable = true; > > return 0; > } > @@ -1699,6 +1817,23 @@ void mpam_resctrl_exit(void) > resctrl_exit(); > } > > +static void mpam_resctrl_teardown_mon(struct mpam_resctrl_mon *mon, struct mpam_class *class) > +{ > + u32 num_mbwu_mon = class->props.num_mbwu_mon; > + > + if (!mon->mbwu_idx_to_mon) > + return; > + > + if (mon->assigned_counters) { > + __free_mbwu_mon(class, mon->assigned_counters, num_mbwu_mon); > + kvfree(mon->assigned_counters); > + mon->assigned_counters = NULL; > + } > + > + kvfree(mon->mbwu_idx_to_mon); > + mon->mbwu_idx_to_mon = NULL; > +} > + > /* > * The driver is detaching an MSC from this class, if resctrl was using it, > * pull on resctrl_exit(). > @@ -1721,6 +1856,8 @@ void mpam_resctrl_teardown_class(struct mpam_class *class) > for_each_mpam_resctrl_mon(mon, eventid) { > if (mon->class == class) { > mon->class = NULL; > + > + mpam_resctrl_teardown_mon(mon, class); > break; > } > } Thanks, Gavin