On Fri, Feb 04, 2022 at 11:39:56AM +0100, Alexander Bluhm wrote: > On Fri, Feb 04, 2022 at 07:32:52PM +1000, David Gwynne wrote: > > as discussed in "m_pullup alingment crash armv7 sparc64", at worst it > > doesnt hurt to have m_pullup maintain the data alignment of payloads, > > and at best it will encourage aligned loads even if the arch allows > > unaligned accesses. aligned loads are faster than unaligned. > > > > ok? > > adj is unsigned int, so assigning a mtod(m0, unsigned long) looks > strange. Of course the higher bits are cut off anyway, but an > explicit & is clearer than a assingment with different types. > > Please use > > adj = mtod(m, unsigned long) & (sizeof(long) - 1); > adj = mtod(m0, unsigned long) & (sizeof(long) - 1); > > otherwise OK bluhm@
it's been pointed out to me that ALIGNBYTES is actually very conservative about alignment rather than optimistic. ie, it's at least sizeof(long) - 1, but can be bigger. i think i've been confusing it with ALIGNED_POINTER. > > > Index: uipc_mbuf.c > > =================================================================== > > RCS file: /cvs/src/sys/kern/uipc_mbuf.c,v > > retrieving revision 1.280 > > diff -u -p -r1.280 uipc_mbuf.c > > --- uipc_mbuf.c 18 Jan 2022 12:38:21 -0000 1.280 > > +++ uipc_mbuf.c 4 Feb 2022 09:30:02 -0000 > > @@ -945,9 +945,11 @@ m_pullup(struct mbuf *m0, int len) > > goto freem0; > > } > > > > - adj = mtod(m, unsigned long) & ALIGNBYTES; > > + adj = mtod(m, unsigned long); > > } else > > - adj = mtod(m0, unsigned long) & ALIGNBYTES; > > + adj = mtod(m0, unsigned long); > > + > > + adj &= sizeof(long) - 1; > > > > tail = head + M_SIZE(m0); > > head += adj;
