Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To:     linux-kernel@vger.kernel.org
Cc:     Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        stable@vger.kernel.org, Wang Xin <xin.wang7@cn.bosch.com>,
        Mark Jonas <mark.jonas@de.bosch.com>,
        Bartosz Golaszewski <brgl@bgdev.pl>
Subject: [PATCH 4.14 26/43] eeprom: at24: fix unexpected timeout under high load
Date:   Tue,  2 Jul 2019 10:02:06 +0200
Message-Id: <20190702080125.196152902@linuxfoundation.org>
In-Reply-To: <20190702080123.904399496@linuxfoundation.org>
References: <20190702080123.904399496@linuxfoundation.org>
User-Agent: quilt/0.66
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

From: Wang Xin <xin.wang7@cn.bosch.com>

commit 9a9e295e7c5c0409c020088b0ae017e6c2b7df6e upstream.

Within at24_loop_until_timeout the timestamp used for timeout checking
is recorded after the I2C transfer and sleep_range(). Under high CPU
load either the execution time for I2C transfer or sleep_range() could
actually be larger than the timeout value. Worst case the I2C transfer
is only tried once because the loop will exit due to the timeout
although the EEPROM is now ready.

To fix this issue the timestamp is recorded at the beginning of each
iteration. That is, before I2C transfer and sleep. Then the timeout
is actually checked against the timestamp of the previous iteration.
This makes sure that even if the timeout is reached, there is still one
more chance to try the I2C transfer in case the EEPROM is ready.

Example:

If you have a system which combines high CPU load with repeated EEPROM
writes you will run into the following scenario.

 - System makes a successful regmap_bulk_write() to EEPROM.
 - System wants to perform another write to EEPROM but EEPROM is still
   busy with the last write.
 - Because of high CPU load the usleep_range() will sleep more than
   25 ms (at24_write_timeout).
 - Within the over-long sleeping the EEPROM finished the previous write
   operation and is ready again.
 - at24_loop_until_timeout() will detect timeout and won't try to write.

Signed-off-by: Wang Xin <xin.wang7@cn.bosch.com>
Signed-off-by: Mark Jonas <mark.jonas@de.bosch.com>
Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>


---
 drivers/misc/eeprom/at24.c |  107 ++++++++++++++++++++++++++++++++-------------
 1 file changed, 77 insertions(+), 30 deletions(-)

