1. Patchew
  2. linux
  3. [PATCH 00/50] big header dependency cleanup targeting sched.h
  4. View patch

[PATCH 44/50] restart_block: Trim includes

Kent Overstreet posted 50 patches 2 years ago
Only 14 patches received!
[PATCH 44/50] restart_block: Trim includes
Posted by Kent Overstreet 2 years ago 
We don't actually use any timekeeping types, no need to pull in
time64.h.

Also, sched.h uses restart_block; add it as a direct dependency.

Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
---
 include/linux/restart_block.h | 2 +-
 include/linux/sched.h         | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/linux/restart_block.h b/include/linux/restart_block.h
index 980a65594412..13f17676c5f4 100644
--- a/include/linux/restart_block.h
+++ b/include/linux/restart_block.h
@@ -7,8 +7,8 @@
 
 #include <linux/compiler.h>
 #include <linux/types.h>
-#include <linux/time64.h>
 
+struct __kernel_timespec;
 struct timespec;
 struct old_timespec32;
 struct pollfd;
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 98885e3a81e8..ec739277c39b 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -33,6 +33,7 @@
 #include <linux/mm_types_task.h>
 #include <linux/task_io_accounting.h>
 #include <linux/posix-timers_types.h>
+#include <linux/restart_block.h>
 #include <linux/rseq.h>
 #include <linux/seqlock_types.h>
 #include <linux/kcsan.h>
-- 
2.43.0
[PATCH 45/50] rseq: Split out rseq.h from sched.h
Posted by Kent Overstreet 2 years ago 
We're trying to get sched.h down to more or less just types only, not
code - rseq can live in its own header.

This helps us kill the dependency on preempt.h in sched.h.

Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
---
 arch/x86/kernel/signal.c         |   1 +
 fs/exec.c                        |   1 +
 include/linux/resume_user_mode.h |   1 +
 include/linux/rseq.h             | 131 +++++++++++++++++++++++++++++++
 include/linux/sched.h            | 125 +----------------------------
 kernel/fork.c                    |   1 +
 kernel/sched/core.c              |   1 +
 7 files changed, 137 insertions(+), 124 deletions(-)
 create mode 100644 include/linux/rseq.h

diff --git a/arch/x86/kernel/signal.c b/arch/x86/kernel/signal.c
index 65fe2094da59..31b6f5dddfc2 100644
--- a/arch/x86/kernel/signal.c
+++ b/arch/x86/kernel/signal.c
@@ -27,6 +27,7 @@
 #include <linux/context_tracking.h>
 #include <linux/entry-common.h>
 #include <linux/syscalls.h>
+#include <linux/rseq.h>
 
 #include <asm/processor.h>
 #include <asm/ucontext.h>
diff --git a/fs/exec.c b/fs/exec.c
index 4aa19b24f281..41773af7e3dc 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -66,6 +66,7 @@
 #include <linux/coredump.h>
 #include <linux/time_namespace.h>
 #include <linux/user_events.h>
+#include <linux/rseq.h>
 
 #include <linux/uaccess.h>
 #include <asm/mmu_context.h>
diff --git a/include/linux/resume_user_mode.h b/include/linux/resume_user_mode.h
index f8f3e958e9cf..e0135e0adae0 100644
--- a/include/linux/resume_user_mode.h
+++ b/include/linux/resume_user_mode.h
@@ -6,6 +6,7 @@
 #include <linux/sched.h>
 #include <linux/task_work.h>
 #include <linux/memcontrol.h>
+#include <linux/rseq.h>
 #include <linux/blk-cgroup.h>
 
 /**
diff --git a/include/linux/rseq.h b/include/linux/rseq.h
new file mode 100644
index 000000000000..bc8af3eb5598
--- /dev/null
+++ b/include/linux/rseq.h
@@ -0,0 +1,131 @@
+/* SPDX-License-Identifier: GPL-2.0+ WITH Linux-syscall-note */
+#ifndef _LINUX_RSEQ_H
+#define _LINUX_RSEQ_H
+
+#ifdef CONFIG_RSEQ
+
+#include <linux/preempt.h>
+#include <linux/sched.h>
+
+/*
+ * Map the event mask on the user-space ABI enum rseq_cs_flags
+ * for direct mask checks.
+ */
+enum rseq_event_mask_bits {
+	RSEQ_EVENT_PREEMPT_BIT	= RSEQ_CS_FLAG_NO_RESTART_ON_PREEMPT_BIT,
+	RSEQ_EVENT_SIGNAL_BIT	= RSEQ_CS_FLAG_NO_RESTART_ON_SIGNAL_BIT,
+	RSEQ_EVENT_MIGRATE_BIT	= RSEQ_CS_FLAG_NO_RESTART_ON_MIGRATE_BIT,
+};
+
+enum rseq_event_mask {
+	RSEQ_EVENT_PREEMPT	= (1U << RSEQ_EVENT_PREEMPT_BIT),
+	RSEQ_EVENT_SIGNAL	= (1U << RSEQ_EVENT_SIGNAL_BIT),
+	RSEQ_EVENT_MIGRATE	= (1U << RSEQ_EVENT_MIGRATE_BIT),
+};
+
+static inline void rseq_set_notify_resume(struct task_struct *t)
+{
+	if (t->rseq)
+		set_tsk_thread_flag(t, TIF_NOTIFY_RESUME);
+}
+
+void __rseq_handle_notify_resume(struct ksignal *sig, struct pt_regs *regs);
+
+static inline void rseq_handle_notify_resume(struct ksignal *ksig,
+					     struct pt_regs *regs)
+{
+	if (current->rseq)
+		__rseq_handle_notify_resume(ksig, regs);
+}
+
+static inline void rseq_signal_deliver(struct ksignal *ksig,
+				       struct pt_regs *regs)
+{
+	preempt_disable();
+	__set_bit(RSEQ_EVENT_SIGNAL_BIT, &current->rseq_event_mask);
+	preempt_enable();
+	rseq_handle_notify_resume(ksig, regs);
+}
+
+/* rseq_preempt() requires preemption to be disabled. */
+static inline void rseq_preempt(struct task_struct *t)
+{
+	__set_bit(RSEQ_EVENT_PREEMPT_BIT, &t->rseq_event_mask);
+	rseq_set_notify_resume(t);
+}
+
+/* rseq_migrate() requires preemption to be disabled. */
+static inline void rseq_migrate(struct task_struct *t)
+{
+	__set_bit(RSEQ_EVENT_MIGRATE_BIT, &t->rseq_event_mask);
+	rseq_set_notify_resume(t);
+}
+
+/*
+ * If parent process has a registered restartable sequences area, the
+ * child inherits. Unregister rseq for a clone with CLONE_VM set.
+ */
+static inline void rseq_fork(struct task_struct *t, unsigned long clone_flags)
+{
+	if (clone_flags & CLONE_VM) {
+		t->rseq = NULL;
+		t->rseq_len = 0;
+		t->rseq_sig = 0;
+		t->rseq_event_mask = 0;
+	} else {
+		t->rseq = current->rseq;
+		t->rseq_len = current->rseq_len;
+		t->rseq_sig = current->rseq_sig;
+		t->rseq_event_mask = current->rseq_event_mask;
+	}
+}
+
+static inline void rseq_execve(struct task_struct *t)
+{
+	t->rseq = NULL;
+	t->rseq_len = 0;
+	t->rseq_sig = 0;
+	t->rseq_event_mask = 0;
+}
+
+#else
+
+static inline void rseq_set_notify_resume(struct task_struct *t)
+{
+}
+static inline void rseq_handle_notify_resume(struct ksignal *ksig,
+					     struct pt_regs *regs)
+{
+}
+static inline void rseq_signal_deliver(struct ksignal *ksig,
+				       struct pt_regs *regs)
+{
+}
+static inline void rseq_preempt(struct task_struct *t)
+{
+}
+static inline void rseq_migrate(struct task_struct *t)
+{
+}
+static inline void rseq_fork(struct task_struct *t, unsigned long clone_flags)
+{
+}
+static inline void rseq_execve(struct task_struct *t)
+{
+}
+
+#endif
+
+#ifdef CONFIG_DEBUG_RSEQ
+
+void rseq_syscall(struct pt_regs *regs);
+
+#else
+
+static inline void rseq_syscall(struct pt_regs *regs)
+{
+}
+
+#endif
+
+#endif /* _LINUX_RSEQ_H */
diff --git a/include/linux/sched.h b/include/linux/sched.h
index ec739277c39b..d528057c99e4 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -34,7 +34,7 @@
 #include <linux/task_io_accounting.h>
 #include <linux/posix-timers_types.h>
 #include <linux/restart_block.h>
