Re: [3/4] [PATCH]Diskdump - yet another crash dump function

[Takao Indoh <indou.takao@soft.fujitsu.com>]

Hi,

Thank you for comment. After I fix these codes, I'll post new version of
diskdump.

Best Regards,
Takao Indoh


>> 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.
>