All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 2/3] cpuidle: allow setting deepest idle
From: Jacob Pan @ 2016-11-14 18:47 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Thomas Gleixner, LKML, Linux PM
  Cc: Arjan van de Ven, Srinivas Pandruvada, Len Brown, Rafael Wysocki,
	Eduardo Valentin, Zhang Rui, Petr Mladek,
	Sebastian Andrzej Siewior, Jacob Pan
In-Reply-To: <1479149278-12418-1-git-send-email-jacob.jun.pan@linux.intel.com>

When idle injection is used to cap power, we need to override
governor's choice of idle states. This patch allows caller to select
the deepest idle state on a CPU therefore achieve the maximum
potential power saving.

Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
---
 drivers/cpuidle/cpuidle.c | 11 +++++++++++
 include/linux/cpuidle.h   |  4 +++-
 kernel/sched/idle.c       |  3 +++
 3 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
index c73207a..887a52a 100644
--- a/drivers/cpuidle/cpuidle.c
+++ b/drivers/cpuidle/cpuidle.c
@@ -97,6 +97,17 @@ static int find_deepest_state(struct cpuidle_driver *drv,
 	return ret;
 }
 
+/* Set the current cpu to use the deepest idle state, override governors */
+void cpuidle_use_deepest_state(bool enable)
+{
+	struct cpuidle_device *dev;
+
+	preempt_disable();
+	dev = cpuidle_get_device();
+	dev->use_deepest_state = enable;
+	preempt_enable();
+}
+
 #ifdef CONFIG_SUSPEND
 /**
  * cpuidle_find_deepest_state - Find the deepest available idle state.
diff --git a/include/linux/cpuidle.h b/include/linux/cpuidle.h
index bb31373..7f93c63 100644
--- a/include/linux/cpuidle.h
+++ b/include/linux/cpuidle.h
@@ -74,6 +74,7 @@ struct cpuidle_state {
 struct cpuidle_device {
 	unsigned int		registered:1;
 	unsigned int		enabled:1;
+	unsigned int		use_deepest_state:1;
 	unsigned int		cpu;
 
 	int			last_residency;
@@ -192,11 +193,12 @@ static inline struct cpuidle_driver *cpuidle_get_cpu_driver(
 static inline struct cpuidle_device *cpuidle_get_device(void) {return NULL; }
 #endif
 
-#if defined(CONFIG_CPU_IDLE) && defined(CONFIG_SUSPEND)
+#if defined(CONFIG_CPU_IDLE)
 extern int cpuidle_find_deepest_state(struct cpuidle_driver *drv,
 				      struct cpuidle_device *dev);
 extern int cpuidle_enter_freeze(struct cpuidle_driver *drv,
 				struct cpuidle_device *dev);
+extern void cpuidle_use_deepest_state(bool enable);
 #else
 static inline int cpuidle_find_deepest_state(struct cpuidle_driver *drv,
 					     struct cpuidle_device *dev)
diff --git a/kernel/sched/idle.c b/kernel/sched/idle.c
index cb6442f..9e80f32 100644
--- a/kernel/sched/idle.c
+++ b/kernel/sched/idle.c
@@ -173,6 +173,9 @@ static void cpuidle_idle_call(void)
 
 		next_state = cpuidle_find_deepest_state(drv, dev);
 		call_cpuidle(drv, dev, next_state);
+	} else if (dev->use_deepest_state) {
+		next_state = cpuidle_find_deepest_state(drv, dev);
+		call_cpuidle(drv, dev, next_state);
 	} else {
 		/*
 		 * Ask the cpuidle framework to choose a convenient idle state.
-- 
1.9.1


^ permalink raw reply related

* [PATCH v2 0/3] Stop sched tick in idle injection task
From: Jacob Pan @ 2016-11-14 18:47 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Thomas Gleixner, LKML, Linux PM
  Cc: Arjan van de Ven, Srinivas Pandruvada, Len Brown, Rafael Wysocki,
	Eduardo Valentin, Zhang Rui, Petr Mladek,
	Sebastian Andrzej Siewior, Jacob Pan

Changelog:
v2:
	- moved duration timer from powerclamp driver to play_idle()
	- unexport cpuidle_use_deepest_state
	- indentation fix

Idle injection drivers today use RT threads to run idle loop. There are
efficiency and accounting issues with the current intel_powerclamp.c
and acpi_pad.c. A while ago, I posted CFS based idle injection patch trying
to address them:
https://lkml.org/lkml/2015/11/13/576

Peter proposed another approach with the introduction of a PF_IDLE flag.
This patchset is based on his original posting:
https://lkml.org/lkml/2014/6/4/56

These patches apply on top of the kworker and cpu hotplug state machine
changes made to Intel powerclamp driver.
https://lkml.org/lkml/2016/10/17/362

Similar changes to ACPI PAD driver is developed along with other
enhancements. It will be posted after this patchset is accepted.

Jacob Pan (2):
  cpuidle: allow setting deepest idle
  thermal/powerclamp: stop sched tick in forced idle

Peter Zijlstra (1):
  idle: add support for tasks that inject idle

 drivers/cpuidle/cpuidle.c          |  11 +++
 drivers/thermal/intel_powerclamp.c |  35 +-------
 include/linux/cpu.h                |   2 +
 include/linux/cpuidle.h            |   4 +-
 include/linux/sched.h              |   3 +-
 kernel/fork.c                      |   3 +
 kernel/sched/core.c                |   1 +
 kernel/sched/idle.c                | 177 +++++++++++++++++++++++--------------
 8 files changed, 136 insertions(+), 100 deletions(-)

-- 
1.9.1


^ permalink raw reply

* Re: [Qemu-devel] [PATCH 07/13] virtio-scsi: always use dataplane path if ioeventfd is active
From: Alex Williamson @ 2016-11-14 18:49 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: cornelia.huck, famz, qemu-devel, stefanha, mst
In-Reply-To: <d24130ca-3912-e27a-0a8c-73b5d948c297@redhat.com>

[-- Attachment #1: Type: text/plain, Size: 686 bytes --]

On Mon, 14 Nov 2016 19:10:54 +0100
Paolo Bonzini <pbonzini@redhat.com> wrote:

> On 14/11/2016 18:09, Alex Williamson wrote:
> > Hmm, fixed yet not fixed.  I get a nice shutdown and it even eliminates
> > a cpu spike shown in virt-manager at the end of shutdown that was
> > typical previously, but then I noticed dmesg showing me segfaults, so I
> > hooked up gdb and:  
> 
> Well, that I don't get the segfault already says something...  Though 
> it's not even clear from the backtrace what is causing the segfault.  
> Let's try the same printf without the change.

Log starting from virsh shutdown.  The final line occurs just as the VM
starts spinning on all vcpus.  Thanks,

Alex

[-- Attachment #2: shutdown.log.bz2 --]
[-- Type: application/x-bzip, Size: 6221 bytes --]

^ permalink raw reply

* [ANNOUNCE] libdrm 2.4.72
From: Matt Turner @ 2016-11-14 18:47 UTC (permalink / raw)
  To: xorg-announce; +Cc: dri-devel


[-- Attachment #1.1: Type: text/plain, Size: 1837 bytes --]

Alex Deucher (1):
      amdgpu: check parameters in amdgpu_query_gpu_info

Chris Wilson (3):
      intel: Export raw GEM mmap interfaces
      intel: Migrate handle/name lookups from linear lists to hashtables
      intel: Look prime handle up in handle hash table

Eric Anholt (1):
      Silence runtime complaints on platform devices

Junwei Zhang (1):
      amdgpu: add the function to get the marketing name (v4)

Matt Turner (4):
      intel: Add uthash.h to Makefile.sources.
      amdgpu: Add amdgpu_asic_id.h to Makefile.sources.
      freedreno: Add fd_ringbuffer_flush2 to symbol check.
      Bump version for release

Michel Dänzer (3):
      headers: Sync drm{,_mode}.h with the kernel
      Add drmModePageFlipTarget
      intel: Add new symbols to intel-symbol-check

Neil Roberts (1):
      intel: Allow some codenames in INTEL_DEVID_OVERRIDE

Rob Clark (3):
      add libsync.h helper
      freedreno: sync uapi header
      freedreno: add fence fd support

Rob Herring (1):
      Return an -ENODEV from drmGetDevice() when no device was found.

git tag: libdrm-2.4.72

https://dri.freedesktop.org/libdrm/libdrm-2.4.72.tar.bz2
MD5:  5ca170c39609430a3c39f15906523448  libdrm-2.4.72.tar.bz2
SHA1: 371772e90a7db67a7544d6377e6e53b19d6eafe6  libdrm-2.4.72.tar.bz2
SHA256: 16295ef61a7dec87216fd74a06225f68e3ac3e95224cb31454d2577ac46ccc89  libdrm-2.4.72.tar.bz2
PGP:  https://dri.freedesktop.org/libdrm/libdrm-2.4.72.tar.bz2.sig

https://dri.freedesktop.org/libdrm/libdrm-2.4.72.tar.gz
MD5:  74253ec3d33d2e1b440159f487abbd04  libdrm-2.4.72.tar.gz
SHA1: b8b5bb8483ec27e426e66c49b5a446f4be09e36a  libdrm-2.4.72.tar.gz
SHA256: aa41b8df7b9a2c6cbd710b36e8b556939762fdc1841af072fdedc1992ae2e900  libdrm-2.4.72.tar.gz
PGP:  https://dri.freedesktop.org/libdrm/libdrm-2.4.72.tar.gz.sig


[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply

* [PATCH v2 1/3] idle: add support for tasks that inject idle
From: Jacob Pan @ 2016-11-14 18:47 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Thomas Gleixner, LKML, Linux PM
  Cc: Arjan van de Ven, Srinivas Pandruvada, Len Brown, Rafael Wysocki,
	Eduardo Valentin, Zhang Rui, Petr Mladek,
	Sebastian Andrzej Siewior, Jacob Pan
In-Reply-To: <1479149278-12418-1-git-send-email-jacob.jun.pan@linux.intel.com>

From: Peter Zijlstra <peterz@infradead.org>

Idle injection drivers such as Intel powerclamp and ACPI PAD drivers use
realtime tasks to take control of CPU then inject idle. There are two issues
with this approach:
 1. Low efficiency: injected idle task is treated as busy so sched ticks do
    not stop during injected idle period, the result of these unwanted
    wakeups can be ~20% loss in power savings.
 2. Idle accounting: injected idle time is presented to user as busy.

This patch addresses the issues by introducing a new PF_IDLE flag which
allows any given task to be treated as idle task while the flag is set.
Therefore, idle injection tasks can run through the normal flow of NOHZ idle
enter/exit to get the correct accounting as well as tick stop when possible.

The implication is that idle task is then no longer limited to PID == 0.
Inheriting PF_IDLE flag from parents is also prevented.

Signed-off-by: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
---
 include/linux/cpu.h   |   2 +
 include/linux/sched.h |   3 +-
 kernel/fork.c         |   3 +
 kernel/sched/core.c   |   1 +
 kernel/sched/idle.c   | 174 +++++++++++++++++++++++++++++++-------------------
 5 files changed, 118 insertions(+), 65 deletions(-)

diff --git a/include/linux/cpu.h b/include/linux/cpu.h
index b886dc1..ac0efae 100644
--- a/include/linux/cpu.h
+++ b/include/linux/cpu.h
@@ -245,6 +245,8 @@ static inline void enable_nonboot_cpus(void) {}
 int cpu_report_state(int cpu);
 int cpu_check_up_prepare(int cpu);
 void cpu_set_state_online(int cpu);
+void play_idle(unsigned long duration_ms);
+
 #ifdef CONFIG_HOTPLUG_CPU
 bool cpu_wait_death(unsigned int cpu, int seconds);
 bool cpu_report_death(void);
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 348f51b..114c7fc 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -2254,6 +2254,7 @@ static inline cputime_t task_gtime(struct task_struct *t)
 /*
  * Per process flags
  */
+#define PF_IDLE		0x00000002	/* I am an IDLE thread */
 #define PF_EXITING	0x00000004	/* getting shut down */
 #define PF_EXITPIDONE	0x00000008	/* pi exit done on shut down */
 #define PF_VCPU		0x00000010	/* I'm a virtual CPU */
