All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Rafael J. Wysocki" <rjw@sisk.pl>
To: linux-arm-kernel@lists.infradead.org
Subject: Re: "cpufreq: fix serialization issues with freq change notifiers" breaks cpufreq too
Date: Mon, 09 Sep 2013 23:12:49 +0000	[thread overview]
Message-ID: <2339390.oQAxMuU2Y5@vostro.rjw.lan> (raw)
In-Reply-To: <Pine.LNX.4.64.1309092330560.12376@axis700.grange>

On Monday, September 09, 2013 11:42:41 PM Guennadi Liakhovetski wrote:
> Hi Rafael
> 
> On Mon, 9 Sep 2013, Rafael J. Wysocki wrote:
> 
> > Hi,
> > 
> > On Monday, September 09, 2013 05:11:10 PM Guennadi Liakhovetski wrote:
> > > Sorry guys, I'm trying my best to stop this patch from propagating to 
> > > stable and to get it fixed asap, so, the CC list might be a bit excessive. 
> > > Also trying to fix the originally spare cc list, which makes it impossible 
> > > for me to reply to the original thread, instead have to start a new one.
> > 
> > I'm not sure what you're talking about.  What exactly was wrong with the
> > original CC list in particular?
> 
> I think you advised once to cc cpufreq related mails to linux-pm too at 
> least.

Yes, I did.

> I haven't found this patch in my pm archive, have I missed it there?

Quite frankly, I don't remember if it was there.  ISTR having it it patchwork,
which would mean that it was there, but well.

> > > Commit
> > > 
> > > commit dceff5ce18801dddc220d6238628619c93bc3cb6
> > > Author: Viresh Kumar <viresh.kumar@linaro.org>
> > > Date:   Sun Sep 1 22:19:37 2013 +0530
> > > 
> > >     cpufreq: fix serialization issues with freq change notifiers
> > > 
> > > breaks .transition_ongoing counting.
> > 
> > Do you know how exactly it breaks that?  If so, care to share that knowledge?
> 
> No, I don't. I only know that in __cpufreq_driver_target() the check for
> 
> 	if (policy->transition_ongoing) {
> 		write_unlock_irqrestore(&cpufreq_driver_lock, flags);
> 		return -EBUSY;
> 	}
> 
> is failing with this patch and cpufreq-cpu0.

OK, we need to figure out that, then.

> > > This leads to cpufreq-cpu0 not working any more. In particular switching the
> > > governor from performance to powersave directly after boot doesn't result in
> > > a frequency switch any more. Reverting this patch fixes the problem again.
> > 
> > However, this is a regression fix, so I'd prefer to fix the problem on top of
> > it instead of reverting this commit entirely.
> 
> If I understood correctly, this patch fixed some warnings, that, however, 
> didn't disrupt functionality, is this right? Whereas the patch really 
> seems to break working set ups.

It fixed warnings that indicated problems and those problems should rather be
avoided.

Thanks,
Rafael


WARNING: multiple messages have this Message-ID (diff)
From: "Rafael J. Wysocki" <rjw@sisk.pl>
To: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Cc: Viresh Kumar <viresh.kumar@linaro.org>, Greg KH <greg@kroah.com>,
	"Rafael J. Wysocki" <rafael.j.wysocki@intel.com>,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, linux-pm@vger.kernel.org,
	cpufreq@vger.kernel.org, linux-sh@vger.kernel.org,
	Magnus Damm <magnus.damm@gmail.com>
Subject: Re: "cpufreq: fix serialization issues with freq change notifiers" breaks cpufreq too
Date: Tue, 10 Sep 2013 01:12:49 +0200	[thread overview]
Message-ID: <2339390.oQAxMuU2Y5@vostro.rjw.lan> (raw)
In-Reply-To: <Pine.LNX.4.64.1309092330560.12376@axis700.grange>

