svn commit: r295707 - in head/sys: cddl/contrib/opensolaris/uts/common/fs/zfs dev/mmc dev/virtio/block geom geom/journal geom/mirror geom/raid geom/raid3 kern

[Warner Losh]

> On May 9, 2016, at 11:04 AM, Alan Somers <asomers at freebsd.org> wrote:
> 
> 
> 
> On Wed, Feb 17, 2016 at 10:16 AM, Warner Losh <imp at freebsd.org> wrote:
> Author: imp
> Date: Wed Feb 17 17:16:02 2016
> New Revision: 295707
> URL: https://svnweb.freebsd.org/changeset/base/295707
> 
> Log:
>   Create an API to reset a struct bio (g_reset_bio). This is mandatory
>   for all struct bio you get back from g_{new,alloc}_bio. Temporary
>   bios that you create on the stack or elsewhere should use this before
>   first use of the bio, and between uses of the bio. At the moment, it
>   is nothing more than a wrapper around bzero, but that may change in
>   the future. The wrapper also removes one place where we encode the
>   size of struct bio in the KBI.
> 
> Modified:
>   head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c
>   head/sys/dev/mmc/mmcsd.c
>   head/sys/dev/virtio/block/virtio_blk.c
>   head/sys/geom/geom.h
>   head/sys/geom/geom_io.c
>   head/sys/geom/journal/g_journal.c
>   head/sys/geom/mirror/g_mirror.c
>   head/sys/geom/raid/g_raid.c
>   head/sys/geom/raid3/g_raid3.c
>   head/sys/kern/kern_physio.c
> 
> smh noticed that while your commit message says that g_reset_bio is mandatory after g_{new,alloc}_bio, your diff only replaced existing calls to bzero.  You didn't insert g_reset_bio calls after all g_alloc_bio calls, for example in vdev_geom_io_start.  Do you intend to follow up this change with a g_reset_bio everywhere that g_alloc_bio is called, or did you mean that g_reset_bio is optional after all bios returned by g_{new,alloc}_bio?

g_reset_bio is required when you reuse the bio returned from g_{new,alloc}_bio. You don’t need to reset it before using it. Only when reusing it where you used to call bzero(). My commit message is at best ambiguous: g_reset_bio() is required to reuse the bio, not to use it in the first place. So all callers to g_alloc_bio() that just use the bio then return it don’t need to change. Callers to g_alloc_bio() that use the bio for multiple I/Os need to call g_reset_bio() between uses of the bio. That’s why I only changed the bzero’s in the tree: those were the only places where the bio was being re-used. Otherwise you wouldn’t need to reset the bio, since it is returned ‘set’ from g_{new,alloc}_bio. Before some other changes I have, it was safe to assume no fields needed to be preserved when you wanted to reuse the bio for multiple I/Os, so the code just used bzero(). After that change, the code needn’t assume what you need to do to a bio to reuse it, other than call this API. In the future, there may be info about which uma pool the I/O came from so we can have a small reserve of bios for low/out of memory situations to allow them to clear more quickly.

Sorry if things were unclear. Hopefully they are clear now.

Warner
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 842 bytes
Desc: Message signed with OpenPGP using GPGMail
URL: <http://lists.freebsd.org/pipermail/svn-src-all/attachments/20160509/b8955f9e/attachment.sig>