Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.36 as permitted sender) client-ip=23.128.96.36;
Message-ID: <95fc35a2-2f19-4ba5-ad3a-7d7ae578289c@amd.com>
Date:   Thu, 26 Oct 2023 08:55:27 -0500
User-Agent: Mozilla Thunderbird
Reply-To: babu.moger@amd.com
Subject: Re: [PATCH] x86/resctrl: mba_MBps: Fall back to total b/w if local
 b/w unavailable
Content-Language: en-US
To:     Peter Newman <peternewman@google.com>,
        Tony Luck <tony.luck@intel.com>
Cc:     Fenghua Yu <fenghua.yu@intel.com>,
        Reinette Chatre <reinette.chatre@intel.com>,
        Jonathan Corbet <corbet@lwn.net>,
        Shuah Khan <skhan@linuxfoundation.org>, x86@kernel.org,
        Shaopeng Tan <tan.shaopeng@fujitsu.com>,
        James Morse <james.morse@arm.com>,
        Jamie Iles <quic_jiles@quicinc.com>,
        Randy Dunlap <rdunlap@infradead.org>,
        linux-kernel@vger.kernel.org, linux-doc@vger.kernel.org,
        patches@lists.linux.dev
References: <20231024181600.8270-1-tony.luck@intel.com>
 <CALPaoChftF-H6GauKq4-c_qBJP1GJbR3-ByE5krsaQF4y4y9oQ@mail.gmail.com>
 <ZTluypa9bCWv4k2n@agluck-desk3>
 <CALPaoCj72V=o60tqsFMRzaeUw-1+rN7pyhsdCyVEV=0tN_CZ7A@mail.gmail.com>
From:   "Moger, Babu" <babu.moger@amd.com>
In-Reply-To: <CALPaoCj72V=o60tqsFMRzaeUw-1+rN7pyhsdCyVEV=0tN_CZ7A@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?a2ttMTdQV1JMY2dBR0lqaXhRd2dHT01vVXFQdXJGNWFYU0YvM1dUczVzRUtP?=
 =?utf-8?B?SHYzZnZCYjQxZFJOZDdQZ0tJdlZGV0l1SlkvZDE2SHZ5QVZGOGdDYmFHeFRV?=
 =?utf-8?B?NGVrcTR2TVVWUy9XWnRmRDJtQWRxaTIrdjhxUDFZWlp3bG02aDBRdWEyMElK?=
 =?utf-8?B?Y2gvU2dMQUI3cWVDdWwvT0pYczU2VzNCUVBRNUl1QWh0cTVPMmJyZmM2M2lj?=
 =?utf-8?B?YUFxSWxWSkhJVkJrd0psbnNVdEo0cDVweHZOKzVHOEV5dk1TSGVtWTVBQXZ3?=
 =?utf-8?B?ZWtGa3lWNFkxS2EwRFk2c3VON3hYcjRlQUtjb2FqMHhMOWNSYitTUkx2V3FW?=
 =?utf-8?B?V1l6RHRQa1QyaWVJUFFtYml4ZlhpSEV1d1RubTg5UXVXZ2d1K2dWVVVsek5M?=
 =?utf-8?B?YURSdEIvUXBLVG9KNUtMbnBKQlBtZGJrUEhuWWVoSXFJSFZBcGhHaSt6NnZY?=
 =?utf-8?B?KzRqNm9RQzljWDRVTVVybm11R3pOTG43ZXcybkhkcXNDajhCODllbkQ5aURO?=
 =?utf-8?B?TWpiV3hla0NVQUw0U1BTQk00dERvb05ZaVJnV2FENFk1VzZNNFllWFJhMFJK?=
 =?utf-8?B?ditBa0JvMXpYeGdueWhGVEg4OXRXM1lzeTJkNCtSTDd6YzBxdFREcHpYQ2hW?=
 =?utf-8?B?WkZKS3UrTk0xNldMZWsvdUFMViszVEhCaUtOdGZ6ZTc2UUxPS3p3eWFZSCs4?=
 =?utf-8?B?K05ZRXdyQ3JyaUlvbmhxbS82dW1xZXZaNlZrdmlJVCtBRFMzTUVMa3NRNmNa?=
 =?utf-8?B?TEMxcnpLWXMyamZvUUU2RG5kVVk4Tm9NSmNOa1djRnE2MmlDRFhYZXN0TFdW?=
 =?utf-8?B?bzRzMkc3eFZ4YkEyR0I2MFZvN2srVDhBYmYyVy9GVUNMQlNTRzA1R0F4a3Fj?=
 =?utf-8?B?dTJycXZLdEhtOEpWQlJ1eS9GbkVVRHNyajFWUnowRzFhVkFCQUxRWGluVUI4?=
 =?utf-8?B?TEE4cEJwUU5rUG0rM3JSWTF6T21ZRytpRFlQWldmdUErdVlhSDBrNUxST05S?=
 =?utf-8?B?bG1SRksvdXVQanpaZGZLVUFad2VqMjZ6SitGaTYwbXZScTZYT2thRFpVWDBI?=
 =?utf-8?B?NHVpdzBSTExodU1pRHJXOThFSkRWK2RTSzFhQjgrVnBFS2pBUHQyRk53MjNy?=
 =?utf-8?B?NnpYeEV6Z2VNN3VQckYzSlZGYUovT3VEcmtMOEQrZjlNUVEyZnV4VjE3R1VN?=
 =?utf-8?B?WGhMVjNVczlhNFpYSW5Ca3dJd3ZQL2t3SzA0SnZybWs2REFXaXJaT0pIRnA3?=
 =?utf-8?B?ZWR5bGwzVzVJaFdFdFg1S01Xb2l5SnY4RTZKZWVuU2RodnVQa3Iyb1k1SE9U?=
 =?utf-8?B?OEg1UjY0NHp6SW1NNnYzdWd3WUZBME9QVndtQ1puRVlETUZRSkV0Q3hvNHVt?=
 =?utf-8?B?UFEwNDk2VFZDSXltRXNzSTQzQ3RYZ3lqNFdIZ1FoMXFJV2FUZ01INnhiU3I5?=
 =?utf-8?B?Kzg4OEpXeGd6YngvM29Eb1Nlb2pCcjhqZHdwUS9TcHU4TkJyK3Mrb0crMnVu?=
 =?utf-8?B?akNacjdITGJ5SVYrTXphbHBkMkdTWFhpK1NUNlRHT0h4Q1BGV3BKYUE1UkRz?=
 =?utf-8?B?QzA5YTh5UzRXcHBuMDhTZW15U0pDS1hoZWdQT09tR2hZUTdJblhkYisvdlB6?=
 =?utf-8?B?ZWtNKzZwMTdLKy9RZExQbXBxNEhld0F3SGh0d0JEL1lOV0ZveVdReElISHlG?=
 =?utf-8?B?VzY4cFlwNmFyWU92aklvekdOaGV1R0lmQkF6K2x3ZG9yckU1akpIZC83aWc1?=
 =?utf-8?B?KzJ0cWFjS3k4NFkyRTNCODE1WSthT2RHZXQwQjNYUXY5ZG1aaURSZG1ZWXNH?=
 =?utf-8?B?czJPVkNyNHpvY0RsU1dlMzdPbHJzYjdERndqQjJJaUNGYnRDNVZWREttUkZZ?=
 =?utf-8?B?NnVUMVNzSWdKWkMyNlVqSG5OUzBKK2paMWpvLzl1ZDNmNWJyUVVRVld1cHgy?=
 =?utf-8?B?bjZJY2Vaek1ybVF1YlBidHNBekhZbjVGTnFibXY4MGFHZTQ4MkRFRzZ3T2xn?=
 =?utf-8?B?WlJPVDZEVHd6T21rdk8wQUVXcmlRd2U4d3ZXc1h2Z0o4dFIycTVwN0tINjRX?=
 =?utf-8?B?aDlDamNNUElhSktQYXg5UVpDYkVXSWpDYVlIY0NJWkJmU0FWY3lGaXlybEVj?=
 =?utf-8?Q?Kuyw=3D?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 20d4e0fd-01c5-47ab-9219-08dbd62b3750