-#include <linux/rseq.h>
+#include <uapi/linux/rseq.h>
 #include <linux/seqlock_types.h>
 #include <linux/kcsan.h>
 #include <linux/rv.h>
@@ -2180,129 +2180,6 @@ static inline bool owner_on_cpu(struct task_struct *owner)
 unsigned long sched_cpu_util(int cpu);
 #endif /* CONFIG_SMP */
 
-#ifdef CONFIG_RSEQ
-
-/*
- * Map the event mask on the user-space ABI enum rseq_cs_flags
- * for direct mask checks.
- */
-enum rseq_event_mask_bits {
-	RSEQ_EVENT_PREEMPT_BIT	= RSEQ_CS_FLAG_NO_RESTART_ON_PREEMPT_BIT,
-	RSEQ_EVENT_SIGNAL_BIT	= RSEQ_CS_FLAG_NO_RESTART_ON_SIGNAL_BIT,
-	RSEQ_EVENT_MIGRATE_BIT	= RSEQ_CS_FLAG_NO_RESTART_ON_MIGRATE_BIT,
-};
-
-enum rseq_event_mask {
-	RSEQ_EVENT_PREEMPT	= (1U << RSEQ_EVENT_PREEMPT_BIT),
-	RSEQ_EVENT_SIGNAL	= (1U << RSEQ_EVENT_SIGNAL_BIT),
-	RSEQ_EVENT_MIGRATE	= (1U << RSEQ_EVENT_MIGRATE_BIT),
-};
-
-static inline void rseq_set_notify_resume(struct task_struct *t)
-{
-	if (t->rseq)
-		set_tsk_thread_flag(t, TIF_NOTIFY_RESUME);
-}
-
-void __rseq_handle_notify_resume(struct ksignal *sig, struct pt_regs *regs);
-
-static inline void rseq_handle_notify_resume(struct ksignal *ksig,
-					     struct pt_regs *regs)
-{
-	if (current->rseq)
-		__rseq_handle_notify_resume(ksig, regs);
-}
-
-static inline void rseq_signal_deliver(struct ksignal *ksig,
-				       struct pt_regs *regs)
-{
-	preempt_disable();
-	__set_bit(RSEQ_EVENT_SIGNAL_BIT, &current->rseq_event_mask);
-	preempt_enable();
-	rseq_handle_notify_resume(ksig, regs);
-}
-
-/* rseq_preempt() requires preemption to be disabled. */
-static inline void rseq_preempt(struct task_struct *t)
-{
-	__set_bit(RSEQ_EVENT_PREEMPT_BIT, &t->rseq_event_mask);
-	rseq_set_notify_resume(t);
-}
-
-/* rseq_migrate() requires preemption to be disabled. */
-static inline void rseq_migrate(struct task_struct *t)
-{
-	__set_bit(RSEQ_EVENT_MIGRATE_BIT, &t->rseq_event_mask);
-	rseq_set_notify_resume(t);
-}
-
-/*
- * If parent process has a registered restartable sequences area, the
- * child inherits. Unregister rseq for a clone with CLONE_VM set.
- */
-static inline void rseq_fork(struct task_struct *t, unsigned long clone_flags)
-{
-	if (clone_flags & CLONE_VM) {
-		t->rseq = NULL;
-		t->rseq_len = 0;
-		t->rseq_sig = 0;
-		t->rseq_event_mask = 0;
-	} else {
-		t->rseq = current->rseq;
-		t->rseq_len = current->rseq_len;
-		t->rseq_sig = current->rseq_sig;
-		t->rseq_event_mask = current->rseq_event_mask;
-	}
-}
-
-static inline void rseq_execve(struct task_struct *t)
-{
-	t->rseq = NULL;
-	t->rseq_len = 0;
-	t->rseq_sig = 0;
-	t->rseq_event_mask = 0;
-}
-
-#else
-
-static inline void rseq_set_notify_resume(struct task_struct *t)
-{
-}
-static inline void rseq_handle_notify_resume(struct ksignal *ksig,
-					     struct pt_regs *regs)
-{
-}
-static inline void rseq_signal_deliver(struct ksignal *ksig,
-				       struct pt_regs *regs)
-{
-}
-static inline void rseq_preempt(struct task_struct *t)
-{
-}
-static inline void rseq_migrate(struct task_struct *t)
-{
-}
-static inline void rseq_fork(struct task_struct *t, unsigned long clone_flags)
-{
-}
-static inline void rseq_execve(struct task_struct *t)
-{
-}
-
-#endif
-
-#ifdef CONFIG_DEBUG_RSEQ
-
-void rseq_syscall(struct pt_regs *regs);
-
-#else
-
-static inline void rseq_syscall(struct pt_regs *regs)
-{
-}
-
-#endif
-
 #ifdef CONFIG_SCHED_CORE
 extern void sched_core_free(struct task_struct *tsk);
 extern void sched_core_fork(struct task_struct *p);
diff --git a/kernel/fork.c b/kernel/fork.c
index 319e61297bfb..53816393995b 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -100,6 +100,7 @@
 #include <linux/stackprotector.h>
 #include <linux/user_events.h>
 #include <linux/iommu.h>
+#include <linux/rseq.h>
 
 #include <asm/pgalloc.h>
 #include <linux/uaccess.h>
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index a708d225c28e..d04cf3c47899 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -57,6 +57,7 @@
 #include <linux/profile.h>
 #include <linux/psi.h>
 #include <linux/rcuwait_api.h>
+#include <linux/rseq.h>
 #include <linux/sched/wake_q.h>
 #include <linux/scs.h>
 #include <linux/slab.h>
