linux-kernel - Re: [PATCH] drivers: Remove unused devm_*

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAAVeFuLgDRKy3a0JOoLxjZjbtbE8s5vpPV+HPe9A-Z2ua8xTGQ@mail.gmail.com>
Date:	Mon, 6 Jan 2014 16:48:39 +0900
From:	Alexandre Courbot <gnurou@...il.com>
To:	Josh Triplett <josh@...htriplett.org>
Cc:	Linus Walleij <linus.walleij@...aro.org>,
	Rashika Kheria <rashika.kheria@...il.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Kishon Vijay Abraham I <kishon@...com>,
	Thierry Reding <thierry.reding@...il.com>,
	Alexandre Courbot <acourbot@...dia.com>,
	"linux-gpio@...r.kernel.org" <linux-gpio@...r.kernel.org>,
	"linux-pwm@...r.kernel.org" <linux-pwm@...r.kernel.org>
Subject: Re: [PATCH] drivers: Remove unused devm_*_put functions

On Sat, Jan 4, 2014 at 5:22 AM, Josh Triplett <josh@...htriplett.org> wrote:
> On Fri, Jan 03, 2014 at 08:07:22PM +0900, Alexandre Courbot wrote:
>> On Thu, Jan 2, 2014 at 10:07 PM, Linus Walleij <linus.walleij@...aro.org> wrote:
>> > On Sat, Dec 21, 2013 at 11:49 AM, Rashika Kheria
>> > <rashika.kheria@...il.com> wrote:
>> >> -/**
>> >> - * devm_gpiod_put - Resource-managed gpiod_put()
>> >> - * @desc:      GPIO descriptor to dispose of
>> >> - *
>> >> - * Dispose of a GPIO descriptor obtained with devm_gpiod_get() or
>> >> - * devm_gpiod_get_index(). Normally this function will not be called as the GPIO
>> >> - * will be disposed of by the resource management code.
>> >> - */
>> >> -void devm_gpiod_put(struct device *dev, struct gpio_desc *desc)
>> >> -{
>> >> -       WARN_ON(devres_release(dev, devm_gpiod_release, devm_gpiod_match,
>> >> -               &desc));
>> >> -}
>> >> -EXPORT_SYMBOL(devm_gpiod_put);
>> >
>> > Alexandre what do you think about this? Can we think of a scenario
>> > where explicit garbage collection is going to be needed or should we
>> > remove this for now?
>>
>> Sorry for the delayed reply, I quickly saw the patch and then the
>> holidays got in the way. :)
>>
>> I think all these functions should be kept. It is true that they are
>> seldomly used, and that the purpose of devm is to garbage-collect
>> resources upon driver removal, but they might (actually, probably
>> will) become needed by someone at some point in the future. One
>> example I can think of is two drivers that collaborate to share the
>> same GPIO line. If they acquire the GPIO through devm_gpiod_get() they
>> will need devm_gpiod_put() to release it so the other driver can
>> acquire it.
>>
>> On a more general note, devm_clk_put(), devm_regulator_put(),
>> devm_pinctrl_put() and probably others devm_*_put() functions are
>> actively used in the kernel, to  support the idea that a devm removal
>> function makes sense. That not all the subsystem-provided functions
>> are used by mainline drivers does not necessary mean they should be
>> removed, especially if they serve a purpose. We should keep our APIs
>> consistent and future-proof, not to mention out-of-tree drivers that
>> may use them.
>>
>> So this patch is a nack as far as I'm concerned, not only the GPIO
>> part, but the whole of it.
>
> As far as I can tell, the few calls to the devm_*_put functions I can
> see look like unnecessary calls as part of driver/device
> uninitialization, and could be removed either directly or with minor
> reworking.  That there are only 1-2 calls to each in the entire kernel
> is also quite telling.
>
> In any case, it's disappointing to have a pile of unused functions in
> the kernel on the theory that they *might* be needed; it's not like it'd
> be hard to retrieve them from git if they're ever needed.

My motivation is mainly API consistency ; and these functions are
small enough to not worry too much about them IMHO. If we *really*
want to get rid of them when they are unneeded, how about defining
them as static inline in the corresponding header file? Since they are
short and seldomly used (when used at all), this may be an acceptable
compromise.

> However, if you're insistent on keeping them, it'd be easy enough to
> provide patches to include an appropriate header with prototypes for
> them instead, which would also eliminate the warning this patch series
> set out to eliminate.

The patch did not mention any warning, unless I missed something.
Would be nice to explicitly mention it in the commit message.

Thanks,
Alex.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/