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

To: tech-kern%netbsd.org@localhost
Subject: Re: POINTER_ALIGNED_P (was: Re: CVS commit: src/sys)
From: christos%zoulas.com@localhost (Christos Zoulas)
Date: Thu, 18 Feb 2021 00:00:10 +0000 (UTC)
In article <5912CA9E-B4E7-423D-A45D-F4693D1C98F5%zoulas.com@localhost>,
Christos Zoulas  <christos%zoulas.com@localhost> wrote:
>-=-=-=-=-=-

Here's the final changes

- Make ALIGNED_POINTER use __alignof(t) instead of sizeof(t). This is more
  correct because it works with non-primitive types and provides the ABI
  alignment for the type the compiler will use.
- Remove all the *_HDR_ALIGNMENT macros and asserts
- Replace POINTER_ALIGNED_P with ACCESSIBLE_POINTER which is identical to
  ALIGNED_POINTER, but returns that the pointer is always aligned if the
  CPU supports unaligned accesses.

Index: sys/mbuf.h
===================================================================
RCS file: /cvsroot/src/sys/sys/mbuf.h,v
retrieving revision 1.231
diff -u -u -r1.231 mbuf.h
--- sys/mbuf.h	17 Feb 2021 22:32:04 -0000	1.231
+++ sys/mbuf.h	17 Feb 2021 23:54:31 -0000
@@ -843,15 +843,21 @@
 	m->m_pkthdr.rcvif_index = n->m_pkthdr.rcvif_index;
 }
 
+#define M_GET_ALIGNED_HDR(m, type, linkhdr) \
+    m_get_aligned_hdr((m), __alignof(type) - 1, sizeof(type), (linkhdr))
+
 static __inline int
-m_get_aligned_hdr(struct mbuf **m, int align, size_t hlen, bool linkhdr)
+m_get_aligned_hdr(struct mbuf **m, int mask, size_t hlen, bool linkhdr)
 {
-	if (POINTER_ALIGNED_P(mtod(*m, void *), align) == 0) {
-		--align;	// Turn into mask
+#ifndef __NO_STRICT_ALIGNMENT
+	if (((uintptr_t)mtod(*m, void *) & mask) != 0)
 		*m = m_copyup(*m, hlen, 
-		      linkhdr ? (max_linkhdr + align) & ~align : 0);
-	} else if (__predict_false((size_t)(*m)->m_len < hlen))
+		      linkhdr ? (max_linkhdr + mask) & ~mask : 0);
+	else
+#endif
+	if (__predict_false((size_t)(*m)->m_len < hlen))
 		*m = m_pullup(*m, hlen);
+
 	return *m == NULL;
 }
 
Index: sys/param.h
===================================================================
RCS file: /cvsroot/src/sys/sys/param.h,v
retrieving revision 1.689
diff -u -u -r1.689 param.h
--- sys/param.h	17 Feb 2021 22:32:04 -0000	1.689
+++ sys/param.h	17 Feb 2021 23:54:31 -0000
@@ -281,16 +281,24 @@
 #define	ALIGN(p)		(((uintptr_t)(p) + ALIGNBYTES) & ~ALIGNBYTES)
 #endif
 #ifndef ALIGNED_POINTER
-#define	ALIGNED_POINTER(p,t)	((((uintptr_t)(p)) & (sizeof(t) - 1)) == 0)
+#define	ALIGNED_POINTER(p,t)	((((uintptr_t)(p)) & (__alignof(t) - 1)) == 0)
 #endif
 #ifndef ALIGNED_POINTER_LOAD
 #define	ALIGNED_POINTER_LOAD(q,p,t)	(*(q) = *((const t *)(p)))
 #endif
 
+/*
+ * Return if the pointer p is accessible for type t. For primitive types
+ * this means that the pointer itself can be dereferenced; for structures
+ * and unions this means that any field can be dereferenced. On CPUs
+ * that allow unaligned pointer access, we always return that the pointer
+ * is accessible to prevent unnecessary copies, although this might not be
+ * necessarily faster.
+ */
 #ifdef __NO_STRICT_ALIGNMENT
