Re: [PATCH 1/2] dt-bindings: trivial: add tfa9879 device

[Peter Rosin]

On 2017-12-02 22:17, Fabio Estevam wrote:
> On Fri, Dec 1, 2017 at 8:44 PM, Peter Rosin  wrote:
>> Add record for NXP TFA9879 Mono BTL Class D audio amplifier.
>>
>> Signed-off-by: Peter Rosin 
>> ---
>>  Documentation/devicetree/bindings/trivial-devices.txt | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/Documentation/devicetree/bindings/trivial-devices.txt 
>> b/Documentation/devicetree/bindings/trivial-devices.txt
>> index af284fbd4d23..4cf30ce4e71f 100644
>> --- a/Documentation/devicetree/bindings/trivial-devices.txt
>> +++ b/Documentation/devicetree/bindings/trivial-devices.txt
>> @@ -157,6 +157,7 @@ nxp,pcf2127 Real-time clock
>>  nxp,pcf2129Real-time clock
>>  nxp,pcf8563Real-time clock/calendar
>>  nxp,pcf85063   Tiny Real-Time Clock
>> +nxp,tfa9879Mono BTL Class D audio amplifier
> 
> This is already described at 
> Documentation/devicetree/bindings/sound/tfa9879.txt

Right. However, the patch adding that should have been sent to me, the
maintainer of the driver. That is carefully recorded in MAINTAINERS. So,
forgive me for assuming that nothing had changed in the driver behind my
back.

Had that patch been sent my way as it should have been, I would have
insisted that maintenance of the bindings had been kept together with
the maintenance of the driver.

Cheers,
Peter

Re: [PATCH 1/2] dt-bindings: trivial: add tfa9879 device

[Peter Rosin]

On 2017-12-02 22:17, Fabio Estevam wrote:
> On Fri, Dec 1, 2017 at 8:44 PM, Peter Rosin  wrote:
>> Add record for NXP TFA9879 Mono BTL Class D audio amplifier.
>>
>> Signed-off-by: Peter Rosin 
>> ---
>>  Documentation/devicetree/bindings/trivial-devices.txt | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/Documentation/devicetree/bindings/trivial-devices.txt 
>> b/Documentation/devicetree/bindings/trivial-devices.txt
>> index af284fbd4d23..4cf30ce4e71f 100644
>> --- a/Documentation/devicetree/bindings/trivial-devices.txt
>> +++ b/Documentation/devicetree/bindings/trivial-devices.txt
>> @@ -157,6 +157,7 @@ nxp,pcf2127 Real-time clock
>>  nxp,pcf2129Real-time clock
>>  nxp,pcf8563Real-time clock/calendar
>>  nxp,pcf85063   Tiny Real-Time Clock
>> +nxp,tfa9879Mono BTL Class D audio amplifier
> 
> This is already described at 
> Documentation/devicetree/bindings/sound/tfa9879.txt

Right. However, the patch adding that should have been sent to me, the
maintainer of the driver. That is carefully recorded in MAINTAINERS. So,
forgive me for assuming that nothing had changed in the driver behind my
back.

Had that patch been sent my way as it should have been, I would have
insisted that maintenance of the bindings had been kept together with
the maintenance of the driver.

Cheers,
Peter

Re: [PATCH] cpu/hotplug: merge cpuhp_bp_states_ap_states as cpuhp_hp_states

[Paul E. McKenney]

On Fri, Dec 01, 2017 at 09:50:05PM +0800, Lai Jiangshan wrote:
> cpuhp_bp_states_ap_states have diffent set of steps
> without any conflicting configed steps, so that they can
> be merged.
> 
> The original `[CPUHP_BRINGUP_CPU] = { },` is removed, because
> the new cpuhp_hp_states has CPUHP_ONLINE index which is larger
> than CPUHP_BRINGUP_CPU.
> 
> Signed-off-by: Lai Jiangshan 

Hello, Lai,

What is this against?  I tried testing it, but it doesn't apply to
v4.15-rc1 or v4.14.

Thanx, Paul

