lkml.org 
[lkml]   [2004]   [Jun]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [3/4] [PATCH]Diskdump - yet another crash dump function
On Thu, Jun 17, 2004 at 08:34:37PM +0900, Takao Indoh wrote:
> Instead of redefinition, I add hooks into the timer/tasklet routines.
>
> ex.
>
> int mod_timer(struct timer_list *timer, unsigned long expires)
> {
> BUG_ON(!timer->function);
>
> + if (crashdump_mode()) {
> + return diskdump_mod_timer(timer, expires);
> + }
>
> check_timer(timer);
>
> Please see the following patches for details.
> How do you think?

Looks good in principle, some more comments on the actual patch below:

> diff -Nur linux-2.6.6.org/include/linux/diskdumplib.h linux-2.6.6/include/linux/diskdumplib.h
> --- linux-2.6.6.org/include/linux/diskdumplib.h 1970-01-01 09:00:00.000000000 +0900
> +++ linux-2.6.6/include/linux/diskdumplib.h 2004-06-17 18:28:01.000000000 +0900

Should probably just go to linux/dump.h or dump_priv.h

> diff -Nur linux-2.6.6.org/include/linux/interrupt.h linux-2.6.6/include/linux/interrupt.h
> --- linux-2.6.6.org/include/linux/interrupt.h 2004-06-04 21:22:09.000000000 +0900
> +++ linux-2.6.6/include/linux/interrupt.h 2004-06-17 18:28:01.000000000 +0900
> @@ -7,6 +7,7 @@
> #include <linux/linkage.h>
> #include <linux/bitops.h>
> #include <linux/preempt.h>
> +#include <linux/diskdumplib.h>
> #include <asm/atomic.h>
> #include <asm/hardirq.h>
> #include <asm/ptrace.h>
> @@ -172,6 +173,11 @@
>
> 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);
> }

Please sprinclde unlikely's all over here. Also the above could be
shortened to

static inline void tasklet_schedule(struct tasklet_struct *t)
{
if (unlikely(crashdump_mode()))
diskdump_tasklet_schedule(t);
else if (!test_and_set_bit(TASKLET_STATE_SCHED, &t->state))
__tasklet_schedule(t);
}
> +int diskdump_schedule_work(struct work_struct *work)
> +{
> + list_add_tail(&work->entry, &diskdump_workq);
> + return 1;
> +}

Should probably just inlined, I guess the function call is bigger
than all of list_add_tail.

> +void diskdump_add_timer(struct timer_list *timer)
> +{
> + timer->base = (void *)1;
> + list_add(&timer->entry, &diskdump_timers);
> +}

dito.

> + /* run timers */
> + list_for_each_safe(t, n, &diskdump_timers) {
> + timer = list_entry(t, struct timer_list, entry);

list_for_each_entry_safe here please.

> @@ -255,6 +255,10 @@
> {
> BUG_ON(!timer->function);
>
> + if (crashdump_mode()) {
> + return diskdump_mod_timer(timer, expires);
> + }

please remove the superflous braces here.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2005-03-22 14:03    [from the cache]
©2003-2011 Jasper Spaans