Re: POINTER_ALIGNED_P (was: Re: CVS commit: src/sys)

2021-02-16 Thread Valery Ushakov
On Tue, Feb 16, 2021 at 18:51:43 -0500, Christos Zoulas wrote: > On Feb 17, 2:20am, u...@stderr.spb.ru (Valery Ushakov) wrote: > -- Subject: Re: POINTER_ALIGNED_P (was: Re: CVS commit: src/sys) > > | Well, it was you who did the definion of ALIGNED_POINTER centralized > | and overridable :) > |

Re: POINTER_ALIGNED_P (was: Re: CVS commit: src/sys)

2021-02-16 Thread Valery Ushakov
On Wed, Feb 17, 2021 at 00:51:03 +0100, Roland Illig wrote: > 17.02.2021 00:25:10 Valery Ushakov : > > On Tue, Feb 16, 2021 at 23:54:46 +0100, Roland Illig wrote: > >> Yes, it does. That's what the "#undef __NO_STRICT_ALIGNMENT" in the > >> test is for. > >> > >> I intentionally placed it

Re: POINTER_ALIGNED_P (was: Re: CVS commit: src/sys)

2021-02-16 Thread Christos Zoulas
On Feb 17, 2:20am, u...@stderr.spb.ru (Valery Ushakov) wrote: -- Subject: Re: POINTER_ALIGNED_P (was: Re: CVS commit: src/sys) | Well, it was you who did the definion of ALIGNED_POINTER centralized | and overridable :) | | revision 1.400 | date: 2012-01-25 00:03:36 +0400; author: christos;

Re: POINTER_ALIGNED_P (was: Re: CVS commit: src/sys)

2021-02-16 Thread Roland Illig
17.02.2021 00:25:10 Valery Ushakov : > On Tue, Feb 16, 2021 at 23:54:46 +0100, Roland Illig wrote: >> Yes, it does.  That's what the "#undef __NO_STRICT_ALIGNMENT" in the >> test is for. >> >> I intentionally placed it between (which defines that >> macro on x86 and some other platforms) and

Re: POINTER_ALIGNED_P (was: Re: CVS commit: src/sys)

2021-02-16 Thread Roland Illig
On 16.02.2021 23:32, Jason Thorpe wrote: On Feb 16, 2021, at 1:56 PM, Roland Illig wrote: A downside of this test is that the macro from gets only evaluated at compile time of the test. The test therefore cannot cover future updates to without being reinstalled itself. But at least for

Re: POINTER_ALIGNED_P (was: Re: CVS commit: src/sys)

