lkml.org 
[lkml]   [2008]   [Mar]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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
========================================================================

\
 
 \ /
  Last update: 2008-03-25 13:31    [from the cache]
©2003-2011 Jasper Spaans. Advertise on this site