From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753858AbcFPIoT (ORCPT ); Thu, 16 Jun 2016 04:44:19 -0400 Received: from mailout3.w1.samsung.com ([210.118.77.13]:18103 "EHLO mailout3.w1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750757AbcFPIoN (ORCPT ); Thu, 16 Jun 2016 04:44:13 -0400 X-AuditID: cbfec7f5-f792a6d000001302-09-576266d91d48 Subject: Re: [PATCH v2] power_supply: fix return value of get_property To: Rhyland Klein , Sebastian Reichel , Dmitry Eremin-Solenikov , David Woodhouse References: <1466028784-24459-1-git-send-email-rklein@nvidia.com> Cc: linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org From: Krzysztof Kozlowski X-Enigmail-Draft-Status: N1110 Message-id: <576266A9.9030906@samsung.com> Date: Thu, 16 Jun 2016 10:43:21 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.8.0 MIME-version: 1.0 In-reply-to: <1466028784-24459-1-git-send-email-rklein@nvidia.com> Content-type: text/plain; charset=windows-1252 Content-transfer-encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFnrGLMWRmVeSWpSXmKPExsVy+t/xy7o305LCDTa+UrKY9OQ9s8XElZOZ LV6/MLS4vGsOm8Xn3iOMFsuOr2a1OL27xIHdY+esu+wem1doeWxa1cnm0dv8js2jb8sqRo/P m+QC2KK4bFJSczLLUov07RK4Mrauki74J1gx+ekBxgbGI3xdjJwcEgImEk/2HGOEsMUkLtxb zwZiCwksZZSY3eAMYT9jlGhZzQ9iCwu4SfzZ2MbcxcjFISKwhFHi545GRogiR4kXKz6zgtjM AlYSrz92s4PYbALGEpuXL2GDWCAn0ds9iQXE5hXQktjz8gdYDYuAqsSibyvBekUFIiRmbf/B BFEjKPFj8j2wek4BJ4mFJxuA6jmA5utJ3L+oBbFKXmLzmrfMExgFZyHpmIVQNQtJ1QJG5lWM oqmlyQXFSem5RnrFibnFpXnpesn5uZsYIaH/dQfj0mNWhxgFOBiVeHgF1ieGC7EmlhVX5h5i lOBgVhLhTU9NChfiTUmsrEotyo8vKs1JLT7EKM3BoiTOO3PX+xAhgfTEktTs1NSC1CKYLBMH p1QDY4e0HsPEWX4nV6x6eTOr0SX2yZJfyYcudC1dxOt+cPqsmTPtd8ZH1otETje/1ym7XeHh ruCgwiVdzMp7J7/Y5Kw6fcuvTd+eZri/S/20x+TGoptHJHJKrrVF/2fecvko3z+9K0Hdi+oj 9vVfv7TY8c0D4x+KXhqBEn4PC/p8Vn6bd/6/UJGWkL0SS3FGoqEWc1FxIgAVuvkfeQIAAA== Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 06/16/2016 12:13 AM, Rhyland Klein wrote: > power_supply_get_property() should ideally return -EAGAIN if it is > called while the power_supply is being registered. There was no way > previously to determine if use_cnt == 0 meant that the power_supply > wasn't fully registered yet, or if it had already been unregistered. > > Add a new boolean to the power_supply struct to simply show if > registration is completed. Lastly, modify the check in > power_supply_show_property() to also ignore -EAGAIN when so it > doesn't complain about not returning the property. > > Signed-off-by: Rhyland Klein > --- > v2: > - Modify power_supply_show_property() to not complain if it > sees a return value of -EAGAIN after calling > power_supply_get_property(). > > drivers/power/power_supply_core.c | 6 +++++- > drivers/power/power_supply_sysfs.c | 2 +- > include/linux/power_supply.h | 1 + > 3 files changed, 7 insertions(+), 2 deletions(-) I don't like it for two reasons: 1. There is still a short window when the information will be inaccurate. See comment below. 2. Although the code is not very complicated but it adds another field and some checks just for differentiating EAGAIN/ENODEV. It is unnecessary complexity. > > diff --git a/drivers/power/power_supply_core.c b/drivers/power/power_supply_core.c > index b13cd074c52a..a39a47672979 100644 > --- a/drivers/power/power_supply_core.c > +++ b/drivers/power/power_supply_core.c > @@ -491,8 +491,11 @@ int power_supply_get_property(struct power_supply *psy, > enum power_supply_property psp, > union power_supply_propval *val) > { > - if (atomic_read(&psy->use_cnt) <= 0) > + if (atomic_read(&psy->use_cnt) <= 0) { > + if (!psy->initialized) > + return -EAGAIN; > return -ENODEV; > + } > > return psy->desc->get_property(psy, psp, val); > } > @@ -775,6 +778,7 @@ __power_supply_register(struct device *parent, > if (rc) > goto create_triggers_failed; > > + psy->initialized = true; If someone calls power_supply_get_property() here, then ENODEV will be returned which is wrong. The problem is not solved entirely... I am not convinced that introduced complexity is worth fixing it. Best regards, Krzysztof