[PATCH] leds: do not overflow sysfs buffer in led_trigger_show

[Zach Brown]

From: Nathan Sullivan 

Per the documentation, use scnprintf instead of sprintf to ensure there
is never more than PAGE_SIZE bytes of trigger names put into the
buffer.

Signed-off-by: Nathan Sullivan 
Signed-off-by: Zach Brown 
---
 drivers/leds/led-triggers.c | 12 +++-
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/leds/led-triggers.c b/drivers/leds/led-triggers.c
index c92702a..8668538 100644
--- a/drivers/leds/led-triggers.c
+++ b/drivers/leds/led-triggers.c
@@ -81,21 +81,23 @@ ssize_t led_trigger_show(struct device *dev, struct 
device_attribute *attr,
down_read(_cdev->trigger_lock);
 
if (!led_cdev->trigger)
-   len += sprintf(buf+len, "[none] ");
+   len += scnprintf(buf+len, PAGE_SIZE - len, "[none] ");
else
-   len += sprintf(buf+len, "none ");
+   len += scnprintf(buf+len, PAGE_SIZE - len, "none ");
 
list_for_each_entry(trig, _list, next_trig) {
if (led_cdev->trigger && !strcmp(led_cdev->trigger->name,
trig->name))
-   len += sprintf(buf+len, "[%s] ", trig->name);
+   len += scnprintf(buf+len, PAGE_SIZE - len, "[%s] ",
+trig->name);
else
-   len += sprintf(buf+len, "%s ", trig->name);
+   len += scnprintf(buf+len, PAGE_SIZE - len, "%s ",
+trig->name);
}
up_read(_cdev->trigger_lock);
up_read(_list_lock);
 
-   len += sprintf(len+buf, "\n");
+   len += scnprintf(len+buf, PAGE_SIZE - len, "\n");
return len;
 }
 EXPORT_SYMBOL_GPL(led_trigger_show);
-- 
2.7.4

[PATCH] leds: do not overflow sysfs buffer in led_trigger_show

[Zach Brown]

From: Nathan Sullivan 

Per the documentation, use scnprintf instead of sprintf to ensure there
is never more than PAGE_SIZE bytes of trigger names put into the
buffer.

Signed-off-by: Nathan Sullivan 
Signed-off-by: Zach Brown 
---
 drivers/leds/led-triggers.c | 12 +++-
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/leds/led-triggers.c b/drivers/leds/led-triggers.c
index c92702a..8668538 100644
--- a/drivers/leds/led-triggers.c
+++ b/drivers/leds/led-triggers.c
@@ -81,21 +81,23 @@ ssize_t led_trigger_show(struct device *dev, struct 
device_attribute *attr,
down_read(_cdev->trigger_lock);
 
if (!led_cdev->trigger)
-   len += sprintf(buf+len, "[none] ");
+   len += scnprintf(buf+len, PAGE_SIZE - len, "[none] ");
else
-   len += sprintf(buf+len, "none ");
+   len += scnprintf(buf+len, PAGE_SIZE - len, "none ");
 
list_for_each_entry(trig, _list, next_trig) {
if (led_cdev->trigger && !strcmp(led_cdev->trigger->name,
trig->name))
-   len += sprintf(buf+len, "[%s] ", trig->name);
+   len += scnprintf(buf+len, PAGE_SIZE - len, "[%s] ",
+trig->name);
else
-   len += sprintf(buf+len, "%s ", trig->name);
+   len += scnprintf(buf+len, PAGE_SIZE - len, "%s ",
+trig->name);
}
up_read(_cdev->trigger_lock);
up_read(_list_lock);
 
-   len += sprintf(len+buf, "\n");
+   len += scnprintf(len+buf, PAGE_SIZE - len, "\n");
return len;
 }
 EXPORT_SYMBOL_GPL(led_trigger_show);
-- 
2.7.4

Re: [PATCHv2 3/4] pci: Determine actual VPD size on first access

[Benjamin Herrenschmidt]

On Tue, 2016-08-16 at 08:23 +1000, Benjamin Herrenschmidt wrote:
> > I don't think desktop users appreciate hangs any more than anyone else, and 
> >  
> > that is one of the symptoms that can arise here without the vfio  
> > coordination.
> 
> And can happen with it as well 

Oh and your response was completely besides the point I was trying
to make, just some passive aggressive noise, thank you.

The point was that if you want the sort of safety that we are trying
to aim for, without additional HW support, you basically have to do
the isolation on a granularity no smaller than a bridge/bus (with the
notable exception of SR-IOV of course).

Otherwise guest *will* be able to harm each other (or the host) in
all sorts of ways if anything because devices will have MMIO backdoors
into their own BAR space, or can be made to DMA to a neighbour, etc...

This is the only safe thing to do (and we are enforcing that on POWER
with our IOMMU groups).

Now that being said, if you want to keep the ability to assign 2 functions
of a device to different guests for your average non-critical desktop user,
that's where you may want to consider two models.

Generally speaking, filtering things for "safety" won't work.

Filtering things to work around bugs in existing guests to avoid crashes
is a different kettle of fish and could be justified but keep in mind that
in most cases a malicious guest will be able to exploit those HW flaws.

Assuming that a device coming back from a guest is functional and not
completely broken and can be re-used without a full PERST or power cycle
is a wrong assumption. It may or may not work, no amount of "filtering"
will fix the fundamental issue. If your HW won't give you access to PERST
well ... blame Intel for not specifying a standard way to generate it in
the first place :-)

Cheers,
Ben.

Re: [PATCHv2 3/4] pci: Determine actual VPD size on first access

[Benjamin Herrenschmidt]

On Tue, 2016-08-16 at 08:23 +1000, Benjamin Herrenschmidt wrote:
> > I don't think desktop users appreciate hangs any more than anyone else, and 
> >  
> > that is one of the symptoms that can arise here without the vfio  
> > coordination.
> 
> And can happen with it as well 

Oh and your response was completely besides the point I was trying
to make, just some passive aggressive noise, thank you.

The point was that if you want the sort of safety that we are trying
to aim for, without additional HW support, you basically have to do
the isolation on a granularity no smaller than a bridge/bus (with the
notable exception of SR-IOV of course).

Otherwise guest *will* be able to harm each other (or the host) in
all sorts of ways if anything because devices will have MMIO backdoors
into their own BAR space, or can be made to DMA to a neighbour, etc...

This is the only safe thing to do (and we are enforcing that on POWER
with our IOMMU groups).

Now that being said, if you want to keep the ability to assign 2 functions
of a device to different guests for your average non-critical desktop user,
that's where you may want to consider two models.

Generally speaking, filtering things for "safety" won't work.

Filtering things to work around bugs in existing guests to avoid crashes
is a different kettle of fish and could be justified but keep in mind that
in most cases a malicious guest will be able to exploit those HW flaws.

Assuming that a device coming back from a guest is functional and not
completely broken and can be re-used without a full PERST or power cycle
is a wrong assumption. It may or may not work, no amount of "filtering"
will fix the fundamental issue. If your HW won't give you access to PERST
well ... blame Intel for not specifying a standard way to generate it in
the first place :-)

Cheers,
Ben.

Re: [PATCH 2/2] power: reset: syscon-reboot-mode: Use managed resource API

[Sebastian Reichel]

Hi,

On Wed, Aug 03, 2016 at 10:04:06PM -0700, Bjorn Andersson wrote:
> Use the managed resource version of reboot_mode_register().
> 
> [...]
>
>  drivers/power/reset/syscon-reboot-mode.c | 12 +---
>  1 file changed, 1 insertion(+), 11 deletions(-)

Thanks, queued.

-- Sebastian


signature.asc
Description: PGP signature

Re: [PATCH 2/2] power: reset: syscon-reboot-mode: Use managed resource API

[Sebastian Reichel]

Hi,

On Wed, Aug 03, 2016 at 10:04:06PM -0700, Bjorn Andersson wrote:
> Use the managed resource version of reboot_mode_register().
> 
> [...]
>
>  drivers/power/reset/syscon-reboot-mode.c | 12 +---
>  1 file changed, 1 insertion(+), 11 deletions(-)

Thanks, queued.

-- Sebastian


signature.asc
Description: PGP signature

Re: [PATCH 1/2] power: reset: reboot-mode: Add managed resource API

[Sebastian Reichel]

Hi,

On Wed, Aug 03, 2016 at 10:04:05PM -0700, Bjorn Andersson wrote:
> Provide managed resource version of reboot_mode_register() and
> reboot_mode_unregister() to simplify implementations.

Thanks, queued.

-- Sebastian


signature.asc
Description: PGP signature

Re: [PATCH 1/2] power: reset: reboot-mode: Add managed resource API

[Sebastian Reichel]

Hi,

On Wed, Aug 03, 2016 at 10:04:05PM -0700, Bjorn Andersson wrote:
> Provide managed resource version of reboot_mode_register() and
> reboot_mode_unregister() to simplify implementations.

Thanks, queued.

-- Sebastian


signature.asc
Description: PGP signature

RE: [PATCH v2 RESEND 2/4] Drivers: hv: balloon: account for gaps in hot add regions

[Alex Ng (LIS)]

> @@ -676,35 +686,63 @@ static void hv_mem_hot_add(unsigned long start,
> unsigned long size,
> 
>  static void hv_online_page(struct page *pg)  {
> - struct list_head *cur;
>   struct hv_hotadd_state *has;
> + struct hv_hotadd_gap *gap;
>   unsigned long cur_start_pgp;
>   unsigned long cur_end_pgp;
> + bool is_gap = false;
> 
>   list_for_each(cur, _device.ha_region_list) {
>   has = list_entry(cur, struct hv_hotadd_state, list);
>   cur_start_pgp = (unsigned long)
> + pfn_to_page(has->start_pfn);
> + cur_end_pgp = (unsigned long)pfn_to_page(has->end_pfn);
> +
> + /* The page belongs to a different HAS. */
> + if (((unsigned long)pg < cur_start_pgp) ||
> + ((unsigned long)pg >= cur_end_pgp))
> + continue;
> +
> + cur_start_pgp = (unsigned long)
>   pfn_to_page(has->covered_start_pfn);
>   cur_end_pgp = (unsigned long)pfn_to_page(has-
> >covered_end_pfn);
> 
> - if (((unsigned long)pg >= cur_start_pgp) &&
> - ((unsigned long)pg < cur_end_pgp)) {
> - /*
> -  * This frame is currently backed; online the
> -  * page.
> -  */
> - __online_page_set_limits(pg);
> - __online_page_increment_counters(pg);
> - __online_page_free(pg);
> + /* The page is not backed. */
> + if (((unsigned long)pg < cur_start_pgp) ||
> + ((unsigned long)pg >= cur_end_pgp))
> + continue;
> +
> + /* Check for gaps. */
> + list_for_each_entry(gap, >gap_list, list) {
> + cur_start_pgp = (unsigned long)
> + pfn_to_page(gap->start_pfn);
> + cur_end_pgp = (unsigned long)
> + pfn_to_page(gap->end_pfn);
> + if (((unsigned long)pg >= cur_start_pgp) &&
> + ((unsigned long)pg < cur_end_pgp)) {
> + is_gap = true;
> + break;
> + }
>   }
> +
> + if (is_gap)
> + break;
> +
> + /* This frame is currently backed; online the page. */
> + __online_page_set_limits(pg);
> + __online_page_increment_counters(pg);
> + __online_page_free(pg);
> + break;
>   }
>  }
> 

We may need to add similar logic to check for gaps in hv_bring_pgs_online().

[...]
>  static unsigned long handle_pg_range(unsigned long pg_start, @@ -834,13
> +881,19 @@ static unsigned long process_hot_add(unsigned long pg_start,
>   unsigned long rg_size)
>  {
>   struct hv_hotadd_state *ha_region = NULL;
> + int covered;
> 
>   if (pfn_cnt == 0)
>   return 0;
> 
> - if (!dm_device.host_specified_ha_region)
> - if (pfn_covered(pg_start, pfn_cnt))
> + if (!dm_device.host_specified_ha_region) {
> + covered = pfn_covered(pg_start, pfn_cnt);
> + if (covered < 0)
> + return 0;

If the hot-add pages aren't covered by any region, then shouldn't it fall 
through instead of returning?
That way the new ha_region can be added to the list and we hot-add the pages 
accordingly.

RE: [PATCH v2 RESEND 2/4] Drivers: hv: balloon: account for gaps in hot add regions

[Alex Ng (LIS)]

> @@ -676,35 +686,63 @@ static void hv_mem_hot_add(unsigned long start,
> unsigned long size,
> 
>  static void hv_online_page(struct page *pg)  {
> - struct list_head *cur;
>   struct hv_hotadd_state *has;
> + struct hv_hotadd_gap *gap;
>   unsigned long cur_start_pgp;
>   unsigned long cur_end_pgp;
> + bool is_gap = false;
> 
>   list_for_each(cur, _device.ha_region_list) {
>   has = list_entry(cur, struct hv_hotadd_state, list);
>   cur_start_pgp = (unsigned long)
> + pfn_to_page(has->start_pfn);
> + cur_end_pgp = (unsigned long)pfn_to_page(has->end_pfn);
> +
> + /* The page belongs to a different HAS. */
> + if (((unsigned long)pg < cur_start_pgp) ||
> + ((unsigned long)pg >= cur_end_pgp))
> + continue;
> +
> + cur_start_pgp = (unsigned long)
>   pfn_to_page(has->covered_start_pfn);
>   cur_end_pgp = (unsigned long)pfn_to_page(has-
> >covered_end_pfn);
> 
> - if (((unsigned long)pg >= cur_start_pgp) &&
> - ((unsigned long)pg < cur_end_pgp)) {
> - /*
> -  * This frame is currently backed; online the
> -  * page.
> -  */
> - __online_page_set_limits(pg);
> - __online_page_increment_counters(pg);
> - __online_page_free(pg);
> + /* The page is not backed. */
> + if (((unsigned long)pg < cur_start_pgp) ||
> + ((unsigned long)pg >= cur_end_pgp))
> + continue;
> +
> + /* Check for gaps. */
> + list_for_each_entry(gap, >gap_list, list) {
> + cur_start_pgp = (unsigned long)
> + pfn_to_page(gap->start_pfn);
> + cur_end_pgp = (unsigned long)
> + pfn_to_page(gap->end_pfn);
> + if (((unsigned long)pg >= cur_start_pgp) &&
> + ((unsigned long)pg < cur_end_pgp)) {
> + is_gap = true;
> + break;
> + }
>   }
> +
> + if (is_gap)
> + break;
> +
> + /* This frame is currently backed; online the page. */
> + __online_page_set_limits(pg);
> + __online_page_increment_counters(pg);
> + __online_page_free(pg);
> + break;
>   }
>  }
> 

We may need to add similar logic to check for gaps in hv_bring_pgs_online().

[...]
>  static unsigned long handle_pg_range(unsigned long pg_start, @@ -834,13
> +881,19 @@ static unsigned long process_hot_add(unsigned long pg_start,
>   unsigned long rg_size)
>  {
>   struct hv_hotadd_state *ha_region = NULL;
> + int covered;
> 
>   if (pfn_cnt == 0)
>   return 0;
> 
> - if (!dm_device.host_specified_ha_region)
> - if (pfn_covered(pg_start, pfn_cnt))
> + if (!dm_device.host_specified_ha_region) {
> + covered = pfn_covered(pg_start, pfn_cnt);
> + if (covered < 0)
> + return 0;

If the hot-add pages aren't covered by any region, then shouldn't it fall 
through instead of returning?
That way the new ha_region can be added to the list and we hot-add the pages 
accordingly.

Re: [PATCH v2 2/5] clk: qcom: ipq4019: Added the apss cpu pll divider clock node

[Stephen Boyd]

On 06/21, Abhishek Sahu wrote:
> diff --git a/drivers/clk/qcom/gcc-ipq4019.c b/drivers/clk/qcom/gcc-ipq4019.c
> index 17ca6d3..4c8a97f 100644
> --- a/drivers/clk/qcom/gcc-ipq4019.c
> +++ b/drivers/clk/qcom/gcc-ipq4019.c
> @@ -154,7 +154,7 @@ static const char * const gcc_xo_ddr_500_200[] = {
>   "xo",
>   "gcc_fepll200_clk",
>   "gcc_fepll500_clk",
> - "ddrpllapss",
> + "gcc_apps_cpu_plldiv_clk",
>  };
>  
>  #define F(f, s, h, m, n) { (f), (s), (2 * (h) - 1), (m), (n) }
> @@ -571,6 +571,7 @@ static struct clk_rcg2 apps_clk_src = {
>   .parent_names = gcc_xo_ddr_500_200,
>   .num_parents = 4,
>   .ops = _rcg2_ops,
> + .flags = CLK_SET_RATE_PARENT,
>   },
>  };
>  
> @@ -1273,6 +1274,114 @@ static struct clk_pll_vco gcc_fepll_vco = {
>   },
>  };
>  
> +/*
> + * Round rate function for APPS CPU PLL Clock divider.
> + * It Returns the input rate without changing it. The hardware supported rate
> + * will be calculated in set function by getting the same from frequency 
> table.
> + */
> +static long clk_cpu_div_round_rate(struct clk_hw *hw, unsigned long rate,
> + unsigned long *p_rate)
> +{
> + return rate;

This needs to calculate what the rate will be if clk_set_rate()
is called with the rate parameter. Blindly returning rate back is not
acceptable.

> +};
> +
> +/*
> + * Clock set rate function for APPS CPU PLL Clock divider.
> + * It looks up the frequency table and updates the PLL divider to 
> corresponding
> + * divider value.
> + */
> +static int clk_cpu_div_set_rate(struct clk_hw *hw, unsigned long rate,
> +   unsigned long parent_rate)
> +{
> + struct clk_pll_div *rcg = to_clk_pll_div(hw);
> + const struct freq_tbl *f;
> + u32 mask;
> + int ret;
> +
> + f = qcom_find_freq(rcg->freq_tbl, rate);
> + if (!f)
> + return -EINVAL;
> +
> + mask = (BIT(rcg->cdiv.width) - 1) << rcg->cdiv.shift;
> + ret = regmap_update_bits(rcg->cdiv.clkr.regmap,
> + rcg->cdiv.reg, mask,
> + (f->pre_div << rcg->cdiv.shift) & mask);

Why are we masking a value that will be masked by
regmap_update_bits()?

> + udelay(1);

Why do we wait? Please add some comment.

> +
> + return 0;
> +};
> +
> +/*
> + * Clock frequency calculation function for APPS CPU PLL Clock divider.
> + * This clock divider is nonlinear so this function calculates the actual
> + * divider and returns the output frequency by dividing VCO Frequency
> + * with this actual divider value.
> + */
> +static unsigned long clk_cpu_div_recalc_rate(struct clk_hw *hw,
> +unsigned long parent_rate)
> +{
> + struct clk_pll_div *rcg = to_clk_pll_div(hw);
> + u32 cdiv, pre_div;
> +
> + regmap_read(rcg->cdiv.clkr.regmap, rcg->cdiv.reg, );
> + cdiv = (cdiv >> rcg->cdiv.shift) & (BIT(rcg->cdiv.width) - 1);
> +
> + /*
> +  * Some dividers have value in 0.5 fraction so multiply both VCO
> +  * frequency(parent_rate) and pre_div with 2 to make integer
> +  * calculation.
> +  */
> + if (cdiv > 10)
> + pre_div = (cdiv + 1) * 2;
> + else
> + pre_div = cdiv + 12;
> +
> + return clk_calc_divider_rate(parent_rate * 2, pre_div);
> +};
> +
> +const struct clk_ops clk_regmap_cpu_div_ops = {

static?

> + .round_rate = clk_cpu_div_round_rate,
> + .set_rate = clk_cpu_div_set_rate,
> + .recalc_rate = clk_cpu_div_recalc_rate,
> +};
> +
> +static const struct freq_tbl ftbl_apps_ddr_pll[] = {
> + {38000, P_XO, 0xD, 0},
> + {40900, P_XO, 0xC, 0, 0},
> + {44400, P_XO, 0xB, 0, 0},
> + {48400, P_XO, 0xA, 0, 0},
> + {50700, P_XO, 0x9, 0, 0},
> + {53200, P_XO, 0x8, 0, 0},
> + {56000, P_XO, 0x7, 0, 0},
> + {59200, P_XO, 0x6, 0, 0},
> + {62600, P_XO, 0x5, 0, 0},
> + {66600, P_XO, 0x4, 0, 0},
> + {71000, P_XO, 0x3, 0, 0},
> + {76100, P_XO, 0x2, 0, 0},
> + {81900, P_XO, 0x1, 0, 0},
> + {88800, P_XO, 0x0, 0, 0},

Style nits from before apply here (lowercase, spaces)

> + {}
> +};
> +
> +static struct clk_pll_div gcc_apps_cpu_plldiv_clk = {
> + .cdiv.reg = 0x2E020,
> + .cdiv.shift = 4,
> + .cdiv.width = 4,
> + .cdiv.clkr = {
> + .enable_reg = 0x2E000,
> + .enable_mask = BIT(0),
> + .hw.init = &(struct clk_init_data){
> + .name = "gcc_apps_cpu_plldiv_clk",
> + .parent_names = (const char *[]){
> + "gcc_apps_ddrpll_vco",
> + },
> + .num_parents = 1,
> + .ops = _regmap_cpu_div_ops,
> + },
> + },
> + .freq_tbl = ftbl_apps_ddr_pll,
> +};

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

Re: [PATCH v2 2/5] clk: qcom: ipq4019: Added the apss cpu pll divider clock node

[Stephen Boyd]

On 06/21, Abhishek Sahu wrote:
> diff --git a/drivers/clk/qcom/gcc-ipq4019.c b/drivers/clk/qcom/gcc-ipq4019.c
> index 17ca6d3..4c8a97f 100644
> --- a/drivers/clk/qcom/gcc-ipq4019.c
> +++ b/drivers/clk/qcom/gcc-ipq4019.c
> @@ -154,7 +154,7 @@ static const char * const gcc_xo_ddr_500_200[] = {
>   "xo",
>   "gcc_fepll200_clk",
>   "gcc_fepll500_clk",
> - "ddrpllapss",
> + "gcc_apps_cpu_plldiv_clk",
>  };
>  
>  #define F(f, s, h, m, n) { (f), (s), (2 * (h) - 1), (m), (n) }
> @@ -571,6 +571,7 @@ static struct clk_rcg2 apps_clk_src = {
>   .parent_names = gcc_xo_ddr_500_200,
>   .num_parents = 4,
>   .ops = _rcg2_ops,
> + .flags = CLK_SET_RATE_PARENT,
>   },
>  };
>  
> @@ -1273,6 +1274,114 @@ static struct clk_pll_vco gcc_fepll_vco = {
>   },
>  };
>  
> +/*
> + * Round rate function for APPS CPU PLL Clock divider.
> + * It Returns the input rate without changing it. The hardware supported rate
> + * will be calculated in set function by getting the same from frequency 
> table.
> + */
> +static long clk_cpu_div_round_rate(struct clk_hw *hw, unsigned long rate,
> + unsigned long *p_rate)
> +{
> + return rate;

This needs to calculate what the rate will be if clk_set_rate()
is called with the rate parameter. Blindly returning rate back is not
acceptable.

> +};
> +
> +/*
> + * Clock set rate function for APPS CPU PLL Clock divider.
> + * It looks up the frequency table and updates the PLL divider to 
> corresponding
> + * divider value.
> + */
> +static int clk_cpu_div_set_rate(struct clk_hw *hw, unsigned long rate,
> +   unsigned long parent_rate)
> +{
> + struct clk_pll_div *rcg = to_clk_pll_div(hw);
> + const struct freq_tbl *f;
> + u32 mask;
> + int ret;
> +
> + f = qcom_find_freq(rcg->freq_tbl, rate);
> + if (!f)
> + return -EINVAL;
> +
> + mask = (BIT(rcg->cdiv.width) - 1) << rcg->cdiv.shift;
> + ret = regmap_update_bits(rcg->cdiv.clkr.regmap,
> + rcg->cdiv.reg, mask,
> + (f->pre_div << rcg->cdiv.shift) & mask);

Why are we masking a value that will be masked by
regmap_update_bits()?

> + udelay(1);

Why do we wait? Please add some comment.

> +
> + return 0;
> +};
> +
> +/*
> + * Clock frequency calculation function for APPS CPU PLL Clock divider.
> + * This clock divider is nonlinear so this function calculates the actual
> + * divider and returns the output frequency by dividing VCO Frequency
> + * with this actual divider value.
> + */
> +static unsigned long clk_cpu_div_recalc_rate(struct clk_hw *hw,
> +unsigned long parent_rate)
> +{
> + struct clk_pll_div *rcg = to_clk_pll_div(hw);
> + u32 cdiv, pre_div;
> +
> + regmap_read(rcg->cdiv.clkr.regmap, rcg->cdiv.reg, );
> + cdiv = (cdiv >> rcg->cdiv.shift) & (BIT(rcg->cdiv.width) - 1);
> +
> + /*
> +  * Some dividers have value in 0.5 fraction so multiply both VCO
> +  * frequency(parent_rate) and pre_div with 2 to make integer
> +  * calculation.
> +  */
> + if (cdiv > 10)
> + pre_div = (cdiv + 1) * 2;
> + else
> + pre_div = cdiv + 12;
> +
> + return clk_calc_divider_rate(parent_rate * 2, pre_div);
> +};
> +
> +const struct clk_ops clk_regmap_cpu_div_ops = {

static?

> + .round_rate = clk_cpu_div_round_rate,
> + .set_rate = clk_cpu_div_set_rate,
> + .recalc_rate = clk_cpu_div_recalc_rate,
> +};
> +
> +static const struct freq_tbl ftbl_apps_ddr_pll[] = {
> + {38000, P_XO, 0xD, 0},
> + {40900, P_XO, 0xC, 0, 0},
> + {44400, P_XO, 0xB, 0, 0},
> + {48400, P_XO, 0xA, 0, 0},
> + {50700, P_XO, 0x9, 0, 0},
> + {53200, P_XO, 0x8, 0, 0},
> + {56000, P_XO, 0x7, 0, 0},
> + {59200, P_XO, 0x6, 0, 0},
> + {62600, P_XO, 0x5, 0, 0},
> + {66600, P_XO, 0x4, 0, 0},
> + {71000, P_XO, 0x3, 0, 0},
> + {76100, P_XO, 0x2, 0, 0},
> + {81900, P_XO, 0x1, 0, 0},
> + {88800, P_XO, 0x0, 0, 0},

Style nits from before apply here (lowercase, spaces)

> + {}
> +};
> +
> +static struct clk_pll_div gcc_apps_cpu_plldiv_clk = {
> + .cdiv.reg = 0x2E020,
> + .cdiv.shift = 4,
> + .cdiv.width = 4,
> + .cdiv.clkr = {
> + .enable_reg = 0x2E000,
> + .enable_mask = BIT(0),
> + .hw.init = &(struct clk_init_data){
> + .name = "gcc_apps_cpu_plldiv_clk",
> + .parent_names = (const char *[]){
> + "gcc_apps_ddrpll_vco",
> + },
> + .num_parents = 1,
> + .ops = _regmap_cpu_div_ops,
> + },
> + },
> + .freq_tbl = ftbl_apps_ddr_pll,
> +};

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

[PATCH v2] Added perf functionality to mmdc driver

[Zhengyu Shen]

MMDC is a multi-mode DDR controller that supports DDR3/DDR3L x16/x32/x64 and
LPDDR2 two channel x16/x32 memory types. MMDC is configurable, high performance,
and optimized. MMDC is present on i.MX6 Quad and i.MX6 QuadPlus devices, but
this driver only supports i.MX6 Quad at the moment. MMDC provides registers
for performance counters which read via this driver to help debug memory
throughput and similar issues.

$ perf stat -a -e 
mmdc/busy-cycles/,mmdc/read-accesses/,mmdc/read-bytes/,mmdc/total-cycles/,mmdc/write-accesses/,mmdc/write-bytes/
 dd if=/dev/zero of=/dev/null bs=1M count=5000
Performance counter stats for 'dd if=/dev/zero of=/dev/null bs=1M count=5000':

 898021787  mmdc/busy-cycles/
  14819600  mmdc/read-accesses/
471.30 MB   mmdc/read-bytes/
2815419216  mmdc/total-cycles/
  13367354  mmdc/write-accesses/
427.76 MB   mmdc/write-bytes/

   5.334757334 seconds time elapsed

Signed-off-by: Zhengyu Shen 
---
change from v1 to v2:
Added cpumask and migration handling support to driver
Validated event during event_init
Added code to properly stop counters
Used perf_invalid_context instead of perf_sw_context
Added hrtimer to poll for overflow 
Added better description
Added support for multiple mmdcs

 arch/arm/mach-imx/mmdc.c | 362 ++-
 1 file changed, 361 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-imx/mmdc.c b/arch/arm/mach-imx/mmdc.c
index db9621c..372b59c 100644
--- a/arch/arm/mach-imx/mmdc.c
+++ b/arch/arm/mach-imx/mmdc.c
@@ -1,5 +1,5 @@
 /*
- * Copyright 2011 Freescale Semiconductor, Inc.
+ * Copyright 2011,2016 Freescale Semiconductor, Inc.
  * Copyright 2011 Linaro Ltd.
  *
  * The code contained herein is licensed under the GNU General Public
@@ -16,6 +16,10 @@
 #include 
 #include 
 #include 
+#include 
+#include 
+#include 
+#include 
 
 #include "common.h"
 
@@ -27,14 +31,341 @@
 #define BM_MMDC_MDMISC_DDR_TYPE0x18
 #define BP_MMDC_MDMISC_DDR_TYPE0x3
 
+#define TOTAL_CYCLES   0x1
+#define BUSY_CYCLES0x2
+#define READ_ACCESSES  0x3
+#define WRITE_ACCESSES 0x4
+#define READ_BYTES 0x5
+#define WRITE_BYTES0x6
+
+/* Enables, resets, freezes, overflow profiling*/
+#define DBG_DIS0x0
+#define DBG_EN 0x1 
+#define DBG_RST0x2
+#define PRF_FRZ0x4
+#define CYC_OVF0x8
+
+#define MMDC_MADPCR0   0x410
+#define MMDC_MADPSR0   0x418
+#define MMDC_MADPSR1   0x41C
+#define MMDC_MADPSR2   0x420
+#define MMDC_MADPSR3   0x424
+#define MMDC_MADPSR4   0x428
+#define MMDC_MADPSR5   0x42C
+
+#define MMDC_NUM_COUNTERS  6
+
+#define to_mmdc_pmu(p) (container_of(p, struct mmdc_pmu, pmu))
+
+static DEFINE_IDA(mmdc_ida);
+
 static int ddr_type;
 
+PMU_EVENT_ATTR_STRING(total-cycles, mmdc_total_cycles, "event=0x01")
+PMU_EVENT_ATTR_STRING(busy-cycles, mmdc_busy_cycles, "event=0x02")
+PMU_EVENT_ATTR_STRING(read-accesses, mmdc_read_accesses, "event=0x03")
+PMU_EVENT_ATTR_STRING(write-accesses, mmdc_write_accesses, "config=0x04")
+PMU_EVENT_ATTR_STRING(read-bytes, mmdc_read_bytes, "event=0x05")
+PMU_EVENT_ATTR_STRING(read-bytes.unit, mmdc_read_bytes_unit, "MB");
+PMU_EVENT_ATTR_STRING(read-bytes.scale, mmdc_read_bytes_scale, "0.01");
+PMU_EVENT_ATTR_STRING(write-bytes, mmdc_write_bytes, "event=0x06")
+PMU_EVENT_ATTR_STRING(write-bytes.unit, mmdc_write_bytes_unit, "MB");
+PMU_EVENT_ATTR_STRING(write-bytes.scale, mmdc_write_bytes_scale, "0.01");
+
+struct mmdc_pmu
+{
+   struct pmu pmu;
+   void __iomem *mmdc_base;
+   cpumask_t cpu;
+   struct notifier_block cpu_nb;
+   struct hrtimer hrtimer;
+   unsigned int irq;
+   struct device *dev;
+   struct perf_event *mmdc_events[MMDC_NUM_COUNTERS];
+};
+
+static unsigned int mmdc_poll_period_us = 100;
+module_param_named(pmu_poll_period_us, mmdc_poll_period_us, uint,
+   S_IRUGO | S_IWUSR);
+
+static ktime_t mmdc_timer_period(void)
+{
+   return ns_to_ktime((u64)mmdc_poll_period_us * 1000);
+}
+
+static ssize_t mmdc_cpumask_show(struct device *dev,
+   struct device_attribute *attr, char *buf)
+{
+   struct mmdc_pmu *pmu_mmdc = dev_get_drvdata(dev);
+   return cpumap_print_to_pagebuf(true, buf, _mmdc->cpu);
+}
+
+static struct device_attribute mmdc_cpumask_attr =
+__ATTR(cpumask, S_IRUGO, mmdc_cpumask_show, NULL);
+
+static struct attribute *mmdc_cpumask_attrs[] = {
+   _cpumask_attr.attr,
+   NULL,
+};
+
+static struct attribute_group mmdc_cpumask_attr_group = {
+   .attrs = mmdc_cpumask_attrs,
+};
+
+static struct attribute *mmdc_events_attrs[] = {
+   _total_cycles.attr.attr,
+   _busy_cycles.attr.attr,
+   _read_accesses.attr.attr,
+   _write_accesses.attr.attr,
+   _read_bytes.attr.attr,
+  

[PATCH v2] Added perf functionality to mmdc driver

[Zhengyu Shen]

MMDC is a multi-mode DDR controller that supports DDR3/DDR3L x16/x32/x64 and
LPDDR2 two channel x16/x32 memory types. MMDC is configurable, high performance,
and optimized. MMDC is present on i.MX6 Quad and i.MX6 QuadPlus devices, but
this driver only supports i.MX6 Quad at the moment. MMDC provides registers
for performance counters which read via this driver to help debug memory
throughput and similar issues.

$ perf stat -a -e 
mmdc/busy-cycles/,mmdc/read-accesses/,mmdc/read-bytes/,mmdc/total-cycles/,mmdc/write-accesses/,mmdc/write-bytes/
 dd if=/dev/zero of=/dev/null bs=1M count=5000
Performance counter stats for 'dd if=/dev/zero of=/dev/null bs=1M count=5000':

 898021787  mmdc/busy-cycles/
  14819600  mmdc/read-accesses/
471.30 MB   mmdc/read-bytes/
2815419216  mmdc/total-cycles/
  13367354  mmdc/write-accesses/
427.76 MB   mmdc/write-bytes/

   5.334757334 seconds time elapsed

Signed-off-by: Zhengyu Shen 
---
change from v1 to v2:
Added cpumask and migration handling support to driver
Validated event during event_init
Added code to properly stop counters
Used perf_invalid_context instead of perf_sw_context
Added hrtimer to poll for overflow 
Added better description
Added support for multiple mmdcs

 arch/arm/mach-imx/mmdc.c | 362 ++-
 1 file changed, 361 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-imx/mmdc.c b/arch/arm/mach-imx/mmdc.c
index db9621c..372b59c 100644
--- a/arch/arm/mach-imx/mmdc.c
+++ b/arch/arm/mach-imx/mmdc.c
@@ -1,5 +1,5 @@
 /*
- * Copyright 2011 Freescale Semiconductor, Inc.
+ * Copyright 2011,2016 Freescale Semiconductor, Inc.
  * Copyright 2011 Linaro Ltd.
  *
  * The code contained herein is licensed under the GNU General Public
@@ -16,6 +16,10 @@
 #include 
 #include 
 #include 
+#include 
+#include 
+#include 
+#include 
 
 #include "common.h"
 
@@ -27,14 +31,341 @@
 #define BM_MMDC_MDMISC_DDR_TYPE0x18
 #define BP_MMDC_MDMISC_DDR_TYPE0x3
 
+#define TOTAL_CYCLES   0x1
+#define BUSY_CYCLES0x2
+#define READ_ACCESSES  0x3
+#define WRITE_ACCESSES 0x4
+#define READ_BYTES 0x5
+#define WRITE_BYTES0x6
+
+/* Enables, resets, freezes, overflow profiling*/
+#define DBG_DIS0x0
+#define DBG_EN 0x1 
+#define DBG_RST0x2
+#define PRF_FRZ0x4
+#define CYC_OVF0x8
+
+#define MMDC_MADPCR0   0x410
+#define MMDC_MADPSR0   0x418
+#define MMDC_MADPSR1   0x41C
+#define MMDC_MADPSR2   0x420
+#define MMDC_MADPSR3   0x424
+#define MMDC_MADPSR4   0x428
+#define MMDC_MADPSR5   0x42C
+
+#define MMDC_NUM_COUNTERS  6
+
+#define to_mmdc_pmu(p) (container_of(p, struct mmdc_pmu, pmu))
+
+static DEFINE_IDA(mmdc_ida);
+
 static int ddr_type;
 
+PMU_EVENT_ATTR_STRING(total-cycles, mmdc_total_cycles, "event=0x01")
+PMU_EVENT_ATTR_STRING(busy-cycles, mmdc_busy_cycles, "event=0x02")
+PMU_EVENT_ATTR_STRING(read-accesses, mmdc_read_accesses, "event=0x03")
+PMU_EVENT_ATTR_STRING(write-accesses, mmdc_write_accesses, "config=0x04")
+PMU_EVENT_ATTR_STRING(read-bytes, mmdc_read_bytes, "event=0x05")
+PMU_EVENT_ATTR_STRING(read-bytes.unit, mmdc_read_bytes_unit, "MB");
+PMU_EVENT_ATTR_STRING(read-bytes.scale, mmdc_read_bytes_scale, "0.01");
+PMU_EVENT_ATTR_STRING(write-bytes, mmdc_write_bytes, "event=0x06")
+PMU_EVENT_ATTR_STRING(write-bytes.unit, mmdc_write_bytes_unit, "MB");
+PMU_EVENT_ATTR_STRING(write-bytes.scale, mmdc_write_bytes_scale, "0.01");
+
+struct mmdc_pmu
+{
+   struct pmu pmu;
+   void __iomem *mmdc_base;
+   cpumask_t cpu;
+   struct notifier_block cpu_nb;
+   struct hrtimer hrtimer;
+   unsigned int irq;
+   struct device *dev;
+   struct perf_event *mmdc_events[MMDC_NUM_COUNTERS];
+};
+
+static unsigned int mmdc_poll_period_us = 100;
+module_param_named(pmu_poll_period_us, mmdc_poll_period_us, uint,
+   S_IRUGO | S_IWUSR);
+
+static ktime_t mmdc_timer_period(void)
+{
+   return ns_to_ktime((u64)mmdc_poll_period_us * 1000);
+}
+
+static ssize_t mmdc_cpumask_show(struct device *dev,
+   struct device_attribute *attr, char *buf)
+{
+   struct mmdc_pmu *pmu_mmdc = dev_get_drvdata(dev);
+   return cpumap_print_to_pagebuf(true, buf, _mmdc->cpu);
+}
+
+static struct device_attribute mmdc_cpumask_attr =
+__ATTR(cpumask, S_IRUGO, mmdc_cpumask_show, NULL);
+
+static struct attribute *mmdc_cpumask_attrs[] = {
+   _cpumask_attr.attr,
+   NULL,
+};
+
+static struct attribute_group mmdc_cpumask_attr_group = {
+   .attrs = mmdc_cpumask_attrs,
+};
+
+static struct attribute *mmdc_events_attrs[] = {
+   _total_cycles.attr.attr,
+   _busy_cycles.attr.attr,
+   _read_accesses.attr.attr,
+   _write_accesses.attr.attr,
+   _read_bytes.attr.attr,
+   

Re: [PATCH v2 4/6] kexec_file: Add mechanism to update kexec segments.

[Andrew Morton]

On Sat, 13 Aug 2016 00:18:23 -0300 Thiago Jung Bauermann 
 wrote:

> kexec_update_segment allows a given segment in kexec_image to have
> its contents updated. This is useful if the current kernel wants to
> send information to the next kernel that is up-to-date at the time of
> reboot.
> 
> ...
>
> @@ -721,6 +721,105 @@ static struct page *kimage_alloc_page(struct kimage 
> *image,
>   return page;
>  }
>  
> +/**
> + * kexec_update_segment - update the contents of a kimage segment
> + * @buffer:  New contents of the segment.
> + * @bufsz:   @buffer size.
> + * @load_addr:   Segment's physical address in the next kernel.
> + * @memsz:   Segment size.
> + *
> + * This function assumes kexec_mutex is held.
> + *
> + * Return: 0 on success, negative errno on error.
> + */
> +int kexec_update_segment(const char *buffer, unsigned long bufsz,
> +  unsigned long load_addr, unsigned long memsz)
> +{
> + int i;
> + unsigned long entry;
> + unsigned long *ptr = NULL;
> + void *dest = NULL;
> +
> + if (kexec_image == NULL) {
> + pr_err("Can't update segment: no kexec image loaded.\n");
> + return -EINVAL;
> + }
> +
> + /*
> +  * kexec_add_buffer rounds up segment sizes to PAGE_SIZE, so
> +  * we have to do it here as well.
> +  */
> + memsz = ALIGN(memsz, PAGE_SIZE);
> +
> + for (i = 0; i < kexec_image->nr_segments; i++)
> + /* We only support updating whole segments. */
> + if (load_addr == kexec_image->segment[i].mem &&
> + memsz == kexec_image->segment[i].memsz) {
> + if (kexec_image->segment[i].do_checksum) {
> + pr_err("Trying to update non-modifiable 
> segment.\n");
> + return -EINVAL;
> + }
> +
> + break;
> + }
> + if (i == kexec_image->nr_segments) {
> + pr_err("Couldn't find segment to update: 0x%lx, size 0x%lx\n",
> +load_addr, memsz);
> + return -EINVAL;
> + }
> +
> + for (entry = kexec_image->head; !(entry & IND_DONE) && memsz;
> +  entry = *ptr++) {
> + void *addr = (void *) (entry & PAGE_MASK);
> +
> + switch (entry & IND_FLAGS) {
> + case IND_DESTINATION:
> + dest = addr;
> + break;
> + case IND_INDIRECTION:
> + ptr = __va(addr);
> + break;
> + case IND_SOURCE:
> + /* Shouldn't happen, but verify just to be safe. */
> + if (dest == NULL) {
> + pr_err("Invalid kexec entries list.");
> + return -EINVAL;
> + }
> +
> + if (dest == (void *) load_addr) {
> + struct page *page;
> + char *ptr;
> + size_t uchunk, mchunk;
> +
> + page = kmap_to_page(addr);
> +
> + ptr = kmap(page);

kmap_atomic() could be used here, and it is appreciably faster.


> + ptr += load_addr & ~PAGE_MASK;
> + mchunk = min_t(size_t, memsz,
> +PAGE_SIZE - (load_addr & 
> ~PAGE_MASK));
> + uchunk = min(bufsz, mchunk);
> + memcpy(ptr, buffer, uchunk);
> +
> + kunmap(page);
> +
> + bufsz -= uchunk;
> + load_addr += mchunk;
> + buffer += mchunk;
> + memsz -= mchunk;
> + }
> + dest += PAGE_SIZE;
> + }
> +
> + /* Shouldn't happen, but verify just to be safe. */
> + if (ptr == NULL) {
> + pr_err("Invalid kexec entries list.");
> + return -EINVAL;
> + }
> + }
> +
> + return 0;
> +}
> +

Re: [PATCH v2 4/6] kexec_file: Add mechanism to update kexec segments.

[Andrew Morton]

On Sat, 13 Aug 2016 00:18:23 -0300 Thiago Jung Bauermann 
 wrote:

> kexec_update_segment allows a given segment in kexec_image to have
> its contents updated. This is useful if the current kernel wants to
> send information to the next kernel that is up-to-date at the time of
> reboot.
> 
> ...
>
> @@ -721,6 +721,105 @@ static struct page *kimage_alloc_page(struct kimage 
> *image,
>   return page;
>  }
>  
> +/**
> + * kexec_update_segment - update the contents of a kimage segment
> + * @buffer:  New contents of the segment.
> + * @bufsz:   @buffer size.
> + * @load_addr:   Segment's physical address in the next kernel.
> + * @memsz:   Segment size.
> + *
> + * This function assumes kexec_mutex is held.
> + *
> + * Return: 0 on success, negative errno on error.
> + */
> +int kexec_update_segment(const char *buffer, unsigned long bufsz,
> +  unsigned long load_addr, unsigned long memsz)
> +{
> + int i;
> + unsigned long entry;
> + unsigned long *ptr = NULL;
> + void *dest = NULL;
> +
> + if (kexec_image == NULL) {
> + pr_err("Can't update segment: no kexec image loaded.\n");
> + return -EINVAL;
> + }
> +
> + /*
> +  * kexec_add_buffer rounds up segment sizes to PAGE_SIZE, so
> +  * we have to do it here as well.
> +  */
> + memsz = ALIGN(memsz, PAGE_SIZE);
> +
> + for (i = 0; i < kexec_image->nr_segments; i++)
> + /* We only support updating whole segments. */
> + if (load_addr == kexec_image->segment[i].mem &&
> + memsz == kexec_image->segment[i].memsz) {
> + if (kexec_image->segment[i].do_checksum) {
> + pr_err("Trying to update non-modifiable 
> segment.\n");
> + return -EINVAL;
> + }
> +
> + break;
> + }
> + if (i == kexec_image->nr_segments) {
> + pr_err("Couldn't find segment to update: 0x%lx, size 0x%lx\n",
> +load_addr, memsz);
> + return -EINVAL;
> + }
> +
> + for (entry = kexec_image->head; !(entry & IND_DONE) && memsz;
> +  entry = *ptr++) {
> + void *addr = (void *) (entry & PAGE_MASK);
> +
> + switch (entry & IND_FLAGS) {
> + case IND_DESTINATION:
> + dest = addr;
> + break;
> + case IND_INDIRECTION:
> + ptr = __va(addr);
> + break;
> + case IND_SOURCE:
> + /* Shouldn't happen, but verify just to be safe. */
> + if (dest == NULL) {
> + pr_err("Invalid kexec entries list.");
> + return -EINVAL;
> + }
> +
> + if (dest == (void *) load_addr) {
> + struct page *page;
> + char *ptr;
> + size_t uchunk, mchunk;
> +
> + page = kmap_to_page(addr);
> +
> + ptr = kmap(page);

kmap_atomic() could be used here, and it is appreciably faster.


> + ptr += load_addr & ~PAGE_MASK;
> + mchunk = min_t(size_t, memsz,
> +PAGE_SIZE - (load_addr & 
> ~PAGE_MASK));
> + uchunk = min(bufsz, mchunk);
> + memcpy(ptr, buffer, uchunk);
> +
> + kunmap(page);
> +
> + bufsz -= uchunk;
> + load_addr += mchunk;
> + buffer += mchunk;
> + memsz -= mchunk;
> + }
> + dest += PAGE_SIZE;
> + }
> +
> + /* Shouldn't happen, but verify just to be safe. */
> + if (ptr == NULL) {
> + pr_err("Invalid kexec entries list.");
> + return -EINVAL;
> + }
> + }
> +
> + return 0;
> +}
> +

Re: [PATCH v2 3/5] clk: qcom: ipq4019: Added the nodes for pcnoc

[Stephen Boyd]

On 06/21, Abhishek Sahu wrote:
> The current ipq4019 clock driver does not have the node for
> PCNOC so this patch adds and registers the PCNOC clock nodes.
> 
> Signed-off-by: Abhishek Sahu 
> ---

This one looks good.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v2 3/5] clk: qcom: ipq4019: Added the nodes for pcnoc

[Stephen Boyd]

On 06/21, Abhishek Sahu wrote:
> The current ipq4019 clock driver does not have the node for
> PCNOC so this patch adds and registers the PCNOC clock nodes.
> 
> Signed-off-by: Abhishek Sahu 
> ---

This one looks good.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [RFC PATCH 0/3] Documentation: switch to pdflatex and fix pdf build

[Mauro Carvalho Chehab]

Hi Jon,

Em Mon, 15 Aug 2016 09:17:52 -0300
Mauro Carvalho Chehab  escreveu:

> Em Mon, 15 Aug 2016 12:40:21 +0300
> Jani Nikula  escreveu:
> 
> > On Sat, 13 Aug 2016, Jonathan Corbet  wrote:  
> > > On Wed, 10 Aug 2016 18:54:06 +0300
> > > Jani Nikula  wrote:
> > >
> > >> With these you should be able to get started with pdf generation. It's a
> > >> quick transition to pdflatex, the patches are not very pretty, but the
> > >> pdf output is. Patch 3/3 works as an example where to add your stuff
> > >> (latex_documents in conf.py) and how.
> > >
> > > OK, now I have a bone to pick with you.
> > >
> > > I applied this, then decided to install the needed toolchain on the
> > > Tumbleweed system I've been playing with; it wanted to install 1,727
> > > packages to get pdflatex.  Pandoc just doesn't seem so bad anymore.
> > 
> > Jon, I sent these to unblock Luis, and as a starting point for a
> > discussion about rst2pdf vs. pdflatex. I didn't mean I'd want these
> > merged as-is! I'm sorry if I didn't make myself clear.
> > 
> > I don't mind at all if you want to drop them.

I played for a while with rst2pdf and LaTeX-based tools, plus
using Sphinx math extension to improve the media documentation.

Here's my findings:

1) I'm pretty sure there's no way for us to get rid of LaTex.

The problem is that the Sphinx math extension depends on LaTeX amsmath
extension. Ok, someone could write some extension in the future to add math
support without requiring LaTeX and fix rst2pdf (or rewrite it), but this
would take time and efforts.

So, for now, I think we should just assume that anyone wanting to generate
pdf docs will need LaTeX (or, more likely, TeTeX).

2) rst2pdf seems to be incompatible with Sphinx math extension,
at least on Sphinx 1.4.5;

3) pdflatex doesn't handle UTF-8 chars. This is a problem for media,
as we use two UTF-8 symbols that are incompatible with the fonts
used by pdflatex:

- ≥ U+2265 (GREATER-THAN OR EQUAL TO) utf-8 character
- ≤ u+2264 (LESS-THAN OR EQUAL TO) utf-8 character

4) pdflatex output is not nice: all black and white; ugly (IMHO) fonts;

5) At media docs, some tables will only print ok in landscape.

After making the media books build, I think that the best way is to
use xelatex instead of pdfdocs. Visually, xelatex output is, IMHO,
nice - and it has colors :-)

It seems that there's yet another option: lualatex. I didn't try to
build with it. So, not sure if its output is better or not, nor if
some extra config for it is needed at conf.py.

I sent a patch series addressing most of the issues above.

If you want to see the output:
https://mchehab.fedorapeople.org/TheLinuxKernel.pdf

This was built on Fedora 24.

Issues:
---

Even with xelatex, there are still some issues to be addressed:

- There are two hacks to build media: one removes the *.h.rst files
  and the other one comments out two C code blocks inside tables;

- There are lots of noise at PDF output. I didn't even try to look
  into them;

- Almost all tables at the media books are mangled. Rotating them to
  landscape can fix several of them. I added support for it, however,
  that requires to manually add LaTeX tags before and after the tables,
  like:

.. raw:: latex

\begin{landscape}



.. raw:: latex

\end{landscape}

As the above tags are LaTeX specific, they should not interfere on 
html (or e-pub) output.

One such example is this table:
"Table 2.18: Packed RGB Image Formats"
(at page 92 of the PDF file)

I'm seeking for a solution to scale it and rotate (as just rotating it
is not enough).

-- 
Thanks,
Mauro

Re: [RFC PATCH 0/3] Documentation: switch to pdflatex and fix pdf build

[Mauro Carvalho Chehab]

Hi Jon,

Em Mon, 15 Aug 2016 09:17:52 -0300
Mauro Carvalho Chehab  escreveu:

> Em Mon, 15 Aug 2016 12:40:21 +0300
> Jani Nikula  escreveu:
> 
> > On Sat, 13 Aug 2016, Jonathan Corbet  wrote:  
> > > On Wed, 10 Aug 2016 18:54:06 +0300
> > > Jani Nikula  wrote:
> > >
> > >> With these you should be able to get started with pdf generation. It's a
> > >> quick transition to pdflatex, the patches are not very pretty, but the
> > >> pdf output is. Patch 3/3 works as an example where to add your stuff
> > >> (latex_documents in conf.py) and how.
> > >
> > > OK, now I have a bone to pick with you.
> > >
> > > I applied this, then decided to install the needed toolchain on the
> > > Tumbleweed system I've been playing with; it wanted to install 1,727
> > > packages to get pdflatex.  Pandoc just doesn't seem so bad anymore.
> > 
> > Jon, I sent these to unblock Luis, and as a starting point for a
> > discussion about rst2pdf vs. pdflatex. I didn't mean I'd want these
> > merged as-is! I'm sorry if I didn't make myself clear.
> > 
> > I don't mind at all if you want to drop them.

I played for a while with rst2pdf and LaTeX-based tools, plus
using Sphinx math extension to improve the media documentation.

Here's my findings:

1) I'm pretty sure there's no way for us to get rid of LaTex.

The problem is that the Sphinx math extension depends on LaTeX amsmath
extension. Ok, someone could write some extension in the future to add math
support without requiring LaTeX and fix rst2pdf (or rewrite it), but this
would take time and efforts.

So, for now, I think we should just assume that anyone wanting to generate
pdf docs will need LaTeX (or, more likely, TeTeX).

2) rst2pdf seems to be incompatible with Sphinx math extension,
at least on Sphinx 1.4.5;

