Re: panic while reading ntfs partition

On Thu, Jul 24, 2003 at 02:14:20PM +0200, Karel J. Bosschaart wrote:

> Not sure if this is useful, but I'm getting a perfectly reproducible
> panic when doing 'grep -R foo .' (as normal user) in a read-only
> mounted ntfs partition on a -current as of ~3 weeks ago:
> 
[...]
>   Panicstring: bundirty: buffer 0xc72d9868 still on queue 2
[...]

Thanks for the report - I thought I'd already fixed this problem. Would you
mind trying the attached patch? It seems to fix the problem for me, but
grep uses an awful lot of memory in the process (perhaps it's trying to
read a "line" from the pagefile containing mostly zeroes.)

See these two for more info on the bug:
http://perforce.freebsd.org/chv.cgi?CH=34967
http://perforce.freebsd.org/chv.cgi?CH=33434


diff -ru sys/fs/ntfs.old/ntfs_subr.c sys/fs/ntfs/ntfs_subr.c
--- sys/fs/ntfs.old/ntfs_subr.c	Fri Jul 25 12:26:18 2003
+++ sys/fs/ntfs/ntfs_subr.c	Fri Jul 25 12:18:09 2003
_at__at_ -1442,9 +1442,16 _at__at_
 		off = ntfs_btocnoff(off);
 
 		while (left && ccl) {
-			tocopy = min(left,
-				  min(ntfs_cntob(ccl) - off, MAXBSIZE - off));
+			/*
+			 * Always read and write single clusters at a time -
+			 * we need to avoid requesting differently-sized
+			 * blocks at the same disk offsets to avoid
+			 * confusing the buffer cache.
+			 */
+			tocopy = min(left, ntfs_cntob(1) - off);
 			cl = ntfs_btocl(tocopy + off);
+			KASSERT(cl == 1 && tocopy <= ntfs_cntob(1),
+			    ("single cluster limit mistake"));
 			ddprintf(("ntfs_writentvattr_plain: write: " \
 				"cn: 0x%x cl: %d, off: %d len: %d, left: %d\n",
 				(u_int32_t) cn, (u_int32_t) cl, 
_at__at_ -1540,23 +1547,19 _at__at_
 				off = ntfs_btocnoff(off);
 
 				while (left && ccl) {
-					tocopy = min(left,
-						  min(ntfs_cntob(ccl) - off,
-						      MAXBSIZE - off));
-					cl = ntfs_btocl(tocopy + off);
-
 					/*
-					 * If 'off' pushes us to next
-					 * block, don't attempt to read whole
-					 * 'tocopy' at once. This is to avoid
-					 * bread() with varying 'size' for
-					 * same 'blkno', which is not good.
+					 * Always read single clusters at a
+					 * time - we need to avoid reading
+					 * differently-sized blocks at the
+					 * same disk offsets to avoid
+					 * confusing the buffer cache.
 					 */
-					if (cl > ntfs_btocl(tocopy)) {
-						tocopy -=
-						    ntfs_btocnoff(tocopy + off);
-						cl--;
-					}
+					tocopy = min(left,
+					    ntfs_cntob(1) - off);
+					cl = ntfs_btocl(tocopy + off);
+					KASSERT(cl == 1 &&
+					    tocopy <= ntfs_cntob(1),
+					    ("single cluster limit mistake"));
 
 					ddprintf(("ntfs_readntvattr_plain: " \
 						"read: cn: 0x%x cl: %d, " \
diff -ru sys/fs/ntfs.old/ntfs_vfsops.c sys/fs/ntfs/ntfs_vfsops.c
--- sys/fs/ntfs.old/ntfs_vfsops.c	Fri Jul 25 12:26:18 2003
+++ sys/fs/ntfs/ntfs_vfsops.c	Fri Jul 25 12:18:09 2003
_at__at_ -311,6 +311,14 _at__at_
 		goto out;
 	ntmp = malloc( sizeof *ntmp, M_NTFSMNT, M_WAITOK | M_ZERO);
 	bcopy( bp->b_data, &ntmp->ntm_bootfile, sizeof(struct bootfile) );
+	/*
+	 * We must not cache the boot block if its size is not exactly
+	 * one cluster in order to avoid confusing the buffer cache when
+	 * the boot file is read later by ntfs_readntvattr_plain(), which
+	 * reads a cluster at a time.
+	 */
+	if (ntfs_cntob(1) != BBSIZE)
+		bp->b_flags |= B_NOCACHE;
 	brelse( bp );
 	bp = NULL;