Re: [git pull] drm for v4.7

2016-05-23 Thread Linus Torvalds
On Mon, May 23, 2016 at 11:59 AM, Linus Torvalds
 wrote:
>
> I'll test this out and look what happens, but I hate getting different
> results than what I'm told to expect.

Hmm. I also get a lot of

  ./usr/include/drm/amdgpu_drm.h:38: userspace cannot reference
function or variable defined in the kernel
  ./usr/include/drm/drm.h:63: userspace cannot reference function or
variable defined in the kernel
  ./usr/include/drm/drm.h:699: userspace cannot reference function or
variable defined in the kernel
  ...

warnings with my allmodconfig build. They do seem to be due to
checkpatch not really grokking the

  #if defined(__cplusplus)
  extern "C" {
  #endif

and thinking that's a sign of a kernel function or variable
declaration being exported to user space, but it's a bit annoying.

Is there a patch pending for this that I'm not aware of, or is it just
that nobody but me hates spurious warnings? Didn't this show up in
linux-next? And if it _did_ show up in linux-next, why was the pull
request not talking about it?

 Linus


Re: [git pull] drm for v4.7

2016-05-23 Thread Linus Torvalds
On Mon, May 23, 2016 at 11:59 AM, Linus Torvalds
 wrote:
>
> I'll test this out and look what happens, but I hate getting different
> results than what I'm told to expect.

Hmm. I also get a lot of

  ./usr/include/drm/amdgpu_drm.h:38: userspace cannot reference
function or variable defined in the kernel
  ./usr/include/drm/drm.h:63: userspace cannot reference function or
variable defined in the kernel
  ./usr/include/drm/drm.h:699: userspace cannot reference function or
variable defined in the kernel
  ...

warnings with my allmodconfig build. They do seem to be due to
checkpatch not really grokking the

  #if defined(__cplusplus)
  extern "C" {
  #endif

and thinking that's a sign of a kernel function or variable
declaration being exported to user space, but it's a bit annoying.

Is there a patch pending for this that I'm not aware of, or is it just
that nobody but me hates spurious warnings? Didn't this show up in
linux-next? And if it _did_ show up in linux-next, why was the pull
request not talking about it?

 Linus


Re: [PATCH] Fix RC5 decoding with Fintek CIR chipset

2016-05-23 Thread David Härdeman
On Sat, May 14, 2016 at 06:01:26PM +0100, Jonathan McDowell wrote:
>Fix RC5 decoding with Fintek CIR chipset
>
>Commit e87b540be2dd02552fb9244d50ae8b4e4619a34b tightened up the RC5
>decoding by adding a check for trailing silence to ensure a valid RC5
>command had been received. Unfortunately the trailer length checked was
>10 units and the Fintek CIR device does not want to provide details of a
>space longer than 6350us. This meant that RC5 remotes working on a
>Fintek setup on 3.16 failed on 3.17 and later. Fix this by shortening
>the trailer check to 6 units (allowing for a previous space in the
>received remote command).
>
>Signed-off-by: Jonathan McDowell 
Signed-off-by: David Härdeman 

>Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=117221
>Cc: sta...@vger.kernel.org
>
>-
>diff --git a/drivers/media/rc/ir-rc5-decoder.c 
>b/drivers/media/rc/ir-rc5-decoder.c
>index 6ffe776..a0fd4e6 100644
>--- a/drivers/media/rc/ir-rc5-decoder.c
>+++ b/drivers/media/rc/ir-rc5-decoder.c
>@@ -29,7 +29,7 @@
> #define RC5_BIT_START (1 * RC5_UNIT)
> #define RC5_BIT_END   (1 * RC5_UNIT)
> #define RC5X_SPACE(4 * RC5_UNIT)
>-#define RC5_TRAILER   (10 * RC5_UNIT) /* In reality, approx 100 */
>+#define RC5_TRAILER   (6 * RC5_UNIT) /* In reality, approx 100 */
> 
> enum rc5_state {
>   STATE_INACTIVE,
>-
>
>J.
>
>-- 
>What did the first punk rock girl wear to your school?
>

-- 
David Härdeman


Re: [PATCH] Fix RC5 decoding with Fintek CIR chipset

2016-05-23 Thread David Härdeman
On Sat, May 14, 2016 at 06:01:26PM +0100, Jonathan McDowell wrote:
>Fix RC5 decoding with Fintek CIR chipset
>
>Commit e87b540be2dd02552fb9244d50ae8b4e4619a34b tightened up the RC5
>decoding by adding a check for trailing silence to ensure a valid RC5
>command had been received. Unfortunately the trailer length checked was
>10 units and the Fintek CIR device does not want to provide details of a
>space longer than 6350us. This meant that RC5 remotes working on a
>Fintek setup on 3.16 failed on 3.17 and later. Fix this by shortening
>the trailer check to 6 units (allowing for a previous space in the
>received remote command).
>
>Signed-off-by: Jonathan McDowell 
Signed-off-by: David Härdeman 

>Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=117221
>Cc: sta...@vger.kernel.org
>
>-
>diff --git a/drivers/media/rc/ir-rc5-decoder.c 
>b/drivers/media/rc/ir-rc5-decoder.c
>index 6ffe776..a0fd4e6 100644
>--- a/drivers/media/rc/ir-rc5-decoder.c
>+++ b/drivers/media/rc/ir-rc5-decoder.c
>@@ -29,7 +29,7 @@
> #define RC5_BIT_START (1 * RC5_UNIT)
> #define RC5_BIT_END   (1 * RC5_UNIT)
> #define RC5X_SPACE(4 * RC5_UNIT)
>-#define RC5_TRAILER   (10 * RC5_UNIT) /* In reality, approx 100 */
>+#define RC5_TRAILER   (6 * RC5_UNIT) /* In reality, approx 100 */
> 
> enum rc5_state {
>   STATE_INACTIVE,
>-
>
>J.
>
>-- 
>What did the first punk rock girl wear to your school?
>

-- 
David Härdeman


Re: [PATCH 3/3] mm, thp: make swapin readahead under down_read of mmap_sem

2016-05-23 Thread Kirill A. Shutemov
On Mon, May 23, 2016 at 02:49:09PM -0400, Rik van Riel wrote:
> On Mon, 2016-05-23 at 20:42 +0200, Michal Hocko wrote:
> > On Mon 23-05-16 20:14:11, Ebru Akagunduz wrote:
> > > 
> > > Currently khugepaged makes swapin readahead under
> > > down_write. This patch supplies to make swapin
> > > readahead under down_read instead of down_write.
> > You are still keeping down_write. Can we do without it altogether?
> > Blocking mmap_sem of a remote proces for write is certainly not nice.
> 
> Maybe Andrea can explain why khugepaged requires
> a down_write of mmap_sem?
> 
> If it were possible to have just down_read that
> would make the code a lot simpler.

You need a down_write() to retract page table. We need to make sure that
nobody sees the page table before we can replace it with huge pmd.

-- 
 Kirill A. Shutemov


Re: [PATCH 3/3] mm, thp: make swapin readahead under down_read of mmap_sem

2016-05-23 Thread Kirill A. Shutemov
On Mon, May 23, 2016 at 02:49:09PM -0400, Rik van Riel wrote:
> On Mon, 2016-05-23 at 20:42 +0200, Michal Hocko wrote:
> > On Mon 23-05-16 20:14:11, Ebru Akagunduz wrote:
> > > 
> > > Currently khugepaged makes swapin readahead under
> > > down_write. This patch supplies to make swapin
> > > readahead under down_read instead of down_write.
> > You are still keeping down_write. Can we do without it altogether?
> > Blocking mmap_sem of a remote proces for write is certainly not nice.
> 
> Maybe Andrea can explain why khugepaged requires
> a down_write of mmap_sem?
> 
> If it were possible to have just down_read that
> would make the code a lot simpler.

You need a down_write() to retract page table. We need to make sure that
nobody sees the page table before we can replace it with huge pmd.

-- 
 Kirill A. Shutemov


Re: [git pull] drm for v4.7

2016-05-23 Thread Linus Torvalds
On Sun, May 22, 2016 at 11:41 PM, Dave Airlie  wrote:
>
> Here's the main drm pull request for 4.7, it's been
> a busy one, and I've been a bit more distracted in
> real life this merge window.

Hmm.

I pulled this, but I think I'll have to unpull again.

Neither the diffstat not the shortlog match what you sent me. There's
four extra commits at the top that aren't mentioned:

  Dave Airlie (3):
  drm/edid: move displayid tiled block parsing into separate function.
  drm/edid: move displayid validation to it's own function.
  drm/edid: add displayid detailed 1 timings to the modelist. (v1.1)

  Tomas Bzatek (1):
  drm/displayid: Iterate over all DisplayID blocks

was that intentional? What happened? Are those commits meant to be
merged, or are they wrong? They _look_ ok, but dammit, according to
your message they shouldn't be there.

I'll test this out and look what happens, but I hate getting different
results than what I'm told to expect.

This is one reason I much prefer getting explicit tags rather than a
random branch. Did you update the branch on purpose and wanted me to
get the new state, or did you update the branch just because you
happened to do development on that branch and pushed it out? With an
explicit tag, there's a much more _intentional_ "push this to Linus"
thing going on, and it's less ambiguous in cases like this.

 Linus


Re: [git pull] drm for v4.7

2016-05-23 Thread Linus Torvalds
On Sun, May 22, 2016 at 11:41 PM, Dave Airlie  wrote:
>
> Here's the main drm pull request for 4.7, it's been
> a busy one, and I've been a bit more distracted in
> real life this merge window.

Hmm.

I pulled this, but I think I'll have to unpull again.

Neither the diffstat not the shortlog match what you sent me. There's
four extra commits at the top that aren't mentioned:

  Dave Airlie (3):
  drm/edid: move displayid tiled block parsing into separate function.
  drm/edid: move displayid validation to it's own function.
  drm/edid: add displayid detailed 1 timings to the modelist. (v1.1)

  Tomas Bzatek (1):
  drm/displayid: Iterate over all DisplayID blocks

was that intentional? What happened? Are those commits meant to be
merged, or are they wrong? They _look_ ok, but dammit, according to
your message they shouldn't be there.

I'll test this out and look what happens, but I hate getting different
results than what I'm told to expect.

This is one reason I much prefer getting explicit tags rather than a
random branch. Did you update the branch on purpose and wanted me to
get the new state, or did you update the branch just because you
happened to do development on that branch and pushed it out? With an
explicit tag, there's a much more _intentional_ "push this to Linus"
thing going on, and it's less ambiguous in cases like this.

 Linus


[PATCH] drm/i915/ilk: Disable SSC for DPLLs if we're not using it

2016-05-23 Thread Lyude
Thanks to Ville Syrjälä for pointing me towards the cause of this issue.

Unfortunately one of the sideaffects of having the refclk for a DPLL
set to SSC is that as long as it's set to SSC, the GPU will prevent us
from powering down any of the pipes or transcoders using it. A couple of
BIOSes, despite the fact they don't actually need it, enable SSC both in
PCH_DREF_CONTROL and on the DPLL configurations. This causes issues on
the first modeset, since we don't expect SSC to be left on and as a
result, can't successfully power down the pipes or the transcoders using
it. Here's an example from this Dell OptiPlex 990:

[drm:intel_modeset_init] SSC enabled by BIOS, overriding VBT which says disabled
[drm:intel_modeset_init] 2 display pipes available.
[drm:intel_update_cdclk] Current CD clock rate: 40 kHz
[drm:intel_update_max_cdclk] Max CD clock rate: 40 kHz
[drm:intel_update_max_cdclk] Max dotclock rate: 36 kHz
vgaarb: device changed decodes: 
PCI::00:02.0,olddecodes=io+mem,decodes=io+mem:owns=io+mem
[drm:intel_crt_reset] crt adpa set to 0xf4
[drm:intel_dp_init_connector] Adding DP connector on port C
[drm:intel_dp_aux_init] registering DPDDC-C bus for card0-DP-1
[drm:ironlake_init_pch_refclk] has_panel 0 has_lvds 0 has_ck505 0
[drm:ironlake_init_pch_refclk] Disabling SSC entirely
… later we try committing the first modeset …
[drm:intel_dump_pipe_config] [CRTC:26][modeset] config 88041b02e800 for 
pipe A
[drm:intel_dump_pipe_config] cpu_transcoder: A
…
[drm:intel_dump_pipe_config] dpll_hw_state: dpll: 0xc4016001, dpll_md: 0x0, 
fp0: 0x20e08, fp1: 0x30d07
[drm:intel_dump_pipe_config] planes on this crtc
[drm:intel_dump_pipe_config] STANDARD PLANE:23 plane: 0.0 idx: 0 enabled
[drm:intel_dump_pipe_config] FB:42, fb = 800x600 format = 0x34325258
[drm:intel_dump_pipe_config] scaler:0 src (0, 0) 800x600 dst (0, 0) 800x600
[drm:intel_dump_pipe_config] CURSOR PLANE:25 plane: 0.1 idx: 1 disabled, 
scaler_id = 0
[drm:intel_dump_pipe_config] STANDARD PLANE:27 plane: 0.1 idx: 2 disabled, 
scaler_id = 0
[drm:intel_get_shared_dpll] CRTC:26 allocated PCH DPLL A
[drm:intel_get_shared_dpll] using PCH DPLL A for pipe A
[drm:ilk_audio_codec_disable] Disable audio codec on port C, pipe A
[drm:intel_disable_pipe] disabling pipe A
[ cut here ]
WARNING: CPU: 1 PID: 130 at drivers/gpu/drm/i915/intel_display.c:1146 
intel_disable_pipe+0x297/0x2d0 [i915]
pipe_off wait timed out
…
---[ end trace 94fc8aa03ae139e8 ]---
[drm:intel_dp_link_down]
[drm:ironlake_crtc_disable [i915]] *ERROR* failed to disable transcoder A

Later modesets succeed since they reset the DPLL's configuration anyway,
but this is enough to get stuck with a big fat warning in dmesg.

Normally we'd need to add some sort of ref counting mechanism to the
CRTCs so we avoid accidentally trying to shut off a shared resource, but
since ignore what the BIOS does anyway and we shut off the refclks
before any modesets, this workaround should suffice for now.

Cc: sta...@vger.kernel.org
Signed-off-by: Lyude 
---
 drivers/gpu/drm/i915/intel_display.c | 19 ++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index d500e6f..3fb6025 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8230,7 +8230,8 @@ static void ironlake_init_pch_refclk(struct drm_device 
*dev)
 {
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_encoder *encoder;
-   u32 val, final;
+   int i;
+   u32 val, temp, final;
bool has_lvds = false;
bool has_cpu_edp = false;
bool has_panel = false;
@@ -8369,6 +8370,22 @@ static void ironlake_init_pch_refclk(struct drm_device 
*dev)
I915_WRITE(PCH_DREF_CONTROL, val);
POSTING_READ(PCH_DREF_CONTROL);
udelay(200);
+
+   /* Unset SSC as the refclk for all of the DPLLs */
+   for (i = 0; i < dev_priv->num_shared_dpll; i++) {
+   temp = I915_READ(PCH_DPLL(i));
+
+   if (!(temp & PLLB_REF_INPUT_SPREADSPECTRUMIN))
+   continue;
+
+   DRM_DEBUG_KMS("Disabling SSC refclk for %s\n",
+ dev_priv->shared_dplls[i].name);
+
+   /* Change refclk to DREFCLK */
+   temp &= ~PLL_REF_INPUT_MASK;
+
+   I915_WRITE(PCH_DPLL(i), temp);
+   }
}
 
BUG_ON(val != final);
-- 
2.5.5



[PATCH] drm/i915/ilk: Disable SSC for DPLLs if we're not using it

2016-05-23 Thread Lyude
Thanks to Ville Syrjälä for pointing me towards the cause of this issue.

Unfortunately one of the sideaffects of having the refclk for a DPLL
set to SSC is that as long as it's set to SSC, the GPU will prevent us
from powering down any of the pipes or transcoders using it. A couple of
BIOSes, despite the fact they don't actually need it, enable SSC both in
PCH_DREF_CONTROL and on the DPLL configurations. This causes issues on
the first modeset, since we don't expect SSC to be left on and as a
result, can't successfully power down the pipes or the transcoders using
it. Here's an example from this Dell OptiPlex 990:

[drm:intel_modeset_init] SSC enabled by BIOS, overriding VBT which says disabled
[drm:intel_modeset_init] 2 display pipes available.
[drm:intel_update_cdclk] Current CD clock rate: 40 kHz
[drm:intel_update_max_cdclk] Max CD clock rate: 40 kHz
[drm:intel_update_max_cdclk] Max dotclock rate: 36 kHz
vgaarb: device changed decodes: 
PCI::00:02.0,olddecodes=io+mem,decodes=io+mem:owns=io+mem
[drm:intel_crt_reset] crt adpa set to 0xf4
[drm:intel_dp_init_connector] Adding DP connector on port C
[drm:intel_dp_aux_init] registering DPDDC-C bus for card0-DP-1
[drm:ironlake_init_pch_refclk] has_panel 0 has_lvds 0 has_ck505 0
[drm:ironlake_init_pch_refclk] Disabling SSC entirely
… later we try committing the first modeset …
[drm:intel_dump_pipe_config] [CRTC:26][modeset] config 88041b02e800 for 
pipe A
[drm:intel_dump_pipe_config] cpu_transcoder: A
…
[drm:intel_dump_pipe_config] dpll_hw_state: dpll: 0xc4016001, dpll_md: 0x0, 
fp0: 0x20e08, fp1: 0x30d07
[drm:intel_dump_pipe_config] planes on this crtc
[drm:intel_dump_pipe_config] STANDARD PLANE:23 plane: 0.0 idx: 0 enabled
[drm:intel_dump_pipe_config] FB:42, fb = 800x600 format = 0x34325258
[drm:intel_dump_pipe_config] scaler:0 src (0, 0) 800x600 dst (0, 0) 800x600
[drm:intel_dump_pipe_config] CURSOR PLANE:25 plane: 0.1 idx: 1 disabled, 
scaler_id = 0
[drm:intel_dump_pipe_config] STANDARD PLANE:27 plane: 0.1 idx: 2 disabled, 
scaler_id = 0
[drm:intel_get_shared_dpll] CRTC:26 allocated PCH DPLL A
[drm:intel_get_shared_dpll] using PCH DPLL A for pipe A
[drm:ilk_audio_codec_disable] Disable audio codec on port C, pipe A
[drm:intel_disable_pipe] disabling pipe A
[ cut here ]
WARNING: CPU: 1 PID: 130 at drivers/gpu/drm/i915/intel_display.c:1146 
intel_disable_pipe+0x297/0x2d0 [i915]
pipe_off wait timed out
…
---[ end trace 94fc8aa03ae139e8 ]---
[drm:intel_dp_link_down]
[drm:ironlake_crtc_disable [i915]] *ERROR* failed to disable transcoder A

Later modesets succeed since they reset the DPLL's configuration anyway,
but this is enough to get stuck with a big fat warning in dmesg.

Normally we'd need to add some sort of ref counting mechanism to the
CRTCs so we avoid accidentally trying to shut off a shared resource, but
since ignore what the BIOS does anyway and we shut off the refclks
before any modesets, this workaround should suffice for now.

Cc: sta...@vger.kernel.org
Signed-off-by: Lyude 
---
 drivers/gpu/drm/i915/intel_display.c | 19 ++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index d500e6f..3fb6025 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8230,7 +8230,8 @@ static void ironlake_init_pch_refclk(struct drm_device 
*dev)
 {
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_encoder *encoder;
-   u32 val, final;
+   int i;
+   u32 val, temp, final;
bool has_lvds = false;
bool has_cpu_edp = false;
bool has_panel = false;
@@ -8369,6 +8370,22 @@ static void ironlake_init_pch_refclk(struct drm_device 
*dev)
I915_WRITE(PCH_DREF_CONTROL, val);
POSTING_READ(PCH_DREF_CONTROL);
udelay(200);
+
+   /* Unset SSC as the refclk for all of the DPLLs */
+   for (i = 0; i < dev_priv->num_shared_dpll; i++) {
+   temp = I915_READ(PCH_DPLL(i));
+
+   if (!(temp & PLLB_REF_INPUT_SPREADSPECTRUMIN))
+   continue;
+
+   DRM_DEBUG_KMS("Disabling SSC refclk for %s\n",
+ dev_priv->shared_dplls[i].name);
+
+   /* Change refclk to DREFCLK */
+   temp &= ~PLL_REF_INPUT_MASK;
+
+   I915_WRITE(PCH_DPLL(i), temp);
+   }
}
 
BUG_ON(val != final);
-- 
2.5.5



Re: [PATCH 3/3] mm, thp: make swapin readahead under down_read of mmap_sem

2016-05-23 Thread Rik van Riel
On Mon, 2016-05-23 at 20:42 +0200, Michal Hocko wrote:
> On Mon 23-05-16 20:14:11, Ebru Akagunduz wrote:
> > 
> > Currently khugepaged makes swapin readahead under
> > down_write. This patch supplies to make swapin
> > readahead under down_read instead of down_write.
> You are still keeping down_write. Can we do without it altogether?
> Blocking mmap_sem of a remote proces for write is certainly not nice.

Maybe Andrea can explain why khugepaged requires
a down_write of mmap_sem?

If it were possible to have just down_read that
would make the code a lot simpler.

-- 
All Rights Reversed.



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


Re: [PATCH 3/3] mm, thp: make swapin readahead under down_read of mmap_sem

2016-05-23 Thread Rik van Riel
On Mon, 2016-05-23 at 20:42 +0200, Michal Hocko wrote:
> On Mon 23-05-16 20:14:11, Ebru Akagunduz wrote:
> > 
> > Currently khugepaged makes swapin readahead under
> > down_write. This patch supplies to make swapin
> > readahead under down_read instead of down_write.
> You are still keeping down_write. Can we do without it altogether?
> Blocking mmap_sem of a remote proces for write is certainly not nice.

Maybe Andrea can explain why khugepaged requires
a down_write of mmap_sem?

If it were possible to have just down_read that
would make the code a lot simpler.

-- 
All Rights Reversed.



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


Re: [PATCH v4 2/5] locking/rwsem: Protect all writes to owner by WRITE_ONCE

2016-05-23 Thread Jason Low
On Sat, 2016-05-21 at 09:04 -0700, Peter Hurley wrote:
> On 05/18/2016 12:58 PM, Jason Low wrote:
> > It should be fine to use the standard READ_ONCE here, even if it's just
> > for documentation, as it's probably not going to cost anything in
> > practice. It would be better to avoid adding any special macros for this
> > which may just add more complexity.
> 
> See, I don't understand this line of reasoning at all.
> 
> I read this as "it's ok to be non-optimal here where were spinning CPU
> time but not ok to be non-optimal generally elsewhere where it's
> way less important like at init time".

So I think there is a difference between using it during init time and
using it here where we're spinning. During init time, initializing the
owner field locklessly is normal. No other thread should be concurrently
be writing to the field, since the structure is just getting
initialized, so there are no surprises there.

Our access of the owner field in this function is special in that we're
using a bit of "lockless magic" to read and write to a field that gets
concurrently accessed without any serialization. Since we're not taking
the wait_lock in a scenario where we'd normally would take a lock, it
would be good to have this documented.

> And by the way, it's not just "here" but _everywhere_.
> What about reading ->on_cpu locklessly?

Sure, we could also use READ_ONCE when reading ->on_cpu  :)



Re: [PATCH v4 2/5] locking/rwsem: Protect all writes to owner by WRITE_ONCE

2016-05-23 Thread Jason Low
On Sat, 2016-05-21 at 09:04 -0700, Peter Hurley wrote:
> On 05/18/2016 12:58 PM, Jason Low wrote:
> > It should be fine to use the standard READ_ONCE here, even if it's just
> > for documentation, as it's probably not going to cost anything in
> > practice. It would be better to avoid adding any special macros for this
> > which may just add more complexity.
> 
> See, I don't understand this line of reasoning at all.
> 
> I read this as "it's ok to be non-optimal here where were spinning CPU
> time but not ok to be non-optimal generally elsewhere where it's
> way less important like at init time".

So I think there is a difference between using it during init time and
using it here where we're spinning. During init time, initializing the
owner field locklessly is normal. No other thread should be concurrently
be writing to the field, since the structure is just getting
initialized, so there are no surprises there.

Our access of the owner field in this function is special in that we're
using a bit of "lockless magic" to read and write to a field that gets
concurrently accessed without any serialization. Since we're not taking
the wait_lock in a scenario where we'd normally would take a lock, it
would be good to have this documented.

> And by the way, it's not just "here" but _everywhere_.
> What about reading ->on_cpu locklessly?

Sure, we could also use READ_ONCE when reading ->on_cpu  :)



[PATCH] misc: at24: Fix typo in at24 header file

2016-05-23 Thread Moritz Fischer
This commit fixes a simple typo s/mvmem/nvmem in the
example.

Signed-off-by: Moritz Fischer 
---
 include/linux/platform_data/at24.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/platform_data/at24.h 
b/include/linux/platform_data/at24.h
index dc9a13e..be830b1 100644
--- a/include/linux/platform_data/at24.h
+++ b/include/linux/platform_data/at24.h
@@ -26,7 +26,7 @@
  *
  * An example in pseudo code for a setup() callback:
  *
- * void get_mac_addr(struct mvmem_device *nvmem, void *context)
+ * void get_mac_addr(struct nvmem_device *nvmem, void *context)
  * {
  * u8 *mac_addr = ethernet_pdata->mac_addr;
  * off_t offset = context;
-- 
2.5.5



[PATCH] misc: at24: Fix typo in at24 header file

2016-05-23 Thread Moritz Fischer
This commit fixes a simple typo s/mvmem/nvmem in the
example.

Signed-off-by: Moritz Fischer 
---
 include/linux/platform_data/at24.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/platform_data/at24.h 
b/include/linux/platform_data/at24.h
index dc9a13e..be830b1 100644
--- a/include/linux/platform_data/at24.h
+++ b/include/linux/platform_data/at24.h
@@ -26,7 +26,7 @@
  *
  * An example in pseudo code for a setup() callback:
  *
- * void get_mac_addr(struct mvmem_device *nvmem, void *context)
+ * void get_mac_addr(struct nvmem_device *nvmem, void *context)
  * {
  * u8 *mac_addr = ethernet_pdata->mac_addr;
  * off_t offset = context;
-- 
2.5.5



RE: livepatch: change to a per-task consistency model

2016-05-23 Thread Jiri Kosina
On Mon, 23 May 2016, David Laight wrote:

> Related, please can we have a flag for the sleep and/or process so that
> an uninterruptible sleep doesn't trigger the 'hung task' detector

TASK_KILLABLE

> and also stops the process counting towards the 'load average'.

TASK_NOLOAD

-- 
Jiri Kosina
SUSE Labs



RE: livepatch: change to a per-task consistency model

2016-05-23 Thread Jiri Kosina
On Mon, 23 May 2016, David Laight wrote:

> Related, please can we have a flag for the sleep and/or process so that
> an uninterruptible sleep doesn't trigger the 'hung task' detector

TASK_KILLABLE

> and also stops the process counting towards the 'load average'.

TASK_NOLOAD

-- 
Jiri Kosina
SUSE Labs



Re: [PATCH 3/4] x86: Rewrite switch_to() code

2016-05-23 Thread Josh Poimboeuf
On Mon, May 23, 2016 at 10:03:54AM -0700, Andy Lutomirski wrote:
> On Mon, May 23, 2016 at 9:46 AM, Josh Poimboeuf  wrote:
> > On Mon, May 23, 2016 at 06:49:03AM -0500, Josh Poimboeuf wrote:
> >> On Mon, May 23, 2016 at 06:47:22AM -0500, Josh Poimboeuf wrote:
> >> > On Mon, May 23, 2016 at 07:14:14AM -0400, Brian Gerst wrote:
> >> > > On Sun, May 22, 2016 at 10:34 PM, Josh Poimboeuf  
> >> > > wrote:
> >> > > > On Sun, May 22, 2016 at 10:59:38AM -0700, Andy Lutomirski wrote:
> >> > > >> cc: Josh Poimboeuf: do you care about the exact stack layout of the
> >> > > >> bottom of the stack of an inactive task?
> >> > > >
> >> > > > So there's one minor issue with this patch, relating to unwinding the
> >> > > > stack of a newly forked task.  For detecting reliable stacks, the
> >> > > > unwinder needs to unwind all the way to the syscall pt_regs to make 
> >> > > > sure
> >> > > > the stack is sane.  But for newly forked tasks, that won't be 
> >> > > > possible
> >> > > > here because the unwinding will stop at the fork_frame instead.
> >> > > >
> >> > > > So from an unwinder standpoint it might be nice for 
> >> > > > copy_thread_tls() to
> >> > > > place a frame pointer on the stack next to the ret_from_fork return
> >> > > > address, so that it would resemble an actual stack frame.  The frame
> >> > > > pointer could probably just be hard-coded to zero.  And then the 
> >> > > > first
> >> > > > bp in fork_frame would need to be a pointer to it instead of zero.  
> >> > > > That
> >> > > > would make it nicely resemble the stack of any other task.
> >> > > >
> >> > > > Alternatively I could teach the unwinder that if the unwinding 
> >> > > > starts at
> >> > > > the fork_frame offset from the end of the stack page, and the saved 
> >> > > > rbp
> >> > > > is zero, it can assume that it's a newly forked task.  But that 
> >> > > > seems a
> >> > > > little more brittle to me, as it requires the unwinder to understand
> >> > > > more of the internal workings of the fork code.
> >> > > >
> >> > > > But overall I think this patch is a really nice cleanup, and other 
> >> > > > than
> >> > > > the above minor issue it should be fine with my reliable unwinder, 
> >> > > > since
> >> > > > rbp is still at the top of the stack.
> >> > >
> >> > > Ok, how about if it pushed RBP first, then we teach get_wchan() to add
> >> > > the fixed offset from thread.sp to get bp?  that way it don't have to
> >> > > push it twice.
> >> >
> >> > In theory I like the idea, and it would work: the unwinder could just
> >> > use the inactive_task_frame struct (as Andy suggested) to find the frame
> >> > pointer.
> >> >
> >> > But I suspect it would break all existing unwinders, both in-tree and
> >> > out-of-tree.  The only out-of-tree one I know of is crash, not sure if
> >> > there are more out there.
> >>
> >> I should mention it would only affect those unwinders which know how to
> >> do sleeping kernel tasks.  So generic tools like gdb wouldn't be
> >> affected.
> >
> > [continuing my conversation with myself...]
> >
> > To clarify, I still think we should do it.  The stack format of a
> > sleeping task isn't exactly an ABI, and I wouldn't expect many tools to
> > rely on it.  I can help with the fixing of in-tree unwinders if needed.
> >
> > Or I could even do the moving of the frame pointer as a separate patch
> > on top of this one, since it might cause breakage elsewhere.
> 
> Do you have any understanding of why there are so many unwinder
> implementations?  Your reliable unwinder seems to be yet another copy
> of more or less the same code.

Yeah, there are way too many instantations of stacktrace_ops and there's
definitely a lot of room for consolidation and simplification.  There
are different requirements needed by all the different codes relying on
dump_trace():

- starting with a given pt_regs
- starting with a given task
- whether to skip sched code functions
- whether to skip the random '?' ktext addresses found on the stack
- whether frame pointers are enabled
- whether the stack is reliable
- output to an arch-independent "struct stack_trace" array

So everybody implements their own callbacks for dump_trace().  It's kind
of a big mess.

> I'd like to see a single, high-quality unwinder implemented as a state
> machine, along the lines of:
> 
> struct unwind_state state;
> unwind_start_inactive_task(, ...); or
> unwind_start_pt_regs(, regs); or whatever.
> unwind_next_frame();
> 
> where, after unwind_next_frame, state encodes whatever registers are
> known (at least bp and ip, but all the GPRs would be nice and are
> probably mandatory for DWARF) and an indication of whether this is a
> real frame or a guessed frame (the things that currently show up as
> '?').

I like the idea of a state machine.  I'll probably end up doing
something like that before introducing the DWARF unwinder.

-- 
Josh


Re: [PATCH 3/4] x86: Rewrite switch_to() code

2016-05-23 Thread Josh Poimboeuf
On Mon, May 23, 2016 at 10:03:54AM -0700, Andy Lutomirski wrote:
> On Mon, May 23, 2016 at 9:46 AM, Josh Poimboeuf  wrote:
> > On Mon, May 23, 2016 at 06:49:03AM -0500, Josh Poimboeuf wrote:
> >> On Mon, May 23, 2016 at 06:47:22AM -0500, Josh Poimboeuf wrote:
> >> > On Mon, May 23, 2016 at 07:14:14AM -0400, Brian Gerst wrote:
> >> > > On Sun, May 22, 2016 at 10:34 PM, Josh Poimboeuf  
> >> > > wrote:
> >> > > > On Sun, May 22, 2016 at 10:59:38AM -0700, Andy Lutomirski wrote:
> >> > > >> cc: Josh Poimboeuf: do you care about the exact stack layout of the
> >> > > >> bottom of the stack of an inactive task?
> >> > > >
> >> > > > So there's one minor issue with this patch, relating to unwinding the
> >> > > > stack of a newly forked task.  For detecting reliable stacks, the
> >> > > > unwinder needs to unwind all the way to the syscall pt_regs to make 
> >> > > > sure
> >> > > > the stack is sane.  But for newly forked tasks, that won't be 
> >> > > > possible
> >> > > > here because the unwinding will stop at the fork_frame instead.
> >> > > >
> >> > > > So from an unwinder standpoint it might be nice for 
> >> > > > copy_thread_tls() to
> >> > > > place a frame pointer on the stack next to the ret_from_fork return
> >> > > > address, so that it would resemble an actual stack frame.  The frame
> >> > > > pointer could probably just be hard-coded to zero.  And then the 
> >> > > > first
> >> > > > bp in fork_frame would need to be a pointer to it instead of zero.  
> >> > > > That
> >> > > > would make it nicely resemble the stack of any other task.
> >> > > >
> >> > > > Alternatively I could teach the unwinder that if the unwinding 
> >> > > > starts at
> >> > > > the fork_frame offset from the end of the stack page, and the saved 
> >> > > > rbp
> >> > > > is zero, it can assume that it's a newly forked task.  But that 
> >> > > > seems a
> >> > > > little more brittle to me, as it requires the unwinder to understand
> >> > > > more of the internal workings of the fork code.
> >> > > >
> >> > > > But overall I think this patch is a really nice cleanup, and other 
> >> > > > than
> >> > > > the above minor issue it should be fine with my reliable unwinder, 
> >> > > > since
> >> > > > rbp is still at the top of the stack.
> >> > >
> >> > > Ok, how about if it pushed RBP first, then we teach get_wchan() to add
> >> > > the fixed offset from thread.sp to get bp?  that way it don't have to
> >> > > push it twice.
> >> >
> >> > In theory I like the idea, and it would work: the unwinder could just
> >> > use the inactive_task_frame struct (as Andy suggested) to find the frame
> >> > pointer.
> >> >
> >> > But I suspect it would break all existing unwinders, both in-tree and
> >> > out-of-tree.  The only out-of-tree one I know of is crash, not sure if
> >> > there are more out there.
> >>
> >> I should mention it would only affect those unwinders which know how to
> >> do sleeping kernel tasks.  So generic tools like gdb wouldn't be
> >> affected.
> >
> > [continuing my conversation with myself...]
> >
> > To clarify, I still think we should do it.  The stack format of a
> > sleeping task isn't exactly an ABI, and I wouldn't expect many tools to
> > rely on it.  I can help with the fixing of in-tree unwinders if needed.
> >
> > Or I could even do the moving of the frame pointer as a separate patch
> > on top of this one, since it might cause breakage elsewhere.
> 
> Do you have any understanding of why there are so many unwinder
> implementations?  Your reliable unwinder seems to be yet another copy
> of more or less the same code.

Yeah, there are way too many instantations of stacktrace_ops and there's
definitely a lot of room for consolidation and simplification.  There
are different requirements needed by all the different codes relying on
dump_trace():

- starting with a given pt_regs
- starting with a given task
- whether to skip sched code functions
- whether to skip the random '?' ktext addresses found on the stack
- whether frame pointers are enabled
- whether the stack is reliable
- output to an arch-independent "struct stack_trace" array

So everybody implements their own callbacks for dump_trace().  It's kind
of a big mess.

> I'd like to see a single, high-quality unwinder implemented as a state
> machine, along the lines of:
> 
> struct unwind_state state;
> unwind_start_inactive_task(, ...); or
> unwind_start_pt_regs(, regs); or whatever.
> unwind_next_frame();
> 
> where, after unwind_next_frame, state encodes whatever registers are
> known (at least bp and ip, but all the GPRs would be nice and are
> probably mandatory for DWARF) and an indication of whether this is a
> real frame or a guessed frame (the things that currently show up as
> '?').

I like the idea of a state machine.  I'll probably end up doing
something like that before introducing the DWARF unwinder.

-- 
Josh


Re: [PATCH 3/3] mm, thp: make swapin readahead under down_read of mmap_sem

2016-05-23 Thread Michal Hocko
On Mon 23-05-16 20:14:11, Ebru Akagunduz wrote:
> Currently khugepaged makes swapin readahead under
> down_write. This patch supplies to make swapin
> readahead under down_read instead of down_write.

You are still keeping down_write. Can we do without it altogether?
Blocking mmap_sem of a remote proces for write is certainly not nice.
-- 
Michal Hocko
SUSE Labs


Re: [PATCH 3/3] mm, thp: make swapin readahead under down_read of mmap_sem

2016-05-23 Thread Michal Hocko
On Mon 23-05-16 20:14:11, Ebru Akagunduz wrote:
> Currently khugepaged makes swapin readahead under
> down_write. This patch supplies to make swapin
> readahead under down_read instead of down_write.

You are still keeping down_write. Can we do without it altogether?
Blocking mmap_sem of a remote proces for write is certainly not nice.
-- 
Michal Hocko
SUSE Labs


[v2 PATCH] mm: make CONFIG_DEFERRED_STRUCT_PAGE_INIT depends on !FLATMEM explicitly

2016-05-23 Thread Yang Shi
Per the suggestion from Michal Hocko [1], DEFERRED_STRUCT_PAGE_INIT requires
some ordering wrt other initialization operations, e.g. page_ext_init has to
happen after the whole memmap is initialized properly. For SPARSEMEM this
requires to wait for page_alloc_init_late. Other memory models (e.g. flatmem)
might have different initialization layouts (page_ext_init_flatmem). Currently
DEFERRED_STRUCT_PAGE_INIT depends on MEMORY_HOTPLUG which in turn
depends on SPARSEMEM || X86_64_ACPI_NUMA
depends on ARCH_ENABLE_MEMORY_HOTPLUG

and X86_64_ACPI_NUMA depends on NUMA which in turn disable FLATMEM
memory model:
config ARCH_FLATMEM_ENABLE
def_bool y
depends on X86_32 && !NUMA

so FLATMEM is ruled out via dependency maze. Be explicit and disable
FLATMEM for DEFERRED_STRUCT_PAGE_INIT so that we do not reintroduce
subtle initialization bugs

[1] http://lkml.kernel.org/r/20160523073157.gd2...@dhcp22.suse.cz

CC: Michal Hocko 
Signed-off-by: Yang Shi 
---
v2:
 Adopted Michal's comments for the commit log

 mm/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/mm/Kconfig b/mm/Kconfig
index 2664c11..22fa818 100644
--- a/mm/Kconfig
+++ b/mm/Kconfig
@@ -649,6 +649,7 @@ config DEFERRED_STRUCT_PAGE_INIT
default n
depends on ARCH_SUPPORTS_DEFERRED_STRUCT_PAGE_INIT
depends on MEMORY_HOTPLUG
+   depends on !FLATMEM
help
  Ordinarily all struct pages are initialised during early boot in a
  single thread. On very large machines this can take a considerable
-- 
2.0.2



[v2 PATCH] mm: make CONFIG_DEFERRED_STRUCT_PAGE_INIT depends on !FLATMEM explicitly

2016-05-23 Thread Yang Shi
Per the suggestion from Michal Hocko [1], DEFERRED_STRUCT_PAGE_INIT requires
some ordering wrt other initialization operations, e.g. page_ext_init has to
happen after the whole memmap is initialized properly. For SPARSEMEM this
requires to wait for page_alloc_init_late. Other memory models (e.g. flatmem)
might have different initialization layouts (page_ext_init_flatmem). Currently
DEFERRED_STRUCT_PAGE_INIT depends on MEMORY_HOTPLUG which in turn
depends on SPARSEMEM || X86_64_ACPI_NUMA
depends on ARCH_ENABLE_MEMORY_HOTPLUG

and X86_64_ACPI_NUMA depends on NUMA which in turn disable FLATMEM
memory model:
config ARCH_FLATMEM_ENABLE
def_bool y
depends on X86_32 && !NUMA

so FLATMEM is ruled out via dependency maze. Be explicit and disable
FLATMEM for DEFERRED_STRUCT_PAGE_INIT so that we do not reintroduce
subtle initialization bugs

[1] http://lkml.kernel.org/r/20160523073157.gd2...@dhcp22.suse.cz

CC: Michal Hocko 
Signed-off-by: Yang Shi 
---
v2:
 Adopted Michal's comments for the commit log

 mm/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/mm/Kconfig b/mm/Kconfig
index 2664c11..22fa818 100644
--- a/mm/Kconfig
+++ b/mm/Kconfig
@@ -649,6 +649,7 @@ config DEFERRED_STRUCT_PAGE_INIT
default n
depends on ARCH_SUPPORTS_DEFERRED_STRUCT_PAGE_INIT
depends on MEMORY_HOTPLUG
+   depends on !FLATMEM
help
  Ordinarily all struct pages are initialised during early boot in a
  single thread. On very large machines this can take a considerable
-- 
2.0.2



[RFC 4/7] iio: Add current_trigger_id alternative

2016-05-23 Thread Crestez Dan Leonard
This allows controlling the current trigger by numeric ID rather than
name.

Signed-off-by: Crestez Dan Leonard 
---
 Documentation/ABI/testing/sysfs-bus-iio |   9 +++
 Documentation/DocBook/iio.tmpl  |   4 +-
 drivers/iio/industrialio-trigger.c  | 115 
 3 files changed, 98 insertions(+), 30 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-bus-iio 
b/Documentation/ABI/testing/sysfs-bus-iio
index df44998..e276032 100644
--- a/Documentation/ABI/testing/sysfs-bus-iio
+++ b/Documentation/ABI/testing/sysfs-bus-iio
@@ -1038,6 +1038,15 @@ Description:
The name of the trigger source being used, as per string given
in /sys/class/iio/triggerY/name.
 
+What:  /sys/bus/iio/devices/iio:deviceX/trigger/current_trigger_id
+KernelVersion: 4.7
+Contact:   linux-...@vger.kernel.org
+Description:
+   The id of the trigger source being used. This is the Y from
+   /sys/class/iio/triggerY. This is an alternative to
+   /sys/bus/iio/devices/iio:deviceX/trigger/current_trigger.
+   Write a negative number to clear.
+
 What:  /sys/bus/iio/devices/iio:deviceX/buffer/length
 KernelVersion: 2.6.35
 Contact:   linux-...@vger.kernel.org
diff --git a/Documentation/DocBook/iio.tmpl b/Documentation/DocBook/iio.tmpl
index f525bf5..599eff0 100644
--- a/Documentation/DocBook/iio.tmpl
+++ b/Documentation/DocBook/iio.tmpl
@@ -523,7 +523,9 @@
   /sys/bus/iio/devices/iio:deviceX/trigger/, this
   directory is created once the device supports a triggered
   buffer. We can associate a trigger with our device by writing
-  the trigger's name in the current_trigger file.
+  the trigger's name in the current_trigger
+  file. Alternatively we can write the numeric id Y from
+  triggerY into current_trigger_id
 
   
 
diff --git a/drivers/iio/industrialio-trigger.c 
b/drivers/iio/industrialio-trigger.c
index ae2806a..e79c64c 100644
--- a/drivers/iio/industrialio-trigger.c
+++ b/drivers/iio/industrialio-trigger.c
@@ -121,6 +121,21 @@ static struct iio_trigger *iio_trigger_find_by_name(const 
char *name,
return trig;
 }
 
+static struct iio_trigger *iio_trigger_find_by_id(int id)
+{
+   struct iio_trigger *trig = NULL, *iter;
+
+   mutex_lock(_trigger_list_lock);
+   list_for_each_entry(iter, _trigger_list, list)
+   if (iter->id == id) {
+   trig = iter;
+   break;
+   }
+   mutex_unlock(_trigger_list_lock);
+
+   return trig;
+}
+
 void iio_trigger_poll(struct iio_trigger *trig)
 {
int i;
@@ -294,7 +309,7 @@ void iio_dealloc_pollfunc(struct iio_poll_func *pf)
 EXPORT_SYMBOL_GPL(iio_dealloc_pollfunc);
 
 /**
- * iio_trigger_read_current() - trigger consumer sysfs query current trigger
+ * iio_trigger_read_current_name() - trigger consumer sysfs query current 
trigger
  * @dev:   device associated with an industrial I/O device
  * @attr:  pointer to the device_attribute structure that
  * is being processed
@@ -306,9 +321,9 @@ EXPORT_SYMBOL_GPL(iio_dealloc_pollfunc);
  * Return: a negative number on failure, the number of characters written
  *on success or 0 if no trigger is available
  */
-static ssize_t iio_trigger_read_current(struct device *dev,
-   struct device_attribute *attr,
-   char *buf)
+static ssize_t iio_trigger_read_current_name(struct device *dev,
+struct device_attribute *attr,
+char *buf)
 {
struct iio_dev *indio_dev = dev_to_iio_dev(dev);
 
@@ -317,28 +332,22 @@ static ssize_t iio_trigger_read_current(struct device 
*dev,
return 0;
 }
 
-/**
- * iio_trigger_write_current() - trigger consumer sysfs set current trigger
- * @dev:   device associated with an industrial I/O device
- * @attr:  device attribute that is being processed
- * @buf:   string buffer that holds the name of the trigger
- * @len:   length of the trigger name held by buf
- *
- * For trigger consumers the current_trigger interface allows the trigger
- * used for this device to be specified at run time based on the trigger's
- * name.
- *
- * Return: negative error code on failure or length of the buffer
- *on success
- */
-static ssize_t iio_trigger_write_current(struct device *dev,
-struct device_attribute *attr,
-const char *buf,
-size_t len)
+static ssize_t iio_trigger_read_current_id(struct device *dev,
+  struct device_attribute *attr,
+  char *buf)
 {
struct iio_dev *indio_dev = 

[RFC 6/7] iio: Refuse to register triggers with duplicate names

2016-05-23 Thread Crestez Dan Leonard
The trigger name is documented as unique but drivers are currently
allowed to register triggers with duplicate names. This should be
considered a bug since it makes the 'current_trigger' interface
unusable.

Signed-off-by: Crestez Dan Leonard 
---
 drivers/iio/industrialio-trigger.c | 21 +
 1 file changed, 21 insertions(+)

diff --git a/drivers/iio/industrialio-trigger.c 
b/drivers/iio/industrialio-trigger.c
index e79c64c..e77503c 100644
--- a/drivers/iio/industrialio-trigger.c
+++ b/drivers/iio/industrialio-trigger.c
@@ -64,6 +64,8 @@ static struct attribute *iio_trig_dev_attrs[] = {
 };
 ATTRIBUTE_GROUPS(iio_trig_dev);
 
+static struct iio_trigger *__iio_trigger_find_by_name(const char *name);
+
 int iio_trigger_register(struct iio_trigger *trig_info)
 {
int ret;
@@ -82,11 +84,18 @@ int iio_trigger_register(struct iio_trigger *trig_info)
 
/* Add to list of available triggers held by the IIO core */
mutex_lock(_trigger_list_lock);
+   if (__iio_trigger_find_by_name(trig_info->name)) {
+   pr_err("Duplicate trigger name '%s'\n", trig_info->name);
+   ret = -EEXIST;
+   goto error_device_del;
+   }
list_add_tail(_info->list, _trigger_list);
mutex_unlock(_trigger_list_lock);
 
return 0;
 
+error_device_del:
+   device_del(_info->dev);
 error_unregister_id:
ida_simple_remove(_trigger_ida, trig_info->id);
return ret;
@@ -105,6 +114,18 @@ void iio_trigger_unregister(struct iio_trigger *trig_info)
 }
 EXPORT_SYMBOL(iio_trigger_unregister);
 
+/* Search for trigger by name, assuming iio_trigger_list_lock held */
+static struct iio_trigger *__iio_trigger_find_by_name(const char *name)
+{
+   struct iio_trigger *iter;
+
+   list_for_each_entry(iter, _trigger_list, list)
+   if (!strcmp(iter->name, name))
+   return iter;
+
+   return NULL;
+}
+
 static struct iio_trigger *iio_trigger_find_by_name(const char *name,
size_t len)
 {
-- 
2.5.5



[RFC 4/7] iio: Add current_trigger_id alternative

2016-05-23 Thread Crestez Dan Leonard
This allows controlling the current trigger by numeric ID rather than
name.

Signed-off-by: Crestez Dan Leonard 
---
 Documentation/ABI/testing/sysfs-bus-iio |   9 +++
 Documentation/DocBook/iio.tmpl  |   4 +-
 drivers/iio/industrialio-trigger.c  | 115 
 3 files changed, 98 insertions(+), 30 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-bus-iio 
b/Documentation/ABI/testing/sysfs-bus-iio
index df44998..e276032 100644
--- a/Documentation/ABI/testing/sysfs-bus-iio
+++ b/Documentation/ABI/testing/sysfs-bus-iio
@@ -1038,6 +1038,15 @@ Description:
The name of the trigger source being used, as per string given
in /sys/class/iio/triggerY/name.
 
+What:  /sys/bus/iio/devices/iio:deviceX/trigger/current_trigger_id
+KernelVersion: 4.7
+Contact:   linux-...@vger.kernel.org
+Description:
+   The id of the trigger source being used. This is the Y from
+   /sys/class/iio/triggerY. This is an alternative to
+   /sys/bus/iio/devices/iio:deviceX/trigger/current_trigger.
+   Write a negative number to clear.
+
 What:  /sys/bus/iio/devices/iio:deviceX/buffer/length
 KernelVersion: 2.6.35
 Contact:   linux-...@vger.kernel.org
diff --git a/Documentation/DocBook/iio.tmpl b/Documentation/DocBook/iio.tmpl
index f525bf5..599eff0 100644
--- a/Documentation/DocBook/iio.tmpl
+++ b/Documentation/DocBook/iio.tmpl
@@ -523,7 +523,9 @@
   /sys/bus/iio/devices/iio:deviceX/trigger/, this
   directory is created once the device supports a triggered
   buffer. We can associate a trigger with our device by writing
-  the trigger's name in the current_trigger file.
+  the trigger's name in the current_trigger
+  file. Alternatively we can write the numeric id Y from
+  triggerY into current_trigger_id
 
   
 
diff --git a/drivers/iio/industrialio-trigger.c 
b/drivers/iio/industrialio-trigger.c
index ae2806a..e79c64c 100644
--- a/drivers/iio/industrialio-trigger.c
+++ b/drivers/iio/industrialio-trigger.c
@@ -121,6 +121,21 @@ static struct iio_trigger *iio_trigger_find_by_name(const 
char *name,
return trig;
 }
 
+static struct iio_trigger *iio_trigger_find_by_id(int id)
+{
+   struct iio_trigger *trig = NULL, *iter;
+
+   mutex_lock(_trigger_list_lock);
+   list_for_each_entry(iter, _trigger_list, list)
+   if (iter->id == id) {
+   trig = iter;
+   break;
+   }
+   mutex_unlock(_trigger_list_lock);
+
+   return trig;
+}
+
 void iio_trigger_poll(struct iio_trigger *trig)
 {
int i;
@@ -294,7 +309,7 @@ void iio_dealloc_pollfunc(struct iio_poll_func *pf)
 EXPORT_SYMBOL_GPL(iio_dealloc_pollfunc);
 
 /**
- * iio_trigger_read_current() - trigger consumer sysfs query current trigger
+ * iio_trigger_read_current_name() - trigger consumer sysfs query current 
trigger
  * @dev:   device associated with an industrial I/O device
  * @attr:  pointer to the device_attribute structure that
  * is being processed
@@ -306,9 +321,9 @@ EXPORT_SYMBOL_GPL(iio_dealloc_pollfunc);
  * Return: a negative number on failure, the number of characters written
  *on success or 0 if no trigger is available
  */
-static ssize_t iio_trigger_read_current(struct device *dev,
-   struct device_attribute *attr,
-   char *buf)
+static ssize_t iio_trigger_read_current_name(struct device *dev,
+struct device_attribute *attr,
+char *buf)
 {
struct iio_dev *indio_dev = dev_to_iio_dev(dev);
 
@@ -317,28 +332,22 @@ static ssize_t iio_trigger_read_current(struct device 
*dev,
return 0;
 }
 
-/**
- * iio_trigger_write_current() - trigger consumer sysfs set current trigger
- * @dev:   device associated with an industrial I/O device
- * @attr:  device attribute that is being processed
- * @buf:   string buffer that holds the name of the trigger
- * @len:   length of the trigger name held by buf
- *
- * For trigger consumers the current_trigger interface allows the trigger
- * used for this device to be specified at run time based on the trigger's
- * name.
- *
- * Return: negative error code on failure or length of the buffer
- *on success
- */
-static ssize_t iio_trigger_write_current(struct device *dev,
-struct device_attribute *attr,
-const char *buf,
-size_t len)
+static ssize_t iio_trigger_read_current_id(struct device *dev,
+  struct device_attribute *attr,
+  char *buf)
 {
struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+
+   

[RFC 6/7] iio: Refuse to register triggers with duplicate names

2016-05-23 Thread Crestez Dan Leonard
The trigger name is documented as unique but drivers are currently
allowed to register triggers with duplicate names. This should be
considered a bug since it makes the 'current_trigger' interface
unusable.

Signed-off-by: Crestez Dan Leonard 
---
 drivers/iio/industrialio-trigger.c | 21 +
 1 file changed, 21 insertions(+)

diff --git a/drivers/iio/industrialio-trigger.c 
b/drivers/iio/industrialio-trigger.c
index e79c64c..e77503c 100644
--- a/drivers/iio/industrialio-trigger.c
+++ b/drivers/iio/industrialio-trigger.c
@@ -64,6 +64,8 @@ static struct attribute *iio_trig_dev_attrs[] = {
 };
 ATTRIBUTE_GROUPS(iio_trig_dev);
 
+static struct iio_trigger *__iio_trigger_find_by_name(const char *name);
+
 int iio_trigger_register(struct iio_trigger *trig_info)
 {
int ret;
@@ -82,11 +84,18 @@ int iio_trigger_register(struct iio_trigger *trig_info)
 
/* Add to list of available triggers held by the IIO core */
mutex_lock(_trigger_list_lock);
+   if (__iio_trigger_find_by_name(trig_info->name)) {
+   pr_err("Duplicate trigger name '%s'\n", trig_info->name);
+   ret = -EEXIST;
+   goto error_device_del;
+   }
list_add_tail(_info->list, _trigger_list);
mutex_unlock(_trigger_list_lock);
 
return 0;
 
+error_device_del:
+   device_del(_info->dev);
 error_unregister_id:
ida_simple_remove(_trigger_ida, trig_info->id);
return ret;
@@ -105,6 +114,18 @@ void iio_trigger_unregister(struct iio_trigger *trig_info)
 }
 EXPORT_SYMBOL(iio_trigger_unregister);
 
+/* Search for trigger by name, assuming iio_trigger_list_lock held */
+static struct iio_trigger *__iio_trigger_find_by_name(const char *name)
+{
+   struct iio_trigger *iter;
+
+   list_for_each_entry(iter, _trigger_list, list)
+   if (!strcmp(iter->name, name))
+   return iter;
+
+   return NULL;
+}
+
 static struct iio_trigger *iio_trigger_find_by_name(const char *name,
size_t len)
 {
-- 
2.5.5



[PATCHv3 3/7] iio: generic_buffer: Add --trigger-num option

2016-05-23 Thread Crestez Dan Leonard
Signed-off-by: Crestez Dan Leonard 
---
 tools/iio/generic_buffer.c | 34 +-
 1 file changed, 29 insertions(+), 5 deletions(-)

diff --git a/tools/iio/generic_buffer.c b/tools/iio/generic_buffer.c
index 3f16e9f..e8c3052 100644
--- a/tools/iio/generic_buffer.c
+++ b/tools/iio/generic_buffer.c
@@ -254,7 +254,9 @@ void print_usage(void)
"  --device-name -n \n"
"  --device-num -N \n"
"Set device by name or number (mandatory)\n"
-   "  -t   Set trigger name\n"
+   "  --trigger-name -t \n"
+   "  --trigger-num -T \n"
+   "Set trigger by name or number\n"
"  -w  Set delay between reads in us (event-less 
mode)\n");
 }
 
@@ -320,6 +322,8 @@ void register_cleanup(void)
 static const struct option longopts[] = {
{ "device-name",1, 0, 'n' },
{ "device-num", 1, 0, 'N' },
+   { "trigger-name",   1, 0, 't' },
+   { "trigger-num",1, 0, 'T' },
{ },
 };
 
@@ -348,7 +352,7 @@ int main(int argc, char **argv)
 
register_cleanup();
 
-   while ((c = getopt_long(argc, argv, "ac:egl:n:N:t:w:", longopts, NULL)) 
!= -1) {
+   while ((c = getopt_long(argc, argv, "ac:egl:n:N:t:T:w:", longopts, 
NULL)) != -1) {
switch (c) {
case 'a':
autochannels = AUTOCHANNELS_ENABLED;
@@ -391,6 +395,12 @@ int main(int argc, char **argv)
case 't':
trigger_name = strdup(optarg);
break;
+   case 'T':
+   errno = 0;
+   trig_num = strtoul(optarg, , 10);
+   if (errno)
+   return -errno;
+   break;
case 'w':
errno = 0;
timedelay = strtoul(optarg, , 10);
@@ -444,7 +454,23 @@ int main(int argc, char **argv)
}
}
 
-   if (!notrigger) {
+   if (notrigger) {
+   printf("trigger-less mode selected\n");
+   } if (trig_num > 0) {
+   char *trig_dev_name;
+   ret = asprintf(_dev_name, "%strigger%d", iio_dir, 
trig_num);
+   if (ret < 0) {
+   return -ENOMEM;
+   }
+   trigger_name = malloc(IIO_MAX_NAME_LENGTH);
+   ret = read_sysfs_string("name", trig_dev_name, trigger_name);
+   free(trig_dev_name);
+   if (ret < 0) {
+   fprintf(stderr, "Failed to read trigger%d name from\n", 
trig_num);
+   return ret;
+   }
+   printf("iio trigger number being used is %d\n", trig_num);
+   } else {
if (!trigger_name) {
/*
 * Build the trigger name. If it is device associated
@@ -481,8 +507,6 @@ int main(int argc, char **argv)
}
 
printf("iio trigger number being used is %d\n", trig_num);
-   } else {
-   printf("trigger-less mode selected\n");
}
 
/*
-- 
2.5.5



[RFC 0/7] Deal with iio trigger names

2016-05-23 Thread Crestez Dan Leonard
IIO documents that trigger names are unique but does not actually guarantee
this. You can easily create a software trigger with a duplicate name if you
enable CONFIG_IIO_HRTIMER_TRIGGER:

mkdir /sys/kernel/config/iio/triggers/hrtimer/\
`cat /sys/bus/iio/devices/trigger0/name`

You can also get in this situation if you connect two identical st_sensors.

My attempt to fix this is by adding a new 'current_trigger_id' ABI which is a
numeric ID (X from triggerX) rather than a non-unique string. I tested that
this works as expected when connecting two LIS3DH to the same system. This is
in patches 4,5.

In theory the current_trigger ABI could be overloaded to looks for something
matching "trigger[0-9]+" but this would be nastier. There would be a need to
prevent registering triggers that match that expression in order to prevent
stuff like:

mkdir /sys/kernel/config/iio/triggers/hrtimer/trigger1

Such an overload wouldn't work on read and make it impossible to unambigiously
determine the currently selected trigger.


Patch 6 disallows registering duplicate trigger names. This makes drivers
which currently do that break at probe time.

Patch 7 attempts to ensure that all drivers include indio_dev->id when calling
iio_trigger_alloc. This obviously changes trigger names.


Either 4,5 or 6,7 fix this issue, but 4,5 causes fewer compat issues.


Patches 1,2,3 are just minor features in tools/generic_buffer. They supersede
the v2 I posted earlier:

https://www.spinics.net/lists/linux-iio/msg24867.html

Crestez Dan Leonard (7):
  iio: generic_buffer: Cleanup when receiving signals
  iio: generic_buffer: Add --device-num option
  iio: generic_buffer: Add --trigger-num option
  iio: Add current_trigger_id alternative
  iio: generic_buffer: Use current_trigger_id
  iio: Refuse to register triggers with duplicate names
  iio: Make trigger names unique

 Documentation/ABI/testing/sysfs-bus-iio|   9 +
 Documentation/DocBook/iio.tmpl |   4 +-
 drivers/iio/adc/max1027.c  |   4 +-
 drivers/iio/common/st_sensors/st_sensors_trigger.c |   2 +-
 drivers/iio/industrialio-trigger.c | 136 ---
 drivers/iio/light/gp2ap020a00f.c   |   4 +-
 tools/iio/generic_buffer.c | 269 ++---
 7 files changed, 309 insertions(+), 119 deletions(-)

-- 
2.5.5



[PATCHv3 3/7] iio: generic_buffer: Add --trigger-num option

2016-05-23 Thread Crestez Dan Leonard
Signed-off-by: Crestez Dan Leonard 
---
 tools/iio/generic_buffer.c | 34 +-
 1 file changed, 29 insertions(+), 5 deletions(-)

diff --git a/tools/iio/generic_buffer.c b/tools/iio/generic_buffer.c
index 3f16e9f..e8c3052 100644
--- a/tools/iio/generic_buffer.c
+++ b/tools/iio/generic_buffer.c
@@ -254,7 +254,9 @@ void print_usage(void)
"  --device-name -n \n"
"  --device-num -N \n"
"Set device by name or number (mandatory)\n"
-   "  -t   Set trigger name\n"
+   "  --trigger-name -t \n"
+   "  --trigger-num -T \n"
+   "Set trigger by name or number\n"
"  -w  Set delay between reads in us (event-less 
mode)\n");
 }
 
@@ -320,6 +322,8 @@ void register_cleanup(void)
 static const struct option longopts[] = {
{ "device-name",1, 0, 'n' },
{ "device-num", 1, 0, 'N' },
+   { "trigger-name",   1, 0, 't' },
+   { "trigger-num",1, 0, 'T' },
{ },
 };
 
@@ -348,7 +352,7 @@ int main(int argc, char **argv)
 
register_cleanup();
 
-   while ((c = getopt_long(argc, argv, "ac:egl:n:N:t:w:", longopts, NULL)) 
!= -1) {
+   while ((c = getopt_long(argc, argv, "ac:egl:n:N:t:T:w:", longopts, 
NULL)) != -1) {
switch (c) {
case 'a':
autochannels = AUTOCHANNELS_ENABLED;
@@ -391,6 +395,12 @@ int main(int argc, char **argv)
case 't':
trigger_name = strdup(optarg);
break;
+   case 'T':
+   errno = 0;
+   trig_num = strtoul(optarg, , 10);
+   if (errno)
+   return -errno;
+   break;
case 'w':
errno = 0;
timedelay = strtoul(optarg, , 10);
@@ -444,7 +454,23 @@ int main(int argc, char **argv)
}
}
 
-   if (!notrigger) {
+   if (notrigger) {
+   printf("trigger-less mode selected\n");
+   } if (trig_num > 0) {
+   char *trig_dev_name;
+   ret = asprintf(_dev_name, "%strigger%d", iio_dir, 
trig_num);
+   if (ret < 0) {
+   return -ENOMEM;
+   }
+   trigger_name = malloc(IIO_MAX_NAME_LENGTH);
+   ret = read_sysfs_string("name", trig_dev_name, trigger_name);
+   free(trig_dev_name);
+   if (ret < 0) {
+   fprintf(stderr, "Failed to read trigger%d name from\n", 
trig_num);
+   return ret;
+   }
+   printf("iio trigger number being used is %d\n", trig_num);
+   } else {
if (!trigger_name) {
/*
 * Build the trigger name. If it is device associated
@@ -481,8 +507,6 @@ int main(int argc, char **argv)
}
 
printf("iio trigger number being used is %d\n", trig_num);
-   } else {
-   printf("trigger-less mode selected\n");
}
 
/*
-- 
2.5.5



[RFC 0/7] Deal with iio trigger names

2016-05-23 Thread Crestez Dan Leonard
IIO documents that trigger names are unique but does not actually guarantee
this. You can easily create a software trigger with a duplicate name if you
enable CONFIG_IIO_HRTIMER_TRIGGER:

mkdir /sys/kernel/config/iio/triggers/hrtimer/\
`cat /sys/bus/iio/devices/trigger0/name`

You can also get in this situation if you connect two identical st_sensors.

My attempt to fix this is by adding a new 'current_trigger_id' ABI which is a
numeric ID (X from triggerX) rather than a non-unique string. I tested that
this works as expected when connecting two LIS3DH to the same system. This is
in patches 4,5.

In theory the current_trigger ABI could be overloaded to looks for something
matching "trigger[0-9]+" but this would be nastier. There would be a need to
prevent registering triggers that match that expression in order to prevent
stuff like:

mkdir /sys/kernel/config/iio/triggers/hrtimer/trigger1

Such an overload wouldn't work on read and make it impossible to unambigiously
determine the currently selected trigger.


Patch 6 disallows registering duplicate trigger names. This makes drivers
which currently do that break at probe time.

Patch 7 attempts to ensure that all drivers include indio_dev->id when calling
iio_trigger_alloc. This obviously changes trigger names.


Either 4,5 or 6,7 fix this issue, but 4,5 causes fewer compat issues.


Patches 1,2,3 are just minor features in tools/generic_buffer. They supersede
the v2 I posted earlier:

https://www.spinics.net/lists/linux-iio/msg24867.html

Crestez Dan Leonard (7):
  iio: generic_buffer: Cleanup when receiving signals
  iio: generic_buffer: Add --device-num option
  iio: generic_buffer: Add --trigger-num option
  iio: Add current_trigger_id alternative
  iio: generic_buffer: Use current_trigger_id
  iio: Refuse to register triggers with duplicate names
  iio: Make trigger names unique

 Documentation/ABI/testing/sysfs-bus-iio|   9 +
 Documentation/DocBook/iio.tmpl |   4 +-
 drivers/iio/adc/max1027.c  |   4 +-
 drivers/iio/common/st_sensors/st_sensors_trigger.c |   2 +-
 drivers/iio/industrialio-trigger.c | 136 ---
 drivers/iio/light/gp2ap020a00f.c   |   4 +-
 tools/iio/generic_buffer.c | 269 ++---
 7 files changed, 309 insertions(+), 119 deletions(-)

-- 
2.5.5



[RFC 7/7] iio: Make trigger names unique

2016-05-23 Thread Crestez Dan Leonard
This changes the format of trigger names for some drivers so that the
result always includes indio_dev->id and is thus unique.

Signed-off-by: Crestez Dan Leonard 
---
 drivers/iio/adc/max1027.c  | 4 ++--
 drivers/iio/common/st_sensors/st_sensors_trigger.c | 2 +-
 drivers/iio/light/gp2ap020a00f.c   | 4 ++--
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/iio/adc/max1027.c b/drivers/iio/adc/max1027.c
index 41d495c..4020257 100644
--- a/drivers/iio/adc/max1027.c
+++ b/drivers/iio/adc/max1027.c
@@ -447,8 +447,8 @@ static int max1027_probe(struct spi_device *spi)
return ret;
}
 
-   st->trig = devm_iio_trigger_alloc(>dev, "%s-trigger",
-   indio_dev->name);
+   st->trig = devm_iio_trigger_alloc(>dev, "%s-dev%d",
+ indio_dev->name, indio_dev->iod);
if (st->trig == NULL) {
ret = -ENOMEM;
dev_err(_dev->dev, "Failed to allocate iio trigger\n");
diff --git a/drivers/iio/common/st_sensors/st_sensors_trigger.c 
b/drivers/iio/common/st_sensors/st_sensors_trigger.c
index da72279..9d3e9ab 100644
--- a/drivers/iio/common/st_sensors/st_sensors_trigger.c
+++ b/drivers/iio/common/st_sensors/st_sensors_trigger.c
@@ -24,7 +24,7 @@ int st_sensors_allocate_trigger(struct iio_dev *indio_dev,
struct st_sensor_data *sdata = iio_priv(indio_dev);
unsigned long irq_trig;
 
-   sdata->trig = iio_trigger_alloc("%s-trigger", indio_dev->name);
+   sdata->trig = iio_trigger_alloc("%s-dev%d", indio_dev->name, 
indio_dev->id);
if (sdata->trig == NULL) {
dev_err(_dev->dev, "failed to allocate iio trigger.\n");
return -ENOMEM;
diff --git a/drivers/iio/light/gp2ap020a00f.c b/drivers/iio/light/gp2ap020a00f.c
index 6d41086..e2efaeb 100644
--- a/drivers/iio/light/gp2ap020a00f.c
+++ b/drivers/iio/light/gp2ap020a00f.c
@@ -1553,8 +1553,8 @@ static int gp2ap020a00f_probe(struct i2c_client *client,
goto error_regulator_disable;
 
/* Allocate trigger */
-   data->trig = devm_iio_trigger_alloc(>dev, "%s-trigger",
-   indio_dev->name);
+   data->trig = devm_iio_trigger_alloc(>dev, "%s-dev%d",
+   indio_dev->name, indio_dev->id);
if (data->trig == NULL) {
err = -ENOMEM;
dev_err(_dev->dev, "Failed to allocate iio trigger.\n");
-- 
2.5.5



[PATCHv3 1/7] iio: generic_buffer: Cleanup when receiving signals

2016-05-23 Thread Crestez Dan Leonard
This will clean (disable buffer/trigger/channels) when doing
something like a CTRL-C. Otherwise restarting generic_buffer requires a
manual echo 0 > buffer/enable

This also cleanup up all the code freeing string buffers at
the end of main. We initialize all pointers to NULL so that cleanup can
all be done under a single error label.

Signed-off-by: Crestez Dan Leonard 
---
Changes since v2: Initialize data to NULL to prevent crash on free.

 tools/iio/generic_buffer.c | 172 -
 1 file changed, 108 insertions(+), 64 deletions(-)

diff --git a/tools/iio/generic_buffer.c b/tools/iio/generic_buffer.c
index 2429c78..972f400 100644
--- a/tools/iio/generic_buffer.c
+++ b/tools/iio/generic_buffer.c
@@ -32,6 +32,8 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 #include "iio_utils.h"
 
 /**
@@ -254,6 +256,65 @@ void print_usage(void)
"  -w  Set delay between reads in us (event-less 
mode)\n");
 }
 
+enum autochan autochannels = AUTOCHANNELS_DISABLED;
+char *dev_dir_name = NULL;
+char *buf_dir_name = NULL;
+bool current_trigger_set = false;
+
+void cleanup(void)
+{
+   int ret;
+
+   /* Disable trigger */
+   if (dev_dir_name && current_trigger_set) {
+   /* Disconnect the trigger - just write a dummy name. */
+   ret = write_sysfs_string("trigger/current_trigger",
+dev_dir_name, "NULL");
+   if (ret < 0)
+   fprintf(stderr, "Failed to disable trigger: %s\n",
+   strerror(-ret));
+   current_trigger_set = false;
+   }
+
+   /* Disable buffer */
+   if (buf_dir_name) {
+   ret = write_sysfs_int("enable", buf_dir_name, 0);
+   if (ret < 0)
+   fprintf(stderr, "Failed to disable buffer: %s\n",
+   strerror(-ret));
+   }
+
+   /* Disable channels if auto-enabled */
+   if (dev_dir_name && autochannels == AUTOCHANNELS_ACTIVE) {
+   ret = enable_disable_all_channels(dev_dir_name, 0);
+   if (ret)
+   fprintf(stderr, "Failed to disable all channels\n");
+   autochannels = AUTOCHANNELS_DISABLED;
+   }
+}
+
+void sig_handler(int signum)
+{
+   fprintf(stderr, "Caught signal %d\n", signum);
+   cleanup();
+   exit(-signum);
+}
+
+void register_cleanup(void)
+{
+   struct sigaction sa = { .sa_handler = sig_handler };
+   const int signums[] = { SIGINT, SIGTERM, SIGABRT };
+   int ret, i;
+
+   for (i = 0; i < ARRAY_SIZE(signums); ++i) {
+   ret = sigaction(signums[i], , NULL);
+   if (ret) {
+   perror("Failed to register signal handler");
+   exit(-1);
+   }
+   }
+}
+
 int main(int argc, char **argv)
 {
unsigned long num_loops = 2;
@@ -261,25 +322,24 @@ int main(int argc, char **argv)
unsigned long buf_len = 128;
 
int ret, c, i, j, toread;
-   int fp;
+   int fp = -1;
 
-   int num_channels;
+   int num_channels = 0;
char *trigger_name = NULL, *device_name = NULL;
-   char *dev_dir_name, *buf_dir_name;
 
-   int datardytrigger = 1;
-   char *data;
+   char *data = NULL;
ssize_t read_size;
int dev_num, trig_num;
-   char *buffer_access;
+   char *buffer_access = NULL;
int scan_size;
int noevents = 0;
int notrigger = 0;
-   enum autochan autochannels = AUTOCHANNELS_DISABLED;
char *dummy;
 
struct iio_channel_info *channels;
 
+   register_cleanup();
+
while ((c = getopt(argc, argv, "ac:egl:n:t:w:")) != -1) {
switch (c) {
case 'a':
@@ -288,8 +348,10 @@ int main(int argc, char **argv)
case 'c':
errno = 0;
num_loops = strtoul(optarg, , 10);
-   if (errno)
-   return -errno;
+   if (errno) {
+   ret = -errno;
+   goto error;
+   }
 
break;
case 'e':
@@ -301,26 +363,30 @@ int main(int argc, char **argv)
case 'l':
errno = 0;
buf_len = strtoul(optarg, , 10);
-   if (errno)
-   return -errno;
+   if (errno) {
+   ret = -errno;
+   goto error;
+   }
 
break;
case 'n':
device_name = optarg;
break;
case 't':
-   trigger_name = optarg;
-   datardytrigger = 0;
+

[PATCHv3 2/7] iio: generic_buffer: Add --device-num option

2016-05-23 Thread Crestez Dan Leonard
This makes it possible to distinguish between iio devices with the same
name.

Signed-off-by: Crestez Dan Leonard 
---
 tools/iio/generic_buffer.c | 69 ++
 1 file changed, 51 insertions(+), 18 deletions(-)

diff --git a/tools/iio/generic_buffer.c b/tools/iio/generic_buffer.c
index 972f400..3f16e9f 100644
--- a/tools/iio/generic_buffer.c
+++ b/tools/iio/generic_buffer.c
@@ -251,7 +251,9 @@ void print_usage(void)
"  -e Disable wait for event (new data)\n"
"  -g Use trigger-less mode\n"
"  -l  Set buffer length to n samples\n"
-   "  -n   Set device name (mandatory)\n"
+   "  --device-name -n \n"
+   "  --device-num -N \n"
+   "Set device by name or number (mandatory)\n"
"  -t   Set trigger name\n"
"  -w  Set delay between reads in us (event-less 
mode)\n");
 }
@@ -315,6 +317,12 @@ void register_cleanup(void)
}
 }
 
+static const struct option longopts[] = {
+   { "device-name",1, 0, 'n' },
+   { "device-num", 1, 0, 'N' },
+   { },
+};
+
 int main(int argc, char **argv)
 {
unsigned long num_loops = 2;
@@ -329,7 +337,7 @@ int main(int argc, char **argv)
 
char *data = NULL;
ssize_t read_size;
-   int dev_num, trig_num;
+   int dev_num = -1, trig_num;
char *buffer_access = NULL;
int scan_size;
int noevents = 0;
@@ -340,7 +348,7 @@ int main(int argc, char **argv)
 
register_cleanup();
 
-   while ((c = getopt(argc, argv, "ac:egl:n:t:w:")) != -1) {
+   while ((c = getopt_long(argc, argv, "ac:egl:n:N:t:w:", longopts, NULL)) 
!= -1) {
switch (c) {
case 'a':
autochannels = AUTOCHANNELS_ENABLED;
@@ -370,7 +378,15 @@ int main(int argc, char **argv)
 
break;
case 'n':
-   device_name = optarg;
+   device_name = strdup(optarg);
+   break;
+   case 'N':
+   errno = 0;
+   dev_num = strtoul(optarg, , 10);
+   if (errno) {
+   ret = -errno;
+   goto error;
+   }
break;
case 't':
trigger_name = strdup(optarg);
@@ -390,26 +406,42 @@ int main(int argc, char **argv)
}
}
 
-   if (!device_name) {
-   fprintf(stderr, "Device name not set\n");
-   print_usage();
-   return -1;
-   }
-
/* Find the device requested */
-   dev_num = find_type_by_name(device_name, "iio:device");
-   if (dev_num < 0) {
-   fprintf(stderr, "Failed to find the %s\n", device_name);
-   ret = dev_num;
+   if (dev_num < 0 && !device_name) {
+   fprintf(stderr, "Device not set\n");
+   print_usage();
+   ret = -1;
+   goto error;
+   } else if (dev_num >= 0 && device_name) {
+   fprintf(stderr, "Only one of --device-num or --device-name 
needs to be set\n");
+   print_usage();
+   ret = -1;
goto error;
+   } else if (dev_num < 0) {
+   dev_num = find_type_by_name(device_name, "iio:device");
+   if (dev_num < 0) {
+   fprintf(stderr, "Failed to find the %s\n", device_name);
+   ret = dev_num;
+   goto error;
+   }
}
-
printf("iio device number being used is %d\n", dev_num);
 
ret = asprintf(_dir_name, "%siio:device%d", iio_dir, dev_num);
-   if (ret < 0) {
-   ret = -ENOMEM;
-   goto error;
+   if (ret < 0)
+   return -ENOMEM;
+   /* Fetch device_name if specified by number */
+   if (!device_name) {
+   device_name = malloc(IIO_MAX_NAME_LENGTH);
+   if (!device_name) {
+   ret = -ENOMEM;
+   goto error;
+   }
+   ret = read_sysfs_string("name", dev_dir_name, device_name);
+   if (ret < 0) {
+   fprintf(stderr, "Failed to read name of device %d\n", 
dev_num);
+   goto error;
+   }
}
 
if (!notrigger) {
@@ -619,6 +651,7 @@ error:
}
free(channels);
free(trigger_name);
+   free(device_name);
free(dev_dir_name);
 
return ret;
-- 
2.5.5



[RFC 5/7] iio: generic_buffer: Use current_trigger_id

2016-05-23 Thread Crestez Dan Leonard
This is a preferred alternative to 'current_trigger'.

Signed-off-by: Crestez Dan Leonard 
---
 tools/iio/generic_buffer.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/iio/generic_buffer.c b/tools/iio/generic_buffer.c
index e8c3052..b23371a 100644
--- a/tools/iio/generic_buffer.c
+++ b/tools/iio/generic_buffer.c
@@ -582,12 +582,12 @@ int main(int argc, char **argv)
 * Set the device trigger to be the data ready trigger found
 * above
 */
-   ret = write_sysfs_string_and_verify("trigger/current_trigger",
-   dev_dir_name,
-   trigger_name);
+   ret = write_sysfs_int_and_verify("trigger/current_trigger_id",
+dev_dir_name,
+trig_num);
if (ret < 0) {
fprintf(stderr,
-   "Failed to write current_trigger file\n");
+   "Failed to write current_trigger_id file\n");
goto error;
}
}
-- 
2.5.5



[RFC 7/7] iio: Make trigger names unique

2016-05-23 Thread Crestez Dan Leonard
This changes the format of trigger names for some drivers so that the
result always includes indio_dev->id and is thus unique.

Signed-off-by: Crestez Dan Leonard 
---
 drivers/iio/adc/max1027.c  | 4 ++--
 drivers/iio/common/st_sensors/st_sensors_trigger.c | 2 +-
 drivers/iio/light/gp2ap020a00f.c   | 4 ++--
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/iio/adc/max1027.c b/drivers/iio/adc/max1027.c
index 41d495c..4020257 100644
--- a/drivers/iio/adc/max1027.c
+++ b/drivers/iio/adc/max1027.c
@@ -447,8 +447,8 @@ static int max1027_probe(struct spi_device *spi)
return ret;
}
 
-   st->trig = devm_iio_trigger_alloc(>dev, "%s-trigger",
-   indio_dev->name);
+   st->trig = devm_iio_trigger_alloc(>dev, "%s-dev%d",
+ indio_dev->name, indio_dev->iod);
if (st->trig == NULL) {
ret = -ENOMEM;
dev_err(_dev->dev, "Failed to allocate iio trigger\n");
diff --git a/drivers/iio/common/st_sensors/st_sensors_trigger.c 
b/drivers/iio/common/st_sensors/st_sensors_trigger.c
index da72279..9d3e9ab 100644
--- a/drivers/iio/common/st_sensors/st_sensors_trigger.c
+++ b/drivers/iio/common/st_sensors/st_sensors_trigger.c
@@ -24,7 +24,7 @@ int st_sensors_allocate_trigger(struct iio_dev *indio_dev,
struct st_sensor_data *sdata = iio_priv(indio_dev);
unsigned long irq_trig;
 
-   sdata->trig = iio_trigger_alloc("%s-trigger", indio_dev->name);
+   sdata->trig = iio_trigger_alloc("%s-dev%d", indio_dev->name, 
indio_dev->id);
if (sdata->trig == NULL) {
dev_err(_dev->dev, "failed to allocate iio trigger.\n");
return -ENOMEM;
diff --git a/drivers/iio/light/gp2ap020a00f.c b/drivers/iio/light/gp2ap020a00f.c
index 6d41086..e2efaeb 100644
--- a/drivers/iio/light/gp2ap020a00f.c
+++ b/drivers/iio/light/gp2ap020a00f.c
@@ -1553,8 +1553,8 @@ static int gp2ap020a00f_probe(struct i2c_client *client,
goto error_regulator_disable;
 
/* Allocate trigger */
-   data->trig = devm_iio_trigger_alloc(>dev, "%s-trigger",
-   indio_dev->name);
+   data->trig = devm_iio_trigger_alloc(>dev, "%s-dev%d",
+   indio_dev->name, indio_dev->id);
if (data->trig == NULL) {
err = -ENOMEM;
dev_err(_dev->dev, "Failed to allocate iio trigger.\n");
-- 
2.5.5



[PATCHv3 1/7] iio: generic_buffer: Cleanup when receiving signals

2016-05-23 Thread Crestez Dan Leonard
This will clean (disable buffer/trigger/channels) when doing
something like a CTRL-C. Otherwise restarting generic_buffer requires a
manual echo 0 > buffer/enable

This also cleanup up all the code freeing string buffers at
the end of main. We initialize all pointers to NULL so that cleanup can
all be done under a single error label.

Signed-off-by: Crestez Dan Leonard 
---
Changes since v2: Initialize data to NULL to prevent crash on free.

 tools/iio/generic_buffer.c | 172 -
 1 file changed, 108 insertions(+), 64 deletions(-)

diff --git a/tools/iio/generic_buffer.c b/tools/iio/generic_buffer.c
index 2429c78..972f400 100644
--- a/tools/iio/generic_buffer.c
+++ b/tools/iio/generic_buffer.c
@@ -32,6 +32,8 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 #include "iio_utils.h"
 
 /**
@@ -254,6 +256,65 @@ void print_usage(void)
"  -w  Set delay between reads in us (event-less 
mode)\n");
 }
 
+enum autochan autochannels = AUTOCHANNELS_DISABLED;
+char *dev_dir_name = NULL;
+char *buf_dir_name = NULL;
+bool current_trigger_set = false;
+
+void cleanup(void)
+{
+   int ret;
+
+   /* Disable trigger */
+   if (dev_dir_name && current_trigger_set) {
+   /* Disconnect the trigger - just write a dummy name. */
+   ret = write_sysfs_string("trigger/current_trigger",
+dev_dir_name, "NULL");
+   if (ret < 0)
+   fprintf(stderr, "Failed to disable trigger: %s\n",
+   strerror(-ret));
+   current_trigger_set = false;
+   }
+
+   /* Disable buffer */
+   if (buf_dir_name) {
+   ret = write_sysfs_int("enable", buf_dir_name, 0);
+   if (ret < 0)
+   fprintf(stderr, "Failed to disable buffer: %s\n",
+   strerror(-ret));
+   }
+
+   /* Disable channels if auto-enabled */
+   if (dev_dir_name && autochannels == AUTOCHANNELS_ACTIVE) {
+   ret = enable_disable_all_channels(dev_dir_name, 0);
+   if (ret)
+   fprintf(stderr, "Failed to disable all channels\n");
+   autochannels = AUTOCHANNELS_DISABLED;
+   }
+}
+
+void sig_handler(int signum)
+{
+   fprintf(stderr, "Caught signal %d\n", signum);
+   cleanup();
+   exit(-signum);
+}
+
+void register_cleanup(void)
+{
+   struct sigaction sa = { .sa_handler = sig_handler };
+   const int signums[] = { SIGINT, SIGTERM, SIGABRT };
+   int ret, i;
+
+   for (i = 0; i < ARRAY_SIZE(signums); ++i) {
+   ret = sigaction(signums[i], , NULL);
+   if (ret) {
+   perror("Failed to register signal handler");
+   exit(-1);
+   }
+   }
+}
+
 int main(int argc, char **argv)
 {
unsigned long num_loops = 2;
@@ -261,25 +322,24 @@ int main(int argc, char **argv)
unsigned long buf_len = 128;
 
int ret, c, i, j, toread;
-   int fp;
+   int fp = -1;
 
-   int num_channels;
+   int num_channels = 0;
char *trigger_name = NULL, *device_name = NULL;
-   char *dev_dir_name, *buf_dir_name;
 
-   int datardytrigger = 1;
-   char *data;
+   char *data = NULL;
ssize_t read_size;
int dev_num, trig_num;
-   char *buffer_access;
+   char *buffer_access = NULL;
int scan_size;
int noevents = 0;
int notrigger = 0;
-   enum autochan autochannels = AUTOCHANNELS_DISABLED;
char *dummy;
 
struct iio_channel_info *channels;
 
+   register_cleanup();
+
while ((c = getopt(argc, argv, "ac:egl:n:t:w:")) != -1) {
switch (c) {
case 'a':
@@ -288,8 +348,10 @@ int main(int argc, char **argv)
case 'c':
errno = 0;
num_loops = strtoul(optarg, , 10);
-   if (errno)
-   return -errno;
+   if (errno) {
+   ret = -errno;
+   goto error;
+   }
 
break;
case 'e':
@@ -301,26 +363,30 @@ int main(int argc, char **argv)
case 'l':
errno = 0;
buf_len = strtoul(optarg, , 10);
-   if (errno)
-   return -errno;
+   if (errno) {
+   ret = -errno;
+   goto error;
+   }
 
break;
case 'n':
device_name = optarg;
break;
case 't':
-   trigger_name = optarg;
-   datardytrigger = 0;
+   trigger_name = 

[PATCHv3 2/7] iio: generic_buffer: Add --device-num option

2016-05-23 Thread Crestez Dan Leonard
This makes it possible to distinguish between iio devices with the same
name.

Signed-off-by: Crestez Dan Leonard 
---
 tools/iio/generic_buffer.c | 69 ++
 1 file changed, 51 insertions(+), 18 deletions(-)

diff --git a/tools/iio/generic_buffer.c b/tools/iio/generic_buffer.c
index 972f400..3f16e9f 100644
--- a/tools/iio/generic_buffer.c
+++ b/tools/iio/generic_buffer.c
@@ -251,7 +251,9 @@ void print_usage(void)
"  -e Disable wait for event (new data)\n"
"  -g Use trigger-less mode\n"
"  -l  Set buffer length to n samples\n"
-   "  -n   Set device name (mandatory)\n"
+   "  --device-name -n \n"
+   "  --device-num -N \n"
+   "Set device by name or number (mandatory)\n"
"  -t   Set trigger name\n"
"  -w  Set delay between reads in us (event-less 
mode)\n");
 }
@@ -315,6 +317,12 @@ void register_cleanup(void)
}
 }
 
+static const struct option longopts[] = {
+   { "device-name",1, 0, 'n' },
+   { "device-num", 1, 0, 'N' },
+   { },
+};
+
 int main(int argc, char **argv)
 {
unsigned long num_loops = 2;
@@ -329,7 +337,7 @@ int main(int argc, char **argv)
 
char *data = NULL;
ssize_t read_size;
-   int dev_num, trig_num;
+   int dev_num = -1, trig_num;
char *buffer_access = NULL;
int scan_size;
int noevents = 0;
@@ -340,7 +348,7 @@ int main(int argc, char **argv)
 
register_cleanup();
 
-   while ((c = getopt(argc, argv, "ac:egl:n:t:w:")) != -1) {
+   while ((c = getopt_long(argc, argv, "ac:egl:n:N:t:w:", longopts, NULL)) 
!= -1) {
switch (c) {
case 'a':
autochannels = AUTOCHANNELS_ENABLED;
@@ -370,7 +378,15 @@ int main(int argc, char **argv)
 
break;
case 'n':
-   device_name = optarg;
+   device_name = strdup(optarg);
+   break;
+   case 'N':
+   errno = 0;
+   dev_num = strtoul(optarg, , 10);
+   if (errno) {
+   ret = -errno;
+   goto error;
+   }
break;
case 't':
trigger_name = strdup(optarg);
@@ -390,26 +406,42 @@ int main(int argc, char **argv)
}
}
 
-   if (!device_name) {
-   fprintf(stderr, "Device name not set\n");
-   print_usage();
-   return -1;
-   }
-
/* Find the device requested */
-   dev_num = find_type_by_name(device_name, "iio:device");
-   if (dev_num < 0) {
-   fprintf(stderr, "Failed to find the %s\n", device_name);
-   ret = dev_num;
+   if (dev_num < 0 && !device_name) {
+   fprintf(stderr, "Device not set\n");
+   print_usage();
+   ret = -1;
+   goto error;
+   } else if (dev_num >= 0 && device_name) {
+   fprintf(stderr, "Only one of --device-num or --device-name 
needs to be set\n");
+   print_usage();
+   ret = -1;
goto error;
+   } else if (dev_num < 0) {
+   dev_num = find_type_by_name(device_name, "iio:device");
+   if (dev_num < 0) {
+   fprintf(stderr, "Failed to find the %s\n", device_name);
+   ret = dev_num;
+   goto error;
+   }
}
-
printf("iio device number being used is %d\n", dev_num);
 
ret = asprintf(_dir_name, "%siio:device%d", iio_dir, dev_num);
-   if (ret < 0) {
-   ret = -ENOMEM;
-   goto error;
+   if (ret < 0)
+   return -ENOMEM;
+   /* Fetch device_name if specified by number */
+   if (!device_name) {
+   device_name = malloc(IIO_MAX_NAME_LENGTH);
+   if (!device_name) {
+   ret = -ENOMEM;
+   goto error;
+   }
+   ret = read_sysfs_string("name", dev_dir_name, device_name);
+   if (ret < 0) {
+   fprintf(stderr, "Failed to read name of device %d\n", 
dev_num);
+   goto error;
+   }
}
 
if (!notrigger) {
@@ -619,6 +651,7 @@ error:
}
free(channels);
free(trigger_name);
+   free(device_name);
free(dev_dir_name);
 
return ret;
-- 
2.5.5



[RFC 5/7] iio: generic_buffer: Use current_trigger_id

2016-05-23 Thread Crestez Dan Leonard
This is a preferred alternative to 'current_trigger'.

Signed-off-by: Crestez Dan Leonard 
---
 tools/iio/generic_buffer.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/iio/generic_buffer.c b/tools/iio/generic_buffer.c
index e8c3052..b23371a 100644
--- a/tools/iio/generic_buffer.c
+++ b/tools/iio/generic_buffer.c
@@ -582,12 +582,12 @@ int main(int argc, char **argv)
 * Set the device trigger to be the data ready trigger found
 * above
 */
-   ret = write_sysfs_string_and_verify("trigger/current_trigger",
-   dev_dir_name,
-   trigger_name);
+   ret = write_sysfs_int_and_verify("trigger/current_trigger_id",
+dev_dir_name,
+trig_num);
if (ret < 0) {
fprintf(stderr,
-   "Failed to write current_trigger file\n");
+   "Failed to write current_trigger_id file\n");
goto error;
}
}
-- 
2.5.5



Re: powerpc/pseries: start rtasd before PCI probing

2016-05-23 Thread Greg Kurz
On Mon, 23 May 2016 20:23:19 +0200
Thomas Huth  wrote:

> On 23.05.2016 10:28, Greg Kurz wrote:
> > A strange behaviour is observed when comparing PCI hotplug in QEMU, between
> > x86 and pseries. If you consider the following steps:
> > - start a VM
> > - add a PCI device via the QEMU monitor before the rtasd has started (for
> >   example starting the VM in paused state, or hotplug during FW or boot
> >   loader)
> > - resume the VM execution
> > 
> > The x86 kernel detects the PCI device, but the pseries one does not.
> > 
> > This happens because the rtasd kernel worker is currently started under
> > device_initcall, while PCI probing happens earlier under subsys_initcall.
> > 
> > As a consequence, if we have a pending RTAS event at boot time, a message
> > is printed and the event is dropped.
> > 
> > This patch moves all the initialization of rtasd to arch_initcall, which is
> > run before subsys_call: this way, logging_enabled is true when the RTAS
> > event pops up and it is not lost anymore.
> > 
> > The proc fs bits stay at device_initcall because they cannot be run before
> > fs_initcall.
> > 
> > Signed-off-by: Greg Kurz 
> > ---
> >  arch/powerpc/kernel/rtasd.c |   19 ++-
> >  1 file changed, 14 insertions(+), 5 deletions(-)  
> 
> By the way, same is true for device UNplugging: When unplugging devices
> in QEMU while the firmware is still running, they are never properly
> removed from the guest. I've checked it, and your patch fixes this
> problem as well! Great :-)
> 
> Tested-by: Thomas Huth 
> 

Indeed, all pending RTAS events were lost... now rtasd will log them, up
to 64 events, but I did not test that far :)

Thanks for testing !

Cheers.

--
Greg



Re: powerpc/pseries: start rtasd before PCI probing

2016-05-23 Thread Greg Kurz
On Mon, 23 May 2016 20:23:19 +0200
Thomas Huth  wrote:

> On 23.05.2016 10:28, Greg Kurz wrote:
> > A strange behaviour is observed when comparing PCI hotplug in QEMU, between
> > x86 and pseries. If you consider the following steps:
> > - start a VM
> > - add a PCI device via the QEMU monitor before the rtasd has started (for
> >   example starting the VM in paused state, or hotplug during FW or boot
> >   loader)
> > - resume the VM execution
> > 
> > The x86 kernel detects the PCI device, but the pseries one does not.
> > 
> > This happens because the rtasd kernel worker is currently started under
> > device_initcall, while PCI probing happens earlier under subsys_initcall.
> > 
> > As a consequence, if we have a pending RTAS event at boot time, a message
> > is printed and the event is dropped.
> > 
> > This patch moves all the initialization of rtasd to arch_initcall, which is
> > run before subsys_call: this way, logging_enabled is true when the RTAS
> > event pops up and it is not lost anymore.
> > 
> > The proc fs bits stay at device_initcall because they cannot be run before
> > fs_initcall.
> > 
> > Signed-off-by: Greg Kurz 
> > ---
> >  arch/powerpc/kernel/rtasd.c |   19 ++-
> >  1 file changed, 14 insertions(+), 5 deletions(-)  
> 
> By the way, same is true for device UNplugging: When unplugging devices
> in QEMU while the firmware is still running, they are never properly
> removed from the guest. I've checked it, and your patch fixes this
> problem as well! Great :-)
> 
> Tested-by: Thomas Huth 
> 

Indeed, all pending RTAS events were lost... now rtasd will log them, up
to 64 events, but I did not test that far :)

Thanks for testing !

Cheers.

--
Greg



Re: [RFC PATCH 03/15] Provide atomic_t functions implemented with ISO-C++11 atomics

2016-05-23 Thread Paul E. McKenney
On Fri, May 20, 2016 at 07:32:09PM +1000, Michael Ellerman wrote:
> On Thu, 2016-05-19 at 08:00 -0700, Paul E. McKenney wrote:
> > On Thu, May 19, 2016 at 04:41:17PM +0200, Peter Zijlstra wrote:
> > > On Thu, May 19, 2016 at 07:22:52AM -0700, Paul E. McKenney wrote:
> > > > Agreed, these sorts of instruction sequences make a lot of sense.
> > > > Of course, if you stuff too many intructions and cache misses between
> > > > the LL and the SC, the SC success probability starts dropping, but short
> > > > seqeunces of non-memory-reference instructions like the above should be
> > > > just fine.
> > > 
> > > In fact, pretty much every single LL/SC arch I've looked at doesn't
> > > allow _any_ loads or stores inside and will guarantee SC failure (or
> > > worse) if you do.
> > 
> > Last I know, PowerPC allowed memory-reference instructions inside, but
> > the more of them you have, the less likely your reservation is to survive.
> > But perhaps I missed some fine print somewhere.  And in any case,
> > omitting them is certainly better.
> 
> There's nothing in the architecture AFAIK.
> 
> Also I don't see anything to indicate that doing more unrelated accesses makes
> the reservation more likely to be lost. Other than it causes you to hold the
> reservation for longer, which increases the chance of some other CPU accessing
> the variable.

And also more likely to hit cache-geometry limitations.

> Having said that, the architecture is written to provide maximum wiggle room
> for implementations. So the list of things that may cause the reservation to 
> be
> lost includes "Implementation-specific characteristics of the coherence
> mechanism", ie. anything.
> 
> > > This immediately disqualifies things like calls/traps/etc.. because
> > > those implicitly issue stores.
> > 
> > Traps for sure.  Not so sure about calls on PowerPC.
> 
> Actually no, exceptions (aka interrupts/traps) are explicitly defined to *not*
> clear the reservation. And function calls are just branches so should also be
> fine.

But don't most interrupt/trap handlers clear the reservation in software?

Thanx, Paul



Re: [RFC PATCH 03/15] Provide atomic_t functions implemented with ISO-C++11 atomics

2016-05-23 Thread Paul E. McKenney
On Fri, May 20, 2016 at 07:32:09PM +1000, Michael Ellerman wrote:
> On Thu, 2016-05-19 at 08:00 -0700, Paul E. McKenney wrote:
> > On Thu, May 19, 2016 at 04:41:17PM +0200, Peter Zijlstra wrote:
> > > On Thu, May 19, 2016 at 07:22:52AM -0700, Paul E. McKenney wrote:
> > > > Agreed, these sorts of instruction sequences make a lot of sense.
> > > > Of course, if you stuff too many intructions and cache misses between
> > > > the LL and the SC, the SC success probability starts dropping, but short
> > > > seqeunces of non-memory-reference instructions like the above should be
> > > > just fine.
> > > 
> > > In fact, pretty much every single LL/SC arch I've looked at doesn't
> > > allow _any_ loads or stores inside and will guarantee SC failure (or
> > > worse) if you do.
> > 
> > Last I know, PowerPC allowed memory-reference instructions inside, but
> > the more of them you have, the less likely your reservation is to survive.
> > But perhaps I missed some fine print somewhere.  And in any case,
> > omitting them is certainly better.
> 
> There's nothing in the architecture AFAIK.
> 
> Also I don't see anything to indicate that doing more unrelated accesses makes
> the reservation more likely to be lost. Other than it causes you to hold the
> reservation for longer, which increases the chance of some other CPU accessing
> the variable.

And also more likely to hit cache-geometry limitations.

> Having said that, the architecture is written to provide maximum wiggle room
> for implementations. So the list of things that may cause the reservation to 
> be
> lost includes "Implementation-specific characteristics of the coherence
> mechanism", ie. anything.
> 
> > > This immediately disqualifies things like calls/traps/etc.. because
> > > those implicitly issue stores.
> > 
> > Traps for sure.  Not so sure about calls on PowerPC.
> 
> Actually no, exceptions (aka interrupts/traps) are explicitly defined to *not*
> clear the reservation. And function calls are just branches so should also be
> fine.

But don't most interrupt/trap handlers clear the reservation in software?

Thanx, Paul



Re: [PATCH] mm: make CONFIG_DEFERRED_STRUCT_PAGE_INIT depends on !FLATMEM explicitly

2016-05-23 Thread Shi, Yang

On 5/23/2016 11:22 AM, Michal Hocko wrote:

On Mon 23-05-16 09:54:31, Yang Shi wrote:

Per the suggestion from Michal Hocko [1], CONFIG_DEFERRED_STRUCT_PAGE_INIT
should be incompatible with FLATMEM, make this explicitly in Kconfig.


I guess the changelog could benefit from some clarification. What
do you think about the following:

"
DEFERRED_STRUCT_PAGE_INIT requires some ordering wrt other
initialization operations, e.g. page_ext_init has to happen after the
whole memmap is initialized properly. For SPARSEMEM this requires to
wait for page_alloc_init_late. Other memory models (e.g. flatmem) might
have different initialization layouts (page_ext_init_flatmem). Currently
DEFERRED_STRUCT_PAGE_INIT depends on MEMORY_HOTPLUG which in turn
depends on SPARSEMEM || X86_64_ACPI_NUMA
depends on ARCH_ENABLE_MEMORY_HOTPLUG

and X86_64_ACPI_NUMA depends on NUMA which in turn disable FLATMEM
memory model:
config ARCH_FLATMEM_ENABLE
def_bool y
depends on X86_32 && !NUMA

so FLATMEM is ruled out via dependency maze. Be explicit and disable
FLATMEM for DEFERRED_STRUCT_PAGE_INIT so that we do not reintroduce
subtle initialization bugs
"


Thanks a lot. It sounds way better. Will address in V2.

Yang





[1] http://lkml.kernel.org/r/20160523073157.gd2...@dhcp22.suse.cz

Signed-off-by: Yang Shi 
---
 mm/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/mm/Kconfig b/mm/Kconfig
index 2664c11..22fa818 100644
--- a/mm/Kconfig
+++ b/mm/Kconfig
@@ -649,6 +649,7 @@ config DEFERRED_STRUCT_PAGE_INIT
default n
depends on ARCH_SUPPORTS_DEFERRED_STRUCT_PAGE_INIT
depends on MEMORY_HOTPLUG
+   depends on !FLATMEM
help
  Ordinarily all struct pages are initialised during early boot in a
  single thread. On very large machines this can take a considerable
--
2.0.2






Re: [PATCH] mm: make CONFIG_DEFERRED_STRUCT_PAGE_INIT depends on !FLATMEM explicitly

2016-05-23 Thread Shi, Yang

On 5/23/2016 11:22 AM, Michal Hocko wrote:

On Mon 23-05-16 09:54:31, Yang Shi wrote:

Per the suggestion from Michal Hocko [1], CONFIG_DEFERRED_STRUCT_PAGE_INIT
should be incompatible with FLATMEM, make this explicitly in Kconfig.


I guess the changelog could benefit from some clarification. What
do you think about the following:

"
DEFERRED_STRUCT_PAGE_INIT requires some ordering wrt other
initialization operations, e.g. page_ext_init has to happen after the
whole memmap is initialized properly. For SPARSEMEM this requires to
wait for page_alloc_init_late. Other memory models (e.g. flatmem) might
have different initialization layouts (page_ext_init_flatmem). Currently
DEFERRED_STRUCT_PAGE_INIT depends on MEMORY_HOTPLUG which in turn
depends on SPARSEMEM || X86_64_ACPI_NUMA
depends on ARCH_ENABLE_MEMORY_HOTPLUG

and X86_64_ACPI_NUMA depends on NUMA which in turn disable FLATMEM
memory model:
config ARCH_FLATMEM_ENABLE
def_bool y
depends on X86_32 && !NUMA

so FLATMEM is ruled out via dependency maze. Be explicit and disable
FLATMEM for DEFERRED_STRUCT_PAGE_INIT so that we do not reintroduce
subtle initialization bugs
"


Thanks a lot. It sounds way better. Will address in V2.

Yang





[1] http://lkml.kernel.org/r/20160523073157.gd2...@dhcp22.suse.cz

Signed-off-by: Yang Shi 
---
 mm/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/mm/Kconfig b/mm/Kconfig
index 2664c11..22fa818 100644
--- a/mm/Kconfig
+++ b/mm/Kconfig
@@ -649,6 +649,7 @@ config DEFERRED_STRUCT_PAGE_INIT
default n
depends on ARCH_SUPPORTS_DEFERRED_STRUCT_PAGE_INIT
depends on MEMORY_HOTPLUG
+   depends on !FLATMEM
help
  Ordinarily all struct pages are initialised during early boot in a
  single thread. On very large machines this can take a considerable
--
2.0.2






Re: [PATCH 3/3] ACPI: ARM64: support for ACPI_TABLE_UPGRADE

2016-05-23 Thread Jon Masters
On 05/23/2016 01:56 PM, Lorenzo Pieralisi wrote:
> On Tue, May 17, 2016 at 12:48:53PM -0400, Jon Masters wrote:
>> On 05/17/2016 12:44 PM, Jon Masters wrote:
>>
>>> 1). During development of a platform, it is much easier to debug
>>> problems with tables if you can test replacement ones without having to
>>> respin the firmware. In the server world, you usually don't have the
>>> firmware source code, so to get it respun could be days-weeks even if
>>> you are working with the authors closely. We have practically used this
>>> feature on a number of platforms already and it will continue.
>>
>> For example, on one platform we were unable to fully boot RHEL(SA) due
>> to a bug in one of the ACPI tables. But I was able to boot the system to
>> a ramdisk containing a uuencode library and then write out the content
>> of the tables over the serial port, then decompile/patch/recompile, and
>> override replacement tables on the system. Then we beat the vendor up
>> with the fixes and the official firmware was corrected.
> 
> Can you explain to me please why you can't do it with GRUB ?

It's doable with GRUB (if you rebuild GRUB modules to include it)

> I am using mainline GRUB and its acpi command all the time to update
> static ACPI tables for testing new features (ie IORT) and it works
> just fine for me (and you can still override the DSDT, which is
> likely to be the main source of bugs, through the CONFIG_ACPI_CUSTOM_DSDT
> config option, that works on ARM in mainline with no changes required).
> 
> I just want to understand if there is really a compelling reason
> for adding this stuff when we can easily implement its features
> through something that is usable today without any kernel changes.

We're looking for x86 feature parity in the base platform. Every time
there's a gratuitous differentiation it's a waste of time for folks
trying to figure out what is different on an ARM system. So if we
completely remove the ACPI override feature from the kernel and make
everyone use GRUB instead, then fine, but ARM shouldn't be special.

Jon.

-- 
Computer Architect | Sent from my Fedora powered laptop



Re: [PATCH 3/3] ACPI: ARM64: support for ACPI_TABLE_UPGRADE

2016-05-23 Thread Jon Masters
On 05/23/2016 01:56 PM, Lorenzo Pieralisi wrote:
> On Tue, May 17, 2016 at 12:48:53PM -0400, Jon Masters wrote:
>> On 05/17/2016 12:44 PM, Jon Masters wrote:
>>
>>> 1). During development of a platform, it is much easier to debug
>>> problems with tables if you can test replacement ones without having to
>>> respin the firmware. In the server world, you usually don't have the
>>> firmware source code, so to get it respun could be days-weeks even if
>>> you are working with the authors closely. We have practically used this
>>> feature on a number of platforms already and it will continue.
>>
>> For example, on one platform we were unable to fully boot RHEL(SA) due
>> to a bug in one of the ACPI tables. But I was able to boot the system to
>> a ramdisk containing a uuencode library and then write out the content
>> of the tables over the serial port, then decompile/patch/recompile, and
>> override replacement tables on the system. Then we beat the vendor up
>> with the fixes and the official firmware was corrected.
> 
> Can you explain to me please why you can't do it with GRUB ?

It's doable with GRUB (if you rebuild GRUB modules to include it)

> I am using mainline GRUB and its acpi command all the time to update
> static ACPI tables for testing new features (ie IORT) and it works
> just fine for me (and you can still override the DSDT, which is
> likely to be the main source of bugs, through the CONFIG_ACPI_CUSTOM_DSDT
> config option, that works on ARM in mainline with no changes required).
> 
> I just want to understand if there is really a compelling reason
> for adding this stuff when we can easily implement its features
> through something that is usable today without any kernel changes.

We're looking for x86 feature parity in the base platform. Every time
there's a gratuitous differentiation it's a waste of time for folks
trying to figure out what is different on an ARM system. So if we
completely remove the ACPI override feature from the kernel and make
everyone use GRUB instead, then fine, but ARM shouldn't be special.

Jon.

-- 
Computer Architect | Sent from my Fedora powered laptop



Re: powerpc/pseries: start rtasd before PCI probing

2016-05-23 Thread Thomas Huth
On 23.05.2016 10:28, Greg Kurz wrote:
> A strange behaviour is observed when comparing PCI hotplug in QEMU, between
> x86 and pseries. If you consider the following steps:
> - start a VM
> - add a PCI device via the QEMU monitor before the rtasd has started (for
>   example starting the VM in paused state, or hotplug during FW or boot
>   loader)
> - resume the VM execution
> 
> The x86 kernel detects the PCI device, but the pseries one does not.
> 
> This happens because the rtasd kernel worker is currently started under
> device_initcall, while PCI probing happens earlier under subsys_initcall.
> 
> As a consequence, if we have a pending RTAS event at boot time, a message
> is printed and the event is dropped.
> 
> This patch moves all the initialization of rtasd to arch_initcall, which is
> run before subsys_call: this way, logging_enabled is true when the RTAS
> event pops up and it is not lost anymore.
> 
> The proc fs bits stay at device_initcall because they cannot be run before
> fs_initcall.
> 
> Signed-off-by: Greg Kurz 
> ---
>  arch/powerpc/kernel/rtasd.c |   19 ++-
>  1 file changed, 14 insertions(+), 5 deletions(-)

By the way, same is true for device UNplugging: When unplugging devices
in QEMU while the firmware is still running, they are never properly
removed from the guest. I've checked it, and your patch fixes this
problem as well! Great :-)

Tested-by: Thomas Huth 



Re: powerpc/pseries: start rtasd before PCI probing

2016-05-23 Thread Thomas Huth
On 23.05.2016 10:28, Greg Kurz wrote:
> A strange behaviour is observed when comparing PCI hotplug in QEMU, between
> x86 and pseries. If you consider the following steps:
> - start a VM
> - add a PCI device via the QEMU monitor before the rtasd has started (for
>   example starting the VM in paused state, or hotplug during FW or boot
>   loader)
> - resume the VM execution
> 
> The x86 kernel detects the PCI device, but the pseries one does not.
> 
> This happens because the rtasd kernel worker is currently started under
> device_initcall, while PCI probing happens earlier under subsys_initcall.
> 
> As a consequence, if we have a pending RTAS event at boot time, a message
> is printed and the event is dropped.
> 
> This patch moves all the initialization of rtasd to arch_initcall, which is
> run before subsys_call: this way, logging_enabled is true when the RTAS
> event pops up and it is not lost anymore.
> 
> The proc fs bits stay at device_initcall because they cannot be run before
> fs_initcall.
> 
> Signed-off-by: Greg Kurz 
> ---
>  arch/powerpc/kernel/rtasd.c |   19 ++-
>  1 file changed, 14 insertions(+), 5 deletions(-)

By the way, same is true for device UNplugging: When unplugging devices
in QEMU while the firmware is still running, they are never properly
removed from the guest. I've checked it, and your patch fixes this
problem as well! Great :-)

Tested-by: Thomas Huth 



Re: [PATCH] mm: make CONFIG_DEFERRED_STRUCT_PAGE_INIT depends on !FLATMEM explicitly

2016-05-23 Thread Michal Hocko
On Mon 23-05-16 09:54:31, Yang Shi wrote:
> Per the suggestion from Michal Hocko [1], CONFIG_DEFERRED_STRUCT_PAGE_INIT
> should be incompatible with FLATMEM, make this explicitly in Kconfig.

I guess the changelog could benefit from some clarification. What
do you think about the following:

"
DEFERRED_STRUCT_PAGE_INIT requires some ordering wrt other
initialization operations, e.g. page_ext_init has to happen after the
whole memmap is initialized properly. For SPARSEMEM this requires to
wait for page_alloc_init_late. Other memory models (e.g. flatmem) might
have different initialization layouts (page_ext_init_flatmem). Currently
DEFERRED_STRUCT_PAGE_INIT depends on MEMORY_HOTPLUG which in turn
depends on SPARSEMEM || X86_64_ACPI_NUMA
depends on ARCH_ENABLE_MEMORY_HOTPLUG

and X86_64_ACPI_NUMA depends on NUMA which in turn disable FLATMEM
memory model:
config ARCH_FLATMEM_ENABLE
def_bool y
depends on X86_32 && !NUMA

so FLATMEM is ruled out via dependency maze. Be explicit and disable
FLATMEM for DEFERRED_STRUCT_PAGE_INIT so that we do not reintroduce
subtle initialization bugs
"

> 
> [1] http://lkml.kernel.org/r/20160523073157.gd2...@dhcp22.suse.cz
> 
> Signed-off-by: Yang Shi 
> ---
>  mm/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/mm/Kconfig b/mm/Kconfig
> index 2664c11..22fa818 100644
> --- a/mm/Kconfig
> +++ b/mm/Kconfig
> @@ -649,6 +649,7 @@ config DEFERRED_STRUCT_PAGE_INIT
>   default n
>   depends on ARCH_SUPPORTS_DEFERRED_STRUCT_PAGE_INIT
>   depends on MEMORY_HOTPLUG
> + depends on !FLATMEM
>   help
> Ordinarily all struct pages are initialised during early boot in a
> single thread. On very large machines this can take a considerable
> -- 
> 2.0.2

-- 
Michal Hocko
SUSE Labs


Re: [PATCH] mm: make CONFIG_DEFERRED_STRUCT_PAGE_INIT depends on !FLATMEM explicitly

2016-05-23 Thread Michal Hocko
On Mon 23-05-16 09:54:31, Yang Shi wrote:
> Per the suggestion from Michal Hocko [1], CONFIG_DEFERRED_STRUCT_PAGE_INIT
> should be incompatible with FLATMEM, make this explicitly in Kconfig.

I guess the changelog could benefit from some clarification. What
do you think about the following:

"
DEFERRED_STRUCT_PAGE_INIT requires some ordering wrt other
initialization operations, e.g. page_ext_init has to happen after the
whole memmap is initialized properly. For SPARSEMEM this requires to
wait for page_alloc_init_late. Other memory models (e.g. flatmem) might
have different initialization layouts (page_ext_init_flatmem). Currently
DEFERRED_STRUCT_PAGE_INIT depends on MEMORY_HOTPLUG which in turn
depends on SPARSEMEM || X86_64_ACPI_NUMA
depends on ARCH_ENABLE_MEMORY_HOTPLUG

and X86_64_ACPI_NUMA depends on NUMA which in turn disable FLATMEM
memory model:
config ARCH_FLATMEM_ENABLE
def_bool y
depends on X86_32 && !NUMA

so FLATMEM is ruled out via dependency maze. Be explicit and disable
FLATMEM for DEFERRED_STRUCT_PAGE_INIT so that we do not reintroduce
subtle initialization bugs
"

> 
> [1] http://lkml.kernel.org/r/20160523073157.gd2...@dhcp22.suse.cz
> 
> Signed-off-by: Yang Shi 
> ---
>  mm/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/mm/Kconfig b/mm/Kconfig
> index 2664c11..22fa818 100644
> --- a/mm/Kconfig
> +++ b/mm/Kconfig
> @@ -649,6 +649,7 @@ config DEFERRED_STRUCT_PAGE_INIT
>   default n
>   depends on ARCH_SUPPORTS_DEFERRED_STRUCT_PAGE_INIT
>   depends on MEMORY_HOTPLUG
> + depends on !FLATMEM
>   help
> Ordinarily all struct pages are initialised during early boot in a
> single thread. On very large machines this can take a considerable
> -- 
> 2.0.2

-- 
Michal Hocko
SUSE Labs


Re: [PATCH] soc: qcom: provide mechanism for drivers to access L2 registers

2016-05-23 Thread Neil Leeder

On 5/23/2016 01:25 PM, Mark Rutland wrote:
> On Fri, May 20, 2016 at 03:13:07PM -0400, Neil Leeder wrote:
>> L2 registers are accessed using a select register and data
>> register pair. To prevent multiple concurrent writes to the
>> select register by independent drivers, the write to the
>> select register and the associated access of the data register
>> are protected with a lock. All drivers accessing the L2
>> registers use the set and get functions provided by
>> l2-accessors to ensure correct reads and writes to L2 registers.
> 
> What will this be used for? (i.e. which drivers want to touch the L2
> registers?).
> 
> Generally we expect FW to configure the caches and interconnect
> appropriately.

The primary use is in the L2 PMU driver, which will be posted shortly.

> 
>> Signed-off-by: Neil Leeder 
>> ---
>>  drivers/soc/qcom/Kconfig  |  9 +
>>  drivers/soc/qcom/Makefile |  1 +
>>  drivers/soc/qcom/l2-accessors.c   | 66 
>> +++
>>  include/linux/soc/qcom/l2-accessors.h | 27 ++
>>  4 files changed, 103 insertions(+)
>>  create mode 100644 drivers/soc/qcom/l2-accessors.c
>>  create mode 100644 include/linux/soc/qcom/l2-accessors.h
> 
> These are awfully generic file names (and function names). Which SoCs
> does this apply to?
> 
> It would be good to give these more specific names.

It's under soc/qcom, and dependent on ARCH_QCOM and (in v2) also on ARM64. It 
applies to all QCOM ARM64 SoCs.
Given that it can only be used in a QCOM driver, and the include path has qcom 
in it, I'd
prefer not to add redundancy by adding another qcom in there.

>> diff --git a/include/linux/soc/qcom/l2-accessors.h 
>> b/include/linux/soc/qcom/l2-accessors.h
>> new file mode 100644
>> index 000..563c114
>> --- /dev/null
>> +++ b/include/linux/soc/qcom/l2-accessors.h
>> @@ -0,0 +1,27 @@
>> +/*
>> + * Copyright (c) 2011-2016 The Linux Foundation. All rights reserved.
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 and
>> + * only version 2 as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> + */
>> +
> 
>> +#ifndef __QCOM_L2_ACCESSORS_H
>> +#define __QCOM_L2_ACCESSORS_H
>> +
>> +#ifdef CONFIG_QCOM_L2_ACCESSORS
>> +void set_l2_indirect_reg(u64 reg_addr, u64 val);
>> +u64 get_l2_indirect_reg(u64 reg_addr);
>> +#else
>> +static inline void set_l2_indirect_reg(u64 reg_addr, u64 val) {}
>> +static inline u64 get_l2_indirect_reg(u64 reg_addr)
>> +{
>> +return 0;
>> +}
> 
> Surely it would be better to error out on any unintentional use of these
> at build time?

This allows building code which is common to ARM SoCs and QCOM SoCs without 
having to ifdef out the
QCOM-specific pieces.

> 
> Thanks,
> Mark.
> 

Thanks,
Neil

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


Re: [PATCH] soc: qcom: provide mechanism for drivers to access L2 registers

2016-05-23 Thread Neil Leeder

On 5/23/2016 01:25 PM, Mark Rutland wrote:
> On Fri, May 20, 2016 at 03:13:07PM -0400, Neil Leeder wrote:
>> L2 registers are accessed using a select register and data
>> register pair. To prevent multiple concurrent writes to the
>> select register by independent drivers, the write to the
>> select register and the associated access of the data register
>> are protected with a lock. All drivers accessing the L2
>> registers use the set and get functions provided by
>> l2-accessors to ensure correct reads and writes to L2 registers.
> 
> What will this be used for? (i.e. which drivers want to touch the L2
> registers?).
> 
> Generally we expect FW to configure the caches and interconnect
> appropriately.

The primary use is in the L2 PMU driver, which will be posted shortly.

> 
>> Signed-off-by: Neil Leeder 
>> ---
>>  drivers/soc/qcom/Kconfig  |  9 +
>>  drivers/soc/qcom/Makefile |  1 +
>>  drivers/soc/qcom/l2-accessors.c   | 66 
>> +++
>>  include/linux/soc/qcom/l2-accessors.h | 27 ++
>>  4 files changed, 103 insertions(+)
>>  create mode 100644 drivers/soc/qcom/l2-accessors.c
>>  create mode 100644 include/linux/soc/qcom/l2-accessors.h
> 
> These are awfully generic file names (and function names). Which SoCs
> does this apply to?
> 
> It would be good to give these more specific names.

It's under soc/qcom, and dependent on ARCH_QCOM and (in v2) also on ARM64. It 
applies to all QCOM ARM64 SoCs.
Given that it can only be used in a QCOM driver, and the include path has qcom 
in it, I'd
prefer not to add redundancy by adding another qcom in there.

>> diff --git a/include/linux/soc/qcom/l2-accessors.h 
>> b/include/linux/soc/qcom/l2-accessors.h
>> new file mode 100644
>> index 000..563c114
>> --- /dev/null
>> +++ b/include/linux/soc/qcom/l2-accessors.h
>> @@ -0,0 +1,27 @@
>> +/*
>> + * Copyright (c) 2011-2016 The Linux Foundation. All rights reserved.
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 and
>> + * only version 2 as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> + */
>> +
> 
>> +#ifndef __QCOM_L2_ACCESSORS_H
>> +#define __QCOM_L2_ACCESSORS_H
>> +
>> +#ifdef CONFIG_QCOM_L2_ACCESSORS
>> +void set_l2_indirect_reg(u64 reg_addr, u64 val);
>> +u64 get_l2_indirect_reg(u64 reg_addr);
>> +#else
>> +static inline void set_l2_indirect_reg(u64 reg_addr, u64 val) {}
>> +static inline u64 get_l2_indirect_reg(u64 reg_addr)
>> +{
>> +return 0;
>> +}
> 
> Surely it would be better to error out on any unintentional use of these
> at build time?

This allows building code which is common to ARM SoCs and QCOM SoCs without 
having to ifdef out the
QCOM-specific pieces.

> 
> Thanks,
> Mark.
> 

Thanks,
Neil

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


[GIT PULL] ftracetest: Use proper logic to find process PID

2016-05-23 Thread Steven Rostedt

Linus,

Reviewing the selftest I recently submitted, I realize that the second part
of it uses my old hack to get the PID of the spawned background tasks,
which doesn't work for all shells, instead of the common use of $!.


Please pull the latest trace-v4.7-3 tree, which can be found at:


  git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git
trace-v4.7-3

Tag SHA1: 773c674f8cf1e96253bef9889e5e26f0f3e8dcbb
Head SHA1: 97f8827a8c7963756ae7d3ee898675b4667eca73


Steven Rostedt (Red Hat) (1):
  ftracetest: Use proper logic to find process PID


 .../selftests/ftrace/test.d/instances/instance-event.tc   | 15 +--
 1 file changed, 5 insertions(+), 10 deletions(-)
---
commit 97f8827a8c7963756ae7d3ee898675b4667eca73
Author: Steven Rostedt (Red Hat) 
Date:   Mon May 23 10:04:46 2016 -0400

ftracetest: Use proper logic to find process PID

Half of the test in instance-event.tc was updated to use $! to find the PID
of the previous background process that was launched, but the second part of
the test still used the parsing of "jobs", which does not work on all shells
like $! does.

Signed-off-by: Steven Rostedt 

diff --git a/tools/testing/selftests/ftrace/test.d/instances/instance-event.tc 
b/tools/testing/selftests/ftrace/test.d/instances/instance-event.tc
index 5f2abd03f16b..4c5a061a5b4e 100644
--- a/tools/testing/selftests/ftrace/test.d/instances/instance-event.tc
+++ b/tools/testing/selftests/ftrace/test.d/instances/instance-event.tc
@@ -92,28 +92,23 @@ instance_slam() {
 }
 
 instance_slam &
-x=`jobs -l`
-p1=`echo $x | cut -d' ' -f2`
+p1=$!
 echo $p1
 
 instance_slam &
-x=`jobs -l | tail -1`
-p2=`echo $x | cut -d' ' -f2`
+p2=$!
 echo $p2
 
 instance_slam &
-x=`jobs -l | tail -1`
-p3=`echo $x | cut -d' ' -f2`
+p3=$!
 echo $p3
 
 instance_slam &
-x=`jobs -l | tail -1`
-p4=`echo $x | cut -d' ' -f2`
+p4=$!
 echo $p4
 
 instance_slam &
-x=`jobs -l | tail -1`
-p5=`echo $x | cut -d' ' -f2`
+p5=$!
 echo $p5
 
 ls -lR >/dev/null


[GIT PULL] ftracetest: Use proper logic to find process PID

2016-05-23 Thread Steven Rostedt

Linus,

Reviewing the selftest I recently submitted, I realize that the second part
of it uses my old hack to get the PID of the spawned background tasks,
which doesn't work for all shells, instead of the common use of $!.


Please pull the latest trace-v4.7-3 tree, which can be found at:


  git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git
trace-v4.7-3

Tag SHA1: 773c674f8cf1e96253bef9889e5e26f0f3e8dcbb
Head SHA1: 97f8827a8c7963756ae7d3ee898675b4667eca73


Steven Rostedt (Red Hat) (1):
  ftracetest: Use proper logic to find process PID


 .../selftests/ftrace/test.d/instances/instance-event.tc   | 15 +--
 1 file changed, 5 insertions(+), 10 deletions(-)
---
commit 97f8827a8c7963756ae7d3ee898675b4667eca73
Author: Steven Rostedt (Red Hat) 
Date:   Mon May 23 10:04:46 2016 -0400

ftracetest: Use proper logic to find process PID

Half of the test in instance-event.tc was updated to use $! to find the PID
of the previous background process that was launched, but the second part of
the test still used the parsing of "jobs", which does not work on all shells
like $! does.

Signed-off-by: Steven Rostedt 

diff --git a/tools/testing/selftests/ftrace/test.d/instances/instance-event.tc 
b/tools/testing/selftests/ftrace/test.d/instances/instance-event.tc
index 5f2abd03f16b..4c5a061a5b4e 100644
--- a/tools/testing/selftests/ftrace/test.d/instances/instance-event.tc
+++ b/tools/testing/selftests/ftrace/test.d/instances/instance-event.tc
@@ -92,28 +92,23 @@ instance_slam() {
 }
 
 instance_slam &
-x=`jobs -l`
-p1=`echo $x | cut -d' ' -f2`
+p1=$!
 echo $p1
 
 instance_slam &
-x=`jobs -l | tail -1`
-p2=`echo $x | cut -d' ' -f2`
+p2=$!
 echo $p2
 
 instance_slam &
-x=`jobs -l | tail -1`
-p3=`echo $x | cut -d' ' -f2`
+p3=$!
 echo $p3
 
 instance_slam &
-x=`jobs -l | tail -1`
-p4=`echo $x | cut -d' ' -f2`
+p4=$!
 echo $p4
 
 instance_slam &
-x=`jobs -l | tail -1`
-p5=`echo $x | cut -d' ' -f2`
+p5=$!
 echo $p5
 
 ls -lR >/dev/null


Re: [PATCH] soc: qcom: provide mechanism for drivers to access L2 registers

2016-05-23 Thread Neil Leeder


On 5/23/2016 01:04 PM, Stephen Boyd wrote:
> On 05/23/2016 08:43 AM, Neil Leeder wrote:
>>
>> On 5/20/2016 05:19 PM, Stephen Boyd wrote:
>>
>>>
>>> Is there a patch to add sysreg.h to arch/arm? It would be nice to use
>>> one l2 accessor API on arm64 and arm.
>>>
>> Sounds like a good thing for the next person who submits a krait L2 patch to 
>> consider.
>>
> 
> Heh, ok. If it isn't supported in this patch then we need to make the
> config depend on ARM64 so that this doesn't fail to compile on ARM.
> 

Ok, I'll add that in v2 too.

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


Re: [PATCH] soc: qcom: provide mechanism for drivers to access L2 registers

2016-05-23 Thread Neil Leeder


On 5/23/2016 01:04 PM, Stephen Boyd wrote:
> On 05/23/2016 08:43 AM, Neil Leeder wrote:
>>
>> On 5/20/2016 05:19 PM, Stephen Boyd wrote:
>>
>>>
>>> Is there a patch to add sysreg.h to arch/arm? It would be nice to use
>>> one l2 accessor API on arm64 and arm.
>>>
>> Sounds like a good thing for the next person who submits a krait L2 patch to 
>> consider.
>>
> 
> Heh, ok. If it isn't supported in this patch then we need to make the
> config depend on ARM64 so that this doesn't fail to compile on ARM.
> 

Ok, I'll add that in v2 too.

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


Re: [v2 PATCH] mm: move page_ext_init after all struct pages are initialized

2016-05-23 Thread Michal Hocko
On Mon 23-05-16 09:42:00, Shi, Yang wrote:
> On 5/23/2016 12:31 AM, Michal Hocko wrote:
> > On Fri 20-05-16 08:41:09, Shi, Yang wrote:
> > > On 5/20/2016 6:16 AM, Michal Hocko wrote:
> > > > On Thu 19-05-16 15:13:26, Yang Shi wrote:
> > > > [...]
> > > > > diff --git a/init/main.c b/init/main.c
> > > > > index b3c6e36..2075faf 100644
> > > > > --- a/init/main.c
> > > > > +++ b/init/main.c
> > > > > @@ -606,7 +606,6 @@ asmlinkage __visible void __init 
> > > > > start_kernel(void)
> > > > >   initrd_start = 0;
> > > > >   }
> > > > >  #endif
> > > > > - page_ext_init();
> > > > >   debug_objects_mem_init();
> > > > >   kmemleak_init();
> > > > >   setup_per_cpu_pageset();
> > > > > @@ -1004,6 +1003,8 @@ static noinline void __init 
> > > > > kernel_init_freeable(void)
> > > > >   sched_init_smp();
> > > > > 
> > > > >   page_alloc_init_late();
> > > > > + /* Initialize page ext after all struct pages are initializaed 
> > > > > */
> > > > > + page_ext_init();
> > > > > 
> > > > >   do_basic_setup();
> > > > 
> > > > I might be missing something but don't we have the same problem with
> > > > CONFIG_FLATMEM? page_ext_init_flatmem is called way earlier. Or
> > > > CONFIG_DEFERRED_STRUCT_PAGE_INIT is never enabled for CONFIG_FLATMEM?
> > > 
> > > Yes, CONFIG_DEFERRED_STRUCT_PAGE_INIT depends on MEMORY_HOTPLUG which
> > > depends on SPARSEMEM. So, this config is not valid for FLATMEM at all.
> > 
> > Well
> > config MEMORY_HOTPLUG
> > bool "Allow for memory hot-add"
> > depends on SPARSEMEM || X86_64_ACPI_NUMA
> > depends on ARCH_ENABLE_MEMORY_HOTPLUG
> > 
> > I wasn't really sure about X86_64_ACPI_NUMA dependency branch which
> > depends on X86_64 && NUMA && ACPI && PCI and that didn't sound like
> > SPARSEMEM only. If the FLATMEM shouldn't exist with
> 
> Actually, FLATMEMT depends on !NUMA.

Ahh, OK, you are right! Thanks for the clarification.
-- 
Michal Hocko
SUSE Labs


Re: [v2 PATCH] mm: move page_ext_init after all struct pages are initialized

2016-05-23 Thread Michal Hocko
On Mon 23-05-16 09:42:00, Shi, Yang wrote:
> On 5/23/2016 12:31 AM, Michal Hocko wrote:
> > On Fri 20-05-16 08:41:09, Shi, Yang wrote:
> > > On 5/20/2016 6:16 AM, Michal Hocko wrote:
> > > > On Thu 19-05-16 15:13:26, Yang Shi wrote:
> > > > [...]
> > > > > diff --git a/init/main.c b/init/main.c
> > > > > index b3c6e36..2075faf 100644
> > > > > --- a/init/main.c
> > > > > +++ b/init/main.c
> > > > > @@ -606,7 +606,6 @@ asmlinkage __visible void __init 
> > > > > start_kernel(void)
> > > > >   initrd_start = 0;
> > > > >   }
> > > > >  #endif
> > > > > - page_ext_init();
> > > > >   debug_objects_mem_init();
> > > > >   kmemleak_init();
> > > > >   setup_per_cpu_pageset();
> > > > > @@ -1004,6 +1003,8 @@ static noinline void __init 
> > > > > kernel_init_freeable(void)
> > > > >   sched_init_smp();
> > > > > 
> > > > >   page_alloc_init_late();
> > > > > + /* Initialize page ext after all struct pages are initializaed 
> > > > > */
> > > > > + page_ext_init();
> > > > > 
> > > > >   do_basic_setup();
> > > > 
> > > > I might be missing something but don't we have the same problem with
> > > > CONFIG_FLATMEM? page_ext_init_flatmem is called way earlier. Or
> > > > CONFIG_DEFERRED_STRUCT_PAGE_INIT is never enabled for CONFIG_FLATMEM?
> > > 
> > > Yes, CONFIG_DEFERRED_STRUCT_PAGE_INIT depends on MEMORY_HOTPLUG which
> > > depends on SPARSEMEM. So, this config is not valid for FLATMEM at all.
> > 
> > Well
> > config MEMORY_HOTPLUG
> > bool "Allow for memory hot-add"
> > depends on SPARSEMEM || X86_64_ACPI_NUMA
> > depends on ARCH_ENABLE_MEMORY_HOTPLUG
> > 
> > I wasn't really sure about X86_64_ACPI_NUMA dependency branch which
> > depends on X86_64 && NUMA && ACPI && PCI and that didn't sound like
> > SPARSEMEM only. If the FLATMEM shouldn't exist with
> 
> Actually, FLATMEMT depends on !NUMA.

Ahh, OK, you are right! Thanks for the clarification.
-- 
Michal Hocko
SUSE Labs


Re: [PATCH v2] KVM: halt-polling: poll if emulated lapic timer will fire soon

2016-05-23 Thread David Matlack
On Sun, May 22, 2016 at 6:26 PM, Yang Zhang  wrote:
> On 2016/5/21 2:37, David Matlack wrote:
>>
>> It's not obvious to me why polling for a timer interrupt would improve
>> context switch latency. Can you explain a bit more?
>
>
> We have a workload which using high resolution timer(less than 1ms) inside
> guest. It rely on the timer to wakeup itself. Sometimes the timer is
> expected to fired just after the VCPU is blocked due to execute halt
> instruction. But the thread who is running in the CPU will turn off the
> hardware interrupt for long time due to disk access. This will cause the
> timer interrupt been blocked until the interrupt is re-open.

Does this happen on the idle thread (swapper)? If not, halt-polling
may not help; it only polls if there are no other runnable threads.

> For optimization, we let VCPU to poll for a while if the next timer will
> arrive soon before schedule out. And the result shows good when running
> several workloads inside guest.

Thanks for the explanation, I appreciate it.

>
> --
> best regards
> yang


Re: [PATCH v2] KVM: halt-polling: poll if emulated lapic timer will fire soon

2016-05-23 Thread David Matlack
On Sun, May 22, 2016 at 6:26 PM, Yang Zhang  wrote:
> On 2016/5/21 2:37, David Matlack wrote:
>>
>> It's not obvious to me why polling for a timer interrupt would improve
>> context switch latency. Can you explain a bit more?
>
>
> We have a workload which using high resolution timer(less than 1ms) inside
> guest. It rely on the timer to wakeup itself. Sometimes the timer is
> expected to fired just after the VCPU is blocked due to execute halt
> instruction. But the thread who is running in the CPU will turn off the
> hardware interrupt for long time due to disk access. This will cause the
> timer interrupt been blocked until the interrupt is re-open.

Does this happen on the idle thread (swapper)? If not, halt-polling
may not help; it only polls if there are no other runnable threads.

> For optimization, we let VCPU to poll for a while if the next timer will
> arrive soon before schedule out. And the result shows good when running
> several workloads inside guest.

Thanks for the explanation, I appreciate it.

>
> --
> best regards
> yang


Re: [PATCH 1/4] isa: Allow ISA-style drivers on modern systems

2016-05-23 Thread Linus Torvalds
On Mon, May 23, 2016 at 7:58 AM, William Breathitt Gray
 wrote:
>
> For now, the ISA_BUS Kconfig option is only be available on X86
> architectures. Support for other architectures may be added as required.

So I'd prefer to see that

> +config ISA_BUS_API
> +   def_bool ISA

part in arch/Kconfig.

Why?

Because other architectures _already_ define that ISA symbol, and we
want the "ISA_BUS_API" to be a complete superset of ISA.

So whenever ISA is enabled, ISA_BUS_API should be enabled.

And the way you did that, that's not true. Now, if you were to enable
ISA on ARM, you'd not get ISA_BUS_API. And that sounds insane to me.
It also sounds *wrong* because it effectively changes the meaning of
this:

  --- a/drivers/base/Makefile
  +++ b/drivers/base/Makefile
  @@ -10,7 +10,7 @@ obj-$(CONFIG_DMA_CMA) += dma-contiguous.o
  -obj-$(CONFIG_ISA)  += isa.o
  +obj-$(CONFIG_ISA_BUS_API)  += isa.o

where now that "isa.c" file gets built only on x86, whereas it *used*
to get built whenever ISA was enabled.

So the reason I suggested a separate ISA_BUS_API config option (that
then a particular architecture can choose to enable, in this case the
x86 choice of selecting ISA_BUS) was _exactly_ this issue. The plain
"ISA" config variable is not limited to x86, and the new subset of it
(the ISA_BUS_API) thus also must not be limited to just x86.

   Linus


Re: [PATCH 1/4] isa: Allow ISA-style drivers on modern systems

2016-05-23 Thread Linus Torvalds
On Mon, May 23, 2016 at 7:58 AM, William Breathitt Gray
 wrote:
>
> For now, the ISA_BUS Kconfig option is only be available on X86
> architectures. Support for other architectures may be added as required.

So I'd prefer to see that

> +config ISA_BUS_API
> +   def_bool ISA

part in arch/Kconfig.

Why?

Because other architectures _already_ define that ISA symbol, and we
want the "ISA_BUS_API" to be a complete superset of ISA.

So whenever ISA is enabled, ISA_BUS_API should be enabled.

And the way you did that, that's not true. Now, if you were to enable
ISA on ARM, you'd not get ISA_BUS_API. And that sounds insane to me.
It also sounds *wrong* because it effectively changes the meaning of
this:

  --- a/drivers/base/Makefile
  +++ b/drivers/base/Makefile
  @@ -10,7 +10,7 @@ obj-$(CONFIG_DMA_CMA) += dma-contiguous.o
  -obj-$(CONFIG_ISA)  += isa.o
  +obj-$(CONFIG_ISA_BUS_API)  += isa.o

where now that "isa.c" file gets built only on x86, whereas it *used*
to get built whenever ISA was enabled.

So the reason I suggested a separate ISA_BUS_API config option (that
then a particular architecture can choose to enable, in this case the
x86 choice of selecting ISA_BUS) was _exactly_ this issue. The plain
"ISA" config variable is not limited to x86, and the new subset of it
(the ISA_BUS_API) thus also must not be limited to just x86.

   Linus


Re: [PATCH v3] KVM: halt-polling: poll if emulated lapic timer will fire soon

2016-05-23 Thread David Matlack
On Sun, May 22, 2016 at 5:42 PM, Wanpeng Li  wrote:
> From: Wanpeng Li 

I'm ok with this patch, but I'd like to better understand the target
workloads. What type of workloads do you expect to benefit from this?

>
> If an emulated lapic timer will fire soon(in the scope of 10us the
> base of dynamic halt-polling, lower-end of message passing workload
> latency TCP_RR's poll time < 10us) we can treat it as a short halt,
> and poll to wait it fire, the fire callback apic_timer_fn() will set
> KVM_REQ_PENDING_TIMER, and this flag will be check during busy poll.
> This can avoid context switch overhead and the latency which we wake
> up vCPU.
>
> This feature is slightly different from current advance expiration
> way. Advance expiration rely on the vCPU is running(do polling before
> vmentry). But in some cases, the timer interrupt may be blocked by
> other thread(i.e., IF bit is clear) and vCPU cannot be scheduled to
> run immediately. So even advance the timer early, vCPU may still see
> the latency. But polling is different, it ensures the vCPU to aware
> the timer expiration before schedule out.
>
> iperf TCP get ~6% bandwidth improvement.

I think my question got lost in the previous thread :). Can you
explain why TCP bandwidth improves with this patch?

>
> Cc: Paolo Bonzini 
> Cc: Radim Krčmář 
> Cc: David Matlack 
> Cc: Christian Borntraeger 
> Cc: Yang Zhang 
> Signed-off-by: Wanpeng Li 
> ---
> v2 -> v3:
>  * add Yang's statement to patch description
> v1 -> v2:
>  * add return statement to non-x86 archs
>  * capture never expire case for x86 (hrtimer is not started)
>
>  arch/arm/include/asm/kvm_host.h |  4 
>  arch/arm64/include/asm/kvm_host.h   |  4 
>  arch/mips/include/asm/kvm_host.h|  4 
>  arch/powerpc/include/asm/kvm_host.h |  4 
>  arch/s390/include/asm/kvm_host.h|  4 
>  arch/x86/kvm/lapic.c| 11 +++
>  arch/x86/kvm/lapic.h|  1 +
>  arch/x86/kvm/x86.c  |  5 +
>  include/linux/kvm_host.h|  1 +
>  virt/kvm/kvm_main.c | 14 ++
>  10 files changed, 48 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
> index 4cd8732..a5fd858 100644
> --- a/arch/arm/include/asm/kvm_host.h
> +++ b/arch/arm/include/asm/kvm_host.h
> @@ -284,6 +284,10 @@ static inline void kvm_arch_sync_events(struct kvm *kvm) 
> {}
>  static inline void kvm_arch_vcpu_uninit(struct kvm_vcpu *vcpu) {}
>  static inline void kvm_arch_sched_in(struct kvm_vcpu *vcpu, int cpu) {}
>  static inline void kvm_arch_vcpu_block_finish(struct kvm_vcpu *vcpu) {}
> +static inline u64 kvm_arch_timer_remaining(struct kvm_vcpu *vcpu)
> +{
> +   return -1ULL;
> +}
>
>  static inline void kvm_arm_init_debug(void) {}
>  static inline void kvm_arm_setup_debug(struct kvm_vcpu *vcpu) {}
> diff --git a/arch/arm64/include/asm/kvm_host.h 
> b/arch/arm64/include/asm/kvm_host.h
> index d49399d..94e227a 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -359,6 +359,10 @@ static inline void kvm_arch_sync_events(struct kvm *kvm) 
> {}
>  static inline void kvm_arch_vcpu_uninit(struct kvm_vcpu *vcpu) {}
>  static inline void kvm_arch_sched_in(struct kvm_vcpu *vcpu, int cpu) {}
>  static inline void kvm_arch_vcpu_block_finish(struct kvm_vcpu *vcpu) {}
> +static inline u64 kvm_arch_timer_remaining(struct kvm_vcpu *vcpu)
> +{
> +   return -1ULL;
> +}
>
>  void kvm_arm_init_debug(void);
>  void kvm_arm_setup_debug(struct kvm_vcpu *vcpu);
> diff --git a/arch/mips/include/asm/kvm_host.h 
> b/arch/mips/include/asm/kvm_host.h
> index 9a37a10..456bc42 100644
> --- a/arch/mips/include/asm/kvm_host.h
> +++ b/arch/mips/include/asm/kvm_host.h
> @@ -813,6 +813,10 @@ static inline void kvm_arch_vcpu_uninit(struct kvm_vcpu 
> *vcpu) {}
>  static inline void kvm_arch_sched_in(struct kvm_vcpu *vcpu, int cpu) {}
>  static inline void kvm_arch_vcpu_blocking(struct kvm_vcpu *vcpu) {}
>  static inline void kvm_arch_vcpu_unblocking(struct kvm_vcpu *vcpu) {}
> +static inline u64 kvm_arch_timer_remaining(struct kvm_vcpu *vcpu)
> +{
> +   return -1ULL;
> +}
>  static inline void kvm_arch_vcpu_block_finish(struct kvm_vcpu *vcpu) {}
>
>  #endif /* __MIPS_KVM_HOST_H__ */
> diff --git a/arch/powerpc/include/asm/kvm_host.h 
> b/arch/powerpc/include/asm/kvm_host.h
> index ec35af3..5986c79 100644
> --- a/arch/powerpc/include/asm/kvm_host.h
> +++ b/arch/powerpc/include/asm/kvm_host.h
> @@ -729,5 +729,9 @@ static inline void kvm_arch_exit(void) {}
>  static inline void kvm_arch_vcpu_blocking(struct kvm_vcpu *vcpu) {}
>  static inline void kvm_arch_vcpu_unblocking(struct kvm_vcpu *vcpu) {}
>  static inline void kvm_arch_vcpu_block_finish(struct kvm_vcpu *vcpu) {}
> +static 

Re: [PATCH v3] KVM: halt-polling: poll if emulated lapic timer will fire soon

2016-05-23 Thread David Matlack
On Sun, May 22, 2016 at 5:42 PM, Wanpeng Li  wrote:
> From: Wanpeng Li 

I'm ok with this patch, but I'd like to better understand the target
workloads. What type of workloads do you expect to benefit from this?

>
> If an emulated lapic timer will fire soon(in the scope of 10us the
> base of dynamic halt-polling, lower-end of message passing workload
> latency TCP_RR's poll time < 10us) we can treat it as a short halt,
> and poll to wait it fire, the fire callback apic_timer_fn() will set
> KVM_REQ_PENDING_TIMER, and this flag will be check during busy poll.
> This can avoid context switch overhead and the latency which we wake
> up vCPU.
>
> This feature is slightly different from current advance expiration
> way. Advance expiration rely on the vCPU is running(do polling before
> vmentry). But in some cases, the timer interrupt may be blocked by
> other thread(i.e., IF bit is clear) and vCPU cannot be scheduled to
> run immediately. So even advance the timer early, vCPU may still see
> the latency. But polling is different, it ensures the vCPU to aware
> the timer expiration before schedule out.
>
> iperf TCP get ~6% bandwidth improvement.

I think my question got lost in the previous thread :). Can you
explain why TCP bandwidth improves with this patch?

>
> Cc: Paolo Bonzini 
> Cc: Radim Krčmář 
> Cc: David Matlack 
> Cc: Christian Borntraeger 
> Cc: Yang Zhang 
> Signed-off-by: Wanpeng Li 
> ---
> v2 -> v3:
>  * add Yang's statement to patch description
> v1 -> v2:
>  * add return statement to non-x86 archs
>  * capture never expire case for x86 (hrtimer is not started)
>
>  arch/arm/include/asm/kvm_host.h |  4 
>  arch/arm64/include/asm/kvm_host.h   |  4 
>  arch/mips/include/asm/kvm_host.h|  4 
>  arch/powerpc/include/asm/kvm_host.h |  4 
>  arch/s390/include/asm/kvm_host.h|  4 
>  arch/x86/kvm/lapic.c| 11 +++
>  arch/x86/kvm/lapic.h|  1 +
>  arch/x86/kvm/x86.c  |  5 +
>  include/linux/kvm_host.h|  1 +
>  virt/kvm/kvm_main.c | 14 ++
>  10 files changed, 48 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
> index 4cd8732..a5fd858 100644
> --- a/arch/arm/include/asm/kvm_host.h
> +++ b/arch/arm/include/asm/kvm_host.h
> @@ -284,6 +284,10 @@ static inline void kvm_arch_sync_events(struct kvm *kvm) 
> {}
>  static inline void kvm_arch_vcpu_uninit(struct kvm_vcpu *vcpu) {}
>  static inline void kvm_arch_sched_in(struct kvm_vcpu *vcpu, int cpu) {}
>  static inline void kvm_arch_vcpu_block_finish(struct kvm_vcpu *vcpu) {}
> +static inline u64 kvm_arch_timer_remaining(struct kvm_vcpu *vcpu)
> +{
> +   return -1ULL;
> +}
>
>  static inline void kvm_arm_init_debug(void) {}
>  static inline void kvm_arm_setup_debug(struct kvm_vcpu *vcpu) {}
> diff --git a/arch/arm64/include/asm/kvm_host.h 
> b/arch/arm64/include/asm/kvm_host.h
> index d49399d..94e227a 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -359,6 +359,10 @@ static inline void kvm_arch_sync_events(struct kvm *kvm) 
> {}
>  static inline void kvm_arch_vcpu_uninit(struct kvm_vcpu *vcpu) {}
>  static inline void kvm_arch_sched_in(struct kvm_vcpu *vcpu, int cpu) {}
>  static inline void kvm_arch_vcpu_block_finish(struct kvm_vcpu *vcpu) {}
> +static inline u64 kvm_arch_timer_remaining(struct kvm_vcpu *vcpu)
> +{
> +   return -1ULL;
> +}
>
>  void kvm_arm_init_debug(void);
>  void kvm_arm_setup_debug(struct kvm_vcpu *vcpu);
> diff --git a/arch/mips/include/asm/kvm_host.h 
> b/arch/mips/include/asm/kvm_host.h
> index 9a37a10..456bc42 100644
> --- a/arch/mips/include/asm/kvm_host.h
> +++ b/arch/mips/include/asm/kvm_host.h
> @@ -813,6 +813,10 @@ static inline void kvm_arch_vcpu_uninit(struct kvm_vcpu 
> *vcpu) {}
>  static inline void kvm_arch_sched_in(struct kvm_vcpu *vcpu, int cpu) {}
>  static inline void kvm_arch_vcpu_blocking(struct kvm_vcpu *vcpu) {}
>  static inline void kvm_arch_vcpu_unblocking(struct kvm_vcpu *vcpu) {}
> +static inline u64 kvm_arch_timer_remaining(struct kvm_vcpu *vcpu)
> +{
> +   return -1ULL;
> +}
>  static inline void kvm_arch_vcpu_block_finish(struct kvm_vcpu *vcpu) {}
>
>  #endif /* __MIPS_KVM_HOST_H__ */
> diff --git a/arch/powerpc/include/asm/kvm_host.h 
> b/arch/powerpc/include/asm/kvm_host.h
> index ec35af3..5986c79 100644
> --- a/arch/powerpc/include/asm/kvm_host.h
> +++ b/arch/powerpc/include/asm/kvm_host.h
> @@ -729,5 +729,9 @@ static inline void kvm_arch_exit(void) {}
>  static inline void kvm_arch_vcpu_blocking(struct kvm_vcpu *vcpu) {}
>  static inline void kvm_arch_vcpu_unblocking(struct kvm_vcpu *vcpu) {}
>  static inline void kvm_arch_vcpu_block_finish(struct kvm_vcpu *vcpu) {}
> +static inline u64 kvm_arch_timer_remaining(struct kvm_vcpu *vcpu)
> +{
> +   return -1ULL;
> +}
>
>  #endif /* __POWERPC_KVM_HOST_H__ */
> diff --git a/arch/s390/include/asm/kvm_host.h 

Re: [PATCH] Input: wacom_w8001 - Ignore bogus idx values in interrupt

2016-05-23 Thread Ping Cheng
On Mon, May 23, 2016 at 9:52 AM, Dmitry Torokhov
 wrote:
> On Sun, May 22, 2016 at 10:21:45PM -0700, Ping Cheng wrote:
>> Hi Chris,
>>
>> On Sun, May 22, 2016 at 6:42 PM, Chris J Arges
>>  wrote:
>> > I've noticed crashes when using my x60t using a coreboot bios. When using
>> > the pen I can produce a crash simply by tapping a few times. This
>> > generates an event which has an idx of 0xc. This in turn crashes the
>> > machine because the array access is greater than W8001_MAX_LENGTH. This
>> > patch checks for bogus values and filters them in order to prevent crashes.
>>
>> Thank you for submitting a patch in addition to reporting the issue.
>>
>> > Signed-off-by: Chris J Arges 
>> > ---
>> >  drivers/input/touchscreen/wacom_w8001.c | 9 +
>> >  1 file changed, 9 insertions(+)
>> >
>> > diff --git a/drivers/input/touchscreen/wacom_w8001.c 
>> > b/drivers/input/touchscreen/wacom_w8001.c
>> > index bab3c6a..c858200 100644
>> > --- a/drivers/input/touchscreen/wacom_w8001.c
>> > +++ b/drivers/input/touchscreen/wacom_w8001.c
>> > @@ -283,6 +283,15 @@ static irqreturn_t w8001_interrupt(struct serio 
>> > *serio,
>> > unsigned char tmp;
>> >
>> > w8001->data[w8001->idx] = data;
>> > +
>> > +   /* ignore bogus idx values */
>> > +   if (w8001->idx >= W8001_MAX_LENGTH) {
>> > +   pr_info("w8001: ignored interrupt: data 0x%02x idx %d\n", 
>> > data,
>> > +   w8001->idx);
>> > +   w8001->idx = 0;
>> > +   return IRQ_HANDLED;
>> > +   }
>> > +
>>
>> I don't have an x60t system to test with. I wonder if your system
>> supports two finger touch or not. We at least have a bug in the code
>> since W8001_MAX_LENGTH should be 13 instead of 11. How come no one had
>> encountered that issue before?
>>
>> I'm going to email a patch to the list. Please test it and let us know
>> your result. Maybe we still need your patch if your device doesn't
>> support two finger touch or the idx=0xc can't be fixed by
>> W8001_MAX_LENGTH=13.
>
> Just so we are clear this version of the patch is buggy as we check the
> index only after [potentially] writing past the array bounds of
> w8001->data[].

Thanks for the heads up. I noticed that last night. Since it breaks
two-finger touch, we won't use it anyway.

My other patch is still necessary though. You'll need to change:

From: wacom 

to

From: Ping Cheng 

I made it on a brand new system, which I didn't setup the environment
properly. I can update the patch if that's what you like...

Ping


Re: [PATCH] Input: wacom_w8001 - Ignore bogus idx values in interrupt

2016-05-23 Thread Ping Cheng
On Mon, May 23, 2016 at 9:52 AM, Dmitry Torokhov
 wrote:
> On Sun, May 22, 2016 at 10:21:45PM -0700, Ping Cheng wrote:
>> Hi Chris,
>>
>> On Sun, May 22, 2016 at 6:42 PM, Chris J Arges
>>  wrote:
>> > I've noticed crashes when using my x60t using a coreboot bios. When using
>> > the pen I can produce a crash simply by tapping a few times. This
>> > generates an event which has an idx of 0xc. This in turn crashes the
>> > machine because the array access is greater than W8001_MAX_LENGTH. This
>> > patch checks for bogus values and filters them in order to prevent crashes.
>>
>> Thank you for submitting a patch in addition to reporting the issue.
>>
>> > Signed-off-by: Chris J Arges 
>> > ---
>> >  drivers/input/touchscreen/wacom_w8001.c | 9 +
>> >  1 file changed, 9 insertions(+)
>> >
>> > diff --git a/drivers/input/touchscreen/wacom_w8001.c 
>> > b/drivers/input/touchscreen/wacom_w8001.c
>> > index bab3c6a..c858200 100644
>> > --- a/drivers/input/touchscreen/wacom_w8001.c
>> > +++ b/drivers/input/touchscreen/wacom_w8001.c
>> > @@ -283,6 +283,15 @@ static irqreturn_t w8001_interrupt(struct serio 
>> > *serio,
>> > unsigned char tmp;
>> >
>> > w8001->data[w8001->idx] = data;
>> > +
>> > +   /* ignore bogus idx values */
>> > +   if (w8001->idx >= W8001_MAX_LENGTH) {
>> > +   pr_info("w8001: ignored interrupt: data 0x%02x idx %d\n", 
>> > data,
>> > +   w8001->idx);
>> > +   w8001->idx = 0;
>> > +   return IRQ_HANDLED;
>> > +   }
>> > +
>>
>> I don't have an x60t system to test with. I wonder if your system
>> supports two finger touch or not. We at least have a bug in the code
>> since W8001_MAX_LENGTH should be 13 instead of 11. How come no one had
>> encountered that issue before?
>>
>> I'm going to email a patch to the list. Please test it and let us know
>> your result. Maybe we still need your patch if your device doesn't
>> support two finger touch or the idx=0xc can't be fixed by
>> W8001_MAX_LENGTH=13.
>
> Just so we are clear this version of the patch is buggy as we check the
> index only after [potentially] writing past the array bounds of
> w8001->data[].

Thanks for the heads up. I noticed that last night. Since it breaks
two-finger touch, we won't use it anyway.

My other patch is still necessary though. You'll need to change:

From: wacom 

to

From: Ping Cheng 

I made it on a brand new system, which I didn't setup the environment
properly. I can update the patch if that's what you like...

Ping


Re: [PATCH] kernel/kcov: unproxify debugfs file's fops

2016-05-23 Thread Kees Cook
On Mon, May 23, 2016 at 6:45 AM, Nicolai Stange <nicsta...@gmail.com> wrote:
> Since commit 49d200deaa68 ("debugfs: prevent access to removed files'
> private data"), a debugfs file's file_operations methods get proxied
> through lifetime aware wrappers.
>
> However, only a certain subset of the file_operations members is supported
> by debugfs and ->mmap isn't among them -- it appears to be NULL from the
> VFS layer's perspective.
>
> This behaviour breaks the /sys/kernel/debug/kcov file introduced
> concurrently with commit 5c9a8750a640 ("kernel: add kcov code coverage").
>
> Since that file never gets removed, there is no file removal race and thus,
> a lifetime checking proxy isn't needed.
>
> Avoid the proxying for /sys/kernel/debug/kcov by creating it via
> debugfs_create_file_unsafe() rather than debugfs_create_file().
>
> Fixes: 49d200deaa68 ("debugfs: prevent access to removed files' private
>   data")
> Fixes: 5c9a8750a640 ("kernel: add kcov code coverage")
> Signed-off-by: Nicolai Stange <nicsta...@gmail.com>
> ---
>  Applicable to linux-next 20160523.
>  In particular, it depends on
>  - c64688081490 ("debugfs: add support for self-protecting attribute file
>   fops")
>  - 5c9a8750a640 ("kernel: add kcov code coverage")
>
>  This issue has been debugged and reported by
>  Sasha Levin <sasha.le...@oracle.com>:
>  http://lkml.kernel.org/g/573f4200.3080...@oracle.com
>
>  kernel/kcov.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/kcov.c b/kernel/kcov.c
> index a02f2dd..4c349dd 100644
> --- a/kernel/kcov.c
> +++ b/kernel/kcov.c
> @@ -264,7 +264,7 @@ static const struct file_operations kcov_fops = {
>
>  static int __init kcov_init(void)
>  {
> -   if (!debugfs_create_file("kcov", 0600, NULL, NULL, _fops)) {
> +   if (!debugfs_create_file_unsafe("kcov", 0600, NULL, NULL, 
> _fops)) {

It might make sense to add a comment above this to describe why
"unsafe" is not unsafe in this case.

-Kees

> pr_err("failed to create kcov in debugfs\n");
> return -ENOMEM;
> }
> --
> 2.8.2
>



-- 
Kees Cook
Chrome OS & Brillo Security


Re: [PATCH] kernel/kcov: unproxify debugfs file's fops

2016-05-23 Thread Kees Cook
On Mon, May 23, 2016 at 6:45 AM, Nicolai Stange  wrote:
> Since commit 49d200deaa68 ("debugfs: prevent access to removed files'
> private data"), a debugfs file's file_operations methods get proxied
> through lifetime aware wrappers.
>
> However, only a certain subset of the file_operations members is supported
> by debugfs and ->mmap isn't among them -- it appears to be NULL from the
> VFS layer's perspective.
>
> This behaviour breaks the /sys/kernel/debug/kcov file introduced
> concurrently with commit 5c9a8750a640 ("kernel: add kcov code coverage").
>
> Since that file never gets removed, there is no file removal race and thus,
> a lifetime checking proxy isn't needed.
>
> Avoid the proxying for /sys/kernel/debug/kcov by creating it via
> debugfs_create_file_unsafe() rather than debugfs_create_file().
>
> Fixes: 49d200deaa68 ("debugfs: prevent access to removed files' private
>   data")
> Fixes: 5c9a8750a640 ("kernel: add kcov code coverage")
> Signed-off-by: Nicolai Stange 
> ---
>  Applicable to linux-next 20160523.
>  In particular, it depends on
>  - c64688081490 ("debugfs: add support for self-protecting attribute file
>   fops")
>  - 5c9a8750a640 ("kernel: add kcov code coverage")
>
>  This issue has been debugged and reported by
>  Sasha Levin :
>  http://lkml.kernel.org/g/573f4200.3080...@oracle.com
>
>  kernel/kcov.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/kcov.c b/kernel/kcov.c
> index a02f2dd..4c349dd 100644
> --- a/kernel/kcov.c
> +++ b/kernel/kcov.c
> @@ -264,7 +264,7 @@ static const struct file_operations kcov_fops = {
>
>  static int __init kcov_init(void)
>  {
> -   if (!debugfs_create_file("kcov", 0600, NULL, NULL, _fops)) {
> +   if (!debugfs_create_file_unsafe("kcov", 0600, NULL, NULL, 
> _fops)) {

It might make sense to add a comment above this to describe why
"unsafe" is not unsafe in this case.

-Kees

> pr_err("failed to create kcov in debugfs\n");
> return -ENOMEM;
> }
> --
> 2.8.2
>



-- 
Kees Cook
Chrome OS & Brillo Security


Applied "regulator: mt6397: Constify struct regulator_ops" to the regulator tree

2016-05-23 Thread Mark Brown
The patch

   regulator: mt6397: Constify struct regulator_ops

has been applied to the regulator tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From 7e3fc9960169ec5eab0ec6810fa24518ea3f3608 Mon Sep 17 00:00:00 2001
From: Henry Chen 
Date: Mon, 23 May 2016 15:30:16 +0800
Subject: [PATCH] regulator: mt6397: Constify struct regulator_ops

Consitify the structure of regulator operations.

Signed-off-by: Henry Chen 
Signed-off-by: Mark Brown 
---
 drivers/regulator/mt6397-regulator.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/regulator/mt6397-regulator.c 
b/drivers/regulator/mt6397-regulator.c
index 17a5b6c2d6a9..1c45abb94409 100644
--- a/drivers/regulator/mt6397-regulator.c
+++ b/drivers/regulator/mt6397-regulator.c
@@ -160,7 +160,7 @@ static int mt6397_get_status(struct regulator_dev *rdev)
return (regval & info->qi) ? REGULATOR_STATUS_ON : REGULATOR_STATUS_OFF;
 }
 
-static struct regulator_ops mt6397_volt_range_ops = {
+static const struct regulator_ops mt6397_volt_range_ops = {
.list_voltage = regulator_list_voltage_linear_range,
.map_voltage = regulator_map_voltage_linear_range,
.set_voltage_sel = regulator_set_voltage_sel_regmap,
@@ -172,7 +172,7 @@ static struct regulator_ops mt6397_volt_range_ops = {
.get_status = mt6397_get_status,
 };
 
-static struct regulator_ops mt6397_volt_table_ops = {
+static const struct regulator_ops mt6397_volt_table_ops = {
.list_voltage = regulator_list_voltage_table,
.map_voltage = regulator_map_voltage_iterate,
.set_voltage_sel = regulator_set_voltage_sel_regmap,
@@ -184,7 +184,7 @@ static struct regulator_ops mt6397_volt_table_ops = {
.get_status = mt6397_get_status,
 };
 
-static struct regulator_ops mt6397_volt_fixed_ops = {
+static const struct regulator_ops mt6397_volt_fixed_ops = {
.list_voltage = regulator_list_voltage_linear,
.enable = regulator_enable_regmap,
.disable = regulator_disable_regmap,
-- 
2.8.1



Applied "spi: Add file patterns for spi device tree bindings" to the spi tree

2016-05-23 Thread Mark Brown
The patch

   spi: Add file patterns for spi device tree bindings

has been applied to the spi tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From c876fec76e3719734f8a055b799c8c4c1930fcfc Mon Sep 17 00:00:00 2001
From: Geert Uytterhoeven 
Date: Sun, 22 May 2016 11:06:21 +0200
Subject: [PATCH] spi: Add file patterns for spi device tree bindings

Submitters of device tree binding documentation may forget to CC
the subsystem maintainer if this is missing.

Signed-off-by: Geert Uytterhoeven 
Signed-off-by: Mark Brown 
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 9c567a431d8d..3aaea9f66eae 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10511,6 +10511,7 @@ L:  linux-...@vger.kernel.org
 T: git git://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git
 Q: http://patchwork.kernel.org/project/spi-devel-general/list/
 S: Maintained
+F: Documentation/devicetree/bindings/spi/
 F: Documentation/spi/
 F: drivers/spi/
 F: include/linux/spi/
-- 
2.8.1



Re: sem_lock() vs qspinlocks

2016-05-23 Thread Linus Torvalds
On Mon, May 23, 2016 at 5:25 AM, Peter Zijlstra  wrote:
>
> Paul has smp_mb__after_unlock_lock() for the RCpc 'upgrade'. How about
> something like:
>
> smp_mb__after_lock()

I'd much rather make the naming be higher level. It's not necessarily
going to be a "mb", and while the problem is about smp, the primitives
it is synchronizing aren't actually smp-specific (ie you're
synchronizing a lock that is relevant on UP too).

So I'd just call it something like

spin_lock_sync_after_lock();

because different locks might have different levels of serialization
(ie maybe a spinlock needs one thing, and a mutex needs another - if
we start worrying about ordering between spin_lock and
mutex_is_locked(), for example, or between mutex_lock() and
spin_is_locked()).

Hmm?

 Linus


Applied "regulator: mt6397: Constify struct regulator_ops" to the regulator tree

2016-05-23 Thread Mark Brown
The patch

   regulator: mt6397: Constify struct regulator_ops

has been applied to the regulator tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From 7e3fc9960169ec5eab0ec6810fa24518ea3f3608 Mon Sep 17 00:00:00 2001
From: Henry Chen 
Date: Mon, 23 May 2016 15:30:16 +0800
Subject: [PATCH] regulator: mt6397: Constify struct regulator_ops

Consitify the structure of regulator operations.

Signed-off-by: Henry Chen 
Signed-off-by: Mark Brown 
---
 drivers/regulator/mt6397-regulator.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/regulator/mt6397-regulator.c 
b/drivers/regulator/mt6397-regulator.c
index 17a5b6c2d6a9..1c45abb94409 100644
--- a/drivers/regulator/mt6397-regulator.c
+++ b/drivers/regulator/mt6397-regulator.c
@@ -160,7 +160,7 @@ static int mt6397_get_status(struct regulator_dev *rdev)
return (regval & info->qi) ? REGULATOR_STATUS_ON : REGULATOR_STATUS_OFF;
 }
 
-static struct regulator_ops mt6397_volt_range_ops = {
+static const struct regulator_ops mt6397_volt_range_ops = {
.list_voltage = regulator_list_voltage_linear_range,
.map_voltage = regulator_map_voltage_linear_range,
.set_voltage_sel = regulator_set_voltage_sel_regmap,
@@ -172,7 +172,7 @@ static struct regulator_ops mt6397_volt_range_ops = {
.get_status = mt6397_get_status,
 };
 
-static struct regulator_ops mt6397_volt_table_ops = {
+static const struct regulator_ops mt6397_volt_table_ops = {
.list_voltage = regulator_list_voltage_table,
.map_voltage = regulator_map_voltage_iterate,
.set_voltage_sel = regulator_set_voltage_sel_regmap,
@@ -184,7 +184,7 @@ static struct regulator_ops mt6397_volt_table_ops = {
.get_status = mt6397_get_status,
 };
 
-static struct regulator_ops mt6397_volt_fixed_ops = {
+static const struct regulator_ops mt6397_volt_fixed_ops = {
.list_voltage = regulator_list_voltage_linear,
.enable = regulator_enable_regmap,
.disable = regulator_disable_regmap,
-- 
2.8.1



Applied "spi: Add file patterns for spi device tree bindings" to the spi tree

2016-05-23 Thread Mark Brown
The patch

   spi: Add file patterns for spi device tree bindings

has been applied to the spi tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From c876fec76e3719734f8a055b799c8c4c1930fcfc Mon Sep 17 00:00:00 2001
From: Geert Uytterhoeven 
Date: Sun, 22 May 2016 11:06:21 +0200
Subject: [PATCH] spi: Add file patterns for spi device tree bindings

Submitters of device tree binding documentation may forget to CC
the subsystem maintainer if this is missing.

Signed-off-by: Geert Uytterhoeven 
Signed-off-by: Mark Brown 
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 9c567a431d8d..3aaea9f66eae 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10511,6 +10511,7 @@ L:  linux-...@vger.kernel.org
 T: git git://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git
 Q: http://patchwork.kernel.org/project/spi-devel-general/list/
 S: Maintained
+F: Documentation/devicetree/bindings/spi/
 F: Documentation/spi/
 F: drivers/spi/
 F: include/linux/spi/
-- 
2.8.1



Re: sem_lock() vs qspinlocks

2016-05-23 Thread Linus Torvalds
On Mon, May 23, 2016 at 5:25 AM, Peter Zijlstra  wrote:
>
> Paul has smp_mb__after_unlock_lock() for the RCpc 'upgrade'. How about
> something like:
>
> smp_mb__after_lock()

I'd much rather make the naming be higher level. It's not necessarily
going to be a "mb", and while the problem is about smp, the primitives
it is synchronizing aren't actually smp-specific (ie you're
synchronizing a lock that is relevant on UP too).

So I'd just call it something like

spin_lock_sync_after_lock();

because different locks might have different levels of serialization
(ie maybe a spinlock needs one thing, and a mutex needs another - if
we start worrying about ordering between spin_lock and
mutex_is_locked(), for example, or between mutex_lock() and
spin_is_locked()).

Hmm?

 Linus


Applied "regulator: pv880x0: Clean up unnecessary header inclusion" to the regulator tree

2016-05-23 Thread Mark Brown
The patch

   regulator: pv880x0: Clean up unnecessary header inclusion

has been applied to the regulator tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From f88112de5a7f3da9ac436ae149c9095c2cb0f4c7 Mon Sep 17 00:00:00 2001
From: Axel Lin 
Date: Mon, 23 May 2016 22:03:23 +0800
Subject: [PATCH] regulator: pv880x0: Clean up unnecessary header inclusion

Signed-off-by: Axel Lin 
Signed-off-by: Mark Brown 
---
 drivers/regulator/pv88060-regulator.c | 3 ---
 drivers/regulator/pv88080-regulator.c | 3 ---
 drivers/regulator/pv88090-regulator.c | 3 ---
 3 files changed, 9 deletions(-)

diff --git a/drivers/regulator/pv88060-regulator.c 
b/drivers/regulator/pv88060-regulator.c
index c448b727f5f8..6c4afc73ecac 100644
--- a/drivers/regulator/pv88060-regulator.c
+++ b/drivers/regulator/pv88060-regulator.c
@@ -14,7 +14,6 @@
  */
 
 #include 
-#include 
 #include 
 #include 
 #include 
@@ -25,8 +24,6 @@
 #include 
 #include 
 #include 
-#include 
-#include 
 #include "pv88060-regulator.h"
 
 #define PV88060_MAX_REGULATORS 14
diff --git a/drivers/regulator/pv88080-regulator.c 
b/drivers/regulator/pv88080-regulator.c
index d7107566c429..81950bdb1cc4 100644
--- a/drivers/regulator/pv88080-regulator.c
+++ b/drivers/regulator/pv88080-regulator.c
@@ -14,7 +14,6 @@
  */
 
 #include 
-#include 
 #include 
 #include 
 #include 
@@ -25,8 +24,6 @@
 #include 
 #include 
 #include 
-#include 
-#include 
 #include "pv88080-regulator.h"
 
 #define PV88080_MAX_REGULATORS 3
diff --git a/drivers/regulator/pv88090-regulator.c 
b/drivers/regulator/pv88090-regulator.c
index 0057c6740d6f..421641175352 100644
--- a/drivers/regulator/pv88090-regulator.c
+++ b/drivers/regulator/pv88090-regulator.c
@@ -14,7 +14,6 @@
  */
 
 #include 
-#include 
 #include 
 #include 
 #include 
@@ -25,8 +24,6 @@
 #include 
 #include 
 #include 
-#include 
-#include 
 #include "pv88090-regulator.h"
 
 #define PV88090_MAX_REGULATORS 5
-- 
2.8.1



Applied "regulator: pv880x0: Clean up unnecessary header inclusion" to the regulator tree

2016-05-23 Thread Mark Brown
The patch

   regulator: pv880x0: Clean up unnecessary header inclusion

has been applied to the regulator tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From f88112de5a7f3da9ac436ae149c9095c2cb0f4c7 Mon Sep 17 00:00:00 2001
From: Axel Lin 
Date: Mon, 23 May 2016 22:03:23 +0800
Subject: [PATCH] regulator: pv880x0: Clean up unnecessary header inclusion

Signed-off-by: Axel Lin 
Signed-off-by: Mark Brown 
---
 drivers/regulator/pv88060-regulator.c | 3 ---
 drivers/regulator/pv88080-regulator.c | 3 ---
 drivers/regulator/pv88090-regulator.c | 3 ---
 3 files changed, 9 deletions(-)

diff --git a/drivers/regulator/pv88060-regulator.c 
b/drivers/regulator/pv88060-regulator.c
index c448b727f5f8..6c4afc73ecac 100644
--- a/drivers/regulator/pv88060-regulator.c
+++ b/drivers/regulator/pv88060-regulator.c
@@ -14,7 +14,6 @@
  */
 
 #include 
-#include 
 #include 
 #include 
 #include 
@@ -25,8 +24,6 @@
 #include 
 #include 
 #include 
-#include 
-#include 
 #include "pv88060-regulator.h"
 
 #define PV88060_MAX_REGULATORS 14
diff --git a/drivers/regulator/pv88080-regulator.c 
b/drivers/regulator/pv88080-regulator.c
index d7107566c429..81950bdb1cc4 100644
--- a/drivers/regulator/pv88080-regulator.c
+++ b/drivers/regulator/pv88080-regulator.c
@@ -14,7 +14,6 @@
  */
 
 #include 
-#include 
 #include 
 #include 
 #include 
@@ -25,8 +24,6 @@
 #include 
 #include 
 #include 
-#include 
-#include 
 #include "pv88080-regulator.h"
 
 #define PV88080_MAX_REGULATORS 3
diff --git a/drivers/regulator/pv88090-regulator.c 
b/drivers/regulator/pv88090-regulator.c
index 0057c6740d6f..421641175352 100644
--- a/drivers/regulator/pv88090-regulator.c
+++ b/drivers/regulator/pv88090-regulator.c
@@ -14,7 +14,6 @@
  */
 
 #include 
-#include 
 #include 
 #include 
 #include 
@@ -25,8 +24,6 @@
 #include 
 #include 
 #include 
-#include 
-#include 
 #include "pv88090-regulator.h"
 
 #define PV88090_MAX_REGULATORS 5
-- 
2.8.1



Applied "regulator: mt6397: Add buck change mode regulator interface for mt6397" to the regulator tree

2016-05-23 Thread Mark Brown
The patch

   regulator: mt6397: Add buck change mode regulator interface for mt6397

has been applied to the regulator tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From 6d26507bd45bf09374d3744ee07fabebc87266f5 Mon Sep 17 00:00:00 2001
From: Henry Chen 
Date: Mon, 23 May 2016 15:13:31 +0800
Subject: [PATCH] regulator: mt6397: Add buck change mode regulator interface
 for mt6397

BUCKs of mt6397 have auto mode and pwm mode.
User can use regulator interfaces to control modes

Signed-off-by: Henry Chen 
Signed-off-by: Mark Brown 
---
 drivers/regulator/mt6397-regulator.c | 89 
 1 file changed, 80 insertions(+), 9 deletions(-)

diff --git a/drivers/regulator/mt6397-regulator.c 
b/drivers/regulator/mt6397-regulator.c
index 1c45abb94409..c6c6aa85e4e8 100644
--- a/drivers/regulator/mt6397-regulator.c
+++ b/drivers/regulator/mt6397-regulator.c
@@ -23,6 +23,9 @@
 #include 
 #include 
 
+#define MT6397_BUCK_MODE_AUTO  0
+#define MT6397_BUCK_MODE_FORCE_PWM 1
+
 /*
  * MT6397 regulators' information
  *
@@ -38,10 +41,14 @@ struct mt6397_regulator_info {
u32 vselon_reg;
u32 vselctrl_reg;
u32 vselctrl_mask;
+   u32 modeset_reg;
+   u32 modeset_mask;
+   u32 modeset_shift;
 };
 
 #define MT6397_BUCK(match, vreg, min, max, step, volt_ranges, enreg,   \
-   vosel, vosel_mask, voselon, vosel_ctrl) \
+   vosel, vosel_mask, voselon, vosel_ctrl, _modeset_reg,   \
+   _modeset_shift) \
 [MT6397_ID_##vreg] = { \
.desc = {   \
.name = #vreg,  \
@@ -62,6 +69,9 @@ struct mt6397_regulator_info {
.vselon_reg = voselon,  \
.vselctrl_reg = vosel_ctrl, \
.vselctrl_mask = BIT(1),\
+   .modeset_reg = _modeset_reg,\
+   .modeset_mask = BIT(_modeset_shift),\
+   .modeset_shift = _modeset_shift \
 }
 
 #define MT6397_LDO(match, vreg, ldo_volt_table, enreg, enbit, vosel,   \
@@ -145,6 +155,63 @@ static const u32 ldo_volt_table7[] = {
130, 150, 180, 200, 250, 280, 300, 330,
 };
 
+static int mt6397_regulator_set_mode(struct regulator_dev *rdev,
+unsigned int mode)
+{
+   struct mt6397_regulator_info *info = rdev_get_drvdata(rdev);
+   int ret, val;
+
+   switch (mode) {
+   case REGULATOR_MODE_FAST:
+   val = MT6397_BUCK_MODE_FORCE_PWM;
+   break;
+   case REGULATOR_MODE_NORMAL:
+   val = MT6397_BUCK_MODE_AUTO;
+   break;
+   default:
+   ret = -EINVAL;
+   goto err_mode;
+   }
+
+   dev_dbg(>dev, "mt6397 buck set_mode %#x, %#x, %#x, %#x\n",
+   info->modeset_reg, info->modeset_mask,
+   info->modeset_shift, val);
+
+   val <<= info->modeset_shift;
+   ret = regmap_update_bits(rdev->regmap, info->modeset_reg,
+info->modeset_mask, val);
+err_mode:
+   if (ret != 0) {
+   dev_err(>dev,
+   "Failed to set mt6397 buck mode: %d\n", ret);
+   return ret;
+   }
+
+   return 0;
+}
+
+static unsigned int mt6397_regulator_get_mode(struct regulator_dev *rdev)
+{
+   struct mt6397_regulator_info *info = rdev_get_drvdata(rdev);
+   int ret, regval;
+
+   ret = regmap_read(rdev->regmap, info->modeset_reg, );
+   if (ret != 0) {
+   dev_err(>dev,
+   "Failed to get mt6397 buck mode: %d\n", ret);
+   return ret;
+   }
+
+   switch ((regval & info->modeset_mask) >> info->modeset_shift) {
+   case MT6397_BUCK_MODE_AUTO:
+   return REGULATOR_MODE_NORMAL;
+   

Applied "regulator: mt6397: Add buck change mode regulator interface for mt6397" to the regulator tree

2016-05-23 Thread Mark Brown
The patch

   regulator: mt6397: Add buck change mode regulator interface for mt6397

has been applied to the regulator tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From 6d26507bd45bf09374d3744ee07fabebc87266f5 Mon Sep 17 00:00:00 2001
From: Henry Chen 
Date: Mon, 23 May 2016 15:13:31 +0800
Subject: [PATCH] regulator: mt6397: Add buck change mode regulator interface
 for mt6397

BUCKs of mt6397 have auto mode and pwm mode.
User can use regulator interfaces to control modes

Signed-off-by: Henry Chen 
Signed-off-by: Mark Brown 
---
 drivers/regulator/mt6397-regulator.c | 89 
 1 file changed, 80 insertions(+), 9 deletions(-)

diff --git a/drivers/regulator/mt6397-regulator.c 
b/drivers/regulator/mt6397-regulator.c
index 1c45abb94409..c6c6aa85e4e8 100644
--- a/drivers/regulator/mt6397-regulator.c
+++ b/drivers/regulator/mt6397-regulator.c
@@ -23,6 +23,9 @@
 #include 
 #include 
 
+#define MT6397_BUCK_MODE_AUTO  0
+#define MT6397_BUCK_MODE_FORCE_PWM 1
+
 /*
  * MT6397 regulators' information
  *
@@ -38,10 +41,14 @@ struct mt6397_regulator_info {
u32 vselon_reg;
u32 vselctrl_reg;
u32 vselctrl_mask;
+   u32 modeset_reg;
+   u32 modeset_mask;
+   u32 modeset_shift;
 };
 
 #define MT6397_BUCK(match, vreg, min, max, step, volt_ranges, enreg,   \
-   vosel, vosel_mask, voselon, vosel_ctrl) \
+   vosel, vosel_mask, voselon, vosel_ctrl, _modeset_reg,   \
+   _modeset_shift) \
 [MT6397_ID_##vreg] = { \
.desc = {   \
.name = #vreg,  \
@@ -62,6 +69,9 @@ struct mt6397_regulator_info {
.vselon_reg = voselon,  \
.vselctrl_reg = vosel_ctrl, \
.vselctrl_mask = BIT(1),\
+   .modeset_reg = _modeset_reg,\
+   .modeset_mask = BIT(_modeset_shift),\
+   .modeset_shift = _modeset_shift \
 }
 
 #define MT6397_LDO(match, vreg, ldo_volt_table, enreg, enbit, vosel,   \
@@ -145,6 +155,63 @@ static const u32 ldo_volt_table7[] = {
130, 150, 180, 200, 250, 280, 300, 330,
 };
 
+static int mt6397_regulator_set_mode(struct regulator_dev *rdev,
+unsigned int mode)
+{
+   struct mt6397_regulator_info *info = rdev_get_drvdata(rdev);
+   int ret, val;
+
+   switch (mode) {
+   case REGULATOR_MODE_FAST:
+   val = MT6397_BUCK_MODE_FORCE_PWM;
+   break;
+   case REGULATOR_MODE_NORMAL:
+   val = MT6397_BUCK_MODE_AUTO;
+   break;
+   default:
+   ret = -EINVAL;
+   goto err_mode;
+   }
+
+   dev_dbg(>dev, "mt6397 buck set_mode %#x, %#x, %#x, %#x\n",
+   info->modeset_reg, info->modeset_mask,
+   info->modeset_shift, val);
+
+   val <<= info->modeset_shift;
+   ret = regmap_update_bits(rdev->regmap, info->modeset_reg,
+info->modeset_mask, val);
+err_mode:
+   if (ret != 0) {
+   dev_err(>dev,
+   "Failed to set mt6397 buck mode: %d\n", ret);
+   return ret;
+   }
+
+   return 0;
+}
+
+static unsigned int mt6397_regulator_get_mode(struct regulator_dev *rdev)
+{
+   struct mt6397_regulator_info *info = rdev_get_drvdata(rdev);
+   int ret, regval;
+
+   ret = regmap_read(rdev->regmap, info->modeset_reg, );
+   if (ret != 0) {
+   dev_err(>dev,
+   "Failed to get mt6397 buck mode: %d\n", ret);
+   return ret;
+   }
+
+   switch ((regval & info->modeset_mask) >> info->modeset_shift) {
+   case MT6397_BUCK_MODE_AUTO:
+   return REGULATOR_MODE_NORMAL;
+   case MT6397_BUCK_MODE_FORCE_PWM:
+   return 

Applied "ASoC: Add file patterns for sound device tree bindings" to the asoc tree

2016-05-23 Thread Mark Brown
The patch

   ASoC: Add file patterns for sound device tree bindings

has been applied to the asoc tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From afc9fb456dd0bc16633757c62e9857de588fdf57 Mon Sep 17 00:00:00 2001
From: Geert Uytterhoeven 
Date: Sun, 22 May 2016 11:06:20 +0200
Subject: [PATCH] ASoC: Add file patterns for sound device tree bindings

Submitters of device tree binding documentation may forget to CC
the subsystem maintainer if this is missing.

Signed-off-by: Geert Uytterhoeven 
Signed-off-by: Mark Brown 
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 09a9cf1e0a8a..9d8795ed25c2 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10435,6 +10435,7 @@ T:  git 
git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
 L: alsa-de...@alsa-project.org (moderated for non-subscribers)
 W: http://alsa-project.org/main/index.php/ASoC
 S: Supported
+F: Documentation/devicetree/bindings/sound/
 F: Documentation/sound/alsa/soc/
 F: sound/soc/
 F: include/sound/soc*
-- 
2.8.1



Applied "ASoC: kirkwood: fix build failure" to the asoc tree

2016-05-23 Thread Mark Brown
The patch

   ASoC: kirkwood: fix build failure

has been applied to the asoc tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From b01518ca88410195ace38ce755c1206588e5c167 Mon Sep 17 00:00:00 2001
From: Sudip Mukherjee 
Date: Mon, 23 May 2016 16:00:54 +0530
Subject: [PATCH] ASoC: kirkwood: fix build failure

While building m32r allmodconfig the build failed with:
ERROR: "bad_dma_ops" [sound/soc/kirkwood/snd-soc-kirkwood.ko] undefined!

To satisfy the dependency CONFIG_SND_KIRKWOOD_SOC should depend on
HAS_DMA.

Signed-off-by: Sudip Mukherjee 
Signed-off-by: Mark Brown 
---
 sound/soc/kirkwood/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/sound/soc/kirkwood/Kconfig b/sound/soc/kirkwood/Kconfig
index 132bb83f8e99..bc3c7b5ac752 100644
--- a/sound/soc/kirkwood/Kconfig
+++ b/sound/soc/kirkwood/Kconfig
@@ -1,6 +1,7 @@
 config SND_KIRKWOOD_SOC
tristate "SoC Audio for the Marvell Kirkwood and Dove chips"
depends on ARCH_DOVE || ARCH_MVEBU || COMPILE_TEST
+   depends on HAS_DMA
help
  Say Y or M if you want to add support for codecs attached to
  the Kirkwood I2S interface. You will also need to select the
-- 
2.8.1



Applied "ASoC: Add file patterns for sound device tree bindings" to the asoc tree

2016-05-23 Thread Mark Brown
The patch

   ASoC: Add file patterns for sound device tree bindings

has been applied to the asoc tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From afc9fb456dd0bc16633757c62e9857de588fdf57 Mon Sep 17 00:00:00 2001
From: Geert Uytterhoeven 
Date: Sun, 22 May 2016 11:06:20 +0200
Subject: [PATCH] ASoC: Add file patterns for sound device tree bindings

Submitters of device tree binding documentation may forget to CC
the subsystem maintainer if this is missing.

Signed-off-by: Geert Uytterhoeven 
Signed-off-by: Mark Brown 
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 09a9cf1e0a8a..9d8795ed25c2 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10435,6 +10435,7 @@ T:  git 
git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
 L: alsa-de...@alsa-project.org (moderated for non-subscribers)
 W: http://alsa-project.org/main/index.php/ASoC
 S: Supported
+F: Documentation/devicetree/bindings/sound/
 F: Documentation/sound/alsa/soc/
 F: sound/soc/
 F: include/sound/soc*
-- 
2.8.1



Applied "ASoC: kirkwood: fix build failure" to the asoc tree

2016-05-23 Thread Mark Brown
The patch

   ASoC: kirkwood: fix build failure

has been applied to the asoc tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From b01518ca88410195ace38ce755c1206588e5c167 Mon Sep 17 00:00:00 2001
From: Sudip Mukherjee 
Date: Mon, 23 May 2016 16:00:54 +0530
Subject: [PATCH] ASoC: kirkwood: fix build failure

While building m32r allmodconfig the build failed with:
ERROR: "bad_dma_ops" [sound/soc/kirkwood/snd-soc-kirkwood.ko] undefined!

To satisfy the dependency CONFIG_SND_KIRKWOOD_SOC should depend on
HAS_DMA.

Signed-off-by: Sudip Mukherjee 
Signed-off-by: Mark Brown 
---
 sound/soc/kirkwood/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/sound/soc/kirkwood/Kconfig b/sound/soc/kirkwood/Kconfig
index 132bb83f8e99..bc3c7b5ac752 100644
--- a/sound/soc/kirkwood/Kconfig
+++ b/sound/soc/kirkwood/Kconfig
@@ -1,6 +1,7 @@
 config SND_KIRKWOOD_SOC
tristate "SoC Audio for the Marvell Kirkwood and Dove chips"
depends on ARCH_DOVE || ARCH_MVEBU || COMPILE_TEST
+   depends on HAS_DMA
help
  Say Y or M if you want to add support for codecs attached to
  the Kirkwood I2S interface. You will also need to select the
-- 
2.8.1



Applied "regmap: Add file patterns for regmap device tree bindings" to the regmap tree

2016-05-23 Thread Mark Brown
The patch

   regmap: Add file patterns for regmap device tree bindings

has been applied to the regmap tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regmap.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From 9398a639c8b175460828ee7c45e6b7a4c3f586b9 Mon Sep 17 00:00:00 2001
From: Geert Uytterhoeven 
Date: Sun, 22 May 2016 11:06:17 +0200
Subject: [PATCH] regmap: Add file patterns for regmap device tree bindings

Submitters of device tree binding documentation may forget to CC
the subsystem maintainer if this is missing.

Signed-off-by: Geert Uytterhoeven 
Signed-off-by: Mark Brown 
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index a727d9959ecd..94a9af5d7f7d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9318,6 +9318,7 @@ M:Mark Brown 
 L: linux-kernel@vger.kernel.org
 T: git git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regmap.git
 S: Supported
+F: Documentation/devicetree/bindings/regmap/
 F: drivers/base/regmap/
 F: include/linux/regmap.h
 
-- 
2.8.1



Applied "regmap: Add file patterns for regmap device tree bindings" to the regmap tree

2016-05-23 Thread Mark Brown
The patch

   regmap: Add file patterns for regmap device tree bindings

has been applied to the regmap tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regmap.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From 9398a639c8b175460828ee7c45e6b7a4c3f586b9 Mon Sep 17 00:00:00 2001
From: Geert Uytterhoeven 
Date: Sun, 22 May 2016 11:06:17 +0200
Subject: [PATCH] regmap: Add file patterns for regmap device tree bindings

Submitters of device tree binding documentation may forget to CC
the subsystem maintainer if this is missing.

Signed-off-by: Geert Uytterhoeven 
Signed-off-by: Mark Brown 
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index a727d9959ecd..94a9af5d7f7d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9318,6 +9318,7 @@ M:Mark Brown 
 L: linux-kernel@vger.kernel.org
 T: git git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regmap.git
 S: Supported
+F: Documentation/devicetree/bindings/regmap/
 F: drivers/base/regmap/
 F: include/linux/regmap.h
 
-- 
2.8.1



Re: [PATCH 1/2] mtd: spi-nor: disable software protection for Macronix flash at startup

2016-05-23 Thread Cyrille Pitchen
Hi Matthias,

Le 23/05/2016 18:32, Matthias Schiffer a écrit :
> On 05/23/2016 04:01 PM, Cyrille Pitchen wrote:
>> Hi Matthias,
>>
>> Le 18/05/2016 15:32, Matthias Schiffer a écrit :
>>> This patch has been tested in OpenWrt for a few months and seems to work
>>> correctly.
>>>
>>> Signed-off-by: Felix Fietkau 
>>> Signed-off-by: Matthias Schiffer 
>>> ---
>>>  drivers/mtd/spi-nor/spi-nor.c | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
>>> index 157841d..d681003 100644
>>> --- a/drivers/mtd/spi-nor/spi-nor.c
>>> +++ b/drivers/mtd/spi-nor/spi-nor.c
>>> @@ -1304,6 +1304,7 @@ int spi_nor_scan(struct spi_nor *nor, const char 
>>> *name, enum read_mode mode)
>>>  
>>> if (JEDEC_MFR(info) == SNOR_MFR_ATMEL ||
>>> JEDEC_MFR(info) == SNOR_MFR_INTEL ||
>>> +   JEDEC_MFR(info) == SNOR_MFR_MACRONIX ||
>>> JEDEC_MFR(info) == SNOR_MFR_SST ||
>>> info->flags & SPI_NOR_HAS_LOCK) {
>>> write_enable(nor);
>>>
>> The line following this patch chunk is "write_sr(nor, 0);" however, if I 
>> refer
>> to the Macronix mx25l25673g datasheet about the Status Register, bits[5:2]
>> (BP0, BP1, BP2 and BP3) are non-volatile and define the protected area.
>> Also bit6 (Quad Enable) is also non-volatile and is used to reassign #WP and
>> #Hold pins to IO2 and IO3 functions needed by Quad SPI protocols on many 
>> other
>> Macronix memories (indeed on the 73g part, the Quad Enable bit is always 1).
>>
>> So you should not write 0 directly into the Status Register if you only want 
>> to
>> clear bit7 (Status Register Write Disable): use a read, modify, write 
>> sequence
>> instead.
>>
>>
>> Best regards,
>>
>> Cyrille
>>
> 
> Hi,
> clearing the protected area (BP0..BP3) is exactly what this code is
> supposed to do; it was already doing this for Atmel, Intel and SST flash,
> and this patch series makes it behave the same for Macronix and Winbond.
> 
> I see that the mx25l25673g defaults to unprotected according to the
> datasheet, but removing the protection again should not hurt. If there is
> actually a problem with writing 0 to the Quad Enable bit, we can of course
> make this conditional and/or just clear the relevant bits.
> 
> I've observed the mx25l6405d coming up with protection bits set (the
> datasheet doesn't seem to specify the initial state of the bits). It's also
> possible that the software protection is enabled by the bootloader, which I
> can't replace; but I'd argue that the kernel should try to get the flash
> chip into a known state on boot, thus always removing the software protection.
> 
> Most of the setup of the flash chip is done after the "write_sr(nor, 0);"
> in spi_nor_scan(). Do you suspect that "write_sr(nor, 0);" might cause
> problems anyways?

Yes I do: it's likely to prevent us from adding support to the Macronix QPI mode
which use the SPI 4-4-4 protocol. As all other Quad SPI protocols on Macronix
memories, it requires the Quad Enable bit to be set so the #Write Protect, 
#Hold,
and #Reset functions are disabled and the associated pins are reassigned to
IO2 and IO3 data lines.

Adding proper support of the QPI mode would allow us to handle the case where
the Macronix memory has already entered its QPI mode before spi_nor_scan() is
called. So previous commands (Read JEDEC ID 0xAF, ...) already use the SPI 4-4-4
protocol. Then if you clear the Quad Enable bit in the Status Register:
1 - I don't whether it safely makes the Macronix exit its QPI mode
2 - the spi-nor framework is already configured to use the SPI 4-4-4 but once
the QE bit is cleared you could no longer use any Quad SPI protocol until
you set this bit again.

Also let forget the QPI mode for now, the Quad Enable bit is non-volatile:
if you clear it then a spurious reset occurs before the spi-nor framework sets 
the bit
back, some bootloaders might find the Macronix memory in a unexpected state 
where
Quad SPI protocols don't work.

Besides, if you just want to clear the BPx bits, there is no need to also clear
the SRWD non-volatile bit at the same time.

I don't think it's good practice to modify non-volatile bits when not needed.

> 
> Regards,
> Matthias
> 

Regards,

Cyrille


Re: [PATCH 1/2] mtd: spi-nor: disable software protection for Macronix flash at startup

2016-05-23 Thread Cyrille Pitchen
Hi Matthias,

Le 23/05/2016 18:32, Matthias Schiffer a écrit :
> On 05/23/2016 04:01 PM, Cyrille Pitchen wrote:
>> Hi Matthias,
>>
>> Le 18/05/2016 15:32, Matthias Schiffer a écrit :
>>> This patch has been tested in OpenWrt for a few months and seems to work
>>> correctly.
>>>
>>> Signed-off-by: Felix Fietkau 
>>> Signed-off-by: Matthias Schiffer 
>>> ---
>>>  drivers/mtd/spi-nor/spi-nor.c | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
>>> index 157841d..d681003 100644
>>> --- a/drivers/mtd/spi-nor/spi-nor.c
>>> +++ b/drivers/mtd/spi-nor/spi-nor.c
>>> @@ -1304,6 +1304,7 @@ int spi_nor_scan(struct spi_nor *nor, const char 
>>> *name, enum read_mode mode)
>>>  
>>> if (JEDEC_MFR(info) == SNOR_MFR_ATMEL ||
>>> JEDEC_MFR(info) == SNOR_MFR_INTEL ||
>>> +   JEDEC_MFR(info) == SNOR_MFR_MACRONIX ||
>>> JEDEC_MFR(info) == SNOR_MFR_SST ||
>>> info->flags & SPI_NOR_HAS_LOCK) {
>>> write_enable(nor);
>>>
>> The line following this patch chunk is "write_sr(nor, 0);" however, if I 
>> refer
>> to the Macronix mx25l25673g datasheet about the Status Register, bits[5:2]
>> (BP0, BP1, BP2 and BP3) are non-volatile and define the protected area.
>> Also bit6 (Quad Enable) is also non-volatile and is used to reassign #WP and
>> #Hold pins to IO2 and IO3 functions needed by Quad SPI protocols on many 
>> other
>> Macronix memories (indeed on the 73g part, the Quad Enable bit is always 1).
>>
>> So you should not write 0 directly into the Status Register if you only want 
>> to
>> clear bit7 (Status Register Write Disable): use a read, modify, write 
>> sequence
>> instead.
>>
>>
>> Best regards,
>>
>> Cyrille
>>
> 
> Hi,
> clearing the protected area (BP0..BP3) is exactly what this code is
> supposed to do; it was already doing this for Atmel, Intel and SST flash,
> and this patch series makes it behave the same for Macronix and Winbond.
> 
> I see that the mx25l25673g defaults to unprotected according to the
> datasheet, but removing the protection again should not hurt. If there is
> actually a problem with writing 0 to the Quad Enable bit, we can of course
> make this conditional and/or just clear the relevant bits.
> 
> I've observed the mx25l6405d coming up with protection bits set (the
> datasheet doesn't seem to specify the initial state of the bits). It's also
> possible that the software protection is enabled by the bootloader, which I
> can't replace; but I'd argue that the kernel should try to get the flash
> chip into a known state on boot, thus always removing the software protection.
> 
> Most of the setup of the flash chip is done after the "write_sr(nor, 0);"
> in spi_nor_scan(). Do you suspect that "write_sr(nor, 0);" might cause
> problems anyways?

Yes I do: it's likely to prevent us from adding support to the Macronix QPI mode
which use the SPI 4-4-4 protocol. As all other Quad SPI protocols on Macronix
memories, it requires the Quad Enable bit to be set so the #Write Protect, 
#Hold,
and #Reset functions are disabled and the associated pins are reassigned to
IO2 and IO3 data lines.

Adding proper support of the QPI mode would allow us to handle the case where
the Macronix memory has already entered its QPI mode before spi_nor_scan() is
called. So previous commands (Read JEDEC ID 0xAF, ...) already use the SPI 4-4-4
protocol. Then if you clear the Quad Enable bit in the Status Register:
1 - I don't whether it safely makes the Macronix exit its QPI mode
2 - the spi-nor framework is already configured to use the SPI 4-4-4 but once
the QE bit is cleared you could no longer use any Quad SPI protocol until
you set this bit again.

Also let forget the QPI mode for now, the Quad Enable bit is non-volatile:
if you clear it then a spurious reset occurs before the spi-nor framework sets 
the bit
back, some bootloaders might find the Macronix memory in a unexpected state 
where
Quad SPI protocols don't work.

Besides, if you just want to clear the BPx bits, there is no need to also clear
the SRWD non-volatile bit at the same time.

I don't think it's good practice to modify non-volatile bits when not needed.

> 
> Regards,
> Matthias
> 

Regards,

Cyrille


Applied "ASoC: dwc: Add helper functions to disable/enable irqs" to the asoc tree

2016-05-23 Thread Mark Brown
The patch

   ASoC: dwc: Add helper functions to disable/enable irqs

has been applied to the asoc tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From df4e7be1832a1ca9fccfb2ed8771a17da6f7fa57 Mon Sep 17 00:00:00 2001
From: Jose Abreu 
Date: Mon, 23 May 2016 11:02:22 +0100
Subject: [PATCH] ASoC: dwc: Add helper functions to disable/enable irqs

Helper functions to disable and enable the I2S interrupts were
added. Only the interrupts of the used channels are enabled.

Also, there is no need to enable irqs at dw_i2s_config(), they
are already enabled at startup.

Signed-off-by: Jose Abreu 
Cc: Carlos Palminha 
Cc: Mark Brown 
Cc: Liam Girdwood 
Cc: Jaroslav Kysela 
Cc: Takashi Iwai 
Cc: Rob Herring 
Cc: Alexey Brodkin 
Cc: linux-snps-...@lists.infradead.org
Cc: alsa-de...@alsa-project.org
Cc: devicet...@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Mark Brown 
---
 sound/soc/dwc/designware_i2s.c | 68 +-
 1 file changed, 41 insertions(+), 27 deletions(-)

diff --git a/sound/soc/dwc/designware_i2s.c b/sound/soc/dwc/designware_i2s.c
index 0db69b7e9617..4c4f0dc24f10 100644
--- a/sound/soc/dwc/designware_i2s.c
+++ b/sound/soc/dwc/designware_i2s.c
@@ -145,26 +145,54 @@ static inline void i2s_clear_irqs(struct dw_i2s_dev *dev, 
u32 stream)
}
 }
 
-static void i2s_start(struct dw_i2s_dev *dev,
- struct snd_pcm_substream *substream)
+static inline void i2s_disable_irqs(struct dw_i2s_dev *dev, u32 stream,
+   int chan_nr)
 {
-   struct i2s_clk_config_data *config = >config;
u32 i, irq;
-   i2s_write_reg(dev->i2s_base, IER, 1);
 
-   if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
-   for (i = 0; i < (config->chan_nr / 2); i++) {
+   if (stream == SNDRV_PCM_STREAM_PLAYBACK) {
+   for (i = 0; i < (chan_nr / 2); i++) {
+   irq = i2s_read_reg(dev->i2s_base, IMR(i));
+   i2s_write_reg(dev->i2s_base, IMR(i), irq | 0x30);
+   }
+   } else {
+   for (i = 0; i < (chan_nr / 2); i++) {
+   irq = i2s_read_reg(dev->i2s_base, IMR(i));
+   i2s_write_reg(dev->i2s_base, IMR(i), irq | 0x03);
+   }
+   }
+}
+
+static inline void i2s_enable_irqs(struct dw_i2s_dev *dev, u32 stream,
+  int chan_nr)
+{
+   u32 i, irq;
+
+   if (stream == SNDRV_PCM_STREAM_PLAYBACK) {
+   for (i = 0; i < (chan_nr / 2); i++) {
irq = i2s_read_reg(dev->i2s_base, IMR(i));
i2s_write_reg(dev->i2s_base, IMR(i), irq & ~0x30);
}
-   i2s_write_reg(dev->i2s_base, ITER, 1);
} else {
-   for (i = 0; i < (config->chan_nr / 2); i++) {
+   for (i = 0; i < (chan_nr / 2); i++) {
irq = i2s_read_reg(dev->i2s_base, IMR(i));
i2s_write_reg(dev->i2s_base, IMR(i), irq & ~0x03);
}
-   i2s_write_reg(dev->i2s_base, IRER, 1);
}
+}
+
+static void i2s_start(struct dw_i2s_dev *dev,
+ struct snd_pcm_substream *substream)
+{
+   struct i2s_clk_config_data *config = >config;
+
+   i2s_write_reg(dev->i2s_base, IER, 1);
+   i2s_enable_irqs(dev, substream->stream, config->chan_nr);
+
+   if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
+   i2s_write_reg(dev->i2s_base, ITER, 1);
+   else
+   i2s_write_reg(dev->i2s_base, IRER, 1);
 
i2s_write_reg(dev->i2s_base, CER, 1);
 }
@@ -172,24 +200,14 @@ static void i2s_start(struct dw_i2s_dev *dev,
 static void i2s_stop(struct dw_i2s_dev *dev,
struct snd_pcm_substream *substream)
 {
-   u32 i = 0, irq;
 
i2s_clear_irqs(dev, substream->stream);
-   if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
+   if (substream->stream == 

Applied "ASoC: dwc: Add helper functions to disable/enable irqs" to the asoc tree

2016-05-23 Thread Mark Brown
The patch

   ASoC: dwc: Add helper functions to disable/enable irqs

has been applied to the asoc tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From df4e7be1832a1ca9fccfb2ed8771a17da6f7fa57 Mon Sep 17 00:00:00 2001
From: Jose Abreu 
Date: Mon, 23 May 2016 11:02:22 +0100
Subject: [PATCH] ASoC: dwc: Add helper functions to disable/enable irqs

Helper functions to disable and enable the I2S interrupts were
added. Only the interrupts of the used channels are enabled.

Also, there is no need to enable irqs at dw_i2s_config(), they
are already enabled at startup.

Signed-off-by: Jose Abreu 
Cc: Carlos Palminha 
Cc: Mark Brown 
Cc: Liam Girdwood 
Cc: Jaroslav Kysela 
Cc: Takashi Iwai 
Cc: Rob Herring 
Cc: Alexey Brodkin 
Cc: linux-snps-...@lists.infradead.org
Cc: alsa-de...@alsa-project.org
Cc: devicet...@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Mark Brown 
---
 sound/soc/dwc/designware_i2s.c | 68 +-
 1 file changed, 41 insertions(+), 27 deletions(-)

diff --git a/sound/soc/dwc/designware_i2s.c b/sound/soc/dwc/designware_i2s.c
index 0db69b7e9617..4c4f0dc24f10 100644
--- a/sound/soc/dwc/designware_i2s.c
+++ b/sound/soc/dwc/designware_i2s.c
@@ -145,26 +145,54 @@ static inline void i2s_clear_irqs(struct dw_i2s_dev *dev, 
u32 stream)
}
 }
 
-static void i2s_start(struct dw_i2s_dev *dev,
- struct snd_pcm_substream *substream)
+static inline void i2s_disable_irqs(struct dw_i2s_dev *dev, u32 stream,
+   int chan_nr)
 {
-   struct i2s_clk_config_data *config = >config;
u32 i, irq;
-   i2s_write_reg(dev->i2s_base, IER, 1);
 
-   if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
-   for (i = 0; i < (config->chan_nr / 2); i++) {
+   if (stream == SNDRV_PCM_STREAM_PLAYBACK) {
+   for (i = 0; i < (chan_nr / 2); i++) {
+   irq = i2s_read_reg(dev->i2s_base, IMR(i));
+   i2s_write_reg(dev->i2s_base, IMR(i), irq | 0x30);
+   }
+   } else {
+   for (i = 0; i < (chan_nr / 2); i++) {
+   irq = i2s_read_reg(dev->i2s_base, IMR(i));
+   i2s_write_reg(dev->i2s_base, IMR(i), irq | 0x03);
+   }
+   }
+}
+
+static inline void i2s_enable_irqs(struct dw_i2s_dev *dev, u32 stream,
+  int chan_nr)
+{
+   u32 i, irq;
+
+   if (stream == SNDRV_PCM_STREAM_PLAYBACK) {
+   for (i = 0; i < (chan_nr / 2); i++) {
irq = i2s_read_reg(dev->i2s_base, IMR(i));
i2s_write_reg(dev->i2s_base, IMR(i), irq & ~0x30);
}
-   i2s_write_reg(dev->i2s_base, ITER, 1);
} else {
-   for (i = 0; i < (config->chan_nr / 2); i++) {
+   for (i = 0; i < (chan_nr / 2); i++) {
irq = i2s_read_reg(dev->i2s_base, IMR(i));
i2s_write_reg(dev->i2s_base, IMR(i), irq & ~0x03);
}
-   i2s_write_reg(dev->i2s_base, IRER, 1);
}
+}
+
+static void i2s_start(struct dw_i2s_dev *dev,
+ struct snd_pcm_substream *substream)
+{
+   struct i2s_clk_config_data *config = >config;
+
+   i2s_write_reg(dev->i2s_base, IER, 1);
+   i2s_enable_irqs(dev, substream->stream, config->chan_nr);
+
+   if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
+   i2s_write_reg(dev->i2s_base, ITER, 1);
+   else
+   i2s_write_reg(dev->i2s_base, IRER, 1);
 
i2s_write_reg(dev->i2s_base, CER, 1);
 }
@@ -172,24 +200,14 @@ static void i2s_start(struct dw_i2s_dev *dev,
 static void i2s_stop(struct dw_i2s_dev *dev,
struct snd_pcm_substream *substream)
 {
-   u32 i = 0, irq;
 
i2s_clear_irqs(dev, substream->stream);
-   if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
+   if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
i2s_write_reg(dev->i2s_base, ITER, 0);
-
-   for (i = 0; i < 4; i++) {
-   irq = i2s_read_reg(dev->i2s_base, IMR(i));
-  

Re: [PATCH 3/3] ACPI: ARM64: support for ACPI_TABLE_UPGRADE

2016-05-23 Thread Lorenzo Pieralisi
On Tue, May 17, 2016 at 12:48:53PM -0400, Jon Masters wrote:
> On 05/17/2016 12:44 PM, Jon Masters wrote:
> 
> > 1). During development of a platform, it is much easier to debug
> > problems with tables if you can test replacement ones without having to
> > respin the firmware. In the server world, you usually don't have the
> > firmware source code, so to get it respun could be days-weeks even if
> > you are working with the authors closely. We have practically used this
> > feature on a number of platforms already and it will continue.
> 
> For example, on one platform we were unable to fully boot RHEL(SA) due
> to a bug in one of the ACPI tables. But I was able to boot the system to
> a ramdisk containing a uuencode library and then write out the content
> of the tables over the serial port, then decompile/patch/recompile, and
> override replacement tables on the system. Then we beat the vendor up
> with the fixes and the official firmware was corrected.

Can you explain to me please why you can't do it with GRUB ?

I am using mainline GRUB and its acpi command all the time to update
static ACPI tables for testing new features (ie IORT) and it works
just fine for me (and you can still override the DSDT, which is
likely to be the main source of bugs, through the CONFIG_ACPI_CUSTOM_DSDT
config option, that works on ARM in mainline with no changes required).

I just want to understand if there is really a compelling reason
for adding this stuff when we can easily implement its features
through something that is usable today without any kernel changes.

Thanks,
Lorenzo


Re: [PATCH 3/3] ACPI: ARM64: support for ACPI_TABLE_UPGRADE

2016-05-23 Thread Lorenzo Pieralisi
On Tue, May 17, 2016 at 12:48:53PM -0400, Jon Masters wrote:
> On 05/17/2016 12:44 PM, Jon Masters wrote:
> 
> > 1). During development of a platform, it is much easier to debug
> > problems with tables if you can test replacement ones without having to
> > respin the firmware. In the server world, you usually don't have the
> > firmware source code, so to get it respun could be days-weeks even if
> > you are working with the authors closely. We have practically used this
> > feature on a number of platforms already and it will continue.
> 
> For example, on one platform we were unable to fully boot RHEL(SA) due
> to a bug in one of the ACPI tables. But I was able to boot the system to
> a ramdisk containing a uuencode library and then write out the content
> of the tables over the serial port, then decompile/patch/recompile, and
> override replacement tables on the system. Then we beat the vendor up
> with the fixes and the official firmware was corrected.

Can you explain to me please why you can't do it with GRUB ?

I am using mainline GRUB and its acpi command all the time to update
static ACPI tables for testing new features (ie IORT) and it works
just fine for me (and you can still override the DSDT, which is
likely to be the main source of bugs, through the CONFIG_ACPI_CUSTOM_DSDT
config option, that works on ARM in mainline with no changes required).

I just want to understand if there is really a compelling reason
for adding this stuff when we can easily implement its features
through something that is usable today without any kernel changes.

Thanks,
Lorenzo


Re: [PATCH] x86: fix potential memleak in do_error_trap

2016-05-23 Thread Michal Hocko
On Mon 23-05-16 17:33:55, Oleg Nesterov wrote:
> On 05/23, Michal Hocko wrote:
> >
> > @@ -271,6 +271,7 @@ static void do_error_trap(struct pt_regs *regs, long 
> > error_code, char *str,
> >
> > if (notify_die(DIE_TRAP, str, regs, error_code, trapnr, signr) !=
> > NOTIFY_STOP) {
> > +   memset(, 0, sizeof(info));
> > cond_local_irq_enable(regs);
> > do_trap(trapnr, signr, str, regs, error_code,
> > fill_trap_info(regs, signr, trapnr, ));
> 
> at first glance fill_trap_info() initializes everything we will copy
> to user-space in copy_siginfo_to_user(__SI_FAULT).

Ohh, you are right. Dunno, how I managed to miss it. Sorry about the
noise.
 
> But even if not, shuldn't we change fill_trap_info() instead ?

Yes that would be the proper place.
-- 
Michal Hocko
SUSE Labs


Re: [PATCH] x86: fix potential memleak in do_error_trap

2016-05-23 Thread Michal Hocko
On Mon 23-05-16 17:33:55, Oleg Nesterov wrote:
> On 05/23, Michal Hocko wrote:
> >
> > @@ -271,6 +271,7 @@ static void do_error_trap(struct pt_regs *regs, long 
> > error_code, char *str,
> >
> > if (notify_die(DIE_TRAP, str, regs, error_code, trapnr, signr) !=
> > NOTIFY_STOP) {
> > +   memset(, 0, sizeof(info));
> > cond_local_irq_enable(regs);
> > do_trap(trapnr, signr, str, regs, error_code,
> > fill_trap_info(regs, signr, trapnr, ));
> 
> at first glance fill_trap_info() initializes everything we will copy
> to user-space in copy_siginfo_to_user(__SI_FAULT).

Ohh, you are right. Dunno, how I managed to miss it. Sorry about the
noise.
 
> But even if not, shuldn't we change fill_trap_info() instead ?

Yes that would be the proper place.
-- 
Michal Hocko
SUSE Labs


Re: [PATCH v2 2/2] i2c: qup: support SMBus block read

2016-05-23 Thread Christ, Austin



On 5/20/2016 2:31 AM, Sricharan wrote:

Hi,



@@ -1128,6 +1173,22 @@ static int qup_i2c_read_one_v2(struct
qup_i2c_dev *qup, struct i2c_msg *msg)
goto err;

qup->blk.pos++;
+
+   /* Handle SMBus block read length */
+   if (qup_i2c_check_msg_len(msg) && (msg->len == 1)) {
+   if (msg->buf[0] > I2C_SMBUS_BLOCK_MAX) {
+   ret = -EPROTO;
+   goto err;
+   }
+   msg->len += msg->buf[0];
+   qup->pos = 0;
+   qup_i2c_set_read_mode_v2(qup, msg->len);
+   qup_i2c_issue_xfer_v2(qup, msg);
+   ret = qup_i2c_wait_for_complete(qup, msg);
+   if (ret)
+   goto err;

Is the issue_xfer_v2 needed inside this here ?

No, qup_i2c_issue_xfer_v2 is needed so that we read rest of the data
that is indicated by the length we read earlier.

So qup_i2c_issue_xfer_v2 writes the tags and there is one already in the loop 
above this
  check. So if you just do qup_i2c_set_read_mode_v2 and qup_i2c_set_blk_data 
inside this
  check, will not be enough ?

Regards,
  Sricharan

In testing, removing the call to qup_i2c_issue_xfer_v2() within the 
conditional statement for SMBus length causes failures. That transfer 
request is needed to read that block of data.


Thanks,
Austin


Re: [PATCH v2 2/2] i2c: qup: support SMBus block read

2016-05-23 Thread Christ, Austin



On 5/20/2016 2:31 AM, Sricharan wrote:

Hi,



@@ -1128,6 +1173,22 @@ static int qup_i2c_read_one_v2(struct
qup_i2c_dev *qup, struct i2c_msg *msg)
goto err;

qup->blk.pos++;
+
+   /* Handle SMBus block read length */
+   if (qup_i2c_check_msg_len(msg) && (msg->len == 1)) {
+   if (msg->buf[0] > I2C_SMBUS_BLOCK_MAX) {
+   ret = -EPROTO;
+   goto err;
+   }
+   msg->len += msg->buf[0];
+   qup->pos = 0;
+   qup_i2c_set_read_mode_v2(qup, msg->len);
+   qup_i2c_issue_xfer_v2(qup, msg);
+   ret = qup_i2c_wait_for_complete(qup, msg);
+   if (ret)
+   goto err;

Is the issue_xfer_v2 needed inside this here ?

No, qup_i2c_issue_xfer_v2 is needed so that we read rest of the data
that is indicated by the length we read earlier.

So qup_i2c_issue_xfer_v2 writes the tags and there is one already in the loop 
above this
  check. So if you just do qup_i2c_set_read_mode_v2 and qup_i2c_set_blk_data 
inside this
  check, will not be enough ?

Regards,
  Sricharan

In testing, removing the call to qup_i2c_issue_xfer_v2() within the 
conditional statement for SMBus length causes failures. That transfer 
request is needed to read that block of data.


Thanks,
Austin


Re: [PATCH 3/3] mm, thp: make swapin readahead under down_read of mmap_sem

2016-05-23 Thread Kirill A. Shutemov
On Mon, May 23, 2016 at 08:14:11PM +0300, Ebru Akagunduz wrote:
> Currently khugepaged makes swapin readahead under
> down_write. This patch supplies to make swapin
> readahead under down_read instead of down_write.
> 
> The patch was tested with a test program that allocates
> 800MB of memory, writes to it, and then sleeps. The system
> was forced to swap out all. Afterwards, the test program
> touches the area by writing, it skips a page in each
> 20 pages of the area.
> 
> Signed-off-by: Ebru Akagunduz 
> ---
>  mm/huge_memory.c | 33 +++--
>  1 file changed, 27 insertions(+), 6 deletions(-)
> 
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index feee44c..668bc07 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -2386,13 +2386,14 @@ static bool hugepage_vma_check(struct vm_area_struct 
> *vma)
>   * but with mmap_sem held to protect against vma changes.
>   */
>  
> -static void __collapse_huge_page_swapin(struct mm_struct *mm,
> +static bool __collapse_huge_page_swapin(struct mm_struct *mm,
>   struct vm_area_struct *vma,
>   unsigned long address, pmd_t *pmd)
>  {
>   unsigned long _address;
>   pte_t *pte, pteval;
>   int swapped_in = 0, ret = 0;
> + struct vm_area_struct *vma_orig = vma;
>  
>   pte = pte_offset_map(pmd, address);
>   for (_address = address; _address < address + HPAGE_PMD_NR*PAGE_SIZE;
> @@ -2402,11 +2403,19 @@ static void __collapse_huge_page_swapin(struct 
> mm_struct *mm,
>   continue;
>   swapped_in++;
>   ret = do_swap_page(mm, vma, _address, pte, pmd,
> -
> FAULT_FLAG_ALLOW_RETRY|FAULT_FLAG_RETRY_NOWAIT,
> +FAULT_FLAG_ALLOW_RETRY,
>  pteval);
> + /* do_swap_page returns VM_FAULT_RETRY with released mmap_sem */
> + if (ret & VM_FAULT_RETRY) {
> + down_read(>mmap_sem);
> + vma = find_vma(mm, address);
> + /* vma is no longer available, don't continue to swapin 
> */
> + if (vma != vma_orig)
> + return false;
> + }
>   if (ret & VM_FAULT_ERROR) {
>   trace_mm_collapse_huge_page_swapin(mm, swapped_in, 0);
> - return;
> + return false;
>   }
>   /* pte is unmapped now, we need to map it */
>   pte = pte_offset_map(pmd, _address);
> @@ -2414,6 +2423,7 @@ static void __collapse_huge_page_swapin(struct 
> mm_struct *mm,
>   pte--;
>   pte_unmap(pte);
>   trace_mm_collapse_huge_page_swapin(mm, swapped_in, 1);
> + return true;
>  }
>  
>  static void collapse_huge_page(struct mm_struct *mm,
> @@ -2459,7 +2469,7 @@ static void collapse_huge_page(struct mm_struct *mm,
>* gup_fast later hanlded by the ptep_clear_flush and the VM
>* handled by the anon_vma lock + PG_lock.
>*/
> - down_write(>mmap_sem);
> + down_read(>mmap_sem);
>   if (unlikely(khugepaged_test_exit(mm))) {
>   result = SCAN_ANY_PROCESS;
>   goto out;
> @@ -2490,9 +2500,20 @@ static void collapse_huge_page(struct mm_struct *mm,
>* Don't perform swapin readahead when the system is under pressure,
>* to avoid unnecessary resource consumption.
>*/
> - if (allocstall == curr_allocstall && swap != 0)
> - __collapse_huge_page_swapin(mm, vma, address, pmd);
> + if (allocstall == curr_allocstall && swap != 0) {
> + /*
> +  * __collapse_huge_page_swapin always returns with mmap_sem
> +  * locked. If it fails, release mmap_sem and jump directly
> +  * label out. Continuing to collapse causes inconsistency.
> +  */
> + if (!__collapse_huge_page_swapin(mm, vma, address, pmd)) {
> + up_read(>mmap_sem);
> + goto out;
> + }
> + }
>  
> + up_read(>mmap_sem);
> + down_write(>mmap_sem);

That's the critical point.

How do you guarantee that the vma will not be destroyed (or changed)
between up_read() and down_write()?

You need at least find_vma() again.

>   anon_vma_lock_write(vma->anon_vma);
>  
>   pte = pte_offset_map(pmd, address);
> -- 
> 1.9.1
> 
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majord...@kvack.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: mailto:"d...@kvack.org;> em...@kvack.org 

-- 
 Kirill A. Shutemov


Re: [PATCH 3/3] mm, thp: make swapin readahead under down_read of mmap_sem

2016-05-23 Thread Kirill A. Shutemov
On Mon, May 23, 2016 at 08:14:11PM +0300, Ebru Akagunduz wrote:
> Currently khugepaged makes swapin readahead under
> down_write. This patch supplies to make swapin
> readahead under down_read instead of down_write.
> 
> The patch was tested with a test program that allocates
> 800MB of memory, writes to it, and then sleeps. The system
> was forced to swap out all. Afterwards, the test program
> touches the area by writing, it skips a page in each
> 20 pages of the area.
> 
> Signed-off-by: Ebru Akagunduz 
> ---
>  mm/huge_memory.c | 33 +++--
>  1 file changed, 27 insertions(+), 6 deletions(-)
> 
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index feee44c..668bc07 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -2386,13 +2386,14 @@ static bool hugepage_vma_check(struct vm_area_struct 
> *vma)
>   * but with mmap_sem held to protect against vma changes.
>   */
>  
> -static void __collapse_huge_page_swapin(struct mm_struct *mm,
> +static bool __collapse_huge_page_swapin(struct mm_struct *mm,
>   struct vm_area_struct *vma,
>   unsigned long address, pmd_t *pmd)
>  {
>   unsigned long _address;
>   pte_t *pte, pteval;
>   int swapped_in = 0, ret = 0;
> + struct vm_area_struct *vma_orig = vma;
>  
>   pte = pte_offset_map(pmd, address);
>   for (_address = address; _address < address + HPAGE_PMD_NR*PAGE_SIZE;
> @@ -2402,11 +2403,19 @@ static void __collapse_huge_page_swapin(struct 
> mm_struct *mm,
>   continue;
>   swapped_in++;
>   ret = do_swap_page(mm, vma, _address, pte, pmd,
> -
> FAULT_FLAG_ALLOW_RETRY|FAULT_FLAG_RETRY_NOWAIT,
> +FAULT_FLAG_ALLOW_RETRY,
>  pteval);
> + /* do_swap_page returns VM_FAULT_RETRY with released mmap_sem */
> + if (ret & VM_FAULT_RETRY) {
> + down_read(>mmap_sem);
> + vma = find_vma(mm, address);
> + /* vma is no longer available, don't continue to swapin 
> */
> + if (vma != vma_orig)
> + return false;
> + }
>   if (ret & VM_FAULT_ERROR) {
>   trace_mm_collapse_huge_page_swapin(mm, swapped_in, 0);
> - return;
> + return false;
>   }
>   /* pte is unmapped now, we need to map it */
>   pte = pte_offset_map(pmd, _address);
> @@ -2414,6 +2423,7 @@ static void __collapse_huge_page_swapin(struct 
> mm_struct *mm,
>   pte--;
>   pte_unmap(pte);
>   trace_mm_collapse_huge_page_swapin(mm, swapped_in, 1);
> + return true;
>  }
>  
>  static void collapse_huge_page(struct mm_struct *mm,
> @@ -2459,7 +2469,7 @@ static void collapse_huge_page(struct mm_struct *mm,
>* gup_fast later hanlded by the ptep_clear_flush and the VM
>* handled by the anon_vma lock + PG_lock.
>*/
> - down_write(>mmap_sem);
> + down_read(>mmap_sem);
>   if (unlikely(khugepaged_test_exit(mm))) {
>   result = SCAN_ANY_PROCESS;
>   goto out;
> @@ -2490,9 +2500,20 @@ static void collapse_huge_page(struct mm_struct *mm,
>* Don't perform swapin readahead when the system is under pressure,
>* to avoid unnecessary resource consumption.
>*/
> - if (allocstall == curr_allocstall && swap != 0)
> - __collapse_huge_page_swapin(mm, vma, address, pmd);
> + if (allocstall == curr_allocstall && swap != 0) {
> + /*
> +  * __collapse_huge_page_swapin always returns with mmap_sem
> +  * locked. If it fails, release mmap_sem and jump directly
> +  * label out. Continuing to collapse causes inconsistency.
> +  */
> + if (!__collapse_huge_page_swapin(mm, vma, address, pmd)) {
> + up_read(>mmap_sem);
> + goto out;
> + }
> + }
>  
> + up_read(>mmap_sem);
> + down_write(>mmap_sem);

That's the critical point.

How do you guarantee that the vma will not be destroyed (or changed)
between up_read() and down_write()?

You need at least find_vma() again.

>   anon_vma_lock_write(vma->anon_vma);
>  
>   pte = pte_offset_map(pmd, address);
> -- 
> 1.9.1
> 
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majord...@kvack.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: mailto:"d...@kvack.org;> em...@kvack.org 

-- 
 Kirill A. Shutemov


Re: [PATCH] mm: memcontrol: fix possible css ref leak on oom

2016-05-23 Thread Michal Hocko
On Mon 23-05-16 19:02:10, Vladimir Davydov wrote:
> mem_cgroup_oom may be invoked multiple times while a process is handling
> a page fault, in which case current->memcg_in_oom will be overwritten
> leaking the previously taken css reference.

Have you seen this happening? I was under impression that the page fault
paths that have oom enabled will not retry allocations.
 
> Signed-off-by: Vladimir Davydov 

That being said I do not have anything against the patch. It is a good
safety net I am just not sure this might happen right now and so the
patch is not stable candidate.

After clarification
Acked-by: Michal Hocko 

> ---
>  mm/memcontrol.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 5b48cd25951b..ef8797d34039 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -1608,7 +1608,7 @@ static void memcg_oom_recover(struct mem_cgroup *memcg)
>  
>  static void mem_cgroup_oom(struct mem_cgroup *memcg, gfp_t mask, int order)
>  {
> - if (!current->memcg_may_oom)
> + if (!current->memcg_may_oom || current->memcg_in_oom)
>   return;
>   /*
>* We are in the middle of the charge context here, so we
> -- 
> 2.1.4

-- 
Michal Hocko
SUSE Labs


Re: [PATCH] mm: memcontrol: fix possible css ref leak on oom

2016-05-23 Thread Michal Hocko
On Mon 23-05-16 19:02:10, Vladimir Davydov wrote:
> mem_cgroup_oom may be invoked multiple times while a process is handling
> a page fault, in which case current->memcg_in_oom will be overwritten
> leaking the previously taken css reference.

Have you seen this happening? I was under impression that the page fault
paths that have oom enabled will not retry allocations.
 
> Signed-off-by: Vladimir Davydov 

That being said I do not have anything against the patch. It is a good
safety net I am just not sure this might happen right now and so the
patch is not stable candidate.

After clarification
Acked-by: Michal Hocko 

> ---
>  mm/memcontrol.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 5b48cd25951b..ef8797d34039 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -1608,7 +1608,7 @@ static void memcg_oom_recover(struct mem_cgroup *memcg)
>  
>  static void mem_cgroup_oom(struct mem_cgroup *memcg, gfp_t mask, int order)
>  {
> - if (!current->memcg_may_oom)
> + if (!current->memcg_may_oom || current->memcg_in_oom)
>   return;
>   /*
>* We are in the middle of the charge context here, so we
> -- 
> 2.1.4

-- 
Michal Hocko
SUSE Labs


<    2   3   4   5   6   7   8   9   10   11   >