Subject: kern/11468: fsck_lfs complains about wrong lfs_avail
To: None <gnats-bugs@gnats.netbsd.org>
From: None <joff@gci-net.com>
List: netbsd-bugs
Date: 11/11/2000 14:30:10
>Number:         11468
>Category:       kern
>Synopsis:       fsck_lfs complains about wrong lfs_avail
>Confidential:   no
>Severity:       serious
>Priority:       high
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sat Nov 11 14:30:00 PST 2000
>Closed-Date:
>Last-Modified:
>Originator:     Jesse Off
>Release:        NetBSD 1.5BETA
>Organization:
>Environment:
NetBSD construct.home 1.5_BETA NetBSD 1.5_BETA (CONSTRUCT) #4: Sat Nov 11 13:20:30 MST 2000 joff@construct.home:/usr/src/sys/arch/i386/compile/CONSTRUCT i386
>Description:
LFS may not properly account fs->lfs_avail for fragment expansion
in lfs_fragextend() if the buffer passed has been locked and dirtied, but
not yet written.  This is usually seen when someone runs fsck_lfs
and it complains that avail is wrong.  This bug could also result in 
certain other nasty problems when "avail" runs low, though I did not 
test for these effects myself.

I've enclosed a patch to fix this problem.  
>How-To-Repeat:
Run:

dd if=/dev/zero of=out bs=1 count=16384

Then immediately unmount and run fsck_lfs.  Notice how fsck_lfs says 
avail is wrong.

Repeat the process if still skeptical.  mount,dd,unmount,fsck...  I was
able to consistantly reproduce the problem with this.
>Fix:
A simple fix to lfs_balloc.c in lfs_fragextend().  With this patch, we notice 
when we're passed a buffer with B_DELWRI or B_LOCKED and cancel the 
write and increment lfs_avail.   Later, when the caller VOP_BWRITE()'s the 
fragment extended buffer, the buffer will be rescheduled for writing and 
lfs_avail will be decremented by the size of the new buffer.  

We could just decrement lfs_avail by the difference in size between the
old and the new, but by letting a later VOP_BWRITE do it, we don't have
to include (because VOP_BWRITE already has) logic for making sure we have 
enough "avail" and potentially waking up the cleaner and sleeping if 
we dont.  I believe this patch does the right thing in this regard.

Index: lfs_balloc.c
===================================================================
RCS file: /cvsroot/syssrc/sys/ufs/lfs/lfs_balloc.c,v
retrieving revision 1.18.2.1
diff -u -r1.18.2.1 lfs_balloc.c
--- lfs_balloc.c        2000/09/14 18:50:17     1.18.2.1
+++ lfs_balloc.c        2000/11/11 22:20:47
@@ -347,6 +347,7 @@
        long bb;
        int error;
        extern long locked_queue_bytes;
+       extern int locked_queue_count;
        struct buf *ibp;
        SEGUSE *sup;
 
@@ -390,8 +391,20 @@
        fs->lfs_bfree -= bb;
        ip->i_lfs_effnblks += bb;
        ip->i_flag |= IN_CHANGE | IN_UPDATE;
-       if((*bpp)->b_flags & B_LOCKED)
-               locked_queue_bytes += (nsize - osize);
+
+       /*
+        * If this buffer has already been locked by a bwrite, but not
+        * yet written, we cancel the write.  
+        */
+       if((*bpp)->b_flags & B_LOCKED) {
+               --locked_queue_count;
+               locked_queue_bytes -= (*bpp)->b_bufsize;
+               (*bpp)->b_flags &= ~B_LOCKED;
+       }
+       if((*bpp)->b_flags & B_DELWRI) {
+               (*bpp)->b_flags &= ~B_DELWRI;
+               fs->lfs_avail += btodb((*bpp)->b_bcount);
+       }
        allocbuf(*bpp, nsize);
        bzero((char *)((*bpp)->b_data) + osize, (u_int)(nsize - osize));
 

>Release-Note:
>Audit-Trail:
>Unformatted: