From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pj1-x1033.google.com (mail-pj1-x1033.google.com [IPv6:2607:f8b0:4864:20::1033]) by sourceware.org (Postfix) with ESMTPS id DF561383F422 for ; Tue, 6 Jul 2021 13:00:31 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org DF561383F422 Received: by mail-pj1-x1033.google.com with SMTP id d9-20020a17090ae289b0290172f971883bso854753pjz.1 for ; Tue, 06 Jul 2021 06:00:31 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=9Y5Fd5cBxjzCn/y6VXEeDw/VXaJatAt3ol0zG7d6F7E=; b=Jg9CjbaNpVZtT+6aTNbXwQMrvBhSUTcsAJe5K1lj/Qstyy1H8ZlTNwt/CreYYrQXa4 RVps2NHHJ/6kCFUilRw68N8H0hqHv7jkP0EacJUJs5Z2AT66p7a+t7dNAceIzZwNBIex 6xHnkK6QbD8ICT4sqlF2HT3h/mwXP+ayahGsmaICk5/1Va3oQaRlx2TB4bFQ1rCO3pKc +6arV3SLhDOkB8VsoqHgjRZD9hRms4zSMOYUNF7eg6V0HjCNZT3uxl5E7rmCaT38RkBT Ere+W/YKETr/bFTIwIQ43pflfKzqx6r26aeEt8l/Nq0/w9QcYK797KWCDS6+FvPdNOZu Yoyw== X-Gm-Message-State: AOAM530Cr6wslBYHmP7i3sobexigOqP6QTw9oqs5tWTB9i2VwLgZCCgP W8LJlFgcwUGs3U/syqE7MzLiIw== X-Google-Smtp-Source: ABdhPJzBKgbrV0/m9z4e7QzNWrSuDjtj+ABOwz9FdK2bNJKF3f9C/iOxCx9s7x2zb5CF2MkXvmItGg== X-Received: by 2002:a17:902:ba89:b029:11e:7a87:207 with SMTP id k9-20020a170902ba89b029011e7a870207mr16658875pls.81.1625576430861; Tue, 06 Jul 2021 06:00:30 -0700 (PDT) Received: from [192.168.1.108] ([177.194.59.218]) by smtp.gmail.com with ESMTPSA id p12sm8407635pfo.94.2021.07.06.06.00.29 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 06 Jul 2021 06:00:30 -0700 (PDT) Subject: Re: [PATCH] Linux: Avoid calling malloc indirectly from __get_nprocs To: Florian Weimer Cc: libc-alpha@sourceware.org, Siddhesh Poyarekar References: <87a6n7jvbe.fsf@oldenburg.str.redhat.com> <6b2c977a-fa75-c457-ceb7-152a3177109c@linaro.org> <87im1nfvy2.fsf@oldenburg.str.redhat.com> From: Adhemerval Zanella Message-ID: <9f426f84-6134-325c-524f-73518f700574@linaro.org> Date: Tue, 6 Jul 2021 10:00:27 -0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.11.0 MIME-Version: 1.0 In-Reply-To: <87im1nfvy2.fsf@oldenburg.str.redhat.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-6.4 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org 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: , X-List-Received-Date: Tue, 06 Jul 2021 13:00:33 -0000 On 06/07/2021 09:50, Florian Weimer wrote: > * Adhemerval Zanella: > >>> +/* Compute the population count of the entire array. */ >>> +static int >>> +__get_nprocs_count (const unsigned long int *array, size_t length) >>> +{ >>> + int count = 0; >>> + for (size_t i = 0; i < length; ++i) >>> + if (__builtin_add_overflow (count, __builtin_popcountl (array[i]), >>> + &count)) >>> + return INT_MAX; >>> + return count; >>> +} >> >> Could we avoid replicate the same logic over different files? We have a >> countbits() on posix/sched_cpucount.c, so I think it would better to move >> it on a sched.h and use it instead (there is no need to really handle >> overflow here, it would require a *very* large buffer...). > > Probably, yes. > > Should I send a patch? I was busy with other stuff last week, but I > should be able to work on fixing this now. I have this one m backlog, I will prob send a patch this week. > >>> +/* __get_nprocs with a large buffer. */ >>> +static int >>> +__get_nprocs_large (void) >>> +{ >>> + /* This code cannot use scratch_buffer because it is used during >>> + malloc initialization. */ >>> + size_t pagesize = GLRO (dl_pagesize); >>> + unsigned long int *page = __mmap (0, pagesize, PROT_READ | PROT_WRITE, >>> + MAP_ANONYMOUS | MAP_PRIVATE, -1, 0); >>> + if (page == MAP_FAILED) >>> + return 2; >>> + int r = INTERNAL_SYSCALL_CALL (sched_getaffinity, 0, pagesize, page); >>> + int count; >>> + if (r > 0) >>> + count = __get_nprocs_count (page, pagesize / sizeof (unsigned long int)); >>> + else if (r == -EINVAL) >>> + /* One page is still not enough to store the bits. A more-or-less >>> + arbitrary value. This assumes t hat such large systems never >>> + happen in practice. */ >>> + count = GLRO (dl_pagesize) * CHAR_BIT; >>> + else >>> + count = 2; >>> + __munmap (page, GLRO (dl_pagesize)); >> >> Maybe use pagesize here since you are defining it. > > Right. > >> I would prefer that since now we don't iterate increasing the buffer >> size for sched_getaffinity we go for a simplified version and use a >> large buffer instead. >> >> Linux currently supports at maximum of 4096 cpus for most >> architectures: > > I'm not sure if that's a good idea. I think some distributions patched > the defaults in the past. The limit isn't really set in stone. But we are still using an artificial limit here, although higher. Couldn't we do the other way around: make malloc use sched_getaffinity with a limited buffer and use either to the saturated value or 2 if syscall failed, and get back to use a scratch_buffer on libc provided symbol (which does not have any limit and simpler to handle than stack plus mmap)? For malloc it does not seems critical to get the fully correct CPU number for large systems.