From owner-svn-src-all@freebsd.org Tue May 17 16:33:45 2016 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id CC341B3F507 for ; Tue, 17 May 2016 16:33:45 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Received: from mail-io0-x233.google.com (mail-io0-x233.google.com [IPv6:2607:f8b0:4001:c06::233]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 961492430 for ; Tue, 17 May 2016 16:33:45 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Received: by mail-io0-x233.google.com with SMTP id d62so30541882iof.2 for ; Tue, 17 May 2016 09:33:45 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bsdimp-com.20150623.gappssmtp.com; s=20150623; h=mime-version:sender:in-reply-to:references:date:message-id:subject :from:to:cc; bh=oI85Sp5bryOMoZrxjEZ8MSsVEKv0t6AF81VcPxelxf0=; b=QlkBwK4xrT/qDjaDh5eGALAJaMgo0SdEjc1LYP6hUyLebPTFSr2hjEpiyT81DFvnJl 1YtkM6nKrBB7m13WnKkNZsqaRyNykrmCu522uZrX3hSt5bg9QSZnoduGUVycQViS43Wk fVpaajr/GVI9F1w3gPWwFhA2j0KqUGAniij7U4VpzUK/LRWK5fz46ENp1v8m41p25baG 0/9JAPbuMGdbDsUbyGukqTwJ7iGiSYCjogQxl/x3xb1vT0trfNu+mPSg7i4wrKHadnYY 7X8TtKIUlJef0ao3SmPRk/WTYQr/jsvbEBl0eW3PjTnixN4zOW/jo5/MBN4Oh/2ACcwy v4Dw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:sender:in-reply-to:references:date :message-id:subject:from:to:cc; bh=oI85Sp5bryOMoZrxjEZ8MSsVEKv0t6AF81VcPxelxf0=; b=knH62wcwgrS5IfwPaBnHr06bttC3XTA6bvbX+YTSbGaXpdY5X4zM0OqOxk6K2N4M9f vvhyrRc0uO50KKOphBh40qF0TqQ75UUSOg3G5a4Pu+Tg6YUubvEGSyCEpNlvLfX6SOeK yxhe5Oxq3urMMEdpDJOFQ+v9eH4lobiyuF8BqvNuhfNlDseX71rjbKTtxzjdVfGFwg2o vi9yMAdwF+/H8fCWRguYYrpQEOHU7yfC6NytQyXwNeS4SLs0vHVzQ9Vmkp/u9uYA6EG2 XCHDcRHBXnTi4ppJ5/daiVjgZBOhnGmvhbS+jausQAbGCXvPTVMt/UBmHWti6aVRcKh/ k0Ug== X-Gm-Message-State: AOPr4FXfauzKQIqjrA7e5Zo64oR0uYe2Hmu01F4Sun7qO3XUogdOuNTNIrSJ2GtyjkmqU4AZYUHhN8I5e2fAyQ== MIME-Version: 1.0 X-Received: by 10.36.70.7 with SMTP id j7mr14139271itb.72.1463502824860; Tue, 17 May 2016 09:33:44 -0700 (PDT) Sender: wlosh@bsdimp.com Received: by 10.79.75.68 with HTTP; Tue, 17 May 2016 09:33:44 -0700 (PDT) X-Originating-IP: [50.253.99.174] In-Reply-To: References: <201602171716.u1HHG2c2098316@repo.freebsd.org> <405a3410-a0ee-a638-eefe-c1f8980e5624@freebsd.org> <14D0B1CA-8F30-4DE4-A4F7-424F75BFE07E@bsdimp.com> Date: Tue, 17 May 2016 10:33:44 -0600 X-Google-Sender-Auth: MzDGjVFsub3HY4ft3otSjO5erDo Message-ID: Subject: Re: 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 From: Warner Losh To: Steven Hartland Cc: Alan Somers , "src-committers@freebsd.org" , "svn-src-all@freebsd.org" , "svn-src-head@freebsd.org" Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-Content-Filtered-By: Mailman/MimeDel 2.1.22 X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 17 May 2016 16:33:45 -0000 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 wrote: > On 09/05/2016 18:21, Warner Losh wrote: > > On May 9, 2016, at 11:14 AM, Steven Hartland wrote: > > > > On 09/05/2016 18:04, Alan Somers wrote: > > > On Wed, Feb 17, 2016 at 10:16 AM, Warner Losh 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 manda= tory after g_{new,alloc}_bio, your diff only replaced existing calls to bze= ro. You didn't insert g_reset_bio calls after all g_alloc_bio calls, for e= xample 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 c= all g_reset_bio. > > I don=E2=80=99t. Please see my other reply. It=E2=80=99s only when you *R= E*use the bio that you need to call g_reset_bio(), not when you use it in t= he 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 fre= sh one hence its not guaranteed to be bzeroed? If so why have the caller re= sponsible, 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=E2=80=99s not the case at all. There=E2=80=99s going to be conte= nts 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=E2=80=99t p= reviously. > > > If the concept of this was to ensure correctly initialised objects from u= ma 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=E2=80=99s to ensure internal state to the bio isn=E2=80=99t 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=E2=80=99ve started documenting CAM, but handn=E2=80=99t worked my way b= ack to geom=E2=80=A6 > > > Thanks for the clarifications Warner, appreciated :) > > Regards > Steve >