All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christian Brauner <christian.brauner@ubuntu.com>
To: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
Cc: "Luck, Tony" <tony.luck@intel.com>,
	"Yu, Fenghua" <fenghua.yu@intel.com>,
	"linux-ia64@vger.kernel.org" <linux-ia64@vger.kernel.org>,
	Al Viro <viro@zeniv.linux.org.uk>, Arnd Bergmann <arnd@arndb.de>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@kernel.org>,
	Sebastian Andrzej Siewior <bigeasy@linutronix.de>,
	"Peter Zijlstra (Intel)" <peterz@infradead.org>,
	Qais Yousef <qais.yousef@arm.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] ia64: enable HAVE_COPY_THREAD_TLS, switch to kernel_clone_args
Date: Thu, 14 May 2020 12:37:08 +0200	[thread overview]
Message-ID: <20200514103708.zw3c5yaoumqpnxrm@wittgenstein> (raw)
In-Reply-To: <20200514103259.tdfjc5ds4igpmoxj@wittgenstein>

On Thu, May 14, 2020 at 12:33:00PM +0200, Christian Brauner wrote:
> On Thu, May 14, 2020 at 12:21:13PM +0200, John Paul Adrian Glaubitz wrote:
> > On 5/14/20 12:19 PM, Christian Brauner wrote:
> > > Scratch that. It's even worse. On ia64 it is _invalid_ to pass a NULL
> > > stack. That's at least what the glibc assembly assumes:
> > > 
> > > 	cmp.eq p6,p0=0,in0
> > > 	cmp.eq p7,p0=0,in1
> > > 	mov r8=EINVAL
> > > 	mov out0=in3		/* Flags are first syscall argument.	*/
> > > 	mov out1=in1		/* Stack address.			*/
> > > (p6)	br.cond.spnt.many __syscall_error	/* no NULL function pointers */
> > > (p7)	br.cond.spnt.many __syscall_error	/* no NULL stack pointers */
> > > 	;;
> > > 	mov out2=in2		/* Stack size.				*/
> > > 
> > > so newer systemd just works by accident on ia64 if at all correctly
> > > afaict.
> > 
> > Hmm, interesting. I really wasn't aware of that. Thanks for the heads-up.
> > 
> > I'll ask Michael whether he can come up for a solution for that problem.
> > 
> > Maybe that's also why systemd crashes.
> 
> Do you have a very minimalistic ia64 userspace preferably without systemd where
> you could simply test. That should give us an idea whether things work:
> 
> #define _GNU_SOURCE
> #include <sys/wait.h>
> #include <sys/utsname.h>
> #include <sched.h>
> #include <string.h>
> #include <stdio.h>
> #include <stdlib.h>
> #include <unistd.h>
> #include <sys/mman.h>
> 
> #define STACK_SIZE (8 * 1024 * 1024) /* standard stack size for threads in glibc */
> 
> int main(int argc, char *argv[])
> {
> 	char *stack;
>         pid_t pid;
> 
> 	stack = mmap(NULL, STACK_SIZE, PROT_READ | PROT_WRITE,
> 		     MAP_PRIVATE | MAP_ANONYMOUS | MAP_STACK, -1, 0);
> 	if (stack == MAP_FAILED)
> 		exit(EXIT_FAILURE);
> 
>         /* 
> 	 * Note that legacy clone() has different argument ordering on
>          * different architectures so this won't work everywhere.
>          */
>         pid = syscall(189 /* __NR_clone2 */, SIGCHLD, stack, STACK_SIZE, NULL, NULL);

Please note that even on ia64 the stack grows down but in contrast to
all other architectures ia64 expects the _lowest_ address to be given
and will add STACK_SIZE to stack itself in copy_thread{_tls}(). (This is
all fixed in clone3() where you're always expected to pass down the
lowest address and the kernel figures it out for you.)

So this is intentional.

Christian

WARNING: multiple messages have this Message-ID (diff)
From: Christian Brauner <christian.brauner@ubuntu.com>
To: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
Cc: "Luck, Tony" <tony.luck@intel.com>,
	"Yu, Fenghua" <fenghua.yu@intel.com>,
	"linux-ia64@vger.kernel.org" <linux-ia64@vger.kernel.org>,
	Al Viro <viro@zeniv.linux.org.uk>, Arnd Bergmann <arnd@arndb.de>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@kernel.org>,
	Sebastian Andrzej Siewior <bigeasy@linutronix.de>,
	"Peter Zijlstra (Intel)" <peterz@infradead.org>,
	Qais Yousef <qais.yousef@arm.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] ia64: enable HAVE_COPY_THREAD_TLS, switch to kernel_clone_args
Date: Thu, 14 May 2020 10:37:08 +0000	[thread overview]
Message-ID: <20200514103708.zw3c5yaoumqpnxrm@wittgenstein> (raw)
In-Reply-To: <20200514103259.tdfjc5ds4igpmoxj@wittgenstein>

