linux-kernel - Re: [PATCH] RWSEM: Rewrite rwsem.c and rwsem-spinlock.c more simply.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Tue, 25 Mar 2008 08:42:48 -0400 (EDT)
From:	"Robert P. J. Day" <rpjday@...shcourse.ca>
To:	Andi Kleen <andi@...stfloor.org>
cc:	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Andrew Morton <akpm@...l.org>
Subject: Re: [PATCH] RWSEM: Rewrite rwsem.c and rwsem-spinlock.c more
 simply.

On Tue, 25 Mar 2008, Andi Kleen wrote:

> > i'm not sure what this means -- which of the transformations in
> > that patch is considered unsafe?  here's a typical simplification:
>
> It is not unsafe, just generates slight worse code.
>
> current is inline assembler and the compiler doesn't know that it
> could cache it in a register because it is not marked pure for
> various reasons. That is why current is often cached explicitely in
> a local variable to tell the compiler that.

  ah, i think i see, thanks.  learn something every day.

> Before you run off and do that everywhere: it is also not a large
> win, just a small one unless current is used very often.

  there's actually not that many explicit calls to either
set_task_state or __set_task_state in the entire tree, and a lot of
those don't count as they really are setting the state for a different
task or for some other reason.  in fact, here's the entire list for
the whole tree:

$ grep -r set_task_state *
arch/powerpc/kernel/semaphore.c:	__set_task_state(tsk, TASK_UNINTERRUPTIBLE);
arch/powerpc/kernel/semaphore.c:		set_task_state(tsk, TASK_UNINTERRUPTIBLE);
arch/powerpc/kernel/semaphore.c:	__set_task_state(tsk, TASK_RUNNING);
arch/powerpc/kernel/semaphore.c:	__set_task_state(tsk, TASK_INTERRUPTIBLE);
arch/powerpc/kernel/semaphore.c:		set_task_state(tsk, TASK_INTERRUPTIBLE);
arch/powerpc/kernel/semaphore.c:	__set_task_state(tsk, TASK_RUNNING);
arch/alpha/kernel/semaphore.c:		set_task_state(tsk, TASK_UNINTERRUPTIBLE);
arch/alpha/kernel/semaphore.c:		set_task_state(tsk, TASK_INTERRUPTIBLE);
arch/parisc/kernel/semaphore.c:		set_task_state(current, TASK_UNINTERRUPTIBLE);
arch/parisc/kernel/semaphore.c:		set_task_state(current, TASK_INTERRUPTIBLE);
arch/mn10300/kernel/semaphore.c:	set_task_state(tsk, TASK_UNINTERRUPTIBLE);
arch/mn10300/kernel/semaphore.c:		set_task_state(tsk, TASK_UNINTERRUPTIBLE);
arch/mn10300/kernel/semaphore.c:	set_task_state(tsk, TASK_INTERRUPTIBLE);
arch/mn10300/kernel/semaphore.c:		set_task_state(tsk, TASK_INTERRUPTIBLE);
arch/s390/kernel/semaphore.c:	__set_task_state(tsk, TASK_UNINTERRUPTIBLE);
arch/s390/kernel/semaphore.c:		set_task_state(tsk, TASK_UNINTERRUPTIBLE);
arch/s390/kernel/semaphore.c:	__set_task_state(tsk, TASK_RUNNING);
arch/s390/kernel/semaphore.c:	__set_task_state(tsk, TASK_INTERRUPTIBLE);
arch/s390/kernel/semaphore.c:		set_task_state(tsk, TASK_INTERRUPTIBLE);
arch/s390/kernel/semaphore.c:	__set_task_state(tsk, TASK_RUNNING);
arch/s390/mm/fault.c:		set_task_state(tsk, TASK_UNINTERRUPTIBLE);
arch/s390/mm/fault.c:			set_task_state(tsk, TASK_RUNNING);
arch/frv/kernel/semaphore.c:	set_task_state(tsk, TASK_UNINTERRUPTIBLE);
arch/frv/kernel/semaphore.c:		set_task_state(tsk, TASK_UNINTERRUPTIBLE);
arch/frv/kernel/semaphore.c:	set_task_state(tsk, TASK_INTERRUPTIBLE);
arch/frv/kernel/semaphore.c:		set_task_state(tsk, TASK_INTERRUPTIBLE);
arch/mips/kernel/semaphore.c:	__set_task_state(tsk, TASK_UNINTERRUPTIBLE);
arch/mips/kernel/semaphore.c:		set_task_state(tsk, TASK_UNINTERRUPTIBLE);
arch/mips/kernel/semaphore.c:	__set_task_state(tsk, TASK_RUNNING);
arch/mips/kernel/semaphore.c:	__set_task_state(tsk, TASK_INTERRUPTIBLE);
arch/mips/kernel/semaphore.c:		set_task_state(tsk, TASK_INTERRUPTIBLE);
arch/mips/kernel/semaphore.c:	__set_task_state(tsk, TASK_RUNNING);
Documentation/scheduler/sched-coding.txt:set_task_state(tsk, state_value)
drivers/mmc/core/sdio_irq.c:		set_task_state(current, TASK_INTERRUPTIBLE);
drivers/mmc/core/sdio_irq.c:		set_task_state(current, TASK_RUNNING);
drivers/mfd/ucb1x00-ts.c:			set_task_state(tsk, TASK_INTERRUPTIBLE);
drivers/mfd/ucb1x00-ts.c:			set_task_state(tsk, TASK_INTERRUPTIBLE);
fs/aio.c:	set_task_state(tsk, TASK_UNINTERRUPTIBLE);
fs/aio.c:		set_task_state(tsk, TASK_UNINTERRUPTIBLE);
fs/aio.c:	__set_task_state(tsk, TASK_RUNNING);
fs/aio.c:			set_task_state(tsk, TASK_INTERRUPTIBLE);
fs/aio.c:		set_task_state(tsk, TASK_RUNNING);
include/linux/sched.h:/* Convenience macros for the sake of set_task_state */
include/linux/sched.h:#define __set_task_state(tsk, state_value)		\
include/linux/sched.h:#define set_task_state(tsk, state_value)		\
kernel/ptrace.c:			__set_task_state(child, TASK_STOPPED);
kernel/fork.c:			__set_task_state(p, TASK_STOPPED);
kernel/mutex.c:		__set_task_state(task, state);
lib/rwsem-spinlock.c:	set_task_state(tsk, TASK_UNINTERRUPTIBLE);
lib/rwsem-spinlock.c:		set_task_state(tsk, TASK_UNINTERRUPTIBLE);
lib/rwsem-spinlock.c:	set_task_state(tsk, TASK_UNINTERRUPTIBLE);
lib/rwsem-spinlock.c:		set_task_state(tsk, TASK_UNINTERRUPTIBLE);
lib/rwsem.c:	set_task_state(tsk, TASK_UNINTERRUPTIBLE);
lib/rwsem.c:		set_task_state(tsk, TASK_UNINTERRUPTIBLE);

  once you ignore the semaphore stuff (and a bit of the rest, like the
stuff in sched.h), there's not really that much left that could be
rewritten with set_current_state() anyway.

rday
--

========================================================================
Robert P. J. Day
Linux Consulting, Training and Annoying Kernel Pedantry:
    Have classroom, will lecture.

http://crashcourse.ca                          Waterloo, Ontario, CANADA
========================================================================
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/