Re: git: 238271f4a66b - main - stand: Add a snarky note about the upstream ZFS situation

On Tue, Apr 18, 2023 at 4:52 PM Jessica Clarke <jrtc27@freebsd.org> wrote:

> On 18 Apr 2023, at 23:49, Warner Losh <imp@bsdimp.com> wrote:
> >
> >
> >
> > On Tue, Apr 18, 2023, 3:34 PM Jessica Clarke <jrtc27@freebsd.org> wrote:
> >> On 18 Apr 2023, at 22:31, Warner Losh <imp@FreeBSD.org> wrote:
> >> >
> >> > The branch main has been updated by imp:
> >> >
> >> > URL:
> https://cgit.FreeBSD.org/src/commit/?id=238271f4a66bd06b8b9a232a82f3ee0882e4cbb9
> >> >
> >> > commit 238271f4a66bd06b8b9a232a82f3ee0882e4cbb9
> >> > Author:     Warner Losh <imp@FreeBSD.org>
> >> > AuthorDate: 2023-04-18 21:29:45 +0000
> >> > Commit:     Warner Losh <imp@FreeBSD.org>
> >> > CommitDate: 2023-04-18 21:31:17 +0000
> >> >
> >> >    stand: Add a snarky note about the upstream ZFS situation
> >> >
> >> >    The latest import of openzfs broke the hacks that we used to omit
> the
> >> >    special registers being used on arm64. Add snarky note documenting
> this
> >> >    situation since it's a mess now since the hack was only partially
> >> >    undone, leaving behind a mess.
> >> >
> >> >    Sponsored by:           Netflix
> >> > ---
> >> > stand/libsa/zfs/Makefile.inc | 4 ++++
> >> > 1 file changed, 4 insertions(+)
> >> >
> >> > diff --git a/stand/libsa/zfs/Makefile.inc
> b/stand/libsa/zfs/Makefile.inc
> >> > index f4cecdbc3085..7660f4ab7baf 100644
> >> > --- a/stand/libsa/zfs/Makefile.inc
> >> > +++ b/stand/libsa/zfs/Makefile.inc
> >> > @@ -19,6 +19,7 @@ ZSTD_SRC+=  zstd_common.c
> >> > ZSTD_SRC+=    zstd_ddict.c zstd_decompress.c zstd_decompress_block.c
> >> > ZSTD_SRC+=    zstd_double_fast.c zstd_fast.c zstd_lazy.c zstd_ldm.c
> >> >
> >> > +# This is completely bogus: We should be able to omit this code
> completely.
> >> > .if ${MACHINE_ARCH} == "aarch64"
> >> > ZFS_SRC_AS =  b3_aarch64_sse2.S b3_aarch64_sse41.S
> >> > .endif
> >> > @@ -90,10 +91,13 @@ CFLAGS.skein_block.c+=    -DSKEIN_LOOP=111
> >> >
> >> > # To find blake3_impl.c in OpenZFS tree for our somehat ugly
> blake3_impl_hack.c
> >> > # that's needed until the necessary tweaks can be upstreamed.
> >> > +# XXX the last import gutted all this since upstream changes broke
> this hack.
> >> > CFLAGS.blake3_impl_hack.c+= -I${OZFS}/module/icp/algs/blake3
> -I${OZFS}/module/icp/include
> >> >
> >> > CWARNFLAGS.zfs.c+= ${NO_WDANGLING_POINTER}
> >> >
> >> > +# Needing to remove the -mgeneral-regs-only is a red flag that this
> is not quite
> >> > +# right. But it's needed at the moment due to the muddled upstream.
> >>
> >> This one isn’t bogus? The file is deliberately using NEON so needs
> >> access to floating-point registers, which LLVM (mostly) enforces for
> >> the assembler, unlike GNU as.
> >
> > No. It's bogus because we should not be using it at all. The generic
> implementation is fast enough for the boot loader and we have a blanket
> policy against using extra register sets. The change wasn't discussed
> before hand and what's really needed are upstream changes to be able to
> omit it entirely.
>
> Oh I see, I misunderstood the point the comment was making.
>

It's also the third or fourth time that I've had to sort this, or similar,
issues out in the boot loader (after spending a week or two initially to
enable the OpenZFS import), so I'm a little frustrated by the situation.

Warner


> Thanks,
> Jess
>
> > > b3_aarch64_sse2.o: b3_aarch64_sse2.S
> > >       ${CC} -c ${CFLAGS:N-mgeneral-regs-only} ${WERROR} ${.IMPSRC} \
> > >           -o ${.TARGET}
>
>