Skip to content

Commit

Permalink
Add DCHECK for the running thread in GPU watchdog V2
Browse files Browse the repository at this point in the history
Some of the functions are allowed to run only on the watchdog thread or
the watched GPU thread. DCHECKs are added to these functions to make sure
multi-threading is running correctly.

For those functions that can run on any threads, no DCHECK is added.

Bug: 949839
Change-Id: If451934db201d8544a427c007995900361663768
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1773436
Reviewed-by: Zhenyao Mo <zmo@chromium.org>
Commit-Queue: Maggie Chen <magchen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#690864}
  • Loading branch information
Maggie Chen authored and Commit Bot committed Aug 27, 2019
1 parent 5dd6e80 commit d51976b
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 5 deletions.
38 changes: 34 additions & 4 deletions gpu/ipc/service/gpu_watchdog_thread_v2.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,13 @@ GpuWatchdogThreadImplV2::GpuWatchdogThreadImplV2(base::TimeDelta timeout,
bool is_test_mode)
: watchdog_timeout_(timeout),
is_test_mode_(is_test_mode),
watched_task_runner_(base::ThreadTaskRunnerHandle::Get()) {
watched_gpu_task_runner_(base::ThreadTaskRunnerHandle::Get()) {
base::MessageLoopCurrent::Get()->AddTaskObserver(this);
Arm();
}

GpuWatchdogThreadImplV2::~GpuWatchdogThreadImplV2() {
DCHECK(watched_task_runner_->BelongsToCurrentThread());
DCHECK(watched_gpu_task_runner_->BelongsToCurrentThread());
Stop(); // stop the watchdog thread

base::MessageLoopCurrent::Get()->RemoveTaskObserver(this);
Expand Down Expand Up @@ -59,6 +59,8 @@ std::unique_ptr<GpuWatchdogThreadImplV2> GpuWatchdogThreadImplV2::Create(
// Do not add power observer during watchdog init, PowerMonitor might not be up
// running yet.
void GpuWatchdogThreadImplV2::AddPowerObserver() {
DCHECK(watched_gpu_task_runner_->BelongsToCurrentThread());

// Forward it to the watchdog thread. Call PowerMonitor::AddObserver on the
// watchdog thread so that OnSuspend and OnResume will be called on watchdog
// thread.
Expand Down Expand Up @@ -86,6 +88,7 @@ void GpuWatchdogThreadImplV2::OnForegrounded() {

// Called from the gpu thread when gpu init has completed
void GpuWatchdogThreadImplV2::OnInitComplete() {
DCHECK(watched_gpu_task_runner_->BelongsToCurrentThread());
Disarm();
}

Expand All @@ -94,40 +97,49 @@ void GpuWatchdogThreadImplV2::OnInitComplete() {
// watchdog is stopped and then restarted in StartSandboxLinux(). Everything
// should be the same and continue after the second init().
void GpuWatchdogThreadImplV2::Init() {
last_arm_disarm_counter_ = base::subtle::NoBarrier_Load(&arm_disarm_counter_);
watchdog_thread_task_runner_ = base::ThreadTaskRunnerHandle::Get();

// Get and Invalidate weak_ptr should be done on the watchdog thread only.
weak_ptr_ = weak_factory_.GetWeakPtr();
task_runner()->PostDelayedTask(
FROM_HERE,
base::BindOnce(&GpuWatchdogThreadImplV2::OnWatchdogTimeout, weak_ptr_),
watchdog_timeout_ * kInitFactor);

last_arm_disarm_counter_ = base::subtle::NoBarrier_Load(&arm_disarm_counter_);
watchdog_start_timeticks_ = base::TimeTicks::Now();
last_on_watchdog_timeout_timeticks_ = watchdog_start_timeticks_;
last_on_watchdog_timeout_time_ = base::Time::Now();

GpuWatchdogHistogram(GpuWatchdogThreadEvent::kGpuWatchdogStart);
}

// Running on the watchdog thread.
void GpuWatchdogThreadImplV2::CleanUp() {
DCHECK(watchdog_thread_task_runner_->BelongsToCurrentThread());
weak_factory_.InvalidateWeakPtrs();
}

void GpuWatchdogThreadImplV2::ReportProgress() {
DCHECK(watched_gpu_task_runner_->BelongsToCurrentThread());
InProgress();
}

void GpuWatchdogThreadImplV2::WillProcessTask(
const base::PendingTask& pending_task) {
DCHECK(watched_gpu_task_runner_->BelongsToCurrentThread());
Arm();
}

void GpuWatchdogThreadImplV2::DidProcessTask(
const base::PendingTask& pending_task) {
DCHECK(watched_gpu_task_runner_->BelongsToCurrentThread());
Disarm();
}

// Running on the watchdog thread.
void GpuWatchdogThreadImplV2::OnSuspend() {
DCHECK(watchdog_thread_task_runner_->BelongsToCurrentThread());
in_power_suspension_ = true;
// Revoke any pending watchdog timeout task
weak_factory_.InvalidateWeakPtrs();
Expand All @@ -136,6 +148,8 @@ void GpuWatchdogThreadImplV2::OnSuspend() {

// Running on the watchdog thread.
void GpuWatchdogThreadImplV2::OnResume() {
DCHECK(watchdog_thread_task_runner_->BelongsToCurrentThread());

in_power_suspension_ = false;
RestartWatchdogTimeoutTask();
resume_timeticks_ = base::TimeTicks::Now();
Expand All @@ -144,12 +158,16 @@ void GpuWatchdogThreadImplV2::OnResume() {

// Running on the watchdog thread.
void GpuWatchdogThreadImplV2::OnAddPowerObserver() {
DCHECK(watchdog_thread_task_runner_->BelongsToCurrentThread());
DCHECK(base::PowerMonitor::IsInitialized());

is_power_observer_added_ = base::PowerMonitor::AddObserver(this);
}

// Running on the watchdog thread.
void GpuWatchdogThreadImplV2::OnWatchdogBackgrounded() {
DCHECK(watchdog_thread_task_runner_->BelongsToCurrentThread());

is_backgrounded_ = true;
// Revoke any pending watchdog timeout task
weak_factory_.InvalidateWeakPtrs();
Expand All @@ -158,13 +176,17 @@ void GpuWatchdogThreadImplV2::OnWatchdogBackgrounded() {

// Running on the watchdog thread.
void GpuWatchdogThreadImplV2::OnWatchdogForegrounded() {
DCHECK(watchdog_thread_task_runner_->BelongsToCurrentThread());

is_backgrounded_ = false;
RestartWatchdogTimeoutTask();
foregrounded_timeticks_ = base::TimeTicks::Now();
}

// Running on the watchdog thread.
void GpuWatchdogThreadImplV2::RestartWatchdogTimeoutTask() {
DCHECK(watchdog_thread_task_runner_->BelongsToCurrentThread());

if (!is_backgrounded_ && !in_power_suspension_) {
// Make the timeout twice long. The system/gpu might be very slow right
// after resume or foregrounded.
Expand All @@ -179,20 +201,26 @@ void GpuWatchdogThreadImplV2::RestartWatchdogTimeoutTask() {
}

void GpuWatchdogThreadImplV2::Arm() {
DCHECK(watched_gpu_task_runner_->BelongsToCurrentThread());

base::subtle::NoBarrier_AtomicIncrement(&arm_disarm_counter_, 1);

// Arm/Disarm are always called in sequence. Now it's an odd number.
DCHECK(base::subtle::NoBarrier_Load(&arm_disarm_counter_) & 1);
}

void GpuWatchdogThreadImplV2::Disarm() {
DCHECK(watched_gpu_task_runner_->BelongsToCurrentThread());

base::subtle::NoBarrier_AtomicIncrement(&arm_disarm_counter_, 1);

// Arm/Disarm are always called in sequence. Now it's an even number.
DCHECK(base::subtle::NoBarrier_Load(&arm_disarm_counter_) % 2 == 0);
}

void GpuWatchdogThreadImplV2::InProgress() {
DCHECK(watched_gpu_task_runner_->BelongsToCurrentThread());

// Increment by 2. This is equivalent to Disarm() + Arm().
base::subtle::NoBarrier_AtomicIncrement(&arm_disarm_counter_, 2);

Expand All @@ -202,6 +230,7 @@ void GpuWatchdogThreadImplV2::InProgress() {

// Running on the watchdog thread.
void GpuWatchdogThreadImplV2::OnWatchdogTimeout() {
DCHECK(watchdog_thread_task_runner_->BelongsToCurrentThread());
DCHECK(!is_backgrounded_);
DCHECK(!in_power_suspension_);
base::subtle::Atomic32 arm_disarm_counter =
Expand Down Expand Up @@ -230,6 +259,7 @@ void GpuWatchdogThreadImplV2::OnWatchdogTimeout() {
}

void GpuWatchdogThreadImplV2::DeliberatelyTerminateToRecoverFromHang() {
DCHECK(watchdog_thread_task_runner_->BelongsToCurrentThread());
// If this is for gpu testing, do not terminate the gpu process.
if (is_test_mode_) {
test_result_timeout_and_gpu_hang_.Set();
Expand Down Expand Up @@ -291,7 +321,7 @@ bool GpuWatchdogThreadImplV2::IsGpuHangDetectedForTesting() {
// This should be called on the test main thread only. It will wait until the
// power observer is added on the watchdog thread.
void GpuWatchdogThreadImplV2::WaitForPowerObserverAddedForTesting() {
DCHECK(watched_task_runner_->BelongsToCurrentThread());
DCHECK(watched_gpu_task_runner_->BelongsToCurrentThread());
DCHECK(is_add_power_observer_called_);

// Just return if it has been added.
Expand Down
3 changes: 2 additions & 1 deletion gpu/ipc/service/gpu_watchdog_thread_v2.h
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,8 @@ class GPU_IPC_SERVICE_EXPORT GpuWatchdogThreadImplV2
// Set by the watchdog thread and Read by the test thread.
base::AtomicFlag test_result_timeout_and_gpu_hang_;

scoped_refptr<base::SingleThreadTaskRunner> watched_task_runner_;
scoped_refptr<base::SingleThreadTaskRunner> watched_gpu_task_runner_;
scoped_refptr<base::SingleThreadTaskRunner> watchdog_thread_task_runner_;

base::WeakPtr<GpuWatchdogThreadImplV2> weak_ptr_;
base::WeakPtrFactory<GpuWatchdogThreadImplV2> weak_factory_{this};
Expand Down

0 comments on commit d51976b

Please sign in to comment.