[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <2cdfbe28-01cc-926d-2f6d-2a974a4c5a74@linux.alibaba.com>
Date: Mon, 24 Oct 2022 10:31:56 +0800
From: Shawn Wang <shawnwang@...ux.alibaba.com>
To: Reinette Chatre <reinette.chatre@...el.com>, fenghua.yu@...el.com
Cc: tglx@...utronix.de, mingo@...hat.com, bp@...en8.de,
dave.hansen@...ux.intel.com, x86@...nel.org, hpa@...or.com,
James Morse <james.morse@....com>, jamie@...iainc.com,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] x86/resctrl: Clear the stale staged config after the
configuration is completed
Hi Reinette,
On 10/22/2022 2:05 AM, Reinette Chatre wrote:
...
>> It may not be enough to just clear staged_config[] when
>> resctrl_arch_update_domains() exits. I think the fix needs to make
>> sure staged_config[] can be cleared where it is set.
>>
>> The modification of staged_config[] comes from two paths:
>>
>> Path 1:
>> rdtgroup_schemata_write() {
>> ...
>> rdtgroup_parse_resource() // set staged_config[]
>> ...
>> resctrl_arch_update_domains() // clear staged_config[]
>> ...
>> }
>>
>> Path 2:
>> rdtgroup_init_alloc() {
>> ...
>> rdtgroup_init_mba()/rdtgroup_init_cat() // set staged_config[]
>> ...
>> resctrl_arch_update_domains() // clear staged_config[]
>> ...
>> }
>>
>> If we clear staged_config[] in resctrl_arch_update_domains(), goto
>> statement for error handling between setting staged_config[] and
>> calling resctrl_arch_update_domains() will be ignored. This can still
>> remain the stale staged_config[].
> ah - indeed. Thank you for catching that.
>
>>
>> I think maybe it is better to put the clearing work where
>> rdtgroup_schemata_write() and rdtgroup_init_alloc() exit.
>>
>
> It may be more robust to let rdtgroup_init_alloc() follow
> how rdtgroup_schemata_write() already ensures that it is
> working with a clean state by clearing staged_config[] before
> placing its staged config within.
>
I want to make sure, do you mean just ignore the stale value and place
the clearing work before staged_config[] is used? If so, maybe the only
thing the fix should do is to add memset() to rdtgroup_init_alloc().
Thanks,
Shawn
Powered by blists - more mailing lists