Re: [v3] Fix missing L2 cache size in /sys/devices/system/cpu
Michael Ellerman m...@ellerman.id.au wrote:
On Thu, 2015-26-02 at 00:04:47 UTC, Dave Olson wrote:
@@ -324,14 +335,33 @@ static bool cache_node_is_unified(const struct
device_node *np)
return of_get_property(np, cache-unified, NULL);
}
+/*
+ * Handle unified caches that have two different types of tags. Most
embedded
+ * use cache-size, etc. for the unified cache size, but open firmware
systems
+ * use d-cache-size, etc. Since they all appear to be consistent, check
on
+ * initialization for which type we are, and use the appropriate structure.
+ */
static struct cache *cache_do_one_devnode_unified(struct device_node *node,
int level)
{
struct cache *cache;
+ int ucache;
pr_debug(creating L%d ucache for %s\n, level, node-full_name);
cache = new_cache(CACHE_TYPE_UNIFIED, level, node);
^^
+ if (of_get_property(node,
+ cache_type_info[CACHE_TYPE_UNIFIED_D].size_prop, NULL)) {
+ ucache = CACHE_TYPE_UNIFIED_D;
+ } else {
+ ucache = CACHE_TYPE_UNIFIED; /* assume embedded */
+ if (of_get_property(node,
+ cache_type_info[CACHE_TYPE_UNIFIED].size_prop, NULL) ==
+ NULL)
+ printk(KERN_WARNING Unified cache property missing\n);
+ }
+
+ cache = new_cache(ucache, level, node);
^^
return cache;
}
That looks fishy. You create a cache, and then throw it away and create
another
one and return that. I don't think that's what you intended, is it?
It looks like I missed something when I regenerated the patch, yes.
My version of cacheinfo.c doesn't have the first new_cache() call.
It would also be cleaner I think if you created another helper, eg.
cache_is_unified_d() to do the property lookup.
OK.
And also I don't think you need to do the second property lookup, especially
if
all you're going to do is print a warning.
I wanted to make sure that if a similar issue arose in the future, that
a message was printed so it got caught and fixed. I don't see a way
to do that without the 2nd lookup.
Let me know what you think, and I'll generate another patch.
Dave Olson
ol...@cumulusnetworks.com
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCHv2 1/1] powerpc: fix missing L2 cache size, etc in /sys/devices/system/cpu
From: Dave Olson ol...@cumulusnetworks.com
Fix missing L2 cache in /sys/devices/system/cpu/cpu0/cache/index2/size
This appears to have been introduced in 2.6.29 by
93197a36a9c16a85fb24cf5a8639f7bf9af838a3.
This caused lscpu to error out on e500v2 devices, and probably others
error: cannot open /sys/devices/system/cpu/cpu0/cache/index2/size: No such
file or directory
Some embedded powerpc sysystems use cache-size in DTS for the unified L2 cache
size, not d-cache-size, so we need to allow for both DTS names. Added a
new CACHE_TYPE_UNIFIED_D cache_type_info structure to handle this.
Patch is againt URL:
git://git.kernel.org/pub/scm/linux/kernel/git/benh/powerpc.git
master branch (which seems quite old, but is listed as the right branch in the
current Linus
MAINTAINERS file; next is also old, test is newer, but I wasn't sure I
should use that).
I can rebase, if desired.
Signed-off-by: Dave Olson ol...@cumulusnetworks.com
---
diff --git a/arch/powerpc/kernel/cacheinfo.c b/arch/powerpc/kernel/cacheinfo.c
index a3c684b..c3971bc 100644
--- a/arch/powerpc/kernel/cacheinfo.c
+++ b/arch/powerpc/kernel/cacheinfo.c
@@ -62,12 +62,22 @@ struct cache_type_info {
};
/* These are used to index the cache_type_info array. */
-#define CACHE_TYPE_UNIFIED 0
-#define CACHE_TYPE_INSTRUCTION 1
-#define CACHE_TYPE_DATA2
+#define CACHE_TYPE_UNIFIED 0 /* cache-size, cache-block-size, etc. */
+#define CACHE_TYPE_UNIFIED_D 1 /* d-cache-size, d-cache-block-size, etc */
+#define CACHE_TYPE_INSTRUCTION 2
+#define CACHE_TYPE_DATA3
static const struct cache_type_info cache_type_info[] = {
{
+ /* Embedded systems that use cache-size, cache-block-size,
+* etc. for the Unified (typically L2) cache. */
+ .name= Unified,
+ .size_prop = cache-size,
+ .line_size_props = { cache-line-size,
+cache-block-size, },
+ .nr_sets_prop= cache-sets,
+ },
+ {
/* PowerPC Processor binding says the [di]-cache-*
* must be equal on unified caches, so just use
* d-cache properties. */
@@ -293,7 +303,8 @@ static struct cache *cache_find_first_sibling(struct cache
*cache)
{
struct cache *iter;
- if (cache-type == CACHE_TYPE_UNIFIED)
+ if (cache-type == CACHE_TYPE_UNIFIED ||
+ cache-type == CACHE_TYPE_UNIFIED_D)
return cache;
list_for_each_entry(iter, cache_list, list)
@@ -324,13 +335,31 @@ static bool cache_node_is_unified(const struct
device_node *np)
return of_get_property(np, cache-unified, NULL);
}
+/*
+ * Handle unified caches that have two different types of tags. Most embedded
+ * use cache-size, etc. for the unified cache size, but open firmware systems
+ * use d-cache-size, etc. Since they all appear to be consistent, check on
+ * initialization for which type we are, and use the appropriate structure.
+ */
static struct cache *__cpuinit cache_do_one_devnode_unified(struct device_node
*node, int level)
{
struct cache *cache;
+ int ucache;
pr_debug(creating L%d ucache for %s\n, level, node-full_name);
- cache = new_cache(CACHE_TYPE_UNIFIED, level, node);
+ if (of_get_property(node,
+ cache_type_info[CACHE_TYPE_UNIFIED_D].size_prop, NULL)) {
+ ucache = CACHE_TYPE_UNIFIED_D;
+ } else {
+ ucache = CACHE_TYPE_UNIFIED; /* assume embedded */
+ if (of_get_property(node,
+ cache_type_info[CACHE_TYPE_UNIFIED].size_prop, NULL) ==
+ NULL)
+ printk(KERN_WARNING Unified cache property missing\n);
+ }
+
+ cache = new_cache(ucache, level, node);
return cache;
}
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 1/1] powerpc: fix missing L2 cache size in /sys/devices/system/cpu
Benjamin Herrenschmidt b...@kernel.crashing.org wrote: On Tue, 2015-02-10 at 00:00 -0800, Dave Olson wrote: OK, now that I understand that's the case, I'll have to go back and re-do the patch to handle both cache-size and d-cache-size for the L2 cache (using whichever is present). I notice that you also didn't modify all the other properties, I would assume you need to also updates in that area ? Maybe you should duplicate the whole structure and have the code look for both. Since we have line_size_props, I can bump that from 2 to 4 entries, and add cache_line_size and cache_block_size, instead of an explict check. I could change size_prop, and nr_sets_prop to be a structure like line_size_props, if you think that's cleaner than the explict check for cache-size, and cache-sets in the functions. These 3 seem to be the only ones at issue, and I should have checked futher to realize that sets and line size were missing. What's the preference for the other 2 missing items? I don't have any power Macs to use for testing, would one of you be willing and able to verify the patch on a power Mac? Dave Olson ol...@cumulusnetworks.com ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 1/1] powerpc: fix missing L2 cache size in /sys/devices/system/cpu
Benjamin Herrenschmidt b...@kernel.crashing.org wrote:
On Mon, 2015-02-09 at 15:43 -0800, Dave Olson wrote:
Michael Ellerman m...@ellerman.id.au wrote:
On Mon, 2015-02-09 at 14:14 -0800, Dave Olson wrote:
From: Dave Olson ol...@cumulusnetworks.com
Fix missing L2 cache size in
/sys/devices/system/cpu/cpu0/cache/index2/size
This bug appears to be introduced in 2.6.29 by
93197a36a9c16a85fb24cf5a8639f7bf9af838a3.
The missing entry caused lscpu to error out on e500v2 devices, and
probably others
error: cannot open /sys/devices/system/cpu/cpu0/cache/index2/size: No
such file or directory
The DTS files we see use cache-size for the unified L2 cache size, not
d-cache-size
Can you convince me that this is not going to break other machines that
have
d-cache-size but not cache-size?
I'm unable to find any dts file that uses d-cache-size for the L2
unified cache. All in the powerpc tree in arch/powerpc/boot/dts/*
are using cache-size in the L2 description for the cache size.
As best as I can tell from looking around, this is universal.
It may be universal for embedded machines using DTS in the kernel tree
but it's definitely not true of any Mac or server machine (from which
there is no DTS in the kernel as we get the DT from the firmware).
OK, now that I understand that's the case, I'll have to go back and
re-do the patch to handle both cache-size and d-cache-size for the
L2 cache (using whichever is present).
I don't have any power Macs to use for testing, would one of you be
willing and able to verify the patch on a power Mac?
The patch below fixes my problem, and I don't think it will break
platforms like the PowerPC Mac that use d-cache-size
=
diff --git a/arch/powerpc/kernel/cacheinfo.c b/arch/powerpc/kernel/cacheinfo.c
index a3c684b..0d1f879 100644
--- a/arch/powerpc/kernel/cacheinfo.c
+++ b/arch/powerpc/kernel/cacheinfo.c
@@ -200,6 +200,10 @@ static int cache_size(const struct cache *cache, unsigned
int *ret)
propname = cache_type_info[cache-type].size_prop;
cache_size = of_get_property(cache-ofnode, propname, NULL);
+ if (!cache_size cache-type == CACHE_TYPE_UNIFIED) {
+/* most embedded systems with L2 use cache-size, allow that also */
+cache_size = of_get_property(cache-ofnode, cache-size, NULL);
+}
if (!cache_size)
return -ENODEV;
=
Thanks,
Dave Olson
ol...@cumulusnetworks.com
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 1/1] powerpc: fix missing L2 cache size in /sys/devices/system/cpu
From: Dave Olson ol...@cumulusnetworks.com
Fix missing L2 cache size in /sys/devices/system/cpu/cpu0/cache/index2/size
This bug appears to be introduced in 2.6.29 by
93197a36a9c16a85fb24cf5a8639f7bf9af838a3.
The missing entry caused lscpu to error out on e500v2 devices, and probably
others
error: cannot open /sys/devices/system/cpu/cpu0/cache/index2/size: No such
file or directory
The DTS files we see use cache-size for the unified L2 cache size, not
d-cache-size
Signed-off-by: Dave Olson ol...@cumulusnetworks.com
---
diff --git a/arch/powerpc/kernel/cacheinfo.c b/arch/powerpc/kernel/cacheinfo.c
index 40198d5..9ca1e9a 100644
--- a/arch/powerpc/kernel/cacheinfo.c
+++ b/arch/powerpc/kernel/cacheinfo.c
@@ -72,7 +72,7 @@ static const struct cache_type_info cache_type_info[] = {
* must be equal on unified caches, so just use
* d-cache properties. */
.name= Unified,
- .size_prop = d-cache-size,
+ .size_prop = cache-size,
.line_size_props = { d-cache-line-size,
d-cache-block-size, },
.nr_sets_prop= d-cache-sets,
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 1/1] powerpc: fix missing L2 cache size in /sys/devices/system/cpu
Michael Ellerman m...@ellerman.id.au wrote: On Mon, 2015-02-09 at 14:14 -0800, Dave Olson wrote: From: Dave Olson ol...@cumulusnetworks.com Fix missing L2 cache size in /sys/devices/system/cpu/cpu0/cache/index2/size This bug appears to be introduced in 2.6.29 by 93197a36a9c16a85fb24cf5a8639f7bf9af838a3. The missing entry caused lscpu to error out on e500v2 devices, and probably others error: cannot open /sys/devices/system/cpu/cpu0/cache/index2/size: No such file or directory The DTS files we see use cache-size for the unified L2 cache size, not d-cache-size Can you convince me that this is not going to break other machines that have d-cache-size but not cache-size? I'm unable to find any dts file that uses d-cache-size for the L2 unified cache. All in the powerpc tree in arch/powerpc/boot/dts/* are using cache-size in the L2 description for the cache size. As best as I can tell from looking around, this is universal. Dave Olson ol...@cumulusnetworks.com ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
