Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 801eb32e authored by Steven Thomas's avatar Steven Thomas
Browse files

Relax performanced vr:app:render permission restriction 

This CL allows apps to request the vr:app:render performanced
policy. Previously we only allowed VrCore or system processes to request
vr:app:render.

We now restrict vr:app:render to a single thread system-wide. If we
allow multiple vr:app:render threads, a misbehaving app can lock up the
entire device.

Allowing apps to directly request vr:app:render has the problem that a
misbehaving background app can "steal" vr:app:render from the foreground
app. I haven't found that to occur in practice, but it can be fixed by
having VrCore decide which app gets vr:app:render, although that will
require some extra communication between the app and VrCore.

Bug: 73722517

Test: - Made local changes to gvr apps and confirmed they can request
vr:app:render.

- Confirmed that vr:app:render transfers successfully when switching
  apps.

- Modified treasure hunt to request vr:app:render policy on multiple
  threads, and confirmed that vr:app:render transfers between the
  threads, so there's only ever one thread with vr:app:render. (This
  test also revealed that without the "only one thread gets
  vr:app:render" restriction, Vega freezes when those threads busy
  loop).

Change-Id: I2fc5ff9fae9ea5160e1637995b3d1261494d1383
parent 193f2310

services/vr/performanced/cpu_set.cpp

+1 −5
Original line number Diff line number Diff line
@@ -106,7 +106,7 @@ std::vector<CpuSet*> CpuSetManager::GetCpuSets() { 
  return sets;

}



std::string CpuSetManager::DumpState() const {

void CpuSetManager::DumpState(std::ostringstream& stream) const {

  size_t max_path = 0;

  std::vector<CpuSet*> sets;
@@ -119,8 +119,6 @@ std::string CpuSetManager::DumpState() const { 
    return a->path() < b->path();

  });



  std::ostringstream stream;



  stream << std::left;

  stream << std::setw(max_path) << "Path";

  stream << " ";
@@ -146,8 +144,6 @@ std::string CpuSetManager::DumpState() const { 
    stream << std::setw(6) << set->GetTasks().size();

    stream << std::endl;

  }



  return stream.str();

}



