lkml.org 
[lkml]   [2003]   [Jan]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [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/

    \
     
     \ /
      Last update: 2005-03-22 13:32    [W:0.051 / U:0.928 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site