3) pdflatex doesn't handle UTF-8 chars. This is a problem for media,
as we use two UTF-8 symbols that are incompatible with the fonts
used by pdflatex:

- ≥ U+2265 (GREATER-THAN OR EQUAL TO) utf-8 character
- ≤ u+2264 (LESS-THAN OR EQUAL TO) utf-8 character

4) pdflatex output is not nice: all black and white; ugly (IMHO) fonts;

5) At media docs, some tables will only print ok in landscape.

After making the media books build, I think that the best way is to
use xelatex instead of pdfdocs. Visually, xelatex output is, IMHO,
nice - and it has colors :-)

It seems that there's yet another option: lualatex. I didn't try to
build with it. So, not sure if its output is better or not, nor if
some extra config for it is needed at conf.py.

I sent a patch series addressing most of the issues above.

If you want to see the output:
https://mchehab.fedorapeople.org/TheLinuxKernel.pdf

This was built on Fedora 24.

Issues:
---

Even with xelatex, there are still some issues to be addressed:

- There are two hacks to build media: one removes the *.h.rst files
  and the other one comments out two C code blocks inside tables;

- There are lots of noise at PDF output. I didn't even try to look
  into them;

- Almost all tables at the media books are mangled. Rotating them to
  landscape can fix several of them. I added support for it, however,
  that requires to manually add LaTeX tags before and after the tables,
  like:

.. raw:: latex

\begin{landscape}



.. raw:: latex

\end{landscape}

As the above tags are LaTeX specific, they should not interfere on 
html (or e-pub) output.

One such example is this table:
"Table 2.18: Packed RGB Image Formats"
(at page 92 of the PDF file)

I'm seeking for a solution to scale it and rotate (as just rotating it
is not enough).

-- 
Thanks,
Mauro

Re: [PATCH v2 5/5] clk: qcom: ipq4019: Added the cpu clock frequency change notifier

[Stephen Boyd]

On 06/21, Abhishek Sahu wrote:
> The current driver code gives the crash or gets hang while switching
> the CPU frequency some time. The APSS CPU Clock divider is not glitch
> free so it the APPS clock need to be switched for stable clock during
> the change.
> 
> This patch adds the frequency change notifier for APSS CPU clock. It
> changes the parent of this clock to stable PLL FEPLL500 when it gets
> for PRE_RATE_CHANGE event. This event will be generated before actual
> clock set operations. The clock set operation will again change its
> corresponding parent by getting the same from frequency table.
> 
> Signed-off-by: Abhishek Sahu 
> ---
>  drivers/clk/qcom/gcc-ipq4019.c | 27 ++-
>  1 file changed, 26 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/qcom/gcc-ipq4019.c b/drivers/clk/qcom/gcc-ipq4019.c
> index df159c2..3c014f7 100644
> --- a/drivers/clk/qcom/gcc-ipq4019.c
> +++ b/drivers/clk/qcom/gcc-ipq4019.c
> @@ -1740,9 +1740,34 @@ static const struct of_device_id 
> gcc_ipq4019_match_table[] = {
>  };
>  MODULE_DEVICE_TABLE(of, gcc_ipq4019_match_table);
>  
> +int cpu_clk_notifier_fn(struct notifier_block *nb,

static?

Please come up with a better name that is not so generic.

> + unsigned long action, void *data)
> +{
> + int err = 0;
> +
> + if (action == PRE_RATE_CHANGE) {
> + err = clk_rcg2_ops.set_parent(_clk_src.clkr.hw,
> + P_FEPLL500);
> + }
> +
> + return notifier_from_errno(err);
> +}
> +
> +struct notifier_block cpu_clk_notifier = {

static?

> + .notifier_call = cpu_clk_notifier_fn,
> +};
> +
>  static int gcc_ipq4019_probe(struct platform_device *pdev)
>  {
> - return qcom_cc_probe(pdev, _ipq4019_desc);
> + int err;
> +
> + err = qcom_cc_probe(pdev, _ipq4019_desc);
> +
> + if (!err)
> + clk_notifier_register(apps_clk_src.clkr.hw.clk,
> + _clk_notifier);

This has to be unregistered on driver remove.

> +
> + return err;
>  }

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v2 5/5] clk: qcom: ipq4019: Added the cpu clock frequency change notifier

[Stephen Boyd]

On 06/21, Abhishek Sahu wrote:
> The current driver code gives the crash or gets hang while switching
> the CPU frequency some time. The APSS CPU Clock divider is not glitch
> free so it the APPS clock need to be switched for stable clock during
> the change.
> 
> This patch adds the frequency change notifier for APSS CPU clock. It
> changes the parent of this clock to stable PLL FEPLL500 when it gets
> for PRE_RATE_CHANGE event. This event will be generated before actual
> clock set operations. The clock set operation will again change its
> corresponding parent by getting the same from frequency table.
> 
> Signed-off-by: Abhishek Sahu 
> ---
>  drivers/clk/qcom/gcc-ipq4019.c | 27 ++-
>  1 file changed, 26 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/qcom/gcc-ipq4019.c b/drivers/clk/qcom/gcc-ipq4019.c
> index df159c2..3c014f7 100644
> --- a/drivers/clk/qcom/gcc-ipq4019.c
> +++ b/drivers/clk/qcom/gcc-ipq4019.c
> @@ -1740,9 +1740,34 @@ static const struct of_device_id 
> gcc_ipq4019_match_table[] = {
>  };
>  MODULE_DEVICE_TABLE(of, gcc_ipq4019_match_table);
>  
> +int cpu_clk_notifier_fn(struct notifier_block *nb,

static?

Please come up with a better name that is not so generic.

> + unsigned long action, void *data)
> +{
> + int err = 0;
> +
> + if (action == PRE_RATE_CHANGE) {
> + err = clk_rcg2_ops.set_parent(_clk_src.clkr.hw,
> + P_FEPLL500);
> + }
> +
> + return notifier_from_errno(err);
> +}
> +
> +struct notifier_block cpu_clk_notifier = {

static?

> + .notifier_call = cpu_clk_notifier_fn,
> +};
> +
>  static int gcc_ipq4019_probe(struct platform_device *pdev)
>  {
> - return qcom_cc_probe(pdev, _ipq4019_desc);
> + int err;
> +
> + err = qcom_cc_probe(pdev, _ipq4019_desc);
> +
> + if (!err)
> + clk_notifier_register(apps_clk_src.clkr.hw.clk,
> + _clk_notifier);

This has to be unregistered on driver remove.

> +
> + return err;
>  }

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH] platform: olpc: make ec explicitly non-modular

[Paul Gortmaker]

The Kconfig entry controlling compilation of this code is:

arch/x86/Kconfig:config OLPC
arch/x86/Kconfig:   bool "One Laptop Per Child support"

...meaning that it currently is not being built as a module by anyone.

Lets remove the couple traces of modular infrastructure use, so that
when reading the driver there is no doubt it is builtin-only.

We delete the MODULE_LICENSE tag etc. since all that information
was (or is now) contained at the top of the file in the comments.

