From owner-svn-src-head@freebsd.org Mon May 9 17:21:46 2016 Return-Path: Delivered-To: svn-src-head@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 7D848B34ADE for ; Mon, 9 May 2016 17:21:46 +0000 (UTC) (envelope-from imp@bsdimp.com) Received: from mail-pa0-x22d.google.com (mail-pa0-x22d.google.com [IPv6:2607:f8b0:400e:c03::22d]) (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 44F05184D for ; Mon, 9 May 2016 17:21:46 +0000 (UTC) (envelope-from imp@bsdimp.com) Received: by mail-pa0-x22d.google.com with SMTP id bt5so74207756pac.3 for ; Mon, 09 May 2016 10:21:46 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bsdimp-com.20150623.gappssmtp.com; s=20150623; h=sender:subject:mime-version:from:in-reply-to:date:cc:message-id :references:to; bh=Pg5DL1v4ms29v/e/hERiE+YvzvJeVzhRJt6H+tOwxAg=; b=fcQ0CxTwsC3JIh0BKkbV0c8fVXEz4IUPgmmWjA9104CBQw0onFwrLNQpkch1mKcL7N x/i3dC0KxtAEJs0Cd3vMUJIteSuAiXzdUwggRLF5G+Oqxrj4t2X/NQTDx04ZoUJKZpa8 Lx1UuAmFV5iKZHJ0o00c/l/Endv7aB0vlPRlJQ3ooG4+n5vOB8Cc0keUAOrPuMNTPAMU lPOTYpYWESnAmAhflBWRhillcUPldNOkfKegK5zy5bjaAh0SSpjfqDpQynRexOZ7f3O2 zGhBQXDWVEChfT7WpIPD+nMUnBKLgUQDD9ThLHx2cYulergfuC+jrKl7sR9vwQ5CARUU esug== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:sender:subject:mime-version:from:in-reply-to :date:cc:message-id:references:to; bh=Pg5DL1v4ms29v/e/hERiE+YvzvJeVzhRJt6H+tOwxAg=; b=jSMXYObCsv7saeBGpr73+DMc+XHD+srhqcAwRStJ9A33FFCz5oYtfr01BCD57fW6AD 0Y8qvswVidzmJ9C4CcsQbK/d/CY/TuoDhfjIR4lBKtyhV2wZ6rWHOvrXCN+cpuu2ypUq lUSJnOj5dyKoTNGXiPvplCxbYQFMFl5l2XCxrJpzBwTk7EcuvwjeMPJpClaBBCGe3wOK 2Fv2cUMJozFZ0qrBEZ+6SqrTVohZ9zYstuKJDrJomaFCaE+KHC53XDPuQvjP23yM2TzG 0ihM8I4DY+eg5kAYgtvIjE5w8ZeP2F+2BvRUxTthgm7Qrvpq+0erEq9c06W8jzdA84uF TA9A== X-Gm-Message-State: AOPr4FUiP3SZZyBKsCtPDACMR5RrkvJdgha8ZZRcpjIBlIW3QUqkH6BQRXDbsD4WBpL/aA== X-Received: by 10.67.10.205 with SMTP id ec13mr51562686pad.16.1462814505599; Mon, 09 May 2016 10:21:45 -0700 (PDT) Received: from [100.127.71.31] ([69.53.245.200]) by smtp.gmail.com with ESMTPSA id r86sm14158790pfb.21.2016.05.09.10.21.44 (version=TLS1 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Mon, 09 May 2016 10:21:45 -0700 (PDT) Sender: Warner Losh 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 Mime-Version: 1.0 (Mac OS X Mail 8.2 \(2104\)) Content-Type: multipart/signed; boundary="Apple-Mail=_041D1166-B93C-41B0-B5C1-8A46666EFDAD"; protocol="application/pgp-signature"; micalg=pgp-sha512 X-Pgp-Agent: GPGMail 2.5.2 From: Warner Losh In-Reply-To: <405a3410-a0ee-a638-eefe-c1f8980e5624@freebsd.org> Date: Mon, 9 May 2016 11:21:42 -0600 Cc: Alan Somers , Warner Losh , "src-committers@freebsd.org" , "svn-src-all@freebsd.org" , "svn-src-head@freebsd.org" Message-Id: <14D0B1CA-8F30-4DE4-A4F7-424F75BFE07E@bsdimp.com> References: <201602171716.u1HHG2c2098316@repo.freebsd.org> <405a3410-a0ee-a638-eefe-c1f8980e5624@freebsd.org> To: Steven Hartland X-Mailer: Apple Mail (2.2104) X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 09 May 2016 17:21:46 -0000 --Apple-Mail=_041D1166-B93C-41B0-B5C1-8A46666EFDAD Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=utf-8 > On May 9, 2016, at 11:14 AM, Steven Hartland = wrote: >=20 >=20 >=20 > On 09/05/2016 18:04, Alan Somers wrote: >>=20 >>=20 >> 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 >>=20 >> 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. >>=20 >> 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 >>=20 >> 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? >>=20 > 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=E2=80=99t. Please see my other reply. It=E2=80=99s 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=E2=80=99s not the case at all. There=E2=80=99s 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=E2=80=99t 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=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=99= t worked my way back to geom=E2=80=A6 Warner --Apple-Mail=_041D1166-B93C-41B0-B5C1-8A46666EFDAD Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename=signature.asc Content-Type: application/pgp-signature; name=signature.asc Content-Description: Message signed with OpenPGP using GPGMail -----BEGIN PGP SIGNATURE----- Comment: GPGTools - https://gpgtools.org iQIcBAEBCgAGBQJXMMcmAAoJEGwc0Sh9sBEAYSYQAM4I+/PnoOVFILRrQ/DLP3Co krJkuy9eG4DPDW3qI1u8Qq3KglKLnANVh/aMqfC7MyccpQ4xf5DkWXObeljZ8W5/ 59XX4uMJbZKNBDoBTM3eAtP3G1GZoHJkypEvmaGR83Yv+3nqcUlA1MQpv0LkTURg kopoGTf7jMgiNGQCyPTzF6WkVrhCBKi0M+UmwfRaaM6Wvcnh4QTolfhErIWbVeqB /K0B2yj9DJiWjDPmgSrSfzSM4iv83bA/+Q9qQE3EqZc3uBuz5m7fw237JE/96+eJ qDRgnw+NuFuaoLjXa+tq6gPhjhT1kJuVtOR0OnrCxnhSijpnqS+8FlICZ3pMjYsw V4V0DdP4+9k4RdG2HOd2x5omVKilULMNUuDrJZEuAn9NeJfMDL9v+4X2zBkwytq2 xuY1UqMfGYUz+J9X7hCG6UhwGoPLoNMATHZCVOJN7gOx6tXe8SD1yUX2Pu8SQamz XoEdGg1lX8hwYj7EXo1wkSr7eihnfNjQuCdcwzWNaJlmzqbwsyWTIYRY7EgIEvl+ kfCjtQXcS62J/hfKOdxxQprJfh4J0yxdgI4ff/gKBncYph28NqdRD6V2UnX/4zyJ TPC7y+4ccIR28DafnSyMoujuoq1u4jMPeAwgBY+guZE5rZN62K6dWzZUxLEi8Xj/ StMWuv4Xwe1ijHOYnwHB =Dqc+ -----END PGP SIGNATURE----- --Apple-Mail=_041D1166-B93C-41B0-B5C1-8A46666EFDAD--