Re: linux-next: manual merge of the akpm-current tree with the tiptree

From: Peter Zijlstra
Date: Tue Jan 14 2014 - 08:33:34 EST


On Tue, Jan 14, 2014 at 02:17:55PM +0100, Geert Uytterhoeven wrote:
> On Tue, Jan 14, 2014 at 1:51 PM, Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote:
> > On Tue, Jan 14, 2014 at 03:53:31PM +1100, Stephen Rothwell wrote:
> >> Today's linux-next merge of the akpm-current tree got a conflict in
> >> kernel/futex.c between commit a52b89ebb6d4 ("futexes: Increase hash table
> >> size for better performance") from the tip tree and commit 61beee6c76e5
> >> ("futex: switch to USER_DS for futex test") from the akpm-current tree.
> >>
> >> @@@ -2869,10 -2748,13 +2871,13 @@@
> >> * implementation, the non-functional ones will return
> >> * -ENOSYS.
> >> */
> >> + fs = get_fs();
> >> + set_fs(USER_DS);
> >> if (cmpxchg_futex_value_locked(&curval, NULL, 0, 0) == -EFAULT)
> >> futex_cmpxchg_enabled = 1;
> >> + set_fs(fs);
> >>
> >
> > This seems terribly broken, the *futex_value*() ops should not need
> > that; they are supposed to access userspace without any of that.
>
> Why don't they need set_fs(USER_DS)?

Why would they need it? These functions explicitly take a __user pointer
and are expected to do whatever is needed to perform the operation. None
of the other futex bits use USER_DS either.


Furthermore, from the Changelog of:
e4f2dfbb5e92be4e46c0625f4f8eb101110f756f

" This patch adds that support, but only for uniprocessor machines,
which is adequate for M68K. For UP it's enough to disable preemption
to ensure mutual exclusion (futexes don't need to care about other
hardware agents), and the mandatory pagefault_disable() does just that. "

It is wrong to rely on the fact that pagefault_disable() disables
preemption. This is fact not true on -rt.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/