From 45623f31bcbb7d26e3d7e039d7db4da9d11188fa Mon Sep 17 00:00:00 2001 From: Kirk McKusick Date: Tue, 22 Jun 1999 01:39:53 +0000 Subject: [PATCH] When allocating new buffers in getnewbuf, there are several points at which we may sleep. So, after completing our buffer allocation we must ensure that another process has not come along and allocated a different buffer with the same identity. We do this by keeping a global counter of the number of buffers that getnewbuf has allocated. We save this count when we enter getnewbuf and check it when we are about to return. If it has changed, then other buffers were allocated while we were in getnewbuf, so we must return NULL to let our parent know that it must recheck to see if it still needs the new buffer. Hopefully this fix will eliminate the creation of duplicate buffers with the same identity and the obscure corruptions that they cause. --- sys/kern/vfs_bio.c | 51 ++++++++++++++++++++++++++++++++++++---------- 1 file changed, 40 insertions(+), 11 deletions(-) diff --git a/sys/kern/vfs_bio.c b/sys/kern/vfs_bio.c index 43acafb905d4..4a7c2e420dd1 100644 --- a/sys/kern/vfs_bio.c +++ b/sys/kern/vfs_bio.c @@ -11,7 +11,7 @@ * 2. Absolutely no warranty of function or purpose is made by the author * John S. Dyson. * - * $Id: vfs_bio.c,v 1.213 1999/06/16 03:19:04 tegge Exp $ + * $Id: vfs_bio.c,v 1.214 1999/06/16 23:27:31 mckusick Exp $ */ /* @@ -1164,8 +1164,7 @@ vfs_bio_awrite(struct buf * bp) * getnewbuf: * * Find and initialize a new buffer header, freeing up existing buffers - * in the bufqueues as necessary. The new buffer is returned with - * flags set to B_BUSY. + * in the bufqueues as necessary. The new buffer is returned locked. * * Important: B_INVAL is not set. If the caller wishes to throw the * buffer away, the caller must set B_INVAL prior to calling brelse(). @@ -1191,6 +1190,8 @@ getnewbuf(struct vnode *vp, daddr_t blkno, int outofspace; int nqindex; int defrag = 0; + static int newbufcnt = 0; + int lastnewbuf = newbufcnt; restart: /* @@ -1230,15 +1231,13 @@ getnewbuf(struct vnode *vp, daddr_t blkno, */ if ((curproc->p_flag & P_FLSINPROG) == 0 && - numfreebuffers < lofreebuffers - ) { + numfreebuffers < lofreebuffers) { nqindex = QUEUE_LRU; nbp = NULL; } else { nqindex = QUEUE_EMPTY; if (outofspace || - (nbp = TAILQ_FIRST(&bufqueues[QUEUE_EMPTY])) == NULL - ) { + (nbp = TAILQ_FIRST(&bufqueues[QUEUE_EMPTY])) == NULL) { nqindex = QUEUE_AGE; nbp = TAILQ_FIRST(&bufqueues[QUEUE_AGE]); if (nbp == NULL) { @@ -1342,8 +1341,7 @@ getnewbuf(struct vnode *vp, daddr_t blkno, * getnewbuf() calls. */ if ((curproc->p_flag & P_FLSINPROG) || - numdirtybuffers < hidirtybuffers - ) { + numdirtybuffers < hidirtybuffers) { if (qindex == QUEUE_LRU) { /* * dbp prevents us from looping forever @@ -1526,9 +1524,40 @@ getnewbuf(struct vnode *vp, daddr_t blkno, } /* - * The bp, if valid, is set to B_BUSY. + * If we have slept at some point in this process and another + * process has managed to allocate a new buffer while we slept, + * we have to return NULL so that our caller can recheck to + * ensure that the other process did not create an identically + * identified buffer to the one we were requesting. We make this + * check by incrementing the static int newbufcnt each time we + * successfully allocate a new buffer. By saving the value of + * newbufcnt in our local lastnewbuf, we can compare newbufcnt + * with lastnewbuf to see if any other process managed to + * allocate a buffer while we were doing so ourselves. + * + * Note that bp, if valid, is locked. */ - return (bp); + if (lastnewbuf == newbufcnt) { + /* + * No buffers allocated, so we can return one if we were + * successful, or continue trying if we were not successful. + */ + if (bp != NULL) { + newbufcnt += 1; + return (bp); + } + goto restart; + } + /* + * Another process allocated a buffer since we were called, so + * we have to free the one we allocated and return NULL to let + * our caller recheck to see if a new buffer is still needed. + */ + if (bp != NULL) { + bp->b_flags |= B_INVAL; + brelse(bp); + } + return (NULL); } /*