[PATCH] timers: Avoid finding cpu for migration of running timers in __mod_timer()
Currently we don't support migration of currently running timers. Current code flow in __mod_timer() is: - Find a cpu where we should migrate the timer - Check if timer is currently running - If yes, don't migrate. In this process, the first step is a unnecessary activiy, if the timer is currently running. This patch tries to avoid it in such cases. Signed-off-by: Viresh Kumar --- kernel/timer.c | 26 +- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/kernel/timer.c b/kernel/timer.c index 8c5e7b9..6e5bf98 100644 --- a/kernel/timer.c +++ b/kernel/timer.c @@ -755,23 +755,23 @@ __mod_timer(struct timer_list *timer, unsigned long expires, debug_activate(timer, expires); - cpu = smp_processor_id(); + /* +* Should we try to migrate timer? +* However we can't change timer's base while it is running, otherwise +* del_timer_sync() can't detect that the timer's handler yet has not +* finished. This also guarantees that the timer is serialized wrt +* itself. +*/ + if (likely(base->running_timer != timer)) { + cpu = smp_processor_id(); #if defined(CONFIG_NO_HZ) && defined(CONFIG_SMP) - if (!pinned && get_sysctl_timer_migration() && idle_cpu(cpu)) - cpu = get_nohz_timer_target(); + if (!pinned && get_sysctl_timer_migration() && idle_cpu(cpu)) + cpu = get_nohz_timer_target(); #endif - new_base = per_cpu(tvec_bases, cpu); + new_base = per_cpu(tvec_bases, cpu); - if (base != new_base) { - /* -* We are trying to schedule the timer on the local CPU. -* However we can't change timer's base while it is running, -* otherwise del_timer_sync() can't detect that the timer's -* handler yet has not finished. This also guarantees that -* the timer is serialized wrt itself. -*/ - if (likely(base->running_timer != timer)) { + if (base != new_base) { /* See the comment in lock_timer_base() */ timer_set_base(timer, NULL); spin_unlock(&base->lock); -- 1.7.12.rc2.18.g61b472e ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [Powertop] [PATCH v2 2/2] Add stubs to support Android platform
On 09/26/2012 04:00 PM, Sergey Senozhatsky wrote: On (09/27/12 00:09), Magnus Fromreide wrote: That they fail to throw exceptions from new is no reason to disable set_new_handler, the newhandler is called by the runtime on out of memory and is intended to allow the user to try fixing the issue. This is true for the noexcept version as well. Is this yet another incompatibility? right. the funny thing is, guess what, gcc actually has "#ifdef __EXCEPTIONS" within operator new() 44 _GLIBCXX_WEAK_DEFINITION void * 45 operator new (std::size_t sz) _GLIBCXX_THROW (std::bad_alloc) 46 { 47 void *p; 48 49 /* malloc (0) is unpredictable; avoid it. */ 50 if (sz == 0) 51 sz = 1; 52 p = (void *) malloc (sz); 53 while (p == 0) 54 { 55 new_handler handler = __new_handler; 56 if (! handler) 57 #ifdef __EXCEPTIONS 58 throw bad_alloc(); 59 #else 60 std::abort(); 61 #endif 62 handler (); 63 p = (void *) malloc (sz); 64 } 65 66 return p; 67 } It tourned out, that __EXCEPTIONS with us (for operator new and STL) since 2001 "2001-02-15 Benjamin Kosnik " So, I guess this is how Google has came up with the idea of C++ w/o exceptions. Wow. -ss Well to be fair, not that I agree any-more, but years ago it was common practice to disable exceptions (via the compiler) for C++ in very specialized "REAL" "Embedded Systems". This practice was problematic in that you needed to have a will defined Error handling design, but saved space. Honestly, its been a long time since I worked on anything that has a "REAL" size requirement, so I don't see the point. On the other hand in some Embedded system The whole reason to using your own error handling design was that in a real embedded system, design considerations like, uptime, failure control and ability to eliminate undefined behaviour were KEY. So taking the time to design was already a given, and size was not a single point factor. (not arguing the C vs C++ point here BTW) So for what ever reason the decision to use C++ happens for an embedded project. Problem was C++ exception were good for insure the program didn't fail, but detailed failure handling became problematic. Sure you could avoid general failures but often especially in an embedded system you found yourself with undefined behaviour that was nearly imposable to handle. OK in general history over :) Unfortunately this practice has been inherited still today in segments of other then embedded. Most commonly you may see this practice in specialized segments like gaming consoles. Such practices are still in valid use in such device as switches, telecommunications, avionic systems, weapon systems, medical devices, ect. So I have an understanding of the issues, "some" of its history, and valid uses. But in my opinion this is still a bad implementation for anyone distributing a general operating system. +/* define stubs for C++ exception handling */ +#define tryif (true) +#define catch(x) if (false) If you should do this then I think it should be spelled #define try if (true) #define catch else if (false) in order to not break if (condition) try { } catch(type variable) { } but it still breaks the syntax for it which can be shown by simply adding an else clause to the if statement. Oh, and furthermore I consider that Android needs a C++ compiler. + +/* Define __NR_perf_event_open if not already defined */ +#if __arm__ +#ifndef __NR_perf_event_open +#define __NR_perf_event_open364 +#endif +#endif + +/* + * bionic libc mbstowcs version returns zero when max parameter + * is zero, resulting infinite loops in powertop source. Add + * mbstowcs wrapper to fix it. + */ +namespace pandroid { + extern "C" inline size_t mbstowcs(wchar_t *dst, + const char *src, size_t len) + { + return ::mbstowcs(dst, src, ::strlen(src)); + } +} + +#define mbstowcs(dst, src, len)pandroid::mbstowcs(dst, src, len) Still broken. If dst isn't a NULL pointer then len is the limit on the length of the destination buffer. In throwing away this you open up for stack smashing attacks. ___ PowerTop mailing list power...@lists.01.org https://lists.01.org/mailman/listinfo/powertop ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [Powertop] [PATCH v2 2/2] Add stubs to support Android platform
On 09/28/2012 10:10 AM, Sergey Senozhatsky wrote: On (09/28/12 09:54), Chris Ferron wrote: [..] Well to be fair, not that I agree any-more, but years ago it was common practice to disable exceptions (via the compiler) for C++ in very specialized "REAL" "Embedded Systems". This practice was problematic in that you needed to have a will defined Error handling design, but saved space. Honestly, its been a long time since I worked on anything that has a "REAL" size requirement, so I don't see the point. On the other hand in some Embedded system The whole reason to using your own error handling design was that in a real embedded system, design considerations like, uptime, failure control and ability to eliminate undefined behaviour were KEY. So taking the time to design was already a given, and size was not a single point factor. (not arguing the C vs C++ point here BTW) So for what ever reason the decision to use C++ happens for an embedded project. Problem was C++ exception were good for insure the program didn't fail, but detailed failure handling became problematic. Sure you could avoid general failures but often especially in an embedded system you found yourself with undefined behaviour that was nearly imposable to handle. OK in general history over :) Unfortunately this practice has been inherited still today in segments of other then embedded. Most commonly you may see this practice in specialized segments like gaming consoles. Such practices are still in valid use in such device as switches, telecommunications, avionic systems, weapon systems, medical devices, ect. So I have an understanding of the issues, "some" of its history, and valid uses. But in my opinion this is still a bad implementation for anyone distributing a general operating system. Sure, I totally agree. Nowadays, with 4 CPU cores in a pocket, I simply don't buy "exceptions are slow" argument. If in some particular project exceptions are so common that they're able to sensibly slow down application, then the project most probably is doing something terribly wrong and mis-concept exceptions. Agree The tragedy is that at this point in time I believe Google will never consider using exceptions due to legacy reasons. -ss ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
[RFC] timer: Migrate running timer
Hi Thomas, I haven't tested it much till now. I am sending this patch just to check if the initial idea looks fine to you guys or not. Till now, we weren't migrating a running timer because with migration del_timer_sync() can't detect that the timer's handler yet has not finished. Now, when can we actually to reach to the code (inside __mod_timer()) where base->running_timer == timer ? i.e. We are trying to migrate current timer I can see only following case: - Timer re-armed itself. i.e. Currently we are running interrupt handler of a timer and it rearmed itself from there. At this time user might have called del_timer_sync() or not. If not, then there is no harm in re-arming the timer? Now, when somebody tries to delete a timer, obviously he doesn't want to run it any more for now. So, why should we ever re-arm a timer, which is scheduled for deletion? This patch tries to fix "migration of running timer", assuming above theory is correct :) So, now when we get a call to del_timer_sync(), we will mark it scheduled for deletion in an additional variable. This would be checked whenever we try to modify/arm it again. If it was currently scheduled for deletion, we must not modify/arm it. And so, whenever we reach to the situation where: base->running_timer == timer We are sure, nobody is waiting in del_timer_sync(). We will clear this flag as soon as the timer is deleted, so that it can be started again after deleting it successfully. Waiting for initial comments on it. Signed-off-by: Viresh Kumar --- include/linux/timer.h | 1 + kernel/timer.c| 58 +-- 2 files changed, 34 insertions(+), 25 deletions(-) diff --git a/include/linux/timer.h b/include/linux/timer.h index 8c5a197..ea36ce9 100644 --- a/include/linux/timer.h +++ b/include/linux/timer.h @@ -22,6 +22,7 @@ struct timer_list { unsigned long data; int slack; + int sched_del; #ifdef CONFIG_TIMER_STATS int start_pid; diff --git a/kernel/timer.c b/kernel/timer.c index 1cf8a91..536e7a3 100644 --- a/kernel/timer.c +++ b/kernel/timer.c @@ -620,6 +620,7 @@ static void do_init_timer(struct timer_list *timer, unsigned int flags, timer->entry.next = NULL; timer->base = (void *)((unsigned long)base | flags); timer->slack = -1; + timer->sched_del = 0; #ifdef CONFIG_TIMER_STATS timer->start_site = NULL; timer->start_pid = -1; @@ -727,38 +728,35 @@ __mod_timer(struct timer_list *timer, unsigned long expires, base = lock_timer_base(timer, &flags); + if (timer->sched_del) { + /* Don't schedule it again, as it is getting deleted */ + ret = -EBUSY; + goto out_unlock; + } + ret = detach_if_pending(timer, base, false); if (!ret && pending_only) goto out_unlock; debug_activate(timer, expires); - /* -* Should we try to migrate timer? -* However we can't change timer's base while it is running, otherwise -* del_timer_sync() can't detect that the timer's handler yet has not -* finished. This also guarantees that the timer is serialized wrt -* itself. -*/ - if (likely(base->running_timer != timer)) { #if defined(CONFIG_NO_HZ) && defined(CONFIG_SMP) - if (!pinned && get_sysctl_timer_migration()) - cpu = get_nohz_timer_target(); - else - cpu = smp_processor_id(); -#else + if (!pinned && get_sysctl_timer_migration()) + cpu = get_nohz_timer_target(); + else cpu = smp_processor_id(); +#else + cpu = smp_processor_id(); #endif - new_base = per_cpu(tvec_bases, cpu); - - if (base != new_base) { - /* See the comment in lock_timer_base() */ - timer_set_base(timer, NULL); - spin_unlock(&base->lock); - base = new_base; - spin_lock(&base->lock); - timer_set_base(timer, base); - } + new_base = per_cpu(tvec_bases, cpu); + + if (base != new_base) { + /* See the comment in lock_timer_base() */ + timer_set_base(timer, NULL); + spin_unlock(&base->lock); + base = new_base; + spin_lock(&base->lock); + timer_set_base(timer, base); } timer->expires = expires; @@ -1037,9 +1035,11 @@ EXPORT_SYMBOL(try_to_del_timer_sync); */ int del_timer_sync(struct timer_list *timer) { -#ifdef CONFIG_LOCKDEP + struct tvec_base *base; unsigned long flags; +#ifdef CONFIG_LOCKDEP + /* * If lockdep gives a backtrace here, please reference * the synchronization rules above. @@ -1049,6 +1049,12 @@ int del_timer_sync(struct timer_list *timer) lock
Re: [Powertop] [PATCH v2 2/2] Add stubs to support Android platform
On (09/28/12 09:54), Chris Ferron wrote: [..] > Well to be fair, not that I agree any-more, but years ago it was > common practice to disable exceptions (via the compiler) for C++ in > very specialized "REAL" "Embedded Systems". This practice was > problematic in that you needed to have a will defined Error handling > design, but saved space. Honestly, its been a long time since I > worked on anything that has a "REAL" size requirement, so I don't see > the point. On the other hand in some Embedded system The whole > reason to using your own error handling design was that in a real > embedded system, design considerations like, uptime, failure control > and ability to eliminate undefined behaviour were KEY. So taking the > time to design was already a given, and size was not a single point > factor. (not arguing the C vs C++ point here BTW) So for what ever > reason the decision to use C++ happens for an embedded project. > Problem was C++ exception were good for insure the program didn't > fail, but detailed failure handling became problematic. Sure you > could avoid general failures but often especially in an embedded > system you found yourself with undefined behaviour that was nearly > imposable to handle. OK in general history over :) > > Unfortunately this practice has been inherited still today in > segments of other then embedded. Most commonly you may see this > practice in specialized segments like gaming consoles. Such practices > are still in valid use in such device as switches, > telecommunications, avionic systems, weapon systems, medical devices, > ect. > > So I have an understanding of the issues, "some" of its history, and > valid uses. But in my opinion this is still a bad implementation for > anyone distributing a general operating system. > > Sure, I totally agree. Nowadays, with 4 CPU cores in a pocket, I simply don't buy "exceptions are slow" argument. If in some particular project exceptions are so common that they're able to sensibly slow down application, then the project most probably is doing something terribly wrong and mis-concept exceptions. The tragedy is that at this point in time I believe Google will never consider using exceptions due to legacy reasons. -ss ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev