Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:5 as permitted sender) client-ip=2620:137:e000::3:5;
Date:   Fri, 29 Sep 2023 08:20:54 +0200
From:   Maciej =?utf-8?Q?Wiecz=C3=B3r-Retman?= 
        <maciej.wieczor-retman@intel.com>
To:     Reinette Chatre <reinette.chatre@intel.com>
CC:     Fenghua Yu <fenghua.yu@intel.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
        Dave Hansen <dave.hansen@linux.intel.com>, <x86@kernel.org>,
        "H. Peter Anvin" <hpa@zytor.com>, <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2 4/4] x86/resctrl: Rename arch_has_sparse_bitmaps
Message-ID: <27gd4em7og62jn4v364emkthna5a2f63nvlxigvyh4p74bcyzo@llgj56vewc73>
References: <cover.1695371055.git.maciej.wieczor-retman@intel.com>
 <e689caf9dbb7664c26515a980084b5f17cb0644e.1695371055.git.maciej.wieczor-retman@intel.com>
 <a1eedcbb-3797-dc0a-918f-eea8a58bbf52@intel.com>
Content-Type: text/plain; charset="iso-8859-1"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <a1eedcbb-3797-dc0a-918f-eea8a58bbf52@intel.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?iso-8859-1?Q?/rct5pBHoSFoHZX64pc6iGgN4bwN3ilkhrHC3lDvG1vh8tL3usZz8e50bs?=
 =?iso-8859-1?Q?+/3T1/auuXhMBj1Rqpx9rBd/zd0tw8fyDaILqTR7JBk0KVU46UtIVciFwF?=
 =?iso-8859-1?Q?v8IBbrHd0g88h6N+LqBbOm30V7FrldXAaXl2iP6sOTQtCGzmxn7NUg+ZZx?=
 =?iso-8859-1?Q?Ia4LY7B1cPGbET8OWHtF/D/MPQd7VlgZjBj28sJizEM1FXxASE67bKseWY?=
 =?iso-8859-1?Q?TBX7VwR6JH6Pj6zczbKQjy5LKXpxl2GTGekGssm2m80c64YFSm6nMnNMXM?=
 =?iso-8859-1?Q?Pe1HSvRY0U7M9yfaidafq1p1ueTz4Nyt45fi8XGD8HFiOhOn5FIQp8oTpk?=
 =?iso-8859-1?Q?YMHNuK8ywGOC6ByJ+YbJtJbc1lzms7I4Oa01t1XAL2bBeo2YTv4y3/2/b9?=
 =?iso-8859-1?Q?6K0CPi7k94MxbXSSkFz17O1WI+naTChHGOZs+rTqUVMp9FilchgFFyxtX3?=
 =?iso-8859-1?Q?nakIe2PEB7IxSg2fxJa4otmPlF3yaLWPLnHvOzR9VdzkdzxEeFgk0cTFct?=
 =?iso-8859-1?Q?OWBNyOK0ZIAfeMzSfH29kZ9BZWY7rJg2kcJkYtHrbtOXy7SmkCG2KjBTR9?=
 =?iso-8859-1?Q?P9s7pE99qykMqLdXW01zGel/2/leu8qTmhH0VcXr8z7xPKGydHVBMQE9Gq?=
 =?iso-8859-1?Q?asb0D+gmzfBraL5Z11Z3RTm+qcRgL1Wfg+WDST0xLeINEuWOqeNBc5x/k5?=
 =?iso-8859-1?Q?et0wdv6WxTqaYbvUFusKpjVdSAFIV9az3riCM2JShwt+dxU2PBjsP6tOWb?=
 =?iso-8859-1?Q?u9Ilo9vMg4R/TaEHZS8J5d8GRIg9jVGwLqDjys9dxYidgJ4BI3W1TMdp0h?=
 =?iso-8859-1?Q?/bm70T9wbP7ipXrXSOFe7f/HX0Z3B+JadIriEy6Z65wr3tiFjuhWJt1b6Z?=
 =?iso-8859-1?Q?NsVtVdq6UV4aL+d6tkvE2J3dmXWA4YoQXmsSG3Bg4H77gezOFqW+e8vzOe?=
 =?iso-8859-1?Q?F5YmV9ZJ8yhM4oLd7qwOJpEHc07QLiMcP5Idybwg4WvX/pmTz4h7XWAcmg?=
 =?iso-8859-1?Q?a54ocfmGXqvKfTGMW5VBGNfWDKJTUprxDaI+h1WaH0wxDmGvUaUjf4Sh9J?=
 =?iso-8859-1?Q?CMnY+ZycS2ou4q49ZrLZ+uDOir/J6yGXJ5ve8DeWaKaArX2hH7aWM5WN0a?=
 =?iso-8859-1?Q?e9wlCurZy6gPgAavWymGkDSS7QqXu6aVGNOTYuYW7P2BKduwDHDJgKDz7p?=
 =?iso-8859-1?Q?Y1sSf7YFgypfmmU/Gp3gptOw5bg+/8RbANisiV1Vmc7t2HgEd4fe3RS1cP?=
 =?iso-8859-1?Q?XjWOPnn2NTxXbcfHeLdXjHE9aUfQUPVlGdjDbGMCcdt3kLzhlzQwva0e2e?=
 =?iso-8859-1?Q?uLjZM2YUBZpPE10wHGr5otRtYZo4xhUP6eS2t7PSDkPpmD3xe44Kqmzq/a?=
 =?iso-8859-1?Q?RYabsf6PXyV3BIyRphSdSD/TcNnyO8aUqO1fUZQu29SUEYmUlk+VzrSL0m?=
 =?iso-8859-1?Q?DkTuwOy+NdQYqQBfc2if3fTmJAtuJW6cOhtS0ER3oNL2j5HKONb77M2vJQ?=
 =?iso-8859-1?Q?ZShhxIUcs8VeYIHUYtZpZYmdOS3xT3A9m6CRaUovzHBxnaZb0df0HE/C2u?=
 =?iso-8859-1?Q?WwZu6+K1JYNhb3M/66nWpEtwCS29Uwl7XTh07w4dt9EIwT08G4zIkf1hDX?=
 =?iso-8859-1?Q?uzlkYktgT2l5LK+3tuhG8acgTp/y0bdsqN6quwEqRVFJff9rz2rPI2foSp?=
 =?iso-8859-1?Q?Vc/jjlIyH30wNyB8mio=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 4339bf6b-4524-435b-1f5d-08dbc0b44018
