Re: [PATCH] memory tier: fix deadlock warning while onlining pages

["Huang, Ying" <ying.huang@intel.com>]

Yanfei Xu <yanfei.xu@intel.com> writes:

> [Snip]
>
> On 8/30/2024 3:11 PM, Huang, Ying wrote:
>>> Fixes: 823430c8e9d9 ("memory tier: consolidate the initialization of memory tiers")
>>> Signed-off-by: Yanfei Xu<yanfei.xu@intel.com>
>>> ---
>>>   mm/memory-tiers.c | 1 -
>>>   1 file changed, 1 deletion(-)
>>>
>>> diff --git a/mm/memory-tiers.c b/mm/memory-tiers.c
>>> index 4775b3a3dabe..dddcd6b38e28 100644
>>> --- a/mm/memory-tiers.c
>>> +++ b/mm/memory-tiers.c
>>> @@ -895,7 +895,6 @@ static int __init memory_tier_init(void)
>>>   	WARN_ON(!node_demotion);
>>>   #endif
>>>   -	guard(mutex)(&memory_tier_lock);
>>>   	/*
>>>   	 * For now we can have 4 faster memory tiers with smaller adistance
>>>   	 * than default DRAM tier.
>> Although it's not absolutely necessary, I still think that it's better
>> to just revert the locking change of memory_tier_init() in commit
>> 823430c8e9d9 ("memory tier: consolidate the initialization of memory
>> tiers").  That is, to use mutex_lock/unlock() and exclude
>> hotplug_memory_notifier() from the locked region.  Because we will
>> always hold memory_tier_lock when working on memory tier related data
>> structures in this way.
>
> Thanks for pointing out it. Will do it as below in v2.
>
> diff --git a/mm/memory-tiers.c b/mm/memory-tiers.c
> index 4775b3a3dabe..ba3738b29cc5 100644
> --- a/mm/memory-tiers.c
> +++ b/mm/memory-tiers.c
> @@ -895,13 +895,14 @@ static int __init memory_tier_init(void)
>         WARN_ON(!node_demotion);
>  #endif
>
> -       guard(mutex)(&memory_tier_lock);
> +       mutex_lock(&memory_tier_lock);
>         /*
>          * For now we can have 4 faster memory tiers with smaller adistance
>          * than default DRAM tier.
>          */
>         default_dram_type =
>         mt_find_alloc_memory_type(MEMTIER_ADISTANCE_DRAM,
>  &default_memory_types);
> +       mutex_unlock(&memory_tier_lock);
>         if (IS_ERR(default_dram_type))
>                 panic("%s() failed to allocate default DRAM tier\n",
>                 __func__);
>
>

LGTM, feel free to add in the future version.

Reviewed-by: "Huang, Ying" <ying.huang@intel.com>