Greg,
The patch is only compile-tested.
[..]
diff --git a/drivers/staging/batman-adv/main.c b/drivers/staging/batman-adv/main.c index 0587940..6ea6420 100644 --- a/drivers/staging/batman-adv/main.c +++ b/drivers/staging/batman-adv/main.c @@ -149,7 +149,7 @@ void dec_module_count(void)
int compare_orig(void *data1, void *data2) {
- return (memcmp(data1, data2, ETH_ALEN) == 0 ? 1 : 0);
- return (compare_ether_addr(data1, data2) == 0 ? 1 : 0);
}
I'm wondering why you accepted this patch despite the raised objections regarding alignment problems. [1][2] Is there something you know that we overlooked ? We haven't heard from Thomas or you since the alignment question came on the table.
Cheers, Marek
[1]https://lists.open-mesh.org/pipermail/b.a.t.m.a.n/2010-November/003640.html [2]https://lists.open-mesh.org/pipermail/b.a.t.m.a.n/2010-November/003643.html
On Wed, Nov 10, 2010 at 11:17:03AM +0100, Marek Lindner wrote:
Greg,
The patch is only compile-tested.
[..]
diff --git a/drivers/staging/batman-adv/main.c b/drivers/staging/batman-adv/main.c index 0587940..6ea6420 100644 --- a/drivers/staging/batman-adv/main.c +++ b/drivers/staging/batman-adv/main.c @@ -149,7 +149,7 @@ void dec_module_count(void)
int compare_orig(void *data1, void *data2) {
- return (memcmp(data1, data2, ETH_ALEN) == 0 ? 1 : 0);
- return (compare_ether_addr(data1, data2) == 0 ? 1 : 0);
}
I'm wondering why you accepted this patch despite the raised objections regarding alignment problems. [1][2]
Because at the end of that thread, it sounded like you all agreed that this patch was acceptable.
If not, then please let me know and I will revert it.
thanks,
greg k-h
On Wednesday 10 November 2010 17:04:55 Greg KH wrote:
I'm wondering why you accepted this patch despite the raised objections regarding alignment problems. [1][2]
Because at the end of that thread, it sounded like you all agreed that this patch was acceptable.
If not, then please let me know and I will revert it.
The end of the thread was that he should remove parts of the patch and resent it - or prove that all the data is 2 bytes aligned.
On Wednesday 03 November 2010 11:56:19 Sven Eckelmann wrote: [...]
I don't think they need to be two bytes aligned, but I might be wrong.
compare_ether_addr uses a two byte pointer to access 3x two bytes. This makes it necessary to have all those 3 bytes aligned to 2 byte boundaries.
[correction sent later: "6x two bytes aligned to 2 byte boundaries"]
Otherwise the compiler has to generate special instructions on architectures which don't support loads on non-aligned addresses. Usually he doesn't do it unless he has some indications that it is necessary (__attribute__ ((packed)) for example).
There is also documentation available on that topic in Documentation/unaligned-memory-access.txt
And maybe it is good to use is_broadcast_ether_addr, but leave compare_ether_addr part open (or prove that we always have those two operands correctly aligned).
[...]
None of that happened yet. And yes, this wasn't the last mail we sent to him to get some response - no answer till now.
Best regards, Sven
On Wed, Nov 10, 2010 at 05:46:37PM +0100, Sven Eckelmann wrote:
On Wednesday 10 November 2010 17:04:55 Greg KH wrote:
I'm wondering why you accepted this patch despite the raised objections regarding alignment problems. [1][2]
Because at the end of that thread, it sounded like you all agreed that this patch was acceptable.
If not, then please let me know and I will revert it.
The end of the thread was that he should remove parts of the patch and resent it - or prove that all the data is 2 bytes aligned.
On Wednesday 03 November 2010 11:56:19 Sven Eckelmann wrote: [...]
I don't think they need to be two bytes aligned, but I might be wrong.
compare_ether_addr uses a two byte pointer to access 3x two bytes. This makes it necessary to have all those 3 bytes aligned to 2 byte boundaries.
[correction sent later: "6x two bytes aligned to 2 byte boundaries"]
Otherwise the compiler has to generate special instructions on architectures which don't support loads on non-aligned addresses. Usually he doesn't do it unless he has some indications that it is necessary (__attribute__ ((packed)) for example).
There is also documentation available on that topic in Documentation/unaligned-memory-access.txt
And maybe it is good to use is_broadcast_ether_addr, but leave compare_ether_addr part open (or prove that we always have those two operands correctly aligned).
[...]
None of that happened yet. And yes, this wasn't the last mail we sent to him to get some response - no answer till now.
Oops, my fault, I've now reverted this patch, sorry about that.
thanks,
greg k-h
On 2010-11-10 at 17:46:37 +0100, Sven Eckelmann sven.eckelmann@gmx.de wrote:
compare_ether_addr uses a two byte pointer to access 3x two bytes. This makes it necessary to have all those 3 bytes aligned to 2 byte boundaries.
[correction sent later: "6x two bytes aligned to 2 byte boundaries"]
Otherwise the compiler has to generate special instructions on architectures which don't support loads on non-aligned addresses. Usually he doesn't do it unless he has some indications that it is necessary (__attribute__ ((packed)) for example).
There is also documentation available on that topic in Documentation/unaligned-memory-access.txt
And maybe it is good to use is_broadcast_ether_addr, but leave compare_ether_addr part open (or prove that we always have those two operands correctly aligned).
[...]
None of that happened yet. And yes, this wasn't the last mail we sent to him to get some response - no answer till now.
I'll send an updated patch, leaving out the compare_ether_addr part.
Cheers Tobias
On Thursday 11 November 2010 02:11:37 Greg KH wrote:
None of that happened yet. And yes, this wasn't the last mail we sent to him to get some response - no answer till now.
Oops, my fault, I've now reverted this patch, sorry about that.
No problem, that can happen.
Thanks, Marek
Tobias,
I'll send an updated patch, leaving out the compare_ether_addr part.
not necessary anymore. Today I included a patch from Sven that does the job.
Thanks for bringing this up, Marek
b.a.t.m.a.n@lists.open-mesh.org