irq: Remove unnecessary warning with affinity_hint

Hi,

If you turn off a PCI device whose driver has set affinity_hint, you will get warning message which does _not_ explain the reason why it appeared from the user's point of view.

  # echo 0 > /sys/bus/pci/slots/65/power

  Apr 28 20:29:39 localhost kernel: ------------[ cut here ]------------
  Apr 28 20:29:39 localhost kernel: WARNING: at kernel/irq/manage.c:1002 __free_irq+0x22d/0x250() (Tainted: P           ---------------   )
  (snip)

Users will misunderstand some problem has happened even though he or she succeeded to turn off the device. I suppose this warning was originally for a debug purpose for driver developers and has incidentally been left.

Just remove the warning is good and enough.

Signed-off-by Seiichi Ikarashi <[email protected]>

Quoted 24 lines · Seiichi Ikarashi

Hi,

If you turn off a PCI device whose driver has set affinity_hint, you will get warning message which does _not_ explain the reason why it appeared from the user's point of view.

  # echo 0 > /sys/bus/pci/slots/65/power

  Apr 28 20:29:39 localhost kernel: ------------[ cut here ]------------
  Apr 28 20:29:39 localhost kernel: WARNING: at kernel/irq/manage.c:1002 __free_irq+0x22d/0x250() (Tainted: P           ---------------   )
  (snip)

Users will misunderstand some problem has happened even though he or she succeeded to turn off the device. I suppose this warning was originally for a debug purpose for driver developers and has incidentally been left.

Just remove the warning is good and enough.

Signed-off-by Seiichi Ikarashi <[email protected]>

kernel/irq/manage.c static struct irqaction *__free_irq(unsi · line 1339

Quoted 6 lines

kernel/irq/manage.c +1 -1

@@ -1335,7 +1335,7 @@ static struct irqaction *__free_irq(unsi
 
 #ifdef CONFIG_SMP
 	/* make sure affinity_hint is cleaned up */
-	if (WARN_ON_ONCE(desc->affinity_hint))
+	if (desc->affinity_hint)
 		desc->affinity_hint = NULL;

Well, drivers that are using irq_set_affinity_hint() are expected to call:

	irq_set_affinity_hint(irq, NULL);

to clear the affinity mask, before releasing the irq. This warning flags drivers that forgot to do that and which might thus leak a dynamically allocated CPU mask (and/or other resources).

Feel free to turn the warning message into a more informative WARN() that will blame the driver that triggered it, if the stack dump into the driver wasn't a clue enough ...

Hi,

Quoted 39 lines · Ingo Molnar

Quoted 24 lines · Seiichi Ikarashi

Hi,

If you turn off a PCI device whose driver has set affinity_hint, you will get warning message which does _not_ explain the reason why it appeared from the user's point of view.

  # echo 0 > /sys/bus/pci/slots/65/power

  Apr 28 20:29:39 localhost kernel: ------------[ cut here ]------------
  Apr 28 20:29:39 localhost kernel: WARNING: at kernel/irq/manage.c:1002 __free_irq+0x22d/0x250() (Tainted: P           ---------------   )
  (snip)

Users will misunderstand some problem has happened even though he or she succeeded to turn off the device. I suppose this warning was originally for a debug purpose for driver developers and has incidentally been left.

Just remove the warning is good and enough.

Signed-off-by Seiichi Ikarashi <[email protected]>

kernel/irq/manage.c static struct irqaction *__free_irq(unsi · line 1339

Quoted 6 lines

kernel/irq/manage.c +1 -1

@@ -1335,7 +1335,7 @@ static struct irqaction *__free_irq(unsi
 
 #ifdef CONFIG_SMP
 	/* make sure affinity_hint is cleaned up */
-	if (WARN_ON_ONCE(desc->affinity_hint))
+	if (desc->affinity_hint)
 		desc->affinity_hint = NULL;

Well, drivers that are using irq_set_affinity_hint() are expected to call:

	irq_set_affinity_hint(irq, NULL);

Quoted 3 lines

to clear the affinity mask, before releasing the irq. This warning flags drivers that forgot to do that and which might thus leak a dynamically allocated CPU mask (and/or other resources).

Calling irq_set_affinity_hint(irq, NULL) does not guarantee that the driver does not forget to deallocate a dynamically allocated CPU mask and/or other resources. [...]

[...] But if calling it with NULL 2nd-arg before releasing the irq is a virtual rule of using irq_set_affinity_hint() interface, I understand it.

Quoted 3 lines

Feel free to turn the warning message into a more informative WARN() that will blame the driver that triggered it, if the stack dump into the driver wasn't a clue enough ...

Still, I do not know leaving the warning message is effective to prevent drivers from potentially leaking resource... considering a kind of cost-effectivenss. Business users (not developers) hate such kind of messages for developers.

Quoted 46 lines · Seiichi Ikarashi

Hi,

Quoted 42 lines · Ingo Molnar

Quoted 24 lines · Seiichi Ikarashi

Hi,

If you turn off a PCI device whose driver has set affinity_hint, you will get warning message which does _not_ explain the reason why it appeared from the user's point of view.

  # echo 0 > /sys/bus/pci/slots/65/power

  Apr 28 20:29:39 localhost kernel: ------------[ cut here ]------------
  Apr 28 20:29:39 localhost kernel: WARNING: at kernel/irq/manage.c:1002 __free_irq+0x22d/0x250() (Tainted: P           ---------------   )
  (snip)

Users will misunderstand some problem has happened even though he or she succeeded to turn off the device. I suppose this warning was originally for a debug purpose for driver developers and has incidentally been left.

Just remove the warning is good and enough.

Signed-off-by Seiichi Ikarashi <[email protected]>

kernel/irq/manage.c static struct irqaction *__free_irq(unsi · line 1339

Quoted 6 lines

kernel/irq/manage.c +1 -1

@@ -1335,7 +1335,7 @@ static struct irqaction *__free_irq(unsi
 
 #ifdef CONFIG_SMP
 	/* make sure affinity_hint is cleaned up */
-	if (WARN_ON_ONCE(desc->affinity_hint))
+	if (desc->affinity_hint)
 		desc->affinity_hint = NULL;

Well, drivers that are using irq_set_affinity_hint() are expected to call:

	irq_set_affinity_hint(irq, NULL);

to clear the affinity mask, before releasing the irq. This warning flags drivers that forgot to do that and which might thus leak a dynamically allocated CPU mask (and/or other resources).

Quoted 3 lines

Calling irq_set_affinity_hint(irq, NULL) does not guarantee that the driver does not forget to deallocate a dynamically allocated CPU mask and/or other resources. [...]

I said 'might leak', not 'guaranteed to leak'.

Calling irq_set_affinity_hint(irq, NULL) is the way this kernel API is specified to be used. Forgetting to do it is a kernel driver bug and triggers a warning message from the kernel's IRQ subsystem.

Quoted 4 lines

[...] But if calling it with NULL 2nd-arg before releasing the irq is a virtual rule of using irq_set_affinity_hint() interface, I understand it.

Quoted 8 lines

Quoted 3 lines

Feel free to turn the warning message into a more informative WARN() that will blame the driver that triggered it, if the stack dump into the driver wasn't a clue enough ...

Still, I do not know leaving the warning message is effective to prevent drivers from potentially leaking resource... considering a kind of cost-effectivenss. Business users (not developers) hate such kind of messages for developers.

it's a warning message pointing out a kernel bug: that irq_set_affinity_hint(irq, NULL) was not called properly.

Messages pointing out kernel bugs should be fixed, not hidden.