* [PATCH v2] x86/i8253: fix possible deadlock when turning off the PIT
@ 2025-03-27 15:22 Fernando Fernandez Mancera
2025-03-27 16:43 ` David Woodhouse
2025-03-27 17:15 ` Thomas Gleixner
0 siblings, 2 replies; 9+ messages in thread
From: Fernando Fernandez Mancera @ 2025-03-27 15:22 UTC (permalink / raw)
To: x86, tglx, linux-kernel; +Cc: dwmw, mhkelley, mingo, Fernando Fernandez Mancera
As the PIT could be disabled during the init, it can possibly cause a
deadlock. hpet_time_init()->pit_timer_init() is called without IRQ off.
It assumes that clockevent_i8253_disable() is IRQ-safe, which it isn't.
Disable interrupt around clockevent_i8253_disable() call in
pit_timer_init().
[ 45.408952] =====================================================
[ 45.408970] WARNING: HARDIRQ-safe -> HARDIRQ-unsafe lock order detected
[ 45.408974] 6.14.0-rc7+ #6 Not tainted
[ 45.408978] -----------------------------------------------------
[ 45.408980] systemd-sleep/3324 [HC0[0]:SC0[0]:HE0:SE1] is trying to acquire:
[ 45.408986] ffffffffb2c23398 (i8253_lock){+.+.}-{2:2}, at: pcspkr_event+0x3f/0xe0 [pcspkr]
[ 45.409004]
and this task is already holding:
[ 45.409006] ffff9c334d7c2230 (&dev->event_lock){-.-.}-{3:3}, at: input_dev_resume+0x21/0x50
[ 45.409023] which would create a new lock dependency:
[ 45.409025] (&dev->event_lock){-.-.}-{3:3} -> (i8253_lock){+.+.}-{2:2}
[ 45.409043]
but this new dependency connects a HARDIRQ-irq-safe lock:
[ 45.409045] (&dev->event_lock){-.-.}-{3:3}
[ 45.409052]
... which became HARDIRQ-irq-safe at:
[ 45.409055] lock_acquire+0xd0/0x2f0
[ 45.409062] _raw_spin_lock_irqsave+0x48/0x70
[ 45.409067] input_event+0x3c/0x80
[ 45.409071] atkbd_receive_byte+0x9b/0x6e0
[ 45.409077] ps2_interrupt+0xb2/0x1d0
[ 45.409082] serio_interrupt+0x4a/0x90
[ 45.409087] i8042_handle_data+0xf8/0x280
[ 45.409091] i8042_interrupt+0x11/0x40
[ 45.409095] __handle_irq_event_percpu+0x87/0x260
[ 45.409100] handle_irq_event+0x38/0x90
[ 45.409105] handle_edge_irq+0x8b/0x230
[ 45.409109] __common_interrupt+0x5c/0x120
[ 45.409114] common_interrupt+0x80/0xa0
[ 45.409120] asm_common_interrupt+0x26/0x40
[ 45.409125] pv_native_safe_halt+0xf/0x20
[ 45.409130] default_idle+0x9/0x20
[ 45.409135] default_idle_call+0x7a/0x1d0
[ 45.409140] do_idle+0x215/0x260
[ 45.409144] cpu_startup_entry+0x29/0x30
[ 45.409149] start_secondary+0x132/0x170
[ 45.409153] common_startup_64+0x13e/0x141
[ 45.409158]
to a HARDIRQ-irq-unsafe lock:
[ 45.409161] (i8253_lock){+.+.}-{2:2}
[ 45.409167]
... which became HARDIRQ-irq-unsafe at:
[ 45.409170] ...
[ 45.409172] lock_acquire+0xd0/0x2f0
[ 45.409177] _raw_spin_lock+0x30/0x40
[ 45.409181] clockevent_i8253_disable+0x1c/0x60
[ 45.409186] pit_timer_init+0x25/0x50
[ 45.409191] hpet_time_init+0x46/0x50
[ 45.409196] x86_late_time_init+0x1b/0x40
[ 45.409201] start_kernel+0x962/0xa00
[ 45.409206] x86_64_start_reservations+0x24/0x30
[ 45.409211] x86_64_start_kernel+0xed/0xf0
[ 45.409215] common_startup_64+0x13e/0x141
[ 45.409220]
other info that might help us debug this:
[ 45.409222] Possible interrupt unsafe locking scenario:
[ 45.409224] CPU0 CPU1
[ 45.409226] ---- ----
[ 45.409228] lock(i8253_lock);
[ 45.409234] local_irq_disable();
[ 45.409237] lock(&dev->event_lock);
[ 45.409243] lock(i8253_lock);
[ 45.409249] <Interrupt>
[ 45.409251] lock(&dev->event_lock);
[ 45.409257]
*** DEADLOCK ***
Fixes: 70e6b7d9ae3c ("x86/i8253: Disable PIT timer 0 when not in use")
Signed-off-by: Fernando Fernandez Mancera <ffmancera@riseup.net>
---
v2: disable local IRQ in pit_timer_init()
---
arch/x86/kernel/i8253.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/x86/kernel/i8253.c b/arch/x86/kernel/i8253.c
index 80e262bb627f..af58a262c81b 100644
--- a/arch/x86/kernel/i8253.c
+++ b/arch/x86/kernel/i8253.c
@@ -40,13 +40,17 @@ static bool __init use_pit(void)
bool __init pit_timer_init(void)
{
+ unsigned long flags;
+
if (!use_pit()) {
/*
* Don't just ignore the PIT. Ensure it's stopped, because
* VMMs otherwise steal CPU time just to pointlessly waggle
* the (masked) IRQ.
*/
+ local_irq_save(flags);
clockevent_i8253_disable();
+ local_irq_restore(flags);
return false;
}
clockevent_i8253_init(true);
--
2.47.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2] x86/i8253: fix possible deadlock when turning off the PIT
2025-03-27 15:22 [PATCH v2] x86/i8253: fix possible deadlock when turning off the PIT Fernando Fernandez Mancera
@ 2025-03-27 16:43 ` David Woodhouse
2025-03-27 17:15 ` Thomas Gleixner
1 sibling, 0 replies; 9+ messages in thread
From: David Woodhouse @ 2025-03-27 16:43 UTC (permalink / raw)
To: Fernando Fernandez Mancera, x86, tglx, linux-kernel; +Cc: mhkelley, mingo
[-- Attachment #1: Type: text/plain, Size: 4370 bytes --]
On Thu, 2025-03-27 at 16:22 +0100, Fernando Fernandez Mancera wrote:
> As the PIT could be disabled during the init, it can possibly cause a
> deadlock. hpet_time_init()->pit_timer_init() is called without IRQ off.
> It assumes that clockevent_i8253_disable() is IRQ-safe, which it isn't.
>
> Disable interrupt around clockevent_i8253_disable() call in
> pit_timer_init().
>
> [ 45.408952] =====================================================
> [ 45.408970] WARNING: HARDIRQ-safe -> HARDIRQ-unsafe lock order detected
> [ 45.408974] 6.14.0-rc7+ #6 Not tainted
> [ 45.408978] -----------------------------------------------------
> [ 45.408980] systemd-sleep/3324 [HC0[0]:SC0[0]:HE0:SE1] is trying to acquire:
> [ 45.408986] ffffffffb2c23398 (i8253_lock){+.+.}-{2:2}, at: pcspkr_event+0x3f/0xe0 [pcspkr]
> [ 45.409004]
> and this task is already holding:
> [ 45.409006] ffff9c334d7c2230 (&dev->event_lock){-.-.}-{3:3}, at: input_dev_resume+0x21/0x50
> [ 45.409023] which would create a new lock dependency:
> [ 45.409025] (&dev->event_lock){-.-.}-{3:3} -> (i8253_lock){+.+.}-{2:2}
> [ 45.409043]
> but this new dependency connects a HARDIRQ-irq-safe lock:
> [ 45.409045] (&dev->event_lock){-.-.}-{3:3}
> [ 45.409052]
> ... which became HARDIRQ-irq-safe at:
> [ 45.409055] lock_acquire+0xd0/0x2f0
> [ 45.409062] _raw_spin_lock_irqsave+0x48/0x70
> [ 45.409067] input_event+0x3c/0x80
> [ 45.409071] atkbd_receive_byte+0x9b/0x6e0
> [ 45.409077] ps2_interrupt+0xb2/0x1d0
> [ 45.409082] serio_interrupt+0x4a/0x90
> [ 45.409087] i8042_handle_data+0xf8/0x280
> [ 45.409091] i8042_interrupt+0x11/0x40
> [ 45.409095] __handle_irq_event_percpu+0x87/0x260
> [ 45.409100] handle_irq_event+0x38/0x90
> [ 45.409105] handle_edge_irq+0x8b/0x230
> [ 45.409109] __common_interrupt+0x5c/0x120
> [ 45.409114] common_interrupt+0x80/0xa0
> [ 45.409120] asm_common_interrupt+0x26/0x40
> [ 45.409125] pv_native_safe_halt+0xf/0x20
> [ 45.409130] default_idle+0x9/0x20
> [ 45.409135] default_idle_call+0x7a/0x1d0
> [ 45.409140] do_idle+0x215/0x260
> [ 45.409144] cpu_startup_entry+0x29/0x30
> [ 45.409149] start_secondary+0x132/0x170
> [ 45.409153] common_startup_64+0x13e/0x141
> [ 45.409158]
> to a HARDIRQ-irq-unsafe lock:
> [ 45.409161] (i8253_lock){+.+.}-{2:2}
> [ 45.409167]
> ... which became HARDIRQ-irq-unsafe at:
> [ 45.409170] ...
> [ 45.409172] lock_acquire+0xd0/0x2f0
> [ 45.409177] _raw_spin_lock+0x30/0x40
> [ 45.409181] clockevent_i8253_disable+0x1c/0x60
> [ 45.409186] pit_timer_init+0x25/0x50
> [ 45.409191] hpet_time_init+0x46/0x50
> [ 45.409196] x86_late_time_init+0x1b/0x40
> [ 45.409201] start_kernel+0x962/0xa00
> [ 45.409206] x86_64_start_reservations+0x24/0x30
> [ 45.409211] x86_64_start_kernel+0xed/0xf0
> [ 45.409215] common_startup_64+0x13e/0x141
> [ 45.409220]
> other info that might help us debug this:
>
> [ 45.409222] Possible interrupt unsafe locking scenario:
>
> [ 45.409224] CPU0 CPU1
> [ 45.409226] ---- ----
> [ 45.409228] lock(i8253_lock);
> [ 45.409234] local_irq_disable();
> [ 45.409237] lock(&dev->event_lock);
> [ 45.409243] lock(i8253_lock);
> [ 45.409249] <Interrupt>
> [ 45.409251] lock(&dev->event_lock);
> [ 45.409257]
> *** DEADLOCK ***
>
> Fixes: 70e6b7d9ae3c ("x86/i8253: Disable PIT timer 0 when not in use")
> Signed-off-by: Fernando Fernandez Mancera <ffmancera@riseup.net>
> ---
> v2: disable local IRQ in pit_timer_init()
Thanks.
Reviewed-by: David Woodhouse <dwmw2@infradead.org>
Worth a WARN_ON(!irqs_disabled()) in clockevent_i8253_disable() too?
[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5069 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] x86/i8253: fix possible deadlock when turning off the PIT
2025-03-27 15:22 [PATCH v2] x86/i8253: fix possible deadlock when turning off the PIT Fernando Fernandez Mancera
2025-03-27 16:43 ` David Woodhouse
@ 2025-03-27 17:15 ` Thomas Gleixner
2025-03-27 19:54 ` Fernando Fernandez Mancera
1 sibling, 1 reply; 9+ messages in thread
From: Thomas Gleixner @ 2025-03-27 17:15 UTC (permalink / raw)
To: Fernando Fernandez Mancera, x86, linux-kernel
Cc: dwmw, mhkelley, mingo, Fernando Fernandez Mancera
On Thu, Mar 27 2025 at 16:22, Fernando Fernandez Mancera wrote:
> As the PIT could be disabled during the init, it can possibly cause a
> deadlock. hpet_time_init()->pit_timer_init() is called without IRQ off.
> It assumes that clockevent_i8253_disable() is IRQ-safe, which it isn't.
It assumes nothing and all the missing interrupt disable is causing is a
lockdep false positive.
Lockdep complains correctly due to the observed contexts, but in reality
there is no possible deadlock at all. Definitely not the one your
subject line is claiming to be possible.
At the point where pit_timer_init() is invoked there is no other usage
of 8253_lock possible because the system is still in the very early boot
stage.
So disabling interrupt here just prevents lockdep triggering a false
positive and not more.
Please analyze problems properly instead of assuming that the lockdep
splat is the ultimate truth.
> bool __init pit_timer_init(void)
> {
> + unsigned long flags;
> +
> if (!use_pit()) {
> /*
> * Don't just ignore the PIT. Ensure it's stopped, because
> * VMMs otherwise steal CPU time just to pointlessly waggle
> * the (masked) IRQ.
> */
> + local_irq_save(flags);
Why save()? You just established that interrupts are enabled here, so
this really wants to be:
scoped_guard(irq)()
clockevent_i8253_disable();
return false;
Thanks,
tglx
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] x86/i8253: fix possible deadlock when turning off the PIT
2025-03-27 17:15 ` Thomas Gleixner
@ 2025-03-27 19:54 ` Fernando Fernandez Mancera
2025-03-27 21:17 ` Thomas Gleixner
0 siblings, 1 reply; 9+ messages in thread
From: Fernando Fernandez Mancera @ 2025-03-27 19:54 UTC (permalink / raw)
To: Thomas Gleixner, x86, linux-kernel; +Cc: dwmw, mhkelley, mingo
On 3/27/25 6:15 PM, Thomas Gleixner wrote:
> On Thu, Mar 27 2025 at 16:22, Fernando Fernandez Mancera wrote:
>> As the PIT could be disabled during the init, it can possibly cause a
>> deadlock. hpet_time_init()->pit_timer_init() is called without IRQ off.
>> It assumes that clockevent_i8253_disable() is IRQ-safe, which it isn't.
>
> It assumes nothing and all the missing interrupt disable is causing is a
> lockdep false positive.
>
> Lockdep complains correctly due to the observed contexts, but in reality
> there is no possible deadlock at all. Definitely not the one your
> subject line is claiming to be possible.
>
> At the point where pit_timer_init() is invoked there is no other usage
> of 8253_lock possible because the system is still in the very early boot
> stage.
>
Thanks for taking the time to review the patch.
I was not aware of this. I took a look to other functions that used
i8253_lock like pcspkr_event() and thought it could be possible.
> So disabling interrupt here just prevents lockdep triggering a false
> positive and not more.
>
> Please analyze problems properly instead of assuming that the lockdep
> splat is the ultimate truth.
I tried, but it seems I failed. I just found out this while working on
something else not related in a VM.
>
>> bool __init pit_timer_init(void)
>> {
>> + unsigned long flags;
>> +
>> if (!use_pit()) {
>> /*
>> * Don't just ignore the PIT. Ensure it's stopped, because
>> * VMMs otherwise steal CPU time just to pointlessly waggle
>> * the (masked) IRQ.
>> */
>> + local_irq_save(flags);
>
> Why save()? You just established that interrupts are enabled here, so
> this really wants to be:
>
I followed Ingo's suggestions on V1 [1]. It made sense to me, if the
problem was the one described on the commit message. So, is there
consensus about this being a false positive? If so, I will send a new
patch just suppressing the warning as suggested below.
[1] https://lore.kernel.org/linux-kernel/Z-B6ob0zLZr81e8i@gmail.com/
> scoped_guard(irq)()
> clockevent_i8253_disable();
> return false;
>
> Thanks,
>
> tglx
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] x86/i8253: fix possible deadlock when turning off the PIT
2025-03-27 19:54 ` Fernando Fernandez Mancera
@ 2025-03-27 21:17 ` Thomas Gleixner
2025-03-27 21:27 ` Ingo Molnar
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Thomas Gleixner @ 2025-03-27 21:17 UTC (permalink / raw)
To: Fernando Fernandez Mancera, x86, linux-kernel; +Cc: dwmw, mhkelley, mingo
On Thu, Mar 27 2025 at 20:54, Fernando Fernandez Mancera wrote:
> On 3/27/25 6:15 PM, Thomas Gleixner wrote:
> I followed Ingo's suggestions on V1 [1]. It made sense to me, if the
> problem was the one described on the commit message. So, is there
> consensus about this being a false positive? If so, I will send a new
> patch just suppressing the warning as suggested below.
I personally don't care whether there is consensus simply because it's a
matter of fact, that at the point where pit_timer_init() is invoked there
can't be concurrency on the lock by any means. Therefore it _is_ a false
positive.
Ingo is right that pit_timer_init() should disable interrupts before
invoking clockevent_i8253_disable() and not inflicting the irqsave() on
the callback function.
But it should do so for the sake of consistency and correctness and not
to "fix" a impossible deadlock or an magically assumed invalid assumption.
The assumption,
- assumed that the author of the offending commit made
any assumptions at all (pun intended) -
that invoking clockevent_i8253_disable() with interrupts enabled at this
point in the boot process is harmless, is completely correct.
Therefore I really prefer to have this described as:
x86/i8253: Invoke clockevent_i8253_disable() with interrupts disabled
with a proper explanation that the current code makes lockdep
(rightfully) complain, but that it has no actual deadlock potential in
the current state of the code.
That means the code change serves two purposes:
1) Prevent lockdep from detecting a false positive
2) Future proving the code
#1 is a matter of fact with the current code
#2 is valuable despite the fact that PIT is a legacy, which won't
suddenly roar its ugly head in unexpected ways.
I know that's word smithing, but I'm observing a increasing tendency of
"fixing" problems based on tooling output without any further analysis.
I'm absolutely not blaming you for that and your patch is fine, except
for the technical details I pointed out and the change log related
issues.
Though I really want people to sit down and think about the factual
impact of a tool based problem observation. Tools are good in detecting
problems, but they are patently bad in properly analysing them. And no,
AI is not going to fix that anytime soon, quite the contrary.
Taking the tools output at face value leads exactly to what triggered my
response:
"fix possible deadlock when turning off the PIT"
which is misleading at best as I explained before.
Wording matters, but maybe that's just me...
Thanks,
tglx
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] x86/i8253: fix possible deadlock when turning off the PIT
2025-03-27 21:17 ` Thomas Gleixner
@ 2025-03-27 21:27 ` Ingo Molnar
2025-03-27 21:36 ` Fernando F. Mancera
2025-03-27 22:52 ` Thomas Gleixner
2 siblings, 0 replies; 9+ messages in thread
From: Ingo Molnar @ 2025-03-27 21:27 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Fernando Fernandez Mancera, x86, linux-kernel, dwmw, mhkelley
* Thomas Gleixner <tglx@linutronix.de> wrote:
> On Thu, Mar 27 2025 at 20:54, Fernando Fernandez Mancera wrote:
> > On 3/27/25 6:15 PM, Thomas Gleixner wrote:
> > I followed Ingo's suggestions on V1 [1]. It made sense to me, if the
> > problem was the one described on the commit message. So, is there
> > consensus about this being a false positive? If so, I will send a new
> > patch just suppressing the warning as suggested below.
>
> I personally don't care whether there is consensus simply because it's a
> matter of fact, that at the point where pit_timer_init() is invoked there
> can't be concurrency on the lock by any means. Therefore it _is_ a false
> positive.
>
> Ingo is right that pit_timer_init() should disable interrupts before
> invoking clockevent_i8253_disable() and not inflicting the irqsave() on
> the callback function.
>
> But it should do so for the sake of consistency and correctness and not
> to "fix" a impossible deadlock or an magically assumed invalid assumption.
>
> The assumption,
>
> - assumed that the author of the offending commit made
> any assumptions at all (pun intended) -
>
> that invoking clockevent_i8253_disable() with interrupts enabled at this
> point in the boot process is harmless, is completely correct.
>
> Therefore I really prefer to have this described as:
>
> x86/i8253: Invoke clockevent_i8253_disable() with interrupts disabled
>
> with a proper explanation that the current code makes lockdep
> (rightfully) complain, but that it has no actual deadlock potential in
> the current state of the code.
>
> That means the code change serves two purposes:
>
> 1) Prevent lockdep from detecting a false positive
>
> 2) Future proving the code
>
> #1 is a matter of fact with the current code
>
> #2 is valuable despite the fact that PIT is a legacy, which won't
> suddenly roar its ugly head in unexpected ways.
>
> I know that's word smithing, but I'm observing a increasing tendency of
> "fixing" problems based on tooling output without any further analysis.
>
> I'm absolutely not blaming you for that and your patch is fine, except
> for the technical details I pointed out and the change log related
> issues.
>
> Though I really want people to sit down and think about the factual
> impact of a tool based problem observation. Tools are good in detecting
> problems, but they are patently bad in properly analysing them. And no,
> AI is not going to fix that anytime soon, quite the contrary.
>
> Taking the tools output at face value leads exactly to what triggered my
> response:
>
> "fix possible deadlock when turning off the PIT"
>
> which is misleading at best as I explained before.
>
> Wording matters, but maybe that's just me...
I fully agree with all of your suggestions.
I suggested a save/restore cycle primarily because I wasn't 100%
certain that IRQs were always enabled in that call chain, and a
superfluous save is better than an unintended IRQ-enable. So it was a
chicken-bit. :-/
Your title suggestion is also much better, it makes it clear that this
is not a potential deadlock.
Wording matters.
Thanks,
Ingo
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] x86/i8253: fix possible deadlock when turning off the PIT
2025-03-27 21:17 ` Thomas Gleixner
2025-03-27 21:27 ` Ingo Molnar
@ 2025-03-27 21:36 ` Fernando F. Mancera
2025-03-27 22:52 ` Thomas Gleixner
2 siblings, 0 replies; 9+ messages in thread
From: Fernando F. Mancera @ 2025-03-27 21:36 UTC (permalink / raw)
To: Thomas Gleixner, x86, linux-kernel; +Cc: dwmw, mhkelley, mingo
On 27/03/2025 22:17, Thomas Gleixner wrote:
> On Thu, Mar 27 2025 at 20:54, Fernando Fernandez Mancera wrote:
>> On 3/27/25 6:15 PM, Thomas Gleixner wrote:
>> I followed Ingo's suggestions on V1 [1]. It made sense to me, if the
>> problem was the one described on the commit message. So, is there
>> consensus about this being a false positive? If so, I will send a new
>> patch just suppressing the warning as suggested below.
>
> I personally don't care whether there is consensus simply because it's a
> matter of fact, that at the point where pit_timer_init() is invoked there
> can't be concurrency on the lock by any means. Therefore it _is_ a false
> positive.
>
> Ingo is right that pit_timer_init() should disable interrupts before
> invoking clockevent_i8253_disable() and not inflicting the irqsave() on
> the callback function.
>
> But it should do so for the sake of consistency and correctness and not
> to "fix" a impossible deadlock or an magically assumed invalid assumption.
>
> The assumption,
>
> - assumed that the author of the offending commit made
> any assumptions at all (pun intended) -
>
> that invoking clockevent_i8253_disable() with interrupts enabled at this
> point in the boot process is harmless, is completely correct.
>
> Therefore I really prefer to have this described as:
>
> x86/i8253: Invoke clockevent_i8253_disable() with interrupts disabled
>
> with a proper explanation that the current code makes lockdep
> (rightfully) complain, but that it has no actual deadlock potential in
> the current state of the code.
>
> That means the code change serves two purposes:
>
> 1) Prevent lockdep from detecting a false positive
>
> 2) Future proving the code
>
> #1 is a matter of fact with the current code
>
> #2 is valuable despite the fact that PIT is a legacy, which won't
> suddenly roar its ugly head in unexpected ways.
>
> I know that's word smithing, but I'm observing a increasing tendency of
> "fixing" problems based on tooling output without any further analysis.
>
> I'm absolutely not blaming you for that and your patch is fine, except
> for the technical details I pointed out and the change log related
> issues.
>
Thanks for taking the time to write this extensive reply. I agree on the
arguments provided here.
> Though I really want people to sit down and think about the factual
> impact of a tool based problem observation. Tools are good in detecting
> problems, but they are patently bad in properly analysing them. And no,
> AI is not going to fix that anytime soon, quite the contrary.
>
I agree on this statement. I just wanted to remark I didn't know that
during the pit_timer_init() call can't be concurrency on the lock at
all. I do know now, tho. Thanks for explaining.
I will provide a new patch with your suggestion to use scoped_guardian()
and wording the commit message and description properly.
Thanks,
ffmancera.
> Taking the tools output at face value leads exactly to what triggered my
> response:
>
> "fix possible deadlock when turning off the PIT"
>
> which is misleading at best as I explained before.
>
> Wording matters, but maybe that's just me...
>
> Thanks,
>
> tglx
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] x86/i8253: fix possible deadlock when turning off the PIT
2025-03-27 21:17 ` Thomas Gleixner
2025-03-27 21:27 ` Ingo Molnar
2025-03-27 21:36 ` Fernando F. Mancera
@ 2025-03-27 22:52 ` Thomas Gleixner
2025-03-27 23:15 ` Fernando F. Mancera
2 siblings, 1 reply; 9+ messages in thread
From: Thomas Gleixner @ 2025-03-27 22:52 UTC (permalink / raw)
To: Fernando Fernandez Mancera, x86, linux-kernel; +Cc: dwmw, mhkelley, mingo
On Thu, Mar 27 2025 at 22:17, Thomas Gleixner wrote:
> Though I really want people to sit down and think about the factual
> impact of a tool based problem observation. Tools are good in detecting
> problems, but they are patently bad in properly analysing them. And no,
> AI is not going to fix that anytime soon, quite the contrary.
May I recommend you to ask your favorite AI model of the moment the
following question:
"Explain the discussion in the email thread starting at:
https://lore.kernel.org/all/20250327152258.3097-1-ffmancera@riseup.net"
I'm sure that I'm patently bad at AI prompt engineering, but that does
not justify the utter insanities which these models threw back at me.
ChapGPT:
"....
This patch adds a selftest (automated test) for the nftables
flowtable feature using a netdevice. It targets the netfilter
subsystem, which is responsible for packet filtering, NAT, and other
packet mangling in the Linux kernel."
Grok:
See
https://tglx.de/~tglx/grok.html
for the full glory of AI hallucinations.
At least those two were halfways reasonable:
Gemini:
"API REQUEST ERROR Reason: Unknown."
Claude:
"I don't have access to the specific URL you've provided ...
my knowledge cutoff was in October 2024 ..."
Seriously?
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] x86/i8253: fix possible deadlock when turning off the PIT
2025-03-27 22:52 ` Thomas Gleixner
@ 2025-03-27 23:15 ` Fernando F. Mancera
0 siblings, 0 replies; 9+ messages in thread
From: Fernando F. Mancera @ 2025-03-27 23:15 UTC (permalink / raw)
To: Thomas Gleixner, x86, linux-kernel; +Cc: dwmw, mhkelley, mingo
On 27/03/2025 23:52, Thomas Gleixner wrote:
> On Thu, Mar 27 2025 at 22:17, Thomas Gleixner wrote:
>> Though I really want people to sit down and think about the factual
>> impact of a tool based problem observation. Tools are good in detecting
>> problems, but they are patently bad in properly analysing them. And no,
>> AI is not going to fix that anytime soon, quite the contrary.
>
> May I recommend you to ask your favorite AI model of the moment the
> following question:
Just in case I gave the wrong impression, I do not like LLMs at all and
do not use them. They strip out your personality from emails/messages
and hallucinate a lot.
>
> "Explain the discussion in the email thread starting at:
> https://lore.kernel.org/all/20250327152258.3097-1-ffmancera@riseup.net"
>
> I'm sure that I'm patently bad at AI prompt engineering, but that does
> not justify the utter insanities which these models threw back at me.
>
> ChapGPT:
>
> "....
> This patch adds a selftest (automated test) for the nftables
> flowtable feature using a netdevice. It targets the netfilter
> subsystem, which is responsible for packet filtering, NAT, and other
> packet mangling in the Linux kernel."
>
Here it is hallucinating using content from a different patch I sent a
while ago. Adding some tests to nftables about flowtables. Probably it
just spotted my email and started to generate text from it.
> Grok:
>
> See
>
> https://tglx.de/~tglx/grok.html
>
> for the full glory of AI hallucinations.
>
Now, my name is Felipe. Great, good to know. Ah, and I also maintain a
network driver too, awesome.
> At least those two were halfways reasonable:
>
> Gemini:
>
> "API REQUEST ERROR Reason: Unknown."
>
> Claude:
>
> "I don't have access to the specific URL you've provided ...
> my knowledge cutoff was in October 2024 ..."
>
> Seriously?
>
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2025-03-27 23:15 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-27 15:22 [PATCH v2] x86/i8253: fix possible deadlock when turning off the PIT Fernando Fernandez Mancera
2025-03-27 16:43 ` David Woodhouse
2025-03-27 17:15 ` Thomas Gleixner
2025-03-27 19:54 ` Fernando Fernandez Mancera
2025-03-27 21:17 ` Thomas Gleixner
2025-03-27 21:27 ` Ingo Molnar
2025-03-27 21:36 ` Fernando F. Mancera
2025-03-27 22:52 ` Thomas Gleixner
2025-03-27 23:15 ` Fernando F. Mancera
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox