| Message ID | 20180409034954.31110-1-npiggin@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | uart: fix uart_opal_flush to take console lock over uart_con_flush | expand |
On Mon, 2018-04-09 at 13:49 +1000, Nicholas Piggin wrote: > Cc: Russell Currey <ruscur@russell.cc> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com> Reviewed-by: Russell Currey <ruscur@russell.cc>
Nicholas Piggin <npiggin@gmail.com> writes: > Cc: Russell Currey <ruscur@russell.cc> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com> > --- > hw/lpc-uart.c | 10 ++++++++-- > 1 file changed, 8 insertions(+), 2 deletions(-) Whoops! Merged to master as of 23dc884f8a0f0b52d3e9312b64b9b02fcd8453a1. I'm kinda going umm-ahh on if should pick back to any stable branch. Maybe? Have we seen an actual problem because of it? Will we?
On Mon, 30 Apr 2018 17:42:20 +1000 Stewart Smith <stewart@linux.ibm.com> wrote: > Nicholas Piggin <npiggin@gmail.com> writes: > > Cc: Russell Currey <ruscur@russell.cc> > > Signed-off-by: Nicholas Piggin <npiggin@gmail.com> > > --- > > hw/lpc-uart.c | 10 ++++++++-- > > 1 file changed, 8 insertions(+), 2 deletions(-) > > Whoops! Merged to master as of 23dc884f8a0f0b52d3e9312b64b9b02fcd8453a1. > > I'm kinda going umm-ahh on if should pick back to any stable > branch. Maybe? Have we seen an actual problem because of it? Will we? > Yeah it would be good if we can. We haven't seen a problem but Linux rarely uses the call at the moment. I'd like to start using it more, but I don't know how advisable that would be for the foreseeable future :( Thanks, Nick
Nicholas Piggin <npiggin@gmail.com> writes: > On Mon, 30 Apr 2018 17:42:20 +1000 > Stewart Smith <stewart@linux.ibm.com> wrote: > >> Nicholas Piggin <npiggin@gmail.com> writes: >> > Cc: Russell Currey <ruscur@russell.cc> >> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com> >> > --- >> > hw/lpc-uart.c | 10 ++++++++-- >> > 1 file changed, 8 insertions(+), 2 deletions(-) >> >> Whoops! Merged to master as of 23dc884f8a0f0b52d3e9312b64b9b02fcd8453a1. >> >> I'm kinda going umm-ahh on if should pick back to any stable >> branch. Maybe? Have we seen an actual problem because of it? Will we? >> > > Yeah it would be good if we can. We haven't seen a problem but Linux > rarely uses the call at the moment. I'd like to start using it more, > but I don't know how advisable that would be for the foreseeable > future :( We could always create a FLUSH2 call which is "flush, but with the locking bug fixed" and that way Linux could know that it's not going to be buggy and miss the lock.
diff --git a/hw/lpc-uart.c b/hw/lpc-uart.c index 3224de9f..74a86b13 100644 --- a/hw/lpc-uart.c +++ b/hw/lpc-uart.c @@ -218,7 +218,7 @@ static uint8_t *out_buf; static uint32_t out_buf_prod; static uint32_t out_buf_cons; -/* Asynchronous flush */ +/* Asynchronous flush, uart_lock must be held */ static int64_t uart_con_flush(void) { bool tx_was_full = tx_full; @@ -388,10 +388,16 @@ static int64_t uart_opal_read(int64_t term_number, int64_t *length, static int64_t uart_opal_flush(int64_t term_number) { + int64_t rc; + if (term_number != 0) return OPAL_PARAMETER; - return uart_con_flush(); + lock(&uart_lock); + rc = uart_con_flush(); + unlock(&uart_lock); + + return rc; } static void __uart_do_poll(u8 trace_ctx)
Cc: Russell Currey <ruscur@russell.cc> Signed-off-by: Nicholas Piggin <npiggin@gmail.com> --- hw/lpc-uart.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)