From mboxrd@z Thu Jan 1 00:00:00 1970 From: mhiramat@kernel.org (Masami Hiramatsu) Date: Thu, 26 Jan 2017 10:49:16 +0900 Subject: [PATCH 2/2 v2] perf tools: Enable bpf prologue for arm64 In-Reply-To: <20170125133201.GC27026@arm.com> References: <20170124190908.GG10340@kernel.org> <20170125072311.22922-1-hekuang@huawei.com> <20170125133201.GC27026@arm.com> Message-ID: <20170126104916.971478fa29083cf4ae14fac3@kernel.org> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Wed, 25 Jan 2017 13:32:01 +0000 Will Deacon wrote: > On Wed, Jan 25, 2017 at 07:23:11AM +0000, He Kuang wrote: > > Since HAVE_KPROBES can be enabled in arm64, this patch introduces > > regs_query_register_offset() to convert register name to offset for > > arm64, so the BPF prologue feature is ready to use. > > > > This patch also changes the 'dwarfnum' to 'offset' in register table, > > so the related functions are consistent with x86. > > Wouldn't it be an awful lot simpler just to leave the code as-is, and > implement regs_query_register_offset in the same way that we implement > get_arch_regstr but return the dwarfnum? No, since the offset is not same as dwarfnum. With this style, the index of array becomes the dwarfnum (the index of each register defined by DWARF) and the "offset" member means the byte-offset of the register in (user_)pt_regs. Those should be different. > I don't really see the point of all the refactoring. Also, from the maintenance point of view, this rewrite work makes the code simply similar to x86 implementation, that will be easier to maintain :) Thank you, -- Masami Hiramatsu From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752576AbdAZBt2 (ORCPT ); Wed, 25 Jan 2017 20:49:28 -0500 Received: from mail.kernel.org ([198.145.29.136]:58466 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752048AbdAZBt1 (ORCPT ); Wed, 25 Jan 2017 20:49:27 -0500 Date: Thu, 26 Jan 2017 10:49:16 +0900 From: Masami Hiramatsu To: Will Deacon Cc: He Kuang , peterz@infradead.org, mingo@redhat.com, acme@kernel.org, alexander.shishkin@linux.intel.com, jolsa@redhat.com, mhiramat@kernel.org, wangnan0@huawei.com, bintian.wang@huawei.com, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH 2/2 v2] perf tools: Enable bpf prologue for arm64 Message-Id: <20170126104916.971478fa29083cf4ae14fac3@kernel.org> In-Reply-To: <20170125133201.GC27026@arm.com> References: <20170124190908.GG10340@kernel.org> <20170125072311.22922-1-hekuang@huawei.com> <20170125133201.GC27026@arm.com> X-Mailer: Sylpheed 3.5.0 (GTK+ 2.24.30; x86_64-pc-linux-gnu) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, 25 Jan 2017 13:32:01 +0000 Will Deacon wrote: > On Wed, Jan 25, 2017 at 07:23:11AM +0000, He Kuang wrote: > > Since HAVE_KPROBES can be enabled in arm64, this patch introduces > > regs_query_register_offset() to convert register name to offset for > > arm64, so the BPF prologue feature is ready to use. > > > > This patch also changes the 'dwarfnum' to 'offset' in register table, > > so the related functions are consistent with x86. > > Wouldn't it be an awful lot simpler just to leave the code as-is, and > implement regs_query_register_offset in the same way that we implement > get_arch_regstr but return the dwarfnum? No, since the offset is not same as dwarfnum. With this style, the index of array becomes the dwarfnum (the index of each register defined by DWARF) and the "offset" member means the byte-offset of the register in (user_)pt_regs. Those should be different. > I don't really see the point of all the refactoring. Also, from the maintenance point of view, this rewrite work makes the code simply similar to x86 implementation, that will be easier to maintain :) Thank you, -- Masami Hiramatsu