On Monday, September 09, 2013 11:42:41 PM Guennadi Liakhovetski wrote:
> Hi Rafael
> 
> On Mon, 9 Sep 2013, Rafael J. Wysocki wrote:
> 
> > Hi,
> > 
> > On Monday, September 09, 2013 05:11:10 PM Guennadi Liakhovetski wrote:
> > > Sorry guys, I'm trying my best to stop this patch from propagating to 
> > > stable and to get it fixed asap, so, the CC list might be a bit excessive. 
> > > Also trying to fix the originally spare cc list, which makes it impossible 
> > > for me to reply to the original thread, instead have to start a new one.
> > 
> > I'm not sure what you're talking about.  What exactly was wrong with the
> > original CC list in particular?
> 
> I think you advised once to cc cpufreq related mails to linux-pm too at 
> least.

Yes, I did.

> I haven't found this patch in my pm archive, have I missed it there?

Quite frankly, I don't remember if it was there.  ISTR having it it patchwork,
which would mean that it was there, but well.

> > > Commit
> > > 
> > > commit dceff5ce18801dddc220d6238628619c93bc3cb6
> > > Author: Viresh Kumar <viresh.kumar@linaro.org>
> > > Date:   Sun Sep 1 22:19:37 2013 +0530
> > > 
> > >     cpufreq: fix serialization issues with freq change notifiers
> > > 
> > > breaks .transition_ongoing counting.
> > 
> > Do you know how exactly it breaks that?  If so, care to share that knowledge?
> 
> No, I don't. I only know that in __cpufreq_driver_target() the check for
> 
> 	if (policy->transition_ongoing) {
> 		write_unlock_irqrestore(&cpufreq_driver_lock, flags);
> 		return -EBUSY;
> 	}
> 
> is failing with this patch and cpufreq-cpu0.

OK, we need to figure out that, then.

> > > This leads to cpufreq-cpu0 not working any more. In particular switching the
> > > governor from performance to powersave directly after boot doesn't result in
> > > a frequency switch any more. Reverting this patch fixes the problem again.
> > 
> > However, this is a regression fix, so I'd prefer to fix the problem on top of
> > it instead of reverting this commit entirely.
> 
> If I understood correctly, this patch fixed some warnings, that, however, 
> didn't disrupt functionality, is this right? Whereas the patch really 
> seems to break working set ups.

It fixed warnings that indicated problems and those problems should rather be
avoided.

Thanks,
Rafael


WARNING: multiple messages have this Message-ID (diff)
From: rjw@sisk.pl (Rafael J. Wysocki)
To: linux-arm-kernel@lists.infradead.org
Subject: "cpufreq: fix serialization issues with freq change notifiers" breaks cpufreq too
Date: Tue, 10 Sep 2013 01:12:49 +0200	[thread overview]
Message-ID: <2339390.oQAxMuU2Y5@vostro.rjw.lan> (raw)
In-Reply-To: <Pine.LNX.4.64.1309092330560.12376@axis700.grange>

On Monday, September 09, 2013 11:42:41 PM Guennadi Liakhovetski wrote:
> Hi Rafael
> 
> On Mon, 9 Sep 2013, Rafael J. Wysocki wrote:
> 
> > Hi,
> > 
> > On Monday, September 09, 2013 05:11:10 PM Guennadi Liakhovetski wrote:
> > > Sorry guys, I'm trying my best to stop this patch from propagating to 
> > > stable and to get it fixed asap, so, the CC list might be a bit excessive. 
> > > Also trying to fix the originally spare cc list, which makes it impossible 
> > > for me to reply to the original thread, instead have to start a new one.
> > 
> > I'm not sure what you're talking about.  What exactly was wrong with the
> > original CC list in particular?
> 
> I think you advised once to cc cpufreq related mails to linux-pm too at 
> least.

Yes, I did.

> I haven't found this patch in my pm archive, have I missed it there?

Quite frankly, I don't remember if it was there.  ISTR having it it patchwork,
which would mean that it was there, but well.

