Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops

On 02/02/2016 10:54 PM, Viresh Kumar wrote: On 02-02-16, 17:32, Saravana Kannan wrote: But if we are expecting sched dvfs to come in, why make it worse for it. It would be completely pointless to try and shoehorn sched dvfs to use cpufreq_governor.c We can move the common part to cpufreq core

Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops

On 02/02/2016 10:57 PM, Viresh Kumar wrote: On 02-02-16, 20:03, Saravana Kannan wrote: This is the hotplug case I mentioned. The sysfs file removals will happen only for the last CPU in that policy (we thankfully optimized that part last year). We also know that multiple CPUs can't be

Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops

On Wed, Feb 3, 2016 at 2:21 PM, Viresh Kumar wrote: > On 03-02-16, 13:42, Rafael J. Wysocki wrote: >> > +static ssize_t governor_show(struct kobject *kobj, struct attribute *attr, >> > +char *buf) >> > +{ >> > + struct dbs_data *dbs_data = to_dbs_data(kobj); >> >

Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops

On 03-02-16, 13:42, Rafael J. Wysocki wrote: > > +static ssize_t governor_show(struct kobject *kobj, struct attribute *attr, > > +char *buf) > > +{ > > + struct dbs_data *dbs_data = to_dbs_data(kobj); > > + struct governor_attr *gattr = to_gov_attr(attr); >

Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops

On Wed, Feb 3, 2016 at 7:58 AM, Viresh Kumar wrote: > On 02-02-16, 22:23, Rafael J. Wysocki wrote: >> On Tue, Feb 2, 2016 at 11:57 AM, Viresh Kumar >> wrote: > >> "The ondemand and conservative governors use the global-attr or >> freq-attr structures to represent sysfs attributes corresponding

Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops

On 03-02-16, 10:51, Juri Lelli wrote: > I also think that sched-dvfs should not use cpufreq_governor.c. It is > useful boilerplate code for ondemand and conservative, as they share lot > of data structures and how they work, but it doesn't necessarily suit > everybody's needs, IMHO. > > OTOH,

Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops

On 03/02/16 12:24, Viresh Kumar wrote: > On 02-02-16, 17:32, Saravana Kannan wrote: > > But if we are expecting sched dvfs to come in, why make it worse for it. It > > would be completely pointless to try and shoehorn sched dvfs to use > > cpufreq_governor.c > > We can move the common part to

Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops

On 03-02-16, 10:51, Juri Lelli wrote: > I also think that sched-dvfs should not use cpufreq_governor.c. It is > useful boilerplate code for ondemand and conservative, as they share lot > of data structures and how they work, but it doesn't necessarily suit > everybody's needs, IMHO. > > OTOH,

Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops

On 03/02/16 12:24, Viresh Kumar wrote: > On 02-02-16, 17:32, Saravana Kannan wrote: > > But if we are expecting sched dvfs to come in, why make it worse for it. It > > would be completely pointless to try and shoehorn sched dvfs to use > > cpufreq_governor.c > > We can move the common part to

Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops

On 03-02-16, 13:42, Rafael J. Wysocki wrote: > > +static ssize_t governor_show(struct kobject *kobj, struct attribute *attr, > > +char *buf) > > +{ > > + struct dbs_data *dbs_data = to_dbs_data(kobj); > > + struct governor_attr *gattr = to_gov_attr(attr); >

Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops

On Wed, Feb 3, 2016 at 2:21 PM, Viresh Kumar wrote: > On 03-02-16, 13:42, Rafael J. Wysocki wrote: >> > +static ssize_t governor_show(struct kobject *kobj, struct attribute *attr, >> > +char *buf) >> > +{ >> > + struct dbs_data *dbs_data

Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops

On Wed, Feb 3, 2016 at 7:58 AM, Viresh Kumar wrote: > On 02-02-16, 22:23, Rafael J. Wysocki wrote: >> On Tue, Feb 2, 2016 at 11:57 AM, Viresh Kumar >> wrote: > >> "The ondemand and conservative governors use the global-attr or >> freq-attr

Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops

On 02/02/2016 10:57 PM, Viresh Kumar wrote: On 02-02-16, 20:03, Saravana Kannan wrote: This is the hotplug case I mentioned. The sysfs file removals will happen only for the last CPU in that policy (we thankfully optimized that part last year). We also know that multiple CPUs can't be

Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops

On 02/02/2016 10:54 PM, Viresh Kumar wrote: On 02-02-16, 17:32, Saravana Kannan wrote: But if we are expecting sched dvfs to come in, why make it worse for it. It would be completely pointless to try and shoehorn sched dvfs to use cpufreq_governor.c We can move the common part to cpufreq core

Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops

On 02-02-16, 22:23, Rafael J. Wysocki wrote: > On Tue, Feb 2, 2016 at 11:57 AM, Viresh Kumar wrote: > "The ondemand and conservative governors use the global-attr or > freq-attr structures to represent sysfs attributes corresponding to > their tunables > (which of them is actually used depends

Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops

On 02-02-16, 20:03, Saravana Kannan wrote: > This is the hotplug case I mentioned. The sysfs file removals will happen > only for the last CPU in that policy (we thankfully optimized that part last > year). We also know that multiple CPUs can't be hotplugged at the same time. > So, in the start of

Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops

On 02-02-16, 17:32, Saravana Kannan wrote: > But if we are expecting sched dvfs to come in, why make it worse for it. It > would be completely pointless to try and shoehorn sched dvfs to use > cpufreq_governor.c We can move the common part to cpufreq core and not make sched-dvfs reuse

Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops

On 02-02-16, 14:21, Saravana Kannan wrote: > I also don't like this patch because it forces governors to either implement > their own macros and management of their attributes or force them to use the > governor structs that come with cpufreq_governor.h. cpufreq_governor.h IMHO > is very ondemand

Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops

On 02-02-16, 17:01, Juri Lelli wrote: > Hi Rafael, > > On 02/02/16 17:35, Rafael J. Wysocki wrote: > > On Tue, Feb 2, 2016 at 4:47 PM, Juri Lelli wrote: > > > This patch cleans things up a lot, that's good. > > > > > > One thing I'm still concerned about, though: don't we need some locking > >

Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops

On 02/02/2016 05:52 PM, Rafael J. Wysocki wrote: On Wed, Feb 3, 2016 at 2:32 AM, Saravana Kannan wrote: On 02/02/2016 05:07 PM, Rafael J. Wysocki wrote: On Wed, Feb 3, 2016 at 12:42 AM, Rafael J. Wysocki wrote: On Tue, Feb 2, 2016 at 11:21 PM, Saravana Kannan wrote: On 02/02/2016 11:40

Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops

On Wed, Feb 3, 2016 at 2:32 AM, Saravana Kannan wrote: > On 02/02/2016 05:07 PM, Rafael J. Wysocki wrote: >> >> On Wed, Feb 3, 2016 at 12:42 AM, Rafael J. Wysocki >> wrote: >>> >>> On Tue, Feb 2, 2016 at 11:21 PM, Saravana Kannan >>> wrote: On 02/02/2016 11:40 AM, Rafael J. Wysocki

Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops

On 02/02/2016 05:07 PM, Rafael J. Wysocki wrote: On Wed, Feb 3, 2016 at 12:42 AM, Rafael J. Wysocki wrote: On Tue, Feb 2, 2016 at 11:21 PM, Saravana Kannan wrote: On 02/02/2016 11:40 AM, Rafael J. Wysocki wrote: On Tue, Feb 2, 2016 at 6:01 PM, Juri Lelli wrote: [cut] I also don't

Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops

On Wed, Feb 3, 2016 at 12:42 AM, Rafael J. Wysocki wrote: > On Tue, Feb 2, 2016 at 11:21 PM, Saravana Kannan > wrote: >> On 02/02/2016 11:40 AM, Rafael J. Wysocki wrote: >>> >>> On Tue, Feb 2, 2016 at 6:01 PM, Juri Lelli wrote: [cut] >> >> I also don't like this patch because it forces

Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops

On Tue, Feb 2, 2016 at 11:21 PM, Saravana Kannan wrote: > On 02/02/2016 11:40 AM, Rafael J. Wysocki wrote: >> >> On Tue, Feb 2, 2016 at 6:01 PM, Juri Lelli wrote: >>> >>> Hi Rafael, >>> >>> On 02/02/16 17:35, Rafael J. Wysocki wrote: On Tue, Feb 2, 2016 at 4:47 PM, Juri Lelli wrote:

Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops

On 02/02/2016 11:40 AM, Rafael J. Wysocki wrote: On Tue, Feb 2, 2016 at 6:01 PM, Juri Lelli wrote: Hi Rafael, On 02/02/16 17:35, Rafael J. Wysocki wrote: On Tue, Feb 2, 2016 at 4:47 PM, Juri Lelli wrote: Hi Viresh, On 02/02/16 16:27, Viresh Kumar wrote: Until now, governors

Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops

On Tue, Feb 2, 2016 at 11:57 AM, Viresh Kumar wrote: First, the subject might be better. What about something like "cpufreq: governor: New sysfs show/store callbacks for governor tunables", for example? > Until now, governors (ondemand/conservative) were using the > 'global-attr' or

Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops

On Tue, Feb 2, 2016 at 6:01 PM, Juri Lelli wrote: > Hi Rafael, > > On 02/02/16 17:35, Rafael J. Wysocki wrote: >> On Tue, Feb 2, 2016 at 4:47 PM, Juri Lelli wrote: >> > Hi Viresh, >> > >> > On 02/02/16 16:27, Viresh Kumar wrote: >> >> Until now, governors (ondemand/conservative) were using the

Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops

Hi Rafael, On 02/02/16 17:35, Rafael J. Wysocki wrote: > On Tue, Feb 2, 2016 at 4:47 PM, Juri Lelli wrote: > > Hi Viresh, > > > > On 02/02/16 16:27, Viresh Kumar wrote: > >> Until now, governors (ondemand/conservative) were using the > >> 'global-attr' or 'freq-attr', depending on the sysfs

Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops

On Tue, Feb 2, 2016 at 4:47 PM, Juri Lelli wrote: > Hi Viresh, > > On 02/02/16 16:27, Viresh Kumar wrote: >> Until now, governors (ondemand/conservative) were using the >> 'global-attr' or 'freq-attr', depending on the sysfs location where we >> want to create governor's directory. >> >> The

Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops

Hi Viresh, On 02/02/16 16:27, Viresh Kumar wrote: > Until now, governors (ondemand/conservative) were using the > 'global-attr' or 'freq-attr', depending on the sysfs location where we > want to create governor's directory. > > The problem is that, in case of 'freq-attr', we are forced to use >

[PATCH 2/5] cpufreq: governor: Create separate sysfs-ops

Until now, governors (ondemand/conservative) were using the 'global-attr' or 'freq-attr', depending on the sysfs location where we want to create governor's directory. The problem is that, in case of 'freq-attr', we are forced to use show()/store() present in cpufreq.c, which always take

Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops

On Tue, Feb 2, 2016 at 11:57 AM, Viresh Kumar wrote: First, the subject might be better. What about something like "cpufreq: governor: New sysfs show/store callbacks for governor tunables", for example? > Until now, governors (ondemand/conservative) were using the >

Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops

On 02/02/2016 11:40 AM, Rafael J. Wysocki wrote: On Tue, Feb 2, 2016 at 6:01 PM, Juri Lelli wrote: Hi Rafael, On 02/02/16 17:35, Rafael J. Wysocki wrote: On Tue, Feb 2, 2016 at 4:47 PM, Juri Lelli wrote: Hi Viresh, On 02/02/16 16:27, Viresh Kumar

Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops

On Tue, Feb 2, 2016 at 11:21 PM, Saravana Kannan wrote: > On 02/02/2016 11:40 AM, Rafael J. Wysocki wrote: >> >> On Tue, Feb 2, 2016 at 6:01 PM, Juri Lelli wrote: >>> >>> Hi Rafael, >>> >>> On 02/02/16 17:35, Rafael J. Wysocki wrote: On Tue,

Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops

On Wed, Feb 3, 2016 at 12:42 AM, Rafael J. Wysocki wrote: > On Tue, Feb 2, 2016 at 11:21 PM, Saravana Kannan > wrote: >> On 02/02/2016 11:40 AM, Rafael J. Wysocki wrote: >>> >>> On Tue, Feb 2, 2016 at 6:01 PM, Juri Lelli wrote:

Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops

On 02/02/2016 05:07 PM, Rafael J. Wysocki wrote: On Wed, Feb 3, 2016 at 12:42 AM, Rafael J. Wysocki wrote: On Tue, Feb 2, 2016 at 11:21 PM, Saravana Kannan wrote: On 02/02/2016 11:40 AM, Rafael J. Wysocki wrote: On Tue, Feb 2, 2016 at 6:01 PM,

Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops

On Wed, Feb 3, 2016 at 2:32 AM, Saravana Kannan wrote: > On 02/02/2016 05:07 PM, Rafael J. Wysocki wrote: >> >> On Wed, Feb 3, 2016 at 12:42 AM, Rafael J. Wysocki >> wrote: >>> >>> On Tue, Feb 2, 2016 at 11:21 PM, Saravana Kannan

Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops

On Tue, Feb 2, 2016 at 6:01 PM, Juri Lelli wrote: > Hi Rafael, > > On 02/02/16 17:35, Rafael J. Wysocki wrote: >> On Tue, Feb 2, 2016 at 4:47 PM, Juri Lelli wrote: >> > Hi Viresh, >> > >> > On 02/02/16 16:27, Viresh Kumar wrote: >> >> Until now, governors

Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops

Hi Viresh, On 02/02/16 16:27, Viresh Kumar wrote: > Until now, governors (ondemand/conservative) were using the > 'global-attr' or 'freq-attr', depending on the sysfs location where we > want to create governor's directory. > > The problem is that, in case of 'freq-attr', we are forced to use >

Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops

Hi Rafael, On 02/02/16 17:35, Rafael J. Wysocki wrote: > On Tue, Feb 2, 2016 at 4:47 PM, Juri Lelli wrote: > > Hi Viresh, > > > > On 02/02/16 16:27, Viresh Kumar wrote: > >> Until now, governors (ondemand/conservative) were using the > >> 'global-attr' or 'freq-attr',

[PATCH 2/5] cpufreq: governor: Create separate sysfs-ops

Until now, governors (ondemand/conservative) were using the 'global-attr' or 'freq-attr', depending on the sysfs location where we want to create governor's directory. The problem is that, in case of 'freq-attr', we are forced to use show()/store() present in cpufreq.c, which always take

Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops

On Tue, Feb 2, 2016 at 4:47 PM, Juri Lelli wrote: > Hi Viresh, > > On 02/02/16 16:27, Viresh Kumar wrote: >> Until now, governors (ondemand/conservative) were using the >> 'global-attr' or 'freq-attr', depending on the sysfs location where we >> want to create governor's

Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops

On 02-02-16, 17:01, Juri Lelli wrote: > Hi Rafael, > > On 02/02/16 17:35, Rafael J. Wysocki wrote: > > On Tue, Feb 2, 2016 at 4:47 PM, Juri Lelli wrote: > > > This patch cleans things up a lot, that's good. > > > > > > One thing I'm still concerned about, though: don't we

Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops

On 02-02-16, 14:21, Saravana Kannan wrote: > I also don't like this patch because it forces governors to either implement > their own macros and management of their attributes or force them to use the > governor structs that come with cpufreq_governor.h. cpufreq_governor.h IMHO > is very ondemand

Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops

On 02-02-16, 17:32, Saravana Kannan wrote: > But if we are expecting sched dvfs to come in, why make it worse for it. It > would be completely pointless to try and shoehorn sched dvfs to use > cpufreq_governor.c We can move the common part to cpufreq core and not make sched-dvfs reuse

Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops

On 02-02-16, 20:03, Saravana Kannan wrote: > This is the hotplug case I mentioned. The sysfs file removals will happen > only for the last CPU in that policy (we thankfully optimized that part last > year). We also know that multiple CPUs can't be hotplugged at the same time. > So, in the start of

Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops

On 02-02-16, 22:23, Rafael J. Wysocki wrote: > On Tue, Feb 2, 2016 at 11:57 AM, Viresh Kumar wrote: > "The ondemand and conservative governors use the global-attr or > freq-attr structures to represent sysfs attributes corresponding to > their tunables > (which of them

Re: [PATCH 2/5] cpufreq: governor: Create separate sysfs-ops

On 02/02/2016 05:52 PM, Rafael J. Wysocki wrote: On Wed, Feb 3, 2016 at 2:32 AM, Saravana Kannan wrote: On 02/02/2016 05:07 PM, Rafael J. Wysocki wrote: On Wed, Feb 3, 2016 at 12:42 AM, Rafael J. Wysocki wrote: On Tue, Feb 2, 2016 at 11:21 PM,