Re: init scripts problems ?

"Also sprach Peter T. Breuer:"
> I think that there may be a "close deadlock" a bit like this ...
> 
>     client slave times out and dies, calling _exit()
> 
>     The parent does NOT receive a signal, and  the kernel release() 
>     function is not called.  Instead, the notionally dead client slave
>     is stuck trying to take a semaphore somewhere in kernel.

I have a patch that seems to help with this "client won't die easily"
problem (the best way to produce it to start the same client TWICE, with
no kill between - the second should starve the first to death in 90s but
the first won't die, which leaves the second stuck - cure with echo 0 to
nbdinfo).

I'll add it here.  I think only the first hunk of the patch is required,
but I haven't had time to check that hypothesis.

Please test.

If this is right, then setting "show_errs=1" would also have cured
things.

It cures a 2.6 kernel deadlock that I don't understand.  It makes the
ioctl that a client sends just before dying preemptively reset the
device, invalidating existing buffers and cache.  It _appears_ that if
the death is allowed to happen without doing this, then the kernel
notices that the last opener has died (or something) and does something
that deadlocks before calling the driver's release function. Maybe
trying to check that the device still exists by reading something?

You'd have thought I could have fixed that by playing with the openers
count, but I didn't succeed first try, whereas this "preemptive
invalidate" hack seems to work.

Of course, it's not _right_ in that you can probably still race it
by writing or reading hard to the device while it is dying. And
it may do some harm by briefly switching the device off and on, which
may be noticed by an overlying layer. But I'd prefer to meet that
problem after checking that this helps.

I have an additional patch that may also help, either by mitigating
the excessive effects of this one, or by curing something else.


--- enbd_base.c.oboe	Sun Dec  4 21:27:35 2005
+++ enbd_base.c	Sun Dec  4 21:20:05 2005
@@ -2640,11 +2640,13 @@
 
 		if (lo->aslot <= 0) {
 			// PTB we were the last client alive, diasable device
+                       do_reset = 1; // PTB do it, show_errs or not!
 			if (atomic_read (&lo->flags) & ENBD_SHOW_ERRS) {
 				// PTB soft_reset will invalidate_buffers
 			        atomic_clear_mask (ENBD_ENABLED, &lo->flags);
-                                do_reset = 1;
-			}
+			} else {
+                                do_enable = 1; // PTB case no show_errs
+                       }
 		} else if (!(atomic_read (&lo->flags) & ENBD_ENABLED)) {
 			// PTB must not call reenable as that clears the queue
                         do_enable = 1;
@@ -3203,14 +3205,13 @@
 
 
 /*
- * PTB - reset the enabled flag on a device and then clear all queues
- * ( call without the spinlock held )  and then enable again.
+ * PTB - reset the enabled flag on a device.
  */
 static void
 enbd_reenable (struct enbd_device *lo)
 {
 
-	int m, n;
+	//int m, n;
 
 	if (!(atomic_read (&lo->flags) & ENBD_INITIALISED))
 		return;
@@ -3219,16 +3220,15 @@
         if ((atomic_read (&lo->flags) & ENBD_ENABLED))
                 return;
 
-	m = enbd_clr_queue (lo);
+	//m = enbd_clr_queue (lo);
 	// PTB - have to call clr_kernel_queue without the io_spinlock held
-	n = enbd_clr_kernel_queue (lo);
+	//n = enbd_clr_kernel_queue (lo);
 
         enbd_enable(lo);
 }
 
 /*
- *  This function launches a thread which wakes for a signal to reenable
- *  the device, and then sets the timer to deleiver the signal.
+ *  This function sets the timer to reenable the device.
  */
 static int
 enbd_reenable_delay (struct enbd_device *lo, int delay)
@@ -3252,7 +3252,9 @@
 enbd_soft_reset (struct enbd_device *lo)
 {
 	int j;
+       int do_flush_requests_after_invalidate = 1;
         const int max_clrq_retries = 100;
+
 	if (!(atomic_read (&lo->flags) & ENBD_INITIALISED) || lo->nslot <= 0) {
 		return -EINVAL;
 	}
@@ -3278,17 +3283,35 @@
         * PTB put back invalidate buffers for use when called from
 	 * clr_sock from enbd_release on request of Rogier Wolff. FIXME.
         */
-        ENBD_ALERT("INVALIDATE DEVICE nd%s\n", lo->devnam);
+        if (1) {
+                struct inode *inode = lo->inode;
+                struct block_device *bdev = inode ? inode->i_bdev : NULL;
+                struct inode *inodetoo = bdev ? bdev->bd_inode : NULL;
+
+                ENBD_ALERT("INVALIDATE DEVICE nd%s BEGIN\n", lo->devnam);
+                if (!inodetoo || inode != inodetoo) {
+                        ENBD_ALERT(
+                            "invalidate would crash on device %p with inode %p\n",
+                            bdev, inodetoo);
+                        ENBD_ALERT( "aborting invalidate\n");
+                        do_flush_requests_after_invalidate = 0;
+                } else {
+
 #if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,12)
         __invalidate_device(lo->inode->i_bdev, 0);
 #else
         __invalidate_device(lo->inode->i_bdev);
 #endif /* LINUX_VERSION_CODE < KERNEL_VERSION(2,6,12) */
+                        ENBD_ALERT("INVALIDATE DEVICE nd%s DONE\n", lo->devnam);
+                }
+        }
+
+        if (do_flush_requests_after_invalidate) {
 
-        if (1) {
                 struct inode *inode = lo->inode;
                 struct block_device *bdev = inode ? inode->i_bdev : NULL;
                 request_queue_t * q; 
+
                 if (!bdev)
                         return 0;
                 q = bdev_get_queue (bdev); 
@@ -3759,6 +3782,17 @@
 	return -1;
 }
 
+static void
+print_slot_pids (struct enbd_device *lo)
+{
+        int i;
+        for (i = 0; i < lo->nslot; i++) {
+                struct enbd_slot * slot =  &lo->slots[i];
+                ENBD_INFO
+                ("device nd%s%d has pid %d\n", lo->devnam, i+1, slot->pid);
+        }
+}
+
 static int
 fixup_slot (struct enbd_device *lo, unsigned int cmd, unsigned long *arg)
 {
@@ -3771,11 +3803,9 @@
 	  case MY_NBD_CLR_REQ:
 	  case MY_NBD_ERR_REQ:
 		// see if we match a known slot pid
-		if (arg && *arg == 0) {
-			islot = find_slot (lo, current->pid);
-			if (islot >= 0)
-				return islot;
-		}
+		islot = find_slot (lo, current->pid); // PTB don't check arg
+		if (islot >= 0)
+			return islot;
 		ENBD_ALERT
 		 ("failed to find slot for pid %d for ioctl %s arg %lx\n",
 		  current->pid,
@@ -3784,6 +3814,7 @@
                   cmd == MY_NBD_ERR_REQ? "MY_NBD_ERR_REQ" :
                   "UNKNOWN",
                   arg ? *arg : 0);
+               print_slot_pids(lo);
 		return islot = -1;
 		break;
 
@@ -3811,6 +3842,7 @@
                   cmd == MY_NBD_SET_SPID? "MY_NBD_SET_SPID" :
                   "UNKNOWN",
                   arg ? *arg : 0);
+               print_slot_pids(lo);
 		return islot = -1;
 		break;
 
@@ -3826,6 +3858,7 @@
                   cmd == MY_NBD_ACK? "MY_NBD_ACK" :
                   "UNKNOWN",
                   arg ? *arg : 0);
+               print_slot_pids(lo);
 	        return islot;
 		break;
 
@@ -3863,7 +3896,8 @@
                                 cmd == MY_NBD_SET_SIG? "MY_NBD_SET_SIG" :
                                 "UNKNOWN",
                                *arg, intval);
                                );
+                       print_slot_pids(lo);
 			return islot = -1;
 		}
 		islot = intval - 1;