@@ -2609,7 +2610,7 @@ extern int sched_setattr(struct task_struct *,
  */
 static inline bool is_idle_task(const struct task_struct *p)
 {
-	return p->pid == 0;
+	return !!(p->flags & PF_IDLE);
 }
 extern struct task_struct *curr_task(int cpu);
 extern void ia64_set_curr_task(int cpu, struct task_struct *p);
diff --git a/kernel/fork.c b/kernel/fork.c
index 997ac1d..73edbcd 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -1819,6 +1819,9 @@ static __latent_entropy struct task_struct *copy_process(
 	threadgroup_change_end(current);
 	perf_event_fork(p);
 
+	/* do not inherit idle task */
+	p->flags &= ~PF_IDLE;
+
 	trace_task_newtask(p, clone_flags);
 	uprobe_copy_process(p, clone_flags);
 
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 154fd68..c95fbcd 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -5279,6 +5279,7 @@ void init_idle(struct task_struct *idle, int cpu)
 	__sched_fork(0, idle);
 	idle->state = TASK_RUNNING;
 	idle->se.exec_start = sched_clock();
+	idle->flags |= PF_IDLE;
 
 	kasan_unpoison_task_stack(idle);
 
diff --git a/kernel/sched/idle.c b/kernel/sched/idle.c
index 1d8718d..cb6442f 100644
--- a/kernel/sched/idle.c
+++ b/kernel/sched/idle.c
@@ -202,76 +202,68 @@ static void cpuidle_idle_call(void)
  *
  * Called with polling cleared.
  */
-static void cpu_idle_loop(void)
+static void do_idle(void)
 {
-	int cpu = smp_processor_id();
+	/*
+	 * If the arch has a polling bit, we maintain an invariant:
+	 *
+	 * Our polling bit is clear if we're not scheduled (i.e. if
+	 * rq->curr != rq->idle).  This means that, if rq->idle has
+	 * the polling bit set, then setting need_resched is
+	 * guaranteed to cause the cpu to reschedule.
+	 */
 
-	while (1) {
-		/*
-		 * If the arch has a polling bit, we maintain an invariant:
-		 *
-		 * Our polling bit is clear if we're not scheduled (i.e. if
-		 * rq->curr != rq->idle).  This means that, if rq->idle has
-		 * the polling bit set, then setting need_resched is
-		 * guaranteed to cause the cpu to reschedule.
-		 */
+	__current_set_polling();
+	tick_nohz_idle_enter();
 
-		__current_set_polling();
-		quiet_vmstat();
-		tick_nohz_idle_enter();
-
-		while (!need_resched()) {
-			check_pgt_cache();
-			rmb();
-
-			if (cpu_is_offline(cpu)) {
-				cpuhp_report_idle_dead();
-				arch_cpu_idle_dead();
-			}
-
-			local_irq_disable();
-			arch_cpu_idle_enter();
-
-			/*
-			 * In poll mode we reenable interrupts and spin.
-			 *
-			 * Also if we detected in the wakeup from idle
-			 * path that the tick broadcast device expired
-			 * for us, we don't want to go deep idle as we
-			 * know that the IPI is going to arrive right
-			 * away
-			 */
-			if (cpu_idle_force_poll || tick_check_broadcast_expired())
-				cpu_idle_poll();
-			else
-				cpuidle_idle_call();
-
-			arch_cpu_idle_exit();
-		}
+	while (!need_resched()) {
+		check_pgt_cache();
+		rmb();
 
-		/*
-		 * Since we fell out of the loop above, we know
-		 * TIF_NEED_RESCHED must be set, propagate it into
-		 * PREEMPT_NEED_RESCHED.
-		 *
-		 * This is required because for polling idle loops we will
-		 * not have had an IPI to fold the state for us.
-		 */
-		preempt_set_need_resched();
-		tick_nohz_idle_exit();
-		__current_clr_polling();
+		if (cpu_is_offline(smp_processor_id())) {
+			cpuhp_report_idle_dead();
+			arch_cpu_idle_dead();
+		}
 
-		/*
-		 * We promise to call sched_ttwu_pending and reschedule
-		 * if need_resched is set while polling is set.  That
-		 * means that clearing polling needs to be visible
-		 * before doing these things.
-		 */
-		smp_mb__after_atomic();
+		local_irq_disable();
+		arch_cpu_idle_enter();
 
-		sched_ttwu_pending();
-		schedule_preempt_disabled();
+	/* In poll mode we reenable interrupts and spin.
+	 * Also if we detected in the wakeup from idle
+	 * path that the tick broadcast device expired
+	 * for us, we don't want to go deep idle as we
+	 * know that the IPI is going to arrive right
+	 * away
+	 */
+		if (cpu_idle_force_poll || tick_check_broadcast_expired())
+			cpu_idle_poll();
+		else
+			cpuidle_idle_call();
+		arch_cpu_idle_exit();
 	}
+
+	/*
+	 * Since we fell out of the loop above, we know
+	 * TIF_NEED_RESCHED must be set, propagate it into
+	 * PREEMPT_NEED_RESCHED.
+	 *
+	 * This is required because for polling idle loops we will
+	 * not have had an IPI to fold the state for us.
+	 */
+	preempt_set_need_resched();
+	tick_nohz_idle_exit();
+	__current_clr_polling();
+
+	/*
+	 * We promise to call sched_ttwu_pending and reschedule
+	 * if need_resched is set while polling is set.  That
+	 * means that clearing polling needs to be visible
+	 * before doing these things.
+	 */
+	smp_mb__after_atomic();
+
+	sched_ttwu_pending();
+	schedule_preempt_disabled();
 }
 
 bool cpu_in_idle(unsigned long pc)
@@ -280,6 +272,58 @@ bool cpu_in_idle(unsigned long pc)
 		pc < (unsigned long)__cpuidle_text_end;
 }
 
+static enum hrtimer_restart idle_inject_timer_fn(struct hrtimer *hrtimer)
+{
+	set_tsk_need_resched(current);
+	return HRTIMER_NORESTART;
+}
+
+void play_idle(unsigned long duration_ms)
+{
+	struct hrtimer timer;
+	unsigned long end_time;
+
+	/*
+	 * Only FIFO tasks can disable the tick since they don't need the forced
+	 * preemption.
+	 */
+	WARN_ON_ONCE(current->policy != SCHED_FIFO);
+	WARN_ON_ONCE(current->nr_cpus_allowed != 1);
+	WARN_ON_ONCE(!(current->flags & PF_KTHREAD));
+	WARN_ON_ONCE(!(current->flags & PF_NO_SETAFFINITY));
+
+	rcu_sleep_check();
+	preempt_disable();
+	current->flags |= PF_IDLE;
+	cpuidle_use_deepest_state(true);
+
+	/*
+	 * If duration is 0, we will return after a natural wake event occurs. If
+	 * duration is none zero, we will go back to sleep if we were woken up earlier.
+	 * We also set up a timer to make sure we don't oversleep in deep idle.
+	 */
+	if (!duration_ms)
+		do_idle();
+	else {
+		hrtimer_init_on_stack(&timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
+		timer.function = idle_inject_timer_fn;
+		hrtimer_start(&timer, ms_to_ktime(duration_ms),
+			HRTIMER_MODE_REL_PINNED);
+		end_time = jiffies + msecs_to_jiffies(duration_ms);
+
+		while (time_after(end_time, jiffies))
+			do_idle();
+	}
+	hrtimer_cancel(&timer);
+
+	cpuidle_use_deepest_state(false);
+	current->flags &= ~PF_IDLE;
+
+	preempt_fold_need_resched();
+	preempt_enable();
+}
+EXPORT_SYMBOL_GPL(play_idle);
+
 void cpu_startup_entry(enum cpuhp_state state)
 {
 	/*
@@ -299,5 +343,7 @@ void cpu_startup_entry(enum cpuhp_state state)
 #endif
 	arch_cpu_idle_prepare();
 	cpuhp_online_idle(state);
-	cpu_idle_loop();
+	while (1)
+		do_idle();
+
 }
-- 
1.9.1

^ permalink raw reply related

* [PATCH] NVMe: Call nvme_pci_disable on error path of nvme_probe_work
From: Keith Busch @ 2016-11-14 18:47 UTC (permalink / raw)

In-Reply-To: <62459369-5c23-8819-d360-58892b4ff1fc@amazon.com>

On Mon, Nov 14, 2016@09:57:27AM +0100, Rashika Kheria wrote:
> Hi everyone,
> 
> Could you please review the following patch? This solves a regression in
> stable 4.4.y tree.

I missed the "Don't unmap" back-port to 4.4.y. I'm not sure, but I think
we may have addressed that differently with something less risky if we
needed that behaviour on 4.4-stable. That's okay, though, this new patch
looks correct. The original was part of a series that fixes this in its
following commit, but it should have looked like this from the beginning.

Acked-by: Keith Busch <keith.busch at intel.com>

 
> > On Tue, Nov 01, 2016@04:27:56PM +0100, Rashika Kheria wrote:
> > > Commit d5537e988eec ("NVMe: Don't unmap controller registers on reset"),
> > > introduced a regression in which it did not replace nvme_dev_unmap()
> > > with nvme_pci_disable() in the error path of nvme_probe_work().
> > > 
> > > This led to the following NVMe driver crash on systems where the devices
> > > did not initialise in the first try.
> > > 
> > > BUG: unable to handle kernel paging request at ffffc90006da001c
> > > IP: [<ffffffffa027b6bb>] nvme_dev_remove+0x5b/0xf0 [nvme]
> > > RIP: e030:[<ffffffffa027b6bb>]  [<ffffffffa027b6bb>]
> > > nvme_dev_remove+0x5b/0xf0 [nvme]
> > > RSP: e02b:ffff8806659c3cb8  EFLAGS: 00010286
> > > RAX: ffffc90006da0000 RBX: ffff88067cbc3000 RCX: 0000000000000006
> > > RDX: 0000000000000007 RSI: 0000000000000007 RDI: ffff8806864eda40
> > > RBP: ffff8806659c3cd8 R08: 0000000000000006 R09: 000000000000fffe
> > > R10: 0000000000000000 R11: 0000000000000000 R12: ffff88067e087000
> > > R13: ffffffffa0281d20 R14: ffff88067e087098 R15: ffff8806799d8598
> > > FS:  00007f880d5ba700(0000) GS:ffff8806864e0000(0000)
> > > knlGS:0000000000000000
> > > CS:  e033 DS: 0000 ES: 0000 CR0: 0000000080050033
> > > CR2: ffffc90006da001c CR3: 0000000676a97000 CR4: 0000000000042660
> > > Call Trace:
> > > [<ffffffffa027b7ea>] nvme_remove+0x9a/0x140 [nvme]
> > > [<ffffffff813503ef>] pci_device_remove+0x3f/0xc0
> > > [<ffffffff81449869>] ? __pm_runtime_idle+0x89/0x90
> > > [<ffffffff8143ed4f>] __device_release_driver+0xaf/0x140
> > > [<ffffffff8143eec8>] device_release_driver+0x28/0x40
> > > [<ffffffff8143db66>] unbind_store+0x96/0xb0
> > > [<ffffffff8143d027>] drv_attr_store+0x27/0x30
> > > [<ffffffff8122e279>] sysfs_kf_write+0x39/0x40
> > > [<ffffffff8122d9e4>] kernfs_fop_write+0xe4/0x160
> > > [<ffffffff811b15df>] __vfs_write+0x2f/0x100
> > > [<ffffffff81003640>] ? syscall_slow_exit_work+0x140/0x180
> > > [<ffffffff81161db9>] ? vm_mmap_pgoff+0xb9/0xe0
> > > [<ffffffff810af981>] ? percpu_down_read+0x11/0x60
> > > [<ffffffff811b2bce>] vfs_write+0xbe/0x190
> > > [<ffffffff811b2d81>] SyS_write+0x51/0xb0
> > > [<ffffffff815b8aee>] entry_SYSCALL_64_fastpath+0x12/0x71
> > > 
> > > Cc: stable at vger.kernel.org # 4.4.y
> > > Cc: Jens Axboe <axboe at fb.com>
> > > Cc: Keith Busch <keith.busch at intel.com>
> > > Cc: Gabriel Krisman Bertazi <krisman at linux.vnet.ibm.com>
> > > Cc: linux-nvme at lists.infradead.org
> > > Fixes: d5537e988eec ("NVMe: Don't unmap controller registers on reset")
> > > Signed-off-by: Rashika Kheria <rashika at amazon.de>
> > > ---
> > >   drivers/nvme/host/pci.c | 2 +-
> > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
> > > index c851bc5..f5d1579 100644
> > > --- a/drivers/nvme/host/pci.c
> > > +++ b/drivers/nvme/host/pci.c
> > > @@ -3184,7 +3184,7 @@ static void nvme_probe_work(struct work_struct *work)
> > >   	nvme_disable_queue(dev, 0);
> > >   	nvme_dev_list_remove(dev);
> > >    unmap:
> > > -	nvme_dev_unmap(dev);
> > > +	nvme_pci_disable(dev);
> > >    out:
> > >   	if (!work_busy(&dev->reset_work))
> > >   		nvme_dead_ctrl(dev);
> > > -- 
> > > 2.10.2
> > > 
> > ---end quoted text---

^ permalink raw reply

* Re: [PATCH v9 2/5] iio: adc: mxs-lradc: Add support for adc driver
From: Marek Vasut @ 2016-11-14 18:46 UTC (permalink / raw)
  To: Ksenija Stanojevic, linux-kernel
  Cc: lee.jones, dmitry.torokhov, linux-input, jic23, knaack.h, lars,
	pmeerw, linux-iio, harald, stefan.wahren, fabio.estevam
In-Reply-To: <c750ee15d03f6cf1a066dda21b1973d74cf9f59f.1478071676.git.ksenija.stanojevic@gmail.com>

On 11/02/2016 08:38 AM, Ksenija Stanojevic wrote:
> Add support for sixteen-channel 12-bit resolution ADC and its functions,
> which include general-purpose ADC readings, battery voltage measurement,
> and die temperature measurement.
> 
> Signed-off-by: Ksenija Stanojevic <ksenija.stanojevic@gmail.com>
> Reviewed-by: Jonathan Cameron <jic23@kernel.org>
> ---

Shouldn't you squash this and 4/5 , in which case this would result in a
move ?

-- 
Best regards,
Marek Vasut

^ permalink raw reply

* [PATCH 2/2] sched/autogroup: a zombie thread must not use autogroup->tg
From: Oleg Nesterov @ 2016-11-14 18:46 UTC (permalink / raw)
  To: Ingo Molnar, Linus Torvalds, Mike Galbraith, Peter Zijlstra
  Cc: hartsjc, vbendel, vlovejoy, linux-kernel
In-Reply-To: <20161114184548.GA15954@redhat.com>

Exactly because for_each_thread() in autogroup_move_group() can't see it
and update its ->sched_task_group before _put() and possibly free().

So the exiting task needs another sched_move_task() before exit_notify()
and we need to re-introduce the PF_EXITING (or similar) check removed by
the previous change for another reason.

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Cc: stable@vger.kernel.org
---
 include/linux/sched.h     |  2 ++
 kernel/exit.c             |  1 +
 kernel/sched/auto_group.c | 19 +++++++++++++++++++
 3 files changed, 22 insertions(+)

diff --git a/include/linux/sched.h b/include/linux/sched.h
index 348f51b..e9c009d 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -2567,6 +2567,7 @@ extern void sched_autogroup_create_attach(struct task_struct *p);
 extern void sched_autogroup_detach(struct task_struct *p);
 extern void sched_autogroup_fork(struct signal_struct *sig);
 extern void sched_autogroup_exit(struct signal_struct *sig);
+extern void sched_autogroup_exit_task(struct task_struct *p);
 #ifdef CONFIG_PROC_FS
 extern void proc_sched_autogroup_show_task(struct task_struct *p, struct seq_file *m);
 extern int proc_sched_autogroup_set_nice(struct task_struct *p, int nice);
@@ -2576,6 +2577,7 @@ static inline void sched_autogroup_create_attach(struct task_struct *p) { }
 static inline void sched_autogroup_detach(struct task_struct *p) { }
 static inline void sched_autogroup_fork(struct signal_struct *sig) { }
 static inline void sched_autogroup_exit(struct signal_struct *sig) { }
+static inline void sched_autogroup_exit_task(struct task_struct *p) { }
 #endif
 
 extern int yield_to(struct task_struct *p, bool preempt);
diff --git a/kernel/exit.c b/kernel/exit.c
index f3dd46d..76e263e 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -837,6 +837,7 @@ void __noreturn do_exit(long code)
 	 */
 	perf_event_exit_task(tsk);
 
+	sched_autogroup_exit_task(tsk);
 	cgroup_exit(tsk);
 
 	/*
diff --git a/kernel/sched/auto_group.c b/kernel/sched/auto_group.c
index ad2b19a..f1c8fd5 100644
--- a/kernel/sched/auto_group.c
+++ b/kernel/sched/auto_group.c
@@ -115,10 +115,26 @@ bool task_wants_autogroup(struct task_struct *p, struct task_group *tg)
 	 * If we race with autogroup_move_group() the caller can use the old
 	 * value of signal->autogroup but in this case sched_move_task() will
 	 * be called again before autogroup_kref_put().
+	 *
+	 * However, there is no way sched_autogroup_exit_task() could tell us
+	 * to avoid autogroup->tg, so we abuse PF_EXITING flag for this case.
 	 */
+	if (p->flags & PF_EXITING)
+		return false;
+
 	return true;
 }
 
+void sched_autogroup_exit_task(struct task_struct *p)
+{
+	/*
+	 * We are going to call exit_notify() and autogroup_move_group() can't
+	 * see this thread after that: we can no longer use signal->autogroup.
+	 * See the PF_EXITING check in task_wants_autogroup().
+	 */
+	sched_move_task(p);
+}
+
 static void
 autogroup_move_group(struct task_struct *p, struct autogroup *ag)
 {
@@ -142,6 +158,9 @@ autogroup_move_group(struct task_struct *p, struct autogroup *ag)
 	 * In the latter case for_each_thread() can not miss a migrating thread,
 	 * cpu_cgroup_attach() must not be possible after cgroup_exit() and it
 	 * can't be removed from thread list, we hold ->siglock.
+	 *
+	 * If an exiting thread was already removed from thread list we rely on
+	 * sched_autogroup_exit_task().
 	 */
 	for_each_thread(p, t)
 		sched_move_task(t);
-- 
2.5.0

^ permalink raw reply related

* [PATCH 1/2] sched/autogroup: autogroup_move_group() must never skip sched_move_task()
From: Oleg Nesterov @ 2016-11-14 18:46 UTC (permalink / raw)
  To: Ingo Molnar, Linus Torvalds, Mike Galbraith, Peter Zijlstra
  Cc: hartsjc, vbendel, vlovejoy, linux-kernel
In-Reply-To: <20161114184548.GA15954@redhat.com>

The PF_EXITING check in task_wants_autogroup() is no longer needed. Remove
it, but see the next patch.

However the comment is correct in that autogroup_move_group() must always
change task_group() for every thread so the sysctl_ check is very wrong;
we can race with cgroups and even sys_setsid() is not safe because a task
running with task_group() == ag->tg must participate in refcounting:

	int main(void)
	{
		int sctl = open("/proc/sys/kernel/sched_autogroup_enabled", O_WRONLY);

		assert(sctl > 0);
		if (fork()) {
			wait(NULL); // destroy the child's ag/tg
			pause();
		}

		assert(pwrite(sctl, "1\n", 2, 0) == 2);
		assert(setsid() > 0);
		if (fork())
			pause();

		kill(getppid(), SIGKILL);
		sleep(1);

		// The child has gone, the grandchild runs with kref == 1
		assert(pwrite(sctl, "0\n", 2, 0) == 2);
		assert(setsid() > 0);

		// runs with the freed ag/tg
		for (;;)
			sleep(1);

		return 0;
	}

crashes the kernel. It doesn't really need sleep(1), it doesn't matter if
autogroup_move_group() actually frees the task_group or this happens later.

Reported-by: Vern Lovejoy <vlovejoy@redhat.com>
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Cc: stable@vger.kernel.org
---
 kernel/sched/auto_group.c | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/kernel/sched/auto_group.c b/kernel/sched/auto_group.c
index a5d966c..ad2b19a 100644
--- a/kernel/sched/auto_group.c
+++ b/kernel/sched/auto_group.c
@@ -111,14 +111,11 @@ bool task_wants_autogroup(struct task_struct *p, struct task_group *tg)
 {
 	if (tg != &root_task_group)
 		return false;
-
 	/*
-	 * We can only assume the task group can't go away on us if
-	 * autogroup_move_group() can see us on ->thread_group list.
+	 * If we race with autogroup_move_group() the caller can use the old
+	 * value of signal->autogroup but in this case sched_move_task() will
+	 * be called again before autogroup_kref_put().
 	 */
-	if (p->flags & PF_EXITING)
-		return false;
-
 	return true;
 }
 
@@ -138,13 +135,17 @@ autogroup_move_group(struct task_struct *p, struct autogroup *ag)
 	}
 
 	p->signal->autogroup = autogroup_kref_get(ag);
-
-	if (!READ_ONCE(sysctl_sched_autogroup_enabled))
-		goto out;
-
+	/*
+	 * We can't avoid sched_move_task() after we changed signal->autogroup,
+	 * this process can already run with task_group() == prev->tg or we can
+	 * race with cgroup code which can read autogroup = prev under rq->lock.
+	 * In the latter case for_each_thread() can not miss a migrating thread,
+	 * cpu_cgroup_attach() must not be possible after cgroup_exit() and it
+	 * can't be removed from thread list, we hold ->siglock.
+	 */
 	for_each_thread(p, t)
 		sched_move_task(t);
-out:
+
 	unlock_task_sighand(p, &flags);
 	autogroup_kref_put(prev);
 }
-- 
2.5.0

^ permalink raw reply related

* [PATCH 0/2] sched/autogroup: use-after-free fixes
From: Oleg Nesterov @ 2016-11-14 18:45 UTC (permalink / raw)
  To: Ingo Molnar, Linus Torvalds, Mike Galbraith, Peter Zijlstra
  Cc: hartsjc, vbendel, vlovejoy, linux-kernel

Two simple fixes for -stable.

Note that this logic was wrong from the very beginning and even if sysctl
doesn't change, see 2/2.

And I think it needs some cleanups in any case, but lets do later/separately.

Please review,

Oleg.

 include/linux/sched.h     |  2 ++
 kernel/exit.c             |  1 +
 kernel/sched/auto_group.c | 36 ++++++++++++++++++++++++++++--------
 3 files changed, 31 insertions(+), 8 deletions(-)

^ permalink raw reply

* Re: [PATCH RFC] mm: Add debug_virt_to_phys()
From: Laura Abbott @ 2016-11-14 18:45 UTC (permalink / raw)
  To: Florian Fainelli, linux-kernel
  Cc: Russell King, Catalin Marinas, Will Deacon, Arnd Bergmann,
	Nicolas Pitre, Chris Brandt, Pratyush Anand, Ard Biesheuvel,
	Mark Rutland, James Morse, Neeraj Upadhyay, Andrew Morton,
	Vlastimil Babka, Michal Hocko, Kirill A. Shutemov,
	Jerome Marchand, Konstantin Khlebnikov, Joonsoo Kim,
	moderated list:ARM PORT,
	open list:GENERIC INCLUDE/ASM HEADER FILES,
	open list:MEMORY MANAGEMENT
In-Reply-To: <20161112004449.30566-1-f.fainelli@gmail.com>

On 11/11/2016 04:44 PM, Florian Fainelli wrote:
> When CONFIG_DEBUG_VM is turned on, virt_to_phys() maps to
> debug_virt_to_phys() which helps catch vmalloc space addresses being
> passed. This is helpful in debugging bogus drivers that just assume
> linear mappings all over the place.
> 
> For ARM, ARM64, Unicore32 and Microblaze, the architectures define
> __virt_to_phys() as being the functional implementation of the address
> translation, so we special case the debug stub to call into
> __virt_to_phys directly.
> 
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
>  arch/arm/include/asm/memory.h      |  4 ++++
>  arch/arm64/include/asm/memory.h    |  4 ++++
>  include/asm-generic/memory_model.h |  4 ++++
>  mm/debug.c                         | 15 +++++++++++++++
>  4 files changed, 27 insertions(+)
> 
> diff --git a/arch/arm/include/asm/memory.h b/arch/arm/include/asm/memory.h
> index 76cbd9c674df..448dec9b8b00 100644
> --- a/arch/arm/include/asm/memory.h
> +++ b/arch/arm/include/asm/memory.h
> @@ -260,11 +260,15 @@ static inline unsigned long __phys_to_virt(phys_addr_t x)
>   * translation for translating DMA addresses.  Use the driver
>   * DMA support - see dma-mapping.h.
>   */
> +#ifndef CONFIG_DEBUG_VM
>  #define virt_to_phys virt_to_phys
>  static inline phys_addr_t virt_to_phys(const volatile void *x)
>  {
>  	return __virt_to_phys((unsigned long)(x));
>  }
> +#else
> +#define virt_to_phys debug_virt_to_phys
> +#endif
>  
>  #define phys_to_virt phys_to_virt
>  static inline void *phys_to_virt(phys_addr_t x)
> diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
> index b71086d25195..c9e436b28523 100644
> --- a/arch/arm64/include/asm/memory.h
> +++ b/arch/arm64/include/asm/memory.h
> @@ -186,11 +186,15 @@ extern u64			kimage_voffset;
>   * translation for translating DMA addresses.  Use the driver
>   * DMA support - see dma-mapping.h.
>   */
> +#ifndef CONFIG_DEBUG_VM
>  #define virt_to_phys virt_to_phys
>  static inline phys_addr_t virt_to_phys(const volatile void *x)
>  {
>  	return __virt_to_phys((unsigned long)(x));
>  }
> +#else
> +#define virt_to_phys debug_virt_to_phys
> +#endif
>  
>  #define phys_to_virt phys_to_virt
>  static inline void *phys_to_virt(phys_addr_t x)
> diff --git a/include/asm-generic/memory_model.h b/include/asm-generic/memory_model.h
> index 5148150cc80b..426085757258 100644
> --- a/include/asm-generic/memory_model.h
> +++ b/include/asm-generic/memory_model.h
> @@ -80,6 +80,10 @@
>  #define page_to_pfn __page_to_pfn
>  #define pfn_to_page __pfn_to_page
>  
> +#ifdef CONFIG_DEBUG_VM
> +unsigned long debug_virt_to_phys(volatile void *address);
> +#endif /* CONFIG_DEBUG_VM */
> +
>  #endif /* __ASSEMBLY__ */
>  
>  #endif
> diff --git a/mm/debug.c b/mm/debug.c
> index 9feb699c5d25..72b2ca9b11f4 100644
> --- a/mm/debug.c
> +++ b/mm/debug.c
> @@ -161,4 +161,19 @@ void dump_mm(const struct mm_struct *mm)
>  	);
>  }
>  
> +#include <asm/memory.h>
> +#include <linux/mm.h>
> +
> +unsigned long debug_virt_to_phys(volatile void *address)
> +{
> +	BUG_ON(is_vmalloc_addr((const void *)address));
> +#if defined(CONFIG_ARM) || defined(CONFIG_ARM64) || defined(CONFIG_UNICORE32) || \
> +	defined(CONFIG_MICROBLAZE)
> +	return __virt_to_phys(address);
> +#else
> +	return virt_to_phys(address);
> +#endif
> +}
> +EXPORT_SYMBOL(debug_virt_to_phys);
> +
>  #endif		/* CONFIG_DEBUG_VM */
> 