-- 
2.43.0
[PATCH 46/50] preempt.h: Kill dependency on list.h
Posted by Kent Overstreet 2 years ago 
We really only need types.h, list.h is big.

Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
---
 include/linux/preempt.h | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/include/linux/preempt.h b/include/linux/preempt.h
index 9aa6358a1a16..7233e9cf1bab 100644
--- a/include/linux/preempt.h
+++ b/include/linux/preempt.h
@@ -9,7 +9,7 @@
 
 #include <linux/linkage.h>
 #include <linux/cleanup.h>
-#include <linux/list.h>
+#include <linux/types.h>
 
 /*
  * We put the hardirq and softirq counter into the preemption
@@ -360,7 +360,9 @@ void preempt_notifier_unregister(struct preempt_notifier *notifier);
 static inline void preempt_notifier_init(struct preempt_notifier *notifier,
 				     struct preempt_ops *ops)
 {
-	INIT_HLIST_NODE(&notifier->link);
+	/* INIT_HLIST_NODE() open coded, to avoid dependency on list.h */
+	notifier->link.next = NULL;
+	notifier->link.pprev = NULL;
 	notifier->ops = ops;
 }
 
-- 
2.43.0
Re: [PATCH 46/50] preempt.h: Kill dependency on list.h
Posted by Matthew Wilcox 2 years ago 
On Fri, Dec 15, 2023 at 10:35:47PM -0500, Kent Overstreet wrote:
> -	INIT_HLIST_NODE(&notifier->link);
> +	/* INIT_HLIST_NODE() open coded, to avoid dependency on list.h */
> +	notifier->link.next = NULL;
> +	notifier->link.pprev = NULL;

Arguably INIT_HLIST_NODE() belongs in types.h -- we already have
RCUREF_INIT() and ATOMIC_INIT() in there.
Re: [PATCH 46/50] preempt.h: Kill dependency on list.h
Posted by Kent Overstreet 2 years ago 
On Sat, Dec 16, 2023 at 06:13:41AM +0000, Matthew Wilcox wrote:
> On Fri, Dec 15, 2023 at 10:35:47PM -0500, Kent Overstreet wrote:
> > -	INIT_HLIST_NODE(&notifier->link);
> > +	/* INIT_HLIST_NODE() open coded, to avoid dependency on list.h */
> > +	notifier->link.next = NULL;
> > +	notifier->link.pprev = NULL;
> 
> Arguably INIT_HLIST_NODE() belongs in types.h -- we already have
> RCUREF_INIT() and ATOMIC_INIT() in there.

I think I'd prefer to keep types.h as minimal as possible - as soon as
we start putting non type stuff in there people won't know what the
distinction is and it'll grow.

preempt.h is a bit unusual too, normally we'd just split out a _types.h
header there but it's not so easy to split up usefully.
Re: [PATCH 46/50] preempt.h: Kill dependency on list.h
Posted by Randy Dunlap 2 years ago 

On 12/16/23 14:35, Kent Overstreet wrote:
> On Sat, Dec 16, 2023 at 06:13:41AM +0000, Matthew Wilcox wrote:
>> On Fri, Dec 15, 2023 at 10:35:47PM -0500, Kent Overstreet wrote:
>>> -	INIT_HLIST_NODE(&notifier->link);
>>> +	/* INIT_HLIST_NODE() open coded, to avoid dependency on list.h */
>>> +	notifier->link.next = NULL;
>>> +	notifier->link.pprev = NULL;
>>
>> Arguably INIT_HLIST_NODE() belongs in types.h -- we already have
>> RCUREF_INIT() and ATOMIC_INIT() in there.
> 
> I think I'd prefer to keep types.h as minimal as possible - as soon as
> we start putting non type stuff in there people won't know what the
> distinction is and it'll grow.
> 
> preempt.h is a bit unusual too, normally we'd just split out a _types.h
> header there but it's not so easy to split up usefully.
> 

I don't feel like I have NAK power, but if I did, I would NAK
open coding of INIT_HLIST_HEAD() or anything like it.
I would expect some $maintainer to do likewise, but I could be
surprised.

-- 
#Randy
https://people.kernel.org/tglx/notes-about-netiquette
https://subspace.kernel.org/etiquette.html
Re: [PATCH 46/50] preempt.h: Kill dependency on list.h
Posted by Matthew Wilcox 2 years ago 
On Sat, Dec 16, 2023 at 04:04:43PM -0800, Randy Dunlap wrote:
> 
> 
> On 12/16/23 14:35, Kent Overstreet wrote:
> > On Sat, Dec 16, 2023 at 06:13:41AM +0000, Matthew Wilcox wrote:
> >> On Fri, Dec 15, 2023 at 10:35:47PM -0500, Kent Overstreet wrote:
> >>> -	INIT_HLIST_NODE(&notifier->link);
> >>> +	/* INIT_HLIST_NODE() open coded, to avoid dependency on list.h */
> >>> +	notifier->link.next = NULL;
> >>> +	notifier->link.pprev = NULL;
> >>
> >> Arguably INIT_HLIST_NODE() belongs in types.h -- we already have
> >> RCUREF_INIT() and ATOMIC_INIT() in there.
> > 
> > I think I'd prefer to keep types.h as minimal as possible - as soon as
> > we start putting non type stuff in there people won't know what the
> > distinction is and it'll grow.
> > 
> > preempt.h is a bit unusual too, normally we'd just split out a _types.h
> > header there but it's not so easy to split up usefully.
> > 
> 
> I don't feel like I have NAK power, but if I did, I would NAK
> open coding of INIT_HLIST_HEAD() or anything like it.
> I would expect some $maintainer to do likewise, but I could be
> surprised.

There is another solution here (although I prefer moving INIT_HLIST_HEAD
into types.h).  The preprocessor allows redefinitions as long as the two
definitions match exactly.  So you can copy INIT_HLIST_HEAD into
preempt.h and if the definition ever changes, we'll notice.
Re: [PATCH 46/50] preempt.h: Kill dependency on list.h
Posted by Kent Overstreet 2 years ago 
On Sun, Dec 17, 2023 at 12:18:17AM +0000, Matthew Wilcox wrote:
> On Sat, Dec 16, 2023 at 04:04:43PM -0800, Randy Dunlap wrote:
> > 
> > 
> > On 12/16/23 14:35, Kent Overstreet wrote:
> > > On Sat, Dec 16, 2023 at 06:13:41AM +0000, Matthew Wilcox wrote:
> > >> On Fri, Dec 15, 2023 at 10:35:47PM -0500, Kent Overstreet wrote:
> > >>> -	INIT_HLIST_NODE(&notifier->link);
> > >>> +	/* INIT_HLIST_NODE() open coded, to avoid dependency on list.h */
> > >>> +	notifier->link.next = NULL;
> > >>> +	notifier->link.pprev = NULL;
> > >>
> > >> Arguably INIT_HLIST_NODE() belongs in types.h -- we already have
> > >> RCUREF_INIT() and ATOMIC_INIT() in there.
> > > 
> > > I think I'd prefer to keep types.h as minimal as possible - as soon as
> > > we start putting non type stuff in there people won't know what the
> > > distinction is and it'll grow.
> > > 
> > > preempt.h is a bit unusual too, normally we'd just split out a _types.h
> > > header there but it's not so easy to split up usefully.
> > > 
> > 
> > I don't feel like I have NAK power, but if I did, I would NAK
> > open coding of INIT_HLIST_HEAD() or anything like it.
> > I would expect some $maintainer to do likewise, but I could be
> > surprised.
> 
> There is another solution here (although I prefer moving INIT_HLIST_HEAD
> into types.h).  The preprocessor allows redefinitions as long as the two
> definitions match exactly.  So you can copy INIT_HLIST_HEAD into
> preempt.h and if the definition ever changes, we'll notice.

