Re: [PATCH 1/6] powernv:idle: Correctly initialize core_idle_state_ptr

2017-05-29 Thread Nicholas Piggin
On Tue, 16 May 2017 14:19:43 +0530
"Gautham R. Shenoy"  wrote:

> From: "Gautham R. Shenoy" 
> 
> The lower 8 bits of core_idle_state_ptr tracks the number of non-idle
> threads in the core. This is supposed to be initialized to bit-map
> corresponding to the threads_per_core. However, currently it is
> initialized to PNV_CORE_IDLE_THREAD_BITS (0xFF). This is correct for
> POWER8 which has 8 threads per core, but not for POWER9 which has 4
> threads per core.
> 
> As a result, on POWER9, core_idle_state_ptr gets initialized to
> 0xFF. In case when all the threads of the core are idle, the bits
> corresponding tracking the idle-threads are non-zero. As a result, the
> idle entry/exit code fails to save/restore per-core hypervisor state
> since it assumes that there are threads in the cores which are still
> active.
> 
> Fix this by correctly initializing the lower bits of the
> core_idle_state_ptr on the basis of threads_per_core.
> 
> Signed-off-by: Gautham R. Shenoy 

This looks good to me.

Until this patch series, we can't enable HV state loss idle modes
on POWER9, is that correct? And after your series does it work?

Reviewed-by: Nicholas Piggin 


Re: [PATCH 1/6] powernv:idle: Correctly initialize core_idle_state_ptr

2017-05-29 Thread Nicholas Piggin
On Tue, 16 May 2017 14:19:43 +0530
"Gautham R. Shenoy"  wrote:

> From: "Gautham R. Shenoy" 
> 
> The lower 8 bits of core_idle_state_ptr tracks the number of non-idle
> threads in the core. This is supposed to be initialized to bit-map
> corresponding to the threads_per_core. However, currently it is
> initialized to PNV_CORE_IDLE_THREAD_BITS (0xFF). This is correct for
> POWER8 which has 8 threads per core, but not for POWER9 which has 4
> threads per core.
> 
> As a result, on POWER9, core_idle_state_ptr gets initialized to
> 0xFF. In case when all the threads of the core are idle, the bits
> corresponding tracking the idle-threads are non-zero. As a result, the
> idle entry/exit code fails to save/restore per-core hypervisor state
> since it assumes that there are threads in the cores which are still
> active.
> 
> Fix this by correctly initializing the lower bits of the
> core_idle_state_ptr on the basis of threads_per_core.
> 
> Signed-off-by: Gautham R. Shenoy 

This looks good to me.

Until this patch series, we can't enable HV state loss idle modes
on POWER9, is that correct? And after your series does it work?

Reviewed-by: Nicholas Piggin 


Re: [PATCH 2/2] xen/input: add multi-touch support

