[PATCH 0/2] kthread: make struct kthread kmalloc'ed

[Oleg Nesterov <oleg@redhat.com>]

Sorry for delay, I was distracted...

On 10/26, Thomas Gleixner wrote:
>
> On Wed, 26 Oct 2016, Oleg Nesterov wrote:
> > On 10/26, Thomas Gleixner wrote:
> > >
> > > On Wed, 26 Oct 2016, Oleg Nesterov wrote:
> > > > +static inline void set_kthread_struct(void *kthread)
> > > > +{
> > > > +	/*
> > > > +	 * We abuse ->set_child_tid to avoid the new member and because it
> > > > +	 * can't be wrongly copied by copy_process(). We also rely on fact
> > > > +	 * that the caller can't exec, so PF_KTHREAD can't be cleared.
> > > > +	 */
> > > > +	current->set_child_tid = (__force void __user *)kthread;
> > >
> > > Can we pretty please avoid this type casting? We only have 5 places using
> > > set_child_tid. So we can really make it a proper union
> >
> > Yes, I thought about anonymous union too, the only problem is that
> > it will need more comments ;)
>
> Be careful with anonymous unions. There are a few pitfalls with older
> compilers. That's why I said make it a proper union and fixup the 5 usage
> sites.

Ah. Then I'd prefer to do this later or in a separate change, unless you
feel strongly. I certainly do not want to update other users at least right
now.

Yes, these 2 type casts do not look nice, but they are hidden in the trivial
helpers. And, for example, if something goes wrong we can trivially change
this code to use, say, sas_ss_sp. Just we need to update the comments to
explain why it is safe too.

Finally. I still hope we will kill struct kthread (I mean, unbloat it and
embed into task_struct), and this means that the proper union should touch
more members. Say, sas_* and/or vfork_done+set/clear_child_tid. I'd like
to do this only once if possible.


I'll try to kill to_live_kthread() tomorrow, didn't have time to do this
today.

Oleg.