From 0f353411d7934f90fc4cbb6d7869a1d38474253c Mon Sep 17 00:00:00 2001 From: Cristiano Giuffrida Date: Mon, 26 Apr 2010 14:43:59 +0000 Subject: [PATCH] Set IPC status code only for RECEIVE --- kernel/ipc.h | 26 +++++++++++++++++++++++--- kernel/proc.c | 26 +++++++++++--------------- 2 files changed, 34 insertions(+), 18 deletions(-) diff --git a/kernel/ipc.h b/kernel/ipc.h index 743825413..3007a7032 100644 --- a/kernel/ipc.h +++ b/kernel/ipc.h @@ -17,8 +17,28 @@ /* IPC status code macros. */ #define IPC_STATUS_REG bx #define IPC_STATUS_GET(p) ((p)->p_reg.IPC_STATUS_REG) -#define IPC_STATUS_SET(p, m) ((p)->p_reg.IPC_STATUS_REG = m) -#define IPC_STATUS_CLEAR(p) IPC_STATUS_SET(p, 0) -#define IPC_STATUS_ADD(p, m) ((p)->p_reg.IPC_STATUS_REG |= m) +#define IPC_STATUS_CLEAR(p) ((p)->p_reg.IPC_STATUS_REG = 0) + +/* + * XXX: the following check is used to set the status code only on RECEIVE. + * SENDREC is not currently atomic for user processes. A process can return + * from SENDREC in a different context than the original when a Posix signal + * handler gets executed. For this reason, it is not safe to manipulate + * the context (i.e. registers) when a process is blocked on a SENDREC. + * Unfortunately, avoiding setting the status code for SENDREC doesn't solve + * the problem entirely because in rare situations it is still necessary to + * override retreg dynamically (and possibly in a different context). + * A possible reliable solution is to improve our Posix signal handling + * implementation and guarantee SENDREC atomicity w.r.t. the process context. + */ +#define IPC_STATUS_ADD(p, m) do { \ + if(!((p)->p_misc_flags & MF_REPLY_PEND)) { \ + (p)->p_reg.IPC_STATUS_REG |= (m); \ + } \ + } while(0) +#define IPC_STATUS_ADD_CALL(p, call) \ + IPC_STATUS_ADD(p, IPC_STATUS_CALL_TO(call)) +#define IPC_STATUS_ADD_FLAGS(p, flags) \ + IPC_STATUS_ADD(p, IPC_STATUS_FLAGS(flags)) #endif /* IPC_H */ diff --git a/kernel/proc.c b/kernel/proc.c index 0053e2774..d7df9a8e9 100644 --- a/kernel/proc.c +++ b/kernel/proc.c @@ -339,8 +339,10 @@ PRIVATE int do_sync_ipc(struct proc * caller_ptr, /* who made the call */ break; /* done, or SEND failed */ /* fall through for SENDREC */ case RECEIVE: - if (call_nr == RECEIVE) + if (call_nr == RECEIVE) { caller_ptr->p_misc_flags &= ~MF_REPLY_PEND; + IPC_STATUS_CLEAR(caller_ptr); /* clear IPC status code */ + } result = mini_receive(caller_ptr, src_dst_e, m_ptr, 0); break; case NOTIFY: @@ -397,9 +399,6 @@ PUBLIC int do_ipc(reg_t r1, reg_t r2, reg_t r3) caller_ptr->p_name, caller_ptr->p_endpoint); } - /* Clear IPC status code. */ - IPC_STATUS_CLEAR(caller_ptr); - /* Now check if the call is known and try to perform the request. The only * system calls that exist in MINIX are sending and receiving messages. * - SENDREC: combines SEND and RECEIVE in a single system call @@ -540,8 +539,7 @@ const int flags; return EFAULT; } else { dst_ptr->p_delivermsg = *m_ptr; - IPC_STATUS_ADD(dst_ptr, - IPC_STATUS_FLAGS(IPC_FLG_MSG_FROM_KERNEL)); + IPC_STATUS_ADD_FLAGS(dst_ptr, IPC_FLG_MSG_FROM_KERNEL); } dst_ptr->p_delivermsg.m_source = caller_ptr->p_endpoint; @@ -549,7 +547,7 @@ const int flags; call = (caller_ptr->p_misc_flags & MF_REPLY_PEND ? SENDREC : (flags & NON_BLOCKING ? SENDNB : SEND)); - IPC_STATUS_ADD(dst_ptr, IPC_STATUS_CALL_TO(call)); + IPC_STATUS_ADD_CALL(dst_ptr, call); RTS_UNSET(dst_ptr, RTS_RECEIVING); } else { if(flags & NON_BLOCKING) { @@ -665,7 +663,7 @@ const int flags; caller_ptr->p_delivermsg.m_source = hisep; caller_ptr->p_misc_flags |= MF_DELIVERMSG; - IPC_STATUS_ADD(caller_ptr, IPC_STATUS_CALL_TO(NOTIFY)); + IPC_STATUS_ADD_CALL(caller_ptr, NOTIFY); return(OK); } @@ -680,7 +678,7 @@ const int flags; r= try_async(caller_ptr); if (r == OK) { - IPC_STATUS_ADD(caller_ptr, IPC_STATUS_CALL_TO(SENDA)); + IPC_STATUS_ADD_CALL(caller_ptr, SENDA); return OK; /* Got a message */ } } @@ -701,15 +699,14 @@ const int flags; RTS_UNSET(*xpp, RTS_SENDING); call = ((*xpp)->p_misc_flags & MF_REPLY_PEND ? SENDREC : SEND); - IPC_STATUS_ADD(caller_ptr, IPC_STATUS_CALL_TO(call)); + IPC_STATUS_ADD_CALL(caller_ptr, call); /* * if the message is originaly from the kernel on behalf of this * process, we must send the status flags accordingly */ if ((*xpp)->p_misc_flags & MF_SENDING_FROM_KERNEL) { - IPC_STATUS_ADD(caller_ptr, - IPC_STATUS_FLAGS(IPC_FLG_MSG_FROM_KERNEL)); + IPC_STATUS_ADD_FLAGS(caller_ptr, IPC_FLG_MSG_FROM_KERNEL); /* we can clean the flag now, not need anymore */ (*xpp)->p_misc_flags &= ~MF_SENDING_FROM_KERNEL; } @@ -775,7 +772,7 @@ PUBLIC int mini_notify( dst_ptr->p_delivermsg.m_source = caller_ptr->p_endpoint; dst_ptr->p_misc_flags |= MF_DELIVERMSG; - IPC_STATUS_ADD(dst_ptr, IPC_STATUS_CALL_TO(NOTIFY)); + IPC_STATUS_ADD_CALL(dst_ptr, NOTIFY); RTS_UNSET(dst_ptr, RTS_RECEIVING); return(OK); @@ -957,8 +954,7 @@ PRIVATE int mini_senda(struct proc *caller_ptr, asynmsg_t *table, size_t size) else { dst_ptr->p_delivermsg.m_source = caller_ptr->p_endpoint; dst_ptr->p_misc_flags |= MF_DELIVERMSG; - IPC_STATUS_ADD(dst_ptr, - IPC_STATUS_CALL_TO(SENDA)); + IPC_STATUS_ADD_CALL(dst_ptr, SENDA); RTS_UNSET(dst_ptr, RTS_RECEIVING); tabent.result = OK; } -- 2.44.0