Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <580b7e0f-1303-bb27-0f9c-f05f793a30d3@intel.com>
Date:   Wed, 28 Sep 2022 08:48:09 -0700
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Firefox/102.0 Thunderbird/102.3.0
Subject: Re: [PATCH 1/5] selftests/resctrl: Clear unused initalization code in
 MBM tests
Content-Language: en-US
To:     "tan.shaopeng@fujitsu.com" <tan.shaopeng@fujitsu.com>,
        Fenghua Yu <fenghua.yu@intel.com>,
        Shuah Khan <shuah@kernel.org>
CC:     "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "linux-kselftest@vger.kernel.org" <linux-kselftest@vger.kernel.org>
References: <20220914015147.3071025-1-tan.shaopeng@jp.fujitsu.com>
 <20220914015147.3071025-2-tan.shaopeng@jp.fujitsu.com>
 <24cbb6e4-d853-6c7e-cffc-daa7412d1eab@intel.com>
 <TYAPR01MB63300E751096E8F613CC7F398B559@TYAPR01MB6330.jpnprd01.prod.outlook.com>
From:   Reinette Chatre <reinette.chatre@intel.com>
In-Reply-To: <TYAPR01MB63300E751096E8F613CC7F398B559@TYAPR01MB6330.jpnprd01.prod.outlook.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?d1lhVEQwSVJRWHlnU3ROV28zNGNWSjdpdGtkYTNjQzMxd0FMaVFEaEd2Q3lr?=
 =?utf-8?B?KzFmY0NBWjRDRVp3ejBZN2o4YnNHa01MeWVzRzZqcUpONitFanl4Y0wvZmh4?=
 =?utf-8?B?ZS81dFNtZ0VTVWFjVWVVY2Urdi9BeFhkQytZTGNUaFdBK0gxTXZMZnpFWUdD?=
 =?utf-8?B?SVp3SUZqOVI4aHgwVjlJNUxFWHlZWWhLcC9xRGk1ZFB3S1VxaW9rSjBpWTcr?=
 =?utf-8?B?M0NWV3JaUFJYTm5qRWtxVll5d2Z1azFnaHo2RHlsWmlXR2VVT2ZKNzdTWm5N?=
 =?utf-8?B?aThLbmdKOUcwR0w4aFpGcElXQ1crZm4yNFZwcWxnc0FhYk9JTEtUbmNoUFE2?=
 =?utf-8?B?QmdpanozNGVtZS9KQ055RWdsa1VFUGQ5WkRjWXM5dkl5VkczL3d6UHRFMnFV?=
 =?utf-8?B?YmluQTBpL2JwNHE1VEcvaHlaMFpMSlNiYkZzZ3RPamZYSzBqR0syRHdYdEhk?=
 =?utf-8?B?MnJlVDdKTDlicWxKWXdObVkwUXdKeDg5OGxneXUyd2ZqNHByekwvcHFLZzdk?=
 =?utf-8?B?MVR4ODlZRGpPU2JpSjJITkNzVHBSWlFwUExCd20zc1JpdGRVWXZ5dmlVRGFO?=
 =?utf-8?B?MUVBM1M3SmJva1l1V3QzcTdCV01FNTRMc29DeWx0ZE5FcG8vYXdKbkJaZEM5?=
 =?utf-8?B?dTdMVXJDekt6U3IyaEk2NDFYak5rNW9tMUZwM3NGNGVhcmhjOGdjbHhybjZj?=
 =?utf-8?B?S1J2SlF0alZVZjVIQVNRL3E0eXJ3UkFHWnlGSTZIOFFoRS9EVGVEZGlMQWpa?=
 =?utf-8?B?U1I3eFlEUTY3aTRDdzg2Z05DSGFYL3FDYjJOclMvWDhFRm50eE1waEFGbTJQ?=
 =?utf-8?B?OGlzbVVXZko1dmVITU40cnRzcHN6MkVRNHQrTUZoNm8zYzZQcEtCWVhiaWFV?=
 =?utf-8?B?NUdpQWFOdWJEN3RrUXhjWlpDb2tsNDFodUphZEVvU1ZHcDlMRGgwdUtDeWQ4?=
 =?utf-8?B?NlRxaWhRb0ZSdVRrQmp6TDVYeDBGOVpaNDcwdUJyTUpBRWJ2dUNEamkydWpL?=
 =?utf-8?B?VWlNYmc3ZGlQNXRUUUZNVzVrUWtCdHBPRENZbFc4VDZoV2ZCZm9nZGJTTVAz?=
 =?utf-8?B?WkJTZFErRUpVYkZGUmdVSFZtVGZHZUVTYmRuNjY4SjZyT1YyRU5iT3p0TnU2?=
 =?utf-8?B?dWl1YkxqZlVlRjk2Y2JZSllzYXU1bXBGc2hrWjdFMjlPdDUyeWJacy9XenNC?=
 =?utf-8?B?VXg4RGs3Nkxnc3dUTG5hWFhKNHNqcFlEV1lUYzM0RjVma3JuK1BDWTlnYlZV?=
 =?utf-8?B?U05YVE5BK0hXY2l0OXhZWFdIWUdCRG5KUDdoZVVzTEdNWEo2S3VRNTNmUTI5?=
 =?utf-8?B?TEgrU2cyVFlOY0R4RW5SNVVqcG85T3Fhc0hDdDk3UndKVWlxWHBjaWhwRVV4?=
 =?utf-8?B?aTFoekZvOWZQeHlFQVNkRzdvQlRMTlJKTE1Ea2RSTjdhSzU2SCtobzFyTG42?=
 =?utf-8?B?NEdrNlg3Vyt2UDlSUGFhaExqRlVPZisvZkxVVWJ0WUk4QVVNNUYxc0grUDNl?=
 =?utf-8?B?d25pMFBEV3gyUEp4QjRON3ZpS3ZvUnNONGtpb3JlWVFiRjU0NG1zcXRjUHRV?=
 =?utf-8?B?dHVzTmZDei9rUjZDekhzd09SN0N0UVE4dmJlREFLUTRVTUpHd0lxUUptUFY4?=
 =?utf-8?B?VEVKUi9Vcm5RcDh6TXJ6TTNObUVBV2R4cllXV0dQNkFxUEEwRkc4QkJVZC9j?=
 =?utf-8?B?RDRSbFdNTkxXTkcrSVV1S25zcVRkMW90U0RzTkNTdHRMdkIwWGJuNzdYYXJC?=
 =?utf-8?B?TUVPbk5INnpSL2dFTnhMN0JTR3BtZGdCVWgzR1J5TE5ML0pkelJSUWZzR2NW?=
 =?utf-8?B?Z0J6NGF2UW9zRWhqaGpnaHdJeTljQTk4VERuUytKdTRZV1dSUXNPS3hTemVx?=
 =?utf-8?B?RzhHbzUyWEtPL0pXRW1sTlNjcGJVSlM4Rk1yMUEzRWh1YjZJRGVtSFpKS1Y5?=
 =?utf-8?B?cjlLclNOeit4d3JWMElza2QwUGQ0SVk2QWdvbUdzenNqY3ZQTFV5VUtTR1lk?=
 =?utf-8?B?cUlFeEdOYnBMS3R5S09PcGVXckJTTlYrejBSUkMrdDBFUXprYUs5UnAvRFBq?=
 =?utf-8?B?RFVGVWs5MTF3VHlYMlAzVzFYZDNDUlpNd2VRdmdWN0VBVE1lV2hiNlM4bGIv?=
 =?utf-8?B?cWxnaGlQQWgzNmZzUEhCbllraW5Xc3FDNG5sMFUvZmpEWDBQWWdsWlI5TUkr?=
 =?utf-8?B?Q0E9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: dae7b708-8343-46bc-4e29-08daa168d9a4