--- a/drivers/misc/eeprom/at24.c
+++ b/drivers/misc/eeprom/at24.c
@@ -113,22 +113,6 @@ MODULE_PARM_DESC(write_timeout, "Time (i
 	((1 << AT24_SIZE_FLAGS | (_flags)) 		\
 	    << AT24_SIZE_BYTELEN | ilog2(_len))
 
-/*
- * Both reads and writes fail if the previous write didn't complete yet. This
- * macro loops a few times waiting at least long enough for one entire page
- * write to work while making sure that at least one iteration is run before
- * checking the break condition.
- *
- * It takes two parameters: a variable in which the future timeout in jiffies
- * will be stored and a temporary variable holding the time of the last
- * iteration of processing the request. Both should be unsigned integers
- * holding at least 32 bits.
- */
-#define loop_until_timeout(tout, op_time)				\
-	for (tout = jiffies + msecs_to_jiffies(write_timeout), op_time = 0; \
-	     op_time ? time_before(op_time, tout) : true;		\
-	     usleep_range(1000, 1500), op_time = jiffies)
-
 static const struct i2c_device_id at24_ids[] = {
 	/* needs 8 addresses as A0-A2 are ignored */
 	{ "24c00",	AT24_DEVICE_MAGIC(128 / 8,	AT24_FLAG_TAKE8ADDR) },
@@ -234,7 +218,14 @@ static ssize_t at24_eeprom_read_smbus(st
 	if (count > I2C_SMBUS_BLOCK_MAX)
 		count = I2C_SMBUS_BLOCK_MAX;
 
-	loop_until_timeout(timeout, read_time) {
+	timeout = jiffies + msecs_to_jiffies(write_timeout);
+	do {
+		/*
+		 * The timestamp shall be taken before the actual operation
+		 * to avoid a premature timeout in case of high CPU load.
+		 */
+		read_time = jiffies;
+
 		status = i2c_smbus_read_i2c_block_data_or_emulated(client,
 								   offset,
 								   count, buf);
@@ -244,7 +235,9 @@ static ssize_t at24_eeprom_read_smbus(st
 
 		if (status == count)
 			return count;
-	}
+
+		usleep_range(1000, 1500);
+	} while (time_before(read_time, timeout));
 
 	return -ETIMEDOUT;
 }
@@ -284,7 +277,14 @@ static ssize_t at24_eeprom_read_i2c(stru
 	msg[1].buf = buf;
 	msg[1].len = count;
 
-	loop_until_timeout(timeout, read_time) {
+	timeout = jiffies + msecs_to_jiffies(write_timeout);
+	do {
+		/*
+		 * The timestamp shall be taken before the actual operation
+		 * to avoid a premature timeout in case of high CPU load.
+		 */
+		read_time = jiffies;
+
 		status = i2c_transfer(client->adapter, msg, 2);
 		if (status == 2)
 			status = count;
@@ -294,7 +294,9 @@ static ssize_t at24_eeprom_read_i2c(stru
 
 		if (status == count)
 			return count;
-	}
+
+		usleep_range(1000, 1500);
+	} while (time_before(read_time, timeout));
 
 	return -ETIMEDOUT;
 }
@@ -343,11 +345,20 @@ static ssize_t at24_eeprom_read_serial(s
 	msg[1].buf = buf;
 	msg[1].len = count;
 
-	loop_until_timeout(timeout, read_time) {
+	timeout = jiffies + msecs_to_jiffies(write_timeout);
+	do {
+		/*
+		 * The timestamp shall be taken before the actual operation
+		 * to avoid a premature timeout in case of high CPU load.
+		 */
+		read_time = jiffies;
+
 		status = i2c_transfer(client->adapter, msg, 2);
 		if (status == 2)
 			return count;
-	}
+
+		usleep_range(1000, 1500);
+	} while (time_before(read_time, timeout));
 
 	return -ETIMEDOUT;
 }
@@ -374,11 +385,20 @@ static ssize_t at24_eeprom_read_mac(stru
 	msg[1].buf = buf;
 	msg[1].len = count;
 
-	loop_until_timeout(timeout, read_time) {
+	timeout = jiffies + msecs_to_jiffies(write_timeout);
+	do {
+		/*
+		 * The timestamp shall be taken before the actual operation
+		 * to avoid a premature timeout in case of high CPU load.
+		 */
+		read_time = jiffies;
+
 		status = i2c_transfer(client->adapter, msg, 2);
 		if (status == 2)
 			return count;
-	}
+
+		usleep_range(1000, 1500);
+	} while (time_before(read_time, timeout));
 
 	return -ETIMEDOUT;
 }
@@ -420,7 +440,14 @@ static ssize_t at24_eeprom_write_smbus_b
 	client = at24_translate_offset(at24, &offset);
 	count = at24_adjust_write_count(at24, offset, count);
 
-	loop_until_timeout(timeout, write_time) {
+	timeout = jiffies + msecs_to_jiffies(write_timeout);
+	do {
+		/*
+		 * The timestamp shall be taken before the actual operation
+		 * to avoid a premature timeout in case of high CPU load.
+		 */
+		write_time = jiffies;
+
 		status = i2c_smbus_write_i2c_block_data(client,
 							offset, count, buf);
 		if (status == 0)
@@ -431,7 +458,9 @@ static ssize_t at24_eeprom_write_smbus_b
 
 		if (status == count)
 			return count;
-	}
+
+		usleep_range(1000, 1500);
+	} while (time_before(write_time, timeout));
 
 	return -ETIMEDOUT;
 }
@@ -446,7 +475,14 @@ static ssize_t at24_eeprom_write_smbus_b
 
 	client = at24_translate_offset(at24, &offset);
 
-	loop_until_timeout(timeout, write_time) {
+	timeout = jiffies + msecs_to_jiffies(write_timeout);
+	do {
+		/*
+		 * The timestamp shall be taken before the actual operation
+		 * to avoid a premature timeout in case of high CPU load.
+		 */
+		write_time = jiffies;
+
 		status = i2c_smbus_write_byte_data(client, offset, buf[0]);
 		if (status == 0)
 			status = count;
@@ -456,7 +492,9 @@ static ssize_t at24_eeprom_write_smbus_b
 
 		if (status == count)
 			return count;
-	}
+
+		usleep_range(1000, 1500);
+	} while (time_before(write_time, timeout));
 
 	return -ETIMEDOUT;
 }
@@ -485,7 +523,14 @@ static ssize_t at24_eeprom_write_i2c(str
 	memcpy(&msg.buf[i], buf, count);
 	msg.len = i + count;
 
-	loop_until_timeout(timeout, write_time) {
+	timeout = jiffies + msecs_to_jiffies(write_timeout);
+	do {
+		/*
+		 * The timestamp shall be taken before the actual operation
+		 * to avoid a premature timeout in case of high CPU load.
+		 */
+		write_time = jiffies;
+
 		status = i2c_transfer(client->adapter, &msg, 1);
 		if (status == 1)
 			status = count;
@@ -495,7 +540,9 @@ static ssize_t at24_eeprom_write_i2c(str
 
 		if (status == count)
 			return count;
-	}
+
+		usleep_range(1000, 1500);
+	} while (time_before(write_time, timeout));
 
 	return -ETIMEDOUT;
 }