2017-05-29 Thread Dmitry Torokhov
On Fri, Apr 21, 2017 at 09:40:36AM +0300, Oleksandr Andrushchenko wrote:
> Hi, Dmitry!
> 
> On 04/21/2017 05:10 AM, Dmitry Torokhov wrote:
> >Hi Oleksandr,
> >
> >On Thu, Apr 13, 2017 at 02:38:04PM +0300, Oleksandr Andrushchenko wrote:
> >>From: Oleksandr Andrushchenko 
> >>
> >>Extend xen_kbdfront to provide multi-touch support
> >>to unprivileged domains.
> >>
> >>Signed-off-by: Oleksandr Andrushchenko 
> >>---
> >>  drivers/input/misc/xen-kbdfront.c | 142 
> >> +-
> >>  1 file changed, 140 insertions(+), 2 deletions(-)
> >>
> >>diff --git a/drivers/input/misc/xen-kbdfront.c 
> >>b/drivers/input/misc/xen-kbdfront.c
> >>index 01c27b4c3288..e5d064aaa237 100644
> >>--- a/drivers/input/misc/xen-kbdfront.c
> >>+++ b/drivers/input/misc/xen-kbdfront.c
> >>@@ -17,6 +17,7 @@
> >>  #include 
> >>  #include 
> >>  #include 
> >>+#include 
> >>  #include 
> >>  #include 
> >>@@ -34,11 +35,14 @@
> >>  struct xenkbd_info {
> >>struct input_dev *kbd;
> >>struct input_dev *ptr;
> >>+   struct input_dev *mtouch;
> >>struct xenkbd_page *page;
> >>int gref;
> >>int irq;
> >>struct xenbus_device *xbdev;
> >>char phys[32];
> >>+   /* current MT slot/contact ID we are injecting events in */
> >>+   int mtouch_cur_contact_id;
> >>  };
> >>  enum { KPARAM_X, KPARAM_Y, KPARAM_CNT };
> >>@@ -47,6 +51,12 @@ module_param_array(ptr_size, int, NULL, 0444);
> >>  MODULE_PARM_DESC(ptr_size,
> >>"Pointing device width, height in pixels (default 800,600)");
> >>+enum { KPARAM_MT_X, KPARAM_MT_Y, KPARAM_MT_CNT };
> >>+static int mtouch_size[KPARAM_MT_CNT] = { XENFB_WIDTH, XENFB_HEIGHT };
> >>+module_param_array(mtouch_size, int, NULL, 0444);
> >>+MODULE_PARM_DESC(ptr_size,
> >>+   "Multi-touch device width, height in pixels (default 800,600)");
> >>+
> >Why do you need separate module parameters for multi-touch device?
> please see below
> >
> >>  static int xenkbd_remove(struct xenbus_device *);
> >>  static int xenkbd_connect_backend(struct xenbus_device *, struct 
> >> xenkbd_info *);
> >>  static void xenkbd_disconnect_backend(struct xenkbd_info *);
> >>@@ -100,6 +110,60 @@ static irqreturn_t input_handler(int rq, void *dev_id)
> >>input_report_rel(dev, REL_WHEEL,
> >> -event->pos.rel_z);
> >>break;
> >>+   case XENKBD_TYPE_MTOUCH:
> >>+   dev = info->mtouch;
> >>+   if (unlikely(!dev))
> >>+   break;
> >>+   if (unlikely(event->mtouch.contact_id !=
> >>+   info->mtouch_cur_contact_id)) {
> >Why is this unlikely? Does contact ID changes once in 1000 packets or
> >even less?
> Mu assumption was that regardless of the fact that we are multi-touch
> device still single touches will come in more frequently
> But I can remove *unlikely* if my assumption is not correct

I think the normal expectation is that "unlikely" is supposed for
something that happens once in a blue moon, so I'd rather remove it.

> >>+   info->mtouch_cur_contact_id =
> >>+   event->mtouch.contact_id;
> >>+   input_mt_slot(dev, event->mtouch.contact_id);
> >>+   }
> >>+   switch (event->mtouch.event_type) {
> >>+   case XENKBD_MT_EV_DOWN:
> >>+   input_mt_report_slot_state(dev, MT_TOOL_FINGER,
> >>+  true);

Should we establish tool event? We have MT_TOOL_PEN, etc.

> >>+   input_event(dev, EV_ABS, ABS_MT_POSITION_X,
> >>+   event->mtouch.u.pos.abs_x);
> >>+   input_event(dev, EV_ABS, ABS_MT_POSITION_Y,
> >>+   event->mtouch.u.pos.abs_y);
> >>+   input_event(dev, EV_ABS, ABS_X,
> >>+   event->mtouch.u.pos.abs_x);
> >>+   input_event(dev, EV_ABS, ABS_Y,
> >>+   event->mtouch.u.pos.abs_y);
> >>+   break;
> >>+   case XENKBD_MT_EV_UP:
> >>+   input_mt_report_slot_state(dev, MT_TOOL_FINGER,
> >>+  false);
> >>+   break;
> >>+   case XENKBD_MT_EV_MOTION:
> >>+   input_event(dev, EV_ABS, ABS_MT_POSITION_X,
> >>+   event->mtouch.u.pos.abs_x);
> >>+   input_event(dev, EV_ABS, ABS_MT_POSITION_Y,
> >>+   event->mtouch.u.pos.abs_y);
> >>+   input_event(dev, EV_ABS, ABS_X,
> >>+   

Re: [PATCH 2/2] xen/input: add multi-touch support

2017-05-29 Thread Dmitry Torokhov
On Fri, Apr 21, 2017 at 09:40:36AM +0300, Oleksandr Andrushchenko wrote:
> Hi, Dmitry!
> 
> On 04/21/2017 05:10 AM, Dmitry Torokhov wrote:
> >Hi Oleksandr,
> >
> >On Thu, Apr 13, 2017 at 02:38:04PM +0300, Oleksandr Andrushchenko wrote:
> >>From: Oleksandr Andrushchenko 
> >>
> >>Extend xen_kbdfront to provide multi-touch support
> >>to unprivileged domains.
> >>
> >>Signed-off-by: Oleksandr Andrushchenko 
> >>---
> >>  drivers/input/misc/xen-kbdfront.c | 142 
> >> +-
> >>  1 file changed, 140 insertions(+), 2 deletions(-)
> >>
> >>diff --git a/drivers/input/misc/xen-kbdfront.c 
> >>b/drivers/input/misc/xen-kbdfront.c
> >>index 01c27b4c3288..e5d064aaa237 100644
> >>--- a/drivers/input/misc/xen-kbdfront.c
> >>+++ b/drivers/input/misc/xen-kbdfront.c
> >>@@ -17,6 +17,7 @@
> >>  #include 
> >>  #include 
> >>  #include 
> >>+#include 
> >>  #include 
> >>  #include 
> >>@@ -34,11 +35,14 @@
> >>  struct xenkbd_info {
> >>struct input_dev *kbd;
> >>struct input_dev *ptr;
> >>+   struct input_dev *mtouch;
> >>struct xenkbd_page *page;
> >>int gref;
> >>int irq;
> >>struct xenbus_device *xbdev;
> >>char phys[32];
> >>+   /* current MT slot/contact ID we are injecting events in */
> >>+   int mtouch_cur_contact_id;
> >>  };
> >>  enum { KPARAM_X, KPARAM_Y, KPARAM_CNT };
> >>@@ -47,6 +51,12 @@ module_param_array(ptr_size, int, NULL, 0444);
> >>  MODULE_PARM_DESC(ptr_size,
> >>"Pointing device width, height in pixels (default 800,600)");
> >>+enum { KPARAM_MT_X, KPARAM_MT_Y, KPARAM_MT_CNT };
> >>+static int mtouch_size[KPARAM_MT_CNT] = { XENFB_WIDTH, XENFB_HEIGHT };
> >>+module_param_array(mtouch_size, int, NULL, 0444);
> >>+MODULE_PARM_DESC(ptr_size,
> >>+   "Multi-touch device width, height in pixels (default 800,600)");
> >>+
> >Why do you need separate module parameters for multi-touch device?
> please see below
> >
> >>  static int xenkbd_remove(struct xenbus_device *);
> >>  static int xenkbd_connect_backend(struct xenbus_device *, struct 
> >> xenkbd_info *);
> >>  static void xenkbd_disconnect_backend(struct xenkbd_info *);
> >>@@ -100,6 +110,60 @@ static irqreturn_t input_handler(int rq, void *dev_id)
> >>input_report_rel(dev, REL_WHEEL,
> >> -event->pos.rel_z);
> >>break;
> >>+   case XENKBD_TYPE_MTOUCH:
> >>+   dev = info->mtouch;
> >>+   if (unlikely(!dev))
> >>+   break;
> >>+   if (unlikely(event->mtouch.contact_id !=
> >>+   info->mtouch_cur_contact_id)) {
> >Why is this unlikely? Does contact ID changes once in 1000 packets or
> >even less?
> Mu assumption was that regardless of the fact that we are multi-touch
> device still single touches will come in more frequently
> But I can remove *unlikely* if my assumption is not correct

I think the normal expectation is that "unlikely" is supposed for
something that happens once in a blue moon, so I'd rather remove it.

> >>+   info->mtouch_cur_contact_id =
> >>+   event->mtouch.contact_id;
> >>+   input_mt_slot(dev, event->mtouch.contact_id);
> >>+   }
> >>+   switch (event->mtouch.event_type) {
> >>+   case XENKBD_MT_EV_DOWN:
> >>+   input_mt_report_slot_state(dev, MT_TOOL_FINGER,
> >>+  true);

Should we establish tool event? We have MT_TOOL_PEN, etc.

> >>+   input_event(dev, EV_ABS, ABS_MT_POSITION_X,
> >>+   event->mtouch.u.pos.abs_x);
> >>+   input_event(dev, EV_ABS, ABS_MT_POSITION_Y,
> >>+   event->mtouch.u.pos.abs_y);
> >>+   input_event(dev, EV_ABS, ABS_X,
> >>+   event->mtouch.u.pos.abs_x);
> >>+   input_event(dev, EV_ABS, ABS_Y,
> >>+   event->mtouch.u.pos.abs_y);
> >>+   break;
> >>+   case XENKBD_MT_EV_UP:
> >>+   input_mt_report_slot_state(dev, MT_TOOL_FINGER,
> >>+  false);
> >>+   break;
> >>+   case XENKBD_MT_EV_MOTION:
> >>+   input_event(dev, EV_ABS, ABS_MT_POSITION_X,
> >>+   event->mtouch.u.pos.abs_x);
> >>+   input_event(dev, EV_ABS, ABS_MT_POSITION_Y,
> >>+   event->mtouch.u.pos.abs_y);
> >>+   input_event(dev, EV_ABS, ABS_X,
> >>+   event->mtouch.u.pos.abs_x);
> >>+   

Re: [PATCH 0/2] nohz: Deal with clock reprogram skipping issues v3

2017-05-29 Thread Ingo Molnar

* Frederic Weisbecker  wrote:

> > Well, this does not answer my question: between latest tip:timers/nohz and 
> > the 
> > patches you posted there's a delta, so it's not just a pure rebase.
> 
> Yeah but like I said, you can forget the series I posted because the diff is
> mostly cosmetic and things are actually ok as they are in tip:timers/nohz
> 
> The only thing that bothers me is the fact that the HEAD of this branch 
> doesn't have
> a changelog or even just a comment.

We can still amend that - is this changelog what you had in mind:

  nohz: Reset next_tick cache even when the timer has no regs

  Handle tick interrupts whose regs are NULL, out of general paranoia. It 
happens 
  when hrtimer_interrupt() is called from non-interrupt contexts, such as 
hotplug
  CPU down events.

?

Or you can send me a longer version as well.

Thanks,

Ingo


Re: [PATCH 0/2] nohz: Deal with clock reprogram skipping issues v3

2017-05-29 Thread Ingo Molnar

* Frederic Weisbecker  wrote:

> > Well, this does not answer my question: between latest tip:timers/nohz and 
> > the 
> > patches you posted there's a delta, so it's not just a pure rebase.
> 
> Yeah but like I said, you can forget the series I posted because the diff is
> mostly cosmetic and things are actually ok as they are in tip:timers/nohz
> 
> The only thing that bothers me is the fact that the HEAD of this branch 
> doesn't have
> a changelog or even just a comment.

We can still amend that - is this changelog what you had in mind:

  nohz: Reset next_tick cache even when the timer has no regs

  Handle tick interrupts whose regs are NULL, out of general paranoia. It 
happens 
  when hrtimer_interrupt() is called from non-interrupt contexts, such as 
hotplug
  CPU down events.

?

Or you can send me a longer version as well.

Thanks,

Ingo


[PATCH v4] Input: mousedev - fix implicit conversion warning

2017-05-29 Thread Nick Desaulniers
Clang warns:

drivers/input/mousedev.c:653:63: error: implicit conversion from 'int'
to 'signed char' changes value from 200 to -56
[-Wconstant-conversion]
  client->ps2[1] = 0x60; client->ps2[2] = 3; client->ps2[3] = 200;
~ ^~~

As far as I can tell, from

http://www.computer-engineering.org/ps2mouse/

Under "Command Set" > "0xE9 (Status Request)"

the value 200 is a valid sample rate. Using unsigned char [], rather than
signed char [], for client->ps2 silences this warning.

Also moves some reused logic into a helper function.

Signed-off-by: Nick Desaulniers 
---
What's new in v4:
* replace mousedev_limit_delta() with update_clamped() that also updates
  the ps2_data and delta values. The use of the temporary val should
  avoid integral conversion and promotion issues.

 drivers/input/mousedev.c | 23 +++
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/drivers/input/mousedev.c b/drivers/input/mousedev.c
index 0e0ff84088fd..e645b8c6f2cb 100644
--- a/drivers/input/mousedev.c
+++ b/drivers/input/mousedev.c
@@ -103,7 +103,7 @@ struct mousedev_client {
spinlock_t packet_lock;
int pos_x, pos_y;
 
-   signed char ps2[6];
+   unsigned char ps2[6];
unsigned char ready, buffer, bufsiz;
unsigned char imexseq, impsseq;
enum mousedev_emul mode;
@@ -571,27 +571,27 @@ static int mousedev_open(struct inode *inode, struct file 
*file)
return error;
 }
 
-static inline int mousedev_limit_delta(int delta, int limit)
+static inline void
+update_clamped(unsigned char *ps2_data, int *delta, int limit)
 {
-   return delta > limit ? limit : (delta < -limit ? -limit : delta);
+   int val = *delta > limit ? limit : (*delta < -limit ? -limit : *delta);
+   *ps2_data = (unsigned char) val;
+   *delta -= val;
 }
 
 static void mousedev_packet(struct mousedev_client *client,
-   signed char *ps2_data)
+   unsigned char *ps2_data)
 {
struct mousedev_motion *p = >packets[client->tail];
 
ps2_data[0] = 0x08 |
((p->dx < 0) << 4) | ((p->dy < 0) << 5) | (p->buttons & 0x07);
-   ps2_data[1] = mousedev_limit_delta(p->dx, 127);
-   ps2_data[2] = mousedev_limit_delta(p->dy, 127);
-   p->dx -= ps2_data[1];
-   p->dy -= ps2_data[2];
+   update_clamped(_data[1], >dx, 127);
+   update_clamped(_data[2], >dy, 127);
 
switch (client->mode) {
case MOUSEDEV_EMUL_EXPS:
-   ps2_data[3] = mousedev_limit_delta(p->dz, 7);
-   p->dz -= ps2_data[3];
+   update_clamped(_data[3], >dz, 7);
ps2_data[3] = (ps2_data[3] & 0x0f) | ((p->buttons & 0x18) << 1);
client->bufsiz = 4;
break;
@@ -599,8 +599,7 @@ static void mousedev_packet(struct mousedev_client *client,
case MOUSEDEV_EMUL_IMPS:
ps2_data[0] |=
((p->buttons & 0x10) >> 3) | ((p->buttons & 0x08) >> 1);
-   ps2_data[3] = mousedev_limit_delta(p->dz, 127);
-   p->dz -= ps2_data[3];
+   update_clamped(_data[3], >dz, 127);
client->bufsiz = 4;
break;
 
-- 
2.11.0



[PATCH v4] Input: mousedev - fix implicit conversion warning

2017-05-29 Thread Nick Desaulniers
Clang warns:

drivers/input/mousedev.c:653:63: error: implicit conversion from 'int'
to 'signed char' changes value from 200 to -56
[-Wconstant-conversion]
  client->ps2[1] = 0x60; client->ps2[2] = 3; client->ps2[3] = 200;
~ ^~~

As far as I can tell, from

http://www.computer-engineering.org/ps2mouse/

Under "Command Set" > "0xE9 (Status Request)"

the value 200 is a valid sample rate. Using unsigned char [], rather than
signed char [], for client->ps2 silences this warning.

Also moves some reused logic into a helper function.

Signed-off-by: Nick Desaulniers 
---
What's new in v4:
* replace mousedev_limit_delta() with update_clamped() that also updates
  the ps2_data and delta values. The use of the temporary val should
  avoid integral conversion and promotion issues.

 drivers/input/mousedev.c | 23 +++
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/drivers/input/mousedev.c b/drivers/input/mousedev.c
index 0e0ff84088fd..e645b8c6f2cb 100644
--- a/drivers/input/mousedev.c
+++ b/drivers/input/mousedev.c
@@ -103,7 +103,7 @@ struct mousedev_client {
spinlock_t packet_lock;
int pos_x, pos_y;
 
-   signed char ps2[6];
+   unsigned char ps2[6];
unsigned char ready, buffer, bufsiz;
unsigned char imexseq, impsseq;
enum mousedev_emul mode;
@@ -571,27 +571,27 @@ static int mousedev_open(struct inode *inode, struct file 
*file)
return error;
 }
 
-static inline int mousedev_limit_delta(int delta, int limit)
+static inline void
+update_clamped(unsigned char *ps2_data, int *delta, int limit)
 {
-   return delta > limit ? limit : (delta < -limit ? -limit : delta);
+   int val = *delta > limit ? limit : (*delta < -limit ? -limit : *delta);
+   *ps2_data = (unsigned char) val;
+   *delta -= val;
 }
 
 static void mousedev_packet(struct mousedev_client *client,
-   signed char *ps2_data)
+   unsigned char *ps2_data)
 {
struct mousedev_motion *p = >packets[client->tail];
 
ps2_data[0] = 0x08 |
((p->dx < 0) << 4) | ((p->dy < 0) << 5) | (p->buttons & 0x07);
-   ps2_data[1] = mousedev_limit_delta(p->dx, 127);
-   ps2_data[2] = mousedev_limit_delta(p->dy, 127);
-   p->dx -= ps2_data[1];
-   p->dy -= ps2_data[2];
+   update_clamped(_data[1], >dx, 127);
+   update_clamped(_data[2], >dy, 127);
 
switch (client->mode) {
case MOUSEDEV_EMUL_EXPS:
-   ps2_data[3] = mousedev_limit_delta(p->dz, 7);
-   p->dz -= ps2_data[3];
+   update_clamped(_data[3], >dz, 7);
ps2_data[3] = (ps2_data[3] & 0x0f) | ((p->buttons & 0x18) << 1);
client->bufsiz = 4;
break;
@@ -599,8 +599,7 @@ static void mousedev_packet(struct mousedev_client *client,
case MOUSEDEV_EMUL_IMPS:
ps2_data[0] |=
((p->buttons & 0x10) >> 3) | ((p->buttons & 0x08) >> 1);
-   ps2_data[3] = mousedev_limit_delta(p->dz, 127);
-   p->dz -= ps2_data[3];
+   update_clamped(_data[3], >dz, 127);
client->bufsiz = 4;
break;
 
-- 
2.11.0



Re: [RESEND PATCH] irq_work: Don't reinvent the wheel but use existing llist API

2017-05-29 Thread Byungchul Park
On Fri, May 12, 2017 at 09:45:35AM +0900, Byungchul Park wrote:
> Although llist provides proper APIs, they are not used. Make them used.

+to pet...@infradead.org
+to mi...@kernel.org
+to rost...@goodmis.org

I am not sure whom should I send this patch to..
Could you check this if you are right person?

> 
> Signed-off-by: Byungchul Park 
> ---
>  kernel/irq_work.c | 6 +-
>  1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/kernel/irq_work.c b/kernel/irq_work.c
> index bcf107c..e2ebe8c 100644
> --- a/kernel/irq_work.c
> +++ b/kernel/irq_work.c
> @@ -138,11 +138,7 @@ static void irq_work_run_list(struct llist_head *list)
>   return;
>  
>   llnode = llist_del_all(list);
> - while (llnode != NULL) {
> - work = llist_entry(llnode, struct irq_work, llnode);
> -
> - llnode = llist_next(llnode);
> -
> + llist_for_each_entry(work, llnode, llnode) {
>   /*
>* Clear the PENDING bit, after this point the @work
>* can be re-used.
> -- 
> 1.9.1


Re: [RESEND PATCH] irq_work: Don't reinvent the wheel but use existing llist API

2017-05-29 Thread Byungchul Park
On Fri, May 12, 2017 at 09:45:35AM +0900, Byungchul Park wrote:
> Although llist provides proper APIs, they are not used. Make them used.

+to pet...@infradead.org
+to mi...@kernel.org
+to rost...@goodmis.org

I am not sure whom should I send this patch to..
Could you check this if you are right person?

> 
> Signed-off-by: Byungchul Park 
> ---
>  kernel/irq_work.c | 6 +-
>  1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/kernel/irq_work.c b/kernel/irq_work.c
> index bcf107c..e2ebe8c 100644
> --- a/kernel/irq_work.c
> +++ b/kernel/irq_work.c
> @@ -138,11 +138,7 @@ static void irq_work_run_list(struct llist_head *list)
>   return;
>  
>   llnode = llist_del_all(list);
> - while (llnode != NULL) {
> - work = llist_entry(llnode, struct irq_work, llnode);
> -
> - llnode = llist_next(llnode);
> -
> + llist_for_each_entry(work, llnode, llnode) {
>   /*
>* Clear the PENDING bit, after this point the @work
>* can be re-used.
> -- 
> 1.9.1


linux-next: Tree for May 30

2017-05-29 Thread Stephen Rothwell
Hi all,

Changes since 20170529:

New trees: reset, reset-fixes

The nand tree gained a conflict against the jc_docs tree.

Non-merge commits (relative to Linus' tree): 3099
 3410 files changed, 126865 insertions(+), 68052 deletions(-)



I have created today's linux-next tree at
git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git
(patches at http://www.kernel.org/pub/linux/kernel/next/ ).  If you
are tracking the linux-next tree using git, you should not use "git pull"
to do so as that will try to merge the new linux-next release with the
old one.  You should use "git fetch" and checkout or reset to the new
master.

You can see which trees have been included by looking in the Next/Trees
file in the source.  There are also quilt-import.log and merge.log
files in the Next directory.  Between each merge, the tree was built
with a ppc64_defconfig for powerpc and an allmodconfig (with
CONFIG_BUILD_DOCSRC=n) for x86_64, a multi_v7_defconfig for arm and a
native build of tools/perf. After the final fixups (if any), I do an
x86_64 modules_install followed by builds for x86_64 allnoconfig,
powerpc allnoconfig (32 and 64 bit), ppc44x_defconfig, allyesconfig
and pseries_le_defconfig and i386, sparc and sparc64 defconfig.

Below is a summary of the state of the merge.

I am currently merging 263 trees (counting Linus' and 40 trees of bug
fix patches pending for the current merge release).

Stats about the size of the tree over time can be seen at
http://neuling.org/linux-next-size.html .

Status of my local build tests will be at
http://kisskb.ellerman.id.au/linux-next .  If maintainers want to give
advice about cross compilers/configs that work, we are always open to add
more builds.

Thanks to Randy Dunlap for doing many randconfig builds.  And to Paul
Gortmaker for triage and bug fixes.

-- 
Cheers,
Stephen Rothwell

$ git checkout master
$ git reset --hard stable
Merging origin/master (3f173bde7e43 Merge tag 'pinctrl-v4.12-2' of 
git://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl)
Merging fixes/master (97da3854c526 Linux 4.11-rc3)
Merging kbuild-current/fixes (05d8cba4a1e8 kbuild: skip install/check of 
headers right under uapi directories)
Merging arc-current/for-curr (a4da5b17736d arc: Set IO-coherency aperture base 
to LINUX_LINK_BASE)
Merging arm-current/fixes (2ea659a9ef48 Linux 4.12-rc1)
Merging m68k-current/for-linus (f6ab4d59a5fe nubus: Add MVC and VSC video card 
definitions)
Merging metag-fixes/fixes (b884a190afce metag/usercopy: Add missing fixups)
Merging powerpc-fixes/fixes (a4700a261072 powerpc: Add PPC_FEATURE userspace 
bits for SCV and DARN instructions)
Merging sparc/master (7485af89a6fd arch/sparc: increase CONFIG_NODES_SHIFT on 
SPARC64 to 5)
Merging fscrypt-current/for-stable (42d97eb0ade3 fscrypt: fix renaming and 
linking special files)
Merging net/master (c21fbe29f858 net: dsa: mv88e6xxx: Add missing static to 
stub functions)
Merging ipsec/master (a486cd23661c xfrm: fix state migration copy replay 
sequence numbers)
Merging netfilter/master (fefa92679dbe netfilter: ctnetlink: fix incorrect 
nf_ct_put during hash resize)
Merging ipvs/master (3c5ab3f395d6 ipvs: SNAT packet replies only for NATed 
connections)
Merging wireless-drivers/master (6d18c732b95c bridge: start hello_timer when 
enabling KERNEL_STP in br_stp_start)
Merging mac80211/master (029c58178b9a Merge tag 'mac80211-for-davem-2017-05-23' 
of git://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211)
Merging sound-current/for-linus (1fc2e41f7af4 ALSA: hda - apply 
STAC_9200_DELL_M22 quirk for Dell Latitude D430)
Merging pci-current/for-linus (bd2df9b1e094 PCI: Make error code types 
consistent in pci_{read,write}_config_*)
Merging driver-core.current/driver-core-linus (08332893e37a Linux 4.12-rc2)
Merging tty.current/tty-linus (5ed02dbb4974 Linux 4.12-rc3)
Merging usb.current/usb-linus (b3addcf0d1f0 usb: musb: dsps: keep VBUS on for 
host-only mode)
Merging usb-gadget-fixes/fixes (a351e9b9fc24 Linux 4.11)
Merging usb-serial-fixes/usb-linus (5ed02dbb4974 Linux 4.12-rc3)
Merging usb-chipidea-fixes/ci-for-usb-stable (cbb22ebcfb99 usb: chipidea: core: 
check before accessing ci_role in ci_role_show)
Merging phy/fixes (a380b78b799b phy: qualcomm: phy-qcom-qmp: fix application of 
sizeof to pointer)
Merging staging.current/staging-linus (3d51b9562673 staging: ccree: add CRYPTO 
dependency)
Merging char-misc.current/char-misc-linus (cdc1daca1b9b MAINTAINERS: Change 
maintainer of genwqe driver)
Merging input-current/for-linus (a04f144059ac Input: elan_i2c - ignore signals 
when finishing updating firmware)
Merging crypto-current/master (f3ad587070d6 crypto: gcm - wait for crypto op 
not signal safe)
Merging ide/master (acfead32f3f9 ide: don't call memcpy with the same source 
and destination)
Merging vfio-fixes/for-linus (39da7c509acf Linux 4.11-rc6)
Merging kselftest-fixes/fixes (2ea659a9ef48 Linux 4.12-rc

linux-next: Tree for May 30

2017-05-29 Thread Stephen Rothwell
Hi all,

Changes since 20170529:

New trees: reset, reset-fixes

The nand tree gained a conflict against the jc_docs tree.

Non-merge commits (relative to Linus' tree): 3099
 3410 files changed, 126865 insertions(+), 68052 deletions(-)



I have created today's linux-next tree at
git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git
(patches at http://www.kernel.org/pub/linux/kernel/next/ ).  If you
are tracking the linux-next tree using git, you should not use "git pull"
to do so as that will try to merge the new linux-next release with the
old one.  You should use "git fetch" and checkout or reset to the new
master.

You can see which trees have been included by looking in the Next/Trees
file in the source.  There are also quilt-import.log and merge.log
files in the Next directory.  Between each merge, the tree was built
with a ppc64_defconfig for powerpc and an allmodconfig (with
CONFIG_BUILD_DOCSRC=n) for x86_64, a multi_v7_defconfig for arm and a
native build of tools/perf. After the final fixups (if any), I do an
x86_64 modules_install followed by builds for x86_64 allnoconfig,
powerpc allnoconfig (32 and 64 bit), ppc44x_defconfig, allyesconfig
and pseries_le_defconfig and i386, sparc and sparc64 defconfig.

Below is a summary of the state of the merge.

I am currently merging 263 trees (counting Linus' and 40 trees of bug
fix patches pending for the current merge release).

Stats about the size of the tree over time can be seen at
http://neuling.org/linux-next-size.html .

Status of my local build tests will be at
http://kisskb.ellerman.id.au/linux-next .  If maintainers want to give
advice about cross compilers/configs that work, we are always open to add
more builds.

Thanks to Randy Dunlap for doing many randconfig builds.  And to Paul
Gortmaker for triage and bug fixes.

-- 
Cheers,
Stephen Rothwell

$ git checkout master
$ git reset --hard stable
Merging origin/master (3f173bde7e43 Merge tag 'pinctrl-v4.12-2' of 
git://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl)
Merging fixes/master (97da3854c526 Linux 4.11-rc3)
Merging kbuild-current/fixes (05d8cba4a1e8 kbuild: skip install/check of 
headers right under uapi directories)
Merging arc-current/for-curr (a4da5b17736d arc: Set IO-coherency aperture base 
to LINUX_LINK_BASE)
Merging arm-current/fixes (2ea659a9ef48 Linux 4.12-rc1)
Merging m68k-current/for-linus (f6ab4d59a5fe nubus: Add MVC and VSC video card 
definitions)
Merging metag-fixes/fixes (b884a190afce metag/usercopy: Add missing fixups)
Merging powerpc-fixes/fixes (a4700a261072 powerpc: Add PPC_FEATURE userspace 
bits for SCV and DARN instructions)
Merging sparc/master (7485af89a6fd arch/sparc: increase CONFIG_NODES_SHIFT on 
SPARC64 to 5)
Merging fscrypt-current/for-stable (42d97eb0ade3 fscrypt: fix renaming and 
linking special files)
Merging net/master (c21fbe29f858 net: dsa: mv88e6xxx: Add missing static to 
stub functions)
Merging ipsec/master (a486cd23661c xfrm: fix state migration copy replay 
sequence numbers)
Merging netfilter/master (fefa92679dbe netfilter: ctnetlink: fix incorrect 
nf_ct_put during hash resize)
Merging ipvs/master (3c5ab3f395d6 ipvs: SNAT packet replies only for NATed 
connections)
Merging wireless-drivers/master (6d18c732b95c bridge: start hello_timer when 
enabling KERNEL_STP in br_stp_start)
Merging mac80211/master (029c58178b9a Merge tag 'mac80211-for-davem-2017-05-23' 
of git://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211)
Merging sound-current/for-linus (1fc2e41f7af4 ALSA: hda - apply 
STAC_9200_DELL_M22 quirk for Dell Latitude D430)
Merging pci-current/for-linus (bd2df9b1e094 PCI: Make error code types 
consistent in pci_{read,write}_config_*)
Merging driver-core.current/driver-core-linus (08332893e37a Linux 4.12-rc2)
Merging tty.current/tty-linus (5ed02dbb4974 Linux 4.12-rc3)
Merging usb.current/usb-linus (b3addcf0d1f0 usb: musb: dsps: keep VBUS on for 
host-only mode)
Merging usb-gadget-fixes/fixes (a351e9b9fc24 Linux 4.11)
Merging usb-serial-fixes/usb-linus (5ed02dbb4974 Linux 4.12-rc3)
Merging usb-chipidea-fixes/ci-for-usb-stable (cbb22ebcfb99 usb: chipidea: core: 
check before accessing ci_role in ci_role_show)
Merging phy/fixes (a380b78b799b phy: qualcomm: phy-qcom-qmp: fix application of 
sizeof to pointer)
Merging staging.current/staging-linus (3d51b9562673 staging: ccree: add CRYPTO 
dependency)
Merging char-misc.current/char-misc-linus (cdc1daca1b9b MAINTAINERS: Change 
maintainer of genwqe driver)
Merging input-current/for-linus (a04f144059ac Input: elan_i2c - ignore signals 
when finishing updating firmware)
Merging crypto-current/master (f3ad587070d6 crypto: gcm - wait for crypto op 
not signal safe)
Merging ide/master (acfead32f3f9 ide: don't call memcpy with the same source 
and destination)
Merging vfio-fixes/for-linus (39da7c509acf Linux 4.11-rc6)
Merging kselftest-fixes/fixes (2ea659a9ef48 Linux 4.12-rc

Re: panel-samsung-s6e3ha2.c:undefined reference to `backlight_device_unregister'

2017-05-29 Thread Hoegeun Kwon

Dear Thierry,

This problem need to [1] patch.

[1] https://patchwork.kernel.org/patch/9688585/

Best regards,
Hoegeun

On 05/28/2017 11:52 AM, kbuild test robot wrote:

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 
master
head:   249f1efd8e3d58f86469fc305b0eda39db18d7ce
commit: ed29f9426d9bf1b5ec7e79dfb28f39de7b3a0069 drm/panel: Add support for 
S6E3HA2 panel driver on TM2 board
date:   7 weeks ago
config: x86_64-randconfig-s3-05280946 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
 git checkout ed29f9426d9bf1b5ec7e79dfb28f39de7b3a0069
 # save the attached .config to linux build tree
 make ARCH=x86_64

All errors (new ones prefixed by >>):

drivers/built-in.o: In function `s6e3ha2_remove':

panel-samsung-s6e3ha2.c:(.text+0x1cc45f): undefined reference to 
`backlight_device_unregister'

drivers/built-in.o: In function `s6e3ha2_probe':

panel-samsung-s6e3ha2.c:(.text+0x1cd24c): undefined reference to 
`backlight_device_register'

panel-samsung-s6e3ha2.c:(.text+0x1cd34d): undefined reference to 
`backlight_device_unregister'

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




Re: panel-samsung-s6e3ha2.c:undefined reference to `backlight_device_unregister'

2017-05-29 Thread Hoegeun Kwon

Dear Thierry,

This problem need to [1] patch.

[1] https://patchwork.kernel.org/patch/9688585/

Best regards,
Hoegeun

On 05/28/2017 11:52 AM, kbuild test robot wrote:

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 
master
head:   249f1efd8e3d58f86469fc305b0eda39db18d7ce
commit: ed29f9426d9bf1b5ec7e79dfb28f39de7b3a0069 drm/panel: Add support for 
S6E3HA2 panel driver on TM2 board
date:   7 weeks ago
config: x86_64-randconfig-s3-05280946 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
 git checkout ed29f9426d9bf1b5ec7e79dfb28f39de7b3a0069
 # save the attached .config to linux build tree
 make ARCH=x86_64

All errors (new ones prefixed by >>):

drivers/built-in.o: In function `s6e3ha2_remove':

panel-samsung-s6e3ha2.c:(.text+0x1cc45f): undefined reference to 
`backlight_device_unregister'

drivers/built-in.o: In function `s6e3ha2_probe':

panel-samsung-s6e3ha2.c:(.text+0x1cd24c): undefined reference to 
`backlight_device_register'

panel-samsung-s6e3ha2.c:(.text+0x1cd34d): undefined reference to 
`backlight_device_unregister'

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




[PATCH v4] sched/deadline: Change the time to replenish runtime for sleep tasks

2017-05-29 Thread Byungchul Park
Hi Juri, Peterz,

I checked if this case actually happens practically but it has not
happened. As expected, it's too rare to happen, though it can save
unnecessary interrupts if all the timings are satisfied as the example
in the commit log.

So I don't think it's that worth as much as I initially thought, but
I'm just curious about your opinions. Could you give me your opinions?
It would be appriciated if you do.

Thanks,
Byungchul

->8-
>From c94ba55ddd6e67fe3f6a0a34e79f884b91b747ab Mon Sep 17 00:00:00 2001
From: Byungchul Park 
Date: Tue, 30 May 2017 11:29:45 +0900
Subject: [PATCH v4] sched/deadline: Change the time to replenish runtime for
 sleep tasks

Let's consider the following example.

  (N+1)th tick
  (N)th tick |
   | |
timeline : o...o...o.o...o...o..o
   ^   ^ ^   ^  ^
   |   | |   |  |
   start   | |   |  |
original runtime |   |  |
 sleep with (-)runtime   |  |
 original deadline  |
  wake up

When this task is woken up, a negative runtime should be considered,
which means that the task should get penalized when assigning runtime,
becasue it already spent more than expected. Current code handles this
by replenishing a runtime in hrtimer callback for deadline. But this
approach has room for improvement:

   We can avoid setting a deadline timer and save an interrupt if the
   task sleeps for long time so that the deadline passes, since that
   can be handled on wake-up.

So does not set a deadline timer in that case and force to replenish it
when waking it up.

Signed-off-by: Byungchul Park 
---
 kernel/sched/deadline.c | 36 +---
 1 file changed, 29 insertions(+), 7 deletions(-)

diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
index 27737f3..e7a7209 100644
--- a/kernel/sched/deadline.c
+++ b/kernel/sched/deadline.c
@@ -498,13 +498,23 @@ static void update_dl_entity(struct sched_dl_entity 
*dl_se,
struct dl_rq *dl_rq = dl_rq_of_se(dl_se);
struct rq *rq = rq_of_dl_rq(dl_rq);
 
-   if (dl_time_before(dl_se->deadline, rq_clock(rq)) ||
-   dl_entity_overflow(dl_se, pi_se, rq_clock(rq))) {
+   if (dl_time_before(dl_se->deadline, rq_clock(rq)))
+   replenish_dl_entity(dl_se, pi_se);
+
+   if (dl_entity_overflow(dl_se, pi_se, rq_clock(rq))) {
dl_se->deadline = rq_clock(rq) + pi_se->dl_deadline;
dl_se->runtime = pi_se->dl_runtime;
}
 }
 
+static inline void stop_dl_timer(struct task_struct *p)
+{
+   struct sched_dl_entity *dl_se = >dl;
+   struct hrtimer *timer = _se->dl_timer;
+
+   hrtimer_try_to_cancel(timer);
+}
+
 /*
  * If the entity depleted all its runtime, and if we want it to sleep
  * while waiting for some new execution time to become available, we
@@ -621,13 +631,11 @@ static enum hrtimer_restart dl_task_timer(struct hrtimer 
*timer)
 * __dequeue_task_dl()
 * prev->on_rq = 0;
 *
-* We can be both throttled and !queued. Replenish the counter
-* but do not enqueue -- wait for our wakeup to do that.
+* We can be both throttled and !queued. Wait for our wakeup to
+* replenish runtime and enqueue p.
 */
-   if (!task_on_rq_queued(p)) {
-   replenish_dl_entity(dl_se, dl_se);
+   if (!task_on_rq_queued(p))
goto unlock;
-   }
 
 #ifdef CONFIG_SMP
if (unlikely(!rq->online)) {
@@ -956,6 +964,13 @@ static void enqueue_task_dl(struct rq *rq, struct 
task_struct *p, int flags)
if (p->dl.dl_throttled && !(flags & ENQUEUE_REPLENISH))
return;
 
+   /*
+* start_dl_timer() would do nothing if already deadline < now.
+* The case will be handled in update_dl_entity().
+*/
+   if (p->dl.dl_throttled && (flags & ENQUEUE_WAKEUP))
+   start_dl_timer(p);
+
enqueue_dl_entity(>dl, pi_se, flags);
 
if (!task_current(rq, p) && tsk_nr_cpus_allowed(p) > 1)
@@ -964,6 +979,13 @@ static void enqueue_task_dl(struct rq *rq, struct 
task_struct *p, int flags)
 
 static void __dequeue_task_dl(struct rq *rq, struct task_struct *p, int flags)
 {
+   /*
+* Runtime will be replenished on wake-up. We can avoid
+* unnecessary timer interrupt in this case.
+*/
+   if (p->dl.dl_throttled && (flags & DEQUEUE_SLEEP))
+   stop_dl_timer(p);
+
dequeue_dl_entity(>dl);
dequeue_pushable_dl_task(rq, p);
 }
-- 
1.9.1



[PATCH v4] sched/deadline: Change the time to replenish runtime for sleep tasks

2017-05-29 Thread Byungchul Park
Hi Juri, Peterz,

I checked if this case actually happens practically but it has not
happened. As expected, it's too rare to happen, though it can save
unnecessary interrupts if all the timings are satisfied as the example
in the commit log.

So I don't think it's that worth as much as I initially thought, but
I'm just curious about your opinions. Could you give me your opinions?
It would be appriciated if you do.

Thanks,
Byungchul

->8-
>From c94ba55ddd6e67fe3f6a0a34e79f884b91b747ab Mon Sep 17 00:00:00 2001
From: Byungchul Park 
Date: Tue, 30 May 2017 11:29:45 +0900
Subject: [PATCH v4] sched/deadline: Change the time to replenish runtime for
 sleep tasks

Let's consider the following example.

  (N+1)th tick
  (N)th tick |
   | |
timeline : o...o...o.o...o...o..o
   ^   ^ ^   ^  ^
   |   | |   |  |
   start   | |   |  |
original runtime |   |  |
 sleep with (-)runtime   |  |
 original deadline  |
  wake up

When this task is woken up, a negative runtime should be considered,
which means that the task should get penalized when assigning runtime,
becasue it already spent more than expected. Current code handles this
by replenishing a runtime in hrtimer callback for deadline. But this
approach has room for improvement:

   We can avoid setting a deadline timer and save an interrupt if the
   task sleeps for long time so that the deadline passes, since that
   can be handled on wake-up.

So does not set a deadline timer in that case and force to replenish it
when waking it up.

Signed-off-by: Byungchul Park 
---
 kernel/sched/deadline.c | 36 +---
 1 file changed, 29 insertions(+), 7 deletions(-)

diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
index 27737f3..e7a7209 100644
--- a/kernel/sched/deadline.c
+++ b/kernel/sched/deadline.c
@@ -498,13 +498,23 @@ static void update_dl_entity(struct sched_dl_entity 
*dl_se,
struct dl_rq *dl_rq = dl_rq_of_se(dl_se);
struct rq *rq = rq_of_dl_rq(dl_rq);
 
-   if (dl_time_before(dl_se->deadline, rq_clock(rq)) ||
-   dl_entity_overflow(dl_se, pi_se, rq_clock(rq))) {
+   if (dl_time_before(dl_se->deadline, rq_clock(rq)))
+   replenish_dl_entity(dl_se, pi_se);
+
+   if (dl_entity_overflow(dl_se, pi_se, rq_clock(rq))) {
dl_se->deadline = rq_clock(rq) + pi_se->dl_deadline;
dl_se->runtime = pi_se->dl_runtime;
}
 }
 
+static inline void stop_dl_timer(struct task_struct *p)
+{
+   struct sched_dl_entity *dl_se = >dl;
+   struct hrtimer *timer = _se->dl_timer;
+
+   hrtimer_try_to_cancel(timer);
+}
+
 /*
  * If the entity depleted all its runtime, and if we want it to sleep
  * while waiting for some new execution time to become available, we
@@ -621,13 +631,11 @@ static enum hrtimer_restart dl_task_timer(struct hrtimer 
*timer)
 * __dequeue_task_dl()
 * prev->on_rq = 0;
 *
-* We can be both throttled and !queued. Replenish the counter
-* but do not enqueue -- wait for our wakeup to do that.
+* We can be both throttled and !queued. Wait for our wakeup to
+* replenish runtime and enqueue p.
 */
-   if (!task_on_rq_queued(p)) {
-   replenish_dl_entity(dl_se, dl_se);
+   if (!task_on_rq_queued(p))
goto unlock;
-   }
 
 #ifdef CONFIG_SMP
if (unlikely(!rq->online)) {
@@ -956,6 +964,13 @@ static void enqueue_task_dl(struct rq *rq, struct 
task_struct *p, int flags)
if (p->dl.dl_throttled && !(flags & ENQUEUE_REPLENISH))
return;
 
+   /*
+* start_dl_timer() would do nothing if already deadline < now.
+* The case will be handled in update_dl_entity().
+*/
+   if (p->dl.dl_throttled && (flags & ENQUEUE_WAKEUP))
+   start_dl_timer(p);
+
enqueue_dl_entity(>dl, pi_se, flags);
 
if (!task_current(rq, p) && tsk_nr_cpus_allowed(p) > 1)
@@ -964,6 +979,13 @@ static void enqueue_task_dl(struct rq *rq, struct 
task_struct *p, int flags)
 
 static void __dequeue_task_dl(struct rq *rq, struct task_struct *p, int flags)
 {
+   /*
+* Runtime will be replenished on wake-up. We can avoid
+* unnecessary timer interrupt in this case.
+*/
+   if (p->dl.dl_throttled && (flags & DEQUEUE_SLEEP))
+   stop_dl_timer(p);
+
dequeue_dl_entity(>dl);
dequeue_pushable_dl_task(rq, p);
 }
-- 
1.9.1



Re: [PATCH] macintosh: move mac_hid driver to input/mouse.

2017-05-29 Thread Dmitry Torokhov
On Mon, May 29, 2017 at 08:03:25PM +0200, Michal Suchánek wrote:
> On Sun, 28 May 2017 10:55:40 -0700
> Dmitry Torokhov  wrote:
> 
> > On Sun, May 28, 2017 at 11:47:58AM +0200, Michal Suchanek wrote:
> > > On Tue, 9 May 2017 17:43:27 -0700
> > > Dmitry Torokhov  wrote:
> 
> > > 
> > > If not then please do your job as maintainer and accept trivial
> > > patches for perfectly working drivers we have now.  
> > 
> > I am doing my job as a maintainer right now. The driver might have
> > been beneficial 15 years ago, when we did not have better options,
> > but I would rather not continue expanding it's use.
> > 
> > The main problem with the driver is that the functionality it is not
> > easily discoverable by end users. And once you plumb it through
> > userspace to present users with options you might as well handle it
> > all in userspace.
> > 
> ...
> > 
> > >   
> > > > 
> > > > What hardware do you believe would benefit from this and why?  
> > > 
> > > Any touchpad hardware where you cannot press two buttons at once to
> > > emulate the third button due to hardware design. And any touchpad
> > > hardware on which some of the buttons are broken when it comes to
> > > it.
> > > 
> > > It is built into a notebook and works fine for moving the cursor but
> > > due to lack of usable buttons you still need a mouse to use the
> > > notebook.  
> > 
> > Have you tried simply redefining keymap of your keyboard to emit
> > BTN_RIGHT/BTN_MIDDLE? Both atkbd and HID keyboards support keymap
> > updates from userspace/udev/hwdb and if there is a driver that does
> > not support it I will take patches fixing that.
> 
> How is that more easily discoverable by users?

It is not, but the benefit that it does not require a new driver and
uses standard tools to update the mapping.

> 
> More importantly how is that mapping supposed to be represented in a
> hwdb file?
> 
> The help text in the hwdb file says:
> 
> # Scan codes are specified as:
> #   KEYBOARD_KEY_=
> # The scan code should be expressed in hex lowercase. The key codes
> # are retrieved and normalized from the kernel input API header.
> 
> So they are converted in some unspecified way.
> 
> The example below defines some mappings, presumably:
> 
> evdev:atkbd:dmi:bvn*:bvr*:bd*:svnAcer*:pn*
> evdev:atkbd:dmi:bvn*:bvr*:bd*:svnGateway*:pnA0A1*:pvr*
> evdev:atkbd:dmi:bvn*:bvr*:bd*:svneMachines:pneMachines*E725:pvr*
>  KEYBOARD_KEY_a5=help   # Fn+F1
>  KEYBOARD_KEY_a6=setup  # Fn+F2
>  KEYBOARD_KEY_a7=battery# Fn+F3
> 
> /usr/include/linux/input-event-codes.h has occurence of battery in
> 
> #define KEY_BATTERY   236
> 
> meaning that the unspecified conversion is probably performed by
> 
> 1) stripping KEY_ prefix
> 2) converting to lowercase
> 
> This is what systemd hwdb check script does in reverse when checking
> the keycode values.
> 
> The BTN_LEFT 0x110 value does not conflict with KEY_* values, though.
> So technically you could include it in the keymap. If you had a tool
> for that.

Hmm, sounds like you want a patch to udev/systemd. For the kernel there
is no difference between keys and buttons, except symbolic names. They
all go into dev->keybit and are reported with input_report_key().

> And if it is not rejected by the kernel.

It should not. setkeycodes definitely works on atkbd.

> And if it does not
> crash your X server which is very picky about receiving pointer events
> from a keyboard or the other way around.

Sounds like you want to make X server more resilient ;)

But really, it all is better solved in userspace, where you can surface
all options to the user. For example Chrome OS uses Alt + mouse button
(or tap) to do right click, I am sure Gnome or KDE has similar support
for right and middle buttons.

Solving this at kernel is wrong place, similarly how we avoid parsing
user gestures (edge scrolling, 2-finger scrolling, pitch/zoom, etc) in
kernel. Yes, we have legacy drivers (like mousedev) that are artefacts
of times when userspace support was not there and it made sense to
covert everything to emulated PS/2, but that time is long gone.

Thanks.

-- 
Dmitry


Re: [PATCH] macintosh: move mac_hid driver to input/mouse.

2017-05-29 Thread Dmitry Torokhov
On Mon, May 29, 2017 at 08:03:25PM +0200, Michal Suchánek wrote:
> On Sun, 28 May 2017 10:55:40 -0700
> Dmitry Torokhov  wrote:
> 
> > On Sun, May 28, 2017 at 11:47:58AM +0200, Michal Suchanek wrote:
> > > On Tue, 9 May 2017 17:43:27 -0700
> > > Dmitry Torokhov  wrote:
> 
> > > 
> > > If not then please do your job as maintainer and accept trivial
> > > patches for perfectly working drivers we have now.  
> > 
> > I am doing my job as a maintainer right now. The driver might have
> > been beneficial 15 years ago, when we did not have better options,
> > but I would rather not continue expanding it's use.
> > 
> > The main problem with the driver is that the functionality it is not
> > easily discoverable by end users. And once you plumb it through
> > userspace to present users with options you might as well handle it
> > all in userspace.
> > 
> ...
> > 
> > >   
> > > > 
> > > > What hardware do you believe would benefit from this and why?  
> > > 
> > > Any touchpad hardware where you cannot press two buttons at once to
> > > emulate the third button due to hardware design. And any touchpad
> > > hardware on which some of the buttons are broken when it comes to
> > > it.
> > > 
> > > It is built into a notebook and works fine for moving the cursor but
> > > due to lack of usable buttons you still need a mouse to use the
> > > notebook.  
> > 
> > Have you tried simply redefining keymap of your keyboard to emit
> > BTN_RIGHT/BTN_MIDDLE? Both atkbd and HID keyboards support keymap
> > updates from userspace/udev/hwdb and if there is a driver that does
> > not support it I will take patches fixing that.
> 
> How is that more easily discoverable by users?

It is not, but the benefit that it does not require a new driver and
uses standard tools to update the mapping.

> 
> More importantly how is that mapping supposed to be represented in a
> hwdb file?
> 
> The help text in the hwdb file says:
> 
> # Scan codes are specified as:
> #   KEYBOARD_KEY_=
> # The scan code should be expressed in hex lowercase. The key codes
> # are retrieved and normalized from the kernel input API header.
> 
> So they are converted in some unspecified way.
> 
> The example below defines some mappings, presumably:
> 
> evdev:atkbd:dmi:bvn*:bvr*:bd*:svnAcer*:pn*
> evdev:atkbd:dmi:bvn*:bvr*:bd*:svnGateway*:pnA0A1*:pvr*
> evdev:atkbd:dmi:bvn*:bvr*:bd*:svneMachines:pneMachines*E725:pvr*
>  KEYBOARD_KEY_a5=help   # Fn+F1
>  KEYBOARD_KEY_a6=setup  # Fn+F2
>  KEYBOARD_KEY_a7=battery# Fn+F3
> 
> /usr/include/linux/input-event-codes.h has occurence of battery in
> 
> #define KEY_BATTERY   236
> 
> meaning that the unspecified conversion is probably performed by
> 
> 1) stripping KEY_ prefix
> 2) converting to lowercase
> 
> This is what systemd hwdb check script does in reverse when checking
> the keycode values.
> 
> The BTN_LEFT 0x110 value does not conflict with KEY_* values, though.
> So technically you could include it in the keymap. If you had a tool
> for that.

Hmm, sounds like you want a patch to udev/systemd. For the kernel there
is no difference between keys and buttons, except symbolic names. They
all go into dev->keybit and are reported with input_report_key().

> And if it is not rejected by the kernel.

It should not. setkeycodes definitely works on atkbd.

> And if it does not
> crash your X server which is very picky about receiving pointer events
> from a keyboard or the other way around.

Sounds like you want to make X server more resilient ;)

But really, it all is better solved in userspace, where you can surface
all options to the user. For example Chrome OS uses Alt + mouse button
(or tap) to do right click, I am sure Gnome or KDE has similar support
for right and middle buttons.

Solving this at kernel is wrong place, similarly how we avoid parsing
user gestures (edge scrolling, 2-finger scrolling, pitch/zoom, etc) in
kernel. Yes, we have legacy drivers (like mousedev) that are artefacts
of times when userspace support was not there and it made sense to
covert everything to emulated PS/2, but that time is long gone.

Thanks.

-- 
Dmitry


Re: [PATCH 5/5] lib/interval_tree: Fast overlap detection

2017-05-29 Thread kbuild test robot
Hi Davidlohr,

[auto build test ERROR on next-20170529]

url:
https://github.com/0day-ci/linux/commits/Davidlohr-Bueso/rbtree-Cache-leftmost-node-internally/20170530-101713
config: x86_64-allmodconfig (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64 

All errors (new ones prefixed by >>):

   include/linux/compiler.h:260:8: sparse: attribute 'no_sanitize_address': 
unknown attribute
   drivers/infiniband/hw/hfi1/mmu_rb.c: In function 'hfi1_mmu_rb_insert':
>> drivers/infiniband/hw/hfi1/mmu_rb.c:183:29: error: passing argument 2 of 
>> '__mmu_int_rb_insert' from incompatible pointer type 
>> [-Werror=incompatible-pointer-types]
 __mmu_int_rb_insert(mnode, >root);
^
   In file included from drivers/infiniband/hw/hfi1/mmu_rb.c:50:0:
   drivers/infiniband/hw/hfi1/mmu_rb.c:90:47: note: expected 'struct 
rb_root_cached *' but argument is of type 'struct rb_root *'
   mmu_node_start, mmu_node_last, static, __mmu_int_rb);
  ^
   include/linux/interval_tree_generic.h:68:15: note: in definition of macro 
'INTERVAL_TREE_DEFINE'
ITSTATIC void ITPREFIX ## _insert(ITSTRUCT *node, \
  ^~~~
>> drivers/infiniband/hw/hfi1/mmu_rb.c:188:30: error: passing argument 2 of 
>> '__mmu_int_rb_remove' from incompatible pointer type 
>> [-Werror=incompatible-pointer-types]
  __mmu_int_rb_remove(mnode, >root);
 ^
   In file included from drivers/infiniband/hw/hfi1/mmu_rb.c:50:0:
   drivers/infiniband/hw/hfi1/mmu_rb.c:90:47: note: expected 'struct 
rb_root_cached *' but argument is of type 'struct rb_root *'
   mmu_node_start, mmu_node_last, static, __mmu_int_rb);
  ^
   include/linux/interval_tree_generic.h:95:15: note: in definition of macro 
'INTERVAL_TREE_DEFINE'
ITSTATIC void ITPREFIX ## _remove(ITSTRUCT *node, \
  ^~~~
   drivers/infiniband/hw/hfi1/mmu_rb.c: In function '__mmu_rb_search':
>> drivers/infiniband/hw/hfi1/mmu_rb.c:205:34: error: passing argument 1 of 
>> '__mmu_int_rb_iter_first' from incompatible pointer type 
>> [-Werror=incompatible-pointer-types]
  node = __mmu_int_rb_iter_first(>root, addr,
 ^
   In file included from drivers/infiniband/hw/hfi1/mmu_rb.c:50:0:
   drivers/infiniband/hw/hfi1/mmu_rb.c:90:47: note: expected 'struct 
rb_root_cached *' but argument is of type 'struct rb_root *'
   mmu_node_start, mmu_node_last, static, __mmu_int_rb);
  ^
   include/linux/interval_tree_generic.h:149:1: note: in definition of macro 
'INTERVAL_TREE_DEFINE'
ITPREFIX ## _iter_first(struct rb_root_cached *root, \
^~~~
   drivers/infiniband/hw/hfi1/mmu_rb.c:208:39: error: passing argument 1 of 
'__mmu_int_rb_iter_first' from incompatible pointer type 
[-Werror=incompatible-pointer-types]
  for (node = __mmu_int_rb_iter_first(>root, addr,
  ^
   In file included from drivers/infiniband/hw/hfi1/mmu_rb.c:50:0:
   drivers/infiniband/hw/hfi1/mmu_rb.c:90:47: note: expected 'struct 
rb_root_cached *' but argument is of type 'struct rb_root *'
   mmu_node_start, mmu_node_last, static, __mmu_int_rb);
  ^
   include/linux/interval_tree_generic.h:149:1: note: in definition of macro 
'INTERVAL_TREE_DEFINE'
ITPREFIX ## _iter_first(struct rb_root_cached *root, \
^~~~
   drivers/infiniband/hw/hfi1/mmu_rb.c: In function 'hfi1_mmu_rb_extract':
   drivers/infiniband/hw/hfi1/mmu_rb.c:229:29: error: passing argument 2 of 
'__mmu_int_rb_remove' from incompatible pointer type 
[-Werror=incompatible-pointer-types]
  __mmu_int_rb_remove(node, >root);
^
   In file included from drivers/infiniband/hw/hfi1/mmu_rb.c:50:0:
   drivers/infiniband/hw/hfi1/mmu_rb.c:90:47: note: expected 'struct 
rb_root_cached *' but argument is of type 'struct rb_root *'
   mmu_node_start, mmu_node_last, static, __mmu_int_rb);
  ^
   include/linux/interval_tree_generic.h:95:15: note: in definition of macro 
'INTERVAL_TREE_DEFINE'
ITSTATIC void ITPREFIX ## _remove(ITSTRUCT *node, \
  ^~~~
   drivers/infiniband/hw/hfi1/mmu_rb.c: In function 'hfi1_mmu_rb_evict':
   drivers/infiniband/hw/hfi1/mmu_rb.c:251:32: error: passing argument 2 of 
'__mmu_int_rb_remove' from incompatible pointer type 
[-Werror=incompatible-pointer-types]
   __mmu_int_rb_remove(rbnode, >root);
   ^
   In file included from drivers/infiniband/hw/hfi1/mmu_rb.c:50:0:
   dri

Re: [PATCH 5/5] lib/interval_tree: Fast overlap detection

2017-05-29 Thread kbuild test robot
Hi Davidlohr,

[auto build test ERROR on next-20170529]

url:
https://github.com/0day-ci/linux/commits/Davidlohr-Bueso/rbtree-Cache-leftmost-node-internally/20170530-101713
config: x86_64-allmodconfig (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64 

All errors (new ones prefixed by >>):

   include/linux/compiler.h:260:8: sparse: attribute 'no_sanitize_address': 
unknown attribute
   drivers/infiniband/hw/hfi1/mmu_rb.c: In function 'hfi1_mmu_rb_insert':
>> drivers/infiniband/hw/hfi1/mmu_rb.c:183:29: error: passing argument 2 of 
>> '__mmu_int_rb_insert' from incompatible pointer type 
>> [-Werror=incompatible-pointer-types]
 __mmu_int_rb_insert(mnode, >root);
^
   In file included from drivers/infiniband/hw/hfi1/mmu_rb.c:50:0:
   drivers/infiniband/hw/hfi1/mmu_rb.c:90:47: note: expected 'struct 
rb_root_cached *' but argument is of type 'struct rb_root *'
   mmu_node_start, mmu_node_last, static, __mmu_int_rb);
  ^
   include/linux/interval_tree_generic.h:68:15: note: in definition of macro 
'INTERVAL_TREE_DEFINE'
ITSTATIC void ITPREFIX ## _insert(ITSTRUCT *node, \
  ^~~~
>> drivers/infiniband/hw/hfi1/mmu_rb.c:188:30: error: passing argument 2 of 
>> '__mmu_int_rb_remove' from incompatible pointer type 
>> [-Werror=incompatible-pointer-types]
  __mmu_int_rb_remove(mnode, >root);
 ^
   In file included from drivers/infiniband/hw/hfi1/mmu_rb.c:50:0:
   drivers/infiniband/hw/hfi1/mmu_rb.c:90:47: note: expected 'struct 
rb_root_cached *' but argument is of type 'struct rb_root *'
   mmu_node_start, mmu_node_last, static, __mmu_int_rb);
  ^
   include/linux/interval_tree_generic.h:95:15: note: in definition of macro 
'INTERVAL_TREE_DEFINE'
ITSTATIC void ITPREFIX ## _remove(ITSTRUCT *node, \
  ^~~~
   drivers/infiniband/hw/hfi1/mmu_rb.c: In function '__mmu_rb_search':
>> drivers/infiniband/hw/hfi1/mmu_rb.c:205:34: error: passing argument 1 of 
>> '__mmu_int_rb_iter_first' from incompatible pointer type 
>> [-Werror=incompatible-pointer-types]
  node = __mmu_int_rb_iter_first(>root, addr,
 ^
   In file included from drivers/infiniband/hw/hfi1/mmu_rb.c:50:0:
   drivers/infiniband/hw/hfi1/mmu_rb.c:90:47: note: expected 'struct 
rb_root_cached *' but argument is of type 'struct rb_root *'
   mmu_node_start, mmu_node_last, static, __mmu_int_rb);
  ^
   include/linux/interval_tree_generic.h:149:1: note: in definition of macro 
'INTERVAL_TREE_DEFINE'
ITPREFIX ## _iter_first(struct rb_root_cached *root, \
^~~~
   drivers/infiniband/hw/hfi1/mmu_rb.c:208:39: error: passing argument 1 of 
'__mmu_int_rb_iter_first' from incompatible pointer type 
[-Werror=incompatible-pointer-types]
  for (node = __mmu_int_rb_iter_first(>root, addr,
  ^
   In file included from drivers/infiniband/hw/hfi1/mmu_rb.c:50:0:
   drivers/infiniband/hw/hfi1/mmu_rb.c:90:47: note: expected 'struct 
rb_root_cached *' but argument is of type 'struct rb_root *'
   mmu_node_start, mmu_node_last, static, __mmu_int_rb);
  ^
   include/linux/interval_tree_generic.h:149:1: note: in definition of macro 
'INTERVAL_TREE_DEFINE'
ITPREFIX ## _iter_first(struct rb_root_cached *root, \
^~~~
   drivers/infiniband/hw/hfi1/mmu_rb.c: In function 'hfi1_mmu_rb_extract':
   drivers/infiniband/hw/hfi1/mmu_rb.c:229:29: error: passing argument 2 of 
'__mmu_int_rb_remove' from incompatible pointer type 
[-Werror=incompatible-pointer-types]
  __mmu_int_rb_remove(node, >root);
^
   In file included from drivers/infiniband/hw/hfi1/mmu_rb.c:50:0:
   drivers/infiniband/hw/hfi1/mmu_rb.c:90:47: note: expected 'struct 
rb_root_cached *' but argument is of type 'struct rb_root *'
   mmu_node_start, mmu_node_last, static, __mmu_int_rb);
  ^
   include/linux/interval_tree_generic.h:95:15: note: in definition of macro 
'INTERVAL_TREE_DEFINE'
ITSTATIC void ITPREFIX ## _remove(ITSTRUCT *node, \
  ^~~~
   drivers/infiniband/hw/hfi1/mmu_rb.c: In function 'hfi1_mmu_rb_evict':
   drivers/infiniband/hw/hfi1/mmu_rb.c:251:32: error: passing argument 2 of 
'__mmu_int_rb_remove' from incompatible pointer type 
[-Werror=incompatible-pointer-types]
   __mmu_int_rb_remove(rbnode, >root);
   ^
   In file included from drivers/infiniband/hw/hfi1/mmu_rb.c:50:0:
   dri

Re: [PATCH RFC] tpm: migrate pubek_show to struct tpm_buf

2017-05-29 Thread Jarkko Sakkinen
On Thu, May 25, 2017 at 04:40:50PM -0600, Jason Gunthorpe wrote:
> On Thu, May 25, 2017 at 03:28:01PM -0700, Jarkko Sakkinen wrote:
> > On Thu, May 25, 2017 at 03:16:13PM -0600, Jason Gunthorpe wrote:
> > > On Thu, May 25, 2017 at 02:11:04PM -0700, Jarkko Sakkinen wrote:
> > > > struct tpm_chip *chip = to_tpm_chip(dev);
> > > > +   char anti_replay[20];
> > > >  
> > > > -   tpm_cmd.header.in = tpm_readpubek_header;
> > > > -   err = tpm_transmit_cmd(chip, NULL, _cmd, 
> > > > READ_PUBEK_RESULT_SIZE,
> > > > +   rc = tpm_buf_init(_buf, TPM_TAG_RQU_COMMAND, 
> > > > TPM_ORD_READPUBEK);
> > > > +   if (rc)
> > > > +   return rc;
> > > > +
> > > > +   /* The checksum is ignored so it doesn't matter what the 
> > > > contents are.
> > > > +*/
> > > > +   tpm_buf_append(_buf, anti_replay, sizeof(anti_replay));
> > > 
> > > It does matter, we do not want to leak random kernel memory incase it
> > > has something sensitive. Zero anti_replay.
> > 
> > If there was a leak it has existed before this change as tpm_cmd was
> > also allocated from stack. And there is not leak because the checksum is
> > not printed.
> 
> It leaks stack memory to the TPM which is not OK.

Right, of course, vtpm_tpm_proxy.

/Jarkko


Re: [PATCH v3] tpm: vtpm_proxy: Suppress error logging when in closed state

2017-05-29 Thread Jarkko Sakkinen
On Thu, May 25, 2017 at 06:29:13PM -0400, Stefan Berger wrote:
> Suppress the error logging when the core TPM driver sends commands
> to the VTPM proxy driver and -EPIPE is returned in case the VTPM
> proxy driver is 'closed' (closed anonymous file descriptor).  This
> error code is only returned by the send function and by tpm_transmit
> when the VTPM proxy driver is being used.
> 
> Signed-off-by: Stefan Berger 

Reviwed-by: Jarkko Sakkinen 

/Jarkko

> ---
>  drivers/char/tpm/tpm-interface.c | 5 +++--
>  drivers/char/tpm/tpm2-cmd.c  | 2 +-
>  2 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm-interface.c 
> b/drivers/char/tpm/tpm-interface.c
> index d711186..d2b4df6 100644
> --- a/drivers/char/tpm/tpm-interface.c
> +++ b/drivers/char/tpm/tpm-interface.c
> @@ -430,8 +430,9 @@ ssize_t tpm_transmit(struct tpm_chip *chip, struct 
> tpm_space *space,
>  
>   rc = chip->ops->send(chip, (u8 *) buf, count);
>   if (rc < 0) {
> - dev_err(>dev,
> - "tpm_transmit: tpm_send: error %d\n", rc);
> + if (rc != -EPIPE)
> + dev_err(>dev,
> + "%s: tpm_send: error %d\n", __func__, rc);
>   goto out;
>   }
>  
> diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
> index 3ee6883..3a99643 100644
> --- a/drivers/char/tpm/tpm2-cmd.c
> +++ b/drivers/char/tpm/tpm2-cmd.c
> @@ -840,7 +840,7 @@ void tpm2_shutdown(struct tpm_chip *chip, u16 
> shutdown_type)
>   /* In places where shutdown command is sent there's no much we can do
>* except print the error code on a system failure.
>*/
> - if (rc < 0)
> + if (rc < 0 && rc != -EPIPE)
>   dev_warn(>dev, "transmit returned %d while stopping the 
> TPM",
>rc);
>  }
> -- 
> 2.4.3
> 


Re: [PATCH v2] mm/oom_kill: count global and memory cgroup oom kills

2017-05-29 Thread Konstantin Khlebnikov
On Tue, May 30, 2017 at 7:29 AM, David Rientjes  wrote:
> On Thu, 25 May 2017, Konstantin Khlebnikov wrote:
>
>> diff --git a/mm/oom_kill.c b/mm/oom_kill.c
>> index 04c9143a8625..dd30a045ef5b 100644
>> --- a/mm/oom_kill.c
>> +++ b/mm/oom_kill.c
>> @@ -876,6 +876,11 @@ static void oom_kill_process(struct oom_control *oc, 
>> const char *message)
>>   /* Get a reference to safely compare mm after task_unlock(victim) */
>>   mm = victim->mm;
>>   mmgrab(mm);
>> +
>> + /* Raise event before sending signal: reaper must see this */
>
> How is the oom reaper involved here?

Task reaper - OOM event should happens before SIGCHLD.

>
>> + count_vm_event(OOM_KILL);
>> + mem_cgroup_count_vm_event(mm, OOM_KILL);
>> +
>>   /*
>>* We should send SIGKILL before setting TIF_MEMDIE in order to prevent
>>* the OOM victim from depleting the memory reserves from the user
>
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majord...@kvack.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: mailto:"d...@kvack.org;> em...@kvack.org 


Re: [PATCH RFC] tpm: migrate pubek_show to struct tpm_buf

2017-05-29 Thread Jarkko Sakkinen
On Thu, May 25, 2017 at 04:40:50PM -0600, Jason Gunthorpe wrote:
> On Thu, May 25, 2017 at 03:28:01PM -0700, Jarkko Sakkinen wrote:
> > On Thu, May 25, 2017 at 03:16:13PM -0600, Jason Gunthorpe wrote:
> > > On Thu, May 25, 2017 at 02:11:04PM -0700, Jarkko Sakkinen wrote:
> > > > struct tpm_chip *chip = to_tpm_chip(dev);
> > > > +   char anti_replay[20];
> > > >  
> > > > -   tpm_cmd.header.in = tpm_readpubek_header;
> > > > -   err = tpm_transmit_cmd(chip, NULL, _cmd, 
> > > > READ_PUBEK_RESULT_SIZE,
> > > > +   rc = tpm_buf_init(_buf, TPM_TAG_RQU_COMMAND, 
> > > > TPM_ORD_READPUBEK);
> > > > +   if (rc)
> > > > +   return rc;
> > > > +
> > > > +   /* The checksum is ignored so it doesn't matter what the 
> > > > contents are.
> > > > +*/
> > > > +   tpm_buf_append(_buf, anti_replay, sizeof(anti_replay));
> > > 
> > > It does matter, we do not want to leak random kernel memory incase it
> > > has something sensitive. Zero anti_replay.
> > 
> > If there was a leak it has existed before this change as tpm_cmd was
> > also allocated from stack. And there is not leak because the checksum is
> > not printed.
> 
> It leaks stack memory to the TPM which is not OK.

Right, of course, vtpm_tpm_proxy.

/Jarkko


Re: [PATCH v3] tpm: vtpm_proxy: Suppress error logging when in closed state

2017-05-29 Thread Jarkko Sakkinen
On Thu, May 25, 2017 at 06:29:13PM -0400, Stefan Berger wrote:
> Suppress the error logging when the core TPM driver sends commands
> to the VTPM proxy driver and -EPIPE is returned in case the VTPM
> proxy driver is 'closed' (closed anonymous file descriptor).  This
> error code is only returned by the send function and by tpm_transmit
> when the VTPM proxy driver is being used.
> 
> Signed-off-by: Stefan Berger 

Reviwed-by: Jarkko Sakkinen 

/Jarkko

> ---
>  drivers/char/tpm/tpm-interface.c | 5 +++--
>  drivers/char/tpm/tpm2-cmd.c  | 2 +-
>  2 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm-interface.c 
> b/drivers/char/tpm/tpm-interface.c
> index d711186..d2b4df6 100644
> --- a/drivers/char/tpm/tpm-interface.c
> +++ b/drivers/char/tpm/tpm-interface.c
> @@ -430,8 +430,9 @@ ssize_t tpm_transmit(struct tpm_chip *chip, struct 
> tpm_space *space,
>  
>   rc = chip->ops->send(chip, (u8 *) buf, count);
>   if (rc < 0) {
> - dev_err(>dev,
> - "tpm_transmit: tpm_send: error %d\n", rc);
> + if (rc != -EPIPE)
> + dev_err(>dev,
> + "%s: tpm_send: error %d\n", __func__, rc);
>   goto out;
>   }
>  
> diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
> index 3ee6883..3a99643 100644
> --- a/drivers/char/tpm/tpm2-cmd.c
> +++ b/drivers/char/tpm/tpm2-cmd.c
> @@ -840,7 +840,7 @@ void tpm2_shutdown(struct tpm_chip *chip, u16 
> shutdown_type)
>   /* In places where shutdown command is sent there's no much we can do
>* except print the error code on a system failure.
>*/
> - if (rc < 0)
> + if (rc < 0 && rc != -EPIPE)
>   dev_warn(>dev, "transmit returned %d while stopping the 
> TPM",
>rc);
>  }
> -- 
> 2.4.3
> 


Re: [PATCH v2] mm/oom_kill: count global and memory cgroup oom kills

2017-05-29 Thread Konstantin Khlebnikov
On Tue, May 30, 2017 at 7:29 AM, David Rientjes  wrote:
> On Thu, 25 May 2017, Konstantin Khlebnikov wrote:
>
>> diff --git a/mm/oom_kill.c b/mm/oom_kill.c
>> index 04c9143a8625..dd30a045ef5b 100644
>> --- a/mm/oom_kill.c
>> +++ b/mm/oom_kill.c
>> @@ -876,6 +876,11 @@ static void oom_kill_process(struct oom_control *oc, 
>> const char *message)
>>   /* Get a reference to safely compare mm after task_unlock(victim) */
>>   mm = victim->mm;
>>   mmgrab(mm);
>> +
>> + /* Raise event before sending signal: reaper must see this */
>
> How is the oom reaper involved here?

Task reaper - OOM event should happens before SIGCHLD.

>
>> + count_vm_event(OOM_KILL);
>> + mem_cgroup_count_vm_event(mm, OOM_KILL);
>> +
>>   /*
>>* We should send SIGKILL before setting TIF_MEMDIE in order to prevent
>>* the OOM victim from depleting the memory reserves from the user
>
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majord...@kvack.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: mailto:"d...@kvack.org;> em...@kvack.org 


[PATCH] Input: synaptics-rmi4 - use %ph to form F34 configuration ID

2017-05-29 Thread Dmitry Torokhov
Instead of printing bytes one by one, let's use %phN to print the buffer in
one go.

Also use hweight8 to count number of partitions instead of inspecting it
bit by bit.

Signed-off-by: Dmitry Torokhov 
---
 drivers/input/rmi4/rmi_f34v7.c | 22 --
 1 file changed, 8 insertions(+), 14 deletions(-)

diff --git a/drivers/input/rmi4/rmi_f34v7.c b/drivers/input/rmi4/rmi_f34v7.c
index ae2db1c3aebf..3991d2943660 100644
--- a/drivers/input/rmi4/rmi_f34v7.c
+++ b/drivers/input/rmi4/rmi_f34v7.c
@@ -9,13 +9,14 @@
  * the Free Software Foundation.
  */
 
+#include 
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
+#include 
 
 #include "rmi_driver.h"
 #include "rmi_f34.h"
@@ -464,7 +465,7 @@ static int rmi_f34v7_read_queries_bl_version(struct 
f34_data *f34)
 static int rmi_f34v7_read_queries(struct f34_data *f34)
 {
int ret;
-   int i, j;
+   int i;
u8 base;
int offset;
u8 *ptable;
@@ -519,9 +520,6 @@ static int rmi_f34v7_read_queries(struct f34_data *f34)
 
if (query_0 & HAS_CONFIG_ID) {
u8 f34_ctrl[CONFIG_ID_SIZE];
-   int i = 0;
-   u8 *p = f34->configuration_id;
-   *p = '\0';
 
ret = rmi_read_block(f34->fn->rmi_dev,
f34->fn->fd.control_base_addr,
@@ -531,13 +529,11 @@ static int rmi_f34v7_read_queries(struct f34_data *f34)
return ret;
 
/* Eat leading zeros */
-   while (i < sizeof(f34_ctrl) && !f34_ctrl[i])
-   i++;
+   for (i = 0; i < sizeof(f34_ctrl) - 1 && !f34_ctrl[i]; i++)
+   /* Empty */;
 
-   for (; i < sizeof(f34_ctrl); i++)
-   p += snprintf(p, f34->configuration_id
- + sizeof(f34->configuration_id) - p,
- "%02X", f34_ctrl[i]);
+   snprintf(f34->configuration_id, sizeof(f34->configuration_id),
+"%*phN", (int)sizeof(f34_ctrl) - i, f34_ctrl + i);
 
rmi_dbg(RMI_DEBUG_FN, >fn->dev, "Configuration ID: %s\n",
f34->configuration_id);
@@ -545,9 +541,7 @@ static int rmi_f34v7_read_queries(struct f34_data *f34)
 
f34->v7.partitions = 0;
for (i = 0; i < sizeof(query_1_7.partition_support); i++)
-   for (j = 0; j < 8; j++)
-   if (query_1_7.partition_support[i] & (1 << j))
-   f34->v7.partitions++;
+   f34->v7.partitions += hweight8(query_1_7.partition_support[i]);
 
rmi_dbg(RMI_DEBUG_FN, >fn->dev, "%s: Supported partitions: %*ph\n",
__func__, sizeof(query_1_7.partition_support),
-- 
2.13.0.219.gdb65acc882-goog


-- 
Dmitry


[PATCH] Input: synaptics-rmi4 - use %ph to form F34 configuration ID

2017-05-29 Thread Dmitry Torokhov
Instead of printing bytes one by one, let's use %phN to print the buffer in
one go.

Also use hweight8 to count number of partitions instead of inspecting it
bit by bit.

Signed-off-by: Dmitry Torokhov 
---
 drivers/input/rmi4/rmi_f34v7.c | 22 --
 1 file changed, 8 insertions(+), 14 deletions(-)

diff --git a/drivers/input/rmi4/rmi_f34v7.c b/drivers/input/rmi4/rmi_f34v7.c
index ae2db1c3aebf..3991d2943660 100644
--- a/drivers/input/rmi4/rmi_f34v7.c
+++ b/drivers/input/rmi4/rmi_f34v7.c
@@ -9,13 +9,14 @@
  * the Free Software Foundation.
  */
 
+#include 
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
+#include 
 
 #include "rmi_driver.h"
 #include "rmi_f34.h"
@@ -464,7 +465,7 @@ static int rmi_f34v7_read_queries_bl_version(struct 
f34_data *f34)
 static int rmi_f34v7_read_queries(struct f34_data *f34)
 {
int ret;
-   int i, j;
+   int i;
u8 base;
int offset;
u8 *ptable;
@@ -519,9 +520,6 @@ static int rmi_f34v7_read_queries(struct f34_data *f34)
 
if (query_0 & HAS_CONFIG_ID) {
u8 f34_ctrl[CONFIG_ID_SIZE];
-   int i = 0;
-   u8 *p = f34->configuration_id;
-   *p = '\0';
 
ret = rmi_read_block(f34->fn->rmi_dev,
f34->fn->fd.control_base_addr,
@@ -531,13 +529,11 @@ static int rmi_f34v7_read_queries(struct f34_data *f34)
return ret;
 
/* Eat leading zeros */
-   while (i < sizeof(f34_ctrl) && !f34_ctrl[i])
-   i++;
+   for (i = 0; i < sizeof(f34_ctrl) - 1 && !f34_ctrl[i]; i++)
+   /* Empty */;
 
-   for (; i < sizeof(f34_ctrl); i++)
-   p += snprintf(p, f34->configuration_id
- + sizeof(f34->configuration_id) - p,
- "%02X", f34_ctrl[i]);
+   snprintf(f34->configuration_id, sizeof(f34->configuration_id),
+"%*phN", (int)sizeof(f34_ctrl) - i, f34_ctrl + i);
 
rmi_dbg(RMI_DEBUG_FN, >fn->dev, "Configuration ID: %s\n",
f34->configuration_id);
@@ -545,9 +541,7 @@ static int rmi_f34v7_read_queries(struct f34_data *f34)
 
f34->v7.partitions = 0;
for (i = 0; i < sizeof(query_1_7.partition_support); i++)
-   for (j = 0; j < 8; j++)
-   if (query_1_7.partition_support[i] & (1 << j))
-   f34->v7.partitions++;
+   f34->v7.partitions += hweight8(query_1_7.partition_support[i]);
 
rmi_dbg(RMI_DEBUG_FN, >fn->dev, "%s: Supported partitions: %*ph\n",
__func__, sizeof(query_1_7.partition_support),
-- 
2.13.0.219.gdb65acc882-goog


-- 
Dmitry


Re: [PATCH 2/2] Input: pm8941-pwrkey: Introduce reboot mode support

2017-05-29 Thread Bjorn Andersson
On Mon 29 May 19:53 PDT 2017, Dmitry Torokhov wrote:

> On Fri, May 26, 2017 at 11:51:30PM -0700, Bjorn Andersson wrote:
> > In some Qualcomm platforms the magic for informing LK which mode to
> > reboot into is stored in the PON_SOFT_RB_SPARE register. Register with
> > the reboot mode helpers to expose this to the user.
> 
> Hmm, is the power key driver the best place to have this? WHy isn't this
> a driver in its own right?
> 

The functionality is part of the "PON" block in the Qualcomm PMICs,
other functionality from this block relates to configuration and
handling related to power-key and reset-key.

Several of these properties are intermingled, so I do believe it's best
to handle them in a single driver; that said, it might no longer be
correct to name the driver "pwrkey" or that it is a "misc input" driver.

Regards,
Bjorn


Re: [PATCH 2/2] Input: pm8941-pwrkey: Introduce reboot mode support

2017-05-29 Thread Bjorn Andersson
On Mon 29 May 19:53 PDT 2017, Dmitry Torokhov wrote:

> On Fri, May 26, 2017 at 11:51:30PM -0700, Bjorn Andersson wrote:
> > In some Qualcomm platforms the magic for informing LK which mode to
> > reboot into is stored in the PON_SOFT_RB_SPARE register. Register with
> > the reboot mode helpers to expose this to the user.
> 
> Hmm, is the power key driver the best place to have this? WHy isn't this
> a driver in its own right?
> 

The functionality is part of the "PON" block in the Qualcomm PMICs,
other functionality from this block relates to configuration and
handling related to power-key and reset-key.

Several of these properties are intermingled, so I do believe it's best
to handle them in a single driver; that said, it might no longer be
correct to name the driver "pwrkey" or that it is a "misc input" driver.

Regards,
Bjorn


Re: [PATCH] tpm, tpmrm: Mark tpmrm_write as static

2017-05-29 Thread Jarkko Sakkinen
On Thu, May 25, 2017 at 07:43:05AM +0200, Peter Huewe wrote:
> sparse complains that tpmrm_write can be made static, and since it is
> right we make it static.
> 
> Signed-off-by: Peter Huewe 

Reviewed-by: Jarkko Sakkinen 

/Jarkko

> ---
>  drivers/char/tpm/tpmrm-dev.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/char/tpm/tpmrm-dev.c b/drivers/char/tpm/tpmrm-dev.c
> index c636e7f..1a0e97a 100644
> --- a/drivers/char/tpm/tpmrm-dev.c
> +++ b/drivers/char/tpm/tpmrm-dev.c
> @@ -45,7 +45,7 @@ static int tpmrm_release(struct inode *inode, struct file 
> *file)
>   return 0;
>  }
>  
> -ssize_t tpmrm_write(struct file *file, const char __user *buf,
> +static ssize_t tpmrm_write(struct file *file, const char __user *buf,
>  size_t size, loff_t *off)
>  {
>   struct file_priv *fpriv = file->private_data;
> -- 
> 2.10.2
> 


Re: [PATCH] tpm, tpmrm: Mark tpmrm_write as static

2017-05-29 Thread Jarkko Sakkinen
On Thu, May 25, 2017 at 07:43:05AM +0200, Peter Huewe wrote:
> sparse complains that tpmrm_write can be made static, and since it is
> right we make it static.
> 
> Signed-off-by: Peter Huewe 

Reviewed-by: Jarkko Sakkinen 

/Jarkko

> ---
>  drivers/char/tpm/tpmrm-dev.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/char/tpm/tpmrm-dev.c b/drivers/char/tpm/tpmrm-dev.c
> index c636e7f..1a0e97a 100644
> --- a/drivers/char/tpm/tpmrm-dev.c
> +++ b/drivers/char/tpm/tpmrm-dev.c
> @@ -45,7 +45,7 @@ static int tpmrm_release(struct inode *inode, struct file 
> *file)
>   return 0;
>  }
>  
> -ssize_t tpmrm_write(struct file *file, const char __user *buf,
> +static ssize_t tpmrm_write(struct file *file, const char __user *buf,
>  size_t size, loff_t *off)
>  {
>   struct file_priv *fpriv = file->private_data;
> -- 
> 2.10.2
> 


Re: [PATCH v2] powerpc/fadump: return error when fadump registration fails

2017-05-29 Thread Mahesh Jagannath Salgaonkar
On 05/27/2017 09:16 PM, Michal Suchanek wrote:
>  - log an error message when registration fails and no error code listed
>  in the switch is returned
>  - translate the hv error code to posix error code and return it from
>  fw_register
>  - return the posix error code from fw_register to the process writing
>  to sysfs
>  - return EEXIST on re-registration
>  - return success on deregistration when fadump is not registered
>  - return ENODEV when no memory is reserved for fadump

Why do we need this ? Userspace can always read back the fadump
registration status from /sys/kernel/fadump_registered (after echo 1 to
it) to find out whether fadump registration succeeded or not.

 /sys/kernel/fadump_registered

This is used to display the fadump registration status as well
as to control (start/stop) the fadump registration.
0 = fadump is not registered.
1 = fadump is registered and ready to handle system crash.

-Mahesh.



Re: [PATCH v2] powerpc/fadump: return error when fadump registration fails

2017-05-29 Thread Mahesh Jagannath Salgaonkar
On 05/27/2017 09:16 PM, Michal Suchanek wrote:
>  - log an error message when registration fails and no error code listed
>  in the switch is returned
>  - translate the hv error code to posix error code and return it from
>  fw_register
>  - return the posix error code from fw_register to the process writing
>  to sysfs
>  - return EEXIST on re-registration
>  - return success on deregistration when fadump is not registered
>  - return ENODEV when no memory is reserved for fadump

Why do we need this ? Userspace can always read back the fadump
registration status from /sys/kernel/fadump_registered (after echo 1 to
it) to find out whether fadump registration succeeded or not.

 /sys/kernel/fadump_registered

This is used to display the fadump registration status as well
as to control (start/stop) the fadump registration.
0 = fadump is not registered.
1 = fadump is registered and ready to handle system crash.

-Mahesh.



Re: [PATCH] clocksource/drivers/fttmr010: Fix aspeed-2500 initialization

2017-05-29 Thread Joel Stanley
On Mon, May 29, 2017 at 5:15 PM, Daniel Lezcano
 wrote:
> On 29/05/2017 08:05, Andrew Jeffery wrote:
>> On Fri, 2017-05-26 at 10:48 +0200, Daniel Lezcano wrote:
>>> The recent changes made the fttmr010 to be more generic and support 
>>> different
>>> timers with a very few differences like moxart or aspeed.
>>>
>>> The aspeed timer uses a countdown and there is a test against the aspeed2400
>>> compatible string to set a flag.
>>>
>>> With the previous patch, we added the aspeed2500 compatible string but 
>>> without
>>> taking care of setting the countdown flag.
>>>
>>> Fix this by specifiying a init function and pass the aspeed flag to a common
>>> init function.
>>>
>>> Signed-off-by: Daniel Lezcano 
>>
>> Tested-by: Andrew Jeffery 
>> Reviewed-by: Andrew Jeffery 

Thanks everyone.

Acked-by: Joel Stanley 

Andrew, I think you had to fix up the clock device tree entries in the
Aspeed device tree. Can you please send me a patch for that?

Cheers,

Joel



>>
>
> Thanks for testing.
>
>   -- Daniel
>
>
> --
>   Linaro.org │ Open source software for ARM SoCs
>
> Follow Linaro:   Facebook |
>  Twitter |
>  Blog
>


Re: [PATCH] clocksource/drivers/fttmr010: Fix aspeed-2500 initialization

2017-05-29 Thread Joel Stanley
On Mon, May 29, 2017 at 5:15 PM, Daniel Lezcano
 wrote:
> On 29/05/2017 08:05, Andrew Jeffery wrote:
>> On Fri, 2017-05-26 at 10:48 +0200, Daniel Lezcano wrote:
>>> The recent changes made the fttmr010 to be more generic and support 
>>> different
>>> timers with a very few differences like moxart or aspeed.
>>>
>>> The aspeed timer uses a countdown and there is a test against the aspeed2400
>>> compatible string to set a flag.
>>>
>>> With the previous patch, we added the aspeed2500 compatible string but 
>>> without
>>> taking care of setting the countdown flag.
>>>
>>> Fix this by specifiying a init function and pass the aspeed flag to a common
>>> init function.
>>>
>>> Signed-off-by: Daniel Lezcano 
>>
>> Tested-by: Andrew Jeffery 
>> Reviewed-by: Andrew Jeffery 

Thanks everyone.

Acked-by: Joel Stanley 

Andrew, I think you had to fix up the clock device tree entries in the
Aspeed device tree. Can you please send me a patch for that?

Cheers,

Joel



>>
>
> Thanks for testing.
>
>   -- Daniel
>
>
> --
>   Linaro.org │ Open source software for ARM SoCs
>
> Follow Linaro:   Facebook |
>  Twitter |
>  Blog
>


[PATCH] MIPS: Lantiq: Fix ASC0/ASC1 clocks

2017-05-29 Thread Martin Schiller
ASC1 is available on every Lantiq SoC (also AmazonSE) and should be
enabled like the other generic xway clocks instead of ASC0, which is
only available for AR9 and Danube.

Signed-off-by: Martin Schiller 
---
 arch/mips/lantiq/xway/sysctrl.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/mips/lantiq/xway/sysctrl.c b/arch/mips/lantiq/xway/sysctrl.c
index 95bec46..cd6dbea 100644
--- a/arch/mips/lantiq/xway/sysctrl.c
+++ b/arch/mips/lantiq/xway/sysctrl.c
@@ -484,9 +484,9 @@ void __init ltq_soc_init(void)
 
/* add our generic xway clocks */
clkdev_add_pmu("1000.fpi", NULL, 0, 0, PMU_FPI);
-   clkdev_add_pmu("1e100400.serial", NULL, 0, 0, PMU_ASC0);
clkdev_add_pmu("1e100a00.gptu", NULL, 1, 0, PMU_GPT);
clkdev_add_pmu("1e100bb0.stp", NULL, 1, 0, PMU_STP);
+   clkdev_add_pmu("1e100c00.serial", NULL, 0, 0, PMU_ASC1);
clkdev_add_pmu("1e104100.dma", NULL, 1, 0, PMU_DMA);
clkdev_add_pmu("1e100800.spi", NULL, 1, 0, PMU_SPI);
clkdev_add_pmu("1e105300.ebu", NULL, 0, 0, PMU_EBU);
@@ -501,7 +501,6 @@ void __init ltq_soc_init(void)
}
 
if (!of_machine_is_compatible("lantiq,ase")) {
-   clkdev_add_pmu("1e100c00.serial", NULL, 0, 0, PMU_ASC1);
clkdev_add_pci();
}
 
-- 
2.1.4



[PATCH] MIPS: Lantiq: Fix ASC0/ASC1 clocks

2017-05-29 Thread Martin Schiller
ASC1 is available on every Lantiq SoC (also AmazonSE) and should be
enabled like the other generic xway clocks instead of ASC0, which is
only available for AR9 and Danube.

Signed-off-by: Martin Schiller 
---
 arch/mips/lantiq/xway/sysctrl.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/mips/lantiq/xway/sysctrl.c b/arch/mips/lantiq/xway/sysctrl.c
index 95bec46..cd6dbea 100644
--- a/arch/mips/lantiq/xway/sysctrl.c
+++ b/arch/mips/lantiq/xway/sysctrl.c
@@ -484,9 +484,9 @@ void __init ltq_soc_init(void)
 
/* add our generic xway clocks */
clkdev_add_pmu("1000.fpi", NULL, 0, 0, PMU_FPI);
-   clkdev_add_pmu("1e100400.serial", NULL, 0, 0, PMU_ASC0);
clkdev_add_pmu("1e100a00.gptu", NULL, 1, 0, PMU_GPT);
clkdev_add_pmu("1e100bb0.stp", NULL, 1, 0, PMU_STP);
+   clkdev_add_pmu("1e100c00.serial", NULL, 0, 0, PMU_ASC1);
clkdev_add_pmu("1e104100.dma", NULL, 1, 0, PMU_DMA);
clkdev_add_pmu("1e100800.spi", NULL, 1, 0, PMU_SPI);
clkdev_add_pmu("1e105300.ebu", NULL, 0, 0, PMU_EBU);
@@ -501,7 +501,6 @@ void __init ltq_soc_init(void)
}
 
if (!of_machine_is_compatible("lantiq,ase")) {
-   clkdev_add_pmu("1e100c00.serial", NULL, 0, 0, PMU_ASC1);
clkdev_add_pci();
}
 
-- 
2.1.4



Re: [linux-sunxi] [PATCH 1/3] ARM: dts: sun7i-a20: Rename bananapi as bananapi m1

2017-05-29 Thread Jagan Teki
On Tue, May 30, 2017 at 3:15 AM, Karsten Merker  wrote:
> On Mon, May 29, 2017 at 07:30:26PM +, Jagan Teki wrote:
>> From: Jagan Teki 
>>
>> from BPI(BIPAI KEJI LIMITED) products the Bananapi board
>> is named as 'Bananapi M1' and this is the starting
>> bananapi board from M1 series.
>>
>> So rename dts and suffix 'M1' on model for the same,
>> so-that next sequence on bananapi starts like M1 Plus, M2 and so..on
>>
>> Signed-off-by: Jagan Teki 
>> ---
>> Note: Bananapi BPI product site
>> http://www.banana-pi.org/product.html
>>
>>  arch/arm/boot/dts/Makefile  |   2 +-
>>  arch/arm/boot/dts/sun7i-a20-bananapi-m1.dts | 286 
>> 
>>  arch/arm/boot/dts/sun7i-a20-bananapi.dts| 286 
>> 
>>  3 files changed, 287 insertions(+), 287 deletions(-)
>>  create mode 100644 arch/arm/boot/dts/sun7i-a20-bananapi-m1.dts
>>  delete mode 100644 arch/arm/boot/dts/sun7i-a20-bananapi.dts
>>
>> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
>> index 45c6e65..1b086f0 100644
>> --- a/arch/arm/boot/dts/Makefile
>> +++ b/arch/arm/boot/dts/Makefile
>> @@ -851,7 +851,7 @@ dtb-$(CONFIG_MACH_SUN6I) += \
>>   sun6i-a31s-sinovoip-bpi-m2.dtb \
>>   sun6i-a31s-yones-toptech-bs1078-v2.dtb
>>  dtb-$(CONFIG_MACH_SUN7I) += \
>> - sun7i-a20-bananapi.dtb \
>> + sun7i-a20-bananapi-m1.dtb \
>>   sun7i-a20-bananapi-m1-plus.dtb \
>>   sun7i-a20-bananapro.dtb \
>>   sun7i-a20-cubieboard2.dtb \
>> diff --git a/arch/arm/boot/dts/sun7i-a20-bananapi-m1.dts 
>> b/arch/arm/boot/dts/sun7i-a20-bananapi-m1.dts
>> new file mode 100644
>> index 000..8b97b89
>> --- /dev/null
>> +++ b/arch/arm/boot/dts/sun7i-a20-bananapi-m1.dts
>> @@ -0,0 +1,286 @@
>> +/*
>> + * Copyright 2014 Hans de Goede 
>> + *
>> + * Hans de Goede 
> [...]
>> +/dts-v1/;
>> +#include "sun7i-a20.dtsi"
>> +#include "sunxi-common-regulators.dtsi"
>> +
>> +#include 
>> +#include 
>> +
>> +/ {
>> + model = "LeMaker Banana Pi M1";
>> + compatible = "lemaker,bananapi", "allwinner,sun7i-a20";
> [...]
>> diff --git a/arch/arm/boot/dts/sun7i-a20-bananapi.dts 
>> b/arch/arm/boot/dts/sun7i-a20-bananapi.dts
>> deleted file mode 100644
>> index ed2f35a..000
>> --- a/arch/arm/boot/dts/sun7i-a20-bananapi.dts
>> +++ /dev/null
>
> NACK!
>
> Please neither rename the dts nor change the model string. Such a
> change would make newer kernels unusable on many existing
> installations without manual fixups by the end user.  Linux
> distributions use databases with model-specific setup information
> (such as the dtb file name, the platform-specific bootscript to
> use, usable kernel flavours (lpae or non-lpae), etc.) on kernel
> installations and kernel upgrades, and those use the model string
> as their key for finding the relevant information.  If you change
> either the dts file name or the model string inside the dts,
> you'll effectively break the proper installation of newer kernel
> versions on existing end user systems.

I understand your concerns about distribution change, but with new
change in 'bananapi' brand owned by BIPAI KEJI(BPI) the model must
need to update and this is not technically as Bananapi board it is
Bananapi  M1 [1]

These are generic changes based on the hardware vendor info.

[1] http://www.banana-pi.org/m1.html

thanks!
-- 
Jagan Teki
Free Software Engineer | www.openedev.com
U-Boot, Linux | Upstream Maintainer
Hyderabad, India.


Re: [linux-sunxi] [PATCH 1/3] ARM: dts: sun7i-a20: Rename bananapi as bananapi m1

2017-05-29 Thread Jagan Teki
On Tue, May 30, 2017 at 3:15 AM, Karsten Merker  wrote:
> On Mon, May 29, 2017 at 07:30:26PM +, Jagan Teki wrote:
>> From: Jagan Teki 
>>
>> from BPI(BIPAI KEJI LIMITED) products the Bananapi board
>> is named as 'Bananapi M1' and this is the starting
>> bananapi board from M1 series.
>>
>> So rename dts and suffix 'M1' on model for the same,
>> so-that next sequence on bananapi starts like M1 Plus, M2 and so..on
>>
>> Signed-off-by: Jagan Teki 
>> ---
>> Note: Bananapi BPI product site
>> http://www.banana-pi.org/product.html
>>
>>  arch/arm/boot/dts/Makefile  |   2 +-
>>  arch/arm/boot/dts/sun7i-a20-bananapi-m1.dts | 286 
>> 
>>  arch/arm/boot/dts/sun7i-a20-bananapi.dts| 286 
>> 
>>  3 files changed, 287 insertions(+), 287 deletions(-)
>>  create mode 100644 arch/arm/boot/dts/sun7i-a20-bananapi-m1.dts
>>  delete mode 100644 arch/arm/boot/dts/sun7i-a20-bananapi.dts
>>
>> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
>> index 45c6e65..1b086f0 100644
>> --- a/arch/arm/boot/dts/Makefile
>> +++ b/arch/arm/boot/dts/Makefile
>> @@ -851,7 +851,7 @@ dtb-$(CONFIG_MACH_SUN6I) += \
>>   sun6i-a31s-sinovoip-bpi-m2.dtb \
>>   sun6i-a31s-yones-toptech-bs1078-v2.dtb
>>  dtb-$(CONFIG_MACH_SUN7I) += \
>> - sun7i-a20-bananapi.dtb \
>> + sun7i-a20-bananapi-m1.dtb \
>>   sun7i-a20-bananapi-m1-plus.dtb \
>>   sun7i-a20-bananapro.dtb \
>>   sun7i-a20-cubieboard2.dtb \
>> diff --git a/arch/arm/boot/dts/sun7i-a20-bananapi-m1.dts 
>> b/arch/arm/boot/dts/sun7i-a20-bananapi-m1.dts
>> new file mode 100644
>> index 000..8b97b89
>> --- /dev/null
>> +++ b/arch/arm/boot/dts/sun7i-a20-bananapi-m1.dts
>> @@ -0,0 +1,286 @@
>> +/*
>> + * Copyright 2014 Hans de Goede 
>> + *
>> + * Hans de Goede 
> [...]
>> +/dts-v1/;
>> +#include "sun7i-a20.dtsi"
>> +#include "sunxi-common-regulators.dtsi"
>> +
>> +#include 
>> +#include 
>> +
>> +/ {
>> + model = "LeMaker Banana Pi M1";
>> + compatible = "lemaker,bananapi", "allwinner,sun7i-a20";
> [...]
>> diff --git a/arch/arm/boot/dts/sun7i-a20-bananapi.dts 
>> b/arch/arm/boot/dts/sun7i-a20-bananapi.dts
>> deleted file mode 100644
>> index ed2f35a..000
>> --- a/arch/arm/boot/dts/sun7i-a20-bananapi.dts
>> +++ /dev/null
>
> NACK!
>
> Please neither rename the dts nor change the model string. Such a
> change would make newer kernels unusable on many existing
> installations without manual fixups by the end user.  Linux
> distributions use databases with model-specific setup information
> (such as the dtb file name, the platform-specific bootscript to
> use, usable kernel flavours (lpae or non-lpae), etc.) on kernel
> installations and kernel upgrades, and those use the model string
> as their key for finding the relevant information.  If you change
> either the dts file name or the model string inside the dts,
> you'll effectively break the proper installation of newer kernel
> versions on existing end user systems.

I understand your concerns about distribution change, but with new
change in 'bananapi' brand owned by BIPAI KEJI(BPI) the model must
need to update and this is not technically as Bananapi board it is
Bananapi  M1 [1]

These are generic changes based on the hardware vendor info.

[1] http://www.banana-pi.org/m1.html

thanks!
-- 
Jagan Teki
Free Software Engineer | www.openedev.com
U-Boot, Linux | Upstream Maintainer
Hyderabad, India.


Re: [PATCH v2] mm/oom_kill: count global and memory cgroup oom kills

2017-05-29 Thread David Rientjes
On Thu, 25 May 2017, Konstantin Khlebnikov wrote:

> diff --git a/mm/oom_kill.c b/mm/oom_kill.c
> index 04c9143a8625..dd30a045ef5b 100644
> --- a/mm/oom_kill.c
> +++ b/mm/oom_kill.c
> @@ -876,6 +876,11 @@ static void oom_kill_process(struct oom_control *oc, 
> const char *message)
>   /* Get a reference to safely compare mm after task_unlock(victim) */
>   mm = victim->mm;
>   mmgrab(mm);
> +
> + /* Raise event before sending signal: reaper must see this */

How is the oom reaper involved here?

> + count_vm_event(OOM_KILL);
> + mem_cgroup_count_vm_event(mm, OOM_KILL);
> +
>   /*
>* We should send SIGKILL before setting TIF_MEMDIE in order to prevent
>* the OOM victim from depleting the memory reserves from the user


Re: [PATCH v2] mm/oom_kill: count global and memory cgroup oom kills

2017-05-29 Thread David Rientjes
On Thu, 25 May 2017, Konstantin Khlebnikov wrote:

> diff --git a/mm/oom_kill.c b/mm/oom_kill.c
> index 04c9143a8625..dd30a045ef5b 100644
> --- a/mm/oom_kill.c
> +++ b/mm/oom_kill.c
> @@ -876,6 +876,11 @@ static void oom_kill_process(struct oom_control *oc, 
> const char *message)
>   /* Get a reference to safely compare mm after task_unlock(victim) */
>   mm = victim->mm;
>   mmgrab(mm);
> +
> + /* Raise event before sending signal: reaper must see this */

How is the oom reaper involved here?

> + count_vm_event(OOM_KILL);
> + mem_cgroup_count_vm_event(mm, OOM_KILL);
> +
>   /*
>* We should send SIGKILL before setting TIF_MEMDIE in order to prevent
>* the OOM victim from depleting the memory reserves from the user


Re: [PATCH v2] mm: introduce MADV_RESET_HUGEPAGE

2017-05-29 Thread David Rientjes
On Mon, 29 May 2017, Mike Rapoport wrote:

> Currently applications can explicitly enable or disable THP for a memory
> region using MADV_HUGEPAGE or MADV_NOHUGEPAGE. However, once either of
> these advises is used, the region will always have
> VM_HUGEPAGE/VM_NOHUGEPAGE flag set in vma->vm_flags.
> The MADV_RESET_HUGEPAGE resets both these flags and allows managing THP in
> the region according to system-wide settings.
> 
> Signed-off-by: Mike Rapoport 
> Acked-by: Kirill A. Shutemov 

I feel like we may be losing some information from the v1 thread regarding 
the usecase.  Would it be possible to add something to the changelog to 
describe what will use this?


Re: [PATCH v2] mm: introduce MADV_RESET_HUGEPAGE

2017-05-29 Thread David Rientjes
On Mon, 29 May 2017, Mike Rapoport wrote:

> Currently applications can explicitly enable or disable THP for a memory
> region using MADV_HUGEPAGE or MADV_NOHUGEPAGE. However, once either of
> these advises is used, the region will always have
> VM_HUGEPAGE/VM_NOHUGEPAGE flag set in vma->vm_flags.
> The MADV_RESET_HUGEPAGE resets both these flags and allows managing THP in
> the region according to system-wide settings.
> 
> Signed-off-by: Mike Rapoport 
> Acked-by: Kirill A. Shutemov 

I feel like we may be losing some information from the v1 thread regarding 
the usecase.  Would it be possible to add something to the changelog to 
describe what will use this?


Re: linux-next: build failure after merge of the rcu tree

2017-05-29 Thread Michael Ellerman
"Paul E. McKenney"  writes:

> On Mon, May 29, 2017 at 04:02:09PM +1000, Stephen Rothwell wrote:
>> Hi Paul,
>> 
>> After merging the rcu tree, today's linux-next build (bfin
>> BF526-EZBRD_defconfig and several other bfin configs) failed like this:
>> 
>> In file included from include/linux/srcu.h:60:0,
>>  from include/linux/notifier.h:15,
>>  from include/linux/memory_hotplug.h:6,
>>  from include/linux/mmzone.h:777,
>>  from include/linux/gfp.h:5,
>>  from include/linux/kmod.h:22,
>>  from include/linux/module.h:13,
>>  from include/linux/moduleloader.h:5,
>>  from arch/blackfin/kernel/module.c:9:
>> include/linux/srcutiny.h: In function 'srcu_torture_stats_print':
>> include/linux/srcutiny.h:96:2: error: 'mod' undeclared (first use in this 
>> function)
>> 
>> Caused by commit
>> 
>>   54ffb22bd841 ("rcutorture: Move SRCU status printing to SRCU 
>> implementations")
>
> And of course this is nothing but a printk().

Ah but it's not, it's a pr_alert():

+   pr_alert("%s%s Tiny SRCU per-CPU(idx=%d): (%hd,%hd)\n",
+tt, tf, idx,
+READ_ONCE(sp->srcu_lock_nesting[!idx]),
+READ_ONCE(sp->srcu_lock_nesting[idx]));

Where pr_alert() is:

#define pr_alert(fmt, ...) \
printk(KERN_ALERT pr_fmt(fmt), ##__VA_ARGS__)


So that's where pr_fmt() is coming into it.

And if any code in module.c called srcu_torture_stats_print(), it would
actually claim to come from "module: ", which would be confusing.

I don't think you use pr_fmt() in the RCU code, so you could skip using
pr_alert() and just use printk(KERN_ALERT "...") and get the same
result, without any interactions with pr_fmt().

cheers


Re: linux-next: build failure after merge of the rcu tree

2017-05-29 Thread Michael Ellerman
"Paul E. McKenney"  writes:

> On Mon, May 29, 2017 at 04:02:09PM +1000, Stephen Rothwell wrote:
>> Hi Paul,
>> 
>> After merging the rcu tree, today's linux-next build (bfin
>> BF526-EZBRD_defconfig and several other bfin configs) failed like this:
>> 
>> In file included from include/linux/srcu.h:60:0,
>>  from include/linux/notifier.h:15,
>>  from include/linux/memory_hotplug.h:6,
>>  from include/linux/mmzone.h:777,
>>  from include/linux/gfp.h:5,
>>  from include/linux/kmod.h:22,
>>  from include/linux/module.h:13,
>>  from include/linux/moduleloader.h:5,
>>  from arch/blackfin/kernel/module.c:9:
>> include/linux/srcutiny.h: In function 'srcu_torture_stats_print':
>> include/linux/srcutiny.h:96:2: error: 'mod' undeclared (first use in this 
>> function)
>> 
>> Caused by commit
>> 
>>   54ffb22bd841 ("rcutorture: Move SRCU status printing to SRCU 
>> implementations")
>
> And of course this is nothing but a printk().

Ah but it's not, it's a pr_alert():

+   pr_alert("%s%s Tiny SRCU per-CPU(idx=%d): (%hd,%hd)\n",
+tt, tf, idx,
+READ_ONCE(sp->srcu_lock_nesting[!idx]),
+READ_ONCE(sp->srcu_lock_nesting[idx]));

Where pr_alert() is:

#define pr_alert(fmt, ...) \
printk(KERN_ALERT pr_fmt(fmt), ##__VA_ARGS__)


So that's where pr_fmt() is coming into it.

And if any code in module.c called srcu_torture_stats_print(), it would
actually claim to come from "module: ", which would be confusing.

I don't think you use pr_fmt() in the RCU code, so you could skip using
pr_alert() and just use printk(KERN_ALERT "...") and get the same
result, without any interactions with pr_fmt().

cheers


Re: [PATCH 1/4] thermal: uniphier: add UniPhier thermal driver

2017-05-29 Thread Kunihiko Hayashi
On Tue, 30 May 2017 08:51:31 +0530 Keerthy  wrote:

> 
> 
> On Tuesday 30 May 2017 08:17 AM, Kunihiko Hayashi wrote:
> > Hi Keerthy,
> > Thank you for your comment.
> > 
> > On Mon, 29 May 2017 15:53:39 +0530 Keerthy  wrote:
> >>
> >>
> >> On Monday 29 May 2017 02:45 PM, Kunihiko Hayashi wrote:
> >>> Add a thermal driver for on-chip PVT (Process, Voltage and Temperature)
> >>> monitoring unit implemented on UniPhier SoCs. This driver supports
> >>> temperature monitoring and alert function.
> >>
> >> The Documentation in patch 2/4 should be squashed into this patch.
> > 
> > According to the following guide,
> > http://elixir.free-electrons.com/linux/latest/source/Documentation/devicetree/bindings/submitting-patches.txt
> > 
> >   1) The Documentation/ portion of the patch should be a separate patch.
> > 
> > I think it should be separated, how about that?
> 
> Then you should reorder the series as we get the warning with checkpatch:
> 
> WARNING: DT compatible string "socionext,uniphier-pxs2-thermal" appears
> un-documented -- check ./Documentation/devicetree/bindings/
> #488: FILE: drivers/thermal/uniphier_thermal.c:367:
> + .compatible = "socionext,uniphier-pxs2-thermal",
> 
> WARNING: DT compatible string "socionext,uniphier-ld20-thermal" appears
> un-documented -- check ./Documentation/devicetree/bindings/
> #492: FILE: drivers/thermal/uniphier_thermal.c:371:
> + .compatible = "socionext,uniphier-ld20-thermal",
> 
> When i apply the first patch.

OK. I also confirmed the warning messages. I'll reorder the patches 1/4 and 2/4.

Best Regards,
Kunihiko Hayashi




Re: [PATCH 1/4] thermal: uniphier: add UniPhier thermal driver

2017-05-29 Thread Kunihiko Hayashi
On Tue, 30 May 2017 08:51:31 +0530 Keerthy  wrote:

> 
> 
> On Tuesday 30 May 2017 08:17 AM, Kunihiko Hayashi wrote:
> > Hi Keerthy,
> > Thank you for your comment.
> > 
> > On Mon, 29 May 2017 15:53:39 +0530 Keerthy  wrote:
> >>
> >>
> >> On Monday 29 May 2017 02:45 PM, Kunihiko Hayashi wrote:
> >>> Add a thermal driver for on-chip PVT (Process, Voltage and Temperature)
> >>> monitoring unit implemented on UniPhier SoCs. This driver supports
> >>> temperature monitoring and alert function.
> >>
> >> The Documentation in patch 2/4 should be squashed into this patch.
> > 
> > According to the following guide,
> > http://elixir.free-electrons.com/linux/latest/source/Documentation/devicetree/bindings/submitting-patches.txt
> > 
> >   1) The Documentation/ portion of the patch should be a separate patch.
> > 
> > I think it should be separated, how about that?
> 
> Then you should reorder the series as we get the warning with checkpatch:
> 
> WARNING: DT compatible string "socionext,uniphier-pxs2-thermal" appears
> un-documented -- check ./Documentation/devicetree/bindings/
> #488: FILE: drivers/thermal/uniphier_thermal.c:367:
> + .compatible = "socionext,uniphier-pxs2-thermal",
> 
> WARNING: DT compatible string "socionext,uniphier-ld20-thermal" appears
> un-documented -- check ./Documentation/devicetree/bindings/
> #492: FILE: drivers/thermal/uniphier_thermal.c:371:
> + .compatible = "socionext,uniphier-ld20-thermal",
> 
> When i apply the first patch.

OK. I also confirmed the warning messages. I'll reorder the patches 1/4 and 2/4.

Best Regards,
Kunihiko Hayashi




linux-next: build warning after merge of the gpio tree

2017-05-29 Thread Stephen Rothwell
Hi Linus,

After merging the gpio tree, today's linux-next build (arm
multi_v7_defconfig) produced this warning:

drivers/gpio/gpiolib.c: In function 'gpiochip_irqchip_init_valid_mask':
drivers/gpio/gpiolib.c:1474:6: warning: unused variable 'i' [-Wunused-variable]
  int i;
  ^

Introduced by commit

  923a654c186c ("gpiolib: Re-use bitmap_fill() instead of open coded loop")

-- 
Cheers,
Stephen Rothwell


linux-next: build warning after merge of the gpio tree

2017-05-29 Thread Stephen Rothwell
Hi Linus,

After merging the gpio tree, today's linux-next build (arm
multi_v7_defconfig) produced this warning:

drivers/gpio/gpiolib.c: In function 'gpiochip_irqchip_init_valid_mask':
drivers/gpio/gpiolib.c:1474:6: warning: unused variable 'i' [-Wunused-variable]
  int i;
  ^

Introduced by commit

  923a654c186c ("gpiolib: Re-use bitmap_fill() instead of open coded loop")

-- 
Cheers,
Stephen Rothwell


Re: [RFC PATCH 0/4] PM / Domains: Add support for explicit control of PM domains

2017-05-29 Thread Rajendra Nayak
[]..

>>> I was proposing to have such a lower-layer by splitting the existing
>>> genpd framework so the drivers would have the option of calling the
>>> lower-level power control functions to look-up pm-domains and control
>>> them directly from their rpm callbacks (if they need to). Same as we do
>>> for clocks. This way you would not need to mess with the genpd ->start()
>>> callback and leave it to the driver to handle itself as it knows what
>>> needs to be done. This assumes that the device is never bound to the
>>> pm-domain by the genpd core.
>>
>> Yes, agree! To me this is the only solution what would really work.
> 
> I agree! :-)
> 
>> Perhaps Rafael can confirm that he is fine with a solution like this?
> 
> Yes and Rafael, please can you also elaborate on what you meant by
> "allow genpd to use either a list of power resources or the on/off
> callbacks provided by itself to cover different use cases"?
> 
> I would like to understand exactly what you meant by allowing genpd to
> use a list of power resources (ie. how you envisioned we could achieve
> this).

While thinking through the problem of devices associated with multiple Power
domains (or power resources) and controlling them individually (or together)
I was wondering if something like a PM domain governor (with PM resource 
level constraints) could help.

So with just one set of PM domain callbacks, its quite easy to control multiple 
power
resources, if they need to be *all* turned on/off together, using something 
similar to
what Jon proposed in his RFC [1]

However, there could be instances where in we might need to control them 
individually
and in such cases we could hook up a PM domain governor which decides if an 
individual
PM resource can be turned on or off while the device is runtime 
suspended/resumed.
We can expose some PM resource level QoS APIs which the drivers can use to 
express their
needs, which the PM domain governor then takes into account during the decision 
making.

if this seems worth pursuing further, I can post some RFCs on these lines and
get the discussion going.

thanks,
Rajendra

[1] https://lkml.org/lkml/2016/9/20/173

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation


Re: [RFC PATCH 0/4] PM / Domains: Add support for explicit control of PM domains

2017-05-29 Thread Rajendra Nayak
[]..

>>> I was proposing to have such a lower-layer by splitting the existing
>>> genpd framework so the drivers would have the option of calling the
>>> lower-level power control functions to look-up pm-domains and control
>>> them directly from their rpm callbacks (if they need to). Same as we do
>>> for clocks. This way you would not need to mess with the genpd ->start()
>>> callback and leave it to the driver to handle itself as it knows what
>>> needs to be done. This assumes that the device is never bound to the
>>> pm-domain by the genpd core.
>>
>> Yes, agree! To me this is the only solution what would really work.
> 
> I agree! :-)
> 
>> Perhaps Rafael can confirm that he is fine with a solution like this?
> 
> Yes and Rafael, please can you also elaborate on what you meant by
> "allow genpd to use either a list of power resources or the on/off
> callbacks provided by itself to cover different use cases"?
> 
> I would like to understand exactly what you meant by allowing genpd to
> use a list of power resources (ie. how you envisioned we could achieve
> this).

While thinking through the problem of devices associated with multiple Power
domains (or power resources) and controlling them individually (or together)
I was wondering if something like a PM domain governor (with PM resource 
level constraints) could help.

So with just one set of PM domain callbacks, its quite easy to control multiple 
power
resources, if they need to be *all* turned on/off together, using something 
similar to
what Jon proposed in his RFC [1]

However, there could be instances where in we might need to control them 
individually
and in such cases we could hook up a PM domain governor which decides if an 
individual
PM resource can be turned on or off while the device is runtime 
suspended/resumed.
We can expose some PM resource level QoS APIs which the drivers can use to 
express their
needs, which the PM domain governor then takes into account during the decision 
making.

if this seems worth pursuing further, I can post some RFCs on these lines and
get the discussion going.

thanks,
Rajendra

[1] https://lkml.org/lkml/2016/9/20/173

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation


Re: [Linux-ima-devel] [PATCH v2 4/5] keys, trusted: modify arguments of tpm_pcr_extend()

2017-05-29 Thread Mimi Zohar
On Fri, 2017-05-05 at 16:21 +0200, Roberto Sassu wrote:
> pcrlock() has been modified to pass the correct arguments
> to tpm_pcr_extend(): the pointer of a tpm2_digest structure containing
> a random value generated by tpm_get_random() and the size of the array (1).

If the number of arguments is wrong, that means the patch that
introduced the change is not bi-sect safe.  (This comment is
applicable to patch 5/5 too.)

Mimi

> Signed-off-by: Roberto Sassu 
> ---
>  security/keys/trusted.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/security/keys/trusted.c b/security/keys/trusted.c
> index 2ae31c5..3eb89e6 100644
> --- a/security/keys/trusted.c
> +++ b/security/keys/trusted.c
> @@ -377,15 +377,15 @@ static int trusted_tpm_send(const u32 chip_num, 
> unsigned char *cmd,
>   */
>  static int pcrlock(const int pcrnum)
>  {
> - unsigned char hash[SHA1_DIGEST_SIZE];
> + struct tpm2_digest digestarg = {.alg_id = TPM2_ALG_SHA1};
>   int ret;
> 
>   if (!capable(CAP_SYS_ADMIN))
>   return -EPERM;
> - ret = tpm_get_random(TPM_ANY_NUM, hash, SHA1_DIGEST_SIZE);
> + ret = tpm_get_random(TPM_ANY_NUM, digestarg.digest, SHA1_DIGEST_SIZE);
>   if (ret != SHA1_DIGEST_SIZE)
>   return ret;
> - return tpm_pcr_extend(TPM_ANY_NUM, pcrnum, hash) ? -EINVAL : 0;
> + return tpm_pcr_extend(TPM_ANY_NUM, pcrnum, 1, ) ? -EINVAL : 0;
>  }
> 
>  /*



Re: [Linux-ima-devel] [PATCH v2 4/5] keys, trusted: modify arguments of tpm_pcr_extend()

2017-05-29 Thread Mimi Zohar
On Fri, 2017-05-05 at 16:21 +0200, Roberto Sassu wrote:
> pcrlock() has been modified to pass the correct arguments
> to tpm_pcr_extend(): the pointer of a tpm2_digest structure containing
> a random value generated by tpm_get_random() and the size of the array (1).

If the number of arguments is wrong, that means the patch that
introduced the change is not bi-sect safe.  (This comment is
applicable to patch 5/5 too.)

Mimi

> Signed-off-by: Roberto Sassu 
> ---
>  security/keys/trusted.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/security/keys/trusted.c b/security/keys/trusted.c
> index 2ae31c5..3eb89e6 100644
> --- a/security/keys/trusted.c
> +++ b/security/keys/trusted.c
> @@ -377,15 +377,15 @@ static int trusted_tpm_send(const u32 chip_num, 
> unsigned char *cmd,
>   */
>  static int pcrlock(const int pcrnum)
>  {
> - unsigned char hash[SHA1_DIGEST_SIZE];
> + struct tpm2_digest digestarg = {.alg_id = TPM2_ALG_SHA1};
>   int ret;
> 
>   if (!capable(CAP_SYS_ADMIN))
>   return -EPERM;
> - ret = tpm_get_random(TPM_ANY_NUM, hash, SHA1_DIGEST_SIZE);
> + ret = tpm_get_random(TPM_ANY_NUM, digestarg.digest, SHA1_DIGEST_SIZE);
>   if (ret != SHA1_DIGEST_SIZE)
>   return ret;
> - return tpm_pcr_extend(TPM_ANY_NUM, pcrnum, hash) ? -EINVAL : 0;
> + return tpm_pcr_extend(TPM_ANY_NUM, pcrnum, 1, ) ? -EINVAL : 0;
>  }
> 
>  /*



Re: [PATCH net-next] net: dsa: b53: remove unused dev argument

2017-05-29 Thread David Miller
From: Florian Fainelli 
Date: Fri, 26 May 2017 15:50:51 -0700

> On 05/26/2017 03:07 PM, Vivien Didelot wrote:
>> The port net device passed to b53_fdb_copy is not used. Remove it.
>> 
>> Signed-off-by: Vivien Didelot 
> 
> Reviewed-by: Florian Fainelli 

Applied.


Re: [PATCH net-next] net: dsa: b53: remove unused dev argument

2017-05-29 Thread David Miller
From: Florian Fainelli 
Date: Fri, 26 May 2017 15:50:51 -0700

> On 05/26/2017 03:07 PM, Vivien Didelot wrote:
>> The port net device passed to b53_fdb_copy is not used. Remove it.
>> 
>> Signed-off-by: Vivien Didelot 
> 
> Reviewed-by: Florian Fainelli 

Applied.


Re: [PATCH net-next] net: dsa: remove dsa_port_is_bridged

2017-05-29 Thread David Miller
From: Vivien Didelot 
Date: Fri, 26 May 2017 18:12:42 -0400

> The helper is only used once and makes the code more complicated that it
> should. Remove it and reorganize the variables so that it fits on 80
> columns.
> 
> Signed-off-by: Vivien Didelot 

Applied, thanks.


Re: [PATCH net-next] net: dsa: remove dsa_port_is_bridged

2017-05-29 Thread David Miller
From: Vivien Didelot 
Date: Fri, 26 May 2017 18:12:42 -0400

> The helper is only used once and makes the code more complicated that it
> should. Remove it and reorganize the variables so that it fits on 80
> columns.
> 
> Signed-off-by: Vivien Didelot 

Applied, thanks.


Re: [PATCH 0/2] PCI: Workaround for bus reset on Cavium cn8xxx root ports

2017-05-29 Thread Jon Masters
Following up on this thread...

On 05/23/2017 06:15 PM, Alex Williamson wrote:
> On Tue, 23 May 2017 14:22:01 -0700
> David Daney  wrote:
> 
>> On 05/23/2017 02:04 PM, Alex Williamson wrote:
>>> On Tue, 23 May 2017 15:47:50 -0500
>>> Bjorn Helgaas  wrote:
>>>   
 On Mon, May 15, 2017 at 05:17:34PM -0700, David Daney wrote:  
> With the recent improvements in arm64 and vfio-pci, we are seeing
> failures like this (on cn8890 based systems):
>
> [  235.622361] Unhandled fault: synchronous external abort (0x96000210) 
> at 0xfc00c1000100
> [  235.630625] Internal error: : 96000210 [#1] PREEMPT SMP
> .
> .
> .
> [  236.208820] [] pci_generic_config_read+0x38/0x9c
> [  236.214992] [] thunder_pem_config_read+0x54/0x1e8
> [  236.221250] [] pci_bus_read_config_dword+0x74/0xa0
> [  236.227596] [] 
> pci_find_next_ext_capability.part.15+0x40/0xb8
> [  236.234896] [] pci_find_ext_capability+0x20/0x30
> [  236.241068] [] pci_restore_vc_state+0x34/0x88
> [  236.246979] [] pci_restore_state.part.37+0x2c/0x1fc
> [  236.253410] [] pci_dev_restore+0x4c/0x50
> [  236.258887] [] pci_bus_restore+0x24/0x4c
> [  236.264362] [] pci_try_reset_bus+0x7c/0xa0
> [  236.270021] [] vfio_pci_ioctl+0xc34/0xc3c [vfio_pci]
> [  236.276547] [] vfio_device_fops_unl_ioctl+0x20/0x30 
> [vfio]
> [  236.283587] [] do_vfs_ioctl+0xac/0x744
> [  236.288890] [] SyS_ioctl+0x84/0x98
> [  236.293846] [] __sys_trace_return+0x0/0x4
>
> These are caused by the inability of the PCIe root port and Intel
> e1000e to sucessfully do a bus reset.

Right now, we have a whole bunch of systems in our labs and across the
industry where people are testing VFIO on Cavium ThunderX platforms. It
is amazing how many people have e1000 cards lying around (this is even
more popular on ARM servers because things like Tianocore usually "just
work" with an e1000 card installed...). I know I have many more e1000s
than I do x86 systems. So we need a solution to not crash on use.

> The proposed fix is to not do a bus reset on these systems.

It's not the best solution, but it's a solution, and there are plenty of
other quirks. I would like to see us figure out a sensible path to
perhaps take this patch now and continue researching to determine
whether there's an even better option later.

The reasons I say this are:

1). It will take some time for various teams with protocol analyzers to
determine which end is not fully spec compliant, while in the interim
everyone has to carry this patch or some other nasty hack anyway.

2). I have an even worse patch in my test kernels (just disable reset
for every device, which is nasty) and I'm sure I'm not alone. It would
be better (IMO) to confine the lack of reset to a subset of devices.



> My hope would be that such analysis would help us understand what's
> really happening and perhaps present a less drastic workaround.  Is the
> point at which we crash simply the first read from the device?  Is the
> link state shown as stable at this point?  Is there any chance that we
> have a timing issue and an additional delay could resolve it?  Is there
> some manipulation of the link state before or after the bus reset that
> produces different results?  If such options have been exhausted, then
> I guess I have no objection, or at least no better solution, and I'll
> keep an eye out for any fallout.  Thanks,

My understanding is that various teams have already spent man months of
time with protocol analyzer equipment trying to figure out who is at
"fault". I have no reason not to believe that they don't intend to
continue that work, since they certainly want to ensure future root port
implementations are event more robust in the presence of both fully
compliant, and also somewhat compliant endpoints. If David affirms that
the work will continue, can we take this for now?

Jon.



Re: [PATCH 0/2] PCI: Workaround for bus reset on Cavium cn8xxx root ports

2017-05-29 Thread Jon Masters
Following up on this thread...

On 05/23/2017 06:15 PM, Alex Williamson wrote:
> On Tue, 23 May 2017 14:22:01 -0700
> David Daney  wrote:
> 
>> On 05/23/2017 02:04 PM, Alex Williamson wrote:
>>> On Tue, 23 May 2017 15:47:50 -0500
>>> Bjorn Helgaas  wrote:
>>>   
 On Mon, May 15, 2017 at 05:17:34PM -0700, David Daney wrote:  
> With the recent improvements in arm64 and vfio-pci, we are seeing
> failures like this (on cn8890 based systems):
>
> [  235.622361] Unhandled fault: synchronous external abort (0x96000210) 
> at 0xfc00c1000100
> [  235.630625] Internal error: : 96000210 [#1] PREEMPT SMP
> .
> .
> .
> [  236.208820] [] pci_generic_config_read+0x38/0x9c
> [  236.214992] [] thunder_pem_config_read+0x54/0x1e8
> [  236.221250] [] pci_bus_read_config_dword+0x74/0xa0
> [  236.227596] [] 
> pci_find_next_ext_capability.part.15+0x40/0xb8
> [  236.234896] [] pci_find_ext_capability+0x20/0x30
> [  236.241068] [] pci_restore_vc_state+0x34/0x88
> [  236.246979] [] pci_restore_state.part.37+0x2c/0x1fc
> [  236.253410] [] pci_dev_restore+0x4c/0x50
> [  236.258887] [] pci_bus_restore+0x24/0x4c
> [  236.264362] [] pci_try_reset_bus+0x7c/0xa0
> [  236.270021] [] vfio_pci_ioctl+0xc34/0xc3c [vfio_pci]
> [  236.276547] [] vfio_device_fops_unl_ioctl+0x20/0x30 
> [vfio]
> [  236.283587] [] do_vfs_ioctl+0xac/0x744
> [  236.288890] [] SyS_ioctl+0x84/0x98
> [  236.293846] [] __sys_trace_return+0x0/0x4
>
> These are caused by the inability of the PCIe root port and Intel
> e1000e to sucessfully do a bus reset.

Right now, we have a whole bunch of systems in our labs and across the
industry where people are testing VFIO on Cavium ThunderX platforms. It
is amazing how many people have e1000 cards lying around (this is even
more popular on ARM servers because things like Tianocore usually "just
work" with an e1000 card installed...). I know I have many more e1000s
than I do x86 systems. So we need a solution to not crash on use.

> The proposed fix is to not do a bus reset on these systems.

It's not the best solution, but it's a solution, and there are plenty of
other quirks. I would like to see us figure out a sensible path to
perhaps take this patch now and continue researching to determine
whether there's an even better option later.

The reasons I say this are:

1). It will take some time for various teams with protocol analyzers to
determine which end is not fully spec compliant, while in the interim
everyone has to carry this patch or some other nasty hack anyway.

2). I have an even worse patch in my test kernels (just disable reset
for every device, which is nasty) and I'm sure I'm not alone. It would
be better (IMO) to confine the lack of reset to a subset of devices.



> My hope would be that such analysis would help us understand what's
> really happening and perhaps present a less drastic workaround.  Is the
> point at which we crash simply the first read from the device?  Is the
> link state shown as stable at this point?  Is there any chance that we
> have a timing issue and an additional delay could resolve it?  Is there
> some manipulation of the link state before or after the bus reset that
> produces different results?  If such options have been exhausted, then
> I guess I have no objection, or at least no better solution, and I'll
> keep an eye out for any fallout.  Thanks,

My understanding is that various teams have already spent man months of
time with protocol analyzer equipment trying to figure out who is at
"fault". I have no reason not to believe that they don't intend to
continue that work, since they certainly want to ensure future root port
implementations are event more robust in the presence of both fully
compliant, and also somewhat compliant endpoints. If David affirms that
the work will continue, can we take this for now?

Jon.



Re: [Linux-ima-devel] [PATCH v2 3/5] tpm: pass multiple digests to tpm_pcr_extend()

2017-05-29 Thread Mimi Zohar
On Fri, 2017-05-05 at 16:21 +0200, Roberto Sassu wrote:
> The tpm_pcr_extend() definition has been modified to take an array of
> tpm2_digest structures, and the size of the array as arguments.
> 
> The function now checks if callers provided a digests for each active
> PCR bank (or a SHA1 digest for TPM 1.2), to follow the recomendation from
> the TCG specifications. See commit c1f92b4b04ad ("tpm: enhance TPM 2.0
> PCR extend to support multiple banks"). All banks should be extended
> because unused banks could be used by an attacker to hide the true
> integrity status of the platform.
> 
> The only allowed exception to the rule above is to pass a SHA1 digest.
> It has been introduced to maintain compatibility with applications that
> expect to interact with a TPM 1.2, and provide only a SHA1 digest.
> In this case, the behavior of tpm_pcr_extend() is unchanged and
> remaining PCR banks are extended with that digest, padded with zeros.
> 
> Signed-off-by: Roberto Sassu 
> ---
> v2
> 
> - tpm_pcr_extend() arguments checked by tpm_pcr_check_input()
> - modified parameters of tpm_pcr_extend()
> 
>  drivers/char/tpm/tpm-interface.c | 76 
> +---
>  drivers/char/tpm/tpm.h   |  6 
>  include/linux/tpm.h  | 11 --
>  3 files changed, 80 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm-interface.c 
> b/drivers/char/tpm/tpm-interface.c
> index aac703e..4b08b02 100644
> --- a/drivers/char/tpm/tpm-interface.c
> +++ b/drivers/char/tpm/tpm-interface.c
> @@ -867,6 +867,55 @@ static int tpm1_pcr_extend(struct tpm_chip *chip, int 
> pcr_idx, const u8 *hash,
>  }
> 
>  /**
> + * tpm_pcr_check_input - check digests argument
> + *
> + * Return values:
> + *   1: input correct
> + *   0: fill digests with SHA1 digest padded with zeros
> + * -EINVAL: input incorrect
> + */
> +static int tpm_pcr_check_input(struct tpm_chip *chip, int count,
> +struct tpm2_digest *digests)
> +{
> + bool sha1_only;
> + int found = 0, not_found = 0;
> + int i, j;
> +
> + if (count <= 0 || digests == NULL)
> + return -EINVAL;
> +
> + sha1_only = (count == 1 && digests[0].alg_id == TPM2_ALG_SHA1);
> +
> + if (!(chip->flags & TPM_CHIP_FLAG_TPM2))
> + return sha1_only ? 1 : -EINVAL;
> +
> + if (sha1_only)
> + return 0;
> +
> + for (i = 0; i < ARRAY_SIZE(chip->active_banks) &&
> +  chip->active_banks[i] != TPM2_ALG_ERROR; i++) {
> + for (j = 0; j < count; j++) {
> + if (digests[j].alg_id == chip->active_banks[i]) {
> + found++;
> + break;
> + }
> + }
> +
> + if (j == count) {
> + dev_dbg(>dev, "%s: missing algorithm 0x%X\n",
> + __func__, chip->active_banks[i]);
> + not_found++;
> + }
> + }
> +
> + if (not_found == 0 && found != count)
> + dev_dbg(>dev,
> + "%s: duplicate or unsupported algorithm\n", __func__);
> +
> + return (not_found == 0 && found == count) ? 1 : -EINVAL;
> +}
> +
> +/**
>   * tpm_pcr_extend - extend pcr value with hash
>   * @chip_num:tpm idx # or AN&
>   * @pcr_idx: pcr idx to extend
> @@ -876,29 +925,46 @@ static int tpm1_pcr_extend(struct tpm_chip *chip, int 
> pcr_idx, const u8 *hash,
>   * isn't, protect against the chip disappearing, by incrementing
>   * the module usage count.
>   */
> -int tpm_pcr_extend(u32 chip_num, int pcr_idx, const u8 *hash)
> +int tpm_pcr_extend(u32 chip_num, int pcr_idx, int count,
> +struct tpm2_digest *digests)
>  {
>   int rc;
>   struct tpm_chip *chip;
>   struct tpm2_digest digest_list[ARRAY_SIZE(chip->active_banks)];
> - u32 count = 0;
> + struct tpm2_digest *digests_ptr = digests;
> + u32 filled_count = 0;
> + u8 *hash;
>   int i;
> 
>   chip = tpm_chip_find_get(chip_num);
>   if (chip == NULL)
>   return -ENODEV;
> 
> - if (chip->flags & TPM_CHIP_FLAG_TPM2) {
> + rc = tpm_pcr_check_input(chip, count, digests);
> + if (rc < 0) {
> + dev_dbg(>dev, "%s: invalid arguments\n", __func__);
> + tpm_put_ops(chip);

This rejects the TPM extend, if ANY of the algorithms are unknown.
Suppose that the standards were updated, TPM vendors add support for
the new algorithm, but the kernel has not been updated to reflect the
new algorithms supported.  As the measurement hash already been added
to the IMA measurement list, verifying the measurement list against a
TPM quote will fail, not just for the unknown algorithm, but for all
algorithms.  Something is very broken with this approach.

Mimi

> + return rc;
> + }
> +
> + hash = digests[0].digest;
> +
> + if (!rc) {
>   memset(digest_list, 0, 

Re: [Linux-ima-devel] [PATCH v2 3/5] tpm: pass multiple digests to tpm_pcr_extend()

2017-05-29 Thread Mimi Zohar
On Fri, 2017-05-05 at 16:21 +0200, Roberto Sassu wrote:
> The tpm_pcr_extend() definition has been modified to take an array of
> tpm2_digest structures, and the size of the array as arguments.
> 
> The function now checks if callers provided a digests for each active
> PCR bank (or a SHA1 digest for TPM 1.2), to follow the recomendation from
> the TCG specifications. See commit c1f92b4b04ad ("tpm: enhance TPM 2.0
> PCR extend to support multiple banks"). All banks should be extended
> because unused banks could be used by an attacker to hide the true
> integrity status of the platform.
> 
> The only allowed exception to the rule above is to pass a SHA1 digest.
> It has been introduced to maintain compatibility with applications that
> expect to interact with a TPM 1.2, and provide only a SHA1 digest.
> In this case, the behavior of tpm_pcr_extend() is unchanged and
> remaining PCR banks are extended with that digest, padded with zeros.
> 
> Signed-off-by: Roberto Sassu 
> ---
> v2
> 
> - tpm_pcr_extend() arguments checked by tpm_pcr_check_input()
> - modified parameters of tpm_pcr_extend()
> 
>  drivers/char/tpm/tpm-interface.c | 76 
> +---
>  drivers/char/tpm/tpm.h   |  6 
>  include/linux/tpm.h  | 11 --
>  3 files changed, 80 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm-interface.c 
> b/drivers/char/tpm/tpm-interface.c
> index aac703e..4b08b02 100644
> --- a/drivers/char/tpm/tpm-interface.c
> +++ b/drivers/char/tpm/tpm-interface.c
> @@ -867,6 +867,55 @@ static int tpm1_pcr_extend(struct tpm_chip *chip, int 
> pcr_idx, const u8 *hash,
>  }
> 
>  /**
> + * tpm_pcr_check_input - check digests argument
> + *
> + * Return values:
> + *   1: input correct
> + *   0: fill digests with SHA1 digest padded with zeros
> + * -EINVAL: input incorrect
> + */
> +static int tpm_pcr_check_input(struct tpm_chip *chip, int count,
> +struct tpm2_digest *digests)
> +{
> + bool sha1_only;
> + int found = 0, not_found = 0;
> + int i, j;
> +
> + if (count <= 0 || digests == NULL)
> + return -EINVAL;
> +
> + sha1_only = (count == 1 && digests[0].alg_id == TPM2_ALG_SHA1);
> +
> + if (!(chip->flags & TPM_CHIP_FLAG_TPM2))
> + return sha1_only ? 1 : -EINVAL;
> +
> + if (sha1_only)
> + return 0;
> +
> + for (i = 0; i < ARRAY_SIZE(chip->active_banks) &&
> +  chip->active_banks[i] != TPM2_ALG_ERROR; i++) {
> + for (j = 0; j < count; j++) {
> + if (digests[j].alg_id == chip->active_banks[i]) {
> + found++;
> + break;
> + }
> + }
> +
> + if (j == count) {
> + dev_dbg(>dev, "%s: missing algorithm 0x%X\n",
> + __func__, chip->active_banks[i]);
> + not_found++;
> + }
> + }
> +
> + if (not_found == 0 && found != count)
> + dev_dbg(>dev,
> + "%s: duplicate or unsupported algorithm\n", __func__);
> +
> + return (not_found == 0 && found == count) ? 1 : -EINVAL;
> +}
> +
> +/**
>   * tpm_pcr_extend - extend pcr value with hash
>   * @chip_num:tpm idx # or AN&
>   * @pcr_idx: pcr idx to extend
> @@ -876,29 +925,46 @@ static int tpm1_pcr_extend(struct tpm_chip *chip, int 
> pcr_idx, const u8 *hash,
>   * isn't, protect against the chip disappearing, by incrementing
>   * the module usage count.
>   */
> -int tpm_pcr_extend(u32 chip_num, int pcr_idx, const u8 *hash)
> +int tpm_pcr_extend(u32 chip_num, int pcr_idx, int count,
> +struct tpm2_digest *digests)
>  {
>   int rc;
>   struct tpm_chip *chip;
>   struct tpm2_digest digest_list[ARRAY_SIZE(chip->active_banks)];
> - u32 count = 0;
> + struct tpm2_digest *digests_ptr = digests;
> + u32 filled_count = 0;
> + u8 *hash;
>   int i;
> 
>   chip = tpm_chip_find_get(chip_num);
>   if (chip == NULL)
>   return -ENODEV;
> 
> - if (chip->flags & TPM_CHIP_FLAG_TPM2) {
> + rc = tpm_pcr_check_input(chip, count, digests);
> + if (rc < 0) {
> + dev_dbg(>dev, "%s: invalid arguments\n", __func__);
> + tpm_put_ops(chip);

This rejects the TPM extend, if ANY of the algorithms are unknown.
Suppose that the standards were updated, TPM vendors add support for
the new algorithm, but the kernel has not been updated to reflect the
new algorithms supported.  As the measurement hash already been added
to the IMA measurement list, verifying the measurement list against a
TPM quote will fail, not just for the unknown algorithm, but for all
algorithms.  Something is very broken with this approach.

Mimi

> + return rc;
> + }
> +
> + hash = digests[0].digest;
> +
> + if (!rc) {
>   memset(digest_list, 0, sizeof(digest_list));
> 
> 

Re: [PATCH 1/4] thermal: uniphier: add UniPhier thermal driver

2017-05-29 Thread Keerthy


On Tuesday 30 May 2017 08:17 AM, Kunihiko Hayashi wrote:
> Hi Keerthy,
> Thank you for your comment.
> 
> On Mon, 29 May 2017 15:53:39 +0530 Keerthy  wrote:
>>
>>
>> On Monday 29 May 2017 02:45 PM, Kunihiko Hayashi wrote:
>>> Add a thermal driver for on-chip PVT (Process, Voltage and Temperature)
>>> monitoring unit implemented on UniPhier SoCs. This driver supports
>>> temperature monitoring and alert function.
>>
>> The Documentation in patch 2/4 should be squashed into this patch.
> 
> According to the following guide,
> http://elixir.free-electrons.com/linux/latest/source/Documentation/devicetree/bindings/submitting-patches.txt
> 
>   1) The Documentation/ portion of the patch should be a separate patch.
> 
> I think it should be separated, how about that?

Then you should reorder the series as we get the warning with checkpatch:

WARNING: DT compatible string "socionext,uniphier-pxs2-thermal" appears
un-documented -- check ./Documentation/devicetree/bindings/
#488: FILE: drivers/thermal/uniphier_thermal.c:367:
+   .compatible = "socionext,uniphier-pxs2-thermal",

WARNING: DT compatible string "socionext,uniphier-ld20-thermal" appears
un-documented -- check ./Documentation/devicetree/bindings/
#492: FILE: drivers/thermal/uniphier_thermal.c:371:
+   .compatible = "socionext,uniphier-ld20-thermal",

When i apply the first patch.

> 
>>>
>>> Signed-off-by: Kunihiko Hayashi 
>>> ---
>>>  drivers/thermal/Kconfig|   8 +
>>>  drivers/thermal/Makefile   |   1 +
>>>  drivers/thermal/uniphier_thermal.c | 390 
>>> +
>>>  3 files changed, 399 insertions(+)
>>>  create mode 100644 drivers/thermal/uniphier_thermal.c
>>>
>>> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
>>> index 776b343..93ccf25 100644
>>> --- a/drivers/thermal/Kconfig
>>> +++ b/drivers/thermal/Kconfig
>>> @@ -453,4 +453,12 @@ config ZX2967_THERMAL
>>>   the primitive temperature sensor embedded in zx2967 SoCs.
>>>   This sensor generates the real time die temperature.
>>>  
>>> +config UNIPHIER_THERMAL
>>> +   tristate "Socionext UniPhier thermal driver"
>>> +   depends on ARCH_UNIPHIER || COMPILE_TEST
>>> +   help
>>> + Enable this to support thermal reporting on UniPhier SoC.
>>> + This driver supports CPU thermal zone to reports the temperture
>>
>> /s/temperture/temperature
>>
>> Or consider re-wording to
>>
>> "Enable this to plug in UniPhier on-chip PVT thermal driver into Linux
>> thermal framework. The driver supports CPU thermal zone temperature
>> reporting and a couple of trip points."
> 
> Thanks for finding a typo and your re-wording. I'll fix it.
> 
>>> + and trip points.
>>> +
>>>  endif
>>> diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
>>> index 7adae20..05b6e7c 100644
>>> --- a/drivers/thermal/Makefile
>>> +++ b/drivers/thermal/Makefile
>>> @@ -58,3 +58,4 @@ obj-$(CONFIG_HISI_THERMAL) += hisi_thermal.o
>>>  obj-$(CONFIG_MTK_THERMAL)  += mtk_thermal.o
>>>  obj-$(CONFIG_GENERIC_ADC_THERMAL)  += thermal-generic-adc.o
>>>  obj-$(CONFIG_ZX2967_THERMAL)   += zx2967_thermal.o
>>> +obj-$(CONFIG_UNIPHIER_THERMAL) += uniphier_thermal.o
>>> diff --git a/drivers/thermal/uniphier_thermal.c 
>>> b/drivers/thermal/uniphier_thermal.c
>>> new file mode 100644
>>> index 000..ae7e5ce
>>> --- /dev/null
>>> +++ b/drivers/thermal/uniphier_thermal.c
>>> @@ -0,0 +1,390 @@
>>> +/**
>>> + * uniphier_thermal.c - Socionext UniPhier thermal driver
>>> + *
>>> + * Copyright 2014 Panasonic Corporation
>>> + * Copyright 2016 Socionext Inc.
>>
>> Update this please.
> 
> I see. I'll update it.
> 
>>> + * All rights reserved.
>>> + *
>>> + * Author:
>>> + * Kunihiko Hayashi 
>>> + *
>>> + * This program is free software: you can redistribute it and/or modify
>>> + * it under the terms of the GNU General Public License version 2  of
>>> + * the License as published by the Free Software Foundation.
>>> + *
>>> + * This program is distributed in the hope that it will be useful,
>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>>> + * GNU General Public License for more details.
>>> + */
>>> +
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +
>>> +#include "thermal_core.h"
>>> +
>>> +/*
>>> + * block registers
>>> + * addresses are the offset from .block_base
>>> + */
>>> +#define PVTCTLEN   0x
>>> +#define PVTCTLEN_ENBIT(0)
>>> +
>>> +#define PVTCTLMODE 0x0004
>>> +#define PVTCTLMODE_MASK0xf
>>> +#define PVTCTLMODE_TEMPMON 0x5
>>> +
>>> +#define EMONREPEAT 0x0040
>>> +#define EMONREPEAT_ENDLESS  

Re: [PATCH 1/4] thermal: uniphier: add UniPhier thermal driver

2017-05-29 Thread Keerthy


On Tuesday 30 May 2017 08:17 AM, Kunihiko Hayashi wrote:
> Hi Keerthy,
> Thank you for your comment.
> 
> On Mon, 29 May 2017 15:53:39 +0530 Keerthy  wrote:
>>
>>
>> On Monday 29 May 2017 02:45 PM, Kunihiko Hayashi wrote:
>>> Add a thermal driver for on-chip PVT (Process, Voltage and Temperature)
>>> monitoring unit implemented on UniPhier SoCs. This driver supports
>>> temperature monitoring and alert function.
>>
>> The Documentation in patch 2/4 should be squashed into this patch.
> 
> According to the following guide,
> http://elixir.free-electrons.com/linux/latest/source/Documentation/devicetree/bindings/submitting-patches.txt
> 
>   1) The Documentation/ portion of the patch should be a separate patch.
> 
> I think it should be separated, how about that?

Then you should reorder the series as we get the warning with checkpatch:

WARNING: DT compatible string "socionext,uniphier-pxs2-thermal" appears
un-documented -- check ./Documentation/devicetree/bindings/
#488: FILE: drivers/thermal/uniphier_thermal.c:367:
+   .compatible = "socionext,uniphier-pxs2-thermal",

WARNING: DT compatible string "socionext,uniphier-ld20-thermal" appears
un-documented -- check ./Documentation/devicetree/bindings/
#492: FILE: drivers/thermal/uniphier_thermal.c:371:
+   .compatible = "socionext,uniphier-ld20-thermal",

When i apply the first patch.

> 
>>>
>>> Signed-off-by: Kunihiko Hayashi 
>>> ---
>>>  drivers/thermal/Kconfig|   8 +
>>>  drivers/thermal/Makefile   |   1 +
>>>  drivers/thermal/uniphier_thermal.c | 390 
>>> +
>>>  3 files changed, 399 insertions(+)
>>>  create mode 100644 drivers/thermal/uniphier_thermal.c
>>>
>>> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
>>> index 776b343..93ccf25 100644
>>> --- a/drivers/thermal/Kconfig
>>> +++ b/drivers/thermal/Kconfig
>>> @@ -453,4 +453,12 @@ config ZX2967_THERMAL
>>>   the primitive temperature sensor embedded in zx2967 SoCs.
>>>   This sensor generates the real time die temperature.
>>>  
>>> +config UNIPHIER_THERMAL
>>> +   tristate "Socionext UniPhier thermal driver"
>>> +   depends on ARCH_UNIPHIER || COMPILE_TEST
>>> +   help
>>> + Enable this to support thermal reporting on UniPhier SoC.
>>> + This driver supports CPU thermal zone to reports the temperture
>>
>> /s/temperture/temperature
>>
>> Or consider re-wording to
>>
>> "Enable this to plug in UniPhier on-chip PVT thermal driver into Linux
>> thermal framework. The driver supports CPU thermal zone temperature
>> reporting and a couple of trip points."
> 
> Thanks for finding a typo and your re-wording. I'll fix it.
> 
>>> + and trip points.
>>> +
>>>  endif
>>> diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
>>> index 7adae20..05b6e7c 100644
>>> --- a/drivers/thermal/Makefile
>>> +++ b/drivers/thermal/Makefile
>>> @@ -58,3 +58,4 @@ obj-$(CONFIG_HISI_THERMAL) += hisi_thermal.o
>>>  obj-$(CONFIG_MTK_THERMAL)  += mtk_thermal.o
>>>  obj-$(CONFIG_GENERIC_ADC_THERMAL)  += thermal-generic-adc.o
>>>  obj-$(CONFIG_ZX2967_THERMAL)   += zx2967_thermal.o
>>> +obj-$(CONFIG_UNIPHIER_THERMAL) += uniphier_thermal.o
>>> diff --git a/drivers/thermal/uniphier_thermal.c 
>>> b/drivers/thermal/uniphier_thermal.c
>>> new file mode 100644
>>> index 000..ae7e5ce
>>> --- /dev/null
>>> +++ b/drivers/thermal/uniphier_thermal.c
>>> @@ -0,0 +1,390 @@
>>> +/**
>>> + * uniphier_thermal.c - Socionext UniPhier thermal driver
>>> + *
>>> + * Copyright 2014 Panasonic Corporation
>>> + * Copyright 2016 Socionext Inc.
>>
>> Update this please.
> 
> I see. I'll update it.
> 
>>> + * All rights reserved.
>>> + *
>>> + * Author:
>>> + * Kunihiko Hayashi 
>>> + *
>>> + * This program is free software: you can redistribute it and/or modify
>>> + * it under the terms of the GNU General Public License version 2  of
>>> + * the License as published by the Free Software Foundation.
>>> + *
>>> + * This program is distributed in the hope that it will be useful,
>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>>> + * GNU General Public License for more details.
>>> + */
>>> +
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +
>>> +#include "thermal_core.h"
>>> +
>>> +/*
>>> + * block registers
>>> + * addresses are the offset from .block_base
>>> + */
>>> +#define PVTCTLEN   0x
>>> +#define PVTCTLEN_ENBIT(0)
>>> +
>>> +#define PVTCTLMODE 0x0004
>>> +#define PVTCTLMODE_MASK0xf
>>> +#define PVTCTLMODE_TEMPMON 0x5
>>> +
>>> +#define EMONREPEAT 0x0040
>>> +#define EMONREPEAT_ENDLESS BIT(24)
>>> +#define EMONREPEAT_PERIOD  0xf
>>> +#define 

Re: Yes, people use FOLL_FORCE ;)

2017-05-29 Thread Linus Torvalds
On Mon, May 29, 2017 at 4:08 PM, Keno Fischer  wrote:
>
> As I said, personally we can patch our software and deal with this, but I 
> think
> a change like this deserves a bit wider discussion, so may I suggest a revert
> of this change for the time being? Maybe there can be a syslog warning such
> that people who use it will notice and have their say on the mailing list.

Oh, we'll just re-instate the kernel behavior, it was more an
optimistic "maybe nobody will notice" thing, and apparently people did
notice.

What I *would* appreciate would be that with the revert, we'd describe
the apps that use FOLL_FORCE, and what they do.

Also, if we can limit it to some simpler cases, that would be good. For example:

 (a) if the "punch through" behavior is only used to punch through
read-only mappings, that one thing. It's FOLL_FORCE + FOLL_WRITE that
used to be a nasty case.

 But it sounds like your JIT case actually uses it for writing -
but if you can write a small blurb about it, that would be nice.

 (b) it would probably be nice to limit FOLL_FORCE in general as much
as possible, so if your case is about writing to your very _own_
memory mapping, as opposed to writing to another process' memory,
maybe we can do something like

if (mm == current->mm)
flags |= FOLL_FORCE;

which at least avoids the whole "let's change the VM in odd ways for a
process that isn't even me".

So no, you should not need any workarounds, but it really would be
good to document what the uses are, and if you are ok with something
like the above that minimizes FOLL_FORCE usage, that would be good.

Mind sending me a patch with a comment or changelog like that?

  Linus


Re: Yes, people use FOLL_FORCE ;)

2017-05-29 Thread Linus Torvalds
On Mon, May 29, 2017 at 4:08 PM, Keno Fischer  wrote:
>
> As I said, personally we can patch our software and deal with this, but I 
> think
> a change like this deserves a bit wider discussion, so may I suggest a revert
> of this change for the time being? Maybe there can be a syslog warning such
> that people who use it will notice and have their say on the mailing list.

Oh, we'll just re-instate the kernel behavior, it was more an
optimistic "maybe nobody will notice" thing, and apparently people did
notice.

What I *would* appreciate would be that with the revert, we'd describe
the apps that use FOLL_FORCE, and what they do.

Also, if we can limit it to some simpler cases, that would be good. For example:

 (a) if the "punch through" behavior is only used to punch through
read-only mappings, that one thing. It's FOLL_FORCE + FOLL_WRITE that
used to be a nasty case.

 But it sounds like your JIT case actually uses it for writing -
but if you can write a small blurb about it, that would be nice.

 (b) it would probably be nice to limit FOLL_FORCE in general as much
as possible, so if your case is about writing to your very _own_
memory mapping, as opposed to writing to another process' memory,
maybe we can do something like

if (mm == current->mm)
flags |= FOLL_FORCE;

which at least avoids the whole "let's change the VM in odd ways for a
process that isn't even me".

So no, you should not need any workarounds, but it really would be
good to document what the uses are, and if you are ok with something
like the above that minimizes FOLL_FORCE usage, that would be good.

Mind sending me a patch with a comment or changelog like that?

  Linus


Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN

2017-05-29 Thread Matt Brown
On 5/29/17 10:46 PM, Casey Schaufler wrote:
> On 5/29/2017 7:00 PM, Matt Brown wrote:
>> Casey Schaufler,
>>
>> First I must start this off by saying I really appreciate your presentation 
>> on
>> LSMs that is up on youtube. I've got a LSM in the works and your talk has
>> helped me a bunch.
> 
> Thank you. Feedback (especially positive) is always appreciated.
> 
>>
>> On 5/29/17 8:27 PM, Casey Schaufler wrote:
>>> On 5/29/2017 4:51 PM, Boris Lukashev wrote:
 With all due respect sir, i believe your review falls short of the
 purpose of this effort - to harden the kernel against flaws in
 userspace. Comments along the line of "if  does it right
 then your patch is pointless" are not relevant to the context of
 securing kernel functions/interfaces. What userspace should do has
 little bearing on defensive measures actually implemented in the
 kernel - if we took the approach of "someone else is responsible for
 that" in military operations, the world would be a much darker and
 different place today. Those who have the luxury of standoff from the
 critical impacts of security vulnerabilities may not take into account
 the fact that peoples lives literally depend on Linux getting a lot
 more secure, and quickly.
>>> You are not going to help anyone with a kernel configuration that
>>> breaks agetty, csh, xemacs and tcsh. The opportunities for using
>>> such a configuration are limited.
>> This patch does not break these programs as you imply. 99% of users of these
>> programs will not be effected. Its not like the TIOCSTI ioctl is a critical
>> part of these programs.
> 
> Most likely not.
> 
>>
>> Also as I've stated elsewhere, this is not breaking userspace because this
>> Kconfig/sysctl defaults to n. If someone is using the programs listed above 
>> in
>> a way that does utilize an unprivileged call to the TIOCSTI ioctl, they can
>> turn this feature off.
> 
> Default "off" does not mean it doesn't break userspace. It means that it might
> not break userspace in your environment. Or it might, depending on the whim of
> the build tool of the day.
> 

By this logic, it seems like any introduced feature which toggles some security
feature could be seen as "breaking userspace." For example:

1. Let there exist a LSM X that is set to "off" by default.
(let's say its a simpler type of MAC ;)
2. There exists an inexperienced user Bob that toggles X to "on".
3. Bob complains that X has broken userspace because he now cannot access his
SSH key from firefox.

build tool's will always have important impacts on a system based on the config
that is used.

My understanding of the "don't break userspace" rule has always been to not
change existing, userspace facing, APIs/ioctls/system calls/etc. I don't
believe that my patch does this.

>>
 If this work were not valuable, it wouldnt be an enabled kernel option
 on a massive number of kernels with attack surfaces reduced by the
 compound protections offered by the grsec patch set.
>>> I'll bet you a beverage that 99-44/100% of the people who have
>>> this enabled have no clue that it's even there. And if they did,
>>> most of them would turn it off.
>>>
>> First, I don't know how to parse "99-44/100%" and therefore do not wish to
>> wager a beverage on such confusing odds ;)
> 
> 99.44%. And I loose a *lot* of beverage bets.
> 
>> Second, as stated above, this feature is off by default. However, I would 
>> expect
>> this sysctl to show up in lists of procedures for hardening linux servers.
> 
> It's esoteric enough that I expect that if anyone got bitten by it
> word would get out and no one would use it thereafter.
> 

As we know in the security world, esoteric things can have major a impact. I
have not looked thought all of these, but I imagine most of them could have
been prevented by this patch.

https://cve.mitre.org/cgi-bin/cvekey.cgi?keyword=tiocsti

>>
 I can't speak for
 the grsec people, but having read a small fraction of the commentary
 around the subject of mainline integration, it seems to me that NAKs
 like this are exactly why they had no interest in even trying - this
 review is based on the cultural views of the kernel community, not on
 the security benefits offered by the work in the current state of
 affairs (where userspace is broken).
>>> A security clamp-down that breaks important stuff is going
>>> to have a tough row to hoe going upstream. Same with a performance
>>> enhancement that breaks things.
>>>
 The purpose of each of these
 protections (being ported over from grsec) is not to offer carte
 blanche defense against all attackers and vectors, but to prevent
 specific classes of bugs from reducing the security posture of the
 system. By implementing these defenses in a layered manner we can
 significantly reduce our kernel attack surface.
>>> Sure, but they have to work right. That's an important reason to do
>>> small 

Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN

2017-05-29 Thread Matt Brown
On 5/29/17 10:46 PM, Casey Schaufler wrote:
> On 5/29/2017 7:00 PM, Matt Brown wrote:
>> Casey Schaufler,
>>
>> First I must start this off by saying I really appreciate your presentation 
>> on
>> LSMs that is up on youtube. I've got a LSM in the works and your talk has
>> helped me a bunch.
> 
> Thank you. Feedback (especially positive) is always appreciated.
> 
>>
>> On 5/29/17 8:27 PM, Casey Schaufler wrote:
>>> On 5/29/2017 4:51 PM, Boris Lukashev wrote:
 With all due respect sir, i believe your review falls short of the
 purpose of this effort - to harden the kernel against flaws in
 userspace. Comments along the line of "if  does it right
 then your patch is pointless" are not relevant to the context of
 securing kernel functions/interfaces. What userspace should do has
 little bearing on defensive measures actually implemented in the
 kernel - if we took the approach of "someone else is responsible for
 that" in military operations, the world would be a much darker and
 different place today. Those who have the luxury of standoff from the
 critical impacts of security vulnerabilities may not take into account
 the fact that peoples lives literally depend on Linux getting a lot
 more secure, and quickly.
>>> You are not going to help anyone with a kernel configuration that
>>> breaks agetty, csh, xemacs and tcsh. The opportunities for using
>>> such a configuration are limited.
>> This patch does not break these programs as you imply. 99% of users of these
>> programs will not be effected. Its not like the TIOCSTI ioctl is a critical
>> part of these programs.
> 
> Most likely not.
> 
>>
>> Also as I've stated elsewhere, this is not breaking userspace because this
>> Kconfig/sysctl defaults to n. If someone is using the programs listed above 
>> in
>> a way that does utilize an unprivileged call to the TIOCSTI ioctl, they can
>> turn this feature off.
> 
> Default "off" does not mean it doesn't break userspace. It means that it might
> not break userspace in your environment. Or it might, depending on the whim of
> the build tool of the day.
> 

By this logic, it seems like any introduced feature which toggles some security
feature could be seen as "breaking userspace." For example:

1. Let there exist a LSM X that is set to "off" by default.
(let's say its a simpler type of MAC ;)
2. There exists an inexperienced user Bob that toggles X to "on".
3. Bob complains that X has broken userspace because he now cannot access his
SSH key from firefox.

build tool's will always have important impacts on a system based on the config
that is used.

My understanding of the "don't break userspace" rule has always been to not
change existing, userspace facing, APIs/ioctls/system calls/etc. I don't
believe that my patch does this.

>>
 If this work were not valuable, it wouldnt be an enabled kernel option
 on a massive number of kernels with attack surfaces reduced by the
 compound protections offered by the grsec patch set.
>>> I'll bet you a beverage that 99-44/100% of the people who have
>>> this enabled have no clue that it's even there. And if they did,
>>> most of them would turn it off.
>>>
>> First, I don't know how to parse "99-44/100%" and therefore do not wish to
>> wager a beverage on such confusing odds ;)
> 
> 99.44%. And I loose a *lot* of beverage bets.
> 
>> Second, as stated above, this feature is off by default. However, I would 
>> expect
>> this sysctl to show up in lists of procedures for hardening linux servers.
> 
> It's esoteric enough that I expect that if anyone got bitten by it
> word would get out and no one would use it thereafter.
> 

As we know in the security world, esoteric things can have major a impact. I
have not looked thought all of these, but I imagine most of them could have
been prevented by this patch.

https://cve.mitre.org/cgi-bin/cvekey.cgi?keyword=tiocsti

>>
 I can't speak for
 the grsec people, but having read a small fraction of the commentary
 around the subject of mainline integration, it seems to me that NAKs
 like this are exactly why they had no interest in even trying - this
 review is based on the cultural views of the kernel community, not on
 the security benefits offered by the work in the current state of
 affairs (where userspace is broken).
>>> A security clamp-down that breaks important stuff is going
>>> to have a tough row to hoe going upstream. Same with a performance
>>> enhancement that breaks things.
>>>
 The purpose of each of these
 protections (being ported over from grsec) is not to offer carte
 blanche defense against all attackers and vectors, but to prevent
 specific classes of bugs from reducing the security posture of the
 system. By implementing these defenses in a layered manner we can
 significantly reduce our kernel attack surface.
>>> Sure, but they have to work right. That's an important reason to do
>>> small 

Re: [PATCH 2/3] Input: synaptics - warn the users when there is a better mode

2017-05-29 Thread Dmitry Torokhov
On Tue, May 23, 2017 at 02:20:51PM -0700, Dmitry Torokhov wrote:
> On Tue, May 23, 2017 at 10:36:56AM +0200, Benjamin Tissoires wrote:
> > The Synaptics touchpads are now either using i2c-hid or rmi-smbus.
> > Warn the users if they are missing the rmi-smbus modules and have no
> > chance of reporting correct data.
> > 
> > Signed-off-by: Benjamin Tissoires 
> > ---
> >  drivers/input/mouse/synaptics.c | 4 
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/drivers/input/mouse/synaptics.c 
> > b/drivers/input/mouse/synaptics.c
> > index 58ff388..fc42259 100644
> > --- a/drivers/input/mouse/synaptics.c
> > +++ b/drivers/input/mouse/synaptics.c
> > @@ -1814,6 +1814,10 @@ int synaptics_init(struct psmouse *psmouse)
> > }
> >  
> > if (SYN_CAP_INTERTOUCH(info.ext_cap_0c)) {
> > +#if !IS_ENABLED(CONFIG_RMI4_SMB) || 
> > !defined(CONFIG_MOUSE_PS2_SYNAPTICS_SMBUS)
> > +   psmouse_warn(psmouse, "The touchpad can support a better bus 
> > than the too old PS/2 protocol.\n"
> > +   "Make sure MOUSE_PS2_SYNAPTICS_SMBUS and RMI4_SMB are 
> > enabled to get a better touchpad experience.\n");
> > +#endif
> 
> I'll pull the checks into "if" to avoid preprocessor and also add checks
> for forcepad PNP IDs as forcepads are not usable with RMI at the moment
> since they need F21.

I made the changes I mentioned and pushed to my for-linus branch, please
take a peek and yell if I screwed up.

> 
> > error = synaptics_setup_intertouch(psmouse, , true);
> > if (!error)
> > return PSMOUSE_SYNAPTICS_SMBUS;
> > -- 
> > 2.9.4
> > 
> 
> Thanks.
> 
> -- 
> Dmitry

-- 
Dmitry


Re: [PATCH 2/3] Input: synaptics - warn the users when there is a better mode

2017-05-29 Thread Dmitry Torokhov
On Tue, May 23, 2017 at 02:20:51PM -0700, Dmitry Torokhov wrote:
> On Tue, May 23, 2017 at 10:36:56AM +0200, Benjamin Tissoires wrote:
> > The Synaptics touchpads are now either using i2c-hid or rmi-smbus.
> > Warn the users if they are missing the rmi-smbus modules and have no
> > chance of reporting correct data.
> > 
> > Signed-off-by: Benjamin Tissoires 
> > ---
> >  drivers/input/mouse/synaptics.c | 4 
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/drivers/input/mouse/synaptics.c 
> > b/drivers/input/mouse/synaptics.c
> > index 58ff388..fc42259 100644
> > --- a/drivers/input/mouse/synaptics.c
> > +++ b/drivers/input/mouse/synaptics.c
> > @@ -1814,6 +1814,10 @@ int synaptics_init(struct psmouse *psmouse)
> > }
> >  
> > if (SYN_CAP_INTERTOUCH(info.ext_cap_0c)) {
> > +#if !IS_ENABLED(CONFIG_RMI4_SMB) || 
> > !defined(CONFIG_MOUSE_PS2_SYNAPTICS_SMBUS)
> > +   psmouse_warn(psmouse, "The touchpad can support a better bus 
> > than the too old PS/2 protocol.\n"
> > +   "Make sure MOUSE_PS2_SYNAPTICS_SMBUS and RMI4_SMB are 
> > enabled to get a better touchpad experience.\n");
> > +#endif
> 
> I'll pull the checks into "if" to avoid preprocessor and also add checks
> for forcepad PNP IDs as forcepads are not usable with RMI at the moment
> since they need F21.

I made the changes I mentioned and pushed to my for-linus branch, please
take a peek and yell if I screwed up.

> 
> > error = synaptics_setup_intertouch(psmouse, , true);
> > if (!error)
> > return PSMOUSE_SYNAPTICS_SMBUS;
> > -- 
> > 2.9.4
> > 
> 
> Thanks.
> 
> -- 
> Dmitry

-- 
Dmitry


Re: [PATCH] Input: synaptics - clear device info before filling in

2017-05-29 Thread Dmitry Torokhov
On Sun, May 28, 2017 at 10:27:07PM -0700, Eric Biggers wrote:
> From: Eric Biggers 
> 
> synaptics_query_hardware() was being passed a
> 'struct synaptics_device_info' in uninitialized stack memory, then not
> always initializing all fields.  This caused garbage to show up in
> certain fields, making the touchpad unusable.
> 
> Fix by zeroing the device info, so all fields default to 0.
> 
> Fixes: 6c53694fb222 ("Input: synaptics - split device info into a separate 
> structure")
> Signed-off-by: Eric Biggers 

Applied, thank you.

> ---
>  drivers/input/mouse/synaptics.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
> index 131df9d3660f..4f97970abc94 100644
> --- a/drivers/input/mouse/synaptics.c
> +++ b/drivers/input/mouse/synaptics.c
> @@ -397,6 +397,8 @@ static int synaptics_query_hardware(struct psmouse 
> *psmouse,
>  {
>   int error;
>  
> + memset(info, 0, sizeof(*info));
> +
>   error = synaptics_identify(psmouse, info);
>   if (error)
>   return error;
> -- 
> 2.13.0
> 

-- 
Dmitry


Re: [PATCH] Input: synaptics - clear device info before filling in

2017-05-29 Thread Dmitry Torokhov
On Sun, May 28, 2017 at 10:27:07PM -0700, Eric Biggers wrote:
> From: Eric Biggers 
> 
> synaptics_query_hardware() was being passed a
> 'struct synaptics_device_info' in uninitialized stack memory, then not
> always initializing all fields.  This caused garbage to show up in
> certain fields, making the touchpad unusable.
> 
> Fix by zeroing the device info, so all fields default to 0.
> 
> Fixes: 6c53694fb222 ("Input: synaptics - split device info into a separate 
> structure")
> Signed-off-by: Eric Biggers 

Applied, thank you.

> ---
>  drivers/input/mouse/synaptics.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
> index 131df9d3660f..4f97970abc94 100644
> --- a/drivers/input/mouse/synaptics.c
> +++ b/drivers/input/mouse/synaptics.c
> @@ -397,6 +397,8 @@ static int synaptics_query_hardware(struct psmouse 
> *psmouse,
>  {
>   int error;
>  
> + memset(info, 0, sizeof(*info));
> +
>   error = synaptics_identify(psmouse, info);
>   if (error)
>   return error;
> -- 
> 2.13.0
> 

-- 
Dmitry


Re: linux-next: build failure after merge of the rcu tree

2017-05-29 Thread Stephen Rothwell
Hi Joe,

On Mon, 29 May 2017 19:20:25 -0700 Joe Perches  wrote:
>
> On Mon, 2017-05-29 at 19:14 -0700, Paul E. McKenney wrote:
> > On Mon, May 29, 2017 at 06:54:26PM -0700, Joe Perches wrote:  
> > > On Tue, 2017-05-30 at 11:40 +1000, Stephen Rothwell wrote:  
> > > > Hi Paul,
> > > >   
> > > > > On Mon, 29 May 2017 14:15:05 -0700 "Paul E. McKenney" 
> > > > >  wrote:
> > > > > 
> > > > > Anyone see any other options?  
> > > 
> > > My preferred option would be removing pr_fmt
> > > and adding a couple new macros.  
> > 
> > Not sure how to evaluate yours and Stephen's changes, but I reverted my
> > conversion to a macro based on the hope that something good will come
> > of this effort.  ;-)  
> 
> Stephen's suggestion makes the format and arguments
> have an apparent mismatch.  What I suggested hides
> the "module %s: ", mod->name bit in the macros (like
> the older pr_fmt use), allows anyone else to #define
> pr_fmt to taste, and keeps the format and arguments in
> agreement.

Yours is much better, mine was just a quick hack ... consider yours

Acked-by: Stephen Rothwell 

-- 
Cheers,
Stephen Rothwell


Re: linux-next: build failure after merge of the rcu tree

2017-05-29 Thread Stephen Rothwell
Hi Joe,

On Mon, 29 May 2017 19:20:25 -0700 Joe Perches  wrote:
>
> On Mon, 2017-05-29 at 19:14 -0700, Paul E. McKenney wrote:
> > On Mon, May 29, 2017 at 06:54:26PM -0700, Joe Perches wrote:  
> > > On Tue, 2017-05-30 at 11:40 +1000, Stephen Rothwell wrote:  
> > > > Hi Paul,
> > > >   
> > > > > On Mon, 29 May 2017 14:15:05 -0700 "Paul E. McKenney" 
> > > > >  wrote:
> > > > > 
> > > > > Anyone see any other options?  
> > > 
> > > My preferred option would be removing pr_fmt
> > > and adding a couple new macros.  
> > 
> > Not sure how to evaluate yours and Stephen's changes, but I reverted my
> > conversion to a macro based on the hope that something good will come
> > of this effort.  ;-)  
> 
> Stephen's suggestion makes the format and arguments
> have an apparent mismatch.  What I suggested hides
> the "module %s: ", mod->name bit in the macros (like
> the older pr_fmt use), allows anyone else to #define
> pr_fmt to taste, and keeps the format and arguments in
> agreement.

Yours is much better, mine was just a quick hack ... consider yours

Acked-by: Stephen Rothwell 

-- 
Cheers,
Stephen Rothwell


Re: CPU_BIG_ENDIAN in generic code (was: Re: [PATCH v3 3/7] arch/sparc: Define config parameter CPU_BIG_ENDIAN)

2017-05-29 Thread Michael Ellerman
Geert Uytterhoeven  writes:

> Hi Michael,
>
> On Mon, May 29, 2017 at 2:07 PM, Michael Ellerman  wrote:
>> Geert Uytterhoeven  writes:
>>> On Tue, May 23, 2017 at 11:45 PM, Babu Moger  wrote:
 Found this problem while enabling queued rwlock on SPARC.
 The parameter CONFIG_CPU_BIG_ENDIAN is used to clear the
 specific byte in qrwlock structure. Without this parameter,
 we clear the wrong byte. Here is the code.

 static inline u8 *__qrwlock_write_byte(struct qrwlock *lock)
  {
 return (u8 *)lock + 3 * IS_BUILTIN(CONFIG_CPU_BIG_ENDIAN);
  }

 Define CPU_BIG_ENDIAN for SPARC to fix it.
>>>
 --- a/arch/sparc/Kconfig
 +++ b/arch/sparc/Kconfig
 @@ -92,6 +92,10 @@ config ARCH_DEFCONFIG
  config ARCH_PROC_KCORE_TEXT
 def_bool y

 +config CPU_BIG_ENDIAN
 +   bool
 +   default y if SPARC
>>>
>>> Nice catch!
>>>
>>> Traditionally, CPU_BIG_ENDIAN and CPU_LITTLE_ENDIAN were defined only on
>>> architectures that may support both.  And it was checked in platform code
>>> and drivers only.
>>> Hence the symbol is lacking from most architectures. Heck, even
>>> architectures that support both may default to one endiannes, and declare
>>> only the symbol for the other endianness:
>>
>> I guess there's a reason we can't use __BIG_ENDIAN__ / __LITTLE_ENDIAN__ ?
>
> I (C/asm) code we can, in Kconfig we cannot.
>
> So far we tried always doing that, but a few checks for the semi-existing
> Kconfig symbol crept in in generic code. Those could be replaced by the __*__
> variants, but consistently having the Kconfig symbols would be useful anyway
> (e.g. to avoid building the broken-on-big-endian ISDN drivers).

Ah OK, the original mail was citing C code, but yeah I guess it would be
handy in Makefiles etc.

cheers


Re: CPU_BIG_ENDIAN in generic code (was: Re: [PATCH v3 3/7] arch/sparc: Define config parameter CPU_BIG_ENDIAN)

2017-05-29 Thread Michael Ellerman
Geert Uytterhoeven  writes:

> Hi Michael,
>
> On Mon, May 29, 2017 at 2:07 PM, Michael Ellerman  wrote:
>> Geert Uytterhoeven  writes:
>>> On Tue, May 23, 2017 at 11:45 PM, Babu Moger  wrote:
 Found this problem while enabling queued rwlock on SPARC.
 The parameter CONFIG_CPU_BIG_ENDIAN is used to clear the
 specific byte in qrwlock structure. Without this parameter,
 we clear the wrong byte. Here is the code.

 static inline u8 *__qrwlock_write_byte(struct qrwlock *lock)
  {
 return (u8 *)lock + 3 * IS_BUILTIN(CONFIG_CPU_BIG_ENDIAN);
  }

 Define CPU_BIG_ENDIAN for SPARC to fix it.
>>>
 --- a/arch/sparc/Kconfig
 +++ b/arch/sparc/Kconfig
 @@ -92,6 +92,10 @@ config ARCH_DEFCONFIG
  config ARCH_PROC_KCORE_TEXT
 def_bool y

 +config CPU_BIG_ENDIAN
 +   bool
 +   default y if SPARC
>>>
>>> Nice catch!
>>>
>>> Traditionally, CPU_BIG_ENDIAN and CPU_LITTLE_ENDIAN were defined only on
>>> architectures that may support both.  And it was checked in platform code
>>> and drivers only.
>>> Hence the symbol is lacking from most architectures. Heck, even
>>> architectures that support both may default to one endiannes, and declare
>>> only the symbol for the other endianness:
>>
>> I guess there's a reason we can't use __BIG_ENDIAN__ / __LITTLE_ENDIAN__ ?
>
> I (C/asm) code we can, in Kconfig we cannot.
>
> So far we tried always doing that, but a few checks for the semi-existing
> Kconfig symbol crept in in generic code. Those could be replaced by the __*__
> variants, but consistently having the Kconfig symbols would be useful anyway
> (e.g. to avoid building the broken-on-big-endian ISDN drivers).

Ah OK, the original mail was citing C code, but yeah I guess it would be
handy in Makefiles etc.

cheers


Re: [PATCH RFC 0/5] *** SPI Slave mode support ***

2017-05-29 Thread Jiada Wang

Hi Fabio

On 05/29/2017 05:01 AM, Fabio Estevam wrote:

Hi Jiada,

On Thu, Apr 27, 2017 at 3:43 AM, Jiada Wang  wrote:


BUT if you think the use case don't need to be supported from SPI core,
then I don't have objection either, I will only submit imx SPI slave support
patch,
after your SPI slave support patch set been applied

Geert's SPI slave series has been applied.

Do you still plan to send imx spi slave support on top of it?

Yes, I am working on it

Thanks,
Jiada

Thanks




Re: [PATCH 2/2] Input: pm8941-pwrkey: Introduce reboot mode support

2017-05-29 Thread Dmitry Torokhov
On Fri, May 26, 2017 at 11:51:30PM -0700, Bjorn Andersson wrote:
> In some Qualcomm platforms the magic for informing LK which mode to
> reboot into is stored in the PON_SOFT_RB_SPARE register. Register with
> the reboot mode helpers to expose this to the user.

Hmm, is the power key driver the best place to have this? WHy isn't this
a driver in its own right?

> 
> Signed-off-by: Bjorn Andersson 
> ---
>  drivers/input/misc/Kconfig |  1 +
>  drivers/input/misc/pm8941-pwrkey.c | 28 
>  2 files changed, 29 insertions(+)
> 
> diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
> index 3872488c3fd7..56552e16fab1 100644
> --- a/drivers/input/misc/Kconfig
> +++ b/drivers/input/misc/Kconfig
> @@ -132,6 +132,7 @@ config INPUT_PCSPKR
>  config INPUT_PM8941_PWRKEY
>   tristate "Qualcomm PM8941 power key support"
>   depends on MFD_SPMI_PMIC
> + select REBOOT_MODE
>   help
> Say Y here if you want support for the power key usually found
> on boards using a Qualcomm PM8941 compatible PMIC.
> diff --git a/drivers/input/misc/pm8941-pwrkey.c 
> b/drivers/input/misc/pm8941-pwrkey.c
> index 18ad956454f1..c48be6dbaa78 100644
> --- a/drivers/input/misc/pm8941-pwrkey.c
> +++ b/drivers/input/misc/pm8941-pwrkey.c
> @@ -22,6 +22,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  
>  #define PON_REV2 0x01
> @@ -42,6 +43,7 @@
>  #define PON_DBC_CTL  0x71
>  #define  PON_DBC_DELAY_MASK  0x7
>  
> +#define PON_SOFT_RB_SPARE0x8f
>  
>  struct pm8941_pwrkey {
>   struct device *dev;
> @@ -52,8 +54,26 @@ struct pm8941_pwrkey {
>  
>   unsigned int revision;
>   struct notifier_block reboot_notifier;
> +
> + struct reboot_mode_driver reboot_mode;
>  };
>  
> +static int pm8941_reboot_mode_write(struct reboot_mode_driver *reboot,
> + unsigned int magic)
> +{
> + struct pm8941_pwrkey *pwrkey = container_of(reboot, struct 
> pm8941_pwrkey,
> + reboot_mode);
> + int ret;
> +
> + ret = regmap_update_bits(pwrkey->regmap,
> +  pwrkey->baseaddr + PON_SOFT_RB_SPARE,
> +  0xfc, magic << 2);
> + if (ret < 0)
> + dev_err(pwrkey->dev, "update reboot mode bits failed\n");
> +
> + return ret;
> +}
> +
>  static int pm8941_reboot_notify(struct notifier_block *nb,
>   unsigned long code, void *unused)
>  {
> @@ -256,6 +276,14 @@ static int pm8941_pwrkey_probe(struct platform_device 
> *pdev)
>   return error;
>   }
>  
> + pwrkey->reboot_mode.dev = >dev;
> + pwrkey->reboot_mode.write = pm8941_reboot_mode_write;
> + error = devm_reboot_mode_register(>dev, >reboot_mode);
> + if (error) {
> + dev_err(>dev, "can't register reboot mode\n");
> + return error;
> + }
> +
>   platform_set_drvdata(pdev, pwrkey);
>   device_init_wakeup(>dev, 1);
>  
> -- 
> 2.12.0
> 

Thanks.

-- 
Dmitry


Re: [PATCH RFC 0/5] *** SPI Slave mode support ***

2017-05-29 Thread Jiada Wang

Hi Fabio

On 05/29/2017 05:01 AM, Fabio Estevam wrote:

Hi Jiada,

On Thu, Apr 27, 2017 at 3:43 AM, Jiada Wang  wrote:


BUT if you think the use case don't need to be supported from SPI core,
then I don't have objection either, I will only submit imx SPI slave support
patch,
after your SPI slave support patch set been applied

Geert's SPI slave series has been applied.

Do you still plan to send imx spi slave support on top of it?

Yes, I am working on it

Thanks,
Jiada

Thanks




Re: [PATCH 2/2] Input: pm8941-pwrkey: Introduce reboot mode support

2017-05-29 Thread Dmitry Torokhov
On Fri, May 26, 2017 at 11:51:30PM -0700, Bjorn Andersson wrote:
> In some Qualcomm platforms the magic for informing LK which mode to
> reboot into is stored in the PON_SOFT_RB_SPARE register. Register with
> the reboot mode helpers to expose this to the user.

Hmm, is the power key driver the best place to have this? WHy isn't this
a driver in its own right?

> 
> Signed-off-by: Bjorn Andersson 
> ---
>  drivers/input/misc/Kconfig |  1 +
>  drivers/input/misc/pm8941-pwrkey.c | 28 
>  2 files changed, 29 insertions(+)
> 
> diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
> index 3872488c3fd7..56552e16fab1 100644
> --- a/drivers/input/misc/Kconfig
> +++ b/drivers/input/misc/Kconfig
> @@ -132,6 +132,7 @@ config INPUT_PCSPKR
>  config INPUT_PM8941_PWRKEY
>   tristate "Qualcomm PM8941 power key support"
>   depends on MFD_SPMI_PMIC
> + select REBOOT_MODE
>   help
> Say Y here if you want support for the power key usually found
> on boards using a Qualcomm PM8941 compatible PMIC.
> diff --git a/drivers/input/misc/pm8941-pwrkey.c 
> b/drivers/input/misc/pm8941-pwrkey.c
> index 18ad956454f1..c48be6dbaa78 100644
> --- a/drivers/input/misc/pm8941-pwrkey.c
> +++ b/drivers/input/misc/pm8941-pwrkey.c
> @@ -22,6 +22,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  
>  #define PON_REV2 0x01
> @@ -42,6 +43,7 @@
>  #define PON_DBC_CTL  0x71
>  #define  PON_DBC_DELAY_MASK  0x7
>  
> +#define PON_SOFT_RB_SPARE0x8f
>  
>  struct pm8941_pwrkey {
>   struct device *dev;
> @@ -52,8 +54,26 @@ struct pm8941_pwrkey {
>  
>   unsigned int revision;
>   struct notifier_block reboot_notifier;
> +
> + struct reboot_mode_driver reboot_mode;
>  };
>  
> +static int pm8941_reboot_mode_write(struct reboot_mode_driver *reboot,
> + unsigned int magic)
> +{
> + struct pm8941_pwrkey *pwrkey = container_of(reboot, struct 
> pm8941_pwrkey,
> + reboot_mode);
> + int ret;
> +
> + ret = regmap_update_bits(pwrkey->regmap,
> +  pwrkey->baseaddr + PON_SOFT_RB_SPARE,
> +  0xfc, magic << 2);
> + if (ret < 0)
> + dev_err(pwrkey->dev, "update reboot mode bits failed\n");
> +
> + return ret;
> +}
> +
>  static int pm8941_reboot_notify(struct notifier_block *nb,
>   unsigned long code, void *unused)
>  {
> @@ -256,6 +276,14 @@ static int pm8941_pwrkey_probe(struct platform_device 
> *pdev)
>   return error;
>   }
>  
> + pwrkey->reboot_mode.dev = >dev;
> + pwrkey->reboot_mode.write = pm8941_reboot_mode_write;
> + error = devm_reboot_mode_register(>dev, >reboot_mode);
> + if (error) {
> + dev_err(>dev, "can't register reboot mode\n");
> + return error;
> + }
> +
>   platform_set_drvdata(pdev, pwrkey);
>   device_init_wakeup(>dev, 1);
>  
> -- 
> 2.12.0
> 

Thanks.

-- 
Dmitry


Re: [PATCH 1/4] thermal: uniphier: add UniPhier thermal driver

2017-05-29 Thread Kunihiko Hayashi
Hi Keerthy,
Thank you for your comment.

On Mon, 29 May 2017 15:53:39 +0530 Keerthy  wrote:
> 
> 
> On Monday 29 May 2017 02:45 PM, Kunihiko Hayashi wrote:
> > Add a thermal driver for on-chip PVT (Process, Voltage and Temperature)
> > monitoring unit implemented on UniPhier SoCs. This driver supports
> > temperature monitoring and alert function.
> 
> The Documentation in patch 2/4 should be squashed into this patch.

According to the following guide,
http://elixir.free-electrons.com/linux/latest/source/Documentation/devicetree/bindings/submitting-patches.txt

  1) The Documentation/ portion of the patch should be a separate patch.

I think it should be separated, how about that?

> > 
> > Signed-off-by: Kunihiko Hayashi 
> > ---
> >  drivers/thermal/Kconfig|   8 +
> >  drivers/thermal/Makefile   |   1 +
> >  drivers/thermal/uniphier_thermal.c | 390 
> > +
> >  3 files changed, 399 insertions(+)
> >  create mode 100644 drivers/thermal/uniphier_thermal.c
> > 
> > diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> > index 776b343..93ccf25 100644
> > --- a/drivers/thermal/Kconfig
> > +++ b/drivers/thermal/Kconfig
> > @@ -453,4 +453,12 @@ config ZX2967_THERMAL
> >   the primitive temperature sensor embedded in zx2967 SoCs.
> >   This sensor generates the real time die temperature.
> >  
> > +config UNIPHIER_THERMAL
> > +   tristate "Socionext UniPhier thermal driver"
> > +   depends on ARCH_UNIPHIER || COMPILE_TEST
> > +   help
> > + Enable this to support thermal reporting on UniPhier SoC.
> > + This driver supports CPU thermal zone to reports the temperture
> 
> /s/temperture/temperature
> 
> Or consider re-wording to
> 
> "Enable this to plug in UniPhier on-chip PVT thermal driver into Linux
> thermal framework. The driver supports CPU thermal zone temperature
> reporting and a couple of trip points."

Thanks for finding a typo and your re-wording. I'll fix it.

> > + and trip points.
> > +
> >  endif
> > diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
> > index 7adae20..05b6e7c 100644
> > --- a/drivers/thermal/Makefile
> > +++ b/drivers/thermal/Makefile
> > @@ -58,3 +58,4 @@ obj-$(CONFIG_HISI_THERMAL) += hisi_thermal.o
> >  obj-$(CONFIG_MTK_THERMAL)  += mtk_thermal.o
> >  obj-$(CONFIG_GENERIC_ADC_THERMAL)  += thermal-generic-adc.o
> >  obj-$(CONFIG_ZX2967_THERMAL)   += zx2967_thermal.o
> > +obj-$(CONFIG_UNIPHIER_THERMAL) += uniphier_thermal.o
> > diff --git a/drivers/thermal/uniphier_thermal.c 
> > b/drivers/thermal/uniphier_thermal.c
> > new file mode 100644
> > index 000..ae7e5ce
> > --- /dev/null
> > +++ b/drivers/thermal/uniphier_thermal.c
> > @@ -0,0 +1,390 @@
> > +/**
> > + * uniphier_thermal.c - Socionext UniPhier thermal driver
> > + *
> > + * Copyright 2014 Panasonic Corporation
> > + * Copyright 2016 Socionext Inc.
> 
> Update this please.

I see. I'll update it.

> > + * All rights reserved.
> > + *
> > + * Author:
> > + * Kunihiko Hayashi 
> > + *
> > + * This program is free software: you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2  of
> > + * the License as published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > + * GNU General Public License for more details.
> > + */
> > +
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#include "thermal_core.h"
> > +
> > +/*
> > + * block registers
> > + * addresses are the offset from .block_base
> > + */
> > +#define PVTCTLEN   0x
> > +#define PVTCTLEN_ENBIT(0)
> > +
> > +#define PVTCTLMODE 0x0004
> > +#define PVTCTLMODE_MASK0xf
> > +#define PVTCTLMODE_TEMPMON 0x5
> > +
> > +#define EMONREPEAT 0x0040
> > +#define EMONREPEAT_ENDLESS BIT(24)
> > +#define EMONREPEAT_PERIOD  0xf
> > +#define EMONREPEAT_PERIOD_100  0x9
> > +
> > +/*
> > + * common registers
> > + * addresses are the offset from .map_base
> > + */
> > +#define PVTCTLSEL  0x0900
> > +#define PVTCTLSEL_MASK 0x7
> > +#define PVTCTLSEL_MONITOR  0
> > +
> > +#define SETALERT0  0x0910
> > +#define SETALERT1  0x0914
> > +#define SETALERT2  0x0918
> > +#define SETALERT_TEMP_OVF  (0xff << 16)
> > +#define SETALERT_TEMP_OVF_VALUE(val)   (((val) & 0xff) << 16)
> > +#define 

Re: [PATCH 1/4] thermal: uniphier: add UniPhier thermal driver

2017-05-29 Thread Kunihiko Hayashi
Hi Keerthy,
Thank you for your comment.

On Mon, 29 May 2017 15:53:39 +0530 Keerthy  wrote:
> 
> 
> On Monday 29 May 2017 02:45 PM, Kunihiko Hayashi wrote:
> > Add a thermal driver for on-chip PVT (Process, Voltage and Temperature)
> > monitoring unit implemented on UniPhier SoCs. This driver supports
> > temperature monitoring and alert function.
> 
> The Documentation in patch 2/4 should be squashed into this patch.

According to the following guide,
http://elixir.free-electrons.com/linux/latest/source/Documentation/devicetree/bindings/submitting-patches.txt

  1) The Documentation/ portion of the patch should be a separate patch.

I think it should be separated, how about that?

> > 
> > Signed-off-by: Kunihiko Hayashi 
> > ---
> >  drivers/thermal/Kconfig|   8 +
> >  drivers/thermal/Makefile   |   1 +
> >  drivers/thermal/uniphier_thermal.c | 390 
> > +
> >  3 files changed, 399 insertions(+)
> >  create mode 100644 drivers/thermal/uniphier_thermal.c
> > 
> > diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> > index 776b343..93ccf25 100644
> > --- a/drivers/thermal/Kconfig
> > +++ b/drivers/thermal/Kconfig
> > @@ -453,4 +453,12 @@ config ZX2967_THERMAL
> >   the primitive temperature sensor embedded in zx2967 SoCs.
> >   This sensor generates the real time die temperature.
> >  
> > +config UNIPHIER_THERMAL
> > +   tristate "Socionext UniPhier thermal driver"
> > +   depends on ARCH_UNIPHIER || COMPILE_TEST
> > +   help
> > + Enable this to support thermal reporting on UniPhier SoC.
> > + This driver supports CPU thermal zone to reports the temperture
> 
> /s/temperture/temperature
> 
> Or consider re-wording to
> 
> "Enable this to plug in UniPhier on-chip PVT thermal driver into Linux
> thermal framework. The driver supports CPU thermal zone temperature
> reporting and a couple of trip points."

Thanks for finding a typo and your re-wording. I'll fix it.

> > + and trip points.
> > +
> >  endif
> > diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
> > index 7adae20..05b6e7c 100644
> > --- a/drivers/thermal/Makefile
> > +++ b/drivers/thermal/Makefile
> > @@ -58,3 +58,4 @@ obj-$(CONFIG_HISI_THERMAL) += hisi_thermal.o
> >  obj-$(CONFIG_MTK_THERMAL)  += mtk_thermal.o
> >  obj-$(CONFIG_GENERIC_ADC_THERMAL)  += thermal-generic-adc.o
> >  obj-$(CONFIG_ZX2967_THERMAL)   += zx2967_thermal.o
> > +obj-$(CONFIG_UNIPHIER_THERMAL) += uniphier_thermal.o
> > diff --git a/drivers/thermal/uniphier_thermal.c 
> > b/drivers/thermal/uniphier_thermal.c
> > new file mode 100644
> > index 000..ae7e5ce
> > --- /dev/null
> > +++ b/drivers/thermal/uniphier_thermal.c
> > @@ -0,0 +1,390 @@
> > +/**
> > + * uniphier_thermal.c - Socionext UniPhier thermal driver
> > + *
> > + * Copyright 2014 Panasonic Corporation
> > + * Copyright 2016 Socionext Inc.
> 
> Update this please.

I see. I'll update it.

> > + * All rights reserved.
> > + *
> > + * Author:
> > + * Kunihiko Hayashi 
> > + *
> > + * This program is free software: you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2  of
> > + * the License as published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > + * GNU General Public License for more details.
> > + */
> > +
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#include "thermal_core.h"
> > +
> > +/*
> > + * block registers
> > + * addresses are the offset from .block_base
> > + */
> > +#define PVTCTLEN   0x
> > +#define PVTCTLEN_ENBIT(0)
> > +
> > +#define PVTCTLMODE 0x0004
> > +#define PVTCTLMODE_MASK0xf
> > +#define PVTCTLMODE_TEMPMON 0x5
> > +
> > +#define EMONREPEAT 0x0040
> > +#define EMONREPEAT_ENDLESS BIT(24)
> > +#define EMONREPEAT_PERIOD  0xf
> > +#define EMONREPEAT_PERIOD_100  0x9
> > +
> > +/*
> > + * common registers
> > + * addresses are the offset from .map_base
> > + */
> > +#define PVTCTLSEL  0x0900
> > +#define PVTCTLSEL_MASK 0x7
> > +#define PVTCTLSEL_MONITOR  0
> > +
> > +#define SETALERT0  0x0910
> > +#define SETALERT1  0x0914
> > +#define SETALERT2  0x0918
> > +#define SETALERT_TEMP_OVF  (0xff << 16)
> > +#define SETALERT_TEMP_OVF_VALUE(val)   (((val) & 0xff) << 16)
> > +#define SETALERT_ENBIT(0)
> > +
> > +#define PMALERTINTCTL

Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN

2017-05-29 Thread Casey Schaufler
On 5/29/2017 7:00 PM, Matt Brown wrote:
> Casey Schaufler,
>
> First I must start this off by saying I really appreciate your presentation on
> LSMs that is up on youtube. I've got a LSM in the works and your talk has
> helped me a bunch.

Thank you. Feedback (especially positive) is always appreciated.

>
> On 5/29/17 8:27 PM, Casey Schaufler wrote:
>> On 5/29/2017 4:51 PM, Boris Lukashev wrote:
>>> With all due respect sir, i believe your review falls short of the
>>> purpose of this effort - to harden the kernel against flaws in
>>> userspace. Comments along the line of "if  does it right
>>> then your patch is pointless" are not relevant to the context of
>>> securing kernel functions/interfaces. What userspace should do has
>>> little bearing on defensive measures actually implemented in the
>>> kernel - if we took the approach of "someone else is responsible for
>>> that" in military operations, the world would be a much darker and
>>> different place today. Those who have the luxury of standoff from the
>>> critical impacts of security vulnerabilities may not take into account
>>> the fact that peoples lives literally depend on Linux getting a lot
>>> more secure, and quickly.
>> You are not going to help anyone with a kernel configuration that
>> breaks agetty, csh, xemacs and tcsh. The opportunities for using
>> such a configuration are limited.
> This patch does not break these programs as you imply. 99% of users of these
> programs will not be effected. Its not like the TIOCSTI ioctl is a critical
> part of these programs.

Most likely not.

>
> Also as I've stated elsewhere, this is not breaking userspace because this
> Kconfig/sysctl defaults to n. If someone is using the programs listed above in
> a way that does utilize an unprivileged call to the TIOCSTI ioctl, they can
> turn this feature off.

Default "off" does not mean it doesn't break userspace. It means that it might
not break userspace in your environment. Or it might, depending on the whim of
the build tool of the day.

>
>>> If this work were not valuable, it wouldnt be an enabled kernel option
>>> on a massive number of kernels with attack surfaces reduced by the
>>> compound protections offered by the grsec patch set.
>> I'll bet you a beverage that 99-44/100% of the people who have
>> this enabled have no clue that it's even there. And if they did,
>> most of them would turn it off.
>>
> First, I don't know how to parse "99-44/100%" and therefore do not wish to
> wager a beverage on such confusing odds ;)

99.44%. And I loose a *lot* of beverage bets.

> Second, as stated above, this feature is off by default. However, I would 
> expect
> this sysctl to show up in lists of procedures for hardening linux servers.

It's esoteric enough that I expect that if anyone got bitten by it
word would get out and no one would use it thereafter.

>
>>> I can't speak for
>>> the grsec people, but having read a small fraction of the commentary
>>> around the subject of mainline integration, it seems to me that NAKs
>>> like this are exactly why they had no interest in even trying - this
>>> review is based on the cultural views of the kernel community, not on
>>> the security benefits offered by the work in the current state of
>>> affairs (where userspace is broken).
>> A security clamp-down that breaks important stuff is going
>> to have a tough row to hoe going upstream. Same with a performance
>> enhancement that breaks things.
>>
>>> The purpose of each of these
>>> protections (being ported over from grsec) is not to offer carte
>>> blanche defense against all attackers and vectors, but to prevent
>>> specific classes of bugs from reducing the security posture of the
>>> system. By implementing these defenses in a layered manner we can
>>> significantly reduce our kernel attack surface.
>> Sure, but they have to work right. That's an important reason to do
>> small changes. A change that isn't acceptable can be rejected without
>> slowing the general progress.
>>
>>> Once userspace catches
>>> up and does things the right way, and has no capacity for doing them
>>> the wrong way (aka, nothing attackers can use to bypass the proper
>>> userspace behavior), then the functionality really does become
>>> pointless, and can then be removed.
>> Well, until someone comes along with yet another spiffy feature
>> like containers and breaks it again. This is why a really good
>> solution is required, and the one proposed isn't up to snuff.
>>
> Can you please state your reasons for why you believe this solution is not "up
> to snuff?" So far myself and others have given what I believe to be sound
> responses to any objections to this patch.

If you can't convince Alan, who know ways more about ttys than anyone
ought to, it's not up to snuff.

>
>>> >From a practical perspective, can alternative solutions be offered
>>> along with NAKs?
>> They often are, but let's face it, not everyone has the time,
>> desire and/or expertise to solve every 

Re: [kernel-hardening] Re: [PATCH v7 2/2] security: tty: make TIOCSTI ioctl require CAP_SYS_ADMIN

2017-05-29 Thread Casey Schaufler
On 5/29/2017 7:00 PM, Matt Brown wrote:
> Casey Schaufler,
>
> First I must start this off by saying I really appreciate your presentation on
> LSMs that is up on youtube. I've got a LSM in the works and your talk has
> helped me a bunch.

Thank you. Feedback (especially positive) is always appreciated.

>
> On 5/29/17 8:27 PM, Casey Schaufler wrote:
>> On 5/29/2017 4:51 PM, Boris Lukashev wrote:
>>> With all due respect sir, i believe your review falls short of the
>>> purpose of this effort - to harden the kernel against flaws in
>>> userspace. Comments along the line of "if  does it right
>>> then your patch is pointless" are not relevant to the context of
>>> securing kernel functions/interfaces. What userspace should do has
>>> little bearing on defensive measures actually implemented in the
>>> kernel - if we took the approach of "someone else is responsible for
>>> that" in military operations, the world would be a much darker and
>>> different place today. Those who have the luxury of standoff from the
>>> critical impacts of security vulnerabilities may not take into account
>>> the fact that peoples lives literally depend on Linux getting a lot
>>> more secure, and quickly.
>> You are not going to help anyone with a kernel configuration that
>> breaks agetty, csh, xemacs and tcsh. The opportunities for using
>> such a configuration are limited.
> This patch does not break these programs as you imply. 99% of users of these
> programs will not be effected. Its not like the TIOCSTI ioctl is a critical
> part of these programs.

Most likely not.

>
> Also as I've stated elsewhere, this is not breaking userspace because this
> Kconfig/sysctl defaults to n. If someone is using the programs listed above in
> a way that does utilize an unprivileged call to the TIOCSTI ioctl, they can
> turn this feature off.

Default "off" does not mean it doesn't break userspace. It means that it might
not break userspace in your environment. Or it might, depending on the whim of
the build tool of the day.

>
>>> If this work were not valuable, it wouldnt be an enabled kernel option
>>> on a massive number of kernels with attack surfaces reduced by the
>>> compound protections offered by the grsec patch set.
>> I'll bet you a beverage that 99-44/100% of the people who have
>> this enabled have no clue that it's even there. And if they did,
>> most of them would turn it off.
>>
> First, I don't know how to parse "99-44/100%" and therefore do not wish to
> wager a beverage on such confusing odds ;)

99.44%. And I loose a *lot* of beverage bets.

> Second, as stated above, this feature is off by default. However, I would 
> expect
> this sysctl to show up in lists of procedures for hardening linux servers.

It's esoteric enough that I expect that if anyone got bitten by it
word would get out and no one would use it thereafter.

>
>>> I can't speak for
>>> the grsec people, but having read a small fraction of the commentary
>>> around the subject of mainline integration, it seems to me that NAKs
>>> like this are exactly why they had no interest in even trying - this
>>> review is based on the cultural views of the kernel community, not on
>>> the security benefits offered by the work in the current state of
>>> affairs (where userspace is broken).
>> A security clamp-down that breaks important stuff is going
>> to have a tough row to hoe going upstream. Same with a performance
>> enhancement that breaks things.
>>
>>> The purpose of each of these
>>> protections (being ported over from grsec) is not to offer carte
>>> blanche defense against all attackers and vectors, but to prevent
>>> specific classes of bugs from reducing the security posture of the
>>> system. By implementing these defenses in a layered manner we can
>>> significantly reduce our kernel attack surface.
>> Sure, but they have to work right. That's an important reason to do
>> small changes. A change that isn't acceptable can be rejected without
>> slowing the general progress.
>>
>>> Once userspace catches
>>> up and does things the right way, and has no capacity for doing them
>>> the wrong way (aka, nothing attackers can use to bypass the proper
>>> userspace behavior), then the functionality really does become
>>> pointless, and can then be removed.
>> Well, until someone comes along with yet another spiffy feature
>> like containers and breaks it again. This is why a really good
>> solution is required, and the one proposed isn't up to snuff.
>>
> Can you please state your reasons for why you believe this solution is not "up
> to snuff?" So far myself and others have given what I believe to be sound
> responses to any objections to this patch.

If you can't convince Alan, who know ways more about ttys than anyone
ought to, it's not up to snuff.

>
>>> >From a practical perspective, can alternative solutions be offered
>>> along with NAKs?
>> They often are, but let's face it, not everyone has the time,
>> desire and/or expertise to solve every 

Re: [PATCH 16/16] platform/x86: dell-wmi: Convert to the WMI bus infrastructure

2017-05-29 Thread Dmitry Torokhov
On Sat, May 27, 2017 at 11:40:52AM -0700, Andy Lutomirski wrote:
> On Sat, May 27, 2017 at 9:17 AM, Dmitry Torokhov
>  wrote:
> > On May 27, 2017 9:04:38 AM PDT, Andy Lutomirski  wrote:
> >>On Sat, May 27, 2017 at 3:50 AM, Pali Rohár 
> >>wrote:
> >>> On Saturday 27 May 2017 07:31:30 Darren Hart wrote:
>  - dell_wmi_input_dev->name = "Dell WMI hotkeys";
>  - dell_wmi_input_dev->phys = "wmi/input0";
>  - dell_wmi_input_dev->id.bustype = BUS_HOST;
>  + priv->input_dev->name = "Dell WMI hotkeys";
>  + priv->input_dev->id.bustype = BUS_HOST;
> >>>
> >>> Is not there BUS_WMI, or something like that? (Just asking)
> >>>
> >>
> >>Jiri and/or Dmitry, what is bustype for, anyway?
> >
> > The bus type could be used to help further  identifying device if it used 
> > same vendor/product for spi and i2c, for example, but there are not many if 
> > them. I'm not sure if anyone actually makes decisions based on it, but it 
> > is part of abi now.
> >
> >>I suppose we could add BUS_PLATFORM.
> >
> > What would be the difference from BUS_HOST?
> >
> 
> If BUS_HOST means that the device is part of the host as opposed to
> being plugged in, then it seems entirely reasonable.

Yes, it basically means platform-specific interface.

-- 
Dmitry


Re: [PATCH 16/16] platform/x86: dell-wmi: Convert to the WMI bus infrastructure

2017-05-29 Thread Dmitry Torokhov
On Sat, May 27, 2017 at 11:40:52AM -0700, Andy Lutomirski wrote:
> On Sat, May 27, 2017 at 9:17 AM, Dmitry Torokhov
>  wrote:
> > On May 27, 2017 9:04:38 AM PDT, Andy Lutomirski  wrote:
> >>On Sat, May 27, 2017 at 3:50 AM, Pali Rohár 
> >>wrote:
> >>> On Saturday 27 May 2017 07:31:30 Darren Hart wrote:
>  - dell_wmi_input_dev->name = "Dell WMI hotkeys";
>  - dell_wmi_input_dev->phys = "wmi/input0";
>  - dell_wmi_input_dev->id.bustype = BUS_HOST;
>  + priv->input_dev->name = "Dell WMI hotkeys";
>  + priv->input_dev->id.bustype = BUS_HOST;
> >>>
> >>> Is not there BUS_WMI, or something like that? (Just asking)
> >>>
> >>
> >>Jiri and/or Dmitry, what is bustype for, anyway?
> >
> > The bus type could be used to help further  identifying device if it used 
> > same vendor/product for spi and i2c, for example, but there are not many if 
> > them. I'm not sure if anyone actually makes decisions based on it, but it 
> > is part of abi now.
> >
> >>I suppose we could add BUS_PLATFORM.
> >
> > What would be the difference from BUS_HOST?
> >
> 
> If BUS_HOST means that the device is part of the host as opposed to
> being plugged in, then it seems entirely reasonable.

Yes, it basically means platform-specific interface.

-- 
Dmitry


Re: [PATCH v3] Input: mousedev - fix implicit conversion warning

2017-05-29 Thread Dmitry Torokhov
On Mon, May 29, 2017 at 12:22:52PM -0700, Nick Desaulniers wrote:
> On Fri, May 26, 2017 at 10:07:46AM -0700, Dmitry Torokhov wrote:
> > If you look at the code of this fucntion we really use ps2_data as
> > signed in calculations, and this change would break that. While making
> > ps2_data u8 might be beneficial we'd need to rework mousedev_packet() to
> > use signed temporaries for dx, dy and dz before stufifng them into
> > ps2_data.
> 
> Is your recommendation then to stick with the simple cast as in V1 of
> this patch, or stick with u8 and rework mousedev_packet()?

I think casts are often mysterious, so, unless we'll end up with worse
object code, I'd switch to u8 and temps.

-- 
Dmitry


Re: [PATCH v3] Input: mousedev - fix implicit conversion warning

2017-05-29 Thread Dmitry Torokhov
On Mon, May 29, 2017 at 12:22:52PM -0700, Nick Desaulniers wrote:
> On Fri, May 26, 2017 at 10:07:46AM -0700, Dmitry Torokhov wrote:
> > If you look at the code of this fucntion we really use ps2_data as
> > signed in calculations, and this change would break that. While making
> > ps2_data u8 might be beneficial we'd need to rework mousedev_packet() to
> > use signed temporaries for dx, dy and dz before stufifng them into
> > ps2_data.
> 
> Is your recommendation then to stick with the simple cast as in V1 of
> this patch, or stick with u8 and rework mousedev_packet()?

I think casts are often mysterious, so, unless we'll end up with worse
object code, I'd switch to u8 and temps.

-- 
Dmitry


Re: [PATCH] ubifs: Add freeze support

2017-05-29 Thread Hyunchul Lee
On Mon, May 29, 2017 at 10:42:37AM +0200, Richard Weinberger wrote:
> Hyunchul,
> 
> Am 29.05.2017 um 04:24 schrieb Hyunchul Lee:
> >>> This is just broken.  First ubifs should still properly propagate
> >>> the errors, and second freezing/unfreezing read only file systems is
> >>> perfectly valid, 
> >>
> >> it is right.
> > 
> > if updating TNC is failed, ubifs might become inconsistant and be switched 
> > to 
> > read-only mode. for example, when ubifs_jnl_update is called to create a 
> > file, 
> > if inserting a znode for new inode is failed, TNC has only a znode for 
> > new dentry. and this can be only recoverd by replay.
> > 
> > is it required to fix this?
> 
> UBIFS is designed to be power-cut tolerant.
> So, UBIFS must not corrupt in any case.
> 
> Which failure are you facing?
> 
> I have the feeling that you try to paper over some other issue. :-)

The failure hasn't happened. I wondered the following situation
should be handled.

ubifs_create
  ubifs_jnl_update
write_head
ubifs_tnc_add_nm  /* (1) add dentry to TNC */
ubifs_tnc_add /* (2) add new inode to TNC */
ubifs_tnc_add /* (3) add parent inode to TNC */

If ubifs_tnc_add(2) fails, TNC would have the index of a dentry 
which points to an invalid inode. So, though ubifs_readdir
emits the dentry, this inode cannot be accessed. Becasue
there isn't the index of the inode.

I know this situation is hardly probable. But UBIFS would
be read-only and inconsitant in this situation, until replay
is completed.

> 
> Thanks,
> //richard

-- 

Thanks,
Hyunchul


Re: [PATCH] ubifs: Add freeze support

2017-05-29 Thread Hyunchul Lee
On Mon, May 29, 2017 at 10:42:37AM +0200, Richard Weinberger wrote:
> Hyunchul,
> 
> Am 29.05.2017 um 04:24 schrieb Hyunchul Lee:
> >>> This is just broken.  First ubifs should still properly propagate
> >>> the errors, and second freezing/unfreezing read only file systems is
> >>> perfectly valid, 
> >>
> >> it is right.
> > 
> > if updating TNC is failed, ubifs might become inconsistant and be switched 
> > to 
> > read-only mode. for example, when ubifs_jnl_update is called to create a 
> > file, 
> > if inserting a znode for new inode is failed, TNC has only a znode for 
> > new dentry. and this can be only recoverd by replay.
> > 
> > is it required to fix this?
> 
> UBIFS is designed to be power-cut tolerant.
> So, UBIFS must not corrupt in any case.
> 
> Which failure are you facing?
> 
> I have the feeling that you try to paper over some other issue. :-)

The failure hasn't happened. I wondered the following situation
should be handled.

ubifs_create
  ubifs_jnl_update
write_head
ubifs_tnc_add_nm  /* (1) add dentry to TNC */
ubifs_tnc_add /* (2) add new inode to TNC */
ubifs_tnc_add /* (3) add parent inode to TNC */

If ubifs_tnc_add(2) fails, TNC would have the index of a dentry 
which points to an invalid inode. So, though ubifs_readdir
emits the dentry, this inode cannot be accessed. Becasue
there isn't the index of the inode.

I know this situation is hardly probable. But UBIFS would
be read-only and inconsitant in this situation, until replay
is completed.

> 
> Thanks,
> //richard

-- 

Thanks,
Hyunchul


Re: [PATCH] spi: add null check before pointer dereference

2017-05-29 Thread Gustavo A. R. Silva

Hi Andi,

Quoting Andi Shyti :


Hi Gustavo,


desc = dmaengine_prep_slave_sg(dma->ch, sgt->sgl, sgt->nents,
   dma->direction, DMA_PREP_INTERRUPT);

+   if (!desc) {
+   dev_err(>master->dev,
+   "%s:dmaengine_prep_slave_sg Failed\n", __func__);
+   return;
+   }
+


I'm sorry, I would nack this patch for now. There was a smilar I
sent before, but, as Krzysztof said, this needs more testing and
a proper solution.



Yeah, I get it.


That's anyway in my todo list.



That's great.


Thanks,
Andi


Thanks!
--
Gustavo A. R. Silva







Re: [PATCH] spi: add null check before pointer dereference

2017-05-29 Thread Gustavo A. R. Silva

Hi Andi,

Quoting Andi Shyti :


Hi Gustavo,


desc = dmaengine_prep_slave_sg(dma->ch, sgt->sgl, sgt->nents,
   dma->direction, DMA_PREP_INTERRUPT);

+   if (!desc) {
+   dev_err(>master->dev,
+   "%s:dmaengine_prep_slave_sg Failed\n", __func__);
+   return;
+   }
+


I'm sorry, I would nack this patch for now. There was a smilar I
sent before, but, as Krzysztof said, this needs more testing and
a proper solution.



Yeah, I get it.


That's anyway in my todo list.



That's great.


Thanks,
Andi


Thanks!
--
Gustavo A. R. Silva







Re: linux-next: build failure after merge of the rcu tree

2017-05-29 Thread Joe Perches
On Mon, 2017-05-29 at 19:14 -0700, Paul E. McKenney wrote:
> On Mon, May 29, 2017 at 06:54:26PM -0700, Joe Perches wrote:
> > On Tue, 2017-05-30 at 11:40 +1000, Stephen Rothwell wrote:
> > > Hi Paul,
> > > 
> > > > On Mon, 29 May 2017 14:15:05 -0700 "Paul E. McKenney" 
> > > >  wrote:
> > > > 
> > > > Anyone see any other options?
> > 
> > My preferred option would be removing pr_fmt
> > and adding a couple new macros.
> 
> Not sure how to evaluate yours and Stephen's changes, but I reverted my
> conversion to a macro based on the hope that something good will come
> of this effort.  ;-)

Stephen's suggestion makes the format and arguments
have an apparent mismatch.  What I suggested hides
the "module %s: ", mod->name bit in the macros (like
the older pr_fmt use), allows anyone else to #define
pr_fmt to taste, and keeps the format and arguments in
agreement.



Re: linux-next: build failure after merge of the rcu tree

2017-05-29 Thread Joe Perches
On Mon, 2017-05-29 at 19:14 -0700, Paul E. McKenney wrote:
> On Mon, May 29, 2017 at 06:54:26PM -0700, Joe Perches wrote:
> > On Tue, 2017-05-30 at 11:40 +1000, Stephen Rothwell wrote:
> > > Hi Paul,
> > > 
> > > > On Mon, 29 May 2017 14:15:05 -0700 "Paul E. McKenney" 
> > > >  wrote:
> > > > 
> > > > Anyone see any other options?
> > 
> > My preferred option would be removing pr_fmt
> > and adding a couple new macros.
> 
> Not sure how to evaluate yours and Stephen's changes, but I reverted my
> conversion to a macro based on the hope that something good will come
> of this effort.  ;-)

Stephen's suggestion makes the format and arguments
have an apparent mismatch.  What I suggested hides
the "module %s: ", mod->name bit in the macros (like
the older pr_fmt use), allows anyone else to #define
pr_fmt to taste, and keeps the format and arguments in
agreement.



Re: linux-next: build failure after merge of the rcu tree

2017-05-29 Thread Paul E. McKenney
On Mon, May 29, 2017 at 06:54:26PM -0700, Joe Perches wrote:
> On Tue, 2017-05-30 at 11:40 +1000, Stephen Rothwell wrote:
> > Hi Paul,
> > 
> > > On Mon, 29 May 2017 14:15:05 -0700 "Paul E. McKenney" 
> > >  wrote:
> > > 
> > > Anyone see any other options?
> 
> My preferred option would be removing pr_fmt
> and adding a couple new macros.

Not sure how to evaluate yours and Stephen's changes, but I reverted my
conversion to a macro based on the hope that something good will come
of this effort.  ;-)

Thanx, Paul

> ---
>  arch/blackfin/kernel/module.c | 39 +--
>  1 file changed, 21 insertions(+), 18 deletions(-)
> 
> diff --git a/arch/blackfin/kernel/module.c b/arch/blackfin/kernel/module.c
> index 0188c933b155..e43aec7eb8d3 100644
> --- a/arch/blackfin/kernel/module.c
> +++ b/arch/blackfin/kernel/module.c
> @@ -4,8 +4,6 @@
>   * Licensed under the GPL-2 or later
>   */
>  
> -#define pr_fmt(fmt) "module %s: " fmt, mod->name
> -
>  #include 
>  #include 
>  #include 
> @@ -16,6 +14,11 @@
>  #include 
>  #include 
>  
> +#define mod_err(mod, fmt, ...)   
> \
> + pr_err("module %s: " fmt, (mod)->name, ##__VA_ARGS__)
> +#define mod_debug(mod, ...)  \
> + pr_debug("module %s: " fmt, (mod)->name, ##__VA_ARGS__)
> +
>  /* Transfer the section to the L1 memory */
>  int
>  module_frob_arch_sections(Elf_Ehdr *hdr, Elf_Shdr *sechdrs,
> @@ -44,7 +47,7 @@ module_frob_arch_sections(Elf_Ehdr *hdr, Elf_Shdr *sechdrs,
>   dest = l1_inst_sram_alloc(s->sh_size);
>   mod->arch.text_l1 = dest;
>   if (dest == NULL) {
> - pr_err("L1 inst memory allocation failed\n");
> + mod_err(mod, "L1 inst memory allocation 
> failed\n");
>   return -1;
>   }
>   dma_memcpy(dest, (void *)s->sh_addr, s->sh_size);
> @@ -56,7 +59,7 @@ module_frob_arch_sections(Elf_Ehdr *hdr, Elf_Shdr *sechdrs,
>   dest = l1_data_sram_alloc(s->sh_size);
>   mod->arch.data_a_l1 = dest;
>   if (dest == NULL) {
> - pr_err("L1 data memory allocation failed\n");
> + mod_err(mod, "L1 data memory allocation 
> failed\n");
>   return -1;
>   }
>   memcpy(dest, (void *)s->sh_addr, s->sh_size);
> @@ -68,7 +71,7 @@ module_frob_arch_sections(Elf_Ehdr *hdr, Elf_Shdr *sechdrs,
>   dest = l1_data_sram_zalloc(s->sh_size);
>   mod->arch.bss_a_l1 = dest;
>   if (dest == NULL) {
> - pr_err("L1 data memory allocation failed\n");
> + mod_err(mod, "L1 data memory allocation 
> failed\n");
>   return -1;
>   }
>  
> @@ -77,7 +80,7 @@ module_frob_arch_sections(Elf_Ehdr *hdr, Elf_Shdr *sechdrs,
>   dest = l1_data_B_sram_alloc(s->sh_size);
>   mod->arch.data_b_l1 = dest;
>   if (dest == NULL) {
> - pr_err("L1 data memory allocation failed\n");
> + mod_err(mod, "L1 data memory allocation 
> failed\n");
>   return -1;
>   }
>   memcpy(dest, (void *)s->sh_addr, s->sh_size);
> @@ -87,7 +90,7 @@ module_frob_arch_sections(Elf_Ehdr *hdr, Elf_Shdr *sechdrs,
>   dest = l1_data_B_sram_alloc(s->sh_size);
>   mod->arch.bss_b_l1 = dest;
>   if (dest == NULL) {
> - pr_err("L1 data memory allocation failed\n");
> + mod_err(mod, "L1 data memory allocation 
> failed\n");
>   return -1;
>   }
>   memset(dest, 0, s->sh_size);
> @@ -99,7 +102,7 @@ module_frob_arch_sections(Elf_Ehdr *hdr, Elf_Shdr *sechdrs,
>   dest = l2_sram_alloc(s->sh_size);
>   mod->arch.text_l2 = dest;
>   if (dest == NULL) {
> - pr_err("L2 SRAM allocation failed\n");
> + mod_err(mod, "L2 SRAM allocation failed\n");
>   return -1;
>   }
>   memcpy(dest, (void *)s->sh_addr, s->sh_size);
> @@ -111,7 +114,7 @@ module_frob_arch_sections(Elf_Ehdr *hdr, Elf_Shdr 
> *sechdrs,
>   dest = l2_sram_alloc(s->sh_size);
>   mod->arch.data_l2 = dest;
>   if (dest == NULL) {
> - pr_err("L2 

Re: linux-next: build failure after merge of the rcu tree

2017-05-29 Thread Paul E. McKenney
On Mon, May 29, 2017 at 06:54:26PM -0700, Joe Perches wrote:
> On Tue, 2017-05-30 at 11:40 +1000, Stephen Rothwell wrote:
> > Hi Paul,
> > 
> > > On Mon, 29 May 2017 14:15:05 -0700 "Paul E. McKenney" 
> > >  wrote:
> > > 
> > > Anyone see any other options?
> 
> My preferred option would be removing pr_fmt
> and adding a couple new macros.

Not sure how to evaluate yours and Stephen's changes, but I reverted my
conversion to a macro based on the hope that something good will come
of this effort.  ;-)

Thanx, Paul

> ---
>  arch/blackfin/kernel/module.c | 39 +--
>  1 file changed, 21 insertions(+), 18 deletions(-)
> 
> diff --git a/arch/blackfin/kernel/module.c b/arch/blackfin/kernel/module.c
> index 0188c933b155..e43aec7eb8d3 100644
> --- a/arch/blackfin/kernel/module.c
> +++ b/arch/blackfin/kernel/module.c
> @@ -4,8 +4,6 @@
>   * Licensed under the GPL-2 or later
>   */
>  
> -#define pr_fmt(fmt) "module %s: " fmt, mod->name
> -
>  #include 
>  #include 
>  #include 
> @@ -16,6 +14,11 @@
>  #include 
>  #include 
>  
> +#define mod_err(mod, fmt, ...)   
> \
> + pr_err("module %s: " fmt, (mod)->name, ##__VA_ARGS__)
> +#define mod_debug(mod, ...)  \
> + pr_debug("module %s: " fmt, (mod)->name, ##__VA_ARGS__)
> +
>  /* Transfer the section to the L1 memory */
>  int
>  module_frob_arch_sections(Elf_Ehdr *hdr, Elf_Shdr *sechdrs,
> @@ -44,7 +47,7 @@ module_frob_arch_sections(Elf_Ehdr *hdr, Elf_Shdr *sechdrs,
>   dest = l1_inst_sram_alloc(s->sh_size);
>   mod->arch.text_l1 = dest;
>   if (dest == NULL) {
> - pr_err("L1 inst memory allocation failed\n");
> + mod_err(mod, "L1 inst memory allocation 
> failed\n");
>   return -1;
>   }
>   dma_memcpy(dest, (void *)s->sh_addr, s->sh_size);
> @@ -56,7 +59,7 @@ module_frob_arch_sections(Elf_Ehdr *hdr, Elf_Shdr *sechdrs,
>   dest = l1_data_sram_alloc(s->sh_size);
>   mod->arch.data_a_l1 = dest;
>   if (dest == NULL) {
> - pr_err("L1 data memory allocation failed\n");
> + mod_err(mod, "L1 data memory allocation 
> failed\n");
>   return -1;
>   }
>   memcpy(dest, (void *)s->sh_addr, s->sh_size);
> @@ -68,7 +71,7 @@ module_frob_arch_sections(Elf_Ehdr *hdr, Elf_Shdr *sechdrs,
>   dest = l1_data_sram_zalloc(s->sh_size);
>   mod->arch.bss_a_l1 = dest;
>   if (dest == NULL) {
> - pr_err("L1 data memory allocation failed\n");
> + mod_err(mod, "L1 data memory allocation 
> failed\n");
>   return -1;
>   }
>  
> @@ -77,7 +80,7 @@ module_frob_arch_sections(Elf_Ehdr *hdr, Elf_Shdr *sechdrs,
>   dest = l1_data_B_sram_alloc(s->sh_size);
>   mod->arch.data_b_l1 = dest;
>   if (dest == NULL) {
> - pr_err("L1 data memory allocation failed\n");
> + mod_err(mod, "L1 data memory allocation 
> failed\n");
>   return -1;
>   }
>   memcpy(dest, (void *)s->sh_addr, s->sh_size);
> @@ -87,7 +90,7 @@ module_frob_arch_sections(Elf_Ehdr *hdr, Elf_Shdr *sechdrs,
>   dest = l1_data_B_sram_alloc(s->sh_size);
>   mod->arch.bss_b_l1 = dest;
>   if (dest == NULL) {
> - pr_err("L1 data memory allocation failed\n");
> + mod_err(mod, "L1 data memory allocation 
> failed\n");
>   return -1;
>   }
>   memset(dest, 0, s->sh_size);
> @@ -99,7 +102,7 @@ module_frob_arch_sections(Elf_Ehdr *hdr, Elf_Shdr *sechdrs,
>   dest = l2_sram_alloc(s->sh_size);
>   mod->arch.text_l2 = dest;
>   if (dest == NULL) {
> - pr_err("L2 SRAM allocation failed\n");
> + mod_err(mod, "L2 SRAM allocation failed\n");
>   return -1;
>   }
>   memcpy(dest, (void *)s->sh_addr, s->sh_size);
> @@ -111,7 +114,7 @@ module_frob_arch_sections(Elf_Ehdr *hdr, Elf_Shdr 
> *sechdrs,
>   dest = l2_sram_alloc(s->sh_size);
>   mod->arch.data_l2 = dest;
>   if (dest == NULL) {
> - pr_err("L2 SRAM allocation failed\n");
> 

[PATCH 3/5] locking/rtmutex: Replace top-waiter and pi_waiters leftmost caching

2017-05-29 Thread Davidlohr Bueso
... with the generic rbtree flavor instead. No changes
in semantics whatsoever.

Signed-off-by: Davidlohr Bueso 
---
 include/linux/init_task.h   |  5 ++---
 include/linux/rtmutex.h |  9 -
 include/linux/sched.h   |  3 +--
 kernel/fork.c   |  3 +--
 kernel/locking/rtmutex-debug.c  |  2 +-
 kernel/locking/rtmutex.c| 36 
 kernel/locking/rtmutex_common.h | 10 +-
 7 files changed, 26 insertions(+), 42 deletions(-)

diff --git a/include/linux/init_task.h b/include/linux/init_task.h
index e049526bc188..d58c3b072f4a 100644
--- a/include/linux/init_task.h
+++ b/include/linux/init_task.h
@@ -181,9 +181,8 @@ extern struct cred init_cred;
 
 #ifdef CONFIG_RT_MUTEXES
 # define INIT_RT_MUTEXES(tsk)  \
-   .pi_waiters = RB_ROOT,  \
-   .pi_top_task = NULL,\
-   .pi_waiters_leftmost = NULL,
+   .pi_waiters = RB_ROOT_CACHED,   \
+   .pi_top_task = NULL,
 #else
 # define INIT_RT_MUTEXES(tsk)
 #endif
diff --git a/include/linux/rtmutex.h b/include/linux/rtmutex.h
index 1abba5ce2a2f..3c65d110fc2d 100644
--- a/include/linux/rtmutex.h
+++ b/include/linux/rtmutex.h
@@ -22,14 +22,13 @@ extern int max_lock_depth; /* for sysctl */
  * The rt_mutex structure
  *
  * @wait_lock: spinlock to protect the structure
- * @waiters:   rbtree root to enqueue waiters in priority order
- * @waiters_leftmost: top waiter
+ * @waiters:   rbtree root to enqueue waiters in priority order;
+ *  caches top-waiter (leftmost node).
  * @owner: the mutex owner
  */
 struct rt_mutex {
raw_spinlock_t  wait_lock;
-   struct rb_root  waiters;
-   struct rb_node  *waiters_leftmost;
+   struct rb_root_cached   waiters;
struct task_struct  *owner;
 #ifdef CONFIG_DEBUG_RT_MUTEXES
int save_state;
@@ -68,7 +67,7 @@ struct hrtimer_sleeper;
 
 #define __RT_MUTEX_INITIALIZER(mutexname) \
{ .wait_lock = __RAW_SPIN_LOCK_UNLOCKED(mutexname.wait_lock) \
-   , .waiters = RB_ROOT \
+   , .waiters = RB_ROOT_CACHED \
, .owner = NULL \
__DEBUG_RT_MUTEX_INITIALIZER(mutexname)}
 
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 6c7b5f426598..f909a3cb8d77 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -778,8 +778,7 @@ struct task_struct {
 
 #ifdef CONFIG_RT_MUTEXES
/* PI waiters blocked on a rt_mutex held by this task: */
-   struct rb_root  pi_waiters;
-   struct rb_node  *pi_waiters_leftmost;
+   struct rb_root_cached   pi_waiters;
/* Updated under owner's pi_lock and rq lock */
struct task_struct  *pi_top_task;
/* Deadlock detection and priority inheritance handling: */
diff --git a/kernel/fork.c b/kernel/fork.c
index ebf3ba03c5b8..5e4985192c36 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -1458,8 +1458,7 @@ static void rt_mutex_init_task(struct task_struct *p)
 {
raw_spin_lock_init(>pi_lock);
 #ifdef CONFIG_RT_MUTEXES
-   p->pi_waiters = RB_ROOT;
-   p->pi_waiters_leftmost = NULL;
+   p->pi_waiters = RB_ROOT_CACHED;
p->pi_top_task = NULL;
p->pi_blocked_on = NULL;
 #endif
diff --git a/kernel/locking/rtmutex-debug.c b/kernel/locking/rtmutex-debug.c
index 58e366ad36f4..0e4d0619d06d 100644
--- a/kernel/locking/rtmutex-debug.c
+++ b/kernel/locking/rtmutex-debug.c
@@ -58,7 +58,7 @@ static void printk_lock(struct rt_mutex *lock, int 
print_owner)
 
 void rt_mutex_debug_task_free(struct task_struct *task)
 {
-   DEBUG_LOCKS_WARN_ON(!RB_EMPTY_ROOT(>pi_waiters));
+   DEBUG_LOCKS_WARN_ON(!RB_EMPTY_ROOT(>pi_waiters.rb_root));
DEBUG_LOCKS_WARN_ON(task->pi_blocked_on);
 }
 
diff --git a/kernel/locking/rtmutex.c b/kernel/locking/rtmutex.c
index 28cd09e635ed..1f5d0c9220b4 100644
--- a/kernel/locking/rtmutex.c
+++ b/kernel/locking/rtmutex.c
@@ -271,10 +271,10 @@ rt_mutex_waiter_equal(struct rt_mutex_waiter *left,
 static void
 rt_mutex_enqueue(struct rt_mutex *lock, struct rt_mutex_waiter *waiter)
 {
-   struct rb_node **link = >waiters.rb_node;
+   struct rb_node **link = >waiters.rb_root.rb_node;
struct rb_node *parent = NULL;
struct rt_mutex_waiter *entry;
-   int leftmost = 1;
+   bool leftmost = true;
 
while (*link) {
parent = *link;
@@ -283,15 +283,12 @@ rt_mutex_enqueue(struct rt_mutex *lock, struct 
rt_mutex_waiter *waiter)
link = >rb_left;
} else {
link = >rb_right;
-   leftmost = 0;
+   leftmost = false;
}
}
 
-   if (leftmost)
-   lock->waiters_leftmost = >tree_entry;
-

[PATCH 3/5] locking/rtmutex: Replace top-waiter and pi_waiters leftmost caching

2017-05-29 Thread Davidlohr Bueso
... with the generic rbtree flavor instead. No changes
in semantics whatsoever.

Signed-off-by: Davidlohr Bueso 
---
 include/linux/init_task.h   |  5 ++---
 include/linux/rtmutex.h |  9 -
 include/linux/sched.h   |  3 +--
 kernel/fork.c   |  3 +--
 kernel/locking/rtmutex-debug.c  |  2 +-
 kernel/locking/rtmutex.c| 36 
 kernel/locking/rtmutex_common.h | 10 +-
 7 files changed, 26 insertions(+), 42 deletions(-)

diff --git a/include/linux/init_task.h b/include/linux/init_task.h
index e049526bc188..d58c3b072f4a 100644
--- a/include/linux/init_task.h
+++ b/include/linux/init_task.h
@@ -181,9 +181,8 @@ extern struct cred init_cred;
 
 #ifdef CONFIG_RT_MUTEXES
 # define INIT_RT_MUTEXES(tsk)  \
-   .pi_waiters = RB_ROOT,  \
-   .pi_top_task = NULL,\
-   .pi_waiters_leftmost = NULL,
+   .pi_waiters = RB_ROOT_CACHED,   \
+   .pi_top_task = NULL,
 #else
 # define INIT_RT_MUTEXES(tsk)
 #endif
diff --git a/include/linux/rtmutex.h b/include/linux/rtmutex.h
index 1abba5ce2a2f..3c65d110fc2d 100644
--- a/include/linux/rtmutex.h
+++ b/include/linux/rtmutex.h
@@ -22,14 +22,13 @@ extern int max_lock_depth; /* for sysctl */
  * The rt_mutex structure
  *
  * @wait_lock: spinlock to protect the structure
- * @waiters:   rbtree root to enqueue waiters in priority order
- * @waiters_leftmost: top waiter
+ * @waiters:   rbtree root to enqueue waiters in priority order;
+ *  caches top-waiter (leftmost node).
  * @owner: the mutex owner
  */
 struct rt_mutex {
raw_spinlock_t  wait_lock;
-   struct rb_root  waiters;
-   struct rb_node  *waiters_leftmost;
+   struct rb_root_cached   waiters;
struct task_struct  *owner;
 #ifdef CONFIG_DEBUG_RT_MUTEXES
int save_state;
@@ -68,7 +67,7 @@ struct hrtimer_sleeper;
 
 #define __RT_MUTEX_INITIALIZER(mutexname) \
{ .wait_lock = __RAW_SPIN_LOCK_UNLOCKED(mutexname.wait_lock) \
-   , .waiters = RB_ROOT \
+   , .waiters = RB_ROOT_CACHED \
, .owner = NULL \
__DEBUG_RT_MUTEX_INITIALIZER(mutexname)}
 
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 6c7b5f426598..f909a3cb8d77 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -778,8 +778,7 @@ struct task_struct {
 
 #ifdef CONFIG_RT_MUTEXES
/* PI waiters blocked on a rt_mutex held by this task: */
-   struct rb_root  pi_waiters;
-   struct rb_node  *pi_waiters_leftmost;
+   struct rb_root_cached   pi_waiters;
/* Updated under owner's pi_lock and rq lock */
struct task_struct  *pi_top_task;
/* Deadlock detection and priority inheritance handling: */
diff --git a/kernel/fork.c b/kernel/fork.c
index ebf3ba03c5b8..5e4985192c36 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -1458,8 +1458,7 @@ static void rt_mutex_init_task(struct task_struct *p)
 {
raw_spin_lock_init(>pi_lock);
 #ifdef CONFIG_RT_MUTEXES
-   p->pi_waiters = RB_ROOT;
-   p->pi_waiters_leftmost = NULL;
+   p->pi_waiters = RB_ROOT_CACHED;
p->pi_top_task = NULL;
p->pi_blocked_on = NULL;
 #endif
diff --git a/kernel/locking/rtmutex-debug.c b/kernel/locking/rtmutex-debug.c
index 58e366ad36f4..0e4d0619d06d 100644
--- a/kernel/locking/rtmutex-debug.c
+++ b/kernel/locking/rtmutex-debug.c
@@ -58,7 +58,7 @@ static void printk_lock(struct rt_mutex *lock, int 
print_owner)
 
 void rt_mutex_debug_task_free(struct task_struct *task)
 {
-   DEBUG_LOCKS_WARN_ON(!RB_EMPTY_ROOT(>pi_waiters));
+   DEBUG_LOCKS_WARN_ON(!RB_EMPTY_ROOT(>pi_waiters.rb_root));
DEBUG_LOCKS_WARN_ON(task->pi_blocked_on);
 }
 
diff --git a/kernel/locking/rtmutex.c b/kernel/locking/rtmutex.c
index 28cd09e635ed..1f5d0c9220b4 100644
--- a/kernel/locking/rtmutex.c
+++ b/kernel/locking/rtmutex.c
@@ -271,10 +271,10 @@ rt_mutex_waiter_equal(struct rt_mutex_waiter *left,
 static void
 rt_mutex_enqueue(struct rt_mutex *lock, struct rt_mutex_waiter *waiter)
 {
-   struct rb_node **link = >waiters.rb_node;
+   struct rb_node **link = >waiters.rb_root.rb_node;
struct rb_node *parent = NULL;
struct rt_mutex_waiter *entry;
-   int leftmost = 1;
+   bool leftmost = true;
 
while (*link) {
parent = *link;
@@ -283,15 +283,12 @@ rt_mutex_enqueue(struct rt_mutex *lock, struct 
rt_mutex_waiter *waiter)
link = >rb_left;
} else {
link = >rb_right;
-   leftmost = 0;
+   leftmost = false;
}
}
 
-   if (leftmost)
-   lock->waiters_leftmost = >tree_entry;
-

[PATCH 2/5] sched/fair: Replace cfs_rq->rb_leftmost

2017-05-29 Thread Davidlohr Bueso
... with the generic rbtree flavor instead. No changes
in semantics whatsoever.

Signed-off-by: Davidlohr Bueso 
---
 kernel/sched/debug.c |  2 +-
 kernel/sched/fair.c  | 35 +++
 kernel/sched/sched.h |  3 +--
 3 files changed, 13 insertions(+), 27 deletions(-)

diff --git a/kernel/sched/debug.c b/kernel/sched/debug.c
index 38f019324f1a..85df87666f6e 100644
--- a/kernel/sched/debug.c
+++ b/kernel/sched/debug.c
@@ -488,7 +488,7 @@ void print_cfs_rq(struct seq_file *m, int cpu, struct 
cfs_rq *cfs_rq)
SPLIT_NS(cfs_rq->exec_clock));
 
raw_spin_lock_irqsave(>lock, flags);
-   if (cfs_rq->rb_leftmost)
+   if (cfs_rq->tasks_timeline.rb_leftmost)
MIN_vruntime = (__pick_first_entity(cfs_rq))->vruntime;
last = __pick_last_entity(cfs_rq);
if (last)
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 47a0c552c77b..63b0c5f4770a 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -523,8 +523,8 @@ static void update_min_vruntime(struct cfs_rq *cfs_rq)
curr = NULL;
}
 
-   if (cfs_rq->rb_leftmost) {
-   struct sched_entity *se = rb_entry(cfs_rq->rb_leftmost,
+   if (cfs_rq->tasks_timeline.rb_leftmost) {
+   struct sched_entity *se = 
rb_entry(cfs_rq->tasks_timeline.rb_leftmost,
   struct sched_entity,
   run_node);
 
@@ -547,10 +547,10 @@ static void update_min_vruntime(struct cfs_rq *cfs_rq)
  */
 static void __enqueue_entity(struct cfs_rq *cfs_rq, struct sched_entity *se)
 {
-   struct rb_node **link = _rq->tasks_timeline.rb_node;
+   struct rb_node **link = _rq->tasks_timeline.rb_root.rb_node;
struct rb_node *parent = NULL;
struct sched_entity *entry;
-   int leftmost = 1;
+   bool leftmost = true;
 
/*
 * Find the right place in the rbtree:
@@ -566,36 +566,23 @@ static void __enqueue_entity(struct cfs_rq *cfs_rq, 
struct sched_entity *se)
link = >rb_left;
} else {
link = >rb_right;
-   leftmost = 0;
+   leftmost = false;
}
}
 
-   /*
-* Maintain a cache of leftmost tree entries (it is frequently
-* used):
-*/
-   if (leftmost)
-   cfs_rq->rb_leftmost = >run_node;
-
rb_link_node(>run_node, parent, link);
-   rb_insert_color(>run_node, _rq->tasks_timeline);
+   rb_insert_color_cached(>run_node,
+  _rq->tasks_timeline, leftmost);
 }
 
 static void __dequeue_entity(struct cfs_rq *cfs_rq, struct sched_entity *se)
 {
-   if (cfs_rq->rb_leftmost == >run_node) {
-   struct rb_node *next_node;
-
-   next_node = rb_next(>run_node);
-   cfs_rq->rb_leftmost = next_node;
-   }
-
-   rb_erase(>run_node, _rq->tasks_timeline);
+   rb_erase_cached(>run_node, _rq->tasks_timeline);
 }
 
 struct sched_entity *__pick_first_entity(struct cfs_rq *cfs_rq)
 {
-   struct rb_node *left = cfs_rq->rb_leftmost;
+   struct rb_node *left = cfs_rq->tasks_timeline.rb_leftmost;
 
if (!left)
return NULL;
@@ -616,7 +603,7 @@ static struct sched_entity *__pick_next_entity(struct 
sched_entity *se)
 #ifdef CONFIG_SCHED_DEBUG
 struct sched_entity *__pick_last_entity(struct cfs_rq *cfs_rq)
 {
-   struct rb_node *last = rb_last(_rq->tasks_timeline);
+   struct rb_node *last = rb_last(_rq->tasks_timeline.rb_root);
 
if (!last)
return NULL;
@@ -9235,7 +9222,7 @@ static void set_curr_task_fair(struct rq *rq)
 
 void init_cfs_rq(struct cfs_rq *cfs_rq)
 {
-   cfs_rq->tasks_timeline = RB_ROOT;
+   cfs_rq->tasks_timeline = RB_ROOT_CACHED;
cfs_rq->min_vruntime = (u64)(-(1LL << 20));
 #ifndef CONFIG_64BIT
cfs_rq->min_vruntime_copy = cfs_rq->min_vruntime;
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index f8cf1d87f065..6fe986abfeee 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -404,8 +404,7 @@ struct cfs_rq {
u64 min_vruntime_copy;
 #endif
 
-   struct rb_root tasks_timeline;
-   struct rb_node *rb_leftmost;
+   struct rb_root_cached tasks_timeline;
 
/*
 * 'curr' points to currently running entity on this cfs_rq.
-- 
2.12.0



[PATCH 2/5] sched/fair: Replace cfs_rq->rb_leftmost

2017-05-29 Thread Davidlohr Bueso
... with the generic rbtree flavor instead. No changes
in semantics whatsoever.

Signed-off-by: Davidlohr Bueso 
---
 kernel/sched/debug.c |  2 +-
 kernel/sched/fair.c  | 35 +++
 kernel/sched/sched.h |  3 +--
 3 files changed, 13 insertions(+), 27 deletions(-)

diff --git a/kernel/sched/debug.c b/kernel/sched/debug.c
index 38f019324f1a..85df87666f6e 100644
--- a/kernel/sched/debug.c
+++ b/kernel/sched/debug.c
@@ -488,7 +488,7 @@ void print_cfs_rq(struct seq_file *m, int cpu, struct 
cfs_rq *cfs_rq)
SPLIT_NS(cfs_rq->exec_clock));
 
raw_spin_lock_irqsave(>lock, flags);
-   if (cfs_rq->rb_leftmost)
+   if (cfs_rq->tasks_timeline.rb_leftmost)
MIN_vruntime = (__pick_first_entity(cfs_rq))->vruntime;
last = __pick_last_entity(cfs_rq);
if (last)
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 47a0c552c77b..63b0c5f4770a 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -523,8 +523,8 @@ static void update_min_vruntime(struct cfs_rq *cfs_rq)
curr = NULL;
}
 
-   if (cfs_rq->rb_leftmost) {
-   struct sched_entity *se = rb_entry(cfs_rq->rb_leftmost,
+   if (cfs_rq->tasks_timeline.rb_leftmost) {
+   struct sched_entity *se = 
rb_entry(cfs_rq->tasks_timeline.rb_leftmost,
   struct sched_entity,
   run_node);
 
@@ -547,10 +547,10 @@ static void update_min_vruntime(struct cfs_rq *cfs_rq)
  */
 static void __enqueue_entity(struct cfs_rq *cfs_rq, struct sched_entity *se)
 {
-   struct rb_node **link = _rq->tasks_timeline.rb_node;
+   struct rb_node **link = _rq->tasks_timeline.rb_root.rb_node;
struct rb_node *parent = NULL;
struct sched_entity *entry;
-   int leftmost = 1;
+   bool leftmost = true;
 
/*
 * Find the right place in the rbtree:
@@ -566,36 +566,23 @@ static void __enqueue_entity(struct cfs_rq *cfs_rq, 
struct sched_entity *se)
link = >rb_left;
} else {
link = >rb_right;
-   leftmost = 0;
+   leftmost = false;
}
}
 
-   /*
-* Maintain a cache of leftmost tree entries (it is frequently
-* used):
-*/
-   if (leftmost)
-   cfs_rq->rb_leftmost = >run_node;
-
rb_link_node(>run_node, parent, link);
-   rb_insert_color(>run_node, _rq->tasks_timeline);
+   rb_insert_color_cached(>run_node,
+  _rq->tasks_timeline, leftmost);
 }
 
 static void __dequeue_entity(struct cfs_rq *cfs_rq, struct sched_entity *se)
 {
-   if (cfs_rq->rb_leftmost == >run_node) {
-   struct rb_node *next_node;
-
-   next_node = rb_next(>run_node);
-   cfs_rq->rb_leftmost = next_node;
-   }
-
-   rb_erase(>run_node, _rq->tasks_timeline);
+   rb_erase_cached(>run_node, _rq->tasks_timeline);
 }
 
 struct sched_entity *__pick_first_entity(struct cfs_rq *cfs_rq)
 {
-   struct rb_node *left = cfs_rq->rb_leftmost;
+   struct rb_node *left = cfs_rq->tasks_timeline.rb_leftmost;
 
if (!left)
return NULL;
@@ -616,7 +603,7 @@ static struct sched_entity *__pick_next_entity(struct 
sched_entity *se)
 #ifdef CONFIG_SCHED_DEBUG
 struct sched_entity *__pick_last_entity(struct cfs_rq *cfs_rq)
 {
-   struct rb_node *last = rb_last(_rq->tasks_timeline);
+   struct rb_node *last = rb_last(_rq->tasks_timeline.rb_root);
 
if (!last)
return NULL;
@@ -9235,7 +9222,7 @@ static void set_curr_task_fair(struct rq *rq)
 
 void init_cfs_rq(struct cfs_rq *cfs_rq)
 {
-   cfs_rq->tasks_timeline = RB_ROOT;
+   cfs_rq->tasks_timeline = RB_ROOT_CACHED;
cfs_rq->min_vruntime = (u64)(-(1LL << 20));
 #ifndef CONFIG_64BIT
cfs_rq->min_vruntime_copy = cfs_rq->min_vruntime;
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index f8cf1d87f065..6fe986abfeee 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -404,8 +404,7 @@ struct cfs_rq {
u64 min_vruntime_copy;
 #endif
 
-   struct rb_root tasks_timeline;
-   struct rb_node *rb_leftmost;
+   struct rb_root_cached tasks_timeline;
 
/*
 * 'curr' points to currently running entity on this cfs_rq.
-- 
2.12.0



  1   2   3   4   5   6   7   8   9   10   >