All of lore.kernel.org
 help / color / mirror / Atom feed
From: Michael Turquette <mturquette@baylibre.com>
To: Viresh Kumar <viresh.kumar@linaro.org>,
	"Juri Lelli" <juri.lelli@arm.com>
Cc: linux-kernel@vger.kernel.org, linux-pm@vger.kernel.org,
	peterz@infradead.org, rjw@rjwysocki.net, steve.muckle@linaro.org,
	vincent.guittot@linaro.org, morten.rasmussen@arm.com,
	dietmar.eggemann@arm.com
Subject: Re: [RFC PATCH 18/19] cpufreq: remove transition_lock
Date: Tue, 12 Jan 2016 16:54:52 -0800	[thread overview]
Message-ID: <20160113005452.10884.77606@quark.deferred.io> (raw)
In-Reply-To: <20160112112409.GJ1084@ubuntu>

Hi Viresh,

Quoting Viresh Kumar (2016-01-12 03:24:09)
> On 11-01-16, 17:35, Juri Lelli wrote:
> > From: Michael Turquette <mturquette@baylibre.com>
> > 
> > transition_lock was introduced to serialize cpufreq transition
> > notifiers. Instead of using a different lock for protecting concurrent
> > modifications of policy, it is better to require that callers of
> > transition notifiers implement appropriate locking (this is already the
> > case AFAICS). Removing transition_lock also simplifies current locking
> > scheme.
> 
> So, are you saying that the reasoning mentioned in this patch are all
> wrong?
> 
> commit 12478cf0c55e ("cpufreq: Make sure frequency transitions are
> serialized")

No, that's not what I'm saying. Quoting that patch:

"""
The key challenge is to allow drivers to begin the transition from one thread
and end it in a completely different thread (this is to enable drivers that do
asynchronous POSTCHANGE notification from bottom-halves, to also use the same
interface).

To achieve this, a 'transition_ongoing' flag, a 'transition_lock' spinlock and a
wait-queue are added per-policy. The flag and the wait-queue are used in
conjunction to create an "uninterrupted flow" from _begin() to _end(). The
spinlock is used to ensure that only one such "flow" is in flight at any given
time. Put together, this provides us all the necessary synchronization.
"""

So the transition_onging flag and wait-queue are all good. That stuff is
just great. This patch doesn't touch it.

What it does change is that it removes a superfluous spinlock that
should never have needed to exist in the first place.
cpufreq_freq_transition_begin is called directly by driver target
callbacks, and it is called by __cpufreq_driver_target.

__cpufreq_driver_target should be using a per-policy lock. Any other
behavior is just insane. I haven't gone through this thread to see if
that change has been made by Juri, but we need to get there either in
this series or the follow-up series that introduces some RCU locking.

Regards,
Mike

> 
> -- 
> viresh

  reply	other threads:[~2016-01-13  1:06 UTC|newest]

Thread overview: 110+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-11 17:35 [RFC PATCH 00/19] cpufreq locking cleanups and documentation Juri Lelli
2016-01-11 17:35 ` [RFC PATCH 01/19] cpufreq: do not expose cpufreq_governor_lock Juri Lelli
2016-01-12  8:56   ` Viresh Kumar
2016-01-11 17:35 ` [RFC PATCH 02/19] cpufreq: merge governor lock and mutex Juri Lelli
2016-01-12  9:00   ` Viresh Kumar
2016-01-11 17:35 ` [RFC PATCH 03/19] cpufreq: kill for_each_policy Juri Lelli
2016-01-12  9:01   ` Viresh Kumar
2016-01-11 17:35 ` [RFC PATCH 04/19] cpufreq: bring data structures close to their locks Juri Lelli
2016-01-11 22:05   ` Peter Zijlstra
2016-01-11 23:03     ` Rafael J. Wysocki
2016-01-12  8:27       ` Peter Zijlstra
2016-01-12 10:43         ` Juri Lelli
2016-01-12 16:47         ` Rafael J. Wysocki
2016-01-11 22:07   ` Peter Zijlstra
2016-01-12  9:27     ` Viresh Kumar
2016-01-12 11:21       ` Juri Lelli
2016-01-12 11:58         ` Peter Zijlstra
2016-01-12 12:36           ` Juri Lelli
2016-01-12 15:26             ` Juri Lelli
2016-01-12 15:58               ` Peter Zijlstra
2016-01-12  9:10   ` Viresh Kumar
2016-01-11 17:35 ` [RFC PATCH 05/19] cpufreq: assert locking when accessing cpufreq_policy_list Juri Lelli
2016-01-12  9:34   ` Viresh Kumar
2016-01-12 11:44     ` Juri Lelli
2016-01-13  5:59       ` Viresh Kumar
2016-01-11 17:35 ` [RFC PATCH 06/19] cpufreq: always access cpufreq_policy_list while holding cpufreq_driver_lock Juri Lelli
2016-01-12  9:57   ` Viresh Kumar
2016-01-12 12:08     ` Juri Lelli
2016-01-13  6:01       ` Viresh Kumar
2016-01-11 17:35 ` [RFC PATCH 07/19] cpufreq: assert locking when accessing cpufreq_governor_list Juri Lelli
2016-01-12 10:01   ` Viresh Kumar
2016-01-12 15:33     ` Juri Lelli
2016-01-11 17:35 ` [RFC PATCH 08/19] cpufreq: fix warning for cpufreq_init_policy unlocked access to cpufreq_governor_list Juri Lelli
2016-01-12 10:09   ` Viresh Kumar
2016-01-12 15:52     ` Juri Lelli
2016-01-13  6:07       ` Viresh Kumar
2016-01-14 16:35         ` Juri Lelli
2016-01-18  5:23           ` Viresh Kumar
2016-01-18 15:19             ` Juri Lelli
2016-01-11 17:35 ` [RFC PATCH 09/19] cpufreq: fix warning for show_scaling_available_governors " Juri Lelli
2016-01-12 10:13   ` Viresh Kumar
2016-01-13 10:25     ` Juri Lelli
2016-01-13 10:32       ` Viresh Kumar
2016-01-11 17:35 ` [RFC PATCH 10/19] cpufreq: assert policy->rwsem is held in cpufreq_set_policy Juri Lelli
2016-01-12 10:15   ` Viresh Kumar
2016-01-11 17:35 ` [RFC PATCH 11/19] cpufreq: assert policy->rwsem is held in __cpufreq_governor Juri Lelli
2016-01-12 10:20   ` Viresh Kumar
2016-01-30  0:33     ` Saravana Kannan
2016-01-30 11:49       ` Rafael J. Wysocki
2016-02-01  6:09         ` Viresh Kumar
2016-02-01 10:22           ` Rafael J. Wysocki
2016-02-01 20:24             ` Saravana Kannan
2016-02-01 21:00               ` Rafael J. Wysocki
2016-02-02  6:36                 ` Viresh Kumar
2016-02-02 21:38                   ` Saravana Kannan
2016-02-02  6:34               ` Viresh Kumar
2016-02-02 21:37                 ` Saravana Kannan
2016-02-03  2:13                   ` Viresh Kumar
2016-02-03  4:04                     ` Saravana Kannan
2016-02-03  5:02                       ` Viresh Kumar
2016-02-03  5:06                         ` Saravana Kannan
2016-02-03  6:59                           ` Viresh Kumar
2016-01-11 17:35 ` [RFC PATCH 12/19] cpufreq: fix locking of policy->rwsem in cpufreq_init_policy Juri Lelli
2016-01-12 10:39   ` Viresh Kumar
2016-01-14 17:58     ` Juri Lelli
2016-01-11 17:35 ` [RFC PATCH 13/19] cpufreq: fix locking of policy->rwsem in cpufreq_offline_prepare Juri Lelli
2016-01-12 10:54   ` Viresh Kumar
2016-01-15 12:37     ` Juri Lelli
2016-01-11 17:35 ` [RFC PATCH 14/19] cpufreq: fix locking of policy->rwsem in cpufreq_offline_finish Juri Lelli
2016-01-12 11:02   ` Viresh Kumar
2016-01-11 17:35 ` [RFC PATCH 15/19] cpufreq: remove useless usage of cpufreq_governor_mutex in __cpufreq_governor Juri Lelli
2016-01-12 11:06   ` Viresh Kumar
2016-01-15 16:30     ` Juri Lelli
2016-01-18  5:50       ` Viresh Kumar
2016-01-19 16:49         ` Juri Lelli
2016-01-20  7:29           ` Viresh Kumar
2016-01-20 10:17             ` Juri Lelli
2016-01-20 10:18               ` Viresh Kumar
2016-01-20 10:27                 ` Juri Lelli
2016-01-20 10:30                   ` Viresh Kumar
2016-01-11 17:35 ` [RFC PATCH 16/19] cpufreq: hold policy->rwsem across CPUFREQ_GOV_POLICY_EXIT Juri Lelli
2016-01-12 11:09   ` Viresh Kumar
2016-01-11 17:35 ` [RFC PATCH 17/19] cpufreq: stop checking for cpufreq_driver being present in cpufreq_cpu_get Juri Lelli
2016-01-12 11:17   ` Viresh Kumar
2016-01-11 17:35 ` [RFC PATCH 18/19] cpufreq: remove transition_lock Juri Lelli
2016-01-12 11:24   ` Viresh Kumar
2016-01-13  0:54     ` Michael Turquette [this message]
2016-01-13  6:31       ` Viresh Kumar
     [not found]         ` <20160113182131.1168.45753@quark.deferred.io>
2016-01-14  9:44           ` Juri Lelli
2016-01-14 10:32           ` Viresh Kumar
2016-01-14 13:52             ` Juri Lelli
2016-01-18  5:09               ` Viresh Kumar
2016-01-19 14:00           ` Peter Zijlstra
2016-01-19 14:42             ` Juri Lelli
2016-01-19 15:30               ` Peter Zijlstra
2016-01-19 16:01                 ` Juri Lelli
2016-01-19 19:17                   ` Peter Zijlstra
2016-01-19 19:21                     ` Peter Zijlstra
2016-01-19 21:52                       ` Rafael J. Wysocki
2016-01-20 17:04                         ` Peter Zijlstra
2016-01-20 22:12                           ` Rafael J. Wysocki
2016-01-20 22:38                             ` Peter Zijlstra
2016-01-20 23:33                               ` Rafael J. Wysocki
2016-01-20 12:59                       ` Juri Lelli
2016-01-11 17:36 ` [RFC PATCH 19/19] cpufreq: documentation: document locking scheme Juri Lelli
2016-01-11 22:45 ` [RFC PATCH 00/19] cpufreq locking cleanups and documentation Rafael J. Wysocki
2016-01-12 10:46   ` Juri Lelli
2016-01-30  0:57 ` Saravana Kannan
2016-02-01  6:02   ` Viresh Kumar
2016-02-01 12:06   ` Juri Lelli

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160113005452.10884.77606@quark.deferred.io \
    --to=mturquette@baylibre.com \
    --cc=dietmar.eggemann@arm.com \
    --cc=juri.lelli@arm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=morten.rasmussen@arm.com \
    --cc=peterz@infradead.org \
    --cc=rjw@rjwysocki.net \
    --cc=steve.muckle@linaro.org \
    --cc=vincent.guittot@linaro.org \
    --cc=viresh.kumar@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.