Messages in this thread | | | Date | Fri, 22 Jun 2007 19:52:47 +0400 | From | Oleg Nesterov <> | Subject | Re: [RFC PATCH 6/6] Convert tasklets to work queues |
| |
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);
Looking closer, I think this is not right, and the smp_mb__before_clear_bit() can't help.
/* t->count == 1 */
work_tasklet_exec() tasklet_enable()
...
set_bit(TASKLET_STATE_PENDING); atomic_dec_and_test(&t->count);
/* t->count == 0 */
// False if (atomic_read(&t->count)) goto out;
// True if (test_and_clear_bit(_PENDING)) tasklet_schedule();
clear_bit(TASKLET_STATE_PENDING);
execute t->func();
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;
Steven, a very stupid suggestion, could you move the code for tasklet_enable() up, closer to tasklet_disable() ?
Oleg.
- 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/
| |