is_vmalloc_addr is necessary but not sufficient. This misses
cases like module addresses. The x86 version (CONFIG_DEBUG_VIRTUAL)
bounds checks against the known linear map to catch all cases.
I'm for a generic approach to this if it can catch all cases
that an architecture specific version would catch.

Thanks,
Laura

^ permalink raw reply

* Re: [PATCH RFC] mm: Add debug_virt_to_phys()
From: Laura Abbott @ 2016-11-14 18:45 UTC (permalink / raw)
  To: Florian Fainelli, linux-kernel
  Cc: Russell King, Catalin Marinas, Will Deacon, Arnd Bergmann,
	Nicolas Pitre, Chris Brandt, Pratyush Anand, Ard Biesheuvel,
	Mark Rutland, James Morse, Neeraj Upadhyay, Andrew Morton,
	Vlastimil Babka, Michal Hocko, Kirill A. Shutemov,
	Jerome Marchand, Konstantin Khlebnikov, Joonsoo Kim,
	"moderated list:ARM PORT" <linux-arm-kernel@
In-Reply-To: <20161112004449.30566-1-f.fainelli@gmail.com>

On 11/11/2016 04:44 PM, Florian Fainelli wrote:
> When CONFIG_DEBUG_VM is turned on, virt_to_phys() maps to
> debug_virt_to_phys() which helps catch vmalloc space addresses being
> passed. This is helpful in debugging bogus drivers that just assume
> linear mappings all over the place.
> 
> For ARM, ARM64, Unicore32 and Microblaze, the architectures define
> __virt_to_phys() as being the functional implementation of the address
> translation, so we special case the debug stub to call into
> __virt_to_phys directly.
> 
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
>  arch/arm/include/asm/memory.h      |  4 ++++
>  arch/arm64/include/asm/memory.h    |  4 ++++
>  include/asm-generic/memory_model.h |  4 ++++
>  mm/debug.c                         | 15 +++++++++++++++
>  4 files changed, 27 insertions(+)
> 
> diff --git a/arch/arm/include/asm/memory.h b/arch/arm/include/asm/memory.h
> index 76cbd9c674df..448dec9b8b00 100644
> --- a/arch/arm/include/asm/memory.h
> +++ b/arch/arm/include/asm/memory.h
> @@ -260,11 +260,15 @@ static inline unsigned long __phys_to_virt(phys_addr_t x)
>   * translation for translating DMA addresses.  Use the driver
>   * DMA support - see dma-mapping.h.
>   */
> +#ifndef CONFIG_DEBUG_VM
>  #define virt_to_phys virt_to_phys
>  static inline phys_addr_t virt_to_phys(const volatile void *x)
>  {
>  	return __virt_to_phys((unsigned long)(x));
>  }
> +#else
> +#define virt_to_phys debug_virt_to_phys
> +#endif
>  
>  #define phys_to_virt phys_to_virt
>  static inline void *phys_to_virt(phys_addr_t x)
> diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
> index b71086d25195..c9e436b28523 100644
> --- a/arch/arm64/include/asm/memory.h
> +++ b/arch/arm64/include/asm/memory.h
> @@ -186,11 +186,15 @@ extern u64			kimage_voffset;
>   * translation for translating DMA addresses.  Use the driver
>   * DMA support - see dma-mapping.h.
>   */
> +#ifndef CONFIG_DEBUG_VM
>  #define virt_to_phys virt_to_phys
>  static inline phys_addr_t virt_to_phys(const volatile void *x)
>  {
>  	return __virt_to_phys((unsigned long)(x));
>  }
> +#else
> +#define virt_to_phys debug_virt_to_phys
> +#endif
>  
>  #define phys_to_virt phys_to_virt
>  static inline void *phys_to_virt(phys_addr_t x)
> diff --git a/include/asm-generic/memory_model.h b/include/asm-generic/memory_model.h
> index 5148150cc80b..426085757258 100644
> --- a/include/asm-generic/memory_model.h
> +++ b/include/asm-generic/memory_model.h
> @@ -80,6 +80,10 @@
>  #define page_to_pfn __page_to_pfn
>  #define pfn_to_page __pfn_to_page
>  
> +#ifdef CONFIG_DEBUG_VM
> +unsigned long debug_virt_to_phys(volatile void *address);
> +#endif /* CONFIG_DEBUG_VM */
> +
>  #endif /* __ASSEMBLY__ */
>  
>  #endif
> diff --git a/mm/debug.c b/mm/debug.c
> index 9feb699c5d25..72b2ca9b11f4 100644
> --- a/mm/debug.c
> +++ b/mm/debug.c
> @@ -161,4 +161,19 @@ void dump_mm(const struct mm_struct *mm)
>  	);
>  }
>  
> +#include <asm/memory.h>
> +#include <linux/mm.h>
> +
> +unsigned long debug_virt_to_phys(volatile void *address)
> +{
> +	BUG_ON(is_vmalloc_addr((const void *)address));
> +#if defined(CONFIG_ARM) || defined(CONFIG_ARM64) || defined(CONFIG_UNICORE32) || \
> +	defined(CONFIG_MICROBLAZE)
> +	return __virt_to_phys(address);
> +#else
> +	return virt_to_phys(address);
> +#endif
> +}
> +EXPORT_SYMBOL(debug_virt_to_phys);
> +
>  #endif		/* CONFIG_DEBUG_VM */
> 

