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

Groups > linux.kernel > #1320764 > unrolled thread

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

Back to article view | Back to linux.kernel

Contents

  [PATCH tip v6 0/5] Simple wait queue support Daniel Wagner <daniel.wagner@bmw-carit.de> - 2016-01-28 16:00 +0100
    [PATCH tip v6 5/5] rcu: use simple wait queues where possible in rcutree Daniel Wagner <daniel.wagner@bmw-carit.de> - 2016-01-28 16:00 +0100
    [PATCH] MIPS: Differentiate between 32 and 64 bit ELF header Daniel Wagner <daniel.wagner@bmw-carit.de> - 2016-01-29 14:30 +0100
      Re: [PATCH] MIPS: Differentiate between 32 and 64 bit ELF header "Maciej W. Rozycki" <macro@imgtec.com> - 2016-02-01 02:00 +0100
        Re: [PATCH] MIPS: Differentiate between 32 and 64 bit ELF header Daniel Wagner <daniel.wagner@bmw-carit.de> - 2016-02-01 17:10 +0100
          Re: [PATCH] MIPS: Differentiate between 32 and 64 bit ELF header "Maciej W. Rozycki" <macro@imgtec.com> - 2016-02-06 18:20 +0100
            [PATCH v3 0/3] Differentiate between 32 and 64 bit ELF header Daniel Wagner <daniel.wagner@bmw-carit.de> - 2016-02-08 16:50 +0100
              [PATCH v3 3/3] mips: Differentiate between 32 and 64 bit ELF header Daniel Wagner <daniel.wagner@bmw-carit.de> - 2016-02-08 16:50 +0100
                Re: [PATCH v3 3/3] mips: Differentiate between 32 and 64 bit ELF  header kbuild test robot <lkp@intel.com> - 2016-02-08 17:30 +0100
                  Re: [PATCH v3 3/3] mips: Differentiate between 32 and 64 bit ELF  header Daniel Wagner <daniel.wagner@bmw-carit.de> - 2016-02-09 09:10 +0100
                Re: [PATCH v3 3/3] mips: Differentiate between 32 and 64 bit ELF  header "Maciej W. Rozycki" <macro@imgtec.com> - 2016-02-08 18:10 +0100
              [PATCH v3 1/3] mips: Use arch specific auxvec.h instead of generic-asm version Daniel Wagner <daniel.wagner@bmw-carit.de> - 2016-02-08 16:50 +0100
                Re: [PATCH v3 1/3] mips: Use arch specific auxvec.h instead of  generic-asm version "Maciej W. Rozycki" <macro@imgtec.com> - 2016-02-08 18:20 +0100
                  Re: [PATCH v3 1/3] mips: Use arch specific auxvec.h instead of  generic-asm version Daniel Wagner <daniel.wagner@bmw-carit.de> - 2016-02-09 08:10 +0100
              [PATCH v3 2/3] crash_dump: Add vmcore_elf32_check_arch Daniel Wagner <daniel.wagner@bmw-carit.de> - 2016-02-08 16:50 +0100
                Re: [PATCH v3 2/3] crash_dump: Add vmcore_elf32_check_arch "Maciej W. Rozycki" <macro@imgtec.com> - 2016-02-08 18:10 +0100
    [PATCH] video: Use bool instead int pointer for get_opt_bool() argument Daniel Wagner <daniel.wagner@bmw-carit.de> - 2016-01-29 14:30 +0100

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

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

[toc] | [next] | [standalone]

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

[Multipart message — attachments visible in raw view] — view raw

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

#1329214 — [PATCH v3 2/3] crash_dump: Add vmcore_elf32_check_arch

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

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

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

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

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

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

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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web