X-MS-Exchange-CrossTenant-AuthSource: CY4PR11MB1862.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 28 Sep 2022 15:48:12.8159
 (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: U72aa1STTW3xx3i5n7/Y+1yLNtDwWNACfS25emGsTG21tYqw4HrBWEE8iQo1tmfMakhF7rkLPBi14S+tLBDLOpE9kYLEtGGnYSs9iiTo5cQ=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MW5PR11MB5930
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-6.8 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,
        RCVD_IN_DNSWL_MED,SPF_HELO_NONE,SPF_NONE autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hi Shaopeng,

On 9/27/2022 2:01 AM, tan.shaopeng@fujitsu.com wrote:
>> On 9/13/2022 6:51 PM, Shaopeng Tan wrote:
>>> There is a comment "Set up shemata with 100% allocation on the first run"
>>> in function mbm_setup(), but the condition "num_of_runs == 0" will
>>> never be met and write_schemata() will never be called to set schemata
>>> to 100%.
>>
>> Thanks for catching this.
>>
>>>
>>> Since umount/mount resctrl file system is run on each resctrl test, at
>>> the same time the default schemata will also be set to 100%.
>>
>> This is the case when a test is run with struct
>> resctrl_val_param->mum_resctrlfs == 1, but if the test is run with struct
>> resctrl_val_param->mum_resctrlfs == 0 then resctrl filesystem will not be
>> remounted.
>>
>> I do think that this setup function should support both cases.
> 
> In mbm test(mbm_test.c), resctrl_val_param.mum_resctrlfs is set to 1 and never be changed,
> and umount/mount resctrl file system is always executed.
> So it is not necessary to run "if (num_of_runs == 0)".

This is true for the current usage. You could also add a warning here
("running test with stale config") if a future test sets mum_resctrlfs - but
with all the current output of the selftests a warning may be lost in the noise.

I think it would just be simpler to support both cases. Having the tests be more
robust is good.

Reinette