linux-kernel - Re: [RFC PATCH v2] clk: Use a new helper in managed functions

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: <CAMuHMdW=0Qf=bdE8Vy75wySRV5wzWhgM=-vhXjc0RhLGwomF_g@mail.gmail.com>
Date:   Thu, 23 Jan 2020 11:32:32 +0100
From:   Geert Uytterhoeven <geert@...ux-m68k.org>
To:     Marc Gonzalez <marc.w.gonzalez@...e.fr>
Cc:     Arnd Bergmann <arnd@...db.de>,
        Kuninori Morimoto <kuninori.morimoto.gx@...esas.com>,
        Sudip Mukherjee <sudipm.mukherjee@...il.com>,
        Stephen Boyd <sboyd@...nel.org>,
        Michael Turquette <mturquette@...libre.com>,
        Dmitry Torokhov <dmitry.torokhov@...il.com>,
        LKML <linux-kernel@...r.kernel.org>,
        Russell King <linux@...linux.org.uk>,
        Guenter Roeck <linux@...ck-us.net>,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        Robin Murphy <robin.murphy@....com>,
        linux-clk <linux-clk@...r.kernel.org>,
        Linux ARM <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [RFC PATCH v2] clk: Use a new helper in managed functions

Hi Marc,

On Thu, Jan 23, 2020 at 11:13 AM Marc Gonzalez <marc.w.gonzalez@...e.fr> wrote:
> On 22/01/2020 14:33, Geert Uytterhoeven wrote:
> > On Wed, Jan 22, 2020 at 2:02 PM Marc Gonzalez wrote:
> >> Introduce devm_add() to factorize devres_alloc/devres_add calls.
> >>
> >> Using that helper produces simpler code and smaller object size:
> >>
> >> 1 file changed, 27 insertions(+), 66 deletions(-)
> >>
> >>     text           data     bss     dec     hex filename
> >> -   1708             80       0    1788     6fc drivers/clk/clk-devres.o
> >> +   1508             80       0    1588     634 drivers/clk/clk-devres.o
> >>
> >> Signed-off-by: Marc Gonzalez <marc.w.gonzalez@...e.fr>

> >> --- a/drivers/base/devres.c
> >> +++ b/drivers/base/devres.c
> >> @@ -685,6 +685,20 @@ int devres_release_group(struct device *dev, void *id)
> >>  }
> >>  EXPORT_SYMBOL_GPL(devres_release_group);
> >>
> >> +void *devm_add(struct device *dev, dr_release_t func, void *arg, size_t size)
> >
> > Is there any advantage of using dr_release_t over "void (*action)(void *)",
> > like devm_add_action() does?  The latter lacks the "device *" parameter.
>
> (I did forget to mention that v1 used devm_add_action.)
> https://patchwork.kernel.org/patch/11262685/
>
> A limitation of devm_add_action is that it stores the void *data argument "as is".
> Users cannot pass the address of a struct on the stack. devm_add() addresses that
> specific use-case, while being a minimal wrapper around devres_alloc + devres_add.
> (devm_add_action adds an extra level of indirection.)

I didn't mean the advantage of devm_add() over devm_add_action(),
but the advantage of dr_release_t, which has a device pointer.

> >> +{
> >> +       void *data = devres_alloc(func, size, GFP_KERNEL);
> >> +
> >> +       if (data) {
> >> +               memcpy(data, arg, size);
> >> +               devres_add(dev, data);
> >> +       } else
> >> +               func(dev, arg);
> >> +
> >> +       return data;
> >
> > Why return data or NULL, instead of 0 or -Efoo, like devm_add_action()?
>
> My intent is to make devm_add a minimal wrapper (it even started out as
> a macro). As such, I just transparently pass the result of devres_alloc.
>
> Do you see an advantage in processing the result?

There are actually two questions to consider here:
  1. Is there a use case for returning the data pointer?
     I.e. will the caller ever use it?
  2. Can there be another failure mode than out-of-memory?
     Changing from NULL to ERR_PTR() later means that all callers
     need to be updated.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@...ux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds