Speed up vdev_geom_open_by_guids

[asomers]

Summary

Speed up vdev_geom_open_by_guids

Speedup is hard to measure because the only time vdev_geom_open_by_guids gets
called on many drives at the same time is during boot. But with vdev_geom_open
hacked to always call vdev_geom_open_by_guids, operations like "zpool create"
speed up by 65%.

sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c

• Read all of a vdev's labels in parallel instead of sequentially.
• In vdev_geom_read_config, don't read the entire label, including the

	  uberblock.  That's a waste of RAM.  Just read the vdev config nvlist.
	  Reduces the IO and RAM involved with tasting from 1MB to 448KB.

Test Plan

ZFS test suite

[asomers]

I'll commit this Friday morning if I don't get any feedback by then.

[smh]

sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c

355 ↗

(On Diff #15751)

Use kmem_zalloc instead of kmem_alloc + bzero?

382 ↗

(On Diff #15751)

Spacing on =

[asomers]

Incorporate feedback from smh

[smh]

sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c

368 ↗

(On Diff #15996)

Pretty sure this isn't needed given its a bzero under the covers and g_alloc_bio specifies M_ZERO.

375 ↗

(On Diff #15996)

Any reason why j++ isn't in the for?

433 ↗

(On Diff #15996)

This looks odd as error is loop invariant, bug?

[asomers]

sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c

368 ↗

(On Diff #15996)

Revision 295707 says we should always use g_reset_bio after g_alloc_bio because g_reset_bio might not always be bzero.

375 ↗

(On Diff #15996)

Nope.

433 ↗

(On Diff #15996)

Yes.

[smh]

sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c

368 ↗

(On Diff #15996)

That sounds like a very odd change as double zeroing is quite an overhead, also doesn't correspond with what was committed as only existing bzero's where changed no addition of g_reset_bio after existing g_alloc_bio / g_new_bio.

If its required by those calls then they should do it, not rely on every single caller performing the two calls?

[imp]

You can remove the g_reset_bio() call. It looks to be not needed since you aren't reusing bios and resetting them with bzero.

sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c

368 ↗

(On Diff #15996)

This call to g_reset_bios is bogus. You just allocated the bio, yo don't need to call it unless you *REUSE* the bio later.

[asomers]

Incorporate feedback from smh and imp

[avg]

sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c

380 ↗

(On Diff #16073)

Do I correctly parse this line as returning an overall failure when at least one command fails?
I think that that makes the code less robust: if reading any single of 4 vdev labels fails for whatever reason (e.g. failing hardware), then we would ignore the successfully read labels.

[asomers]

sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c

380 ↗

(On Diff #16073)

I'm afraid that you're correct. It's not completely straightforward to fix since there isn't a 1-1 relationship between commands and bios. I'll try to come up with a fix.

[asomers]

make vdev_geom_io return separate errors for each command instead of a single
overall error.

[avg]

LGTM