Re: [PATCH] x86/traps: rename conditional_{sti,cli} to cond_local_irq_{enable,disable}

From: Alexander Kuleshov
Date: Sun Jan 17 2016 - 07:45:44 EST


On 01-17-16, Borislav Petkov wrote:
> On Sun, Jan 17, 2016 at 12:53:41PM +0600, Alexander Kuleshov wrote:
> So I gave you the example with preempt_enable_no_resched() but you have
> sched_preempt_enable_no_resched() below. Why?
>
> Does the traps.c code look like scheduler code and there you have to use
> scheduler primitives? Or was there another reason for it I'm not seeing
> right now?

Sorry, no, you are right here. Seems that put wrong version during reading
of linux/preempt.h.

> Also, for your next submission, always try to answer to the question
> "Why is the change being done" in the commit message instead of
> explaining what you're doing. Because "what you're doing" we can see,
> *why* you're doing it is the much more interesting question.
>

Agree. Especially for *git blame*.

> For example, I would've written:
>
> "Make the preemption and interrupt flag handling more readable in the
> traps.c code. While at it, remove silly helpers and rename others to
> more understandable names so that one doesn't have to go and lookup the
> function definition when looking at the code flow."
>
> Or something with a similar effect...

Thank you Borislav for help. I will update commit message and
s/sched_preempt_enable_no_resched/preempt_enable_no_resched in the patch,
and resend it.