I've certainly done the same with this.. congratulations - it panics. it's not helpful - the issue is not when it goes negative. the issue is missed increments in one of the many nfs cases, and a kassert in this case doesn't help you find that. We've fixed it several times and then something gets shuffled in nfs and the bug comes back. (I think I've got guenther's last call graph of NFS he made that helped me find it the last time).
I prefer to fix this in a later diff by disentangling this shit from nfs and doing it in bufq - as I mentioned to you privately. On Tue, Mar 19, 2013 at 9:17 AM, Ted Unangst <[email protected]> wrote: > Maybe we want, maybe we don't. Here's a check to prevent a counter > from going negative, which usually means an accounting mistake > somewhere. On the shortcomiing side, it only catches double > decrements, not missing decrements, so maybe it's not worthwhile. > Anyway, it was like five minutes to make, figured I'd show it. > > Index: lib/libkern/libkern.h > =================================================================== > RCS file: /cvs/src/sys/lib/libkern/libkern.h,v > retrieving revision 1.29 > diff -u -p -r1.29 libkern.h > --- lib/libkern/libkern.h 26 Apr 2012 01:22:31 -0000 1.29 > +++ lib/libkern/libkern.h 19 Mar 2013 15:10:41 -0000 > @@ -116,14 +116,11 @@ abs(int j) > > #ifndef DIAGNOSTIC > #define KASSERT(e) ((void)0) > +#define DECREMENT(x) x-- > #else > -#ifdef __STDC__ > #define KASSERT(e) ((e) ? (void)0 : > \ > __assert("diagnostic ", __FILE__, __LINE__, #e)) > -#else > -#define KASSERT(e) ((e) ? (void)0 : > \ > - __assert("diagnostic ", __FILE__, __LINE__, "e")) > -#endif > +#define DECREMENT(x) decrement_checked((x)--, __FILE__, __LINE__, #x) > #endif > > #ifndef DEBUG > @@ -141,6 +138,7 @@ abs(int j) > /* Prototypes for non-quad routines. */ > void __assert(const char *, const char *, int, const char *) > __attribute__ ((__noreturn__)); > +int64_t decrement_checked(int64_t, const char *, int, const char *); > int bcmp(const void *, const void *, size_t); > void bzero(void *, size_t); > void explicit_bzero(void *, size_t); > Index: kern/subr_prf.c > =================================================================== > RCS file: /cvs/src/sys/kern/subr_prf.c,v > retrieving revision 1.76 > diff -u -p -r1.76 subr_prf.c > --- kern/subr_prf.c 3 Apr 2011 16:46:19 -0000 1.76 > +++ kern/subr_prf.c 19 Mar 2013 15:13:27 -0000 > @@ -161,6 +161,14 @@ __assert(const char *t, const char *f, i > t, e, f, l); > } > > +int64_t > +decrement_checked(int64_t x, const char *f, int l, const char *e) > +{ > + if (x <= 0) > + __assert("decrement ", f, l, e); > + return x; > +} > + > /* > * tablefull: warn that a system table is full > */ > Index: kern/vfs_bio.c > =================================================================== > RCS file: /cvs/src/sys/kern/vfs_bio.c,v > retrieving revision 1.146 > diff -u -p -r1.146 vfs_bio.c > --- kern/vfs_bio.c 17 Feb 2013 17:39:29 -0000 1.146 > +++ kern/vfs_bio.c 19 Mar 2013 15:12:12 -0000 > @@ -129,7 +129,7 @@ bremfree(struct buf *bp) > bcstats.numcleanpages -= atop(bp->b_bufsize); > } else { > bcstats.numdirtypages -= atop(bp->b_bufsize); > - bcstats.delwribufs--; > + DECREMENT(bcstats.delwribufs); > } > TAILQ_REMOVE(dp, bp, b_freelist); > } > @@ -155,7 +155,7 @@ buf_put(struct buf *bp) > #endif > > LIST_REMOVE(bp, b_list); > - bcstats.numbufs--; > + DECREMENT(bcstats.numbufs); > > if (buf_dealloc_mem(bp) != 0) > return; > @@ -1221,9 +1221,9 @@ biodone(struct buf *bp) > if (bcstats.numbufs && > (!(ISSET(bp->b_flags, B_RAW) || ISSET(bp->b_flags, B_PHYS)))) { > if (!ISSET(bp->b_flags, B_READ)) > - bcstats.pendingwrites--; > + DECREMENT(bcstats.pendingwrites); > else > - bcstats.pendingreads--; > + DECREMENT(bcstats.pendingreads); > } > if (ISSET(bp->b_flags, B_CALL)) { /* if necessary, call out */ > CLR(bp->b_flags, B_CALL); /* but note callout done */ >
