From: John Harrison <John.C.Harrison@Intel.com>
To: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
Intel-GFX@Lists.FreeDesktop.Org
Subject: Re: [PATCH v7 4/8] drm/i915: Fix clean up of file client list on execbuff failure
Date: Thu, 21 Apr 2016 11:15:55 +0100 [thread overview]
Message-ID: <5718A85B.3010107@Intel.com> (raw)
In-Reply-To: <57187F37.6020205@linux.intel.com>
On 21/04/2016 08:20, Maarten Lankhorst wrote:
> Op 20-04-16 om 19:09 schreef John.C.Harrison@Intel.com:
>> From: John Harrison <John.C.Harrison@Intel.com>
>>
>> If an execbuff IOCTL call fails for some reason, it would leave the
>> request in the client list. The request clean up code would remove
>> this but only later on and only after the reference count has dropped
>> to zero. The entire sequence is contained within the driver mutex
>> lock. However, there is still a hole such that any code which does not
>> require the mutex lock could still find the request on the client list
>> and start using it. That would lead to broken reference counts, use of
>> dangling pointers and all sorts of other nastiness.
>>
>> The throttle IOCTL in particular does not acquire the mutex and does
>> process the client list. And the likely situation of the execbuff
>> IOCTL failing is when the system is busy with lots of work
>> outstanding. That is exactly the situation where the throttle IOCTL
>> would try to wait on a request.
>>
>> Currently, this hole is tiny - the gap between the reference count
>> dropping to zero and the free function being called in response.
>> However the next patch in this series enlarges that gap considerably
>> by deferring the free function (to remove the need for the mutex lock
>> when unreferencing requests).
>>
> Seems to already have been fixed by
>
> commit aa9b78104fe3210758fa9e6c644e9a108d371e8b
> Author: Chris Wilson <chris@chris-wilson.co.uk>
> Date: Wed Apr 13 17:35:15 2016 +0100
>
> drm/i915: Late request cancellations are harmful
>
I don't believe that patch is the best way to solve the problem.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2016-04-21 10:15 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-04-20 17:09 [PATCH v7 0/8] Convert requests to use struct fence John.C.Harrison
2016-04-20 17:09 ` [PATCH v7 1/8] drm/i915: " John.C.Harrison
2016-04-21 7:06 ` Maarten Lankhorst
2016-04-21 10:26 ` John Harrison
2016-04-21 11:12 ` Maarten Lankhorst
2016-04-21 7:09 ` Maarten Lankhorst
2016-04-20 17:09 ` [PATCH v7 2/8] drm/i915: Removed now redudant parameter to i915_gem_request_completed() John.C.Harrison
2016-04-21 7:19 ` Maarten Lankhorst
2016-04-21 10:18 ` John Harrison
2016-04-20 17:09 ` [PATCH v7 3/8] drm/i915: Add per context timelines to fence object John.C.Harrison
2016-04-20 17:44 ` Chris Wilson
2016-04-21 11:37 ` John Harrison
2016-04-20 17:09 ` [PATCH v7 4/8] drm/i915: Fix clean up of file client list on execbuff failure John.C.Harrison
2016-04-21 7:20 ` Maarten Lankhorst
2016-04-21 10:15 ` John Harrison [this message]
2016-04-20 17:09 ` [PATCH v7 5/8] drm/i915: Delay the freeing of requests until retire time John.C.Harrison
2016-04-21 7:41 ` Maarten Lankhorst
2016-04-20 17:09 ` [PATCH v7 6/8] drm/i915: Interrupt driven fences John.C.Harrison
2016-04-21 7:38 ` Maarten Lankhorst
2016-04-20 17:09 ` [PATCH v7 7/8] drm/i915: Updated request structure tracing John.C.Harrison
2016-04-20 17:09 ` [PATCH v7 8/8] drm/i915: Cache last IRQ seqno to reduce IRQ overhead John.C.Harrison
2016-04-22 15:37 ` [PATCH v7 0/8] Convert requests to use struct fence John Harrison
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=5718A85B.3010107@Intel.com \
--to=john.c.harrison@intel.com \
--cc=Intel-GFX@Lists.FreeDesktop.Org \
--cc=maarten.lankhorst@linux.intel.com \
/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.