| Message ID | 20241128201027.10396-3-farbere@amazon.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Improve interrupt handling during machine kexec | expand |
Hi Eliav,
kernel test robot noticed the following build errors:
[auto build test ERROR on powerpc/next]
[also build test ERROR on powerpc/fixes tip/irq/core arm64/for-next/core linus/master v6.12 next-20241128]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Eliav-Farber/kexec-Consolidate-machine_kexec_mask_interrupts-implementation/20241129-041259
base: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
patch link: https://lore.kernel.org/r/20241128201027.10396-3-farbere%40amazon.com
patch subject: [PATCH v3 2/2] kexec: Prevent redundant IRQ masking by checking state before shutdown
config: x86_64-rhel-9.4 (https://download.01.org/0day-ci/archive/20241129/202411291251.RwA1dKZL-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241129/202411291251.RwA1dKZL-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202411291251.RwA1dKZL-lkp@intel.com/
All errors (new ones prefixed by >>):
kernel/kexec_core.c: In function 'machine_kexec_mask_interrupts':
kernel/kexec_core.c:1085:24: error: implicit declaration of function 'irq_desc_get_chip' [-Werror=implicit-function-declaration]
1085 | chip = irq_desc_get_chip(desc);
| ^~~~~~~~~~~~~~~~~
kernel/kexec_core.c:1085:22: warning: assignment to 'struct irq_chip *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
1085 | chip = irq_desc_get_chip(desc);
| ^
>> kernel/kexec_core.c:1086:31: error: implicit declaration of function 'irqd_is_started' [-Werror=implicit-function-declaration]
1086 | if (!chip || !irqd_is_started(&desc->irq_data))
| ^~~~~~~~~~~~~~~
kernel/kexec_core.c:1086:52: error: invalid use of undefined type 'struct irq_desc'
1086 | if (!chip || !irqd_is_started(&desc->irq_data))
| ^~
kernel/kexec_core.c:1097:38: error: invalid use of undefined type 'struct irq_chip'
1097 | if (check_eoi && chip->irq_eoi && irqd_irq_inprogress(&desc->irq_data))
| ^~
kernel/kexec_core.c:1097:51: error: implicit declaration of function 'irqd_irq_inprogress' [-Werror=implicit-function-declaration]
1097 | if (check_eoi && chip->irq_eoi && irqd_irq_inprogress(&desc->irq_data))
| ^~~~~~~~~~~~~~~~~~~
kernel/kexec_core.c:1097:76: error: invalid use of undefined type 'struct irq_desc'
1097 | if (check_eoi && chip->irq_eoi && irqd_irq_inprogress(&desc->irq_data))
| ^~
kernel/kexec_core.c:1098:29: error: invalid use of undefined type 'struct irq_chip'
1098 | chip->irq_eoi(&desc->irq_data);
| ^~
kernel/kexec_core.c:1098:44: error: invalid use of undefined type 'struct irq_desc'
1098 | chip->irq_eoi(&desc->irq_data);
| ^~
>> kernel/kexec_core.c:1100:17: error: implicit declaration of function 'irq_shutdown'; did you mean 'timer_shutdown'? [-Werror=implicit-function-declaration]
1100 | irq_shutdown(desc);
| ^~~~~~~~~~~~
| timer_shutdown
cc1: some warnings being treated as errors
vim +/irqd_is_started +1086 kernel/kexec_core.c
1075
1076 void machine_kexec_mask_interrupts(void)
1077 {
1078 unsigned int i;
1079 struct irq_desc *desc;
1080
1081 for_each_irq_desc(i, desc) {
1082 struct irq_chip *chip;
1083 int check_eoi = 1;
1084
1085 chip = irq_desc_get_chip(desc);
> 1086 if (!chip || !irqd_is_started(&desc->irq_data))
1087 continue;
1088
1089 if (IS_ENABLED(CONFIG_ARM64)) {
1090 /*
1091 * First try to remove the active state. If this fails, try to EOI the
1092 * interrupt.
1093 */
1094 check_eoi = irq_set_irqchip_state(i, IRQCHIP_STATE_ACTIVE, false);
1095 }
1096
1097 if (check_eoi && chip->irq_eoi && irqd_irq_inprogress(&desc->irq_data))
1098 chip->irq_eoi(&desc->irq_data);
1099
> 1100 irq_shutdown(desc);
diff --git a/include/linux/irq.h b/include/linux/irq.h index fa711f80957b..48a3df728c47 100644 --- a/include/linux/irq.h +++ b/include/linux/irq.h @@ -694,6 +694,9 @@ extern int irq_chip_request_resources_parent(struct irq_data *data); extern void irq_chip_release_resources_parent(struct irq_data *data); #endif +/* Shut down the interrupt */ +extern void irq_shutdown(struct irq_desc *desc); + /* Handling of unhandled and spurious interrupts: */ extern void note_interrupt(struct irq_desc *desc, irqreturn_t action_ret); diff --git a/kernel/irq/internals.h b/kernel/irq/internals.h index fe0272cd84a5..1f9287b1ccb7 100644 --- a/kernel/irq/internals.h +++ b/kernel/irq/internals.h @@ -88,7 +88,6 @@ extern int irq_activate(struct irq_desc *desc); extern int irq_activate_and_startup(struct irq_desc *desc, bool resend); extern int irq_startup(struct irq_desc *desc, bool resend, bool force); -extern void irq_shutdown(struct irq_desc *desc); extern void irq_shutdown_and_deactivate(struct irq_desc *desc); extern void irq_enable(struct irq_desc *desc); extern void irq_disable(struct irq_desc *desc); diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c index 6e1e420946e0..928b4387502b 100644 --- a/kernel/kexec_core.c +++ b/kernel/kexec_core.c @@ -1083,7 +1083,7 @@ void machine_kexec_mask_interrupts(void) int check_eoi = 1; chip = irq_desc_get_chip(desc); - if (!chip) + if (!chip || !irqd_is_started(&desc->irq_data)) continue; if (IS_ENABLED(CONFIG_ARM64)) { @@ -1097,10 +1097,6 @@ void machine_kexec_mask_interrupts(void) if (check_eoi && chip->irq_eoi && irqd_irq_inprogress(&desc->irq_data)) chip->irq_eoi(&desc->irq_data); - if (chip->irq_mask) - chip->irq_mask(&desc->irq_data); - - if (chip->irq_disable && !irqd_irq_disabled(&desc->irq_data)) - chip->irq_disable(&desc->irq_data); + irq_shutdown(desc); } }
During machine kexec, the function machine_kexec_mask_interrupts() is responsible for disabling or masking all interrupts. While the irq_disable hook ensures that an already-disabled IRQ is not disabled again, the current implementation unconditionally invokes the irq_mask() function for every interrupt descriptor, even when the interrupt is already masked. A specific issue was observed in the crash kernel flow after unbinding a device (prior to kexec) that used a GPIO as an IRQ source. The warning was triggered by the gpiochip_disable_irq() function, which attempted to clear the FLAG_IRQ_IS_ENABLED flag when FLAG_USED_AS_IRQ was not set: ``` void gpiochip_disable_irq(struct gpio_chip *gc, unsigned int offset) { struct gpio_desc *desc = gpiochip_get_desc(gc, offset); if (!IS_ERR(desc) && !WARN_ON(!test_bit(FLAG_USED_AS_IRQ, &desc->flags))) clear_bit(FLAG_IRQ_IS_ENABLED, &desc->flags); } ``` This issue surfaced after commit a8173820f441 ("gpio: gpiolib: Allow GPIO IRQs to lazy disable") introduced lazy disablement for GPIO IRQs. It replaced disable/enable hooks with mask/unmask hooks. Unlike the disable hook, the mask hook doesn't handle already-masked IRQs. When a GPIO-IRQ driver is unbound, the IRQ is released, triggering __irq_disable() and irq_state_set_masked(). A subsequent call to machine_kexec_mask_interrupts() re-invokes chip->irq_mask(). This results in a call chain, including gpiochip_irq_mask() and gpiochip_disable_irq(). Since FLAG_USED_AS_IRQ was cleared earlier, a warning occurs. This patch addresses the issue by: - Replacing the calls to irq_mask() and irq_disable() hooks with a simplified call to irq_shutdown(). - Checking if the interrupt is started (irqd_is_started) before calling the shutdown. As part of this change, the irq_shutdown() declaration was moved from kernel/irq/internals.h to include/linux/irq.h to make it accessible outside the kernel/irq/ directory, as the former can only be included within that directory. Signed-off-by: Eliav Farber <farbere@amazon.com> --- V2 -> V3: - Check if IRQ is started using irqd_is_started(). - Use irq_shutdown() instead of irq_disable(). include/linux/irq.h | 3 +++ kernel/irq/internals.h | 1 - kernel/kexec_core.c | 8 ++------ 3 files changed, 5 insertions(+), 7 deletions(-)