> > > Commit
> > > 
> > > commit dceff5ce18801dddc220d6238628619c93bc3cb6
> > > Author: Viresh Kumar <viresh.kumar@linaro.org>
> > > Date:   Sun Sep 1 22:19:37 2013 +0530
> > > 
> > >     cpufreq: fix serialization issues with freq change notifiers
> > > 
> > > breaks .transition_ongoing counting.
> > 
> > Do you know how exactly it breaks that?  If so, care to share that knowledge?
> 
> No, I don't. I only know that in __cpufreq_driver_target() the check for
> 
> 	if (policy->transition_ongoing) {
> 		write_unlock_irqrestore(&cpufreq_driver_lock, flags);
> 		return -EBUSY;
> 	}
> 
> is failing with this patch and cpufreq-cpu0.

OK, we need to figure out that, then.

> > > This leads to cpufreq-cpu0 not working any more. In particular switching the
> > > governor from performance to powersave directly after boot doesn't result in
> > > a frequency switch any more. Reverting this patch fixes the problem again.
> > 
> > However, this is a regression fix, so I'd prefer to fix the problem on top of
> > it instead of reverting this commit entirely.
> 
> If I understood correctly, this patch fixed some warnings, that, however, 
> didn't disrupt functionality, is this right? Whereas the patch really 
> seems to break working set ups.

It fixed warnings that indicated problems and those problems should rather be
avoided.

Thanks,
Rafael

  reply	other threads:[~2013-09-09 23:12 UTC|newest]

