On Fri, 2007-06-22 at 19:52 +0400, Oleg Nesterov wrote: > On 06/22, Steven Rostedt wrote: > > > > > truct tasklet_struct, work); > > > > + > > > > + if (unlikely(atomic_read(&t->count))) { > > > > + pr_debug("tasklet disabled %s %p\n", t->n, t); > > > > + set_bit(TASKLET_STATE_PENDING, &t->state); > > > > + smp_mb(); > > > > + /* make sure we were not just enabled */ > > > > + if (likely(atomic_read(&t->count))) > > > > + goto out; > > > > + clear_bit(TASKLET_STATE_PENDING, &t->state);
Yeah, I knew of the race but didn't think that running a tasklet function twice would cause much harm here. But not running it when it needs to run, can have quite a negative impact. > > So, t->func() will be executed twice because tasklet_enable() does > tasklet_schedule(). > > > So I think we need a fix for work_tasklet_exec, > > - clear_bit(TASKLET_STATE_PENDING); > + if (!test_and_clear_bit(TASKLET_STATE_PENDING)) > goto out; > OK, I like this. I'll add it in the next round. > > > Steven, a very stupid suggestion, could you move the code for tasklet_enable() > up, closer to tasklet_disable() ? Not a stupid suggestion. I'll accommodate it. Thanks, -- Steve - 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/