Re: ffs_truncate3 panics

On Thu, Aug 09, 2018 at 08:38:50PM +0000, Rick Macklem wrote:
> >BTW, does NFS server use extended attributes ?  What for ?  Can you, please,
> >point out the code which does this ?
> For the pNFS service, there are two system namespace extended attributes for
> each file stored on the service.
> pnfsd.dsfile - Stores where the data for the file is. Can be displayed by the
>      pnfsdsfile(8) command.
> 
> pnfsd.dsattr - Cached attributes that change when a file is written (size, mtime,
> change) so that the MDS doesn't have to do a Getattr on the data server for every client Getattr.
> 

My reading of the nfsd code + ffs extattr handling reminds me that you
already reported this issue some time ago.  I suspected ufs_balloc() at
that time.

Now I think that the situation with the stray buffers hanging on the
queue is legitimate, ffs_extread() might create such buffer and release
it to a clean queue, then removal of the file would see inode with no
allocated ext blocks but with the buffer.

I think the easiest way to handle it is to always flush buffers and pages
in the ext attr range, regardless of the number of allocated ext blocks.
Patch below was not tested.
diff --git a/sys/ufs/ffs/ffs_inode.c b/sys/ufs/ffs/ffs_inode.c
index 3cf58558c18..2ffd861f3b4 100644
--- a/sys/ufs/ffs/ffs_inode.c
+++ b/sys/ufs/ffs/ffs_inode.c
_at__at_ -244,22 +244,19 _at__at_ ffs_truncate(vp, length, flags, cred)
 		extblocks = btodb(fragroundup(fs, ip->i_din2->di_extsize));
 		datablocks -= extblocks;
 	}
-	if ((flags & IO_EXT) && extblocks > 0) {
+	if ((flags & IO_EXT) != 0) {
 		if (length != 0)
 			panic("ffs_truncate: partial trunc of extdata");
 		if (softdeptrunc || journaltrunc) {
 			if ((flags & IO_NORMAL) == 0)
 				goto extclean;
 			needextclean = 1;
-		} else {
-			if ((error = ffs_syncvnode(vp, MNT_WAIT, 0)) != 0)
-				return (error);
+		} else if ((error = ffs_syncvnode(vp, MNT_WAIT, 0)) != 0)
+			return (error);
+		if (extblocks > 0) {
 #ifdef QUOTA
 			(void) chkdq(ip, -extblocks, NOCRED, 0);
 #endif
-			vinvalbuf(vp, V_ALT, 0, 0);
-			vn_pages_remove(vp,
-			    OFF_TO_IDX(lblktosize(fs, -extblocks)), 0);
 			osize = ip->i_din2->di_extsize;
 			ip->i_din2->di_blocks -= extblocks;
 			ip->i_din2->di_extsize = 0;
_at__at_ -278,6 +275,8 _at__at_ ffs_truncate(vp, length, flags, cred)
 				    vp->v_type, NULL, SINGLETON);
 			}
 		}
+		vinvalbuf(vp, V_ALT, 0, 0);
+		vn_pages_remove(vp, OFF_TO_IDX(lblktosize(fs, -UFS_NXADDR)), 0);
 	}
 	if ((flags & IO_NORMAL) == 0)
 		return (0);
_at__at_ -631,7 +630,10 _at__at_ ffs_truncate(vp, length, flags, cred)
 		softdep_journal_freeblocks(ip, cred, length, IO_EXT);
 	else
 		softdep_setup_freeblocks(ip, length, IO_EXT);
-	return (ffs_update(vp, waitforupdate));
+	error = ffs_update(vp, waitforupdate);
+	vinvalbuf(vp, V_ALT, 0, 0);
+	vn_pages_remove(vp, OFF_TO_IDX(lblktosize(fs, -UFS_NXADDR)), 0);
+	return (error);
 }
 
 /*