Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 28 Jun 1999 10:06:10 -0700
From:      Kirk McKusick <mckusick@flamingo.McKusick.COM>
To:        Peter Wemm <peter@netplex.com.au>
Cc:        Julian Elischer <julian@whistle.com>, Matthew Dillon <dillon@apollo.backplane.com>, Alan Cox <alc@cs.rice.edu>, Mike Smith <mike@smith.net.au>, "John S. Dyson" <toor@dyson.iquest.net>, dg@root.com, dyson@iquest.net, current@freebsd.org, Greg Lehey <grog@lemis.com>
Subject:   Re: Found the startup panic - ccd ( patch included ) 
Message-ID:  <199906281706.KAA02893@flamingo.McKusick.COM>
In-Reply-To: Your message of "Tue, 29 Jun 1999 00:57:38 %2B0800." <19990628165738.E590283@overcee.netplex.com.au> 

next in thread | previous in thread | raw e-mail | index | archive | help
This looks correct to me. I originally had the BUF_KERNPROC macro
just changing the target buffer, but then changed it to follow
chains in the hopes of reducing the instances of its use. The
way that you have used it below is much clearer and should definitely
be put in. Also I hope that your fix to breadn will clear up Greg's
problem with NFS.

	Kirk

=-=-=-=-=-=-=-=

To: Kirk McKusick <mckusick@flamingo.McKusick.COM>
Cc: Julian Elischer <julian@whistle.com>,
    Matthew Dillon <dillon@apollo.backplane.com>,
    Alan Cox <alc@cs.rice.edu>, Mike Smith <mike@smith.net.au>,
    "John S. Dyson" <toor@dyson.iquest.net>, dg@root.com,
    dyson@iquest.net,
    current@freebsd.org, Greg Lehey <grog@lemis.com>
Subject: Re: Found the startup panic - ccd ( patch included ) 
In-reply-to: Your message of "Mon, 28 Jun 1999 04:54:07 MST."
	     <199906281154.EAA02335@flamingo.McKusick.COM> 
Date: Tue, 29 Jun 1999 00:57:38 +0800
From: Peter Wemm <peter@netplex.com.au>
Message-Id: <19990628165738.E590283@overcee.netplex.com.au>

Kirk McKusick wrote:
[..]

I've been tinkering around for a while and think I have at least a
partial fix for the remaining problems.

Certain places use B_CALL and have biodone() from the b_iodone routine,
so we can't reliably use B_ASYNC as an indicator of needing to reassign
to LK_KERNPROC.  We have to do it on a case-by-case basis.

It's easier to do cluster_head processing at the point it's gathered
rather than in BUF_KERNPROC().

vfs_cluster.c is confusing, but I think I've figured out how to get it
right. I'm not 100% sure about checking for B_CALL in both cases prior
to VOP_STRATEGY(), and maybe reqbp needs to be considered for the first
read in cluster_read().

Also, I think the inline BUF_*() routines/macros would be better as
routines in something like vfs_bio.c as the internals cause problems
with prototypes etc.

I think this patch fixes the remaining panics in pageouts and
clustering.

Index: kern/vfs_bio.c
===================================================================
RCS file: /home/ncvs/src/sys/kern/vfs_bio.c,v
retrieving revision 1.218
diff -u -r1.218 vfs_bio.c
--- vfs_bio.c	1999/06/28 15:32:10	1.218
+++ vfs_bio.c	1999/06/28 16:48:53
@@ -517,7 +517,8 @@
	if (curproc != NULL)
		curproc->p_stats->p_ru.ru_oublock++;
	splx(s);
-	BUF_KERNPROC(bp);
+	if (oldflags & B_ASYNC)
+		BUF_KERNPROC(bp);
	VOP_STRATEGY(bp->b_vp, bp);
 
	/*
Index: kern/vfs_cluster.c
===================================================================
RCS file: /home/ncvs/src/sys/kern/vfs_cluster.c,v
retrieving revision 1.84
diff -u -r1.84 vfs_cluster.c
--- vfs_cluster.c	1999/06/26 02:46:08	1.84
+++ vfs_cluster.c	1999/06/28 16:48:53
@@ -252,7 +252,8 @@
		if ((bp->b_flags & B_CLUSTER) == 0)
			vfs_busy_pages(bp, 0);
		bp->b_flags &= ~(B_ERROR|B_INVAL);
-		BUF_KERNPROC(bp);
+		if (bp->b_flags & (B_ASYNC|B_CALL))
+			BUF_KERNPROC(bp);
		error = VOP_STRATEGY(vp, bp);
		curproc->p_stats->p_ru.ru_inblock++;
	}
@@ -286,7 +287,8 @@
			if ((rbp->b_flags & B_CLUSTER) == 0)
				vfs_busy_pages(rbp, 0);
			rbp->b_flags &= ~(B_ERROR|B_INVAL);
-			BUF_KERNPROC(rbp);
+			if (rbp->b_flags & (B_ASYNC|B_CALL))
+				BUF_KERNPROC(rbp);
			(void) VOP_STRATEGY(vp, rbp);
			curproc->p_stats->p_ru.ru_inblock++;
		}
@@ -414,6 +416,11 @@
				break;
			}
		}
+		/*
+		 * XXX fbp from caller may not be B_ASYNC, but we are going
+		 * to biodone() it in cluster_callback() anyway
+		 */
+		BUF_KERNPROC(tbp);
		TAILQ_INSERT_TAIL(&bp->b_cluster.cluster_head,
			tbp, b_cluster.cluster_entry);
		for (j = 0; j < tbp->b_npages; j += 1) {
@@ -788,6 +795,7 @@
			reassignbuf(tbp, tbp->b_vp);	/* put on clean list */
			++tbp->b_vp->v_numoutput;
			splx(s);
+			BUF_KERNPROC(tbp);
			TAILQ_INSERT_TAIL(&bp->b_cluster.cluster_head,
				tbp, b_cluster.cluster_entry);
		}
Index: sys/buf.h
===================================================================
RCS file: /home/ncvs/src/sys/sys/buf.h,v
retrieving revision 1.73
diff -u -r1.73 buf.h
--- buf.h	1999/06/27 11:40:03	1.73
+++ buf.h	1999/06/28 16:48:53
@@ -315,17 +315,8 @@
 static __inline void
 BUF_KERNPROC(struct buf *bp)
 {
-	struct buf *nbp;
-	int s;
 
-	s = splbio();
-	if (bp->b_flags & B_ASYNC)
-		bp->b_lock.lk_lockholder = LK_KERNPROC;
-	for (nbp = TAILQ_FIRST(&bp->b_cluster.cluster_head);
-	     nbp; nbp = TAILQ_NEXT(&nbp->b_cluster, cluster_entry))
-		if (nbp->b_flags & B_ASYNC)
-			nbp->b_lock.lk_lockholder = LK_KERNPROC;
-	splx(s);
+	bp->b_lock.lk_lockholder = LK_KERNPROC;
 }
 /*
  * Find out the number of references to a lock.


Cheers,
-Peter
--
Peter Wemm - peter@FreeBSD.org; peter@yahoo-inc.com; peter@netplex.com.au



To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-current" in the body of the message




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?199906281706.KAA02893>