linux-kernel - Re: [PATCH] mmc: sdhci: Use "mmc" directly rather than "host->mmc"

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]

Date:   Thu, 11 Mar 2021 11:57:03 +0100
From:   Ulf Hansson <ulf.hansson@...aro.org>
To:     Jisheng Zhang <Jisheng.Zhang@...aptics.com>
Cc:     Adrian Hunter <adrian.hunter@...el.com>,
        "linux-mmc@...r.kernel.org" <linux-mmc@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] mmc: sdhci: Use "mmc" directly rather than "host->mmc"

On Thu, 11 Mar 2021 at 10:40, Jisheng Zhang <Jisheng.Zhang@...aptics.com> wrote:
>
> Clean up the code to use the "mmc" directly instead of "host->mmc".
> If the code sits in hot code path, this clean up also brings trvial
> performance improvement. Take the sdhci_post_req() for example:
>
> before the patch:
>      ...
>      8d0:       a9be7bfd        stp     x29, x30, [sp, #-32]!
>      8d4:       910003fd        mov     x29, sp
>      8d8:       f9000bf3        str     x19, [sp, #16]
>      8dc:       f9400833        ldr     x19, [x1, #16]
>      8e0:       b9404261        ldr     w1, [x19, #64]
>      8e4:       34000161        cbz     w1, 910 <sdhci_post_req+0x50>
>      8e8:       f9424400        ldr     x0, [x0, #1160]
>      8ec:       d2800004        mov     x4, #0x0                        // #0
>      8f0:       b9401a61        ldr     w1, [x19, #24]
>      8f4:       b9403262        ldr     w2, [x19, #48]
>      8f8:       f9400000        ldr     x0, [x0]
>      8fc:       f278003f        tst     x1, #0x100
>      900:       f9401e61        ldr     x1, [x19, #56]
>      904:       1a9f17e3        cset    w3, eq  // eq = none
>      908:       11000463        add     w3, w3, #0x1
>      90c:       94000000        bl      0 <dma_unmap_sg_attrs>
>      ...
>
> After the patch:
>      ...
>      8d0:       a9be7bfd        stp     x29, x30, [sp, #-32]!
>      8d4:       910003fd        mov     x29, sp
>      8d8:       f9000bf3        str     x19, [sp, #16]
>      8dc:       f9400833        ldr     x19, [x1, #16]
>      8e0:       b9404261        ldr     w1, [x19, #64]
>      8e4:       34000141        cbz     w1, 90c <sdhci_post_req+0x4c>
>      8e8:       b9401a61        ldr     w1, [x19, #24]
>      8ec:       d2800004        mov     x4, #0x0                        // #0
>      8f0:       b9403262        ldr     w2, [x19, #48]
>      8f4:       f9400000        ldr     x0, [x0]
>      8f8:       f278003f        tst     x1, #0x100
>      8fc:       f9401e61        ldr     x1, [x19, #56]
>      900:       1a9f17e3        cset    w3, eq  // eq = none
>      904:       11000463        add     w3, w3, #0x1
>      908:       94000000        bl      0 <dma_unmap_sg_attrs>
>      ...
>
> We saved one ldr instruction: "ldr     x0, [x0, #1160]"

Nice!

Even if I think the cleanup of code makes sense alone.

>
> Signed-off-by: Jisheng Zhang <Jisheng.Zhang@...aptics.com>
> ---
>  drivers/mmc/host/sdhci.c | 33 ++++++++++++++++-----------------
>  1 file changed, 16 insertions(+), 17 deletions(-)
>

[...]

> @@ -2489,14 +2489,14 @@ void sdhci_enable_sdio_irq(struct mmc_host *mmc, int enable)
>         unsigned long flags;
>
>         if (enable)
> -               pm_runtime_get_noresume(host->mmc->parent);
> +               pm_runtime_get_noresume(mmc->parent);

Maybe use mmc_dev(mmc) instead? At least I think I would appreciate
consistency in the entire c-file, today it seems like both
"mmc->parent" and mmc_dev(mmc) are being used.

[...]

Kind regards
Uffe