[PATCH tip v6 0/5] Simple wait queue support

[PATCH tip v6 0/5] Simple wait queue support

[Daniel Wagner]

Hi,

The -Werror=incompatible-pointer-types compiler flag is now
unconditionally added. The compile test with various configurations
and architectures didn't show up any show problem. *fingers crossed*

Obviously, the series is rebased and I rerun the KVM tests
to gather more up to date values. That's all for this version.

These patches are against

  tip/sched/core 0905f04eb21fc1c2e690bed5d0418a061d56c225

also available as git tree:

  git://git.kernel.org/pub/scm/linux/kernel/git/wagi/linux.git tip-swait

cheers,
daniel

changes since v5:
 - unconditionally add -Werror=incompatible-pointer-types
 - updated KVM statistics in commit message
 - rebased on tip/sched/core 
 - added ack-by PeterZ

changes since v4:
 - replaced patch #2 which tried to force to compiler to
   exit with an error by using compile time assertion type
   check macros. Instead use -Werror=incompatible-pointer-types
   to tell the compiler to barf loudly.
 - fixed wrong API usage in patch 4 as reported by Boqun.

changes since v3
 - rebased it on tip/sched/core (KVM bits have changed slightly)
 - added compile time type check assertion
 - added non lazy version of swake_up_locked()

changes since v2
 - rebased again on tip/master. The patches apply
   cleanly on v4.3-rc6 too.
 - fixed up mips
 - reordered patches to avoid lockdep warning when doing bissect.
 - remove unnecessary initialization of rsp->rda in rcu_init_one().

changes since v1 (PATCH v0)
 - rebased and fixed some typos found by cross building
   for S390, ARM and powerpc. For some unknown reason didn't catch
   them last time.
 - dropped completion patches because it is not clear yet
   how to handle complete_all() calls hard-irq/atomic contexts
   and swake_up_all.

changes since v0 (RFC v0)
 - promoted the series to PATCH state instead of RFC
 - fixed a few fallouts with build all and some cross compilers
   such ARM, PowerPC, S390.
 - Added the simple waitqueue transformation for KVM from -rt
   including some numbers requested by Paolo.
 - Added a commit message to PeterZ's patch. Hope he likes it.

[I got the numbering wrong in v1, so instead 'PATCH v1' you find it
 as 'PATCH v0' series]

v5: https://lkml.org/lkml/2015/11/30/318
v4: https://lwn.net/Articles/665655/
v3: https://lwn.net/Articles/661415/
v2: https://lwn.net/Articles/660628/
v1: https://lwn.net/Articles/656942/
v0: https://lwn.net/Articles/653586/

Daniel Wagner (2):
  kbuild: Add option to turn incompatible pointer check into error
  rcu: Do not call rcu_nocb_gp_cleanup() while holding rnp->lock

Marcelo Tosatti (1):
  KVM: use simple waitqueue for vcpu->wq

Paul Gortmaker (1):
  rcu: use simple wait queues where possible in rcutree

Peter Zijlstra (Intel) (1):
  wait.[ch]: Introduce the simple waitqueue (swait) implementation

 Makefile                            |   3 +
 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 +-
 include/linux/swait.h               | 172 ++++++++++++++++++++++++++++++++++++
 kernel/rcu/tree.c                   |  24 ++---
 kernel/rcu/tree.h                   |  12 +--
 kernel/rcu/tree_plugin.h            |  32 ++++---
 kernel/sched/Makefile               |   2 +-
 kernel/sched/swait.c                | 123 ++++++++++++++++++++++++++
 virt/kvm/async_pf.c                 |   4 +-
 virt/kvm/kvm_main.c                 |  17 ++--
 18 files changed, 380 insertions(+), 69 deletions(-)
 create mode 100644 include/linux/swait.h
 create mode 100644 kernel/sched/swait.c

-- 
2.5.0

[PATCH tip v6 1/5] wait.[ch]: Introduce the simple waitqueue (swait) implementation

