Messages in this thread | | | Date | Fri, 12 Jun 2020 16:41:20 +0200 | From | Christian Brauner <> | Subject | Re: [PATCH] nios2: enable HAVE_COPY_THREAD_TLS, switch to kernel_clone_args |
| |
On Fri, Jun 12, 2020 at 06:20:48AM +0000, Tan, Ley Foon wrote: > > > > -----Original Message----- > > From: Christian Brauner <christian.brauner@ubuntu.com> > > Sent: Sunday, May 24, 2020 6:06 PM > > To: Tan, Ley Foon <ley.foon.tan@intel.com>; Arnd Bergmann > > <arnd@arndb.de> > > Cc: linux-kernel@vger.kernel.org; Thomas Gleixner <tglx@linutronix.de>; > > Sebastian Andrzej Siewior <bigeasy@linutronix.de>; Christian Brauner > > <christian.brauner@ubuntu.com> > > Subject: [PATCH] nios2: enable HAVE_COPY_THREAD_TLS, switch to > > kernel_clone_args > > > > This is part of a larger series that aims at getting rid of the > > copy_thread()/copy_thread_tls() split that makes the process creation > > codepaths in the kernel more convoluted and error-prone than they need to > > be. > > I'm converting all the remaining arches that haven't yet switched and am > > collecting individual acks. Once I have them, I'll send the whole series > > removing the copy_thread()/copy_thread_tls() split, the > > HAVE_COPY_THREAD_TLS define and the legacy do_fork() helper. The only > > kernel-wide process creation entry point for anything not going directly > > through the syscall path will then be based on struct kernel_clone_args. > > No more danger of weird process creation abi quirks between architectures > > hopefully, and easier to maintain overall. > > It also unblocks implementing clone3() on architectures not support > > copy_thread_tls(). Any architecture that wants to implement clone3() will > > need to select HAVE_COPY_THREAD_TLS and thus need to implement > > copy_thread_tls(). So both goals are connected but independently beneficial. > > > > HAVE_COPY_THREAD_TLS means that a given architecture supports > > CLONE_SETTLS and not setting it should usually mean that the architectures > > doesn't implement it but that's not how things are. In fact all architectures > > support CLONE_TLS it's just that they don't follow the calling convention that > > HAVE_COPY_THREAD_TLS implies. That means all architectures can be > > switched over to select HAVE_COPY_THREAD_TLS. Once that is done we can > > remove that macro (yay, less code), the unnecessary do_fork() export in > > kernel/fork.c, and also rename copy_thread_tls() back to copy_thread(). At > > this point > > copy_thread() becomes the main architecture specific part of process > > creation but it will be the same layout and calling convention for all > > architectures. (Once that is done we can probably cleanup each > > copy_thread() function even more but that's for the future.) > > > > Since nios2 does support CLONE_SETTLS there's no reason to not select > > HAVE_COPY_THREAD_TLS. This brings us one step closer to getting rid of the > > copy_thread()/copy_thread_tls() split we still have and ultimately the > > HAVE_COPY_THREAD_TLS define in general. A lot of architectures have > > already converted and nios2 is one of the few hat haven't yet. This also > > unblocks implementing the clone3() syscall on nios2. Once that is done we > > can get of another ARCH_WANTS_* macro. > > > > Once Any architecture that supports HAVE_COPY_THREAD_TLS cannot call > > the > > do_fork() helper anymore. This is fine and intended since it should be > > removed in favor of the new, cleaner _do_fork() calling convention based on > > struct kernel_clone_args. In fact, most architectures have already switched. > > With this patch, nios2 joins the other arches which can't use the fork(), > > vfork(), clone(), clone3() syscalls directly and who follow the new process > > creation calling convention that is based on struct kernel_clone_args which > > we introduced a while back. This means less custom assembly in the > > architectures entry path to set up the registers before calling into the process > > creation helper and it is easier to to support new features without having to > > adapt calling conventions. It also unifies all process creation paths between > > fork(), vfork(), clone(), and clone3(). (We can't fix the ABI nightmare that > > legacy > > clone() is but we can prevent stuff like this happening in the future.) > > > > For some more context, please see: > > commit 606e9ad20094f6d500166881d301f31a51bc8aa7 > > Merge: ac61145a725a 457677c70c76 > > Author: Linus Torvalds <torvalds@linux-foundation.org> > > Date: Sat Jan 11 15:33:48 2020 -0800 > > > > Merge tag 'clone3-tls-v5.5-rc6' of > > git://git.kernel.org/pub/scm/linux/kernel/git/brauner/linux > > > > Pull thread fixes from Christian Brauner: > > "This contains a series of patches to fix CLONE_SETTLS when used with > > clone3(). > > > > The clone3() syscall passes the tls argument through struct clone_args > > instead of a register. This means, all architectures that do not > > implement copy_thread_tls() but still support CLONE_SETTLS via > > copy_thread() expecting the tls to be located in a register argument > > based on clone() are currently unfortunately broken. Their tls value > > will be garbage. > > > > The patch series fixes this on all architectures that currently define > > __ARCH_WANT_SYS_CLONE3. It also adds a compile-time check to > > ensure > > that any architecture that enables clone3() in the future is forced to > > also implement copy_thread_tls(). > > > > My ultimate goal is to get rid of the copy_thread()/copy_thread_tls() > > split and just have copy_thread_tls() at some point in the not too > > distant future (Maybe even renaming copy_thread_tls() back to simply > > copy_thread() once the old function is ripped from all arches). This > > is dependent now on all arches supporting clone3(). > > > > While all relevant arches do that now there are still four missing: > > ia64, m68k, sh and sparc. They have the system call reserved, but not > > implemented. Once they all implement clone3() we can get rid of > > ARCH_WANT_SYS_CLONE3 and HAVE_COPY_THREAD_TLS. > > > > Note that in the meantime, m68k has already switched to the new calling > > convention. And I've got sparc patches acked by Dave and ia64 is already > > done too. You can find a link to a booting qemu nios2 system with all the > > changes here at [1]. > > > > [1]: https://asciinema.org/a/333353 > > Cc: linux-kernel@vger.kernel.org > > Cc: Thomas Gleixner <tglx@linutronix.de> > > Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de> > > Cc: Ley Foon Tan <ley.foon.tan@intel.com> > > Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com> > Acked-by: Ley Foon Tan <ley.foon.tan@intel.com>
Thanks!
> > Assume this patch will merge with your series.
If people agree with it, then yep. Just waiting for rc1 before sending it out.
Christian
| |