New issue
Advanced search Search tips

Issue 816389 link

Starred by 3 users

Issue metadata

Status: Available
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , iOS
Pri: 3
Type: Feature



Sign in to add a comment

Support process/thread priorities on POSIX

Project Member Reported by gab@chromium.org, Feb 26 2018

Issue description

Turns out we may have had it wrong that backgrounding processes on POSIX (in particular Android) was not possible!

We don't background processes today on POSIX because bringing priority back up requires root, SYS_CAP_NICE privilege to be precise (or so we thought!).

It was pointed to us by the Android libc folks that this is incorrect. SYS_CAP_NICE is required to raise to any priority but there's a soft ceiling defined by the OS which is allowed without root (i.e. we could most likely lower a renderer process' priority and then bringing back up to normal when it's foregrounded)!

Quoting them:
"""
Linux actually supports raising priority with non-root, but the ceiling priority is limited by rlimit(RLIMIT_NICE), https://linux.die.net/man/2/getrlimit. 
The RLIMIT_NICE value can be checked by `ulimit -e`. If the value is 0, it doesn't allow raising priority for processes without SYS_CAP_NICE.
If the value is 40, then you can set priority in the range of [-20, 19] at any process. On Android, this value is 40.
""
 

Comment 1 by gab@chromium.org, Apr 12 2018

Labels: Hotlist-GoodFirstBug

Comment 2 by gab@chromium.org, May 3 2018

Labels: -Type-Bug Type-Feature
Project Member

Comment 3 by bugdroid1@chromium.org, Jul 23

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/830c166036b9b73995d99906f2ba925ba25d6d6f

commit 830c166036b9b73995d99906f2ba925ba25d6d6f
Author: Nicolas Ouellet-payeur <nicolaso@chromium.org>
Date: Mon Jul 23 15:43:45 2018

Update SetProcessBackgrounded() test on Linux

The test was missing EXPECT_*() statements on other platforms than Mac/Windows.

Bug: 816389
Change-Id: Ib50b315445e810025f403faaad7bd1f288369b7f
Reviewed-on: https://chromium-review.googlesource.com/1145623
Reviewed-by: Gabriel Charette <gab@chromium.org>
Commit-Queue: Nicolas Ouellet-Payeur <nicolaso@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577175}
[modify] https://crrev.com/830c166036b9b73995d99906f2ba925ba25d6d6f/base/process/process_unittest.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Jul 24

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/79d4727bbc0a6a0809ec109f773b69ed6fa0b58f

commit 79d4727bbc0a6a0809ec109f773b69ed6fa0b58f
Author: Nicolas Ouellet-payeur <nicolaso@chromium.org>
Date: Tue Jul 24 17:05:21 2018

Replace 20 with NZERO in process_linux.cc

We were using a magic number for what is technically a platform-specific constant, which hurts readability & portability.

Bug: 816389
Change-Id: I9be8d32d08f2e1f224e04a594269b1e1d73fbade
Reviewed-on: https://chromium-review.googlesource.com/1146996
Reviewed-by: Gabriel Charette <gab@chromium.org>
Commit-Queue: Nicolas Ouellet-Payeur <nicolaso@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577589}
[modify] https://crrev.com/79d4727bbc0a6a0809ec109f773b69ed6fa0b58f/base/process/process_linux.cc

Project Member

Comment 5 by bugdroid1@chromium.org, Sep 7

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/eb1903ca0fdfa6c3072b3d28a6789c8adf041432

commit eb1903ca0fdfa6c3072b3d28a6789c8adf041432
Author: Francois Doray <fdoray@chromium.org>
Date: Fri Sep 07 04:40:33 2018

[Base] Change CanIncreaseCurrentThreadPriority() to CanIncreaseThreadPriority(ThreadPriority).

The function is renamed from CanIncreaseCurrentThreadPriority() to
CanIncreaseThreadPriority() so it's clear that it doesn't return a
per-thread value.

Also, a ThreadPriority argument is added to allow implementations to
return a different value depending on the target priority. On Linux
for example, a priority increase should succeed if the target priority
is within the range allowed by RLIMIT_NICE.

This is a no-op CL from an execution point of view.

Bug: 816389
Change-Id: I98cb640527cc2cccb85db45395210e41d6bc5f64
Reviewed-on: https://chromium-review.googlesource.com/1187506
Commit-Queue: François Doray <fdoray@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#589432}
[modify] https://crrev.com/eb1903ca0fdfa6c3072b3d28a6789c8adf041432/base/task/task_scheduler/environment_config.cc
[modify] https://crrev.com/eb1903ca0fdfa6c3072b3d28a6789c8adf041432/base/threading/platform_thread.h
[modify] https://crrev.com/eb1903ca0fdfa6c3072b3d28a6789c8adf041432/base/threading/platform_thread_fuchsia.cc
[modify] https://crrev.com/eb1903ca0fdfa6c3072b3d28a6789c8adf041432/base/threading/platform_thread_mac.mm
[modify] https://crrev.com/eb1903ca0fdfa6c3072b3d28a6789c8adf041432/base/threading/platform_thread_posix.cc
[modify] https://crrev.com/eb1903ca0fdfa6c3072b3d28a6789c8adf041432/base/threading/platform_thread_unittest.cc
[modify] https://crrev.com/eb1903ca0fdfa6c3072b3d28a6789c8adf041432/base/threading/platform_thread_win.cc

Project Member

Comment 6 by bugdroid1@chromium.org, Sep 13

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/48171e2ace28e9440af605cd279a3f646c5762e2

commit 48171e2ace28e9440af605cd279a3f646c5762e2
Author: Francois Doray <fdoray@chromium.org>
Date: Thu Sep 13 16:19:40 2018

[Base] Check RLIMIT_NICE in CanIncreaseThreadPriority().

Previously, CanIncreaseThreadPriority() always returned false
on POSIX (except Mac) for non-root user. This was wrong because any
user can increase the priority of a thread within the range allowed
by RLIMIT_NICE. The distinction is important on Android, where the
default range allows any user to set any thread priority (vs. Linux
where the default range doesn't allow any priority increase).

With this CL, PlatformThread::CanIncreaseThreadPriority()
checks the value of RLIMIT_NICE and returns true if the target
nice value is within the allowed range.

Bug: 816389
Change-Id: I82b30cd9c8ac895f4a0f88fd72b61fdaa17b3073
Reviewed-on: https://chromium-review.googlesource.com/1181940
Commit-Queue: François Doray <fdoray@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#591034}
[modify] https://crrev.com/48171e2ace28e9440af605cd279a3f646c5762e2/base/BUILD.gn
[add] https://crrev.com/48171e2ace28e9440af605cd279a3f646c5762e2/base/posix/can_lower_nice_to.cc
[add] https://crrev.com/48171e2ace28e9440af605cd279a3f646c5762e2/base/posix/can_lower_nice_to.h
[modify] https://crrev.com/48171e2ace28e9440af605cd279a3f646c5762e2/base/process/process_linux.cc
[modify] https://crrev.com/48171e2ace28e9440af605cd279a3f646c5762e2/base/threading/platform_thread_posix.cc
[modify] https://crrev.com/48171e2ace28e9440af605cd279a3f646c5762e2/base/threading/platform_thread_unittest.cc

Project Member

Comment 7 by bugdroid1@chromium.org, Oct 19

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9c69fce42af5c1ac47e1b155051b9e945147ae4d

commit 9c69fce42af5c1ac47e1b155051b9e945147ae4d
Author: Gabriel Charette <gab@chromium.org>
Date: Fri Oct 19 13:47:06 2018

[PlatformThread] Document and harden each platform's CanIncreaseThreadPriority()

And fix CanIncreaseThreadPriority(REALTIME_AUDIO) to use the
per-platform path.

Bug: 816389
Change-Id: I15409a7e00f8ed0f132d348bb42c93e574a83f73
Reviewed-on: https://chromium-review.googlesource.com/c/1283021
Reviewed-by: François Doray <fdoray@chromium.org>
Commit-Queue: Gabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#601131}
[modify] https://crrev.com/9c69fce42af5c1ac47e1b155051b9e945147ae4d/base/threading/platform_thread_android.cc
[modify] https://crrev.com/9c69fce42af5c1ac47e1b155051b9e945147ae4d/base/threading/platform_thread_internal_posix.h
[modify] https://crrev.com/9c69fce42af5c1ac47e1b155051b9e945147ae4d/base/threading/platform_thread_linux.cc
[modify] https://crrev.com/9c69fce42af5c1ac47e1b155051b9e945147ae4d/base/threading/platform_thread_posix.cc
[modify] https://crrev.com/9c69fce42af5c1ac47e1b155051b9e945147ae4d/base/threading/platform_thread_unittest.cc

Sign in to add a comment