-#define	POINTER_ALIGNED_P(p, a)		1
+#define	ACCESSIBLE_POINTER(p, t)	1
 #else
-#define	POINTER_ALIGNED_P(p, a)		(((uintptr_t)(p) & ((a) - 1)) == 0)
+#define	ACCESSIBLE_POINTER(p, t)	ALIGNED_POINTER(p, t)
 #endif
 
 /*
Index: net/if_arp.h
===================================================================
RCS file: /cvsroot/src/sys/net/if_arp.h,v
retrieving revision 1.42
diff -u -u -r1.42 if_arp.h
--- net/if_arp.h	17 Feb 2021 22:32:04 -0000	1.42
+++ net/if_arp.h	17 Feb 2021 23:54:31 -0000
@@ -72,8 +72,6 @@
 	uint8_t  ar_tpa[];	/* target protocol address */
 #endif
 };
-#define	ARP_HDR_ALIGNMENT	__alignof(struct arphdr)
-__CTASSERT(ARP_HDR_ALIGNMENT == 2);
 
 static __inline uint8_t *
 ar_data(struct arphdr *ap)
Index: net/if_bridge.c
===================================================================
RCS file: /cvsroot/src/sys/net/if_bridge.c,v
retrieving revision 1.178
diff -u -u -r1.178 if_bridge.c
--- net/if_bridge.c	14 Feb 2021 20:58:34 -0000	1.178
+++ net/if_bridge.c	17 Feb 2021 23:54:31 -0000
@@ -2806,7 +2806,7 @@
 	if (*mp == NULL)
 		return -1;
 
