Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:2 as permitted sender) client-ip=2620:137:e000::3:2;
Date:   Tue, 12 Sep 2023 08:57:17 +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 2/3] x86/resctrl: Add sparse_bitmaps file in info
Message-ID: <w73nwlw6q5drlarcah56rhyknzaktu32iwp3bm7x7idgxdbmtt@m5xf44glqkf6>
References: <cover.1693557919.git.maciej.wieczor-retman@intel.com>
 <27fc083358bc215676354818ad5a7abebd00e332.1693557919.git.maciej.wieczor-retman@intel.com>
 <6b172280-7dbd-9d97-4e00-ce231f084904@intel.com>
Content-Type: text/plain; charset="iso-8859-1"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <6b172280-7dbd-9d97-4e00-ce231f084904@intel.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?iso-8859-1?Q?G3u/MxBOH4e4Oaynz4z6++LKY8AMnmEKQ6Rkn4TIBWzKK98toowTjgjK+m?=
 =?iso-8859-1?Q?znpe+ByakGi+7V9JKCv+FLKmaAfvfpHLz3w3KL8ayybtxyttM2NzeENkHM?=
 =?iso-8859-1?Q?AbWTDDEBWohWV4xeKf+vK7GDyyX3N3QXJyCkagQUslamySlBu/78MmLvDQ?=
 =?iso-8859-1?Q?FNMv+149qMmdJjCQOD9kg3pA8K+hAci5g8IoJRWNCGLjJRxyZEWqp/rkrg?=
 =?iso-8859-1?Q?6SKbm9ZeDnKGYu8N3ajY31rbFUKLRU8J82q8xuJZwmGKiN9GAHXrSpRTsd?=
 =?iso-8859-1?Q?y6nYdVTVgMSIB576uuOdIS3O5s56lNE4Zu//STHgaJOfHz6zzolwZYm7uF?=
 =?iso-8859-1?Q?Rr3cD5iBCVQ3DjpWu2B2CrB5iL+1GUUU7BsTXZbKCgmK2DLYakzdYrKRG7?=
 =?iso-8859-1?Q?K7dPdOe6Z6of/UQgGSGjaxCZBpfgFvXJxDLLDCVrDpaAygXwYSJ9xydTXY?=
 =?iso-8859-1?Q?oDMgjRnSrMMXmWkFf6xb5F8FMGVTWf99xLJ9J0wH13Twegf111nVwtiK0d?=
 =?iso-8859-1?Q?Xeoj5Ifw94vVAVHKVtfhM3xXu6b73y6o3qv6qShCtYwa489YH9JcoY44Mr?=
 =?iso-8859-1?Q?Sw5TWwUleh3I6ZcbeGh5o89fbESiQCQVoJu0eKr8MsbtC2xMf4qQ9+vb2Y?=
 =?iso-8859-1?Q?bNqp6zRsTS1B4D4mZsEnp7os7Xy5O8CIih8kyg7wQnLoerf+OQVJlAW5Ci?=
 =?iso-8859-1?Q?7RGFhpDrJBgkbb3X7dEN+Q2OkFW6xFRAAGpyF7UntaxdwPTF90M2+LAeg4?=
 =?iso-8859-1?Q?9kVffsVYfJegzKYsSw7a9nSjjufbp/Z/7GipLC8O8MY8S/Qfa7CiJN4wVu?=
 =?iso-8859-1?Q?N1JzWR1iUloQF/RAMmmGTJ0XDkTGCMluHeHwR7kqlmwMAbXcMhpKI4/bPi?=
 =?iso-8859-1?Q?dk5o1ZaWIjrZpTRiGLG3EIUCFIcfGyb+t6Qc1J59+9iiniQpqVpwOuV056?=
 =?iso-8859-1?Q?W6IOpkYwlh1GYQfaWsWaJETldIxQnOb4TRep83Bn+hp/f6cgrNsDTvBSiL?=
 =?iso-8859-1?Q?QGrp61ApRL+P7ZQxt53b6kc3MBl9m3lRLYZsi8odumwsA55ULfl84Gz+xH?=
 =?iso-8859-1?Q?SBhafedU3N2lcv+a8Rk/6xvABCqIrynwJtzZMJbQ4bGguJRwpZtH2lxhpZ?=
 =?iso-8859-1?Q?mm1gWaEaqg4MrdBsuMw/DtrQx1Tnp3YnKNcWG1DdUssiP4rJ1VEcbKhB9l?=
 =?iso-8859-1?Q?RhhGQXn7e4+h9HCw1Z/R/j0WCYUzA02a6EDj0lHTIcJ2x6ydyq/HKtuWTt?=
 =?iso-8859-1?Q?U9hhOSfbGsb2fIC/7u+jbYgLLc/OmI4PyrUxdRPQ9YOhuFROUqwWs2/4YS?=
 =?iso-8859-1?Q?kevkkyqj2795/9oBxqz+xQj7uX1VjfU6vnXe5PU5UEe2gEhNpAdKpLfvKG?=
 =?iso-8859-1?Q?9aL9SuA456tSnWstpuOGXvC+JPLB6HRFmDJL/eHVWEFgApV/wX/vVZvgb8?=
 =?iso-8859-1?Q?uEgM74GeulAZSrHUVziR06dIkJz6ILip0sd+aoyYvr0aO+ECQcQwt6M8t0?=
 =?iso-8859-1?Q?0ACUedJcbafMLCObwTQqjVNc91e5xBs41Mg+JgyjhSz3167+0AK14BIvqO?=
 =?iso-8859-1?Q?u2m8e6WEKjaSTee2Kp5Ru51kANSlznvMpKlhfPAfeym4wxXgcaGUEPqQ4e?=
 =?iso-8859-1?Q?1cniScR1SY9+0YOOl9ZHj1ihpeyFNwJPi/tULZiMUJALJaUPPGudzPQusQ?=
 =?iso-8859-1?Q?t06dqbP6Z+ko5L2mSmQ=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: c59cfb23-a1ef-42d7-b10d-08dbb35d84a1
