1. Patchew
  2. linux
  3. View series

[PATCH] tty/serial: Cancel work queue before closing uart

Wenhua Lin posted 1 patch 2 years, 4 months ago 
drivers/tty/serial/sprd_serial.c | 4 ++++
1 file changed, 4 insertions(+)
[PATCH] tty/serial: Cancel work queue before closing uart
Posted by Wenhua Lin 2 years, 4 months ago 
When the system constantly sleeps and wankes up, plugging and unplugging
the USB will probalility trigger a kernel crash problem. The reason is
that at this time, the system entered deep and turned off uart, and the
abnormal behavior of plugging and upplugging the USB triggered the read
data process of uart, causing access to uart to hang. The final solution
we came up with is to cancel the work queue before shutting down uart
, while ensuring that there is no uart business.

Signed-off-by: Wenhua Lin <Wenhua.Lin@unisoc.com>
---
 drivers/tty/serial/sprd_serial.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/tty/serial/sprd_serial.c b/drivers/tty/serial/sprd_serial.c
index b58f51296ace..eddff4360155 100644
--- a/drivers/tty/serial/sprd_serial.c
+++ b/drivers/tty/serial/sprd_serial.c
@@ -20,6 +20,7 @@
 #include <linux/slab.h>
 #include <linux/tty.h>
 #include <linux/tty_flip.h>
+#include "../tty.h"
 
 /* device name */
 #define UART_NR_MAX		8
