On Thu, Feb 13, 2014 at 03:55:47PM +0000, Sudeep Holla wrote:
> Hi Greg,
>
> On 11/02/14 00:13, Greg Kroah-Hartman wrote:
> > On Mon, Feb 10, 2014 at 06:09:58PM +0000, Sudeep Holla wrote:
> >> On 07/02/14 19:29, Greg Kroah-Hartman wrote:
> >>> On Fri, Feb 07, 2014 at 04:49:16PM +0000, Sudeep Holla wrote:
> >>>> From: Sudeep Holla <sudeep.holla@xxxxxxx>
> >>>>
> >>>> This patch adds initial support for providing processor cache information
> >>>> to userspace through sysfs interface. This is based on already existing
> >>>> implementations(x86, ia64, s390 and powerpc) and hence the interface is
> >>>> intended to be fully compatible.
> >>>>
> >>>> The main purpose of this generic support is to avoid further code
> >>>> duplication to support new architectures and also to unify all the existing
> >>>> different implementations.
> >>>>
> >>>> This implementation maintains the hierarchy of cache objects which reflects
> >>>> the system's cache topology. Cache objects are instantiated as needed as
> >>>> CPUs come online. The cache objects are replicated per-cpu even if they are
> >>>> shared. A per-cpu array of cache information maintained is used mainly for
> >>>> sysfs-related book keeping.
> >>>
> >>> I thought I asked that you not use "raw" kobjects for this, instead
> >>> using 'struct device' or just an attribute group?
> >>>
> >>
> >> Correct, sorry I should have mentioned here instead of cover letter that it's
> >> not yet done. Since the changes involved other architectures, I posted v2 to get
> >> early feedback and testing. More over it's one place to fix now instead of 4
> >> unlike before.
> >
> > Ok, I'll wait to review it after you do the device conversion.
> >
> >> Just adding cache as a device as you suggested won't suffice here. As we need to
> >> track multiple cache indices for each cpu, devices are needed for each cache
> >> index. I tried using device_create_with_groups which provides all we need in one
> >> api for cache indices but since cpu devices are not associated with any class,
> >> it fails if class is NULL. Any suggestions ?
> >
> > Make the cpu devices be part of a class?
>
> I was able to convert these to use struct device instead of raw kobjects. But it
> requires some changes in order to retain the existing sysfs path mainly due to
> the fact that cpus are using legacy subsys_system_register and introducing
> cpu_class conflicts with cpu bus. The base changes in the driver core is as
> below. Is this acceptable ? or any other alternate suggestions ?
>
> --->8
> drivers/base/bus.c | 2 ++
> drivers/base/core.c | 8 ++++++++
> drivers/base/cpu.c | 7 +++++++
> include/linux/cpu.h | 2 ++
> 4 files changed, 19 insertions(+)
>
> diff --git a/drivers/base/bus.c b/drivers/base/bus.c
> index 59dc808..c33bfdb 100644
> --- a/drivers/base/bus.c
> +++ b/drivers/base/bus.c
> @@ -518,10 +518,12 @@ int bus_add_device(struct device *dev)
> &dev->kobj, dev_name(dev));
> if (error)
> goto out_id;
> + if (!dev->class) {
> error = sysfs_create_link(&dev->kobj,
> &dev->bus->p->subsys.kobj, "subsystem");
> if (error)
> goto out_subsys;
> + }
> klist_add_tail(&dev->p->knode_bus, &bus->p->klist_devices);
> }
> return 0;
This change worries me, does it cause anything else to happen in sysfs
that looks differently from what it does today?
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index 2b56717..ef81984 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -10,6 +10,7 @@
> *
> */
>
> +#include <linux/cpu.h>
> #include <linux/device.h>
> #include <linux/err.h>
> #include <linux/init.h>
> @@ -759,6 +760,8 @@ static struct kobject *get_device_parent(struct device *dev,
> return &block_class.p->subsys.kobj;
> }
> #endif
> + if (dev->class == cpu_class)
> + return &parent->kobj;
I don't think this is ok :)
Oh wait, we do this for disk classes today, ick, ok, nevermind, but
comment it really well please.
> /*
> * If we have no parent, we live in "virtual".
> @@ -843,6 +846,8 @@ static int device_add_class_symlinks(struct device *dev)
> if (sysfs_deprecated && dev->class == &block_class)
> return 0;
> #endif
> + if (dev->class == cpu_class)
> + return 0;
Same here.
> /* link in the class directory pointing to the device */
> error = sysfs_create_link(&dev->class->p->subsys.kobj,
> @@ -873,6 +878,9 @@ static void device_remove_class_symlinks(struct device *dev)
> if (sysfs_deprecated && dev->class == &block_class)
> return;
> #endif
> + if (dev->class == cpu_class)
> + return;
And here.
thanks,
greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
