| Message ID | 20150804163334.GA31842@redhat.com |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
On Tue, Aug 04, 2015 at 06:33:34PM +0200, Oleg Nesterov wrote: > Commit 1fbe4b46caca "net: pktgen: kill the Wait for kthread_stop > code in pktgen_thread_worker()" removed (in particular) the final > __set_current_state(TASK_RUNNING) and I didn't notice the previous > set_current_state(TASK_INTERRUPTIBLE). This triggers the warning > in __might_sleep() after return. > > Afaics, we can simply remove both set_current_state()'s, and we > could do this a long ago right after ef87979c273a2 "pktgen: better > scheduler friendliness" which changed pktgen_thread_worker() to > use wait_event_interruptible_timeout(). > > Reported-by: Huang Ying <ying.huang@intel.com> > Signed-off-by: Oleg Nesterov <oleg@redhat.com> Interesting that it didn't happen in my tests yet per description it should have. Huang, did you do anything special to trigger this? -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 08/04, Marcelo Ricardo Leitner wrote: > > On Tue, Aug 04, 2015 at 06:33:34PM +0200, Oleg Nesterov wrote: > > Commit 1fbe4b46caca "net: pktgen: kill the Wait for kthread_stop > > code in pktgen_thread_worker()" removed (in particular) the final > > __set_current_state(TASK_RUNNING) and I didn't notice the previous > > set_current_state(TASK_INTERRUPTIBLE). This triggers the warning > > in __might_sleep() after return. > > > > Afaics, we can simply remove both set_current_state()'s, and we > > could do this a long ago right after ef87979c273a2 "pktgen: better > > scheduler friendliness" which changed pktgen_thread_worker() to > > use wait_event_interruptible_timeout(). > > > > Reported-by: Huang Ying <ying.huang@intel.com> > > Signed-off-by: Oleg Nesterov <oleg@redhat.com> > > Interesting that it didn't happen in my tests yet per description it > should have. Huang, did you do anything special to trigger this? Do you have CONFIG_DEBUG_ATOMIC_SLEEP=y ? Oleg. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Aug 04, 2015 at 07:08:27PM +0200, Oleg Nesterov wrote: > On 08/04, Marcelo Ricardo Leitner wrote: > > > > On Tue, Aug 04, 2015 at 06:33:34PM +0200, Oleg Nesterov wrote: > > > Commit 1fbe4b46caca "net: pktgen: kill the Wait for kthread_stop > > > code in pktgen_thread_worker()" removed (in particular) the final > > > __set_current_state(TASK_RUNNING) and I didn't notice the previous > > > set_current_state(TASK_INTERRUPTIBLE). This triggers the warning > > > in __might_sleep() after return. > > > > > > Afaics, we can simply remove both set_current_state()'s, and we > > > could do this a long ago right after ef87979c273a2 "pktgen: better > > > scheduler friendliness" which changed pktgen_thread_worker() to > > > use wait_event_interruptible_timeout(). > > > > > > Reported-by: Huang Ying <ying.huang@intel.com> > > > Signed-off-by: Oleg Nesterov <oleg@redhat.com> > > > > Interesting that it didn't happen in my tests yet per description it > > should have. Huang, did you do anything special to trigger this? > > Do you have CONFIG_DEBUG_ATOMIC_SLEEP=y ? Ahh yes, probably not. Thanks. Marcelo -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Oleg Nesterov <oleg@redhat.com> Date: Tue, 4 Aug 2015 18:33:34 +0200 > Commit 1fbe4b46caca "net: pktgen: kill the Wait for kthread_stop > code in pktgen_thread_worker()" removed (in particular) the final > __set_current_state(TASK_RUNNING) and I didn't notice the previous > set_current_state(TASK_INTERRUPTIBLE). This triggers the warning > in __might_sleep() after return. > > Afaics, we can simply remove both set_current_state()'s, and we > could do this a long ago right after ef87979c273a2 "pktgen: better > scheduler friendliness" which changed pktgen_thread_worker() to > use wait_event_interruptible_timeout(). > > Reported-by: Huang Ying <ying.huang@intel.com> > Signed-off-by: Oleg Nesterov <oleg@redhat.com> Applied, thank you. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/core/pktgen.c b/net/core/pktgen.c index 8e0181a..6717684 100644 --- a/net/core/pktgen.c +++ b/net/core/pktgen.c @@ -3432,8 +3432,6 @@ static int pktgen_thread_worker(void *arg) set_freezable(); - __set_current_state(TASK_RUNNING); - while (!kthread_should_stop()) { pkt_dev = next_to_run(t); @@ -3478,7 +3476,6 @@ static int pktgen_thread_worker(void *arg) try_to_freeze(); } - set_current_state(TASK_INTERRUPTIBLE); pr_debug("%s stopping all device\n", t->tsk->comm); pktgen_stop(t);
Commit 1fbe4b46caca "net: pktgen: kill the Wait for kthread_stop code in pktgen_thread_worker()" removed (in particular) the final __set_current_state(TASK_RUNNING) and I didn't notice the previous set_current_state(TASK_INTERRUPTIBLE). This triggers the warning in __might_sleep() after return. Afaics, we can simply remove both set_current_state()'s, and we could do this a long ago right after ef87979c273a2 "pktgen: better scheduler friendliness" which changed pktgen_thread_worker() to use wait_event_interruptible_timeout(). Reported-by: Huang Ying <ying.huang@intel.com> Signed-off-by: Oleg Nesterov <oleg@redhat.com> --- net/core/pktgen.c | 3 --- 1 files changed, 0 insertions(+), 3 deletions(-)