[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

[PATCH tip v6 2/5] kbuild: Add option to turn incompatible pointer check into error

[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

[PATCH tip v6 3/5] KVM: use simple waitqueue for vcpu->wq

[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

[PATCH tip v6 4/5] rcu: Do not call rcu_nocb_gp_cleanup() while holding rnp->lock

[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

[PATCH tip v6 5/5] rcu: use simple wait queues where possible in rcutree

[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

Re: [PATCH tip v6 2/5] kbuild: Add option to turn incompatible pointer check into error

[Daniel Wagner]

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.

Re: [PATCH tip v6 2/5] kbuild: Add option to turn incompatible pointer check into error

[Daniel Wagner]

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.

Re: [PATCH tip v6 3/5] KVM: use simple waitqueue for vcpu->wq

[Daniel Wagner]

I missed one to fixup for ARM. I'll send an update.

Re: [PATCH tip v6 3/5] KVM: use simple waitqueue for vcpu->wq

[Daniel Wagner]

I missed one to fixup for ARM. I'll send an update.

[PATCH] video: Use bool instead int pointer for get_opt_bool() argument

[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

[PATCH] video: Use bool instead int pointer for get_opt_bool() argument

[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

[PATCH] MIPS: Differentiate between 32 and 64 bit ELF header

[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

Re: [PATCH tip v6 2/5] kbuild: Add option to turn incompatible pointer check into error

[Paul Gortmaker]

[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.
--

Re: [PATCH tip v6 2/5] kbuild: Add option to turn incompatible pointer check into error

[Paul Gortmaker]

[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.
--

Re: [PATCH] MIPS: Differentiate between 32 and 64 bit ELF header

[Maciej W. Rozycki]

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

Re: [PATCH] MIPS: Differentiate between 32 and 64 bit ELF header

[Maciej W. Rozycki]

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

Re: [PATCH tip v6 2/5] kbuild: Add option to turn incompatible pointer check into error

[Daniel Wagner]

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

Re: [PATCH tip v6 2/5] kbuild: Add option to turn incompatible pointer check into error

[Daniel Wagner]

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

Re: [PATCH] MIPS: Differentiate between 32 and 64 bit ELF header

[Daniel Wagner]

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

Re: [PATCH] MIPS: Differentiate between 32 and 64 bit ELF header

[Daniel Wagner]

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

Re: [PATCH tip v6 2/5] kbuild: Add option to turn incompatible pointer check into error

[Daniel Wagner]

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

Re: [PATCH tip v6 2/5] kbuild: Add option to turn incompatible pointer check into error

[Daniel Wagner]

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

Re: [PATCH] MIPS: Differentiate between 32 and 64 bit ELF header

[Maciej W. Rozycki]

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

Re: [PATCH] MIPS: Differentiate between 32 and 64 bit ELF header

[Maciej W. Rozycki]

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

Re: [PATCH tip v6 2/5] kbuild: Add option to turn incompatible pointer check into error

[Paul Gortmaker]

[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

Re: [PATCH tip v6 2/5] kbuild: Add option to turn incompatible pointer check into error

[Paul Gortmaker]

[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

[PATCH v3 0/3] Differentiate between 32 and 64 bit ELF header

[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

[PATCH v3 1/3] mips: Use arch specific auxvec.h instead of generic-asm version

[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

[PATCH v3 2/3] crash_dump: Add vmcore_elf32_check_arch

[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

[PATCH v3 3/3] mips: Differentiate between 32 and 64 bit ELF header

[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

Re: [PATCH v3 3/3] mips: Differentiate between 32 and 64 bit ELF header

[kbuild test robot]

[-- 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 --]

Re: [PATCH v3 3/3] mips: Differentiate between 32 and 64 bit ELF header

[kbuild test robot]

[-- 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 --]

Re: [PATCH v3 3/3] mips: Differentiate between 32 and 64 bit ELF header

[Maciej W. Rozycki]

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

Re: [PATCH v3 3/3] mips: Differentiate between 32 and 64 bit ELF header

[Maciej W. Rozycki]

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

Re: [PATCH v3 2/3] crash_dump: Add vmcore_elf32_check_arch

[Maciej W. Rozycki]

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

Re: [PATCH v3 2/3] crash_dump: Add vmcore_elf32_check_arch

[Maciej W. Rozycki]

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

Re: [PATCH v3 1/3] mips: Use arch specific auxvec.h instead of generic-asm version

[Maciej W. Rozycki]

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

Re: [PATCH v3 1/3] mips: Use arch specific auxvec.h instead of generic-asm version

[Maciej W. Rozycki]

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

Re: [PATCH v3 1/3] mips: Use arch specific auxvec.h instead of generic-asm version

[Daniel Wagner]

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

Re: [PATCH v3 1/3] mips: Use arch specific auxvec.h instead of generic-asm version

[Daniel Wagner]

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

Re: [PATCH v3 3/3] mips: Differentiate between 32 and 64 bit ELF header

[Daniel Wagner]

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

Re: [PATCH v3 3/3] mips: Differentiate between 32 and 64 bit ELF header

[Daniel Wagner]

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

Re: [PATCH v3 1/3] mips: Use arch specific auxvec.h instead of generic-asm version

[Maciej W. Rozycki]

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

Re: [PATCH v3 1/3] mips: Use arch specific auxvec.h instead of generic-asm version

[Maciej W. Rozycki]

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

Re: [PATCH v3 3/3] mips: Differentiate between 32 and 64 bit ELF header

[Maciej W. Rozycki]

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

Re: [PATCH v3 3/3] mips: Differentiate between 32 and 64 bit ELF header

[Maciej W. Rozycki]

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

Re: [PATCH v3 1/3] mips: Use arch specific auxvec.h instead of generic-asm version

[Daniel Wagner]

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

Re: [PATCH v3 1/3] mips: Use arch specific auxvec.h instead of generic-asm version

[Daniel Wagner]

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

Re: [PATCH v3 3/3] mips: Differentiate between 32 and 64 bit ELF header

[Daniel Wagner]

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

Re: [PATCH v3 3/3] mips: Differentiate between 32 and 64 bit ELF header

[Daniel Wagner]

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

Re: [PATCH v3 1/3] mips: Use arch specific auxvec.h instead of generic-asm version

[Maciej W. Rozycki]

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

Re: [PATCH v3 1/3] mips: Use arch specific auxvec.h instead of generic-asm version

[Maciej W. Rozycki]

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

Re: [PATCH v3 3/3] mips: Differentiate between 32 and 64 bit ELF header

[Maciej W. Rozycki]

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

Re: [PATCH v3 3/3] mips: Differentiate between 32 and 64 bit ELF header

[Maciej W. Rozycki]

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

Re: [PATCH v3 3/3] mips: Differentiate between 32 and 64 bit ELF header

[Daniel Wagner]

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

Re: [PATCH v3 3/3] mips: Differentiate between 32 and 64 bit ELF header

[Daniel Wagner]

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

Re: [PATCH v3 1/3] mips: Use arch specific auxvec.h instead of generic-asm version

[Daniel Wagner]

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.

Re: [PATCH v3 1/3] mips: Use arch specific auxvec.h instead of generic-asm version

[Daniel Wagner]

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.

[PATCH v4 0/2] Differentiate between 32 and 64 bit ELF header

[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

[PATCH v4 1/2] crash_dump: Add vmcore_elf32_check_arch

[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

[PATCH v4 2/2] mips: Differentiate between 32 and 64 bit ELF header

[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

Re: [PATCH v4 2/2] mips: Differentiate between 32 and 64 bit ELF header

[Ralf Baechle]

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

Re: [PATCH v4 2/2] mips: Differentiate between 32 and 64 bit ELF header

[Maciej W. Rozycki]

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

Re: [PATCH v4 2/2] mips: Differentiate between 32 and 64 bit ELF header

[Maciej W. Rozycki]

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

Re: [PATCH v4 2/2] mips: Differentiate between 32 and 64 bit ELF header

[Daniel Wagner]

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

Re: [PATCH v4 2/2] mips: Differentiate between 32 and 64 bit ELF header

[Daniel Wagner]

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

Re: [PATCH v4 2/2] mips: Differentiate between 32 and 64 bit ELF header

[Maciej W. Rozycki]

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

Re: [PATCH v4 2/2] mips: Differentiate between 32 and 64 bit ELF header

[Maciej W. Rozycki]

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

Re: [PATCH v4 2/2] mips: Differentiate between 32 and 64 bit ELF header

[Ralf Baechle]

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

Re: [PATCH tip v6 2/5] kbuild: Add option to turn incompatible pointer check into error

[Daniel Wagner]

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

Re: [PATCH tip v6 2/5] kbuild: Add option to turn incompatible pointer check into error

[Daniel Wagner]

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