LKML Archive on lore.kernel.org
 help / Atom feed
* [PATCH] ARM: hide finish_arch_post_lock_switch from modules
@ 2016-05-13 13:30 Arnd Bergmann
  2016-05-13 13:51 ` Guenter Roeck
  2016-05-13 14:15 ` [tip:sched/core] ARM: Hide finish_arch_post_lock_switch() " tip-bot for Steven Rostedt
  0 siblings, 2 replies; 3+ messages in thread
From: Arnd Bergmann @ 2016-05-13 13:30 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Andy Lutomirski, Steven Rostedt, Russell King - ARM Linux,
	Guenter Roeck, Ard Biesheuvel, Peter Zijlstra,
	Frederic Weisbecker, linux-kernel, Andy Lutomirski,
	Thomas Gleixner, Borislav Petkov, Mel Gorman, linux-arm-kernel,
	Arnd Bergmann

From: Steven Rostedt <rostedt@goodmis.org>

The introduction of switch_mm_irqs_off() brought back an old bug
regarding the use of preempt_enable_no_resched:

As part of 62b94a08da1b ("sched/preempt: Take away
preempt_enable_no_resched() from modules"), the definition of
preempt_enable_no_resched() is only available in built-in code,
not in loadable modules, so we can't generally use it from header
files. However, the ARM version of finish_arch_post_lock_switch()
calls preempt_enable_no_resched() and is defined as a static
inline function in asm/mmu_context.h. This in turn means we cannot
include asm/mmu_context.h from modules.

With today's tip tree, asm/mmu_context.h gets included from
linux/mmu_context.h, which is normally the exact pattern one would
expect, but unfortunately, linux/mmu_context.h can be included from
the vhost driver that is a loadable module, now causing this compile
time error:

In file included from ../include/linux/mmu_context.h:4:0,
                 from ../drivers/vhost/vhost.c:18:
../arch/arm/include/asm/mmu_context.h: In function 'finish_arch_post_lock_switch':
../arch/arm/include/asm/mmu_context.h:88:3: error: implicit declaration of function 'preempt_enable_no_resched' [-Werror=implicit-function-declaration]
   preempt_enable_no_resched();

Andy already tried to fix the bug by including linux/preempt.h
from asm/mmu_context.h, but that didn't help. I suggested reordering
the header files, which wasn't popular, but Steve came up with this
workaround instead:

The finish_arch_post_lock_switch() definition is now also hidden
inside of #ifdef MODULE, so we don't see anything referencing
preempt_enable_no_resched() from a header file. I've built a
few hundred randconfig kernels with this, and did not see any
new problems.

Fixes: f98db6013c55 ("sched/core: Add switch_mm_irqs_off() and use it in the scheduler")
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Acked-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 arch/arm/include/asm/mmu_context.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/include/asm/mmu_context.h b/arch/arm/include/asm/mmu_context.h
index ed73babc0dc9..3cc14dd8587c 100644
--- a/arch/arm/include/asm/mmu_context.h
+++ b/arch/arm/include/asm/mmu_context.h
@@ -67,6 +67,7 @@ static inline void check_and_switch_context(struct mm_struct *mm,
 		cpu_switch_mm(mm->pgd, mm);
 }
 
+#ifndef MODULE
 #define finish_arch_post_lock_switch \
 	finish_arch_post_lock_switch
 static inline void finish_arch_post_lock_switch(void)
@@ -88,6 +89,7 @@ static inline void finish_arch_post_lock_switch(void)
 		preempt_enable_no_resched();
 	}
 }
+#endif /* !MODULE */
 
 #endif	/* CONFIG_MMU */
 
-- 
2.7.0

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

* Re: [PATCH] ARM: hide finish_arch_post_lock_switch from modules
  2016-05-13 13:30 [PATCH] ARM: hide finish_arch_post_lock_switch from modules Arnd Bergmann
@ 2016-05-13 13:51 ` Guenter Roeck
  2016-05-13 14:15 ` [tip:sched/core] ARM: Hide finish_arch_post_lock_switch() " tip-bot for Steven Rostedt
  1 sibling, 0 replies; 3+ messages in thread
