* [PATCH] hangcheck-timer: use pr_crit and fix coding style
@ 2025-10-18 16:00 Clint George
2025-10-22 12:24 ` David Hunter
0 siblings, 1 reply; 4+ messages in thread
From: Clint George @ 2025-10-18 16:00 UTC (permalink / raw)
To: david.hunter.linux; +Cc: linux-kernel-mentees, skhan, Clint George
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);
Signed-off-by: Clint George <clintbgeorge@gmail.com>
---
drivers/char/hangcheck-timer.c | 25 +++++++++++++++----------
1 file changed, 15 insertions(+), 10 deletions(-)
diff --git a/drivers/char/hangcheck-timer.c b/drivers/char/hangcheck-timer.c
index 497fc167c..a95e6d608 100644
--- a/drivers/char/hangcheck-timer.c
+++ b/drivers/char/hangcheck-timer.c
@@ -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);
--
2.34.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] hangcheck-timer: use pr_crit and fix coding style
2025-10-18 16:00 [PATCH] hangcheck-timer: use pr_crit and fix coding style Clint George
@ 2025-10-22 12:24 ` David Hunter
0 siblings, 0 replies; 4+ messages in thread
From: David Hunter @ 2025-10-22 12:24 UTC (permalink / raw)
To: Clint George; +Cc: linux-kernel-mentees, skhan
On 10/18/25 12:00, Clint George wrote:
> 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);
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH] hangcheck-timer: use pr_crit and fix coding style
@ 2025-11-05 14:05 Clint George
2025-11-06 7:51 ` Greg KH
0 siblings, 1 reply; 4+ messages in thread
From: Clint George @ 2025-11-05 14:05 UTC (permalink / raw)
To: arnd, gregkh
Cc: linux-kernel-mentees, skhan, khalid, david.hunter.linux,
linux-kernel, Clint George
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.
Signed-off-by: Clint George <clintbgeorge@gmail.com>
---
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(-)
diff --git a/drivers/char/hangcheck-timer.c b/drivers/char/hangcheck-timer.c
index 497fc167c..a95e6d608 100644
--- a/drivers/char/hangcheck-timer.c
+++ b/drivers/char/hangcheck-timer.c
@@ -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);
--
2.34.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] hangcheck-timer: use pr_crit and fix coding style
2025-11-05 14:05 Clint George
@ 2025-11-06 7:51 ` Greg KH
0 siblings, 0 replies; 4+ messages in thread
From: Greg KH @ 2025-11-06 7:51 UTC (permalink / raw)
To: Clint George
Cc: arnd, linux-kernel-mentees, skhan, khalid, david.hunter.linux,
linux-kernel
On Wed, Nov 05, 2025 at 07:35:41PM +0530, Clint George wrote:
> 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.
Please wrap changelog comments at 72 columns.
Also, please split this up into "one patch per logical thing", as you
shouldn't be mixing them into one change.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2025-11-06 7:51 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-18 16:00 [PATCH] hangcheck-timer: use pr_crit and fix coding style Clint George
2025-10-22 12:24 ` David Hunter
-- strict thread matches above, loose matches on Subject: below --
2025-11-05 14:05 Clint George
2025-11-06 7:51 ` Greg KH
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox