Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1234930 > unrolled thread

[RFC][PATCH 00/11] sched: Killing PREEMPT_ACTIVE

Back to article view | Back to linux.kernel

Contents

  [RFC][PATCH 00/11] sched: Killing PREEMPT_ACTIVE Peter Zijlstra <peterz@infradead.org> - 2015-09-29 11:50 +0200
    [RFC][PATCH 12/11] sched: Add preempt_count invariant check Peter Zijlstra <peterz@infradead.org> - 2015-09-29 12:40 +0200
      Re: [RFC][PATCH 12/11] sched: Add preempt_count invariant check Peter Zijlstra <peterz@infradead.org> - 2015-09-29 12:40 +0200
      [RFC][PATCH v2 12/11] sched: Add preempt_count invariant check Peter Zijlstra <peterz@infradead.org> - 2015-09-29 13:10 +0200
        Re: [RFC][PATCH v2 12/11] sched: Add preempt_count invariant check Linus Torvalds <torvalds@linux-foundation.org> - 2015-09-29 14:00 +0200
          Re: [RFC][PATCH v2 12/11] sched: Add preempt_count invariant check Peter Zijlstra <peterz@infradead.org> - 2015-09-29 14:10 +0200
    Re: [RFC][PATCH 00/11] sched: Killing PREEMPT_ACTIVE Ingo Molnar <mingo@kernel.org> - 2015-09-29 12:40 +0200

#1234930 — [RFC][PATCH 00/11] sched: Killing PREEMPT_ACTIVE

This series kills PREEMPT_ACTIVE dead, its a tad risky, but survives light
testing on x86_64 and seems to compile on everything else.

---
 arch/x86/include/asm/preempt.h     |  5 +--
 arch/x86/include/asm/thread_info.h |  2 --
 arch/x86/kernel/process_32.c       |  8 -----
 arch/x86/kernel/process_64.c       |  8 -----
 include/asm-generic/preempt.h      |  2 +-
 include/linux/preempt.h            | 20 +----------
 include/linux/sched.h              |  7 ++--
 include/trace/events/sched.h       | 22 +++++-------
 kernel/exit.c                      |  4 ++-
 kernel/sched/core.c                | 70 +++++++++++++++++++++-----------------
 kernel/trace/ftrace.c              |  2 +-
 kernel/trace/trace_sched_switch.c  |  3 +-
 kernel/trace/trace_sched_wakeup.c  |  2 +-
 13 files changed, 60 insertions(+), 95 deletions(-)

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1234949 — [RFC][PATCH 12/11] sched: Add preempt_count invariant check

Ingo requested I keep my debug check for the preempt_count invariant.

Requested-by: Ingo Molnar <mingo@kernel.org>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 kernel/sched/core.c |    5 +++++
 1 file changed, 5 insertions(+)

--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -2503,6 +2503,11 @@ static struct rq *finish_task_switch(str
 	struct mm_struct *mm = rq->prev_mm;
 	long prev_state;
 
+	if (unlikely(WARN_ONCE(preempt_count() != 2,
+			       "corrupted preempt_count: %s/%d/0x%x\n",
+			       current->comm, current->pid, preempt_count())))
+		preempt_count_set(INIT_PREEMPT_COUNT);
+
 	rq->prev_mm = NULL;
 
 	/*
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1234955 — Re: [RFC][PATCH 12/11] sched: Add preempt_count invariant check

On Tue, Sep 29, 2015 at 12:27:55PM +0200, Peter Zijlstra wrote:
> 
> Ingo requested I keep my debug check for the preempt_count invariant.
> 
> Requested-by: Ingo Molnar <mingo@kernel.org>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> ---
>  kernel/sched/core.c |    5 +++++
>  1 file changed, 5 insertions(+)
> 
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -2503,6 +2503,11 @@ static struct rq *finish_task_switch(str
>  	struct mm_struct *mm = rq->prev_mm;
>  	long prev_state;
>  
> +	if (unlikely(WARN_ONCE(preempt_count() != 2,

That should have: s/2/2*PREEMPT_DISABLE_OFFSET/ or something, otherwise
!PREEMPT_COUNT kernels will go funny.

> +			       "corrupted preempt_count: %s/%d/0x%x\n",
> +			       current->comm, current->pid, preempt_count())))
> +		preempt_count_set(INIT_PREEMPT_COUNT);
> +
>  	rq->prev_mm = NULL;
>  
>  	/*
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1234973 — [RFC][PATCH v2 12/11] sched: Add preempt_count invariant check

Ingo requested I keep my debug check for the preempt_count invariant.

Requested-by: Ingo Molnar <mingo@kernel.org>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 include/asm-generic/preempt.h |    2 +-
 include/linux/sched.h         |   17 +++++++++--------
 kernel/sched/core.c           |    5 +++++
 3 files changed, 15 insertions(+), 9 deletions(-)

--- a/include/asm-generic/preempt.h
+++ b/include/asm-generic/preempt.h
@@ -24,7 +24,7 @@ static __always_inline void preempt_coun
  * must be macros to avoid header recursion hell
  */
 #define init_task_preempt_count(p) do { \
-	task_thread_info(p)->preempt_count = 2*PREEMPT_DISABLED; \
+	task_thread_info(p)->preempt_count = INIT_PREEMPT_COUNT; \
 } while (0)
 
 #define init_idle_preempt_count(p, cpu) do { \
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -599,17 +599,18 @@ struct task_cputime_atomic {
 		.sum_exec_runtime = ATOMIC64_INIT(0),		\
 	}
 
