1. Patchew
  2. linux
  3. [PATCH v2 0/2] Remove problematic include in <asm/set_memory.h>
  4. View patch

[PATCH v2 2/2] x86/mm: Remove unnecessary include in set_memory.h

Kevin Brodsky posted 2 patches 1 year ago
[PATCH v2 2/2] x86/mm: Remove unnecessary include in set_memory.h
Posted by Kevin Brodsky 1 year ago 
Commit 03b122da74b2 ("x86/sgx: Hook arch_memory_failure() into
mainline code") included <linux/mm.h> in asm/set_memory.h to provide
some helper. However commit b3fdf9398a16 ("x86/mce: relocate
set{clear}_mce_nospec() functions") moved the inline definitions
someplace else, and now set_memory.h just declares a bunch of
functions.

No need for the whole linux/mm.h for declaring functions; just
remove that include. This helps avoid circular dependency headaches
(e.g. if linux/mm.h ends up including <linux/set_memory.h>).

This change requires a couple of include fixups not to break the
build:

* asm/smp.h: including <asm/thread_info.h> directly relies on
  <linux/thread_info.h> having already been included, because the
  former needs the BAD_STACK/NOT_STACK constants defined in the
  latter. This is no longer the case when asm/smp.h is included from
  some driver file - just include <linux/thread_info.h> to stay out
  of trouble.

* sev-guest.c relies on <asm/set_memory.h> including <linux/mm.h>,
  so we just need to make that include explicit.

Signed-off-by: Kevin Brodsky <kevin.brodsky@arm.com>
---
 arch/x86/include/asm/set_memory.h       | 1 -
 arch/x86/include/asm/smp.h              | 2 +-
 drivers/virt/coco/sev-guest/sev-guest.c | 1 +
 3 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/set_memory.h b/arch/x86/include/asm/set_memory.h
index 6586d533fe3a..8d9f1c9aaa4c 100644
--- a/arch/x86/include/asm/set_memory.h
+++ b/arch/x86/include/asm/set_memory.h
@@ -2,7 +2,6 @@
 #ifndef _ASM_X86_SET_MEMORY_H
 #define _ASM_X86_SET_MEMORY_H
 
-#include <linux/mm.h>
 #include <asm/page.h>
 #include <asm-generic/set_memory.h>
 
diff --git a/arch/x86/include/asm/smp.h b/arch/x86/include/asm/smp.h
index ca073f40698f..2ca1da5f16d9 100644
--- a/arch/x86/include/asm/smp.h
+++ b/arch/x86/include/asm/smp.h
@@ -3,10 +3,10 @@
 #define _ASM_X86_SMP_H
 #ifndef __ASSEMBLY__
 #include <linux/cpumask.h>
+#include <linux/thread_info.h>
 
 #include <asm/cpumask.h>
 #include <asm/current.h>
-#include <asm/thread_info.h>
 
 DECLARE_PER_CPU_READ_MOSTLY(cpumask_var_t, cpu_sibling_map);
 DECLARE_PER_CPU_READ_MOSTLY(cpumask_var_t, cpu_core_map);
diff --git a/drivers/virt/coco/sev-guest/sev-guest.c b/drivers/virt/coco/sev-guest/sev-guest.c
index b699771be029..e134bee818fa 100644
--- a/drivers/virt/coco/sev-guest/sev-guest.c
+++ b/drivers/virt/coco/sev-guest/sev-guest.c
@@ -23,6 +23,7 @@
 #include <linux/cleanup.h>
 #include <linux/uuid.h>
 #include <linux/configfs.h>
+#include <linux/mm.h>
 #include <uapi/linux/sev-guest.h>
 #include <uapi/linux/psp-sev.h>
 
-- 
2.47.0
Re: [PATCH v2 2/2] x86/mm: Remove unnecessary include in set_memory.h
Posted by Ingo Molnar 9 months, 2 weeks ago 
* Kevin Brodsky <kevin.brodsky@arm.com> wrote:

> Commit 03b122da74b2 ("x86/sgx: Hook arch_memory_failure() into
> mainline code") included <linux/mm.h> in asm/set_memory.h to provide
> some helper. However commit b3fdf9398a16 ("x86/mce: relocate
> set{clear}_mce_nospec() functions") moved the inline definitions
> someplace else, and now set_memory.h just declares a bunch of
> functions.
> 
> No need for the whole linux/mm.h for declaring functions; just
> remove that include. This helps avoid circular dependency headaches
> (e.g. if linux/mm.h ends up including <linux/set_memory.h>).
> 
> This change requires a couple of include fixups not to break the
> build:
> 
> * asm/smp.h: including <asm/thread_info.h> directly relies on
>   <linux/thread_info.h> having already been included, because the
>   former needs the BAD_STACK/NOT_STACK constants defined in the
>   latter. This is no longer the case when asm/smp.h is included from
>   some driver file - just include <linux/thread_info.h> to stay out
>   of trouble.
> 
> * sev-guest.c relies on <asm/set_memory.h> including <linux/mm.h>,
>   so we just need to make that include explicit.
> 
> Signed-off-by: Kevin Brodsky <kevin.brodsky@arm.com>
> ---
>  arch/x86/include/asm/set_memory.h       | 1 -
>  arch/x86/include/asm/smp.h              | 2 +-
>  drivers/virt/coco/sev-guest/sev-guest.c | 1 +
>  3 files changed, 2 insertions(+), 2 deletions(-)

So I tried to pick up this patch belatedly, but there's more places 
that mistakenly learned to rely on the stray <linux/mm.h> inclusion, 
for example on x86 defconfig-ish kernels:


  In file included from drivers/gpu/drm/i915/gt/intel_ggtt.c:6:
  ./arch/x86/include/asm/set_memory.h:40:57: error: unknown type name ‘pgprot_t’
  40 | int __set_memory_prot(unsigned long addr, int numpages, pgprot_t prot);
  |                                                         ^~~~~~~~

Thanks,

	Ingo
Re: [PATCH v2 2/2] x86/mm: Remove unnecessary include in set_memory.h
Posted by Ingo Molnar 9 months, 2 weeks ago 
* Ingo Molnar <mingo@kernel.org> wrote:

> So I tried to pick up this patch belatedly, but there's more places 
> that mistakenly learned to rely on the stray <linux/mm.h> inclusion, 
> for example on x86 defconfig-ish kernels:
> 
> 
>   In file included from drivers/gpu/drm/i915/gt/intel_ggtt.c:6:
>   ./arch/x86/include/asm/set_memory.h:40:57: error: unknown type name ‘pgprot_t’
>   40 | int __set_memory_prot(unsigned long addr, int numpages, pgprot_t prot);
>   |                                                         ^~~~~~~~

BTW., I did a few touchups to the changelog (see below) - mind picking 
that up once you submit -v2?

Thanks,

	Ingo

======================>
From: Kevin Brodsky <kevin.brodsky@arm.com>
Date: Thu, 12 Dec 2024 08:09:04 +0000
Subject: [PATCH] x86/mm: Reduce header dependencies in <asm/set_memory.h>

Commit:

  03b122da74b2 ("x86/sgx: Hook arch_memory_failure() into mainline code")

... added <linux/mm.h> to <asm/set_memory.h> to provide some helpers.

However the following commit:

  b3fdf9398a16 ("x86/mce: relocate set{clear}_mce_nospec() functions")

... moved the inline definitions someplace else, and now <asm/set_memory.h>
just declares a bunch of mostly self-contained functions.

No need for the whole <linux/mm.h> inclusion to declare functions; just
remove that include. This helps avoid circular dependency headaches
(e.g. if <linux/mm.h> ends up including <linux/set_memory.h>).

This change requires a couple of include fixups not to break the
build:

* <asm/smp.h>: including <asm/thread_info.h> directly relies on
  <linux/thread_info.h> having already been included, because the
  former needs the BAD_STACK/NOT_STACK constants defined in the
  latter. This is no longer the case when <asm/smp.h> is included from
  some driver file - just include <linux/thread_info.h> to stay out
  of trouble.

* sev-guest.c relies on <asm/set_memory.h> including <linux/mm.h>,
  so we just need to make that include explicit.

[ mingo: Cleaned up the changelog ]

Signed-off-by: Kevin Brodsky <kevin.brodsky@arm.com>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Acked-by: David Hildenbrand <david@redhat.com>
Link: https://lore.kernel.org/r/20241212080904.2089632-3-kevin.brodsky@arm.com
---
 arch/x86/include/asm/set_memory.h       | 1 -
 arch/x86/include/asm/smp.h              | 2 +-
 drivers/virt/coco/sev-guest/sev-guest.c | 1 +
 3 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/set_memory.h b/arch/x86/include/asm/set_memory.h
index cc62ef70ccc0..023994fe6115 100644
--- a/arch/x86/include/asm/set_memory.h
+++ b/arch/x86/include/asm/set_memory.h
@@ -2,7 +2,6 @@
 #ifndef _ASM_X86_SET_MEMORY_H
 #define _ASM_X86_SET_MEMORY_H
 
-#include <linux/mm.h>
 #include <asm/page.h>
 #include <asm-generic/set_memory.h>
 
diff --git a/arch/x86/include/asm/smp.h b/arch/x86/include/asm/smp.h
index ca073f40698f..2ca1da5f16d9 100644
--- a/arch/x86/include/asm/smp.h
+++ b/arch/x86/include/asm/smp.h
@@ -3,10 +3,10 @@
 #define _ASM_X86_SMP_H
 #ifndef __ASSEMBLY__
 #include <linux/cpumask.h>
+#include <linux/thread_info.h>
 
 #include <asm/cpumask.h>
 #include <asm/current.h>
-#include <asm/thread_info.h>
 
 DECLARE_PER_CPU_READ_MOSTLY(cpumask_var_t, cpu_sibling_map);
 DECLARE_PER_CPU_READ_MOSTLY(cpumask_var_t, cpu_core_map);
diff --git a/drivers/virt/coco/sev-guest/sev-guest.c b/drivers/virt/coco/sev-guest/sev-guest.c
index 264b6523fe52..ddec5677e247 100644
--- a/drivers/virt/coco/sev-guest/sev-guest.c
+++ b/drivers/virt/coco/sev-guest/sev-guest.c
@@ -23,6 +23,7 @@
 #include <linux/cleanup.h>
 #include <linux/uuid.h>
 #include <linux/configfs.h>
+#include <linux/mm.h>
 #include <uapi/linux/sev-guest.h>
 #include <uapi/linux/psp-sev.h>
Re: [PATCH v2 2/2] x86/mm: Remove unnecessary include in set_memory.h
Posted by Kevin Brodsky 9 months, 2 weeks ago 
On 27/02/2025 14:03, Ingo Molnar wrote:
> * Ingo Molnar <mingo@kernel.org> wrote:
>
>> So I tried to pick up this patch belatedly, but there's more places 
>> that mistakenly learned to rely on the stray <linux/mm.h> inclusion, 
>> for example on x86 defconfig-ish kernels:
>>
>>
>>   In file included from drivers/gpu/drm/i915/gt/intel_ggtt.c:6:
>>   ./arch/x86/include/asm/set_memory.h:40:57: error: unknown type name ‘pgprot_t’
>>   40 | int __set_memory_prot(unsigned long addr, int numpages, pgprot_t prot);
>>   |                                                         ^~~~~~~~

This patch relies on patch 1 in this series, which removes
__set_memory_prot(). I seem to be able to build x86_64_defconfig without
issue with both patches applies on the latest mainline.

> BTW., I did a few touchups to the changelog (see below) - mind picking 
> that up once you submit -v2?

Very happy with the touchups, thanks for having a look! Given my comment
above, I don't believe there is a need to post a v3 - feel free to amend
the commit message when applying the patches.

- Kevin
Re: [PATCH v2 2/2] x86/mm: Remove unnecessary include in set_memory.h
Posted by Ingo Molnar 9 months, 2 weeks ago 
* Kevin Brodsky <kevin.brodsky@arm.com> wrote:

> On 27/02/2025 14:03, Ingo Molnar wrote:
> > * Ingo Molnar <mingo@kernel.org> wrote:
> >
> >> So I tried to pick up this patch belatedly, but there's more places 
> >> that mistakenly learned to rely on the stray <linux/mm.h> inclusion, 
> >> for example on x86 defconfig-ish kernels:
> >>
> >>
> >>   In file included from drivers/gpu/drm/i915/gt/intel_ggtt.c:6:
> >>   ./arch/x86/include/asm/set_memory.h:40:57: error: unknown type name ‘pgprot_t’
> >>   40 | int __set_memory_prot(unsigned long addr, int numpages, pgprot_t prot);
> >>   |                                                         ^~~~~~~~
> 
> This patch relies on patch 1 in this series, which removes 
> __set_memory_prot(). I seem to be able to build x86_64_defconfig 
> without issue with both patches applies on the latest mainline.

Oh, the 1/2 patch was missing from my mailbox (my mbox's fault, not yours),
and apparently the 'PATCH 2/2' tag wasn't a big enough of a clue for me
that there's a dependent patch. ;-)

Anyway, I re-tested it with both patches applied and it's all looking 
good now, and I have applied them to tip:x86/headers.

Thanks,

	Ingo
Re: [PATCH v2 2/2] x86/mm: Remove unnecessary include in set_memory.h
Posted by Kevin Brodsky 9 months, 2 weeks ago 
On 28/02/2025 18:14, Ingo Molnar wrote:
> * Kevin Brodsky <kevin.brodsky@arm.com> wrote:
>
>> On 27/02/2025 14:03, Ingo Molnar wrote:
>>> * Ingo Molnar <mingo@kernel.org> wrote:
>>>
>>>> So I tried to pick up this patch belatedly, but there's more places 
>>>> that mistakenly learned to rely on the stray <linux/mm.h> inclusion, 
>>>> for example on x86 defconfig-ish kernels:
>>>>
>>>>
>>>>   In file included from drivers/gpu/drm/i915/gt/intel_ggtt.c:6:
>>>>   ./arch/x86/include/asm/set_memory.h:40:57: error: unknown type name ‘pgprot_t’
>>>>   40 | int __set_memory_prot(unsigned long addr, int numpages, pgprot_t prot);
>>>>   |                                                         ^~~~~~~~
>> This patch relies on patch 1 in this series, which removes 
>> __set_memory_prot(). I seem to be able to build x86_64_defconfig 
>> without issue with both patches applies on the latest mainline.
> Oh, the 1/2 patch was missing from my mailbox (my mbox's fault, not yours),
> and apparently the 'PATCH 2/2' tag wasn't a big enough of a clue for me
> that there's a dependent patch. ;-)

Ah that would explain it! Naughty mailbox eating patches :)

> Anyway, I re-tested it with both patches applied and it's all looking 
> good now, and I have applied them to tip:x86/headers.

Great, thank you!

- Kevin
[tip: x86/headers] x86/mm: Reduce header dependencies in <asm/set_memory.h>
Posted by tip-bot2 for Kevin Brodsky 9 months, 2 weeks ago 
The following commit has been merged into the x86/headers branch of tip:

Commit-ID:     95c4cc5a585400982ae5b3bf9e3be6de71768376
Gitweb:        https://git.kernel.org/tip/95c4cc5a585400982ae5b3bf9e3be6de71768376
Author:        Kevin Brodsky <kevin.brodsky@arm.com>
AuthorDate:    Thu, 12 Dec 2024 08:09:04 
Committer:     Ingo Molnar <mingo@kernel.org>
CommitterDate: Fri, 28 Feb 2025 17:35:22 +01:00

x86/mm: Reduce header dependencies in <asm/set_memory.h>

Commit:

  03b122da74b2 ("x86/sgx: Hook arch_memory_failure() into mainline code")

... added <linux/mm.h> to <asm/set_memory.h> to provide some helpers.

However the following commit:

  b3fdf9398a16 ("x86/mce: relocate set{clear}_mce_nospec() functions")

... moved the inline definitions someplace else, and now <asm/set_memory.h>
just declares a bunch of mostly self-contained functions.

No need for the whole <linux/mm.h> inclusion to declare functions; just
remove that include. This helps avoid circular dependency headaches
(e.g. if <linux/mm.h> ends up including <linux/set_memory.h>).

This change requires a couple of include fixups not to break the
build:

* <asm/smp.h>: including <asm/thread_info.h> directly relies on
  <linux/thread_info.h> having already been included, because the
  former needs the BAD_STACK/NOT_STACK constants defined in the
  latter. This is no longer the case when <asm/smp.h> is included from
  some driver file - just include <linux/thread_info.h> to stay out
  of trouble.

* sev-guest.c relies on <asm/set_memory.h> including <linux/mm.h>,
  so we just need to make that include explicit.

[ mingo: Cleaned up the changelog ]

Signed-off-by: Kevin Brodsky <kevin.brodsky@arm.com>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Acked-by: David Hildenbrand <david@redhat.com>
Link: https://lore.kernel.org/r/20241212080904.2089632-3-kevin.brodsky@arm.com
---
 arch/x86/include/asm/set_memory.h       | 1 -
 arch/x86/include/asm/smp.h              | 2 +-
 drivers/virt/coco/sev-guest/sev-guest.c | 1 +
 3 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/set_memory.h b/arch/x86/include/asm/set_memory.h
index 6586d53..8d9f1c9 100644
--- a/arch/x86/include/asm/set_memory.h
+++ b/arch/x86/include/asm/set_memory.h
@@ -2,7 +2,6 @@
 #ifndef _ASM_X86_SET_MEMORY_H
 #define _ASM_X86_SET_MEMORY_H
 
-#include <linux/mm.h>
 #include <asm/page.h>
 #include <asm-generic/set_memory.h>
 
diff --git a/arch/x86/include/asm/smp.h b/arch/x86/include/asm/smp.h
index ca073f4..2ca1da5 100644
--- a/arch/x86/include/asm/smp.h
+++ b/arch/x86/include/asm/smp.h
@@ -3,10 +3,10 @@
 #define _ASM_X86_SMP_H
 #ifndef __ASSEMBLY__
 #include <linux/cpumask.h>
+#include <linux/thread_info.h>
 
 #include <asm/cpumask.h>
 #include <asm/current.h>
-#include <asm/thread_info.h>
 
 DECLARE_PER_CPU_READ_MOSTLY(cpumask_var_t, cpu_sibling_map);
 DECLARE_PER_CPU_READ_MOSTLY(cpumask_var_t, cpu_core_map);
diff --git a/drivers/virt/coco/sev-guest/sev-guest.c b/drivers/virt/coco/sev-guest/sev-guest.c
index 264b652..ddec567 100644
--- a/drivers/virt/coco/sev-guest/sev-guest.c
+++ b/drivers/virt/coco/sev-guest/sev-guest.c
@@ -23,6 +23,7 @@
 #include <linux/cleanup.h>
 #include <linux/uuid.h>
 #include <linux/configfs.h>
+#include <linux/mm.h>
 #include <uapi/linux/sev-guest.h>
 #include <uapi/linux/psp-sev.h>

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.