diff mbox

[v2] core/console: refactor __flush_console()

Message ID 1469420185-24862-1-git-send-email-oohall@gmail.com
State Accepted
Headers show

Commit Message

Oliver O'Halloran July 25, 2016, 4:16 a.m. UTC 
Simplifies the flushing logic so that we only call into
con_driver->write() once. The existing implementation splits the
function into a normal path and a separate path when the in memory
console has wrapped. The logic is the same in both branches and
__flush_console() has enough bizarre crap happening with it's
not-a-lock-but-actually-a-lock flag variable.

Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
Spelling-corrected-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
---
 core/console.c | 49 +++++++++++++++++++++++++------------------------
 1 file changed, 25 insertions(+), 24 deletions(-)

Comments

Stewart Smith Feb. 23, 2017, 4:04 a.m. UTC | #1 
Oliver O'Halloran <oohall@gmail.com> writes:
> Simplifies the flushing logic so that we only call into
> con_driver->write() once. The existing implementation splits the
> function into a normal path and a separate path when the in memory
> console has wrapped. The logic is the same in both branches and
> __flush_console() has enough bizarre crap happening with it's
> not-a-lock-but-actually-a-lock flag variable.
>
> Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
> Spelling-corrected-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
> ---
>  core/console.c | 49 +++++++++++++++++++++++++------------------------
>  1 file changed, 25 insertions(+), 24 deletions(-)

So, I seem to be adequately convinced this is okay and merged it to
master as of c55f3f83789fdc865d7052beaab5ebedea55b042.

Now to wipe up the tears after looking at __flush_console() again.
diff mbox

Patch

diff --git a/core/console.c b/core/console.c
index 730ac245fd94..73ee922b0a50 100644
--- a/core/console.c
+++ b/core/console.c
@@ -151,35 +151,36 @@  bool __flush_console(bool flush_to_drivers)
 	}
 	in_flush = true;
 
+	/*
+	 * NB: this must appear after the in_flush check since it modifies
+	 *     con_out.
+	 */
+	if (!flush_to_drivers) {
+		con_out = con_in;
+		in_flush = false;
+		return false;
+	}
+
 	do {
 		more_flush = false;
+
 		if (con_out > con_in) {
 			req = INMEM_CON_OUT_LEN - con_out;
-			if (!flush_to_drivers) {
-				len = req;
-			} else {
-				unlock(&con_lock);
-				len = con_driver->write(con_buf + con_out,
-							req);
-				lock(&con_lock);
-			}
-			con_out = (con_out + len) % INMEM_CON_OUT_LEN;
-			if (len < req)
-				goto bail;
-		}
-		if (con_out < con_in) {
-			if (!flush_to_drivers) {
-				len = con_in - con_out;
-			} else {
-				unlock(&con_lock);
-				len = con_driver->write(con_buf + con_out,
-							con_in - con_out);
-				lock(&con_lock);
-			}
-			con_out = (con_out + len) % INMEM_CON_OUT_LEN;
-		}
+			more_flush = true;
+		} else
+			req = con_in - con_out;
+
+		unlock(&con_lock);
+		len = con_driver->write(con_buf + con_out, req);
+		lock(&con_lock);
+
+		con_out = (con_out + len) % INMEM_CON_OUT_LEN;
+
+		/* write error? */
+		if (len < req)
+			break;
 	} while(more_flush);
-bail:
+
 	in_flush = false;
 	return con_out != con_in;
 }