I like it.
Re: [PATCH 46/50] preempt.h: Kill dependency on list.h
Posted by Randy Dunlap 2 years ago 

On 12/16/23 16:20, Kent Overstreet wrote:
> On Sun, Dec 17, 2023 at 12:18:17AM +0000, Matthew Wilcox wrote:
>> On Sat, Dec 16, 2023 at 04:04:43PM -0800, Randy Dunlap wrote:
>>>
>>>
>>> On 12/16/23 14:35, Kent Overstreet wrote:
>>>> On Sat, Dec 16, 2023 at 06:13:41AM +0000, Matthew Wilcox wrote:
>>>>> On Fri, Dec 15, 2023 at 10:35:47PM -0500, Kent Overstreet wrote:
>>>>>> -	INIT_HLIST_NODE(&notifier->link);
>>>>>> +	/* INIT_HLIST_NODE() open coded, to avoid dependency on list.h */
>>>>>> +	notifier->link.next = NULL;
>>>>>> +	notifier->link.pprev = NULL;
>>>>>
>>>>> Arguably INIT_HLIST_NODE() belongs in types.h -- we already have
>>>>> RCUREF_INIT() and ATOMIC_INIT() in there.
>>>>
>>>> I think I'd prefer to keep types.h as minimal as possible - as soon as
>>>> we start putting non type stuff in there people won't know what the
>>>> distinction is and it'll grow.
>>>>
>>>> preempt.h is a bit unusual too, normally we'd just split out a _types.h
>>>> header there but it's not so easy to split up usefully.
>>>>
>>>
>>> I don't feel like I have NAK power, but if I did, I would NAK
>>> open coding of INIT_HLIST_HEAD() or anything like it.
>>> I would expect some $maintainer to do likewise, but I could be
>>> surprised.
>>
>> There is another solution here (although I prefer moving INIT_HLIST_HEAD
>> into types.h).  The preprocessor allows redefinitions as long as the two
>> definitions match exactly.  So you can copy INIT_HLIST_HEAD into
>> preempt.h and if the definition ever changes, we'll notice.
> 
> I like it.

Possible to revert 490d6ab170c9 ? although with something list
this inserted:

	struct hlist_node *_p = h;