is_vmalloc_addr is necessary but not sufficient. This misses
cases like module addresses. The x86 version (CONFIG_DEBUG_VIRTUAL)
bounds checks against the known linear map to catch all cases.
I'm for a generic approach to this if it can catch all cases
that an architecture specific version would catch.

Thanks,
Laura

^ permalink raw reply

* Re: [PATCH RFC] mm: Add debug_virt_to_phys()
From: Laura Abbott @ 2016-11-14 18:45 UTC (permalink / raw)
  To: Florian Fainelli, linux-kernel
  Cc: Russell King, Catalin Marinas, Will Deacon, Arnd Bergmann,
	Nicolas Pitre, Chris Brandt, Pratyush Anand, Ard Biesheuvel,
	Mark Rutland, James Morse, Neeraj Upadhyay, Andrew Morton,
	Vlastimil Babka, Michal Hocko, Kirill A. Shutemov,
	Jerome Marchand, Konstantin Khlebnikov, Joonsoo Kim,
	moderated list:ARM PORT,
	open list:GENERIC INCLUDE/ASM HEADER FILES,
	open list:MEMORY MANAGEMENT
In-Reply-To: <20161112004449.30566-1-f.fainelli@gmail.com>

On 11/11/2016 04:44 PM, Florian Fainelli wrote:
> When CONFIG_DEBUG_VM is turned on, virt_to_phys() maps to
> debug_virt_to_phys() which helps catch vmalloc space addresses being
> passed. This is helpful in debugging bogus drivers that just assume
> linear mappings all over the place.
> 
> For ARM, ARM64, Unicore32 and Microblaze, the architectures define
> __virt_to_phys() as being the functional implementation of the address
> translation, so we special case the debug stub to call into
> __virt_to_phys directly.
> 
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
>  arch/arm/include/asm/memory.h      |  4 ++++
>  arch/arm64/include/asm/memory.h    |  4 ++++
>  include/asm-generic/memory_model.h |  4 ++++
>  mm/debug.c                         | 15 +++++++++++++++
>  4 files changed, 27 insertions(+)
> 
> diff --git a/arch/arm/include/asm/memory.h b/arch/arm/include/asm/memory.h
> index 76cbd9c674df..448dec9b8b00 100644
> --- a/arch/arm/include/asm/memory.h
> +++ b/arch/arm/include/asm/memory.h
> @@ -260,11 +260,15 @@ static inline unsigned long __phys_to_virt(phys_addr_t x)
>   * translation for translating DMA addresses.  Use the driver
>   * DMA support - see dma-mapping.h.
>   */
> +#ifndef CONFIG_DEBUG_VM
>  #define virt_to_phys virt_to_phys
>  static inline phys_addr_t virt_to_phys(const volatile void *x)
>  {
>  	return __virt_to_phys((unsigned long)(x));
>  }
> +#else
> +#define virt_to_phys debug_virt_to_phys
> +#endif
>  
>  #define phys_to_virt phys_to_virt
>  static inline void *phys_to_virt(phys_addr_t x)
> diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
> index b71086d25195..c9e436b28523 100644
> --- a/arch/arm64/include/asm/memory.h
> +++ b/arch/arm64/include/asm/memory.h
> @@ -186,11 +186,15 @@ extern u64			kimage_voffset;
>   * translation for translating DMA addresses.  Use the driver
>   * DMA support - see dma-mapping.h.
>   */
> +#ifndef CONFIG_DEBUG_VM
>  #define virt_to_phys virt_to_phys
>  static inline phys_addr_t virt_to_phys(const volatile void *x)
>  {
>  	return __virt_to_phys((unsigned long)(x));
>  }
> +#else
> +#define virt_to_phys debug_virt_to_phys
> +#endif
>  
>  #define phys_to_virt phys_to_virt
>  static inline void *phys_to_virt(phys_addr_t x)
> diff --git a/include/asm-generic/memory_model.h b/include/asm-generic/memory_model.h
> index 5148150cc80b..426085757258 100644
> --- a/include/asm-generic/memory_model.h
> +++ b/include/asm-generic/memory_model.h
> @@ -80,6 +80,10 @@
>  #define page_to_pfn __page_to_pfn
>  #define pfn_to_page __pfn_to_page
>  
> +#ifdef CONFIG_DEBUG_VM
> +unsigned long debug_virt_to_phys(volatile void *address);
> +#endif /* CONFIG_DEBUG_VM */
> +
>  #endif /* __ASSEMBLY__ */
>  
>  #endif
> diff --git a/mm/debug.c b/mm/debug.c
> index 9feb699c5d25..72b2ca9b11f4 100644
> --- a/mm/debug.c
> +++ b/mm/debug.c
> @@ -161,4 +161,19 @@ void dump_mm(const struct mm_struct *mm)
>  	);
>  }
>  
> +#include <asm/memory.h>
> +#include <linux/mm.h>
> +
> +unsigned long debug_virt_to_phys(volatile void *address)
> +{
> +	BUG_ON(is_vmalloc_addr((const void *)address));
> +#if defined(CONFIG_ARM) || defined(CONFIG_ARM64) || defined(CONFIG_UNICORE32) || \
> +	defined(CONFIG_MICROBLAZE)
> +	return __virt_to_phys(address);
> +#else
> +	return virt_to_phys(address);
> +#endif
> +}
> +EXPORT_SYMBOL(debug_virt_to_phys);
> +
>  #endif		/* CONFIG_DEBUG_VM */
> 

