From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Geert Uytterhoeven <geert+renesas@glider.be>
Cc: Jiri Slaby <jslaby@suse.com>,
Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>,
Simon Horman <horms+renesas@verge.net.au>,
Magnus Damm <magnus.damm@gmail.com>,
Yoshinori Sato <ysato@users.sourceforge.jp>,
Laurent Pinchart <Laurent.pinchart@ideasonboard.com>,
Michael Turquette <mturquette@baylibre.com>,
linux-serial@vger.kernel.org, linux-sh@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1b/1] serial: sh-sci: Remove cpufreq notifier to fix crash/deadlock
Date: Fri, 08 Jan 2016 05:09:04 +0000 [thread overview]
Message-ID: <20160108050904.GH30293@kroah.com> (raw)
In-Reply-To: <1452018997-14103-3-git-send-email-geert+renesas@glider.be>
On Tue, Jan 05, 2016 at 07:36:37PM +0100, Geert Uytterhoeven wrote:
> The BSP team noticed that there is spin/mutex lock issue on sh-sci when
> CPUFREQ is used. The issue is that the notifier function may call
> mutex_lock() while the spinlock is held, which can lead to a BUG().
> This may happen if CPUFREQ is changed while another CPU calls
> clk_get_rate().
>
> Taking the spinlock was added to the notifier function in commit
> e552de2413edad1a ("sh-sci: add platform device private data"), to
> protect the list of serial ports against modification during traversal.
> At that time the Common Clock Framework didn't exist yet, and
> clk_get_rate() just returned clk->rate without taking a mutex.
> Note that since commit d535a2305facf9b4 ("serial: sh-sci: Require a
> device per port mapping."), there's no longer a list of serial ports to
> traverse, and taking the spinlock became superfluous.
>
> To fix the issue, just remove the cpufreq notifier:
> 1. The notifier doesn't work correctly: all it does is update stored
> clock rates; it does not update the divider in the hardware.
> The divider will only be updated when calling sci_set_termios().
> I believe this was broken back in 2004, when the old
> drivers/char/sh-sci.c driver (where the notifier did update the
> divider) was replaced by drivers/serial/sh-sci.c (where the
> notifier just updated port->uartclk).
> Cfr. full-history-linux commits 6f8deaef2e9675d9 ("[PATCH] sh: port
> sh-sci driver to the new API") and 3f73fe878dc9210a ("[PATCH]
> Remove old sh-sci driver").
> 2. On modern SoCs, the sh-sci parent clock rate is no longer related
> to the CPU clock rate anyway, so using a cpufreq notifier is
> futile.
>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> This version applies against scif-clk-sck-brg-for-v4.5.
I took this version. If the 1a needs to go to 4.4-stable, email it, and
the git commit id to stable@vger.kernel.org when this ends up in Linus's
tree.
thanks,
greg k-h
WARNING: multiple messages have this Message-ID (diff)
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Geert Uytterhoeven <geert+renesas@glider.be>
Cc: Jiri Slaby <jslaby@suse.com>,
Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>,
Simon Horman <horms+renesas@verge.net.au>,
Magnus Damm <magnus.damm@gmail.com>,
Yoshinori Sato <ysato@users.sourceforge.jp>,
Laurent Pinchart <Laurent.pinchart@ideasonboard.com>,
Michael Turquette <mturquette@baylibre.com>,
linux-serial@vger.kernel.org, linux-sh@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1b/1] serial: sh-sci: Remove cpufreq notifier to fix crash/deadlock
Date: Thu, 7 Jan 2016 21:09:04 -0800 [thread overview]
Message-ID: <20160108050904.GH30293@kroah.com> (raw)
In-Reply-To: <1452018997-14103-3-git-send-email-geert+renesas@glider.be>
On Tue, Jan 05, 2016 at 07:36:37PM +0100, Geert Uytterhoeven wrote:
> The BSP team noticed that there is spin/mutex lock issue on sh-sci when
> CPUFREQ is used. The issue is that the notifier function may call
> mutex_lock() while the spinlock is held, which can lead to a BUG().
> This may happen if CPUFREQ is changed while another CPU calls
> clk_get_rate().
>
> Taking the spinlock was added to the notifier function in commit
> e552de2413edad1a ("sh-sci: add platform device private data"), to
> protect the list of serial ports against modification during traversal.
> At that time the Common Clock Framework didn't exist yet, and
> clk_get_rate() just returned clk->rate without taking a mutex.
> Note that since commit d535a2305facf9b4 ("serial: sh-sci: Require a
> device per port mapping."), there's no longer a list of serial ports to
> traverse, and taking the spinlock became superfluous.
>
> To fix the issue, just remove the cpufreq notifier:
> 1. The notifier doesn't work correctly: all it does is update stored
> clock rates; it does not update the divider in the hardware.
> The divider will only be updated when calling sci_set_termios().
> I believe this was broken back in 2004, when the old
> drivers/char/sh-sci.c driver (where the notifier did update the
> divider) was replaced by drivers/serial/sh-sci.c (where the
> notifier just updated port->uartclk).
> Cfr. full-history-linux commits 6f8deaef2e9675d9 ("[PATCH] sh: port
> sh-sci driver to the new API") and 3f73fe878dc9210a ("[PATCH]
> Remove old sh-sci driver").
> 2. On modern SoCs, the sh-sci parent clock rate is no longer related
> to the CPU clock rate anyway, so using a cpufreq notifier is
> futile.
>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> This version applies against scif-clk-sck-brg-for-v4.5.
I took this version. If the 1a needs to go to 4.4-stable, email it, and
the git commit id to stable@vger.kernel.org when this ends up in Linus's
tree.
thanks,
greg k-h
next prev parent reply other threads:[~2016-01-08 5:09 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-01-05 18:36 [PATCH 0/1] serial: sh-sci: Remove cpufreq notifier to fix crash/deadlock Geert Uytterhoeven
2016-01-05 18:36 ` Geert Uytterhoeven
2016-01-05 18:36 ` [PATCH 1a/1] " Geert Uytterhoeven
2016-01-05 18:36 ` Geert Uytterhoeven
2016-01-05 18:36 ` [PATCH 1b/1] " Geert Uytterhoeven
2016-01-05 18:36 ` Geert Uytterhoeven
2016-01-08 5:09 ` Greg Kroah-Hartman [this message]
2016-01-08 5:09 ` Greg Kroah-Hartman
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=20160108050904.GH30293@kroah.com \
--to=gregkh@linuxfoundation.org \
--cc=Laurent.pinchart@ideasonboard.com \
--cc=geert+renesas@glider.be \
--cc=horms+renesas@verge.net.au \
--cc=jslaby@suse.com \
--cc=kuninori.morimoto.gx@renesas.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-serial@vger.kernel.org \
--cc=linux-sh@vger.kernel.org \
--cc=magnus.damm@gmail.com \
--cc=mturquette@baylibre.com \
--cc=ysato@users.sourceforge.jp \
/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.