driver model, duplicate names question

driver model, duplicate names question

[Srinivas Pandruvada]

Hi Greg,

I would like to create tree like structure using device model (struct 
device, device_register/device_unregister) using parent/child 
relationship while creation. I want to be able to create duplicate 
names, when their parents are different, similar to a directory structure.
I see that I can't create devices with duplicate names (device names), 
even when their parents are different.

How can I allow duplicate names when their parents are different devices?
I want to avoid flat model as I have parent child relationship and there 
will be too many devices using flat model.

Why, I need?
I am going to publish RFC for a new power cap class driver. We have a 
multiple controllers under power cap class (they are devices). Under 
which there are multiple power zones, with parent/child relationships. 
Currently I have to use  kobject_init_and_add, which I want to avoid and 
just use device_register. Other places, wherever such relationships are 
required, kobjects are used like cpufreq.

Thanks,
Srinivas Pandruvada
Open Source Technology Center,
Intel Corp.

Re: driver model, duplicate names question

[Greg KH]

On Tue, Jul 16, 2013 at 09:34:57AM -0700, Srinivas Pandruvada wrote:
> Hi Greg,
> 
> I would like to create tree like structure using device model (struct 
> device, device_register/device_unregister) using parent/child 
> relationship while creation. I want to be able to create duplicate 
> names, when their parents are different, similar to a directory structure.
> I see that I can't create devices with duplicate names (device names), 
> even when their parents are different.

We actually check that?  Nice, I didn't realize that :)

> How can I allow duplicate names when their parents are different devices?
> I want to avoid flat model as I have parent child relationship and there 
> will be too many devices using flat model.

Devices on the same bus shouldn't have the same name, but if they are in
a "tree", it should be ok.  What check is erroring out?

> Why, I need?
> I am going to publish RFC for a new power cap class driver. We have a 
> multiple controllers under power cap class (they are devices). Under 
> which there are multiple power zones, with parent/child relationships. 
> Currently I have to use  kobject_init_and_add, which I want to avoid and 
> just use device_register. Other places, wherever such relationships are 
> required, kobjects are used like cpufreq.

Yes, you shouldn't use "raw" kobject calls at all, so we should fix
this.

thanks,

greg k-h

Re: driver model, duplicate names question

[Srinivas Pandruvada]

