Message ID | 2df8703a77c3bce4fbdcf523a9b5c3ddf7491b9b.1603693614.git.zong.li@sifive.com |
---|---|
State | Superseded |
Headers |
Return-Path: <libc-alpha-bounces@sourceware.org> X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 768823857C7E; Mon, 26 Oct 2020 07:24:57 +0000 (GMT) X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from mail-pf1-x442.google.com (mail-pf1-x442.google.com [IPv6:2607:f8b0:4864:20::442]) by sourceware.org (Postfix) with ESMTPS id E1C9E3857C73 for <libc-alpha@sourceware.org>; Mon, 26 Oct 2020 07:24:53 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org E1C9E3857C73 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=sifive.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=zong.li@sifive.com Received: by mail-pf1-x442.google.com with SMTP id e7so5690249pfn.12 for <libc-alpha@sourceware.org>; Mon, 26 Oct 2020 00:24:53 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sifive.com; s=google; h=from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=hQdcidD413w9dbB/Z41PWwqc0Qt3JhAD3PwpKwkzTUA=; b=SCJ85gcPIoJqMGtCvtrfKCMnjU491/UTwBZrabqOLQxo+WGX9/QmXnNHf+NapXqRWL lg1C0FgyqWIKZ2Af/1+/q81BDL5m7QukENTccsUAEbnQfNsSBDvdAuyFcGt6uzK67hsl /Ra5ZoQgRswbNc2jhEbRjPg0G7oFCFPSXwtuk/6KXTdxLgl/ltCK0w9FMLr06fev9mQo h3+nvW0Bp4AQ3Lf3RhH+5YCzIRrskfpwdt9W2uTT2KEvgBUgz1bwVfVMjSfvWBJnJCT0 4iRUo9WqIWcTjVD7OD3KfvxOcJcsUXnIRUDE4GrsHaFTNy8Q8I7w+fyoOyrtItowQ5H6 yPnw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=hQdcidD413w9dbB/Z41PWwqc0Qt3JhAD3PwpKwkzTUA=; b=eH84HMgrttkr5SFHYOxtGjsBKYTKy42B2JFUjMtqHU/VDy0BzWlB+sjsPn25MmopoZ M9mUqigvqBCTIyi0gTRiOqX6umJkr2rdFXi/UjCuCE5djDqPUoRU+ElFvS/ZrKHZp10Z 8aWhNy8uzDMlYPOlQuf2nXSCx9wBOSRVQAFlG9STWf6ajuHUBpsDn0u/BWQSz4jAJuYn MvSl2mZMbF9SIS7LdFtHgRN9/uQDjcz04pbFWZixzG+QfgAv5VX090iQJRy2HAcBDbYt zQVJVBiJPIWQSX0wQUZSFISZjQvFjTAnGeaP+bIHDI9gcUBNAcYv/xtQUOgdsKvTKz2Z AR9g== X-Gm-Message-State: AOAM531zSAVd5a+unFqxyhEXluRDp680Fo7JU99Cg4EkXmeiAdymvBYJ f2Ez8IeMC1nLmNOPcJYIeHqV8ooskdqz1N7A X-Google-Smtp-Source: ABdhPJy5RNr2o5/tBqEsl3hcohPcZlJfAppJzLDkRedKZAMmCuBiKS9pYkElHZeqO0sjWcvIWVFwdg== X-Received: by 2002:a63:d252:: with SMTP id t18mr11494189pgi.300.1603697092674; Mon, 26 Oct 2020 00:24:52 -0700 (PDT) Received: from hsinchu02.internal.sifive.com (114-34-229-221.HINET-IP.hinet.net. [114.34.229.221]) by smtp.gmail.com with ESMTPSA id c12sm10519670pgi.14.2020.10.26.00.24.50 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 26 Oct 2020 00:24:52 -0700 (PDT) From: Zong Li <zong.li@sifive.com> To: libc-alpha@sourceware.org, joseph@codesourcery.com Subject: [PATCH 1/1] riscv: Get cache information through sysconf Date: Mon, 26 Oct 2020 15:24:47 +0800 Message-Id: <2df8703a77c3bce4fbdcf523a9b5c3ddf7491b9b.1603693614.git.zong.li@sifive.com> X-Mailer: git-send-email 2.28.0 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-13.9 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list <libc-alpha.sourceware.org> List-Unsubscribe: <https://sourceware.org/mailman/options/libc-alpha>, <mailto:libc-alpha-request@sourceware.org?subject=unsubscribe> List-Archive: <https://sourceware.org/pipermail/libc-alpha/> List-Post: <mailto:libc-alpha@sourceware.org> List-Help: <mailto:libc-alpha-request@sourceware.org?subject=help> List-Subscribe: <https://sourceware.org/mailman/listinfo/libc-alpha>, <mailto:libc-alpha-request@sourceware.org?subject=subscribe> Cc: Zong Li <zong.li@sifive.com>, andrew@sifive.com Errors-To: libc-alpha-bounces@sourceware.org Sender: "Libc-alpha" <libc-alpha-bounces@sourceware.org> |
Series |
[1/1] riscv: Get cache information through sysconf
|
|
Commit Message
Zong Li
Oct. 26, 2020, 7:24 a.m. UTC
Add support to query cache information on RISC-V through sysconf() function. The cache information had been added in AUX vector of RISC-V architecture in Linux kernel v.5.10-rc1. --- sysdeps/unix/sysv/linux/riscv/sysconf.c | 74 +++++++++++++++++++++++++ 1 file changed, 74 insertions(+) create mode 100644 sysdeps/unix/sysv/linux/riscv/sysconf.c
Comments
* Zong Li: > Add support to query cache information on RISC-V through sysconf() > function. The cache information had been added in AUX vector of RISC-V > architecture in Linux kernel v.5.10-rc1. I think this needs error reporting in case the auxiliary vector does not contain the requested information.
On Mon, Oct 26, 2020 at 4:27 PM Florian Weimer <fw@deneb.enyo.de> wrote: > > * Zong Li: > > > Add support to query cache information on RISC-V through sysconf() > > function. The cache information had been added in AUX vector of RISC-V > > architecture in Linux kernel v.5.10-rc1. > > I think this needs error reporting in case the auxiliary vector does > not contain the requested information. Does the error reporting mean that we need to check the errno and return the error number when AUX vector is unavailable?
* Zong Li: > On Mon, Oct 26, 2020 at 4:27 PM Florian Weimer <fw@deneb.enyo.de> wrote: >> >> * Zong Li: >> >> > Add support to query cache information on RISC-V through sysconf() >> > function. The cache information had been added in AUX vector of RISC-V >> > architecture in Linux kernel v.5.10-rc1. >> >> I think this needs error reporting in case the auxiliary vector does >> not contain the requested information. > > Does the error reporting mean that we need to check the errno and > return the error number when AUX vector is unavailable? I think you need to return -1 and arrange for errno being set. getauxval sets errno, but the masking and shifting means that sysconf does not necessarily return -1.
On Tue, Oct 27, 2020 at 3:29 PM Florian Weimer <fw@deneb.enyo.de> wrote: > > * Zong Li: > > > On Mon, Oct 26, 2020 at 4:27 PM Florian Weimer <fw@deneb.enyo.de> wrote: > >> > >> * Zong Li: > >> > >> > Add support to query cache information on RISC-V through sysconf() > >> > function. The cache information had been added in AUX vector of RISC-V > >> > architecture in Linux kernel v.5.10-rc1. > >> > >> I think this needs error reporting in case the auxiliary vector does > >> not contain the requested information. > > > > Does the error reporting mean that we need to check the errno and > > return the error number when AUX vector is unavailable? > > I think you need to return -1 and arrange for errno being set. > getauxval sets errno, but the masking and shifting means that > sysconf does not necessarily return -1. Yeah, it is good to me for returning -1. For errno, it seems to me that we could keep the errno which is set by getauxval, the ENOENT is good to present the lack of information in the AUX vector, so we don't set the errno again. I was wondering if the following changes are good to you as well? diff --git a/sysdeps/unix/sysv/linux/riscv/sysconf.c b/sysdeps/unix/sysv/linux/riscv/sysconf.c index e73095528c..22ac4f8d19 100644 --- a/sysdeps/unix/sysv/linux/riscv/sysconf.c +++ b/sysdeps/unix/sysv/linux/riscv/sysconf.c @@ -24,19 +24,34 @@ static long int linux_sysconf (int name); static inline long int sysconf_get_cache_associativity (unsigned long type) { - return (__getauxval (type) & 0xffff0000) >> 16; + unsigned long int val = __getauxval (type); + + if (errno == ENOENT) + return -1; + else + return (val & 0xffff0000) >> 16; } static inline long int sysconf_get_cache_linesize (unsigned long type) { - return __getauxval (type) & 0xffff; + unsigned long int val = __getauxval (type); + + if (errno == ENOENT) + return -1; + else + return val & 0xffff; } static inline long int sysconf_get_cache_size (unsigned long type) { - return __getauxval (type); + unsigned long int val = __getauxval (type); + + if (errno == ENOENT) + return -1; + else + return val; }
On Tue, Oct 27, 2020 at 12:46 AM Zong Li <zong.li@sifive.com> wrote: > > On Tue, Oct 27, 2020 at 3:29 PM Florian Weimer <fw@deneb.enyo.de> wrote: > > > > * Zong Li: > > > > > On Mon, Oct 26, 2020 at 4:27 PM Florian Weimer <fw@deneb.enyo.de> wrote: > > >> > > >> * Zong Li: > > >> > > >> > Add support to query cache information on RISC-V through sysconf() > > >> > function. The cache information had been added in AUX vector of RISC-V > > >> > architecture in Linux kernel v.5.10-rc1. > > >> > > >> I think this needs error reporting in case the auxiliary vector does > > >> not contain the requested information. > > > > > > Does the error reporting mean that we need to check the errno and > > > return the error number when AUX vector is unavailable? > > > > I think you need to return -1 and arrange for errno being set. > > getauxval sets errno, but the masking and shifting means that > > sysconf does not necessarily return -1. > > Yeah, it is good to me for returning -1. For errno, it seems to me > that we could keep the errno which is set by getauxval, the ENOENT is > good to present the lack of information in the AUX vector, so we don't > set the errno again. I was wondering if the following changes are good > to you as well? I believe getauxval does not clear errno on success, so to use this approach, you'd need to save errno, clear it, call getauxval, check errno, then OR the saved errno into the new one. Instead, can we treat a getauxval return value of 0 as an error in these cases? Then, we could check for a 0 return value, set errno, and return -1. > > diff --git a/sysdeps/unix/sysv/linux/riscv/sysconf.c > b/sysdeps/unix/sysv/linux/riscv/sysconf.c > index e73095528c..22ac4f8d19 100644 > --- a/sysdeps/unix/sysv/linux/riscv/sysconf.c > +++ b/sysdeps/unix/sysv/linux/riscv/sysconf.c > @@ -24,19 +24,34 @@ static long int linux_sysconf (int name); > static inline long int > sysconf_get_cache_associativity (unsigned long type) > { > - return (__getauxval (type) & 0xffff0000) >> 16; > + unsigned long int val = __getauxval (type); > + > + if (errno == ENOENT) > + return -1; > + else > + return (val & 0xffff0000) >> 16; > } > > static inline long int > sysconf_get_cache_linesize (unsigned long type) > { > - return __getauxval (type) & 0xffff; > + unsigned long int val = __getauxval (type); > + > + if (errno == ENOENT) > + return -1; > + else > + return val & 0xffff; > } > > static inline long int > sysconf_get_cache_size (unsigned long type) > { > - return __getauxval (type); > + unsigned long int val = __getauxval (type); > + > + if (errno == ENOENT) > + return -1; > + else > + return val; > }
On Okt 27 2020, Zong Li wrote: > diff --git a/sysdeps/unix/sysv/linux/riscv/sysconf.c > b/sysdeps/unix/sysv/linux/riscv/sysconf.c > index e73095528c..22ac4f8d19 100644 > --- a/sysdeps/unix/sysv/linux/riscv/sysconf.c > +++ b/sysdeps/unix/sysv/linux/riscv/sysconf.c > @@ -24,19 +24,34 @@ static long int linux_sysconf (int name); > static inline long int > sysconf_get_cache_associativity (unsigned long type) > { > - return (__getauxval (type) & 0xffff0000) >> 16; > + unsigned long int val = __getauxval (type); > + > + if (errno == ENOENT) > + return -1; > + else > + return (val & 0xffff0000) >> 16; You need to clear errno before calling __getauxval. Andreas.
On Tue, Oct 27, 2020 at 1:29 AM Andreas Schwab <schwab@linux-m68k.org> wrote: > > On Okt 27 2020, Zong Li wrote: > > > diff --git a/sysdeps/unix/sysv/linux/riscv/sysconf.c > > b/sysdeps/unix/sysv/linux/riscv/sysconf.c > > index e73095528c..22ac4f8d19 100644 > > --- a/sysdeps/unix/sysv/linux/riscv/sysconf.c > > +++ b/sysdeps/unix/sysv/linux/riscv/sysconf.c > > @@ -24,19 +24,34 @@ static long int linux_sysconf (int name); > > static inline long int > > sysconf_get_cache_associativity (unsigned long type) > > { > > - return (__getauxval (type) & 0xffff0000) >> 16; > > + unsigned long int val = __getauxval (type); > > + > > + if (errno == ENOENT) > > + return -1; > > + else > > + return (val & 0xffff0000) >> 16; > > You need to clear errno before calling __getauxval. Err, what Andreas said. > > Andreas. > > -- > Andreas Schwab, schwab@linux-m68k.org > GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1 > "And now for something completely different."
On Tue, Oct 27, 2020 at 4:13 PM Andrew Waterman <andrew@sifive.com> wrote: > > On Tue, Oct 27, 2020 at 12:46 AM Zong Li <zong.li@sifive.com> wrote: > > > > On Tue, Oct 27, 2020 at 3:29 PM Florian Weimer <fw@deneb.enyo.de> wrote: > > > > > > * Zong Li: > > > > > > > On Mon, Oct 26, 2020 at 4:27 PM Florian Weimer <fw@deneb.enyo.de> wrote: > > > >> > > > >> * Zong Li: > > > >> > > > >> > Add support to query cache information on RISC-V through sysconf() > > > >> > function. The cache information had been added in AUX vector of RISC-V > > > >> > architecture in Linux kernel v.5.10-rc1. > > > >> > > > >> I think this needs error reporting in case the auxiliary vector does > > > >> not contain the requested information. > > > > > > > > Does the error reporting mean that we need to check the errno and > > > > return the error number when AUX vector is unavailable? > > > > > > I think you need to return -1 and arrange for errno being set. > > > getauxval sets errno, but the masking and shifting means that > > > sysconf does not necessarily return -1. > > > > Yeah, it is good to me for returning -1. For errno, it seems to me > > that we could keep the errno which is set by getauxval, the ENOENT is > > good to present the lack of information in the AUX vector, so we don't > > set the errno again. I was wondering if the following changes are good > > to you as well? > > I believe getauxval does not clear errno on success, so to use this > approach, you'd need to save errno, clear it, call getauxval, check > errno, then OR the saved errno into the new one. > Okay, thanks for pointing out that, I would fix it in the next version. > Instead, can we treat a getauxval return value of 0 as an error in > these cases? Then, we could check for a 0 return value, set errno, > and return -1. > The return value of 0 from getauxval couldn't recognize the error for the following difference: - AUX vector doesn't contain the requested information. - AUX vector contains the information, but it gets the zero value due to no caches in real. So we might still need to use ENOENT which is set by getauxval to recognize the difference. > > > > > diff --git a/sysdeps/unix/sysv/linux/riscv/sysconf.c > > b/sysdeps/unix/sysv/linux/riscv/sysconf.c > > index e73095528c..22ac4f8d19 100644 > > --- a/sysdeps/unix/sysv/linux/riscv/sysconf.c > > +++ b/sysdeps/unix/sysv/linux/riscv/sysconf.c > > @@ -24,19 +24,34 @@ static long int linux_sysconf (int name); > > static inline long int > > sysconf_get_cache_associativity (unsigned long type) > > { > > - return (__getauxval (type) & 0xffff0000) >> 16; > > + unsigned long int val = __getauxval (type); > > + > > + if (errno == ENOENT) > > + return -1; > > + else > > + return (val & 0xffff0000) >> 16; > > } > > > > static inline long int > > sysconf_get_cache_linesize (unsigned long type) > > { > > - return __getauxval (type) & 0xffff; > > + unsigned long int val = __getauxval (type); > > + > > + if (errno == ENOENT) > > + return -1; > > + else > > + return val & 0xffff; > > } > > > > static inline long int > > sysconf_get_cache_size (unsigned long type) > > { > > - return __getauxval (type); > > + unsigned long int val = __getauxval (type); > > + > > + if (errno == ENOENT) > > + return -1; > > + else > > + return val; > > }
On Tue, Oct 27, 2020 at 4:32 PM Andrew Waterman <andrew@sifive.com> wrote: > > On Tue, Oct 27, 2020 at 1:29 AM Andreas Schwab <schwab@linux-m68k.org> wrote: > > > > On Okt 27 2020, Zong Li wrote: > > > > > diff --git a/sysdeps/unix/sysv/linux/riscv/sysconf.c > > > b/sysdeps/unix/sysv/linux/riscv/sysconf.c > > > index e73095528c..22ac4f8d19 100644 > > > --- a/sysdeps/unix/sysv/linux/riscv/sysconf.c > > > +++ b/sysdeps/unix/sysv/linux/riscv/sysconf.c > > > @@ -24,19 +24,34 @@ static long int linux_sysconf (int name); > > > static inline long int > > > sysconf_get_cache_associativity (unsigned long type) > > > { > > > - return (__getauxval (type) & 0xffff0000) >> 16; > > > + unsigned long int val = __getauxval (type); > > > + > > > + if (errno == ENOENT) > > > + return -1; > > > + else > > > + return (val & 0xffff0000) >> 16; > > > > You need to clear errno before calling __getauxval. > > Err, what Andreas said. > Thanks to reviewing that, fix it in the next version. > > > > Andreas. > > > > -- > > Andreas Schwab, schwab@linux-m68k.org > > GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1 > > "And now for something completely different."
* Zong Li: > Okay, thanks for pointing out that, I would fix it in the next > version. I posted a patch that should simplify the implementation: Subject: [PATCH] misc: Add internal __getauxval2 function Thanks, Florian
On Tue, Oct 27, 2020 at 4:52 PM Zong Li <zong.li@sifive.com> wrote: > > On Tue, Oct 27, 2020 at 4:32 PM Andrew Waterman <andrew@sifive.com> wrote: > > > > On Tue, Oct 27, 2020 at 1:29 AM Andreas Schwab <schwab@linux-m68k.org> wrote: > > > > > > On Okt 27 2020, Zong Li wrote: > > > > > > > diff --git a/sysdeps/unix/sysv/linux/riscv/sysconf.c > > > > b/sysdeps/unix/sysv/linux/riscv/sysconf.c > > > > index e73095528c..22ac4f8d19 100644 > > > > --- a/sysdeps/unix/sysv/linux/riscv/sysconf.c > > > > +++ b/sysdeps/unix/sysv/linux/riscv/sysconf.c > > > > @@ -24,19 +24,34 @@ static long int linux_sysconf (int name); > > > > static inline long int > > > > sysconf_get_cache_associativity (unsigned long type) > > > > { > > > > - return (__getauxval (type) & 0xffff0000) >> 16; > > > > + unsigned long int val = __getauxval (type); > > > > + > > > > + if (errno == ENOENT) > > > > + return -1; > > > > + else > > > > + return (val & 0xffff0000) >> 16; > > > > > > You need to clear errno before calling __getauxval. > > > > Err, what Andreas said. > > > > Thanks to reviewing that, fix it in the next version. > > > > > > > Andreas. > > > > > > -- > > > Andreas Schwab, schwab@linux-m68k.org > > > GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1 > > > "And now for something completely different." Before sending the next version, I'd like to make sure that I actually pick up the suggestions, so I give the changes here as follows. I clear the errno before calling getauxval, return -1 and keep errno as ENOENT if we get failure in getauxval, save and restore the original errno number if getauxval is successful. static inline long int sysconf_get_cache_associativity (unsigned long type) { - return (__getauxval (type) & 0xffff0000) >> 16; + unsigned long int val; + int save_errno = errno; + + __set_errno (0); + val = __getauxval (type); + + if (errno == ENOENT) + return -1; + + __set_errno (save_errno); + return (val & 0xffff0000) >> 16; } static inline long int sysconf_get_cache_linesize (unsigned long type) { - return __getauxval (type) & 0xffff; + unsigned long int val; + int save_errno = errno; + + __set_errno (0); + val = __getauxval (type); + + if (errno == ENOENT) + return -1; + + __set_errno (save_errno); + return val & 0xffff; } static inline long int sysconf_get_cache_size (unsigned long type) { - return __getauxval (type); + unsigned long int val; + int save_errno = errno; + + __set_errno (0); + val = __getauxval (type); + + if (errno == ENOENT) + return -1; + + __set_errno (save_errno); + return val; }
On Tue, Oct 27, 2020 at 5:44 PM Florian Weimer <fweimer@redhat.com> wrote: > > * Zong Li: > > > Okay, thanks for pointing out that, I would fix it in the next > > version. > > I posted a patch that should simplify the implementation: > > Subject: [PATCH] misc: Add internal __getauxval2 function > Many thanks, I should base on top of your patch, let me change to use __getauxval2 here. > Thanks, > Florian > -- > Red Hat GmbH, https://de.redhat.com/ , Registered seat: Grasbrunn, > Commercial register: Amtsgericht Muenchen, HRB 153243, > Managing Directors: Charles Cachera, Brian Klemm, Laurie Krebs, Michael O'Neill >
* Zong Li: > On Tue, Oct 27, 2020 at 5:44 PM Florian Weimer <fweimer@redhat.com> wrote: >> >> * Zong Li: >> >> > Okay, thanks for pointing out that, I would fix it in the next >> > version. >> >> I posted a patch that should simplify the implementation: >> >> Subject: [PATCH] misc: Add internal __getauxval2 function >> > > Many thanks, I should base on top of your patch, let me change to use > __getauxval2 here. It probably makes sense to add your own static local function getauxval2_einval, with the same signature as __getauxval2, but which also sets EINVAL on error. Thanks, Florian
On Tue, Oct 27, 2020 at 6:15 PM Florian Weimer <fweimer@redhat.com> wrote: > > * Zong Li: > > > On Tue, Oct 27, 2020 at 5:44 PM Florian Weimer <fweimer@redhat.com> wrote: > >> > >> * Zong Li: > >> > >> > Okay, thanks for pointing out that, I would fix it in the next > >> > version. > >> > >> I posted a patch that should simplify the implementation: > >> > >> Subject: [PATCH] misc: Add internal __getauxval2 function > >> > > > > Many thanks, I should base on top of your patch, let me change to use > > __getauxval2 here. > > It probably makes sense to add your own static local function > getauxval2_einval, with the same signature as __getauxval2, but which > also sets EINVAL on error. > Like my previous mail, I'd like to make sure that I actually pick up all suggestions before I send the next version. This modification adds a local getauxval2_einval function for internal use to handle the error setting. Thanks for everyone's review. +static bool +getauxval2_einval (unsigned long int type, unsigned long int *result) +{ + int save_errno = errno; + + __set_errno (0); + + if (!__getauxval2 (type, result)) + { + __set_errno (EINVAL); + return false; + } + + __set_errno (save_errno); + + return true; +} + static inline long int sysconf_get_cache_associativity (unsigned long type) { - return (__getauxval (type) & 0xffff0000) >> 16; + unsigned long int result; + + if (getauxval2_einval (type, &result)) + return (result & 0xffff0000) >> 16; + + return -1; } static inline long int sysconf_get_cache_linesize (unsigned long type) { - return __getauxval (type) & 0xffff; + unsigned long int result; + + if (getauxval2_einval (type, &result)) + return result & 0xffff; + + return -1; } static inline long int sysconf_get_cache_size (unsigned long type) { - return __getauxval (type); + unsigned long int result; + + if (getauxval2_einval (type, &result)) + return result; + + return -1; } > Thanks, > Florian > -- > Red Hat GmbH, https://de.redhat.com/ , Registered seat: Grasbrunn, > Commercial register: Amtsgericht Muenchen, HRB 153243, > Managing Directors: Charles Cachera, Brian Klemm, Laurie Krebs, Michael O'Neill >
* Zong Li: > Like my previous mail, I'd like to make sure that I actually pick up > all suggestions before I send the next version. This modification adds > a local getauxval2_einval function for internal use to handle the > error setting. Thanks for everyone's review. > > +static bool > +getauxval2_einval (unsigned long int type, unsigned long int *result) > +{ > + int save_errno = errno; > + > + __set_errno (0); > + > + if (!__getauxval2 (type, result)) > + { > + __set_errno (EINVAL); > + return false; > + } > + > + __set_errno (save_errno); > + > + return true; > +} If you use __getauxval2, you don't have to save and restore errno. __set_errno (EINVAL) on error is enough. getauxval2_einval should probably be declared inline. The rest looks like what I would expect. Thanks, Florian
On Okt 29 2020, Zong Li wrote: > +static bool > +getauxval2_einval (unsigned long int type, unsigned long int *result) > +{ > + int save_errno = errno; > + > + __set_errno (0); There is no need to clear errno, since you no longer depend on its value. Andreas.
On Okt 29 2020, Florian Weimer via Libc-alpha wrote:
> getauxval2_einval should probably be declared inline.
For static functions inline is implied.
Andreas.
On Thu, Oct 29, 2020 at 4:53 PM Florian Weimer <fweimer@redhat.com> wrote: > > * Zong Li: > > > Like my previous mail, I'd like to make sure that I actually pick up > > all suggestions before I send the next version. This modification adds > > a local getauxval2_einval function for internal use to handle the > > error setting. Thanks for everyone's review. > > > > +static bool > > +getauxval2_einval (unsigned long int type, unsigned long int *result) > > +{ > > + int save_errno = errno; > > + > > + __set_errno (0); > > + > > + if (!__getauxval2 (type, result)) > > + { > > + __set_errno (EINVAL); > > + return false; > > + } > > + > > + __set_errno (save_errno); > > + > > + return true; > > +} > > If you use __getauxval2, you don't have to save and restore errno. > __set_errno (EINVAL) on error is enough. Okay, remove it in the next version. > > getauxval2_einval should probably be declared inline. > > The rest looks like what I would expect. > > Thanks, > Florian > -- > Red Hat GmbH, https://de.redhat.com/ , Registered seat: Grasbrunn, > Commercial register: Amtsgericht Muenchen, HRB 153243, > Managing Directors: Charles Cachera, Brian Klemm, Laurie Krebs, Michael O'Neill >
On Thu, Oct 29, 2020 at 5:32 PM Andreas Schwab <schwab@linux-m68k.org> wrote: > > On Okt 29 2020, Zong Li wrote: > > > +static bool > > +getauxval2_einval (unsigned long int type, unsigned long int *result) > > +{ > > + int save_errno = errno; > > + > > + __set_errno (0); > > There is no need to clear errno, since you no longer depend on its > value. > Okay, only set error in the next version. > Andreas. > > -- > Andreas Schwab, schwab@linux-m68k.org > GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1 > "And now for something completely different."
On Thu, Oct 29, 2020 at 5:56 PM Andreas Schwab <schwab@linux-m68k.org> wrote: > > On Okt 29 2020, Florian Weimer via Libc-alpha wrote: > > > getauxval2_einval should probably be declared inline. > > For static functions inline is implied. > There are other functions which I declare explicitly as inline, it might be good to me to make them be consistent. > Andreas. > > -- > Andreas Schwab, schwab@linux-m68k.org > GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1 > "And now for something completely different."
On Mon, 26 Oct 2020 00:24:47 PDT (-0700), zong.li@sifive.com wrote: > Add support to query cache information on RISC-V through sysconf() > function. The cache information had been added in AUX vector of RISC-V > architecture in Linux kernel v.5.10-rc1. > --- > sysdeps/unix/sysv/linux/riscv/sysconf.c | 74 +++++++++++++++++++++++++ > 1 file changed, 74 insertions(+) > create mode 100644 sysdeps/unix/sysv/linux/riscv/sysconf.c > > diff --git a/sysdeps/unix/sysv/linux/riscv/sysconf.c b/sysdeps/unix/sysv/linux/riscv/sysconf.c > new file mode 100644 > index 0000000000..e73095528c > --- /dev/null > +++ b/sysdeps/unix/sysv/linux/riscv/sysconf.c > @@ -0,0 +1,74 @@ > +/* Copyright (C) 2020 Free Software Foundation, Inc. > + This file is part of the GNU C Library. > + > + The GNU C Library is free software; you can redistribute it and/or > + modify it under the terms of the GNU Lesser General Public > + License as published by the Free Software Foundation; either > + version 2.1 of the License, or (at your option) any later version. > + > + The GNU C Library is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + Lesser General Public License for more details. > + > + You should have received a copy of the GNU Lesser General Public > + License along with the GNU C Library. If not, see > + <https://www.gnu.org/licenses/>. */ > + > +#include <errno.h> > +#include <unistd.h> > +#include <sys/auxv.h> > + > +static long int linux_sysconf (int name); > + > +static inline long int > +sysconf_get_cache_associativity (unsigned long type) > +{ > + return (__getauxval (type) & 0xffff0000) >> 16; > +} > + > +static inline long int > +sysconf_get_cache_linesize (unsigned long type) > +{ > + return __getauxval (type) & 0xffff; > +} > + > +static inline long int > +sysconf_get_cache_size (unsigned long type) > +{ > + return __getauxval (type); > +} > + > +/* Get the value of the system variable NAME. */ > +long int > +__sysconf (int name) > +{ > + switch (name) > + { > + case _SC_LEVEL1_ICACHE_SIZE: > + return sysconf_get_cache_size (AT_L1I_CACHESIZE); > + case _SC_LEVEL1_ICACHE_ASSOC: > + return sysconf_get_cache_associativity (AT_L1I_CACHEGEOMETRY); > + case _SC_LEVEL1_ICACHE_LINESIZE: > + return sysconf_get_cache_linesize (AT_L1I_CACHEGEOMETRY); > + case _SC_LEVEL1_DCACHE_SIZE: > + return sysconf_get_cache_size (AT_L1D_CACHESIZE); > + case _SC_LEVEL1_DCACHE_ASSOC: > + return sysconf_get_cache_associativity (AT_L1D_CACHEGEOMETRY); > + case _SC_LEVEL1_DCACHE_LINESIZE: > + return sysconf_get_cache_linesize (AT_L1D_CACHEGEOMETRY); > + case _SC_LEVEL2_CACHE_SIZE: > + return sysconf_get_cache_size (AT_L2_CACHESIZE); > + case _SC_LEVEL2_CACHE_ASSOC: > + return sysconf_get_cache_associativity (AT_L2_CACHEGEOMETRY); > + case _SC_LEVEL2_CACHE_LINESIZE: > + return sysconf_get_cache_linesize (AT_L2_CACHEGEOMETRY); > + default: > + return linux_sysconf (name); > + } > +} > + > +/* Now the generic Linux version. */ > +#undef __sysconf > +#define __sysconf static linux_sysconf > +#include <sysdeps/unix/sysv/linux/sysconf.c> Reviewed-by: Palmer Dabbelt <palmerdabbelt@google.com> Acked-by: Palmer Dabbelt <palmerdabbelt@google.com>
On Fri, Nov 6, 2020 at 12:57 PM Palmer Dabbelt <palmer@dabbelt.com> wrote: > > On Mon, 26 Oct 2020 00:24:47 PDT (-0700), zong.li@sifive.com wrote: > > Add support to query cache information on RISC-V through sysconf() > > function. The cache information had been added in AUX vector of RISC-V > > architecture in Linux kernel v.5.10-rc1. > > --- > > sysdeps/unix/sysv/linux/riscv/sysconf.c | 74 +++++++++++++++++++++++++ > > 1 file changed, 74 insertions(+) > > create mode 100644 sysdeps/unix/sysv/linux/riscv/sysconf.c > > > > diff --git a/sysdeps/unix/sysv/linux/riscv/sysconf.c b/sysdeps/unix/sysv/linux/riscv/sysconf.c > > new file mode 100644 > > index 0000000000..e73095528c > > --- /dev/null > > +++ b/sysdeps/unix/sysv/linux/riscv/sysconf.c > > @@ -0,0 +1,74 @@ > > +/* Copyright (C) 2020 Free Software Foundation, Inc. > > + This file is part of the GNU C Library. > > + > > + The GNU C Library is free software; you can redistribute it and/or > > + modify it under the terms of the GNU Lesser General Public > > + License as published by the Free Software Foundation; either > > + version 2.1 of the License, or (at your option) any later version. > > + > > + The GNU C Library is distributed in the hope that it will be useful, > > + but WITHOUT ANY WARRANTY; without even the implied warranty of > > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > > + Lesser General Public License for more details. > > + > > + You should have received a copy of the GNU Lesser General Public > > + License along with the GNU C Library. If not, see > > + <https://www.gnu.org/licenses/>. */ > > + > > +#include <errno.h> > > +#include <unistd.h> > > +#include <sys/auxv.h> > > + > > +static long int linux_sysconf (int name); > > + > > +static inline long int > > +sysconf_get_cache_associativity (unsigned long type) > > +{ > > + return (__getauxval (type) & 0xffff0000) >> 16; > > +} > > + > > +static inline long int > > +sysconf_get_cache_linesize (unsigned long type) > > +{ > > + return __getauxval (type) & 0xffff; > > +} > > + > > +static inline long int > > +sysconf_get_cache_size (unsigned long type) > > +{ > > + return __getauxval (type); > > +} > > + > > +/* Get the value of the system variable NAME. */ > > +long int > > +__sysconf (int name) > > +{ > > + switch (name) > > + { > > + case _SC_LEVEL1_ICACHE_SIZE: > > + return sysconf_get_cache_size (AT_L1I_CACHESIZE); > > + case _SC_LEVEL1_ICACHE_ASSOC: > > + return sysconf_get_cache_associativity (AT_L1I_CACHEGEOMETRY); > > + case _SC_LEVEL1_ICACHE_LINESIZE: > > + return sysconf_get_cache_linesize (AT_L1I_CACHEGEOMETRY); > > + case _SC_LEVEL1_DCACHE_SIZE: > > + return sysconf_get_cache_size (AT_L1D_CACHESIZE); > > + case _SC_LEVEL1_DCACHE_ASSOC: > > + return sysconf_get_cache_associativity (AT_L1D_CACHEGEOMETRY); > > + case _SC_LEVEL1_DCACHE_LINESIZE: > > + return sysconf_get_cache_linesize (AT_L1D_CACHEGEOMETRY); > > + case _SC_LEVEL2_CACHE_SIZE: > > + return sysconf_get_cache_size (AT_L2_CACHESIZE); > > + case _SC_LEVEL2_CACHE_ASSOC: > > + return sysconf_get_cache_associativity (AT_L2_CACHEGEOMETRY); > > + case _SC_LEVEL2_CACHE_LINESIZE: > > + return sysconf_get_cache_linesize (AT_L2_CACHEGEOMETRY); > > + default: > > + return linux_sysconf (name); > > + } > > +} > > + > > +/* Now the generic Linux version. */ > > +#undef __sysconf > > +#define __sysconf static linux_sysconf > > +#include <sysdeps/unix/sysv/linux/sysconf.c> > > Reviewed-by: Palmer Dabbelt <palmerdabbelt@google.com> > Acked-by: Palmer Dabbelt <palmerdabbelt@google.com> Thanks for the review. Can someone help to pick this patch if it is good enough, thanks.
diff --git a/sysdeps/unix/sysv/linux/riscv/sysconf.c b/sysdeps/unix/sysv/linux/riscv/sysconf.c new file mode 100644 index 0000000000..e73095528c --- /dev/null +++ b/sysdeps/unix/sysv/linux/riscv/sysconf.c @@ -0,0 +1,74 @@ +/* Copyright (C) 2020 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library. If not, see + <https://www.gnu.org/licenses/>. */ + +#include <errno.h> +#include <unistd.h> +#include <sys/auxv.h> + +static long int linux_sysconf (int name); + +static inline long int +sysconf_get_cache_associativity (unsigned long type) +{ + return (__getauxval (type) & 0xffff0000) >> 16; +} + +static inline long int +sysconf_get_cache_linesize (unsigned long type) +{ + return __getauxval (type) & 0xffff; +} + +static inline long int +sysconf_get_cache_size (unsigned long type) +{ + return __getauxval (type); +} + +/* Get the value of the system variable NAME. */ +long int +__sysconf (int name) +{ + switch (name) + { + case _SC_LEVEL1_ICACHE_SIZE: + return sysconf_get_cache_size (AT_L1I_CACHESIZE); + case _SC_LEVEL1_ICACHE_ASSOC: + return sysconf_get_cache_associativity (AT_L1I_CACHEGEOMETRY); + case _SC_LEVEL1_ICACHE_LINESIZE: + return sysconf_get_cache_linesize (AT_L1I_CACHEGEOMETRY); + case _SC_LEVEL1_DCACHE_SIZE: + return sysconf_get_cache_size (AT_L1D_CACHESIZE); + case _SC_LEVEL1_DCACHE_ASSOC: + return sysconf_get_cache_associativity (AT_L1D_CACHEGEOMETRY); + case _SC_LEVEL1_DCACHE_LINESIZE: + return sysconf_get_cache_linesize (AT_L1D_CACHEGEOMETRY); + case _SC_LEVEL2_CACHE_SIZE: + return sysconf_get_cache_size (AT_L2_CACHESIZE); + case _SC_LEVEL2_CACHE_ASSOC: + return sysconf_get_cache_associativity (AT_L2_CACHEGEOMETRY); + case _SC_LEVEL2_CACHE_LINESIZE: + return sysconf_get_cache_linesize (AT_L2_CACHEGEOMETRY); + default: + return linux_sysconf (name); + } +} + +/* Now the generic Linux version. */ +#undef __sysconf +#define __sysconf static linux_sysconf +#include <sysdeps/unix/sysv/linux/sysconf.c>