linux-kernel - Re: [PATCH v2 0/2] Avoid regmap debugfs collisions in qcom llcc driver

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: <CAE=gft6SmWH3-Td-mZZPn-3=EzwexEdYTR00z5NCP-X1sspihA@mail.gmail.com>
Date:   Wed, 9 Oct 2019 09:01:22 -0700
From:   Evan Green <evgreen@...omium.org>
To:     Stephen Boyd <swboyd@...omium.org>
Cc:     Bjorn Andersson <bjorn.andersson@...aro.org>,
        Andy Gross <agross@...nel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        linux-arm-msm <linux-arm-msm@...r.kernel.org>,
        Venkata Narendra Kumar Gutta <vnkgutta@...eaurora.org>
Subject: Re: [PATCH v2 0/2] Avoid regmap debugfs collisions in qcom llcc driver

On Tue, Oct 8, 2019 at 6:58 PM Stephen Boyd <swboyd@...omium.org> wrote:
>
> Quoting Bjorn Andersson (2019-10-08 16:55:04)
> > On Tue 08 Oct 16:45 PDT 2019, Stephen Boyd wrote:
> > >     @@ drivers/soc/qcom/llcc-slice.c
> > >
> > >       static struct llcc_drv_data *drv_data = (void *) -EPROBE_DEFER;
> > >
> > >     --static const struct regmap_config llcc_regmap_config = {
> > >     +-static struct regmap_config llcc_regmap_config = {
> > >      -        .reg_bits = 32,
> > >      -        .reg_stride = 4,
> > >      -        .val_bits = 32,
> > >     @@ drivers/soc/qcom/llcc-slice.c: static struct regmap *qcom_llcc_init_mmio(struct
> > >       {
> > >               struct resource *res;
> > >               void __iomem *base;
> > >     -+        static struct regmap_config llcc_regmap_config = {
> > >     ++        struct regmap_config llcc_regmap_config = {
> >
> > Now that this isn't static I like the end result better. Not sure about
> > the need for splitting it in two patches, but if Evan is happy I'll take
> > it.
> >
>
> Well I split it into bug fix and micro-optimization so backport choices
> can be made. But yeah, I hope Evan is happy enough to provide a
> reviewed-by tag!

It's definitely better without the static local since it no longer has
the cognitive trap, but I still don't really get why we're messing
with the global v. local aspect of it. We're now inconsistent with
every other caller of this function, and for what exactly? We've
traded some data space for a call to memset() and some instructions. I
would have thought anecdotally that memory was the cheaper thing (ie
cpu speeds stopped increasing awhile ago, but memory is still getting
cheaper).

But either way it's correct, so really it's fine if you ignore me :)
-Evan