Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756546Ab2EVOAQ (ORCPT ); Tue, 22 May 2012 10:00:16 -0400 Received: from hqemgate04.nvidia.com ([216.228.121.35]:18892 "EHLO hqemgate04.nvidia.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753047Ab2EVOAO (ORCPT ); Tue, 22 May 2012 10:00:14 -0400 X-PGP-Universal: processed; by hqnvupgp06.nvidia.com on Tue, 22 May 2012 06:58:45 -0700 Date: Tue, 22 May 2012 16:58:13 +0300 From: Peter De Schrijver To: Saravana Kannan CC: Mike Turquette , Arnd Bergman , "linux-arm-kernel@lists.infradead.org" , Paul Walmsley , Sascha Hauer , Mark Brown , Shawn Guo , Stephen Warren , Andrew Lunn , Russell King , Linus Walleij , Stephen Boyd , "linux-arm-msm@vger.kernel.org" , Magnus Damm , "linux-kernel@vger.kernel.org" , Rob Herring , Richard Zhao , Grant Likely , Deepak Saxena , Amit Kucheria , Jamie Iles , Jeremy Kerr , Thomas Gleixner Subject: Re: [PATCH] clk: Fix race conditions between clk_set_parent() and clk_enable() Message-ID: <20120522135813.GL8730@tbergstrom-lnx.Nvidia.com> References: <1336798797-8724-1-git-send-email-skannan@codeaurora.org> <4FB29E7C.7010606@codeaurora.org> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: <4FB29E7C.7010606@codeaurora.org> User-Agent: Mutt/1.5.20 (2009-06-14) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1777 Lines: 43 On Tue, May 15, 2012 at 08:20:44PM +0200, Saravana Kannan wrote: > On 05/11/2012 09:59 PM, Saravana Kannan wrote: > > Without this patch, the following race conditions are possible. > > > > Race condition 1: > > * clk-A has two parents - clk-X and clk-Y. > > * All three are disabled and clk-X is current parent. > > * Thread A: clk_set_parent(clk-A, clk-Y). > > * Thread A: > > * Thread A: Grabs enable lock. > > * Thread A: Sees enable count of clk-A is 0, so doesn't enable clk-Y. > > * Thread A: Releases enable lock. > > * Thread B: Calls clk_enable(clk-A), which in turn enables clk-X. > > * Thread A: Switches clk-A's parent to clk-Y in hardware. > > > > clk-A is now enabled in software, but not clocking in hardware. > > > > Race condition 2: > > * clk-A has two parents - clk-X and clk-Y. > > * All three are disabled and clk-X is current parent. > > * Thread A: clk_set_parent(clk-A, clk-Y). > > * Thread A: > > * Thread A: Switches parent in hardware to clk-Y. > > * Thread A: Grabs enable lock. > > * Thread A: Sees enable count of clk-A is 0, so doesn't disable clk-X. > > * Thread A: Releases enable lock. > > * Thread B: Calls clk_enable(clk-A) > > * Thread B: Software state still says parent is clk-X. > > * Thread B: So, enables clk-X and then itself. > > * Thread A: Updates parent in software state to clk-Y. > > This looks correct to me. Is there any usecase where enabling/disabling a clock would require sleeping but changing the parent would not? Cheers, Peter. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/