Re: [PATCH v2 2/3] futex: Create set_robust_list2
From: Peter Zijlstra <peterz@infradead.org>
Date: 2024-11-04 11:22:56
Also in:
lkml
On Fri, Nov 01, 2024 at 01:21:46PM -0300, André Almeida wrote:
quoted hunk ↗ jump to hunk
@@ -1046,24 +1095,44 @@ static inline void exit_pi_state_list(struct task_struct *curr) { } static void futex_cleanup(struct task_struct *tsk) { + struct robust_list2_entry *curr, *n; + struct list_head *list2 = &tsk->robust_list2; + #ifdef CONFIG_64BIT if (unlikely(tsk->robust_list)) { - exit_robust_list64(tsk); + exit_robust_list64(tsk, tsk->robust_list); tsk->robust_list = NULL; } #else if (unlikely(tsk->robust_list)) { - exit_robust_list32(tsk); + exit_robust_list32(tsk, (struct robust_list_head32 *) tsk->robust_list); tsk->robust_list = NULL; } #endif #ifdef CONFIG_COMPAT if (unlikely(tsk->compat_robust_list)) { - exit_robust_list32(tsk); + exit_robust_list32(tsk, tsk->compat_robust_list); tsk->compat_robust_list = NULL; } #endif + /* + * Walk through the linked list, parsing robust lists and freeing the + * allocated lists + */ + if (unlikely(!list_empty(list2))) { + list_for_each_entry_safe(curr, n, list2, list) { + if (curr->head != NULL) { + if (curr->list_type == ROBUST_LIST_64BIT) + exit_robust_list64(tsk, curr->head); + else if (curr->list_type == ROBUST_LIST_32BIT) + exit_robust_list32(tsk, curr->head); + curr->head = NULL; + } + list_del_init(&curr->list); + kfree(curr); + } + } if (unlikely(!list_empty(&tsk->pi_state_list))) exit_pi_state_list(tsk);
I'm still digesting this, but the above seems particularly silly. Should not the legacy lists also be on the list of lists? I mean, it makes no sense to have two completely separate means of tracking lists.