[PATCH 0/3] serial: 8250_dw: Fix clk-notifier/port suspend deadlock

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hans has discovered that there is a potential deadlock between the ref
clock change notifier and the port suspension procedures {see the link at
the bottom of the letter}. Indeed the deadlock is possible if the port
suspension is initiated during the ref clock rate change:

    CPU0 (suspend CPU/UART)   CPU1 (update clock)
             ----                    ----
    lock(&port->mutex);
                              lock((work_completion)(&data->clk_work));
                              lock(&port->mutex);
    lock((work_completion)(&data->clk_work));

    *** DEADLOCK ***

So the CPU performing the UART port shutdown procedure will wait until the
ref clock change notifier is finished (worker is flushed), while the later
will wait for a port mutex being released.

A possible solution to bypass the deadlock is to move the worker flush out
of the critical section protected by the TTY port mutex. For instance we
can register and de-register the clock change notifier in the port probe
and remove methods instead of having them called from the port
startup/shutdown callbacks. But in order to do that we need to make sure
that the serial8250_update_uartclk() method is safe to be used while the
port is shutted down. Alas the current implementation doesn't provide that
safety. The solution described above is introduced in the framework of
this patchset. See individual patches for details.

Link: https://lore.kernel.org/linux-serial/f1cd5c75-9cda-6896-a4e2-42c5bfc3f5c3@xxxxxxxxxx

Hans, could you test the patchset out on your Cherry Trail (x86)-based
devices? After that we can merge it in into the kernels 5.8 and 5.9 if
there is no objections against the fix.

Note, in order to have the fix working for the older kernel all of patches
need to be backported.

Fixes: cc816969d7b5 ("serial: 8250_dw: Fix common clocks usage race condition")
Fixes: 868f3ee6e452 ("serial: 8250: Add 8250 port clock update method")
Reported-by: Hans de Goede <hdegoede@xxxxxxxxxx>
Signed-off-by: Serge Semin <Sergey.Semin@xxxxxxxxxxxxxxxxxxxx>
Cc: Alexey Malahov <Alexey.Malahov@xxxxxxxxxxxxxxxxxxxx>
Cc: Pavel Parkhomenko <Pavel.Parkhomenko@xxxxxxxxxxxxxxxxxxxx>
Cc: Andy Shevchenko <andy.shevchenko@xxxxxxxxx>
Cc: Maxime Ripard <mripard@xxxxxxxxxx>
Cc: Will Deacon <will@xxxxxxxxxx>
Cc: Russell King <linux@xxxxxxxxxxxxxxx>
Cc: linux-arm-kernel@xxxxxxxxxxxxxxxxxxx
Cc: linux-serial@xxxxxxxxxxxxxxx
Cc: linux-kernel@xxxxxxxxxxxxxxx

Serge Semin (3):
  serial: 8250: Discard RTS/DTS setting from clock update method
  serial: 8250: Skip uninitialized TTY port baud rate update
  serial: 8250_dw: Fix clk-notifier/port suspend deadlock

 drivers/tty/serial/8250/8250_dw.c   | 54 ++++++++++-------------------
 drivers/tty/serial/8250/8250_port.c |  5 ++-
 2 files changed, 23 insertions(+), 36 deletions(-)

-- 
2.27.0




[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux PPP]     [Linux FS]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Linmodem]     [Device Mapper]     [Linux Kernel for ARM]

  Powered by Linux