Netdev List
 help / color / mirror / Atom feed
From: Nicholas Mc Guire <hofrat@osadl.org>
To: Derek Chickles <derek.chickles@caviumnetworks.com>
Cc: Satanand Burla <satananda.burla@caviumnetworks.com>,
	Felix Manlunas <felix.manlunas@caviumnetworks.com>,
	Raghu Vatsavayi <raghu.vatsavayi@caviumnetworks.com>,
	"David S. Miller" <davem@davemloft.net>,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	Nicholas Mc Guire <hofrat@osadl.org>
Subject: [PATCH] liquidio: make timeout HZ independent and readable
Date: Mon,  2 Jul 2018 20:13:15 +0200	[thread overview]
Message-ID: <1530555195-28129-1-git-send-email-hofrat@osadl.org> (raw)

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@osadl.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

             reply	other threads:[~2018-07-02 18:13 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-07-02 18:13 Nicholas Mc Guire [this message]
2018-07-06  0:19 ` [PATCH] liquidio: make timeout HZ independent and readable Chickles, Derek
2018-07-06  2:07 ` David Miller

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=1530555195-28129-1-git-send-email-hofrat@osadl.org \
    --to=hofrat@osadl.org \
    --cc=davem@davemloft.net \
    --cc=derek.chickles@caviumnetworks.com \
    --cc=felix.manlunas@caviumnetworks.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=raghu.vatsavayi@caviumnetworks.com \
    --cc=satananda.burla@caviumnetworks.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