[PATCH 1/1] riscv: Get cache information through sysconf

[PATCH 1/1] riscv: Get cache information through sysconf

[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.
---
 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>
-- 
2.28.0

Re: [PATCH 1/1] riscv: Get cache information through sysconf

[Florian Weimer]

* 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.

Re: [PATCH 1/1] riscv: Get cache information through sysconf

[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?

Re: [PATCH 1/1] riscv: Get cache information through sysconf

[Florian Weimer]

* 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.

Re: [PATCH 1/1] riscv: Get cache information through sysconf

[Zong Li]

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;
 }

Re: [PATCH 1/1] riscv: Get cache information through sysconf

[Andrew Waterman]

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;
>  }

Re: [PATCH 1/1] riscv: Get cache information through sysconf

[Andreas Schwab]

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.

-- 
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."

Re: [PATCH 1/1] riscv: Get cache information through sysconf

[Andrew Waterman]

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."

Re: [PATCH 1/1] riscv: Get cache information through sysconf

[Zong Li]

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;
> >  }

Re: [PATCH 1/1] riscv: Get cache information through sysconf

[Zong Li]

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."

Re: [PATCH 1/1] riscv: Get cache information through sysconf

[Florian Weimer]

* 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
-- 
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

Re: [PATCH 1/1] riscv: Get cache information through sysconf

[Zong Li]

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;
 }

Re: [PATCH 1/1] riscv: Get cache information through sysconf

[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.

> 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
>

Re: [PATCH 1/1] riscv: Get cache information through sysconf

[Florian Weimer]

* 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
-- 
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

Re: [PATCH 1/1] riscv: Get cache information through sysconf

[Zong Li]

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
>

Re: [PATCH 1/1] riscv: Get cache information through sysconf

[Florian Weimer]

* 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
-- 
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

Re: [PATCH 1/1] riscv: Get cache information through sysconf

[Andreas Schwab]

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.

-- 
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."

Re: [PATCH 1/1] riscv: Get cache information through sysconf

[Andreas Schwab]

On Okt 29 2020, Florian Weimer via Libc-alpha wrote:

> getauxval2_einval should probably be declared inline.

For static functions inline is implied.

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."

Re: [PATCH 1/1] riscv: Get cache information through sysconf

[Zong Li]

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
>

Re: [PATCH 1/1] riscv: Get cache information through sysconf

[Zong Li]

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."

Re: [PATCH 1/1] riscv: Get cache information through sysconf

[Zong Li]

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."

Re: [PATCH 1/1] riscv: Get cache information through sysconf

[Palmer Dabbelt]

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>

Re: [PATCH 1/1] riscv: Get cache information through sysconf

[Zong Li]

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.