Speed up "zpool import" on FreeBSD in the presence of many zvols

[asomers]

Motivation and Context

By default, FreeBSD does not allow zpools to be backed by zvols (that can be changed with the "vfs.zfs.vol.recursive" sysctl). When that sysctl is set to 0, the kernel does not attempt to read zvols when looking for vdevs. But the zpool command still does.

Description

This change brings the zpool command into line with the kernel's behavior. It speeds "zpool import" when an already imported pool has many zvols, or a zvol with many snapshots.

How Has This Been Tested?

Tested with FreeBSD's ZFS test suite. And it shipped in FreeBSD 12.2-RELEASE.

Performance-wise, the time to import a nonexistent pool is linear in the number of geom devices present. That includes both disks and zvols. With an SSD-backed pool on my FreeBSD 13.0-CURRENT test system, I find that the import time takes about 13.3s with 10,000 zvols present. With the patch, that falls to 0.79s. For comparison, when no zvols are present it takes only 0.09s.

https://svnweb.freebsd.org/base?view=revision&revision=357235
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=241083
https://reviews.freebsd.org/D22077

Obtained from: FreeBSD
PR: 241083
Reported by: Martin Birgmeier d8zNeCFG@aon.at
Sponsored by: Axcient
Signed-off-by: Alan Somers asomers@gmail.com

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Performance enhancement (non-breaking change which improves efficiency)
• Code cleanup (non-breaking change which makes code smaller or more readable)
• Breaking change (fix or feature that would cause existing functionality to change)
• Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
• Documentation (a change to man pages or other documentation)

Checklist:

• My code follows the OpenZFS code style requirements.
• I have updated the documentation accordingly.
• I have read the contributing document.
• I have added tests to cover my changes.
• I have run the ZFS Test Suite with this change applied.
• All commit messages are properly formatted and contain Signed-off-by.

[asomers]

cc the original reviewers: @amotin @freqlabs

[amotin]

This part looks good to me. Just not sure why move some headers forward.

I am just thinking whether we should also somehow filter out /dev/zvol from the zpool_find_import_scan or it would be too big hack?

[asomers]

This part looks good to me. Just not sure why move some headers forward.

<sys/types.h> has to be first. And since I was in there, I moved all of the sys/ headers to the top as per style(9).

I am just thinking whether we should also somehow filter out /dev/zvol from the zpool_find_import_scan or it would be too big hack?

Good point. The zpool import -s option hadn't been added when I originally wrote this change, so I didn't think about it. I'll look into it now.

[asomers]

I am just thinking whether we should also somehow filter out /dev/zvol from the zpool_find_import_scan or it would be too big hack?

Good point. The zpool import -s option hadn't been added when I originally wrote this change, so I didn't think about it. I'll look into it now.

I don't think it would be worthwhile. zpool_find_import_scan is only used with the -s and -d options to zpool import. -s, AFAICT, is never useful on FreeBSD. And when using -d, the user must explicitly include /dev/zvol; it's not the default. So I don't see any point to changing zpool_find_import_scan.