CC AmitP and linaro kernel,
Apparently, some APPs are very aggressive using v3.15 kernel's header, since PR_SET_THP_DISABLE is introduced from 3.15. and PR_SET_TIMERSLACK_PID never appears in mainline kernel, only in android.
Amit,
Guess we need to revert this SET_TIMERSLACK_PID if APP keep using DHP_DIS. Any concern/suggestion of this issue?
Thanks!
On 06/23/2015 05:46 PM, Mark Brown wrote:
Hi,
Adding Kevin and Alex - I'm not responsible for the LSK any more, Kevin has taken over.
Thanks, Mark
On 23 June 2015 at 10:32, YiPing Xu <xuyiping@hisilicon.com mailto:xuyiping@hisilicon.com> wrote:
hi In the commit below, LSK3.10 add PR_SET_TIMERSLACK_PID to prctl, but the code does not really work. https://git.linaro.org/kernel/linux-linaro-stable.git/commitdiff/800f7e9e8d8c607b70d384a62db02490a73dfbab And the PR_SET_TIMERSLACK_PID has value 41, which is used as PR_SET_THP_DISABLE in kernel main tree. see https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/include/uapi/linux/prctl.h -- line 184 In lsk-v3.10-15.05, the commit below make PR_SET_TIMERSLACK_PID works. We find a lot of APP use value "41" as PR_SET_THP_DISABLE, and the kernel get a poor performance https://git.linaro.org/kernel/linux-linaro-stable.git/commit/e1a60cbe0312adbf471ee057c5d7b917ae5e80c5 Shall we revert the PR_SET_TIMERSLACK_PID code, or keep it as same with the kernel main tree?
+ John and Sumit for comments,
On 24 June 2015 at 06:22, Alex Shi alex.shi@linaro.org wrote:
CC AmitP and linaro kernel,
Apparently, some APPs are very aggressive using v3.15 kernel's header, since PR_SET_THP_DISABLE is introduced from 3.15. and PR_SET_TIMERSLACK_PID never appears in mainline kernel, only in android.
Amit,
Guess we need to revert this SET_TIMERSLACK_PID if APP keep using DHP_DIS. Any concern/suggestion of this issue?
I'm not sure if reverting SET_TIMERSLACK_PID patch in lsk-3.10-android kernel is a good call here. A quick search show that AOSP master still use SET_TIMERSLACK_PID to set/change the scheduling priority[1] of the background binder and art threads, to name a few.
If we are running into significant performance hit with APPs using PR_SET_THP_DISABLE on Android kernel, with conflicting prctl value of SET_TIMERSLACK_PID, then we should either just submit a change in AOSP to update SET_TIMERSLACK_PID value accordingly and/or also try to backport PR_SET_THP_DISABLE to aosp/android-3.10 kernel if we have a significant usecase for that. Let me know if it make any sense.
Regards, Amit Pundir
[1] https://android.googlesource.com/platform/system/core/+/master/libcutils/sch...
Thanks!
On 06/23/2015 05:46 PM, Mark Brown wrote:
Hi,
Adding Kevin and Alex - I'm not responsible for the LSK any more, Kevin has taken over.
Thanks, Mark
On 23 June 2015 at 10:32, YiPing Xu <xuyiping@hisilicon.com mailto:xuyiping@hisilicon.com> wrote:
hi In the commit below, LSK3.10 add PR_SET_TIMERSLACK_PID to prctl, but the code does not really work. https://git.linaro.org/kernel/linux-linaro-stable.git/commitdiff/800f7e9e8d8c607b70d384a62db02490a73dfbab And the PR_SET_TIMERSLACK_PID has value 41, which is used as PR_SET_THP_DISABLE in kernel main tree. see https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/include/uapi/linux/prctl.h -- line 184 In lsk-v3.10-15.05, the commit below make PR_SET_TIMERSLACK_PID works. We find a lot of APP use value "41" as PR_SET_THP_DISABLE, and the kernel get a poor performance https://git.linaro.org/kernel/linux-linaro-stable.git/commit/e1a60cbe0312adbf471ee057c5d7b917ae5e80c5 Shall we revert the PR_SET_TIMERSLACK_PID code, or keep it as same with the kernel main tree?
On Wed, Jun 24, 2015 at 12:59 AM, Amit Pundir amit.pundir@linaro.org wrote:
- John and Sumit for comments,
On 24 June 2015 at 06:22, Alex Shi alex.shi@linaro.org wrote:
CC AmitP and linaro kernel,
Apparently, some APPs are very aggressive using v3.15 kernel's header, since PR_SET_THP_DISABLE is introduced from 3.15. and PR_SET_TIMERSLACK_PID never appears in mainline kernel, only in android.
Amit,
Guess we need to revert this SET_TIMERSLACK_PID if APP keep using DHP_DIS. Any concern/suggestion of this issue?
I'm not sure if reverting SET_TIMERSLACK_PID patch in lsk-3.10-android kernel is a good call here. A quick search show that AOSP master still use SET_TIMERSLACK_PID to set/change the scheduling priority[1] of the background binder and art threads, to name a few.
Yea. I suspect for the Android lsk kernel we will want to add a Kconfig option, which will enable either SET_TIMERSLACK_PID or PR_SET_THP_DISABLE, so that only one is supported at a time. That way for non-android kernels the TIMERSLACK can be effectively dropped at build time, and for android kernels we can preserve it.
If we are running into significant performance hit with APPs using PR_SET_THP_DISABLE on Android kernel, with conflicting prctl value of SET_TIMERSLACK_PID, then we should either just submit a change in AOSP to update SET_TIMERSLACK_PID value accordingly
Yea. We should look at what they did for the 3.18 kernel since it seems like it would have hit this problem already.
and/or also try to backport PR_SET_THP_DISABLE to aosp/android-3.10 kernel if we have a significant usecase for that. Let me know if it make any sense.
Is there any context about which app is using this? If we want to be super clever, we might be able to check if two or three arguments were passed to us, and use that info to decide if its likely SET_THP_DISABLE usage or SET_TIMERSLACK_PID.
But long term, we need to get TIMERSLACK_PID renumbered and pushed upstream.
thanks -john
On 06/25/2015 12:37 AM, John Stultz wrote:
On Wed, Jun 24, 2015 at 12:59 AM, Amit Pundir amit.pundir@linaro.org wrote:
- John and Sumit for comments,
On 24 June 2015 at 06:22, Alex Shi alex.shi@linaro.org wrote:
CC AmitP and linaro kernel,
Apparently, some APPs are very aggressive using v3.15 kernel's header, since PR_SET_THP_DISABLE is introduced from 3.15. and PR_SET_TIMERSLACK_PID never appears in mainline kernel, only in android.
Amit,
Guess we need to revert this SET_TIMERSLACK_PID if APP keep using DHP_DIS. Any concern/suggestion of this issue?
I'm not sure if reverting SET_TIMERSLACK_PID patch in lsk-3.10-android kernel is a good call here. A quick search show that AOSP master still use SET_TIMERSLACK_PID to set/change the scheduling priority[1] of the background binder and art threads, to name a few.
Yea. I suspect for the Android lsk kernel we will want to add a Kconfig option, which will enable either SET_TIMERSLACK_PID or PR_SET_THP_DISABLE, so that only one is supported at a time. That way for non-android kernels the TIMERSLACK can be effectively dropped at build time, and for android kernels we can preserve it.
It's a reasonable solutions.
If we are running into significant performance hit with APPs using PR_SET_THP_DISABLE on Android kernel, with conflicting prctl value of SET_TIMERSLACK_PID, then we should either just submit a change in AOSP to update SET_TIMERSLACK_PID value accordingly
Yea. We should look at what they did for the 3.18 kernel since it seems like it would have hit this problem already.
In 3.18 android the timerslack is 43. But the key in user APP, how to make sure the user level APP know the 41 is for THP_DIS and 43 is for TIMERSLACK?
and/or also try to backport PR_SET_THP_DISABLE to aosp/android-3.10 kernel if we have a significant usecase for that. Let me know if it make any sense.
Is there any context about which app is using this? If we want to be super clever, we might be able to check if two or three arguments were passed to us, and use that info to decide if its likely SET_THP_DISABLE usage or SET_TIMERSLACK_PID.
But long term, we need to get TIMERSLACK_PID renumbered and pushed upstream.
yes, that is best solution.
Anyway, I will look at if we can backport THP_DIS to 3.10 and change the timeslack to 43.
Peter, With all the possible solution, which one is your prefer?
thanks -john
On Thu, Jun 25, 2015 at 7:37 AM, Alex Shi alex.shi@linaro.org wrote:
On 06/25/2015 12:37 AM, John Stultz wrote:
On Wed, Jun 24, 2015 at 12:59 AM, Amit Pundir amit.pundir@linaro.org wrote:
- John and Sumit for comments,
On 24 June 2015 at 06:22, Alex Shi alex.shi@linaro.org wrote:
CC AmitP and linaro kernel,
Apparently, some APPs are very aggressive using v3.15 kernel's header, since PR_SET_THP_DISABLE is introduced from 3.15. and PR_SET_TIMERSLACK_PID never appears in mainline kernel, only in android.
Amit,
Guess we need to revert this SET_TIMERSLACK_PID if APP keep using DHP_DIS. Any concern/suggestion of this issue?
I'm not sure if reverting SET_TIMERSLACK_PID patch in lsk-3.10-android kernel is a good call here. A quick search show that AOSP master still use SET_TIMERSLACK_PID to set/change the scheduling priority[1] of the background binder and art threads, to name a few.
Yea. I suspect for the Android lsk kernel we will want to add a Kconfig option, which will enable either SET_TIMERSLACK_PID or PR_SET_THP_DISABLE, so that only one is supported at a time. That way for non-android kernels the TIMERSLACK can be effectively dropped at build time, and for android kernels we can preserve it.
It's a reasonable solutions.
If we are running into significant performance hit with APPs using PR_SET_THP_DISABLE on Android kernel, with conflicting prctl value of SET_TIMERSLACK_PID, then we should either just submit a change in AOSP to update SET_TIMERSLACK_PID value accordingly
Yea. We should look at what they did for the 3.18 kernel since it seems like it would have hit this problem already.
In 3.18 android the timerslack is 43. But the key in user APP, how to make sure the user level APP know the 41 is for THP_DIS and 43 is for TIMERSLACK?
and/or also try to backport PR_SET_THP_DISABLE to aosp/android-3.10 kernel if we have a significant usecase for that. Let me know if it make any sense.
Is there any context about which app is using this? If we want to be super clever, we might be able to check if two or three arguments were passed to us, and use that info to decide if its likely SET_THP_DISABLE usage or SET_TIMERSLACK_PID.
But long term, we need to get TIMERSLACK_PID renumbered and pushed upstream.
yes, that is best solution.
Anyway, I will look at if we can backport THP_DIS to 3.10 and change the timeslack to 43.
Just as a heads up, in mainline we have:
#define PR_MPX_ENABLE_MANAGEMENT 43 #define PR_MPX_DISABLE_MANAGEMENT 44 #define PR_SET_FP_MODE 45 #define PR_GET_FP_MODE 46
So it might be good to suggest SET_TIMERSLACK_PID to be set to a large number so we avoid constant collsions while it is out of tree.
thanks -john
On Thu, Jun 25, 2015 at 3:28 PM, John Stultz john.stultz@linaro.org wrote:
On Thu, Jun 25, 2015 at 7:37 AM, Alex Shi alex.shi@linaro.org wrote:
On 06/25/2015 12:37 AM, John Stultz wrote:
On Wed, Jun 24, 2015 at 12:59 AM, Amit Pundir amit.pundir@linaro.org wrote:
- John and Sumit for comments,
On 24 June 2015 at 06:22, Alex Shi alex.shi@linaro.org wrote:
CC AmitP and linaro kernel,
Apparently, some APPs are very aggressive using v3.15 kernel's header, since PR_SET_THP_DISABLE is introduced from 3.15. and PR_SET_TIMERSLACK_PID never appears in mainline kernel, only in android.
Amit,
Guess we need to revert this SET_TIMERSLACK_PID if APP keep using DHP_DIS. Any concern/suggestion of this issue?
I'm not sure if reverting SET_TIMERSLACK_PID patch in lsk-3.10-android kernel is a good call here. A quick search show that AOSP master still use SET_TIMERSLACK_PID to set/change the scheduling priority[1] of the background binder and art threads, to name a few.
Yea. I suspect for the Android lsk kernel we will want to add a Kconfig option, which will enable either SET_TIMERSLACK_PID or PR_SET_THP_DISABLE, so that only one is supported at a time. That way for non-android kernels the TIMERSLACK can be effectively dropped at build time, and for android kernels we can preserve it.
It's a reasonable solutions.
If we are running into significant performance hit with APPs using PR_SET_THP_DISABLE on Android kernel, with conflicting prctl value of SET_TIMERSLACK_PID, then we should either just submit a change in AOSP to update SET_TIMERSLACK_PID value accordingly
Yea. We should look at what they did for the 3.18 kernel since it seems like it would have hit this problem already.
In 3.18 android the timerslack is 43. But the key in user APP, how to make sure the user level APP know the 41 is for THP_DIS and 43 is for TIMERSLACK?
and/or also try to backport PR_SET_THP_DISABLE to aosp/android-3.10 kernel if we have a significant usecase for that. Let me know if it make any sense.
Is there any context about which app is using this? If we want to be super clever, we might be able to check if two or three arguments were passed to us, and use that info to decide if its likely SET_THP_DISABLE usage or SET_TIMERSLACK_PID.
But long term, we need to get TIMERSLACK_PID renumbered and pushed upstream.
yes, that is best solution.
Anyway, I will look at if we can backport THP_DIS to 3.10 and change the timeslack to 43.
Just as a heads up, in mainline we have:
#define PR_MPX_ENABLE_MANAGEMENT 43 #define PR_MPX_DISABLE_MANAGEMENT 44 #define PR_SET_FP_MODE 45 #define PR_GET_FP_MODE 46
So it might be good to suggest SET_TIMERSLACK_PID to be set to a large number so we avoid constant collsions while it is out of tree.
Spurred by this, I've folded the related patches together and submitted for comment to lkml. https://lkml.org/lkml/2015/6/25/691
thanks -john
For current 3.10 LSK, I ma going to submit a commit as following:
Any concern for this patch?
---------------- commit 25af66fbddd13fdcf85a7a9d0d4e790e7ca23ff8 Author: Alex Shi alex.shi@linaro.org Date: Mon Jun 29 15:05:53 2015 +0800
prctl: resolve PR_SET_TIMERSLACK_PID/PR_SET_THP_DISABLE conflict
In android kernel the PR_SET_TIMERSLACK_PID defined as number 41 which conflict with PR_SET_THP_DISABLE in upstream kernel from 3.15.
So redefine it's number to 47 to avoid the possible confliction.
Signed-off-by: Alex Shi alex.shi@linaro.org
diff --git a/include/uapi/linux/prctl.h b/include/uapi/linux/prctl.h index 28bb0b3..8c6edff 100644 --- a/include/uapi/linux/prctl.h +++ b/include/uapi/linux/prctl.h @@ -153,7 +153,7 @@ * arg2 slack value, 0 means "use default" * arg3 pid of the thread whose timer slack needs to be set */ -#define PR_SET_TIMERSLACK_PID 41 +#define PR_SET_TIMERSLACK_PID 47
#define PR_SET_VMA 0x53564d41 # define PR_SET_VMA_ANON_NAME 0
On 06/26/2015 07:52 AM, John Stultz wrote:
Just as a heads up, in mainline we have:
#define PR_MPX_ENABLE_MANAGEMENT 43 #define PR_MPX_DISABLE_MANAGEMENT 44 #define PR_SET_FP_MODE 45 #define PR_GET_FP_MODE 46
So it might be good to suggest SET_TIMERSLACK_PID to be set to a large number so we avoid constant collsions while it is out of tree.
Spurred by this, I've folded the related patches together and submitted for comment to lkml. https://lkml.org/lkml/2015/6/25/691
On 29 June 2015 at 12:41, Alex Shi alex.shi@linaro.org wrote:
For current 3.10 LSK, I ma going to submit a commit as following:
Any concern for this patch?
commit 25af66fbddd13fdcf85a7a9d0d4e790e7ca23ff8 Author: Alex Shi alex.shi@linaro.org Date: Mon Jun 29 15:05:53 2015 +0800
prctl: resolve PR_SET_TIMERSLACK_PID/PR_SET_THP_DISABLE conflict In android kernel the PR_SET_TIMERSLACK_PID defined as number 41 which conflict with PR_SET_THP_DISABLE in upstream kernel from 3.15. So redefine it's number to 47 to avoid the possible confliction. Signed-off-by: Alex Shi <alex.shi@linaro.org>diff --git a/include/uapi/linux/prctl.h b/include/uapi/linux/prctl.h index 28bb0b3..8c6edff 100644 --- a/include/uapi/linux/prctl.h +++ b/include/uapi/linux/prctl.h @@ -153,7 +153,7 @@
- arg2 slack value, 0 means "use default"
- arg3 pid of the thread whose timer slack needs to be set
*/ -#define PR_SET_TIMERSLACK_PID 41 +#define PR_SET_TIMERSLACK_PID 47
Problem is that the Android userspace is redefining PR_SET_TIMERSLACK_PID back to 41 [1]. So this kernel patch alone would not help.
Regards, Amit Pundir
[1] https://android.googlesource.com/platform/system/core/+/master/libcutils/sch... .......... // This prctl is only available in Android kernels. #define PR_SET_TIMERSLACK_PID 41 .....................
#define PR_SET_VMA 0x53564d41 # define PR_SET_VMA_ANON_NAME 0
On 06/26/2015 07:52 AM, John Stultz wrote:
Just as a heads up, in mainline we have:
#define PR_MPX_ENABLE_MANAGEMENT 43 #define PR_MPX_DISABLE_MANAGEMENT 44 #define PR_SET_FP_MODE 45 #define PR_GET_FP_MODE 46
So it might be good to suggest SET_TIMERSLACK_PID to be set to a large number so we avoid constant collsions while it is out of tree.
Spurred by this, I've folded the related patches together and submitted for comment to lkml. https://lkml.org/lkml/2015/6/25/691
On Mon, Jun 29, 2015 at 7:12 AM, Amit Pundir amit.pundir@linaro.org wrote:
On 29 June 2015 at 12:41, Alex Shi alex.shi@linaro.org wrote:
For current 3.10 LSK, I ma going to submit a commit as following:
Any concern for this patch?
commit 25af66fbddd13fdcf85a7a9d0d4e790e7ca23ff8 Author: Alex Shi alex.shi@linaro.org Date: Mon Jun 29 15:05:53 2015 +0800
prctl: resolve PR_SET_TIMERSLACK_PID/PR_SET_THP_DISABLE conflict In android kernel the PR_SET_TIMERSLACK_PID defined as number 41 which conflict with PR_SET_THP_DISABLE in upstream kernel from 3.15. So redefine it's number to 47 to avoid the possible confliction. Signed-off-by: Alex Shi <alex.shi@linaro.org>diff --git a/include/uapi/linux/prctl.h b/include/uapi/linux/prctl.h index 28bb0b3..8c6edff 100644 --- a/include/uapi/linux/prctl.h +++ b/include/uapi/linux/prctl.h @@ -153,7 +153,7 @@
- arg2 slack value, 0 means "use default"
- arg3 pid of the thread whose timer slack needs to be set
*/ -#define PR_SET_TIMERSLACK_PID 41 +#define PR_SET_TIMERSLACK_PID 47
Problem is that the Android userspace is redefining PR_SET_TIMERSLACK_PID back to 41 [1]. So this kernel patch alone would not help.
Yea. I think we need to ping Ruchi at google and try to sync up with their plans. If their userspace is going to switch the PR number based on kernel versions we will also be in trouble. (My usage of 47 was only in the context of submitting it upstream. Once its upstream, that would be the number to migrate to, but not before then).
It might be better to have a patch which sets it to 127 or something which obviously won't collide in the near term, and then userspace can probe 127 first to see if its on a newer kernel, then fall back to 41 if its on an older kernel.
thanks -john
On 06/30/2015 01:25 AM, John Stultz wrote:
diff --git a/include/uapi/linux/prctl.h b/include/uapi/linux/prctl.h index 28bb0b3..8c6edff 100644 --- a/include/uapi/linux/prctl.h +++ b/include/uapi/linux/prctl.h @@ -153,7 +153,7 @@
- arg2 slack value, 0 means "use default"
- arg3 pid of the thread whose timer slack needs to be set
*/ -#define PR_SET_TIMERSLACK_PID 41 +#define PR_SET_TIMERSLACK_PID 47
Problem is that the Android userspace is redefining PR_SET_TIMERSLACK_PID back to 41 [1]. So this kernel patch alone would not help.
Yea. I think we need to ping Ruchi at google and try to sync up with their plans. If their userspace is going to switch the PR number based on kernel versions we will also be in trouble. (My usage of 47 was only in the context of submitting it upstream. Once its upstream, that would be the number to migrate to, but not before then).
Yes, merge this into upstream is the right way, but it's also a long way.
Amit & John, Would you like to ping google to do sth on this issue?
YinPing & Peter,
Before everything settling down, seems there is no good way to change sth in LSK. So you may need to modify your kernel according to your using scenarios.
It might be better to have a patch which sets it to 127 or something which obviously won't collide in the near term, and then userspace can probe 127 first to see if its on a newer kernel, then fall back to 41 if its on an older kernel.
That need user APP to do this change. It looks more trouble to require this to common user.
On 2015/6/30 11:03, Alex Shi wrote:
On 06/30/2015 01:25 AM, John Stultz wrote:
diff --git a/include/uapi/linux/prctl.h b/include/uapi/linux/prctl.h index 28bb0b3..8c6edff 100644 --- a/include/uapi/linux/prctl.h +++ b/include/uapi/linux/prctl.h @@ -153,7 +153,7 @@
- arg2 slack value, 0 means "use default"
- arg3 pid of the thread whose timer slack needs to be set
*/ -#define PR_SET_TIMERSLACK_PID 41 +#define PR_SET_TIMERSLACK_PID 47
Problem is that the Android userspace is redefining PR_SET_TIMERSLACK_PID back to 41 [1]. So this kernel patch alone would not help.
Yea. I think we need to ping Ruchi at google and try to sync up with their plans. If their userspace is going to switch the PR number based on kernel versions we will also be in trouble. (My usage of 47 was only in the context of submitting it upstream. Once its upstream, that would be the number to migrate to, but not before then).
Yes, merge this into upstream is the right way, but it's also a long way.
Amit & John, Would you like to ping google to do sth on this issue?
YinPing & Peter,
Before everything settling down, seems there is no good way to change sth in LSK. So you may need to modify your kernel according to your using scenarios.
ok,
for compatibility, it is better to upstream this code as soon as possible.
It might be better to have a patch which sets it to 127 or something which obviously won't collide in the near term, and then userspace can probe 127 first to see if its on a newer kernel, then fall back to 41 if its on an older kernel.
That need user APP to do this change. It looks more trouble to require this to common user.
.
linaro-kernel@lists.linaro.org
-
Alex Shi -
Amit Pundir -
John Stultz -
YiPing Xu