kern/52127: possible LFS locking fix

[coypu%sdf.org@localhost]

>Number:         52127
>Category:       kern
>Synopsis:       possible LFS locking fix
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Fri Mar 31 19:20:00 +0000 2017
>Originator:     coypu
>Release:        NetBSD 7.99.67
>Organization:
>Environment:
NetBSD loggy 7.99.67 NetBSD 7.99.67 (GENERIC) #6: Fri Mar 31 21:44:18 IDT 2017  fly@loggy:/home/fly/obj/sys/arch/amd64/compile/GENERIC amd64

>Description:
Running firefox I keep hitting this assert (especially when deleting all history, but not just).

System panicked: kernel diagnostic assertion "(oip->i_size != 0 || lfs_dino_getblocks(fs, oip->i_din) == 0)" failed: file "/usr/src/sys/ufs/lfs/lfs_inode.c", line 595 truncate to 0 but 31 blks/31 effblks

should make a test of it, but couldn't reproduce it in other means.
I do know it uses an sqlite database for history.

Merely keeping lfs_lock for the assert is sufficient, but I think we need to hold onto it whenever calling lfs_dino_{set,get}blocks.

after applying the diff, there is an added getblocks not protected by lfs_lock in the start of the same function lfs_truncate, in code meant to deal with v1 inode format (fs->maxsymlinklen=0)
>How-To-Repeat:

>Fix:
Hold onto lfs_lock when calling lfs_dino_{set,get}blocks
in lfs_truncate.

(presumably) holding lfs_lock prevents us from grabbing seglock*
on entry to other code which calls setblocks, such as
lfs_update_single.

Now the assert that we truncated to 0 but the effnblks != 0
stops firing, and I can run firefox normally again.

* Not sure if this part is correct

Index: lfs_inode.c
===================================================================
RCS file: /cvsroot/src/sys/ufs/lfs/lfs_inode.c,v
retrieving revision 1.153
diff -u -r1.153 lfs_inode.c
--- lfs_inode.c	21 Mar 2017 09:53:00 -0000	1.153
+++ lfs_inode.c	31 Mar 2017 18:54:37 -0000
@@ -573,11 +573,11 @@
 	oip->i_size = length;
 	lfs_dino_setsize(fs, oip->i_din, oip->i_size);
 	oip->i_lfs_effnblks -= blocksreleased;
+
+	mutex_enter(&lfs_lock);
 	lfs_dino_setblocks(fs, oip->i_din,
 	    lfs_dino_getblocks(fs, oip->i_din) - real_released);
-	mutex_enter(&lfs_lock);
 	lfs_sb_addbfree(fs, blocksreleased);
-	mutex_exit(&lfs_lock);
 
 	KASSERTMSG((oip->i_size != 0 ||
 		lfs_dino_getblocks(fs, oip->i_din) == 0),
@@ -592,7 +592,6 @@
 	/*
 	 * If we truncated to zero, take us off the paging queue.
 	 */
-	mutex_enter(&lfs_lock);
 	if (oip->i_size == 0 && oip->i_flags & IN_PAGING) {
 		oip->i_flags &= ~IN_PAGING;
 		TAILQ_REMOVE(&fs->lfs_pchainhd, oip, i_lfs_pchain);