@@ -1221,7 +1222,10 @@ static int sprd_probe(struct platform_device *pdev)
 static int sprd_suspend(struct device *dev)
 {
 	struct sprd_uart_port *sup = dev_get_drvdata(dev);
+	struct uart_port *uport = &sup->port;
+	struct tty_port *tty = &uport->state->port;
 
+	tty_buffer_cancel_work(tty);
 	uart_suspend_port(&sprd_uart_driver, &sup->port);
 
 	return 0;
-- 
2.17.1
Re: [PATCH] tty/serial: Cancel work queue before closing uart
Posted by Greg Kroah-Hartman 2 years, 3 months ago 
On Fri, Aug 18, 2023 at 11:15:32AM +0800, Wenhua Lin wrote:
> When the system constantly sleeps and wankes up, plugging and unplugging
> the USB will probalility trigger a kernel crash problem. The reason is
> that at this time, the system entered deep and turned off uart, and the
> abnormal behavior of plugging and upplugging the USB triggered the read
> data process of uart, causing access to uart to hang. The final solution
> we came up with is to cancel the work queue before shutting down uart
> , while ensuring that there is no uart business.
> 
> Signed-off-by: Wenhua Lin <Wenhua.Lin@unisoc.com>
> ---
>  drivers/tty/serial/sprd_serial.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/tty/serial/sprd_serial.c b/drivers/tty/serial/sprd_serial.c
> index b58f51296ace..eddff4360155 100644
> --- a/drivers/tty/serial/sprd_serial.c
> +++ b/drivers/tty/serial/sprd_serial.c
> @@ -20,6 +20,7 @@
>  #include <linux/slab.h>
>  #include <linux/tty.h>
>  #include <linux/tty_flip.h>
> +#include "../tty.h"
>  
>  /* device name */
>  #define UART_NR_MAX		8
> @@ -1221,7 +1222,10 @@ static int sprd_probe(struct platform_device *pdev)
>  static int sprd_suspend(struct device *dev)
>  {
>  	struct sprd_uart_port *sup = dev_get_drvdata(dev);
> +	struct uart_port *uport = &sup->port;
> +	struct tty_port *tty = &uport->state->port;
>  
> +	tty_buffer_cancel_work(tty);

What does this serial port have to do with the USB subsystem in your
changelog text?

And as the kernel bot said, this breaks the build, you can't do this
within a serial driver, sorry.

greg k-h
Re: [PATCH] tty/serial: Cancel work queue before closing uart
Posted by wenhua lin 2 years, 3 months ago 
On Tue, Aug 22, 2023 at 9:27 PM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Fri, Aug 18, 2023 at 11:15:32AM +0800, Wenhua Lin wrote:
> > When the system constantly sleeps and wankes up, plugging and unplugging
> > the USB will probalility trigger a kernel crash problem. The reason is
> > that at this time, the system entered deep and turned off uart, and the
> > abnormal behavior of plugging and upplugging the USB triggered the read
> > data process of uart, causing access to uart to hang. The final solution
> > we came up with is to cancel the work queue before shutting down uart
> > , while ensuring that there is no uart business.
> >
> > Signed-off-by: Wenhua Lin <Wenhua.Lin@unisoc.com>
> > ---
> >  drivers/tty/serial/sprd_serial.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/drivers/tty/serial/sprd_serial.c b/drivers/tty/serial/sprd_serial.c
> > index b58f51296ace..eddff4360155 100644
> > --- a/drivers/tty/serial/sprd_serial.c
> > +++ b/drivers/tty/serial/sprd_serial.c
> > @@ -20,6 +20,7 @@
> >  #include <linux/slab.h>
> >  #include <linux/tty.h>
> >  #include <linux/tty_flip.h>
> > +#include "../tty.h"
> >
> >  /* device name */
> >  #define UART_NR_MAX          8
> > @@ -1221,7 +1222,10 @@ static int sprd_probe(struct platform_device *pdev)
> >  static int sprd_suspend(struct device *dev)
> >  {
> >       struct sprd_uart_port *sup = dev_get_drvdata(dev);
> > +     struct uart_port *uport = &sup->port;
> > +     struct tty_port *tty = &uport->state->port;
> >
> > +     tty_buffer_cancel_work(tty);
>
> What does this serial port have to do with the USB subsystem in your
> changelog text?
>
> And as the kernel bot said, this breaks the build, you can't do this
> within a serial driver, sorry.

This modification is not directly related to the usb subsystem.
I just described a problem scenario, such as the previous comment.
There may be problems with this solution. I will make changes on patch v2,
thank you for your review

>
> greg k-h
Re: [PATCH] tty/serial: Cancel work queue before closing uart
Posted by Ilpo Järvinen 2 years, 3 months ago 
On Fri, 18 Aug 2023, Wenhua Lin wrote:

I've problems following your description below due to grammar errors.

> When the system constantly sleeps and wankes up, plugging and unplugging

wakes

> the USB will probalility trigger a kernel crash problem.

probalility is typoed and I cannot guess which of the words you meant, 
please fix.

If there's a known crash you're fixing here, please quote the crash 
message in the changelog (and you should probably add Fixes: tag too in 
that case).

> The reason is
> that at this time, the system entered deep and turned off uart, and the

"entered deep" lacks probably some word?

> abnormal behavior of plugging and upplugging the USB triggered the read

unplugging.

Why call that abnormal behavior? Isn't USB expected to removed.

> data process of uart, causing access to uart to hang.

Are you saying a read was triggered while the UART was suspended or what?

> The final solution
> we came up with is to cancel the work queue before shutting down uart
> , while ensuring that there is no uart business.

", while ensuring" -> to ensure

"uart business" is too vague, you should replace it with something more 
concrete.

Thanks.

--
 i.

> 
> Signed-off-by: Wenhua Lin <Wenhua.Lin@unisoc.com>
> ---
>  drivers/tty/serial/sprd_serial.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/tty/serial/sprd_serial.c b/drivers/tty/serial/sprd_serial.c
> index b58f51296ace..eddff4360155 100644
> --- a/drivers/tty/serial/sprd_serial.c
> +++ b/drivers/tty/serial/sprd_serial.c
> @@ -20,6 +20,7 @@
>  #include <linux/slab.h>
>  #include <linux/tty.h>
>  #include <linux/tty_flip.h>
> +#include "../tty.h"
>  
>  /* device name */
>  #define UART_NR_MAX		8
> @@ -1221,7 +1222,10 @@ static int sprd_probe(struct platform_device *pdev)
>  static int sprd_suspend(struct device *dev)
>  {
>  	struct sprd_uart_port *sup = dev_get_drvdata(dev);
> +	struct uart_port *uport = &sup->port;
> +	struct tty_port *tty = &uport->state->port;
>  
> +	tty_buffer_cancel_work(tty);
>  	uart_suspend_port(&sprd_uart_driver, &sup->port);
>  
>  	return 0;
>
Re: [PATCH] tty/serial: Cancel work queue before closing uart
Posted by wenhua lin 2 years, 3 months ago 
On Tue, Aug 22, 2023 at 4:23 PM Ilpo Järvinen
<ilpo.jarvinen@linux.intel.com> wrote:
>
> On Fri, 18 Aug 2023, Wenhua Lin wrote:
>
> I've problems following your description below due to grammar errors.
>
> > When the system constantly sleeps and wankes up, plugging and unplugging
>
> wakes
>
> > the USB will probalility trigger a kernel crash problem.
>
> probalility is typoed and I cannot guess which of the words you meant,
> please fix.
>
> If there's a known crash you're fixing here, please quote the crash
> message in the changelog (and you should probably add Fixes: tag too in
> that case).
>
> > The reason is
> > that at this time, the system entered deep and turned off uart, and the
>
> "entered deep" lacks probably some word?
>
> > abnormal behavior of plugging and upplugging the USB triggered the read
>
> unplugging.
>
> Why call that abnormal behavior? Isn't USB expected to removed.
>
> > data process of uart, causing access to uart to hang.
>
> Are you saying a read was triggered while the UART was suspended or what?
>
> > The final solution
> > we came up with is to cancel the work queue before shutting down uart
> > , while ensuring that there is no uart business.
>
> ", while ensuring" -> to ensure
>
> "uart business" is too vague, you should replace it with something more
> concrete.

We will fix it in patch v2, thank you  for your review.

>
> Thanks.
>
> --
>  i.
>
> >
> > Signed-off-by: Wenhua Lin <Wenhua.Lin@unisoc.com>
> > ---
> >  drivers/tty/serial/sprd_serial.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/drivers/tty/serial/sprd_serial.c b/drivers/tty/serial/sprd_serial.c
> > index b58f51296ace..eddff4360155 100644
> > --- a/drivers/tty/serial/sprd_serial.c
> > +++ b/drivers/tty/serial/sprd_serial.c
> > @@ -20,6 +20,7 @@
> >  #include <linux/slab.h>
> >  #include <linux/tty.h>
> >  #include <linux/tty_flip.h>
> > +#include "../tty.h"
> >
> >  /* device name */
> >  #define UART_NR_MAX          8
> > @@ -1221,7 +1222,10 @@ static int sprd_probe(struct platform_device *pdev)
> >  static int sprd_suspend(struct device *dev)
> >  {
> >       struct sprd_uart_port *sup = dev_get_drvdata(dev);
> > +     struct uart_port *uport = &sup->port;
> > +     struct tty_port *tty = &uport->state->port;
> >
> > +     tty_buffer_cancel_work(tty);
> >       uart_suspend_port(&sprd_uart_driver, &sup->port);
> >
> >       return 0;
> >
Re: [PATCH] tty/serial: Cancel work queue before closing uart
Posted by kernel test robot 2 years, 3 months ago 
Hi Wenhua,

kernel test robot noticed the following build errors:

[auto build test ERROR on tty/tty-testing]
[also build test ERROR on tty/tty-next tty/tty-linus usb/usb-testing usb/usb-next usb/usb-linus linus/master v6.5-rc7 next-20230821]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Wenhua-Lin/tty-serial-Cancel-work-queue-before-closing-uart/20230818-111905
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git tty-testing
patch link:    https://lore.kernel.org/r/20230818031532.15591-1-Wenhua.Lin%40unisoc.com
patch subject: [PATCH] tty/serial: Cancel work queue before closing uart
config: i386-allmodconfig (https://download.01.org/0day-ci/archive/20230822/202308221254.RnJddA0P-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce: (https://download.01.org/0day-ci/archive/20230822/202308221254.RnJddA0P-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202308221254.RnJddA0P-lkp@intel.com/

All errors (new ones prefixed by >>, old ones prefixed by <<):

WARNING: modpost: missing MODULE_DESCRIPTION() in net/netfilter/ipvs/ip_vs_lc.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/netfilter/ipvs/ip_vs_wlc.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/netfilter/ipvs/ip_vs_fo.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/netfilter/ipvs/ip_vs_ovf.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/netfilter/ipvs/ip_vs_lblc.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/netfilter/ipvs/ip_vs_lblcr.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/netfilter/ipvs/ip_vs_dh.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/netfilter/ipvs/ip_vs_sh.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/netfilter/ipvs/ip_vs_sed.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/netfilter/ipvs/ip_vs_nq.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/netfilter/ipvs/ip_vs_twos.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/netfilter/ipvs/ip_vs_ftp.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/netfilter/ipvs/ip_vs_pe_sip.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/ipv4/netfilter/nf_defrag_ipv4.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/ipv4/netfilter/nf_reject_ipv4.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/ipv4/netfilter/iptable_nat.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/ipv4/netfilter/iptable_raw.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/ipv4/ip_tunnel.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/ipv4/ipip.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/ipv4/ip_gre.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/ipv4/udp_tunnel.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/ipv4/ip_vti.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/ipv4/ah4.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/ipv4/esp4.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/ipv4/xfrm4_tunnel.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/ipv4/tunnel4.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/ipv4/inet_diag.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/ipv4/tcp_diag.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/ipv4/udp_diag.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/ipv4/raw_diag.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/xfrm/xfrm_algo.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/xfrm/xfrm_user.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/unix/unix_diag.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/ipv6/netfilter/ip6table_raw.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/ipv6/netfilter/ip6table_nat.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/ipv6/netfilter/nf_defrag_ipv6.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/ipv6/netfilter/nf_reject_ipv6.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/ipv6/ah6.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/ipv6/esp6.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/ipv6/xfrm6_tunnel.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/ipv6/tunnel6.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/ipv6/mip6.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/ipv6/sit.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/ipv6/ip6_udp_tunnel.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/bpfilter/bpfilter.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/dsa/tag_ar9331.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/dsa/tag_brcm.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/dsa/tag_dsa.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/dsa/tag_gswip.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/dsa/tag_hellcreek.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/dsa/tag_ksz.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/dsa/tag_lan9303.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/dsa/tag_mtk.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/dsa/tag_none.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/dsa/tag_ocelot.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/dsa/tag_ocelot_8021q.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/dsa/tag_qca.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/dsa/tag_rtl4_a.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/dsa/tag_rtl8_4.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/dsa/tag_rzn1_a5psw.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/dsa/tag_sja1105.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/dsa/tag_trailer.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/dsa/tag_xrs700x.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/8021q/8021q.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/xdp/xsk_diag.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/mptcp/mptcp_diag.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/mptcp/mptcp_crypto_test.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/mptcp/mptcp_token_test.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/packet/af_packet.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/packet/af_packet_diag.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/key/af_key.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/bridge/netfilter/nf_conntrack_bridge.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/bridge/netfilter/ebtables.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/bridge/netfilter/ebtable_broute.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/bridge/netfilter/ebtable_filter.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/bridge/netfilter/ebtable_nat.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/bridge/bridge.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/sunrpc/sunrpc.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/sunrpc/auth_gss/auth_rpcgss.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/sunrpc/auth_gss/rpcsec_gss_krb5.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/kcm/kcm.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/atm/atm.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/atm/lec.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/atm/mpoa.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/sctp/sctp_diag.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/tipc/diag.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/smc/smc_diag.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/caif/caif.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/caif/chnl_net.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/caif/caif_socket.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/caif/caif_usb.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/6lowpan/6lowpan.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/ieee802154/6lowpan/ieee802154_6lowpan.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/ieee802154/ieee802154_socket.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/nfc/nci/nci.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/nfc/nci/nci_spi.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/nfc/nfc_digital.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/vmw_vsock/vsock_diag.o
WARNING: modpost: missing MODULE_DESCRIPTION() in net/hsr/hsr.o
WARNING: modpost: missing MODULE_DESCRIPTION() in arch/x86/video/fbdev.o
>> ERROR: modpost: "tty_buffer_cancel_work" [drivers/tty/serial/sprd_serial.ko] undefined!

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.