[0/4] serial: qcom-geni: fix console shutdown hang

Message ID 20230307164405.14218-1-johan+linaro@kernel.org
Headers
Series serial: qcom-geni: fix console shutdown hang |

Message

Johan Hovold March 7, 2023, 4:44 p.m. UTC
  This series fixes some of the fallout after a recent series adding
support for DMA transfers to the Qualcomm geni serial driver.

Most importantly it fixes a hang during reboot when using a serial
console and the getty is stopped during reboot.

Doug just posted an equivalent fix here:

	https://lore.kernel.org/lkml/20230307073155.1.Iaab0159b8d268060a0e131ebb27125af4750ef99@changeid

but the commit message only mentions the regression with respect to
kgdb, which is not as widely used serial consoles generally, so I
figured I'd post my version for completeness.

Either version of that fix should address the immediate regression, but
fixing the underlying problems which have been there since the driver
was first merged is going to be a bit more involved.

The rest of the series fixes a few bugs in the new DMA support that I
found while investigating the console regression.

Johan


Johan Hovold (4):
  serial: qcom-geni: fix console shutdown hang
  serial: qcom-geni: fix DMA mapping leak on shutdown
  serial: qcom-geni: fix mapping of empty DMA buffer
  serial: qcom-geni: drop bogus uart_write_wakeup()

 drivers/tty/serial/qcom_geni_serial.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)
  

Comments

Bartosz Golaszewski March 7, 2023, 4:44 p.m. UTC | #1
On Tue, 7 Mar 2023 at 17:43, Johan Hovold <johan+linaro@kernel.org> wrote:
>
> This series fixes some of the fallout after a recent series adding
> support for DMA transfers to the Qualcomm geni serial driver.
>
> Most importantly it fixes a hang during reboot when using a serial
> console and the getty is stopped during reboot.
>
> Doug just posted an equivalent fix here:
>
>         https://lore.kernel.org/lkml/20230307073155.1.Iaab0159b8d268060a0e131ebb27125af4750ef99@changeid
>
> but the commit message only mentions the regression with respect to
> kgdb, which is not as widely used serial consoles generally, so I
> figured I'd post my version for completeness.
>
> Either version of that fix should address the immediate regression, but
> fixing the underlying problems which have been there since the driver
> was first merged is going to be a bit more involved.
>
> The rest of the series fixes a few bugs in the new DMA support that I
> found while investigating the console regression.
>
> Johan
>
>
> Johan Hovold (4):
>   serial: qcom-geni: fix console shutdown hang
>   serial: qcom-geni: fix DMA mapping leak on shutdown
>   serial: qcom-geni: fix mapping of empty DMA buffer
>   serial: qcom-geni: drop bogus uart_write_wakeup()
>
>  drivers/tty/serial/qcom_geni_serial.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
>
> --
> 2.39.2
>

Hey Johan,

Douglas and Srini beat you to these fixes but thanks!

Bart
  
Bartosz Golaszewski March 7, 2023, 4:47 p.m. UTC | #2
On Tue, 7 Mar 2023 at 17:44, Bartosz Golaszewski
<bartosz.golaszewski@linaro.org> wrote:
>
> On Tue, 7 Mar 2023 at 17:43, Johan Hovold <johan+linaro@kernel.org> wrote:
> >
> > This series fixes some of the fallout after a recent series adding
> > support for DMA transfers to the Qualcomm geni serial driver.
> >
> > Most importantly it fixes a hang during reboot when using a serial
> > console and the getty is stopped during reboot.
> >
> > Doug just posted an equivalent fix here:
> >
> >         https://lore.kernel.org/lkml/20230307073155.1.Iaab0159b8d268060a0e131ebb27125af4750ef99@changeid
> >
> > but the commit message only mentions the regression with respect to
> > kgdb, which is not as widely used serial consoles generally, so I
> > figured I'd post my version for completeness.
> >
> > Either version of that fix should address the immediate regression, but
> > fixing the underlying problems which have been there since the driver
> > was first merged is going to be a bit more involved.
> >
> > The rest of the series fixes a few bugs in the new DMA support that I
> > found while investigating the console regression.
> >
> > Johan
> >
> >
> > Johan Hovold (4):
> >   serial: qcom-geni: fix console shutdown hang
> >   serial: qcom-geni: fix DMA mapping leak on shutdown
> >   serial: qcom-geni: fix mapping of empty DMA buffer
> >   serial: qcom-geni: drop bogus uart_write_wakeup()
> >
> >  drivers/tty/serial/qcom_geni_serial.c | 11 +++++++----
> >  1 file changed, 7 insertions(+), 4 deletions(-)
> >
> > --
> > 2.39.2
> >
>
> Hey Johan,
>
> Douglas and Srini beat you to these fixes but thanks!
>
> Bart

