Re: Buf cache, swap spl, and NFS patches need review.

2000-03-22 Thread Paul Saab

Matthew Dillon ([EMAIL PROTECTED]) wrote:
 swap-SPL:
 
   Required splvm()'s had to be added to two places in the swap subsystem
   (but I don't think the lack of these spl's is the cause of the
   recently reported crashes, though it is possible).

There are a few more that I still get if I dont add spl protection.
I need to commit the SPLASSERT stuff but I'm scared about how badly it
will break for people that run with INVARIANTS.

I'll attach the whole SPLASSERT to the VM here.

Also, I went through vfs_bio.c and added SPLASSERT there and it
goes kaboom really quick.  bdwrite calls everything with no spl
proctection when it really should.  My full SPLASSERT patch is at
http://people.freebsd.org/~ps/splassert.diff.  I should just go ahead and
submit this.  This patch doesn't have any of the vm patches we discussed
last month.

-- 
Paul Saab
Technical Yahoo
[EMAIL PROTECTED] - [EMAIL PROTECTED] - [EMAIL PROTECTED]
Do You .. uhh .. Yahoo!?


Index: src/sys/vm/swap_pager.c
diff -u src/sys/vm/swap_pager.c:1.1.1.1 src/sys/vm/swap_pager.c:1.6
--- src/sys/vm/swap_pager.c:1.1.1.1 Tue Feb 29 02:15:04 2000
+++ src/sys/vm/swap_pager.c Thu Mar 16 02:32:32 2000
@@ -65,6 +65,7 @@
  * @(#)swap_pager.c8.9 (Berkeley) 3/21/94
  *
  * $FreeBSD: src/sys/vm/swap_pager.c,v 1.130 1999/12/28 07:30:54 peter Exp $
+ * $PMS: src/sys/vm/swap_pager.c,v 1.6 2000/03/16 10:32:32 ps Exp $
  */
 
 #include sys/param.h
@@ -214,6 +215,8 @@
 static __inline void
 swp_sizecheck()
 {
+   SPLASSERT(vm, "swp_sizecheck");
+
if (vm_swap_size  nswap_lowat) {
if (swap_pager_almost_full == 0) {
printf("swap_pager: out of swap space\n");
@@ -462,6 +465,8 @@
int npages;
 {
daddr_t blk;
+   
+   SPLASSERT(vm, "swp_pager_getswapspace");
 
if ((blk = blist_alloc(swapblist, npages)) == SWAPBLK_NONE) {
if (swap_pager_full != 2) {
@@ -496,6 +501,8 @@
daddr_t blk;
int npages;
 {
+   SPLASSERT(vm, "swp_pager_freeswapspace");
+
blist_free(swapblist, blk, npages);
vm_swap_size += npages;
swp_sizecheck();
@@ -702,8 +709,6 @@
  * distance.  We do not try to restrict it to the swap device stripe
  * (that is handled in getpages/putpages).  It probably isn't worth
  * doing here.
- *
- * This routine must be called at splvm().
  */
 
 boolean_t
@@ -714,14 +719,17 @@
int *after;
 {
daddr_t blk0;
+   int s;
 
/*
 * do we have good backing store at the requested index ?
 */
 
+   s = splvm();
blk0 = swp_pager_meta_ctl(object, pindex, 0);
 
if (blk0 == SWAPBLK_NONE) {
+   splx(s);
if (before)
*before = 0;
if (after)
@@ -764,7 +772,7 @@
}
*after = (i - 1);
}
-
+   splx(s);
return (TRUE);
 }
 
@@ -784,14 +792,17 @@
  * depends on it.
  *
  * This routine may not block
- * This routine must be called at splvm()
  */
 
 static void
 swap_pager_unswapped(m)
vm_page_t m;
 {
+   int s;
+
+   s = splvm();
swp_pager_meta_ctl(m-object, m-pindex, SWM_FREE);
+   splx(s);
 }
 
 /*
@@ -1230,8 +1241,13 @@
 * force sync if not pageout process
 */
 
-   if (object-type != OBJT_SWAP)
+   if (object-type != OBJT_SWAP) {
+   int s;
+   
+   s = splvm();
swp_pager_meta_build(object, 0, SWAPBLK_NONE);
+   splx(s);
+   }
 
if (curproc != pageproc)
sync = TRUE;
@@ -1682,6 +1698,8 @@
struct swblock **pswap;
struct swblock *swap;
 
+   SPLASSERT(vm, "swp_pager_hash");
+
index = ~SWAP_META_MASK;
pswap = swhash[(index ^ (int)(intptr_t)object)  swhash_mask];
 
@@ -1720,6 +1738,8 @@
struct swblock *swap;
struct swblock **pswap;
 
+   CONDSPLASSERT(object-type == OBJT_SWAP, vm, "swp_pager_meta_build");
+
/*
 * Convert default object to swap object if necessary
 */
@@ -1810,6 +1830,8 @@
 static void
 swp_pager_meta_free(vm_object_t object, vm_pindex_t index, daddr_t count)
 {
+   SPLASSERT(vm, "swp_pager_meta_free");
+
if (object-type != OBJT_SWAP)
return;
 
@@ -1856,6 +1878,8 @@
 {
daddr_t index = 0;
 
+   SPLASSERT(vm, "swp_pager_meta_free_all");
+
if (object-type != OBJT_SWAP)
return;
 
@@ -1924,6 +1948,8 @@
struct swblock **pswap;
struct swblock *swap;
daddr_t r1;
+
+   SPLASSERT(vm, "swp_pager_meta_ctl");
 
/*
 * The meta data only exists of the object is OBJT_SWAP 
Index: src/sys/vm/vm_page.c
diff -u src/sys/vm/vm_page.c:1.1.1.1 src/sys/vm/vm_page.c:1.2
--- src/sys/vm/vm_page.c:1.1.1.1Tue Feb 29 02:15:04 2000
+++ src/sys/vm/vm_page.cTue Feb 29 02:28:01 2000
@@ -35,6 

Re: Buf cache, swap spl, and NFS patches need review.

2000-03-22 Thread Matthew Dillon


:Content-Type: text/plain; charset=us-ascii
:
:Matthew Dillon ([EMAIL PROTECTED]) wrote:
: swap-SPL:
: 
:  Required splvm()'s had to be added to two places in the swap subsystem
:  (but I don't think the lack of these spl's is the cause of the
:  recently reported crashes, though it is possible).
:
:There are a few more that I still get if I dont add spl protection.
:I need to commit the SPLASSERT stuff but I'm scared about how badly it
:will break for people that run with INVARIANTS.
:
:I'll attach the whole SPLASSERT to the VM here.
:
:Also, I went through vfs_bio.c and added SPLASSERT there and it
:goes kaboom really quick.  bdwrite calls everything with no spl
:proctection when it really should.  My full SPLASSERT patch is at
:http://people.freebsd.org/~ps/splassert.diff.  I should just go ahead and
:submit this.  This patch doesn't have any of the vm patches we discussed
:last month.

Lets do this one step at a time.  Let me get in my (relatively minor)
spl fixes, then I'll take a look at your vfs_bio.c stuff -- there's got
to be some cockpit trouble there because for the most part vfs_bio.c
does *NOT* need to run at splbio().  Only the buffer queue adds and drops
and hash table ops need to run at splbio().  Once you have a buffer in
hand and locked you own it.

-Matt




To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message