Re: [PATCH v2 4/6] cpufreq: schedutil: update CFS util only if used

[Joel Fernandes]

On Tue, Jul 4, 2017 at 10:34 AM, Patrick Bellasi
 wrote:
> Currently the utilization of the FAIR class is collected before locking
> the policy. Although that should not be a big issue for most cases, we
> also don't really know how much latency there can be between the
> utilization reading and its usage.
>
> Let's get the FAIR utilization right before its usage to be better in
> sync with the current status of a CPU.
>
> Signed-off-by: Patrick Bellasi 
> Cc: Ingo Molnar 
> Cc: Peter Zijlstra 
> Cc: Rafael J. Wysocki 
> Cc: Viresh Kumar 
> Cc: linux-kernel@vger.kernel.org
> Cc: linux...@vger.kernel.org
> ---
>  kernel/sched/cpufreq_schedutil.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/kernel/sched/cpufreq_schedutil.c 
> b/kernel/sched/cpufreq_schedutil.c
> index 98704d8..df433f1 100644
> --- a/kernel/sched/cpufreq_schedutil.c
> +++ b/kernel/sched/cpufreq_schedutil.c
> @@ -308,10 +308,9 @@ static void sugov_update_shared(struct update_util_data 
> *hook, u64 time,
> if (unlikely(current == sg_policy->thread))
> return;
>
> -   sugov_get_util(, );
> -
> raw_spin_lock(_policy->update_lock);
>
> +   sugov_get_util(, );
> sg_cpu->util = util;
> sg_cpu->max = max;

Is your concern that there will we spinlock contention before calling
sugov_get_util?

If that's the case, with your patch it seems to me such contention
(and hence spinning) itself could cause the utilization to be inflated
- thus calling sugov_get_util after acquiring the lock will not be as
accurate as before. In that case it seems to me its better to let
sugov_get_util be called before acquiring the lock (as before).

thanks,

-Joel

Re: [PATCH v2 4/6] cpufreq: schedutil: update CFS util only if used

[Joel Fernandes]

On Tue, Jul 4, 2017 at 10:34 AM, Patrick Bellasi
 wrote:
> Currently the utilization of the FAIR class is collected before locking
> the policy. Although that should not be a big issue for most cases, we
> also don't really know how much latency there can be between the
> utilization reading and its usage.
>
> Let's get the FAIR utilization right before its usage to be better in
> sync with the current status of a CPU.
>
> Signed-off-by: Patrick Bellasi 
> Cc: Ingo Molnar 
> Cc: Peter Zijlstra 
> Cc: Rafael J. Wysocki 
> Cc: Viresh Kumar 
> Cc: linux-kernel@vger.kernel.org
> Cc: linux...@vger.kernel.org
> ---
>  kernel/sched/cpufreq_schedutil.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/kernel/sched/cpufreq_schedutil.c 
> b/kernel/sched/cpufreq_schedutil.c
> index 98704d8..df433f1 100644
> --- a/kernel/sched/cpufreq_schedutil.c
> +++ b/kernel/sched/cpufreq_schedutil.c
> @@ -308,10 +308,9 @@ static void sugov_update_shared(struct update_util_data 
> *hook, u64 time,
> if (unlikely(current == sg_policy->thread))
> return;
>
> -   sugov_get_util(, );
> -
> raw_spin_lock(_policy->update_lock);
>
> +   sugov_get_util(, );
> sg_cpu->util = util;
> sg_cpu->max = max;

Is your concern that there will we spinlock contention before calling
sugov_get_util?

If that's the case, with your patch it seems to me such contention
(and hence spinning) itself could cause the utilization to be inflated
- thus calling sugov_get_util after acquiring the lock will not be as
accurate as before. In that case it seems to me its better to let
sugov_get_util be called before acquiring the lock (as before).

thanks,

-Joel

Re: [RFC v2 3/5] hmem: add heterogeneous memory sysfs support

[John Hubbard]

On 07/06/2017 02:52 PM, Ross Zwisler wrote:
[...]
> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
> index b1aacfc..31e3f20 100644
> --- a/drivers/acpi/Makefile
> +++ b/drivers/acpi/Makefile
> @@ -72,6 +72,7 @@ obj-$(CONFIG_ACPI_PROCESSOR)+= processor.o
>  obj-$(CONFIG_ACPI)   += container.o
>  obj-$(CONFIG_ACPI_THERMAL)   += thermal.o
>  obj-$(CONFIG_ACPI_NFIT)  += nfit/
> +obj-$(CONFIG_ACPI_HMEM)  += hmem/
>  obj-$(CONFIG_ACPI)   += acpi_memhotplug.o
>  obj-$(CONFIG_ACPI_HOTPLUG_IOAPIC) += ioapic.o
>  obj-$(CONFIG_ACPI_BATTERY)   += battery.o

Hi Ross,

Following are a series of suggestions, intended to clarify naming just
enough so that, when Jerome's HMM patchset lands, we'll be able to
tell the difference between the two types of Heterogeneous Memory.


> diff --git a/drivers/acpi/hmem/Kconfig b/drivers/acpi/hmem/Kconfig
> new file mode 100644
> index 000..09282be
> --- /dev/null
> +++ b/drivers/acpi/hmem/Kconfig
> @@ -0,0 +1,7 @@
> +config ACPI_HMEM
> + bool "ACPI Heterogeneous Memory Support"

How about:

   bool "ACPI Heterogeneous Memory Attribute Table Support"

The idea here, and throughout, is that this type of 
Heterogeneous Memory support is all about "the Heterogeneous Memory
that you found via ACPI's Heterogeneous Memory Attribute Table".

That's different from "the Heterogeneous Memory that you found
when you installed a PCIe device that supports HMM". Or, at least
it is different, until the day that someone decides to burn in
support for an HMM device, into the ACPI tables. Seems unlikely,
though. :) And even so, I think it would still work.


> + depends on ACPI_NUMA
> + depends on SYSFS
> + help
> +   Exports a sysfs representation of the ACPI Heterogeneous Memory
> +   Attributes Table (HMAT).
> diff --git a/drivers/acpi/hmem/Makefile b/drivers/acpi/hmem/Makefile
> new file mode 100644
> index 000..d2aa546
> --- /dev/null
> +++ b/drivers/acpi/hmem/Makefile
> @@ -0,0 +1,2 @@
> +obj-$(CONFIG_ACPI_HMEM) := hmem.o
> +hmem-y := core.o initiator.o target.o
> diff --git a/drivers/acpi/hmem/core.c b/drivers/acpi/hmem/core.c
> new file mode 100644
> index 000..f7638db
> --- /dev/null
> +++ b/drivers/acpi/hmem/core.c
> @@ -0,0 +1,569 @@
> +/*
> + * Heterogeneous memory representation in sysfs

Heterogeneous memory, as discovered via ACPI's Heterogeneous Memory
Attribute Table: representation in sysfs

> + *
> + * Copyright (c) 2017, Intel Corporation.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope 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 

[...]

> diff --git a/drivers/acpi/hmem/hmem.h b/drivers/acpi/hmem/hmem.h
> new file mode 100644
> index 000..38ff540
> --- /dev/null
> +++ b/drivers/acpi/hmem/hmem.h
> @@ -0,0 +1,47 @@
> +/*
> + * Heterogeneous memory representation in sysfs

Heterogeneous memory, as discovered via ACPI's Heterogeneous Memory
Attribute Table: representation in sysfs

> + *
> + * Copyright (c) 2017, Intel Corporation.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope 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.
> + */
> +
> +#ifndef _ACPI_HMEM_H_
> +#define _ACPI_HMEM_H_
> +
> +struct memory_initiator {
> + struct list_head list;
> + struct device dev;
> +
> + /* only one of the following three will be set */
> + struct acpi_srat_cpu_affinity *cpu;
> + struct acpi_srat_x2apic_cpu_affinity *x2apic;
> + struct acpi_srat_gicc_affinity *gicc;
> +
> + int pxm;
> + bool is_registered;
> +};
> +#define to_memory_initiator(d) container_of((d), struct memory_initiator, 
> dev)
> +
> +struct memory_target {
> + struct list_head list;
> + struct device dev;
> + struct acpi_srat_mem_affinity *ma;
> + struct acpi_hmat_address_range *spa;
> + struct memory_initiator *local_init;
> +
> + bool is_cached;
> + bool is_registered;
> +};
> +#define to_memory_target(d) container_of((d), struct memory_target, dev)
> +
> +extern const struct attribute_group *memory_initiator_attribute_groups[];
> +extern const struct attribute_group *memory_target_attribute_groups[];
> +#endif /* _ACPI_HMEM_H_ */
> diff --git a/drivers/acpi/hmem/initiator.c 

Re: [RFC v2 3/5] hmem: add heterogeneous memory sysfs support

[John Hubbard]

On 07/06/2017 02:52 PM, Ross Zwisler wrote:
[...]
> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
> index b1aacfc..31e3f20 100644
> --- a/drivers/acpi/Makefile
> +++ b/drivers/acpi/Makefile
> @@ -72,6 +72,7 @@ obj-$(CONFIG_ACPI_PROCESSOR)+= processor.o
>  obj-$(CONFIG_ACPI)   += container.o
>  obj-$(CONFIG_ACPI_THERMAL)   += thermal.o
>  obj-$(CONFIG_ACPI_NFIT)  += nfit/
> +obj-$(CONFIG_ACPI_HMEM)  += hmem/
>  obj-$(CONFIG_ACPI)   += acpi_memhotplug.o
>  obj-$(CONFIG_ACPI_HOTPLUG_IOAPIC) += ioapic.o
>  obj-$(CONFIG_ACPI_BATTERY)   += battery.o

Hi Ross,

Following are a series of suggestions, intended to clarify naming just
enough so that, when Jerome's HMM patchset lands, we'll be able to
tell the difference between the two types of Heterogeneous Memory.


> diff --git a/drivers/acpi/hmem/Kconfig b/drivers/acpi/hmem/Kconfig
> new file mode 100644
> index 000..09282be
> --- /dev/null
> +++ b/drivers/acpi/hmem/Kconfig
> @@ -0,0 +1,7 @@
> +config ACPI_HMEM
> + bool "ACPI Heterogeneous Memory Support"

How about:

   bool "ACPI Heterogeneous Memory Attribute Table Support"

The idea here, and throughout, is that this type of 
Heterogeneous Memory support is all about "the Heterogeneous Memory
that you found via ACPI's Heterogeneous Memory Attribute Table".

That's different from "the Heterogeneous Memory that you found
when you installed a PCIe device that supports HMM". Or, at least
it is different, until the day that someone decides to burn in
support for an HMM device, into the ACPI tables. Seems unlikely,
though. :) And even so, I think it would still work.


> + depends on ACPI_NUMA
> + depends on SYSFS
> + help
> +   Exports a sysfs representation of the ACPI Heterogeneous Memory
> +   Attributes Table (HMAT).
> diff --git a/drivers/acpi/hmem/Makefile b/drivers/acpi/hmem/Makefile
> new file mode 100644
> index 000..d2aa546
> --- /dev/null
> +++ b/drivers/acpi/hmem/Makefile
> @@ -0,0 +1,2 @@
> +obj-$(CONFIG_ACPI_HMEM) := hmem.o
> +hmem-y := core.o initiator.o target.o
> diff --git a/drivers/acpi/hmem/core.c b/drivers/acpi/hmem/core.c
> new file mode 100644
> index 000..f7638db
> --- /dev/null
> +++ b/drivers/acpi/hmem/core.c
> @@ -0,0 +1,569 @@
> +/*
> + * Heterogeneous memory representation in sysfs

Heterogeneous memory, as discovered via ACPI's Heterogeneous Memory
Attribute Table: representation in sysfs

> + *
> + * Copyright (c) 2017, Intel Corporation.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope 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 

[...]

> diff --git a/drivers/acpi/hmem/hmem.h b/drivers/acpi/hmem/hmem.h
> new file mode 100644
> index 000..38ff540
> --- /dev/null
> +++ b/drivers/acpi/hmem/hmem.h
> @@ -0,0 +1,47 @@
> +/*
> + * Heterogeneous memory representation in sysfs

Heterogeneous memory, as discovered via ACPI's Heterogeneous Memory
Attribute Table: representation in sysfs

> + *
> + * Copyright (c) 2017, Intel Corporation.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope 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.
> + */
> +
> +#ifndef _ACPI_HMEM_H_
> +#define _ACPI_HMEM_H_
> +
> +struct memory_initiator {
> + struct list_head list;
> + struct device dev;
> +
> + /* only one of the following three will be set */
> + struct acpi_srat_cpu_affinity *cpu;
> + struct acpi_srat_x2apic_cpu_affinity *x2apic;
> + struct acpi_srat_gicc_affinity *gicc;
> +
> + int pxm;
> + bool is_registered;
> +};
> +#define to_memory_initiator(d) container_of((d), struct memory_initiator, 
> dev)
> +
> +struct memory_target {
> + struct list_head list;
> + struct device dev;
> + struct acpi_srat_mem_affinity *ma;
> + struct acpi_hmat_address_range *spa;
> + struct memory_initiator *local_init;
> +
> + bool is_cached;
> + bool is_registered;
> +};
> +#define to_memory_target(d) container_of((d), struct memory_target, dev)
> +
> +extern const struct attribute_group *memory_initiator_attribute_groups[];
> +extern const struct attribute_group *memory_target_attribute_groups[];
> +#endif /* _ACPI_HMEM_H_ */
> diff --git a/drivers/acpi/hmem/initiator.c 

[PATCH] drm/udl: Make page_flip asynchronous

[Dawid Kurek]

In page_flip vblank is sent with no delay. Driver does not know when the
actual update is present on the display and has no means for getting
this information from a device. It is practically impossible to say
exactly *when* as there is also i.e. a usb delay.

When we are unable to determine when the vblank actually happens we may
assume it will behave accordingly, i.e. it will present frames with
proper timing. In the worst case scenario it should take up to duration
of one frame (we may get new frame in the device just after presenting
current one so we would need to wait for the whole frame).

Because of the asynchronous nature of the delay we need to synchronize:
 * read/write vrefresh/page_flip data when changing mode and
   preparing/executing vblank
 * USB requests to prevent interleaved access to URBs for two different
   frame buffers

All those changes are backports from ChromeOS:
  1. https://chromium-review.googlesource.com/250622
  2. https://chromium-review.googlesource.com/249450
  partially, only change in udl_modeset.c for 'udl_flip_queue'
  3. https://chromium-review.googlesource.com/321378
  4. https://chromium-review.googlesource.com/324119
+ fixes for checkpatch and latest drm changes

Cc: h...@chromium.org
Cc: marc...@chromium.org
Cc: za...@chromium.org
Cc: db...@google.com
Signed-off-by: Dawid Kurek 
---
 drivers/gpu/drm/udl/udl_drv.h |   4 +
 drivers/gpu/drm/udl/udl_fb.c  |  28 ---
 drivers/gpu/drm/udl/udl_main.c|   3 +
 drivers/gpu/drm/udl/udl_modeset.c | 160 ++
 4 files changed, 171 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/udl/udl_drv.h b/drivers/gpu/drm/udl/udl_drv.h
index 2a75ab8..b93fb8d 100644
--- a/drivers/gpu/drm/udl/udl_drv.h
+++ b/drivers/gpu/drm/udl/udl_drv.h
@@ -47,6 +47,7 @@ struct urb_list {
 };
 
 struct udl_fbdev;