and then use _p instead of h (or the old macro's 'ptr')

The code looks the same to me, although I could have mucked something
up:     https://godbolt.org/z/z76nsqGx3

although Andrew prefers inlines for type checking.


-- 
#Randy
https://people.kernel.org/tglx/notes-about-netiquette
https://subspace.kernel.org/etiquette.html
Re: [PATCH 46/50] preempt.h: Kill dependency on list.h
Posted by Kent Overstreet 2 years ago 
On Sat, Dec 16, 2023 at 06:03:36PM -0800, Randy Dunlap wrote:
> 
> 
> On 12/16/23 16:20, Kent Overstreet wrote:
> > On Sun, Dec 17, 2023 at 12:18:17AM +0000, Matthew Wilcox wrote:
> >> On Sat, Dec 16, 2023 at 04:04:43PM -0800, Randy Dunlap wrote:
> >>>
> >>>
> >>> On 12/16/23 14:35, Kent Overstreet wrote:
> >>>> On Sat, Dec 16, 2023 at 06:13:41AM +0000, Matthew Wilcox wrote:
> >>>>> On Fri, Dec 15, 2023 at 10:35:47PM -0500, Kent Overstreet wrote:
> >>>>>> -	INIT_HLIST_NODE(&notifier->link);
> >>>>>> +	/* INIT_HLIST_NODE() open coded, to avoid dependency on list.h */
> >>>>>> +	notifier->link.next = NULL;
> >>>>>> +	notifier->link.pprev = NULL;
> >>>>>
> >>>>> Arguably INIT_HLIST_NODE() belongs in types.h -- we already have
> >>>>> RCUREF_INIT() and ATOMIC_INIT() in there.
> >>>>
> >>>> I think I'd prefer to keep types.h as minimal as possible - as soon as
> >>>> we start putting non type stuff in there people won't know what the
> >>>> distinction is and it'll grow.
> >>>>
> >>>> preempt.h is a bit unusual too, normally we'd just split out a _types.h
> >>>> header there but it's not so easy to split up usefully.
> >>>>
> >>>
> >>> I don't feel like I have NAK power, but if I did, I would NAK
> >>> open coding of INIT_HLIST_HEAD() or anything like it.
> >>> I would expect some $maintainer to do likewise, but I could be
> >>> surprised.
> >>
> >> There is another solution here (although I prefer moving INIT_HLIST_HEAD
> >> into types.h).  The preprocessor allows redefinitions as long as the two
> >> definitions match exactly.  So you can copy INIT_HLIST_HEAD into
> >> preempt.h and if the definition ever changes, we'll notice.
> > 
> > I like it.
> 
> Possible to revert 490d6ab170c9 ? although with something list
> this inserted:
> 
> 	struct hlist_node *_p = h;
> and then use _p instead of h (or the old macro's 'ptr')
> 
> The code looks the same to me, although I could have mucked something
> up:     https://godbolt.org/z/z76nsqGx3
> 
> although Andrew prefers inlines for type checking.

I prefer inlines whenever possible too, a macro should really be a
signal that 'something interesting is going on here'.

I'm just going with my original version.
Re: [PATCH 46/50] preempt.h: Kill dependency on list.h
Posted by Kent Overstreet 2 years ago 
On Sat, Dec 16, 2023 at 04:04:43PM -0800, Randy Dunlap wrote:
> 
> 
> On 12/16/23 14:35, Kent Overstreet wrote:
> > On Sat, Dec 16, 2023 at 06:13:41AM +0000, Matthew Wilcox wrote:
> >> On Fri, Dec 15, 2023 at 10:35:47PM -0500, Kent Overstreet wrote:
> >>> -	INIT_HLIST_NODE(&notifier->link);
> >>> +	/* INIT_HLIST_NODE() open coded, to avoid dependency on list.h */
> >>> +	notifier->link.next = NULL;
> >>> +	notifier->link.pprev = NULL;
> >>
> >> Arguably INIT_HLIST_NODE() belongs in types.h -- we already have
> >> RCUREF_INIT() and ATOMIC_INIT() in there.
> > 
> > I think I'd prefer to keep types.h as minimal as possible - as soon as
> > we start putting non type stuff in there people won't know what the
> > distinction is and it'll grow.
> > 
> > preempt.h is a bit unusual too, normally we'd just split out a _types.h
> > header there but it's not so easy to split up usefully.
> > 
> 
> I don't feel like I have NAK power, but if I did, I would NAK
> open coding of INIT_HLIST_HEAD() or anything like it.
> I would expect some $maintainer to do likewise, but I could be
> surprised.

It's INIT_HLIST_HEAD(), there's approximately zero chance of the
implementation changing, and there's a comment.
Re: [PATCH 46/50] preempt.h: Kill dependency on list.h
Posted by Randy Dunlap 2 years ago 

On 12/16/23 16:18, Kent Overstreet wrote:
> On Sat, Dec 16, 2023 at 04:04:43PM -0800, Randy Dunlap wrote:
>>
>>
>> On 12/16/23 14:35, Kent Overstreet wrote:
>>> On Sat, Dec 16, 2023 at 06:13:41AM +0000, Matthew Wilcox wrote:
>>>> On Fri, Dec 15, 2023 at 10:35:47PM -0500, Kent Overstreet wrote:
>>>>> -	INIT_HLIST_NODE(&notifier->link);
>>>>> +	/* INIT_HLIST_NODE() open coded, to avoid dependency on list.h */
>>>>> +	notifier->link.next = NULL;
>>>>> +	notifier->link.pprev = NULL;
>>>>
>>>> Arguably INIT_HLIST_NODE() belongs in types.h -- we already have
>>>> RCUREF_INIT() and ATOMIC_INIT() in there.
>>>
>>> I think I'd prefer to keep types.h as minimal as possible - as soon as
>>> we start putting non type stuff in there people won't know what the
>>> distinction is and it'll grow.
>>>
>>> preempt.h is a bit unusual too, normally we'd just split out a _types.h
>>> header there but it's not so easy to split up usefully.
>>>
>>
>> I don't feel like I have NAK power, but if I did, I would NAK
>> open coding of INIT_HLIST_HEAD() or anything like it.
>> I would expect some $maintainer to do likewise, but I could be
>> surprised.
> 
> It's INIT_HLIST_HEAD(), there's approximately zero chance of the
> implementation changing, and there's a comment.

s/_HEAD/_NODE/ for both of us.  :)

-- 
#Randy
https://people.kernel.org/tglx/notes-about-netiquette
https://subspace.kernel.org/etiquette.html
Re: [PATCH 46/50] preempt.h: Kill dependency on list.h
Posted by Randy Dunlap 2 years ago 

On 12/15/23 22:13, Matthew Wilcox wrote:
> On Fri, Dec 15, 2023 at 10:35:47PM -0500, Kent Overstreet wrote:
>> -	INIT_HLIST_NODE(&notifier->link);
>> +	/* INIT_HLIST_NODE() open coded, to avoid dependency on list.h */
>> +	notifier->link.next = NULL;
>> +	notifier->link.pprev = NULL;
> 
> Arguably INIT_HLIST_NODE() belongs in types.h -- we already have
> RCUREF_INIT() and ATOMIC_INIT() in there.
> 

That would be far better than open-coding it.

-- 
#Randy
https://people.kernel.org/tglx/notes-about-netiquette
https://subspace.kernel.org/etiquette.html
[PATCH 47/50] thread_info, uaccess.h: Move HARDENED_USERCOPY to better location
Posted by Kent Overstreet 2 years ago 
thread_info.h is needed by sched.h, and we're trying to slim down
dependencies there - bug.h is a big one.

And the HARDENED_USERCOPY stuff is used in uaccess.h, so it makes more
sense there anyways.

Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
---
 include/linux/thread_info.h | 49 -------------------------------------
 include/linux/uaccess.h     | 49 +++++++++++++++++++++++++++++++++++++
 include/linux/uio.h         |  2 +-
 3 files changed, 50 insertions(+), 50 deletions(-)

diff --git a/include/linux/thread_info.h b/include/linux/thread_info.h
index 9ea0b28068f4..85d99c556cb5 100644
--- a/include/linux/thread_info.h
+++ b/include/linux/thread_info.h
@@ -10,7 +10,6 @@
 
 #include <linux/types.h>
 #include <linux/limits.h>
-#include <linux/bug.h>
 #include <linux/restart_block.h>
 #include <linux/errno.h>
 
@@ -204,54 +203,6 @@ static inline int arch_within_stack_frames(const void * const stack,
 }
 #endif
 
-#ifdef CONFIG_HARDENED_USERCOPY
-extern void __check_object_size(const void *ptr, unsigned long n,
-					bool to_user);
-
-static __always_inline void check_object_size(const void *ptr, unsigned long n,
-					      bool to_user)
-{
-	if (!__builtin_constant_p(n))
-		__check_object_size(ptr, n, to_user);
-}
-#else
-static inline void check_object_size(const void *ptr, unsigned long n,
-				     bool to_user)
-{ }
-#endif /* CONFIG_HARDENED_USERCOPY */
-
-extern void __compiletime_error("copy source size is too small")
-__bad_copy_from(void);
-extern void __compiletime_error("copy destination size is too small")
-__bad_copy_to(void);
-
-void __copy_overflow(int size, unsigned long count);
-
-static inline void copy_overflow(int size, unsigned long count)
-{
-	if (IS_ENABLED(CONFIG_BUG))
-		__copy_overflow(size, count);
-}
-
-static __always_inline __must_check bool
-check_copy_size(const void *addr, size_t bytes, bool is_source)
-{
-	int sz = __builtin_object_size(addr, 0);
-	if (unlikely(sz >= 0 && sz < bytes)) {
-		if (!__builtin_constant_p(bytes))
-			copy_overflow(sz, bytes);
-		else if (is_source)
-			__bad_copy_from();
-		else
-			__bad_copy_to();
-		return false;
-	}
-	if (WARN_ON_ONCE(bytes > INT_MAX))
-		return false;
-	check_object_size(addr, bytes, is_source);
-	return true;
-}
-
 #ifndef arch_setup_new_exec
 static inline void arch_setup_new_exec(void) { }
 #endif
diff --git a/include/linux/uaccess.h b/include/linux/uaccess.h
index 3064314f4832..3e93ee64d6f8 100644
--- a/include/linux/uaccess.h
+++ b/include/linux/uaccess.h
@@ -2,6 +2,7 @@
 #ifndef __LINUX_UACCESS_H__
 #define __LINUX_UACCESS_H__
 
+#include <linux/bug.h>
 #include <linux/fault-inject-usercopy.h>
 #include <linux/instrumented.h>
 #include <linux/minmax.h>
@@ -32,6 +33,54 @@
 })
 #endif
 
