NetBSD-Bugs archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

bin/51425: fix count of B_BUSY in sbin/fsck_lfs/bufcache.c



>Number:         51425
>Category:       bin
>Synopsis:       fix count of B_BUSY in sbin/fsck_lfs/bufcache.c
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    bin-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Thu Aug 18 15:55:00 +0000 2016
>Originator:     Jose Luis Rodriguez Garcia
>Release:        Current and previous releases
>Organization:
	
>Environment:
	
	
>Description:
	There are severaul faults in bufcache.c handling the count (bp->b_vp->v_usecount) of busy buffers (B_BUSY) on a vnode:
	1- getblk only increases bp->b_vp->v_usecount whe there is a buffer free in the free list for the block in question. If the block is in the cache bp->b_vp->v_usecount isn't increased when getblk returns a B_BUSY buffer.
	2- There are serveral places of the code that bufdestroy is called immediatily after bremfree. It produces that v_usecount that grows indefinitely. It is usually harmless because usually the vnode is destroyed after of this operations. The exception is the function reload_ifile where a bremfree/buf_destroy is done on all buffers of the of vnode of the ifile and it isn't erased.

The v_usecount value seems it is only used in vget funtion as criteria for destroy vnodes:
                if (LIST_EMPTY(&vp->v_dirtyblkhd) &&
                    vp->v_usecount == 0 &&
                    !(vp->v_uflag & VU_DIROP))
                        tossvp = vp;
        }
        /* Don't let vnode list grow arbitrarily */
        if (nvnodes > VNODE_CACHE_SIZE && tossvp) {
                vnode_destroy(tossvp);
        }

	The fix I propose is only tested doing a build.sh. If somebody suggest some way of checking the correctness of the patch, I will try to check it.
	The fix is some ugly because the programs sometimes call directly to bremfree/bufdestroy and other times call to bread/brelse. The API seems inconsistent.

	It can be interesting to pull the patch to previous releases.
>How-To-Repeat:
	Code review
>Fix:
Index: bufcache.c
===================================================================
RCS file: /cvsroot/src/sbin/fsck_lfs/bufcache.c,v
retrieving revision 1.18
diff -u -r1.18 bufcache.c
--- bufcache.c	18 Aug 2016 08:08:02 -0000	1.18
+++ bufcache.c	18 Aug 2016 15:07:34 -0000
@@ -161,6 +161,7 @@
 		free(bp->b_data);
 	free(bp);
 	--nbufs;
+	--bp->b_vp->v_usecount;
 }
 
 /* Remove a buffer from its free list. */
@@ -265,6 +266,7 @@
 		if (bp) {
 			if (bp->b_flags & B_DELWRI)
 				VOP_STRATEGY(bp);
+			++bp->b_vp->v_usecount;
 			buf_destroy(bp);
 			break;
 		}
@@ -286,6 +288,7 @@
 	LIST_INSERT_HEAD(&bufhash[bp->b_hashval], bp, b_hash);
 	LIST_INSERT_HEAD(&vp->v_cleanblkhd, bp, b_vnbufs);
 	bp->b_flags = B_BUSY;
+	++bp->b_vp->v_usecount;
 
 	return bp;
 }



Home | Main Index | Thread Index | Old Index