From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Message-ID: <544ABB51.1040107@kernel.dk> Date: Fri, 24 Oct 2014 14:49:21 -0600 From: Jens Axboe MIME-Version: 1.0 Subject: Re: [PATCH] fio: fix alignement to prevent bus error on ARM References: <1414183357-31504-1-git-send-email-gwendal@chromium.org> In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit To: Mike Frysinger , Gwendal Grignou Cc: fio@vger.kernel.org, Puthikorn Voravootivat List-ID: On 10/24/2014 02:47 PM, Mike Frysinger wrote: > the code seems to be fine with gcc-isms, so i might suggest anon unions > instead to make it harder to break: > union { > uint64_t __aligner; > uint16_t continue_on_error; > }; It's not a bad idea... Your commit broke alignment further down, btw, but I fixed it up. I'll be happy to take anon union patches for the alignment parts :-) -- Jens Axboe From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: MIME-Version: 1.0 Sender: vapier@google.com In-Reply-To: <1414183357-31504-1-git-send-email-gwendal@chromium.org> References: <1414183357-31504-1-git-send-email-gwendal@chromium.org> From: Mike Frysinger Date: Fri, 24 Oct 2014 16:47:36 -0400 Message-ID: Subject: Re: [PATCH] fio: fix alignement to prevent bus error on ARM Content-Type: multipart/alternative; boundary=bcaec511e5faddf12c0506314b4e To: Gwendal Grignou Cc: axboe@kernel.dk, fio@vger.kernel.org, Puthikorn Voravootivat List-ID: --bcaec511e5faddf12c0506314b4e Content-Type: text/plain; charset=UTF-8 the code seems to be fine with gcc-isms, so i might suggest anon unions instead to make it harder to break: union { uint64_t __aligner; uint16_t continue_on_error; }; -mike On Fri, Oct 24, 2014 at 4:42 PM, Gwendal Grignou wrote: > Add a filler field to be ensure 64bit alignment. > Otherwise, we would trigger SIGBUS error in sum_stat() > > Signed-off-by: Gwendal Grignou > --- > stat.h | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/stat.h b/stat.h > index 32ea226..9595e59 100644 > --- a/stat.h > +++ b/stat.h > @@ -172,6 +172,7 @@ struct thread_stat { > * IO Error related stats > */ > uint16_t continue_on_error; > + uint16_t filler[3]; > uint64_t total_err_count; > uint32_t first_error; > > -- > 2.1.0.rc2.206.gedb03e5 > > --bcaec511e5faddf12c0506314b4e Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable
the code seems to be fine with gcc-isms, so i might sugges= t anon unions instead to make it harder to break:
union {
=C2= =A0 uint64_t __aligner;
=C2=A0 uint16_t continue_on_error;
<= div>};
-mike

On Fri, Oct 24, 2014 at 4:42 PM, Gwendal Grignou <gwen= dal@chromium.org> wrote:
Ad= d a filler field to be ensure 64bit alignment.
Otherwise, we would trigger SIGBUS error in sum_stat()

Signed-off-by: Gwendal Grignou <= gwendal@chromium.org>
---
=C2=A0stat.h | 1 +
=C2=A01 file changed, 1 insertion(+)

diff --git a/stat.h b/stat.h
index 32ea226..9595e59 100644
--- a/stat.h
+++ b/stat.h
@@ -172,6 +172,7 @@ struct thread_stat {
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0* IO Error related stats
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0*/
=C2=A0 =C2=A0 =C2=A0 =C2=A0 uint16_t continue_on_error;
+=C2=A0 =C2=A0 =C2=A0 =C2=A0uint16_t filler[3];
=C2=A0 =C2=A0 =C2=A0 =C2=A0 uint64_t total_err_count;
=C2=A0 =C2=A0 =C2=A0 =C2=A0 uint32_t first_error;

--
2.1.0.rc2.206.gedb03e5


--bcaec511e5faddf12c0506314b4e-- From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Message-ID: <544ABA12.9020804@kernel.dk> Date: Fri, 24 Oct 2014 14:44:02 -0600 From: Jens Axboe MIME-Version: 1.0 Subject: Re: [PATCH] fio: fix alignement to prevent bus error on ARM References: <1414183357-31504-1-git-send-email-gwendal@chromium.org> In-Reply-To: <1414183357-31504-1-git-send-email-gwendal@chromium.org> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit To: Gwendal Grignou Cc: fio@vger.kernel.org, vapier@chromium.org, puthik@chromium.org List-ID: On 10/24/2014 02:42 PM, Gwendal Grignou wrote: > Add a filler field to be ensure 64bit alignment. > Otherwise, we would trigger SIGBUS error in sum_stat() Thanks, applied. -- Jens Axboe From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Return-Path: From: Gwendal Grignou Subject: [PATCH] fio: fix alignement to prevent bus error on ARM Date: Fri, 24 Oct 2014 13:42:37 -0700 Message-Id: <1414183357-31504-1-git-send-email-gwendal@chromium.org> To: axboe@kernel.dk Cc: fio@vger.kernel.org, vapier@chromium.org, puthik@chromium.org, Gwendal Grignou List-ID: Add a filler field to be ensure 64bit alignment. Otherwise, we would trigger SIGBUS error in sum_stat() Signed-off-by: Gwendal Grignou --- stat.h | 1 + 1 file changed, 1 insertion(+) diff --git a/stat.h b/stat.h index 32ea226..9595e59 100644 --- a/stat.h +++ b/stat.h @@ -172,6 +172,7 @@ struct thread_stat { * IO Error related stats */ uint16_t continue_on_error; + uint16_t filler[3]; uint64_t total_err_count; uint32_t first_error; -- 2.1.0.rc2.206.gedb03e5 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Return-Path: From: Gwendal Grignou Subject: [PATCH v5] fio: Fix padding properly Date: Mon, 27 Oct 2014 09:50:35 -0700 Message-Id: <1414428635-1598-1-git-send-email-gwendal@chromium.org> In-Reply-To: <544ABB51.1040107@kernel.dk> References: <544ABB51.1040107@kernel.dk> To: axboe@kernel.dk Cc: fio@vger.kernel.org, vapier@chromium.org, puthik@chromium.org, Gwendal Grignou List-ID: Completely fix padding: - use anonymous union for padding - remove remaining padding in thread_stat. Signed-off-by: Gwendal Grignou --- libfio.c | 2 ++ stat.h | 9 ++++++--- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/libfio.c b/libfio.c index b823bd4..1abf39a 100644 --- a/libfio.c +++ b/libfio.c @@ -305,6 +305,8 @@ int initialize_fio(char *envp[]) * access (ARM). */ compiletime_assert((offsetof(struct thread_stat, percentile_list) % 8) == 0, "stat percentile_list"); + compiletime_assert((offsetof(struct thread_stat, total_run_time) % 8) == 0, "total_run_time"); + compiletime_assert((offsetof(struct thread_stat, total_err_count) % 8) == 0, "total_err_count"); compiletime_assert((offsetof(struct thread_stat, latency_percentile) % 8) == 0, "stat latency_percentile"); compiletime_assert((offsetof(struct thread_options_pack, zipf_theta) % 8) == 0, "zipf_theta"); compiletime_assert((offsetof(struct thread_options_pack, pareto_h) % 8) == 0, "pareto_h"); diff --git a/stat.h b/stat.h index 16b3d1a..7a0dd55 100644 --- a/stat.h +++ b/stat.h @@ -158,6 +158,8 @@ struct thread_stat { uint32_t io_u_lat_u[FIO_IO_U_LAT_U_NR]; uint32_t io_u_lat_m[FIO_IO_U_LAT_M_NR]; uint32_t io_u_plat[DDIR_RWDIR_CNT][FIO_IO_U_PLAT_NR]; + uint32_t pad1; + uint64_t total_io_u[3]; uint64_t short_io_u[3]; uint64_t drop_io_u[3]; @@ -171,8 +173,10 @@ struct thread_stat { /* * IO Error related stats */ - uint16_t continue_on_error; - uint16_t filler[3]; + union { + uint16_t continue_on_error; + uint64_t pad2; + }; uint64_t total_err_count; uint32_t first_error; @@ -181,7 +185,6 @@ struct thread_stat { uint32_t latency_depth; uint64_t latency_target; - uint32_t pad; fio_fp64_t latency_percentile; uint64_t latency_window; } __attribute__((packed)); -- 2.1.0.rc2.206.gedb03e5 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Message-ID: <544E5BA4.1070505@kernel.dk> Date: Mon, 27 Oct 2014 08:50:12 -0600 From: Jens Axboe MIME-Version: 1.0 Subject: Re: [PATCH v4] fio: Fix padding properly References: <544ABB51.1040107@kernel.dk> <1414278272-14168-1-git-send-email-gwendal@chromium.org> In-Reply-To: <1414278272-14168-1-git-send-email-gwendal@chromium.org> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit To: Gwendal Grignou Cc: fio@vger.kernel.org, vapier@chromium.org, puthik@chromium.org List-ID: On 10/25/2014 05:04 PM, Gwendal Grignou wrote: > diff --git a/stat.h b/stat.h > index 16b3d1a..db83f65 100644 > --- a/stat.h > +++ b/stat.h > @@ -158,6 +158,8 @@ struct thread_stat { > uint32_t io_u_lat_u[FIO_IO_U_LAT_U_NR]; > uint32_t io_u_lat_m[FIO_IO_U_LAT_M_NR]; > uint32_t io_u_plat[DDIR_RWDIR_CNT][FIO_IO_U_PLAT_NR]; > + uint32_t pad; > + > uint64_t total_io_u[3]; > uint64_t short_io_u[3]; > uint64_t drop_io_u[3]; > @@ -171,8 +173,10 @@ struct thread_stat { > /* > * IO Error related stats > */ > - uint16_t continue_on_error; > - uint16_t filler[3]; > + union { > + uint16_t continue_on_error; > + uint64_t pad; > + }; > uint64_t total_err_count; > uint32_t first_error; > These two hunks are both in struct thread_stat, the compile wont be happy about the duplicate naming... -- Jens Axboe From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Return-Path: From: Gwendal Grignou Subject: [PATCH v4] fio: Fix padding properly Date: Sat, 25 Oct 2014 16:04:32 -0700 Message-Id: <1414278272-14168-1-git-send-email-gwendal@chromium.org> In-Reply-To: <544ABB51.1040107@kernel.dk> References: <544ABB51.1040107@kernel.dk> To: axboe@kernel.dk Cc: fio@vger.kernel.org, vapier@chromium.org, puthik@chromium.org, Gwendal Grignou List-ID: Completely fix padding: - use anonymous union for padding. - move existing padding in thread_stat. - add alignment checks. Signed-off-by: Gwendal Grignou --- libfio.c | 2 ++ stat.h | 9 ++++++--- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/libfio.c b/libfio.c index b823bd4..1abf39a 100644 --- a/libfio.c +++ b/libfio.c @@ -305,6 +305,8 @@ int initialize_fio(char *envp[]) * access (ARM). */ compiletime_assert((offsetof(struct thread_stat, percentile_list) % 8) == 0, "stat percentile_list"); + compiletime_assert((offsetof(struct thread_stat, total_run_time) % 8) == 0, "total_run_time"); + compiletime_assert((offsetof(struct thread_stat, total_err_count) % 8) == 0, "total_err_count"); compiletime_assert((offsetof(struct thread_stat, latency_percentile) % 8) == 0, "stat latency_percentile"); compiletime_assert((offsetof(struct thread_options_pack, zipf_theta) % 8) == 0, "zipf_theta"); compiletime_assert((offsetof(struct thread_options_pack, pareto_h) % 8) == 0, "pareto_h"); diff --git a/stat.h b/stat.h index 16b3d1a..db83f65 100644 --- a/stat.h +++ b/stat.h @@ -158,6 +158,8 @@ struct thread_stat { uint32_t io_u_lat_u[FIO_IO_U_LAT_U_NR]; uint32_t io_u_lat_m[FIO_IO_U_LAT_M_NR]; uint32_t io_u_plat[DDIR_RWDIR_CNT][FIO_IO_U_PLAT_NR]; + uint32_t pad; + uint64_t total_io_u[3]; uint64_t short_io_u[3]; uint64_t drop_io_u[3]; @@ -171,8 +173,10 @@ struct thread_stat { /* * IO Error related stats */ - uint16_t continue_on_error; - uint16_t filler[3]; + union { + uint16_t continue_on_error; + uint64_t pad; + }; uint64_t total_err_count; uint32_t first_error; @@ -181,7 +185,6 @@ struct thread_stat { uint32_t latency_depth; uint64_t latency_target; - uint32_t pad; fio_fp64_t latency_percentile; uint64_t latency_window; } __attribute__((packed)); -- 2.1.0.rc2.206.gedb03e5 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from plane.gmane.org ([80.91.229.3]:42841 "EHLO plane.gmane.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754745AbaJXVkG (ORCPT ); Fri, 24 Oct 2014 17:40:06 -0400 Received: from list by plane.gmane.org with local (Exim 4.69) (envelope-from ) id 1XhmaZ-0001ZT-Hi for fio@vger.kernel.org; Fri, 24 Oct 2014 23:40:03 +0200 Received: from 216.239.45.82 ([216.239.45.82]) by main.gmane.org with esmtp (Gmexim 0.1 (Debian)) id 1AlnuQ-0007hv-00 for ; Fri, 24 Oct 2014 23:40:03 +0200 Received: from gwendal by 216.239.45.82 with local (Gmexim 0.1 (Debian)) id 1AlnuQ-0007hv-00 for ; Fri, 24 Oct 2014 23:40:03 +0200 From: Gwendal Grignou Subject: Re: [PATCH v2] fio: Fix padding properly Date: Fri, 24 Oct 2014 21:21:16 +0000 (UTC) Message-ID: References: <544ABB51.1040107@kernel.dk> <1414184783-10472-1-git-send-email-gwendal@chromium.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Sender: fio-owner@vger.kernel.org List-Id: fio@vger.kernel.org To: fio@vger.kernel.org Gwendal Grignou writes: > > Completely fix padding: Ignore this patch, I did not properly test it. Gwendal. From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Return-Path: From: Gwendal Grignou Subject: [PATCH v3] fio: Fix padding properly Date: Fri, 24 Oct 2014 14:32:59 -0700 Message-Id: <1414186379-24282-1-git-send-email-gwendal@chromium.org> In-Reply-To: <544ABB51.1040107@kernel.dk> References: <544ABB51.1040107@kernel.dk> To: axboe@kernel.dk Cc: fio@vger.kernel.org, vapier@chromium.org, puthik@chromium.org, Gwendal Grignou List-ID: Completely fix padding: - use anonymous union for padding - remove remaining padding in thread_stat. Signed-off-by: Gwendal Grignou --- stat.h | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/stat.h b/stat.h index 16b3d1a..c2c30b2 100644 --- a/stat.h +++ b/stat.h @@ -171,8 +171,10 @@ struct thread_stat { /* * IO Error related stats */ - uint16_t continue_on_error; - uint16_t filler[3]; + union { + uint16_t continue_on_error; + uint64_t pad; + }; uint64_t total_err_count; uint32_t first_error; @@ -181,7 +183,6 @@ struct thread_stat { uint32_t latency_depth; uint64_t latency_target; - uint32_t pad; fio_fp64_t latency_percentile; uint64_t latency_window; } __attribute__((packed)); -- 2.1.0.rc2.206.gedb03e5 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Return-Path: From: Gwendal Grignou Subject: [PATCH v2] fio: Fix padding properly Date: Fri, 24 Oct 2014 14:06:23 -0700 Message-Id: <1414184783-10472-1-git-send-email-gwendal@chromium.org> In-Reply-To: <544ABB51.1040107@kernel.dk> References: <544ABB51.1040107@kernel.dk> To: axboe@kernel.dk Cc: fio@vger.kernel.org, vapier@chromium.org, puthik@chromium.org, Gwendal Grignou List-ID: Completely fix padding: - use anonymous union for padding - remove remaining padding in thread_stat. Signed-off-by: Gwendal Grignou --- stat.h | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/stat.h b/stat.h index 16b3d1a..1257835 100644 --- a/stat.h +++ b/stat.h @@ -171,8 +171,10 @@ struct thread_stat { /* * IO Error related stats */ - uint16_t continue_on_error; - uint16_t filler[3]; + union { + uint16_t continue_on_error; + uint64_t pad; + } uint64_t total_err_count; uint32_t first_error; @@ -181,7 +183,6 @@ struct thread_stat { uint32_t latency_depth; uint64_t latency_target; - uint32_t pad; fio_fp64_t latency_percentile; uint64_t latency_window; } __attribute__((packed)); -- 2.1.0.rc2.206.gedb03e5