From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751945AbdB1CbV (ORCPT ); Mon, 27 Feb 2017 21:31:21 -0500 Received: from mail.kernel.org ([198.145.29.136]:52162 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751378AbdB1CbS (ORCPT ); Mon, 27 Feb 2017 21:31:18 -0500 Date: Mon, 27 Feb 2017 16:06:01 -0500 From: Steven Rostedt To: David Daney Cc: Jason Baron , linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, linux-mips@linux-mips.org, linuxppc-dev@lists.ozlabs.org, Ingo Molnar , Benjamin Herrenschmidt , Paul Mackerras , Michael Ellerman , Anton Blanchard , Rabin Vincent , Russell King , Ralf Baechle , Chris Metcalf , Zhigang Lu Subject: Re: [PATCH] jump_label: align jump_entry table to at least 4-bytes Message-ID: <20170227160601.5b79a1fe@gandalf.local.home> In-Reply-To: References: <1488221364-13905-1-git-send-email-jbaron@akamai.com> <93219edf-0f6d-5cc7-309c-c998f16fe7ac@akamai.com> X-Mailer: Claws Mail 3.14.0 (GTK+ 2.24.31; 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 Mon, 27 Feb 2017 11:59:50 -0800 David Daney wrote: > For me the size is not the important issue, it is the alignment of the > struct jump_entry entries in the table. I don't understand how your > patch helps, and I cannot Acked-by unless I understand what is being > done and can see that it is both correct and necessary. You brought up a very good point and I'm glad that I had Jason Cc all the arch maintainers in one patch. I think jump_labels may be much more broken than we think, and Jason's fix doesn't fix anything. We had this same issues with tracepoints. I'm looking at jump_label_init, and how we iterate over an array of struct jump_entry's that was put together by the linker. The problem is that jump_entry is not a power of 2 in size. struct jump_entry { jump_label_t code; jump_label_t target; jump_label_t key; }; When putting together arrays of this kind, the linker is in its right to add padding for alignment, in the middle of the array! It has no idea that this is an array, and there's nothing stopping the linker from messing it up. For those structs that are a power of 2 in size, there's no reason for the linker to do anything else, and it "just works". There's plenty of instances in the kernel that depend on this. I'm thinking that the sort algorithm either hid the problem or fixed it somehow (I'm guessing it hid the problem). I hit the same issue with trace event structures. The solution was to create the array of pointers to each structure, and dereference the structures from the array. See commit e4a9ea5ee ("tracing: Replace trace_event struct array with pointer array") -- Steve From mboxrd@z Thu Jan 1 00:00:00 1970 From: rostedt@goodmis.org (Steven Rostedt) Date: Mon, 27 Feb 2017 16:06:01 -0500 Subject: [PATCH] jump_label: align jump_entry table to at least 4-bytes In-Reply-To: References: <1488221364-13905-1-git-send-email-jbaron@akamai.com> <93219edf-0f6d-5cc7-309c-c998f16fe7ac@akamai.com> Message-ID: <20170227160601.5b79a1fe@gandalf.local.home> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Mon, 27 Feb 2017 11:59:50 -0800 David Daney wrote: > For me the size is not the important issue, it is the alignment of the > struct jump_entry entries in the table. I don't understand how your > patch helps, and I cannot Acked-by unless I understand what is being > done and can see that it is both correct and necessary. You brought up a very good point and I'm glad that I had Jason Cc all the arch maintainers in one patch. I think jump_labels may be much more broken than we think, and Jason's fix doesn't fix anything. We had this same issues with tracepoints. I'm looking at jump_label_init, and how we iterate over an array of struct jump_entry's that was put together by the linker. The problem is that jump_entry is not a power of 2 in size. struct jump_entry { jump_label_t code; jump_label_t target; jump_label_t key; }; When putting together arrays of this kind, the linker is in its right to add padding for alignment, in the middle of the array! It has no idea that this is an array, and there's nothing stopping the linker from messing it up. For those structs that are a power of 2 in size, there's no reason for the linker to do anything else, and it "just works". There's plenty of instances in the kernel that depend on this. I'm thinking that the sort algorithm either hid the problem or fixed it somehow (I'm guessing it hid the problem). I hit the same issue with trace event structures. The solution was to create the array of pointers to each structure, and dereference the structures from the array. See commit e4a9ea5ee ("tracing: Replace trace_event struct array with pointer array") -- Steve