[-- Attachment #1: Type: text/plain, Size: 2086 bytes --]

Thanks for the quick response. Here I am creating virtual devices using 
device_register.
I have attached a simple test program, which will give error.

This is my intention:

$> cd /sys/class/test_class
$> ls
power_zone_cpu_package_0
power_zone_cpu_package_1


$> cd power_zone_cpu_package_0
$> ls
power-zone-cpu-0

$> cd ..

$> cd power_zone_cpu_package_1
$> ls
power-zone-cpu-0


The error will be "
Error:
sysfs: cannot create duplicate filename '/class/test_class/power-zone-cpu-0'

I want to avoid a flat model, if there is a server system with many CPU 
packages with multiple cores in each.

Thanks,
Srinivas











On 07/16/2013 09:44 AM, Greg KH wrote:
> On Tue, Jul 16, 2013 at 09:34:57AM -0700, Srinivas Pandruvada wrote:
>> Hi Greg,
>>
>> I would like to create tree like structure using device model (struct
>> device, device_register/device_unregister) using parent/child
>> relationship while creation. I want to be able to create duplicate
>> names, when their parents are different, similar to a directory structure.
>> I see that I can't create devices with duplicate names (device names),
>> even when their parents are different.
> We actually check that?  Nice, I didn't realize that :)
>
>> How can I allow duplicate names when their parents are different devices?
>> I want to avoid flat model as I have parent child relationship and there
>> will be too many devices using flat model.
> Devices on the same bus shouldn't have the same name, but if they are in
> a "tree", it should be ok.  What check is erroring out?
>
>> Why, I need?
>> I am going to publish RFC for a new power cap class driver. We have a
>> multiple controllers under power cap class (they are devices). Under
>> which there are multiple power zones, with parent/child relationships.
>> Currently I have to use  kobject_init_and_add, which I want to avoid and
>> just use device_register. Other places, wherever such relationships are
>> required, kobjects are used like cpufreq.
> Yes, you shouldn't use "raw" kobject calls at all, so we should fix
> this.
>
> thanks,
>
> greg k-h
>


[-- Attachment #2: test_dev_create.c --]
[-- Type: text/x-csrc, Size: 2189 bytes --]

#include <linux/module.h>
#include <linux/device.h>
#include <linux/err.h>
#include <linux/slab.h>

static struct device dev_par_0;
static struct device dev_par_0_child_0;

static struct device dev_par_1;
static struct device dev_par_1_child_0;


static struct class test_class_class = {
	.name = "test_class",
};

static int create_test_devices(void)
{
	int result;

	dev_set_name(&dev_par_0, "power_zone_cpu_package_0");

	dev_par_0.class = &test_class_class;
	result = device_register(&dev_par_0);
	if (result) {
		pr_err("device_register failed for parent_0\n");
		return result;
	}
	
	dev_set_name(&dev_par_1, "power_zone_cpu_package_1");
	dev_par_1.class = &test_class_class;
	result = device_register(&dev_par_1);
	if (result) {
		pr_err("device_register failed for parent_0\n");		
		goto error_par1;
	}
	

	dev_set_name(&dev_par_0_child_0, "power-zone-cpu-0");
	dev_par_0_child_0.class = &test_class_class;
	dev_par_0_child_0.parent = &dev_par_0;
	result = device_register(&dev_par_0_child_0);
	if (result) {
		pr_err("device_register failed for dev_par_0_child_0\n");		
		goto error_par0_child_0;
	}

	dev_set_name(&dev_par_1_child_0, "power-zone-cpu-0");
	dev_par_1_child_0.class = &test_class_class;
	dev_par_1_child_0.parent = &dev_par_1;
	result = device_register(&dev_par_1_child_0);
	if (result) {
		pr_err("device_register failed for dev_par_1_child_0\n");		
		goto error_par0_child_1;
	}

	return 0;

error_par0_child_1:
	device_unregister(&dev_par_0_child_0);
error_par0_child_0:
	device_unregister(&dev_par_1);
error_par1:
	device_unregister(&dev_par_0);

	return result;
}




static int __init dev_create_test_init(void)
{
	int result = 0;

	pr_debug("dev_create_test_init\n");
	result = class_register(&test_class_class);
	if (result)
		return result;

	result = create_test_devices();

	return result;
}

static void dev_create_test_exit(void)
{
	pr_debug("dev_create_test_exit\n");

	device_unregister(&dev_par_0_child_0);
	device_unregister(&dev_par_1_child_0);	
	device_unregister(&dev_par_0);
	device_unregister(&dev_par_1);

	class_unregister(&test_class_class);
}


module_init(dev_create_test_init)
module_exit(dev_create_test_exit)
MODULE_LICENSE("GPL v2");

Re: driver model, duplicate names question

[Greg KH]

On Tue, Jul 16, 2013 at 11:29:42AM -0700, Srinivas Pandruvada wrote:
> Thanks for the quick response. Here I am creating virtual devices using 
> device_register.
> I have attached a simple test program, which will give error.
> 
> This is my intention:
> 
> $> cd /sys/class/test_class
> $> ls
> power_zone_cpu_package_0
> power_zone_cpu_package_1

Wait, you are mixing a class and a "real" bus up.  This will fail as
your devices all end up on the virtual "bus" with the same name, in the
same location on the bus (look in /sys/devices/virtual/ for where they
will end up at.

That will fail, and rightly so.

Try using this with the proper 'struct bus_type' and let me know if
creating a device there with the same name will also fail.

Oh crud, it will, because we can't create symlinks with the same bus
type in the /sys/bus/BUSTYPE/devices/ directory.

So, don't use the same name for a device on the same bus, that way
causes confusion :)

Let's get back to your original "problem", what again are you trying to
solve?  There should be a way to resolve this without having to deal
with duplicate names, perhaps you just want an attribute group with a
common name?

thanks,

greg k-h

Re: driver model, duplicate names question

[Srinivas Pandruvada]

Hi,

I am assigned to do add a powercap class. There are several 
technologies, which will allow to
add a power budget to an individual device. For example, you can set a 
power budget to
a individual physical cpu package, each core and uncore devices, GPUs, 
DRAM etc.

+The Power Capping framework organizes power capping devices under a tree structure.
+At the root level, each device is under some "controller", which is the enabler
+of technology. For example this can be "RAPL".
+Under each controllers, there are multiple power zones, which can be independently
+monitored and controlled.
+Each power zone can be organized as a tree with parent, children and siblings.
+Each power zone defines attributes to enable power monitoring and constraints.
+
+Example Sys-FS Interface
+
+/sys/class/power_cap/intel-rapl
+├── package-0
+│   ├── constraint-0
+│   │   ├── name
+│   │   ├── power_limit_uw
+│   │   └── time_window_us
+│   ├── constraint-1
+│   │   ├── name
+│   │   ├── power_limit_uw
+│   │   └── time_window_us
+│   ├── core
+│   │   ├── constraint-0
+│   │   │   ├── name
+│   │   │   ├── power_limit_uw
+│   │   │   └── time_window_us
+│   │   ├── energy_uj
+│   │   └── max_energy_range_uj
+│   ├── dram
+│   │   ├── constraint-0
+│   │   │   ├── name
+│   │   │   ├── power_limit_uw
+│   │   │   └── time_window_us
+│   │   ├── energy_uj
+│   │   └── max_energy_range_uj
+│   ├── energy_uj
+│   ├── max_energy_range_uj
+│   └── max_power_range_uw
+├── package-1
+│   ├── constraint-0
+│   │   ├── name
+│   │   ├── power_limit_uw
+│   │   └── time_window_us
+│   ├── constraint-1
+│   │   ├── name
+│   │   ├── power_limit_uw
+│   │   └── time_window_us
+│   ├── core
+│   │   ├── constraint-0
+│   │   │   ├── name
+│   │   │   ├── power_limit_uw
+│   │   │   └── time_window_us
+│   │   ├── energy_uj
+│   │   └── max_energy_range_uj
+│   ├── dram
+│   │   ├── constraint-0
+│   │   │   ├── name
+│   │   │   ├── power_limit_uw
+│   │   │   └── time_window_us
+│   │   ├── energy_uj
+│   │   └── max_energy_range_uj
+│   ├── energy_uj
+│   ├── max_energy_range_uj
+│   └── max_power_range_uw
+├── power
+│   ├── async
+│   ├── autosuspend_delay_ms
+│   ├── control
+│   ├── runtime_active_kids
+│   ├── runtime_active_time
+│   ├── runtime_enabled
+│   ├── runtime_status
+│   ├── runtime_suspended_time
+│   └── runtime_usage
+├── subsystem -> ../../../../class/power_cap
+└── uevent
+
+For example, above powercap sys-fs tree represents RAPL(Running Average Power Limit)
+type controls available in the Intel® 64 and IA-32 Processor Architectures. Here
+under controller "intel-rapl" there are two CPU packages (package-0/1), which can
+provide power monitoring and controls. A RAPL controller provides control for each
+CPU package, so it can have one node for each package. In addition to providing
+monitoring and control at package level, each package is further divided into
+power zones (called domains in the RAPL specifications). Here zones represent controls
+for core and dram  parts. These zones can be represented as children of package.
+Under RAPL framework there are two constraints, one for short term and one for long term,
+with two different time windows. These can be represented as two constraints, with
+different time windows, power limits and names.


Thanks,
Srinivas




On 07/16/2013 11:31 AM, Greg KH wrote:
> On Tue, Jul 16, 2013 at 11:29:42AM -0700, Srinivas Pandruvada wrote:
>> Thanks for the quick response. Here I am creating virtual devices using
>> device_register.
>> I have attached a simple test program, which will give error.
>>
>> This is my intention:
>>
>> $> cd /sys/class/test_class
>> $> ls
>> power_zone_cpu_package_0
>> power_zone_cpu_package_1
> Wait, you are mixing a class and a "real" bus up.  This will fail as
> your devices all end up on the virtual "bus" with the same name, in the
> same location on the bus (look in /sys/devices/virtual/ for where they
> will end up at.
>
> That will fail, and rightly so.
>
> Try using this with the proper 'struct bus_type' and let me know if
> creating a device there with the same name will also fail.
>
> Oh crud, it will, because we can't create symlinks with the same bus
> type in the /sys/bus/BUSTYPE/devices/ directory.
>
> So, don't use the same name for a device on the same bus, that way
> causes confusion :)
>
> Let's get back to your original "problem", what again are you trying to
> solve?  There should be a way to resolve this without having to deal
> with duplicate names, perhaps you just want an attribute group with a
> common name?
>
> thanks,
>
> greg k-h
>

Re: driver model, duplicate names question

[Greg KH]

A: No.
Q: Should I include quotations after my reply?

http://daringfireball.net/2007/07/on_top

On Tue, Jul 16, 2013 at 11:54:31AM -0700, Srinivas Pandruvada wrote:
> Hi,
> 
> I am assigned to do add a powercap class. There are several 
> technologies, which will allow to add a power budget to an individual
> device. For example, you can set a power budget to a individual
> physical cpu package, each core and uncore devices, GPUs, DRAM etc.

"classes" all reference a "device" in the system, I don't see that in
your tree below, where does that come in?  How do I, as someone who
created a device in the system know to create a your new powercap class
for it?

In other words, are you _sure_ you want a class here and not something
else (i.e. a bus?)

> +The Power Capping framework organizes power capping devices under a tree structure.
> +At the root level, each device is under some "controller", which is the enabler
> +of technology. For example this can be "RAPL".
> +Under each controllers, there are multiple power zones, which can be independently
> +monitored and controlled.
> +Each power zone can be organized as a tree with parent, children and siblings.
> +Each power zone defines attributes to enable power monitoring and constraints.

Ah, this sounds like you want to be a bus, as you have a controller, and
then devices attached to it.

> +Example Sys-FS Interface
> +
> +/sys/class/power_cap/intel-rapl
> +├── package-0
> +│   ├── constraint-0
> +│   │   ├── name
> +│   │   ├── power_limit_uw
> +│   │   └── time_window_us
> +│   ├── constraint-1
> +│   │   ├── name
> +│   │   ├── power_limit_uw
> +│   │   └── time_window_us
> +│   ├── core
> +│   │   ├── constraint-0
> +│   │   │   ├── name
> +│   │   │   ├── power_limit_uw
> +│   │   │   └── time_window_us
> +│   │   ├── energy_uj
> +│   │   └── max_energy_range_uj
> +│   ├── dram
> +│   │   ├── constraint-0
> +│   │   │   ├── name
> +│   │   │   ├── power_limit_uw
> +│   │   │   └── time_window_us
> +│   │   ├── energy_uj
> +│   │   └── max_energy_range_uj
> +│   ├── energy_uj
> +│   ├── max_energy_range_uj
> +│   └── max_power_range_uw
> +├── package-1
> +│   ├── constraint-0
> +│   │   ├── name
> +│   │   ├── power_limit_uw
> +│   │   └── time_window_us
> +│   ├── constraint-1
> +│   │   ├── name
> +│   │   ├── power_limit_uw
> +│   │   └── time_window_us
> +│   ├── core
> +│   │   ├── constraint-0
> +│   │   │   ├── name
> +│   │   │   ├── power_limit_uw
> +│   │   │   └── time_window_us
> +│   │   ├── energy_uj
> +│   │   └── max_energy_range_uj
> +│   ├── dram
> +│   │   ├── constraint-0
> +│   │   │   ├── name
> +│   │   │   ├── power_limit_uw
> +│   │   │   └── time_window_us
> +│   │   ├── energy_uj
> +│   │   └── max_energy_range_uj
> +│   ├── energy_uj
> +│   ├── max_energy_range_uj
> +│   └── max_power_range_uw
> +├── power
> +│   ├── async
> +│   ├── autosuspend_delay_ms
> +│   ├── control
> +│   ├── runtime_active_kids
> +│   ├── runtime_active_time
> +│   ├── runtime_enabled
> +│   ├── runtime_status
> +│   ├── runtime_suspended_time
> +│   └── runtime_usage
> +├── subsystem -> ../../../../class/power_cap
> +└── uevent

Ick.  Rewrite this to use a bus and you should be fine, right?  Don't
use a class, a class is only to be used if you have a device that is a
specific "type of thing".  Like a tty device, it is a class, as lots of
different "real" devices can have tty ports on them (usb, pci, pcmcia,
platform, etc.)

Rethink this using a bus and see if that solves your issues.  You get a
hierarchy with that.  And you can have different "types" of devices on
your bus, making it easy to tell the difference between a "package" and
a "constraint".

Does that help?

greg k-h

Re: driver model, duplicate names question

[Greg KH]

On Tue, Jul 16, 2013 at 12:33:03PM -0700, Srinivas Pandruvada wrote:
> >> +Example Sys-FS Interface
> >> +
> >> +/sys/class/power_cap/intel-rapl
> >> +├── package-0
> >> +│   ├── constraint-0
> >> +│   │   ├── name
> >> +│   │   ├── power_limit_uw
> >> +│   │   └── time_window_us
> >> +│   ├── constraint-1
> >> +│   │   ├── name
> >> +│   │   ├── power_limit_uw
> >> +│   │   └── time_window_us
> >> +│   ├── core
> >> +│   │   ├── constraint-0
> >> +│   │   │   ├── name
> >> +│   │   │   ├── power_limit_uw
> >> +│   │   │   └── time_window_us
> >> +│   │   ├── energy_uj
> >> +│   │   └── max_energy_range_uj
> >> +│   ├── dram
> >> +│   │   ├── constraint-0
> >> +│   │   │   ├── name
> >> +│   │   │   ├── power_limit_uw
> >> +│   │   │   └── time_window_us
> >> +│   │   ├── energy_uj
> >> +│   │   └── max_energy_range_uj
> >> +│   ├── energy_uj
> >> +│   ├── max_energy_range_uj
> >> +│   └── max_power_range_uw
> >> +├── package-1
> >> +│   ├── constraint-0
> >> +│   │   ├── name
> >> +│   │   ├── power_limit_uw
> >> +│   │   └── time_window_us
> >> +│   ├── constraint-1
> >> +│   │   ├── name
> >> +│   │   ├── power_limit_uw
> >> +│   │   └── time_window_us
> >> +│   ├── core
> >> +│   │   ├── constraint-0
> >> +│   │   │   ├── name
> >> +│   │   │   ├── power_limit_uw
> >> +│   │   │   └── time_window_us
> >> +│   │   ├── energy_uj
> >> +│   │   └── max_energy_range_uj
> >> +│   ├── dram
> >> +│   │   ├── constraint-0
> >> +│   │   │   ├── name
> >> +│   │   │   ├── power_limit_uw
> >> +│   │   │   └── time_window_us
> >> +│   │   ├── energy_uj
> >> +│   │   └── max_energy_range_uj
> >> +│   ├── energy_uj
> >> +│   ├── max_energy_range_uj
> >> +│   └── max_power_range_uw
> >> +├── power
> >> +│   ├── async
> >> +│   ├── autosuspend_delay_ms
> >> +│   ├── control
> >> +│   ├── runtime_active_kids
> >> +│   ├── runtime_active_time
> >> +│   ├── runtime_enabled
> >> +│   ├── runtime_status
> >> +│   ├── runtime_suspended_time
> >> +│   └── runtime_usage
> >> +├── subsystem -> ../../../../class/power_cap
> >> +└── uevent
> > Ick.  Rewrite this to use a bus and you should be fine, right?  Don't
> > use a class, a class is only to be used if you have a device that is a
> > specific "type of thing".  Like a tty device, it is a class, as lots of
> > different "real" devices can have tty ports on them (usb, pci, pcmcia,
> > platform, etc.)
> >
> > Rethink this using a bus and see if that solves your issues.  You get a
> > hierarchy with that.  And you can have different "types" of devices on
> > your bus, making it easy to tell the difference between a "package" and
> > a "constraint".
> >
> > Does that help?
> I will experiment your suggestion. I see this class analogous to 
> "/sys/class/thermal",
> , where the thermal class provides a set of consistent interface for all 
> thermal devices.

But thermal devices are not "real" at all.  There are just a number of
"cooling devices" on a virtual bus and not attached to any type of a
real device at all.

There's also no hierarchy that I can see with the thermal class, but you
want to have this, so you will have to do something different because
classes do not have hierarchies.

So try using a device and a bus and see if that helps out.  If not,
please let me know.

thanks,

greg k-h

Re: driver model, duplicate names question

[Srinivas Pandruvada]

On 07/16/2013 12:04 PM, Greg KH wrote:
> A: No.
> Q: Should I include quotations after my reply?
>
> http://daringfireball.net/2007/07/on_top
Glad to learn something new today.
>
> On Tue, Jul 16, 2013 at 11:54:31AM -0700, Srinivas Pandruvada wrote:
>> Hi,
>>
>> I am assigned to do add a powercap class. There are several
>> technologies, which will allow to add a power budget to an individual
>> device. For example, you can set a power budget to a individual
>> physical cpu package, each core and uncore devices, GPUs, DRAM etc.
> "classes" all reference a "device" in the system, I don't see that in
> your tree below, where does that come in?  How do I, as someone who
> created a device in the system know to create a your new powercap class
> for it?
>
> In other words, are you _sure_ you want a class here and not something
> else (i.e. a bus?)
>
>> +The Power Capping framework organizes power capping devices under a tree structure.
>> +At the root level, each device is under some "controller", which is the enabler
>> +of technology. For example this can be "RAPL".
>> +Under each controllers, there are multiple power zones, which can be independently
>> +monitored and controlled.
>> +Each power zone can be organized as a tree with parent, children and siblings.
>> +Each power zone defines attributes to enable power monitoring and constraints.
> Ah, this sounds like you want to be a bus, as you have a controller, and
> then devices attached to it.
>
>> +Example Sys-FS Interface
>> +
>> +/sys/class/power_cap/intel-rapl
>> +├── package-0
>> +│   ├── constraint-0
>> +│   │   ├── name
>> +│   │   ├── power_limit_uw
>> +│   │   └── time_window_us
>> +│   ├── constraint-1
>> +│   │   ├── name
>> +│   │   ├── power_limit_uw
>> +│   │   └── time_window_us
>> +│   ├── core
>> +│   │   ├── constraint-0
>> +│   │   │   ├── name
>> +│   │   │   ├── power_limit_uw
>> +│   │   │   └── time_window_us
>> +│   │   ├── energy_uj
>> +│   │   └── max_energy_range_uj
>> +│   ├── dram
>> +│   │   ├── constraint-0
>> +│   │   │   ├── name
>> +│   │   │   ├── power_limit_uw
>> +│   │   │   └── time_window_us
>> +│   │   ├── energy_uj
>> +│   │   └── max_energy_range_uj
>> +│   ├── energy_uj
>> +│   ├── max_energy_range_uj
>> +│   └── max_power_range_uw
>> +├── package-1
>> +│   ├── constraint-0
>> +│   │   ├── name
>> +│   │   ├── power_limit_uw
>> +│   │   └── time_window_us
>> +│   ├── constraint-1
>> +│   │   ├── name
>> +│   │   ├── power_limit_uw
>> +│   │   └── time_window_us
>> +│   ├── core
>> +│   │   ├── constraint-0
>> +│   │   │   ├── name
>> +│   │   │   ├── power_limit_uw
>> +│   │   │   └── time_window_us
>> +│   │   ├── energy_uj
>> +│   │   └── max_energy_range_uj
>> +│   ├── dram
>> +│   │   ├── constraint-0
>> +│   │   │   ├── name
>> +│   │   │   ├── power_limit_uw
>> +│   │   │   └── time_window_us
>> +│   │   ├── energy_uj
>> +│   │   └── max_energy_range_uj
>> +│   ├── energy_uj
>> +│   ├── max_energy_range_uj
>> +│   └── max_power_range_uw
>> +├── power
>> +│   ├── async
>> +│   ├── autosuspend_delay_ms
>> +│   ├── control
>> +│   ├── runtime_active_kids
>> +│   ├── runtime_active_time
>> +│   ├── runtime_enabled
>> +│   ├── runtime_status
>> +│   ├── runtime_suspended_time
>> +│   └── runtime_usage
>> +├── subsystem -> ../../../../class/power_cap
>> +└── uevent
> Ick.  Rewrite this to use a bus and you should be fine, right?  Don't
> use a class, a class is only to be used if you have a device that is a
> specific "type of thing".  Like a tty device, it is a class, as lots of
> different "real" devices can have tty ports on them (usb, pci, pcmcia,
> platform, etc.)
>
> Rethink this using a bus and see if that solves your issues.  You get a
> hierarchy with that.  And you can have different "types" of devices on
> your bus, making it easy to tell the difference between a "package" and
> a "constraint".
>
> Does that help?
I will experiment your suggestion. I see this class analogous to 
"/sys/class/thermal",
, where the thermal class provides a set of consistent interface for all 
thermal devices.
> greg k-h
>
Thanks,
Srinivas

Re: driver model, duplicate names question

[Srinivas Pandruvada]

On 07/16/2013 12:32 PM, Greg KH wrote:
> On Tue, Jul 16, 2013 at 12:33:03PM -0700, Srinivas Pandruvada wrote:
>>>> +Example Sys-FS Interface
>>>> +
>>>> +/sys/class/power_cap/intel-rapl
>>>> +├── package-0
>>>> +│   ├── constraint-0
>>>> +│   │   ├── name
>>>> +│   │   ├── power_limit_uw
>>>> +│   │   └── time_window_us
>>>> +│   ├── constraint-1
>>>> +│   │   ├── name
>>>> +│   │   ├── power_limit_uw
>>>> +│   │   └── time_window_us
>>>> +│   ├── core
>>>> +│   │   ├── constraint-0
>>>> +│   │   │   ├── name
>>>> +│   │   │   ├── power_limit_uw
>>>> +│   │   │   └── time_window_us
>>>> +│   │   ├── energy_uj
>>>> +│   │   └── max_energy_range_uj
>>>> +│   ├── dram
>>>> +│   │   ├── constraint-0
>>>> +│   │   │   ├── name
>>>> +│   │   │   ├── power_limit_uw
>>>> +│   │   │   └── time_window_us
>>>> +│   │   ├── energy_uj
>>>> +│   │   └── max_energy_range_uj
>>>> +│   ├── energy_uj
>>>> +│   ├── max_energy_range_uj
>>>> +│   └── max_power_range_uw
>>>> +├── package-1
>>>> +│   ├── constraint-0
>>>> +│   │   ├── name
>>>> +│   │   ├── power_limit_uw
>>>> +│   │   └── time_window_us
>>>> +│   ├── constraint-1
>>>> +│   │   ├── name
>>>> +│   │   ├── power_limit_uw
>>>> +│   │   └── time_window_us
>>>> +│   ├── core
>>>> +│   │   ├── constraint-0
>>>> +│   │   │   ├── name
>>>> +│   │   │   ├── power_limit_uw
>>>> +│   │   │   └── time_window_us
>>>> +│   │   ├── energy_uj
>>>> +│   │   └── max_energy_range_uj
>>>> +│   ├── dram
>>>> +│   │   ├── constraint-0
>>>> +│   │   │   ├── name
>>>> +│   │   │   ├── power_limit_uw
>>>> +│   │   │   └── time_window_us
>>>> +│   │   ├── energy_uj
>>>> +│   │   └── max_energy_range_uj
>>>> +│   ├── energy_uj
>>>> +│   ├── max_energy_range_uj
>>>> +│   └── max_power_range_uw
>>>> +├── power
>>>> +│   ├── async
>>>> +│   ├── autosuspend_delay_ms
>>>> +│   ├── control
>>>> +│   ├── runtime_active_kids
>>>> +│   ├── runtime_active_time
>>>> +│   ├── runtime_enabled
>>>> +│   ├── runtime_status
>>>> +│   ├── runtime_suspended_time
>>>> +│   └── runtime_usage
>>>> +├── subsystem -> ../../../../class/power_cap
>>>> +└── uevent
>>> Ick.  Rewrite this to use a bus and you should be fine, right?  Don't
>>> use a class, a class is only to be used if you have a device that is a
>>> specific "type of thing".  Like a tty device, it is a class, as lots of
>>> different "real" devices can have tty ports on them (usb, pci, pcmcia,
>>> platform, etc.)
>>>
>>> Rethink this using a bus and see if that solves your issues.  You get a
>>> hierarchy with that.  And you can have different "types" of devices on
>>> your bus, making it easy to tell the difference between a "package" and
>>> a "constraint".
>>>
>>> Does that help?
>> I will experiment your suggestion. I see this class analogous to
>> "/sys/class/thermal",
>> , where the thermal class provides a set of consistent interface for all
>> thermal devices.
> But thermal devices are not "real" at all.  There are just a number of
> "cooling devices" on a virtual bus and not attached to any type of a
> real device at all.
Similar to cooling drivers, the power cap client drivers don't have to 
be real.
For example intel rapl just uses x86 MSRs. But some other drivers can be 
using
PCIe and use this framework to export control to users.
> There's also no hierarchy that I can see with the thermal class, but you
> want to have this, so you will have to do something different because
> classes do not have hierarchies.
The reason, we tried not have hierarchy for ease of management from
a user space. If I use this flat model, then showing relationships need 
to use
some sort of linking like regulator class. Do you have preference for 
such model?
> So try using a device and a bus and see if that helps out.  If not,
> please let me know.
Looking at this possibility.
> thanks,
>
> greg k-h
>
Thanks,
Srinivas

Re: driver model, duplicate names question

[Greg KH]

On Wed, Jul 17, 2013 at 10:50:19AM -0700, Srinivas Pandruvada wrote:
> On 07/16/2013 12:32 PM, Greg KH wrote:

<snip>

lkml rejects html email, care to resend this so that others can see it?

thanks,

greg k-h

Re: driver model, duplicate names question

[Srinivas Pandruvada]

On 07/16/2013 12:32 PM, Greg KH wrote:
> On Tue, Jul 16, 2013 at 12:33:03PM -0700, Srinivas Pandruvada wrote:
>>>> +Example Sys-FS Interface
>>>> +
>>>> +/sys/class/power_cap/intel-rapl
>>>> +├── package-0
>>>> +│   ├── constraint-0
>>>> +│   │   ├── name
>>>> +│   │   ├── power_limit_uw
>>>> +│   │   └── time_window_us
>>>> +│   ├── constraint-1
>>>> +│   │   ├── name
>>>> +│   │   ├── power_limit_uw
>>>> +│   │   └── time_window_us
>>>> +│   ├── core
>>>> +│   │   ├── constraint-0
>>>> +│   │   │   ├── name
>>>> +│   │   │   ├── power_limit_uw
>>>> +│   │   │   └── time_window_us
>>>> +│   │   ├── energy_uj
>>>> +│   │   └── max_energy_range_uj
>>>> +│   ├── dram
>>>> +│   │   ├── constraint-0
>>>> +│   │   │   ├── name
>>>> +│   │   │   ├── power_limit_uw
>>>> +│   │   │   └── time_window_us
>>>> +│   │   ├── energy_uj
>>>> +│   │   └── max_energy_range_uj
>>>> +│   ├── energy_uj
>>>> +│   ├── max_energy_range_uj
>>>> +│   └── max_power_range_uw
>>>> +├── package-1
>>>> +│   ├── constraint-0
>>>> +│   │   ├── name
>>>> +│   │   ├── power_limit_uw
>>>> +│   │   └── time_window_us
>>>> +│   ├── constraint-1
>>>> +│   │   ├── name
>>>> +│   │   ├── power_limit_uw
>>>> +│   │   └── time_window_us
>>>> +│   ├── core
>>>> +│   │   ├── constraint-0
>>>> +│   │   │   ├── name
>>>> +│   │   │   ├── power_limit_uw
>>>> +│   │   │   └── time_window_us
>>>> +│   │   ├── energy_uj
>>>> +│   │   └── max_energy_range_uj
>>>> +│   ├── dram
>>>> +│   │   ├── constraint-0
>>>> +│   │   │   ├── name
>>>> +│   │   │   ├── power_limit_uw
>>>> +│   │   │   └── time_window_us
>>>> +│   │   ├── energy_uj
>>>> +│   │   └── max_energy_range_uj
>>>> +│   ├── energy_uj
>>>> +│   ├── max_energy_range_uj
>>>> +│   └── max_power_range_uw
>>>> +├── power
>>>> +│   ├── async
>>>> +│   ├── autosuspend_delay_ms
>>>> +│   ├── control
>>>> +│   ├── runtime_active_kids
>>>> +│   ├── runtime_active_time
>>>> +│   ├── runtime_enabled
>>>> +│   ├── runtime_status
>>>> +│   ├── runtime_suspended_time
>>>> +│   └── runtime_usage
>>>> +├── subsystem -> ../../../../class/power_cap
>>>> +└── uevent
>>> Ick.  Rewrite this to use a bus and you should be fine, right?  Don't
>>> use a class, a class is only to be used if you have a device that is a
>>> specific "type of thing".  Like a tty device, it is a class, as lots of
>>> different "real" devices can have tty ports on them (usb, pci, pcmcia,
>>> platform, etc.)
>>>
>>> Rethink this using a bus and see if that solves your issues.  You get a
>>> hierarchy with that.  And you can have different "types" of devices on
>>> your bus, making it easy to tell the difference between a "package" and
>>> a "constraint".
>>>
>>> Does that help?
>> I will experiment your suggestion. I see this class analogous to
>> "/sys/class/thermal",
>> , where the thermal class provides a set of consistent interface for all
>> thermal devices.
> But thermal devices are not "real" at all.  There are just a number of
> "cooling devices" on a virtual bus and not attached to any type of a
> real device at all.
>
> There's also no hierarchy that I can see with the thermal class, but you
> want to have this, so you will have to do something different because
> classes do not have hierarchies.
>
> So try using a device and a bus and see if that helps out.  If not,
> please let me know.
Experimented by using a device and a bus. As your initial mail pointed 
out, it still fails.
It will try to create symlink to /sys/bus/BUSTYPE/devices/, which will 
prevent duplicate names.
Someone suggested to do like usb by creating file system entries, which 
will require me to
wear a body armour to post upstream for review.

So I think the solution is to use prevent duplicate names. So in the 
above example of sys-fs:

"package-0" may be called power_zone#, with attribute "name" = 
"package_0". Its children can
be called power_zone#:#. I will still use parent child relationships 
during device_register.
For constraints, I will be using attributes like constraint_0_name, 
constraint_0_power_limit etc. under each power_zone.
Hope this is an acceptable solution.
> thanks,
>
> greg k-h
>

Re: driver model, duplicate names question

[Greg KH]

On Wed, Jul 17, 2013 at 11:09:44AM -0700, Srinivas Pandruvada wrote:
> > But thermal devices are not "real" at all.  There are just a number of
> > "cooling devices" on a virtual bus and not attached to any type of a
> > real device at all.
> >
> > There's also no hierarchy that I can see with the thermal class, but you
> > want to have this, so you will have to do something different because
> > classes do not have hierarchies.
> >
> > So try using a device and a bus and see if that helps out.  If not,
> > please let me know.
> Experimented by using a device and a bus. As your initial mail pointed 
> out, it still fails.
> It will try to create symlink to /sys/bus/BUSTYPE/devices/, which will 
> prevent duplicate names.

Where are you going to create a symlink from?  That doesn't make sense,
just use a class if you want a link.

> Someone suggested to do like usb by creating file system entries, which 
> will require me to wear a body armour to post upstream for review.

So a separate filesystem alltogether?  That would work, but you then
loose any benifit that the driver core and sysfs provides you, for no
benifit other than looking at a pretty tree in a filesystem.  I don't
see the real need for that at all.

> So I think the solution is to use prevent duplicate names. So in the 
> above example of sys-fs:
> 
> "package-0" may be called power_zone#, with attribute "name" = 
> "package_0". Its children can
> be called power_zone#:#. I will still use parent child relationships 
> during device_register.
> For constraints, I will be using attributes like constraint_0_name, 
> constraint_0_power_limit etc. under each power_zone.
> Hope this is an acceptable solution.

That's what other busses have used for this very reason :)

Also look at the "type" of device you are creating for your bus, that
will help you differentiate them from each other instead of having to
rely on the names of them to determine what is going on within the
kernel.

good luck,

greg k-h

Re: driver model, duplicate names question

[Srinivas Pandruvada]

On 07/17/2013 11:31 AM, Greg KH wrote:
> On Wed, Jul 17, 2013 at 11:09:44AM -0700, Srinivas Pandruvada wrote:
>>> But thermal devices are not "real" at all.  There are just a number of
>>> "cooling devices" on a virtual bus and not attached to any type of a
>>> real device at all.
>>>
>>> There's also no hierarchy that I can see with the thermal class, but you
>>> want to have this, so you will have to do something different because
>>> classes do not have hierarchies.
>>>
>>> So try using a device and a bus and see if that helps out.  If not,
>>> please let me know.
>> Experimented by using a device and a bus. As your initial mail pointed
>> out, it still fails.
>> It will try to create symlink to /sys/bus/BUSTYPE/devices/, which will
>> prevent duplicate names.
> Where are you going to create a symlink from?  That doesn't make sense,
> just use a class if you want a link.
Sorry, my reply may not be clear here. I am not creating any symlinks.
device_register will create a symlink. For example, if I create a bus 
named "test",
with two parents and one child each:
ls -l /sys/bus/test/devices
lrwxrwxrwx 1 root root 0 Jul 17 09:05 child_0 -> 
../../../devices/parent_0/child_0
lrwxrwxrwx 1 root root 0 Jul 17 09:05 child_1 -> 
../../../devices/parent_1/child_1
lrwxrwxrwx 1 root root 0 Jul 17 09:05 parent_0 -> ../../../devices/parent_0
lrwxrwxrwx 1 root root 0 Jul 17 09:05 parent_1 -> ../../../devices/parent_1

>> Someone suggested to do like usb by creating file system entries, which
>> will require me to wear a body armour to post upstream for review.
> So a separate filesystem alltogether?  That would work, but you then
> loose any benifit that the driver core and sysfs provides you, for no
> benifit other than looking at a pretty tree in a filesystem.  I don't
> see the real need for that at all.
>
>> So I think the solution is to use prevent duplicate names. So in the
>> above example of sys-fs:
>>
>> "package-0" may be called power_zone#, with attribute "name" =
>> "package_0". Its children can
>> be called power_zone#:#. I will still use parent child relationships
>> during device_register.
>> For constraints, I will be using attributes like constraint_0_name,
>> constraint_0_power_limit etc. under each power_zone.
>> Hope this is an acceptable solution.
> That's what other busses have used for this very reason :)
>
> Also look at the "type" of device you are creating for your bus, that
> will help you differentiate them from each other instead of having to
> rely on the names of them to determine what is going on within the
> kernel.
<Thanks for your time and help.>
> good luck,
>
> greg k-h
>