Thread overview: 103+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-09-09 15:11 "cpufreq: fix serialization issues with freq change notifiers" breaks cpufreq too Guennadi Liakhovetski
2013-09-09 15:11 ` Guennadi Liakhovetski
2013-09-09 15:11 ` Guennadi Liakhovetski
2013-09-09 15:11 ` Guennadi Liakhovetski
2013-09-09 20:57 ` Rafael J. Wysocki
2013-09-09 21:08   ` Rafael J. Wysocki
2013-09-09 21:08   ` Rafael J. Wysocki
2013-09-09 21:42   ` Guennadi Liakhovetski
2013-09-09 21:42     ` Guennadi Liakhovetski
2013-09-09 21:42     ` Guennadi Liakhovetski
2013-09-09 23:12     ` Rafael J. Wysocki [this message]
2013-09-09 23:12       ` Rafael J. Wysocki
2013-09-09 23:12       ` Rafael J. Wysocki
2013-09-10  1:46       ` Rafael J. Wysocki
2013-09-10  1:46         ` Rafael J. Wysocki
2013-09-10 11:29 ` Viresh Kumar
2013-09-10 11:41   ` Viresh Kumar
2013-09-10 11:29   ` Viresh Kumar
2013-09-10 11:29   ` Viresh Kumar
2013-09-10 11:49   ` Rafael J. Wysocki
2013-09-10 11:49     ` Rafael J. Wysocki
2013-09-10 11:49     ` Rafael J. Wysocki
2013-09-10 11:49     ` Rafael J. Wysocki
2013-09-10 15:14     ` Viresh Kumar
2013-09-10 15:26       ` Viresh Kumar
2013-09-10 15:14       ` Viresh Kumar
2013-09-10 15:14       ` Viresh Kumar
2013-09-10 19:46       ` Rafael J. Wysocki
2013-09-10 19:46         ` Rafael J. Wysocki
2013-09-10 19:46         ` Rafael J. Wysocki
2013-09-10 19:46         ` Rafael J. Wysocki
2013-09-11  8:38         ` Viresh Kumar
2013-09-11  8:50           ` Viresh Kumar
2013-09-11  8:38           ` Viresh Kumar
2013-09-11  8:38           ` Viresh Kumar
2013-09-11 13:18           ` Rafael J. Wysocki
2013-09-11 13:18             ` Rafael J. Wysocki
2013-09-11 13:18             ` Rafael J. Wysocki
2013-09-11 13:18             ` Rafael J. Wysocki
2013-09-12  0:39             ` Viresh Kumar
2013-09-12  0:51               ` Viresh Kumar
2013-09-12  0:39               ` Viresh Kumar
2013-09-12  0:39               ` Viresh Kumar
2013-09-12  0:43               ` Rafael J. Wysocki
2013-09-12  0:43                 ` Rafael J. Wysocki
2013-09-12  0:43                 ` Rafael J. Wysocki
2013-09-12  0:43                 ` Rafael J. Wysocki
2013-09-12  5:36                 ` Viresh Kumar
2013-09-12  5:48                   ` Viresh Kumar
2013-09-12  5:36                   ` Viresh Kumar
2013-09-12  5:36                   ` Viresh Kumar
2013-09-12 10:50                   ` Rafael J. Wysocki
2013-09-12 11:01                     ` Rafael J. Wysocki
2013-09-12 11:01                     ` Rafael J. Wysocki
2013-09-12 11:01                     ` Rafael J. Wysocki
2013-09-12 10:52                     ` Viresh Kumar
2013-09-12 10:52                       ` Viresh Kumar
2013-09-12 10:52                       ` Viresh Kumar
2013-09-12 10:52                       ` Viresh Kumar
2013-09-10 15:12   ` Guennadi Liakhovetski
2013-09-10 15:12     ` Guennadi Liakhovetski
2013-09-10 15:12     ` Guennadi Liakhovetski
2013-09-10 15:12     ` Guennadi Liakhovetski
2013-09-10 15:26     ` Viresh Kumar
2013-09-10 15:38       ` Viresh Kumar
2013-09-10 15:26       ` Viresh Kumar
2013-09-10 15:26       ` Viresh Kumar
2013-09-10 16:22       ` Guennadi Liakhovetski
2013-09-10 16:22         ` Guennadi Liakhovetski
2013-09-10 16:22         ` Guennadi Liakhovetski
2013-09-10 16:22         ` Guennadi Liakhovetski
2013-09-10 16:34         ` Viresh Kumar
2013-09-10 16:46           ` Viresh Kumar
2013-09-10 16:34           ` Viresh Kumar
2013-09-10 16:34           ` Viresh Kumar
2013-09-10 17:07           ` Guennadi Liakhovetski
2013-09-10 17:07             ` Guennadi Liakhovetski
2013-09-10 17:07             ` Guennadi Liakhovetski
2013-09-10 17:07             ` Guennadi Liakhovetski
2013-09-11  8:06             ` Viresh Kumar
2013-09-11  8:18               ` Viresh Kumar
2013-09-11  8:06               ` Viresh Kumar
2013-09-11  8:06               ` Viresh Kumar
2013-09-11  8:15               ` Guennadi Liakhovetski
2013-09-11  8:15                 ` Guennadi Liakhovetski
2013-09-11  8:15                 ` Guennadi Liakhovetski
2013-09-11  8:15                 ` Guennadi Liakhovetski
2013-09-11  8:39                 ` Viresh Kumar
2013-09-11  8:51                   ` Viresh Kumar
2013-09-11  8:39                   ` Viresh Kumar
2013-09-11  8:39                   ` Viresh Kumar
2013-09-11 13:28                 ` Rafael J. Wysocki
2013-09-11 13:28                   ` Rafael J. Wysocki
2013-09-11 13:28                   ` Rafael J. Wysocki
2013-09-11 13:28                   ` Rafael J. Wysocki
2013-09-12  7:47               ` Guennadi Liakhovetski
2013-09-12  7:47                 ` Guennadi Liakhovetski
2013-09-12  7:47                 ` Guennadi Liakhovetski
2013-09-12  7:47                 ` Guennadi Liakhovetski
2013-09-12  7:51                 ` Viresh Kumar
2013-09-12  7:51                   ` Viresh Kumar
2013-09-12  7:51                   ` Viresh Kumar
2013-09-12  7:51                   ` Viresh Kumar

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=2339390.oQAxMuU2Y5@vostro.rjw.lan \
    --to=rjw@sisk.pl \
    --cc=linux-arm-kernel@lists.infradead.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.