From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-0.8 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id E813CC10DCE for ; Wed, 18 Mar 2020 19:40:46 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 944F120752 for ; Wed, 18 Mar 2020 19:40:46 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=intel-com.20150623.gappssmtp.com header.i=@intel-com.20150623.gappssmtp.com header.b="gDEy+R2L" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 944F120752 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=intel.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 2D8C46B0080; Wed, 18 Mar 2020 15:40:46 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 289996B0081; Wed, 18 Mar 2020 15:40:46 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 178E16B0082; Wed, 18 Mar 2020 15:40:46 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0149.hostedemail.com [216.40.44.149]) by kanga.kvack.org (Postfix) with ESMTP id 008756B0080 for ; Wed, 18 Mar 2020 15:40:45 -0400 (EDT) Received: from smtpin20.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay01.hostedemail.com (Postfix) with ESMTP id A1FE1180AD811 for ; Wed, 18 Mar 2020 19:40:45 +0000 (UTC) X-FDA: 76609500450.20.car84_40ccaebf39516 X-HE-Tag: car84_40ccaebf39516 X-Filterd-Recvd-Size: 6089 Received: from mail-ot1-f68.google.com (mail-ot1-f68.google.com [209.85.210.68]) by imf38.hostedemail.com (Postfix) with ESMTP for ; Wed, 18 Mar 2020 19:40:45 +0000 (UTC) Received: by mail-ot1-f68.google.com with SMTP id 111so26785911oth.13 for ; Wed, 18 Mar 2020 12:40:45 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=intel-com.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=QTYHkA2/Uh4OFgR0EZrmjZd5fIPhzmLZ3uQLwoOmx2g=; b=gDEy+R2Lse4L/fQbyJWgZOeLHaZsgOT9f1Y7J0Kw87Hae4ZgON1k7OrfLipKDJS/Md jrazupwvY61UcuMHtKL2qS+2Wtn76GBVd3isW+CFmdirUwIg1e1WWL23Ca2Zib2yh90w vsjX0H9oakmyDOovSNaEHMQCq4Ao905Zg7nvMNyOFnI8aHs1NZ3HhIewTGRLhQkMI9/y UmFw8txpE2ZgbXcpRRTNHAn8FNESyrDXqvrO5rNr01THHuvvALxkWVcFmdtl/sSpuo+b ZG3Ir28mufPFFmzOIlJ8087zjs3dFsFFvKkl7PupBAIMkeABQUSavzoqkqC9PMoyKAxx UKlw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=QTYHkA2/Uh4OFgR0EZrmjZd5fIPhzmLZ3uQLwoOmx2g=; b=Xl1dSh1ltjUEe/O+in45Jzrs0ljlCRpka95Mx/oAuutfrzZKYhEUsG59TDY9A6BcfK zNJV2Ego99BbsiFk+G7AkeuHy2MqmwQnt22JjDdFOCACGxUfOFDcFWNK8431Ks1RdjWk e7/3gfu6B023iBQl6yLEIzGakD3RerO/ufCsUZFgQIxhPsrbozoGAdrsXtMbOg154oGx EJGpy41ed7rH3JxLt/5tTKyCwEHmZs7Wq8PgKnuRc/bscAdI1rmWhEoPFKMem6QDNASz 3w9g9r+xfXEA2gud8QRP0ftpHSdbBJEaZ0xNT/dLmu1cnSh9P9cDlfEwZ7lcEH06ofRc xofw== X-Gm-Message-State: ANhLgQ0lot51QmNqmOhPl2JrwXiA5vO9sXWJ4ww26Fw+Y45TbreelaCR m6Bqq06eE+v04xICTrFmCWAHUNvHmBfhirxdAYQ6WQ== X-Google-Smtp-Source: ADFU+vsHjg43svEJLOJaJAO4nWWs10gZ2SpYDSJYdqXm0sC9itsMzSgh4mmpEBfwmh7L5gYFmidgFcyg3CtXvF3zVM4= X-Received: by 2002:a05:6830:57b:: with SMTP id f27mr5466399otc.363.1584560444262; Wed, 18 Mar 2020 12:40:44 -0700 (PDT) MIME-Version: 1.0 References: <20200317135035.GA19442@SDF.ORG> <202003171435.41F7F0DF9@keescook> <20200317230612.GB19442@SDF.ORG> <202003171619.23210A7E0@keescook> <20200318014410.GA2281@SDF.ORG> <20200318082035.GB2281@SDF.ORG> <20200318192934.GD2281@SDF.ORG> In-Reply-To: <20200318192934.GD2281@SDF.ORG> From: Dan Williams Date: Wed, 18 Mar 2020 12:40:33 -0700 Message-ID: Subject: Re: [PATCH v2] mm/shuffle.c: Fix races in add_to_free_area_random() To: George Spelvin Cc: Kees Cook , Linux MM , Andrew Morton Content-Type: text/plain; charset="UTF-8" X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Wed, Mar 18, 2020 at 12:29 PM George Spelvin wrote: > > On Wed, Mar 18, 2020 at 10:36:10AM -0700, Dan Williams wrote: > > On Wed, Mar 18, 2020 at 1:20 AM George Spelvin wrote: > >> On Tue, Mar 17, 2020 at 08:53:55PM -0700, Dan Williams wrote: > >>> I had the impression that unless unlikely is "mostly never" then it > >>> can do more harm than good. Is a branch guaranteed to be taken every > >>> BITS_PER_LONG'th occurrence really a candidate for unlikely() > >>> annotation? > >> > >> I had to look this up. GCC manual: > >> > >> For the purposes of branch prediction optimizations, the probability > >> that a '__builtin_expect' expression is 'true' is controlled by GCC's > >> 'builtin-expect-probability' parameter, which defaults to 90%. You can > >> also use '__builtin_expect_with_probability' to explicitly assign a > >> probability value to individual expressions. > >> > >> So I think that <= 10% is good enough, which is true in this case. > >> > >> I was tring to encourage the compiler to: > >> * Place this code path out of line, and > >> * Not do the stack manipulations (build a frame, spill registers) > >> needed for a non-leaf function if this path isn't taken. > > > > Understood, I think it's ok in this case because the shuffling only > > happens for order-10 page free events by default so it will be > > difficult to measure the perf impact either way. But in other kernel > > contexts I think unlikely() annotation should come with numbers, 90% > > not taken is not sufficient in and of itself. > > I'm not sure I fully understand your point. I *think* you're > editorializing on unlikely() in general and not this specific code, > but it's a little hard to follow. Yes, editorializing on unlikely(). Specifically I would normally ask for perf numbers to show that the hint is worth it, but I talked myself out of asking for that in this case. > Your mention of "order-10 page free events" is confusing. Do you mean > "(order-10 page) free events", i.e. freeing of 1024 consecutive pages? > Or are you using "order" as a synonym for "approximately" and you mean > "approximately 10 (page free event)s"? I'm referring to this: if (is_shuffle_order(order)) add_to_free_area_random(page, &zone->free_area[order], Where shuffle order is MAX_ORDER-1. I.e. this code is only triggered when we might be releasing a 4MB buddy-page. > We both agree (I hope) that the number here is obvious on brief > inspection: 1/BITS_PER_LONG. > > GCC's heuristics are tuned to value cycles on the fast path 9x as much as > cycles on the slow path, so it will spend up to 9 cycles on the slow path > to save a cycle on the fast path. > > I've found one comment (https://pastebin.com/S8Y8tqZy) saying that > GCC < 9.x was a lot sloppier on the cost ratio and could pessimize > the code if the branch was more than ~ 1% taken. Perhaps that's what > you're remembering? Yes, thanks for that digging! > Fortunately, 1/64 = 1.56% is fairly close to 1%. so I'm not too > worried. Makes sense.