X-MS-Exchange-CrossTenant-AuthSource: MN0PR11MB6231.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 29 Sep 2023 06:21:00.6290
 (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: 3uUmuyMKEfk4Y1L1zchpQrHGWD2ZpfKkOFVzR0myBYKsLmJ7iHqUcnacAf82SzGHdyyNLCirOvFW5T90NRVE7/aq9wDj0lS3+EGnoMcEnT0=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN9PR11MB5529
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-0.9 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 autolearn=unavailable
	autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on groat.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 (groat.vger.email [0.0.0.0]); Thu, 28 Sep 2023 23:21:27 -0700 (PDT)

On 2023-09-28 at 13:47:49 -0700, Reinette Chatre wrote:
>Hi Maciej
>
>Could you please move this patch to the beginning of this series?
>Having it in the end results in a lot of churn with significant
>changes to new code introduced in this series. All this can be avoided
>by providing a smaller patch at the beginning of the series.

Sure

>On 9/22/2023 1:48 AM, Maciej Wieczor-Retman wrote:
>> Both AMD and Intel documentations use capacity bitmasks terminology
>> rather than capacity bitmaps. Also bitmask term is much more widely
>> used inside x86 resctrl code.
>
>Since resctrl is intended to support many architectures and Arm coming
>soon (and they use bitmap) I do not think we should use the vendor's
>language as a motivation. For me there are three reasons supporting the
>rename:
>* arch_has_sparse_bitmaps is the *only* instance in resctrl that uses
>  the bitmap term for capacity bitmasks, all other parts of resctrl refers
>  to it as a bitmask
>* bitmask is the established term used in resctrl documentation
>  (Documentation/arch/x86/resctrl.rst)
>* bitmask is the term already exposed to user space via resctrl ("cbm_mask")
>
>Finally, why do the rename as part of this work? This can be motivated
>with something like:
>	A later patch exposes the value of arch_has_sparse_bitmaps to
>	user space via the existing term of a bitmask. Rename
>	arch_has_sparse_bitmaps to arch_has_sparse_bitmasks to ensure 
>	consistent terminology throughout resctrl.

Okay, I see your point. My only reason for mentioning x86 was that the
changes are done in arch/x86 directory. But I can see that there is no
need to mention it here really. I'll redo the patch message accordingly.

>> 
>> Unify the naming convention by renaming arch_has_sparse_bitmaps struct
>> member to arch_has_sparse_bitmasks.
>> 
>> Suggested-by: Reinette Chatre <reinette.chatre@intel.com>
>> Signed-off-by: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>
>> ---
>
>...
>
>> diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h
>> index 8334eeacfec5..83c2cbf7136d 100644
>> --- a/include/linux/resctrl.h
>> +++ b/include/linux/resctrl.h
>> @@ -57,7 +57,7 @@ struct resctrl_staged_config {
>>   * @list:		all instances of this resource
>>   * @id:			unique id for this instance
>>   * @cpu_mask:		which CPUs share this resource
>> - * @rmid_busy_llc:	bitmap of which limbo RMIDs are above threshold
>> + * @rmid_busy_llc:	bitmask of which limbo RMIDs are above threshold
>>   * @mbm_total:		saved state for MBM total bandwidth
>>   * @mbm_local:		saved state for MBM local bandwidth
>>   * @mbm_over:		worker to periodically read MBM h/w counters
>
>Please drop this hunk. rmid_busy_llc is indeed a bitmap.

Okay, thanks for catching this.

-- 
Kind regards
Maciej Wiecz?r-Retman