All of lore.kernel.org
 help / color / mirror / Atom feed
From: James Hogan <james.hogan@imgtec.com>
To: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: "H. Peter Anvin" <hpa@zytor.com>,
	linux-arch@vger.kernel.org, linux-kernel@vger.kernel.org,
	Arnd Bergmann <arnd@arndb.de>,
	Balbir Singh <bsingharora@gmail.com>,
	Ingo Molnar <mingo@kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Eric Paris <eparis@redhat.com>, Will Drewry <wad@chromium.org>,
	James Morris <james.l.morris@oracle.com>
Subject: Re: [PATCH v2 03/44] Add CONFIG_HAVE_64BIT_ALIGNED_STRUCT for taskstats
Date: Mon, 17 Dec 2012 09:51:04 +0000	[thread overview]
Message-ID: <50CEEB08.1080707@imgtec.com> (raw)
In-Reply-To: <CAMuHMdXpw+hWgcqDiFVdDTsFBEThjvB_Y0jcoZZHS-JGTBvYRg@mail.gmail.com>

On 10/12/12 12:55, Geert Uytterhoeven wrote:
> On Mon, Dec 10, 2012 at 11:22 AM, James Hogan <james.hogan@imgtec.com> wrote:
>> On 08/12/12 03:43, H. Peter Anvin wrote:
>>> On 12/05/2012 08:08 AM, James Hogan wrote:
>>>> On 64 bit architectures with no efficient unaligned access, taskstats
>>>> has to add some padding to a reply to prevent unaligned access warnings.
>>>> However this also needs to apply to 32 bit architectures with 64 bit
>>>> struct alignment such as metag (which has 64 bit memory accesses).
>>>
>>> Wait... 64-bit struct alignment on structures with only 32-bit members?
>>>  That might be... interesting... in a number of places...
>>
>> I'll rewrite the description as it's a bit misleading. On metag 64bit
>> struct alignment is required when it contains 64bit members, not if it
>> only contains 32bit members. Although metag is a 32bit arch, it can do
>> 64bit memory accesses which must be aligned.
> 
> The C alignment rules should take care of this automatically (struct alignment
> is the maximum alignment of its members).

Hi Geert,

Please see the comment in mk_reply in kernel/taskstats.c. The structure
is being serialised after 2 NLA headers and a pid which is why the extra
padding needs to be added manually.

Cheers
James

> 
> You only have to override this manually in cases like this, where
> 64-bit quantities
> are stored in char arrays:
> 
>  struct buffer_data_page {
>         u64              time_stamp;    /* page time stamp */
>         local_t          commit;        /* write committed index */
> -       unsigned char    data[];        /* data of buffer page */
> +       unsigned char    data[] RB_ALIGN_DATA;  /* data of buffer page */
>  };
> 
> (cfr. your other patch).
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds
> 

WARNING: multiple messages have this Message-ID (diff)
From: James Hogan <james.hogan@imgtec.com>
To: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: "H. Peter Anvin" <hpa@zytor.com>, <linux-arch@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>, Arnd Bergmann <arnd@arndb.de>,
	Balbir Singh <bsingharora@gmail.com>,
	Ingo Molnar <mingo@kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Eric Paris <eparis@redhat.com>, Will Drewry <wad@chromium.org>,
	James Morris <james.l.morris@oracle.com>
Subject: Re: [PATCH v2 03/44] Add CONFIG_HAVE_64BIT_ALIGNED_STRUCT for taskstats
Date: Mon, 17 Dec 2012 09:51:04 +0000	[thread overview]
Message-ID: <50CEEB08.1080707@imgtec.com> (raw)
In-Reply-To: <CAMuHMdXpw+hWgcqDiFVdDTsFBEThjvB_Y0jcoZZHS-JGTBvYRg@mail.gmail.com>

