public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: John Stultz <john.stultz@linaro.org>
To: LKML <linux-kernel@vger.kernel.org>
Cc: Prakash Kamliya <pkamliya@codeaurora.org>,
	Colin Cross <ccross@android.com>,
	Android Kernel Team <kernel-team@android.com>,
	John Stultz <john.stultz@linaro.org>
Subject: [PATCH 06/16] staging: sync: Signal pt before sync_timeline object gets destroyed
Date: Mon,  3 Feb 2014 10:16:18 -0800	[thread overview]
Message-ID: <1391451388-23906-7-git-send-email-john.stultz@linaro.org> (raw)
In-Reply-To: <1391451388-23906-1-git-send-email-john.stultz@linaro.org>

From: Prakash Kamliya <pkamliya@codeaurora.org>

There is a race condition

Assume we have *one* sync_fence object, with *one* sync_pt
which belongs to *one* sync_timeline, given this condition,
sync_timeline->kref will have two counts, one for sync_timeline
(implicit) and another for sync_pt.

Assume following is the situation on CPU

Theead-1 : (Thread which calls sync_timeline_destroy())
  -> (some function calls)
   -> sync_timeline_destory()
    -> sync_timeline_signal() (CPU is inside this
function after putting reference to sync_timeline)

At this time Thread-2 comes and does following

Thread-2 : (fclose on fence fd)
> sync_fence_release() -> because of fclose() on fence object
 -> sync_fence_free()
  -> sync_pt_free()
   -> kref_put(&pt->parent->kref, sync_timeline_free);
    -> sync_timeline_free() (CPU is inside this because
this time kref will be zero after _put)

Thread-2 will free sync_timeline object before Thread-1
has finished its work inside sync_timeline_signal.

With this change we signals all sync_pt before putting
reference to sync_timeline object.

c: Greg KH <gregkh@linuxfoundation.org>
Cc: Colin Cross <ccross@android.com>
Cc: Android Kernel Team <kernel-team@android.com>
Signed-off-by: Prakash Kamliya <pkamliya@codeaurora.org>
[jstultz: minor commit subject tweak]
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 drivers/staging/android/sync.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c
index 38e5d3b..fec2d1c 100644
--- a/drivers/staging/android/sync.c
+++ b/drivers/staging/android/sync.c
@@ -92,14 +92,14 @@ static void sync_timeline_free(struct kref *kref)
 void sync_timeline_destroy(struct sync_timeline *obj)
 {
 	obj->destroyed = true;
+	smp_wmb();
 
 	/*
-	 * If this is not the last reference, signal any children
-	 * that their parent is going away.
+	 * signal any children that their parent is going away.
 	 */
+	sync_timeline_signal(obj);
 
-	if (!kref_put(&obj->kref, sync_timeline_free))
-		sync_timeline_signal(obj);
+	kref_put(&obj->kref, sync_timeline_free);
 }
 EXPORT_SYMBOL(sync_timeline_destroy);
 
-- 
1.8.3.2


  parent reply	other threads:[~2014-02-03 18:16 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-02-03 18:16 [PATCH 00/16] [RFC] Staging updates from the Android tree John Stultz
2014-02-03 18:16 ` [PATCH 01/16] staging: android: Split uapi out of android_alarm.h John Stultz
2014-02-03 18:16 ` [PATCH 02/16] staging: android: Split uapi out of ashmem.h John Stultz
2014-02-03 18:16 ` [PATCH 03/16] staging: android: split uapi out of sync.h and sw_sync.h John Stultz
2014-02-03 18:16 ` [PATCH 04/16] staging: android: Split uapi out of binder.h John Stultz
2014-02-03 18:16 ` [PATCH 05/16] staging: sw_sync: Add stubs for kernels without CONFIG_SW_SYNC John Stultz
2014-02-03 18:16 ` John Stultz [this message]
2014-02-03 18:16 ` [PATCH 07/16] staging: sync: Fix a race condition between release_obj and print_obj John Stultz
2014-02-03 18:16 ` [PATCH 08/16] staging: ashmem: Avoid deadlock between read and mmap calls John Stultz
2014-02-03 18:16 ` [PATCH 09/16] staging: ion: Create separate heap and client debugfs directories John Stultz
2014-02-03 18:16 ` [PATCH 10/16] staging: ion: Fix debugfs handling of multiple kernel clients John Stultz
2014-02-03 18:16 ` [PATCH 11/16] staging: ion: Store a copy of the client name on client creation John Stultz
2014-02-03 18:16 ` [PATCH 12/16] staging: ion: Make sure all clients are exposed in debugfs John Stultz
2014-02-03 18:16 ` [PATCH 13/16] staging: ion: Fix overflow and list bugs in system heap John Stultz
2014-02-03 18:16 ` [PATCH 14/16] staging: ion: Fix ION_IOC_FREE compat ioctl John Stultz
2014-02-03 18:16 ` [PATCH 15/16] staging: ion: Move shrinker out of heaps John Stultz
2014-02-03 18:16 ` [PATCH 16/16] staging: ion: Add private buffer flag to skip page pooling on free John Stultz
2014-02-04  2:06 ` [PATCH 00/16] [RFC] Staging updates from the Android tree Greg KH
2014-02-04 20:14   ` John Stultz

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=1391451388-23906-7-git-send-email-john.stultz@linaro.org \
    --to=john.stultz@linaro.org \
    --cc=ccross@android.com \
    --cc=kernel-team@android.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pkamliya@codeaurora.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox