[lkml]   [2008]   [Mar]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
SubjectRe: [PATCH] RWSEM: Rewrite rwsem.c and rwsem-spinlock.c more simply.
On Tue, 25 Mar 2008, Andi Kleen wrote:

> "Robert P. J. Day" <> 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?


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. Waterloo, Ontario, CANADA

 \ /
  Last update: 2008-03-25 13:31    [W:0.329 / U:0.420 seconds]
©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site