X-MS-Exchange-CrossTenant-AuthSource: MW3PR12MB4553.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 26 Oct 2023 13:55:30.4304
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: a5dM8WVrz8H7xdabOmy9jAteyvPhcECUI6RuhA5oyI9Lb1cOc5hEpzNHlEciXTKA
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY8PR12MB9036
X-Spam-Status: No, score=-0.8 required=5.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED,
	DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED
	autolearn=unavailable autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on pete.vger.email
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (pete.vger.email [0.0.0.0]); Thu, 26 Oct 2023 06:55:51 -0700 (PDT)

Hi Tony,

On 10/25/23 16:06, Peter Newman wrote:
> Hi Tony,
> 
> On Wed, Oct 25, 2023, 21:38 Tony Luck <tony.luck@intel.com> wrote:
>>
>> On Wed, Oct 25, 2023 at 02:46:53PM +0200, Peter Newman wrote:
>>
>>>> +static struct mbm_state *get_mbm_data(struct rdt_domain *dom_mbm, int rmid)
>>>> +{
>>>> +       if (is_mbm_local_enabled())
>>>> +               return &dom_mbm->mbm_local[rmid];
>>>> +
>>>> +       return &dom_mbm->mbm_total[rmid];
>>>> +}
>>>
>>> That looks very similar to the get_mbm_state() function I added to
>>> this same file recently:
>>>
>>> https://lore.kernel.org/all/20221220164132.443083-2-peternewman%40google.com
>>>
>>> I think the name you picked is misleadingly general. "local if
>>> available, otherwise total" seems to be a choice specific to the mbps
>>> controller. I think these functions should be reconciled a little
>>> better.
>>>
>>
>> Peter (and Babu, who made the same point about get_mbm_state().
>>
>> Do you want to see your function extended to do the "pick an MBM event?"
> 
> What I meant was I think it would be enough to just give the function
> you added a name that's more specific to the Mbps controller use case.
> For example, get_mba_sc_mbm_state().

I actually liked this idea. Add a new function get_mba_sc_mbm_state. That
way we exactly know why this function is used. I see you already sent a v2
making the event global. Making it global may not be good idea. Can you
please update the patch and resend. Also please add the comment about why
you are adding that function.
-- 
Thanks
Babu Moger