On Thu, May 14, 2020 at 12:33:00PM +0200, Christian Brauner wrote:
> On Thu, May 14, 2020 at 12:21:13PM +0200, John Paul Adrian Glaubitz wrote:
> > On 5/14/20 12:19 PM, Christian Brauner wrote:
> > > Scratch that. It's even worse. On ia64 it is _invalid_ to pass a NULL
> > > stack. That's at least what the glibc assembly assumes:
> > > 
> > > 	cmp.eq p6,p0=0,in0
> > > 	cmp.eq p7,p0=0,in1
> > > 	mov r8=EINVAL
> > > 	mov out0=in3		/* Flags are first syscall argument.	*/
> > > 	mov out1=in1		/* Stack address.			*/
> > > (p6)	br.cond.spnt.many __syscall_error	/* no NULL function pointers */
> > > (p7)	br.cond.spnt.many __syscall_error	/* no NULL stack pointers */
> > > 	;;
> > > 	mov out2=in2		/* Stack size.				*/
> > > 
> > > so newer systemd just works by accident on ia64 if at all correctly
> > > afaict.
> > 
> > Hmm, interesting. I really wasn't aware of that. Thanks for the heads-up.
> > 
> > I'll ask Michael whether he can come up for a solution for that problem.
> > 
> > Maybe that's also why systemd crashes.
> 
> Do you have a very minimalistic ia64 userspace preferably without systemd where
> you could simply test. That should give us an idea whether things work:
> 
> #define _GNU_SOURCE
> #include <sys/wait.h>
> #include <sys/utsname.h>
> #include <sched.h>
> #include <string.h>
> #include <stdio.h>
> #include <stdlib.h>
> #include <unistd.h>
> #include <sys/mman.h>
> 
> #define STACK_SIZE (8 * 1024 * 1024) /* standard stack size for threads in glibc */
> 
> int main(int argc, char *argv[])
> {
> 	char *stack;
>         pid_t pid;
> 
> 	stack = mmap(NULL, STACK_SIZE, PROT_READ | PROT_WRITE,
> 		     MAP_PRIVATE | MAP_ANONYMOUS | MAP_STACK, -1, 0);
> 	if (stack = MAP_FAILED)
> 		exit(EXIT_FAILURE);
> 
>         /* 
> 	 * Note that legacy clone() has different argument ordering on
>          * different architectures so this won't work everywhere.
>          */
>         pid = syscall(189 /* __NR_clone2 */, SIGCHLD, stack, STACK_SIZE, NULL, NULL);

Please note that even on ia64 the stack grows down but in contrast to
all other architectures ia64 expects the _lowest_ address to be given
and will add STACK_SIZE to stack itself in copy_thread{_tls}(). (This is
all fixed in clone3() where you're always expected to pass down the
lowest address and the kernel figures it out for you.)

So this is intentional.

Christian

  parent reply	other threads:[~2020-05-14 10:37 UTC|newest]

Thread overview: 51+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-13 20:48 [PATCH] ia64: enable HAVE_COPY_THREAD_TLS, switch to kernel_clone_args Christian Brauner
2020-05-13 20:48 ` Christian Brauner
2020-05-13 21:19 ` Luck, Tony
2020-05-13 21:26   ` John Paul Adrian Glaubitz
2020-05-13 21:26     ` John Paul Adrian Glaubitz
2020-05-14  7:46     ` Christian Brauner
2020-05-14  7:46       ` Christian Brauner
2020-05-14  7:53       ` John Paul Adrian Glaubitz
2020-05-14  7:53         ` John Paul Adrian Glaubitz
2020-05-14  7:58         ` Christian Brauner
2020-05-14  7:58           ` Christian Brauner
2020-05-14  8:24           ` John Paul Adrian Glaubitz
2020-05-14  8:24             ` John Paul Adrian Glaubitz
2020-05-14  8:37             ` Christian Brauner
2020-05-14  8:37               ` Christian Brauner
2020-05-14  8:51               ` John Paul Adrian Glaubitz
2020-05-14  8:51                 ` John Paul Adrian Glaubitz
2020-05-14  9:48         ` John Paul Adrian Glaubitz
2020-05-14  9:48           ` John Paul Adrian Glaubitz
2020-05-14 10:04           ` Christian Brauner
2020-05-14 10:04             ` Christian Brauner
2020-05-14 10:08             ` John Paul Adrian Glaubitz
2020-05-14 10:08               ` John Paul Adrian Glaubitz
2020-05-14 10:15               ` Christian Brauner
2020-05-14 10:15                 ` Christian Brauner
2020-05-14 10:19                 ` Christian Brauner
2020-05-14 10:19                   ` Christian Brauner
2020-05-14 10:21                   ` John Paul Adrian Glaubitz
2020-05-14 10:21                     ` John Paul Adrian Glaubitz
2020-05-14 10:32                     ` Christian Brauner
2020-05-14 10:32                       ` Christian Brauner
2020-05-14 10:35                       ` John Paul Adrian Glaubitz
2020-05-14 10:35                         ` John Paul Adrian Glaubitz
2020-05-14 10:39                         ` Christian Brauner
2020-05-14 10:39                           ` Christian Brauner
2020-05-14 10:37                       ` Christian Brauner [this message]
2020-05-14 10:37                         ` Christian Brauner
2020-05-14 10:45                       ` Andreas Schwab
2020-05-14 10:45                         ` Andreas Schwab
2020-05-14 10:51                         ` Christian Brauner
2020-05-14 10:51                           ` Christian Brauner
2020-05-14 10:37                   ` Andreas Schwab
2020-05-14 10:37                     ` Andreas Schwab
2020-05-14 13:00           ` Christian Brauner
2020-05-14 13:00             ` Christian Brauner
2020-05-14  8:58       ` Sebastian Andrzej Siewior
2020-05-14  8:58         ` Sebastian Andrzej Siewior
2020-05-14  9:57         ` Christian Brauner
2020-05-14  9:57           ` Christian Brauner
2020-05-14  7:50   ` Christian Brauner
2020-05-14  7:50     ` Christian Brauner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200514103708.zw3c5yaoumqpnxrm@wittgenstein \
    --to=christian.brauner@ubuntu.com \
    --cc=arnd@arndb.de \
    --cc=bigeasy@linutronix.de \
    --cc=fenghua.yu@intel.com \
    --cc=glaubitz@physik.fu-berlin.de \
    --cc=linux-ia64@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=peterz@infradead.org \
    --cc=qais.yousef@arm.com \
    --cc=tglx@linutronix.de \
    --cc=tony.luck@intel.com \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.