Re: [PATCH] sched/core: don't include asm/mmu_context from drivers
On Thursday 12 May 2016 22:46:56 Andy Lutomirski wrote: > On Fri, Apr 29, 2016 at 6:42 AM, Steven Rostedtwrote: > > On Fri, 29 Apr 2016 10:52:32 +0200 > > Arnd Bergmann wrote: > > > >> This reverts the earlier fix attempt and works around the problem > >> by including both linux/mmu_context.h and asm/mmu_context.h from > >> kernel/sched/core.c. This is not a good solution but seems less > >> hacky than the alternatives. > > > > What about simply not compiling finish_arch_post_lock_switch() when > > building modules? > > > > (untested, not compiled or anything) > > > > Signed-off-by: Steven Rostedt > > --- > > diff --git a/arch/arm/include/asm/mmu_context.h > > b/arch/arm/include/asm/mmu_context.h > > index fa5b42d44985..3f22d1b6bac8 100644 > > --- a/arch/arm/include/asm/mmu_context.h > > +++ b/arch/arm/include/asm/mmu_context.h > > @@ -66,6 +66,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) > > @@ -87,6 +88,7 @@ static inline void finish_arch_post_lock_switch(void) > > preempt_enable_no_resched(); > > } > > } > > +#endif /* !MODULE */ > > > > #endif /* CONFIG_MMU */ > > > > > Can someone in arm land ack this so Ingo can apply it? > Sorry I forgot about this when I had my original patch in the randconfig patch stack. I've reverted this now and am testing with Steve's version. If I see no other regressions, I'll resend this with a proper changelog and Russell's Ack. Arnd
Re: [PATCH] sched/core: don't include asm/mmu_context from drivers
On Thursday 12 May 2016 22:46:56 Andy Lutomirski wrote: > On Fri, Apr 29, 2016 at 6:42 AM, Steven Rostedt wrote: > > On Fri, 29 Apr 2016 10:52:32 +0200 > > Arnd Bergmann wrote: > > > >> This reverts the earlier fix attempt and works around the problem > >> by including both linux/mmu_context.h and asm/mmu_context.h from > >> kernel/sched/core.c. This is not a good solution but seems less > >> hacky than the alternatives. > > > > What about simply not compiling finish_arch_post_lock_switch() when > > building modules? > > > > (untested, not compiled or anything) > > > > Signed-off-by: Steven Rostedt > > --- > > diff --git a/arch/arm/include/asm/mmu_context.h > > b/arch/arm/include/asm/mmu_context.h > > index fa5b42d44985..3f22d1b6bac8 100644 > > --- a/arch/arm/include/asm/mmu_context.h > > +++ b/arch/arm/include/asm/mmu_context.h > > @@ -66,6 +66,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) > > @@ -87,6 +88,7 @@ static inline void finish_arch_post_lock_switch(void) > > preempt_enable_no_resched(); > > } > > } > > +#endif /* !MODULE */ > > > > #endif /* CONFIG_MMU */ > > > > > Can someone in arm land ack this so Ingo can apply it? > Sorry I forgot about this when I had my original patch in the randconfig patch stack. I've reverted this now and am testing with Steve's version. If I see no other regressions, I'll resend this with a proper changelog and Russell's Ack. Arnd
Re: [PATCH] sched/core: don't include asm/mmu_context from drivers
On Thu, May 12, 2016 at 10:46:56PM -0700, Andy Lutomirski wrote: > On Fri, Apr 29, 2016 at 6:42 AM, Steven Rostedtwrote: > > On Fri, 29 Apr 2016 10:52:32 +0200 > > Arnd Bergmann wrote: > > > >> This reverts the earlier fix attempt and works around the problem > >> by including both linux/mmu_context.h and asm/mmu_context.h from > >> kernel/sched/core.c. This is not a good solution but seems less > >> hacky than the alternatives. > > > > What about simply not compiling finish_arch_post_lock_switch() when > > building modules? > > > > (untested, not compiled or anything) > > > > Signed-off-by: Steven Rostedt > > --- > > diff --git a/arch/arm/include/asm/mmu_context.h > > b/arch/arm/include/asm/mmu_context.h > > index fa5b42d44985..3f22d1b6bac8 100644 > > --- a/arch/arm/include/asm/mmu_context.h > > +++ b/arch/arm/include/asm/mmu_context.h > > @@ -66,6 +66,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) > > @@ -87,6 +88,7 @@ static inline void finish_arch_post_lock_switch(void) > > preempt_enable_no_resched(); > > } > > } > > +#endif /* !MODULE */ > > > > #endif /* CONFIG_MMU */ > > > > > Can someone in arm land ack this so Ingo can apply it? Sorry, I'm simply unable to read every message that comes in. Acked-by: Russell King -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net.
Re: [PATCH] sched/core: don't include asm/mmu_context from drivers
On Thu, May 12, 2016 at 10:46:56PM -0700, Andy Lutomirski wrote: > On Fri, Apr 29, 2016 at 6:42 AM, Steven Rostedt wrote: > > On Fri, 29 Apr 2016 10:52:32 +0200 > > Arnd Bergmann wrote: > > > >> This reverts the earlier fix attempt and works around the problem > >> by including both linux/mmu_context.h and asm/mmu_context.h from > >> kernel/sched/core.c. This is not a good solution but seems less > >> hacky than the alternatives. > > > > What about simply not compiling finish_arch_post_lock_switch() when > > building modules? > > > > (untested, not compiled or anything) > > > > Signed-off-by: Steven Rostedt > > --- > > diff --git a/arch/arm/include/asm/mmu_context.h > > b/arch/arm/include/asm/mmu_context.h > > index fa5b42d44985..3f22d1b6bac8 100644 > > --- a/arch/arm/include/asm/mmu_context.h > > +++ b/arch/arm/include/asm/mmu_context.h > > @@ -66,6 +66,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) > > @@ -87,6 +88,7 @@ static inline void finish_arch_post_lock_switch(void) > > preempt_enable_no_resched(); > > } > > } > > +#endif /* !MODULE */ > > > > #endif /* CONFIG_MMU */ > > > > > Can someone in arm land ack this so Ingo can apply it? Sorry, I'm simply unable to read every message that comes in. Acked-by: Russell King -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net.
Re: [PATCH] sched/core: don't include asm/mmu_context from drivers
On Fri, Apr 29, 2016 at 6:42 AM, Steven Rostedtwrote: > On Fri, 29 Apr 2016 10:52:32 +0200 > Arnd Bergmann wrote: > >> This reverts the earlier fix attempt and works around the problem >> by including both linux/mmu_context.h and asm/mmu_context.h from >> kernel/sched/core.c. This is not a good solution but seems less >> hacky than the alternatives. > > What about simply not compiling finish_arch_post_lock_switch() when > building modules? > > (untested, not compiled or anything) > > Signed-off-by: Steven Rostedt > --- > diff --git a/arch/arm/include/asm/mmu_context.h > b/arch/arm/include/asm/mmu_context.h > index fa5b42d44985..3f22d1b6bac8 100644 > --- a/arch/arm/include/asm/mmu_context.h > +++ b/arch/arm/include/asm/mmu_context.h > @@ -66,6 +66,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) > @@ -87,6 +88,7 @@ static inline void finish_arch_post_lock_switch(void) > preempt_enable_no_resched(); > } > } > +#endif /* !MODULE */ > > #endif /* CONFIG_MMU */ > Can someone in arm land ack this so Ingo can apply it? --Andy -- Andy Lutomirski AMA Capital Management, LLC
Re: [PATCH] sched/core: don't include asm/mmu_context from drivers
On Fri, Apr 29, 2016 at 6:42 AM, Steven Rostedt wrote: > On Fri, 29 Apr 2016 10:52:32 +0200 > Arnd Bergmann wrote: > >> This reverts the earlier fix attempt and works around the problem >> by including both linux/mmu_context.h and asm/mmu_context.h from >> kernel/sched/core.c. This is not a good solution but seems less >> hacky than the alternatives. > > What about simply not compiling finish_arch_post_lock_switch() when > building modules? > > (untested, not compiled or anything) > > Signed-off-by: Steven Rostedt > --- > diff --git a/arch/arm/include/asm/mmu_context.h > b/arch/arm/include/asm/mmu_context.h > index fa5b42d44985..3f22d1b6bac8 100644 > --- a/arch/arm/include/asm/mmu_context.h > +++ b/arch/arm/include/asm/mmu_context.h > @@ -66,6 +66,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) > @@ -87,6 +88,7 @@ static inline void finish_arch_post_lock_switch(void) > preempt_enable_no_resched(); > } > } > +#endif /* !MODULE */ > > #endif /* CONFIG_MMU */ > Can someone in arm land ack this so Ingo can apply it? --Andy -- Andy Lutomirski AMA Capital Management, LLC
Re: [PATCH] sched/core: don't include asm/mmu_context from drivers
On Fri, Apr 29, 2016 at 8:37 AM, Arnd Bergmannwrote: > On Friday 29 April 2016 09:42:18 Steven Rostedt wrote: >> On Fri, 29 Apr 2016 10:52:32 +0200 >> Arnd Bergmann wrote: >> >> > This reverts the earlier fix attempt and works around the problem >> > by including both linux/mmu_context.h and asm/mmu_context.h from >> > kernel/sched/core.c. This is not a good solution but seems less >> > hacky than the alternatives. >> >> What about simply not compiling finish_arch_post_lock_switch() when >> building modules? >> >> (untested, not compiled or anything) >> >> Signed-off-by: Steven Rostedt >> > > It should work as well. > > I think I suggested doing that the last time the problem came up > a few years ago, but we ended up not including the header instead, > so I kept doing that. > > Arnd This variant looks considerably nicer to me. --Andy
Re: [PATCH] sched/core: don't include asm/mmu_context from drivers
On Fri, Apr 29, 2016 at 8:37 AM, Arnd Bergmann wrote: > On Friday 29 April 2016 09:42:18 Steven Rostedt wrote: >> On Fri, 29 Apr 2016 10:52:32 +0200 >> Arnd Bergmann wrote: >> >> > This reverts the earlier fix attempt and works around the problem >> > by including both linux/mmu_context.h and asm/mmu_context.h from >> > kernel/sched/core.c. This is not a good solution but seems less >> > hacky than the alternatives. >> >> What about simply not compiling finish_arch_post_lock_switch() when >> building modules? >> >> (untested, not compiled or anything) >> >> Signed-off-by: Steven Rostedt >> > > It should work as well. > > I think I suggested doing that the last time the problem came up > a few years ago, but we ended up not including the header instead, > so I kept doing that. > > Arnd This variant looks considerably nicer to me. --Andy
Re: [PATCH] sched/core: don't include asm/mmu_context from drivers
On Friday 29 April 2016 09:42:18 Steven Rostedt wrote: > On Fri, 29 Apr 2016 10:52:32 +0200 > Arnd Bergmannwrote: > > > This reverts the earlier fix attempt and works around the problem > > by including both linux/mmu_context.h and asm/mmu_context.h from > > kernel/sched/core.c. This is not a good solution but seems less > > hacky than the alternatives. > > What about simply not compiling finish_arch_post_lock_switch() when > building modules? > > (untested, not compiled or anything) > > Signed-off-by: Steven Rostedt > It should work as well. I think I suggested doing that the last time the problem came up a few years ago, but we ended up not including the header instead, so I kept doing that. Arnd
Re: [PATCH] sched/core: don't include asm/mmu_context from drivers
On Friday 29 April 2016 09:42:18 Steven Rostedt wrote: > On Fri, 29 Apr 2016 10:52:32 +0200 > Arnd Bergmann wrote: > > > This reverts the earlier fix attempt and works around the problem > > by including both linux/mmu_context.h and asm/mmu_context.h from > > kernel/sched/core.c. This is not a good solution but seems less > > hacky than the alternatives. > > What about simply not compiling finish_arch_post_lock_switch() when > building modules? > > (untested, not compiled or anything) > > Signed-off-by: Steven Rostedt > It should work as well. I think I suggested doing that the last time the problem came up a few years ago, but we ended up not including the header instead, so I kept doing that. Arnd
Re: [PATCH] sched/core: don't include asm/mmu_context from drivers
On Fri, 29 Apr 2016 10:52:32 +0200 Arnd Bergmannwrote: > This reverts the earlier fix attempt and works around the problem > by including both linux/mmu_context.h and asm/mmu_context.h from > kernel/sched/core.c. This is not a good solution but seems less > hacky than the alternatives. What about simply not compiling finish_arch_post_lock_switch() when building modules? (untested, not compiled or anything) Signed-off-by: Steven Rostedt --- diff --git a/arch/arm/include/asm/mmu_context.h b/arch/arm/include/asm/mmu_context.h index fa5b42d44985..3f22d1b6bac8 100644 --- a/arch/arm/include/asm/mmu_context.h +++ b/arch/arm/include/asm/mmu_context.h @@ -66,6 +66,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) @@ -87,6 +88,7 @@ static inline void finish_arch_post_lock_switch(void) preempt_enable_no_resched(); } } +#endif /* !MODULE */ #endif /* CONFIG_MMU */
Re: [PATCH] sched/core: don't include asm/mmu_context from drivers
On Fri, 29 Apr 2016 10:52:32 +0200 Arnd Bergmann wrote: > This reverts the earlier fix attempt and works around the problem > by including both linux/mmu_context.h and asm/mmu_context.h from > kernel/sched/core.c. This is not a good solution but seems less > hacky than the alternatives. What about simply not compiling finish_arch_post_lock_switch() when building modules? (untested, not compiled or anything) Signed-off-by: Steven Rostedt --- diff --git a/arch/arm/include/asm/mmu_context.h b/arch/arm/include/asm/mmu_context.h index fa5b42d44985..3f22d1b6bac8 100644 --- a/arch/arm/include/asm/mmu_context.h +++ b/arch/arm/include/asm/mmu_context.h @@ -66,6 +66,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) @@ -87,6 +88,7 @@ static inline void finish_arch_post_lock_switch(void) preempt_enable_no_resched(); } } +#endif /* !MODULE */ #endif /* CONFIG_MMU */
[PATCH] sched/core: don't include asm/mmu_context from drivers
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 obviously didn't help. This reverts the earlier fix attempt and works around the problem by including both linux/mmu_context.h and asm/mmu_context.h from kernel/sched/core.c. This is not a good solution but seems less hacky than the alternatives. Signed-off-by: Arnd BergmannFixes: f98db6013c55 ("sched/core: Add switch_mm_irqs_off() and use it in the scheduler") Reverts: 88f10e37e150 ("sched/core, ARM: Include linux/preempt.h from asm/mmu_context.h") --- arch/arm/include/asm/mmu_context.h | 1 - include/linux/mmu_context.h| 2 -- kernel/sched/core.c| 1 + 3 files changed, 1 insertion(+), 3 deletions(-) diff --git a/arch/arm/include/asm/mmu_context.h b/arch/arm/include/asm/mmu_context.h index ed73babc0dc9..fa5b42d44985 100644 --- a/arch/arm/include/asm/mmu_context.h +++ b/arch/arm/include/asm/mmu_context.h @@ -15,7 +15,6 @@ #include #include -#include #include #include #include diff --git a/include/linux/mmu_context.h b/include/linux/mmu_context.h index a4441784503b..88d270706792 100644 --- a/include/linux/mmu_context.h +++ b/include/linux/mmu_context.h @@ -1,8 +1,6 @@ #ifndef _LINUX_MMU_CONTEXT_H #define _LINUX_MMU_CONTEXT_H -#include - struct mm_struct; void use_mm(struct mm_struct *mm); diff --git a/kernel/sched/core.c b/kernel/sched/core.c index c82ca6eccfec..baa2a5152658 100644 --- a/kernel/sched/core.c +++ b/kernel/sched/core.c @@ -78,6 +78,7 @@ #include #include #include +#include #include #ifdef CONFIG_PARAVIRT #include -- 2.7.0
[PATCH] sched/core: don't include asm/mmu_context from drivers
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 obviously didn't help. This reverts the earlier fix attempt and works around the problem by including both linux/mmu_context.h and asm/mmu_context.h from kernel/sched/core.c. This is not a good solution but seems less hacky than the alternatives. Signed-off-by: Arnd Bergmann Fixes: f98db6013c55 ("sched/core: Add switch_mm_irqs_off() and use it in the scheduler") Reverts: 88f10e37e150 ("sched/core, ARM: Include linux/preempt.h from asm/mmu_context.h") --- arch/arm/include/asm/mmu_context.h | 1 - include/linux/mmu_context.h| 2 -- kernel/sched/core.c| 1 + 3 files changed, 1 insertion(+), 3 deletions(-) diff --git a/arch/arm/include/asm/mmu_context.h b/arch/arm/include/asm/mmu_context.h index ed73babc0dc9..fa5b42d44985 100644 --- a/arch/arm/include/asm/mmu_context.h +++ b/arch/arm/include/asm/mmu_context.h @@ -15,7 +15,6 @@ #include #include -#include #include #include #include diff --git a/include/linux/mmu_context.h b/include/linux/mmu_context.h index a4441784503b..88d270706792 100644 --- a/include/linux/mmu_context.h +++ b/include/linux/mmu_context.h @@ -1,8 +1,6 @@ #ifndef _LINUX_MMU_CONTEXT_H #define _LINUX_MMU_CONTEXT_H -#include - struct mm_struct; void use_mm(struct mm_struct *mm); diff --git a/kernel/sched/core.c b/kernel/sched/core.c index c82ca6eccfec..baa2a5152658 100644 --- a/kernel/sched/core.c +++ b/kernel/sched/core.c @@ -78,6 +78,7 @@ #include #include #include +#include #include #ifdef CONFIG_PARAVIRT #include -- 2.7.0