From 80846c4a79da30c85da71b1dfcb359257dabbbb7 Mon Sep 17 00:00:00 2001 From: Ben Gras Date: Wed, 1 May 2013 19:02:06 +0000 Subject: [PATCH] kernel ipc debug: various fixes . add receive hooks in the kernel to print asynchronously delivered messages . do not rely on MF_REPLY_PEND to decide between calls and errors, as that isn't reliable for asynchronous messages; try both instead . add _sendcall() that extract-mfield.sh can then reliably recognize the fields for messages that are sent with just send() . add DEBUG_DUMPIPC_NAMES to restrict printed messages to from/to given process names Change-Id: Ia65eb02a69a2b58e73bf9f009987be06dda774a3 --- drivers/tty/tty.c | 17 +------ include/minix/syslib.h | 3 ++ kernel/Makefile | 2 +- kernel/debug.c | 89 ++++++++++++++++++++++++++++--------- kernel/debug.h | 3 ++ kernel/extract-mfield.sh | 2 +- kernel/proc.c | 6 +++ lib/libsys/Makefile | 1 + lib/libsys/send_taskreply.c | 20 +++++++++ lib/libsys/taskcall.c | 7 +++ 10 files changed, 112 insertions(+), 38 deletions(-) create mode 100644 lib/libsys/send_taskreply.c diff --git a/drivers/tty/tty.c b/drivers/tty/tty.c index b1463bcca..8c28b0927 100644 --- a/drivers/tty/tty.c +++ b/drivers/tty/tty.c @@ -1660,25 +1660,12 @@ int proc_nr; /* to whom should the reply go? */ int status; /* reply code */ { /* Send a reply to a process that wanted to read or write data. */ - message tty_mess; - - tty_mess.m_type = code; - tty_mess.REP_ENDPT = proc_nr; - tty_mess.REP_STATUS = status; + assert(code == TASK_REPLY); /* Don't reply to KERNEL (kernel messages) */ if (replyee == KERNEL) return; - /* TTY is not supposed to send a TTY_REVIVE message. The - * REVIVE message is gone, TTY_REVIVE is only used as an internal - * placeholder for something that is not supposed to be a message. - */ - if(code == TTY_REVIVE) { - printf("%s:%d: ", file, line); - panic("tty_reply sending TTY_REVIVE"); - } - - status = send(replyee, &tty_mess); + status = send_taskreply(replyee, proc_nr, status); if (status != OK) printf("tty`tty_reply: send to %d failed: %d\n", replyee, status); } diff --git a/include/minix/syslib.h b/include/minix/syslib.h index 4d373843d..a5fcefc6f 100644 --- a/include/minix/syslib.h +++ b/include/minix/syslib.h @@ -25,6 +25,7 @@ struct rs_pci; *==========================================================================*/ int _taskcall(endpoint_t who, int syscallnr, message *msgptr); int _kernel_call(int syscallnr, message *msgptr); +int _sendcall(endpoint_t who, int type, message *msgptr); int sys_abort(int how); int sys_enable_iop(endpoint_t proc_ep); @@ -157,6 +158,8 @@ int sys_umap_data_fb(endpoint_t proc_ep, vir_bytes vir_addr, vir_bytes int sys_umap_remote(endpoint_t proc_ep, endpoint_t grantee, int seg, vir_bytes vir_addr, vir_bytes bytes, phys_bytes *phys_addr); +int send_taskreply(endpoint_t who, endpoint_t endpoint, int status); + /* Shorthands for sys_getinfo() system call. */ #define sys_getkinfo(dst) sys_getinfo(GET_KINFO, dst, 0,0,0) #define sys_getloadinfo(dst) sys_getinfo(GET_LOADINFO, dst, 0,0,0) diff --git a/kernel/Makefile b/kernel/Makefile index 0a1d6f7f7..f89a68b81 100644 --- a/kernel/Makefile +++ b/kernel/Makefile @@ -72,7 +72,7 @@ CPPFLAGS+= -DUSE_TRACE CLEANFILES+=extracted-errno.h extracted-mfield.h extracted-mtype.h procoffsets.h -debug.d: extracted-errno.h extracted-mfield.h extracted-mtype.h +debug.o: extracted-errno.h extracted-mfield.h extracted-mtype.h extracted-errno.h: extract-errno.sh ../include/errno.h ${_MKTARGET_CREATE} diff --git a/kernel/debug.c b/kernel/debug.c index 45db4f705..751d17a6b 100644 --- a/kernel/debug.c +++ b/kernel/debug.c @@ -313,25 +313,45 @@ void print_proc_recursive(struct proc *pp) } #if DEBUG_DUMPIPC -static const char *mtypename(int mtype, int iscall) +static const char *mtypename(int mtype, int *possible_callname) { - /* use generated file to recognize message types */ - if (iscall) { - switch(mtype) { -#define IDENT(x) case x: return #x; + char *callname = NULL, *errname = NULL; + /* use generated file to recognize message types + * + * we try to match both error numbers and call numbers, as the + * reader can probably decide from context what's going on. + * + * whenever it might be a call number we tell the caller so the + * call message fields can be decoded if known. + */ + switch(mtype) { +#define IDENT(x) case x: callname = #x; *possible_callname = 1; break; #include "extracted-mtype.h" #undef IDENT - } - } else { - switch(mtype) { -#define IDENT(x) case x: return #x; + } + switch(mtype) { +#define IDENT(x) case x: errname = #x; break; #include "extracted-errno.h" #undef IDENT - } } /* no match */ - return NULL; + if(!errname && !callname) + return NULL; + + /* 2 matches */ + if(errname && callname) { + static char typename[100]; + strcpy(typename, errname); + strcat(typename, " / "); + strcat(typename, callname); + return typename; + } + + if(errname) return errname; + + assert(callname); + return callname; } static void printproc(struct proc *rp) @@ -353,24 +373,51 @@ static void printparam(const char *name, const void *data, size_t size) } } +#ifdef DEBUG_DUMPIPC_NAMES +static int namematch(char **names, int nnames, char *name) +{ + int i; + for(i = 0; i < nnames; i++) + if(!strcmp(names[i], name)) + return 1; + return 0; +} +#endif + static void printmsg(message *msg, struct proc *src, struct proc *dst, - char operation, int iscall, int printparams) + char operation, int printparams) { const char *name; - int mtype = msg->m_type; + int mtype = msg->m_type, mightbecall = 0; + +#ifdef DEBUG_DUMPIPC_NAMES + { + char *names[] = DEBUG_DUMPIPC_NAMES; + int nnames = sizeof(names)/sizeof(names[0]); + + /* skip printing messages for messages neither to + * or from DEBUG_DUMPIPC_EP if it is defined; either + * can be NULL to indicate kernel + */ + if(!(src && namematch(names, nnames, src->p_name)) && + !(dst && namematch(names, nnames, dst->p_name))) { + return; + } + } +#endif /* source, destination and message type */ printf("%c", operation); printproc(src); printproc(dst); - name = mtypename(mtype, iscall); + name = mtypename(mtype, &mightbecall); if (name) { - printf(" %s(%d)", name, mtype); + printf(" %s(%d/0x%x)", name, mtype, mtype); } else { - printf(" %d", mtype); + printf(" %d/0x%x", mtype, mtype); } - if (iscall && printparams) { + if (mightbecall && printparams) { #define IDENT(x, y) if (mtype == x) printparam(#y, &msg->y, sizeof(msg->y)); #include "extracted-mfield.h" #undef IDENT @@ -473,14 +520,14 @@ static void statmsg(message *msg, struct proc *srcp, struct proc *dstp) void hook_ipc_msgkcall(message *msg, struct proc *proc) { #if DEBUG_DUMPIPC - printmsg(msg, proc, NULL, 'k', 1, 1); + printmsg(msg, proc, NULL, 'k', 1); #endif } void hook_ipc_msgkresult(message *msg, struct proc *proc) { #if DEBUG_DUMPIPC - printmsg(msg, NULL, proc, 'k', 0, 0); + printmsg(msg, NULL, proc, 'k', 0); #endif #if DEBUG_IPCSTATS statmsg(msg, proc, NULL); @@ -490,7 +537,7 @@ void hook_ipc_msgkresult(message *msg, struct proc *proc) void hook_ipc_msgrecv(message *msg, struct proc *src, struct proc *dst) { #if DEBUG_DUMPIPC - printmsg(msg, src, dst, 'r', src->p_misc_flags & MF_REPLY_PEND, 0); + printmsg(msg, src, dst, 'r', 0); #endif #if DEBUG_IPCSTATS statmsg(msg, src, dst); @@ -500,7 +547,7 @@ void hook_ipc_msgrecv(message *msg, struct proc *src, struct proc *dst) void hook_ipc_msgsend(message *msg, struct proc *src, struct proc *dst) { #if DEBUG_DUMPIPC - printmsg(msg, src, dst, 's', src->p_misc_flags & MF_REPLY_PEND, 1); + printmsg(msg, src, dst, 's', 1); #endif } diff --git a/kernel/debug.h b/kernel/debug.h index f84427e20..a41dd77f0 100644 --- a/kernel/debug.h +++ b/kernel/debug.h @@ -46,6 +46,9 @@ */ #define DEBUG_DUMPIPC 0 +/* If defined, restrict DEBUG_DUMPIPC to particular process names */ +/* #define DEBUG_DUMPIPC_NAMES { "tty", "inet" } */ + /* DEBUG_IPCSTATS collects information on who sends messages to whom. */ #define DEBUG_IPCSTATS 0 diff --git a/kernel/extract-mfield.sh b/kernel/extract-mfield.sh index a84ba8f56..4a1664768 100644 --- a/kernel/extract-mfield.sh +++ b/kernel/extract-mfield.sh @@ -5,7 +5,7 @@ set -e find_files_and_lines() ( find ../lib/libc/sys-minix ../lib/libsys -name '*.c' | \ - xargs egrep -n '((_syscall|_taskcall)\([^,][^,]*,[ ]*|_kernel_call\()[A-Z_][A-Z0-9_]*,[ ]*&m\)' | \ + xargs egrep -n '((_sendcall|_syscall|_taskcall)\([^,][^,]*,[ ]*|_kernel_call\()[A-Z_][A-Z0-9_]*,[ ]*&m\)' | \ cut -d: -f1,2 ) diff --git a/kernel/proc.c b/kernel/proc.c index 8fb066dcd..a15039315 100644 --- a/kernel/proc.c +++ b/kernel/proc.c @@ -1223,6 +1223,9 @@ int try_deliver_senda(struct proc *caller_ptr, dst_ptr->p_misc_flags |= MF_DELIVERMSG; IPC_STATUS_ADD_CALL(dst_ptr, SENDA); RTS_UNSET(dst_ptr, RTS_RECEIVING); +#if DEBUG_IPC_HOOK + hook_ipc_msgrecv(&dst_ptr->p_delivermsg, caller_ptr, dst_ptr); +#endif } else if (r == OK) { /* Inform receiver that something is pending */ set_sys_bit(priv(dst_ptr)->s_asyn_pending, @@ -1398,6 +1401,9 @@ static int try_one(struct proc *src_ptr, struct proc *dst_ptr) dst_ptr->p_delivermsg = tabent.msg; dst_ptr->p_delivermsg.m_source = src_ptr->p_endpoint; dst_ptr->p_misc_flags |= MF_DELIVERMSG; +#if DEBUG_IPC_HOOK + hook_ipc_msgrecv(&dst_ptr->p_delivermsg, src_ptr, dst_ptr); +#endif store_result: /* Store results for sender */ diff --git a/lib/libsys/Makefile b/lib/libsys/Makefile index 490f62e75..ba949cbda 100644 --- a/lib/libsys/Makefile +++ b/lib/libsys/Makefile @@ -75,6 +75,7 @@ SRCS+= \ sys_vsafecopy.c \ sys_vtimer.c \ sys_vumap.c \ + send_taskreply.c \ taskcall.c \ tickdelay.c \ timers.c \ diff --git a/lib/libsys/send_taskreply.c b/lib/libsys/send_taskreply.c new file mode 100644 index 000000000..c14daf573 --- /dev/null +++ b/lib/libsys/send_taskreply.c @@ -0,0 +1,20 @@ + +#include + +#include "syslib.h" + +/*===========================================================================* + * sys_taskreply * + *===========================================================================*/ +int send_taskreply(endpoint_t who, endpoint_t endpoint, int status) +{ + message m; + + memset(&m, 0, sizeof(m)); + + m.REP_ENDPT = endpoint; + m.REP_STATUS = status; + + return _sendcall(who, TASK_REPLY, &m); +} + diff --git a/lib/libsys/taskcall.c b/lib/libsys/taskcall.c index b547471b7..eafa99cd1 100644 --- a/lib/libsys/taskcall.c +++ b/lib/libsys/taskcall.c @@ -6,6 +6,13 @@ #include #include +int _sendcall(endpoint_t who, int type, message *msgptr) +{ + msgptr->m_type = type; + return send(who, msgptr); +} + + int _taskcall(who, syscallnr, msgptr) endpoint_t who; int syscallnr;