public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/4][Diskdump]Update patches
@ 2004-06-16 12:39 Takao Indoh
  2004-06-16 12:45 ` [PATCH 1/4][Diskdump]Update patches Takao Indoh
                   ` (4 more replies)
  0 siblings, 5 replies; 9+ messages in thread
From: Takao Indoh @ 2004-06-16 12:39 UTC (permalink / raw)
  To: linux-kernel

Hi!

I fixed diskdump patches except timer problem.

- Fix some codes which Arjan van de Ven pointed out
- Replace dev_t with block_device

Source code of tool(diskdumptuils) can be downloaded from
 http://sourceforge.net/projects/lkdump

Regarding timer problem, please see previous mail I sent.
 http://marc.theaimsgroup.com/?l=linux-kernel&m=108722344204595&w=2
In short, Timer problem is as follows.
> What is a problem?  Scsi driver uses timer/tasklet to complete I/O.
> But, diskdump disables interrupt, so timer/taslket doesn't work!

There are three ways to solve this problem so far.

(1) Redefine timer/taslket routines. This method was adopted in the
    first patch.

+#if defined(CONFIG_DISKDUMP) || defined(CONFIG_DISKDUMP_MODULE)
+#undef  add_timer
+#define add_timer       diskdump_add_timer
+#undef  del_timer_sync
+#define del_timer_sync  diskdump_del_timer
+#undef  del_timer
+#define del_timer       diskdump_del_timer
+#undef  mod_timer
+#define mod_timer       diskdump_mod_timer
+
+#define tasklet_schedule        diskdump_tasklet_schedule
+#endif