+#ifdef CONFIG_HARDENED_USERCOPY
+extern void __check_object_size(const void *ptr, unsigned long n,
+					bool to_user);
+
+static __always_inline void check_object_size(const void *ptr, unsigned long n,
+					      bool to_user)
+{
+	if (!__builtin_constant_p(n))
+		__check_object_size(ptr, n, to_user);
+}
+#else
+static inline void check_object_size(const void *ptr, unsigned long n,
+				     bool to_user)
+{ }
+#endif /* CONFIG_HARDENED_USERCOPY */
+
+extern void __compiletime_error("copy source size is too small")
+__bad_copy_from(void);
+extern void __compiletime_error("copy destination size is too small")
+__bad_copy_to(void);
+
+void __copy_overflow(int size, unsigned long count);
+
+static inline void copy_overflow(int size, unsigned long count)
+{
+	if (IS_ENABLED(CONFIG_BUG))
+		__copy_overflow(size, count);
+}
+
+static __always_inline __must_check bool
+check_copy_size(const void *addr, size_t bytes, bool is_source)
+{
+	int sz = __builtin_object_size(addr, 0);
+	if (unlikely(sz >= 0 && sz < bytes)) {
+		if (!__builtin_constant_p(bytes))
+			copy_overflow(sz, bytes);
+		else if (is_source)
+			__bad_copy_from();
+		else
+			__bad_copy_to();
+		return false;
+	}
+	if (WARN_ON_ONCE(bytes > INT_MAX))
+		return false;
+	check_object_size(addr, bytes, is_source);
+	return true;
+}
+
 /*
  * Architectures should provide two primitives (raw_copy_{to,from}_user())
  * and get rid of their private instances of copy_{to,from}_user() and
diff --git a/include/linux/uio.h b/include/linux/uio.h
index b6214cbf2a43..084262b68106 100644
--- a/include/linux/uio.h
+++ b/include/linux/uio.h
@@ -6,7 +6,7 @@
 #define __LINUX_UIO_H
 
 #include <linux/kernel.h>
-#include <linux/thread_info.h>
+#include <linux/uaccess.h>
 #include <linux/mm_types.h>
 #include <uapi/linux/uio.h>
 
-- 
2.43.0
[PATCH 48/50] Kill unnecessary kernel.h include
Posted by Kent Overstreet 2 years ago 
More trimming down unnecessary includes.

Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
---
 arch/x86/include/asm/current.h | 1 +
 arch/x86/include/asm/percpu.h  | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/current.h b/arch/x86/include/asm/current.h
index a1168e7b69e5..dd4b67101bb7 100644
--- a/arch/x86/include/asm/current.h
+++ b/arch/x86/include/asm/current.h
@@ -2,6 +2,7 @@
 #ifndef _ASM_X86_CURRENT_H
 #define _ASM_X86_CURRENT_H
 
+#include <linux/build_bug.h>
 #include <linux/compiler.h>
 
 #ifndef __ASSEMBLY__
diff --git a/arch/x86/include/asm/percpu.h b/arch/x86/include/asm/percpu.h
index 20624b80f890..5e01883eb51e 100644
--- a/arch/x86/include/asm/percpu.h
+++ b/arch/x86/include/asm/percpu.h
@@ -24,8 +24,8 @@
 
 #else /* ...!ASSEMBLY */
 
-#include <linux/kernel.h>
 #include <linux/stringify.h>
+#include <asm/asm.h>
 
 #ifdef CONFIG_SMP
 #define __percpu_prefix		"%%"__stringify(__percpu_seg)":"
-- 
2.43.0
[PATCH 49/50] kill unnecessary thread_info.h include
Posted by Kent Overstreet 2 years ago 
Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
---
 arch/x86/include/asm/fpu/types.h | 2 ++
 arch/x86/include/asm/preempt.h   | 1 -
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/fpu/types.h b/arch/x86/include/asm/fpu/types.h
index eb810074f1e7..3dad7cf25505 100644
--- a/arch/x86/include/asm/fpu/types.h
+++ b/arch/x86/include/asm/fpu/types.h
@@ -5,6 +5,8 @@
 #ifndef _ASM_X86_FPU_H
 #define _ASM_X86_FPU_H
 
+#include <asm/page_types.h>
+
 /*
  * The legacy x87 FPU state format, as saved by FSAVE and
  * restored by the FRSTOR instructions:
diff --git a/arch/x86/include/asm/preempt.h b/arch/x86/include/asm/preempt.h
index 4527e1430c6d..af77235fded6 100644
--- a/arch/x86/include/asm/preempt.h
+++ b/arch/x86/include/asm/preempt.h
@@ -6,7 +6,6 @@
 #include <asm/percpu.h>
 #include <asm/current.h>
 
-#include <linux/thread_info.h>
 #include <linux/static_call_types.h>
 
 /* We use the MSB mostly because its available */
-- 
2.43.0
[PATCH 50/50] Kill sched.h dependency on rcupdate.h
Posted by Kent Overstreet 2 years ago 
by moving cond_resched_rcu() to rcupdate.h, we can kill another big
sched.h dependency.

Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
---
 include/linux/rcupdate.h | 11 +++++++++++
 include/linux/sched.h    | 13 +++----------
 2 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
index f7206b2623c9..8ebfa57e0164 100644
--- a/include/linux/rcupdate.h
+++ b/include/linux/rcupdate.h
@@ -1058,4 +1058,15 @@ extern int rcu_normal;
 
 DEFINE_LOCK_GUARD_0(rcu, rcu_read_lock(), rcu_read_unlock())
 
+#if defined(CONFIG_DEBUG_ATOMIC_SLEEP) || !defined(CONFIG_PREEMPT_RCU)
+#define cond_resched_rcu()		\
+do {					\
+	rcu_read_unlock();		\
+	cond_resched();			\
+	rcu_read_lock();		\
+} while (0)
+#else
+#define cond_resched_rcu()
+#endif
+
 #endif /* __LINUX_RCUPDATE_H */
diff --git a/include/linux/sched.h b/include/linux/sched.h
index d528057c99e4..b781ac7e0a02 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -10,8 +10,11 @@
 #include <uapi/linux/sched.h>
 
 #include <asm/current.h>
+#include <linux/thread_info.h>
+#include <linux/preempt.h>
 
 #include <linux/irqflags_types.h>
+#include <linux/smp_types.h>
 #include <linux/pid_types.h>
 #include <linux/sem_types.h>
 #include <linux/shm.h>
@@ -22,7 +25,6 @@
 #include <linux/timer_types.h>
 #include <linux/seccomp_types.h>
 #include <linux/nodemask_types.h>
-#include <linux/rcupdate.h>
 #include <linux/refcount_types.h>
 #include <linux/resource.h>
 #include <linux/latencytop.h>
@@ -2058,15 +2060,6 @@ extern int __cond_resched_rwlock_write(rwlock_t *lock);
 	__cond_resched_rwlock_write(lock);					\
 })
 
-static inline void cond_resched_rcu(void)
-{
-#if defined(CONFIG_DEBUG_ATOMIC_SLEEP) || !defined(CONFIG_PREEMPT_RCU)
-	rcu_read_unlock();
-	cond_resched();
-	rcu_read_lock();
-#endif
-}
-
 #ifdef CONFIG_PREEMPT_DYNAMIC
 
 extern bool preempt_model_none(void);
-- 
2.43.0
Re: [PATCH 50/50] Kill sched.h dependency on rcupdate.h
Posted by Geert Uytterhoeven 2 years ago 
Hi Kent,

On Sat, Dec 16, 2023 at 4:39 AM Kent Overstreet
<kent.overstreet@linux.dev> wrote:
> by moving cond_resched_rcu() to rcupdate.h, we can kill another big
> sched.h dependency.
>
> Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>

