Messages in this thread |  | | Date | Tue, 21 Jan 2003 08:11:58 +0000 | From | Christoph Hellwig <> | Subject | Re: [PATCH][2.5] hangcheck-timer |
| |
> +#include <linux/module.h> > +#include <linux/config.h>
I can't your see the driver reference CONFIG_* directly anywhere.
> +#define VERSION_STR "0.5.0" > + > +static void version_hash_print (void) > +{ > + printk(KERN_INFO "I/O fencing modules %s\n", VERSION_STR); > +}
This message is a bit misleading, isn't it?
> +static int hangcheck_tick = DEFAULT_IOFENCE_TICK; > +static int hangcheck_margin = DEFAULT_IOFENCE_MARGIN; > +static int hangcheck_reboot = 0; /* Do not reboot */
no need to initialize static variables to zero, they'll just go into .bss.
> +/* options - modular */ > +MODULE_PARM(hangcheck_tick,"i"); > +MODULE_PARM_DESC(hangcheck_tick, "Timer delay."); > +MODULE_PARM(hangcheck_margin,"i"); > +MODULE_PARM_DESC(hangcheck_margin, "If the hangcheck timer has been delayed more than hangcheck_margin seconds, the driver will fire."); > +MODULE_PARM(hangcheck_reboot,"i"); > +MODULE_PARM_DESC(hangcheck_reboot, "If nonzero, the machine will reboot when the timer margin is exceeded.");
It might be worth using Rusty's new module paramters for new code submitted to 2.5 instead of the legacy interfaces.
> +#if 0 > + printk(KERN_CRIT "tsc_diff = %lu.%lu, predicted diff is %lu.%lu.\n", > + (unsigned long) ((tsc_diff >> 32) & 0xFFFFFFFFULL), > + (unsigned long) (tsc_diff & 0xFFFFFFFFULL), > + (unsigned long) ((hangcheck_tsc_margin >> 32) & 0xFFFFFFFFULL), > + (unsigned long) (hangcheck_tsc_margin & 0xFFFFFFFFULL)); > + printk(KERN_CRIT "hangcheck_margin = %lu, HZ = %lu, current_cpu_data.loops_per_jiffy = %lu.\n", hangcheck_margin, HZ, current_cpu_data.loops_per_jiffy); > +#endif
#if DEBUG maybe? or VERBOSE?
> +static int __init hangcheck_init(void) > +{ > + version_hash_print(); > + printk("Hangcheck: starting hangcheck timer (tick is %d seconds, margin is %d seconds).\n", > + hangcheck_tick, hangcheck_margin);
Two startup messages seems like a bit too much.
> +} /* hangcheck_init() */
Bah 8)
> +static void __exit hangcheck_exit(void) > +{ > + printk("Stopping hangcheck timer.\n");
Again, this is exteremly verbose.
> + lock_kernel(); > + del_timer(&hangcheck_ticktock); > + unlock_kernel();
No need for BKL here, but you might want to use del_timer_sync.
- 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/
|  |