From owner-p4-projects@FreeBSD.ORG Fri Apr 10 20:41:47 2009 Return-Path: Delivered-To: p4-projects@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 32767) id 9D0631065673; Fri, 10 Apr 2009 20:41:47 +0000 (UTC) Delivered-To: perforce@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 44CBB106566B for ; Fri, 10 Apr 2009 20:41:47 +0000 (UTC) (envelope-from zec@fer.hr) Received: from repoman.freebsd.org (repoman.freebsd.org [IPv6:2001:4f8:fff6::29]) by mx1.freebsd.org (Postfix) with ESMTP id 296948FC21 for ; Fri, 10 Apr 2009 20:41:47 +0000 (UTC) (envelope-from zec@fer.hr) Received: from repoman.freebsd.org (localhost [127.0.0.1]) by repoman.freebsd.org (8.14.3/8.14.3) with ESMTP id n3AKfltb088584 for ; Fri, 10 Apr 2009 20:41:47 GMT (envelope-from zec@fer.hr) Received: (from perforce@localhost) by repoman.freebsd.org (8.14.3/8.14.3/Submit) id n3AKfkN0088580 for perforce@freebsd.org; Fri, 10 Apr 2009 20:41:46 GMT (envelope-from zec@fer.hr) Date: Fri, 10 Apr 2009 20:41:46 GMT Message-Id: <200904102041.n3AKfkN0088580@repoman.freebsd.org> X-Authentication-Warning: repoman.freebsd.org: perforce set sender to zec@fer.hr using -f From: Marko Zec To: Perforce Change Reviews Cc: Subject: PERFORCE change 160461 for review X-BeenThere: p4-projects@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: p4 projects tree changes List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 10 Apr 2009 20:41:48 -0000 http://perforce.freebsd.org/chv.cgi?CH=160461 Change 160461 by zec@zec_tpx32 on 2009/04/10 20:41:17 Merge from vimage branch an explicit check for circular (or for any reason unresolved) dependencies between vnet modules. This check, scheduled right before the kernel going multiuser, looks at the vinet_modpending TAILQ for any pending vnet modules, prints them out, and panics. The effectivenes of this check can be easily demonstrated by say adding a dependency on INET6 to INET, while INET6 is already depending on INET, so none of them would ever get registered. bz suggested that we might wish to commit this as a separate patch, but I'm inclined to proclaim this check to be an integral and essential part of the vnet_mod_registration framework, though it doesn't help with checking for unresolved vnet_mod dependencies for modules kldloaded after system initialization is complete. While here, attempt to improve consistency and verbosity of a few comments and debugging printf-s. Affected files ... .. //depot/projects/vimage-commit2/src/sys/kern/kern_vimage.c#13 edit .. //depot/projects/vimage-commit2/src/sys/sys/kernel.h#7 edit Differences ... ==== //depot/projects/vimage-commit2/src/sys/kern/kern_vimage.c#13 (text+ko) ==== @@ -61,32 +61,40 @@ { struct vnet_modlink *vml, *vml_iter; - /* Do not register the same module instance more than once. */ + /* Do not register the same {module, iarg} pair more than once. */ TAILQ_FOREACH(vml_iter, &vnet_modlink_head, vml_mod_le) if (vml_iter->vml_modinfo == vmi && vml_iter->vml_iarg == iarg) break; if (vml_iter != NULL) - panic("attempt to register an already registered vnet module"); + panic("registering an already registered vnet module: %s", + vml_iter->vml_modinfo->vmi_name); vml = malloc(sizeof(struct vnet_modlink), M_VIMAGE, M_NOWAIT); /* * XXX we support only statically assigned module IDs at the time. * In principle modules should be able to get a dynamically * assigned ID at registration time. + * + * If a module is registered in multiple instances, then each + * instance must have both iarg and iname set. */ - VNET_ASSERT(vmi->vmi_id > 0 || vmi->vmi_id < VNET_MOD_MAX); - VNET_ASSERT(!((iarg == NULL) ^ (iname == NULL))); + if (vmi->vmi_id >= VNET_MOD_MAX) + panic("invalid vnet module ID: %d", vmi->vmi_id); + if (vmi->vmi_name == NULL) + panic("vnet module with no name: %d", vmi->vmi_id); + if ((iarg == NULL) ^ (iname == NULL)) + panic("invalid vnet module instance: %s", vmi->vmi_name); vml->vml_modinfo = vmi; vml->vml_iarg = iarg; vml->vml_iname = iname; - /* Check whether the module we depend on is already registered */ + /* Check whether the module we depend on is already registered. */ if (vmi->vmi_dependson != vmi->vmi_id) { TAILQ_FOREACH(vml_iter, &vnet_modlink_head, vml_mod_le) if (vml_iter->vml_modinfo->vmi_id == vmi->vmi_dependson) - break; /* Depencency found, we are done */ + break; /* Depencency found, we are done. */ if (vml_iter == NULL) { #ifdef DEBUG_ORDERING printf("dependency %d missing for vnet mod %s," @@ -116,7 +124,7 @@ CURVNET_RESTORE(); } - /* Check for pending modules depending on us */ + /* Check for pending modules depending on us. */ do { TAILQ_FOREACH(vml_iter, &vnet_modpending_head, vml_mod_le) if (vml_iter->vml_modinfo->vmi_dependson == @@ -141,7 +149,7 @@ const struct vnet_modinfo *vmi = vml->vml_modinfo; #ifdef DEBUG_ORDERING - printf("instatiating vnet_%s", vmi->vmi_name); + printf("instantiating vnet_%s", vmi->vmi_name); if (vml->vml_iarg) printf("/%s", vml->vml_iname); printf(": "); @@ -156,7 +164,7 @@ if (vmi->vmi_struct_size) { void *mem = malloc(vmi->vmi_struct_size, M_VNET, M_NOWAIT | M_ZERO); - if (mem == NULL) /* XXX should return error, not panic */ + if (mem == NULL) /* XXX should return error, not panic. */ panic("vi_alloc: malloc for %s\n", vmi->vmi_name); curvnet->mod_data[vmi->vmi_id] = mem; } @@ -200,8 +208,27 @@ { TAILQ_INIT(&vnet_modlink_head); + TAILQ_INIT(&vnet_modpending_head); +} + +static void +vi_init_done(void *unused) +{ + struct vnet_modlink *vml_iter; + + if (TAILQ_EMPTY(&vnet_modpending_head)) + return; + + printf("vnet modules with unresolved dependencies:\n"); + TAILQ_FOREACH(vml_iter, &vnet_modpending_head, vml_mod_le) + printf(" %d:%s depending on %d\n", + vml_iter->vml_modinfo->vmi_id, + vml_iter->vml_modinfo->vmi_name, + vml_iter->vml_modinfo->vmi_dependson); + panic("going nowhere without my vnet modules!"); } SYSINIT(vimage, SI_SUB_VIMAGE, SI_ORDER_FIRST, vi_init, NULL); +SYSINIT(vimage_done, SI_SUB_VIMAGE_DONE, SI_ORDER_FIRST, vi_init_done, NULL); #endif /* !VIMAGE_GLOBALS */ ==== //depot/projects/vimage-commit2/src/sys/sys/kernel.h#7 (text+ko) ==== @@ -163,6 +163,7 @@ SI_SUB_SWAP = 0xc000000, /* swap */ SI_SUB_INTRINSIC_POST = 0xd000000, /* proc 0 cleanup*/ SI_SUB_SYSCALLS = 0xd800000, /* register system calls */ + SI_SUB_VIMAGE_DONE = 0xdc00000, /* vnet registration complete */ SI_SUB_KTHREAD_INIT = 0xe000000, /* init process*/ SI_SUB_KTHREAD_PAGE = 0xe400000, /* pageout daemon*/ SI_SUB_KTHREAD_VM = 0xe800000, /* vm daemon*/