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

David Hildenbrand
2025-05-21T15:40:12Z

On 21.05.25 17:37, Joshua Hahn wrote:
> On Wed, 21 May 2025 15:04:00 +0200 David Hildenbrand <david@redhat.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