Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <f89cd87c-7d1c-d8e6-ed95-6876f0201872@igalia.com>
Date:   Thu, 1 Sep 2022 12:59:36 -0300
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.12.0
Subject: Re: [PATCH V3] firmware: google: Test spinlock on panic path to avoid
 lockups
Content-Language: en-US
To:     Greg KH <gregkh@linuxfoundation.org>, evgreen@chromium.org
Cc:     arnd@arndb.de, linux-efi@vger.kernel.org,
        linux-kernel@vger.kernel.org, kernel@gpiccoli.net, ardb@kernel.org,
        davidgow@google.com, jwerner@chromium.org,
        Petr Mladek <pmladek@suse.com>
References: <20220819155059.451674-1-gpiccoli@igalia.com>
 <YxDVPqVkdgQbAIvY@kroah.com>
From:   "Guilherme G. Piccoli" <gpiccoli@igalia.com>
In-Reply-To: <YxDVPqVkdgQbAIvY@kroah.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

+ Petr, since this was extensively discussed in the original thread [0]
and maybe he can help with the argument.

[0]
https://lore.kernel.org/lkml/20220427224924.592546-1-gpiccoli@igalia.com/


On 01/09/2022 12:52, Greg KH wrote:
> [...]

>> +	 * Perform the lock check before effectively trying
>> +	 * to acquire it on gsmi_shutdown_reason() to avoid
>> +	 * potential lockups in atomic context.
>> +	 */
>> +	if (spin_is_locked(&gsmi_dev.lock))
>> +		return NOTIFY_DONE;
>> +
> 
> What happens if the lock is grabbed right after testing for it?
> Shouldn't you use lockdep_assert_held() instead as the documentation
> says to?

How, if in this point only a single CPU (this one, executing the code)
is running?

Remember this is the panic path - before this point we disabled all
other CPUs, except this one executing the code. So, either the lock was
taken (and we bail), or it wasn't and it's safe to continue.