From: Cristiano Giuffrida <cristiano@minix3.org>
Date: Mon, 26 Apr 2010 14:43:59 +0000 (+0000)
Subject: Set IPC status code only for RECEIVE
X-Git-Tag: v3.1.7~125
X-Git-Url: http://zhaoyanbai.com/repos/%22http:/www.isc.org/icons/man.host.html?a=commitdiff_plain;h=0f353411d7934f90fc4cbb6d7869a1d38474253c;p=minix.git

Set IPC status code only for RECEIVE
---

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;
 			}