Messages in this thread | | | Date | Tue, 25 Mar 2008 08:27:35 -0400 (EDT) | From | "Robert P. J. Day" <> | Subject | Re: [PATCH] RWSEM: Rewrite rwsem.c and rwsem-spinlock.c more simply. |
| |
On Tue, 25 Mar 2008, Andi Kleen wrote:
> "Robert P. J. Day" <rpjday@crashcourse.ca> writes: > > > Rewrite these source files more simply by deleting the superfluous > > "tsk" task_struct pointer and rephrasing in terms of the "current" > > task pointer. > > This is likely a code pessimization because "current" is inline > assembler and many gcc versions cannot CSE it.
i'm not sure what this means -- which of the transformations in that patch is considered unsafe? here's a typical simplification:
- tsk = current; - set_task_state(tsk, TASK_UNINTERRUPTIBLE); + set_current_state(TASK_UNINTERRUPTIBLE);
there's all sorts of usage of set_current_state() throughout the tree. how is simplifying the code in these two files in exactly the same way any different? or am i missing something because this involves semaphores?
rday
p.s. given this bit from sched.h:
... #define __set_task_state(tsk, state_value) \ do { (tsk)->state = (state_value); } while (0) #define set_task_state(tsk, state_value) \ set_mb((tsk)->state, (state_value))
...
#define __set_current_state(state_value) \ do { current->state = (state_value); } while (0) #define set_current_state(state_value) \ set_mb(current->state, (state_value)) ...
it's not clear why set_current_state() and __current_state() are defined the way they are when it would seem to be simpler (and less error-prone) to just write:
#define __set_current_state(sv) __set_task_state(current, sv) #define set_current_state(sv) set_task_state(current, sv)
the law of parsimony and all that. or, once again, is there something subtle i'm not seeing?
--
======================================================================== Robert P. J. Day Linux Consulting, Training and Annoying Kernel Pedantry: Have classroom, will lecture.
http://crashcourse.ca Waterloo, Ontario, CANADA ========================================================================
| |