From owner-svn-src-all@freebsd.org Thu May 24 04:44:22 2018 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id DEDA7EFC892; Thu, 24 May 2018 04:44:21 +0000 (UTC) (envelope-from mjguzik@gmail.com) Received: from mail-qt0-x244.google.com (mail-qt0-x244.google.com [IPv6:2607:f8b0:400d:c0d::244]) (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 782E073164; Thu, 24 May 2018 04:44:21 +0000 (UTC) (envelope-from mjguzik@gmail.com) Received: by mail-qt0-x244.google.com with SMTP id h2-v6so406821qtp.7; Wed, 23 May 2018 21:44:21 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=A/fb3ysf/Fsr65Eve1LVFb2srxQm+N+waaNmDKwf7hM=; b=UPWzptv6pOTPaovJqMaPog4QB4JGwCq1iYMuQuylB7PowD6aVjuSg/04hoWtPcV03W XF/Ovc+k+3VLwbcbRH2fqZc1r0RUQT9RTxYG3QzNDrtuNJerTjTZQGh6ssfQsBr3KpIB qvz+HNLolRYTIvJz2JWVOshM66wLACkU8dJyYHCi/Ds9Ij5fS23vNcCAql4/o4V4ZPh9 2graaylF1jZrYSyop2Wn/E/ckasxcsH5u888mv3OhuaT5ullLc6SSKx/EDKayR3Qncgt L1sxAklKOE53PRhK/Z7CWCzM3PDBq2f0NZ3trt2Mu+vOZeJ29qxX4c5P/COGYyH58ul+ O4Eg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=A/fb3ysf/Fsr65Eve1LVFb2srxQm+N+waaNmDKwf7hM=; b=QqVGtHBTe/7cAu66XcmzIFEZlI55AuZ8bcEdBGX6Sk9/YL87RjNtXNJHBAWOkXdKgx fddOUqF6/3195DSH7EFEXplhUlV6gXPBXqlRQfR+Vuo0yVf2o8rYSIhD457lwr8CfBeC OiIJj8n5SBbxsmT7AgLuooCvOuwqR1Fzxbv/v5SjHH9LetpMW04q6sKdec1ll87kEH+H cTeTz4dLSKZbhOo8NyB5wr/ItJv7XPOGeiDJGqlJq9/KImJbpnFZyH2qACmwn+If+fxH SO8bLKKdx1HiVuN6QroEbja70wzLEXYa5GLTHxyPUbQP+FYuQi05bCGga8Wd1X4APAw7 kWVQ== X-Gm-Message-State: ALKqPwdUhMID0rEN70LyQFIHMm0Whh2qkqzvrzLBKVXrKkgnksTFZKEE GZ4EQIqWGhcBWKKb9StAgmepNQBzyqm2ov7MHgU= X-Google-Smtp-Source: AB8JxZpYHBe/eReuMlYu+CoQUmS1vdRGj6bTHhogJ+SUibhvELdq47GlevAG0jaE9wn33QjRnDZdJbUscrX0G4eh7+w= X-Received: by 2002:ac8:1b59:: with SMTP id p25-v6mr5239960qtk.237.1527137060913; Wed, 23 May 2018 21:44:20 -0700 (PDT) MIME-Version: 1.0 Received: by 2002:ac8:1c4e:0:0:0:0:0 with HTTP; Wed, 23 May 2018 21:44:20 -0700 (PDT) In-Reply-To: References: <201805231700.w4NH05hs047395@repo.freebsd.org> <2281830.zrSQodBeDb@ralph.baldwin.cx> From: Mateusz Guzik Date: Thu, 24 May 2018 06:44:20 +0200 Message-ID: Subject: Re: svn commit: r334104 - in head/sys: netinet sys To: "Jonathan T. Looney" Cc: Matthew Macy , John Baldwin , src-committers , svn-src-all@freebsd.org, svn-src-head@freebsd.org Content-Type: text/plain; charset="UTF-8" X-Content-Filtered-By: Mailman/MimeDel 2.1.26 X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.26 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: Thu, 24 May 2018 04:44:22 -0000 On Thu, May 24, 2018 at 2:40 AM, Jonathan T. Looney wrote: > On Wed, May 23, 2018 at 7:13 PM, Matthew Macy wrote: > > > > On Wed, May 23, 2018 at 11:52 AM, John Baldwin wrote: > > > On Wednesday, May 23, 2018 05:00:05 PM Matt Macy wrote: > > >> Author: mmacy > > >> Date: Wed May 23 17:00:05 2018 > > >> New Revision: 334104 > > >> URL: https://svnweb.freebsd.org/changeset/base/334104 > > >> > > >> Log: > > >> epoch: allow for conditionally asserting that the epoch context > fields > > >> are unused by zeroing on INVARIANTS builds > > > > > > Is M_ZERO really so bad that you need to make it conditional? > > > > In this case not at all. It's only exercised by sysctl handlers. I'm > > mostly responding to an inquiry by jtl. However, gratuitous M_ZERO > > usage does have a cumulative adverse performance impact. > > I appreciate you making this change. And, I do think it is worth avoiding > M_ZERO where it is unnecessary, for the reason you state. > > I agree that M_ZERO for no reason should be avoided, especially so with the current implementation of said zeroing (mandatory call to bzero, which is not fast either). > > > > I would probably have preferred something like 'M_ZERO_INVARIANTS' > > > instead perhaps (or M_ZERO_EPOCH) that only controls M_ZERO and is > > > still or'd with M_WAITOK or M_NOWAIT. > > > > Yes. I like that better too. Thanks. > > Yes, that does seem better. > > I fundamentally disagree with this part. If a known value of a given field is needed for assertion purposes, you can add (possibly conditional) code setting this specific value. It probably should not be zero if it can be helped. Conditional zeroing of the *whole* struct depending on invariants will *hide* uninitialized memory read bugs - production kernel will have whatever it happens to find, while *debug* kernel will guarantee to have all the values zeroed. In fact the flag actively combats redzoning. if the resulting allocation is zeroed, poisoning is actively neutered. But only if debug is enabled. That said, I find the change harmful. #define epoch_debug_init or similar can be used here instead. -- Mateusz Guzik