From: Guenter Roeck @ 2016-05-13 13:51 UTC (permalink / raw)
  To: Arnd Bergmann, Ingo Molnar
  Cc: Andy Lutomirski, Steven Rostedt, Russell King - ARM Linux,
	Ard Biesheuvel, Peter Zijlstra, Frederic Weisbecker,
	linux-kernel, Andy Lutomirski, Thomas Gleixner, Borislav Petkov,
	Mel Gorman, linux-arm-kernel

On 05/13/2016 06:30 AM, Arnd Bergmann wrote:
> From: Steven Rostedt <rostedt@goodmis.org>
>
> The introduction of switch_mm_irqs_off() brought back an old bug
> regarding the use of preempt_enable_no_resched:
>
> As part of 62b94a08da1b ("sched/preempt: Take away
> preempt_enable_no_resched() from modules"), the definition of
> preempt_enable_no_resched() is only available in built-in code,
> not in loadable modules, so we can't generally use it from header
> files. However, the ARM version of finish_arch_post_lock_switch()
> calls preempt_enable_no_resched() and is defined as a static
> inline function in asm/mmu_context.h. This in turn means we cannot
> include asm/mmu_context.h from modules.
>
> With today's tip tree, asm/mmu_context.h gets included from
> linux/mmu_context.h, which is normally the exact pattern one would
> expect, but unfortunately, linux/mmu_context.h can be included from
> the vhost driver that is a loadable module, now causing this compile
> time error:
>
> In file included from ../include/linux/mmu_context.h:4:0,
>                   from ../drivers/vhost/vhost.c:18:
> ../arch/arm/include/asm/mmu_context.h: In function 'finish_arch_post_lock_switch':
> ../arch/arm/include/asm/mmu_context.h:88:3: error: implicit declaration of function 'preempt_enable_no_resched' [-Werror=implicit-function-declaration]
>     preempt_enable_no_resched();
>
> Andy already tried to fix the bug by including linux/preempt.h
> from asm/mmu_context.h, but that didn't help. I suggested reordering
> the header files, which wasn't popular, but Steve came up with this
> workaround instead:
>
> The finish_arch_post_lock_switch() definition is now also hidden
> inside of #ifdef MODULE, so we don't see anything referencing
> preempt_enable_no_resched() from a header file. I've built a
> few hundred randconfig kernels with this, and did not see any
> new problems.
>
> Fixes: f98db6013c55 ("sched/core: Add switch_mm_irqs_off() and use it in the scheduler")
> Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Acked-by: Russell King <rmk+kernel@arm.linux.org.uk>

Tested-by: Guenter Roeck <linux@roeck-us.net>

> ---
>   arch/arm/include/asm/mmu_context.h | 2 ++
>   1 file changed, 2 insertions(+)
>
> diff --git a/arch/arm/include/asm/mmu_context.h b/arch/arm/include/asm/mmu_context.h
> index ed73babc0dc9..3cc14dd8587c 100644
> --- a/arch/arm/include/asm/mmu_context.h
> +++ b/arch/arm/include/asm/mmu_context.h
> @@ -67,6 +67,7 @@ static inline void check_and_switch_context(struct mm_struct *mm,
>   		cpu_switch_mm(mm->pgd, mm);
>   }
>
> +#ifndef MODULE
>   #define finish_arch_post_lock_switch \
>   	finish_arch_post_lock_switch
>   static inline void finish_arch_post_lock_switch(void)
> @@ -88,6 +89,7 @@ static inline void finish_arch_post_lock_switch(void)
>   		preempt_enable_no_resched();
>   	}
>   }
> +#endif /* !MODULE */
>
>   #endif	/* CONFIG_MMU */
>
>

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

* [tip:sched/core] ARM: Hide finish_arch_post_lock_switch() from modules
  2016-05-13 13:30 [PATCH] ARM: hide finish_arch_post_lock_switch from modules Arnd Bergmann
  2016-05-13 13:51 ` Guenter Roeck
@ 2016-05-13 14:15 ` " tip-bot for Steven Rostedt
  1 sibling, 0 replies; 3+ messages in thread
From: tip-bot for Steven Rostedt @ 2016-05-13 14:15 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: torvalds, eranian, acme, tglx, alexander.shishkin, bp,
	ard.biesheuvel, rostedt, hpa, mingo, jolsa, arnd, linux, luto,
	linux, vincent.weaver, rmk+kernel, fweisbec, luto, mgorman,
	linux-kernel, peterz

