[ENBD] stability issues, big red neon sign
Peter T. Breuer
enbd@lists.community.tummy.com
Wed, 27 Mar 2002 14:13:13 +0100 (MET)
"A month of sundays ago Arne Wiebalck wrote:"
> I think maxcpus is the _number_ of CPUs activated in smp mode.
I thought it was the high ID.
> at least, maxcpus=1 on boot results in /proc/cpuinfo claiming that there
> is only one cpu with id 0.
You seem to be right. Look at smpboot.c:
/*
* Setup routine for controlling SMP activation
*
* Command-line option of "nosmp" or "maxcpus=0" will disable SMP
* activation entirely (the MPS table probe still happens, though).
*
* Command-line option of "maxcpus=<NUM>", where <NUM> is an integer
* greater than 0, limits the maximum number of CPUs activated in
* SMP mode to <NUM>.
*/
aha.
> as I said, with maxcpus=0 (or the equivalent nosmp option) my machines
> refuse to boot. no idea why...
Well, it appears that cpu 0 needs specific kicking.
> since I was not sure what exactly the maxcpus=1 option results in and what
> exactly is the difference to maxcpus=0, I compiled the UP kernel :)
> > I'll do that as soon as you resolve my minor confusion above!
>
> hope, I could clear things up - at least as far as my tests are concerned.
Yes. You are running with one processor, and it has been activated.
OK, here is a serialization patch, barely tested. It serializes only
client daemon accesses. What it does is make the
nbd_get_req .... (talk to server) ... nbd_ack
cycle atomic. The daemon takes the device semaphore on the way in to
nbd_get_req, and releases it on the way out of nbd_ack.
It is to be hoped that the action on error is OK. There is a timeout
for getting the semaphore. If there is an error when the semaphore is
held I have arranged that the semaphore is released and the error is
returned to the caller. I believe this is correct with respect to the
client's half of the protocol (and I _have_ looked at it), which will
retry 3 times if there is an error on nbd_get_req, and will ignore it if
there was an error in the nbd_ack (the kernel will resupply the request
later if it wasn't satisfied).
I have confirmed that the patch compiles and runs. I haven't checked if
it does what it is supposed to do!
Good luck!
If it has an effect, we're on the right track. If it doesn't have any
effect, then the problem is in the routine that's called by the kernel
when there are requests on the device queue. That's called under the
i/o spinlock, so I don't see how it can interfere with _itself_. Maybe
it's not protected sufficiently against multiple other threads .. oh,
well, let me know ...
Peter
--- nbd-2.4.28/linux/drivers/block/nbd.c Mon Mar 25 00:22:52 2002
+++ nbd-2.4.29/linux/drivers/block/nbd.c Wed Mar 27 13:50:23 2002
@@ -1288,13 +1288,21 @@
NBD_DEBUG (3, "(%d): entered\n", slot->i);
+ // PTB we should have the DEBUG smp semaphore at this point, if we do!
+
if (!(slot->flags & NBD_SLOT_BUFFERED)) {
NBD_DEBUG (1, "(%d): exited INVAL\n", slot->i);
+ // PTB we release the DEBUG smp semaphore that we had
+ up(&lo->sem);
+ wake_up_interruptible (&lo->wq);
return -EINVAL;
}
if (slot->buffer != buffer) {
if (slot->nerrs++ < 3)
NBD_ALERT ("(%d): user buffer changed\n", slot->i);
+ // PTB we release the DEBUG smp semaphore that we had
+ up(&lo->sem);
+ wake_up_interruptible (&lo->wq);
return -EINVAL;
}
@@ -1570,6 +1578,9 @@
slot->flags &= ~NBD_SLOT_RUNNING;
if (reply.flags & NBD_REPLY_IOCTL)
NBD_DEBUG (1, "(%d): exited OK\n", slot->i);
+ // PTB release the DEBUG smp semaphore
+ up(&lo->sem);
+ wake_up_interruptible (&lo->wq);
return 0;
error_out:
@@ -1582,6 +1593,9 @@
result = result < 0 ? result : -ENODEV;
if (reply.flags & NBD_REPLY_IOCTL)
NBD_DEBUG (1, "(%d): exited FAIL %d\n", slot->i, result);
+ // PTB release the DEBUG smp semaphore and let the client retry
+ up(&lo->sem);
+ wake_up_interruptible (&lo->wq);
return result;
}
@@ -1835,6 +1849,14 @@
//if (timeo < timeout)
// timeout = timeo;
+ while (down_trylock(&lo->sem) != 0) {
+ // PTB serializing for DEBUG smp
+ interruptible_sleep_on_timeout (&lo->wq, 1);
+ if (jiffies > start_time + timeout) {
+ return -ETIME;
+ }
+ }
+
atomic_inc (&lo->cthreads); // PTB - client thread enters
slot->flags |= NBD_SLOT_RUNNING;
slot->cli_age = jiffies;
@@ -2222,6 +2244,7 @@
slot->flags &= ~NBD_SLOT_RUNNING;
NBD_DEBUG (1, "exiting\n");
+ // PTB if we took the DEBUG smp semaphore, don't necesarily release it yet
return 0;
error_out:
@@ -2230,6 +2253,10 @@
slot->flags &= ~NBD_SLOT_RUNNING;
result = result < 0 ? result : -ENODEV;
+ // PTB release the DEBUG smp semaphore and let the client retry
+ up(&lo->sem);
+ wake_up_interruptible (&lo->wq);
+
NBD_DEBUG (1, "(%d): error exit %d\n", islot, result);
return result;
}
@@ -5464,6 +5491,7 @@
init_waitqueue_head (&lo->wq);
NBD_INIT_LIST_HEAD (&lo->req);
init_waitqueue_head (&lo->req_wq);
+ init_MUTEX(&lo->sem);
for (j = 0; j < NBD_MAXCONN; j++) {
nbd_blksizes[i * NBD_MAXCONN + j] = lo->blksize;
nbd_bytesizes[i * NBD_MAXCONN + j] = lo->bytesize;
--- nbd-2.4.28/linux/include/linux/nbd.h Mon Mar 25 00:27:03 2002
+++ nbd-2.4.29/linux/include/linux/nbd.h Wed Mar 27 12:39:46 2002
@@ -261,6 +261,7 @@
struct wait_queue* req_wq;
#endif
char ctldta[4*4]; /* PTB ioctl data buffer */
+ struct semaphore sem; /* PTB debugging aid */
};
#endif /* MAJOR_NR */