Cc: Darren Hart 
Cc: Andres Salomon 
Cc: platform-driver-...@vger.kernel.org
Signed-off-by: Paul Gortmaker 
---
 drivers/platform/olpc/olpc-ec.c | 8 +++-
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/platform/olpc/olpc-ec.c b/drivers/platform/olpc/olpc-ec.c
index f99b183d5296..374a8028fec7 100644
--- a/drivers/platform/olpc/olpc-ec.c
+++ b/drivers/platform/olpc/olpc-ec.c
@@ -1,6 +1,8 @@
 /*
  * Generic driver for the OLPC Embedded Controller.
  *
+ * Author: Andres Salomon 
+ *
  * Copyright (C) 2011-2012 One Laptop per Child Foundation.
  *
  * Licensed under the GPL v2 or later.
@@ -12,7 +14,7 @@
 #include 
 #include 
 #include 
-#include 
+#include 
 #include 
 #include 
 #include 
@@ -326,8 +328,4 @@ static int __init olpc_ec_init_module(void)
 {
return platform_driver_register(_ec_plat_driver);
 }
-
 arch_initcall(olpc_ec_init_module);
-
-MODULE_AUTHOR("Andres Salomon ");
-MODULE_LICENSE("GPL");
-- 
2.8.4

[PATCH] platform: intel_pmic: make gpio explicitly non-modular

[Paul Gortmaker]

The Kconfig entry controlling compilation of this code is:

drivers/platform/x86/Kconfig:config GPIO_INTEL_PMIC
drivers/platform/x86/Kconfig:   bool "Intel PMIC GPIO support"

...meaning that it currently is not being built as a module by anyone.

Lets remove the couple traces of modular infrastructure use, so that
when reading the driver there is no doubt it is builtin-only.

We delete the MODULE_LICENSE tag etc. since all that information
was (or is now) contained at the top of the file in the comments.

We don't replace module.h with init.h since the file already has that.

Cc: Darren Hart 
Cc: Alek Du 
Cc: platform-driver-...@vger.kernel.org
Signed-off-by: Paul Gortmaker 
---
 drivers/platform/x86/intel_pmic_gpio.c | 8 ++--
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/platform/x86/intel_pmic_gpio.c 
b/drivers/platform/x86/intel_pmic_gpio.c
index 63b371d6ee55..91ae58510d92 100644
--- a/drivers/platform/x86/intel_pmic_gpio.c
+++ b/drivers/platform/x86/intel_pmic_gpio.c
@@ -1,6 +1,8 @@
 /* Moorestown PMIC GPIO (access through IPC) driver
  * Copyright (c) 2008 - 2009, Intel Corporation.
  *
+ * Author: Alek Du 
+ *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 2 as
  * published by the Free Software Foundation.
@@ -21,7 +23,6 @@
 
 #define pr_fmt(fmt) "%s: " fmt, __func__
 
-#include 
 #include 
 #include 
 #include 
@@ -322,9 +323,4 @@ static int __init platform_pmic_gpio_init(void)
 {
return platform_driver_register(_pmic_gpio_driver);
 }
-
 subsys_initcall(platform_pmic_gpio_init);
-
-MODULE_AUTHOR("Alek Du ");
-MODULE_DESCRIPTION("Intel Moorestown PMIC GPIO driver");
-MODULE_LICENSE("GPL v2");
-- 
2.8.4

[PATCH] platform: olpc: make ec explicitly non-modular

[Paul Gortmaker]

The Kconfig entry controlling compilation of this code is:

arch/x86/Kconfig:config OLPC
arch/x86/Kconfig:   bool "One Laptop Per Child support"

...meaning that it currently is not being built as a module by anyone.

Lets remove the couple traces of modular infrastructure use, so that
when reading the driver there is no doubt it is builtin-only.

We delete the MODULE_LICENSE tag etc. since all that information
was (or is now) contained at the top of the file in the comments.

Cc: Darren Hart 
Cc: Andres Salomon 
Cc: platform-driver-...@vger.kernel.org
Signed-off-by: Paul Gortmaker 
---
 drivers/platform/olpc/olpc-ec.c | 8 +++-
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/platform/olpc/olpc-ec.c b/drivers/platform/olpc/olpc-ec.c
index f99b183d5296..374a8028fec7 100644
--- a/drivers/platform/olpc/olpc-ec.c
+++ b/drivers/platform/olpc/olpc-ec.c
@@ -1,6 +1,8 @@
 /*
  * Generic driver for the OLPC Embedded Controller.
  *
+ * Author: Andres Salomon 
+ *
  * Copyright (C) 2011-2012 One Laptop per Child Foundation.
  *
  * Licensed under the GPL v2 or later.
@@ -12,7 +14,7 @@
 #include 
 #include 
 #include 
-#include 
+#include 
 #include 
 #include 
 #include 
@@ -326,8 +328,4 @@ static int __init olpc_ec_init_module(void)
 {
return platform_driver_register(_ec_plat_driver);
 }
-
 arch_initcall(olpc_ec_init_module);
-
-MODULE_AUTHOR("Andres Salomon ");
-MODULE_LICENSE("GPL");
-- 
2.8.4

[PATCH] platform: intel_pmic: make gpio explicitly non-modular

[Paul Gortmaker]

The Kconfig entry controlling compilation of this code is:

drivers/platform/x86/Kconfig:config GPIO_INTEL_PMIC
drivers/platform/x86/Kconfig:   bool "Intel PMIC GPIO support"

...meaning that it currently is not being built as a module by anyone.

Lets remove the couple traces of modular infrastructure use, so that
when reading the driver there is no doubt it is builtin-only.

We delete the MODULE_LICENSE tag etc. since all that information
was (or is now) contained at the top of the file in the comments.

We don't replace module.h with init.h since the file already has that.

Cc: Darren Hart 
Cc: Alek Du 
Cc: platform-driver-...@vger.kernel.org
Signed-off-by: Paul Gortmaker 
---
 drivers/platform/x86/intel_pmic_gpio.c | 8 ++--
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/platform/x86/intel_pmic_gpio.c 
b/drivers/platform/x86/intel_pmic_gpio.c
index 63b371d6ee55..91ae58510d92 100644
--- a/drivers/platform/x86/intel_pmic_gpio.c
+++ b/drivers/platform/x86/intel_pmic_gpio.c
@@ -1,6 +1,8 @@
 /* Moorestown PMIC GPIO (access through IPC) driver
  * Copyright (c) 2008 - 2009, Intel Corporation.
  *
+ * Author: Alek Du 
+ *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 2 as
  * published by the Free Software Foundation.
@@ -21,7 +23,6 @@
 
 #define pr_fmt(fmt) "%s: " fmt, __func__
 
-#include 
 #include 
 #include 
 #include 
@@ -322,9 +323,4 @@ static int __init platform_pmic_gpio_init(void)
 {
return platform_driver_register(_pmic_gpio_driver);
 }
-
 subsys_initcall(platform_pmic_gpio_init);
-
-MODULE_AUTHOR("Alek Du ");
-MODULE_DESCRIPTION("Intel Moorestown PMIC GPIO driver");
-MODULE_LICENSE("GPL v2");
-- 
2.8.4

Re: [PATCH v2 4/5] clk: qcom: ipq4019: Added the all frequencies for apps cpu

[Stephen Boyd]

On 06/21, Abhishek Sahu wrote:
> @@ -554,11 +554,21 @@ static struct clk_rcg2  sdcc1_apps_clk_src = {
>  };
>  
>  static const struct freq_tbl ftbl_gcc_apps_clk[] = {
> - F(4800, P_XO,  1, 0, 0),
> - F(2, P_FEPLL200,   1, 0, 0),
> - F(5, P_FEPLL500,   1, 0, 0),
> - F(62600, P_DDRPLLAPSS, 1, 0, 0),
> - { }
> + {4800, P_XO,  1, 0, 0},
> + {2, P_FEPLL200,   1, 0, 0},
> + {38000, P_DDRPLLAPSS, 1, 0, 0},
> + {40900, P_DDRPLLAPSS, 1, 0, 0},
> + {44400, P_DDRPLLAPSS, 1, 0, 0},
> + {48400, P_DDRPLLAPSS, 1, 0, 0},
> + {5, P_FEPLL500,   1, 0, 0},
> + {50700, P_DDRPLLAPSS, 1, 0, 0},
> + {53200, P_DDRPLLAPSS, 1, 0, 0},
> + {56000, P_DDRPLLAPSS, 1, 0, 0},
> + {59200, P_DDRPLLAPSS, 1, 0, 0},
> + {62600, P_DDRPLLAPSS, 1, 0, 0},
> + {66600, P_DDRPLLAPSS, 1, 0, 0},
> + {71000, P_DDRPLLAPSS, 1, 0, 0},

Please keep using F macro just in case the hid changes.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v2 4/5] clk: qcom: ipq4019: Added the all frequencies for apps cpu

[Stephen Boyd]

On 06/21, Abhishek Sahu wrote:
> @@ -554,11 +554,21 @@ static struct clk_rcg2  sdcc1_apps_clk_src = {
>  };
>  
>  static const struct freq_tbl ftbl_gcc_apps_clk[] = {
> - F(4800, P_XO,  1, 0, 0),
> - F(2, P_FEPLL200,   1, 0, 0),
> - F(5, P_FEPLL500,   1, 0, 0),
> - F(62600, P_DDRPLLAPSS, 1, 0, 0),
> - { }
> + {4800, P_XO,  1, 0, 0},
> + {2, P_FEPLL200,   1, 0, 0},
> + {38000, P_DDRPLLAPSS, 1, 0, 0},
> + {40900, P_DDRPLLAPSS, 1, 0, 0},
> + {44400, P_DDRPLLAPSS, 1, 0, 0},
> + {48400, P_DDRPLLAPSS, 1, 0, 0},
> + {5, P_FEPLL500,   1, 0, 0},
> + {50700, P_DDRPLLAPSS, 1, 0, 0},
> + {53200, P_DDRPLLAPSS, 1, 0, 0},
> + {56000, P_DDRPLLAPSS, 1, 0, 0},
> + {59200, P_DDRPLLAPSS, 1, 0, 0},
> + {62600, P_DDRPLLAPSS, 1, 0, 0},
> + {66600, P_DDRPLLAPSS, 1, 0, 0},
> + {71000, P_DDRPLLAPSS, 1, 0, 0},

Please keep using F macro just in case the hid changes.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCHv2 3/4] pci: Determine actual VPD size on first access

[Benjamin Herrenschmidt]

On Mon, 2016-08-15 at 17:59 +, Rustad, Mark D wrote:
> > Benjamin Herrenschmidt  wrote:
> 
> > 
> > We may want some kind of "strict" vs. "relaxed" model here to
> > differenciate the desktop user wanting to give a function to his/her
> > windows partition and doesn't care about strict isolation vs. the cloud
> > data center.
> 
> I don't think desktop users appreciate hangs any more than anyone else, and  
> that is one of the symptoms that can arise here without the vfio  
> coordination.

And can happen with it as well 

Put it this way, we have a hole the size of a football field and we are
spending all that time "plugging" it by putting a 3 foot gate in the
middle of it... ugh.

VFIO shouldn't try to get between the device driver and the HW, it doesn't
work. It cannot work.

Cheers,
Ben.

Re: [PATCHv2 3/4] pci: Determine actual VPD size on first access

[Benjamin Herrenschmidt]

On Mon, 2016-08-15 at 17:59 +, Rustad, Mark D wrote:
> > Benjamin Herrenschmidt  wrote:
> 
> > 
> > We may want some kind of "strict" vs. "relaxed" model here to
> > differenciate the desktop user wanting to give a function to his/her
> > windows partition and doesn't care about strict isolation vs. the cloud
> > data center.
> 
> I don't think desktop users appreciate hangs any more than anyone else, and  
> that is one of the symptoms that can arise here without the vfio  
> coordination.

And can happen with it as well 

Put it this way, we have a hole the size of a football field and we are
spending all that time "plugging" it by putting a 3 foot gate in the
middle of it... ugh.

VFIO shouldn't try to get between the device driver and the HW, it doesn't
work. It cannot work.

Cheers,
Ben.

Re: [LKP] [lkp] [xfs] 68a9f5e700: aim7.jobs-per-min -13.6% regression

[Dave Chinner]

On Sun, Aug 14, 2016 at 10:12:20PM -0700, Linus Torvalds wrote:
> On Aug 14, 2016 10:00 PM, "Dave Chinner"  wrote:
> >
> > > What does it say if you annotate that _raw_spin_unlock_irqrestore()
> function?
> > 
> >¿
> >¿Disassembly of section load0:
> >¿
> >¿81e628b0 :
> >¿  nop
> >¿  push   %rbp
> >¿  mov%rsp,%rbp
> >¿  movb   $0x0,(%rdi)
> >¿  nop
> >¿  mov%rsi,%rdi
> >¿  push   %rdi
> >¿  popfq
> >  99.35 ¿  nop
> 
> Yeah, that's a good disassembly of a non-debug spin unlock, and the symbols
> are fine, but the profile is not valid. That's an interrupt point, right
> after the popf that enables interiors again.
> 
> I don't know why 'perf' isn't working on your machine, but it clearly
> isn't.
> 
> Has it ever worked on that machine?

It's working the same as it's worked since I started using it many
years ago.

> What cpu is it?

Intel(R) Xeon(R) CPU E5-4620 0 @ 2.20GHz

> Are you running in some
> virtualized environment without performance counters, perhaps?

I've mentioned a couple of times in this thread that I'm testing
inside a VM. It's the same VM I've been running performance tests in
since early 2010. Nobody has complained that the profiles I've
posted are useless before, and not once in all that time have they
been wrong in indicating a spinning lock contention point.

i.e. In previous cases where I've measured double digit CPU usage
numbers in a spin_unlock variant, it's always been a result of
spinlock contention. And fixing the algorithmic problem that lead to
the spinlock showing up in the profile in the first place has always
substantially improved performance and scalability.

As such, I'm always going to treat a locking profile like that as
contention because even if it isn't contending *on my machine*,
that amount of work being done under a spinning lock is /way too
much/ and it *will* cause contention problems with larger machines.

> It's not actually the unlock that is expensive, and there is no contention
> on the lock (if there had been, the numbers would have been entirely
> different for the debug case, which makes locking an order of magnitude
> more expensive). All the cost of everything that happened while interrupts
> were disabled is just accounted to the instruction after they were enabled
> again.

Right, but that does not make the profile data useless, nor you
should shoot the messenger because they weren't supplied with
information you think should have been in the message. The message
still says that the majority of the overhead is in
__remove_mapping(), and it's an excessive amount of work being done
inside the tree_lock with interrupts disabled

Cheers,

Dave.
-- 
Dave Chinner
da...@fromorbit.com

Re: [LKP] [lkp] [xfs] 68a9f5e700: aim7.jobs-per-min -13.6% regression

[Dave Chinner]

On Sun, Aug 14, 2016 at 10:12:20PM -0700, Linus Torvalds wrote:
> On Aug 14, 2016 10:00 PM, "Dave Chinner"  wrote:
> >
> > > What does it say if you annotate that _raw_spin_unlock_irqrestore()
> function?
> > 
> >¿
> >¿Disassembly of section load0:
> >¿
> >¿81e628b0 :
> >¿  nop
> >¿  push   %rbp
> >¿  mov%rsp,%rbp
> >¿  movb   $0x0,(%rdi)
> >¿  nop
> >¿  mov%rsi,%rdi
> >¿  push   %rdi
> >¿  popfq
> >  99.35 ¿  nop
> 
> Yeah, that's a good disassembly of a non-debug spin unlock, and the symbols
> are fine, but the profile is not valid. That's an interrupt point, right
> after the popf that enables interiors again.
> 
> I don't know why 'perf' isn't working on your machine, but it clearly
> isn't.
> 
> Has it ever worked on that machine?

It's working the same as it's worked since I started using it many
years ago.

> What cpu is it?

Intel(R) Xeon(R) CPU E5-4620 0 @ 2.20GHz

> Are you running in some
> virtualized environment without performance counters, perhaps?

I've mentioned a couple of times in this thread that I'm testing
inside a VM. It's the same VM I've been running performance tests in
since early 2010. Nobody has complained that the profiles I've
posted are useless before, and not once in all that time have they
been wrong in indicating a spinning lock contention point.

i.e. In previous cases where I've measured double digit CPU usage
numbers in a spin_unlock variant, it's always been a result of
spinlock contention. And fixing the algorithmic problem that lead to
the spinlock showing up in the profile in the first place has always
substantially improved performance and scalability.

As such, I'm always going to treat a locking profile like that as
contention because even if it isn't contending *on my machine*,
that amount of work being done under a spinning lock is /way too
much/ and it *will* cause contention problems with larger machines.

> It's not actually the unlock that is expensive, and there is no contention
> on the lock (if there had been, the numbers would have been entirely
> different for the debug case, which makes locking an order of magnitude
> more expensive). All the cost of everything that happened while interrupts
> were disabled is just accounted to the instruction after they were enabled
> again.

Right, but that does not make the profile data useless, nor you
should shoot the messenger because they weren't supplied with
information you think should have been in the message. The message
still says that the majority of the overhead is in
__remove_mapping(), and it's an excessive amount of work being done
inside the tree_lock with interrupts disabled

Cheers,

Dave.
-- 
Dave Chinner
da...@fromorbit.com

Re: [PATCH v2 1/5] clk: qcom: ipq4019: Added the clock nodes and operations for pll

[Stephen Boyd]

On 06/21, Abhishek Sahu wrote:
> The current ipq4019 clock driver registered the PLL clocks and
> dividers as fixed clock. These fixed clock needs to be removed
> from driver probe function and same need to be registered with
> clock framework. These PLL clocks should be programmed only
> once and the same are being programmed already by the boot
> loader so the set rate operation is not required for these
> clocks. Only the rate can be calculated by clock operations
> in clock driver file so this patch adds the same.
> 
> The PLL takes the reference clock from XO and generates the
> intermediate VCO frequency. This VCO frequency will be divided
> down by different PLL internal dividers. Some of the PLL
> internal dividers are fixed while other are programmable.
> 
> This patch does the following changes.
> 1. Operation for calculating PLL intermediate VCO frequency by
>reading the reference clock divider and feedback divider from
>register. Since VCO frequency falls outside the limit of
>unsigned long for IPQ4019, so this operation will return the
>VCO frequency in KHz.
> 
> 2. Operation for calculating the internal PLL divider clock
>frequency. Clock Divider node should give either fixed
>divider value or divider table(maps the register divider
>value to actual divider value).
> 
> 3. Adds and registers clock nodes for VCO(APPS DDR PLL and FE
>PLL) and PLL internal dividers(SDCC, FEPLL 500 Mhz, FEPLL
>200 Mhz, FEPLL 125 Mhz, FEPLL 125 Mhz with delay,
>programmable WCSS 2G and 5G).
> 
> 4. Changes the regmap limit from 0x2DFFF to 0x2 for
>supporting the PLL registers read.
> 
> 5. Changes the fixed clock name to have consistency in all
>clock names
> 
> Signed-off-by: Abhishek Sahu 

Thanks for fixing this up, just some minor things to fix.

> diff --git a/drivers/clk/qcom/gcc-ipq4019.c b/drivers/clk/qcom/gcc-ipq4019.c
> index 3cd1af0..17ca6d3 100644
> --- a/drivers/clk/qcom/gcc-ipq4019.c
> +++ b/drivers/clk/qcom/gcc-ipq4019.c
> @@ -20,6 +20,11 @@
>  #include 
>  #include 
>  #include 
> +#include 

Is this include used?

> +#include 

Is this include used?

> +#include 
> +#include 
> +#include 

Are both includes needed?

> @@ -40,6 +55,20 @@ enum {
>   P_DDRPLLAPSS,
>  };
>  
> +struct clk_pll_vco {
> + u32 fdbkdiv_shift;
> + u32 fdbkdiv_width;
> + struct clk_regmap_div cdiv;
> +};
> +
> +struct clk_pll_div {
> + u32 fixed_div;
> + const u8 *parent_map;
> + struct clk_regmap_div cdiv;
> + const struct clk_div_table *div_table;
> + const struct freq_tbl *freq_tbl;
> +};
> +
>  static struct parent_map gcc_xo_200_500_map[] = {
>   { P_XO, 0 },
>   { P_FEPLL200, 1 },
> @@ -48,8 +77,8 @@ static struct parent_map gcc_xo_200_500_map[] = {
>  
>  static const char * const gcc_xo_200_500[] = {
>   "xo",
> - "fepll200",
> - "fepll500",
> + "gcc_fepll200_clk",
> + "gcc_fepll500_clk",
>  };
>  
>  static struct parent_map gcc_xo_200_map[] = {
> @@ -59,7 +88,7 @@ static struct parent_map gcc_xo_200_map[] = {
>  
>  static const char * const gcc_xo_200[] = {
>   "xo",
> - "fepll200",
> + "gcc_fepll200_clk",
>  };
>  
>  static struct parent_map gcc_xo_200_spi_map[] = {
> @@ -69,7 +98,7 @@ static struct parent_map gcc_xo_200_spi_map[] = {
>  
>  static const char * const gcc_xo_200_spi[] = {
>   "xo",
> - "fepll200",
> + "gcc_fepll200_clk",
>  };
>  
>  static struct parent_map gcc_xo_sdcc1_500_map[] = {
> @@ -80,8 +109,8 @@ static struct parent_map gcc_xo_sdcc1_500_map[] = {
>  
>  static const char * const gcc_xo_sdcc1_500[] = {
>   "xo",
> - "ddrpll",
> - "fepll500",
> + "gcc_apps_sdcc_clk",
> + "gcc_fepll500_clk",
>  };
>  
>  static struct parent_map gcc_xo_wcss2g_map[] = {
> @@ -91,7 +120,7 @@ static struct parent_map gcc_xo_wcss2g_map[] = {
>  
>  static const char * const gcc_xo_wcss2g[] = {
>   "xo",
> - "fepllwcss2g",
> + "gcc_fepllwcss2g_clk",
>  };
>  
>  static struct parent_map gcc_xo_wcss5g_map[] = {
> @@ -101,7 +130,7 @@ static struct parent_map gcc_xo_wcss5g_map[] = {
>  
>  static const char * const gcc_xo_wcss5g[] = {
>   "xo",
> - "fepllwcss5g",
> + "gcc_fepllwcss5g_clk",
>  };
>  
>  static struct parent_map gcc_xo_125_dly_map[] = {
> @@ -111,7 +140,7 @@ static struct parent_map gcc_xo_125_dly_map[] = {
>  
>  static const char * const gcc_xo_125_dly[] = {
>   "xo",
> - "fepll125dly",
> + "gcc_fepll125dly_clk",
>  };
>  
>  static struct parent_map gcc_xo_ddr_500_200_map[] = {
> @@ -123,8 +152,8 @@ static struct parent_map gcc_xo_ddr_500_200_map[] = {
>  
>  static const char * const gcc_xo_ddr_500_200[] = {
>   "xo",
> - "fepll200",
> - "fepll500",
> + "gcc_fepll200_clk",
> + "gcc_fepll500_clk",
>   "ddrpllapss",
>  };

Was it necessary to change all these names? The gcc_ prefix
doesn't seem to be adding much besides noise to this patch.

>  
> @@ -506,7 

Re: [PATCH v2 1/5] clk: qcom: ipq4019: Added the clock nodes and operations for pll

[Stephen Boyd]

On 06/21, Abhishek Sahu wrote:
> The current ipq4019 clock driver registered the PLL clocks and
> dividers as fixed clock. These fixed clock needs to be removed
> from driver probe function and same need to be registered with
> clock framework. These PLL clocks should be programmed only
> once and the same are being programmed already by the boot
> loader so the set rate operation is not required for these
> clocks. Only the rate can be calculated by clock operations
> in clock driver file so this patch adds the same.
> 
> The PLL takes the reference clock from XO and generates the
> intermediate VCO frequency. This VCO frequency will be divided
> down by different PLL internal dividers. Some of the PLL
> internal dividers are fixed while other are programmable.
> 
> This patch does the following changes.
> 1. Operation for calculating PLL intermediate VCO frequency by
>reading the reference clock divider and feedback divider from
>register. Since VCO frequency falls outside the limit of
>unsigned long for IPQ4019, so this operation will return the
>VCO frequency in KHz.
> 
> 2. Operation for calculating the internal PLL divider clock
>frequency. Clock Divider node should give either fixed
>divider value or divider table(maps the register divider
>value to actual divider value).
> 
> 3. Adds and registers clock nodes for VCO(APPS DDR PLL and FE
>PLL) and PLL internal dividers(SDCC, FEPLL 500 Mhz, FEPLL
>200 Mhz, FEPLL 125 Mhz, FEPLL 125 Mhz with delay,
>programmable WCSS 2G and 5G).
> 
> 4. Changes the regmap limit from 0x2DFFF to 0x2 for
>supporting the PLL registers read.
> 
> 5. Changes the fixed clock name to have consistency in all
>clock names
> 
> Signed-off-by: Abhishek Sahu 

Thanks for fixing this up, just some minor things to fix.

> diff --git a/drivers/clk/qcom/gcc-ipq4019.c b/drivers/clk/qcom/gcc-ipq4019.c
> index 3cd1af0..17ca6d3 100644
> --- a/drivers/clk/qcom/gcc-ipq4019.c
> +++ b/drivers/clk/qcom/gcc-ipq4019.c
> @@ -20,6 +20,11 @@
>  #include 
>  #include 
>  #include 
> +#include 

Is this include used?

> +#include 

Is this include used?

> +#include 
> +#include 
> +#include 

Are both includes needed?

> @@ -40,6 +55,20 @@ enum {
>   P_DDRPLLAPSS,
>  };
>  
> +struct clk_pll_vco {
> + u32 fdbkdiv_shift;
> + u32 fdbkdiv_width;
> + struct clk_regmap_div cdiv;
> +};
> +
> +struct clk_pll_div {
> + u32 fixed_div;
> + const u8 *parent_map;
> + struct clk_regmap_div cdiv;
> + const struct clk_div_table *div_table;
> + const struct freq_tbl *freq_tbl;
> +};
> +
>  static struct parent_map gcc_xo_200_500_map[] = {
>   { P_XO, 0 },
>   { P_FEPLL200, 1 },
> @@ -48,8 +77,8 @@ static struct parent_map gcc_xo_200_500_map[] = {
>  
>  static const char * const gcc_xo_200_500[] = {
>   "xo",
> - "fepll200",
> - "fepll500",
> + "gcc_fepll200_clk",
> + "gcc_fepll500_clk",
>  };
>  
>  static struct parent_map gcc_xo_200_map[] = {
> @@ -59,7 +88,7 @@ static struct parent_map gcc_xo_200_map[] = {
>  
>  static const char * const gcc_xo_200[] = {
>   "xo",
> - "fepll200",
> + "gcc_fepll200_clk",
>  };
>  
>  static struct parent_map gcc_xo_200_spi_map[] = {
> @@ -69,7 +98,7 @@ static struct parent_map gcc_xo_200_spi_map[] = {
>  
>  static const char * const gcc_xo_200_spi[] = {
>   "xo",
> - "fepll200",
> + "gcc_fepll200_clk",
>  };
>  
>  static struct parent_map gcc_xo_sdcc1_500_map[] = {
> @@ -80,8 +109,8 @@ static struct parent_map gcc_xo_sdcc1_500_map[] = {
>  
>  static const char * const gcc_xo_sdcc1_500[] = {
>   "xo",
> - "ddrpll",
> - "fepll500",
> + "gcc_apps_sdcc_clk",
> + "gcc_fepll500_clk",
>  };
>  
>  static struct parent_map gcc_xo_wcss2g_map[] = {
> @@ -91,7 +120,7 @@ static struct parent_map gcc_xo_wcss2g_map[] = {
>  
>  static const char * const gcc_xo_wcss2g[] = {
>   "xo",
> - "fepllwcss2g",
> + "gcc_fepllwcss2g_clk",
>  };
>  
>  static struct parent_map gcc_xo_wcss5g_map[] = {
> @@ -101,7 +130,7 @@ static struct parent_map gcc_xo_wcss5g_map[] = {
>  
>  static const char * const gcc_xo_wcss5g[] = {
>   "xo",
> - "fepllwcss5g",
> + "gcc_fepllwcss5g_clk",
>  };
>  
>  static struct parent_map gcc_xo_125_dly_map[] = {
> @@ -111,7 +140,7 @@ static struct parent_map gcc_xo_125_dly_map[] = {
>  
>  static const char * const gcc_xo_125_dly[] = {
>   "xo",
> - "fepll125dly",
> + "gcc_fepll125dly_clk",
>  };
>  
>  static struct parent_map gcc_xo_ddr_500_200_map[] = {
> @@ -123,8 +152,8 @@ static struct parent_map gcc_xo_ddr_500_200_map[] = {
>  
>  static const char * const gcc_xo_ddr_500_200[] = {
>   "xo",
> - "fepll200",
> - "fepll500",
> + "gcc_fepll200_clk",
> + "gcc_fepll500_clk",
>   "ddrpllapss",
>  };

Was it necessary to change all these names? The gcc_ prefix
doesn't seem to be adding much besides noise to this patch.

>  
> @@ -506,7 +535,7 @@ static const 

Re: [PATCH] time,virt: resync steal time when guest & host lose sync

[Wanpeng Li]

2016-08-15 23:00 GMT+08:00 Rik van Riel :
> On Mon, 2016-08-15 at 16:53 +0800, Wanpeng Li wrote:
>> 2016-08-12 23:58 GMT+08:00 Rik van Riel :
>> [...]
>> > Wanpeng, does the patch below work for you?
>>
>> It will break steal time for full dynticks guest, and there is a
>> calltrace of thread_group_cputime_adjusted call stack, RIP is
>> cputime_adjust+0xff/0x130.
>
> How?  This patch is equivalent to passing ULONG_MAX to
> steal_account_process_time, which you tried to no ill
> effect before.

https://lkml.org/lkml/2016/8/15/217 I sent out a patch yesterday which
can fix the regression. Your patch breaks full dynticks guest since
vtime doesn't depend on clock ticks, so we should keep the max cputime
limit for it as my patch description mentioned, remove the limit for
vtime results in the calltrace. My patch does what Paolo suggested
https://lkml.org/lkml/2016/8/12/380 for lost ticks scenario.

Regards,
Wanpeng Li

Re: [PATCH] time,virt: resync steal time when guest & host lose sync

[Wanpeng Li]

2016-08-15 23:00 GMT+08:00 Rik van Riel :
> On Mon, 2016-08-15 at 16:53 +0800, Wanpeng Li wrote:
>> 2016-08-12 23:58 GMT+08:00 Rik van Riel :
>> [...]
>> > Wanpeng, does the patch below work for you?
>>
>> It will break steal time for full dynticks guest, and there is a
>> calltrace of thread_group_cputime_adjusted call stack, RIP is
>> cputime_adjust+0xff/0x130.
>
> How?  This patch is equivalent to passing ULONG_MAX to
> steal_account_process_time, which you tried to no ill
> effect before.

https://lkml.org/lkml/2016/8/15/217 I sent out a patch yesterday which
can fix the regression. Your patch breaks full dynticks guest since
vtime doesn't depend on clock ticks, so we should keep the max cputime
limit for it as my patch description mentioned, remove the limit for
vtime results in the calltrace. My patch does what Paolo suggested
https://lkml.org/lkml/2016/8/12/380 for lost ticks scenario.

Regards,
Wanpeng Li

Re: [PATCH v2 2/2] cpufreq / sched: Pass runqueue pointer to cpufreq_update_util()

[Steve Muckle]

LGTM

On Fri, Aug 12, 2016 at 02:06:44AM +0200, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki 
> 
> All of the callers of cpufreq_update_util() pass rq_clock(rq) to it
> as the time argument and some of them check whether or not cpu_of(rq)
> is equal to smp_processor_id() before calling it, so rework it to
> take a runqueue pointer as the argument and move the rq_clock(rq)
> evaluation into it.
> 
> Additionally, provide a wrapper checking cpu_of(rq) against
> smp_processor_id() for the cpufreq_update_util() callers that
> need it.
> 
> Signed-off-by: Rafael J. Wysocki 
> ---
> 
> This is a new patch based on the [2/2] from v1.
> 
> ---
>  kernel/sched/deadline.c |3 +--
>  kernel/sched/fair.c |4 +---
>  kernel/sched/rt.c   |3 +--
>  kernel/sched/sched.h|   15 +++
>  4 files changed, 14 insertions(+), 11 deletions(-)
> 
> Index: linux-pm/kernel/sched/deadline.c
> ===
> --- linux-pm.orig/kernel/sched/deadline.c
> +++ linux-pm/kernel/sched/deadline.c
> @@ -733,8 +733,7 @@ static void update_curr_dl(struct rq *rq
>   }
>  
>   /* kick cpufreq (see the comment in kernel/sched/sched.h). */
> - if (cpu_of(rq) == smp_processor_id())
> - cpufreq_update_util(rq_clock(rq), SCHED_CPUFREQ_DL);
> + cpufreq_update_this_cpu(rq, SCHED_CPUFREQ_DL);
>  
>   schedstat_set(curr->se.statistics.exec_max,
> max(curr->se.statistics.exec_max, delta_exec));
> Index: linux-pm/kernel/sched/fair.c
> ===
> --- linux-pm.orig/kernel/sched/fair.c
> +++ linux-pm/kernel/sched/fair.c
> @@ -2876,8 +2876,6 @@ static inline void update_tg_load_avg(st
>  static inline void cfs_rq_util_change(struct cfs_rq *cfs_rq)
>  {
>   if (_rq()->cfs == cfs_rq) {
> - struct rq *rq = rq_of(cfs_rq);
> -
>   /*
>* There are a few boundary cases this might miss but it should
>* get called often enough that that should (hopefully) not be
> @@ -2894,7 +2892,7 @@ static inline void cfs_rq_util_change(st
>*
>* See cpu_util().
>*/
> - cpufreq_update_util(rq_clock(rq), 0);
> + cpufreq_update_util(rq_of(cfs_rq), 0);
>   }
>  }
>  
> Index: linux-pm/kernel/sched/rt.c
> ===
> --- linux-pm.orig/kernel/sched/rt.c
> +++ linux-pm/kernel/sched/rt.c
> @@ -958,8 +958,7 @@ static void update_curr_rt(struct rq *rq
>   return;
>  
>   /* Kick cpufreq (see the comment in kernel/sched/sched.h). */
> - if (cpu_of(rq) == smp_processor_id())
> - cpufreq_update_util(rq_clock(rq), SCHED_CPUFREQ_RT);
> + cpufreq_update_this_cpu(rq, SCHED_CPUFREQ_RT);
>  
>   schedstat_set(curr->se.statistics.exec_max,
> max(curr->se.statistics.exec_max, delta_exec));
> Index: linux-pm/kernel/sched/sched.h
> ===
> --- linux-pm.orig/kernel/sched/sched.h
> +++ linux-pm/kernel/sched/sched.h
> @@ -1763,7 +1763,7 @@ DECLARE_PER_CPU(struct update_util_data
>  
>  /**
>   * cpufreq_update_util - Take a note about CPU utilization changes.
> - * @time: Current time.
> + * @rq: Runqueue to carry out the update for.
>   * @flags: Update reason flags.
>   *
>   * This function is called by the scheduler on the CPU whose utilization is
> @@ -1783,16 +1783,23 @@ DECLARE_PER_CPU(struct update_util_data
>   * but that really is a band-aid.  Going forward it should be replaced with
>   * solutions targeted more specifically at RT and DL tasks.
>   */
> -static inline void cpufreq_update_util(u64 time, unsigned int flags)
> +static inline void cpufreq_update_util(struct rq *rq, unsigned int flags)
>  {
>   struct update_util_data *data;
>  
>   data = rcu_dereference_sched(*this_cpu_ptr(_update_util_data));
>   if (data)
> - data->func(data, time, flags);
> + data->func(data, rq_clock(rq), flags);
> +}
> +
> +static inline void cpufreq_update_this_cpu(struct rq *rq, unsigned int flags)
> +{
> + if (cpu_of(rq) == smp_processor_id())
> + cpufreq_update_util(rq, flags);
>  }
>  #else
> -static inline void cpufreq_update_util(u64 time, unsigned int flags) {}
> +static inline void cpufreq_update_util(struct rq *rq, unsigned int flags) {}
> +static inline void cpufreq_update_this_cpu(struct rq *rq, unsigned int 
> flags) {}
>  #endif /* CONFIG_CPU_FREQ */
>  
>  #ifdef arch_scale_freq_capacity
> 

Re: [PATCH] iio: magnetometer: mag3110: claim direct mode during raw reads

[Alison Schofield]

On Mon, Aug 15, 2016 at 05:04:24PM +0100, Jonathan Cameron wrote:
> On 01/08/16 16:48, Alison Schofield wrote:
> > Driver was checking for direct mode but not locking it.  Use
> > claim/release helper functions to guarantee the device stays
> > in direct mode during raw reads.
> > 
> > Signed-off-by: Alison Schofield 
> > Cc: Daniel Baluta 
> > ---
> >  drivers/iio/magnetometer/mag3110.c | 21 +
> >  1 file changed, 13 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/iio/magnetometer/mag3110.c 
> > b/drivers/iio/magnetometer/mag3110.c
> > index f2be4a0..90574ff 100644
> > --- a/drivers/iio/magnetometer/mag3110.c
> > +++ b/drivers/iio/magnetometer/mag3110.c
> > @@ -154,34 +154,39 @@ static int mag3110_read_raw(struct iio_dev *indio_dev,
> >  
> > switch (mask) {
> > case IIO_CHAN_INFO_RAW:
> > -   if (iio_buffer_enabled(indio_dev))
> > -   return -EBUSY;
> > +   ret = iio_device_claim_direct_mode(indio_dev);
> > +   if (ret)
> > +   return ret;
> >  
> > switch (chan->type) {
> > case IIO_MAGN: /* in 0.1 uT / LSB */
> > ret = mag3110_read(data, buffer);
> > if (ret < 0)
> > -   return ret;
> > +   goto release;
> > *val = sign_extend32(
> > be16_to_cpu(buffer[chan->scan_index]), 15);
> > -   return IIO_VAL_INT;
> > +   ret = IIO_VAL_INT;
> No break?
oops...needs one and the next case too. Thanks!


> > case IIO_TEMP: /* in 1 C / LSB */
> > mutex_lock(>lock);
> > ret = mag3110_request(data);
> > if (ret < 0) {
> > mutex_unlock(>lock);
> > -   return ret;
> > +   goto release;
> > }
> > ret = i2c_smbus_read_byte_data(data->client,
> > MAG3110_DIE_TEMP);
> > mutex_unlock(>lock);
> > if (ret < 0)
> > -   return ret;
> > +   goto release;
> > *val = sign_extend32(ret, 7);
> > -   return IIO_VAL_INT;
> > +   ret = IIO_VAL_INT;
> No break here either
> > default:
> > -   return -EINVAL;
> > +   ret = -EINVAL;
> > }
> > +release:
> > +   iio_device_release_direct_mode(indio_dev);
> > +   return ret;
> > +
> > case IIO_CHAN_INFO_SCALE:
> > switch (chan->type) {
> > case IIO_MAGN:
> > 
> 

Re: [PATCH v2 2/2] cpufreq / sched: Pass runqueue pointer to cpufreq_update_util()

[Steve Muckle]

LGTM

On Fri, Aug 12, 2016 at 02:06:44AM +0200, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki 
> 
> All of the callers of cpufreq_update_util() pass rq_clock(rq) to it
> as the time argument and some of them check whether or not cpu_of(rq)
> is equal to smp_processor_id() before calling it, so rework it to
> take a runqueue pointer as the argument and move the rq_clock(rq)
> evaluation into it.
> 
> Additionally, provide a wrapper checking cpu_of(rq) against
> smp_processor_id() for the cpufreq_update_util() callers that
> need it.
> 
> Signed-off-by: Rafael J. Wysocki 
> ---
> 
> This is a new patch based on the [2/2] from v1.
> 
> ---
>  kernel/sched/deadline.c |3 +--
>  kernel/sched/fair.c |4 +---
>  kernel/sched/rt.c   |3 +--
>  kernel/sched/sched.h|   15 +++
>  4 files changed, 14 insertions(+), 11 deletions(-)
> 
> Index: linux-pm/kernel/sched/deadline.c
> ===
> --- linux-pm.orig/kernel/sched/deadline.c
> +++ linux-pm/kernel/sched/deadline.c
> @@ -733,8 +733,7 @@ static void update_curr_dl(struct rq *rq
>   }
>  
>   /* kick cpufreq (see the comment in kernel/sched/sched.h). */
> - if (cpu_of(rq) == smp_processor_id())
> - cpufreq_update_util(rq_clock(rq), SCHED_CPUFREQ_DL);
> + cpufreq_update_this_cpu(rq, SCHED_CPUFREQ_DL);
>  
>   schedstat_set(curr->se.statistics.exec_max,
> max(curr->se.statistics.exec_max, delta_exec));
> Index: linux-pm/kernel/sched/fair.c
> ===
> --- linux-pm.orig/kernel/sched/fair.c
> +++ linux-pm/kernel/sched/fair.c
> @@ -2876,8 +2876,6 @@ static inline void update_tg_load_avg(st
>  static inline void cfs_rq_util_change(struct cfs_rq *cfs_rq)
>  {
>   if (_rq()->cfs == cfs_rq) {
> - struct rq *rq = rq_of(cfs_rq);
> -
>   /*
>* There are a few boundary cases this might miss but it should
>* get called often enough that that should (hopefully) not be
> @@ -2894,7 +2892,7 @@ static inline void cfs_rq_util_change(st
>*
>* See cpu_util().
>*/
> - cpufreq_update_util(rq_clock(rq), 0);
> + cpufreq_update_util(rq_of(cfs_rq), 0);
>   }
>  }
>  
> Index: linux-pm/kernel/sched/rt.c
> ===
> --- linux-pm.orig/kernel/sched/rt.c
> +++ linux-pm/kernel/sched/rt.c
> @@ -958,8 +958,7 @@ static void update_curr_rt(struct rq *rq
>   return;
>  
>   /* Kick cpufreq (see the comment in kernel/sched/sched.h). */
> - if (cpu_of(rq) == smp_processor_id())
> - cpufreq_update_util(rq_clock(rq), SCHED_CPUFREQ_RT);
> + cpufreq_update_this_cpu(rq, SCHED_CPUFREQ_RT);
>  
>   schedstat_set(curr->se.statistics.exec_max,
> max(curr->se.statistics.exec_max, delta_exec));
> Index: linux-pm/kernel/sched/sched.h
> ===
> --- linux-pm.orig/kernel/sched/sched.h
> +++ linux-pm/kernel/sched/sched.h
> @@ -1763,7 +1763,7 @@ DECLARE_PER_CPU(struct update_util_data
>  
>  /**
>   * cpufreq_update_util - Take a note about CPU utilization changes.
> - * @time: Current time.
> + * @rq: Runqueue to carry out the update for.
>   * @flags: Update reason flags.
>   *
>   * This function is called by the scheduler on the CPU whose utilization is
> @@ -1783,16 +1783,23 @@ DECLARE_PER_CPU(struct update_util_data
>   * but that really is a band-aid.  Going forward it should be replaced with
>   * solutions targeted more specifically at RT and DL tasks.
>   */
> -static inline void cpufreq_update_util(u64 time, unsigned int flags)
> +static inline void cpufreq_update_util(struct rq *rq, unsigned int flags)
>  {
>   struct update_util_data *data;
>  
>   data = rcu_dereference_sched(*this_cpu_ptr(_update_util_data));
>   if (data)
> - data->func(data, time, flags);
> + data->func(data, rq_clock(rq), flags);
> +}
> +
> +static inline void cpufreq_update_this_cpu(struct rq *rq, unsigned int flags)
> +{
> + if (cpu_of(rq) == smp_processor_id())
> + cpufreq_update_util(rq, flags);
>  }
>  #else
> -static inline void cpufreq_update_util(u64 time, unsigned int flags) {}
> +static inline void cpufreq_update_util(struct rq *rq, unsigned int flags) {}
> +static inline void cpufreq_update_this_cpu(struct rq *rq, unsigned int 
> flags) {}
>  #endif /* CONFIG_CPU_FREQ */
>  
>  #ifdef arch_scale_freq_capacity
> 

Re: [PATCH] iio: magnetometer: mag3110: claim direct mode during raw reads

[Alison Schofield]

On Mon, Aug 15, 2016 at 05:04:24PM +0100, Jonathan Cameron wrote:
> On 01/08/16 16:48, Alison Schofield wrote:
> > Driver was checking for direct mode but not locking it.  Use
> > claim/release helper functions to guarantee the device stays
> > in direct mode during raw reads.
> > 
> > Signed-off-by: Alison Schofield 
> > Cc: Daniel Baluta 
> > ---
> >  drivers/iio/magnetometer/mag3110.c | 21 +
> >  1 file changed, 13 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/iio/magnetometer/mag3110.c 
> > b/drivers/iio/magnetometer/mag3110.c
> > index f2be4a0..90574ff 100644
> > --- a/drivers/iio/magnetometer/mag3110.c
> > +++ b/drivers/iio/magnetometer/mag3110.c
> > @@ -154,34 +154,39 @@ static int mag3110_read_raw(struct iio_dev *indio_dev,
> >  
> > switch (mask) {
> > case IIO_CHAN_INFO_RAW:
> > -   if (iio_buffer_enabled(indio_dev))
> > -   return -EBUSY;
> > +   ret = iio_device_claim_direct_mode(indio_dev);
> > +   if (ret)
> > +   return ret;
> >  
> > switch (chan->type) {
> > case IIO_MAGN: /* in 0.1 uT / LSB */
> > ret = mag3110_read(data, buffer);
> > if (ret < 0)
> > -   return ret;
> > +   goto release;
> > *val = sign_extend32(
> > be16_to_cpu(buffer[chan->scan_index]), 15);
> > -   return IIO_VAL_INT;
> > +   ret = IIO_VAL_INT;
> No break?
oops...needs one and the next case too. Thanks!


> > case IIO_TEMP: /* in 1 C / LSB */
> > mutex_lock(>lock);
> > ret = mag3110_request(data);
> > if (ret < 0) {
> > mutex_unlock(>lock);
> > -   return ret;
> > +   goto release;
> > }
> > ret = i2c_smbus_read_byte_data(data->client,
> > MAG3110_DIE_TEMP);
> > mutex_unlock(>lock);
> > if (ret < 0)
> > -   return ret;
> > +   goto release;
> > *val = sign_extend32(ret, 7);
> > -   return IIO_VAL_INT;
> > +   ret = IIO_VAL_INT;
> No break here either
> > default:
> > -   return -EINVAL;
> > +   ret = -EINVAL;
> > }
> > +release:
> > +   iio_device_release_direct_mode(indio_dev);
> > +   return ret;
> > +
> > case IIO_CHAN_INFO_SCALE:
> > switch (chan->type) {
> > case IIO_MAGN:
> > 
> 

Re: [PATCH v2 1/2] cpufreq / sched: Pass flags to cpufreq_update_util()

[Steve Muckle]

LGTM

On Fri, Aug 12, 2016 at 02:04:42AM +0200, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki 
> 
> It is useful to know the reason why cpufreq_update_util() has just
> been called and that can be passed as flags to cpufreq_update_util()
> and to the ->func() callback in struct update_util_data.  However,
> doing that in addition to passing the util and max arguments they
> already take would be clumsy, so avoid it.
> 
> Instead, use the observation that the schedutil governor is part
> of the scheduler proper, so it can access scheduler data directly.
> This allows the util and max arguments of cpufreq_update_util()
> and the ->func() callback in struct update_util_data to be replaced
> with a flags one, but schedutil has to be modified to follow.
> 
> Thus make the schedutil governor obtain the CFS utilization
> information from the scheduler and use the "RT" and "DL" flags
> instead of the special utilization value of ULONG_MAX to track
> updates from the RT and DL sched classes.  Make it non-modular
> too to avoid having to export scheduler variables to modules at
> large.
> 
> Next, update all of the other users of cpufreq_update_util()
> and the ->func() callback in struct update_util_data accordingly.
> 
> Suggested-by: Peter Zijlstra 
> Signed-off-by: Rafael J. Wysocki 
> ---
> 
> v1 -> v2: Do not check cpu_of(rq) against smp_processor_id() in
>   cfs_rq_util_change().
> 
> ---
>  drivers/cpufreq/Kconfig|5 --
>  drivers/cpufreq/cpufreq_governor.c |2 -
>  drivers/cpufreq/intel_pstate.c |2 -
>  include/linux/sched.h  |   12 --
>  kernel/sched/cpufreq.c |2 -
>  kernel/sched/cpufreq_schedutil.c   |   67 
> -
>  kernel/sched/deadline.c|4 +-
>  kernel/sched/fair.c|   10 +
>  kernel/sched/rt.c  |4 +-
>  kernel/sched/sched.h   |   31 +
>  10 files changed, 66 insertions(+), 73 deletions(-)
> 
> Index: linux-pm/drivers/cpufreq/cpufreq_governor.c
> ===
> --- linux-pm.orig/drivers/cpufreq/cpufreq_governor.c
> +++ linux-pm/drivers/cpufreq/cpufreq_governor.c
> @@ -260,7 +260,7 @@ static void dbs_irq_work(struct irq_work
>  }
>  
>  static void dbs_update_util_handler(struct update_util_data *data, u64 time,
> - unsigned long util, unsigned long max)
> + unsigned int flags)
>  {
>   struct cpu_dbs_info *cdbs = container_of(data, struct cpu_dbs_info, 
> update_util);
>   struct policy_dbs_info *policy_dbs = cdbs->policy_dbs;
> Index: linux-pm/drivers/cpufreq/intel_pstate.c
> ===
> --- linux-pm.orig/drivers/cpufreq/intel_pstate.c
> +++ linux-pm/drivers/cpufreq/intel_pstate.c
> @@ -1329,7 +1329,7 @@ static inline void intel_pstate_adjust_b
>  }
>  
>  static void intel_pstate_update_util(struct update_util_data *data, u64 time,
> -  unsigned long util, unsigned long max)
> +  unsigned int flags)
>  {
>   struct cpudata *cpu = container_of(data, struct cpudata, update_util);
>   u64 delta_ns = time - cpu->sample.time;
> Index: linux-pm/include/linux/sched.h
> ===
> --- linux-pm.orig/include/linux/sched.h
> +++ linux-pm/include/linux/sched.h
> @@ -3469,15 +3469,19 @@ static inline unsigned long rlimit_max(u
>   return task_rlimit_max(current, limit);
>  }
>  
> +#define SCHED_CPUFREQ_RT (1U << 0)
> +#define SCHED_CPUFREQ_DL (1U << 1)
> +
> +#define SCHED_CPUFREQ_RT_DL  (SCHED_CPUFREQ_RT | SCHED_CPUFREQ_DL)
> +
>  #ifdef CONFIG_CPU_FREQ
>  struct update_util_data {
> - void (*func)(struct update_util_data *data,
> -  u64 time, unsigned long util, unsigned long max);
> +   void (*func)(struct update_util_data *data, u64 time, unsigned int 
> flags);
>  };
>  
>  void cpufreq_add_update_util_hook(int cpu, struct update_util_data *data,
> - void (*func)(struct update_util_data *data, u64 time,
> -  unsigned long util, unsigned long max));
> +   void (*func)(struct update_util_data *data, u64 time,
> + unsigned int flags));
>  void cpufreq_remove_update_util_hook(int cpu);
>  #endif /* CONFIG_CPU_FREQ */
>  
> Index: linux-pm/kernel/sched/cpufreq.c
> ===
> --- linux-pm.orig/kernel/sched/cpufreq.c
> +++ linux-pm/kernel/sched/cpufreq.c
> @@ -33,7 +33,7 @@ DEFINE_PER_CPU(struct update_util_data *
>   */
>  void cpufreq_add_update_util_hook(int cpu, struct update_util_data *data,
>   void (*func)(struct 

Re: [PATCH v2 1/2] cpufreq / sched: Pass flags to cpufreq_update_util()

[Steve Muckle]

LGTM

On Fri, Aug 12, 2016 at 02:04:42AM +0200, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki 
> 
> It is useful to know the reason why cpufreq_update_util() has just
> been called and that can be passed as flags to cpufreq_update_util()
> and to the ->func() callback in struct update_util_data.  However,
> doing that in addition to passing the util and max arguments they
> already take would be clumsy, so avoid it.
> 
> Instead, use the observation that the schedutil governor is part
> of the scheduler proper, so it can access scheduler data directly.
> This allows the util and max arguments of cpufreq_update_util()
> and the ->func() callback in struct update_util_data to be replaced
> with a flags one, but schedutil has to be modified to follow.
> 
> Thus make the schedutil governor obtain the CFS utilization
> information from the scheduler and use the "RT" and "DL" flags
> instead of the special utilization value of ULONG_MAX to track
> updates from the RT and DL sched classes.  Make it non-modular
> too to avoid having to export scheduler variables to modules at
> large.
> 
> Next, update all of the other users of cpufreq_update_util()
> and the ->func() callback in struct update_util_data accordingly.
> 
> Suggested-by: Peter Zijlstra 
> Signed-off-by: Rafael J. Wysocki 
> ---
> 
> v1 -> v2: Do not check cpu_of(rq) against smp_processor_id() in
>   cfs_rq_util_change().
> 
> ---
>  drivers/cpufreq/Kconfig|5 --
>  drivers/cpufreq/cpufreq_governor.c |2 -
>  drivers/cpufreq/intel_pstate.c |2 -
>  include/linux/sched.h  |   12 --
>  kernel/sched/cpufreq.c |2 -
>  kernel/sched/cpufreq_schedutil.c   |   67 
> -
>  kernel/sched/deadline.c|4 +-
>  kernel/sched/fair.c|   10 +
>  kernel/sched/rt.c  |4 +-
>  kernel/sched/sched.h   |   31 +
>  10 files changed, 66 insertions(+), 73 deletions(-)
> 
> Index: linux-pm/drivers/cpufreq/cpufreq_governor.c
> ===
> --- linux-pm.orig/drivers/cpufreq/cpufreq_governor.c
> +++ linux-pm/drivers/cpufreq/cpufreq_governor.c
> @@ -260,7 +260,7 @@ static void dbs_irq_work(struct irq_work
>  }
>  
>  static void dbs_update_util_handler(struct update_util_data *data, u64 time,
> - unsigned long util, unsigned long max)
> + unsigned int flags)
>  {
>   struct cpu_dbs_info *cdbs = container_of(data, struct cpu_dbs_info, 
> update_util);
>   struct policy_dbs_info *policy_dbs = cdbs->policy_dbs;
> Index: linux-pm/drivers/cpufreq/intel_pstate.c
> ===
> --- linux-pm.orig/drivers/cpufreq/intel_pstate.c
> +++ linux-pm/drivers/cpufreq/intel_pstate.c
> @@ -1329,7 +1329,7 @@ static inline void intel_pstate_adjust_b
>  }
>  
>  static void intel_pstate_update_util(struct update_util_data *data, u64 time,
> -  unsigned long util, unsigned long max)
> +  unsigned int flags)
>  {
>   struct cpudata *cpu = container_of(data, struct cpudata, update_util);
>   u64 delta_ns = time - cpu->sample.time;
> Index: linux-pm/include/linux/sched.h
> ===
> --- linux-pm.orig/include/linux/sched.h
> +++ linux-pm/include/linux/sched.h
> @@ -3469,15 +3469,19 @@ static inline unsigned long rlimit_max(u
>   return task_rlimit_max(current, limit);
>  }
>  
> +#define SCHED_CPUFREQ_RT (1U << 0)
> +#define SCHED_CPUFREQ_DL (1U << 1)
> +
> +#define SCHED_CPUFREQ_RT_DL  (SCHED_CPUFREQ_RT | SCHED_CPUFREQ_DL)
> +
>  #ifdef CONFIG_CPU_FREQ
>  struct update_util_data {
> - void (*func)(struct update_util_data *data,
> -  u64 time, unsigned long util, unsigned long max);
> +   void (*func)(struct update_util_data *data, u64 time, unsigned int 
> flags);
>  };
>  
>  void cpufreq_add_update_util_hook(int cpu, struct update_util_data *data,
> - void (*func)(struct update_util_data *data, u64 time,
> -  unsigned long util, unsigned long max));
> +   void (*func)(struct update_util_data *data, u64 time,
> + unsigned int flags));
>  void cpufreq_remove_update_util_hook(int cpu);
>  #endif /* CONFIG_CPU_FREQ */
>  
> Index: linux-pm/kernel/sched/cpufreq.c
> ===
> --- linux-pm.orig/kernel/sched/cpufreq.c
> +++ linux-pm/kernel/sched/cpufreq.c
> @@ -33,7 +33,7 @@ DEFINE_PER_CPU(struct update_util_data *
>   */
>  void cpufreq_add_update_util_hook(int cpu, struct update_util_data *data,
>   void (*func)(struct update_util_data *data, u64 time,
> -  unsigned long 

Re: [PATCH 1/2] clk: fixed-factor: Remove export symbol on setup function

[Stephen Boyd]

On 08/12, Stephen Boyd wrote:
> This function is marked __init, so it can't possibly need to be
> exported to modules. Remove the marking.
> 
> Cc: Gregory CLEMENT 
> Signed-off-by: Stephen Boyd 
> ---

Applied to clk-next

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH 1/2] clk: fixed-factor: Remove export symbol on setup function

[Stephen Boyd]

On 08/12, Stephen Boyd wrote:
> This function is marked __init, so it can't possibly need to be
> exported to modules. Remove the marking.
> 
> Cc: Gregory CLEMENT 
> Signed-off-by: Stephen Boyd 
> ---

Applied to clk-next

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH 2/2] clk: fixed-rate: Remove export symbol on setup function

[Stephen Boyd]

On 08/12, Stephen Boyd wrote:
> This function is only called by builtin code, but we always
> exported it and had marked it as __init before commit
> e4eda8e0654c (clk: remove exported function from __init section,
> 2013-01-06) removed that marking. Given that it isn't used by
> modules, lets unexport it and add back __init.
> 
> Cc: Denis Efremov 
> Signed-off-by: Stephen Boyd 
> ---

Applied to clk-next

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH 2/2] clk: fixed-rate: Remove export symbol on setup function

[Stephen Boyd]

On 08/12, Stephen Boyd wrote:
> This function is only called by builtin code, but we always
> exported it and had marked it as __init before commit
> e4eda8e0654c (clk: remove exported function from __init section,
> 2013-01-06) removed that marking. Given that it isn't used by
> modules, lets unexport it and add back __init.
> 
> Cc: Denis Efremov 
> Signed-off-by: Stephen Boyd 
> ---

Applied to clk-next

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: Building a subdirectory ignores parent subdir-ccflags

[Michal Marek]

Dne 15.8.2016 v 23:29 Joe Perches napsal(a):
> On Mon, 2016-08-15 at 14:14 -0700, Joe Perches wrote:
>> On Mon, 2016-08-15 at 23:04 +0200, Greg Kroah-Hartman wrote:
>>> On Mon, Aug 15, 2016 at 12:33:23PM -0700, Joe Perches wrote:
 Start to rationalize include paths in source code files.
>> []
 diff --git a/drivers/staging/lustre/Makefile 
 b/drivers/staging/lustre/Makefile
>> []
 @@ -1,2 +1,5 @@
 +subdir-ccflags-y += -I$(srctree)/drivers/staging/lustre/include/
 +subdir-ccflags-y += -I$(srctree)/drivers/staging/lustre/lustre/include/
 +
  obj-$(CONFIG_LNET)+= lnet/
  obj-$(CONFIG_LUSTRE_FS)   += lustre/
>>> This is good, but does this break the subdir make command:
>>> make M=drivers/staging/lustre/foo_dir/
>>> ?
>> hmm, yeah, it does.  Oh well, nevermind for awhile.
>>> I remember the last time I tried to clean this up, it took a while...
>> It seems like something the build tools should
>> handle correctly now, but I'll look at it.
> 
> Perhaps making a specific directory should also walk up
> any parent directory Makefiles looking for subdir flags.
> 
> Is that unreasonable?  Any suggestions?

I suggest to do make drivers/staging/lustre/. If building the lustre
subdirectories is going to be a common use case, then you can propagate
the subdir-ccflags-y assignment down to the individual Makefiles.

Michal

Re: Building a subdirectory ignores parent subdir-ccflags

[Michal Marek]

Dne 15.8.2016 v 23:29 Joe Perches napsal(a):
> On Mon, 2016-08-15 at 14:14 -0700, Joe Perches wrote:
>> On Mon, 2016-08-15 at 23:04 +0200, Greg Kroah-Hartman wrote:
>>> On Mon, Aug 15, 2016 at 12:33:23PM -0700, Joe Perches wrote:
 Start to rationalize include paths in source code files.
>> []
 diff --git a/drivers/staging/lustre/Makefile 
 b/drivers/staging/lustre/Makefile
>> []
 @@ -1,2 +1,5 @@
 +subdir-ccflags-y += -I$(srctree)/drivers/staging/lustre/include/
 +subdir-ccflags-y += -I$(srctree)/drivers/staging/lustre/lustre/include/
 +
  obj-$(CONFIG_LNET)+= lnet/
  obj-$(CONFIG_LUSTRE_FS)   += lustre/
>>> This is good, but does this break the subdir make command:
>>> make M=drivers/staging/lustre/foo_dir/
>>> ?
>> hmm, yeah, it does.  Oh well, nevermind for awhile.
>>> I remember the last time I tried to clean this up, it took a while...
>> It seems like something the build tools should
>> handle correctly now, but I'll look at it.
> 
> Perhaps making a specific directory should also walk up
> any parent directory Makefiles looking for subdir flags.
> 
> Is that unreasonable?  Any suggestions?

I suggest to do make drivers/staging/lustre/. If building the lustre
subdirectories is going to be a common use case, then you can propagate
the subdir-ccflags-y assignment down to the individual Makefiles.

Michal

Re: [PATCH 3.2 44/94] wext: Fix 32 bit iwpriv compatibility issue with 64 bit Kernel

[Ben Hutchings]

On Mon, 2016-08-15 at 07:51 +0200, Johannes Berg wrote:
> On Sat, 2016-08-13 at 17:42 +, Ben Hutchings wrote:
> > 
> > 3.2.82-rc1 review patch.  If anyone has any objections, please let me
> > know.
> > 
> > --
> > 
> > > > From: Prasun Maiti 
> > 
> > commit 3d5fdff46c4b2b9534fa2f9fc78e90a48e0ff724 upstream.
> > 
> 
> Did you just include this by accident? You had pointed out yourself
> that this was broken if anything but iwpoint was transferred, and since
> the Marvell people shouldn't be using wext anyway I reverted it
> already.

Yes, this was an accident and I'll drop it.

Ben.

-- 
Ben Hutchings
This sentence contradicts itself - no actually it doesn't.


signature.asc
Description: This is a digitally signed message part

Re: [PATCH 3.2 44/94] wext: Fix 32 bit iwpriv compatibility issue with 64 bit Kernel

[Ben Hutchings]

On Mon, 2016-08-15 at 07:51 +0200, Johannes Berg wrote:
> On Sat, 2016-08-13 at 17:42 +, Ben Hutchings wrote:
> > 
> > 3.2.82-rc1 review patch.  If anyone has any objections, please let me
> > know.
> > 
> > --
> > 
> > > > From: Prasun Maiti 
> > 
> > commit 3d5fdff46c4b2b9534fa2f9fc78e90a48e0ff724 upstream.
> > 
> 
> Did you just include this by accident? You had pointed out yourself
> that this was broken if anything but iwpoint was transferred, and since
> the Marvell people shouldn't be using wext anyway I reverted it
> already.

Yes, this was an accident and I'll drop it.

Ben.

-- 
Ben Hutchings
This sentence contradicts itself - no actually it doesn't.


signature.asc
Description: This is a digitally signed message part

[PATCH 07/11] perf symbols: Fix annotation of objects with debuginfo files

[Arnaldo Carvalho de Melo]

From: Anton Blanchard 

Commit 73cdf0c6ea9c ("perf symbols: Record text offset in dso
to calculate objdump address") started storing the offset of
the text section for all DSOs:

   if (elf_section_by_name(elf, , , ".text", NULL))
   dso->text_offset = tshdr.sh_addr - tshdr.sh_offset;

Unfortunately this breaks debuginfo files, because we need to calculate
the offset of the text section in the associated executable file. As a
result perf annotate returns junk for all debuginfo files.

Fix this by using runtime_ss->elf which should point at the executable
when parsing a debuginfo file.

Signed-off-by: Anton Blanchard 
Reviewed-by: Naveen N. Rao 
Tested-by: Wang Nan 
Cc: Peter Zijlstra 
Cc: Ravi Bangoria 
Cc: sta...@vger.kernel.org # v4.6+
Fixes: 73cdf0c6ea9c ("perf symbols: Record text offset in dso to calculate 
objdump address")
Link: http://lkml.kernel.org/r/20160813115533.6de17912@kryten
Signed-off-by: Arnaldo Carvalho de Melo 
---
 tools/perf/util/symbol-elf.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index a34321e9b44d..a811c13a74d6 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -837,7 +837,8 @@ int dso__load_sym(struct dso *dso, struct map *map,
sec = syms_ss->symtab;
shdr = syms_ss->symshdr;
 
-   if (elf_section_by_name(elf, , , ".text", NULL))
+   if (elf_section_by_name(runtime_ss->elf, _ss->ehdr, ,
+   ".text", NULL))
dso->text_offset = tshdr.sh_addr - tshdr.sh_offset;
 
if (runtime_ss->opdsec)
-- 
2.7.4

[PATCH 07/11] perf symbols: Fix annotation of objects with debuginfo files

[Arnaldo Carvalho de Melo]

From: Anton Blanchard 

Commit 73cdf0c6ea9c ("perf symbols: Record text offset in dso
to calculate objdump address") started storing the offset of
the text section for all DSOs:

   if (elf_section_by_name(elf, , , ".text", NULL))
   dso->text_offset = tshdr.sh_addr - tshdr.sh_offset;

Unfortunately this breaks debuginfo files, because we need to calculate
the offset of the text section in the associated executable file. As a
result perf annotate returns junk for all debuginfo files.

Fix this by using runtime_ss->elf which should point at the executable
when parsing a debuginfo file.

Signed-off-by: Anton Blanchard 
Reviewed-by: Naveen N. Rao 
Tested-by: Wang Nan 
Cc: Peter Zijlstra 
Cc: Ravi Bangoria 
Cc: sta...@vger.kernel.org # v4.6+
Fixes: 73cdf0c6ea9c ("perf symbols: Record text offset in dso to calculate 
objdump address")
Link: http://lkml.kernel.org/r/20160813115533.6de17912@kryten
Signed-off-by: Arnaldo Carvalho de Melo 
---
 tools/perf/util/symbol-elf.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index a34321e9b44d..a811c13a74d6 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -837,7 +837,8 @@ int dso__load_sym(struct dso *dso, struct map *map,
sec = syms_ss->symtab;
shdr = syms_ss->symshdr;
 
-   if (elf_section_by_name(elf, , , ".text", NULL))
+   if (elf_section_by_name(runtime_ss->elf, _ss->ehdr, ,
+   ".text", NULL))
dso->text_offset = tshdr.sh_addr - tshdr.sh_offset;
 
if (runtime_ss->opdsec)
-- 
2.7.4

[PATCH 04/11] perf jitdump: Add the right header to get the major()/minor() definitions

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo 

Noticed on Fedora Rawhide:

  $ gcc --version
  gcc (GCC) 6.1.1 20160721 (Red Hat 6.1.1-4)
  $ rpm -q glibc
  glibc-2.24.90-1.fc26.x86_64
  $

CC   /tmp/build/perf/util/jitdump.o
  util/jitdump.c: In function 'jit_repipe_code_load':
  util/jitdump.c:428:2: error: '__major_from_sys_types' is deprecated:
In the GNU C Library, `major' is defined by .
For historical compatibility, it is currently defined by
 as well, but we plan to remove this soon.
To use `major', include  directly.
If you did not intend to use a system-defined macro `major',
you should #undef it after including .
[-Werror=deprecated-declarations]
event->mmap2.maj   = major(st.st_dev);
^
  In file included from /usr/include/features.h:397:0,
   from /usr/include/sys/types.h:25,
   from util/jitdump.c:1:
  /usr/include/sys/sysmacros.h:87:1: note: declared here
   __SYSMACROS_DEFINE_MAJOR (__SYSMACROS_FST_IMPL_TEMPL)

Fix it following that recomendation.

Cc: Adrian Hunter 
Cc: David Ahern 
Cc: Jiri Olsa 
Cc: Namhyung Kim 
Cc: Stephane Eranian 
Cc: Wang Nan 
Link: http://lkml.kernel.org/n/tip-3majvd0adhfr25rvx4v5e...@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo 
---
 tools/perf/util/jitdump.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/perf/util/jitdump.c b/tools/perf/util/jitdump.c
index 9f3305f6b6d5..95f0884aae02 100644
--- a/tools/perf/util/jitdump.c
+++ b/tools/perf/util/jitdump.c
@@ -1,3 +1,4 @@
+#include 
 #include 
 #include 
 #include 
-- 
2.7.4

[PATCH 04/11] perf jitdump: Add the right header to get the major()/minor() definitions

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo 

Noticed on Fedora Rawhide:

  $ gcc --version
  gcc (GCC) 6.1.1 20160721 (Red Hat 6.1.1-4)
  $ rpm -q glibc
  glibc-2.24.90-1.fc26.x86_64
  $

CC   /tmp/build/perf/util/jitdump.o
  util/jitdump.c: In function 'jit_repipe_code_load':
  util/jitdump.c:428:2: error: '__major_from_sys_types' is deprecated:
In the GNU C Library, `major' is defined by .
For historical compatibility, it is currently defined by
 as well, but we plan to remove this soon.
To use `major', include  directly.
If you did not intend to use a system-defined macro `major',
you should #undef it after including .
[-Werror=deprecated-declarations]
event->mmap2.maj   = major(st.st_dev);
^
  In file included from /usr/include/features.h:397:0,
   from /usr/include/sys/types.h:25,
   from util/jitdump.c:1:
  /usr/include/sys/sysmacros.h:87:1: note: declared here
   __SYSMACROS_DEFINE_MAJOR (__SYSMACROS_FST_IMPL_TEMPL)

Fix it following that recomendation.

Cc: Adrian Hunter 
Cc: David Ahern 
Cc: Jiri Olsa 
Cc: Namhyung Kim 
Cc: Stephane Eranian 
Cc: Wang Nan 
Link: http://lkml.kernel.org/n/tip-3majvd0adhfr25rvx4v5e...@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo 
---
 tools/perf/util/jitdump.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/perf/util/jitdump.c b/tools/perf/util/jitdump.c
index 9f3305f6b6d5..95f0884aae02 100644
--- a/tools/perf/util/jitdump.c
+++ b/tools/perf/util/jitdump.c
@@ -1,3 +1,4 @@
+#include 
 #include 
 #include 
 #include 
-- 
2.7.4

[PATCH 02/11] perf tools mem: Fix -t store option for record command

[Arnaldo Carvalho de Melo]

From: Jiri Olsa 

Michael reported 'perf mem -t store record' being broken.  The reason is
latest rework of this area:

  commit acbe613e0c03 ("perf tools: Add monitored events array")

We don't mark perf_mem_events store record when -t store option is
specified.

Committer notes:

Before:

  # perf mem -t store record usleep 1
  [ perf record: Woken up 1 times to write data ]
  [ perf record: Captured and wrote 0.020 MB perf.data (7 samples) ]
  # perf evlist
  cycles:ppp
  #

After:

  # perf mem -t store record usleep 1
  [ perf record: Woken up 1 times to write data ]
  [ perf record: Captured and wrote 0.020 MB perf.data (7 samples) ]
  # perf evlist
  cpu/mem-stores/P
  #

Reported-by: Michael Petlan 
Signed-off-by: Jiri Olsa 
Tested-by: Arnaldo Carvalho de Melo 
Cc: David Ahern 
Cc: Namhyung Kim 
Cc: Peter Zijlstra 
Fixes: acbe613e0c03 ("perf tools: Add monitored events array")
Link: 
http://lkml.kernel.org/r/1470905457-18311-1-git-send-email-jo...@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo 
---
 tools/perf/builtin-mem.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/perf/builtin-mem.c b/tools/perf/builtin-mem.c
index d608a2c9e48c..d1ce29be560e 100644
--- a/tools/perf/builtin-mem.c
+++ b/tools/perf/builtin-mem.c
@@ -88,6 +88,9 @@ static int __cmd_record(int argc, const char **argv, struct 
perf_mem *mem)
if (mem->operation & MEM_OPERATION_LOAD)
perf_mem_events[PERF_MEM_EVENTS__LOAD].record = true;
 
+   if (mem->operation & MEM_OPERATION_STORE)
+   perf_mem_events[PERF_MEM_EVENTS__STORE].record = true;
+
if (perf_mem_events[PERF_MEM_EVENTS__LOAD].record)
rec_argv[i++] = "-W";
 
-- 
2.7.4

[PATCH 02/11] perf tools mem: Fix -t store option for record command

[Arnaldo Carvalho de Melo]

From: Jiri Olsa 

Michael reported 'perf mem -t store record' being broken.  The reason is
latest rework of this area:

  commit acbe613e0c03 ("perf tools: Add monitored events array")

We don't mark perf_mem_events store record when -t store option is
specified.

Committer notes:

Before:

  # perf mem -t store record usleep 1
  [ perf record: Woken up 1 times to write data ]
  [ perf record: Captured and wrote 0.020 MB perf.data (7 samples) ]
  # perf evlist
  cycles:ppp
  #

After:

  # perf mem -t store record usleep 1
  [ perf record: Woken up 1 times to write data ]
  [ perf record: Captured and wrote 0.020 MB perf.data (7 samples) ]
  # perf evlist
  cpu/mem-stores/P
  #

Reported-by: Michael Petlan 
Signed-off-by: Jiri Olsa 
Tested-by: Arnaldo Carvalho de Melo 
Cc: David Ahern 
Cc: Namhyung Kim 
Cc: Peter Zijlstra 
Fixes: acbe613e0c03 ("perf tools: Add monitored events array")
Link: 
http://lkml.kernel.org/r/1470905457-18311-1-git-send-email-jo...@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo 
---
 tools/perf/builtin-mem.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/perf/builtin-mem.c b/tools/perf/builtin-mem.c
index d608a2c9e48c..d1ce29be560e 100644
--- a/tools/perf/builtin-mem.c
+++ b/tools/perf/builtin-mem.c
@@ -88,6 +88,9 @@ static int __cmd_record(int argc, const char **argv, struct 
perf_mem *mem)
if (mem->operation & MEM_OPERATION_LOAD)
perf_mem_events[PERF_MEM_EVENTS__LOAD].record = true;
 
+   if (mem->operation & MEM_OPERATION_STORE)
+   perf_mem_events[PERF_MEM_EVENTS__STORE].record = true;
+
if (perf_mem_events[PERF_MEM_EVENTS__LOAD].record)
rec_argv[i++] = "-W";
 
-- 
2.7.4

[PATCH 11/11] perf intel-pt: Fix occasional decoding errors when tracing system-wide

[Arnaldo Carvalho de Melo]

From: Adrian Hunter 

In order to successfully decode Intel PT traces, context switch events
are needed from the moment the trace starts. Currently that is ensured
by using the 'immediate' flag which enables the switch event when it is
opened.

However, since commit 86c2786994bd ("perf intel-pt: Add support for
PERF_RECORD_SWITCH") that might not always happen. When tracing
system-wide the context switch event is added to the tracking event
which was not set as 'immediate'. Change that so it is.

Signed-off-by: Adrian Hunter 
Cc: Jiri Olsa 
Cc: sta...@vger.kernel.org # v4.4+
Fixes: 86c2786994bd ("perf intel-pt: Add support for PERF_RECORD_SWITCH")
Link: 
http://lkml.kernel.org/r/1471245784-22580-1-git-send-email-adrian.hun...@intel.com
Signed-off-by: Arnaldo Carvalho de Melo 
---
 tools/perf/arch/x86/util/intel-pt.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/tools/perf/arch/x86/util/intel-pt.c 
b/tools/perf/arch/x86/util/intel-pt.c
index fb51457ba338..a2412e9d883b 100644
--- a/tools/perf/arch/x86/util/intel-pt.c
+++ b/tools/perf/arch/x86/util/intel-pt.c
@@ -501,7 +501,7 @@ static int intel_pt_recording_options(struct 
auxtrace_record *itr,
struct intel_pt_recording *ptr =
container_of(itr, struct intel_pt_recording, itr);
struct perf_pmu *intel_pt_pmu = ptr->intel_pt_pmu;
-   bool have_timing_info;
+   bool have_timing_info, need_immediate = false;
struct perf_evsel *evsel, *intel_pt_evsel = NULL;
const struct cpu_map *cpus = evlist->cpus;
bool privileged = geteuid() == 0 || perf_event_paranoid() < 0;
@@ -655,6 +655,7 @@ static int intel_pt_recording_options(struct 
auxtrace_record *itr,
ptr->have_sched_switch = 3;
} else {
opts->record_switch_events = true;
+   need_immediate = true;
if (cpu_wide)
ptr->have_sched_switch = 3;
else
@@ -700,6 +701,9 @@ static int intel_pt_recording_options(struct 
auxtrace_record *itr,
tracking_evsel->attr.freq = 0;
tracking_evsel->attr.sample_period = 1;
 
+   if (need_immediate)
+   tracking_evsel->immediate = true;
+
/* In per-cpu case, always need the time of mmap events etc */
if (!cpu_map__empty(cpus)) {
perf_evsel__set_sample_bit(tracking_evsel, TIME);
-- 
2.7.4

[PATCH 11/11] perf intel-pt: Fix occasional decoding errors when tracing system-wide

[Arnaldo Carvalho de Melo]

From: Adrian Hunter 

In order to successfully decode Intel PT traces, context switch events
are needed from the moment the trace starts. Currently that is ensured
by using the 'immediate' flag which enables the switch event when it is
opened.

However, since commit 86c2786994bd ("perf intel-pt: Add support for
PERF_RECORD_SWITCH") that might not always happen. When tracing
system-wide the context switch event is added to the tracking event
which was not set as 'immediate'. Change that so it is.

Signed-off-by: Adrian Hunter 
Cc: Jiri Olsa 
Cc: sta...@vger.kernel.org # v4.4+
Fixes: 86c2786994bd ("perf intel-pt: Add support for PERF_RECORD_SWITCH")
Link: 
http://lkml.kernel.org/r/1471245784-22580-1-git-send-email-adrian.hun...@intel.com
Signed-off-by: Arnaldo Carvalho de Melo 
---
 tools/perf/arch/x86/util/intel-pt.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/tools/perf/arch/x86/util/intel-pt.c 
b/tools/perf/arch/x86/util/intel-pt.c
index fb51457ba338..a2412e9d883b 100644
--- a/tools/perf/arch/x86/util/intel-pt.c
+++ b/tools/perf/arch/x86/util/intel-pt.c
@@ -501,7 +501,7 @@ static int intel_pt_recording_options(struct 
auxtrace_record *itr,
struct intel_pt_recording *ptr =
container_of(itr, struct intel_pt_recording, itr);
struct perf_pmu *intel_pt_pmu = ptr->intel_pt_pmu;
-   bool have_timing_info;
+   bool have_timing_info, need_immediate = false;
struct perf_evsel *evsel, *intel_pt_evsel = NULL;
const struct cpu_map *cpus = evlist->cpus;
bool privileged = geteuid() == 0 || perf_event_paranoid() < 0;
@@ -655,6 +655,7 @@ static int intel_pt_recording_options(struct 
auxtrace_record *itr,
ptr->have_sched_switch = 3;
} else {
opts->record_switch_events = true;
+   need_immediate = true;
if (cpu_wide)
ptr->have_sched_switch = 3;
else
@@ -700,6 +701,9 @@ static int intel_pt_recording_options(struct 
auxtrace_record *itr,
tracking_evsel->attr.freq = 0;
tracking_evsel->attr.sample_period = 1;
 
+   if (need_immediate)
+   tracking_evsel->immediate = true;
+
/* In per-cpu case, always need the time of mmap events etc */
if (!cpu_map__empty(cpus)) {
perf_evsel__set_sample_bit(tracking_evsel, TIME);
-- 
2.7.4

[PATCH 03/11] perf ppc64le: Fix build failure when libelf is not present

[Arnaldo Carvalho de Melo]

From: Ravi Bangoria 

arch__post_process_probe_trace_events() calls get_target_map() to
prepare symbol table. get_target_map() is defined inside
util/probe-event.c.

probe-event.c will only get included in perf binary if CONFIG_LIBELF is
set.  Hence arch__post_process_probe_trace_events() needs to be defined
inside #ifdef HAVE_LIBELF_SUPPORT to solve compilation error.

Reported-and-Tested-by: Anton Blanchard 
Tested-by: Arnaldo Carvalho de Melo 
Cc: Peter Zijlstra 
Cc: Alexander Shishkin 
Cc: Balbir Singh 
Cc: Naveen N. Rao 
Cc: Ananth N Mavinakayanahalli 
Cc: Masami Hiramatsu 
Cc: Wang Nan 
Cc: Namhyung Kim 
Link: http://lkml.kernel.org/r/57abff88.8030...@linux.vnet.ibm.com
[ Thunderbird MUA mangled it, fix that ]
Signed-off-by: Arnaldo Carvalho de Melo 
---
 tools/perf/arch/powerpc/util/sym-handling.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/perf/arch/powerpc/util/sym-handling.c 
b/tools/perf/arch/powerpc/util/sym-handling.c
index 8d4dc97d80ba..35745a733100 100644
--- a/tools/perf/arch/powerpc/util/sym-handling.c
+++ b/tools/perf/arch/powerpc/util/sym-handling.c
@@ -97,6 +97,7 @@ void arch__fix_tev_from_maps(struct perf_probe_event *pev,
}
 }
 
+#ifdef HAVE_LIBELF_SUPPORT
 void arch__post_process_probe_trace_events(struct perf_probe_event *pev,
   int ntevs)
 {
@@ -118,5 +119,6 @@ void arch__post_process_probe_trace_events(struct 
perf_probe_event *pev,
}
}
 }
+#endif /* HAVE_LIBELF_SUPPORT */
 
 #endif
-- 
2.7.4

[PATCH 10/11] tools: Sync kvm related header files for arm64 and s390

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo 

>From a quick look nothing stands out as requiring changes to kvm tools
such as tools/perf/arch/s390/util/kvm-stat.c.

Silences these header checking warnings:

  $ make -C tools/perf
  make: Entering directory '/home/acme/git/linux/tools/perf'
BUILD:   Doing 'make -j4' parallel build
  Warning: tools/arch/s390/include/uapi/asm/kvm.h differs from kernel
  Warning: tools/arch/s390/include/uapi/asm/sie.h differs from kernel
  Warning: tools/arch/arm64/include/uapi/asm/kvm.h differs from kernel
  

Cc: Adrian Hunter 
Cc: Alexander Yarygin 
Cc: David Ahern 
Cc: Hemant Kumar 
Cc: Jiri Olsa 
Cc: Michael Ellerman 
Cc: Namhyung Kim 
Cc: Naveen N. Rao 
Cc: Scott  Wood 
Cc: Srikar Dronamraju 
Cc: Wang Nan 
Link: http://lkml.kernel.org/n/tip-btutge414g516qmh6r5ie...@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo 
---
 tools/arch/arm64/include/uapi/asm/kvm.h |  2 ++
 tools/arch/s390/include/uapi/asm/kvm.h  | 41 +
 tools/arch/s390/include/uapi/asm/sie.h  |  1 +
 3 files changed, 44 insertions(+)

diff --git a/tools/arch/arm64/include/uapi/asm/kvm.h 
b/tools/arch/arm64/include/uapi/asm/kvm.h
index f209ea151dca..3051f86a9b5f 100644
--- a/tools/arch/arm64/include/uapi/asm/kvm.h
+++ b/tools/arch/arm64/include/uapi/asm/kvm.h
@@ -87,9 +87,11 @@ struct kvm_regs {
 /* Supported VGICv3 address types  */
 #define KVM_VGIC_V3_ADDR_TYPE_DIST 2
 #define KVM_VGIC_V3_ADDR_TYPE_REDIST   3
+#define KVM_VGIC_ITS_ADDR_TYPE 4
 
 #define KVM_VGIC_V3_DIST_SIZE  SZ_64K
 #define KVM_VGIC_V3_REDIST_SIZE(2 * SZ_64K)
+#define KVM_VGIC_V3_ITS_SIZE   (2 * SZ_64K)
 
 #define KVM_ARM_VCPU_POWER_OFF 0 /* CPU is started in OFF state */
 #define KVM_ARM_VCPU_EL1_32BIT 1 /* CPU running a 32bit VM */
diff --git a/tools/arch/s390/include/uapi/asm/kvm.h 
b/tools/arch/s390/include/uapi/asm/kvm.h
index 3b8e99ef9d58..a2ffec4139ad 100644
--- a/tools/arch/s390/include/uapi/asm/kvm.h
+++ b/tools/arch/s390/include/uapi/asm/kvm.h
@@ -93,6 +93,47 @@ struct kvm_s390_vm_cpu_machine {
__u64 fac_list[256];
 };
 
+#define KVM_S390_VM_CPU_PROCESSOR_FEAT 2
+#define KVM_S390_VM_CPU_MACHINE_FEAT   3
+
+#define KVM_S390_VM_CPU_FEAT_NR_BITS   1024
+#define KVM_S390_VM_CPU_FEAT_ESOP  0
+#define KVM_S390_VM_CPU_FEAT_SIEF2 1
+#define KVM_S390_VM_CPU_FEAT_64BSCAO   2
+#define KVM_S390_VM_CPU_FEAT_SIIF  3
+#define KVM_S390_VM_CPU_FEAT_GPERE 4
+#define KVM_S390_VM_CPU_FEAT_GSLS  5
+#define KVM_S390_VM_CPU_FEAT_IB6
+#define KVM_S390_VM_CPU_FEAT_CEI   7
+#define KVM_S390_VM_CPU_FEAT_IBS   8
+#define KVM_S390_VM_CPU_FEAT_SKEY  9
+#define KVM_S390_VM_CPU_FEAT_CMMA  10
+#define KVM_S390_VM_CPU_FEAT_PFMFI 11
+#define KVM_S390_VM_CPU_FEAT_SIGPIF12
+struct kvm_s390_vm_cpu_feat {
+   __u64 feat[16];
+};
+
+#define KVM_S390_VM_CPU_PROCESSOR_SUBFUNC  4
+#define KVM_S390_VM_CPU_MACHINE_SUBFUNC5
+/* for "test bit" instructions MSB 0 bit ordering, for "query" raw blocks */
+struct kvm_s390_vm_cpu_subfunc {
+   __u8 plo[32];   /* always */
+   __u8 ptff[16];  /* with TOD-clock steering */
+   __u8 kmac[16];  /* with MSA */
+   __u8 kmc[16];   /* with MSA */
+   __u8 km[16];/* with MSA */
+   __u8 kimd[16];  /* with MSA */
+   __u8 klmd[16];  /* with MSA */
+   __u8 pckmo[16]; /* with MSA3 */
+   __u8 kmctr[16]; /* with MSA4 */
+   __u8 kmf[16];   /* with MSA4 */
+   __u8 kmo[16];   /* with MSA4 */
+   __u8 pcc[16];   /* with MSA4 */
+   __u8 ppno[16];  /* with MSA5 */
+   __u8 reserved[1824];
+};
+
 /* kvm attributes for crypto */
 #define KVM_S390_VM_CRYPTO_ENABLE_AES_KW   0
 #define KVM_S390_VM_CRYPTO_ENABLE_DEA_KW   1
diff --git a/tools/arch/s390/include/uapi/asm/sie.h 
b/tools/arch/s390/include/uapi/asm/sie.h
index 8fb5d4a6dd25..3ac634368939 100644
--- a/tools/arch/s390/include/uapi/asm/sie.h
+++ b/tools/arch/s390/include/uapi/asm/sie.h
@@ -140,6 +140,7 @@
exit_code_ipa0(0xB2, 0x4c, "TAR"),  \
exit_code_ipa0(0xB2, 0x50, "CSP"),  \
exit_code_ipa0(0xB2, 0x54, "MVPG"), \
+   exit_code_ipa0(0xB2, 0x56, "STHYI"),\
exit_code_ipa0(0xB2, 0x58, "BSG"),  \
exit_code_ipa0(0xB2, 0x5a, "BSA"),  \
exit_code_ipa0(0xB2, 0x5f, "CHSC"), \
-- 
2.7.4

[PATCH 08/11] perf probe: Check for dup and fdopen failures

[Arnaldo Carvalho de Melo]

From: Colin Ian King 

dup and fdopen can potentially fail, so add some extra
error handling checks rather than assuming they always work.

Signed-off-by: Colin King 
Acked-by: Masami Hiramatsu 
Cc: Alexander Shishkin 
Cc: Jiri Olsa 
Cc: Namhyung Kim 
Cc: Peter Zijlstra 
Cc: Wang Nan 
Link: 
http://lkml.kernel.org/r/1471038296-12956-1-git-send-email-colin.k...@canonical.com
[ Free resources when those functions (now being verified) fail ]
Signed-off-by: Arnaldo Carvalho de Melo 
---
 tools/perf/util/probe-file.c | 24 
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c
index 9aed9c332da6..a8e76233c088 100644
--- a/tools/perf/util/probe-file.c
+++ b/tools/perf/util/probe-file.c
@@ -133,7 +133,7 @@ int probe_file__open_both(int *kfd, int *ufd, int flag)
 /* Get raw string list of current kprobe_events  or uprobe_events */
 struct strlist *probe_file__get_rawlist(int fd)
 {
-   int ret, idx;
+   int ret, idx, fddup;
FILE *fp;
char buf[MAX_CMDLEN];
char *p;
@@ -144,7 +144,14 @@ struct strlist *probe_file__get_rawlist(int fd)
 
sl = strlist__new(NULL, NULL);
 
-   fp = fdopen(dup(fd), "r");
+   fddup = dup(fd);
+   if (fddup < 0)
+   goto out_free_sl;
+
+   fp = fdopen(fddup, "r");
+   if (!fp)
+   goto out_close_fddup;
+
while (!feof(fp)) {
p = fgets(buf, MAX_CMDLEN, fp);
if (!p)
@@ -163,6 +170,12 @@ struct strlist *probe_file__get_rawlist(int fd)
fclose(fp);
 
return sl;
+
+out_close_fddup:
+   close(fddup);
+out_free_sl:
+   strlist__delete(sl);
+   return NULL;
 }
 
 static struct strlist *__probe_file__get_namelist(int fd, bool include_group)
@@ -447,10 +460,13 @@ static int probe_cache__load(struct probe_cache *pcache)
 {
struct probe_cache_entry *entry = NULL;
char buf[MAX_CMDLEN], *p;
-   int ret = 0;
+   int ret = 0, fddup;
FILE *fp;
 
-   fp = fdopen(dup(pcache->fd), "r");
+   fddup = dup(pcache->fd);
+   if (fddup < 0)
+   return -errno;
+   fp = fdopen(fddup, "r");
if (!fp)
return -EINVAL;
 
-- 
2.7.4

[PATCH 03/11] perf ppc64le: Fix build failure when libelf is not present

[Arnaldo Carvalho de Melo]

From: Ravi Bangoria 

arch__post_process_probe_trace_events() calls get_target_map() to
prepare symbol table. get_target_map() is defined inside
util/probe-event.c.

probe-event.c will only get included in perf binary if CONFIG_LIBELF is
set.  Hence arch__post_process_probe_trace_events() needs to be defined
inside #ifdef HAVE_LIBELF_SUPPORT to solve compilation error.

Reported-and-Tested-by: Anton Blanchard 
Tested-by: Arnaldo Carvalho de Melo 
Cc: Peter Zijlstra 
Cc: Alexander Shishkin 
Cc: Balbir Singh 
Cc: Naveen N. Rao 
Cc: Ananth N Mavinakayanahalli 
Cc: Masami Hiramatsu 
Cc: Wang Nan 
Cc: Namhyung Kim 
Link: http://lkml.kernel.org/r/57abff88.8030...@linux.vnet.ibm.com
[ Thunderbird MUA mangled it, fix that ]
Signed-off-by: Arnaldo Carvalho de Melo 
---
 tools/perf/arch/powerpc/util/sym-handling.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/perf/arch/powerpc/util/sym-handling.c 
b/tools/perf/arch/powerpc/util/sym-handling.c
index 8d4dc97d80ba..35745a733100 100644
--- a/tools/perf/arch/powerpc/util/sym-handling.c
+++ b/tools/perf/arch/powerpc/util/sym-handling.c
@@ -97,6 +97,7 @@ void arch__fix_tev_from_maps(struct perf_probe_event *pev,
}
 }
 
+#ifdef HAVE_LIBELF_SUPPORT
 void arch__post_process_probe_trace_events(struct perf_probe_event *pev,
   int ntevs)
 {
@@ -118,5 +119,6 @@ void arch__post_process_probe_trace_events(struct 
perf_probe_event *pev,
}
}
 }
+#endif /* HAVE_LIBELF_SUPPORT */
 
 #endif
-- 
2.7.4

[PATCH 10/11] tools: Sync kvm related header files for arm64 and s390

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo 

>From a quick look nothing stands out as requiring changes to kvm tools
such as tools/perf/arch/s390/util/kvm-stat.c.

Silences these header checking warnings:

  $ make -C tools/perf
  make: Entering directory '/home/acme/git/linux/tools/perf'
BUILD:   Doing 'make -j4' parallel build
  Warning: tools/arch/s390/include/uapi/asm/kvm.h differs from kernel
  Warning: tools/arch/s390/include/uapi/asm/sie.h differs from kernel
  Warning: tools/arch/arm64/include/uapi/asm/kvm.h differs from kernel
  

Cc: Adrian Hunter 
Cc: Alexander Yarygin 
Cc: David Ahern 
Cc: Hemant Kumar 
Cc: Jiri Olsa 
Cc: Michael Ellerman 
Cc: Namhyung Kim 
Cc: Naveen N. Rao 
Cc: Scott  Wood 
Cc: Srikar Dronamraju 
Cc: Wang Nan 
Link: http://lkml.kernel.org/n/tip-btutge414g516qmh6r5ie...@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo 
---
 tools/arch/arm64/include/uapi/asm/kvm.h |  2 ++
 tools/arch/s390/include/uapi/asm/kvm.h  | 41 +
 tools/arch/s390/include/uapi/asm/sie.h  |  1 +
 3 files changed, 44 insertions(+)

diff --git a/tools/arch/arm64/include/uapi/asm/kvm.h 
b/tools/arch/arm64/include/uapi/asm/kvm.h
index f209ea151dca..3051f86a9b5f 100644
--- a/tools/arch/arm64/include/uapi/asm/kvm.h
+++ b/tools/arch/arm64/include/uapi/asm/kvm.h
@@ -87,9 +87,11 @@ struct kvm_regs {
 /* Supported VGICv3 address types  */
 #define KVM_VGIC_V3_ADDR_TYPE_DIST 2
 #define KVM_VGIC_V3_ADDR_TYPE_REDIST   3
+#define KVM_VGIC_ITS_ADDR_TYPE 4
 
 #define KVM_VGIC_V3_DIST_SIZE  SZ_64K
 #define KVM_VGIC_V3_REDIST_SIZE(2 * SZ_64K)
+#define KVM_VGIC_V3_ITS_SIZE   (2 * SZ_64K)
 
 #define KVM_ARM_VCPU_POWER_OFF 0 /* CPU is started in OFF state */
 #define KVM_ARM_VCPU_EL1_32BIT 1 /* CPU running a 32bit VM */
diff --git a/tools/arch/s390/include/uapi/asm/kvm.h 
b/tools/arch/s390/include/uapi/asm/kvm.h
index 3b8e99ef9d58..a2ffec4139ad 100644
--- a/tools/arch/s390/include/uapi/asm/kvm.h
+++ b/tools/arch/s390/include/uapi/asm/kvm.h
@@ -93,6 +93,47 @@ struct kvm_s390_vm_cpu_machine {
__u64 fac_list[256];
 };
 
+#define KVM_S390_VM_CPU_PROCESSOR_FEAT 2
+#define KVM_S390_VM_CPU_MACHINE_FEAT   3
+
+#define KVM_S390_VM_CPU_FEAT_NR_BITS   1024
+#define KVM_S390_VM_CPU_FEAT_ESOP  0
+#define KVM_S390_VM_CPU_FEAT_SIEF2 1
+#define KVM_S390_VM_CPU_FEAT_64BSCAO   2
+#define KVM_S390_VM_CPU_FEAT_SIIF  3
+#define KVM_S390_VM_CPU_FEAT_GPERE 4
+#define KVM_S390_VM_CPU_FEAT_GSLS  5
+#define KVM_S390_VM_CPU_FEAT_IB6
+#define KVM_S390_VM_CPU_FEAT_CEI   7
+#define KVM_S390_VM_CPU_FEAT_IBS   8
+#define KVM_S390_VM_CPU_FEAT_SKEY  9
+#define KVM_S390_VM_CPU_FEAT_CMMA  10
+#define KVM_S390_VM_CPU_FEAT_PFMFI 11
+#define KVM_S390_VM_CPU_FEAT_SIGPIF12
+struct kvm_s390_vm_cpu_feat {
+   __u64 feat[16];
+};
+
+#define KVM_S390_VM_CPU_PROCESSOR_SUBFUNC  4
+#define KVM_S390_VM_CPU_MACHINE_SUBFUNC5
+/* for "test bit" instructions MSB 0 bit ordering, for "query" raw blocks */
+struct kvm_s390_vm_cpu_subfunc {
+   __u8 plo[32];   /* always */
+   __u8 ptff[16];  /* with TOD-clock steering */
+   __u8 kmac[16];  /* with MSA */
+   __u8 kmc[16];   /* with MSA */
+   __u8 km[16];/* with MSA */
+   __u8 kimd[16];  /* with MSA */
+   __u8 klmd[16];  /* with MSA */
+   __u8 pckmo[16]; /* with MSA3 */
+   __u8 kmctr[16]; /* with MSA4 */
+   __u8 kmf[16];   /* with MSA4 */
+   __u8 kmo[16];   /* with MSA4 */
+   __u8 pcc[16];   /* with MSA4 */
+   __u8 ppno[16];  /* with MSA5 */
+   __u8 reserved[1824];
+};
+
 /* kvm attributes for crypto */
 #define KVM_S390_VM_CRYPTO_ENABLE_AES_KW   0
 #define KVM_S390_VM_CRYPTO_ENABLE_DEA_KW   1
diff --git a/tools/arch/s390/include/uapi/asm/sie.h 
b/tools/arch/s390/include/uapi/asm/sie.h
index 8fb5d4a6dd25..3ac634368939 100644
--- a/tools/arch/s390/include/uapi/asm/sie.h
+++ b/tools/arch/s390/include/uapi/asm/sie.h
@@ -140,6 +140,7 @@
exit_code_ipa0(0xB2, 0x4c, "TAR"),  \
exit_code_ipa0(0xB2, 0x50, "CSP"),  \
exit_code_ipa0(0xB2, 0x54, "MVPG"), \
+   exit_code_ipa0(0xB2, 0x56, "STHYI"),\
exit_code_ipa0(0xB2, 0x58, "BSG"),  \
exit_code_ipa0(0xB2, 0x5a, "BSA"),  \
exit_code_ipa0(0xB2, 0x5f, "CHSC"), \
-- 
2.7.4

[PATCH 08/11] perf probe: Check for dup and fdopen failures

[Arnaldo Carvalho de Melo]

From: Colin Ian King 

dup and fdopen can potentially fail, so add some extra
error handling checks rather than assuming they always work.

Signed-off-by: Colin King 
Acked-by: Masami Hiramatsu 
Cc: Alexander Shishkin 
Cc: Jiri Olsa 
Cc: Namhyung Kim 
Cc: Peter Zijlstra 
Cc: Wang Nan 
Link: 
http://lkml.kernel.org/r/1471038296-12956-1-git-send-email-colin.k...@canonical.com
[ Free resources when those functions (now being verified) fail ]
Signed-off-by: Arnaldo Carvalho de Melo 
---
 tools/perf/util/probe-file.c | 24 
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c
index 9aed9c332da6..a8e76233c088 100644
--- a/tools/perf/util/probe-file.c
+++ b/tools/perf/util/probe-file.c
@@ -133,7 +133,7 @@ int probe_file__open_both(int *kfd, int *ufd, int flag)
 /* Get raw string list of current kprobe_events  or uprobe_events */
 struct strlist *probe_file__get_rawlist(int fd)
 {
-   int ret, idx;
+   int ret, idx, fddup;
FILE *fp;
char buf[MAX_CMDLEN];
char *p;
@@ -144,7 +144,14 @@ struct strlist *probe_file__get_rawlist(int fd)
 
sl = strlist__new(NULL, NULL);
 
-   fp = fdopen(dup(fd), "r");
+   fddup = dup(fd);
+   if (fddup < 0)
+   goto out_free_sl;
+
+   fp = fdopen(fddup, "r");
+   if (!fp)
+   goto out_close_fddup;
+
while (!feof(fp)) {
p = fgets(buf, MAX_CMDLEN, fp);
if (!p)
@@ -163,6 +170,12 @@ struct strlist *probe_file__get_rawlist(int fd)
fclose(fp);
 
return sl;
+
+out_close_fddup:
+   close(fddup);
+out_free_sl:
+   strlist__delete(sl);
+   return NULL;
 }
 
 static struct strlist *__probe_file__get_namelist(int fd, bool include_group)
@@ -447,10 +460,13 @@ static int probe_cache__load(struct probe_cache *pcache)
 {
struct probe_cache_entry *entry = NULL;
char buf[MAX_CMDLEN], *p;
-   int ret = 0;
+   int ret = 0, fddup;
FILE *fp;
 
-   fp = fdopen(dup(pcache->fd), "r");
+   fddup = dup(pcache->fd);
+   if (fddup < 0)
+   return -errno;
+   fp = fdopen(fddup, "r");
if (!fp)
return -EINVAL;
 
-- 
2.7.4

[PATCH 05/11] perf script: Show proper message when failed list scripts

[Arnaldo Carvalho de Melo]

From: He Kuang 

Perf shows the usage message when perf scripts folder failed to open,
which misleads users to let them think the command is being mistyped.

This patch shows a proper message and guides users to check the
PERF_EXEC_PATH environment variable in that case.

Before:

  $ perf script --list

  Usage: perf script []
   or: perf script [] record 

[PATCH 06/11] perf script: Don't disable use_callchain if input is pipe

[Arnaldo Carvalho de Melo]

From: He Kuang 

Because perf data from pipe do not have a header with evsel attr, we
should not check that and disable symbol_conf.use_callchain. Otherwise,
perf script won't show callchains even if the data stream contains
callchain.

Before:
  $ perf record -g -o - uname |perf script
  Linux
  [ perf record: Woken up 1 times to write data ]
  [ perf record: Captured and wrote 0.000 MB - ]
  uname  1828 182630.186578:  25 cpu-clock:  ..b9499 setup_arg_pages
  uname  1828 182630.186850:  25 cpu-clock:  ..83b20 ___might_sleep
  uname  1828 182630.187153:  25 cpu-clock:  ..4b6be file_map_prot_ch
  ...

After:
  $ perf record -g -o - uname |perf script
  Linux
  [ perf record: Woken up 1 times to write data ]
  [ perf record: Captured and wrote 0.000 MB - ]
  uname  1833 182675.927099: 25 cpu-clock:
  ba5520 _raw_spin_lock+0xfe200040 ([kernel.kallsyms])
  389dd4 expand_downwards+0xfe200154 ([kernel.kallsyms])
  389f34 expand_stack+0xfe200024 ([kernel.kallsyms])
  3b957e setup_arg_pages+0xfe20019e ([kernel.kallsyms])
  40c80f load_elf_binary+0xfe20042f ([kernel.kallsyms])
  ...

Signed-off-by: He Kuang 
Tested-by: Arnaldo Carvalho de Melo 
Cc: Alexander Shishkin 
Cc: Peter Zijlstra 
Cc: Wang Nan 
Link: 
http://lkml.kernel.org/r/1470309943-153909-2-git-send-email-heku...@huawei.com
Signed-off-by: Arnaldo Carvalho de Melo 
---
 tools/perf/builtin-script.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index 45c51eb6cab4..c859e59dfe3e 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -371,14 +371,16 @@ static int perf_session__check_output_opt(struct 
perf_session *session)
 
if (!no_callchain) {
bool use_callchain = false;
+   bool not_pipe = false;
 
evlist__for_each_entry(session->evlist, evsel) {
+   not_pipe = true;
if (evsel->attr.sample_type & PERF_SAMPLE_CALLCHAIN) {
use_callchain = true;
break;
}
}
-   if (!use_callchain)
+   if (not_pipe && !use_callchain)
symbol_conf.use_callchain = false;
}
 
-- 
2.7.4

[PATCH 05/11] perf script: Show proper message when failed list scripts

[Arnaldo Carvalho de Melo]

From: He Kuang 

Perf shows the usage message when perf scripts folder failed to open,
which misleads users to let them think the command is being mistyped.

This patch shows a proper message and guides users to check the
PERF_EXEC_PATH environment variable in that case.

Before:

  $ perf script --list

  Usage: perf script []
   or: perf script [] record 

[PATCH 06/11] perf script: Don't disable use_callchain if input is pipe

[Arnaldo Carvalho de Melo]

From: He Kuang 

Because perf data from pipe do not have a header with evsel attr, we
should not check that and disable symbol_conf.use_callchain. Otherwise,
perf script won't show callchains even if the data stream contains
callchain.

Before:
  $ perf record -g -o - uname |perf script
  Linux
  [ perf record: Woken up 1 times to write data ]
  [ perf record: Captured and wrote 0.000 MB - ]
  uname  1828 182630.186578:  25 cpu-clock:  ..b9499 setup_arg_pages
  uname  1828 182630.186850:  25 cpu-clock:  ..83b20 ___might_sleep
  uname  1828 182630.187153:  25 cpu-clock:  ..4b6be file_map_prot_ch
  ...

After:
  $ perf record -g -o - uname |perf script
  Linux
  [ perf record: Woken up 1 times to write data ]
  [ perf record: Captured and wrote 0.000 MB - ]
  uname  1833 182675.927099: 25 cpu-clock:
  ba5520 _raw_spin_lock+0xfe200040 ([kernel.kallsyms])
  389dd4 expand_downwards+0xfe200154 ([kernel.kallsyms])
  389f34 expand_stack+0xfe200024 ([kernel.kallsyms])
  3b957e setup_arg_pages+0xfe20019e ([kernel.kallsyms])
  40c80f load_elf_binary+0xfe20042f ([kernel.kallsyms])
  ...

Signed-off-by: He Kuang 
Tested-by: Arnaldo Carvalho de Melo 
Cc: Alexander Shishkin 
Cc: Peter Zijlstra 
Cc: Wang Nan 
Link: 
http://lkml.kernel.org/r/1470309943-153909-2-git-send-email-heku...@huawei.com
Signed-off-by: Arnaldo Carvalho de Melo 
---
 tools/perf/builtin-script.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index 45c51eb6cab4..c859e59dfe3e 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -371,14 +371,16 @@ static int perf_session__check_output_opt(struct 
perf_session *session)
 
if (!no_callchain) {
bool use_callchain = false;
+   bool not_pipe = false;
 
evlist__for_each_entry(session->evlist, evsel) {
+   not_pipe = true;
if (evsel->attr.sample_type & PERF_SAMPLE_CALLCHAIN) {
use_callchain = true;
break;
}
}
-   if (!use_callchain)
+   if (not_pipe && !use_callchain)
symbol_conf.use_callchain = false;
}
 
-- 
2.7.4

[PATCH 01/11] perf intel-pt: Fix ip compression

[Arnaldo Carvalho de Melo]

From: Adrian Hunter 

The June 2015 Intel SDM introduced IP Compression types 4 and 6. Refer
to section 36.4.2.2 Target IP (TIP) Packet - IP Compression.

Existing Intel PT packet decoder did not support type 4, and got type 6
wrong.  Because type 3 and type 4 have the same number of bytes, the
packet 'count' has been changed from being the number of ip bytes to
being the type code.  That allows the Intel PT decoder to correctly
decide whether to sign-extend or use the last ip.  However that also
meant the code had to be adjusted in a number of places.

Currently hardware is not using the new compression types, so this fix
has no effect on existing hardware.

Signed-off-by: Adrian Hunter 
Cc: Jiri Olsa 
Link: 
http://lkml.kernel.org/r/1469005206-3049-1-git-send-email-adrian.hun...@intel.com
Signed-off-by: Arnaldo Carvalho de Melo 
---
 .../perf/util/intel-pt-decoder/intel-pt-decoder.c  | 44 +++---
 .../util/intel-pt-decoder/intel-pt-pkt-decoder.c   | 24 
 2 files changed, 40 insertions(+), 28 deletions(-)

diff --git a/tools/perf/util/intel-pt-decoder/intel-pt-decoder.c 
b/tools/perf/util/intel-pt-decoder/intel-pt-decoder.c
index 9c8f15da86ce..8ff6c6a61291 100644
--- a/tools/perf/util/intel-pt-decoder/intel-pt-decoder.c
+++ b/tools/perf/util/intel-pt-decoder/intel-pt-decoder.c
@@ -123,8 +123,6 @@ struct intel_pt_decoder {
bool have_calc_cyc_to_tsc;
int exec_mode;
unsigned int insn_bytes;
-   uint64_t sign_bit;
-   uint64_t sign_bits;
uint64_t period;
enum intel_pt_period_type period_type;
uint64_t tot_insn_cnt;
@@ -191,9 +189,6 @@ struct intel_pt_decoder *intel_pt_decoder_new(struct 
intel_pt_params *params)
decoder->data   = params->data;
decoder->return_compression = params->return_compression;
 
-   decoder->sign_bit   = (uint64_t)1 << 47;
-   decoder->sign_bits  = ~(((uint64_t)1 << 48) - 1);
-
decoder->period = params->period;
decoder->period_type= params->period_type;
 
@@ -362,21 +357,30 @@ int intel_pt__strerror(int code, char *buf, size_t buflen)
return 0;
 }
 
-static uint64_t intel_pt_calc_ip(struct intel_pt_decoder *decoder,
-const struct intel_pt_pkt *packet,
+static uint64_t intel_pt_calc_ip(const struct intel_pt_pkt *packet,
 uint64_t last_ip)
 {
uint64_t ip;
 
switch (packet->count) {
-   case 2:
+   case 1:
ip = (last_ip & (uint64_t)0xULL) |
 packet->payload;
break;
-   case 4:
+   case 2:
ip = (last_ip & (uint64_t)0xULL) |
 packet->payload;
break;
+   case 3:
+   ip = packet->payload;
+   /* Sign-extend 6-byte ip */
+   if (ip & (uint64_t)0x8000ULL)
+   ip |= (uint64_t)0xULL;
+   break;
+   case 4:
+   ip = (last_ip & (uint64_t)0xULL) |
+packet->payload;
+   break;
case 6:
ip = packet->payload;
break;
@@ -384,16 +388,12 @@ static uint64_t intel_pt_calc_ip(struct intel_pt_decoder 
*decoder,
return 0;
}
 
-   if (ip & decoder->sign_bit)
-   return ip | decoder->sign_bits;
-
return ip;
 }
 
 static inline void intel_pt_set_last_ip(struct intel_pt_decoder *decoder)
 {
-   decoder->last_ip = intel_pt_calc_ip(decoder, >packet,
-   decoder->last_ip);
+   decoder->last_ip = intel_pt_calc_ip(>packet, decoder->last_ip);
 }
 
 static inline void intel_pt_set_ip(struct intel_pt_decoder *decoder)
@@ -1657,6 +1657,12 @@ next:
}
 }
 
+static inline bool intel_pt_have_ip(struct intel_pt_decoder *decoder)
+{
+   return decoder->last_ip || decoder->packet.count == 0 ||
+  decoder->packet.count == 3 || decoder->packet.count == 6;
+}
+
 /* Walk PSB+ packets to get in sync. */
 static int intel_pt_walk_psb(struct intel_pt_decoder *decoder)
 {
@@ -1677,8 +1683,7 @@ static int intel_pt_walk_psb(struct intel_pt_decoder 
*decoder)
 
case INTEL_PT_FUP:
decoder->pge = true;
-   if (decoder->last_ip || decoder->packet.count == 6 ||
-   decoder->packet.count == 0) {
+   if (intel_pt_have_ip(decoder)) {
uint64_t current_ip = decoder->ip;
 
intel_pt_set_ip(decoder);
@@ -1767,8 +1772,7 @@ static int intel_pt_walk_to_ip(struct intel_pt_decoder 
*decoder)
case INTEL_PT_TIP_PGE:
case INTEL_PT_TIP:

[GIT PULL 00/11] perf/urgent fixes

[Arnaldo Carvalho de Melo]

Hi Ingo,

Please consider pulling,

- Arnaldo

Build stats:

 # time dm
  1 70.159253018 alpine:3.4: Ok
  2 27.099391445 android-ndk:r12b-arm: Ok
  3 75.359247352 archlinux:latest: Ok
  4 24.340381467 centos:5: Ok
  5 35.444981358 centos:6: Ok
  6 40.638249015 centos:7: Ok
  7 39.903273551 debian:7: Ok
  8 44.413434336 debian:8: Ok
  9 75.444927554 debian:experimental: Ok
 10 74.050811017 fedora:20: Ok
 11 77.325297310 fedora:21: Ok
 12 76.934955654 fedora:22: Ok
 13 77.173183115 fedora:23: Ok
 14 78.870701061 fedora:24: Ok
 15 82.441966844 fedora:rawhide: Ok
 16 81.840268590 mageia:5: Ok
 17 74.529050646 opensuse:13.2: Ok
 18 76.367891421 opensuse:42.1: Ok
 19 82.874433572 opensuse:tumbleweed: Ok
 20 63.525497311 ubuntu:12.04.5: Ok
 21 69.943145955 ubuntu:14.04.4: Ok
 22 72.413641422 ubuntu:15.10: Ok
 23 69.335646559 ubuntu:16.04: Ok
 24 56.204402973 ubuntu:16.04-x-arm: Ok
 25 56.601927116 ubuntu:16.04-x-arm64: Ok
 26 32.073176756 ubuntu:16.04-x-armhf: Ok
 27 55.799523589 ubuntu:16.04-x-powerpc64: Ok
 28 56.579047498 ubuntu:16.04-x-powerpc64el: Ok
 29 55.715073756 ubuntu:16.04-x-s390: Ok
 30 76.034846449 ubuntu:16.10: Ok
1879.44s

 real   31m20.253s
 user   0m1.768s
 sys0m2.067s
 #

* The ones taking longer are doing more stuff:
 - Building objtool where supported
 - Building perf twice, with NO_LIBELF=1 and without it.

[PATCH 01/11] perf intel-pt: Fix ip compression

[Arnaldo Carvalho de Melo]

From: Adrian Hunter 

The June 2015 Intel SDM introduced IP Compression types 4 and 6. Refer
to section 36.4.2.2 Target IP (TIP) Packet - IP Compression.

Existing Intel PT packet decoder did not support type 4, and got type 6
wrong.  Because type 3 and type 4 have the same number of bytes, the
packet 'count' has been changed from being the number of ip bytes to
being the type code.  That allows the Intel PT decoder to correctly
decide whether to sign-extend or use the last ip.  However that also
meant the code had to be adjusted in a number of places.

Currently hardware is not using the new compression types, so this fix
has no effect on existing hardware.

Signed-off-by: Adrian Hunter 
Cc: Jiri Olsa 
Link: 
http://lkml.kernel.org/r/1469005206-3049-1-git-send-email-adrian.hun...@intel.com
Signed-off-by: Arnaldo Carvalho de Melo 
---
 .../perf/util/intel-pt-decoder/intel-pt-decoder.c  | 44 +++---
 .../util/intel-pt-decoder/intel-pt-pkt-decoder.c   | 24 
 2 files changed, 40 insertions(+), 28 deletions(-)

diff --git a/tools/perf/util/intel-pt-decoder/intel-pt-decoder.c 
b/tools/perf/util/intel-pt-decoder/intel-pt-decoder.c
index 9c8f15da86ce..8ff6c6a61291 100644
--- a/tools/perf/util/intel-pt-decoder/intel-pt-decoder.c
+++ b/tools/perf/util/intel-pt-decoder/intel-pt-decoder.c
@@ -123,8 +123,6 @@ struct intel_pt_decoder {
bool have_calc_cyc_to_tsc;
int exec_mode;
unsigned int insn_bytes;
-   uint64_t sign_bit;
-   uint64_t sign_bits;
uint64_t period;
enum intel_pt_period_type period_type;
uint64_t tot_insn_cnt;
@@ -191,9 +189,6 @@ struct intel_pt_decoder *intel_pt_decoder_new(struct 
intel_pt_params *params)
decoder->data   = params->data;
decoder->return_compression = params->return_compression;
 
-   decoder->sign_bit   = (uint64_t)1 << 47;
-   decoder->sign_bits  = ~(((uint64_t)1 << 48) - 1);
-
decoder->period = params->period;
decoder->period_type= params->period_type;
 
@@ -362,21 +357,30 @@ int intel_pt__strerror(int code, char *buf, size_t buflen)
return 0;
 }
 
-static uint64_t intel_pt_calc_ip(struct intel_pt_decoder *decoder,
-const struct intel_pt_pkt *packet,
+static uint64_t intel_pt_calc_ip(const struct intel_pt_pkt *packet,
 uint64_t last_ip)
 {
uint64_t ip;
 
switch (packet->count) {
-   case 2:
+   case 1:
ip = (last_ip & (uint64_t)0xULL) |
 packet->payload;
break;
-   case 4:
+   case 2:
ip = (last_ip & (uint64_t)0xULL) |
 packet->payload;
break;
+   case 3:
+   ip = packet->payload;
+   /* Sign-extend 6-byte ip */
+   if (ip & (uint64_t)0x8000ULL)
+   ip |= (uint64_t)0xULL;
+   break;
+   case 4:
+   ip = (last_ip & (uint64_t)0xULL) |
+packet->payload;
+   break;
case 6:
ip = packet->payload;
break;
@@ -384,16 +388,12 @@ static uint64_t intel_pt_calc_ip(struct intel_pt_decoder 
*decoder,
return 0;
}
 
-   if (ip & decoder->sign_bit)
-   return ip | decoder->sign_bits;
-
return ip;
 }
 
 static inline void intel_pt_set_last_ip(struct intel_pt_decoder *decoder)
 {
-   decoder->last_ip = intel_pt_calc_ip(decoder, >packet,
-   decoder->last_ip);
+   decoder->last_ip = intel_pt_calc_ip(>packet, decoder->last_ip);
 }
 
 static inline void intel_pt_set_ip(struct intel_pt_decoder *decoder)
@@ -1657,6 +1657,12 @@ next:
}
 }
 
+static inline bool intel_pt_have_ip(struct intel_pt_decoder *decoder)
+{
+   return decoder->last_ip || decoder->packet.count == 0 ||
+  decoder->packet.count == 3 || decoder->packet.count == 6;
+}
+
 /* Walk PSB+ packets to get in sync. */
 static int intel_pt_walk_psb(struct intel_pt_decoder *decoder)
 {
@@ -1677,8 +1683,7 @@ static int intel_pt_walk_psb(struct intel_pt_decoder 
*decoder)
 
case INTEL_PT_FUP:
decoder->pge = true;
-   if (decoder->last_ip || decoder->packet.count == 6 ||
-   decoder->packet.count == 0) {
+   if (intel_pt_have_ip(decoder)) {
uint64_t current_ip = decoder->ip;
 
intel_pt_set_ip(decoder);
@@ -1767,8 +1772,7 @@ static int intel_pt_walk_to_ip(struct intel_pt_decoder 
*decoder)
case INTEL_PT_TIP_PGE:
case INTEL_PT_TIP:
decoder->pge = decoder->packet.type != INTEL_PT_TIP_PGD;
-   if 

[GIT PULL 00/11] perf/urgent fixes

[Arnaldo Carvalho de Melo]

Hi Ingo,

Please consider pulling,

- Arnaldo

Build stats:

 # time dm
  1 70.159253018 alpine:3.4: Ok
  2 27.099391445 android-ndk:r12b-arm: Ok
  3 75.359247352 archlinux:latest: Ok
  4 24.340381467 centos:5: Ok
  5 35.444981358 centos:6: Ok
  6 40.638249015 centos:7: Ok
  7 39.903273551 debian:7: Ok
  8 44.413434336 debian:8: Ok
  9 75.444927554 debian:experimental: Ok
 10 74.050811017 fedora:20: Ok
 11 77.325297310 fedora:21: Ok
 12 76.934955654 fedora:22: Ok
 13 77.173183115 fedora:23: Ok
 14 78.870701061 fedora:24: Ok
 15 82.441966844 fedora:rawhide: Ok
 16 81.840268590 mageia:5: Ok
 17 74.529050646 opensuse:13.2: Ok
 18 76.367891421 opensuse:42.1: Ok
 19 82.874433572 opensuse:tumbleweed: Ok
 20 63.525497311 ubuntu:12.04.5: Ok
 21 69.943145955 ubuntu:14.04.4: Ok
 22 72.413641422 ubuntu:15.10: Ok
 23 69.335646559 ubuntu:16.04: Ok
 24 56.204402973 ubuntu:16.04-x-arm: Ok
 25 56.601927116 ubuntu:16.04-x-arm64: Ok
 26 32.073176756 ubuntu:16.04-x-armhf: Ok
 27 55.799523589 ubuntu:16.04-x-powerpc64: Ok
 28 56.579047498 ubuntu:16.04-x-powerpc64el: Ok
 29 55.715073756 ubuntu:16.04-x-s390: Ok
 30 76.034846449 ubuntu:16.10: Ok
1879.44s

 real   31m20.253s
 user   0m1.768s
 sys0m2.067s
 #

* The ones taking longer are doing more stuff:
 - Building objtool where supported
 - Building perf twice, with NO_LIBELF=1 and without it.

[PATCH 09/11] perf probe: Release resources on error when handling exit paths

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo 

Cc: Adrian Hunter 
Cc: Colin King 
Cc: David Ahern 
Cc: Jiri Olsa 
Cc: Masami Hiramatsu 
Cc: Namhyung Kim 
Cc: Wang Nan 
Link: http://lkml.kernel.org/n/tip-zh2j4iqimralugke5qq7d...@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo 
---
 tools/perf/util/probe-file.c | 12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c
index a8e76233c088..9c3b9ed5b3c3 100644
--- a/tools/perf/util/probe-file.c
+++ b/tools/perf/util/probe-file.c
@@ -143,6 +143,8 @@ struct strlist *probe_file__get_rawlist(int fd)
return NULL;
 
sl = strlist__new(NULL, NULL);
+   if (sl == NULL)
+   return NULL;
 
fddup = dup(fd);
if (fddup < 0)
@@ -163,14 +165,16 @@ struct strlist *probe_file__get_rawlist(int fd)
ret = strlist__add(sl, buf);
if (ret < 0) {
pr_debug("strlist__add failed (%d)\n", ret);
-   strlist__delete(sl);
-   return NULL;
+   goto out_close_fp;
}
}
fclose(fp);
 
return sl;
 
+out_close_fp:
+   fclose(fp);
+   goto out_free_sl;
 out_close_fddup:
close(fddup);
 out_free_sl:
@@ -467,8 +471,10 @@ static int probe_cache__load(struct probe_cache *pcache)
if (fddup < 0)
return -errno;
fp = fdopen(fddup, "r");
-   if (!fp)
+   if (!fp) {
+   close(fddup);
return -EINVAL;
+   }
 
while (!feof(fp)) {
if (!fgets(buf, MAX_CMDLEN, fp))
-- 
2.7.4

[PATCH 09/11] perf probe: Release resources on error when handling exit paths

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo 

Cc: Adrian Hunter 
Cc: Colin King 
Cc: David Ahern 
Cc: Jiri Olsa 
Cc: Masami Hiramatsu 
Cc: Namhyung Kim 
Cc: Wang Nan 
Link: http://lkml.kernel.org/n/tip-zh2j4iqimralugke5qq7d...@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo 
---
 tools/perf/util/probe-file.c | 12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c
index a8e76233c088..9c3b9ed5b3c3 100644
--- a/tools/perf/util/probe-file.c
+++ b/tools/perf/util/probe-file.c
@@ -143,6 +143,8 @@ struct strlist *probe_file__get_rawlist(int fd)
return NULL;
 
sl = strlist__new(NULL, NULL);
+   if (sl == NULL)
+   return NULL;
 
fddup = dup(fd);
if (fddup < 0)
@@ -163,14 +165,16 @@ struct strlist *probe_file__get_rawlist(int fd)
ret = strlist__add(sl, buf);
if (ret < 0) {
pr_debug("strlist__add failed (%d)\n", ret);
-   strlist__delete(sl);
-   return NULL;
+   goto out_close_fp;
}
}
fclose(fp);
 
return sl;
 
+out_close_fp:
+   fclose(fp);
+   goto out_free_sl;
 out_close_fddup:
close(fddup);
 out_free_sl:
@@ -467,8 +471,10 @@ static int probe_cache__load(struct probe_cache *pcache)
if (fddup < 0)
return -errno;
fp = fdopen(fddup, "r");
-   if (!fp)
+   if (!fp) {
+   close(fddup);
return -EINVAL;
+   }
 
while (!feof(fp)) {
if (!fgets(buf, MAX_CMDLEN, fp))
-- 
2.7.4

Re: [PATCH 2/2] power: surface3_power: MSHW0011 rev-eng implementation

[Sebastian Reichel]

Hi,

On Fri, Jul 29, 2016 at 05:49:26PM +0200, Benjamin Tissoires wrote:
> MSHW0011 replaces the battery firmware by using ACPI operation regions.
> The values have been obtained by reverse engineering, and are subject to
> errors. Looks like it works on overall pretty well.
> 
> I couldn't manage to get the IRQ correctly triggered, so I am using a
> good old polling thread to check for changes.

With CONFIG_ACPI_BATTERY being in drivers/acpi/ and this not
using the power-supply subsystem's framework at all, I wonder
if this driver should also go to drivers/acpi/.

-- Sebastian


signature.asc
Description: PGP signature

Re: [PATCH 2/2] power: surface3_power: MSHW0011 rev-eng implementation

[Sebastian Reichel]

Hi,

On Fri, Jul 29, 2016 at 05:49:26PM +0200, Benjamin Tissoires wrote:
> MSHW0011 replaces the battery firmware by using ACPI operation regions.
> The values have been obtained by reverse engineering, and are subject to
> errors. Looks like it works on overall pretty well.
> 
> I couldn't manage to get the IRQ correctly triggered, so I am using a
> good old polling thread to check for changes.

With CONFIG_ACPI_BATTERY being in drivers/acpi/ and this not
using the power-supply subsystem's framework at all, I wonder
if this driver should also go to drivers/acpi/.

-- Sebastian


signature.asc
Description: PGP signature

Re: [PATCH] Map in physical addresses in efi_map_region_fixed

[H. Peter Anvin]

On August 15, 2016 11:47:31 AM PDT, Alex Thorlton  wrote:
>On Mon, Aug 15, 2016 at 05:07:09PM +0200, Borislav Petkov wrote:
>> On Mon, Aug 15, 2016 at 01:42:58PM +0100, Matt Fleming wrote:
>> > (Cc'ing Boris and Dave)
>> > 
>> > On Fri, 05 Aug, at 06:59:35PM, Alex Thorlton wrote:
>> > > This is a simple change to add in the physical mappings as well
>as the
>> > > virtual mappings in efi_map_region_fixed.  The motivation here is
>to
>> > > get access to EFI runtime code that is only available via the 1:1
>> > > mappings on a kexec'd kernel.
>> 
>> So I don't understand: the whole jumping through hoops so that we
>have
>> stable virtual mappings just so that the kexec-ed kernel can call EFI
>> runtime, is now useless?!
>
>Well, anyone who is using the mappings in -4G down range still needs
>your code to map their stuff into the appropriate spot in the kexec'd
>kernel, so anybody who is doing things in the most up-to-date manner
>still makes use of your new code, right?
>
>AFAIK, we pass up the efi_runtime_map to the kexec'd kernel, and then
>process the memory descriptors one by one, mapping in their virtual
>addresses during kexec_enter_virtual_mode.  All of this relies on your
>efforts to pass up the virtual mappings.  The only thing we're adding
>here is the physical mappings, to match what is availble in the primary
>kernel.
>
>> What if the physical address is occupied by the kexec kernel?
>
>Ah, that's a good question.  I usually don't force the placement of my
>crashkernel, so I can't exactly comment intelligently on what will
>happen in that situation.  I will look into this, but in our case, I
>believe that the primary kernel would either fail to boot, or fail to
>load the kexec kernel if we placed it over the range where are EFI code
>gets mapped in, which I think is an issue even without this patch.
>
>IIRC, the crashkernel memory gets reserved really early, so I'd imagine
>we'd hit the former problem, since we will try to remap into that same
>space in uv_bios_init.
>
>This is sort of a hand-wavey answer - I will investigate the his
>further to
>make sure that I'm not making any stupid assumptions (there's a good
>chance that I am :)
>
>> Why do you guys need the physical mapping all of a sudden?
>
>It's not that we need it all of the sudden, necessarily, it's just that
>we've had to make other changes to make things work with the new,
>(almost) completely isolated, EFI page tables.  We ended up choosing
>the
>lesser of two evils, and have decided to temporarily rely on the
>physical address of our runtime code, instead of continuing to rely on
>EFI_OLD_MEMMAP.
>
>I guess what I'm saying is, if we hadn't been relying on some
>semi-undefined behavior in the EFI memory mapping scheme, we would've
>been relying on the physical address for quite a while, since nobody
>would have been mapping in the virtual address for us.
>
>It's important to note that we've been dancing back and forth between
>workarounds for some slightly inorrect assumptions, and some actual bug
>fixes.  We're working to get everything 100% compatible with the new
>memory mapping schemes, but there're a few pieces of the puzzle we
>haven't gotten around to yet.
>
>> Your patch is basically rendering all the effort moot and we could've
>> saved ourselves all that trouble of doing all that virtual address
>> mapping and done the 1:1 thing.
>> 
>> Which really is probably simpler since we have an EFI-specific page
>> table and running EFI in the kexec-ed kernel would mean basically
>> recreating it.
>> 
>> What am I missing?
>
>I don't think it renders all of your effort worthless.  It just allows
>those who haven't had a chance to completely update their code to work
>with the new mapping schemes a way to utilize EFI runtime callbacks, in
>a kexec'd kernel.
>
>I do understand that, in a perfect world, we would be able to just hop
>right in and use your memory mapping scheme.  At the same time, I don't
>see how this is that much different from what we already do in the
>primary, non-kexec'd kernel.
>
>If there are strong objections to this change, I won't pursue it
>further.  We will be able to achieve the same effect once we've had a
>chance to update our code to register a callback with
>SetVirtualAddressMap to fix up our function pointer.  This is on my
>upcoming to-do list, but it'll be a little bit before I have a chance
>to
>get it finished.
>
>Thanks for the input, Boris!
>
>- Alex

So to answer the implicit question: we have found UEFI stacks in the field 
which fail without the physical mappings present, and we have found stacks 
which fail without a nontrivial SetAddressMapping.
-- 
Sent from my Android device with K-9 Mail. Please excuse brevity and formatting.

Re: [PATCH] Map in physical addresses in efi_map_region_fixed

[H. Peter Anvin]

On August 15, 2016 11:47:31 AM PDT, Alex Thorlton  wrote:
>On Mon, Aug 15, 2016 at 05:07:09PM +0200, Borislav Petkov wrote:
>> On Mon, Aug 15, 2016 at 01:42:58PM +0100, Matt Fleming wrote:
>> > (Cc'ing Boris and Dave)
>> > 
>> > On Fri, 05 Aug, at 06:59:35PM, Alex Thorlton wrote:
>> > > This is a simple change to add in the physical mappings as well
>as the
>> > > virtual mappings in efi_map_region_fixed.  The motivation here is
>to
>> > > get access to EFI runtime code that is only available via the 1:1
>> > > mappings on a kexec'd kernel.
>> 
>> So I don't understand: the whole jumping through hoops so that we
>have
>> stable virtual mappings just so that the kexec-ed kernel can call EFI
>> runtime, is now useless?!
>
>Well, anyone who is using the mappings in -4G down range still needs
>your code to map their stuff into the appropriate spot in the kexec'd
>kernel, so anybody who is doing things in the most up-to-date manner
>still makes use of your new code, right?
>
>AFAIK, we pass up the efi_runtime_map to the kexec'd kernel, and then
>process the memory descriptors one by one, mapping in their virtual
>addresses during kexec_enter_virtual_mode.  All of this relies on your
>efforts to pass up the virtual mappings.  The only thing we're adding
>here is the physical mappings, to match what is availble in the primary
>kernel.
>
>> What if the physical address is occupied by the kexec kernel?
>
>Ah, that's a good question.  I usually don't force the placement of my
>crashkernel, so I can't exactly comment intelligently on what will
>happen in that situation.  I will look into this, but in our case, I
>believe that the primary kernel would either fail to boot, or fail to
>load the kexec kernel if we placed it over the range where are EFI code
>gets mapped in, which I think is an issue even without this patch.
>
>IIRC, the crashkernel memory gets reserved really early, so I'd imagine
>we'd hit the former problem, since we will try to remap into that same
>space in uv_bios_init.
>
>This is sort of a hand-wavey answer - I will investigate the his
>further to
>make sure that I'm not making any stupid assumptions (there's a good
>chance that I am :)
>
>> Why do you guys need the physical mapping all of a sudden?
>
>It's not that we need it all of the sudden, necessarily, it's just that
>we've had to make other changes to make things work with the new,
>(almost) completely isolated, EFI page tables.  We ended up choosing
>the
>lesser of two evils, and have decided to temporarily rely on the
>physical address of our runtime code, instead of continuing to rely on
>EFI_OLD_MEMMAP.
>
>I guess what I'm saying is, if we hadn't been relying on some
>semi-undefined behavior in the EFI memory mapping scheme, we would've
>been relying on the physical address for quite a while, since nobody
>would have been mapping in the virtual address for us.
>
>It's important to note that we've been dancing back and forth between
>workarounds for some slightly inorrect assumptions, and some actual bug
>fixes.  We're working to get everything 100% compatible with the new
>memory mapping schemes, but there're a few pieces of the puzzle we
>haven't gotten around to yet.
>
>> Your patch is basically rendering all the effort moot and we could've
>> saved ourselves all that trouble of doing all that virtual address
>> mapping and done the 1:1 thing.
>> 
>> Which really is probably simpler since we have an EFI-specific page
>> table and running EFI in the kexec-ed kernel would mean basically
>> recreating it.
>> 
>> What am I missing?
>
>I don't think it renders all of your effort worthless.  It just allows
>those who haven't had a chance to completely update their code to work
>with the new mapping schemes a way to utilize EFI runtime callbacks, in
>a kexec'd kernel.
>
>I do understand that, in a perfect world, we would be able to just hop
>right in and use your memory mapping scheme.  At the same time, I don't
>see how this is that much different from what we already do in the
>primary, non-kexec'd kernel.
>
>If there are strong objections to this change, I won't pursue it
>further.  We will be able to achieve the same effect once we've had a
>chance to update our code to register a callback with
>SetVirtualAddressMap to fix up our function pointer.  This is on my
>upcoming to-do list, but it'll be a little bit before I have a chance
>to
>get it finished.
>
>Thanks for the input, Boris!
>
>- Alex

So to answer the implicit question: we have found UEFI stacks in the field 
which fail without the physical mappings present, and we have found stacks 
which fail without a nontrivial SetAddressMapping.
-- 
Sent from my Android device with K-9 Mail. Please excuse brevity and formatting.

Re: [BUGFIX PATCH 2/2] brcmfmac: Change vif_event_lock to spinlock

[Arend Van Spriel]

On 15-8-2016 11:41, Masami Hiramatsu wrote:
> Change vif_event_lock to spinlock from mutex, since this lock is
> used in wait_event_timeout() via vif_event_equals(). This caused
> a warning report as below.
> 
> As far as I can see, this lock protects regions where updating
> structure members, not function calls. Also, since those
> regions are not called from interrupt handlers (of course, it
> was a mutex), spin_lock is used instead of spin_lock_irqsave.
> 
> [  186.678550] [ cut here ]
> [  186.678556] WARNING: CPU: 2 PID: 7140 at 
> /home/mhiramat/ksrc/linux/kernel/sched/core.c:7545 __might_sleep+0x7c/0x80
> [  186.678560] do not call blocking ops when !TASK_RUNNING; state=2 set at 
> [] prepare_to_wait_event+0x60/0x100
> [  186.678560] Modules linked in: brcmfmac xt_CHECKSUM rfcomm ipt_MASQUERADE 
> nf_nat_masquerade_ipv4 xt_addrtype br_netfilter xt_tcpudp ip6t_rpfilter 
> ip6t_REJECT nf_reject_ipv6 ipt_REJECT nf_reject_ipv4 xt_conntrack ip_set 
> nfnetlink ebtable_nat ebtable_broute bridge stp llc ebtable_filter ebtables 
> ip6table_raw ip6table_security ip6table_nat nf_conntrack_ipv6 nf_defrag_ipv6 
> nf_nat_ipv6 ip6table_mangle ip6table_filter ip6_tables iptable_raw 
> iptable_security iptable_nat nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 
> nf_nat nf_conntrack iptable_mangle iptable_filter ip_tables x_tables bnep 
> nls_iso8859_1 i2c_designware_platform i2c_designware_core snd_hda_codec_hdmi 
> snd_hda_codec_realtek dcdbas snd_hda_codec_generic snd_hda_intel 
> snd_hda_codec intel_rapl snd_hda_core x86_pkg_temp_thermal intel_powerclamp 
> coretemp
> [  186.678594]  snd_pcm crct10dif_pclmul crc32_pclmul aesni_intel aes_x86_64 
> joydev glue_helper snd_hwdep lrw gf128mul uvcvideo ablk_helper snd_seq_midi 
> cryptd snd_seq_midi_event snd_rawmidi videobuf2_vmalloc videobuf2_memops 
> snd_seq input_leds videobuf2_v4l2 cfg80211 videobuf2_core snd_timer videodev 
> serio_raw btusb snd_seq_device media btrtl rtsx_pci_ms snd mei_me memstick 
> hid_multitouch mei soundcore brcmutil idma64 virt_dma intel_lpss_pci 
> processor_thermal_device intel_soc_dts_iosf hci_uart btbcm btqca btintel 
> bluetooth int3403_thermal dell_smo8800 intel_lpss_acpi intel_lpss 
> int3402_thermal int340x_thermal_zone intel_hid mac_hid int3400_thermal shpchp 
> sparse_keymap acpi_pad acpi_thermal_rel acpi_als kfifo_buf industrialio 
> kvm_intel kvm irqbypass parport_pc ppdev lp parport autofs4 btrfs xor raid6_pq
> [  186.678631]  usbhid nouveau ttm i915 rtsx_pci_sdmmc mxm_wmi i2c_algo_bit 
> drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops psmouse drm ahci 
> rtsx_pci nvme nvme_core libahci i2c_hid hid pinctrl_sunrisepoint video wmi 
> pinctrl_intel fjes [last unloaded: brcmfmac]
> [  186.678646] CPU: 2 PID: 7140 Comm: wpa_supplicant Not tainted 4.8.0-rc1+ #8
> [  186.678647] Hardware name: Dell Inc. XPS 15 9550/0N7TVV, BIOS 01.02.00 
> 04/07/2016
> [  186.678648]   9d8c64b5b900 98442f23 
> 9d8c64b5b950
> [  186.678651]   9d8c64b5b940 9808b22b 
> 1d79000d
> [  186.678653]  98c75e78 026c  
> 9d8c2706d058
> [  186.678655] Call Trace:
> [  186.678659]  [] dump_stack+0x85/0xc2
> [  186.678666]  [] __warn+0xcb/0xf0
> [  186.678668]  [] warn_slowpath_fmt+0x4f/0x60
> [  186.678671]  [] ? prepare_to_wait_event+0x60/0x100
> [  186.678672]  [] ? prepare_to_wait_event+0x60/0x100
> [  186.678674]  [] __might_sleep+0x7c/0x80
> [  186.678680]  [] mutex_lock_nested+0x33/0x3b0
> [  186.678682]  [] ? trace_hardirqs_on+0xd/0x10
> [  186.678689]  [] brcmf_cfg80211_wait_vif_event+0xcd/0x130 
> [brcmfmac]
> [  186.678691]  [] ? wake_atomic_t_function+0x60/0x60
> [  186.678697]  [] brcmf_p2p_del_vif+0xf9/0x220 [brcmfmac]
> [  186.678702]  [] brcmf_cfg80211_del_iface+0x21b/0x270 
> [brcmfmac]
> [  186.678716]  [] nl80211_del_interface+0xfe/0x3a0 
> [cfg80211]
> [  186.678718]  [] genl_family_rcv_msg+0x1b5/0x370
> [  186.678720]  [] ? trace_hardirqs_on+0xd/0x10
> [  186.678721]  [] genl_rcv_msg+0x7d/0xb0
> [  186.678722]  [] ? genl_family_rcv_msg+0x370/0x370
> [  186.678724]  [] netlink_rcv_skb+0x97/0xb0
> [  186.678726]  [] genl_rcv+0x28/0x40
> [  186.678727]  [] netlink_unicast+0x1d3/0x2f0
> [  186.678729]  [] ? netlink_unicast+0x14b/0x2f0
> [  186.678731]  [] netlink_sendmsg+0x2eb/0x3a0
> [  186.678733]  [] sock_sendmsg+0x38/0x50
> [  186.678734]  [] ___sys_sendmsg+0x27f/0x290
> [  186.678737]  [] ? mntput_no_expire+0x5/0x3f0
> [  186.678739]  [] ? mntput_no_expire+0x8e/0x3f0
> [  186.678741]  [] ? mntput_no_expire+0x5/0x3f0
> [  186.678743]  [] ? mntput+0x24/0x40
> [  186.678744]  [] ? __fput+0x190/0x200
> [  186.678746]  [] __sys_sendmsg+0x45/0x80
> [  186.678748]  [] SyS_sendmsg+0x12/0x20
> [  186.678749]  [] entry_SYSCALL_64_fastpath+0x23/0xc1
> [  186.678751]  [] ? trace_hardirqs_off_caller+0x1f/0xc0
> [  186.678752] ---[ end trace e224d66c5d8408b5 ]---

Acked-by: Arend van Spriel 

Re: [BUGFIX PATCH 2/2] brcmfmac: Change vif_event_lock to spinlock

[Arend Van Spriel]

On 15-8-2016 11:41, Masami Hiramatsu wrote:
> Change vif_event_lock to spinlock from mutex, since this lock is
> used in wait_event_timeout() via vif_event_equals(). This caused
> a warning report as below.
> 
> As far as I can see, this lock protects regions where updating
> structure members, not function calls. Also, since those
> regions are not called from interrupt handlers (of course, it
> was a mutex), spin_lock is used instead of spin_lock_irqsave.
> 
> [  186.678550] [ cut here ]
> [  186.678556] WARNING: CPU: 2 PID: 7140 at 
> /home/mhiramat/ksrc/linux/kernel/sched/core.c:7545 __might_sleep+0x7c/0x80
> [  186.678560] do not call blocking ops when !TASK_RUNNING; state=2 set at 
> [] prepare_to_wait_event+0x60/0x100
> [  186.678560] Modules linked in: brcmfmac xt_CHECKSUM rfcomm ipt_MASQUERADE 
> nf_nat_masquerade_ipv4 xt_addrtype br_netfilter xt_tcpudp ip6t_rpfilter 
> ip6t_REJECT nf_reject_ipv6 ipt_REJECT nf_reject_ipv4 xt_conntrack ip_set 
> nfnetlink ebtable_nat ebtable_broute bridge stp llc ebtable_filter ebtables 
> ip6table_raw ip6table_security ip6table_nat nf_conntrack_ipv6 nf_defrag_ipv6 
> nf_nat_ipv6 ip6table_mangle ip6table_filter ip6_tables iptable_raw 
> iptable_security iptable_nat nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 
> nf_nat nf_conntrack iptable_mangle iptable_filter ip_tables x_tables bnep 
> nls_iso8859_1 i2c_designware_platform i2c_designware_core snd_hda_codec_hdmi 
> snd_hda_codec_realtek dcdbas snd_hda_codec_generic snd_hda_intel 
> snd_hda_codec intel_rapl snd_hda_core x86_pkg_temp_thermal intel_powerclamp 
> coretemp
> [  186.678594]  snd_pcm crct10dif_pclmul crc32_pclmul aesni_intel aes_x86_64 
> joydev glue_helper snd_hwdep lrw gf128mul uvcvideo ablk_helper snd_seq_midi 
> cryptd snd_seq_midi_event snd_rawmidi videobuf2_vmalloc videobuf2_memops 
> snd_seq input_leds videobuf2_v4l2 cfg80211 videobuf2_core snd_timer videodev 
> serio_raw btusb snd_seq_device media btrtl rtsx_pci_ms snd mei_me memstick 
> hid_multitouch mei soundcore brcmutil idma64 virt_dma intel_lpss_pci 
> processor_thermal_device intel_soc_dts_iosf hci_uart btbcm btqca btintel 
> bluetooth int3403_thermal dell_smo8800 intel_lpss_acpi intel_lpss 
> int3402_thermal int340x_thermal_zone intel_hid mac_hid int3400_thermal shpchp 
> sparse_keymap acpi_pad acpi_thermal_rel acpi_als kfifo_buf industrialio 
> kvm_intel kvm irqbypass parport_pc ppdev lp parport autofs4 btrfs xor raid6_pq
> [  186.678631]  usbhid nouveau ttm i915 rtsx_pci_sdmmc mxm_wmi i2c_algo_bit 
> drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops psmouse drm ahci 
> rtsx_pci nvme nvme_core libahci i2c_hid hid pinctrl_sunrisepoint video wmi 
> pinctrl_intel fjes [last unloaded: brcmfmac]
> [  186.678646] CPU: 2 PID: 7140 Comm: wpa_supplicant Not tainted 4.8.0-rc1+ #8
> [  186.678647] Hardware name: Dell Inc. XPS 15 9550/0N7TVV, BIOS 01.02.00 
> 04/07/2016
> [  186.678648]   9d8c64b5b900 98442f23 
> 9d8c64b5b950
> [  186.678651]   9d8c64b5b940 9808b22b 
> 1d79000d
> [  186.678653]  98c75e78 026c  
> 9d8c2706d058
> [  186.678655] Call Trace:
> [  186.678659]  [] dump_stack+0x85/0xc2
> [  186.678666]  [] __warn+0xcb/0xf0
> [  186.678668]  [] warn_slowpath_fmt+0x4f/0x60
> [  186.678671]  [] ? prepare_to_wait_event+0x60/0x100
> [  186.678672]  [] ? prepare_to_wait_event+0x60/0x100
> [  186.678674]  [] __might_sleep+0x7c/0x80
> [  186.678680]  [] mutex_lock_nested+0x33/0x3b0
> [  186.678682]  [] ? trace_hardirqs_on+0xd/0x10
> [  186.678689]  [] brcmf_cfg80211_wait_vif_event+0xcd/0x130 
> [brcmfmac]
> [  186.678691]  [] ? wake_atomic_t_function+0x60/0x60
> [  186.678697]  [] brcmf_p2p_del_vif+0xf9/0x220 [brcmfmac]
> [  186.678702]  [] brcmf_cfg80211_del_iface+0x21b/0x270 
> [brcmfmac]
> [  186.678716]  [] nl80211_del_interface+0xfe/0x3a0 
> [cfg80211]
> [  186.678718]  [] genl_family_rcv_msg+0x1b5/0x370
> [  186.678720]  [] ? trace_hardirqs_on+0xd/0x10
> [  186.678721]  [] genl_rcv_msg+0x7d/0xb0
> [  186.678722]  [] ? genl_family_rcv_msg+0x370/0x370
> [  186.678724]  [] netlink_rcv_skb+0x97/0xb0
> [  186.678726]  [] genl_rcv+0x28/0x40
> [  186.678727]  [] netlink_unicast+0x1d3/0x2f0
> [  186.678729]  [] ? netlink_unicast+0x14b/0x2f0
> [  186.678731]  [] netlink_sendmsg+0x2eb/0x3a0
> [  186.678733]  [] sock_sendmsg+0x38/0x50
> [  186.678734]  [] ___sys_sendmsg+0x27f/0x290
> [  186.678737]  [] ? mntput_no_expire+0x5/0x3f0
> [  186.678739]  [] ? mntput_no_expire+0x8e/0x3f0
> [  186.678741]  [] ? mntput_no_expire+0x5/0x3f0
> [  186.678743]  [] ? mntput+0x24/0x40
> [  186.678744]  [] ? __fput+0x190/0x200
> [  186.678746]  [] __sys_sendmsg+0x45/0x80
> [  186.678748]  [] SyS_sendmsg+0x12/0x20
> [  186.678749]  [] entry_SYSCALL_64_fastpath+0x23/0xc1
> [  186.678751]  [] ? trace_hardirqs_off_caller+0x1f/0xc0
> [  186.678752] ---[ end trace e224d66c5d8408b5 ]---

Acked-by: Arend van Spriel 
> Signed-off-by: 

[PATCH] clk: Return errors from clk providers in __of_clk_get_from_provider()

[Stephen Boyd]

Before commit 0861e5b8cf80 (clk: Add clk_hw OF clk providers,
2016-02-05) __of_clk_get_from_provider() would return an error
pointer of the provider's choosing if there was a provider
registered and EPROBE_DEFER otherwise. After that commit, it
would return EPROBE_DEFER regardless of whether or not the
provider returned an error. This is odd and can lead to behavior
where clk consumers keep probe deferring when they should be
seeing some other error.

Let's restore the previous behavior where we only return
EPROBE_DEFER when there isn't a provider in our of_clk_providers
list. Otherwise, return the error from the last provider we find
that matches the node.

Reported-by: Masahiro Yamada 
Fixes: 0861e5b8cf80 ("clk: Add clk_hw OF clk providers")
Signed-off-by: Stephen Boyd 
---
 drivers/clk/clk.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 6298715d0e44..71cc56712666 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -3194,7 +3194,7 @@ struct clk *__of_clk_get_from_provider(struct 
of_phandle_args *clkspec,
 {
struct of_clk_provider *provider;
struct clk *clk = ERR_PTR(-EPROBE_DEFER);
-   struct clk_hw *hw = ERR_PTR(-EPROBE_DEFER);
+   struct clk_hw *hw;
 
if (!clkspec)
return ERR_PTR(-EINVAL);
@@ -3202,12 +3202,13 @@ struct clk *__of_clk_get_from_provider(struct 
of_phandle_args *clkspec,
/* Check if we have such a provider in our array */
mutex_lock(_clk_mutex);
list_for_each_entry(provider, _clk_providers, link) {
-   if (provider->node == clkspec->np)
+   if (provider->node == clkspec->np) {
hw = __of_clk_get_hw_from_provider(provider, clkspec);
-   if (!IS_ERR(hw)) {
clk = __clk_create_clk(hw, dev_id, con_id);
+   }
 
-   if (!IS_ERR(clk) && !__clk_get(clk)) {
+   if (!IS_ERR(clk)) {
+   if (!__clk_get(clk)) {
__clk_free_clk(clk);
clk = ERR_PTR(-ENOENT);
}
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH] clk: Return errors from clk providers in __of_clk_get_from_provider()

[Stephen Boyd]

Before commit 0861e5b8cf80 (clk: Add clk_hw OF clk providers,
2016-02-05) __of_clk_get_from_provider() would return an error
pointer of the provider's choosing if there was a provider
registered and EPROBE_DEFER otherwise. After that commit, it
would return EPROBE_DEFER regardless of whether or not the
provider returned an error. This is odd and can lead to behavior
where clk consumers keep probe deferring when they should be
seeing some other error.

Let's restore the previous behavior where we only return
EPROBE_DEFER when there isn't a provider in our of_clk_providers
list. Otherwise, return the error from the last provider we find
that matches the node.

Reported-by: Masahiro Yamada 
Fixes: 0861e5b8cf80 ("clk: Add clk_hw OF clk providers")
Signed-off-by: Stephen Boyd 
---
 drivers/clk/clk.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 6298715d0e44..71cc56712666 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -3194,7 +3194,7 @@ struct clk *__of_clk_get_from_provider(struct 
of_phandle_args *clkspec,
 {
struct of_clk_provider *provider;
struct clk *clk = ERR_PTR(-EPROBE_DEFER);
-   struct clk_hw *hw = ERR_PTR(-EPROBE_DEFER);
+   struct clk_hw *hw;
 
if (!clkspec)
return ERR_PTR(-EINVAL);
@@ -3202,12 +3202,13 @@ struct clk *__of_clk_get_from_provider(struct 
of_phandle_args *clkspec,
/* Check if we have such a provider in our array */
mutex_lock(_clk_mutex);
list_for_each_entry(provider, _clk_providers, link) {
-   if (provider->node == clkspec->np)
+   if (provider->node == clkspec->np) {
hw = __of_clk_get_hw_from_provider(provider, clkspec);
-   if (!IS_ERR(hw)) {
clk = __clk_create_clk(hw, dev_id, con_id);
+   }
 
-   if (!IS_ERR(clk) && !__clk_get(clk)) {
+   if (!IS_ERR(clk)) {
+   if (!__clk_get(clk)) {
__clk_free_clk(clk);
clk = ERR_PTR(-ENOENT);
}
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [BUGFIX PATCH 1/2] brcmfmac: Check rtnl_lock is locked when removing interface

[Arend Van Spriel]

On 15-8-2016 13:52, Rafał Miłecki wrote:
> On 15 August 2016 at 12:57, Kalle Valo  wrote:
>> Rafał Miłecki  writes:
>>
 Signed-off-by: Masami Hiramatsu 
>>>
>>> Fixes: a63b09872c1d ("brcmfmac: delete interface directly in code that sent 
>>> fw request")
>>> Acked-by: Rafał Miłecki 
>>>
>>> Kalle: I'm acking this as bugfix for 4.8 release.
>>
>> Ok. I'll wait few days for more comments before I apply this.
> 
> Sure.
> 
> 
>> (I assume you are talking only about patch 1)
> 
> Yes, I'll leave mutex vs. spinlock to the experts :)

Don't know who the experts are. Surely not me :-p

I made an uneducated design decision using a mutex for this. The
reasoning for using a regular spinlock make sense. So I will go and ack
that patch.

Regards,
Arend

Re: [BUGFIX PATCH 1/2] brcmfmac: Check rtnl_lock is locked when removing interface

[Arend Van Spriel]

On 15-8-2016 13:52, Rafał Miłecki wrote:
> On 15 August 2016 at 12:57, Kalle Valo  wrote:
>> Rafał Miłecki  writes:
>>
 Signed-off-by: Masami Hiramatsu 
>>>
>>> Fixes: a63b09872c1d ("brcmfmac: delete interface directly in code that sent 
>>> fw request")
>>> Acked-by: Rafał Miłecki 
>>>
>>> Kalle: I'm acking this as bugfix for 4.8 release.
>>
>> Ok. I'll wait few days for more comments before I apply this.
> 
> Sure.
> 
> 
>> (I assume you are talking only about patch 1)
> 
> Yes, I'll leave mutex vs. spinlock to the experts :)

Don't know who the experts are. Surely not me :-p

I made an uneducated design decision using a mutex for this. The
reasoning for using a regular spinlock make sense. So I will go and ack
that patch.

Regards,
Arend

Re: [PATCH 19/58] staging: lustre: llite: add md_op_data parameter to ll_get_dir_page

[James Simmons]

> > From: wang di 
> > 
> > Pass in struct md_op_data for ll_get_dir_page function.
> > 
> > Signed-off-by: wang di 
> > Reviewed-on: http://review.whamcloud.com/7043
> > Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-3531
> > Reviewed-by: John L. Hammond 
> > Reviewed-by: Jinshan Xiong 
> > Reviewed-by: Andreas Dilger 
> > Reviewed-by: Oleg Drokin 
> > Signed-off-by: James Simmons 
> > ---
> >  drivers/staging/lustre/lustre/llite/dir.c  |8 
> >  .../staging/lustre/lustre/llite/llite_internal.h   |4 ++--
> >  drivers/staging/lustre/lustre/llite/statahead.c|   15 +++
> >  3 files changed, 17 insertions(+), 10 deletions(-)
> 
> This patch fails to apply, so I have to stop here in the patch series.
> 
> So I'm guesing your second patch series also will fail to apply, so can
> you resend all of the outstanding patches you have sent me after
> rebasing on my staging-testing branch?

Do you mind if I combine them into one series? Also I have a few more 
patches I like to include. Is it okay to add those as well?

Re: [PATCH 19/58] staging: lustre: llite: add md_op_data parameter to ll_get_dir_page

[James Simmons]

> > From: wang di 
> > 
> > Pass in struct md_op_data for ll_get_dir_page function.
> > 
> > Signed-off-by: wang di 
> > Reviewed-on: http://review.whamcloud.com/7043
> > Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-3531
> > Reviewed-by: John L. Hammond 
> > Reviewed-by: Jinshan Xiong 
> > Reviewed-by: Andreas Dilger 
> > Reviewed-by: Oleg Drokin 
> > Signed-off-by: James Simmons 
> > ---
> >  drivers/staging/lustre/lustre/llite/dir.c  |8 
> >  .../staging/lustre/lustre/llite/llite_internal.h   |4 ++--
> >  drivers/staging/lustre/lustre/llite/statahead.c|   15 +++
> >  3 files changed, 17 insertions(+), 10 deletions(-)
> 
> This patch fails to apply, so I have to stop here in the patch series.
> 
> So I'm guesing your second patch series also will fail to apply, so can
> you resend all of the outstanding patches you have sent me after
> rebasing on my staging-testing branch?

Do you mind if I combine them into one series? Also I have a few more 
patches I like to include. Is it okay to add those as well?

Re: [PATCH v3 1/4] hwmon: iio_hwmon: delay probing with late_initcall

[Jonathan Cameron]

On 15 August 2016 18:07:30 BST, Guenter Roeck  wrote:
>On Mon, Aug 15, 2016 at 04:40:21PM +0100, Jonathan Cameron wrote:
>> On 26/07/16 17:04, Guenter Roeck wrote:
>> > On Tue, Jul 26, 2016 at 12:00:33PM +0200, Alexander Stein wrote:
>> >> On Tuesday 26 July 2016 11:33:59, Quentin Schulz wrote:
>> >>> On 26/07/2016 11:05, Alexander Stein wrote:
>>  On Tuesday 26 July 2016 10:24:48, Quentin Schulz wrote:
>> > On 26/07/2016 10:21, Alexander Stein wrote:
>> >> On Tuesday 26 July 2016 09:43:44, Quentin Schulz wrote:
>> >>> iio_channel_get_all returns -ENODEV when it cannot find
>either phandles
>> >>> and
>> >>> properties in the Device Tree or channels whose
>consumer_dev_name
>> >>> matches
>> >>> iio_hwmon in iio_map_list. The iio_map_list is filled in by
>iio drivers
>> >>> which might be probed after iio_hwmon.
>> >>
>> >> Would it work if iio_channel_get_all returning ENODEV is used
>for
>> >> returning
>> >> EPROBE_DEFER in iio_channel_get_all? Using late initcalls for
>> >> driver/device
>> >> dependencies seems not right for me at this place.
>> >
>> > Then what if the iio_channel_get_all is called outside of the
>probe of a
>> > driver? We'll have to change the error code, things we are
>apparently
>> > trying to avoid (see v2 patches' discussions).
>> 
>>  Maybe I didn't express my idea enough. I don't want to change
>the behavior
>>  of iio_channel_get_all at all. Just the result evaluation of
>>  iio_channel_get_all in iio_hwmon_probe. I have something link
>the patch
>>  below in mind.
>> 
>>  Best regards,
>>  Alexander
>>  ---
>>  diff --git a/drivers/hwmon/iio_hwmon.c
>b/drivers/hwmon/iio_hwmon.c
>>  index b550ba5..e32d150 100644
>>  --- a/drivers/hwmon/iio_hwmon.c
>>  +++ b/drivers/hwmon/iio_hwmon.c
>>  @@ -73,8 +73,12 @@ static int iio_hwmon_probe(struct
>platform_device
>>  *pdev)
>> 
>>  name = dev->of_node->name;
>>  
>>  channels = iio_channel_get_all(dev);
>> 
>>  -   if (IS_ERR(channels))
>>  -   return PTR_ERR(channels);
>>  +   if (IS_ERR(channels)) {
>>  +   if (PTR_ERR(channels) == -ENODEV)
>>  +   return -EPROBE_DEFER;
>>  +   else
>>  +   return PTR_ERR(channels);
>>  +   }
>> 
>>  st = devm_kzalloc(dev, sizeof(*st), GFP_KERNEL);
>>  if (st == NULL) {
>> >>>
>> >>> Indeed, I misunderstood what you told me.
>> >>>
>> >>> Actually, the patch you proposed is part of my v1
>> >>> (https://lkml.org/lkml/2016/6/28/203) and v2
>> >>> (https://lkml.org/lkml/2016/7/15/215).
>> >>> Jonathan and Guenter didn't really like the idea of changing the
>-ENODEV
>> >>> in -EPROBE_DEFER.
>> >>
>> >> Thanks for the links.
>> >>
>> >>> What I thought you were proposing was to change the -ENODEV
>return code
>> >>> inside iio_channel_get_all. This cannot be an option since the
>function
>> >>> might be called outside of a probe (it is not yet, but might be
>in the
>> >>> future?).
>> >>
>> >> AFAICS this is a helper function not knowing about device probing
>itself. And 
>> >> it should stay at that.
>> >>
>> >>> Of what I understood, two possibilities are then possible
>(proposed
>> >>> either by Guenter or Jonathan): either rework the iio framework
>to
>> >>> register iio map array earlier or to use late_initcall instead of
>init
>> >>> for the driver consuming the iio channels.
>> >>
>> >> Interestingly using this problem would not arise due to module
>dependencies. 
>> >> But using late_initcall would mean this needs to be done on any
>driver using 
>> >> iio channels? I would rather keep those consumers simple.
>> >>
>> > Me too, but that would imply a solution in iio. The change you
>propose above
>> > isn't exactly simple either, and would also be needed in each
>consumer driver.
>> > 
>> > Just for the record, I dislike the late_initcall solution as well,
>but I prefer
>> > it over blindly converting ENODEV to EPROBE_DEFER.
>> I'm falling on the other side on this one right now. Though I'd be
>tempted
>> to renaming the function to something like
>iio_channel_get_all_or_defer
>> to make it explicit that it can result in deferred probing.
>> 
>Would this new function return -EPROBE_DEFER instead of -ENODEV ?
Yes. Though whether it really adds much over doing that in drivers isn't clear.

Hmm. Needs more thought...
>
>Thanks,
>Guenter
>--
>To unsubscribe from this list: send the line "unsubscribe linux-iio" in
>the body of a message to majord...@vger.kernel.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

Re: [PATCH v3 1/4] hwmon: iio_hwmon: delay probing with late_initcall

[Jonathan Cameron]

On 15 August 2016 18:07:30 BST, Guenter Roeck  wrote:
>On Mon, Aug 15, 2016 at 04:40:21PM +0100, Jonathan Cameron wrote:
>> On 26/07/16 17:04, Guenter Roeck wrote:
>> > On Tue, Jul 26, 2016 at 12:00:33PM +0200, Alexander Stein wrote:
>> >> On Tuesday 26 July 2016 11:33:59, Quentin Schulz wrote:
>> >>> On 26/07/2016 11:05, Alexander Stein wrote:
>>  On Tuesday 26 July 2016 10:24:48, Quentin Schulz wrote:
>> > On 26/07/2016 10:21, Alexander Stein wrote:
>> >> On Tuesday 26 July 2016 09:43:44, Quentin Schulz wrote:
>> >>> iio_channel_get_all returns -ENODEV when it cannot find
>either phandles
>> >>> and
>> >>> properties in the Device Tree or channels whose
>consumer_dev_name
>> >>> matches
>> >>> iio_hwmon in iio_map_list. The iio_map_list is filled in by
>iio drivers
>> >>> which might be probed after iio_hwmon.
>> >>
>> >> Would it work if iio_channel_get_all returning ENODEV is used
>for
>> >> returning
>> >> EPROBE_DEFER in iio_channel_get_all? Using late initcalls for
>> >> driver/device
>> >> dependencies seems not right for me at this place.
>> >
>> > Then what if the iio_channel_get_all is called outside of the
>probe of a
>> > driver? We'll have to change the error code, things we are
>apparently
>> > trying to avoid (see v2 patches' discussions).
>> 
>>  Maybe I didn't express my idea enough. I don't want to change
>the behavior
>>  of iio_channel_get_all at all. Just the result evaluation of
>>  iio_channel_get_all in iio_hwmon_probe. I have something link
>the patch
>>  below in mind.
>> 
>>  Best regards,
>>  Alexander
>>  ---
>>  diff --git a/drivers/hwmon/iio_hwmon.c
>b/drivers/hwmon/iio_hwmon.c
>>  index b550ba5..e32d150 100644
>>  --- a/drivers/hwmon/iio_hwmon.c
>>  +++ b/drivers/hwmon/iio_hwmon.c
>>  @@ -73,8 +73,12 @@ static int iio_hwmon_probe(struct
>platform_device
>>  *pdev)
>> 
>>  name = dev->of_node->name;
>>  
>>  channels = iio_channel_get_all(dev);
>> 
>>  -   if (IS_ERR(channels))
>>  -   return PTR_ERR(channels);
>>  +   if (IS_ERR(channels)) {
>>  +   if (PTR_ERR(channels) == -ENODEV)
>>  +   return -EPROBE_DEFER;
>>  +   else
>>  +   return PTR_ERR(channels);
>>  +   }
>> 
>>  st = devm_kzalloc(dev, sizeof(*st), GFP_KERNEL);
>>  if (st == NULL) {
>> >>>
>> >>> Indeed, I misunderstood what you told me.
>> >>>
>> >>> Actually, the patch you proposed is part of my v1
>> >>> (https://lkml.org/lkml/2016/6/28/203) and v2
>> >>> (https://lkml.org/lkml/2016/7/15/215).
>> >>> Jonathan and Guenter didn't really like the idea of changing the
>-ENODEV
>> >>> in -EPROBE_DEFER.
>> >>
>> >> Thanks for the links.
>> >>
>> >>> What I thought you were proposing was to change the -ENODEV
>return code
>> >>> inside iio_channel_get_all. This cannot be an option since the
>function
>> >>> might be called outside of a probe (it is not yet, but might be
>in the
>> >>> future?).
>> >>
>> >> AFAICS this is a helper function not knowing about device probing
>itself. And 
>> >> it should stay at that.
>> >>
>> >>> Of what I understood, two possibilities are then possible
>(proposed
>> >>> either by Guenter or Jonathan): either rework the iio framework
>to
>> >>> register iio map array earlier or to use late_initcall instead of
>init
>> >>> for the driver consuming the iio channels.
>> >>
>> >> Interestingly using this problem would not arise due to module
>dependencies. 
>> >> But using late_initcall would mean this needs to be done on any
>driver using 
>> >> iio channels? I would rather keep those consumers simple.
>> >>
>> > Me too, but that would imply a solution in iio. The change you
>propose above
>> > isn't exactly simple either, and would also be needed in each
>consumer driver.
>> > 
>> > Just for the record, I dislike the late_initcall solution as well,
>but I prefer
>> > it over blindly converting ENODEV to EPROBE_DEFER.
>> I'm falling on the other side on this one right now. Though I'd be
>tempted
>> to renaming the function to something like
>iio_channel_get_all_or_defer
>> to make it explicit that it can result in deferred probing.
>> 
>Would this new function return -EPROBE_DEFER instead of -ENODEV ?
Yes. Though whether it really adds much over doing that in drivers isn't clear.

Hmm. Needs more thought...
>
>Thanks,
>Guenter
>--
>To unsubscribe from this list: send the line "unsubscribe linux-iio" in
>the body of a message to majord...@vger.kernel.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

Re: [PATCH 4.7 00/41] 4.7.1-stable review

[Kevin Hilman]

Greg Kroah-Hartman  writes:

> On Sun, Aug 14, 2016 at 06:44:02PM -0700, kernelci.org bot wrote:
>> stable-rc boot: 468 boots: 1 failed, 432 passed with 33 offline, 2 conflicts 
>> (v4.7-41-g57464803070d)
>> 
>> Full Boot Summary: 
>> https://kernelci.org/boot/all/job/stable-rc/kernel/v4.7-41-g57464803070d/
>> Full Build Summary: 
>> https://kernelci.org/build/stable-rc/kernel/v4.7-41-g57464803070d/
>> 
>> Tree: stable-rc
>> Branch: local/linux-4.7.y
>> Git Describe: v4.7-41-g57464803070d
>> Git Commit: 57464803070db4979012aba1322f675d42a5c280
>> Git URL: 
>> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git
>> Tested: 85 unique boards, 21 SoC families, 35 builds out of 143
>> 
>> Boot Failure Detected: https://kernelci.org/boot/?v4.7-41-g57464803070d
>> 
>> arm:
>> 
>> multi_v7_defconfig+CONFIG_LKDTM=y:
>> sun5i-r8-chip: 1 failed lab
>
> Is this boot failure to be expected for the 4.7 tree?

Nope, but  I re-tried it and it seems to be working fine, so it can be
considered a PASS.  

Kevin

Re: [PATCH 4.7 00/41] 4.7.1-stable review

[Kevin Hilman]

Greg Kroah-Hartman  writes:

> On Sun, Aug 14, 2016 at 06:44:02PM -0700, kernelci.org bot wrote:
>> stable-rc boot: 468 boots: 1 failed, 432 passed with 33 offline, 2 conflicts 
>> (v4.7-41-g57464803070d)
>> 
>> Full Boot Summary: 
>> https://kernelci.org/boot/all/job/stable-rc/kernel/v4.7-41-g57464803070d/
>> Full Build Summary: 
>> https://kernelci.org/build/stable-rc/kernel/v4.7-41-g57464803070d/
>> 
>> Tree: stable-rc
>> Branch: local/linux-4.7.y
>> Git Describe: v4.7-41-g57464803070d
>> Git Commit: 57464803070db4979012aba1322f675d42a5c280
>> Git URL: 
>> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git
>> Tested: 85 unique boards, 21 SoC families, 35 builds out of 143
>> 
>> Boot Failure Detected: https://kernelci.org/boot/?v4.7-41-g57464803070d
>> 
>> arm:
>> 
>> multi_v7_defconfig+CONFIG_LKDTM=y:
>> sun5i-r8-chip: 1 failed lab
>
> Is this boot failure to be expected for the 4.7 tree?

Nope, but  I re-tried it and it seems to be working fine, so it can be
considered a PASS.  

Kevin

Re: [RFC PATCH v5 3/3] net: phy: Add gmiitorgmii converter support

[Andrew Lunn]

> Signed-off-by: Kedareswara rao Appana 
> ---
> Thanks a lot Andrew for your inputs.
> Changes for v5:
> --> Fixed return values in the probe as suggested by punnaiah.
> --> Added a mask for the converter speed as suggested by punnaiah.
> +/* Xilinx GMII2RGMII Converter driver
> + *
> + * Copyright (C) 2016 Xilinx, Inc.
> + *
> + * Author: Kedareswara rao Appana 

Not cool

https://github.com/lunn/linux/commit/03d375489ceb56e171056f44d0fe9c34ca5a098e

Notice the

Not-Signed-off-by: Andrew Lunn 

and the Copyright i added?

In various emails i gave you the basic idea how this should be done, a
framework of code, and then took your code which did not even compile
and made the core of it work. This is as much my code as your code, so
i expect my Copyright to be kept on this code. And since this is my
code, i need to give a Signed-off-by, which i was not yet ready to
give since the code was not complete or working.

Please put both my Copyright back and Not-Signed-Off-by back and post
the next version as RFC. Once i'm happy this code is O.K, i will give
you a Signed-off-by.

Andrew

Re: [RFC PATCH v5 3/3] net: phy: Add gmiitorgmii converter support

[Andrew Lunn]

> Signed-off-by: Kedareswara rao Appana 
> ---
> Thanks a lot Andrew for your inputs.
> Changes for v5:
> --> Fixed return values in the probe as suggested by punnaiah.
> --> Added a mask for the converter speed as suggested by punnaiah.
> +/* Xilinx GMII2RGMII Converter driver
> + *
> + * Copyright (C) 2016 Xilinx, Inc.
> + *
> + * Author: Kedareswara rao Appana 

Not cool

https://github.com/lunn/linux/commit/03d375489ceb56e171056f44d0fe9c34ca5a098e

Notice the

Not-Signed-off-by: Andrew Lunn 

and the Copyright i added?

In various emails i gave you the basic idea how this should be done, a
framework of code, and then took your code which did not even compile
and made the core of it work. This is as much my code as your code, so
i expect my Copyright to be kept on this code. And since this is my
code, i need to give a Signed-off-by, which i was not yet ready to
give since the code was not complete or working.

Please put both my Copyright back and Not-Signed-Off-by back and post
the next version as RFC. Once i'm happy this code is O.K, i will give
you a Signed-off-by.

Andrew

Building a subdirectory ignores parent subdir-ccflags (was: Re: [PATCH 1/2] staging: lustre: Add include path to Makefile)

[Joe Perches]

On Mon, 2016-08-15 at 14:14 -0700, Joe Perches wrote:
> On Mon, 2016-08-15 at 23:04 +0200, Greg Kroah-Hartman wrote:
> > On Mon, Aug 15, 2016 at 12:33:23PM -0700, Joe Perches wrote:
> > > Start to rationalize include paths in source code files.
> []
> > > diff --git a/drivers/staging/lustre/Makefile 
> > > b/drivers/staging/lustre/Makefile
> []
> > > @@ -1,2 +1,5 @@
> > > +subdir-ccflags-y += -I$(srctree)/drivers/staging/lustre/include/
> > > +subdir-ccflags-y += -I$(srctree)/drivers/staging/lustre/lustre/include/
> > > +
> > >  obj-$(CONFIG_LNET)   += lnet/
> > >  obj-$(CONFIG_LUSTRE_FS)  += lustre/
> > This is good, but does this break the subdir make command:
> > make M=drivers/staging/lustre/foo_dir/
> > ?
> hmm, yeah, it does.  Oh well, nevermind for awhile.
> > I remember the last time I tried to clean this up, it took a while...
> It seems like something the build tools should
> handle correctly now, but I'll look at it.

Perhaps making a specific directory should also walk up
any parent directory Makefiles looking for subdir flags.

Is that unreasonable?  Any suggestions?

Building a subdirectory ignores parent subdir-ccflags (was: Re: [PATCH 1/2] staging: lustre: Add include path to Makefile)

[Joe Perches]

On Mon, 2016-08-15 at 14:14 -0700, Joe Perches wrote:
> On Mon, 2016-08-15 at 23:04 +0200, Greg Kroah-Hartman wrote:
> > On Mon, Aug 15, 2016 at 12:33:23PM -0700, Joe Perches wrote:
> > > Start to rationalize include paths in source code files.
> []
> > > diff --git a/drivers/staging/lustre/Makefile 
> > > b/drivers/staging/lustre/Makefile
> []
> > > @@ -1,2 +1,5 @@
> > > +subdir-ccflags-y += -I$(srctree)/drivers/staging/lustre/include/
> > > +subdir-ccflags-y += -I$(srctree)/drivers/staging/lustre/lustre/include/
> > > +
> > >  obj-$(CONFIG_LNET)   += lnet/
> > >  obj-$(CONFIG_LUSTRE_FS)  += lustre/
> > This is good, but does this break the subdir make command:
> > make M=drivers/staging/lustre/foo_dir/
> > ?
> hmm, yeah, it does.  Oh well, nevermind for awhile.
> > I remember the last time I tried to clean this up, it took a while...
> It seems like something the build tools should
> handle correctly now, but I'll look at it.

Perhaps making a specific directory should also walk up
any parent directory Makefiles looking for subdir flags.

Is that unreasonable?  Any suggestions?

Re: [PATCH v3 6/6] clk: mvebu: Add the peripheral clock driver for Armada 3700

[Stephen Boyd]

On 07/19, Gregory CLEMENT wrote:
> +
> +static const struct clk_ops clk_double_div_ops = {
> + .recalc_rate = clk_double_div_recalc_rate,
> +};
> +
> +static const struct of_device_id armada_3700_periph_clock_of_match[] = {
> + { .compatible = "marvell,armada-3700-periph-clock-nb",
> +   .data = data_nb, },
> + { .compatible = "marvell,armada-3700-periph-clock-sb",
> + .data = data_sb, },
> + { }
> +};
> +static int armada_3700_add_composite_clk(const struct clk_periph_data *data,

Put a newline between the function and array please.

> +  void __iomem *reg, spinlock_t *lock,
> +  struct device *dev, struct clk_hw *hw)
> +{
> + const struct clk_ops *mux_ops = NULL, *gate_ops = NULL,
> + *rate_ops = NULL;
> + struct clk_hw *mux_hw = NULL, *gate_hw = NULL, *rate_hw = NULL;
> +
> + if (data->mux_hw) {
> + struct clk_mux *mux;
> +
> + mux_hw = data->mux_hw;
> + mux = to_clk_mux(mux_hw);
> + mux->lock = lock;
> + mux_ops = mux_hw->init->ops;
> + mux->reg = reg + (u64)mux->reg;

This file spews a ton of sparse errors because of address space
assignment violations due to the reuse of the reg property as a
pointer to iomem and an offset. This style never makes me feel
great because the code may run multiple times if we have
something like probe defer happening and then we're modifying
what is essentially static data (the offset) many times.

Would it be hard to rework this to be more of a descriptor style
that allocates the clk_hw and wrapping structures on the heap
instead? That would avoid these types of problems from cropping
up in the future.

> + }
> +
> + if (data->gate_hw) {
> + struct clk_gate *gate;
> +
> + gate_hw = data->gate_hw;
> + gate = to_clk_gate(gate_hw);
> + gate->lock = lock;
> + gate_ops = gate_hw->init->ops;
> + gate->reg = reg + (u64)gate->reg;
> + }
> +
> + if (data->rate_hw) {
> + rate_hw = data->rate_hw;
> + rate_ops = rate_hw->init->ops;
> + if (data->is_double_div) {
> + struct clk_double_div *rate;
> +
> + rate =  to_clk_double_div(rate_hw);
> + rate->reg1 = reg + (u64)rate->reg1;
> + rate->reg2 = reg + (u64)rate->reg2;
> + } else {
> + struct clk_divider *rate = to_clk_divider(rate_hw);
> + const struct clk_div_table *clkt;
> + int table_size = 0;
> +
> + rate->reg = reg + (u64)rate->reg;
> + for (clkt = rate->table; clkt->div; clkt++)
> + table_size++;
> + rate->width = order_base_2(table_size);
> + rate->lock = lock;
> + }
> + }
> +
> + hw = clk_hw_register_composite(dev, data->name, data->parent_names,
> +data->num_parents, mux_hw,
> +mux_ops, rate_hw, rate_ops,
> +gate_hw, gate_ops, CLK_IGNORE_UNUSED);
> +
> + if (IS_ERR(hw))
> + return PTR_ERR(hw);
> +
> + return 0;

This can be return PTR_ERR_OR_ZERO();

> +}
> +
> +static int armada_3700_periph_clock_probe(struct platform_device *pdev)
> +{
> + struct clk_periph_driver_data *driver_data;
> + struct device_node *np = pdev->dev.of_node;
> + const struct clk_periph_data *data;
> + struct device *dev = >dev;
> + int num_periph = 0, i, ret;
> + struct resource *res;
> + void __iomem *reg;
> +
> + data = of_device_get_match_data(dev);
> + if (!data)
> + return -ENODEV;
> +
> + while (data[num_periph].name)
> + num_periph++;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + reg = devm_ioremap_resource(dev, res);
> + if (IS_ERR(reg)) {
> + dev_err(dev, "Could not map the periph clock registers\n");

devm_ioremap_resource() should already spit out an error.

> + return PTR_ERR(reg);
> + }
-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v3 5/6] dt-bindings: clock: add DT binding for the peripheral clocks on Armada 3700

[Stephen Boyd]

On 07/19, Gregory CLEMENT wrote:
> This commit adds the DT binding documentation for the peripheral clocks
> used in the Marvell Armada 3700 SoCs.
> 
> Signed-off-by: Gregory CLEMENT 
> Acked-by: Rob Herring 
> ---

Applied to clk-next

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v3 5/6] dt-bindings: clock: add DT binding for the peripheral clocks on Armada 3700

[Stephen Boyd]

On 07/19, Gregory CLEMENT wrote:
> This commit adds the DT binding documentation for the peripheral clocks
> used in the Marvell Armada 3700 SoCs.
> 
> Signed-off-by: Gregory CLEMENT 
> Acked-by: Rob Herring 
> ---

Applied to clk-next

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v3 6/6] clk: mvebu: Add the peripheral clock driver for Armada 3700

[Stephen Boyd]

On 07/19, Gregory CLEMENT wrote:
> +
> +static const struct clk_ops clk_double_div_ops = {
> + .recalc_rate = clk_double_div_recalc_rate,
> +};
> +
> +static const struct of_device_id armada_3700_periph_clock_of_match[] = {
> + { .compatible = "marvell,armada-3700-periph-clock-nb",
> +   .data = data_nb, },
> + { .compatible = "marvell,armada-3700-periph-clock-sb",
> + .data = data_sb, },
> + { }
> +};
> +static int armada_3700_add_composite_clk(const struct clk_periph_data *data,

Put a newline between the function and array please.

> +  void __iomem *reg, spinlock_t *lock,
> +  struct device *dev, struct clk_hw *hw)
> +{
> + const struct clk_ops *mux_ops = NULL, *gate_ops = NULL,
> + *rate_ops = NULL;
> + struct clk_hw *mux_hw = NULL, *gate_hw = NULL, *rate_hw = NULL;
> +
> + if (data->mux_hw) {
> + struct clk_mux *mux;
> +
> + mux_hw = data->mux_hw;
> + mux = to_clk_mux(mux_hw);
> + mux->lock = lock;
> + mux_ops = mux_hw->init->ops;
> + mux->reg = reg + (u64)mux->reg;

This file spews a ton of sparse errors because of address space
assignment violations due to the reuse of the reg property as a
pointer to iomem and an offset. This style never makes me feel
great because the code may run multiple times if we have
something like probe defer happening and then we're modifying
what is essentially static data (the offset) many times.

Would it be hard to rework this to be more of a descriptor style
that allocates the clk_hw and wrapping structures on the heap
instead? That would avoid these types of problems from cropping
up in the future.

> + }
> +
> + if (data->gate_hw) {
> + struct clk_gate *gate;
> +
> + gate_hw = data->gate_hw;
> + gate = to_clk_gate(gate_hw);
> + gate->lock = lock;
> + gate_ops = gate_hw->init->ops;
> + gate->reg = reg + (u64)gate->reg;
> + }
> +
> + if (data->rate_hw) {
> + rate_hw = data->rate_hw;
> + rate_ops = rate_hw->init->ops;
> + if (data->is_double_div) {
> + struct clk_double_div *rate;
> +
> + rate =  to_clk_double_div(rate_hw);
> + rate->reg1 = reg + (u64)rate->reg1;
> + rate->reg2 = reg + (u64)rate->reg2;
> + } else {
> + struct clk_divider *rate = to_clk_divider(rate_hw);
> + const struct clk_div_table *clkt;
> + int table_size = 0;
> +
> + rate->reg = reg + (u64)rate->reg;
> + for (clkt = rate->table; clkt->div; clkt++)
> + table_size++;
> + rate->width = order_base_2(table_size);
> + rate->lock = lock;
> + }
> + }
> +
> + hw = clk_hw_register_composite(dev, data->name, data->parent_names,
> +data->num_parents, mux_hw,
> +mux_ops, rate_hw, rate_ops,
> +gate_hw, gate_ops, CLK_IGNORE_UNUSED);
> +
> + if (IS_ERR(hw))
> + return PTR_ERR(hw);
> +
> + return 0;

This can be return PTR_ERR_OR_ZERO();

> +}
> +
> +static int armada_3700_periph_clock_probe(struct platform_device *pdev)
> +{
> + struct clk_periph_driver_data *driver_data;
> + struct device_node *np = pdev->dev.of_node;
> + const struct clk_periph_data *data;
> + struct device *dev = >dev;
> + int num_periph = 0, i, ret;
> + struct resource *res;
> + void __iomem *reg;
> +
> + data = of_device_get_match_data(dev);
> + if (!data)
> + return -ENODEV;
> +
> + while (data[num_periph].name)
> + num_periph++;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + reg = devm_ioremap_resource(dev, res);
> + if (IS_ERR(reg)) {
> + dev_err(dev, "Could not map the periph clock registers\n");

devm_ioremap_resource() should already spit out an error.

> + return PTR_ERR(reg);
> + }
-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [LKP] [lkp] [xfs] 68a9f5e700: aim7.jobs-per-min -13.6% regression

[Dave Chinner]

On Mon, Aug 15, 2016 at 10:14:55PM +0800, Fengguang Wu wrote:
> Hi Christoph,
> 
> On Sun, Aug 14, 2016 at 06:17:24PM +0200, Christoph Hellwig wrote:
> >Snipping the long contest:
> >
> >I think there are three observations here:
> >
> >(1) removing the mark_page_accessed (which is the only significant
> >change in the parent commit)  hurts the
> >aim7/1BRD_48G-xfs-disk_rr-3000-performance/ivb44 test.
> >I'd still rather stick to the filemap version and let the
> >VM people sort it out.  How do the numbers for this test
> >look for XFS vs say ext4 and btrfs?
> >(2) lots of additional spinlock contention in the new case.  A quick
> >check shows that I fat-fingered my rewrite so that we do
> >the xfs_inode_set_eofblocks_tag call now for the pure lookup
> >case, and pretty much all new cycles come from that.
> >(3) Boy, are those xfs_inode_set_eofblocks_tag calls expensive, and
> >we're already doing way to many even without my little bug above.
> >
> >So I've force pushed a new version of the iomap-fixes branch with
> >(2) fixed, and also a little patch to xfs_inode_set_eofblocks_tag a
> >lot less expensive slotted in before that.  Would be good to see
> >the numbers with that.
> 
> The aim7 1BRD tests finished and there are ups and downs, with overall
> performance remain flat.
> 
> 99091700659f4df9  74a242ad94d13436a1644c0b45  bf4dc6e4ecc2a3d042029319bc  
> testcase/testparams/testbox
>   --  --  
> ---

What do these commits refer to, please? They mean nothing without
the commit names

/me goes searching. Ok:

99091700659 is the top of Linus' tree
74a242ad94d is 
bf4dc6e4ecc is the latest in Christoph's tree (because it's
mentioned below)

> %stddev %change %stddev %change %stddev
> \  |\  |
> \ 159926  157324  158574
> GEO-MEAN aim7.jobs-per-min
> 70897   5%  74137   4%  73775
> aim7/1BRD_48G-xfs-creat-clo-1500-performance/ivb44
>485217 ±  3%492431  477533
> aim7/1BRD_48G-xfs-disk_rd-9000-performance/ivb44
>360451 -19% 292980 -17% 299377
> aim7/1BRD_48G-xfs-disk_rr-3000-performance/ivb44

So, why does random read go backwards by 20%? The iomap IO path
patches we are testing only affect the write path, so this
doesn't make a whole lot of sense.

>338114  338410   5% 354078
> aim7/1BRD_48G-xfs-disk_rw-3000-performance/ivb44
> 60130 ±  5% 4%  62438   5%  62923
> aim7/1BRD_48G-xfs-disk_src-3000-performance/ivb44
>403144  397790  410648
> aim7/1BRD_48G-xfs-disk_wrt-3000-performance/ivb44

And this is the test the original regression was reported for:

gcc-6/performance/profile/1BRD_48G/xfs/x86_64-rhel/3000/debian-x86_64-2015-02-07.cgz/ivb44/disk_wrt/aim7

And that shows no improvement at all. The orginal regression was:

484435 ±  0% -13.3% 420004 ±  0%  aim7.jobs-per-min

So it's still 15% down on the orginal performance which, again,
doesn't make a whole lot of sense given the improvement in so many
other tests I've run

> 26327   26534   26128
> aim7/1BRD_48G-xfs-sync_disk_rw-600-performance/ivb44
> 
> The new commit bf4dc6e ("xfs: rewrite and optimize the delalloc write
> path") improves the aim7/1BRD_48G-xfs-disk_rw-3000-performance/ivb44
> case by 5%. Here are the detailed numbers:
> 
> aim7/1BRD_48G-xfs-disk_rw-3000-performance/ivb44

Not important at all. We need the results for the disk_wrt regression
we are chasing (disk_wrt-3000) so we can see how the code change
affected behaviour.

> Here are the detailed numbers for the slowed down case:
> 
> aim7/1BRD_48G-xfs-disk_rr-3000-performance/ivb44
> 
> 99091700659f4df9  bf4dc6e4ecc2a3d042029319bc
>   --
> %stddev  change %stddev
> \  |\
>360451 -17% 299377aim7.jobs-per-min
> 12806 481%  74447
> aim7.time.involuntary_context_switches
.
> 19377 459% 108364vmstat.system.cs
.
>   487 ± 89%  3e+04  26448 ± 57%  
> latency_stats.max.down.xfs_buf_lock._xfs_buf_find.xfs_buf_get_map.xfs_buf_read_map.xfs_trans_read_buf_map.xfs_read_agf.xfs_alloc_read_agf.xfs_alloc_fix_freelist.xfs_free_extent_fix_freelist.xfs_free_extent.xfs_trans_free_extent
>  1823 ± 82%  2e+061913796 ± 38%  
> 

Re: [LKP] [lkp] [xfs] 68a9f5e700: aim7.jobs-per-min -13.6% regression

[Dave Chinner]

On Mon, Aug 15, 2016 at 10:14:55PM +0800, Fengguang Wu wrote:
> Hi Christoph,
> 
> On Sun, Aug 14, 2016 at 06:17:24PM +0200, Christoph Hellwig wrote:
> >Snipping the long contest:
> >
> >I think there are three observations here:
> >
> >(1) removing the mark_page_accessed (which is the only significant
> >change in the parent commit)  hurts the
> >aim7/1BRD_48G-xfs-disk_rr-3000-performance/ivb44 test.
> >I'd still rather stick to the filemap version and let the
> >VM people sort it out.  How do the numbers for this test
> >look for XFS vs say ext4 and btrfs?
> >(2) lots of additional spinlock contention in the new case.  A quick
> >check shows that I fat-fingered my rewrite so that we do
> >the xfs_inode_set_eofblocks_tag call now for the pure lookup
> >case, and pretty much all new cycles come from that.
> >(3) Boy, are those xfs_inode_set_eofblocks_tag calls expensive, and
> >we're already doing way to many even without my little bug above.
> >
> >So I've force pushed a new version of the iomap-fixes branch with
> >(2) fixed, and also a little patch to xfs_inode_set_eofblocks_tag a
> >lot less expensive slotted in before that.  Would be good to see
> >the numbers with that.
> 
> The aim7 1BRD tests finished and there are ups and downs, with overall
> performance remain flat.
> 
> 99091700659f4df9  74a242ad94d13436a1644c0b45  bf4dc6e4ecc2a3d042029319bc  
> testcase/testparams/testbox
>   --  --  
> ---

What do these commits refer to, please? They mean nothing without
the commit names

/me goes searching. Ok:

99091700659 is the top of Linus' tree
74a242ad94d is 
bf4dc6e4ecc is the latest in Christoph's tree (because it's
mentioned below)

> %stddev %change %stddev %change %stddev
> \  |\  |
> \ 159926  157324  158574
> GEO-MEAN aim7.jobs-per-min
> 70897   5%  74137   4%  73775
> aim7/1BRD_48G-xfs-creat-clo-1500-performance/ivb44
>485217 ±  3%492431  477533
> aim7/1BRD_48G-xfs-disk_rd-9000-performance/ivb44
>360451 -19% 292980 -17% 299377
> aim7/1BRD_48G-xfs-disk_rr-3000-performance/ivb44

So, why does random read go backwards by 20%? The iomap IO path
patches we are testing only affect the write path, so this
doesn't make a whole lot of sense.

>338114  338410   5% 354078
> aim7/1BRD_48G-xfs-disk_rw-3000-performance/ivb44
> 60130 ±  5% 4%  62438   5%  62923
> aim7/1BRD_48G-xfs-disk_src-3000-performance/ivb44
>403144  397790  410648
> aim7/1BRD_48G-xfs-disk_wrt-3000-performance/ivb44

And this is the test the original regression was reported for:

gcc-6/performance/profile/1BRD_48G/xfs/x86_64-rhel/3000/debian-x86_64-2015-02-07.cgz/ivb44/disk_wrt/aim7

And that shows no improvement at all. The orginal regression was:

484435 ±  0% -13.3% 420004 ±  0%  aim7.jobs-per-min

So it's still 15% down on the orginal performance which, again,
doesn't make a whole lot of sense given the improvement in so many
other tests I've run

> 26327   26534   26128
> aim7/1BRD_48G-xfs-sync_disk_rw-600-performance/ivb44
> 
> The new commit bf4dc6e ("xfs: rewrite and optimize the delalloc write
> path") improves the aim7/1BRD_48G-xfs-disk_rw-3000-performance/ivb44
> case by 5%. Here are the detailed numbers:
> 
> aim7/1BRD_48G-xfs-disk_rw-3000-performance/ivb44

Not important at all. We need the results for the disk_wrt regression
we are chasing (disk_wrt-3000) so we can see how the code change
affected behaviour.

> Here are the detailed numbers for the slowed down case:
> 
> aim7/1BRD_48G-xfs-disk_rr-3000-performance/ivb44
> 
> 99091700659f4df9  bf4dc6e4ecc2a3d042029319bc
>   --
> %stddev  change %stddev
> \  |\
>360451 -17% 299377aim7.jobs-per-min
> 12806 481%  74447
> aim7.time.involuntary_context_switches
.
> 19377 459% 108364vmstat.system.cs
.
>   487 ± 89%  3e+04  26448 ± 57%  
> latency_stats.max.down.xfs_buf_lock._xfs_buf_find.xfs_buf_get_map.xfs_buf_read_map.xfs_trans_read_buf_map.xfs_read_agf.xfs_alloc_read_agf.xfs_alloc_fix_freelist.xfs_free_extent_fix_freelist.xfs_free_extent.xfs_trans_free_extent
>  1823 ± 82%  2e+061913796 ± 38%  
>