On Mon Mar 27, 2023 at 8:26 PM AEST, Christophe Leroy wrote: > > > Le 27/03/2023 à 08:36, Nicholas Piggin a écrit : > > On Mon Mar 27, 2023 at 8:15 AM AEST, Jens Axboe wrote: > >> Powerpc sets up PF_KTHREAD and PF_IO_WORKER with a NULL pt_regs, which > >> from my (arguably very short) checking is not commonly done for other > >> archs. This is fine, except when PF_IO_WORKER's have been created and > >> the task does something that causes a coredump to be generated. Then we > >> get this crash: > > > > Hey Jens, > > > > Thanks for the testing and the patch. > > > > I think your patch would work, but I'd be inclined to give the IO worker > > a pt_regs so it looks more like other archs and a regular user thread. > > > > Your IO worker bug reminded me to resurrect some copy_thread patches I > > had and I think they should do that > > > > https://lists.ozlabs.org/pipermail/linuxppc-dev/2023-March/256271.html > > > > I wouldn't ask you to test it until I've at least tried, do you have a > > test case that triggers this? > > I fact, most architectures don't have thread.regs, but rely on something > like: > > #define task_pt_regs(p) \ > ((struct pt_regs *)(THREAD_SIZE + task_stack_page(p)) - 1) > > > In powerpc, thread.regs was there because of the regs not being at the > same place in the stack depending on which interrupt it was. > > However with the two commits below, we now have stable fixed location > for the regs, so thread.regs shouldn't be needed anymore: > - db297c3b07af ("powerpc/32: Don't save thread.regs on interrupt entry") > - b5cfc9cd7b04 ("powerpc/32: Fix critical and debug interrupts on BOOKE") > > But in the meantime, thread.regs started to be used for additional > purpose, like knowing if it is a user thread or a kernel thread by using > thread.regs nullity. > > > Now that thread.regs doesn't change anymore at each interrupt, it would > probably be worth dropping it and falling back to task_pt_regs() as > defined on most architecture. > Knowing whether a thread is a kernel or user thread can for instance be > known with PF_KTHREAD flag, so no need of thread.regs for that.
That would be nice if we can define regs that way, I agree. We should look into doing that. Thanks, Nick
