From 9a7074f4cb924ae82908691b9014ec3046fa7a8a Mon Sep 17 00:00:00 2001 From: phk Date: Wed, 10 Mar 2004 08:49:08 +0000 Subject: Rearrange some of the GEOM debugging tools to be more structured. Retire g_sanity() and corresponding debugflag (0x8) Retire g_{stall,release}_events(). Under #ifdef DIAGNOSTIC: Make g_valid_obj() an official function and have it return an an non-zero integer which indicates the kind of object when found. Implement G_VALID_{CLASS,GEOM,CONSUMER,PROVIDER}() macros based on g_valid_obj(). Sprinkle calls to these macros liberally over the infrastructure. Always check that we do not free a live object. --- sys/geom/geom.h | 30 +++++++++++----- sys/geom/geom_dump.c | 1 - sys/geom/geom_event.c | 27 ++++---------- sys/geom/geom_int.h | 4 +-- sys/geom/geom_subr.c | 99 ++++++++++++++++++++++----------------------------- 5 files changed, 71 insertions(+), 90 deletions(-) (limited to 'sys/geom') diff --git a/sys/geom/geom.h b/sys/geom/geom.h index 63db46a..31002fe 100644 --- a/sys/geom/geom.h +++ b/sys/geom/geom.h @@ -221,13 +221,29 @@ int g_handleattr_off_t(struct bio *bp, const char *attribute, off_t val); struct g_consumer * g_new_consumer(struct g_geom *gp); struct g_geom * g_new_geomf(struct g_class *mp, const char *fmt, ...); struct g_provider * g_new_providerf(struct g_geom *gp, const char *fmt, ...); -void g_sanity(void const *ptr); void g_spoil(struct g_provider *pp, struct g_consumer *cp); int g_std_access(struct g_provider *pp, int dr, int dw, int de); void g_std_done(struct bio *bp); void g_std_spoiled(struct g_consumer *cp); void g_wither_geom(struct g_geom *gp, int error); +#ifdef DIAGNOSTIC +int g_valid_obj(void const *ptr); +#define G_VALID_CLASS(foo) \ + KASSERT(g_valid_obj(foo) == 1, ("%p is not a g_class", foo)) +#define G_VALID_GEOM(foo) \ + KASSERT(g_valid_obj(foo) == 2, ("%p is not a g_geom", foo)) +#define G_VALID_CONSUMER(foo) \ + KASSERT(g_valid_obj(foo) == 3, ("%p is not a g_consumer", foo)) +#define G_VALID_PROVIDER(foo) \ + KASSERT(g_valid_obj(foo) == 4, ("%p is not a g_provider", foo)) +#else +#define G_VALID_CLASS(foo) do { } while (0) +#define G_VALID_GEOM(foo) do { } while (0) +#define G_VALID_CONSUMER(foo) do { } while (0) +#define G_VALID_PROVIDER(foo) do { } while (0) +#endif + int g_modevent(module_t, int, void *); /* geom_io.c */ @@ -258,16 +274,17 @@ g_malloc(int size, int flags) void *p; p = malloc(size, M_GEOM, flags); - g_sanity(p); - /* printf("malloc(%d, %x) -> %p\n", size, flags, p); */ return (p); } static __inline void g_free(void *ptr) { - g_sanity(ptr); - /* printf("free(%p)\n", ptr); */ + +#ifdef DIAGNOSTIC + KASSERT(g_valid_obj(ptr) == 0, + ("g_free(%p) of live object, type %d", ptr, g_valid_obj(ptr))); +#endif free(ptr, M_GEOM); } @@ -281,19 +298,16 @@ extern struct sx topology_lock; #define g_topology_unlock() \ do { \ - g_sanity(NULL); \ sx_xunlock(&topology_lock); \ } while (0) #define g_topology_assert() \ do { \ - g_sanity(NULL); \ sx_assert(&topology_lock, SX_XLOCKED); \ } while (0) #define g_topology_assert_not() \ do { \ - g_sanity(NULL); \ sx_assert(&topology_lock, SX_UNLOCKED); \ } while (0) diff --git a/sys/geom/geom_dump.c b/sys/geom/geom_dump.c index f110ceb..fd7001f 100644 --- a/sys/geom/geom_dump.c +++ b/sys/geom/geom_dump.c @@ -273,7 +273,6 @@ g_trace(int level, const char *fmt, ...) { va_list ap; - g_sanity(NULL); if (!(g_debugflags & level)) return; va_start(ap, fmt); diff --git a/sys/geom/geom_event.c b/sys/geom/geom_event.c index cf86b50..afb214f 100644 --- a/sys/geom/geom_event.c +++ b/sys/geom/geom_event.c @@ -59,7 +59,6 @@ static struct event_tailq_head g_events = TAILQ_HEAD_INITIALIZER(g_events); static u_int g_pending_events; static TAILQ_HEAD(,g_provider) g_doorstep = TAILQ_HEAD_INITIALIZER(g_doorstep); static struct mtx g_eventlock; -static struct sx g_eventstall; #define G_N_EVENTREFS 20 @@ -84,23 +83,10 @@ g_waitidle(void) } void -g_stall_events(void) -{ - - sx_xlock(&g_eventstall); -} - -void -g_release_events(void) -{ - - sx_xunlock(&g_eventstall); -} - -void g_orphan_provider(struct g_provider *pp, int error) { + /* G_VALID_PROVIDER(pp) We likely lack topology lock */ g_trace(G_T_TOPOLOGY, "g_orphan_provider(%p(%s), %d)", pp, pp->name, error); KASSERT(error != 0, @@ -128,8 +114,9 @@ g_orphan_register(struct g_provider *pp) struct g_consumer *cp, *cp2; int wf; - g_trace(G_T_TOPOLOGY, "g_orphan_register(%s)", pp->name); g_topology_assert(); + G_VALID_PROVIDER(pp); + g_trace(G_T_TOPOLOGY, "g_orphan_register(%s)", pp->name); wf = pp->flags & G_PF_WITHER; pp->flags &= ~G_PF_WITHER; @@ -166,13 +153,14 @@ one_event(void) struct g_event *ep; struct g_provider *pp; - sx_xlock(&g_eventstall); g_topology_lock(); for (;;) { mtx_lock(&g_eventlock); pp = TAILQ_FIRST(&g_doorstep); - if (pp != NULL) + if (pp != NULL) { + G_VALID_PROVIDER(pp); TAILQ_REMOVE(&g_doorstep, pp, orphan); + } mtx_unlock(&g_eventlock); if (pp == NULL) break; @@ -183,7 +171,6 @@ one_event(void) if (ep == NULL) { mtx_unlock(&g_eventlock); g_topology_unlock(); - sx_xunlock(&g_eventstall); return (0); } TAILQ_REMOVE(&g_events, ep, events); @@ -201,7 +188,6 @@ one_event(void) if (g_pending_events == 0) wakeup(&g_pending_events); g_topology_unlock(); - sx_xunlock(&g_eventstall); return (1); } @@ -337,5 +323,4 @@ g_event_init() { mtx_init(&g_eventlock, "GEOM orphanage", NULL, MTX_DEF); - sx_init(&g_eventstall, "GEOM event stalling"); } diff --git a/sys/geom/geom_int.h b/sys/geom/geom_int.h index 952b6c6..1b94549 100644 --- a/sys/geom/geom_int.h +++ b/sys/geom/geom_int.h @@ -44,7 +44,7 @@ extern int g_debugflags; * 1 G_T_TOPOLOGY * 2 G_T_BIO * 4 G_T_ACCESS - * 8 Enable sanity checks + * 8 (unused) * 16 Allow footshooting on rank#1 providers * 32 G_T_DETAILS */ @@ -68,8 +68,6 @@ void g_conftxt(void *, int flag); /* geom_event.c */ void g_event_init(void); void g_run_events(void); -void g_stall_events(void); -void g_release_events(void); /* geom_subr.c */ extern struct class_list_head g_classes; diff --git a/sys/geom/geom_subr.c b/sys/geom/geom_subr.c index ea7691f..720a3d6 100644 --- a/sys/geom/geom_subr.c +++ b/sys/geom/geom_subr.c @@ -57,8 +57,6 @@ struct class_list_head g_classes = LIST_HEAD_INITIALIZER(g_classes); static struct g_tailq_head geoms = TAILQ_HEAD_INITIALIZER(geoms); char *g_wait_event, *g_wait_up, *g_wait_down, *g_wait_sim; -static int g_valid_obj(void const *ptr); - struct g_hh00 { struct g_class *mp; int error; @@ -125,6 +123,7 @@ g_unload_class(void *arg, int flag) g_topology_assert(); hh = arg; mp = hh->mp; + G_VALID_CLASS(mp); g_trace(G_T_TOPOLOGY, "g_unload_class(%s)", mp->name); /* @@ -216,7 +215,7 @@ g_new_geomf(struct g_class *mp, const char *fmt, ...) struct sbuf *sb; g_topology_assert(); - KASSERT(g_valid_obj(mp), ("g_new_geom_f() on alien class %p", mp)); + G_VALID_CLASS(mp); sb = sbuf_new(NULL, NULL, 0, SBUF_AUTOEXTEND); va_start(ap, fmt); sbuf_vprintf(sb, fmt, ap); @@ -239,8 +238,9 @@ void g_destroy_geom(struct g_geom *gp) { - g_trace(G_T_TOPOLOGY, "g_destroy_geom(%p(%s))", gp, gp->name); g_topology_assert(); + G_VALID_GEOM(gp); + g_trace(G_T_TOPOLOGY, "g_destroy_geom(%p(%s))", gp, gp->name); KASSERT(LIST_EMPTY(&gp->consumer), ("g_destroy_geom(%s) with consumer(s) [%p]", gp->name, LIST_FIRST(&gp->consumer))); @@ -255,7 +255,7 @@ g_destroy_geom(struct g_geom *gp) } /* - * This function is called (repeatedly) until has withered away. + * This function is called (repeatedly) until the has withered away. */ void g_wither_geom(struct g_geom *gp, int error) @@ -264,11 +264,12 @@ g_wither_geom(struct g_geom *gp, int error) struct g_consumer *cp, *cp2; static int once_is_enough; + g_topology_assert(); + G_VALID_GEOM(gp); if (once_is_enough) return; once_is_enough = 1; g_trace(G_T_TOPOLOGY, "g_wither_geom(%p(%s))", gp, gp->name); - g_topology_assert(); if (!(gp->flags & G_GEOM_WITHER)) { gp->flags |= G_GEOM_WITHER; LIST_FOREACH(pp, &gp->provider, provider) @@ -299,6 +300,7 @@ g_new_consumer(struct g_geom *gp) struct g_consumer *cp; g_topology_assert(); + G_VALID_GEOM(gp); KASSERT(!(gp->flags & G_GEOM_WITHER), ("g_new_consumer on WITHERing geom(%s) (class %s)", gp->name, gp->class->name)); @@ -319,8 +321,9 @@ g_destroy_consumer(struct g_consumer *cp) { struct g_geom *gp; - g_trace(G_T_TOPOLOGY, "g_destroy_consumer(%p)", cp); g_topology_assert(); + G_VALID_CONSUMER(cp); + g_trace(G_T_TOPOLOGY, "g_destroy_consumer(%p)", cp); KASSERT (cp->provider == NULL, ("g_destroy_consumer but attached")); KASSERT (cp->acr == 0, ("g_destroy_consumer with acr")); KASSERT (cp->acw == 0, ("g_destroy_consumer with acw")); @@ -348,6 +351,7 @@ g_new_provider_event(void *arg, int flag) if (g_shutdown) return; pp = arg; + G_VALID_PROVIDER(pp); LIST_FOREACH(mp, &g_classes, class) { if (mp->taste == NULL) continue; @@ -359,14 +363,6 @@ g_new_provider_event(void *arg, int flag) continue; mp->taste(mp, pp, 0); g_topology_assert(); - /* - * XXX: Bandaid for 5.2-RELEASE - * XXX: DO NOT REPLICATE THIS CODE! - */ - if (!g_valid_obj(pp)) { - printf("g_provider %p disappeared while tasting\n", pp); - return; - } } } @@ -379,6 +375,7 @@ g_new_providerf(struct g_geom *gp, const char *fmt, ...) va_list ap; g_topology_assert(); + G_VALID_GEOM(gp); KASSERT(gp->start != NULL, ("new provider on geom(%s) without ->start (class %s)", gp->name, gp->class->name)); @@ -408,6 +405,7 @@ void g_error_provider(struct g_provider *pp, int error) { + /* G_VALID_PROVIDER(pp); We may not have g_topology */ pp->error = error; } @@ -435,6 +433,7 @@ g_destroy_provider(struct g_provider *pp) struct g_geom *gp; g_topology_assert(); + G_VALID_PROVIDER(pp); KASSERT(LIST_EMPTY(&pp->consumers), ("g_destroy_provider but attached")); KASSERT (pp->acr == 0, ("g_destroy_provider with acr")); @@ -474,6 +473,7 @@ redo_rank(struct g_geom *gp) int n, m; g_topology_assert(); + G_VALID_GEOM(gp); /* Invalidate this geoms rank and move it to the tail */ gp1 = TAILQ_NEXT(gp, geoms); @@ -523,6 +523,8 @@ g_attach(struct g_consumer *cp, struct g_provider *pp) int error; g_topology_assert(); + G_VALID_CONSUMER(cp); + G_VALID_PROVIDER(pp); KASSERT(cp->provider == NULL, ("attach but attached")); cp->provider = pp; LIST_INSERT_HEAD(&pp->consumers, cp, consumers); @@ -540,9 +542,9 @@ g_detach(struct g_consumer *cp) { struct g_provider *pp; - g_trace(G_T_TOPOLOGY, "g_detach(%p)", cp); - KASSERT(cp != (void*)0xd0d0d0d0, ("ARGH!")); g_topology_assert(); + G_VALID_CONSUMER(cp); + g_trace(G_T_TOPOLOGY, "g_detach(%p)", cp); KASSERT(cp->provider != NULL, ("detach but not attached")); KASSERT(cp->acr == 0, ("detach but nonzero acr")); KASSERT(cp->acw == 0, ("detach but nonzero acw")); @@ -573,12 +575,14 @@ g_access(struct g_consumer *cp, int dcr, int dcw, int dce) int pr,pw,pe; int error; + g_topology_assert(); + G_VALID_CONSUMER(cp); pp = cp->provider; + G_VALID_PROVIDER(pp); g_trace(G_T_ACCESS, "g_access(%p(%s), %d, %d, %d)", cp, pp->name, dcr, dcw, dce); - g_topology_assert(); KASSERT(cp->provider != NULL, ("access but not attached")); KASSERT(cp->acr + dcr >= 0, ("access resulting in negative acr")); KASSERT(cp->acw + dcw >= 0, ("access resulting in negative acw")); @@ -688,10 +692,12 @@ g_handleattr(struct bio *bp, const char *attribute, void *val, int len) } int -g_std_access(struct g_provider *pp __unused, +g_std_access(struct g_provider *pp, int dr __unused, int dw __unused, int de __unused) { + g_topology_assert(); + G_VALID_PROVIDER(pp); return (0); } @@ -718,8 +724,9 @@ g_std_spoiled(struct g_consumer *cp) struct g_geom *gp; struct g_provider *pp; - g_trace(G_T_TOPOLOGY, "g_std_spoiled(%p)", cp); g_topology_assert(); + G_VALID_CONSUMER(cp); + g_trace(G_T_TOPOLOGY, "g_std_spoiled(%p)", cp); g_detach(cp); gp = cp->geom; LIST_FOREACH(pp, &gp->provider, provider) @@ -752,6 +759,7 @@ g_spoil_event(void *arg, int flag) if (flag == EV_CANCEL) return; pp = arg; + G_VALID_PROVIDER(pp); for (cp = LIST_FIRST(&pp->consumers); cp != NULL; cp = cp2) { cp2 = LIST_NEXT(cp, consumers); if (!cp->spoiled) @@ -770,6 +778,8 @@ g_spoil(struct g_provider *pp, struct g_consumer *cp) struct g_consumer *cp2; g_topology_assert(); + G_VALID_PROVIDER(pp); + G_VALID_CONSUMER(cp); LIST_FOREACH(cp2, &pp->consumers, consumers) { if (cp2 == cp) @@ -798,11 +808,16 @@ g_getattr__(const char *attr, struct g_consumer *cp, void *var, int len) return (0); } +#ifdef DIAGNOSTIC /* - * XXX: Bandaid for 5.2. - * XXX: DO NOT EVEN THINK ABOUT CALLING THIS FUNCTION! + * This function walks (topologically unsafely) the mesh and return a + * non-zero integer if it finds the argument pointer is an object. + * The return value indicates which type of object it is belived to be. + * If topology is not locked, this function is potentially dangerous, + * but since it is for debugging purposes and can be useful for instance + * from DDB, we do not assert topology lock is held. */ -static int +int g_valid_obj(void const *ptr) { struct g_class *mp; @@ -810,50 +825,20 @@ g_valid_obj(void const *ptr) struct g_consumer *cp; struct g_provider *pp; - g_topology_assert(); LIST_FOREACH(mp, &g_classes, class) { if (ptr == mp) return (1); LIST_FOREACH(gp, &mp->geom, geom) { if (ptr == gp) - return (1); + return (2); LIST_FOREACH(cp, &gp->consumer, consumer) if (ptr == cp) - return (1); + return (3); LIST_FOREACH(pp, &gp->provider, provider) if (ptr == pp) - return (1); + return (4); } } return(0); } - -/* - * Check if the given pointer is a live object - */ - -void -g_sanity(void const *ptr) -{ - struct g_class *mp; - struct g_geom *gp; - struct g_consumer *cp; - struct g_provider *pp; - - if (!(g_debugflags & 0x8)) - return; - LIST_FOREACH(mp, &g_classes, class) { - KASSERT(mp != ptr, ("Ptr is live class")); - LIST_FOREACH(gp, &mp->geom, geom) { - KASSERT(gp != ptr, ("Ptr is live geom")); - KASSERT(gp->name != ptr, ("Ptr is live geom's name")); - LIST_FOREACH(cp, &gp->consumer, consumer) { - KASSERT(cp != ptr, ("Ptr is live consumer")); - } - LIST_FOREACH(pp, &gp->provider, provider) { - KASSERT(pp != ptr, ("Ptr is live provider")); - } - } - } -} - +#endif -- cgit v1.1