> ---
>  kernel/cpu.c | 42 +++---
>  1 file changed, 15 insertions(+), 27 deletions(-)
> 
> diff --git a/kernel/cpu.c b/kernel/cpu.c
> index 7891aecc6aec..c27963066333 100644
> --- a/kernel/cpu.c
> +++ b/kernel/cpu.c
> @@ -124,8 +124,7 @@ struct cpuhp_step {
>  };
> 
>  static DEFINE_MUTEX(cpuhp_state_mutex);
> -static struct cpuhp_step cpuhp_bp_states[];
> -static struct cpuhp_step cpuhp_ap_states[];
> +static struct cpuhp_step cpuhp_hp_states[];
> 
>  static bool cpuhp_is_ap_state(enum cpuhp_state state)
>  {
> @@ -138,10 +137,7 @@ static bool cpuhp_is_ap_state(enum cpuhp_state state)
> 
>  static struct cpuhp_step *cpuhp_get_step(enum cpuhp_state state)
>  {
> - struct cpuhp_step *sp;
> -
> - sp = cpuhp_is_ap_state(state) ? cpuhp_ap_states : cpuhp_bp_states;
> - return sp + state;
> + return cpuhp_hp_states + state;
>  }
> 
>  /**
> @@ -1224,7 +1220,7 @@ int __boot_cpu_id;
>  #endif /* CONFIG_SMP */
> 
>  /* Boot processor state steps */
> -static struct cpuhp_step cpuhp_bp_states[] = {
> +static struct cpuhp_step cpuhp_hp_states[] = {
>   [CPUHP_OFFLINE] = {
>   .name   = "offline",
>   .startup.single = NULL,
> @@ -1289,24 +1285,6 @@ static struct cpuhp_step cpuhp_bp_states[] = {
>   .teardown.single= NULL,
>   .cant_stop  = true,
>   },
> - /*
> -  * Handled on controll processor until the plugged processor manages
> -  * this itself.
> -  */
> - [CPUHP_TEARDOWN_CPU] = {
> - .name   = "cpu:teardown",
> - .startup.single = NULL,
> - .teardown.single= takedown_cpu,
> - .cant_stop  = true,
> - },
> -#else
> - [CPUHP_BRINGUP_CPU] = { },
> -#endif
> -};
> -
> -/* Application processor state steps */
> -static struct cpuhp_step cpuhp_ap_states[] = {
> -#ifdef CONFIG_SMP
>   /* Final state before CPU kills itself */
>   [CPUHP_AP_IDLE_DEAD] = {
>   .name   = "idle:dead",
> @@ -1340,6 +1318,16 @@ static struct cpuhp_step cpuhp_ap_states[] = {
>   [CPUHP_AP_ONLINE] = {
>   .name   = "ap:online",
>   },
> + /*
> +  * Handled on controll processor until the plugged processor manages
> +  * this itself.
> +  */
> + [CPUHP_TEARDOWN_CPU] = {
> + .name   = "cpu:teardown",
> + .startup.single = NULL,
> + .teardown.single= takedown_cpu,
> + .cant_stop  = true,
> + },
>   /* Handle smpboot threads park/unpark */
>   [CPUHP_AP_SMPBOOT_THREADS] = {
>   .name   = "smpboot/threads:online",
> @@ -1408,11 +1396,11 @@ static int cpuhp_reserve_state(enum cpuhp_state state)
> 
>   switch (state) {
>   case CPUHP_AP_ONLINE_DYN:
> - step = cpuhp_ap_states + CPUHP_AP_ONLINE_DYN;
> + step = cpuhp_hp_states + CPUHP_AP_ONLINE_DYN;
>   end = CPUHP_AP_ONLINE_DYN_END;
>   break;
>   case CPUHP_BP_PREPARE_DYN:
> - step = cpuhp_bp_states + CPUHP_BP_PREPARE_DYN;
> + step = cpuhp_hp_states + CPUHP_BP_PREPARE_DYN;
>   end = CPUHP_BP_PREPARE_DYN_END;
>   break;
>   default:
> -- 
> 2.13.6 (Apple Git-96)
> 

Re: [PATCH] cpu/hotplug: merge cpuhp_bp_states_ap_states as cpuhp_hp_states

[Paul E. McKenney]

On Fri, Dec 01, 2017 at 09:50:05PM +0800, Lai Jiangshan wrote:
> cpuhp_bp_states_ap_states have diffent set of steps
> without any conflicting configed steps, so that they can
> be merged.
> 
> The original `[CPUHP_BRINGUP_CPU] = { },` is removed, because
> the new cpuhp_hp_states has CPUHP_ONLINE index which is larger
> than CPUHP_BRINGUP_CPU.
> 
> Signed-off-by: Lai Jiangshan 

Hello, Lai,

What is this against?  I tried testing it, but it doesn't apply to
v4.15-rc1 or v4.14.

Thanx, Paul

> ---
>  kernel/cpu.c | 42 +++---
>  1 file changed, 15 insertions(+), 27 deletions(-)
> 
> diff --git a/kernel/cpu.c b/kernel/cpu.c
> index 7891aecc6aec..c27963066333 100644
> --- a/kernel/cpu.c
> +++ b/kernel/cpu.c
> @@ -124,8 +124,7 @@ struct cpuhp_step {
>  };
> 
>  static DEFINE_MUTEX(cpuhp_state_mutex);
> -static struct cpuhp_step cpuhp_bp_states[];
> -static struct cpuhp_step cpuhp_ap_states[];
> +static struct cpuhp_step cpuhp_hp_states[];
> 
>  static bool cpuhp_is_ap_state(enum cpuhp_state state)
>  {
> @@ -138,10 +137,7 @@ static bool cpuhp_is_ap_state(enum cpuhp_state state)
> 
>  static struct cpuhp_step *cpuhp_get_step(enum cpuhp_state state)
>  {
> - struct cpuhp_step *sp;
> -
> - sp = cpuhp_is_ap_state(state) ? cpuhp_ap_states : cpuhp_bp_states;
> - return sp + state;
> + return cpuhp_hp_states + state;
>  }
> 
>  /**
> @@ -1224,7 +1220,7 @@ int __boot_cpu_id;
>  #endif /* CONFIG_SMP */
> 
>  /* Boot processor state steps */
> -static struct cpuhp_step cpuhp_bp_states[] = {
> +static struct cpuhp_step cpuhp_hp_states[] = {
>   [CPUHP_OFFLINE] = {
>   .name   = "offline",
>   .startup.single = NULL,
> @@ -1289,24 +1285,6 @@ static struct cpuhp_step cpuhp_bp_states[] = {
>   .teardown.single= NULL,
>   .cant_stop  = true,
>   },
> - /*
> -  * Handled on controll processor until the plugged processor manages
> -  * this itself.
> -  */
> - [CPUHP_TEARDOWN_CPU] = {
> - .name   = "cpu:teardown",
> - .startup.single = NULL,
> - .teardown.single= takedown_cpu,
> - .cant_stop  = true,
> - },
> -#else
> - [CPUHP_BRINGUP_CPU] = { },
> -#endif
> -};
> -
> -/* Application processor state steps */
> -static struct cpuhp_step cpuhp_ap_states[] = {
> -#ifdef CONFIG_SMP
>   /* Final state before CPU kills itself */
>   [CPUHP_AP_IDLE_DEAD] = {
>   .name   = "idle:dead",
> @@ -1340,6 +1318,16 @@ static struct cpuhp_step cpuhp_ap_states[] = {
>   [CPUHP_AP_ONLINE] = {
>   .name   = "ap:online",
>   },
> + /*
> +  * Handled on controll processor until the plugged processor manages
> +  * this itself.
> +  */
> + [CPUHP_TEARDOWN_CPU] = {
> + .name   = "cpu:teardown",
> + .startup.single = NULL,
> + .teardown.single= takedown_cpu,
> + .cant_stop  = true,
> + },
>   /* Handle smpboot threads park/unpark */
>   [CPUHP_AP_SMPBOOT_THREADS] = {
>   .name   = "smpboot/threads:online",
> @@ -1408,11 +1396,11 @@ static int cpuhp_reserve_state(enum cpuhp_state state)
> 
>   switch (state) {
>   case CPUHP_AP_ONLINE_DYN:
> - step = cpuhp_ap_states + CPUHP_AP_ONLINE_DYN;
> + step = cpuhp_hp_states + CPUHP_AP_ONLINE_DYN;
>   end = CPUHP_AP_ONLINE_DYN_END;
>   break;
>   case CPUHP_BP_PREPARE_DYN:
> - step = cpuhp_bp_states + CPUHP_BP_PREPARE_DYN;
> + step = cpuhp_hp_states + CPUHP_BP_PREPARE_DYN;
>   end = CPUHP_BP_PREPARE_DYN_END;
>   break;
>   default:
> -- 
> 2.13.6 (Apple Git-96)
> 

Re: [PATCH] refcount_t: documentation for memory ordering differences

[Andrea Parri]

On Sun, Dec 03, 2017 at 07:20:03AM +0100, Andrea Parri wrote:
> On Fri, Dec 01, 2017 at 12:34:23PM -0800, Randy Dunlap wrote:
> > On 11/29/2017 04:36 AM, Elena Reshetova wrote:
> > > Some functions from refcount_t API provide different
> > > memory ordering guarantees that their atomic counterparts.
> > > This adds a document outlining these differences.
> > > 
> > > Signed-off-by: Elena Reshetova 
> > > ---
> > >  Documentation/core-api/index.rst  |   1 +
> > >  Documentation/core-api/refcount-vs-atomic.rst | 129 
> > > ++
> > >  2 files changed, 130 insertions(+)
> > >  create mode 100644 Documentation/core-api/refcount-vs-atomic.rst
> > 
> > > diff --git a/Documentation/core-api/refcount-vs-atomic.rst 
> > > b/Documentation/core-api/refcount-vs-atomic.rst
> > > new file mode 100644
> > > index 000..5619d48
> > > --- /dev/null
> > > +++ b/Documentation/core-api/refcount-vs-atomic.rst
> > > @@ -0,0 +1,129 @@
> > > +===
> > > +refcount_t API compared to atomic_t
> > > +===
> > > +
> > > +The goal of refcount_t API is to provide a minimal API for implementing
> > > +an object's reference counters. While a generic architecture-independent
> > > +implementation from lib/refcount.c uses atomic operations underneath,
> > > +there are a number of differences between some of the refcount_*() and
> > > +atomic_*() functions with regards to the memory ordering guarantees.
> > > +This document outlines the differences and provides respective examples
> > > +in order to help maintainers validate their code against the change in
> > > +these memory ordering guarantees.
> > > +
> > > +memory-barriers.txt and atomic_t.txt provide more background to the
> > > +memory ordering in general and for atomic operations specifically.
> > > +
> > > +Relevant types of memory ordering
> > > +=
> > > +
> > > +**Note**: the following section only covers some of the memory
> > > +ordering types that are relevant for the atomics and reference
> > > +counters and used through this document. For a much broader picture
> > > +please consult memory-barriers.txt document.
> > > +
> > > +In the absence of any memory ordering guarantees (i.e. fully unordered)
> > > +atomics & refcounters only provide atomicity and
> > > +program order (po) relation (on the same CPU). It guarantees that
> > > +each atomic_*() and refcount_*() operation is atomic and instructions
> > > +are executed in program order on a single CPU.
> > > +This is implemented using READ_ONCE()/WRITE_ONCE() and
> > > +compare-and-swap primitives.
> > > +
> > > +A strong (full) memory ordering guarantees that all prior loads and
> > > +stores (all po-earlier instructions) on the same CPU are completed
> > > +before any po-later instruction is executed on the same CPU.
> > > +It also guarantees that all po-earlier stores on the same CPU
> > > +and all propagated stores from other CPUs must propagate to all
> > > +other CPUs before any po-later instruction is executed on the original
> > > +CPU (A-cumulative property). This is implemented using smp_mb().
> > 
> > I don't know what "A-cumulative property" means, and google search didn't
> > either.
> 
> The description above seems to follow the (informal) definition given in:
> 
>   
> https://github.com/aparri/memory-model/blob/master/Documentation/explanation.txt
>   (c.f., in part., Sect. 13-14)
> 
> and formalized by the LKMM. (The notion of A-cumulativity also appears, in
> different contexts, in some memory consistency literature, e.g.,
> 
>   http://www.cl.cam.ac.uk/~pes20/ppc-supplemental/index.html
>   http://www.cl.cam.ac.uk/~pes20/armv8-mca/
>   https://arxiv.org/abs/1308.6810 )
> 
> A typical illustration of A-cumulativity (for smp_store_release(), say) is
> given with the following program:
> 
> int x = 0;
> int y = 0;
> 
> void thread0()
> {
>   WRITE_ONCE(x, 1);
> }
> 
> void thread1()
> {
>   int r0;
> 
>   r0 = READ_ONCE(x);
>   smp_store_release(, 1);
> }
> 
> void thread2()
> {
>   int r1;
>   int r2;
> 
>   r1 = READ_ONCE(y);
>   smp_rmb();
>   r2 = READ_ONCE(x);
> }
> 
> (This is a variation of the so called "message-passing" pattern, where the
>  stores are "distributed" over two threads; see also
> 
>   
> https://github.com/aparri/memory-model/blob/master/litmus-tests/WRC%2Bpooncerelease%2Brmbonceonce%2BOnce.litmus
>  )
> 
> The question we want to address is whether the final state
> 
>   (r0 == 1 && r1 == 1 && r2 == 0)
> 
> can be reached/is allowed, and the answer is no (due to the A-cumulativity
> of the store-release).
> 
> By contrast, dependencies provides no (A-)cumulativity; for example, if we
> modify the previous program by replacing the store-release with a data dep.
> as follows:
> 
> int x = 0;
> int y = 0;
> 
> void thread0()
> {
>   WRITE_ONCE(x, 1);
> }
> 
> void thread1()
> {
>   int r0;

Re: [PATCH] refcount_t: documentation for memory ordering differences

[Andrea Parri]

On Sun, Dec 03, 2017 at 07:20:03AM +0100, Andrea Parri wrote:
> On Fri, Dec 01, 2017 at 12:34:23PM -0800, Randy Dunlap wrote:
> > On 11/29/2017 04:36 AM, Elena Reshetova wrote:
> > > Some functions from refcount_t API provide different
> > > memory ordering guarantees that their atomic counterparts.
> > > This adds a document outlining these differences.
> > > 
> > > Signed-off-by: Elena Reshetova 
> > > ---
> > >  Documentation/core-api/index.rst  |   1 +
> > >  Documentation/core-api/refcount-vs-atomic.rst | 129 
> > > ++
> > >  2 files changed, 130 insertions(+)
> > >  create mode 100644 Documentation/core-api/refcount-vs-atomic.rst
> > 
> > > diff --git a/Documentation/core-api/refcount-vs-atomic.rst 
> > > b/Documentation/core-api/refcount-vs-atomic.rst
> > > new file mode 100644
> > > index 000..5619d48
> > > --- /dev/null
> > > +++ b/Documentation/core-api/refcount-vs-atomic.rst
> > > @@ -0,0 +1,129 @@
> > > +===
> > > +refcount_t API compared to atomic_t
> > > +===
> > > +
> > > +The goal of refcount_t API is to provide a minimal API for implementing
> > > +an object's reference counters. While a generic architecture-independent
> > > +implementation from lib/refcount.c uses atomic operations underneath,
> > > +there are a number of differences between some of the refcount_*() and
> > > +atomic_*() functions with regards to the memory ordering guarantees.
> > > +This document outlines the differences and provides respective examples
> > > +in order to help maintainers validate their code against the change in
> > > +these memory ordering guarantees.
> > > +
> > > +memory-barriers.txt and atomic_t.txt provide more background to the
> > > +memory ordering in general and for atomic operations specifically.
> > > +
> > > +Relevant types of memory ordering
> > > +=
> > > +
> > > +**Note**: the following section only covers some of the memory
> > > +ordering types that are relevant for the atomics and reference
> > > +counters and used through this document. For a much broader picture
> > > +please consult memory-barriers.txt document.
> > > +
> > > +In the absence of any memory ordering guarantees (i.e. fully unordered)
> > > +atomics & refcounters only provide atomicity and
> > > +program order (po) relation (on the same CPU). It guarantees that
> > > +each atomic_*() and refcount_*() operation is atomic and instructions
> > > +are executed in program order on a single CPU.
> > > +This is implemented using READ_ONCE()/WRITE_ONCE() and
> > > +compare-and-swap primitives.
> > > +
> > > +A strong (full) memory ordering guarantees that all prior loads and
> > > +stores (all po-earlier instructions) on the same CPU are completed
> > > +before any po-later instruction is executed on the same CPU.
> > > +It also guarantees that all po-earlier stores on the same CPU
> > > +and all propagated stores from other CPUs must propagate to all
> > > +other CPUs before any po-later instruction is executed on the original
> > > +CPU (A-cumulative property). This is implemented using smp_mb().
> > 
> > I don't know what "A-cumulative property" means, and google search didn't
> > either.
> 
> The description above seems to follow the (informal) definition given in:
> 
>   
> https://github.com/aparri/memory-model/blob/master/Documentation/explanation.txt
>   (c.f., in part., Sect. 13-14)
> 
> and formalized by the LKMM. (The notion of A-cumulativity also appears, in
> different contexts, in some memory consistency literature, e.g.,
> 
>   http://www.cl.cam.ac.uk/~pes20/ppc-supplemental/index.html
>   http://www.cl.cam.ac.uk/~pes20/armv8-mca/
>   https://arxiv.org/abs/1308.6810 )
> 
> A typical illustration of A-cumulativity (for smp_store_release(), say) is
> given with the following program:
> 
> int x = 0;
> int y = 0;
> 
> void thread0()
> {
>   WRITE_ONCE(x, 1);
> }
> 
> void thread1()
> {
>   int r0;
> 
>   r0 = READ_ONCE(x);
>   smp_store_release(, 1);
> }
> 
> void thread2()
> {
>   int r1;
>   int r2;
> 
>   r1 = READ_ONCE(y);
>   smp_rmb();
>   r2 = READ_ONCE(x);
> }
> 
> (This is a variation of the so called "message-passing" pattern, where the
>  stores are "distributed" over two threads; see also
> 
>   
> https://github.com/aparri/memory-model/blob/master/litmus-tests/WRC%2Bpooncerelease%2Brmbonceonce%2BOnce.litmus
>  )
> 
> The question we want to address is whether the final state
> 
>   (r0 == 1 && r1 == 1 && r2 == 0)
> 
> can be reached/is allowed, and the answer is no (due to the A-cumulativity
> of the store-release).
> 
> By contrast, dependencies provides no (A-)cumulativity; for example, if we
> modify the previous program by replacing the store-release with a data dep.
> as follows:
> 
> int x = 0;
> int y = 0;
> 
> void thread0()
> {
>   WRITE_ONCE(x, 1);
> }
> 
> void thread1()
> {
>   int r0;
> 
>   r0 = 

Re: [PATCH] refcount_t: documentation for memory ordering differences

[Andrea Parri]

On Fri, Dec 01, 2017 at 12:34:23PM -0800, Randy Dunlap wrote:
> On 11/29/2017 04:36 AM, Elena Reshetova wrote:
> > Some functions from refcount_t API provide different
> > memory ordering guarantees that their atomic counterparts.
> > This adds a document outlining these differences.
> > 
> > Signed-off-by: Elena Reshetova 
> > ---
> >  Documentation/core-api/index.rst  |   1 +
> >  Documentation/core-api/refcount-vs-atomic.rst | 129 
> > ++
> >  2 files changed, 130 insertions(+)
> >  create mode 100644 Documentation/core-api/refcount-vs-atomic.rst
> 
> > diff --git a/Documentation/core-api/refcount-vs-atomic.rst 
> > b/Documentation/core-api/refcount-vs-atomic.rst
> > new file mode 100644
> > index 000..5619d48
> > --- /dev/null
> > +++ b/Documentation/core-api/refcount-vs-atomic.rst
> > @@ -0,0 +1,129 @@
> > +===
> > +refcount_t API compared to atomic_t
> > +===
> > +
> > +The goal of refcount_t API is to provide a minimal API for implementing
> > +an object's reference counters. While a generic architecture-independent
> > +implementation from lib/refcount.c uses atomic operations underneath,
> > +there are a number of differences between some of the refcount_*() and
> > +atomic_*() functions with regards to the memory ordering guarantees.
> > +This document outlines the differences and provides respective examples
> > +in order to help maintainers validate their code against the change in
> > +these memory ordering guarantees.
> > +
> > +memory-barriers.txt and atomic_t.txt provide more background to the
> > +memory ordering in general and for atomic operations specifically.
> > +
> > +Relevant types of memory ordering
> > +=
> > +
> > +**Note**: the following section only covers some of the memory
> > +ordering types that are relevant for the atomics and reference
> > +counters and used through this document. For a much broader picture
> > +please consult memory-barriers.txt document.
> > +
> > +In the absence of any memory ordering guarantees (i.e. fully unordered)
> > +atomics & refcounters only provide atomicity and
> > +program order (po) relation (on the same CPU). It guarantees that
> > +each atomic_*() and refcount_*() operation is atomic and instructions
> > +are executed in program order on a single CPU.
> > +This is implemented using READ_ONCE()/WRITE_ONCE() and
> > +compare-and-swap primitives.
> > +
> > +A strong (full) memory ordering guarantees that all prior loads and
> > +stores (all po-earlier instructions) on the same CPU are completed
> > +before any po-later instruction is executed on the same CPU.
> > +It also guarantees that all po-earlier stores on the same CPU
> > +and all propagated stores from other CPUs must propagate to all
> > +other CPUs before any po-later instruction is executed on the original
> > +CPU (A-cumulative property). This is implemented using smp_mb().
> 
> I don't know what "A-cumulative property" means, and google search didn't
> either.

The description above seems to follow the (informal) definition given in:

  
https://github.com/aparri/memory-model/blob/master/Documentation/explanation.txt
  (c.f., in part., Sect. 13-14)

and formalized by the LKMM. (The notion of A-cumulativity also appears, in
different contexts, in some memory consistency literature, e.g.,

  http://www.cl.cam.ac.uk/~pes20/ppc-supplemental/index.html
  http://www.cl.cam.ac.uk/~pes20/armv8-mca/
  https://arxiv.org/abs/1308.6810 )

A typical illustration of A-cumulativity (for smp_store_release(), say) is
given with the following program:

int x = 0;
int y = 0;

void thread0()
{
WRITE_ONCE(x, 1);
}

void thread1()
{
int r0;

r0 = READ_ONCE(x);
smp_store_release(, 1);
}

void thread2()
{
int r1;
int r2;

r1 = READ_ONCE(y);
smp_rmb();
r2 = READ_ONCE(x);
}

(This is a variation of the so called "message-passing" pattern, where the
 stores are "distributed" over two threads; see also

  
https://github.com/aparri/memory-model/blob/master/litmus-tests/WRC%2Bpooncerelease%2Brmbonceonce%2BOnce.litmus
 )

The question we want to address is whether the final state

  (r0 == 1 && r1 == 1 && r2 == 0)

can be reached/is allowed, and the answer is no (due to the A-cumulativity
of the store-release).

By contrast, dependencies provides no (A-)cumulativity; for example, if we
modify the previous program by replacing the store-release with a data dep.
as follows:

int x = 0;
int y = 0;

void thread0()
{
WRITE_ONCE(x, 1);
}

void thread1()
{
int r0;

r0 = READ_ONCE(x);
WRITE_ONCE(x, r0);
}

void thread2()
{
int r1;
int r2;

r1 = READ_ONCE(y);
smp_rmb();
r2 = READ_ONCE(x);
}

then that same final state is allowed (and observed on some PPC machines).

  Andrea


> 
> Is it non-cumulative, similar to 

Re: [PATCH] refcount_t: documentation for memory ordering differences

[Andrea Parri]

On Fri, Dec 01, 2017 at 12:34:23PM -0800, Randy Dunlap wrote:
> On 11/29/2017 04:36 AM, Elena Reshetova wrote:
> > Some functions from refcount_t API provide different
> > memory ordering guarantees that their atomic counterparts.
> > This adds a document outlining these differences.
> > 
> > Signed-off-by: Elena Reshetova 
> > ---
> >  Documentation/core-api/index.rst  |   1 +
> >  Documentation/core-api/refcount-vs-atomic.rst | 129 
> > ++
> >  2 files changed, 130 insertions(+)
> >  create mode 100644 Documentation/core-api/refcount-vs-atomic.rst
> 
> > diff --git a/Documentation/core-api/refcount-vs-atomic.rst 
> > b/Documentation/core-api/refcount-vs-atomic.rst
> > new file mode 100644
> > index 000..5619d48
> > --- /dev/null
> > +++ b/Documentation/core-api/refcount-vs-atomic.rst
> > @@ -0,0 +1,129 @@
> > +===
> > +refcount_t API compared to atomic_t
> > +===
> > +
> > +The goal of refcount_t API is to provide a minimal API for implementing
> > +an object's reference counters. While a generic architecture-independent
> > +implementation from lib/refcount.c uses atomic operations underneath,
> > +there are a number of differences between some of the refcount_*() and
> > +atomic_*() functions with regards to the memory ordering guarantees.
> > +This document outlines the differences and provides respective examples
> > +in order to help maintainers validate their code against the change in
> > +these memory ordering guarantees.
> > +
> > +memory-barriers.txt and atomic_t.txt provide more background to the
> > +memory ordering in general and for atomic operations specifically.
> > +
> > +Relevant types of memory ordering
> > +=
> > +
> > +**Note**: the following section only covers some of the memory
> > +ordering types that are relevant for the atomics and reference
> > +counters and used through this document. For a much broader picture
> > +please consult memory-barriers.txt document.
> > +
> > +In the absence of any memory ordering guarantees (i.e. fully unordered)
> > +atomics & refcounters only provide atomicity and
> > +program order (po) relation (on the same CPU). It guarantees that
> > +each atomic_*() and refcount_*() operation is atomic and instructions
> > +are executed in program order on a single CPU.
> > +This is implemented using READ_ONCE()/WRITE_ONCE() and
> > +compare-and-swap primitives.
> > +
> > +A strong (full) memory ordering guarantees that all prior loads and
> > +stores (all po-earlier instructions) on the same CPU are completed
> > +before any po-later instruction is executed on the same CPU.
> > +It also guarantees that all po-earlier stores on the same CPU
> > +and all propagated stores from other CPUs must propagate to all
> > +other CPUs before any po-later instruction is executed on the original
> > +CPU (A-cumulative property). This is implemented using smp_mb().
> 
> I don't know what "A-cumulative property" means, and google search didn't
> either.

The description above seems to follow the (informal) definition given in:

  
https://github.com/aparri/memory-model/blob/master/Documentation/explanation.txt
  (c.f., in part., Sect. 13-14)

and formalized by the LKMM. (The notion of A-cumulativity also appears, in
different contexts, in some memory consistency literature, e.g.,

  http://www.cl.cam.ac.uk/~pes20/ppc-supplemental/index.html
  http://www.cl.cam.ac.uk/~pes20/armv8-mca/
  https://arxiv.org/abs/1308.6810 )

A typical illustration of A-cumulativity (for smp_store_release(), say) is
given with the following program:

int x = 0;
int y = 0;

void thread0()
{
WRITE_ONCE(x, 1);
}

void thread1()
{
int r0;

r0 = READ_ONCE(x);
smp_store_release(, 1);
}

void thread2()
{
int r1;
int r2;

r1 = READ_ONCE(y);
smp_rmb();
r2 = READ_ONCE(x);
}

(This is a variation of the so called "message-passing" pattern, where the
 stores are "distributed" over two threads; see also

  
https://github.com/aparri/memory-model/blob/master/litmus-tests/WRC%2Bpooncerelease%2Brmbonceonce%2BOnce.litmus
 )

The question we want to address is whether the final state

  (r0 == 1 && r1 == 1 && r2 == 0)

can be reached/is allowed, and the answer is no (due to the A-cumulativity
of the store-release).

By contrast, dependencies provides no (A-)cumulativity; for example, if we
modify the previous program by replacing the store-release with a data dep.
as follows:

int x = 0;
int y = 0;

void thread0()
{
WRITE_ONCE(x, 1);
}

void thread1()
{
int r0;

r0 = READ_ONCE(x);
WRITE_ONCE(x, r0);
}

void thread2()
{
int r1;
int r2;

r1 = READ_ONCE(y);
smp_rmb();
r2 = READ_ONCE(x);
}

then that same final state is allowed (and observed on some PPC machines).

  Andrea


> 
> Is it non-cumulative, similar to typical vs. atypical, where 

Re: [PATCH] ARM: dts: keystone-k2l: Add the second gpio bank node

[santosh.shilim...@oracle.com]

On 11/13/17 9:12 AM, Santosh Shilimkar wrote:

On 11/9/2017 3:42 AM, Keerthy wrote:

In case of k2l there are 2 more banks with 16 pins each.
Adding the node as the da-vinci driver now supports multiple
banks.

Signed-off-by: Keerthy 
---

This is based on linux-next branch. Boot tested on keystone-k2l-evm.


Will pick this up Keerthy !!


Applied

Re: [PATCH] ARM: dts: keystone-k2l: Add the second gpio bank node

[santosh.shilim...@oracle.com]

On 11/13/17 9:12 AM, Santosh Shilimkar wrote:

On 11/9/2017 3:42 AM, Keerthy wrote:

In case of k2l there are 2 more banks with 16 pins each.
Adding the node as the da-vinci driver now supports multiple
banks.

Signed-off-by: Keerthy 
---

This is based on linux-next branch. Boot tested on keystone-k2l-evm.


Will pick this up Keerthy !!


Applied

Re: PROBLEM: Asus C201 video mode problems on HDMI hotplug (regression)

[Nick Bowler]

Hi Jose,

On 2017-12-02 17:11 +, Jose Abreu wrote:
> On 01-12-2017 00:11, Nick Bowler wrote:
> > Another data point... the following patch appears sufficient to
> > restore working behaviour.
[...]
> I don't think you can do this. The phy pll lock check is
> recommended and can indicate hw failure. Can you please check if
> this untested, uncompiled patch makes it work correctly ?

Your patch changes things.  With this applied on top of 4.15-rc1
it is failing 100% of the time instead of only half of the time.

I brought the original test equipment back to the setup so I can
see the video and pink bar again.  The symptoms remain the same
(unexpected size, pink bar, and no audio).

PS: your patch seems to have been line wrapped which made it a
bit annoying to apply.

> -->8---
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> index bf14214..456fc54 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> @@ -1669,7 +1669,7 @@ static void
> hdmi_disable_overflow_interrupts(struct dw_hdmi *hdmi)
>  
>  static int dw_hdmi_setup(struct dw_hdmi *hdmi, struct
> drm_display_mode *mode)
>  {
> -   int ret;
> +   int ret, vsync_len = mode->vsync_end - mode->vsync_start;
>  
> hdmi_disable_overflow_interrupts(hdmi);
>  
> @@ -1722,6 +1722,14 @@ static int dw_hdmi_setup(struct dw_hdmi
> *hdmi, struct drm_display_mode *mode)
> return ret;
> hdmi->phy.enabled = true;
>  
> +   /* Reset all clock domains */
> +   hdmi_writeb(hdmi, 0x00, HDMI_MC_SWRSTZ);
> +
> +   /* Rewrite vsync register to latch previous written values */
> +   if (mode->flags & DRM_MODE_FLAG_INTERLACE)
> +   vsync_len /= 2;
> +   hdmi_writeb(hdmi, vsync_len, HDMI_FC_VSYNCINWIDTH);
> +
> /* HDMI Initialization Step B.3 */
> dw_hdmi_enable_video_path(hdmi);
>  
> -->8---
>
> I would expect this patch to end your wrong image issue but the
> audio part may be a different problem.

It is very consistent: pink bar <=> no audio.

My suspicion is that the audio problem is just the wrong video mode
on the sink side messing things up, but I have no way of confirming
that (that I know of).

Thanks,
  Nick

Re: PROBLEM: Asus C201 video mode problems on HDMI hotplug (regression)

[Nick Bowler]

Hi Jose,

On 2017-12-02 17:11 +, Jose Abreu wrote:
> On 01-12-2017 00:11, Nick Bowler wrote:
> > Another data point... the following patch appears sufficient to
> > restore working behaviour.
[...]
> I don't think you can do this. The phy pll lock check is
> recommended and can indicate hw failure. Can you please check if
> this untested, uncompiled patch makes it work correctly ?

Your patch changes things.  With this applied on top of 4.15-rc1
it is failing 100% of the time instead of only half of the time.

I brought the original test equipment back to the setup so I can
see the video and pink bar again.  The symptoms remain the same
(unexpected size, pink bar, and no audio).

PS: your patch seems to have been line wrapped which made it a
bit annoying to apply.

> -->8---
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> index bf14214..456fc54 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> @@ -1669,7 +1669,7 @@ static void
> hdmi_disable_overflow_interrupts(struct dw_hdmi *hdmi)
>  
>  static int dw_hdmi_setup(struct dw_hdmi *hdmi, struct
> drm_display_mode *mode)
>  {
> -   int ret;
> +   int ret, vsync_len = mode->vsync_end - mode->vsync_start;
>  
> hdmi_disable_overflow_interrupts(hdmi);
>  
> @@ -1722,6 +1722,14 @@ static int dw_hdmi_setup(struct dw_hdmi
> *hdmi, struct drm_display_mode *mode)
> return ret;
> hdmi->phy.enabled = true;
>  
> +   /* Reset all clock domains */
> +   hdmi_writeb(hdmi, 0x00, HDMI_MC_SWRSTZ);
> +
> +   /* Rewrite vsync register to latch previous written values */
> +   if (mode->flags & DRM_MODE_FLAG_INTERLACE)
> +   vsync_len /= 2;
> +   hdmi_writeb(hdmi, vsync_len, HDMI_FC_VSYNCINWIDTH);
> +
> /* HDMI Initialization Step B.3 */
> dw_hdmi_enable_video_path(hdmi);
>  
> -->8---
>
> I would expect this patch to end your wrong image issue but the
> audio part may be a different problem.

It is very consistent: pink bar <=> no audio.

My suspicion is that the audio problem is just the wrong video mode
on the sink side messing things up, but I have no way of confirming
that (that I know of).

Thanks,
  Nick

Re: netfilter: xt_bpf: Fix XT_BPF_MODE_FD_PINNED mode of 'xt_bpf_info_v1'

[Willem de Bruijn]

>> OK...  See vfs.git#untested.mkobj; it really needs testing, though - 
>> mq_open(2)
>> passes LTP tests, but that's not saying much, and BPF side is completely
>> untested.
>
> ... and FWIW, completely untested patch for net/netfilter/xt_bpf.c follows:

Thanks a lot for this fix.

The tree including the bpf fix passes this basic xt_bpf test:

  mount -t bpf bpf /sys/fs/bpf
  ./pin /sys/fs/bpf/pass
  iptables -A INPUT -m bpf --object-pinned /sys/fs/bpf/five -j LOG
  iptables -L INPUT
  iptables -F INPUT

where pin is as follows:

diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
index adeaa1302f34..0cd2bb8d634b 100644
--- a/samples/bpf/Makefile
+++ b/samples/bpf/Makefile
@@ -41,6 +41,7 @@ hostprogs-y += xdp_redirect_map
 hostprogs-y += xdp_redirect_cpu
 hostprogs-y += xdp_monitor
 hostprogs-y += syscall_tp
+hostprogs-y += pin

 # Libbpf dependencies
 LIBBPF := ../../tools/lib/bpf/bpf.o
@@ -89,6 +90,7 @@ xdp_redirect_map-objs := bpf_load.o $(LIBBPF)
xdp_redirect_map_user.o
 xdp_redirect_cpu-objs := bpf_load.o $(LIBBPF) xdp_redirect_cpu_user.o
 xdp_monitor-objs := bpf_load.o $(LIBBPF) xdp_monitor_user.o
 syscall_tp-objs := bpf_load.o $(LIBBPF) syscall_tp_user.o
+pin-objs := $(LIBBPF) pin.o

 # Tell kbuild to always build the programs
 always := $(hostprogs-y)
diff --git a/samples/bpf/pin.c b/samples/bpf/pin.c
new file mode 100644
index ..826e86784edf
--- /dev/null
+++ b/samples/bpf/pin.c
@@ -0,0 +1,41 @@
+#define _GNU_SOURCE
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "libbpf.h"
+#include "bpf_load.h"
+
+static char log_buf[1 << 16];
+
+int main(int argc, char **argv)
+{
+   struct bpf_insn prog[] = {
+   BPF_MOV64_IMM(BPF_REG_0, 1),
+   BPF_EXIT_INSN(),
+   };
+   int fd;
+
+   if (argc != 2)
+   error(1, 0, "Usage: %s \n", argv[0]);
+
+   fd = bpf_load_program(BPF_PROG_TYPE_SOCKET_FILTER, prog,
+ sizeof(prog) / sizeof(prog[0]),
+ "GPL", 0, log_buf, sizeof(log_buf));
+   if (fd == -1)
+   error(1, errno, "load: %s", log_buf);
+
+   if (bpf_obj_pin(fd, argv[1]))
+   error(1, errno, "pin");
+
+   if (close(fd))
+   error(1, errno, "close");
+
+   return 0;
+}

Re: netfilter: xt_bpf: Fix XT_BPF_MODE_FD_PINNED mode of 'xt_bpf_info_v1'

[Willem de Bruijn]

>> OK...  See vfs.git#untested.mkobj; it really needs testing, though - 
>> mq_open(2)
>> passes LTP tests, but that's not saying much, and BPF side is completely
>> untested.
>
> ... and FWIW, completely untested patch for net/netfilter/xt_bpf.c follows:

Thanks a lot for this fix.

The tree including the bpf fix passes this basic xt_bpf test:

  mount -t bpf bpf /sys/fs/bpf
  ./pin /sys/fs/bpf/pass
  iptables -A INPUT -m bpf --object-pinned /sys/fs/bpf/five -j LOG
  iptables -L INPUT
  iptables -F INPUT

where pin is as follows:

diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
index adeaa1302f34..0cd2bb8d634b 100644
--- a/samples/bpf/Makefile
+++ b/samples/bpf/Makefile
@@ -41,6 +41,7 @@ hostprogs-y += xdp_redirect_map
 hostprogs-y += xdp_redirect_cpu
 hostprogs-y += xdp_monitor
 hostprogs-y += syscall_tp
+hostprogs-y += pin

 # Libbpf dependencies
 LIBBPF := ../../tools/lib/bpf/bpf.o
@@ -89,6 +90,7 @@ xdp_redirect_map-objs := bpf_load.o $(LIBBPF)
xdp_redirect_map_user.o
 xdp_redirect_cpu-objs := bpf_load.o $(LIBBPF) xdp_redirect_cpu_user.o
 xdp_monitor-objs := bpf_load.o $(LIBBPF) xdp_monitor_user.o
 syscall_tp-objs := bpf_load.o $(LIBBPF) syscall_tp_user.o
+pin-objs := $(LIBBPF) pin.o

 # Tell kbuild to always build the programs
 always := $(hostprogs-y)
diff --git a/samples/bpf/pin.c b/samples/bpf/pin.c
new file mode 100644
index ..826e86784edf
--- /dev/null
+++ b/samples/bpf/pin.c
@@ -0,0 +1,41 @@
+#define _GNU_SOURCE
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "libbpf.h"
+#include "bpf_load.h"
+
+static char log_buf[1 << 16];
+
+int main(int argc, char **argv)
+{
+   struct bpf_insn prog[] = {
+   BPF_MOV64_IMM(BPF_REG_0, 1),
+   BPF_EXIT_INSN(),
+   };
+   int fd;
+
+   if (argc != 2)
+   error(1, 0, "Usage: %s \n", argv[0]);
+
+   fd = bpf_load_program(BPF_PROG_TYPE_SOCKET_FILTER, prog,
+ sizeof(prog) / sizeof(prog[0]),
+ "GPL", 0, log_buf, sizeof(log_buf));
+   if (fd == -1)
+   error(1, errno, "load: %s", log_buf);
+
+   if (bpf_obj_pin(fd, argv[1]))
+   error(1, errno, "pin");
+
+   if (close(fd))
+   error(1, errno, "close");
+
+   return 0;
+}

Re: [PATCH] staging: pi433: fix coding style in function rf69_set_rx_cfg()

[kbuild test robot]

Hi Victor,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on staging/staging-testing]
[also build test ERROR on v4.15-rc1 next-20171201]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Victor-Carvalho/staging-pi433-fix-coding-style-in-function-rf69_set_rx_cfg/20171203-112632
config: i386-randconfig-x003-201749 (attached as .config)
compiler: gcc-7 (Debian 7.2.0-12) 7.2.1 20171025
reproduce:
# save the attached .config to linux build tree
make ARCH=i386 

All errors (new ones prefixed by >>):

   drivers/staging/pi433/pi433_if.c: In function 'rf69_set_rx_cfg':
>> drivers/staging/pi433/pi433_if.c:209:2: error: 'else' without a previous 'if'
 else
 ^~~~

vim +209 drivers/staging/pi433/pi433_if.c

874bcba6 Marcus Wolf 2017-07-16  178  
874bcba6 Marcus Wolf 2017-07-16  179  static int
874bcba6 Marcus Wolf 2017-07-16  180  rf69_set_rx_cfg(struct pi433_device 
*dev, struct pi433_rx_cfg *rx_cfg)
874bcba6 Marcus Wolf 2017-07-16  181  {
125a452c Elia Geretto2017-07-31  182int ret;
874bcba6 Marcus Wolf 2017-07-16  183int payload_length;
874bcba6 Marcus Wolf 2017-07-16  184  
874bcba6 Marcus Wolf 2017-07-16  185/* receiver config */
874bcba6 Marcus Wolf 2017-07-16  186
SET_CHECKED(rf69_set_frequency(dev->spi, rx_cfg->frequency));
874bcba6 Marcus Wolf 2017-07-16  187
SET_CHECKED(rf69_set_bit_rate(dev->spi, rx_cfg->bit_rate));
874bcba6 Marcus Wolf 2017-07-16  188
SET_CHECKED(rf69_set_modulation(dev->spi, rx_cfg->modulation));
2c58352f Victor Carvalho 2017-11-30  189
SET_CHECKED(rf69_set_antenna_impedance(dev->spi,
2c58352f Victor Carvalho 2017-11-30  190
   rx_cfg->antenna_impedance));
874bcba6 Marcus Wolf 2017-07-16  191
SET_CHECKED(rf69_set_rssi_threshold(dev->spi, rx_cfg->rssi_threshold));
2c58352f Victor Carvalho 2017-11-30  192
SET_CHECKED(rf69_set_ook_threshold_dec(dev->spi,
2c58352f Victor Carvalho 2017-11-30  193
   rx_cfg->thresholdDecrement));
2c58352f Victor Carvalho 2017-11-30  194
SET_CHECKED(rf69_set_bandwidth(dev->spi, rx_cfg->bw_mantisse,
2c58352f Victor Carvalho 2017-11-30  195   
rx_cfg->bw_exponent));
2c58352f Victor Carvalho 2017-11-30  196
SET_CHECKED(rf69_set_bandwidth_during_afc(dev->spi,
2c58352f Victor Carvalho 2017-11-30  197
  rx_cfg->bw_mantisse,
2c58352f Victor Carvalho 2017-11-30  198
  rx_cfg->bw_exponent));
874bcba6 Marcus Wolf 2017-07-16  199
SET_CHECKED(rf69_set_dagc(dev->spi, rx_cfg->dagc));
874bcba6 Marcus Wolf 2017-07-16  200  
874bcba6 Marcus Wolf 2017-07-16  201dev->rx_bytes_to_drop = 
rx_cfg->bytes_to_drop;
874bcba6 Marcus Wolf 2017-07-16  202  
874bcba6 Marcus Wolf 2017-07-16  203/* packet config */
874bcba6 Marcus Wolf 2017-07-16  204/* enable */
874bcba6 Marcus Wolf 2017-07-16  205
SET_CHECKED(rf69_set_sync_enable(dev->spi, rx_cfg->enable_sync));
874bcba6 Marcus Wolf 2017-07-16  206if (rx_cfg->enable_sync == 
optionOn)
2c58352f Victor Carvalho 2017-11-30  207
SET_CHECKED(rf69_set_fifo_fill_condition(dev->spi,
2c58352f Victor Carvalho 2017-11-30  208
 afterSyncInterrupt));
874bcba6 Marcus Wolf 2017-07-16 @209else
874bcba6 Marcus Wolf 2017-07-16  210
SET_CHECKED(rf69_set_fifo_fill_condition(dev->spi, always));
2c58352f Victor Carvalho 2017-11-30  211  
125a452c Elia Geretto2017-07-31  212if (rx_cfg->enable_length_byte 
== optionOn) {
125a452c Elia Geretto2017-07-31  213ret = 
rf69_set_packet_format(dev->spi, packetLengthVar);
125a452c Elia Geretto2017-07-31  214if (ret < 0)
125a452c Elia Geretto2017-07-31  215return ret;
125a452c Elia Geretto2017-07-31  216} else {
125a452c Elia Geretto2017-07-31  217ret = 
rf69_set_packet_format(dev->spi, packetLengthFix);
125a452c Elia Geretto2017-07-31  218if (ret < 0)
125a452c Elia Geretto2017-07-31  219return ret;
125a452c Elia Geretto2017-07-31  220}
2c58352f Victor Carvalho 2017-11-30  221  
2c58352f Victor Carvalho 2017-11-30  222
SET_CHECKED(rf69_set_adressFiltering
2c58352f Victor Carvalho 2017-11-30  223(dev->spi, 
rx_cfg->enable_address_filtering));
874bcba6 Marcus Wolf 2017-07-16  224
SET_CHECKED(rf69_set_crc_enable(dev->spi, rx_cfg->enable_crc));
874bcba6 Marcus Wolf 2017-07-16  225  
874bcba6 Marcus Wolf 2017-07-16  226

Re: [PATCH] staging: pi433: fix coding style in function rf69_set_rx_cfg()

[kbuild test robot]

Hi Victor,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on staging/staging-testing]
[also build test ERROR on v4.15-rc1 next-20171201]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Victor-Carvalho/staging-pi433-fix-coding-style-in-function-rf69_set_rx_cfg/20171203-112632
config: i386-randconfig-x003-201749 (attached as .config)
compiler: gcc-7 (Debian 7.2.0-12) 7.2.1 20171025
reproduce:
# save the attached .config to linux build tree
make ARCH=i386 

All errors (new ones prefixed by >>):

   drivers/staging/pi433/pi433_if.c: In function 'rf69_set_rx_cfg':
>> drivers/staging/pi433/pi433_if.c:209:2: error: 'else' without a previous 'if'
 else
 ^~~~

vim +209 drivers/staging/pi433/pi433_if.c

874bcba6 Marcus Wolf 2017-07-16  178  
874bcba6 Marcus Wolf 2017-07-16  179  static int
874bcba6 Marcus Wolf 2017-07-16  180  rf69_set_rx_cfg(struct pi433_device 
*dev, struct pi433_rx_cfg *rx_cfg)
874bcba6 Marcus Wolf 2017-07-16  181  {
125a452c Elia Geretto2017-07-31  182int ret;
874bcba6 Marcus Wolf 2017-07-16  183int payload_length;
874bcba6 Marcus Wolf 2017-07-16  184  
874bcba6 Marcus Wolf 2017-07-16  185/* receiver config */
874bcba6 Marcus Wolf 2017-07-16  186
SET_CHECKED(rf69_set_frequency(dev->spi, rx_cfg->frequency));
874bcba6 Marcus Wolf 2017-07-16  187
SET_CHECKED(rf69_set_bit_rate(dev->spi, rx_cfg->bit_rate));
874bcba6 Marcus Wolf 2017-07-16  188
SET_CHECKED(rf69_set_modulation(dev->spi, rx_cfg->modulation));
2c58352f Victor Carvalho 2017-11-30  189
SET_CHECKED(rf69_set_antenna_impedance(dev->spi,
2c58352f Victor Carvalho 2017-11-30  190
   rx_cfg->antenna_impedance));
874bcba6 Marcus Wolf 2017-07-16  191
SET_CHECKED(rf69_set_rssi_threshold(dev->spi, rx_cfg->rssi_threshold));
2c58352f Victor Carvalho 2017-11-30  192
SET_CHECKED(rf69_set_ook_threshold_dec(dev->spi,
2c58352f Victor Carvalho 2017-11-30  193
   rx_cfg->thresholdDecrement));
2c58352f Victor Carvalho 2017-11-30  194
SET_CHECKED(rf69_set_bandwidth(dev->spi, rx_cfg->bw_mantisse,
2c58352f Victor Carvalho 2017-11-30  195   
rx_cfg->bw_exponent));
2c58352f Victor Carvalho 2017-11-30  196
SET_CHECKED(rf69_set_bandwidth_during_afc(dev->spi,
2c58352f Victor Carvalho 2017-11-30  197
  rx_cfg->bw_mantisse,
2c58352f Victor Carvalho 2017-11-30  198
  rx_cfg->bw_exponent));
874bcba6 Marcus Wolf 2017-07-16  199
SET_CHECKED(rf69_set_dagc(dev->spi, rx_cfg->dagc));
874bcba6 Marcus Wolf 2017-07-16  200  
874bcba6 Marcus Wolf 2017-07-16  201dev->rx_bytes_to_drop = 
rx_cfg->bytes_to_drop;
874bcba6 Marcus Wolf 2017-07-16  202  
874bcba6 Marcus Wolf 2017-07-16  203/* packet config */
874bcba6 Marcus Wolf 2017-07-16  204/* enable */
874bcba6 Marcus Wolf 2017-07-16  205
SET_CHECKED(rf69_set_sync_enable(dev->spi, rx_cfg->enable_sync));
874bcba6 Marcus Wolf 2017-07-16  206if (rx_cfg->enable_sync == 
optionOn)
2c58352f Victor Carvalho 2017-11-30  207
SET_CHECKED(rf69_set_fifo_fill_condition(dev->spi,
2c58352f Victor Carvalho 2017-11-30  208
 afterSyncInterrupt));
874bcba6 Marcus Wolf 2017-07-16 @209else
874bcba6 Marcus Wolf 2017-07-16  210
SET_CHECKED(rf69_set_fifo_fill_condition(dev->spi, always));
2c58352f Victor Carvalho 2017-11-30  211  
125a452c Elia Geretto2017-07-31  212if (rx_cfg->enable_length_byte 
== optionOn) {
125a452c Elia Geretto2017-07-31  213ret = 
rf69_set_packet_format(dev->spi, packetLengthVar);
125a452c Elia Geretto2017-07-31  214if (ret < 0)
125a452c Elia Geretto2017-07-31  215return ret;
125a452c Elia Geretto2017-07-31  216} else {
125a452c Elia Geretto2017-07-31  217ret = 
rf69_set_packet_format(dev->spi, packetLengthFix);
125a452c Elia Geretto2017-07-31  218if (ret < 0)
125a452c Elia Geretto2017-07-31  219return ret;
125a452c Elia Geretto2017-07-31  220}
2c58352f Victor Carvalho 2017-11-30  221  
2c58352f Victor Carvalho 2017-11-30  222
SET_CHECKED(rf69_set_adressFiltering
2c58352f Victor Carvalho 2017-11-30  223(dev->spi, 
rx_cfg->enable_address_filtering));
874bcba6 Marcus Wolf 2017-07-16  224
SET_CHECKED(rf69_set_crc_enable(dev->spi, rx_cfg->enable_crc));
874bcba6 Marcus Wolf 2017-07-16  225  
874bcba6 Marcus Wolf 2017-07-16  226

Re: [PATCH] mmap.2: MAP_FIXED is no longer discouraged

[John Hubbard]

On 12/02/2017 02:19 PM, Matthew Wilcox wrote:
> On Sat, Dec 02, 2017 at 07:49:20PM +0100, Jann Horn wrote:
>> On Sat, Dec 2, 2017 at 4:05 PM, Matthew Wilcox  wrote:
>>> On Fri, Dec 01, 2017 at 06:16:26PM -0800, john.hubb...@gmail.com wrote:
[...]
> 
> Maybe that should be up front rather than buried at the end of the sentence.
> 
> "In a multi-threaded process, the address space can change in response to
> virtually any library call.  This is because almost any library call may be
> implemented by using dlopen(3) to load another shared library, which will be
> mapped into the process's address space.  The PAM libraries are an excellent
> example, as well as more obvious examples like brk(2), malloc(3) and even
> pthread_create(3)."
> 
> What do you think?
> 

Hi Matthew,

Here is a new version, based on your and Jann's comments. I also added a
reference to MAP_FIXED_SAFE. If it looks close, I'll send a v2 with proper
formatting applied.

I did wonder briefly if your ATM reference was a oblique commentary about
security, but then realized...you probably just needed some cash. :)

-

This option is extremely hazardous (when used on its own) and moderately
non-portable.

On portability: a process's memory map may change significantly from one
run to the next, depending on library versions, kernel versions and ran‐
dom numbers.

On hazards: this option forcibly removes pre-existing  mappings,  making
it easy for a multi-threaded process to corrupt its own address space.

For  example,  thread  A  looks  through /proc//maps and locates an
available address range, while thread B simultaneously acquires part  or
all  of  that  same  address range. Thread A then calls mmap(MAP_FIXED),
effectively overwriting thread B's mapping.

Thread B need not create a mapping directly;  simply  making  a  library
call that, internally, uses dlopen(3) to load some other shared library,
will suffice. The dlopen(3) call will map the library into the process's
address  space.  Furthermore, almost any library call may be implemented
using this technique.  Examples include brk(2), malloc(3),  pthread_cre‐
ate(3), and the PAM libraries (http://www.linux-pam.org).

Given the above limitations, one of the very few ways to use this option
safely is: mmap() a region, without specifying MAP_FIXED.  Then,  within
that  region,  call  mmap(MAP_FIXED) to suballocate regions. This avoids
both the portability problem (because the first mmap call lets the  ker‐
nel pick the address), and the address space corruption problem (because
the region being overwritten is already owned by the calling thread).

Newer kernels (Linux 4.16 and later) have a MAP_FIXED_SAFE  option  that
avoids  the  corruption  problem; if available, MAP_FIXED_SAFE should be
preferred over MAP_FIXED.


thanks,
John Hubbard
NVIDIA

Re: [PATCH] mmap.2: MAP_FIXED is no longer discouraged

[John Hubbard]

On 12/02/2017 02:19 PM, Matthew Wilcox wrote:
> On Sat, Dec 02, 2017 at 07:49:20PM +0100, Jann Horn wrote:
>> On Sat, Dec 2, 2017 at 4:05 PM, Matthew Wilcox  wrote:
>>> On Fri, Dec 01, 2017 at 06:16:26PM -0800, john.hubb...@gmail.com wrote:
[...]
> 
> Maybe that should be up front rather than buried at the end of the sentence.
> 
> "In a multi-threaded process, the address space can change in response to
> virtually any library call.  This is because almost any library call may be
> implemented by using dlopen(3) to load another shared library, which will be
> mapped into the process's address space.  The PAM libraries are an excellent
> example, as well as more obvious examples like brk(2), malloc(3) and even
> pthread_create(3)."
> 
> What do you think?
> 

Hi Matthew,

Here is a new version, based on your and Jann's comments. I also added a
reference to MAP_FIXED_SAFE. If it looks close, I'll send a v2 with proper
formatting applied.

I did wonder briefly if your ATM reference was a oblique commentary about
security, but then realized...you probably just needed some cash. :)

-

This option is extremely hazardous (when used on its own) and moderately
non-portable.

On portability: a process's memory map may change significantly from one
run to the next, depending on library versions, kernel versions and ran‐
dom numbers.

On hazards: this option forcibly removes pre-existing  mappings,  making
it easy for a multi-threaded process to corrupt its own address space.

For  example,  thread  A  looks  through /proc//maps and locates an
available address range, while thread B simultaneously acquires part  or
all  of  that  same  address range. Thread A then calls mmap(MAP_FIXED),
effectively overwriting thread B's mapping.

Thread B need not create a mapping directly;  simply  making  a  library
call that, internally, uses dlopen(3) to load some other shared library,
will suffice. The dlopen(3) call will map the library into the process's
address  space.  Furthermore, almost any library call may be implemented
using this technique.  Examples include brk(2), malloc(3),  pthread_cre‐
ate(3), and the PAM libraries (http://www.linux-pam.org).

Given the above limitations, one of the very few ways to use this option
safely is: mmap() a region, without specifying MAP_FIXED.  Then,  within
that  region,  call  mmap(MAP_FIXED) to suballocate regions. This avoids
both the portability problem (because the first mmap call lets the  ker‐
nel pick the address), and the address space corruption problem (because
the region being overwritten is already owned by the calling thread).

Newer kernels (Linux 4.16 and later) have a MAP_FIXED_SAFE  option  that
avoids  the  corruption  problem; if available, MAP_FIXED_SAFE should be
preferred over MAP_FIXED.


thanks,
John Hubbard
NVIDIA

Re: [PATCH 6/6] ARM: configs: keystone_defconfig: Enable few peripheral drivers

[santosh.shilim...@oracle.com]

On 11/22/17 11:51 PM, Vignesh R wrote:

Enable drivers for QSPI, LEDS, gpio-decoder that are present on 66AK2G EVM
and 66AK2G ICE boards.

Signed-off-by: Vignesh R 
---

Please submit a patch also to enable all these peripherals
in multi-v7 config. Just enable all remainder
options enabled in keystone config also in multi-v7.

Am pushing this series to next now.

Regards,
Santosh

Re: [PATCH 6/6] ARM: configs: keystone_defconfig: Enable few peripheral drivers

[santosh.shilim...@oracle.com]

On 11/22/17 11:51 PM, Vignesh R wrote:

Enable drivers for QSPI, LEDS, gpio-decoder that are present on 66AK2G EVM
and 66AK2G ICE boards.

Signed-off-by: Vignesh R 
---

Please submit a patch also to enable all these peripherals
in multi-v7 config. Just enable all remainder
options enabled in keystone config also in multi-v7.

Am pushing this series to next now.

Regards,
Santosh

Re: [PATCH net-next v4 2/2] net: ethernet: socionext: add AVE ethernet driver

[kbuild test robot]

Hi Kunihiko,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on net-next/master]

url:
https://github.com/0day-ci/linux/commits/Kunihiko-Hayashi/dt-bindings-net-add-DT-bindings-for-Socionext-UniPhier-AVE/20171203-095248
config: alpha-allyesconfig (attached as .config)
compiler: alpha-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=alpha 

All warnings (new ones prefixed by >>):

   drivers/net/ethernet/socionext/sni_ave.c: In function 
'ave_pfsel_set_promisc':
>> drivers/net/ethernet/socionext/sni_ave.c:172:27: warning: large integer 
>> implicitly truncated to unsigned type [-Woverflow]
#define AVE_PFMBYTE_MASK0 (~GENMASK(7, 6))
  ^
>> drivers/net/ethernet/socionext/sni_ave.c:1046:9: note: in expansion of macro 
>> 'AVE_PFMBYTE_MASK0'
 writel(AVE_PFMBYTE_MASK0, priv->base + AVE_PFMBYTE(entry));
^

vim +172 drivers/net/ethernet/socionext/sni_ave.c

   170  
   171  /* Packet filter */
 > 172  #define AVE_PFMBYTE_MASK0   (~GENMASK(7, 6))
   173  #define AVE_PFMBYTE_MASK1   GENMASK(25, 0)
   174  #define AVE_PFMBIT_MASK GENMASK(15, 0)
   175  

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


.config.gz
Description: application/gzip

Re: [PATCH net-next v4 2/2] net: ethernet: socionext: add AVE ethernet driver

[kbuild test robot]

Hi Kunihiko,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on net-next/master]

url:
https://github.com/0day-ci/linux/commits/Kunihiko-Hayashi/dt-bindings-net-add-DT-bindings-for-Socionext-UniPhier-AVE/20171203-095248
config: alpha-allyesconfig (attached as .config)
compiler: alpha-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=alpha 

All warnings (new ones prefixed by >>):

   drivers/net/ethernet/socionext/sni_ave.c: In function 
'ave_pfsel_set_promisc':
>> drivers/net/ethernet/socionext/sni_ave.c:172:27: warning: large integer 
>> implicitly truncated to unsigned type [-Woverflow]
#define AVE_PFMBYTE_MASK0 (~GENMASK(7, 6))
  ^
>> drivers/net/ethernet/socionext/sni_ave.c:1046:9: note: in expansion of macro 
>> 'AVE_PFMBYTE_MASK0'
 writel(AVE_PFMBYTE_MASK0, priv->base + AVE_PFMBYTE(entry));
^

vim +172 drivers/net/ethernet/socionext/sni_ave.c

   170  
   171  /* Packet filter */
 > 172  #define AVE_PFMBYTE_MASK0   (~GENMASK(7, 6))
   173  #define AVE_PFMBYTE_MASK1   GENMASK(25, 0)
   174  #define AVE_PFMBIT_MASK GENMASK(15, 0)
   175  

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


.config.gz
Description: application/gzip

Re: New ORC unwinder in 4.14 broke cross-compilation?

[Randy Dunlap]

On 11/13/2017 10:33 AM, Andrew Randrianasulu wrote:
> Hello!
> 
> I was building my new 4.14 kernel, and on first instance I got working kernel 
> + 
> modules, booted ok, found my fb console was missing, recompiled kernel with 
> framebuffer console built-in (it resetted itself from M to N, because I was 
> using menuconfig on .config from 4.12 where I had fbcon = M). After reboot 
> into 
> new kernel everything works, so far. But if I select new ORC unwinder under 
> kernel hacking submenu - I get compilation error early:
> 
> guest@slax:/dev/shm/src/linux-2.6$ LANG=C make ARCH=x86_64 
> CROSS_COMPILE=x86_64-unknown-linux-gnu-
>   CHK include/config/kernel.release
>   CHK include/generated/uapi/linux/version.h
>   CHK include/generated/utsrelease.h
>   CHK include/generated/bounds.h
>   CHK include/generated/timeconst.h
>   CHK include/generated/asm-offsets.h
>   CALLscripts/checksyscalls.sh
>   DESCEND  objtool
>   CC   /dev/shm/src/linux-2.6/tools/objtool/orc_dump.o
> orc_dump.c: In function 'orc_dump':
> orc_dump.c:105:2: error: passing argument 2 of 'elf_getshdrnum' from 
> incompatible pointer type [-Werror]
>   if (elf_getshdrnum(elf, _sections)) {
>   ^
> In file included from /usr/include/gelf.h:32:0,
>  from elf.h:22,
>  from warn.h:26,
>  from orc_dump.c:20:
> /usr/include/libelf.h:244:12: note: expected 'size_t *' but argument is of 
> type 'long unsigned int *'
>  extern int elf_getshdrnum (Elf *__elf, size_t *__dst);
> ^
> orc_dump.c:190:4: error: format '%lx' expects argument of type 'long unsigned 
> int', but argument 3 has type 'Elf64_Sxword' [-Werror=format=]
> printf("%s+%lx:", name, rela.r_addend);
> ^
> cc1: all warnings being treated as errors
> mv: cannot stat '/dev/shm/src/linux-2.6/tools/objtool/.orc_dump.o.tmp': No 
> such 
> file or directory
> make[3]: *** [/dev/shm/src/linux-2.6/tools/objtool/orc_dump.o] Error 1
> make[2]: *** [/dev/shm/src/linux-2.6/tools/objtool/objtool-in.o] Error 2
> make[1]: *** [objtool] Error 2
> make: *** [tools/objtool] Error 2
> 
> 
> --
> 
> I try to cross-compile from 32-bit slackware with self-built minimal 
> cross-compiler:
> 
> LANG=C /opt/kgcc64/bin/x86_64-unknown-linux-gnu-gcc --version
> x86_64-unknown-linux-gnu-gcc (GCC) 4.9.2
> Copyright (C) 2014 Free Software Foundation, Inc.
> This is free software; see the source for copying conditions.  There is NO
> warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
> 
> I basically just followed steps from 
> https://www.linux-mips.org/wiki/Toolchains 
> but replaced mips with x86_64 . It worked on many kernels for long time. This 
> new kernel first complained about missing gelf.h include, and after 
> installing 
> elfutils-0.163-i586-1 (compiled on 32-bit slackware) I now have this error. 
> Problem is, even if I unselect ORC unwinder and choose Frame-pointer based 
> unwinder again - error remain, at least after make clean.
> 
> I usually monitor LKML via web-interface, but please CC me just in case I 
> miss 
> my answers. Attached .config after changing unwinder back to frame-pointer 
> based.

Hi,
Please try the patch that was posted today:
https://marc.info/?l=linux-kernel=151225312110866=2



-- 
~Randy

Re: New ORC unwinder in 4.14 broke cross-compilation?

[Randy Dunlap]

On 11/13/2017 10:33 AM, Andrew Randrianasulu wrote:
> Hello!
> 
> I was building my new 4.14 kernel, and on first instance I got working kernel 
> + 
> modules, booted ok, found my fb console was missing, recompiled kernel with 
> framebuffer console built-in (it resetted itself from M to N, because I was 
> using menuconfig on .config from 4.12 where I had fbcon = M). After reboot 
> into 
> new kernel everything works, so far. But if I select new ORC unwinder under 
> kernel hacking submenu - I get compilation error early:
> 
> guest@slax:/dev/shm/src/linux-2.6$ LANG=C make ARCH=x86_64 
> CROSS_COMPILE=x86_64-unknown-linux-gnu-
>   CHK include/config/kernel.release
>   CHK include/generated/uapi/linux/version.h
>   CHK include/generated/utsrelease.h
>   CHK include/generated/bounds.h
>   CHK include/generated/timeconst.h
>   CHK include/generated/asm-offsets.h
>   CALLscripts/checksyscalls.sh
>   DESCEND  objtool
>   CC   /dev/shm/src/linux-2.6/tools/objtool/orc_dump.o
> orc_dump.c: In function 'orc_dump':
> orc_dump.c:105:2: error: passing argument 2 of 'elf_getshdrnum' from 
> incompatible pointer type [-Werror]
>   if (elf_getshdrnum(elf, _sections)) {
>   ^
> In file included from /usr/include/gelf.h:32:0,
>  from elf.h:22,
>  from warn.h:26,
>  from orc_dump.c:20:
> /usr/include/libelf.h:244:12: note: expected 'size_t *' but argument is of 
> type 'long unsigned int *'
>  extern int elf_getshdrnum (Elf *__elf, size_t *__dst);
> ^
> orc_dump.c:190:4: error: format '%lx' expects argument of type 'long unsigned 
> int', but argument 3 has type 'Elf64_Sxword' [-Werror=format=]
> printf("%s+%lx:", name, rela.r_addend);
> ^
> cc1: all warnings being treated as errors
> mv: cannot stat '/dev/shm/src/linux-2.6/tools/objtool/.orc_dump.o.tmp': No 
> such 
> file or directory
> make[3]: *** [/dev/shm/src/linux-2.6/tools/objtool/orc_dump.o] Error 1
> make[2]: *** [/dev/shm/src/linux-2.6/tools/objtool/objtool-in.o] Error 2
> make[1]: *** [objtool] Error 2
> make: *** [tools/objtool] Error 2
> 
> 
> --
> 
> I try to cross-compile from 32-bit slackware with self-built minimal 
> cross-compiler:
> 
> LANG=C /opt/kgcc64/bin/x86_64-unknown-linux-gnu-gcc --version
> x86_64-unknown-linux-gnu-gcc (GCC) 4.9.2
> Copyright (C) 2014 Free Software Foundation, Inc.
> This is free software; see the source for copying conditions.  There is NO
> warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
> 
> I basically just followed steps from 
> https://www.linux-mips.org/wiki/Toolchains 
> but replaced mips with x86_64 . It worked on many kernels for long time. This 
> new kernel first complained about missing gelf.h include, and after 
> installing 
> elfutils-0.163-i586-1 (compiled on 32-bit slackware) I now have this error. 
> Problem is, even if I unselect ORC unwinder and choose Frame-pointer based 
> unwinder again - error remain, at least after make clean.
> 
> I usually monitor LKML via web-interface, but please CC me just in case I 
> miss 
> my answers. Attached .config after changing unwinder back to frame-pointer 
> based.

Hi,
Please try the patch that was posted today:
https://marc.info/?l=linux-kernel=151225312110866=2



-- 
~Randy

Re: [GIT PULL] RISC-V Cleanups and ABI Fixes for 4.15-rc2

[Andrea Parri]

Hi Palmer,

On Fri, Dec 01, 2017 at 01:39:12PM -0800, Palmer Dabbelt wrote:
> The following changes since commit 4fbd8d194f06c8a3fd2af1ce560ddb31f7ec8323:
> 
>   Linux 4.15-rc1 (2017-11-26 16:01:47 -0800)
> 
> are available in the git repository at:
> 
>   ssh://g...@gitolite.kernel.org/pub/scm/linux/kernel/git/palmer/linux.git 
> tags/riscv-for-linus-4.15-rc2_cleanups
> 
> for you to fetch changes up to 3b62de26cf5ef17340a0e986d3e53eb4f74f96d5:
> 
>   RISC-V: Fixes for clean allmodconfig build (2017-12-01 13:31:31 -0800)
> 
> 
> RISC-V Cleanups and ABI Fixes for 4.15-rc2
> 
> This tag contains a handful of small cleanups that are a result of
> feedback that didn't make it into our original patch set, either because
> the feedback hadn't been given yet, I missed the original emails, or
> we weren't ready to submit the changes yet.
> 
> I've been maintaining the various cleanup patch sets I have as their own
> branches, which I then merged together and signed.  Each merge commit
> has a short summary of the changes, and each branch is based on your
> latest tag (4.15-rc1, in this case).  If this isn't the right way to do
> this then feel free to suggest something else, but it seems sane to me.
> 
> Here's a short summary of the changes, roughly in order of how
> interesting they are.
> 
> * libgcc.h has been moved from include/lib, where it's the only member,
>   to include/linux.  This is meant to avoid tab completion conflicts.
> * VDSO entries for clock_get/gettimeofday/getcpu have been added.  These
>   are simple syscalls now, but we want to let glibc use them from the
>   start so we can make them faster later.
> * A VDSO entry for instruction cache flushing has been added so
>   userspace can flush the instruction cache.
> * The VDSO symbol versions for __vdso_cmpxchg{32,64} have been removed,
>   as those VDSO entries don't actually exist.
> * __io_writes has been corrected to respect the given type.
> * A new READ_ONCE in arch_spin_is_locked().
> * __test_and_op_bit_ord() is now actually ordered.
> * Various small fixes throughout the tree to enable allmodconfig to
>   build cleanly.
> * Removal of some dead code in our atomic support headers.
> * Improvements to various comments in our atomic support headers.
> 
> 
> Andrew Waterman (3):
>   RISC-V: Add VDSO entries for clock_get/gettimeofday/getcpu
>   RISC-V: Flush I$ when making a dirty page executable
>   RISC-V: Allow userspace to flush the instruction cache
> 
> Christoph Hellwig (1):
>   move libgcc.h to include/linux
> 
> Olof Johansson (8):
>   RISC-V: use generic serial.h
>   RISC-V: use RISCV_{INT,SHORT} instead of {INT,SHORT} for asm macros
>   RISC-V: io.h: type fixes for warnings
>   RISC-V: move empty_zero_page definition to C and export it
>   RISC-V: Export some expected symbols for modules
>   RISC-V: Provide stub of setup_profiling_timer()
>   RISC-V: Use define for get_cycles like other architectures
>   RISC-V: Add missing include
> 
> Palmer Dabbelt (16):
>   RISC-V: Remove __vdso_cmpxchg{32,64} symbol versions
>   RISC-V: Remove unused arguments from ATOMIC_OP
>   RISC-V: Comment on why {,cmp}xchg is ordered how it is
>   RISC-V: Remove __smp_bp__{before,after}_atomic
>   RISC-V: Remove smb_mb__{before,after}_spinlock()

I wonder whether you really meant to remove smp_mb__after_spinlock():
on the one hand, this primitive doesn't seem "obsolete" (as suggested
by the commit message); on the other hand, the Draft Specification at

  https://marc.info/?l=linux-kernel=151218405830993=2

suggests that you need "to strengthen" the generic implementation for
this primitive (considered the current spinlock.h in riscv).  What am
I missing?

  Andrea


>   RISC-V: __test_and_op_bit_ord should be strongly ordered
>   RISC-V: Add READ_ONCE in arch_spin_is_locked()
>   RISC-V: `sfence.vma` orderes the instruction cache
>   RISC-V: remove spin_unlock_wait()
>   RISC-V: Clean up an unused include
>   RISC-V: __io_writes should respect the length argument
>   RISC-V Atomic Cleanups
>   RISC-V: User-Visible Changes
>   RISC-V: __io_writes should respect the length argument
>   move libgcc.h to include/linux
>   RISC-V: Fixes for clean allmodconfig build
> 
>  arch/riscv/include/asm/Kbuild  |   1 +
>  arch/riscv/include/asm/asm.h   |  12 ++--
>  arch/riscv/include/asm/atomic.h| 103 
> +
>  arch/riscv/include/asm/barrier.h   |  23 
>  arch/riscv/include/asm/bitops.h|   2 +-
>  arch/riscv/include/asm/bug.h   |   6 +-
>  arch/riscv/include/asm/cacheflush.h|  30 --
>  arch/riscv/include/asm/io.h|  18 +++---
>  arch/riscv/include/asm/mmu.h   |   4 ++
>  arch/riscv/include/asm/mmu_context.h   |  45 

Re: [GIT PULL] RISC-V Cleanups and ABI Fixes for 4.15-rc2

[Andrea Parri]

Hi Palmer,

On Fri, Dec 01, 2017 at 01:39:12PM -0800, Palmer Dabbelt wrote:
> The following changes since commit 4fbd8d194f06c8a3fd2af1ce560ddb31f7ec8323:
> 
>   Linux 4.15-rc1 (2017-11-26 16:01:47 -0800)
> 
> are available in the git repository at:
> 
>   ssh://g...@gitolite.kernel.org/pub/scm/linux/kernel/git/palmer/linux.git 
> tags/riscv-for-linus-4.15-rc2_cleanups
> 
> for you to fetch changes up to 3b62de26cf5ef17340a0e986d3e53eb4f74f96d5:
> 
>   RISC-V: Fixes for clean allmodconfig build (2017-12-01 13:31:31 -0800)
> 
> 
> RISC-V Cleanups and ABI Fixes for 4.15-rc2
> 
> This tag contains a handful of small cleanups that are a result of
> feedback that didn't make it into our original patch set, either because
> the feedback hadn't been given yet, I missed the original emails, or
> we weren't ready to submit the changes yet.
> 
> I've been maintaining the various cleanup patch sets I have as their own
> branches, which I then merged together and signed.  Each merge commit
> has a short summary of the changes, and each branch is based on your
> latest tag (4.15-rc1, in this case).  If this isn't the right way to do
> this then feel free to suggest something else, but it seems sane to me.
> 
> Here's a short summary of the changes, roughly in order of how
> interesting they are.
> 
> * libgcc.h has been moved from include/lib, where it's the only member,
>   to include/linux.  This is meant to avoid tab completion conflicts.
> * VDSO entries for clock_get/gettimeofday/getcpu have been added.  These
>   are simple syscalls now, but we want to let glibc use them from the
>   start so we can make them faster later.
> * A VDSO entry for instruction cache flushing has been added so
>   userspace can flush the instruction cache.
> * The VDSO symbol versions for __vdso_cmpxchg{32,64} have been removed,
>   as those VDSO entries don't actually exist.
> * __io_writes has been corrected to respect the given type.
> * A new READ_ONCE in arch_spin_is_locked().
> * __test_and_op_bit_ord() is now actually ordered.
> * Various small fixes throughout the tree to enable allmodconfig to
>   build cleanly.
> * Removal of some dead code in our atomic support headers.
> * Improvements to various comments in our atomic support headers.
> 
> 
> Andrew Waterman (3):
>   RISC-V: Add VDSO entries for clock_get/gettimeofday/getcpu
>   RISC-V: Flush I$ when making a dirty page executable
>   RISC-V: Allow userspace to flush the instruction cache
> 
> Christoph Hellwig (1):
>   move libgcc.h to include/linux
> 
> Olof Johansson (8):
>   RISC-V: use generic serial.h
>   RISC-V: use RISCV_{INT,SHORT} instead of {INT,SHORT} for asm macros
>   RISC-V: io.h: type fixes for warnings
>   RISC-V: move empty_zero_page definition to C and export it
>   RISC-V: Export some expected symbols for modules
>   RISC-V: Provide stub of setup_profiling_timer()
>   RISC-V: Use define for get_cycles like other architectures
>   RISC-V: Add missing include
> 
> Palmer Dabbelt (16):
>   RISC-V: Remove __vdso_cmpxchg{32,64} symbol versions
>   RISC-V: Remove unused arguments from ATOMIC_OP
>   RISC-V: Comment on why {,cmp}xchg is ordered how it is
>   RISC-V: Remove __smp_bp__{before,after}_atomic
>   RISC-V: Remove smb_mb__{before,after}_spinlock()

I wonder whether you really meant to remove smp_mb__after_spinlock():
on the one hand, this primitive doesn't seem "obsolete" (as suggested
by the commit message); on the other hand, the Draft Specification at

  https://marc.info/?l=linux-kernel=151218405830993=2

suggests that you need "to strengthen" the generic implementation for
this primitive (considered the current spinlock.h in riscv).  What am
I missing?

  Andrea


>   RISC-V: __test_and_op_bit_ord should be strongly ordered
>   RISC-V: Add READ_ONCE in arch_spin_is_locked()
>   RISC-V: `sfence.vma` orderes the instruction cache
>   RISC-V: remove spin_unlock_wait()
>   RISC-V: Clean up an unused include
>   RISC-V: __io_writes should respect the length argument
>   RISC-V Atomic Cleanups
>   RISC-V: User-Visible Changes
>   RISC-V: __io_writes should respect the length argument
>   move libgcc.h to include/linux
>   RISC-V: Fixes for clean allmodconfig build
> 
>  arch/riscv/include/asm/Kbuild  |   1 +
>  arch/riscv/include/asm/asm.h   |  12 ++--
>  arch/riscv/include/asm/atomic.h| 103 
> +
>  arch/riscv/include/asm/barrier.h   |  23 
>  arch/riscv/include/asm/bitops.h|   2 +-
>  arch/riscv/include/asm/bug.h   |   6 +-
>  arch/riscv/include/asm/cacheflush.h|  30 --
>  arch/riscv/include/asm/io.h|  18 +++---
>  arch/riscv/include/asm/mmu.h   |   4 ++
>  arch/riscv/include/asm/mmu_context.h   |  45 

Re: [PATCH RFC] stat.2: Document that stat can fail with EINTR

[Keno Fischer]

Resending as plain text (apologies for those receiving it twice, and
those that got
an HTML copy, I'm used to my mail client switching that over
automatically, which
for some reason didn't happen here).


This is exactly the discussion I want to generate, so thank you.
I should point out that I'm not advocating for anything other
than clarity of what kernel behavior user space may assume.


On Sat, Dec 2, 2017 at 9:25 PM, Matthew Wilcox  wrote:
> On Sat, Dec 02, 2017 at 07:23:59PM -0500, Keno Fischer wrote:
>> The catalyst for this patch was me experiencing EINTR errors when
>> using the 9p file system. In linux commit 9523feac, the 9p file
>> system was changed to use wait_event_killable instead of
>> wait_event_interruptible, which does indeed address my problem,
>> but also makes me a bit unhappy, because uninterruptable waits
>> prevents things like ^C'ing the execution and some debugging
>> tools which depend on being able to cancel long-running operations
>> by sending signals.
>
> Wait, wait, wait.  killable is not uninterruptible.  It's "can accept
> a signal if the signal is fatal".  ie userspace will never see it.
> So, no, it doesn't prevent ^C.  It does prevent the debugging tool you're
> talking about from working, because it's handling the signal, so it's not
> fatal.

This probably shows that I've been in REPL based environments too long,
that catch SIGINT ;). You are of course correct that a fatal SIGINT would
still be delivered.

>> I realize I'm probably 20 years too late here, but it feels like
>> clarificaion on what to expect from the kernel would still go a long
>> way here.
>
> A change to user-visible behaviour has to be opt-in.

I agree. However, it was my impression that stat() can return EINTR
depending on the file system. Prior to the referenced commit,
this was certainly true on 9p and I suspect it's not the only network file
system for which this is true (though prior to my experiencing this
with 9p, the only
time I've ever experienced it was on HPC clusters with who knows what
code providing the network filesystem). If it is indeed the case that
an EINTR return from stat() and similar is illegal and should be considered
a kernel bug, a statement to that extent all I'm looking for here.

Re: [PATCH RFC] stat.2: Document that stat can fail with EINTR

[Keno Fischer]

Resending as plain text (apologies for those receiving it twice, and
those that got
an HTML copy, I'm used to my mail client switching that over
automatically, which
for some reason didn't happen here).


This is exactly the discussion I want to generate, so thank you.
I should point out that I'm not advocating for anything other
than clarity of what kernel behavior user space may assume.


On Sat, Dec 2, 2017 at 9:25 PM, Matthew Wilcox  wrote:
> On Sat, Dec 02, 2017 at 07:23:59PM -0500, Keno Fischer wrote:
>> The catalyst for this patch was me experiencing EINTR errors when
>> using the 9p file system. In linux commit 9523feac, the 9p file
>> system was changed to use wait_event_killable instead of
>> wait_event_interruptible, which does indeed address my problem,
>> but also makes me a bit unhappy, because uninterruptable waits
>> prevents things like ^C'ing the execution and some debugging
>> tools which depend on being able to cancel long-running operations
>> by sending signals.
>
> Wait, wait, wait.  killable is not uninterruptible.  It's "can accept
> a signal if the signal is fatal".  ie userspace will never see it.
> So, no, it doesn't prevent ^C.  It does prevent the debugging tool you're
> talking about from working, because it's handling the signal, so it's not
> fatal.

This probably shows that I've been in REPL based environments too long,
that catch SIGINT ;). You are of course correct that a fatal SIGINT would
still be delivered.

>> I realize I'm probably 20 years too late here, but it feels like
>> clarificaion on what to expect from the kernel would still go a long
>> way here.
>
> A change to user-visible behaviour has to be opt-in.

I agree. However, it was my impression that stat() can return EINTR
depending on the file system. Prior to the referenced commit,
this was certainly true on 9p and I suspect it's not the only network file
system for which this is true (though prior to my experiencing this
with 9p, the only
time I've ever experienced it was on HPC clusters with who knows what
code providing the network filesystem). If it is indeed the case that
an EINTR return from stat() and similar is illegal and should be considered
a kernel bug, a statement to that extent all I'm looking for here.

[PATCH] bluetooth: hci_ll: remove \n from kernel messages

[David Lechner]

The bt_* printk macros include a \n already, so we don't need extra ones
here.

Signed-off-by: David Lechner 
---
 drivers/bluetooth/hci_ll.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/bluetooth/hci_ll.c b/drivers/bluetooth/hci_ll.c
index 42142be..0f0a3a2 100644
--- a/drivers/bluetooth/hci_ll.c
+++ b/drivers/bluetooth/hci_ll.c
@@ -628,11 +628,11 @@ static int download_firmware(struct ll_device *lldev)
break;
}
if (cmd->prefix != 1)
-   bt_dev_dbg(lldev->hu.hdev, "command type %d\n", 
cmd->prefix);
+   bt_dev_dbg(lldev->hu.hdev, "command type %d", 
cmd->prefix);
 
skb = __hci_cmd_sync(lldev->hu.hdev, cmd->opcode, 
cmd->plen, >speed, HCI_INIT_TIMEOUT);
if (IS_ERR(skb)) {
-   bt_dev_err(lldev->hu.hdev, "send command 
failed\n");
+   bt_dev_err(lldev->hu.hdev, "send command 
failed");
err = PTR_ERR(skb);
goto out_rel_fw;
}
-- 
2.7.4

[PATCH] bluetooth: hci_ll: remove \n from kernel messages

[David Lechner]

The bt_* printk macros include a \n already, so we don't need extra ones
here.

Signed-off-by: David Lechner 
---
 drivers/bluetooth/hci_ll.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/bluetooth/hci_ll.c b/drivers/bluetooth/hci_ll.c
index 42142be..0f0a3a2 100644
--- a/drivers/bluetooth/hci_ll.c
+++ b/drivers/bluetooth/hci_ll.c
@@ -628,11 +628,11 @@ static int download_firmware(struct ll_device *lldev)
break;
}
if (cmd->prefix != 1)
-   bt_dev_dbg(lldev->hu.hdev, "command type %d\n", 
cmd->prefix);
+   bt_dev_dbg(lldev->hu.hdev, "command type %d", 
cmd->prefix);
 
skb = __hci_cmd_sync(lldev->hu.hdev, cmd->opcode, 
cmd->plen, >speed, HCI_INIT_TIMEOUT);
if (IS_ERR(skb)) {
-   bt_dev_err(lldev->hu.hdev, "send command 
failed\n");
+   bt_dev_err(lldev->hu.hdev, "send command 
failed");
err = PTR_ERR(skb);
goto out_rel_fw;
}
-- 
2.7.4

[PATCH] bluetooth: serdev: hci_ll: Wait for CTS instead of using msleep

[David Lechner]

When a TI Bluetooth chip is reset, it takes about 100ms for the RTS line of
the chip to deassert. For my use case with a TI CC2560A chip, this delay
was not long enough and caused the local UART to never transmit at all (TI
AM1808 SoC UART2).

We can wait for the CTS signal using serdev_device_wait_for_cts() instead
of trying to guess using msleep().

Also changed the comment to be more informative while we are touching this
code.

Signed-off-by: David Lechner 
---
 drivers/bluetooth/hci_ll.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/bluetooth/hci_ll.c b/drivers/bluetooth/hci_ll.c
index 1f6b933..b295cf8 100644
--- a/drivers/bluetooth/hci_ll.c
+++ b/drivers/bluetooth/hci_ll.c
@@ -674,11 +674,15 @@ static int ll_setup(struct hci_uart *hu)
serdev_device_set_flow_control(serdev, true);
 
do {
-   /* Configure BT_EN to HIGH state */
+   /* Reset the Bluetooth device */
gpiod_set_value_cansleep(lldev->enable_gpio, 0);
msleep(5);
gpiod_set_value_cansleep(lldev->enable_gpio, 1);
-   msleep(100);
+   err = serdev_device_wait_for_cts(serdev, true, 200);
+   if (err) {
+   bt_dev_err(hu->hdev, "Failed to get CTS");
+   return err;
+   }
 
err = download_firmware(lldev);
if (!err)
-- 
2.7.4

[PATCH] bluetooth: serdev: hci_ll: Wait for CTS instead of using msleep

[David Lechner]

When a TI Bluetooth chip is reset, it takes about 100ms for the RTS line of
the chip to deassert. For my use case with a TI CC2560A chip, this delay
was not long enough and caused the local UART to never transmit at all (TI
AM1808 SoC UART2).

We can wait for the CTS signal using serdev_device_wait_for_cts() instead
of trying to guess using msleep().

Also changed the comment to be more informative while we are touching this
code.

Signed-off-by: David Lechner 
---
 drivers/bluetooth/hci_ll.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/bluetooth/hci_ll.c b/drivers/bluetooth/hci_ll.c
index 1f6b933..b295cf8 100644
--- a/drivers/bluetooth/hci_ll.c
+++ b/drivers/bluetooth/hci_ll.c
@@ -674,11 +674,15 @@ static int ll_setup(struct hci_uart *hu)
serdev_device_set_flow_control(serdev, true);
 
do {
-   /* Configure BT_EN to HIGH state */
+   /* Reset the Bluetooth device */
gpiod_set_value_cansleep(lldev->enable_gpio, 0);
msleep(5);
gpiod_set_value_cansleep(lldev->enable_gpio, 1);
-   msleep(100);
+   err = serdev_device_wait_for_cts(serdev, true, 200);
+   if (err) {
+   bt_dev_err(hu->hdev, "Failed to get CTS");
+   return err;
+   }
 
err = download_firmware(lldev);
if (!err)
-- 
2.7.4

Re: [PATCH net,stable v4 0/3] vhost: fix a few skb leaks

[David Miller]

From: w...@redhat.com
Date: Fri,  1 Dec 2017 05:10:35 -0500

> Matthew found a roughly 40% tcp throughput regression with commit
> c67df11f(vhost_net: try batch dequing from skb array) as discussed
> in the following thread:
> https://www.mail-archive.com/netdev@vger.kernel.org/msg187936.html

Series applied and queued up for -stable.

Re: [kernel-hardening][PATCH v2 3/3] arm: mm: dump: add checking for writable and executable pages

[park jinbum]

I agree with your opinion, Laura.
I'll make a new version to take advantage of the existing pg_level and
bits arrays.

Thanks,
Jinbum Park.

2017-12-02 6:59 GMT+09:00 Laura Abbott :
> On 12/01/2017 03:34 AM, Jinbum Park wrote:
>>
>> +static inline bool is_prot_ro(struct pg_state *st)
>> +{
>> +   if (st->level < 4) {
>> +   #ifdef CONFIG_ARM_LPAE
>> +   if ((st->current_prot &
>> +   (L_PMD_SECT_RDONLY | PMD_SECT_AP2)) ==
>> +   (L_PMD_SECT_RDONLY | PMD_SECT_AP2))
>> +   return true;
>> +   #elif __LINUX_ARM_ARCH__ >= 6
>> +   if ((st->current_prot &
>> +   (PMD_SECT_APX | PMD_SECT_AP_READ | PMD_SECT_AP_WRITE)) ==
>> +   (PMD_SECT_APX | PMD_SECT_AP_WRITE))
>> +   return true;
>> +   #else
>> +   if ((st->current_prot &
>> +   (PMD_SECT_AP_READ | PMD_SECT_AP_WRITE)) == 0)
>> +   return true;
>> +   #endif
>> +   } else {
>> +   if ((st->current_prot & L_PTE_RDONLY) == L_PTE_RDONLY)
>> +   return true;
>> +   }
>> +
>> +   return false;
>> +}
>> +
>> +static inline bool is_prot_nx(struct pg_state *st)
>> +{
>> +   if (st->level < 4) {
>> +   if ((st->current_prot & PMD_SECT_XN) == PMD_SECT_XN)
>> +   return true;
>> +   } else {
>> +   if ((st->current_prot & L_PTE_XN) == L_PTE_XN)
>> +   return true;
>> +   }
>> +
>> +   return false;
>> +}
>
>
> I know arm64 checks the bits directly, but the arm32 code is a bit
> more fiddly and I have mixed feelings about copying and pasting
> the checks. It would be cleaner if we could take advantage of
> the existing pg_level and bits arrays. I also don't have my heart
> set on this so if nobody else objects, the code can stay as is.
>
> Thanks,
> Laura

Re: [PATCH net,stable v4 0/3] vhost: fix a few skb leaks

[David Miller]

From: w...@redhat.com
Date: Fri,  1 Dec 2017 05:10:35 -0500

> Matthew found a roughly 40% tcp throughput regression with commit
> c67df11f(vhost_net: try batch dequing from skb array) as discussed
> in the following thread:
> https://www.mail-archive.com/netdev@vger.kernel.org/msg187936.html

Series applied and queued up for -stable.

Re: [kernel-hardening][PATCH v2 3/3] arm: mm: dump: add checking for writable and executable pages

[park jinbum]

I agree with your opinion, Laura.
I'll make a new version to take advantage of the existing pg_level and
bits arrays.

Thanks,
Jinbum Park.

2017-12-02 6:59 GMT+09:00 Laura Abbott :
> On 12/01/2017 03:34 AM, Jinbum Park wrote:
>>
>> +static inline bool is_prot_ro(struct pg_state *st)
>> +{
>> +   if (st->level < 4) {
>> +   #ifdef CONFIG_ARM_LPAE
>> +   if ((st->current_prot &
>> +   (L_PMD_SECT_RDONLY | PMD_SECT_AP2)) ==
>> +   (L_PMD_SECT_RDONLY | PMD_SECT_AP2))
>> +   return true;
>> +   #elif __LINUX_ARM_ARCH__ >= 6
>> +   if ((st->current_prot &
>> +   (PMD_SECT_APX | PMD_SECT_AP_READ | PMD_SECT_AP_WRITE)) ==
>> +   (PMD_SECT_APX | PMD_SECT_AP_WRITE))
>> +   return true;
>> +   #else
>> +   if ((st->current_prot &
>> +   (PMD_SECT_AP_READ | PMD_SECT_AP_WRITE)) == 0)
>> +   return true;
>> +   #endif
>> +   } else {
>> +   if ((st->current_prot & L_PTE_RDONLY) == L_PTE_RDONLY)
>> +   return true;
>> +   }
>> +
>> +   return false;
>> +}
>> +
>> +static inline bool is_prot_nx(struct pg_state *st)
>> +{
>> +   if (st->level < 4) {
>> +   if ((st->current_prot & PMD_SECT_XN) == PMD_SECT_XN)
>> +   return true;
>> +   } else {
>> +   if ((st->current_prot & L_PTE_XN) == L_PTE_XN)
>> +   return true;
>> +   }
>> +
>> +   return false;
>> +}
>
>
> I know arm64 checks the bits directly, but the arm32 code is a bit
> more fiddly and I have mixed feelings about copying and pasting
> the checks. It would be cleaner if we could take advantage of
> the existing pg_level and bits arrays. I also don't have my heart
> set on this so if nobody else objects, the code can stay as is.
>
> Thanks,
> Laura

Re: [PATCH RFC] stat.2: Document that stat can fail with EINTR

[Matthew Wilcox]

On Sat, Dec 02, 2017 at 07:23:59PM -0500, Keno Fischer wrote:
> The catalyst for this patch was me experiencing EINTR errors when
> using the 9p file system. In linux commit 9523feac, the 9p file
> system was changed to use wait_event_killable instead of
> wait_event_interruptible, which does indeed address my problem,
> but also makes me a bit unhappy, because uninterruptable waits
> prevents things like ^C'ing the execution and some debugging
> tools which depend on being able to cancel long-running operations
> by sending signals.

Wait, wait, wait.  killable is not uninterruptible.  It's "can accept
a signal if the signal is fatal".  ie userspace will never see it.
So, no, it doesn't prevent ^C.  It does prevent the debugging tool you're
talking about from working, because it's handling the signal, so it's not
fatal.

> I realize I'm probably 20 years too late here, but it feels like
> clarificaion on what to expect from the kernel would still go a long
> way here.  

A change to user-visible behaviour has to be opt-in.  So here's an idea --
a prctl() (or whatever) that says "I can handle EINTR on any syscall".
It would effectively change the *_killable logic to return EINTR on any
signal, not just fatal ones.  Best of luck auditing every syscall your
application  makes ... and every library it uses ... maybe dlopen()ed
like PAM modules ...

But we could do it!  And it's more sensible than "I want to change
individual syscalls one at a time as I notice each one is a problem".

Re: [PATCH RFC] stat.2: Document that stat can fail with EINTR

[Matthew Wilcox]

On Sat, Dec 02, 2017 at 07:23:59PM -0500, Keno Fischer wrote:
> The catalyst for this patch was me experiencing EINTR errors when
> using the 9p file system. In linux commit 9523feac, the 9p file
> system was changed to use wait_event_killable instead of
> wait_event_interruptible, which does indeed address my problem,
> but also makes me a bit unhappy, because uninterruptable waits
> prevents things like ^C'ing the execution and some debugging
> tools which depend on being able to cancel long-running operations
> by sending signals.

Wait, wait, wait.  killable is not uninterruptible.  It's "can accept
a signal if the signal is fatal".  ie userspace will never see it.
So, no, it doesn't prevent ^C.  It does prevent the debugging tool you're
talking about from working, because it's handling the signal, so it's not
fatal.

> I realize I'm probably 20 years too late here, but it feels like
> clarificaion on what to expect from the kernel would still go a long
> way here.  

A change to user-visible behaviour has to be opt-in.  So here's an idea --
a prctl() (or whatever) that says "I can handle EINTR on any syscall".
It would effectively change the *_killable logic to return EINTR on any
signal, not just fatal ones.  Best of luck auditing every syscall your
application  makes ... and every library it uses ... maybe dlopen()ed
like PAM modules ...

But we could do it!  And it's more sensible than "I want to change
individual syscalls one at a time as I notice each one is a problem".

Re: [PATCH net-next 0/2] net: dsa: cross-chip FDB support

[David Miller]

From: Vivien Didelot 
Date: Thu, 30 Nov 2017 12:56:41 -0500

> DSA can have interconnected switches. For instance, the ZII Dev Rev B
> board described in arch/arm/boot/dts/vf610-zii-dev-rev-b.dts has a
> switch fabric composed of 3 switch devices like this:
> 
>   lan4 lan6
> CPU (eth1)|  lan5 |  lan7
>   |   | | | |
>[0 1 2 3 4 6 5]---[6 0 1 2 3 4 5]---[9 0 1 2 3 4 5 6 7 8]
> | | |   | | | |
> lan0  |  lan2   lan3  lan8  |  optical4
>lan1  optical3
> 
> One current issue with DSA is cross-chip FDB. If we add a static MAC
> address on lan3, only its parent switch 1 (the one in the middle) will
> be programmed. That is not correct in a cross-chip environment, because
> the DSA ports connecting to switch 1 of adjacent switch 0 (on the left)
> and switch 2 (on the right) must be programmed too.
> 
> Without this patchset, a dump of the hardware FDB of switches 0, 1 and 2
> after programming a MAC address on lan3 looks like this (*):
> 
> # bridge fdb add 11:22:33:44:55:66 dev lan3
> # cat /sys/kernel/debug/mv88e6xxx/sw*/atu/0 | grep -v FID
>0  ff:ff:ff:ff:ff:ffMC_STATIC   n  0 1 2 3 4 5 6
>0  11:22:33:44:55:66MC_STATIC_MGMT_PO   n  0 - - - - - -
>0  ff:ff:ff:ff:ff:ffMC_STATIC   n  0 1 2 3 4 5 6
>0  ff:ff:ff:ff:ff:ffMC_STATIC   n  0 1 2 3 4 5 6 7 8 9
> 
> With this patchset applied, adjacent DSA ports get programmed too:
> 
> # bridge fdb add 11:22:33:44:55:66 dev lan3
> # cat /sys/kernel/debug/mv88e6xxx/sw*/atu/0 | grep -v FID
>0  11:22:33:44:55:66MC_STATIC_MGMT_PO   n  - - - - - 5 -
>0  ff:ff:ff:ff:ff:ffMC_STATIC   n  0 1 2 3 4 5 6
>0  11:22:33:44:55:66MC_STATIC_MGMT_PO   n  0 - - - - - -
>0  ff:ff:ff:ff:ff:ffMC_STATIC   n  0 1 2 3 4 5 6
>0  11:22:33:44:55:66MC_STATIC_MGMT_PO   n  - - - - - - - - - 9
>0  ff:ff:ff:ff:ff:ffMC_STATIC   n  0 1 2 3 4 5 6 7 8 9
 ...

Series applied, thanks.

Re: [PATCH net-next 0/2] net: dsa: cross-chip FDB support

[David Miller]

From: Vivien Didelot 
Date: Thu, 30 Nov 2017 12:56:41 -0500

> DSA can have interconnected switches. For instance, the ZII Dev Rev B
> board described in arch/arm/boot/dts/vf610-zii-dev-rev-b.dts has a
> switch fabric composed of 3 switch devices like this:
> 
>   lan4 lan6
> CPU (eth1)|  lan5 |  lan7
>   |   | | | |
>[0 1 2 3 4 6 5]---[6 0 1 2 3 4 5]---[9 0 1 2 3 4 5 6 7 8]
> | | |   | | | |
> lan0  |  lan2   lan3  lan8  |  optical4
>lan1  optical3
> 
> One current issue with DSA is cross-chip FDB. If we add a static MAC
> address on lan3, only its parent switch 1 (the one in the middle) will
> be programmed. That is not correct in a cross-chip environment, because
> the DSA ports connecting to switch 1 of adjacent switch 0 (on the left)
> and switch 2 (on the right) must be programmed too.
> 
> Without this patchset, a dump of the hardware FDB of switches 0, 1 and 2
> after programming a MAC address on lan3 looks like this (*):
> 
> # bridge fdb add 11:22:33:44:55:66 dev lan3
> # cat /sys/kernel/debug/mv88e6xxx/sw*/atu/0 | grep -v FID
>0  ff:ff:ff:ff:ff:ffMC_STATIC   n  0 1 2 3 4 5 6
>0  11:22:33:44:55:66MC_STATIC_MGMT_PO   n  0 - - - - - -
>0  ff:ff:ff:ff:ff:ffMC_STATIC   n  0 1 2 3 4 5 6
>0  ff:ff:ff:ff:ff:ffMC_STATIC   n  0 1 2 3 4 5 6 7 8 9
> 
> With this patchset applied, adjacent DSA ports get programmed too:
> 
> # bridge fdb add 11:22:33:44:55:66 dev lan3
> # cat /sys/kernel/debug/mv88e6xxx/sw*/atu/0 | grep -v FID
>0  11:22:33:44:55:66MC_STATIC_MGMT_PO   n  - - - - - 5 -
>0  ff:ff:ff:ff:ff:ffMC_STATIC   n  0 1 2 3 4 5 6
>0  11:22:33:44:55:66MC_STATIC_MGMT_PO   n  0 - - - - - -
>0  ff:ff:ff:ff:ff:ffMC_STATIC   n  0 1 2 3 4 5 6
>0  11:22:33:44:55:66MC_STATIC_MGMT_PO   n  - - - - - - - - - 9
>0  ff:ff:ff:ff:ff:ffMC_STATIC   n  0 1 2 3 4 5 6 7 8 9
 ...

Series applied, thanks.

Re: [PATCH net-next 0/4] net: dsa: simplify switchdev prepare phase

[David Miller]

From: Vivien Didelot 
Date: Thu, 30 Nov 2017 11:23:56 -0500

> This patch series brings no functional changes.
> 
> It removes the unused switchdev_trans arguments from the dsa_switch_ops
> for both MDB and VLAN operations, and provides functions to prepare and
> add these objects for a given bitmap of ports.

Series applied.

Re: [PATCH net-next 0/4] net: dsa: simplify switchdev prepare phase

[David Miller]

From: Vivien Didelot 
Date: Thu, 30 Nov 2017 11:23:56 -0500

> This patch series brings no functional changes.
> 
> It removes the unused switchdev_trans arguments from the dsa_switch_ops
> for both MDB and VLAN operations, and provides functions to prepare and
> add these objects for a given bitmap of ports.

Series applied.

[PATCH v2 0/8] perf tools: perf tools: Clarify overwrite and backward, bugfix

[Wang Nan]

THe final result of this patchset is removing the concept of
'forward/backward', merge them into the concept of 'overwrite'.

Patch 1 to 5 clear arguments lists of many functions, remove the
'overwrite'. Because all callers of these functions doesn't need
the overwrite be set, we can simply remove them from arguments
lists and adjust code as if a 'false' is given.

Patch 6 fix a bug that forget to setting readonly for overwrite
ring buffers.

Patch 7 is suggested by Liang Kan, prevent dumpping duplicated
data if there's no so many events between two dumpping commands.

Patch 8 is 's/backward/overwrite'. After patch 8, the concept of
'backward' is removed from most of the code, make it uniform with
user interface ('--overwrite').

Cc: Kan Liang 
Cc: Arnaldo Carvalho de Melo 
Cc: Jiri Olsa 
Cc: Namhyung Kim 
Cc: Zhang Mengting 

Wang Nan (8):
  perf tools: Remove 'overwrite' parameter from perf_evlist__mmap
  perf tools: Remove 'overwrite' parameter from perf_evlist__mmap_ex
  perf tools: Remove evlist->overwrite
  perf tools: Remove overwrite from arguments list of perf_mmap__push
  perf tools: Remove overwrite and check_messup from mmap read
  perf mmap: Fix perf backward recording
  perf tool: Don't discard prev in backward mode
  perf tools: Replace 'backward' to 'overwrite' in evlist, mmap and
record

 tools/perf/arch/x86/tests/perf-time-to-tsc.c |  2 +-
 tools/perf/builtin-kvm.c |  2 +-
 tools/perf/builtin-record.c  | 16 +++---
 tools/perf/builtin-top.c |  2 +-
 tools/perf/builtin-trace.c   |  2 +-
 tools/perf/tests/backward-ring-buffer.c  |  6 +--
 tools/perf/tests/bpf.c   |  2 +-
 tools/perf/tests/code-reading.c  |  2 +-
 tools/perf/tests/keep-tracking.c |  2 +-
 tools/perf/tests/mmap-basic.c|  2 +-
 tools/perf/tests/openat-syscall-tp-fields.c  |  2 +-
 tools/perf/tests/perf-record.c   |  2 +-
 tools/perf/tests/sw-clock.c  |  2 +-
 tools/perf/tests/switch-tracking.c   |  2 +-
 tools/perf/tests/task-exit.c |  2 +-
 tools/perf/util/evlist.c | 53 ++--
 tools/perf/util/evlist.h |  8 ++-
 tools/perf/util/mmap.c   | 73 ++--
 tools/perf/util/mmap.h   |  4 +-
 tools/perf/util/python.c |  2 +-
 20 files changed, 81 insertions(+), 107 deletions(-)

-- 
2.10.1

[PATCH v2 0/8] perf tools: perf tools: Clarify overwrite and backward, bugfix

[Wang Nan]

THe final result of this patchset is removing the concept of
'forward/backward', merge them into the concept of 'overwrite'.

Patch 1 to 5 clear arguments lists of many functions, remove the
'overwrite'. Because all callers of these functions doesn't need
the overwrite be set, we can simply remove them from arguments
lists and adjust code as if a 'false' is given.

Patch 6 fix a bug that forget to setting readonly for overwrite
ring buffers.

Patch 7 is suggested by Liang Kan, prevent dumpping duplicated
data if there's no so many events between two dumpping commands.

Patch 8 is 's/backward/overwrite'. After patch 8, the concept of
'backward' is removed from most of the code, make it uniform with
user interface ('--overwrite').

Cc: Kan Liang 
Cc: Arnaldo Carvalho de Melo 
Cc: Jiri Olsa 
Cc: Namhyung Kim 
Cc: Zhang Mengting 

Wang Nan (8):
  perf tools: Remove 'overwrite' parameter from perf_evlist__mmap
  perf tools: Remove 'overwrite' parameter from perf_evlist__mmap_ex
  perf tools: Remove evlist->overwrite
  perf tools: Remove overwrite from arguments list of perf_mmap__push
  perf tools: Remove overwrite and check_messup from mmap read
  perf mmap: Fix perf backward recording
  perf tool: Don't discard prev in backward mode
  perf tools: Replace 'backward' to 'overwrite' in evlist, mmap and
record

 tools/perf/arch/x86/tests/perf-time-to-tsc.c |  2 +-
 tools/perf/builtin-kvm.c |  2 +-
 tools/perf/builtin-record.c  | 16 +++---
 tools/perf/builtin-top.c |  2 +-
 tools/perf/builtin-trace.c   |  2 +-
 tools/perf/tests/backward-ring-buffer.c  |  6 +--
 tools/perf/tests/bpf.c   |  2 +-
 tools/perf/tests/code-reading.c  |  2 +-
 tools/perf/tests/keep-tracking.c |  2 +-
 tools/perf/tests/mmap-basic.c|  2 +-
 tools/perf/tests/openat-syscall-tp-fields.c  |  2 +-
 tools/perf/tests/perf-record.c   |  2 +-
 tools/perf/tests/sw-clock.c  |  2 +-
 tools/perf/tests/switch-tracking.c   |  2 +-
 tools/perf/tests/task-exit.c |  2 +-
 tools/perf/util/evlist.c | 53 ++--
 tools/perf/util/evlist.h |  8 ++-
 tools/perf/util/mmap.c   | 73 ++--
 tools/perf/util/mmap.h   |  4 +-
 tools/perf/util/python.c |  2 +-
 20 files changed, 81 insertions(+), 107 deletions(-)

-- 
2.10.1

[PATCH v2 6/8] perf mmap: Fix perf backward recording

[Wang Nan]

perf record backward recording doesn't work as we expected: it never
overwrite when ring buffer full.

Test:

(Run a busy printing task background like this:

 while True:
 print 123

send SIGUSR2 to perf to capture snapshot.)

 # ./perf record --overwrite -e raw_syscalls:sys_enter -e raw_syscalls:sys_exit 
--exclude-perf -a --switch-output
 [ perf record: dump data: Woken up 1 times ]
 [ perf record: Dump perf.data.2017110101520743 ]
 [ perf record: dump data: Woken up 1 times ]
 [ perf record: Dump perf.data.2017110101521251 ]
 [ perf record: dump data: Woken up 1 times ]
 [ perf record: Dump perf.data.2017110101521692 ]
 ^C[ perf record: Woken up 1 times to write data ]
 [ perf record: Dump perf.data.2017110101521936 ]
 [ perf record: Captured and wrote 0.826 MB perf.data. ]

 # ./perf script -i ./perf.data.2017110101520743 | head -n3
 perf  2717 [000] 12449.310785: raw_syscalls:sys_enter: NR 16 (5, 
2400, 0, 59, 100, 0)
 perf  2717 [000] 12449.310790: raw_syscalls:sys_enter: NR 7 
(4112340, 2, , 3df, 100, 0)
   python  2545 [000] 12449.310800:  raw_syscalls:sys_exit: NR 1 = 4
 # ./perf script -i ./perf.data.2017110101521251 | head -n3
 perf  2717 [000] 12449.310785: raw_syscalls:sys_enter: NR 16 (5, 
2400, 0, 59, 100, 0)
 perf  2717 [000] 12449.310790: raw_syscalls:sys_enter: NR 7 
(4112340, 2, , 3df, 100, 0)
   python  2545 [000] 12449.310800:  raw_syscalls:sys_exit: NR 1 = 4
 # ./perf script -i ./perf.data.2017110101521692 | head -n3
 perf  2717 [000] 12449.310785: raw_syscalls:sys_enter: NR 16 (5, 
2400, 0, 59, 100, 0)
 perf  2717 [000] 12449.310790: raw_syscalls:sys_enter: NR 7 
(4112340, 2, , 3df, 100, 0)
   python  2545 [000] 12449.310800:  raw_syscalls:sys_exit: NR 1 = 4

Timestamps are never change, but my background task is a dead loop, can
easily overwhelme the ring buffer.

This patch fix it by force unsetting PROT_WRITE for backward ring
buffer, so all backward ring buffer become overwrite ring buffer.

Test result:

 # ./perf record --overwrite -e raw_syscalls:sys_enter -e raw_syscalls:sys_exit 
--exclude-perf -a --switch-output
 [ perf record: dump data: Woken up 1 times ]
 [ perf record: Dump perf.data.2017110101285323 ]
 [ perf record: dump data: Woken up 1 times ]
 [ perf record: Dump perf.data.2017110101290053 ]
 [ perf record: dump data: Woken up 1 times ]
 [ perf record: Dump perf.data.2017110101290446 ]
 ^C[ perf record: Woken up 1 times to write data ]
 [ perf record: Dump perf.data.2017110101290837 ]
 [ perf record: Captured and wrote 0.826 MB perf.data. ]
 # ./perf script -i ./perf.data.2017110101285323 | head -n3
   python  2545 [000] 11064.268083:  raw_syscalls:sys_exit: NR 1 = 4
   python  2545 [000] 11064.268084: raw_syscalls:sys_enter: NR 1 (1, 
12cc330, 4, 7fc237280370, 7fc2373d0700, 2c7b0)
   python  2545 [000] 11064.268086:  raw_syscalls:sys_exit: NR 1 = 4
 # ./perf script -i ./perf.data.2017110101290 | head -n3
 failed to open ./perf.data.2017110101290: No such file or directory
 # ./perf script -i ./perf.data.2017110101290053 | head -n3
   python  2545 [000] 11071.564062: raw_syscalls:sys_enter: NR 1 (1, 
12cc330, 4, 7fc237280370, 7fc2373d0700, 2c7b0)
   python  2545 [000] 11071.564064:  raw_syscalls:sys_exit: NR 1 = 4
   python  2545 [000] 11071.564066: raw_syscalls:sys_enter: NR 1 (1, 
12cc330, 4, 7fc237280370, 7fc2373d0700, 2c7b0)
 # ./perf script -i ./perf.data.2017110101290 | head -n3
 perf.data.2017110101290053  perf.data.2017110101290446  
perf.data.2017110101290837
 # ./perf script -i ./perf.data.2017110101290446 | head -n3
 sshd  1321 [000] 11075.499473:  raw_syscalls:sys_exit: NR 14 = 0
 sshd  1321 [000] 11075.499474: raw_syscalls:sys_enter: NR 14 (2, 
7ffe98899490, 0, 8, 0, 3000)
 sshd  1321 [000] 11075.499474:  raw_syscalls:sys_exit: NR 14 = 0
 # ./perf script -i ./perf.data.2017110101290837 | head -n3
   python  2545 [000] 11079.280844:  raw_syscalls:sys_exit: NR 1 = 4
   python  2545 [000] 11079.280847: raw_syscalls:sys_enter: NR 1 (1, 
12cc330, 4, 7fc237280370, 7fc2373d0700, 2c7b0)
   python  2545 [000] 11079.280850:  raw_syscalls:sys_exit: NR 1 = 4

Signed-off-by: Wang Nan 
---
 tools/perf/util/evlist.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index 68c1f95..bb70aef 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -799,7 +799,7 @@ perf_evlist__should_poll(struct perf_evlist *evlist 
__maybe_unused,
 }
 
 static int perf_evlist__mmap_per_evsel(struct perf_evlist *evlist, int idx,
-  struct mmap_params *mp, int cpu_idx,
+  struct mmap_params mp, int cpu_idx,
   int thread, int *_output, int 

[PATCH v2 7/8] perf tools: Don't discard prev in backward mode

[Wang Nan]

Perf record can switch output. The new output should only store the
data after switching. However, in overwrite backward mode, the new
output still have the data from old output. That also brings extra
overhead.

At the end of mmap_read, the position of processed ring buffer is
saved in md->prev. Next mmap_read should be end in md->prev if it is
not overwriten. That avoids to process duplicate data.
However, the md->prev is discarded. So next mmap_read has to process
whole valid ring buffer, which probably include the old processed
data.

Avoid calling backward_rb_find_range() when md->prev is still
available.

Signed-off-by: Wang Nan 
Tested-by: Kan Liang 
---
 tools/perf/util/mmap.c | 33 +++--
 1 file changed, 15 insertions(+), 18 deletions(-)

diff --git a/tools/perf/util/mmap.c b/tools/perf/util/mmap.c
index 3f262e7..5f8cb15 100644
--- a/tools/perf/util/mmap.c
+++ b/tools/perf/util/mmap.c
@@ -267,18 +267,6 @@ static int backward_rb_find_range(void *buf, int mask, u64 
head, u64 *start, u64
return -1;
 }
 
-static int rb_find_range(void *data, int mask, u64 head, u64 old,
-u64 *start, u64 *end, bool backward)
-{
-   if (!backward) {
-   *start = old;
-   *end = head;
-   return 0;
-   }
-
-   return backward_rb_find_range(data, mask, head, start, end);
-}
-
 int perf_mmap__push(struct perf_mmap *md, bool backward,
void *to, int push(void *to, void *buf, size_t size))
 {
@@ -290,19 +278,28 @@ int perf_mmap__push(struct perf_mmap *md, bool backward,
void *buf;
int rc = 0;
 
-   if (rb_find_range(data, md->mask, head, old, , , backward))
-   return -1;
+   start = backward ? head : old;
+   end = backward ? old : head;
 
if (start == end)
return 0;
 
size = end - start;
if (size > (unsigned long)(md->mask) + 1) {
-   WARN_ONCE(1, "failed to keep up with mmap data. (warn only 
once)\n");
+   if (!backward) {
+   WARN_ONCE(1, "failed to keep up with mmap data. (warn 
only once)\n");
 
-   md->prev = head;
-   perf_mmap__consume(md, backward);
-   return 0;
+   md->prev = head;
+   perf_mmap__consume(md, backward);
+   return 0;
+   }
+
+   /*
+* Backward ring buffer is full. We still have a chance to read
+* most of data from it.
+*/
+   if (backward_rb_find_range(data, md->mask, head, , ))
+   return -1;
}
 
if ((start & md->mask) + size != (end & md->mask)) {
-- 
2.10.1

[PATCH v2 6/8] perf mmap: Fix perf backward recording

[Wang Nan]

perf record backward recording doesn't work as we expected: it never
overwrite when ring buffer full.

Test:

(Run a busy printing task background like this:

 while True:
 print 123

send SIGUSR2 to perf to capture snapshot.)

 # ./perf record --overwrite -e raw_syscalls:sys_enter -e raw_syscalls:sys_exit 
--exclude-perf -a --switch-output
 [ perf record: dump data: Woken up 1 times ]
 [ perf record: Dump perf.data.2017110101520743 ]
 [ perf record: dump data: Woken up 1 times ]
 [ perf record: Dump perf.data.2017110101521251 ]
 [ perf record: dump data: Woken up 1 times ]
 [ perf record: Dump perf.data.2017110101521692 ]
 ^C[ perf record: Woken up 1 times to write data ]
 [ perf record: Dump perf.data.2017110101521936 ]
 [ perf record: Captured and wrote 0.826 MB perf.data. ]

 # ./perf script -i ./perf.data.2017110101520743 | head -n3
 perf  2717 [000] 12449.310785: raw_syscalls:sys_enter: NR 16 (5, 
2400, 0, 59, 100, 0)
 perf  2717 [000] 12449.310790: raw_syscalls:sys_enter: NR 7 
(4112340, 2, , 3df, 100, 0)
   python  2545 [000] 12449.310800:  raw_syscalls:sys_exit: NR 1 = 4
 # ./perf script -i ./perf.data.2017110101521251 | head -n3
 perf  2717 [000] 12449.310785: raw_syscalls:sys_enter: NR 16 (5, 
2400, 0, 59, 100, 0)
 perf  2717 [000] 12449.310790: raw_syscalls:sys_enter: NR 7 
(4112340, 2, , 3df, 100, 0)
   python  2545 [000] 12449.310800:  raw_syscalls:sys_exit: NR 1 = 4
 # ./perf script -i ./perf.data.2017110101521692 | head -n3
 perf  2717 [000] 12449.310785: raw_syscalls:sys_enter: NR 16 (5, 
2400, 0, 59, 100, 0)
 perf  2717 [000] 12449.310790: raw_syscalls:sys_enter: NR 7 
(4112340, 2, , 3df, 100, 0)
   python  2545 [000] 12449.310800:  raw_syscalls:sys_exit: NR 1 = 4

Timestamps are never change, but my background task is a dead loop, can
easily overwhelme the ring buffer.

This patch fix it by force unsetting PROT_WRITE for backward ring
buffer, so all backward ring buffer become overwrite ring buffer.

Test result:

 # ./perf record --overwrite -e raw_syscalls:sys_enter -e raw_syscalls:sys_exit 
--exclude-perf -a --switch-output
 [ perf record: dump data: Woken up 1 times ]
 [ perf record: Dump perf.data.2017110101285323 ]
 [ perf record: dump data: Woken up 1 times ]
 [ perf record: Dump perf.data.2017110101290053 ]
 [ perf record: dump data: Woken up 1 times ]
 [ perf record: Dump perf.data.2017110101290446 ]
 ^C[ perf record: Woken up 1 times to write data ]
 [ perf record: Dump perf.data.2017110101290837 ]
 [ perf record: Captured and wrote 0.826 MB perf.data. ]
 # ./perf script -i ./perf.data.2017110101285323 | head -n3
   python  2545 [000] 11064.268083:  raw_syscalls:sys_exit: NR 1 = 4
   python  2545 [000] 11064.268084: raw_syscalls:sys_enter: NR 1 (1, 
12cc330, 4, 7fc237280370, 7fc2373d0700, 2c7b0)
   python  2545 [000] 11064.268086:  raw_syscalls:sys_exit: NR 1 = 4
 # ./perf script -i ./perf.data.2017110101290 | head -n3
 failed to open ./perf.data.2017110101290: No such file or directory
 # ./perf script -i ./perf.data.2017110101290053 | head -n3
   python  2545 [000] 11071.564062: raw_syscalls:sys_enter: NR 1 (1, 
12cc330, 4, 7fc237280370, 7fc2373d0700, 2c7b0)
   python  2545 [000] 11071.564064:  raw_syscalls:sys_exit: NR 1 = 4
   python  2545 [000] 11071.564066: raw_syscalls:sys_enter: NR 1 (1, 
12cc330, 4, 7fc237280370, 7fc2373d0700, 2c7b0)
 # ./perf script -i ./perf.data.2017110101290 | head -n3
 perf.data.2017110101290053  perf.data.2017110101290446  
perf.data.2017110101290837
 # ./perf script -i ./perf.data.2017110101290446 | head -n3
 sshd  1321 [000] 11075.499473:  raw_syscalls:sys_exit: NR 14 = 0
 sshd  1321 [000] 11075.499474: raw_syscalls:sys_enter: NR 14 (2, 
7ffe98899490, 0, 8, 0, 3000)
 sshd  1321 [000] 11075.499474:  raw_syscalls:sys_exit: NR 14 = 0
 # ./perf script -i ./perf.data.2017110101290837 | head -n3
   python  2545 [000] 11079.280844:  raw_syscalls:sys_exit: NR 1 = 4
   python  2545 [000] 11079.280847: raw_syscalls:sys_enter: NR 1 (1, 
12cc330, 4, 7fc237280370, 7fc2373d0700, 2c7b0)
   python  2545 [000] 11079.280850:  raw_syscalls:sys_exit: NR 1 = 4

Signed-off-by: Wang Nan 
---
 tools/perf/util/evlist.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index 68c1f95..bb70aef 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -799,7 +799,7 @@ perf_evlist__should_poll(struct perf_evlist *evlist 
__maybe_unused,
 }
 
 static int perf_evlist__mmap_per_evsel(struct perf_evlist *evlist, int idx,
-  struct mmap_params *mp, int cpu_idx,
+  struct mmap_params mp, int cpu_idx,
   int thread, int *_output, int 
*_output_backward)
 

[PATCH v2 7/8] perf tools: Don't discard prev in backward mode

[Wang Nan]

Perf record can switch output. The new output should only store the
data after switching. However, in overwrite backward mode, the new
output still have the data from old output. That also brings extra
overhead.

At the end of mmap_read, the position of processed ring buffer is
saved in md->prev. Next mmap_read should be end in md->prev if it is
not overwriten. That avoids to process duplicate data.
However, the md->prev is discarded. So next mmap_read has to process
whole valid ring buffer, which probably include the old processed
data.

Avoid calling backward_rb_find_range() when md->prev is still
available.

Signed-off-by: Wang Nan 
Tested-by: Kan Liang 
---
 tools/perf/util/mmap.c | 33 +++--
 1 file changed, 15 insertions(+), 18 deletions(-)

diff --git a/tools/perf/util/mmap.c b/tools/perf/util/mmap.c
index 3f262e7..5f8cb15 100644
--- a/tools/perf/util/mmap.c
+++ b/tools/perf/util/mmap.c
@@ -267,18 +267,6 @@ static int backward_rb_find_range(void *buf, int mask, u64 
head, u64 *start, u64
return -1;
 }
 
-static int rb_find_range(void *data, int mask, u64 head, u64 old,
-u64 *start, u64 *end, bool backward)
-{
-   if (!backward) {
-   *start = old;
-   *end = head;
-   return 0;
-   }
-
-   return backward_rb_find_range(data, mask, head, start, end);
-}
-
 int perf_mmap__push(struct perf_mmap *md, bool backward,
void *to, int push(void *to, void *buf, size_t size))
 {
@@ -290,19 +278,28 @@ int perf_mmap__push(struct perf_mmap *md, bool backward,
void *buf;
int rc = 0;
 
-   if (rb_find_range(data, md->mask, head, old, , , backward))
-   return -1;
+   start = backward ? head : old;
+   end = backward ? old : head;
 
if (start == end)
return 0;
 
size = end - start;
if (size > (unsigned long)(md->mask) + 1) {
-   WARN_ONCE(1, "failed to keep up with mmap data. (warn only 
once)\n");
+   if (!backward) {
+   WARN_ONCE(1, "failed to keep up with mmap data. (warn 
only once)\n");
 
-   md->prev = head;
-   perf_mmap__consume(md, backward);
-   return 0;
+   md->prev = head;
+   perf_mmap__consume(md, backward);
+   return 0;
+   }
+
+   /*
+* Backward ring buffer is full. We still have a chance to read
+* most of data from it.
+*/
+   if (backward_rb_find_range(data, md->mask, head, , ))
+   return -1;
}
 
if ((start & md->mask) + size != (end & md->mask)) {
-- 
2.10.1

[PATCH v2 5/8] perf tools: Remove overwrite and check_messup from mmap read

[Wang Nan]

All perf_mmap__read_forward() read from read-write ring buffer,
so no need check_messup. Reading from backward ring buffer doesn't
require check_messup because it never mess up. Cleanup arguments
lists.

Signed-off-by: Wang Nan 
---
 tools/perf/util/evlist.c |  2 +-
 tools/perf/util/mmap.c   | 28 
 tools/perf/util/mmap.h   |  2 +-
 3 files changed, 6 insertions(+), 26 deletions(-)

diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index a59134f..68c1f95 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -711,7 +711,7 @@ union perf_event *perf_evlist__mmap_read_forward(struct 
perf_evlist *evlist, int
 * No need for read-write ring buffer: kernel stop outputting when
 * it hit md->prev (perf_mmap__consume()).
 */
-   return perf_mmap__read_forward(md, false);
+   return perf_mmap__read_forward(md);
 }
 
 union perf_event *perf_evlist__mmap_read_backward(struct perf_evlist *evlist, 
int idx)
diff --git a/tools/perf/util/mmap.c b/tools/perf/util/mmap.c
index 703ed41..3f262e7 100644
--- a/tools/perf/util/mmap.c
+++ b/tools/perf/util/mmap.c
@@ -21,33 +21,13 @@ size_t perf_mmap__mmap_len(struct perf_mmap *map)
 }
 
 /* When check_messup is true, 'end' must points to a good entry */
-static union perf_event *perf_mmap__read(struct perf_mmap *map, bool 
check_messup,
+static union perf_event *perf_mmap__read(struct perf_mmap *map,
 u64 start, u64 end, u64 *prev)
 {
unsigned char *data = map->base + page_size;
union perf_event *event = NULL;
int diff = end - start;
 
-   if (check_messup) {
-   /*
-* If we're further behind than half the buffer, there's a 
chance
-* the writer will bite our tail and mess up the samples under 
us.
-*
-* If we somehow ended up ahead of the 'end', we got messed up.
-*
-* In either case, truncate and restart at 'end'.
-*/
-   if (diff > map->mask / 2 || diff < 0) {
-   fprintf(stderr, "WARNING: failed to keep up with mmap 
data.\n");
-
-   /*
-* 'end' points to a known good entry, start there.
-*/
-   start = end;
-   diff = 0;
-   }
-   }
-
if (diff >= (int)sizeof(event->header)) {
size_t size;
 
@@ -89,7 +69,7 @@ static union perf_event *perf_mmap__read(struct perf_mmap 
*map, bool check_messu
return event;
 }
 
-union perf_event *perf_mmap__read_forward(struct perf_mmap *map, bool 
check_messup)
+union perf_event *perf_mmap__read_forward(struct perf_mmap *map)
 {
u64 head;
u64 old = map->prev;
@@ -102,7 +82,7 @@ union perf_event *perf_mmap__read_forward(struct perf_mmap 
*map, bool check_mess
 
head = perf_mmap__read_head(map);
 
-   return perf_mmap__read(map, check_messup, old, head, >prev);
+   return perf_mmap__read(map, old, head, >prev);
 }
 
 union perf_event *perf_mmap__read_backward(struct perf_mmap *map)
@@ -138,7 +118,7 @@ union perf_event *perf_mmap__read_backward(struct perf_mmap 
*map)
else
end = head + map->mask + 1;
 
-   return perf_mmap__read(map, false, start, end, >prev);
+   return perf_mmap__read(map, start, end, >prev);
 }
 
 void perf_mmap__read_catchup(struct perf_mmap *map)
diff --git a/tools/perf/util/mmap.h b/tools/perf/util/mmap.h
index 2c3d291..d640273 100644
--- a/tools/perf/util/mmap.h
+++ b/tools/perf/util/mmap.h
@@ -86,7 +86,7 @@ static inline void perf_mmap__write_tail(struct perf_mmap 
*md, u64 tail)
pc->data_tail = tail;
 }
 
-union perf_event *perf_mmap__read_forward(struct perf_mmap *map, bool 
check_messup);
+union perf_event *perf_mmap__read_forward(struct perf_mmap *map);
 union perf_event *perf_mmap__read_backward(struct perf_mmap *map);
 
 int perf_mmap__push(struct perf_mmap *md, bool backward,
-- 
2.10.1

[PATCH v2 5/8] perf tools: Remove overwrite and check_messup from mmap read

[Wang Nan]

All perf_mmap__read_forward() read from read-write ring buffer,
so no need check_messup. Reading from backward ring buffer doesn't
require check_messup because it never mess up. Cleanup arguments
lists.

Signed-off-by: Wang Nan 
---
 tools/perf/util/evlist.c |  2 +-
 tools/perf/util/mmap.c   | 28 
 tools/perf/util/mmap.h   |  2 +-
 3 files changed, 6 insertions(+), 26 deletions(-)

diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index a59134f..68c1f95 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -711,7 +711,7 @@ union perf_event *perf_evlist__mmap_read_forward(struct 
perf_evlist *evlist, int
 * No need for read-write ring buffer: kernel stop outputting when
 * it hit md->prev (perf_mmap__consume()).
 */
-   return perf_mmap__read_forward(md, false);
+   return perf_mmap__read_forward(md);
 }
 
 union perf_event *perf_evlist__mmap_read_backward(struct perf_evlist *evlist, 
int idx)
diff --git a/tools/perf/util/mmap.c b/tools/perf/util/mmap.c
index 703ed41..3f262e7 100644
--- a/tools/perf/util/mmap.c
+++ b/tools/perf/util/mmap.c
@@ -21,33 +21,13 @@ size_t perf_mmap__mmap_len(struct perf_mmap *map)
 }
 
 /* When check_messup is true, 'end' must points to a good entry */
-static union perf_event *perf_mmap__read(struct perf_mmap *map, bool 
check_messup,
+static union perf_event *perf_mmap__read(struct perf_mmap *map,
 u64 start, u64 end, u64 *prev)
 {
unsigned char *data = map->base + page_size;
union perf_event *event = NULL;
int diff = end - start;
 
-   if (check_messup) {
-   /*
-* If we're further behind than half the buffer, there's a 
chance
-* the writer will bite our tail and mess up the samples under 
us.
-*
-* If we somehow ended up ahead of the 'end', we got messed up.
-*
-* In either case, truncate and restart at 'end'.
-*/
-   if (diff > map->mask / 2 || diff < 0) {
-   fprintf(stderr, "WARNING: failed to keep up with mmap 
data.\n");
-
-   /*
-* 'end' points to a known good entry, start there.
-*/
-   start = end;
-   diff = 0;
-   }
-   }
-
if (diff >= (int)sizeof(event->header)) {
size_t size;
 
@@ -89,7 +69,7 @@ static union perf_event *perf_mmap__read(struct perf_mmap 
*map, bool check_messu
return event;
 }
 
-union perf_event *perf_mmap__read_forward(struct perf_mmap *map, bool 
check_messup)
+union perf_event *perf_mmap__read_forward(struct perf_mmap *map)
 {
u64 head;
u64 old = map->prev;
@@ -102,7 +82,7 @@ union perf_event *perf_mmap__read_forward(struct perf_mmap 
*map, bool check_mess
 
head = perf_mmap__read_head(map);
 
-   return perf_mmap__read(map, check_messup, old, head, >prev);
+   return perf_mmap__read(map, old, head, >prev);
 }
 
 union perf_event *perf_mmap__read_backward(struct perf_mmap *map)
@@ -138,7 +118,7 @@ union perf_event *perf_mmap__read_backward(struct perf_mmap 
*map)
else
end = head + map->mask + 1;
 
-   return perf_mmap__read(map, false, start, end, >prev);
+   return perf_mmap__read(map, start, end, >prev);
 }
 
 void perf_mmap__read_catchup(struct perf_mmap *map)
diff --git a/tools/perf/util/mmap.h b/tools/perf/util/mmap.h
index 2c3d291..d640273 100644
--- a/tools/perf/util/mmap.h
+++ b/tools/perf/util/mmap.h
@@ -86,7 +86,7 @@ static inline void perf_mmap__write_tail(struct perf_mmap 
*md, u64 tail)
pc->data_tail = tail;
 }
 
-union perf_event *perf_mmap__read_forward(struct perf_mmap *map, bool 
check_messup);
+union perf_event *perf_mmap__read_forward(struct perf_mmap *map);
 union perf_event *perf_mmap__read_backward(struct perf_mmap *map);
 
 int perf_mmap__push(struct perf_mmap *md, bool backward,
-- 
2.10.1

[PATCH v2 4/8] perf tools: Remove overwrite from arguments list of perf_mmap__push

[Wang Nan]

'overwrite' argument is always 'false'. Revmove it from arguments
list of perf_mmap__push.

Signed-off-by: Wang Nan 
---
 tools/perf/builtin-record.c | 2 +-
 tools/perf/util/mmap.c  | 6 +++---
 tools/perf/util/mmap.h  | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 3bc6cee..26b8571 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -500,7 +500,7 @@ static int record__mmap_read_evlist(struct record *rec, 
struct perf_evlist *evli
struct auxtrace_mmap *mm = [i].auxtrace_mmap;
 
if (maps[i].base) {
-   if (perf_mmap__push([i], false, backward, rec, 
record__pushfn) != 0) {
+   if (perf_mmap__push([i], backward, rec, 
record__pushfn) != 0) {
rc = -1;
goto out;
}
diff --git a/tools/perf/util/mmap.c b/tools/perf/util/mmap.c
index 9fe5f9c..703ed41 100644
--- a/tools/perf/util/mmap.c
+++ b/tools/perf/util/mmap.c
@@ -299,7 +299,7 @@ static int rb_find_range(void *data, int mask, u64 head, 
u64 old,
return backward_rb_find_range(data, mask, head, start, end);
 }
 
-int perf_mmap__push(struct perf_mmap *md, bool overwrite, bool backward,
+int perf_mmap__push(struct perf_mmap *md, bool backward,
void *to, int push(void *to, void *buf, size_t size))
 {
u64 head = perf_mmap__read_head(md);
@@ -321,7 +321,7 @@ int perf_mmap__push(struct perf_mmap *md, bool overwrite, 
bool backward,
WARN_ONCE(1, "failed to keep up with mmap data. (warn only 
once)\n");
 
md->prev = head;
-   perf_mmap__consume(md, overwrite || backward);
+   perf_mmap__consume(md, backward);
return 0;
}
 
@@ -346,7 +346,7 @@ int perf_mmap__push(struct perf_mmap *md, bool overwrite, 
bool backward,
}
 
md->prev = head;
-   perf_mmap__consume(md, overwrite || backward);
+   perf_mmap__consume(md, backward);
 out:
return rc;
 }
diff --git a/tools/perf/util/mmap.h b/tools/perf/util/mmap.h
index efd78b8..2c3d291 100644
--- a/tools/perf/util/mmap.h
+++ b/tools/perf/util/mmap.h
@@ -89,7 +89,7 @@ static inline void perf_mmap__write_tail(struct perf_mmap 
*md, u64 tail)
 union perf_event *perf_mmap__read_forward(struct perf_mmap *map, bool 
check_messup);
 union perf_event *perf_mmap__read_backward(struct perf_mmap *map);
 
-int perf_mmap__push(struct perf_mmap *md, bool overwrite, bool backward,
+int perf_mmap__push(struct perf_mmap *md, bool backward,
void *to, int push(void *to, void *buf, size_t size));
 
 size_t perf_mmap__mmap_len(struct perf_mmap *map);
-- 
2.10.1

[PATCH v2 4/8] perf tools: Remove overwrite from arguments list of perf_mmap__push

[Wang Nan]

'overwrite' argument is always 'false'. Revmove it from arguments
list of perf_mmap__push.

Signed-off-by: Wang Nan 
---
 tools/perf/builtin-record.c | 2 +-
 tools/perf/util/mmap.c  | 6 +++---
 tools/perf/util/mmap.h  | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 3bc6cee..26b8571 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -500,7 +500,7 @@ static int record__mmap_read_evlist(struct record *rec, 
struct perf_evlist *evli
struct auxtrace_mmap *mm = [i].auxtrace_mmap;
 
if (maps[i].base) {
-   if (perf_mmap__push([i], false, backward, rec, 
record__pushfn) != 0) {
+   if (perf_mmap__push([i], backward, rec, 
record__pushfn) != 0) {
rc = -1;
goto out;
}
diff --git a/tools/perf/util/mmap.c b/tools/perf/util/mmap.c
index 9fe5f9c..703ed41 100644
--- a/tools/perf/util/mmap.c
+++ b/tools/perf/util/mmap.c
@@ -299,7 +299,7 @@ static int rb_find_range(void *data, int mask, u64 head, 
u64 old,
return backward_rb_find_range(data, mask, head, start, end);
 }
 
-int perf_mmap__push(struct perf_mmap *md, bool overwrite, bool backward,
+int perf_mmap__push(struct perf_mmap *md, bool backward,
void *to, int push(void *to, void *buf, size_t size))
 {
u64 head = perf_mmap__read_head(md);
@@ -321,7 +321,7 @@ int perf_mmap__push(struct perf_mmap *md, bool overwrite, 
bool backward,
WARN_ONCE(1, "failed to keep up with mmap data. (warn only 
once)\n");
 
md->prev = head;
-   perf_mmap__consume(md, overwrite || backward);
+   perf_mmap__consume(md, backward);
return 0;
}
 
@@ -346,7 +346,7 @@ int perf_mmap__push(struct perf_mmap *md, bool overwrite, 
bool backward,
}
 
md->prev = head;
-   perf_mmap__consume(md, overwrite || backward);
+   perf_mmap__consume(md, backward);
 out:
return rc;
 }
diff --git a/tools/perf/util/mmap.h b/tools/perf/util/mmap.h
index efd78b8..2c3d291 100644
--- a/tools/perf/util/mmap.h
+++ b/tools/perf/util/mmap.h
@@ -89,7 +89,7 @@ static inline void perf_mmap__write_tail(struct perf_mmap 
*md, u64 tail)
 union perf_event *perf_mmap__read_forward(struct perf_mmap *map, bool 
check_messup);
 union perf_event *perf_mmap__read_backward(struct perf_mmap *map);
 
-int perf_mmap__push(struct perf_mmap *md, bool overwrite, bool backward,
+int perf_mmap__push(struct perf_mmap *md, bool backward,
void *to, int push(void *to, void *buf, size_t size));
 
 size_t perf_mmap__mmap_len(struct perf_mmap *map);
-- 
2.10.1

[PATCH v2 2/8] perf tools: Remove 'overwrite' parameter from perf_evlist__mmap_ex

[Wang Nan]

All users of perf_evlist__mmap_ex set !overwrite. Remove it from its
arguments list.

Signed-off-by: Wang Nan 
---
 tools/perf/builtin-record.c | 2 +-
 tools/perf/util/evlist.c| 8 
 tools/perf/util/evlist.h| 2 +-
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index e304bc4..08070f8 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -301,7 +301,7 @@ static int record__mmap_evlist(struct record *rec,
struct record_opts *opts = >opts;
char msg[512];
 
-   if (perf_evlist__mmap_ex(evlist, opts->mmap_pages, false,
+   if (perf_evlist__mmap_ex(evlist, opts->mmap_pages,
 opts->auxtrace_mmap_pages,
 opts->auxtrace_snapshot_mode) < 0) {
if (errno == EPERM) {
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index 3c1778b..93272d9 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -1052,14 +1052,14 @@ int perf_evlist__parse_mmap_pages(const struct option 
*opt, const char *str,
  * Return: %0 on success, negative error code otherwise.
  */
 int perf_evlist__mmap_ex(struct perf_evlist *evlist, unsigned int pages,
-bool overwrite, unsigned int auxtrace_pages,
+unsigned int auxtrace_pages,
 bool auxtrace_overwrite)
 {
struct perf_evsel *evsel;
const struct cpu_map *cpus = evlist->cpus;
const struct thread_map *threads = evlist->threads;
struct mmap_params mp = {
-   .prot = PROT_READ | (overwrite ? 0 : PROT_WRITE),
+   .prot = PROT_READ | PROT_WRITE,
};
 
if (!evlist->mmap)
@@ -1070,7 +1070,7 @@ int perf_evlist__mmap_ex(struct perf_evlist *evlist, 
unsigned int pages,
if (evlist->pollfd.entries == NULL && perf_evlist__alloc_pollfd(evlist) 
< 0)
return -ENOMEM;
 
-   evlist->overwrite = overwrite;
+   evlist->overwrite = false;
evlist->mmap_len = perf_evlist__mmap_size(pages);
pr_debug("mmap size %zuB\n", evlist->mmap_len);
mp.mask = evlist->mmap_len - page_size - 1;
@@ -1093,7 +1093,7 @@ int perf_evlist__mmap_ex(struct perf_evlist *evlist, 
unsigned int pages,
 
 int perf_evlist__mmap(struct perf_evlist *evlist, unsigned int pages)
 {
-   return perf_evlist__mmap_ex(evlist, pages, false, 0, false);
+   return perf_evlist__mmap_ex(evlist, pages, 0, false);
 }
 
 int perf_evlist__create_maps(struct perf_evlist *evlist, struct target *target)
diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h
index f0f2c8b..424a3d6 100644
--- a/tools/perf/util/evlist.h
+++ b/tools/perf/util/evlist.h
@@ -169,7 +169,7 @@ int perf_evlist__parse_mmap_pages(const struct option *opt,
 unsigned long perf_event_mlock_kb_in_pages(void);
 
 int perf_evlist__mmap_ex(struct perf_evlist *evlist, unsigned int pages,
-bool overwrite, unsigned int auxtrace_pages,
+unsigned int auxtrace_pages,
 bool auxtrace_overwrite);
 int perf_evlist__mmap(struct perf_evlist *evlist, unsigned int pages);
 void perf_evlist__munmap(struct perf_evlist *evlist);
-- 
2.10.1

[PATCH v2 2/8] perf tools: Remove 'overwrite' parameter from perf_evlist__mmap_ex

[Wang Nan]

All users of perf_evlist__mmap_ex set !overwrite. Remove it from its
arguments list.

Signed-off-by: Wang Nan 
---
 tools/perf/builtin-record.c | 2 +-
 tools/perf/util/evlist.c| 8 
 tools/perf/util/evlist.h| 2 +-
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index e304bc4..08070f8 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -301,7 +301,7 @@ static int record__mmap_evlist(struct record *rec,
struct record_opts *opts = >opts;
char msg[512];
 
-   if (perf_evlist__mmap_ex(evlist, opts->mmap_pages, false,
+   if (perf_evlist__mmap_ex(evlist, opts->mmap_pages,
 opts->auxtrace_mmap_pages,
 opts->auxtrace_snapshot_mode) < 0) {
if (errno == EPERM) {
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index 3c1778b..93272d9 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -1052,14 +1052,14 @@ int perf_evlist__parse_mmap_pages(const struct option 
*opt, const char *str,
  * Return: %0 on success, negative error code otherwise.
  */
 int perf_evlist__mmap_ex(struct perf_evlist *evlist, unsigned int pages,
-bool overwrite, unsigned int auxtrace_pages,
+unsigned int auxtrace_pages,
 bool auxtrace_overwrite)
 {
struct perf_evsel *evsel;
const struct cpu_map *cpus = evlist->cpus;
const struct thread_map *threads = evlist->threads;
struct mmap_params mp = {
-   .prot = PROT_READ | (overwrite ? 0 : PROT_WRITE),
+   .prot = PROT_READ | PROT_WRITE,
};
 
if (!evlist->mmap)
@@ -1070,7 +1070,7 @@ int perf_evlist__mmap_ex(struct perf_evlist *evlist, 
unsigned int pages,
if (evlist->pollfd.entries == NULL && perf_evlist__alloc_pollfd(evlist) 
< 0)
return -ENOMEM;
 
-   evlist->overwrite = overwrite;
+   evlist->overwrite = false;
evlist->mmap_len = perf_evlist__mmap_size(pages);
pr_debug("mmap size %zuB\n", evlist->mmap_len);
mp.mask = evlist->mmap_len - page_size - 1;
@@ -1093,7 +1093,7 @@ int perf_evlist__mmap_ex(struct perf_evlist *evlist, 
unsigned int pages,
 
 int perf_evlist__mmap(struct perf_evlist *evlist, unsigned int pages)
 {
-   return perf_evlist__mmap_ex(evlist, pages, false, 0, false);
+   return perf_evlist__mmap_ex(evlist, pages, 0, false);
 }
 
 int perf_evlist__create_maps(struct perf_evlist *evlist, struct target *target)
diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h
index f0f2c8b..424a3d6 100644
--- a/tools/perf/util/evlist.h
+++ b/tools/perf/util/evlist.h
@@ -169,7 +169,7 @@ int perf_evlist__parse_mmap_pages(const struct option *opt,
 unsigned long perf_event_mlock_kb_in_pages(void);
 
 int perf_evlist__mmap_ex(struct perf_evlist *evlist, unsigned int pages,
-bool overwrite, unsigned int auxtrace_pages,
+unsigned int auxtrace_pages,
 bool auxtrace_overwrite);
 int perf_evlist__mmap(struct perf_evlist *evlist, unsigned int pages);
 void perf_evlist__munmap(struct perf_evlist *evlist);
-- 
2.10.1

[PATCH v2 8/8] perf tools: Replace 'backward' to 'overwrite' in evlist, mmap and record

[Wang Nan]

Remove the backward/forward concept to make it uniform with user
interface (the '--overwrite' option).

Signed-off-by: Wang Nan 
---
 tools/perf/builtin-record.c | 14 +++---
 tools/perf/tests/backward-ring-buffer.c |  4 ++--
 tools/perf/util/evlist.c| 30 +++---
 tools/perf/util/evlist.h|  2 +-
 tools/perf/util/mmap.c  | 22 +++---
 5 files changed, 36 insertions(+), 36 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 26b8571..0a5749e 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -479,7 +479,7 @@ static struct perf_event_header finished_round_event = {
 };
 
 static int record__mmap_read_evlist(struct record *rec, struct perf_evlist 
*evlist,
-   bool backward)
+   bool overwrite)
 {
u64 bytes_written = rec->bytes_written;
int i;
@@ -489,18 +489,18 @@ static int record__mmap_read_evlist(struct record *rec, 
struct perf_evlist *evli
if (!evlist)
return 0;
 
-   maps = backward ? evlist->backward_mmap : evlist->mmap;
+   maps = overwrite ? evlist->overwrite_mmap : evlist->mmap;
if (!maps)
return 0;
 
-   if (backward && evlist->bkw_mmap_state != BKW_MMAP_DATA_PENDING)
+   if (overwrite && evlist->bkw_mmap_state != BKW_MMAP_DATA_PENDING)
return 0;
 
for (i = 0; i < evlist->nr_mmaps; i++) {
struct auxtrace_mmap *mm = [i].auxtrace_mmap;
 
if (maps[i].base) {
-   if (perf_mmap__push([i], backward, rec, 
record__pushfn) != 0) {
+   if (perf_mmap__push([i], overwrite, rec, 
record__pushfn) != 0) {
rc = -1;
goto out;
}
@@ -520,7 +520,7 @@ static int record__mmap_read_evlist(struct record *rec, 
struct perf_evlist *evli
if (bytes_written != rec->bytes_written)
rc = record__write(rec, _round_event, 
sizeof(finished_round_event));
 
-   if (backward)
+   if (overwrite)
perf_evlist__toggle_bkw_mmap(evlist, BKW_MMAP_EMPTY);
 out:
return rc;
@@ -692,8 +692,8 @@ perf_evlist__pick_pc(struct perf_evlist *evlist)
if (evlist) {
if (evlist->mmap && evlist->mmap[0].base)
return evlist->mmap[0].base;
-   if (evlist->backward_mmap && evlist->backward_mmap[0].base)
-   return evlist->backward_mmap[0].base;
+   if (evlist->overwrite_mmap && evlist->overwrite_mmap[0].base)
+   return evlist->overwrite_mmap[0].base;
}
return NULL;
 }
diff --git a/tools/perf/tests/backward-ring-buffer.c 
b/tools/perf/tests/backward-ring-buffer.c
index cf37e43..4035d43 100644
--- a/tools/perf/tests/backward-ring-buffer.c
+++ b/tools/perf/tests/backward-ring-buffer.c
@@ -33,8 +33,8 @@ static int count_samples(struct perf_evlist *evlist, int 
*sample_count,
for (i = 0; i < evlist->nr_mmaps; i++) {
union perf_event *event;
 
-   perf_mmap__read_catchup(>backward_mmap[i]);
-   while ((event = 
perf_mmap__read_backward(>backward_mmap[i])) != NULL) {
+   perf_mmap__read_catchup(>overwrite_mmap[i]);
+   while ((event = 
perf_mmap__read_backward(>overwrite_mmap[i])) != NULL) {
const u32 type = event->header.type;
 
switch (type) {
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index bb70aef..2774528a 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -125,7 +125,7 @@ static void perf_evlist__purge(struct perf_evlist *evlist)
 void perf_evlist__exit(struct perf_evlist *evlist)
 {
zfree(>mmap);
-   zfree(>backward_mmap);
+   zfree(>overwrite_mmap);
fdarray__exit(>pollfd);
 }
 
@@ -675,11 +675,11 @@ static int perf_evlist__set_paused(struct perf_evlist 
*evlist, bool value)
 {
int i;
 
-   if (!evlist->backward_mmap)
+   if (!evlist->overwrite_mmap)
return 0;
 
for (i = 0; i < evlist->nr_mmaps; i++) {
-   int fd = evlist->backward_mmap[i].fd;
+   int fd = evlist->overwrite_mmap[i].fd;
int err;
 
if (fd < 0)
@@ -749,16 +749,16 @@ static void perf_evlist__munmap_nofree(struct perf_evlist 
*evlist)
for (i = 0; i < evlist->nr_mmaps; i++)
perf_mmap__munmap(>mmap[i]);
 
-   if (evlist->backward_mmap)
+   if (evlist->overwrite_mmap)
for (i = 0; i < evlist->nr_mmaps; i++)
-   perf_mmap__munmap(>backward_mmap[i]);
+   perf_mmap__munmap(>overwrite_mmap[i]);
 }
 
 void perf_evlist__munmap(struct 

[PATCH v2 8/8] perf tools: Replace 'backward' to 'overwrite' in evlist, mmap and record

[Wang Nan]

Remove the backward/forward concept to make it uniform with user
interface (the '--overwrite' option).

Signed-off-by: Wang Nan 
---
 tools/perf/builtin-record.c | 14 +++---
 tools/perf/tests/backward-ring-buffer.c |  4 ++--
 tools/perf/util/evlist.c| 30 +++---
 tools/perf/util/evlist.h|  2 +-
 tools/perf/util/mmap.c  | 22 +++---
 5 files changed, 36 insertions(+), 36 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 26b8571..0a5749e 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -479,7 +479,7 @@ static struct perf_event_header finished_round_event = {
 };
 
 static int record__mmap_read_evlist(struct record *rec, struct perf_evlist 
*evlist,
-   bool backward)
+   bool overwrite)
 {
u64 bytes_written = rec->bytes_written;
int i;
@@ -489,18 +489,18 @@ static int record__mmap_read_evlist(struct record *rec, 
struct perf_evlist *evli
if (!evlist)
return 0;
 
-   maps = backward ? evlist->backward_mmap : evlist->mmap;
+   maps = overwrite ? evlist->overwrite_mmap : evlist->mmap;
if (!maps)
return 0;
 
-   if (backward && evlist->bkw_mmap_state != BKW_MMAP_DATA_PENDING)
+   if (overwrite && evlist->bkw_mmap_state != BKW_MMAP_DATA_PENDING)
return 0;
 
for (i = 0; i < evlist->nr_mmaps; i++) {
struct auxtrace_mmap *mm = [i].auxtrace_mmap;
 
if (maps[i].base) {
-   if (perf_mmap__push([i], backward, rec, 
record__pushfn) != 0) {
+   if (perf_mmap__push([i], overwrite, rec, 
record__pushfn) != 0) {
rc = -1;
goto out;
}
@@ -520,7 +520,7 @@ static int record__mmap_read_evlist(struct record *rec, 
struct perf_evlist *evli
if (bytes_written != rec->bytes_written)
rc = record__write(rec, _round_event, 
sizeof(finished_round_event));
 
-   if (backward)
+   if (overwrite)
perf_evlist__toggle_bkw_mmap(evlist, BKW_MMAP_EMPTY);
 out:
return rc;
@@ -692,8 +692,8 @@ perf_evlist__pick_pc(struct perf_evlist *evlist)
if (evlist) {
if (evlist->mmap && evlist->mmap[0].base)
return evlist->mmap[0].base;
-   if (evlist->backward_mmap && evlist->backward_mmap[0].base)
-   return evlist->backward_mmap[0].base;
+   if (evlist->overwrite_mmap && evlist->overwrite_mmap[0].base)
+   return evlist->overwrite_mmap[0].base;
}
return NULL;
 }
diff --git a/tools/perf/tests/backward-ring-buffer.c 
b/tools/perf/tests/backward-ring-buffer.c
index cf37e43..4035d43 100644
--- a/tools/perf/tests/backward-ring-buffer.c
+++ b/tools/perf/tests/backward-ring-buffer.c
@@ -33,8 +33,8 @@ static int count_samples(struct perf_evlist *evlist, int 
*sample_count,
for (i = 0; i < evlist->nr_mmaps; i++) {
union perf_event *event;
 
-   perf_mmap__read_catchup(>backward_mmap[i]);
-   while ((event = 
perf_mmap__read_backward(>backward_mmap[i])) != NULL) {
+   perf_mmap__read_catchup(>overwrite_mmap[i]);
+   while ((event = 
perf_mmap__read_backward(>overwrite_mmap[i])) != NULL) {
const u32 type = event->header.type;
 
switch (type) {
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index bb70aef..2774528a 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -125,7 +125,7 @@ static void perf_evlist__purge(struct perf_evlist *evlist)
 void perf_evlist__exit(struct perf_evlist *evlist)
 {
zfree(>mmap);
-   zfree(>backward_mmap);
+   zfree(>overwrite_mmap);
fdarray__exit(>pollfd);
 }
 
@@ -675,11 +675,11 @@ static int perf_evlist__set_paused(struct perf_evlist 
*evlist, bool value)
 {
int i;
 
-   if (!evlist->backward_mmap)
+   if (!evlist->overwrite_mmap)
return 0;
 
for (i = 0; i < evlist->nr_mmaps; i++) {
-   int fd = evlist->backward_mmap[i].fd;
+   int fd = evlist->overwrite_mmap[i].fd;
int err;
 
if (fd < 0)
@@ -749,16 +749,16 @@ static void perf_evlist__munmap_nofree(struct perf_evlist 
*evlist)
for (i = 0; i < evlist->nr_mmaps; i++)
perf_mmap__munmap(>mmap[i]);
 
-   if (evlist->backward_mmap)
+   if (evlist->overwrite_mmap)
for (i = 0; i < evlist->nr_mmaps; i++)
-   perf_mmap__munmap(>backward_mmap[i]);
+   perf_mmap__munmap(>overwrite_mmap[i]);
 }
 
 void perf_evlist__munmap(struct perf_evlist *evlist)
 {

[PATCH v2 1/8] perf tools: Remove 'overwrite' parameter from perf_evlist__mmap

[Wang Nan]

Now all perf_evlist__mmap's users doesn't set 'overwrite'. Remove it from
arguments list.

Signed-off-by: Wang Nan 
---
 tools/perf/arch/x86/tests/perf-time-to-tsc.c | 2 +-
 tools/perf/builtin-kvm.c | 2 +-
 tools/perf/builtin-top.c | 2 +-
 tools/perf/builtin-trace.c   | 2 +-
 tools/perf/tests/backward-ring-buffer.c  | 2 +-
 tools/perf/tests/bpf.c   | 2 +-
 tools/perf/tests/code-reading.c  | 2 +-
 tools/perf/tests/keep-tracking.c | 2 +-
 tools/perf/tests/mmap-basic.c| 2 +-
 tools/perf/tests/openat-syscall-tp-fields.c  | 2 +-
 tools/perf/tests/perf-record.c   | 2 +-
 tools/perf/tests/sw-clock.c  | 2 +-
 tools/perf/tests/switch-tracking.c   | 2 +-
 tools/perf/tests/task-exit.c | 2 +-
 tools/perf/util/evlist.c | 5 ++---
 tools/perf/util/evlist.h | 3 +--
 tools/perf/util/python.c | 2 +-
 17 files changed, 18 insertions(+), 20 deletions(-)

diff --git a/tools/perf/arch/x86/tests/perf-time-to-tsc.c 
b/tools/perf/arch/x86/tests/perf-time-to-tsc.c
index b59678e..06abe81 100644
--- a/tools/perf/arch/x86/tests/perf-time-to-tsc.c
+++ b/tools/perf/arch/x86/tests/perf-time-to-tsc.c
@@ -84,7 +84,7 @@ int test__perf_time_to_tsc(struct test *test __maybe_unused, 
int subtest __maybe
 
CHECK__(perf_evlist__open(evlist));
 
-   CHECK__(perf_evlist__mmap(evlist, UINT_MAX, false));
+   CHECK__(perf_evlist__mmap(evlist, UINT_MAX));
 
pc = evlist->mmap[0].base;
ret = perf_read_tsc_conversion(pc, );
diff --git a/tools/perf/builtin-kvm.c b/tools/perf/builtin-kvm.c
index 597c7de..9885316 100644
--- a/tools/perf/builtin-kvm.c
+++ b/tools/perf/builtin-kvm.c
@@ -1044,7 +1044,7 @@ static int kvm_live_open_events(struct perf_kvm_stat *kvm)
goto out;
}
 
-   if (perf_evlist__mmap(evlist, kvm->opts.mmap_pages, false) < 0) {
+   if (perf_evlist__mmap(evlist, kvm->opts.mmap_pages) < 0) {
ui__error("Failed to mmap the events: %s\n",
  str_error_r(errno, sbuf, sizeof(sbuf)));
perf_evlist__close(evlist);
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 0077724..540461f 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -907,7 +907,7 @@ static int perf_top__start_counters(struct perf_top *top)
}
}
 
-   if (perf_evlist__mmap(evlist, opts->mmap_pages, false) < 0) {
+   if (perf_evlist__mmap(evlist, opts->mmap_pages) < 0) {
ui__error("Failed to mmap with %d (%s)\n",
errno, str_error_r(errno, msg, sizeof(msg)));
goto out_err;
diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
index 84debdb..7c57898 100644
--- a/tools/perf/builtin-trace.c
+++ b/tools/perf/builtin-trace.c
@@ -2437,7 +2437,7 @@ static int trace__run(struct trace *trace, int argc, 
const char **argv)
if (err < 0)
goto out_error_apply_filters;
 
-   err = perf_evlist__mmap(evlist, trace->opts.mmap_pages, false);
+   err = perf_evlist__mmap(evlist, trace->opts.mmap_pages);
if (err < 0)
goto out_error_mmap;
 
diff --git a/tools/perf/tests/backward-ring-buffer.c 
b/tools/perf/tests/backward-ring-buffer.c
index 43a8c6a..cf37e43 100644
--- a/tools/perf/tests/backward-ring-buffer.c
+++ b/tools/perf/tests/backward-ring-buffer.c
@@ -59,7 +59,7 @@ static int do_test(struct perf_evlist *evlist, int mmap_pages,
int err;
char sbuf[STRERR_BUFSIZE];
 
-   err = perf_evlist__mmap(evlist, mmap_pages, false);
+   err = perf_evlist__mmap(evlist, mmap_pages);
if (err < 0) {
pr_debug("perf_evlist__mmap: %s\n",
 str_error_r(errno, sbuf, sizeof(sbuf)));
diff --git a/tools/perf/tests/bpf.c b/tools/perf/tests/bpf.c
index 34c22cd..c433dd3 100644
--- a/tools/perf/tests/bpf.c
+++ b/tools/perf/tests/bpf.c
@@ -167,7 +167,7 @@ static int do_test(struct bpf_object *obj, int 
(*func)(void),
goto out_delete_evlist;
}
 
-   err = perf_evlist__mmap(evlist, opts.mmap_pages, false);
+   err = perf_evlist__mmap(evlist, opts.mmap_pages);
if (err < 0) {
pr_debug("perf_evlist__mmap: %s\n",
 str_error_r(errno, sbuf, sizeof(sbuf)));
diff --git a/tools/perf/tests/code-reading.c b/tools/perf/tests/code-reading.c
index fcc8984..3bf7b14 100644
--- a/tools/perf/tests/code-reading.c
+++ b/tools/perf/tests/code-reading.c
@@ -639,7 +639,7 @@ static int do_test_code_reading(bool try_kcore)
break;
}
 
-   ret = perf_evlist__mmap(evlist, UINT_MAX, false);
+   ret = perf_evlist__mmap(evlist, UINT_MAX);
if (ret < 0) {
pr_debug("perf_evlist__mmap failed\n");

[PATCH v2 3/8] perf tools: Remove evlist->overwrite

[Wang Nan]

evlist->overwrite is set to false in all users. It can be removed.

Signed-off-by: Wang Nan 
---
 tools/perf/builtin-record.c | 2 +-
 tools/perf/util/evlist.c| 5 ++---
 tools/perf/util/evlist.h| 1 -
 3 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 08070f8..3bc6cee 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -500,7 +500,7 @@ static int record__mmap_read_evlist(struct record *rec, 
struct perf_evlist *evli
struct auxtrace_mmap *mm = [i].auxtrace_mmap;
 
if (maps[i].base) {
-   if (perf_mmap__push([i], evlist->overwrite, 
backward, rec, record__pushfn) != 0) {
+   if (perf_mmap__push([i], false, backward, rec, 
record__pushfn) != 0) {
rc = -1;
goto out;
}
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index 93272d9..a59134f 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -711,7 +711,7 @@ union perf_event *perf_evlist__mmap_read_forward(struct 
perf_evlist *evlist, int
 * No need for read-write ring buffer: kernel stop outputting when
 * it hit md->prev (perf_mmap__consume()).
 */
-   return perf_mmap__read_forward(md, evlist->overwrite);
+   return perf_mmap__read_forward(md, false);
 }
 
 union perf_event *perf_evlist__mmap_read_backward(struct perf_evlist *evlist, 
int idx)
@@ -738,7 +738,7 @@ void perf_evlist__mmap_read_catchup(struct perf_evlist 
*evlist, int idx)
 
 void perf_evlist__mmap_consume(struct perf_evlist *evlist, int idx)
 {
-   perf_mmap__consume(>mmap[idx], evlist->overwrite);
+   perf_mmap__consume(>mmap[idx], false);
 }
 
 static void perf_evlist__munmap_nofree(struct perf_evlist *evlist)
@@ -1070,7 +1070,6 @@ int perf_evlist__mmap_ex(struct perf_evlist *evlist, 
unsigned int pages,
if (evlist->pollfd.entries == NULL && perf_evlist__alloc_pollfd(evlist) 
< 0)
return -ENOMEM;
 
-   evlist->overwrite = false;
evlist->mmap_len = perf_evlist__mmap_size(pages);
pr_debug("mmap size %zuB\n", evlist->mmap_len);
mp.mask = evlist->mmap_len - page_size - 1;
diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h
index 424a3d6..eec3377 100644
--- a/tools/perf/util/evlist.h
+++ b/tools/perf/util/evlist.h
@@ -31,7 +31,6 @@ struct perf_evlist {
int  nr_entries;
int  nr_groups;
int  nr_mmaps;
-   bool overwrite;
bool enabled;
bool has_user_cpus;
size_t   mmap_len;
-- 
2.10.1

[PATCH v2 1/8] perf tools: Remove 'overwrite' parameter from perf_evlist__mmap

[Wang Nan]

Now all perf_evlist__mmap's users doesn't set 'overwrite'. Remove it from
arguments list.

Signed-off-by: Wang Nan 
---
 tools/perf/arch/x86/tests/perf-time-to-tsc.c | 2 +-
 tools/perf/builtin-kvm.c | 2 +-
 tools/perf/builtin-top.c | 2 +-
 tools/perf/builtin-trace.c   | 2 +-
 tools/perf/tests/backward-ring-buffer.c  | 2 +-
 tools/perf/tests/bpf.c   | 2 +-
 tools/perf/tests/code-reading.c  | 2 +-
 tools/perf/tests/keep-tracking.c | 2 +-
 tools/perf/tests/mmap-basic.c| 2 +-
 tools/perf/tests/openat-syscall-tp-fields.c  | 2 +-
 tools/perf/tests/perf-record.c   | 2 +-
 tools/perf/tests/sw-clock.c  | 2 +-
 tools/perf/tests/switch-tracking.c   | 2 +-
 tools/perf/tests/task-exit.c | 2 +-
 tools/perf/util/evlist.c | 5 ++---
 tools/perf/util/evlist.h | 3 +--
 tools/perf/util/python.c | 2 +-
 17 files changed, 18 insertions(+), 20 deletions(-)

diff --git a/tools/perf/arch/x86/tests/perf-time-to-tsc.c 
b/tools/perf/arch/x86/tests/perf-time-to-tsc.c
index b59678e..06abe81 100644
--- a/tools/perf/arch/x86/tests/perf-time-to-tsc.c
+++ b/tools/perf/arch/x86/tests/perf-time-to-tsc.c
@@ -84,7 +84,7 @@ int test__perf_time_to_tsc(struct test *test __maybe_unused, 
int subtest __maybe
 
CHECK__(perf_evlist__open(evlist));
 
-   CHECK__(perf_evlist__mmap(evlist, UINT_MAX, false));
+   CHECK__(perf_evlist__mmap(evlist, UINT_MAX));
 
pc = evlist->mmap[0].base;
ret = perf_read_tsc_conversion(pc, );
diff --git a/tools/perf/builtin-kvm.c b/tools/perf/builtin-kvm.c
index 597c7de..9885316 100644
--- a/tools/perf/builtin-kvm.c
+++ b/tools/perf/builtin-kvm.c
@@ -1044,7 +1044,7 @@ static int kvm_live_open_events(struct perf_kvm_stat *kvm)
goto out;
}
 
-   if (perf_evlist__mmap(evlist, kvm->opts.mmap_pages, false) < 0) {
+   if (perf_evlist__mmap(evlist, kvm->opts.mmap_pages) < 0) {
ui__error("Failed to mmap the events: %s\n",
  str_error_r(errno, sbuf, sizeof(sbuf)));
perf_evlist__close(evlist);
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 0077724..540461f 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -907,7 +907,7 @@ static int perf_top__start_counters(struct perf_top *top)
}
}
 
-   if (perf_evlist__mmap(evlist, opts->mmap_pages, false) < 0) {
+   if (perf_evlist__mmap(evlist, opts->mmap_pages) < 0) {
ui__error("Failed to mmap with %d (%s)\n",
errno, str_error_r(errno, msg, sizeof(msg)));
goto out_err;
diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
index 84debdb..7c57898 100644
--- a/tools/perf/builtin-trace.c
+++ b/tools/perf/builtin-trace.c
@@ -2437,7 +2437,7 @@ static int trace__run(struct trace *trace, int argc, 
const char **argv)
if (err < 0)
goto out_error_apply_filters;
 
-   err = perf_evlist__mmap(evlist, trace->opts.mmap_pages, false);
+   err = perf_evlist__mmap(evlist, trace->opts.mmap_pages);
if (err < 0)
goto out_error_mmap;
 
diff --git a/tools/perf/tests/backward-ring-buffer.c 
b/tools/perf/tests/backward-ring-buffer.c
index 43a8c6a..cf37e43 100644
--- a/tools/perf/tests/backward-ring-buffer.c
+++ b/tools/perf/tests/backward-ring-buffer.c
@@ -59,7 +59,7 @@ static int do_test(struct perf_evlist *evlist, int mmap_pages,
int err;
char sbuf[STRERR_BUFSIZE];
 
-   err = perf_evlist__mmap(evlist, mmap_pages, false);
+   err = perf_evlist__mmap(evlist, mmap_pages);
if (err < 0) {
pr_debug("perf_evlist__mmap: %s\n",
 str_error_r(errno, sbuf, sizeof(sbuf)));
diff --git a/tools/perf/tests/bpf.c b/tools/perf/tests/bpf.c
index 34c22cd..c433dd3 100644
--- a/tools/perf/tests/bpf.c
+++ b/tools/perf/tests/bpf.c
@@ -167,7 +167,7 @@ static int do_test(struct bpf_object *obj, int 
(*func)(void),
goto out_delete_evlist;
}
 
-   err = perf_evlist__mmap(evlist, opts.mmap_pages, false);
+   err = perf_evlist__mmap(evlist, opts.mmap_pages);
if (err < 0) {
pr_debug("perf_evlist__mmap: %s\n",
 str_error_r(errno, sbuf, sizeof(sbuf)));
diff --git a/tools/perf/tests/code-reading.c b/tools/perf/tests/code-reading.c
index fcc8984..3bf7b14 100644
--- a/tools/perf/tests/code-reading.c
+++ b/tools/perf/tests/code-reading.c
@@ -639,7 +639,7 @@ static int do_test_code_reading(bool try_kcore)
break;
}
 
-   ret = perf_evlist__mmap(evlist, UINT_MAX, false);
+   ret = perf_evlist__mmap(evlist, UINT_MAX);
if (ret < 0) {
pr_debug("perf_evlist__mmap failed\n");
goto 

[PATCH v2 3/8] perf tools: Remove evlist->overwrite

[Wang Nan]

evlist->overwrite is set to false in all users. It can be removed.

Signed-off-by: Wang Nan 
---
 tools/perf/builtin-record.c | 2 +-
 tools/perf/util/evlist.c| 5 ++---
 tools/perf/util/evlist.h| 1 -
 3 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 08070f8..3bc6cee 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -500,7 +500,7 @@ static int record__mmap_read_evlist(struct record *rec, 
struct perf_evlist *evli
struct auxtrace_mmap *mm = [i].auxtrace_mmap;
 
if (maps[i].base) {
-   if (perf_mmap__push([i], evlist->overwrite, 
backward, rec, record__pushfn) != 0) {
+   if (perf_mmap__push([i], false, backward, rec, 
record__pushfn) != 0) {
rc = -1;
goto out;
}
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index 93272d9..a59134f 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -711,7 +711,7 @@ union perf_event *perf_evlist__mmap_read_forward(struct 
perf_evlist *evlist, int
 * No need for read-write ring buffer: kernel stop outputting when
 * it hit md->prev (perf_mmap__consume()).
 */
-   return perf_mmap__read_forward(md, evlist->overwrite);
+   return perf_mmap__read_forward(md, false);
 }
 
 union perf_event *perf_evlist__mmap_read_backward(struct perf_evlist *evlist, 
int idx)
@@ -738,7 +738,7 @@ void perf_evlist__mmap_read_catchup(struct perf_evlist 
*evlist, int idx)
 
 void perf_evlist__mmap_consume(struct perf_evlist *evlist, int idx)
 {
-   perf_mmap__consume(>mmap[idx], evlist->overwrite);
+   perf_mmap__consume(>mmap[idx], false);
 }
 
 static void perf_evlist__munmap_nofree(struct perf_evlist *evlist)
@@ -1070,7 +1070,6 @@ int perf_evlist__mmap_ex(struct perf_evlist *evlist, 
unsigned int pages,
if (evlist->pollfd.entries == NULL && perf_evlist__alloc_pollfd(evlist) 
< 0)
return -ENOMEM;
 
-   evlist->overwrite = false;
evlist->mmap_len = perf_evlist__mmap_size(pages);
pr_debug("mmap size %zuB\n", evlist->mmap_len);
mp.mask = evlist->mmap_len - page_size - 1;
diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h
index 424a3d6..eec3377 100644
--- a/tools/perf/util/evlist.h
+++ b/tools/perf/util/evlist.h
@@ -31,7 +31,6 @@ struct perf_evlist {
int  nr_entries;
int  nr_groups;
int  nr_mmaps;
-   bool overwrite;
bool enabled;
bool has_user_cpus;
size_t   mmap_len;
-- 
2.10.1

Re: [PATCH v18 05/10] xbitmap: add more operations

[Tetsuo Handa]

Matthew Wilcox wrote:
> On Fri, Dec 01, 2017 at 03:09:08PM +, Wang, Wei W wrote:
> > On Friday, December 1, 2017 9:02 PM, Tetsuo Handa wrote:
> > > If start == end is legal,
> > > 
> > >for (; start < end; start = (start | (IDA_BITMAP_BITS - 1)) + 1) {
> > > 
> > > makes this loop do nothing because 10 < 10 is false.
> > 
> > How about "start <= end "?
> 
> Don't ask Tetsuo for his opinion, write some userspace code that uses it.
> 

Please be sure to prepare for "end == -1UL" case, for "start < end" will become
true when "start = (start | (IDA_BITMAP_BITS - 1)) + 1" made "start == 0" due to
overflow.

Re: [PATCH v18 05/10] xbitmap: add more operations

[Tetsuo Handa]

Matthew Wilcox wrote:
> On Fri, Dec 01, 2017 at 03:09:08PM +, Wang, Wei W wrote:
> > On Friday, December 1, 2017 9:02 PM, Tetsuo Handa wrote:
> > > If start == end is legal,
> > > 
> > >for (; start < end; start = (start | (IDA_BITMAP_BITS - 1)) + 1) {
> > > 
> > > makes this loop do nothing because 10 < 10 is false.
> > 
> > How about "start <= end "?
> 
> Don't ask Tetsuo for his opinion, write some userspace code that uses it.
> 

Please be sure to prepare for "end == -1UL" case, for "start < end" will become
true when "start = (start | (IDA_BITMAP_BITS - 1)) + 1" made "start == 0" due to
overflow.

Re: [PATCH v18 05/10] xbitmap: add more operations

[Tetsuo Handa]

Matthew Wilcox wrote:
> On Thu, Nov 30, 2017 at 10:35:03PM +0900, Tetsuo Handa wrote:
> > According to xb_set_bit(), it seems to me that we are trying to avoid 
> > memory allocation
> > for "struct ida_bitmap" when all set bits within a 1024-bits bitmap reside 
> > in the first
> > 61 bits.
> > 
> > But does such saving help? Is there characteristic bias that majority of 
> > set bits resides
> > in the first 61 bits, for "bit" is "unsigned long" which holds a page 
> > number (isn't it)?
> > If no such bias, wouldn't eliminating radix_tree_exception() case and 
> > always storing
> > "struct ida_bitmap" simplifies the code (and make the processing faster)?
> 
> It happens all the time.  The vast majority of users of the IDA set
> low bits.  Also, it's the first 62 bits -- going up to 63 bits with the
> XArray rewrite.

Oops, 0...61 is 62 bits.

What I meant is below (untested) patch. If correct, it can save lines and make
the code easier to read.

 lib/radix-tree.c | 20 +
 lib/xbitmap.c| 88 +---
 2 files changed, 8 insertions(+), 100 deletions(-)

diff --git a/lib/radix-tree.c b/lib/radix-tree.c
index a039588..fb84b91 100644
--- a/lib/radix-tree.c
+++ b/lib/radix-tree.c
@@ -2152,25 +2152,7 @@ int ida_pre_get(struct ida *ida, gfp_t gfp)
  */
 __must_check bool xb_preload(gfp_t gfp)
 {
-   if (!this_cpu_read(ida_bitmap)) {
-   struct ida_bitmap *bitmap = kmalloc(sizeof(*bitmap), gfp);
-
-   if (!bitmap)
-   return false;
-   /*
-* The per-CPU variable is updated with preemption enabled.
-* If the calling task is unlucky to be scheduled to another
-* CPU which has no ida_bitmap allocation, it will be detected
-* when setting a bit (i.e. __xb_set_bit()).
-*/
-   bitmap = this_cpu_cmpxchg(ida_bitmap, NULL, bitmap);
-   kfree(bitmap);
-   }
-
-   if (__radix_tree_preload(gfp, XB_PRELOAD_SIZE) < 0)
-   return false;
-
-   return true;
+   return __radix_tree_preload(gfp, XB_PRELOAD_SIZE) == 0;
 }
 EXPORT_SYMBOL(xb_preload);
 
diff --git a/lib/xbitmap.c b/lib/xbitmap.c
index 816dd3e..426d168 100644
--- a/lib/xbitmap.c
+++ b/lib/xbitmap.c
@@ -18,7 +18,7 @@
  * This function is used to set a bit in the xbitmap. If the bitmap that @bit
  * resides in is not there, the per-cpu ida_bitmap will be taken.
  *
- * Returns: 0 on success. %-EAGAIN indicates that @bit was not set.
+ * Returns: 0 on success. Negative value on failure.
  */
 int xb_set_bit(struct xb *xb, unsigned long bit)
 {
@@ -28,46 +28,19 @@ int xb_set_bit(struct xb *xb, unsigned long bit)
struct radix_tree_node *node;
void **slot;
struct ida_bitmap *bitmap;
-   unsigned long ebit;
 
bit %= IDA_BITMAP_BITS;
-   ebit = bit + 2;
 
err = __radix_tree_create(root, index, 0, , );
if (err)
return err;
bitmap = rcu_dereference_raw(*slot);
-   if (radix_tree_exception(bitmap)) {
-   unsigned long tmp = (unsigned long)bitmap;
-
-   if (ebit < BITS_PER_LONG) {
-   tmp |= 1UL << ebit;
-   rcu_assign_pointer(*slot, (void *)tmp);
-   return 0;
-   }
-   bitmap = this_cpu_xchg(ida_bitmap, NULL);
-   if (!bitmap) {
-   __radix_tree_delete(root, node, slot);
-   return -EAGAIN;
-   }
-   memset(bitmap, 0, sizeof(*bitmap));
-   bitmap->bitmap[0] = tmp >> RADIX_TREE_EXCEPTIONAL_SHIFT;
-   rcu_assign_pointer(*slot, bitmap);
-   }
-
if (!bitmap) {
-   if (ebit < BITS_PER_LONG) {
-   bitmap = (void *)((1UL << ebit) |
-   RADIX_TREE_EXCEPTIONAL_ENTRY);
-   __radix_tree_replace(root, node, slot, bitmap, NULL);
-   return 0;
-   }
-   bitmap = this_cpu_xchg(ida_bitmap, NULL);
+   bitmap = kzalloc(sizeof(*bitmap), GFP_NOWAIT | __GFP_NOWARN);
if (!bitmap) {
__radix_tree_delete(root, node, slot);
-   return -EAGAIN;
+   return -ENOMEM;
}
-   memset(bitmap, 0, sizeof(*bitmap));
__radix_tree_replace(root, node, slot, bitmap, NULL);
}
 
@@ -82,7 +55,7 @@ int xb_set_bit(struct xb *xb, unsigned long bit)
  *  @bit: index of the bit to set
  *
  * A wrapper of the xb_preload() and xb_set_bit().
- * Returns: 0 on success; -EAGAIN or -ENOMEM on error.
+ * Returns: 0 on success; -ENOMEM on error.
  */
 int xb_preload_and_set_bit(struct xb *xb, unsigned long bit, gfp_t gfp)
 {
@@ -113,25 +86,10 @@ void xb_clear_bit(struct xb *xb, unsigned long bit)
struct 

Re: [PATCH v18 05/10] xbitmap: add more operations

[Tetsuo Handa]

Matthew Wilcox wrote:
> On Thu, Nov 30, 2017 at 10:35:03PM +0900, Tetsuo Handa wrote:
> > According to xb_set_bit(), it seems to me that we are trying to avoid 
> > memory allocation
> > for "struct ida_bitmap" when all set bits within a 1024-bits bitmap reside 
> > in the first
> > 61 bits.
> > 
> > But does such saving help? Is there characteristic bias that majority of 
> > set bits resides
> > in the first 61 bits, for "bit" is "unsigned long" which holds a page 
> > number (isn't it)?
> > If no such bias, wouldn't eliminating radix_tree_exception() case and 
> > always storing
> > "struct ida_bitmap" simplifies the code (and make the processing faster)?
> 
> It happens all the time.  The vast majority of users of the IDA set
> low bits.  Also, it's the first 62 bits -- going up to 63 bits with the
> XArray rewrite.

Oops, 0...61 is 62 bits.

What I meant is below (untested) patch. If correct, it can save lines and make
the code easier to read.

 lib/radix-tree.c | 20 +
 lib/xbitmap.c| 88 +---
 2 files changed, 8 insertions(+), 100 deletions(-)

diff --git a/lib/radix-tree.c b/lib/radix-tree.c
index a039588..fb84b91 100644
--- a/lib/radix-tree.c
+++ b/lib/radix-tree.c
@@ -2152,25 +2152,7 @@ int ida_pre_get(struct ida *ida, gfp_t gfp)
  */
 __must_check bool xb_preload(gfp_t gfp)
 {
-   if (!this_cpu_read(ida_bitmap)) {
-   struct ida_bitmap *bitmap = kmalloc(sizeof(*bitmap), gfp);
-
-   if (!bitmap)
-   return false;
-   /*
-* The per-CPU variable is updated with preemption enabled.
-* If the calling task is unlucky to be scheduled to another
-* CPU which has no ida_bitmap allocation, it will be detected
-* when setting a bit (i.e. __xb_set_bit()).
-*/
-   bitmap = this_cpu_cmpxchg(ida_bitmap, NULL, bitmap);
-   kfree(bitmap);
-   }
-
-   if (__radix_tree_preload(gfp, XB_PRELOAD_SIZE) < 0)
-   return false;
-
-   return true;
+   return __radix_tree_preload(gfp, XB_PRELOAD_SIZE) == 0;
 }
 EXPORT_SYMBOL(xb_preload);
 
diff --git a/lib/xbitmap.c b/lib/xbitmap.c
index 816dd3e..426d168 100644
--- a/lib/xbitmap.c
+++ b/lib/xbitmap.c
@@ -18,7 +18,7 @@
  * This function is used to set a bit in the xbitmap. If the bitmap that @bit
  * resides in is not there, the per-cpu ida_bitmap will be taken.
  *
- * Returns: 0 on success. %-EAGAIN indicates that @bit was not set.
+ * Returns: 0 on success. Negative value on failure.
  */
 int xb_set_bit(struct xb *xb, unsigned long bit)
 {
@@ -28,46 +28,19 @@ int xb_set_bit(struct xb *xb, unsigned long bit)
struct radix_tree_node *node;
void **slot;
struct ida_bitmap *bitmap;
-   unsigned long ebit;
 
bit %= IDA_BITMAP_BITS;
-   ebit = bit + 2;
 
err = __radix_tree_create(root, index, 0, , );
if (err)
return err;
bitmap = rcu_dereference_raw(*slot);
-   if (radix_tree_exception(bitmap)) {
-   unsigned long tmp = (unsigned long)bitmap;
-
-   if (ebit < BITS_PER_LONG) {
-   tmp |= 1UL << ebit;
-   rcu_assign_pointer(*slot, (void *)tmp);
-   return 0;
-   }
-   bitmap = this_cpu_xchg(ida_bitmap, NULL);
-   if (!bitmap) {
-   __radix_tree_delete(root, node, slot);
-   return -EAGAIN;
-   }
-   memset(bitmap, 0, sizeof(*bitmap));
-   bitmap->bitmap[0] = tmp >> RADIX_TREE_EXCEPTIONAL_SHIFT;
-   rcu_assign_pointer(*slot, bitmap);
-   }
-
if (!bitmap) {
-   if (ebit < BITS_PER_LONG) {
-   bitmap = (void *)((1UL << ebit) |
-   RADIX_TREE_EXCEPTIONAL_ENTRY);
-   __radix_tree_replace(root, node, slot, bitmap, NULL);
-   return 0;
-   }
-   bitmap = this_cpu_xchg(ida_bitmap, NULL);
+   bitmap = kzalloc(sizeof(*bitmap), GFP_NOWAIT | __GFP_NOWARN);
if (!bitmap) {
__radix_tree_delete(root, node, slot);
-   return -EAGAIN;
+   return -ENOMEM;
}
-   memset(bitmap, 0, sizeof(*bitmap));
__radix_tree_replace(root, node, slot, bitmap, NULL);
}
 
@@ -82,7 +55,7 @@ int xb_set_bit(struct xb *xb, unsigned long bit)
  *  @bit: index of the bit to set
  *
  * A wrapper of the xb_preload() and xb_set_bit().
- * Returns: 0 on success; -EAGAIN or -ENOMEM on error.
+ * Returns: 0 on success; -ENOMEM on error.
  */
 int xb_preload_and_set_bit(struct xb *xb, unsigned long bit, gfp_t gfp)
 {
@@ -113,25 +86,10 @@ void xb_clear_bit(struct xb *xb, unsigned long bit)
struct 

Re: [PATCH 2/2] ipc: Fix ipc data structures inconsistency

[Davidlohr Bueso]

On Sat, 02 Dec 2017, Philippe Mikoyan wrote:


On Fri, 1 Dec 2017 09:20:07 -0800
Davidlohr Bueso  wrote:



Hmm yeah that's pretty fishy, also shm_atime = 0, no?



Yeah, definitely, other data structure fields can also be
inconsistent, and applying not only to shmem, but also to
other ipc mechanisms.


Right.



Thank you for noting the typo, 'll send fixed version in next
message(without another patch, see below).


Ok, I had yet to look at that patch.

Re: [PATCH 2/2] ipc: Fix ipc data structures inconsistency

[Davidlohr Bueso]

On Sat, 02 Dec 2017, Philippe Mikoyan wrote:


On Fri, 1 Dec 2017 09:20:07 -0800
Davidlohr Bueso  wrote:



Hmm yeah that's pretty fishy, also shm_atime = 0, no?



Yeah, definitely, other data structure fields can also be
inconsistent, and applying not only to shmem, but also to
other ipc mechanisms.


Right.



Thank you for noting the typo, 'll send fixed version in next
message(without another patch, see below).


Ok, I had yet to look at that patch.

Re: [GIT PULL] hash addresses printed with %p

[Dave Young]

On 12/02/17 at 10:22pm, Matt Fleming wrote:
> (Cc'ing Dave since this is used for kexec on EFI)
> 
> On Fri, 01 Dec, at 09:54:43AM, Ard Biesheuvel wrote:
> > On 1 December 2017 at 09:48, Greg Kroah-Hartman
> >  wrote:
> > > On Thu, Nov 30, 2017 at 05:18:42PM +, Ard Biesheuvel wrote:
> > >> On 30 November 2017 at 17:10, Greg Kroah-Hartman
> > >>  wrote:
> > >> > On Thu, Nov 30, 2017 at 04:32:35PM +, Greg Kroah-Hartman wrote:
> > >> >> On Wed, Nov 29, 2017 at 01:36:25PM -0800, Linus Torvalds wrote:
> > >> >> > On Wed, Nov 29, 2017 at 1:14 PM, Linus Torvalds
> > >> >> >  wrote:
> > >> >> > >
> > >> >> > > Not because %pK itself changed, but because the semantics of %p 
> > >> >> > > did.
> > >> >> > > The baseline moved, and the "safe" version did not.
> > >> >> >
> > >> >> > Btw, that baseline for me is now that I can do
> > >> >> >
> > >> >> >   ./scripts/leaking_addresses.pl | wc -l
> > >> >> >   18
> > >> >> >
> > >> >> > and of those 18 hits, six are false positives (looks like bitmaps in
> > >> >> > the uevent keys).
> > >> >> >
> > >> >> > The remaining 12 are from the EFI runtime map files
> > >> >> > (/sys/firmware/efi/runtime-map/*). They should presumably not be
> > >> >> > world-readable, but sadly the kset_create_and_add() helper seems to 
> > >> >> > do
> > >> >> > that by default.
> > >> >> >
> > >> >> > I think the sysfs code makes it insanely too easy to make things
> > >> >> > world-readable. You try to be careful, and mark things read-only 
> > >> >> > etc,
> > >> >> > but __ATTR_RO() jkust means S_IRUGO, which means world-readable.
> > >> >> >
> > >> >> > There seems to be no convenient model for kobjects having better
> > >> >> > permissions. Greg?
> > >> >>
> > >> >> They can just use __ATTR() which lets you set the exact mode settings
> > >> >> that are wanted.
> > >> >>
> > >> >> Something like the patch below, which breaks the build as the
> > >> >> map_attributes are "odd", but you get the idea.  The EFI developers 
> > >> >> can
> > >> >> fix this up properly :)
> > >> >>
> > >> >> Note, this only accounts for 5 attributes, what is the whole list?
> > >> >
> > >> > Ah, it's the virt_addr file 12 times, I just ran it on my laptop:
> > >> >
> > >> > /sys/firmware/efi/runtime-map/7/virt_addr: 0xfffeea6ea000
> > >> > /sys/firmware/efi/runtime-map/5/virt_addr: 0xfffeee88b000
> > >> > /sys/firmware/efi/runtime-map/3/virt_addr: 0xfffefea0
> > >> > /sys/firmware/efi/runtime-map/11/virt_addr: 0xfffed9c0
> > >> > /sys/firmware/efi/runtime-map/1/virt_addr: 0xfffefee0
> > >> > /sys/firmware/efi/runtime-map/8/virt_addr: 0xfffedba4e000
> > >> > /sys/firmware/efi/runtime-map/6/virt_addr: 0xfffeee2de000
> > >> > /sys/firmware/efi/runtime-map/4/virt_addr: 0xfffeeea0
> > >> > /sys/firmware/efi/runtime-map/2/virt_addr: 0xfffefec0
> > >> > /sys/firmware/efi/runtime-map/10/virt_addr: 0xfffed9c6
> > >> > /sys/firmware/efi/runtime-map/0/virt_addr: 0xfffeff00
> > >> > /sys/firmware/efi/runtime-map/9/virt_addr: 0xfffedb9c9000
> > >> >
> > >> > So changing it to use __ATTR() should fix this remaning leakage up.
> > >> > That is if we even really need to export these values at all.  What 
> > >> > does
> > >> > userspace do with them?  Shouldn't they just be in debugfs instead?
> > >> >
> > >>
> > >> These are the virtual mappings UEFI firmware regions, which must
> > >> remain in the same place across kexec reboots. So kexec tooling
> > >> consumes this information and passes it on to the incoming kernel in
> > >> some way.
> > >>
> > >> Note that these are not kernel addresses, so while I agree they should
> > >> not be world readable, they won't give you any clue as to where the
> > >> kernel itself is mapped.
> > >>
> > >> So the recommendation is to switch to __ATTR( ... 0400 ... ) instead?
> > >> If so, I'll code up a patch.
> > >
> > > If these pointers are not "real", I recommend just leaving them as-is.
> > 
> > That's not what I said :-)
> > 
> > These are real pointers, and stuff will actually be mapped there
> > (although I am not intimately familiar with the way x86 does this, but
> > on ARM [which does not have these sysfs nodes in the first place],
> > these mappings are only live during the time a UEFI runtime service
> > call is in progress, and IIRC, work was underway to do the same for
> > x86). So while these values don't correlate with the placement of
> > kernel data structures, they could still be useful for an attacker to
> > figure out where exploitable firmware memory regions are located,
> > especially given that some of these may be mapped RWX.
> 
> These are mappings of the EFI firmware's runtime regions, dynamically
> allocated by the kernel starting at EFI_VA_START. Because we only get
> one chance to tell the firmware where we placed its regions (via
> SetVirtualAddressMap()) we have to guarantee that any 

Re: [GIT PULL] hash addresses printed with %p

[Dave Young]

On 12/02/17 at 10:22pm, Matt Fleming wrote:
> (Cc'ing Dave since this is used for kexec on EFI)
> 
> On Fri, 01 Dec, at 09:54:43AM, Ard Biesheuvel wrote:
> > On 1 December 2017 at 09:48, Greg Kroah-Hartman
> >  wrote:
> > > On Thu, Nov 30, 2017 at 05:18:42PM +, Ard Biesheuvel wrote:
> > >> On 30 November 2017 at 17:10, Greg Kroah-Hartman
> > >>  wrote:
> > >> > On Thu, Nov 30, 2017 at 04:32:35PM +, Greg Kroah-Hartman wrote:
> > >> >> On Wed, Nov 29, 2017 at 01:36:25PM -0800, Linus Torvalds wrote:
> > >> >> > On Wed, Nov 29, 2017 at 1:14 PM, Linus Torvalds
> > >> >> >  wrote:
> > >> >> > >
> > >> >> > > Not because %pK itself changed, but because the semantics of %p 
> > >> >> > > did.
> > >> >> > > The baseline moved, and the "safe" version did not.
> > >> >> >
> > >> >> > Btw, that baseline for me is now that I can do
> > >> >> >
> > >> >> >   ./scripts/leaking_addresses.pl | wc -l
> > >> >> >   18
> > >> >> >
> > >> >> > and of those 18 hits, six are false positives (looks like bitmaps in
> > >> >> > the uevent keys).
> > >> >> >
> > >> >> > The remaining 12 are from the EFI runtime map files
> > >> >> > (/sys/firmware/efi/runtime-map/*). They should presumably not be
> > >> >> > world-readable, but sadly the kset_create_and_add() helper seems to 
> > >> >> > do
> > >> >> > that by default.
> > >> >> >
> > >> >> > I think the sysfs code makes it insanely too easy to make things
> > >> >> > world-readable. You try to be careful, and mark things read-only 
> > >> >> > etc,
> > >> >> > but __ATTR_RO() jkust means S_IRUGO, which means world-readable.
> > >> >> >
> > >> >> > There seems to be no convenient model for kobjects having better
> > >> >> > permissions. Greg?
> > >> >>
> > >> >> They can just use __ATTR() which lets you set the exact mode settings
> > >> >> that are wanted.
> > >> >>
> > >> >> Something like the patch below, which breaks the build as the
> > >> >> map_attributes are "odd", but you get the idea.  The EFI developers 
> > >> >> can
> > >> >> fix this up properly :)
> > >> >>
> > >> >> Note, this only accounts for 5 attributes, what is the whole list?
> > >> >
> > >> > Ah, it's the virt_addr file 12 times, I just ran it on my laptop:
> > >> >
> > >> > /sys/firmware/efi/runtime-map/7/virt_addr: 0xfffeea6ea000
> > >> > /sys/firmware/efi/runtime-map/5/virt_addr: 0xfffeee88b000
> > >> > /sys/firmware/efi/runtime-map/3/virt_addr: 0xfffefea0
> > >> > /sys/firmware/efi/runtime-map/11/virt_addr: 0xfffed9c0
> > >> > /sys/firmware/efi/runtime-map/1/virt_addr: 0xfffefee0
> > >> > /sys/firmware/efi/runtime-map/8/virt_addr: 0xfffedba4e000
> > >> > /sys/firmware/efi/runtime-map/6/virt_addr: 0xfffeee2de000
> > >> > /sys/firmware/efi/runtime-map/4/virt_addr: 0xfffeeea0
> > >> > /sys/firmware/efi/runtime-map/2/virt_addr: 0xfffefec0
> > >> > /sys/firmware/efi/runtime-map/10/virt_addr: 0xfffed9c6
> > >> > /sys/firmware/efi/runtime-map/0/virt_addr: 0xfffeff00
> > >> > /sys/firmware/efi/runtime-map/9/virt_addr: 0xfffedb9c9000
> > >> >
> > >> > So changing it to use __ATTR() should fix this remaning leakage up.
> > >> > That is if we even really need to export these values at all.  What 
> > >> > does
> > >> > userspace do with them?  Shouldn't they just be in debugfs instead?
> > >> >
> > >>
> > >> These are the virtual mappings UEFI firmware regions, which must
> > >> remain in the same place across kexec reboots. So kexec tooling
> > >> consumes this information and passes it on to the incoming kernel in
> > >> some way.
> > >>
> > >> Note that these are not kernel addresses, so while I agree they should
> > >> not be world readable, they won't give you any clue as to where the
> > >> kernel itself is mapped.
> > >>
> > >> So the recommendation is to switch to __ATTR( ... 0400 ... ) instead?
> > >> If so, I'll code up a patch.
> > >
> > > If these pointers are not "real", I recommend just leaving them as-is.
> > 
> > That's not what I said :-)
> > 
> > These are real pointers, and stuff will actually be mapped there
> > (although I am not intimately familiar with the way x86 does this, but
> > on ARM [which does not have these sysfs nodes in the first place],
> > these mappings are only live during the time a UEFI runtime service
> > call is in progress, and IIRC, work was underway to do the same for
> > x86). So while these values don't correlate with the placement of
> > kernel data structures, they could still be useful for an attacker to
> > figure out where exploitable firmware memory regions are located,
> > especially given that some of these may be mapped RWX.
> 
> These are mappings of the EFI firmware's runtime regions, dynamically
> allocated by the kernel starting at EFI_VA_START. Because we only get
> one chance to tell the firmware where we placed its regions (via
> SetVirtualAddressMap()) we have to guarantee that any subsequent kexec
> reboots use the same addresses.
> 
> So that's why they're exported to 

Re: [GIT PULL] revert ARM SCPI changes for v4.15-rc1

[Olof Johansson]

On Fri, Dec 01, 2017 at 11:53:05AM -0800, Kevin Hilman wrote:
> Arnd, Olof,
> 
> These ARM SCPI changes caused SCPI regressions resulting in CPUfreq
> failures on most Amlogic SoCs (found by kernelci.org.)
> 
> Unfortunately, this was not caught in linux-next due to other
> bugs/panics on these platforms masking this problem so we've only found
> it since we've fixed the other issues.
> 
> Since we're already in the -rc cycle, I'd prefer to revert to a known
> working state (that of v4.14) rather than finding/reverting a subset,
> which would just lead to another untested state.
> 
> These changes can then have some time to be better reviewed and tested
> and resubmitted for v4.16.
> 
> I've tested this revert on the affect Amlogic SoCs and verified that
> we're back to the previous (working) condition.
> 
> Also, I'm sending the pull directly to arm-soc instead of Sudeeep
> because I understand that Sudeep is currently out-of-office and unlikely
> to be able to address this himself during the -rc cycle.
> 

Sounds like the right approach here. I've merged this and added the above text
to the merge commit as well.


-Olof

Re: [GIT PULL] revert ARM SCPI changes for v4.15-rc1

[Olof Johansson]

On Fri, Dec 01, 2017 at 11:53:05AM -0800, Kevin Hilman wrote:
> Arnd, Olof,
> 
> These ARM SCPI changes caused SCPI regressions resulting in CPUfreq
> failures on most Amlogic SoCs (found by kernelci.org.)
> 
> Unfortunately, this was not caught in linux-next due to other
> bugs/panics on these platforms masking this problem so we've only found
> it since we've fixed the other issues.
> 
> Since we're already in the -rc cycle, I'd prefer to revert to a known
> working state (that of v4.14) rather than finding/reverting a subset,
> which would just lead to another untested state.
> 
> These changes can then have some time to be better reviewed and tested
> and resubmitted for v4.16.
> 
> I've tested this revert on the affect Amlogic SoCs and verified that
> we're back to the previous (working) condition.
> 
> Also, I'm sending the pull directly to arm-soc instead of Sudeeep
> because I understand that Sudeep is currently out-of-office and unlikely
> to be able to address this himself during the -rc cycle.
> 

Sounds like the right approach here. I've merged this and added the above text
to the merge commit as well.


-Olof

Re: [GIT PULL] ARM: uniphier: fixes for v4.15

[Olof Johansson]

On Wed, Nov 29, 2017 at 11:01:57PM +0900, Masahiro Yamada wrote:
> Hi Arnd, Olof,
> 
> Here are some fixes of ARM UniPhier SoC family.
> There is a mistake in the IRQ number.  Otherwise, I wouldn't have done
> this early pull request.  One more trivial patch to avoid conflict.
> Also, MAINTAINERS update, which is always safe.
> Please pull!
> 
> 
> The following changes since commit 4fbd8d194f06c8a3fd2af1ce560ddb31f7ec8323:
> 
>   Linux 4.15-rc1 (2017-11-26 16:01:47 -0800)
> 
> are available in the git repository at:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/masahiroy/linux-uniphier.git
> tags/uniphier-fixes-v4.15
> 
> for you to fetch changes up to 0308cadcec379e510f498c48c34aafe582f1af88:
> 
>   MAINTAINERS: exclude other Socionext SoC DT files from ARM/UNIPHIER
> entry (2017-11-29 22:22:10 +0900)
> 
> 
> UniPhier ARM SoC fixes for v4.15
> 
> - Fix IRQ number of PXs3 SoC
> - Remove redundant interrupt-parent properties
> - Fix arm64 DT path in MAINTAINERS

Merged, thanks.


-Olof

Re: [GIT PULL] tee fix for v4.15

[Olof Johansson]

On Wed, Nov 29, 2017 at 11:16:50AM +0100, Jens Wiklander wrote:
> Hi arm-soc maiantainers,
> 
> Please pull this OP-TEE driver init fix for v4.15
> 
> Thanks,
> Jens
> 
> The following changes since commit 4fbd8d194f06c8a3fd2af1ce560ddb31f7ec8323:
> 
>   Linux 4.15-rc1 (2017-11-26 16:01:47 -0800)
> 
> are available in the git repository at:
> 
>   https://git.linaro.org/people/jens.wiklander/linux-tee.git 
> tags/tee-drv-fix-for-4.15
> 
> for you to fetch changes up to f044113113dd95ba73916bde10e804d3cdfa2662:
> 
>   optee: fix invalid of_node_put() in optee_driver_init() (2017-11-29 
> 10:24:57 +0100)

Merged, thanks.


-Olof

Re: [GIT PULL] ARM: uniphier: fixes for v4.15

[Olof Johansson]

On Wed, Nov 29, 2017 at 11:01:57PM +0900, Masahiro Yamada wrote:
> Hi Arnd, Olof,
> 
> Here are some fixes of ARM UniPhier SoC family.
> There is a mistake in the IRQ number.  Otherwise, I wouldn't have done
> this early pull request.  One more trivial patch to avoid conflict.
> Also, MAINTAINERS update, which is always safe.
> Please pull!
> 
> 
> The following changes since commit 4fbd8d194f06c8a3fd2af1ce560ddb31f7ec8323:
> 
>   Linux 4.15-rc1 (2017-11-26 16:01:47 -0800)
> 
> are available in the git repository at:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/masahiroy/linux-uniphier.git
> tags/uniphier-fixes-v4.15
> 
> for you to fetch changes up to 0308cadcec379e510f498c48c34aafe582f1af88:
> 
>   MAINTAINERS: exclude other Socionext SoC DT files from ARM/UNIPHIER
> entry (2017-11-29 22:22:10 +0900)
> 
> 
> UniPhier ARM SoC fixes for v4.15
> 
> - Fix IRQ number of PXs3 SoC
> - Remove redundant interrupt-parent properties
> - Fix arm64 DT path in MAINTAINERS

Merged, thanks.


-Olof

Re: [GIT PULL] tee fix for v4.15

[Olof Johansson]

On Wed, Nov 29, 2017 at 11:16:50AM +0100, Jens Wiklander wrote:
> Hi arm-soc maiantainers,
> 
> Please pull this OP-TEE driver init fix for v4.15
> 
> Thanks,
> Jens
> 
> The following changes since commit 4fbd8d194f06c8a3fd2af1ce560ddb31f7ec8323:
> 
>   Linux 4.15-rc1 (2017-11-26 16:01:47 -0800)
> 
> are available in the git repository at:
> 
>   https://git.linaro.org/people/jens.wiklander/linux-tee.git 
> tags/tee-drv-fix-for-4.15
> 
> for you to fetch changes up to f044113113dd95ba73916bde10e804d3cdfa2662:
> 
>   optee: fix invalid of_node_put() in optee_driver_init() (2017-11-29 
> 10:24:57 +0100)

Merged, thanks.


-Olof

Re: [PATCH] arm64: dts: sort vendor subdirectories in Makefile alphabetically

[Olof Johansson]

On Sat, Nov 18, 2017 at 12:10:03PM +0900, Masahiro Yamada wrote:
> The list is almost sorted.  Move "lg" up to complete it.
> 
> Signed-off-by: Masahiro Yamada 

Applied to fixes, thanks.


-Olof

Re: [PATCH] arm64: dts: sort vendor subdirectories in Makefile alphabetically

[Olof Johansson]

On Sat, Nov 18, 2017 at 12:10:03PM +0900, Masahiro Yamada wrote:
> The list is almost sorted.  Move "lg" up to complete it.
> 
> Signed-off-by: Masahiro Yamada 

Applied to fixes, thanks.


-Olof

Re: [PATCH] iio: accel: bmc150: Add OF device ID table

[Javier Martinez Canillas]

Hello Jonathan,

On 12/02/2017 01:02 PM, Jonathan Cameron wrote:
> On Fri,  1 Dec 2017 12:10:58 +0100
> Javier Martinez Canillas  wrote:
> 
>> The driver doesn't have a struct of_device_id table but supported devices
>> are registered via Device Trees. This is working on the assumption that a
>> I2C device registered via OF will always match a legacy I2C device ID and
>> that the MODALIAS reported will always be of the form i2c:.
>>
>> But this could change in the future so the correct approach is to have an
>> OF device ID table if the devices are registered via OF.
>>
>> The I2C device ID table entries have the .driver_data field set, but they
>> are not used in the driver so weren't set in the OF device table entries.
>>
>> Signed-off-by: Javier Martinez Canillas 
> 
> Applied to the togreg branch of iio.git and pushed out as testing
> for the autobuilders to play with it.
> 
> Would be nice to do the spi counterpart at somepoint, but as this is
> clearly an improvement on nothing I applied this one as step 1.
>

Ok, I can do the same for SPI. I'll post a patch for it next week.
 
> Good point about the data fields though - we should probably clean those
> out as misleading.
>

Same for this, I can post a cleanup patch to get rid of the .data fields.

> Thanks,
> 
> Jonathan

Best regards,
-- 
Javier Martinez Canillas
Software Engineer - Desktop Hardware Enablement
Red Hat

Re: [PATCH] iio: accel: bmc150: Add OF device ID table

[Javier Martinez Canillas]

Hello Jonathan,

On 12/02/2017 01:02 PM, Jonathan Cameron wrote:
> On Fri,  1 Dec 2017 12:10:58 +0100
> Javier Martinez Canillas  wrote:
> 
>> The driver doesn't have a struct of_device_id table but supported devices
>> are registered via Device Trees. This is working on the assumption that a
>> I2C device registered via OF will always match a legacy I2C device ID and
>> that the MODALIAS reported will always be of the form i2c:.
>>
>> But this could change in the future so the correct approach is to have an
>> OF device ID table if the devices are registered via OF.
>>
>> The I2C device ID table entries have the .driver_data field set, but they
>> are not used in the driver so weren't set in the OF device table entries.
>>
>> Signed-off-by: Javier Martinez Canillas 
> 
> Applied to the togreg branch of iio.git and pushed out as testing
> for the autobuilders to play with it.
> 
> Would be nice to do the spi counterpart at somepoint, but as this is
> clearly an improvement on nothing I applied this one as step 1.
>

Ok, I can do the same for SPI. I'll post a patch for it next week.
 
> Good point about the data fields though - we should probably clean those
> out as misleading.
>

Same for this, I can post a cleanup patch to get rid of the .data fields.

> Thanks,
> 
> Jonathan

Best regards,
-- 
Javier Martinez Canillas
Software Engineer - Desktop Hardware Enablement
Red Hat

RE: [PATCH 1/2] x86/mce/AMD: Define function to get SMCA bank type

[Ghannam, Yazen]

> -Original Message-
> From: linux-edac-ow...@vger.kernel.org [mailto:linux-edac-
> ow...@vger.kernel.org] On Behalf Of Borislav Petkov
> Sent: Saturday, December 2, 2017 9:22 AM
> To: Ghannam, Yazen 
> Cc: linux-e...@vger.kernel.org; Tony Luck ;
> x...@kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH 1/2] x86/mce/AMD: Define function to get SMCA bank
> type
> 
> On Fri, Dec 01, 2017 at 09:50:33AM -0600, Yazen Ghannam wrote:
> > From: Yazen Ghannam 
> >
> > Scalable MCA systems have various types of banks. The bank's type can
> > determine how we handle errors from it. For example, if a bank represents
> > a UMC then we will need to convert its address from a normalized address
> > to a system physical address before handling the error.
> >
> > Define an exported function to return a bank's SMCA type.
> >
> > Signed-off-by: Yazen Ghannam 
> > ---
> >  arch/x86/include/asm/mce.h   |  1 +
> >  arch/x86/kernel/cpu/mcheck/mce_amd.c | 11 +++
> >  2 files changed, 12 insertions(+)
> >
> > diff --git a/arch/x86/include/asm/mce.h b/arch/x86/include/asm/mce.h
> > index b1e8d8db921f..9ab8bf32e61c 100644
> > --- a/arch/x86/include/asm/mce.h
> > +++ b/arch/x86/include/asm/mce.h
> > @@ -376,6 +376,7 @@ struct smca_bank {
> >  extern struct smca_bank smca_banks[MAX_NR_BANKS];
> >
> >  extern const char *smca_get_long_name(enum smca_bank_types t);
> > +extern unsigned int smca_get_bank_type(struct mce *m);
> >
> >  extern int mce_threshold_create_device(unsigned int cpu);
> >  extern int mce_threshold_remove_device(unsigned int cpu);
> > diff --git a/arch/x86/kernel/cpu/mcheck/mce_amd.c
> b/arch/x86/kernel/cpu/mcheck/mce_amd.c
> > index a38ab1fa53a2..bc0510a4f6c0 100644
> > --- a/arch/x86/kernel/cpu/mcheck/mce_amd.c
> > +++ b/arch/x86/kernel/cpu/mcheck/mce_amd.c
> > @@ -110,6 +110,17 @@ const char *smca_get_long_name(enum
> smca_bank_types t)
> >  }
> >  EXPORT_SYMBOL_GPL(smca_get_long_name);
> >
> > +unsigned int smca_get_bank_type(struct mce *m)
> > +{
> > +   struct smca_bank bank = smca_banks[m->bank];
> > +
> > +   if (!bank.hwid)
> > +   return N_SMCA_BANK_TYPES;
> > +
> > +   return bank.hwid->bank_type;
> > +}
> > +EXPORT_SYMBOL_GPL(smca_get_bank_type);
> 
> Why are you exporting it if it is used in mce_amd.c only anyway?
> 

I was thinking it could be used in edac/mce_amd.c also.

I don't have a use at the moment, so I can change this if you'd like.

Thanks,
Yazen

RE: [PATCH 1/2] x86/mce/AMD: Define function to get SMCA bank type

[Ghannam, Yazen]

> -Original Message-
> From: linux-edac-ow...@vger.kernel.org [mailto:linux-edac-
> ow...@vger.kernel.org] On Behalf Of Borislav Petkov
> Sent: Saturday, December 2, 2017 9:22 AM
> To: Ghannam, Yazen 
> Cc: linux-e...@vger.kernel.org; Tony Luck ;
> x...@kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH 1/2] x86/mce/AMD: Define function to get SMCA bank
> type
> 
> On Fri, Dec 01, 2017 at 09:50:33AM -0600, Yazen Ghannam wrote:
> > From: Yazen Ghannam 
> >
> > Scalable MCA systems have various types of banks. The bank's type can
> > determine how we handle errors from it. For example, if a bank represents
> > a UMC then we will need to convert its address from a normalized address
> > to a system physical address before handling the error.
> >
> > Define an exported function to return a bank's SMCA type.
> >
> > Signed-off-by: Yazen Ghannam 
> > ---
> >  arch/x86/include/asm/mce.h   |  1 +
> >  arch/x86/kernel/cpu/mcheck/mce_amd.c | 11 +++
> >  2 files changed, 12 insertions(+)
> >
> > diff --git a/arch/x86/include/asm/mce.h b/arch/x86/include/asm/mce.h
> > index b1e8d8db921f..9ab8bf32e61c 100644
> > --- a/arch/x86/include/asm/mce.h
> > +++ b/arch/x86/include/asm/mce.h
> > @@ -376,6 +376,7 @@ struct smca_bank {
> >  extern struct smca_bank smca_banks[MAX_NR_BANKS];
> >
> >  extern const char *smca_get_long_name(enum smca_bank_types t);
> > +extern unsigned int smca_get_bank_type(struct mce *m);
> >
> >  extern int mce_threshold_create_device(unsigned int cpu);
> >  extern int mce_threshold_remove_device(unsigned int cpu);
> > diff --git a/arch/x86/kernel/cpu/mcheck/mce_amd.c
> b/arch/x86/kernel/cpu/mcheck/mce_amd.c
> > index a38ab1fa53a2..bc0510a4f6c0 100644
> > --- a/arch/x86/kernel/cpu/mcheck/mce_amd.c
> > +++ b/arch/x86/kernel/cpu/mcheck/mce_amd.c
> > @@ -110,6 +110,17 @@ const char *smca_get_long_name(enum
> smca_bank_types t)
> >  }
> >  EXPORT_SYMBOL_GPL(smca_get_long_name);
> >
> > +unsigned int smca_get_bank_type(struct mce *m)
> > +{
> > +   struct smca_bank bank = smca_banks[m->bank];
> > +
> > +   if (!bank.hwid)
> > +   return N_SMCA_BANK_TYPES;
> > +
> > +   return bank.hwid->bank_type;
> > +}
> > +EXPORT_SYMBOL_GPL(smca_get_bank_type);
> 
> Why are you exporting it if it is used in mce_amd.c only anyway?
> 

I was thinking it could be used in edac/mce_amd.c also.

I don't have a use at the moment, so I can change this if you'd like.

Thanks,
Yazen

Re: [PATCH] fix system_state checking in early_ioremap

[Dave Young]

On 12/02/17 at 11:34am, Dave Young wrote:
> Since below commit earlyprintk=efi,keep does not work any more with a warning
> in mm/early_ioremap.c: WARN_ON(system_state >= SYSTEM_RUNNING):

Should be WARN_ON(system_state != SYSTEM_BOOTING) in original code, copy
paste wrongly, if need a resend please let me know :)

> commit 69a78ff226fe ("init: Introduce SYSTEM_SCHEDULING state")
> 
> Reason is the the original assumption is SYSTEM_BOOTING equal to
> system_state < SYSTEM_RUNNING. But with commit 69a78ff226fe it is not true
> any more. Change the WARN_ON to check system_state >= SYSTEM_RUNNING instead.
> 
> Signed-off-by: Dave Young 
> ---
>  mm/early_ioremap.c |2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> --- linux-x86.orig/mm/early_ioremap.c
> +++ linux-x86/mm/early_ioremap.c
> @@ -111,7 +111,7 @@ __early_ioremap(resource_size_t phys_add
>   enum fixed_addresses idx;
>   int i, slot;
>  
> - WARN_ON(system_state != SYSTEM_BOOTING);
> + WARN_ON(system_state >= SYSTEM_RUNNING);
>  
>   slot = -1;
>   for (i = 0; i < FIX_BTMAPS_SLOTS; i++) {

Re: [PATCH] fix system_state checking in early_ioremap

[Dave Young]

On 12/02/17 at 11:34am, Dave Young wrote:
> Since below commit earlyprintk=efi,keep does not work any more with a warning
> in mm/early_ioremap.c: WARN_ON(system_state >= SYSTEM_RUNNING):

Should be WARN_ON(system_state != SYSTEM_BOOTING) in original code, copy
paste wrongly, if need a resend please let me know :)

> commit 69a78ff226fe ("init: Introduce SYSTEM_SCHEDULING state")
> 
> Reason is the the original assumption is SYSTEM_BOOTING equal to
> system_state < SYSTEM_RUNNING. But with commit 69a78ff226fe it is not true
> any more. Change the WARN_ON to check system_state >= SYSTEM_RUNNING instead.
> 
> Signed-off-by: Dave Young 
> ---
>  mm/early_ioremap.c |2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> --- linux-x86.orig/mm/early_ioremap.c
> +++ linux-x86/mm/early_ioremap.c
> @@ -111,7 +111,7 @@ __early_ioremap(resource_size_t phys_add
>   enum fixed_addresses idx;
>   int i, slot;
>  
> - WARN_ON(system_state != SYSTEM_BOOTING);
> + WARN_ON(system_state >= SYSTEM_RUNNING);
>  
>   slot = -1;
>   for (i = 0; i < FIX_BTMAPS_SLOTS; i++) {

[PATCH RFC] stat.2: Document that stat can fail with EINTR

[Keno Fischer]

Particularly on network file systems, a stat call may require
submitting a message over the network and waiting interruptably
for a reply.

Signed-off-by: Keno Fischer 
---

The catalyst for this patch was me experiencing EINTR errors when
using the 9p file system. In linux commit 9523feac, the 9p file
system was changed to use wait_event_killable instead of
wait_event_interruptible, which does indeed address my problem,
but also makes me a bit unhappy, because uninterruptable waits
prevents things like ^C'ing the execution and some debugging
tools which depend on being able to cancel long-running operations
by sending signals. I'd like to ask the user space applications I
care about to properly handle such situations (either by using
SA_RESTART or by explicitly handling EINTR), but it's a bit of a
hard sell if EINTR isn't documented to be a possibility. I'm hoping
this doc PATCH will generate a discussion of whether EINTR is an
appropriate thing for stat (as a stand in for a file system call that's
not read/write) to return. If so, I'd be happy to submit
patches to other file system-related syscalls along these same lines.

I realize I'm probably 20 years too late here, but it feels like
clarificaion on what to expect from the kernel would still go a long
way here.  

 man2/stat.2 | 5 +
 1 file changed, 5 insertions(+)

diff --git a/man2/stat.2 b/man2/stat.2
index dad9a01..f10235a 100644
--- a/man2/stat.2
+++ b/man2/stat.2
@@ -452,6 +452,11 @@ Invalid flag specified in
 is relative and
 .I dirfd
 is a file descriptor referring to a file other than a directory.
+.TP
+.B EINTR
+The call was interrupted by delivery of a signal caught by a handler; see
+.BR signal (7).
+The possibility of this error is file-system dependent.
 .SH VERSIONS
 .BR fstatat ()
 was added to Linux in kernel 2.6.16;
-- 
2.8.1

[PATCH RFC] stat.2: Document that stat can fail with EINTR

[Keno Fischer]

Particularly on network file systems, a stat call may require
submitting a message over the network and waiting interruptably
for a reply.

Signed-off-by: Keno Fischer 
---

The catalyst for this patch was me experiencing EINTR errors when
using the 9p file system. In linux commit 9523feac, the 9p file
system was changed to use wait_event_killable instead of
wait_event_interruptible, which does indeed address my problem,
but also makes me a bit unhappy, because uninterruptable waits
prevents things like ^C'ing the execution and some debugging
tools which depend on being able to cancel long-running operations
by sending signals. I'd like to ask the user space applications I
care about to properly handle such situations (either by using
SA_RESTART or by explicitly handling EINTR), but it's a bit of a
hard sell if EINTR isn't documented to be a possibility. I'm hoping
this doc PATCH will generate a discussion of whether EINTR is an
appropriate thing for stat (as a stand in for a file system call that's
not read/write) to return. If so, I'd be happy to submit
patches to other file system-related syscalls along these same lines.

I realize I'm probably 20 years too late here, but it feels like
clarificaion on what to expect from the kernel would still go a long
way here.  

 man2/stat.2 | 5 +
 1 file changed, 5 insertions(+)

diff --git a/man2/stat.2 b/man2/stat.2
index dad9a01..f10235a 100644
--- a/man2/stat.2
+++ b/man2/stat.2
@@ -452,6 +452,11 @@ Invalid flag specified in
 is relative and
 .I dirfd
 is a file descriptor referring to a file other than a directory.
+.TP
+.B EINTR
+The call was interrupted by delivery of a signal caught by a handler; see
+.BR signal (7).
+The possibility of this error is file-system dependent.
 .SH VERSIONS
 .BR fstatat ()
 was added to Linux in kernel 2.6.16;
-- 
2.8.1

Re: [PATCH] mmap.2: MAP_FIXED is no longer discouraged

[John Hubbard]

On 12/02/2017 02:19 PM, Matthew Wilcox wrote:
> On Sat, Dec 02, 2017 at 07:49:20PM +0100, Jann Horn wrote:
>> On Sat, Dec 2, 2017 at 4:05 PM, Matthew Wilcox  wrote:
>>> On Fri, Dec 01, 2017 at 06:16:26PM -0800, john.hubb...@gmail.com wrote:
 MAP_FIXED has been widely used for a very long time, yet the man
 page still claims that "the use of this option is discouraged".
>>>
>>> I think we should continue to discourage the use of this option, but
>>> I'm going to include some of your text in my replacement paragraph ...
>>>
>>> -Because requiring a fixed address for a mapping is less portable,
>>> -the use of this option is discouraged.
>>> +The use of this option is discouraged because it forcibly unmaps any
>>> +existing mapping at that address.  Programs which use this option need
>>> +to be aware that their memory map may change significantly from one run to
>>> +the next, depending on library versions, kernel versions and random 
>>> numbers.
>>
>> How about adding something explicit about when it's okay to use MAP_FIXED?
>> "This option should only be used to displace an existing mapping that is
>> controlled by the caller, or part of such a mapping." or something like that?
>>
>>> +In a threaded process, checking the existing mappings can race against
>>> +a new dynamic library being loaded
>>
>> malloc() and its various callers can also cause mmap() calls, which is 
>> probably
>> more relevant than library loading.
> 
> That's a bit more expected though.  "I called malloc and my address
> space changed".  Well, yeah.  But "I called getpwnam and my address
> space changed" is a bit more surprising.  Don't you think?
> 
> Maybe that should be up front rather than buried at the end of the sentence.
> 
> "In a multi-threaded process, the address space can change in response to
> virtually any library call.  This is because almost any library call may be
> implemented by using dlopen(3) to load another shared library, which will be
> mapped into the process's address space.  The PAM libraries are an excellent
> example, as well as more obvious examples like brk(2), malloc(3) and even
> pthread_create(3)."
> 
> What do you think?
> 

I'm working on some updated wording to capture these points. I'm even slower
at writing than I am at coding, so there will be a somewhat-brief pause here... 
:)

thanks,
John Hubbard
NVIDIA

Re: [PATCH] mmap.2: MAP_FIXED is no longer discouraged

[John Hubbard]

On 12/02/2017 02:19 PM, Matthew Wilcox wrote:
> On Sat, Dec 02, 2017 at 07:49:20PM +0100, Jann Horn wrote:
>> On Sat, Dec 2, 2017 at 4:05 PM, Matthew Wilcox  wrote:
>>> On Fri, Dec 01, 2017 at 06:16:26PM -0800, john.hubb...@gmail.com wrote:
 MAP_FIXED has been widely used for a very long time, yet the man
 page still claims that "the use of this option is discouraged".
>>>
>>> I think we should continue to discourage the use of this option, but
>>> I'm going to include some of your text in my replacement paragraph ...
>>>
>>> -Because requiring a fixed address for a mapping is less portable,
>>> -the use of this option is discouraged.
>>> +The use of this option is discouraged because it forcibly unmaps any
>>> +existing mapping at that address.  Programs which use this option need
>>> +to be aware that their memory map may change significantly from one run to
>>> +the next, depending on library versions, kernel versions and random 
>>> numbers.
>>
>> How about adding something explicit about when it's okay to use MAP_FIXED?
>> "This option should only be used to displace an existing mapping that is
>> controlled by the caller, or part of such a mapping." or something like that?
>>
>>> +In a threaded process, checking the existing mappings can race against
>>> +a new dynamic library being loaded
>>
>> malloc() and its various callers can also cause mmap() calls, which is 
>> probably
>> more relevant than library loading.
> 
> That's a bit more expected though.  "I called malloc and my address
> space changed".  Well, yeah.  But "I called getpwnam and my address
> space changed" is a bit more surprising.  Don't you think?
> 
> Maybe that should be up front rather than buried at the end of the sentence.
> 
> "In a multi-threaded process, the address space can change in response to
> virtually any library call.  This is because almost any library call may be
> implemented by using dlopen(3) to load another shared library, which will be
> mapped into the process's address space.  The PAM libraries are an excellent
> example, as well as more obvious examples like brk(2), malloc(3) and even
> pthread_create(3)."
> 
> What do you think?
> 

I'm working on some updated wording to capture these points. I'm even slower
at writing than I am at coding, so there will be a somewhat-brief pause here... 
:)

thanks,
John Hubbard
NVIDIA

[PATCHv2] staging: pi433: pi433_if.c remove SET_CHECKED macro

[Nguyen Phan Quang Minh]

The macro calls its argument -a function- twice, makes the calling
function return prematurely -skipping resource cleanup code- and hurts
understandability.

Signed-off-by: Nguyen Phan Quang Minh 
---
Base on Dan's feedback, I checked and the macro return indeed skips over
some cleanup code, hence remove it.
The code is not pretty, any hints on how to do it better is greatly
appreciated.

 drivers/staging/pi433/pi433_if.c | 237 ---
 1 file changed, 174 insertions(+), 63 deletions(-)

diff --git a/drivers/staging/pi433/pi433_if.c b/drivers/staging/pi433/pi433_if.c
index 3404cb9722c9..c41243f74363 100644
--- a/drivers/staging/pi433/pi433_if.c
+++ b/drivers/staging/pi433/pi433_if.c
@@ -119,12 +119,6 @@ struct pi433_instance {
struct pi433_tx_cfg tx_cfg;
 };
 
-/*-*/
-
-/* macro for checked access of registers of radio module */
-#define SET_CHECKED(retval) \
-   if (retval < 0) \
-   return retval;
 
 /*-*/
 
@@ -183,28 +177,53 @@ rf69_set_rx_cfg(struct pi433_device *dev, struct 
pi433_rx_cfg *rx_cfg)
int payload_length;
 
/* receiver config */
-   SET_CHECKED(rf69_set_frequency  (dev->spi, rx_cfg->frequency));
-   SET_CHECKED(rf69_set_bit_rate   (dev->spi, rx_cfg->bit_rate));
-   SET_CHECKED(rf69_set_modulation (dev->spi, rx_cfg->modulation));
-   SET_CHECKED(rf69_set_antenna_impedance   (dev->spi, 
rx_cfg->antenna_impedance));
-   SET_CHECKED(rf69_set_rssi_threshold  (dev->spi, 
rx_cfg->rssi_threshold));
-   SET_CHECKED(rf69_set_ook_threshold_dec   (dev->spi, 
rx_cfg->thresholdDecrement));
-   SET_CHECKED(rf69_set_bandwidth   (dev->spi, 
rx_cfg->bw_mantisse, rx_cfg->bw_exponent));
-   SET_CHECKED(rf69_set_bandwidth_during_afc(dev->spi, 
rx_cfg->bw_mantisse, rx_cfg->bw_exponent));
-   SET_CHECKED(rf69_set_dagc(dev->spi, rx_cfg->dagc));
+   ret = rf69_set_frequency(dev->spi, rx_cfg->frequency);
+   if (ret < 0)
+   return ret;
+   ret = rf69_set_bit_rate(dev->spi, rx_cfg->bit_rate);
+   if (ret < 0)
+   return ret;
+   ret = rf69_set_modulation(dev->spi, rx_cfg->modulation);
+   if (ret < 0)
+   return ret;
+   ret = rf69_set_antenna_impedance(dev->spi, rx_cfg->antenna_impedance);
+   if (ret < 0)
+   return ret;
+   ret = rf69_set_rssi_threshold(dev->spi, rx_cfg->rssi_threshold);
+   if (ret < 0)
+   return ret;
+   ret = rf69_set_ook_threshold_dec(dev->spi, rx_cfg->thresholdDecrement);
+   if (ret < 0)
+   return ret;
+   ret = rf69_set_bandwidth(dev->spi, rx_cfg->bw_mantisse, 
rx_cfg->bw_exponent);
+   if (ret < 0)
+   return ret;
+   ret = rf69_set_bandwidth_during_afc(dev->spi, rx_cfg->bw_mantisse,
+   rx_cfg->bw_exponent);
+   if (ret < 0)
+   return ret;
+   ret = rf69_set_dagc(dev->spi, rx_cfg->dagc);
+   if (ret < 0)
+   return ret;
 
dev->rx_bytes_to_drop = rx_cfg->bytes_to_drop;
 
/* packet config */
/* enable */
-   SET_CHECKED(rf69_set_sync_enable(dev->spi, rx_cfg->enable_sync));
+   ret = rf69_set_sync_enable(dev->spi, rx_cfg->enable_sync);
+   if (ret < 0)
+   return ret;
if (rx_cfg->enable_sync == optionOn)
{
-   SET_CHECKED(rf69_set_fifo_fill_condition(dev->spi, 
afterSyncInterrupt));
+   ret = rf69_set_fifo_fill_condition(dev->spi, 
afterSyncInterrupt);
+   if (ret < 0)
+   return ret;
}
else
{
-   SET_CHECKED(rf69_set_fifo_fill_condition(dev->spi, always));
+   ret = rf69_set_fifo_fill_condition(dev->spi, always);
+   if (ret < 0)
+   return ret;
}
if (rx_cfg->enable_length_byte == optionOn) {
ret = rf69_set_packet_format(dev->spi, packetLengthVar);
@@ -215,36 +234,54 @@ rf69_set_rx_cfg(struct pi433_device *dev, struct 
pi433_rx_cfg *rx_cfg)
if (ret < 0)
return ret;
}
-   SET_CHECKED(rf69_set_adressFiltering(dev->spi, 
rx_cfg->enable_address_filtering));
-   SET_CHECKED(rf69_set_crc_enable (dev->spi, rx_cfg->enable_crc));
+   ret = rf69_set_adressFiltering(dev->spi, 
rx_cfg->enable_address_filtering);
+   if (ret < 0)
+   return ret;
+   ret = rf69_set_crc_enable(dev->spi, rx_cfg->enable_crc);
+   if (ret < 0)
+   return ret;
 
/* lengths */
-   SET_CHECKED(rf69_set_sync_size(dev->spi, rx_cfg->sync_length));
+   ret = rf69_set_sync_size(dev->spi, rx_cfg->sync_length);
+   if (ret < 0)
+   return ret;
if 

[PATCHv2] staging: pi433: pi433_if.c remove SET_CHECKED macro

[Nguyen Phan Quang Minh]

The macro calls its argument -a function- twice, makes the calling
function return prematurely -skipping resource cleanup code- and hurts
understandability.

Signed-off-by: Nguyen Phan Quang Minh 
---
Base on Dan's feedback, I checked and the macro return indeed skips over
some cleanup code, hence remove it.
The code is not pretty, any hints on how to do it better is greatly
appreciated.

 drivers/staging/pi433/pi433_if.c | 237 ---
 1 file changed, 174 insertions(+), 63 deletions(-)

diff --git a/drivers/staging/pi433/pi433_if.c b/drivers/staging/pi433/pi433_if.c
index 3404cb9722c9..c41243f74363 100644
--- a/drivers/staging/pi433/pi433_if.c
+++ b/drivers/staging/pi433/pi433_if.c
@@ -119,12 +119,6 @@ struct pi433_instance {
struct pi433_tx_cfg tx_cfg;
 };
 
-/*-*/
-
-/* macro for checked access of registers of radio module */
-#define SET_CHECKED(retval) \
-   if (retval < 0) \
-   return retval;
 
 /*-*/
 
@@ -183,28 +177,53 @@ rf69_set_rx_cfg(struct pi433_device *dev, struct 
pi433_rx_cfg *rx_cfg)
int payload_length;
 
/* receiver config */
-   SET_CHECKED(rf69_set_frequency  (dev->spi, rx_cfg->frequency));
-   SET_CHECKED(rf69_set_bit_rate   (dev->spi, rx_cfg->bit_rate));
-   SET_CHECKED(rf69_set_modulation (dev->spi, rx_cfg->modulation));
-   SET_CHECKED(rf69_set_antenna_impedance   (dev->spi, 
rx_cfg->antenna_impedance));
-   SET_CHECKED(rf69_set_rssi_threshold  (dev->spi, 
rx_cfg->rssi_threshold));
-   SET_CHECKED(rf69_set_ook_threshold_dec   (dev->spi, 
rx_cfg->thresholdDecrement));
-   SET_CHECKED(rf69_set_bandwidth   (dev->spi, 
rx_cfg->bw_mantisse, rx_cfg->bw_exponent));
-   SET_CHECKED(rf69_set_bandwidth_during_afc(dev->spi, 
rx_cfg->bw_mantisse, rx_cfg->bw_exponent));
-   SET_CHECKED(rf69_set_dagc(dev->spi, rx_cfg->dagc));
+   ret = rf69_set_frequency(dev->spi, rx_cfg->frequency);
+   if (ret < 0)
+   return ret;
+   ret = rf69_set_bit_rate(dev->spi, rx_cfg->bit_rate);
+   if (ret < 0)
+   return ret;
+   ret = rf69_set_modulation(dev->spi, rx_cfg->modulation);
+   if (ret < 0)
+   return ret;
+   ret = rf69_set_antenna_impedance(dev->spi, rx_cfg->antenna_impedance);
+   if (ret < 0)
+   return ret;
+   ret = rf69_set_rssi_threshold(dev->spi, rx_cfg->rssi_threshold);
+   if (ret < 0)
+   return ret;
+   ret = rf69_set_ook_threshold_dec(dev->spi, rx_cfg->thresholdDecrement);
+   if (ret < 0)
+   return ret;
+   ret = rf69_set_bandwidth(dev->spi, rx_cfg->bw_mantisse, 
rx_cfg->bw_exponent);
+   if (ret < 0)
+   return ret;
+   ret = rf69_set_bandwidth_during_afc(dev->spi, rx_cfg->bw_mantisse,
+   rx_cfg->bw_exponent);
+   if (ret < 0)
+   return ret;
+   ret = rf69_set_dagc(dev->spi, rx_cfg->dagc);
+   if (ret < 0)
+   return ret;
 
dev->rx_bytes_to_drop = rx_cfg->bytes_to_drop;
 
/* packet config */
/* enable */
-   SET_CHECKED(rf69_set_sync_enable(dev->spi, rx_cfg->enable_sync));
+   ret = rf69_set_sync_enable(dev->spi, rx_cfg->enable_sync);
+   if (ret < 0)
+   return ret;
if (rx_cfg->enable_sync == optionOn)
{
-   SET_CHECKED(rf69_set_fifo_fill_condition(dev->spi, 
afterSyncInterrupt));
+   ret = rf69_set_fifo_fill_condition(dev->spi, 
afterSyncInterrupt);
+   if (ret < 0)
+   return ret;
}
else
{
-   SET_CHECKED(rf69_set_fifo_fill_condition(dev->spi, always));
+   ret = rf69_set_fifo_fill_condition(dev->spi, always);
+   if (ret < 0)
+   return ret;
}
if (rx_cfg->enable_length_byte == optionOn) {
ret = rf69_set_packet_format(dev->spi, packetLengthVar);
@@ -215,36 +234,54 @@ rf69_set_rx_cfg(struct pi433_device *dev, struct 
pi433_rx_cfg *rx_cfg)
if (ret < 0)
return ret;
}
-   SET_CHECKED(rf69_set_adressFiltering(dev->spi, 
rx_cfg->enable_address_filtering));
-   SET_CHECKED(rf69_set_crc_enable (dev->spi, rx_cfg->enable_crc));
+   ret = rf69_set_adressFiltering(dev->spi, 
rx_cfg->enable_address_filtering);
+   if (ret < 0)
+   return ret;
+   ret = rf69_set_crc_enable(dev->spi, rx_cfg->enable_crc);
+   if (ret < 0)
+   return ret;
 
/* lengths */
-   SET_CHECKED(rf69_set_sync_size(dev->spi, rx_cfg->sync_length));
+   ret = rf69_set_sync_size(dev->spi, rx_cfg->sync_length);
+   if (ret < 0)
+   return ret;
if 

Re: [GIT PULL] hash addresses printed with %p

[Matt Fleming]

(Cc'ing Dave since this is used for kexec on EFI)

On Fri, 01 Dec, at 09:54:43AM, Ard Biesheuvel wrote:
> On 1 December 2017 at 09:48, Greg Kroah-Hartman
>  wrote:
> > On Thu, Nov 30, 2017 at 05:18:42PM +, Ard Biesheuvel wrote:
> >> On 30 November 2017 at 17:10, Greg Kroah-Hartman
> >>  wrote:
> >> > On Thu, Nov 30, 2017 at 04:32:35PM +, Greg Kroah-Hartman wrote:
> >> >> On Wed, Nov 29, 2017 at 01:36:25PM -0800, Linus Torvalds wrote:
> >> >> > On Wed, Nov 29, 2017 at 1:14 PM, Linus Torvalds
> >> >> >  wrote:
> >> >> > >
> >> >> > > Not because %pK itself changed, but because the semantics of %p did.
> >> >> > > The baseline moved, and the "safe" version did not.
> >> >> >
> >> >> > Btw, that baseline for me is now that I can do
> >> >> >
> >> >> >   ./scripts/leaking_addresses.pl | wc -l
> >> >> >   18
> >> >> >
> >> >> > and of those 18 hits, six are false positives (looks like bitmaps in
> >> >> > the uevent keys).
> >> >> >
> >> >> > The remaining 12 are from the EFI runtime map files
> >> >> > (/sys/firmware/efi/runtime-map/*). They should presumably not be
> >> >> > world-readable, but sadly the kset_create_and_add() helper seems to do
> >> >> > that by default.
> >> >> >
> >> >> > I think the sysfs code makes it insanely too easy to make things
> >> >> > world-readable. You try to be careful, and mark things read-only etc,
> >> >> > but __ATTR_RO() jkust means S_IRUGO, which means world-readable.
> >> >> >
> >> >> > There seems to be no convenient model for kobjects having better
> >> >> > permissions. Greg?
> >> >>
> >> >> They can just use __ATTR() which lets you set the exact mode settings
> >> >> that are wanted.
> >> >>
> >> >> Something like the patch below, which breaks the build as the
> >> >> map_attributes are "odd", but you get the idea.  The EFI developers can
> >> >> fix this up properly :)
> >> >>
> >> >> Note, this only accounts for 5 attributes, what is the whole list?
> >> >
> >> > Ah, it's the virt_addr file 12 times, I just ran it on my laptop:
> >> >
> >> > /sys/firmware/efi/runtime-map/7/virt_addr: 0xfffeea6ea000
> >> > /sys/firmware/efi/runtime-map/5/virt_addr: 0xfffeee88b000
> >> > /sys/firmware/efi/runtime-map/3/virt_addr: 0xfffefea0
> >> > /sys/firmware/efi/runtime-map/11/virt_addr: 0xfffed9c0
> >> > /sys/firmware/efi/runtime-map/1/virt_addr: 0xfffefee0
> >> > /sys/firmware/efi/runtime-map/8/virt_addr: 0xfffedba4e000
> >> > /sys/firmware/efi/runtime-map/6/virt_addr: 0xfffeee2de000
> >> > /sys/firmware/efi/runtime-map/4/virt_addr: 0xfffeeea0
> >> > /sys/firmware/efi/runtime-map/2/virt_addr: 0xfffefec0
> >> > /sys/firmware/efi/runtime-map/10/virt_addr: 0xfffed9c6
> >> > /sys/firmware/efi/runtime-map/0/virt_addr: 0xfffeff00
> >> > /sys/firmware/efi/runtime-map/9/virt_addr: 0xfffedb9c9000
> >> >
> >> > So changing it to use __ATTR() should fix this remaning leakage up.
> >> > That is if we even really need to export these values at all.  What does
> >> > userspace do with them?  Shouldn't they just be in debugfs instead?
> >> >
> >>
> >> These are the virtual mappings UEFI firmware regions, which must
> >> remain in the same place across kexec reboots. So kexec tooling
> >> consumes this information and passes it on to the incoming kernel in
> >> some way.
> >>
> >> Note that these are not kernel addresses, so while I agree they should
> >> not be world readable, they won't give you any clue as to where the
> >> kernel itself is mapped.
> >>
> >> So the recommendation is to switch to __ATTR( ... 0400 ... ) instead?
> >> If so, I'll code up a patch.
> >
> > If these pointers are not "real", I recommend just leaving them as-is.
> 
> That's not what I said :-)
> 
> These are real pointers, and stuff will actually be mapped there
> (although I am not intimately familiar with the way x86 does this, but
> on ARM [which does not have these sysfs nodes in the first place],
> these mappings are only live during the time a UEFI runtime service
> call is in progress, and IIRC, work was underway to do the same for
> x86). So while these values don't correlate with the placement of
> kernel data structures, they could still be useful for an attacker to
> figure out where exploitable firmware memory regions are located,
> especially given that some of these may be mapped RWX.

These are mappings of the EFI firmware's runtime regions, dynamically
allocated by the kernel starting at EFI_VA_START. Because we only get
one chance to tell the firmware where we placed its regions (via
SetVirtualAddressMap()) we have to guarantee that any subsequent kexec
reboots use the same addresses.

So that's why they're exported to userspace through sysfs.

Like Ard said, these mappings are not mapped into the regular process
address space. Instead, they're only used while making EFI runtime
calls.

But this is still an information leak. And I 

Re: [GIT PULL] hash addresses printed with %p

[Matt Fleming]

(Cc'ing Dave since this is used for kexec on EFI)

On Fri, 01 Dec, at 09:54:43AM, Ard Biesheuvel wrote:
> On 1 December 2017 at 09:48, Greg Kroah-Hartman
>  wrote:
> > On Thu, Nov 30, 2017 at 05:18:42PM +, Ard Biesheuvel wrote:
> >> On 30 November 2017 at 17:10, Greg Kroah-Hartman
> >>  wrote:
> >> > On Thu, Nov 30, 2017 at 04:32:35PM +, Greg Kroah-Hartman wrote:
> >> >> On Wed, Nov 29, 2017 at 01:36:25PM -0800, Linus Torvalds wrote:
> >> >> > On Wed, Nov 29, 2017 at 1:14 PM, Linus Torvalds
> >> >> >  wrote:
> >> >> > >
> >> >> > > Not because %pK itself changed, but because the semantics of %p did.
> >> >> > > The baseline moved, and the "safe" version did not.
> >> >> >
> >> >> > Btw, that baseline for me is now that I can do
> >> >> >
> >> >> >   ./scripts/leaking_addresses.pl | wc -l
> >> >> >   18
> >> >> >
> >> >> > and of those 18 hits, six are false positives (looks like bitmaps in
> >> >> > the uevent keys).
> >> >> >
> >> >> > The remaining 12 are from the EFI runtime map files
> >> >> > (/sys/firmware/efi/runtime-map/*). They should presumably not be
> >> >> > world-readable, but sadly the kset_create_and_add() helper seems to do
> >> >> > that by default.
> >> >> >
> >> >> > I think the sysfs code makes it insanely too easy to make things
> >> >> > world-readable. You try to be careful, and mark things read-only etc,
> >> >> > but __ATTR_RO() jkust means S_IRUGO, which means world-readable.
> >> >> >
> >> >> > There seems to be no convenient model for kobjects having better
> >> >> > permissions. Greg?
> >> >>
> >> >> They can just use __ATTR() which lets you set the exact mode settings
> >> >> that are wanted.
> >> >>
> >> >> Something like the patch below, which breaks the build as the
> >> >> map_attributes are "odd", but you get the idea.  The EFI developers can
> >> >> fix this up properly :)
> >> >>
> >> >> Note, this only accounts for 5 attributes, what is the whole list?
> >> >
> >> > Ah, it's the virt_addr file 12 times, I just ran it on my laptop:
> >> >
> >> > /sys/firmware/efi/runtime-map/7/virt_addr: 0xfffeea6ea000
> >> > /sys/firmware/efi/runtime-map/5/virt_addr: 0xfffeee88b000
> >> > /sys/firmware/efi/runtime-map/3/virt_addr: 0xfffefea0
> >> > /sys/firmware/efi/runtime-map/11/virt_addr: 0xfffed9c0
> >> > /sys/firmware/efi/runtime-map/1/virt_addr: 0xfffefee0
> >> > /sys/firmware/efi/runtime-map/8/virt_addr: 0xfffedba4e000
> >> > /sys/firmware/efi/runtime-map/6/virt_addr: 0xfffeee2de000
> >> > /sys/firmware/efi/runtime-map/4/virt_addr: 0xfffeeea0
> >> > /sys/firmware/efi/runtime-map/2/virt_addr: 0xfffefec0
> >> > /sys/firmware/efi/runtime-map/10/virt_addr: 0xfffed9c6
> >> > /sys/firmware/efi/runtime-map/0/virt_addr: 0xfffeff00
> >> > /sys/firmware/efi/runtime-map/9/virt_addr: 0xfffedb9c9000
> >> >
> >> > So changing it to use __ATTR() should fix this remaning leakage up.
> >> > That is if we even really need to export these values at all.  What does
> >> > userspace do with them?  Shouldn't they just be in debugfs instead?
> >> >
> >>
> >> These are the virtual mappings UEFI firmware regions, which must
> >> remain in the same place across kexec reboots. So kexec tooling
> >> consumes this information and passes it on to the incoming kernel in
> >> some way.
> >>
> >> Note that these are not kernel addresses, so while I agree they should
> >> not be world readable, they won't give you any clue as to where the
> >> kernel itself is mapped.
> >>
> >> So the recommendation is to switch to __ATTR( ... 0400 ... ) instead?
> >> If so, I'll code up a patch.
> >
> > If these pointers are not "real", I recommend just leaving them as-is.
> 
> That's not what I said :-)
> 
> These are real pointers, and stuff will actually be mapped there
> (although I am not intimately familiar with the way x86 does this, but
> on ARM [which does not have these sysfs nodes in the first place],
> these mappings are only live during the time a UEFI runtime service
> call is in progress, and IIRC, work was underway to do the same for
> x86). So while these values don't correlate with the placement of
> kernel data structures, they could still be useful for an attacker to
> figure out where exploitable firmware memory regions are located,
> especially given that some of these may be mapped RWX.

These are mappings of the EFI firmware's runtime regions, dynamically
allocated by the kernel starting at EFI_VA_START. Because we only get
one chance to tell the firmware where we placed its regions (via
SetVirtualAddressMap()) we have to guarantee that any subsequent kexec
reboots use the same addresses.

So that's why they're exported to userspace through sysfs.

Like Ard said, these mappings are not mapped into the regular process
address space. Instead, they're only used while making EFI runtime
calls.

But this is still an information leak. And I think _ATTR(..0400) is
the right way to fix it. Dave, could you double-check my logic 

Re: [PATCH] mmap.2: MAP_FIXED is no longer discouraged

[Matthew Wilcox]

On Sat, Dec 02, 2017 at 07:49:20PM +0100, Jann Horn wrote:
> On Sat, Dec 2, 2017 at 4:05 PM, Matthew Wilcox  wrote:
> > On Fri, Dec 01, 2017 at 06:16:26PM -0800, john.hubb...@gmail.com wrote:
> >> MAP_FIXED has been widely used for a very long time, yet the man
> >> page still claims that "the use of this option is discouraged".
> >
> > I think we should continue to discourage the use of this option, but
> > I'm going to include some of your text in my replacement paragraph ...
> >
> > -Because requiring a fixed address for a mapping is less portable,
> > -the use of this option is discouraged.
> > +The use of this option is discouraged because it forcibly unmaps any
> > +existing mapping at that address.  Programs which use this option need
> > +to be aware that their memory map may change significantly from one run to
> > +the next, depending on library versions, kernel versions and random 
> > numbers.
> 
> How about adding something explicit about when it's okay to use MAP_FIXED?
> "This option should only be used to displace an existing mapping that is
> controlled by the caller, or part of such a mapping." or something like that?
> 
> > +In a threaded process, checking the existing mappings can race against
> > +a new dynamic library being loaded
> 
> malloc() and its various callers can also cause mmap() calls, which is 
> probably
> more relevant than library loading.

That's a bit more expected though.  "I called malloc and my address
space changed".  Well, yeah.  But "I called getpwnam and my address
space changed" is a bit more surprising.  Don't you think?

Maybe that should be up front rather than buried at the end of the sentence.

"In a multi-threaded process, the address space can change in response to
virtually any library call.  This is because almost any library call may be
implemented by using dlopen(3) to load another shared library, which will be
mapped into the process's address space.  The PAM libraries are an excellent
example, as well as more obvious examples like brk(2), malloc(3) and even
pthread_create(3)."

What do you think?

Re: [PATCH] mmap.2: MAP_FIXED is no longer discouraged

[Matthew Wilcox]

On Sat, Dec 02, 2017 at 07:49:20PM +0100, Jann Horn wrote:
> On Sat, Dec 2, 2017 at 4:05 PM, Matthew Wilcox  wrote:
> > On Fri, Dec 01, 2017 at 06:16:26PM -0800, john.hubb...@gmail.com wrote:
> >> MAP_FIXED has been widely used for a very long time, yet the man
> >> page still claims that "the use of this option is discouraged".
> >
> > I think we should continue to discourage the use of this option, but
> > I'm going to include some of your text in my replacement paragraph ...
> >
> > -Because requiring a fixed address for a mapping is less portable,
> > -the use of this option is discouraged.
> > +The use of this option is discouraged because it forcibly unmaps any
> > +existing mapping at that address.  Programs which use this option need
> > +to be aware that their memory map may change significantly from one run to
> > +the next, depending on library versions, kernel versions and random 
> > numbers.
> 
> How about adding something explicit about when it's okay to use MAP_FIXED?
> "This option should only be used to displace an existing mapping that is
> controlled by the caller, or part of such a mapping." or something like that?
> 
> > +In a threaded process, checking the existing mappings can race against
> > +a new dynamic library being loaded
> 
> malloc() and its various callers can also cause mmap() calls, which is 
> probably
> more relevant than library loading.

That's a bit more expected though.  "I called malloc and my address
space changed".  Well, yeah.  But "I called getpwnam and my address
space changed" is a bit more surprising.  Don't you think?

Maybe that should be up front rather than buried at the end of the sentence.

"In a multi-threaded process, the address space can change in response to
virtually any library call.  This is because almost any library call may be
implemented by using dlopen(3) to load another shared library, which will be
mapped into the process's address space.  The PAM libraries are an excellent
example, as well as more obvious examples like brk(2), malloc(3) and even
pthread_create(3)."

What do you think?

[PATCH] objtool: Fix 64-bit build on 32-bit host

[Josh Poimboeuf]

From: Mikulas Patocka 

The new ORC unwinder breaks the build of a 64-bit kernel on a 32-bit
host.  Building the kernel on a i386 or x32 host fails with:

  orc_dump.c: In function 'orc_dump':
  orc_dump.c:105:26: error: passing argument 2 of 'elf_getshdrnum' from 
incompatible pointer type [-Werror=incompatible-pointer-types]
if (elf_getshdrnum(elf, _sections)) {
^
  In file included from /usr/local/include/gelf.h:32:0,
   from elf.h:22,
   from warn.h:26,
   from orc_dump.c:20:
  /usr/local/include/libelf.h:304:12: note: expected 'size_t * {aka unsigned 
int *}' but argument is of type 'long unsigned int *'
   extern int elf_getshdrnum (Elf *__elf, size_t *__dst);
  ^~
  orc_dump.c:190:17: error: format '%lx' expects argument of type 'long 
unsigned int', but argument 3 has type 'Elf64_Sxword {aka long long int}' 
[-Werror=format=]
  printf("%s+%lx:", name, rela.r_addend);
 ~~^  ~
 %llx

Fix the build failure.

Another problem is that if the user specifies HOSTCC or HOSTLD
variables, they are ignored in the objtool makefile.  Change the
makefile to respect these variables.

Fixes: 627fce14809b ("objtool: Add ORC unwind table generation")
Signed-off-by: Mikulas Patocka 
Signed-off-by: Josh Poimboeuf 
---
 tools/objtool/Makefile   | 8 +---
 tools/objtool/orc_dump.c | 8 +---
 2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/tools/objtool/Makefile b/tools/objtool/Makefile
index 0f94af3ccaaa..ae0272f9a091 100644
--- a/tools/objtool/Makefile
+++ b/tools/objtool/Makefile
@@ -7,9 +7,11 @@ ARCH := x86
 endif
 
 # always use the host compiler
-CC = gcc
-LD = ld
-AR = ar
+HOSTCC ?= gcc
+HOSTLD ?= ld
+CC  = $(HOSTCC)
+LD  = $(HOSTLD)
+AR  = ar
 
 ifeq ($(srctree),)
 srctree := $(patsubst %/,%,$(dir $(CURDIR)))
diff --git a/tools/objtool/orc_dump.c b/tools/objtool/orc_dump.c
index 36c5bf6a2675..301a2ae0e783 100644
--- a/tools/objtool/orc_dump.c
+++ b/tools/objtool/orc_dump.c
@@ -76,7 +76,8 @@ int orc_dump(const char *_objname)
int fd, nr_entries, i, *orc_ip = NULL, orc_size = 0;
struct orc_entry *orc = NULL;
char *name;
-   unsigned long nr_sections, orc_ip_addr = 0;
+   size_t nr_sections;
+   Elf64_Addr orc_ip_addr = 0;
size_t shstrtab_idx;
Elf *elf;
Elf_Scn *scn;
@@ -187,10 +188,11 @@ int orc_dump(const char *_objname)
return -1;
}
 
-   printf("%s+%lx:", name, rela.r_addend);
+   printf("%s+%llx:", name, (unsigned long 
long)rela.r_addend);
 
} else {
-   printf("%lx:", orc_ip_addr + (i * sizeof(int)) + 
orc_ip[i]);
+   printf("%llx:",
+  (unsigned long long)(orc_ip_addr + (i * 
sizeof(int)) + orc_ip[i]));
}
 
 
-- 
2.13.6

[PATCH] objtool: Fix 64-bit build on 32-bit host

[Josh Poimboeuf]

From: Mikulas Patocka 

The new ORC unwinder breaks the build of a 64-bit kernel on a 32-bit
host.  Building the kernel on a i386 or x32 host fails with:

  orc_dump.c: In function 'orc_dump':
  orc_dump.c:105:26: error: passing argument 2 of 'elf_getshdrnum' from 
incompatible pointer type [-Werror=incompatible-pointer-types]
if (elf_getshdrnum(elf, _sections)) {
^
  In file included from /usr/local/include/gelf.h:32:0,
   from elf.h:22,
   from warn.h:26,
   from orc_dump.c:20:
  /usr/local/include/libelf.h:304:12: note: expected 'size_t * {aka unsigned 
int *}' but argument is of type 'long unsigned int *'
   extern int elf_getshdrnum (Elf *__elf, size_t *__dst);
  ^~
  orc_dump.c:190:17: error: format '%lx' expects argument of type 'long 
unsigned int', but argument 3 has type 'Elf64_Sxword {aka long long int}' 
[-Werror=format=]
  printf("%s+%lx:", name, rela.r_addend);
 ~~^  ~
 %llx

Fix the build failure.

Another problem is that if the user specifies HOSTCC or HOSTLD
variables, they are ignored in the objtool makefile.  Change the
makefile to respect these variables.

Fixes: 627fce14809b ("objtool: Add ORC unwind table generation")
Signed-off-by: Mikulas Patocka 
Signed-off-by: Josh Poimboeuf 
---
 tools/objtool/Makefile   | 8 +---
 tools/objtool/orc_dump.c | 8 +---
 2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/tools/objtool/Makefile b/tools/objtool/Makefile
index 0f94af3ccaaa..ae0272f9a091 100644
--- a/tools/objtool/Makefile
+++ b/tools/objtool/Makefile
@@ -7,9 +7,11 @@ ARCH := x86
 endif
 
 # always use the host compiler
-CC = gcc
-LD = ld
-AR = ar
+HOSTCC ?= gcc
+HOSTLD ?= ld
+CC  = $(HOSTCC)
+LD  = $(HOSTLD)
+AR  = ar
 
 ifeq ($(srctree),)
 srctree := $(patsubst %/,%,$(dir $(CURDIR)))
diff --git a/tools/objtool/orc_dump.c b/tools/objtool/orc_dump.c
index 36c5bf6a2675..301a2ae0e783 100644
--- a/tools/objtool/orc_dump.c
+++ b/tools/objtool/orc_dump.c
@@ -76,7 +76,8 @@ int orc_dump(const char *_objname)
int fd, nr_entries, i, *orc_ip = NULL, orc_size = 0;
struct orc_entry *orc = NULL;
char *name;
-   unsigned long nr_sections, orc_ip_addr = 0;
+   size_t nr_sections;
+   Elf64_Addr orc_ip_addr = 0;
size_t shstrtab_idx;
Elf *elf;
Elf_Scn *scn;
@@ -187,10 +188,11 @@ int orc_dump(const char *_objname)
return -1;
}
 
-   printf("%s+%lx:", name, rela.r_addend);
+   printf("%s+%llx:", name, (unsigned long 
long)rela.r_addend);
 
} else {
-   printf("%lx:", orc_ip_addr + (i * sizeof(int)) + 
orc_ip[i]);
+   printf("%llx:",
+  (unsigned long long)(orc_ip_addr + (i * 
sizeof(int)) + orc_ip[i]));
}
 
 
-- 
2.13.6

Re: netfilter: xt_bpf: Fix XT_BPF_MODE_FD_PINNED mode of 'xt_bpf_info_v1'

[Al Viro]

On Sat, Dec 02, 2017 at 06:48:50PM +, Al Viro wrote:
> On Fri, Dec 01, 2017 at 09:47:00PM +0100, Daniel Borkmann wrote:
> 
> > > Might want to replace security_path_mknod() with something saner, while 
> > > we are
> > > at it.
> > > 
> > > Objections?
> > 
> > No, thanks for looking into this, and sorry for this fugly hack! :( Not
> > that this doesn't make it any better, but I think back then I took it
> > over from mqueue implementation ... should have known better and looking
> > into making this generic instead, sigh. The above looks good to me, so
> > no objections from my side and thanks for working on it!
> > 
> > > PS: mqueue.c would also benefit from such primitive - do_create() there 
> > > would
> > > simply pass attr as callback's argument into vfs_mkobj(), with callback 
> > > being
> > > the guts of mqueue_create()...
> 
> OK...  See vfs.git#untested.mkobj; it really needs testing, though - 
> mq_open(2)
> passes LTP tests, but that's not saying much, and BPF side is completely
> untested.

... and FWIW, completely untested patch for net/netfilter/xt_bpf.c follows:

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index e55e4255a210..a7000e4775e7 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -514,6 +514,9 @@ static inline struct bpf_prog *bpf_prog_get_type(u32 ufd,
return bpf_prog_get_type_dev(ufd, type, false);
 }
 
+struct bpf_prog *bpf_prog_get_type_path(const char *name, enum bpf_prog_type 
type);
+bool bpf_prog_get_ok(struct bpf_prog *, enum bpf_prog_type *, bool);
+
 int bpf_prog_offload_compile(struct bpf_prog *prog);
 void bpf_prog_offload_destroy(struct bpf_prog *prog);
 
diff --git a/kernel/bpf/inode.c b/kernel/bpf/inode.c
index 2b75faccc771..9d1050dc2a7a 100644
--- a/kernel/bpf/inode.c
+++ b/kernel/bpf/inode.c
@@ -364,6 +364,45 @@ int bpf_obj_get_user(const char __user *pathname, int 
flags)
 }
 EXPORT_SYMBOL_GPL(bpf_obj_get_user);
 
+static struct bpf_prog *__get_prog_inode(struct inode *inode, enum 
bpf_prog_type type)
+{
+   struct bpf_prog *prog;
+   int ret = inode_permission(inode, MAY_READ | MAY_WRITE);
+   if (ret)
+   return ERR_PTR(ret);
+
+   if (inode->i_op == _map_iops)
+   return ERR_PTR(-EINVAL);
+   if (inode->i_op != _prog_iops)
+   return ERR_PTR(-EACCES);
+
+   prog = inode->i_private;
+
+   ret = security_bpf_prog(prog);
+   if (ret < 0)
+   return ERR_PTR(ret);
+
+   if (!bpf_prog_get_ok(prog, , false))
+   return ERR_PTR(-EINVAL);
+
+   return bpf_prog_inc(prog);
+}
+
+struct bpf_prog *bpf_prog_get_type_path(const char *name, enum bpf_prog_type 
type)
+{
+   struct bpf_prog *prog;
+   struct path path;
+   int ret = kern_path(name, LOOKUP_FOLLOW, );
+   if (ret)
+   return ERR_PTR(ret);
+   prog = __get_prog_inode(d_backing_inode(path.dentry), type);
+   if (!IS_ERR(prog))
+   touch_atime();
+   path_put();
+   return prog;
+}
+EXPORT_SYMBOL(bpf_prog_get_type_path);
+
 static void bpf_evict_inode(struct inode *inode)
 {
enum bpf_type type;
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 2c4cfeaa8d5e..5cb783fc8224 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -1057,7 +1057,7 @@ struct bpf_prog *bpf_prog_inc_not_zero(struct bpf_prog 
*prog)
 }
 EXPORT_SYMBOL_GPL(bpf_prog_inc_not_zero);
 
-static bool bpf_prog_get_ok(struct bpf_prog *prog,
+bool bpf_prog_get_ok(struct bpf_prog *prog,
enum bpf_prog_type *attach_type, bool attach_drv)
 {
/* not an attachment, just a refcount inc, always allow */
diff --git a/net/netfilter/xt_bpf.c b/net/netfilter/xt_bpf.c
index 041da0d9c06f..fa2ca0a13619 100644
--- a/net/netfilter/xt_bpf.c
+++ b/net/netfilter/xt_bpf.c
@@ -52,18 +52,8 @@ static int __bpf_mt_check_fd(int fd, struct bpf_prog **ret)
 
 static int __bpf_mt_check_path(const char *path, struct bpf_prog **ret)
 {
-   mm_segment_t oldfs = get_fs();
-   int retval, fd;
-
-   set_fs(KERNEL_DS);
-   fd = bpf_obj_get_user(path, 0);
-   set_fs(oldfs);
-   if (fd < 0)
-   return fd;
-
-   retval = __bpf_mt_check_fd(fd, ret);
-   sys_close(fd);
-   return retval;
+   *ret = bpf_prog_get_type_path(path, BPF_PROG_TYPE_SOCKET_FILTER);
+   return PTR_ERR_OR_ZERO(*ret);
 }
 
 static int bpf_mt_check(const struct xt_mtchk_param *par)

Re: netfilter: xt_bpf: Fix XT_BPF_MODE_FD_PINNED mode of 'xt_bpf_info_v1'

[Al Viro]

On Sat, Dec 02, 2017 at 06:48:50PM +, Al Viro wrote:
> On Fri, Dec 01, 2017 at 09:47:00PM +0100, Daniel Borkmann wrote:
> 
> > > Might want to replace security_path_mknod() with something saner, while 
> > > we are
> > > at it.
> > > 
> > > Objections?
> > 
> > No, thanks for looking into this, and sorry for this fugly hack! :( Not
> > that this doesn't make it any better, but I think back then I took it
> > over from mqueue implementation ... should have known better and looking
> > into making this generic instead, sigh. The above looks good to me, so
> > no objections from my side and thanks for working on it!
> > 
> > > PS: mqueue.c would also benefit from such primitive - do_create() there 
> > > would
> > > simply pass attr as callback's argument into vfs_mkobj(), with callback 
> > > being
> > > the guts of mqueue_create()...
> 
> OK...  See vfs.git#untested.mkobj; it really needs testing, though - 
> mq_open(2)
> passes LTP tests, but that's not saying much, and BPF side is completely
> untested.

... and FWIW, completely untested patch for net/netfilter/xt_bpf.c follows:

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index e55e4255a210..a7000e4775e7 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -514,6 +514,9 @@ static inline struct bpf_prog *bpf_prog_get_type(u32 ufd,
return bpf_prog_get_type_dev(ufd, type, false);
 }
 
+struct bpf_prog *bpf_prog_get_type_path(const char *name, enum bpf_prog_type 
type);
+bool bpf_prog_get_ok(struct bpf_prog *, enum bpf_prog_type *, bool);
+
 int bpf_prog_offload_compile(struct bpf_prog *prog);
 void bpf_prog_offload_destroy(struct bpf_prog *prog);
 
diff --git a/kernel/bpf/inode.c b/kernel/bpf/inode.c
index 2b75faccc771..9d1050dc2a7a 100644
--- a/kernel/bpf/inode.c
+++ b/kernel/bpf/inode.c
@@ -364,6 +364,45 @@ int bpf_obj_get_user(const char __user *pathname, int 
flags)
 }
 EXPORT_SYMBOL_GPL(bpf_obj_get_user);
 
+static struct bpf_prog *__get_prog_inode(struct inode *inode, enum 
bpf_prog_type type)
+{
+   struct bpf_prog *prog;
+   int ret = inode_permission(inode, MAY_READ | MAY_WRITE);
+   if (ret)
+   return ERR_PTR(ret);
+
+   if (inode->i_op == _map_iops)
+   return ERR_PTR(-EINVAL);
+   if (inode->i_op != _prog_iops)
+   return ERR_PTR(-EACCES);
+
+   prog = inode->i_private;
+
+   ret = security_bpf_prog(prog);
+   if (ret < 0)
+   return ERR_PTR(ret);
+
+   if (!bpf_prog_get_ok(prog, , false))
+   return ERR_PTR(-EINVAL);
+
+   return bpf_prog_inc(prog);
+}
+
+struct bpf_prog *bpf_prog_get_type_path(const char *name, enum bpf_prog_type 
type)
+{
+   struct bpf_prog *prog;
+   struct path path;
+   int ret = kern_path(name, LOOKUP_FOLLOW, );
+   if (ret)
+   return ERR_PTR(ret);
+   prog = __get_prog_inode(d_backing_inode(path.dentry), type);
+   if (!IS_ERR(prog))
+   touch_atime();
+   path_put();
+   return prog;
+}
+EXPORT_SYMBOL(bpf_prog_get_type_path);
+
 static void bpf_evict_inode(struct inode *inode)
 {
enum bpf_type type;
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 2c4cfeaa8d5e..5cb783fc8224 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -1057,7 +1057,7 @@ struct bpf_prog *bpf_prog_inc_not_zero(struct bpf_prog 
*prog)
 }
 EXPORT_SYMBOL_GPL(bpf_prog_inc_not_zero);
 
-static bool bpf_prog_get_ok(struct bpf_prog *prog,
+bool bpf_prog_get_ok(struct bpf_prog *prog,
enum bpf_prog_type *attach_type, bool attach_drv)
 {
/* not an attachment, just a refcount inc, always allow */
diff --git a/net/netfilter/xt_bpf.c b/net/netfilter/xt_bpf.c
index 041da0d9c06f..fa2ca0a13619 100644
--- a/net/netfilter/xt_bpf.c
+++ b/net/netfilter/xt_bpf.c
@@ -52,18 +52,8 @@ static int __bpf_mt_check_fd(int fd, struct bpf_prog **ret)
 
 static int __bpf_mt_check_path(const char *path, struct bpf_prog **ret)
 {
-   mm_segment_t oldfs = get_fs();
-   int retval, fd;
-
-   set_fs(KERNEL_DS);
-   fd = bpf_obj_get_user(path, 0);
-   set_fs(oldfs);
-   if (fd < 0)
-   return fd;
-
-   retval = __bpf_mt_check_fd(fd, ret);
-   sys_close(fd);
-   return retval;
+   *ret = bpf_prog_get_type_path(path, BPF_PROG_TYPE_SOCKET_FILTER);
+   return PTR_ERR_OR_ZERO(*ret);
 }
 
 static int bpf_mt_check(const struct xt_mtchk_param *par)

Re: [PATCH v2 11/18] drm/sun4i: Add A83T support

[Jernej Škrabec]

Dne četrtek, 30. november 2017 ob 16:33:12 CET je Maxime Ripard napisal(a):
> On Tue, Nov 28, 2017 at 11:33:44PM +0100, Jernej Škrabec wrote:
> > Hi!
> > 
> > Dne torek, 28. november 2017 ob 23:00:14 CET je Maxime Ripard napisal(a):
> > > On Tue, Nov 28, 2017 at 04:48:55PM +0100, Jernej Škrabec wrote:
> > > > > On Mon, Nov 27, 2017 at 05:01:49PM +0100, Jernej Škrabec wrote:
> > > > > > Dne ponedeljek, 27. november 2017 ob 16:41:35 CET je Maxime Ripard
> > > > 
> > > > napisal(a):
> > > > > > > Add support for the A83T display pipeline.
> > > > > > > 
> > > > > > > Reviewed-by: Chen-Yu Tsai 
> > > > > > > Signed-off-by: Maxime Ripard 
> > > > > > > ---
> > > > > > > 
> > > > > > >  Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt |
> > > > > > >  3
> > > > > > >  +++
> > > > > > >  drivers/gpu/drm/sun4i/sun4i_drv.c |
> > > > > > >  2
> > > > > > >  ++
> > > > > > >  drivers/gpu/drm/sun4i/sun4i_tcon.c|
> > > > > > >  5
> > > > > > >  +
> > > > > > >  drivers/gpu/drm/sun4i/sun8i_mixer.c   |
> > > > > > >  4
> > > > > > >  
> > > > > > >  4 files changed, 14 insertions(+)
> > > > > > > 
> > > > > > > diff --git
> > > > > > > a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt
> > > > > > > b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt
> > > > > > > index
> > > > > > > d4259a4f5171..d6b52e5c48c0 100644
> > > > > > > ---
> > > > > > > a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt
> > > > > > > +++
> > > > > > > b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt
> > > > > > > 
> > > > > > > @@ -93,6 +93,7 @@ Required properties:
> > > > > > > * allwinner,sun6i-a31s-tcon
> > > > > > > * allwinner,sun7i-a20-tcon
> > > > > > > * allwinner,sun8i-a33-tcon
> > > > > > > 
> > > > > > > +   * allwinner,sun8i-a83t-tcon-lcd
> > > > > > > 
> > > > > > > * allwinner,sun8i-v3s-tcon
> > > > > > >   
> > > > > > >   - reg: base address and size of memory-mapped region
> > > > > > >   - interrupts: interrupt associated to this IP
> > > > > > > 
> > > > > > > @@ -224,6 +225,7 @@ supported.
> > > > > > > 
> > > > > > >  Required properties:
> > > > > > >- compatible: value must be one of:
> > > > > > > +* allwinner,sun8i-a83t-de2-mixer
> > > > > > 
> > > > > > What will be the name of the second mixer, once support for HDMI
> > > > > > is
> > > > > > added?
> > > > > > Should we start directly with 0 and 1 postfix ?
> > > > > 
> > > > > What are the differences exactly without the two mixers?
> > > > 
> > > > Mixer properties:
> > > > - mixer index (0 or 1), important for determining CCSC base (see my
> > > > patches)
> > > 
> > > Is that the only thing we need to determine?
> > 
> > For now, mixer index is important only for determining CCSC base in
> > conjuction with VEP capability. Obviously, I can't exclude that there is
> > some other case where that mixer index is needed.
> 
> That's unfortunate...

I take a deeper look today about that issue and it seems that mixer index is 
really needed only here. I did only regex search and not line by line check, 
so take this with grain of salt.

Additionally, it seems that this comparison is kind of a hack for V3s. Channel 
output CSC is done through VEP (video enhancement processor, maybe?) unit. 
Those units are only on VI channels. But every SoC with DE2 I checked except 
V3s have 1 VI channel and VEP enabled on first mixer. V3s on the other hand 
have 2 VI channels and no VEP, but same addresses should work for channel 
output CSC according to the code. Of course, that is only a theory, but if you 
want, we can go with quirks structure. Or better yet, with quirk property, 
since there may be some (future?) SoC with two mixers and no VEP unit 
supported.

BTW, how should be DE3 on H6 handled in the future? Registers offsets seems to 
be same within same DE2 and DE3 units, just extended. However, some base 
addresses are different, for example, those connected to VEP. I don't think 
new driver is needed, just some quirks structure which would tell DE version.

Best regards,
Jernej

> 
> > Can't we just add reg property for that?
> 
> No, reg is here specifically for the bus address, not for an index,
> and in general, indices are poorly perceived and have been subject to
> a lot of debate in the past. Hence why I'd really like to avoid any
> solution looking like this. But I guess we don't really have the
> choice either.
> 
> Maxime
> 
> --
> Maxime Ripard, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com

Re: [PATCH v2 11/18] drm/sun4i: Add A83T support

[Jernej Škrabec]

Dne četrtek, 30. november 2017 ob 16:33:12 CET je Maxime Ripard napisal(a):
> On Tue, Nov 28, 2017 at 11:33:44PM +0100, Jernej Škrabec wrote:
> > Hi!
> > 
> > Dne torek, 28. november 2017 ob 23:00:14 CET je Maxime Ripard napisal(a):
> > > On Tue, Nov 28, 2017 at 04:48:55PM +0100, Jernej Škrabec wrote:
> > > > > On Mon, Nov 27, 2017 at 05:01:49PM +0100, Jernej Škrabec wrote:
> > > > > > Dne ponedeljek, 27. november 2017 ob 16:41:35 CET je Maxime Ripard
> > > > 
> > > > napisal(a):
> > > > > > > Add support for the A83T display pipeline.
> > > > > > > 
> > > > > > > Reviewed-by: Chen-Yu Tsai 
> > > > > > > Signed-off-by: Maxime Ripard 
> > > > > > > ---
> > > > > > > 
> > > > > > >  Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt |
> > > > > > >  3
> > > > > > >  +++
> > > > > > >  drivers/gpu/drm/sun4i/sun4i_drv.c |
> > > > > > >  2
> > > > > > >  ++
> > > > > > >  drivers/gpu/drm/sun4i/sun4i_tcon.c|
> > > > > > >  5
> > > > > > >  +
> > > > > > >  drivers/gpu/drm/sun4i/sun8i_mixer.c   |
> > > > > > >  4
> > > > > > >  
> > > > > > >  4 files changed, 14 insertions(+)
> > > > > > > 
> > > > > > > diff --git
> > > > > > > a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt
> > > > > > > b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt
> > > > > > > index
> > > > > > > d4259a4f5171..d6b52e5c48c0 100644
> > > > > > > ---
> > > > > > > a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt
> > > > > > > +++
> > > > > > > b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt
> > > > > > > 
> > > > > > > @@ -93,6 +93,7 @@ Required properties:
> > > > > > > * allwinner,sun6i-a31s-tcon
> > > > > > > * allwinner,sun7i-a20-tcon
> > > > > > > * allwinner,sun8i-a33-tcon
> > > > > > > 
> > > > > > > +   * allwinner,sun8i-a83t-tcon-lcd
> > > > > > > 
> > > > > > > * allwinner,sun8i-v3s-tcon
> > > > > > >   
> > > > > > >   - reg: base address and size of memory-mapped region
> > > > > > >   - interrupts: interrupt associated to this IP
> > > > > > > 
> > > > > > > @@ -224,6 +225,7 @@ supported.
> > > > > > > 
> > > > > > >  Required properties:
> > > > > > >- compatible: value must be one of:
> > > > > > > +* allwinner,sun8i-a83t-de2-mixer
> > > > > > 
> > > > > > What will be the name of the second mixer, once support for HDMI
> > > > > > is
> > > > > > added?
> > > > > > Should we start directly with 0 and 1 postfix ?
> > > > > 
> > > > > What are the differences exactly without the two mixers?
> > > > 
> > > > Mixer properties:
> > > > - mixer index (0 or 1), important for determining CCSC base (see my
> > > > patches)
> > > 
> > > Is that the only thing we need to determine?
> > 
> > For now, mixer index is important only for determining CCSC base in
> > conjuction with VEP capability. Obviously, I can't exclude that there is
> > some other case where that mixer index is needed.
> 
> That's unfortunate...

I take a deeper look today about that issue and it seems that mixer index is 
really needed only here. I did only regex search and not line by line check, 
so take this with grain of salt.

Additionally, it seems that this comparison is kind of a hack for V3s. Channel 
output CSC is done through VEP (video enhancement processor, maybe?) unit. 
Those units are only on VI channels. But every SoC with DE2 I checked except 
V3s have 1 VI channel and VEP enabled on first mixer. V3s on the other hand 
have 2 VI channels and no VEP, but same addresses should work for channel 
output CSC according to the code. Of course, that is only a theory, but if you 
want, we can go with quirks structure. Or better yet, with quirk property, 
since there may be some (future?) SoC with two mixers and no VEP unit 
supported.

BTW, how should be DE3 on H6 handled in the future? Registers offsets seems to 
be same within same DE2 and DE3 units, just extended. However, some base 
addresses are different, for example, those connected to VEP. I don't think 
new driver is needed, just some quirks structure which would tell DE version.

Best regards,
Jernej

> 
> > Can't we just add reg property for that?
> 
> No, reg is here specifically for the bus address, not for an index,
> and in general, indices are poorly perceived and have been subject to
> a lot of debate in the past. Hence why I'd really like to avoid any
> solution looking like this. But I guess we don't really have the
> choice either.
> 
> Maxime
> 
> --
> Maxime Ripard, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com

[PATCH/RFC] DT: leds: Fix 'label' property description and add 'colour' property

[Jacek Anaszewski]

Label property was imposed a uniqueness requirement, which was erroneous,
since ePAPR defines it to "a human readable string describing a device".

Also the binding description misleadingly suggested direct usage of label
for LED class device name, whereas it should only define a LED function.

Therefore an additional 'colour' property is being introduced, which together
with the parent DT node name used for devicename shall be used for naming LED
class device according to the patterh ::.

Fixes: 116b8e164116 ("DT: leds: Add uniqueness requirement for 'label' 
property")
Signed-off-by: Jacek Anaszewski 
---
 Documentation/devicetree/bindings/leds/common.txt | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/leds/common.txt 
b/Documentation/devicetree/bindings/leds/common.txt
index 1d4afe9..16973ee 100644
--- a/Documentation/devicetree/bindings/leds/common.txt
+++ b/Documentation/devicetree/bindings/leds/common.txt
@@ -14,10 +14,9 @@ Optional properties for child nodes:
 - led-sources : List of device current outputs the LED is connected to. The
outputs are identified by the numbers that must be defined
in the LED device binding documentation.
-- label : The label for this LED. If omitted, the label is taken from the node
- name (excluding the unit address). It has to uniquely identify
- a device, i.e. no other LED class device can be assigned the same
- label.
+- label : The label for this LED. It should describe its function. If omitted,
+  the label is taken from the node name (excluding the unit address).
+- colour : Colour of the LED.
 
 - default-state : The initial state of the LED. Valid values are "on", "off",
   and "keep". If the LED is already on or off and the default-state property is
-- 
2.1.4

[PATCH/RFC] DT: leds: Fix 'label' property description and add 'colour' property

[Jacek Anaszewski]

Label property was imposed a uniqueness requirement, which was erroneous,
since ePAPR defines it to "a human readable string describing a device".

Also the binding description misleadingly suggested direct usage of label
for LED class device name, whereas it should only define a LED function.

Therefore an additional 'colour' property is being introduced, which together
with the parent DT node name used for devicename shall be used for naming LED
class device according to the patterh ::.

Fixes: 116b8e164116 ("DT: leds: Add uniqueness requirement for 'label' 
property")
Signed-off-by: Jacek Anaszewski 
---
 Documentation/devicetree/bindings/leds/common.txt | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/leds/common.txt 
b/Documentation/devicetree/bindings/leds/common.txt
index 1d4afe9..16973ee 100644
--- a/Documentation/devicetree/bindings/leds/common.txt
+++ b/Documentation/devicetree/bindings/leds/common.txt
@@ -14,10 +14,9 @@ Optional properties for child nodes:
 - led-sources : List of device current outputs the LED is connected to. The
outputs are identified by the numbers that must be defined
in the LED device binding documentation.
-- label : The label for this LED. If omitted, the label is taken from the node
- name (excluding the unit address). It has to uniquely identify
- a device, i.e. no other LED class device can be assigned the same
- label.
+- label : The label for this LED. It should describe its function. If omitted,
+  the label is taken from the node name (excluding the unit address).
+- colour : Colour of the LED.
 
 - default-state : The initial state of the LED. Valid values are "on", "off",
   and "keep". If the LED is already on or off and the default-state property is
-- 
2.1.4