Thanks for your patch, which is now commit dc00f26faea81dc0 ("Kill
sched.h dependency on rcupdate.h") in next-20231220.

Reported-by: noreply@ellerman.id.au

$ make ARCH=m68k defconfig arch/m68k/kernel/asm-offsets.i
*** Default configuration is based on 'multi_defconfig'
#
# No change to .config
#
  UPD     include/config/kernel.release
  UPD     include/generated/utsrelease.h
  CC      arch/m68k/kernel/asm-offsets.s
In file included from ./include/asm-generic/bug.h:7,
                 from ./arch/m68k/include/asm/bug.h:32,
                 from ./include/linux/bug.h:5,
                 from ./include/linux/thread_info.h:13,
                 from ./arch/m68k/include/asm/processor.h:11,
                 from ./include/linux/sched.h:13,
                 from arch/m68k/kernel/asm-offsets.c:15:
./arch/m68k/include/asm/processor.h: In function ‘set_fc’:
./arch/m68k/include/asm/processor.h:91:15: error: implicit declaration
of function ‘in_interrupt’ [-Werror=implicit-function-declaration]
   91 |  WARN_ON_ONCE(in_interrupt());
      |               ^~~~~~~~~~~~
./include/linux/once_lite.h:28:27: note: in definition of macro
‘DO_ONCE_LITE_IF’
   28 |   bool __ret_do_once = !!(condition);   \
      |                           ^~~~~~~~~
./arch/m68k/include/asm/processor.h:91:2: note: in expansion of macro
‘WARN_ON_ONCE’
   91 |  WARN_ON_ONCE(in_interrupt());
      |  ^~~~~~~~~~~~
cc1: some warnings being treated as errors
make[3]: *** [scripts/Makefile.build:116:
arch/m68k/kernel/asm-offsets.s] Error 1
make[2]: *** [Makefile:1191: prepare0] Error 2
make[1]: *** [Makefile:350: __build_one_by_one] Error 2
make: *** [Makefile:234: __sub-make] Error 2

> --- a/include/linux/rcupdate.h
> +++ b/include/linux/rcupdate.h
> @@ -1058,4 +1058,15 @@ extern int rcu_normal;
>
>  DEFINE_LOCK_GUARD_0(rcu, rcu_read_lock(), rcu_read_unlock())
>
> +#if defined(CONFIG_DEBUG_ATOMIC_SLEEP) || !defined(CONFIG_PREEMPT_RCU)
> +#define cond_resched_rcu()             \
> +do {                                   \
> +       rcu_read_unlock();              \
> +       cond_resched();                 \
> +       rcu_read_lock();                \
> +} while (0)
> +#else
> +#define cond_resched_rcu()
> +#endif
> +
>  #endif /* __LINUX_RCUPDATE_H */
> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index d528057c99e4..b781ac7e0a02 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -10,8 +10,11 @@
>  #include <uapi/linux/sched.h>
>
>  #include <asm/current.h>
> +#include <linux/thread_info.h>
> +#include <linux/preempt.h>
>
>  #include <linux/irqflags_types.h>
> +#include <linux/smp_types.h>
>  #include <linux/pid_types.h>
>  #include <linux/sem_types.h>
>  #include <linux/shm.h>
> @@ -22,7 +25,6 @@
>  #include <linux/timer_types.h>
>  #include <linux/seccomp_types.h>
>  #include <linux/nodemask_types.h>
> -#include <linux/rcupdate.h>
>  #include <linux/refcount_types.h>
>  #include <linux/resource.h>
>  #include <linux/latencytop.h>
> @@ -2058,15 +2060,6 @@ extern int __cond_resched_rwlock_write(rwlock_t *lock);
>         __cond_resched_rwlock_write(lock);                                      \
>  })
>
> -static inline void cond_resched_rcu(void)
> -{
> -#if defined(CONFIG_DEBUG_ATOMIC_SLEEP) || !defined(CONFIG_PREEMPT_RCU)
> -       rcu_read_unlock();
> -       cond_resched();
> -       rcu_read_lock();
> -#endif
> -}
> -
>  #ifdef CONFIG_PREEMPT_DYNAMIC
>
>  extern bool preempt_model_none(void);

Gr{oetje,eeting}s,

                        Geert


