netdev - Re: [PATCH v3 net-next 6/7] net: dsa: realtek-smi: add rtl8365mb subdriver for RTL8365MB-VC

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: <20211015174047.6552112a@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
Date:   Fri, 15 Oct 2021 17:40:47 -0700
From:   Jakub Kicinski <kuba@...nel.org>
To:     Alvin Šipraga <alvin@...s.dk>
Cc:     Linus Walleij <linus.walleij@...aro.org>,
        Andrew Lunn <andrew@...n.ch>,
        Vivien Didelot <vivien.didelot@...il.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        Vladimir Oltean <olteanv@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        Rob Herring <robh+dt@...nel.org>,
        Heiner Kallweit <hkallweit1@...il.com>,
        Russell King <linux@...linux.org.uk>,
        Alvin Šipraga <alsi@...g-olufsen.dk>,
        Michael Rasmussen <mir@...g-olufsen.dk>,
        netdev@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 net-next 6/7] net: dsa: realtek-smi: add rtl8365mb
 subdriver for RTL8365MB-VC

On Fri, 15 Oct 2021 19:10:27 +0200 Alvin Šipraga wrote:
> v2 -> v3:
>   - collect Florian's Reviewed-by
>   - move IRQ setup earlier in probe per Florian's suggestion
>   - follow Jakub's suggestion and use the standard ethtool stats API

Thanks a lot for doing this. The code LGTM, the only thing that stands
out is the use of spin_locks(). I couldn't quickly parse out what bus
this device hangs off, if it's MMIO and registers can be read without
sleeping you could potentially get rid of the delayed work to read
stats, but I think you need to switch to
regmap_read_poll_timeout_atomic() because regmap_read_poll_timeout()
itself can sleep.

If the register access sleeps (I2C, SPI, MDIO etc) you need to switch
from a spin lock to a mutex.

Either way CONFIG_DEBUG_ATOMIC_SLEEP is your friend.