Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933347AbcDLLdD (ORCPT ); Tue, 12 Apr 2016 07:33:03 -0400 Received: from down.free-electrons.com ([37.187.137.238]:33504 "EHLO mail.free-electrons.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S932307AbcDLLdA (ORCPT ); Tue, 12 Apr 2016 07:33:00 -0400 Date: Tue, 12 Apr 2016 13:32:55 +0200 From: Boris Brezillon To: Thierry Reding Cc: linux-pwm@vger.kernel.org, Mike Turquette , Stephen Boyd , linux-clk@vger.kernel.org, Mark Brown , Liam Girdwood , Kamil Debski , lm-sensors@lm-sensors.org, Jean Delvare , Guenter Roeck , Dmitry Torokhov , linux-input@vger.kernel.org, Bryan Wu , Richard Purdie , Jacek Anaszewski , linux-leds@vger.kernel.org, Maxime Ripard , Chen-Yu Tsai , linux-sunxi@googlegroups.com, Joachim Eastwood , Thomas Petazzoni , Heiko Stuebner , linux-rockchip@lists.infradead.org, Jingoo Han , Lee Jones , linux-fbdev@vger.kernel.org, Jean-Christophe Plagniol-Villard , Tomi Valkeinen , Robert Jarzmik , Alexandre Belloni , Kukjin Kim , Krzysztof Kozlowski , linux-samsung-soc@vger.kernel.org, intel-gfx@lists.freedesktop.org, Daniel Vetter , Jani Nikula , Jonathan Corbet , linux-doc@vger.kernel.org, David Airlie , Daniel Vetter , dri-devel@lists.freedesktop.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, Hartley Sweeten , Ryan Mallon , Alexander Shiyan , Milo Kim Subject: Re: [PATCH v5 04/46] pwm: get rid of pwm->lock Message-ID: <20160412133255.73cea027@bbrezillon> In-Reply-To: <20160412112246.GJ18882@ulmo.ba.sec> References: <1459368249-13241-1-git-send-email-boris.brezillon@free-electrons.com> <1459368249-13241-5-git-send-email-boris.brezillon@free-electrons.com> <20160412112246.GJ18882@ulmo.ba.sec> X-Mailer: Claws Mail 3.12.0 (GTK+ 2.24.28; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1650 Lines: 38 Hi Thierry, On Tue, 12 Apr 2016 13:22:46 +0200 Thierry Reding wrote: > On Wed, Mar 30, 2016 at 10:03:27PM +0200, Boris Brezillon wrote: > > PWM devices are not protected against concurrent accesses. The lock in > > pwm_device might let PWM users think it is, but it's actually only > > protecting the enabled state. > > > > Removing this lock should be fine as long as all PWM users are aware that > > accesses to the PWM device have to be serialized, which seems to be the > > case for all of them except the sysfs interface. > > Patch the sysfs code by adding a lock to the pwm_export struct and making > > sure it's taken for all accesses to the exported PWM device. > > > > Signed-off-by: Boris Brezillon > > --- > > drivers/pwm/core.c | 19 ++++-------------- > > drivers/pwm/sysfs.c | 57 ++++++++++++++++++++++++++++++++++++++++++----------- > > include/linux/pwm.h | 2 -- > > 3 files changed, 50 insertions(+), 28 deletions(-) > > This is a little overzealous. Only accesses that can cause races need to > be protected by the lock. All of the *_show() callbacks don't modify the > PWM device in any way, so there is no need to protect them against > concurrent accesses. This is probably true for this set of changes, but what will happen when we'll switch to the atomic API? There's no guarantee that pwm->state = *newstate is done atomically, and you may see a partially updated state when calling pwm_get_state() while another thread is calling pwm_apply_state(). -- Boris Brezillon, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com