Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Fri, 16 Apr 2021 10:51:09 +0200
From:   Johan Hovold <johan@kernel.org>
To:     Erwan LE RAY <erwan.leray@foss.st.com>
Cc:     dillon min <dillon.minfei@gmail.com>,
        Greg KH <gregkh@linuxfoundation.org>, jirislaby@kernel.org,
        Maxime Coquelin <mcoquelin.stm32@gmail.com>,
        Alexandre TORGUE <alexandre.torgue@foss.st.com>,
        kernel test robot <lkp@intel.com>,
        linux-serial@vger.kernel.org,
        linux-stm32@st-md-mailman.stormreply.com,
        Linux ARM <linux-arm-kernel@lists.infradead.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        kbuild-all@lists.01.org, clang-built-linux@googlegroups.com,
        Gerald Baeza <gerald.baeza@foss.st.com>
Subject: Re: [PATCH v2] serial: stm32: optimize spin lock usage
Message-ID: <YHlP/SryZXr/nNLM@hovoldconsulting.com>
References: <1618219898-4600-1-git-send-email-dillon.minfei@gmail.com>
 <YHRGPpQ03XgBMkiy@hovoldconsulting.com>
 <CAL9mu0JF-9hy3Z_ytpEO+hzKh0D+f-0gYaUBEA0v28EOHpC80w@mail.gmail.com>
 <CAL9mu0Ke97FUZ03jvdH8Lz2qRnVY82B7tAEtjbhW97sPOVkAxQ@mail.gmail.com>
 <e17fddfb-f9b8-238f-da74-a4746f33134f@foss.st.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <e17fddfb-f9b8-238f-da74-a4746f33134f@foss.st.com>
Precedence: bulk

[ Please avoid top-posting. ]

On Thu, Apr 15, 2021 at 07:09:14PM +0200, Erwan LE RAY wrote:
> Hi Dillon,
> 
> STM32MP151 is mono-core, but both STM32MP153 and STM32MP157 are 
> dual-core (see 
> https://www.st.com/content/st_com/en/products/microcontrollers-microprocessors/stm32-arm-cortex-mpus.html).
> So your point is fully relevant, thanks.
> 
> ST already fixed the same issue in st-asc.c driver in the past (see 
> ef49ffd8), because a systematic deadlock was detected with RT kernel.

That's not the same issue. The above mentioned commit fixed an issue on
*RT* where local_irq_save() should be avoided.

> You proposed a first implementation in your patch, and a second one in 
> the discussion. It seems that your initial proposal (ie your V2 patch) 
> is the most standard one (implemented in 6 drivers). The second 
> implementation is implemented by only 1 company.
> 
> It looks that the solution is to avoid locking in the sysrq case and 
> trylock in the oops_in_progress case (see detailed analysis in 
> 677fe555cbfb1).
>
> So your initial patch looks to the right proposal, but it would be safer 
> if Greg could confirm it.

That would only fix the RT issue (and by making the sysrq one slightly
worse).

Using uart_unlock_and_check_sysrq() would address both issues.

Johan