From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752134AbbDAI3o (ORCPT ); Wed, 1 Apr 2015 04:29:44 -0400 Received: from mail-wg0-f45.google.com ([74.125.82.45]:33882 "EHLO mail-wg0-f45.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750849AbbDAI3k (ORCPT ); Wed, 1 Apr 2015 04:29:40 -0400 Date: Wed, 1 Apr 2015 10:29:35 +0200 From: Ingo Molnar To: Denys Vlasenko Cc: Linus Torvalds , Steven Rostedt , Borislav Petkov , "H. Peter Anvin" , Andy Lutomirski , Oleg Nesterov , Frederic Weisbecker , Alexei Starovoitov , Will Drewry , Kees Cook , x86@kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 7/9] x86/asm/entry/32: tidy up some instructions Message-ID: <20150401082934.GA23533@gmail.com> References: <1427821211-25099-1-git-send-email-dvlasenk@redhat.com> <1427821211-25099-7-git-send-email-dvlasenk@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1427821211-25099-7-git-send-email-dvlasenk@redhat.com> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org * Denys Vlasenko wrote: > After TESTs, use logically correct JZ mnemonic instead of JE > (this doesn't change code). > > Tidy up CMPW insns: > > Modern CPUs are not good with 16-bit operations. > The instructions with 16-bit immediates are especially bad, > on many CPUs they cause length changing prefix stall > in the decoders, costing ~6 cycles to recover. > > Replace CMPWs with CMPLs. > Of these, for form with 8-bit sign-extended immediates > it is a win because they are smaller now > (no 0x66 prefix anymore); > ones with 16-bit immediates are faster. This patch does JE->JZ transitions, but it also does CMPW instruction tweaking - which was buggy as Brian (miraculously!) noticed. This isn't the first such incident, and I made this point about three times already in the past, but it appears I've not made it loud enough: which part of 'do not put two unrelated changes into the same patch' did you not understand?? We _DO NOT PUT_ multiple, unrelated changes to assembly files into a single patch! And we _especially_ don't mix them up under a meaningless, repetitive, misleading 'tidy up instructions' title! Full stop. The titles of the two patches should have been something like: x86/asm/entry/32: Convert JNE to JNZ mnemonics, to improve readability x86/asm/entry/32: Optimize CMPW to CMPL instructions, to make use of automatic zero-extend We were lucky that Brian was alert enough to have read through a misleadingly titled, seemingly harmless patch and noticed the bug in your patch, but heck you made it hard!!! And no, it's not a problem if you create a dozen trivial looking patches and have to wait a bit more for them to trickle into the maintainer tree: asm patches are seldom trivial, and even if they are trivial, both reviewability and bisectability will improve from the process. You are doing a nice job improving the x86/asm/entry code, but if you cannot create suitably conservative, maximally reviewable and maximally bisectable patches to x86/asm then I won't be able to apply assembly patches from you! Thanks, Ingo