is_vmalloc_addr is necessary but not sufficient. This misses
cases like module addresses. The x86 version (CONFIG_DEBUG_VIRTUAL)
bounds checks against the known linear map to catch all cases.
I'm for a generic approach to this if it can catch all cases
that an architecture specific version would catch.

Thanks,
Laura

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply

* [PATCH RFC] mm: Add debug_virt_to_phys()
From: Laura Abbott @ 2016-11-14 18:45 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161112004449.30566-1-f.fainelli@gmail.com>

On 11/11/2016 04:44 PM, Florian Fainelli wrote:
> When CONFIG_DEBUG_VM is turned on, virt_to_phys() maps to
> debug_virt_to_phys() which helps catch vmalloc space addresses being
> passed. This is helpful in debugging bogus drivers that just assume
> linear mappings all over the place.
> 
> For ARM, ARM64, Unicore32 and Microblaze, the architectures define
> __virt_to_phys() as being the functional implementation of the address
> translation, so we special case the debug stub to call into
> __virt_to_phys directly.
> 
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
>  arch/arm/include/asm/memory.h      |  4 ++++
>  arch/arm64/include/asm/memory.h    |  4 ++++
>  include/asm-generic/memory_model.h |  4 ++++
>  mm/debug.c                         | 15 +++++++++++++++
>  4 files changed, 27 insertions(+)
> 
> diff --git a/arch/arm/include/asm/memory.h b/arch/arm/include/asm/memory.h
> index 76cbd9c674df..448dec9b8b00 100644
> --- a/arch/arm/include/asm/memory.h
> +++ b/arch/arm/include/asm/memory.h
> @@ -260,11 +260,15 @@ static inline unsigned long __phys_to_virt(phys_addr_t x)
>   * translation for translating DMA addresses.  Use the driver
>   * DMA support - see dma-mapping.h.
>   */
> +#ifndef CONFIG_DEBUG_VM
>  #define virt_to_phys virt_to_phys
>  static inline phys_addr_t virt_to_phys(const volatile void *x)
>  {
>  	return __virt_to_phys((unsigned long)(x));
>  }
> +#else
> +#define virt_to_phys debug_virt_to_phys
> +#endif
>  
>  #define phys_to_virt phys_to_virt
>  static inline void *phys_to_virt(phys_addr_t x)
> diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
> index b71086d25195..c9e436b28523 100644
> --- a/arch/arm64/include/asm/memory.h
> +++ b/arch/arm64/include/asm/memory.h
> @@ -186,11 +186,15 @@ extern u64			kimage_voffset;
>   * translation for translating DMA addresses.  Use the driver
>   * DMA support - see dma-mapping.h.
>   */
> +#ifndef CONFIG_DEBUG_VM
>  #define virt_to_phys virt_to_phys
>  static inline phys_addr_t virt_to_phys(const volatile void *x)
>  {
>  	return __virt_to_phys((unsigned long)(x));
>  }
> +#else
> +#define virt_to_phys debug_virt_to_phys
> +#endif
>  
>  #define phys_to_virt phys_to_virt
>  static inline void *phys_to_virt(phys_addr_t x)
> diff --git a/include/asm-generic/memory_model.h b/include/asm-generic/memory_model.h
> index 5148150cc80b..426085757258 100644
> --- a/include/asm-generic/memory_model.h
> +++ b/include/asm-generic/memory_model.h
> @@ -80,6 +80,10 @@
>  #define page_to_pfn __page_to_pfn
>  #define pfn_to_page __pfn_to_page
>  
> +#ifdef CONFIG_DEBUG_VM
> +unsigned long debug_virt_to_phys(volatile void *address);
> +#endif /* CONFIG_DEBUG_VM */
> +
>  #endif /* __ASSEMBLY__ */
>  
>  #endif
> diff --git a/mm/debug.c b/mm/debug.c
> index 9feb699c5d25..72b2ca9b11f4 100644
> --- a/mm/debug.c
> +++ b/mm/debug.c
> @@ -161,4 +161,19 @@ void dump_mm(const struct mm_struct *mm)
>  	);
>  }
>  
> +#include <asm/memory.h>
> +#include <linux/mm.h>
> +
> +unsigned long debug_virt_to_phys(volatile void *address)
> +{
> +	BUG_ON(is_vmalloc_addr((const void *)address));
> +#if defined(CONFIG_ARM) || defined(CONFIG_ARM64) || defined(CONFIG_UNICORE32) || \
> +	defined(CONFIG_MICROBLAZE)
> +	return __virt_to_phys(address);
> +#else
> +	return virt_to_phys(address);
> +#endif
> +}
> +EXPORT_SYMBOL(debug_virt_to_phys);
> +
>  #endif		/* CONFIG_DEBUG_VM */
> 

is_vmalloc_addr is necessary but not sufficient. This misses
cases like module addresses. The x86 version (CONFIG_DEBUG_VIRTUAL)
bounds checks against the known linear map to catch all cases.
I'm for a generic approach to this if it can catch all cases
that an architecture specific version would catch.

Thanks,
Laura

^ permalink raw reply

* Re: [PATCH 2/2] Add basic support for DW CSI-2 Host IPK
From: kbuild test robot @ 2016-11-14 18:44 UTC (permalink / raw)
  To: Ramiro Oliveira
  Cc: kbuild-all, robh+dt, mark.rutland, mchehab, devicetree,
	linux-kernel, linux-media, davem, gregkh, geert+renesas, akpm,
	linux, hverkuil, laurent.pinchart+renesas, arnd, sudipm.mukherjee,
	tiffany.lin, minghsiu.tsai, jean-christophe.trotin,
	andrew-ct.chen, simon.horman, songjun.wu, bparrot,
	CARLOS.PALMINHA, Ramiro.Oliveira
In-Reply-To: <c1699c43562aaae69ab851ff3955086131119c51.1479132355.git.roliveir@synopsys.com>

