Re: [PATCH] os: Turn off the scheduler alarm after an arbitrary length of time
On Wed, 2010-08-04 at 18:18 +0300, Vignatti Tiago (Nokia-MS/Helsinki) wrote: On Mon, Aug 02, 2010 at 07:34:57PM +0200, ext Adam Jackson wrote: On Mon, 2010-08-02 at 17:40 +0300, Tiago Vignatti wrote: just wondering what made you think 10 is a good number? I'd be lying if I said I thought very hard about it. The way the defaults work out, the maximum timeslice is 200ms, which is 10 times as long as the scheduler tick. Another 10x is 2 seconds, which is: - well into wtf why is it stuck territory - a bit before anyone would have thought to attach strace - long enough that no amount of scheduler punishment will improve the user experience But it's not something that's interesting to be precise about. I understand your intention to shut up SIGALRM in strace when a client is doing a busy rendering. But I'm afraid adding this feature will impact in the overall scheduling - not that we're doing great on this area, but would require more study to see if this prioritization of a busy client doesn't negatively affects other clients... it's just hard to validate your changes. By the time we turn the alarm off (in this patch), we've already accumulated several points of penalty for the busy client. It'll be punished, just not _infinitely_ punished like before. BTW, I didn't tried your patch, but are you sure this works as expected? If I understood correctly, this only works for single large requests and for other cases SmartScheduleStartTimer is going to be called always after another request wakes the server, re-starting the SIGALRM madness. So if there's always chunks of large requests you're going to see SIGALRM handler anyway. Sure. I'm not trying to solve that. I'm trying to solve a single stuck request being misinterpreted as SIGALRM party instead of DRM command stuck forever. - ajax signature.asc Description: This is a digitally signed message part ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] os: Turn off the scheduler alarm after an arbitrary length of time
On Mon, Aug 02, 2010 at 07:34:57PM +0200, ext Adam Jackson wrote: On Mon, 2010-08-02 at 17:40 +0300, Tiago Vignatti wrote: just wondering what made you think 10 is a good number? I'd be lying if I said I thought very hard about it. The way the defaults work out, the maximum timeslice is 200ms, which is 10 times as long as the scheduler tick. Another 10x is 2 seconds, which is: - well into wtf why is it stuck territory - a bit before anyone would have thought to attach strace - long enough that no amount of scheduler punishment will improve the user experience But it's not something that's interesting to be precise about. I understand your intention to shut up SIGALRM in strace when a client is doing a busy rendering. But I'm afraid adding this feature will impact in the overall scheduling - not that we're doing great on this area, but would require more study to see if this prioritization of a busy client doesn't negatively affects other clients... it's just hard to validate your changes. BTW, I didn't tried your patch, but are you sure this works as expected? If I understood correctly, this only works for single large requests and for other cases SmartScheduleStartTimer is going to be called always after another request wakes the server, re-starting the SIGALRM madness. So if there's always chunks of large requests you're going to see SIGALRM handler anyway. Cheers, Tiago ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] os: Turn off the scheduler alarm after an arbitrary length of time
On Thu, Jul 29, 2010 at 11:41:10PM +0200, ext Adam Jackson wrote: If we're stuck in some difficult bit of processing, there's no point in punishing the client too strongly. Worse, the continuous stream of SIGALRM shows up in strace and thus people report infinite stream of SIGALRM rather than whatever the bug actually is. So if we're more than 10x past the maximum timeslice, just turn the signal off. just wondering what made you think 10 is a good number? Tiago ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] os: Turn off the scheduler alarm after an arbitrary length of time
On Mon, 2010-08-02 at 17:40 +0300, Tiago Vignatti wrote: On Thu, Jul 29, 2010 at 11:41:10PM +0200, ext Adam Jackson wrote: If we're stuck in some difficult bit of processing, there's no point in punishing the client too strongly. Worse, the continuous stream of SIGALRM shows up in strace and thus people report infinite stream of SIGALRM rather than whatever the bug actually is. So if we're more than 10x past the maximum timeslice, just turn the signal off. just wondering what made you think 10 is a good number? I'd be lying if I said I thought very hard about it. The way the defaults work out, the maximum timeslice is 200ms, which is 10 times as long as the scheduler tick. Another 10x is 2 seconds, which is: - well into wtf why is it stuck territory - a bit before anyone would have thought to attach strace - long enough that no amount of scheduler punishment will improve the user experience But it's not something that's interesting to be precise about. - ajax signature.asc Description: This is a digitally signed message part ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] os: Turn off the scheduler alarm after an arbitrary length of time
On Thu, Jul 29, 2010 at 4:41 PM, Adam Jackson a...@redhat.com wrote: ... the continuous stream of SIGALRM shows up in strace and thus people report infinite stream of SIGALRM rather than whatever the bug actually is. I really hate those SIGALRMs. Reviewed-by: Patrick E. Kane pekan...@gmail.com Pat --- On Thu, Jul 29, 2010 at 4:41 PM, Adam Jackson a...@redhat.com wrote: If we're stuck in some difficult bit of processing, there's no point in punishing the client too strongly. Worse, the continuous stream of SIGALRM shows up in strace and thus people report infinite stream of SIGALRM rather than whatever the bug actually is. So if we're more than 10x past the maximum timeslice, just turn the signal off. Signed-off-by: Adam Jackson a...@redhat.com --- os/utils.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/os/utils.c b/os/utils.c index 51455cc..a3be61d 100644 --- a/os/utils.c +++ b/os/utils.c @@ -1163,6 +1163,9 @@ static void SmartScheduleTimer (int sig) { SmartScheduleTime += SmartScheduleInterval; + + if (SmartScheduleTime = 10 * SmartScheduleMaxSlice) + SmartScheduleStopTimer(); } #endif -- 1.7.2 ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
[PATCH] os: Turn off the scheduler alarm after an arbitrary length of time
If we're stuck in some difficult bit of processing, there's no point in punishing the client too strongly. Worse, the continuous stream of SIGALRM shows up in strace and thus people report infinite stream of SIGALRM rather than whatever the bug actually is. So if we're more than 10x past the maximum timeslice, just turn the signal off. Signed-off-by: Adam Jackson a...@redhat.com --- os/utils.c |3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/os/utils.c b/os/utils.c index 51455cc..a3be61d 100644 --- a/os/utils.c +++ b/os/utils.c @@ -1163,6 +1163,9 @@ static void SmartScheduleTimer (int sig) { SmartScheduleTime += SmartScheduleInterval; + +if (SmartScheduleTime = 10 * SmartScheduleMaxSlice) + SmartScheduleStopTimer(); } #endif -- 1.7.2 ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel