Patch Detail
get:
Show a patch.
patch:
Update a patch.
put:
Update a patch.
GET /api/patches/761263/?format=api
{ "id": 761263, "url": "http://patchwork.ozlabs.org/api/patches/761263/?format=api", "web_url": "http://patchwork.ozlabs.org/project/skiboot/patch/20170511175856.20586-1-npiggin@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": "<20170511175856.20586-1-npiggin@gmail.com>", "list_archive_url": null, "date": "2017-05-11T17:58:56", "name": "[RFC] wait loop improvements", "commit_ref": null, "pull_url": null, "state": "superseded", "archived": false, "hash": "df6631ecad160c7dac1b3333913021e289dc83a9", "submitter": { "id": 69518, "url": "http://patchwork.ozlabs.org/api/people/69518/?format=api", "name": "Nicholas Piggin", "email": "npiggin@gmail.com" }, "delegate": null, "mbox": "http://patchwork.ozlabs.org/project/skiboot/patch/20170511175856.20586-1-npiggin@gmail.com/mbox/", "series": [], "comments": "http://patchwork.ozlabs.org/api/patches/761263/comments/", "check": "pending", "checks": "http://patchwork.ozlabs.org/api/patches/761263/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 3wP18C4Wv2z9s06\n\tfor <incoming@patchwork.ozlabs.org>;\n\tFri, 12 May 2017 03:59:23 +1000 (AEST)", "from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3])\n\tby lists.ozlabs.org (Postfix) with ESMTP id 3wP18C2sM6zDqcM\n\tfor <incoming@patchwork.ozlabs.org>;\n\tFri, 12 May 2017 03:59:23 +1000 (AEST)", "from mail-pg0-x241.google.com (mail-pg0-x241.google.com\n\t[IPv6:2607:f8b0:400e:c05::241])\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 3wP1805d2CzDqZf\n\tfor <skiboot@lists.ozlabs.org>; Fri, 12 May 2017 03:59:11 +1000 (AEST)", "by mail-pg0-x241.google.com with SMTP id u187so4406543pgb.1\n\tfor <skiboot@lists.ozlabs.org>; Thu, 11 May 2017 10:59:11 -0700 (PDT)", "from roar.au.ibm.com ([210.185.118.93])\n\tby smtp.gmail.com with ESMTPSA id\n\tq128sm1191986pfb.74.2017.05.11.10.59.07\n\t(version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);\n\tThu, 11 May 2017 10:59:08 -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=\"Tj6ElMHt\"; 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=\"Tj6ElMHt\"; dkim-atps=neutral", "lists.ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"Tj6ElMHt\"; 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;\n\tbh=Pa8l8j1hng3TPd2mdFdNy/qXjoWfzkfvSNOdxURvniA=;\n\tb=Tj6ElMHtChHKoKPPARk0931E19LVtODbkAzcltIlI3qY6cwZwzBArq1irt6sT1/0+B\n\tpLXr7oSiEkruJjrvDIXVeiJo8bBiWkhmflzSJ0NPYD+BmfkzdwMgkFR0rEDQA7n9F2SI\n\tTe5tDinLeqqFPUbselQBs4ItXCWdwhnS/qdajowgIn7RRI5DJul2V5Y4+5o8ZQjFqO3R\n\tgDATZeOK6qIDg5zhu2/S89sZtHOa/yg0ga2dSQXp81z7N3QaLfNWaByivtMOhlnCNQzI\n\t2nVHxZg9V2ZvfhamEMc7Jt5oAf16HHTSsLdO/8H/JqR1AQA1rsRPNe4uTysb3Qpk60HY\n\t/gIw==", "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;\n\tbh=Pa8l8j1hng3TPd2mdFdNy/qXjoWfzkfvSNOdxURvniA=;\n\tb=o/oqAPFwi7ZBr3a6GSbUtMKYuYWXdkKq9AUWkOEohvSgviGQ4x613KysYx318EGCmY\n\tuwMRqXxej4hCQLx5B60UN0YprZq2C9Ycxj80cRObnUBlndwvmXsGedxRbiH5HJDL+0cS\n\t7CopNGtPY6alW4jXEfmMzpol0yLn2WrCI+4XSaSc3+Kv17I6AUp3ZlPO3bFQhUH3ZAif\n\t2kFbliwxU26XTCSgFPnEYsik3z5LZ46qYpWvmadnFPuAWGbzUBaS/MY0mkahvYxF5WhU\n\t3lMNjG6IP3fe0JcCe4329abio2bcOpC74Zc3mP7hTswRQQ7kxEZoY6w5l8THlQOr3V+s\n\tQPlQ==", "X-Gm-Message-State": "AODbwcA3PhIS9yhFk4TWVAJ6HIbMR8NgHIWlB+vsqb5Elcbbkf27yAVf\n\tilClTpaH7twt8A==", "X-Received": "by 10.99.96.3 with SMTP id u3mr20022pgb.69.1494525549825;\n\tThu, 11 May 2017 10:59:09 -0700 (PDT)", "From": "Nicholas Piggin <npiggin@gmail.com>", "To": "skiboot@lists.ozlabs.org", "Date": "Fri, 12 May 2017 03:58:56 +1000", "Message-Id": "<20170511175856.20586-1-npiggin@gmail.com>", "X-Mailer": "git-send-email 2.11.0", "Subject": "[Skiboot] [RFC][PATCH] wait loop improvements", "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": "This patch has a few improvements to wait loops. Primarily two\nthings:\n\n- Running busy-wait loops entirely in lowest SMT priority rather than\n calling cpu_relax(), which is not all that suitable an interface\n for the job. This gives other threads more cycles to do useful work.\n\n- Reducing the amount of unnecessary timeout waits. These take quite\n a while to run in the simulator.\n\nThese reduce time to boot a 2 core, 16 thread POWER8 sim to Linux\nuserspace from about 43 seconds on my laptop to 13. Haven't tested on\nreal hardware -- one possibly dodgy thing is changing HILE concurrently,\nbut there doesn't seem to be any other protection against that.\n\nJust a request for comments at the moment. The long SMP boot times\nin the simulator were bugging me so I can clean this up and split\nit into pieces if there is interest to merge it. (might be some small\ngains on hardware here and there, but unlikely to be too significant I\nthink)\n\nThanks,\nNick\n---\n core/cpu.c | 50 ++++++++++++++++++++++++++++++++++++++++++++------\n core/hmi.c | 4 +++-\n core/init.c | 4 ++--\n core/lock.c | 5 ++++-\n core/timebase.c | 26 +++++++-------------------\n core/timer.c | 5 ++++-\n hw/lpc-uart.c | 10 +++++++---\n include/cpu.h | 1 +\n include/processor.h | 2 ++\n 9 files changed, 74 insertions(+), 33 deletions(-)", "diff": "diff --git a/core/cpu.c b/core/cpu.c\nindex c7e650da..8c24cbfc 100644\n--- a/core/cpu.c\n+++ b/core/cpu.c\n@@ -289,10 +289,15 @@ void cpu_process_jobs(void)\n static void cpu_idle_default(enum cpu_wake_cause wake_on __unused)\n {\n \t/* Maybe do something better for simulators ? */\n-\tcpu_relax();\n-\tcpu_relax();\n-\tcpu_relax();\n-\tcpu_relax();\n+\tif (wake_on == cpu_wake_on_job) {\n+\t\tstruct cpu_thread *cpu = this_cpu();\n+\n+\t\tsmt_lowest();\n+\t\t/* Check for jobs again */\n+\t\twhile (!cpu_check_jobs(cpu))\n+\t\t\tbarrier();\n+\t\tsmt_medium();\n+\t}\n }\n \n static void cpu_idle_p8(enum cpu_wake_cause wake_on)\n@@ -385,6 +390,28 @@ void cpu_idle(enum cpu_wake_cause wake_on)\n \t}\n }\n \n+void cpu_idle_delay(unsigned long delay, unsigned long min_pm)\n+{\n+\tunsigned long end = mftb() + delay;\n+\n+\tif (pm_enabled && delay > min_pm) {\n+\t\tfor (;;) {\n+\t\t\tif (delay >= 0x7fffffff)\n+\t\t\t\tdelay = 0x7fffffff;\n+\t\t\tmtspr(SPR_DEC, delay);\n+\t\t\tcpu_idle(cpu_wake_on_dec);\n+\n+\t\t\tif (tb_compare(mftb(), end) == TB_AAFTERB)\n+\t\t\t\tbreak;\n+\t\t}\n+\t} else {\n+\t\tsmt_lowest();\n+\t\twhile (tb_compare(mftb(), end) != TB_AAFTERB)\n+\t\t\tbarrier();\n+\t\tsmt_medium();\n+\t}\n+}\n+\n void cpu_process_local_jobs(void)\n {\n \tstruct cpu_thread *cpu = first_available_cpu();\n@@ -1000,6 +1027,7 @@ static void cpu_change_hile(void *hilep)\n static int64_t cpu_change_all_hile(bool hile)\n {\n \tstruct cpu_thread *cpu;\n+\tbool done;\n \n \tprlog(PR_INFO, \"CPU: Switching HILE on all CPUs to %d\\n\", hile);\n \n@@ -1010,9 +1038,19 @@ static int64_t cpu_change_all_hile(bool hile)\n \t\t\tcpu_change_hile(&hile);\n \t\t\tcontinue;\n \t\t}\n-\t\tcpu_wait_job(cpu_queue_job(cpu, \"cpu_change_hile\",\n-\t\t\t\t\t cpu_change_hile, &hile), true);\n+\t\tcpu_queue_job(cpu, \"cpu_change_hile\", cpu_change_hile, &hile);\n \t}\n+\n+\tsmt_lowest();\n+\tdo {\n+\t\tdone = true;\n+\t\tfor_each_available_cpu(cpu) {\n+\t\t\tif (cpu->current_hile != hile)\n+\t\t\t\tdone = false;\n+\t\t}\n+\t} while (!done);\n+\tsmt_medium();\n+\n \treturn OPAL_SUCCESS;\n }\n \ndiff --git a/core/hmi.c b/core/hmi.c\nindex e55a85b2..06d83476 100644\n--- a/core/hmi.c\n+++ b/core/hmi.c\n@@ -599,6 +599,7 @@ static void wait_for_subcore_threads(void)\n {\n \tuint64_t timeout = 0;\n \n+\tsmt_lowest();\n \twhile (!(*(this_cpu()->core_hmi_state_ptr) & HMI_STATE_CLEANUP_DONE)) {\n \t\t/*\n \t\t * We use a fixed number of TIMEOUT_LOOPS rather\n@@ -616,8 +617,9 @@ static void wait_for_subcore_threads(void)\n \t\t\tprlog(PR_DEBUG, \"HMI: TB pre-recovery timeout\\n\");\n \t\t\tbreak;\n \t\t}\n-\t\tcpu_relax();\n+\t\tbarrier();\n \t}\n+\tsmt_medium();\n }\n \n /*\ndiff --git a/core/init.c b/core/init.c\nindex dce10fd6..434b4ff3 100644\n--- a/core/init.c\n+++ b/core/init.c\n@@ -1068,9 +1068,9 @@ void __noreturn __secondary_cpu_entry(void)\n \t/* Wait for work to do */\n \twhile(true) {\n \t\tif (cpu_check_jobs(cpu))\n-\t\t cpu_process_jobs();\n+\t\t\tcpu_process_jobs();\n \t\telse\n-\t\t cpu_idle(cpu_wake_on_job);\n+\t\t\tcpu_idle(cpu_wake_on_job);\n \t}\n }\n \ndiff --git a/core/lock.c b/core/lock.c\nindex e82048bf..0868f2ba 100644\n--- a/core/lock.c\n+++ b/core/lock.c\n@@ -93,7 +93,10 @@ void lock(struct lock *l)\n \tfor (;;) {\n \t\tif (try_lock(l))\n \t\t\tbreak;\n-\t\tcpu_relax();\n+\t\tsmt_lowest();\n+\t\twhile (l->lock_val)\n+\t\t\tbarrier();\n+\t\tsmt_medium();\n \t}\n }\n \ndiff --git a/core/timebase.c b/core/timebase.c\nindex a3c0fec5..6c2f656e 100644\n--- a/core/timebase.c\n+++ b/core/timebase.c\n@@ -24,8 +24,8 @@ unsigned long tb_hz = 512000000;\n \n static void time_wait_poll(unsigned long duration)\n {\n-\tunsigned long remaining = duration;\n-\tunsigned long end = mftb() + duration;\n+\tunsigned long now = mftb();\n+\tunsigned long end = now + duration;\n \tunsigned long period = msecs_to_tb(5);\n \n \tif (this_cpu()->tb_invalid) {\n@@ -33,7 +33,9 @@ static void time_wait_poll(unsigned long duration)\n \t\treturn;\n \t}\n \n-\twhile (tb_compare(mftb(), end) != TB_AAFTERB) {\n+\twhile (tb_compare(now, end) != TB_AAFTERB) {\n+\t\tunsigned long remaining = now - end;\n+\n \t\t/* Call pollers periodically but not continually to avoid\n \t\t * bouncing cachelines due to lock contention. */\n \t\tif (remaining >= period) {\n@@ -43,7 +45,7 @@ static void time_wait_poll(unsigned long duration)\n \t\t} else\n \t\t\ttime_wait_nopoll(remaining);\n \n-\t\tcpu_relax();\n+\t\tnow = mftb();\n \t}\n }\n \n@@ -64,7 +66,6 @@ void time_wait(unsigned long duration)\n \n void time_wait_nopoll(unsigned long duration)\n {\n-\tunsigned long end = mftb() + duration;\n \tunsigned long min = usecs_to_tb(10);\n \n \tif (this_cpu()->tb_invalid) {\n@@ -72,20 +73,7 @@ void time_wait_nopoll(unsigned long duration)\n \t\treturn;\n \t}\n \n-\tfor (;;) {\n-\t\tuint64_t delay, tb = mftb();\n-\n-\t\tif (tb_compare(tb, end) == TB_AAFTERB)\n-\t\t\tbreak;\n-\t\tdelay = end - tb;\n-\t\tif (delay >= 0x7fffffff)\n-\t\t\tdelay = 0x7fffffff;\n-\t\tif (delay >= min) {\n-\t\t\tmtspr(SPR_DEC, delay);\n-\t\t\tcpu_idle(cpu_wake_on_dec);\n-\t\t} else\n-\t\t\tcpu_relax();\n-\t}\n+\tcpu_idle_delay(duration, min);\n }\n \n void time_wait_ms(unsigned long ms)\ndiff --git a/core/timer.c b/core/timer.c\nindex 75489963..4b5925fe 100644\n--- a/core/timer.c\n+++ b/core/timer.c\n@@ -45,7 +45,10 @@ static void __sync_timer(struct timer *t)\n \n \twhile (t->running) {\n \t\tunlock(&timer_lock);\n-\t\tcpu_relax();\n+\t\tsmt_lowest();\n+\t\twhile (t->running)\n+\t\t\tbarrier();\n+\t\tsmt_medium();\n \t\t/* Should we call the pollers here ? */\n \t\tlock(&timer_lock);\n \t}\ndiff --git a/hw/lpc-uart.c b/hw/lpc-uart.c\nindex 6e349ef9..912bffa5 100644\n--- a/hw/lpc-uart.c\n+++ b/hw/lpc-uart.c\n@@ -115,10 +115,14 @@ static void uart_check_tx_room(void)\n \n static void uart_wait_tx_room(void)\n {\n-\twhile(!tx_room) {\n+\twhile (!tx_room) {\n \t\tuart_check_tx_room();\n-\t\tif (!tx_room)\n-\t\t\tcpu_relax();\n+\t\tif (!tx_room) {\n+\t\t\tsmt_lowest();\n+\t\t\twhile (!tx_room)\n+\t\t\t\tbarrier();\n+\t\t\tsmt_medium();\n+\t\t}\n \t}\n }\n \ndiff --git a/include/cpu.h b/include/cpu.h\nindex 1e147aa8..4c58e34a 100644\n--- a/include/cpu.h\n+++ b/include/cpu.h\n@@ -276,5 +276,6 @@ extern unsigned long __attrconst cpu_stack_bottom(unsigned int pir);\n extern unsigned long __attrconst cpu_stack_top(unsigned int pir);\n \n extern void cpu_idle(enum cpu_wake_cause wake_on);\n+extern void cpu_idle_delay(unsigned long delay, unsigned long min_pm);\n \n #endif /* __CPU_H */\ndiff --git a/include/processor.h b/include/processor.h\nindex b98dff8d..5906b865 100644\n--- a/include/processor.h\n+++ b/include/processor.h\n@@ -197,6 +197,7 @@\n #define smt_medium_low\tor 6,6,6\n #define smt_extra_high\tor 7,7,7\n #define smt_very_low\tor 31,31,31\n+#define smt_lowest\tsmt_low ; smt_very_low\n \n #else /* __ASSEMBLY__ */\n \n@@ -214,6 +215,7 @@ static inline void smt_medium_high(void){ asm volatile(\"or 5,5,5\");\t}\n static inline void smt_medium_low(void)\t{ asm volatile(\"or 6,6,6\");\t}\n static inline void smt_extra_high(void)\t{ asm volatile(\"or 7,7,7\");\t}\n static inline void smt_very_low(void)\t{ asm volatile(\"or 31,31,31\");\t}\n+static inline void smt_lowest(void)\t{ smt_low(); smt_very_low();\t}\n \n /*\n * SPR access functions\n", "prefixes": [ "RFC" ] }