This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] S390: Get cache information via sysconf.
- From: Andreas Krebbel <krebbel at linux dot vnet dot ibm dot com>
- To: Stefan Liebler <stli at linux dot vnet dot ibm dot com>, libc-alpha at sourceware dot org
- Date: Fri, 24 Apr 2015 10:07:38 +0200
- Subject: Re: [PATCH] S390: Get cache information via sysconf.
- Authentication-results: sourceware.org; auth=none
- References: <mgr1vn$864$1 at ger dot gmane dot org>
On 04/17/2015 03:31 PM, Stefan Liebler wrote:
> Hi,
>
> this patch adds support to query cache information on s390
> via sysconf() function - e.g. with _SC_LEVEL1_ICACHE_SIZE.
> The attributes size, linesize and assoc can be queried
> for cache level 1 - 4 via "extract cpu attribute" instruction,
> which was first available with z10.
>
> Tested on s390/s390x.
>
> Ok to commit?
>
> Bye
> Stefan
>
> ---
> 2015-04-17 Stefan Liebler <stli@linux.vnet.ibm.com>
>
> * sysdeps/unix/sysv/linux/s390/sysconf.c: New File.
>
Thanks! Just some minor nits.
+static long
+__getCacheInfo (int level, int attr, int type)
Urgs camel case ;) Why to start it with __?
+ /* check arguments. */
+ if ((level < 1 || level > CACHE_LEVEL_MAX)
+ || (attr < CACHE_ATTR_LINESIZE || attr > CACHE_ATTR_ASSOC)
+ || (type < CACHE_TYPE_DATA || type > CACHE_TYPE_INSTRUCTION))
+ return 0L;
+
+ /* check if ecag-instruction is available. */
+ /* ecag - extract CPU attribute (only in zarch; arch >= z10; in as 2.24) */
+ if (((GLRO (dl_hwcap) & HWCAP_S390_STFLE) == 0)
+#if !defined __s390x__
+ || ((GLRO (dl_hwcap) & HWCAP_S390_ZARCH) == 0)
+ || ((GLRO (dl_hwcap) & HWCAP_S390_HIGH_GPRS) == 0)
+#endif
Could you please remove the superfluous brackets.
+ /* store facility list and check for z10.
+ (see ifunc-resolver for details) */
...
+ /* check cache topology, if cache is available at this level. */
...
+ /* get cache information for level, attribute and type. */
...
Comments usually start with a capital letter.
Perhaps this qualifies for an entry in the NEWS file?
Bye,
-Andreas-