hangcheck-timer: use pr_crit and fix coding style
PATCH 1/1 hangcheck-timer: use pr_crit and fix coding style
Fix coding style issues and improve logging in hangcheck-timer such as replace printk(KERN_CRIT ...) to pr_crit(...) where applicable and replace non-standard %Ld with %lld in debug prints and use proper pr_debug/pr_crit for kernel logging.
No functional changes were made to hangcheck logic.
Testing:
- Verified timer firing and margin detection using "hangtest" module that i created (I will paste the code below for reference) and checked dmesg logs for expected output: "Hangcheck: hangcheck value past margin!".
- Used Static Analysis tools
- Ensured module builds and inserts cleanly after changes.
Let me know if you want me to do more testing on this module.
[] hangtest.c (testing module i created to test hangcheck-timer module):
MODULE_LICENSE("GPL");MODULE_AUTHOR("Clint George"); MODULE_DESCRIPTION("Hang test for testing hangcheck-timer");
static int hang_duration = 20;module_param(hang_duration, int, 0644);
static int __init hangtest_init(void) {
unsigned long timeout;
printk(KERN_INFO "hangtest: Disabling interrupts for %d seconds...\n", hang_duration);
local_irq_disable();
preempt_disable();
timeout = jiffies + (hang_duration * HZ);
while (time_before(jiffies, timeout)) {
cpu_relax();
barrier();
}
preempt_enable();
local_irq_enable();
printk(KERN_INFO "hangtest: Interrupts re-enabled\n");// return 0;
return -EINVAL; // Return error so module doesn't stay loaded
}
static void __exit hangtest_exit(void) {
printk(KERN_INFO "hangtest: Exit\n");
}module_init(hangtest_init);module_exit(hangtest_exit);
--- drivers/char/hangcheck-timer.c | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-)
drivers/char/hangcheck-timer.c +15 -10
@@ -69,7 +69,8 @@ MODULE_VERSION(VERSION_STR);static int __init hangcheck_parse_tick (char *str) {int par; -if (get_option (&str,&par)) + +if (get_option (&str, &par)) hangcheck_tick = par;return 1 ; }
@@ -77,7 +78,8 @@ static int __init hangcheck_parse_tick(char *str)static int __init hangcheck_parse_margin (char *str) {int par; -if (get_option (&str,&par)) + +if (get_option (&str, &par)) hangcheck_margin = par;return 1 ; }
@@ -85,7 +87,8 @@ static int __init hangcheck_parse_margin(char *str)static int __init hangcheck_parse_reboot (char *str) {int par; -if (get_option (&str,&par)) + +if (get_option (&str, &par)) hangcheck_reboot = par;return 1 ; }
@@ -93,7 +96,8 @@ static int __init hangcheck_parse_reboot(char *str)static int __init hangcheck_parse_dump_tasks (char *str) {int par; -if (get_option (&str,&par)) + +if (get_option (&str, &par)) hangcheck_dump_tasks = par;return 1 ; }
@@ -126,23 +130,24 @@ static void hangcheck_fire(struct timer_list *unused)if (tsc_diff > hangcheck_tsc_margin) {if (hangcheck_dump_tasks) { -printk (KERN_CRIT "Hangcheck: Task state:\n" ); +pr_crit ("Hangcheck: Task state:\n" ); +#ifdef CONFIG_MAGIC_SYSRQ handle_sysrq ('t' );#endif /* CONFIG_MAGIC_SYSRQ */ }if (hangcheck_reboot) { -printk (KERN_CRIT "Hangcheck: hangcheck is restarting the machine.\n" ); +pr_crit ("Hangcheck: hangcheck is restarting the machine.\n" );emergency_restart (); }else { -printk (KERN_CRIT "Hangcheck: hangcheck value past margin!\n" ); +pr_crit ("Hangcheck: hangcheck value past margin!\n" ); } }#if 0 /* * Enable to investigate delays in detail */ -printk ("Hangcheck: called %Ld ns since last time (%Ld ns overshoot)\n" , +pr_debug ("Hangcheck: called %lld ns since last time (%lld ns overshoot)\n" , tsc_diff, tsc_diff - hangcheck_tick*TIMER_FREQ );#endif mod_timer (&hangcheck_ticktock, jiffies + (hangcheck_tick*HZ ));
@@ -152,7 +157,7 @@ static void hangcheck_fire(struct timer_list *unused)static int __init hangcheck_init (void ) { -printk ("Hangcheck: starting hangcheck timer %s (tick is %d seconds, margin is %d seconds).\n" , +pr_debug ("Hangcheck: starting hangcheck timer %s (tick is %d seconds, margin is %d seconds).\n" ,VERSION_STR , hangcheck_tick, hangcheck_margin); hangcheck_tsc_margin = (unsigned long long )hangcheck_margin + hangcheck_tick;
@@ -168,7 +173,7 @@ static int __init hangcheck_init(void) static void __exit hangcheck_exit(void) {timer_delete_sync (&hangcheck_ticktock); -printk ("Hangcheck: Stopped hangcheck timer.\n" ); +pr_debug ("Hangcheck: Stopped hangcheck timer.\n" ); }module_init (hangcheck_init);
Signature · 1 line
2.34.1
Quoted 38 lines · Clint George
Testing:
- Verified timer firing and margin detection using "hangtest" module that i created (I will paste the code below for reference) and checked dmesg logs for expected output: "Hangcheck: hangcheck value past margin!".
- Used Static Analysis tools
- Ensured module builds and inserts cleanly after changes.Let me know if you want me to do more testing on this module.
[]
hangtest.c(testing module i created to test hangcheck-timer module):
MODULE_LICENSE("GPL");MODULE_AUTHOR("Clint George");MODULE_DESCRIPTION("Hang test for testing hangcheck-timer");static int
hang_duration= 20;module_param(hang_duration, int, 0644);static int __init hangtest_init(void) { unsigned long timeout; printk(KERN_INFO "hangtest: Disabling interrupts for %d seconds...\n", hang_duration); local_irq_disable(); preempt_disable(); timeout = jiffies + (hang_duration * HZ); while (time_before(jiffies, timeout)) { cpu_relax(); barrier(); } preempt_enable(); local_irq_enable(); printk(KERN_INFO "hangtest: Interrupts re-enabled\n");// return 0;
return-EINVAL; // Return error so module doesn't stay loaded
}
Quoted 6 lines
static void __exit hangtest_exit(void) { printk(KERN_INFO "hangtest: Exit\n"); }
module_init(hangtest_init);module_exit(hangtest_exit);
Well done for describing the testing you did, but try to put it in the change log. As of now, it is in the commit message, which means that if your patch is committed, people using git log, would be able to see all of this, but this is something that is only needed for the maintainers, not for future code viewers.
Plus, they can always recreate your patch if they need to.
Thanks,
David Hunter