netdev - [PATCH] liquidio: make timeout HZ independent and readable

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-next>] [day] [month] [year] [list]

Date:   Mon,  2 Jul 2018 20:13:15 +0200
From:   Nicholas Mc Guire <hofrat@...dl.org>
To:     Derek Chickles <derek.chickles@...iumnetworks.com>
Cc:     Satanand Burla <satananda.burla@...iumnetworks.com>,
        Felix Manlunas <felix.manlunas@...iumnetworks.com>,
        Raghu Vatsavayi <raghu.vatsavayi@...iumnetworks.com>,
        "David S. Miller" <davem@...emloft.net>, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org, Nicholas Mc Guire <hofrat@...dl.org>
Subject: [PATCH] liquidio: make timeout HZ independent and readable

schedule_timeout_* takes a timeout in jiffies but the code currently is
passing in a constant which makes this timeout HZ dependent. So define
a constant with (hopefully) meaningful name and pass it through
msecs_to_jiffies() to fix the HZ dependency.

Signed-off-by: Nicholas Mc Guire <hofrat@...dl.org>
commit f21fb3ed364b ("Add support of Cavium Liquidio ethernet adapters")
---

Problem found by experimental coccinelle script

The current wait time can vary by a factor 10 depending on the HZ
setting chose, which does not seem reasonable here.

The below patch sets the timeout to 1s - which is the current duration
assuming a setting of HZ== 100. It is though not clear if this is the
intent or if it should be shorter as it is not clear what HZ setting
was assumed during design and used for testing.

This needs an ack by someone who knows the device and can confirm that
waiting 1s for in-flight requests on device removal is reasonable.

Patch was compile tested with: x86_64_defconfig (implies
CONFIG_NET_VENDOR_CAVIUM=y)
(with a large number of sparse warnings though unrelated to the
proposed change)

Patch is against 4.18-rc2 (localversion-next is -next-20180702)

 drivers/net/ethernet/cavium/liquidio/lio_main.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/cavium/liquidio/lio_main.c b/drivers/net/ethernet/cavium/liquidio/lio_main.c
index 7cb4e75..b2d0598 100644
--- a/drivers/net/ethernet/cavium/liquidio/lio_main.c
+++ b/drivers/net/ethernet/cavium/liquidio/lio_main.c
@@ -91,6 +91,9 @@ static int octeon_console_debug_enabled(u32 console)
  */
 #define LIO_SYNC_OCTEON_TIME_INTERVAL_MS 60000
 
+/* time to wait for possible in-flight requests in milliseconds */
+#define WAIT_INFLIGHT_REQUEST	msecs_to_jiffies(1000)
+
 struct lio_trusted_vf_ctx {
 	struct completion complete;
 	int status;
@@ -259,7 +262,7 @@ static inline void pcierror_quiesce_device(struct octeon_device *oct)
 	force_io_queues_off(oct);
 
 	/* To allow for in-flight requests */
-	schedule_timeout_uninterruptible(100);
+	schedule_timeout_uninterruptible(WAIT_INFLIGHT_REQUEST);
 
 	if (wait_for_pending_requests(oct))
 		dev_err(&oct->pci_dev->dev, "There were pending requests\n");
-- 
2.1.4