--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
Re: [PATCH 50/50] Kill sched.h dependency on rcupdate.h
Posted by Kent Overstreet 2 years ago 
On Wed, Dec 20, 2023 at 12:59:44PM +0100, Geert Uytterhoeven wrote:
> Hi Kent,
> 
> On Sat, Dec 16, 2023 at 4:39 AM Kent Overstreet
> <kent.overstreet@linux.dev> wrote:
> > by moving cond_resched_rcu() to rcupdate.h, we can kill another big
> > sched.h dependency.
> >
> > Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
> 
> Thanks for your patch, which is now commit dc00f26faea81dc0 ("Kill
> sched.h dependency on rcupdate.h") in next-20231220.
> 
> Reported-by: noreply@ellerman.id.au
> 
> $ make ARCH=m68k defconfig arch/m68k/kernel/asm-offsets.i
> *** Default configuration is based on 'multi_defconfig'
> #
> # No change to .config
> #
>   UPD     include/config/kernel.release
>   UPD     include/generated/utsrelease.h
>   CC      arch/m68k/kernel/asm-offsets.s
> In file included from ./include/asm-generic/bug.h:7,
>                  from ./arch/m68k/include/asm/bug.h:32,
>                  from ./include/linux/bug.h:5,
>                  from ./include/linux/thread_info.h:13,
>                  from ./arch/m68k/include/asm/processor.h:11,
>                  from ./include/linux/sched.h:13,
>                  from arch/m68k/kernel/asm-offsets.c:15:
> ./arch/m68k/include/asm/processor.h: In function ‘set_fc’:
> ./arch/m68k/include/asm/processor.h:91:15: error: implicit declaration
> of function ‘in_interrupt’ [-Werror=implicit-function-declaration]
>    91 |  WARN_ON_ONCE(in_interrupt());
>       |               ^~~~~~~~~~~~
> ./include/linux/once_lite.h:28:27: note: in definition of macro
> ‘DO_ONCE_LITE_IF’
>    28 |   bool __ret_do_once = !!(condition);   \
>       |                           ^~~~~~~~~
> ./arch/m68k/include/asm/processor.h:91:2: note: in expansion of macro
> ‘WARN_ON_ONCE’
>    91 |  WARN_ON_ONCE(in_interrupt());
>       |  ^~~~~~~~~~~~
> cc1: some warnings being treated as errors
> make[3]: *** [scripts/Makefile.build:116:
> arch/m68k/kernel/asm-offsets.s] Error 1
> make[2]: *** [Makefile:1191: prepare0] Error 2
> make[1]: *** [Makefile:350: __build_one_by_one] Error 2
> make: *** [Makefile:234: __sub-make] Error 2

Applying this fix:

commit 0d7bdfe9726b275c7e9398047763a144c790b575
Author: Kent Overstreet <kent.overstreet@linux.dev>
Date:   Wed Dec 20 16:39:21 2023 -0500

    m68k: Fix missing include
    
    Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>

diff --git a/arch/m68k/include/asm/processor.h b/arch/m68k/include/asm/processor.h
index 7a2da780830b..8f2676c3a988 100644
--- a/arch/m68k/include/asm/processor.h
+++ b/arch/m68k/include/asm/processor.h
@@ -8,6 +8,7 @@
 #ifndef __ASM_M68K_PROCESSOR_H
 #define __ASM_M68K_PROCESSOR_H
 
+#include <linux/preempt.h>
 #include <linux/thread_info.h>
 #include <asm/fpu.h>
 #include <asm/ptrace.h>
Re: [PATCH 50/50] Kill sched.h dependency on rcupdate.h
Posted by Geert Uytterhoeven 1 year, 12 months ago 
Hi Kent,

On Wed, Dec 20, 2023 at 10:40 PM Kent Overstreet
<kent.overstreet@linux.dev> wrote:
> On Wed, Dec 20, 2023 at 12:59:44PM +0100, Geert Uytterhoeven wrote:
> > On Sat, Dec 16, 2023 at 4:39 AM Kent Overstreet
> > <kent.overstreet@linux.dev> wrote:
> > > by moving cond_resched_rcu() to rcupdate.h, we can kill another big
> > > sched.h dependency.
> > >
> > > Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
> >
> > Thanks for your patch, which is now commit dc00f26faea81dc0 ("Kill
> > sched.h dependency on rcupdate.h") in next-20231220.
> >
> > Reported-by: noreply@ellerman.id.au
> >
> > $ make ARCH=m68k defconfig arch/m68k/kernel/asm-offsets.i
> > *** Default configuration is based on 'multi_defconfig'
> > #
> > # No change to .config
> > #
> >   UPD     include/config/kernel.release
> >   UPD     include/generated/utsrelease.h
> >   CC      arch/m68k/kernel/asm-offsets.s
> > In file included from ./include/asm-generic/bug.h:7,
> >                  from ./arch/m68k/include/asm/bug.h:32,
> >                  from ./include/linux/bug.h:5,
> >                  from ./include/linux/thread_info.h:13,
> >                  from ./arch/m68k/include/asm/processor.h:11,
> >                  from ./include/linux/sched.h:13,
> >                  from arch/m68k/kernel/asm-offsets.c:15:
> > ./arch/m68k/include/asm/processor.h: In function ‘set_fc’:
> > ./arch/m68k/include/asm/processor.h:91:15: error: implicit declaration
> > of function ‘in_interrupt’ [-Werror=implicit-function-declaration]
> >    91 |  WARN_ON_ONCE(in_interrupt());
> >       |               ^~~~~~~~~~~~
> > ./include/linux/once_lite.h:28:27: note: in definition of macro
> > ‘DO_ONCE_LITE_IF’
> >    28 |   bool __ret_do_once = !!(condition);   \
> >       |                           ^~~~~~~~~
> > ./arch/m68k/include/asm/processor.h:91:2: note: in expansion of macro
> > ‘WARN_ON_ONCE’
> >    91 |  WARN_ON_ONCE(in_interrupt());
> >       |  ^~~~~~~~~~~~
> > cc1: some warnings being treated as errors
> > make[3]: *** [scripts/Makefile.build:116:
> > arch/m68k/kernel/asm-offsets.s] Error 1
> > make[2]: *** [Makefile:1191: prepare0] Error 2
> > make[1]: *** [Makefile:350: __build_one_by_one] Error 2
> > make: *** [Makefile:234: __sub-make] Error 2
>
> Applying this fix:
>
> commit 0d7bdfe9726b275c7e9398047763a144c790b575
> Author: Kent Overstreet <kent.overstreet@linux.dev>
> Date:   Wed Dec 20 16:39:21 2023 -0500
>
>     m68k: Fix missing include
>
>     Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>

LGTM.
Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
Re: [PATCH 50/50] Kill sched.h dependency on rcupdate.h
Posted by Paul E. McKenney 2 years ago 
On Fri, Dec 15, 2023 at 10:35:51PM -0500, Kent Overstreet wrote:
> by moving cond_resched_rcu() to rcupdate.h, we can kill another big
> sched.h dependency.
> 
> Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>

Could you please instead move the cond_resched_rcu() function to
include/linux/rcupdate_wait.h?  This would avoid breaking Ingo's
separation that makes it possible to include rcupdate.h without also
pulling in sched.h.

							Thanx, Paul

> ---
>  include/linux/rcupdate.h | 11 +++++++++++
>  include/linux/sched.h    | 13 +++----------
>  2 files changed, 14 insertions(+), 10 deletions(-)
> 
> diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
> index f7206b2623c9..8ebfa57e0164 100644
> --- a/include/linux/rcupdate.h
> +++ b/include/linux/rcupdate.h
> @@ -1058,4 +1058,15 @@ extern int rcu_normal;
>  
>  DEFINE_LOCK_GUARD_0(rcu, rcu_read_lock(), rcu_read_unlock())
>  
> +#if defined(CONFIG_DEBUG_ATOMIC_SLEEP) || !defined(CONFIG_PREEMPT_RCU)
> +#define cond_resched_rcu()		\
> +do {					\
> +	rcu_read_unlock();		\
> +	cond_resched();			\
> +	rcu_read_lock();		\
> +} while (0)
> +#else
> +#define cond_resched_rcu()
> +#endif
> +
>  #endif /* __LINUX_RCUPDATE_H */
> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index d528057c99e4..b781ac7e0a02 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -10,8 +10,11 @@
>  #include <uapi/linux/sched.h>
>  
>  #include <asm/current.h>
> +#include <linux/thread_info.h>
> +#include <linux/preempt.h>
>  
>  #include <linux/irqflags_types.h>
> +#include <linux/smp_types.h>
>  #include <linux/pid_types.h>
>  #include <linux/sem_types.h>
>  #include <linux/shm.h>
> @@ -22,7 +25,6 @@
>  #include <linux/timer_types.h>
>  #include <linux/seccomp_types.h>
>  #include <linux/nodemask_types.h>
> -#include <linux/rcupdate.h>
>  #include <linux/refcount_types.h>
>  #include <linux/resource.h>
>  #include <linux/latencytop.h>
> @@ -2058,15 +2060,6 @@ extern int __cond_resched_rwlock_write(rwlock_t *lock);
>  	__cond_resched_rwlock_write(lock);					\
>  })
>  
> -static inline void cond_resched_rcu(void)
> -{
> -#if defined(CONFIG_DEBUG_ATOMIC_SLEEP) || !defined(CONFIG_PREEMPT_RCU)
> -	rcu_read_unlock();
> -	cond_resched();
> -	rcu_read_lock();
> -#endif
> -}
> -
>  #ifdef CONFIG_PREEMPT_DYNAMIC
>  
>  extern bool preempt_model_none(void);
> -- 
> 2.43.0
>
Re: [PATCH 50/50] Kill sched.h dependency on rcupdate.h
Posted by Kent Overstreet 2 years ago 
On Sat, Dec 16, 2023 at 11:35:04AM -0800, Paul E. McKenney wrote:
> On Fri, Dec 15, 2023 at 10:35:51PM -0500, Kent Overstreet wrote:
> > by moving cond_resched_rcu() to rcupdate.h, we can kill another big
> > sched.h dependency.
> > 
> > Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
> 
> Could you please instead move the cond_resched_rcu() function to
> include/linux/rcupdate_wait.h?  This would avoid breaking Ingo's
> separation that makes it possible to include rcupdate.h without also
> pulling in sched.h.

Yep, will do.

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.