Nevermind, I read your other message now. And also patch 3/4 looks right.

Bart
  
Johan Hovold March 7, 2023, 5:03 p.m. UTC | #3
On Tue, Mar 07, 2023 at 05:47:27PM +0100, Bartosz Golaszewski wrote:
> On Tue, 7 Mar 2023 at 17:44, Bartosz Golaszewski
> <bartosz.golaszewski@linaro.org> wrote:
> >
> > On Tue, 7 Mar 2023 at 17:43, Johan Hovold <johan+linaro@kernel.org> wrote:
> > >
> > > This series fixes some of the fallout after a recent series adding
> > > support for DMA transfers to the Qualcomm geni serial driver.
> > >
> > > Most importantly it fixes a hang during reboot when using a serial
> > > console and the getty is stopped during reboot.
> > >
> > > Doug just posted an equivalent fix here:
> > >
> > >         https://lore.kernel.org/lkml/20230307073155.1.Iaab0159b8d268060a0e131ebb27125af4750ef99@changeid
> > >
> > > but the commit message only mentions the regression with respect to
> > > kgdb, which is not as widely used serial consoles generally, so I
> > > figured I'd post my version for completeness.
> > >
> > > Either version of that fix should address the immediate regression, but
> > > fixing the underlying problems which have been there since the driver
> > > was first merged is going to be a bit more involved.
> > >
> > > The rest of the series fixes a few bugs in the new DMA support that I
> > > found while investigating the console regression.
> > >
> > > Johan
> > >
> > >
> > > Johan Hovold (4):
> > >   serial: qcom-geni: fix console shutdown hang
> > >   serial: qcom-geni: fix DMA mapping leak on shutdown
> > >   serial: qcom-geni: fix mapping of empty DMA buffer
> > >   serial: qcom-geni: drop bogus uart_write_wakeup()
> > >
> > >  drivers/tty/serial/qcom_geni_serial.c | 11 +++++++----
> > >  1 file changed, 7 insertions(+), 4 deletions(-)
> > >
> > > --
> > > 2.39.2
> > >
> >
> > Hey Johan,
> >
> > Douglas and Srini beat you to these fixes but thanks!

> Nevermind, I read your other message now. And also patch 3/4 looks right.

Heh, this hang has been in linux-next for over a month and I've
actively tried to not spend time on investigating it in the hope that
someone else would be beat me to it before I moved to 6.3-rc. :)

Obviously I may be a bit biased, but I prefer this series over the
alternate fixes as the commit messages are a bit more complete and my
version of the empty DMA buffer fix is a bit cleaner.

Johan
  
Srinivas Kandagatla March 8, 2023, 2:27 p.m. UTC | #4
On 07/03/2023 17:03, Johan Hovold wrote:
> On Tue, Mar 07, 2023 at 05:47:27PM +0100, Bartosz Golaszewski wrote:
>> On Tue, 7 Mar 2023 at 17:44, Bartosz Golaszewski
>> <bartosz.golaszewski@linaro.org> wrote:
>>>
>>> On Tue, 7 Mar 2023 at 17:43, Johan Hovold <johan+linaro@kernel.org> wrote:
>>>>
>>>> This series fixes some of the fallout after a recent series adding
>>>> support for DMA transfers to the Qualcomm geni serial driver.
>>>>
>>>> Most importantly it fixes a hang during reboot when using a serial
>>>> console and the getty is stopped during reboot.
>>>>
>>>> Doug just posted an equivalent fix here:
>>>>
>>>>          https://lore.kernel.org/lkml/20230307073155.1.Iaab0159b8d268060a0e131ebb27125af4750ef99@changeid
>>>>
>>>> but the commit message only mentions the regression with respect to
>>>> kgdb, which is not as widely used serial consoles generally, so I
>>>> figured I'd post my version for completeness.
>>>>
>>>> Either version of that fix should address the immediate regression, but
>>>> fixing the underlying problems which have been there since the driver
>>>> was first merged is going to be a bit more involved.
>>>>
>>>> The rest of the series fixes a few bugs in the new DMA support that I
>>>> found while investigating the console regression.
>>>>
>>>> Johan
>>>>
>>>>
>>>> Johan Hovold (4):
>>>>    serial: qcom-geni: fix console shutdown hang
>>>>    serial: qcom-geni: fix DMA mapping leak on shutdown
>>>>    serial: qcom-geni: fix mapping of empty DMA buffer
>>>>    serial: qcom-geni: drop bogus uart_write_wakeup()
>>>>
>>>>   drivers/tty/serial/qcom_geni_serial.c | 11 +++++++----
>>>>   1 file changed, 7 insertions(+), 4 deletions(-)
>>>>
>>>> --
>>>> 2.39.2
>>>>
>>>
>>> Hey Johan,
>>>
>>> Douglas and Srini beat you to these fixes but thanks!
> 
>> Nevermind, I read your other message now. And also patch 3/4 looks right.
> 
> Heh, this hang has been in linux-next for over a month and I've
> actively tried to not spend time on investigating it in the hope that
> someone else would be beat me to it before I moved to 6.3-rc. :)
> 
> Obviously I may be a bit biased, but I prefer this series over the
> alternate fixes as the commit messages are a bit more complete and my
> version of the empty DMA buffer fix is a bit cleaner.

