I've updated the g_bio man page with all the info presented here. Please let me know if I missed anything, or if you have suggestions for improvement.
Warner On Mon, May 9, 2016 at 12:21 PM, Steven Hartland <ste...@multiplay.co.uk> wrote: > On 09/05/2016 18:21, Warner Losh wrote: > > On May 9, 2016, at 11:14 AM, Steven Hartland <ste...@multiplay.co.uk> > <ste...@multiplay.co.uk> wrote: > > > > On 09/05/2016 18:04, Alan Somers wrote: > > > On Wed, Feb 17, 2016 at 10:16 AM, Warner Losh <i...@freebsd.org> > <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? > > > Yer I was just penning this too: > This commit was just referenced in https://reviews.freebsd.org/D6153 > It seems rather odd to require all callers to g_{new,alloc}_bio to also call > g_reset_bio. > > I don’t. Please see my other reply. It’s only when you *RE*use the bio that > you need to call g_reset_bio(), not when you use it in the first place. You > can no longer call bzero() on the bio to reset it. > > > I assume this is because uma can return an existing object instead of fresh > one hence its not guaranteed to be bzeroed? If so why have the caller > responsible, seems petty error prone. A quick look at users of g_alloc_bio it > seems like this is something that's not done currently done in all places, > even some usages of memset still hanging around, are these cases a bug? > > No. That’s not the case at all. There’s going to be contents of the BIO that > cannot be blithely cleared by the users of the memory. Many other structures > in the kernel are like this, but bio wasn’t previously. > > > If the concept of this was to ensure correctly initialised objects from uma > would the callback handers to uma_zcreate not be a better option as that > would guarantee things are correct instead of leaving it to the caller? > > No. It’s to ensure internal state to the bio isn’t blown away by a subsequent > bzero() call before calling g_destroy_bio(). > > > As a side matter, this area really needs some man pages so the its clear to > all what is needed and when. > > Agreed. The whole storage stack, however, is wonderfully under-documented. > I’ve started documenting CAM, but handn’t worked my way back to geom… > > > Thanks for the clarifications Warner, appreciated :) > > Regards > Steve > _______________________________________________ svn-src-all@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
