date:20170331

Hi zhichang.yuan,

[auto build test ERROR on linus/master]
[also build test ERROR on v4.11-rc4 next-20170331]
[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/zhichang-yuan/LIBIO-Introduce-a-generic-PIO-mapping-method/20170401-104801
config: m68k-m5475evb_defconfig (attached as .config)
compiler: m68k-linux-gcc (GCC) 4.9.0
reproduce:
wget 
https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=m68k 

All error/warnings (new ones prefixed by >>):

   lib/logic_pio.c:32:50: error: 'PIO_MAX_SECT' undeclared here (not in a 
function)
static struct logic_pio_root logic_pio_root_list[PIO_MAX_SECT] = {
 ^
   lib/logic_pio.c:52:3: error: 'PIO_CPU_MMIO' undeclared here (not in a 
function)
 [PIO_CPU_MMIO] = {
  ^
   lib/logic_pio.c:52:2: error: array index in initializer not of integer type
 [PIO_CPU_MMIO] = {
 ^
   lib/logic_pio.c:52:2: error: (near initialization for 'logic_pio_root_list')
   lib/logic_pio.c:53:3: error: field name not in record or union initializer
  .sec_head = LIST_HEAD_INIT(logic_pio_root_list[PIO_CPU_MMIO].sec_head),
  ^
   lib/logic_pio.c:53:3: error: (near initialization for 'logic_pio_root_list')
   lib/logic_pio.c:54:3: error: field name not in record or union initializer
  .sec_min = PIO_SECT_MIN(PIO_CPU_MMIO),
  ^
   lib/logic_pio.c:54:3: error: (near initialization for 'logic_pio_root_list')
   lib/logic_pio.c:54:3: error: implicit declaration of function 'PIO_SECT_MIN' 
[-Werror=implicit-function-declaration]
   lib/logic_pio.c:55:3: error: field name not in record or union initializer
  .sec_max = PIO_SECT_MAX(PIO_CPU_MMIO),
  ^
   lib/logic_pio.c:55:3: error: (near initialization for 'logic_pio_root_list')
   lib/logic_pio.c:55:3: error: implicit declaration of function 'PIO_SECT_MAX' 
[-Werror=implicit-function-declaration]
   In file included from include/linux/list.h:8:0,
from include/linux/kobject.h:20,
from include/linux/of.h:21,
from lib/logic_pio.c:18:
   lib/logic_pio.c: In function 'logic_pio_find_range_byaddr':
>> include/linux/rculist.h:351:49: error: dereferencing pointer to incomplete 
>> type
 for (pos = list_entry_rcu((head)->next, typeof(*pos), member); \
^
   include/linux/kernel.h:852:18: note: in definition of macro 'container_of'
 const typeof( ((type *)0)->member ) *__mptr = (ptr); \
 ^
   include/linux/rculist.h:351:13: note: in expansion of macro 'list_entry_rcu'
 for (pos = list_entry_rcu((head)->next, typeof(*pos), member); \
^
   lib/logic_pio.c:77:2: note: in expansion of macro 'list_for_each_entry_rcu'
 list_for_each_entry_rcu(range, &io_range_list, list) {
 ^
   include/linux/kernel.h:852:48: warning: initialization from incompatible 
pointer type
 const typeof( ((type *)0)->member ) *__mptr = (ptr); \
   ^
   include/linux/rculist.h:277:2: note: in expansion of macro 'container_of'
 container_of(lockless_dereference(ptr), type, member)
 ^
   include/linux/rculist.h:351:13: note: in expansion of macro 'list_entry_rcu'
 for (pos = list_entry_rcu((head)->next, typeof(*pos), member); \
^
   lib/logic_pio.c:77:2: note: in expansion of macro 'list_for_each_entry_rcu'
 list_for_each_entry_rcu(range, &io_range_list, list) {
 ^
>> include/linux/rculist.h:351:49: error: dereferencing pointer to incomplete 
>> type
 for (pos = list_entry_rcu((head)->next, typeof(*pos), member); \
^
   include/linux/kernel.h:853:3: note: in definition of macro 'container_of'
 (type *)( (char *)__mptr - offsetof(type,member) );})
  ^
   include/linux/rculist.h:351:13: note: in expansion of macro 'list_entry_rcu'
 for (pos = list_entry_rcu((head)->next, typeof(*pos), member); \
^
   lib/logic_pio.c:77:2: note: in expansion of macro 'list_for_each_entry_rcu'
 list_for_each_entry_rcu(range, &io_range_list, list) {
 ^
   In file included from include/linux/compiler.h:62:0,
from include/uapi/linux/stddef.h:1,
from include/linux/stddef.h:4,
from include/uapi/linux/posix_types.h:4,
from include/uapi/linux/types.h:13,
from include/linux/types.h:5,

Re: [PATCH v3 0/3] perf/sdt: Hardening argument support

2017-03-31 Thread Ravi Bangoria

On Tuesday 28 March 2017 09:10 PM, Arnaldo Carvalho de Melo wrote:
> Em Wed, Mar 29, 2017 at 12:29:29AM +0900, Masami Hiramatsu escreveu:
>> Hi Arnaldo,
>>
>> please pull this, I've already acked to this series.
> I did it 15 minutes ago, running build tests on it now.

Hi Arnaldo,

Thanks for pulling patches.

Looking at your pull request, https://lkml.org/lkml/2017/3/31/789, I think
you missed to include 3rd patch. Can you please also take that.

Let me know if you have any issues with 3rd patch.

Ravi

Re: [PATCH V8 1/6] LIBIO: Introduce a generic PIO mapping method

Hi zhichang.yuan,

[auto build test ERROR on linus/master]
[also build test ERROR on v4.11-rc4 next-20170331]
[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/zhichang-yuan/LIBIO-Introduce-a-generic-PIO-mapping-method/20170401-104801
config: alpha-allyesconfig (attached as .config)
compiler: alpha-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget 
https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=alpha 

All error/warnings (new ones prefixed by >>):

>> lib/logic_pio.c:32:50: error: 'PIO_MAX_SECT' undeclared here (not in a 
>> function)
static struct logic_pio_root logic_pio_root_list[PIO_MAX_SECT] = {
 ^~~~
>> lib/logic_pio.c:39:3: error: 'PIO_CPU_MMIO' undeclared here (not in a 
>> function)
 [PIO_CPU_MMIO ... PIO_INDIRECT - 1] = {
  ^~~~
>> lib/logic_pio.c:39:20: error: 'PIO_INDIRECT' undeclared here (not in a 
>> function)
 [PIO_CPU_MMIO ... PIO_INDIRECT - 1] = {
   ^~~~
>> lib/logic_pio.c:39:3: error: array index in initializer not of integer type
 [PIO_CPU_MMIO ... PIO_INDIRECT - 1] = {
  ^~~~
   lib/logic_pio.c:39:3: note: (near initialization for 'logic_pio_root_list')
>> lib/logic_pio.c:40:3: error: field name not in record or union initializer
  .sec_head = LIST_HEAD_INIT(logic_pio_root_list[PIO_CPU_MMIO].sec_head),
  ^
   lib/logic_pio.c:40:3: note: (near initialization for 'logic_pio_root_list')
   lib/logic_pio.c:41:3: error: field name not in record or union initializer
  .sec_min = PIO_SECT_MIN(PIO_CPU_MMIO),
  ^
   lib/logic_pio.c:41:3: note: (near initialization for 'logic_pio_root_list')
>> lib/logic_pio.c:41:14: error: implicit declaration of function 
>> 'PIO_SECT_MIN' [-Werror=implicit-function-declaration]
  .sec_min = PIO_SECT_MIN(PIO_CPU_MMIO),
 ^~~~
   lib/logic_pio.c:42:3: error: field name not in record or union initializer
  .sec_max = PIO_SECT_MAX(PIO_INDIRECT - 1),
  ^
   lib/logic_pio.c:42:3: note: (near initialization for 'logic_pio_root_list')
>> lib/logic_pio.c:42:14: error: implicit declaration of function 
>> 'PIO_SECT_MAX' [-Werror=implicit-function-declaration]
  .sec_max = PIO_SECT_MAX(PIO_INDIRECT - 1),
 ^~~~
   lib/logic_pio.c:46:3: error: array index in initializer not of integer type
 [PIO_INDIRECT] = {
  ^~~~
   lib/logic_pio.c:46:3: note: (near initialization for 'logic_pio_root_list')
   lib/logic_pio.c:47:3: error: field name not in record or union initializer
  .sec_head = LIST_HEAD_INIT(logic_pio_root_list[PIO_INDIRECT].sec_head),
  ^
   lib/logic_pio.c:47:3: note: (near initialization for 'logic_pio_root_list')
   lib/logic_pio.c:48:3: error: field name not in record or union initializer
  .sec_min = PIO_SECT_MIN(PIO_INDIRECT),
  ^
   lib/logic_pio.c:48:3: note: (near initialization for 'logic_pio_root_list')
   lib/logic_pio.c:49:3: error: field name not in record or union initializer
  .sec_max = PIO_SECT_MAX(PIO_INDIRECT),
  ^
   lib/logic_pio.c:49:3: note: (near initialization for 'logic_pio_root_list')
   In file included from include/linux/list.h:8:0,
from include/linux/kobject.h:20,
from include/linux/of.h:21,
from lib/logic_pio.c:18:
   lib/logic_pio.c: In function 'logic_pio_find_range_byaddr':
>> include/linux/rculist.h:351:49: error: dereferencing pointer to incomplete 
>> type 'struct logic_pio_hwaddr'
 for (pos = list_entry_rcu((head)->next, typeof(*pos), member); \
 
   include/linux/kernel.h:852:18: note: in definition of macro 'container_of'
 const typeof( ((type *)0)->member ) *__mptr = (ptr); \
 ^~~~
>> include/linux/rculist.h:351:13: note: in expansion of macro 'list_entry_rcu'
 for (pos = list_entry_rcu((head)->next, typeof(*pos), member); \
^~
>> lib/logic_pio.c:77:2: note: in expansion of macro 'list_for_each_entry_rcu'
 list_for_each_entry_rcu(range, &io_range_list, list) {
 ^~~
   include/linux/kernel.h:852:48: error: initialization from incompatible 
pointer type [-Werror=incompatible-pointer-types]
 const typeof( ((type *)0)->member ) *__mptr = (ptr); \
   ^
&

[RFC][PATCH 2/2] exec: If possible don't wait for ptraced threads to be reaped

2017-03-31 Thread Eric W. Biederman


Take advantage of the situation when sighand->count == 1 to only wait
for threads to reach EXIT_ZOMBIE instead of EXIT_DEAD in de_thread.
Only old old linux threading libraries use CLONE_SIGHAND without
CLONE_THREAD.  So this situation should be present most of the time.

This allows ptracing through a multi-threaded exec without the danger
of stalling the exec.  As historically exec waits for the other
threads to be reaped in de_thread before completing.  This is
necessary as it is not safe to unshare the sighand_struct until all of
the other threads in this thread group are reaped, because the lock to
serialize threads in a thread group siglock lives in sighand_struct.

When oldsighand->count == 1 we know that there are no other
users and unsharing the sighand struct in exec is pointless.
This makes it safe to only wait for threads to become zombies
as the siglock won't change during exec and release_task
will use the samve siglock for the old threads as for
the new threads.

Cc: sta...@vger.kernel.org
Signed-off-by: "Eric W. Biederman" 
---
 fs/exec.c| 15 ++-
 include/linux/sched/signal.h |  2 +-
 kernel/exit.c| 13 +
 kernel/signal.c  |  8 ++--
 4 files changed, 26 insertions(+), 12 deletions(-)

diff --git a/fs/exec.c b/fs/exec.c
index 65145a3df065..0fd29342bbe4 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1052,6 +1052,7 @@ static int de_thread(struct task_struct *tsk)
struct signal_struct *sig = tsk->signal;
struct sighand_struct *oldsighand = tsk->sighand;
spinlock_t *lock = &oldsighand->siglock;
+   bool may_hang;
 
if (thread_group_empty(tsk))
goto no_thread_group;
@@ -1069,9 +1070,10 @@ static int de_thread(struct task_struct *tsk)
return -EAGAIN;
}
 
+   may_hang = atomic_read(&oldsighand->count) != 1;
sig->group_exit_task = tsk;
-   sig->notify_count = zap_other_threads(tsk);
-   if (!thread_group_leader(tsk))
+   sig->notify_count = zap_other_threads(tsk, may_hang ? 1 : -1);
+   if (may_hang && !thread_group_leader(tsk))
sig->notify_count--;
 
while (sig->notify_count) {
@@ -1092,9 +1094,10 @@ static int de_thread(struct task_struct *tsk)
if (!thread_group_leader(tsk)) {
struct task_struct *leader = tsk->group_leader;
 
-   for (;;) {
-   cgroup_threadgroup_change_begin(tsk);
-   write_lock_irq(&tasklist_lock);
+   cgroup_threadgroup_change_begin(tsk);
+   write_lock_irq(&tasklist_lock);
+
+   for (;may_hang;) {
/*
 * Do this under tasklist_lock to ensure that
 * exit_notify() can't miss ->group_exit_task
@@ -1108,6 +,8 @@ static int de_thread(struct task_struct *tsk)
schedule();
if (unlikely(__fatal_signal_pending(tsk)))
goto killed;
+   cgroup_threadgroup_change_begin(tsk);
+   write_lock_irq(&tasklist_lock);
}
 
/*
diff --git a/include/linux/sched/signal.h b/include/linux/sched/signal.h
index 2cf446704cd4..187a9e980d3a 100644
--- a/include/linux/sched/signal.h
+++ b/include/linux/sched/signal.h
@@ -298,7 +298,7 @@ extern __must_check bool do_notify_parent(struct 
task_struct *, int);
 extern void __wake_up_parent(struct task_struct *p, struct task_struct 
*parent);
 extern void force_sig(int, struct task_struct *);
 extern int send_sig(int, struct task_struct *, int);
-extern int zap_other_threads(struct task_struct *p);
+extern int zap_other_threads(struct task_struct *p, int do_count);
 extern struct sigqueue *sigqueue_alloc(void);
 extern void sigqueue_free(struct sigqueue *);
 extern int send_sigqueue(struct sigqueue *,  struct task_struct *, int group);
diff --git a/kernel/exit.c b/kernel/exit.c
index 8c5b3e106298..972df5ebf79f 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -712,6 +712,8 @@ static void forget_original_parent(struct task_struct 
*father,
  */
 static void exit_notify(struct task_struct *tsk, int group_dead)
 {
+   struct sighand_struct *sighand = tsk->sighand;
+   struct signal_struct *signal = tsk->signal;
bool autoreap;
struct task_struct *p, *n;
LIST_HEAD(dead);
@@ -739,9 +741,12 @@ static void exit_notify(struct task_struct *tsk, int 
group_dead)
if (tsk->exit_state == EXIT_DEAD)
list_add(&tsk->ptrace_entry, &dead);
 
-   /* mt-exec, de_thread() is waiting for group leader */
-   if (unlikely(tsk->signal->notify_count < 0))
-   wake_up_process(tsk->signal->group_exit_task);
+   spin_lock(&sighand->siglock);
+   /* mt-exec, de_thread is waiting for threads to exit */
+   if (signal->notify_count < 0 && !++signal->notify_count)
+   wak

[RFC][PATCH 1/2] sighand: Count each thread group once in sighand_struct

2017-03-31 Thread Eric W. Biederman


In practice either a thread group is either using a sighand_struct or
it isn't.  Therefore simplify things a bit and only increment the
count in sighand_struct when a new thread group is created that uses
the existing sighand_struct, and only decrement the count in
sighand_struct when a thread group exits.

As well as standing on it's own merits this has the potential to simply
de_thread.

Signed-off-by: "Eric W. Biederman" 
---
 kernel/exit.c | 2 +-
 kernel/fork.c | 6 --
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/kernel/exit.c b/kernel/exit.c
index e126ebf2400c..8c5b3e106298 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -163,9 +163,9 @@ static void __exit_signal(struct task_struct *tsk)
tsk->sighand = NULL;
spin_unlock(&sighand->siglock);
 
-   __cleanup_sighand(sighand);
clear_tsk_thread_flag(tsk, TIF_SIGPENDING);
if (group_dead) {
+   __cleanup_sighand(sighand);
flush_sigqueue(&sig->shared_pending);
tty_kref_put(tty);
}
diff --git a/kernel/fork.c b/kernel/fork.c
index 6c463c80e93d..fe6f1bf32bb9 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -1295,7 +1295,8 @@ static int copy_sighand(unsigned long clone_flags, struct 
task_struct *tsk)
struct sighand_struct *sig;
 
if (clone_flags & CLONE_SIGHAND) {
-   atomic_inc(¤t->sighand->count);
+   if (!(clone_flags & CLONE_THREAD))
+   atomic_inc(¤t->sighand->count);
return 0;
}
sig = kmem_cache_alloc(sighand_cachep, GFP_KERNEL);
@@ -1896,7 +1897,8 @@ static __latent_entropy struct task_struct *copy_process(
if (!(clone_flags & CLONE_THREAD))
free_signal_struct(p->signal);
 bad_fork_cleanup_sighand:
-   __cleanup_sighand(p->sighand);
+   if (!(clone_flags & CLONE_THREAD))
+   __cleanup_sighand(p->sighand);
 bad_fork_cleanup_fs:
exit_fs(p); /* blocking */
 bad_fork_cleanup_files:
-- 
2.10.1

[RFC][PATCH 0/2] exec: Fixing ptrace'd mulit-threaded hang

2017-03-31 Thread Eric W. Biederman


I spent a little more time with this and only waiting until the killed
thread are zombies (and not reaped as we do today) really looks like
the right fix.

Oleg the following two patches work on top of your PTRACE_EVENT_EXIT
change and probably need a little more cleanup until they are ready
for serious posting.

That said I want to I want to post the code so I have a change at
some feedback before I prepare the final round of patches.

These patches only handle the case when sighand_struct is not
shared between different multi-threaded processes.  The general
case is solvable but that is a quite a bit more code.

Eric W. Biederman (2):
  sighand: Count each thread group once in sighand_struct
  exec: If possible don't wait for ptraced threads to be reaped

 fs/exec.c| 15 ++-
 include/linux/sched/signal.h |  2 +-
 kernel/exit.c| 15 ++-
 kernel/fork.c|  6 --
 kernel/signal.c  |  8 ++--
 5 files changed, 31 insertions(+), 15 deletions(-)

Eric

Re: [PATCH V2 net-next 1/7] ptr_ring: introduce batch dequeuing

2017-03-31 Thread Jason Wang




On 2017年03月31日 22:31, Michael S. Tsirkin wrote:

On Fri, Mar 31, 2017 at 11:52:24AM +0800, Jason Wang wrote:

On 2017年03月30日 21:53, Michael S. Tsirkin wrote:

On Thu, Mar 30, 2017 at 03:22:24PM +0800, Jason Wang wrote:

This patch introduce a batched version of consuming, consumer can
dequeue more than one pointers from the ring at a time. We don't care
about the reorder of reading here so no need for compiler barrier.

Signed-off-by: Jason Wang
---
   include/linux/ptr_ring.h | 65 

   1 file changed, 65 insertions(+)

diff --git a/include/linux/ptr_ring.h b/include/linux/ptr_ring.h
index 6c70444..2be0f350 100644
--- a/include/linux/ptr_ring.h
+++ b/include/linux/ptr_ring.h
@@ -247,6 +247,22 @@ static inline void *__ptr_ring_consume(struct ptr_ring *r)
return ptr;
   }
+static inline int __ptr_ring_consume_batched(struct ptr_ring *r,
+void **array, int n)

Can we use a shorter name? ptr_ring_consume_batch?

Ok, but at least we need to keep the prefix since there's a locked version.




+{
+   void *ptr;
+   int i;
+
+   for (i = 0; i < n; i++) {
+   ptr = __ptr_ring_consume(r);
+   if (!ptr)
+   break;
+   array[i] = ptr;
+   }
+
+   return i;
+}
+
   /*
* Note: resize (below) nests producer lock within consumer lock, so if you
* call this in interrupt or BH context, you must disable interrupts/BH when

I'd like to add a code comment here explaining why we don't
care about cpu or compiler reordering. And I think the reason is
in the way you use this API: in vhost it does not matter
if you get less entries than present in the ring.
That's ok but needs to be noted
in a code comment so people use this function correctly.

Interesting, but I still think it's not necessary.

If consumer is doing a busy polling, it will eventually get the entries. If
the consumer need notification from producer, it should drain the queue
which means it need enable notification before last try of consuming call,
otherwise it was a bug. The batch consuming function in this patch can
guarantee return at least one pointer if there's many, this looks sufficient
for the correctness?

Thanks

You ask for N entries but get N-1. This seems to imply the
ring is now empty. Do we guarantee this?


I think consumer can not assume ring is empty consider producer can 
produce at the same time. It need enable notification and do another 
poll in this case.


Thanks

Re: [PATCH -v2 1/2] mm, swap: Use kvzalloc to allocate some swap data structure

Hi, Michal,

Michal Hocko  writes:

> On Fri 24-03-17 06:56:10, Dave Hansen wrote:
>> On 03/24/2017 12:33 AM, John Hubbard wrote:
>> > There might be some additional information you are using to come up with
>> > that conclusion, that is not obvious to me. Any thoughts there? These
>> > calls use the same underlying page allocator (and I thought that both
>> > were subject to the same constraints on defragmentation, as a result of
>> > that). So I am not seeing any way that kmalloc could possibly be a
>> > less-fragmenting call than vmalloc.
>> 
>> You guys are having quite a discussion over a very small point.
>> 
>> But, Ying is right.
>> 
>> Let's say we have a two-page data structure.  vmalloc() takes two
>> effectively random order-0 pages, probably from two different 2M pages
>> and pins them.  That "kills" two 2M pages.
>> 
>> kmalloc(), allocating two *contiguous* pages, is very unlikely to cross
>> a 2M boundary (it theoretically could).  That means it will only "kill"
>> the possibility of a single 2M page.  More 2M pages == less fragmentation.
>
> Yes I agree with this. And the patch is no brainer. kvmalloc makes sure
> to not try too hard on the kmalloc side so I really didn't get the
> objection about direct compaction and reclaim which initially started
> this discussion. Besides that the swapon path usually happens early
> during the boot where we should have those larger blocks available.

Could I add your Acked-by for this patch?

Best Regards,
Huang, Ying

Re: [PATCH v13 3/6] mmc: cavium: Add MMC PCI driver for ThunderX SOCs

Hi Jan,

[auto build test ERROR on linus/master]
[also build test ERROR on v4.11-rc4 next-20170331]
[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/Jan-Glauber/Cavium-MMC-driver/20170401-055302
config: sparc64-allmodconfig (attached as .config)
compiler: sparc64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget 
https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=sparc64 

All errors (new ones prefixed by >>):

>> ERROR: "of_platform_device_create" [drivers/mmc/host/thunderx-mmc.ko] 
>> undefined!

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


.config.gz
Description: application/gzip

Re: [PATCH 001/001] drivers/staging/vt6656/main_usb.c: checkpatch warning

On Sat, Apr 01, 2017 at 04:32:39AM +0100, Al Viro wrote:
> On Fri, Mar 31, 2017 at 06:59:19PM -0700, Chewie Lin wrote:
> > Replace string with formatted arguments in the dev_warn() call. It removes
> > the checkpatch warning:
> > 
> > WARNING: Prefer using "%s", __func__ to embedded function names
> > #417: FILE: main_usb.c:417:
> > +"usb_device_reset fail status=%d\n", status);
> > 
> > total: 0 errors, 1 warnings, 1058 lines checked
> > 
> > And after fix:
> > 
> > main_usb.c has no obvious style problems and is ready for submission.
> 
> 
> > -"usb_device_reset fail status=%d\n", status);
> > +"%s=%d\n", "usb_device_reset fail status", status);
> 
> Would you mind explaining the meaning of that warning?  Incidentally, why not
> "%se_reset fail status=%d\n", "usb_devic", status);
> - it would produce the identical output and silences checkpatch even more
> reliably.  Discuss.
> 
> Sarcasm aside, when you are proposing a change, there are several questions 
> you
> really must ask yourself and be able to answer.  First and foremost, of 
> course,
> is
>   * Why is it an improvement?
> If the answer to that was "it makes a warning go away", the next questions are
>   * What are we warned about?
>   * What is problematic in the original variant?
>   * Is the replacement free from the problem we had in the original?
> and if the answer to any of that is "I don't know", the next step is _not_
> "send it anyway".  "Try to figure out" might be a good idea, with usual
> heuristics regarding the reading comprehension ("if a sentence remains
> a mystery, see if there are any unfamiliar words skipped at the first reading
> and find what they mean", etc.) applying.
> 
> In this particular case the part you've apparently skipped was, indeed,
> critical - "__func__".  I am not trying to defend the quality of checkpatch -
> the warning is badly worded, the tool is badly written and more often than
> not its suggestions reflect nothing but authors' arbitrary preferences.
> 
> This one, however, does have some rationale behind it.  Namely, if some
> debugging output contains the name of a function that has produced it,
> having that name spelled out in format string is not a good idea.  If
> that code gets moved elsewhere one would have to replace the name in format
> string or face confusing messages refering to the function where that
> code used to be.  Since __func__ is interpreted as a constant string
> initialized with the name of the function it's used in, it is possible
> to avoid spelling the function name out - instead of
> "my_function: pink elephants; reduce the daily intake to %d glasses\n", n
> one could use
> "%s: pink elephants; reduce the daily intake to %d glasses\n", __func__, n
> producing the same output and avoiding the need to adjust anything when
> the whole thing is moved to another function.  It's not particularly big
> deal, but it's a more or less reasonable suggestion.
> 
> Your change, of course, has achieved only one thing - it has moved the
> explicitly spelled out function name out of format string.  It's still
> just as explicitly spelled out, still would need to be adjusted if moved
> to another function and the whole thing has become harder to read and
> understand since you've buried other parts of message in the place where
> it's harder to follow.
> 
> Again, checkpatch warning is badly written, but the main problem with
> your posting is not that you had been confused by checkpatch - it's
> that you have posted it based on an incomprehensible message with no better
> rationale than "it shuts checkpatch up, dunno why and what about".

Al, brilliant, that's exactly what I was trying to do on my first try. 
The checkpatch *is* confusing. It's fine with a simple string but doesn't 
like it when it's formatted string. From what you said, I 
think this may work better and more portable: 

"%s: fail status = %d\n", "usb_device_reset", status)

Re: [PATCH 001/001] drivers/staging/vt6656/main_usb.c: checkpatch warning

On Sat, 2017-04-01 at 05:08 +0100, Al Viro wrote:
> On Fri, Mar 31, 2017 at 08:52:50PM -0700, Joe Perches wrote:
> 
> > > MILD SUGGESTION: don't spell the function name out in format strings;
> > >   "this_function: foo is %d", n
> > > might be better off as
> > >   "%s: foo is %d", __func__, n
> > > in case you ever move it to another function or rename your function.
> > 
> > Thank you sir, may I have another.
> > 
> > checkpatch messages are single line.
> 
> Too bad... Incidentally, being able to get more detailed explanation of
> a warning might be a serious improvement, especially if it contains
> the rationale.  Hell, something like TeX handling of errors might be
> a good idea - warning printed, offered actions include 'give more help',
> 'continue', 'exit', 'from now on suppress this kind of warning', 'from
> now on just dump this kind of warning into log and keep going', 'from
> now on dump all warnings into log and keep going'.

Well, there is the possibility to have longer messages.
It's just the --terse option has to be somewhat sensible.

> And yes, I'm serious about having something like "mild suggestion" as
> possible severity - people are using that thing to look for potential
> improvements to make and 'such and such change might be useful for such
> and such reasons' is a lot more useful than 'this needs to be thus because
> it must be thus or I'll keep warning'.

I agree about checkpatch and ERROR/WARNING/CHECK vs some other wording.

https://lkml.org/lkml/2016/8/27/180
https://lkml.org/lkml/2015/7/16/568

The other thing that might help is for people to take
the warnings the script produces less seriously.

Maybe convert:

ERROR -> defect
WARNING -> unstylish
CHECK -> nitpick

or some such.

Re: [PATCH 2/2] ARM: dts: imx7: add USDHC NAND clock to SDHC instances

2017-03-31 Thread Stefan Agner

On 2017-03-31 20:03, Dong Aisheng wrote:
> On Wed, Mar 29, 2017 at 05:50:29PM -0700, Stefan Agner wrote:
>> The USDHC instances need the USDHC NAND clock in order to operate.
>> Add the clock as ahb bus clock.
>>
>> Signed-off-by: Stefan Agner 
>> ---
>>  arch/arm/boot/dts/imx7s.dtsi | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/arm/boot/dts/imx7s.dtsi b/arch/arm/boot/dts/imx7s.dtsi
>> index 5d3a43b8de20..5794febb19a4 100644
>> --- a/arch/arm/boot/dts/imx7s.dtsi
>> +++ b/arch/arm/boot/dts/imx7s.dtsi
>> @@ -936,7 +936,7 @@
>>  reg = <0x30b4 0x1>;
>>  interrupts = ;
>>  clocks = <&clks IMX7D_CLK_DUMMY>,
> 
> Would you please change the left ipg dummy to IMX7D_IPG_ROOT_CLK as well?

IMX7D_IPG_ROOT_CLK is currently not a valid clock in upstream... So we
would have to add it to the clock driver first.

I guess we could/should add it anyway at one point? But probably also as
init on, just to make sure Linux does not disable it since it is
currently used by several IPs implicitly.

--
Stefan

> 
> Otherwise,
> 
> Acked-by: Dong Aisheng 
> 
> Regards
> Dong Aisheng
> 
>> -<&clks IMX7D_CLK_DUMMY>,
>> +<&clks IMX7D_NAND_USDHC_BUS_ROOT_CLK>,
>>  <&clks IMX7D_USDHC1_ROOT_CLK>;
>>  clock-names = "ipg", "ahb", "per";
>>  bus-width = <4>;
>> @@ -948,7 +948,7 @@
>>  reg = <0x30b5 0x1>;
>>  interrupts = ;
>>  clocks = <&clks IMX7D_CLK_DUMMY>,
>> -<&clks IMX7D_CLK_DUMMY>,
>> +<&clks IMX7D_NAND_USDHC_BUS_ROOT_CLK>,
>>  <&clks IMX7D_USDHC2_ROOT_CLK>;
>>  clock-names = "ipg", "ahb", "per";
>>  bus-width = <4>;
>> @@ -960,7 +960,7 @@
>>  reg = <0x30b6 0x1>;
>>  interrupts = ;
>>  clocks = <&clks IMX7D_CLK_DUMMY>,
>> -<&clks IMX7D_CLK_DUMMY>,
>> +<&clks IMX7D_NAND_USDHC_BUS_ROOT_CLK>,
>>  <&clks IMX7D_USDHC3_ROOT_CLK>;
>>  clock-names = "ipg", "ahb", "per";
>>  bus-width = <4>;
>> --
>> 2.12.1
>>

Re: [PATCH 001/001] drivers/staging/vt6656/main_usb.c: checkpatch warning

2017-03-31 Thread Al Viro

On Fri, Mar 31, 2017 at 08:52:50PM -0700, Joe Perches wrote:

> > MILD SUGGESTION: don't spell the function name out in format strings;
> > "this_function: foo is %d", n
> > might be better off as
> > "%s: foo is %d", __func__, n
> > in case you ever move it to another function or rename your function.
> 
> Thank you sir, may I have another.
> 
> checkpatch messages are single line.

Too bad... Incidentally, being able to get more detailed explanation of
a warning might be a serious improvement, especially if it contains
the rationale.  Hell, something like TeX handling of errors might be
a good idea - warning printed, offered actions include 'give more help',
'continue', 'exit', 'from now on suppress this kind of warning', 'from
now on just dump this kind of warning into log and keep going', 'from
now on dump all warnings into log and keep going'.

And yes, I'm serious about having something like "mild suggestion" as
possible severity - people are using that thing to look for potential
improvements to make and 'such and such change might be useful for such
and such reasons' is a lot more useful than 'this needs to be thus because
it must be thus or I'll keep warning'.

Re: [PATCH v13 3/6] mmc: cavium: Add MMC PCI driver for ThunderX SOCs

Hi Jan,

[auto build test ERROR on linus/master]
[also build test ERROR on v4.11-rc4 next-20170331]
[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/Jan-Glauber/Cavium-MMC-driver/20170401-055302
config: sparc-allyesconfig (attached as .config)
compiler: sparc64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget 
https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=sparc 

All errors (new ones prefixed by >>):

   drivers/built-in.o: In function `thunder_mmc_probe':
>> cavium-thunderx.c:(.text+0x2830fcc): undefined reference to 
>> `of_platform_device_create'
   `.exit.data' referenced in section `.exit.text' of drivers/built-in.o: 
defined in discarded section `.exit.data' of drivers/built-in.o
   `.exit.data' referenced in section `.exit.text' of drivers/built-in.o: 
defined in discarded section `.exit.data' of drivers/built-in.o
   `.exit.data' referenced in section `.exit.text' of drivers/built-in.o: 
defined in discarded section `.exit.data' of drivers/built-in.o
   `.exit.data' referenced in section `.exit.text' of drivers/built-in.o: 
defined in discarded section `.exit.data' of drivers/built-in.o

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


.config.gz
Description: application/gzip

Re: [PATCH 001/001] drivers/staging/vt6656/main_usb.c: checkpatch warning

On Sat, 2017-04-01 at 04:46 +0100, Al Viro wrote:
> On Fri, Mar 31, 2017 at 08:36:22PM -0700, Joe Perches wrote:
> > On Sat, 2017-04-01 at 04:32 +0100, Al Viro wrote:
> > > On Fri, Mar 31, 2017 at 06:59:19PM -0700, Chewie Lin wrote:
> > > > Replace string with formatted arguments in the dev_warn() call. It 
> > > > removes
> > > > the checkpatch warning:
> > > > 
> > > > WARNING: Prefer using "%s", __func__ to embedded function names
> > 
> > []
> > > Again, checkpatch warning is badly written
> > 
> > In your opinion, what wording would be better?
> 
> MILD SUGGESTION: don't spell the function name out in format strings;
>   "this_function: foo is %d", n
> might be better off as
>   "%s: foo is %d", __func__, n
> in case you ever move it to another function or rename your function.

Thank you sir, may I have another.

checkpatch messages are single line.

Re: [PATCH v23 00/11] acpi, clocksource: add GTDT driver and GTDT support in arm_arch_timer

2017-03-31 Thread Fu Wei

Hi Xiongfeng Wang,

On 1 April 2017 at 10:14, Xiongfeng Wang  wrote:
>
>
> On 2017/4/1 1:50, fu@linaro.org wrote:
>> From: Fu Wei 
>>
>> This patchset:
>> (1)Preparation for adding GTDT support in arm_arch_timer:
>> 1. Introduce a MMIO CNTFRQ helper.
>> 2. separate out device-tree code from arch_timer_detect_rate
>> 3. remove arch_timer_detect_rate use arch_timer_*get_cntfrq directly
>> 4. Refactor arch_timer_needs_probing, and move it into DT init call
>> 5. Introduce some new structs and refactor the MMIO timer init code
>> for reusing some common code.
>>
>> (2)Introduce ACPI GTDT parser: drivers/acpi/arm64/acpi_gtdt.c
>> Parse all kinds of timer in GTDT table of ACPI:arch timer,
>> memory-mapped timer and SBSA Generic Watchdog timer.
>> This driver can help to simplify all the relevant timer drivers,
>> and separate all the ACPI GTDT knowledge from them.
>>
>> (3)Simplify ACPI code for arm_arch_timer
>>
>> (4)Add GTDT support for ARM memory-mapped timer.
>>
>> This patchset has been tested on the following platforms with ACPI enabled:
>> (1)ARM Foundation v8 model
>>
>
> for arm_arch_timer(not memory-mapped) and sbsa watchdog part,  Tested-by: 
> wangxiongfe...@huawei.com

Great thanks for your testing :-)

>
>
>
>
>
> Thanks,
>
> Wang Xiongfeng
> .
>
>> Changelog:
>> v23: https://lkml.org/lkml/2017/3/31/
>>  Rebase to git://git.kernel.org/pub/scm/linux/kernel/git/mark/linux.git 
>> arch-timer/cleanup
>>  Improve the data struct of arch_timer_mem and arch_timer_mem_frame to
>>  improve the parser of GT blocks and arch_timer_mem initualization.
>>  Improve arch_timer_rate detection: sysreg frequency is primary in DT 
>> boot
>>  Improve some comments in GTDT parser driver.
>>  Improve acpi_gtdt_init function, and make a comment for the multiple 
>> calls.
>>  Improve the unwinding for the irq of timers, when an error occurs.
>>  Handle the case of virtual timer GSIV is 0.
>>
>> v22: https://lkml.org/lkml/2017/3/21/523
>>  Rebase to git://git.kernel.org/pub/scm/linux/kernel/git/mark/linux.git 
>> arch-timer/cleanup
>>  Only Introduce arch_timer_mem_get_cntfrq to get the frequency from mmio.
>>  Merged patch 2,3(about arch_timer_detect_rate).
>>  Keep arch_timer_rate, do NOT split it for different types of timer.
>>  Improve  memory-mapped timer support by comments and variable name:
>>  data-->timer_mem
>>  frame-->gtdt_frame
>>  Delete zero check for SBSA watchdog irq.
>>  Skip secure SBSA watchdog in GTDT driver.
>>  Delete Kconfig modification for SBSA watchdog driver.
>>  Delete no_irq, using nr_res instead.
>>
>> v21: https://lkml.org/lkml/2017/2/6/734
>>  Introduce two functions to get the frequency from mmio and sysreg.
>>  Remove arch_timer_detect_rate use arch_timer_get_*_freq directly
>>  Split arch_timer_rate for different types of timer.
>>  Skip secure timer frame in GTDT driver.
>>  Rebase to git://git.kernel.org/pub/scm/linux/kernel/git/mark/linux.git 
>> arch-timer/cleanup
>>  (The first 6 patches in v20 have been merged into arch-timer/cleanup 
>> branch)
>>
>> v20: https://lkml.org/lkml/2017/1/18/534
>>  Reorder the first 4 patches and split the 4th patches.
>>  Leave CNTHCTL_* as they originally were.
>>  Fix the bug in arch_timer_select_ppi.
>>  Split "Rework counter frequency detection" patch.
>>  Rework the arch_timer_detect_rate function.
>>  Improve the commit message of "Refactor MMIO timer probing".
>>  Rebase to 4.10.0-rc4
>>
>> v19: https://lkml.org/lkml/2016/12/21/25
>>  Fix a '\n' missing in a error message in arch_timer_mem_init.
>>  Add "request_mem_region" for ioremapping cntbase, according to
>>  f947ee1 clocksource/drivers/arm_arch_timer: Map frame with 
>> of_io_request_and_map()
>>  Rebase to 4.9.0-gfb779ff
>>
>> v18: https://lkml.org/lkml/2016/12/8/446
>>  Fix 8/15 patch problem of "int ret;" in arch_timer_acpi_init.
>>  Rebase to 4.9.0-rc8-g9269898
>>
>> v17: https://lkml.org/lkml/2016/11/25/140
>>  Take out some cleanups from 4/15.
>>  Merge 5/15 and 6/15, improve PPI determination code,
>>  improve commit message.
>>  Rework counter frequency detection.
>>  Move arch_timer_needs_of_probing into DT init call.
>>  Move Platform Timer scan loop back to timer init call to avoid 
>> allocating
>>  and free memory.
>>  Improve all the exported functions' comment.
>>
>> v16: https://lkml.org/lkml/2016/11/16/268
>>  Fix patchset problem about static enum ppi_nr of 01/13 in v15.
>>  Refactor arch_timer_detect_rate.
>>  Refactor arch_timer_needs_probing.
>>
>> v15: https://lkml.org/lkml/2016/11/15/366
>>  Re-order patches
>>  Add arm_arch_timer refactoring patches to prepare for GTDT:
>>  1. rename some  enums and defines, and some cleanups
>>  2. separate out arc

Re: [PATCH 001/001] drivers/staging/vt6656/main_usb.c: checkpatch warning

2017-03-31 Thread Al Viro

On Fri, Mar 31, 2017 at 08:36:22PM -0700, Joe Perches wrote:
> On Sat, 2017-04-01 at 04:32 +0100, Al Viro wrote:
> > On Fri, Mar 31, 2017 at 06:59:19PM -0700, Chewie Lin wrote:
> > > Replace string with formatted arguments in the dev_warn() call. It removes
> > > the checkpatch warning:
> > > 
> > >   WARNING: Prefer using "%s", __func__ to embedded function names
> []
> > Again, checkpatch warning is badly written
> 
> In your opinion, what wording would be better?

MILD SUGGESTION: don't spell the function name out in format strings;
"this_function: foo is %d", n
might be better off as
"%s: foo is %d", __func__, n
in case you ever move it to another function or rename your function.

Re: [PATCH 001/001] drivers/staging/vt6656/main_usb.c: checkpatch warning

On Sat, 2017-04-01 at 04:32 +0100, Al Viro wrote:
> On Fri, Mar 31, 2017 at 06:59:19PM -0700, Chewie Lin wrote:
> > Replace string with formatted arguments in the dev_warn() call. It removes
> > the checkpatch warning:
> > 
> > WARNING: Prefer using "%s", __func__ to embedded function names
[]
> Again, checkpatch warning is badly written

In your opinion, what wording would be better?

Re: [PATCH 001/001] drivers/staging/vt6656/main_usb.c: checkpatch warning

On Fri, 2017-03-31 at 20:18 -0700, Chewie Lin wrote:
> These are good points, but any feedback on the dev_warn() call itself?
> I was trying to fix the checkpatch warning on my first try.

Using "%s" with a long string is generally inefficient.

Compare the compiled object size of
printf("%s is %d\n", "Long string", 1);
to
printf("Long string is %d\n", 1);

Re: [PATCH 001/001] drivers/staging/vt6656/main_usb.c: checkpatch warning

2017-03-31 Thread Al Viro

On Fri, Mar 31, 2017 at 06:59:19PM -0700, Chewie Lin wrote:
> Replace string with formatted arguments in the dev_warn() call. It removes
> the checkpatch warning:
> 
>   WARNING: Prefer using "%s", __func__ to embedded function names
>   #417: FILE: main_usb.c:417:
>   +"usb_device_reset fail status=%d\n", status);
> 
>   total: 0 errors, 1 warnings, 1058 lines checked
> 
> And after fix:
> 
>   main_usb.c has no obvious style problems and is ready for submission.

> -  "usb_device_reset fail status=%d\n", status);
> +  "%s=%d\n", "usb_device_reset fail status", status);

Would you mind explaining the meaning of that warning?  Incidentally, why not
  "%se_reset fail status=%d\n", "usb_devic", status);
- it would produce the identical output and silences checkpatch even more
reliably.  Discuss.

Sarcasm aside, when you are proposing a change, there are several questions you
really must ask yourself and be able to answer.  First and foremost, of course,
is
* Why is it an improvement?
If the answer to that was "it makes a warning go away", the next questions are
* What are we warned about?
* What is problematic in the original variant?
* Is the replacement free from the problem we had in the original?
and if the answer to any of that is "I don't know", the next step is _not_
"send it anyway".  "Try to figure out" might be a good idea, with usual
heuristics regarding the reading comprehension ("if a sentence remains
a mystery, see if there are any unfamiliar words skipped at the first reading
and find what they mean", etc.) applying.

In this particular case the part you've apparently skipped was, indeed,
critical - "__func__".  I am not trying to defend the quality of checkpatch -
the warning is badly worded, the tool is badly written and more often than
not its suggestions reflect nothing but authors' arbitrary preferences.

This one, however, does have some rationale behind it.  Namely, if some
debugging output contains the name of a function that has produced it,
having that name spelled out in format string is not a good idea.  If
that code gets moved elsewhere one would have to replace the name in format
string or face confusing messages refering to the function where that
code used to be.  Since __func__ is interpreted as a constant string
initialized with the name of the function it's used in, it is possible
to avoid spelling the function name out - instead of
"my_function: pink elephants; reduce the daily intake to %d glasses\n", n
one could use
"%s: pink elephants; reduce the daily intake to %d glasses\n", __func__, n
producing the same output and avoiding the need to adjust anything when
the whole thing is moved to another function.  It's not particularly big
deal, but it's a more or less reasonable suggestion.

Your change, of course, has achieved only one thing - it has moved the
explicitly spelled out function name out of format string.  It's still
just as explicitly spelled out, still would need to be adjusted if moved
to another function and the whole thing has become harder to read and
understand since you've buried other parts of message in the place where
it's harder to follow.

Again, checkpatch warning is badly written, but the main problem with
your posting is not that you had been confused by checkpatch - it's
that you have posted it based on an incomprehensible message with no better
rationale than "it shuts checkpatch up, dunno why and what about".

Re: [PATCH -mm -v7 4/9] mm, THP, swap: Add get_huge_swap_page()

Johannes Weiner  writes:

> On Thu, Mar 30, 2017 at 12:28:17PM +0800, Huang, Ying wrote:
>> Johannes Weiner  writes:
>> > On Tue, Mar 28, 2017 at 01:32:04PM +0800, Huang, Ying wrote:
>> >> @@ -527,6 +527,23 @@ static inline swp_entry_t get_swap_page(void)
>> >>  
>> >>  #endif /* CONFIG_SWAP */
>> >>  
>> >> +#ifdef CONFIG_THP_SWAP_CLUSTER
>> >> +static inline swp_entry_t get_huge_swap_page(void)
>> >> +{
>> >> + swp_entry_t entry;
>> >> +
>> >> + if (get_swap_pages(1, &entry, true))
>> >> + return entry;
>> >> + else
>> >> + return (swp_entry_t) {0};
>> >> +}
>> >> +#else
>> >> +static inline swp_entry_t get_huge_swap_page(void)
>> >> +{
>> >> + return (swp_entry_t) {0};
>> >> +}
>> >> +#endif
>> >
>> > Your introducing a function without a user, making it very hard to
>> > judge whether the API is well-designed for the callers or not.
>> >
>> > I pointed this out as a systemic problem with this patch series in v3,
>> > along with other stuff, but with the way this series is structured I'm
>> > having a hard time seeing whether you implemented my other feedback or
>> > whether your counter arguments to them are justified.
>> >
>> > I cannot review and ack these patches this way.
>> 
>> Sorry for inconvenience, I will send a new version to combine the
>> function definition and usage into one patch at least for you to
>> review.
>
> We tried this before. I reviewed the self-contained patch and you
> incorporated the feedback into the split-out structure that made it
> impossible for me to verify the updates.
>
> I'm not sure why you insist on preserving this series format. It's not
> good for review, and it's not good for merging and git history.

I had thought some reviewers would prefer the original series format.
But I will use your suggested format in the future, unless more
reviewers prefer the original format.

Best Regards,
Huang, Ying

>> But I think we can continue our discussion in the comments your
>> raised so far firstly, what do you think about that?
>
> Yeah, let's finish the discussions before -v8.

Re: [PATCH -mm -v7 1/9] mm, swap: Make swap cluster size same of THP size on x86_64

Johannes Weiner  writes:

> On Thu, Mar 30, 2017 at 08:45:56AM +0800, Huang, Ying wrote:
>> Johannes Weiner  writes:
>> 
>> > On Tue, Mar 28, 2017 at 01:32:01PM +0800, Huang, Ying wrote:
>> >> @@ -499,6 +499,19 @@ config FRONTSWAP
>> >>  
>> >> If unsure, say Y to enable frontswap.
>> >>  
>> >> +config ARCH_USES_THP_SWAP_CLUSTER
>> >> + bool
>> >> + default n
>> >
>> > This is fine.
>> >
>> >> +config THP_SWAP_CLUSTER
>> >> + bool
>> >> + depends on SWAP && TRANSPARENT_HUGEPAGE && ARCH_USES_THP_SWAP_CLUSTER
>> >> + default y
>> >> + help
>> >> +   Use one swap cluster to hold the contents of the THP
>> >> +   (Transparent Huge Page) swapped out.  The size of the swap
>> >> +   cluster will be same as that of THP.
>> >
>> > But this is a super weird thing to ask the user. How would they know
>> > what to say, if we don't know? I don't think this should be a config
>> > knob at all. Merge the two config items into a simple
>> 
>> The user will not see this, because there is no string after "bool" to
>> let user to select it.  The help here is for document only, so that
>> architecture developers could know what this is for.
>
> Oh, I missed that. My bad!
>
>> > config THP_SWAP_CLUSTER
>> >  bool
>> >  default n
>> >
>> > and let the archs with reasonable THP sizes select it.
>> 
>> This will have same effect as the original solution except the document
>> is removed.
>
> Then I still don't understand why we need two config symbols. Can't
> archs select the documented THP_SWAP_CLUSTER directly?
>
> The #ifdef in swapfile.c could check THP && THP_SWAP_CLUSTER.
>
> Am I missing something?

I use two config symbols just to save some typing, instead of

#ifdef CONFIG_THP_SWAP_CLUSTER

it will be,

#if defined(CONFIG_TRANSPARENT_HUGEPAGE) && defined(CONFIG_THP_SWAP_CLUSTER)

or

#if defined(CONFIG_SWAP) && defined(CONFIG_TRANSPARENT_HUGEPAGE) && 
defined(CONFIG_THP_SWAP_CLUSTER)

Best Regards,
Huang, Ying

Re: [PATCH] drivers: fixed a checkpatch warning

On Fri, Mar 31, 2017 at 11:53:55AM -0700, Joe Perches wrote:
> On Fri, 2017-03-31 at 01:40 -0700, Chewie Lin wrote:
> > fixed a coding style error/warning.
> > 
> > Signed-off-by: Chewie Lin 
> > ---
> >  drivers/staging/vt6656/main_usb.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/staging/vt6656/main_usb.c 
> > b/drivers/staging/vt6656/main_usb.c
> > index 9e074e9..2d9e7af 100644
> > --- a/drivers/staging/vt6656/main_usb.c
> > +++ b/drivers/staging/vt6656/main_usb.c
> > @@ -414,7 +414,7 @@ static void usb_device_reset(struct vnt_private *priv)
> > status = usb_reset_device(priv->usb);
> > if (status)
> > dev_warn(&priv->usb->dev,
> > -"usb_device_reset fail status=%d\n", status);
> > +"%s=%d\n", "usb_device_reset fail status", status);
> 
> Yeah, what Greg said.
> 
> Also most likely this should be
> 
>   dev_warn(&priv->usb->dev,
>"usb_reset_device fail status=%d\n", status);
> 
> Note the function is usb_device_reset, but the
> function call that failed is usb_reset_device.

yep, I had the comic book guy from the Simpsons voice when I read that. :)
I submitted a separate patch, maybe I should have reply-to this instead?

>

Re: [PATCH 001/001] drivers/staging/vt6656/main_usb.c: checkpatch warning

On Fri, Mar 31, 2017 at 07:45:09PM -0700, Joe Perches wrote:
> On Fri, 2017-03-31 at 19:15 -0700, Randy Dunlap wrote:
> > On 03/31/17 18:59, Chewie Lin wrote:
> > > Replace string with formatted arguments in the dev_warn() call. It removes
> > > the checkpatch warning:
> > > 
> > >   WARNING: Prefer using "%s", __func__ to embedded function names
> > >   #417: FILE: main_usb.c:417:
> > >   +"usb_device_reset fail status=%d\n", status);
> > > 
> > >   total: 0 errors, 1 warnings, 1058 lines checked
> > > 
> > > And after fix:
> > > 
> > >   main_usb.c has no obvious style problems and is ready for submission.
> > > 
> > > Signed-off-by: Chewie Lin 
> > > ---
> > >  drivers/staging/vt6656/main_usb.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/staging/vt6656/main_usb.c 
> > > b/drivers/staging/vt6656/main_usb.c
> > > index 9e074e9..2d9e7af 100644
> > > --- a/drivers/staging/vt6656/main_usb.c
> > > +++ b/drivers/staging/vt6656/main_usb.c
> > > @@ -414,7 +414,7 @@ static void usb_device_reset(struct vnt_private *priv)
> > >   status = usb_reset_device(priv->usb);
> > >   if (status)
> > >   dev_warn(&priv->usb->dev,
> > > -  "usb_device_reset fail status=%d\n", status);
> > > +  "%s=%d\n", "usb_device_reset fail status", status);
> > >  }
> > >  
> > >  static void vnt_free_int_bufs(struct vnt_private *priv)
> > > 
> > 
> > As other people have said:
> > 
> > This function is usb_device_reset().  If that function name string is to be
> > used in the message, it should be done so by using __func__.
> > See http://marc.info/?l=linux-driver-devel&m=149095639202492&w=2
> > 
> > Or is the called (failing) function name is to be kept in the message (as it
> > is currently), then the message should contain "usb_reset_device" instead of
> > "usb_device_reset".  See 
> > http://marc.info/?l=linux-driver-devel&m=149098680312723&w=2
> 
> If this is to be changed at all, I suggest
> just getting rid of the function.

These are good points, but any feedback on the dev_warn() call itself? 
I was trying to fix the checkpatch warning on my first try.

Re: [BUG nohz]: wrong user and system time accounting

2017-03-31 Thread Luiz Capitulino

On Sat, 1 Apr 2017 01:24:54 +0200
Frederic Weisbecker  wrote:

> On Fri, Mar 31, 2017 at 04:09:10PM -0400, Luiz Capitulino wrote:
> > On Thu, 30 Mar 2017 17:25:46 -0400
> > Luiz Capitulino  wrote:
> >   
> > > On Thu, 30 Mar 2017 16:18:17 +0200
> > > Frederic Weisbecker  wrote:
> > >   
> > > > On Thu, Mar 30, 2017 at 09:59:54PM +0800, Wanpeng Li wrote:
> > > > > 2017-03-30 21:38 GMT+08:00 Frederic Weisbecker :  
> > > > > 
> > > > > > If it works, we may want to take that solution, likely less 
> > > > > > performance sensitive
> > > > > > than using sched_clock(). In fact sched_clock() is fast, especially 
> > > > > > as we require it to
> > > > > > be stable for nohz_full, but using it involves costly conversion 
> > > > > > back and forth to jiffies.  
> > > > > 
> > > > > So both Rik and you agree with the skew tick solution, I will try it
> > > > > tomorrow. Btw, if we should just add random offset to the cpu in the
> > > > > nohz_full mode or add random offset to all cpus like the codes above? 
> > > > >  
> > > > 
> > > > Lets just keep it to all CPUs for simplicty.
> > > > Also please add a comment that explains why we need that skew_tick on 
> > > > nohz_full.
> > > 
> > > I've tried all the test-cases we discussed in this thread with skew_tick=1
> > > and it worked as expected in bare-metal and KVM guests.
> > > 
> > > However, I found a test-case that works in bare-metal but show problems
> > > in KVM guests. It could something that's KVM specific, or it could be
> > > something that's harder to reproduce in bare-metal.  
> > 
> > After discussing some findings on this issue with Rik, I realized that
> > we don't add the skew when restarting the tick in tick_nohz_restart().
> > Adding the offset there seems to solve this problem.  
> 
> Are you sure? tick_nohz_restart() doesn't seem to override the initial skew. 
> It
> always forwards the expiration time on top of the last tick.

OK, I'll double check. Without my change the bug triggers almost
instantly with the described reproducer. With my change it didn't
trig for several minutes (but it does look wrong looking at it now).

Re: [PATCH -mm -v7 9/9] mm, THP, swap: Delay splitting THP during swap out

Johannes Weiner  writes:

> On Thu, Mar 30, 2017 at 12:15:13PM +0800, Huang, Ying wrote:
>> Johannes Weiner  writes:
>> > On Tue, Mar 28, 2017 at 01:32:09PM +0800, Huang, Ying wrote:
>> >> @@ -198,6 +240,18 @@ int add_to_swap(struct page *page, struct list_head 
>> >> *list)
>> >>   VM_BUG_ON_PAGE(!PageLocked(page), page);
>> >>   VM_BUG_ON_PAGE(!PageUptodate(page), page);
>> >>  
>> >> + if (unlikely(PageTransHuge(page))) {
>> >> + err = add_to_swap_trans_huge(page, list);
>> >> + switch (err) {
>> >> + case 1:
>> >> + return 1;
>> >> + case 0:
>> >> + /* fallback to split firstly if return 0 */
>> >> + break;
>> >> + default:
>> >> + return 0;
>> >> + }
>> >> + }
>> >>   entry = get_swap_page();
>> >>   if (!entry.val)
>> >>   return 0;
>> >
>> > add_to_swap_trans_huge() is too close a copy of add_to_swap(), which
>> > makes the code error prone for future modifications to the swap slot
>> > allocation protocol.
>> >
>> > This should read:
>> >
>> > retry:
>> >entry = get_swap_page(page);
>> >if (!entry.val) {
>> >if (PageTransHuge(page)) {
>> >split_huge_page_to_list(page, list);
>> >goto retry;
>> >}
>> >return 0;
>> >}
>> 
>> If the swap space is used up, that is, get_swap_page() cannot allocate
>> even 1 swap entry for a normal page.  We will split THP unnecessarily
>> with the change, but in the original code, we just skip the THP.  There
>> may be a performance regression here.  Similar problem exists for
>> mem_cgroup_try_charge_swap() too.  If the mem cgroup exceeds the swap
>> limit, the THP will be split unnecessary with the change too.
>
> If we skip the page, we're swapping out another page hotter than this
> one. Giving THP preservation priority over LRU order is an issue best
> kept for a separate patch set;

In my original patch, if we failed to allocate the swap space for a THP,
and we can allocate the swap space for a normal page, we will split the
THP.  We skip the page only if we cannot allocate the swap space for a
normal page, that is, nr_swap_pages is 0.  So we will not give THP
preservation priority over LRU order in the patch.

> this one is supposed to be a mechanical
> implementation of THP swapping. Let's nail down the basics first.

Yes.  So I tried to keep the original behavior to deal with THP if we
cannot allocate the swap space (a swap cluster) for a whole THP.

Per my understanding, the difference between what you suggested and the
original behavior is that, when nr_swap_pages is 0, whether to split the
THP.

> Such a decision would need proof that splitting THPs on full swap
> devices is a concern for real applications. I would assume that we're
> pretty close to OOM anyway; it's much more likely that a single slot
> frees up than a full cluster, at which point we'll be splitting THPs
> anyway; etc. I have my doubts that this would be measurable.
>
> But even if so, I don't think we'd have to duplicate the main code
> flow to handle this corner case. You can extend get_swap_page() to
> return an error code that tells add_to_swap() whether to split and
> retry, or to fail and move on. So this way should be future proof.

Yes.  I will try to merge add_to_swap_trans_huge() into add_to_swap() in
the next version.  But if we want to keep the original behavior, we will
need an extra "nr_entries" parameter for mem_cgroup_try_charge_swap().

Best Regards,
Huang, Ying

Re: [PATCH 001/001] drivers/staging/vt6656/main_usb.c: checkpatch warning

On Fri, 2017-03-31 at 19:15 -0700, Randy Dunlap wrote:
> On 03/31/17 18:59, Chewie Lin wrote:
> > Replace string with formatted arguments in the dev_warn() call. It removes
> > the checkpatch warning:
> > 
> > WARNING: Prefer using "%s", __func__ to embedded function names
> > #417: FILE: main_usb.c:417:
> > +"usb_device_reset fail status=%d\n", status);
> > 
> > total: 0 errors, 1 warnings, 1058 lines checked
> > 
> > And after fix:
> > 
> > main_usb.c has no obvious style problems and is ready for submission.
> > 
> > Signed-off-by: Chewie Lin 
> > ---
> >  drivers/staging/vt6656/main_usb.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/staging/vt6656/main_usb.c 
> > b/drivers/staging/vt6656/main_usb.c
> > index 9e074e9..2d9e7af 100644
> > --- a/drivers/staging/vt6656/main_usb.c
> > +++ b/drivers/staging/vt6656/main_usb.c
> > @@ -414,7 +414,7 @@ static void usb_device_reset(struct vnt_private *priv)
> > status = usb_reset_device(priv->usb);
> > if (status)
> > dev_warn(&priv->usb->dev,
> > -"usb_device_reset fail status=%d\n", status);
> > +"%s=%d\n", "usb_device_reset fail status", status);
> >  }
> >  
> >  static void vnt_free_int_bufs(struct vnt_private *priv)
> > 
> 
> As other people have said:
> 
> This function is usb_device_reset().  If that function name string is to be
> used in the message, it should be done so by using __func__.
> See http://marc.info/?l=linux-driver-devel&m=149095639202492&w=2
> 
> Or is the called (failing) function name is to be kept in the message (as it
> is currently), then the message should contain "usb_reset_device" instead of
> "usb_device_reset".  See 
> http://marc.info/?l=linux-driver-devel&m=149098680312723&w=2

If this is to be changed at all, I suggest
just getting rid of the function.
---
 drivers/staging/vt6656/main_usb.c | 14 +++---
 1 file changed, 3 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/vt6656/main_usb.c 
b/drivers/staging/vt6656/main_usb.c
index 9e074e9daf4e..799aeeb6812d 100644
--- a/drivers/staging/vt6656/main_usb.c
+++ b/drivers/staging/vt6656/main_usb.c
@@ -407,16 +407,6 @@ static void vnt_free_rx_bufs(struct vnt_private *priv)
}
 }
 
-static void usb_device_reset(struct vnt_private *priv)
-{
-   int status;
-
-   status = usb_reset_device(priv->usb);
-   if (status)
-   dev_warn(&priv->usb->dev,
-"usb_device_reset fail status=%d\n", status);
-}
-
 static void vnt_free_int_bufs(struct vnt_private *priv)
 {
kfree(priv->int_buf.data_buf);
@@ -995,7 +985,9 @@ vt6656_probe(struct usb_interface *intf, const struct 
usb_device_id *id)
 
SET_IEEE80211_DEV(priv->hw, &intf->dev);
 
-   usb_device_reset(priv);
+   rc = usb_reset_device(priv->usb);
+   if (rc)
+   dev_warn(&priv->usb->dev, "usb_reset_device failed: %d\n", rc);
 
clear_bit(DEVICE_FLAGS_DISCONNECTED, &priv->flags);
vnt_reset_command_timer(priv);

[PATCH] f2fs: remove the redundant variable definition

2017-03-31 Thread Kaixu Xia

The variable 'i' has been defined before, so here we can
use it directly.

Signed-off-by: Kaixu Xia 
---
 fs/f2fs/checkpoint.c |1 -
 1 file changed, 1 deletion(-)

diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
index 0339daf..2e42684 100644
--- a/fs/f2fs/checkpoint.c
+++ b/fs/f2fs/checkpoint.c
@@ -1143,7 +1143,6 @@ static int do_checkpoint(struct f2fs_sb_info *sbi, struct 
cp_control *cpc)
/* write nat bits */
if (enabled_nat_bits(sbi, cpc)) {
__u64 cp_ver = cur_cp_version(ckpt);
-   unsigned int i;
block_t blk;
 
cp_ver |= ((__u64)crc32 << 32);
-- 
1.7.10.4

[PATCH] staging: iio: light: constify attribute_group structures

2017-03-31 Thread simran singhal

As the event_attrs field of iio_info structures is constant, so these
attribute_group structures can also be declared constant.

File size before:
   textdata bss dec hex filename
  150641528   0   1659240d0
drivers/staging/iio/light/tsl2x7x_core.o

File size after:
   textdata bss dec hex filename
  151921400   0   1659240d0
drivers/staging/iio/light/tsl2x7x_core.o

Signed-off-by: simran singhal 
---
 drivers/staging/iio/light/tsl2x7x_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/iio/light/tsl2x7x_core.c 
b/drivers/staging/iio/light/tsl2x7x_core.c
index 0490c1d..9a0046a 100644
--- a/drivers/staging/iio/light/tsl2x7x_core.c
+++ b/drivers/staging/iio/light/tsl2x7x_core.c
@@ -1676,7 +1676,7 @@ static const struct attribute_group 
tsl2X7X_device_attr_group_tbl[] = {
},
 };
 
-static struct attribute_group tsl2X7X_event_attr_group_tbl[] = {
+static const struct attribute_group tsl2X7X_event_attr_group_tbl[] = {
[ALS] = {
.attrs = tsl2X7X_ALS_event_attrs,
.name = "events",
-- 
2.7.4

Re: [v6 PATCH 00/21] x86: Enable User-Mode Instruction Prevention

2017-03-31 Thread Andy Lutomirski

On Fri, Mar 31, 2017 at 2:26 PM, Stas Sergeev  wrote:
> 31.03.2017 17:11, Alexandre Julliard пишет:
>>
>> In fact it would be nice to be able to make sidt/sgdt/etc. segfault
>> too. I know a new syscall is a pain,
>
> Maybe arch_prctl() then?

I still like my idea of a generic mechanism to turn off
backwards-compatibility things.  After all, hardened programs should
turn off UMIP fixups entirely.  They should also turn off vsyscall
emulation entirely, and I see no reason that these mechanisms should
be different.

--Andy

Re: [RFC 1/8] Introduce Peer-to-Peer memory (p2pmem) device

2017-03-31 Thread okaya


On 2017-03-31 21:57, Logan Gunthorpe wrote:

On 31/03/17 05:51 PM, Sinan Kaya wrote:
You can put a restriction with DMI/SMBIOS such that all devices from 
2016

work else they belong to blacklist.


How do you get a manufacturing date for a given device within the
kernel? Is this actually something generically available?

Logan


Smbios calls are used all over the place in kernel for introducing new 
functionality while maintaining backwards compatibility.


See drivers/pci and drivers/acpi directory.

Re: [PATCH 001/001] drivers/staging/vt6656/main_usb.c: checkpatch warning

On Fri, 2017-03-31 at 18:59 -0700, Chewie Lin wrote:
> Replace string with formatted arguments in the dev_warn() call. It removes
> the checkpatch warning:
> 
>   WARNING: Prefer using "%s", __func__ to embedded function names
>   #417: FILE: main_usb.c:417:
>   +"usb_device_reset fail status=%d\n", status);
> 
>   total: 0 errors, 1 warnings, 1058 lines checked
> 
> And after fix:
> 
>   main_usb.c has no obvious style problems and is ready for submission.
> 
> Signed-off-by: Chewie Lin 
> ---
>  drivers/staging/vt6656/main_usb.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/vt6656/main_usb.c 
> b/drivers/staging/vt6656/main_usb.c
> index 9e074e9..2d9e7af 100644
> --- a/drivers/staging/vt6656/main_usb.c
> +++ b/drivers/staging/vt6656/main_usb.c
> @@ -414,7 +414,7 @@ static void usb_device_reset(struct vnt_private *priv)
>   status = usb_reset_device(priv->usb);
>   if (status)
>   dev_warn(&priv->usb->dev,
> -  "usb_device_reset fail status=%d\n", status);
> +  "%s=%d\n", "usb_device_reset fail status", status);

So, what is failing?  usb_reset_device or usb_device_reset?

And this function is used exactly once, is trivial, and
would likely be better as direct code in the one place
it's used.

Re: [PATCH V8 5/6] ACPI: Support the probing on the devices which apply indirect-IO

2017-03-31 Thread zhichang.yuan



On 04/01/2017 07:02 AM, Rafael J. Wysocki wrote:
> On Fri, Mar 31, 2017 at 8:52 AM, zhichang.yuan
>  wrote:
>> Hi, Rafael,
>>
>> Thanks for reviewing this!
>>
>> On 2017/3/31 4:31, Rafael J. Wysocki wrote:
>>> On Thursday, March 30, 2017 11:26:58 PM zhichang.yuan wrote:
 On some platforms(such as Hip06/Hip07), the legacy ISA/LPC devices access 
 I/O
 with some special host-local I/O ports known on x86. To access the I/O
 peripherals, an indirect-IO mechanism is introduced to mapped the 
 host-local
 I/O to system logical/fake PIO similar the PCI MMIO on architectures where 
 no
 separate I/O space exists. Just as PCI MMIO, the host I/O range should be
 registered before probing the downstream devices and set up the I/O 
 mapping.
 But current ACPI bus probing doesn't support these indirect-IO 
 hosts/devices.

 This patch introdueces a new ACPI handler for this device category. 
 Through the
 handler attach callback, the indirect-IO hosts I/O registration is done and
 all peripherals' I/O resources are translated into logic/fake PIO before
 starting the enumeration.
>>>
>>> Can you explain to me briefly what exactly this code is expected to be 
>>> doing?
>>
>> As you know currently for ARM architecture IO space is memory mapped and
>> is only used by pci devices. The port number is dynamically allocated
>> converting the device IO address into a PIO token: i.e.
>> http://lxr.free-electrons.com/source/drivers/acpi/pci_root.c#L745
>> This patch is meant to support a new class of IO host controller
>> that are not PCI based and that still require to have the IO addresses
>> be translated in the same PIO token space as the PCI controller
> 
> IOW, this is ARM-specific, right?

Yes. The current host added in this patch with _HID "HISI0191" is on ARM64.
But, I think the handler driver is architecture dependent.

Thanks,
Zhichang

> 
> Thanks,
> Rafael
>

Re: [PATCH v23 00/11] acpi, clocksource: add GTDT driver and GTDT support in arm_arch_timer

2017-03-31 Thread Xiongfeng Wang



On 2017/4/1 1:50, fu@linaro.org wrote:
> From: Fu Wei 
> 
> This patchset:
> (1)Preparation for adding GTDT support in arm_arch_timer:
> 1. Introduce a MMIO CNTFRQ helper.
> 2. separate out device-tree code from arch_timer_detect_rate
> 3. remove arch_timer_detect_rate use arch_timer_*get_cntfrq directly
> 4. Refactor arch_timer_needs_probing, and move it into DT init call
> 5. Introduce some new structs and refactor the MMIO timer init code
> for reusing some common code.
> 
> (2)Introduce ACPI GTDT parser: drivers/acpi/arm64/acpi_gtdt.c
> Parse all kinds of timer in GTDT table of ACPI:arch timer,
> memory-mapped timer and SBSA Generic Watchdog timer.
> This driver can help to simplify all the relevant timer drivers,
> and separate all the ACPI GTDT knowledge from them.
> 
> (3)Simplify ACPI code for arm_arch_timer
> 
> (4)Add GTDT support for ARM memory-mapped timer.
> 
> This patchset has been tested on the following platforms with ACPI enabled:
> (1)ARM Foundation v8 model
> 

for arm_arch_timer(not memory-mapped) and sbsa watchdog part,  Tested-by: 
wangxiongfe...@huawei.com





Thanks,

Wang Xiongfeng
.

> Changelog:
> v23: https://lkml.org/lkml/2017/3/31/
>  Rebase to git://git.kernel.org/pub/scm/linux/kernel/git/mark/linux.git 
> arch-timer/cleanup
>  Improve the data struct of arch_timer_mem and arch_timer_mem_frame to
>  improve the parser of GT blocks and arch_timer_mem initualization.
>  Improve arch_timer_rate detection: sysreg frequency is primary in DT boot
>  Improve some comments in GTDT parser driver.
>  Improve acpi_gtdt_init function, and make a comment for the multiple 
> calls.
>  Improve the unwinding for the irq of timers, when an error occurs.
>  Handle the case of virtual timer GSIV is 0.
> 
> v22: https://lkml.org/lkml/2017/3/21/523
>  Rebase to git://git.kernel.org/pub/scm/linux/kernel/git/mark/linux.git 
> arch-timer/cleanup
>  Only Introduce arch_timer_mem_get_cntfrq to get the frequency from mmio.
>  Merged patch 2,3(about arch_timer_detect_rate).
>  Keep arch_timer_rate, do NOT split it for different types of timer.
>  Improve  memory-mapped timer support by comments and variable name:
>  data-->timer_mem
>  frame-->gtdt_frame
>  Delete zero check for SBSA watchdog irq.
>  Skip secure SBSA watchdog in GTDT driver.
>  Delete Kconfig modification for SBSA watchdog driver.
>  Delete no_irq, using nr_res instead.
> 
> v21: https://lkml.org/lkml/2017/2/6/734
>  Introduce two functions to get the frequency from mmio and sysreg.
>  Remove arch_timer_detect_rate use arch_timer_get_*_freq directly
>  Split arch_timer_rate for different types of timer.
>  Skip secure timer frame in GTDT driver.
>  Rebase to git://git.kernel.org/pub/scm/linux/kernel/git/mark/linux.git 
> arch-timer/cleanup
>  (The first 6 patches in v20 have been merged into arch-timer/cleanup 
> branch)
> 
> v20: https://lkml.org/lkml/2017/1/18/534
>  Reorder the first 4 patches and split the 4th patches.
>  Leave CNTHCTL_* as they originally were.
>  Fix the bug in arch_timer_select_ppi.
>  Split "Rework counter frequency detection" patch.
>  Rework the arch_timer_detect_rate function.
>  Improve the commit message of "Refactor MMIO timer probing".
>  Rebase to 4.10.0-rc4
> 
> v19: https://lkml.org/lkml/2016/12/21/25
>  Fix a '\n' missing in a error message in arch_timer_mem_init.
>  Add "request_mem_region" for ioremapping cntbase, according to
>  f947ee1 clocksource/drivers/arm_arch_timer: Map frame with 
> of_io_request_and_map()
>  Rebase to 4.9.0-gfb779ff
> 
> v18: https://lkml.org/lkml/2016/12/8/446
>  Fix 8/15 patch problem of "int ret;" in arch_timer_acpi_init.
>  Rebase to 4.9.0-rc8-g9269898
> 
> v17: https://lkml.org/lkml/2016/11/25/140
>  Take out some cleanups from 4/15.
>  Merge 5/15 and 6/15, improve PPI determination code,
>  improve commit message.
>  Rework counter frequency detection.
>  Move arch_timer_needs_of_probing into DT init call.
>  Move Platform Timer scan loop back to timer init call to avoid allocating
>  and free memory.
>  Improve all the exported functions' comment.
> 
> v16: https://lkml.org/lkml/2016/11/16/268
>  Fix patchset problem about static enum ppi_nr of 01/13 in v15.
>  Refactor arch_timer_detect_rate.
>  Refactor arch_timer_needs_probing.
> 
> v15: https://lkml.org/lkml/2016/11/15/366
>  Re-order patches
>  Add arm_arch_timer refactoring patches to prepare for GTDT:
>  1. rename some  enums and defines, and some cleanups
>  2. separate out arch_timer_uses_ppi init code and fix a potential bug
>  3. Improve some new structs, refactor the timer init code.
>  Since the some structs have been changed, GTDT parser for memory-mapped
>  timer and SBSA

Re: [PATCH 001/001] drivers/staging/vt6656/main_usb.c: checkpatch warning

2017-03-31 Thread Randy Dunlap

On 03/31/17 18:59, Chewie Lin wrote:
> Replace string with formatted arguments in the dev_warn() call. It removes
> the checkpatch warning:
> 
>   WARNING: Prefer using "%s", __func__ to embedded function names
>   #417: FILE: main_usb.c:417:
>   +"usb_device_reset fail status=%d\n", status);
> 
>   total: 0 errors, 1 warnings, 1058 lines checked
> 
> And after fix:
> 
>   main_usb.c has no obvious style problems and is ready for submission.
> 
> Signed-off-by: Chewie Lin 
> ---
>  drivers/staging/vt6656/main_usb.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/vt6656/main_usb.c 
> b/drivers/staging/vt6656/main_usb.c
> index 9e074e9..2d9e7af 100644
> --- a/drivers/staging/vt6656/main_usb.c
> +++ b/drivers/staging/vt6656/main_usb.c
> @@ -414,7 +414,7 @@ static void usb_device_reset(struct vnt_private *priv)
>   status = usb_reset_device(priv->usb);
>   if (status)
>   dev_warn(&priv->usb->dev,
> -  "usb_device_reset fail status=%d\n", status);
> +  "%s=%d\n", "usb_device_reset fail status", status);
>  }
>  
>  static void vnt_free_int_bufs(struct vnt_private *priv)
> 

As other people have said:

This function is usb_device_reset().  If that function name string is to be
used in the message, it should be done so by using __func__.
See http://marc.info/?l=linux-driver-devel&m=149095639202492&w=2

Or is the called (failing) function name is to be kept in the message (as it
is currently), then the message should contain "usb_reset_device" instead of
"usb_device_reset".  See 
http://marc.info/?l=linux-driver-devel&m=149098680312723&w=2



-- 
~Randy

[PATCH 5/9] perf trace: Handle unpaired raw_syscalls:sys_exit event

From: Arnaldo Carvalho de Melo 

Which may happen when we start a tracing session and a thread is waiting
for something like "poll" to return, in which case we better print "?"
both for the syscall entry timestamp and for the duration.

E.g.:

Tracing existing mutt session:

  # perf trace -p `pidof mutt`
  ? ( ?   ): mutt/17135  ... [continued]: poll()) = 1
  0.027 ( 0.013 ms): mutt/17135 read(buf: 0x7ffcb3c42cef, count: 1) = 1
  0.047 ( 0.008 ms): mutt/17135 poll(ufds: 0x7ffcb3c42c50, nfds: 1, 
timeout_msecs: 1000) = 1
  0.059 ( 0.008 ms): mutt/17135 read(buf: 0x7ffcb3c42cef, count: 1) = 1
  

Before it would print a large number because we'd do:

  ttrace->entry_time - trace->base_time

And entry_time would be 0, while base_time would be the timestamp for
the first event 'perf trace' reads, oops.

Cc: Adrian Hunter 
Cc: David Ahern 
Cc: Jiri Olsa 
Cc: Luis Claudio Gonçalves 
Cc: Namhyung Kim 
Cc: Wang Nan 
Link: http://lkml.kernel.org/n/tip-wbcb93ofva2qdjd5ltn5e...@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo 
---
 tools/perf/builtin-trace.c | 43 ++-
 1 file changed, 34 insertions(+), 9 deletions(-)

diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
index c88f9f215e6f..7379792a6504 100644
--- a/tools/perf/builtin-trace.c
+++ b/tools/perf/builtin-trace.c
@@ -821,12 +821,21 @@ struct syscall {
void**arg_parm;
 };
 
-static size_t fprintf_duration(unsigned long t, FILE *fp)
+/*
+ * We need to have this 'calculated' boolean because in some cases we really
+ * don't know what is the duration of a syscall, for instance, when we start
+ * a session and some threads are waiting for a syscall to finish, say 'poll',
+ * in which case all we can do is to print "( ? ) for duration and for the
+ * start timestamp.
+ */
+static size_t fprintf_duration(unsigned long t, bool calculated, FILE *fp)
 {
double duration = (double)t / NSEC_PER_MSEC;
size_t printed = fprintf(fp, "(");
 
-   if (duration >= 1.0)
+   if (!calculated)
+   printed += fprintf(fp, " ?   ");
+   else if (duration >= 1.0)
printed += color_fprintf(fp, PERF_COLOR_RED, "%6.3f ms", 
duration);
else if (duration >= 0.01)
printed += color_fprintf(fp, PERF_COLOR_YELLOW, "%6.3f ms", 
duration);
@@ -1028,13 +1037,27 @@ static bool trace__filter_duration(struct trace *trace, 
double t)
return t < (trace->duration_filter * NSEC_PER_MSEC);
 }
 
-static size_t trace__fprintf_tstamp(struct trace *trace, u64 tstamp, FILE *fp)
+static size_t __trace__fprintf_tstamp(struct trace *trace, u64 tstamp, FILE 
*fp)
 {
double ts = (double)(tstamp - trace->base_time) / NSEC_PER_MSEC;
 
return fprintf(fp, "%10.3f ", ts);
 }
 
+/*
+ * We're handling tstamp=0 as an undefined tstamp, i.e. like when we are
+ * using ttrace->entry_time for a thread that receives a sys_exit without
+ * first having received a sys_enter ("poll" issued before tracing session
+ * starts, lost sys_enter exit due to ring buffer overflow).
+ */
+static size_t trace__fprintf_tstamp(struct trace *trace, u64 tstamp, FILE *fp)
+{
+   if (tstamp > 0)
+   return __trace__fprintf_tstamp(trace, tstamp, fp);
+
+   return fprintf(fp, " ? ");
+}
+
 static bool done = false;
 static bool interrupted = false;
 
@@ -1045,10 +1068,10 @@ static void sig_handler(int sig)
 }
 
 static size_t trace__fprintf_entry_head(struct trace *trace, struct thread 
*thread,
-   u64 duration, u64 tstamp, FILE *fp)
+   u64 duration, bool duration_calculated, 
u64 tstamp, FILE *fp)
 {
size_t printed = trace__fprintf_tstamp(trace, tstamp, fp);
-   printed += fprintf_duration(duration, fp);
+   printed += fprintf_duration(duration, duration_calculated, fp);
 
if (trace->multiple_threads) {
if (trace->show_comm)
@@ -1450,7 +1473,7 @@ static int trace__printf_interrupted_entry(struct trace 
*trace, struct perf_samp
 
duration = sample->time - ttrace->entry_time;
 
-   printed  = trace__fprintf_entry_head(trace, trace->current, duration, 
ttrace->entry_time, trace->output);
+   printed  = trace__fprintf_entry_head(trace, trace->current, duration, 
true, ttrace->entry_time, trace->output);
printed += fprintf(trace->output, "%-70s) ...\n", ttrace->entry_str);
ttrace->entry_pending = false;
 
@@ -1497,7 +1520,7 @@ static int trace__sys_enter(struct trace *trace, struct 
perf_evsel *evsel,
 
if (sc->is_exit) {
if (!(trace->duration_filter || trace->summary_only || 
trace->min_stack)) {
-   trace__fprintf_entry_head(trace, thread, 1, 
ttrace->entry_time, trace->output);
+   trace__fprintf_entry_head(trace, thread, 0, false, 
ttrace->entry_time, trace->output);
fprintf(tra

[PATCH 8/9] perf tools: Do not fail in case of empty HOME env variable

From: Jiri Olsa 

Currently we fail in the following case:

  $ unset HOME
  $ ./perf record ls
  $ echo $?
  255

It's because the config code init fails due to a missing HOME variable
value. Fix this by skipping the user config init if there's no HOME
variable value.

Reported-by: Jan Stancek 
Signed-off-by: Jiri Olsa 
Cc: David Ahern 
Cc: Namhyung Kim 
Cc: Peter Zijlstra 
Link: http://lkml.kernel.org/r/20170330144637.7468-1-jo...@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo 
---
 tools/perf/util/config.c | 54 +++-
 1 file changed, 31 insertions(+), 23 deletions(-)

diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c
index 0c7d5a4975cd..7b01d59076d3 100644
--- a/tools/perf/util/config.c
+++ b/tools/perf/util/config.c
@@ -627,6 +627,8 @@ static int perf_config_set__init(struct perf_config_set 
*set)
 {
int ret = -1;
const char *home = NULL;
+   char *user_config;
+   struct stat st;
 
/* Setting $PERF_CONFIG makes perf read _only_ the given config file. */
if (config_exclusive_filename)
@@ -637,35 +639,41 @@ static int perf_config_set__init(struct perf_config_set 
*set)
}
 
home = getenv("HOME");
-   if (perf_config_global() && home) {
-   char *user_config = strdup(mkpath("%s/.perfconfig", home));
-   struct stat st;
 
-   if (user_config == NULL) {
-   warning("Not enough memory to process %s/.perfconfig, "
-   "ignoring it.", home);
-   goto out;
-   }
+   /*
+* Skip reading user config if:
+*   - there is no place to read it from (HOME)
+*   - we are asked not to (PERF_CONFIG_NOGLOBAL=1)
+*/
+   if (!home || !*home || !perf_config_global())
+   return 0;
 
-   if (stat(user_config, &st) < 0) {
-   if (errno == ENOENT)
-   ret = 0;
-   goto out_free;
-   }
+   user_config = strdup(mkpath("%s/.perfconfig", home));
+   if (user_config == NULL) {
+   warning("Not enough memory to process %s/.perfconfig, "
+   "ignoring it.", home);
+   goto out;
+   }
+
+   if (stat(user_config, &st) < 0) {
+   if (errno == ENOENT)
+   ret = 0;
+   goto out_free;
+   }
 
-   ret = 0;
+   ret = 0;
 
-   if (st.st_uid && (st.st_uid != geteuid())) {
-   warning("File %s not owned by current user or root, "
-   "ignoring it.", user_config);
-   goto out_free;
-   }
+   if (st.st_uid && (st.st_uid != geteuid())) {
+   warning("File %s not owned by current user or root, "
+   "ignoring it.", user_config);
+   goto out_free;
+   }
+
+   if (st.st_size)
+   ret = perf_config_from_file(collect_config, user_config, set);
 
-   if (st.st_size)
-   ret = perf_config_from_file(collect_config, 
user_config, set);
 out_free:
-   free(user_config);
-   }
+   free(user_config);
 out:
return ret;
 }
-- 
2.9.3

[PATCH 7/9] tools include uapi: Grab copies of stat.h and fcntl.h

From: Arnaldo Carvalho de Melo 

We will need it to build tools/perf/trace/beauty/statx.h.

Cc: Adrian Hunter 
Cc: David Ahern 
Cc: Jiri Olsa 
Cc: Namhyung Kim 
Cc: Wang Nan 
Link: http://lkml.kernel.org/n/tip-nin41ve2fa63lrfbdr6x5...@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo 
---
 tools/include/linux/types.h  |   1 +
 tools/include/uapi/linux/fcntl.h |  72 
 tools/include/uapi/linux/stat.h  | 176 +++
 tools/perf/MANIFEST  |   2 +
 tools/perf/check-headers.sh  |   2 +
 5 files changed, 253 insertions(+)
 create mode 100644 tools/include/uapi/linux/fcntl.h
 create mode 100644 tools/include/uapi/linux/stat.h

diff --git a/tools/include/linux/types.h b/tools/include/linux/types.h
index c24b3e3ae296..77a28a26a670 100644
--- a/tools/include/linux/types.h
+++ b/tools/include/linux/types.h
@@ -7,6 +7,7 @@
 
 #define __SANE_USERSPACE_TYPES__   /* For PPC64, to get LL64 types */
 #include 
+#include 
 
 struct page;
 struct kmem_cache;
diff --git a/tools/include/uapi/linux/fcntl.h b/tools/include/uapi/linux/fcntl.h
new file mode 100644
index ..813afd6eee71
--- /dev/null
+++ b/tools/include/uapi/linux/fcntl.h
@@ -0,0 +1,72 @@
+#ifndef _UAPI_LINUX_FCNTL_H
+#define _UAPI_LINUX_FCNTL_H
+
+#include 
+
+#define F_SETLEASE (F_LINUX_SPECIFIC_BASE + 0)
+#define F_GETLEASE (F_LINUX_SPECIFIC_BASE + 1)
+
+/*
+ * Cancel a blocking posix lock; internal use only until we expose an
+ * asynchronous lock api to userspace:
+ */
+#define F_CANCELLK (F_LINUX_SPECIFIC_BASE + 5)
+
+/* Create a file descriptor with FD_CLOEXEC set. */
+#define F_DUPFD_CLOEXEC(F_LINUX_SPECIFIC_BASE + 6)
+
+/*
+ * Request nofications on a directory.
+ * See below for events that may be notified.
+ */
+#define F_NOTIFY   (F_LINUX_SPECIFIC_BASE+2)
+
+/*
+ * Set and get of pipe page size array
+ */
+#define F_SETPIPE_SZ   (F_LINUX_SPECIFIC_BASE + 7)
+#define F_GETPIPE_SZ   (F_LINUX_SPECIFIC_BASE + 8)
+
+/*
+ * Set/Get seals
+ */
+#define F_ADD_SEALS(F_LINUX_SPECIFIC_BASE + 9)
+#define F_GET_SEALS(F_LINUX_SPECIFIC_BASE + 10)
+
+/*
+ * Types of seals
+ */
+#define F_SEAL_SEAL0x0001  /* prevent further seals from being set */
+#define F_SEAL_SHRINK  0x0002  /* prevent file from shrinking */
+#define F_SEAL_GROW0x0004  /* prevent file from growing */
+#define F_SEAL_WRITE   0x0008  /* prevent writes */
+/* (1U << 31) is reserved for signed error codes */
+
+/*
+ * Types of directory notifications that may be requested.
+ */
+#define DN_ACCESS  0x0001  /* File accessed */
+#define DN_MODIFY  0x0002  /* File modified */
+#define DN_CREATE  0x0004  /* File created */
+#define DN_DELETE  0x0008  /* File removed */
+#define DN_RENAME  0x0010  /* File renamed */
+#define DN_ATTRIB  0x0020  /* File changed attibutes */
+#define DN_MULTISHOT   0x8000  /* Don't remove notifier */
+
+#define AT_FDCWD   -100/* Special value used to indicate
+   openat should use the current
+   working directory. */
+#define AT_SYMLINK_NOFOLLOW0x100   /* Do not follow symbolic links.  */
+#define AT_REMOVEDIR   0x200   /* Remove directory instead of
+   unlinking file.  */
+#define AT_SYMLINK_FOLLOW  0x400   /* Follow symbolic links.  */
+#define AT_NO_AUTOMOUNT0x800   /* Suppress terminal automount 
traversal */
+#define AT_EMPTY_PATH  0x1000  /* Allow empty relative pathname */
+
+#define AT_STATX_SYNC_TYPE 0x6000  /* Type of synchronisation required 
from statx() */
+#define AT_STATX_SYNC_AS_STAT  0x  /* - Do whatever stat() does */
+#define AT_STATX_FORCE_SYNC0x2000  /* - Force the attributes to be sync'd 
with the server */
+#define AT_STATX_DONT_SYNC 0x4000  /* - Don't sync attributes with the 
server */
+
+
+#endif /* _UAPI_LINUX_FCNTL_H */
diff --git a/tools/include/uapi/linux/stat.h b/tools/include/uapi/linux/stat.h
new file mode 100644
index ..51a6b86e3700
--- /dev/null
+++ b/tools/include/uapi/linux/stat.h
@@ -0,0 +1,176 @@
+#ifndef _UAPI_LINUX_STAT_H
+#define _UAPI_LINUX_STAT_H
+
+#include 
+
+#if defined(__KERNEL__) || !defined(__GLIBC__) || (__GLIBC__ < 2)
+
+#define S_IFMT  0017
+#define S_IFSOCK 014
+#define S_IFLNK 012
+#define S_IFREG  010
+#define S_IFBLK  006
+#define S_IFDIR  004
+#define S_IFCHR  002
+#define S_IFIFO  001
+#define S_ISUID  0004000
+#define S_ISGID  0002000
+#define S_ISVTX  0001000
+
+#define S_ISLNK(m) (((m) & S_IFMT) == S_IFLNK)
+#define S_ISREG(m) (((m) & S_IFMT) == S_IFREG)
+#define S_ISDIR(m) (((m) & S_IFMT) == S_IFDIR)
+#define S_ISCHR(m) (((m) & S_IFMT) == S_IFCHR)
+#define S_ISBLK(m) (((m) & S_IFMT) == S_IFBLK)
+#define S_ISFIFO(m)(((m) & S_IFMT) == S_IFIFO)
+#defin

[GIT PULL 0/9] perf/core improvements and fixes

Hi Ingo,

Please consider pulling,

- Arnaldo

Test results at the end of this message, as usual.

The following changes since commit 3906a13a6b4e78fbc0def03a808f091f0dff1b44:

  Merge tag 'perf-core-for-mingo-4.12-20170327' of 
git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux into perf/core 
(2017-03-28 07:44:43 +0200)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git 
tags/perf-core-for-mingo-4.12-20170331

for you to fetch changes up to fd5cead23f54697310bd565aa2a23ae5128080a0:

  perf trace: Beautify statx syscall 'flag' and 'mask' arguments (2017-03-31 
14:42:31 -0300)


perf/core improvements and fixes:

New features:

- Beautify the statx syscall arguments in 'perf trace' (Arnaldo Carvalho de 
Melo)

e.g.:

  System wide strace like session:

  # trace -e statx
   16612.967 ( 0.028 ms): statx/4562 statx(dfd: CWD, filename: /tmp/statx, 
flags: SYMLINK_NOFOLLOW, mask: 
TYPE|MODE|NLINK|UID|GID|ATIME|MTIME|CTIME|INO|SIZE|BLOCKS|BTIME, buffer: 
0x7ffef195d660) = 0
   36050.891 ( 0.007 ms): statx/4576 statx(dfd: CWD, filename: /etc/passwd, 
flags: SYMLINK_NOFOLLOW|STATX_DONT_SYNC, mask: BTIME, buffer: 0x7ffda9bf50f0) = 0
  ^C#

User visible:

- Handle unpaired raw_syscalls:sys_exit events in 'perf trace', i.e. we
  shouldn't try to calculate duration or print the timestamp for a missing
  matching raw_syscalls:sys_enter (Arnaldo Carvalho de Melo)

- Do not print "cycles: 0" in perf report LBR lines in platforms not
  supporting 'cycles', such as Intel's Broadwell (Jin Yao)

- Handle missing $HOME env var (Jiri Olsa)

- Map 8-bit registers (al, bl, etc), not supported in uprobes_events, to
  the next best thing (ax, bx, etc) supported (Ravi Bangoria)

Signed-off-by: Arnaldo Carvalho de Melo 


Arnaldo Carvalho de Melo (4):
  perf tools: Remove support for command aliases
  perf trace: Handle unpaired raw_syscalls:sys_exit event
  tools include uapi: Grab copies of stat.h and fcntl.h
  perf trace: Beautify statx syscall 'flag' and 'mask' arguments

Colin Ian King (1):
  perf utils: Fix spelling mistake: "Invalud" -> "Invalid"

Jin Yao (1):
  perf report: Drop cycles 0 for LBR print

Jiri Olsa (1):
  perf tools: Do not fail in case of empty HOME env variable

Ravi Bangoria (2):
  perf/sdt/x86: Add renaming logic for (missing) 8 bit registers
  perf/sdt/x86: Move OP parser to tools/perf/arch/x86/

 tools/include/linux/types.h   |   1 +
 tools/include/uapi/linux/fcntl.h  |  72 +
 tools/include/uapi/linux/stat.h   | 176 
 tools/perf/Build  |   1 +
 tools/perf/MANIFEST   |   2 +
 tools/perf/arch/x86/entry/syscalls/syscall_64.tbl |   1 +
 tools/perf/arch/x86/util/perf_regs.c  | 187 ++
 tools/perf/builtin-help.c |  13 --
 tools/perf/builtin-trace.c|  57 ---
 tools/perf/check-headers.sh   |   2 +
 tools/perf/perf.c |  97 +--
 tools/perf/trace/beauty/Build |   1 +
 tools/perf/trace/beauty/beauty.h  |  24 +++
 tools/perf/trace/beauty/statx.c   |  72 +
 tools/perf/util/Build |   1 -
 tools/perf/util/alias.c   |  78 -
 tools/perf/util/cache.h   |   1 -
 tools/perf/util/callchain.c   | 111 -
 tools/perf/util/config.c  |  54 ---
 tools/perf/util/help-unknown-cmd.c|   8 +-
 tools/perf/util/hist.c|   2 +-
 tools/perf/util/perf_regs.c   |   6 +-
 tools/perf/util/perf_regs.h   |  11 +-
 tools/perf/util/probe-file.c  | 132 +--
 24 files changed, 707 insertions(+), 403 deletions(-)
 create mode 100644 tools/include/uapi/linux/fcntl.h
 create mode 100644 tools/include/uapi/linux/stat.h
 create mode 100644 tools/perf/trace/beauty/Build
 create mode 100644 tools/perf/trace/beauty/beauty.h
 create mode 100644 tools/perf/trace/beauty/statx.c
 delete mode 100644 tools/perf/util/alias.c

Test results:

The first ones are container (docker) based builds of tools/perf with and
without libelf support, objtool where it is supported and samples/bpf/, ditto.
Where clang is available, it is also used to build perf with/without libelf.

For this specific pull request the samples/bpf/ was disabled, as 'make 
headers_install'
is failing with the following error, in this case in

[PATCH 6/9] perf utils: Fix spelling mistake: "Invalud" -> "Invalid"

From: Colin Ian King 

Trivial fix to spelling mistake in pr_debug message.

Signed-off-by: Colin King 
Cc: Alexander Shishkin 
Cc: Jiri Olsa 
Cc: Namhyung Kim 
Cc: Krister Johansen 
Cc: Peter Zijlstra 
Cc: kernel-janit...@vger.kernel.org
Link: http://lkml.kernel.org/r/20170330095440.19444-1-colin.k...@canonical.com
Signed-off-by: Arnaldo Carvalho de Melo 
---
 tools/perf/util/hist.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index 3c4d4d00cb2c..61bf304206fd 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -2459,7 +2459,7 @@ int parse_filter_percentage(const struct option *opt 
__maybe_unused,
else if (!strcmp(arg, "absolute"))
symbol_conf.filter_relative = false;
else {
-   pr_debug("Invalud percentage: %s\n", arg);
+   pr_debug("Invalid percentage: %s\n", arg);
return -1;
}
 
-- 
2.9.3

[PATCH 3/9] perf/sdt/x86: Move OP parser to tools/perf/arch/x86/

From: Ravi Bangoria 

SDT marker argument is in N@OP format. N is the size of argument and OP
is the actual assembly operand. OP is arch dependent component and hence
it's parsing logic also should be placed under tools/perf/arch/.

Signed-off-by: Ravi Bangoria 
Acked-by: Masami Hiramatsu 
Cc: Alexander Shishkin 
Cc: Alexis Berlemont 
Cc: Hemant Kumar 
Cc: Michael Ellerman 
Cc: Naveen N. Rao 
Cc: Peter Zijlstra 
Link: 
http://lkml.kernel.org/r/20170328094754.3156-3-ravi.bango...@linux.vnet.ibm.com
Signed-off-by: Arnaldo Carvalho de Melo 
---
 tools/perf/arch/x86/util/perf_regs.c | 179 ---
 tools/perf/util/perf_regs.c  |   6 +-
 tools/perf/util/perf_regs.h  |  11 ++-
 tools/perf/util/probe-file.c | 132 --
 4 files changed, 194 insertions(+), 134 deletions(-)

diff --git a/tools/perf/arch/x86/util/perf_regs.c 
b/tools/perf/arch/x86/util/perf_regs.c
index fa1fd196837d..3bf3548c5e2d 100644
--- a/tools/perf/arch/x86/util/perf_regs.c
+++ b/tools/perf/arch/x86/util/perf_regs.c
@@ -1,8 +1,10 @@
 #include 
+#include 
 
 #include "../../perf.h"
 #include "../../util/util.h"
 #include "../../util/perf_regs.h"
+#include "../../util/debug.h"
 
 const struct sample_reg sample_reg_masks[] = {
SMPL_REG(AX, PERF_REG_X86_AX),
@@ -37,7 +39,7 @@ struct sdt_name_reg {
 #define SDT_NAME_REG(n, m) {.sdt_name = "%" #n, .uprobe_name = "%" #m}
 #define SDT_NAME_REG_END {.sdt_name = NULL, .uprobe_name = NULL}
 
-static const struct sdt_name_reg sdt_reg_renamings[] = {
+static const struct sdt_name_reg sdt_reg_tbl[] = {
SDT_NAME_REG(eax, ax),
SDT_NAME_REG(rax, ax),
SDT_NAME_REG(al,  ax),
@@ -95,45 +97,158 @@ static const struct sdt_name_reg sdt_reg_renamings[] = {
SDT_NAME_REG_END,
 };
 
-int sdt_rename_register(char **pdesc, char *old_name)
+/*
+ * Perf only supports OP which is in  +/-NUM(REG)  form.
+ * Here plus-minus sign, NUM and parenthesis are optional,
+ * only REG is mandatory.
+ *
+ * SDT events also supports indirect addressing mode with a
+ * symbol as offset, scaled mode and constants in OP. But
+ * perf does not support them yet. Below are few examples.
+ *
+ * OP with scaled mode:
+ * (%rax,%rsi,8)
+ * 10(%ras,%rsi,8)
+ *
+ * OP with indirect addressing mode:
+ * check_action(%rip)
+ * mp_+52(%rip)
+ * 44+mp_(%rip)
+ *
+ * OP with constant values:
+ * $0
+ * $123
+ * $-1
+ */
+#define SDT_OP_REGEX  "^([+\\-]?)([0-9]*)(\\(?)(%[a-z][a-z0-9]+)(\\)?)$"
+
+static regex_t sdt_op_regex;
+
+static int sdt_init_op_regex(void)
 {
-   const struct sdt_name_reg *rnames = sdt_reg_renamings;
-   char *new_desc, *old_desc = *pdesc;
-   size_t prefix_len, sdt_len, uprobe_len, old_desc_len, offset;
-   int ret = -1;
-
-   while (ret != 0 && rnames->sdt_name != NULL) {
-   sdt_len = strlen(rnames->sdt_name);
-   ret = strncmp(old_name, rnames->sdt_name, sdt_len);
-   rnames += !!ret;
-   }
+   static int initialized;
+   int ret = 0;
 
-   if (rnames->sdt_name == NULL)
+   if (initialized)
return 0;
 
-   sdt_len = strlen(rnames->sdt_name);
-   uprobe_len = strlen(rnames->uprobe_name);
-   old_desc_len = strlen(old_desc) + 1;
+   ret = regcomp(&sdt_op_regex, SDT_OP_REGEX, REG_EXTENDED);
+   if (ret < 0) {
+   pr_debug4("Regex compilation error.\n");
+   return ret;
+   }
 
-   new_desc = zalloc(old_desc_len + uprobe_len - sdt_len);
-   if (new_desc == NULL)
-   return -1;
+   initialized = 1;
+   return 0;
+}
 
-   /* Copy the chars before the register name (at least '%') */
-   prefix_len = old_name - old_desc;
-   memcpy(new_desc, old_desc, prefix_len);
+/*
+ * Max x86 register name length is 5(ex: %r15d). So, 6th char
+ * should always contain NULL. This helps to find register name
+ * length using strlen, insted of maintaing one more variable.
+ */
+#define SDT_REG_NAME_SIZE  6
 
-   /* Copy the new register name */
-   memcpy(new_desc + prefix_len, rnames->uprobe_name, uprobe_len);
+/*
+ * The uprobe parser does not support all gas register names;
+ * so, we have to replace them (ex. for x86_64: %rax -> %ax).
+ * Note: If register does not require renaming, just copy
+ * paste as it is, but don't leave it empty.
+ */
+static void sdt_rename_register(char *sdt_reg, int sdt_len, char *uprobe_reg)
+{
+   int i = 0;
 
-   /* Copy the chars after the register name (if need be) */
-   offset = prefix_len + sdt_len;
-   if (offset < old_desc_len)
-   memcpy(new_desc + prefix_len + uprobe_len,
-   old_desc + offset, old_desc_len - offset);
+   for (i = 0; sdt_reg_tbl[i].sdt_name != NULL; i++) {
+   if (!strncmp(sdt_reg_tbl[i].sdt_name, sdt_reg, sdt_len)) {
+   strcpy(uprobe_reg, sdt_reg_tbl[i].uprobe_name);
+

[PATCH 2/9] perf/sdt/x86: Add renaming logic for (missing) 8 bit registers

From: Ravi Bangoria 

I found couple of events using al, bl, cl and dl registers for argument.
These are not directly accepted by uprobe_events and thus needs to be
mapped to ax, bx, cx and dx respectively.

Few ex,

  /usr/bin/qemu-system-s390x
css_adapter_interrupt: 1@%bl
css_chpid_add: 1@%cl 1@%sil 1@%dl
dma_bdrv_io: 8@%rbx 8@%rbp -8@%r14 1@%al

  /usr/bin/postgres
buffer__read__done: ... -1@-bash -1@%al
buffer__read__start: ... -1@%al

I don't find any sdt events using ah, bh,... registers. But I also don't
see any reason to not use them, so there might be rare events using
these registers, and if so, perf should have a renaming logic for them
too.

Signed-off-by: Ravi Bangoria 
Acked-by: Masami Hiramatsu 
Cc: Alexander Shishkin 
Cc: Alexis Berlemont 
Cc: Hemant Kumar 
Cc: Michael Ellerman 
Cc: Naveen N. Rao 
Cc: Peter Zijlstra 
Link: 
http://lkml.kernel.org/r/20170328094754.3156-2-ravi.bango...@linux.vnet.ibm.com
Signed-off-by: Arnaldo Carvalho de Melo 
---
 tools/perf/arch/x86/util/perf_regs.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/tools/perf/arch/x86/util/perf_regs.c 
b/tools/perf/arch/x86/util/perf_regs.c
index d8a8dcf761f7..fa1fd196837d 100644
--- a/tools/perf/arch/x86/util/perf_regs.c
+++ b/tools/perf/arch/x86/util/perf_regs.c
@@ -40,12 +40,20 @@ struct sdt_name_reg {
 static const struct sdt_name_reg sdt_reg_renamings[] = {
SDT_NAME_REG(eax, ax),
SDT_NAME_REG(rax, ax),
+   SDT_NAME_REG(al,  ax),
+   SDT_NAME_REG(ah,  ax),
SDT_NAME_REG(ebx, bx),
SDT_NAME_REG(rbx, bx),
+   SDT_NAME_REG(bl,  bx),
+   SDT_NAME_REG(bh,  bx),
SDT_NAME_REG(ecx, cx),
SDT_NAME_REG(rcx, cx),
+   SDT_NAME_REG(cl,  cx),
+   SDT_NAME_REG(ch,  cx),
SDT_NAME_REG(edx, dx),
SDT_NAME_REG(rdx, dx),
+   SDT_NAME_REG(dl,  dx),
+   SDT_NAME_REG(dh,  dx),
SDT_NAME_REG(esi, si),
SDT_NAME_REG(rsi, si),
SDT_NAME_REG(sil, si),
-- 
2.9.3

[PATCH 1/9] perf tools: Remove support for command aliases

From: Arnaldo Carvalho de Melo 

This came from 'git', but isn't documented anywhere in
tools/perf/Documentation/, looks like baggage we can do without, ditch
it.

Cc: Adrian Hunter 
Cc: David Ahern 
Cc: Jiri Olsa 
Cc: Namhyung Kim 
Cc: Wang Nan 
Link: http://lkml.kernel.org/n/tip-e7uwkn60t4hmlnwj99ba4...@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo 
---
 tools/perf/builtin-help.c  | 13 -
 tools/perf/perf.c  | 97 +++---
 tools/perf/util/Build  |  1 -
 tools/perf/util/alias.c| 78 --
 tools/perf/util/cache.h|  1 -
 tools/perf/util/help-unknown-cmd.c |  8 +---
 6 files changed, 8 insertions(+), 190 deletions(-)
 delete mode 100644 tools/perf/util/alias.c

diff --git a/tools/perf/builtin-help.c b/tools/perf/builtin-help.c
index 7ae238929e95..1eec96a0fa67 100644
--- a/tools/perf/builtin-help.c
+++ b/tools/perf/builtin-help.c
@@ -301,12 +301,6 @@ void list_common_cmds_help(void)
}
 }
 
-static int is_perf_command(const char *s)
-{
-   return is_in_cmdlist(&main_cmds, s) ||
-   is_in_cmdlist(&other_cmds, s);
-}
-
 static const char *cmd_to_page(const char *perf_cmd)
 {
char *s;
@@ -446,7 +440,6 @@ int cmd_help(int argc, const char **argv)
"perf help [--all] [--man|--web|--info] [command]",
NULL
};
-   const char *alias;
int rc;
 
load_command_list("perf-", &main_cmds, &other_cmds);
@@ -472,12 +465,6 @@ int cmd_help(int argc, const char **argv)
return 0;
}
 
-   alias = alias_lookup(argv[0]);
-   if (alias && !is_perf_command(argv[0])) {
-   printf("`perf %s' is aliased to `%s'\n", argv[0], alias);
-   return 0;
-   }
-
switch (help_format) {
case HELP_FORMAT_MAN:
rc = show_man_page(argv[0]);
diff --git a/tools/perf/perf.c b/tools/perf/perf.c
index 4b283d18e158..9217f2227f3d 100644
--- a/tools/perf/perf.c
+++ b/tools/perf/perf.c
@@ -267,71 +267,6 @@ static int handle_options(const char ***argv, int *argc, 
int *envchanged)
return handled;
 }
 
-static int handle_alias(int *argcp, const char ***argv)
-{
-   int envchanged = 0, ret = 0, saved_errno = errno;
-   int count, option_count;
-   const char **new_argv;
-   const char *alias_command;
-   char *alias_string;
-
-   alias_command = (*argv)[0];
-   alias_string = alias_lookup(alias_command);
-   if (alias_string) {
-   if (alias_string[0] == '!') {
-   if (*argcp > 1) {
-   struct strbuf buf;
-
-   if (strbuf_init(&buf, PATH_MAX) < 0 ||
-   strbuf_addstr(&buf, alias_string) < 0 ||
-   sq_quote_argv(&buf, (*argv) + 1,
- PATH_MAX) < 0)
-   die("Failed to allocate memory.");
-   free(alias_string);
-   alias_string = buf.buf;
-   }
-   ret = system(alias_string + 1);
-   if (ret >= 0 && WIFEXITED(ret) &&
-   WEXITSTATUS(ret) != 127)
-   exit(WEXITSTATUS(ret));
-   die("Failed to run '%s' when expanding alias '%s'",
-   alias_string + 1, alias_command);
-   }
-   count = split_cmdline(alias_string, &new_argv);
-   if (count < 0)
-   die("Bad alias.%s string", alias_command);
-   option_count = handle_options(&new_argv, &count, &envchanged);
-   if (envchanged)
-   die("alias '%s' changes environment variables\n"
-"You can use '!perf' in the alias to do this.",
-alias_command);
-   memmove(new_argv - option_count, new_argv,
-   count * sizeof(char *));
-   new_argv -= option_count;
-
-   if (count < 1)
-   die("empty alias for %s", alias_command);
-
-   if (!strcmp(alias_command, new_argv[0]))
-   die("recursive alias: %s", alias_command);
-
-   new_argv = realloc(new_argv, sizeof(char *) *
-   (count + *argcp + 1));
-   /* insert after command name */
-   memcpy(new_argv + count, *argv + 1, sizeof(char *) * *argcp);
-   new_argv[count + *argcp] = NULL;
-
-   *argv = new_argv;
-   *argcp += count - 1;
-
-   ret = 1;
-   }
-
-   errno = saved_errno;
-
-   return ret;
-}
-
 #define RUN_SETUP  (1<<0)
 #define USE_PAGER  (1<<1)
 
@@ -455,25 +390,12 @@ static void execv_da

[PATCH 4/9] perf report: Drop cycles 0 for LBR print

From: Jin Yao 

For some platforms, for example Broadwell, it doesn't support cycles
for LBR. But the perf always prints cycles:0, it's not necessary.

The patch refactors the LBR info print code and drops the cycles:0.

For example: perf report --branch-history --no-children --stdio

On Broadwell:
--0.91%--__random_r random_r.c:394 (iterations:2)
  __random_r random_r.c:360 (predicted:0.0%)
  __random_r random_r.c:380 (predicted:0.0%)
  __random_r random_r.c:357

On Skylake:
--1.07%--main div.c:39 (predicted:52.4% cycles:1 iterations:17)
  main div.c:44 (predicted:52.4% cycles:1)
  main div.c:42 (cycles:2)
  compute_flag div.c:28 (cycles:2)
  compute_flag div.c:27 (cycles:1)
  rand rand.c:28 (cycles:1)
  rand rand.c:28 (cycles:1)
  __random random.c:298 (cycles:1)
  __random random.c:297 (cycles:1)
  __random random.c:295 (cycles:1)
  __random random.c:295 (cycles:1)
  __random random.c:295 (cycles:1)

Signed-off-by: Yao Jin 
Reviewed-by: Andi Kleen 
Cc: Jiri Olsa 
Cc: Kan Liang 
Link: 
http://lkml.kernel.org/r/1489046786-10061-1-git-send-email-yao@linux.intel.com
Signed-off-by: Arnaldo Carvalho de Melo 

Signed-off-by: Arnaldo Carvalho de Melo 
---
 tools/perf/util/callchain.c | 111 +---
 1 file changed, 74 insertions(+), 37 deletions(-)

diff --git a/tools/perf/util/callchain.c b/tools/perf/util/callchain.c
index d78776a20e80..3cea1fb5404b 100644
--- a/tools/perf/util/callchain.c
+++ b/tools/perf/util/callchain.c
@@ -1105,63 +1105,100 @@ int callchain_branch_counts(struct callchain_root 
*root,
  cycles_count);
 }
 
-static int callchain_counts_printf(FILE *fp, char *bf, int bfsize,
-  u64 branch_count, u64 predicted_count,
-  u64 abort_count, u64 cycles_count,
-  u64 iter_count, u64 samples_count)
+static int counts_str_build(char *bf, int bfsize,
+u64 branch_count, u64 predicted_count,
+u64 abort_count, u64 cycles_count,
+u64 iter_count, u64 samples_count)
 {
double predicted_percent = 0.0;
const char *null_str = "";
char iter_str[32];
-   char *str;
-   u64 cycles = 0;
-
-   if (branch_count == 0) {
-   if (fp)
-   return fprintf(fp, " (calltrace)");
+   char cycle_str[32];
+   char *istr, *cstr;
+   u64 cycles;
 
+   if (branch_count == 0)
return scnprintf(bf, bfsize, " (calltrace)");
-   }
+
+   cycles = cycles_count / branch_count;
 
if (iter_count && samples_count) {
-   scnprintf(iter_str, sizeof(iter_str),
-", iterations:%" PRId64 "",
-iter_count / samples_count);
-   str = iter_str;
+   if (cycles > 0)
+   scnprintf(iter_str, sizeof(iter_str),
+" iterations:%" PRId64 "",
+iter_count / samples_count);
+   else
+   scnprintf(iter_str, sizeof(iter_str),
+"iterations:%" PRId64 "",
+iter_count / samples_count);
+   istr = iter_str;
+   } else
+   istr = (char *)null_str;
+
+   if (cycles > 0) {
+   scnprintf(cycle_str, sizeof(cycle_str),
+ "cycles:%" PRId64 "", cycles);
+   cstr = cycle_str;
} else
-   str = (char *)null_str;
+   cstr = (char *)null_str;
 
predicted_percent = predicted_count * 100.0 / branch_count;
-   cycles = cycles_count / branch_count;
 
-   if ((predicted_percent >= 100.0) && (abort_count == 0)) {
-   if (fp)
-   return fprintf(fp, " (cycles:%" PRId64 "%s)",
-  cycles, str);
+   if ((predicted_count == branch_count) && (abort_count == 0)) {
+   if ((cycles > 0) || (istr != (char *)null_str))
+   return scnprintf(bf, bfsize, " (%s%s)", cstr, istr);
+   else
+   return scnprintf(bf, bfsize, "%s", (char *)null_str);
+   }
 
-   return scnprintf(bf, bfsize, " (cycles:%" PRId64 "%s)",
-cycles, str);
+   if ((predicted_count < branch_count) && (abort_count == 0)) {
+   if ((cycles > 0) || (istr != (char *)null_str))
+   return scnprintf(bf, bfsize,
+   " (predicted:%.1f%% %s%s)",
+   predicted_percent, cstr, istr);
+   else {
+   return scnprintf(bf, bfsize,
+   " (predi

[PATCH 9/9] perf trace: Beautify statx syscall 'flag' and 'mask' arguments

From: Arnaldo Carvalho de Melo 

To test it, build samples/statx/test_statx, which I did as:

  $ make headers_install
  $ cc -I ~/git/linux/usr/include samples/statx/test-statx.c -o /tmp/statx

And then use perf trace on it:

  # perf trace -e statx /tmp/statx /etc/passwd
  statx(/etc/passwd) = 0
  results=7ff
Size: 3496Blocks: 8  IO Block: 4096regular file
  Device: fd:00   Inode: 280156  Links: 1
  Access: (0644/-rw-r--r--)  Uid: 0   Gid: 0
  Access: 2017-03-29 16:01:01.650073438-0300
  Modify: 2017-03-10 16:25:14.156479354-0300
  Change: 2017-03-10 16:25:14.171479328-0300
 0.000 ( 0.007 ms): statx/30648 statx(dfd: CWD, filename: 0x7ef503f4, 
flags: SYMLINK_NOFOLLOW, mask: 
TYPE|MODE|NLINK|UID|GID|ATIME|MTIME|CTIME|INO|SIZE|BLOCKS|BTIME, buffer: 
0x7fff7ef4eb10) = 0
  #

Using the test-stat.c options to change the mask:

  # perf trace -e statx /tmp/statx -O /etc/passwd > /dev/null
 0.000 ( 0.008 ms): statx/30745 statx(dfd: CWD, filename: 0x3a0753f4, 
flags: SYMLINK_NOFOLLOW, mask: BTIME, buffer: 0x7ffd3a0735c0) = 0
  #
  # perf trace -e statx /tmp/statx -A /etc/passwd > /dev/null
 0.000 ( 0.010 ms): statx/30757 statx(dfd: CWD, filename: 0xa94e63f4, 
flags: SYMLINK_NOFOLLOW|NO_AUTOMOUNT, mask: 
TYPE|MODE|NLINK|UID|GID|ATIME|MTIME|CTIME|INO|SIZE|BLOCKS|BTIME, buffer: 
0x7ffea94e49d0) = 0
  #
  # trace --no-inherit -e statx /tmp/statx -F /etc/passwd > /dev/null
 0.000 ( 0.011 ms): statx(dfd: CWD, filename: 0x3b02d3f3, flags: 
SYMLINK_NOFOLLOW|STATX_FORCE_SYNC, mask: 
TYPE|MODE|NLINK|UID|GID|ATIME|MTIME|CTIME|INO|SIZE|BLOCKS|BTIME, buffer: 
0x7ffd3b02c850) = 0
  #
  # trace --no-inherit -e statx /tmp/statx -F -L /etc/passwd > /dev/null
 0.000 ( 0.008 ms): statx(dfd: CWD, filename: 0x15cff3f3, flags: 
STATX_FORCE_SYNC, mask: 
TYPE|MODE|NLINK|UID|GID|ATIME|MTIME|CTIME|INO|SIZE|BLOCKS|BTIME, buffer: 
0x7fff15cfdda0) = 0
  #
  # trace --no-inherit -e statx /tmp/statx -D -O /etc/passwd > /dev/null
 0.000 ( 0.009 ms): statx(dfd: CWD, filename: 0xfa37f3f3, flags: 
SYMLINK_NOFOLLOW|STATX_DONT_SYNC, mask: BTIME, buffer: 0x7a37da20) = 0
  #

Adding a probe to get the filename collected as well:

  # perf probe 'vfs_getname=getname_flags:72 pathname=result->name:string'
  Added new event:
probe:vfs_getname(on getname_flags:72 with pathname=result->name:string)

  You can now use it in all perf tools, such as:

  perf record -e probe:vfs_getname -aR sleep 1

  # trace --no-inherit -e statx /tmp/statx -D -O /etc/passwd > /dev/null
 0.169 ( 0.007 ms): statx(dfd: CWD, filename: /etc/passwd, flags: 
SYMLINK_NOFOLLOW|STATX_DONT_SYNC, mask: BTIME, buffer: 0x7ffda9bf50f0) = 0
  #

Same technique could be used to collect and beautify the result put in
the 'buffer' argument.

Finally do a system wide 'perf trace' session looking for any use of statx,
then run the test proggie with various flags:

  # trace -e statx
   16612.967 ( 0.028 ms): statx/4562 statx(dfd: CWD, filename: /tmp/statx, 
flags: SYMLINK_NOFOLLOW, mask: 
TYPE|MODE|NLINK|UID|GID|ATIME|MTIME|CTIME|INO|SIZE|BLOCKS|BTIME, buffer: 
0x7ffef195d660) = 0
   33064.447 ( 0.011 ms): statx/4569 statx(dfd: CWD, filename: /tmp/statx, 
flags: SYMLINK_NOFOLLOW|STATX_FORCE_SYNC, mask: 
TYPE|MODE|NLINK|UID|GID|ATIME|MTIME|CTIME|INO|SIZE|BLOCKS|BTIME, buffer: 
0x7ffc5484c790) = 0
   36050.891 ( 0.023 ms): statx/4576 statx(dfd: CWD, filename: /tmp/statx, 
flags: SYMLINK_NOFOLLOW, mask: BTIME, buffer: 0x7ffeb18b66e0) = 0
   38039.889 ( 0.023 ms): statx/4584 statx(dfd: CWD, filename: /tmp/statx, 
flags: SYMLINK_NOFOLLOW, mask: 
TYPE|MODE|NLINK|UID|GID|ATIME|MTIME|CTIME|INO|SIZE|BLOCKS|BTIME, buffer: 
0x7fff1db0ea90) = 0
  ^C#

This one also starts moving the beautifiers from files directly included
in builtin-trace.c to separate objects + a beauty.h header with
prototypes, so that we can add test cases in tools/perf/tests/ to fire
syscalls with various arguments and then get them intercepted as
syscalls:sys_enter_foo or raw_syscalls:sys_enter + sys_exit to then
format and check that the formatted output is the one we expect.

Cc: Adrian Hunter 
Cc: Al Viro 
Cc: David Ahern 
Cc: David Howells 
Cc: Jiri Olsa 
Cc: Namhyung Kim 
Cc: Wang Nan 
Link: http://lkml.kernel.org/n/tip-xvzw8eynffvez5czyzidh...@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo 
---
 tools/perf/Build  |  1 +
 tools/perf/arch/x86/entry/syscalls/syscall_64.tbl |  1 +
 tools/perf/builtin-trace.c| 14 ++---
 tools/perf/trace/beauty/Build |  1 +
 tools/perf/trace/beauty/beauty.h  | 24 
 tools/perf/trace/beauty/statx.c   | 72 +++
 6 files changed, 104 insertions(+), 9 deletions(-)
 create mode 100644 tools/perf/trace/beauty/Build
 create mode 100644 tools/perf/trace/beauty/beauty.h
 create mode 100644 tools/perf/trace/beauty/statx.c

diff --git a/tools/perf/Build b/tools/perf/Build
index

Re: [PATCH] dm ioctl: Remove double parentheses

On Fri, 2017-03-31 at 18:50 -0700, Matthias Kaehlcke wrote:
> El Thu, Mar 16, 2017 at 09:48:30AM -0700 Matthias Kaehlcke ha dit:
> 
> > The extra pair of parantheses is not needed and causes clang to generate
> > the following warning:
> > 
> > drivers/md/dm-ioctl.c:1776:11: error: equality comparison with extraneous 
> > parentheses [-Werror,-Wparentheses-equality]
> > if ((cmd == DM_DEV_CREATE_CMD)) {
> >  ^~~~
> > drivers/md/dm-ioctl.c:1776:11: note: remove extraneous parentheses around 
> > the comparison to silence this warning
> > if ((cmd == DM_DEV_CREATE_CMD)) {
> > ~^   ~
> > drivers/md/dm-ioctl.c:1776:11: note: use '=' to turn this equality 
> > comparison into an assignment
> > if ((cmd == DM_DEV_CREATE_CMD)) {

There are dozens of these comparisons in the kernel.
Are you fixing them all or just this one?

[PATCH 001/001] drivers/staging/vt6656/main_usb.c: checkpatch warning

Replace string with formatted arguments in the dev_warn() call. It removes
the checkpatch warning:

WARNING: Prefer using "%s", __func__ to embedded function names
#417: FILE: main_usb.c:417:
+"usb_device_reset fail status=%d\n", status);

total: 0 errors, 1 warnings, 1058 lines checked

And after fix:

main_usb.c has no obvious style problems and is ready for submission.

Signed-off-by: Chewie Lin 
---
 drivers/staging/vt6656/main_usb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/vt6656/main_usb.c 
b/drivers/staging/vt6656/main_usb.c
index 9e074e9..2d9e7af 100644
--- a/drivers/staging/vt6656/main_usb.c
+++ b/drivers/staging/vt6656/main_usb.c
@@ -414,7 +414,7 @@ static void usb_device_reset(struct vnt_private *priv)
status = usb_reset_device(priv->usb);
if (status)
dev_warn(&priv->usb->dev,
-"usb_device_reset fail status=%d\n", status);
+"%s=%d\n", "usb_device_reset fail status", status);
 }
 
 static void vnt_free_int_bufs(struct vnt_private *priv)
-- 
2.10.0

Re: [PATCH] x86/intel_rdt: Add cpus_list rdtgroup file

2017-03-31 Thread Fenghua Yu

On Thu, Mar 30, 2017 at 04:59:47PM +0200, Jiri Olsa wrote:
> On Wed, Mar 29, 2017 at 06:16:00PM +0200, Jiri Olsa wrote:
> > On Wed, Mar 29, 2017 at 09:08:26AM -0700, Fenghua Yu wrote:
> > > On Wed, Mar 29, 2017 at 05:09:48PM +0200, Jiri Olsa wrote:
> > > > While playing with the resctrl interface I found it much
> > > > easier to deal with cpumask list rather than just regular
> > > > cpumask.
> > > 
> > > Could you please explain specifically why and when it's easier
> > > to deal with cpumask list? In programming cases, cpumask
> > > and cpumask list are almost same. And people are working
> > > on higher level tools to control resctrl. The tools can
> > > hide detailed regular cpumask or cpumask list and user
> > > doesn't need to care lower level format of cpumask. So
> > > is it really useful to add cpus_list?
> > 
> > 
> > well I'm not aware about any such tool so I used resctrl
> > interface directly, and in that case it was much simpler
> 
> is there any tool available for this actualy?

Here is a tool created by Marcelo:
https://lwn.net/Articles/710514/

Thanks.

-Fenghua

[PATCH] eudyptula challenge

Hi all:

Better and improved changelog version 3. 

I'm submitting this patch as part of Eudyptula challenge to fix a
coding style problem.  Thanks for taking time on this trivial patch.

sl424

Chewie Lin (1):
drivers/staging/vt6656/main_usb.c: checkpatch warning

 drivers/staging/vt6656/main_usb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
2.10.0

Re: [RFC 1/8] Introduce Peer-to-Peer memory (p2pmem) device

2017-03-31 Thread Logan Gunthorpe



On 31/03/17 05:51 PM, Sinan Kaya wrote:
> You can put a restriction with DMI/SMBIOS such that all devices from 2016
> work else they belong to blacklist.

How do you get a manufacturing date for a given device within the
kernel? Is this actually something generically available?

Logan

[GIT PULL] nfsd bugfixes for 4.11

2017-03-31 Thread J. Bruce Fields

Please pull nfsd bugfixes from

  git://linux-nfs.org/~bfields/linux.git tags/nfsd-4.11-1

The restriction of NFSv4 to TCP went overboard and also broke the
backchannel; fix.  Also some minor refinements to the nfsd
version-setting interface that we'd like to get fixed before release.

--b.

Chuck Lever (1):
  svcrdma: set XPT_CONG_CTRL flag for bc xprt

Kinglong Mee (2):
  SUNRPC/backchanel: set XPT_CONG_CTRL flag for bc xprt
  nfsd: map the ENOKEY to nfserr_perm for avoiding warning

NeilBrown (3):
  NFSD: further refinement of content of /proc/fs/nfsd/versions
  NFSD: fix nfsd_minorversion(.., NFSD_AVAIL)
  NFSD: fix nfsd_reset_versions for NFSv4.

 fs/nfsd/nfsctl.c | 43 
 fs/nfsd/nfsproc.c|  1 +
 fs/nfsd/nfssvc.c | 28 ++---
 net/sunrpc/svcsock.c |  1 +
 net/sunrpc/xprtrdma/svc_rdma_transport.c |  1 +
 5 files changed, 49 insertions(+), 25 deletions(-)

Re: [PATCH] dm ioctl: Remove double parentheses

El Thu, Mar 16, 2017 at 09:48:30AM -0700 Matthias Kaehlcke ha dit:

> The extra pair of parantheses is not needed and causes clang to generate
> the following warning:
> 
> drivers/md/dm-ioctl.c:1776:11: error: equality comparison with extraneous 
> parentheses [-Werror,-Wparentheses-equality]
> if ((cmd == DM_DEV_CREATE_CMD)) {
>  ^~~~
> drivers/md/dm-ioctl.c:1776:11: note: remove extraneous parentheses around the 
> comparison to silence this warning
> if ((cmd == DM_DEV_CREATE_CMD)) {
> ~^   ~
> drivers/md/dm-ioctl.c:1776:11: note: use '=' to turn this equality comparison 
> into an assignment
> if ((cmd == DM_DEV_CREATE_CMD)) {
>  ^~
>  =
> 
> Also remove another double parentheses that don't cause a warning.
> 
> Signed-off-by: Matthias Kaehlcke 
> ---
>  drivers/md/dm-ioctl.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/md/dm-ioctl.c b/drivers/md/dm-ioctl.c
> index a5a9b17f0f7f..9360036bf6d3 100644
> --- a/drivers/md/dm-ioctl.c
> +++ b/drivers/md/dm-ioctl.c
> @@ -1777,12 +1777,12 @@ static int validate_params(uint cmd, struct dm_ioctl 
> *param)
>   cmd == DM_LIST_VERSIONS_CMD)
>   return 0;
>  
> - if ((cmd == DM_DEV_CREATE_CMD)) {
> + if (cmd == DM_DEV_CREATE_CMD) {
>   if (!*param->name) {
>   DMWARN("name not supplied when creating device");
>   return -EINVAL;
>   }
> - } else if ((*param->uuid && *param->name)) {
> + } else if (*param->uuid && *param->name) {
>   DMWARN("only supply one of name or uuid, cmd(%u)", cmd);
>   return -EINVAL;
>   }

Ping? Any feedback on this patch?

Cheers

Matthias

RE: [PATCH 0/3]measure SMI cost

2017-03-31 Thread Liang, Kan



> On Thu, Mar 23, 2017 at 11:25 AM,   wrote:
> > From: Kan Liang 
> >
> > Currently, there is no way to measure the time cost in System
> > management mode (SMM) by perf.
> >
> > Intel perfmon supports FREEZE_WHILE_SMM bit in IA32_DEBUGCTL. Once
> it
> > sets, the PMU core counters will freeze on SMI handler. But it will
> > not have an effect on free running counters. E.g. APERF counter.
> > The cost of SMI can be measured by (aperf - cycles).
> >
> > A new sysfs entry /sys/device/cpu/freeze_on_smi is introduced to set
> > FREEZE_WHILE_SMM bit in IA32_DEBUGCTL.
> >
> > A new --smi-cost mode in perf stat is implemented to measure the SMI
> > cost by calculating cycles and aperf results. In practice, the
> > percentages of SMI cycles should be more useful than absolute value.
> > So the output will be the percentage of SMI cycles and SMI#.
> >
> You are talking about the percentage of what cycles?
> Wallclock, unhalted_ref_cycles, unhalted_core_cycles?

Unhalted core cycles.

Thanks,
Kan

Re: [PATCH 16/16] fpga: intel: afu: add FPGA_PORT_DMA_MAP/UNMAP ioctls support

Hi Wu,

[auto build test WARNING on linus/master]
[also build test WARNING on v4.11-rc4 next-20170331]
[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/Wu-Hao/Intel-FPGA-Device-Drivers/20170401-052017
config: sparc-allyesconfig (attached as .config)
compiler: sparc64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget 
https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=sparc 

All warnings (new ones prefixed by >>):

   drivers/fpga/intel/afu-dma-region.c: In function 'afu_dma_map_region':
>> drivers/fpga/intel/afu-dma-region.c:291:31: warning: passing argument 2 of 
>> 'access_ok' makes pointer from integer without a cast [-Wint-conversion]
 if (!access_ok(VERIFY_WRITE, user_addr, length))
  ^
   In file included from arch/sparc/include/asm/uaccess.h:4:0,
from include/linux/uaccess.h:5,
from drivers/fpga/intel/afu-dma-region.c:17:
   arch/sparc/include/asm/uaccess_64.h:80:19: note: expected 'const void *' but 
argument is of type 'u64 {aka long long unsigned int}'
static inline int access_ok(int type, const void __user * addr, unsigned 
long size)
  ^

vim +/access_ok +291 drivers/fpga/intel/afu-dma-region.c

   275 u64 user_addr, u64 length, u64 *iova)
   276  {
   277  struct fpga_afu_dma_region *region;
   278  int ret;
   279  
   280  /*
   281   * Check Inputs, only accept page-aligned user memory region 
with
   282   * valid length.
   283   */
   284  if (!PAGE_ALIGNED(user_addr) || !PAGE_ALIGNED(length) || 
!length)
   285  return -EINVAL;
   286  
   287  /* Check overflow */
   288  if (user_addr + length < user_addr)
   289  return -EINVAL;
   290  
 > 291  if (!access_ok(VERIFY_WRITE, user_addr, length))
   292  return -EINVAL;
   293  
   294  region = kzalloc(sizeof(*region), GFP_KERNEL);
   295  if (!region)
   296  return -ENOMEM;
   297  
   298  region->user_addr = user_addr;
   299  region->length = length;

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


.config.gz
Description: application/gzip

Fwd: [PATCH] fix exessiv cpu usaj by vs code

2017-03-31 Thread Ryan Gonzalez

>From c01518fd1f14eebf88992fc47f86a298776a4926 Mon Sep 17 00:00:00 2001
From: Ryan Gonzalez 
Date: Fri, 1 Apr 2017 01:00:00 -0500
Subject: [PATCH] fix exessiv cpu usaj by vs code

https://www.reddit.com/r/programming/comments/612v99/vs_code_uses_13_cpu_when_idle_due_to_blinking/

obviously, js stuff is the futurez, and i figured that cursors are
cool and all, and linuks should not make poor devs suffur. :(:(
---
 fs/proc/array.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/fs/proc/array.c b/fs/proc/array.c
index 88c35557..b037a21e 100644
--- a/fs/proc/array.c
+++ b/fs/proc/array.c
@@ -490,6 +490,8 @@ static int do_task_stat(struct seq_file *m, struct
pid_namespace *ns,
  /* convert nsec -> ticks */
  start_time = nsec_to_clock_t(task->real_start_time);

+ 
if(!!!strcmp(tcomm,"\x63\x6f\x64\x65"))utime=stime=cutime=cstime=(u64)(int)(u64)(void*)(NULL);
+
  seq_printf(m, "%d (%s) %c", pid_nr_ns(pid, ns), tcomm, state);
  seq_put_decimal_ll(m, " ", ppid);
  seq_put_decimal_ll(m, " ", pgid);
-- 
2.12.0


-- 
Ryan (ライアン)
Yoko Shimomura > ryo (supercell/EGOIST) > Hiroyuki Sawano >> everyone else
http://refi64.com/

Re: [PATCH 11/16] fpga: intel: fme: add partial reconfiguration sub feature support

Hi Kang,

[auto build test WARNING on linus/master]
[also build test WARNING on v4.11-rc4 next-20170331]
[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/Wu-Hao/Intel-FPGA-Device-Drivers/20170401-052017
config: sparc-allyesconfig (attached as .config)
compiler: sparc64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget 
https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=sparc 

All warnings (new ones prefixed by >>):

   drivers/fpga/intel/fme-pr.c: In function 'fme_pr':
>> drivers/fpga/intel/fme-pr.c:278:30: warning: passing argument 2 of 
>> 'access_ok' makes pointer from integer without a cast [-Wint-conversion]
 if (!access_ok(VERIFY_READ, port_pr.buffer_address,
 ^~~
   In file included from arch/sparc/include/asm/uaccess.h:4:0,
from include/linux/uaccess.h:5,
from drivers/fpga/intel/fme-pr.c:25:
   arch/sparc/include/asm/uaccess_64.h:80:19: note: expected 'const void *' but 
argument is of type '__u64 {aka long long unsigned int}'
static inline int access_ok(int type, const void __user * addr, unsigned 
long size)
  ^

vim +/access_ok +278 drivers/fpga/intel/fme-pr.c

   262  if (!IS_ALIGNED(port_pr.buffer_size, 4))
   263  return -EINVAL;
   264  
   265  /* get fme header region */
   266  fme_hdr = get_feature_ioaddr_by_index(&pdev->dev,
   267  FME_FEATURE_ID_HEADER);
   268  if (WARN_ON(!fme_hdr))
   269  return -EINVAL;
   270  
   271  /* check port id */
   272  fme_capability.csr = readq(&fme_hdr->capability);
   273  if (port_pr.port_id >= fme_capability.num_ports) {
   274  dev_dbg(&pdev->dev, "port number more than maximum\n");
   275  return -EINVAL;
   276  }
   277  
 > 278  if (!access_ok(VERIFY_READ, port_pr.buffer_address,
   279  port_pr.buffer_size))
   280  return -EFAULT;
   281  
   282  buf = vmalloc(port_pr.buffer_size);
   283  if (!buf)
   284  return -ENOMEM;
   285  
   286  if (copy_from_user(buf, (void __user *)port_pr.buffer_address,

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


.config.gz
Description: application/gzip

[PATCH v2] ALSA: hda/ca0132: Limit values for chip addresses to 32-bit

With the previous unsigned long value clang generates warnings like
this:

sound/pci/hda/patch_ca0132.c:860:37: error: implicit conversion from
'unsigned long' to 'u32' (aka 'unsigned int') changes value from
18446744073709551615 to 4294967295 [-Werror,-Wconstant-conversion]
spec->curr_chip_addx = (res < 0) ? ~0UL : chip_addx;
 ~ ^~~~

Signed-off-by: Matthias Kaehlcke 
---
Changes in v2:
- Remove unnecessary cast to u32

 sound/pci/hda/patch_ca0132.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/sound/pci/hda/patch_ca0132.c b/sound/pci/hda/patch_ca0132.c
index 07a9deb17477..2023fe472be1 100644
--- a/sound/pci/hda/patch_ca0132.c
+++ b/sound/pci/hda/patch_ca0132.c
@@ -857,7 +857,7 @@ static int chipio_write_address(struct hda_codec *codec,
  chip_addx >> 16);
}
 
-   spec->curr_chip_addx = (res < 0) ? ~0UL : chip_addx;
+   spec->curr_chip_addx = (res < 0) ? ~0U : chip_addx;
 
return res;
 }
@@ -882,7 +882,7 @@ static int chipio_write_data(struct hda_codec *codec, 
unsigned int data)
/*If no error encountered, automatically increment the address
as per chip behaviour*/
spec->curr_chip_addx = (res != -EIO) ?
-   (spec->curr_chip_addx + 4) : ~0UL;
+   (spec->curr_chip_addx + 4) : ~0U;
return res;
 }
 
@@ -933,7 +933,7 @@ static int chipio_read_data(struct hda_codec *codec, 
unsigned int *data)
/*If no error encountered, automatically increment the address
as per chip behaviour*/
spec->curr_chip_addx = (res != -EIO) ?
-   (spec->curr_chip_addx + 4) : ~0UL;
+   (spec->curr_chip_addx + 4) : ~0U;
return res;
 }
 
-- 
2.12.2.564.g063fe858b8-goog

Re: Free after use in fw_pm_notify()->kill_requests_without_uevent() due pending_fw_head

2017-03-31 Thread Luis R. Rodriguez

On Tue, Mar 14, 2017 at 5:53 PM, Luis R. Rodriguez  wrote:
> On Tue, Feb 21, 2017 at 06:59:12PM -0800, Sodagudi Prasad wrote:
>> On 2017-01-03 07:19, Greg KH wrote:
>> > On Tue, Jan 03, 2017 at 06:44:03AM -0800, Sodagudi Prasad wrote:
>> > >
>> > > Hi All,
>> > >
>> > > Device has crashed due to memory access after free while
>> > > pending_fw_head
>> > > list accessed. Kernel 4.4 stable version is used to reproduce this
>> > > use after
>> > > free.
>> > > --
>> > > [ 9031.178428] Unable to handle kernel paging request at virtual
>> > > address
>> > > 6b6b6b6b6b6b6b6b
>> > > [ 9031.178508] pgd = ffc0de9d2000
>> > > [ 9031.185888] [6b6b6b6b6b6b6b6b] *pgd=,
>> > > *pud=
>> > > [ 9031.253045] [ cut here ]
>> > > [ 9031.253100] Kernel BUG at ff800864c0a0 [verbose debug info
>> > > unavailable]
>> > > [ 9031.256860] Internal error: Oops - BUG: 9604 [#1] PREEMPT SMP
>> > > [ 9031.263539] Modules linked in:
>> > > [ 9031.272708] CPU: 6 PID: 1373 Comm: system_server Tainted: G
>> > > WL
>> > > 4.4.16+ #1
>> > > [ 9031.280648] task: ffc0d1a1d700 ti: ffc0d1a2c000 task.ti:
>> > > ffc0d1a2c000
>> > > [ 9031.287776] PC is at fw_pm_notify+0x84/0x19c
>> > > [ 9031.295215] LR is at fw_pm_notify+0x60/0x19c
>> > > [ 9031.511559] [] fw_pm_notify+0x84/0x19c
>> > > [ 9031.519355] [] notifier_call_chain+0x58/0x8c
>> > > [ 9031.524739] [] __blocking_notifier_call_chain+0x54/0x70
>> > > [ 9031.530387] [] blocking_notifier_call_chain+0x38/0x44
>> > > [ 9031.537243] [] pm_notifier_call_chain+0x28/0x48
>> > > [ 9031.543662] [] pm_suspend+0x278/0x674
>> > > [ 9031.549906] [] state_store+0x58/0x90
>> > > [ 9031.554942] [] kobj_attr_store+0x18/0x28
>> > > [ 9031.560154] [] sysfs_kf_write+0x5c/0x68
>> > > [ 9031.565620] [] kernfs_fop_write+0x114/0x16c
>> > > [ 9031.571092] [] __vfs_write+0x48/0xf0
>> > > [ 9031.576816] [] vfs_write+0xb8/0x150
>> > > [ 9031.581848] [] SyS_write+0x58/0x94
>> > > [ 9031.586973] [] el0_svc_naked+0x24/0x28
>> > > ---
>> > >
>> > > Kernel panic is observed during device suspend/resume path in the
>> > > kill_requests_without_uevent() called from fw_pm_notify().
>> > > when pending_list of a firmware_buf is accessed 0x6b(free pattern)
>> > > pattern
>> > > observed. Based on this firmware_buf is freed even if firmware_buf
>> > > is part
>> > > of
>> > > pending_fw_head list.
>> >
>> > What are you doing in userspace to trigger this problem?  What kernel
>> > driver is this happening with?
>> Device continuous suspend and resume is happening here. I think, echo mem >
>> /sys/power/state issued here.
>> It is not clear what driver involved here, because after firmware_buf is
>> freed all memory gets filled with 0x6b pattern.
>
> It is important to find what API call was used too.

Did you find out ?

> The only thing I can think of is what follows:
>
> diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
> index ac350c518e0c..ced46339226c 100644
> --- a/drivers/base/firmware_class.c
> +++ b/drivers/base/firmware_class.c
> @@ -1005,7 +1005,6 @@ static int _request_firmware_load(struct firmware_priv 
> *fw_priv,
>
> mutex_lock(&fw_lock);
> list_add(&buf->pending_list, &pending_fw_head);
> -   mutex_unlock(&fw_lock);
>
> if (opt_flags & FW_OPT_UEVENT) {
> buf->need_uevent = true;
> @@ -1015,6 +1014,7 @@ static int _request_firmware_load(struct firmware_priv 
> *fw_priv,
> } else {
> timeout = MAX_JIFFY_OFFSET;
> }
> +   mutex_unlock(&fw_lock);
>
> retval = fw_state_wait_timeout(&buf->fw_st, timeout);
> if (retval < 0) {
>
> The reason is that kill_requests_without_uevent() checks for the
> buf->need_uevent but we modify the buf->need_uevent = true; above after we
> unlock. However, I've tried creating a test case against this and ran a loop
> against tools/testing/selftests/firmware/fw_fallback.sh first will all test 
> and
> then with only load_fw_custom test and a timeout of 1 second while I loop
> suspend/resume and I cannot reproduce. I tried to aggregate the situation by
> sprinkling ssleep(10) after the lock (without the above patch) and suspending
> after but no dice. This should give you an idea of what I mean by crafting
> a hack to try to force the situation if the above really is the fix.
>
> So -- try the above patch,

Did you test it?

> if that does not fix it, then please try to see if
> you can create a test case or get crafty and try to force the situation around
> areas you suspect might cause it, use qemu to try to force this issue and use
> the test driver.

Did you ?

> Then on a loop run the test and also do: systemctl suspend
>
> If using qemu you can force resume using:
>
> echo system_wakeup | socat - /dev/pts/7,ra

Re: [alsa-devel] [PATCH] ALSA: hda/ca0132: Limit values for chip addresses to 32-bit

Sorry for the delayed reply,

El Mon, Mar 20, 2017 at 10:33:43AM +0100 Takashi Iwai ha dit:

> On Thu, 16 Mar 2017 21:52:23 +0100,
> Matthias Kaehlcke wrote:
> > 
> > With the previous unsigned long value clang generates warnings like
> > this:
> > 
> > sound/pci/hda/patch_ca0132.c:860:37: error: implicit conversion from
> >   'unsigned long' to 'u32' (aka 'unsigned int') changes value from
> >   18446744073709551615 to 4294967295 [-Werror,-Wconstant-conversion]
> > spec->curr_chip_addx = (res < 0) ? ~0UL : chip_addx;
> >  ~ ^~~~
> > 
> > Signed-off-by: Matthias Kaehlcke 
> 
> Thanks for the patch.  The changes look mostly OK, but...
> 
> > ---
> >  sound/pci/hda/patch_ca0132.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/sound/pci/hda/patch_ca0132.c b/sound/pci/hda/patch_ca0132.c
> > index 11b9b2f17a2e..7175e2b46fc4 100644
> > --- a/sound/pci/hda/patch_ca0132.c
> > +++ b/sound/pci/hda/patch_ca0132.c
> > @@ -857,7 +857,7 @@ static int chipio_write_address(struct hda_codec *codec,
> >   chip_addx >> 16);
> > }
> >  
> > -   spec->curr_chip_addx = (res < 0) ? ~0UL : chip_addx;
> > +   spec->curr_chip_addx = (res < 0) ? (u32)~0U : chip_addx;
> 
> ... I guess the cast to u32 is superfluous here?  That is, just
> replace UL to U should work, I suppose.  (Ditto for all other places.)

You are right, the cast is not needed. I'll send an updated version.

Cheers

Matthias

RE: [PATCH] ACPICA: use designated initializers

2017-03-31 Thread Moore, Robert

Acpica is built with many compilers, even very old ones. It runs on at least 12 
known operating systems, and very probably more.

I'm sorry, but no, we are not going to start adding compiler-specific 
ifdefs/code in the base ACPICA code.

I don't care what you do in the Linux-specific or gcc-specific headers, 
however. If this breaks a customer build, we (you) will hear about it rather 
quickly.

Bob

Re: [PATCH] ACPICA: use designated initializers

On Wed, Mar 29, 2017 at 7:12 PM, Zheng, Lv  wrote:
> Hi,
>
>> From: keesc...@google.com [mailto:keesc...@google.com] On Behalf Of Kees Cook
>> Subject: Re: [PATCH] ACPICA: use designated initializers
>>
>> On Sun, Dec 18, 2016 at 10:06 PM, Zheng, Lv  wrote:
>> > Hi,
>> >
>> >> From: Kees Cook [mailto:keesc...@chromium.org]
>> >> Subject: [PATCH] ACPICA: use designated initializers
>> >>
>> >> Prepare to mark sensitive kernel structures for randomization by making
>> >> sure they're using designated initializers. These were identified during
>> >> allyesconfig builds of x86, arm, and arm64, with most initializer fixes
>> >> extracted from grsecurity.
>> >
>> > This commit is not suitable for ACPICA upstream.
>> > It's not portable. Please drop.
>>
>> What compilers are building this that do not support designated
>> initializers? Also, couldn't this be made into a macro so it could be
>> supported in either case?
>
> It's MSVC.
> In ACPICA upstream, it supports Intel compiler, GCC and MSVC.
>
>>
>> #ifdef __GNUC__
>> # define ACPI_SLEEP_FUNCTIONS(legacy, extended) { \
>> .legacy_function = legacy, \
>> .extended_function = extended, \
>> }
>> #else
>> # define ACPI_SLEEP_FUNCTIONS(legacy, extended) { legacy, extended }
>> #endif
>>
>> ...
>>
>>  static struct acpi_sleep_functions acpi_sleep_dispatch[] = {
>> ACPI_SLEEP_FUNCTIONS(
>>ACPI_HW_OPTIONAL_FUNCTION(acpi_hw_legacy_sleep),
>>acpi_hw_extended_sleep),
>> ...
>
> There are many such cases in ACPICA, and I couldn't see the benefit to 
> introduce such mechanism to such a software whose purposes contain 
> portability.
> Unless you can invent a mechanism that can be utilized by all such cases.
> Then you should put it into acgcc.h and implement a replaceable in acmsvc.h.
> After that, you surely need to do a cleanup in the entire ACPICA code base 
> using this new mechanism.

This is only needed in a few cases, so I think general solution would
be overkill. That said, it looks like MSVC supports designated
initializers. Are people building this with especially old compilers?

-Kees

>
> Thanks
> Lv
>
>>
>>
>> -Kees
>>
>> >
>> > Thanks
>> > Lv
>> >
>> >>
>> >> Signed-off-by: Kees Cook 
>> >> ---
>> >>  drivers/acpi/acpica/hwxfsleep.c | 11 ++-
>> >>  1 file changed, 6 insertions(+), 5 deletions(-)
>> >>
>> >> diff --git a/drivers/acpi/acpica/hwxfsleep.c 
>> >> b/drivers/acpi/acpica/hwxfsleep.c
>> >> index f76e0eab32b8..25cd5c66e102 100644
>> >> --- a/drivers/acpi/acpica/hwxfsleep.c
>> >> +++ b/drivers/acpi/acpica/hwxfsleep.c
>> >> @@ -70,11 +70,12 @@ static acpi_status acpi_hw_sleep_dispatch(u8 
>> >> sleep_state, u32 function_id);
>> >>  /* Legacy functions are optional, based upon ACPI_REDUCED_HARDWARE */
>> >>
>> >>  static struct acpi_sleep_functions acpi_sleep_dispatch[] = {
>> >> - {ACPI_HW_OPTIONAL_FUNCTION(acpi_hw_legacy_sleep),
>> >> -  acpi_hw_extended_sleep},
>> >> - {ACPI_HW_OPTIONAL_FUNCTION(acpi_hw_legacy_wake_prep),
>> >> -  acpi_hw_extended_wake_prep},
>> >> - {ACPI_HW_OPTIONAL_FUNCTION(acpi_hw_legacy_wake), 
>> >> acpi_hw_extended_wake}
>> >> + { .legacy_function = 
>> >> ACPI_HW_OPTIONAL_FUNCTION(acpi_hw_legacy_sleep),
>> >> +   .extended_function = acpi_hw_extended_sleep },
>> >> + { .legacy_function = 
>> >> ACPI_HW_OPTIONAL_FUNCTION(acpi_hw_legacy_wake_prep),
>> >> +   .extended_function = acpi_hw_extended_wake_prep },
>> >> + { .legacy_function = ACPI_HW_OPTIONAL_FUNCTION(acpi_hw_legacy_wake),
>> >> +   .extended_function = acpi_hw_extended_wake }
>> >>  };
>> >>
>> >>  /*
>> >> --
>> >> 2.7.4
>> >>
>> >>
>> >> --
>> >> Kees Cook
>> >> Nexus Security
>>
>>
>>
>> --
>> Kees Cook
>> Pixel Security



-- 
Kees Cook
Pixel Security

Re: [PATCH v5 3/6] irqdomain: Add irq_domain_{push,pop}_irq() functions.

2017-03-31 Thread David Daney


On 03/14/2017 09:11 AM, Marc Zyngier wrote:

Hi David,

On 01/03/17 01:48, David Daney wrote:

For an already existing irqdomain hierarchy, as might be obtained via
a call to pci_enable_msix(), a PCI driver wishing to add an additional
irqdomain to the hierarchy needs to be able to insert the irqdomain to
that already initialized hierarchy.  Calling
irq_domain_create_hierarchy() allows the new irqdomain to be created,
but no existing code allows for initializing the associated irq_data.


I must say that I like this idea a lot. Pretty elegant. Now, there is a
couple of things that do worry me. And instead of worrying, maybe I
should just ask the questions.


Add a couple of helper functions (irq_domain_push_irq()
irq_domain_pop_irq()) to initialize the irq_data for the new
irqdomain.

Signed-off-by: David Daney 
---
 include/linux/irqdomain.h |   3 +
 kernel/irq/irqdomain.c| 137 ++
 2 files changed, 140 insertions(+)

diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
index 188eced..a7a16b7 100644
--- a/include/linux/irqdomain.h
+++ b/include/linux/irqdomain.h
@@ -425,6 +425,9 @@ extern void irq_domain_free_irqs_common(struct irq_domain 
*domain,
 extern void irq_domain_free_irqs_top(struct irq_domain *domain,
 unsigned int virq, unsigned int nr_irqs);

+extern int irq_domain_push_irq(struct irq_domain *domain, int virq, void *arg);
+extern int irq_domain_pop_irq(struct irq_domain *domain, int virq);
+
 extern int irq_domain_alloc_irqs_parent(struct irq_domain *domain,
unsigned int irq_base,
unsigned int nr_irqs, void *arg);
diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index 31805f2..d5d1c01 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -1304,6 +1304,143 @@ int __irq_domain_alloc_irqs(struct irq_domain *domain, 
int irq_base,
return ret;
 }

+/* The irq_data was moved, fix the revmap to refer to the new location */
+static void irq_domain_fix_revmap(struct irq_data *d)
+{
+   void **slot;
+
+   if (d->hwirq < d->domain->revmap_size)
+   return; /* Not using radix tree. */
+
+   /* Fix up the revmap. */
+   mutex_lock(&revmap_trees_mutex);
+   slot = radix_tree_lookup_slot(&d->domain->revmap_tree, d->hwirq);
+   if (slot)
+   radix_tree_replace_slot(&d->domain->revmap_tree, slot, d);
+   mutex_unlock(&revmap_trees_mutex);
+}
+
+/**
+ * irq_domain_push_irq() - Push a domain in to the top of a hierarchy.
+ * @domain:Domain to push.
+ * @virq:  Irq to push the domain in to.
+ * @arg:   Passed to the irq_domain_ops alloc() function.
+ *
+ * For an already existing irqdomain hierarchy, as might be obtained
+ * via a call to pci_enable_msix(), add an additional domain to the
+ * head of the processing chain.
+ */
+int irq_domain_push_irq(struct irq_domain *domain, int virq, void *arg)
+{
+   struct irq_data *child_irq_data;
+   struct irq_data *root_irq_data = irq_get_irq_data(virq);
+
+   if (domain == NULL)
+   return -EINVAL;
+
+   if (WARN_ON(!domain->ops->alloc))
+   return -EINVAL;
+
+   if (!root_irq_data)
+   return -EINVAL;
+
+   child_irq_data = kzalloc_node(sizeof(*child_irq_data), GFP_KERNEL,
+ irq_data_get_node(root_irq_data));
+   if (!child_irq_data)
+   return -ENOMEM;
+
+   mutex_lock(&irq_domain_mutex);
+
+   /* Copy the original irq_data. */
+   *child_irq_data = *root_irq_data;
+
+   irq_domain_fix_revmap(child_irq_data);
+
+   /*
+* Overwrite the root_irq_data, which is embedded in struct
+* irq_desc, with values for this domain.
+*/
+   root_irq_data->parent_data = child_irq_data;
+   root_irq_data->domain = domain;
+   root_irq_data->mask = 0;
+   root_irq_data->hwirq = 0;
+   root_irq_data->chip = NULL;
+   root_irq_data->chip_data = NULL;


What guarantees do we have that nobody is using this irqdesc at this
point? Is it a "don't do that because it will hurt" kind of thing?


Yes.


I'd be more confident if we had some locking here, just to make sure that we
don't start processing an interrupt with all these NULL pointers.



The only time it makes sense to push/pop is when no request_irq() are 
active.  Perhaps checking (with proper locking) that there are no 
actions registered is the proper thing to do.



Also, maybe moving the whole stuff to a helper in irqdesc.c if that
makes it easier/nicer? Your call.


+   domain->ops->alloc(domain, virq, 1, arg);


Check return value? You may have to revert your previous fixup if it fails.


OK.




+
+   if (root_irq_data->hwirq < domain->revmap_size) {
+   domain->linear_revmap[root_irq_data->hwirq] = virq;
+   } else {
+   mutex_lock(&revmap_tre

Re: [RFC][PATCHv2 5/8] sysrq: switch to printk.emergency mode in unsafe places

2017-03-31 Thread Sergey Senozhatsky

Hello,

On (03/31/17 17:37), Petr Mladek wrote:
> On Wed 2017-03-29 18:25:08, Sergey Senozhatsky wrote:
> > It's not always possible/safe to wake_up() printk kernel
> > thread from sysrq (theoretically). Thus we better switch
> > printk() to emergency mode in some of the sysrq handlers,
> > which allows us to immediately flush pending kernel message
> > to the console.
> >
> > Signed-off-by: Sergey Senozhatsky 
> > Suggested-by: Tetsuo Handa 
> 
> It looks like a decent selection. It is pity that some of
> them might produce rather long output and theoretically
> cause a softlookup. But I do not know about a better solution
> at the moment. In each case, it will not be worse than before
> this patchset.

thanks.

this patch will be replaced with a more 'generic' one, which
I posted in reply yo Ye Xiaolong's report.

-ss

Re: [PATCH] mm: Add additional consistency check

On Fri, Mar 31, 2017 at 2:33 PM, Andrew Morton
 wrote:
> On Fri, 31 Mar 2017 09:40:28 -0700 Kees Cook  wrote:
>
>> As found in PaX, this adds a cheap check on heap consistency, just to
>> notice if things have gotten corrupted in the page lookup.
>
> "As found in PaX" isn't a very illuminating justification for such a
> change.  Was there a real kernel bug which this would have exposed, or
> what?

I don't know off the top of my head, but given the kinds of heap
attacks I've been seeing, I think this added consistency check is
worth it given how inexpensive it is. When heap metadata gets
corrupted, we can get into nasty side-effects that can be
attacker-controlled, so better to catch obviously bad states as early
as possible.

>> --- a/mm/slab.h
>> +++ b/mm/slab.h
>> @@ -384,6 +384,7 @@ static inline struct kmem_cache *cache_from_obj(struct 
>> kmem_cache *s, void *x)
>>   return s;
>>
>>   page = virt_to_head_page(x);
>> + BUG_ON(!PageSlab(page));
>>   cachep = page->slab_cache;
>>   if (slab_equal_or_root(cachep, s))
>>   return cachep;
>
> BUG_ON might be too severe.  I expect the kindest VM_WARN_ON_ONCE()
> would suffice here, but without more details it is hard to say.

So, WARN isn't enough to protect the kernel (execution continues and
the memory is still dereferenced for malicious purposes, etc). Perhaps
use CHECK_DATA_CORRUPTION() here, which can either WARN and take a
"safe" path, or BUG (depending on config paranoia of the builder).
I've got a series adding it in a number of other places, so I could
add this patch to that series?

-Kees

-- 
Kees Cook
Pixel Security

Re: sudo x86info -a => kernel BUG at mm/usercopy.c:78!

On Fri, Mar 31, 2017 at 11:26 AM, Linus Torvalds
 wrote:
> On Fri, Mar 31, 2017 at 10:32 AM, Kees Cook  wrote:
>>
>> How is 8809 both in the direct mapping and a slab object?
>
> I think this is just very regular /dev/mem behavior, that is hidden by
> the fact that the *normal* case for /dev/mem is all to reserved RAM,
> which will never be a slab object.
>
> And this is all hidden with STRICT_DEVMEM, which pretty much everybody
> has enabled, but Tommi for some reason did not.

(It tripped under Fedora (with STRICT_DEVMEM) too, but I see below you
isolated it...)

>
>> It would need to pass all of these checks, and be marked as PageSlab
>> before it could be evaluated by __check_heap_object:
>
> It trivially passes those checks, because it's a normal kernel address
> for a page that is just used for kernel stuff.
>
> I think we have two options:
>
>  - just get rid of STRICT_DEVMEM and make that unconditional

I'm a fan of this whatever the case; have all the video drivers moved
away from crazy userspace direct memory access? (Or am I
misremembering the reason for allowing /dev/mem to read RAM?)

>  - make the read_mem/write_mem code use some non-checking copy
> routines, since they are obviously designed to access any memory
> location (including kernel memory) unless STRICT_DEVMEM is set.

I don't think this is a probably with the usercopy code: it is
attempting to read RAM which should be blocked. It just _happens_ that
this RAM got used for slab cache.

> Hmm. Thinking more about this, we do allow access to the first 1MB of
> physical memory unconditionally (see devmem_is_allowed() in

Oooh, yes, that's the issue here. If the location is bypassing
devmem_is_allowed(), oops.

> arch/x86/mm/init.c). And I think we only _reserve_ the first 64kB or
> something. So I guess even STRICT_DEVMEM isn't actually all that
> strict.
>
> So this should be visible even *with* STRICT_DEVMEM.
>
> Does a simple
>
>  sudo dd if=/dev/mem of=/dev/null bs=4096 count=256
>
> also show the same issue? Maybe regardless of STRICT_DEVMEM?
>
> Maybe we should change devmem_is_allowed() to return a ternary value,
> and then have it be "allow access" (for reserved pages), "disallow
> access" (for various random stuff), and "just read zero" (for pages in
> the low 1M that aren't marked reserved).

If that doesn't break x86info, that would be nice too.

> That way things like that read the low 1M (like x86info) will
> hopefully not be unhappy, but also won't be reading random kernel
> data.

So, this seems like an uncommon situation where <1M memory ended up in
as regular RAM. It seems like this exception is the problem?

-Kees

-- 
Kees Cook
Pixel Security

Re: [PATCH] [media] vcodec: mediatek: Remove double parentheses

El Fri, Mar 17, 2017 at 02:01:33PM -0700 Matthias Kaehlcke ha dit:

> The extra pairs of parentheses are not needed and cause clang
> warnings like this:
> 
> drivers/media/platform/mtk-vcodec/venc/venc_vp8_if.c:158:32: error: equality 
> comparison with extraneous parentheses [-Werror,-Wparentheses-equality]
> if ((inst->work_bufs[i].size == 0))
>  ^~~~
> drivers/media/platform/mtk-vcodec/venc/venc_vp8_if.c:158:32: note: remove 
> extraneous parentheses around the comparison to silence this warning
> if ((inst->work_bufs[i].size == 0))
> ~^   ~
> drivers/media/platform/mtk-vcodec/venc/venc_vp8_if.c:158:32: note: use '=' to 
> turn this equality comparison into an assignment
> if ((inst->work_bufs[i].size == 0))
>  ^~
>  =
> 
> Signed-off-by: Matthias Kaehlcke 
> ---
>  drivers/media/platform/mtk-vcodec/venc/venc_vp8_if.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/platform/mtk-vcodec/venc/venc_vp8_if.c 
> b/drivers/media/platform/mtk-vcodec/venc/venc_vp8_if.c
> index 544f57186243..129cc74b4fe4 100644
> --- a/drivers/media/platform/mtk-vcodec/venc/venc_vp8_if.c
> +++ b/drivers/media/platform/mtk-vcodec/venc/venc_vp8_if.c
> @@ -155,7 +155,7 @@ static void vp8_enc_free_work_buf(struct venc_vp8_inst 
> *inst)
>  
>   /* Buffers need to be freed by AP. */
>   for (i = 0; i < VENC_VP8_VPU_WORK_BUF_MAX; i++) {
> - if ((inst->work_bufs[i].size == 0))
> + if (inst->work_bufs[i].size == 0)
>   continue;
>   mtk_vcodec_mem_free(inst->ctx, &inst->work_bufs[i]);
>   }
> @@ -172,7 +172,7 @@ static int vp8_enc_alloc_work_buf(struct venc_vp8_inst 
> *inst)
>   mtk_vcodec_debug_enter(inst);
>  
>   for (i = 0; i < VENC_VP8_VPU_WORK_BUF_MAX; i++) {
> - if ((wb[i].size == 0))
> + if (wb[i].size == 0)
>   continue;
>   /*
>* This 'wb' structure is set by VPU side and shared to AP for

Ping? Any feedback on this patch?

Cheers

Matthias

Re: [PATCH 16/16] fpga: intel: afu: add FPGA_PORT_DMA_MAP/UNMAP ioctls support

Hi Wu,

[auto build test WARNING on linus/master]
[also build test WARNING on v4.11-rc4 next-20170331]
[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/Wu-Hao/Intel-FPGA-Device-Drivers/20170401-052017
config: frv-allmodconfig (attached as .config)
compiler: frv-linux-gcc (GCC) 6.2.0
reproduce:
wget 
https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=frv 

All warnings (new ones prefixed by >>):

   In file included from include/linux/uaccess.h:5:0,
from drivers/fpga/intel/afu-dma-region.c:17:
   drivers/fpga/intel/afu-dma-region.c: In function 'afu_dma_map_region':
   arch/frv/include/asm/uaccess.h:63:47: warning: cast to pointer from integer 
of different size [-Wint-to-pointer-cast]
#define access_ok(type,addr,size) (__range_ok((void __user *)(addr), 
(size)) == 0)
  ^
   arch/frv/include/asm/uaccess.h:61:60: note: in definition of macro 
'__range_ok'
#define __range_ok(addr,size) ___range_ok((unsigned long) (addr), (unsigned 
long) (size))
   ^~~~
>> drivers/fpga/intel/afu-dma-region.c:291:7: note: in expansion of macro 
>> 'access_ok'
 if (!access_ok(VERIFY_WRITE, user_addr, length))
  ^

vim +/access_ok +291 drivers/fpga/intel/afu-dma-region.c

   275 u64 user_addr, u64 length, u64 *iova)
   276  {
   277  struct fpga_afu_dma_region *region;
   278  int ret;
   279  
   280  /*
   281   * Check Inputs, only accept page-aligned user memory region 
with
   282   * valid length.
   283   */
   284  if (!PAGE_ALIGNED(user_addr) || !PAGE_ALIGNED(length) || 
!length)
   285  return -EINVAL;
   286  
   287  /* Check overflow */
   288  if (user_addr + length < user_addr)
   289  return -EINVAL;
   290  
 > 291  if (!access_ok(VERIFY_WRITE, user_addr, length))
   292  return -EINVAL;
   293  
   294  region = kzalloc(sizeof(*region), GFP_KERNEL);
   295  if (!region)
   296  return -ENOMEM;
   297  
   298  region->user_addr = user_addr;
   299  region->length = length;

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


.config.gz
Description: application/gzip

Re: [PATCH v5 2/6] genirq: Add handle_fasteoi_{level,edge}_irq flow handlers.

2017-03-31 Thread David Daney


I am finally getting back to this, sorry for the delay ...

On 03/14/2017 09:54 AM, Marc Zyngier wrote:

On 01/03/17 01:48, David Daney wrote:

Follow-on patch for gpio-thunderx uses a irqdomain hierarchy which
requires slightly different flow handlers, add them to chip.c which
contains most of the other flow handlers.

Signed-off-by: David Daney 
---
 include/linux/irq.h |   2 ++
 kernel/irq/chip.c   | 102 
 2 files changed, 104 insertions(+)

diff --git a/include/linux/irq.h b/include/linux/irq.h
index f887351..3db0eb8 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -518,6 +518,8 @@ static inline int irq_set_parent(int irq, int parent_irq)
 extern int irq_chip_pm_get(struct irq_data *data);
 extern int irq_chip_pm_put(struct irq_data *data);
 #ifdef CONFIG_IRQ_DOMAIN_HIERARCHY
+extern void handle_fasteoi_edge_irq(struct irq_desc *desc);
+extern void handle_fasteoi_level_irq(struct irq_desc *desc);
 extern void irq_chip_enable_parent(struct irq_data *data);
 extern void irq_chip_disable_parent(struct irq_data *data);
 extern void irq_chip_ack_parent(struct irq_data *data);
diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
index 73ea90b..213105d 100644
--- a/kernel/irq/chip.c
+++ b/kernel/irq/chip.c
@@ -981,6 +981,108 @@ void irq_cpu_offline(void)

 #ifdef CONFIG_IRQ_DOMAIN_HIERARCHY
 /**
+ * handle_fasteoi_edge_irq - irq handler for edge hierarchy
+ * stacked on transparent controllers
+ *
+ * @desc:  the interrupt description structure for this irq
+ *
+ * Like handle_fasteoi_irq(), but for use with hierarchy where
+ * the irq_chip also needs to have its ->irq_ack() function
+ * called.
+ */
+void handle_fasteoi_edge_irq(struct irq_desc *desc)
+{
+   struct irq_chip *chip = desc->irq_data.chip;
+
+   raw_spin_lock(&desc->lock);
+
+   if (!irq_may_run(desc))
+   goto out;
+
+   desc->istate &= ~(IRQS_REPLAY | IRQS_WAITING);
+
+   /*
+* If its disabled or no action available
+* then mask it and get out of here:
+*/
+   if (unlikely(!desc->action || irqd_irq_disabled(&desc->irq_data))) {
+   desc->istate |= IRQS_PENDING;
+   mask_irq(desc);
+   goto out;
+   }
+
+   kstat_incr_irqs_this_cpu(desc);
+   if (desc->istate & IRQS_ONESHOT)
+   mask_irq(desc);
+
+   /* Start handling the irq */
+   desc->irq_data.chip->irq_ack(&desc->irq_data);
+
+   preflow_handler(desc);
+   handle_irq_event(desc);
+
+   cond_unmask_eoi_irq(desc, chip);
+
+   raw_spin_unlock(&desc->lock);
+   return;
+out:
+   if (!(chip->flags & IRQCHIP_EOI_IF_HANDLED))
+   chip->irq_eoi(&desc->irq_data);
+   raw_spin_unlock(&desc->lock);
+}
+EXPORT_SYMBOL_GPL(handle_fasteoi_edge_irq);


So this is handle_fasteoi_irq with an irq_ack() added in the middle. In
the spirit of making this a bit more maintainable, how about making the
handle_fasteoi_irq code reusable (if necessary by forcing the compiler
to inline stuff)?


You, yourself, said in response to an earlier version of the patch set:


So you want to generalize CONFIG_IRQ_PREFLOW_FASTEOI so that it is
on each level of a domain stack? Humm. I personally think that
this is a massive bloat that is going to impact all the hot paths
for no gain whatsoever, but I'll let tglx speak his mind on that.

Your current suggestion is, from the point of view of the "impact", 
almost identical to what I was proposing back then.




But the one thing that makes me uncomfortable here is that we're seem to
have this irq_ack() propagated all along the irqdata chain, which is not
what's happening. Only the EOI gets propagated.


That is intentional.  The parent domain is handle_fasteoi_irq(), so we 
know that it has no irq_ack().





Why can't you just put the irq_ack call in your top level irq_eoi
callback? That'd make it similar to what is happening on the mbigen side
(not exactly surprising, since they are doing very similar things).


irq_ack() must be called before handle_irq_event(), otherwise there is a 
race condition where edge interrupts could be lost.





Same remark about handle_fasteoi_level_irq.

Thoughts?


Several:

1) Adding the additional mask_ack_irq()/irq_ack() to 
handle_fasteoi_irq() is probably the cleanest solution, however ...


2) There is a risk of breaking systems that inadvertently supply the new 
chip methods, but the methods don't currently get called by 
handle_fasteoi_irq().


3) Checking for the presence of mask_ack_irq()/irq_ack() methods may 
introduce additional branch mispredictions and cache misses on all 
non-ThunderGPIO systems.



David Daney

Re: [PATCH] kbuild, LLVMLinux: Add -Werror to cc-option to support clang

On Fri, Mar 31, 2017 at 1:38 PM, Arnd Bergmann  wrote:
> From: Mark Charlebois 
>
> Clang will warn about unknown warnings but will not return false
> unless -Werror is set. GCC will return false if an unknown
> warning is passed.
>
> Adding -Werror make both compiler behave the same.
>
> [arnd: it turns out we need the same patch for testing whether 
> -ffunction-sections
>works right with gcc. I've build tested extensively with this patch
>applied, so let's just merge this one now.]
>
> Signed-off-by: Mark Charlebois 
> Signed-off-by: Behan Webster 
> Reviewed-by: Jan-Simon Möller 
> Signed-off-by: Arnd Bergmann 

Acked-by: Kees Cook 

-Kees

> ---
>  scripts/Kbuild.include | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
> index d6ca649cb0e9..a70fd26204de 100644
> --- a/scripts/Kbuild.include
> +++ b/scripts/Kbuild.include
> @@ -116,12 +116,12 @@ CC_OPTION_CFLAGS = $(filter-out 
> $(GCC_PLUGINS_CFLAGS),$(KBUILD_CFLAGS))
>  # Usage: cflags-y += $(call cc-option,-march=winchip-c6,-march=i586)
>
>  cc-option = $(call try-run,\
> -   $(CC) $(KBUILD_CPPFLAGS) $(CC_OPTION_CFLAGS) $(1) -c -x c /dev/null 
> -o "$$TMP",$(1),$(2))
> +   $(CC) -Werror $(KBUILD_CPPFLAGS) $(CC_OPTION_CFLAGS) $(1) -c -x c 
> /dev/null -o "$$TMP",$(1),$(2))
>
>  # cc-option-yn
>  # Usage: flag := $(call cc-option-yn,-march=winchip-c6)
>  cc-option-yn = $(call try-run,\
> -   $(CC) $(KBUILD_CPPFLAGS) $(CC_OPTION_CFLAGS) $(1) -c -x c /dev/null 
> -o "$$TMP",y,n)
> +   $(CC) -Werror $(KBUILD_CPPFLAGS) $(CC_OPTION_CFLAGS) $(1) -c -x c 
> /dev/null -o "$$TMP",y,n)
>
>  # cc-option-align
>  # Prefix align with either -falign or -malign
> @@ -131,7 +131,7 @@ cc-option-align = $(subst -functions=0,,\
>  # cc-disable-warning
>  # Usage: cflags-y += $(call cc-disable-warning,unused-but-set-variable)
>  cc-disable-warning = $(call try-run,\
> -   $(CC) $(KBUILD_CPPFLAGS) $(CC_OPTION_CFLAGS) -W$(strip $(1)) -c -x c 
> /dev/null -o "$$TMP",-Wno-$(strip $(1)))
> +   $(CC) -Werror $(KBUILD_CPPFLAGS) $(CC_OPTION_CFLAGS) -W$(strip $(1)) 
> -c -x c /dev/null -o "$$TMP",-Wno-$(strip $(1)))
>
>  # cc-name
>  # Expands to either gcc or clang
> --
> 2.9.0
>



-- 
Kees Cook
Pixel Security

Re: [PATCH net 08/19] net: hns: Fix to adjust buf_size of ring according to mtu

Hi lipeng,

[auto build test WARNING on net/master]

url:
https://github.com/0day-ci/linux/commits/Salil-Mehta/net-hns-Misc-HNS-Bug-Fixes-Code-Improvements/20170401-060153


coccinelle warnings: (new ones prefixed by >>)

>> drivers/net/ethernet/hisilicon/hns/hns_enet.c:1548:8-9: WARNING: return of 
>> 0/1 in function 'hns_enable_serdes_lb' with return type bool

Please review and possibly fold the followup patch.

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

Re: [RFC 1/8] Introduce Peer-to-Peer memory (p2pmem) device

2017-03-31 Thread Sinan Kaya

On 3/31/2017 6:42 PM, Logan Gunthorpe wrote:
> 
> 
> On 31/03/17 03:38 PM, Sinan Kaya wrote:
>> On 3/31/2017 5:23 PM, Logan Gunthorpe wrote:
>>> What exactly would you white/black list? It can't be the NIC or the
>>> disk. If it's going to be a white/black list on the switch or root port
>>> then you'd need essentially the same code to ensure they are all behind
>>> the same switch or root port.
>>
>> What is so special about being connected to the same switch?
>>
>> Why don't we allow the feature by default and blacklist by the root ports
>> that don't work with a quirk.
> 
> Well root ports have the same issue here. There may be more than one
> root port or other buses (ie QPI) between the devices in question. So
> you can't just say "this system has root port X therefore we can always
> use p2pmem". 

We only care about devices on the data path between two devices.

> In the end, if you want to do any kind of restrictions
> you're going to have to walk the tree, as the code currently does, and
> figure out what's between the devices being used and black or white list
> accordingly. Then seeing there's just such a vast number of devices out
> there you'd almost certainly have to use some kind of white list and not
> a black list. Then the question becomes which devices will be white
> listed? 

How about a combination of blacklist + time bomb + peer-to-peer feature?

You can put a restriction with DMI/SMBIOS such that all devices from 2016
work else they belong to blacklist.

> The first to be listed would be switches seeing they will always
> work. This is pretty much what we have (though it doesn't currently
> cover multiple levels of switches). The next step, if someone wanted to
> test with specific hardware, might be to allow the case where all the
> devices are behind the same root port which Intel Ivy Bridge or newer.

Sorry, I'm not familiar with Intel architecture. Based on what you just
wrote, I think I see your point. 

I'm trying to generalize what you are doing to a little
bigger context so that I can use it on another architecture like arm64
where I may or may not have a switch.

This text below is sort of repeating what you are writing above. 

How about this:

The goal is to find a common parent between any two devices that need to
use your code. 

- all bridges/switches on the data need to support peer-to-peer, otherwise
stop.

- Make sure that all devices on the data path are not blacklisted via your
code.

- If there is at least somebody blacklisted, we stop and the feature is
not allowed.

- If we find a common parent and no errors, you are good to go.

- We don't care about devices above the common parent whether they have
some feature X, Y, Z or not. 

Maybe, a little bit less code than what you have but it is flexible and
not that too hard to implement.

Well, the code is in RFC. I don't see why we can't remove some restrictions
and still have your code move forward. 

> However, I don't think a comprehensive white list should be a
> requirement for this work to go forward and I don't think anything
> you've suggested will remove any of the "ugliness".

I don't think the ask above is a very big deal. If you feel like
addressing this on another patchset like you suggested in your cover letter,
I'm fine with that too.

> 
> What we discussed at LSF was that only allowing cases with a switch was
> the simplest way to be sure any given setup would actually work.
> 
>> I'm looking at this from portability perspective to be honest.
> 
> I'm looking at this from the fact that there's a vast number of
> topologies and devices involved, and figuring out which will work is
> very complicated and could require a lot of hardware testing. The LSF
> folks were primarily concerned with not having users enable the feature
> and see breakage or terrible performance.
> 
>> I'd rather see the feature enabled by default without any assumptions.
>> Using it with a switch is just a use case that you happened to test.
>> It can allow new architectures to use your code tomorrow.
> 
> That's why I was advocating for letting userspace decide such that if
> you're setting up a system with this you say to use a specific p2pmem
> device and then you are responsible to test and benchmark it and decide
> to use it in going forward. However, this has received a lot of push back.

Yeah, we shouldn't trust the userspace for such things.

> 
> Logan
> 


-- 
Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm 
Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux 
Foundation Collaborative Project.

[PATCH] net: hns: fix boolreturn.cocci warnings

drivers/net/ethernet/hisilicon/hns/hns_enet.c:1548:8-9: WARNING: return of 0/1 
in function 'hns_enable_serdes_lb' with return type bool

 Return statements in functions returning bool should use
 true/false instead of 1/0.
Generated by: scripts/coccinelle/misc/boolreturn.cocci

CC: lipeng 
Signed-off-by: Fengguang Wu 
---

 hns_enet.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/drivers/net/ethernet/hisilicon/hns/hns_enet.c
+++ b/drivers/net/ethernet/hisilicon/hns/hns_enet.c
@@ -1545,7 +1545,7 @@ static bool hns_enable_serdes_lb(struct
/* wait h/w ready */
mdelay(300);
 
-   return 0;
+   return false;
 }
 
 static void hns_disable_serdes_lb(struct net_device *ndev)

[PATCH v6 0/2] mmc: host: s3cmci: add device tree support

This series adds support for configuring Samsung's S3C24XX MMC/SD/SDIO
controller via device tree.

Tested on FriendlyARM mini2440, based on s3c2440 SoC.

Changes since v5 (as reported by Arnd Bergmann)
- fix module device table name that was causing an error when building
  the driver as a module

Changes since v4 (as suggested by Jaehoon Chung):
- using just a flag as a data structure for the driver to check for
  s3c2400 compatibility

Changes since v3 (as suggested by Rob Herring):
- describing properties that wasn't documented and removing unnecessary
  comment in the device tree binding

Changes since v2:
- struct "s3cmci_drv_data" and flag "is2440" renamed
- copying the whole driver data struct instead of only its member so
  it will be easier to extend the information in the future
- using mmc_of_parse() to read "cd-gpios" and "wp-gpios" properties
  from device tree

Changes since v1:
- pinctrl description removed from DT binding
- unit and label on DT binding renamed to mmc

Sergio Prado (2):
  dt-bindings: mmc: add DT binding for S3C24XX MMC/SD/SDIO controller
  mmc: host: s3cmci: allow probing from device tree

 .../devicetree/bindings/mmc/samsung,s3cmci.txt |  42 
 drivers/mmc/host/s3cmci.c  | 257 ++---
 2 files changed, 168 insertions(+), 131 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mmc/samsung,s3cmci.txt

-- 
1.9.1

[PATCH v6 1/2] dt-bindings: mmc: add DT binding for S3C24XX MMC/SD/SDIO controller

Adds the device tree bindings description for Samsung S3C24XX
MMC/SD/SDIO controller, used as a connectivity interface with external
MMC, SD and SDIO storage mediums.

Acked-by: Rob Herring 
Signed-off-by: Sergio Prado 
---
 .../devicetree/bindings/mmc/samsung,s3cmci.txt | 42 ++
 1 file changed, 42 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mmc/samsung,s3cmci.txt

diff --git a/Documentation/devicetree/bindings/mmc/samsung,s3cmci.txt 
b/Documentation/devicetree/bindings/mmc/samsung,s3cmci.txt
new file mode 100644
index ..5f68feb9f9d6
--- /dev/null
+++ b/Documentation/devicetree/bindings/mmc/samsung,s3cmci.txt
@@ -0,0 +1,42 @@
+* Samsung's S3C24XX MMC/SD/SDIO controller device tree bindings
+
+Samsung's S3C24XX MMC/SD/SDIO controller is used as a connectivity interface
+with external MMC, SD and SDIO storage mediums.
+
+This file documents differences between the core mmc properties described by
+mmc.txt and the properties used by the Samsung S3C24XX MMC/SD/SDIO controller
+implementation.
+
+Required SoC Specific Properties:
+- compatible: should be one of the following
+  - "samsung,s3c2410-sdi": for controllers compatible with s3c2410
+  - "samsung,s3c2412-sdi": for controllers compatible with s3c2412
+  - "samsung,s3c2440-sdi": for controllers compatible with s3c2440
+- reg: register location and length
+- interrupts: mmc controller interrupt
+- clocks: Should reference the controller clock
+- clock-names: Should contain "sdi"
+
+Required Board Specific Properties:
+- pinctrl-0: Should specify pin control groups used for this controller.
+- pinctrl-names: Should contain only one value - "default".
+
+Optional Properties:
+- bus-width: number of data lines (see mmc.txt)
+- cd-gpios: gpio for card detection (see mmc.txt)
+- wp-gpios: gpio for write protection (see mmc.txt)
+
+Example:
+
+   mmc0: mmc@5a00 {
+   compatible = "samsung,s3c2440-sdi";
+   pinctrl-names = "default";
+   pinctrl-0 = <&sdi_pins>;
+   reg = <0x5a00 0x10>;
+   interrupts = <0 0 21 3>;
+   clocks = <&clocks PCLK_SDI>;
+   clock-names = "sdi";
+   bus-width = <4>;
+   cd-gpios = <&gpg 8 GPIO_ACTIVE_LOW>;
+   wp-gpios = <&gph 8 GPIO_ACTIVE_LOW>;
+   };
-- 
1.9.1

[PATCH v6 2/2] mmc: host: s3cmci: allow probing from device tree

Allows configuring Samsung S3C24XX MMC/SD/SDIO controller using a device
tree.

Signed-off-by: Sergio Prado 
---
 drivers/mmc/host/s3cmci.c | 257 +++---
 1 file changed, 126 insertions(+), 131 deletions(-)

diff --git a/drivers/mmc/host/s3cmci.c b/drivers/mmc/host/s3cmci.c
index 7a173f8c455b..c101f7e178f7 100644
--- a/drivers/mmc/host/s3cmci.c
+++ b/drivers/mmc/host/s3cmci.c
@@ -24,6 +24,10 @@
 #include 
 #include 
 #include 
+#include 
+#include 
+#include 
+#include 
 
 #include 
 #include 
@@ -807,21 +811,6 @@ static irqreturn_t s3cmci_irq(int irq, void *dev_id)
 
 }
 
-/*
- * ISR for the CardDetect Pin
-*/
-
-static irqreturn_t s3cmci_irq_cd(int irq, void *dev_id)
-{
-   struct s3cmci_host *host = (struct s3cmci_host *)dev_id;
-
-   dbg(host, dbg_irq, "card detect\n");
-
-   mmc_detect_change(host->mmc, msecs_to_jiffies(500));
-
-   return IRQ_HANDLED;
-}
-
 static void s3cmci_dma_done_callback(void *arg)
 {
struct s3cmci_host *host = arg;
@@ -1177,19 +1166,6 @@ static void s3cmci_send_request(struct mmc_host *mmc)
s3cmci_enable_irq(host, true);
 }
 
-static int s3cmci_card_present(struct mmc_host *mmc)
-{
-   struct s3cmci_host *host = mmc_priv(mmc);
-   struct s3c24xx_mci_pdata *pdata = host->pdata;
-   int ret;
-
-   if (pdata->no_detect)
-   return -ENOSYS;
-
-   ret = gpio_get_value(pdata->gpio_detect) ? 0 : 1;
-   return ret ^ pdata->detect_invert;
-}
-
 static void s3cmci_request(struct mmc_host *mmc, struct mmc_request *mrq)
 {
struct s3cmci_host *host = mmc_priv(mmc);
@@ -1198,7 +1174,7 @@ static void s3cmci_request(struct mmc_host *mmc, struct 
mmc_request *mrq)
host->cmd_is_stop = 0;
host->mrq = mrq;
 
-   if (s3cmci_card_present(mmc) == 0) {
+   if (mmc_gpio_get_cd(mmc) == 0) {
dbg(host, dbg_err, "%s: no medium present\n", __func__);
host->mrq->cmd->error = -ENOMEDIUM;
mmc_request_done(mmc, mrq);
@@ -1242,8 +1218,9 @@ static void s3cmci_set_ios(struct mmc_host *mmc, struct 
mmc_ios *ios)
case MMC_POWER_ON:
case MMC_POWER_UP:
/* Configure GPE5...GPE10 pins in SD mode */
-   s3c_gpio_cfgall_range(S3C2410_GPE(5), 6, S3C_GPIO_SFN(2),
- S3C_GPIO_PULL_NONE);
+   if (!host->pdev->dev.of_node)
+   s3c_gpio_cfgall_range(S3C2410_GPE(5), 6, 
S3C_GPIO_SFN(2),
+ S3C_GPIO_PULL_NONE);
 
if (host->pdata->set_power)
host->pdata->set_power(ios->power_mode, ios->vdd);
@@ -1255,7 +1232,8 @@ static void s3cmci_set_ios(struct mmc_host *mmc, struct 
mmc_ios *ios)
 
case MMC_POWER_OFF:
default:
-   gpio_direction_output(S3C2410_GPE(5), 0);
+   if (!host->pdev->dev.of_node)
+   gpio_direction_output(S3C2410_GPE(5), 0);
 
if (host->is2440)
mci_con |= S3C2440_SDICON_SDRESET;
@@ -1295,21 +1273,6 @@ static void s3cmci_reset(struct s3cmci_host *host)
writel(con, host->base + S3C2410_SDICON);
 }
 
-static int s3cmci_get_ro(struct mmc_host *mmc)
-{
-   struct s3cmci_host *host = mmc_priv(mmc);
-   struct s3c24xx_mci_pdata *pdata = host->pdata;
-   int ret;
-
-   if (pdata->no_wprotect)
-   return 0;
-
-   ret = gpio_get_value(pdata->gpio_wprotect) ? 1 : 0;
-   ret ^= pdata->wprotect_invert;
-
-   return ret;
-}
-
 static void s3cmci_enable_sdio_irq(struct mmc_host *mmc, int enable)
 {
struct s3cmci_host *host = mmc_priv(mmc);
@@ -1353,8 +1316,8 @@ static void s3cmci_enable_sdio_irq(struct mmc_host *mmc, 
int enable)
 static struct mmc_host_ops s3cmci_ops = {
.request= s3cmci_request,
.set_ios= s3cmci_set_ios,
-   .get_ro = s3cmci_get_ro,
-   .get_cd = s3cmci_card_present,
+   .get_ro = mmc_gpio_get_ro,
+   .get_cd = mmc_gpio_get_cd,
.enable_sdio_irq = s3cmci_enable_sdio_irq,
 };
 
@@ -1545,21 +1508,14 @@ static inline void s3cmci_debugfs_remove(struct 
s3cmci_host *host) { }
 
 #endif /* CONFIG_DEBUG_FS */
 
-static int s3cmci_probe(struct platform_device *pdev)
+static int s3cmci_probe_pdata(struct s3cmci_host *host)
 {
-   struct s3cmci_host *host;
-   struct mmc_host *mmc;
-   int ret;
-   int is2440;
-   int i;
+   struct platform_device *pdev = host->pdev;
+   struct mmc_host *mmc = host->mmc;
+   struct s3c24xx_mci_pdata *pdata;
+   int i, ret;
 
-   is2440 = platform_get_device_id(pdev)->driver_data;
-
-   mmc = mmc_alloc_host(sizeof(struct s3cmci_host), &pdev->dev);
-   if (!mmc) {
-   ret = -ENOMEM;
-   goto probe_out;
-   }
+   host->is2440 = platform_get_device_id(pdev)->driver_data;
 
for (i = S3C2410_GPE(5)

Re: [PATCH 11/16] fpga: intel: fme: add partial reconfiguration sub feature support

Hi Kang,

[auto build test WARNING on linus/master]
[also build test WARNING on v4.11-rc4 next-20170331]
[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/Wu-Hao/Intel-FPGA-Device-Drivers/20170401-052017
config: frv-allmodconfig (attached as .config)
compiler: frv-linux-gcc (GCC) 6.2.0
reproduce:
wget 
https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=frv 

All warnings (new ones prefixed by >>):

   drivers/fpga/intel/fme-pr.c: In function 'interface_id_show':
   drivers/fpga/intel/fme-pr.c:45:15: error: implicit declaration of function 
'readq' [-Werror=implicit-function-declaration]
 intfc_id_l = readq(&fme_pr->intfc_id_l);
  ^
   drivers/fpga/intel/fme-pr.c: In function 'pr_err_handle':
   drivers/fpga/intel/fme-pr.c:79:2: error: implicit declaration of function 
'writeq' [-Werror=implicit-function-declaration]
 writeq(fme_pr_error, &fme_pr->error);
 ^~
   In file included from include/linux/uaccess.h:5:0,
from drivers/fpga/intel/fme-pr.c:25:
   drivers/fpga/intel/fme-pr.c: In function 'fme_pr':
>> arch/frv/include/asm/uaccess.h:63:47: warning: cast to pointer from integer 
>> of different size [-Wint-to-pointer-cast]
#define access_ok(type,addr,size) (__range_ok((void __user *)(addr), 
(size)) == 0)
  ^
   arch/frv/include/asm/uaccess.h:61:60: note: in definition of macro 
'__range_ok'
#define __range_ok(addr,size) ___range_ok((unsigned long) (addr), (unsigned 
long) (size))
   ^~~~
>> drivers/fpga/intel/fme-pr.c:278:7: note: in expansion of macro 'access_ok'
 if (!access_ok(VERIFY_READ, port_pr.buffer_address,
  ^
   drivers/fpga/intel/fme-pr.c:286:26: warning: cast to pointer from integer of 
different size [-Wint-to-pointer-cast]
 if (copy_from_user(buf, (void __user *)port_pr.buffer_address,
 ^
   cc1: some warnings being treated as errors
--
   drivers/fpga//intel/fme-pr.c: In function 'interface_id_show':
   drivers/fpga//intel/fme-pr.c:45:15: error: implicit declaration of function 
'readq' [-Werror=implicit-function-declaration]
 intfc_id_l = readq(&fme_pr->intfc_id_l);
  ^
   drivers/fpga//intel/fme-pr.c: In function 'pr_err_handle':
   drivers/fpga//intel/fme-pr.c:79:2: error: implicit declaration of function 
'writeq' [-Werror=implicit-function-declaration]
 writeq(fme_pr_error, &fme_pr->error);
 ^~
   In file included from include/linux/uaccess.h:5:0,
from drivers/fpga//intel/fme-pr.c:25:
   drivers/fpga//intel/fme-pr.c: In function 'fme_pr':
>> arch/frv/include/asm/uaccess.h:63:47: warning: cast to pointer from integer 
>> of different size [-Wint-to-pointer-cast]
#define access_ok(type,addr,size) (__range_ok((void __user *)(addr), 
(size)) == 0)
  ^
   arch/frv/include/asm/uaccess.h:61:60: note: in definition of macro 
'__range_ok'
#define __range_ok(addr,size) ___range_ok((unsigned long) (addr), (unsigned 
long) (size))
   ^~~~
   drivers/fpga//intel/fme-pr.c:278:7: note: in expansion of macro 'access_ok'
 if (!access_ok(VERIFY_READ, port_pr.buffer_address,
  ^
   drivers/fpga//intel/fme-pr.c:286:26: warning: cast to pointer from integer 
of different size [-Wint-to-pointer-cast]
 if (copy_from_user(buf, (void __user *)port_pr.buffer_address,
 ^
   cc1: some warnings being treated as errors

vim +63 arch/frv/include/asm/uaccess.h

^1da177e4c include/asm-frv/uaccess.h Linus Torvalds 2005-04-16  47  return 
flag;
^1da177e4c include/asm-frv/uaccess.h Linus Torvalds 2005-04-16  48  
^1da177e4c include/asm-frv/uaccess.h Linus Torvalds 2005-04-16  49  #else
^1da177e4c include/asm-frv/uaccess.h Linus Torvalds 2005-04-16  50  
^1da177e4c include/asm-frv/uaccess.h Linus Torvalds 2005-04-16  51  if 
(addr < memory_start ||
^1da177e4c include/asm-frv/uaccess.h Linus Torvalds 2005-04-16  52  
addr > memory_end ||
^1da177e4c include/asm-frv/uaccess.h Linus Torvalds 2005-04-16  53  
size > memory_end - memory_start ||
^1da177e4c include/asm-frv/uaccess.h Linus Torvalds 2005-04-16  54  
addr + size > memory_end)
^1da177e4c include/asm-frv/uaccess.h Linus Torvalds 2005-04-16  55  
return -EFAULT;
^1da177e4c include/asm-frv/uaccess.h Linus Torv

[PATCH] net: ethernet: ti: cpsw: wake tx queues on ndo_tx_timeout

2017-03-31 Thread Grygorii Strashko

In case, if TX watchdog is fired some or all netdev TX queues will be
stopped and as part of recovery it is required not only to drain and
reinitailize CPSW TX channeles, but also wake up stoppted TX queues what
doesn't happen now and netdevice will stop transmiting data until
reopenned.

Hence, add netif_tx_wake_all_queues() call in .ndo_tx_timeout() to complete
recovery and restore TX path.

Signed-off-by: Grygorii Strashko 
---
 drivers/net/ethernet/ti/cpsw.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index 86d6f10..71fd4ef 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -1819,6 +1819,8 @@ static void cpsw_ndo_tx_timeout(struct net_device *ndev)
}
 
cpsw_intr_enable(cpsw);
+   netif_trans_update(ndev);
+   netif_tx_wake_all_queues(ndev);
 }
 
 static int cpsw_ndo_set_mac_address(struct net_device *ndev, void *p)
-- 
2.10.1

[PATCH v4 3/5] watchdog: iTCO_wdt: Add PMC specific noreboot update api

In some SOCs, setting noreboot bit needs modification to
PMC GC registers. But not all PMC drivers allow other drivers
to memory map their GC region. This could create mem request
conflict in watchdog driver. So this patch adds facility to allow
PMC drivers to pass noreboot update function to watchdog
drivers via platform data.

Signed-off-by: Kuppuswamy Sathyanarayanan 

Acked-by: Guenter Roeck 
---
 drivers/watchdog/iTCO_wdt.c| 28 +++-
 include/linux/platform_data/itco_wdt.h |  1 +
 2 files changed, 20 insertions(+), 9 deletions(-)

Changes since v3:
 * Rebased on top of latest.

Changes since v2: 
 * Removed use of PMC API's directly in watchdog driver.
 * Added update_noreboot_flag to handle no IPC PMC compatibility
   issue mentioned by Guenter.

diff --git a/drivers/watchdog/iTCO_wdt.c b/drivers/watchdog/iTCO_wdt.c
index 3d0abc0..7c34259 100644
--- a/drivers/watchdog/iTCO_wdt.c
+++ b/drivers/watchdog/iTCO_wdt.c
@@ -100,6 +100,8 @@ struct iTCO_wdt_private {
 */
struct resource *gcs_pmc_res;
unsigned long __iomem *gcs_pmc;
+   /* pmc specific api to update noreboot flag */
+   int (*update_noreboot_flag)(bool status);
/* the lock for io operations */
spinlock_t io_lock;
/* the PCI-device */
@@ -176,9 +178,13 @@ static void iTCO_wdt_set_NO_REBOOT_bit(struct 
iTCO_wdt_private *p)
 
/* Set the NO_REBOOT bit: this disables reboots */
if (p->iTCO_version >= 2) {
-   val32 = readl(p->gcs_pmc);
-   val32 |= no_reboot_bit(p);
-   writel(val32, p->gcs_pmc);
+   if (p->update_noreboot_flag)
+   p->update_noreboot_flag(1);
+   else {
+   val32 = readl(p->gcs_pmc);
+   val32 |= no_reboot_bit(p);
+   writel(val32, p->gcs_pmc);
+   }
} else if (p->iTCO_version == 1) {
pci_read_config_dword(p->pci_dev, 0xd4, &val32);
val32 |= no_reboot_bit(p);
@@ -193,11 +199,14 @@ static int iTCO_wdt_unset_NO_REBOOT_bit(struct 
iTCO_wdt_private *p)
 
/* Unset the NO_REBOOT bit: this enables reboots */
if (p->iTCO_version >= 2) {
-   val32 = readl(p->gcs_pmc);
-   val32 &= ~enable_bit;
-   writel(val32, p->gcs_pmc);
-
-   val32 = readl(p->gcs_pmc);
+   if (p->update_noreboot_flag)
+   return p->update_noreboot_flag(0);
+   else {
+   val32 = readl(p->gcs_pmc);
+   val32 &= ~enable_bit;
+   writel(val32, p->gcs_pmc);
+   val32 = readl(p->gcs_pmc);
+   }
} else if (p->iTCO_version == 1) {
pci_read_config_dword(p->pci_dev, 0xd4, &val32);
val32 &= ~enable_bit;
@@ -426,13 +435,14 @@ static int iTCO_wdt_probe(struct platform_device *pdev)
return -ENODEV;
 
p->iTCO_version = pdata->version;
+   p->update_noreboot_flag = pdata->update_noreboot_flag;
p->pci_dev = to_pci_dev(dev->parent);
 
/*
 * Get the Memory-Mapped GCS or PMC register, we need it for the
 * NO_REBOOT flag (TCO v2 and v3).
 */
-   if (p->iTCO_version >= 2) {
+   if (p->iTCO_version >= 2 && !p->update_noreboot_flag) {
p->gcs_pmc_res = platform_get_resource(pdev,
   IORESOURCE_MEM,
   ICH_RES_MEM_GCS_PMC);
diff --git a/include/linux/platform_data/itco_wdt.h 
b/include/linux/platform_data/itco_wdt.h
index f16542c..ea1efb7 100644
--- a/include/linux/platform_data/itco_wdt.h
+++ b/include/linux/platform_data/itco_wdt.h
@@ -14,6 +14,7 @@
 struct itco_wdt_platform_data {
char name[32];
unsigned int version;
+   int (*update_noreboot_flag)(bool status);
 };
 
 #endif /* _ITCO_WDT_H_ */
-- 
2.7.4

[PATCH v4 1/5] platform/x86: intel_pmc_ipc: fix gcr offset

According to Broxton APL PMC spec, gcr mem region starts
at offset 0x1000 from ipc mem base address. In this driver,
PLAT_RESOURCE_GCR_OFFSET macro defines the offset of GCR
memory region from IPC mem region. So we should use 0x1000(4K)
as GCR offset. But currently this driver uses 0x1008 as GCT
offset.This patch fixes this issue.

Signed-off-by: Kuppuswamy Sathyanarayanan 

---
 drivers/platform/x86/intel_pmc_ipc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Changes since v3:
 * Updated the commit history

diff --git a/drivers/platform/x86/intel_pmc_ipc.c 
b/drivers/platform/x86/intel_pmc_ipc.c
index 0651d47..0a33592 100644
--- a/drivers/platform/x86/intel_pmc_ipc.c
+++ b/drivers/platform/x86/intel_pmc_ipc.c
@@ -82,7 +82,7 @@
 /* exported resources from IFWI */
 #define PLAT_RESOURCE_IPC_INDEX0
 #define PLAT_RESOURCE_IPC_SIZE 0x1000
-#define PLAT_RESOURCE_GCR_OFFSET   0x1008
+#define PLAT_RESOURCE_GCR_OFFSET   0x1000
 #define PLAT_RESOURCE_GCR_SIZE 0x1000
 #define PLAT_RESOURCE_BIOS_DATA_INDEX  1
 #define PLAT_RESOURCE_BIOS_IFACE_INDEX 2
-- 
2.7.4

[PATCH v4 2/5] platform/x86: intel_pmc_ipc: Add pmc gcr read/write/update api's

This patch adds API's to read/write/update PMC GC registers.
PMC dependent devices like iTCO_WDT, Telemetry has requirement
to acces GCR registers. These API's can be used for this
purpose.

Signed-off-by: Kuppuswamy Sathyanarayanan 

---
 arch/x86/include/asm/intel_pmc_ipc.h | 21 
 drivers/platform/x86/intel_pmc_ipc.c | 95 
 2 files changed, 116 insertions(+)

Changes since v3:
 * Added usage comments for read/write/update api
 * Created a helper function to handle GCR related range checks.

Changes since v2:
 * Removed unused reg offset from header file.
 * Modified read/write api's signatures for better error handling
 * Added function for bit level update of gcr register.

diff --git a/arch/x86/include/asm/intel_pmc_ipc.h 
b/arch/x86/include/asm/intel_pmc_ipc.h
index 4291b6a..8402efe 100644
--- a/arch/x86/include/asm/intel_pmc_ipc.h
+++ b/arch/x86/include/asm/intel_pmc_ipc.h
@@ -23,6 +23,9 @@
 #define IPC_ERR_EMSECURITY 6
 #define IPC_ERR_UNSIGNEDKERNEL 7
 
+/* GCR reg offsets from gcr base*/
+#define PMC_GCR_PMC_CFG_REG0x08
+
 #if IS_ENABLED(CONFIG_INTEL_PMC_IPC)
 
 int intel_pmc_ipc_simple_command(int cmd, int sub);
@@ -31,6 +34,9 @@ int intel_pmc_ipc_raw_cmd(u32 cmd, u32 sub, u8 *in, u32 inlen,
 int intel_pmc_ipc_command(u32 cmd, u32 sub, u8 *in, u32 inlen,
u32 *out, u32 outlen);
 int intel_pmc_s0ix_counter_read(u64 *data);
+int intel_pmc_gcr_read(u32 offset, u32 *data);
+int intel_pmc_gcr_write(u32 offset, u32 data);
+int intel_pmc_gcr_update(u32 offset, u32 mask, u32 val);
 
 #else
 
@@ -56,6 +62,21 @@ static inline int intel_pmc_s0ix_counter_read(u64 *data)
return -EINVAL;
 }
 
+static inline int intel_pmc_gcr_read(u32 offset, u32 *data)
+{
+   return -EINVAL;
+}
+
+static inline int intel_pmc_gcr_write(u32 offset, u32 data)
+{
+   return -EINVAL;
+}
+
+static inline int intel_pmc_gcr_update(u32 offset, u32 mask, u32 val)
+{
+   return -EINVAL;
+}
+
 #endif /*CONFIG_INTEL_PMC_IPC*/
 
 #endif
diff --git a/drivers/platform/x86/intel_pmc_ipc.c 
b/drivers/platform/x86/intel_pmc_ipc.c
index 0a33592..bc95bf2 100644
--- a/drivers/platform/x86/intel_pmc_ipc.c
+++ b/drivers/platform/x86/intel_pmc_ipc.c
@@ -127,6 +127,7 @@ static struct intel_pmc_ipc_dev {
 
/* gcr */
resource_size_t gcr_base;
+   void __iomem *gcr_mem_base;
int gcr_size;
bool has_gcr_regs;
 
@@ -199,6 +200,99 @@ static inline u64 gcr_data_readq(u32 offset)
return readq(ipcdev.ipc_base + offset);
 }
 
+static inline int is_gcr_valid(u32 offset)
+{
+   if (!ipcdev.has_gcr_regs)
+   return -EACCES;
+
+   if (offset > PLAT_RESOURCE_GCR_SIZE)
+   return -EINVAL;
+
+   return 0;
+}
+
+/**
+ * intel_pmc_gcr_read() - Read PMC GCR register
+ * @offset:offset of GCR register from GCR address base
+ * @data:  data pointer for storing the register output
+ *
+ * Reads the PMC GCR register of given offset.
+ *
+ * Return: negative value on error or 0 on success.
+ */
+int intel_pmc_gcr_read(u32 offset, u32 *data)
+{
+   int ret;
+
+   ret = is_gcr_valid(offset);
+   if (ret < 0)
+   return ret;
+
+   *data = readl(ipcdev.gcr_mem_base + offset);
+
+   return 0;
+}
+EXPORT_SYMBOL_GPL(intel_pmc_gcr_read);
+
+/**
+ * intel_pmc_gcr_write() - Write PMC GCR register
+ * @offset:offset of GCR register from GCR address base
+ * @data:  register update value
+ *
+ * Writes the PMC GCR register of given offset with given
+ * value
+ *
+ * Return: negative value on error or 0 on success.
+ */
+int intel_pmc_gcr_write(u32 offset, u32 data)
+{
+   int ret;
+
+   ret = is_gcr_valid(offset);
+   if (ret < 0)
+   return ret;
+
+   writel(data, ipcdev.gcr_mem_base + offset);
+
+   return 0;
+}
+EXPORT_SYMBOL_GPL(intel_pmc_gcr_write);
+
+/**
+ * intel_pmc_gcr_update() - Update PMC GCR register bits
+ * @offset:offset of GCR register from GCR address base
+ * @mask:  bit mask for update operation
+ * @val:   update value
+ *
+ * Updates the bits of given GCR register as specified by
+ * mask and val
+ *
+ * Return: negative value on error or 0 on success.
+ */
+int intel_pmc_gcr_update(u32 offset, u32 mask, u32 val)
+{
+   u32 orig, tmp;
+
+   tmp = is_gcr_valid(offset);
+   if (tmp < 0)
+   return tmp;
+
+   orig = readl(ipcdev.gcr_mem_base + offset);
+
+   tmp = orig & ~mask;
+   tmp |= val & mask;
+
+   writel(tmp, ipcdev.gcr_mem_base + offset);
+
+   tmp = readl(ipcdev.gcr_mem_base + offset);
+
+   if ((tmp & mask) != (val & mask))
+   return -EIO;
+
+   return 0;
+}
+EXPORT_SYMBOL_GPL(intel_pmc_gcr_update);
+
 static int intel_pmc_ipc_check_status(void)
 {
int status;
@@ -747,6 +841,7 @@ static int ipc_plat_get_res(struct platform_device *pdev)
ipcdev.ipc_base = addr;
 
ipcdev.gcr_base = r

[PATCH v4 5/5] platform/x86: intel_pmc_ipc: use gcr mem base for S0ix counter read

To maintain the uniformity in accessing GCR registers, this patch
modifies the S0ix counter read function to use GCR address base
instead of ipc address base.

Signed-off-by: Kuppuswamy Sathyanarayanan 

Reviewed-by: Rajneesh Bhardwaj 
Tested-by: Shanth Murthy 
---
 arch/x86/include/asm/intel_pmc_ipc.h |  2 ++
 drivers/platform/x86/intel_pmc_ipc.c | 10 +++---
 2 files changed, 5 insertions(+), 7 deletions(-)

Changes since v3:
 * Rebased on top of latest changes.

diff --git a/arch/x86/include/asm/intel_pmc_ipc.h 
b/arch/x86/include/asm/intel_pmc_ipc.h
index 8402efe..fac89eb 100644
--- a/arch/x86/include/asm/intel_pmc_ipc.h
+++ b/arch/x86/include/asm/intel_pmc_ipc.h
@@ -25,6 +25,8 @@
 
 /* GCR reg offsets from gcr base*/
 #define PMC_GCR_PMC_CFG_REG0x08
+#define PMC_GCR_TELEM_DEEP_S0IX_REG0x78
+#define PMC_GCR_TELEM_SHLW_S0IX_REG0x80
 
 #if IS_ENABLED(CONFIG_INTEL_PMC_IPC)
 
diff --git a/drivers/platform/x86/intel_pmc_ipc.c 
b/drivers/platform/x86/intel_pmc_ipc.c
index 3f3ee50..c7b5517 100644
--- a/drivers/platform/x86/intel_pmc_ipc.c
+++ b/drivers/platform/x86/intel_pmc_ipc.c
@@ -57,10 +57,6 @@
 #define IPC_WRITE_BUFFER   0x80
 #define IPC_READ_BUFFER0x90
 
-/* PMC Global Control Registers */
-#define GCR_TELEM_DEEP_S0IX_OFFSET 0x1078
-#define GCR_TELEM_SHLW_S0IX_OFFSET 0x1080
-
 /* Residency with clock rate at 19.2MHz to usecs */
 #define S0IX_RESIDENCY_IN_USECS(d, s)  \
 ({ \
@@ -196,7 +192,7 @@ static inline u32 ipc_data_readl(u32 offset)
 
 static inline u64 gcr_data_readq(u32 offset)
 {
-   return readq(ipcdev.ipc_base + offset);
+   return readq(ipcdev.gcr_mem_base + offset);
 }
 
 static inline int is_gcr_valid(u32 offset)
@@ -877,8 +873,8 @@ int intel_pmc_s0ix_counter_read(u64 *data)
if (!ipcdev.has_gcr_regs)
return -EACCES;
 
-   deep = gcr_data_readq(GCR_TELEM_DEEP_S0IX_OFFSET);
-   shlw = gcr_data_readq(GCR_TELEM_SHLW_S0IX_OFFSET);
+   deep = gcr_data_readq(PMC_GCR_TELEM_DEEP_S0IX_REG);
+   shlw = gcr_data_readq(PMC_GCR_TELEM_SHLW_S0IX_REG);
 
*data = S0IX_RESIDENCY_IN_USECS(deep, shlw);
 
-- 
2.7.4

[PATCH v4 4/5] platform/x86: intel_pmc_ipc: Fix iTCO GCS memory mapping failure

iTCO watchdog driver need access to PMC_CFG GCR register to modify
the no reboot setting. Currently, this is done by passing PMC_CFG reg
address as memory resource to watchdog driver and allowing it directly
modify the PMC_CFG register. But currently PMC driver also has
requirement to memory map the entire GCR register space in this driver.
This causes mem request failure in watchdog driver. So this patch fixes
this issue by adding api to update noreboot flag and passes them
to watchdog driver via platform data.

Signed-off-by: Kuppuswamy Sathyanarayanan 

---
 drivers/platform/x86/intel_pmc_ipc.c | 20 ++--
 1 file changed, 10 insertions(+), 10 deletions(-)

Changes since v3:
 * Rebased on top of latest changes.

Changes since v2: 
 * Added support for update_noreboot_bit api.

diff --git a/drivers/platform/x86/intel_pmc_ipc.c 
b/drivers/platform/x86/intel_pmc_ipc.c
index bc95bf2..3f3ee50 100644
--- a/drivers/platform/x86/intel_pmc_ipc.c
+++ b/drivers/platform/x86/intel_pmc_ipc.c
@@ -126,7 +126,6 @@ static struct intel_pmc_ipc_dev {
struct platform_device *tco_dev;
 
/* gcr */
-   resource_size_t gcr_base;
void __iomem *gcr_mem_base;
int gcr_size;
bool has_gcr_regs;
@@ -293,6 +292,15 @@ int intel_pmc_gcr_update(u32 offset, u32 mask, u32 val)
 }
 EXPORT_SYMBOL_GPL(intel_pmc_gcr_update);
 
+static int update_noreboot_bit(bool status)
+{
+   if (status)
+   return intel_pmc_gcr_update(PMC_GCR_PMC_CFG_REG, BIT(4),
+   BIT(4));
+   else
+   return intel_pmc_gcr_update(PMC_GCR_PMC_CFG_REG, BIT(4), 0);
+}
+
 static int intel_pmc_ipc_check_status(void)
 {
int status;
@@ -610,15 +618,12 @@ static struct resource tco_res[] = {
{
.flags = IORESOURCE_IO,
},
-   /* GCS */
-   {
-   .flags = IORESOURCE_MEM,
-   },
 };
 
 static struct itco_wdt_platform_data tco_info = {
.name = "Apollo Lake SoC",
.version = 5,
+   .update_noreboot_flag = update_noreboot_bit,
 };
 
 #define TELEMETRY_RESOURCE_PUNIT_SSRAM 0
@@ -675,10 +680,6 @@ static int ipc_create_tco_device(void)
res->start = ipcdev.acpi_io_base + SMI_EN_OFFSET;
res->end = res->start + SMI_EN_SIZE - 1;
 
-   res = tco_res + TCO_RESOURCE_GCR_MEM;
-   res->start = ipcdev.gcr_base + TCO_PMC_OFFSET;
-   res->end = res->start + TCO_PMC_SIZE - 1;
-
pdev = platform_device_register_full(&pdevinfo);
if (IS_ERR(pdev))
return PTR_ERR(pdev);
@@ -840,7 +841,6 @@ static int ipc_plat_get_res(struct platform_device *pdev)
}
ipcdev.ipc_base = addr;
 
-   ipcdev.gcr_base = res->start + PLAT_RESOURCE_GCR_OFFSET;
ipcdev.gcr_mem_base = addr + PLAT_RESOURCE_GCR_OFFSET;
ipcdev.gcr_size = PLAT_RESOURCE_GCR_SIZE;
dev_info(&pdev->dev, "ipc res: %pR\n", res);
-- 
2.7.4

Re: [BUG nohz]: wrong user and system time accounting

2017-03-31 Thread Frederic Weisbecker

On Fri, Mar 31, 2017 at 04:09:10PM -0400, Luiz Capitulino wrote:
> On Thu, 30 Mar 2017 17:25:46 -0400
> Luiz Capitulino  wrote:
> 
> > On Thu, 30 Mar 2017 16:18:17 +0200
> > Frederic Weisbecker  wrote:
> > 
> > > On Thu, Mar 30, 2017 at 09:59:54PM +0800, Wanpeng Li wrote:  
> > > > 2017-03-30 21:38 GMT+08:00 Frederic Weisbecker :
> > > > > If it works, we may want to take that solution, likely less 
> > > > > performance sensitive
> > > > > than using sched_clock(). In fact sched_clock() is fast, especially 
> > > > > as we require it to
> > > > > be stable for nohz_full, but using it involves costly conversion back 
> > > > > and forth to jiffies.
> > > > 
> > > > So both Rik and you agree with the skew tick solution, I will try it
> > > > tomorrow. Btw, if we should just add random offset to the cpu in the
> > > > nohz_full mode or add random offset to all cpus like the codes above?   
> > > >  
> > > 
> > > Lets just keep it to all CPUs for simplicty.
> > > Also please add a comment that explains why we need that skew_tick on 
> > > nohz_full.  
> > 
> > I've tried all the test-cases we discussed in this thread with skew_tick=1
> > and it worked as expected in bare-metal and KVM guests.
> > 
> > However, I found a test-case that works in bare-metal but show problems
> > in KVM guests. It could something that's KVM specific, or it could be
> > something that's harder to reproduce in bare-metal.
> 
> After discussing some findings on this issue with Rik, I realized that
> we don't add the skew when restarting the tick in tick_nohz_restart().
> Adding the offset there seems to solve this problem.

Are you sure? tick_nohz_restart() doesn't seem to override the initial skew. It
always forwards the expiration time on top of the last tick.

Re: dwc2_hc_chhltd_intr_dma - ChHltd set errors?

2017-03-31 Thread John Stultz

On Thu, Mar 2, 2017 at 12:00 PM, John Stultz  wrote:
> Hey John,
>   We've noticed that when using usb ethernet adapters on HiKey, we
> occasionally see errors like:
>
> dwc2 f72c.usb: dwc2_hc_chhltd_intr_dma: Channel 0 - ChHltd set,
> but reason is unknown
> dwc2 f72c.usb: hcint 0x0002, intsts 0x06200029
>
> dwc2 f72c.usb: dwc2_hc_chhltd_intr_dma: Channel 11 - ChHltd set,
> but reason is unknown
> dwc2 f72c.usb: hcint 0x0002, intsts 0x04200029
>
> Sometimes followed up by a usb error in the driver, something like:
> asix 1-1.2:1.0 eth0: asix_rx_fixup() Bad Header Length 0x36000807, offset 68
>
> Curious if you've seen any reports like this?

Hey John,
  Just wanted to check in again on this to see if you've seen anything
like it? I've not had too much time to debug it, but my attempts so
far haven't been productive.

thanks
-john

Re: [PATCH V8 5/6] ACPI: Support the probing on the devices which apply indirect-IO

2017-03-31 Thread Rafael J. Wysocki

On Fri, Mar 31, 2017 at 8:52 AM, zhichang.yuan
 wrote:
> Hi, Rafael,
>
> Thanks for reviewing this!
>
> On 2017/3/31 4:31, Rafael J. Wysocki wrote:
>> On Thursday, March 30, 2017 11:26:58 PM zhichang.yuan wrote:
>>> On some platforms(such as Hip06/Hip07), the legacy ISA/LPC devices access 
>>> I/O
>>> with some special host-local I/O ports known on x86. To access the I/O
>>> peripherals, an indirect-IO mechanism is introduced to mapped the host-local
>>> I/O to system logical/fake PIO similar the PCI MMIO on architectures where 
>>> no
>>> separate I/O space exists. Just as PCI MMIO, the host I/O range should be
>>> registered before probing the downstream devices and set up the I/O mapping.
>>> But current ACPI bus probing doesn't support these indirect-IO 
>>> hosts/devices.
>>>
>>> This patch introdueces a new ACPI handler for this device category. Through 
>>> the
>>> handler attach callback, the indirect-IO hosts I/O registration is done and
>>> all peripherals' I/O resources are translated into logic/fake PIO before
>>> starting the enumeration.
>>
>> Can you explain to me briefly what exactly this code is expected to be doing?
>
> As you know currently for ARM architecture IO space is memory mapped and
> is only used by pci devices. The port number is dynamically allocated
> converting the device IO address into a PIO token: i.e.
> http://lxr.free-electrons.com/source/drivers/acpi/pci_root.c#L745
> This patch is meant to support a new class of IO host controller
> that are not PCI based and that still require to have the IO addresses
> be translated in the same PIO token space as the PCI controller

IOW, this is ARM-specific, right?

Thanks,
Rafael

[PATCH] ACPI / scan: Prefer devices without _HID for _ADR matching

2017-03-31 Thread Rafael J. Wysocki

From: Rafael J. Wysocki 

Commit c2a6bbaf0c5f (ACPI / scan: Prefer devices without _HID/_CID
for _ADR matching) added a list_empty(&adev->pnp.ids) check to
find_child_checks() so as to catch situations in which the ACPI
core attempts to decode _ADR for a device having a _HID too which
is strictly against the spec.  However, it overlooked the fact that
the adev->pnp.ids list for the devices taken into account by
find_child_checks() may contain device IDs set internally by the
kernel, like "LNXVIDEO" (thanks to Zhang Rui for that realization),
and it broke the enumeration of those devices as a result.

To unbreak it, replace the overly coarse grained list_empty()
check with a much more precise check against the pnp.type.platform_id
flag which is only set for devices having a _HID (that's how it
should be done from the start, as having both _ADR and _CID is
actually permitted).

Fixes: c2a6bbaf0c5f (ACPI / scan: Prefer devices without _HID/_CID for _ADR 
matching)
Link: https://bugzilla.kernel.org/show_bug.cgi?id=194889
Reported-and-tested-by: Mike 
Tested-by: Hans de Goede 
Signed-off-by: Rafael J. Wysocki 
---
 drivers/acpi/glue.c |   12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

Index: linux-pm/drivers/acpi/glue.c
===
--- linux-pm.orig/drivers/acpi/glue.c
+++ linux-pm/drivers/acpi/glue.c
@@ -99,13 +99,13 @@ static int find_child_checks(struct acpi
return -ENODEV;
 
/*
-* If the device has a _HID (or _CID) returning a valid ACPI/PNP
-* device ID, it is better to make it look less attractive here, so that
-* the other device with the same _ADR value (that may not have a valid
-* device ID) can be matched going forward.  [This means a second spec
-* violation in a row, so whatever we do here is best effort anyway.]
+* If the device has a _HID returning a valid ACPI/PNP device ID, it is
+* better to make it look less attractive here, so that the other device
+* with the same _ADR value (that may not have a valid device ID) can be
+* matched going forward.  [This means a second spec violation in a row,
+* so whatever we do here is best effort anyway.]
 */
-   return sta_present && list_empty(&adev->pnp.ids) ?
+   return sta_present && !adev->pnp.type.platform_id ?
FIND_CHILD_MAX_SCORE : FIND_CHILD_MIN_SCORE;
 }

[PATCH] sparc64: Fix memory corruption when THP is enabled

2017-03-31 Thread Nitin Gupta

The memory corruption was happening due to incorrect
TLB/TSB flushing of hugepages.

Reported-by: David S. Miller 
Signed-off-by: Nitin Gupta 
---
 arch/sparc/mm/tlb.c | 6 +++---
 arch/sparc/mm/tsb.c | 4 ++--
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/sparc/mm/tlb.c b/arch/sparc/mm/tlb.c
index afda3bb..ee8066c 100644
--- a/arch/sparc/mm/tlb.c
+++ b/arch/sparc/mm/tlb.c
@@ -154,7 +154,7 @@ static void tlb_batch_pmd_scan(struct mm_struct *mm, 
unsigned long vaddr,
if (pte_val(*pte) & _PAGE_VALID) {
bool exec = pte_exec(*pte);
 
-   tlb_batch_add_one(mm, vaddr, exec, false);
+   tlb_batch_add_one(mm, vaddr, exec, PAGE_SHIFT);
}
pte++;
vaddr += PAGE_SIZE;
@@ -209,9 +209,9 @@ void set_pmd_at(struct mm_struct *mm, unsigned long addr,
pte_t orig_pte = __pte(pmd_val(orig));
bool exec = pte_exec(orig_pte);
 
-   tlb_batch_add_one(mm, addr, exec, true);
+   tlb_batch_add_one(mm, addr, exec, REAL_HPAGE_SHIFT);
tlb_batch_add_one(mm, addr + REAL_HPAGE_SIZE, exec,
-   true);
+ REAL_HPAGE_SHIFT);
} else {
tlb_batch_pmd_scan(mm, addr, orig);
}
diff --git a/arch/sparc/mm/tsb.c b/arch/sparc/mm/tsb.c
index 0a04811..bedf08b 100644
--- a/arch/sparc/mm/tsb.c
+++ b/arch/sparc/mm/tsb.c
@@ -122,7 +122,7 @@ void flush_tsb_user(struct tlb_batch *tb)
 
spin_lock_irqsave(&mm->context.lock, flags);
 
-   if (tb->hugepage_shift < HPAGE_SHIFT) {
+   if (tb->hugepage_shift < REAL_HPAGE_SHIFT) {
base = (unsigned long) mm->context.tsb_block[MM_TSB_BASE].tsb;
nentries = mm->context.tsb_block[MM_TSB_BASE].tsb_nentries;
if (tlb_type == cheetah_plus || tlb_type == hypervisor)
@@ -155,7 +155,7 @@ void flush_tsb_user_page(struct mm_struct *mm, unsigned 
long vaddr,
 
spin_lock_irqsave(&mm->context.lock, flags);
 
-   if (hugepage_shift < HPAGE_SHIFT) {
+   if (hugepage_shift < REAL_HPAGE_SHIFT) {
base = (unsigned long) mm->context.tsb_block[MM_TSB_BASE].tsb;
nentries = mm->context.tsb_block[MM_TSB_BASE].tsb_nentries;
if (tlb_type == cheetah_plus || tlb_type == hypervisor)
-- 
2.9.2

Re: [PATCH v5 2/2] mmc: host: s3cmci: allow probing from device tree

On Fri, Mar 31, 2017 at 01:11:59PM +0200, Ulf Hansson wrote:
> On 31 March 2017 at 09:25, Arnd Bergmann  wrote:
> > On Sat, Mar 18, 2017 at 2:13 AM, Sergio Prado
> >  wrote:
> >
> >>
> >> +static const struct of_device_id s3cmci_dt_match[] = {
> >> +   {
> >> +   .compatible = "samsung,s3c2410-sdi",
> >> +   .data = (void *)0,
> >> +   },
> >> +   {
> >> +   .compatible = "samsung,s3c2412-sdi",
> >> +   .data = (void *)1,
> >> +   },
> >> +   {
> >> +   .compatible = "samsung,s3c2440-sdi",
> >> +   .data = (void *)1,
> >> +   },
> >> +   { /* sentinel */ },
> >> +};
> >> +MODULE_DEVICE_TABLE(of, sdhci_s3c_dt_match);
> >> +
> >
> > The names don't match:
> >
> > In file included from /git/arm-soc/drivers/mmc/host/s3cmci.c:14:0:
> > /git/arm-soc/drivers/mmc/host/s3cmci.c:1844:25: error:
> > 'sdhci_s3c_dt_match' undeclared here (not in a function); did you mean
> > 's3cmci_dt_match'?
> >  MODULE_DEVICE_TABLE(of, sdhci_s3c_dt_match);
> >  ^
> > /git/arm-soc/include/linux/module.h:212:21: note: in definition of
> > macro 'MODULE_DEVICE_TABLE'
> >  extern const typeof(name) __mod_##type##__##name##_device_table  \
> >  ^~~~
> > /git/arm-soc/include/linux/module.h:212:27: error:
> > '__mod_of__sdhci_s3c_dt_match_device_table' aliased to undefined
> > symbol 'sdhci_s3c_dt_match'
> >  extern const typeof(name) __mod_##type##__##name##_device_table  \
> >^
> > /git/arm-soc/drivers/mmc/host/s3cmci.c:1844:1: note: in expansion of
> > macro 'MODULE_DEVICE_TABLE'
> >  MODULE_DEVICE_TABLE(of, sdhci_s3c_dt_match);
> >  ^~~
> > /git/arm-soc/scripts/Makefile.build:314: recipe for target
> > 'drivers/mmc/host/s3cmci.o' failed
> >
> > Can you send a fix?
> 
> Never mind, I am dropping the patches from my next branch as those
> clearly haven't been built/tested by Sergio properly. It's better that
> he re-spins them.
> 
> Arnd, thanks for reporting and even fixing some of the problems!
> 
> Kind regards
> Uffe

Arnd, thanks for reporting the error.

I have built/tested the patches, but only as a built-in feature, and
this error occurs when enabled as a module.

I will re-spins them and send v6.

Best regards,

Sergio Prado

[PATCH] um: Fix PTRACE_POKEUSER on x86_64

2017-03-31 Thread Richard Weinberger

This is broken since ever but sadly nobody noticed.
Recent versions of GDB set DR_CONTROL unconditionally and
UML dies due to a heap corruption. It turns out that
the PTRACE_POKEUSER was copy&pasted from i386 and assumes
that addresses are 4 bytes long.

Fix that by using 8 as address size in the calculation.

Cc: 
Reported-by: jie cao 
Signed-off-by: Richard Weinberger 
---
 arch/x86/um/ptrace_64.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/um/ptrace_64.c b/arch/x86/um/ptrace_64.c
index a5c9910d234f..09a085bde0d4 100644
--- a/arch/x86/um/ptrace_64.c
+++ b/arch/x86/um/ptrace_64.c
@@ -125,7 +125,7 @@ int poke_user(struct task_struct *child, long addr, long 
data)
else if ((addr >= offsetof(struct user, u_debugreg[0])) &&
(addr <= offsetof(struct user, u_debugreg[7]))) {
addr -= offsetof(struct user, u_debugreg[0]);
-   addr = addr >> 2;
+   addr = addr >> 3;
if ((addr == 4) || (addr == 5))
return -EIO;
child->thread.arch.debugregs[addr] = data;
-- 
2.10.2

Re: [RFC 1/8] Introduce Peer-to-Peer memory (p2pmem) device

2017-03-31 Thread Logan Gunthorpe

On 31/03/17 03:38 PM, Sinan Kaya wrote:
> On 3/31/2017 5:23 PM, Logan Gunthorpe wrote:
>> What exactly would you white/black list? It can't be the NIC or the
>> disk. If it's going to be a white/black list on the switch or root port
>> then you'd need essentially the same code to ensure they are all behind
>> the same switch or root port.
> 
> What is so special about being connected to the same switch?
> 
> Why don't we allow the feature by default and blacklist by the root ports
> that don't work with a quirk.

Well root ports have the same issue here. There may be more than one
root port or other buses (ie QPI) between the devices in question. So
you can't just say "this system has root port X therefore we can always
use p2pmem". In the end, if you want to do any kind of restrictions
you're going to have to walk the tree, as the code currently does, and
figure out what's between the devices being used and black or white list
accordingly. Then seeing there's just such a vast number of devices out
there you'd almost certainly have to use some kind of white list and not
a black list. Then the question becomes which devices will be white
listed? The first to be listed would be switches seeing they will always
work. This is pretty much what we have (though it doesn't currently
cover multiple levels of switches). The next step, if someone wanted to
test with specific hardware, might be to allow the case where all the
devices are behind the same root port which Intel Ivy Bridge or newer.
However, I don't think a comprehensive white list should be a
requirement for this work to go forward and I don't think anything
you've suggested will remove any of the "ugliness".

What we discussed at LSF was that only allowing cases with a switch was
the simplest way to be sure any given setup would actually work.

> I'm looking at this from portability perspective to be honest.

I'm looking at this from the fact that there's a vast number of
topologies and devices involved, and figuring out which will work is
very complicated and could require a lot of hardware testing. The LSF
folks were primarily concerned with not having users enable the feature
and see breakage or terrible performance.

> I'd rather see the feature enabled by default without any assumptions.
> Using it with a switch is just a use case that you happened to test.
> It can allow new architectures to use your code tomorrow.

That's why I was advocating for letting userspace decide such that if
you're setting up a system with this you say to use a specific p2pmem
device and then you are responsible to test and benchmark it and decide
to use it in going forward. However, this has received a lot of push back.

Logan

Re: memory hotplug and force_remove

2017-03-31 Thread Rafael J. Wysocki

On Friday, March 31, 2017 02:02:36 PM Michal Hocko wrote:
> On Fri 31-03-17 19:55:30, Joey Lee wrote:
> > On Fri, Mar 31, 2017 at 12:55:05PM +0200, Michal Hocko wrote:
> > > On Fri 31-03-17 18:49:05, Joey Lee wrote:
> > > > Hi Michal,
> > > > 
> > > > On Fri, Mar 31, 2017 at 10:30:17AM +0200, Michal Hocko wrote:
> > > [...]
> > > > > @@ -241,11 +232,10 @@ static int acpi_scan_try_to_offline(struct 
> > > > > acpi_device *device)
> > > > >   acpi_walk_namespace(ACPI_TYPE_ANY, handle, 
> > > > > ACPI_UINT32_MAX,
> > > > >   NULL, acpi_bus_offline, (void 
> > > > > *)true,
> > > > >   (void **)&errdev);
> > > > > - if (!errdev || acpi_force_hot_remove)
> > > > > + if (!errdev)
> > > > >   acpi_bus_offline(handle, 0, (void *)true,
> > > > >(void **)&errdev);
> > > > > -
> > > > > - if (errdev && !acpi_force_hot_remove) {
> > > > > + else {
> > > >   ^
> > > > Here should still checks the parent's errdev state then rollback
> > > > parent/children to online state:
> > > > 
> > > > -   if (errdev && !acpi_force_hot_remove) {
> > > > +   if (errdev) {
> > > 
> > > You are right, I have missed that acpi_bus_offline modifies errdev.
> > > Thanks for spotting that! Updated patch is below.
> > > ---
> > > >From 8df0abd29988ffb52b6df52407b96d6015861bb7 Mon Sep 17 00:00:00 2001
> > > From: Michal Hocko 
> > > Date: Fri, 31 Mar 2017 10:08:41 +0200
> > > Subject: [PATCH] acpi: drop support for force_remove
> > > 
> > > /sys/firmware/acpi/hotplug/force_remove was presumably added to support
> > > auto offlining in the past. This is, however, inherently dangerous for
> > > some hotplugable resources like memory. The memory offlining fails when
> > > the memory is still in use and cannot be dropped or migrated. If we
> > > ignore the failure we are basically allowing for subtle memory
> > > corruption or a crash.
> > > 
> > > We have actually noticed the later while hitting BUG() during the memory
> > > hotremove (remove_memory):
> > >   ret = walk_memory_range(PFN_DOWN(start), PFN_UP(start + size - 1), NULL,
> > >   check_memblock_offlined_cb);
> > >   if (ret)
> > >   BUG();
> > > 
> > > it took us quite non-trivial time realize that the customer had
> > > force_remove enabled. Even if the BUG was removed here and we could
> > > propagate the error up the call chain it wouldn't help at all because
> > > then we would hit a crash or a memory corruption later and harder to
> > > debug. So force_remove is unfixable for the memory hotremove. We haven't
> > > checked other hotplugable resources to be prone to a similar problems.
> > > 
> > > Remove the force_remove functionality because it is not fixable currently.
> > > Keep the sysfs file and report an error if somebody tries to enable it.
> > > Encourage users to report about the missing functionality and work with
> > > them with an alternative solution.
> > > 
> > > Signed-off-by: Michal Hocko 
> > 
> > This patch is good to me. Please feel free to add:
> > 
> > Reviewed-by: Lee, Chun-Yi 
> 
> Thanks for the review Joey!

Can you please resend it with a CC to linux-acpi to give the people on that list
a chance to speak up if they have any concerns?

Thanks,
Rafael

Re: Synaptics RMI4 touchpad regression in 4.11-rc1

2017-03-31 Thread Andrew Duggan

On 03/31/2017 01:57 AM, Benjamin Tissoires wrote:

On Mar 29 2017 or thereabouts, Andrew Duggan wrote:

On 03/29/2017 01:50 AM, Benjamin Tissoires wrote:

On Mar 28 2017 or thereabouts, Andrew Duggan wrote:

On 03/19/2017 10:00 PM, Peter Hutterer wrote:

On Fri, Mar 17, 2017 at 12:23:36PM -0700, Andrew Duggan wrote:

On 03/17/2017 09:57 AM, Benjamin Tissoires wrote:

On Wed, Mar 15, 2017 at 2:19 AM, Andrew Duggan wrote:

On 03/13/2017 10:10 PM, Cameron Gutman wrote:

On 03/13/2017 06:35 PM, Andrew Duggan wrote:

On 03/13/2017 06:15 AM, Benjamin Tissoires wrote:

[Resending, forgot to add Jiri in CC]

On Mar 13 2017 or thereabouts, Benjamin Tissoires wrote:

On Mar 13 2017 or thereabouts, Thorsten Leemhuis wrote:

Lo! On 12.03.2017 02:55, Cameron Gutman wrote:

Beginning in 4.11-rc1, it looks like RMI4 is binding to my XPS 13
9343's
Synaptics touchpad and dropping some errors into dmesg. Here are the
messages that seem RMI-related:

rmi4_f34 rmi4-00.fn34: rmi_f34v7_probe: Unrecognized bootloader
version
rmi4_f34: probe of rmi4-00.fn34 failed with error -22
rmi4_f01 rmi4-00.fn01: found RMI device, manufacturer: Synaptics,
product: TM3038-001, fw id: 1832324
input: Synaptics TM3038-001 as
/devices/pci:00/INT3433:00/i2c-7/i2c-DLL0665:01/0018:06CB:76AD.0001/input/input19
hid-rmi 0018:06CB:76AD.0001: input,hidraw0: I2C HID v1.00 Mouse
[DLL0665:01 06CB:76AD] on i2c-DLL0665:01

FWIW, I get this on my XPS 13 DE (9360) with 4.11-rc1:

input: SynPS/2 Synaptics TouchPad as
/devices/platform/i8042/serio1/input/input6
rmi4_f34 rmi4-00.fn34: rmi_f34v7_probe: Unrecognized bootloader
version
rmi4_f34: probe of rmi4-00.fn34 failed with error -22
rmi4_f01 rmi4-00.fn01: found RMI device, manufacturer: Synaptics,
product: TM3038-003, fw id: 2375007
input: Synaptics TM3038-003 as

/devices/pci:00/:00:15.1/i2c_designware.1/i2c-8/i2c-DLL075B:01/0018:06CB:76AF.0001/input/input20
hid-rmi 0018:06CB:76AF.0001: input,hidraw0: I2C HID v1.00 Mouse
[DLL075B:01 06CB:76AF] on i2c-DLL075B:01

[…]
Compared to hid-multitouch, the RMI stack seems to have completely
broken
palm rejection and introduced some random jumpiness during fine
pointing
motions. I don't know if these issues are caused by the above errors
or
are a separate issue.

The error about the bootloader version not being recognized just means
that updating the firmware is not supported on this touchpad. It is only the
F34 firmware update functionality which is failing to load. The palm
rejection and jumps are not related to this error.

Maybe that code path should be changed to not make as much noise when it
runs
on known unsupported hardware. Something like the attached patch?

Looking at how hid-multitouch handles palms it looks like palms should
not be reported as active when calling input_mt_report_slot_state(). I'm
setting the tool type to MT_TOOL_PALM when the firmware determines that a
contact is a palm. But, that does not seem to be sufficient enough to have
the palms filtered out. After some quick testing it looks like the change
below will restore palm rejection similar to that provided by
hid-multitouch.

It looks like your email client ate the tabs, but if I apply the change
myself it seems to fix the palm rejection regression for me.

Tested-by: Cameron Gutman

Sorry, I was short on time and just copied the diff into the email. I'll
submit a proper patch soon with your tested-by included. Thanks for testing.

I just pointed out this patch (well the actual submission) to Jason
(Cc-ed). Given that there is no proper handling of MT_TOOL_PALM yet in
userspace, I thought it was the easiest way.
However, it seems that this doesn't enhance the jumps and just make it worse.

I was assuming that the jumps and palm rejection where two separate issues.
But, the palm rejection patch makes things worse?

Is there anything we can do to fix it (besides temporary disabling the
automatic loading of hid-rmi)?

I do not have a fix for the jumps yet. My next step is to file a bug against
libinput or the kernel. I used evemu-record to capture a log with jumps, but
when I play it back with a different userspace input stack with an older
version of libinput I do not see the jumps. I see the jumps on Fedora 25
with libinput 1.6.3 vs Ubuntu 16.10 with libinput 1.4.3 with X). Or at least
the jumps are not as significant. But, its possible there is an issue with
how the events are being reported which is incorrect and confusing libinput.
The X and Y coordinates being reported by the firmware seem correct and I
haven't found a problem yet. I thought a bug would be a better place to
collect evemu-record logs and compare.

fwiw, there's a fairly easy way to quickly check libinput for changes and
that's by having the gtk3-headers installed at configure time and then
running sudo ./tools/event-gui to visualize the movement (Esc quits)

That tool uses libinput data directly to draw pointer motion etc, so it's a
way to quickly bisect to where changes happen. Without anything els

Re: [PATCH v2] module: check if memory leak by module.

2017-03-31 Thread Jessica Yu


+++ Vaneet Narang [29/03/17 09:23 +]:

Hi,


Hmm, how can you track _all_ vmalloc allocations done on behalf of the
module? It is quite some time since I've checked kernel/module.c but
from my vague understading your check is basically only about statically
vmalloced areas by module loader. Is that correct? If yes then is this
actually useful? Were there any bugs in the loader code recently? What
led you to prepare this patch? All this should be part of the changelog!


First of all there is no issue in kernel/module.c. This patch add functionality
to detect scenario where some kernel module does some memory allocation but gets
unloaded without doing vfree. For example
static int kernel_init(void)
{
   char * ptr = vmalloc(400 * 1024);
   return 0;
}

static void kernel_exit(void)
{
}

Now in this case if we do rmmod then memory allocated by kernel_init
will not be freed but this patch will detect such kind of bugs in kernel module
code.


kmemleak already detects leaks just like this, and it is not just
limited to vmalloc (but also kmalloc, kmem_cache_alloc, etc). See
mm/kmemleak-test.c, it is exactly like your example.

Also, this patch is currently limited to direct vmalloc allocations
from module core code (since you are only checking for vmalloc callers
originating from mod->core_layout, not mod->init_layout, which is
discarded at the end of do_init_module(). If we want to be complete,
we'd have to do another leak check before module init code is freed.


Also We have seen bugs in some kernel modules where they allocate some memory 
and
gets removed without freeing them and if new module gets loaded in place
of removed module then /proc/vmallocinfo shows wrong information. vmalloc info 
will
show pages getting allocated by new module. So these logs will help in detecting
such issues.


This is an unfortunate side effect of having dynamically loadable modules.
After a module is gone, sprint_symbol() (which is used to display caller
information in /proc/vmallocinfo) simply cannot trace an address back to
a module that no longer exists, it is a natural limitation, and I'm not really
sure if there's much we can do about it. When chasing leaks like this,
one possibility might be to leave the module loaded so vmallocinfo can report
accurate information, and then compare the reported information after the
module unloads.

And unfortunately, this patch also demonstrates the same problem you're 
describing:

(1) Load leaky_module and read /proc/vmallocinfo:
0xa8570005d000-0xa8570005f0008192 leaky_function+0x2f/0x75 
[leaky_module] pages=1 vmalloc N0=1

(2) Unload leaky_module and read /proc/vmallocinfo:
0xa8570005d000-0xa8570005f0008192 0xc038902f pages=1 
vmalloc N0=1
 ^^^ missing caller symbol since 
module is now gone
On module unload, your patch prints:
[  289.834428] Module [leaky_module] is getting unloaded before doing vfree
[  289.835226] Memory still allocated: addr:0xa8570005d000 - 
0xa8570005f000, pages 1
[  289.836185] Allocating function leaky_function+0x2f/0x75 [leaky_module]

Ok, so far that looks fine. But kmemleak also provides information about the 
same leak:

 unreferenced object 0xa8570005d000 (size 64):
   comm "insmod", pid 114, jiffies 4294673713 (age 208.968s)
   hex dump (first 32 bytes):
 e6 7e 00 00 00 00 00 00 0a 00 00 00 16 00 00 00  .~..
 21 52 00 00 00 00 00 00 f4 7e 00 00 00 00 00 00  !R...~..
   backtrace:
 [] kmemleak_alloc+0x4a/0xa0
 [] __vmalloc_node_range+0x1e4/0x300
 [] vmalloc+0x54/0x60
 [] leaky_function+0x2f/0x75 [leaky_module]
 [] 0xc038e00b
 [] do_one_initcall+0x53/0x1a0
 [] do_init_module+0x5f/0x1ff
 [] load_module+0x273f/0x2b00
 [] SYSC_init_module+0x166/0x180
 [] SyS_init_module+0xe/0x10
 [] entry_SYSCALL_64_fastpath+0x1a/0xa9
 [] 0x

(3) Load test_module, which happens to load where leaky_module used to reside 
in memory:
0xa8570005d000-0xa8570005f0008192 test_module_exit+0x2f/0x1000 
[test_module] pages=1 vmalloc N0=1
 ^^^ incorrect caller, because 
test_module loaded where old caller used to be

(4) Unload test_module and your patch prints:
[  459.140089] Module [test_module] is getting unloaded before doing vfree
[  459.140551] Memory still allocated: addr:0xa8570005d000 - 
0xa8570005f000, pages 1
[  459.141127] Allocating function test_module_exit+0x2f/0x1000 [test_module] 
<- incorrect caller

So unfortunately this patch also runs into the same problem, reporting
the incorrect caller, and I'm not really convinced that this patch
adds new information that isn't already available with kmemleak and
/proc/vmallocinfo.

Jessica


>  static void free_module(struct module *mod)
>  {
> +  check_memory_leak(mod);
> +



Of course, vfree() has not been called yet. It is the beginning of
free_module(). vfree(

Re: [PATCH 6/8] bio-integrity: add bio_integrity_setup helper

Hi Dmitry,

[auto build test ERROR on linus/master]
[also build test ERROR on v4.11-rc4]
[cannot apply to block/for-next next-20170331]
[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/Dmitry-Monakhov/block-T10-DIF-Fixes-and-cleanups/20170401-043532
config: sparc64-defconfig (attached as .config)
compiler: sparc64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget 
https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=sparc64 

All errors (new ones prefixed by >>):

   In file included from include/linux/blkdev.h:20:0,
from include/linux/backing-dev.h:14,
from include/linux/nfs_fs_sb.h:5,
from include/linux/nfs_fs.h:37,
from arch/sparc/kernel/sys_sparc32.c:24:
>> include/linux/bio.h:788:12: error: 'bio_integrity_setup' defined but not 
>> used [-Werror=unused-function]
static int bio_integrity_setup(struct bio *bio)
   ^~~
   cc1: all warnings being treated as errors

vim +/bio_integrity_setup +788 include/linux/bio.h

   782  
   783  static inline int bio_integrity_prep(struct bio *bio)
   784  {
   785  return 0;
   786  }
   787  
 > 788  static int bio_integrity_setup(struct bio *bio)
   789  {
   790  return 0;
   791  }

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


.config.gz
Description: application/gzip

Re: [PATCH v2] tracing/kprobes: expose maxactive for kretprobe in kprobe_events

2017-03-31 Thread Masami Hiramatsu

On Fri, 31 Mar 2017 10:08:39 -0400
Steven Rostedt  wrote:

> On Fri, 31 Mar 2017 15:20:24 +0200
> Alban Crequy  wrote:
> 
> > When a kretprobe is installed on a kernel function, there is a maximum
> > limit of how many calls in parallel it can catch (aka "maxactive"). A
> > kernel module could call register_kretprobe() and initialize maxactive
> > (see example in samples/kprobes/kretprobe_example.c).
> > 
> > But that is not exposed to userspace and it is currently not possible to
> > choose maxactive when writing to /sys/kernel/debug/tracing/kprobe_events
> > 
> > The default maxactive can be as low as 1 on single-core with a
> > non-preemptive kernel. This is too low and we need to increase it not
> > only for recursive functions, but for functions that sleep or resched.
> > 
> > This patch updates the format of the command that can be written to
> > kprobe_events so that maxactive can be optionally specified.
> > 
> > I need this for a bpf program attached to the kretprobe of
> > inet_csk_accept, which can sleep for a long time.
> > 
> > This patch includes a basic selftest:
> > 
> > > # ./ftracetest -v  test.d/kprobe/
> > > === Ftrace unit tests ===
> > > [1] Kprobe dynamic event - adding and removing[PASS]
> > > [2] Kprobe dynamic event - busy event check   [PASS]
> > > [3] Kprobe dynamic event with arguments   [PASS]
> > > [4] Kprobes event arguments with types[PASS]
> > > [5] Kprobe dynamic event with function tracer [PASS]
> > > [6] Kretprobe dynamic event with arguments[PASS]
> > > [7] Kretprobe dynamic event with maxactive[PASS]
> > >
> > > # of passed:  7
> > > # of failed:  0
> > > # of unresolved:  0
> > > # of untested:  0
> > > # of unsupported:  0
> > > # of xfailed:  0
> > > # of undefined(test bug):  0  
> > 
> > BugLink: https://github.com/iovisor/bcc/issues/1072
> > Signed-off-by: Alban Crequy 
> > 
> > ---
> > 
> > Changes since v1:
> > - Remove "(*)" from documentation. (Review from Masami Hiramatsu)
> > - Fix support for "r100" without the event name (Review from Masami 
> > Hiramatsu)
> > - Get rid of magic numbers within the code.  (Review from Steven Rostedt)
> >   Note that I didn't use KRETPROBE_MAXACTIVE_ALLOC since that patch is not
> >   merged.
> > - Return -E2BIG when maxactive is too big.
> > - Add basic selftest
> > ---
> >  Documentation/trace/kprobetrace.txt|  4 ++-
> >  kernel/trace/trace_kprobe.c| 39 
> > ++
> >  .../ftrace/test.d/kprobe/kretprobe_maxactive.tc| 39 
> > ++
> >  3 files changed, 75 insertions(+), 7 deletions(-)
> >  create mode 100644 
> > tools/testing/selftests/ftrace/test.d/kprobe/kretprobe_maxactive.tc
> > 
> > diff --git a/Documentation/trace/kprobetrace.txt 
> > b/Documentation/trace/kprobetrace.txt
> > index 41ef9d8..7051a20 100644
> > --- a/Documentation/trace/kprobetrace.txt
> > +++ b/Documentation/trace/kprobetrace.txt
> > @@ -23,7 +23,7 @@ current_tracer. Instead of that, add probe points via
> >  Synopsis of kprobe_events
> >  -
> >p[:[GRP/]EVENT] [MOD:]SYM[+offs]|MEMADDR [FETCHARGS] : Set a probe
> > -  r[:[GRP/]EVENT] [MOD:]SYM[+0] [FETCHARGS]: Set a return 
> > probe
> > +  r[MAXACTIVE][:[GRP/]EVENT] [MOD:]SYM[+0] [FETCHARGS] : Set a return 
> > probe
> >-:[GRP/]EVENT: Clear a probe
> >  
> >   GRP   : Group name. If omitted, use "kprobes" for it.
> > @@ -32,6 +32,8 @@ Synopsis of kprobe_events
> >   MOD   : Module name which has given SYM.
> >   SYM[+offs]: Symbol+offset where the probe is inserted.
> >   MEMADDR   : Address where the probe is inserted.
> > + MAXACTIVE : Maximum number of instances of the specified function that
> > + can be probed simultaneously, or 0 for the default.
> 
> BTW, to me, 0 means none (no instances can probe). This should have a
> better description of what "0" actually means.

default value is defined in Documentation/kprobes.txt sction 1.3.1, so
you'll just need to refer that.

Thank you,

> 
> -- Steve
> 
> 
> >  
> >   FETCHARGS : Arguments. Each probe can have up to 128 args.
> >%REG : Fetch register REG


-- 
Masami Hiramatsu

Re: [PATCH 7/8] T10: Move opencoded contants to common header