* [PATCH v2 0/1] tty: serial: samsung: add spin_lock in console_write [not found] <CGME20220406081823epcas2p3789ef3ac3956e4713dd1b55c8a49fb05@epcas2p3.samsung.com> @ 2022-04-06 8:22 ` Jaewon Kim 0 siblings, 0 replies; 20+ messages in thread From: Jaewon Kim @ 2022-04-06 8:22 UTC (permalink / raw) To: Krzysztof Kozlowski, Greg Kroah-Hartman, Alim Akhtar, Jiri Slaby, linux-samsung-soc Cc: linux-arm-kernel, linux-serial, linux-kernel, Chanho Park, Jaewon Kim When console and printk log are printed at the same time, they are called through tty driver and console driver concurrently. In this case, this could lead to potintial issue that data loss or fifo full. This issue also occurred with other drivers and has been fixed. "serial: amba-pl011: lock console writes against interrupts" - https://lkml.org/lkml/2012/2/1/495 --- Chnages since v1: - locked variable type changed bool from int - spin_lock() changed to spin_ock_irqsave() Jaewon Kim (1): tty: serial: samsung: add spin_lock for interrupt and console_write drivers/tty/serial/samsung_tty.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) -- 2.35.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 0/1] tty: serial: samsung: add spin_lock in console_write @ 2022-04-06 8:22 ` Jaewon Kim 0 siblings, 0 replies; 20+ messages in thread From: Jaewon Kim @ 2022-04-06 8:22 UTC (permalink / raw) To: Krzysztof Kozlowski, Greg Kroah-Hartman, Alim Akhtar, Jiri Slaby, linux-samsung-soc Cc: linux-arm-kernel, linux-serial, linux-kernel, Chanho Park, Jaewon Kim When console and printk log are printed at the same time, they are called through tty driver and console driver concurrently. In this case, this could lead to potintial issue that data loss or fifo full. This issue also occurred with other drivers and has been fixed. "serial: amba-pl011: lock console writes against interrupts" - https://lkml.org/lkml/2012/2/1/495 --- Chnages since v1: - locked variable type changed bool from int - spin_lock() changed to spin_ock_irqsave() Jaewon Kim (1): tty: serial: samsung: add spin_lock for interrupt and console_write drivers/tty/serial/samsung_tty.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) -- 2.35.1 ^ permalink raw reply [flat|nested] 20+ messages in thread
[parent not found: <CGME20220406081823epcas2p2f7afa27e2402c4fc02c9bee5972bed4f@epcas2p2.samsung.com>]
* [PATCH 1/1] tty: serial: samsung: add spin_lock for interrupt and console_write [not found] ` <CGME20220406081823epcas2p2f7afa27e2402c4fc02c9bee5972bed4f@epcas2p2.samsung.com> @ 2022-04-06 8:22 ` Jaewon Kim 0 siblings, 0 replies; 20+ messages in thread From: Jaewon Kim @ 2022-04-06 8:22 UTC (permalink / raw) To: Krzysztof Kozlowski, Greg Kroah-Hartman, Alim Akhtar, Jiri Slaby, linux-samsung-soc Cc: linux-arm-kernel, linux-serial, linux-kernel, Chanho Park, Jaewon Kim The console_write and IRQ handler can run concurrently. Problems may occurs console_write is continuously executed while the IRQ handler is running. Signed-off-by: Jaewon Kim <jaewon02.kim@samsung.com> --- drivers/tty/serial/samsung_tty.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/drivers/tty/serial/samsung_tty.c b/drivers/tty/serial/samsung_tty.c index e1585fbae909..9db479d728b5 100644 --- a/drivers/tty/serial/samsung_tty.c +++ b/drivers/tty/serial/samsung_tty.c @@ -2480,12 +2480,24 @@ s3c24xx_serial_console_write(struct console *co, const char *s, unsigned int count) { unsigned int ucon = rd_regl(cons_uart, S3C2410_UCON); + unsigned long flags; + bool locked = 1; /* not possible to xmit on unconfigured port */ if (!s3c24xx_port_configured(ucon)) return; + if (cons_uart->sysrq) + locked = 0; + else if (oops_in_progress) + locked = spin_trylock_irqsave(&cons_uart->lock, flags); + else + spin_lock_irqsave(&cons_uart->lock, flags); + uart_console_write(cons_uart, s, count, s3c24xx_serial_console_putchar); + + if (locked) + spin_unlock_irqrestore(&cons_uart->lock, flags); } /* Shouldn't be __init, as it can be instantiated from other module */ -- 2.35.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 1/1] tty: serial: samsung: add spin_lock for interrupt and console_write @ 2022-04-06 8:22 ` Jaewon Kim 0 siblings, 0 replies; 20+ messages in thread From: Jaewon Kim @ 2022-04-06 8:22 UTC (permalink / raw) To: Krzysztof Kozlowski, Greg Kroah-Hartman, Alim Akhtar, Jiri Slaby, linux-samsung-soc Cc: linux-arm-kernel, linux-serial, linux-kernel, Chanho Park, Jaewon Kim The console_write and IRQ handler can run concurrently. Problems may occurs console_write is continuously executed while the IRQ handler is running. Signed-off-by: Jaewon Kim <jaewon02.kim@samsung.com> --- drivers/tty/serial/samsung_tty.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/drivers/tty/serial/samsung_tty.c b/drivers/tty/serial/samsung_tty.c index e1585fbae909..9db479d728b5 100644 --- a/drivers/tty/serial/samsung_tty.c +++ b/drivers/tty/serial/samsung_tty.c @@ -2480,12 +2480,24 @@ s3c24xx_serial_console_write(struct console *co, const char *s, unsigned int count) { unsigned int ucon = rd_regl(cons_uart, S3C2410_UCON); + unsigned long flags; + bool locked = 1; /* not possible to xmit on unconfigured port */ if (!s3c24xx_port_configured(ucon)) return; + if (cons_uart->sysrq) + locked = 0; + else if (oops_in_progress) + locked = spin_trylock_irqsave(&cons_uart->lock, flags); + else + spin_lock_irqsave(&cons_uart->lock, flags); + uart_console_write(cons_uart, s, count, s3c24xx_serial_console_putchar); + + if (locked) + spin_unlock_irqrestore(&cons_uart->lock, flags); } /* Shouldn't be __init, as it can be instantiated from other module */ -- 2.35.1 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 1/1] tty: serial: samsung: add spin_lock for interrupt and console_write 2022-04-06 8:22 ` Jaewon Kim @ 2022-04-06 8:21 ` Greg Kroah-Hartman -1 siblings, 0 replies; 20+ messages in thread From: Greg Kroah-Hartman @ 2022-04-06 8:21 UTC (permalink / raw) To: Jaewon Kim Cc: Krzysztof Kozlowski, Alim Akhtar, Jiri Slaby, linux-samsung-soc, linux-arm-kernel, linux-serial, linux-kernel, Chanho Park On Wed, Apr 06, 2022 at 05:22:16PM +0900, Jaewon Kim wrote: > The console_write and IRQ handler can run concurrently. > Problems may occurs console_write is continuously executed while > the IRQ handler is running. > > Signed-off-by: Jaewon Kim <jaewon02.kim@samsung.com> > --- > drivers/tty/serial/samsung_tty.c | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/drivers/tty/serial/samsung_tty.c b/drivers/tty/serial/samsung_tty.c > index e1585fbae909..9db479d728b5 100644 > --- a/drivers/tty/serial/samsung_tty.c > +++ b/drivers/tty/serial/samsung_tty.c > @@ -2480,12 +2480,24 @@ s3c24xx_serial_console_write(struct console *co, const char *s, > unsigned int count) > { > unsigned int ucon = rd_regl(cons_uart, S3C2410_UCON); > + unsigned long flags; > + bool locked = 1; "1" is not a boolean :) _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/1] tty: serial: samsung: add spin_lock for interrupt and console_write @ 2022-04-06 8:21 ` Greg Kroah-Hartman 0 siblings, 0 replies; 20+ messages in thread From: Greg Kroah-Hartman @ 2022-04-06 8:21 UTC (permalink / raw) To: Jaewon Kim Cc: Krzysztof Kozlowski, Alim Akhtar, Jiri Slaby, linux-samsung-soc, linux-arm-kernel, linux-serial, linux-kernel, Chanho Park On Wed, Apr 06, 2022 at 05:22:16PM +0900, Jaewon Kim wrote: > The console_write and IRQ handler can run concurrently. > Problems may occurs console_write is continuously executed while > the IRQ handler is running. > > Signed-off-by: Jaewon Kim <jaewon02.kim@samsung.com> > --- > drivers/tty/serial/samsung_tty.c | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/drivers/tty/serial/samsung_tty.c b/drivers/tty/serial/samsung_tty.c > index e1585fbae909..9db479d728b5 100644 > --- a/drivers/tty/serial/samsung_tty.c > +++ b/drivers/tty/serial/samsung_tty.c > @@ -2480,12 +2480,24 @@ s3c24xx_serial_console_write(struct console *co, const char *s, > unsigned int count) > { > unsigned int ucon = rd_regl(cons_uart, S3C2410_UCON); > + unsigned long flags; > + bool locked = 1; "1" is not a boolean :) ^ permalink raw reply [flat|nested] 20+ messages in thread
* RE: [PATCH 1/1] tty: serial: samsung: add spin_lock for interrupt and console_write 2022-04-06 8:21 ` Greg Kroah-Hartman @ 2022-04-06 8:39 ` Jaewon Kim -1 siblings, 0 replies; 20+ messages in thread From: Jaewon Kim @ 2022-04-06 8:39 UTC (permalink / raw) To: 'Greg Kroah-Hartman' Cc: 'Krzysztof Kozlowski', 'Alim Akhtar', 'Jiri Slaby', linux-samsung-soc, linux-arm-kernel, linux-serial, linux-kernel, 'Chanho Park' Hello On 22. 4. 6. 17:21, Greg Kroah-Hartman wrote: > On Wed, Apr 06, 2022 at 05:22:16PM +0900, Jaewon Kim wrote: > > The console_write and IRQ handler can run concurrently. > > Problems may occurs console_write is continuously executed while the > > IRQ handler is running. > > > > Signed-off-by: Jaewon Kim <jaewon02.kim@samsung.com> > > --- > > drivers/tty/serial/samsung_tty.c | 12 ++++++++++++ > > 1 file changed, 12 insertions(+) > > > > diff --git a/drivers/tty/serial/samsung_tty.c > > b/drivers/tty/serial/samsung_tty.c > > index e1585fbae909..9db479d728b5 100644 > > --- a/drivers/tty/serial/samsung_tty.c > > +++ b/drivers/tty/serial/samsung_tty.c > > @@ -2480,12 +2480,24 @@ s3c24xx_serial_console_write(struct console *co, const char *s, > > unsigned int count) > > { > > unsigned int ucon = rd_regl(cons_uart, S3C2410_UCON); > > + unsigned long flags; > > + bool locked = 1; > > "1" is not a boolean :) return value of spin_trylock() is 1 or 0. It seems better to keep it as an int than to change it to bool. I will return it to int. Thanks Jaewon Kim _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 20+ messages in thread
* RE: [PATCH 1/1] tty: serial: samsung: add spin_lock for interrupt and console_write @ 2022-04-06 8:39 ` Jaewon Kim 0 siblings, 0 replies; 20+ messages in thread From: Jaewon Kim @ 2022-04-06 8:39 UTC (permalink / raw) To: 'Greg Kroah-Hartman' Cc: 'Krzysztof Kozlowski', 'Alim Akhtar', 'Jiri Slaby', linux-samsung-soc, linux-arm-kernel, linux-serial, linux-kernel, 'Chanho Park' Hello On 22. 4. 6. 17:21, Greg Kroah-Hartman wrote: > On Wed, Apr 06, 2022 at 05:22:16PM +0900, Jaewon Kim wrote: > > The console_write and IRQ handler can run concurrently. > > Problems may occurs console_write is continuously executed while the > > IRQ handler is running. > > > > Signed-off-by: Jaewon Kim <jaewon02.kim@samsung.com> > > --- > > drivers/tty/serial/samsung_tty.c | 12 ++++++++++++ > > 1 file changed, 12 insertions(+) > > > > diff --git a/drivers/tty/serial/samsung_tty.c > > b/drivers/tty/serial/samsung_tty.c > > index e1585fbae909..9db479d728b5 100644 > > --- a/drivers/tty/serial/samsung_tty.c > > +++ b/drivers/tty/serial/samsung_tty.c > > @@ -2480,12 +2480,24 @@ s3c24xx_serial_console_write(struct console *co, const char *s, > > unsigned int count) > > { > > unsigned int ucon = rd_regl(cons_uart, S3C2410_UCON); > > + unsigned long flags; > > + bool locked = 1; > > "1" is not a boolean :) return value of spin_trylock() is 1 or 0. It seems better to keep it as an int than to change it to bool. I will return it to int. Thanks Jaewon Kim ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/1] tty: serial: samsung: add spin_lock for interrupt and console_write 2022-04-06 8:39 ` Jaewon Kim @ 2022-04-06 9:48 ` Jiri Slaby -1 siblings, 0 replies; 20+ messages in thread From: Jiri Slaby @ 2022-04-06 9:48 UTC (permalink / raw) To: Jaewon Kim, 'Greg Kroah-Hartman' Cc: 'Krzysztof Kozlowski', 'Alim Akhtar', linux-samsung-soc, linux-arm-kernel, linux-serial, linux-kernel, 'Chanho Park' On 06. 04. 22, 10:39, Jaewon Kim wrote: > On 22. 4. 6. 17:21, Greg Kroah-Hartman wrote: >> On Wed, Apr 06, 2022 at 05:22:16PM +0900, Jaewon Kim wrote: >>> The console_write and IRQ handler can run concurrently. >>> Problems may occurs console_write is continuously executed while the >>> IRQ handler is running. >>> >>> Signed-off-by: Jaewon Kim <jaewon02.kim@samsung.com> >>> --- >>> drivers/tty/serial/samsung_tty.c | 12 ++++++++++++ >>> 1 file changed, 12 insertions(+) >>> >>> diff --git a/drivers/tty/serial/samsung_tty.c >>> b/drivers/tty/serial/samsung_tty.c >>> index e1585fbae909..9db479d728b5 100644 >>> --- a/drivers/tty/serial/samsung_tty.c >>> +++ b/drivers/tty/serial/samsung_tty.c >>> @@ -2480,12 +2480,24 @@ s3c24xx_serial_console_write(struct console *co, const char *s, >>> unsigned int count) >>> { >>> unsigned int ucon = rd_regl(cons_uart, S3C2410_UCON); >>> + unsigned long flags; >>> + bool locked = 1; >> >> "1" is not a boolean :) > > return value of spin_trylock() is 1 or 0. > It seems better to keep it as an int than to change it to bool. > I will return it to int. Hi, no, do not that. Simply use bool/true/false. thanks, -- js suse labs _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/1] tty: serial: samsung: add spin_lock for interrupt and console_write @ 2022-04-06 9:48 ` Jiri Slaby 0 siblings, 0 replies; 20+ messages in thread From: Jiri Slaby @ 2022-04-06 9:48 UTC (permalink / raw) To: Jaewon Kim, 'Greg Kroah-Hartman' Cc: 'Krzysztof Kozlowski', 'Alim Akhtar', linux-samsung-soc, linux-arm-kernel, linux-serial, linux-kernel, 'Chanho Park' On 06. 04. 22, 10:39, Jaewon Kim wrote: > On 22. 4. 6. 17:21, Greg Kroah-Hartman wrote: >> On Wed, Apr 06, 2022 at 05:22:16PM +0900, Jaewon Kim wrote: >>> The console_write and IRQ handler can run concurrently. >>> Problems may occurs console_write is continuously executed while the >>> IRQ handler is running. >>> >>> Signed-off-by: Jaewon Kim <jaewon02.kim@samsung.com> >>> --- >>> drivers/tty/serial/samsung_tty.c | 12 ++++++++++++ >>> 1 file changed, 12 insertions(+) >>> >>> diff --git a/drivers/tty/serial/samsung_tty.c >>> b/drivers/tty/serial/samsung_tty.c >>> index e1585fbae909..9db479d728b5 100644 >>> --- a/drivers/tty/serial/samsung_tty.c >>> +++ b/drivers/tty/serial/samsung_tty.c >>> @@ -2480,12 +2480,24 @@ s3c24xx_serial_console_write(struct console *co, const char *s, >>> unsigned int count) >>> { >>> unsigned int ucon = rd_regl(cons_uart, S3C2410_UCON); >>> + unsigned long flags; >>> + bool locked = 1; >> >> "1" is not a boolean :) > > return value of spin_trylock() is 1 or 0. > It seems better to keep it as an int than to change it to bool. > I will return it to int. Hi, no, do not that. Simply use bool/true/false. thanks, -- js suse labs ^ permalink raw reply [flat|nested] 20+ messages in thread
* RE: [PATCH 1/1] tty: serial: samsung: add spin_lock for interrupt and console_write 2022-04-06 9:48 ` Jiri Slaby @ 2022-04-06 11:19 ` Jaewon Kim -1 siblings, 0 replies; 20+ messages in thread From: Jaewon Kim @ 2022-04-06 11:19 UTC (permalink / raw) To: 'Jiri Slaby', 'Greg Kroah-Hartman' Cc: 'Krzysztof Kozlowski', 'Alim Akhtar', linux-samsung-soc, linux-arm-kernel, linux-serial, linux-kernel, 'Chanho Park' On 22. 4. 6. 18:48, Jiri Slaby wrote:> On 06. 04. 22, 10:39, Jaewon Kim wrote: > > On 22. 4. 6. 17:21, Greg Kroah-Hartman wrote: > >> On Wed, Apr 06, 2022 at 05:22:16PM +0900, Jaewon Kim wrote: > >>> The console_write and IRQ handler can run concurrently. > >>> Problems may occurs console_write is continuously executed while the > >>> IRQ handler is running. > >>> > >>> Signed-off-by: Jaewon Kim <jaewon02.kim@samsung.com> > >>> --- > >>> drivers/tty/serial/samsung_tty.c | 12 ++++++++++++ > >>> 1 file changed, 12 insertions(+) > >>> > >>> diff --git a/drivers/tty/serial/samsung_tty.c > >>> b/drivers/tty/serial/samsung_tty.c > >>> index e1585fbae909..9db479d728b5 100644 > >>> --- a/drivers/tty/serial/samsung_tty.c > >>> +++ b/drivers/tty/serial/samsung_tty.c > >>> @@ -2480,12 +2480,24 @@ s3c24xx_serial_console_write(struct console *co, const char *s, > >>> unsigned int count) > >>> { > >>> unsigned int ucon = rd_regl(cons_uart, S3C2410_UCON); > >>> + unsigned long flags; > >>> + bool locked = 1; > >> > >> "1" is not a boolean :) > > > > return value of spin_trylock() is 1 or 0. > > It seems better to keep it as an int than to change it to bool. > > I will return it to int. > > Hi, no, do not that. Simply use bool/true/false. > Okay, Thanks to review. I will fix 1 to bool in next v3 patch. > thanks, > -- > js > suse labs Thanks Jaewon Kim _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 20+ messages in thread
* RE: [PATCH 1/1] tty: serial: samsung: add spin_lock for interrupt and console_write @ 2022-04-06 11:19 ` Jaewon Kim 0 siblings, 0 replies; 20+ messages in thread From: Jaewon Kim @ 2022-04-06 11:19 UTC (permalink / raw) To: 'Jiri Slaby', 'Greg Kroah-Hartman' Cc: 'Krzysztof Kozlowski', 'Alim Akhtar', linux-samsung-soc, linux-arm-kernel, linux-serial, linux-kernel, 'Chanho Park' On 22. 4. 6. 18:48, Jiri Slaby wrote:> On 06. 04. 22, 10:39, Jaewon Kim wrote: > > On 22. 4. 6. 17:21, Greg Kroah-Hartman wrote: > >> On Wed, Apr 06, 2022 at 05:22:16PM +0900, Jaewon Kim wrote: > >>> The console_write and IRQ handler can run concurrently. > >>> Problems may occurs console_write is continuously executed while the > >>> IRQ handler is running. > >>> > >>> Signed-off-by: Jaewon Kim <jaewon02.kim@samsung.com> > >>> --- > >>> drivers/tty/serial/samsung_tty.c | 12 ++++++++++++ > >>> 1 file changed, 12 insertions(+) > >>> > >>> diff --git a/drivers/tty/serial/samsung_tty.c > >>> b/drivers/tty/serial/samsung_tty.c > >>> index e1585fbae909..9db479d728b5 100644 > >>> --- a/drivers/tty/serial/samsung_tty.c > >>> +++ b/drivers/tty/serial/samsung_tty.c > >>> @@ -2480,12 +2480,24 @@ s3c24xx_serial_console_write(struct console *co, const char *s, > >>> unsigned int count) > >>> { > >>> unsigned int ucon = rd_regl(cons_uart, S3C2410_UCON); > >>> + unsigned long flags; > >>> + bool locked = 1; > >> > >> "1" is not a boolean :) > > > > return value of spin_trylock() is 1 or 0. > > It seems better to keep it as an int than to change it to bool. > > I will return it to int. > > Hi, no, do not that. Simply use bool/true/false. > Okay, Thanks to review. I will fix 1 to bool in next v3 patch. > thanks, > -- > js > suse labs Thanks Jaewon Kim ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/1] tty: serial: samsung: add spin_lock for interrupt and console_write 2022-04-06 11:19 ` Jaewon Kim @ 2022-04-06 11:21 ` Jiri Slaby -1 siblings, 0 replies; 20+ messages in thread From: Jiri Slaby @ 2022-04-06 11:21 UTC (permalink / raw) To: Jaewon Kim, 'Greg Kroah-Hartman' Cc: 'Krzysztof Kozlowski', 'Alim Akhtar', linux-samsung-soc, linux-arm-kernel, linux-serial, linux-kernel, 'Chanho Park' On 06. 04. 22, 13:19, Jaewon Kim wrote: > Okay, Thanks to review. > I will fix 1 to bool in next v3 patch. (And 0 to false.) -- js suse labs _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/1] tty: serial: samsung: add spin_lock for interrupt and console_write @ 2022-04-06 11:21 ` Jiri Slaby 0 siblings, 0 replies; 20+ messages in thread From: Jiri Slaby @ 2022-04-06 11:21 UTC (permalink / raw) To: Jaewon Kim, 'Greg Kroah-Hartman' Cc: 'Krzysztof Kozlowski', 'Alim Akhtar', linux-samsung-soc, linux-arm-kernel, linux-serial, linux-kernel, 'Chanho Park' On 06. 04. 22, 13:19, Jaewon Kim wrote: > Okay, Thanks to review. > I will fix 1 to bool in next v3 patch. (And 0 to false.) -- js suse labs ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 0/1] tty: serial: samsung: add spin_lock in console_write @ 2022-04-05 3:38 Jaewon Kim [not found] ` <CGME20220405033448epcas2p397080e15c54369d24eaf94c2a27bd06c@epcas2p3.samsung.com> 0 siblings, 1 reply; 20+ messages in thread From: Jaewon Kim @ 2022-04-05 3:38 UTC (permalink / raw) To: Krzysztof Kozlowski, Greg Kroah-Hartman, Alim Akhtar, Jiri Slaby, linux-samsung-soc Cc: linux-arm-kernel, linux-serial, linux-kernel, Chanho Park, Jaewon Kim When console and printk log are printed at the same time, they are called through tty driver and console driver concurrently. In this case, this could lead to potintial issue that data loss or fifo full. This issue also occurred with other drivers and has been fixed. "serial: amba-pl011: lock console writes against interrupts" Jaewon Kim (1): tty: serial: samsung: add spin_lock for interrupt and console_write drivers/tty/serial/samsung_tty.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) -- 2.35.1 ^ permalink raw reply [flat|nested] 20+ messages in thread
[parent not found: <CGME20220405033448epcas2p397080e15c54369d24eaf94c2a27bd06c@epcas2p3.samsung.com>]
* [PATCH 1/1] tty: serial: samsung: add spin_lock for interrupt and console_write [not found] ` <CGME20220405033448epcas2p397080e15c54369d24eaf94c2a27bd06c@epcas2p3.samsung.com> @ 2022-04-05 3:38 ` Jaewon Kim 0 siblings, 0 replies; 20+ messages in thread From: Jaewon Kim @ 2022-04-05 3:38 UTC (permalink / raw) To: Krzysztof Kozlowski, Greg Kroah-Hartman, Alim Akhtar, Jiri Slaby, linux-samsung-soc Cc: linux-arm-kernel, linux-serial, linux-kernel, Chanho Park, Jaewon Kim The console_write and IRQ handler can run concurrently. Problems may occurs console_write is continuously executed while the IRQ handler is running. Signed-off-by: Jaewon Kim <jaewon02.kim@samsung.com> --- drivers/tty/serial/samsung_tty.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/drivers/tty/serial/samsung_tty.c b/drivers/tty/serial/samsung_tty.c index e1585fbae909..d362e8e114f1 100644 --- a/drivers/tty/serial/samsung_tty.c +++ b/drivers/tty/serial/samsung_tty.c @@ -2480,12 +2480,26 @@ s3c24xx_serial_console_write(struct console *co, const char *s, unsigned int count) { unsigned int ucon = rd_regl(cons_uart, S3C2410_UCON); + unsigned long flags; + int locked = 1; /* not possible to xmit on unconfigured port */ if (!s3c24xx_port_configured(ucon)) return; + local_irq_save(flags); + if (cons_uart->sysrq) + locked = 0; + else if (oops_in_progress) + locked = spin_trylock(&cons_uart->lock); + else + spin_lock(&cons_uart->lock); + uart_console_write(cons_uart, s, count, s3c24xx_serial_console_putchar); + + if (locked) + spin_unlock(&cons_uart->lock); + local_irq_restore(flags); } /* Shouldn't be __init, as it can be instantiated from other module */ -- 2.35.1 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 1/1] tty: serial: samsung: add spin_lock for interrupt and console_write @ 2022-04-05 3:38 ` Jaewon Kim 0 siblings, 0 replies; 20+ messages in thread From: Jaewon Kim @ 2022-04-05 3:38 UTC (permalink / raw) To: Krzysztof Kozlowski, Greg Kroah-Hartman, Alim Akhtar, Jiri Slaby, linux-samsung-soc Cc: linux-arm-kernel, linux-serial, linux-kernel, Chanho Park, Jaewon Kim The console_write and IRQ handler can run concurrently. Problems may occurs console_write is continuously executed while the IRQ handler is running. Signed-off-by: Jaewon Kim <jaewon02.kim@samsung.com> --- drivers/tty/serial/samsung_tty.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/drivers/tty/serial/samsung_tty.c b/drivers/tty/serial/samsung_tty.c index e1585fbae909..d362e8e114f1 100644 --- a/drivers/tty/serial/samsung_tty.c +++ b/drivers/tty/serial/samsung_tty.c @@ -2480,12 +2480,26 @@ s3c24xx_serial_console_write(struct console *co, const char *s, unsigned int count) { unsigned int ucon = rd_regl(cons_uart, S3C2410_UCON); + unsigned long flags; + int locked = 1; /* not possible to xmit on unconfigured port */ if (!s3c24xx_port_configured(ucon)) return; + local_irq_save(flags); + if (cons_uart->sysrq) + locked = 0; + else if (oops_in_progress) + locked = spin_trylock(&cons_uart->lock); + else + spin_lock(&cons_uart->lock); + uart_console_write(cons_uart, s, count, s3c24xx_serial_console_putchar); + + if (locked) + spin_unlock(&cons_uart->lock); + local_irq_restore(flags); } /* Shouldn't be __init, as it can be instantiated from other module */ -- 2.35.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 1/1] tty: serial: samsung: add spin_lock for interrupt and console_write 2022-04-05 3:38 ` Jaewon Kim @ 2022-04-05 5:01 ` Greg Kroah-Hartman -1 siblings, 0 replies; 20+ messages in thread From: Greg Kroah-Hartman @ 2022-04-05 5:01 UTC (permalink / raw) To: Jaewon Kim Cc: Krzysztof Kozlowski, Alim Akhtar, Jiri Slaby, linux-samsung-soc, linux-arm-kernel, linux-serial, linux-kernel, Chanho Park On Tue, Apr 05, 2022 at 12:38:54PM +0900, Jaewon Kim wrote: > The console_write and IRQ handler can run concurrently. > Problems may occurs console_write is continuously executed while > the IRQ handler is running. > > Signed-off-by: Jaewon Kim <jaewon02.kim@samsung.com> > --- > drivers/tty/serial/samsung_tty.c | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) What commit does this fix? > > diff --git a/drivers/tty/serial/samsung_tty.c b/drivers/tty/serial/samsung_tty.c > index e1585fbae909..d362e8e114f1 100644 > --- a/drivers/tty/serial/samsung_tty.c > +++ b/drivers/tty/serial/samsung_tty.c > @@ -2480,12 +2480,26 @@ s3c24xx_serial_console_write(struct console *co, const char *s, > unsigned int count) > { > unsigned int ucon = rd_regl(cons_uart, S3C2410_UCON); > + unsigned long flags; > + int locked = 1; bool? > > /* not possible to xmit on unconfigured port */ > if (!s3c24xx_port_configured(ucon)) > return; > > + local_irq_save(flags); > + if (cons_uart->sysrq) > + locked = 0; > + else if (oops_in_progress) > + locked = spin_trylock(&cons_uart->lock); > + else > + spin_lock(&cons_uart->lock); > + > uart_console_write(cons_uart, s, count, s3c24xx_serial_console_putchar); > + > + if (locked) > + spin_unlock(&cons_uart->lock); > + local_irq_restore(flags); Why is irq_save required as well as a spinlock? thanks, greg k-h ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/1] tty: serial: samsung: add spin_lock for interrupt and console_write @ 2022-04-05 5:01 ` Greg Kroah-Hartman 0 siblings, 0 replies; 20+ messages in thread From: Greg Kroah-Hartman @ 2022-04-05 5:01 UTC (permalink / raw) To: Jaewon Kim Cc: Krzysztof Kozlowski, Alim Akhtar, Jiri Slaby, linux-samsung-soc, linux-arm-kernel, linux-serial, linux-kernel, Chanho Park On Tue, Apr 05, 2022 at 12:38:54PM +0900, Jaewon Kim wrote: > The console_write and IRQ handler can run concurrently. > Problems may occurs console_write is continuously executed while > the IRQ handler is running. > > Signed-off-by: Jaewon Kim <jaewon02.kim@samsung.com> > --- > drivers/tty/serial/samsung_tty.c | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) What commit does this fix? > > diff --git a/drivers/tty/serial/samsung_tty.c b/drivers/tty/serial/samsung_tty.c > index e1585fbae909..d362e8e114f1 100644 > --- a/drivers/tty/serial/samsung_tty.c > +++ b/drivers/tty/serial/samsung_tty.c > @@ -2480,12 +2480,26 @@ s3c24xx_serial_console_write(struct console *co, const char *s, > unsigned int count) > { > unsigned int ucon = rd_regl(cons_uart, S3C2410_UCON); > + unsigned long flags; > + int locked = 1; bool? > > /* not possible to xmit on unconfigured port */ > if (!s3c24xx_port_configured(ucon)) > return; > > + local_irq_save(flags); > + if (cons_uart->sysrq) > + locked = 0; > + else if (oops_in_progress) > + locked = spin_trylock(&cons_uart->lock); > + else > + spin_lock(&cons_uart->lock); > + > uart_console_write(cons_uart, s, count, s3c24xx_serial_console_putchar); > + > + if (locked) > + spin_unlock(&cons_uart->lock); > + local_irq_restore(flags); Why is irq_save required as well as a spinlock? thanks, greg k-h _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 20+ messages in thread
* RE: [PATCH 1/1] tty: serial: samsung: add spin_lock for interrupt and console_write 2022-04-05 5:01 ` Greg Kroah-Hartman @ 2022-04-05 6:40 ` Jaewon Kim -1 siblings, 0 replies; 20+ messages in thread From: Jaewon Kim @ 2022-04-05 6:40 UTC (permalink / raw) To: 'Greg Kroah-Hartman' Cc: 'Krzysztof Kozlowski', 'Alim Akhtar', 'Jiri Slaby', linux-samsung-soc, linux-arm-kernel, linux-serial, linux-kernel, 'Chanho Park' Hello On 22. 4. 5. 14:01, Greg Kroah-Hartman wrote: > On Tue, Apr 05, 2022 at 12:38:54PM +0900, Jaewon Kim wrote: > > The console_write and IRQ handler can run concurrently. > > Problems may occurs console_write is continuously executed while the > > IRQ handler is running. > > > > Signed-off-by: Jaewon Kim <jaewon02.kim@samsung.com> > > --- > > drivers/tty/serial/samsung_tty.c | 14 ++++++++++++++ > > 1 file changed, 14 insertions(+) > > What commit does this fix? This is not an issue caused by anohter commits. There was potential issue from the beginning. Other drivers were fixed, but samsung_tty was not. PL011 patch : https://lkml.org/lkml/2012/2/1/495 > > > > > diff --git a/drivers/tty/serial/samsung_tty.c > > b/drivers/tty/serial/samsung_tty.c > > index e1585fbae909..d362e8e114f1 100644 > > --- a/drivers/tty/serial/samsung_tty.c > > +++ b/drivers/tty/serial/samsung_tty.c > > @@ -2480,12 +2480,26 @@ s3c24xx_serial_console_write(struct console *co, const char *s, > > unsigned int count) > > { > > unsigned int ucon = rd_regl(cons_uart, S3C2410_UCON); > > + unsigned long flags; > > + int locked = 1; > > bool? It is return value of spin_trylock() I used int because mose drivers used int. If you guide to change int to bool, I will change it. > > > > > /* not possible to xmit on unconfigured port */ > > if (!s3c24xx_port_configured(ucon)) > > return; > > > > + local_irq_save(flags); > > + if (cons_uart->sysrq) > > + locked = 0; > > + else if (oops_in_progress) > > + locked = spin_trylock(&cons_uart->lock); > > + else > > + spin_lock(&cons_uart->lock); > > + > > uart_console_write(cons_uart, s, count, > > s3c24xx_serial_console_putchar); > > + > > + if (locked) > > + spin_unlock(&cons_uart->lock); > > + local_irq_restore(flags); > > Why is irq_save required as well as a spinlock? No special reason. I will change spin_trylock() -? spin_trylock_irqsave(). spin_lock -> spin_lock_irqsave(). And, remove local_irq_save/restore. It looks more clean. > > thanks, > > greg k-h Thanks Jaewon Kim ^ permalink raw reply [flat|nested] 20+ messages in thread
* RE: [PATCH 1/1] tty: serial: samsung: add spin_lock for interrupt and console_write @ 2022-04-05 6:40 ` Jaewon Kim 0 siblings, 0 replies; 20+ messages in thread From: Jaewon Kim @ 2022-04-05 6:40 UTC (permalink / raw) To: 'Greg Kroah-Hartman' Cc: 'Krzysztof Kozlowski', 'Alim Akhtar', 'Jiri Slaby', linux-samsung-soc, linux-arm-kernel, linux-serial, linux-kernel, 'Chanho Park' Hello On 22. 4. 5. 14:01, Greg Kroah-Hartman wrote: > On Tue, Apr 05, 2022 at 12:38:54PM +0900, Jaewon Kim wrote: > > The console_write and IRQ handler can run concurrently. > > Problems may occurs console_write is continuously executed while the > > IRQ handler is running. > > > > Signed-off-by: Jaewon Kim <jaewon02.kim@samsung.com> > > --- > > drivers/tty/serial/samsung_tty.c | 14 ++++++++++++++ > > 1 file changed, 14 insertions(+) > > What commit does this fix? This is not an issue caused by anohter commits. There was potential issue from the beginning. Other drivers were fixed, but samsung_tty was not. PL011 patch : https://lkml.org/lkml/2012/2/1/495 > > > > > diff --git a/drivers/tty/serial/samsung_tty.c > > b/drivers/tty/serial/samsung_tty.c > > index e1585fbae909..d362e8e114f1 100644 > > --- a/drivers/tty/serial/samsung_tty.c > > +++ b/drivers/tty/serial/samsung_tty.c > > @@ -2480,12 +2480,26 @@ s3c24xx_serial_console_write(struct console *co, const char *s, > > unsigned int count) > > { > > unsigned int ucon = rd_regl(cons_uart, S3C2410_UCON); > > + unsigned long flags; > > + int locked = 1; > > bool? It is return value of spin_trylock() I used int because mose drivers used int. If you guide to change int to bool, I will change it. > > > > > /* not possible to xmit on unconfigured port */ > > if (!s3c24xx_port_configured(ucon)) > > return; > > > > + local_irq_save(flags); > > + if (cons_uart->sysrq) > > + locked = 0; > > + else if (oops_in_progress) > > + locked = spin_trylock(&cons_uart->lock); > > + else > > + spin_lock(&cons_uart->lock); > > + > > uart_console_write(cons_uart, s, count, > > s3c24xx_serial_console_putchar); > > + > > + if (locked) > > + spin_unlock(&cons_uart->lock); > > + local_irq_restore(flags); > > Why is irq_save required as well as a spinlock? No special reason. I will change spin_trylock() -? spin_trylock_irqsave(). spin_lock -> spin_lock_irqsave(). And, remove local_irq_save/restore. It looks more clean. > > thanks, > > greg k-h Thanks Jaewon Kim _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2022-04-06 13:48 UTC | newest] Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <CGME20220406081823epcas2p3789ef3ac3956e4713dd1b55c8a49fb05@epcas2p3.samsung.com> 2022-04-06 8:22 ` [PATCH v2 0/1] tty: serial: samsung: add spin_lock in console_write Jaewon Kim 2022-04-06 8:22 ` Jaewon Kim [not found] ` <CGME20220406081823epcas2p2f7afa27e2402c4fc02c9bee5972bed4f@epcas2p2.samsung.com> 2022-04-06 8:22 ` [PATCH 1/1] tty: serial: samsung: add spin_lock for interrupt and console_write Jaewon Kim 2022-04-06 8:22 ` Jaewon Kim 2022-04-06 8:21 ` Greg Kroah-Hartman 2022-04-06 8:21 ` Greg Kroah-Hartman 2022-04-06 8:39 ` Jaewon Kim 2022-04-06 8:39 ` Jaewon Kim 2022-04-06 9:48 ` Jiri Slaby 2022-04-06 9:48 ` Jiri Slaby 2022-04-06 11:19 ` Jaewon Kim 2022-04-06 11:19 ` Jaewon Kim 2022-04-06 11:21 ` Jiri Slaby 2022-04-06 11:21 ` Jiri Slaby 2022-04-05 3:38 [PATCH 0/1] tty: serial: samsung: add spin_lock in console_write Jaewon Kim [not found] ` <CGME20220405033448epcas2p397080e15c54369d24eaf94c2a27bd06c@epcas2p3.samsung.com> 2022-04-05 3:38 ` [PATCH 1/1] tty: serial: samsung: add spin_lock for interrupt and console_write Jaewon Kim 2022-04-05 3:38 ` Jaewon Kim 2022-04-05 5:01 ` Greg Kroah-Hartman 2022-04-05 5:01 ` Greg Kroah-Hartman 2022-04-05 6:40 ` Jaewon Kim 2022-04-05 6:40 ` Jaewon Kim
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.