From 43e2cbcf82bf0d553c0cb190d4043a6eed86a617 Mon Sep 17 00:00:00 2001 From: Jakub Jermar Date: Sat, 10 Sep 2016 15:56:43 +0200 Subject: [PATCH] Implement interruptible wait for messages sent by the kernel This pertains specifically to handling the IPC_M_PAGE_IN requests sent by the kernel on behalf of the faulting thread. If the external pager process does not respond for some reason, the user may decide to kill the blocked client. That will result in interrupting the client from the sleep. The kernel will then perform cleanup. This changeset implements the cleanup by either forgetting the call (i.e. donating it to the callee) or waiting for the arriving answer, whichever of the two succeeds. --- kernel/generic/src/ipc/ipc.c | 103 ++++++++++++++++++++++++++++++---------- kernel/generic/src/ipc/sysipc.c | 28 +++++++++-- 2 files changed, 102 insertions(+), 29 deletions(-) diff --git a/kernel/generic/src/ipc/ipc.c b/kernel/generic/src/ipc/ipc.c index 85d5e8de9..2309946da 100644 --- a/kernel/generic/src/ipc/ipc.c +++ b/kernel/generic/src/ipc/ipc.c @@ -58,6 +58,8 @@ #include #include +static void ipc_forget_call(call_t *); + /** Open channel that is assigned automatically to new tasks */ answerbox_t *ipc_phone_0 = NULL; @@ -208,11 +210,52 @@ int ipc_call_sync(phone_t *phone, call_t *request) slab_free(ipc_answerbox_slab, mybox); return rc; } - // TODO: forget the call if interrupted - (void) ipc_wait_for_call(mybox, SYNCH_NO_TIMEOUT, SYNCH_FLAGS_NONE); + + call_t *answer = ipc_wait_for_call(mybox, SYNCH_NO_TIMEOUT, + SYNCH_FLAGS_INTERRUPTIBLE); + if (!answer) { + + /* + * The sleep was interrupted. + * + * There are two possibilities now: + * 1) the call gets answered before we manage to forget it + * 2) we manage to forget the call before it gets answered + */ + + spinlock_lock(&request->forget_lock); + spinlock_lock(&TASK->active_calls_lock); + + ASSERT(!request->forget); + + bool answered = !request->active; + if (!answered) { + /* + * The call is not yet answered and we won the race to + * forget it. + */ + ipc_forget_call(request); /* releases locks */ + rc = EINTR; + + } else { + spinlock_unlock(&TASK->active_calls_lock); + spinlock_unlock(&request->forget_lock); + } + + if (answered) { + /* + * The other side won the race to answer the call. + * It is safe to wait for the answer uninterruptibly + * now. + */ + answer = ipc_wait_for_call(mybox, SYNCH_NO_TIMEOUT, + SYNCH_FLAGS_NONE); + } + } + ASSERT(!answer || request == answer); slab_free(ipc_answerbox_slab, mybox); - return EOK; + return rc; } /** Answer a message which was not dispatched and is not listed in any queue. @@ -620,6 +663,36 @@ restart_phones: ipc_call_free(call); } +static void ipc_forget_call(call_t *call) +{ + ASSERT(spinlock_locked(&TASK->active_calls_lock)); + ASSERT(spinlock_locked(&call->forget_lock)); + + /* + * Forget the call and donate it to the task which holds up the answer. + */ + + call->forget = true; + call->sender = NULL; + list_remove(&call->ta_link); + + /* + * The call may be freed by _ipc_answer_free_call() before we are done + * with it; to avoid working with a destroyed call_t structure, we + * must hold a reference to it. + */ + ipc_call_hold(call); + + spinlock_unlock(&call->forget_lock); + spinlock_unlock(&TASK->active_calls_lock); + + atomic_dec(&call->caller_phone->active_calls); + + SYSIPC_OP(request_forget, call); + + ipc_call_release(call); +} + static void ipc_forget_all_active_calls(void) { call_t *call; @@ -648,29 +721,7 @@ restart: goto restart; } - /* - * Forget the call and donate it to the task which holds up the answer. - */ - - call->forget = true; - call->sender = NULL; - list_remove(&call->ta_link); - - /* - * The call may be freed by _ipc_answer_free_call() before we are done - * with it; to avoid working with a destroyed call_t structure, we - * must hold a reference to it. - */ - ipc_call_hold(call); - - spinlock_unlock(&call->forget_lock); - spinlock_unlock(&TASK->active_calls_lock); - - atomic_dec(&call->caller_phone->active_calls); - - SYSIPC_OP(request_forget, call); - - ipc_call_release(call); + ipc_forget_call(call); goto restart; } diff --git a/kernel/generic/src/ipc/sysipc.c b/kernel/generic/src/ipc/sysipc.c index 450176d52..51eb91150 100644 --- a/kernel/generic/src/ipc/sysipc.c +++ b/kernel/generic/src/ipc/sysipc.c @@ -284,14 +284,36 @@ int ipc_req_internal(int phoneid, ipc_data_t *data) udebug_stoppable_begin(); #endif - rc = ipc_call_sync(phone, call); + ipc_call_hold(call); + rc = ipc_call_sync(phone, call); + spinlock_lock(&call->forget_lock); + bool forgotten = call->forget; + spinlock_unlock(&call->forget_lock); + ipc_call_release(call); #ifdef CONFIG_UDEBUG udebug_stoppable_end(); #endif - if (rc != EOK) - return EINTR; + if (rc != EOK) { + if (!forgotten) { + /* + * There was an error, but it did not result + * in the call being forgotten. In fact, the + * call was not even sent. We are still + * its owners and are responsible for its + * deallocation. + */ + ipc_call_free(call); + } else { + /* + * The call was forgotten and it changed hands. + * We are no longer expected to free it. + */ + ASSERT(rc == EINTR); + } + return rc; + } process_answer(call); } else -- 2.11.4.GIT