Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753466AbdFVRaJ (ORCPT ); Thu, 22 Jun 2017 13:30:09 -0400 Received: from mail-it0-f54.google.com ([209.85.214.54]:35445 "EHLO mail-it0-f54.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752066AbdFVRaH (ORCPT ); Thu, 22 Jun 2017 13:30:07 -0400 MIME-Version: 1.0 In-Reply-To: <63d913f28bc64bd4ea66a39a532f0b59ee015382.1498039056.git.pabeni@redhat.com> References: <63d913f28bc64bd4ea66a39a532f0b59ee015382.1498039056.git.pabeni@redhat.com> From: Linus Torvalds Date: Thu, 22 Jun 2017 10:30:06 -0700 X-Google-Sender-Auth: S4xm7u7rObXklNlD70KFLnz5NiY Message-ID: Subject: Re: [PATCH] x86/uaccess: use unrolled string copy for short strings To: Paolo Abeni Cc: "the arch/x86 maintainers" , Thomas Gleixner , Ingo Molnar , "H. Peter Anvin" , Al Viro , Kees Cook , Hannes Frederic Sowa , Linux Kernel Mailing List Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1235 Lines: 31 On Wed, Jun 21, 2017 at 4:09 AM, Paolo Abeni wrote: > > + if (len <= 64) > + return copy_user_generic_unrolled(to, from, len); > + > /* > * If CPU has ERMS feature, use copy_user_enhanced_fast_string. > * Otherwise, if CPU has rep_good feature, use copy_user_generic_string. NAK. Please do *not* do this. It puts the check in completely the wrong place for several reasons: (a) it puts it in the inlined caller case (which could be ok for constant sizes, but not in general) (b) it uses the copy_user_generic_unrolled() function that will then just test the size *AGAIN* against small cases. so it's both bigger than necessary, and stupid. So if you want to do this optimization, I'd argue that you should just do it inside the copy_user_enhanced_fast_string() function itself, the same way we already handle the really small case specially in copy_user_generic_string(). And do *not* use the unrolled code, which isn't used for small copies anyway - rewrite the "copy_user_generic_unrolled" function in that same asm file to have the non-unrolled cases (label "17" and forward) accessible, so that you don't bother re-testing the size. Linus