void CpuSetManager::MoveUnboundTasks(const std::string& target_set) {

services/vr/performanced/cpu_set.h

+2 −1
Original line number Diff line number Diff line
@@ -5,6 +5,7 @@ 


#include <memory>

#include <mutex>

#include <sstream>

#include <string>

#include <unordered_map>

#include <vector>
@@ -83,7 +84,7 @@ class CpuSetManager { 
  // to shield the system from interference from unbound kernel threads.

  void MoveUnboundTasks(const std::string& target_set);



  std::string DumpState() const;

  void DumpState(std::ostringstream& stream) const;



  operator bool() const { return root_set_ != nullptr; }

services/vr/performanced/performance_service.cpp

+75 −8
Original line number Diff line number Diff line 
#include "performance_service.h"



#include <sstream>



#include <sched.h>

#include <sys/prctl.h>

#include <unistd.h>
@@ -31,6 +33,10 @@ const char kCpuSetBasePath[] = "/dev/cpuset"; 


const char kRootCpuSet[] = "/";



const char kVrAppRenderPolicy[] = "vr:app:render";



const bool kAllowAppsToRequestVrAppRenderPolicy = false;



constexpr unsigned long kTimerSlackForegroundNs = 50000;

constexpr unsigned long kTimerSlackBackgroundNs = 40000000;
@@ -133,6 +139,17 @@ PerformanceService::PerformanceService() 
  using AllowRootSystemTrusted =

      CheckOr<Trusted, UserId<AID_ROOT, AID_SYSTEM>, GroupId<AID_SYSTEM>>;



  auto vr_app_render_permission_check = [](

      const pdx::Message& sender, const Task& task) {

          // For vr:app:render, in addition to system/root apps and VrCore, we

          // also allow apps to request vr:app:render if

          // kAllowAppsToRequestVrAppRenderPolicy == true, but not for other

          // apps, only for themselves.

          return (task && task.thread_group_id() == sender.GetProcessId() &&

                  kAllowAppsToRequestVrAppRenderPolicy)

              || AllowRootSystemTrusted::Check(sender, task);

      };



  partition_permission_check_ = AllowRootSystemTrusted::Check;



  // Setup the scheduler classes.
@@ -184,11 +201,11 @@ PerformanceService::PerformanceService() 
        .priority = fifo_medium + 2,

        .permission_check = AllowRootSystemTrusted::Check,

        "/system/performance"}},

      {"vr:app:render",

      {kVrAppRenderPolicy,

       {.timer_slack = kTimerSlackForegroundNs,

        .scheduler_policy = SCHED_FIFO | SCHED_RESET_ON_FORK,

        .priority = fifo_medium + 1,

        .permission_check = AllowRootSystemTrusted::Check,

        .permission_check = vr_app_render_permission_check,

        "/application/performance"}},

      {"normal",

       {.timer_slack = kTimerSlackForegroundNs,
@@ -215,7 +232,10 @@ bool PerformanceService::IsInitialized() const { 
}



std::string PerformanceService::DumpState(size_t /*max_length*/) {

  return cpuset_.DumpState();

  std::ostringstream stream;

  stream << "vr_app_render_thread: " << vr_app_render_thread_ << std::endl;

  cpuset_.DumpState(stream);

  return stream.str();

}



Status<void> PerformanceService::OnSetSchedulerPolicy(
@@ -241,7 +261,12 @@ Status<void> PerformanceService::OnSetSchedulerPolicy( 
    // Make sure the sending process is allowed to make the requested change to

    // this task.

    if (!config.IsAllowed(message, task))

      return ErrorStatus(EINVAL);

      return ErrorStatus(EPERM);



    if (scheduler_policy == kVrAppRenderPolicy) {

      // We only allow one vr:app:render thread at a time

      SetVrAppRenderThread(task_id);

    }



    // Get the thread group's cpu set. Policies that do not specify a cpuset

    // should default to this cpuset.
@@ -299,14 +324,16 @@ Status<void> PerformanceService::OnSetSchedulerPolicy( 
Status<void> PerformanceService::OnSetCpuPartition(

    Message& message, pid_t task_id, const std::string& partition) {

  Task task(task_id);

  if (!task || task.thread_group_id() != message.GetProcessId())

  if (!task)

    return ErrorStatus(EINVAL);

  if (task.thread_group_id() != message.GetProcessId())

    return ErrorStatus(EPERM);



  // Temporary permission check.

  // TODO(eieio): Replace this with a configuration file.

  if (partition_permission_check_ &&

      !partition_permission_check_(message, task)) {

    return ErrorStatus(EINVAL);

    return ErrorStatus(EPERM);

  }



  auto target_set = cpuset_.Lookup(partition);
@@ -333,7 +360,12 @@ Status<void> PerformanceService::OnSetSchedulerClass( 
    // Make sure the sending process is allowed to make the requested change to

    // this task.

    if (!config.IsAllowed(message, task))

      return ErrorStatus(EINVAL);

      return ErrorStatus(EPERM);



    if (scheduler_class == kVrAppRenderPolicy) {

      // We only allow one vr:app:render thread at a time

      SetVrAppRenderThread(task_id);

    }



    struct sched_param param;

    param.sched_priority = config.priority;
@@ -356,8 +388,10 @@ Status<std::string> PerformanceService::OnGetCpuPartition(Message& message, 
                                                          pid_t task_id) {

  // Make sure the task id is valid and belongs to the sending process.

  Task task(task_id);

  if (!task || task.thread_group_id() != message.GetProcessId())

  if (!task)

    return ErrorStatus(EINVAL);

  if (task.thread_group_id() != message.GetProcessId())

    return ErrorStatus(EPERM);



  return task.GetCpuSetPath();

}
@@ -390,5 +424,38 @@ Status<void> PerformanceService::HandleMessage(Message& message) { 
  }

}



void PerformanceService::SetVrAppRenderThread(pid_t new_vr_app_render_thread) {

  ALOGI("SetVrAppRenderThread old=%d new=%d",

      vr_app_render_thread_, new_vr_app_render_thread);



  if (vr_app_render_thread_ >= 0 &&

      vr_app_render_thread_ != new_vr_app_render_thread) {

    // Restore the default scheduler policy and priority on the previous

    // vr:app:render thread.

    struct sched_param param;

    param.sched_priority = 0;

    if (sched_setscheduler(vr_app_render_thread_, SCHED_NORMAL, &param) < 0) {

      if (errno == ESRCH) {

        ALOGI("Failed to revert %s scheduler policy. Couldn't find thread %d."

            " Was the app killed?", kVrAppRenderPolicy, vr_app_render_thread_);

      } else {

        ALOGE("Failed to revert %s scheduler policy: %s",

            kVrAppRenderPolicy, strerror(errno));

      }

    }



    // Restore the default timer slack on the previous vr:app:render thread.

    prctl(PR_SET_TIMERSLACK_PID, kTimerSlackForegroundNs,

        vr_app_render_thread_);

  }



  // We could also reset the thread's cpuset here, but the cpuset is already

  // managed by Android. Better to let Android adjust the cpuset as the app

  // moves to the background, rather than adjust it ourselves here, and risk

  // stomping on the value set by Android.



  vr_app_render_thread_ = new_vr_app_render_thread;

}



}  // namespace dvr

}  // namespace android

services/vr/performanced/performance_service.h

+10 −0
Original line number Diff line number Diff line
@@ -39,6 +39,14 @@ class PerformanceService : public pdx::ServiceBase<PerformanceService> { 
  pdx::Status<std::string> OnGetCpuPartition(pdx::Message& message,

                                             pid_t task_id);



  // Set which thread gets the vr:app:render policy. Only one thread at a time

  // is allowed to have vr:app:render. If multiple threads are allowed

  // vr:app:render, and those threads busy loop, the system can freeze. When

  // SetVrAppRenderThread() is called, the thread which we had previously

  // assigned vr:app:render will have its scheduling policy reset to default

  // values.

  void SetVrAppRenderThread(pid_t new_vr_app_render_thread);



  CpuSetManager cpuset_;



  int sched_fifo_min_priority_;
@@ -70,6 +78,8 @@ class PerformanceService : public pdx::ServiceBase<PerformanceService> { 
  std::function<bool(const pdx::Message& message, const Task& task)>

      partition_permission_check_;



  pid_t vr_app_render_thread_ = -1;



  PerformanceService(const PerformanceService&) = delete;

  void operator=(const PerformanceService&) = delete;

};