Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH v1 11/20] x86/resctrl: Calculate bandwidth from the total
 bytes counter
To:     James Morse <james.morse@arm.com>, <x86@kernel.org>,
        <linux-kernel@vger.kernel.org>
CC:     Fenghua Yu <fenghua.yu@intel.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
        H Peter Anvin <hpa@zytor.com>,
        Babu Moger <Babu.Moger@amd.com>,
        <shameerali.kolothum.thodi@huawei.com>,
        Jamie Iles <jamie@nuviainc.com>,
        "D Scott Phillips OS" <scott@os.amperecomputing.com>,
        <lcherian@marvell.com>, <bobo.shaobowang@huawei.com>
References: <20210729223610.29373-1-james.morse@arm.com>
 <20210729223610.29373-12-james.morse@arm.com>
From:   Reinette Chatre <reinette.chatre@intel.com>
Message-ID: <15febd4d-11d9-8456-60ee-994e66efdc98@intel.com>
Date:   Wed, 1 Sep 2021 14:31:15 -0700
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101
 Firefox/78.0 Thunderbird/78.13.0
In-Reply-To: <20210729223610.29373-12-james.morse@arm.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?V1huZlk1YThpTGRWSjhDZ05ZcXVHWnpaQ1VtN3kwbUZXT2NqN3R3bG5paU9Q?=
 =?utf-8?B?NkNXSjRhSmJxODRoMzNpT243aWZWNDJLS0VNM1I5WG9qMURYSjFLNEovMXkr?=
 =?utf-8?B?OXBVM1AvZ2lyenFKRDZOMFZCMGR2UXBvU0kzOEVsb1NsaTZlb3h1UjZENzNq?=
 =?utf-8?B?YWE3RXhLNmRXK0Yrajh5RitNN01VZWdIZVB1RDBuZ2xhZ2RtT1VOT1ZqT0Ux?=
 =?utf-8?B?SjBITFlhQ21zTmFyb255eGRLbEl3NDJRV1JuWUliY1laYmJZUHdlOXo3RjFo?=
 =?utf-8?B?MllZN0xVUkQyK2R5VHRNN01rMUZYT09lSHlPVVMwMWo1VmVpUmNBU1o5R05N?=
 =?utf-8?B?bGcyQmhFTFBPMUVYL29QOU1qajkzMlgxTUEzS1UwT1lmMjNCN2pOalBDelFi?=
 =?utf-8?B?YXY1S1IzQlFMdW42ZGlUZXBrRnVLMUFKbmtnQWVzM3AxSTdqaTg2bXJteU8w?=
 =?utf-8?B?VGxHMDQrWFI2aU5sRVp6VzNteEwzQk93UkdGTXBFTllXdDhHMVpTRUtJcWpu?=
 =?utf-8?B?T2JLRGZZM3gzTTRXUWVPRkwzRFhId3ZFYzdsMkhsQVN1VlNpOFpkcStUMlNq?=
 =?utf-8?B?NlVmbHJQMmN5T0NRTXRTbTc2R3haeW9HUWIrUjViUzA5aVhyRHR6NUdPaTJp?=
 =?utf-8?B?V2ZpMnB3ZDg4emxrWnh3VnFTaDRVcnd2RWtwWEdUb2owL3NDazFCNTd1RVM3?=
 =?utf-8?B?a09DSXBVWFNtcGhEQTJxZ0hlY2RCeERSd09WM2I1TmtINStwTUhuRGg3WjJC?=
 =?utf-8?B?eFNOZ1pSWkNVUlo0SHRMUnRKS3lXSjlnenVZRjBZaEI2Z05JVzhNRlB5RVRK?=
 =?utf-8?B?K0lWM2d5TGpJbnRrN01kQ29STUxNMzI5YjE3VmthVWtQbHNLTXBwL0xVUVZB?=
 =?utf-8?B?UWZ3Qmp6VEpaSit0OGVsUE1Xa1UzejBPQmdROWlSZTBRMCt2b1p3ejFEbFB6?=
 =?utf-8?B?eWpXTmtmaFNRdnQvaHMrYkwyUUNqM1dlZjVDRmxiVlNuLzJNbldrLzRhbmpU?=
 =?utf-8?B?aDhPRTdJN3JYYnRyWHZSam9KdG9zcWJjdi96Yy9iTkd5WTVubVZNQ0tyanBS?=
 =?utf-8?B?enBtTTUwRS8yUmN4U2ZlaUNiK1dJV2JsSm5ub2p4NDJ5L2hsdU5ERXFOZ2d4?=
 =?utf-8?B?R1NmWEl1NjBsOGxnM1NJUlloenBOYTJROXdoWUp1a0lmTnRpaCtBUUhpTkZ6?=
 =?utf-8?B?Zm9xNmVVNE5hbUpYaW9GMzVIcU5UYUVZTldnN3VwdVZZdDA3Y1AzVEdjdlJq?=
 =?utf-8?B?eURBMnpuR1F0cHhrYitqcVZwNzEzRTdaY0kvOW50N1J4R0Y5bHFTTUxtTnlZ?=
 =?utf-8?B?UHhna1Fjclk1WVZCVWlwMmZzT3h5My92bVBBU04rY0dmeWs0UEprRjNxVUh1?=
 =?utf-8?B?NkpTczhqNHdTQjJHeDFxVnBNbHNMZjZscjFiN25WcW5pYWRFRXVOVW1GTzh5?=
 =?utf-8?B?a3dUTjBCano4SVVoYnBXUjZxUFhoMUxxOStPUTNzQmRJSURaMmtaWW5WM2xs?=
 =?utf-8?B?U3lzM08rbUszZDFtS2NZeUNIYmx6Qkp1WTFKQ3VLU2JRQVJUenJlZ3N1ZWRI?=
 =?utf-8?B?NWU1UGkyOHNsVEVqRDlxVHAwa3FsUFUvZ2JrSyswa3ZidTZxL3p2U2tTV1Bw?=
 =?utf-8?B?YWY2MXBaaGJPcHBHU1BnYWtmNFVMVGZHblJnK212TmVjRGxlWHYzUmdCVVpB?=
 =?utf-8?B?VHlXdUV4RFJKeVF2WlRDUkh4SGpBL29EZFVyVXFVN2lhbEZCb2RiVTMxNUNi?=
 =?utf-8?Q?cMBFbkuUOtgAmXF+nO69l5SnExTEknN/s81OkOb?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 278e06a1-a7de-4482-2661-08d96d8fd6a7
X-MS-Exchange-CrossTenant-AuthSource: BN0PR11MB5744.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 01 Sep 2021 21:31:20.0009
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: I5u5gdSb4TJI/QLo/vusICZQd/+92igLJpdXoHRYEcEgvllYuzJzXHYd4s3nUVUk7mSBGSu8oTXgN1AoBusdHAPZU6U4x5MUjvEMK8wr7zU=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN6PR11MB1905
X-OriginatorOrg: intel.com
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hi James,

Apologies but I find the changelog hard to understand.

On 7/29/2021 3:36 PM, James Morse wrote:
> mbm_bw_count() maintains its own copy of prev_msr to allow it to
> calculate the bandwidth as the number of chunks counted since the
> last time mbm_bw_count() was invoked.

ok, I understand there is an extra copy

> 
> The prev_msr and chunks values are in a format specific to the
> architecture. MPAM's monitor scaling can be enabled for some
> counters and not others. If the value is changed once the bandwidth
> counter passes some threshold, the prev_msr values need to be
> converted to the new scale. Having two prev_msr values is a
> hindrance to moving this logic behind an architecture specific
> function that returns the counters number of bytes.

Above talks about format ... it is not clear how it connects to 
introduction where there was mention of an "extra copy"

> 
> Change mbm_bw_count() to calculate the total number of bytes the
> counter has seen in the same way as __mon_event_count(), then
> calculate the bandwidth from that. prev_bw_msr is replaced by
> prev_bw_chunks, the chunks value from the last time mbm_bw_count()
> was invoked.

It is not clear to me how this change connects to either the extra copy 
or the format comments. It describes the changes made but it is not 
clear what the problem being solved is.

It is also not clear to me what the impact of this change on the non 
software controller flow is (where data is read from user space). What 
is the impact now that these two flows no longer track the previous read 
value separately? Would user space still see similar data as before this 
change? How would the software controller sharing control of prev_msr 
affect the data?

Reinette