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: <7d8ae4f8-8900-5a06-5b7b-d4a3aea0673e@kernel.org>
Date:   Mon, 14 Aug 2023 08:28:36 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.14.0
Subject: Re: [PATCH] serial: 8250: drop lockdep annotation from
 serial8250_clear_IER()
Content-Language: en-US
To:     John Ogness <john.ogness@linutronix.de>, gregkh@linuxfoundation.org
Cc:     linux-serial@vger.kernel.org, linux-kernel@vger.kernel.org,
        Vlastimil Babka <vbabka@suse.cz>,
        Thomas Gleixner <tglx@linutronix.de>,
        Petr Mladek <pmladek@suse.com>
References: <20230811064340.13400-1-jirislaby@kernel.org>
 <878rae175n.fsf@jogness.linutronix.de>
From:   Jiri Slaby <jirislaby@kernel.org>
In-Reply-To: <878rae175n.fsf@jogness.linutronix.de>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 14. 08. 23, 8:15, John Ogness wrote:
> On 2023-08-11, "Jiri Slaby (SUSE)" <jirislaby@kernel.org> wrote:
>> The port lock is not always held when calling serial8250_clear_IER().
>> When an oops is in progress, the lock is tried to be taken and when it
>> is not, a warning is issued:
> 
> Yes, and that is a potential deadlock. The warning is correct.

Could you elaborate on how can not-taking a lock be a potential deadlock?

>> Therefore, remove the annotation as it doesn't hold for all invocations.
> 
> ... because those invocations are broken by design.

Perhaps. But the system is crashing. Better to emit something without 
the lock rather than nothing (and wait for the lock infinitely).

>> The other option would be to make the lockdep test conditional on
>> 'oops_in_progress' or pass 'locked' from serial8250_console_write(). I
>> don't think, that is worth it.
> 
> The proper thing to do is to fix the invocation. The upcoming atomic
> console implementation for the 8250 does exactly that.

So what does it do?

> If this patch gets accepted (which it appears it will be), I will revert
> it in my series implementing the 8250 atomic console.

That's fine as soon as the warning is not a problem.

thanks,
-- 
js
suse labs