Re: [PATCH 1/3] sched: add sched_task_call()

Vojtech Pavlik
2015-02-19T16:34:03Z

On Thu, Feb 19, 2015 at 10:24:29AM -0600, Josh Poimboeuf wrote:

> > No, these tasks will _never_ make syscalls. So you need to guarantee
> > they don't accidentally enter the kernel while you flip them. Something
> > like so should do.
> >
> > You set TIF_ENTER_WAIT on them, check they're still in userspace, flip
> > them then clear TIF_ENTER_WAIT.
>
> Ah, that's a good idea. But how do we check if they're in user space?

I don't see the benefit in holding them in a loop - you can just as well
flip them from the syscall code as kGraft does.

> > I still absolutely hate you need to disturb userspace like that. Signals
> > are quite visible and perturb userspace state.
>
> Yeah, SIGSTOP on a sleeping task can be intrusive to user space if it
> results in EINTR being returned from a system call. It's not ideal, but
> it's much less intrusive than something like suspend.
>
> But anyway we leave it up to the user to decide whether they want to
> take that risk, or wait for the task to wake up on its own, or cancel
> the patch.
>
> > Also, you cannot SIGCONT a task that was SIGSTOP'ed by userspace for
> > what they thought was a good reason. You'd wreck their state.
>
> Hm, that's a good point. Maybe use the freezer instead of signals?
>
> (Again this would only be for those user tasks which are sleeping on a
> patched function)
>
> > > But now I'm thinking that kthreads will almost never be a problem. Most
> > > kthreads are basically this:
> >
> > You guys are way too optimistic; maybe its because I've worked on
> > realtime stuff too much, but I'm always looking at worst cases. If you
> > can't handle those, I feel you might as well not bother :-)
>
> Well, I think we're already resigned to the fact that live patching
> won't work for every patch, every time. And that the patch author must
> know what they're doing, and must do it carefully.
>
> Our target worst case is that the patching fails gracefully and the user
> can't patch their system with that particular patch. Or that the system
> remains in a partially patched state forever, if the user is ok with
> that.
>
> > > Patching thread_fn wouldn't be possible unless we killed the thread.
> >
> > It is, see kthread_park().
>
> When the kthread returns from kthread_parkme(), it'll still be running
> the old thread_fn code, regardless of whether we flipped the task's
> patch state.

We could instrument kthread_parkme() to be able to return to a different
function, but it'd be a bit crazy indeed.

--
Vojtech Pavlik
Director SUSE Labs