Re: [PATCH 0/2] kthread: kill task_get_live_kthread()

[Thomas Gleixner]

Oleg,

On Mon, 11 Mar 2013, Oleg Nesterov wrote:
> Imho, task_get_live_kthread() is very confusing and unneeded.
> 2a1d4460 copied get_task_struct() + "vfork_done != NULL" from
> kthread_stop(), but only kthread_stop() needs them both.
> 
> It needs get_task_struct() because kthread_stop() can be used
> when the caller doesn't have a reference but we know that this
> thread can't exit itself.
> 
> At the same time, if it can exit we do not need get_task_struct()
> (the caller must have a reference) but we need to ensure we do not
> use to_kthread(NULL) if it has exited.
> 
> I think that kthread_park/unpark can simply use to_kthread(), but
> this series only removes get_task_struct() and keeps "alive" check.
>
> 
> But the actual reason for this cleanup is that I do not understand
> why park/unpark abuse kthread.c.

It's not abusing it :)
 
> Thomas, can't we move kthread->parked/cpu to smpboot_thread_data
> and move all this code into kernel/smpboot.c? Just for example,
> why kthread() does __kthread_parkme() ? smpboot_thread_fn() can do
> this at the start.

No objection. When I implemented this, I thought this would be the
correct place and I followed the conventions of kthread.c ...
 
> Or this would be wrong/undesirable by some reason?

Need to think about it, but probably it was just either copy and paste
stupidity or some form of paranoia.

What's the issue with that, other than some superflous task_get/put
calls ?

Thanks,

	tglx

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/