Re: set_current_mode()

[Juan Quintela <quintela@mandrakesoft.com>]

>>>>> "dan" == Dan Aloni <karrde@callisto.yi.org> writes:

dan> Question: Why some kernel code uses "current->mode = ", and another use
dan> set_current_mode() ? I understand set_current_mode() has some SMP
dan> implications... I see ugly mixed usage like in:

dan> inode.c:164

dan> add_wait_queue(&inode->i_wait, &wait);
dan> repeat:
dan> set_current_state(TASK_UNINTERRUPTIBLE);
dan> if (inode->i_state & I_LOCK) {
dan> schedule();
dan> goto repeat;
dan> }
dan> remove_wait_queue(&inode->i_wait, &wait);
current-> state = TASK_RUNNING;


dan> Shouldn't it be something like:

dan> set_current_state(TASK_RUNNING);
dan> remove_wait_queue(&inode->i_wait, &wait);

I don't remind the details, but the problem is efficiency.
in UP builds, set_current_state(xxx) == current->state = xxx

But in SMP set_current_state() has memory barriers all around.  That
makes it _very_ slow (you need a memory barrier, which means make all
the CPUs wait for you).  That is the reason when you use
set_current_state only when needed.   I think that you only need the
barrier when you change from running to any other state.  For the
other changes you don't need the memory barrier.

At some point, with the help of Phillip Rumpf and somebody else (sorry
I don't remind the names), I did an unfinished patch, that a complile
time put the right code for set_current_state (i.e. they only put the
memory barriers when needed and you could always use the
set_current_state() macro.  /me thinks that you only need to do the
memory barrier when you change to RUNNING, but I really have forget
about that.  I begin the patch not because I understand the
set_current_state() semantics, it was because I think that the actual
mode is really bad :(

Later, Juan.


-- 
In theory, practice and theory are the same, but in practice they 
are different -- Larry McVoy
-
Kernelnewbies: Help each other learn about the Linux kernel.
Archive:       http://mail.nl.linux.org/
IRC Channel:   irc.openprojects.net / #kernelnewbies
Web Page:      http://www.kernelnewbies.org/