Patch Detail
get:
Show a patch.
patch:
Update a patch.
put:
Update a patch.
GET /api/patches/777582/?format=api
{ "id": 777582, "url": "http://patchwork.ozlabs.org/api/patches/777582/?format=api", "web_url": "http://patchwork.ozlabs.org/project/skiboot/patch/20170619060138.5314-5-oohall@gmail.com/", "project": { "id": 44, "url": "http://patchwork.ozlabs.org/api/projects/44/?format=api", "name": "skiboot firmware development", "link_name": "skiboot", "list_id": "skiboot.lists.ozlabs.org", "list_email": "skiboot@lists.ozlabs.org", "web_url": "http://github.com/open-power/skiboot", "scm_url": "http://github.com/open-power/skiboot", "webscm_url": "", "list_archive_url": "", "list_archive_url_format": "", "commit_url_format": "" }, "msgid": "<20170619060138.5314-5-oohall@gmail.com>", "list_archive_url": null, "date": "2017-06-19T06:01:37", "name": "[5/6] console: rework flushing to the console driver", "commit_ref": null, "pull_url": null, "state": "changes-requested", "archived": false, "hash": "c74a1297ddbc3e238deac441a14bf75c52e144d2", "submitter": { "id": 68108, "url": "http://patchwork.ozlabs.org/api/people/68108/?format=api", "name": "Oliver O'Halloran", "email": "oohall@gmail.com" }, "delegate": null, "mbox": "http://patchwork.ozlabs.org/project/skiboot/patch/20170619060138.5314-5-oohall@gmail.com/mbox/", "series": [], "comments": "http://patchwork.ozlabs.org/api/patches/777582/comments/", "check": "pending", "checks": "http://patchwork.ozlabs.org/api/patches/777582/checks/", "tags": {}, "related": [], "headers": { "Return-Path": "<skiboot-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org>", "X-Original-To": [ "incoming@patchwork.ozlabs.org", "skiboot@lists.ozlabs.org" ], "Delivered-To": [ "patchwork-incoming@bilbo.ozlabs.org", "skiboot@lists.ozlabs.org" ], "Received": [ "from lists.ozlabs.org (lists.ozlabs.org [103.22.144.68])\n\t(using TLSv1.2 with cipher ADH-AES256-GCM-SHA384 (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3wrgQC6TjFz9s76\n\tfor <incoming@patchwork.ozlabs.org>;\n\tMon, 19 Jun 2017 16:03:31 +1000 (AEST)", "from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3])\n\tby lists.ozlabs.org (Postfix) with ESMTP id 3wrgQC5KqTzDqDF\n\tfor <incoming@patchwork.ozlabs.org>;\n\tMon, 19 Jun 2017 16:03:31 +1000 (AEST)", "from mail-pg0-x243.google.com (mail-pg0-x243.google.com\n\t[IPv6:2607:f8b0:400e:c05::243])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128\n\tbits)) (No client certificate requested)\n\tby lists.ozlabs.org (Postfix) with ESMTPS id 3wrgPM0NFSzDqgf\n\tfor <skiboot@lists.ozlabs.org>; Mon, 19 Jun 2017 16:02:47 +1000 (AEST)", "by mail-pg0-x243.google.com with SMTP id e187so5449894pgc.3\n\tfor <skiboot@lists.ozlabs.org>; Sun, 18 Jun 2017 23:02:46 -0700 (PDT)", "from flat-canetoad.ozlabs.ibm.com ([122.99.82.10])\n\tby smtp.gmail.com with ESMTPSA id\n\tx64sm19638626pff.123.2017.06.18.23.02.43\n\t(version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);\n\tSun, 18 Jun 2017 23:02:44 -0700 (PDT)" ], "Authentication-Results": [ "ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"WMad+Nak\"; dkim-atps=neutral", "lists.ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"WMad+Nak\"; dkim-atps=neutral", "lists.ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"WMad+Nak\"; dkim-atps=neutral" ], "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025;\n\th=from:to:cc:subject:date:message-id:in-reply-to:references;\n\tbh=B9ntuELt+MPote0hvuhtQiUnEvDUd3mykEnhkkEuM38=;\n\tb=WMad+Nakliac4p9VlI1pIBR2zY9oWRamlUHJnDo0TU32JOWaAcYR4Bd2WbH9ea5Qkb\n\t6sKFJQX4LVXu57q0b2JYYhfy8HUXOBWztc2YbUv99JY3e6iOrqFZhfF8fbVgWxACGsHH\n\tvxnlfVgIUvLsqTb/vzermr6uEqPLukvRNAuRsluGlhQI2wgJxEoTyuUslvVcrtZ7wi5e\n\tRPhPg8jMBkhlqebDEbuaQcBpA6+khjxVbBtlXAYqYjWvwqT5uA7T8OTJQHJx3zWYjNe+\n\tVMSNf7Z/A2NLgjog/wHyChP9H1GR4leyYjU03wH9wP41Wis0kH+mhDJ6sIiXVZQaAV7a\n\tPxBg==", "X-Google-DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to\n\t:references;\n\tbh=B9ntuELt+MPote0hvuhtQiUnEvDUd3mykEnhkkEuM38=;\n\tb=bO3TIedQy3X26IrQUEvjCdir+jeOr/ZbYfBz4BeU1IljALhryqyJo3cozni1mKwIlP\n\tu9SGwQ33exgmqXBUIIA+V5Ivk7MS6nUp3Xm1v+X1ErAM15q2bBDAjXJmVm9SmLc7iMoS\n\thJUm/pDjf9jit3cmIJl23i860sIgzceuiRX3LzVko0bOu0h4X9QJQysovUT9WVbk2MPB\n\tFXXqlKn3kFcysuxXOsxhizgvpAY97ltlknqsElM/6jchQe2adQWVOA11Ois62XdasJQq\n\taRDhP7uA8optOwqzaozRA2vTnVEoyj/2J9umCsKK/D7VJ8NrR3tywUMX7G4bP9B4KGI9\n\ttpfw==", "X-Gm-Message-State": "AKS2vOzbnrhOtHhIi8Zb3/4oXjVaS7C0/mMbl/RUN3OqP0olkPBN4aSe\n\tX5F3WsZt1mSu0LBF", "X-Received": "by 10.84.151.99 with SMTP id i90mr27879974pli.81.1497852165026; \n\tSun, 18 Jun 2017 23:02:45 -0700 (PDT)", "From": "Oliver O'Halloran <oohall@gmail.com>", "To": "skiboot@lists.ozlabs.org", "Date": "Mon, 19 Jun 2017 16:01:37 +1000", "Message-Id": "<20170619060138.5314-5-oohall@gmail.com>", "X-Mailer": "git-send-email 2.9.3", "In-Reply-To": "<20170619060138.5314-1-oohall@gmail.com>", "References": "<20170619060138.5314-1-oohall@gmail.com>", "Subject": "[Skiboot] [PATCH 5/6] console: rework flushing to the console driver", "X-BeenThere": "skiboot@lists.ozlabs.org", "X-Mailman-Version": "2.1.23", "Precedence": "list", "List-Id": "Mailing list for skiboot development <skiboot.lists.ozlabs.org>", "List-Unsubscribe": "<https://lists.ozlabs.org/options/skiboot>,\n\t<mailto:skiboot-request@lists.ozlabs.org?subject=unsubscribe>", "List-Archive": "<http://lists.ozlabs.org/pipermail/skiboot/>", "List-Post": "<mailto:skiboot@lists.ozlabs.org>", "List-Help": "<mailto:skiboot-request@lists.ozlabs.org?subject=help>", "List-Subscribe": "<https://lists.ozlabs.org/listinfo/skiboot>,\n\t<mailto:skiboot-request@lists.ozlabs.org?subject=subscribe>", "MIME-Version": "1.0", "Content-Type": "text/plain; charset=\"utf-8\"", "Content-Transfer-Encoding": "base64", "Errors-To": "skiboot-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org", "Sender": "\"Skiboot\"\n\t<skiboot-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org>" }, "content": "There has been a long standing bug in skiboot when a second thread\nwrites into the log buffer while another thread is currently flushing.\nIn this situation the second thread sets a flag to notify the flushing\nthread that there new data has been written into the log buffer.\nUnforunately the flushing thread loses the log level information when\nthis happens and as a result messages that should be filtered are\nwritten to the console. This patch reworks the flushing process so that\nthe flushing thread will parse the start of each log line to determine\nthe log level of that line and only flush when it should.\n\nSigned-off-by: Oliver O'Halloran <oohall@gmail.com>\n---\n core/console-log.c | 71 +++++++++++++++++++++++++++++++++++\n core/console.c | 106 +++++++++++++++++++++++++++++++++++++++++------------\n 2 files changed, 153 insertions(+), 24 deletions(-)", "diff": "diff --git a/core/console-log.c b/core/console-log.c\nindex 642b39ca7988..0b8cef53fc64 100644\n--- a/core/console-log.c\n+++ b/core/console-log.c\n@@ -26,6 +26,7 @@\n #include \"stdio.h\"\n #include \"console.h\"\n #include \"timebase.h\"\n+#include \"ctype.h\"\n \n static int vprlog(int log_level, const char *fmt, va_list ap)\n {\n@@ -44,6 +45,7 @@ static int vprlog(int log_level, const char *fmt, va_list ap)\n \tif (log_level > (debug_descriptor.console_log_levels >> 4))\n \t\treturn 0;\n \n+\t/* if this changes parse_loghdr() needs to be updated too */\n \tcount = snprintf(buffer, sizeof(buffer), \"[%5lu.%09lu,%d] \",\n \t\t\t tb_to_secs(tb), tb_remaining_nsecs(tb), log_level);\n \tcount+= vsnprintf(buffer+count, sizeof(buffer)-count, fmt, ap);\n@@ -56,6 +58,75 @@ static int vprlog(int log_level, const char *fmt, va_list ap)\n \treturn count;\n }\n \n+/*\n+ * This parses a log entry to find it's log-level. If the log entry does not\n+ * have a valid log header it'll return -1.\n+ *\n+ * d - run of decial digits\n+ * l - log level digit\n+ *\n+ * Other characters are literals\n+ */\n+static const char pattern[] = \"[d.d,l] \";\n+#define LOGHDR_MAX (24 * 2 + 4)\n+\n+/*\n+ * These functions live in console.c, but we need to access them from here.\n+ * For parsing the log header.\n+ */\n+\n+extern int conbuf_get(int offset);\n+\n+int __parse_loghdr(int offset, int *log_level);\n+int __parse_loghdr(int offset, int *log_level)\n+{\n+\tint i, run;\n+\n+\t*log_level = -1;\n+\n+\tfor (i = 0; pattern[i]; i++) {\n+\t\tint c = conbuf_get(offset);\n+\t\tif (c < 0 || c == '\\n')\n+\t\t\treturn offset;\n+\n+\t\tswitch (pattern[i]) {\n+\t\tcase 'd': /* match a run of digits/spaces */\n+\t\t\tfor (run = 0; ; offset++, run++) {\n+\t\t\t\tc = conbuf_get(offset);\n+\t\t\t\tif (c < 0 || c == '\\n' || run > LOGHDR_MAX)\n+\t\t\t\t\treturn offset;\n+\n+\t\t\t\tif (!isdigit(c) && !isspace(c))\n+\t\t\t\t\tbreak;\n+\t\t\t}\n+\n+\t\t\t/* didn't match the pattern */\n+\t\t\tif (!run)\n+\t\t\t\treturn offset;\n+\n+\t\t\tbreak;\n+\n+\t\tcase 'l': /* extract log level */\n+\t\t\tif (!isdigit(c))\n+\t\t\t\treturn offset;\n+\n+\t\t\t*log_level = c - '0';\n+\t\t\toffset++;\n+\n+\t\t\tbreak;\n+\n+\t\tdefault: /* match a literal */\n+\t\t\tif (pattern[i] != c)\n+\t\t\t\treturn offset;\n+\n+\t\t\toffset++;\n+\t\t}\n+\t}\n+\n+\treturn offset;\n+}\n+\n+\n /* we don't return anything as what on earth are we going to do\n * if we actually fail to print a log message? Print a log message about it?\n * Callers shouldn't care, prlog and friends should do something generically\ndiff --git a/core/console.c b/core/console.c\nindex b9129c9f3766..fbab5a38e21e 100644\n--- a/core/console.c\n+++ b/core/console.c\n@@ -27,6 +27,15 @@\n #include <processor.h>\n #include <cpu.h>\n \n+/*\n+ * ring buffer pointers\n+ *\n+ * con_in = offset the next character will be written to\n+ * con_out = offset of the next character to be flushed\n+ *\n+ * when con_in == con_out there is no data to be flushed\n+ */\n+\n static char *con_buf = (char *)INMEM_CON_START;\n static size_t con_in;\n static size_t con_out;\n@@ -95,16 +104,50 @@ void clear_console(void)\n }\n \n /*\n- * Flush the console buffer into the driver, returns true\n- * if there is more to go.\n- * Optionally can skip flushing to drivers, leaving messages\n- * just in memory console.\n+ * The prototypes here are because we want to use these in console-log.c\n+ * The log header parsing lives in there, but we still need access to the\n+ * log buffer.\n+ */\n+int conbuf_get(int offset);\n+int conbuf_get(int offset)\n+{\n+\toffset %= memcons.obuf_size;\n+\n+\tif (offset == con_in)\n+\t\treturn -1;\n+\n+\treturn con_buf[offset];\n+}\n+\n+extern int __parse_loghdr(int offset, int *log_level);\n+static int parse_loghdr(int start, int *log_level)\n+{\n+\tint offset = __parse_loghdr(start, log_level);\n+\n+\twhile (1) {\n+\t\tint c = conbuf_get(offset);\n+\n+\t\tif (c < 0)\n+\t\t\tbreak;\n+\t\toffset++;\n+\n+\t\tif (c == '\\n')\n+\t\t\tbreak;\n+\t}\n+\n+\treturn offset - start;\n+}\n+\n+/*\n+ * Flush the console buffer into the driver. Returns true\n+ * if there is more to go, but that only happens when the\n+ * underlying driver failed so don't call it again.\n */\n-static bool __flush_console(bool flush_to_drivers)\n+static bool __flush_console(bool flush_to_drivers __unused)\n {\n+\tint flush_lvl = debug_descriptor.console_log_levels & 0xf;\n \tstruct cpu_thread *cpu = this_cpu();\n-\tsize_t req, len = 0;\n-\tstatic bool in_flush, more_flush;\n+\tstatic bool in_flush;\n \n \t/* Is there anything to flush ? Bail out early if not */\n \tif (con_in == con_out || !con_driver)\n@@ -131,10 +174,9 @@ static bool __flush_console(bool flush_to_drivers)\n \t * concurrent attempts at flushing the same chunk of buffer\n \t * by other processors.\n \t */\n-\tif (in_flush) {\n-\t\tmore_flush = true;\n+\tif (in_flush)\n \t\treturn false;\n-\t}\n+\n \tin_flush = true;\n \n \t/*\n@@ -148,25 +190,41 @@ static bool __flush_console(bool flush_to_drivers)\n \t}\n \n \tdo {\n-\t\tmore_flush = false;\n-\n-\t\tif (con_out > con_in) {\n-\t\t\treq = INMEM_CON_OUT_LEN - con_out;\n-\t\t\tmore_flush = true;\n+\t\tint start, req, len, log_lvl;\n+\n+\t\tstart = con_out;\n+\n+\t\t/*\n+\t\t * NB: Input that does not start with a valid log skiboot\n+\t\t * log header is always flushed. This can happen due to\n+\t\t * writes into the dummy OPAL console or because a line\n+\t\t * was only partially flushed.\n+\t\t */\n+\t\treq = parse_loghdr(start, &log_lvl);\n+\t\tif (log_lvl <= flush_lvl) {\n+\t\t\t/*\n+\t\t\t * It this messages crosses the ring buffer edge then\n+\t\t\t * truncate and write the rest on the next iteration.\n+\t\t\t */\n+\t\t\tint end = (start + req) % memcons.obuf_size;\n+\t\t\tif (end < start)\n+\t\t\t\treq = memcons.obuf_size - start;\n+\n+\t\t\tunlock(&con_lock);\n+\t\t\tlen = con_driver->write(con_buf + con_out, req);\n+\t\t\tlock(&con_lock);\n \t\t} else\n-\t\t\treq = con_in - con_out;\n+\t\t\tlen = req;\n \n-\t\tunlock(&con_lock);\n-\t\tlen = con_driver->write(con_buf + con_out, req);\n-\t\tlock(&con_lock);\n+\t\tcon_out += len;\n+\t\tcon_out %= memcons.obuf_size;\n \n-\t\tcon_out = (con_out + len) % INMEM_CON_OUT_LEN;\n-\n-\t\t/* write error? */\n \t\tif (len < req)\n-\t\t\tbreak;\n-\t} while(more_flush);\n+\t\t\tgoto bail;\n+\n+\t} while (con_out != con_in);\n \n+bail:\n \tin_flush = false;\n \treturn con_out != con_in;\n }\n", "prefixes": [ "5/6" ] }