lkml.org 
[lkml]   [2004]   [Jun]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    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    [W:0.028 / U:0.460 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site