On 10/12/12 12:55, Geert Uytterhoeven wrote:
> On Mon, Dec 10, 2012 at 11:22 AM, James Hogan <james.hogan@imgtec.com> wrote:
>> On 08/12/12 03:43, H. Peter Anvin wrote:
>>> On 12/05/2012 08:08 AM, James Hogan wrote:
>>>> On 64 bit architectures with no efficient unaligned access, taskstats
>>>> has to add some padding to a reply to prevent unaligned access warnings.
>>>> However this also needs to apply to 32 bit architectures with 64 bit
>>>> struct alignment such as metag (which has 64 bit memory accesses).
>>>
>>> Wait... 64-bit struct alignment on structures with only 32-bit members?
>>>  That might be... interesting... in a number of places...
>>
>> I'll rewrite the description as it's a bit misleading. On metag 64bit
>> struct alignment is required when it contains 64bit members, not if it
>> only contains 32bit members. Although metag is a 32bit arch, it can do
>> 64bit memory accesses which must be aligned.
> 
> The C alignment rules should take care of this automatically (struct alignment
> is the maximum alignment of its members).

Hi Geert,

Please see the comment in mk_reply in kernel/taskstats.c. The structure
is being serialised after 2 NLA headers and a pid which is why the extra
padding needs to be added manually.

Cheers
James

> 
> You only have to override this manually in cases like this, where
> 64-bit quantities
> are stored in char arrays:
> 
>  struct buffer_data_page {
>         u64              time_stamp;    /* page time stamp */
>         local_t          commit;        /* write committed index */
> -       unsigned char    data[];        /* data of buffer page */
> +       unsigned char    data[] RB_ALIGN_DATA;  /* data of buffer page */
>  };
> 
> (cfr. your other patch).
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds
> 


  reply	other threads:[~2012-12-17  9:51 UTC|newest]

Thread overview: 151+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-12-05 16:08 [PATCH v2 00/44] Meta Linux Kernel Port James Hogan
2012-12-05 16:08 ` James Hogan
2012-12-05 16:08 ` [PATCH v2 01/44] asm-generic/io.h: remove asm/cacheflush.h include James Hogan
2012-12-05 16:08   ` James Hogan
2012-12-05 16:08 ` [PATCH v2 02/44] asm-generic/unistd.h: handle symbol prefixes in cond_syscall James Hogan
2012-12-05 16:08   ` James Hogan
2012-12-05 16:08 ` [PATCH v2 03/44] Add CONFIG_HAVE_64BIT_ALIGNED_STRUCT for taskstats James Hogan
2012-12-05 16:08   ` James Hogan
2012-12-08  3:43   ` H. Peter Anvin
2012-12-10 10:22     ` James Hogan
2012-12-10 10:22       ` James Hogan
2012-12-10 12:55       ` Geert Uytterhoeven
2012-12-17  9:51         ` James Hogan [this message]
2012-12-17  9:51           ` James Hogan
2012-12-17 19:11           ` David Miller
2012-12-05 16:08 ` [PATCH v2 04/44] trace/ring_buffer: handle 64bit aligned structs James Hogan
2012-12-05 16:08   ` James Hogan
2012-12-08  1:24   ` Steven Rostedt
2012-12-10 10:27     ` James Hogan
2012-12-10 10:27       ` James Hogan
2012-12-05 16:08 ` [PATCH v2 05/44] Revert some of "binfmt_elf: cleanups" James Hogan
2012-12-05 16:08   ` James Hogan
     [not found] ` <1354723742-6195-1-git-send-email-james.hogan-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