2021-02-16 Thread Valery Ushakov
On Tue, Feb 16, 2021 at 17:53:09 -0500, Christos Zoulas wrote: > In this case "type" is a struct and we have __alignof() to handle > it, but does this give the right answer? > > Also ALIGNED_POINTER is not conditional to __NO_STRICT_ALIGNMENT and > can be overriden (the opposite goes for

Re: POINTER_ALIGNED_P (was: Re: CVS commit: src/sys)

2021-02-16 Thread Valery Ushakov
On Tue, Feb 16, 2021 at 23:54:46 +0100, Roland Illig wrote: > On 16.02.2021 23:40, Valery Ushakov wrote: > > On Tue, Feb 16, 2021 at 22:56:19 +0100, Roland Illig wrote: > > > > > On 16.02.2021 19:46, Roy Marples wrote: > > > > I suggest we change POINTER_ALIGNED_P to accept the alignment value

Re: POINTER_ALIGNED_P (was: Re: CVS commit: src/sys)

2021-02-16 Thread Roland Illig
On 16.02.2021 23:40, Valery Ushakov wrote: On Tue, Feb 16, 2021 at 22:56:19 +0100, Roland Illig wrote: On 16.02.2021 19:46, Roy Marples wrote: I suggest we change POINTER_ALIGNED_P to accept the alignment value we want rather than the bitwise test we supply, like so: #define

Re: POINTER_ALIGNED_P (was: Re: CVS commit: src/sys)

2021-02-16 Thread Christos Zoulas
In this case "type" is a struct and we have __alignof() to handle it, but does this give the right answer? Also ALIGNED_POINTER is not conditional to __NO_STRICT_ALIGNMENT and can be overriden (the opposite goes for POINTER_ALIGNED_P) I am all for having one macro, but how can we satisfy all

Re: POINTER_ALIGNED_P (was: Re: CVS commit: src/sys)

2021-02-16 Thread Valery Ushakov
On Tue, Feb 16, 2021 at 22:56:19 +0100, Roland Illig wrote: > On 16.02.2021 19:46, Roy Marples wrote: > > I suggest we change POINTER_ALIGNED_P to accept the alignment value we > > want rather than the bitwise test we supply, like so: > > > > #define POINTER_ALIGNED_P(p, a) (((uintptr_t)(p) &

Re: POINTER_ALIGNED_P (was: Re: CVS commit: src/sys)

2021-02-16 Thread Jason Thorpe
> On Feb 16, 2021, at 1:56 PM, Roland Illig wrote: > > A downside of this test is that the macro from gets only > evaluated at compile time of the test. The test therefore cannot cover > future updates to without being reinstalled itself. But > at least for the release builds, it ensures a

Re: POINTER_ALIGNED_P (was: Re: CVS commit: src/sys)

2021-02-16 Thread Roland Illig
On 16.02.2021 19:46, Roy Marples wrote: I suggest we change POINTER_ALIGNED_P to accept the alignment value we want rather than the bitwise test we supply, like so: #define    POINTER_ALIGNED_P(p, a)    (((uintptr_t)(p) & ((a) - 1)) That's a good definition, clear, simple, obvious,

Re: CVS commit: src/sys

2021-02-16 Thread Christos Zoulas
Yes, I agree. I am going to change that. christos > On Feb 16, 2021, at 1:46 PM, Roy Marples wrote: > > Hi Christos > > On 14/02/2021 20:58, Christos Zoulas wrote: >> Module Name: src >> Committed By:christos >> Date:Sun Feb 14 20:58:35 UTC 2021 >> Modified Files: >>

Re: CVS commit: src/sys

2021-02-16 Thread Roy Marples
Hi Christos On 14/02/2021 20:58, Christos Zoulas wrote: Module Name:src Committed By: christos Date: Sun Feb 14 20:58:35 UTC 2021 Modified Files: src/sys/net: if_arp.h if_bridge.c src/sys/netinet: icmp_private.h if_arp.c igmp_var.h in_l2tp.c ip_flow.c

Re: CVS commit: src/sys/netinet

2021-02-16 Thread Martin Husemann
On Tue, Feb 16, 2021 at 09:29:15AM +, Roy Marples wrote: > In my testing on aarch64 and octeon (both of which I think are strict > alignment) neither need pullups nor copyups as the mbuf already has enough > and arphrd is aligned correctly already. Ah, we asserted too much alignment - indeed,

Re: CVS commit: src/sys/netinet

2021-02-16 Thread Roy Marples
On 16/02/2021 09:20, Martin Husemann wrote: On Tue, Feb 16, 2021 at 08:26:40AM +, Roy Marples wrote: Is that because ARP_HDR_ALIGNMENT is forcing 4 byte alignment? The KASSERT a few lines below triggerd, we need to be consistent. For the purposes of using just the header we define I'm

Re: CVS commit: src/sys/netinet

2021-02-16 Thread Martin Husemann
On Tue, Feb 16, 2021 at 08:26:40AM +, Roy Marples wrote: > Is that because ARP_HDR_ALIGNMENT is forcing 4 byte alignment? The KASSERT a few lines below triggerd, we need to be consistent. > For the purposes of using just the header we define I'm pretty sure we can > use 2 byte alignment and

Re: CVS commit: src/sys/netinet

2021-02-16 Thread Roy Marples
On 16/02/2021 05:44, Martin Husemann wrote: Module Name:src Committed By: martin Date: Tue Feb 16 05:44:14 UTC 2021 Modified Files: src/sys/netinet: if_arp.c Log Message: Undo previous backout: alignment is needed here. The reason for the previous backout was a