From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-16.3 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id BFE9AC43381 for ; Sat, 13 Feb 2021 12:16:50 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 83C8164DA8 for ; Sat, 13 Feb 2021 12:16:50 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229662AbhBMMQt (ORCPT ); Sat, 13 Feb 2021 07:16:49 -0500 Received: from mail.kernel.org ([198.145.29.99]:49262 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229592AbhBMMQs (ORCPT ); Sat, 13 Feb 2021 07:16:48 -0500 Received: by mail.kernel.org (Postfix) with ESMTPSA id CAE7864DD6; Sat, 13 Feb 2021 12:16:05 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linuxfoundation.org; s=korg; t=1613218567; bh=4xFxyQXalGrldKkL8r7xY5JedLCnGKpV3hkcAB/m1wc=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=P1CMoTjyFG+N6rtSr9mS6CNFYhgNtCzBL8hhnGR+JH2R7vCweELe4lRrbbVp6RPiN en8N4dDDqTgnZnQzGSQYwxOSm6ZS746+QT/AOxTto/0mhC5yLC163HMYpnUPGWDmdT +Rd5L3vt5ecVTsaxeRi5phB99KKthyXaLvTdZ5ig= Date: Sat, 13 Feb 2021 13:16:02 +0100 From: Greg Kroah-Hartman To: Matti Vaittinen Cc: mazziesaccount@gmail.com, "Rafael J. Wysocki" , MyungJoo Ham , Chanwoo Choi , Andy Gross , Bjorn Andersson , Jean Delvare , Guenter Roeck , Hans de Goede , Mark Gross , Sebastian Reichel , Chen-Yu Tsai , Liam Girdwood , Mark Brown , Wim Van Sebroeck , Saravana Kannan , Heikki Krogerus , Andy Shevchenko , Joerg Roedel , Dan Williams , Bartosz Golaszewski , linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org, linux-hwmon@vger.kernel.org, platform-driver-x86@vger.kernel.org, linux-pm@vger.kernel.org, linux-watchdog@vger.kernel.org Subject: Re: [RFC PATCH 1/7] drivers: base: Add resource managed version of delayed work init Message-ID: References: <1230b0d2ba99ad546d72ab079e76cb1b3df32afb.1613216412.git.matti.vaittinen@fi.rohmeurope.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1230b0d2ba99ad546d72ab079e76cb1b3df32afb.1613216412.git.matti.vaittinen@fi.rohmeurope.com> Precedence: bulk List-ID: X-Mailing-List: platform-driver-x86@vger.kernel.org On Sat, Feb 13, 2021 at 01:58:44PM +0200, Matti Vaittinen wrote: > A few drivers which need a delayed work-queue must cancel work at exit. > Some of those implement remove solely for this purpose. Help drivers > to avoid unnecessary remove and error-branch implementation by adding > managed verision of delayed work initialization > > Signed-off-by: Matti Vaittinen That's not a good idea. As this would kick in when the device is removed from the system, not when it is unbound from the driver, right? > --- > drivers/base/devres.c | 33 +++++++++++++++++++++++++++++++++ > include/linux/device.h | 5 +++++ > 2 files changed, 38 insertions(+) > > diff --git a/drivers/base/devres.c b/drivers/base/devres.c > index fb9d5289a620..2879595bb5a4 100644 > --- a/drivers/base/devres.c > +++ b/drivers/base/devres.c > @@ -1231,3 +1231,36 @@ void devm_free_percpu(struct device *dev, void __percpu *pdata) > (void *)pdata)); > } > EXPORT_SYMBOL_GPL(devm_free_percpu); > + > +static void dev_delayed_work_drop(struct device *dev, void *res) > +{ > + cancel_delayed_work_sync(*(struct delayed_work **)res); > +} > + > +/** > + * devm_delayed_work_autocancel - Resource-managed work allocation > + * @dev: Device which lifetime work is bound to > + * @pdata: work to be cancelled when device exits > + * > + * Initialize work which is automatically cancelled when device exits. There is no such thing in the driver model as "when device exits". Please use the proper terminology as I do not understand what you think this is doing here... > + * A few drivers need delayed work which must be cancelled before driver > + * is unload to avoid accessing removed resources. > + * devm_delayed_work_autocancel() can be used to omit the explicit > + * cancelleation when driver is unload. > + */ > +int devm_delayed_work_autocancel(struct device *dev, struct delayed_work *w, > + void (*worker)(struct work_struct *work)) > +{ > + struct delayed_work **ptr; > + > + ptr = devres_alloc(dev_delayed_work_drop, sizeof(*ptr), GFP_KERNEL); > + if (!ptr) > + return -ENOMEM; > + > + INIT_DELAYED_WORK(w, worker); > + *ptr = w; > + devres_add(dev, ptr); > + > + return 0; > +} > +EXPORT_SYMBOL_GPL(devm_delayed_work_autocancel); > diff --git a/include/linux/device.h b/include/linux/device.h > index 1779f90eeb4c..192456198de7 100644 > --- a/include/linux/device.h > +++ b/include/linux/device.h > @@ -27,6 +27,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -249,6 +250,10 @@ void __iomem *devm_of_iomap(struct device *dev, > struct device_node *node, int index, > resource_size_t *size); > > +/* delayed work which is cancelled when driver exits */ Not when the "driver exits". There is two different lifespans here (well 3). Code and data*2. Don't confuse them as that will just cause lots of problems. The move toward more and more "devm" functions is not the way to go as they just more and more make things easier to get wrong. APIs should be impossible to get wrong, this one is going to be almost impossible to get right. thanks, greg k-h