All of lore.kernel.org
 help / color / mirror / Atom feed
* sched: Remove pointless preemption disable in sched_submit_work()
@ 2021-09-28 14:30 Thomas Gleixner
  2021-09-28 14:41 ` Jens Axboe
  2021-09-29  1:26 ` Lai Jiangshan
  0 siblings, 2 replies; 6+ messages in thread
From: Thomas Gleixner @ 2021-09-28 14:30 UTC (permalink / raw)
  To: LKML; +Cc: Peter Zijlstra, Ingo Molnar, Peter Oskolkov, Tejun Heo, Jens Axboe

Neither wq_worker_sleeping() nor io_wq_worker_sleeping() require to be invoked
with preemption disabled:

  - The worker flag checks operations only need to be serialized against
    the worker thread itself.

  - The accounting and worker pool operations are serialized with locks.

which means that disabling preemption has neither a reason nor a
value. Remove it.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 kernel/sched/core.c |    2 --
 1 file changed, 2 deletions(-)

--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -6327,12 +6327,10 @@ static inline void sched_submit_work(str
 	 * requires it.
 	 */
 	if (task_flags & (PF_WQ_WORKER | PF_IO_WORKER)) {
-		preempt_disable();
 		if (task_flags & PF_WQ_WORKER)
 			wq_worker_sleeping(tsk);
 		else
 			io_wq_worker_sleeping(tsk);
-		preempt_enable_no_resched();
 	}
 
 	if (tsk_is_pi_blocked(tsk))

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: sched: Remove pointless preemption disable in sched_submit_work()
  2021-09-28 14:30 sched: Remove pointless preemption disable in sched_submit_work() Thomas Gleixner
@ 2021-09-28 14:41 ` Jens Axboe
  2021-09-29  1:26 ` Lai Jiangshan
  1 sibling, 0 replies; 6+ messages in thread
From: Jens Axboe @ 2021-09-28 14:41 UTC (permalink / raw)
  To: Thomas Gleixner, LKML
  Cc: Peter Zijlstra, Ingo Molnar, Peter Oskolkov, Tejun Heo

On 9/28/21 8:30 AM, Thomas Gleixner wrote:
> Neither wq_worker_sleeping() nor io_wq_worker_sleeping() require to be invoked
> with preemption disabled:
> 
>   - The worker flag checks operations only need to be serialized against
>     the worker thread itself.
> 
>   - The accounting and worker pool operations are serialized with locks.
> 
> which means that disabling preemption has neither a reason nor a
> value. Remove it.

Reviewed-by: Jens Axboe <axboe@kernel.dk>

-- 
Jens Axboe


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: sched: Remove pointless preemption disable in sched_submit_work()
  2021-09-28 14:30 sched: Remove pointless preemption disable in sched_submit_work() Thomas Gleixner
  2021-09-28 14:41 ` Jens Axboe
@ 2021-09-29  1:26 ` Lai Jiangshan
  2021-09-29  9:37   ` [PATCH v2] " Thomas Gleixner
  1 sibling, 1 reply; 6+ messages in thread
From: Lai Jiangshan @ 2021-09-29  1:26 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: LKML, Peter Zijlstra, Ingo Molnar, Peter Oskolkov, Tejun Heo, Jens Axboe

On Tue, Sep 28, 2021 at 10:31 PM Thomas Gleixner <tglx@linutronix.de> wrote:
>
> Neither wq_worker_sleeping() nor io_wq_worker_sleeping() require to be invoked
> with preemption disabled:
>
>   - The worker flag checks operations only need to be serialized against
>     the worker thread itself.
>
>   - The accounting and worker pool operations are serialized with locks.
>
> which means that disabling preemption has neither a reason nor a
> value. Remove it.
>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> ---
>  kernel/sched/core.c |    2 --
>  1 file changed, 2 deletions(-)
>
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -6327,12 +6327,10 @@ static inline void sched_submit_work(str
>          * requires it.
>          */
>         if (task_flags & (PF_WQ_WORKER | PF_IO_WORKER)) {

Hello

The comment about preemption above this if branch and wq_worker_sleeping()
needs to be removed.

Thanks
Lai

Reviewed-by: Lai Jiangshan <jiangshanlai@gmail.com>

> -               preempt_disable();
>                 if (task_flags & PF_WQ_WORKER)
>                         wq_worker_sleeping(tsk);
>                 else
>                         io_wq_worker_sleeping(tsk);
> -               preempt_enable_no_resched();
>         }
>
>         if (tsk_is_pi_blocked(tsk))

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2] sched: Remove pointless preemption disable in sched_submit_work()
  2021-09-29  1:26 ` Lai Jiangshan
@ 2021-09-29  9:37   ` Thomas Gleixner
  2021-10-01 15:05     ` [tip: sched/core] " tip-bot2 for Thomas Gleixner
  2021-10-05 14:11     ` tip-bot2 for Thomas Gleixner
  0 siblings, 2 replies; 6+ messages in thread
From: Thomas Gleixner @ 2021-09-29  9:37 UTC (permalink / raw)
  To: Lai Jiangshan
  Cc: LKML, Peter Zijlstra, Ingo Molnar, Peter Oskolkov, Tejun Heo, Jens Axboe

Neither wq_worker_sleeping() nor io_wq_worker_sleeping() require to be invoked
with preemption disabled:

  - The worker flag checks operations only need to be serialized against
    the worker thread itself.

  - The accounting and worker pool operations are serialized with locks.

which means that disabling preemption has neither a reason nor a
value. Remove it and update the stale comment.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Reviewed-by: Lai Jiangshan <jiangshanlai@gmail.com>
Reviewed-by: Jens Axboe <axboe@kernel.dk>
---
V2: Update comment as pointed out by Lai
---
 kernel/sched/core.c |   13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -6319,20 +6319,14 @@ static inline void sched_submit_work(str
 
 	task_flags = tsk->flags;
 	/*
-	 * If a worker went to sleep, notify and ask workqueue whether
-	 * it wants to wake up a task to maintain concurrency.
-	 * As this function is called inside the schedule() context,
-	 * we disable preemption to avoid it calling schedule() again
-	 * in the possible wakeup of a kworker and because wq_worker_sleeping()
-	 * requires it.
+	 * If a worker goes to sleep, notify and ask workqueue whether it
+	 * wants to wake up a task to maintain concurrency.
 	 */
 	if (task_flags & (PF_WQ_WORKER | PF_IO_WORKER)) {
-		preempt_disable();
 		if (task_flags & PF_WQ_WORKER)
 			wq_worker_sleeping(tsk);
 		else
 			io_wq_worker_sleeping(tsk);
-		preempt_enable_no_resched();
 	}
 
 	if (tsk_is_pi_blocked(tsk))

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [tip: sched/core] sched: Remove pointless preemption disable in sched_submit_work()
  2021-09-29  9:37   ` [PATCH v2] " Thomas Gleixner
@ 2021-10-01 15:05     ` tip-bot2 for Thomas Gleixner
  2021-10-05 14:11     ` tip-bot2 for Thomas Gleixner
  1 sibling, 0 replies; 6+ messages in thread
From: tip-bot2 for Thomas Gleixner @ 2021-10-01 15:05 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: Thomas Gleixner, Peter Zijlstra (Intel),
	Lai Jiangshan, Jens Axboe, x86, linux-kernel

The following commit has been merged into the sched/core branch of tip:

Commit-ID:     579df3f2325191322b96906a5898462253a45ede
Gitweb:        https://git.kernel.org/tip/579df3f2325191322b96906a5898462253a45ede
Author:        Thomas Gleixner <tglx@linutronix.de>
AuthorDate:    Wed, 29 Sep 2021 11:37:32 +02:00
Committer:     Peter Zijlstra <peterz@infradead.org>
CommitterDate: Fri, 01 Oct 2021 13:58:07 +02:00

sched: Remove pointless preemption disable in sched_submit_work()

Neither wq_worker_sleeping() nor io_wq_worker_sleeping() require to be invoked
with preemption disabled:

  - The worker flag checks operations only need to be serialized against
    the worker thread itself.

  - The accounting and worker pool operations are serialized with locks.

which means that disabling preemption has neither a reason nor a
value. Remove it and update the stale comment.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: Lai Jiangshan <jiangshanlai@gmail.com>
Reviewed-by: Jens Axboe <axboe@kernel.dk>
Link: https://lkml.kernel.org/r/8735pnafj7.ffs@tglx
---
 kernel/sched/core.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 8e49b17..d469c7b 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -6097,20 +6097,14 @@ static inline void sched_submit_work(struct task_struct *tsk)
 
 	task_flags = tsk->flags;
 	/*
-	 * If a worker went to sleep, notify and ask workqueue whether
-	 * it wants to wake up a task to maintain concurrency.
-	 * As this function is called inside the schedule() context,
-	 * we disable preemption to avoid it calling schedule() again
-	 * in the possible wakeup of a kworker and because wq_worker_sleeping()
-	 * requires it.
+	 * If a worker goes to sleep, notify and ask workqueue whether it
+	 * wants to wake up a task to maintain concurrency.
 	 */
 	if (task_flags & (PF_WQ_WORKER | PF_IO_WORKER)) {
-		preempt_disable();
 		if (task_flags & PF_WQ_WORKER)
 			wq_worker_sleeping(tsk);
 		else
 			io_wq_worker_sleeping(tsk);
-		preempt_enable_no_resched();
 	}
 
 	if (tsk_is_pi_blocked(tsk))

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [tip: sched/core] sched: Remove pointless preemption disable in sched_submit_work()
  2021-09-29  9:37   ` [PATCH v2] " Thomas Gleixner
  2021-10-01 15:05     ` [tip: sched/core] " tip-bot2 for Thomas Gleixner
@ 2021-10-05 14:11     ` tip-bot2 for Thomas Gleixner
  1 sibling, 0 replies; 6+ messages in thread
From: tip-bot2 for Thomas Gleixner @ 2021-10-05 14:11 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: Thomas Gleixner, Peter Zijlstra (Intel),
	Lai Jiangshan, Jens Axboe, x86, linux-kernel

The following commit has been merged into the sched/core branch of tip:

Commit-ID:     b945efcdd07d86cece1cce68503aae91f107eacb
Gitweb:        https://git.kernel.org/tip/b945efcdd07d86cece1cce68503aae91f107eacb
Author:        Thomas Gleixner <tglx@linutronix.de>
AuthorDate:    Wed, 29 Sep 2021 11:37:32 +02:00
Committer:     Peter Zijlstra <peterz@infradead.org>
CommitterDate: Tue, 05 Oct 2021 15:52:15 +02:00

sched: Remove pointless preemption disable in sched_submit_work()

Neither wq_worker_sleeping() nor io_wq_worker_sleeping() require to be invoked
with preemption disabled:

  - The worker flag checks operations only need to be serialized against
    the worker thread itself.

  - The accounting and worker pool operations are serialized with locks.

which means that disabling preemption has neither a reason nor a
value. Remove it and update the stale comment.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: Lai Jiangshan <jiangshanlai@gmail.com>
Reviewed-by: Jens Axboe <axboe@kernel.dk>
Link: https://lkml.kernel.org/r/8735pnafj7.ffs@tglx
---
 kernel/sched/core.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index e33b03c..e47d7e5 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -6251,20 +6251,14 @@ static inline void sched_submit_work(struct task_struct *tsk)
 
 	task_flags = tsk->flags;
 	/*
-	 * If a worker went to sleep, notify and ask workqueue whether
-	 * it wants to wake up a task to maintain concurrency.
-	 * As this function is called inside the schedule() context,
-	 * we disable preemption to avoid it calling schedule() again
-	 * in the possible wakeup of a kworker and because wq_worker_sleeping()
-	 * requires it.
+	 * If a worker goes to sleep, notify and ask workqueue whether it
+	 * wants to wake up a task to maintain concurrency.
 	 */
 	if (task_flags & (PF_WQ_WORKER | PF_IO_WORKER)) {
-		preempt_disable();
 		if (task_flags & PF_WQ_WORKER)
 			wq_worker_sleeping(tsk);
 		else
 			io_wq_worker_sleeping(tsk);
-		preempt_enable_no_resched();
 	}
 
 	if (tsk_is_pi_blocked(tsk))

^ permalink raw reply related	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-10-05 14:12 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-28 14:30 sched: Remove pointless preemption disable in sched_submit_work() Thomas Gleixner
2021-09-28 14:41 ` Jens Axboe
2021-09-29  1:26 ` Lai Jiangshan
2021-09-29  9:37   ` [PATCH v2] " Thomas Gleixner
2021-10-01 15:05     ` [tip: sched/core] " tip-bot2 for Thomas Gleixner
2021-10-05 14:11     ` tip-bot2 for Thomas Gleixner

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.