* [PATCH] drm/atomic: Fix memleak on ERESTARTSYS during non-blocking commits
@ 2018-01-17 11:51 Maarten Lankhorst
2018-01-17 18:29 ` [Intel-gfx] " Sean Paul
0 siblings, 1 reply; 9+ messages in thread
From: Maarten Lankhorst @ 2018-01-17 11:51 UTC (permalink / raw)
To: dri-devel
Cc: intel-gfx, Harry Wentland, Leo (Sunpeng) Li, stable,
Maarten Lankhorst
From: "Leo (Sunpeng) Li" <sunpeng.li@amd.com>
During a non-blocking commit, it is possible to return before the
commit_tail work is queued (-ERESTARTSYS, for example).
Since a reference on the crtc commit object is obtained for the pending
vblank event when preparing the commit, the above situation will leave
us with an extra reference.
Therefore, if the commit_tail worker has not consumed the event at the
end of a commit, release it's reference.
Changes since v1:
- Also check for state->event->base.completion being set, to
handle the case where stall_checks() fails in setup_crtc_commit().
Changes since v2:
- Add a flag to drm_crtc_commit, to prevent dereferencing a freed event.
i915 may unreference the state in a worker.
Fixes: 24835e442f28 ("drm: reference count event->completion")
Cc: <stable@vger.kernel.org> # v4.11+
Signed-off-by: Leo (Sunpeng) Li <sunpeng.li@amd.com>
Acked-by: Harry Wentland <harry.wentland@amd.com> #v1
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
drivers/gpu/drm/drm_atomic_helper.c | 15 +++++++++++++++
include/drm/drm_atomic.h | 9 +++++++++
2 files changed, 24 insertions(+)
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index ab4032167094..ae3cbfe9e01c 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -1878,6 +1878,8 @@ int drm_atomic_helper_setup_commit(struct drm_atomic_state *state,
new_crtc_state->event->base.completion = &commit->flip_done;
new_crtc_state->event->base.completion_release = release_crtc_commit;
drm_crtc_commit_get(commit);
+
+ commit->abort_completion = true;
}
for_each_oldnew_connector_in_state(state, conn, old_conn_state, new_conn_state, i) {
@@ -3421,8 +3423,21 @@ EXPORT_SYMBOL(drm_atomic_helper_crtc_duplicate_state);
void __drm_atomic_helper_crtc_destroy_state(struct drm_crtc_state *state)
{
if (state->commit) {
+ /*
+ * In the event that a non-blocking commit returns
+ * -ERESTARTSYS before the commit_tail work is queued, we will
+ * have an extra reference to the commit object. Release it, if
+ * the event has not been consumed by the worker.
+ *
+ * state->event may be freed, so we can't directly look at
+ * state->event->base.completion.
+ */
+ if (state->event && state->commit->abort_completion)
+ drm_crtc_commit_put(state->commit);
+
kfree(state->commit->event);
state->commit->event = NULL;
+
drm_crtc_commit_put(state->commit);
}
diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
index 1c27526c499e..cf13842a6dbd 100644
--- a/include/drm/drm_atomic.h
+++ b/include/drm/drm_atomic.h
@@ -134,6 +134,15 @@ struct drm_crtc_commit {
* &drm_pending_vblank_event pointer to clean up private events.
*/
struct drm_pending_vblank_event *event;
+
+ /**
+ * @abort_completion:
+ *
+ * A flag that's set after drm_atomic_helper_setup_commit takes a second
+ * reference for the completion of $drm_crtc_state.event. It's used by
+ * the free code to remove the second reference if commit fails.
+ */
+ bool abort_completion;
};
struct __drm_planes_state {
--
2.15.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [Intel-gfx] [PATCH] drm/atomic: Fix memleak on ERESTARTSYS during non-blocking commits
2018-01-17 11:51 [PATCH] drm/atomic: Fix memleak on ERESTARTSYS during non-blocking commits Maarten Lankhorst
@ 2018-01-17 18:29 ` Sean Paul
2018-01-17 18:39 ` Maarten Lankhorst
0 siblings, 1 reply; 9+ messages in thread
From: Sean Paul @ 2018-01-17 18:29 UTC (permalink / raw)
To: Maarten Lankhorst
Cc: dri-devel, Leo (Sunpeng) Li, intel-gfx, Harry Wentland, stable
On Wed, Jan 17, 2018 at 12:51:08PM +0100, Maarten Lankhorst wrote:
> From: "Leo (Sunpeng) Li" <sunpeng.li@amd.com>
>
> During a non-blocking commit, it is possible to return before the
> commit_tail work is queued (-ERESTARTSYS, for example).
>
> Since a reference on the crtc commit object is obtained for the pending
> vblank event when preparing the commit, the above situation will leave
> us with an extra reference.
>
> Therefore, if the commit_tail worker has not consumed the event at the
> end of a commit, release it's reference.
>
> Changes since v1:
> - Also check for state->event->base.completion being set, to
> handle the case where stall_checks() fails in setup_crtc_commit().
> Changes since v2:
> - Add a flag to drm_crtc_commit, to prevent dereferencing a freed event.
> i915 may unreference the state in a worker.
>
> Fixes: 24835e442f28 ("drm: reference count event->completion")
> Cc: <stable@vger.kernel.org> # v4.11+
> Signed-off-by: Leo (Sunpeng) Li <sunpeng.li@amd.com>
> Acked-by: Harry Wentland <harry.wentland@amd.com> #v1
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
> drivers/gpu/drm/drm_atomic_helper.c | 15 +++++++++++++++
> include/drm/drm_atomic.h | 9 +++++++++
> 2 files changed, 24 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index ab4032167094..ae3cbfe9e01c 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -1878,6 +1878,8 @@ int drm_atomic_helper_setup_commit(struct drm_atomic_state *state,
> new_crtc_state->event->base.completion = &commit->flip_done;
> new_crtc_state->event->base.completion_release = release_crtc_commit;
> drm_crtc_commit_get(commit);
> +
> + commit->abort_completion = true;
> }
>
> for_each_oldnew_connector_in_state(state, conn, old_conn_state, new_conn_state, i) {
> @@ -3421,8 +3423,21 @@ EXPORT_SYMBOL(drm_atomic_helper_crtc_duplicate_state);
> void __drm_atomic_helper_crtc_destroy_state(struct drm_crtc_state *state)
> {
> if (state->commit) {
> + /*
> + * In the event that a non-blocking commit returns
> + * -ERESTARTSYS before the commit_tail work is queued, we will
> + * have an extra reference to the commit object. Release it, if
> + * the event has not been consumed by the worker.
> + *
> + * state->event may be freed, so we can't directly look at
> + * state->event->base.completion.
> + */
> + if (state->event && state->commit->abort_completion)
> + drm_crtc_commit_put(state->commit);
> +
> kfree(state->commit->event);
> state->commit->event = NULL;
> +
> drm_crtc_commit_put(state->commit);
> }
>
> diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
> index 1c27526c499e..cf13842a6dbd 100644
> --- a/include/drm/drm_atomic.h
> +++ b/include/drm/drm_atomic.h
> @@ -134,6 +134,15 @@ struct drm_crtc_commit {
> * &drm_pending_vblank_event pointer to clean up private events.
> */
> struct drm_pending_vblank_event *event;
> +
> + /**
> + * @abort_completion:
> + *
> + * A flag that's set after drm_atomic_helper_setup_commit takes a second
> + * reference for the completion of $drm_crtc_state.event. It's used by
> + * the free code to remove the second reference if commit fails.
> + */
Perhaps it's just me, or I'm oversimplifying the problem. I think this would
be easier to understand if we just dropped the additional reference at the point
of failure (ie: in swap_state). That way we don't have to add Yet Another Piece
Of State.
Sean
> + bool abort_completion;
> };
>
> struct __drm_planes_state {
> --
> 2.15.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Sean Paul, Software Engineer, Google / Chromium OS
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [Intel-gfx] [PATCH] drm/atomic: Fix memleak on ERESTARTSYS during non-blocking commits
2018-01-17 18:29 ` [Intel-gfx] " Sean Paul
@ 2018-01-17 18:39 ` Maarten Lankhorst
2018-01-17 20:18 ` Sean Paul
0 siblings, 1 reply; 9+ messages in thread
From: Maarten Lankhorst @ 2018-01-17 18:39 UTC (permalink / raw)
To: Sean Paul; +Cc: dri-devel, Leo (Sunpeng) Li, intel-gfx, Harry Wentland, stable
Op 17-01-18 om 19:29 schreef Sean Paul:
> On Wed, Jan 17, 2018 at 12:51:08PM +0100, Maarten Lankhorst wrote:
>> From: "Leo (Sunpeng) Li" <sunpeng.li@amd.com>
>>
>> During a non-blocking commit, it is possible to return before the
>> commit_tail work is queued (-ERESTARTSYS, for example).
>>
>> Since a reference on the crtc commit object is obtained for the pending
>> vblank event when preparing the commit, the above situation will leave
>> us with an extra reference.
>>
>> Therefore, if the commit_tail worker has not consumed the event at the
>> end of a commit, release it's reference.
>>
>> Changes since v1:
>> - Also check for state->event->base.completion being set, to
>> handle the case where stall_checks() fails in setup_crtc_commit().
>> Changes since v2:
>> - Add a flag to drm_crtc_commit, to prevent dereferencing a freed event.
>> i915 may unreference the state in a worker.
>>
>> Fixes: 24835e442f28 ("drm: reference count event->completion")
>> Cc: <stable@vger.kernel.org> # v4.11+
>> Signed-off-by: Leo (Sunpeng) Li <sunpeng.li@amd.com>
>> Acked-by: Harry Wentland <harry.wentland@amd.com> #v1
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> ---
>> drivers/gpu/drm/drm_atomic_helper.c | 15 +++++++++++++++
>> include/drm/drm_atomic.h | 9 +++++++++
>> 2 files changed, 24 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
>> index ab4032167094..ae3cbfe9e01c 100644
>> --- a/drivers/gpu/drm/drm_atomic_helper.c
>> +++ b/drivers/gpu/drm/drm_atomic_helper.c
>> @@ -1878,6 +1878,8 @@ int drm_atomic_helper_setup_commit(struct drm_atomic_state *state,
>> new_crtc_state->event->base.completion = &commit->flip_done;
>> new_crtc_state->event->base.completion_release = release_crtc_commit;
>> drm_crtc_commit_get(commit);
>> +
>> + commit->abort_completion = true;
>> }
>>
>> for_each_oldnew_connector_in_state(state, conn, old_conn_state, new_conn_state, i) {
>> @@ -3421,8 +3423,21 @@ EXPORT_SYMBOL(drm_atomic_helper_crtc_duplicate_state);
>> void __drm_atomic_helper_crtc_destroy_state(struct drm_crtc_state *state)
>> {
>> if (state->commit) {
>> + /*
>> + * In the event that a non-blocking commit returns
>> + * -ERESTARTSYS before the commit_tail work is queued, we will
>> + * have an extra reference to the commit object. Release it, if
>> + * the event has not been consumed by the worker.
>> + *
>> + * state->event may be freed, so we can't directly look at
>> + * state->event->base.completion.
>> + */
>> + if (state->event && state->commit->abort_completion)
>> + drm_crtc_commit_put(state->commit);
>> +
>> kfree(state->commit->event);
>> state->commit->event = NULL;
>> +
>> drm_crtc_commit_put(state->commit);
>> }
>>
>> diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
>> index 1c27526c499e..cf13842a6dbd 100644
>> --- a/include/drm/drm_atomic.h
>> +++ b/include/drm/drm_atomic.h
>> @@ -134,6 +134,15 @@ struct drm_crtc_commit {
>> * &drm_pending_vblank_event pointer to clean up private events.
>> */
>> struct drm_pending_vblank_event *event;
>> +
>> + /**
>> + * @abort_completion:
>> + *
>> + * A flag that's set after drm_atomic_helper_setup_commit takes a second
>> + * reference for the completion of $drm_crtc_state.event. It's used by
>> + * the free code to remove the second reference if commit fails.
>> + */
> Perhaps it's just me, or I'm oversimplifying the problem. I think this would
> be easier to understand if we just dropped the additional reference at the point
> of failure (ie: in swap_state). That way we don't have to add Yet Another Piece
> Of State.
That's assuming nothing can fail between setup_commit() and swap_state() and
also that the driver implementing atomci puts no calls in between. And also
assumes that even setup_commit has proper rollback. I think it's overkill,
and we have no choice but to do it like this. :(
~Maarten
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [Intel-gfx] [PATCH] drm/atomic: Fix memleak on ERESTARTSYS during non-blocking commits
2018-01-17 18:39 ` Maarten Lankhorst
@ 2018-01-17 20:18 ` Sean Paul
2018-01-29 15:41 ` Leo Li
0 siblings, 1 reply; 9+ messages in thread
From: Sean Paul @ 2018-01-17 20:18 UTC (permalink / raw)
To: Maarten Lankhorst
Cc: dri-devel, Leo (Sunpeng) Li, Intel Graphics Development,
Harry Wentland, stable
On Wed, Jan 17, 2018 at 10:39 AM, Maarten Lankhorst
<maarten.lankhorst@linux.intel.com> wrote:
> Op 17-01-18 om 19:29 schreef Sean Paul:
>> On Wed, Jan 17, 2018 at 12:51:08PM +0100, Maarten Lankhorst wrote:
>>> From: "Leo (Sunpeng) Li" <sunpeng.li@amd.com>
>>>
>>> During a non-blocking commit, it is possible to return before the
>>> commit_tail work is queued (-ERESTARTSYS, for example).
>>>
>>> Since a reference on the crtc commit object is obtained for the pending
>>> vblank event when preparing the commit, the above situation will leave
>>> us with an extra reference.
>>>
>>> Therefore, if the commit_tail worker has not consumed the event at the
>>> end of a commit, release it's reference.
>>>
>>> Changes since v1:
>>> - Also check for state->event->base.completion being set, to
>>> handle the case where stall_checks() fails in setup_crtc_commit().
>>> Changes since v2:
>>> - Add a flag to drm_crtc_commit, to prevent dereferencing a freed event.
>>> i915 may unreference the state in a worker.
>>>
>>> Fixes: 24835e442f28 ("drm: reference count event->completion")
>>> Cc: <stable@vger.kernel.org> # v4.11+
>>> Signed-off-by: Leo (Sunpeng) Li <sunpeng.li@amd.com>
>>> Acked-by: Harry Wentland <harry.wentland@amd.com> #v1
>>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>>> ---
>>> drivers/gpu/drm/drm_atomic_helper.c | 15 +++++++++++++++
>>> include/drm/drm_atomic.h | 9 +++++++++
>>> 2 files changed, 24 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
>>> index ab4032167094..ae3cbfe9e01c 100644
>>> --- a/drivers/gpu/drm/drm_atomic_helper.c
>>> +++ b/drivers/gpu/drm/drm_atomic_helper.c
>>> @@ -1878,6 +1878,8 @@ int drm_atomic_helper_setup_commit(struct drm_atomic_state *state,
>>> new_crtc_state->event->base.completion = &commit->flip_done;
>>> new_crtc_state->event->base.completion_release = release_crtc_commit;
>>> drm_crtc_commit_get(commit);
>>> +
>>> + commit->abort_completion = true;
>>> }
>>>
>>> for_each_oldnew_connector_in_state(state, conn, old_conn_state, new_conn_state, i) {
>>> @@ -3421,8 +3423,21 @@ EXPORT_SYMBOL(drm_atomic_helper_crtc_duplicate_state);
>>> void __drm_atomic_helper_crtc_destroy_state(struct drm_crtc_state *state)
>>> {
>>> if (state->commit) {
>>> + /*
>>> + * In the event that a non-blocking commit returns
>>> + * -ERESTARTSYS before the commit_tail work is queued, we will
>>> + * have an extra reference to the commit object. Release it, if
>>> + * the event has not been consumed by the worker.
>>> + *
>>> + * state->event may be freed, so we can't directly look at
>>> + * state->event->base.completion.
>>> + */
>>> + if (state->event && state->commit->abort_completion)
>>> + drm_crtc_commit_put(state->commit);
>>> +
>>> kfree(state->commit->event);
>>> state->commit->event = NULL;
>>> +
>>> drm_crtc_commit_put(state->commit);
>>> }
>>>
>>> diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
>>> index 1c27526c499e..cf13842a6dbd 100644
>>> --- a/include/drm/drm_atomic.h
>>> +++ b/include/drm/drm_atomic.h
>>> @@ -134,6 +134,15 @@ struct drm_crtc_commit {
>>> * &drm_pending_vblank_event pointer to clean up private events.
>>> */
>>> struct drm_pending_vblank_event *event;
>>> +
>>> + /**
>>> + * @abort_completion:
>>> + *
>>> + * A flag that's set after drm_atomic_helper_setup_commit takes a second
>>> + * reference for the completion of $drm_crtc_state.event. It's used by
>>> + * the free code to remove the second reference if commit fails.
>>> + */
>> Perhaps it's just me, or I'm oversimplifying the problem. I think this would
>> be easier to understand if we just dropped the additional reference at the point
>> of failure (ie: in swap_state). That way we don't have to add Yet Another Piece
>> Of State.
>
> That's assuming nothing can fail between setup_commit() and swap_state() and
> also that the driver implementing atomci puts no calls in between. And also
> assumes that even setup_commit has proper rollback. I think it's overkill,
> and we have no choice but to do it like this. :(
>
yeah, fair enough.
Reviewed-by: Sean Paul <seanpaul@chromium.org>
> ~Maarten
>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [Intel-gfx] [PATCH] drm/atomic: Fix memleak on ERESTARTSYS during non-blocking commits
2018-01-17 20:18 ` Sean Paul
@ 2018-01-29 15:41 ` Leo Li
2018-01-30 10:28 ` Maarten Lankhorst
0 siblings, 1 reply; 9+ messages in thread
From: Leo Li @ 2018-01-29 15:41 UTC (permalink / raw)
To: dri-devel
Cc: Maarten Lankhorst, Intel Graphics Development, Harry Wentland,
stable, Daniel Vetter, Jani Nikula, Sean Paul
Updated IGT results seem sane:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7698/shards.html
Would someone be able to apply this patch?
Thanks,
Leo
On 2018-01-17 03:18 PM, Sean Paul wrote:
> On Wed, Jan 17, 2018 at 10:39 AM, Maarten Lankhorst
> <maarten.lankhorst@linux.intel.com> wrote:
>> Op 17-01-18 om 19:29 schreef Sean Paul:
>>> On Wed, Jan 17, 2018 at 12:51:08PM +0100, Maarten Lankhorst wrote:
>>>> From: "Leo (Sunpeng) Li" <sunpeng.li@amd.com>
>>>>
>>>> During a non-blocking commit, it is possible to return before the
>>>> commit_tail work is queued (-ERESTARTSYS, for example).
>>>>
>>>> Since a reference on the crtc commit object is obtained for the pending
>>>> vblank event when preparing the commit, the above situation will leave
>>>> us with an extra reference.
>>>>
>>>> Therefore, if the commit_tail worker has not consumed the event at the
>>>> end of a commit, release it's reference.
>>>>
>>>> Changes since v1:
>>>> - Also check for state->event->base.completion being set, to
>>>> handle the case where stall_checks() fails in setup_crtc_commit().
>>>> Changes since v2:
>>>> - Add a flag to drm_crtc_commit, to prevent dereferencing a freed event.
>>>> i915 may unreference the state in a worker.
>>>>
>>>> Fixes: 24835e442f28 ("drm: reference count event->completion")
>>>> Cc: <stable@vger.kernel.org> # v4.11+
>>>> Signed-off-by: Leo (Sunpeng) Li <sunpeng.li@amd.com>
>>>> Acked-by: Harry Wentland <harry.wentland@amd.com> #v1
>>>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>>>> ---
>>>> drivers/gpu/drm/drm_atomic_helper.c | 15 +++++++++++++++
>>>> include/drm/drm_atomic.h | 9 +++++++++
>>>> 2 files changed, 24 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
>>>> index ab4032167094..ae3cbfe9e01c 100644
>>>> --- a/drivers/gpu/drm/drm_atomic_helper.c
>>>> +++ b/drivers/gpu/drm/drm_atomic_helper.c
>>>> @@ -1878,6 +1878,8 @@ int drm_atomic_helper_setup_commit(struct drm_atomic_state *state,
>>>> new_crtc_state->event->base.completion = &commit->flip_done;
>>>> new_crtc_state->event->base.completion_release = release_crtc_commit;
>>>> drm_crtc_commit_get(commit);
>>>> +
>>>> + commit->abort_completion = true;
>>>> }
>>>>
>>>> for_each_oldnew_connector_in_state(state, conn, old_conn_state, new_conn_state, i) {
>>>> @@ -3421,8 +3423,21 @@ EXPORT_SYMBOL(drm_atomic_helper_crtc_duplicate_state);
>>>> void __drm_atomic_helper_crtc_destroy_state(struct drm_crtc_state *state)
>>>> {
>>>> if (state->commit) {
>>>> + /*
>>>> + * In the event that a non-blocking commit returns
>>>> + * -ERESTARTSYS before the commit_tail work is queued, we will
>>>> + * have an extra reference to the commit object. Release it, if
>>>> + * the event has not been consumed by the worker.
>>>> + *
>>>> + * state->event may be freed, so we can't directly look at
>>>> + * state->event->base.completion.
>>>> + */
>>>> + if (state->event && state->commit->abort_completion)
>>>> + drm_crtc_commit_put(state->commit);
>>>> +
>>>> kfree(state->commit->event);
>>>> state->commit->event = NULL;
>>>> +
>>>> drm_crtc_commit_put(state->commit);
>>>> }
>>>>
>>>> diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
>>>> index 1c27526c499e..cf13842a6dbd 100644
>>>> --- a/include/drm/drm_atomic.h
>>>> +++ b/include/drm/drm_atomic.h
>>>> @@ -134,6 +134,15 @@ struct drm_crtc_commit {
>>>> * &drm_pending_vblank_event pointer to clean up private events.
>>>> */
>>>> struct drm_pending_vblank_event *event;
>>>> +
>>>> + /**
>>>> + * @abort_completion:
>>>> + *
>>>> + * A flag that's set after drm_atomic_helper_setup_commit takes a second
>>>> + * reference for the completion of $drm_crtc_state.event. It's used by
>>>> + * the free code to remove the second reference if commit fails.
>>>> + */
>>> Perhaps it's just me, or I'm oversimplifying the problem. I think this would
>>> be easier to understand if we just dropped the additional reference at the point
>>> of failure (ie: in swap_state). That way we don't have to add Yet Another Piece
>>> Of State.
>>
>> That's assuming nothing can fail between setup_commit() and swap_state() and
>> also that the driver implementing atomci puts no calls in between. And also
>> assumes that even setup_commit has proper rollback. I think it's overkill,
>> and we have no choice but to do it like this. :(
>>
>
> yeah, fair enough.
>
> Reviewed-by: Sean Paul <seanpaul@chromium.org>
>
>> ~Maarten
>>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [Intel-gfx] [PATCH] drm/atomic: Fix memleak on ERESTARTSYS during non-blocking commits
2018-01-29 15:41 ` Leo Li
@ 2018-01-30 10:28 ` Maarten Lankhorst
2018-01-31 19:57 ` Harry Wentland
0 siblings, 1 reply; 9+ messages in thread
From: Maarten Lankhorst @ 2018-01-30 10:28 UTC (permalink / raw)
To: Leo Li, dri-devel
Cc: Intel Graphics Development, Harry Wentland, stable, Daniel Vetter,
Jani Nikula, Sean Paul
Op 29-01-18 om 16:41 schreef Leo Li:
> Updated IGT results seem sane:
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7698/shards.html
>
> Would someone be able to apply this patch?
>
Thanks for the reminder, pushed.
~Maarten
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/atomic: Fix memleak on ERESTARTSYS during non-blocking commits
2018-01-30 10:28 ` Maarten Lankhorst
@ 2018-01-31 19:57 ` Harry Wentland
2018-02-01 10:30 ` Maarten Lankhorst
0 siblings, 1 reply; 9+ messages in thread
From: Harry Wentland @ 2018-01-31 19:57 UTC (permalink / raw)
To: Maarten Lankhorst, Leo Li, dri-devel
Cc: Intel Graphics Development, stable, Daniel Vetter, Jani Nikula,
Sean Paul
On 2018-01-30 05:28 AM, Maarten Lankhorst wrote:
> Op 29-01-18 om 16:41 schreef Leo Li:
>> Updated IGT results seem sane:
>> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7698/shards.html
>>
>> Would someone be able to apply this patch?
>>
> Thanks for the reminder, pushed.
>
Thanks, Maarten. I see it in drm-misc-next.
Would someone be able to pull this into drm-misc-fixes as well, or can I just I apply this myself with the following dim commands?
dim checkout drm-misc-fixes
dim cherry-pick 1c6ceeee6ebb
dim push-branch
Harry
> ~Maarten
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/atomic: Fix memleak on ERESTARTSYS during non-blocking commits
2018-01-31 19:57 ` Harry Wentland
@ 2018-02-01 10:30 ` Maarten Lankhorst
2018-02-01 14:29 ` Harry Wentland
0 siblings, 1 reply; 9+ messages in thread
From: Maarten Lankhorst @ 2018-02-01 10:30 UTC (permalink / raw)
To: Harry Wentland, Leo Li, dri-devel
Cc: Intel Graphics Development, stable, Daniel Vetter, Jani Nikula,
Sean Paul
Op 31-01-18 om 20:57 schreef Harry Wentland:
> On 2018-01-30 05:28 AM, Maarten Lankhorst wrote:
>> Op 29-01-18 om 16:41 schreef Leo Li:
>>> Updated IGT results seem sane:
>>> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7698/shards.html
>>>
>>> Would someone be able to apply this patch?
>>>
>> Thanks for the reminder, pushed.
>>
> Thanks, Maarten. I see it in drm-misc-next.
>
> Would someone be able to pull this into drm-misc-fixes as well, or can I just I apply this myself with the following dim commands?
>
> dim checkout drm-misc-fixes
> dim cherry-pick 1c6ceeee6ebb
> dim push-branch
My bad, pushed to the right branch. :)
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/atomic: Fix memleak on ERESTARTSYS during non-blocking commits
2018-02-01 10:30 ` Maarten Lankhorst
@ 2018-02-01 14:29 ` Harry Wentland
0 siblings, 0 replies; 9+ messages in thread
From: Harry Wentland @ 2018-02-01 14:29 UTC (permalink / raw)
To: Maarten Lankhorst, Leo Li, dri-devel
Cc: Intel Graphics Development, stable, Daniel Vetter, Jani Nikula,
Sean Paul
On 2018-02-01 05:30 AM, Maarten Lankhorst wrote:
> Op 31-01-18 om 20:57 schreef Harry Wentland:
>> On 2018-01-30 05:28 AM, Maarten Lankhorst wrote:
>>> Op 29-01-18 om 16:41 schreef Leo Li:
>>>> Updated IGT results seem sane:
>>>> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7698/shards.html
>>>>
>>>> Would someone be able to apply this patch?
>>>>
>>> Thanks for the reminder, pushed.
>>>
>> Thanks, Maarten. I see it in drm-misc-next.
>>
>> Would someone be able to pull this into drm-misc-fixes as well, or can I just I apply this myself with the following dim commands?
>>
>> dim checkout drm-misc-fixes
>> dim cherry-pick 1c6ceeee6ebb
>> dim push-branch
> My bad, pushed to the right branch. :)
>
Thanks, Maarten.
Harry
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2018-02-01 14:29 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-01-17 11:51 [PATCH] drm/atomic: Fix memleak on ERESTARTSYS during non-blocking commits Maarten Lankhorst
2018-01-17 18:29 ` [Intel-gfx] " Sean Paul
2018-01-17 18:39 ` Maarten Lankhorst
2018-01-17 20:18 ` Sean Paul
2018-01-29 15:41 ` Leo Li
2018-01-30 10:28 ` Maarten Lankhorst
2018-01-31 19:57 ` Harry Wentland
2018-02-01 10:30 ` Maarten Lankhorst
2018-02-01 14:29 ` Harry Wentland
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).