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