linux-kernel - Re: [PATCH v3 13/16] usb: phy: tegra: Keep CPU interrupts enabled

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: <ad1a2b09-12b0-112e-1556-6faf6a01c330@gmail.com>
Date:   Thu, 2 Jan 2020 17:40:48 +0300
From:   Dmitry Osipenko <digetx@...il.com>
To:     Michał Mirosław <mirq-linux@...e.qmqm.pl>
Cc:     Rob Herring <robh+dt@...nel.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Peter Chen <Peter.Chen@....com>,
        Thierry Reding <thierry.reding@...il.com>,
        Jonathan Hunter <jonathanh@...dia.com>,
        Felipe Balbi <balbi@...nel.org>, devicetree@...r.kernel.org,
        linux-usb@...r.kernel.org, linux-tegra@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 13/16] usb: phy: tegra: Keep CPU interrupts enabled

30.12.2019 23:36, Michał Mirosław пишет:
> On Sat, Dec 28, 2019 at 11:33:55PM +0300, Dmitry Osipenko wrote:
>> There is no good reason for disabling of CPU interrupts in order to
>> protect the utmip_pad_count modification.
> 
> Since there are sleeping functions called outside of the locked sections,
> this should be a mutex instead. OTOH if the spin_lock is to protect register
> write against IRQ handler, then the patch is wrong.
> 
> [...]
>> -	spin_unlock_irqrestore(&utmip_pad_lock, flags);
>> +	spin_unlock(&utmip_pad_lock);
>>  
>>  	clk_disable_unprepare(phy->pad_clk);

Hello Michał,

This spinlock isn't for protecting from the IRQ handler, it's used
solely to protect modification of the registers that are shared by all
USB controllers.

It's possible to use mutex instead of spinlock here, but it doesn't
bring any benefits because mutex is more useful when protected code
could block for a long time due to sleep or whatever, while spinlock is
much more efficient when protected code doesn't sleep and takes no more
than dozens microseconds to execute (which is the case here).

In this particular case of the Tegra USB PHY driver, the chance of
getting a block on taking the utmip_pad_lock is zero unless USB
controller drivers will start to use asynchronous probing. So this patch
does a very minor clean-up change and nothing more.