[-- Attachment #1: Type: text/plain, Size: 5550 bytes --]

Hi Ramiro,

[auto build test WARNING on linuxtv-media/master]
[also build test WARNING on v4.9-rc5]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Ramiro-Oliveira/Add-support-for-the-DW-IP-Prototyping-Kits-for-MIPI-CSI-2-Host/20161115-014759
base:   git://linuxtv.org/media_tree.git master
config: ia64-allmodconfig (attached as .config)
compiler: ia64-linux-gcc (GCC) 6.2.0
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=ia64 

All warnings (new ones prefixed by >>):

   In file included from include/linux/kernel.h:13:0,
                    from include/linux/list.h:8,
                    from include/linux/module.h:9,
                    from drivers/media/platform/dwc/dw_mipi_csi.h:12,
                    from drivers/media/platform/dwc/dw_mipi_csi.c:14:
   drivers/media/platform/dwc/dw_mipi_csi.c: In function 'dw_mipi_csi_irq1':
>> drivers/media/platform/dwc/dw_mipi_csi.c:437:9: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
            (uint32_t) dev->base_address, ind_status);
            ^
   include/linux/printk.h:398:17: note: in definition of macro 'printk_ratelimited'
      printk(fmt, ##__VA_ARGS__);    \
                    ^~~~~~~~~~~
>> drivers/media/platform/dwc/dw_mipi_csi.c:436:3: note: in expansion of macro 'pr_info_ratelimited'
      pr_info_ratelimited("%08X CSI INT PHY FATAL: %08X\n",
      ^~~~~~~~~~~~~~~~~~~
   drivers/media/platform/dwc/dw_mipi_csi.c:443:9: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
            (uint32_t) dev->base_address, ind_status);
            ^
   include/linux/printk.h:398:17: note: in definition of macro 'printk_ratelimited'
      printk(fmt, ##__VA_ARGS__);    \
                    ^~~~~~~~~~~
   drivers/media/platform/dwc/dw_mipi_csi.c:442:3: note: in expansion of macro 'pr_info_ratelimited'
      pr_info_ratelimited("%08X CSI INT PKT FATAL: %08X\n",
      ^~~~~~~~~~~~~~~~~~~
   drivers/media/platform/dwc/dw_mipi_csi.c:449:9: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
            (uint32_t) dev->base_address, ind_status);
            ^
   include/linux/printk.h:398:17: note: in definition of macro 'printk_ratelimited'
      printk(fmt, ##__VA_ARGS__);    \
                    ^~~~~~~~~~~
   drivers/media/platform/dwc/dw_mipi_csi.c:448:3: note: in expansion of macro 'pr_info_ratelimited'
      pr_info_ratelimited("%08X CSI INT FRAME FATAL: %08X\n",
      ^~~~~~~~~~~~~~~~~~~
   drivers/media/platform/dwc/dw_mipi_csi.c:455:9: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
            (uint32_t) dev->base_address, ind_status);
            ^
   include/linux/printk.h:398:17: note: in definition of macro 'printk_ratelimited'
      printk(fmt, ##__VA_ARGS__);    \
                    ^~~~~~~~~~~
   drivers/media/platform/dwc/dw_mipi_csi.c:454:3: note: in expansion of macro 'pr_info_ratelimited'
      pr_info_ratelimited("%08X CSI INT PHY: %08X\n",
      ^~~~~~~~~~~~~~~~~~~
   drivers/media/platform/dwc/dw_mipi_csi.c:461:9: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
            (uint32_t) dev->base_address, ind_status);
            ^
   include/linux/printk.h:398:17: note: in definition of macro 'printk_ratelimited'
      printk(fmt, ##__VA_ARGS__);    \
                    ^~~~~~~~~~~
   drivers/media/platform/dwc/dw_mipi_csi.c:460:3: note: in expansion of macro 'pr_info_ratelimited'
      pr_info_ratelimited("%08X CSI INT PKT: %08X\n",
      ^~~~~~~~~~~~~~~~~~~
   drivers/media/platform/dwc/dw_mipi_csi.c:467:9: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
            (uint32_t) dev->base_address, ind_status);
            ^
   include/linux/printk.h:398:17: note: in definition of macro 'printk_ratelimited'
      printk(fmt, ##__VA_ARGS__);    \
                    ^~~~~~~~~~~
   drivers/media/platform/dwc/dw_mipi_csi.c:466:3: note: in expansion of macro 'pr_info_ratelimited'
      pr_info_ratelimited("%08X CSI INT LINE: %08X\n",
      ^~~~~~~~~~~~~~~~~~~
   drivers/media/platform/dwc/dw_mipi_csi.c:473:9: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
            (uint32_t) dev->base_address, ind_status);
            ^
   include/linux/printk.h:398:17: note: in definition of macro 'printk_ratelimited'
      printk(fmt, ##__VA_ARGS__);    \
                    ^~~~~~~~~~~
   drivers/media/platform/dwc/dw_mipi_csi.c:472:3: note: in expansion of macro 'pr_info_ratelimited'
      pr_info_ratelimited("%08X CSI INT IPI: %08X\n",
      ^~~~~~~~~~~~~~~~~~~

vim +437 drivers/media/platform/dwc/dw_mipi_csi.c

   430	
   431		int_status = dw_mipi_csi_read(dev, R_CSI2_INTERRUPT);
   432		spin_lock_irqsave(&dev->slock, flags);
   433	
   434		if (int_status & CSI2_INT_PHY_FATAL) {
   435			ind_status = dw_mipi_csi_read(dev, R_CSI2_INT_PHY_FATAL);
 > 436			pr_info_ratelimited("%08X CSI INT PHY FATAL: %08X\n",
 > 437					    (uint32_t) dev->base_address, ind_status);
   438		}
   439	
   440		if (int_status & CSI2_INT_PKT_FATAL) {

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 45123 bytes --]

^ permalink raw reply

* Re: [RFC PATCH KERNEL 0/4] x86/xen: untangle PV and PVHVM guest support code
From: Boris Ostrovsky @ 2016-11-14 18:47 UTC (permalink / raw)
  To: David Vrabel, Vitaly Kuznetsov, xen-devel
  Cc: Juergen Gross, Andrew Jones, x86
In-Reply-To: <5cb811d5-a190-475b-a313-2656321aaec7@citrix.com>

On 11/14/2016 01:21 PM, David Vrabel wrote:
> On 14/11/16 17:17, Vitaly Kuznetsov wrote:
>> Hi,
>>
>> I have a long-standing idea to separate PV and PVHVM code in kernel and 
>> introduce Kconfig options to make it possible to enable the required
>> parts only breaking the current 'all or nothing' approach.
>>
>> Motivation:
>> - Xen related x86 code in kernel is rather big and it is unclear which
>>   parts of it are required for PV, for HVM or for both. With PVH coming
>>   into picture is becomes even more tangled. It makes it hard to
>>   understand/audit the code.
>>
>> - In some case we may want to avoid bloating kernel by supporting Xen
>>   guests we don't need. In particular, 90% of the code in arch/x86/xen/ is
>>   required to support PV guests and one may require PVHVM support only.
>>
>> - PV guests are supposed to go away one day and such code separation would
>>   help us to get ready.
> All good reasons.
>
>> This RFC adds XEN_PV Kconfig option and makes it possible to build PV-only
>> and PVHVM-only kernels. It also makes it possible to disable Dom0 support.
>> The series is incomplete and probably dirty in some places, I didn't pay
>> much attention to the current PVH implementation as (as far as I
>> understand) it is supposed to be replaced with PVHv2 but before investing
>> more I'd like to get opinions whether such refactoring will be welcomed.
> This series might be best done after PVHv1 is removed.  Boris, any
> thoughts on the best approach here?

I would prefer to wait until at least domU PVHv2 (together with removal
of v1) happens. As soon as I am done with ACPI hotplug on the hypervisor
side I will post the new version.

-boris

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply

* [GIT PULL] ARM: at91: drivers for 4.10
From: Alexandre Belloni @ 2016-11-14 18:44 UTC (permalink / raw)
  To: Arnd Bergmann, Olof Johansson, arm
  Cc: Nicolas Ferre, linux-arm-kernel, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 1802 bytes --]

Hi Arnd, Olof

A few fixes for the memory drivers and the support for the Secure SRAM
found on sama5d2.

The following changes since commit 1001354ca34179f3db924eb66672442a173147dc:

  Linux 4.9-rc1 (2016-10-15 12:17:50 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/abelloni/linux.git tags/at91-ab-4.10-drivers

for you to fetch changes up to 2ae2e28852f21ec9efc527c1f3ecc5f7c7e27e42:

  misc: sram: add Atmel securam support (2016-11-07 23:43:28 +0100)

----------------------------------------------------------------
Drivers for 4.10:

 - few fixes for the memory drivers
 - minimal security module driver
 - support for the Secure SRAM

----------------------------------------------------------------
Alexandre Belloni (4):
      Documentation: dt: atmel-at91: Document secumod bindings
      ARM: at91: add secumod register definitions
      misc: sram: document new compatible
      misc: sram: add Atmel securam support

Wei Yongjun (2):
      memory: atmel-ebi: fix return value check in at91_ebi_dev_disable()
      memory: atmel-sdramc: use builtin_platform_driver to simplify the code

 .../devicetree/bindings/arm/atmel-at91.txt         | 16 +++++++++
 Documentation/devicetree/bindings/sram/sram.txt    |  2 +-
 drivers/memory/atmel-ebi.c                         |  2 +-
 drivers/memory/atmel-sdramc.c                      |  6 +---
 drivers/misc/sram.c                                | 42 ++++++++++++++++++----
 include/soc/at91/atmel-secumod.h                   | 19 ++++++++++
 6 files changed, 73 insertions(+), 14 deletions(-)
 create mode 100644 include/soc/at91/atmel-secumod.h

-- 
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

^ permalink raw reply

* Re: [PATCH v2 01/11] x86/domctl: Add XEN_DOMCTL_set_avail_vcpus
From: Boris Ostrovsky @ 2016-11-14 18:44 UTC (permalink / raw)
  To: Andrew Cooper, xen-devel; +Cc: wei.liu2, ian.jackson, jbeulich, roger.pau
In-Reply-To: <788e5183-8827-14ba-d72e-202a2c10728c@citrix.com>

On 11/14/2016 01:19 PM, Andrew Cooper wrote:
> On 14/11/16 17:48, Boris Ostrovsky wrote:
>> On 11/14/2016 12:17 PM, Andrew Cooper wrote:
>>>>> I am not convinced though that we can start enforcing this new VCPU
>>>>> count, at least for PV guests. They expect to start all max VCPUs and
>>>>> then offline them. This, of course, can be fixed but all non-updated
>>>>> kernels will stop booting.
>>>> How about we don't clear _VPF_down if the bit in the availability bitmap
>>>> is not set?
>>> This is yet another PV mess.  We clearly need to quirk PV guests as the
>>> exception to sanity, given that they expect (and have been able to)
>>> online all cpus at start-of-day.
>>>
>>> To avoid race conditions, you necessarily need to be able to set a
>>> reduced permitted map before asking the VM to unplug.
>>>
>>> For HVM guests, we can set a proper permitted map at boot, and really
>>> should do so.
>>>
>>> For PV guests, we have to wait until it has completed its SMP bringup
>>> before reducing the permitted set.  Therefore, making the initial
>>> set_avail_vcpus call could be deferred until the first unplug request?
>> I am not sure how we can determine in the hypervisor that a guest has
>> completed the bringup: I don't think we can rely on the last VCPU (which
>> is maxvcpu-1) doing VCPUOP_up. Just to mess up with the hypervisor the
>> guest may decide to only bring up (maxvcpus-2) VCPUs. In other words, we
>> can't assume a well-behaved guest.
> I wasn't suggesting relying on the guest.  I was referring to the first
> unplug request at the toolstack level.

I don't think waiting for toolstack's (un)plug request is going to help
much --- the request may never come and the guest will be able to use
all maxvcpus.


>
>> And then, even if we do determine the point when (maxvcpus-1) VCPUs are
>> all up, when do we clamp them down to avail_vcpus? For the same reason,
>> we can't assume that the guest will VCPUOP_down all extra VCPUs.
> If at some point we observe all vcpus being up, then we could set the
> restricted map then.  However, I can't think of a useful way of
> identifying this point.

Exactly.

The question is then, if we can't do this for PV, should we still do it
for HVM?

>
>>> It also occurs to me that you necessarily need a get_avail_vcpus
>>> hypercall to be able to use this interface sensibly from the toolstack.
>> We could modify getdomaininfo but that would make set_avail_vcpus domctl
>> non-symmetrical.
>>
>> And what would the use of this information be anyway?
> Well, for a start, this information needs to move in the migration
> stream, or by migrating a VM you will lose its current availability
> bitmap and reintroduce the problem we are trying to solve.

Oh, right, of course.

-boris



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply

* Re: [PATCH v3 5/6] grep: enable recurse-submodules to work on <tree> objects
From: Jonathan Tan @ 2016-11-14 18:44 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, sbeller, Brandon Williams
In-Reply-To: <xmqqk2c6x79c.fsf@gitster.mtv.corp.google.com>

On 11/14/2016 10:10 AM, Junio C Hamano wrote:
> Brandon Williams <bmwill@google.com> writes:
>
>> Teach grep to recursively search in submodules when provided with a
>> <tree> object. This allows grep to search a submodule based on the state
>> of the submodule that is present in a commit of the super project.
>>
>> When grep is provided with a <tree> object, the name of the object is
>> prefixed to all output.  In order to provide uniformity of output
>> between the parent and child processes the option `--parent-basename`
>> has been added so that the child can preface all of it's output with the
>> name of the parent's object instead of the name of the commit SHA1 of
>> the submodule. This changes output from the command
>> `git grep -e. -l --recurse-submodules HEAD` from:
>> HEAD:file
>> <commit sha1 of submodule>:sub/file
>>
>> to:
>> HEAD:file
>> HEAD:sub/file
>>
>> Signed-off-by: Brandon Williams <bmwill@google.com>
>> ---
>
> Unrelated tangent, but this makes readers wonder what the updated
> trailer code would do to the last paragraph ;-).  Does it behave
> sensibly (with some sane definition of sensibleness)?
>
> I am guessing that it would, because neither To: or HEAD: is what we
> normally recognize as a known trailer block element.

Yes, it behaves sensibly :-) because "Signed-off-by:" is preceded by a 
blank line, so the trailer block consists only of that line.

Having said that, it is probably better to indent those examples in the 
commit message (by at least one space or one tab) - then they will never 
be confused with trailers (once my patch set is in).

^ permalink raw reply

* [GIT PULL] ARM: at91: drivers for 4.10
From: Alexandre Belloni @ 2016-11-14 18:44 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Arnd, Olof

A few fixes for the memory drivers and the support for the Secure SRAM
found on sama5d2.

The following changes since commit 1001354ca34179f3db924eb66672442a173147dc:

  Linux 4.9-rc1 (2016-10-15 12:17:50 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/abelloni/linux.git tags/at91-ab-4.10-drivers

for you to fetch changes up to 2ae2e28852f21ec9efc527c1f3ecc5f7c7e27e42:

  misc: sram: add Atmel securam support (2016-11-07 23:43:28 +0100)

----------------------------------------------------------------
Drivers for 4.10:

 - few fixes for the memory drivers
 - minimal security module driver
 - support for the Secure SRAM

----------------------------------------------------------------
Alexandre Belloni (4):
      Documentation: dt: atmel-at91: Document secumod bindings
      ARM: at91: add secumod register definitions
      misc: sram: document new compatible
      misc: sram: add Atmel securam support

Wei Yongjun (2):
      memory: atmel-ebi: fix return value check in at91_ebi_dev_disable()
      memory: atmel-sdramc: use builtin_platform_driver to simplify the code

 .../devicetree/bindings/arm/atmel-at91.txt         | 16 +++++++++
 Documentation/devicetree/bindings/sram/sram.txt    |  2 +-
 drivers/memory/atmel-ebi.c                         |  2 +-
 drivers/memory/atmel-sdramc.c                      |  6 +---
 drivers/misc/sram.c                                | 42 ++++++++++++++++++----
 include/soc/at91/atmel-secumod.h                   | 19 ++++++++++
 6 files changed, 73 insertions(+), 14 deletions(-)
 create mode 100644 include/soc/at91/atmel-secumod.h

-- 
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161114/287d05f1/attachment-0001.sig>

^ permalink raw reply

* Re: [PATCH 2/2] Add basic support for DW CSI-2 Host IPK
From: kbuild test robot @ 2016-11-14 18:44 UTC (permalink / raw)
  Cc: kbuild-all-JC7UmRfGjtg, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	mark.rutland-5wv7dgnIgG8, mchehab-DgEjT+Ai2ygdnm+yROfE0A,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-media-u79uwXL29TY76Z2rM5mHXA, davem-fT/PcQaiUtIeIZ0/mPfg9Q,
	gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
	geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ,
	akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
	linux-0h96xk9xTtrk1uMJSBkQmQ, hverkuil-qWit8jRvyhVmR6Xm/wNWPw,
	laurent.pinchart+renesas-ryLnwIuWjnjg/C1BVhZhaw, arnd-r2nGTMty4D4,
	sudipm.mukherjee-Re5JQEeQqe8AvxtiuMwx3w,
	tiffany.lin-NuS5LvNUpcJWk0Htik3J/w,
	minghsiu.tsai-NuS5LvNUpcJWk0Htik3J/w,
	jean-christophe.trotin-qxv4g6HH51o,
	andrew-ct.chen-NuS5LvNUpcJWk0Htik3J/w,
	simon.horman-wFxRvT7yatFl57MIdRCFDg,
	songjun.wu-UWL1GkI3JZL3oGB3hsPCZA, bparrot-l0cyMroinI0,
	CARLOS.PALMINHA-HKixBCOQz3hWk0Htik3J/w,
	Ramiro.Oliveira-HKixBCOQz3hWk0Htik3J/w
In-Reply-To: <c1699c43562aaae69ab851ff3955086131119c51.1479132355.git.roliveir-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>

[-- Attachment #1: Type: text/plain, Size: 5550 bytes --]

Hi Ramiro,

[auto build test WARNING on linuxtv-media/master]
[also build test WARNING on v4.9-rc5]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Ramiro-Oliveira/Add-support-for-the-DW-IP-Prototyping-Kits-for-MIPI-CSI-2-Host/20161115-014759
base:   git://linuxtv.org/media_tree.git master
config: ia64-allmodconfig (attached as .config)
compiler: ia64-linux-gcc (GCC) 6.2.0
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=ia64 

All warnings (new ones prefixed by >>):

   In file included from include/linux/kernel.h:13:0,
                    from include/linux/list.h:8,
                    from include/linux/module.h:9,
                    from drivers/media/platform/dwc/dw_mipi_csi.h:12,
                    from drivers/media/platform/dwc/dw_mipi_csi.c:14:
   drivers/media/platform/dwc/dw_mipi_csi.c: In function 'dw_mipi_csi_irq1':
>> drivers/media/platform/dwc/dw_mipi_csi.c:437:9: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
            (uint32_t) dev->base_address, ind_status);
            ^
   include/linux/printk.h:398:17: note: in definition of macro 'printk_ratelimited'
      printk(fmt, ##__VA_ARGS__);    \
                    ^~~~~~~~~~~
>> drivers/media/platform/dwc/dw_mipi_csi.c:436:3: note: in expansion of macro 'pr_info_ratelimited'
      pr_info_ratelimited("%08X CSI INT PHY FATAL: %08X\n",
      ^~~~~~~~~~~~~~~~~~~
   drivers/media/platform/dwc/dw_mipi_csi.c:443:9: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
            (uint32_t) dev->base_address, ind_status);
            ^
   include/linux/printk.h:398:17: note: in definition of macro 'printk_ratelimited'
      printk(fmt, ##__VA_ARGS__);    \
                    ^~~~~~~~~~~
   drivers/media/platform/dwc/dw_mipi_csi.c:442:3: note: in expansion of macro 'pr_info_ratelimited'
      pr_info_ratelimited("%08X CSI INT PKT FATAL: %08X\n",
      ^~~~~~~~~~~~~~~~~~~
   drivers/media/platform/dwc/dw_mipi_csi.c:449:9: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
            (uint32_t) dev->base_address, ind_status);
            ^
   include/linux/printk.h:398:17: note: in definition of macro 'printk_ratelimited'
      printk(fmt, ##__VA_ARGS__);    \
                    ^~~~~~~~~~~
   drivers/media/platform/dwc/dw_mipi_csi.c:448:3: note: in expansion of macro 'pr_info_ratelimited'
      pr_info_ratelimited("%08X CSI INT FRAME FATAL: %08X\n",
      ^~~~~~~~~~~~~~~~~~~
   drivers/media/platform/dwc/dw_mipi_csi.c:455:9: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
            (uint32_t) dev->base_address, ind_status);
            ^
   include/linux/printk.h:398:17: note: in definition of macro 'printk_ratelimited'
      printk(fmt, ##__VA_ARGS__);    \
                    ^~~~~~~~~~~
   drivers/media/platform/dwc/dw_mipi_csi.c:454:3: note: in expansion of macro 'pr_info_ratelimited'
      pr_info_ratelimited("%08X CSI INT PHY: %08X\n",
      ^~~~~~~~~~~~~~~~~~~
   drivers/media/platform/dwc/dw_mipi_csi.c:461:9: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
            (uint32_t) dev->base_address, ind_status);
            ^
   include/linux/printk.h:398:17: note: in definition of macro 'printk_ratelimited'
      printk(fmt, ##__VA_ARGS__);    \
                    ^~~~~~~~~~~
   drivers/media/platform/dwc/dw_mipi_csi.c:460:3: note: in expansion of macro 'pr_info_ratelimited'
      pr_info_ratelimited("%08X CSI INT PKT: %08X\n",
      ^~~~~~~~~~~~~~~~~~~
   drivers/media/platform/dwc/dw_mipi_csi.c:467:9: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
            (uint32_t) dev->base_address, ind_status);
            ^
   include/linux/printk.h:398:17: note: in definition of macro 'printk_ratelimited'
      printk(fmt, ##__VA_ARGS__);    \
                    ^~~~~~~~~~~
   drivers/media/platform/dwc/dw_mipi_csi.c:466:3: note: in expansion of macro 'pr_info_ratelimited'
      pr_info_ratelimited("%08X CSI INT LINE: %08X\n",
      ^~~~~~~~~~~~~~~~~~~
   drivers/media/platform/dwc/dw_mipi_csi.c:473:9: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
            (uint32_t) dev->base_address, ind_status);
            ^
   include/linux/printk.h:398:17: note: in definition of macro 'printk_ratelimited'
      printk(fmt, ##__VA_ARGS__);    \
                    ^~~~~~~~~~~
   drivers/media/platform/dwc/dw_mipi_csi.c:472:3: note: in expansion of macro 'pr_info_ratelimited'
      pr_info_ratelimited("%08X CSI INT IPI: %08X\n",
      ^~~~~~~~~~~~~~~~~~~

vim +437 drivers/media/platform/dwc/dw_mipi_csi.c

   430	
   431		int_status = dw_mipi_csi_read(dev, R_CSI2_INTERRUPT);
   432		spin_lock_irqsave(&dev->slock, flags);
   433	
   434		if (int_status & CSI2_INT_PHY_FATAL) {
   435			ind_status = dw_mipi_csi_read(dev, R_CSI2_INT_PHY_FATAL);
 > 436			pr_info_ratelimited("%08X CSI INT PHY FATAL: %08X\n",
 > 437					    (uint32_t) dev->base_address, ind_status);
   438		}
   439	
   440		if (int_status & CSI2_INT_PKT_FATAL) {

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 45123 bytes --]

^ permalink raw reply

* Re: [PATCH tip/core/rcu 5/7] torture: Trace long read-side delays
From: Paul E. McKenney @ 2016-11-14 18:44 UTC (permalink / raw)
  To: Josh Triplett
  Cc: linux-kernel, mingo, jiangshanlai, dipankar, akpm,
	mathieu.desnoyers, tglx, peterz, rostedt, dhowells, edumazet,
	dvhart, fweisbec, oleg, bobby.prani
In-Reply-To: <20161114172110.yybjdk6p6754nlih@jtriplet-mobl2.jf.intel.com>

On Mon, Nov 14, 2016 at 09:21:10AM -0800, Josh Triplett wrote:
> On Mon, Nov 14, 2016 at 08:57:11AM -0800, Paul E. McKenney wrote:
> > Although rcutorture will occasionally do a 50-millisecond grace-period
> > delay, these delays are quite rare.  And rightly so, because otherwise
> > the read rate would be quite low.  Thie means that it can be important
> > to identify whether or not a given run contained a long-delay read.
> > This commit therefore inserts a trace_rcu_torture_read() event to flag
> > runs containing long delays.
> > 
> > Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> 
> A couple of apparent typos below.  With those fixed:
> Reviewed-by: Josh Triplett <josh@joshtriplett.org>
> 
> >  include/trace/events/rcu.h |  5 ++++-
> >  kernel/rcu/rcutorture.c    | 11 ++++++++++-
> >  2 files changed, 14 insertions(+), 2 deletions(-)
> > 
> > diff --git a/include/trace/events/rcu.h b/include/trace/events/rcu.h
> > index d3e756539d44..b31e05bc8e26 100644
> > --- a/include/trace/events/rcu.h
> > +++ b/include/trace/events/rcu.h
> > @@ -698,7 +698,10 @@ TRACE_EVENT(rcu_batch_end,
> >  /*
> >   * Tracepoint for rcutorture readers.  The first argument is the name
> >   * of the RCU flavor from rcutorture's viewpoint and the second argument
> > - * is the callback address.
> > + * is the callback address.  The third callback is the start time in
> > + * seconds, and the last two arguments are the grace period numbers
> > + * and the beginning and end of the read, respectively.  Note that the
> > + * callback address can be NULL.
> 
> s/third callback/third argument/?

Good catch, fixed!

> Also, s/and the beginning/of the beginning/?

Let's see...  "the last two arguments are the grace period numbers and
the beginning and end of the read, respectively."  -ENONSENSE for sure.

I believe that the "and" needs to become "at" as follows:

"the last two arguments are the grace period numbers at the beginning
and end of the read, respectively."

Does that help?

							Thanx, Paul

> >  TRACE_EVENT(rcu_torture_read,
> >  
> > diff --git a/kernel/rcu/rcutorture.c b/kernel/rcu/rcutorture.c
> > index bf08fee53dc7..87c51225ceec 100644
> > --- a/kernel/rcu/rcutorture.c
> > +++ b/kernel/rcu/rcutorture.c
> > @@ -289,15 +289,24 @@ static int rcu_torture_read_lock(void) __acquires(RCU)
> >  
> >  static void rcu_read_delay(struct torture_random_state *rrsp)
> >  {
> > +	unsigned long started;
> > +	unsigned long completed;
> >  	const unsigned long shortdelay_us = 200;
> >  	const unsigned long longdelay_ms = 50;
> > +	unsigned long long ts;
> >  
> >  	/* We want a short delay sometimes to make a reader delay the grace
> >  	 * period, and we want a long delay occasionally to trigger
> >  	 * force_quiescent_state. */
> >  
> > -	if (!(torture_random(rrsp) % (nrealreaders * 2000 * longdelay_ms)))
> > +	if (!(torture_random(rrsp) % (nrealreaders * 2000 * longdelay_ms))) {
> > +		started = cur_ops->completed();
> > +		ts = rcu_trace_clock_local();
> >  		mdelay(longdelay_ms);
> > +		completed = cur_ops->completed();
> > +		do_trace_rcu_torture_read(cur_ops->name, NULL, ts,
> > +					  started, completed);
> > +	}
> >  	if (!(torture_random(rrsp) % (nrealreaders * 2 * shortdelay_us)))
> >  		udelay(shortdelay_us);
> >  #ifdef CONFIG_PREEMPT
> > -- 
> > 2.5.2
> > 
> 

^ permalink raw reply

* Re: [PATCH v2 2/2] of: changesets: Introduce changeset helper methods
From: Frank Rowand @ 2016-11-14 18:44 UTC (permalink / raw)
  To: Hans de Goede, Rob Herring, Pantelis Antoniou
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <b2cef3fb-cbb4-f34b-cb9a-84578bb67751-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

On 11/14/16 03:04, Hans de Goede wrote:
> Hi,
> 
> On 14-11-16 08:34, Frank Rowand wrote:
>> Hi Hans, Pantelis,
>>
>> On 11/12/16 18:15, Frank Rowand wrote:
>>> On 11/04/16 07:42, Hans de Goede wrote:
>>>> From: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
>>>>
>>>> Changesets are very powerful, but the lack of a helper API
>>>> makes using them cumbersome. Introduce a simple copy based
>>>> API that makes things considerably easier.
>>>>
>>>> To wit, adding a property using the raw API.
>>>>
>>>>     struct property *prop;
>>>>     prop = kzalloc(sizeof(*prop)), GFP_KERNEL);
>>>>     prop->name = kstrdup("compatible");
>>>>     prop->value = kstrdup("foo,bar");
>>>>     prop->length = strlen(prop->value) + 1;
>>>>     of_changeset_add_property(ocs, np, prop);
>>>>
>>>> while using the helper API
>>>>
>>>>     of_changeset_add_property_string(ocs, np, "compatible",
>>>>             "foo,bar");
>>>>
>>>> Signed-off-by: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
>>>> Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>>>> ---
>>>> Changes in v2 (hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org):
>>>> -Address review comments from:
>>>>  https://www.spinics.net/lists/kernel/msg2252845.html
>>>
>>> That points to the May 9 version 1 patches from Pantelis (as expected),
>>> but containing 4, not 2, patches.  Patch 1/4 was applied.  Patch 4/4
>>> seems to have disappeared?
>>>
>>> Pantelis then sent a version 2 set of the patches on May 16.
>>>
>>> Your version is a modification of the May 9 patches (as would be expected
>>> of a version 2).  It is confusing to have two different version 2 patch
>>> sets.  I don't have any brilliant ideas on how this patch set could have
>>> been named differently to avoid that confusion.
>>>
>>> The point of this little side-track is simply to note the existence of two
>>> different version 2 series so I won't be confused when I revisit this
>>> thread in the future.
>>>
>>>>  -Simplify (and fix) __of_changeset_add_update_property_copy OOM handling
>>>>  -Remove (by manual inlining) these 2 static helpers:
>>>>   __of_changeset_add_update_property_u32
>>>>   __of_changeset_add_update_property_bool
>>>>  -Remove the following exported helper method:
>>>>   of_changeset_node_move_to
>>>
>>> Not all comments were addressed.
>>>
>>> There are some other changes made that are not noted in the changelog.
>>>
>>> I am still reading through the patches. I will reply again either with
>>> a reviewed-by or specific comments when I finish.
>>
>> Replying here for the entire patchset (there was no patch 0 to reply to).
>>
>> After reading through the patches, my reply is meta instead of specific
>> comments about the code.
>>
>> There are very few users of change sets in tree.  I do not see the need to
>> add these helpers until such users are likely to appear.
>>
>> I would expect change sets to be _mostly_ used internally by the device tree
>> overlay framework, not directly by drivers.  If change sets are an attractive
>> technology for drivers, I want to approach that usage very carefully to avoid
>> inappropriate use, which could be very difficult to reign in after the fact.
>>
>> Even if helpers should be added, this seems to be an overly complex approach.
>> If the need for these helpers becomes apparent I can provide review comments
>> with the specifics about how it appears to be overly complex.
>>
>> Can you please  provide some more insights into the needs driving the desire
>> to have change set helpers and the expected use cases of them?  Please put
>> your architect's hat on when replying to this question.
> 
> My use case for this is discussed in this thread:
> https://www.spinics.net/lists/arm-kernel/msg536111.html
> 
> With the dt-bindings for the hardware-manager I want to add here:
> https://www.spinics.net/lists/arm-kernel/msg536109.html
> 
> Note that there is a lot of discussion in this thread whether or
> not this belongs in the kernel. I strongly believe though that
> some functionality like this will be needed in the kernel for
> ARM+dt devices going forward, just like there is plenty of x86
> code which adjusts itself to specific hardware, because whether
> we like it or not hardware (revisions) will always have quirks.

Thanks! That context should have been provided with the patches.

The use case discussion is important and I am paying a lot of
attention to that discussion and many other discussions about
dynamic device trees.  I don't think it makes sense to apply the
change set helper patches yet, given the unsettled state of the
various dynamic device tree discussions.

-Frank
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH net 2/3] bpf, mlx5: fix various refcount/prog issues in mlx5e_xdp_set
From: Saeed Mahameed @ 2016-11-14 18:27 UTC (permalink / raw)
  To: Daniel Borkmann, davem
  Cc: alexei.starovoitov, bblanco, tariqt, zhiyisun, ranas, netdev
In-Reply-To: <03741f7075af64e83d23add379bdab41204396b0.1479080215.git.daniel@iogearbox.net>



On 11/14/2016 02:43 AM, Daniel Borkmann wrote:
> There are multiple issues in mlx5e_xdp_set():
> 
> 1) prog can be NULL, so calling unconditionally into bpf_prog_add(prog,
>    priv->params.num_channels) can end badly.

not correct, if prog is null we will never get to bpf_prog_add:

        reset = (!priv->xdp_prog || !prog);
        [...]
	if (!test_bit(MLX5E_STATE_OPENED, &priv->state) || reset)
		goto unlock;
        bpf_prog_add...
         

> 
> 2) The batched bpf_prog_add() should be done at an earlier point in
>    time. This makes sure that we cannot fail anymore at the time we
>    want to set the program for each channel. This only means that we
>    have to undo the bpf_prog_add() in case we return early due to
>    reset or device not in MLX5E_STATE_OPENED yet. Note, err is 0 here.
> 

It is delayed for a reason, we do delayed batched bpf_prog_add() 
only when reset is not required (exchanging prog/old_prg) when both prog and old_prog are not null,
which means the only thing that could fail in this case is bpf_prog_add.

so i don't see any reason for changing the logic, checking for  bpf_prog_add return value would be sufficient.

Sorry I need to go for now, I will continue reviewing this patch later.  but this patch looks a little bit exaggerated.

> 3) When swapping the priv->xdp_prog, then no extra reference count must
>    be taken since we got that from call path via dev_change_xdp_fd()
>    already. Otherwise, we'd never be able to free the program. Also,
>    bpf_prog_add() without checking the return code could fail.
> 
> Fixes: 86994156c736 ("net/mlx5e: XDP fast RX drop bpf programs support")
> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
> ---
>  drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 25 ++++++++++++++++++-----
>  include/linux/bpf.h                               |  5 +++++
>  kernel/bpf/syscall.c                              | 11 ++++++++++
>  3 files changed, 36 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> index 2b83667..c90610a 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> @@ -3125,6 +3125,17 @@ static int mlx5e_xdp_set(struct net_device *netdev, struct bpf_prog *prog)
>  		goto unlock;
>  	}
>  
> +	if (prog) {
> +		/* num_channels is invariant here, so we can take the
> +		 * batched reference right upfront.
> +		 */
> +		prog = bpf_prog_add(prog, priv->params.num_channels);
> +		if (IS_ERR(prog)) {
> +			err = PTR_ERR(prog);
> +			goto unlock;
> +		}
> +	}
> +
>  	was_opened = test_bit(MLX5E_STATE_OPENED, &priv->state);
>  	/* no need for full reset when exchanging programs */
>  	reset = (!priv->xdp_prog || !prog);
> @@ -3132,10 +3143,10 @@ static int mlx5e_xdp_set(struct net_device *netdev, struct bpf_prog *prog)
>  	if (was_opened && reset)
>  		mlx5e_close_locked(netdev);
>  
> -	/* exchange programs */
> +	/* exchange programs, extra prog reference we got from caller
> +	 * as long as we don't fail from this point onwards.
> +	 */
>  	old_prog = xchg(&priv->xdp_prog, prog);
> -	if (prog)
> -		bpf_prog_add(prog, 1);
>  	if (old_prog)
>  		bpf_prog_put(old_prog);
>  
> @@ -3146,12 +3157,11 @@ static int mlx5e_xdp_set(struct net_device *netdev, struct bpf_prog *prog)
>  		mlx5e_open_locked(netdev);
>  
>  	if (!test_bit(MLX5E_STATE_OPENED, &priv->state) || reset)
> -		goto unlock;
> +		goto unlock_put;
>  
>  	/* exchanging programs w/o reset, we update ref counts on behalf
>  	 * of the channels RQs here.
>  	 */
> -	bpf_prog_add(prog, priv->params.num_channels);
>  	for (i = 0; i < priv->params.num_channels; i++) {
>  		struct mlx5e_channel *c = priv->channel[i];
>  
> @@ -3173,6 +3183,11 @@ static int mlx5e_xdp_set(struct net_device *netdev, struct bpf_prog *prog)
>  unlock:
>  	mutex_unlock(&priv->state_lock);
>  	return err;
> +unlock_put:
> +	/* reference on priv->xdp_prog is still held at this point */
> +	if (prog)
> +		bpf_prog_sub(prog, priv->params.num_channels);
> +	goto unlock;
>  }
>  
>  static bool mlx5e_xdp_attached(struct net_device *dev)
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index c201017..ca495fd 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -234,6 +234,7 @@ u64 bpf_event_output(struct bpf_map *map, u64 flags, void *meta, u64 meta_size,
>  struct bpf_prog *bpf_prog_get(u32 ufd);
>  struct bpf_prog *bpf_prog_get_type(u32 ufd, enum bpf_prog_type type);
>  struct bpf_prog *bpf_prog_add(struct bpf_prog *prog, int i);
> +void bpf_prog_sub(struct bpf_prog *prog, int i);
>  struct bpf_prog *bpf_prog_inc(struct bpf_prog *prog);
>  void bpf_prog_put(struct bpf_prog *prog);
>  
> @@ -303,6 +304,10 @@ static inline struct bpf_prog *bpf_prog_add(struct bpf_prog *prog, int i)
>  	return ERR_PTR(-EOPNOTSUPP);
>  }
>  
> +static inline void bpf_prog_sub(struct bpf_prog *prog, int i)
> +{
> +}
> +
>  static inline void bpf_prog_put(struct bpf_prog *prog)
>  {
>  }
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 751e806..a0fca9f 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -682,6 +682,17 @@ struct bpf_prog *bpf_prog_add(struct bpf_prog *prog, int i)
>  }
>  EXPORT_SYMBOL_GPL(bpf_prog_add);
>  
> +void bpf_prog_sub(struct bpf_prog *prog, int i)
> +{
> +	/* Only to be used for undoing previous bpf_prog_add() in some
> +	 * error path. We still know that another entity in our call
> +	 * path holds a reference to the program, thus atomic_sub() can
> +	 * be safely used in such cases!
> +	 */
> +	WARN_ON(atomic_sub_return(i, &prog->aux->refcnt) == 0);
> +}
> +EXPORT_SYMBOL_GPL(bpf_prog_sub);
> +
>  struct bpf_prog *bpf_prog_inc(struct bpf_prog *prog)
>  {
>  	return bpf_prog_add(prog, 1);
> 

^ permalink raw reply

* Re: Announcing btrfs-dedupe
From: Zygo Blaxell @ 2016-11-14 18:43 UTC (permalink / raw)
  To: James Pharaoh; +Cc: Mark Fasheh, linux-btrfs
In-Reply-To: <a4e42724-d84a-e4f9-a3ea-d1fec31a5629@wellbehavedsoftware.com>

[-- Attachment #1: Type: text/plain, Size: 2736 bytes --]

On Mon, Nov 14, 2016 at 07:22:59PM +0100, James Pharaoh wrote:
> On 14/11/16 19:07, Zygo Blaxell wrote:
> >There is also a still-unresolved problem where the filesystem CPU usage
> >rises exponentially for some operations depending on the number of shared
> >references to an extent.  Files which contain blocks with more than a few
> >thousand shared references can trigger this problem.  A file over 1TB can
> >keep the kernel busy at 100% CPU for over 40 minutes at a time.
> 
> Yes, I see this all the time. For my use cases, I don't really care about
> "shared references" as blocks of files, but am happy to simply deduplicate
> at the whole-file level. I wonder if this still will have the same effect,
> however. I guess that this could be mitigated in a tool, but this is going
> to be both annoying and not the most elegant solution.

If you have huge files (1TB+) this can be a problem even with whole-file
deduplications (which are really just extent-level deduplications applied
to the entire file).  The CPU time is a product of file size and extent
reference count with some other multipliers on top.

I've hacked around it by timing how long it takes to manipulate the data,
and blacklisting any hash value or block address that takes more than
10 seconds to process (if such a block is found after blacklisting, just
skip processing the block/extent/file entirely).  It turns out there are
very few of these in practice (only a few hundred per TB) but these few
hundred block hash values occur millions of times in a large data corpus.

> One thing I am keen to understand is if BTRFS will automatically ignore a
> request to deduplicate a file if it is already deduplicated? Given the
> performance I see when doing a repeat deduplication, it seems to me that it
> can't be doing so, although this could be caused by the CPU usage you
> mention above.

As far as I can tell btrfs doesn't do anything different in this
case--it'll happily repeat the entire lock/read/compare/delete/insert
sequence even if the outcome cannot be different from the initial
conditions.  Due to limitations of VFS caching it'll read the same blocks
from storage hardware twice, too.

> In any case, I'm considering some digging into the filesystem structures to
> see if I can work this out myself before i do any deduplication. I'm fairly
> sure this should be relatively simple to work out, at least well enough for
> my purposes.

I used FIEMAP (then later replaced it with SEARCH_V2 for speed) to map
the extents to physical addresses before deduping them.  If you're only
going to do whole-file dedup then you only need to care about the physical
address of the first non-hole extent.


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

^ permalink raw reply


This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.