-#ifdef CONFIG_PREEMPT_COUNT
-#define PREEMPT_DISABLED	(1 + PREEMPT_ENABLED)
-#else
-#define PREEMPT_DISABLED	PREEMPT_ENABLED
-#endif
+#define PREEMPT_DISABLED	(PREEMPT_DISABLE_OFFSET + PREEMPT_ENABLED)
 
 /*
- * Disable preemption until the scheduler is running.
- * Reset by start_kernel()->sched_init()->init_idle().
+ * Initial preempt_count value; reflects the preempt_count schedule invariant
+ * which states that during schedule preempt_count() == 2.
+ *
+ * This also results in the kernel starting with preemption disabled until
+ * the scheduler is initialized, see:
+ *
+ *   start_kernel()->sched_init()->init_idle().
  */
-#define INIT_PREEMPT_COUNT	PREEMPT_DISABLED
+#define INIT_PREEMPT_COUNT	(2*PREEMPT_DISABLE_OFFSET + PREEMPT_ENABLED)
 
 /**
  * struct thread_group_cputimer - thread group interval timer counts
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -2503,6 +2503,11 @@ static struct rq *finish_task_switch(str
 	struct mm_struct *mm = rq->prev_mm;
 	long prev_state;
 
+	if (unlikely(WARN_ONCE(preempt_count() != 2*PREEMPT_DISABLE_OFFSET,
+			       "corrupted preempt_count: %s/%d/0x%x\n",
+			       current->comm, current->pid, preempt_count())))
+		preempt_count_set(INIT_PREEMPT_COUNT);
+
 	rq->prev_mm = NULL;
 
 	/*
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1234997 — Re: [RFC][PATCH v2 12/11] sched: Add preempt_count invariant check

On Tue, Sep 29, 2015 at 6:56 AM, Peter Zijlstra <peterz@infradead.org> wrote:
>
> + * Initial preempt_count value; reflects the preempt_count schedule invariant
> + * which states that during schedule preempt_count() == 2.

Is this actually *true*?

preempt_count() is two only if preemption is enabled. But spinlocks do
not actually update the preemption count if there is no preemption, so
these kinds of checks and comments that aren't even inside #ifdef
CONFIG_PREEMPT seem to be actively misleading.

I do believe that the preempt count is stable - but the actual value
would seem to depend on config options.

            Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1235008 — Re: [RFC][PATCH v2 12/11] sched: Add preempt_count invariant check

On Tue, Sep 29, 2015 at 07:55:49AM -0400, Linus Torvalds wrote:
> On Tue, Sep 29, 2015 at 6:56 AM, Peter Zijlstra <peterz@infradead.org> wrote:
> >
> > + * Initial preempt_count value; reflects the preempt_count schedule invariant
> > + * which states that during schedule preempt_count() == 2.
> 
> Is this actually *true*?
> 
> preempt_count() is two only if preemption is enabled. But spinlocks do
> not actually update the preemption count if there is no preemption, so
> these kinds of checks and comments that aren't even inside #ifdef
> CONFIG_PREEMPT seem to be actively misleading.
> 
> I do believe that the preempt count is stable - but the actual value
> would seem to depend on config options.

Right, 2*PREEMPT_DISABLE_OFFSET is the 'right' number. That ends up
being 0 for !PREEMPT_COUNT configs.

I'll update the Changelog to be more accurate on this.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1234960

* Peter Zijlstra <peterz@infradead.org> wrote:

> This series kills PREEMPT_ACTIVE dead, its a tad risky, but survives light 
> testing on x86_64 and seems to compile on everything else.

I've applied the first three patches because they look low risk.

Will wait a bit with the rest.

Thanks,

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web