X-MS-Exchange-CrossTenant-AuthSource: MN0PR11MB6231.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 12 Sep 2023 06:57:24.3039
 (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: YRderypUq0ygRcXNcz5Ydxw/rd2tjDRFu/q0EwCMoGS3XOMYjqvPju3LyDZMqIF7Nk9VTutX9Q+NRGneJu8g9pCYFrQPtU0B+uaCbnDKSZ4=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ2PR11MB8500
X-OriginatorOrg: intel.com
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 (agentk.vger.email [0.0.0.0]); Mon, 11 Sep 2023 23:57:35 -0700 (PDT)
X-Spam-Status: No, score=-2.0 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,T_SCC_BODY_TEXT_LINE
	autolearn=unavailable autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on agentk.vger.email

Hello,

On 2023-09-11 at 13:05:30 -0700, Reinette Chatre wrote:
>Hi Maciej,
>
>On 9/1/2023 1:55 AM, Wieczor-Retman, Maciej wrote:
>> From: Fenghua Yu <fenghua.yu@intel.com>
>> 
>> Add the interface in resctrl FS to show if sparse CAT bitmaps are
>
>resctrl is intended to be a generic interface so modifying it
>with a vendor specific change is not ok. This is not what the
>patch is doing though so the changelog can just be modified to
>not create the impression. Perhaps just:
> "sparse CAT bitmaps" -> "sparse cache allocation bit masks"

Okay, I'll change it, thanks.

>> supported on the platform. Reading the file returns either a "1" if
>> non-contiguous 1s are supported and "0" otherwise. The file path is
>> /sys/fs/resctrl/info/{resource}/sparse_bitmaps, where {resource} can be
>> either "L2" or "L3" depending on their support in the CAT feature.
>
>No CAT here. L2 and L3 are the hardcoded cache allocation resources
>so "depending on their support in the CAT feature" can just be removed.

I'll change it.

>> 
>> Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
>> Signed-off-by: Wieczor-Retman, Maciej <maciej.wieczor-retman@intel.com>
>> ---
>>  arch/x86/kernel/cpu/resctrl/rdtgroup.c | 18 ++++++++++++++++++
>>  1 file changed, 18 insertions(+)
>> 
>> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>> index 725344048f85..4d27354f3f30 100644
>> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>> @@ -895,6 +895,17 @@ static int rdt_shareable_bits_show(struct kernfs_open_file *of,
>>  	return 0;
>>  }
>>  
>> +static int rdt_has_sparse_bitmaps_show(struct kernfs_open_file *of,
>> +				       struct seq_file *seq, void *v)
>> +{
>> +	struct resctrl_schema *s = of->kn->parent->priv;
>> +	struct rdt_resource *r = s->res;
>> +
>> +	seq_printf(seq, "%u\n", r->cache.arch_has_sparse_bitmaps);
>> +
>> +	return 0;
>> +}
>> +
>>  /**
>>   * rdt_bit_usage_show - Display current usage of resources
>>   *
>> @@ -1839,6 +1850,13 @@ static struct rftype res_common_files[] = {
>>  		.seq_show	= rdtgroup_size_show,
>>  		.fflags		= RF_CTRL_BASE,
>>  	},
>> +	{
>> +		.name		= "sparse_bitmaps",
>> +		.mode		= 0444,
>> +		.kf_ops		= &rdtgroup_kf_single_ops,
>> +		.seq_show	= rdt_has_sparse_bitmaps_show,
>> +		.fflags		= RF_CTRL_INFO | RFTYPE_RES_CACHE,
>> +	},
>>  
>>  };
>>  
>
>I think it is essential to use consistent terminology. To help with
>this I reviewed the resctrl documentation and found no mention of
>"bitmap" ... yet many instances of bit mask and even a clear official
>term of "Cache Bit Masks (CBM)". The user interface is thus already
>established and using the term "bit mask". Looking through the
>AMD and Intel specs I also only see "bit mask". I think "bitmap"
>sneaked in via an Arm contribution as motivated by their spec's
>use of the term "cache portion bitmap". Since "bit mask" is already
>in the user documentation and also in the interface via "cbm_mask"
>I'd prefer that we stick with "bit mask" in user interface instead
>of creating fragmentation with a new term.
>
>Considering this I'd like to propose "sparse_masks" to match existing
>"cbm_mask".
>
>Please review this series to be consistent in this regard. Note that
>patch 2 refers to "bitmaps" and then patch 3 switches to "bitmasks"
>... patch 3 already uses the term "sparse bitmasks".
>
>I think that it may also help to add a patch to this series that
>renames arch_has_sparse_bitmaps to arch_has_sparse_bitmasks.

Okay, I'll go through the patches and unify the names to "bit mask"
rather than "bitmap" etc.

>Reinette

-- 
Kind regards
Maciej Wiecz?r-Retman