[PATCH 06/19] dlm: align midcomms message buffer

[PATCH 06/19] dlm: align midcomms message buffer

[David Teigland]

From: Fabio M. Di Nitto <fabbione@ubuntu.com>

gcc does not guarantee that a static buffer is 64bit aligned. This change
allows sparc64 to work.

Signed-off-by: Fabio M. Di Nitto <fabbione@ubuntu.com>
Signed-off-by: David Teigland <teigland@redhat.com>
---
 fs/dlm/midcomms.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/fs/dlm/midcomms.c b/fs/dlm/midcomms.c
index f8c69dd..da653b5 100644
--- a/fs/dlm/midcomms.c
+++ b/fs/dlm/midcomms.c
@@ -58,7 +58,7 @@ static void copy_from_cb(void *dst, const void *base, unsigned offset,
 int dlm_process_incoming_buffer(int nodeid, const void *base,
 				unsigned offset, unsigned len, unsigned limit)
 {
-	unsigned char __tmp[DLM_INBUF_LEN];
+	unsigned char __tmp[DLM_INBUF_LEN] __attribute__((aligned(64)));
 	struct dlm_header *msg = (struct dlm_header *) __tmp;
 	int ret = 0;
 	int err = 0;
-- 
1.5.3.3

Re: [PATCH 06/19] dlm: align midcomms message buffer

[Andrew Morton]

> On Thu, 24 Jan 2008 10:50:29 -0600 David Teigland <teigland@redhat.com> wrote:
> From: Fabio M. Di Nitto <fabbione@ubuntu.com>
> 
> gcc does not guarantee that a static buffer is 64bit aligned. This change
> allows sparc64 to work.
> 

This buffer is not static: changelog needs fixing: s/static/auto/

> ---
>  fs/dlm/midcomms.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/fs/dlm/midcomms.c b/fs/dlm/midcomms.c
> index f8c69dd..da653b5 100644
> --- a/fs/dlm/midcomms.c
> +++ b/fs/dlm/midcomms.c
> @@ -58,7 +58,7 @@ static void copy_from_cb(void *dst, const void *base, unsigned offset,
>  int dlm_process_incoming_buffer(int nodeid, const void *base,
>  				unsigned offset, unsigned len, unsigned limit)
>  {
> -	unsigned char __tmp[DLM_INBUF_LEN];
> +	unsigned char __tmp[DLM_INBUF_LEN] __attribute__((aligned(64)));
>  	struct dlm_header *msg = (struct dlm_header *) __tmp;
>  	int ret = 0;
>  	int err = 0;

Why does DLM require that this thing be 64-bit aligned?

It all looks rather ugly.  Can't this stuff be implemeted within the C type
system somehow?

Re: [PATCH 06/19] dlm: align midcomms message buffer

[Fabio M. Di Nitto]

On Sat, 26 Jan 2008, Andrew Morton wrote:

>> On Thu, 24 Jan 2008 10:50:29 -0600 David Teigland <teigland@redhat.com> wrote:
>> From: Fabio M. Di Nitto <fabbione@ubuntu.com>
>>
>> gcc does not guarantee that a static buffer is 64bit aligned. This change
>> allows sparc64 to work.
>>
>
> This buffer is not static: changelog needs fixing: s/static/auto/

thanks

>
>> ---
>>  fs/dlm/midcomms.c |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/fs/dlm/midcomms.c b/fs/dlm/midcomms.c
>> index f8c69dd..da653b5 100644
>> --- a/fs/dlm/midcomms.c
>> +++ b/fs/dlm/midcomms.c
>> @@ -58,7 +58,7 @@ static void copy_from_cb(void *dst, const void *base, unsigned offset,
>>  int dlm_process_incoming_buffer(int nodeid, const void *base,
>>  				unsigned offset, unsigned len, unsigned limit)
>>  {
>> -	unsigned char __tmp[DLM_INBUF_LEN];
>> +	unsigned char __tmp[DLM_INBUF_LEN] __attribute__((aligned(64)));
>>  	struct dlm_header *msg = (struct dlm_header *) __tmp;
>>  	int ret = 0;
>>  	int err = 0;
>
> Why does DLM require that this thing be 64-bit aligned?

i spotted this running on sparc64 and the kernel was both complaning about 
TSP not being aligned and the data on the other side of the wire were 
wrong.

> It all looks rather ugly.  Can't this stuff be implemeted within the C type
> system somehow?

Yes, i will rework it.

Thanks for the feedback.

Fabio

--
I'm going to make him an offer he can't refuse.

Re: [PATCH 06/19] dlm: align midcomms message buffer

[Fabio M. Di Nitto]

On Sat, 26 Jan 2008, Andrew Morton wrote:

>> On Thu, 24 Jan 2008 10:50:29 -0600 David Teigland <teigland@redhat.com> wrote:
>> From: Fabio M. Di Nitto <fabbione@ubuntu.com>
>>
>> gcc does not guarantee that a static buffer is 64bit aligned. This change
>> allows sparc64 to work.
>>
>
> This buffer is not static: changelog needs fixing: s/static/auto/
>
>> ---
>>  fs/dlm/midcomms.c |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/fs/dlm/midcomms.c b/fs/dlm/midcomms.c
>> index f8c69dd..da653b5 100644
>> --- a/fs/dlm/midcomms.c
>> +++ b/fs/dlm/midcomms.c
>> @@ -58,7 +58,7 @@ static void copy_from_cb(void *dst, const void *base, unsigned offset,
>>  int dlm_process_incoming_buffer(int nodeid, const void *base,
>>  				unsigned offset, unsigned len, unsigned limit)
>>  {
>> -	unsigned char __tmp[DLM_INBUF_LEN];
>> +	unsigned char __tmp[DLM_INBUF_LEN] __attribute__((aligned(64)));
>>  	struct dlm_header *msg = (struct dlm_header *) __tmp;
>>  	int ret = 0;
>>  	int err = 0;
>
> Why does DLM require that this thing be 64-bit aligned?
>
> It all looks rather ugly.  Can't this stuff be implemeted within the C type
> system somehow?
>

how about this one:

>From adfe3b0654583d34b0840d20a69e4306d5b98caf Mon Sep 17 00:00:00 2001
Message-Id: <adfe3b0654583d34b0840d20a69e4306d5b98caf.1201584946.git.fabbione@ubuntu.com>
From: Fabio M. Di Nitto <fabbione@ubuntu.com>
Date: Tue, 29 Jan 2008 06:35:20 +0100
Subject: [PATCH 1/1] dlm: align midcomms message buffer

gcc does not guarantee that an auto buffer is 64bit aligned.
This change allows sparc64 to work.

Signed-off-by: Fabio M. Di Nitto <fabbione@ubuntu.com>
---
:100644 100644 f8c69dd... 53b7af2... M	fs/dlm/midcomms.c
  fs/dlm/midcomms.c |   13 ++++++++-----
  1 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/fs/dlm/midcomms.c b/fs/dlm/midcomms.c
index f8c69dd..53b7af2 100644
--- a/fs/dlm/midcomms.c
+++ b/fs/dlm/midcomms.c
@@ -58,8 +58,11 @@ static void copy_from_cb(void *dst, const void *base, unsigned offset,
  int dlm_process_incoming_buffer(int nodeid, const void *base,
  				unsigned offset, unsigned len, unsigned limit)
  {
-	unsigned char __tmp[DLM_INBUF_LEN];
-	struct dlm_header *msg = (struct dlm_header *) __tmp;
+	union {
+		unsigned char __buf[DLM_INBUF_LEN];
+		struct dlm_header dlm;
+	} __tmp;
+	struct dlm_header *msg = &__tmp.dlm;
  	int ret = 0;
  	int err = 0;
  	uint16_t msglen;
@@ -100,8 +103,8 @@ int dlm_process_incoming_buffer(int nodeid, const void *base,
  		   in the buffer on the stack (which should work for most
  		   ordinary messages). */

-		if (msglen > sizeof(__tmp) &&
-		    msg == (struct dlm_header *) __tmp) {
+		if (msglen > DLM_INBUF_LEN &&
+		    msg == &__tmp.dlm) {
  			msg = kmalloc(dlm_config.ci_buffer_size, GFP_KERNEL);
  			if (msg == NULL)
  				return ret;
@@ -119,7 +122,7 @@ int dlm_process_incoming_buffer(int nodeid, const void *base,
  		dlm_receive_buffer(msg, nodeid);
  	}

-	if (msg != (struct dlm_header *) __tmp)
+	if (msg != &__tmp.dlm)
  		kfree(msg);

  	return err ? err : ret;
-- 
1.5.3.8


--
I'm going to make him an offer he can't refuse.