(2) Add new code into the timer/taslket routines themselves.

 static inline void add_timer(struct timer_list * timer)
 {
-	__mod_timer(timer, timer->expires);
+	if(crashdump_mode())
+		diskdump_add_timer(timer);
+	else
+		__mod_timer(timer, timer->expires);
 }


 int del_timer_sync(struct timer_list *timer)
 {
 	tvec_base_t *base;
 	int i, ret = 0;

+	if(crashdump_mode()) {
+		diskdump_del_timer(timer);
+		return 0;
+	}
+
 	check_timer(timer);


 int mod_timer(struct timer_list *timer, unsigned long expires)
 {
 	BUG_ON(!timer->function);
 
+	if(crashdump_mode()) {
+		diskdump_mod_timer(timer, expires);
+		return 0;
+	}
+
 	check_timer(timer);


 static inline void tasklet_schedule(struct tasklet_struct *t)
 {
+	if(crashdump_mode()) {
+		diskdump_tasklet_schedule(t);
+		return;
+	}
+
 	if (!test_and_set_bit(TASKLET_STATE_SCHED, &t->state))
 		__tasklet_schedule(t);
 }


(3) Change polling handler of driver not to use timer/tasklet.


The method (1) was already rejected because of its ugliness. The
method (3) needs many extra codes and makes handler of driver too big
and complex. I think the method (2) is the simplest though it make
common codes dirty.

Please feel free to comment.

Best Regards,
Takao Indoh

^ permalink raw reply	[flat|nested] 9+ messages in thread
* [PATCH 0/4][Diskdump]Update patches
@ 2004-06-22 13:47 Takao Indoh
  2004-06-22 13:59 ` [PATCH 1/4][Diskdump]Update patches Takao Indoh
  0 siblings, 1 reply; 9+ messages in thread
From: Takao Indoh @ 2004-06-22 13:47 UTC (permalink / raw)
  To: linux-kernel; +Cc: Ingo Molnar, Christoph Hellwig, Andi Kleen

Hi!

I update the Diskdump patches!

- fix timer problem
- support kernel 2.6.7

Source code of tool(diskdumptuils) can be downloaded from
 http://sourceforge.net/projects/lkdump

TODO:
- Replace proc interface with sysfs.
- Merge scsi_dump with scsi_mod.


I solved the timer problem by the method which Ingo Molnar proposed.

On Thu, 17 Jun 2004 14:13:56 +0200, Ingo Molnar wrote:

>but there's another possible method (suggested by Alan Cox) that
>requires no changes to the timer API hotpaths: 'clear' all timer lists
>upon a crash [once all CPUs have stopped and irqs are disabled] and just
>let the drivers use the normal timer APIs. Drive timer execution via a
>polling method.
>
>this basically approximates your polling based implementation but uses
>the existing kernel timer data structures and timer mechanism so should
>be robust and compatible. It doesnt rely on any previous state (because
>all currently pending timers are discarded) so it's as crash-safe as
>possible.


The following is core part of patches related to timer problem.
The complete patch is posted later.


diff -Nur linux-2.6.7.org/include/linux/interrupt.h linux-2.6.7/include/linux/interrupt.h
--- linux-2.6.7.org/include/linux/interrupt.h	2004-06-22 10:27:34.000000000 +0900
+++ linux-2.6.7/include/linux/interrupt.h	2004-06-22 22:26:39.000000000 +0900
@@ -246,4 +246,8 @@
 extern int probe_irq_off(unsigned long);	/* returns 0 or negative on failure */
 extern unsigned int probe_irq_mask(unsigned long);	/* returns mask of ISA interrupts */
 
+
+extern void dump_clear_tasklet(void);
+extern void dump_run_tasklet(void);
+
 #endif
diff -Nur linux-2.6.7.org/include/linux/timer.h linux-2.6.7/include/linux/timer.h
--- linux-2.6.7.org/include/linux/timer.h	2004-06-22 10:27:31.000000000 +0900
+++ linux-2.6.7/include/linux/timer.h	2004-06-22 22:26:39.000000000 +0900
@@ -99,4 +99,7 @@
 extern void run_local_timers(void);
 extern void it_real_fn(unsigned long);
 
+extern void dump_clear_timers(void);
+extern void dump_run_timers(void);
+
 #endif
diff -Nur linux-2.6.7.org/include/linux/workqueue.h linux-2.6.7/include/linux/workqueue.h
--- linux-2.6.7.org/include/linux/workqueue.h	2004-06-22 10:27:35.000000000 +0900
+++ linux-2.6.7/include/linux/workqueue.h	2004-06-22 22:26:39.000000000 +0900
@@ -84,4 +84,7 @@
 	return ret;
 }
 
+extern void dump_clear_workqueue(void);
+extern void dump_run_workqueue(void);
+
 #endif
diff -Nur linux-2.6.7.org/kernel/softirq.c linux-2.6.7/kernel/softirq.c
--- linux-2.6.7.org/kernel/softirq.c	2004-06-22 10:27:25.000000000 +0900
+++ linux-2.6.7/kernel/softirq.c	2004-06-22 22:26:39.000000000 +0900
@@ -314,6 +314,38 @@
 
 EXPORT_SYMBOL(tasklet_kill);
 
+struct tasklet_head saved_tasklet;
+
+void dump_clear_tasklet(void)
+{
+	saved_tasklet.list = __get_cpu_var(tasklet_vec).list;
+	__get_cpu_var(tasklet_vec).list = NULL;
+}
+
+EXPORT_SYMBOL(dump_clear_tasklet);
+
+void dump_run_tasklet(void)
+{
+	struct tasklet_struct *list;
+
+	list = __get_cpu_var(tasklet_vec).list;
+	__get_cpu_var(tasklet_vec).list = NULL;
+
+	while (list) {
+		struct tasklet_struct *t = list;
+		list = list->next;
+
+		if (!atomic_read(&t->count) &&
+		    (test_and_clear_bit(TASKLET_STATE_SCHED, &t->state)))
+				t->func(t->data);
+
+		t->next = __get_cpu_var(tasklet_vec).list;
+		__get_cpu_var(tasklet_vec).list = t;
+	}
+}
+
+EXPORT_SYMBOL(dump_run_tasklet);
+
 void __init softirq_init(void)
 {
 	open_softirq(TASKLET_SOFTIRQ, tasklet_action, NULL);
diff -Nur linux-2.6.7.org/kernel/timer.c linux-2.6.7/kernel/timer.c
--- linux-2.6.7.org/kernel/timer.c	2004-06-22 10:27:25.000000000 +0900
+++ linux-2.6.7/kernel/timer.c	2004-06-22 22:26:39.000000000 +0900
@@ -31,6 +31,7 @@
 #include <linux/time.h>
 #include <linux/jiffies.h>
 #include <linux/cpu.h>
+#include <linux/delay.h>
 
 #include <asm/uaccess.h>
 #include <asm/unistd.h>
@@ -424,7 +425,6 @@
 {
 	struct timer_list *timer;
 
-	spin_lock_irq(&base->lock);
 	while (time_after_eq(jiffies, base->timer_jiffies)) {
 		struct list_head work_list = LIST_HEAD_INIT(work_list);
 		struct list_head *head = &work_list;
@@ -460,6 +460,12 @@
 		}
 	}
 	set_running_timer(base, NULL);
+}
+
+static inline void _run_timers(tvec_base_t *base)
+{
+	spin_lock_irq(&base->lock);
+	__run_timers(base);
 	spin_unlock_irq(&base->lock);
 }
 
@@ -909,7 +915,7 @@
 	tvec_base_t *base = &__get_cpu_var(tvec_bases);
 
 	if (time_after_eq(jiffies, base->timer_jiffies))
-		__run_timers(base);
+		_run_timers(base);
 }
 
 /*
@@ -1105,6 +1111,12 @@
 	struct timer_list timer;
 	unsigned long expire;
 
+	if (unlikely(crashdump_mode())) {
+		mdelay(timeout);
+		set_current_state(TASK_RUNNING);
+		return timeout;
+	}
+
 	switch (timeout)
 	{
 	case MAX_SCHEDULE_TIMEOUT:
@@ -1308,7 +1320,7 @@
 	return 0;
 }
 
-static void __devinit init_timers_cpu(int cpu)
+static void /* __devinit */ init_timers_cpu(int cpu)
 {
 	int j;
 	tvec_base_t *base;
@@ -1327,6 +1339,27 @@
 	base->timer_jiffies = jiffies;
 }
 
+static tvec_base_t saved_tvec_base;
+
+void dump_clear_timers(void)
+{
+	tvec_base_t *base = &per_cpu(tvec_bases, smp_processor_id());
+
+	memcpy(&saved_tvec_base, base, sizeof(saved_tvec_base));
+	init_timers_cpu(smp_processor_id());
+}
+
+EXPORT_SYMBOL(dump_clear_timers);
+
+void dump_run_timers(void)
+{
+	tvec_base_t *base = &__get_cpu_var(tvec_bases);
+
+	__run_timers(base);
+}
+
+EXPORT_SYMBOL(dump_run_timers);
+
 #ifdef CONFIG_HOTPLUG_CPU
 static int migrate_timer_list(tvec_base_t *new_base, struct list_head *head)
 {
diff -Nur linux-2.6.7.org/kernel/workqueue.c linux-2.6.7/kernel/workqueue.c
--- linux-2.6.7.org/kernel/workqueue.c	2004-06-22 10:27:25.000000000 +0900
+++ linux-2.6.7/kernel/workqueue.c	2004-06-22 22:26:39.000000000 +0900
@@ -424,6 +424,37 @@
 
 }
 
+struct cpu_workqueue_struct saved_cwq;
+
+void dump_clear_workqueue(void)
+{
+	int cpu = smp_processor_id();
+	struct cpu_workqueue_struct *cwq = keventd_wq->cpu_wq + cpu;
+
+	memcpy(&saved_cwq, cwq, sizeof(saved_cwq));
+	spin_lock_init(&cwq->lock);
+	INIT_LIST_HEAD(&cwq->worklist);
+	init_waitqueue_head(&cwq->more_work);
+	init_waitqueue_head(&cwq->work_done);
+}
+
+void dump_run_workqueue(void)
+{
+	struct cpu_workqueue_struct *cwq;
+
+	cwq = keventd_wq->cpu_wq + smp_processor_id();
+	while (!list_empty(&cwq->worklist)) {
+		struct work_struct *work = list_entry(cwq->worklist.next,
+						struct work_struct, entry);
+		void (*f) (void *) = work->func;
+		void *data = work->data;
+
+		list_del_init(cwq->worklist.next);
+		clear_bit(0, &work->pending);
+		f(data);
+	}
+}
+
 #ifdef CONFIG_HOTPLUG_CPU
 /* Take the work from this (downed) CPU. */
 static void take_over_work(struct workqueue_struct *wq, unsigned int cpu)
@@ -507,3 +538,6 @@
 EXPORT_SYMBOL(schedule_delayed_work);
 EXPORT_SYMBOL(flush_scheduled_work);
 
+EXPORT_SYMBOL(dump_clear_workqueue);
+EXPORT_SYMBOL(dump_run_workqueue);
+

^ permalink raw reply	[flat|nested] 9+ messages in thread
* [PATCH 0/4][Diskdump]Update patches
@ 2004-07-07  8:26 Takao Indoh
  2004-07-07  8:30 ` [PATCH 1/4][Diskdump]Update patches Takao Indoh
  0 siblings, 1 reply; 9+ messages in thread
From: Takao Indoh @ 2004-07-07  8:26 UTC (permalink / raw)
  To: linux-kernel

Hi!

I release new diskdump patches.

- Support sysfs interface instead of proc
- Fix some bugs

Source code can be downloaded from
 http://sourceforge.net/projects/lkdump


The main change from previous version is replacing proc interface with
sysfs.
If you want to add a new dump device /dev/sda3, 

  # echo 3 > /sys/block/sda/device/dump

Like this, you need write the partition number into the appropriate
sysfs entry. You can also do the same thing using diskdumpctl command, 
which is included in the diskdumputils.

  # diskdumpctl /dev/sda3

If you want to remove the dump device /dev/sda3, 

  # echo -3 > /sys/block/sda/device/dump

or

  # diskdumpctl -u /dev/sda1

Here is a part of patch of sysfs interface.
I added a new attribute "dump" to the /sys/block/sdX/device. The handler
of show/store operation calls a function which is registered via
sdev_dump_handler_register(). When the scsi_dump module is installed,
sysfs handler of scsi_dump is registered using 
sdev_dump_handler_register().

Please feel free to comment!


diff -Nur linux-2.6.7.org/drivers/scsi/scsi_sysfs.c linux-2.6.7/drivers/scsi/scsi_sysfs.c
--- linux-2.6.7.org/drivers/scsi/scsi_sysfs.c	2004-06-22 10:27:50.000000000 +0900
+++ linux-2.6.7/drivers/scsi/scsi_sysfs.c	2004-07-07 17:01:26.146353152 +0900
@@ -375,6 +375,66 @@
 
 DEVICE_ATTR(state, S_IRUGO | S_IWUSR, show_state_field, store_state_field);
 
+static DECLARE_MUTEX(sdev_dump_mutex);
+
+static struct device_attribute sdev_dump_attr = {
+	.show  = NULL,
+	.store = NULL,
+};
+
+int sdev_dump_handler_register(struct device_attribute* attr)
+{
+	if (sdev_dump_attr.show || sdev_dump_attr.store)
+		return -EEXIST;
+
+	down(&sdev_dump_mutex);
+	sdev_dump_attr = *attr;
+	up(&sdev_dump_mutex);
+
+	return 0;
+}
+
+EXPORT_SYMBOL(sdev_dump_handler_register);
+
+void sdev_dump_handler_unregister(void)
+{
+	down(&sdev_dump_mutex);
+	sdev_dump_attr.show  = NULL;
+	sdev_dump_attr.store = NULL;
+	up(&sdev_dump_mutex);
+}
+
+EXPORT_SYMBOL(sdev_dump_handler_unregister);
+
+static ssize_t
+sdev_store_dump(struct device *dev, const char *buf, size_t count)
+{
+	ssize_t ret = count;
+
+	down(&sdev_dump_mutex);
+	if (sdev_dump_attr.store)
+		ret = sdev_dump_attr.store(dev, buf, count);
+	up(&sdev_dump_mutex);
+
+	return ret;
+}
+
+static ssize_t
+sdev_show_dump(struct device *dev, char *buf)
+{
+	ssize_t ret;
+
+	down(&sdev_dump_mutex);
+	if (sdev_dump_attr.show)
+		ret = sdev_dump_attr.show(dev, buf);
+	else
+		ret = snprintf(buf, 20, "handler not found\n");
+	up(&sdev_dump_mutex);
+
+	return ret;
+}
+
+DEVICE_ATTR(dump, S_IRUGO | S_IWUSR, sdev_show_dump, sdev_store_dump);
 
 /* Default template for device attributes.  May NOT be modified */
 static struct device_attribute *scsi_sysfs_sdev_attrs[] = {
@@ -389,6 +449,7 @@
 	&dev_attr_delete,
 	&dev_attr_state,
 	&dev_attr_timeout,
+	&dev_attr_dump,
 	NULL
 };
 
 

Best Regards,
Takao Indoh

^ permalink raw reply	[flat|nested] 9+ messages in thread
* [PATCH 0/4][Diskdump]Update patches
@ 2004-07-09  7:15 Takao Indoh
  2004-07-09  7:19 ` [PATCH 1/4][Diskdump]Update patches Takao Indoh
  0 siblings, 1 reply; 9+ messages in thread
From: Takao Indoh @ 2004-07-09  7:15 UTC (permalink / raw)
  To: linux-kernel; +Cc: arjanv, hch

Hi all,

I improved the patches which I posted on 7th Jul.

- Improve sysfs interface code
- Bug fix related to eh_timeout of scsi_cmd

Source code can be downloaded from
 http://sourceforge.net/projects/lkdump


On Mon, 21 Jun 2004 10:01:29 +0200, Arjan van de Ven wrote:

>On Mon, Jun 21, 2004 at 04:59:52PM +0900, Takao Indoh wrote:
>> Hi,
>> 
>> Now I am fixing diskdump according to comments by you and Christoph.
>> 
>> On Fri, 11 Jun 2004 13:50:45 +0200, Arjan van de Ven wrote:
>> 
>> >> +#ifdef CONFIG_PROC_FS
>> >> +static int proc_ioctl(struct inode *inode, struct file *file, unsigned 
>> >> int cmd, unsigned long param)
>> >
>> >
>> >ehhh this looks evil
>> 
>> Do you mean I should use not ioctl but the following style?
>> 
>> echo "add /dev/hda1" > /proc/diskdump
>> echo "delete /dev/hda1" > /proc/diskdump
>
>well no since /dev/hda is pointless; major/minor pairs maybe.
>But why in /proc???? it sounds like a sysfs job to me, where you probably
>want to represent a dump relationship with a symlink, and use "rm" to remove
>an entry..

As I wrote in the previous mail, I replaced proc interface with
sysfs.

If you want to add a new dump device /dev/sda3,

  # echo 3 > /sys/block/sda/device/dump

If you want to remove the dump device /dev/sda3,

  # echo -3 > /sys/block/sda/device/dump


I added a new attribute "dump" to the /sys/block/sdX/device.

diff -Nur linux-2.6.7.org/drivers/scsi/scsi_sysfs.c linux-2.6.7/drivers/scsi/scsi_sysfs.c
--- linux-2.6.7.org/drivers/scsi/scsi_sysfs.c	2004-06-22 10:27:50.000000000 +0900
+++ linux-2.6.7/drivers/scsi/scsi_sysfs.c	2004-07-09 11:36:41.451983216 +0900
@@ -11,6 +11,7 @@
 #include <linux/init.h>
 #include <linux/blkdev.h>
 #include <linux/device.h>
+#include <linux/diskdump.h>
 
 #include <scsi/scsi_host.h>
 #include <scsi/scsi_transport.h>
@@ -375,6 +376,7 @@
 
 DEVICE_ATTR(state, S_IRUGO | S_IWUSR, show_state_field, store_state_field);
 
+DEVICE_ATTR(dump, S_IRUGO | S_IWUSR, diskdump_sysfs_show, diskdump_sysfs_store);
 
 /* Default template for device attributes.  May NOT be modified */
 static struct device_attribute *scsi_sysfs_sdev_attrs[] = {
@@ -389,6 +391,7 @@
 	&dev_attr_delete,
 	&dev_attr_state,
 	&dev_attr_timeout,
+	&dev_attr_dump,
 	NULL
 };
 

diskdump_sysfs_show/store is the sysfs handler of diskdump. These
functions passes struct device to the diskdump module.
Diskdump changes "struct device" to "struct scsi_device" like this:

+static void *scsi_dump_probe(struct device *dev)
+{
+	struct scsi_device *sdev;
+
+	if ((dev->bus == NULL) || (dev->bus->name == NULL) ||
+	    strncmp(dev->bus->name, "scsi", 4))
+		return NULL;
+
+	sdev =  to_scsi_device(dev);
+	if (!sdev->host->hostt->dump_poll)
+		return NULL;
+
+	return sdev;
+}

I used device->bus->name to confirm device is scsi device or not.

Please comment.

Best Regards,
Takao Indoh

^ permalink raw reply	[flat|nested] 9+ messages in thread
end of thread, other threads:[~2004-07-09  7:21 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2004-06-16 12:39 [PATCH 0/4][Diskdump]Update patches Takao Indoh
2004-06-16 12:45 ` [PATCH 1/4][Diskdump]Update patches Takao Indoh
2004-06-16 12:46 ` [PATCH 2/4][Diskdump]Update patches Takao Indoh
2004-06-16 12:48 ` [PATCH 3/4][Diskdump]Update patches Takao Indoh
2004-06-16 12:50 ` [PATCH 4/4][Diskdump]Update patches Takao Indoh
2004-06-16 17:25 ` [PATCH 0/4][Diskdump]Update patches Jeff Garzik
  -- strict thread matches above, loose matches on Subject: below --
2004-06-22 13:47 Takao Indoh
2004-06-22 13:59 ` [PATCH 1/4][Diskdump]Update patches Takao Indoh
2004-07-07  8:26 [PATCH 0/4][Diskdump]Update patches Takao Indoh
2004-07-07  8:30 ` [PATCH 1/4][Diskdump]Update patches Takao Indoh
2004-07-09  7:15 [PATCH 0/4][Diskdump]Update patches Takao Indoh
2004-07-09  7:19 ` [PATCH 1/4][Diskdump]Update patches Takao Indoh

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox