smp: kill unnecessary variable - csd_flags
PATCH 1/1 smp: kill unnecessary variable - csd_flags
We used csd_flags formerly because we allocated csd_data by kmalloc when "wait == 0". When fail to allocation, we will fall back to on-stack allocation. "csd_data" might be invalid after generic_exec_single return.
But now we use per cpu data for single cpu ipi calls, and csd_data can't fall back to on-stack allocation when "wait == 0".
So csd_flags is unnecessary now. Remove it.
--- kernel/smp.c | 11 +---------- 1 files changed, 1 insertions(+), 10 deletions(-)
kernel/smp.c +1 -10
@@ -186,25 +186,16 @@ void generic_smp_call_function_single_interrupt(void)while (!list_empty (&list)) {struct call_single_data *csd; -unsigned int csd_flags; csd = list_entry(list.next, struct call_single_data, list);list_del (&csd->list); -/* -* 'csd' can be invalid after this call if flags == 0 -* (when called through generic_exec_single()), -* so save them away before making the call: -*/ - csd_flags = csd->flags; - csd->func (csd->info);/* * Unlocked CSDs are valid through generic_exec_single(): */ -if (csd_flags &CSD_FLAG_LOCK ) -csd_unlock (csd); +csd_unlock (csd); } }
Signature · 1 line
1.6.0.2
Quoted 9 lines · Xie XiuQi
We used
csd_flagsformerly because we allocatedcsd_databy kmalloc when "wait == 0". When fail to allocation, we will fall back to on-stack allocation. "csd_data" might be invalid aftergeneric_exec_singlereturn.But now we use per cpu data for single cpu ipi calls, and
csd_datacan't fall back to on-stack allocation when "wait == 0".So
csd_flagsis unnecessary now. Remove it.
The much simpler argument is that both callsites of generic_exec_single() do an unconditional csd_lock().
Quoted 23 lines
Signed-off-by Xie XiuQi <[email protected]>--- kernel/smp.c | 11 +---------- 1 files changed, 1 insertions(+), 10 deletions(-)kernel/smp.c +0
-6@@ -186,25 +186,16 @@ void generic_smp_call_function_single_interrupt(void)while (!list_empty (&list)) {struct call_single_data *csd; -unsigned int csd_flags; csd = list_entry(list.next, struct call_single_data, list);list_del (&csd->list); -/* -* 'csd' can be invalid after this call if flags == 0 -* (when called through generic_exec_single()), -* so save them away before making the call: -*/
kernel/smp.c void generic_smp_call_function_single_interrupt(void) · line 198
Quoted 10 lines
kernel/smp.c +1
-4- csd_flags = csd->flags; - csd->func (csd->info);/* * Unlocked CSDs are valid through generic_exec_single(): */ -if (csd_flags &CSD_FLAG_LOCK ) -csd_unlock (csd); +csd_unlock (csd);
The comment is completely useless and confusing after this; why do you leave it in?
Quoted 6 lines · Peter Zijlstra
Quoted 5 lines · Xie XiuQi
We used
csd_flagsformerly because we allocatedcsd_databy kmalloc when "wait == 0". When fail to allocation, we will fall back to on-stack allocation. "csd_data" might be invalid aftergeneric_exec_singlereturn.
Quoted 8 lines
Quoted 4 lines
But now we use per cpu data for single cpu ipi calls, and
csd_datacan't fall back to on-stack allocation when "wait == 0".So
csd_flagsis unnecessary now. Remove it.The much simpler argument is that both callsites of
generic_exec_single()do an unconditionalcsd_lock().
That's right. All csd_data are protected by csd_lock.
Quoted 27 lines
Quoted 27 lines
Signed-off-by Xie XiuQi <[email protected]>--- kernel/smp.c | 11 +---------- 1 files changed, 1 insertions(+), 10 deletions(-)kernel/smp.c +0
-8@@ -186,25 +186,16 @@ void generic_smp_call_function_single_interrupt(void)while (!list_empty (&list)) {struct call_single_data *csd; -unsigned int csd_flags; csd = list_entry(list.next, struct call_single_data, list);list_del (&csd->list); -/* -* 'csd' can be invalid after this call if flags == 0 -* (when called through generic_exec_single()), -* so save them away before making the call: -*/ - csd_flags = csd->flags; - csd->func (csd->info);
Quoted 10 lines
kernel/smp.c void generic_smp_call_function_single_interrupt(void) · line 198
Quoted 6 lines
kernel/smp.c +1
-2/* * Unlocked CSDs are valid through generic_exec_single(): */ -if (csd_flags &CSD_FLAG_LOCK ) -csd_unlock (csd); +csd_unlock (csd);The comment is completely useless and confusing after this; why do you leave it in?
Thanks for your comment, I'll remove the comment and send a new version soon.