Re: [PATCH] drm/i915: Remove second level open-coded rcu work

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

Quoting Tvrtko Ursulin (2019-02-28 09:38:36)
> 
> On 28/02/2019 08:29, Chris Wilson wrote:
> > @@ -143,7 +141,15 @@ static u32 __i915_gem_park(struct drm_i915_private *i915)
> >   
> >       intel_display_power_put(i915, POWER_DOMAIN_GT_IRQ, wakeref);
> >   
> > -     return i915->gt.epoch;
> > +     /*
> > +      * When we are idle, it is an opportune time to reap our caches.
> > +      * However, we have many objects that utilise RCU and the ordered
> > +      * i915->wq that this work is executing on. To try and flush any
> > +      * pending frees now we are idle, we first wait for an RCU grace
> > +      * period, and then queue a task (that will run last on the wq) to
> > +      * shrink and re-optimize the caches.
> > +      */
> > +     i915_globals_park();
> 
> I think this comment would be better placed in i915_globals_park.

In i915_globals_park(), we have

        /*
         * Defer shrinking the global slab caches (and other work) until
         * after a RCU grace period has completed with no activity. This
         * is to try and reduce the latency impact on the consumers caused
         * by us shrinking the caches the same time as they are trying to
         * allocate, with the assumption being that if we idle long enough
         * for an RCU grace period to elapse since the last use, it is likely
         * to be longer until we need the caches again.
         */

Yeah, the old comment is a bit too verbose now, but I think it's still
worth having a comment there to explain what's about to be done and why
we call it now.

/* Tell the world we are idle and reap the benefits! */

Too subtle?
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx