linux-kernel - Re: [PATCH] x86/traps: rename conditional_{sti,cli} to cond_local_irq

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Sun, 17 Jan 2016 18:41:58 +0600
From:	Alexander Kuleshov <kuleshovmail@...il.com>
To:	Borislav Petkov <bp@...e.de>
Cc:	Alexander Kuleshov <kuleshovmail@...il.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...hat.com>,
	"H . Peter Anvin" <hpa@...or.com>, x86@...nel.org,
	Andy Lutomirski <luto@...nel.org>,
	Dave Hansen <dave.hansen@...ux.intel.com>,
	Wang Nan <wangnan0@...wei.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] x86/traps: rename conditional_{sti,cli} to
 cond_local_irq_{enable,disable}

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.