Oleg Nesterov wrote:
George Anzinger wrote:
Oleg Nesterov wrote:
This patch removes timer_active/set_timer_inactive which plays with
timer_list's internals in favour of using try_to_del_timer_sync(),
which was introduced in the previous patch.
Is there a particular reason for this, like it does not work, for example, or
are you just trying to clean up code?
It's a cleanup, I think that current code is correct.
If this currently works, please leave it alone.
Ok.
We also note that this code is the subject of a patch to the RT patch to cover
the same issue when softirqs are run from threads and therefor allow
posix_timer_fn to be preempted. (That fix being mainly to expand usage from
just SMP to SMP || SOFTIRQ_PREEMPT.)
I guess you are talking about this patch:
http://marc.theaimsgroup.com/?l=linux-kernel&m=111566867218576
Also, I think that del_timer_sync and friends need to be turned on if soft_irq
is preemptable.
I agree completely.
+ * For RT the timer call backs are preemptable. This means that folks
+ * trying to delete timers may run into timers that are "active" for
+ * long times. To help out with this we provide a wake up function to
+ * wake up a caller who wants waking when a timer clears the call back.
+ * This is the same sort of thing that the del_timer_sync does, but we
+ * need (in the HRT case) to cover two lists and not just the one.
+ */
+#ifdef CONFIG_PREEMPT_SOFTIRQS
+#include <linux/wait.h>
+static DECLARE_WAIT_QUEUE_HEAD(timer_wake_queue);
+#define wake_timer_waiters() wake_up(&timer_wake_queue)
+#define wait_for_timer(timer) wait_event(timer_wake_queue, !timer_active(timer))
I'm not an expert at all, so I may be wrong, but I don't think
it's a good idea.
I think it is bad if __run_timers() could be preempted while
->running_timer != NULL. This will interact badly with __mod_timer,
del_timer_sync. I think that __run_timers() should do:
set_running_timer(base, timer);
preempt_disable();
spin_unlock_irq(&base->lock);
timer->function();
set_running_timer(base, NULL);
preempt_enable();
spin_lock_irq(&base->lock);
What do you think?
First, I think we need to get Ingo in the discussion. :)
Second, the RT patch has been running this way with little problems, save a
REALLY intense test we (Monta Vista) have run that, from time to time, shows
this to be a problem in the posix-timer code that is fixed by including
SOFTIRQ_PREEMPT as well as SMP in the timer ifdefs.
One thing I do see there (in the RT patch) is a change to del_timer_sync to wait
for the timer call back to complete rather than to loop...
--
George Anzinger [email protected]
High-res-timers: http://sourceforge.net/projects/high-res-timers/
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
