> > Agreed; however, I also don't want to introduce unnecessary
> > bloat, so I need to understand first what cases need it - it
> > is kind of hard for me. Care to let me know some gotchas?
>
> set_current_state() includes a write barrier to ensure the setting of
> the state is flushed before any further instructions. This is to
> provide a memory barrier for weak-ordering processors that
> can and will rearrange the writes.
It is what I was expecting, given xchg() being in the equation;
then it is reduced to a problem of guessing what can be the
delay for the flushing of the write ... beautiful ...
So, in that scenario, it means that:
- any setting before a return should be barriered unless we
return to a place[s] known to be harmless
- any setting to TASK_RUNNING should be kind of safe
- exec.c:de_thread(),
while (atomic_read(&oldsig->count) > count) {
oldsig->group_exit_task = current;
- current->state = TASK_UNINTERRUPTIBLE;
+ __set_current_state(TASK_UNINTERRUPTIBLE);
spin_unlock_irq(&oldsig->siglock);
Should be safe, as spin_unlock_irq() will do memory clobber
on sti() [undependant from UP/SMP].
- namei.c:do_follow_link()
if (current->total_link_count >= 40)
goto loop;
if (need_resched()) {
- current->state = TASK_RUNNING;
+ __set_current_state(TASK_RUNNING);
schedule();
}
err = security_inode_follow_link(dentry, nd);
There is a function for it, cond_resched().
So, sending an updated patch right now
> Not all processors like those made by your employer are
> strongly-ordered :)
You'd be surprised how little about those gory details I do know :]
Inaky Perez-Gonzalez -- Not speaking for Intel - opinions are my own [or my
fault]
On Wed, 2002-12-18 at 20:53, Perez-Gonzalez, Inaky wrote:
> - any setting before a return should be barriered unless we
> return to a place[s] known to be harmless
Not sure.
> - any setting to TASK_RUNNING should be kind of safe
Yes, I agree. It may race, but with what?
> - exec.c:de_thread(),
>
> while (atomic_read(&oldsig->count) > count) {
> oldsig->group_exit_task = current;
> - current->state = TASK_UNINTERRUPTIBLE;
> + __set_current_state(TASK_UNINTERRUPTIBLE);
> spin_unlock_irq(&oldsig->siglock);
>
> Should be safe, as spin_unlock_irq() will do memory clobber
> on sti() [undependant from UP/SMP].
The memory clobber only acts as a compiler barrier and insures the
compiler does not reorder the statements from the order in the C code.
What we need is a memory barrier to ensure the processor does not
reorder statements. In other words, the processor can completely
rearrange loads and stores as they are issued to it, as long as it does
not break obvious data dependencies. On a weakly ordered processor,
sans memory barrier, there is no telling when and where a store will
actually reach memory. This is regardless of the order of the C code or
anything else.
That said, I do not know if the above example is a problem or not. On a
very quick glance, the only issue I saw is the one I pointed out
earlier, and you fixed it.
Robert Love