linux-kernel - Re: [PATCH v9] mm/mempolicy: Weighted Interleave Auto-tuning

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <6ce8f663-2bcc-43a0-bbd8-71fc36005e86@redhat.com>
Date: Wed, 21 May 2025 17:40:07 +0200
From: David Hildenbrand <david@...hat.com>
To: Joshua Hahn <joshua.hahnjy@...il.com>
Cc: gourry@...rry.net, akpm@...ux-foundation.org, harry.yoo@...cle.com,
 ying.huang@...ux.alibaba.com, honggyu.kim@...com, yunjeong.mun@...com,
 gregkh@...uxfoundation.org, rakie.kim@...com, rafael@...nel.org,
 lenb@...nel.org, dan.j.williams@...el.com, Jonathan.Cameron@...wei.com,
 dave.jiang@...el.com, horen.chuang@...ux.dev, hannes@...xchg.org,
 osalvador@...e.de, linux-kernel@...r.kernel.org, linux-acpi@...r.kernel.org,
 linux-mm@...ck.org, kernel-team@...a.com
Subject: Re: [PATCH v9] mm/mempolicy: Weighted Interleave Auto-tuning

On 21.05.25 17:37, Joshua Hahn wrote:
> On Wed, 21 May 2025 15:04:00 +0200 David Hildenbrand <david@...hat.com> wrote:
> 
>> On 20.05.25 16:12, Joshua Hahn wrote:
> 
> [...snip...]
>   
>> [...]
>>
>>> -static void iw_table_free(void)
>>> +static void wi_state_free(void)
>>>    {
>>> -	u8 *old;
>>> +	struct weighted_interleave_state *old_wi_state;
>>>    
>>> -	mutex_lock(&iw_table_lock);
>>> -	old = rcu_dereference_protected(iw_table,
>>> -					lockdep_is_held(&iw_table_lock));
>>> -	rcu_assign_pointer(iw_table, NULL);
>>> -	mutex_unlock(&iw_table_lock);
>>> +	mutex_lock(&wi_state_lock);
>>> +
>>> +	old_wi_state = rcu_dereference_protected(wi_state,
>>> +			lockdep_is_held(&wi_state_lock));
>>> +	if (!old_wi_state) {
>>> +		mutex_unlock(&wi_state_lock);
>>> +		goto out;
>>> +	}
>>>    
>>> +	rcu_assign_pointer(wi_state, NULL);
>>> +	mutex_unlock(&wi_state_lock);
>>
>> Just one nit: if written as:
>>
>> ...
>> rcu_assign_pointer(wi_state, NULL);
>> mutex_unlock(&wi_state_lock);
>>
>> old_wi_state = ...
>> if (old_wi_state) {
>> 	synchronize_rcu();
>> 	kfree(old_wi_state);
>> }
>> kfree(&wi_group->wi_kobj);
>>
>> You can easily avoid the goto.
> 
> Ah I see, thank you for the suggestion!
> I think we would have to move the "old_wi_state = ..." to be inside
> the lock and before the rcu_assign_pointer since wi_state will be
> NULL at that point if we do not, but other than that, I think this
> is a great optimization over the version I have : -)
> 
> I will send in a fix patch for this later as a cleanup patch, if
> that sounds good with you!

Sure, you can do a cleanup on top.

-- 
Cheers,

David / dhildenb