All of lore.kernel.org
 help / color / mirror / Atom feed
From: Nathan Zimmer <nzimmer@sgi.com>
To: "Rafael J. Wysocki" <rjw@sisk.pl>
Cc: paulmck@linux.vnet.ibm.com,
	Viresh Kumar <viresh.kumar@linaro.org>,
	cpufreq@vger.kernel.org, linux-pm@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH linux-next v8] cpufreq: convert the cpufreq_driver to use the rcu
Date: Mon, 29 Apr 2013 16:43:40 -0500	[thread overview]
Message-ID: <517EE98C.40300@sgi.com> (raw)
In-Reply-To: <2810635.tjpGLjVH1J@vostro.rjw.lan>

On 04/29/2013 04:47 PM, Rafael J. Wysocki wrote:
> On Monday, April 29, 2013 02:37:28 PM Paul E. McKenney wrote:
>> On Mon, Apr 29, 2013 at 12:22:32AM +0200, Rafael J. Wysocki wrote:
>>> On Thursday, April 04, 2013 09:57:19 PM Viresh Kumar wrote:
>>>> On 4 April 2013 20:23, Nathan Zimmer <nzimmer@sgi.com> wrote:
>>>>> We eventually would like to remove the rwlock cpufreq_driver_lock or convert
>>>>> it back to a spinlock and protect the read sections with RCU.  The first step in
>>>>> that is moving the cpufreq_driver to use the rcu.
>>>>> I don't see an easy wasy to protect the cpufreq_cpu_data structure with the
>>>>> RCU, so I am leaving it with the rwlock for now since under certain configs
>>>>> __cpufreq_cpu_get is hot spot with 256+ cores.
>>>>>
>>>>> v5: Go a different way and split up the lock and use the rcu
>>>>> v6: use bools instead of checking function pointers
>>>>>      covert the cpufreq_data_lock to a rwlock
>>>>> v7: Rebase to use the already accepted half
>>>>> v8: Correct have_governor_per_policy
>>>>>      Reviewed location of rcu_read_(un)lock in several spots
>>>> Sorry for long delay or too many versions of this patch :)
>>>>
>>>> Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
>>> Unfortunately, I had to revert this one, because it is obviously buggy.  Why?
>>> Because it adds rcu_read_lock()/rcu_read_unlock() around sysfs_create_file()
>>> which may sleep due to a GFP_KERNEL memory allocation.  Sorry for failing to
>>> notice that earlier.
>> One workaround might be to use SRCU, which allows sleeping in its
>> critical sections.
> Agreed, but at this point of the cycle I'd just preferred to do the revert and
> start over.
>
> Thanks,
> Rafael
>
>
Agreed, I think that would be cleanest.  I probably won't have time to 
get to it this week though.

Nate


WARNING: multiple messages have this Message-ID (diff)
From: Nathan Zimmer <nzimmer@sgi.com>
To: "Rafael J. Wysocki" <rjw@sisk.pl>
Cc: <paulmck@linux.vnet.ibm.com>,
	Viresh Kumar <viresh.kumar@linaro.org>, <cpufreq@vger.kernel.org>,
	<linux-pm@vger.kernel.org>, <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH linux-next v8] cpufreq: convert the cpufreq_driver to use the rcu
Date: Mon, 29 Apr 2013 16:43:40 -0500	[thread overview]
Message-ID: <517EE98C.40300@sgi.com> (raw)
In-Reply-To: <2810635.tjpGLjVH1J@vostro.rjw.lan>

On 04/29/2013 04:47 PM, Rafael J. Wysocki wrote:
> On Monday, April 29, 2013 02:37:28 PM Paul E. McKenney wrote:
>> On Mon, Apr 29, 2013 at 12:22:32AM +0200, Rafael J. Wysocki wrote:
>>> On Thursday, April 04, 2013 09:57:19 PM Viresh Kumar wrote:
>>>> On 4 April 2013 20:23, Nathan Zimmer <nzimmer@sgi.com> wrote:
>>>>> We eventually would like to remove the rwlock cpufreq_driver_lock or convert
>>>>> it back to a spinlock and protect the read sections with RCU.  The first step in
>>>>> that is moving the cpufreq_driver to use the rcu.
>>>>> I don't see an easy wasy to protect the cpufreq_cpu_data structure with the
>>>>> RCU, so I am leaving it with the rwlock for now since under certain configs
>>>>> __cpufreq_cpu_get is hot spot with 256+ cores.
>>>>>
>>>>> v5: Go a different way and split up the lock and use the rcu
>>>>> v6: use bools instead of checking function pointers
>>>>>      covert the cpufreq_data_lock to a rwlock
>>>>> v7: Rebase to use the already accepted half
>>>>> v8: Correct have_governor_per_policy
>>>>>      Reviewed location of rcu_read_(un)lock in several spots
>>>> Sorry for long delay or too many versions of this patch :)
>>>>
>>>> Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
>>> Unfortunately, I had to revert this one, because it is obviously buggy.  Why?
>>> Because it adds rcu_read_lock()/rcu_read_unlock() around sysfs_create_file()
>>> which may sleep due to a GFP_KERNEL memory allocation.  Sorry for failing to
>>> notice that earlier.
>> One workaround might be to use SRCU, which allows sleeping in its
>> critical sections.
> Agreed, but at this point of the cycle I'd just preferred to do the revert and
> start over.
>
> Thanks,
> Rafael
>
>
Agreed, I think that would be cleanest.  I probably won't have time to 
get to it this week though.

Nate


  reply	other threads:[~2013-04-29 21:43 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <AKohpo=Um9LGhGtMB0z9C2jxmS3oHwV7i_Gmsq5rR80ZcGYyLw@mail.gmail.com>
2013-04-03 15:03 ` [PATCH] cpufreq: convert the cpufreq_driver to use the rcu Nathan Zimmer
2013-04-03 15:32   ` Viresh Kumar
2013-04-03 16:37     ` Nathan Zimmer
2013-04-03 16:37       ` Nathan Zimmer
2013-04-04 14:53       ` [PATCH linux-next v8] " Nathan Zimmer
2013-04-04 16:27         ` Viresh Kumar
2013-04-28 22:22           ` Rafael J. Wysocki
2013-04-29 21:37             ` Paul E. McKenney
2013-04-29 21:47               ` Rafael J. Wysocki
2013-04-29 21:43                 ` Nathan Zimmer [this message]
2013-04-29 21:43                   ` Nathan Zimmer

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=517EE98C.40300@sgi.com \
    --to=nzimmer@sgi.com \
    --cc=cpufreq@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=paulmck@linux.vnet.ibm.com \
    --cc=rjw@sisk.pl \
    --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.