> On May 9, 2016, at 11:04 AM, Alan Somers <asom...@freebsd.org> wrote: > > > > On Wed, Feb 17, 2016 at 10:16 AM, Warner Losh <i...@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
signature.asc
Description: Message signed with OpenPGP using GPGMail