2012-12-05 16:08   ` [PATCH v2 06/44] of/vendor-prefixes: add Imagination Technologies James Hogan
2012-12-05 16:08     ` James Hogan
2012-12-05 16:08     ` James Hogan
2012-12-05 22:28     ` Grant Likely
2012-12-05 22:28       ` Grant Likely
2012-12-05 22:28       ` Grant Likely
2012-12-06  9:24       ` James Hogan
2012-12-06  9:24         ` James Hogan
2012-12-05 16:08 ` [PATCH v2 07/44] metag: Add MAINTAINERS entry James Hogan
2012-12-05 16:08   ` James Hogan
2012-12-05 16:08 ` [PATCH v2 08/44] metag: Headers for core arch constants James Hogan
2012-12-05 16:08   ` James Hogan
2012-12-05 16:08 ` [PATCH v2 09/44] metag: Header for core memory mapped registers James Hogan
2012-12-05 16:08   ` James Hogan
2012-12-05 16:08 ` [PATCH v2 10/44] metag: Boot James Hogan
2012-12-05 16:08   ` James Hogan
2012-12-05 16:08 ` [PATCH v2 11/44] metag; TBX header James Hogan
2012-12-05 16:08   ` James Hogan
2012-12-05 16:08 ` [PATCH v2 12/44] metag: TBX source James Hogan
2012-12-05 16:08   ` James Hogan
2012-12-05 18:53   ` Joe Perches
2012-12-06  9:35     ` James Hogan
2012-12-06  9:35       ` James Hogan
2012-12-06 12:59       ` Joe Perches
2012-12-06 15:03         ` James Hogan
2012-12-06 15:03           ` James Hogan
2012-12-05 16:08 ` [PATCH v2 13/44] metag: Cache/TLB handling James Hogan
2012-12-05 16:08   ` James Hogan
2012-12-05 16:08 ` [PATCH v2 14/44] metag: Memory management James Hogan
2012-12-05 16:08   ` James Hogan
2012-12-05 16:08 ` [PATCH v2 15/44] metag: Memory handling James Hogan
2012-12-05 16:08   ` James Hogan
2012-12-05 16:08 ` [PATCH v2 16/44] metag: Huge TLB James Hogan
2012-12-05 16:08   ` James Hogan
2012-12-05 16:08 ` [PATCH v2 17/44] metag: Highmem support James Hogan
2012-12-05 16:08   ` James Hogan
2012-12-05 16:08 ` [PATCH v2 18/44] metag: TCM support James Hogan
2012-12-05 16:08   ` James Hogan
2012-12-05 16:08 ` [PATCH v2 19/44] metag: Signal handling James Hogan
2012-12-05 16:08   ` James Hogan
2012-12-05 17:16   ` Al Viro
2012-12-06 11:17     ` James Hogan
2012-12-06 11:17       ` James Hogan
2012-12-06 22:09       ` [braindump][RFC] signals and syscall restarts (Re: [PATCH v2 19/44] metag: Signal handling) Al Viro
2012-12-08  7:44         ` Al Viro
2012-12-15 16:26           ` Jonas Bonn
2012-12-15 17:07             ` Al Viro
2012-12-08 18:14         ` Al Viro
2012-12-12  9:44           ` James Hogan
2012-12-12  9:44             ` James Hogan
2012-12-10 10:40         ` James Hogan
2012-12-10 10:40           ` James Hogan
2012-12-05 16:08 ` [PATCH v2 20/44] metag: Device tree James Hogan
2012-12-05 16:08   ` James Hogan
2012-12-05 16:08 ` [PATCH v2 21/44] metag: ptrace James Hogan
2012-12-05 16:08   ` James Hogan
2012-12-05 16:08 ` [PATCH v2 22/44] metag: Time keeping James Hogan
2012-12-05 16:08   ` James Hogan
2013-01-04 10:05   ` Vineet Gupta
2013-01-04 10:05     ` Vineet Gupta
2013-01-04 12:21     ` James Hogan
2013-01-04 12:21       ` James Hogan
2013-01-04 12:48       ` Vineet Gupta
2013-01-04 12:48         ` Vineet Gupta
2013-01-04 13:11         ` James Hogan
2013-01-04 13:11           ` James Hogan
2012-12-05 16:08 ` [PATCH v2 23/44] metag: Traps James Hogan
2012-12-05 16:08   ` James Hogan
2012-12-05 17:40   ` Al Viro
2012-12-06 11:43     ` James Hogan
2012-12-06 11:43       ` James Hogan
2012-12-05 16:08 ` [PATCH v2 24/44] metag: IRQ handling James Hogan
2012-12-05 16:08   ` James Hogan
2012-12-05 16:08 ` [PATCH v2 25/44] metag: System Calls James Hogan
2012-12-05 16:08   ` James Hogan
2012-12-05 16:08 ` [PATCH v2 26/44] metag: Scheduling/Process management James Hogan
2012-12-05 16:08   ` James Hogan
2012-12-05 16:08 ` [PATCH v2 27/44] metag: Module support James Hogan
2012-12-05 16:08   ` James Hogan
2012-12-05 16:08 ` [PATCH v2 28/44] metag: Atomics, locks and bitops James Hogan
2012-12-05 16:08   ` James Hogan
2012-12-05 16:08 ` [PATCH v2 29/44] metag: Basic documentation James Hogan
2012-12-05 16:08   ` James Hogan
2012-12-05 16:08 ` [PATCH v2 30/44] metag: SMP support James Hogan
2012-12-05 16:08   ` James Hogan
2012-12-05 16:08 ` [PATCH v2 31/44] metag: DMA James Hogan
2012-12-05 16:08   ` James Hogan
2012-12-05 16:08 ` [PATCH v2 32/44] metag: Optimised library functions James Hogan
2012-12-05 16:08   ` James Hogan
2012-12-05 16:08 ` [PATCH v2 33/44] metag: Stack unwinding James Hogan
2012-12-05 16:08   ` James Hogan
2012-12-05 16:08 ` [PATCH v2 34/44] metag: Various other headers James Hogan
2012-12-05 16:08   ` James Hogan
2012-12-05 16:08 ` [PATCH v2 35/44] mm: define VM_GROWSUP for CONFIG_METAG James Hogan
2012-12-05 16:08   ` James Hogan
2012-12-05 16:08 ` [PATCH v2 36/44] Add metag to various Kconfig dependency lists James Hogan
2012-12-05 16:08   ` James Hogan
2012-12-05 16:08 ` [PATCH v2 37/44] metag: Build infrastructure James Hogan
2012-12-05 16:08   ` James Hogan
2012-12-05 16:08 ` [PATCH v2 38/44] metag: Perf James Hogan
2012-12-05 16:08   ` James Hogan
2012-12-05 16:08 ` [PATCH v2 39/44] metag: ftrace support James Hogan
2012-12-05 16:08   ` James Hogan
2012-12-05 16:08 ` [PATCH v2 40/44] scripts/checkstack.pl: Add metag support James Hogan
2012-12-05 16:08   ` James Hogan
2012-12-05 16:08 ` [PATCH v2 41/44] metag: OProfile James Hogan
2012-12-05 16:08   ` James Hogan
2012-12-05 16:09 ` [PATCH v2 42/44] metag: Add JTAG Debug Adapter (DA) support James Hogan
2012-12-05 16:09   ` James Hogan
2012-12-05 16:09 ` [PATCH v2 43/44] tty/metag_da: Add metag DA TTY driver James Hogan
2012-12-05 16:09   ` James Hogan
2012-12-05 17:24   ` Alan Cox
2012-12-05 17:24     ` Alan Cox
2013-01-04 14:11     ` James Hogan
2013-01-04 14:11       ` James Hogan
2013-01-04 17:00       ` Alan Cox
2013-01-04 17:00         ` Alan Cox
2013-01-07 11:30         ` James Hogan
2013-01-07 11:30           ` James Hogan
2013-01-07 11:54           ` Alan Cox
2013-01-07 11:54             ` Alan Cox
2012-12-05 16:09 ` [PATCH v2 44/44] fs: imgdafs: Add IMG DAFS filesystem for metag James Hogan
2012-12-05 16:09   ` James Hogan
2012-12-05 17:11 ` [PATCH v2 00/44] Meta Linux Kernel Port Al Viro
2012-12-05 18:39   ` Al Viro
2012-12-18 16:09     ` James Hogan
2012-12-18 16:09       ` James Hogan
2012-12-06  9:19   ` James Hogan
2012-12-06  9:19     ` James Hogan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=50CEEB08.1080707@imgtec.com \
    --to=james.hogan@imgtec.com \
    --cc=akpm@linux-foundation.org \
    --cc=arnd@arndb.de \
    --cc=bsingharora@gmail.com \
    --cc=eparis@redhat.com \
    --cc=geert@linux-m68k.org \
    --cc=hpa@zytor.com \
    --cc=james.l.morris@oracle.com \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=wad@chromium.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.