Messages in this thread | | | Subject | Re: [RFC PATCH 6/6] Convert tasklets to work queues | From | Steven Rostedt <> | Date | Fri, 22 Jun 2007 12:35:25 -0400 |
| |
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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |