Matt,
Attached is a patch with some hacks related to dumping from interrupt
context in the situation where the underlying device driver relies on
bottom-halfs/softirqs for its i/o completion processing.
We may not hit this case with all drivers. I discovered today that
Alt+Sysrq+c dumping seemed to be working on our setup now even when
I took out these patches, and spent a lot of time puzzling over why it
did, till it turned out that we were using the old aic7xxx driver which
doesn't appear to be depending on bottom-halfs for i/o completion.
That may be the case with IDE too.
The new aic7xxx driver seems to use scsi_done rather than scsi_old_done
which does appears to use bottom-halfs.
We haven't had a chance to try to test this again with the new aic7xxx driver
to verify this today with 4.0 patch. But since I'll be on vacation from
day-after (19th-29th), just decided to send this out to you, so you can
look through it and see if this makes sense, and perhaps try this out
in your setup and let us know what you think.
The changes are fairly simple, as you can see:
- Set the irq and bh counts to zero during dumping in addition to sti()
complete the pretence that we aren't in interrupt context. Restore the
original settings back after dump. (See comments in patch)
- Avoid wakeups on the dumping thread happening during i/o completion. It
is not necessary since we are never scheduled out, and it could result
in unexpected side effects when we continue after dump (non-disruptive
dump case) if dump i/o happens in the context of the idle thread
(which could be the case for Alt+Sysrq+c interrupts).
In order to achieve this with minimal intrusion, kiobuf_end_io has been
modified to issue wakeups only if an end_io routine has not been specified.
This would be in line with the way it happens with bh end_io, and
appears to be an acceptable change in general (from what I could tell
when I checked with a few people)
I don't see any users of kiobuf->end_io in the kernel tree, but there
could be some outside. If so, they would be impacted. So that's something
to look out for.
So dump_iobuf now uses a dummy end_io function, so that it can bypass
the need for wakeup.
- Also changed dump_kernel_write() to use iobuf->blocks[] instead of
allocating b[KIO_MAX_SECTORS] on stack, while at it.
- Call dump() with the correct regs from sysrq, instead of panic, so that
we can support both disruptive and non-disruptive dumps correctly.
This is obviously only a temporary workaround, not a reliable solution
for longer term - just until we have the dump driver interface or other
solution and a deferred dumping scheme for non-disruptive dumps.
But it could possibly be used for getting Alt+Sysrq+c dumps for now.
The patch is with respect to the latest lkcd4.0 tree that we tried out
earlier today.
------------- PATCH HERE -------------------------------------
diff -ur linux-2.4.8+lkcd/drivers/char/sysrq.c suparna/drivers/char/sysrq.c
--- linux-2.4.8+lkcd/drivers/char/sysrq.c Wed Oct 17 16:05:56 2001
+++ suparna/drivers/char/sysrq.c Wed Oct 17 16:02:23 2001
@@ -93,7 +93,7 @@
#if defined(CONFIG_DUMP) || defined(CONFIG_DUMP_MODULE)
case 'c':
- panic("sysrq");
+ dump("sysrq", pt_regs);
break;
#endif
diff -ur linux-2.4.8+lkcd/drivers/dump/dump_base.c
suparna/drivers/dump/dump_base.c
--- linux-2.4.8+lkcd/drivers/dump/dump_base.c Wed Oct 17 16:04:18 2001
+++ suparna/drivers/dump/dump_base.c Wed Oct 17 16:01:34 2001
@@ -252,6 +252,10 @@
static char dpcpage[DUMP_DPC_PAGE_SIZE]; /* buffer used for compression */
static unsigned long dump_save_flags; /* save_flags()/restore_flags() */
+/* for dumping from interrupt context (Fixme) */
+static int saved_irq_count; /* remember the current irq nesting level */
+static int saved_bh_count; /* remember if we were in soft irq context */
+
/* used for dump compressors */
static struct list_head dump_compress_list =
LIST_HEAD_INIT(dump_compress_list);
@@ -383,7 +387,8 @@
dump_kernel_write(void)
{
int err = 0, iosize, i;
- unsigned long b[KIO_MAX_SECTORS], blocknr, blocks, limit;
+ unsigned long *b=dump_iobuf->blocks;
+ unsigned long blocknr, blocks, limit;
/* check the device size to make sure we are in-line */
if (blk_size[MAJOR(dump_device)]) {
@@ -565,6 +570,15 @@
unsigned int stage = 0;
int cpu = smp_processor_id();
+ if (in_interrupt()) {
+ printk("Dumping from interrupt handler !\n");
+ printk("Uncertain scenario - but will try my best\n");
+ /*
+ * Must be an unrelated interrupt, not in the middle of io !
+ * If we've panic'ed in the middle of io we should take
+ * another approach
+ */
+ }
/* see if there's something to do before we re-enable interrupts */
(void)__dump_silence_system(stage);
@@ -573,6 +587,26 @@
dumping_cpu = cpu;
dump_in_progress = TRUE;
save_flags(dump_save_flags);
+
+ /* -------------------------------------------------- */
+ /* Kludge - dump from interrupt context is unreliable (Fixme)
+ *
+ * We do this so that softirqs initiated for dump i/o
+ * get processed and we don't hang while waiting for i/o
+ * to complete or in any irq synchronization attempt.
+ *
+ * This is not quite legal of course, as it has the side
+ * effect of making all interrupts & softirqs triggered
+ * while dump is in progress complete before currently
+ * pending softirqs and the currently executing interrupt
+ * code.
+ */
+ saved_irq_count = local_irq_count(cpu);
+ saved_bh_count = local_bh_count(cpu);
+ local_irq_count(cpu) = 0;
+ local_bh_count(cpu) = 0;
+ /* -----------------------------------------------------*/
+
sti(); /* enable interrupts just in case ... */
/* now increment the stage and do stuff after interrupts are enabled */
@@ -597,6 +631,8 @@
/* restore flags and other dump state fields */
restore_flags(dump_save_flags);
+ local_irq_count(dumping_cpu) = saved_irq_count;
+ local_bh_count(dumping_cpu) = saved_bh_count;
dump_in_progress = FALSE;
dump_okay = TRUE;
@@ -1003,6 +1039,11 @@
return (-EAGAIN);
}
+void dump_iobuf_end_io(struct kiobuf *iobuf)
+{
+ /* No wakeup needed since we've stopped scheduling */
+ return;
+}
/*
* Name: dump_open_kdev()
* Func: Try to open the kdev_t argument as the real dump device.
@@ -1076,6 +1117,7 @@
dump_iobuf->offset = 0;
dump_iobuf->length = DUMP_PAGE_SZ;
dump_iobuf->nr_pages = (DUMP_PAGE_SZ >> PAGE_SHIFT);
+ dump_iobuf->end_io = dump_iobuf_end_io;
/* assign the new dump file structure */
dump_device = tmp_dump_device;
diff -ur linux-2.4.8+lkcd/fs/iobuf.c suparna/fs/iobuf.c
--- linux-2.4.8+lkcd/fs/iobuf.c Wed Oct 17 16:05:03 2001
+++ suparna/fs/iobuf.c Wed Oct 17 16:01:49 2001
@@ -17,8 +17,10 @@
if (atomic_dec_and_test(&kiobuf->io_count)) {
if (kiobuf->end_io)
+ /* the end_io fn should take care of waiters too */
kiobuf->end_io(kiobuf);
- wake_up(&kiobuf->wait_queue);
+ else
+ wake_up(&kiobuf->wait_queue);
}
}
@@ -121,7 +123,6 @@
iobuf->array_len = wanted;
return 0;
}
-
void kiobuf_wait_for_io(struct kiobuf *kiobuf)
{
|