* [PATCH tip v6 1/5] wait.[ch]: Introduce the simple waitqueue (swait) implementation
2016-01-28 14:44 [PATCH tip v6 0/5] Simple wait queue support Daniel Wagner
@ 2016-01-28 14:44 ` Daniel Wagner
2016-01-28 14:44 ` [PATCH tip v6 2/5] kbuild: Add option to turn incompatible pointer check into error Daniel Wagner
` (5 subsequent siblings)
6 siblings, 0 replies; 72+ messages in thread
From: Daniel Wagner @ 2016-01-28 14:44 UTC (permalink / raw)
To: linux-kernel, linux-rt-users
Cc: linux-kbuild, Marcelo Tosatti, Paolo Bonzini, Paul E . McKenney,
Paul Gortmaker, Peter Zijlstra, Thomas Gleixner, Steven Rostedt,
Boqun Feng, Daniel Wagner
From: "Peter Zijlstra (Intel)" <peterz@infradead.org>
The existing wait queue support has support for custom wake up call
backs, wake flags, wake key (passed to call back) and exclusive
flags that allow wakers to be tagged as exclusive, for limiting
the number of wakers.
In a lot of cases, none of these features are used, and hence we
can benefit from a slimmed down version that lowers memory overhead
and reduces runtime overhead.
The concept originated from -rt, where waitqueues are a constant
source of trouble, as we can't convert the head lock to a raw
spinlock due to fancy and long lasting callbacks.
With the removal of custom callbacks, we can use a raw lock for
queue list manipulations, hence allowing the simple wait support
to be used in -rt.
Signed-off-by: Daniel Wagner <daniel.wagner@bmw-carit.de>
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Originally-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Paul Gortmaker <paul.gortmaker@windriver.com>
[Patch is from PeterZ which is based on Thomas version.
Commit message is written by Paul G.
Daniel:
- And some compile issues fixed.
- Added non-lazy implementation of swake_up_locked as suggested by Boqun Feng.]
---
include/linux/swait.h | 172 ++++++++++++++++++++++++++++++++++++++++++++++++++
kernel/sched/Makefile | 2 +-
kernel/sched/swait.c | 123 ++++++++++++++++++++++++++++++++++++
3 files changed, 296 insertions(+), 1 deletion(-)
create mode 100644 include/linux/swait.h
create mode 100644 kernel/sched/swait.c
diff --git a/include/linux/swait.h b/include/linux/swait.h
new file mode 100644
index 0000000..c1f9c62
--- /dev/null
+++ b/include/linux/swait.h
@@ -0,0 +1,172 @@
+#ifndef _LINUX_SWAIT_H
+#define _LINUX_SWAIT_H
+
+#include <linux/list.h>
+#include <linux/stddef.h>
+#include <linux/spinlock.h>
+#include <asm/current.h>
+
+/*
+ * Simple wait queues
+ *
+ * While these are very similar to the other/complex wait queues (wait.h) the
+ * most important difference is that the simple waitqueue allows for
+ * deterministic behaviour -- IOW it has strictly bounded IRQ and lock hold
+ * times.
+ *
+ * In order to make this so, we had to drop a fair number of features of the
+ * other waitqueue code; notably:
+ *
+ * - mixing INTERRUPTIBLE and UNINTERRUPTIBLE sleeps on the same waitqueue;
+ * all wakeups are TASK_NORMAL in order to avoid O(n) lookups for the right
+ * sleeper state.
+ *
+ * - the exclusive mode; because this requires preserving the list order
+ * and this is hard.
+ *
+ * - custom wake functions; because you cannot give any guarantees about
+ * random code.
+ *
+ * As a side effect of this; the data structures are slimmer.
+ *
+ * One would recommend using this wait queue where possible.
+ */
+
+struct task_struct;
+
+struct swait_queue_head {
+ raw_spinlock_t lock;
+ struct list_head task_list;
+};
+
+struct swait_queue {
+ struct task_struct *task;
+ struct list_head task_list;
+};
+
+#define __SWAITQUEUE_INITIALIZER(name) { \
+ .task = current, \
+ .task_list = LIST_HEAD_INIT((name).task_list), \
+}
+
+#define DECLARE_SWAITQUEUE(name) \
+ struct swait_queue name = __SWAITQUEUE_INITIALIZER(name)
+
+#define __SWAIT_QUEUE_HEAD_INITIALIZER(name) { \
+ .lock = __RAW_SPIN_LOCK_UNLOCKED(name.lock), \
+ .task_list = LIST_HEAD_INIT((name).task_list), \
+}
+
+#define DECLARE_SWAIT_QUEUE_HEAD(name) \
+ struct swait_queue_head name = __SWAIT_QUEUE_HEAD_INITIALIZER(name)
+
+extern void __init_swait_queue_head(struct swait_queue_head *q, const char *name,
+ struct lock_class_key *key);
+
+#define init_swait_queue_head(q) \
+ do { \
+ static struct lock_class_key __key; \
+ __init_swait_queue_head((q), #q, &__key); \
+ } while (0)
+
+#ifdef CONFIG_LOCKDEP
+# define __SWAIT_QUEUE_HEAD_INIT_ONSTACK(name) \
+ ({ init_swait_queue_head(&name); name; })
+# define DECLARE_SWAIT_QUEUE_HEAD_ONSTACK(name) \
+ struct swait_queue_head name = __SWAIT_QUEUE_HEAD_INIT_ONSTACK(name)
+#else
+# define DECLARE_SWAIT_QUEUE_HEAD_ONSTACK(name) \
+ DECLARE_SWAIT_QUEUE_HEAD(name)
+#endif
+
+static inline int swait_active(struct swait_queue_head *q)
+{
+ return !list_empty(&q->task_list);
+}
+
+extern void swake_up(struct swait_queue_head *q);
+extern void swake_up_all(struct swait_queue_head *q);
+extern void swake_up_locked(struct swait_queue_head *q);
+
+extern void __prepare_to_swait(struct swait_queue_head *q, struct swait_queue *wait);
+extern void prepare_to_swait(struct swait_queue_head *q, struct swait_queue *wait, int state);
+extern long prepare_to_swait_event(struct swait_queue_head *q, struct swait_queue *wait, int state);
+
+extern void __finish_swait(struct swait_queue_head *q, struct swait_queue *wait);
+extern void finish_swait(struct swait_queue_head *q, struct swait_queue *wait);
+
+/* as per ___wait_event() but for swait, therefore "exclusive == 0" */
+#define ___swait_event(wq, condition, state, ret, cmd) \
+({ \
+ struct swait_queue __wait; \
+ long __ret = ret; \
+ \
+ INIT_LIST_HEAD(&__wait.task_list); \
+ for (;;) { \
+ long __int = prepare_to_swait_event(&wq, &__wait, state);\
+ \
+ if (condition) \
+ break; \
+ \
+ if (___wait_is_interruptible(state) && __int) { \
+ __ret = __int; \
+ break; \
+ } \
+ \
+ cmd; \
+ } \
+ finish_swait(&wq, &__wait); \
+ __ret; \
+})
+
+#define __swait_event(wq, condition) \
+ (void)___swait_event(wq, condition, TASK_UNINTERRUPTIBLE, 0, \
+ schedule())
+
+#define swait_event(wq, condition) \
+do { \
+ if (condition) \
+ break; \
+ __swait_event(wq, condition); \
+} while (0)
+
+#define __swait_event_timeout(wq, condition, timeout) \
+ ___swait_event(wq, ___wait_cond_timeout(condition), \
+ TASK_UNINTERRUPTIBLE, timeout, \
+ __ret = schedule_timeout(__ret))
+
+#define swait_event_timeout(wq, condition, timeout) \
+({ \
+ long __ret = timeout; \
+ if (!___wait_cond_timeout(condition)) \
+ __ret = __swait_event_timeout(wq, condition, timeout); \
+ __ret; \
+})
+
+#define __swait_event_interruptible(wq, condition) \
+ ___swait_event(wq, condition, TASK_INTERRUPTIBLE, 0, \
+ schedule())
+
+#define swait_event_interruptible(wq, condition) \
+({ \
+ int __ret = 0; \
+ if (!(condition)) \
+ __ret = __swait_event_interruptible(wq, condition); \
+ __ret; \
+})
+
+#define __swait_event_interruptible_timeout(wq, condition, timeout) \
+ ___swait_event(wq, ___wait_cond_timeout(condition), \
+ TASK_INTERRUPTIBLE, timeout, \
+ __ret = schedule_timeout(__ret))
+
+#define swait_event_interruptible_timeout(wq, condition, timeout) \
+({ \
+ long __ret = timeout; \
+ if (!___wait_cond_timeout(condition)) \
+ __ret = __swait_event_interruptible_timeout(wq, \
+ condition, timeout); \
+ __ret; \
+})
+
+#endif /* _LINUX_SWAIT_H */
diff --git a/kernel/sched/Makefile b/kernel/sched/Makefile
index 6768797..7d4cba2 100644
--- a/kernel/sched/Makefile
+++ b/kernel/sched/Makefile
@@ -13,7 +13,7 @@ endif
obj-y += core.o loadavg.o clock.o cputime.o
obj-y += idle_task.o fair.o rt.o deadline.o stop_task.o
-obj-y += wait.o completion.o idle.o
+obj-y += wait.o swait.o completion.o idle.o
obj-$(CONFIG_SMP) += cpupri.o cpudeadline.o
obj-$(CONFIG_SCHED_AUTOGROUP) += auto_group.o
obj-$(CONFIG_SCHEDSTATS) += stats.o
diff --git a/kernel/sched/swait.c b/kernel/sched/swait.c
new file mode 100644
index 0000000..82f0dff
--- /dev/null
+++ b/kernel/sched/swait.c
@@ -0,0 +1,123 @@
+#include <linux/sched.h>
+#include <linux/swait.h>
+
+void __init_swait_queue_head(struct swait_queue_head *q, const char *name,
+ struct lock_class_key *key)
+{
+ raw_spin_lock_init(&q->lock);
+ lockdep_set_class_and_name(&q->lock, key, name);
+ INIT_LIST_HEAD(&q->task_list);
+}
+EXPORT_SYMBOL(__init_swait_queue_head);
+
+/*
+ * The thing about the wake_up_state() return value; I think we can ignore it.
+ *
+ * If for some reason it would return 0, that means the previously waiting
+ * task is already running, so it will observe condition true (or has already).
+ */
+void swake_up_locked(struct swait_queue_head *q)
+{
+ struct swait_queue *curr;
+
+ if (list_empty(&q->task_list))
+ return;
+
+ curr = list_first_entry(&q->task_list, typeof(*curr), task_list);
+ wake_up_process(curr->task);
+ list_del_init(&curr->task_list);
+}
+EXPORT_SYMBOL(swake_up_locked);
+
+void swake_up(struct swait_queue_head *q)
+{
+ unsigned long flags;
+
+ if (!swait_active(q))
+ return;
+
+ raw_spin_lock_irqsave(&q->lock, flags);
+ swake_up_locked(q);
+ raw_spin_unlock_irqrestore(&q->lock, flags);
+}
+EXPORT_SYMBOL(swake_up);
+
+/*
+ * Does not allow usage from IRQ disabled, since we must be able to
+ * release IRQs to guarantee bounded hold time.
+ */
+void swake_up_all(struct swait_queue_head *q)
+{
+ struct swait_queue *curr;
+ LIST_HEAD(tmp);
+
+ if (!swait_active(q))
+ return;
+
+ raw_spin_lock_irq(&q->lock);
+ list_splice_init(&q->task_list, &tmp);
+ while (!list_empty(&tmp)) {
+ curr = list_first_entry(&tmp, typeof(*curr), task_list);
+
+ wake_up_state(curr->task, TASK_NORMAL);
+ list_del_init(&curr->task_list);
+
+ if (list_empty(&tmp))
+ break;
+
+ raw_spin_unlock_irq(&q->lock);
+ raw_spin_lock_irq(&q->lock);
+ }
+ raw_spin_unlock_irq(&q->lock);
+}
+EXPORT_SYMBOL(swake_up_all);
+
+void __prepare_to_swait(struct swait_queue_head *q, struct swait_queue *wait)
+{
+ wait->task = current;
+ if (list_empty(&wait->task_list))
+ list_add(&wait->task_list, &q->task_list);
+}
+
+void prepare_to_swait(struct swait_queue_head *q, struct swait_queue *wait, int state)
+{
+ unsigned long flags;
+
+ raw_spin_lock_irqsave(&q->lock, flags);
+ __prepare_to_swait(q, wait);
+ set_current_state(state);
+ raw_spin_unlock_irqrestore(&q->lock, flags);
+}
+EXPORT_SYMBOL(prepare_to_swait);
+
+long prepare_to_swait_event(struct swait_queue_head *q, struct swait_queue *wait, int state)
+{
+ if (signal_pending_state(state, current))
+ return -ERESTARTSYS;
+
+ prepare_to_swait(q, wait, state);
+
+ return 0;
+}
+EXPORT_SYMBOL(prepare_to_swait_event);
+
+void __finish_swait(struct swait_queue_head *q, struct swait_queue *wait)
+{
+ __set_current_state(TASK_RUNNING);
+ if (!list_empty(&wait->task_list))
+ list_del_init(&wait->task_list);
+}
+
+void finish_swait(struct swait_queue_head *q, struct swait_queue *wait)
+{
+ unsigned long flags;
+
+ __set_current_state(TASK_RUNNING);
+
+ if (!list_empty_careful(&wait->task_list)) {
+ raw_spin_lock_irqsave(&q->lock, flags);
+ list_del_init(&wait->task_list);
+ raw_spin_unlock_irqrestore(&q->lock, flags);
+ }
+}
+EXPORT_SYMBOL(finish_swait);
--
2.5.0
^ permalink raw reply related [flat|nested] 72+ messages in thread* [PATCH tip v6 2/5] kbuild: Add option to turn incompatible pointer check into error
2016-01-28 14:44 [PATCH tip v6 0/5] Simple wait queue support Daniel Wagner
2016-01-28 14:44 ` [PATCH tip v6 1/5] wait.[ch]: Introduce the simple waitqueue (swait) implementation Daniel Wagner
@ 2016-01-28 14:44 ` Daniel Wagner
2016-01-29 12:17 ` Daniel Wagner
2016-01-28 14:44 ` [PATCH tip v6 3/5] KVM: use simple waitqueue for vcpu->wq Daniel Wagner
` (4 subsequent siblings)
6 siblings, 1 reply; 72+ messages in thread
From: Daniel Wagner @ 2016-01-28 14:44 UTC (permalink / raw)
To: linux-kernel, linux-rt-users
Cc: linux-kbuild, Marcelo Tosatti, Paolo Bonzini, Paul E . McKenney,
Paul Gortmaker, Peter Zijlstra, Thomas Gleixner, Steven Rostedt,
Boqun Feng, Daniel Wagner
With the introduction of the simple wait API we have two very
similar APIs in the kernel. For example wake_up() and swake_up()
is only one character away. Although the compiler will warn
happily the wrong usage it keeps on going an even links the kernel.
Thomas and Peter would rather like to see early missuses reported
as error early on.
In a first attempt we tried to wrap all swait and wait calls
into a macro which has an compile time type assertion. The result
was pretty ugly and wasn't able to catch all wrong usages.
woken_wake_function(), autoremove_wake_function() and wake_bit_function()
are assigned as function pointers. Wrapping them with a macro around is
not possible. Prefixing them with '_' was also not a real option
because there some users in the kernel which do use them as well.
All in all this attempt looked to intrusive and too ugly.
An alternative is to turn the pointer type check into an error which
catches wrong type uses. Obviously not only the swait/wait ones. That
isn't a bad thing either.
Signed-off-by: Daniel Wagner <daniel.wagner@bmw-carit.de>
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
---
Makefile | 3 +++
1 file changed, 3 insertions(+)
diff --git a/Makefile b/Makefile
index 9d94ade..653fd08 100644
--- a/Makefile
+++ b/Makefile
@@ -767,6 +767,9 @@ KBUILD_CFLAGS += $(call cc-option,-Werror=strict-prototypes)
# Prohibit date/time macros, which would make the build non-deterministic
KBUILD_CFLAGS += $(call cc-option,-Werror=date-time)
+# enforce correct pointer usage
+KBUILD_CFLAGS += $(call cc-option,-Werror=incompatible-pointer-types)
+
# use the deterministic mode of AR if available
KBUILD_ARFLAGS := $(call ar-option,D)
--
2.5.0
^ permalink raw reply related [flat|nested] 72+ messages in thread* Re: [PATCH tip v6 2/5] kbuild: Add option to turn incompatible pointer check into error
2016-01-28 14:44 ` [PATCH tip v6 2/5] kbuild: Add option to turn incompatible pointer check into error Daniel Wagner
@ 2016-01-29 12:17 ` Daniel Wagner
0 siblings, 0 replies; 72+ messages in thread
From: Daniel Wagner @ 2016-01-29 12:17 UTC (permalink / raw)
To: linux-kernel, linux-rt-users
Cc: linux-kbuild, Marcelo Tosatti, Paolo Bonzini, Paul E . McKenney,
Paul Gortmaker, Peter Zijlstra, Thomas Gleixner, Steven Rostedt,
Boqun Feng
On 01/28/2016 03:44 PM, Daniel Wagner wrote:
> +# enforce correct pointer usage
> +KBUILD_CFLAGS += $(call cc-option,-Werror=incompatible-pointer-types)
> +
As it turns out there are a few fallouts by that one. I'll send fixes
for it.
^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH tip v6 2/5] kbuild: Add option to turn incompatible pointer check into error
@ 2016-01-29 12:17 ` Daniel Wagner
0 siblings, 0 replies; 72+ messages in thread
From: Daniel Wagner @ 2016-01-29 12:17 UTC (permalink / raw)
To: linux-kernel, linux-rt-users
Cc: linux-kbuild, Marcelo Tosatti, Paolo Bonzini, Paul E . McKenney,
Paul Gortmaker, Peter Zijlstra, Thomas Gleixner, Steven Rostedt,
Boqun Feng
On 01/28/2016 03:44 PM, Daniel Wagner wrote:
> +# enforce correct pointer usage
> +KBUILD_CFLAGS += $(call cc-option,-Werror=incompatible-pointer-types)
> +
As it turns out there are a few fallouts by that one. I'll send fixes
for it.
^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH tip v6 2/5] kbuild: Add option to turn incompatible pointer check into error
2016-01-29 12:17 ` Daniel Wagner
@ 2016-01-29 18:55 ` Paul Gortmaker
-1 siblings, 0 replies; 72+ messages in thread
From: Paul Gortmaker @ 2016-01-29 18:55 UTC (permalink / raw)
To: Daniel Wagner
Cc: linux-kernel, linux-rt-users, linux-kbuild, Marcelo Tosatti,
Paolo Bonzini, Paul E . McKenney, Peter Zijlstra,
Thomas Gleixner, Steven Rostedt, Boqun Feng
[Re: [PATCH tip v6 2/5] kbuild: Add option to turn incompatible pointer check into error] On 29/01/2016 (Fri 13:17) Daniel Wagner wrote:
> On 01/28/2016 03:44 PM, Daniel Wagner wrote:
> > +# enforce correct pointer usage
> > +KBUILD_CFLAGS += $(call cc-option,-Werror=incompatible-pointer-types)
> > +
>
> As it turns out there are a few fallouts by that one. I'll send fixes
> for it.
Did you try non-x86 builds with this applied? I'd be really surprised
if there were just a few, once you did allyesconfig/allmodconfig for
ARM, MIPS, PPC, etc.
P.
--
^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH tip v6 2/5] kbuild: Add option to turn incompatible pointer check into error
@ 2016-01-29 18:55 ` Paul Gortmaker
0 siblings, 0 replies; 72+ messages in thread
From: Paul Gortmaker @ 2016-01-29 18:55 UTC (permalink / raw)
To: Daniel Wagner
Cc: linux-kernel, linux-rt-users, linux-kbuild, Marcelo Tosatti,
Paolo Bonzini, Paul E . McKenney, Peter Zijlstra,
Thomas Gleixner, Steven Rostedt, Boqun Feng
[Re: [PATCH tip v6 2/5] kbuild: Add option to turn incompatible pointer check into error] On 29/01/2016 (Fri 13:17) Daniel Wagner wrote:
> On 01/28/2016 03:44 PM, Daniel Wagner wrote:
> > +# enforce correct pointer usage
> > +KBUILD_CFLAGS += $(call cc-option,-Werror=incompatible-pointer-types)
> > +
>
> As it turns out there are a few fallouts by that one. I'll send fixes
> for it.
Did you try non-x86 builds with this applied? I'd be really surprised
if there were just a few, once you did allyesconfig/allmodconfig for
ARM, MIPS, PPC, etc.
P.
--
^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH tip v6 2/5] kbuild: Add option to turn incompatible pointer check into error
2016-01-29 18:55 ` Paul Gortmaker
@ 2016-02-01 6:49 ` Daniel Wagner
-1 siblings, 0 replies; 72+ messages in thread
From: Daniel Wagner @ 2016-02-01 6:49 UTC (permalink / raw)
To: Paul Gortmaker
Cc: linux-kernel, linux-rt-users, linux-kbuild, Marcelo Tosatti,
Paolo Bonzini, Paul E . McKenney, Peter Zijlstra,
Thomas Gleixner, Steven Rostedt, Boqun Feng
On 01/29/2016 07:55 PM, Paul Gortmaker wrote:
> [Re: [PATCH tip v6 2/5] kbuild: Add option to turn incompatible pointer check into error] On 29/01/2016 (Fri 13:17) Daniel Wagner wrote:
>
>> On 01/28/2016 03:44 PM, Daniel Wagner wrote:
>>> +# enforce correct pointer usage
>>> +KBUILD_CFLAGS += $(call cc-option,-Werror=incompatible-pointer-types)
>>> +
>>
>> As it turns out there are a few fallouts by that one. I'll send fixes
>> for it.
>
> Did you try non-x86 builds with this applied? I'd be really surprised
> if there were just a few, once you did allyesconfig/allmodconfig for
> ARM, MIPS, PPC, etc.
I have tried this with non-x86 builds and apart of a few problems all
looked fine. As it turns out I was using too old cross tools from
kernel.org [1]. Luckily Fengguang's kbuild robot did catch a bunch of
them (see the patches in this series).
Since Thomas was also surprised that only a bunch of them showed up,
I'll better give it another go with more recent compilers.
cheers,
daniel
[1] https://www.kernel.org/pub/tools/crosstool
^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH tip v6 2/5] kbuild: Add option to turn incompatible pointer check into error
@ 2016-02-01 6:49 ` Daniel Wagner
0 siblings, 0 replies; 72+ messages in thread
From: Daniel Wagner @ 2016-02-01 6:49 UTC (permalink / raw)
To: Paul Gortmaker
Cc: linux-kernel, linux-rt-users, linux-kbuild, Marcelo Tosatti,
Paolo Bonzini, Paul E . McKenney, Peter Zijlstra,
Thomas Gleixner, Steven Rostedt, Boqun Feng
On 01/29/2016 07:55 PM, Paul Gortmaker wrote:
> [Re: [PATCH tip v6 2/5] kbuild: Add option to turn incompatible pointer check into error] On 29/01/2016 (Fri 13:17) Daniel Wagner wrote:
>
>> On 01/28/2016 03:44 PM, Daniel Wagner wrote:
>>> +# enforce correct pointer usage
>>> +KBUILD_CFLAGS += $(call cc-option,-Werror=incompatible-pointer-types)
>>> +
>>
>> As it turns out there are a few fallouts by that one. I'll send fixes
>> for it.
>
> Did you try non-x86 builds with this applied? I'd be really surprised
> if there were just a few, once you did allyesconfig/allmodconfig for
> ARM, MIPS, PPC, etc.
I have tried this with non-x86 builds and apart of a few problems all
looked fine. As it turns out I was using too old cross tools from
kernel.org [1]. Luckily Fengguang's kbuild robot did catch a bunch of
them (see the patches in this series).
Since Thomas was also surprised that only a bunch of them showed up,
I'll better give it another go with more recent compilers.
cheers,
daniel
[1] https://www.kernel.org/pub/tools/crosstool
^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH tip v6 2/5] kbuild: Add option to turn incompatible pointer check into error
2016-02-01 6:49 ` Daniel Wagner
@ 2016-02-05 8:16 ` Daniel Wagner
-1 siblings, 0 replies; 72+ messages in thread
From: Daniel Wagner @ 2016-02-05 8:16 UTC (permalink / raw)
To: Paul Gortmaker
Cc: linux-kernel, linux-rt-users, linux-kbuild, Marcelo Tosatti,
Paolo Bonzini, Paul E . McKenney, Peter Zijlstra,
Thomas Gleixner, Steven Rostedt, Boqun Feng
On 02/01/2016 07:49 AM, Daniel Wagner wrote:
> On 01/29/2016 07:55 PM, Paul Gortmaker wrote:
>> [Re: [PATCH tip v6 2/5] kbuild: Add option to turn incompatible pointer check into error] On 29/01/2016 (Fri 13:17) Daniel Wagner wrote:
>>
>>> On 01/28/2016 03:44 PM, Daniel Wagner wrote:
>>>> +# enforce correct pointer usage
>>>> +KBUILD_CFLAGS += $(call cc-option,-Werror=incompatible-pointer-types)
>>>> +
>>>
>>> As it turns out there are a few fallouts by that one. I'll send fixes
>>> for it.
>>
>> Did you try non-x86 builds with this applied? I'd be really surprised
>> if there were just a few, once you did allyesconfig/allmodconfig for
>> ARM, MIPS, PPC, etc.
>
> I have tried this with non-x86 builds and apart of a few problems all
> looked fine. As it turns out I was using too old cross tools from
> kernel.org [1]. Luckily Fengguang's kbuild robot did catch a bunch of
> them (see the patches in this series).
It turns out this week was particular bad for doing anything productive.
Anyway, I found some time to fire up some cross compilers and it looks
promising.
I used the cross compiler version 5.2.1 shipped by Fedora 23
and run allyesconfig/allmodconfig for ARM, ARM64, MIPS64, PPC64
(swait-v7 and 4.5-rc2). No new errors popped up.
With some luck I get some more architectures covered soon.
cheers,
daniel
^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH tip v6 2/5] kbuild: Add option to turn incompatible pointer check into error
@ 2016-02-05 8:16 ` Daniel Wagner
0 siblings, 0 replies; 72+ messages in thread
From: Daniel Wagner @ 2016-02-05 8:16 UTC (permalink / raw)
To: Paul Gortmaker
Cc: linux-kernel, linux-rt-users, linux-kbuild, Marcelo Tosatti,
Paolo Bonzini, Paul E . McKenney, Peter Zijlstra,
Thomas Gleixner, Steven Rostedt, Boqun Feng
On 02/01/2016 07:49 AM, Daniel Wagner wrote:
> On 01/29/2016 07:55 PM, Paul Gortmaker wrote:
>> [Re: [PATCH tip v6 2/5] kbuild: Add option to turn incompatible pointer check into error] On 29/01/2016 (Fri 13:17) Daniel Wagner wrote:
>>
>>> On 01/28/2016 03:44 PM, Daniel Wagner wrote:
>>>> +# enforce correct pointer usage
>>>> +KBUILD_CFLAGS += $(call cc-option,-Werror=incompatible-pointer-types)
>>>> +
>>>
>>> As it turns out there are a few fallouts by that one. I'll send fixes
>>> for it.
>>
>> Did you try non-x86 builds with this applied? I'd be really surprised
>> if there were just a few, once you did allyesconfig/allmodconfig for
>> ARM, MIPS, PPC, etc.
>
> I have tried this with non-x86 builds and apart of a few problems all
> looked fine. As it turns out I was using too old cross tools from
> kernel.org [1]. Luckily Fengguang's kbuild robot did catch a bunch of
> them (see the patches in this series).
It turns out this week was particular bad for doing anything productive.
Anyway, I found some time to fire up some cross compilers and it looks
promising.
I used the cross compiler version 5.2.1 shipped by Fedora 23
and run allyesconfig/allmodconfig for ARM, ARM64, MIPS64, PPC64
(swait-v7 and 4.5-rc2). No new errors popped up.
With some luck I get some more architectures covered soon.
cheers,
daniel
^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH tip v6 2/5] kbuild: Add option to turn incompatible pointer check into error
2016-02-05 8:16 ` Daniel Wagner
@ 2016-02-07 4:39 ` Paul Gortmaker
-1 siblings, 0 replies; 72+ messages in thread
From: Paul Gortmaker @ 2016-02-07 4:39 UTC (permalink / raw)
To: Daniel Wagner
Cc: linux-kernel, linux-rt-users, linux-kbuild, Marcelo Tosatti,
Paolo Bonzini, Paul E . McKenney, Peter Zijlstra,
Thomas Gleixner, Steven Rostedt, Boqun Feng
[Re: [PATCH tip v6 2/5] kbuild: Add option to turn incompatible pointer check into error] On 05/02/2016 (Fri 09:16) Daniel Wagner wrote:
> On 02/01/2016 07:49 AM, Daniel Wagner wrote:
> > On 01/29/2016 07:55 PM, Paul Gortmaker wrote:
> >> [Re: [PATCH tip v6 2/5] kbuild: Add option to turn incompatible pointer check into error] On 29/01/2016 (Fri 13:17) Daniel Wagner wrote:
> >>
> >>> On 01/28/2016 03:44 PM, Daniel Wagner wrote:
> >>>> +# enforce correct pointer usage
> >>>> +KBUILD_CFLAGS += $(call cc-option,-Werror=incompatible-pointer-types)
> >>>> +
> >>>
> >>> As it turns out there are a few fallouts by that one. I'll send fixes
> >>> for it.
> >>
> >> Did you try non-x86 builds with this applied? I'd be really surprised
> >> if there were just a few, once you did allyesconfig/allmodconfig for
> >> ARM, MIPS, PPC, etc.
> >
> > I have tried this with non-x86 builds and apart of a few problems all
> > looked fine. As it turns out I was using too old cross tools from
> > kernel.org [1]. Luckily Fengguang's kbuild robot did catch a bunch of
> > them (see the patches in this series).
>
> It turns out this week was particular bad for doing anything productive.
> Anyway, I found some time to fire up some cross compilers and it looks
> promising.
>
> I used the cross compiler version 5.2.1 shipped by Fedora 23
> and run allyesconfig/allmodconfig for ARM, ARM64, MIPS64, PPC64
> (swait-v7 and 4.5-rc2). No new errors popped up.
SOunds good ; guess my gut feeling about this causing more fallout was
off the mark.
P.
--
>
> With some luck I get some more architectures covered soon.
>
> cheers,
> daniel
^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH tip v6 2/5] kbuild: Add option to turn incompatible pointer check into error
@ 2016-02-07 4:39 ` Paul Gortmaker
0 siblings, 0 replies; 72+ messages in thread
From: Paul Gortmaker @ 2016-02-07 4:39 UTC (permalink / raw)
To: Daniel Wagner
Cc: linux-kernel, linux-rt-users, linux-kbuild, Marcelo Tosatti,
Paolo Bonzini, Paul E . McKenney, Peter Zijlstra,
Thomas Gleixner, Steven Rostedt, Boqun Feng
[Re: [PATCH tip v6 2/5] kbuild: Add option to turn incompatible pointer check into error] On 05/02/2016 (Fri 09:16) Daniel Wagner wrote:
> On 02/01/2016 07:49 AM, Daniel Wagner wrote:
> > On 01/29/2016 07:55 PM, Paul Gortmaker wrote:
> >> [Re: [PATCH tip v6 2/5] kbuild: Add option to turn incompatible pointer check into error] On 29/01/2016 (Fri 13:17) Daniel Wagner wrote:
> >>
> >>> On 01/28/2016 03:44 PM, Daniel Wagner wrote:
> >>>> +# enforce correct pointer usage
> >>>> +KBUILD_CFLAGS += $(call cc-option,-Werror=incompatible-pointer-types)
> >>>> +
> >>>
> >>> As it turns out there are a few fallouts by that one. I'll send fixes
> >>> for it.
> >>
> >> Did you try non-x86 builds with this applied? I'd be really surprised
> >> if there were just a few, once you did allyesconfig/allmodconfig for
> >> ARM, MIPS, PPC, etc.
> >
> > I have tried this with non-x86 builds and apart of a few problems all
> > looked fine. As it turns out I was using too old cross tools from
> > kernel.org [1]. Luckily Fengguang's kbuild robot did catch a bunch of
> > them (see the patches in this series).
>
> It turns out this week was particular bad for doing anything productive.
> Anyway, I found some time to fire up some cross compilers and it looks
> promising.
>
> I used the cross compiler version 5.2.1 shipped by Fedora 23
> and run allyesconfig/allmodconfig for ARM, ARM64, MIPS64, PPC64
> (swait-v7 and 4.5-rc2). No new errors popped up.
SOunds good ; guess my gut feeling about this causing more fallout was
off the mark.
P.
--
>
> With some luck I get some more architectures covered soon.
>
> cheers,
> daniel
^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH tip v6 2/5] kbuild: Add option to turn incompatible pointer check into error
2016-02-07 4:39 ` Paul Gortmaker
@ 2016-02-17 13:04 ` Daniel Wagner
-1 siblings, 0 replies; 72+ messages in thread
From: Daniel Wagner @ 2016-02-17 13:04 UTC (permalink / raw)
To: Paul Gortmaker
Cc: linux-kernel, linux-rt-users, linux-kbuild, Marcelo Tosatti,
Paolo Bonzini, Paul E . McKenney, Peter Zijlstra,
Thomas Gleixner, Steven Rostedt, Boqun Feng
On 02/07/2016 05:39 AM, Paul Gortmaker wrote:
>> I used the cross compiler version 5.2.1 shipped by Fedora 23
>> and run allyesconfig/allmodconfig for ARM, ARM64, MIPS64, PPC64
>> (swait-v7 and 4.5-rc2). No new errors popped up.
>
> SOunds good ; guess my gut feeling about this causing more fallout was
> off the mark.
The fallouts I found are on the way to mainline or are already
in mainline:
fbdev: https://git.kernel.org/tomba/c/206fc20598157ce15597822cf01b94377e30075b
mips: https://git.kernel.org/torvalds/c/f4d3d504198d464e406171cfa554a59bd4773d79
There was also a hickup on alpha. The patch is on the way.
Here is the list of archs and config I tried out. First run was with the
config listed, followed by a allyesconfig and allmodconfig build.
# name ARCH CROSS_COMPILE config
declare -a config=(
"alpha alpha alpha-linux-gnu- defconfig"
"arm32 arm arm-linux-gnu- versatile_defconfig"
"arm64 arm64 aarch64-linux-gnu- defconfig"
"cris10 cris cris-linux-gnu- etrax-100lx_defconfig"
"frv frv frv-linux-gnu- defconfig"
"ia64 ia64 ia64-linux-gnu- generic_defconfig"
"m68k m68k m68k-linux-gnu- amiga_defconfig"
"mips32 mips mips64-linux-gnu- ar7_defconfig"
"mips64 mips mips64-linux-gnu- bigsur_defconfig"
"ppc64 powerpc powerpc64-linux-gnu- pseries_defconfig"
"s390 s390 s390x-linux-gnu- defconfig"
"sparc32 sparc sparc64-linux-gnu- sparc32_defconfig"
"sparc64 sparc sparc64-linux-gnu- sparc64_defconfig"
"um_x86_64 um gcc defconfig"
"x86_64 x86_64 gcc defconfig"
# known broken configurations on F23
# "alpha alpha alpha-linux-gnu- allyesconfig"
# "avr32 avr32 avr32-linux-gnu- defconfig"
# "blackfin blackfin bfin-linux-gnu- BF518F-EZBRD_defconfig"
# "cris32 cris cris-linux-gnu- etraxfs_defconfig"
# "m32r m32r m32r-linux-gnu- mappi3.smp_defconfig"
# "parisc parisc hppa-linux-gnu- default_defconfig"
# "parisc64 parisc hppa-linux-gnu- a500_defconfig"
# "ppc32 powerpc powerpc64-linux-gnu- mpc512x_defconfig"
# "sh32 sh sh-linux-gnu- defconfig"
# "sh64 sh64 sh64-linux-gnu- defconfig"
# "tile tile tile-linux-gnu- defconfig"
# "xtensa xtensa xtensa-linux-gnu- common_defconfig"
)
cheers,
daniel
^ permalink raw reply [flat|nested] 72+ messages in thread* Re: [PATCH tip v6 2/5] kbuild: Add option to turn incompatible pointer check into error
@ 2016-02-17 13:04 ` Daniel Wagner
0 siblings, 0 replies; 72+ messages in thread
From: Daniel Wagner @ 2016-02-17 13:04 UTC (permalink / raw)
To: Paul Gortmaker
Cc: linux-kernel, linux-rt-users, linux-kbuild, Marcelo Tosatti,
Paolo Bonzini, Paul E . McKenney, Peter Zijlstra,
Thomas Gleixner, Steven Rostedt, Boqun Feng
On 02/07/2016 05:39 AM, Paul Gortmaker wrote:
>> I used the cross compiler version 5.2.1 shipped by Fedora 23
>> and run allyesconfig/allmodconfig for ARM, ARM64, MIPS64, PPC64
>> (swait-v7 and 4.5-rc2). No new errors popped up.
>
> SOunds good ; guess my gut feeling about this causing more fallout was
> off the mark.
The fallouts I found are on the way to mainline or are already
in mainline:
fbdev: https://git.kernel.org/tomba/c/206fc20598157ce15597822cf01b94377e30075b
mips: https://git.kernel.org/torvalds/c/f4d3d504198d464e406171cfa554a59bd4773d79
There was also a hickup on alpha. The patch is on the way.
Here is the list of archs and config I tried out. First run was with the
config listed, followed by a allyesconfig and allmodconfig build.
# name ARCH CROSS_COMPILE config
declare -a config=(
"alpha alpha alpha-linux-gnu- defconfig"
"arm32 arm arm-linux-gnu- versatile_defconfig"
"arm64 arm64 aarch64-linux-gnu- defconfig"
"cris10 cris cris-linux-gnu- etrax-100lx_defconfig"
"frv frv frv-linux-gnu- defconfig"
"ia64 ia64 ia64-linux-gnu- generic_defconfig"
"m68k m68k m68k-linux-gnu- amiga_defconfig"
"mips32 mips mips64-linux-gnu- ar7_defconfig"
"mips64 mips mips64-linux-gnu- bigsur_defconfig"
"ppc64 powerpc powerpc64-linux-gnu- pseries_defconfig"
"s390 s390 s390x-linux-gnu- defconfig"
"sparc32 sparc sparc64-linux-gnu- sparc32_defconfig"
"sparc64 sparc sparc64-linux-gnu- sparc64_defconfig"
"um_x86_64 um gcc defconfig"
"x86_64 x86_64 gcc defconfig"
# known broken configurations on F23
# "alpha alpha alpha-linux-gnu- allyesconfig"
# "avr32 avr32 avr32-linux-gnu- defconfig"
# "blackfin blackfin bfin-linux-gnu- BF518F-EZBRD_defconfig"
# "cris32 cris cris-linux-gnu- etraxfs_defconfig"
# "m32r m32r m32r-linux-gnu- mappi3.smp_defconfig"
# "parisc parisc hppa-linux-gnu- default_defconfig"
# "parisc64 parisc hppa-linux-gnu- a500_defconfig"
# "ppc32 powerpc powerpc64-linux-gnu- mpc512x_defconfig"
# "sh32 sh sh-linux-gnu- defconfig"
# "sh64 sh64 sh64-linux-gnu- defconfig"
# "tile tile tile-linux-gnu- defconfig"
# "xtensa xtensa xtensa-linux-gnu- common_defconfig"
)
cheers,
daniel
^ permalink raw reply [flat|nested] 72+ messages in thread
* [PATCH tip v6 3/5] KVM: use simple waitqueue for vcpu->wq
2016-01-28 14:44 [PATCH tip v6 0/5] Simple wait queue support Daniel Wagner
2016-01-28 14:44 ` [PATCH tip v6 1/5] wait.[ch]: Introduce the simple waitqueue (swait) implementation Daniel Wagner
2016-01-28 14:44 ` [PATCH tip v6 2/5] kbuild: Add option to turn incompatible pointer check into error Daniel Wagner
@ 2016-01-28 14:44 ` Daniel Wagner
2016-01-29 12:18 ` Daniel Wagner
2016-01-28 14:44 ` [PATCH tip v6 4/5] rcu: Do not call rcu_nocb_gp_cleanup() while holding rnp->lock Daniel Wagner
` (3 subsequent siblings)
6 siblings, 1 reply; 72+ messages in thread
From: Daniel Wagner @ 2016-01-28 14:44 UTC (permalink / raw)
To: linux-kernel, linux-rt-users
Cc: linux-kbuild, Marcelo Tosatti, Paolo Bonzini, Paul E . McKenney,
Paul Gortmaker, Peter Zijlstra, Thomas Gleixner, Steven Rostedt,
Boqun Feng, Daniel Wagner
From: Marcelo Tosatti <mtosatti@redhat.com>
The problem:
On -rt, an emulated LAPIC timer instances has the following path:
1) hard interrupt
2) ksoftirqd is scheduled
3) ksoftirqd wakes up vcpu thread
4) vcpu thread is scheduled
This extra context switch introduces unnecessary latency in the
LAPIC path for a KVM guest.
The solution:
Allow waking up vcpu thread from hardirq context,
thus avoiding the need for ksoftirqd to be scheduled.
Normal waitqueues make use of spinlocks, which on -RT
are sleepable locks. Therefore, waking up a waitqueue
waiter involves locking a sleeping lock, which
is not allowed from hard interrupt context.
cyclictest command line:
This patch reduces the average latency in my tests from 14us to 11us.
Daniel writes:
Paolo asked for numbers from kvm-unit-tests/tscdeadline_latency
benchmark on mainline. The test was run 1000 times on
tip/sched/core 4.4.0-rc8-01134-g0905f04:
./x86-run x86/tscdeadline_latency.flat -cpu host
with idle=poll.
The test seems not to deliver really stable numbers though most of
them are smaller. Paolo write:
"Anything above ~10000 cycles means that the host went to C1 or
lower---the number means more or less nothing in that case.
The mean shows an improvement indeed."
Before:
min max mean std
count 1000.000000 1000.000000 1000.000000 1000.000000
mean 5162.596000 2019270.084000 5824.491541 20681.645558
std 75.431231 622607.723969 89.575700 6492.272062
min 4466.000000 23928.000000 5537.926500 585.864966
25% 5163.000000 1613252.750000 5790.132275 16683.745433
50% 5175.000000 2281919.000000 5834.654000 23151.990026
75% 5190.000000 2382865.750000 5861.412950 24148.206168
max 5228.000000 4175158.000000 6254.827300 46481.048691
After
min max mean std
count 1000.000000 1000.00000 1000.000000 1000.000000
mean 5143.511000 2076886.10300 5813.312474 21207.357565
std 77.668322 610413.09583 86.541500 6331.915127
min 4427.000000 25103.00000 5529.756600 559.187707
25% 5148.000000 1691272.75000 5784.889825 17473.518244
50% 5160.000000 2308328.50000 5832.025000 23464.837068
75% 5172.000000 2393037.75000 5853.177675 24223.969976
max 5222.000000 3922458.00000 6186.720500 42520.379830
[Patch was originaly based on the swait implementation found in the -rt
tree. Daniel ported it to mainline's version and gathered the
benchmark numbers for tscdeadline_latency test.]
Signed-off-by: Daniel Wagner <daniel.wagner@bmw-carit.de>
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Marcelo Tosatti <mtosatti@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
---
arch/arm/kvm/arm.c | 4 ++--
arch/arm/kvm/psci.c | 4 ++--
arch/mips/kvm/mips.c | 8 ++++----
arch/powerpc/include/asm/kvm_host.h | 4 ++--
arch/powerpc/kvm/book3s_hv.c | 23 +++++++++++------------
arch/s390/include/asm/kvm_host.h | 2 +-
arch/s390/kvm/interrupt.c | 4 ++--
arch/x86/kvm/lapic.c | 6 +++---
include/linux/kvm_host.h | 5 +++--
virt/kvm/async_pf.c | 4 ++--
virt/kvm/kvm_main.c | 17 ++++++++---------
11 files changed, 40 insertions(+), 41 deletions(-)
diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
index e06fd29..b9ea64b 100644
--- a/arch/arm/kvm/arm.c
+++ b/arch/arm/kvm/arm.c
@@ -507,9 +507,9 @@ static void kvm_arm_resume_guest(struct kvm *kvm)
static void vcpu_sleep(struct kvm_vcpu *vcpu)
{
- wait_queue_head_t *wq = kvm_arch_vcpu_wq(vcpu);
+ struct swait_queue_head *wq = kvm_arch_vcpu_wq(vcpu);
- wait_event_interruptible(*wq, ((!vcpu->arch.power_off) &&
+ swait_event_interruptible(*wq, ((!vcpu->arch.power_off) &&
(!vcpu->arch.pause)));
}
diff --git a/arch/arm/kvm/psci.c b/arch/arm/kvm/psci.c
index a9b3b90..c2b1315 100644
--- a/arch/arm/kvm/psci.c
+++ b/arch/arm/kvm/psci.c
@@ -70,7 +70,7 @@ static unsigned long kvm_psci_vcpu_on(struct kvm_vcpu *source_vcpu)
{
struct kvm *kvm = source_vcpu->kvm;
struct kvm_vcpu *vcpu = NULL;
- wait_queue_head_t *wq;
+ struct swait_queue_head *wq;
unsigned long cpu_id;
unsigned long context_id;
phys_addr_t target_pc;
@@ -119,7 +119,7 @@ static unsigned long kvm_psci_vcpu_on(struct kvm_vcpu *source_vcpu)
smp_mb(); /* Make sure the above is visible */
wq = kvm_arch_vcpu_wq(vcpu);
- wake_up_interruptible(wq);
+ swake_up(wq);
return PSCI_RET_SUCCESS;
}
diff --git a/arch/mips/kvm/mips.c b/arch/mips/kvm/mips.c
index b9b803f..28e9acb 100644
--- a/arch/mips/kvm/mips.c
+++ b/arch/mips/kvm/mips.c
@@ -445,8 +445,8 @@ int kvm_vcpu_ioctl_interrupt(struct kvm_vcpu *vcpu,
dvcpu->arch.wait = 0;
- if (waitqueue_active(&dvcpu->wq))
- wake_up_interruptible(&dvcpu->wq);
+ if (swait_active(&dvcpu->wq))
+ swake_up(&dvcpu->wq);
return 0;
}
@@ -1174,8 +1174,8 @@ static void kvm_mips_comparecount_func(unsigned long data)
kvm_mips_callbacks->queue_timer_int(vcpu);
vcpu->arch.wait = 0;
- if (waitqueue_active(&vcpu->wq))
- wake_up_interruptible(&vcpu->wq);
+ if (swait_active(&vcpu->wq))
+ swake_up(&vcpu->wq);
}
/* low level hrtimer wake routine */
diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h
index cfa758c..f8673ff 100644
--- a/arch/powerpc/include/asm/kvm_host.h
+++ b/arch/powerpc/include/asm/kvm_host.h
@@ -286,7 +286,7 @@ struct kvmppc_vcore {
struct list_head runnable_threads;
struct list_head preempt_list;
spinlock_t lock;
- wait_queue_head_t wq;
+ struct swait_queue_head wq;
spinlock_t stoltb_lock; /* protects stolen_tb and preempt_tb */
u64 stolen_tb;
u64 preempt_tb;
@@ -626,7 +626,7 @@ struct kvm_vcpu_arch {
u8 prodded;
u32 last_inst;
- wait_queue_head_t *wqp;
+ struct swait_queue_head *wqp;
struct kvmppc_vcore *vcore;
int ret;
int trap;
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index a7352b5..df34a64 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -114,11 +114,11 @@ static bool kvmppc_ipi_thread(int cpu)
static void kvmppc_fast_vcpu_kick_hv(struct kvm_vcpu *vcpu)
{
int cpu;
- wait_queue_head_t *wqp;
+ struct swait_queue_head *wqp;
wqp = kvm_arch_vcpu_wq(vcpu);
- if (waitqueue_active(wqp)) {
- wake_up_interruptible(wqp);
+ if (swait_active(wqp)) {
+ swake_up(wqp);
++vcpu->stat.halt_wakeup;
}
@@ -707,8 +707,8 @@ int kvmppc_pseries_do_hcall(struct kvm_vcpu *vcpu)
tvcpu->arch.prodded = 1;
smp_mb();
if (vcpu->arch.ceded) {
- if (waitqueue_active(&vcpu->wq)) {
- wake_up_interruptible(&vcpu->wq);
+ if (swait_active(&vcpu->wq)) {
+ swake_up(&vcpu->wq);
vcpu->stat.halt_wakeup++;
}
}
@@ -1447,7 +1447,7 @@ static struct kvmppc_vcore *kvmppc_vcore_create(struct kvm *kvm, int core)
INIT_LIST_HEAD(&vcore->runnable_threads);
spin_lock_init(&vcore->lock);
spin_lock_init(&vcore->stoltb_lock);
- init_waitqueue_head(&vcore->wq);
+ init_swait_queue_head(&vcore->wq);
vcore->preempt_tb = TB_NIL;
vcore->lpcr = kvm->arch.lpcr;
vcore->first_vcpuid = core * threads_per_subcore;
@@ -2519,10 +2519,9 @@ static void kvmppc_vcore_blocked(struct kvmppc_vcore *vc)
{
struct kvm_vcpu *vcpu;
int do_sleep = 1;
+ DECLARE_SWAITQUEUE(wait);
- DEFINE_WAIT(wait);
-
- prepare_to_wait(&vc->wq, &wait, TASK_INTERRUPTIBLE);
+ prepare_to_swait(&vc->wq, &wait, TASK_INTERRUPTIBLE);
/*
* Check one last time for pending exceptions and ceded state after
@@ -2536,7 +2535,7 @@ static void kvmppc_vcore_blocked(struct kvmppc_vcore *vc)
}
if (!do_sleep) {
- finish_wait(&vc->wq, &wait);
+ finish_swait(&vc->wq, &wait);
return;
}
@@ -2544,7 +2543,7 @@ static void kvmppc_vcore_blocked(struct kvmppc_vcore *vc)
trace_kvmppc_vcore_blocked(vc, 0);
spin_unlock(&vc->lock);
schedule();
- finish_wait(&vc->wq, &wait);
+ finish_swait(&vc->wq, &wait);
spin_lock(&vc->lock);
vc->vcore_state = VCORE_INACTIVE;
trace_kvmppc_vcore_blocked(vc, 1);
@@ -2600,7 +2599,7 @@ static int kvmppc_run_vcpu(struct kvm_run *kvm_run, struct kvm_vcpu *vcpu)
kvmppc_start_thread(vcpu, vc);
trace_kvm_guest_enter(vcpu);
} else if (vc->vcore_state == VCORE_SLEEPING) {
- wake_up(&vc->wq);
+ swake_up(&vc->wq);
}
}
diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
index efaac2c..c66831f 100644
--- a/arch/s390/include/asm/kvm_host.h
+++ b/arch/s390/include/asm/kvm_host.h
@@ -427,7 +427,7 @@ struct kvm_s390_irq_payload {
struct kvm_s390_local_interrupt {
spinlock_t lock;
struct kvm_s390_float_interrupt *float_int;
- wait_queue_head_t *wq;
+ struct swait_queue_head *wq;
atomic_t *cpuflags;
DECLARE_BITMAP(sigp_emerg_pending, KVM_MAX_VCPUS);
struct kvm_s390_irq_payload irq;
diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
index 6a75352..cc862c4 100644
--- a/arch/s390/kvm/interrupt.c
+++ b/arch/s390/kvm/interrupt.c
@@ -868,13 +868,13 @@ no_timer:
void kvm_s390_vcpu_wakeup(struct kvm_vcpu *vcpu)
{
- if (waitqueue_active(&vcpu->wq)) {
+ if (swait_active(&vcpu->wq)) {
/*
* The vcpu gave up the cpu voluntarily, mark it as a good
* yield-candidate.
*/
vcpu->preempted = true;
- wake_up_interruptible(&vcpu->wq);
+ swake_up(&vcpu->wq);
vcpu->stat.halt_wakeup++;
}
}
diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index 4d30b86..34dc14f 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -1195,7 +1195,7 @@ static void apic_update_lvtt(struct kvm_lapic *apic)
static void apic_timer_expired(struct kvm_lapic *apic)
{
struct kvm_vcpu *vcpu = apic->vcpu;
- wait_queue_head_t *q = &vcpu->wq;
+ struct swait_queue_head *q = &vcpu->wq;
struct kvm_timer *ktimer = &apic->lapic_timer;
if (atomic_read(&apic->lapic_timer.pending))
@@ -1204,8 +1204,8 @@ static void apic_timer_expired(struct kvm_lapic *apic)
atomic_inc(&apic->lapic_timer.pending);
kvm_set_pending_timer(vcpu);
- if (waitqueue_active(q))
- wake_up_interruptible(q);
+ if (swait_active(q))
+ swake_up(q);
if (apic_lvtt_tscdeadline(apic))
ktimer->expired_tscdeadline = ktimer->tscdeadline;
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index c923350..c690acc 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -25,6 +25,7 @@
#include <linux/irqflags.h>
#include <linux/context_tracking.h>
#include <linux/irqbypass.h>
+#include <linux/swait.h>
#include <asm/signal.h>
#include <linux/kvm.h>
@@ -243,7 +244,7 @@ struct kvm_vcpu {
int fpu_active;
int guest_fpu_loaded, guest_xcr0_loaded;
unsigned char fpu_counter;
- wait_queue_head_t wq;
+ struct swait_queue_head wq;
struct pid *pid;
int sigset_active;
sigset_t sigset;
@@ -794,7 +795,7 @@ static inline bool kvm_arch_has_assigned_device(struct kvm *kvm)
}
#endif
-static inline wait_queue_head_t *kvm_arch_vcpu_wq(struct kvm_vcpu *vcpu)
+static inline struct swait_queue_head *kvm_arch_vcpu_wq(struct kvm_vcpu *vcpu)
{
#ifdef __KVM_HAVE_ARCH_WQP
return vcpu->arch.wqp;
diff --git a/virt/kvm/async_pf.c b/virt/kvm/async_pf.c
index 77d42be..cd8477c 100644
--- a/virt/kvm/async_pf.c
+++ b/virt/kvm/async_pf.c
@@ -98,8 +98,8 @@ static void async_pf_execute(struct work_struct *work)
* This memory barrier pairs with prepare_to_wait's set_current_state()
*/
smp_mb();
- if (waitqueue_active(&vcpu->wq))
- wake_up_interruptible(&vcpu->wq);
+ if (swait_active(&vcpu->wq))
+ swake_up(&vcpu->wq);
mmput(mm);
kvm_put_kvm(vcpu->kvm);
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 484079e..43fad2e 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -226,8 +226,7 @@ int kvm_vcpu_init(struct kvm_vcpu *vcpu, struct kvm *kvm, unsigned id)
vcpu->kvm = kvm;
vcpu->vcpu_id = id;
vcpu->pid = NULL;
- vcpu->halt_poll_ns = 0;
- init_waitqueue_head(&vcpu->wq);
+ init_swait_queue_head(&vcpu->wq);
kvm_async_pf_vcpu_init(vcpu);
vcpu->pre_pcpu = -1;
@@ -1999,7 +1998,7 @@ static int kvm_vcpu_check_block(struct kvm_vcpu *vcpu)
void kvm_vcpu_block(struct kvm_vcpu *vcpu)
{
ktime_t start, cur;
- DEFINE_WAIT(wait);
+ DECLARE_SWAITQUEUE(wait);
bool waited = false;
u64 block_ns;
@@ -2024,7 +2023,7 @@ void kvm_vcpu_block(struct kvm_vcpu *vcpu)
kvm_arch_vcpu_blocking(vcpu);
for (;;) {
- prepare_to_wait(&vcpu->wq, &wait, TASK_INTERRUPTIBLE);
+ prepare_to_swait(&vcpu->wq, &wait, TASK_INTERRUPTIBLE);
if (kvm_vcpu_check_block(vcpu) < 0)
break;
@@ -2033,7 +2032,7 @@ void kvm_vcpu_block(struct kvm_vcpu *vcpu)
schedule();
}
- finish_wait(&vcpu->wq, &wait);
+ finish_swait(&vcpu->wq, &wait);
cur = ktime_get();
kvm_arch_vcpu_unblocking(vcpu);
@@ -2065,11 +2064,11 @@ void kvm_vcpu_kick(struct kvm_vcpu *vcpu)
{
int me;
int cpu = vcpu->cpu;
- wait_queue_head_t *wqp;
+ struct swait_queue_head *wqp;
wqp = kvm_arch_vcpu_wq(vcpu);
- if (waitqueue_active(wqp)) {
- wake_up_interruptible(wqp);
+ if (swait_active(wqp)) {
+ swake_up(wqp);
++vcpu->stat.halt_wakeup;
}
@@ -2170,7 +2169,7 @@ void kvm_vcpu_on_spin(struct kvm_vcpu *me)
continue;
if (vcpu == me)
continue;
- if (waitqueue_active(&vcpu->wq) && !kvm_arch_vcpu_runnable(vcpu))
+ if (swait_active(&vcpu->wq) && !kvm_arch_vcpu_runnable(vcpu))
continue;
if (!kvm_vcpu_eligible_for_directed_yield(vcpu))
continue;
--
2.5.0
^ permalink raw reply related [flat|nested] 72+ messages in thread* Re: [PATCH tip v6 3/5] KVM: use simple waitqueue for vcpu->wq
2016-01-28 14:44 ` [PATCH tip v6 3/5] KVM: use simple waitqueue for vcpu->wq Daniel Wagner
@ 2016-01-29 12:18 ` Daniel Wagner
0 siblings, 0 replies; 72+ messages in thread
From: Daniel Wagner @ 2016-01-29 12:18 UTC (permalink / raw)
To: linux-kernel, linux-rt-users
Cc: linux-kbuild, Marcelo Tosatti, Paolo Bonzini, Paul E . McKenney,
Paul Gortmaker, Peter Zijlstra, Thomas Gleixner, Steven Rostedt,
Boqun Feng
I missed one to fixup for ARM. I'll send an update.
^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH tip v6 3/5] KVM: use simple waitqueue for vcpu->wq
@ 2016-01-29 12:18 ` Daniel Wagner
0 siblings, 0 replies; 72+ messages in thread
From: Daniel Wagner @ 2016-01-29 12:18 UTC (permalink / raw)
To: linux-kernel, linux-rt-users
Cc: linux-kbuild, Marcelo Tosatti, Paolo Bonzini, Paul E . McKenney,
Paul Gortmaker, Peter Zijlstra, Thomas Gleixner, Steven Rostedt,
Boqun Feng
I missed one to fixup for ARM. I'll send an update.
^ permalink raw reply [flat|nested] 72+ messages in thread
* [PATCH tip v6 4/5] rcu: Do not call rcu_nocb_gp_cleanup() while holding rnp->lock
2016-01-28 14:44 [PATCH tip v6 0/5] Simple wait queue support Daniel Wagner
` (2 preceding siblings ...)
2016-01-28 14:44 ` [PATCH tip v6 3/5] KVM: use simple waitqueue for vcpu->wq Daniel Wagner
@ 2016-01-28 14:44 ` Daniel Wagner
2016-01-28 14:44 ` [PATCH tip v6 5/5] rcu: use simple wait queues where possible in rcutree Daniel Wagner
` (2 subsequent siblings)
6 siblings, 0 replies; 72+ messages in thread
From: Daniel Wagner @ 2016-01-28 14:44 UTC (permalink / raw)
To: linux-kernel, linux-rt-users
Cc: linux-kbuild, Marcelo Tosatti, Paolo Bonzini, Paul E . McKenney,
Paul Gortmaker, Peter Zijlstra, Thomas Gleixner, Steven Rostedt,
Boqun Feng, Daniel Wagner
rcu_nocb_gp_cleanup() is called while holding rnp->lock. Currently,
this is okay because the wake_up_all() in rcu_nocb_gp_cleanup() will
not enable the IRQs. lockdep is happy.
By switching over using swait this is not true anymore. swake_up_all()
enables the IRQs while processing the waiters. __do_softirq() can now
run and will eventually call rcu_process_callbacks() which wants to
grap nrp->lock.
Let's move the rcu_nocb_gp_cleanup() call outside the lock before we
switch over to swait.
If we would hold the rnp->lock and use swait, lockdep reports
following:
=================================
[ INFO: inconsistent lock state ]
4.2.0-rc5-00025-g9a73ba0 #136 Not tainted
---------------------------------
inconsistent {IN-SOFTIRQ-W} -> {SOFTIRQ-ON-W} usage.
rcu_preempt/8 [HC0[0]:SC0[0]:HE1:SE1] takes:
(rcu_node_1){+.?...}, at: [<ffffffff811387c7>] rcu_gp_kthread+0xb97/0xeb0
{IN-SOFTIRQ-W} state was registered at:
[<ffffffff81109b9f>] __lock_acquire+0xd5f/0x21e0
[<ffffffff8110be0f>] lock_acquire+0xdf/0x2b0
[<ffffffff81841cc9>] _raw_spin_lock_irqsave+0x59/0xa0
[<ffffffff81136991>] rcu_process_callbacks+0x141/0x3c0
[<ffffffff810b1a9d>] __do_softirq+0x14d/0x670
[<ffffffff810b2214>] irq_exit+0x104/0x110
[<ffffffff81844e96>] smp_apic_timer_interrupt+0x46/0x60
[<ffffffff81842e70>] apic_timer_interrupt+0x70/0x80
[<ffffffff810dba66>] rq_attach_root+0xa6/0x100
[<ffffffff810dbc2d>] cpu_attach_domain+0x16d/0x650
[<ffffffff810e4b42>] build_sched_domains+0x942/0xb00
[<ffffffff821777c2>] sched_init_smp+0x509/0x5c1
[<ffffffff821551e3>] kernel_init_freeable+0x172/0x28f
[<ffffffff8182cdce>] kernel_init+0xe/0xe0
[<ffffffff8184231f>] ret_from_fork+0x3f/0x70
irq event stamp: 76
hardirqs last enabled at (75): [<ffffffff81841330>] _raw_spin_unlock_irq+0x30/0x60
hardirqs last disabled at (76): [<ffffffff8184116f>] _raw_spin_lock_irq+0x1f/0x90
softirqs last enabled at (0): [<ffffffff810a8df2>] copy_process.part.26+0x602/0x1cf0
softirqs last disabled at (0): [< (null)>] (null)
other info that might help us debug this:
Possible unsafe locking scenario:
CPU0
----
lock(rcu_node_1);
<Interrupt>
lock(rcu_node_1);
*** DEADLOCK ***
1 lock held by rcu_preempt/8:
#0: (rcu_node_1){+.?...}, at: [<ffffffff811387c7>] rcu_gp_kthread+0xb97/0xeb0
stack backtrace:
CPU: 0 PID: 8 Comm: rcu_preempt Not tainted 4.2.0-rc5-00025-g9a73ba0 #136
Hardware name: Dell Inc. PowerEdge R820/066N7P, BIOS 2.0.20 01/16/2014
0000000000000000 000000006d7e67d8 ffff881fb081fbd8 ffffffff818379e0
0000000000000000 ffff881fb0812a00 ffff881fb081fc38 ffffffff8110813b
0000000000000000 0000000000000001 ffff881f00000001 ffffffff8102fa4f
Call Trace:
[<ffffffff818379e0>] dump_stack+0x4f/0x7b
[<ffffffff8110813b>] print_usage_bug+0x1db/0x1e0
[<ffffffff8102fa4f>] ? save_stack_trace+0x2f/0x50
[<ffffffff811087ad>] mark_lock+0x66d/0x6e0
[<ffffffff81107790>] ? check_usage_forwards+0x150/0x150
[<ffffffff81108898>] mark_held_locks+0x78/0xa0
[<ffffffff81841330>] ? _raw_spin_unlock_irq+0x30/0x60
[<ffffffff81108a28>] trace_hardirqs_on_caller+0x168/0x220
[<ffffffff81108aed>] trace_hardirqs_on+0xd/0x10
[<ffffffff81841330>] _raw_spin_unlock_irq+0x30/0x60
[<ffffffff810fd1c7>] swake_up_all+0xb7/0xe0
[<ffffffff811386e1>] rcu_gp_kthread+0xab1/0xeb0
[<ffffffff811089bf>] ? trace_hardirqs_on_caller+0xff/0x220
[<ffffffff81841341>] ? _raw_spin_unlock_irq+0x41/0x60
[<ffffffff81137c30>] ? rcu_barrier+0x20/0x20
[<ffffffff810d2014>] kthread+0x104/0x120
[<ffffffff81841330>] ? _raw_spin_unlock_irq+0x30/0x60
[<ffffffff810d1f10>] ? kthread_create_on_node+0x260/0x260
[<ffffffff8184231f>] ret_from_fork+0x3f/0x70
[<ffffffff810d1f10>] ? kthread_create_on_node+0x260/0x260
Signed-off-by: Daniel Wagner <daniel.wagner@bmw-carit.de>
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
---
kernel/rcu/tree.c | 4 +++-
kernel/rcu/tree.h | 3 ++-
kernel/rcu/tree_plugin.h | 16 +++++++++++++---
3 files changed, 18 insertions(+), 5 deletions(-)
diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index f07343b..baf6d09 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -1590,7 +1590,6 @@ static int rcu_future_gp_cleanup(struct rcu_state *rsp, struct rcu_node *rnp)
int needmore;
struct rcu_data *rdp = this_cpu_ptr(rsp->rda);
- rcu_nocb_gp_cleanup(rsp, rnp);
rnp->need_future_gp[c & 0x1] = 0;
needmore = rnp->need_future_gp[(c + 1) & 0x1];
trace_rcu_future_gp(rnp, rdp, c,
@@ -1991,6 +1990,7 @@ static void rcu_gp_cleanup(struct rcu_state *rsp)
int nocb = 0;
struct rcu_data *rdp;
struct rcu_node *rnp = rcu_get_root(rsp);
+ wait_queue_head_t *sq;
WRITE_ONCE(rsp->gp_activity, jiffies);
raw_spin_lock_irq(&rnp->lock);
@@ -2029,7 +2029,9 @@ static void rcu_gp_cleanup(struct rcu_state *rsp)
needgp = __note_gp_changes(rsp, rnp, rdp) || needgp;
/* smp_mb() provided by prior unlock-lock pair. */
nocb += rcu_future_gp_cleanup(rsp, rnp);
+ sq = rcu_nocb_gp_get(rnp);
raw_spin_unlock_irq(&rnp->lock);
+ rcu_nocb_gp_cleanup(sq);
cond_resched_rcu_qs();
WRITE_ONCE(rsp->gp_activity, jiffies);
rcu_gp_slow(rsp, gp_cleanup_delay);
diff --git a/kernel/rcu/tree.h b/kernel/rcu/tree.h
index 9fb4e23..aa47e2c 100644
--- a/kernel/rcu/tree.h
+++ b/kernel/rcu/tree.h
@@ -607,7 +607,8 @@ static void zero_cpu_stall_ticks(struct rcu_data *rdp);
static void increment_cpu_stall_ticks(void);
static bool rcu_nocb_cpu_needs_barrier(struct rcu_state *rsp, int cpu);
static void rcu_nocb_gp_set(struct rcu_node *rnp, int nrq);
-static void rcu_nocb_gp_cleanup(struct rcu_state *rsp, struct rcu_node *rnp);
+static wait_queue_head_t *rcu_nocb_gp_get(struct rcu_node *rnp);
+static void rcu_nocb_gp_cleanup(wait_queue_head_t *sq);
static void rcu_init_one_nocb(struct rcu_node *rnp);
static bool __call_rcu_nocb(struct rcu_data *rdp, struct rcu_head *rhp,
bool lazy, unsigned long flags);
diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
index 630c197..99b1ce6 100644
--- a/kernel/rcu/tree_plugin.h
+++ b/kernel/rcu/tree_plugin.h
@@ -1822,9 +1822,9 @@ early_param("rcu_nocb_poll", parse_rcu_nocb_poll);
* Wake up any no-CBs CPUs' kthreads that were waiting on the just-ended
* grace period.
*/
-static void rcu_nocb_gp_cleanup(struct rcu_state *rsp, struct rcu_node *rnp)
+static void rcu_nocb_gp_cleanup(wait_queue_head_t *sq)
{
- wake_up_all(&rnp->nocb_gp_wq[rnp->completed & 0x1]);
+ wake_up_all(sq);
}
/*
@@ -1840,6 +1840,11 @@ static void rcu_nocb_gp_set(struct rcu_node *rnp, int nrq)
rnp->need_future_gp[(rnp->completed + 1) & 0x1] += nrq;
}
+static wait_queue_head_t *rcu_nocb_gp_get(struct rcu_node *rnp)
+{
+ return &rnp->nocb_gp_wq[rnp->completed & 0x1];
+}
+
static void rcu_init_one_nocb(struct rcu_node *rnp)
{
init_waitqueue_head(&rnp->nocb_gp_wq[0]);
@@ -2514,7 +2519,7 @@ static bool rcu_nocb_cpu_needs_barrier(struct rcu_state *rsp, int cpu)
return false;
}
-static void rcu_nocb_gp_cleanup(struct rcu_state *rsp, struct rcu_node *rnp)
+static void rcu_nocb_gp_cleanup(wait_queue_head_t *sq)
{
}
@@ -2522,6 +2527,11 @@ static void rcu_nocb_gp_set(struct rcu_node *rnp, int nrq)
{
}
+static wait_queue_head_t *rcu_nocb_gp_get(struct rcu_node *rnp)
+{
+ return NULL;
+}
+
static void rcu_init_one_nocb(struct rcu_node *rnp)
{
}
--
2.5.0
^ permalink raw reply related [flat|nested] 72+ messages in thread* [PATCH tip v6 5/5] rcu: use simple wait queues where possible in rcutree
2016-01-28 14:44 [PATCH tip v6 0/5] Simple wait queue support Daniel Wagner
` (3 preceding siblings ...)
2016-01-28 14:44 ` [PATCH tip v6 4/5] rcu: Do not call rcu_nocb_gp_cleanup() while holding rnp->lock Daniel Wagner
@ 2016-01-28 14:44 ` Daniel Wagner
2016-01-29 13:23 ` Daniel Wagner
2016-01-29 13:28 ` [PATCH] MIPS: Differentiate between 32 and 64 bit ELF header Daniel Wagner
6 siblings, 0 replies; 72+ messages in thread
From: Daniel Wagner @ 2016-01-28 14:44 UTC (permalink / raw)
To: linux-kernel, linux-rt-users
Cc: linux-kbuild, Marcelo Tosatti, Paolo Bonzini, Paul E . McKenney,
Paul Gortmaker, Peter Zijlstra, Thomas Gleixner, Steven Rostedt,
Boqun Feng, Daniel Wagner
From: Paul Gortmaker <paul.gortmaker@windriver.com>
As of commit dae6e64d2bcfd4b06304ab864c7e3a4f6b5fedf4 ("rcu: Introduce
proper blocking to no-CBs kthreads GP waits") the RCU subsystem started
making use of wait queues.
Here we convert all additions of RCU wait queues to use simple wait queues,
since they don't need the extra overhead of the full wait queue features.
Originally this was done for RT kernels[1], since we would get things like...
BUG: sleeping function called from invalid context at kernel/rtmutex.c:659
in_atomic(): 1, irqs_disabled(): 1, pid: 8, name: rcu_preempt
Pid: 8, comm: rcu_preempt Not tainted
Call Trace:
[<ffffffff8106c8d0>] __might_sleep+0xd0/0xf0
[<ffffffff817d77b4>] rt_spin_lock+0x24/0x50
[<ffffffff8106fcf6>] __wake_up+0x36/0x70
[<ffffffff810c4542>] rcu_gp_kthread+0x4d2/0x680
[<ffffffff8105f910>] ? __init_waitqueue_head+0x50/0x50
[<ffffffff810c4070>] ? rcu_gp_fqs+0x80/0x80
[<ffffffff8105eabb>] kthread+0xdb/0xe0
[<ffffffff8106b912>] ? finish_task_switch+0x52/0x100
[<ffffffff817e0754>] kernel_thread_helper+0x4/0x10
[<ffffffff8105e9e0>] ? __init_kthread_worker+0x60/0x60
[<ffffffff817e0750>] ? gs_change+0xb/0xb
...and hence simple wait queues were deployed on RT out of necessity
(as simple wait uses a raw lock), but mainline might as well take
advantage of the more streamline support as well.
[1] This is a carry forward of work from v3.10-rt; the original conversion
was by Thomas on an earlier -rt version, and Sebastian extended it to
additional post-3.10 added RCU waiters; here I've added a commit log and
unified the RCU changes into one, and uprev'd it to match mainline RCU.
Signed-off-by: Daniel Wagner <daniel.wagner@bmw-carit.de>
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Paul Gortmaker <paul.gortmaker@windriver.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
---
kernel/rcu/tree.c | 22 +++++++++++-----------
kernel/rcu/tree.h | 13 +++++++------
kernel/rcu/tree_plugin.h | 26 +++++++++++++-------------
3 files changed, 31 insertions(+), 30 deletions(-)
diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index baf6d09..c3bfbaa 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -1610,7 +1610,7 @@ static void rcu_gp_kthread_wake(struct rcu_state *rsp)
!READ_ONCE(rsp->gp_flags) ||
!rsp->gp_kthread)
return;
- wake_up(&rsp->gp_wq);
+ swake_up(&rsp->gp_wq);
}
/*
@@ -1990,7 +1990,7 @@ static void rcu_gp_cleanup(struct rcu_state *rsp)
int nocb = 0;
struct rcu_data *rdp;
struct rcu_node *rnp = rcu_get_root(rsp);
- wait_queue_head_t *sq;
+ struct swait_queue_head *sq;
WRITE_ONCE(rsp->gp_activity, jiffies);
raw_spin_lock_irq(&rnp->lock);
@@ -2078,7 +2078,7 @@ static int __noreturn rcu_gp_kthread(void *arg)
READ_ONCE(rsp->gpnum),
TPS("reqwait"));
rsp->gp_state = RCU_GP_WAIT_GPS;
- wait_event_interruptible(rsp->gp_wq,
+ swait_event_interruptible(rsp->gp_wq,
READ_ONCE(rsp->gp_flags) &
RCU_GP_FLAG_INIT);
rsp->gp_state = RCU_GP_DONE_GPS;
@@ -2108,7 +2108,7 @@ static int __noreturn rcu_gp_kthread(void *arg)
READ_ONCE(rsp->gpnum),
TPS("fqswait"));
rsp->gp_state = RCU_GP_WAIT_FQS;
- ret = wait_event_interruptible_timeout(rsp->gp_wq,
+ ret = swait_event_interruptible_timeout(rsp->gp_wq,
rcu_gp_fqs_check_wake(rsp, &gf), j);
rsp->gp_state = RCU_GP_DOING_FQS;
/* Locking provides needed memory barriers. */
@@ -2232,7 +2232,7 @@ static void rcu_report_qs_rsp(struct rcu_state *rsp, unsigned long flags)
WARN_ON_ONCE(!rcu_gp_in_progress(rsp));
WRITE_ONCE(rsp->gp_flags, READ_ONCE(rsp->gp_flags) | RCU_GP_FLAG_FQS);
raw_spin_unlock_irqrestore(&rcu_get_root(rsp)->lock, flags);
- rcu_gp_kthread_wake(rsp);
+ swake_up(&rsp->gp_wq); /* Memory barrier implied by swake_up() path. */
}
/*
@@ -2893,7 +2893,7 @@ static void force_quiescent_state(struct rcu_state *rsp)
}
WRITE_ONCE(rsp->gp_flags, READ_ONCE(rsp->gp_flags) | RCU_GP_FLAG_FQS);
raw_spin_unlock_irqrestore(&rnp_old->lock, flags);
- rcu_gp_kthread_wake(rsp);
+ swake_up(&rsp->gp_wq); /* Memory barrier implied by swake_up() path. */
}
/*
@@ -3526,7 +3526,7 @@ static void __rcu_report_exp_rnp(struct rcu_state *rsp, struct rcu_node *rnp,
raw_spin_unlock_irqrestore(&rnp->lock, flags);
if (wake) {
smp_mb(); /* EGP done before wake_up(). */
- wake_up(&rsp->expedited_wq);
+ swake_up(&rsp->expedited_wq);
}
break;
}
@@ -3783,7 +3783,7 @@ static void synchronize_sched_expedited_wait(struct rcu_state *rsp)
jiffies_start = jiffies;
for (;;) {
- ret = wait_event_interruptible_timeout(
+ ret = swait_event_timeout(
rsp->expedited_wq,
sync_rcu_preempt_exp_done(rnp_root),
jiffies_stall);
@@ -3791,7 +3791,7 @@ static void synchronize_sched_expedited_wait(struct rcu_state *rsp)
return;
if (ret < 0) {
/* Hit a signal, disable CPU stall warnings. */
- wait_event(rsp->expedited_wq,
+ swait_event(rsp->expedited_wq,
sync_rcu_preempt_exp_done(rnp_root));
return;
}
@@ -4457,8 +4457,8 @@ static void __init rcu_init_one(struct rcu_state *rsp,
}
}
- init_waitqueue_head(&rsp->gp_wq);
- init_waitqueue_head(&rsp->expedited_wq);
+ init_swait_queue_head(&rsp->gp_wq);
+ init_swait_queue_head(&rsp->expedited_wq);
rnp = rsp->level[rcu_num_lvls - 1];
for_each_possible_cpu(i) {
while (i > rnp->grphi)
diff --git a/kernel/rcu/tree.h b/kernel/rcu/tree.h
index aa47e2c..8a69b6d 100644
--- a/kernel/rcu/tree.h
+++ b/kernel/rcu/tree.h
@@ -27,6 +27,7 @@
#include <linux/threads.h>
#include <linux/cpumask.h>
#include <linux/seqlock.h>
+#include <linux/swait.h>
#include <linux/stop_machine.h>
/*
@@ -241,7 +242,7 @@ struct rcu_node {
/* Refused to boost: not sure why, though. */
/* This can happen due to race conditions. */
#ifdef CONFIG_RCU_NOCB_CPU
- wait_queue_head_t nocb_gp_wq[2];
+ struct swait_queue_head nocb_gp_wq[2];
/* Place for rcu_nocb_kthread() to wait GP. */
#endif /* #ifdef CONFIG_RCU_NOCB_CPU */
int need_future_gp[2];
@@ -393,7 +394,7 @@ struct rcu_data {
atomic_long_t nocb_q_count_lazy; /* invocation (all stages). */
struct rcu_head *nocb_follower_head; /* CBs ready to invoke. */
struct rcu_head **nocb_follower_tail;
- wait_queue_head_t nocb_wq; /* For nocb kthreads to sleep on. */
+ struct swait_queue_head nocb_wq; /* For nocb kthreads to sleep on. */
struct task_struct *nocb_kthread;
int nocb_defer_wakeup; /* Defer wakeup of nocb_kthread. */
@@ -472,7 +473,7 @@ struct rcu_state {
unsigned long gpnum; /* Current gp number. */
unsigned long completed; /* # of last completed gp. */
struct task_struct *gp_kthread; /* Task for grace periods. */
- wait_queue_head_t gp_wq; /* Where GP task waits. */
+ struct swait_queue_head gp_wq; /* Where GP task waits. */
short gp_flags; /* Commands for GP task. */
short gp_state; /* GP kthread sleep state. */
@@ -504,7 +505,7 @@ struct rcu_state {
atomic_long_t expedited_workdone3; /* # done by others #3. */
atomic_long_t expedited_normal; /* # fallbacks to normal. */
atomic_t expedited_need_qs; /* # CPUs left to check in. */
- wait_queue_head_t expedited_wq; /* Wait for check-ins. */
+ struct swait_queue_head expedited_wq; /* Wait for check-ins. */
int ncpus_snap; /* # CPUs seen last time. */
unsigned long jiffies_force_qs; /* Time at which to invoke */
@@ -607,8 +608,8 @@ static void zero_cpu_stall_ticks(struct rcu_data *rdp);
static void increment_cpu_stall_ticks(void);
static bool rcu_nocb_cpu_needs_barrier(struct rcu_state *rsp, int cpu);
static void rcu_nocb_gp_set(struct rcu_node *rnp, int nrq);
-static wait_queue_head_t *rcu_nocb_gp_get(struct rcu_node *rnp);
-static void rcu_nocb_gp_cleanup(wait_queue_head_t *sq);
+static struct swait_queue_head *rcu_nocb_gp_get(struct rcu_node *rnp);
+static void rcu_nocb_gp_cleanup(struct swait_queue_head *sq);
static void rcu_init_one_nocb(struct rcu_node *rnp);
static bool __call_rcu_nocb(struct rcu_data *rdp, struct rcu_head *rhp,
bool lazy, unsigned long flags);
diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
index 99b1ce6..3891984 100644
--- a/kernel/rcu/tree_plugin.h
+++ b/kernel/rcu/tree_plugin.h
@@ -1822,9 +1822,9 @@ early_param("rcu_nocb_poll", parse_rcu_nocb_poll);
* Wake up any no-CBs CPUs' kthreads that were waiting on the just-ended
* grace period.
*/
-static void rcu_nocb_gp_cleanup(wait_queue_head_t *sq)
+static void rcu_nocb_gp_cleanup(struct swait_queue_head *sq)
{
- wake_up_all(sq);
+ swake_up_all(sq);
}
/*
@@ -1840,15 +1840,15 @@ static void rcu_nocb_gp_set(struct rcu_node *rnp, int nrq)
rnp->need_future_gp[(rnp->completed + 1) & 0x1] += nrq;
}
-static wait_queue_head_t *rcu_nocb_gp_get(struct rcu_node *rnp)
+static struct swait_queue_head *rcu_nocb_gp_get(struct rcu_node *rnp)
{
return &rnp->nocb_gp_wq[rnp->completed & 0x1];
}
static void rcu_init_one_nocb(struct rcu_node *rnp)
{
- init_waitqueue_head(&rnp->nocb_gp_wq[0]);
- init_waitqueue_head(&rnp->nocb_gp_wq[1]);
+ init_swait_queue_head(&rnp->nocb_gp_wq[0]);
+ init_swait_queue_head(&rnp->nocb_gp_wq[1]);
}
#ifndef CONFIG_RCU_NOCB_CPU_ALL
@@ -1873,7 +1873,7 @@ static void wake_nocb_leader(struct rcu_data *rdp, bool force)
if (READ_ONCE(rdp_leader->nocb_leader_sleep) || force) {
/* Prior smp_mb__after_atomic() orders against prior enqueue. */
WRITE_ONCE(rdp_leader->nocb_leader_sleep, false);
- wake_up(&rdp_leader->nocb_wq);
+ swake_up(&rdp_leader->nocb_wq);
}
}
@@ -2086,7 +2086,7 @@ static void rcu_nocb_wait_gp(struct rcu_data *rdp)
*/
trace_rcu_future_gp(rnp, rdp, c, TPS("StartWait"));
for (;;) {
- wait_event_interruptible(
+ swait_event_interruptible(
rnp->nocb_gp_wq[c & 0x1],
(d = ULONG_CMP_GE(READ_ONCE(rnp->completed), c)));
if (likely(d))
@@ -2114,7 +2114,7 @@ wait_again:
/* Wait for callbacks to appear. */
if (!rcu_nocb_poll) {
trace_rcu_nocb_wake(my_rdp->rsp->name, my_rdp->cpu, "Sleep");
- wait_event_interruptible(my_rdp->nocb_wq,
+ swait_event_interruptible(my_rdp->nocb_wq,
!READ_ONCE(my_rdp->nocb_leader_sleep));
/* Memory barrier handled by smp_mb() calls below and repoll. */
} else if (firsttime) {
@@ -2189,7 +2189,7 @@ wait_again:
* List was empty, wake up the follower.
* Memory barriers supplied by atomic_long_add().
*/
- wake_up(&rdp->nocb_wq);
+ swake_up(&rdp->nocb_wq);
}
}
@@ -2210,7 +2210,7 @@ static void nocb_follower_wait(struct rcu_data *rdp)
if (!rcu_nocb_poll) {
trace_rcu_nocb_wake(rdp->rsp->name, rdp->cpu,
"FollowerSleep");
- wait_event_interruptible(rdp->nocb_wq,
+ swait_event_interruptible(rdp->nocb_wq,
READ_ONCE(rdp->nocb_follower_head));
} else if (firsttime) {
/* Don't drown trace log with "Poll"! */
@@ -2369,7 +2369,7 @@ void __init rcu_init_nohz(void)
static void __init rcu_boot_init_nocb_percpu_data(struct rcu_data *rdp)
{
rdp->nocb_tail = &rdp->nocb_head;
- init_waitqueue_head(&rdp->nocb_wq);
+ init_swait_queue_head(&rdp->nocb_wq);
rdp->nocb_follower_tail = &rdp->nocb_follower_head;
}
@@ -2519,7 +2519,7 @@ static bool rcu_nocb_cpu_needs_barrier(struct rcu_state *rsp, int cpu)
return false;
}
-static void rcu_nocb_gp_cleanup(wait_queue_head_t *sq)
+static void rcu_nocb_gp_cleanup(struct swait_queue_head *sq)
{
}
@@ -2527,7 +2527,7 @@ static void rcu_nocb_gp_set(struct rcu_node *rnp, int nrq)
{
}
-static wait_queue_head_t *rcu_nocb_gp_get(struct rcu_node *rnp)
+static struct swait_queue_head *rcu_nocb_gp_get(struct rcu_node *rnp)
{
return NULL;
}
--
2.5.0
^ permalink raw reply related [flat|nested] 72+ messages in thread* [PATCH] video: Use bool instead int pointer for get_opt_bool() argument
2016-01-28 14:44 [PATCH tip v6 0/5] Simple wait queue support Daniel Wagner
@ 2016-01-29 13:23 ` Daniel Wagner
2016-01-28 14:44 ` [PATCH tip v6 2/5] kbuild: Add option to turn incompatible pointer check into error Daniel Wagner
` (5 subsequent siblings)
6 siblings, 0 replies; 72+ messages in thread
From: Daniel Wagner @ 2016-01-29 13:23 UTC (permalink / raw)
To: linux-kernel, linux-fbdev; +Cc: Maik Broemme, Daniel Wagner
As the function name already indicates that get_opt_bool() parses
for a bool. It is not a surprise that compiler is complaining
about it when -Werror=incompatible-pointer-types is used:
drivers/video/fbdev/intelfb/intelfbdrv.c: In function ‘intelfb_setup’:
drivers/video/fbdev/intelfb/intelfbdrv.c:353:39: error: passing argument 3 of ‘get_opt_bool’ from incompatible pointer type [-Werror=incompatible-pointer-types]
if (get_opt_bool(this_opt, "accel", &accel))
Signed-off-by: Daniel Wagner <daniel.wagner@bmw-carit.de>
Reported-by: Fengguang Wu <fengguang.wu@intel.com>
---
Hi,
In the 'simple wait queue support' series is a patch
which turns on -Werror=incompatible-pointer-types which will
result in a compile error for intelfb.
https://lkml.org/lkml/2016/1/28/462
Even if that patch wont make it, this one makes sense (at least
for me :))
I'll prepend this patch to the next version of the series in order
to see if I got rid of all incompatible pointer types errors caught
by the kbuild test robot.
cheers,
daniel
drivers/video/fbdev/intelfb/intelfbdrv.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/intelfb/intelfbdrv.c b/drivers/video/fbdev/intelfb/intelfbdrv.c
index bbec737..bf20744 100644
--- a/drivers/video/fbdev/intelfb/intelfbdrv.c
+++ b/drivers/video/fbdev/intelfb/intelfbdrv.c
@@ -302,7 +302,7 @@ static __inline__ int get_opt_int(const char *this_opt, const char *name,
}
static __inline__ int get_opt_bool(const char *this_opt, const char *name,
- int *ret)
+ bool *ret)
{
if (!ret)
return 0;
--
2.5.0
^ permalink raw reply related [flat|nested] 72+ messages in thread* [PATCH] video: Use bool instead int pointer for get_opt_bool() argument
@ 2016-01-29 13:23 ` Daniel Wagner
0 siblings, 0 replies; 72+ messages in thread
From: Daniel Wagner @ 2016-01-29 13:23 UTC (permalink / raw)
To: linux-kernel, linux-fbdev; +Cc: Maik Broemme, Daniel Wagner
As the function name already indicates that get_opt_bool() parses
for a bool. It is not a surprise that compiler is complaining
about it when -Werror=incompatible-pointer-types is used:
drivers/video/fbdev/intelfb/intelfbdrv.c: In function ‘intelfb_setup’:
drivers/video/fbdev/intelfb/intelfbdrv.c:353:39: error: passing argument 3 of ‘get_opt_bool’ from incompatible pointer type [-Werror=incompatible-pointer-types]
if (get_opt_bool(this_opt, "accel", &accel))
Signed-off-by: Daniel Wagner <daniel.wagner@bmw-carit.de>
Reported-by: Fengguang Wu <fengguang.wu@intel.com>
---
Hi,
In the 'simple wait queue support' series is a patch
which turns on -Werror=incompatible-pointer-types which will
result in a compile error for intelfb.
https://lkml.org/lkml/2016/1/28/462
Even if that patch wont make it, this one makes sense (at least
for me :))
I'll prepend this patch to the next version of the series in order
to see if I got rid of all incompatible pointer types errors caught
by the kbuild test robot.
cheers,
daniel
drivers/video/fbdev/intelfb/intelfbdrv.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/intelfb/intelfbdrv.c b/drivers/video/fbdev/intelfb/intelfbdrv.c
index bbec737..bf20744 100644
--- a/drivers/video/fbdev/intelfb/intelfbdrv.c
+++ b/drivers/video/fbdev/intelfb/intelfbdrv.c
@@ -302,7 +302,7 @@ static __inline__ int get_opt_int(const char *this_opt, const char *name,
}
static __inline__ int get_opt_bool(const char *this_opt, const char *name,
- int *ret)
+ bool *ret)
{
if (!ret)
return 0;
--
2.5.0
^ permalink raw reply related [flat|nested] 72+ messages in thread
* [PATCH] MIPS: Differentiate between 32 and 64 bit ELF header
2016-01-28 14:44 [PATCH tip v6 0/5] Simple wait queue support Daniel Wagner
` (5 preceding siblings ...)
2016-01-29 13:23 ` Daniel Wagner
@ 2016-01-29 13:28 ` Daniel Wagner
2016-02-01 0:52 ` Maciej W. Rozycki
6 siblings, 1 reply; 72+ messages in thread
From: Daniel Wagner @ 2016-01-29 13:28 UTC (permalink / raw)
To: linux-mips, linux-kernel; +Cc: Ralf Baechle, Daniel Wagner
Depending on the configuration either the 32 or 64 bit version of
elf_check_arch() is defined. parse_crash_elf32_headers() does
some basic verification of the ELF header via elf_check_arch().
parse_crash_elf64_headers() does it via vmcore_elf64_check_arch()
which expands to the same elf_check_check().
In file included from include/linux/elf.h:4:0,
from fs/proc/vmcore.c:13:
fs/proc/vmcore.c: In function 'parse_crash_elf64_headers':
>> arch/mips/include/asm/elf.h:228:23: error: initialization from incompatible pointer type [-Werror=incompatible-pointer-types]
struct elfhdr *__h = (hdr); \
^
include/linux/crash_dump.h:41:37: note: in expansion of macro 'elf_check_arch'
#define vmcore_elf64_check_arch(x) (elf_check_arch(x) || vmcore_elf_check_arch_cross(x))
^
fs/proc/vmcore.c:1015:4: note: in expansion of macro 'vmcore_elf64_check_arch'
!vmcore_elf64_check_arch(&ehdr) ||
^
Since the MIPS ELF header for 32 bit and 64 bit differ we need
to check accordingly.
Signed-off-by: Daniel Wagner <daniel.wagner@bmw-carit.de>
Reported-by: Fengguang Wu <fengguang.wu@intel.com>
---
Hi,
In the 'simple wait queue support' series is a patch
which turns on -Werror=incompatible-pointer-types which will
result in a compile error for MIPS.
https://lkml.org/lkml/2016/1/28/462
I am not completely sure if this is the right approach but I could
get rid of the errors by this.
I'll prepend this patch to the next version of the series in order
to see if I got rid of all incompatible pointer types errors caught
by the kbuild test robot.
cheers,
daniel
arch/mips/include/asm/elf.h | 68 ++++++++++++++++++++++++---------------------
1 file changed, 37 insertions(+), 31 deletions(-)
diff --git a/arch/mips/include/asm/elf.h b/arch/mips/include/asm/elf.h
index cefb7a5..7ba0a47 100644
--- a/arch/mips/include/asm/elf.h
+++ b/arch/mips/include/asm/elf.h
@@ -205,27 +205,10 @@ struct mips_elf_abiflags_v0 {
#define MIPS_ABI_FP_64 6 /* -mips32r2 -mfp64 */
#define MIPS_ABI_FP_64A 7 /* -mips32r2 -mfp64 -mno-odd-spreg */
-#ifdef CONFIG_32BIT
-
-/*
- * In order to be sure that we don't attempt to execute an O32 binary which
- * requires 64 bit FP (FR=1) on a system which does not support it we refuse
- * to execute any binary which has bits specified by the following macro set
- * in its ELF header flags.
- */
-#ifdef CONFIG_MIPS_O32_FP64_SUPPORT
-# define __MIPS_O32_FP64_MUST_BE_ZERO 0
-#else
-# define __MIPS_O32_FP64_MUST_BE_ZERO EF_MIPS_FP64
-#endif
-
-/*
- * This is used to ensure we don't load something for the wrong architecture.
- */
-#define elf_check_arch(hdr) \
+#define elf_check_arch_32(hdr) \
({ \
int __res = 1; \
- struct elfhdr *__h = (hdr); \
+ Elf32_Ehdr *__h = (hdr); \
\
if (__h->e_machine != EM_MIPS) \
__res = 0; \
@@ -242,6 +225,40 @@ struct mips_elf_abiflags_v0 {
__res; \
})
+#define elf_check_arch_64(hdr) \
+({ \
+ int __res = 1; \
+ Elf64_Ehdr *__h = (hdr); \
+ \
+ if (__h->e_machine != EM_MIPS) \
+ __res = 0; \
+ if (__h->e_ident[EI_CLASS] != ELFCLASS64) \
+ __res = 0; \
+ \
+ __res; \
+})
+
+#define vmcore_elf64_check_arch(x) (elf_check_arch_64(x) || vmcore_elf_check_arch_cross(x))
+
+#ifdef CONFIG_32BIT
+
+/*
+ * In order to be sure that we don't attempt to execute an O32 binary which
+ * requires 64 bit FP (FR=1) on a system which does not support it we refuse
+ * to execute any binary which has bits specified by the following macro set
+ * in its ELF header flags.
+ */
+#ifdef CONFIG_MIPS_O32_FP64_SUPPORT
+# define __MIPS_O32_FP64_MUST_BE_ZERO 0
+#else
+# define __MIPS_O32_FP64_MUST_BE_ZERO EF_MIPS_FP64
+#endif
+
+/*
+ * This is used to ensure we don't load something for the wrong architecture.
+ */
+#define elf_check_arch(x) elf_check_arch_32(x)
+
/*
* These are used to set parameters in the core dumps.
*/
@@ -253,18 +270,7 @@ struct mips_elf_abiflags_v0 {
/*
* This is used to ensure we don't load something for the wrong architecture.
*/
-#define elf_check_arch(hdr) \
-({ \
- int __res = 1; \
- struct elfhdr *__h = (hdr); \
- \
- if (__h->e_machine != EM_MIPS) \
- __res = 0; \
- if (__h->e_ident[EI_CLASS] != ELFCLASS64) \
- __res = 0; \
- \
- __res; \
-})
+#define elf_check_arch(x) elf_check_arch_64(x)
/*
* These are used to set parameters in the core dumps.
--
2.5.0
^ permalink raw reply related [flat|nested] 72+ messages in thread* Re: [PATCH] MIPS: Differentiate between 32 and 64 bit ELF header
@ 2016-02-01 0:52 ` Maciej W. Rozycki
0 siblings, 0 replies; 72+ messages in thread
From: Maciej W. Rozycki @ 2016-02-01 0:52 UTC (permalink / raw)
To: Daniel Wagner; +Cc: linux-mips, linux-kernel, Ralf Baechle
On Fri, 29 Jan 2016, Daniel Wagner wrote:
> Depending on the configuration either the 32 or 64 bit version of
> elf_check_arch() is defined. parse_crash_elf32_headers() does
> some basic verification of the ELF header via elf_check_arch().
> parse_crash_elf64_headers() does it via vmcore_elf64_check_arch()
> which expands to the same elf_check_check().
>
> In file included from include/linux/elf.h:4:0,
> from fs/proc/vmcore.c:13:
> fs/proc/vmcore.c: In function 'parse_crash_elf64_headers':
> >> arch/mips/include/asm/elf.h:228:23: error: initialization from incompatible pointer type [-Werror=incompatible-pointer-types]
> struct elfhdr *__h = (hdr); \
> ^
> include/linux/crash_dump.h:41:37: note: in expansion of macro 'elf_check_arch'
> #define vmcore_elf64_check_arch(x) (elf_check_arch(x) || vmcore_elf_check_arch_cross(x))
> ^
> fs/proc/vmcore.c:1015:4: note: in expansion of macro 'vmcore_elf64_check_arch'
> !vmcore_elf64_check_arch(&ehdr) ||
> ^
>
> Since the MIPS ELF header for 32 bit and 64 bit differ we need
> to check accordingly.
I fail to see how it can work as it stands given that `elf_check_arch' is
called from the same source file both on a pointer to `Elf32_Ehdr' and one
to `Elf64_Ehdr'. However the MIPS implementations of `elf_check_arch'
only use an auxiliary variable to avoid multiple evaluation of a macro
argument and therefore instead I recommend the use of the usual approach
taken in such a situation within a statement expression, that is to
declare the variable with `typeof' rather than an explicit type. As an
upside this will minimise code disruption as well.
For consistency I suggest making the same change to the `elf_check_arch'
definitions in arch/mips/kernel/binfmt_elf*.c as well.
Maciej
^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH] MIPS: Differentiate between 32 and 64 bit ELF header
@ 2016-02-01 0:52 ` Maciej W. Rozycki
0 siblings, 0 replies; 72+ messages in thread
From: Maciej W. Rozycki @ 2016-02-01 0:52 UTC (permalink / raw)
To: Daniel Wagner; +Cc: linux-mips, linux-kernel, Ralf Baechle
On Fri, 29 Jan 2016, Daniel Wagner wrote:
> Depending on the configuration either the 32 or 64 bit version of
> elf_check_arch() is defined. parse_crash_elf32_headers() does
> some basic verification of the ELF header via elf_check_arch().
> parse_crash_elf64_headers() does it via vmcore_elf64_check_arch()
> which expands to the same elf_check_check().
>
> In file included from include/linux/elf.h:4:0,
> from fs/proc/vmcore.c:13:
> fs/proc/vmcore.c: In function 'parse_crash_elf64_headers':
> >> arch/mips/include/asm/elf.h:228:23: error: initialization from incompatible pointer type [-Werror=incompatible-pointer-types]
> struct elfhdr *__h = (hdr); \
> ^
> include/linux/crash_dump.h:41:37: note: in expansion of macro 'elf_check_arch'
> #define vmcore_elf64_check_arch(x) (elf_check_arch(x) || vmcore_elf_check_arch_cross(x))
> ^
> fs/proc/vmcore.c:1015:4: note: in expansion of macro 'vmcore_elf64_check_arch'
> !vmcore_elf64_check_arch(&ehdr) ||
> ^
>
> Since the MIPS ELF header for 32 bit and 64 bit differ we need
> to check accordingly.
I fail to see how it can work as it stands given that `elf_check_arch' is
called from the same source file both on a pointer to `Elf32_Ehdr' and one
to `Elf64_Ehdr'. However the MIPS implementations of `elf_check_arch'
only use an auxiliary variable to avoid multiple evaluation of a macro
argument and therefore instead I recommend the use of the usual approach
taken in such a situation within a statement expression, that is to
declare the variable with `typeof' rather than an explicit type. As an
upside this will minimise code disruption as well.
For consistency I suggest making the same change to the `elf_check_arch'
definitions in arch/mips/kernel/binfmt_elf*.c as well.
Maciej
^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH] MIPS: Differentiate between 32 and 64 bit ELF header
@ 2016-02-01 16:07 ` Daniel Wagner
0 siblings, 0 replies; 72+ messages in thread
From: Daniel Wagner @ 2016-02-01 16:07 UTC (permalink / raw)
To: Maciej W. Rozycki; +Cc: linux-mips, linux-kernel, Ralf Baechle
On 02/01/2016 01:52 AM, Maciej W. Rozycki wrote:
> On Fri, 29 Jan 2016, Daniel Wagner wrote:
>
>> Depending on the configuration either the 32 or 64 bit version of
>> elf_check_arch() is defined. parse_crash_elf32_headers() does
>> some basic verification of the ELF header via elf_check_arch().
>> parse_crash_elf64_headers() does it via vmcore_elf64_check_arch()
>> which expands to the same elf_check_check().
>>
>> In file included from include/linux/elf.h:4:0,
>> from fs/proc/vmcore.c:13:
>> fs/proc/vmcore.c: In function 'parse_crash_elf64_headers':
>>>> arch/mips/include/asm/elf.h:228:23: error: initialization from incompatible pointer type [-Werror=incompatible-pointer-types]
>> struct elfhdr *__h = (hdr); \
>> ^
>> include/linux/crash_dump.h:41:37: note: in expansion of macro 'elf_check_arch'
>> #define vmcore_elf64_check_arch(x) (elf_check_arch(x) || vmcore_elf_check_arch_cross(x))
>> ^
>> fs/proc/vmcore.c:1015:4: note: in expansion of macro 'vmcore_elf64_check_arch'
>> !vmcore_elf64_check_arch(&ehdr) ||
>> ^
>>
>> Since the MIPS ELF header for 32 bit and 64 bit differ we need
>> to check accordingly.
>
> I fail to see how it can work as it stands given that `elf_check_arch' is
> called from the same source file both on a pointer to `Elf32_Ehdr' and one
> to `Elf64_Ehdr'. However the MIPS implementations of `elf_check_arch'
> only use an auxiliary variable to avoid multiple evaluation of a macro
> argument and therefore instead I recommend the use of the usual approach
> taken in such a situation within a statement expression, that is to
> declare the variable with `typeof' rather than an explicit type. As an
> upside this will minimise code disruption as well.
Good point on the type for hdr. Thought elf_check_arch() implementation
differ on 32 bit and 64 bit implementation. I played a bit around and the
simplest version I found was this here:
diff --git a/arch/mips/include/asm/elf.h b/arch/mips/include/asm/elf.h
index b01a6ff..8c88238 100644
--- a/arch/mips/include/asm/elf.h
+++ b/arch/mips/include/asm/elf.h
@@ -205,8 +205,6 @@ struct mips_elf_abiflags_v0 {
#define MIPS_ABI_FP_64 6 /* -mips32r2 -mfp64 */
#define MIPS_ABI_FP_64A 7 /* -mips32r2 -mfp64 -mno-odd-spreg */
-#ifdef CONFIG_32BIT
-
/*
* In order to be sure that we don't attempt to execute an O32 binary which
* requires 64 bit FP (FR=1) on a system which does not support it we refuse
@@ -225,23 +223,30 @@ struct mips_elf_abiflags_v0 {
#define elf_check_arch(hdr) \
({ \
int __res = 1; \
- struct elfhdr *__h = (hdr); \
+ typeof(*(hdr)) *__h = (hdr); \
\
if (__h->e_machine != EM_MIPS) \
__res = 0; \
- if (__h->e_ident[EI_CLASS] != ELFCLASS32) \
- __res = 0; \
- if ((__h->e_flags & EF_MIPS_ABI2) != 0) \
- __res = 0; \
- if (((__h->e_flags & EF_MIPS_ABI) != 0) && \
- ((__h->e_flags & EF_MIPS_ABI) != EF_MIPS_ABI_O32)) \
- __res = 0; \
- if (__h->e_flags & __MIPS_O32_FP64_MUST_BE_ZERO) \
- __res = 0; \
+ if (__same_type(hdr, Elf32_Ehdr *)) { \
+ if (__h->e_ident[EI_CLASS] != ELFCLASS32) \
+ __res = 0; \
+ if ((__h->e_flags & EF_MIPS_ABI2) != 0) \
+ __res = 0; \
+ if (((__h->e_flags & EF_MIPS_ABI) != 0) && \
+ ((__h->e_flags & EF_MIPS_ABI) != EF_MIPS_ABI_O32)) \
+ __res = 0; \
+ if (__h->e_flags & __MIPS_O32_FP64_MUST_BE_ZERO) \
+ __res = 0; \
+ } else if (__same_type(hdr, Elf64_Ehdr *)) { \
+ if (__h->e_ident[EI_CLASS] != ELFCLASS64) \
+ __res = 0; \
+ } \
\
__res; \
})
+#ifdef CONFIG_32BIT
+
/*
* These are used to set parameters in the core dumps.
*/
@@ -250,21 +255,6 @@ struct mips_elf_abiflags_v0 {
#endif /* CONFIG_32BIT */
#ifdef CONFIG_64BIT
-/*
- * This is used to ensure we don't load something for the wrong architecture.
- */
-#define elf_check_arch(hdr) \
-({ \
- int __res = 1; \
- struct elfhdr *__h = (hdr); \
- \
- if (__h->e_machine != EM_MIPS) \
- __res = 0; \
- if (__h->e_ident[EI_CLASS] != ELFCLASS64) \
- __res = 0; \
- \
- __res; \
-})
/*
* These are used to set parameters in the core dumps.
Not sure if that is what you had in mind.
cheers,
daniel
^ permalink raw reply related [flat|nested] 72+ messages in thread* Re: [PATCH] MIPS: Differentiate between 32 and 64 bit ELF header
@ 2016-02-01 16:07 ` Daniel Wagner
0 siblings, 0 replies; 72+ messages in thread
From: Daniel Wagner @ 2016-02-01 16:07 UTC (permalink / raw)
To: Maciej W. Rozycki; +Cc: linux-mips, linux-kernel, Ralf Baechle
On 02/01/2016 01:52 AM, Maciej W. Rozycki wrote:
> On Fri, 29 Jan 2016, Daniel Wagner wrote:
>
>> Depending on the configuration either the 32 or 64 bit version of
>> elf_check_arch() is defined. parse_crash_elf32_headers() does
>> some basic verification of the ELF header via elf_check_arch().
>> parse_crash_elf64_headers() does it via vmcore_elf64_check_arch()
>> which expands to the same elf_check_check().
>>
>> In file included from include/linux/elf.h:4:0,
>> from fs/proc/vmcore.c:13:
>> fs/proc/vmcore.c: In function 'parse_crash_elf64_headers':
>>>> arch/mips/include/asm/elf.h:228:23: error: initialization from incompatible pointer type [-Werror=incompatible-pointer-types]
>> struct elfhdr *__h = (hdr); \
>> ^
>> include/linux/crash_dump.h:41:37: note: in expansion of macro 'elf_check_arch'
>> #define vmcore_elf64_check_arch(x) (elf_check_arch(x) || vmcore_elf_check_arch_cross(x))
>> ^
>> fs/proc/vmcore.c:1015:4: note: in expansion of macro 'vmcore_elf64_check_arch'
>> !vmcore_elf64_check_arch(&ehdr) ||
>> ^
>>
>> Since the MIPS ELF header for 32 bit and 64 bit differ we need
>> to check accordingly.
>
> I fail to see how it can work as it stands given that `elf_check_arch' is
> called from the same source file both on a pointer to `Elf32_Ehdr' and one
> to `Elf64_Ehdr'. However the MIPS implementations of `elf_check_arch'
> only use an auxiliary variable to avoid multiple evaluation of a macro
> argument and therefore instead I recommend the use of the usual approach
> taken in such a situation within a statement expression, that is to
> declare the variable with `typeof' rather than an explicit type. As an
> upside this will minimise code disruption as well.
Good point on the type for hdr. Thought elf_check_arch() implementation
differ on 32 bit and 64 bit implementation. I played a bit around and the
simplest version I found was this here:
diff --git a/arch/mips/include/asm/elf.h b/arch/mips/include/asm/elf.h
index b01a6ff..8c88238 100644
--- a/arch/mips/include/asm/elf.h
+++ b/arch/mips/include/asm/elf.h
@@ -205,8 +205,6 @@ struct mips_elf_abiflags_v0 {
#define MIPS_ABI_FP_64 6 /* -mips32r2 -mfp64 */
#define MIPS_ABI_FP_64A 7 /* -mips32r2 -mfp64 -mno-odd-spreg */
-#ifdef CONFIG_32BIT
-
/*
* In order to be sure that we don't attempt to execute an O32 binary which
* requires 64 bit FP (FR=1) on a system which does not support it we refuse
@@ -225,23 +223,30 @@ struct mips_elf_abiflags_v0 {
#define elf_check_arch(hdr) \
({ \
int __res = 1; \
- struct elfhdr *__h = (hdr); \
+ typeof(*(hdr)) *__h = (hdr); \
\
if (__h->e_machine != EM_MIPS) \
__res = 0; \
- if (__h->e_ident[EI_CLASS] != ELFCLASS32) \
- __res = 0; \
- if ((__h->e_flags & EF_MIPS_ABI2) != 0) \
- __res = 0; \
- if (((__h->e_flags & EF_MIPS_ABI) != 0) && \
- ((__h->e_flags & EF_MIPS_ABI) != EF_MIPS_ABI_O32)) \
- __res = 0; \
- if (__h->e_flags & __MIPS_O32_FP64_MUST_BE_ZERO) \
- __res = 0; \
+ if (__same_type(hdr, Elf32_Ehdr *)) { \
+ if (__h->e_ident[EI_CLASS] != ELFCLASS32) \
+ __res = 0; \
+ if ((__h->e_flags & EF_MIPS_ABI2) != 0) \
+ __res = 0; \
+ if (((__h->e_flags & EF_MIPS_ABI) != 0) && \
+ ((__h->e_flags & EF_MIPS_ABI) != EF_MIPS_ABI_O32)) \
+ __res = 0; \
+ if (__h->e_flags & __MIPS_O32_FP64_MUST_BE_ZERO) \
+ __res = 0; \
+ } else if (__same_type(hdr, Elf64_Ehdr *)) { \
+ if (__h->e_ident[EI_CLASS] != ELFCLASS64) \
+ __res = 0; \
+ } \
\
__res; \
})
+#ifdef CONFIG_32BIT
+
/*
* These are used to set parameters in the core dumps.
*/
@@ -250,21 +255,6 @@ struct mips_elf_abiflags_v0 {
#endif /* CONFIG_32BIT */
#ifdef CONFIG_64BIT
-/*
- * This is used to ensure we don't load something for the wrong architecture.
- */
-#define elf_check_arch(hdr) \
-({ \
- int __res = 1; \
- struct elfhdr *__h = (hdr); \
- \
- if (__h->e_machine != EM_MIPS) \
- __res = 0; \
- if (__h->e_ident[EI_CLASS] != ELFCLASS64) \
- __res = 0; \
- \
- __res; \
-})
/*
* These are used to set parameters in the core dumps.
Not sure if that is what you had in mind.
cheers,
daniel
^ permalink raw reply related [flat|nested] 72+ messages in thread* Re: [PATCH] MIPS: Differentiate between 32 and 64 bit ELF header
@ 2016-02-06 17:16 ` Maciej W. Rozycki
0 siblings, 0 replies; 72+ messages in thread
From: Maciej W. Rozycki @ 2016-02-06 17:16 UTC (permalink / raw)
To: Daniel Wagner; +Cc: linux-mips, linux-kernel, Ralf Baechle
On Mon, 1 Feb 2016, Daniel Wagner wrote:
> >> Depending on the configuration either the 32 or 64 bit version of
> >> elf_check_arch() is defined. parse_crash_elf32_headers() does
> >> some basic verification of the ELF header via elf_check_arch().
> >> parse_crash_elf64_headers() does it via vmcore_elf64_check_arch()
> >> which expands to the same elf_check_check().
> >>
> >> In file included from include/linux/elf.h:4:0,
> >> from fs/proc/vmcore.c:13:
> >> fs/proc/vmcore.c: In function 'parse_crash_elf64_headers':
> >>>> arch/mips/include/asm/elf.h:228:23: error: initialization from incompatible pointer type [-Werror=incompatible-pointer-types]
> >> struct elfhdr *__h = (hdr); \
> >> ^
> >> include/linux/crash_dump.h:41:37: note: in expansion of macro 'elf_check_arch'
> >> #define vmcore_elf64_check_arch(x) (elf_check_arch(x) || vmcore_elf_check_arch_cross(x))
> >> ^
> >> fs/proc/vmcore.c:1015:4: note: in expansion of macro 'vmcore_elf64_check_arch'
> >> !vmcore_elf64_check_arch(&ehdr) ||
> >> ^
> >>
> >> Since the MIPS ELF header for 32 bit and 64 bit differ we need
> >> to check accordingly.
> >
> > I fail to see how it can work as it stands given that `elf_check_arch' is
> > called from the same source file both on a pointer to `Elf32_Ehdr' and one
> > to `Elf64_Ehdr'. However the MIPS implementations of `elf_check_arch'
> > only use an auxiliary variable to avoid multiple evaluation of a macro
> > argument and therefore instead I recommend the use of the usual approach
> > taken in such a situation within a statement expression, that is to
> > declare the variable with `typeof' rather than an explicit type. As an
> > upside this will minimise code disruption as well.
>
> Good point on the type for hdr. Thought elf_check_arch() implementation
> differ on 32 bit and 64 bit implementation. I played a bit around and the
> simplest version I found was this here:
Umm, somehow I didn't really realise this code wants ELF32 and ELF64
checks both at once -- does it actually make sense? Is a core file from a
kernel crash dump ever going to be the opposite kind to the newly booted
kernel?
Anyway sorry about my confusion and the point above aside I don't really
like the idea of merging both `elf_check_arch' variations into one, it
just looks messy to me. So your original patch was somewhat better after
all, but I think it wasn't enough; for one it didn't handle the 32-bit
case in a 64-bit kernel.
What I think we want to do here is to draw a clear line between ELF32 and
ELF64. So first in include/linux/crash_dump.h:
#ifndef vmcore_elf32_check_arch
#define vmcore_elf32_check_arch elf_check_arch
#endif
and use `vmcore_elf32_check_arch' rather than `elf_check_arch' in
`parse_crash_elf32_headers' in fs/proc/vmcore.c (I think the checks for
ELFCLASS32/ELFCLASS64 ought to go first too, although that'd have to be a
separate patch).
Then we can redefine `vmcore_elf32_check_arch' and
`vmcore_elf64_check_arch' along your first proposal (although we don't
need to refer to `vmcore_elf_check_arch_cross' as we don't define it
anyway). However given that IIUC we're dealing with kernel rather than
userland images here I think we want to skip all the ABI peculiarities and
just accept anything that is compatible with the architecture. It'll then
be the business of whatever tool is going to handle this image to sort out
the details.
So to make things plain we just need:
#define mips_elf_check_machine(x) ((x)->e_machine == EM_MIPS)
#define vmcore_elf32_check_arch mips_elf_check_machine
#define vmcore_elf64_check_arch mips_elf_check_machine
in arch/mips/include/asm/elf.h (and then our definitions of
`elf_check_arch' can be rewritten to use `mips_elf_check_machine' too,
also in arch/mips/kernel/binfmt_elf?32.c).
Questions or comments?
Maciej
^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH] MIPS: Differentiate between 32 and 64 bit ELF header
@ 2016-02-06 17:16 ` Maciej W. Rozycki
0 siblings, 0 replies; 72+ messages in thread
From: Maciej W. Rozycki @ 2016-02-06 17:16 UTC (permalink / raw)
To: Daniel Wagner; +Cc: linux-mips, linux-kernel, Ralf Baechle
On Mon, 1 Feb 2016, Daniel Wagner wrote:
> >> Depending on the configuration either the 32 or 64 bit version of
> >> elf_check_arch() is defined. parse_crash_elf32_headers() does
> >> some basic verification of the ELF header via elf_check_arch().
> >> parse_crash_elf64_headers() does it via vmcore_elf64_check_arch()
> >> which expands to the same elf_check_check().
> >>
> >> In file included from include/linux/elf.h:4:0,
> >> from fs/proc/vmcore.c:13:
> >> fs/proc/vmcore.c: In function 'parse_crash_elf64_headers':
> >>>> arch/mips/include/asm/elf.h:228:23: error: initialization from incompatible pointer type [-Werror=incompatible-pointer-types]
> >> struct elfhdr *__h = (hdr); \
> >> ^
> >> include/linux/crash_dump.h:41:37: note: in expansion of macro 'elf_check_arch'
> >> #define vmcore_elf64_check_arch(x) (elf_check_arch(x) || vmcore_elf_check_arch_cross(x))
> >> ^
> >> fs/proc/vmcore.c:1015:4: note: in expansion of macro 'vmcore_elf64_check_arch'
> >> !vmcore_elf64_check_arch(&ehdr) ||
> >> ^
> >>
> >> Since the MIPS ELF header for 32 bit and 64 bit differ we need
> >> to check accordingly.
> >
> > I fail to see how it can work as it stands given that `elf_check_arch' is
> > called from the same source file both on a pointer to `Elf32_Ehdr' and one
> > to `Elf64_Ehdr'. However the MIPS implementations of `elf_check_arch'
> > only use an auxiliary variable to avoid multiple evaluation of a macro
> > argument and therefore instead I recommend the use of the usual approach
> > taken in such a situation within a statement expression, that is to
> > declare the variable with `typeof' rather than an explicit type. As an
> > upside this will minimise code disruption as well.
>
> Good point on the type for hdr. Thought elf_check_arch() implementation
> differ on 32 bit and 64 bit implementation. I played a bit around and the
> simplest version I found was this here:
Umm, somehow I didn't really realise this code wants ELF32 and ELF64
checks both at once -- does it actually make sense? Is a core file from a
kernel crash dump ever going to be the opposite kind to the newly booted
kernel?
Anyway sorry about my confusion and the point above aside I don't really
like the idea of merging both `elf_check_arch' variations into one, it
just looks messy to me. So your original patch was somewhat better after
all, but I think it wasn't enough; for one it didn't handle the 32-bit
case in a 64-bit kernel.
What I think we want to do here is to draw a clear line between ELF32 and
ELF64. So first in include/linux/crash_dump.h:
#ifndef vmcore_elf32_check_arch
#define vmcore_elf32_check_arch elf_check_arch
#endif
and use `vmcore_elf32_check_arch' rather than `elf_check_arch' in
`parse_crash_elf32_headers' in fs/proc/vmcore.c (I think the checks for
ELFCLASS32/ELFCLASS64 ought to go first too, although that'd have to be a
separate patch).
Then we can redefine `vmcore_elf32_check_arch' and
`vmcore_elf64_check_arch' along your first proposal (although we don't
need to refer to `vmcore_elf_check_arch_cross' as we don't define it
anyway). However given that IIUC we're dealing with kernel rather than
userland images here I think we want to skip all the ABI peculiarities and
just accept anything that is compatible with the architecture. It'll then
be the business of whatever tool is going to handle this image to sort out
the details.
So to make things plain we just need:
#define mips_elf_check_machine(x) ((x)->e_machine == EM_MIPS)
#define vmcore_elf32_check_arch mips_elf_check_machine
#define vmcore_elf64_check_arch mips_elf_check_machine
in arch/mips/include/asm/elf.h (and then our definitions of
`elf_check_arch' can be rewritten to use `mips_elf_check_machine' too,
also in arch/mips/kernel/binfmt_elf?32.c).
Questions or comments?
Maciej
^ permalink raw reply [flat|nested] 72+ messages in thread
* [PATCH v3 0/3] Differentiate between 32 and 64 bit ELF header
2016-02-06 17:16 ` Maciej W. Rozycki
(?)
@ 2016-02-08 15:44 ` Daniel Wagner
2016-02-08 15:44 ` [PATCH v3 1/3] mips: Use arch specific auxvec.h instead of generic-asm version Daniel Wagner
` (2 more replies)
-1 siblings, 3 replies; 72+ messages in thread
From: Daniel Wagner @ 2016-02-08 15:44 UTC (permalink / raw)
To: Maciej W. Rozycki; +Cc: Ralf Baechle, linux-kernel, linux-mips, Daniel Wagner
Hi Maciej,
Thanks a lot for your input. It looks like we getting somewhere.
This version is much smaller and not so invasive as the prevision one.
I had some trouble with my cross compile setup. The first patch addresses
this problem. If I got it right it is just a missing include wrapper file.
cheers,
daniel
Daniel Wagner (3):
mips: Use arch specific auxvec.h instead of generic-asm version
crash_dump: Add vmcore_elf32_check_arch
mips: Differentiate between 32 and 64 bit ELF header
arch/mips/include/asm/auxvec.h | 1 +
arch/mips/include/asm/elf.h | 9 +++++++--
arch/mips/kernel/binfmt_elfn32.c | 2 +-
arch/mips/kernel/binfmt_elfo32.c | 2 +-
fs/proc/vmcore.c | 2 +-
include/linux/crash_dump.h | 8 ++++++--
6 files changed, 17 insertions(+), 7 deletions(-)
create mode 100644 arch/mips/include/asm/auxvec.h
--
2.5.0
^ permalink raw reply [flat|nested] 72+ messages in thread* [PATCH v3 1/3] mips: Use arch specific auxvec.h instead of generic-asm version
2016-02-08 15:44 ` [PATCH v3 0/3] " Daniel Wagner
@ 2016-02-08 15:44 ` Daniel Wagner
2016-02-08 17:19 ` Maciej W. Rozycki
2016-02-08 15:44 ` [PATCH v3 2/3] crash_dump: Add vmcore_elf32_check_arch Daniel Wagner
2016-02-08 15:44 ` [PATCH v3 3/3] mips: Differentiate between 32 and 64 bit ELF header Daniel Wagner
2 siblings, 1 reply; 72+ messages in thread
From: Daniel Wagner @ 2016-02-08 15:44 UTC (permalink / raw)
To: Maciej W. Rozycki; +Cc: Ralf Baechle, linux-kernel, linux-mips, Daniel Wagner
The generic auxvec.h is used instead the arch specific version.
This happens when cross compiling the kernel.
mips64-linux-gnu-gcc (GCC) 5.2.1 20151104 (Red Hat Cross 5.2.1-4)
arch/mips/kernel/../../../fs/binfmt_elf.c: In function ‘create_elf_tables’:
./arch/mips/include/asm/elf.h:425:14: error: ‘AT_SYSINFO_EHDR’ undeclared (first use in this function)
NEW_AUX_ENT(AT_SYSINFO_EHDR, \
^
arch/mips/kernel/../../../fs/binfmt_elf.c:222:26: note: in definition of macro ‘NEW_AUX_ENT’
elf_info[ei_index++] = id; \
^
arch/mips/kernel/../../../fs/binfmt_elf.c:233:2: note: in expansion of macro ‘ARCH_DLINFO’
ARCH_DLINFO;
^
./arch/mips/include/asm/elf.h:425:14: note: each undeclared identifier is reported only once for each function it appears in
NEW_AUX_ENT(AT_SYSINFO_EHDR, \
^
arch/mips/kernel/../../../fs/binfmt_elf.c:222:26: note: in definition of macro ‘NEW_AUX_ENT’
elf_info[ei_index++] = id; \
^
arch/mips/kernel/../../../fs/binfmt_elf.c:233:2: note: in expansion of macro ‘ARCH_DLINFO’
ARCH_DLINFO;
^
Signed-off-by: Daniel Wagner <daniel.wagner@bmw-carit.de>
---
arch/mips/include/asm/auxvec.h | 1 +
1 file changed, 1 insertion(+)
create mode 100644 arch/mips/include/asm/auxvec.h
diff --git a/arch/mips/include/asm/auxvec.h b/arch/mips/include/asm/auxvec.h
new file mode 100644
index 0000000..fbd388c
--- /dev/null
+++ b/arch/mips/include/asm/auxvec.h
@@ -0,0 +1 @@
+#include <uapi/asm/auxvec.h>
--
2.5.0
^ permalink raw reply related [flat|nested] 72+ messages in thread* Re: [PATCH v3 1/3] mips: Use arch specific auxvec.h instead of generic-asm version
@ 2016-02-08 17:19 ` Maciej W. Rozycki
0 siblings, 0 replies; 72+ messages in thread
From: Maciej W. Rozycki @ 2016-02-08 17:19 UTC (permalink / raw)
To: Daniel Wagner; +Cc: Ralf Baechle, linux-kernel, linux-mips
On Mon, 8 Feb 2016, Daniel Wagner wrote:
> The generic auxvec.h is used instead the arch specific version.
> This happens when cross compiling the kernel.
>
> mips64-linux-gnu-gcc (GCC) 5.2.1 20151104 (Red Hat Cross 5.2.1-4)
>
> arch/mips/kernel/../../../fs/binfmt_elf.c: In function ‘create_elf_tables’:
> ./arch/mips/include/asm/elf.h:425:14: error: ‘AT_SYSINFO_EHDR’ undeclared (first use in this function)
There must be something wrong with your setup, or maybe a bug somewhere
in our build machinery you just happened to trigger. Most of us routinely
use a cross-compiler to build the kernel and you're the first one to
report the problem.
Can you report the compiler invocation that has lead to this error?
Have you used a default config or a custom one?
> diff --git a/arch/mips/include/asm/auxvec.h b/arch/mips/include/asm/auxvec.h
> new file mode 100644
> index 0000000..fbd388c
> --- /dev/null
> +++ b/arch/mips/include/asm/auxvec.h
> @@ -0,0 +1 @@
> +#include <uapi/asm/auxvec.h>
You're not supposed to require a header in asm/ merely to include a
header of the same name from uapi/asm/ as there are normally
-I./arch/mips/include and -I./arch/mips/include/uapi options present both
at once, in this order, on the compiler's invocation line. So:
#include <asm/auxvec.h>
will pull the header from uapi/asm/ if none is present in asm/.
Maciej
^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH v3 1/3] mips: Use arch specific auxvec.h instead of generic-asm version
@ 2016-02-08 17:19 ` Maciej W. Rozycki
0 siblings, 0 replies; 72+ messages in thread
From: Maciej W. Rozycki @ 2016-02-08 17:19 UTC (permalink / raw)
To: Daniel Wagner; +Cc: Ralf Baechle, linux-kernel, linux-mips
On Mon, 8 Feb 2016, Daniel Wagner wrote:
> The generic auxvec.h is used instead the arch specific version.
> This happens when cross compiling the kernel.
>
> mips64-linux-gnu-gcc (GCC) 5.2.1 20151104 (Red Hat Cross 5.2.1-4)
>
> arch/mips/kernel/../../../fs/binfmt_elf.c: In function ‘create_elf_tables’:
> ./arch/mips/include/asm/elf.h:425:14: error: ‘AT_SYSINFO_EHDR’ undeclared (first use in this function)
There must be something wrong with your setup, or maybe a bug somewhere
in our build machinery you just happened to trigger. Most of us routinely
use a cross-compiler to build the kernel and you're the first one to
report the problem.
Can you report the compiler invocation that has lead to this error?
Have you used a default config or a custom one?
> diff --git a/arch/mips/include/asm/auxvec.h b/arch/mips/include/asm/auxvec.h
> new file mode 100644
> index 0000000..fbd388c
> --- /dev/null
> +++ b/arch/mips/include/asm/auxvec.h
> @@ -0,0 +1 @@
> +#include <uapi/asm/auxvec.h>
You're not supposed to require a header in asm/ merely to include a
header of the same name from uapi/asm/ as there are normally
-I./arch/mips/include and -I./arch/mips/include/uapi options present both
at once, in this order, on the compiler's invocation line. So:
#include <asm/auxvec.h>
will pull the header from uapi/asm/ if none is present in asm/.
Maciej
^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH v3 1/3] mips: Use arch specific auxvec.h instead of generic-asm version
@ 2016-02-09 7:01 ` Daniel Wagner
0 siblings, 0 replies; 72+ messages in thread
From: Daniel Wagner @ 2016-02-09 7:01 UTC (permalink / raw)
To: Maciej W. Rozycki; +Cc: Ralf Baechle, linux-kernel, linux-mips
Good Morning,
On 02/08/2016 06:19 PM, Maciej W. Rozycki wrote:
> On Mon, 8 Feb 2016, Daniel Wagner wrote:
>
>> The generic auxvec.h is used instead the arch specific version.
>> This happens when cross compiling the kernel.
>>
>> mips64-linux-gnu-gcc (GCC) 5.2.1 20151104 (Red Hat Cross 5.2.1-4)
>>
>> arch/mips/kernel/../../../fs/binfmt_elf.c: In function ‘create_elf_tables’:
>> ./arch/mips/include/asm/elf.h:425:14: error: ‘AT_SYSINFO_EHDR’ undeclared (first use in this function)
>
> There must be something wrong with your setup, or maybe a bug somewhere
> in our build machinery you just happened to trigger. Most of us routinely
> use a cross-compiler to build the kernel and you're the first one to
> report the problem.
Yeah, I thought so too and I would also bet on the toolchain. After
'fixing' this small problem I got a nice and shiny binary without any
other warnings or errors.
> Can you report the compiler invocation that has lead to this error?
/usr/bin/mips64-linux-gnu-gcc -Wp,-MD,fs/.binfmt_elf.o.d -nostdinc -isystem /usr/lib/gcc/mips64-linux-gnu/5.2.1/include -I./arch/mips/include -Iarch/mips/include/generated/uapi -Iarch/mips/include/generated -Iinclude -I./arch/mips/include/uapi -Iarch/mips/include/generated/uapi -I./include/uapi -Iinclude/generated/uapi -include ./include/linux/kconfig.h -D__KERNEL__ -DVMLINUX_LOAD_ADDRESS=0xffffffff88002000 -DDATAOFFSET=0 -Wall -Wundef -Wstrict-prototypes -Wno-trigraphs -fno-strict-aliasing -fno-common -Werror-implicit-function-declaration -Wno-format-security -std=gnu89 -mno-check-zero-division -mabi=32 -G 0 -mno-abicalls -fno-pic -pipe -msoft-float -DGAS_HAS_SET_HARDFLOAT -Wa,-msoft-float -ffreestanding -march=r5000 -Wa,--trap -I./arch/mips/include/asm/mach-ip22 -I./arch/mips/include/asm/mach-generic -fno-delete-null-pointer-checks -O2 --param=allow-store-data-races=0 -Wframe-larger-than=1024 -fno-stack-protector -Wno-unused-but-set-variable -fomit-frame-pointer -fno-var-tracking
-assignments -Wdeclaration-after-statement -Wno-pointer-sign -fno-strict-overflow -fconserve-stack -Werror=implicit-int -Werror=strict-prototypes -Werror=date-time -DCC_HAVE_ASM_GOTO -D"KBUILD_STR(s)=#s" -D"KBUILD_BASENAME=KBUILD_STR(binfmt_elf)" -D"KBUILD_MODNAME=KBUILD_STR(binfmt_elf)" -c -o fs/.tmp_binfmt_elf.o fs/binfmt_elf.c
> Have you used a default config or a custom one?
I used the default one per 'make defconfig ARCH=mips
CROSS_COMPILE=/usr/bin/mips64-linux-gnu-' with Fedora 23 MIPS cross
toolchain.
>> diff --git a/arch/mips/include/asm/auxvec.h b/arch/mips/include/asm/auxvec.h
>> new file mode 100644
>> index 0000000..fbd388c
>> --- /dev/null
>> +++ b/arch/mips/include/asm/auxvec.h
>> @@ -0,0 +1 @@
>> +#include <uapi/asm/auxvec.h>
>
> You're not supposed to require a header in asm/ merely to include a
> header of the same name from uapi/asm/ as there are normally
> -I./arch/mips/include and -I./arch/mips/include/uapi options present both
> at once, in this order, on the compiler's invocation line. So:
>
> #include <asm/auxvec.h>
>
> will pull the header from uapi/asm/ if none is present in asm/.
Okay, thanks for the explanation. I was pretty confused by the build
machinery and saw this include for ARM arch which provides also a their
own uapi/asm/auxvec.h
Also I looked at the cpp output and saw that there was no uapi/asm/auxvec.h
included instead it pulls arch/mips/include/generated/uapi/asm/auxvec.h
Not working version:
# 1 "arch/mips/include/generated/uapi/asm/auxvec.h" 1
# 1 "./include/uapi/asm-generic/auxvec.h" 1
# 1 "arch/mips/include/generated/uapi/asm/auxvec.h" 2
# 5 "include/uapi/linux/auxvec.h" 2
# 5 "include/linux/auxvec.h" 2
# 12 "./arch/mips/include/asm/elf.h" 2
# 1 "include/linux/fs.h" 1
working version:
# 1 "./arch/mips/include/asm/auxvec.h" 1
# 1 "./arch/mips/include/uapi/asm/auxvec.h" 1
# 1 "./arch/mips/include/asm/auxvec.h" 2
# 5 "include/uapi/linux/auxvec.h" 2
# 5 "include/linux/auxvec.h" 2
# 12 "./arch/mips/include/asm/elf.h" 2
# 1 "include/linux/fs.h" 1
I've uploaded the cpp output and the config just in case:
https://www.monom.org/data/mips/auxvec/
I am still pretty confused about what should happen in which order.
Maybe I should call the Confuse-A-Cat squat team.
cheers,
daniel
^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH v3 1/3] mips: Use arch specific auxvec.h instead of generic-asm version
@ 2016-02-09 7:01 ` Daniel Wagner
0 siblings, 0 replies; 72+ messages in thread
From: Daniel Wagner @ 2016-02-09 7:01 UTC (permalink / raw)
To: Maciej W. Rozycki; +Cc: Ralf Baechle, linux-kernel, linux-mips
Good Morning,
On 02/08/2016 06:19 PM, Maciej W. Rozycki wrote:
> On Mon, 8 Feb 2016, Daniel Wagner wrote:
>
>> The generic auxvec.h is used instead the arch specific version.
>> This happens when cross compiling the kernel.
>>
>> mips64-linux-gnu-gcc (GCC) 5.2.1 20151104 (Red Hat Cross 5.2.1-4)
>>
>> arch/mips/kernel/../../../fs/binfmt_elf.c: In function ‘create_elf_tables’:
>> ./arch/mips/include/asm/elf.h:425:14: error: ‘AT_SYSINFO_EHDR’ undeclared (first use in this function)
>
> There must be something wrong with your setup, or maybe a bug somewhere
> in our build machinery you just happened to trigger. Most of us routinely
> use a cross-compiler to build the kernel and you're the first one to
> report the problem.
Yeah, I thought so too and I would also bet on the toolchain. After
'fixing' this small problem I got a nice and shiny binary without any
other warnings or errors.
> Can you report the compiler invocation that has lead to this error?
/usr/bin/mips64-linux-gnu-gcc -Wp,-MD,fs/.binfmt_elf.o.d -nostdinc -isystem /usr/lib/gcc/mips64-linux-gnu/5.2.1/include -I./arch/mips/include -Iarch/mips/include/generated/uapi -Iarch/mips/include/generated -Iinclude -I./arch/mips/include/uapi -Iarch/mips/include/generated/uapi -I./include/uapi -Iinclude/generated/uapi -include ./include/linux/kconfig.h -D__KERNEL__ -DVMLINUX_LOAD_ADDRESS=0xffffffff88002000 -DDATAOFFSET=0 -Wall -Wundef -Wstrict-prototypes -Wno-trigraphs -fno-strict-aliasing -fno-common -Werror-implicit-function-declaration -Wno-format-security -std=gnu89 -mno-check-zero-division -mabi=32 -G 0 -mno-abicalls -fno-pic -pipe -msoft-float -DGAS_HAS_SET_HARDFLOAT -Wa,-msoft-float -ffreestanding -march=r5000 -Wa,--trap -I./arch/mips/include/asm/mach-ip22 -I./arch/mips/include/asm/mach-generic -fno-delete-null-pointer-checks -O2 --param=allow-store-data-races=0 -Wframe-larger-than=1024 -fno-stack-protector -Wno-unused-but-set-variable -fomit-frame-pointer -fno-var-tracking
-assignments -Wdeclaration-after-statement -Wno-pointer-sign -fno-strict-overflow -fconserve-stack -Werror=implicit-int -Werror=strict-prototypes -Werror=date-time -DCC_HAVE_ASM_GOTO -D"KBUILD_STR(s)=#s" -D"KBUILD_BASENAME=KBUILD_STR(binfmt_elf)" -D"KBUILD_MODNAME=KBUILD_STR(binfmt_elf)" -c -o fs/.tmp_binfmt_elf.o fs/binfmt_elf.c
> Have you used a default config or a custom one?
I used the default one per 'make defconfig ARCH=mips
CROSS_COMPILE=/usr/bin/mips64-linux-gnu-' with Fedora 23 MIPS cross
toolchain.
>> diff --git a/arch/mips/include/asm/auxvec.h b/arch/mips/include/asm/auxvec.h
>> new file mode 100644
>> index 0000000..fbd388c
>> --- /dev/null
>> +++ b/arch/mips/include/asm/auxvec.h
>> @@ -0,0 +1 @@
>> +#include <uapi/asm/auxvec.h>
>
> You're not supposed to require a header in asm/ merely to include a
> header of the same name from uapi/asm/ as there are normally
> -I./arch/mips/include and -I./arch/mips/include/uapi options present both
> at once, in this order, on the compiler's invocation line. So:
>
> #include <asm/auxvec.h>
>
> will pull the header from uapi/asm/ if none is present in asm/.
Okay, thanks for the explanation. I was pretty confused by the build
machinery and saw this include for ARM arch which provides also a their
own uapi/asm/auxvec.h
Also I looked at the cpp output and saw that there was no uapi/asm/auxvec.h
included instead it pulls arch/mips/include/generated/uapi/asm/auxvec.h
Not working version:
# 1 "arch/mips/include/generated/uapi/asm/auxvec.h" 1
# 1 "./include/uapi/asm-generic/auxvec.h" 1
# 1 "arch/mips/include/generated/uapi/asm/auxvec.h" 2
# 5 "include/uapi/linux/auxvec.h" 2
# 5 "include/linux/auxvec.h" 2
# 12 "./arch/mips/include/asm/elf.h" 2
# 1 "include/linux/fs.h" 1
working version:
# 1 "./arch/mips/include/asm/auxvec.h" 1
# 1 "./arch/mips/include/uapi/asm/auxvec.h" 1
# 1 "./arch/mips/include/asm/auxvec.h" 2
# 5 "include/uapi/linux/auxvec.h" 2
# 5 "include/linux/auxvec.h" 2
# 12 "./arch/mips/include/asm/elf.h" 2
# 1 "include/linux/fs.h" 1
I've uploaded the cpp output and the config just in case:
https://www.monom.org/data/mips/auxvec/
I am still pretty confused about what should happen in which order.
Maybe I should call the Confuse-A-Cat squat team.
cheers,
daniel
^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH v3 1/3] mips: Use arch specific auxvec.h instead of generic-asm version
@ 2016-02-09 11:46 ` Maciej W. Rozycki
0 siblings, 0 replies; 72+ messages in thread
From: Maciej W. Rozycki @ 2016-02-09 11:46 UTC (permalink / raw)
To: Daniel Wagner; +Cc: Ralf Baechle, linux-kernel, linux-mips
On Tue, 9 Feb 2016, Daniel Wagner wrote:
> Also I looked at the cpp output and saw that there was no uapi/asm/auxvec.h
> included instead it pulls arch/mips/include/generated/uapi/asm/auxvec.h
Hmm, did you update your source in an old build tree and reuse it for a
new build? The rule to make arch/mips/include/generated/uapi/asm/auxvec.h
was removed with commit ebb5e78cc634 ("MIPS: Initial implementation of a
VDSO") as arch/mips/include/uapi/asm/auxvec.h was added, in the 4.4-rc1
timeframe. So the generated version is not supposed to be there anymore.
Can you try `make mrproper' (stash away your .config) and see if the
problem goes away?
Maciej
^ permalink raw reply [flat|nested] 72+ messages in thread* Re: [PATCH v3 1/3] mips: Use arch specific auxvec.h instead of generic-asm version
@ 2016-02-09 11:46 ` Maciej W. Rozycki
0 siblings, 0 replies; 72+ messages in thread
From: Maciej W. Rozycki @ 2016-02-09 11:46 UTC (permalink / raw)
To: Daniel Wagner; +Cc: Ralf Baechle, linux-kernel, linux-mips
On Tue, 9 Feb 2016, Daniel Wagner wrote:
> Also I looked at the cpp output and saw that there was no uapi/asm/auxvec.h
> included instead it pulls arch/mips/include/generated/uapi/asm/auxvec.h
Hmm, did you update your source in an old build tree and reuse it for a
new build? The rule to make arch/mips/include/generated/uapi/asm/auxvec.h
was removed with commit ebb5e78cc634 ("MIPS: Initial implementation of a
VDSO") as arch/mips/include/uapi/asm/auxvec.h was added, in the 4.4-rc1
timeframe. So the generated version is not supposed to be there anymore.
Can you try `make mrproper' (stash away your .config) and see if the
problem goes away?
Maciej
^ permalink raw reply [flat|nested] 72+ messages in thread* Re: [PATCH v3 1/3] mips: Use arch specific auxvec.h instead of generic-asm version
@ 2016-02-09 12:37 ` Daniel Wagner
0 siblings, 0 replies; 72+ messages in thread
From: Daniel Wagner @ 2016-02-09 12:37 UTC (permalink / raw)
To: Maciej W. Rozycki; +Cc: Ralf Baechle, linux-kernel, linux-mips
On 02/09/2016 12:46 PM, Maciej W. Rozycki wrote:
> On Tue, 9 Feb 2016, Daniel Wagner wrote:
>
>> Also I looked at the cpp output and saw that there was no uapi/asm/auxvec.h
>> included instead it pulls arch/mips/include/generated/uapi/asm/auxvec.h
>
> Hmm, did you update your source in an old build tree and reuse it for a
> new build? The rule to make arch/mips/include/generated/uapi/asm/auxvec.h
> was removed with commit ebb5e78cc634 ("MIPS: Initial implementation of a
> VDSO") as arch/mips/include/uapi/asm/auxvec.h was added, in the 4.4-rc1
> timeframe. So the generated version is not supposed to be there anymore.
>
> Can you try `make mrproper' (stash away your .config) and see if the
> problem goes away?
Indeed, 'make mrproper' did the trick. I am sorry for the noise. Until
now I never had to use mrproper before and therefore didn't think of it.
Thanks a lot!
Daniel
^ permalink raw reply [flat|nested] 72+ messages in thread* Re: [PATCH v3 1/3] mips: Use arch specific auxvec.h instead of generic-asm version
@ 2016-02-09 12:37 ` Daniel Wagner
0 siblings, 0 replies; 72+ messages in thread
From: Daniel Wagner @ 2016-02-09 12:37 UTC (permalink / raw)
To: Maciej W. Rozycki; +Cc: Ralf Baechle, linux-kernel, linux-mips
On 02/09/2016 12:46 PM, Maciej W. Rozycki wrote:
> On Tue, 9 Feb 2016, Daniel Wagner wrote:
>
>> Also I looked at the cpp output and saw that there was no uapi/asm/auxvec.h
>> included instead it pulls arch/mips/include/generated/uapi/asm/auxvec.h
>
> Hmm, did you update your source in an old build tree and reuse it for a
> new build? The rule to make arch/mips/include/generated/uapi/asm/auxvec.h
> was removed with commit ebb5e78cc634 ("MIPS: Initial implementation of a
> VDSO") as arch/mips/include/uapi/asm/auxvec.h was added, in the 4.4-rc1
> timeframe. So the generated version is not supposed to be there anymore.
>
> Can you try `make mrproper' (stash away your .config) and see if the
> problem goes away?
Indeed, 'make mrproper' did the trick. I am sorry for the noise. Until
now I never had to use mrproper before and therefore didn't think of it.
Thanks a lot!
Daniel
^ permalink raw reply [flat|nested] 72+ messages in thread* Re: [PATCH v3 1/3] mips: Use arch specific auxvec.h instead of generic-asm version
@ 2016-02-09 14:51 ` Maciej W. Rozycki
0 siblings, 0 replies; 72+ messages in thread
From: Maciej W. Rozycki @ 2016-02-09 14:51 UTC (permalink / raw)
To: Daniel Wagner; +Cc: Ralf Baechle, linux-kernel, linux-mips
On Tue, 9 Feb 2016, Daniel Wagner wrote:
> > Can you try `make mrproper' (stash away your .config) and see if the
> > problem goes away?
>
> Indeed, 'make mrproper' did the trick. I am sorry for the noise. Until
> now I never had to use mrproper before and therefore didn't think of it.
People have been being hit by stale generated files recently and I reckon
effort has been taken to address the issue by removing them automagically
somehow on rebuilds. Until that has been complete you're advised to clean
your build tree after an update, or maybe rebuild speculatively and then
clean only if something actually breaks.
Maciej
^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH v3 1/3] mips: Use arch specific auxvec.h instead of generic-asm version
@ 2016-02-09 14:51 ` Maciej W. Rozycki
0 siblings, 0 replies; 72+ messages in thread
From: Maciej W. Rozycki @ 2016-02-09 14:51 UTC (permalink / raw)
To: Daniel Wagner; +Cc: Ralf Baechle, linux-kernel, linux-mips
On Tue, 9 Feb 2016, Daniel Wagner wrote:
> > Can you try `make mrproper' (stash away your .config) and see if the
> > problem goes away?
>
> Indeed, 'make mrproper' did the trick. I am sorry for the noise. Until
> now I never had to use mrproper before and therefore didn't think of it.
People have been being hit by stale generated files recently and I reckon
effort has been taken to address the issue by removing them automagically
somehow on rebuilds. Until that has been complete you're advised to clean
your build tree after an update, or maybe rebuild speculatively and then
clean only if something actually breaks.
Maciej
^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH v3 1/3] mips: Use arch specific auxvec.h instead of generic-asm version
@ 2016-02-10 8:51 ` Daniel Wagner
0 siblings, 0 replies; 72+ messages in thread
From: Daniel Wagner @ 2016-02-10 8:51 UTC (permalink / raw)
To: Maciej W. Rozycki; +Cc: Ralf Baechle, linux-kernel, linux-mips
On 02/09/2016 03:51 PM, Maciej W. Rozycki wrote:
> On Tue, 9 Feb 2016, Daniel Wagner wrote:
>
>>> Can you try `make mrproper' (stash away your .config) and see if the
>>> problem goes away?
>>
>> Indeed, 'make mrproper' did the trick. I am sorry for the noise. Until
>> now I never had to use mrproper before and therefore didn't think of it.
>
> People have been being hit by stale generated files recently and I reckon
> effort has been taken to address the issue by removing them automagically
> somehow on rebuilds. Until that has been complete you're advised to clean
> your build tree after an update, or maybe rebuild speculatively and then
> clean only if something actually breaks.
FWIW, I just found out that running mrproper before building
fuloong2e_defconfig is fixing the problem reported kbuild robot too.
^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH v3 1/3] mips: Use arch specific auxvec.h instead of generic-asm version
@ 2016-02-10 8:51 ` Daniel Wagner
0 siblings, 0 replies; 72+ messages in thread
From: Daniel Wagner @ 2016-02-10 8:51 UTC (permalink / raw)
To: Maciej W. Rozycki; +Cc: Ralf Baechle, linux-kernel, linux-mips
On 02/09/2016 03:51 PM, Maciej W. Rozycki wrote:
> On Tue, 9 Feb 2016, Daniel Wagner wrote:
>
>>> Can you try `make mrproper' (stash away your .config) and see if the
>>> problem goes away?
>>
>> Indeed, 'make mrproper' did the trick. I am sorry for the noise. Until
>> now I never had to use mrproper before and therefore didn't think of it.
>
> People have been being hit by stale generated files recently and I reckon
> effort has been taken to address the issue by removing them automagically
> somehow on rebuilds. Until that has been complete you're advised to clean
> your build tree after an update, or maybe rebuild speculatively and then
> clean only if something actually breaks.
FWIW, I just found out that running mrproper before building
fuloong2e_defconfig is fixing the problem reported kbuild robot too.
^ permalink raw reply [flat|nested] 72+ messages in thread
* [PATCH v3 2/3] crash_dump: Add vmcore_elf32_check_arch
2016-02-08 15:44 ` [PATCH v3 0/3] " Daniel Wagner
2016-02-08 15:44 ` [PATCH v3 1/3] mips: Use arch specific auxvec.h instead of generic-asm version Daniel Wagner
@ 2016-02-08 15:44 ` Daniel Wagner
2016-02-08 17:05 ` Maciej W. Rozycki
2016-02-08 15:44 ` [PATCH v3 3/3] mips: Differentiate between 32 and 64 bit ELF header Daniel Wagner
2 siblings, 1 reply; 72+ messages in thread
From: Daniel Wagner @ 2016-02-08 15:44 UTC (permalink / raw)
To: Maciej W. Rozycki; +Cc: Ralf Baechle, linux-kernel, linux-mips, Daniel Wagner
parse_crash_elf{32|64}_headers will check the headers via the
elf_check_arch respectively vmcore_elf64_check_arch macro.
The MIPS architecture implements those two macros differently.
In order to make the differentiation more explicit, let's introduce
an vmcore_elf32_check_arch to allow the archs to overwrite it.
Signed-off-by: Daniel Wagner <daniel.wagner@bmw-carit.de>
Suggested-by: Maciej W. Rozycki <macro@imgtec.com>
---
fs/proc/vmcore.c | 2 +-
include/linux/crash_dump.h | 8 ++++++--
2 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/fs/proc/vmcore.c b/fs/proc/vmcore.c
index 4e61388..c8ed209 100644
--- a/fs/proc/vmcore.c
+++ b/fs/proc/vmcore.c
@@ -1068,7 +1068,7 @@ static int __init parse_crash_elf32_headers(void)
/* Do some basic Verification. */
if (memcmp(ehdr.e_ident, ELFMAG, SELFMAG) != 0 ||
(ehdr.e_type != ET_CORE) ||
- !elf_check_arch(&ehdr) ||
+ !vmcore_elf32_check_arch(&ehdr) ||
ehdr.e_ident[EI_CLASS] != ELFCLASS32||
ehdr.e_ident[EI_VERSION] != EV_CURRENT ||
ehdr.e_version != EV_CURRENT ||
diff --git a/include/linux/crash_dump.h b/include/linux/crash_dump.h
index 3849fce..3873697 100644
--- a/include/linux/crash_dump.h
+++ b/include/linux/crash_dump.h
@@ -34,9 +34,13 @@ void vmcore_cleanup(void);
/*
* Architecture code can redefine this if there are any special checks
- * needed for 64-bit ELF vmcores. In case of 32-bit only architecture,
- * this can be set to zero.
+ * needed for 32-bit ELF or 64-bit ELF vmcores. In case of 32-bit
+ * only architecture, vmcore_elf64_check_arch can be set to zero.
*/
+#ifndef vmcore_elf32_check_arch
+#define vmcore_elf32_check_arch(x) elf_check_arch(x)
+#endif
+
#ifndef vmcore_elf64_check_arch
#define vmcore_elf64_check_arch(x) (elf_check_arch(x) || vmcore_elf_check_arch_cross(x))
#endif
--
2.5.0
^ permalink raw reply related [flat|nested] 72+ messages in thread* Re: [PATCH v3 2/3] crash_dump: Add vmcore_elf32_check_arch
@ 2016-02-08 17:05 ` Maciej W. Rozycki
0 siblings, 0 replies; 72+ messages in thread
From: Maciej W. Rozycki @ 2016-02-08 17:05 UTC (permalink / raw)
To: Daniel Wagner; +Cc: Ralf Baechle, linux-kernel, linux-mips
On Mon, 8 Feb 2016, Daniel Wagner wrote:
> parse_crash_elf{32|64}_headers will check the headers via the
> elf_check_arch respectively vmcore_elf64_check_arch macro.
>
> The MIPS architecture implements those two macros differently.
> In order to make the differentiation more explicit, let's introduce
> an vmcore_elf32_check_arch to allow the archs to overwrite it.
>
> Signed-off-by: Daniel Wagner <daniel.wagner@bmw-carit.de>
> Suggested-by: Maciej W. Rozycki <macro@imgtec.com>
> ---
Reviewed-by: Maciej W. Rozycki <macro@imgtec.com>
Maciej
^ permalink raw reply [flat|nested] 72+ messages in thread* Re: [PATCH v3 2/3] crash_dump: Add vmcore_elf32_check_arch
@ 2016-02-08 17:05 ` Maciej W. Rozycki
0 siblings, 0 replies; 72+ messages in thread
From: Maciej W. Rozycki @ 2016-02-08 17:05 UTC (permalink / raw)
To: Daniel Wagner; +Cc: Ralf Baechle, linux-kernel, linux-mips
On Mon, 8 Feb 2016, Daniel Wagner wrote:
> parse_crash_elf{32|64}_headers will check the headers via the
> elf_check_arch respectively vmcore_elf64_check_arch macro.
>
> The MIPS architecture implements those two macros differently.
> In order to make the differentiation more explicit, let's introduce
> an vmcore_elf32_check_arch to allow the archs to overwrite it.
>
> Signed-off-by: Daniel Wagner <daniel.wagner@bmw-carit.de>
> Suggested-by: Maciej W. Rozycki <macro@imgtec.com>
> ---
Reviewed-by: Maciej W. Rozycki <macro@imgtec.com>
Maciej
^ permalink raw reply [flat|nested] 72+ messages in thread
* [PATCH v3 3/3] mips: Differentiate between 32 and 64 bit ELF header
2016-02-08 15:44 ` [PATCH v3 0/3] " Daniel Wagner
2016-02-08 15:44 ` [PATCH v3 1/3] mips: Use arch specific auxvec.h instead of generic-asm version Daniel Wagner
2016-02-08 15:44 ` [PATCH v3 2/3] crash_dump: Add vmcore_elf32_check_arch Daniel Wagner
@ 2016-02-08 15:44 ` Daniel Wagner
2016-02-08 16:22 ` kbuild test robot
2016-02-08 16:58 ` Maciej W. Rozycki
2 siblings, 2 replies; 72+ messages in thread
From: Daniel Wagner @ 2016-02-08 15:44 UTC (permalink / raw)
To: Maciej W. Rozycki; +Cc: Ralf Baechle, linux-kernel, linux-mips, Daniel Wagner
Depending on the configuration either the 32 or 64 bit version of
elf_check_arch() is defined. parse_crash_elf{32|64}_headers() does
some basic verification of the ELF header via
vmcore_elf{32|64}_check_arch() which happen to map to elf_check_arch().
Since the implementation 32 and 64 bit version of elf_check_arch()
differ, we use the wrong type:
In file included from include/linux/elf.h:4:0,
from fs/proc/vmcore.c:13:
fs/proc/vmcore.c: In function 'parse_crash_elf64_headers':
>> arch/mips/include/asm/elf.h:228:23: error: initialization from incompatible pointer type [-Werror=incompatible-pointer-types]
struct elfhdr *__h = (hdr); \
^
include/linux/crash_dump.h:41:37: note: in expansion of macro 'elf_check_arch'
#define vmcore_elf64_check_arch(x) (elf_check_arch(x) || vmcore_elf_check_arch_cross(x))
^
fs/proc/vmcore.c:1015:4: note: in expansion of macro 'vmcore_elf64_check_arch'
!vmcore_elf64_check_arch(&ehdr) ||
^
Therefore, we rather define vmcore_elf{32|64}_check_arch() as a
basic machine check and use it also in binfm_elf?32.c as well.
Signed-off-by: Daniel Wagner <daniel.wagner@bmw-carit.de>
Suggested-by: Maciej W. Rozycki <macro@imgtec.com>
Reported-by: Fengguang Wu <fengguang.wu@intel.com>
---
arch/mips/include/asm/elf.h | 9 +++++++--
arch/mips/kernel/binfmt_elfn32.c | 2 +-
arch/mips/kernel/binfmt_elfo32.c | 2 +-
3 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/arch/mips/include/asm/elf.h b/arch/mips/include/asm/elf.h
index cefb7a5..189e279 100644
--- a/arch/mips/include/asm/elf.h
+++ b/arch/mips/include/asm/elf.h
@@ -205,6 +205,11 @@ struct mips_elf_abiflags_v0 {
#define MIPS_ABI_FP_64 6 /* -mips32r2 -mfp64 */
#define MIPS_ABI_FP_64A 7 /* -mips32r2 -mfp64 -mno-odd-spreg */
+#define mips_elf_check_machine(x) ((x)->e_machine == EM_MIPS)
+
+#define vmcore_elf32_check_arch mips_elf_check_machine
+#define vmcore_elf64_check_arch mips_elf_check_machine
+
#ifdef CONFIG_32BIT
/*
@@ -227,7 +232,7 @@ struct mips_elf_abiflags_v0 {
int __res = 1; \
struct elfhdr *__h = (hdr); \
\
- if (__h->e_machine != EM_MIPS) \
+ if (!mips_elf_check_machine(__h)) \
__res = 0; \
if (__h->e_ident[EI_CLASS] != ELFCLASS32) \
__res = 0; \
@@ -258,7 +263,7 @@ struct mips_elf_abiflags_v0 {
int __res = 1; \
struct elfhdr *__h = (hdr); \
\
- if (__h->e_machine != EM_MIPS) \
+ if (!mips_elf_check_machine(__h)) \
__res = 0; \
if (__h->e_ident[EI_CLASS] != ELFCLASS64) \
__res = 0; \
diff --git a/arch/mips/kernel/binfmt_elfn32.c b/arch/mips/kernel/binfmt_elfn32.c
index 1188e00..1b992c6 100644
--- a/arch/mips/kernel/binfmt_elfn32.c
+++ b/arch/mips/kernel/binfmt_elfn32.c
@@ -35,7 +35,7 @@ typedef elf_fpreg_t elf_fpregset_t[ELF_NFPREG];
int __res = 1; \
struct elfhdr *__h = (hdr); \
\
- if (__h->e_machine != EM_MIPS) \
+ if (!mips_elf_check_machine(__h)) \
__res = 0; \
if (__h->e_ident[EI_CLASS] != ELFCLASS32) \
__res = 0; \
diff --git a/arch/mips/kernel/binfmt_elfo32.c b/arch/mips/kernel/binfmt_elfo32.c
index 9287678..abd3aff 100644
--- a/arch/mips/kernel/binfmt_elfo32.c
+++ b/arch/mips/kernel/binfmt_elfo32.c
@@ -47,7 +47,7 @@ typedef elf_fpreg_t elf_fpregset_t[ELF_NFPREG];
int __res = 1; \
struct elfhdr *__h = (hdr); \
\
- if (__h->e_machine != EM_MIPS) \
+ if (!mips_elf_check_machine(__h)) \
__res = 0; \
if (__h->e_ident[EI_CLASS] != ELFCLASS32) \
__res = 0; \
--
2.5.0
^ permalink raw reply related [flat|nested] 72+ messages in thread* Re: [PATCH v3 3/3] mips: Differentiate between 32 and 64 bit ELF header
@ 2016-02-08 16:22 ` kbuild test robot
0 siblings, 0 replies; 72+ messages in thread
From: kbuild test robot @ 2016-02-08 16:22 UTC (permalink / raw)
To: Daniel Wagner
Cc: kbuild-all, Maciej W. Rozycki, Ralf Baechle, linux-kernel,
linux-mips, Daniel Wagner
[-- Attachment #1: Type: text/plain, Size: 2172 bytes --]
Hi Daniel,
[auto build test ERROR on v4.5-rc3]
[also build test ERROR on next-20160208]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]
url: https://github.com/0day-ci/linux/commits/Daniel-Wagner/Differentiate-between-32-and-64-bit-ELF-header/20160208-234759
config: mips-fuloong2e_defconfig (attached as .config)
reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=mips
All error/warnings (new ones prefixed by >>):
arch/mips/kernel/../../../fs/binfmt_elf.c: In function 'load_elf_interp':
>> arch/mips/kernel/binfmt_elfn32.c:38:7: error: implicit declaration of function 'mips_elf_check_machine' [-Werror=implicit-function-declaration]
if (!mips_elf_check_machine(__h)) \
^
>> arch/mips/kernel/../../../fs/binfmt_elf.c:536:7: note: in expansion of macro 'elf_check_arch'
if (!elf_check_arch(interp_elf_ex))
^
cc1: some warnings being treated as errors
--
arch/mips/kernel/../../../fs/binfmt_elf.c: In function 'load_elf_interp':
>> arch/mips/kernel/binfmt_elfo32.c:50:7: error: implicit declaration of function 'mips_elf_check_machine' [-Werror=implicit-function-declaration]
if (!mips_elf_check_machine(__h)) \
^
>> arch/mips/kernel/../../../fs/binfmt_elf.c:536:7: note: in expansion of macro 'elf_check_arch'
if (!elf_check_arch(interp_elf_ex))
^
cc1: some warnings being treated as errors
vim +/mips_elf_check_machine +38 arch/mips/kernel/binfmt_elfn32.c
32 */
33 #define elf_check_arch(hdr) \
34 ({ \
35 int __res = 1; \
36 struct elfhdr *__h = (hdr); \
37 \
> 38 if (!mips_elf_check_machine(__h)) \
39 __res = 0; \
40 if (__h->e_ident[EI_CLASS] != ELFCLASS32) \
41 __res = 0; \
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 16752 bytes --]
^ permalink raw reply [flat|nested] 72+ messages in thread* Re: [PATCH v3 3/3] mips: Differentiate between 32 and 64 bit ELF header
@ 2016-02-08 16:22 ` kbuild test robot
0 siblings, 0 replies; 72+ messages in thread
From: kbuild test robot @ 2016-02-08 16:22 UTC (permalink / raw)
To: Daniel Wagner
Cc: kbuild-all, Maciej W. Rozycki, Ralf Baechle, linux-kernel, linux-mips
[-- Attachment #1: Type: text/plain, Size: 2172 bytes --]
Hi Daniel,
[auto build test ERROR on v4.5-rc3]
[also build test ERROR on next-20160208]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]
url: https://github.com/0day-ci/linux/commits/Daniel-Wagner/Differentiate-between-32-and-64-bit-ELF-header/20160208-234759
config: mips-fuloong2e_defconfig (attached as .config)
reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=mips
All error/warnings (new ones prefixed by >>):
arch/mips/kernel/../../../fs/binfmt_elf.c: In function 'load_elf_interp':
>> arch/mips/kernel/binfmt_elfn32.c:38:7: error: implicit declaration of function 'mips_elf_check_machine' [-Werror=implicit-function-declaration]
if (!mips_elf_check_machine(__h)) \
^
>> arch/mips/kernel/../../../fs/binfmt_elf.c:536:7: note: in expansion of macro 'elf_check_arch'
if (!elf_check_arch(interp_elf_ex))
^
cc1: some warnings being treated as errors
--
arch/mips/kernel/../../../fs/binfmt_elf.c: In function 'load_elf_interp':
>> arch/mips/kernel/binfmt_elfo32.c:50:7: error: implicit declaration of function 'mips_elf_check_machine' [-Werror=implicit-function-declaration]
if (!mips_elf_check_machine(__h)) \
^
>> arch/mips/kernel/../../../fs/binfmt_elf.c:536:7: note: in expansion of macro 'elf_check_arch'
if (!elf_check_arch(interp_elf_ex))
^
cc1: some warnings being treated as errors
vim +/mips_elf_check_machine +38 arch/mips/kernel/binfmt_elfn32.c
32 */
33 #define elf_check_arch(hdr) \
34 ({ \
35 int __res = 1; \
36 struct elfhdr *__h = (hdr); \
37 \
> 38 if (!mips_elf_check_machine(__h)) \
39 __res = 0; \
40 if (__h->e_ident[EI_CLASS] != ELFCLASS32) \
41 __res = 0; \
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 16752 bytes --]
^ permalink raw reply [flat|nested] 72+ messages in thread* Re: [PATCH v3 3/3] mips: Differentiate between 32 and 64 bit ELF header
@ 2016-02-09 8:03 ` Daniel Wagner
0 siblings, 0 replies; 72+ messages in thread
From: Daniel Wagner @ 2016-02-09 8:03 UTC (permalink / raw)
To: Maciej W. Rozycki
Cc: kbuild test robot, kbuild-all, Ralf Baechle, linux-kernel, linux-mips
Hi Maciej,
On 02/08/2016 05:22 PM, kbuild test robot wrote:
> Hi Daniel,
>
> [auto build test ERROR on v4.5-rc3]
> [also build test ERROR on next-20160208]
> [if your patch is applied to the wrong git tree, please drop us a note to help improving the system]
>
> url: https://github.com/0day-ci/linux/commits/Daniel-Wagner/Differentiate-between-32-and-64-bit-ELF-header/20160208-234759
> config: mips-fuloong2e_defconfig (attached as .config)
> reproduce:
> wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # save the attached .config to linux build tree
> make.cross ARCH=mips
>
> All error/warnings (new ones prefixed by >>):
>
> arch/mips/kernel/../../../fs/binfmt_elf.c: In function 'load_elf_interp':
>>> arch/mips/kernel/binfmt_elfn32.c:38:7: error: implicit declaration of function 'mips_elf_check_machine' [-Werror=implicit-function-declaration]
> if (!mips_elf_check_machine(__h)) \
> ^
>>> arch/mips/kernel/../../../fs/binfmt_elf.c:536:7: note: in expansion of macro 'elf_check_arch'
> if (!elf_check_arch(interp_elf_ex))
> ^
> cc1: some warnings being treated as errors
> --
> arch/mips/kernel/../../../fs/binfmt_elf.c: In function 'load_elf_interp':
Hmm how I was able to build binfmt_elfo32.o because it should suffer
from the same problem.
I think reusing mips_elf_check_machine() in binfmt_elf?32.c is only
going to work if we include arch/mips/include/asm/elf.h. Though this
looks kind of wrong.
Should I add a mips_elf_check_machine() to binfmt_elf?32.c as well or
just leave them as they are at this point?
cheers,
daniel
^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH v3 3/3] mips: Differentiate between 32 and 64 bit ELF header
@ 2016-02-09 8:03 ` Daniel Wagner
0 siblings, 0 replies; 72+ messages in thread
From: Daniel Wagner @ 2016-02-09 8:03 UTC (permalink / raw)
To: Maciej W. Rozycki
Cc: kbuild test robot, kbuild-all, Ralf Baechle, linux-kernel, linux-mips
Hi Maciej,
On 02/08/2016 05:22 PM, kbuild test robot wrote:
> Hi Daniel,
>
> [auto build test ERROR on v4.5-rc3]
> [also build test ERROR on next-20160208]
> [if your patch is applied to the wrong git tree, please drop us a note to help improving the system]
>
> url: https://github.com/0day-ci/linux/commits/Daniel-Wagner/Differentiate-between-32-and-64-bit-ELF-header/20160208-234759
> config: mips-fuloong2e_defconfig (attached as .config)
> reproduce:
> wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # save the attached .config to linux build tree
> make.cross ARCH=mips
>
> All error/warnings (new ones prefixed by >>):
>
> arch/mips/kernel/../../../fs/binfmt_elf.c: In function 'load_elf_interp':
>>> arch/mips/kernel/binfmt_elfn32.c:38:7: error: implicit declaration of function 'mips_elf_check_machine' [-Werror=implicit-function-declaration]
> if (!mips_elf_check_machine(__h)) \
> ^
>>> arch/mips/kernel/../../../fs/binfmt_elf.c:536:7: note: in expansion of macro 'elf_check_arch'
> if (!elf_check_arch(interp_elf_ex))
> ^
> cc1: some warnings being treated as errors
> --
> arch/mips/kernel/../../../fs/binfmt_elf.c: In function 'load_elf_interp':
Hmm how I was able to build binfmt_elfo32.o because it should suffer
from the same problem.
I think reusing mips_elf_check_machine() in binfmt_elf?32.c is only
going to work if we include arch/mips/include/asm/elf.h. Though this
looks kind of wrong.
Should I add a mips_elf_check_machine() to binfmt_elf?32.c as well or
just leave them as they are at this point?
cheers,
daniel
^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH v3 3/3] mips: Differentiate between 32 and 64 bit ELF header
@ 2016-02-09 12:32 ` Maciej W. Rozycki
0 siblings, 0 replies; 72+ messages in thread
From: Maciej W. Rozycki @ 2016-02-09 12:32 UTC (permalink / raw)
To: Daniel Wagner
Cc: kbuild test robot, kbuild-all, Ralf Baechle, linux-kernel, linux-mips
On Tue, 9 Feb 2016, Daniel Wagner wrote:
> > All error/warnings (new ones prefixed by >>):
> >
> > arch/mips/kernel/../../../fs/binfmt_elf.c: In function 'load_elf_interp':
> >>> arch/mips/kernel/binfmt_elfn32.c:38:7: error: implicit declaration of function 'mips_elf_check_machine' [-Werror=implicit-function-declaration]
> > if (!mips_elf_check_machine(__h)) \
> > ^
> >>> arch/mips/kernel/../../../fs/binfmt_elf.c:536:7: note: in expansion of macro 'elf_check_arch'
> > if (!elf_check_arch(interp_elf_ex))
> > ^
> > cc1: some warnings being treated as errors
> > --
> > arch/mips/kernel/../../../fs/binfmt_elf.c: In function 'load_elf_interp':
>
> Hmm how I was able to build binfmt_elfo32.o because it should suffer
> from the same problem.
>
> I think reusing mips_elf_check_machine() in binfmt_elf?32.c is only
> going to work if we include arch/mips/include/asm/elf.h. Though this
> looks kind of wrong.
But neither binfmt_elf?32.c actually expands `elf_check_arch' and both
include fs/binfmt_elf.c at the end, which in turn includes <linux/elf.h>,
which in turn does include <asm/elf.h> before expanding `elf_check_arch',
and consequently at that point `mips_elf_check_machine' will have been
already defined. So things are all right, except you need to define the
macro outside `#ifndef ELF_ARCH'.
I suggest moving it down, right below the conditional, rather than up as
the top of the file contains generic MIPS ELF stuff. I think all the
three macros can go together, it doesn't appear to me they need to depend
on `ELF_ARCH', and we can fix it up if ever in the future they have to.
FWIW I think all the MIPS ABI flags stuff also needs to go outside the
conditional, because it's ABI agnostic. I'll make the right change myself
on top of your fixes. It'll remove a little bit of code duplication,
which is always welcome.
Maciej
^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH v3 3/3] mips: Differentiate between 32 and 64 bit ELF header
@ 2016-02-09 12:32 ` Maciej W. Rozycki
0 siblings, 0 replies; 72+ messages in thread
From: Maciej W. Rozycki @ 2016-02-09 12:32 UTC (permalink / raw)
To: Daniel Wagner
Cc: kbuild test robot, kbuild-all, Ralf Baechle, linux-kernel, linux-mips
On Tue, 9 Feb 2016, Daniel Wagner wrote:
> > All error/warnings (new ones prefixed by >>):
> >
> > arch/mips/kernel/../../../fs/binfmt_elf.c: In function 'load_elf_interp':
> >>> arch/mips/kernel/binfmt_elfn32.c:38:7: error: implicit declaration of function 'mips_elf_check_machine' [-Werror=implicit-function-declaration]
> > if (!mips_elf_check_machine(__h)) \
> > ^
> >>> arch/mips/kernel/../../../fs/binfmt_elf.c:536:7: note: in expansion of macro 'elf_check_arch'
> > if (!elf_check_arch(interp_elf_ex))
> > ^
> > cc1: some warnings being treated as errors
> > --
> > arch/mips/kernel/../../../fs/binfmt_elf.c: In function 'load_elf_interp':
>
> Hmm how I was able to build binfmt_elfo32.o because it should suffer
> from the same problem.
>
> I think reusing mips_elf_check_machine() in binfmt_elf?32.c is only
> going to work if we include arch/mips/include/asm/elf.h. Though this
> looks kind of wrong.
But neither binfmt_elf?32.c actually expands `elf_check_arch' and both
include fs/binfmt_elf.c at the end, which in turn includes <linux/elf.h>,
which in turn does include <asm/elf.h> before expanding `elf_check_arch',
and consequently at that point `mips_elf_check_machine' will have been
already defined. So things are all right, except you need to define the
macro outside `#ifndef ELF_ARCH'.
I suggest moving it down, right below the conditional, rather than up as
the top of the file contains generic MIPS ELF stuff. I think all the
three macros can go together, it doesn't appear to me they need to depend
on `ELF_ARCH', and we can fix it up if ever in the future they have to.
FWIW I think all the MIPS ABI flags stuff also needs to go outside the
conditional, because it's ABI agnostic. I'll make the right change myself
on top of your fixes. It'll remove a little bit of code duplication,
which is always welcome.
Maciej
^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH v3 3/3] mips: Differentiate between 32 and 64 bit ELF header
@ 2016-02-09 12:38 ` Daniel Wagner
0 siblings, 0 replies; 72+ messages in thread
From: Daniel Wagner @ 2016-02-09 12:38 UTC (permalink / raw)
To: Maciej W. Rozycki
Cc: kbuild test robot, kbuild-all, Ralf Baechle, linux-kernel, linux-mips
On 02/09/2016 01:32 PM, Maciej W. Rozycki wrote:
> FWIW I think all the MIPS ABI flags stuff also needs to go outside the
> conditional, because it's ABI agnostic. I'll make the right change myself
> on top of your fixes. It'll remove a little bit of code duplication,
> which is always welcome.
Great, thanks for taking care of it.
cheers,
daniel
^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH v3 3/3] mips: Differentiate between 32 and 64 bit ELF header
@ 2016-02-09 12:38 ` Daniel Wagner
0 siblings, 0 replies; 72+ messages in thread
From: Daniel Wagner @ 2016-02-09 12:38 UTC (permalink / raw)
To: Maciej W. Rozycki
Cc: kbuild test robot, kbuild-all, Ralf Baechle, linux-kernel, linux-mips
On 02/09/2016 01:32 PM, Maciej W. Rozycki wrote:
> FWIW I think all the MIPS ABI flags stuff also needs to go outside the
> conditional, because it's ABI agnostic. I'll make the right change myself
> on top of your fixes. It'll remove a little bit of code duplication,
> which is always welcome.
Great, thanks for taking care of it.
cheers,
daniel
^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH v3 3/3] mips: Differentiate between 32 and 64 bit ELF header
@ 2016-02-09 19:44 ` Maciej W. Rozycki
0 siblings, 0 replies; 72+ messages in thread
From: Maciej W. Rozycki @ 2016-02-09 19:44 UTC (permalink / raw)
To: Daniel Wagner
Cc: kbuild test robot, kbuild-all, Ralf Baechle, linux-kernel, linux-mips
On Tue, 9 Feb 2016, Daniel Wagner wrote:
> > FWIW I think all the MIPS ABI flags stuff also needs to go outside the
> > conditional, because it's ABI agnostic. I'll make the right change myself
> > on top of your fixes. It'll remove a little bit of code duplication,
> > which is always welcome.
>
> Great, thanks for taking care of it.
My ABI flags change has passed testing and I'm ready to post it, will you
be respinning your patch soon?
Maciej
^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH v3 3/3] mips: Differentiate between 32 and 64 bit ELF header
@ 2016-02-09 19:44 ` Maciej W. Rozycki
0 siblings, 0 replies; 72+ messages in thread
From: Maciej W. Rozycki @ 2016-02-09 19:44 UTC (permalink / raw)
To: Daniel Wagner
Cc: kbuild test robot, kbuild-all, Ralf Baechle, linux-kernel, linux-mips
On Tue, 9 Feb 2016, Daniel Wagner wrote:
> > FWIW I think all the MIPS ABI flags stuff also needs to go outside the
> > conditional, because it's ABI agnostic. I'll make the right change myself
> > on top of your fixes. It'll remove a little bit of code duplication,
> > which is always welcome.
>
> Great, thanks for taking care of it.
My ABI flags change has passed testing and I'm ready to post it, will you
be respinning your patch soon?
Maciej
^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH v3 3/3] mips: Differentiate between 32 and 64 bit ELF header
@ 2016-02-10 6:28 ` Daniel Wagner
0 siblings, 0 replies; 72+ messages in thread
From: Daniel Wagner @ 2016-02-10 6:28 UTC (permalink / raw)
To: Maciej W. Rozycki
Cc: kbuild test robot, kbuild-all, Ralf Baechle, linux-kernel, linux-mips
On 02/09/2016 08:44 PM, Maciej W. Rozycki wrote:
> On Tue, 9 Feb 2016, Daniel Wagner wrote:
>
>>> FWIW I think all the MIPS ABI flags stuff also needs to go outside the
>>> conditional, because it's ABI agnostic. I'll make the right change myself
>>> on top of your fixes. It'll remove a little bit of code duplication,
>>> which is always welcome.
>>
>> Great, thanks for taking care of it.
>
> My ABI flags change has passed testing and I'm ready to post it, will you
> be respinning your patch soon?
I was waiting for your cleanups and base my patches on top of it. Looks
like a small misunderstanding on my side :) I'll start on v4 right now.
cheers,
daniel
^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH v3 3/3] mips: Differentiate between 32 and 64 bit ELF header
@ 2016-02-10 6:28 ` Daniel Wagner
0 siblings, 0 replies; 72+ messages in thread
From: Daniel Wagner @ 2016-02-10 6:28 UTC (permalink / raw)
To: Maciej W. Rozycki
Cc: kbuild test robot, kbuild-all, Ralf Baechle, linux-kernel, linux-mips
On 02/09/2016 08:44 PM, Maciej W. Rozycki wrote:
> On Tue, 9 Feb 2016, Daniel Wagner wrote:
>
>>> FWIW I think all the MIPS ABI flags stuff also needs to go outside the
>>> conditional, because it's ABI agnostic. I'll make the right change myself
>>> on top of your fixes. It'll remove a little bit of code duplication,
>>> which is always welcome.
>>
>> Great, thanks for taking care of it.
>
> My ABI flags change has passed testing and I'm ready to post it, will you
> be respinning your patch soon?
I was waiting for your cleanups and base my patches on top of it. Looks
like a small misunderstanding on my side :) I'll start on v4 right now.
cheers,
daniel
^ permalink raw reply [flat|nested] 72+ messages in thread
* [PATCH v4 0/2] Differentiate between 32 and 64 bit ELF header
2016-02-10 6:28 ` Daniel Wagner
(?)
@ 2016-02-10 9:21 ` Daniel Wagner
2016-02-10 9:21 ` [PATCH v4 1/2] crash_dump: Add vmcore_elf32_check_arch Daniel Wagner
2016-02-10 9:21 ` [PATCH v4 2/2] mips: Differentiate between 32 and 64 bit ELF header Daniel Wagner
-1 siblings, 2 replies; 72+ messages in thread
From: Daniel Wagner @ 2016-02-10 9:21 UTC (permalink / raw)
To: Maciej W. Rozycki; +Cc: Ralf Baechle, linux-kernel, linux-mips, Daniel Wagner
Hi Maciej,
I did test compile a few different configurations and with and without
mrproper upfront. All looks fine now. Let's see what still goes wrong :)
cheers,
daniel
Daniel Wagner (2):
crash_dump: Add vmcore_elf32_check_arch
mips: Differentiate between 32 and 64 bit ELF header
arch/mips/include/asm/elf.h | 9 +++++++--
arch/mips/kernel/binfmt_elfn32.c | 2 +-
arch/mips/kernel/binfmt_elfo32.c | 2 +-
fs/proc/vmcore.c | 2 +-
include/linux/crash_dump.h | 8 ++++++--
5 files changed, 16 insertions(+), 7 deletions(-)
--
2.5.0
^ permalink raw reply [flat|nested] 72+ messages in thread
* [PATCH v4 1/2] crash_dump: Add vmcore_elf32_check_arch
2016-02-10 9:21 ` [PATCH v4 0/2] " Daniel Wagner
@ 2016-02-10 9:21 ` Daniel Wagner
2016-02-10 9:21 ` [PATCH v4 2/2] mips: Differentiate between 32 and 64 bit ELF header Daniel Wagner
1 sibling, 0 replies; 72+ messages in thread
From: Daniel Wagner @ 2016-02-10 9:21 UTC (permalink / raw)
To: Maciej W. Rozycki; +Cc: Ralf Baechle, linux-kernel, linux-mips, Daniel Wagner
parse_crash_elf{32|64}_headers will check the headers via the
elf_check_arch respectively vmcore_elf64_check_arch macro.
The MIPS architecture implements those two macros differently.
In order to make the differentiation more explicit, let's introduce
an vmcore_elf32_check_arch to allow the archs to overwrite it.
Signed-off-by: Daniel Wagner <daniel.wagner@bmw-carit.de>
Suggested-by: Maciej W. Rozycki <macro@imgtec.com>
Reviewed-by: Maciej W. Rozycki <macro@imgtec.com>
---
fs/proc/vmcore.c | 2 +-
include/linux/crash_dump.h | 8 ++++++--
2 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/fs/proc/vmcore.c b/fs/proc/vmcore.c
index 4e61388..c8ed209 100644
--- a/fs/proc/vmcore.c
+++ b/fs/proc/vmcore.c
@@ -1068,7 +1068,7 @@ static int __init parse_crash_elf32_headers(void)
/* Do some basic Verification. */
if (memcmp(ehdr.e_ident, ELFMAG, SELFMAG) != 0 ||
(ehdr.e_type != ET_CORE) ||
- !elf_check_arch(&ehdr) ||
+ !vmcore_elf32_check_arch(&ehdr) ||
ehdr.e_ident[EI_CLASS] != ELFCLASS32||
ehdr.e_ident[EI_VERSION] != EV_CURRENT ||
ehdr.e_version != EV_CURRENT ||
diff --git a/include/linux/crash_dump.h b/include/linux/crash_dump.h
index 3849fce..3873697 100644
--- a/include/linux/crash_dump.h
+++ b/include/linux/crash_dump.h
@@ -34,9 +34,13 @@ void vmcore_cleanup(void);
/*
* Architecture code can redefine this if there are any special checks
- * needed for 64-bit ELF vmcores. In case of 32-bit only architecture,
- * this can be set to zero.
+ * needed for 32-bit ELF or 64-bit ELF vmcores. In case of 32-bit
+ * only architecture, vmcore_elf64_check_arch can be set to zero.
*/
+#ifndef vmcore_elf32_check_arch
+#define vmcore_elf32_check_arch(x) elf_check_arch(x)
+#endif
+
#ifndef vmcore_elf64_check_arch
#define vmcore_elf64_check_arch(x) (elf_check_arch(x) || vmcore_elf_check_arch_cross(x))
#endif
--
2.5.0
^ permalink raw reply related [flat|nested] 72+ messages in thread* [PATCH v4 2/2] mips: Differentiate between 32 and 64 bit ELF header
2016-02-10 9:21 ` [PATCH v4 0/2] " Daniel Wagner
2016-02-10 9:21 ` [PATCH v4 1/2] crash_dump: Add vmcore_elf32_check_arch Daniel Wagner
@ 2016-02-10 9:21 ` Daniel Wagner
2016-02-11 10:49 ` Ralf Baechle
1 sibling, 1 reply; 72+ messages in thread
From: Daniel Wagner @ 2016-02-10 9:21 UTC (permalink / raw)
To: Maciej W. Rozycki; +Cc: Ralf Baechle, linux-kernel, linux-mips, Daniel Wagner
Depending on the configuration either the 32 or 64 bit version of
elf_check_arch() is defined. parse_crash_elf{32|64}_headers() does
some basic verification of the ELF header via
vmcore_elf{32|64}_check_arch() which happen to map to elf_check_arch().
Since the implementation 32 and 64 bit version of elf_check_arch()
differ, we use the wrong type:
In file included from include/linux/elf.h:4:0,
from fs/proc/vmcore.c:13:
fs/proc/vmcore.c: In function 'parse_crash_elf64_headers':
>> arch/mips/include/asm/elf.h:228:23: error: initialization from incompatible pointer type [-Werror=incompatible-pointer-types]
struct elfhdr *__h = (hdr); \
^
include/linux/crash_dump.h:41:37: note: in expansion of macro 'elf_check_arch'
#define vmcore_elf64_check_arch(x) (elf_check_arch(x) || vmcore_elf_check_arch_cross(x))
^
fs/proc/vmcore.c:1015:4: note: in expansion of macro 'vmcore_elf64_check_arch'
!vmcore_elf64_check_arch(&ehdr) ||
^
Therefore, we rather define vmcore_elf{32|64}_check_arch() as a
basic machine check and use it also in binfm_elf?32.c as well.
Signed-off-by: Daniel Wagner <daniel.wagner@bmw-carit.de>
Suggested-by: Maciej W. Rozycki <macro@imgtec.com>
Reviewed-by: Maciej W. Rozycki <macro@imgtec.com>
Reported-by: Fengguang Wu <fengguang.wu@intel.com>
---
arch/mips/include/asm/elf.h | 9 +++++++--
arch/mips/kernel/binfmt_elfn32.c | 2 +-
arch/mips/kernel/binfmt_elfo32.c | 2 +-
3 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/arch/mips/include/asm/elf.h b/arch/mips/include/asm/elf.h
index cefb7a5..e090fc3 100644
--- a/arch/mips/include/asm/elf.h
+++ b/arch/mips/include/asm/elf.h
@@ -227,7 +227,7 @@ struct mips_elf_abiflags_v0 {
int __res = 1; \
struct elfhdr *__h = (hdr); \
\
- if (__h->e_machine != EM_MIPS) \
+ if (!mips_elf_check_machine(__h)) \
__res = 0; \
if (__h->e_ident[EI_CLASS] != ELFCLASS32) \
__res = 0; \
@@ -258,7 +258,7 @@ struct mips_elf_abiflags_v0 {
int __res = 1; \
struct elfhdr *__h = (hdr); \
\
- if (__h->e_machine != EM_MIPS) \
+ if (!mips_elf_check_machine(__h)) \
__res = 0; \
if (__h->e_ident[EI_CLASS] != ELFCLASS64) \
__res = 0; \
@@ -285,6 +285,11 @@ struct mips_elf_abiflags_v0 {
#endif /* !defined(ELF_ARCH) */
+#define mips_elf_check_machine(x) ((x)->e_machine == EM_MIPS)
+
+#define vmcore_elf32_check_arch mips_elf_check_machine
+#define vmcore_elf64_check_arch mips_elf_check_machine
+
struct mips_abi;
extern struct mips_abi mips_abi;
diff --git a/arch/mips/kernel/binfmt_elfn32.c b/arch/mips/kernel/binfmt_elfn32.c
index 1188e00..1b992c6 100644
--- a/arch/mips/kernel/binfmt_elfn32.c
+++ b/arch/mips/kernel/binfmt_elfn32.c
@@ -35,7 +35,7 @@ typedef elf_fpreg_t elf_fpregset_t[ELF_NFPREG];
int __res = 1; \
struct elfhdr *__h = (hdr); \
\
- if (__h->e_machine != EM_MIPS) \
+ if (!mips_elf_check_machine(__h)) \
__res = 0; \
if (__h->e_ident[EI_CLASS] != ELFCLASS32) \
__res = 0; \
diff --git a/arch/mips/kernel/binfmt_elfo32.c b/arch/mips/kernel/binfmt_elfo32.c
index 9287678..abd3aff 100644
--- a/arch/mips/kernel/binfmt_elfo32.c
+++ b/arch/mips/kernel/binfmt_elfo32.c
@@ -47,7 +47,7 @@ typedef elf_fpreg_t elf_fpregset_t[ELF_NFPREG];
int __res = 1; \
struct elfhdr *__h = (hdr); \
\
- if (__h->e_machine != EM_MIPS) \
+ if (!mips_elf_check_machine(__h)) \
__res = 0; \
if (__h->e_ident[EI_CLASS] != ELFCLASS32) \
__res = 0; \
--
2.5.0
^ permalink raw reply related [flat|nested] 72+ messages in thread* Re: [PATCH v4 2/2] mips: Differentiate between 32 and 64 bit ELF header
2016-02-10 9:21 ` [PATCH v4 2/2] mips: Differentiate between 32 and 64 bit ELF header Daniel Wagner
@ 2016-02-11 10:49 ` Ralf Baechle
2016-02-11 12:04 ` Maciej W. Rozycki
0 siblings, 1 reply; 72+ messages in thread
From: Ralf Baechle @ 2016-02-11 10:49 UTC (permalink / raw)
To: Daniel Wagner; +Cc: Maciej W. Rozycki, linux-kernel, linux-mips
On Wed, Feb 10, 2016 at 10:21:21AM +0100, Daniel Wagner wrote:
> Depending on the configuration either the 32 or 64 bit version of
> elf_check_arch() is defined. parse_crash_elf{32|64}_headers() does
> some basic verification of the ELF header via
> vmcore_elf{32|64}_check_arch() which happen to map to elf_check_arch().
> Since the implementation 32 and 64 bit version of elf_check_arch()
> differ, we use the wrong type:
>
> In file included from include/linux/elf.h:4:0,
> from fs/proc/vmcore.c:13:
> fs/proc/vmcore.c: In function 'parse_crash_elf64_headers':
> >> arch/mips/include/asm/elf.h:228:23: error: initialization from incompatible pointer type [-Werror=incompatible-pointer-types]
> struct elfhdr *__h = (hdr); \
> ^
> include/linux/crash_dump.h:41:37: note: in expansion of macro 'elf_check_arch'
> #define vmcore_elf64_check_arch(x) (elf_check_arch(x) || vmcore_elf_check_arch_cross(x))
> ^
> fs/proc/vmcore.c:1015:4: note: in expansion of macro 'vmcore_elf64_check_arch'
> !vmcore_elf64_check_arch(&ehdr) ||
> ^
>
> Therefore, we rather define vmcore_elf{32|64}_check_arch() as a
> basic machine check and use it also in binfm_elf?32.c as well.
>
> Signed-off-by: Daniel Wagner <daniel.wagner@bmw-carit.de>
> Suggested-by: Maciej W. Rozycki <macro@imgtec.com>
> Reviewed-by: Maciej W. Rozycki <macro@imgtec.com>
> Reported-by: Fengguang Wu <fengguang.wu@intel.com>
Thanks, applied.
I'm getting a less spectacular warning from gcc 5.2:
CC fs/proc/vmcore.o
fs/proc/vmcore.c: In function ‘parse_crash_elf64_headers’:
fs/proc/vmcore.c:939:47: warning: initialization from incompatible pointer type [-Wincompatible-pointer-types]
Ralf
^ permalink raw reply [flat|nested] 72+ messages in thread* Re: [PATCH v4 2/2] mips: Differentiate between 32 and 64 bit ELF header
@ 2016-02-11 12:04 ` Maciej W. Rozycki
0 siblings, 0 replies; 72+ messages in thread
From: Maciej W. Rozycki @ 2016-02-11 12:04 UTC (permalink / raw)
To: Ralf Baechle; +Cc: Daniel Wagner, linux-kernel, linux-mips
On Thu, 11 Feb 2016, Ralf Baechle wrote:
> > Signed-off-by: Daniel Wagner <daniel.wagner@bmw-carit.de>
> > Suggested-by: Maciej W. Rozycki <macro@imgtec.com>
> > Reviewed-by: Maciej W. Rozycki <macro@imgtec.com>
> > Reported-by: Fengguang Wu <fengguang.wu@intel.com>
>
> Thanks, applied.
>
> I'm getting a less spectacular warning from gcc 5.2:
>
> CC fs/proc/vmcore.o
> fs/proc/vmcore.c: In function ‘parse_crash_elf64_headers’:
> fs/proc/vmcore.c:939:47: warning: initialization from incompatible pointer type [-Wincompatible-pointer-types]
Yes, the temporaries still need to have their pointed types changed, to
`Elf32_Ehdr' and `Elf64_Ehdr' respectively, as in the original change.
I had it mentioned in a WIP version of my review (stating that it would
verify that the correct type is used by the caller), but then deleted that
part inadvertently, sigh.
Daniel, sorry for the extra iteration.
Maciej
^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH v4 2/2] mips: Differentiate between 32 and 64 bit ELF header
@ 2016-02-11 12:04 ` Maciej W. Rozycki
0 siblings, 0 replies; 72+ messages in thread
From: Maciej W. Rozycki @ 2016-02-11 12:04 UTC (permalink / raw)
To: Ralf Baechle; +Cc: Daniel Wagner, linux-kernel, linux-mips
On Thu, 11 Feb 2016, Ralf Baechle wrote:
> > Signed-off-by: Daniel Wagner <daniel.wagner@bmw-carit.de>
> > Suggested-by: Maciej W. Rozycki <macro@imgtec.com>
> > Reviewed-by: Maciej W. Rozycki <macro@imgtec.com>
> > Reported-by: Fengguang Wu <fengguang.wu@intel.com>
>
> Thanks, applied.
>
> I'm getting a less spectacular warning from gcc 5.2:
>
> CC fs/proc/vmcore.o
> fs/proc/vmcore.c: In function ‘parse_crash_elf64_headers’:
> fs/proc/vmcore.c:939:47: warning: initialization from incompatible pointer type [-Wincompatible-pointer-types]
Yes, the temporaries still need to have their pointed types changed, to
`Elf32_Ehdr' and `Elf64_Ehdr' respectively, as in the original change.
I had it mentioned in a WIP version of my review (stating that it would
verify that the correct type is used by the caller), but then deleted that
part inadvertently, sigh.
Daniel, sorry for the extra iteration.
Maciej
^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH v4 2/2] mips: Differentiate between 32 and 64 bit ELF header
@ 2016-02-11 12:14 ` Daniel Wagner
0 siblings, 0 replies; 72+ messages in thread
From: Daniel Wagner @ 2016-02-11 12:14 UTC (permalink / raw)
To: Maciej W. Rozycki, Ralf Baechle; +Cc: linux-kernel, linux-mips
On 02/11/2016 01:04 PM, Maciej W. Rozycki wrote:
> On Thu, 11 Feb 2016, Ralf Baechle wrote:
>> Thanks, applied.
>>
>> I'm getting a less spectacular warning from gcc 5.2:
>>
>> CC fs/proc/vmcore.o
>> fs/proc/vmcore.c: In function ‘parse_crash_elf64_headers’:
>> fs/proc/vmcore.c:939:47: warning: initialization from incompatible pointer type [-Wincompatible-pointer-types]
>
> Yes, the temporaries still need to have their pointed types changed, to
> `Elf32_Ehdr' and `Elf64_Ehdr' respectively, as in the original change.
>
> I had it mentioned in a WIP version of my review (stating that it would
> verify that the correct type is used by the caller), but then deleted that
> part inadvertently, sigh.
Yeah, that part fall through the cracks.
> Daniel, sorry for the extra iteration.
No problem. Just a sec.
cheers,
daniel
^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH v4 2/2] mips: Differentiate between 32 and 64 bit ELF header
@ 2016-02-11 12:14 ` Daniel Wagner
0 siblings, 0 replies; 72+ messages in thread
From: Daniel Wagner @ 2016-02-11 12:14 UTC (permalink / raw)
To: Maciej W. Rozycki, Ralf Baechle; +Cc: linux-kernel, linux-mips
On 02/11/2016 01:04 PM, Maciej W. Rozycki wrote:
> On Thu, 11 Feb 2016, Ralf Baechle wrote:
>> Thanks, applied.
>>
>> I'm getting a less spectacular warning from gcc 5.2:
>>
>> CC fs/proc/vmcore.o
>> fs/proc/vmcore.c: In function ‘parse_crash_elf64_headers’:
>> fs/proc/vmcore.c:939:47: warning: initialization from incompatible pointer type [-Wincompatible-pointer-types]
>
> Yes, the temporaries still need to have their pointed types changed, to
> `Elf32_Ehdr' and `Elf64_Ehdr' respectively, as in the original change.
>
> I had it mentioned in a WIP version of my review (stating that it would
> verify that the correct type is used by the caller), but then deleted that
> part inadvertently, sigh.
Yeah, that part fall through the cracks.
> Daniel, sorry for the extra iteration.
No problem. Just a sec.
cheers,
daniel
^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH v4 2/2] mips: Differentiate between 32 and 64 bit ELF header
@ 2016-02-11 14:58 ` Maciej W. Rozycki
0 siblings, 0 replies; 72+ messages in thread
From: Maciej W. Rozycki @ 2016-02-11 14:58 UTC (permalink / raw)
To: Ralf Baechle; +Cc: Daniel Wagner, linux-kernel, linux-mips
On Thu, 11 Feb 2016, Maciej W. Rozycki wrote:
> > > Signed-off-by: Daniel Wagner <daniel.wagner@bmw-carit.de>
> > > Suggested-by: Maciej W. Rozycki <macro@imgtec.com>
> > > Reviewed-by: Maciej W. Rozycki <macro@imgtec.com>
> > > Reported-by: Fengguang Wu <fengguang.wu@intel.com>
> >
> > Thanks, applied.
> >
> > I'm getting a less spectacular warning from gcc 5.2:
> >
> > CC fs/proc/vmcore.o
> > fs/proc/vmcore.c: In function ‘parse_crash_elf64_headers’:
> > fs/proc/vmcore.c:939:47: warning: initialization from incompatible pointer type [-Wincompatible-pointer-types]
>
> Yes, the temporaries still need to have their pointed types changed, to
> `Elf32_Ehdr' and `Elf64_Ehdr' respectively, as in the original change.
>
> I had it mentioned in a WIP version of my review (stating that it would
> verify that the correct type is used by the caller), but then deleted that
> part inadvertently, sigh.
Hold on, I was right in dropping it actually.
With your v4 change in place all `parse_crash_elf64_headers' is supposed
to call is `mips_elf_check_machine' and that doesn't make any
intialisations, it just dereferences the pointer passed once. This error
does not make any sense to me and line 939 isn't even in
`parse_crash_elf64_headers', which starts at line 999, it's in
`process_ptload_program_headers_elf32'.
So Ralf, what tree are you using that is off from LMO/Linus by 60 lines?
BTW line 939 at LMO and in Linus's tree looks like:
Elf32_Phdr *phdr_ptr;
and the pointer is assigned to at line 944 like this:
phdr_ptr = (Elf32_Phdr*)(elfptr + sizeof(Elf32_Ehdr)); /* PT_NOTE hdr */
so this does not explain the error. I get a clean build with this version
of Daniel's patches, both 32-bit and 64-bit, and FAOD with
CONFIG_PROC_VMCORE=y.
Daniel, please hold on with further updates, before this is cleared.
Your v4 looks fine to me AFAICT, no need to change types. Sorry about
this all confusion, something's clearly broken somewhere -- maybe due to
someone else's unpublished patch.
Maciej
^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH v4 2/2] mips: Differentiate between 32 and 64 bit ELF header
@ 2016-02-11 14:58 ` Maciej W. Rozycki
0 siblings, 0 replies; 72+ messages in thread
From: Maciej W. Rozycki @ 2016-02-11 14:58 UTC (permalink / raw)
To: Ralf Baechle; +Cc: Daniel Wagner, linux-kernel, linux-mips
On Thu, 11 Feb 2016, Maciej W. Rozycki wrote:
> > > Signed-off-by: Daniel Wagner <daniel.wagner@bmw-carit.de>
> > > Suggested-by: Maciej W. Rozycki <macro@imgtec.com>
> > > Reviewed-by: Maciej W. Rozycki <macro@imgtec.com>
> > > Reported-by: Fengguang Wu <fengguang.wu@intel.com>
> >
> > Thanks, applied.
> >
> > I'm getting a less spectacular warning from gcc 5.2:
> >
> > CC fs/proc/vmcore.o
> > fs/proc/vmcore.c: In function ‘parse_crash_elf64_headers’:
> > fs/proc/vmcore.c:939:47: warning: initialization from incompatible pointer type [-Wincompatible-pointer-types]
>
> Yes, the temporaries still need to have their pointed types changed, to
> `Elf32_Ehdr' and `Elf64_Ehdr' respectively, as in the original change.
>
> I had it mentioned in a WIP version of my review (stating that it would
> verify that the correct type is used by the caller), but then deleted that
> part inadvertently, sigh.
Hold on, I was right in dropping it actually.
With your v4 change in place all `parse_crash_elf64_headers' is supposed
to call is `mips_elf_check_machine' and that doesn't make any
intialisations, it just dereferences the pointer passed once. This error
does not make any sense to me and line 939 isn't even in
`parse_crash_elf64_headers', which starts at line 999, it's in
`process_ptload_program_headers_elf32'.
So Ralf, what tree are you using that is off from LMO/Linus by 60 lines?
BTW line 939 at LMO and in Linus's tree looks like:
Elf32_Phdr *phdr_ptr;
and the pointer is assigned to at line 944 like this:
phdr_ptr = (Elf32_Phdr*)(elfptr + sizeof(Elf32_Ehdr)); /* PT_NOTE hdr */
so this does not explain the error. I get a clean build with this version
of Daniel's patches, both 32-bit and 64-bit, and FAOD with
CONFIG_PROC_VMCORE=y.
Daniel, please hold on with further updates, before this is cleared.
Your v4 looks fine to me AFAICT, no need to change types. Sorry about
this all confusion, something's clearly broken somewhere -- maybe due to
someone else's unpublished patch.
Maciej
^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH v4 2/2] mips: Differentiate between 32 and 64 bit ELF header
2016-02-11 14:58 ` Maciej W. Rozycki
(?)
@ 2016-02-11 15:30 ` Ralf Baechle
-1 siblings, 0 replies; 72+ messages in thread
From: Ralf Baechle @ 2016-02-11 15:30 UTC (permalink / raw)
To: Maciej W. Rozycki; +Cc: Daniel Wagner, linux-kernel, linux-mips
On Thu, Feb 11, 2016 at 02:58:55PM +0000, Maciej W. Rozycki wrote:
> Date: Thu, 11 Feb 2016 14:58:55 +0000
> From: "Maciej W. Rozycki" <macro@imgtec.com>
> To: Ralf Baechle <ralf@linux-mips.org>
> CC: Daniel Wagner <daniel.wagner@bmw-carit.de>,
> linux-kernel@vger.kernel.org, linux-mips@linux-mips.org
> Subject: Re: [PATCH v4 2/2] mips: Differentiate between 32 and 64 bit ELF
> header
> Content-Type: text/plain; charset="ISO-8859-7"
>
> On Thu, 11 Feb 2016, Maciej W. Rozycki wrote:
>
> > > > Signed-off-by: Daniel Wagner <daniel.wagner@bmw-carit.de>
> > > > Suggested-by: Maciej W. Rozycki <macro@imgtec.com>
> > > > Reviewed-by: Maciej W. Rozycki <macro@imgtec.com>
> > > > Reported-by: Fengguang Wu <fengguang.wu@intel.com>
> > >
> > > Thanks, applied.
> > >
> > > I'm getting a less spectacular warning from gcc 5.2:
> > >
> > > CC fs/proc/vmcore.o
> > > fs/proc/vmcore.c: In function ‘parse_crash_elf64_headers’:
> > > fs/proc/vmcore.c:939:47: warning: initialization from incompatible pointer type [-Wincompatible-pointer-types]
> >
> > Yes, the temporaries still need to have their pointed types changed, to
> > `Elf32_Ehdr' and `Elf64_Ehdr' respectively, as in the original change.
> >
> > I had it mentioned in a WIP version of my review (stating that it would
> > verify that the correct type is used by the caller), but then deleted that
> > part inadvertently, sigh.
>
> Hold on, I was right in dropping it actually.
>
> With your v4 change in place all `parse_crash_elf64_headers' is supposed
> to call is `mips_elf_check_machine' and that doesn't make any
> intialisations, it just dereferences the pointer passed once. This error
> does not make any sense to me and line 939 isn't even in
> `parse_crash_elf64_headers', which starts at line 999, it's in
> `process_ptload_program_headers_elf32'.
>
> So Ralf, what tree are you using that is off from LMO/Linus by 60 lines?
That was 3.16, the oldest version affected. But I'm getting the same
messages with different line numbers on more recent kernels including
the master branch.
Ralf
^ permalink raw reply [flat|nested] 72+ messages in thread
* Re: [PATCH v3 3/3] mips: Differentiate between 32 and 64 bit ELF header
@ 2016-02-08 16:58 ` Maciej W. Rozycki
0 siblings, 0 replies; 72+ messages in thread
From: Maciej W. Rozycki @ 2016-02-08 16:58 UTC (permalink / raw)
To: Daniel Wagner; +Cc: Ralf Baechle, linux-kernel, linux-mips
On Mon, 8 Feb 2016, Daniel Wagner wrote:
> Therefore, we rather define vmcore_elf{32|64}_check_arch() as a
> basic machine check and use it also in binfm_elf?32.c as well.
>
> Signed-off-by: Daniel Wagner <daniel.wagner@bmw-carit.de>
> Suggested-by: Maciej W. Rozycki <macro@imgtec.com>
> Reported-by: Fengguang Wu <fengguang.wu@intel.com>
> ---
Reviewed-by: Maciej W. Rozycki <macro@imgtec.com>
Maciej
^ permalink raw reply [flat|nested] 72+ messages in thread* Re: [PATCH v3 3/3] mips: Differentiate between 32 and 64 bit ELF header
@ 2016-02-08 16:58 ` Maciej W. Rozycki
0 siblings, 0 replies; 72+ messages in thread
From: Maciej W. Rozycki @ 2016-02-08 16:58 UTC (permalink / raw)
To: Daniel Wagner; +Cc: Ralf Baechle, linux-kernel, linux-mips
On Mon, 8 Feb 2016, Daniel Wagner wrote:
> Therefore, we rather define vmcore_elf{32|64}_check_arch() as a
> basic machine check and use it also in binfm_elf?32.c as well.
>
> Signed-off-by: Daniel Wagner <daniel.wagner@bmw-carit.de>
> Suggested-by: Maciej W. Rozycki <macro@imgtec.com>
> Reported-by: Fengguang Wu <fengguang.wu@intel.com>
> ---
Reviewed-by: Maciej W. Rozycki <macro@imgtec.com>
Maciej
^ permalink raw reply [flat|nested] 72+ messages in thread