All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mika Kuoppala <mika.kuoppala@linux.intel.com>
To: Chris Wilson <chris@chris-wilson.co.uk>, intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 01/22] drm/i915/gem: Make caps.scheduler static
Date: Tue, 06 Aug 2019 14:50:14 +0300	[thread overview]
Message-ID: <87d0hitsw9.fsf@gaia.fi.intel.com> (raw)
In-Reply-To: <156508881763.2892.12035903141916655325@skylake-alporthouse-com>

Chris Wilson <chris@chris-wilson.co.uk> writes:

> Quoting Mika Kuoppala (2019-08-06 11:40:48)
>> Chris Wilson <chris@chris-wilson.co.uk> writes:
>> > diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
>> > index 8ac7d14ec8c9..81094f250bdb 100644
>> > --- a/drivers/gpu/drm/i915/i915_request.c
>> > +++ b/drivers/gpu/drm/i915/i915_request.c
>> > @@ -1198,7 +1198,6 @@ struct i915_request *__i915_request_commit(struct i915_request *rq)
>> >        */
>> >       local_bh_disable();
>> >       i915_sw_fence_commit(&rq->semaphore);
>> > -     rcu_read_lock(); /* RCU serialisation for set-wedged protection */
>> 
>> We don't need to protect against attr changes anymore so yes...
>> 
>> >       if (engine->schedule) {
>> >               struct i915_sched_attr attr = rq->gem_context->sched;
>> >  
>> > @@ -1228,7 +1227,6 @@ struct i915_request *__i915_request_commit(struct i915_request *rq)
>> >  
>> >               engine->schedule(rq, &attr);
>> 
>> but will now schedule during wedged. Didn't notice anything that
>> would blowup on reordering but is this intentional?
>
> How do you think it will blow up? engine->schedule remains untouched
> over wedged, and all it is doing is traversing the dependency lists
> (which remain intact) and the scheduler list (which is controlled by
> locking and cleared inside engine->cancel_requests, after which point it
> will remain clear as nop_submit_request should not care).
>
> I don't think there's a bad interaction there between i915_schedule()
> and nop_submit_request...

Didn't find anything that would blow up. Just a notable change in
behaviour so tried to poke around a bit. The less we special case
the better so nothing against the idea.

Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>

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

  reply	other threads:[~2019-08-06 11:50 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-06  9:05 [PATCH 01/22] drm/i915/gem: Make caps.scheduler static Chris Wilson
2019-08-06  9:05 ` [PATCH 02/22] drm/i915/gt: Move the [class][inst] lookup for engines onto the GT Chris Wilson
2019-08-06  9:05 ` [PATCH 03/22] drm/i915: Defer final intel_wakeref_put to process context Chris Wilson
2019-08-06 16:29   ` Chris Wilson
2019-08-06  9:05 ` [PATCH 04/22] drm/i915/pmu: Use GT parked for estimating RC6 while asleep Chris Wilson
2019-08-06  9:05 ` [PATCH 05/22] drm/i915: Drop the fudge warning on ring restart for ctg/elk Chris Wilson
2019-08-06  9:05 ` [PATCH 06/22] drm/i915/selftests: Pass intel_context to mock_request Chris Wilson
2019-08-06  9:05 ` [PATCH 07/22] drm/i915/gt: Make deferred context allocation explicit Chris Wilson
2019-08-06  9:05 ` [PATCH 08/22] drm/i915: Push the ring creation flags to the backend Chris Wilson
2019-08-06  9:05 ` [PATCH 09/22] drm/i915: Lift timeline into intel_context Chris Wilson
2019-08-06  9:05 ` [PATCH 10/22] drm/i915: Use intel_engine_lookup_user for probing HAS_BSD etc Chris Wilson
2019-08-06  9:05 ` [PATCH 11/22] drm/i915: Isolate i915_getparam_ioctl() Chris Wilson
2019-08-06  9:05 ` [PATCH 12/22] drm/i915: Only include active engines in the capture state Chris Wilson
2019-08-06  9:05 ` [PATCH 13/22] drm/i915: Make debugfs/per_file_stats scale better Chris Wilson
2019-08-06  9:05 ` [PATCH 14/22] drm/i915/gt: Track timeline activeness in enter/exit Chris Wilson
2019-08-06  9:05 ` [PATCH 15/22] drm/i915/gt: Convert timeline tracking to spinlock Chris Wilson
2019-08-06  9:05 ` [PATCH 16/22] drm/i915/gt: Guard timeline pinning with its own mutex Chris Wilson
2019-08-06  9:05 ` [PATCH 17/22] drm/i915: Protect request retirement with timeline->mutex Chris Wilson
2019-08-06  9:05 ` [PATCH 18/22] drm/i915/gt: Mark context->active_count as protected by timeline->mutex Chris Wilson
2019-08-06  9:05 ` [PATCH 19/22] drm/i915: Forgo last_fence active request tracking Chris Wilson
2019-08-06  9:05 ` [PATCH 20/22] drm/i915/overlay: Switch to using i915_active tracking Chris Wilson
2019-08-06  9:05 ` [PATCH 21/22] drm/i915: Extract intel_frontbuffer active tracking Chris Wilson
2019-08-07 14:45   ` kbuild test robot
2019-08-06  9:05 ` [PATCH 22/22] drm/i915: Markup expected timeline locks for i915_active Chris Wilson
2019-08-06 10:40 ` [PATCH 01/22] drm/i915/gem: Make caps.scheduler static Mika Kuoppala
2019-08-06 10:53   ` Chris Wilson
2019-08-06 11:50     ` Mika Kuoppala [this message]
2019-08-06 11:19 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [01/22] " Patchwork
2019-08-06 11:30 ` ✗ Fi.CI.SPARSE: " Patchwork
2019-08-06 11:43 ` ✓ Fi.CI.BAT: success " Patchwork
2019-08-06 20:14 ` ✗ Fi.CI.IGT: failure " Patchwork
2019-08-06 20:20   ` Chris Wilson
2019-08-06 21:24     ` Chris Wilson

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=87d0hitsw9.fsf@gaia.fi.intel.com \
    --to=mika.kuoppala@linux.intel.com \
    --cc=chris@chris-wilson.co.uk \
    --cc=intel-gfx@lists.freedesktop.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.