1. 14 6月, 2016 1 次提交
    • R
      cpufreq: conservative: Do not use transition notifications · d352cf47
      Rafael J. Wysocki 提交于
      The conservative governor registers a transition notifier so it
      can update its internal requested_freq value if it falls out of the
      policy->min...policy->max range, but requested_freq is not really
      necessary.
      
      That value is used to track the frequency requested by the governor
      previously, but policy->cur can be used instead of it and then the
      governor will not have to worry about updating the tracked value when
      the current frequency changes independently (for example, as a result
      of min or max changes).
      
      Accodringly, drop requested_freq from struct cs_policy_dbs_info
      and modify cs_dbs_timer() to use policy->cur instead of it.
      While at it, notice that __cpufreq_driver_target() clamps its
      target_freq argument between policy->min and policy->max, so
      the callers of it don't have to do that and make additional
      changes in cs_dbs_timer() in accordance with that.
      
      After these changes the transition notifier used by the conservative
      governor is not necessary any more, so drop it, which also makes it
      possible to drop the struct cs_governor definition and simplify the
      code accordingly.
      Signed-off-by: NRafael J. Wysocki <rafael.j.wysocki@intel.com>
      Acked-by: NViresh Kumar <viresh.kumar@linaro.org>
      d352cf47
  2. 03 6月, 2016 3 次提交
    • R
      cpufreq: Drop the 'initialized' field from struct cpufreq_governor · 9a15fb2c
      Rafael J. Wysocki 提交于
      The 'initialized' field in struct cpufreq_governor is only used by
      the conservative governor (as a usage counter) and the way that
      happens is far from straightforward and arguably incorrect.
      
      Namely, the value of 'initialized' is checked by
      cpufreq_dbs_governor_init() and cpufreq_dbs_governor_exit() and
      the results of those checks are passed (as the second argument) to
      the ->init() and ->exit() callbacks in struct dbs_governor.  Those
      callbacks are only implemented by the ondemand and conservative
      governors and ondemand doesn't use their second argument at all.
      In turn, the conservative governor uses it to decide whether or not
      to either register or unregister a transition notifier.
      
      That whole mechanism is not only unnecessarily convoluted, but also
      racy, because the 'initialized' field of struct cpufreq_governor is
      updated in cpufreq_init_governor() and cpufreq_exit_governor() under
      policy->rwsem which doesn't help if one of these functions is run
      twice in parallel for different policies (which isn't impossible in
      principle), for example.
      
      Instead of it, add a proper usage counter to the conservative
      governor and update it from cs_init() and cs_exit() which is
      guaranteed to be non-racy, as those functions are only called
      under gov_dbs_data_mutex which is global.
      
      With that in place, drop the 'initialized' field from struct
      cpufreq_governor as it is not used any more.
      Signed-off-by: NRafael J. Wysocki <rafael.j.wysocki@intel.com>
      Acked-by: NViresh Kumar <viresh.kumar@linaro.org>
      9a15fb2c
    • V
      cpufreq: governor: Remove prints from allocation failures · a69d6b29
      Viresh Kumar 提交于
      These aren't required anymore as the allocation core already prints such
      messages. Remove the redundant ones.
      Signed-off-by: NViresh Kumar <viresh.kumar@linaro.org>
      Signed-off-by: NRafael J. Wysocki <rafael.j.wysocki@intel.com>
      a69d6b29
    • R
      cpufreq: governor: Get rid of governor events · e788892b
      Rafael J. Wysocki 提交于
      The design of the cpufreq governor API is not very straightforward,
      as struct cpufreq_governor provides only one callback to be invoked
      from different code paths for different purposes.  The purpose it is
      invoked for is determined by its second "event" argument, causing it
      to act as a "callback multiplexer" of sorts.
      
      Unfortunately, that leads to extra complexity in governors, some of
      which implement the ->governor() callback as a switch statement
      that simply checks the event argument and invokes a separate function
      to handle that specific event.
      
      That extra complexity can be eliminated by replacing the all-purpose
      ->governor() callback with a family of callbacks to carry out specific
      governor operations: initialization and exit, start and stop and policy
      limits updates.  That also turns out to reduce the code size too, so
      do it.
      Signed-off-by: NRafael J. Wysocki <rafael.j.wysocki@intel.com>
      Acked-by: NViresh Kumar <viresh.kumar@linaro.org>
      e788892b
  3. 02 4月, 2016 1 次提交
  4. 09 3月, 2016 21 次提交
  5. 05 2月, 2016 1 次提交
  6. 07 12月, 2015 1 次提交
  7. 26 9月, 2015 1 次提交
  8. 21 7月, 2015 2 次提交
  9. 18 7月, 2015 2 次提交
  10. 15 6月, 2015 2 次提交
    • V
      cpufreq: governor: Serialize governor callbacks · 732b6d61
      Viresh Kumar 提交于
      There are several races reported in cpufreq core around governors (only
      ondemand and conservative) by different people.
      
      There are at least two race scenarios present in governor code:
       (a) Concurrent access/updates of governor internal structures.
      
       It is possible that fields such as 'dbs_data->usage_count', etc.  are
       accessed simultaneously for different policies using same governor
       structure (i.e. CPUFREQ_HAVE_GOVERNOR_PER_POLICY flag unset). And
       because of this we can dereference bad pointers.
      
       For example consider a system with two CPUs with separate 'struct
       cpufreq_policy' instances. CPU0 governor: ondemand and CPU1: powersave.
       CPU0 switching to powersave and CPU1 to ondemand:
      	CPU0				CPU1
      
      	store*				store*
      
      	cpufreq_governor_exit()		cpufreq_governor_init()
      					dbs_data = cdata->gdbs_data;
      
      	if (!--dbs_data->usage_count)
      		kfree(dbs_data);
      
      					dbs_data->usage_count++;
      					*Bad pointer dereference*
      
       There are other races possible between EXIT and START/STOP/LIMIT as
       well. Its really complicated.
      
       (b) Switching governor state in bad sequence:
      
       For example trying to switch a governor to START state, when the
       governor is in EXIT state. There are some checks present in
       __cpufreq_governor() but they aren't sufficient as they compare events
       against 'policy->governor_enabled', where as we need to take governor's
       state into account, which can be used by multiple policies.
      
      These two issues need to be solved separately and the responsibility
      should be properly divided between cpufreq and governor core.
      
      The first problem is more about the governor core, as it needs to
      protect its structures properly. And the second problem should be fixed
      in cpufreq core instead of governor, as its all about sequence of
      events.
      
      This patch is trying to solve only the first problem.
      
      There are two types of data we need to protect,
      - 'struct common_dbs_data': No matter what, there is going to be a
        single copy of this per governor.
      - 'struct dbs_data': With CPUFREQ_HAVE_GOVERNOR_PER_POLICY flag set, we
        will have per-policy copy of this data, otherwise a single copy.
      
      Because of such complexities, the mutex present in 'struct dbs_data' is
      insufficient to solve our problem. For example we need to protect
      fetching of 'dbs_data' from different structures at the beginning of
      cpufreq_governor_dbs(), to make sure it isn't currently being updated.
      
      This can be fixed if we can guarantee serialization of event parsing
      code for an individual governor. This is best solved with a mutex per
      governor, and the placeholder for that is 'struct common_dbs_data'.
      
      And so this patch moves the mutex from 'struct dbs_data' to 'struct
      common_dbs_data' and takes it at the beginning and drops it at the end
      of cpufreq_governor_dbs().
      
      Tested with and without following configuration options:
      
      CONFIG_LOCKDEP_SUPPORT=y
      CONFIG_DEBUG_RT_MUTEXES=y
      CONFIG_DEBUG_PI_LIST=y
      CONFIG_DEBUG_SPINLOCK=y
      CONFIG_DEBUG_MUTEXES=y
      CONFIG_DEBUG_LOCK_ALLOC=y
      CONFIG_PROVE_LOCKING=y
      CONFIG_LOCKDEP=y
      CONFIG_DEBUG_ATOMIC_SLEEP=y
      Signed-off-by: NViresh Kumar <viresh.kumar@linaro.org>
      Reviewed-by: NPreeti U Murthy <preeti@linux.vnet.ibm.com>
      Signed-off-by: NRafael J. Wysocki <rafael.j.wysocki@intel.com>
      732b6d61
    • V
      cpufreq: governor: register notifier from cs_init() · 8e0484d2
      Viresh Kumar 提交于
      Notifiers are required only for conservative governor and the common
      governor code is unnecessarily polluted with that. Handle that from
      cs_init/exit() instead of cpufreq_governor_dbs().
      Signed-off-by: NViresh Kumar <viresh.kumar@linaro.org>
      Reviewed-by: NPreeti U Murthy <preeti@linux.vnet.ibm.com>
      Signed-off-by: NRafael J. Wysocki <rafael.j.wysocki@intel.com>
      8e0484d2
  11. 13 11月, 2013 1 次提交
  12. 08 11月, 2013 1 次提交
  13. 29 8月, 2013 1 次提交
  14. 08 8月, 2013 2 次提交