From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-Status: No, score=-4.1 required=3.0 tests=AWL,BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,MAILING_LIST_MULTI, SPF_HELO_PASS,SPF_PASS shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by dcvr.yhbt.net (Postfix) with ESMTPS id 10C4C1F5AE for ; Fri, 3 Jul 2020 17:54:24 +0000 (UTC) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 3168E3842429; Fri, 3 Jul 2020 17:54:23 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 3168E3842429 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1593798863; bh=YAdTnLakWRvLNDrwkvzdBQvPTDVbQuHW5tpNp5MGZsY=; h=References:In-Reply-To:Date:Subject:To:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=PX1ksjjlQj9nAXsOO7zhUIe721DHeJ3RR5g8f8WVRQGi9XY5b8m7XtdcR8VMBCL+M X5k16M4bs9qixPz7e2s7DeNMhZqS9ddExHX58o2gKjlWJ3X3tpWmgnLta4WUHMT4OX vDQOlN/+FDMO6zX0rI0+icHOudlKVJmcvs9AtpGM= Received: from mail-io1-xd42.google.com (mail-io1-xd42.google.com [IPv6:2607:f8b0:4864:20::d42]) by sourceware.org (Postfix) with ESMTPS id 8B30A3842429 for ; Fri, 3 Jul 2020 17:54:20 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 8B30A3842429 Received: by mail-io1-xd42.google.com with SMTP id q74so9642879iod.1 for ; Fri, 03 Jul 2020 10:54:20 -0700 (PDT) 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=YAdTnLakWRvLNDrwkvzdBQvPTDVbQuHW5tpNp5MGZsY=; b=rLJbdf/1wxRu5go8V0iw2DhPLLMRCeQ3t6LYX4gBqxY52zi5NoQGo4LAeze6+OlZsZ WS6J11la8zgIQa9PhPP9EUtBjl0K4Qt/QYWWjE9/IHRxrsUmNWSlfZU9RIYDVWdio78t gcQ65sTzYgu6X9EPx6E7J2wL/GKjBmurjszVass4kz/3+gU/dfR5TsQTb9KKW/SKjTfm ldvIzVlPUkeFESAgl0Pf/iJJ2gelgMMztWYMCjUmk4WcVK0BXWYlUd2QbCnMk1xZsUqO 769rPdOC4gZ1oOzk7zVjS6Ky4hHr+Wd7kB4wsHmSSW0DCVHVX/J7qDO+yVDMXrW+Dc9c uFcg== X-Gm-Message-State: AOAM530cMGENzivNi5ZeDvsq/Uun+Dz37aO0WrAp4eMOgKh8TGX0DHpt //8o3qgt2P+aiCodgJ98qBNq5kblbm3MbkaIp3qJBkI6 X-Google-Smtp-Source: ABdhPJwjxu+t30eZWpOflbcrSoWta5bslPbsIHDxux2/yPip+pJNiHb8WkmTsHKO6k1AbWzhEVc6apslYkvfPfzf2Pw= X-Received: by 2002:a02:cd91:: with SMTP id l17mr39322411jap.88.1593798859957; Fri, 03 Jul 2020 10:54:19 -0700 (PDT) MIME-Version: 1.0 References: <20200605224550.GA1253830@gmail.com> <20200702190840.GA1474341@gmail.com> <369c062d-209c-5894-f8c7-e236753f946b@redhat.com> <20200703165452.GA226121@gmail.com> <5b3b0c76-e2d6-8836-f63f-9224d17aca7a@redhat.com> In-Reply-To: <5b3b0c76-e2d6-8836-f63f-9224d17aca7a@redhat.com> Date: Fri, 3 Jul 2020 10:53:43 -0700 Message-ID: Subject: Re: [PATCH] x86: Add thresholds for "rep movsb/stosb" to tunables To: "Carlos O'Donell" Content-Type: text/plain; charset="UTF-8" X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: "H.J. Lu via Libc-alpha" Reply-To: "H.J. Lu" Cc: Hushiyuan , Florian Weimer , GNU C Library Errors-To: libc-alpha-bounces@sourceware.org Sender: "Libc-alpha" On Fri, Jul 3, 2020 at 10:43 AM Carlos O'Donell wrote: > > On 7/3/20 12:54 PM, H.J. Lu wrote: > > On Fri, Jul 03, 2020 at 12:14:01PM -0400, Carlos O'Donell wrote: > >> On 7/2/20 3:08 PM, H.J. Lu wrote: > >>> On Thu, Jul 02, 2020 at 02:00:54PM -0400, Carlos O'Donell wrote: > >>>> On 6/6/20 5:51 PM, H.J. Lu wrote: > >>>>> On Fri, Jun 5, 2020 at 3:45 PM H.J. Lu wrote: > >>>>>> > >>>>>> On Thu, Jun 04, 2020 at 02:00:35PM -0700, H.J. Lu wrote: > >>>>>>> On Mon, Jun 1, 2020 at 7:08 PM Carlos O'Donell wrote: > >>>>>>>> > >>>>>>>> On Mon, Jun 1, 2020 at 6:44 PM H.J. Lu wrote: > >>>>>>>>> Tunables are designed to pass info from user to glibc, not the other > >>>>>>>>> way around. When __libc_main is called, init_cacheinfo is never > >>>>>>>>> called. I can call init_cacheinfo from __libc_main. But there is no > >>>>>>>>> interface to update min and max values from init_cacheinfo. I don't > >>>>>>>>> think --list-tunables will work here without changes to tunables. > >>>>>>>> > >>>>>>>> You have a dynamic threshold. > >>>>>>>> > >>>>>>>> You have to tell the user what that minimum is, otherwise they can't > >>>>>>>> use the tunable reliably. > >>>>>>>> > >>>>>>>> This is the first instance of a min/max that is dynamically determined. > >>>>>>>> > >>>>>>>> You must fetch the cache info ahead of the tunable initialization, that > >>>>>>>> is you must call init_cacheinfo before __init_tunables. > >>>>>>>> > >>>>>>>> You can initialize the tunable data dynamically like this: > >>>>>>>> > >>>>>>>> /* Dynamically set the min and max of glibc.foo.bar. */ > >>>>>>>> tunable_id_t id = TUNABLE_ENUM_NAME (glibc, foo, bar); > >>>>>>>> tunable_list[id].type.min = lowval; > >>>>>>>> tunable_list[id].type.max = highval; > >>>>>>>> > >>>>>>>> We do something similar for maybe_enable_malloc_check. > >>>>>>>> > >>>>>>>> Then once the tunables are parsed, and the cpu features are loaded > >>>>>>>> you can print the tunables, and the printed tunables will have meaningful > >>>>>>>> min and max values. > >>>>>>>> > >>>>>>>> If you have circular dependency, then you must process the cpu features > >>>>>>>> first without reading from the tunables, then allow the tunables to be > >>>>>>>> initialized from the system, *then* process the tunables to alter the existing > >>>>>>>> cpu feature settings. > >>>>>>>> > >>>>>>> > >>>>>>> How about this? I got > >>>>>>> > >>>>>> > >>>>>> Here is the updated patch, which depends on > >>>>>> > >>>>>> https://sourceware.org/pipermail/libc-alpha/2020-June/114820.html > >>>>>> > >>>>>> to add "%d" support to _dl_debug_vdprintf. I got > >>>>>> > >>>>>> $ ./elf/ld.so ./libc.so --list-tunables > >>>>>> glibc.elision.skip_lock_after_retries: 3 (min: -2147483648, max: 2147483647) > >>>>>> glibc.malloc.trim_threshold: 0x0 (min: 0x0, max: 0xffffffff) > >>>>>> glibc.malloc.perturb: 0 (min: 0, max: 255) > >>>>>> glibc.cpu.x86_shared_cache_size: 0x100000 (min: 0x0, max: 0xffffffff) > >>>>>> glibc.elision.tries: 3 (min: -2147483648, max: 2147483647) > >>>>>> glibc.elision.enable: 0 (min: 0, max: 1) > >>>>>> glibc.malloc.mxfast: 0x0 (min: 0x0, max: 0xffffffff) > >>>>>> glibc.elision.skip_lock_busy: 3 (min: -2147483648, max: 2147483647) > >>>>>> glibc.malloc.top_pad: 0x0 (min: 0x0, max: 0xffffffff) > >>>>>> glibc.cpu.x86_non_temporal_threshold: 0x600000 (min: 0x0, max: 0xffffffff) > >>>>>> glibc.cpu.x86_shstk: > >>>>>> glibc.cpu.hwcap_mask: 0x6 (min: 0x0, max: 0xffffffff) > >>>>>> glibc.malloc.mmap_max: 0 (min: -2147483648, max: 2147483647) > >>>>>> glibc.elision.skip_trylock_internal_abort: 3 (min: -2147483648, max: 2147483647) > >>>>>> glibc.malloc.tcache_unsorted_limit: 0x0 (min: 0x0, max: 0xffffffff) > >>>>>> glibc.cpu.x86_ibt: > >>>>>> glibc.cpu.hwcaps: > >>>>>> glibc.elision.skip_lock_internal_abort: 3 (min: -2147483648, max: 2147483647) > >>>>>> glibc.malloc.arena_max: 0x0 (min: 0x1, max: 0xffffffff) > >>>>>> glibc.malloc.mmap_threshold: 0x0 (min: 0x0, max: 0xffffffff) > >>>>>> glibc.cpu.x86_data_cache_size: 0x8000 (min: 0x0, max: 0xffffffff) > >>>>>> glibc.malloc.tcache_count: 0x0 (min: 0x0, max: 0xffffffff) > >>>>>> glibc.malloc.arena_test: 0x0 (min: 0x1, max: 0xffffffff) > >>>>>> glibc.pthread.mutex_spin_count: 100 (min: 0, max: 32767) > >>>>>> glibc.malloc.tcache_max: 0x0 (min: 0x0, max: 0xffffffff) > >>>>>> glibc.malloc.check: 0 (min: 0, max: 3) > >>>>>> $ > >>>>>> > >>>>>> Ok for master? > >>>>>> > >>>>> > >>>>> Here is the updated patch. To support --list-tunables, a target should add > >>>>> > >>>>> CPPFLAGS-version.c = -DLIBC_MAIN=__libc_main_body > >>>>> CPPFLAGS-libc-main.S = -DLIBC_MAIN=__libc_main_body > >>>>> > >>>>> and start.S should be updated to define __libc_main and call > >>>>> __libc_main_body: > >>>>> > >>>>> extern void __libc_main_body (int argc, char **argv) > >>>>> __attribute__ ((noreturn, visibility ("hidden"))); > >>>>> > >>>>> when LIBC_MAIN is defined. > >>>> > >>>> I like where this patch is going, but the __libc_main wiring up means > >>>> we'll have to delay this until glibc 2.33 opens for development and > >>>> give the architectures time to fill in the required pieces of assembly. > >>>> > >>>> Can we split this into: > >>>> > >>>> (a) Minimum required to implement the feature e.g. just the tunable without > >>>> my requested changes. > >>>> > >>>> (b) A second patch which implements the --list-tunables that users can > >>>> then use to know what the values they can choose are. > >>>> > >>>> That way we can commit (a) right now, and then commit (b) when we > >>>> reopen for development? > >>>> > >>> > >>> Like this? > >> > >> Almost. > >> > >> Why do we still use a constructor? > >> > >> Why don't we accurately set the min and max? > >> > >> +#if HAVE_TUNABLES > >> + TUNABLE_UPDATE (x86_non_temporal_threshold, long int, > >> + __x86_shared_non_temporal_threshold, 0, > >> + (long int) -1); > >> + TUNABLE_UPDATE (x86_rep_movsb_threshold, long int, > >> + __x86_rep_movsb_threshold, > >> + minimum_rep_movsb_threshold, (long int) -1); > >> + TUNABLE_UPDATE (x86_rep_stosb_threshold, long int, > >> + __x86_rep_stosb_threshold, 0, (long int) -1); > >> > >> A min and max of 0 and -1 respectively could have been set in the tunables > >> list file and are not dynamic? > >> > >> I'd expect your patch would do everything except actually implement > >> --list-tunables. > > > > Here is the followup patch which does it. > > > >> > >> We need a manual page, and I accept that showing a "lower value" will > >> have to wait for --list-tunables. > >> > >> Otherwise the patch is looking ready. > > > > > > Are these 2 patches OK for trunk? > > Could you please post the patches in a distinct thread with a clear > subject, that way I know exactly what I'm applying and testing. > I'll review those ASAP so we can get something in place. > Done: https://sourceware.org/pipermail/libc-alpha/2020-July/115759.html -- H.J.