linux-kernel - Re: [PATCH V2] futex: set FLAGS_HAS_TIMEOUT during demux for FUTEX

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]

Message-ID: <1302810514.2744.39.camel@edumazet-laptop>
Date:	Thu, 14 Apr 2011 21:48:34 +0200
From:	Eric Dumazet <eric.dumazet@...il.com>
To:	Darren Hart <dvhart@...ux.intel.com>
Cc:	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Peter Zijlstra <peterz@...radead.org>,
	Ingo Molnar <mingo@...e.hu>, John Kacur <jkacur@...hat.com>
Subject: Re: [PATCH V2] futex: set FLAGS_HAS_TIMEOUT during demux for
 FUTEX_WAIT

Le jeudi 14 avril 2011 à 12:11 -0700, Darren Hart a écrit :

> I would say anything calling SYS_FUTEX is the futex slow path. The fast
> path is cmpxchg in user space.
> 

Thats not a good reason to make it slower than necessary...

> It was. My thinking was that it was inconsistent to have the
> FLAGS_HAS_TIMEOUT only available if a signal was received and a restart
> was required. This is the only place it is currently needed, but the
> inconsistency concerns me.
> 

I dont call this inconsistency, but right place for the code.

> How about the following, it reuses an existing if block and ensure the
> FLAGS_HAS_TIMEOUT is always set if a timeout is used. It means the
> FLAG_HAS_TIMEOUT is not available in the other futex_* routines with
> timeouts (futex_lock_pi and futex_wait_requeue_pi), but they use absolute
> timeouts and don't need it for restart - I can agree to that, although
> I'm not keen on FLAG_HAS_TIMEOUT not being set whenever timeout is. That
> could be added in the same way to the other functions if needed in the
> future.

I dont understand why you insist setting in fast path a flag that is
useless, unless we hit restart logic [ What I call the slow path in
futex syscall ]

It seems more natural and efficient to me to go back to previous code.

Maybe rename FLAG_HAS_TIMEOUT to FLAG_HAS_TIMEOUT_ON_RESTART if you
want, to make clear what is the meaning of this flag.

Now if you have plans to use this flag in futex code, outside of restart
logic, please share them with us :)



--
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/