* [PATCH] kthread: Use TASK_IDLE state for newly created kernel threads @ 2020-05-20 11:55 Pavankumar Kondeti 2020-05-20 18:18 ` Greg Kroah-Hartman 0 siblings, 1 reply; 6+ messages in thread From: Pavankumar Kondeti @ 2020-05-20 11:55 UTC (permalink / raw) To: linux-kernel Cc: Vinayak Menon, Pavankumar Kondeti, Greg Kroah-Hartman, Ben Dooks, Peter Zijlstra (Intel), Thomas Gleixner, Steven Rostedt (VMware), Liang Chen When kernel threads are created for later use, they will be in TASK_UNINTERRUPTIBLE state until they are woken up. This results in increased loadavg and false hung task reports. To fix this, use TASK_IDLE state instead of TASK_UNINTERRUPTIBLE when a kernel thread schedules out for the first time. Signed-off-by: Pavankumar Kondeti <pkondeti@codeaurora.org> --- kernel/kthread.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/kernel/kthread.c b/kernel/kthread.c index bfbfa48..b74ed8e 100644 --- a/kernel/kthread.c +++ b/kernel/kthread.c @@ -250,7 +250,7 @@ static int kthread(void *_create) current->vfork_done = &self->exited; /* OK, tell user we're spawned, wait for stop or wakeup */ - __set_current_state(TASK_UNINTERRUPTIBLE); + __set_current_state(TASK_IDLE); create->result = current; /* * Thread is going to call schedule(), do not preempt it, @@ -428,7 +428,7 @@ static void __kthread_bind(struct task_struct *p, unsigned int cpu, long state) void kthread_bind_mask(struct task_struct *p, const struct cpumask *mask) { - __kthread_bind_mask(p, mask, TASK_UNINTERRUPTIBLE); + __kthread_bind_mask(p, mask, TASK_IDLE); } /** @@ -442,7 +442,7 @@ void kthread_bind_mask(struct task_struct *p, const struct cpumask *mask) */ void kthread_bind(struct task_struct *p, unsigned int cpu) { - __kthread_bind(p, cpu, TASK_UNINTERRUPTIBLE); + __kthread_bind(p, cpu, TASK_IDLE); } EXPORT_SYMBOL(kthread_bind); -- Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project. ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] kthread: Use TASK_IDLE state for newly created kernel threads 2020-05-20 11:55 [PATCH] kthread: Use TASK_IDLE state for newly created kernel threads Pavankumar Kondeti @ 2020-05-20 18:18 ` Greg Kroah-Hartman 2020-05-21 1:35 ` Pavan Kondeti 0 siblings, 1 reply; 6+ messages in thread From: Greg Kroah-Hartman @ 2020-05-20 18:18 UTC (permalink / raw) To: Pavankumar Kondeti Cc: linux-kernel, Vinayak Menon, Ben Dooks, Peter Zijlstra (Intel), Thomas Gleixner, Steven Rostedt (VMware), Liang Chen On Wed, May 20, 2020 at 05:25:09PM +0530, Pavankumar Kondeti wrote: > When kernel threads are created for later use, they will be in > TASK_UNINTERRUPTIBLE state until they are woken up. This results > in increased loadavg and false hung task reports. To fix this, > use TASK_IDLE state instead of TASK_UNINTERRUPTIBLE when > a kernel thread schedules out for the first time. > > Signed-off-by: Pavankumar Kondeti <pkondeti@codeaurora.org> > --- > kernel/kthread.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/kernel/kthread.c b/kernel/kthread.c > index bfbfa48..b74ed8e 100644 > --- a/kernel/kthread.c > +++ b/kernel/kthread.c > @@ -250,7 +250,7 @@ static int kthread(void *_create) > current->vfork_done = &self->exited; > > /* OK, tell user we're spawned, wait for stop or wakeup */ > - __set_current_state(TASK_UNINTERRUPTIBLE); > + __set_current_state(TASK_IDLE); > create->result = current; > /* > * Thread is going to call schedule(), do not preempt it, > @@ -428,7 +428,7 @@ static void __kthread_bind(struct task_struct *p, unsigned int cpu, long state) > > void kthread_bind_mask(struct task_struct *p, const struct cpumask *mask) > { > - __kthread_bind_mask(p, mask, TASK_UNINTERRUPTIBLE); > + __kthread_bind_mask(p, mask, TASK_IDLE); > } > > /** > @@ -442,7 +442,7 @@ void kthread_bind_mask(struct task_struct *p, const struct cpumask *mask) > */ > void kthread_bind(struct task_struct *p, unsigned int cpu) > { > - __kthread_bind(p, cpu, TASK_UNINTERRUPTIBLE); > + __kthread_bind(p, cpu, TASK_IDLE); > } > EXPORT_SYMBOL(kthread_bind); It's as if people never read mailing lists: https://lore.kernel.org/r/DM6PR11MB3531D3B164357B2DC476102DDFC90@DM6PR11MB3531.namprd11.prod.outlook.com Given that this is an identical resend of the previous patch, why are you doing so, and what has changed since that original rejection? greg k-h ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] kthread: Use TASK_IDLE state for newly created kernel threads 2020-05-20 18:18 ` Greg Kroah-Hartman @ 2020-05-21 1:35 ` Pavan Kondeti 2020-05-21 5:56 ` Greg Kroah-Hartman 0 siblings, 1 reply; 6+ messages in thread From: Pavan Kondeti @ 2020-05-21 1:35 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: linux-kernel, Vinayak Menon, Ben Dooks, Peter Zijlstra (Intel), Thomas Gleixner, Steven Rostedt (VMware), Liang Chen On Wed, May 20, 2020 at 08:18:58PM +0200, Greg Kroah-Hartman wrote: > On Wed, May 20, 2020 at 05:25:09PM +0530, Pavankumar Kondeti wrote: > > When kernel threads are created for later use, they will be in > > TASK_UNINTERRUPTIBLE state until they are woken up. This results > > in increased loadavg and false hung task reports. To fix this, > > use TASK_IDLE state instead of TASK_UNINTERRUPTIBLE when > > a kernel thread schedules out for the first time. > > > > Signed-off-by: Pavankumar Kondeti <pkondeti@codeaurora.org> > > --- > > kernel/kthread.c | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/kernel/kthread.c b/kernel/kthread.c > > index bfbfa48..b74ed8e 100644 > > --- a/kernel/kthread.c > > +++ b/kernel/kthread.c > > @@ -250,7 +250,7 @@ static int kthread(void *_create) > > current->vfork_done = &self->exited; > > > > /* OK, tell user we're spawned, wait for stop or wakeup */ > > - __set_current_state(TASK_UNINTERRUPTIBLE); > > + __set_current_state(TASK_IDLE); > > create->result = current; > > /* > > * Thread is going to call schedule(), do not preempt it, > > @@ -428,7 +428,7 @@ static void __kthread_bind(struct task_struct *p, unsigned int cpu, long state) > > > > void kthread_bind_mask(struct task_struct *p, const struct cpumask *mask) > > { > > - __kthread_bind_mask(p, mask, TASK_UNINTERRUPTIBLE); > > + __kthread_bind_mask(p, mask, TASK_IDLE); > > } > > > > /** > > @@ -442,7 +442,7 @@ void kthread_bind_mask(struct task_struct *p, const struct cpumask *mask) > > */ > > void kthread_bind(struct task_struct *p, unsigned int cpu) > > { > > - __kthread_bind(p, cpu, TASK_UNINTERRUPTIBLE); > > + __kthread_bind(p, cpu, TASK_IDLE); > > } > > EXPORT_SYMBOL(kthread_bind); > > It's as if people never read mailing lists: > https://lore.kernel.org/r/DM6PR11MB3531D3B164357B2DC476102DDFC90@DM6PR11MB3531.namprd11.prod.outlook.com > > Given that this is an identical resend of the previous patch, why are > you doing so, and what has changed since that original rejection? > I did not know that it is attempted before. Thanks for pointing to the previous discussion. We have seen hung task reports from customers and it is due to a downstream change which create bunch of kernel threads for later use. From Peter's reply, I understood that one must wake up the kthread after creation and put it in INTERRUPTIBLE sleep. I will pass on the message. Thanks, Pavan -- Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] kthread: Use TASK_IDLE state for newly created kernel threads 2020-05-21 1:35 ` Pavan Kondeti @ 2020-05-21 5:56 ` Greg Kroah-Hartman 2020-05-21 9:51 ` Pavan Kondeti 0 siblings, 1 reply; 6+ messages in thread From: Greg Kroah-Hartman @ 2020-05-21 5:56 UTC (permalink / raw) To: Pavan Kondeti Cc: linux-kernel, Vinayak Menon, Ben Dooks, Peter Zijlstra (Intel), Thomas Gleixner, Steven Rostedt (VMware), Liang Chen On Thu, May 21, 2020 at 07:05:44AM +0530, Pavan Kondeti wrote: > On Wed, May 20, 2020 at 08:18:58PM +0200, Greg Kroah-Hartman wrote: > > On Wed, May 20, 2020 at 05:25:09PM +0530, Pavankumar Kondeti wrote: > > > When kernel threads are created for later use, they will be in > > > TASK_UNINTERRUPTIBLE state until they are woken up. This results > > > in increased loadavg and false hung task reports. To fix this, > > > use TASK_IDLE state instead of TASK_UNINTERRUPTIBLE when > > > a kernel thread schedules out for the first time. > > > > > > Signed-off-by: Pavankumar Kondeti <pkondeti@codeaurora.org> > > > --- > > > kernel/kthread.c | 6 +++--- > > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > > > diff --git a/kernel/kthread.c b/kernel/kthread.c > > > index bfbfa48..b74ed8e 100644 > > > --- a/kernel/kthread.c > > > +++ b/kernel/kthread.c > > > @@ -250,7 +250,7 @@ static int kthread(void *_create) > > > current->vfork_done = &self->exited; > > > > > > /* OK, tell user we're spawned, wait for stop or wakeup */ > > > - __set_current_state(TASK_UNINTERRUPTIBLE); > > > + __set_current_state(TASK_IDLE); > > > create->result = current; > > > /* > > > * Thread is going to call schedule(), do not preempt it, > > > @@ -428,7 +428,7 @@ static void __kthread_bind(struct task_struct *p, unsigned int cpu, long state) > > > > > > void kthread_bind_mask(struct task_struct *p, const struct cpumask *mask) > > > { > > > - __kthread_bind_mask(p, mask, TASK_UNINTERRUPTIBLE); > > > + __kthread_bind_mask(p, mask, TASK_IDLE); > > > } > > > > > > /** > > > @@ -442,7 +442,7 @@ void kthread_bind_mask(struct task_struct *p, const struct cpumask *mask) > > > */ > > > void kthread_bind(struct task_struct *p, unsigned int cpu) > > > { > > > - __kthread_bind(p, cpu, TASK_UNINTERRUPTIBLE); > > > + __kthread_bind(p, cpu, TASK_IDLE); > > > } > > > EXPORT_SYMBOL(kthread_bind); > > > > It's as if people never read mailing lists: > > https://lore.kernel.org/r/DM6PR11MB3531D3B164357B2DC476102DDFC90@DM6PR11MB3531.namprd11.prod.outlook.com > > > > Given that this is an identical resend of the previous patch, why are > > you doing so, and what has changed since that original rejection? > > > I did not know that it is attempted before. Thanks for pointing to the > previous discussion. > > We have seen hung task reports from customers and it is due to a downstream > change which create bunch of kernel threads for later use. Do you have a pointer to that specific change? > From Peter's reply, I understood that one must wake up the kthread > after creation and put it in INTERRUPTIBLE sleep. I will pass on the > message. Just go fix that code, it sounds like it's in your tree already :) thanks, greg k-h ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] kthread: Use TASK_IDLE state for newly created kernel threads 2020-05-21 5:56 ` Greg Kroah-Hartman @ 2020-05-21 9:51 ` Pavan Kondeti 2020-05-21 12:03 ` Greg Kroah-Hartman 0 siblings, 1 reply; 6+ messages in thread From: Pavan Kondeti @ 2020-05-21 9:51 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: linux-kernel, Vinayak Menon, Ben Dooks, Peter Zijlstra (Intel), Thomas Gleixner, Steven Rostedt (VMware), Liang Chen On Thu, May 21, 2020 at 07:56:39AM +0200, Greg Kroah-Hartman wrote: > On Thu, May 21, 2020 at 07:05:44AM +0530, Pavan Kondeti wrote: > > On Wed, May 20, 2020 at 08:18:58PM +0200, Greg Kroah-Hartman wrote: > > > On Wed, May 20, 2020 at 05:25:09PM +0530, Pavankumar Kondeti wrote: > > > > When kernel threads are created for later use, they will be in > > > > TASK_UNINTERRUPTIBLE state until they are woken up. This results > > > > in increased loadavg and false hung task reports. To fix this, > > > > use TASK_IDLE state instead of TASK_UNINTERRUPTIBLE when > > > > a kernel thread schedules out for the first time. > > > > > > > > Signed-off-by: Pavankumar Kondeti <pkondeti@codeaurora.org> > > > > --- > > > > kernel/kthread.c | 6 +++--- > > > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > > > > > diff --git a/kernel/kthread.c b/kernel/kthread.c > > > > index bfbfa48..b74ed8e 100644 > > > > --- a/kernel/kthread.c > > > > +++ b/kernel/kthread.c > > > > @@ -250,7 +250,7 @@ static int kthread(void *_create) > > > > current->vfork_done = &self->exited; > > > > > > > > /* OK, tell user we're spawned, wait for stop or wakeup */ > > > > - __set_current_state(TASK_UNINTERRUPTIBLE); > > > > + __set_current_state(TASK_IDLE); > > > > create->result = current; > > > > /* > > > > * Thread is going to call schedule(), do not preempt it, > > > > @@ -428,7 +428,7 @@ static void __kthread_bind(struct task_struct *p, unsigned int cpu, long state) > > > > > > > > void kthread_bind_mask(struct task_struct *p, const struct cpumask *mask) > > > > { > > > > - __kthread_bind_mask(p, mask, TASK_UNINTERRUPTIBLE); > > > > + __kthread_bind_mask(p, mask, TASK_IDLE); > > > > } > > > > > > > > /** > > > > @@ -442,7 +442,7 @@ void kthread_bind_mask(struct task_struct *p, const struct cpumask *mask) > > > > */ > > > > void kthread_bind(struct task_struct *p, unsigned int cpu) > > > > { > > > > - __kthread_bind(p, cpu, TASK_UNINTERRUPTIBLE); > > > > + __kthread_bind(p, cpu, TASK_IDLE); > > > > } > > > > EXPORT_SYMBOL(kthread_bind); > > > > > > It's as if people never read mailing lists: > > > https://lore.kernel.org/r/DM6PR11MB3531D3B164357B2DC476102DDFC90@DM6PR11MB3531.namprd11.prod.outlook.com > > > > > > Given that this is an identical resend of the previous patch, why are > > > you doing so, and what has changed since that original rejection? > > > > > I did not know that it is attempted before. Thanks for pointing to the > > previous discussion. > > > > We have seen hung task reports from customers and it is due to a downstream > > change which create bunch of kernel threads for later use. > > Do you have a pointer to that specific change? > Here it is https://source.codeaurora.org/quic/la/kernel/msm-4.19/commit/drivers/staging/android/ion/ion_system_heap.c?h=LA.UM.8.12.r1-11300-sm8250.0&id=0734b477f1e77cb9f91f5e5c0d7742d3113f2cd3 > > From Peter's reply, I understood that one must wake up the kthread > > after creation and put it in INTERRUPTIBLE sleep. I will pass on the > > message. > > Just go fix that code, it sounds like it's in your tree already :) > Yes, we will fix it. Thanks, Pavan -- Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] kthread: Use TASK_IDLE state for newly created kernel threads 2020-05-21 9:51 ` Pavan Kondeti @ 2020-05-21 12:03 ` Greg Kroah-Hartman 0 siblings, 0 replies; 6+ messages in thread From: Greg Kroah-Hartman @ 2020-05-21 12:03 UTC (permalink / raw) To: Pavan Kondeti Cc: linux-kernel, Vinayak Menon, Ben Dooks, Peter Zijlstra (Intel), Thomas Gleixner, Steven Rostedt (VMware), Liang Chen On Thu, May 21, 2020 at 03:21:25PM +0530, Pavan Kondeti wrote: > On Thu, May 21, 2020 at 07:56:39AM +0200, Greg Kroah-Hartman wrote: > > On Thu, May 21, 2020 at 07:05:44AM +0530, Pavan Kondeti wrote: > > > On Wed, May 20, 2020 at 08:18:58PM +0200, Greg Kroah-Hartman wrote: > > > > On Wed, May 20, 2020 at 05:25:09PM +0530, Pavankumar Kondeti wrote: > > > > > When kernel threads are created for later use, they will be in > > > > > TASK_UNINTERRUPTIBLE state until they are woken up. This results > > > > > in increased loadavg and false hung task reports. To fix this, > > > > > use TASK_IDLE state instead of TASK_UNINTERRUPTIBLE when > > > > > a kernel thread schedules out for the first time. > > > > > > > > > > Signed-off-by: Pavankumar Kondeti <pkondeti@codeaurora.org> > > > > > --- > > > > > kernel/kthread.c | 6 +++--- > > > > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > > > > > > > diff --git a/kernel/kthread.c b/kernel/kthread.c > > > > > index bfbfa48..b74ed8e 100644 > > > > > --- a/kernel/kthread.c > > > > > +++ b/kernel/kthread.c > > > > > @@ -250,7 +250,7 @@ static int kthread(void *_create) > > > > > current->vfork_done = &self->exited; > > > > > > > > > > /* OK, tell user we're spawned, wait for stop or wakeup */ > > > > > - __set_current_state(TASK_UNINTERRUPTIBLE); > > > > > + __set_current_state(TASK_IDLE); > > > > > create->result = current; > > > > > /* > > > > > * Thread is going to call schedule(), do not preempt it, > > > > > @@ -428,7 +428,7 @@ static void __kthread_bind(struct task_struct *p, unsigned int cpu, long state) > > > > > > > > > > void kthread_bind_mask(struct task_struct *p, const struct cpumask *mask) > > > > > { > > > > > - __kthread_bind_mask(p, mask, TASK_UNINTERRUPTIBLE); > > > > > + __kthread_bind_mask(p, mask, TASK_IDLE); > > > > > } > > > > > > > > > > /** > > > > > @@ -442,7 +442,7 @@ void kthread_bind_mask(struct task_struct *p, const struct cpumask *mask) > > > > > */ > > > > > void kthread_bind(struct task_struct *p, unsigned int cpu) > > > > > { > > > > > - __kthread_bind(p, cpu, TASK_UNINTERRUPTIBLE); > > > > > + __kthread_bind(p, cpu, TASK_IDLE); > > > > > } > > > > > EXPORT_SYMBOL(kthread_bind); > > > > > > > > It's as if people never read mailing lists: > > > > https://lore.kernel.org/r/DM6PR11MB3531D3B164357B2DC476102DDFC90@DM6PR11MB3531.namprd11.prod.outlook.com > > > > > > > > Given that this is an identical resend of the previous patch, why are > > > > you doing so, and what has changed since that original rejection? > > > > > > > I did not know that it is attempted before. Thanks for pointing to the > > > previous discussion. > > > > > > We have seen hung task reports from customers and it is due to a downstream > > > change which create bunch of kernel threads for later use. > > > > Do you have a pointer to that specific change? > > > Here it is > https://source.codeaurora.org/quic/la/kernel/msm-4.19/commit/drivers/staging/android/ion/ion_system_heap.c?h=LA.UM.8.12.r1-11300-sm8250.0&id=0734b477f1e77cb9f91f5e5c0d7742d3113f2cd3 Ick, ion system heaps :( Anyway, why not just use a workqueue for this, why a full kernel thread? What does that help with? It also might fix those crazy hacks you have in there for "cpu affinity" as all of that would be taken care of for you automatically with a workqueue, right? good luck! greg k-h ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-05-21 12:03 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-05-20 11:55 [PATCH] kthread: Use TASK_IDLE state for newly created kernel threads Pavankumar Kondeti 2020-05-20 18:18 ` Greg Kroah-Hartman 2020-05-21 1:35 ` Pavan Kondeti 2020-05-21 5:56 ` Greg Kroah-Hartman 2020-05-21 9:51 ` Pavan Kondeti 2020-05-21 12:03 ` Greg Kroah-Hartman
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).