linux-kernel - Re: [PATCH 0/2] kthread: kill task_get_live

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <alpine.LFD.2.02.1303112210490.22263@ionos>
Date:	Mon, 11 Mar 2013 22:15:38 +0100 (CET)
From:	Thomas Gleixner <tglx@...utronix.de>
To:	Oleg Nesterov <oleg@...hat.com>
cc:	Andrew Morton <akpm@...ux-foundation.org>,
	Namhyung Kim <namhyung@...nel.org>,
	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>,
	Peter Zijlstra <peterz@...radead.org>,
	Rusty Russell <rusty@...tcorp.com.au>,
	"Srivatsa S. Bhat" <srivatsa.bhat@...ux.vnet.ibm.com>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 0/2] kthread: kill task_get_live_kthread()

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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/