| Message ID | 20220406082216.11206-2-jaewon02.kim@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | tty: serial: samsung: add spin_lock in console_write | expand |
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 :)
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
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,
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
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.)
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 */
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(+)