Received-SPF: pass (google.com: domain of linux-kernel+bounces-146490-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
From: Norihiko Hama <Norihiko.Hama@alpsalpine.com>
To: stern@rowland.harvard.edu,
	mdharm-usb@one-eyed-alien.net,
	gregkh@linuxfoundation.org,
	linux-doc@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	linux-usb@vger.kernel.org,
	usb-storage@lists.one-eyed-alien.net
Cc: Norihiko Hama <Norihiko.Hama@alpsalpine.com>
Subject: [PATCH v3] usb-storage: Optimize scan delay more precisely
Date: Tue, 16 Apr 2024 17:28:21 +0900
Message-Id: <20240416082821.10164-1-Norihiko.Hama@alpsalpine.com>
Content-Transfer-Encoding: 7bit
Content-Type: text/plain
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?us-ascii?Q?u1doW0Tr7H8gCttOqhJTgTWDYNTuOkisMWESJoTDHnZLUPdB1JR+Hf0wg/Xs?=
 =?us-ascii?Q?3Xn6e2eyW143lxeQCGqpnY3nlL5WGMRog0dYugdWmXPZOuqJf2Wz5lLf0ya2?=
 =?us-ascii?Q?FkBrGFbZxhRD3x0v13qOuR+XfS+IbQBO2aA1WFeeQeTX3n+duQpVrsXH7ze/?=
 =?us-ascii?Q?hUrB0fouWdsehGmXaim9RmXCC9OSCmPNxvjBoplK92nmfkylbxTGG2jOHw4z?=
 =?us-ascii?Q?T9sbXqvsyv3R2Op9AB58G4plZffMViaABVrxTI4XfMyKcixnQSYwdEqSqk9/?=
 =?us-ascii?Q?h3ZYBrjPrqd7z8/46nnXMoazreUrVDEXjqtWew3rh9i/ENDr1ML7BEcsTk7o?=
 =?us-ascii?Q?N6h1ndKbh+XZRM7o+IPHtm25kLksRldvsNKI38NRXKdQF0kJ3VFIStvwzir7?=
 =?us-ascii?Q?DMhsuWDg62IW4ymTyOLeNWoLAE4tqnwGLSPKj4Qch6ELfzAELksyv3mkz1W7?=
 =?us-ascii?Q?TcjU7WvB4uFSiTu/H0L7gjLYywYfEI0LD20VSvaa/UdiVfgRzbQFCpial5/B?=
 =?us-ascii?Q?3K+w48PH+nfWfMwCHM6UaWb3HpdkjWVom/JeHunYHFDxQE3FCyhfyDvATo5y?=
 =?us-ascii?Q?Rcd6OZnhFCrCi0ctTDwM5Z8fUEmHZ7reQBZQajyppV6YBijdF1GKPA8Hq7jk?=
 =?us-ascii?Q?NFEvxQC8fjDDRhVctnSVS4iY6k+M9B0G6kYnqolTpf/XX2cmR338wgcuaAnb?=
 =?us-ascii?Q?X2hjhLMRrhuclMTDc8LQ5x1c6kcRRK534Gq8XHnarPh7s+vxLbMDUDQmYkCK?=
 =?us-ascii?Q?5vvhGKbfUKwNjejaQqIZvMlzLfsqrWGuwmX4Ig/L9DgXD4oArYuGYMf4q21s?=
 =?us-ascii?Q?VqIeNTIFv3V45Ri3xVm0JTqSV+xAuTLy4oqwaDlL1daxcmGFxD26JQadWfFY?=
 =?us-ascii?Q?KKG87a5nfOReK+JZJSUkU8MM0J4ltZfN8a5YeM3htVIGS4Txmwj7/M2eEMFN?=
 =?us-ascii?Q?wUxUBPMZvHBTJRhD1U61SeYEfIY9CxoOlRLTkTcQ5agPfptVIhKGvVqNCL+s?=
 =?us-ascii?Q?wxZoricp78NvIHjcrlCsHCIK9FimJmYn3Ieui9dFJInvnPY/2JAQXvSKfATo?=
 =?us-ascii?Q?UFinsPsg+i7EfPDbxLKS6YjN9rc0TNKtGcEVcP9U+lEICZx5c+dHBh762usn?=
 =?us-ascii?Q?cfV989lLs8QwR/cyI6S+uJ08UoYi4uxQZ4oHsvtpCmNABfejFSsD4SeuzSrT?=
 =?us-ascii?Q?P7p1c99RUfRsHvApNAVj6D1i78Vps4YreaOnGXQkMoTdteKPQJMgOSKMbHPB?=
 =?us-ascii?Q?XQOX0CDhwB6Eu1UMql+tHn9cNNnfOgQm3RfW3OtxnE0MkgP6Hukd3VZttW++?=
 =?us-ascii?Q?rVaCEIUiIdvDT8ehMPAwvSoSSmklqZ3OGngPz3J/WWZBl9YGdyfMBI010B9S?=
 =?us-ascii?Q?MBmwh4PbMr5F5YCQ8lMrvgaqqUuNdyj/MvGQ81TRJCBbI195R/ko34olEl4w?=
 =?us-ascii?Q?CaOWWd2gbq38144f7laB5paIQMHHOy5DG0gONtoeztZduFTzSMm3gLz6eWgq?=
 =?us-ascii?Q?XX/SJWJbS+OQKlSrk1R5Qyi6U23u0vXJntNximl/jiH+XrCKaCtboHvlhi36?=
 =?us-ascii?Q?LEDG5DWguqvzQevVVXiCWbtYKbXtEW3tOdc0lrzW?=
X-OriginatorOrg: alpsalpine.com
X-MS-Exchange-CrossTenant-Network-Message-Id: b701d5b9-0bb1-4b49-bef2-08dc5dee4b9b
X-MS-Exchange-CrossTenant-AuthSource: TYVPR01MB10781.jpnprd01.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 16 Apr 2024 08:22:03.3454
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 57e76998-77bd-4b82-a424-198f46eb2254
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: mitbWUcGLO7JMGVEfCU2Xf86MUyPQ3nC9kULGvpkV/WVsbAe2+fT11Fmkob5FeOri38y2TLhP+bmwOEkRKM2FQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: TYCPR01MB6753

Current storage scan delay is reduced by the following old commit.

a4a47bc03fe5 ("Lower USB storage settling delay to something more reasonable")

It means that delay is at least 'one second', or zero with delay_use=0.
'one second' is still long delay especially for embedded system but
when delay_use is set to 0 (no delay), still error observed on some USB drives.

So delay_use should not be set to 0 but 'one second' is quite long.
Especially for embedded system, it's important for end user
how quickly access to USB drive when it's connected.
That's why we have a chance to minimize such a constant long delay.

This patch optimizes scan delay more precisely
to minimize delay time but not to have any problems on USB drives
by extending module parameter 'delay_use' in milliseconds internally.
The parameter 'delay_use' is changed to be parsed as 3 decimal point value
if it has digit values with '.'.
It makes the range of value to 1 / 1000 in internal 32-bit value
but it's still enough to set the delay time.
By default, delay time is 'one second' for backward compatibility.

For example, it seems to be good by changing delay_use=0.1,
that is 100 millisecond delay without issues for most USB pen drives.

Signed-off-by: Norihiko Hama <Norihiko.Hama@alpsalpine.com>
---
 .../admin-guide/kernel-parameters.txt         | 10 +++
 drivers/usb/storage/usb.c                     | 72 +++++++++++++++++--
 2 files changed, 78 insertions(+), 4 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 561d0dd776c7..ae1eb5988706 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -6190,6 +6190,16 @@
 	usb-storage.delay_use=
 			[UMS] The delay in seconds before a new device is
 			scanned for Logical Units (default 1).
+			To specify more precise delay, supports 3 decimal point.
+			The range of decimal point is in milliseconds,
+			hence the minimum value is "0.001".
+			Example:
+				delay_use=1
+					1 second delay
+				delay_use=0.1
+					0.1 second delay
+				delay_use=2.55
+					2.55 second elay
 
 	usb-storage.quirks=
 			[UMS] A list of quirks entries to supplement or
diff --git a/drivers/usb/storage/usb.c b/drivers/usb/storage/usb.c
index 90aa9c12ffac..d5eb7dd8f3b1 100644
--- a/drivers/usb/storage/usb.c
+++ b/drivers/usb/storage/usb.c
@@ -67,9 +67,73 @@ MODULE_AUTHOR("Matthew Dharm <mdharm-usb@one-eyed-alien.net>");
 MODULE_DESCRIPTION("USB Mass Storage driver for Linux");
 MODULE_LICENSE("GPL");
 
-static unsigned int delay_use = 1;
-module_param(delay_use, uint, S_IRUGO | S_IWUSR);
-MODULE_PARM_DESC(delay_use, "seconds to delay before using a new device");
+static unsigned int delay_use = 1 * MSEC_PER_SEC;
+
+static int delay_use_set(const char *s, const struct kernel_param *kp)
+{
+	unsigned int delay_ms = 0;
+	int frac = 3, ret;
+	char *p = skip_spaces(s);
+	char buf[16];
+
+	if (strlen(p) >= sizeof(buf) - frac)
+		return -EINVAL;
+
+	strscpy_pad(buf, p, sizeof(buf));
+
+	p = strchr(buf, '.');
+	if (p) {
+		int i = 0;
+		char *tmp = p + 1;
+
+		while (tmp[i] && tmp[i] != '\n')
+			*p++ = tmp[i++];
+
+		if (i == 0 || i > frac)
+			return -EINVAL;
+		frac -= i;
+	} else {
+		p = buf + strlen(buf) - 1;
+		if (*p != '\n')
+			p++;
+	}
+	while (frac-- > 0)
+		*p++ = '0';
+	*p = '\0';
+
+	ret = kstrtouint(buf, 10, &delay_ms);
+	if (ret < 0)
+		return ret;
+
+	*((unsigned int *)kp->arg) = delay_ms;
+	return 0;
+}
+
+static int delay_use_get(char *s, const struct kernel_param *kp)
+{
+	unsigned int delay_ms = *((unsigned int *)kp->arg);
+	unsigned int rem = do_div(delay_ms, MSEC_PER_SEC);
+	int len;
+	char buf[16];
+
+	len = scnprintf(buf, sizeof(buf), "%d", delay_ms);
+	if (rem) {
+		len += scnprintf(buf + len, sizeof(buf) - len, ".%03d", rem);
+		while (buf[len - 1] == '0') {
+			buf[len - 1] = '\0';
+			if (--len <= 1)
+				break;
+		}
+	}
+	return scnprintf(s, PAGE_SIZE, "%s\n", buf);
+}
+
+static const struct kernel_param_ops delay_use_ops = {
+	.set = delay_use_set,
+	.get = delay_use_get,
+};
+module_param_cb(delay_use, &delay_use_ops, &delay_use, 0644);
+MODULE_PARM_DESC(delay_use, "time to delay before using a new device");
 
 static char quirks[128];
 module_param_string(quirks, quirks, sizeof(quirks), S_IRUGO | S_IWUSR);
@@ -1066,7 +1130,7 @@ int usb_stor_probe2(struct us_data *us)
 	if (delay_use > 0)
 		dev_dbg(dev, "waiting for device to settle before scanning\n");
 	queue_delayed_work(system_freezable_wq, &us->scan_dwork,
-			delay_use * HZ);
+			msecs_to_jiffies(delay_use));
 	return 0;
 
 	/* We come here if there are any problems */
-- 
2.17.1