Don't set BIO_DONE if a bio_done handler is provided

[markj]

Summary

This change re-applies r273143. The original motivation for this change
was to fix callers of biowait(), all of which follow the pattern:

bp->bio_done = NULL;
g_io_request(bp);
biowait(bp);
g_destroy_bio(bp);

(Note that the biowait() calls in ZFS are not compiled on FreeBSD.)

Because the g_disk optimization in r256880 introduced a trick where
the bio_done handler is overridden once the request is dispatched, it
was racy to set BIO_DONE before the g_disk bio_done handler completed.
This g_disk handler takes care to call biodone() with the original
bio_done handler, and this will wake up waiters in biowait(). However,
with r273638 there is no code to exercise this race.

Isilon's FS uses biowait() on BIOs with a non-NULL bio_done handler.
This is done e.g., to serialize updates to a superblock-like state
block. The bio_done handler is responsible for ensuring that waiters are
awoken. Right now it does so by setting bio_done = NULL and calling
biodone() again. With this use-case, it is also incorrect to set BIO_DONE
before calling the handler.

The cause of the corruption mentioned in r273638 is not clear to me, but
I believe that r273143 on its own cannot be the cause. It is
intrinsically racy to set BIO_DONE in common code when a bio_done handler
is provided.

[imp]

Does this require all the bio_done handlers to set BIO_DONE?

[markj]

In D9070#187375, @imp wrote:

Does this require all the bio_done handlers to set BIO_DONE?

No. BIO_DONE is only relevant when there is a possibility that a thread has called biowait() on the BIO. Currently, all callers dispatch BIOs with bp->bio_done = NULL, so BIO_DONE is still set in biodone() as expected. If any GEOMs use the trick of "chaining" bio_done handlers by inserting their own, they are also responsible for ensuring the original bio_done handler is called, which is still responsible for setting BIO_DONE as needed.

[mav]

While motivation with double biodone() call can be questioned, I agree that setting BIO_DONE without any locking in case of bio_done() present is just calling for problems.