* [PATCH RFC/RFT v3 0/9] drivers: cacheinfo support
@ 2014-02-19 16:06 Sudeep Holla
2014-02-19 16:06 ` [PATCH RFC/RFT v3 6/9] powerpc: move cacheinfo sysfs to generic cacheinfo infrastructure Sudeep Holla
0 siblings, 1 reply; 7+ messages in thread
From: Sudeep Holla @ 2014-02-19 16:06 UTC (permalink / raw)
To: linux-kernel
Cc: linux-s390, linux-ia64, Greg Kroah-Hartman, x86, sudeep.holla,
linux390, linuxppc-dev, linux-arm-kernel
From: Sudeep Holla <sudeep.holla@arm.com>
Hi,
This series adds a generic cacheinfo support similar to topology. The
implementation is based on x86 cacheinfo support. Currently x86, powerpc,
ia64 and s390 have their own implementations. While adding similar support
to ARM and ARM64, here is the attempt to make it generic quite similar to
topology info support. It also adds the missing ABI documentation for
the cacheinfo sysfs which is already being used.
It moves all the existing different implementations on x86, ia64, powerpc
and s390 to use the generic cacheinfo infrastructure introduced here.
These changes on non-ARM platforms are only compile tested and hence
the request for testing too.
This series also adds support for ARM and ARM64 architectures based on
the generic support.
Changes v2[2]->v3:
- Added new class "cpu" to group all cpu devices
- Converted all "raw" kobjects used in cacheinfo to device_attr
by creating cache index devices
- Added back s390 show_cacheinfo for /proc/cpuinfo
- Added disable_sysfs to cache_info for preventing a cache node
to be exposed through sysfs if required(used on s390)
Changes v1[1]->v2[2]:
- Extended the generic cacheinfo support to accomodate all
the existing implementations
- Moved all the existing implementations to use this new
generic infrastructure
- Added missing ABI documentation as suggested by Greg KH
- Added support for unimplemented CTR on pre-ARMv6 implementations
as suggested by Russell. However the ctr_info_list is not yet
populated
- not yet changed to device_attr as suggested by Greg KH,
registering cache as device won't eliminate the need of kobject
unless each index of cache is registered as a device which don't
seem to be good idea, but now it's unified it can be done easily
in one place if needed
[1] https://lkml.org/lkml/2014/1/8/523
[2] https://lkml.org/lkml/2014/2/7/654
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-ia64@vger.kernel.org
Cc: linux390@de.ibm.com
Cc: linux-s390@vger.kernel.org
Cc: x86@kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-arm-kernel@lists.infradead.org
---
Sudeep Holla (9):
drivers: base: add new class "cpu" to group cpu devices
drivers: base: support cpu cache information interface to userspace
via sysfs
ia64: move cacheinfo sysfs to generic cacheinfo infrastructure
s390: move cacheinfo sysfs to generic cacheinfo infrastructure
x86: move cacheinfo sysfs to generic cacheinfo infrastructure
powerpc: move cacheinfo sysfs to generic cacheinfo infrastructure
ARM64: kernel: add support for cpu cache information
ARM: kernel: add support for cpu cache information
ARM: kernel: add outer cache support for cacheinfo implementation
Documentation/ABI/testing/sysfs-devices-system-cpu | 40 +
arch/arm/include/asm/outercache.h | 13 +
arch/arm/kernel/Makefile | 1 +
arch/arm/kernel/cacheinfo.c | 248 ++++++
arch/arm/mm/Kconfig | 13 +
arch/arm/mm/cache-l2x0.c | 14 +
arch/arm/mm/cache-tauros2.c | 35 +
arch/arm/mm/cache-xsc3l2.c | 15 +
arch/arm64/kernel/Makefile | 2 +-
arch/arm64/kernel/cacheinfo.c | 134 ++++
arch/ia64/kernel/topology.c | 399 ++--------
arch/powerpc/kernel/cacheinfo.c | 831 +++------------------
arch/powerpc/kernel/cacheinfo.h | 8 -
arch/powerpc/kernel/sysfs.c | 4 -
arch/s390/kernel/cache.c | 388 +++-------
arch/x86/kernel/cpu/intel_cacheinfo.c | 647 ++++------------
drivers/base/Makefile | 2 +-
drivers/base/cacheinfo.c | 485 ++++++++++++
drivers/base/core.c | 35 +-
drivers/base/cpu.c | 7 +
include/linux/cacheinfo.h | 55 ++
include/linux/cpu.h | 2 +
22 files changed, 1523 insertions(+), 1855 deletions(-)
create mode 100644 arch/arm/kernel/cacheinfo.c
create mode 100644 arch/arm64/kernel/cacheinfo.c
delete mode 100644 arch/powerpc/kernel/cacheinfo.h
create mode 100644 drivers/base/cacheinfo.c
create mode 100644 include/linux/cacheinfo.h
--
1.8.3.2
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH RFC/RFT v3 6/9] powerpc: move cacheinfo sysfs to generic cacheinfo infrastructure
2014-02-19 16:06 [PATCH RFC/RFT v3 0/9] drivers: cacheinfo support Sudeep Holla
@ 2014-02-19 16:06 ` Sudeep Holla
2014-03-07 4:06 ` Anshuman Khandual
0 siblings, 1 reply; 7+ messages in thread
From: Sudeep Holla @ 2014-02-19 16:06 UTC (permalink / raw)
To: linux-kernel; +Cc: Paul Mackerras, linuxppc-dev, sudeep.holla
From: Sudeep Holla <sudeep.holla@arm.com>
This patch removes the redundant sysfs cacheinfo code by making use of
the newly introduced generic cacheinfo infrastructure.
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: linuxppc-dev@lists.ozlabs.org
---
arch/powerpc/kernel/cacheinfo.c | 831 ++++++----------------------------------
arch/powerpc/kernel/cacheinfo.h | 8 -
arch/powerpc/kernel/sysfs.c | 4 -
3 files changed, 109 insertions(+), 734 deletions(-)
delete mode 100644 arch/powerpc/kernel/cacheinfo.h
diff --git a/arch/powerpc/kernel/cacheinfo.c b/arch/powerpc/kernel/cacheinfo.c
index 2912b87..05b7580 100644
--- a/arch/powerpc/kernel/cacheinfo.c
+++ b/arch/powerpc/kernel/cacheinfo.c
@@ -10,38 +10,10 @@
* 2 as published by the Free Software Foundation.
*/
+#include <linux/cacheinfo.h>
#include <linux/cpu.h>
-#include <linux/cpumask.h>
#include <linux/kernel.h>
-#include <linux/kobject.h>
-#include <linux/list.h>
-#include <linux/notifier.h>
#include <linux/of.h>
-#include <linux/percpu.h>
-#include <linux/slab.h>
-#include <asm/prom.h>
-
-#include "cacheinfo.h"
-
-/* per-cpu object for tracking:
- * - a "cache" kobject for the top-level directory
- * - a list of "index" objects representing the cpu's local cache hierarchy
- */
-struct cache_dir {
- struct kobject *kobj; /* bare (not embedded) kobject for cache
- * directory */
- struct cache_index_dir *index; /* list of index objects */
-};
-
-/* "index" object: each cpu's cache directory has an index
- * subdirectory corresponding to a cache object associated with the
- * cpu. This object's lifetime is managed via the embedded kobject.
- */
-struct cache_index_dir {
- struct kobject kobj;
- struct cache_index_dir *next; /* next index in parent directory */
- struct cache *cache;
-};
/* Template for determining which OF properties to query for a given
* cache type */
@@ -60,11 +32,6 @@ struct cache_type_info {
const char *nr_sets_prop;
};
-/* These are used to index the cache_type_info array. */
-#define CACHE_TYPE_UNIFIED 0
-#define CACHE_TYPE_INSTRUCTION 1
-#define CACHE_TYPE_DATA 2
-
static const struct cache_type_info cache_type_info[] = {
{
/* PowerPC Processor binding says the [di]-cache-*
@@ -77,246 +44,115 @@ static const struct cache_type_info cache_type_info[] = {
.nr_sets_prop = "d-cache-sets",
},
{
- .name = "Instruction",
- .size_prop = "i-cache-size",
- .line_size_props = { "i-cache-line-size",
- "i-cache-block-size", },
- .nr_sets_prop = "i-cache-sets",
- },
- {
.name = "Data",
.size_prop = "d-cache-size",
.line_size_props = { "d-cache-line-size",
"d-cache-block-size", },
.nr_sets_prop = "d-cache-sets",
},
+ {
+ .name = "Instruction",
+ .size_prop = "i-cache-size",
+ .line_size_props = { "i-cache-line-size",
+ "i-cache-block-size", },
+ .nr_sets_prop = "i-cache-sets",
+ },
};
-/* Cache object: each instance of this corresponds to a distinct cache
- * in the system. There are separate objects for Harvard caches: one
- * each for instruction and data, and each refers to the same OF node.
- * The refcount of the OF node is elevated for the lifetime of the
- * cache object. A cache object is released when its shared_cpu_map
- * is cleared (see cache_cpu_clear).
- *
- * A cache object is on two lists: an unsorted global list
- * (cache_list) of cache objects; and a singly-linked list
- * representing the local cache hierarchy, which is ordered by level
- * (e.g. L1d -> L1i -> L2 -> L3).
- */
-struct cache {
- struct device_node *ofnode; /* OF node for this cache, may be cpu */
- struct cpumask shared_cpu_map; /* online CPUs using this cache */
- int type; /* split cache disambiguation */
- int level; /* level not explicit in device tree */
- struct list_head list; /* global list of cache objects */
- struct cache *next_local; /* next cache of >= level */
-};
-
-static DEFINE_PER_CPU(struct cache_dir *, cache_dir_pcpu);
-
-/* traversal/modification of this list occurs only at cpu hotplug time;
- * access is serialized by cpu hotplug locking
- */
-static LIST_HEAD(cache_list);
-
-static struct cache_index_dir *kobj_to_cache_index_dir(struct kobject *k)
-{
- return container_of(k, struct cache_index_dir, kobj);
-}
-
-static const char *cache_type_string(const struct cache *cache)
+static inline int get_cacheinfo_idx(enum cache_type type)
{
- return cache_type_info[cache->type].name;
-}
-
-static void cache_init(struct cache *cache, int type, int level,
- struct device_node *ofnode)
-{
- cache->type = type;
- cache->level = level;
- cache->ofnode = of_node_get(ofnode);
- INIT_LIST_HEAD(&cache->list);
- list_add(&cache->list, &cache_list);
-}
-
-static struct cache *new_cache(int type, int level, struct device_node *ofnode)
-{
- struct cache *cache;
-
- cache = kzalloc(sizeof(*cache), GFP_KERNEL);
- if (cache)
- cache_init(cache, type, level, ofnode);
-
- return cache;
-}
-
-static void release_cache_debugcheck(struct cache *cache)
-{
- struct cache *iter;
-
- list_for_each_entry(iter, &cache_list, list)
- WARN_ONCE(iter->next_local == cache,
- "cache for %s(%s) refers to cache for %s(%s)\n",
- iter->ofnode->full_name,
- cache_type_string(iter),
- cache->ofnode->full_name,
- cache_type_string(cache));
-}
-
-static void release_cache(struct cache *cache)
-{
- if (!cache)
- return;
-
- pr_debug("freeing L%d %s cache for %s\n", cache->level,
- cache_type_string(cache), cache->ofnode->full_name);
-
- release_cache_debugcheck(cache);
- list_del(&cache->list);
- of_node_put(cache->ofnode);
- kfree(cache);
-}
-
-static void cache_cpu_set(struct cache *cache, int cpu)
-{
- struct cache *next = cache;
-
- while (next) {
- WARN_ONCE(cpumask_test_cpu(cpu, &next->shared_cpu_map),
- "CPU %i already accounted in %s(%s)\n",
- cpu, next->ofnode->full_name,
- cache_type_string(next));
- cpumask_set_cpu(cpu, &next->shared_cpu_map);
- next = next->next_local;
- }
+ if (type == CACHE_TYPE_UNIFIED)
+ return 0;
+ else
+ return type;
}
-static int cache_size(const struct cache *cache, unsigned int *ret)
+static int cache_size(struct cache_info *this_leaf)
{
const char *propname;
const __be32 *cache_size;
+ int ct_idx;
- propname = cache_type_info[cache->type].size_prop;
-
- cache_size = of_get_property(cache->ofnode, propname, NULL);
- if (!cache_size)
- return -ENODEV;
-
- *ret = of_read_number(cache_size, 1);
- return 0;
-}
-
-static int cache_size_kb(const struct cache *cache, unsigned int *ret)
-{
- unsigned int size;
+ ct_idx = get_cacheinfo_idx(this_leaf->type);
+ propname = cache_type_info[ct_idx].size_prop;
- if (cache_size(cache, &size))
+ cache_size = of_get_property(this_leaf->of_node, propname, NULL);
+ if (!cache_size) {
+ this_leaf->size = 0;
return -ENODEV;
-
- *ret = size / 1024;
- return 0;
+ } else {
+ this_leaf->size = of_read_number(cache_size, 1);
+ return 0;
+ }
}
/* not cache_line_size() because that's a macro in include/linux/cache.h */
-static int cache_get_line_size(const struct cache *cache, unsigned int *ret)
+static int cache_get_line_size(struct cache_info *this_leaf)
{
const __be32 *line_size;
- int i, lim;
+ int i, lim, ct_idx;
- lim = ARRAY_SIZE(cache_type_info[cache->type].line_size_props);
+ ct_idx = get_cacheinfo_idx(this_leaf->type);
+ lim = ARRAY_SIZE(cache_type_info[ct_idx].line_size_props);
for (i = 0; i < lim; i++) {
const char *propname;
- propname = cache_type_info[cache->type].line_size_props[i];
- line_size = of_get_property(cache->ofnode, propname, NULL);
+ propname = cache_type_info[ct_idx].line_size_props[i];
+ line_size = of_get_property(this_leaf->of_node, propname, NULL);
if (line_size)
break;
}
- if (!line_size)
+ if (!line_size) {
+ this_leaf->coherency_line_size = 0;
return -ENODEV;
-
- *ret = of_read_number(line_size, 1);
- return 0;
+ } else {
+ this_leaf->coherency_line_size = of_read_number(line_size, 1);
+ return 0;
+ }
}
-static int cache_nr_sets(const struct cache *cache, unsigned int *ret)
+static int cache_nr_sets(struct cache_info *this_leaf)
{
const char *propname;
const __be32 *nr_sets;
+ int ct_idx;
- propname = cache_type_info[cache->type].nr_sets_prop;
+ ct_idx = get_cacheinfo_idx(this_leaf->type);
+ propname = cache_type_info[ct_idx].nr_sets_prop;
- nr_sets = of_get_property(cache->ofnode, propname, NULL);
- if (!nr_sets)
+ nr_sets = of_get_property(this_leaf->of_node, propname, NULL);
+ if (!nr_sets) {
+ this_leaf->number_of_sets = 0;
return -ENODEV;
-
- *ret = of_read_number(nr_sets, 1);
- return 0;
+ } else {
+ this_leaf->number_of_sets = of_read_number(nr_sets, 1);
+ return 0;
+ }
}
-static int cache_associativity(const struct cache *cache, unsigned int *ret)
+static int cache_associativity(struct cache_info *this_leaf)
{
- unsigned int line_size;
- unsigned int nr_sets;
- unsigned int size;
-
- if (cache_nr_sets(cache, &nr_sets))
- goto err;
+ unsigned int line_size = this_leaf->coherency_line_size;
+ unsigned int nr_sets = this_leaf->number_of_sets;
+ unsigned int size = this_leaf->size;
/* If the cache is fully associative, there is no need to
* check the other properties.
*/
if (nr_sets == 1) {
- *ret = 0;
+ this_leaf->ways_of_associativity = 0;
return 0;
}
- if (cache_get_line_size(cache, &line_size))
- goto err;
- if (cache_size(cache, &size))
- goto err;
-
- if (!(nr_sets > 0 && size > 0 && line_size > 0))
- goto err;
-
- *ret = (size / nr_sets) / line_size;
- return 0;
-err:
- return -ENODEV;
-}
-
-/* helper for dealing with split caches */
-static struct cache *cache_find_first_sibling(struct cache *cache)
-{
- struct cache *iter;
-
- if (cache->type == CACHE_TYPE_UNIFIED)
- return cache;
-
- list_for_each_entry(iter, &cache_list, list)
- if (iter->ofnode == cache->ofnode && iter->next_local == cache)
- return iter;
-
- return cache;
-}
-
-/* return the first cache on a local list matching node */
-static struct cache *cache_lookup_by_node(const struct device_node *node)
-{
- struct cache *cache = NULL;
- struct cache *iter;
-
- list_for_each_entry(iter, &cache_list, list) {
- if (iter->ofnode != node)
- continue;
- cache = cache_find_first_sibling(iter);
- break;
+ if (!(nr_sets > 0 && size > 0 && line_size > 0)) {
+ this_leaf->ways_of_associativity = 0;
+ return -ENODEV;
+ } else {
+ this_leaf->ways_of_associativity = (size / nr_sets) / line_size;
+ return 0;
}
-
- return cache;
}
static bool cache_node_is_unified(const struct device_node *np)
@@ -324,523 +160,74 @@ static bool cache_node_is_unified(const struct device_node *np)
return of_get_property(np, "cache-unified", NULL);
}
-static struct cache *cache_do_one_devnode_unified(struct device_node *node,
- int level)
-{
- struct cache *cache;
-
- pr_debug("creating L%d ucache for %s\n", level, node->full_name);
-
- cache = new_cache(CACHE_TYPE_UNIFIED, level, node);
-
- return cache;
-}
-
-static struct cache *cache_do_one_devnode_split(struct device_node *node,
- int level)
-{
- struct cache *dcache, *icache;
-
- pr_debug("creating L%d dcache and icache for %s\n", level,
- node->full_name);
-
- dcache = new_cache(CACHE_TYPE_DATA, level, node);
- icache = new_cache(CACHE_TYPE_INSTRUCTION, level, node);
-
- if (!dcache || !icache)
- goto err;
-
- dcache->next_local = icache;
-
- return dcache;
-err:
- release_cache(dcache);
- release_cache(icache);
- return NULL;
-}
-
-static struct cache *cache_do_one_devnode(struct device_node *node, int level)
-{
- struct cache *cache;
-
- if (cache_node_is_unified(node))
- cache = cache_do_one_devnode_unified(node, level);
- else
- cache = cache_do_one_devnode_split(node, level);
-
- return cache;
-}
-
-static struct cache *cache_lookup_or_instantiate(struct device_node *node,
- int level)
-{
- struct cache *cache;
-
- cache = cache_lookup_by_node(node);
-
- WARN_ONCE(cache && cache->level != level,
- "cache level mismatch on lookup (got %d, expected %d)\n",
- cache->level, level);
-
- if (!cache)
- cache = cache_do_one_devnode(node, level);
-
- return cache;
-}
-
-static void link_cache_lists(struct cache *smaller, struct cache *bigger)
-{
- while (smaller->next_local) {
- if (smaller->next_local == bigger)
- return; /* already linked */
- smaller = smaller->next_local;
- }
-
- smaller->next_local = bigger;
-}
-
-static void do_subsidiary_caches_debugcheck(struct cache *cache)
-{
- WARN_ON_ONCE(cache->level != 1);
- WARN_ON_ONCE(strcmp(cache->ofnode->type, "cpu"));
-}
-
-static void do_subsidiary_caches(struct cache *cache)
-{
- struct device_node *subcache_node;
- int level = cache->level;
-
- do_subsidiary_caches_debugcheck(cache);
-
- while ((subcache_node = of_find_next_cache_node(cache->ofnode))) {
- struct cache *subcache;
-
- level++;
- subcache = cache_lookup_or_instantiate(subcache_node, level);
- of_node_put(subcache_node);
- if (!subcache)
- break;
-
- link_cache_lists(cache, subcache);
- cache = subcache;
- }
-}
-
-static struct cache *cache_chain_instantiate(unsigned int cpu_id)
-{
- struct device_node *cpu_node;
- struct cache *cpu_cache = NULL;
-
- pr_debug("creating cache object(s) for CPU %i\n", cpu_id);
-
- cpu_node = of_get_cpu_node(cpu_id, NULL);
- WARN_ONCE(!cpu_node, "no OF node found for CPU %i\n", cpu_id);
- if (!cpu_node)
- goto out;
-
- cpu_cache = cache_lookup_or_instantiate(cpu_node, 1);
- if (!cpu_cache)
- goto out;
-
- do_subsidiary_caches(cpu_cache);
-
- cache_cpu_set(cpu_cache, cpu_id);
-out:
- of_node_put(cpu_node);
-
- return cpu_cache;
-}
-
-static struct cache_dir *cacheinfo_create_cache_dir(unsigned int cpu_id)
-{
- struct cache_dir *cache_dir;
- struct device *dev;
- struct kobject *kobj = NULL;
-
- dev = get_cpu_device(cpu_id);
- WARN_ONCE(!dev, "no dev for CPU %i\n", cpu_id);
- if (!dev)
- goto err;
-
- kobj = kobject_create_and_add("cache", &dev->kobj);
- if (!kobj)
- goto err;
-
- cache_dir = kzalloc(sizeof(*cache_dir), GFP_KERNEL);
- if (!cache_dir)
- goto err;
-
- cache_dir->kobj = kobj;
-
- WARN_ON_ONCE(per_cpu(cache_dir_pcpu, cpu_id) != NULL);
-
- per_cpu(cache_dir_pcpu, cpu_id) = cache_dir;
-
- return cache_dir;
-err:
- kobject_put(kobj);
- return NULL;
-}
-
-static void cache_index_release(struct kobject *kobj)
-{
- struct cache_index_dir *index;
-
- index = kobj_to_cache_index_dir(kobj);
-
- pr_debug("freeing index directory for L%d %s cache\n",
- index->cache->level, cache_type_string(index->cache));
-
- kfree(index);
-}
-
-static ssize_t cache_index_show(struct kobject *k, struct attribute *attr, char *buf)
+static void ci_leaf_init(struct cache_info *this_leaf,
+ enum cache_type type, unsigned int level)
{
- struct kobj_attribute *kobj_attr;
-
- kobj_attr = container_of(attr, struct kobj_attribute, attr);
-
- return kobj_attr->show(k, kobj_attr, buf);
-}
-
-static struct cache *index_kobj_to_cache(struct kobject *k)
-{
- struct cache_index_dir *index;
-
- index = kobj_to_cache_index_dir(k);
-
- return index->cache;
+ this_leaf->level = level;
+ this_leaf->type = type;
+ cache_size(this_leaf);
+ cache_get_line_size(this_leaf);
+ cache_nr_sets(this_leaf);
+ cache_associativity(this_leaf);
}
-static ssize_t size_show(struct kobject *k, struct kobj_attribute *attr, char *buf)
+int init_cache_level(unsigned int cpu)
{
- unsigned int size_kb;
- struct cache *cache;
+ struct device_node *np;
+ struct device *cpu_dev = get_cpu_device(cpu);
+ struct cpu_cacheinfo *this_cpu_ci = get_cpu_cacheinfo(cpu);
+ unsigned int level = 0, leaves = 0;
- cache = index_kobj_to_cache(k);
-
- if (cache_size_kb(cache, &size_kb))
+ if (!cpu_dev) {
+ pr_err("No cpu device for CPU %d\n", cpu);
return -ENODEV;
-
- return sprintf(buf, "%uK\n", size_kb);
-}
-
-static struct kobj_attribute cache_size_attr =
- __ATTR(size, 0444, size_show, NULL);
-
-
-static ssize_t line_size_show(struct kobject *k, struct kobj_attribute *attr, char *buf)
-{
- unsigned int line_size;
- struct cache *cache;
-
- cache = index_kobj_to_cache(k);
-
- if (cache_get_line_size(cache, &line_size))
- return -ENODEV;
-
- return sprintf(buf, "%u\n", line_size);
-}
-
-static struct kobj_attribute cache_line_size_attr =
- __ATTR(coherency_line_size, 0444, line_size_show, NULL);
-
-static ssize_t nr_sets_show(struct kobject *k, struct kobj_attribute *attr, char *buf)
-{
- unsigned int nr_sets;
- struct cache *cache;
-
- cache = index_kobj_to_cache(k);
-
- if (cache_nr_sets(cache, &nr_sets))
- return -ENODEV;
-
- return sprintf(buf, "%u\n", nr_sets);
-}
-
-static struct kobj_attribute cache_nr_sets_attr =
- __ATTR(number_of_sets, 0444, nr_sets_show, NULL);
-
-static ssize_t associativity_show(struct kobject *k, struct kobj_attribute *attr, char *buf)
-{
- unsigned int associativity;
- struct cache *cache;
-
- cache = index_kobj_to_cache(k);
-
- if (cache_associativity(cache, &associativity))
- return -ENODEV;
-
- return sprintf(buf, "%u\n", associativity);
-}
-
-static struct kobj_attribute cache_assoc_attr =
- __ATTR(ways_of_associativity, 0444, associativity_show, NULL);
-
-static ssize_t type_show(struct kobject *k, struct kobj_attribute *attr, char *buf)
-{
- struct cache *cache;
-
- cache = index_kobj_to_cache(k);
-
- return sprintf(buf, "%s\n", cache_type_string(cache));
-}
-
-static struct kobj_attribute cache_type_attr =
- __ATTR(type, 0444, type_show, NULL);
-
-static ssize_t level_show(struct kobject *k, struct kobj_attribute *attr, char *buf)
-{
- struct cache_index_dir *index;
- struct cache *cache;
-
- index = kobj_to_cache_index_dir(k);
- cache = index->cache;
-
- return sprintf(buf, "%d\n", cache->level);
-}
-
-static struct kobj_attribute cache_level_attr =
- __ATTR(level, 0444, level_show, NULL);
-
-static ssize_t shared_cpu_map_show(struct kobject *k, struct kobj_attribute *attr, char *buf)
-{
- struct cache_index_dir *index;
- struct cache *cache;
- int len;
- int n = 0;
-
- index = kobj_to_cache_index_dir(k);
- cache = index->cache;
- len = PAGE_SIZE - 2;
-
- if (len > 1) {
- n = cpumask_scnprintf(buf, len, &cache->shared_cpu_map);
- buf[n++] = '\n';
- buf[n] = '\0';
}
- return n;
-}
-
-static struct kobj_attribute cache_shared_cpu_map_attr =
- __ATTR(shared_cpu_map, 0444, shared_cpu_map_show, NULL);
-
-/* Attributes which should always be created -- the kobject/sysfs core
- * does this automatically via kobj_type->default_attrs. This is the
- * minimum data required to uniquely identify a cache.
- */
-static struct attribute *cache_index_default_attrs[] = {
- &cache_type_attr.attr,
- &cache_level_attr.attr,
- &cache_shared_cpu_map_attr.attr,
- NULL,
-};
-
-/* Attributes which should be created if the cache device node has the
- * right properties -- see cacheinfo_create_index_opt_attrs
- */
-static struct kobj_attribute *cache_index_opt_attrs[] = {
- &cache_size_attr,
- &cache_line_size_attr,
- &cache_nr_sets_attr,
- &cache_assoc_attr,
-};
-
-static const struct sysfs_ops cache_index_ops = {
- .show = cache_index_show,
-};
-
-static struct kobj_type cache_index_type = {
- .release = cache_index_release,
- .sysfs_ops = &cache_index_ops,
- .default_attrs = cache_index_default_attrs,
-};
-
-static void cacheinfo_create_index_opt_attrs(struct cache_index_dir *dir)
-{
- const char *cache_name;
- const char *cache_type;
- struct cache *cache;
- char *buf;
- int i;
-
- buf = kmalloc(PAGE_SIZE, GFP_KERNEL);
- if (!buf)
- return;
-
- cache = dir->cache;
- cache_name = cache->ofnode->full_name;
- cache_type = cache_type_string(cache);
-
- /* We don't want to create an attribute that can't provide a
- * meaningful value. Check the return value of each optional
- * attribute's ->show method before registering the
- * attribute.
- */
- for (i = 0; i < ARRAY_SIZE(cache_index_opt_attrs); i++) {
- struct kobj_attribute *attr;
- ssize_t rc;
-
- attr = cache_index_opt_attrs[i];
-
- rc = attr->show(&dir->kobj, attr, buf);
- if (rc <= 0) {
- pr_debug("not creating %s attribute for "
- "%s(%s) (rc = %zd)\n",
- attr->attr.name, cache_name,
- cache_type, rc);
- continue;
- }
- if (sysfs_create_file(&dir->kobj, &attr->attr))
- pr_debug("could not create %s attribute for %s(%s)\n",
- attr->attr.name, cache_name, cache_type);
+ np = cpu_dev->of_node;
+ if (!np) {
+ pr_err("Failed to find cpu%d device node\n", cpu);
+ return -ENOENT;
}
- kfree(buf);
-}
-
-static void cacheinfo_create_index_dir(struct cache *cache, int index,
- struct cache_dir *cache_dir)
-{
- struct cache_index_dir *index_dir;
- int rc;
-
- index_dir = kzalloc(sizeof(*index_dir), GFP_KERNEL);
- if (!index_dir)
- goto err;
-
- index_dir->cache = cache;
-
- rc = kobject_init_and_add(&index_dir->kobj, &cache_index_type,
- cache_dir->kobj, "index%d", index);
- if (rc)
- goto err;
-
- index_dir->next = cache_dir->index;
- cache_dir->index = index_dir;
-
- cacheinfo_create_index_opt_attrs(index_dir);
-
- return;
-err:
- kfree(index_dir);
-}
-
-static void cacheinfo_sysfs_populate(unsigned int cpu_id,
- struct cache *cache_list)
-{
- struct cache_dir *cache_dir;
- struct cache *cache;
- int index = 0;
-
- cache_dir = cacheinfo_create_cache_dir(cpu_id);
- if (!cache_dir)
- return;
-
- cache = cache_list;
- while (cache) {
- cacheinfo_create_index_dir(cache, index, cache_dir);
- index++;
- cache = cache->next_local;
+ while (np) {
+ leaves += cache_node_is_unified(np) ? 1 : 2;
+ level++;
+ of_node_put(np);
+ np = of_find_next_cache_node(np);
}
-}
-
-void cacheinfo_cpu_online(unsigned int cpu_id)
-{
- struct cache *cache;
-
- cache = cache_chain_instantiate(cpu_id);
- if (!cache)
- return;
-
- cacheinfo_sysfs_populate(cpu_id, cache);
-}
-
-#ifdef CONFIG_HOTPLUG_CPU /* functions needed for cpu offline */
-
-static struct cache *cache_lookup_by_cpu(unsigned int cpu_id)
-{
- struct device_node *cpu_node;
- struct cache *cache;
-
- cpu_node = of_get_cpu_node(cpu_id, NULL);
- WARN_ONCE(!cpu_node, "no OF node found for CPU %i\n", cpu_id);
- if (!cpu_node)
- return NULL;
-
- cache = cache_lookup_by_node(cpu_node);
- of_node_put(cpu_node);
+ this_cpu_ci->num_levels = level;
+ this_cpu_ci->num_leaves = leaves;
- return cache;
+ return 0;
}
-static void remove_index_dirs(struct cache_dir *cache_dir)
+int populate_cache_leaves(unsigned int cpu)
{
- struct cache_index_dir *index;
-
- index = cache_dir->index;
+ struct cpu_cacheinfo *this_cpu_ci = get_cpu_cacheinfo(cpu);
+ struct cache_info *this_leaf = this_cpu_ci->info_list;
+ struct device *cpu_dev = get_cpu_device(cpu);
+ struct device_node *np;
+ unsigned int level, idx;
- while (index) {
- struct cache_index_dir *next;
-
- next = index->next;
- kobject_put(&index->kobj);
- index = next;
+ np = of_node_get(cpu_dev->of_node);
+ if (!np) {
+ pr_err("Failed to find cpu%d device node\n", cpu);
+ return -ENOENT;
}
-}
-
-static void remove_cache_dir(struct cache_dir *cache_dir)
-{
- remove_index_dirs(cache_dir);
- /* Remove cache dir from sysfs */
- kobject_del(cache_dir->kobj);
-
- kobject_put(cache_dir->kobj);
-
- kfree(cache_dir);
-}
-
-static void cache_cpu_clear(struct cache *cache, int cpu)
-{
- while (cache) {
- struct cache *next = cache->next_local;
-
- WARN_ONCE(!cpumask_test_cpu(cpu, &cache->shared_cpu_map),
- "CPU %i not accounted in %s(%s)\n",
- cpu, cache->ofnode->full_name,
- cache_type_string(cache));
-
- cpumask_clear_cpu(cpu, &cache->shared_cpu_map);
-
- /* Release the cache object if all the cpus using it
- * are offline */
- if (cpumask_empty(&cache->shared_cpu_map))
- release_cache(cache);
-
- cache = next;
+ for (idx = 0, level = 1; level <= this_cpu_ci->num_levels &&
+ idx < this_cpu_ci->num_leaves; idx++, level++) {
+ if (!this_leaf)
+ return -EINVAL;
+
+ this_leaf->of_node = np;
+ if (cache_node_is_unified(np)) {
+ ci_leaf_init(this_leaf++, CACHE_TYPE_UNIFIED, level);
+ } else {
+ ci_leaf_init(this_leaf++, CACHE_TYPE_DATA, level);
+ ci_leaf_init(this_leaf++, CACHE_TYPE_INST, level);
+ }
+ np = of_find_next_cache_node(np);
}
+ return 0;
}
-void cacheinfo_cpu_offline(unsigned int cpu_id)
-{
- struct cache_dir *cache_dir;
- struct cache *cache;
-
- /* Prevent userspace from seeing inconsistent state - remove
- * the sysfs hierarchy first */
- cache_dir = per_cpu(cache_dir_pcpu, cpu_id);
-
- /* careful, sysfs population may have failed */
- if (cache_dir)
- remove_cache_dir(cache_dir);
-
- per_cpu(cache_dir_pcpu, cpu_id) = NULL;
-
- /* clear the CPU's bit in its cache chain, possibly freeing
- * cache objects */
- cache = cache_lookup_by_cpu(cpu_id);
- if (cache)
- cache_cpu_clear(cache, cpu_id);
-}
-#endif /* CONFIG_HOTPLUG_CPU */
diff --git a/arch/powerpc/kernel/cacheinfo.h b/arch/powerpc/kernel/cacheinfo.h
deleted file mode 100644
index a7b74d3..0000000
--- a/arch/powerpc/kernel/cacheinfo.h
+++ /dev/null
@@ -1,8 +0,0 @@
-#ifndef _PPC_CACHEINFO_H
-#define _PPC_CACHEINFO_H
-
-/* These are just hooks for sysfs.c to use. */
-extern void cacheinfo_cpu_online(unsigned int cpu_id);
-extern void cacheinfo_cpu_offline(unsigned int cpu_id);
-
-#endif /* _PPC_CACHEINFO_H */
diff --git a/arch/powerpc/kernel/sysfs.c b/arch/powerpc/kernel/sysfs.c
index 97e1dc9..2bc61d3 100644
--- a/arch/powerpc/kernel/sysfs.c
+++ b/arch/powerpc/kernel/sysfs.c
@@ -19,8 +19,6 @@
#include <asm/pmc.h>
#include <asm/firmware.h>
-#include "cacheinfo.h"
-
#ifdef CONFIG_PPC64
#include <asm/paca.h>
#include <asm/lppaca.h>
@@ -730,7 +728,6 @@ static void register_cpu_online(unsigned int cpu)
device_create_file(s, &dev_attr_altivec_idle_wait_time);
}
#endif
- cacheinfo_cpu_online(cpu);
}
#ifdef CONFIG_HOTPLUG_CPU
@@ -811,7 +808,6 @@ static void unregister_cpu_online(unsigned int cpu)
device_remove_file(s, &dev_attr_altivec_idle_wait_time);
}
#endif
- cacheinfo_cpu_offline(cpu);
}
#ifdef CONFIG_ARCH_CPU_PROBE_RELEASE
--
1.8.3.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH RFC/RFT v3 6/9] powerpc: move cacheinfo sysfs to generic cacheinfo infrastructure
2014-02-19 16:06 ` [PATCH RFC/RFT v3 6/9] powerpc: move cacheinfo sysfs to generic cacheinfo infrastructure Sudeep Holla
@ 2014-03-07 4:06 ` Anshuman Khandual
2014-03-07 6:14 ` Anshuman Khandual
0 siblings, 1 reply; 7+ messages in thread
From: Anshuman Khandual @ 2014-03-07 4:06 UTC (permalink / raw)
To: Sudeep Holla; +Cc: linuxppc-dev, Paul Mackerras, linux-kernel
On 02/19/2014 09:36 PM, Sudeep Holla wrote:
> From: Sudeep Holla <sudeep.holla@arm.com>
>
> This patch removes the redundant sysfs cacheinfo code by making use of
> the newly introduced generic cacheinfo infrastructure.
>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: linuxppc-dev@lists.ozlabs.org
> ---
> arch/powerpc/kernel/cacheinfo.c | 831 ++++++----------------------------------
> arch/powerpc/kernel/cacheinfo.h | 8 -
> arch/powerpc/kernel/sysfs.c | 4 -
> 3 files changed, 109 insertions(+), 734 deletions(-)
> delete mode 100644 arch/powerpc/kernel/cacheinfo.h
>
> diff --git a/arch/powerpc/kernel/cacheinfo.c b/arch/powerpc/kernel/cacheinfo.c
> index 2912b87..05b7580 100644
> --- a/arch/powerpc/kernel/cacheinfo.c
> +++ b/arch/powerpc/kernel/cacheinfo.c
> @@ -10,38 +10,10 @@
> * 2 as published by the Free Software Foundation.
> */
>
> +#include <linux/cacheinfo.h>
> #include <linux/cpu.h>
> -#include <linux/cpumask.h>
> #include <linux/kernel.h>
> -#include <linux/kobject.h>
> -#include <linux/list.h>
> -#include <linux/notifier.h>
> #include <linux/of.h>
> -#include <linux/percpu.h>
> -#include <linux/slab.h>
> -#include <asm/prom.h>
> -
> -#include "cacheinfo.h"
> -
> -/* per-cpu object for tracking:
> - * - a "cache" kobject for the top-level directory
> - * - a list of "index" objects representing the cpu's local cache hierarchy
> - */
> -struct cache_dir {
> - struct kobject *kobj; /* bare (not embedded) kobject for cache
> - * directory */
> - struct cache_index_dir *index; /* list of index objects */
> -};
> -
> -/* "index" object: each cpu's cache directory has an index
> - * subdirectory corresponding to a cache object associated with the
> - * cpu. This object's lifetime is managed via the embedded kobject.
> - */
> -struct cache_index_dir {
> - struct kobject kobj;
> - struct cache_index_dir *next; /* next index in parent directory */
> - struct cache *cache;
> -};
>
> /* Template for determining which OF properties to query for a given
> * cache type */
> @@ -60,11 +32,6 @@ struct cache_type_info {
> const char *nr_sets_prop;
> };
>
> -/* These are used to index the cache_type_info array. */
> -#define CACHE_TYPE_UNIFIED 0
> -#define CACHE_TYPE_INSTRUCTION 1
> -#define CACHE_TYPE_DATA 2
> -
> static const struct cache_type_info cache_type_info[] = {
> {
> /* PowerPC Processor binding says the [di]-cache-*
> @@ -77,246 +44,115 @@ static const struct cache_type_info cache_type_info[] = {
> .nr_sets_prop = "d-cache-sets",
> },
> {
> - .name = "Instruction",
> - .size_prop = "i-cache-size",
> - .line_size_props = { "i-cache-line-size",
> - "i-cache-block-size", },
> - .nr_sets_prop = "i-cache-sets",
> - },
> - {
> .name = "Data",
> .size_prop = "d-cache-size",
> .line_size_props = { "d-cache-line-size",
> "d-cache-block-size", },
> .nr_sets_prop = "d-cache-sets",
> },
> + {
> + .name = "Instruction",
> + .size_prop = "i-cache-size",
> + .line_size_props = { "i-cache-line-size",
> + "i-cache-block-size", },
> + .nr_sets_prop = "i-cache-sets",
> + },
> };
Hey Sudeep,
After applying this patch, the cache_type_info array looks like this.
static const struct cache_type_info cache_type_info[] = {
{
/*
* PowerPC Processor binding says the [di]-cache-*
* must be equal on unified caches, so just use
* d-cache properties.
*/
.name = "Unified",
.size_prop = "d-cache-size",
.line_size_props = { "d-cache-line-size",
"d-cache-block-size", },
.nr_sets_prop = "d-cache-sets",
},
{
.name = "Data",
.size_prop = "d-cache-size",
.line_size_props = { "d-cache-line-size",
"d-cache-block-size", },
.nr_sets_prop = "d-cache-sets",
},
{
.name = "Instruction",
.size_prop = "i-cache-size",
.line_size_props = { "i-cache-line-size",
"i-cache-block-size", },
.nr_sets_prop = "i-cache-sets",
},
};
and this function computes the the array index for any given cache type
define for PowerPC.
static inline int get_cacheinfo_idx(enum cache_type type)
{
if (type == CACHE_TYPE_UNIFIED)
return 0;
else
return type;
}
These types are define in include/linux/cacheinfo.h as
enum cache_type {
CACHE_TYPE_NOCACHE = 0,
CACHE_TYPE_INST = BIT(0), ---> 1
CACHE_TYPE_DATA = BIT(1), ---> 2
CACHE_TYPE_SEPARATE = CACHE_TYPE_INST | CACHE_TYPE_DATA,
CACHE_TYPE_UNIFIED = BIT(2),
};
When it is UNIFIED we return index 0, which is correct. But the index
for instruction and data cache seems to be swapped which wrong. This
will fetch invalid properties for any given cache type.
I have done some initial review and testing for this patch's impact on
PowerPC (ppc64 POWER specifically). I am trying to do some code clean-up
and re-arrangements. Will post out soon. Thanks !
Regards
Anshuman
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH RFC/RFT v3 6/9] powerpc: move cacheinfo sysfs to generic cacheinfo infrastructure
2014-03-07 4:06 ` Anshuman Khandual
@ 2014-03-07 6:14 ` Anshuman Khandual
2014-03-10 11:12 ` Sudeep Holla
0 siblings, 1 reply; 7+ messages in thread
From: Anshuman Khandual @ 2014-03-07 6:14 UTC (permalink / raw)
To: Sudeep Holla; +Cc: linuxppc-dev, Paul Mackerras, linux-kernel
On 03/07/2014 09:36 AM, Anshuman Khandual wrote:
> On 02/19/2014 09:36 PM, Sudeep Holla wrote:
>> From: Sudeep Holla <sudeep.holla@arm.com>
>>
>> This patch removes the redundant sysfs cacheinfo code by making use of
>> the newly introduced generic cacheinfo infrastructure.
>>
>> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
>> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>> Cc: Paul Mackerras <paulus@samba.org>
>> Cc: linuxppc-dev@lists.ozlabs.org
>> ---
>> arch/powerpc/kernel/cacheinfo.c | 831 ++++++----------------------------------
>> arch/powerpc/kernel/cacheinfo.h | 8 -
>> arch/powerpc/kernel/sysfs.c | 4 -
>> 3 files changed, 109 insertions(+), 734 deletions(-)
>> delete mode 100644 arch/powerpc/kernel/cacheinfo.h
>>
>> diff --git a/arch/powerpc/kernel/cacheinfo.c b/arch/powerpc/kernel/cacheinfo.c
>> index 2912b87..05b7580 100644
>> --- a/arch/powerpc/kernel/cacheinfo.c
>> +++ b/arch/powerpc/kernel/cacheinfo.c
>> @@ -10,38 +10,10 @@
>> * 2 as published by the Free Software Foundation.
>> */
>>
>> +#include <linux/cacheinfo.h>
>> #include <linux/cpu.h>
>> -#include <linux/cpumask.h>
>> #include <linux/kernel.h>
>> -#include <linux/kobject.h>
>> -#include <linux/list.h>
>> -#include <linux/notifier.h>
>> #include <linux/of.h>
>> -#include <linux/percpu.h>
>> -#include <linux/slab.h>
>> -#include <asm/prom.h>
>> -
>> -#include "cacheinfo.h"
>> -
>> -/* per-cpu object for tracking:
>> - * - a "cache" kobject for the top-level directory
>> - * - a list of "index" objects representing the cpu's local cache hierarchy
>> - */
>> -struct cache_dir {
>> - struct kobject *kobj; /* bare (not embedded) kobject for cache
>> - * directory */
>> - struct cache_index_dir *index; /* list of index objects */
>> -};
>> -
>> -/* "index" object: each cpu's cache directory has an index
>> - * subdirectory corresponding to a cache object associated with the
>> - * cpu. This object's lifetime is managed via the embedded kobject.
>> - */
>> -struct cache_index_dir {
>> - struct kobject kobj;
>> - struct cache_index_dir *next; /* next index in parent directory */
>> - struct cache *cache;
>> -};
>>
>> /* Template for determining which OF properties to query for a given
>> * cache type */
>> @@ -60,11 +32,6 @@ struct cache_type_info {
>> const char *nr_sets_prop;
>> };
>>
>> -/* These are used to index the cache_type_info array. */
>> -#define CACHE_TYPE_UNIFIED 0
>> -#define CACHE_TYPE_INSTRUCTION 1
>> -#define CACHE_TYPE_DATA 2
>> -
>> static const struct cache_type_info cache_type_info[] = {
>> {
>> /* PowerPC Processor binding says the [di]-cache-*
>> @@ -77,246 +44,115 @@ static const struct cache_type_info cache_type_info[] = {
>> .nr_sets_prop = "d-cache-sets",
>> },
>> {
>> - .name = "Instruction",
>> - .size_prop = "i-cache-size",
>> - .line_size_props = { "i-cache-line-size",
>> - "i-cache-block-size", },
>> - .nr_sets_prop = "i-cache-sets",
>> - },
>> - {
>> .name = "Data",
>> .size_prop = "d-cache-size",
>> .line_size_props = { "d-cache-line-size",
>> "d-cache-block-size", },
>> .nr_sets_prop = "d-cache-sets",
>> },
>> + {
>> + .name = "Instruction",
>> + .size_prop = "i-cache-size",
>> + .line_size_props = { "i-cache-line-size",
>> + "i-cache-block-size", },
>> + .nr_sets_prop = "i-cache-sets",
>> + },
>> };
>
>
> Hey Sudeep,
>
> After applying this patch, the cache_type_info array looks like this.
>
> static const struct cache_type_info cache_type_info[] = {
> {
> /*
> * PowerPC Processor binding says the [di]-cache-*
> * must be equal on unified caches, so just use
> * d-cache properties.
> */
> .name = "Unified",
> .size_prop = "d-cache-size",
> .line_size_props = { "d-cache-line-size",
> "d-cache-block-size", },
> .nr_sets_prop = "d-cache-sets",
> },
> {
> .name = "Data",
> .size_prop = "d-cache-size",
> .line_size_props = { "d-cache-line-size",
> "d-cache-block-size", },
> .nr_sets_prop = "d-cache-sets",
> },
> {
> .name = "Instruction",
> .size_prop = "i-cache-size",
> .line_size_props = { "i-cache-line-size",
> "i-cache-block-size", },
> .nr_sets_prop = "i-cache-sets",
> },
> };
>
> and this function computes the the array index for any given cache type
> define for PowerPC.
>
> static inline int get_cacheinfo_idx(enum cache_type type)
> {
> if (type == CACHE_TYPE_UNIFIED)
> return 0;
> else
> return type;
> }
>
> These types are define in include/linux/cacheinfo.h as
>
> enum cache_type {
> CACHE_TYPE_NOCACHE = 0,
> CACHE_TYPE_INST = BIT(0), ---> 1
> CACHE_TYPE_DATA = BIT(1), ---> 2
> CACHE_TYPE_SEPARATE = CACHE_TYPE_INST | CACHE_TYPE_DATA,
> CACHE_TYPE_UNIFIED = BIT(2),
> };
>
> When it is UNIFIED we return index 0, which is correct. But the index
> for instruction and data cache seems to be swapped which wrong. This
> will fetch invalid properties for any given cache type.
>
> I have done some initial review and testing for this patch's impact on
> PowerPC (ppc64 POWER specifically). I am trying to do some code clean-up
> and re-arrangements. Will post out soon. Thanks !
It does not work correctly on POWER.
The new patchset adds some more attributes for every cache entry apart from
what we used to have on PowerPC before. From the ABI perspective, the old ones
should reflect the correct value in the same manner as before. Looks like
the generic code will make any attribute as "Unknown" if the arch code does
not populate them in the respective callback.
Here are some problems found on a POWER7 system
(1) L1 instruction cache (cpu<N>/cache/index1/)
====== Before patch ======
coherency_line_size: 128
level: 1
shared_cpu_map: 00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,
00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,
00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,
00000000,00000000,00000000,00000000,00000f00
size: 32K
type: Instruction
===== After patch ========
coherency_line_size: Unknown ----> Wrong
level: 1
shared_cpu_map: 00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,
00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,
00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,
00000000,00000000,00000000,00000000,00ffffff ----> Wrong
size: 0K ----> Wrong
type: Instruction
(2) L3 cache (cpu<N>/cache/index3/)
====== Before patch ======
number_of_sets: 1
size: 4096K
ways_of_associativity: 0
===== After patch ========
number_of_sets: 1
size: 4096K
ways_of_associativity: Unknown ----> Wrong
Need to revisit this implementation on PowerPC and figure out the cause of these problems.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH RFC/RFT v3 6/9] powerpc: move cacheinfo sysfs to generic cacheinfo infrastructure
2014-03-07 6:14 ` Anshuman Khandual
@ 2014-03-10 11:12 ` Sudeep Holla
2014-03-21 3:44 ` Anshuman Khandual
0 siblings, 1 reply; 7+ messages in thread
From: Sudeep Holla @ 2014-03-10 11:12 UTC (permalink / raw)
To: Anshuman Khandual
Cc: linuxppc-dev@lists.ozlabs.org, Paul Mackerras,
linux-kernel@vger.kernel.org, Sudeep Holla
Hi Anshuman,
On 07/03/14 06:14, Anshuman Khandual wrote:
> On 03/07/2014 09:36 AM, Anshuman Khandual wrote:
>> On 02/19/2014 09:36 PM, Sudeep Holla wrote:
>>> From: Sudeep Holla <sudeep.holla@arm.com>
>>>
>>> This patch removes the redundant sysfs cacheinfo code by making use of
>>> the newly introduced generic cacheinfo infrastructure.
>>>
>>> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
>>> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>>> Cc: Paul Mackerras <paulus@samba.org>
>>> Cc: linuxppc-dev@lists.ozlabs.org
>>> ---
>>> arch/powerpc/kernel/cacheinfo.c | 831 ++++++-------------------------=
---------
>>> arch/powerpc/kernel/cacheinfo.h | 8 -
>>> arch/powerpc/kernel/sysfs.c | 4 -
>>> 3 files changed, 109 insertions(+), 734 deletions(-)
>>> delete mode 100644 arch/powerpc/kernel/cacheinfo.h
>>>
>>> diff --git a/arch/powerpc/kernel/cacheinfo.c b/arch/powerpc/kernel/cach=
einfo.c
>>> index 2912b87..05b7580 100644
>>> --- a/arch/powerpc/kernel/cacheinfo.c
>>> +++ b/arch/powerpc/kernel/cacheinfo.c
>>> @@ -10,38 +10,10 @@
>>> * 2 as published by the Free Software Foundation.
>>> */
>>>
>>> +#include <linux/cacheinfo.h>
>>> #include <linux/cpu.h>
>>> -#include <linux/cpumask.h>
>>> #include <linux/kernel.h>
>>> -#include <linux/kobject.h>
>>> -#include <linux/list.h>
>>> -#include <linux/notifier.h>
>>> #include <linux/of.h>
>>> -#include <linux/percpu.h>
>>> -#include <linux/slab.h>
>>> -#include <asm/prom.h>
>>> -
>>> -#include "cacheinfo.h"
>>> -
>>> -/* per-cpu object for tracking:
>>> - * - a "cache" kobject for the top-level directory
>>> - * - a list of "index" objects representing the cpu's local cache hier=
archy
>>> - */
>>> -struct cache_dir {
>>> -=09struct kobject *kobj; /* bare (not embedded) kobject for cache
>>> -=09=09=09 * directory */
>>> -=09struct cache_index_dir *index; /* list of index objects */
>>> -};
>>> -
>>> -/* "index" object: each cpu's cache directory has an index
>>> - * subdirectory corresponding to a cache object associated with the
>>> - * cpu. This object's lifetime is managed via the embedded kobject.
>>> - */
>>> -struct cache_index_dir {
>>> -=09struct kobject kobj;
>>> -=09struct cache_index_dir *next; /* next index in parent directory */
>>> -=09struct cache *cache;
>>> -};
>>>
>>> /* Template for determining which OF properties to query for a given
>>> * cache type */
>>> @@ -60,11 +32,6 @@ struct cache_type_info {
>>> =09const char *nr_sets_prop;
>>> };
>>>
>>> -/* These are used to index the cache_type_info array. */
>>> -#define CACHE_TYPE_UNIFIED 0
>>> -#define CACHE_TYPE_INSTRUCTION 1
>>> -#define CACHE_TYPE_DATA 2
>>> -
>>> static const struct cache_type_info cache_type_info[] =3D {
>>> =09{
>>> =09=09/* PowerPC Processor binding says the [di]-cache-*
>>> @@ -77,246 +44,115 @@ static const struct cache_type_info cache_type_in=
fo[] =3D {
>>> =09=09.nr_sets_prop =3D "d-cache-sets",
>>> =09},
>>> =09{
>>> -=09=09.name =3D "Instruction",
>>> -=09=09.size_prop =3D "i-cache-size",
>>> -=09=09.line_size_props =3D { "i-cache-line-size",
>>> -=09=09=09=09 "i-cache-block-size", },
>>> -=09=09.nr_sets_prop =3D "i-cache-sets",
>>> -=09},
>>> -=09{
>>> =09=09.name =3D "Data",
>>> =09=09.size_prop =3D "d-cache-size",
>>> =09=09.line_size_props =3D { "d-cache-line-size",
>>> =09=09=09=09 "d-cache-block-size", },
>>> =09=09.nr_sets_prop =3D "d-cache-sets",
>>> =09},
>>> +=09{
>>> +=09=09.name =3D "Instruction",
>>> +=09=09.size_prop =3D "i-cache-size",
>>> +=09=09.line_size_props =3D { "i-cache-line-size",
>>> +=09=09=09=09 "i-cache-block-size", },
>>> +=09=09.nr_sets_prop =3D "i-cache-sets",
>>> +=09},
>>> };
>>
>>
>> Hey Sudeep,
>>
>> After applying this patch, the cache_type_info array looks like this.
>>
>> static const struct cache_type_info cache_type_info[] =3D {
>> {
>> /*
>> * PowerPC Processor binding says the [di]-cache-*
>> * must be equal on unified caches, so just use
>> * d-cache properties.
>> */
>> .name =3D "Unified",
>> .size_prop =3D "d-cache-size",
>> .line_size_props =3D { "d-cache-line-size",
>> "d-cache-block-size", },
>> .nr_sets_prop =3D "d-cache-sets",
>> },
>> {
>> .name =3D "Data",
>> .size_prop =3D "d-cache-size",
>> .line_size_props =3D { "d-cache-line-size",
>> "d-cache-block-size", },
>> .nr_sets_prop =3D "d-cache-sets",
>> },
>> {
>> .name =3D "Instruction",
>> .size_prop =3D "i-cache-size",
>> .line_size_props =3D { "i-cache-line-size",
>> "i-cache-block-size", },
>> .nr_sets_prop =3D "i-cache-sets",
>> },
>> };
>>
>> and this function computes the the array index for any given cache type
>> define for PowerPC.
>>
>> static inline int get_cacheinfo_idx(enum cache_type type)
>> {
>> if (type =3D=3D CACHE_TYPE_UNIFIED)
>> return 0;
>> else
>> return type;
>> }
>>
>> These types are define in include/linux/cacheinfo.h as
>>
>> enum cache_type {
>> CACHE_TYPE_NOCACHE =3D 0,
>> CACHE_TYPE_INST =3D BIT(0),=09=09---> 1
>> CACHE_TYPE_DATA =3D BIT(1),=09=09---> 2
>> CACHE_TYPE_SEPARATE =3D CACHE_TYPE_INST | CACHE_TYPE_DATA,
>> CACHE_TYPE_UNIFIED =3D BIT(2),
>> };
>>
>> When it is UNIFIED we return index 0, which is correct. But the index
>> for instruction and data cache seems to be swapped which wrong. This
>> will fetch invalid properties for any given cache type.
>>
Ah, that's silly mistake on my side, will fix it.
>> I have done some initial review and testing for this patch's impact on
>> PowerPC (ppc64 POWER specifically). I am trying to do some code clean-up
>> and re-arrangements. Will post out soon. Thanks !
Thanks for taking time for testing and reviewing these patches.
>
> It does not work correctly on POWER.
>
> The new patchset adds some more attributes for every cache entry apart fr=
om
> what we used to have on PowerPC before. From the ABI perspective, the old=
ones
> should reflect the correct value in the same manner as before. Looks like
> the generic code will make any attribute as "Unknown" if the arch code do=
es
> not populate them in the respective callback.
>
Yes this is on my list, I need to avoid populating the sysfs files with=20
"Unknown" as value, will do that in next version.
> Here are some problems found on a POWER7 system
>
> (1) L1 instruction cache (cpu<N>/cache/index1/)
>
> =09=3D=3D=3D=3D=3D=3D Before patch =3D=3D=3D=3D=3D=3D
>
> =09coherency_line_size: =09128
> =09level:=09=09=091
> =09shared_cpu_map:=09=0900000000,00000000,00000000,00000000,00000000,0000=
0000,00000000,00000000,00000000,
> =09=09=0900000000,00000000,00000000,00000000,00000000,00000000,0=
0000000,00000000,00000000,
> =09=09=09=0900000000,00000000,00000000,00000000,00000000,00000000,0000000=
0,00000000,00000000,
> =09=09=09=0900000000,00000000,00000000,00000000,00000f00
> =09size:=09=09=0932K
> =09type:=09=09=09Instruction
>
> =09=3D=3D=3D=3D=3D After patch =3D=3D=3D=3D=3D=3D=3D=3D
>
> =09coherency_line_size:=09Unknown=09=09=09=09=09=09----> Wrong
> =09level:=09=09=091
> =09shared_cpu_map:=09=0900000000,00000000,00000000,00000000,00000000,0000=
0000,00000000,00000000,00000000,
> =09=09=0900000000,00000000,00000000,00000000,00000000,00000000,0=
0000000,00000000,00000000,
> =09=09=09=0900000000,00000000,00000000,00000000,00000000,00000000,0000000=
0,00000000,00000000,
> =09=09=09=0900000000,00000000,00000000,00000000,00ffffff=09----> Wrong
> =09size:=09=09=090K=09=09=09=09=09=09----> Wrong
> =09type:=09=09=09Instruction=09
>
> (2) L3 cache (cpu<N>/cache/index3/)
>
> =09=3D=3D=3D=3D=3D=3D Before patch =3D=3D=3D=3D=3D=3D
>
> =09number_of_sets:=09=091
> =09size:=09=09=094096K
> =09ways_of_associativity:=090
>
> =09=3D=3D=3D=3D=3D After patch =3D=3D=3D=3D=3D=3D=3D=3D
>
> =09number_of_sets:=09=091
> =09size:=09=09=094096K
> =09ways_of_associativity:=09Unknown=09=09----> Wrong
>
> Need to revisit this implementation on PowerPC and figure out the cause o=
f these problems.
>
Yes, based on the logs you have provided, I will check for the root=20
cause of these issues. I will get back with questions if I need=20
clarifications.
Regards,
Sudeep
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH RFC/RFT v3 6/9] powerpc: move cacheinfo sysfs to generic cacheinfo infrastructure
2014-03-10 11:12 ` Sudeep Holla
@ 2014-03-21 3:44 ` Anshuman Khandual
2014-03-21 12:04 ` Sudeep Holla
0 siblings, 1 reply; 7+ messages in thread
From: Anshuman Khandual @ 2014-03-21 3:44 UTC (permalink / raw)
To: Sudeep Holla
Cc: linuxppc-dev@lists.ozlabs.org, Paul Mackerras,
linux-kernel@vger.kernel.org
On 03/10/2014 04:42 PM, Sudeep Holla wrote:
> Hi Anshuman,
>
> On 07/03/14 06:14, Anshuman Khandual wrote:
>> On 03/07/2014 09:36 AM, Anshuman Khandual wrote:
>>> On 02/19/2014 09:36 PM, Sudeep Holla wrote:
>>>> From: Sudeep Holla <sudeep.holla@arm.com>
>>>>
>>>> This patch removes the redundant sysfs cacheinfo code by making use of
>>>> the newly introduced generic cacheinfo infrastructure.
>>>>
>>>> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
>>>> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>>>> Cc: Paul Mackerras <paulus@samba.org>
>>>> Cc: linuxppc-dev@lists.ozlabs.org
>>>> ---
>>>> arch/powerpc/kernel/cacheinfo.c | 831
>>>> ++++++----------------------------------
>>>> arch/powerpc/kernel/cacheinfo.h | 8 -
>>>> arch/powerpc/kernel/sysfs.c | 4 -
>>>> 3 files changed, 109 insertions(+), 734 deletions(-)
>>>> delete mode 100644 arch/powerpc/kernel/cacheinfo.h
>>>>
>>>> diff --git a/arch/powerpc/kernel/cacheinfo.c
>>>> b/arch/powerpc/kernel/cacheinfo.c
>>>> index 2912b87..05b7580 100644
>>>> --- a/arch/powerpc/kernel/cacheinfo.c
>>>> +++ b/arch/powerpc/kernel/cacheinfo.c
>>>> @@ -10,38 +10,10 @@
>>>> * 2 as published by the Free Software Foundation.
>>>> */
>>>>
>>>> +#include <linux/cacheinfo.h>
>>>> #include <linux/cpu.h>
>>>> -#include <linux/cpumask.h>
>>>> #include <linux/kernel.h>
>>>> -#include <linux/kobject.h>
>>>> -#include <linux/list.h>
>>>> -#include <linux/notifier.h>
>>>> #include <linux/of.h>
>>>> -#include <linux/percpu.h>
>>>> -#include <linux/slab.h>
>>>> -#include <asm/prom.h>
>>>> -
>>>> -#include "cacheinfo.h"
>>>> -
>>>> -/* per-cpu object for tracking:
>>>> - * - a "cache" kobject for the top-level directory
>>>> - * - a list of "index" objects representing the cpu's local cache
>>>> hierarchy
>>>> - */
>>>> -struct cache_dir {
>>>> - struct kobject *kobj; /* bare (not embedded) kobject for cache
>>>> - * directory */
>>>> - struct cache_index_dir *index; /* list of index objects */
>>>> -};
>>>> -
>>>> -/* "index" object: each cpu's cache directory has an index
>>>> - * subdirectory corresponding to a cache object associated with the
>>>> - * cpu. This object's lifetime is managed via the embedded kobject.
>>>> - */
>>>> -struct cache_index_dir {
>>>> - struct kobject kobj;
>>>> - struct cache_index_dir *next; /* next index in parent directory */
>>>> - struct cache *cache;
>>>> -};
>>>>
>>>> /* Template for determining which OF properties to query for a given
>>>> * cache type */
>>>> @@ -60,11 +32,6 @@ struct cache_type_info {
>>>> const char *nr_sets_prop;
>>>> };
>>>>
>>>> -/* These are used to index the cache_type_info array. */
>>>> -#define CACHE_TYPE_UNIFIED 0
>>>> -#define CACHE_TYPE_INSTRUCTION 1
>>>> -#define CACHE_TYPE_DATA 2
>>>> -
>>>> static const struct cache_type_info cache_type_info[] = {
>>>> {
>>>> /* PowerPC Processor binding says the [di]-cache-*
>>>> @@ -77,246 +44,115 @@ static const struct cache_type_info
>>>> cache_type_info[] = {
>>>> .nr_sets_prop = "d-cache-sets",
>>>> },
>>>> {
>>>> - .name = "Instruction",
>>>> - .size_prop = "i-cache-size",
>>>> - .line_size_props = { "i-cache-line-size",
>>>> - "i-cache-block-size", },
>>>> - .nr_sets_prop = "i-cache-sets",
>>>> - },
>>>> - {
>>>> .name = "Data",
>>>> .size_prop = "d-cache-size",
>>>> .line_size_props = { "d-cache-line-size",
>>>> "d-cache-block-size", },
>>>> .nr_sets_prop = "d-cache-sets",
>>>> },
>>>> + {
>>>> + .name = "Instruction",
>>>> + .size_prop = "i-cache-size",
>>>> + .line_size_props = { "i-cache-line-size",
>>>> + "i-cache-block-size", },
>>>> + .nr_sets_prop = "i-cache-sets",
>>>> + },
>>>> };
>>>
>>>
>>> Hey Sudeep,
>>>
>>> After applying this patch, the cache_type_info array looks like this.
>>>
>>> static const struct cache_type_info cache_type_info[] = {
>>> {
>>> /*
>>> * PowerPC Processor binding says the [di]-cache-*
>>> * must be equal on unified caches, so just use
>>> * d-cache properties.
>>> */
>>> .name = "Unified",
>>> .size_prop = "d-cache-size",
>>> .line_size_props = { "d-cache-line-size",
>>> "d-cache-block-size", },
>>> .nr_sets_prop = "d-cache-sets",
>>> },
>>> {
>>> .name = "Data",
>>> .size_prop = "d-cache-size",
>>> .line_size_props = { "d-cache-line-size",
>>> "d-cache-block-size", },
>>> .nr_sets_prop = "d-cache-sets",
>>> },
>>> {
>>> .name = "Instruction",
>>> .size_prop = "i-cache-size",
>>> .line_size_props = { "i-cache-line-size",
>>> "i-cache-block-size", },
>>> .nr_sets_prop = "i-cache-sets",
>>> },
>>> };
>>>
>>> and this function computes the the array index for any given cache type
>>> define for PowerPC.
>>>
>>> static inline int get_cacheinfo_idx(enum cache_type type)
>>> {
>>> if (type == CACHE_TYPE_UNIFIED)
>>> return 0;
>>> else
>>> return type;
>>> }
>>>
>>> These types are define in include/linux/cacheinfo.h as
>>>
>>> enum cache_type {
>>> CACHE_TYPE_NOCACHE = 0,
>>> CACHE_TYPE_INST = BIT(0), ---> 1
>>> CACHE_TYPE_DATA = BIT(1), ---> 2
>>> CACHE_TYPE_SEPARATE = CACHE_TYPE_INST | CACHE_TYPE_DATA,
>>> CACHE_TYPE_UNIFIED = BIT(2),
>>> };
>>>
>>> When it is UNIFIED we return index 0, which is correct. But the index
>>> for instruction and data cache seems to be swapped which wrong. This
>>> will fetch invalid properties for any given cache type.
>>>
>
> Ah, that's silly mistake on my side, will fix it.
>
>>> I have done some initial review and testing for this patch's impact on
>>> PowerPC (ppc64 POWER specifically). I am trying to do some code clean-up
>>> and re-arrangements. Will post out soon. Thanks !
>
> Thanks for taking time for testing and reviewing these patches.
Now that you got some of the problems to work on and resend the patches, I will
hold on to the clean up patches I had.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH RFC/RFT v3 6/9] powerpc: move cacheinfo sysfs to generic cacheinfo infrastructure
2014-03-21 3:44 ` Anshuman Khandual
@ 2014-03-21 12:04 ` Sudeep Holla
0 siblings, 0 replies; 7+ messages in thread
From: Sudeep Holla @ 2014-03-21 12:04 UTC (permalink / raw)
To: Anshuman Khandual
Cc: linuxppc-dev@lists.ozlabs.org, Paul Mackerras,
linux-kernel@vger.kernel.org, Sudeep Holla
Hi Anshuman,
On 21/03/14 03:44, Anshuman Khandual wrote:
> On 03/10/2014 04:42 PM, Sudeep Holla wrote:
>> Hi Anshuman,
>>
>> On 07/03/14 06:14, Anshuman Khandual wrote:
>>> On 03/07/2014 09:36 AM, Anshuman Khandual wrote:
>>>> On 02/19/2014 09:36 PM, Sudeep Holla wrote:
>>>>> From: Sudeep Holla <sudeep.holla@arm.com>
>>>>>
>>>>> This patch removes the redundant sysfs cacheinfo code by making use o=
f
>>>>> the newly introduced generic cacheinfo infrastructure.
[...]
>>>> When it is UNIFIED we return index 0, which is correct. But the index
>>>> for instruction and data cache seems to be swapped which wrong. This
>>>> will fetch invalid properties for any given cache type.
>>>>
>>
>> Ah, that's silly mistake on my side, will fix it.
>>
>>>> I have done some initial review and testing for this patch's impact on
>>>> PowerPC (ppc64 POWER specifically). I am trying to do some code clean-=
up
>>>> and re-arrangements. Will post out soon. Thanks !
>>
>> Thanks for taking time for testing and reviewing these patches.
>=20
> Now that you got some of the problems to work on and resend the patches, =
I will
> hold on to the clean up patches I had.
>=20
I have done most of the changes but still unable to find why the shared_cpu=
_map
is getting incorrect on PPC. All the other wrong entries are fixed. Any clu=
e on
shared_cpu_map ?
Regards,
Sudeep
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2014-03-21 12:03 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-02-19 16:06 [PATCH RFC/RFT v3 0/9] drivers: cacheinfo support Sudeep Holla
2014-02-19 16:06 ` [PATCH RFC/RFT v3 6/9] powerpc: move cacheinfo sysfs to generic cacheinfo infrastructure Sudeep Holla
2014-03-07 4:06 ` Anshuman Khandual
2014-03-07 6:14 ` Anshuman Khandual
2014-03-10 11:12 ` Sudeep Holla
2014-03-21 3:44 ` Anshuman Khandual
2014-03-21 12:04 ` Sudeep Holla
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).