From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by smtp.subspace.kernel.org (Postfix) with ESMTP id C9ADB3AE6F3 for ; Thu, 16 Jul 2026 09:31:41 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=217.140.110.172 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1784194303; cv=none; b=Y74GeDsjLRqjWmFA08qWRf3yXbc6kR/y6vo3cn9kzE4jB2KdpCYulLWvYr0TERDnmHS3xQU/Eztz9YPeAzuM5Ztv84t+1za/vtWHyayXZxXhEjfIIkyc85da9dsiutglxFbqqX+YdsjtvnntXzBUX5Ye/tYfVGrWEqwXWbhFzks= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1784194303; c=relaxed/simple; bh=JEcIu08VithIjzoyjCx134aiQ/C+mCAK2uMYCOenBiU=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=AoozbIDdrx3PxBWaMEhewCxMkjR8ScEzWuOnW+n+TKx4kU4O4Fo3i+GfLFUICAxwSifVqhv+v5ArtlnfVulnX9s2S87TSxDHHki+35efIthW0k838w4EibYgOxwYMuMbWn3CTTGRtQHGWbbgwXxPXxDc95bJ3yCYJFHTzoz/kdM= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=arm.com; spf=pass smtp.mailfrom=arm.com; dkim=pass (1024-bit key) header.d=arm.com header.i=@arm.com header.b=l8OxbNCR; arc=none smtp.client-ip=217.140.110.172 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=arm.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=arm.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=arm.com header.i=@arm.com header.b="l8OxbNCR" Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id F0F2C2F; Thu, 16 Jul 2026 02:31:36 -0700 (PDT) Received: from [10.2.212.8] (e134344.arm.com [10.2.212.8]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id DC27F3F905; Thu, 16 Jul 2026 02:31:39 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=arm.com; s=foss; t=1784194301; bh=JEcIu08VithIjzoyjCx134aiQ/C+mCAK2uMYCOenBiU=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=l8OxbNCROkH+TLnhbhlwGvlFqEdoPRIRZJiWf5lR7t6Pzra6W4VYxN8P6YRScIHnb 8j85IK5SeifslnPFKi4BGbiyaeAORr9SIuA9lCgCm/ut8QwaPAsqSgvD02vmeXEK9R Mdvq0r8wFmQ3uEqp9uGD7Y/7FU8/Rb4QCmhVzeLc= Message-ID: <33654cc4-9b36-4255-b761-3249174a97d9@arm.com> Date: Thu, 16 Jul 2026 10:31:38 +0100 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Thunderbird Daily Subject: Re: [PATCH v1 04/11] arm_mpam: Add missing mon_sel locking in MBWU restore To: Gavin Shan Cc: james.morse@arm.com, reinette.chatre@intel.com, fenghuay@nvidia.com, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, dave.martin@arm.com, andre.przywara@arm.com References: <20260710115546.29644-1-ben.horgan@arm.com> <20260710115546.29644-5-ben.horgan@arm.com> <8e7f992c-db87-4b79-8d92-d9a44295a0a6@redhat.com> Content-Language: en-US From: Ben Horgan In-Reply-To: <8e7f992c-db87-4b79-8d92-d9a44295a0a6@redhat.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Hi Gavin, On 7/16/26 05:29, Gavin Shan wrote: > On 7/10/26 9:55 PM, Ben Horgan wrote: >> The mon_sel_lock is used to protect the mbwu_state, as well as h/w accesses >> that use MPAMCFG_MON_SEL. However, in mpam_restore_mbwu_state() mbwu_state >> is accessed without holding the mon_sel_lock. >> >> Add the missing locking. >> >> Fixes: 41e8a14950e1 ("arm_mpam: Track bandwidth counter state for power management") >> Signed-off-by: Ben Horgan >> --- >>   drivers/resctrl/mpam_devices.c | 8 ++++++++ >>   1 file changed, 8 insertions(+) >> >> diff --git a/drivers/resctrl/mpam_devices.c b/drivers/resctrl/mpam_devices.c >> index 11b10c3bc334..b34e2a368516 100644 >> --- a/drivers/resctrl/mpam_devices.c >> +++ b/drivers/resctrl/mpam_devices.c >> @@ -1648,16 +1648,24 @@ static int mpam_restore_mbwu_state(void *_ris) >>       u64 val; >>       struct mon_read mwbu_arg; >>       struct mpam_msc_ris *ris = _ris; >> +    struct mpam_msc *msc = ris->vmsc->msc; >>       struct mpam_class *class = ris->vmsc->comp->class; >>         for (i = 0; i < ris->props.num_mbwu_mon; i++) { >> +        if (WARN_ON_ONCE(!mpam_mon_sel_lock(msc))) >> +            return -EIO; >> + > > The code can be simplified by avoiding the unnecessary nested conditional statement, > something like below: > >         if (!ris->mbwu_state[i].enabled) { >             mpam_mon_sel_unlock(msc); >             continue; >         } > >         mwbu_arg.ris = ris; >                 mwbu_arg.ctx = &ris->mbwu_state[i].cfg; >         mwbu_arg.type = mpam_msmon_choose_counter(class); >         mwbu_arg.val = &val; > >                 mpam_mon_sel_unlock(msc); > >                 __ris_msmon_read(&mwbu_arg); > >>           if (ris->mbwu_state[i].enabled) { >>               mwbu_arg.ris = ris; >>               mwbu_arg.ctx = &ris->mbwu_state[i].cfg; >>               mwbu_arg.type = mpam_msmon_choose_counter(class); >>               mwbu_arg.val = &val; >>   +            mpam_mon_sel_unlock(msc); >> + >>               __ris_msmon_read(&mwbu_arg); >> +        } else { >> +            mpam_mon_sel_unlock(msc); >>           } >>       } >>   Ok. I think it will be clearer what's best here once Andre's error propagation in his MPAM-Fb series has crystalized. Thanks, Ben > > Thanks, > Gavin >