* Christoph Lameter <c...@linux.com> wrote: > On Tue, 24 Sep 2013, Ingo Molnar wrote: > > > During past review of your series Peter Zijlstra very explicitly told you > > to reuse (and unify with) the preempt checks in lib/smp_processor_id.c! > > See debug_smp_processor_id(). > > No he did not. He mentioned something about debug_smp_processor_id() at > the end of a post after talking about something else. Given your > comments now I see what was meant. That was not really obvious in the > first place.
Holy cow, this is what PeterZ wrote to you a week ago: > +#ifdef CONFIG_PREEMPT > +extern void this_cpu_preempt_check(void); > +#else > +static inline void this_cpu_preempt_check(void) { } > +#endif How about re-using debug_smp_processor_id() instead? http://lkml.org/lkml/2013/9/16/137 Firstly, that sentence is as damn obvious as it gets. Secondly, even if it wasn't obvious to you, a 'git grep debug_smp_processor_id' would have told you the story. Thirdly, if it wasn't obvious to you, and if you didn't think of using git grep, how about ... asking? If a reviewer gives you review feedback then you should address _EVERY_ single review feedback and not just ignore it... I get the impression that you are trying to deny your excessive sloppiness in this thread - there's no other way to put it really. > > The problem isn't just that you are duplicating code and adding > > unnecessary #ifdefs into the wrong place, the bigger problem is that > > you are implementing weak checks which creates unnecessary raw_*() > > pollution all across the kernel. > > what kind of idiotic comment is that? I am using a single function > preemptible(). How is that duplicating anything? as PeterZ pointed it out, we have well-working preempt checks in debug_smp_processor_id() / lib/smp_processor_id.c. Instead of reusing that you created a new preempt check, plopped your new, 25 lines, duplicated check into an ugly #ifdef section into sched/core.c - see that gem attached further below. > > Your lack of cooperation is getting ridiculous! > > And this kind of insulting behavior is really discouraging people to do > work on the kernel. Pointing out your repeated lack of cooperation in this matter is a statement of facts, not an 'insulting behavior'. Your wasting of other people's time is simply not acceptable. That I called you out on it might be embarrassing to you but there's a really easy solution to that: implement reviewer and maintainer requests and don't send sloppy patches repeatedly. Ingo > Index: linux/kernel/sched/core.c > =================================================================== > --- linux.orig/kernel/sched/core.c 2013-09-23 10:24:47.371629684 -0500 > +++ linux/kernel/sched/core.c 2013-09-23 10:24:47.371629684 -0500 > @@ -2566,6 +2566,29 @@ asmlinkage void __sched preempt_schedule > exception_exit(prev_state); > } > > +#ifdef CONFIG_DEBUG_PREEMPT > +/* > + * This function is called if the kernel is compiled with preempt > + * support for each __this_cpu operations. It verifies that > + * preemption has been disabled. > + * > + * The function cannot be a macro due to the low level nature > + * of the per cpu header files. > + */ > +void __this_cpu_preempt_check(void) > +{ > + int p; > + > + p = preemptible(); > + if (p) { > + printk(KERN_ERR "__this_cpu but preemptable." > + " preempt_count=%d irqs_disabled=%d\n", > + preempt_count(), irqs_disabled()); > + dump_stack(); > + } > + > +} > +#endif /* CONFIG_DEBUG_PREEMPT */ > #endif /* CONFIG_PREEMPT */ > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/