On Thu, 19 Apr 2018 18:08:28 +0200
Oleg Nesterov <oleg@xxxxxxxxxx> wrote:
> On 04/19, Martin Schwidefsky wrote:
> >
> > On Thu, 19 Apr 2018 16:07:29 +0200
> > Oleg Nesterov <oleg@xxxxxxxxxx> wrote:
> >
> > > On 04/19, Martin Schwidefsky wrote:
> > > >
> > > > In a multi-threaded program any thread can call execve(). If this
> > > > is not done by the thread group leader, the de_thread() function
> > > > replaces the pid of the task that calls execve() with the pid of
> > > > thread group leader. If the task reaches user space again without
> > > > going over __switch_to() the sampling tag is still set to the old
> > > > pid.
> > >
> > > If this is the only reason for arch_change_pid() hook, then perhaps
> > > it would be better/simpler to add it into de_thread() right after
> > > change_pid(tsk, PIDTYPE_PID, task_pid(leader)) ?
> > >
> > > note also that this way arch_change_pid() doesn't need any checks and
> > > any arguments, you can simply do
> > >
> > > void arch_change_pid(void)
> > > {
> > > S390_lowcore.current_pid = current->pid;
> > > if (test_facility(40))
> > > lpp(&S390_lowcore.lpp);
> > > }
> >
> > Yeah, the first patch I created to verify that this indeed the problem
> > basically looked like your proposal. But then why not make it a more
> > general hook for any kind of PID change?
>
> Well, I doubt very much we will ever need an arch-specific hook for
> sys_setsid() or setpgid()... plus
>
> > Dunno if this is a worthwhile
> > approach, the simpler version for sure works as well.
>
> and perhaps you can make another, even more simple change? can't you just
> introduce the s390 version of arch_setup_new_exec, something like
>
> void arch_setup_new_exec(void)
> {
> if (S390_lowcore.current_pid != current->pid) {
> S390_lowcore.current_pid = current->pid;
> ...
> }
> }
>
> ?
Nice, set_new_exec is called after flush_old_exec/de_thread. Yes, that
should work as well and does not require a new arch hook.
Thanks Oleg!
--
blue skies,
Martin.
"Reality continues to ruin my life." - Calvin.
