Re: [PATCH v2 2/3] futex: Create set_robust_list2
From: André Almeida <andrealmeid@igalia.com>
Date: 2024-11-04 21:56:00
Also in:
lkml
Hi Peter, Em 04/11/2024 08:22, Peter Zijlstra escreveu:
On Fri, Nov 01, 2024 at 01:21:46PM -0300, André Almeida wrote:quoted
@@ -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.
You are asking if, whenever someone calls set_robust_list() or compat_set_robust_list() to be inserted into ¤t->robust_list2 instead of using tsk->robust_list and tsk->compat_robust_list? I was thinking of doing that, but my current implementation has a kmalloc() call for every insertion, and I wasn't sure if I could add this new latency to the old set_robust_list() syscall. Assuming it is usually called just once during the thread initialization perhaps it shouldn't cause much harm I guess.