Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758002Ab3HHQyg (ORCPT ); Thu, 8 Aug 2013 12:54:36 -0400 Received: from mail-wi0-f172.google.com ([209.85.212.172]:43783 "EHLO mail-wi0-f172.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757864Ab3HHQye (ORCPT ); Thu, 8 Aug 2013 12:54:34 -0400 Date: Thu, 8 Aug 2013 18:54:30 +0200 From: Frederic Weisbecker To: Oleg Nesterov Cc: Linus Torvalds , Grazvydas Ignotas , Felipe Contreras , Linux Kernel Mailing List , Ingo Molnar , Denys Vlasenko Subject: Re: [PATCH 0/1] (Was: Linux 3.11-rc4) Message-ID: <20130808165427.GC32049@somewhere> References: <20130806154314.GA398@redhat.com> <20130807192734.GA8395@redhat.com> <20130808154107.GA28971@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20130808154107.GA28971@redhat.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2925 Lines: 69 On Thu, Aug 08, 2013 at 05:41:07PM +0200, Oleg Nesterov wrote: > On 08/07, Linus Torvalds wrote: > > > > Now, I do agree that the debug registers are *much* less likely to > > have those kinds of really subtle issues, so maybe relaxing some of > > the tests might be reasonable. I'd be a bit nervous about it, but if > > it's *only* the length/alignment, and Intel people can be convinced > > that it doesn't result in any nasty undefined behavior (as long as the > > address is in user space), maybe we could make that change just to > > make it easier for Wine. > > Oh, I do not know. And again, this way a user can't notice the problem > if the arguments are wrong. > > But personally I think it would be nice to cleanup the perf interface, > although probably it is too later. > > On x86 execute breakpoints are only a single byte, which has to be > the first byte of the instruction. IOW the hardware requires len = 1 > in dr7 or it doesn't work (iirc). > > But for some reason perf requires bp_len = sizeof(long), not 1. And > note that it sets info->len = X86_BREAKPOINT_LEN_X. The comment says: > > x86 inst breakpoints need to have a specific undefined len > > but despite its "special" name LEN_X is simply LEN_1, and other code > relies on this fact. > > Now, ptrace correctly requires DR_LEN_1. So arch_bp_generic_fields() > translates this into "gen_len = sizeof(long)" for validation. > > arch_build_bp_info() thinks that X86_BREAKPOINT_EXECUTE should have > ->bp_len == sizeof(long), so we translate it back into LEN_1 internally. I did this interface and I'm sorry about it. This bp_len == sizeof(long) requirement comes from a very buggy conception I had at the time I wrote that. I thought it would be pretty intuitive to assume that instruction breakpoints should be the size of the instruction itself as a generic interface for all archs. But at least x86 instructions size aren't static. That sizeof(long) assumption just popped up from nowhere at 5 am two years ago I guess :-( And worse: I realized that mistake later but never moved it in the top of the TODO-list pile because I had the feeling that nobody was using the perf breakpoint interface anyway. I'm all for fixing this. May be we can start by backporting a patch that ignores the value of gen_len for instruction breakpoints in x86? I don't know how other archs use it. I need to check. But this bp_len should rather be used for range breakpoints on archs that support it. I hope we can still reuse it if the damage of my initial misconception isn't too widely expanded. What do you think? > > This looks confusing, imho. And imho X86_BREAKPOINT_LEN_X should die. Yep. Thanks. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/