I don't mind, as long as the bugs are fixed.


--srini
> 
> Johan
  
Srinivas Kandagatla March 8, 2023, 2:29 p.m. UTC | #5
On 07/03/2023 16:44, Johan Hovold wrote:
> This series fixes some of the fallout after a recent series adding
> support for DMA transfers to the Qualcomm geni serial driver.
> 
> Most importantly it fixes a hang during reboot when using a serial
> console and the getty is stopped during reboot.
> 
> Doug just posted an equivalent fix here:
> 
> 	https://lore.kernel.org/lkml/20230307073155.1.Iaab0159b8d268060a0e131ebb27125af4750ef99@changeid
> 
> but the commit message only mentions the regression with respect to
> kgdb, which is not as widely used serial consoles generally, so I
> figured I'd post my version for completeness.
> 
> Either version of that fix should address the immediate regression, but
> fixing the underlying problems which have been there since the driver
> was first merged is going to be a bit more involved.
> 
> The rest of the series fixes a few bugs in the new DMA support that I
> found while investigating the console regression.
> 
> Johan
> 
> 
> Johan Hovold (4):
>    serial: qcom-geni: fix console shutdown hang
>    serial: qcom-geni: fix DMA mapping leak on shutdown
>    serial: qcom-geni: fix mapping of empty DMA buffer
>    serial: qcom-geni: drop bogus uart_write_wakeup()
> 


Tested this series on RB5

Reviewed-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
Tested-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>



--srini
>   drivers/tty/serial/qcom_geni_serial.c | 11 +++++++----
>   1 file changed, 7 insertions(+), 4 deletions(-)
>
  
Andrew Halaney March 8, 2023, 5:24 p.m. UTC | #6
On Tue, Mar 07, 2023 at 05:44:01PM +0100, Johan Hovold wrote:
> This series fixes some of the fallout after a recent series adding
> support for DMA transfers to the Qualcomm geni serial driver.
> 
> Most importantly it fixes a hang during reboot when using a serial
> console and the getty is stopped during reboot.
> 
> Doug just posted an equivalent fix here:
> 
> 	https://lore.kernel.org/lkml/20230307073155.1.Iaab0159b8d268060a0e131ebb27125af4750ef99@changeid
> 
> but the commit message only mentions the regression with respect to
> kgdb, which is not as widely used serial consoles generally, so I
> figured I'd post my version for completeness.
> 
> Either version of that fix should address the immediate regression, but
> fixing the underlying problems which have been there since the driver
> was first merged is going to be a bit more involved.
> 
> The rest of the series fixes a few bugs in the new DMA support that I
> found while investigating the console regression.
> 
> Johan
> 
> 
> Johan Hovold (4):
>   serial: qcom-geni: fix console shutdown hang
>   serial: qcom-geni: fix DMA mapping leak on shutdown
>   serial: qcom-geni: fix mapping of empty DMA buffer
>   serial: qcom-geni: drop bogus uart_write_wakeup()
> 
>  drivers/tty/serial/qcom_geni_serial.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> -- 
> 2.39.2
> 

Realized this has been affecting me (with me blaming it on something
else prior) off and on. Thanks for the fix!

Reviewed-by: Andrew Halaney <ahalaney@redhat.com>
Tested-by: Andrew Halaney <ahalaney@redhat.com> # sa8540p-ride