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

Groups > linux.kernel > #1333002 > unrolled thread

[PATCH] arm64: remove redundant preempt.h

Back to article view | Back to linux.kernel

Contents

  [PATCH] arm64: remove redundant preempt.h Yang Shi <yang.shi@linaro.org> - 2016-02-12 19:20 +0100
    Re: [PATCH] arm64: remove redundant preempt.h "Shi, Yang" <yang.shi@linaro.org> - 2016-02-20 01:10 +0100
      Re: [PATCH] arm64: remove redundant preempt.h Will Deacon <will.deacon@arm.com> - 2016-02-22 11:00 +0100
        Re: [PATCH] arm64: remove redundant preempt.h Mark Rutland <mark.rutland@arm.com> - 2016-02-22 18:10 +0100

#1333002 — [PATCH] arm64: remove redundant preempt.h

preempt.h has been included by sched.h, so it is not necessary to include
both, just keep sched.h.

Signed-off-by: Yang Shi <yang.shi@linaro.org>
---
Happened to find this when checking preempt.h include for another patch review.
Build test is passed with and without CONFIG_PREEMPT enabled.

 arch/arm64/kernel/cpuinfo.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/arm64/kernel/cpuinfo.c b/arch/arm64/kernel/cpuinfo.c
index 212ae63..e2b37ff 100644
--- a/arch/arm64/kernel/cpuinfo.c
+++ b/arch/arm64/kernel/cpuinfo.c
@@ -25,7 +25,6 @@
 #include <linux/init.h>
 #include <linux/kernel.h>
 #include <linux/personality.h>
-#include <linux/preempt.h>
 #include <linux/printk.h>
 #include <linux/seq_file.h>
 #include <linux/sched.h>
-- 
2.0.2

[toc] | [next] | [standalone]

#1338584

Hi Will & Catalin,

Any comment on this one?

Thanks,
Yang

On 2/12/2016 9:56 AM, Yang Shi wrote:
> preempt.h has been included by sched.h, so it is not necessary to include
> both, just keep sched.h.
>
> Signed-off-by: Yang Shi <yang.shi@linaro.org>
> ---
> Happened to find this when checking preempt.h include for another patch review.
> Build test is passed with and without CONFIG_PREEMPT enabled.
>
>   arch/arm64/kernel/cpuinfo.c | 1 -
>   1 file changed, 1 deletion(-)
>
> diff --git a/arch/arm64/kernel/cpuinfo.c b/arch/arm64/kernel/cpuinfo.c
> index 212ae63..e2b37ff 100644
> --- a/arch/arm64/kernel/cpuinfo.c
> +++ b/arch/arm64/kernel/cpuinfo.c
> @@ -25,7 +25,6 @@
>   #include <linux/init.h>
>   #include <linux/kernel.h>
>   #include <linux/personality.h>
> -#include <linux/preempt.h>
>   #include <linux/printk.h>
>   #include <linux/seq_file.h>
>   #include <linux/sched.h>
>

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

#1339302

On Fri, Feb 19, 2016 at 04:07:02PM -0800, Shi, Yang wrote:
> Any comment on this one?

I don't really see the point in it, to be honest. Relying on implicit
#includes is usually a bad idea, there are other files in the kernel
including both of these headers and there's not actually a problem to
fix as far as I can tell.

Will

> On 2/12/2016 9:56 AM, Yang Shi wrote:
> >preempt.h has been included by sched.h, so it is not necessary to include
> >both, just keep sched.h.
> >
> >Signed-off-by: Yang Shi <yang.shi@linaro.org>
> >---
> >Happened to find this when checking preempt.h include for another patch review.
> >Build test is passed with and without CONFIG_PREEMPT enabled.
> >
> >  arch/arm64/kernel/cpuinfo.c | 1 -
> >  1 file changed, 1 deletion(-)
> >
> >diff --git a/arch/arm64/kernel/cpuinfo.c b/arch/arm64/kernel/cpuinfo.c
> >index 212ae63..e2b37ff 100644
> >--- a/arch/arm64/kernel/cpuinfo.c
> >+++ b/arch/arm64/kernel/cpuinfo.c
> >@@ -25,7 +25,6 @@
> >  #include <linux/init.h>
> >  #include <linux/kernel.h>
> >  #include <linux/personality.h>
> >-#include <linux/preempt.h>
> >  #include <linux/printk.h>
> >  #include <linux/seq_file.h>
> >  #include <linux/sched.h>
> >
> 

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

#1339734

On Mon, Feb 22, 2016 at 09:52:05AM +0000, Will Deacon wrote:
> On Fri, Feb 19, 2016 at 04:07:02PM -0800, Shi, Yang wrote:
> > Any comment on this one?
> 
> I don't really see the point in it, to be honest. Relying on implicit
> #includes is usually a bad idea,

FWIW, in this case we aren't relying on implicit includes. That's when
you explicitly use something but haven't included the relevant header
directly.

For everything we explicitly use, we have included the relevant headers.

Anything that uses preempt.h implicitly should've pulled in preempt.h
via its headers.

> there are other files in the kernel including both of these headers
> and there's not actually a problem to fix as far as I can tell.

Up to you.

The patch does appear correct in that the include is redundant, but its
presence is admittedly not harmful.

Mark.

> 
> Will
> 
> > On 2/12/2016 9:56 AM, Yang Shi wrote:
> > >preempt.h has been included by sched.h, so it is not necessary to include
> > >both, just keep sched.h.
> > >
> > >Signed-off-by: Yang Shi <yang.shi@linaro.org>
> > >---
> > >Happened to find this when checking preempt.h include for another patch review.
> > >Build test is passed with and without CONFIG_PREEMPT enabled.
> > >
> > >  arch/arm64/kernel/cpuinfo.c | 1 -
> > >  1 file changed, 1 deletion(-)
> > >
> > >diff --git a/arch/arm64/kernel/cpuinfo.c b/arch/arm64/kernel/cpuinfo.c
> > >index 212ae63..e2b37ff 100644
> > >--- a/arch/arm64/kernel/cpuinfo.c
> > >+++ b/arch/arm64/kernel/cpuinfo.c
> > >@@ -25,7 +25,6 @@
> > >  #include <linux/init.h>
> > >  #include <linux/kernel.h>
> > >  #include <linux/personality.h>
> > >-#include <linux/preempt.h>
> > >  #include <linux/printk.h>
> > >  #include <linux/seq_file.h>
> > >  #include <linux/sched.h>
> > >
> > 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web