Disable loop unrolling in skein in sys/boot
ClosedPublic
Actions

Authored by allanjude on Sep 8 2016, 3:33 AM.

Details

Reviewers

emaste
imp
sbruno
tsoome

Commits

rS306751: Disable loop unrolling in skein for sys/boot

Summary

When tsoome@ added skein support to the ZFS boot code it caused an explosion in code size

The default for the C version of skein is to unroll all loops for skein 256 and 512

Disabling loop unrolling saves 20-28 kb in each binary

-r-xr-xr-x 1 root wheel 120336 Sep 7 23:18 boot1.efi*
-r-xr-xr-x 1 root wheel 98832 Sep 7 23:23 boot1.efi*

-r--r--r-- 1 root wheel 141298 Sep 7 23:18 gptzfsboot
-r--r--r-- 1 root wheel 111906 Sep 7 23:23 gptzfsboot

-r-xr-xr-x 1 root wheel 348160 Sep 7 23:18 loader*
-r-xr-xr-x 1 root wheel 348160 Sep 7 23:27 loader*

-r-xr-xr-x 1 root wheel 445256 Sep 7 23:18 loader.efi*
-r-xr-xr-x 1 root wheel 423752 Sep 7 23:23 loader.efi*

-r--r--r-- 1 root wheel 409976 Sep 7 23:18 userboot.so
-r--r--r-- 1 root wheel 389496 Sep 7 23:27 userboot.so

-r-xr-xr-x 1 root wheel 446464 Sep 7 23:18 zfsloader*
-r-xr-xr-x 1 root wheel 417792 Sep 7 23:27 zfsloader*

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

allanjude updated this revision to Diff 20166.Sep 8 2016, 3:33 AM

allanjude retitled this revision from to Disable loop unrolling in skein in sys/boot.

allanjude updated this object.

allanjude edited the test plan for this revision. (Show Details)

allanjude added reviewers: tsoome, imp, emaste, sbruno.

Herald added a subscriber: delphij. · View Herald TranscriptSep 8 2016, 3:33 AM

ok, I did the same -DSKEIN_LOOP=111 build with my illumos port; /boot is with binaries before this change, the results are:
-r-xr-xr-x 1 root sys 195584 sept 5 17:58 /boot/bootia32.efi
-r-xr-xr-x 1 root sys 153088 sept 5 17:58 /boot/bootx64.efi
-r-xr-xr-x 1 root sys 532968 sept 5 17:58 /boot/loader32.efi
-r-xr-xr-x 1 root sys 550275 sept 5 17:58 /boot/loader64.efi
-r--r--r-- 1 root sys 405504 sept 5 17:58 /boot/zfsloader
-rwxr-xr-x 1 tsoome staff 147968 sept 8 12:54 efi/boot1/amd64/bootx64.efi
-rwxr-xr-x 1 tsoome staff 169472 sept 8 12:54 efi/boot1/i386/bootia32.efi
-rwxr-xr-x 1 tsoome staff 545155 sept 8 12:54 efi/loader/amd64/loader64.efi
-rwxr-xr-x 1 tsoome staff 507368 sept 8 12:54 efi/loader/i386/loader32.efi
-rw-r--r-- 1 tsoome staff 380928 sept 8 12:54 i386/loader/zfsloader

So, indeed, the space saving is there. Good catch.

emaste accepted this revision.Sep 8 2016, 2:59 PM

emaste edited edge metadata.

This revision is now accepted and ready to land.Sep 8 2016, 2:59 PM

test boot seems also to be ok, so I'm happy.

emaste mentioned this in D7801: Start of EFI32 boot loader support.Sep 15 2016, 7:41 PM

sbruno added a parent revision: D7801: Start of EFI32 boot loader support.Sep 15 2016, 8:03 PM

Closed by commit rS306751: Disable loop unrolling in skein for sys/boot (authored by allanjude). · Explain WhyOct 6 2016, 3:32 AM

This revision was automatically updated to reflect the committed changes.

Is it possible this broke UEFI booting from RAIDZ pools with checksum=skein? If I omit "-O checksum=skein" from my zpool create step everything works great, but with skein checksums, boot1 sees my pool (prints my pool name after "found the following pools" message) but hangs where it should have run loader.efi and then requires a hard reset.

In D7826#176314, @lstewart wrote:

Is it possible this broke UEFI booting from RAIDZ pools with checksum=skein? If I omit "-O checksum=skein" from my zpool create step everything works great, but with skein checksums, boot1 sees my pool (prints my pool name after "found the following pools" message) but hangs where it should have run loader.efi and then requires a hard reset.

well, if you got the hung system and not hung without skein, then obviously there has to be some connection.

If you can have more tests, could you try to create the pool itself without skein, but some dataset with skein, then it would be possible to get loader.efi running and check if files from dataset with skein are accessible or will cause trouble.. The pool layout by itself should not really affect the checksumming... What version/build etc are you using btw?

In D7826#176314, @lstewart wrote:

Is it possible this broke UEFI booting from RAIDZ pools with checksum=skein? If I omit "-O checksum=skein" from my zpool create step everything works great, but with skein checksums, boot1 sees my pool (prints my pool name after "found the following pools" message) but hangs where it should have run loader.efi and then requires a hard reset.

Yes, apparently there is an issue, I'm not sure yet about the root cause, but I was able to reproduce with 2+1 raidz config. Could you please file an bugreport?

To clarify, I did not test prior to this commit, so am not sure if it ever worked. I merely looked up svn log history on sys/boot and this seemed like a plausible culprit assuming there has been a regression.

I've been testing using this FreeBSD-CURRENT snapshot: ftp://ftp.freebsd.org/pub/FreeBSD/snapshots/amd64/amd64/ISO-IMAGES/12.0/FreeBSD-12.0-CURRENT-amd64-20161021-r307747-memstick.img.xz

As you have been able to reproduce, I will skip the experimentation you asked for and file a bug report. Thanks for looking into it.