On Tue, Oct 25, 2016 at 5:43 PM, Oleg Nesterov <o...@redhat.com> wrote: > On 10/25, Oleg Nesterov wrote: >> >> On 10/25, Roman Pen wrote: >> > >> > This patch avoids allocation of kthread structure on a stack, and simply >> > uses kmalloc. >> >> Oh. I didn't even read this patch, but I have to admit I personally do not >> like it. I can be wrong, but imo this is the step to the wrong direction. > > And after I tried to actually read it I dislike it even more, sorry Roman. > Starting from the fact it moves kthread_create_info into struct kthread.
that can be changed, of course, as I told, I wanted to keep allocations/ deallocations simpler. >> struct kthread is already bloated, we should not bloat it more. Instead >> we should kill it. And to_kthread() too, at least in its current form. > > Yes, but even if we can't or do not want to do this, even if we want to > kmalloc struct kthread, I really think it should not be refcounted > separately from task_struct. it is already like that, we have to get/put references on a task stack. > > something like the patch in http://marc.info/?l=linux-kernel&m=146715459127804 the key function in that patch is: free_kthread_struct(tsk); so if we teach the generic free_task() to deal with kthreads, that of course solves these kind of problems. I did not consider that variant. > > Either way to_live_kthread() must go away. Currently we can't avoid it > because we abuse vfork_done, but as I already said we no longer need this. There is something which I do not understand. You still need to have a connection (a pointer) between task_struct and private data (kthread AND private data, whatever), which is passed by the user of kthread API. You still need to find a victim in a task_struct and abuse it :) So in particular I do not understand this comment from the patch above where you abuse 'current->set_child_tid': * This is the ugly but simple hack we will hopefully remove soon. how you are going to avoid this abuse of set_child_tid? or vfork_done? because vfork_done is not only for waking up (yes, I totally agree, we can reuse task_work), it is also for getting a private data (like workqueue uses it): task_struct->vfork_done->kthread->data. -- Roman