On 2024/3/19 23:01, Mike Rapoport wrote:
> On Tue, Mar 19, 2024 at 09:22:03AM +0800, Leizhen (ThunderTown) wrote:
>>
>>
>> On 2024/3/19 0:33, Mike Rapoport wrote:
>>> On Mon, Mar 18, 2024 at 09:57:14PM +0800, thunder.leizhen@xxxxxxxxxxxxxxx wrote:
>>>> From: Zhen Lei <thunder.leizhen@xxxxxxxxxx>
>>>>
>>>> The local variable 'page_poisoning_requested' is assigned true at only
>>>> one point. It can be eliminated by moving the code that depends on it
>>>> to the location where it is assigned true. This also make the moved
>>>> code to be compiled only if CONFIG_PAGE_POISONING is set.
>>>
>>> I don't see it as much of an improvement and code readability becomes worse
>>> IMO.
>>
>> Yes, the moved branch will be optimized by the compiler if CONFIG_PAGE_POISONING is
>> not set. But for a reader, he can simply skip over that moved branch when
>> CONFIG_PAGE_POISONING is not set.
>
> Saving one branch at init does not justify the churn and reduced
> readability.
OK, I got it.
>
>>>> Signed-off-by: Zhen Lei <thunder.leizhen@xxxxxxxxxx>
>>>> ---
>>>> mm/mm_init.c | 17 +++++++----------
>>>> 1 file changed, 7 insertions(+), 10 deletions(-)
>>>>
>>>> diff --git a/mm/mm_init.c b/mm/mm_init.c
>>>> index 549e76af8f82a8e..3eb217130bcb2b5 100644
>>>> --- a/mm/mm_init.c
>>>> +++ b/mm/mm_init.c
>>>> @@ -2614,7 +2614,6 @@ DEFINE_STATIC_KEY_MAYBE(CONFIG_DEBUG_VM, check_pages_enabled);
>>>> */
>>>> static void __init mem_debugging_and_hardening_init(void)
>>>> {
>>>> - bool page_poisoning_requested = false;
>>>> bool want_check_pages = false;
>>>>
>>>> #ifdef CONFIG_PAGE_POISONING
>>>> @@ -2626,18 +2625,16 @@ static void __init mem_debugging_and_hardening_init(void)
>>>> (!IS_ENABLED(CONFIG_ARCH_SUPPORTS_DEBUG_PAGEALLOC) &&
>>>> debug_pagealloc_enabled())) {
>>>> static_branch_enable(&_page_poisoning_enabled);
>>>> - page_poisoning_requested = true;
>>>> want_check_pages = true;
>>>> - }
>>>> -#endif
>>>>
>>>> - if ((_init_on_alloc_enabled_early || _init_on_free_enabled_early) &&
>>>> - page_poisoning_requested) {
>>>> - pr_info("mem auto-init: CONFIG_PAGE_POISONING is on, "
>>>> - "will take precedence over init_on_alloc and init_on_free\n");
>>>> - _init_on_alloc_enabled_early = false;
>>>> - _init_on_free_enabled_early = false;
>>>> + if (_init_on_alloc_enabled_early || _init_on_free_enabled_early) {
>>>> + pr_info("mem auto-init: CONFIG_PAGE_POISONING is on, "
>>>> + "will take precedence over init_on_alloc and init_on_free\n");
>>>> + _init_on_alloc_enabled_early = false;
>>>> + _init_on_free_enabled_early = false;
>>>> + }
>>>> }
>>>> +#endif
>>>>
>>>> if (_init_on_alloc_enabled_early) {
>>>> want_check_pages = true;
>>>> --
>>>> 2.34.1
>>>>
>>>
>>
>> --
>> Regards,
>> Zhen Lei
>>
>
--
Regards,
Zhen Lei
