| Message ID | 20180423113754.28424-1-mika.kuoppala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Apr 23, 2018 at 4:37 AM, Mika Kuoppala <mika.kuoppala@linux.intel.com> wrote: > We use jiffies to determine when wait expires. However > Imre did find out that jiffies can and will do a >1 > increments on certain situations [1]. When this happens > in a wait_for loop, we return timeout errorneously > much earlier than what the real wallclock would say. > > We can't afford our waits to timeout prematurely. Isn't this also the reason why we have *_to_jiffies_timeout() family of functions? Should those be converted as well? Lucas De Marchi > Discard jiffies and change to ktime to detect timeouts. > > v2: added bugzilla entry (Imre), added stable (Chris) > > Reported-by: Imre Deak <imre.deak@intel.com> > References: https://lkml.org/lkml/2018/4/18/798 [1] > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105771 > Cc: Imre Deak <imre.deak@intel.com> > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Cc: <stable@vger.kernel.org> > Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com> > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> > --- > drivers/gpu/drm/i915/intel_drv.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index 44ed248f1fe9..33ff2638c92b 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -49,12 +49,12 @@ > * check the condition before the timeout. > */ > #define __wait_for(OP, COND, US, Wmin, Wmax) ({ \ > - unsigned long timeout__ = jiffies + usecs_to_jiffies(US) + 1; \ > + const ktime_t end__ = ktime_add_ns(ktime_get_raw(), 1000ll * (US)); \ > long wait__ = (Wmin); /* recommended min for usleep is 10 us */ \ > int ret__; \ > might_sleep(); \ > for (;;) { \ > - bool expired__ = time_after(jiffies, timeout__); \ > + const bool expired__ = ktime_after(ktime_get_raw(), end__); \ > OP; \ > if (COND) { \ > ret__ = 0; \ > -- > 2.14.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Mon, Apr 23, 2018 at 08:35:05AM -0700, Lucas De Marchi wrote: > On Mon, Apr 23, 2018 at 4:37 AM, Mika Kuoppala > <mika.kuoppala@linux.intel.com> wrote: > > We use jiffies to determine when wait expires. However > > Imre did find out that jiffies can and will do a >1 > > increments on certain situations [1]. When this happens > > in a wait_for loop, we return timeout errorneously > > much earlier than what the real wallclock would say. > > > > We can't afford our waits to timeout prematurely. > > Isn't this also the reason why we have *_to_jiffies_timeout() family > of functions? The reason for those is that you can sample jiffies between two clock tick updates and calculate a short expiry time. So a different issue than missed updates of jiffies due to some latency. > Should those be converted as well? > > Lucas De Marchi > > > Discard jiffies and change to ktime to detect timeouts. > > > > v2: added bugzilla entry (Imre), added stable (Chris) > > > > Reported-by: Imre Deak <imre.deak@intel.com> > > References: https://lkml.org/lkml/2018/4/18/798 [1] > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105771 > > Cc: Imre Deak <imre.deak@intel.com> > > Cc: Chris Wilson <chris@chris-wilson.co.uk> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Cc: <stable@vger.kernel.org> > > Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com> > > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> > > --- > > drivers/gpu/drm/i915/intel_drv.h | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > > index 44ed248f1fe9..33ff2638c92b 100644 > > --- a/drivers/gpu/drm/i915/intel_drv.h > > +++ b/drivers/gpu/drm/i915/intel_drv.h > > @@ -49,12 +49,12 @@ > > * check the condition before the timeout. > > */ > > #define __wait_for(OP, COND, US, Wmin, Wmax) ({ \ > > - unsigned long timeout__ = jiffies + usecs_to_jiffies(US) + 1; \ > > + const ktime_t end__ = ktime_add_ns(ktime_get_raw(), 1000ll * (US)); \ > > long wait__ = (Wmin); /* recommended min for usleep is 10 us */ \ > > int ret__; \ > > might_sleep(); \ > > for (;;) { \ > > - bool expired__ = time_after(jiffies, timeout__); \ > > + const bool expired__ = ktime_after(ktime_get_raw(), end__); \ > > OP; \ > > if (COND) { \ > > ret__ = 0; \ > > -- > > 2.14.1 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > -- > Lucas De Marchi > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 44ed248f1fe9..33ff2638c92b 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -49,12 +49,12 @@ * check the condition before the timeout. */ #define __wait_for(OP, COND, US, Wmin, Wmax) ({ \ - unsigned long timeout__ = jiffies + usecs_to_jiffies(US) + 1; \ + const ktime_t end__ = ktime_add_ns(ktime_get_raw(), 1000ll * (US)); \ long wait__ = (Wmin); /* recommended min for usleep is 10 us */ \ int ret__; \ might_sleep(); \ for (;;) { \ - bool expired__ = time_after(jiffies, timeout__); \ + const bool expired__ = ktime_after(ktime_get_raw(), end__); \ OP; \ if (COND) { \ ret__ = 0; \