Re: [PATCH 2/2] drm/i915: Align the retire_requests worker to the nearest second

[Chris Wilson <chris@chris-wilson.co.uk>]

On Fri, 05 Oct 2012 08:18:17 -0700, Arjan van de Ven <arjan@linux.intel.com> wrote:
> On 10/5/2012 6:53 AM, Chris Wilson wrote:
> > By using round_jiffies() we can align the wakeup of our worker to the
> > nearest second in order to batch wakeups and reduce system load, which
> > is useful for unimportant coarse tasks like our retire_requests.
> > 
> > Suggested-by: Arjan van de Ven <arjan@linux.intel.com>
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Arjan van de Ven <arjan@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_gem.c |   14 +++++++++++---
> >  1 file changed, 11 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> > index 8e05d53..706f481 100644
> > --- a/drivers/gpu/drm/i915/i915_gem.c
> > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > @@ -2084,6 +2084,11 @@ i915_gem_next_request_seqno(struct intel_ring_buffer *ring)
> >  	return ring->outstanding_lazy_request;
> >  }
> >  
> > +static unsigned long round_jiffies_delay(unsigned long delay)
> > +{
> > +	return round_jiffies_relative(delay) - jiffies;
> > +}
> 
> this is buggy
> 
> 
> > +
> >  int
> >  i915_add_request(struct intel_ring_buffer *ring,
> >  		 struct drm_file *file,
> > @@ -2155,7 +2160,8 @@ i915_add_request(struct intel_ring_buffer *ring,
> >  		}
> >  		if (was_empty) {
> >  			queue_delayed_work(dev_priv->wq,
> > -					   &dev_priv->mm.retire_work, HZ);
> > +					   &dev_priv->mm.retire_work,
> > +					   round_jiffies_delay(HZ));
> 
> when used like this
> 
> 
> round_jiffies() rounds absolute jiffies towards the next second
> 
> round_jiffies_relative() already subtracts jiffies from the result, like
> the helper that you're trying to invent here does ;=)
> 
> doing that double up is a bad idea.

For some reason the example I read convinced me that
round_jiffies_relative() returned the absolute jiffie for the relative
delay so that we could put it straight into mod_timer().

Again we can use round up here as well.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre