Messages in this thread |  | | From | "Chickles, Derek" <> | | Subject | RE: [PATCH] liquidio: make timeout HZ independent and readable | | Date | Fri, 6 Jul 2018 00:19:51 +0000 |
| |
> From: Nicholas Mc Guire [mailto:hofrat@osadl.org] > Sent: Monday, July 02, 2018 11:13 AM > 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@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
Looks fine.
Acked-by: Derek Chickles <derek.chickles@caviumnetworks.com>
|  |