NetBSD Problem Report #51425
From jose@pc2.bigmail.com Thu Aug 18 15:53:54 2016
Return-Path: <jose@pc2.bigmail.com>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
(Client CN "mail.netbsd.org", Issuer "Postmaster NetBSD.org" (verified OK))
by mollari.NetBSD.org (Postfix) with ESMTPS id 09F967A218
for <gnats-bugs@gnats.NetBSD.org>; Thu, 18 Aug 2016 15:53:54 +0000 (UTC)
Message-Id: <20160818155351.3A66123EBE41@pc2.bigmail.com>
Date: Thu, 18 Aug 2016 17:53:51 +0200 (CEST)
From: joseyluis@gmail.com
Reply-To: joseyluis@gmail.com
To: gnats-bugs@NetBSD.org
Subject: fix count of B_BUSY in sbin/fsck_lfs/bufcache.c
X-Send-Pr-Version: 3.95
>Number: 51425
>Category: bin
>Synopsis: fix count of B_BUSY in sbin/fsck_lfs/bufcache.c
>Confidential: no
>Severity: serious
>Priority: medium
>Responsible: dholland
>State: open
>Class: sw-bug
>Submitter-Id: net
>Arrival-Date: Thu Aug 18 15:55:00 +0000 2016
>Last-Modified: Sat Dec 03 11:17:55 +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;
}
>Release-Note:
>Audit-Trail:
Responsible-Changed-From-To: bin-bug-people->dholland
Responsible-Changed-By: maya@NetBSD.org
Responsible-Changed-When: Sat, 03 Dec 2016 11:17:55 +0000
Responsible-Changed-Why:
lfs
also, it is possible to exercise filesystem code somewhat by running tests(7). you might want to look at misc/py-anita too - easier than testing a change on your own machine.
>Unformatted:
(Contact us)
$NetBSD: query-full-pr,v 1.39 2013/11/01 18:47:49 spz Exp $
$NetBSD: gnats_config.sh,v 1.8 2006/05/07 09:23:38 tsutsui Exp $
Copyright © 1994-2014
The NetBSD Foundation, Inc. ALL RIGHTS RESERVED.