Commit-ID:  ef0491ea17f8019821c7e9c8e801184ecf17f85a
Gitweb:     http://git.kernel.org/tip/ef0491ea17f8019821c7e9c8e801184ecf17f85a
Author:     Steven Rostedt <rostedt@goodmis.org>
AuthorDate: Fri, 13 May 2016 15:30:13 +0200
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Fri, 13 May 2016 16:12:53 +0200

ARM: Hide finish_arch_post_lock_switch() from modules

The introduction of switch_mm_irqs_off() brought back an old bug
regarding the use of preempt_enable_no_resched:

As part of:

  62b94a08da1b ("sched/preempt: Take away preempt_enable_no_resched() from modules")

the definition of preempt_enable_no_resched() is only available in
built-in code, not in loadable modules, so we can't generally use
it from header files.

However, the ARM version of finish_arch_post_lock_switch()
calls preempt_enable_no_resched() and is defined as a static
inline function in asm/mmu_context.h. This in turn means we cannot
include asm/mmu_context.h from modules.

With today's tip tree, asm/mmu_context.h gets included from
linux/mmu_context.h, which is normally the exact pattern one would
expect, but unfortunately, linux/mmu_context.h can be included from
the vhost driver that is a loadable module, now causing this compile
time error with modular configs:

  In file included from ../include/linux/mmu_context.h:4:0,
                   from ../drivers/vhost/vhost.c:18:
  ../arch/arm/include/asm/mmu_context.h: In function 'finish_arch_post_lock_switch':
  ../arch/arm/include/asm/mmu_context.h:88:3: error: implicit declaration of function 'preempt_enable_no_resched' [-Werror=implicit-function-declaration]
     preempt_enable_no_resched();

Andy already tried to fix the bug by including linux/preempt.h
from asm/mmu_context.h, but that didn't help. Arnd suggested reordering
the header files, which wasn't popular, so let's use this
workaround instead:

The finish_arch_post_lock_switch() definition is now also hidden
inside of #ifdef MODULE, so we don't see anything referencing
preempt_enable_no_resched() from a header file. I've built a
few hundred randconfig kernels with this, and did not see any
new problems.

Tested-by: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Acked-by: Russell King <rmk+kernel@arm.linux.org.uk>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Borislav Petkov <bp@suse.de>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Mel Gorman <mgorman@techsingularity.net>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Russell King - ARM Linux <linux@armlinux.org.uk>
Cc: Stephane Eranian <eranian@google.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Vince Weaver <vincent.weaver@maine.edu>
Cc: linux-arm-kernel@lists.infradead.org
Fixes: f98db6013c55 ("sched/core: Add switch_mm_irqs_off() and use it in the scheduler")
Link: http://lkml.kernel.org/r/1463146234-161304-1-git-send-email-arnd@arndb.de
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/arm/include/asm/mmu_context.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/include/asm/mmu_context.h b/arch/arm/include/asm/mmu_context.h
index ed73bab..3cc14dd 100644
--- a/arch/arm/include/asm/mmu_context.h
+++ b/arch/arm/include/asm/mmu_context.h
@@ -67,6 +67,7 @@ static inline void check_and_switch_context(struct mm_struct *mm,
 		cpu_switch_mm(mm->pgd, mm);
 }
 
+#ifndef MODULE
 #define finish_arch_post_lock_switch \
 	finish_arch_post_lock_switch
 static inline void finish_arch_post_lock_switch(void)
@@ -88,6 +89,7 @@ static inline void finish_arch_post_lock_switch(void)
 		preempt_enable_no_resched();
 	}
 }
+#endif /* !MODULE */
 
 #endif	/* CONFIG_MMU */
 

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

end of thread, back to index

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-13 13:30 [PATCH] ARM: hide finish_arch_post_lock_switch from modules Arnd Bergmann
2016-05-13 13:51 ` Guenter Roeck
2016-05-13 14:15 ` [tip:sched/core] ARM: Hide finish_arch_post_lock_switch() " tip-bot for Steven Rostedt

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \
		linux-kernel@vger.kernel.org linux-kernel@archiver.kernel.org
	public-inbox-index lkml


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/ public-inbox