linux-kernel - [PATCH 1/9] vmbus: only reschedule tasklet if time limit exceeded

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <1488677238-5150-1-git-send-email-kys@exchange.microsoft.com>
Date:   Sat,  4 Mar 2017 18:27:10 -0700
From:   kys@...hange.microsoft.com
To:     gregkh@...uxfoundation.org, linux-kernel@...r.kernel.org,
        devel@...uxdriverproject.org, olaf@...fle.de, apw@...onical.com,
        vkuznets@...hat.com, jasowang@...hat.com,
        leann.ogasawara@...onical.com, marcelo.cerri@...onical.com
Cc:     Stephen Hemminger <stephen@...workplumber.org>,
        Stephen Hemminger <sthemmin@...rosoft.com>,
        "K. Y. Srinivasan" <kys@...rosoft.com>
Subject: [PATCH 1/9] vmbus: only reschedule tasklet if time limit exceeded

From: Stephen Hemminger <stephen@...workplumber.org>

The change to reschedule tasklet if more data arrives in ring buffer
can cause performance regression if host timing is such that the
next response happens in small window.

Go back to a modified version of the original looping behavior.
If the race occurs in a small time, then loop. But if the tasklet
has been running for a long interval due to flood, then reschedule
the tasklet to allow migration to ksoftirqd.

Signed-off-by: Stephen Hemminger <sthemmin@...rosoft.com>
Signed-off-by: K. Y. Srinivasan <kys@...rosoft.com>
---
 drivers/hv/connection.c |   65 ++++++++++++++++++++++++----------------------
 1 files changed, 34 insertions(+), 31 deletions(-)

diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c
index a8366fe..fce27fb 100644
--- a/drivers/hv/connection.c
+++ b/drivers/hv/connection.c
@@ -296,44 +296,47 @@ struct vmbus_channel *relid2channel(u32 relid)
 
 /*
  * vmbus_on_event - Process a channel event notification
+ *
+ * For batched channels (default) optimize host to guest signaling
+ * by ensuring:
+ * 1. While reading the channel, we disable interrupts from host.
+ * 2. Ensure that we process all posted messages from the host
+ *    before returning from this callback.
+ * 3. Once we return, enable signaling from the host. Once this
+ *    state is set we check to see if additional packets are
+ *    available to read. In this case we repeat the process.
+ *    If this tasklet has been running for a long time
+ *    then reschedule ourselves.
  */
 void vmbus_on_event(unsigned long data)
 {
 	struct vmbus_channel *channel = (void *) data;
-	void (*callback_fn)(void *);
+	unsigned long time_limit = jiffies + 2;
 
-	/*
-	 * A channel once created is persistent even when there
-	 * is no driver handling the device. An unloading driver
-	 * sets the onchannel_callback to NULL on the same CPU
-	 * as where this interrupt is handled (in an interrupt context).
-	 * Thus, checking and invoking the driver specific callback takes
-	 * care of orderly unloading of the driver.
-	 */
-	callback_fn = READ_ONCE(channel->onchannel_callback);
-	if (unlikely(callback_fn == NULL))
-		return;
-
-	(*callback_fn)(channel->channel_callback_context);
-
-	if (channel->callback_mode == HV_CALL_BATCHED) {
-		/*
-		 * This callback reads the messages sent by the host.
-		 * We can optimize host to guest signaling by ensuring:
-		 * 1. While reading the channel, we disable interrupts from
-		 *    host.
-		 * 2. Ensure that we process all posted messages from the host
-		 *    before returning from this callback.
-		 * 3. Once we return, enable signaling from the host. Once this
-		 *    state is set we check to see if additional packets are
-		 *    available to read. In this case we repeat the process.
+	do {
+		void (*callback_fn)(void *);
+
+		/* A channel once created is persistent even when
+		 * there is no driver handling the device. An
+		 * unloading driver sets the onchannel_callback to NULL.
 		 */
-		if (hv_end_read(&channel->inbound) != 0) {
-			hv_begin_read(&channel->inbound);
+		callback_fn = READ_ONCE(channel->onchannel_callback);
+		if (unlikely(callback_fn == NULL))
+			return;
 
-			tasklet_schedule(&channel->callback_event);
-		}
-	}
+		(*callback_fn)(channel->channel_callback_context);
+
+		if (channel->callback_mode != HV_CALL_BATCHED)
+			return;
+
+		if (likely(hv_end_read(&channel->inbound) == 0))
+			return;
+
+		hv_begin_read(&channel->inbound);
+	} while (likely(time_before(jiffies, time_limit)));
+
+	/* The time limit (2 jiffies) has been reached */
+	tasklet_schedule(&channel->callback_event);
 }
 
 /*
-- 
1.7.1