-	if (m_get_aligned_hdr(&m, IP_HDR_ALIGNMENT, sizeof(*ip), true) != 0) {
+	if (M_GET_ALIGNED_HDR(&m, struct ip, true) != 0) {
 		/* XXXJRT new stat, please */
 		ip_statinc(IP_STAT_TOOSMALL);
 		goto bad;
@@ -2900,7 +2900,7 @@
 	 * it.  Otherwise, if it is aligned, make sure the entire base
 	 * IPv6 header is in the first mbuf of the chain.
 	 */
-	if (m_get_aligned_hdr(&m, IP6_HDR_ALIGNMENT, sizeof(*ip6), true) != 0) {
+	if (M_GET_ALIGNED_HDR(&m, struct ip6_hdr, true) != 0) {
 		struct ifnet *inifp = m_get_rcvif_NOMPSAFE(m);
 		/* XXXJRT new stat, please */
 		ip6_statinc(IP6_STAT_TOOSMALL);
Index: netinet/if_arp.c
===================================================================
RCS file: /cvsroot/src/sys/netinet/if_arp.c,v
retrieving revision 1.306
diff -u -u -r1.306 if_arp.c
--- netinet/if_arp.c	16 Feb 2021 10:22:52 -0000	1.306
+++ netinet/if_arp.c	17 Feb 2021 23:54:31 -0000
@@ -706,7 +706,7 @@
 				goto badlen;
 		}
 		ar = mtod(m, struct arphdr *);
-		KASSERT(POINTER_ALIGNED_P(ar, ARP_HDR_ALIGNMENT));
+		KASSERT(ACCESSIBLE_POINTER(ar, struct arphdr));
 
 		rcvif = m_get_rcvif(m, &s);
 		if (__predict_false(rcvif == NULL)) {
@@ -735,7 +735,7 @@
 			if ((m = m_pullup(m, arplen)) == NULL)
 				goto badlen;
 			ar = mtod(m, struct arphdr *);
-			KASSERT(POINTER_ALIGNED_P(ar, ARP_HDR_ALIGNMENT));
+			KASSERT(ACCESSIBLE_POINTER(ar, struct arphdr));
 		}
 
 		switch (ntohs(ar->ar_pro)) {
Index: netinet/in_l2tp.c
===================================================================
RCS file: /cvsroot/src/sys/netinet/in_l2tp.c,v
retrieving revision 1.19
diff -u -u -r1.19 in_l2tp.c
--- netinet/in_l2tp.c	14 Feb 2021 20:58:35 -0000	1.19
+++ netinet/in_l2tp.c	17 Feb 2021 23:54:31 -0000
@@ -197,8 +197,7 @@
 		error = ENOBUFS;
 		goto out;
 	}
-	if (m_get_aligned_hdr(&m, IP_HDR_ALIGNMENT, sizeof(iphdr), false) != 0)
-	{
+	if (M_GET_ALIGNED_HDR(&m, struct ip, false) != 0) {
 		error = ENOBUFS;
 		goto out;
 	}
Index: netinet/ip_flow.c
===================================================================
RCS file: /cvsroot/src/sys/netinet/ip_flow.c,v
retrieving revision 1.84
diff -u -u -r1.84 ip_flow.c
--- netinet/ip_flow.c	15 Feb 2021 03:41:01 -0000	1.84
+++ netinet/ip_flow.c	17 Feb 2021 23:54:31 -0000
@@ -231,7 +231,7 @@
 	 * IP header with no option and valid version and length
 	 */
 	ip = mtod(m, struct ip *);
-	if (!POINTER_ALIGNED_P(ip, IP_HDR_ALIGNMENT)) {
+	if (!ACCESSIBLE_POINTER(ip, struct ip)) {
 		memcpy(&ip_store, mtod(m, const void *), sizeof(ip_store));
 		ip = &ip_store;
 	}
@@ -313,7 +313,7 @@
 	 *
 	 * XXX Use m_copyback_cow(9) here? --dyoung
 	 */
-	if (!POINTER_ALIGNED_P(mtod(m, void *), IP_HDR_ALIGNMENT))
+	if (!ACCESSIBLE_POINTER(mtod(m, void *), struct ip))
 		memcpy(mtod(m, void *), &ip_store, sizeof(ip_store));
 
 	/*
Index: netinet/ip_input.c
===================================================================
RCS file: /cvsroot/src/sys/netinet/ip_input.c,v
retrieving revision 1.398
diff -u -u -r1.398 ip_input.c
--- netinet/ip_input.c	14 Feb 2021 20:58:35 -0000	1.398
+++ netinet/ip_input.c	17 Feb 2021 23:54:31 -0000
@@ -454,7 +454,7 @@
 	 * it.  Otherwise, if it is aligned, make sure the entire
 	 * base IP header is in the first mbuf of the chain.
 	 */
-	if (m_get_aligned_hdr(&m, IP_HDR_ALIGNMENT, sizeof(*ip), true) != 0) {
+	if (M_GET_ALIGNED_HDR(&m, struct ip, true) != 0) {
 		/* XXXJRT new stat, please */
 		IP_STATINC(IP_STAT_TOOSMALL);
 		goto out;
Index: netinet/ip_private.h
===================================================================
RCS file: /cvsroot/src/sys/netinet/ip_private.h,v
retrieving revision 1.5
diff -u -u -r1.5 ip_private.h
--- netinet/ip_private.h	17 Feb 2021 22:32:04 -0000	1.5
+++ netinet/ip_private.h	17 Feb 2021 23:54:31 -0000
@@ -43,8 +43,6 @@
 #define	IP_STATINC(x)		_NET_STATINC(ipstat_percpu, x)
 #define	IP_STATDEC(x)		_NET_STATDEC(ipstat_percpu, x)
 
-#define	IP_HDR_ALIGNMENT	__alignof(struct ip)
-__CTASSERT(IP_HDR_ALIGNMENT == 4);
 #endif /* _KERNEL */
 
 #endif /* !_NETINET_IP_PRIVATE_H_ */
Index: netinet/tcp_input.c
===================================================================
RCS file: /cvsroot/src/sys/netinet/tcp_input.c,v
retrieving revision 1.425
diff -u -u -r1.425 tcp_input.c
--- netinet/tcp_input.c	14 Feb 2021 20:58:35 -0000	1.425
+++ netinet/tcp_input.c	17 Feb 2021 23:54:31 -0000
@@ -1274,7 +1274,7 @@
 	 * Enforce alignment requirements that are violated in
 	 * some cases, see kern/50766 for details.
 	 */
-	if (POINTER_ALIGNED_P(th, TCP_HDR_ALIGNMENT) == 0) {
+	if (ACCESSIBLE_POINTER(th, struct tcp_hdr) == 0) {
 		m = m_copyup(m, off + sizeof(struct tcphdr), 0);
 		if (m == NULL) {
 			TCP_STATINC(TCP_STAT_RCVSHORT);
@@ -1282,7 +1282,7 @@
 		}
 		th = (struct tcphdr *)(mtod(m, char *) + off);
 	}
-	KASSERT(POINTER_ALIGNED_P(th, TCP_HDR_ALIGNMENT));
+	KASSERT(ACCESSIBLE_POINTER(th, struct tcp_hdr));
 
 	/*
 	 * Get IP and TCP header.
@@ -1362,7 +1362,7 @@
 			TCP_STATINC(TCP_STAT_RCVSHORT);
 			return;
 		}
-		KASSERT(POINTER_ALIGNED_P(th, TCP_HDR_ALIGNMENT));
+		KASSERT(ACCESSIBLE_POINTER(th, struct tcp_hdr));
 		optlen = thlen - sizeof(struct tcphdr);
 		optp = ((u_int8_t *)th) + sizeof(struct tcphdr);
 
Index: netinet/tcp_private.h
===================================================================
RCS file: /cvsroot/src/sys/netinet/tcp_private.h,v
retrieving revision 1.5
diff -u -u -r1.5 tcp_private.h
--- netinet/tcp_private.h	17 Feb 2021 22:32:04 -0000	1.5
+++ netinet/tcp_private.h	17 Feb 2021 23:54:31 -0000
@@ -43,8 +43,6 @@
 #define	TCP_STATINC(x)		_NET_STATINC(tcpstat_percpu, x)
 #define	TCP_STATADD(x, v)	_NET_STATADD(tcpstat_percpu, x, v)
 
-#define	TCP_HDR_ALIGNMENT	__alignof(struct tcphdr)
-__CTASSERT(TCP_HDR_ALIGNMENT == 4);
 #endif /* _KERNEL */
 
 #endif /* !_NETINET_TCP_PRIVATE_H_ */
Index: netinet/udp_private.h
===================================================================
RCS file: /cvsroot/src/sys/netinet/udp_private.h,v
retrieving revision 1.5
diff -u -u -r1.5 udp_private.h
--- netinet/udp_private.h	17 Feb 2021 22:32:04 -0000	1.5
+++ netinet/udp_private.h	17 Feb 2021 23:54:31 -0000
@@ -39,8 +39,6 @@
 
 #define	UDP_STATINC(x)		_NET_STATINC(udpstat_percpu, x)
 
-#define	UDP_HDR_ALIGNMENT	__alignof(struct udphdr)
-__CTASSERT(UDP_HDR_ALIGNMENT == 2);
 #endif /* _KERNEL */
 
 #endif /* !_NETINET_UDP_PRIVATE_H_ */
Index: netinet/udp_usrreq.c
===================================================================
RCS file: /cvsroot/src/sys/netinet/udp_usrreq.c,v
retrieving revision 1.260
diff -u -u -r1.260 udp_usrreq.c
--- netinet/udp_usrreq.c	14 Feb 2021 20:58:35 -0000	1.260
+++ netinet/udp_usrreq.c	17 Feb 2021 23:54:31 -0000
@@ -335,7 +335,7 @@
 	 * Enforce alignment requirements that are violated in
 	 * some cases, see kern/50766 for details.
 	 */
-	if (POINTER_ALIGNED_P(uh, UDP_HDR_ALIGNMENT) == 0) {
+	if (ACCESSIBLE_POINTER(uh, struct udphdr) == 0) {
 		m = m_copyup(m, iphlen + sizeof(struct udphdr), 0);
 		if (m == NULL) {
 			UDP_STATINC(UDP_STAT_HDROPS);
@@ -344,7 +344,7 @@
 		ip = mtod(m, struct ip *);
 		uh = (struct udphdr *)(mtod(m, char *) + iphlen);
 	}
-	KASSERT(POINTER_ALIGNED_P(uh, UDP_HDR_ALIGNMENT));
+	KASSERT(ACCESSIBLE_POINTER(uh, struct udphdr));
 
 	/* destination port of 0 is illegal, based on RFC768. */
 	if (uh->uh_dport == 0)
Index: netinet6/icmp6.c
===================================================================
RCS file: /cvsroot/src/sys/netinet6/icmp6.c,v
retrieving revision 1.249
diff -u -u -r1.249 icmp6.c
--- netinet6/icmp6.c	15 Feb 2021 10:13:45 -0000	1.249
+++ netinet6/icmp6.c	17 Feb 2021 23:54:31 -0000
@@ -538,7 +538,7 @@
 	 * Enforce alignment requirements that are violated in
 	 * some cases, see kern/50766 for details.
 	 */
-	if (POINTER_ALIGNED_P(icmp6, IP6_HDR_ALIGNMENT) == 0) {
+	if (ACCESSIBLE_POINTER(icmp6, struct ip6_hdr) == 0) {
 		m = m_copyup(m, off + sizeof(struct icmp6_hdr), 0);
 		if (m == NULL) {
 			ICMP6_STATINC(ICMP6_STAT_TOOSHORT);
@@ -548,7 +548,7 @@
 		ip6 = mtod(m, struct ip6_hdr *);
 		icmp6 = (struct icmp6_hdr *)(mtod(m, char *) + off);
 	}
-	KASSERT(POINTER_ALIGNED_P(icmp6, IP6_HDR_ALIGNMENT));
+	KASSERT(ACCESSIBLE_POINTER(icmp6, struct ip6_hdr));
 
 	/*
 	 * calculate the checksum
Index: netinet6/in6_l2tp.c
===================================================================
RCS file: /cvsroot/src/sys/netinet6/in6_l2tp.c,v
retrieving revision 1.20
diff -u -u -r1.20 in6_l2tp.c
--- netinet6/in6_l2tp.c	14 Feb 2021 20:58:35 -0000	1.20
+++ netinet6/in6_l2tp.c	17 Feb 2021 23:54:31 -0000
@@ -193,8 +193,7 @@
 	M_PREPEND(m, sizeof(struct ip6_hdr), M_DONTWAIT);
 	if (m == NULL)
 		return ENOBUFS;
-	if (m_get_aligned_hdr(&m, IP6_HDR_ALIGNMENT, sizeof(ip6hdr),
-	    false) != 0)
+	if (M_GET_ALIGNED_HDR(&m, struct ip6_hdr, false) != 0)
 		return ENOBUFS;
 	memcpy(mtod(m, struct ip6_hdr *), &ip6hdr, sizeof(struct ip6_hdr));
 
Index: netinet6/ip6_flow.c
===================================================================
RCS file: /cvsroot/src/sys/netinet6/ip6_flow.c,v
retrieving revision 1.41
diff -u -u -r1.41 ip6_flow.c
--- netinet6/ip6_flow.c	14 Feb 2021 20:58:35 -0000	1.41
+++ netinet6/ip6_flow.c	17 Feb 2021 23:54:31 -0000
@@ -283,7 +283,7 @@
 	if ((m->m_flags & (M_BCAST|M_MCAST)) != 0)
 		goto out;
 
-	if (POINTER_ALIGNED_P(mtod(m, const void *), IP6_HDR_ALIGNMENT) == 0) {
+	if (ACCESSIBLE_POINTER(mtod(m, const void *), struct ip6_hdr) == 0) {
 		if ((m = m_copyup(m, sizeof(struct ip6_hdr),
 		    (max_linkhdr + 3) & ~3)) == NULL) {
 			ret = 1;
Index: netinet6/ip6_input.c
===================================================================
RCS file: /cvsroot/src/sys/netinet6/ip6_input.c,v
retrieving revision 1.223
diff -u -u -r1.223 ip6_input.c
--- netinet6/ip6_input.c	14 Feb 2021 20:58:35 -0000	1.223
+++ netinet6/ip6_input.c	17 Feb 2021 23:54:31 -0000
@@ -301,7 +301,7 @@
 	 * it.  Otherwise, if it is aligned, make sure the entire base
 	 * IPv6 header is in the first mbuf of the chain.
 	 */
-	if (m_get_aligned_hdr(&m, IP6_HDR_ALIGNMENT, sizeof(*ip6), true) != 0) {
+	if (M_GET_ALIGNED_HDR(&m, struct ip6_hdr, true) != 0) {
 		/* XXXJRT new stat, please */
 		IP6_STATINC(IP6_STAT_TOOSMALL);
 		in6_ifstat_inc(rcvif, ifs6_in_hdrerr);
@@ -602,7 +602,7 @@
 			rtcache_percpu_putref(ip6_forward_rt_percpu);
 			return;
 		}
-		KASSERT(POINTER_ALIGNED_P(hbh, IP6_HDR_ALIGNMENT));
+		KASSERT(ACCESSIBLE_POINTER(hbh, struct ip6_hdr));
 		nxt = hbh->ip6h_nxt;
 
 		/*
@@ -875,7 +875,7 @@
 		IP6_STATINC(IP6_STAT_TOOSHORT);
 		return -1;
 	}
-	KASSERT(POINTER_ALIGNED_P(hbh, IP6_HDR_ALIGNMENT));
+	KASSERT(ACCESSIBLE_POINTER(hbh, struct ip6_hdr));
 	off += hbhlen;
 	hbhlen -= sizeof(struct ip6_hbh);
 
@@ -1215,7 +1215,7 @@
 				IP6_STATINC(IP6_STAT_TOOSHORT);
 				return;
 			}
-			KASSERT(POINTER_ALIGNED_P(ip6e, IP6_HDR_ALIGNMENT));
+			KASSERT(ACCESSIBLE_POINTER(ip6e, struct ip6_hdr));
 
 			switch (nxt) {
 			case IPPROTO_DSTOPTS:
Index: netinet6/ip6_private.h
===================================================================
RCS file: /cvsroot/src/sys/netinet6/ip6_private.h,v
retrieving revision 1.5
diff -u -u -r1.5 ip6_private.h
--- netinet6/ip6_private.h	17 Feb 2021 22:32:04 -0000	1.5
+++ netinet6/ip6_private.h	17 Feb 2021 23:54:31 -0000
@@ -43,8 +43,6 @@
 #define	IP6_STATINC(x)		_NET_STATINC(ip6stat_percpu, x)
 #define	IP6_STATDEC(x)		_NET_STATDEC(ip6stat_percpu, x)
 
-#define	IP6_HDR_ALIGNMENT	__alignof(struct ip6_hdr)
-__CTASSERT(IP6_HDR_ALIGNMENT == 4);
 #endif /* _KERNEL */
 
 #endif /* !_NETINET_IP6_PRIVATE_H_ */
Index: netinet6/udp6_usrreq.c
===================================================================
RCS file: /cvsroot/src/sys/netinet6/udp6_usrreq.c,v
retrieving revision 1.149
diff -u -u -r1.149 udp6_usrreq.c
--- netinet6/udp6_usrreq.c	14 Feb 2021 20:58:35 -0000	1.149
+++ netinet6/udp6_usrreq.c	17 Feb 2021 23:54:31 -0000
@@ -665,7 +665,7 @@
 	 * Enforce alignment requirements that are violated in
 	 * some cases, see kern/50766 for details.
 	 */
-	if (POINTER_ALIGNED_P(uh, UDP_HDR_ALIGNMENT) == 0) {
+	if (ACCESSIBLE_POINTER(uh, struct udphdr) == 0) {
 		m = m_copyup(m, off + sizeof(struct udphdr), 0);
 		if (m == NULL) {
 			IP6_STATINC(IP6_STAT_TOOSHORT);
@@ -674,7 +674,7 @@
 		ip6 = mtod(m, struct ip6_hdr *);
 		uh = (struct udphdr *)(mtod(m, char *) + off);
 	}
-	KASSERT(POINTER_ALIGNED_P(uh, UDP_HDR_ALIGNMENT));
+	KASSERT(ACCESSIBLE_POINTER(uh, struct udphdr));
 	ulen = ntohs((u_short)uh->uh_ulen);
 
 	/*
Prev by Date: Re: fsync_range and O_RDONLY
Next by Date: Re: fsync_range and O_RDONLY
Previous by Thread: fsync_range and O_RDONLY
Next by Thread: fsync error reporting
Indexes:
Home | Main Index | Thread Index | Old Index