linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Lv Zheng <lv.zheng@intel.com>
To: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>,
	Len Brown <len.brown@intel.com>
Cc: Lv Zheng <lv.zheng@intel.com>, Lv Zheng <zetalog@gmail.com>,
	<linux-kernel@vger.kernel.org>,
	linux-acpi@vger.kernel.org
Subject: [PATCH v2 3/5] ACPI / EC: Refine command storm prevention support.
Date: Fri,  6 Feb 2015 08:58:05 +0800	[thread overview]
Message-ID: <c102e0e999906a7f38ad7dd6c9d8e708e33467ec.1423183648.git.lv.zheng@intel.com> (raw)
In-Reply-To: <cover.1423183647.git.lv.zheng@intel.com>

This patch refines EC command storm prevention support.

Current command storming code is wrong, when the storming condition is
detected, it only flags the condition without doing anything for the
current command but performing storming prevention for the follow-up
commands. So:
1. The first command which suffers from the storming still suffers from
   storming.
2. The follow-up commands which may not suffer from the storming are
   unconditionally forced into the storming prevention mode.
Ideally, we should only enable storm prevention immediately after detection
for the current command so that the next command can try the
power/performance efficient interrupt mode again.

This patch improves the command storm prevention by disabling GPE right
after the detection and re-enabling it right before completing the command
transaction using the GPE storming prevention APIs. This thus deploys the
following GPE handling model:
1. acpi_enable_gpe()/acpi_disable_gpe() for reference count changes:
   This set of APIs are used for EC usage reference counting.
2. acpi_set_gpe(ACPI_GPE_ENABLE)/acpi_set_gpe(ACPI_GPE_DISABLE):
   This set of APIs are used for preventing GPE storm. They must be invoked
   when the reference count > 0.
   Note that as the storming prevention should always happen when there is
   an outstanding request, or GPE enabling value will be messed up by the
   races. This patch also adds BUG_ON() to enforces this rule to prevent
   future bugs.

The msleep(1) used after completing a transaction is useless now as this
sounds like a guard time only useful for platforms that need the
EC_FLAGS_MSI quirks while we have fixed GPE race issues using the previous
raw handler mode enabling. It is kept to avoid regressions. A seperate
patch which deletes EC_FLAGS_MSI quirks should take care of deleting it.

Signed-off-by: Lv Zheng <lv.zheng@intel.com>
---
 drivers/acpi/ec.c |   55 +++++++++++++++++++++++++++++++++++------------------
 1 file changed, 36 insertions(+), 19 deletions(-)

diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
index 1fa1463..982b67f 100644
--- a/drivers/acpi/ec.c
+++ b/drivers/acpi/ec.c
@@ -77,11 +77,12 @@ enum ec_command {
 
 enum {
 	EC_FLAGS_QUERY_PENDING,		/* Query is pending */
-	EC_FLAGS_GPE_STORM,		/* GPE storm detected */
 	EC_FLAGS_HANDLERS_INSTALLED,	/* Handlers for GPE and
 					 * OpReg are installed */
 	EC_FLAGS_STARTED,		/* Driver is started */
 	EC_FLAGS_STOPPED,		/* Driver is stopped */
+	EC_FLAGS_COMMAND_STORM,		/* GPE storms occurred to the
+					 * current command processing */
 };
 
 #define ACPI_EC_COMMAND_POLL		0x01 /* Available for command byte */
@@ -229,8 +230,10 @@ static inline void acpi_ec_enable_gpe(struct acpi_ec *ec, bool open)
 {
 	if (open)
 		acpi_enable_gpe(NULL, ec->gpe);
-	else
+	else {
+		BUG_ON(ec->reference_count < 1);
 		acpi_set_gpe(NULL, ec->gpe, ACPI_GPE_ENABLE);
+	}
 	if (acpi_ec_is_gpe_raised(ec)) {
 		/*
 		 * On some platforms, EN=1 writes cannot trigger GPE. So
@@ -246,8 +249,10 @@ static inline void acpi_ec_disable_gpe(struct acpi_ec *ec, bool close)
 {
 	if (close)
 		acpi_disable_gpe(NULL, ec->gpe);
-	else
+	else {
+		BUG_ON(ec->reference_count < 1);
 		acpi_set_gpe(NULL, ec->gpe, ACPI_GPE_DISABLE);
+	}
 }
 
 static inline void acpi_ec_clear_gpe(struct acpi_ec *ec)
@@ -290,6 +295,24 @@ static void acpi_ec_complete_request(struct acpi_ec *ec)
 		wake_up(&ec->wait);
 }
 
+static void acpi_ec_set_storm(struct acpi_ec *ec, u8 flag)
+{
+	if (!test_bit(flag, &ec->flags)) {
+		acpi_ec_disable_gpe(ec, false);
+		pr_debug("+++++ Polling enabled +++++\n");
+		set_bit(flag, &ec->flags);
+	}
+}
+
+static void acpi_ec_clear_storm(struct acpi_ec *ec, u8 flag)
+{
+	if (test_bit(flag, &ec->flags)) {
+		clear_bit(flag, &ec->flags);
+		acpi_ec_enable_gpe(ec, false);
+		pr_debug("+++++ Polling disabled +++++\n");
+	}
+}
+
 /*
  * acpi_ec_submit_flushable_request() - Increase the reference count unless
  *                                      the flush operation is not in
@@ -404,8 +427,13 @@ err:
 	 * otherwise will take a not handled IRQ as a false one.
 	 */
 	if (!(status & ACPI_EC_FLAG_SCI)) {
-		if (in_interrupt() && t)
-			++t->irq_count;
+		if (in_interrupt() && t) {
+			if (t->irq_count < ec_storm_threshold)
+				++t->irq_count;
+			/* Allow triggering on 0 threshold */
+			if (t->irq_count == ec_storm_threshold)
+				acpi_ec_set_storm(ec, EC_FLAGS_COMMAND_STORM);
+		}
 	}
 out:
 	if (status & ACPI_EC_FLAG_SCI)
@@ -482,6 +510,8 @@ static int acpi_ec_transaction_unlocked(struct acpi_ec *ec,
 	spin_unlock_irqrestore(&ec->lock, tmp);
 	ret = ec_poll(ec);
 	spin_lock_irqsave(&ec->lock, tmp);
+	if (t->irq_count == ec_storm_threshold)
+		acpi_ec_clear_storm(ec, EC_FLAGS_COMMAND_STORM);
 	pr_debug("***** Command(%s) stopped *****\n",
 		 acpi_ec_cmd_string(t->command));
 	ec->curr = NULL;
@@ -509,24 +539,11 @@ static int acpi_ec_transaction(struct acpi_ec *ec, struct transaction *t)
 			goto unlock;
 		}
 	}
-	/* disable GPE during transaction if storm is detected */
-	if (test_bit(EC_FLAGS_GPE_STORM, &ec->flags)) {
-		/* It has to be disabled, so that it doesn't trigger. */
-		acpi_ec_disable_gpe(ec, false);
-	}
 
 	status = acpi_ec_transaction_unlocked(ec, t);
 
-	if (test_bit(EC_FLAGS_GPE_STORM, &ec->flags)) {
+	if (test_bit(EC_FLAGS_COMMAND_STORM, &ec->flags))
 		msleep(1);
-		/* It is safe to enable the GPE outside of the transaction. */
-		acpi_ec_enable_gpe(ec, false);
-	} else if (t->irq_count > ec_storm_threshold) {
-		pr_info("GPE storm detected(%d GPEs), "
-			"transactions will use polling mode\n",
-			t->irq_count);
-		set_bit(EC_FLAGS_GPE_STORM, &ec->flags);
-	}
 	if (ec->global_lock)
 		acpi_release_global_lock(glk);
 unlock:
-- 
1.7.10


  parent reply	other threads:[~2015-02-06  0:58 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-11-03  5:16 [PATCH 0/6] ACPI/EC: Cleanups of command flushing and event polling Lv Zheng
2014-11-03  5:16 ` [PATCH 1/6] ACPI/EC: Introduce STARTED/STOPPED flags to replace BLOCKED flag Lv Zheng
2014-11-05  2:52   ` Zheng, Lv
2014-11-18 13:23     ` Kirill A. Shutemov
2014-11-18 21:20       ` Rafael J. Wysocki
2014-11-19  8:55         ` Zheng, Lv
2014-11-19 12:16         ` Kirill A. Shutemov
2014-11-20  2:19           ` [RFC PATCH v3 1/2] ACPICA: Events: Remove duplicated sanity check in acpi_ev_enable_gpe() Lv Zheng
2014-11-20  2:19           ` [RFC PATCH v3 2/2] ACPICA: Events: Introduce ACPI_GPE_HANDLER_RAW to fix 2 issues for the current GPE APIs Lv Zheng
2014-11-20  2:20           ` [PATCH 1/6] ACPI/EC: Introduce STARTED/STOPPED flags to replace BLOCKED flag Zheng, Lv
2014-11-20 21:33             ` Kirill A. Shutemov
2014-11-21  0:42               ` Zheng, Lv
2014-11-21  0:55               ` Zheng, Lv
2014-11-20  2:34           ` Zheng, Lv
2014-11-20 21:34             ` Kirill A. Shutemov
2014-11-20  6:44           ` [RFC PATCH v4] ACPICA/Events: Add support to ensure GPE is disabled by default for handlers Lv Zheng
2014-11-20  6:47             ` Zheng, Lv
2014-11-20 22:15               ` Kirill A. Shutemov
2014-11-21  1:36                 ` Zheng, Lv
2015-02-06  0:57           ` [PATCH v2 0/5] ACPI / EC: Add reference counting for requests and cleans up the grace periods support Lv Zheng
2015-02-06  0:57             ` [PATCH v2 1/5] ACPI / EC: Introduce STARTED/STOPPED flags to replace BLOCKED flag Lv Zheng
2015-02-06  0:57             ` [PATCH v2 2/5] ACPI / EC: Add command flushing support Lv Zheng
2015-02-06  0:58             ` Lv Zheng [this message]
2015-02-06  0:58             ` [PATCH v2 4/5] ACPI / EC: Add query " Lv Zheng
2015-02-06  0:58             ` [PATCH v2 5/5] ACPI / EC: Add GPE reference counting debugging messages Lv Zheng
2015-02-06  1:26             ` [PATCH v2 0/5] ACPI / EC: Add reference counting for requests and cleans up the grace periods support Rafael J. Wysocki
2015-02-09  2:23               ` Zheng, Lv
2015-02-12  1:17                 ` Rafael J. Wysocki
2015-02-16  6:41                   ` Zheng, Lv
2014-11-03  5:16 ` [PATCH 2/6] ACPI/EC: Enhance the checks to apply to QR_EC transactions Lv Zheng
2014-11-03  5:16 ` [PATCH 3/6] ACPI/EC: Add reference counting for query handlers Lv Zheng
2014-11-03  5:16 ` [PATCH 4/6] ACPI/EC: Add command flushing support Lv Zheng
2014-11-03  5:16 ` [PATCH 5/6] ACPI/EC: Cleanup QR_SC command processing by adding a kernel thread to poll EC events Lv Zheng
2014-11-12  1:16   ` Rafael J. Wysocki
2014-11-13  2:31     ` Zheng, Lv
2014-11-13  2:58       ` Rafael J. Wysocki
2014-11-13  2:52         ` Zheng, Lv
2014-11-13 22:37           ` Rafael J. Wysocki
2014-11-14  1:21             ` Zheng, Lv
2014-11-14 23:38               ` Rafael J. Wysocki
2014-11-03  5:16 ` [PATCH 6/6] ACPI/EC: Add GPE reference counting debugging messages Lv Zheng

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=c102e0e999906a7f38ad7dd6c9d8e708e33467ec.1423183648.git.lv.zheng@intel.com \
    --to=lv.zheng@intel.com \
    --cc=len.brown@intel.com \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rafael.j.wysocki@intel.com \
    --cc=zetalog@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).