+struct udl_flip_queue;
 
 struct udl_device {
struct device *dev;
@@ -66,6 +67,9 @@ struct udl_device {
atomic_t bytes_identical; /* saved effort with backbuffer comparison */
atomic_t bytes_sent; /* to usb, after compression including overhead */
atomic_t cpu_kcycles_used; /* transpired during pixel processing */
+
+   struct udl_flip_queue *flip_queue;
+   struct mutex transfer_lock; /* to serialize transfers */
 };
 
 struct udl_gem_object {
diff --git a/drivers/gpu/drm/udl/udl_fb.c b/drivers/gpu/drm/udl/udl_fb.c
index 4a65003..6dd9a49 100644
--- a/drivers/gpu/drm/udl/udl_fb.c
+++ b/drivers/gpu/drm/udl/udl_fb.c
@@ -82,7 +82,7 @@ int udl_handle_damage(struct udl_framebuffer *fb, int x, int 
y,
 {
struct drm_device *dev = fb->base.dev;
struct udl_device *udl = dev->dev_private;
-   int i, ret;
+   int i, ret = 0;
char *cmd;
cycles_t start_cycles, end_cycles;
int bytes_sent = 0;
@@ -91,18 +91,22 @@ int udl_handle_damage(struct udl_framebuffer *fb, int x, 
int y,
int aligned_x;
int bpp = fb->base.format->cpp[0];
 
+   mutex_lock(>transfer_lock);
+
if (!fb->active_16)
-   return 0;
+   goto out;
 
if (!fb->obj->vmapping) {
ret = udl_gem_vmap(fb->obj);
if (ret == -ENOMEM) {
DRM_ERROR("failed to vmap fb\n");
-   return 0;
+   ret = 0;
+   goto out;
}
if (!fb->obj->vmapping) {
DRM_ERROR("failed to vmapping\n");
-   return 0;
+   ret = 0;
+   goto out;
}
}
 
@@ -112,14 +116,18 @@ int udl_handle_damage(struct udl_framebuffer *fb, int x, 
int y,
 
if ((width <= 0) ||
(x + width > fb->base.width) ||
-   (y + height > fb->base.height))
-   return -EINVAL;
+   (y + height > fb->base.height)) {
+   ret = -EINVAL;
+   goto out;
+   }
 
start_cycles = get_cycles();
 
urb = udl_get_urb(dev);
-   if (!urb)
-   return 0;
+   if (!urb) {
+   ret = 0;
+   goto out;
+   }
cmd = urb->transfer_buffer;
 
for (i = y; i < y + height ; i++) {
@@ -151,7 +159,9 @@ int udl_handle_damage(struct udl_framebuffer *fb, int x, 
int y,
>> 10)), /* Kcycles */
   >cpu_kcycles_used);
 
-   return 0;
+out:
+   mutex_unlock(>transfer_lock);
+   return ret;
 }
 
 static int udl_fb_mmap(struct fb_info *info, struct vm_area_struct *vma)
diff --git a/drivers/gpu/drm/udl/udl_main.c b/drivers/gpu/drm/udl/udl_main.c
index a9d93b8..d376233 100644
--- a/drivers/gpu/drm/udl/udl_main.c
+++ b/drivers/gpu/drm/udl/udl_main.c
@@ -319,6 +319,8 @@ int udl_driver_load(struct drm_device *dev, unsigned long 
flags)
if (!udl)
return -ENOMEM;
 
+   mutex_init(>transfer_lock);
+

[PATCH] drm/udl: Make page_flip asynchronous

[Dawid Kurek]

In page_flip vblank is sent with no delay. Driver does not know when the
actual update is present on the display and has no means for getting
this information from a device. It is practically impossible to say
exactly *when* as there is also i.e. a usb delay.

When we are unable to determine when the vblank actually happens we may
assume it will behave accordingly, i.e. it will present frames with
proper timing. In the worst case scenario it should take up to duration
of one frame (we may get new frame in the device just after presenting
current one so we would need to wait for the whole frame).

Because of the asynchronous nature of the delay we need to synchronize:
 * read/write vrefresh/page_flip data when changing mode and
   preparing/executing vblank
 * USB requests to prevent interleaved access to URBs for two different
   frame buffers

All those changes are backports from ChromeOS:
  1. https://chromium-review.googlesource.com/250622
  2. https://chromium-review.googlesource.com/249450
  partially, only change in udl_modeset.c for 'udl_flip_queue'
  3. https://chromium-review.googlesource.com/321378
  4. https://chromium-review.googlesource.com/324119
+ fixes for checkpatch and latest drm changes

Cc: h...@chromium.org
Cc: marc...@chromium.org
Cc: za...@chromium.org
Cc: db...@google.com
Signed-off-by: Dawid Kurek 
---
 drivers/gpu/drm/udl/udl_drv.h |   4 +
 drivers/gpu/drm/udl/udl_fb.c  |  28 ---
 drivers/gpu/drm/udl/udl_main.c|   3 +
 drivers/gpu/drm/udl/udl_modeset.c | 160 ++
 4 files changed, 171 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/udl/udl_drv.h b/drivers/gpu/drm/udl/udl_drv.h
index 2a75ab8..b93fb8d 100644
--- a/drivers/gpu/drm/udl/udl_drv.h
+++ b/drivers/gpu/drm/udl/udl_drv.h
@@ -47,6 +47,7 @@ struct urb_list {
 };
 
 struct udl_fbdev;
+struct udl_flip_queue;
 
 struct udl_device {
struct device *dev;
@@ -66,6 +67,9 @@ struct udl_device {
atomic_t bytes_identical; /* saved effort with backbuffer comparison */
atomic_t bytes_sent; /* to usb, after compression including overhead */
atomic_t cpu_kcycles_used; /* transpired during pixel processing */
+
+   struct udl_flip_queue *flip_queue;
+   struct mutex transfer_lock; /* to serialize transfers */
 };
 
 struct udl_gem_object {
diff --git a/drivers/gpu/drm/udl/udl_fb.c b/drivers/gpu/drm/udl/udl_fb.c
index 4a65003..6dd9a49 100644
--- a/drivers/gpu/drm/udl/udl_fb.c
+++ b/drivers/gpu/drm/udl/udl_fb.c
@@ -82,7 +82,7 @@ int udl_handle_damage(struct udl_framebuffer *fb, int x, int 
y,
 {
struct drm_device *dev = fb->base.dev;
struct udl_device *udl = dev->dev_private;
-   int i, ret;
+   int i, ret = 0;
char *cmd;
cycles_t start_cycles, end_cycles;
int bytes_sent = 0;
@@ -91,18 +91,22 @@ int udl_handle_damage(struct udl_framebuffer *fb, int x, 
int y,
int aligned_x;
int bpp = fb->base.format->cpp[0];
 
+   mutex_lock(>transfer_lock);
+
if (!fb->active_16)
-   return 0;
+   goto out;
 
if (!fb->obj->vmapping) {
ret = udl_gem_vmap(fb->obj);
if (ret == -ENOMEM) {
DRM_ERROR("failed to vmap fb\n");
-   return 0;
+   ret = 0;
+   goto out;
}
if (!fb->obj->vmapping) {
DRM_ERROR("failed to vmapping\n");
-   return 0;
+   ret = 0;
+   goto out;
}
}
 
@@ -112,14 +116,18 @@ int udl_handle_damage(struct udl_framebuffer *fb, int x, 
int y,
 
if ((width <= 0) ||
(x + width > fb->base.width) ||
-   (y + height > fb->base.height))
-   return -EINVAL;
+   (y + height > fb->base.height)) {
+   ret = -EINVAL;
+   goto out;
+   }
 
start_cycles = get_cycles();
 
urb = udl_get_urb(dev);
-   if (!urb)
-   return 0;
+   if (!urb) {
+   ret = 0;
+   goto out;
+   }
cmd = urb->transfer_buffer;
 
for (i = y; i < y + height ; i++) {
@@ -151,7 +159,9 @@ int udl_handle_damage(struct udl_framebuffer *fb, int x, 
int y,
>> 10)), /* Kcycles */
   >cpu_kcycles_used);
 
-   return 0;
+out:
+   mutex_unlock(>transfer_lock);
+   return ret;
 }
 
 static int udl_fb_mmap(struct fb_info *info, struct vm_area_struct *vma)
diff --git a/drivers/gpu/drm/udl/udl_main.c b/drivers/gpu/drm/udl/udl_main.c
index a9d93b8..d376233 100644
--- a/drivers/gpu/drm/udl/udl_main.c
+++ b/drivers/gpu/drm/udl/udl_main.c
@@ -319,6 +319,8 @@ int udl_driver_load(struct drm_device *dev, unsigned long 
flags)
if (!udl)
return -ENOMEM;
 
+   mutex_init(>transfer_lock);
+
udl->udev = udev;

Re: [RFC][PATCH] exec: Use init rlimits for setuid exec

[Kees Cook]

On Thu, Jul 6, 2017 at 10:39 PM, Linus Torvalds
 wrote:
> On Thu, Jul 6, 2017 at 10:15 PM, Kees Cook  wrote:
>>
>> I always say this backwards. :P Default is top-down (allocate at high
>> addresses and work down toward low). With unlimited stack, allocations
>> start at low addresses and work up. Here's the results (shown with
>> randomize_va_space sysctl set to 0):
>
> But this doesn't affect the stack layout itself.
>
> So we could do the stack copying without much caring, because that
> happens first, right?
>
> So I think we can do all the envp/argv copying first, and then - as we
> change the credentials, change the rlimit. And the string copies
> wouldn't need to care much - although I guess they are also fine
> checking against a possible *smaller* stack rlimit, which is actually
> what we'd want.

Yup, agreed.

> And I think the credentials switch (which is the point of no return
> anyway) happens before we start mmap'ing the executable etc. We used
> to have some odd code there and do it in the completely wrong order
> (checking that the binary was executable for the *old* user, which
> makes no sense, iirc)

Yeah, it all happens in setup_new_exec(). The first thing is layout
selection, then switching credentials. It could be made to take a hint
from GNU_STACK (which was parsed before setup_new_exec() is called),
check security_bprm_secureexec() and then make the rlimit changes, all
before the layout selection.

> So I'm getting the sense that none of this should be a problem.
>
> But it's entirely possible that I missed something, and am just full
> of shit. Our execve() path has traditionally been very hard to read.
> It's actually gotten a bit better, but the whole "jump back and forth
> between the generic fs/exec.c code and the binfmt code" is certainly
> still there.

Yeah, there might be something else lurking here, but so far, I'm
satisfied this will work too.

-Kees

-- 
Kees Cook
Pixel Security

Re: [RFC][PATCH] exec: Use init rlimits for setuid exec

[Kees Cook]

On Thu, Jul 6, 2017 at 10:39 PM, Linus Torvalds
 wrote:
> On Thu, Jul 6, 2017 at 10:15 PM, Kees Cook  wrote:
>>
>> I always say this backwards. :P Default is top-down (allocate at high
>> addresses and work down toward low). With unlimited stack, allocations
>> start at low addresses and work up. Here's the results (shown with
>> randomize_va_space sysctl set to 0):
>
> But this doesn't affect the stack layout itself.
>
> So we could do the stack copying without much caring, because that
> happens first, right?
>
> So I think we can do all the envp/argv copying first, and then - as we
> change the credentials, change the rlimit. And the string copies
> wouldn't need to care much - although I guess they are also fine
> checking against a possible *smaller* stack rlimit, which is actually
> what we'd want.

Yup, agreed.

> And I think the credentials switch (which is the point of no return
> anyway) happens before we start mmap'ing the executable etc. We used
> to have some odd code there and do it in the completely wrong order
> (checking that the binary was executable for the *old* user, which
> makes no sense, iirc)

Yeah, it all happens in setup_new_exec(). The first thing is layout
selection, then switching credentials. It could be made to take a hint
from GNU_STACK (which was parsed before setup_new_exec() is called),
check security_bprm_secureexec() and then make the rlimit changes, all
before the layout selection.

> So I'm getting the sense that none of this should be a problem.
>
> But it's entirely possible that I missed something, and am just full
> of shit. Our execve() path has traditionally been very hard to read.
> It's actually gotten a bit better, but the whole "jump back and forth
> between the generic fs/exec.c code and the binfmt code" is certainly
> still there.

Yeah, there might be something else lurking here, but so far, I'm
satisfied this will work too.

-Kees

-- 
Kees Cook
Pixel Security

Re: [PATCH v2] ASoC: samsung: i2s: Support more resolution rates

[Krzysztof Kozlowski]

On Fri, Jul 07, 2017 at 10:31:10AM +0900, Jaechul Lee wrote:
> This driver can support more frequencies over 96KHz. There are no reasons
> to limit the frequency range below 96KHz. If codecs/amps or something else
> can't support higher resolution rates, the constraints would be set rates
> properly because each drivers have its own limits.
> 
> I added the 'pcm_rates' field to the dai_data to be set rates by the
> compatibilities. As a result, rates will be set each devices respectively.
> For example of exynos5433, rates will be set from 8KHz to 192KHz.

Code looks okay but commtit message could be still improved. Simply, the
goal of this change is to add 192 kHz ferquency to Exynos5433 and
Exynos7 DAI.

Reviewed-by: Krzysztof Kozlowski 

Best regards,
Krzysztof


> 
> Signed-off-by: Jaechul Lee 
> ---
> v2:
>  - changed the name of variable to pcm_rates. 
>  
>  - removed duplicated code.
>  - modified commit message.
> ---
>  sound/soc/samsung/i2s.c | 20 +---
>  1 file changed, 13 insertions(+), 7 deletions(-)
> 
> diff --git a/sound/soc/samsung/i2s.c b/sound/soc/samsung/i2s.c
> index af3ba4d4ccc5..c9f87f7bae99 100644
> --- a/sound/soc/samsung/i2s.c
> +++ b/sound/soc/samsung/i2s.c
> @@ -50,6 +50,7 @@ struct samsung_i2s_variant_regs {
>  
>  struct samsung_i2s_dai_data {
>   u32 quirks;
> + unsigned int pcm_rates;
>   const struct samsung_i2s_variant_regs *i2s_variant_regs;
>  };
>  
> @@ -1076,13 +1077,13 @@ static const struct snd_soc_component_driver 
> samsung_i2s_component = {
>   .name   = "samsung-i2s",
>  };
>  
> -#define SAMSUNG_I2S_RATESSNDRV_PCM_RATE_8000_96000
> -
>  #define SAMSUNG_I2S_FMTS (SNDRV_PCM_FMTBIT_S8 | \
>   SNDRV_PCM_FMTBIT_S16_LE | \
>   SNDRV_PCM_FMTBIT_S24_LE)
>  
> -static struct i2s_dai *i2s_alloc_dai(struct platform_device *pdev, bool sec)
> +static struct i2s_dai *i2s_alloc_dai(struct platform_device *pdev,
> + const struct samsung_i2s_dai_data *i2s_dai_data,
> + bool sec)
>  {
>   struct i2s_dai *i2s;
>  
> @@ -1101,13 +1102,13 @@ static struct i2s_dai *i2s_alloc_dai(struct 
> platform_device *pdev, bool sec)
>   i2s->i2s_dai_drv.resume = i2s_resume;
>   i2s->i2s_dai_drv.playback.channels_min = 1;
>   i2s->i2s_dai_drv.playback.channels_max = 2;
> - i2s->i2s_dai_drv.playback.rates = SAMSUNG_I2S_RATES;
> + i2s->i2s_dai_drv.playback.rates = i2s_dai_data->pcm_rates;
>   i2s->i2s_dai_drv.playback.formats = SAMSUNG_I2S_FMTS;
>  
>   if (!sec) {
>   i2s->i2s_dai_drv.capture.channels_min = 1;
>   i2s->i2s_dai_drv.capture.channels_max = 2;
> - i2s->i2s_dai_drv.capture.rates = SAMSUNG_I2S_RATES;
> + i2s->i2s_dai_drv.capture.rates = i2s_dai_data->pcm_rates;
>   i2s->i2s_dai_drv.capture.formats = SAMSUNG_I2S_FMTS;
>   }
>   return i2s;
> @@ -1242,7 +1243,7 @@ static int samsung_i2s_probe(struct platform_device 
> *pdev)
>   i2s_dai_data = (struct samsung_i2s_dai_data *)
>   platform_get_device_id(pdev)->driver_data;
>  
> - pri_dai = i2s_alloc_dai(pdev, false);
> + pri_dai = i2s_alloc_dai(pdev, i2s_dai_data, false);
>   if (!pri_dai) {
>   dev_err(>dev, "Unable to alloc I2S_pri\n");
>   return -ENOMEM;
> @@ -1316,7 +1317,7 @@ static int samsung_i2s_probe(struct platform_device 
> *pdev)
>   goto err_disable_clk;
>  
>   if (quirks & QUIRK_SEC_DAI) {
> - sec_dai = i2s_alloc_dai(pdev, true);
> + sec_dai = i2s_alloc_dai(pdev, i2s_dai_data, true);
>   if (!sec_dai) {
>   dev_err(>dev, "Unable to alloc I2S_sec\n");
>   ret = -ENOMEM;
> @@ -1452,29 +1453,34 @@ static const struct samsung_i2s_variant_regs 
> i2sv5_i2s1_regs = {
>  
>  static const struct samsung_i2s_dai_data i2sv3_dai_type = {
>   .quirks = QUIRK_NO_MUXPSR,
> + .pcm_rates = SNDRV_PCM_RATE_8000_96000,
>   .i2s_variant_regs = _regs,
>  };
>  
>  static const struct samsung_i2s_dai_data i2sv5_dai_type = {
>   .quirks = QUIRK_PRI_6CHAN | QUIRK_SEC_DAI | QUIRK_NEED_RSTCLR |
>   QUIRK_SUPPORTS_IDMA,
> + .pcm_rates = SNDRV_PCM_RATE_8000_96000,
>   .i2s_variant_regs = _regs,
>  };
>  
>  static const struct samsung_i2s_dai_data i2sv6_dai_type = {
>   .quirks = QUIRK_PRI_6CHAN | QUIRK_SEC_DAI | QUIRK_NEED_RSTCLR |
>   QUIRK_SUPPORTS_TDM | QUIRK_SUPPORTS_IDMA,
> + .pcm_rates = SNDRV_PCM_RATE_8000_96000,
>   .i2s_variant_regs = _regs,
>  };
>  
>  static const struct samsung_i2s_dai_data i2sv7_dai_type = {
>   .quirks = QUIRK_PRI_6CHAN | QUIRK_SEC_DAI | QUIRK_NEED_RSTCLR |
>   QUIRK_SUPPORTS_TDM,
> + .pcm_rates = 

Re: [PATCH v2] ASoC: samsung: i2s: Support more resolution rates

[Krzysztof Kozlowski]

On Fri, Jul 07, 2017 at 10:31:10AM +0900, Jaechul Lee wrote:
> This driver can support more frequencies over 96KHz. There are no reasons
> to limit the frequency range below 96KHz. If codecs/amps or something else
> can't support higher resolution rates, the constraints would be set rates
> properly because each drivers have its own limits.
> 
> I added the 'pcm_rates' field to the dai_data to be set rates by the
> compatibilities. As a result, rates will be set each devices respectively.
> For example of exynos5433, rates will be set from 8KHz to 192KHz.

Code looks okay but commtit message could be still improved. Simply, the
goal of this change is to add 192 kHz ferquency to Exynos5433 and
Exynos7 DAI.

Reviewed-by: Krzysztof Kozlowski 

Best regards,
Krzysztof


> 
> Signed-off-by: Jaechul Lee 
> ---
> v2:
>  - changed the name of variable to pcm_rates. 
>  
>  - removed duplicated code.
>  - modified commit message.
> ---
>  sound/soc/samsung/i2s.c | 20 +---
>  1 file changed, 13 insertions(+), 7 deletions(-)
> 
> diff --git a/sound/soc/samsung/i2s.c b/sound/soc/samsung/i2s.c
> index af3ba4d4ccc5..c9f87f7bae99 100644
> --- a/sound/soc/samsung/i2s.c
> +++ b/sound/soc/samsung/i2s.c
> @@ -50,6 +50,7 @@ struct samsung_i2s_variant_regs {
>  
>  struct samsung_i2s_dai_data {
>   u32 quirks;
> + unsigned int pcm_rates;
>   const struct samsung_i2s_variant_regs *i2s_variant_regs;
>  };
>  
> @@ -1076,13 +1077,13 @@ static const struct snd_soc_component_driver 
> samsung_i2s_component = {
>   .name   = "samsung-i2s",
>  };
>  
> -#define SAMSUNG_I2S_RATESSNDRV_PCM_RATE_8000_96000
> -
>  #define SAMSUNG_I2S_FMTS (SNDRV_PCM_FMTBIT_S8 | \
>   SNDRV_PCM_FMTBIT_S16_LE | \
>   SNDRV_PCM_FMTBIT_S24_LE)
>  
> -static struct i2s_dai *i2s_alloc_dai(struct platform_device *pdev, bool sec)
> +static struct i2s_dai *i2s_alloc_dai(struct platform_device *pdev,
> + const struct samsung_i2s_dai_data *i2s_dai_data,
> + bool sec)
>  {
>   struct i2s_dai *i2s;
>  
> @@ -1101,13 +1102,13 @@ static struct i2s_dai *i2s_alloc_dai(struct 
> platform_device *pdev, bool sec)
>   i2s->i2s_dai_drv.resume = i2s_resume;
>   i2s->i2s_dai_drv.playback.channels_min = 1;
>   i2s->i2s_dai_drv.playback.channels_max = 2;
> - i2s->i2s_dai_drv.playback.rates = SAMSUNG_I2S_RATES;
> + i2s->i2s_dai_drv.playback.rates = i2s_dai_data->pcm_rates;
>   i2s->i2s_dai_drv.playback.formats = SAMSUNG_I2S_FMTS;
>  
>   if (!sec) {
>   i2s->i2s_dai_drv.capture.channels_min = 1;
>   i2s->i2s_dai_drv.capture.channels_max = 2;
> - i2s->i2s_dai_drv.capture.rates = SAMSUNG_I2S_RATES;
> + i2s->i2s_dai_drv.capture.rates = i2s_dai_data->pcm_rates;
>   i2s->i2s_dai_drv.capture.formats = SAMSUNG_I2S_FMTS;
>   }
>   return i2s;
> @@ -1242,7 +1243,7 @@ static int samsung_i2s_probe(struct platform_device 
> *pdev)
>   i2s_dai_data = (struct samsung_i2s_dai_data *)
>   platform_get_device_id(pdev)->driver_data;
>  
> - pri_dai = i2s_alloc_dai(pdev, false);
> + pri_dai = i2s_alloc_dai(pdev, i2s_dai_data, false);
>   if (!pri_dai) {
>   dev_err(>dev, "Unable to alloc I2S_pri\n");
>   return -ENOMEM;
> @@ -1316,7 +1317,7 @@ static int samsung_i2s_probe(struct platform_device 
> *pdev)
>   goto err_disable_clk;
>  
>   if (quirks & QUIRK_SEC_DAI) {
> - sec_dai = i2s_alloc_dai(pdev, true);
> + sec_dai = i2s_alloc_dai(pdev, i2s_dai_data, true);
>   if (!sec_dai) {
>   dev_err(>dev, "Unable to alloc I2S_sec\n");
>   ret = -ENOMEM;
> @@ -1452,29 +1453,34 @@ static const struct samsung_i2s_variant_regs 
> i2sv5_i2s1_regs = {
>  
>  static const struct samsung_i2s_dai_data i2sv3_dai_type = {
>   .quirks = QUIRK_NO_MUXPSR,
> + .pcm_rates = SNDRV_PCM_RATE_8000_96000,
>   .i2s_variant_regs = _regs,
>  };
>  
>  static const struct samsung_i2s_dai_data i2sv5_dai_type = {
>   .quirks = QUIRK_PRI_6CHAN | QUIRK_SEC_DAI | QUIRK_NEED_RSTCLR |
>   QUIRK_SUPPORTS_IDMA,
> + .pcm_rates = SNDRV_PCM_RATE_8000_96000,
>   .i2s_variant_regs = _regs,
>  };
>  
>  static const struct samsung_i2s_dai_data i2sv6_dai_type = {
>   .quirks = QUIRK_PRI_6CHAN | QUIRK_SEC_DAI | QUIRK_NEED_RSTCLR |
>   QUIRK_SUPPORTS_TDM | QUIRK_SUPPORTS_IDMA,
> + .pcm_rates = SNDRV_PCM_RATE_8000_96000,
>   .i2s_variant_regs = _regs,
>  };
>  
>  static const struct samsung_i2s_dai_data i2sv7_dai_type = {
>   .quirks = QUIRK_PRI_6CHAN | QUIRK_SEC_DAI | QUIRK_NEED_RSTCLR |
>   QUIRK_SUPPORTS_TDM,
> + .pcm_rates = SNDRV_PCM_RATE_8000_192000,
>   

Re: [RFC][PATCH] exec: Use init rlimits for setuid exec

[Kees Cook]

On Thu, Jul 6, 2017 at 10:36 PM, Andy Lutomirski  wrote:
> On Thu, Jul 6, 2017 at 10:15 PM, Kees Cook  wrote:
>> On Thu, Jul 6, 2017 at 10:10 PM, Kees Cook  wrote:
>>> On Thu, Jul 6, 2017 at 9:48 PM, Andy Lutomirski  wrote:
 How about a much simpler solution: don't read rlimit at all in
 copy_strings(), let alone try to enforce it.  Instead, just before the
 point of no return, check how much stack space is already used and, if
 it's more than an appropriate threshold (e.g. 1/4 of the rlimit),
 abort.  Sure, this adds overhead if we're going to abort, but does
 that really matter?
>>>
>>> We should avoid using up tons of memory and then failing. Better to
>>> cap it as we use it. Plumbing a sane value into this shouldn't be hard
>>> at all. Just making this a hardcoded 2MB seems sane (1/4 of 8MB).
>
> Aren't there real use cases that use many megs of arguments?

They'd be relatively new since the args were pretty limited before.
I'd be curious to see them.

> We could probably get away with saying max(rlimit(RLIMIT_STACK), 2MB)
> as long as we make sure later on that we don't screw up if we've
> overallocated?

min, not max, but yeah. Here's part of what I have for get_arg_page():

rlim = current->signal->rlim;
-   if (size > READ_ONCE(rlim[RLIMIT_STACK].rlim_cur) / 4)
+   arg_stack = READ_ONCE(rlim[RLIMIT_STACK].rlim_cur);
+   arg_stack = min_t(unsigned long, arg_stack, _STK_LIM) / 4;
+   if (size > arg_stack)
goto fail;

>>> IIUC, this is a big deal on 32-bit. Unlimited stack triggers top-down
>>> mmap instead of bottom-up. I mean, I'd be delighted to get rid of
>>> this, but I thought it was relied on by userspace.
>>
>> I always say this backwards. :P Default is top-down (allocate at high
>> addresses and work down toward low). With unlimited stack, allocations
>> start at low addresses and work up. Here's the results (shown with
>> randomize_va_space sysctl set to 0):
>
> Uhh, crikey!  Where's the code that does that?

That was the call path I quoted earlier:

> The stack rlimit defines the mmap layout too:
>
> do_execveat_common() ->
> exec_binprm() ->
> search_binary_handler() ->
> fmt->load_binary (load_elf_binary()) ->
> setup_new_exec() ->
> arch_pick_mmap_layout() ->
> mmap_is_legacy() ->
> rlimit(RLIMIT_STACK) == RLIM_INFINITY

i.e. arch_pick_mmap_layout().

-Kees

-- 
Kees Cook
Pixel Security

Re: [RFC][PATCH] exec: Use init rlimits for setuid exec

[Kees Cook]

On Thu, Jul 6, 2017 at 10:36 PM, Andy Lutomirski  wrote:
> On Thu, Jul 6, 2017 at 10:15 PM, Kees Cook  wrote:
>> On Thu, Jul 6, 2017 at 10:10 PM, Kees Cook  wrote:
>>> On Thu, Jul 6, 2017 at 9:48 PM, Andy Lutomirski  wrote:
 How about a much simpler solution: don't read rlimit at all in
 copy_strings(), let alone try to enforce it.  Instead, just before the
 point of no return, check how much stack space is already used and, if
 it's more than an appropriate threshold (e.g. 1/4 of the rlimit),
 abort.  Sure, this adds overhead if we're going to abort, but does
 that really matter?
>>>
>>> We should avoid using up tons of memory and then failing. Better to
>>> cap it as we use it. Plumbing a sane value into this shouldn't be hard
>>> at all. Just making this a hardcoded 2MB seems sane (1/4 of 8MB).
>
> Aren't there real use cases that use many megs of arguments?

They'd be relatively new since the args were pretty limited before.
I'd be curious to see them.

> We could probably get away with saying max(rlimit(RLIMIT_STACK), 2MB)
> as long as we make sure later on that we don't screw up if we've
> overallocated?

min, not max, but yeah. Here's part of what I have for get_arg_page():

rlim = current->signal->rlim;
-   if (size > READ_ONCE(rlim[RLIMIT_STACK].rlim_cur) / 4)
+   arg_stack = READ_ONCE(rlim[RLIMIT_STACK].rlim_cur);
+   arg_stack = min_t(unsigned long, arg_stack, _STK_LIM) / 4;
+   if (size > arg_stack)
goto fail;

>>> IIUC, this is a big deal on 32-bit. Unlimited stack triggers top-down
>>> mmap instead of bottom-up. I mean, I'd be delighted to get rid of
>>> this, but I thought it was relied on by userspace.
>>
>> I always say this backwards. :P Default is top-down (allocate at high
>> addresses and work down toward low). With unlimited stack, allocations
>> start at low addresses and work up. Here's the results (shown with
>> randomize_va_space sysctl set to 0):
>
> Uhh, crikey!  Where's the code that does that?

That was the call path I quoted earlier:

> The stack rlimit defines the mmap layout too:
>
> do_execveat_common() ->
> exec_binprm() ->
> search_binary_handler() ->
> fmt->load_binary (load_elf_binary()) ->
> setup_new_exec() ->
> arch_pick_mmap_layout() ->
> mmap_is_legacy() ->
> rlimit(RLIMIT_STACK) == RLIM_INFINITY

i.e. arch_pick_mmap_layout().

-Kees

-- 
Kees Cook
Pixel Security

[PATCH] staging: rtl8712: fix "Alignment match open parenthesis"

[Arushi Singhal]

Fix checkpatch issues: "CHECK: Alignment should match open parenthesis".

Signed-off-by: Arushi Singhal 
---
 drivers/staging/rtl8712/mlme_linux.c| 4 ++--
 drivers/staging/rtl8712/rtl8712_cmd.c   | 2 +-
 drivers/staging/rtl8712/rtl8712_efuse.c | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/rtl8712/mlme_linux.c 
b/drivers/staging/rtl8712/mlme_linux.c
index 2037265..a077069 100644
--- a/drivers/staging/rtl8712/mlme_linux.c
+++ b/drivers/staging/rtl8712/mlme_linux.c
@@ -111,8 +111,8 @@ void r8712_os_indicate_disconnect(struct _adapter *adapter)
 */
 
memcpy([0],
-   >securitypriv.PMKIDList[0],
-   sizeof(struct RT_PMKID_LIST) * NUM_PMKID_CACHE);
+  >securitypriv.PMKIDList[0],
+  sizeof(struct RT_PMKID_LIST) * NUM_PMKID_CACHE);
backupPMKIDIndex = adapter->securitypriv.PMKIDIndex;
backupTKIPCountermeasure =
adapter->securitypriv.btkip_countermeasure;
diff --git a/drivers/staging/rtl8712/rtl8712_cmd.c 
b/drivers/staging/rtl8712/rtl8712_cmd.c
index 5346c65..0104ace 100644
--- a/drivers/staging/rtl8712/rtl8712_cmd.c
+++ b/drivers/staging/rtl8712/rtl8712_cmd.c
@@ -385,7 +385,7 @@ int r8712_cmd_thread(void *context)
if (blnPending)
wr_sz += 8;   /* Append 8 bytes */
r8712_write_mem(padapter, RTL8712_DMA_H2CCMD, wr_sz,
-  (u8 *)pdesc);
+   (u8 *)pdesc);
pcmdpriv->cmd_seq++;
if (pcmd->cmdcode == GEN_CMD_CODE(_CreateBss)) {
pcmd->res = H2C_SUCCESS;
diff --git a/drivers/staging/rtl8712/rtl8712_efuse.c 
b/drivers/staging/rtl8712/rtl8712_efuse.c
index 205298e..d90213e 100644
--- a/drivers/staging/rtl8712/rtl8712_efuse.c
+++ b/drivers/staging/rtl8712/rtl8712_efuse.c
@@ -347,7 +347,7 @@ static u8 fix_header(struct _adapter *padapter, u8 header, 
u16 header_addr)
ret = false;
if (value == 0xFF) /* write again */
efuse_one_byte_write(padapter, addr,
-   pkt.data[i * 2]);
+pkt.data[i * 2]);
}
if (!efuse_one_byte_read(padapter, addr + 1, )) {
ret = false;
-- 
2.7.4

[PATCH] staging: rtl8712: fix "Alignment match open parenthesis"

[Arushi Singhal]

Fix checkpatch issues: "CHECK: Alignment should match open parenthesis".

Signed-off-by: Arushi Singhal 
---
 drivers/staging/rtl8712/mlme_linux.c| 4 ++--
 drivers/staging/rtl8712/rtl8712_cmd.c   | 2 +-
 drivers/staging/rtl8712/rtl8712_efuse.c | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/rtl8712/mlme_linux.c 
b/drivers/staging/rtl8712/mlme_linux.c
index 2037265..a077069 100644
--- a/drivers/staging/rtl8712/mlme_linux.c
+++ b/drivers/staging/rtl8712/mlme_linux.c
@@ -111,8 +111,8 @@ void r8712_os_indicate_disconnect(struct _adapter *adapter)
 */
 
memcpy([0],
-   >securitypriv.PMKIDList[0],
-   sizeof(struct RT_PMKID_LIST) * NUM_PMKID_CACHE);
+  >securitypriv.PMKIDList[0],
+  sizeof(struct RT_PMKID_LIST) * NUM_PMKID_CACHE);
backupPMKIDIndex = adapter->securitypriv.PMKIDIndex;
backupTKIPCountermeasure =
adapter->securitypriv.btkip_countermeasure;
diff --git a/drivers/staging/rtl8712/rtl8712_cmd.c 
b/drivers/staging/rtl8712/rtl8712_cmd.c
index 5346c65..0104ace 100644
--- a/drivers/staging/rtl8712/rtl8712_cmd.c
+++ b/drivers/staging/rtl8712/rtl8712_cmd.c
@@ -385,7 +385,7 @@ int r8712_cmd_thread(void *context)
if (blnPending)
wr_sz += 8;   /* Append 8 bytes */
r8712_write_mem(padapter, RTL8712_DMA_H2CCMD, wr_sz,
-  (u8 *)pdesc);
+   (u8 *)pdesc);
pcmdpriv->cmd_seq++;
if (pcmd->cmdcode == GEN_CMD_CODE(_CreateBss)) {
pcmd->res = H2C_SUCCESS;
diff --git a/drivers/staging/rtl8712/rtl8712_efuse.c 
b/drivers/staging/rtl8712/rtl8712_efuse.c
index 205298e..d90213e 100644
--- a/drivers/staging/rtl8712/rtl8712_efuse.c
+++ b/drivers/staging/rtl8712/rtl8712_efuse.c
@@ -347,7 +347,7 @@ static u8 fix_header(struct _adapter *padapter, u8 header, 
u16 header_addr)
ret = false;
if (value == 0xFF) /* write again */
efuse_one_byte_write(padapter, addr,
-   pkt.data[i * 2]);
+pkt.data[i * 2]);
}
if (!efuse_one_byte_read(padapter, addr + 1, )) {
ret = false;
-- 
2.7.4

Re: [RFC][PATCH] exec: Use init rlimits for setuid exec

[Linus Torvalds]

On Thu, Jul 6, 2017 at 10:15 PM, Kees Cook  wrote:
>
> I always say this backwards. :P Default is top-down (allocate at high
> addresses and work down toward low). With unlimited stack, allocations
> start at low addresses and work up. Here's the results (shown with
> randomize_va_space sysctl set to 0):

But this doesn't affect the stack layout itself.

So we could do the stack copying without much caring, because that
happens first, right?

So I think we can do all the envp/argv copying first, and then - as we
change the credentials, change the rlimit. And the string copies
wouldn't need to care much - although I guess they are also fine
checking against a possible *smaller* stack rlimit, which is actually
what we'd want.

And I think the credentials switch (which is the point of no return
anyway) happens before we start mmap'ing the executable etc. We used
to have some odd code there and do it in the completely wrong order
(checking that the binary was executable for the *old* user, which
makes no sense, iirc)

So I'm getting the sense that none of this should be a problem.

But it's entirely possible that I missed something, and am just full
of shit. Our execve() path has traditionally been very hard to read.
It's actually gotten a bit better, but the whole "jump back and forth
between the generic fs/exec.c code and the binfmt code" is certainly
still there.

 Linus

Re: [RFC][PATCH] exec: Use init rlimits for setuid exec

[Linus Torvalds]

On Thu, Jul 6, 2017 at 10:15 PM, Kees Cook  wrote:
>
> I always say this backwards. :P Default is top-down (allocate at high
> addresses and work down toward low). With unlimited stack, allocations
> start at low addresses and work up. Here's the results (shown with
> randomize_va_space sysctl set to 0):

But this doesn't affect the stack layout itself.

So we could do the stack copying without much caring, because that
happens first, right?

So I think we can do all the envp/argv copying first, and then - as we
change the credentials, change the rlimit. And the string copies
wouldn't need to care much - although I guess they are also fine
checking against a possible *smaller* stack rlimit, which is actually
what we'd want.

And I think the credentials switch (which is the point of no return
anyway) happens before we start mmap'ing the executable etc. We used
to have some odd code there and do it in the completely wrong order
(checking that the binary was executable for the *old* user, which
makes no sense, iirc)

So I'm getting the sense that none of this should be a problem.

But it's entirely possible that I missed something, and am just full
of shit. Our execve() path has traditionally been very hard to read.
It's actually gotten a bit better, but the whole "jump back and forth
between the generic fs/exec.c code and the binfmt code" is certainly
still there.

 Linus

Re: [RFC][PATCH] exec: Use init rlimits for setuid exec

[Andy Lutomirski]

On Thu, Jul 6, 2017 at 10:15 PM, Kees Cook  wrote:
> On Thu, Jul 6, 2017 at 10:10 PM, Kees Cook  wrote:
>> On Thu, Jul 6, 2017 at 9:48 PM, Andy Lutomirski  wrote:
>>> How about a much simpler solution: don't read rlimit at all in
>>> copy_strings(), let alone try to enforce it.  Instead, just before the
>>> point of no return, check how much stack space is already used and, if
>>> it's more than an appropriate threshold (e.g. 1/4 of the rlimit),
>>> abort.  Sure, this adds overhead if we're going to abort, but does
>>> that really matter?
>>
>> We should avoid using up tons of memory and then failing. Better to
>> cap it as we use it. Plumbing a sane value into this shouldn't be hard
>> at all. Just making this a hardcoded 2MB seems sane (1/4 of 8MB).

Aren't there real use cases that use many megs of arguments?

We could probably get away with saying max(rlimit(RLIMIT_STACK), 2MB)
as long as we make sure later on that we don't screw up if we've
overallocated?

>>
>>> I don't see why using rlimit for layout control makes any sense
>>> whatsoever.  Is there some historical reason we need that?  As far as
>>> I can see (on insufficient inspection) is that the kernel is trying to
>>> guarantee that, if we have so much arg crap that our remaining stack
>>> is less than 128k, then we don't exceed our limit by a little bit.
>>
>> IIUC, this is a big deal on 32-bit. Unlimited stack triggers top-down
>> mmap instead of bottom-up. I mean, I'd be delighted to get rid of
>> this, but I thought it was relied on by userspace.
>
> I always say this backwards. :P Default is top-down (allocate at high
> addresses and work down toward low). With unlimited stack, allocations
> start at low addresses and work up. Here's the results (shown with
> randomize_va_space sysctl set to 0):

Uhh, crikey!  Where's the code that does that?

Re: [RFC][PATCH] exec: Use init rlimits for setuid exec

[Andy Lutomirski]

On Thu, Jul 6, 2017 at 10:15 PM, Kees Cook  wrote:
> On Thu, Jul 6, 2017 at 10:10 PM, Kees Cook  wrote:
>> On Thu, Jul 6, 2017 at 9:48 PM, Andy Lutomirski  wrote:
>>> How about a much simpler solution: don't read rlimit at all in
>>> copy_strings(), let alone try to enforce it.  Instead, just before the
>>> point of no return, check how much stack space is already used and, if
>>> it's more than an appropriate threshold (e.g. 1/4 of the rlimit),
>>> abort.  Sure, this adds overhead if we're going to abort, but does
>>> that really matter?
>>
>> We should avoid using up tons of memory and then failing. Better to
>> cap it as we use it. Plumbing a sane value into this shouldn't be hard
>> at all. Just making this a hardcoded 2MB seems sane (1/4 of 8MB).

Aren't there real use cases that use many megs of arguments?

We could probably get away with saying max(rlimit(RLIMIT_STACK), 2MB)
as long as we make sure later on that we don't screw up if we've
overallocated?

>>
>>> I don't see why using rlimit for layout control makes any sense
>>> whatsoever.  Is there some historical reason we need that?  As far as
>>> I can see (on insufficient inspection) is that the kernel is trying to
>>> guarantee that, if we have so much arg crap that our remaining stack
>>> is less than 128k, then we don't exceed our limit by a little bit.
>>
>> IIUC, this is a big deal on 32-bit. Unlimited stack triggers top-down
>> mmap instead of bottom-up. I mean, I'd be delighted to get rid of
>> this, but I thought it was relied on by userspace.
>
> I always say this backwards. :P Default is top-down (allocate at high
> addresses and work down toward low). With unlimited stack, allocations
> start at low addresses and work up. Here's the results (shown with
> randomize_va_space sysctl set to 0):

Uhh, crikey!  Where's the code that does that?

[PATCH] regulator: qcom_smd: add NULL check on of_match_device() return value

[Gustavo A. R. Silva]

Check return value from call to of_match_device()
in order to prevent a NULL pointer dereference.

In case of NULL print error message and return.

Signed-off-by: Gustavo A. R. Silva 
---
 drivers/regulator/qcom_smd-regulator.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/drivers/regulator/qcom_smd-regulator.c 
b/drivers/regulator/qcom_smd-regulator.c
index f35994a..940fe1b 100644
--- a/drivers/regulator/qcom_smd-regulator.c
+++ b/drivers/regulator/qcom_smd-regulator.c
@@ -570,6 +570,11 @@ static int rpm_reg_probe(struct platform_device *pdev)
}
 
match = of_match_device(rpm_of_match, >dev);
+   if (!match) {
+   dev_err(>dev, "failed to match device\n");
+   return -ENODEV;
+   }
+
for (reg = match->data; reg->name; reg++) {
vreg = devm_kzalloc(>dev, sizeof(*vreg), GFP_KERNEL);
if (!vreg)
-- 
2.5.0

[PATCH] regulator: qcom_smd: add NULL check on of_match_device() return value

[Gustavo A. R. Silva]

Check return value from call to of_match_device()
in order to prevent a NULL pointer dereference.

In case of NULL print error message and return.

Signed-off-by: Gustavo A. R. Silva 
---
 drivers/regulator/qcom_smd-regulator.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/drivers/regulator/qcom_smd-regulator.c 
b/drivers/regulator/qcom_smd-regulator.c
index f35994a..940fe1b 100644
--- a/drivers/regulator/qcom_smd-regulator.c
+++ b/drivers/regulator/qcom_smd-regulator.c
@@ -570,6 +570,11 @@ static int rpm_reg_probe(struct platform_device *pdev)
}
 
match = of_match_device(rpm_of_match, >dev);
+   if (!match) {
+   dev_err(>dev, "failed to match device\n");
+   return -ENODEV;
+   }
+
for (reg = match->data; reg->name; reg++) {
vreg = devm_kzalloc(>dev, sizeof(*vreg), GFP_KERNEL);
if (!vreg)
-- 
2.5.0

Re: [RFC v2 0/5] surface heterogeneous memory performance information

[John Hubbard]

On 07/06/2017 02:52 PM, Ross Zwisler wrote:
[...]
> 
> The naming collision between Jerome's "Heterogeneous Memory Management
> (HMM)" and this "Heterogeneous Memory (HMEM)" series is unfortunate, but I
> was trying to stick with the word "Heterogeneous" because of the naming of
> the ACPI 6.2 Heterogeneous Memory Attribute Table table.  Suggestions for
> better naming are welcome.
> 

Hi Ross,

Say, most of the places (file names, function and variable names, and even
print statements) where this patchset uses hmem or HMEM, it really seems to
mean, the Heterogeneous Memory Attribute Table. That's not *always* true, but
given that it's a pretty severe naming conflict, how about just changing:

hmem --> hmat
HMEM --> HMAT

...everywhere? Then you still have Heterogeneous Memory in the name, but
there is enough lexical distance (is that a thing? haha) between HMM and HMAT
to keep us all sane. :)

With or without the above suggestion, there are a few places (Kconfig, comments,
prints) where we can more easily make it clear that HMM != HMEM (or HMAT), 
so for those I can just comment on them separately in the individual patches.

thanks,
john h

Re: [RFC v2 0/5] surface heterogeneous memory performance information

[John Hubbard]

On 07/06/2017 02:52 PM, Ross Zwisler wrote:
[...]
> 
> The naming collision between Jerome's "Heterogeneous Memory Management
> (HMM)" and this "Heterogeneous Memory (HMEM)" series is unfortunate, but I
> was trying to stick with the word "Heterogeneous" because of the naming of
> the ACPI 6.2 Heterogeneous Memory Attribute Table table.  Suggestions for
> better naming are welcome.
> 

Hi Ross,

Say, most of the places (file names, function and variable names, and even
print statements) where this patchset uses hmem or HMEM, it really seems to
mean, the Heterogeneous Memory Attribute Table. That's not *always* true, but
given that it's a pretty severe naming conflict, how about just changing:

hmem --> hmat
HMEM --> HMAT

...everywhere? Then you still have Heterogeneous Memory in the name, but
there is enough lexical distance (is that a thing? haha) between HMM and HMAT
to keep us all sane. :)

With or without the above suggestion, there are a few places (Kconfig, comments,
prints) where we can more easily make it clear that HMM != HMEM (or HMAT), 
so for those I can just comment on them separately in the individual patches.

thanks,
john h

[PATCH] regulator: qcom_rpm-regulator: add NULL check on of_match_device() return value

[Gustavo A. R. Silva]

Check return value from call to of_match_device()
in order to prevent a NULL pointer dereference.

In case of NULL print error message and return.

Signed-off-by: Gustavo A. R. Silva 
---
 drivers/regulator/qcom_rpm-regulator.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/drivers/regulator/qcom_rpm-regulator.c 
b/drivers/regulator/qcom_rpm-regulator.c
index 1b2acc4..88dc0b0 100644
--- a/drivers/regulator/qcom_rpm-regulator.c
+++ b/drivers/regulator/qcom_rpm-regulator.c
@@ -959,6 +959,11 @@ static int rpm_reg_probe(struct platform_device *pdev)
}
 
match = of_match_device(rpm_of_match, >dev);
+   if (!match) {
+   dev_err(>dev, "failed to match device\n");
+   return -ENODEV;
+   }
+
for (reg = match->data; reg->name; reg++) {
vreg = devm_kmalloc(>dev, sizeof(*vreg), GFP_KERNEL);
if (!vreg)
-- 
2.5.0

[PATCH] regulator: qcom_rpm-regulator: add NULL check on of_match_device() return value

[Gustavo A. R. Silva]

Check return value from call to of_match_device()
in order to prevent a NULL pointer dereference.

In case of NULL print error message and return.

Signed-off-by: Gustavo A. R. Silva 
---
 drivers/regulator/qcom_rpm-regulator.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/drivers/regulator/qcom_rpm-regulator.c 
b/drivers/regulator/qcom_rpm-regulator.c
index 1b2acc4..88dc0b0 100644
--- a/drivers/regulator/qcom_rpm-regulator.c
+++ b/drivers/regulator/qcom_rpm-regulator.c
@@ -959,6 +959,11 @@ static int rpm_reg_probe(struct platform_device *pdev)
}
 
match = of_match_device(rpm_of_match, >dev);
+   if (!match) {
+   dev_err(>dev, "failed to match device\n");
+   return -ENODEV;
+   }
+
for (reg = match->data; reg->name; reg++) {
vreg = devm_kmalloc(>dev, sizeof(*vreg), GFP_KERNEL);
if (!vreg)
-- 
2.5.0

RE: [PATCH 2/2] net: ethernet: fsl: add phy reset after clk enable option

[Andy Duan]

From: Richard Leitner  Sent: Thursday, July 06, 
2017 9:06 PM
>To: Andy Duan ; robh...@kernel.org;
>mark.rutl...@arm.com
>Cc: net...@vger.kernel.org; devicet...@vger.kernel.org; linux-
>ker...@vger.kernel.org; d...@g0hl1n.net; Richard Leitner
>
>Subject: [PATCH 2/2] net: ethernet: fsl: add phy reset after clk enable option
>
>Some PHYs (for example the LAN8710) doesn't allow turning the clocks off and
>on again without reset (according to their datasheet). Exactly this behaviour
>was introduced for power saving reasons by commit e8fcfcd5684a
>("net: fec: optimize the clock management to save power") Therefore add a
>devictree option to perform a PHY reset and configuration after every clock
>enable.
>
>For a better understanding here's a outline of the time response of the clock
>and reset lines before and after this patch:
>
> v--fec_probe()  v--fec_enet_open()
> v   v
>   w/o patch eCLK:
>___|
>   w/o patch nRST: __--
>   w/o patch CONF:
>___XX___
>
>   w/  patch eCLK:
>___|
>   w/  patch nRST: __--__--
>   w/  patch CONF:
>___XX__XX___
> ^   ^
> ^--fec_probe()  ^--fec_enet_open()
>
>In our case this problem does occur at about every 10th to 50th POR of an
>LAN8710 connected to an i.MX6 SoC. The typical symptom of this problem is a
>"swinging" ethernet link. Similar issues were experienced by users of the NXP
>forum:
>   https://community.nxp.com/thread/389902
>   https://community.nxp.com/message/309354
>With this patch applied the issue didn't occur for at least a few hundred PORs
>of our board.
>
>Fixes: e8fcfcd5684a ("net: fec: optimize the clock management to sa...")
>Signed-off-by: Richard Leitner 
>---
> Documentation/devicetree/bindings/net/fsl-fec.txt |  3 +++
> drivers/net/ethernet/freescale/fec.h  |  1 +
> drivers/net/ethernet/freescale/fec_main.c | 16 
> 3 files changed, 20 insertions(+)
>
>diff --git a/Documentation/devicetree/bindings/net/fsl-fec.txt
>b/Documentation/devicetree/bindings/net/fsl-fec.txt
>index 6f55bdd..1766579 100644
>--- a/Documentation/devicetree/bindings/net/fsl-fec.txt
>+++ b/Documentation/devicetree/bindings/net/fsl-fec.txt
>@@ -23,6 +23,9 @@ Optional properties:
> - phy-handle : phandle to the PHY device connected to this device.
> - fixed-link : Assume a fixed link. See fixed-link.txt in the same directory.
>   Use instead of phy-handle.
>+- phy-reset-after-clk-enable : If present then a phy reset and
>+configuration
>+  will be performed everytime after the clocks are enabled. This is
>+required
>+  for some PHYs to work properly.
> - fsl,num-tx-queues : The property is valid for enet-avb IP, which supports
>   hw multi queues. Should specify the tx queue number, otherwise set tx
>queue
>   number to 1.
>diff --git a/drivers/net/ethernet/freescale/fec.h
>b/drivers/net/ethernet/freescale/fec.h
>index 3da8084..d4f658a 100644
>--- a/drivers/net/ethernet/freescale/fec.h
>+++ b/drivers/net/ethernet/freescale/fec.h
>@@ -538,6 +538,7 @@ struct fec_enet_private {
>   int phy_reset_duration;
>   int phy_reset_post_delay;
>   boolphy_reset_active_high;
>+  boolphy_reset_after_clk_enable;
>
>   struct  napi_struct napi;
>   int csum_flags;
>diff --git a/drivers/net/ethernet/freescale/fec_main.c
>b/drivers/net/ethernet/freescale/fec_main.c
>index cbbf7b8..4e744f3 100644
>--- a/drivers/net/ethernet/freescale/fec_main.c
>+++ b/drivers/net/ethernet/freescale/fec_main.c
>@@ -68,6 +68,7 @@
>
> static void set_multicast_list(struct net_device *ndev);  static void
>fec_enet_itr_coal_init(struct net_device *ndev);
>+static int fec_reset_phy(struct net_device *ndev);
>
> #define DRIVER_NAME   "fec"
>
>@@ -1862,6 +1863,18 @@ static int fec_enet_clk_enable(struct net_device
>*ndev, bool enable)
>   ret = clk_prepare_enable(fep->clk_ref);
>   if (ret)
>   goto failed_clk_ref;
>+
>+  /* reset the phy after clk is enabled if it's configured */
>+  if (fep->phy_reset_after_clk_enable) {
>+  ret = fec_reset_phy(ndev);
>+  if (ret)
>+  goto failed_reset;
>+  if (ndev->phydev) {
>+  ret = phy_init_hw(ndev->phydev);
>+  if (ret)
>+  goto failed_reset;
>+  }
>+  }

Since it is common issue so long 

RE: [PATCH 2/2] net: ethernet: fsl: add phy reset after clk enable option

[Andy Duan]

From: Richard Leitner  Sent: Thursday, July 06, 
2017 9:06 PM
>To: Andy Duan ; robh...@kernel.org;
>mark.rutl...@arm.com
>Cc: net...@vger.kernel.org; devicet...@vger.kernel.org; linux-
>ker...@vger.kernel.org; d...@g0hl1n.net; Richard Leitner
>
>Subject: [PATCH 2/2] net: ethernet: fsl: add phy reset after clk enable option
>
>Some PHYs (for example the LAN8710) doesn't allow turning the clocks off and
>on again without reset (according to their datasheet). Exactly this behaviour
>was introduced for power saving reasons by commit e8fcfcd5684a
>("net: fec: optimize the clock management to save power") Therefore add a
>devictree option to perform a PHY reset and configuration after every clock
>enable.
>
>For a better understanding here's a outline of the time response of the clock
>and reset lines before and after this patch:
>
> v--fec_probe()  v--fec_enet_open()
> v   v
>   w/o patch eCLK:
>___|
>   w/o patch nRST: __--
>   w/o patch CONF:
>___XX___
>
>   w/  patch eCLK:
>___|
>   w/  patch nRST: __--__--
>   w/  patch CONF:
>___XX__XX___
> ^   ^
> ^--fec_probe()  ^--fec_enet_open()
>
>In our case this problem does occur at about every 10th to 50th POR of an
>LAN8710 connected to an i.MX6 SoC. The typical symptom of this problem is a
>"swinging" ethernet link. Similar issues were experienced by users of the NXP
>forum:
>   https://community.nxp.com/thread/389902
>   https://community.nxp.com/message/309354
>With this patch applied the issue didn't occur for at least a few hundred PORs
>of our board.
>
>Fixes: e8fcfcd5684a ("net: fec: optimize the clock management to sa...")
>Signed-off-by: Richard Leitner 
>---
> Documentation/devicetree/bindings/net/fsl-fec.txt |  3 +++
> drivers/net/ethernet/freescale/fec.h  |  1 +
> drivers/net/ethernet/freescale/fec_main.c | 16 
> 3 files changed, 20 insertions(+)
>
>diff --git a/Documentation/devicetree/bindings/net/fsl-fec.txt
>b/Documentation/devicetree/bindings/net/fsl-fec.txt
>index 6f55bdd..1766579 100644
>--- a/Documentation/devicetree/bindings/net/fsl-fec.txt
>+++ b/Documentation/devicetree/bindings/net/fsl-fec.txt
>@@ -23,6 +23,9 @@ Optional properties:
> - phy-handle : phandle to the PHY device connected to this device.
> - fixed-link : Assume a fixed link. See fixed-link.txt in the same directory.
>   Use instead of phy-handle.
>+- phy-reset-after-clk-enable : If present then a phy reset and
>+configuration
>+  will be performed everytime after the clocks are enabled. This is
>+required
>+  for some PHYs to work properly.
> - fsl,num-tx-queues : The property is valid for enet-avb IP, which supports
>   hw multi queues. Should specify the tx queue number, otherwise set tx
>queue
>   number to 1.
>diff --git a/drivers/net/ethernet/freescale/fec.h
>b/drivers/net/ethernet/freescale/fec.h
>index 3da8084..d4f658a 100644
>--- a/drivers/net/ethernet/freescale/fec.h
>+++ b/drivers/net/ethernet/freescale/fec.h
>@@ -538,6 +538,7 @@ struct fec_enet_private {
>   int phy_reset_duration;
>   int phy_reset_post_delay;
>   boolphy_reset_active_high;
>+  boolphy_reset_after_clk_enable;
>
>   struct  napi_struct napi;
>   int csum_flags;
>diff --git a/drivers/net/ethernet/freescale/fec_main.c
>b/drivers/net/ethernet/freescale/fec_main.c
>index cbbf7b8..4e744f3 100644
>--- a/drivers/net/ethernet/freescale/fec_main.c
>+++ b/drivers/net/ethernet/freescale/fec_main.c
>@@ -68,6 +68,7 @@
>
> static void set_multicast_list(struct net_device *ndev);  static void
>fec_enet_itr_coal_init(struct net_device *ndev);
>+static int fec_reset_phy(struct net_device *ndev);
>
> #define DRIVER_NAME   "fec"
>
>@@ -1862,6 +1863,18 @@ static int fec_enet_clk_enable(struct net_device
>*ndev, bool enable)
>   ret = clk_prepare_enable(fep->clk_ref);
>   if (ret)
>   goto failed_clk_ref;
>+
>+  /* reset the phy after clk is enabled if it's configured */
>+  if (fep->phy_reset_after_clk_enable) {
>+  ret = fec_reset_phy(ndev);
>+  if (ret)
>+  goto failed_reset;
>+  if (ndev->phydev) {
>+  ret = phy_init_hw(ndev->phydev);
>+  if (ret)
>+  goto failed_reset;
>+  }
>+  }

Since it is common issue so long as using the PHY, can you move it into smsc 
phy driver like in .smsc_phy_reset() function ?
And get the 

Re: [PATCH v2 3/6] cpufreq: schedutil: ensure max frequency while running RT/DL tasks

[Joel Fernandes]

Hi,

On Wed, Jul 5, 2017 at 6:41 AM, Patrick Bellasi  wrote:
[..]
>
>> But what about clearing the sched-class's flag from .pick_next_task() 
>> callback
>> when they return NULL ?
>>
>> What about something like this instead (completely untested), with which we
>> don't need the 2/3 patch as well:
>
> Just had a fast check but I think something like that can work.
> We had an internal discussion with Brendan (in CC now) which had a
> similar proposal.
>
> Main counter arguments for me was:
> 1. we wanna to reduce the pollution of scheduling classes code with
>schedutil specific code, unless strictly necessary
> 2. we never had a "clear bit" semantics for flags updates
>
> Thus this proposal seemed to me less of a discontinuity wrt the
> current interface. However, something similar to what you propose
> below should also work. Let's collect some more feedback...
>

I was going to say something similar. I feel its much cleaner if the
scheduler clears the flags that it set, thus taking "ownership" of the
flag. I feel that will avoid complication like this where the governor
has to peek into what's currently running and such (and also help with
the suggestion I made for patch 2/3.

Maybe the interface needs an extension for "clear flag" semantics?

thanks,

-Joel

Re: [PATCH v2 3/6] cpufreq: schedutil: ensure max frequency while running RT/DL tasks

[Joel Fernandes]

Hi,

On Wed, Jul 5, 2017 at 6:41 AM, Patrick Bellasi  wrote:
[..]
>
>> But what about clearing the sched-class's flag from .pick_next_task() 
>> callback
>> when they return NULL ?
>>
>> What about something like this instead (completely untested), with which we
>> don't need the 2/3 patch as well:
>
> Just had a fast check but I think something like that can work.
> We had an internal discussion with Brendan (in CC now) which had a
> similar proposal.
>
> Main counter arguments for me was:
> 1. we wanna to reduce the pollution of scheduling classes code with
>schedutil specific code, unless strictly necessary
> 2. we never had a "clear bit" semantics for flags updates
>
> Thus this proposal seemed to me less of a discontinuity wrt the
> current interface. However, something similar to what you propose
> below should also work. Let's collect some more feedback...
>

I was going to say something similar. I feel its much cleaner if the
scheduler clears the flags that it set, thus taking "ownership" of the
flag. I feel that will avoid complication like this where the governor
has to peek into what's currently running and such (and also help with
the suggestion I made for patch 2/3.

Maybe the interface needs an extension for "clear flag" semantics?

thanks,

-Joel

Re: [git pull] vfs.git pile 10

[Linus Torvalds]

On Thu, Jul 6, 2017 at 2:11 AM, Al Viro  wrote:
> uaccess str...() dead code removals.

Side note: you left a couple of references to strlen_user() still in the tree.

None of them *matter* (two comments and one declaration for the
function that no longer exists), but it just strikes me as a bit of an
odd oversight.

Are those removals perhaps in other trees (and you didn't want to
clash) or was it literally just "oops, I caught almost everything
except for those three cases"?

 Linus

Re: [git pull] vfs.git pile 10

[Linus Torvalds]

On Thu, Jul 6, 2017 at 2:11 AM, Al Viro  wrote:
> uaccess str...() dead code removals.

Side note: you left a couple of references to strlen_user() still in the tree.

None of them *matter* (two comments and one declaration for the
function that no longer exists), but it just strikes me as a bit of an
odd oversight.

Are those removals perhaps in other trees (and you didn't want to
clash) or was it literally just "oops, I caught almost everything
except for those three cases"?

 Linus

Re: [RFC][PATCH] exec: Use init rlimits for setuid exec

[Kees Cook]

On Thu, Jul 6, 2017 at 10:10 PM, Kees Cook  wrote:
> On Thu, Jul 6, 2017 at 9:48 PM, Andy Lutomirski  wrote:
>> How about a much simpler solution: don't read rlimit at all in
>> copy_strings(), let alone try to enforce it.  Instead, just before the
>> point of no return, check how much stack space is already used and, if
>> it's more than an appropriate threshold (e.g. 1/4 of the rlimit),
>> abort.  Sure, this adds overhead if we're going to abort, but does
>> that really matter?
>
> We should avoid using up tons of memory and then failing. Better to
> cap it as we use it. Plumbing a sane value into this shouldn't be hard
> at all. Just making this a hardcoded 2MB seems sane (1/4 of 8MB).
>
>> I don't see why using rlimit for layout control makes any sense
>> whatsoever.  Is there some historical reason we need that?  As far as
>> I can see (on insufficient inspection) is that the kernel is trying to
>> guarantee that, if we have so much arg crap that our remaining stack
>> is less than 128k, then we don't exceed our limit by a little bit.
>
> IIUC, this is a big deal on 32-bit. Unlimited stack triggers top-down
> mmap instead of bottom-up. I mean, I'd be delighted to get rid of
> this, but I thought it was relied on by userspace.

I always say this backwards. :P Default is top-down (allocate at high
addresses and work down toward low). With unlimited stack, allocations
start at low addresses and work up. Here's the results (shown with
randomize_va_space sysctl set to 0):

$ ulimit -s
8192
$ cat /proc/self/maps
08048000-0805 r-xp  fc:01 843/bin/cat
0805-08051000 r--p 7000 fc:01 843/bin/cat
08051000-08052000 rw-p 8000 fc:01 843/bin/cat
08052000-08073000 rw-p  00:00 0  [heap]
b7be7000-b7de7000 r--p  fc:01 403307 /usr/lib/locale/locale-archive
b7de7000-b7f9a000 r-xp  fc:01 276980
/lib/i386-linux-gnu/libc-2.24.so
b7f9a000-b7f9b000 ---p 001b3000 fc:01 276980
/lib/i386-linux-gnu/libc-2.24.so
b7f9b000-b7f9d000 r--p 001b3000 fc:01 276980
/lib/i386-linux-gnu/libc-2.24.so
b7f9d000-b7f9e000 rw-p 001b5000 fc:01 276980
/lib/i386-linux-gnu/libc-2.24.so
b7f9e000-b7fa1000 rw-p  00:00 0
b7fb2000-b7fd7000 rw-p  00:00 0
b7fd7000-b7fd9000 r--p  00:00 0  [vvar]
b7fd9000-b7fdb000 r-xp  00:00 0  [vdso]
b7fdb000-b7ffd000 r-xp  fc:01 276959 /lib/i386-linux-gnu/ld-2.24.so
b7ffd000-b7ffe000 r--p 002d7000 fc:01 403307 /usr/lib/locale/locale-archive
b7ffe000-b7fff000 r--p 00022000 fc:01 276959 /lib/i386-linux-gnu/ld-2.24.so
b7fff000-b800 rw-p 00023000 fc:01 276959 /lib/i386-linux-gnu/ld-2.24.so
bffdf000-c000 rw-p  00:00 0  [stack]
$ ulimit -s unlimited
$ cat /proc/self/maps
08048000-0805 r-xp  fc:01 843/bin/cat
0805-08051000 r--p 7000 fc:01 843/bin/cat
08051000-08052000 rw-p 8000 fc:01 843/bin/cat
08052000-08073000 rw-p  00:00 0  [heap]
4000-40022000 r-xp  fc:01 276959 /lib/i386-linux-gnu/ld-2.24.so
40022000-40023000 r--p 002d7000 fc:01 403307 /usr/lib/locale/locale-archive
40023000-40024000 r--p 00022000 fc:01 276959 /lib/i386-linux-gnu/ld-2.24.so
40024000-40025000 rw-p 00023000 fc:01 276959 /lib/i386-linux-gnu/ld-2.24.so
40025000-40027000 r--p  00:00 0  [vvar]
40027000-40029000 r-xp  00:00 0  [vdso]
40029000-4004e000 rw-p  00:00 0
4005f000-40212000 r-xp  fc:01 276980
/lib/i386-linux-gnu/libc-2.24.so
40212000-40213000 ---p 001b3000 fc:01 276980
/lib/i386-linux-gnu/libc-2.24.so
40213000-40215000 r--p 001b3000 fc:01 276980
/lib/i386-linux-gnu/libc-2.24.so
40215000-40216000 rw-p 001b5000 fc:01 276980
/lib/i386-linux-gnu/libc-2.24.so
40216000-40219000 rw-p  00:00 0
40219000-40419000 r--p  fc:01 403307 /usr/lib/locale/locale-archive
bffdf000-c000 rw-p  00:00 0  [stack]


-- 
Kees Cook
Pixel Security

Re: [RFC][PATCH] exec: Use init rlimits for setuid exec

[Kees Cook]

On Thu, Jul 6, 2017 at 10:10 PM, Kees Cook  wrote:
> On Thu, Jul 6, 2017 at 9:48 PM, Andy Lutomirski  wrote:
>> How about a much simpler solution: don't read rlimit at all in
>> copy_strings(), let alone try to enforce it.  Instead, just before the
>> point of no return, check how much stack space is already used and, if
>> it's more than an appropriate threshold (e.g. 1/4 of the rlimit),
>> abort.  Sure, this adds overhead if we're going to abort, but does
>> that really matter?
>
> We should avoid using up tons of memory and then failing. Better to
> cap it as we use it. Plumbing a sane value into this shouldn't be hard
> at all. Just making this a hardcoded 2MB seems sane (1/4 of 8MB).
>
>> I don't see why using rlimit for layout control makes any sense
>> whatsoever.  Is there some historical reason we need that?  As far as
>> I can see (on insufficient inspection) is that the kernel is trying to
>> guarantee that, if we have so much arg crap that our remaining stack
>> is less than 128k, then we don't exceed our limit by a little bit.
>
> IIUC, this is a big deal on 32-bit. Unlimited stack triggers top-down
> mmap instead of bottom-up. I mean, I'd be delighted to get rid of
> this, but I thought it was relied on by userspace.

I always say this backwards. :P Default is top-down (allocate at high
addresses and work down toward low). With unlimited stack, allocations
start at low addresses and work up. Here's the results (shown with
randomize_va_space sysctl set to 0):

$ ulimit -s
8192
$ cat /proc/self/maps
08048000-0805 r-xp  fc:01 843/bin/cat
0805-08051000 r--p 7000 fc:01 843/bin/cat
08051000-08052000 rw-p 8000 fc:01 843/bin/cat
08052000-08073000 rw-p  00:00 0  [heap]
b7be7000-b7de7000 r--p  fc:01 403307 /usr/lib/locale/locale-archive
b7de7000-b7f9a000 r-xp  fc:01 276980
/lib/i386-linux-gnu/libc-2.24.so
b7f9a000-b7f9b000 ---p 001b3000 fc:01 276980
/lib/i386-linux-gnu/libc-2.24.so
b7f9b000-b7f9d000 r--p 001b3000 fc:01 276980
/lib/i386-linux-gnu/libc-2.24.so
b7f9d000-b7f9e000 rw-p 001b5000 fc:01 276980
/lib/i386-linux-gnu/libc-2.24.so
b7f9e000-b7fa1000 rw-p  00:00 0
b7fb2000-b7fd7000 rw-p  00:00 0
b7fd7000-b7fd9000 r--p  00:00 0  [vvar]
b7fd9000-b7fdb000 r-xp  00:00 0  [vdso]
b7fdb000-b7ffd000 r-xp  fc:01 276959 /lib/i386-linux-gnu/ld-2.24.so
b7ffd000-b7ffe000 r--p 002d7000 fc:01 403307 /usr/lib/locale/locale-archive
b7ffe000-b7fff000 r--p 00022000 fc:01 276959 /lib/i386-linux-gnu/ld-2.24.so
b7fff000-b800 rw-p 00023000 fc:01 276959 /lib/i386-linux-gnu/ld-2.24.so
bffdf000-c000 rw-p  00:00 0  [stack]
$ ulimit -s unlimited
$ cat /proc/self/maps
08048000-0805 r-xp  fc:01 843/bin/cat
0805-08051000 r--p 7000 fc:01 843/bin/cat
08051000-08052000 rw-p 8000 fc:01 843/bin/cat
08052000-08073000 rw-p  00:00 0  [heap]
4000-40022000 r-xp  fc:01 276959 /lib/i386-linux-gnu/ld-2.24.so
40022000-40023000 r--p 002d7000 fc:01 403307 /usr/lib/locale/locale-archive
40023000-40024000 r--p 00022000 fc:01 276959 /lib/i386-linux-gnu/ld-2.24.so
40024000-40025000 rw-p 00023000 fc:01 276959 /lib/i386-linux-gnu/ld-2.24.so
40025000-40027000 r--p  00:00 0  [vvar]
40027000-40029000 r-xp  00:00 0  [vdso]
40029000-4004e000 rw-p  00:00 0
4005f000-40212000 r-xp  fc:01 276980
/lib/i386-linux-gnu/libc-2.24.so
40212000-40213000 ---p 001b3000 fc:01 276980
/lib/i386-linux-gnu/libc-2.24.so
40213000-40215000 r--p 001b3000 fc:01 276980
/lib/i386-linux-gnu/libc-2.24.so
40215000-40216000 rw-p 001b5000 fc:01 276980
/lib/i386-linux-gnu/libc-2.24.so
40216000-40219000 rw-p  00:00 0
40219000-40419000 r--p  fc:01 403307 /usr/lib/locale/locale-archive
bffdf000-c000 rw-p  00:00 0  [stack]


-- 
Kees Cook
Pixel Security

[PATCH] HID: hid-logitech-hidpp: add NULL check on devm_kmemdup() return value

[Gustavo A. R. Silva]

Check return value from call to devm_kmemdup()
in order to prevent a NULL pointer dereference.

Signed-off-by: Gustavo A. R. Silva 
---
 drivers/hid/hid-logitech-hidpp.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 41b3946..501e16a 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -2732,6 +2732,9 @@ static int hidpp_initialize_battery(struct hidpp_device 
*hidpp)
 hidpp_battery_props,
 sizeof(hidpp_battery_props),
 GFP_KERNEL);
+   if (!battery_props)
+   return -ENOMEM;
+
num_battery_props = ARRAY_SIZE(hidpp_battery_props) - 2;
 
if (hidpp->capabilities & HIDPP_CAPABILITY_BATTERY_MILEAGE)
-- 
2.5.0

[PATCH] HID: hid-logitech-hidpp: add NULL check on devm_kmemdup() return value

[Gustavo A. R. Silva]

Check return value from call to devm_kmemdup()
in order to prevent a NULL pointer dereference.

Signed-off-by: Gustavo A. R. Silva 
---
 drivers/hid/hid-logitech-hidpp.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 41b3946..501e16a 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -2732,6 +2732,9 @@ static int hidpp_initialize_battery(struct hidpp_device 
*hidpp)
 hidpp_battery_props,
 sizeof(hidpp_battery_props),
 GFP_KERNEL);
+   if (!battery_props)
+   return -ENOMEM;
+
num_battery_props = ARRAY_SIZE(hidpp_battery_props) - 2;
 
if (hidpp->capabilities & HIDPP_CAPABILITY_BATTERY_MILEAGE)
-- 
2.5.0

Re: [PATCH] perf report: Fix broken arrow at row 0 in annotate view

[Jin, Yao]

Hi Arnaldo,

Could this patch be merged?

Otherwise the jump arrow is broken when it's displayed at the row 0 in 
annotate view.


Thanks

Jin Yao

On 6/8/2017 2:01 PM, Jin Yao wrote:

When the jump instruction is displayed at the row 0 in annotate view,
the arrow is broken. An example:

  16.86 │   ┌──je 82
   0.01 │  movsd  (%rsp),%xmm0
│  movsd  0x8(%rsp),%xmm4
│  movsd  0x8(%rsp),%xmm1
│  movsd  (%rsp),%xmm3
│  divsd  %xmm4,%xmm0
│  divsd  %xmm3,%xmm1
│  movsd  (%rsp),%xmm2
│  addsd  %xmm1,%xmm0
│  addsd  %xmm2,%xmm0
│  movsd  %xmm0,(%rsp)
│82:   sub$0x1,%ebx
  83.03 │↑ jne38
│  add$0x10,%rsp
│  xor%eax,%eax
│  pop%rbx
│← retq

The patch increments the row number before checking with 0.

Signed-off-by: Jin Yao 
---
  tools/perf/ui/browser.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/ui/browser.c b/tools/perf/ui/browser.c
index a4d3762..83874b0 100644
--- a/tools/perf/ui/browser.c
+++ b/tools/perf/ui/browser.c
@@ -704,7 +704,7 @@ static void __ui_browser__line_arrow_down(struct ui_browser 
*browser,
ui_browser__gotorc(browser, row, column + 1);
SLsmg_draw_hline(2);
  
-		if (row++ == 0)

+   if (++row == 0)
goto out;
} else
row = 0;

Re: [PATCH] perf report: Fix broken arrow at row 0 in annotate view

[Jin, Yao]

Hi Arnaldo,

Could this patch be merged?

Otherwise the jump arrow is broken when it's displayed at the row 0 in 
annotate view.


Thanks

Jin Yao

On 6/8/2017 2:01 PM, Jin Yao wrote:

When the jump instruction is displayed at the row 0 in annotate view,
the arrow is broken. An example:

  16.86 │   ┌──je 82
   0.01 │  movsd  (%rsp),%xmm0
│  movsd  0x8(%rsp),%xmm4
│  movsd  0x8(%rsp),%xmm1
│  movsd  (%rsp),%xmm3
│  divsd  %xmm4,%xmm0
│  divsd  %xmm3,%xmm1
│  movsd  (%rsp),%xmm2
│  addsd  %xmm1,%xmm0
│  addsd  %xmm2,%xmm0
│  movsd  %xmm0,(%rsp)
│82:   sub$0x1,%ebx
  83.03 │↑ jne38
│  add$0x10,%rsp
│  xor%eax,%eax
│  pop%rbx
│← retq

The patch increments the row number before checking with 0.

Signed-off-by: Jin Yao 
---
  tools/perf/ui/browser.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/ui/browser.c b/tools/perf/ui/browser.c
index a4d3762..83874b0 100644
--- a/tools/perf/ui/browser.c
+++ b/tools/perf/ui/browser.c
@@ -704,7 +704,7 @@ static void __ui_browser__line_arrow_down(struct ui_browser 
*browser,
ui_browser__gotorc(browser, row, column + 1);
SLsmg_draw_hline(2);
  
-		if (row++ == 0)

+   if (++row == 0)
goto out;
} else
row = 0;

Re: [RFC][PATCH] exec: Use init rlimits for setuid exec

[Kees Cook]

On Thu, Jul 6, 2017 at 9:48 PM, Andy Lutomirski  wrote:
> How about a much simpler solution: don't read rlimit at all in
> copy_strings(), let alone try to enforce it.  Instead, just before the
> point of no return, check how much stack space is already used and, if
> it's more than an appropriate threshold (e.g. 1/4 of the rlimit),
> abort.  Sure, this adds overhead if we're going to abort, but does
> that really matter?

We should avoid using up tons of memory and then failing. Better to
cap it as we use it. Plumbing a sane value into this shouldn't be hard
at all. Just making this a hardcoded 2MB seems sane (1/4 of 8MB).

> I don't see why using rlimit for layout control makes any sense
> whatsoever.  Is there some historical reason we need that?  As far as
> I can see (on insufficient inspection) is that the kernel is trying to
> guarantee that, if we have so much arg crap that our remaining stack
> is less than 128k, then we don't exceed our limit by a little bit.

IIUC, this is a big deal on 32-bit. Unlimited stack triggers top-down
mmap instead of bottom-up. I mean, I'd be delighted to get rid of
this, but I thought it was relied on by userspace.

-Kees

-- 
Kees Cook
Pixel Security

Re: [RFC][PATCH] exec: Use init rlimits for setuid exec

[Kees Cook]

On Thu, Jul 6, 2017 at 9:48 PM, Andy Lutomirski  wrote:
> How about a much simpler solution: don't read rlimit at all in
> copy_strings(), let alone try to enforce it.  Instead, just before the
> point of no return, check how much stack space is already used and, if
> it's more than an appropriate threshold (e.g. 1/4 of the rlimit),
> abort.  Sure, this adds overhead if we're going to abort, but does
> that really matter?

We should avoid using up tons of memory and then failing. Better to
cap it as we use it. Plumbing a sane value into this shouldn't be hard
at all. Just making this a hardcoded 2MB seems sane (1/4 of 8MB).

> I don't see why using rlimit for layout control makes any sense
> whatsoever.  Is there some historical reason we need that?  As far as
> I can see (on insufficient inspection) is that the kernel is trying to
> guarantee that, if we have so much arg crap that our remaining stack
> is less than 128k, then we don't exceed our limit by a little bit.

IIUC, this is a big deal on 32-bit. Unlimited stack triggers top-down
mmap instead of bottom-up. I mean, I'd be delighted to get rid of
this, but I thought it was relied on by userspace.

-Kees

-- 
Kees Cook
Pixel Security

Re: [git pull] vfs.git pile 11

[Linus Torvalds]

On Thu, Jul 6, 2017 at 2:20 PM, Al Viro  wrote:
>
> Linus, could you hold that one back until tomorrow?  I want to tweak the
> last commit in there a bit, but I want to give it a local beating first...

Ok, dropping this one. All your other branches are merged now.

Linus

Re: [git pull] vfs.git pile 11

[Linus Torvalds]

On Thu, Jul 6, 2017 at 2:20 PM, Al Viro  wrote:
>
> Linus, could you hold that one back until tomorrow?  I want to tweak the
> last commit in there a bit, but I want to give it a local beating first...

Ok, dropping this one. All your other branches are merged now.

Linus

Re: [PATCH] PCI: Do not enable extended tags on pre-dated (v1.x) systems

[Jike Song]

On Wed, Jul 5, 2017 at 9:19 PM, Sinan Kaya  wrote:
> According to extended tags ECN document, all PCIe receivers are expected
> to support extended tags support. It should be safe to enable extended
> tags on endpoints without checking compatibility.
>
> This assumption seems to be working fine except for the legacy systems.
> The ECN has been written against PCIE spec version 2.0. Therefore, we need
> to exclude all version 1.0 devices from this change as there is HW out
> there that can't handle extended tags.
>
> Note that the default value of Extended Tags Enable bit is implementation
> specific. Therefore, we are clearing the bit by default when incompatible
> HW is found without assuming that value is zero.
>
> Reported-by: Wim ten Have 
> Link: 
> https://pcisig.com/sites/default/files/specification_documents/ECN_Extended_Tag_Enable_Default_05Sept2008_final.pdf
> Link: https://bugzilla.redhat.com/show_bug.cgi?id=1467674
> Fixes: 60db3a4d8cc9 ("PCI: Enable PCIe Extended Tags if supported")
> Tested-by: Wim ten Have 
> Signed-off-by: Sinan Kaya 
> ---
>  drivers/pci/probe.c | 52 +---
>  1 file changed, 45 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 19c8950..5e39013 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -1684,21 +1684,58 @@ static void program_hpp_type2(struct pci_dev *dev, 
> struct hpp_type2 *hpp)
>  */
>  }
>
> -static void pci_configure_extended_tags(struct pci_dev *dev)
> +static bool pcie_bus_exttags_supported(struct pci_bus *bus)
> +{
> +   bool exttags_supported = true;
> +   struct pci_dev *bridge;
> +   int rc;
> +   u16 flags;
> +
> +   bridge = bus->self;
> +   while (bridge) {
> +   if (pci_is_pcie(bridge)) {
> +   rc = pcie_capability_read_word(bridge, PCI_EXP_FLAGS,
> +  );
> +   if (!rc && ((flags & PCI_EXP_FLAGS_VERS) < 2)) {
> +   exttags_supported = false;
> +   break;
> +   }
> +   }
> +   if (!bridge->bus->parent)
> +   break;
> +   bridge = bridge->bus->parent->self;
> +   }
> +
> +   return exttags_supported;
> +}
> +
> +static int pcie_bus_configure_exttags(struct pci_dev *dev, void *data)
>  {
> u32 dev_cap;
> int ret;
> +   bool supported;
>
> if (!pci_is_pcie(dev))
> -   return;
> +   return 0;
>
> ret = pcie_capability_read_dword(dev, PCI_EXP_DEVCAP, _cap);
> if (ret)
> -   return;
> +   return 0;
>
> -   if (dev_cap & PCI_EXP_DEVCAP_EXT_TAG)
> -   pcie_capability_set_word(dev, PCI_EXP_DEVCTL,
> -PCI_EXP_DEVCTL_EXT_TAG);
> +   if (dev_cap & PCI_EXP_DEVCAP_EXT_TAG) {
> +   supported = pcie_bus_exttags_supported(dev->bus);
> +

Maybe checking the version of this endpoint at first? Do you expect a
v1 endpoint
to be working under v2+ ports?

-- 
Thanks,
Jike

Re: [RFC][PATCH] exec: Use init rlimits for setuid exec

[Linus Torvalds]

On Thu, Jul 6, 2017 at 9:48 PM, Andy Lutomirski  wrote:
>
> How about a much simpler solution: don't read rlimit at all in
> copy_strings(), let alone try to enforce it.

People have historically relied on E2BIG and then splitting things
into multiple chunks (ie do the whole 'xargs' thing).

But I agree that *if* we abort cleanly with E2BIG, then it's fine if
we copy a bit too much first. It's just that we need to check early
enough that an E2BIG error is possible (ie not after the "point of no
return").

And yes, I think we can get rid of the layout differences based on
rlimit. After all, if I read that right the thing currently clamps
that "gap" thing to 128MB anyway (and that's on the *low* side).

 Linus

Re: [RFC][PATCH] exec: Use init rlimits for setuid exec

[Linus Torvalds]

On Thu, Jul 6, 2017 at 9:48 PM, Andy Lutomirski  wrote:
>
> How about a much simpler solution: don't read rlimit at all in
> copy_strings(), let alone try to enforce it.

People have historically relied on E2BIG and then splitting things
into multiple chunks (ie do the whole 'xargs' thing).

But I agree that *if* we abort cleanly with E2BIG, then it's fine if
we copy a bit too much first. It's just that we need to check early
enough that an E2BIG error is possible (ie not after the "point of no
return").

And yes, I think we can get rid of the layout differences based on
rlimit. After all, if I read that right the thing currently clamps
that "gap" thing to 128MB anyway (and that's on the *low* side).

 Linus

Re: [PATCH] PCI: Do not enable extended tags on pre-dated (v1.x) systems

[Jike Song]

On Wed, Jul 5, 2017 at 9:19 PM, Sinan Kaya  wrote:
> According to extended tags ECN document, all PCIe receivers are expected
> to support extended tags support. It should be safe to enable extended
> tags on endpoints without checking compatibility.
>
> This assumption seems to be working fine except for the legacy systems.
> The ECN has been written against PCIE spec version 2.0. Therefore, we need
> to exclude all version 1.0 devices from this change as there is HW out
> there that can't handle extended tags.
>
> Note that the default value of Extended Tags Enable bit is implementation
> specific. Therefore, we are clearing the bit by default when incompatible
> HW is found without assuming that value is zero.
>
> Reported-by: Wim ten Have 
> Link: 
> https://pcisig.com/sites/default/files/specification_documents/ECN_Extended_Tag_Enable_Default_05Sept2008_final.pdf
> Link: https://bugzilla.redhat.com/show_bug.cgi?id=1467674
> Fixes: 60db3a4d8cc9 ("PCI: Enable PCIe Extended Tags if supported")
> Tested-by: Wim ten Have 
> Signed-off-by: Sinan Kaya 
> ---
>  drivers/pci/probe.c | 52 +---
>  1 file changed, 45 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 19c8950..5e39013 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -1684,21 +1684,58 @@ static void program_hpp_type2(struct pci_dev *dev, 
> struct hpp_type2 *hpp)
>  */
>  }
>
> -static void pci_configure_extended_tags(struct pci_dev *dev)
> +static bool pcie_bus_exttags_supported(struct pci_bus *bus)
> +{
> +   bool exttags_supported = true;
> +   struct pci_dev *bridge;
> +   int rc;
> +   u16 flags;
> +
> +   bridge = bus->self;
> +   while (bridge) {
> +   if (pci_is_pcie(bridge)) {
> +   rc = pcie_capability_read_word(bridge, PCI_EXP_FLAGS,
> +  );
> +   if (!rc && ((flags & PCI_EXP_FLAGS_VERS) < 2)) {
> +   exttags_supported = false;
> +   break;
> +   }
> +   }
> +   if (!bridge->bus->parent)
> +   break;
> +   bridge = bridge->bus->parent->self;
> +   }
> +
> +   return exttags_supported;
> +}
> +
> +static int pcie_bus_configure_exttags(struct pci_dev *dev, void *data)
>  {
> u32 dev_cap;
> int ret;
> +   bool supported;
>
> if (!pci_is_pcie(dev))
> -   return;
> +   return 0;
>
> ret = pcie_capability_read_dword(dev, PCI_EXP_DEVCAP, _cap);
> if (ret)
> -   return;
> +   return 0;
>
> -   if (dev_cap & PCI_EXP_DEVCAP_EXT_TAG)
> -   pcie_capability_set_word(dev, PCI_EXP_DEVCTL,
> -PCI_EXP_DEVCTL_EXT_TAG);
> +   if (dev_cap & PCI_EXP_DEVCAP_EXT_TAG) {
> +   supported = pcie_bus_exttags_supported(dev->bus);
> +

Maybe checking the version of this endpoint at first? Do you expect a
v1 endpoint
to be working under v2+ ports?

-- 
Thanks,
Jike

[PATCH v4 2/2] perf report: Implement visual marker for macro fusion in annotate

[Jin Yao]

For marking the fused instructions clearly, This patch adds a
line before the first instruction of pair and joins it with the
arrow of the jump.

For example, when je is selected in annotate view, the line
before cmpl is displayed and joins the arrow of je.

   │   ┌──cmpl   $0x0,argp_program_version_hook
 81.93 │   ├──je 20
   │   │  lock   cmpxchg %esi,0x38a9a4(%rip)
   │   │↓ jne29
   │   │↓ jmp43
 11.47 │20:└─→cmpxch %esi,0x38a999(%rip)

That means the cmpl+je is fused instruction pair and they should
be considered together.

Change-log:
---
v4: Since the first patch in patch set is changed, this is mainly
changed for compilation.

v3: Use Arnaldo's fix to let the display be better.
To get the evsel->evlist->env->cpuid, save the evsel in
annotate_browser.

v2: No more changes, just uses a new function "ins__is_fused"
to check if the instructions are fused.

v1: Initial post

Signed-off-by: Jin Yao 
---
 tools/perf/ui/browser.c   | 29 +
 tools/perf/ui/browser.h   |  2 ++
 tools/perf/ui/browsers/annotate.c | 26 ++
 tools/perf/util/annotate.c|  5 +
 tools/perf/util/annotate.h|  1 +
 5 files changed, 63 insertions(+)

diff --git a/tools/perf/ui/browser.c b/tools/perf/ui/browser.c
index a4d3762..9ef7677 100644
--- a/tools/perf/ui/browser.c
+++ b/tools/perf/ui/browser.c
@@ -738,6 +738,35 @@ void __ui_browser__line_arrow(struct ui_browser *browser, 
unsigned int column,
__ui_browser__line_arrow_down(browser, column, start, end);
 }
 
+void ui_browser__mark_fused(struct ui_browser *browser, unsigned int column,
+   unsigned int row, bool arrow_down)
+{
+   unsigned int end_row;
+
+   if (row >= browser->top_idx)
+   end_row = row - browser->top_idx;
+   else
+   return;
+
+   SLsmg_set_char_set(1);
+
+   if (arrow_down) {
+   ui_browser__gotorc(browser, end_row, column - 1);
+   SLsmg_write_char(SLSMG_ULCORN_CHAR);
+   ui_browser__gotorc(browser, end_row, column);
+   SLsmg_draw_hline(2);
+   ui_browser__gotorc(browser, end_row + 1, column - 1);
+   SLsmg_write_char(SLSMG_LTEE_CHAR);
+   } else {
+   ui_browser__gotorc(browser, end_row, column - 1);
+   SLsmg_write_char(SLSMG_LTEE_CHAR);
+   ui_browser__gotorc(browser, end_row, column);
+   SLsmg_draw_hline(2);
+   }
+
+   SLsmg_set_char_set(0);
+}
+
 void ui_browser__init(void)
 {
int i = 0;
diff --git a/tools/perf/ui/browser.h b/tools/perf/ui/browser.h
index be3b70e..a12eff7 100644
--- a/tools/perf/ui/browser.h
+++ b/tools/perf/ui/browser.h
@@ -43,6 +43,8 @@ void ui_browser__printf(struct ui_browser *browser, const 
char *fmt, ...);
 void ui_browser__write_graph(struct ui_browser *browser, int graph);
 void __ui_browser__line_arrow(struct ui_browser *browser, unsigned int column,
  u64 start, u64 end);
+void ui_browser__mark_fused(struct ui_browser *browser, unsigned int column,
+   unsigned int row, bool arrow_down);
 void __ui_browser__show_title(struct ui_browser *browser, const char *title);
 void ui_browser__show_title(struct ui_browser *browser, const char *title);
 int ui_browser__show(struct ui_browser *browser, const char *title,
diff --git a/tools/perf/ui/browsers/annotate.c 
b/tools/perf/ui/browsers/annotate.c
index ae05ebb..b376637 100644
--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -273,6 +273,25 @@ static bool disasm_line__is_valid_jump(struct disasm_line 
*dl, struct symbol *sy
return true;
 }
 
+static bool is_fused(struct annotate_browser *ab, struct disasm_line *cursor)
+{
+   struct disasm_line *pos = list_prev_entry(cursor, node);
+   const char *name;
+
+   if (!pos)
+   return false;
+
+   if (ins__is_lock(>ins))
+   name = pos->ops.locked.ins.name;
+   else
+   name = pos->ins.name;
+
+   if (!name || !cursor->ins.name)
+   return false;
+
+   return ins__is_fused(ab->arch, name, cursor->ins.name);
+}
+
 static void annotate_browser__draw_current_jump(struct ui_browser *browser)
 {
struct annotate_browser *ab = container_of(browser, struct 
annotate_browser, b);
@@ -308,6 +327,13 @@ static void annotate_browser__draw_current_jump(struct 
ui_browser *browser)
ui_browser__set_color(browser, HE_COLORSET_JUMP_ARROWS);
__ui_browser__line_arrow(browser, pcnt_width + 2 + ab->addr_width,
 from, to);
+
+   if (is_fused(ab, cursor)) {
+   ui_browser__mark_fused(browser,
+  pcnt_width + 3 + ab->addr_width,
+  from - 1,
+ 

[PATCH v4 2/2] perf report: Implement visual marker for macro fusion in annotate

[Jin Yao]

For marking the fused instructions clearly, This patch adds a
line before the first instruction of pair and joins it with the
arrow of the jump.

For example, when je is selected in annotate view, the line
before cmpl is displayed and joins the arrow of je.

   │   ┌──cmpl   $0x0,argp_program_version_hook
 81.93 │   ├──je 20
   │   │  lock   cmpxchg %esi,0x38a9a4(%rip)
   │   │↓ jne29
   │   │↓ jmp43
 11.47 │20:└─→cmpxch %esi,0x38a999(%rip)

That means the cmpl+je is fused instruction pair and they should
be considered together.

Change-log:
---
v4: Since the first patch in patch set is changed, this is mainly
changed for compilation.

v3: Use Arnaldo's fix to let the display be better.
To get the evsel->evlist->env->cpuid, save the evsel in
annotate_browser.

v2: No more changes, just uses a new function "ins__is_fused"
to check if the instructions are fused.

v1: Initial post

Signed-off-by: Jin Yao 
---
 tools/perf/ui/browser.c   | 29 +
 tools/perf/ui/browser.h   |  2 ++
 tools/perf/ui/browsers/annotate.c | 26 ++
 tools/perf/util/annotate.c|  5 +
 tools/perf/util/annotate.h|  1 +
 5 files changed, 63 insertions(+)

diff --git a/tools/perf/ui/browser.c b/tools/perf/ui/browser.c
index a4d3762..9ef7677 100644
--- a/tools/perf/ui/browser.c
+++ b/tools/perf/ui/browser.c
@@ -738,6 +738,35 @@ void __ui_browser__line_arrow(struct ui_browser *browser, 
unsigned int column,
__ui_browser__line_arrow_down(browser, column, start, end);
 }
 
+void ui_browser__mark_fused(struct ui_browser *browser, unsigned int column,
+   unsigned int row, bool arrow_down)
+{
+   unsigned int end_row;
+
+   if (row >= browser->top_idx)
+   end_row = row - browser->top_idx;
+   else
+   return;
+
+   SLsmg_set_char_set(1);
+
+   if (arrow_down) {
+   ui_browser__gotorc(browser, end_row, column - 1);
+   SLsmg_write_char(SLSMG_ULCORN_CHAR);
+   ui_browser__gotorc(browser, end_row, column);
+   SLsmg_draw_hline(2);
+   ui_browser__gotorc(browser, end_row + 1, column - 1);
+   SLsmg_write_char(SLSMG_LTEE_CHAR);
+   } else {
+   ui_browser__gotorc(browser, end_row, column - 1);
+   SLsmg_write_char(SLSMG_LTEE_CHAR);
+   ui_browser__gotorc(browser, end_row, column);
+   SLsmg_draw_hline(2);
+   }
+
+   SLsmg_set_char_set(0);
+}
+
 void ui_browser__init(void)
 {
int i = 0;
diff --git a/tools/perf/ui/browser.h b/tools/perf/ui/browser.h
index be3b70e..a12eff7 100644
--- a/tools/perf/ui/browser.h
+++ b/tools/perf/ui/browser.h
@@ -43,6 +43,8 @@ void ui_browser__printf(struct ui_browser *browser, const 
char *fmt, ...);
 void ui_browser__write_graph(struct ui_browser *browser, int graph);
 void __ui_browser__line_arrow(struct ui_browser *browser, unsigned int column,
  u64 start, u64 end);
+void ui_browser__mark_fused(struct ui_browser *browser, unsigned int column,
+   unsigned int row, bool arrow_down);
 void __ui_browser__show_title(struct ui_browser *browser, const char *title);
 void ui_browser__show_title(struct ui_browser *browser, const char *title);
 int ui_browser__show(struct ui_browser *browser, const char *title,
diff --git a/tools/perf/ui/browsers/annotate.c 
b/tools/perf/ui/browsers/annotate.c
index ae05ebb..b376637 100644
--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -273,6 +273,25 @@ static bool disasm_line__is_valid_jump(struct disasm_line 
*dl, struct symbol *sy
return true;
 }
 
+static bool is_fused(struct annotate_browser *ab, struct disasm_line *cursor)
+{
+   struct disasm_line *pos = list_prev_entry(cursor, node);
+   const char *name;
+
+   if (!pos)
+   return false;
+
+   if (ins__is_lock(>ins))
+   name = pos->ops.locked.ins.name;
+   else
+   name = pos->ins.name;
+
+   if (!name || !cursor->ins.name)
+   return false;
+
+   return ins__is_fused(ab->arch, name, cursor->ins.name);
+}
+
 static void annotate_browser__draw_current_jump(struct ui_browser *browser)
 {
struct annotate_browser *ab = container_of(browser, struct 
annotate_browser, b);
@@ -308,6 +327,13 @@ static void annotate_browser__draw_current_jump(struct 
ui_browser *browser)
ui_browser__set_color(browser, HE_COLORSET_JUMP_ARROWS);
__ui_browser__line_arrow(browser, pcnt_width + 2 + ab->addr_width,
 from, to);
+
+   if (is_fused(ab, cursor)) {
+   ui_browser__mark_fused(browser,
+  pcnt_width + 3 + ab->addr_width,
+  from - 1,
+  to > from ? true : 

[PATCH v4 1/2] perf util: Check for fused instruction

[Jin Yao]

Macro fusion merges two instructions to a single micro-op. Intel
core platform performs this hardware optimization under limited
circumstances.

For example, CMP + JCC can be "fused" and executed /retired
together. While with sampling this can result in the sample
sometimes being on the JCC and sometimes on the CMP.
So for the fused instruction pair, they could be considered
together.

On Nehalem, fused instruction pairs:
cmp/test + jcc.

On other new CPU:
cmp/test/add/sub/and/inc/dec + jcc.

This patch adds an x86-specific function which checks if 2
instructions are in a "fused" pair. For non-x86 arch, the
function is just NULL.

Change-log:
---
v4: Move the CPU model checking to symbol__disassemble and
save the CPU family/model in arch structure.

It avoids checking every time when jump arrow printed.

v3: Add checking for Nehalem (CMP, TEST). For other newer
Intel CPUs just check it by default (CMP, TEST, ADD,
SUB, AND, INC, DEC).

v2: Remove the origial weak function. Arnaldo points out
that doing it as a weak function that will be overridden
by the host arch doesn't work. So now it's implemented
as an arch-specific function.

v1: Initial post

Signed-off-by: Jin Yao 
---
 tools/perf/arch/x86/annotate/instructions.c | 46 +
 tools/perf/builtin-top.c|  2 +-
 tools/perf/ui/browsers/annotate.c   |  4 ++-
 tools/perf/ui/gtk/annotate.c|  2 +-
 tools/perf/util/annotate.c  | 22 --
 tools/perf/util/annotate.h  |  3 +-
 6 files changed, 73 insertions(+), 6 deletions(-)

diff --git a/tools/perf/arch/x86/annotate/instructions.c 
b/tools/perf/arch/x86/annotate/instructions.c
index c1625f2..d84b720 100644
--- a/tools/perf/arch/x86/annotate/instructions.c
+++ b/tools/perf/arch/x86/annotate/instructions.c
@@ -76,3 +76,49 @@ static struct ins x86__instructions[] = {
{ .name = "xbeginq",.ops = _ops, },
{ .name = "retq",   .ops = _ops,  },
 };
+
+static bool x86__ins_is_fused(struct arch *arch, const char *ins1,
+ const char *ins2)
+{
+   if (arch->family != 6 || arch->model < 0x1e || strstr(ins2, "jmp"))
+   return false;
+
+   if (arch->model == 0x1e) {
+   /* Nehalem */
+   if ((strstr(ins1, "cmp") && !strstr(ins1, "xchg")) ||
+strstr(ins1, "test")) {
+   return true;
+   }
+   } else {
+   /* Newer platform */
+   if ((strstr(ins1, "cmp") && !strstr(ins1, "xchg")) ||
+strstr(ins1, "test") ||
+strstr(ins1, "add") ||
+strstr(ins1, "sub") ||
+strstr(ins1, "and") ||
+strstr(ins1, "inc") ||
+strstr(ins1, "dec")) {
+   return true;
+   }
+   }
+
+   return false;
+}
+
+static int x86__cpuid_parse(struct arch *arch, char *cpuid)
+{
+   unsigned int family, model, stepping;
+   int ret;
+
+   /*
+* cpuid = "GenuineIntel,family,model,stepping"
+*/
+   ret = sscanf(cpuid, "%*[^,],%u,%u,%u", , , );
+   if (ret == 3) {
+   arch->family = family;
+   arch->model = model;
+   return 0;
+   }
+
+   return -1;
+}
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 6052376..022486d 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -134,7 +134,7 @@ static int perf_top__parse_source(struct perf_top *top, 
struct hist_entry *he)
return err;
}
 
-   err = symbol__disassemble(sym, map, NULL, 0, NULL);
+   err = symbol__disassemble(sym, map, NULL, 0, NULL, NULL);
if (err == 0) {
 out_assign:
top->sym_filter_entry = he;
diff --git a/tools/perf/ui/browsers/annotate.c 
b/tools/perf/ui/browsers/annotate.c
index 27f41f2..ae05ebb 100644
--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -9,6 +9,7 @@
 #include "../../util/symbol.h"
 #include "../../util/evsel.h"
 #include "../../util/config.h"
+#include "../../util/evlist.h"
 #include 
 #include 
 #include 
@@ -1074,7 +1075,8 @@ int symbol__tui_annotate(struct symbol *sym, struct map 
*map,
}
 
err = symbol__disassemble(sym, map, perf_evsel__env_arch(evsel),
- sizeof_bdl, );
+ sizeof_bdl, ,
+ evsel->evlist->env->cpuid);
if (err) {
char msg[BUFSIZ];
symbol__strerror_disassemble(sym, map, err, msg, sizeof(msg));
diff --git a/tools/perf/ui/gtk/annotate.c b/tools/perf/ui/gtk/annotate.c
index d903fd4..87e3760 100644
--- a/tools/perf/ui/gtk/annotate.c
+++ b/tools/perf/ui/gtk/annotate.c
@@ -169,7 +169,7 @@ static int symbol__gtk_annotate(struct 

[PATCH v4 1/2] perf util: Check for fused instruction

[Jin Yao]

Macro fusion merges two instructions to a single micro-op. Intel
core platform performs this hardware optimization under limited
circumstances.

For example, CMP + JCC can be "fused" and executed /retired
together. While with sampling this can result in the sample
sometimes being on the JCC and sometimes on the CMP.
So for the fused instruction pair, they could be considered
together.

On Nehalem, fused instruction pairs:
cmp/test + jcc.

On other new CPU:
cmp/test/add/sub/and/inc/dec + jcc.

This patch adds an x86-specific function which checks if 2
instructions are in a "fused" pair. For non-x86 arch, the
function is just NULL.

Change-log:
---
v4: Move the CPU model checking to symbol__disassemble and
save the CPU family/model in arch structure.

It avoids checking every time when jump arrow printed.

v3: Add checking for Nehalem (CMP, TEST). For other newer
Intel CPUs just check it by default (CMP, TEST, ADD,
SUB, AND, INC, DEC).

v2: Remove the origial weak function. Arnaldo points out
that doing it as a weak function that will be overridden
by the host arch doesn't work. So now it's implemented
as an arch-specific function.

v1: Initial post

Signed-off-by: Jin Yao 
---
 tools/perf/arch/x86/annotate/instructions.c | 46 +
 tools/perf/builtin-top.c|  2 +-
 tools/perf/ui/browsers/annotate.c   |  4 ++-
 tools/perf/ui/gtk/annotate.c|  2 +-
 tools/perf/util/annotate.c  | 22 --
 tools/perf/util/annotate.h  |  3 +-
 6 files changed, 73 insertions(+), 6 deletions(-)

diff --git a/tools/perf/arch/x86/annotate/instructions.c 
b/tools/perf/arch/x86/annotate/instructions.c
index c1625f2..d84b720 100644
--- a/tools/perf/arch/x86/annotate/instructions.c
+++ b/tools/perf/arch/x86/annotate/instructions.c
@@ -76,3 +76,49 @@ static struct ins x86__instructions[] = {
{ .name = "xbeginq",.ops = _ops, },
{ .name = "retq",   .ops = _ops,  },
 };
+
+static bool x86__ins_is_fused(struct arch *arch, const char *ins1,
+ const char *ins2)
+{
+   if (arch->family != 6 || arch->model < 0x1e || strstr(ins2, "jmp"))
+   return false;
+
+   if (arch->model == 0x1e) {
+   /* Nehalem */
+   if ((strstr(ins1, "cmp") && !strstr(ins1, "xchg")) ||
+strstr(ins1, "test")) {
+   return true;
+   }
+   } else {
+   /* Newer platform */
+   if ((strstr(ins1, "cmp") && !strstr(ins1, "xchg")) ||
+strstr(ins1, "test") ||
+strstr(ins1, "add") ||
+strstr(ins1, "sub") ||
+strstr(ins1, "and") ||
+strstr(ins1, "inc") ||
+strstr(ins1, "dec")) {
+   return true;
+   }
+   }
+
+   return false;
+}
+
+static int x86__cpuid_parse(struct arch *arch, char *cpuid)
+{
+   unsigned int family, model, stepping;
+   int ret;
+
+   /*
+* cpuid = "GenuineIntel,family,model,stepping"
+*/
+   ret = sscanf(cpuid, "%*[^,],%u,%u,%u", , , );
+   if (ret == 3) {
+   arch->family = family;
+   arch->model = model;
+   return 0;
+   }
+
+   return -1;
+}
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 6052376..022486d 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -134,7 +134,7 @@ static int perf_top__parse_source(struct perf_top *top, 
struct hist_entry *he)
return err;
}
 
-   err = symbol__disassemble(sym, map, NULL, 0, NULL);
+   err = symbol__disassemble(sym, map, NULL, 0, NULL, NULL);
if (err == 0) {
 out_assign:
top->sym_filter_entry = he;
diff --git a/tools/perf/ui/browsers/annotate.c 
b/tools/perf/ui/browsers/annotate.c
index 27f41f2..ae05ebb 100644
--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -9,6 +9,7 @@
 #include "../../util/symbol.h"
 #include "../../util/evsel.h"
 #include "../../util/config.h"
+#include "../../util/evlist.h"
 #include 
 #include 
 #include 
@@ -1074,7 +1075,8 @@ int symbol__tui_annotate(struct symbol *sym, struct map 
*map,
}
 
err = symbol__disassemble(sym, map, perf_evsel__env_arch(evsel),
- sizeof_bdl, );
+ sizeof_bdl, ,
+ evsel->evlist->env->cpuid);
if (err) {
char msg[BUFSIZ];
symbol__strerror_disassemble(sym, map, err, msg, sizeof(msg));
diff --git a/tools/perf/ui/gtk/annotate.c b/tools/perf/ui/gtk/annotate.c
index d903fd4..87e3760 100644
--- a/tools/perf/ui/gtk/annotate.c
+++ b/tools/perf/ui/gtk/annotate.c
@@ -169,7 +169,7 @@ static int symbol__gtk_annotate(struct symbol *sym, struct 
map 

[PATCH v4 0/2] perf report: Implement visual marker for macro fusion in annotate

[Jin Yao]

Macro fusion merges two instructions to a single micro-op. Intel
core platform performs this hardware optimization under limited
circumstances. For example, CMP + JCC can be "fused" and executed
/retired together. While with sampling this can result in the
sample sometimes being on the JCC and sometimes on the CMP.
So for the fused instruction pair, they could be considered
together.

On Nehalem, fused instruction pairs:
cmp/test + jcc.

On other new CPU:
cmp/test/add/sub/and/inc/dec + jcc.

This patch series marks the case clearly by joining the fused
instruction pair in the arrow of the jump.

For example:

   │   ┌──cmpl   $0x0,argp_program_version_hook
 81.93 │   ├──je 20
   │   │  lock   cmpxchg %esi,0x38a9a4(%rip)
   │   │↓ jne29
   │   │↓ jmp43
 11.47 │20:└─→cmpxch %esi,0x38a999(%rip)

Change-log:
---
v4: Move the CPU model checking to symbol__disassemble and save
the family/model in arch structure. It avoids checking everytime
when display the jump arrows.

The patch set is still performing the fused checking when user
moves the cursor on the jump instruction.

v3: 1.  Add checking for Nehalem (CMP, TEST). For other newer
Intel CPUs just check it by default (CMP, TEST, ADD,
SUB, AND, INC, DEC).

2.  Use Arnaldo's fix to let the display be better

v2: According to Arnaldo's comments, remove the weak function and
use an arch-specific function instead to check fused instruction
pair.

v1: Inital post

Jin Yao (2):
  perf util: Check for fused instruction
  perf report: Implement visual marker for macro fusion in annotate

 tools/perf/arch/x86/annotate/instructions.c | 46 +
 tools/perf/builtin-top.c|  2 +-
 tools/perf/ui/browser.c | 29 ++
 tools/perf/ui/browser.h |  2 ++
 tools/perf/ui/browsers/annotate.c   | 30 ++-
 tools/perf/ui/gtk/annotate.c|  2 +-
 tools/perf/util/annotate.c  | 27 +++--
 tools/perf/util/annotate.h  |  4 ++-
 8 files changed, 136 insertions(+), 6 deletions(-)

-- 
2.7.4

[PATCH v4 0/2] perf report: Implement visual marker for macro fusion in annotate

[Jin Yao]

Macro fusion merges two instructions to a single micro-op. Intel
core platform performs this hardware optimization under limited
circumstances. For example, CMP + JCC can be "fused" and executed
/retired together. While with sampling this can result in the
sample sometimes being on the JCC and sometimes on the CMP.
So for the fused instruction pair, they could be considered
together.

On Nehalem, fused instruction pairs:
cmp/test + jcc.

On other new CPU:
cmp/test/add/sub/and/inc/dec + jcc.

This patch series marks the case clearly by joining the fused
instruction pair in the arrow of the jump.

For example:

   │   ┌──cmpl   $0x0,argp_program_version_hook
 81.93 │   ├──je 20
   │   │  lock   cmpxchg %esi,0x38a9a4(%rip)
   │   │↓ jne29
   │   │↓ jmp43
 11.47 │20:└─→cmpxch %esi,0x38a999(%rip)

Change-log:
---
v4: Move the CPU model checking to symbol__disassemble and save
the family/model in arch structure. It avoids checking everytime
when display the jump arrows.

The patch set is still performing the fused checking when user
moves the cursor on the jump instruction.

v3: 1.  Add checking for Nehalem (CMP, TEST). For other newer
Intel CPUs just check it by default (CMP, TEST, ADD,
SUB, AND, INC, DEC).

2.  Use Arnaldo's fix to let the display be better

v2: According to Arnaldo's comments, remove the weak function and
use an arch-specific function instead to check fused instruction
pair.

v1: Inital post

Jin Yao (2):
  perf util: Check for fused instruction
  perf report: Implement visual marker for macro fusion in annotate

 tools/perf/arch/x86/annotate/instructions.c | 46 +
 tools/perf/builtin-top.c|  2 +-
 tools/perf/ui/browser.c | 29 ++
 tools/perf/ui/browser.h |  2 ++
 tools/perf/ui/browsers/annotate.c   | 30 ++-
 tools/perf/ui/gtk/annotate.c|  2 +-
 tools/perf/util/annotate.c  | 27 +++--
 tools/perf/util/annotate.h  |  4 ++-
 8 files changed, 136 insertions(+), 6 deletions(-)

-- 
2.7.4

Re: [PATCH] iio: multiplexer: add NULL check on devm_kzalloc() and devm_kmemdup() return values

[Peter Rosin]

On 2017-07-07 06:53, Gustavo A. R. Silva wrote:
> Check return values from call to devm_kzalloc() and devm_kmemup()

If someone cares enough: s/devm_kmemup/evm_kmemdup/


> in order to prevent a NULL pointer dereference.
> 
> This issue was detected using Coccinelle and the following semantic patch:
> 
> @@
> expression x;
> identifier fld;
> @@
> 
> * x = devm_kzalloc(...);
>... when != x == NULL
>x->fld
> 
> Cc: Peter Rosin 
> Signed-off-by: Gustavo A. R. Silva 

Either way,

Reviewed-by: Peter Rosin 

Thanks!

Re: [PATCH] iio: multiplexer: add NULL check on devm_kzalloc() and devm_kmemdup() return values

[Peter Rosin]

On 2017-07-07 06:53, Gustavo A. R. Silva wrote:
> Check return values from call to devm_kzalloc() and devm_kmemup()

If someone cares enough: s/devm_kmemup/evm_kmemdup/


> in order to prevent a NULL pointer dereference.
> 
> This issue was detected using Coccinelle and the following semantic patch:
> 
> @@
> expression x;
> identifier fld;
> @@
> 
> * x = devm_kzalloc(...);
>... when != x == NULL
>x->fld
> 
> Cc: Peter Rosin 
> Signed-off-by: Gustavo A. R. Silva 

Either way,

Reviewed-by: Peter Rosin 

Thanks!

linux-next: Tree for Jul 7

[Stephen Rothwell]

Hi all,

Please do not add any v4.14 material to you linux-next included branches
until after v4.13-rc1 has been released.

Changes since 20170706:

The f2fs tree gained a conflict against Linus' tree.

The akpm tree lost a patch that turned up elsewhere.

Non-merge commits (relative to Linus' tree): 4006
 3387 files changed, 456543 insertions(+), 62457 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. And
finally, a simple boot test of the powerpc pseries_le_defconfig kernel
in qemu.

Below is a summary of the state of the merge.

I am currently merging 266 trees (counting Linus' and 41 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 (90311148415a Merge tag 'scsi-misc' of 
git://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi)
Merging fixes/master (b4b8cbf679c4 Cavium CNN55XX: fix broken default Kconfig 
entry)
Merging kbuild-current/fixes (ad8181060788 kconfig: fix sparse warnings in 
nconfig)
Merging arc-current/for-curr (c0bc126f97fb Linux 4.12-rc7)
Merging arm-current/fixes (9e25ebfe56ec ARM: 8685/1: ensure memblock-limit is 
pmd-aligned)
Merging m68k-current/for-linus (204a2be30a7a m68k: Remove ptrace_signal_deliver)
Merging metag-fixes/fixes (b884a190afce metag/usercopy: Add missing fixups)
Merging powerpc-fixes/fixes (d6bd8194e286 powerpc/32: Avoid miscompilation 
w/GCC 4.6.3 - don't inline copy_to/from_user())
Merging sparc/master (dbd2667a4fb9 sparc64: Fix gup_huge_pmd)
Merging fscrypt-current/for-stable (42d97eb0ade3 fscrypt: fix renaming and 
linking special files)
Merging net/master (f630c38ef0d7 vrf: fix bug_on triggered by rx when 
destroying a vrf)
Merging ipsec/master (ca3a1b856636 esp6_offload: Fix IP6CB(skb)->nhoff for ESP 
GRO)
Merging netfilter/master (c644bd79c0a7 Merge 
git://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf)
Merging ipvs/master (3c5ab3f395d6 ipvs: SNAT packet replies only for NATed 
connections)
Merging wireless-drivers/master (35abcd4f9f30 brcmfmac: fix uninitialized 
warning in brcmf_usb_probe_phase2())
Merging mac80211/master (4b153ca989a9 Merge tag 'mac80211-for-davem-2017-06-16' 
of git://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211)
Merging sound-current/for-linus (6ede2b7df92f ALSA: opl4: Move inline before 
return type)
Merging pci-current/for-linus (98dbf5af4fdd PCI: endpoint: Select CRC32 to fix 
test build error)
Merging driver-core.current/driver-core-linus (650fc870a2ef Merge tag 
'docs-4.13' of git://git.lwn.net/linux)
Merging tty.current/tty-linus (650fc870a2ef Merge tag 'docs-4.13' of 
git://git.lwn.net/linux)
Merging usb.current/usb-linus (dec08194ffec xhci: Limit USB2 port wake support 
for AMD Promontory hosts)
Merging usb-gadget-fixes/fixes (f50b878fed33 USB: gadget: fix GPF in gadgetfs)
Merging usb-serial-fixes/usb-linus (996fab55d864 USB: serial: qcserial: new 
Sierra Wireless EM7305 device ID)
Merging usb-chipidea-fixes/ci-for-usb-stable (cbb22ebcfb99 usb: chipidea: core: 
check before accessing ci_role in ci_role_show)
Merging phy/fixes (9605bc46433d phy: qualcomm: phy-qcom-qmp: fix application of 
sizeof to pointer)
Merging staging.current/staging-linus (650fc870a2ef Merge tag 'docs-4.13' of 
git://git.lwn.net/linux)
Merging char-misc.current/char-misc-linus (650fc870a2ef Merge tag 'docs-4.13' 
of git://git.lwn.net/linux)
Merging input-current/for-linus (ede2e7cdc58e Merge branch 'next' into 
for-linus)
Merging crypto-current/master (b82ce24426a4 crypto: sh

linux-next: Tree for Jul 7

[Stephen Rothwell]

Hi all,

Please do not add any v4.14 material to you linux-next included branches
until after v4.13-rc1 has been released.

Changes since 20170706:

The f2fs tree gained a conflict against Linus' tree.

The akpm tree lost a patch that turned up elsewhere.

Non-merge commits (relative to Linus' tree): 4006
 3387 files changed, 456543 insertions(+), 62457 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. And
finally, a simple boot test of the powerpc pseries_le_defconfig kernel
in qemu.

Below is a summary of the state of the merge.

I am currently merging 266 trees (counting Linus' and 41 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 (90311148415a Merge tag 'scsi-misc' of 
git://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi)
Merging fixes/master (b4b8cbf679c4 Cavium CNN55XX: fix broken default Kconfig 
entry)
Merging kbuild-current/fixes (ad8181060788 kconfig: fix sparse warnings in 
nconfig)
Merging arc-current/for-curr (c0bc126f97fb Linux 4.12-rc7)
Merging arm-current/fixes (9e25ebfe56ec ARM: 8685/1: ensure memblock-limit is 
pmd-aligned)
Merging m68k-current/for-linus (204a2be30a7a m68k: Remove ptrace_signal_deliver)
Merging metag-fixes/fixes (b884a190afce metag/usercopy: Add missing fixups)
Merging powerpc-fixes/fixes (d6bd8194e286 powerpc/32: Avoid miscompilation 
w/GCC 4.6.3 - don't inline copy_to/from_user())
Merging sparc/master (dbd2667a4fb9 sparc64: Fix gup_huge_pmd)
Merging fscrypt-current/for-stable (42d97eb0ade3 fscrypt: fix renaming and 
linking special files)
Merging net/master (f630c38ef0d7 vrf: fix bug_on triggered by rx when 
destroying a vrf)
Merging ipsec/master (ca3a1b856636 esp6_offload: Fix IP6CB(skb)->nhoff for ESP 
GRO)
Merging netfilter/master (c644bd79c0a7 Merge 
git://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf)
Merging ipvs/master (3c5ab3f395d6 ipvs: SNAT packet replies only for NATed 
connections)
Merging wireless-drivers/master (35abcd4f9f30 brcmfmac: fix uninitialized 
warning in brcmf_usb_probe_phase2())
Merging mac80211/master (4b153ca989a9 Merge tag 'mac80211-for-davem-2017-06-16' 
of git://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211)
Merging sound-current/for-linus (6ede2b7df92f ALSA: opl4: Move inline before 
return type)
Merging pci-current/for-linus (98dbf5af4fdd PCI: endpoint: Select CRC32 to fix 
test build error)
Merging driver-core.current/driver-core-linus (650fc870a2ef Merge tag 
'docs-4.13' of git://git.lwn.net/linux)
Merging tty.current/tty-linus (650fc870a2ef Merge tag 'docs-4.13' of 
git://git.lwn.net/linux)
Merging usb.current/usb-linus (dec08194ffec xhci: Limit USB2 port wake support 
for AMD Promontory hosts)
Merging usb-gadget-fixes/fixes (f50b878fed33 USB: gadget: fix GPF in gadgetfs)
Merging usb-serial-fixes/usb-linus (996fab55d864 USB: serial: qcserial: new 
Sierra Wireless EM7305 device ID)
Merging usb-chipidea-fixes/ci-for-usb-stable (cbb22ebcfb99 usb: chipidea: core: 
check before accessing ci_role in ci_role_show)
Merging phy/fixes (9605bc46433d phy: qualcomm: phy-qcom-qmp: fix application of 
sizeof to pointer)
Merging staging.current/staging-linus (650fc870a2ef Merge tag 'docs-4.13' of 
git://git.lwn.net/linux)
Merging char-misc.current/char-misc-linus (650fc870a2ef Merge tag 'docs-4.13' 
of git://git.lwn.net/linux)
Merging input-current/for-linus (ede2e7cdc58e Merge branch 'next' into 
for-linus)
Merging crypto-current/master (b82ce24426a4 crypto: sh

[PATCH] arcnet: com20020-pci: Fix an error handling path in 'com20020pci_probe()'

[Christophe JAILLET]

If this memory allocation fails, we should go through the error handling
path as done everywhere else in this function before returning.

Signed-off-by: Christophe JAILLET 
---
 drivers/net/arcnet/com20020-pci.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/arcnet/com20020-pci.c 
b/drivers/net/arcnet/com20020-pci.c
index 239de38fbd6a..8e89a2ac071e 100644
--- a/drivers/net/arcnet/com20020-pci.c
+++ b/drivers/net/arcnet/com20020-pci.c
@@ -196,8 +196,10 @@ static int com20020pci_probe(struct pci_dev *pdev,
 
card = devm_kzalloc(>dev, sizeof(struct com20020_dev),
GFP_KERNEL);
-   if (!card)
-   return -ENOMEM;
+   if (!card) {
+   ret = -ENOMEM;
+   goto out_port;
+   }
 
card->index = i;
card->pci_priv = priv;
-- 
2.11.0

[PATCH] arcnet: com20020-pci: Fix an error handling path in 'com20020pci_probe()'

[Christophe JAILLET]

If this memory allocation fails, we should go through the error handling
path as done everywhere else in this function before returning.

Signed-off-by: Christophe JAILLET 
---
 drivers/net/arcnet/com20020-pci.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/arcnet/com20020-pci.c 
b/drivers/net/arcnet/com20020-pci.c
index 239de38fbd6a..8e89a2ac071e 100644
--- a/drivers/net/arcnet/com20020-pci.c
+++ b/drivers/net/arcnet/com20020-pci.c
@@ -196,8 +196,10 @@ static int com20020pci_probe(struct pci_dev *pdev,
 
card = devm_kzalloc(>dev, sizeof(struct com20020_dev),
GFP_KERNEL);
-   if (!card)
-   return -ENOMEM;
+   if (!card) {
+   ret = -ENOMEM;
+   goto out_port;
+   }
 
card->index = i;
card->pci_priv = priv;
-- 
2.11.0

[PATCH] iio: multiplexer: add NULL check on devm_kzalloc() and devm_kmemdup() return values

[Gustavo A. R. Silva]

Check return values from call to devm_kzalloc() and devm_kmemup()
in order to prevent a NULL pointer dereference.

This issue was detected using Coccinelle and the following semantic patch:

@@
expression x;
identifier fld;
@@

* x = devm_kzalloc(...);
   ... when != x == NULL
   x->fld

Cc: Peter Rosin 
Signed-off-by: Gustavo A. R. Silva 
---
Changes in v2:
 Add NULL check on devm_kmemup() return value.

 drivers/iio/multiplexer/iio-mux.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/drivers/iio/multiplexer/iio-mux.c 
b/drivers/iio/multiplexer/iio-mux.c
index 37ba007..74831fc 100644
--- a/drivers/iio/multiplexer/iio-mux.c
+++ b/drivers/iio/multiplexer/iio-mux.c
@@ -285,6 +285,9 @@ static int mux_configure_channel(struct device *dev, struct 
mux *mux,
child->ext_info_cache = devm_kzalloc(dev,
 sizeof(*child->ext_info_cache) *
 num_ext_info, GFP_KERNEL);
+   if (!child->ext_info_cache)
+   return -ENOMEM;
+
for (i = 0; i < num_ext_info; ++i) {
child->ext_info_cache[i].size = -1;
 
@@ -309,6 +312,9 @@ static int mux_configure_channel(struct device *dev, struct 
mux *mux,
 
child->ext_info_cache[i].data = devm_kmemdup(dev, page, ret + 1,
 GFP_KERNEL);
+   if (!child->ext_info_cache[i].data)
+   return -ENOMEM;
+
child->ext_info_cache[i].data[ret] = 0;
child->ext_info_cache[i].size = ret;
}
-- 
2.5.0

[PATCH] iio: multiplexer: add NULL check on devm_kzalloc() and devm_kmemdup() return values

[Gustavo A. R. Silva]

Check return values from call to devm_kzalloc() and devm_kmemup()
in order to prevent a NULL pointer dereference.

This issue was detected using Coccinelle and the following semantic patch:

@@
expression x;
identifier fld;
@@

* x = devm_kzalloc(...);
   ... when != x == NULL
   x->fld

Cc: Peter Rosin 
Signed-off-by: Gustavo A. R. Silva 
---
Changes in v2:
 Add NULL check on devm_kmemup() return value.

 drivers/iio/multiplexer/iio-mux.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/drivers/iio/multiplexer/iio-mux.c 
b/drivers/iio/multiplexer/iio-mux.c
index 37ba007..74831fc 100644
--- a/drivers/iio/multiplexer/iio-mux.c
+++ b/drivers/iio/multiplexer/iio-mux.c
@@ -285,6 +285,9 @@ static int mux_configure_channel(struct device *dev, struct 
mux *mux,
child->ext_info_cache = devm_kzalloc(dev,
 sizeof(*child->ext_info_cache) *
 num_ext_info, GFP_KERNEL);
+   if (!child->ext_info_cache)
+   return -ENOMEM;
+
for (i = 0; i < num_ext_info; ++i) {
child->ext_info_cache[i].size = -1;
 
@@ -309,6 +312,9 @@ static int mux_configure_channel(struct device *dev, struct 
mux *mux,
 
child->ext_info_cache[i].data = devm_kmemdup(dev, page, ret + 1,
 GFP_KERNEL);
+   if (!child->ext_info_cache[i].data)
+   return -ENOMEM;
+
child->ext_info_cache[i].data[ret] = 0;
child->ext_info_cache[i].size = ret;
}
-- 
2.5.0

Re: [PATCH] iio: multiplexer: add NULL check on devm_kzalloc() return value

[Gustavo A. R. Silva]

Quoting Peter Rosin :


On 2017-07-07 06:35, Gustavo A. R. Silva wrote:

Hi Peter,

Quoting Peter Rosin :


On 2017-07-07 00:08, Gustavo A. R. Silva wrote:

Check return value from call to devm_kzalloc()
in order to prevent a NULL pointer dereference.


Right, thanks for finding that one! There's another one inside the
for loop that is just starting in the context of this patch. Care
to fix checking the return value of that devm_kmemdup as well?



Sure, I'll send a new patch shortly.


And someone should perhaps teach Coccinelle about devm_kmemdup...



Good catch, I just implemented that script.


This issue was detected using Coccinelle and the following semantic patch:

@@
expression x;
identifier fld;
@@

* x = devm_kzalloc(...);
  ... when != x == NULL
  x->fld




One of these blank lines should perhaps be a "Fixes:" tag?



mmm, I don't get this...


If you add a Fixes-tag, like below, you help the stable kernel maintainers
decide what to look at. In this case it might be overkill since the thing
you fix is so fresh and does not apply to any old kernel. But I think it
is a good habit...

Fixes: 7ba9df54b091 ("iio: multiplexer: new iio category and iio-mux driver")

(and it is a bit unusual to see two blank lines before the SoB-tag)

Sorry for not spelling it out the first time.



Oh, I get it now. Thank you very much for clarifying. I will  
definitely keep that in mind for future fixes when that could apply.


Thanks!
--
Gustavo A. R. Silva

Re: [PATCH] iio: multiplexer: add NULL check on devm_kzalloc() return value

[Gustavo A. R. Silva]

Quoting Peter Rosin :


On 2017-07-07 06:35, Gustavo A. R. Silva wrote:

Hi Peter,

Quoting Peter Rosin :


On 2017-07-07 00:08, Gustavo A. R. Silva wrote:

Check return value from call to devm_kzalloc()
in order to prevent a NULL pointer dereference.


Right, thanks for finding that one! There's another one inside the
for loop that is just starting in the context of this patch. Care
to fix checking the return value of that devm_kmemdup as well?



Sure, I'll send a new patch shortly.


And someone should perhaps teach Coccinelle about devm_kmemdup...



Good catch, I just implemented that script.


This issue was detected using Coccinelle and the following semantic patch:

@@
expression x;
identifier fld;
@@

* x = devm_kzalloc(...);
  ... when != x == NULL
  x->fld




One of these blank lines should perhaps be a "Fixes:" tag?



mmm, I don't get this...


If you add a Fixes-tag, like below, you help the stable kernel maintainers
decide what to look at. In this case it might be overkill since the thing
you fix is so fresh and does not apply to any old kernel. But I think it
is a good habit...

Fixes: 7ba9df54b091 ("iio: multiplexer: new iio category and iio-mux driver")

(and it is a bit unusual to see two blank lines before the SoB-tag)

Sorry for not spelling it out the first time.



Oh, I get it now. Thank you very much for clarifying. I will  
definitely keep that in mind for future fixes when that could apply.


Thanks!
--
Gustavo A. R. Silva

Re: [RFC][PATCH] exec: Use init rlimits for setuid exec

[Andy Lutomirski]

On Thu, Jul 6, 2017 at 12:12 PM, Kees Cook  wrote:
> On Thu, Jul 6, 2017 at 10:52 AM, Linus Torvalds
>  wrote:
>> On Thu, Jul 6, 2017 at 10:29 AM, Kees Cook  wrote:

  (a) minimal: just use our existing default stack (and stack _only_)
 limit value for suid binaries that actually get extra permissions: {
 _STK_LIM, RLIM_INFINITY }.
>>>
>>> This would look a lot like the existing patch; it'd just not copy the
>>> init process rlimits.
>>
>> Can't we just do the final rlimit setting so late in execve that we
>> don't need that whole "saved_rlimit" thing?
>
> The stack rlimit defines the mmap layout too:
>
> do_execveat_common() ->
> exec_binprm() ->
> search_binary_handler() ->
> fmt->load_binary (load_elf_binary()) ->
> setup_new_exec() ->
> arch_pick_mmap_layout() ->
> mmap_is_legacy() ->
> rlimit(RLIMIT_STACK) == RLIM_INFINITY
>
> exec_binprm() happens after the other stack setup (copy_strings()), so
> if we wanted to avoid saved_rlimit, we'd have to replumb how
> arch_pick_mmap_layout() works and how copy_strings() performs its
> calculations (neither looks too terrible).

How about a much simpler solution: don't read rlimit at all in
copy_strings(), let alone try to enforce it.  Instead, just before the
point of no return, check how much stack space is already used and, if
it's more than an appropriate threshold (e.g. 1/4 of the rlimit),
abort.  Sure, this adds overhead if we're going to abort, but does
that really matter?

I don't see why using rlimit for layout control makes any sense
whatsoever.  Is there some historical reason we need that?  As far as
I can see (on insufficient inspection) is that the kernel is trying to
guarantee that, if we have so much arg crap that our remaining stack
is less than 128k, then we don't exceed our limit by a little bit.

Re: [RFC][PATCH] exec: Use init rlimits for setuid exec

[Andy Lutomirski]

On Thu, Jul 6, 2017 at 12:12 PM, Kees Cook  wrote:
> On Thu, Jul 6, 2017 at 10:52 AM, Linus Torvalds
>  wrote:
>> On Thu, Jul 6, 2017 at 10:29 AM, Kees Cook  wrote:

  (a) minimal: just use our existing default stack (and stack _only_)
 limit value for suid binaries that actually get extra permissions: {
 _STK_LIM, RLIM_INFINITY }.
>>>
>>> This would look a lot like the existing patch; it'd just not copy the
>>> init process rlimits.
>>
>> Can't we just do the final rlimit setting so late in execve that we
>> don't need that whole "saved_rlimit" thing?
>
> The stack rlimit defines the mmap layout too:
>
> do_execveat_common() ->
> exec_binprm() ->
> search_binary_handler() ->
> fmt->load_binary (load_elf_binary()) ->
> setup_new_exec() ->
> arch_pick_mmap_layout() ->
> mmap_is_legacy() ->
> rlimit(RLIMIT_STACK) == RLIM_INFINITY
>
> exec_binprm() happens after the other stack setup (copy_strings()), so
> if we wanted to avoid saved_rlimit, we'd have to replumb how
> arch_pick_mmap_layout() works and how copy_strings() performs its
> calculations (neither looks too terrible).

How about a much simpler solution: don't read rlimit at all in
copy_strings(), let alone try to enforce it.  Instead, just before the
point of no return, check how much stack space is already used and, if
it's more than an appropriate threshold (e.g. 1/4 of the rlimit),
abort.  Sure, this adds overhead if we're going to abort, but does
that really matter?

I don't see why using rlimit for layout control makes any sense
whatsoever.  Is there some historical reason we need that?  As far as
I can see (on insufficient inspection) is that the kernel is trying to
guarantee that, if we have so much arg crap that our remaining stack
is less than 128k, then we don't exceed our limit by a little bit.

Re: [PATCH v2] arm: eBPF JIT compiler

[Shubham Bansal]

Okay Kees. I will take a look at it.
Best,
Shubham Bansal


On Fri, Jul 7, 2017 at 10:12 AM, Kees Cook  wrote:
> On Wed, Jul 5, 2017 at 8:49 PM, Shubham Bansal
>  wrote:
>> Hi Kees,
>>
>> Problem is my ARM machine don't have clang and iproute2 which is
>> keeping me from testing the bpf tail calls.
>>
>> You should do the following to test it,.
>>
>> 1. tools/testing/selftests/bpf/
>> 2. make
>> 3. sudo ./test_progs
>>
>> And, before testing, you have to do "make headers_install".
>> These tests are for tail calls with the attached patch. If its too
>> much work, Can you please upload your arm image so that I can test it?
>> I just need a good machine.
>
> I've got all this set up now, and it faults during the test:
>
> Unable to handle kernel NULL pointer dereference at virtual address 0008
> ...
> CPU: 0 PID: 1922 Comm: test_progs Not tainted 4.12.0+ #60
> ...
> PC is at __htab_map_lookup_elem+0x54/0x1f4
>
> I'll see if I can send you this disk image...
>
> -Kees
>
>
> --
> Kees Cook
> Pixel Security

Re: [PATCH v2] arm: eBPF JIT compiler

[Shubham Bansal]

Okay Kees. I will take a look at it.
Best,
Shubham Bansal


On Fri, Jul 7, 2017 at 10:12 AM, Kees Cook  wrote:
> On Wed, Jul 5, 2017 at 8:49 PM, Shubham Bansal
>  wrote:
>> Hi Kees,
>>
>> Problem is my ARM machine don't have clang and iproute2 which is
>> keeping me from testing the bpf tail calls.
>>
>> You should do the following to test it,.
>>
>> 1. tools/testing/selftests/bpf/
>> 2. make
>> 3. sudo ./test_progs
>>
>> And, before testing, you have to do "make headers_install".
>> These tests are for tail calls with the attached patch. If its too
>> much work, Can you please upload your arm image so that I can test it?
>> I just need a good machine.
>
> I've got all this set up now, and it faults during the test:
>
> Unable to handle kernel NULL pointer dereference at virtual address 0008
> ...
> CPU: 0 PID: 1922 Comm: test_progs Not tainted 4.12.0+ #60
> ...
> PC is at __htab_map_lookup_elem+0x54/0x1f4
>
> I'll see if I can send you this disk image...
>
> -Kees
>
>
> --
> Kees Cook
> Pixel Security

Re: [PATCH 2/2] serial: earlycon: Make early_con as __initdata

[Sergey Senozhatsky]

On (07/06/17 11:38), Matt Redfearn wrote:
> All early console drivers that may be registered as the earlycon are
> marked __init to be placed in the init section. The drivers' code and
> data are freed during free_initmem_default() but the early console is
> not unregistered in printk_late_init() as the init_section_intersects()
> test fails. This leads to the earlycon still being active, potentially
> with dangling pointers into the freed init section, which may have been
> poisoned by the slab debugger. Attempting to use the boot console after
> this will likely lead to weird behaviour and kernel crashes.

people want to use early-con as a panic() console fallback.

-ss

> Fix this by marking the early_con struct __initdata so that the
> init_section_intersects() test succeeds and the console is unregistered
> in printk_late_init() before the init section is freed.
> 
> The 8250 earlycon, on which the generic earlycon was based, had these
> attributes on it's console struct. The switch to the generic
> implementation in commit d2fd6810a823 ("tty/serial: convert 8250 to
> generic earlycon") appears to have broken unregistraton of the boot
> console when its code and data are in __init.
> 
> Fixes: d2fd6810a823 ("tty/serial: convert 8250 to generic earlycon")
> Signed-off-by: Matt Redfearn 
> 
> ---
> 
>  drivers/tty/serial/earlycon.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c
> index c3651540e1ba..388aaafcca2b 100644
> --- a/drivers/tty/serial/earlycon.c
> +++ b/drivers/tty/serial/earlycon.c
> @@ -29,13 +29,13 @@
>  
>  #include 
>  
> -static struct console early_con = {
> +static struct console early_con __initdata = {
>   .name = "uart", /* fixed up at earlycon registration */
>   .flags =CON_PRINTBUFFER | CON_BOOT,
>   .index =0,
>  };
>  
> -static struct earlycon_device early_console_dev = {
> +static struct earlycon_device early_console_dev __initdata = {
>   .con = _con,
>  };
>  
> -- 
> 2.7.4
> 

Re: [PATCH 2/2] serial: earlycon: Make early_con as __initdata

[Sergey Senozhatsky]

On (07/06/17 11:38), Matt Redfearn wrote:
> All early console drivers that may be registered as the earlycon are
> marked __init to be placed in the init section. The drivers' code and
> data are freed during free_initmem_default() but the early console is
> not unregistered in printk_late_init() as the init_section_intersects()
> test fails. This leads to the earlycon still being active, potentially
> with dangling pointers into the freed init section, which may have been
> poisoned by the slab debugger. Attempting to use the boot console after
> this will likely lead to weird behaviour and kernel crashes.

people want to use early-con as a panic() console fallback.

-ss

> Fix this by marking the early_con struct __initdata so that the
> init_section_intersects() test succeeds and the console is unregistered
> in printk_late_init() before the init section is freed.
> 
> The 8250 earlycon, on which the generic earlycon was based, had these
> attributes on it's console struct. The switch to the generic
> implementation in commit d2fd6810a823 ("tty/serial: convert 8250 to
> generic earlycon") appears to have broken unregistraton of the boot
> console when its code and data are in __init.
> 
> Fixes: d2fd6810a823 ("tty/serial: convert 8250 to generic earlycon")
> Signed-off-by: Matt Redfearn 
> 
> ---
> 
>  drivers/tty/serial/earlycon.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c
> index c3651540e1ba..388aaafcca2b 100644
> --- a/drivers/tty/serial/earlycon.c
> +++ b/drivers/tty/serial/earlycon.c
> @@ -29,13 +29,13 @@
>  
>  #include 
>  
> -static struct console early_con = {
> +static struct console early_con __initdata = {
>   .name = "uart", /* fixed up at earlycon registration */
>   .flags =CON_PRINTBUFFER | CON_BOOT,
>   .index =0,
>  };
>  
> -static struct earlycon_device early_console_dev = {
> +static struct earlycon_device early_console_dev __initdata = {
>   .con = _con,
>  };
>  
> -- 
> 2.7.4
> 

Re: [PATCH] iio: multiplexer: add NULL check on devm_kzalloc() return value

[Peter Rosin]

On 2017-07-07 06:35, Gustavo A. R. Silva wrote:
> Hi Peter,
> 
> Quoting Peter Rosin :
> 
>> On 2017-07-07 00:08, Gustavo A. R. Silva wrote:
>>> Check return value from call to devm_kzalloc()
>>> in order to prevent a NULL pointer dereference.
>>
>> Right, thanks for finding that one! There's another one inside the
>> for loop that is just starting in the context of this patch. Care
>> to fix checking the return value of that devm_kmemdup as well?
>>
> 
> Sure, I'll send a new patch shortly.
> 
>> And someone should perhaps teach Coccinelle about devm_kmemdup...
>>
> 
> Good catch, I just implemented that script.
> 
>>> This issue was detected using Coccinelle and the following semantic patch:
>>>
>>> @@
>>> expression x;
>>> identifier fld;
>>> @@
>>>
>>> * x = devm_kzalloc(...);
>>>   ... when != x == NULL
>>>   x->fld
>>>
>>>
>>
>> One of these blank lines should perhaps be a "Fixes:" tag?
>>
> 
> mmm, I don't get this...

If you add a Fixes-tag, like below, you help the stable kernel maintainers
decide what to look at. In this case it might be overkill since the thing
you fix is so fresh and does not apply to any old kernel. But I think it
is a good habit...

Fixes: 7ba9df54b091 ("iio: multiplexer: new iio category and iio-mux driver")

(and it is a bit unusual to see two blank lines before the SoB-tag)

Sorry for not spelling it out the first time.

Cheers,
peda

Re: [PATCH] iio: multiplexer: add NULL check on devm_kzalloc() return value

[Peter Rosin]

On 2017-07-07 06:35, Gustavo A. R. Silva wrote:
> Hi Peter,
> 
> Quoting Peter Rosin :
> 
>> On 2017-07-07 00:08, Gustavo A. R. Silva wrote:
>>> Check return value from call to devm_kzalloc()
>>> in order to prevent a NULL pointer dereference.
>>
>> Right, thanks for finding that one! There's another one inside the
>> for loop that is just starting in the context of this patch. Care
>> to fix checking the return value of that devm_kmemdup as well?
>>
> 
> Sure, I'll send a new patch shortly.
> 
>> And someone should perhaps teach Coccinelle about devm_kmemdup...
>>
> 
> Good catch, I just implemented that script.
> 
>>> This issue was detected using Coccinelle and the following semantic patch:
>>>
>>> @@
>>> expression x;
>>> identifier fld;
>>> @@
>>>
>>> * x = devm_kzalloc(...);
>>>   ... when != x == NULL
>>>   x->fld
>>>
>>>
>>
>> One of these blank lines should perhaps be a "Fixes:" tag?
>>
> 
> mmm, I don't get this...

If you add a Fixes-tag, like below, you help the stable kernel maintainers
decide what to look at. In this case it might be overkill since the thing
you fix is so fresh and does not apply to any old kernel. But I think it
is a good habit...

Fixes: 7ba9df54b091 ("iio: multiplexer: new iio category and iio-mux driver")

(and it is a bit unusual to see two blank lines before the SoB-tag)

Sorry for not spelling it out the first time.

Cheers,
peda

Re: [x86/time] 03fa63cc96: ACPI_Error:Table[DMAR]is_not_invalidated_during_early_boot_stage(#/tbxface -#)

[Dou Liyang]

Hi Thomas,

At 07/07/2017 11:04 AM, Ye Xiaolong wrote:

On 07/07, Dou Liyang wrote:

Hi xiaolong,

Really thanks for your testing.

At 07/07/2017 09:54 AM, Ye Xiaolong wrote:

On 07/06, Thomas Gleixner wrote:

On Thu, 6 Jul 2017, kernel test robot wrote:


commit: 03fa63cc96ab35592e0a7d522b8edbc1e6b02d22 ("x86/time: Initialize interrupt 
mode behind timer init")



++++
|| 43436935b7 | 03fa63cc96 |
++++
| boot_successes | 0  | 4  |
++++


So 03fa63cc96 makes the box boot again. I'm confused as usual by the
output of this tool.,


kern  :info  : [0.005000] tsc: Fast TSC calibration using PIT
kern  :info  : [0.006000] tsc: Detected 2195.020 MHz processor
kern  :info  : [0.007000] Calibrating delay loop (skipped), value 
calculated using timer frequency.. 4390.04 BogoMIPS (lpj=2195020)
kern  :info  : [0.008001] pid_max: default: 90112 minimum: 704
kern  :info  : [0.009037] ACPI: Core revision 20170303
kern  :err   : [0.010002] ACPI Error: Table [DMAR] is not invalidated 
during early boot stage (20170303/tbxface-193)


Sure we have a error message here, but compared to what? Compared to
something which does not boot at all?


Sorry for the confusion, here commit 43436935b7 boot failed due to OOM which
happened at the late stage of kernel boot while the ACPI error showed at the
early boot stage for commit 03fa63cc96 and it didn't appear in 43436935b7's
dmesg.



let's make the problem clearly firstly:

1) Commit 43436935b7 ("x86/xen: Bypass intr mode setup in enlighten_pv
system") made kernel boot failed, which caused by OOM.



It is so interesting!

This patch only work for XEN in pv mode, if we boot a kernel w/o XEN,
it will not work in fact.



2) Commit 03fa63cc96 ("x86/time: Initialize interrupt mode behind
timer init") can make the kernel boot success again, but with an ACPI
error happened.


Indeed, the ACPI Error appeared with this patch.

If x2apic is enabled in x86-64, initializing interrupt mode contains
irq remapping setup and the ACPI DMAR table initialization.

default_setup_apic_routing()
  enable_IR_x2apic()
irq_remapping_prepare()
  intel_prepare_irq_remapping()
dmar_table_init()

this patch make interrupt mode setup before acpi_early_init() which has
the check of ACPI table state in acpi_reallocate_root_table() where the
ACPI Error generated.

I have got a machine which supports x2apic. I am tracking the code,
try to find out more.

Thanks,

dou.


And both *1* and *2* used the same configuration showed in the
attachment.

Does anything I missed?


Yes, this is exactly what I meant.

Thanks,
Xiaolong


Thanks,

dou.


Thanks,
Xiaolong



Thanks,

tglx

Re: [x86/time] 03fa63cc96: ACPI_Error:Table[DMAR]is_not_invalidated_during_early_boot_stage(#/tbxface -#)

[Dou Liyang]

Hi Thomas,

At 07/07/2017 11:04 AM, Ye Xiaolong wrote:

On 07/07, Dou Liyang wrote:

Hi xiaolong,

Really thanks for your testing.

At 07/07/2017 09:54 AM, Ye Xiaolong wrote:

On 07/06, Thomas Gleixner wrote:

On Thu, 6 Jul 2017, kernel test robot wrote:


commit: 03fa63cc96ab35592e0a7d522b8edbc1e6b02d22 ("x86/time: Initialize interrupt 
mode behind timer init")



++++
|| 43436935b7 | 03fa63cc96 |
++++
| boot_successes | 0  | 4  |
++++


So 03fa63cc96 makes the box boot again. I'm confused as usual by the
output of this tool.,


kern  :info  : [0.005000] tsc: Fast TSC calibration using PIT
kern  :info  : [0.006000] tsc: Detected 2195.020 MHz processor
kern  :info  : [0.007000] Calibrating delay loop (skipped), value 
calculated using timer frequency.. 4390.04 BogoMIPS (lpj=2195020)
kern  :info  : [0.008001] pid_max: default: 90112 minimum: 704
kern  :info  : [0.009037] ACPI: Core revision 20170303
kern  :err   : [0.010002] ACPI Error: Table [DMAR] is not invalidated 
during early boot stage (20170303/tbxface-193)


Sure we have a error message here, but compared to what? Compared to
something which does not boot at all?


Sorry for the confusion, here commit 43436935b7 boot failed due to OOM which
happened at the late stage of kernel boot while the ACPI error showed at the
early boot stage for commit 03fa63cc96 and it didn't appear in 43436935b7's
dmesg.



let's make the problem clearly firstly:

1) Commit 43436935b7 ("x86/xen: Bypass intr mode setup in enlighten_pv
system") made kernel boot failed, which caused by OOM.



It is so interesting!

This patch only work for XEN in pv mode, if we boot a kernel w/o XEN,
it will not work in fact.



2) Commit 03fa63cc96 ("x86/time: Initialize interrupt mode behind
timer init") can make the kernel boot success again, but with an ACPI
error happened.


Indeed, the ACPI Error appeared with this patch.

If x2apic is enabled in x86-64, initializing interrupt mode contains
irq remapping setup and the ACPI DMAR table initialization.

default_setup_apic_routing()
  enable_IR_x2apic()
irq_remapping_prepare()
  intel_prepare_irq_remapping()
dmar_table_init()

this patch make interrupt mode setup before acpi_early_init() which has
the check of ACPI table state in acpi_reallocate_root_table() where the
ACPI Error generated.

I have got a machine which supports x2apic. I am tracking the code,
try to find out more.

Thanks,

dou.


And both *1* and *2* used the same configuration showed in the
attachment.

Does anything I missed?


Yes, this is exactly what I meant.

Thanks,
Xiaolong


Thanks,

dou.


Thanks,
Xiaolong



Thanks,

tglx

Re: [PATCH 1/2] printk: Unconditionally unregister boot consoles if in init section

[Sergey Senozhatsky]

On (07/06/17 11:38), Matt Redfearn wrote:
> Commit 4c30c6f566c0 ("kernel/printk: do not turn off bootconsole in
> printk_late_init() if keep_bootcon") added a check on keep_bootcon to
> ensure that boot consoles were kept around until the real console is
> registered.
> This can lead to problems if the boot console data and code are in the
> init section, since it can be freed before the boot console is
> deregistered. This was fixed by commit 81cc26f2bd11 ("printk: only
> unregister boot consoles when necessary").
> The keep_bootcon flag prevents the unregistration of a boot console,
> even if it's data and code reside in the init section and are about to
> be freed. This can lead to crashes and weird panics when the bootconsole
> is accessed after free, especially if page poisoning is in use and the
> code / data have been overwritten with a poison value.
> To prevent this, always free the boot console if it is within the init
> section.
> 
> Fixes 81cc26f2bd11 ("printk: only unregister boot consoles when necessary").
> Signed-off-by: Matt Redfearn 

well... hm. don't specify `keep_bootcon' if your bootcon access initdata?
keeping `early_printk' sometimes can be helpful and people even want to
use `early_printk' as a panic() console fallback (because of those nasty
deadlocks that spoil printk->call_console_drivers()).

if someone asked to `keep_bootcon' but we actually don't keep it, then
what's the purpose of the flag and
pr_info("debug: skip boot console de-registration.\n")?


> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index a1db38abac5b..b5411f44eed4 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -2660,7 +2660,7 @@ static int __init printk_late_init(void)
>   int ret;
>  
>   for_each_console(con) {
> - if (!keep_bootcon && con->flags & CON_BOOT) {
> + if (con->flags & CON_BOOT) {
>   /*
>* Make sure to unregister boot consoles whose data
>* resides in the init section before the init section

what about bootconsoles that are still not getting de-registered due
to keep_bootcon in register_console()?

-ss

Re: [PATCH 1/2] printk: Unconditionally unregister boot consoles if in init section

[Sergey Senozhatsky]

On (07/06/17 11:38), Matt Redfearn wrote:
> Commit 4c30c6f566c0 ("kernel/printk: do not turn off bootconsole in
> printk_late_init() if keep_bootcon") added a check on keep_bootcon to
> ensure that boot consoles were kept around until the real console is
> registered.
> This can lead to problems if the boot console data and code are in the
> init section, since it can be freed before the boot console is
> deregistered. This was fixed by commit 81cc26f2bd11 ("printk: only
> unregister boot consoles when necessary").
> The keep_bootcon flag prevents the unregistration of a boot console,
> even if it's data and code reside in the init section and are about to
> be freed. This can lead to crashes and weird panics when the bootconsole
> is accessed after free, especially if page poisoning is in use and the
> code / data have been overwritten with a poison value.
> To prevent this, always free the boot console if it is within the init
> section.
> 
> Fixes 81cc26f2bd11 ("printk: only unregister boot consoles when necessary").
> Signed-off-by: Matt Redfearn 

well... hm. don't specify `keep_bootcon' if your bootcon access initdata?
keeping `early_printk' sometimes can be helpful and people even want to
use `early_printk' as a panic() console fallback (because of those nasty
deadlocks that spoil printk->call_console_drivers()).

if someone asked to `keep_bootcon' but we actually don't keep it, then
what's the purpose of the flag and
pr_info("debug: skip boot console de-registration.\n")?


> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index a1db38abac5b..b5411f44eed4 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -2660,7 +2660,7 @@ static int __init printk_late_init(void)
>   int ret;
>  
>   for_each_console(con) {
> - if (!keep_bootcon && con->flags & CON_BOOT) {
> + if (con->flags & CON_BOOT) {
>   /*
>* Make sure to unregister boot consoles whose data
>* resides in the init section before the init section

what about bootconsoles that are still not getting de-registered due
to keep_bootcon in register_console()?

-ss

Re: [PATCH v2 2/6] cpufreq: schedutil: reset sg_cpus's flags at IDLE enter

[Joel Fernandes]

On Tue, Jul 4, 2017 at 10:34 AM, Patrick Bellasi
 wrote:
> Currently, sg_cpu's flags are set to the value defined by the last call of
> the cpufreq_update_util()/cpufreq_update_this_cpu(); for RT/DL classes
> this corresponds to the SCHED_CPUFREQ_{RT/DL} flags always being set.
>
> When multiple CPU shares the same frequency domain it might happen that a
> CPU which executed a RT task, right before entering IDLE, has one of the
> SCHED_CPUFREQ_RT_DL flags set, permanently, until it exits IDLE.
>
> Although such an idle CPU is _going to be_ ignored by the
> sugov_next_freq_shared():
>   1. this kind of "useless RT requests" are ignored only if more then
>  TICK_NSEC have elapsed since the last update
>   2. we can still potentially trigger an already too late switch to
>  MAX, which starts also a new throttling interval
>   3. the internal state machine is not consistent with what the
>  scheduler knows, i.e. the CPU is now actually idle
>
> Thus, in sugov_next_freq_shared(), where utilisation and flags are
> aggregated across all the CPUs of a frequency domain, it can turn out
> that all the CPUs of that domain can run unnecessary at the maximum OPP
> until another event happens in the idle CPU, which eventually clear the
> SCHED_CPUFREQ_{RT/DL} flag, or the IDLE CPUs gets ignored after
> TICK_NSEC since the CPU entering IDLE.
>
> Such a behaviour can harm the energy efficiency of systems where RT
> workloads are not so frequent and other CPUs in the same frequency
> domain are running small utilisation workloads, which is a quite common
> scenario in mobile embedded systems.
>
> This patch proposes a solution which is aligned with the current principle
> to update the flags each time a scheduling event happens. The scheduling
> of the idle_task on a CPU is considered one of such meaningful events.
> That's why when the idle_task is selected for execution we poke the
> schedutil policy to reset the flags for that CPU.
>
> No frequency transitions are activated at that point, which is fair in
> case the RT workload should come back in the future. However, this still
> allows other CPUs in the same frequency domain to scale down the
> frequency in case that should be possible.
>
> Signed-off-by: Patrick Bellasi 
> Cc: Ingo Molnar 
> Cc: Peter Zijlstra 
> Cc: Rafael J. Wysocki 
> Cc: Viresh Kumar 
> Cc: linux-kernel@vger.kernel.org
> Cc: linux...@vger.kernel.org
>
> ---
> Changes from v1:
> - added "unlikely()" around the statement (SteveR)
> ---
>  include/linux/sched/cpufreq.h| 1 +
>  kernel/sched/cpufreq_schedutil.c | 7 +++
>  kernel/sched/idle_task.c | 4 
>  3 files changed, 12 insertions(+)
>
> diff --git a/include/linux/sched/cpufreq.h b/include/linux/sched/cpufreq.h
> index d2be2cc..36ac8d2 100644
> --- a/include/linux/sched/cpufreq.h
> +++ b/include/linux/sched/cpufreq.h
> @@ -10,6 +10,7 @@
>  #define SCHED_CPUFREQ_RT   (1U << 0)
>  #define SCHED_CPUFREQ_DL   (1U << 1)
>  #define SCHED_CPUFREQ_IOWAIT   (1U << 2)
> +#define SCHED_CPUFREQ_IDLE (1U << 3)
>
>  #define SCHED_CPUFREQ_RT_DL(SCHED_CPUFREQ_RT | SCHED_CPUFREQ_DL)
>
> diff --git a/kernel/sched/cpufreq_schedutil.c 
> b/kernel/sched/cpufreq_schedutil.c
> index eaba6d6..004ae18 100644
> --- a/kernel/sched/cpufreq_schedutil.c
> +++ b/kernel/sched/cpufreq_schedutil.c
> @@ -304,6 +304,12 @@ static void sugov_update_shared(struct update_util_data 
> *hook, u64 time,
>
> sg_cpu->util = util;
> sg_cpu->max = max;
> +
> +   /* CPU is entering IDLE, reset flags without triggering an update */
> +   if (unlikely(flags & SCHED_CPUFREQ_IDLE)) {
> +   sg_cpu->flags = 0;
> +   goto done;
> +   }

Instead of defining a new flag for idle, wouldn't another way be to
just clear the flag from the RT scheduling class with an extra call to
cpufreq_update_util with flags = 0 during dequeue_rt_entity? That
seems to me to be also the right place to clear the flag since the
flag is set in the corresponding class to begin with.

thanks,

-Joel

Re: [PATCH v2 2/6] cpufreq: schedutil: reset sg_cpus's flags at IDLE enter

[Joel Fernandes]

On Tue, Jul 4, 2017 at 10:34 AM, Patrick Bellasi
 wrote:
> Currently, sg_cpu's flags are set to the value defined by the last call of
> the cpufreq_update_util()/cpufreq_update_this_cpu(); for RT/DL classes
> this corresponds to the SCHED_CPUFREQ_{RT/DL} flags always being set.
>
> When multiple CPU shares the same frequency domain it might happen that a
> CPU which executed a RT task, right before entering IDLE, has one of the
> SCHED_CPUFREQ_RT_DL flags set, permanently, until it exits IDLE.
>
> Although such an idle CPU is _going to be_ ignored by the
> sugov_next_freq_shared():
>   1. this kind of "useless RT requests" are ignored only if more then
>  TICK_NSEC have elapsed since the last update
>   2. we can still potentially trigger an already too late switch to
>  MAX, which starts also a new throttling interval
>   3. the internal state machine is not consistent with what the
>  scheduler knows, i.e. the CPU is now actually idle
>
> Thus, in sugov_next_freq_shared(), where utilisation and flags are
> aggregated across all the CPUs of a frequency domain, it can turn out
> that all the CPUs of that domain can run unnecessary at the maximum OPP
> until another event happens in the idle CPU, which eventually clear the
> SCHED_CPUFREQ_{RT/DL} flag, or the IDLE CPUs gets ignored after
> TICK_NSEC since the CPU entering IDLE.
>
> Such a behaviour can harm the energy efficiency of systems where RT
> workloads are not so frequent and other CPUs in the same frequency
> domain are running small utilisation workloads, which is a quite common
> scenario in mobile embedded systems.
>
> This patch proposes a solution which is aligned with the current principle
> to update the flags each time a scheduling event happens. The scheduling
> of the idle_task on a CPU is considered one of such meaningful events.
> That's why when the idle_task is selected for execution we poke the
> schedutil policy to reset the flags for that CPU.
>
> No frequency transitions are activated at that point, which is fair in
> case the RT workload should come back in the future. However, this still
> allows other CPUs in the same frequency domain to scale down the
> frequency in case that should be possible.
>
> Signed-off-by: Patrick Bellasi 
> Cc: Ingo Molnar 
> Cc: Peter Zijlstra 
> Cc: Rafael J. Wysocki 
> Cc: Viresh Kumar 
> Cc: linux-kernel@vger.kernel.org
> Cc: linux...@vger.kernel.org
>
> ---
> Changes from v1:
> - added "unlikely()" around the statement (SteveR)
> ---
>  include/linux/sched/cpufreq.h| 1 +
>  kernel/sched/cpufreq_schedutil.c | 7 +++
>  kernel/sched/idle_task.c | 4 
>  3 files changed, 12 insertions(+)
>
> diff --git a/include/linux/sched/cpufreq.h b/include/linux/sched/cpufreq.h
> index d2be2cc..36ac8d2 100644
> --- a/include/linux/sched/cpufreq.h
> +++ b/include/linux/sched/cpufreq.h
> @@ -10,6 +10,7 @@
>  #define SCHED_CPUFREQ_RT   (1U << 0)
>  #define SCHED_CPUFREQ_DL   (1U << 1)
>  #define SCHED_CPUFREQ_IOWAIT   (1U << 2)
> +#define SCHED_CPUFREQ_IDLE (1U << 3)
>
>  #define SCHED_CPUFREQ_RT_DL(SCHED_CPUFREQ_RT | SCHED_CPUFREQ_DL)
>
> diff --git a/kernel/sched/cpufreq_schedutil.c 
> b/kernel/sched/cpufreq_schedutil.c
> index eaba6d6..004ae18 100644
> --- a/kernel/sched/cpufreq_schedutil.c
> +++ b/kernel/sched/cpufreq_schedutil.c
> @@ -304,6 +304,12 @@ static void sugov_update_shared(struct update_util_data 
> *hook, u64 time,
>
> sg_cpu->util = util;
> sg_cpu->max = max;
> +
> +   /* CPU is entering IDLE, reset flags without triggering an update */
> +   if (unlikely(flags & SCHED_CPUFREQ_IDLE)) {
> +   sg_cpu->flags = 0;
> +   goto done;
> +   }

Instead of defining a new flag for idle, wouldn't another way be to
just clear the flag from the RT scheduling class with an extra call to
cpufreq_update_util with flags = 0 during dequeue_rt_entity? That
seems to me to be also the right place to clear the flag since the
flag is set in the corresponding class to begin with.

thanks,

-Joel

Re: [PATCH v2] arm: eBPF JIT compiler

[Kees Cook]

On Wed, Jul 5, 2017 at 8:49 PM, Shubham Bansal
 wrote:
> Hi Kees,
>
> Problem is my ARM machine don't have clang and iproute2 which is
> keeping me from testing the bpf tail calls.
>
> You should do the following to test it,.
>
> 1. tools/testing/selftests/bpf/
> 2. make
> 3. sudo ./test_progs
>
> And, before testing, you have to do "make headers_install".
> These tests are for tail calls with the attached patch. If its too
> much work, Can you please upload your arm image so that I can test it?
> I just need a good machine.

I've got all this set up now, and it faults during the test:

Unable to handle kernel NULL pointer dereference at virtual address 0008
...
CPU: 0 PID: 1922 Comm: test_progs Not tainted 4.12.0+ #60
...
PC is at __htab_map_lookup_elem+0x54/0x1f4

I'll see if I can send you this disk image...

-Kees


-- 
Kees Cook
Pixel Security

Re: [PATCH v2] arm: eBPF JIT compiler

[Kees Cook]

On Wed, Jul 5, 2017 at 8:49 PM, Shubham Bansal
 wrote:
> Hi Kees,
>
> Problem is my ARM machine don't have clang and iproute2 which is
> keeping me from testing the bpf tail calls.
>
> You should do the following to test it,.
>
> 1. tools/testing/selftests/bpf/
> 2. make
> 3. sudo ./test_progs
>
> And, before testing, you have to do "make headers_install".
> These tests are for tail calls with the attached patch. If its too
> much work, Can you please upload your arm image so that I can test it?
> I just need a good machine.

I've got all this set up now, and it faults during the test:

Unable to handle kernel NULL pointer dereference at virtual address 0008
...
CPU: 0 PID: 1922 Comm: test_progs Not tainted 4.12.0+ #60
...
PC is at __htab_map_lookup_elem+0x54/0x1f4

I'll see if I can send you this disk image...

-Kees


-- 
Kees Cook
Pixel Security

Re: [PATCH] iio: multiplexer: add NULL check on devm_kzalloc() return value

[Gustavo A. R. Silva]

Hi Peter,

Quoting Peter Rosin :


On 2017-07-07 00:08, Gustavo A. R. Silva wrote:

Check return value from call to devm_kzalloc()
in order to prevent a NULL pointer dereference.


Right, thanks for finding that one! There's another one inside the
for loop that is just starting in the context of this patch. Care
to fix checking the return value of that devm_kmemdup as well?



Sure, I'll send a new patch shortly.


And someone should perhaps teach Coccinelle about devm_kmemdup...



Good catch, I just implemented that script.


This issue was detected using Coccinelle and the following semantic patch:

@@
expression x;
identifier fld;
@@

* x = devm_kzalloc(...);
  ... when != x == NULL
  x->fld




One of these blank lines should perhaps be a "Fixes:" tag?



mmm, I don't get this...


Cheers,
peda


Signed-off-by: Gustavo A. R. Silva 
---
 drivers/iio/multiplexer/iio-mux.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/iio/multiplexer/iio-mux.c  
b/drivers/iio/multiplexer/iio-mux.c

index 37ba007..a8d672b 100644
--- a/drivers/iio/multiplexer/iio-mux.c
+++ b/drivers/iio/multiplexer/iio-mux.c
@@ -285,6 +285,9 @@ static int mux_configure_channel(struct device  
*dev, struct mux *mux,

child->ext_info_cache = devm_kzalloc(dev,
 sizeof(*child->ext_info_cache) *
 num_ext_info, GFP_KERNEL);
+   if (!child->ext_info_cache)
+   return -ENOMEM;
+
for (i = 0; i < num_ext_info; ++i) {
child->ext_info_cache[i].size = -1;




Thanks!
--
Gustavo A. R. Silva

Re: [PATCH] iio: multiplexer: add NULL check on devm_kzalloc() return value

[Gustavo A. R. Silva]

Hi Peter,

Quoting Peter Rosin :


On 2017-07-07 00:08, Gustavo A. R. Silva wrote:

Check return value from call to devm_kzalloc()
in order to prevent a NULL pointer dereference.


Right, thanks for finding that one! There's another one inside the
for loop that is just starting in the context of this patch. Care
to fix checking the return value of that devm_kmemdup as well?



Sure, I'll send a new patch shortly.


And someone should perhaps teach Coccinelle about devm_kmemdup...



Good catch, I just implemented that script.


This issue was detected using Coccinelle and the following semantic patch:

@@
expression x;
identifier fld;
@@

* x = devm_kzalloc(...);
  ... when != x == NULL
  x->fld




One of these blank lines should perhaps be a "Fixes:" tag?



mmm, I don't get this...


Cheers,
peda


Signed-off-by: Gustavo A. R. Silva 
---
 drivers/iio/multiplexer/iio-mux.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/iio/multiplexer/iio-mux.c  
b/drivers/iio/multiplexer/iio-mux.c

index 37ba007..a8d672b 100644
--- a/drivers/iio/multiplexer/iio-mux.c
+++ b/drivers/iio/multiplexer/iio-mux.c
@@ -285,6 +285,9 @@ static int mux_configure_channel(struct device  
*dev, struct mux *mux,

child->ext_info_cache = devm_kzalloc(dev,
 sizeof(*child->ext_info_cache) *
 num_ext_info, GFP_KERNEL);
+   if (!child->ext_info_cache)
+   return -ENOMEM;
+
for (i = 0; i < num_ext_info; ++i) {
child->ext_info_cache[i].size = -1;




Thanks!
--
Gustavo A. R. Silva

Re: [PATCH] target: make device_mutex and device_list static

[Nicholas A. Bellinger]

On Wed, 2017-07-05 at 13:15 -0500, Mike Christie wrote:
> On 07/04/2017 03:44 AM, Colin King wrote:
> > From: Colin Ian King 
> > 
> > Variables device_mutex and device_list static are local to the source,
> > so make them static.
> > 
> > Cleans up sparse warnings:
> > "symbol 'device_list' was not declared. Should it be static?"
> > "symbol 'device_mutex' was not declared. Should it be static?"
> > 
> > Signed-off-by: Colin Ian King 
> > ---
> >  drivers/target/target_core_device.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/target/target_core_device.c 
> > b/drivers/target/target_core_device.c
> > index 3ae8fbf01bdf..bbcef3bc66c8 100644
> > --- a/drivers/target/target_core_device.c
> > +++ b/drivers/target/target_core_device.c
> > @@ -49,8 +49,8 @@
> >  #include "target_core_pr.h"
> >  #include "target_core_ua.h"
> >  
> > -DEFINE_MUTEX(device_mutex);
> > -LIST_HEAD(device_list);
> > +static DEFINE_MUTEX(device_mutex);
> > +static LIST_HEAD(device_list);
> >  static DEFINE_IDR(devices_idr);
> >  
> >  static struct se_hba *lun0_hba;
> > 
> 
> My fault. Thanks.
> 
> Reviewed-by: Mike Christie 

Applied.

Thanks Colin + MNC.

Re: [PATCH] target: make device_mutex and device_list static

[Nicholas A. Bellinger]

On Wed, 2017-07-05 at 13:15 -0500, Mike Christie wrote:
> On 07/04/2017 03:44 AM, Colin King wrote:
> > From: Colin Ian King 
> > 
> > Variables device_mutex and device_list static are local to the source,
> > so make them static.
> > 
> > Cleans up sparse warnings:
> > "symbol 'device_list' was not declared. Should it be static?"
> > "symbol 'device_mutex' was not declared. Should it be static?"
> > 
> > Signed-off-by: Colin Ian King 
> > ---
> >  drivers/target/target_core_device.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/target/target_core_device.c 
> > b/drivers/target/target_core_device.c
> > index 3ae8fbf01bdf..bbcef3bc66c8 100644
> > --- a/drivers/target/target_core_device.c
> > +++ b/drivers/target/target_core_device.c
> > @@ -49,8 +49,8 @@
> >  #include "target_core_pr.h"
> >  #include "target_core_ua.h"
> >  
> > -DEFINE_MUTEX(device_mutex);
> > -LIST_HEAD(device_list);
> > +static DEFINE_MUTEX(device_mutex);
> > +static LIST_HEAD(device_list);
> >  static DEFINE_IDR(devices_idr);
> >  
> >  static struct se_hba *lun0_hba;
> > 
> 
> My fault. Thanks.
> 
> Reviewed-by: Mike Christie 

Applied.

Thanks Colin + MNC.

Re: [PATCH] extcon: int3496: constify acpi_device_id.

[Chanwoo Choi]

On 2017년 07월 07일 01:55, Arvind Yadav wrote:
> acpi_device_id are not supposed to change at runtime. All functions
> working with acpi_device_id provided by  work with
> const acpi_device_id. So mark the non-const structs as const.
> 
> File size before:
>text  data bss dec hex filename
>1733   352   02085 825 
> drivers/extcon/extcon-intel-int3496.o
> 
> File size After adding 'const':
>text  data bss dec hex filename
>1797   272   02069 815 
> drivers/extcon/extcon-intel-int3496.o
> 
> Signed-off-by: Arvind Yadav 
> ---
>  drivers/extcon/extcon-intel-int3496.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/extcon/extcon-intel-int3496.c 
> b/drivers/extcon/extcon-intel-int3496.c
> index 9d17984..49bf9c6 100644
> --- a/drivers/extcon/extcon-intel-int3496.c
> +++ b/drivers/extcon/extcon-intel-int3496.c
> @@ -174,7 +174,7 @@ static int int3496_remove(struct platform_device *pdev)
>   return 0;
>  }
>  
> -static struct acpi_device_id int3496_acpi_match[] = {
> +static const struct acpi_device_id int3496_acpi_match[] = {
>   { "INT3496" },
>   { }
>  };
> 

Applied it.

-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

Re: [PATCH] extcon: int3496: constify acpi_device_id.

[Chanwoo Choi]

On 2017년 07월 07일 01:55, Arvind Yadav wrote:
> acpi_device_id are not supposed to change at runtime. All functions
> working with acpi_device_id provided by  work with
> const acpi_device_id. So mark the non-const structs as const.
> 
> File size before:
>text  data bss dec hex filename
>1733   352   02085 825 
> drivers/extcon/extcon-intel-int3496.o
> 
> File size After adding 'const':
>text  data bss dec hex filename
>1797   272   02069 815 
> drivers/extcon/extcon-intel-int3496.o
> 
> Signed-off-by: Arvind Yadav 
> ---
>  drivers/extcon/extcon-intel-int3496.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/extcon/extcon-intel-int3496.c 
> b/drivers/extcon/extcon-intel-int3496.c
> index 9d17984..49bf9c6 100644
> --- a/drivers/extcon/extcon-intel-int3496.c
> +++ b/drivers/extcon/extcon-intel-int3496.c
> @@ -174,7 +174,7 @@ static int int3496_remove(struct platform_device *pdev)
>   return 0;
>  }
>  
> -static struct acpi_device_id int3496_acpi_match[] = {
> +static const struct acpi_device_id int3496_acpi_match[] = {
>   { "INT3496" },
>   { }
>  };
> 

Applied it.

-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

Re: [PATCH] iio: multiplexer: add NULL check on devm_kzalloc() return value

[Peter Rosin]

On 2017-07-07 00:08, Gustavo A. R. Silva wrote:
> Check return value from call to devm_kzalloc()
> in order to prevent a NULL pointer dereference.

Right, thanks for finding that one! There's another one inside the
for loop that is just starting in the context of this patch. Care
to fix checking the return value of that devm_kmemdup as well?

And someone should perhaps teach Coccinelle about devm_kmemdup...

> This issue was detected using Coccinelle and the following semantic patch:
> 
> @@
> expression x;
> identifier fld;
> @@
> 
> * x = devm_kzalloc(...);
>   ... when != x == NULL
>   x->fld
> 
> 

One of these blank lines should perhaps be a "Fixes:" tag?

Cheers,
peda

> Signed-off-by: Gustavo A. R. Silva 
> ---
>  drivers/iio/multiplexer/iio-mux.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/iio/multiplexer/iio-mux.c 
> b/drivers/iio/multiplexer/iio-mux.c
> index 37ba007..a8d672b 100644
> --- a/drivers/iio/multiplexer/iio-mux.c
> +++ b/drivers/iio/multiplexer/iio-mux.c
> @@ -285,6 +285,9 @@ static int mux_configure_channel(struct device *dev, 
> struct mux *mux,
>   child->ext_info_cache = devm_kzalloc(dev,
>sizeof(*child->ext_info_cache) *
>num_ext_info, GFP_KERNEL);
> + if (!child->ext_info_cache)
> + return -ENOMEM;
> +
>   for (i = 0; i < num_ext_info; ++i) {
>   child->ext_info_cache[i].size = -1;
>  
> 

Re: [PATCH] iio: multiplexer: add NULL check on devm_kzalloc() return value

[Peter Rosin]

On 2017-07-07 00:08, Gustavo A. R. Silva wrote:
> Check return value from call to devm_kzalloc()
> in order to prevent a NULL pointer dereference.

Right, thanks for finding that one! There's another one inside the
for loop that is just starting in the context of this patch. Care
to fix checking the return value of that devm_kmemdup as well?

And someone should perhaps teach Coccinelle about devm_kmemdup...

> This issue was detected using Coccinelle and the following semantic patch:
> 
> @@
> expression x;
> identifier fld;
> @@
> 
> * x = devm_kzalloc(...);
>   ... when != x == NULL
>   x->fld
> 
> 

One of these blank lines should perhaps be a "Fixes:" tag?

Cheers,
peda

> Signed-off-by: Gustavo A. R. Silva 
> ---
>  drivers/iio/multiplexer/iio-mux.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/iio/multiplexer/iio-mux.c 
> b/drivers/iio/multiplexer/iio-mux.c
> index 37ba007..a8d672b 100644
> --- a/drivers/iio/multiplexer/iio-mux.c
> +++ b/drivers/iio/multiplexer/iio-mux.c
> @@ -285,6 +285,9 @@ static int mux_configure_channel(struct device *dev, 
> struct mux *mux,
>   child->ext_info_cache = devm_kzalloc(dev,
>sizeof(*child->ext_info_cache) *
>num_ext_info, GFP_KERNEL);
> + if (!child->ext_info_cache)
> + return -ENOMEM;
> +
>   for (i = 0; i < num_ext_info; ++i) {
>   child->ext_info_cache[i].size = -1;
>  
> 

[PATCH v3 2/3] dt-bindings: cpufreq: move MediaTek cpufreq dt-bindings document to proper place

[sean.wang]

From: Sean Wang 

The old place is Documentation/devicetree/bindings/clock/ that would
let people hard to find how to use MediaTek cpufreq driver, so moving
it to the appropriate place as other cpufreq drivers done would be
better.

Signed-off-by: Sean Wang 
Acked-by: Viresh Kumar 
---
 .../bindings/{clock/mt8173-cpu-dvfs.txt => cpufreq/cpufreq-mediatek.txt}  | 0
 1 file changed, 0 insertions(+), 0 deletions(-)
 rename Documentation/devicetree/bindings/{clock/mt8173-cpu-dvfs.txt => 
cpufreq/cpufreq-mediatek.txt} (100%)

diff --git a/Documentation/devicetree/bindings/clock/mt8173-cpu-dvfs.txt 
b/Documentation/devicetree/bindings/cpufreq/cpufreq-mediatek.txt
similarity index 100%
rename from Documentation/devicetree/bindings/clock/mt8173-cpu-dvfs.txt
rename to Documentation/devicetree/bindings/cpufreq/cpufreq-mediatek.txt
-- 
2.7.4

[PATCH v3 0/3] some fixups for MediaTek cpufreq driver

[sean.wang]

From: Sean Wang 

Changes since v2:
- correct the typo in the binding document

Changes since v1:
- drop those patches already accepted
- refine the commit messages and Kconfig dependency
- Kconfig menu entry and file name itself are updated with more
generic name to drop "MT8173" since this driver actually supports
all MediaTek SoCs.
- generate patchset again with git format-patch -C -M --thread=shallow
- fix binding examples by replacing '@' with '-' as the OPP nodes will
never have a "reg" property.

Hi,

The purpose of the series is
- (patch 1 to 3) to fix up current Mediatek cpufreq driver
can't work with the latest tree since one required CPU clock muxer missing
would cause the driver getting the resource fails when driver probe gets
called.
- (patch 4) to enable cpufreq feature on MT2701/MT7623 platform.
- (patch 5 to 6) to update the binding document to reflect
latest driver logic and add more examples guiding people how to apply for
Mediatek cpufreq driver.

Sean Wang (3):
  cpufreq: mediatek: Add support of cpufreq to MT2701/MT7623 SoC
  dt-bindings: cpufreq: move MediaTek cpufreq dt-bindings document to
proper place
  dt-bindings: cpufreq: enhance MediaTek cpufreq dt-binding document

 .../devicetree/bindings/clock/mt8173-cpu-dvfs.txt  |  83 ---
 .../bindings/cpufreq/cpufreq-mediatek.txt  | 247 +
 drivers/cpufreq/Kconfig.arm|   8 +-
 drivers/cpufreq/Makefile   |   2 +-
 .../cpufreq/{mt8173-cpufreq.c => mtk-cpufreq.c}|   2 +
 5 files changed, 254 insertions(+), 88 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/clock/mt8173-cpu-dvfs.txt
 create mode 100644 
Documentation/devicetree/bindings/cpufreq/cpufreq-mediatek.txt
 rename drivers/cpufreq/{mt8173-cpufreq.c => mtk-cpufreq.c} (99%)

-- 
2.7.4

[PATCH v3 2/3] dt-bindings: cpufreq: move MediaTek cpufreq dt-bindings document to proper place

[sean.wang]

From: Sean Wang 

The old place is Documentation/devicetree/bindings/clock/ that would
let people hard to find how to use MediaTek cpufreq driver, so moving
it to the appropriate place as other cpufreq drivers done would be
better.

Signed-off-by: Sean Wang 
Acked-by: Viresh Kumar 
---
 .../bindings/{clock/mt8173-cpu-dvfs.txt => cpufreq/cpufreq-mediatek.txt}  | 0
 1 file changed, 0 insertions(+), 0 deletions(-)
 rename Documentation/devicetree/bindings/{clock/mt8173-cpu-dvfs.txt => 
cpufreq/cpufreq-mediatek.txt} (100%)

diff --git a/Documentation/devicetree/bindings/clock/mt8173-cpu-dvfs.txt 
b/Documentation/devicetree/bindings/cpufreq/cpufreq-mediatek.txt
similarity index 100%
rename from Documentation/devicetree/bindings/clock/mt8173-cpu-dvfs.txt
rename to Documentation/devicetree/bindings/cpufreq/cpufreq-mediatek.txt
-- 
2.7.4

[PATCH v3 0/3] some fixups for MediaTek cpufreq driver

[sean.wang]

From: Sean Wang 

Changes since v2:
- correct the typo in the binding document

Changes since v1:
- drop those patches already accepted
- refine the commit messages and Kconfig dependency
- Kconfig menu entry and file name itself are updated with more
generic name to drop "MT8173" since this driver actually supports
all MediaTek SoCs.
- generate patchset again with git format-patch -C -M --thread=shallow
- fix binding examples by replacing '@' with '-' as the OPP nodes will
never have a "reg" property.

Hi,

The purpose of the series is
- (patch 1 to 3) to fix up current Mediatek cpufreq driver
can't work with the latest tree since one required CPU clock muxer missing
would cause the driver getting the resource fails when driver probe gets
called.
- (patch 4) to enable cpufreq feature on MT2701/MT7623 platform.
- (patch 5 to 6) to update the binding document to reflect
latest driver logic and add more examples guiding people how to apply for
Mediatek cpufreq driver.

Sean Wang (3):
  cpufreq: mediatek: Add support of cpufreq to MT2701/MT7623 SoC
  dt-bindings: cpufreq: move MediaTek cpufreq dt-bindings document to
proper place
  dt-bindings: cpufreq: enhance MediaTek cpufreq dt-binding document

 .../devicetree/bindings/clock/mt8173-cpu-dvfs.txt  |  83 ---
 .../bindings/cpufreq/cpufreq-mediatek.txt  | 247 +
 drivers/cpufreq/Kconfig.arm|   8 +-
 drivers/cpufreq/Makefile   |   2 +-
 .../cpufreq/{mt8173-cpufreq.c => mtk-cpufreq.c}|   2 +
 5 files changed, 254 insertions(+), 88 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/clock/mt8173-cpu-dvfs.txt
 create mode 100644 
Documentation/devicetree/bindings/cpufreq/cpufreq-mediatek.txt
 rename drivers/cpufreq/{mt8173-cpufreq.c => mtk-cpufreq.c} (99%)

-- 
2.7.4

[PATCH v3 3/3] dt-bindings: cpufreq: enhance MediaTek cpufreq dt-binding document

[sean.wang]

From: Sean Wang 

Update binding document with adding operating-points-v2 as the required
property and the cooling level as the optional properties and adding more
examples guiding people how to use MediaTek cpufreq driver for MediaTek
SoCs.

Signed-off-by: Sean Wang 
Acked-by: Viresh Kumar 
---
 .../bindings/cpufreq/cpufreq-mediatek.txt  | 170 -
 1 file changed, 167 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/cpufreq/cpufreq-mediatek.txt 
b/Documentation/devicetree/bindings/cpufreq/cpufreq-mediatek.txt
index 52b457c..f640308 100644
--- a/Documentation/devicetree/bindings/cpufreq/cpufreq-mediatek.txt
+++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-mediatek.txt
@@ -1,4 +1,5 @@
-Device Tree Clock bindins for CPU DVFS of Mediatek MT8173 SoC
+Binding for MediaTek's CPUFreq driver
+=
 
 Required properties:
 - clocks: A list of phandle + clock-specifier pairs for the clocks listed in 
clock names.
@@ -9,6 +10,8 @@ Required properties:
  transition and not stable yet.
Please refer to 
Documentation/devicetree/bindings/clk/clock-bindings.txt for
generic clock consumer properties.
+- operating-points-v2: Please refer to 
Documentation/devicetree/bindings/opp/opp.txt
+   for detail.
 - proc-supply: Regulator for Vproc of CPU cluster.
 
 Optional properties:
@@ -17,9 +20,166 @@ Optional properties:
   Vsram to fit SoC specific needs. When absent, the voltage scaling
   flow is handled by hardware, hence no software "voltage 
tracking" is
   needed.
+- #cooling-cells:
+- cooling-min-level:
+- cooling-max-level:
+   Please refer to Documentation/devicetree/bindings/thermal/thermal.txt
+   for detail.
+
+Example 1 (MT7623 SoC):
+
+   cpu_opp_table: opp_table {
+   compatible = "operating-points-v2";
+   opp-shared;
+
+   opp-59800 {
+   opp-hz = /bits/ 64 <59800>;
+   opp-microvolt = <105>;
+   };
+
+   opp-74750 {
+   opp-hz = /bits/ 64 <74750>;
+   opp-microvolt = <105>;
+   };
+
+   opp-104000 {
+   opp-hz = /bits/ 64 <104000>;
+   opp-microvolt = <115>;
+   };
+
+   opp-119600 {
+   opp-hz = /bits/ 64 <119600>;
+   opp-microvolt = <120>;
+   };
+
+   opp-13 {
+   opp-hz = /bits/ 64 <13>;
+   opp-microvolt = <130>;
+   };
+   };
+
+   cpu0: cpu@0 {
+   device_type = "cpu";
+   compatible = "arm,cortex-a7";
+   reg = <0x0>;
+   clocks = < CLK_INFRA_CPUSEL>,
+< CLK_APMIXED_MAINPLL>;
+   clock-names = "cpu", "intermediate";
+   operating-points-v2 = <_opp_table>;
+   #cooling-cells = <2>;
+   cooling-min-level = <0>;
+   cooling-max-level = <7>;
+   };
+   cpu@1 {
+   device_type = "cpu";
+   compatible = "arm,cortex-a7";
+   reg = <0x1>;
+   operating-points-v2 = <_opp_table>;
+   };
+   cpu@2 {
+   device_type = "cpu";
+   compatible = "arm,cortex-a7";
+   reg = <0x2>;
+   operating-points-v2 = <_opp_table>;
+   };
+   cpu@3 {
+   device_type = "cpu";
+   compatible = "arm,cortex-a7";
+   reg = <0x3>;
+   operating-points-v2 = <_opp_table>;
+   };
+
+Example 2 (MT8173 SoC):
+   cpu_opp_table_a: opp_table_a {
+   compatible = "operating-points-v2";
+   opp-shared;
+
+   opp-50700 {
+   opp-hz = /bits/ 64 <50700>;
+   opp-microvolt = <859000>;
+   };
+
+   opp-70200 {
+   opp-hz = /bits/ 64 <70200>;
+   opp-microvolt = <908000>;
+   };
+
+   opp-100100 {
+   opp-hz = /bits/ 64 <100100>;
+   opp-microvolt = <983000>;
+   };
+
+   opp-110500 {
+   opp-hz = /bits/ 64 <110500>;
+   opp-microvolt = <1009000>;
+   };
+
+   opp-118300 {
+   opp-hz = /bits/ 64 <118300>;
+   opp-microvolt = <1028000>;
+   };
+
+   opp-140400 {
+   opp-hz = /bits/ 64 <140400>;
+   opp-microvolt = <1083000>;
+   };
+
+ 

[PATCH v3 1/3] cpufreq: mediatek: Add support of cpufreq to MT2701/MT7623 SoC

[sean.wang]

From: Sean Wang 

MT2701/MT7623 is a 32-bit ARMv7 based quad-core (4 * Cortex-A7) with
single cluster and this hardware is also compatible with the existing
driver through enabling CPU frequency feature with operating-points-v2
bindings. Also, this driver actually supports all MediaTek SoCs, the
Kconfig menu entry and file name itself should be updated with more
generic name to drop "MT8173"

Signed-off-by: Sean Wang 
Acked-by: Viresh Kumar 
---
 drivers/cpufreq/Kconfig.arm | 8 
 drivers/cpufreq/Makefile| 2 +-
 drivers/cpufreq/{mt8173-cpufreq.c => mtk-cpufreq.c} | 2 ++
 3 files changed, 7 insertions(+), 5 deletions(-)
 rename drivers/cpufreq/{mt8173-cpufreq.c => mtk-cpufreq.c} (99%)

diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
index 74ed7e9..79aece7 100644
--- a/drivers/cpufreq/Kconfig.arm
+++ b/drivers/cpufreq/Kconfig.arm
@@ -87,14 +87,14 @@ config ARM_KIRKWOOD_CPUFREQ
  This adds the CPUFreq driver for Marvell Kirkwood
  SoCs.
 
-config ARM_MT8173_CPUFREQ
-   tristate "Mediatek MT8173 CPUFreq support"
+config ARM_MEDIATEK_CPUFREQ
+   tristate "CPU Frequency scaling support for MediaTek SoCs"
depends on ARCH_MEDIATEK && REGULATOR
-   depends on ARM64 || (ARM_CPU_TOPOLOGY && COMPILE_TEST)
+   depends on ARM || ARM64 || COMPILE_TEST
depends on !CPU_THERMAL || THERMAL
select PM_OPP
help
- This adds the CPUFreq driver support for Mediatek MT8173 SoC.
+ This adds the CPUFreq driver support for MediaTek SoCs.
 
 config ARM_OMAP2PLUS_CPUFREQ
bool "TI OMAP2+"
diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile
index b7e78f0..4956f5d 100644
--- a/drivers/cpufreq/Makefile
+++ b/drivers/cpufreq/Makefile
@@ -58,7 +58,7 @@ obj-$(CONFIG_ARM_EXYNOS5440_CPUFREQ)  += exynos5440-cpufreq.o
 obj-$(CONFIG_ARM_HIGHBANK_CPUFREQ) += highbank-cpufreq.o
 obj-$(CONFIG_ARM_IMX6Q_CPUFREQ)+= imx6q-cpufreq.o
 obj-$(CONFIG_ARM_KIRKWOOD_CPUFREQ) += kirkwood-cpufreq.o
-obj-$(CONFIG_ARM_MT8173_CPUFREQ)   += mt8173-cpufreq.o
+obj-$(CONFIG_ARM_MEDIATEK_CPUFREQ) += mtk-cpufreq.o
 obj-$(CONFIG_ARM_OMAP2PLUS_CPUFREQ)+= omap-cpufreq.o
 obj-$(CONFIG_ARM_PXA2xx_CPUFREQ)   += pxa2xx-cpufreq.o
 obj-$(CONFIG_PXA3xx)   += pxa3xx-cpufreq.o
diff --git a/drivers/cpufreq/mt8173-cpufreq.c b/drivers/cpufreq/mtk-cpufreq.c
similarity index 99%
rename from drivers/cpufreq/mt8173-cpufreq.c
rename to drivers/cpufreq/mtk-cpufreq.c
index fd1886f..481ec77 100644
--- a/drivers/cpufreq/mt8173-cpufreq.c
+++ b/drivers/cpufreq/mtk-cpufreq.c
@@ -575,6 +575,8 @@ static struct platform_driver mt8173_cpufreq_platdrv = {
 
 /* List of machines supported by this driver */
 static const struct of_device_id mt8173_cpufreq_machines[] __initconst = {
+   { .compatible = "mediatek,mt2701", },
+   { .compatible = "mediatek,mt7623", },
{ .compatible = "mediatek,mt817x", },
{ .compatible = "mediatek,mt8173", },
{ .compatible = "mediatek,mt8176", },
-- 
2.7.4

[PATCH v3 3/3] dt-bindings: cpufreq: enhance MediaTek cpufreq dt-binding document

[sean.wang]

From: Sean Wang 

Update binding document with adding operating-points-v2 as the required
property and the cooling level as the optional properties and adding more
examples guiding people how to use MediaTek cpufreq driver for MediaTek
SoCs.

Signed-off-by: Sean Wang 
Acked-by: Viresh Kumar 
---
 .../bindings/cpufreq/cpufreq-mediatek.txt  | 170 -
 1 file changed, 167 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/cpufreq/cpufreq-mediatek.txt 
b/Documentation/devicetree/bindings/cpufreq/cpufreq-mediatek.txt
index 52b457c..f640308 100644
--- a/Documentation/devicetree/bindings/cpufreq/cpufreq-mediatek.txt
+++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-mediatek.txt
@@ -1,4 +1,5 @@
-Device Tree Clock bindins for CPU DVFS of Mediatek MT8173 SoC
+Binding for MediaTek's CPUFreq driver
+=
 
 Required properties:
 - clocks: A list of phandle + clock-specifier pairs for the clocks listed in 
clock names.
@@ -9,6 +10,8 @@ Required properties:
  transition and not stable yet.
Please refer to 
Documentation/devicetree/bindings/clk/clock-bindings.txt for
generic clock consumer properties.
+- operating-points-v2: Please refer to 
Documentation/devicetree/bindings/opp/opp.txt
+   for detail.
 - proc-supply: Regulator for Vproc of CPU cluster.
 
 Optional properties:
@@ -17,9 +20,166 @@ Optional properties:
   Vsram to fit SoC specific needs. When absent, the voltage scaling
   flow is handled by hardware, hence no software "voltage 
tracking" is
   needed.
+- #cooling-cells:
+- cooling-min-level:
+- cooling-max-level:
+   Please refer to Documentation/devicetree/bindings/thermal/thermal.txt
+   for detail.
+
+Example 1 (MT7623 SoC):
+
+   cpu_opp_table: opp_table {
+   compatible = "operating-points-v2";
+   opp-shared;
+
+   opp-59800 {
+   opp-hz = /bits/ 64 <59800>;
+   opp-microvolt = <105>;
+   };
+
+   opp-74750 {
+   opp-hz = /bits/ 64 <74750>;
+   opp-microvolt = <105>;
+   };
+
+   opp-104000 {
+   opp-hz = /bits/ 64 <104000>;
+   opp-microvolt = <115>;
+   };
+
+   opp-119600 {
+   opp-hz = /bits/ 64 <119600>;
+   opp-microvolt = <120>;
+   };
+
+   opp-13 {
+   opp-hz = /bits/ 64 <13>;
+   opp-microvolt = <130>;
+   };
+   };
+
+   cpu0: cpu@0 {
+   device_type = "cpu";
+   compatible = "arm,cortex-a7";
+   reg = <0x0>;
+   clocks = < CLK_INFRA_CPUSEL>,
+< CLK_APMIXED_MAINPLL>;
+   clock-names = "cpu", "intermediate";
+   operating-points-v2 = <_opp_table>;
+   #cooling-cells = <2>;
+   cooling-min-level = <0>;
+   cooling-max-level = <7>;
+   };
+   cpu@1 {
+   device_type = "cpu";
+   compatible = "arm,cortex-a7";
+   reg = <0x1>;
+   operating-points-v2 = <_opp_table>;
+   };
+   cpu@2 {
+   device_type = "cpu";
+   compatible = "arm,cortex-a7";
+   reg = <0x2>;
+   operating-points-v2 = <_opp_table>;
+   };
+   cpu@3 {
+   device_type = "cpu";
+   compatible = "arm,cortex-a7";
+   reg = <0x3>;
+   operating-points-v2 = <_opp_table>;
+   };
+
+Example 2 (MT8173 SoC):
+   cpu_opp_table_a: opp_table_a {
+   compatible = "operating-points-v2";
+   opp-shared;
+
+   opp-50700 {
+   opp-hz = /bits/ 64 <50700>;
+   opp-microvolt = <859000>;
+   };
+
+   opp-70200 {
+   opp-hz = /bits/ 64 <70200>;
+   opp-microvolt = <908000>;
+   };
+
+   opp-100100 {
+   opp-hz = /bits/ 64 <100100>;
+   opp-microvolt = <983000>;
+   };
+
+   opp-110500 {
+   opp-hz = /bits/ 64 <110500>;
+   opp-microvolt = <1009000>;
+   };
+
+   opp-118300 {
+   opp-hz = /bits/ 64 <118300>;
+   opp-microvolt = <1028000>;
+   };
+
+   opp-140400 {
+   opp-hz = /bits/ 64 <140400>;
+   opp-microvolt = <1083000>;
+   };
+
+   opp-150800 {
+   opp-hz = /bits/ 

[PATCH v3 1/3] cpufreq: mediatek: Add support of cpufreq to MT2701/MT7623 SoC

[sean.wang]

From: Sean Wang 

MT2701/MT7623 is a 32-bit ARMv7 based quad-core (4 * Cortex-A7) with
single cluster and this hardware is also compatible with the existing
driver through enabling CPU frequency feature with operating-points-v2
bindings. Also, this driver actually supports all MediaTek SoCs, the
Kconfig menu entry and file name itself should be updated with more
generic name to drop "MT8173"

Signed-off-by: Sean Wang 
Acked-by: Viresh Kumar 
---
 drivers/cpufreq/Kconfig.arm | 8 
 drivers/cpufreq/Makefile| 2 +-
 drivers/cpufreq/{mt8173-cpufreq.c => mtk-cpufreq.c} | 2 ++
 3 files changed, 7 insertions(+), 5 deletions(-)
 rename drivers/cpufreq/{mt8173-cpufreq.c => mtk-cpufreq.c} (99%)

diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
index 74ed7e9..79aece7 100644
--- a/drivers/cpufreq/Kconfig.arm
+++ b/drivers/cpufreq/Kconfig.arm
@@ -87,14 +87,14 @@ config ARM_KIRKWOOD_CPUFREQ
  This adds the CPUFreq driver for Marvell Kirkwood
  SoCs.
 
-config ARM_MT8173_CPUFREQ
-   tristate "Mediatek MT8173 CPUFreq support"
+config ARM_MEDIATEK_CPUFREQ
+   tristate "CPU Frequency scaling support for MediaTek SoCs"
depends on ARCH_MEDIATEK && REGULATOR
-   depends on ARM64 || (ARM_CPU_TOPOLOGY && COMPILE_TEST)
+   depends on ARM || ARM64 || COMPILE_TEST
depends on !CPU_THERMAL || THERMAL
select PM_OPP
help
- This adds the CPUFreq driver support for Mediatek MT8173 SoC.
+ This adds the CPUFreq driver support for MediaTek SoCs.
 
 config ARM_OMAP2PLUS_CPUFREQ
bool "TI OMAP2+"
diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile
index b7e78f0..4956f5d 100644
--- a/drivers/cpufreq/Makefile
+++ b/drivers/cpufreq/Makefile
@@ -58,7 +58,7 @@ obj-$(CONFIG_ARM_EXYNOS5440_CPUFREQ)  += exynos5440-cpufreq.o
 obj-$(CONFIG_ARM_HIGHBANK_CPUFREQ) += highbank-cpufreq.o
 obj-$(CONFIG_ARM_IMX6Q_CPUFREQ)+= imx6q-cpufreq.o
 obj-$(CONFIG_ARM_KIRKWOOD_CPUFREQ) += kirkwood-cpufreq.o
-obj-$(CONFIG_ARM_MT8173_CPUFREQ)   += mt8173-cpufreq.o
+obj-$(CONFIG_ARM_MEDIATEK_CPUFREQ) += mtk-cpufreq.o
 obj-$(CONFIG_ARM_OMAP2PLUS_CPUFREQ)+= omap-cpufreq.o
 obj-$(CONFIG_ARM_PXA2xx_CPUFREQ)   += pxa2xx-cpufreq.o
 obj-$(CONFIG_PXA3xx)   += pxa3xx-cpufreq.o
diff --git a/drivers/cpufreq/mt8173-cpufreq.c b/drivers/cpufreq/mtk-cpufreq.c
similarity index 99%
rename from drivers/cpufreq/mt8173-cpufreq.c
rename to drivers/cpufreq/mtk-cpufreq.c
index fd1886f..481ec77 100644
--- a/drivers/cpufreq/mt8173-cpufreq.c
+++ b/drivers/cpufreq/mtk-cpufreq.c
@@ -575,6 +575,8 @@ static struct platform_driver mt8173_cpufreq_platdrv = {
 
 /* List of machines supported by this driver */
 static const struct of_device_id mt8173_cpufreq_machines[] __initconst = {
+   { .compatible = "mediatek,mt2701", },
+   { .compatible = "mediatek,mt7623", },
{ .compatible = "mediatek,mt817x", },
{ .compatible = "mediatek,mt8173", },
{ .compatible = "mediatek,mt8176", },
-- 
2.7.4

Re: [RFC PATCH v1 3/8] sched/cpufreq_schedutil: make worker kthread be SCHED_DEADLINE

[Joel Fernandes]

Hi Juri,

On Wed, Jul 5, 2017 at 1:59 AM, Juri Lelli  wrote:
> Worker kthread needs to be able to change frequency for all other
> threads.
>
> Make it special, just under STOP class.
>
> Signed-off-by: Juri Lelli 
> Cc: Peter Zijlstra 
> Cc: Ingo Molnar 
> Cc: Rafael J. Wysocki 
> Cc: Viresh Kumar 
> Cc: Luca Abeni 
> Cc: Claudio Scordino 
> ---
> Changes from RFCv0:
>
>  - use a high bit of sched_flags and don't expose the new flag to
>userspace (also add derisory comments) (Peter)
> ---
>  include/linux/sched.h|  1 +
>  kernel/sched/core.c  | 15 +--
>  kernel/sched/cpufreq_schedutil.c | 15 ---
>  kernel/sched/deadline.c  | 13 +
>  kernel/sched/sched.h | 20 +++-
>  5 files changed, 58 insertions(+), 6 deletions(-)
>
> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index 1f0f427e0292..0ba767fdedae 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -1359,6 +1359,7 @@ extern int idle_cpu(int cpu);
>  extern int sched_setscheduler(struct task_struct *, int, const struct 
> sched_param *);
>  extern int sched_setscheduler_nocheck(struct task_struct *, int, const 
> struct sched_param *);
>  extern int sched_setattr(struct task_struct *, const struct sched_attr *);
> +extern int sched_setattr_nocheck(struct task_struct *, const struct 
> sched_attr *);
>  extern struct task_struct *idle_task(int cpu);
>
>  /**
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index 5186797908dc..0e40c7ff2bf4 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -3998,7 +3998,9 @@ static int __sched_setscheduler(struct task_struct *p,
> }
>
> if (attr->sched_flags &
> -   ~(SCHED_FLAG_RESET_ON_FORK | SCHED_FLAG_RECLAIM))
> +   ~(SCHED_FLAG_RESET_ON_FORK |
> + SCHED_FLAG_RECLAIM |
> + SCHED_FLAG_SPECIAL))
> return -EINVAL;

I was thinking if its better to name SCHED_FLAG_SPECIAL something more
descriptive than "special", since it will not be clear looking at
other parts of the code that this is used for the frequency scaling
thread or that its a DL task. I was thinking since this flag is
specifically setup for the sugov thread frequency scaling purpose, a
better flag name than SPECIAL might be SCHED_FLAG_DL_FREQ_SCALE or
SCHED_FLAG_FREQ_SCALE. But I am Ok with whatever makes sense to do
here. Thanks,

Regards,

-Joel

Re: [RFC PATCH v1 3/8] sched/cpufreq_schedutil: make worker kthread be SCHED_DEADLINE

[Joel Fernandes]

Hi Juri,

On Wed, Jul 5, 2017 at 1:59 AM, Juri Lelli  wrote:
> Worker kthread needs to be able to change frequency for all other
> threads.
>
> Make it special, just under STOP class.
>
> Signed-off-by: Juri Lelli 
> Cc: Peter Zijlstra 
> Cc: Ingo Molnar 
> Cc: Rafael J. Wysocki 
> Cc: Viresh Kumar 
> Cc: Luca Abeni 
> Cc: Claudio Scordino 
> ---
> Changes from RFCv0:
>
>  - use a high bit of sched_flags and don't expose the new flag to
>userspace (also add derisory comments) (Peter)
> ---
>  include/linux/sched.h|  1 +
>  kernel/sched/core.c  | 15 +--
>  kernel/sched/cpufreq_schedutil.c | 15 ---
>  kernel/sched/deadline.c  | 13 +
>  kernel/sched/sched.h | 20 +++-
>  5 files changed, 58 insertions(+), 6 deletions(-)
>
> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index 1f0f427e0292..0ba767fdedae 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -1359,6 +1359,7 @@ extern int idle_cpu(int cpu);
>  extern int sched_setscheduler(struct task_struct *, int, const struct 
> sched_param *);
>  extern int sched_setscheduler_nocheck(struct task_struct *, int, const 
> struct sched_param *);
>  extern int sched_setattr(struct task_struct *, const struct sched_attr *);
> +extern int sched_setattr_nocheck(struct task_struct *, const struct 
> sched_attr *);
>  extern struct task_struct *idle_task(int cpu);
>
>  /**
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index 5186797908dc..0e40c7ff2bf4 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -3998,7 +3998,9 @@ static int __sched_setscheduler(struct task_struct *p,
> }
>
> if (attr->sched_flags &
> -   ~(SCHED_FLAG_RESET_ON_FORK | SCHED_FLAG_RECLAIM))
> +   ~(SCHED_FLAG_RESET_ON_FORK |
> + SCHED_FLAG_RECLAIM |
> + SCHED_FLAG_SPECIAL))
> return -EINVAL;

I was thinking if its better to name SCHED_FLAG_SPECIAL something more
descriptive than "special", since it will not be clear looking at
other parts of the code that this is used for the frequency scaling
thread or that its a DL task. I was thinking since this flag is
specifically setup for the sugov thread frequency scaling purpose, a
better flag name than SPECIAL might be SCHED_FLAG_DL_FREQ_SCALE or
SCHED_FLAG_FREQ_SCALE. But I am Ok with whatever makes sense to do
here. Thanks,

Regards,

-Joel

Re: [PATCH 1/2] genirq: Get the fwnode back for irqchips being probed via ACPI namespace

[Hanjun Guo]

On 2017/7/6 21:05, Marc Zyngier wrote:
> On 06/07/17 10:01, Hanjun Guo wrote:
>> Hi Marc,
>>
>> On 2017/7/6 15:43, Marc Zyngier wrote:
>>> On 06/07/17 05:35, Hanjun Guo wrote:
 From: Hanjun Guo 

 commit d59f6617eef0 (genirq: Allow fwnode to carry name information only)
 forgot to do "domain->fwnode = fwnode;" for irqchips being probed via
 ACPI namesapce (DSDT/SSDT), that will break platforms with irqchip such
 as mbigen or qcom irq combiner, set the fwnode back to fix the issue.

 Reported-by: John Garry 
 Signed-off-by: Hanjun Guo 
 ---
  kernel/irq/irqdomain.c | 3 +--
  1 file changed, 1 insertion(+), 2 deletions(-)

 diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
 index 14fe862..1bc38fa 100644
 --- a/kernel/irq/irqdomain.c
 +++ b/kernel/irq/irqdomain.c
 @@ -151,7 +151,6 @@ struct irq_domain *__irq_domain_add(struct 
 fwnode_handle *fwnode, int size,
domain->flags |= IRQ_DOMAIN_NAME_ALLOCATED;
break;
default:
 -  domain->fwnode = fwnode;
domain->name = fwid->name;
break;
}
 @@ -172,7 +171,6 @@ struct irq_domain *__irq_domain_add(struct 
 fwnode_handle *fwnode, int size,
strreplace(name, '/', ':');
  
domain->name = name;
 -  domain->fwnode = fwnode;
domain->flags |= IRQ_DOMAIN_NAME_ALLOCATED;
}
  
 @@ -196,6 +194,7 @@ struct irq_domain *__irq_domain_add(struct 
 fwnode_handle *fwnode, int size,
INIT_RADIX_TREE(>revmap_tree, GFP_KERNEL);
domain->ops = ops;
domain->host_data = host_data;
 +  domain->fwnode = fwnode;
domain->hwirq_max = hwirq_max;
domain->revmap_size = size;
domain->revmap_direct_max_irq = direct_max;

>>> This doesn't seem right.
>>>
>>> Why is is_fwnode_irqchip() returning false when presented with an
>>> irqchip probed via the ACPI namespace? That's what you should consider
>>> fixing instead of moving that code around.
>> irqchips probed via the ACPI namespace will have a fwnode handler
>> with the fwnode type FWNODE_ACPI, similar to irqchips probed
>> via DT having a fwnode handler with type FWNODE_OF, in this
>> function with DT case, fwnode is set for irqdomain's fwnode,
>> ACPI just reuse that code because they are similar.
>>
>> is_fwnode_irqchip() just checks if it's FWNODE_IRQCHIP, which is only
>> available for irqchips probing via ACPI static tables such as ITS, GIC
>> on ARM, or via DMAR on x86, those cases don't have a fwnode handler then
>> need to create one via __irq_domain_alloc_fwnode(), which is different
>> from DT/ACPI namesapce scan mechanism. So the patch above it's the best
>> solution I got so far which just resume the code as before, correct me
>> if you have something new :)
> This violates the irqdomain API that takes two kind of fwnodes: 
> FWNODE_OF or FWNODE_IRQCHIP, and no others. You got away with it so 
> far, but it is over.
>
> You have two choices here: either you allocate a FWNODE_IRQCHIP in your
> irqchip driver, and use this as a handle for your IRQ domain, or you 
> make the irqdomain code able to deal with any FWNODE_ACPI fwnode.

Later one is better to me as allocate a FWNODE_IRQCHIP in the irqchip
driver will override the FWNODE_ACPI handler.

>
> Does the following hack work for you?

Yes, it works. shall we go this way with a proper fix?

>
> diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
> index 14fe862aa2e3..37f4aa3985b3 100644
> --- a/kernel/irq/irqdomain.c
> +++ b/kernel/irq/irqdomain.c
> @@ -155,6 +155,8 @@ struct irq_domain *__irq_domain_add(struct fwnode_handle 
> *fwnode, int size,
>   domain->name = fwid->name;
>   break;
>   }
> + } else if (is_acpi_device_node(fwnode)) {
> + domain->fwnode = fwnode;
>   } else if (of_node) {
>   char *name;
>  
> If it does, we can then find weird and wonderful ways to give the
> domain a shiny name in debugfs...

How about the weird way below (using dev_name() which shows ACPI HID + number) ?

+#include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -172,6 +174,15 @@ struct irq_domain *__irq_domain_add(struct fwnode_handle 
*fwnode, int size,
 
domain->name = name;
domain->flags |= IRQ_DOMAIN_NAME_ALLOCATED;
+ } else if (is_acpi_device_node(fwnode)) {
+ struct acpi_device *adev = to_acpi_device_node(fwnode);
+
+ domain->name = kstrdup(dev_name(>dev), GFP_KERNEL);
+ if (!domain->name)
+ return NULL;
+
+ domain->fwnode = fwnode;
+ domain->flags |= IRQ_DOMAIN_NAME_ALLOCATED;
}

But my hack code retrieving the ACPI data in irq domain core is really

Re: [PATCH 1/2] genirq: Get the fwnode back for irqchips being probed via ACPI namespace

[Hanjun Guo]

On 2017/7/6 21:05, Marc Zyngier wrote:
> On 06/07/17 10:01, Hanjun Guo wrote:
>> Hi Marc,
>>
>> On 2017/7/6 15:43, Marc Zyngier wrote:
>>> On 06/07/17 05:35, Hanjun Guo wrote:
 From: Hanjun Guo 

 commit d59f6617eef0 (genirq: Allow fwnode to carry name information only)
 forgot to do "domain->fwnode = fwnode;" for irqchips being probed via
 ACPI namesapce (DSDT/SSDT), that will break platforms with irqchip such
 as mbigen or qcom irq combiner, set the fwnode back to fix the issue.

 Reported-by: John Garry 
 Signed-off-by: Hanjun Guo 
 ---
  kernel/irq/irqdomain.c | 3 +--
  1 file changed, 1 insertion(+), 2 deletions(-)

 diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
 index 14fe862..1bc38fa 100644
 --- a/kernel/irq/irqdomain.c
 +++ b/kernel/irq/irqdomain.c
 @@ -151,7 +151,6 @@ struct irq_domain *__irq_domain_add(struct 
 fwnode_handle *fwnode, int size,
domain->flags |= IRQ_DOMAIN_NAME_ALLOCATED;
break;
default:
 -  domain->fwnode = fwnode;
domain->name = fwid->name;
break;
}
 @@ -172,7 +171,6 @@ struct irq_domain *__irq_domain_add(struct 
 fwnode_handle *fwnode, int size,
strreplace(name, '/', ':');
  
domain->name = name;
 -  domain->fwnode = fwnode;
domain->flags |= IRQ_DOMAIN_NAME_ALLOCATED;
}
  
 @@ -196,6 +194,7 @@ struct irq_domain *__irq_domain_add(struct 
 fwnode_handle *fwnode, int size,
INIT_RADIX_TREE(>revmap_tree, GFP_KERNEL);
domain->ops = ops;
domain->host_data = host_data;
 +  domain->fwnode = fwnode;
domain->hwirq_max = hwirq_max;
domain->revmap_size = size;
domain->revmap_direct_max_irq = direct_max;

>>> This doesn't seem right.
>>>
>>> Why is is_fwnode_irqchip() returning false when presented with an
>>> irqchip probed via the ACPI namespace? That's what you should consider
>>> fixing instead of moving that code around.
>> irqchips probed via the ACPI namespace will have a fwnode handler
>> with the fwnode type FWNODE_ACPI, similar to irqchips probed
>> via DT having a fwnode handler with type FWNODE_OF, in this
>> function with DT case, fwnode is set for irqdomain's fwnode,
>> ACPI just reuse that code because they are similar.
>>
>> is_fwnode_irqchip() just checks if it's FWNODE_IRQCHIP, which is only
>> available for irqchips probing via ACPI static tables such as ITS, GIC
>> on ARM, or via DMAR on x86, those cases don't have a fwnode handler then
>> need to create one via __irq_domain_alloc_fwnode(), which is different
>> from DT/ACPI namesapce scan mechanism. So the patch above it's the best
>> solution I got so far which just resume the code as before, correct me
>> if you have something new :)
> This violates the irqdomain API that takes two kind of fwnodes: 
> FWNODE_OF or FWNODE_IRQCHIP, and no others. You got away with it so 
> far, but it is over.
>
> You have two choices here: either you allocate a FWNODE_IRQCHIP in your
> irqchip driver, and use this as a handle for your IRQ domain, or you 
> make the irqdomain code able to deal with any FWNODE_ACPI fwnode.

Later one is better to me as allocate a FWNODE_IRQCHIP in the irqchip
driver will override the FWNODE_ACPI handler.

>
> Does the following hack work for you?

Yes, it works. shall we go this way with a proper fix?

>
> diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
> index 14fe862aa2e3..37f4aa3985b3 100644
> --- a/kernel/irq/irqdomain.c
> +++ b/kernel/irq/irqdomain.c
> @@ -155,6 +155,8 @@ struct irq_domain *__irq_domain_add(struct fwnode_handle 
> *fwnode, int size,
>   domain->name = fwid->name;
>   break;
>   }
> + } else if (is_acpi_device_node(fwnode)) {
> + domain->fwnode = fwnode;
>   } else if (of_node) {
>   char *name;
>  
> If it does, we can then find weird and wonderful ways to give the
> domain a shiny name in debugfs...

How about the weird way below (using dev_name() which shows ACPI HID + number) ?

+#include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -172,6 +174,15 @@ struct irq_domain *__irq_domain_add(struct fwnode_handle 
*fwnode, int size,
 
domain->name = name;
domain->flags |= IRQ_DOMAIN_NAME_ALLOCATED;
+ } else if (is_acpi_device_node(fwnode)) {
+ struct acpi_device *adev = to_acpi_device_node(fwnode);
+
+ domain->name = kstrdup(dev_name(>dev), GFP_KERNEL);
+ if (!domain->name)
+ return NULL;
+
+ domain->fwnode = fwnode;
+ domain->flags |= IRQ_DOMAIN_NAME_ALLOCATED;
}

But my hack code retrieving the ACPI data in irq domain core is really
weird :)

Thanks
Hanjun

Re: Make HWSPINLOCK a menuconfig to ease disabling

[Dave Jones]

On Thu, Jul 06, 2017 at 11:23:50PM +, Linux Kernel wrote:

 > Make HWSPINLOCK a menuconfig to ease disabling
 > 
 > So that there's no need to get into the submenu to disable all related 
 > config
 > entries.

Here's how that looks on x86...

*
* Hardware Spinlock drivers
*
Hardware Spinlock drivers (HWSPINLOCK) [N/m/y] (NEW) ?

There is no help available for this option.
Symbol: HWSPINLOCK [=n]
Type  : tristate
Prompt: Hardware Spinlock drivers
  Location:
-> Device Drivers
  Defined at drivers/hwspinlock/Kconfig:5



No help ? Ok whatever, let's see what happens..

Hardware Spinlock drivers (HWSPINLOCK) [N/m/y] (NEW) m

And then nothing, because none of the drivers that depend on this are available 
for x86
because of stuff like this..


 >  config HWSPINLOCK_OMAP
 >  tristate "OMAP Hardware Spinlock device"
 >  depends on ARCH_OMAP4 || SOC_OMAP5 || SOC_DRA7XX || SOC_AM33XX || 
 > SOC_AM43XX

 >  config HWSPINLOCK_QCOM
 >  tristate "Qualcomm Hardware Spinlock device"
 >  depends on ARCH_QCOM

 >  config HWSPINLOCK_SIRF
 >  tristate "SIRF Hardware Spinlock device"
 >  depends on ARCH_SIRF

 >  config HSEM_U8500
 >  tristate "STE Hardware Semaphore functionality"
 >  depends on ARCH_U8500

HWSPINLOCK needs all those depends too.

Dave

Re: Make HWSPINLOCK a menuconfig to ease disabling

[Dave Jones]

On Thu, Jul 06, 2017 at 11:23:50PM +, Linux Kernel wrote:

 > Make HWSPINLOCK a menuconfig to ease disabling
 > 
 > So that there's no need to get into the submenu to disable all related 
 > config
 > entries.

Here's how that looks on x86...

*
* Hardware Spinlock drivers
*
Hardware Spinlock drivers (HWSPINLOCK) [N/m/y] (NEW) ?

There is no help available for this option.
Symbol: HWSPINLOCK [=n]
Type  : tristate
Prompt: Hardware Spinlock drivers
  Location:
-> Device Drivers
  Defined at drivers/hwspinlock/Kconfig:5



No help ? Ok whatever, let's see what happens..

Hardware Spinlock drivers (HWSPINLOCK) [N/m/y] (NEW) m

And then nothing, because none of the drivers that depend on this are available 
for x86
because of stuff like this..


 >  config HWSPINLOCK_OMAP
 >  tristate "OMAP Hardware Spinlock device"
 >  depends on ARCH_OMAP4 || SOC_OMAP5 || SOC_DRA7XX || SOC_AM33XX || 
 > SOC_AM43XX

 >  config HWSPINLOCK_QCOM
 >  tristate "Qualcomm Hardware Spinlock device"
 >  depends on ARCH_QCOM

 >  config HWSPINLOCK_SIRF
 >  tristate "SIRF Hardware Spinlock device"
 >  depends on ARCH_SIRF

 >  config HSEM_U8500
 >  tristate "STE Hardware Semaphore functionality"
 >  depends on ARCH_U8500

HWSPINLOCK needs all those depends too.

Dave

Re: [RFC PATCH v1 4/8] sched/cpufreq_schedutil: split utilization signals

[Joel Fernandes]

Hi Juri,

On Wed, Jul 5, 2017 at 1:59 AM, Juri Lelli  wrote:
> To be able to treat utilization signals of different scheduling classes
> in different ways (e.g., CFS signal might be stale while DEADLINE signal
> is never stale by design) we need to split sugov_cpu::util signal in two:
> util_cfs and util_dl.
>
> This patch does that by also changing sugov_get_util() parameter list.
> After this change, aggregation of the different signals has to be performed
> by sugov_get_util() users (so that they can decide what to do with the
> different signals).
>
> Suggested-by: Rafael J. Wysocki 
> Signed-off-by: Juri Lelli 
> Cc: Peter Zijlstra 
> Cc: Ingo Molnar 
> Cc: Rafael J. Wysocki 
> Cc: Viresh Kumar 
> Cc: Luca Abeni 
> Cc: Claudio Scordino 
> ---
> Changes from RFCv0:
>
>  - refactor aggregation of utilization in sugov_aggregate_util()
> ---
>  kernel/sched/cpufreq_schedutil.c | 28 
>  1 file changed, 16 insertions(+), 12 deletions(-)
>
> diff --git a/kernel/sched/cpufreq_schedutil.c 
> b/kernel/sched/cpufreq_schedutil.c
> index ba6227625f24..e835fa886225 100644
> --- a/kernel/sched/cpufreq_schedutil.c
> +++ b/kernel/sched/cpufreq_schedutil.c
> @@ -58,7 +58,8 @@ struct sugov_cpu {
> u64 last_update;
>
> /* The fields below are only needed when sharing a policy. */
> -   unsigned long util;
> +   unsigned long util_cfs;
> +   unsigned long util_dl;
> unsigned long max;
> unsigned int flags;
>
> @@ -154,20 +155,24 @@ static unsigned int get_next_freq(struct sugov_policy 
> *sg_policy,
> return cpufreq_driver_resolve_freq(policy, freq);
>  }
>
> -static void sugov_get_util(unsigned long *util, unsigned long *max)
> +static void sugov_get_util(struct sugov_cpu *sg_cpu)
>  {
> struct rq *rq = this_rq();
> -   unsigned long dl_util = (rq->dl.running_bw * SCHED_CAPACITY_SCALE)
> -   >> BW_SHIFT;
>
> -   *max = arch_scale_cpu_capacity(NULL, smp_processor_id());
> +   sg_cpu->max = arch_scale_cpu_capacity(NULL, smp_processor_id());
> +   sg_cpu->util_cfs = rq->cfs.avg.util_avg;
> +   sg_cpu->util_dl = (rq->dl.running_bw * SCHED_CAPACITY_SCALE)
> + >> BW_SHIFT;
> +}
>
> +static unsigned long sugov_aggregate_util(struct sugov_cpu *sg_cpu)
> +{
> /*
>  * Ideally we would like to set util_dl as min/guaranteed freq and
>  * util_cfs + util_dl as requested freq. However, cpufreq is not yet
>  * ready for such an interface. So, we only do the latter for now.
>  */
> -   *util = min(rq->cfs.avg.util_avg + dl_util, *max);
> +   return min(sg_cpu->util_cfs + sg_cpu->util_dl, sg_cpu->max);
>  }

I am wondering why the need for a separate aggregation API. To me, it
looks like using sugov_get_util to set the sg_cpu util elements and
then do the aggregation at the same time would have the same effect
(without changing the existing parameter list). Is this to handle a
future usecase where aggregation may need to be done differently? For
all the user's of sugov_get_util, aggregation is done in the same way.
Anyway if I missed something, sorry for the noise.

thanks,

-Joel

Re: [RFC PATCH v1 4/8] sched/cpufreq_schedutil: split utilization signals

[Joel Fernandes]

Hi Juri,

On Wed, Jul 5, 2017 at 1:59 AM, Juri Lelli  wrote:
> To be able to treat utilization signals of different scheduling classes
> in different ways (e.g., CFS signal might be stale while DEADLINE signal
> is never stale by design) we need to split sugov_cpu::util signal in two:
> util_cfs and util_dl.
>
> This patch does that by also changing sugov_get_util() parameter list.
> After this change, aggregation of the different signals has to be performed
> by sugov_get_util() users (so that they can decide what to do with the
> different signals).
>
> Suggested-by: Rafael J. Wysocki 
> Signed-off-by: Juri Lelli 
> Cc: Peter Zijlstra 
> Cc: Ingo Molnar 
> Cc: Rafael J. Wysocki 
> Cc: Viresh Kumar 
> Cc: Luca Abeni 
> Cc: Claudio Scordino 
> ---
> Changes from RFCv0:
>
>  - refactor aggregation of utilization in sugov_aggregate_util()
> ---
>  kernel/sched/cpufreq_schedutil.c | 28 
>  1 file changed, 16 insertions(+), 12 deletions(-)
>
> diff --git a/kernel/sched/cpufreq_schedutil.c 
> b/kernel/sched/cpufreq_schedutil.c
> index ba6227625f24..e835fa886225 100644
> --- a/kernel/sched/cpufreq_schedutil.c
> +++ b/kernel/sched/cpufreq_schedutil.c
> @@ -58,7 +58,8 @@ struct sugov_cpu {
> u64 last_update;
>
> /* The fields below are only needed when sharing a policy. */
> -   unsigned long util;
> +   unsigned long util_cfs;
> +   unsigned long util_dl;
> unsigned long max;
> unsigned int flags;
>
> @@ -154,20 +155,24 @@ static unsigned int get_next_freq(struct sugov_policy 
> *sg_policy,
> return cpufreq_driver_resolve_freq(policy, freq);
>  }
>
> -static void sugov_get_util(unsigned long *util, unsigned long *max)
> +static void sugov_get_util(struct sugov_cpu *sg_cpu)
>  {
> struct rq *rq = this_rq();
> -   unsigned long dl_util = (rq->dl.running_bw * SCHED_CAPACITY_SCALE)
> -   >> BW_SHIFT;
>
> -   *max = arch_scale_cpu_capacity(NULL, smp_processor_id());
> +   sg_cpu->max = arch_scale_cpu_capacity(NULL, smp_processor_id());
> +   sg_cpu->util_cfs = rq->cfs.avg.util_avg;
> +   sg_cpu->util_dl = (rq->dl.running_bw * SCHED_CAPACITY_SCALE)
> + >> BW_SHIFT;
> +}
>
> +static unsigned long sugov_aggregate_util(struct sugov_cpu *sg_cpu)
> +{
> /*
>  * Ideally we would like to set util_dl as min/guaranteed freq and
>  * util_cfs + util_dl as requested freq. However, cpufreq is not yet
>  * ready for such an interface. So, we only do the latter for now.
>  */
> -   *util = min(rq->cfs.avg.util_avg + dl_util, *max);
> +   return min(sg_cpu->util_cfs + sg_cpu->util_dl, sg_cpu->max);
>  }

I am wondering why the need for a separate aggregation API. To me, it
looks like using sugov_get_util to set the sg_cpu util elements and
then do the aggregation at the same time would have the same effect
(without changing the existing parameter list). Is this to handle a
future usecase where aggregation may need to be done differently? For
all the user's of sugov_get_util, aggregation is done in the same way.
Anyway if I missed something, sorry for the noise.

thanks,

-Joel