generic-ipi: Initialize call_single_queue before enabling interrupt
Subsystems (2)
PATCH v2 1/1 generic-ipi: Initialize call_single_queue before enabling interrupt
Hi all,
This patch renames init_call_single_data() to call_function_init() and calls it in start_kernel() so that call_single_queue can be initialized before enabling interrupt.
There is a problem that kdump(2nd kernel) sometimes hangs up due to pending IPI from 1st kernel. Kernel panic occurs because IPI comes before call_single_queue is initialized. The details are as follows.
(1) 2nd kernel boot up
(2) A pending IPI from 1st kernel comes when irqs are first enabled in start_kernel().
(3) Kernel tries to handle the interrupt, but call_single_queue is not initialized yet at this point. As a result, in the generic_smp_call_function_single_interrupt(), NULL pointer dereference occurs when list_replace_init() tries to access &q->list.next.
Therefore this patch changes the name of init_call_single_data() to call_function_init() and calls it before local_irq_enable() in start_kernel().
v2:
- Rename init_call_single_data() to call_function_init() and calls it in start_kernel()
- Change insert position in start_kernel().
- Adjust for CONFIG_SMP/CONFIG_USE_GENERIC_SMP_HELPERS options
- Rebased to Linus's latest tree
v1:
https://lkml.org/lkml/2011/3/25/317
- Divide init_call_single_data() into two functions,
o init_call_single_data: initialize call_single_queue
o init_hotplug_cfd: initialize hotplug_cfd_notifier
And call init_call_single_data before local_irq_enable() in start_kernel().
v0:
https://lkml.org/lkml/2011/3/23/417
- In generic_smp_call_function_single_interrupt(), check if call_single_queue was initialized or not, and just return if not initialized.
--- include/linux/smp.h | 5 ++++- init/main.c | 1 + kernel/smp.c | 5 +---- 3 files changed, 6 insertions(+), 5 deletions(-)
include/linux/smp.h +4 -1
@@ -85,12 +85,15 @@ int smp_call_function_any(const struct cpumask *mask,* Generic and arch helpers */ #ifdef CONFIG_USE_GENERIC_SMP_HELPERS +void __init call_function_init (void );void generic_smp_call_function_single_interrupt (void );void generic_smp_call_function_interrupt (void );void ipi_call_lock (void );void ipi_call_unlock (void );void ipi_call_lock_irq (void );void ipi_call_unlock_irq (void ); +#else +static inline void call_function_init (void ) { }#endif /*
@@ -144,7 +147,7 @@ static inline void smp_send_reschedule(int cpu) { }#define smp_prepare_boot_cpu () do {} while (0)#define smp_call_function_many (mask, func, info, wait) \ (up_smp_call_function(func, info)) -static inline void init_call_single_data (void ) { } +static inline void call_function_init (void ) { } static inline int smp_call_function_any(const struct cpumask *mask, smp_call_func_t func,
init/main.c +1 -0
@@ -539,6 +539,7 @@ asmlinkage void __init start_kernel(void)timekeeping_init ();time_init ();profile_init (); +call_function_init ();if (!irqs_disabled ())printk (KERN_CRIT "start_kernel(): bug: interrupts were " "enabled early\n" );
kernel/smp.c +1 -4
@@ -74,7 +74,7 @@ static struct notifier_block __cpuinitdata hotplug_cfd_notifier = { .notifier_call = hotplug_cfd, }; -static int __cpuinit init_call_single_data (void ) +void __init call_function_init (void ) {void *cpu = (void *)(long )smp_processor_id ();int i;
@@ -88,10 +88,7 @@ static int __cpuinit init_call_single_data(void)hotplug_cfd (&hotplug_cfd_notifier,CPU_UP_PREPARE , cpu);register_cpu_notifier (&hotplug_cfd_notifier); - -return 0 ; } -early_initcall (init_call_single_data);/* * csd_lock/csd_unlock used to serialize access to per-cpu csd resources
_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec
Quoted 21 lines · Takao Indoh
Hi all,
This patch renames
init_call_single_data()tocall_function_init()and calls it instart_kernel()so thatcall_single_queuecan be initialized before enabling interrupt.There is a problem that kdump(2nd kernel) sometimes hangs up due to pending IPI from 1st kernel. Kernel panic occurs because IPI comes before
call_single_queueis initialized. The details are as follows.
(1) 2nd kernel boot up
(2) A pending IPI from 1st kernel comes when irqs are first enabled instart_kernel().
(3) Kernel tries to handle the interrupt, butcall_single_queueis not initialized yet at this point. As a result, in thegeneric_smp_call_function_single_interrupt(), NULL pointer dereference occurs whenlist_replace_init()tries to access&q->list.next.
Therefore this patch changes the name ofinit_call_single_data()tocall_function_init()and calls it beforelocal_irq_enable()instart_kernel().
Quoted 7 lines
v2:
- Renameinit_call_single_data()tocall_function_init()and calls it instart_kernel()
- Change insert position instart_kernel().
- Adjust for CONFIG_SMP/CONFIG_USE_GENERIC_SMP_HELPERS options
- Rebased to Linus's latest tree
Looks good to my eyes,
Thanks.
Quoted 105 lines · Takao Indoh
Hi all,
This patch renames
init_call_single_data()tocall_function_init()and calls it instart_kernel()so thatcall_single_queuecan be initialized before enabling interrupt.There is a problem that kdump(2nd kernel) sometimes hangs up due to pending IPI from 1st kernel. Kernel panic occurs because IPI comes before
call_single_queueis initialized. The details are as follows.
(1) 2nd kernel boot up
(2) A pending IPI from 1st kernel comes when irqs are first enabled instart_kernel().
(3) Kernel tries to handle the interrupt, butcall_single_queueis not initialized yet at this point. As a result, in thegeneric_smp_call_function_single_interrupt(), NULL pointer dereference occurs whenlist_replace_init()tries to access&q->list.next.
Therefore this patch changes the name ofinit_call_single_data()tocall_function_init()and calls it beforelocal_irq_enable()instart_kernel().v2:
- Renameinit_call_single_data()tocall_function_init()and calls it instart_kernel()
- Change insert position instart_kernel().
- Adjust for CONFIG_SMP/CONFIG_USE_GENERIC_SMP_HELPERS options
- Rebased to Linus's latest treev1:
https://lkml.org/lkml/2011/3/25/317
- Divideinit_call_single_data()into two functions,
oinit_call_single_data: initializecall_single_queue
oinit_hotplug_cfd: initializehotplug_cfd_notifier
And callinit_call_single_databeforelocal_irq_enable()instart_kernel().v0:
https://lkml.org/lkml/2011/3/23/417
- Ingeneric_smp_call_function_single_interrupt(), check ifcall_single_queuewas initialized or not, and just return if not initialized.Signed-off-by Takao Indoh <[email protected]>--- include/linux/smp.h | 5 ++++- init/main.c | 1 + kernel/smp.c | 5 +---- 3 files changed, 6 insertions(+), 5 deletions(-)include/linux/smp.h +4
-1@@ -85,12 +85,15 @@ int smp_call_function_any(const struct cpumask *mask,* Generic and arch helpers */ #ifdef CONFIG_USE_GENERIC_SMP_HELPERS +void __init call_function_init (void );void generic_smp_call_function_single_interrupt (void );void generic_smp_call_function_interrupt (void );void ipi_call_lock (void );void ipi_call_unlock (void );void ipi_call_lock_irq (void );void ipi_call_unlock_irq (void ); +#else +static inline void call_function_init (void ) { }#endif /* @@ -144,7 +147,7 @@ static inline void smp_send_reschedule(int cpu) { }#define smp_prepare_boot_cpu () do {} while (0)#define smp_call_function_many (mask, func, info, wait) \ (up_smp_call_function(func, info)) -static inline void init_call_single_data (void ) { } +static inline void call_function_init (void ) { } static inline int smp_call_function_any(const struct cpumask *mask, smp_call_func_t func,init/main.c +1
-0@@ -539,6 +539,7 @@ asmlinkage void __init start_kernel(void)timekeeping_init ();time_init ();profile_init (); +call_function_init ();if (!irqs_disabled ())printk (KERN_CRIT "start_kernel(): bug: interrupts were " "enabled early\n" );kernel/smp.c +1
-1@@ -74,7 +74,7 @@ static struct notifier_block __cpuinitdata hotplug_cfd_notifier = { .notifier_call = hotplug_cfd, }; -static int __cpuinit init_call_single_data (void ) +void __init call_function_init (void ) {void *cpu = (void *)(long )smp_processor_id ();int i;
kernel/smp.c static int __cpuinit init_call_single_data(void) · line 94
Quoted 10 lines
kernel/smp.c +0
-3@@ -88,10 +88,7 @@ static int __cpuinit init_call_single_data(void)hotplug_cfd (&hotplug_cfd_notifier,CPU_UP_PREPARE , cpu);register_cpu_notifier (&hotplug_cfd_notifier); - -return 0 ; } -early_initcall (init_call_single_data);/* * csd_lock/csd_unlock used to serialize access to per-cpu csd resources
Quoted 4 lines
_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec
_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec
Quoted 83 lines · Takao Indoh
Hi all,
This patch renames
init_call_single_data()tocall_function_init()and calls it instart_kernel()so thatcall_single_queuecan be initialized before enabling interrupt.There is a problem that kdump(2nd kernel) sometimes hangs up due to pending IPI from 1st kernel. Kernel panic occurs because IPI comes before
call_single_queueis initialized. The details are as follows.
(1) 2nd kernel boot up
(2) A pending IPI from 1st kernel comes when irqs are first enabled instart_kernel().
(3) Kernel tries to handle the interrupt, butcall_single_queueis not initialized yet at this point. As a result, in thegeneric_smp_call_function_single_interrupt(), NULL pointer dereference occurs whenlist_replace_init()tries to access&q->list.next.
Therefore this patch changes the name ofinit_call_single_data()tocall_function_init()and calls it beforelocal_irq_enable()instart_kernel().v2:
- Renameinit_call_single_data()tocall_function_init()and calls it instart_kernel()
- Change insert position instart_kernel().
- Adjust for CONFIG_SMP/CONFIG_USE_GENERIC_SMP_HELPERS options
- Rebased to Linus's latest treev1:
https://lkml.org/lkml/2011/3/25/317
- Divideinit_call_single_data()into two functions,
oinit_call_single_data: initializecall_single_queue
oinit_hotplug_cfd: initializehotplug_cfd_notifier
And callinit_call_single_databeforelocal_irq_enable()instart_kernel().v0:
https://lkml.org/lkml/2011/3/23/417
- Ingeneric_smp_call_function_single_interrupt(), check ifcall_single_queuewas initialized or not, and just return if not initialized.Signed-off-by Takao Indoh <[email protected]>--- include/linux/smp.h | 5 ++++- init/main.c | 1 + kernel/smp.c | 5 +---- 3 files changed, 6 insertions(+), 5 deletions(-)include/linux/smp.h +4
-1@@ -85,12 +85,15 @@ int smp_call_function_any(const struct cpumask *mask,* Generic and arch helpers */ #ifdef CONFIG_USE_GENERIC_SMP_HELPERS +void __init call_function_init (void );void generic_smp_call_function_single_interrupt (void );void generic_smp_call_function_interrupt (void );void ipi_call_lock (void );void ipi_call_unlock (void );void ipi_call_lock_irq (void );void ipi_call_unlock_irq (void ); +#else +static inline void call_function_init (void ) { }#endif /* @@ -144,7 +147,7 @@ static inline void smp_send_reschedule(int cpu) { }#define smp_prepare_boot_cpu () do {} while (0)#define smp_call_function_many (mask, func, info, wait) \ (up_smp_call_function(func, info)) -static inline void init_call_single_data (void ) { } +static inline void call_function_init (void ) { } static inline int smp_call_function_any(const struct cpumask *mask, smp_call_func_t func,init/main.c +0
-0
init/main.c asmlinkage void __init start_kernel(void) · line 542
Quoted 4 lines
init/main.c +1
-0@@ -539,6 +539,7 @@ asmlinkage void __init start_kernel(void)timekeeping_init ();time_init ();profile_init (); +call_function_init ();
Takao,
So by calling this function before we enable interrupts we have made sure that per cpu call_single_queue has been initialized and q->list also has been initiliazed and it is an empty list now.
After enabling the interrupts, I am assuming we will call generic_smp_call_function_single_interrupt().
this function does.
raw_spin_lock(&q->lock);
list_replace_init(&q->list, &list);
raw_spin_unlock(&q->lock);
while (!list_empty(&list)) {
struct call_single_data *data;
data = list_entry(list.next, struct call_single_data,
list);
list_del(&data->list);Looking at the code of list_replace_init(), I think we will have odd results if q->list is empty. Looks like list->next will be pointing to &q->list?
IIUC, q->list sould be empty when we get pending IPI from previous kernel because any function scheduled for execution must have been inserted on previous kernel's data structures and here we are building fresh data structures.
If that is the case, I think above code should have weared interaction. We should think that "list" is not empty and try to execute a data item q->list which is actually not a data item.
What am I missing here. After your patch, have to debugged it and noticed how list_replace_init() does on empty lists and what's the result of list_empty(list)?
Thanks
Vivek
Quoted 17 lines
init/main.c +0
-0if (!irqs_disabled ())printk (KERN_CRIT "start_kernel(): bug: interrupts were " "enabled early\n" );kernel/smp.c +1
-1@@ -74,7 +74,7 @@ static struct notifier_block __cpuinitdata hotplug_cfd_notifier = { .notifier_call = hotplug_cfd, }; -static int __cpuinit init_call_single_data (void ) +void __init call_function_init (void ) {void *cpu = (void *)(long )smp_processor_id ();int i;
kernel/smp.c static int __cpuinit init_call_single_data(void) · line 94
Quoted 10 lines
kernel/smp.c +0
-3@@ -88,10 +88,7 @@ static int __cpuinit init_call_single_data(void)hotplug_cfd (&hotplug_cfd_notifier,CPU_UP_PREPARE , cpu);register_cpu_notifier (&hotplug_cfd_notifier); - -return 0 ; } -early_initcall (init_call_single_data);/* * csd_lock/csd_unlock used to serialize access to per-cpu csd resources
_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec
Quoted 120 lines · Vivek Goyal
Quoted 83 lines · Takao Indoh
Hi all,
This patch renames
init_call_single_data()tocall_function_init()and calls it instart_kernel()so thatcall_single_queuecan be initialized before enabling interrupt.There is a problem that kdump(2nd kernel) sometimes hangs up due to pending IPI from 1st kernel. Kernel panic occurs because IPI comes before
call_single_queueis initialized. The details are as follows.
(1) 2nd kernel boot up
(2) A pending IPI from 1st kernel comes when irqs are first enabled instart_kernel().
(3) Kernel tries to handle the interrupt, butcall_single_queueis not initialized yet at this point. As a result, in thegeneric_smp_call_function_single_interrupt(), NULL pointer dereference occurs whenlist_replace_init()tries to access&q->list.next.
Therefore this patch changes the name ofinit_call_single_data()tocall_function_init()and calls it beforelocal_irq_enable()instart_kernel().v2:
- Renameinit_call_single_data()tocall_function_init()and calls it instart_kernel()
- Change insert position instart_kernel().
- Adjust for CONFIG_SMP/CONFIG_USE_GENERIC_SMP_HELPERS options
- Rebased to Linus's latest treev1:
https://lkml.org/lkml/2011/3/25/317
- Divideinit_call_single_data()into two functions,
oinit_call_single_data: initializecall_single_queue
oinit_hotplug_cfd: initializehotplug_cfd_notifier
And callinit_call_single_databeforelocal_irq_enable()instart_kernel().v0:
https://lkml.org/lkml/2011/3/23/417
- Ingeneric_smp_call_function_single_interrupt(), check ifcall_single_queuewas initialized or not, and just return if not initialized.Signed-off-by Takao Indoh <[email protected]>--- include/linux/smp.h | 5 ++++- init/main.c | 1 + kernel/smp.c | 5 +---- 3 files changed, 6 insertions(+), 5 deletions(-)include/linux/smp.h +4
-1@@ -85,12 +85,15 @@ int smp_call_function_any(const struct cpumask *mask,* Generic and arch helpers */ #ifdef CONFIG_USE_GENERIC_SMP_HELPERS +void __init call_function_init (void );void generic_smp_call_function_single_interrupt (void );void generic_smp_call_function_interrupt (void );void ipi_call_lock (void );void ipi_call_unlock (void );void ipi_call_lock_irq (void );void ipi_call_unlock_irq (void ); +#else +static inline void call_function_init (void ) { }#endif /* @@ -144,7 +147,7 @@ static inline void smp_send_reschedule(int cpu) { }#define smp_prepare_boot_cpu () do {} while (0)#define smp_call_function_many (mask, func, info, wait) \ (up_smp_call_function(func, info)) -static inline void init_call_single_data (void ) { } +static inline void call_function_init (void ) { } static inline int smp_call_function_any(const struct cpumask *mask, smp_call_func_t func,init/main.c +0
-0init/main.c asmlinkage void __init start_kernel(void) · line 542
Quoted 4 lines
init/main.c +1
-0@@ -539,6 +539,7 @@ asmlinkage void __init start_kernel(void)timekeeping_init ();time_init ();profile_init (); +call_function_init ();Takao,
So by calling this function before we enable interrupts we have made sure that per cpu
call_single_queuehas been initialized andq->listalso has been initiliazed and it is an empty list now.After enabling the interrupts, I am assuming we will call
generic_smp_call_function_single_interrupt().this function does.
raw_spin_lock(&q->lock); list_replace_init(&q->list, &list); raw_spin_unlock(&q->lock); while (!list_empty(&list)) { struct call_single_data *data; data = list_entry(list.next, struct call_single_data, list); list_del(&data->list);Looking at the code of
list_replace_init(), I think we will have odd results ifq->listis empty. Looks likelist->nextwill be pointing to&q->list?IIUC,
q->listsould be empty when we get pending IPI from previous kernel because any function scheduled for execution must have been inserted on previous kernel's data structures and here we are building fresh data structures.
Quoted 7 lines
If that is the case, I think above code should have weared interaction. We should think that "list" is not empty and try to execute a data item
q->listwhich is actually not a data item.What am I missing here. After your patch, have to debugged it and noticed how
list_replace_init()does on empty lists and what's the result oflist_empty(list)?
When list_replace_init(&q->list, &list) is called, they are changed as followed.
/* list_replace */
(A) &list->next = &q->list->next;
(B) &list->next->prev = &list;
(C) &list->prev = &q->list->prev;
(D) &list->prev->next = &list;
/* INIT_LIST_HEAD */
(E) &q->list->next = &q->list;
(F) &q->list->prev = &q->list;
So, if q->list is empty, each list is changed like this.
(Initial state)
list.next ==> &list
list.prev ==> &listq->list.next ==> &q->listq->list.prev ==> &q->list
(A)
list.next ==> &q->list
list.prev ==> &listq->list.next ==> &q->listq->list.prev ==> &q->list
(B)
list.next ==> &q->list
list.prev ==> &listq->list.next ==> &q->listq->list.prev ==> &list
(C)
list.next ==> &q->list
list.prev ==> &listq->list.next ==> &q->listq->list.prev ==> &list
(D)
list.next ==> &list
list.prev ==> &listq->list.next ==> &q->listq->list.prev ==> &list
(E)
list.next ==> &list
list.prev ==> &listq->list.next ==> &q->listq->list.prev ==> &list
(F)
list.next ==> &list
list.prev ==> &listq->list.next ==> &q->listq->list.prev ==> &q->list
So, list_empty(list)? is always false, if I am not missing something.
Thanks,
Takao Indoh
Quoted 34 lines
Thanks
VivekQuoted 28 lines
init/main.c +0
-0if (!irqs_disabled ())printk (KERN_CRIT "start_kernel(): bug: interrupts were " "enabled early\n" );kernel/smp.c +1
-1@@ -74,7 +74,7 @@ static struct notifier_block __cpuinitdata hotplug_cfd_notifier = { .notifier_call = hotplug_cfd, }; -static int __cpuinit init_call_single_data (void ) +void __init call_function_init (void ) {void *cpu = (void *)(long )smp_processor_id ();kernel/smp.c +0
-3int i;@@ -88,10 +88,7 @@ static int __cpuinit init_call_single_data(void)hotplug_cfd (&hotplug_cfd_notifier,CPU_UP_PREPARE , cpu);register_cpu_notifier (&hotplug_cfd_notifier); - -return 0 ; } -early_initcall (init_call_single_data);/* * csd_lock/csd_unlock used to serialize access to per-cpu csd resources
Quoted 181 lines · Takao Indoh
Quoted 120 lines · Vivek Goyal
Quoted 83 lines · Takao Indoh
Hi all,
This patch renames
init_call_single_data()tocall_function_init()and calls it instart_kernel()so thatcall_single_queuecan be initialized before enabling interrupt.There is a problem that kdump(2nd kernel) sometimes hangs up due to pending IPI from 1st kernel. Kernel panic occurs because IPI comes before
call_single_queueis initialized. The details are as follows.
(1) 2nd kernel boot up
(2) A pending IPI from 1st kernel comes when irqs are first enabled instart_kernel().
(3) Kernel tries to handle the interrupt, butcall_single_queueis not initialized yet at this point. As a result, in thegeneric_smp_call_function_single_interrupt(), NULL pointer dereference occurs whenlist_replace_init()tries to access&q->list.next.
Therefore this patch changes the name ofinit_call_single_data()tocall_function_init()and calls it beforelocal_irq_enable()instart_kernel().v2:
- Renameinit_call_single_data()tocall_function_init()and calls it instart_kernel()
- Change insert position instart_kernel().
- Adjust for CONFIG_SMP/CONFIG_USE_GENERIC_SMP_HELPERS options
- Rebased to Linus's latest treev1:
https://lkml.org/lkml/2011/3/25/317
- Divideinit_call_single_data()into two functions,
oinit_call_single_data: initializecall_single_queue
oinit_hotplug_cfd: initializehotplug_cfd_notifier
And callinit_call_single_databeforelocal_irq_enable()instart_kernel().v0:
https://lkml.org/lkml/2011/3/23/417
- Ingeneric_smp_call_function_single_interrupt(), check ifcall_single_queuewas initialized or not, and just return if not initialized.Signed-off-by Takao Indoh <[email protected]>--- include/linux/smp.h | 5 ++++- init/main.c | 1 + kernel/smp.c | 5 +---- 3 files changed, 6 insertions(+), 5 deletions(-)include/linux/smp.h +4
-1@@ -85,12 +85,15 @@ int smp_call_function_any(const struct cpumask *mask,* Generic and arch helpers */ #ifdef CONFIG_USE_GENERIC_SMP_HELPERS +void __init call_function_init (void );void generic_smp_call_function_single_interrupt (void );void generic_smp_call_function_interrupt (void );void ipi_call_lock (void );void ipi_call_unlock (void );void ipi_call_lock_irq (void );void ipi_call_unlock_irq (void ); +#else +static inline void call_function_init (void ) { }#endif /* @@ -144,7 +147,7 @@ static inline void smp_send_reschedule(int cpu) { }#define smp_prepare_boot_cpu () do {} while (0)#define smp_call_function_many (mask, func, info, wait) \ (up_smp_call_function(func, info)) -static inline void init_call_single_data (void ) { } +static inline void call_function_init (void ) { } static inline int smp_call_function_any(const struct cpumask *mask, smp_call_func_t func,init/main.c +0
-0init/main.c asmlinkage void __init start_kernel(void) · line 542
Quoted 4 lines
init/main.c +1
-0@@ -539,6 +539,7 @@ asmlinkage void __init start_kernel(void)timekeeping_init ();time_init ();profile_init (); +call_function_init ();Takao,
So by calling this function before we enable interrupts we have made sure that per cpu
call_single_queuehas been initialized andq->listalso has been initiliazed and it is an empty list now.After enabling the interrupts, I am assuming we will call
generic_smp_call_function_single_interrupt().this function does.
raw_spin_lock(&q->lock); list_replace_init(&q->list, &list); raw_spin_unlock(&q->lock); while (!list_empty(&list)) { struct call_single_data *data; data = list_entry(list.next, struct call_single_data, list); list_del(&data->list);Looking at the code of
list_replace_init(), I think we will have odd results ifq->listis empty. Looks likelist->nextwill be pointing to&q->list?IIUC,
q->listsould be empty when we get pending IPI from previous kernel because any function scheduled for execution must have been inserted on previous kernel's data structures and here we are building fresh data structures.Quoted 7 lines
If that is the case, I think above code should have weared interaction. We should think that "list" is not empty and try to execute a data item
q->listwhich is actually not a data item.What am I missing here. After your patch, have to debugged it and noticed how
list_replace_init()does on empty lists and what's the result oflist_empty(list)?When
list_replace_init(&q->list, &list)is called, they are changed as followed./*
list_replace*/
(A)&list->next=&q->list->next;
(B)&list->next->prev= &list;
(C)&list->prev=&q->list->prev;
(D)&list->prev->next= &list;/*
INIT_LIST_HEAD*/
(E)&q->list->next=&q->list;
(F)&q->list->prev=&q->list;So, if
q->listis empty, each list is changed like this.(Initial state)
list.next ==> &list
list.prev ==> &listq->list.next==>&q->listq->list.prev==>&q->list(A)
list.next ==>&q->list
list.prev ==> &listq->list.next==>&q->listq->list.prev==>&q->list(B)
list.next ==>&q->list
list.prev ==> &listq->list.next==>&q->listq->list.prev==> &list(C)
list.next ==>&q->list
list.prev ==> &listq->list.next==>&q->listq->list.prev==> &list(D)
list.next ==> &list
list.prev ==> &listq->list.next==>&q->listq->list.prev==> &list(E)
list.next ==> &list
list.prev ==> &listq->list.next==>&q->listq->list.prev==> &list
Quoted 8 lines
(F)
list.next ==> &list
list.prev ==> &listq->list.next==>&q->listq->list.prev==>&q->listSo,
list_empty(list)? is always false, if I am not missing something.
No, list_empty(list)? is always *true*.
Thanks,
Takao Indoh
Quoted 34 lines
Quoted 34 lines
Thanks
VivekQuoted 28 lines
init/main.c +0
-0if (!irqs_disabled ())printk (KERN_CRIT "start_kernel(): bug: interrupts were " "enabled early\n" );kernel/smp.c +1
-1@@ -74,7 +74,7 @@ static struct notifier_block __cpuinitdata hotplug_cfd_notifier = { .notifier_call = hotplug_cfd, }; -static int __cpuinit init_call_single_data (void ) +void __init call_function_init (void ) {void *cpu = (void *)(long )smp_processor_id ();kernel/smp.c +0
-3int i;@@ -88,10 +88,7 @@ static int __cpuinit init_call_single_data(void)hotplug_cfd (&hotplug_cfd_notifier,CPU_UP_PREPARE , cpu);register_cpu_notifier (&hotplug_cfd_notifier); - -return 0 ; } -early_initcall (init_call_single_data);/* * csd_lock/csd_unlock used to serialize access to per-cpu csd resources
Vivek Goyal
Acked-byQuoted 48 lines · Takao Indoh
Quoted 46 lines · Takao Indoh
Quoted 44 lines · Vivek Goyal
Quoted 43 lines · Takao Indoh
Hi all,
This patch renames
init_call_single_data()tocall_function_init()and calls it instart_kernel()so thatcall_single_queuecan be initialized before enabling interrupt.There is a problem that kdump(2nd kernel) sometimes hangs up due to pending IPI from 1st kernel. Kernel panic occurs because IPI comes before
call_single_queueis initialized. The details are as follows.
(1) 2nd kernel boot up
(2) A pending IPI from 1st kernel comes when irqs are first enabled instart_kernel().
(3) Kernel tries to handle the interrupt, butcall_single_queueis not initialized yet at this point. As a result, in thegeneric_smp_call_function_single_interrupt(), NULL pointer dereference occurs whenlist_replace_init()tries to access&q->list.next.
Therefore this patch changes the name ofinit_call_single_data()tocall_function_init()and calls it beforelocal_irq_enable()instart_kernel().v2:
- Renameinit_call_single_data()tocall_function_init()and calls it instart_kernel()
- Change insert position instart_kernel().
- Adjust for CONFIG_SMP/CONFIG_USE_GENERIC_SMP_HELPERS options
- Rebased to Linus's latest treev1:
https://lkml.org/lkml/2011/3/25/317
- Divideinit_call_single_data()into two functions,
oinit_call_single_data: initializecall_single_queue
oinit_hotplug_cfd: initializehotplug_cfd_notifier
And callinit_call_single_databeforelocal_irq_enable()instart_kernel().v0:
https://lkml.org/lkml/2011/3/23/417
- Ingeneric_smp_call_function_single_interrupt(), check ifcall_single_queuewas initialized or not, and just return if not initialized.Signed-off-by Takao Indoh <[email protected]>
[..]
Quoted 4 lines
Quoted 1 line
So,
list_empty(list)? is always false, if I am not missing something.No,
list_empty(list)? is always *true*.
ok, I got it. So list_replace_init() is fine even if q->list is empty. So when a pending IPI from previous kernel comes, q->list will be empty and we will simply do nothing. Sounds reaosnable to me.
Thanks
Vivek
_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec
Just a reminder...
Thanks for acks, but I'm still waiting for inclusion. I tested 3.0.0-rc3 and kdump still failed due to pending IPI from 1st kernel. Need this patch to fix it.
Thanks,
Takao Indoh
Quoted 115 lines · Takao Indoh
Hi all,
This patch renames
init_call_single_data()tocall_function_init()and calls it instart_kernel()so thatcall_single_queuecan be initialized before enabling interrupt.There is a problem that kdump(2nd kernel) sometimes hangs up due to pending IPI from 1st kernel. Kernel panic occurs because IPI comes before
call_single_queueis initialized. The details are as follows.
(1) 2nd kernel boot up
(2) A pending IPI from 1st kernel comes when irqs are first enabled instart_kernel().
(3) Kernel tries to handle the interrupt, butcall_single_queueis not initialized yet at this point. As a result, in thegeneric_smp_call_function_single_interrupt(), NULL pointer dereference occurs whenlist_replace_init()tries to access&q->list.next.
Therefore this patch changes the name ofinit_call_single_data()tocall_function_init()and calls it beforelocal_irq_enable()instart_kernel().v2:
- Renameinit_call_single_data()tocall_function_init()and calls it instart_kernel()
- Change insert position instart_kernel().
- Adjust for CONFIG_SMP/CONFIG_USE_GENERIC_SMP_HELPERS options
- Rebased to Linus's latest treev1:
https://lkml.org/lkml/2011/3/25/317
- Divideinit_call_single_data()into two functions,
oinit_call_single_data: initializecall_single_queue
oinit_hotplug_cfd: initializehotplug_cfd_notifier
And callinit_call_single_databeforelocal_irq_enable()instart_kernel().v0:
https://lkml.org/lkml/2011/3/23/417
- Ingeneric_smp_call_function_single_interrupt(), check ifcall_single_queuewas initialized or not, and just return if not initialized.Signed-off-by Takao Indoh <[email protected]>---
include/linux/smp.h | 5 ++++- init/main.c | 1 + kernel/smp.c | 5 +---- 3 files changed, 6 insertions(+), 5 deletions(-)include/linux/smp.h +4
-1@@ -85,12 +85,15 @@ int smp_call_function_any(const struct cpumask *mask,* Generic and arch helpers */ #ifdef CONFIG_USE_GENERIC_SMP_HELPERS +void __init call_function_init (void );void generic_smp_call_function_single_interrupt (void );void generic_smp_call_function_interrupt (void );void ipi_call_lock (void );void ipi_call_unlock (void );void ipi_call_lock_irq (void );void ipi_call_unlock_irq (void ); +#else +static inline void call_function_init (void ) { }#endif /* @@ -144,7 +147,7 @@ static inline void smp_send_reschedule(int cpu) { }#define smp_prepare_boot_cpu () do {} while (0)#define smp_call_function_many (mask, func, info, wait) \ (up_smp_call_function(func, info)) -static inline void init_call_single_data (void ) { } +static inline void call_function_init (void ) { }static inline int
smp_call_function_any(const struct cpumask *mask,smp_call_func_tfunc,init/main.c +1
-0@@ -539,6 +539,7 @@ asmlinkage void __init start_kernel(void)timekeeping_init ();time_init ();profile_init (); +call_function_init ();if (!irqs_disabled ())printk (KERN_CRIT "start_kernel(): bug: interrupts were " "enabled early\n" );kernel/smp.c +1
-1@@ -74,7 +74,7 @@ static struct notifier_block __cpuinitdata hotplug_cfd_notifier = { .notifier_call = hotplug_cfd, }; -static int __cpuinit init_call_single_data (void ) +void __init call_function_init (void ) {void *cpu = (void *)(long )smp_processor_id ();int i;kernel/smp.c +0
-3@@ -88,10 +88,7 @@ static int __cpuinit init_call_single_data(void)hotplug_cfd (&hotplug_cfd_notifier,CPU_UP_PREPARE , cpu);register_cpu_notifier (&hotplug_cfd_notifier); - -return 0 ; } -early_initcall (init_call_single_data);/* * csd_lock/csd_unlock used to serialize access to per-cpu csd resources
Quoted 5 lines · Takao Indoh
Just a reminder...
Thanks for acks, but I'm still waiting for inclusion. I tested 3.0.0-rc3 and kdump still failed due to pending IPI from 1st kernel. Need this patch to fix it.
For any future patch that has a review discussion (like this one) please collect Acks and resend the patch with the acks included once the discussion results in a final patch, otherwise maintainers might skip the patch as "still being reviewed".
I've collected the acks here and started testing your patch.
Thanks,
Ingo
_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec
Quoted 9 lines · Ingo Molnar
Quoted 5 lines · Takao Indoh
Just a reminder...
Thanks for acks, but I'm still waiting for inclusion. I tested 3.0.0-rc3 and kdump still failed due to pending IPI from 1st kernel. Need this patch to fix it.
Quoted 6 lines
For any future patch that has a review discussion (like this one) please collect Acks and resend the patch with the acks included once the discussion results in a final patch, otherwise maintainers might skip the patch as "still being reviewed".
I've collected the acks here and started testing your patch.
Ok, understood. Thanks for your advice.
Thanks,
Takao Indoh
Quoted 4 lines
Thanks,
Ingo
[tip:sched/urgent] generic-ipi: Fix kexec boot crash by initializing call_single_queue before enabling interrupts
In reply to Takao IndohCommit-ID: d8ad7d1123a960cc9f276bd499f9325c6f5e1bd1 Gitweb: http://git.kernel.org/tip/d8ad7d1123a960cc9f276bd499f9325c6f5e1bd1 Author: Takao Indoh <[email protected]> AuthorDate: Tue, 29 Mar 2011 12:35:04 -0400 Committer: Ingo Molnar <[email protected]> CommitDate: Fri, 17 Jun 2011 10:17:12 +0200
generic-ipi: Fix kexec boot crash by initializing call_single_queue before enabling interrupts
There is a problem that kdump(2nd kernel) sometimes hangs up due to a pending IPI from 1st kernel. Kernel panic occurs because IPI comes before call_single_queue is initialized.
To fix the crash, rename init_call_single_data() to call_function_init() and call it in start_kernel() so that call_single_queue can be initialized before enabling interrupts.
The details of the crash are:
(1) 2nd kernel boots up
(2) A pending IPI from 1st kernel comes when irqs are first enabled in start_kernel().
(3) Kernel tries to handle the interrupt, but call_single_queue is not initialized yet at this point. As a result, in the generic_smp_call_function_single_interrupt(), NULL pointer dereference occurs when list_replace_init() tries to access &q->list.next.
Therefore this patch changes the name of init_call_single_data() to call_function_init() and calls it before local_irq_enable() in start_kernel().
--- include/linux/smp.h | 5 ++++- init/main.c | 1 + kernel/smp.c | 5 +---- 3 files changed, 6 insertions(+), 5 deletions(-)
include/linux/smp.h +4 -1
@@ -85,12 +85,15 @@ int smp_call_function_any(const struct cpumask *mask,* Generic and arch helpers */ #ifdef CONFIG_USE_GENERIC_SMP_HELPERS +void __init call_function_init (void );void generic_smp_call_function_single_interrupt (void );void generic_smp_call_function_interrupt (void );void ipi_call_lock (void );void ipi_call_unlock (void );void ipi_call_lock_irq (void );void ipi_call_unlock_irq (void ); +#else +static inline void call_function_init (void ) { }#endif /*
@@ -134,7 +137,7 @@ static inline void smp_send_reschedule(int cpu) { }#define smp_prepare_boot_cpu () do {} while (0)#define smp_call_function_many (mask, func, info, wait) \ (up_smp_call_function(func, info)) -static inline void init_call_single_data (void ) { } +static inline void call_function_init (void ) { } static inline int smp_call_function_any(const struct cpumask *mask, smp_call_func_t func,
init/main.c +1 -0
@@ -542,6 +542,7 @@ asmlinkage void __init start_kernel(void)timekeeping_init ();time_init ();profile_init (); +call_function_init ();if (!irqs_disabled ())printk (KERN_CRIT "start_kernel(): bug: interrupts were " "enabled early\n" );
kernel/smp.c +1 -4
@@ -74,7 +74,7 @@ static struct notifier_block __cpuinitdata hotplug_cfd_notifier = { .notifier_call = hotplug_cfd, }; -static int __cpuinit init_call_single_data (void ) +void __init call_function_init (void ) {void *cpu = (void *)(long )smp_processor_id ();int i;
@@ -88,10 +88,7 @@ static int __cpuinit init_call_single_data(void)hotplug_cfd (&hotplug_cfd_notifier,CPU_UP_PREPARE , cpu);register_cpu_notifier (&hotplug_cfd_notifier); - -return 0 ; } -early_initcall (init_call_single_data);/* * csd_lock/csd_unlock used to serialize access to per-cpu csd resources