Linux Documentation
 help / color / mirror / Atom feed
* Re: [PATCH v2 3/9] rcu/sync: Remove custom check for reader-section
From: Joel Fernandes @ 2019-07-13 15:36 UTC (permalink / raw)
  To: Paul E. McKenney
  Cc: linux-kernel, Oleg Nesterov, Alexey Kuznetsov, Bjorn Helgaas,
	Borislav Petkov, c0d1n61at3, David S. Miller, edumazet,
	Greg Kroah-Hartman, Hideaki YOSHIFUJI, H. Peter Anvin,
	Ingo Molnar, Jonathan Corbet, Josh Triplett, keescook,
	kernel-hardening, kernel-team, Lai Jiangshan, Len Brown,
	linux-acpi, linux-doc, linux-pci, linux-pm, Mathieu Desnoyers,
	neilb, netdev, Pavel Machek, peterz, Rafael J. Wysocki,
	Rasmus Villemoes, rcu, Steven Rostedt, Tejun Heo, Thomas Gleixner,
	will, maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)
In-Reply-To: <20190713144108.GD26519@linux.ibm.com>

On Sat, Jul 13, 2019 at 07:41:08AM -0700, Paul E. McKenney wrote:
> On Sat, Jul 13, 2019 at 09:30:49AM -0400, Joel Fernandes wrote:
> > On Sat, Jul 13, 2019 at 01:21:14AM -0700, Paul E. McKenney wrote:
> > > On Fri, Jul 12, 2019 at 11:10:08PM -0400, Joel Fernandes wrote:
> > > > On Fri, Jul 12, 2019 at 11:01:50PM -0400, Joel Fernandes wrote:
> > > > > On Fri, Jul 12, 2019 at 04:32:06PM -0700, Paul E. McKenney wrote:
> > > > > > On Fri, Jul 12, 2019 at 05:35:59PM -0400, Joel Fernandes wrote:
> > > > > > > On Fri, Jul 12, 2019 at 01:00:18PM -0400, Joel Fernandes (Google) wrote:
> > > > > > > > The rcu/sync code was doing its own check whether we are in a reader
> > > > > > > > section. With RCU consolidating flavors and the generic helper added in
> > > > > > > > this series, this is no longer need. We can just use the generic helper
> > > > > > > > and it results in a nice cleanup.
> > > > > > > > 
> > > > > > > > Cc: Oleg Nesterov <oleg@redhat.com>
> > > > > > > > Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
> > > > > > > 
> > > > > > > Hi Oleg,
> > > > > > > Slightly unrelated to the patch,
> > > > > > > I tried hard to understand this comment below in percpu_down_read() but no dice.
> > > > > > > 
> > > > > > > I do understand how rcu sync and percpu rwsem works, however the comment
> > > > > > > below didn't make much sense to me. For one, there's no readers_fast anymore
> > > > > > > so I did not follow what readers_fast means. Could the comment be updated to
> > > > > > > reflect latest changes?
> > > > > > > Also could you help understand how is a writer not able to change
> > > > > > > sem->state and count the per-cpu read counters at the same time as the
> > > > > > > comment tries to say?
> > > > > > > 
> > > > > > > 	/*
> > > > > > > 	 * We are in an RCU-sched read-side critical section, so the writer
> > > > > > > 	 * cannot both change sem->state from readers_fast and start checking
> > > > > > > 	 * counters while we are here. So if we see !sem->state, we know that
> > > > > > > 	 * the writer won't be checking until we're past the preempt_enable()
> > > > > > > 	 * and that once the synchronize_rcu() is done, the writer will see
> > > > > > > 	 * anything we did within this RCU-sched read-size critical section.
> > > > > > > 	 */
> > > > > > > 
> > > > > > > Also,
> > > > > > > I guess we could get rid of all of the gp_ops struct stuff now that since all
> > > > > > > the callbacks are the same now. I will post that as a follow-up patch to this
> > > > > > > series.
> > > > > > 
> > > > > > Hello, Joel,
> > > > > > 
> > > > > > Oleg has a set of patches updating this code that just hit mainline
> > > > > > this week.  These patches get rid of the code that previously handled
> > > > > > RCU's multiple flavors.  Or are you looking at current mainline and
> > > > > > me just missing your point?
> > > > > > 
> > > > > 
> > > > > Hi Paul,
> > > > > You are right on point. I have a bad habit of not rebasing my trees. In this
> > > > > case the feature branch of mine in concern was based on v5.1. Needless to
> > > > > say, I need to rebase my tree.
> > > > > 
> > > > > Yes, this sync clean up patch does conflict when I rebase, but other patches
> > > > > rebase just fine.
> > > > > 
> > > > > The 2 options I see are:
> > > > > 1. Let us drop this patch for now and I resend it later.
> > > > > 2. I resend all patches based on Linus's master branch.
> > > > 
> > > > Below is the updated patch based on Linus master branch:
> > > > 
> > > > ---8<-----------------------
> > > > 
> > > > >From 5f40c9a07fcf3d6dafc2189599d0ba9443097d0f Mon Sep 17 00:00:00 2001
> > > > From: "Joel Fernandes (Google)" <joel@joelfernandes.org>
> > > > Date: Fri, 12 Jul 2019 12:13:27 -0400
> > > > Subject: [PATCH v2.1 3/9] rcu/sync: Remove custom check for reader-section
> > > > 
> > > > The rcu/sync code was doing its own check whether we are in a reader
> > > > section. With RCU consolidating flavors and the generic helper added in
> > > > this series, this is no longer need. We can just use the generic helper
> > > > and it results in a nice cleanup.
> > > > 
> > > > Cc: Oleg Nesterov <oleg@redhat.com>
> > > > Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
> > > > ---
> > > >  include/linux/rcu_sync.h | 4 +---
> > > >  1 file changed, 1 insertion(+), 3 deletions(-)
> > > > 
> > > > diff --git a/include/linux/rcu_sync.h b/include/linux/rcu_sync.h
> > > > index 9b83865d24f9..0027d4c8087c 100644
> > > > --- a/include/linux/rcu_sync.h
> > > > +++ b/include/linux/rcu_sync.h
> > > > @@ -31,9 +31,7 @@ struct rcu_sync {
> > > >   */
> > > >  static inline bool rcu_sync_is_idle(struct rcu_sync *rsp)
> > > >  {
> > > > -	RCU_LOCKDEP_WARN(!rcu_read_lock_held() &&
> > > > -			 !rcu_read_lock_bh_held() &&
> > > > -			 !rcu_read_lock_sched_held(),
> > > > +	RCU_LOCKDEP_WARN(!rcu_read_lock_any_held(),
> > > 
> > > I believe that replacing rcu_read_lock_sched_held() with preemptible()
> > > in a CONFIG_PREEMPT=n kernel will give you false-positive splats here.
> > > If you have not already done so, could you please give it a try?
> > 
> > Hi Paul,
> > I don't think it will cause splats for !CONFIG_PREEMPT.
> > 
> > Currently, rcu_read_lock_any_held() introduced in this patch returns true if
> > !preemptible(). This means that:
> > 
> > The following expression above:
> > RCU_LOCKDEP_WARN(!rcu_read_lock_any_held(),...)
> > 
> > Becomes:
> > RCU_LOCKDEP_WARN(preemptible(), ...)
> > 
> > For, CONFIG_PREEMPT=n kernels, this means:
> > RCU_LOCKDEP_WARN(0, ...)
> > 
> > Which would mean no splats. Or, did I miss the point?
> 
> I suggest trying it out on a CONFIG_PREEMPT=n kernel.

Sure, will do, sorry did not try it out yet because was busy with weekend
chores but will do soon, thanks!


^ permalink raw reply

* Re: [PATCH v2 3/9] rcu/sync: Remove custom check for reader-section
From: Paul E. McKenney @ 2019-07-13 14:41 UTC (permalink / raw)
  To: Joel Fernandes
  Cc: linux-kernel, Oleg Nesterov, Alexey Kuznetsov, Bjorn Helgaas,
	Borislav Petkov, c0d1n61at3, David S. Miller, edumazet,
	Greg Kroah-Hartman, Hideaki YOSHIFUJI, H. Peter Anvin,
	Ingo Molnar, Jonathan Corbet, Josh Triplett, keescook,
	kernel-hardening, kernel-team, Lai Jiangshan, Len Brown,
	linux-acpi, linux-doc, linux-pci, linux-pm, Mathieu Desnoyers,
	neilb, netdev, Pavel Machek, peterz, Rafael J. Wysocki,
	Rasmus Villemoes, rcu, Steven Rostedt, Tejun Heo, Thomas Gleixner,
	will, maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)
In-Reply-To: <20190713133049.GA133650@google.com>

On Sat, Jul 13, 2019 at 09:30:49AM -0400, Joel Fernandes wrote:
> On Sat, Jul 13, 2019 at 01:21:14AM -0700, Paul E. McKenney wrote:
> > On Fri, Jul 12, 2019 at 11:10:08PM -0400, Joel Fernandes wrote:
> > > On Fri, Jul 12, 2019 at 11:01:50PM -0400, Joel Fernandes wrote:
> > > > On Fri, Jul 12, 2019 at 04:32:06PM -0700, Paul E. McKenney wrote:
> > > > > On Fri, Jul 12, 2019 at 05:35:59PM -0400, Joel Fernandes wrote:
> > > > > > On Fri, Jul 12, 2019 at 01:00:18PM -0400, Joel Fernandes (Google) wrote:
> > > > > > > The rcu/sync code was doing its own check whether we are in a reader
> > > > > > > section. With RCU consolidating flavors and the generic helper added in
> > > > > > > this series, this is no longer need. We can just use the generic helper
> > > > > > > and it results in a nice cleanup.
> > > > > > > 
> > > > > > > Cc: Oleg Nesterov <oleg@redhat.com>
> > > > > > > Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
> > > > > > 
> > > > > > Hi Oleg,
> > > > > > Slightly unrelated to the patch,
> > > > > > I tried hard to understand this comment below in percpu_down_read() but no dice.
> > > > > > 
> > > > > > I do understand how rcu sync and percpu rwsem works, however the comment
> > > > > > below didn't make much sense to me. For one, there's no readers_fast anymore
> > > > > > so I did not follow what readers_fast means. Could the comment be updated to
> > > > > > reflect latest changes?
> > > > > > Also could you help understand how is a writer not able to change
> > > > > > sem->state and count the per-cpu read counters at the same time as the
> > > > > > comment tries to say?
> > > > > > 
> > > > > > 	/*
> > > > > > 	 * We are in an RCU-sched read-side critical section, so the writer
> > > > > > 	 * cannot both change sem->state from readers_fast and start checking
> > > > > > 	 * counters while we are here. So if we see !sem->state, we know that
> > > > > > 	 * the writer won't be checking until we're past the preempt_enable()
> > > > > > 	 * and that once the synchronize_rcu() is done, the writer will see
> > > > > > 	 * anything we did within this RCU-sched read-size critical section.
> > > > > > 	 */
> > > > > > 
> > > > > > Also,
> > > > > > I guess we could get rid of all of the gp_ops struct stuff now that since all
> > > > > > the callbacks are the same now. I will post that as a follow-up patch to this
> > > > > > series.
> > > > > 
> > > > > Hello, Joel,
> > > > > 
> > > > > Oleg has a set of patches updating this code that just hit mainline
> > > > > this week.  These patches get rid of the code that previously handled
> > > > > RCU's multiple flavors.  Or are you looking at current mainline and
> > > > > me just missing your point?
> > > > > 
> > > > 
> > > > Hi Paul,
> > > > You are right on point. I have a bad habit of not rebasing my trees. In this
> > > > case the feature branch of mine in concern was based on v5.1. Needless to
> > > > say, I need to rebase my tree.
> > > > 
> > > > Yes, this sync clean up patch does conflict when I rebase, but other patches
> > > > rebase just fine.
> > > > 
> > > > The 2 options I see are:
> > > > 1. Let us drop this patch for now and I resend it later.
> > > > 2. I resend all patches based on Linus's master branch.
> > > 
> > > Below is the updated patch based on Linus master branch:
> > > 
> > > ---8<-----------------------
> > > 
> > > >From 5f40c9a07fcf3d6dafc2189599d0ba9443097d0f Mon Sep 17 00:00:00 2001
> > > From: "Joel Fernandes (Google)" <joel@joelfernandes.org>
> > > Date: Fri, 12 Jul 2019 12:13:27 -0400
> > > Subject: [PATCH v2.1 3/9] rcu/sync: Remove custom check for reader-section
> > > 
> > > The rcu/sync code was doing its own check whether we are in a reader
> > > section. With RCU consolidating flavors and the generic helper added in
> > > this series, this is no longer need. We can just use the generic helper
> > > and it results in a nice cleanup.
> > > 
> > > Cc: Oleg Nesterov <oleg@redhat.com>
> > > Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
> > > ---
> > >  include/linux/rcu_sync.h | 4 +---
> > >  1 file changed, 1 insertion(+), 3 deletions(-)
> > > 
> > > diff --git a/include/linux/rcu_sync.h b/include/linux/rcu_sync.h
> > > index 9b83865d24f9..0027d4c8087c 100644
> > > --- a/include/linux/rcu_sync.h
> > > +++ b/include/linux/rcu_sync.h
> > > @@ -31,9 +31,7 @@ struct rcu_sync {
> > >   */
> > >  static inline bool rcu_sync_is_idle(struct rcu_sync *rsp)
> > >  {
> > > -	RCU_LOCKDEP_WARN(!rcu_read_lock_held() &&
> > > -			 !rcu_read_lock_bh_held() &&
> > > -			 !rcu_read_lock_sched_held(),
> > > +	RCU_LOCKDEP_WARN(!rcu_read_lock_any_held(),
> > 
> > I believe that replacing rcu_read_lock_sched_held() with preemptible()
> > in a CONFIG_PREEMPT=n kernel will give you false-positive splats here.
> > If you have not already done so, could you please give it a try?
> 
> Hi Paul,
> I don't think it will cause splats for !CONFIG_PREEMPT.
> 
> Currently, rcu_read_lock_any_held() introduced in this patch returns true if
> !preemptible(). This means that:
> 
> The following expression above:
> RCU_LOCKDEP_WARN(!rcu_read_lock_any_held(),...)
> 
> Becomes:
> RCU_LOCKDEP_WARN(preemptible(), ...)
> 
> For, CONFIG_PREEMPT=n kernels, this means:
> RCU_LOCKDEP_WARN(0, ...)
> 
> Which would mean no splats. Or, did I miss the point?

I suggest trying it out on a CONFIG_PREEMPT=n kernel.

							Thanx, Paul

^ permalink raw reply

* Re: [PATCH v2 3/9] rcu/sync: Remove custom check for reader-section
From: Joel Fernandes @ 2019-07-13 13:30 UTC (permalink / raw)
  To: Paul E. McKenney
  Cc: linux-kernel, Oleg Nesterov, Alexey Kuznetsov, Bjorn Helgaas,
	Borislav Petkov, c0d1n61at3, David S. Miller, edumazet,
	Greg Kroah-Hartman, Hideaki YOSHIFUJI, H. Peter Anvin,
	Ingo Molnar, Jonathan Corbet, Josh Triplett, keescook,
	kernel-hardening, kernel-team, Lai Jiangshan, Len Brown,
	linux-acpi, linux-doc, linux-pci, linux-pm, Mathieu Desnoyers,
	neilb, netdev, Pavel Machek, peterz, Rafael J. Wysocki,
	Rasmus Villemoes, rcu, Steven Rostedt, Tejun Heo, Thomas Gleixner,
	will, maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)
In-Reply-To: <20190713082114.GA26519@linux.ibm.com>

On Sat, Jul 13, 2019 at 01:21:14AM -0700, Paul E. McKenney wrote:
> On Fri, Jul 12, 2019 at 11:10:08PM -0400, Joel Fernandes wrote:
> > On Fri, Jul 12, 2019 at 11:01:50PM -0400, Joel Fernandes wrote:
> > > On Fri, Jul 12, 2019 at 04:32:06PM -0700, Paul E. McKenney wrote:
> > > > On Fri, Jul 12, 2019 at 05:35:59PM -0400, Joel Fernandes wrote:
> > > > > On Fri, Jul 12, 2019 at 01:00:18PM -0400, Joel Fernandes (Google) wrote:
> > > > > > The rcu/sync code was doing its own check whether we are in a reader
> > > > > > section. With RCU consolidating flavors and the generic helper added in
> > > > > > this series, this is no longer need. We can just use the generic helper
> > > > > > and it results in a nice cleanup.
> > > > > > 
> > > > > > Cc: Oleg Nesterov <oleg@redhat.com>
> > > > > > Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
> > > > > 
> > > > > Hi Oleg,
> > > > > Slightly unrelated to the patch,
> > > > > I tried hard to understand this comment below in percpu_down_read() but no dice.
> > > > > 
> > > > > I do understand how rcu sync and percpu rwsem works, however the comment
> > > > > below didn't make much sense to me. For one, there's no readers_fast anymore
> > > > > so I did not follow what readers_fast means. Could the comment be updated to
> > > > > reflect latest changes?
> > > > > Also could you help understand how is a writer not able to change
> > > > > sem->state and count the per-cpu read counters at the same time as the
> > > > > comment tries to say?
> > > > > 
> > > > > 	/*
> > > > > 	 * We are in an RCU-sched read-side critical section, so the writer
> > > > > 	 * cannot both change sem->state from readers_fast and start checking
> > > > > 	 * counters while we are here. So if we see !sem->state, we know that
> > > > > 	 * the writer won't be checking until we're past the preempt_enable()
> > > > > 	 * and that once the synchronize_rcu() is done, the writer will see
> > > > > 	 * anything we did within this RCU-sched read-size critical section.
> > > > > 	 */
> > > > > 
> > > > > Also,
> > > > > I guess we could get rid of all of the gp_ops struct stuff now that since all
> > > > > the callbacks are the same now. I will post that as a follow-up patch to this
> > > > > series.
> > > > 
> > > > Hello, Joel,
> > > > 
> > > > Oleg has a set of patches updating this code that just hit mainline
> > > > this week.  These patches get rid of the code that previously handled
> > > > RCU's multiple flavors.  Or are you looking at current mainline and
> > > > me just missing your point?
> > > > 
> > > 
> > > Hi Paul,
> > > You are right on point. I have a bad habit of not rebasing my trees. In this
> > > case the feature branch of mine in concern was based on v5.1. Needless to
> > > say, I need to rebase my tree.
> > > 
> > > Yes, this sync clean up patch does conflict when I rebase, but other patches
> > > rebase just fine.
> > > 
> > > The 2 options I see are:
> > > 1. Let us drop this patch for now and I resend it later.
> > > 2. I resend all patches based on Linus's master branch.
> > 
> > Below is the updated patch based on Linus master branch:
> > 
> > ---8<-----------------------
> > 
> > >From 5f40c9a07fcf3d6dafc2189599d0ba9443097d0f Mon Sep 17 00:00:00 2001
> > From: "Joel Fernandes (Google)" <joel@joelfernandes.org>
> > Date: Fri, 12 Jul 2019 12:13:27 -0400
> > Subject: [PATCH v2.1 3/9] rcu/sync: Remove custom check for reader-section
> > 
> > The rcu/sync code was doing its own check whether we are in a reader
> > section. With RCU consolidating flavors and the generic helper added in
> > this series, this is no longer need. We can just use the generic helper
> > and it results in a nice cleanup.
> > 
> > Cc: Oleg Nesterov <oleg@redhat.com>
> > Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
> > ---
> >  include/linux/rcu_sync.h | 4 +---
> >  1 file changed, 1 insertion(+), 3 deletions(-)
> > 
> > diff --git a/include/linux/rcu_sync.h b/include/linux/rcu_sync.h
> > index 9b83865d24f9..0027d4c8087c 100644
> > --- a/include/linux/rcu_sync.h
> > +++ b/include/linux/rcu_sync.h
> > @@ -31,9 +31,7 @@ struct rcu_sync {
> >   */
> >  static inline bool rcu_sync_is_idle(struct rcu_sync *rsp)
> >  {
> > -	RCU_LOCKDEP_WARN(!rcu_read_lock_held() &&
> > -			 !rcu_read_lock_bh_held() &&
> > -			 !rcu_read_lock_sched_held(),
> > +	RCU_LOCKDEP_WARN(!rcu_read_lock_any_held(),
> 
> I believe that replacing rcu_read_lock_sched_held() with preemptible()
> in a CONFIG_PREEMPT=n kernel will give you false-positive splats here.
> If you have not already done so, could you please give it a try?

Hi Paul,
I don't think it will cause splats for !CONFIG_PREEMPT.

Currently, rcu_read_lock_any_held() introduced in this patch returns true if
!preemptible(). This means that:

The following expression above:
RCU_LOCKDEP_WARN(!rcu_read_lock_any_held(),...)

Becomes:
RCU_LOCKDEP_WARN(preemptible(), ...)

For, CONFIG_PREEMPT=n kernels, this means:
RCU_LOCKDEP_WARN(0, ...)

Which would mean no splats. Or, did I miss the point?

thanks,

 - Joel


^ permalink raw reply

* [PATCH] tracing/fgraph: support recording function return values
From: Changbin Du @ 2019-07-13 12:10 UTC (permalink / raw)
  To: rostedt
  Cc: mingo, corbet, linux, catalin.marinas, will, tglx, bp, hpa, x86,
	linux-doc, linux-kernel, linux-arm-kernel, Changbin Du

This patch adds a new trace option 'funcgraph-retval' and is disabled by
default. When this option is enabled, fgraph tracer will show the return
value of each function. This is useful to find/analyze a original error
source in a call graph.

One limitation is that the kernel doesn't know the prototype of functions.
So fgraph assumes all functions have a retvalue of type int. You must ignore
the value of *void* function. And if the retvalue looks like an error code
then both hexadecimal and decimal number are displayed.

In this patch, only x86 and ARM platforms are supported.

Here is example showing the error is caused by vmx_create_vcpu() and the
error code is -5 (-EIO).

Here is an example:
with echo 1 > /sys/kernel/debug/tracing/options/funcgraph-retval

 3)               |  kvm_vm_ioctl() {
 3)               |    mutex_lock() {
 3)               |      _cond_resched() {
 3)   0.234 us    |        rcu_all_qs(); /* ret=0x80000000 */
 3)   0.704 us    |      } /* ret=0x0 */
 3)   1.226 us    |    } /* ret=0x0 */
 3)   0.247 us    |    mutex_unlock(); /* ret=0xffff8880738ed040 */
 3)               |    kvm_arch_vcpu_create() {
 3)               |      vmx_create_vcpu() {
 3) + 17.969 us   |        kmem_cache_alloc(); /* ret=0xffff88813a980040 */
 3) + 15.948 us   |        kmem_cache_alloc(); /* ret=0xffff88813aa99200 */
 3)   0.653 us    |        allocate_vpid.part.88(); /* ret=0x1 */
 3)   6.964 us    |        kvm_vcpu_init(); /* ret=0xfffffffb */
 3)   0.323 us    |        free_vpid.part.89(); /* ret=0x1 */
 3)   9.985 us    |        kmem_cache_free(); /* ret=0x80000000 */
 3)   9.491 us    |        kmem_cache_free(); /* ret=0x80000000 */
 3) + 69.858 us   |      } /* ret=0xfffffffffffffffb/-5 */
 3) + 70.631 us   |    } /* ret=0xfffffffffffffffb/-5 */
 3)               |    mutex_lock() {
 3)               |      _cond_resched() {
 3)   0.199 us    |        rcu_all_qs(); /* ret=0x80000000 */
 3)   0.594 us    |      } /* ret=0x0 */
 3)   1.067 us    |    } /* ret=0x0 */
 3)   0.337 us    |    mutex_unlock(); /* ret=0xffff8880738ed040 */
 3) + 92.730 us   |  } /* ret=0xfffffffffffffffb/-5 */

Checking above fgraph output, we can easily know the original error is
raised from function vmx_create_vcpu().

Signed-off-by: Changbin Du <changbin.du@gmail.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
---
 Documentation/trace/ftrace.rst       |  5 ++++
 arch/arm/kernel/entry-ftrace.S       |  1 +
 arch/arm64/kernel/entry-ftrace.S     |  1 +
 arch/x86/kernel/ftrace_32.S          |  1 +
 arch/x86/kernel/ftrace_64.S          |  1 +
 include/linux/ftrace.h               |  1 +
 kernel/trace/Kconfig                 |  4 +++
 kernel/trace/fgraph.c                | 17 +++++++++++-
 kernel/trace/trace.h                 |  1 +
 kernel/trace/trace_entries.h         |  1 +
 kernel/trace/trace_functions_graph.c | 39 ++++++++++++++++++++++++----
 11 files changed, 66 insertions(+), 6 deletions(-)

diff --git a/Documentation/trace/ftrace.rst b/Documentation/trace/ftrace.rst
index f60079259669..50dfb0378213 100644
--- a/Documentation/trace/ftrace.rst
+++ b/Documentation/trace/ftrace.rst
@@ -1264,6 +1264,11 @@ Options for function_graph tracer:
 	only a closing curly bracket "}" is displayed for
 	the return of a function.
 
+  funcgraph-retval - At the end of each function (the return) the
+    return value the function  (though the function may not
+    really have it) is displayed in hex and negative number
+    if it looks like a error code.
+
   sleep-time
 	When running function graph tracer, to include
 	the time a task schedules out in its function.
diff --git a/arch/arm/kernel/entry-ftrace.S b/arch/arm/kernel/entry-ftrace.S
index a74289ebc803..94e8209f1a6b 100644
--- a/arch/arm/kernel/entry-ftrace.S
+++ b/arch/arm/kernel/entry-ftrace.S
@@ -259,6 +259,7 @@ ENDPROC(ftrace_graph_regs_caller)
 	.globl return_to_handler
 return_to_handler:
 	stmdb	sp!, {r0-r3}
+	mov	r1, r0			@ return value
 	mov	r0, fp			@ frame pointer
 	bl	ftrace_return_to_handler
 	mov	lr, r0			@ r0 has real ret addr
diff --git a/arch/arm64/kernel/entry-ftrace.S b/arch/arm64/kernel/entry-ftrace.S
index 33d003d80121..b7ce416969d0 100644
--- a/arch/arm64/kernel/entry-ftrace.S
+++ b/arch/arm64/kernel/entry-ftrace.S
@@ -199,6 +199,7 @@ ENTRY(return_to_handler)
 	stp x4, x5, [sp, #32]
 	stp x6, x7, [sp, #48]
 
+	mov	x1, x0			// return value
 	mov	x0, x29			//     parent's fp
 	bl	ftrace_return_to_handler// addr = ftrace_return_to_hander(fp);
 	mov	x30, x0			// restore the original return address
diff --git a/arch/x86/kernel/ftrace_32.S b/arch/x86/kernel/ftrace_32.S
index 073aab525d80..539e80577a83 100644
--- a/arch/x86/kernel/ftrace_32.S
+++ b/arch/x86/kernel/ftrace_32.S
@@ -185,6 +185,7 @@ END(ftrace_graph_caller)
 return_to_handler:
 	pushl	%eax
 	pushl	%edx
+	movl	%eax, %edx
 	movl	$0, %eax
 	call	ftrace_return_to_handler
 	movl	%eax, %ecx
diff --git a/arch/x86/kernel/ftrace_64.S b/arch/x86/kernel/ftrace_64.S
index 809d54397dba..3b31a1959025 100644
--- a/arch/x86/kernel/ftrace_64.S
+++ b/arch/x86/kernel/ftrace_64.S
@@ -304,6 +304,7 @@ ENTRY(return_to_handler)
 	movq %rax, (%rsp)
 	movq %rdx, 8(%rsp)
 	movq %rbp, %rdi
+	movq %rax, %rsi
 
 	call ftrace_return_to_handler
 
diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index 25e2995d4a4c..83881bc47b50 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -734,6 +734,7 @@ struct ftrace_graph_ret {
 	unsigned long overrun;
 	unsigned long long calltime;
 	unsigned long long rettime;
+	unsigned long retval;
 	int depth;
 } __packed;
 
diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
index 564e5fdb025f..7d189ea461e8 100644
--- a/kernel/trace/Kconfig
+++ b/kernel/trace/Kconfig
@@ -25,6 +25,9 @@ config HAVE_FUNCTION_GRAPH_TRACER
 	help
 	  See Documentation/trace/ftrace-design.rst
 
+config HAVE_FTRACE_RETVAL
+	bool
+
 config HAVE_DYNAMIC_FTRACE
 	bool
 	help
@@ -161,6 +164,7 @@ config FUNCTION_GRAPH_TRACER
 	depends on HAVE_FUNCTION_GRAPH_TRACER
 	depends on FUNCTION_TRACER
 	depends on !X86_32 || !CC_OPTIMIZE_FOR_SIZE
+	select HAVE_FTRACE_RETVAL if (X86 || ARM)
 	default y
 	help
 	  Enable the kernel to trace a function at both its return
diff --git a/kernel/trace/fgraph.c b/kernel/trace/fgraph.c
index 8dfd5021b933..df27fe3a35f9 100644
--- a/kernel/trace/fgraph.c
+++ b/kernel/trace/fgraph.c
@@ -206,13 +206,15 @@ static struct notifier_block ftrace_suspend_notifier = {
  * Send the trace to the ring-buffer.
  * @return the original return address.
  */
-unsigned long ftrace_return_to_handler(unsigned long frame_pointer)
+static unsigned long _ftrace_return_to_handler(unsigned long frame_pointer,
+					       unsigned long retval)
 {
 	struct ftrace_graph_ret trace;
 	unsigned long ret;
 
 	ftrace_pop_return_trace(&trace, &ret, frame_pointer);
 	trace.rettime = trace_clock_local();
+	trace.retval = retval;
 	ftrace_graph_return(&trace);
 	/*
 	 * The ftrace_graph_return() may still access the current
@@ -232,6 +234,19 @@ unsigned long ftrace_return_to_handler(unsigned long frame_pointer)
 	return ret;
 }
 
+#if defined(CONFIG_HAVE_FTRACE_RETVAL)
+unsigned long ftrace_return_to_handler(unsigned long frame_pointer,
+				       unsigned long retval)
+{
+	return _ftrace_return_to_handler(frame_pointer, retval);
+}
+#else
+unsigned long ftrace_return_to_handler(unsigned long frame_pointer)
+{
+	return _ftrace_return_to_handler(frame_pointer, 0);
+}
+#endif
+
 /**
  * ftrace_graph_get_ret_stack - return the entry of the shadow stack
  * @task: The task to read the shadow stack from
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 005f08629b8b..483eecf0e9c3 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -905,6 +905,7 @@ static __always_inline bool ftrace_hash_empty(struct ftrace_hash *hash)
 #define TRACE_GRAPH_PRINT_TAIL          0x100
 #define TRACE_GRAPH_SLEEP_TIME          0x200
 #define TRACE_GRAPH_GRAPH_TIME          0x400
+#define TRACE_GRAPH_PRINT_RETVAL	0x800
 #define TRACE_GRAPH_PRINT_FILL_SHIFT	28
 #define TRACE_GRAPH_PRINT_FILL_MASK	(0x3 << TRACE_GRAPH_PRINT_FILL_SHIFT)
 
diff --git a/kernel/trace/trace_entries.h b/kernel/trace/trace_entries.h
index fc8e97328e54..4b5312cfed66 100644
--- a/kernel/trace/trace_entries.h
+++ b/kernel/trace/trace_entries.h
@@ -99,6 +99,7 @@ FTRACE_ENTRY_PACKED(funcgraph_exit, ftrace_graph_ret_entry,
 		__field_desc(	unsigned long,	ret,		func	)
 		__field_desc(	unsigned long long, ret,	calltime)
 		__field_desc(	unsigned long long, ret,	rettime	)
+		__field_desc(	unsigned long,	ret,		retval	)
 		__field_desc(	unsigned long,	ret,		overrun	)
 		__field_desc(	int,		ret,		depth	)
 	),
diff --git a/kernel/trace/trace_functions_graph.c b/kernel/trace/trace_functions_graph.c
index 69ebf3c2f1b5..399382cf175b 100644
--- a/kernel/trace/trace_functions_graph.c
+++ b/kernel/trace/trace_functions_graph.c
@@ -66,6 +66,11 @@ static struct tracer_opt trace_opts[] = {
 	{ TRACER_OPT(graph-time, TRACE_GRAPH_GRAPH_TIME) },
 #endif
 
+#ifdef CONFIG_HAVE_FTRACE_RETVAL
+	/* Display return value of function */
+	{ TRACER_OPT(funcgraph-retval, TRACE_GRAPH_PRINT_RETVAL) },
+#endif
+
 	{ } /* Empty entry */
 };
 
@@ -624,6 +629,18 @@ print_graph_duration(struct trace_array *tr, unsigned long long duration,
 	trace_seq_puts(s, "|  ");
 }
 
+static void print_graph_retval(struct trace_seq *s, unsigned long val, bool comment)
+{
+	if (comment)
+		trace_seq_printf(s, " /* ");
+	if (IS_ERR_VALUE(val))
+		trace_seq_printf(s, "ret=0x%lx/%ld", val, val);
+	else
+		trace_seq_printf(s, "ret=0x%lx", val);
+	if (comment)
+		trace_seq_printf(s, " */");
+}
+
 /* Case of a leaf function on its call entry */
 static enum print_line_t
 print_graph_entry_leaf(struct trace_iterator *iter,
@@ -668,7 +685,10 @@ print_graph_entry_leaf(struct trace_iterator *iter,
 	for (i = 0; i < call->depth * TRACE_GRAPH_INDENT; i++)
 		trace_seq_putc(s, ' ');
 
-	trace_seq_printf(s, "%ps();\n", (void *)call->func);
+	trace_seq_printf(s, "%ps();", (void *)call->func);
+	if (flags & TRACE_GRAPH_PRINT_RETVAL)
+		print_graph_retval(s, graph_ret->retval, true);
+	trace_seq_puts(s, "\n");
 
 	print_graph_irq(iter, graph_ret->func, TRACE_GRAPH_RET,
 			cpu, iter->ent->pid, flags);
@@ -953,10 +973,19 @@ print_graph_return(struct ftrace_graph_ret *trace, struct trace_seq *s,
 	 * belongs to, write out the function name. Always do
 	 * that if the funcgraph-tail option is enabled.
 	 */
-	if (func_match && !(flags & TRACE_GRAPH_PRINT_TAIL))
-		trace_seq_puts(s, "}\n");
-	else
-		trace_seq_printf(s, "} /* %ps */\n", (void *)trace->func);
+	if (func_match && !(flags & TRACE_GRAPH_PRINT_TAIL)) {
+		trace_seq_puts(s, "}");
+		if (flags & TRACE_GRAPH_PRINT_RETVAL)
+			print_graph_retval(s, trace->retval, true);
+		trace_seq_puts(s, "\n");
+	} else {
+		trace_seq_printf(s, "} /* %ps", (void *)trace->func);
+		if (flags & TRACE_GRAPH_PRINT_RETVAL) {
+			trace_seq_puts(s, ", ");
+			print_graph_retval(s, trace->retval, false);
+		}
+		trace_seq_puts(s, " */\n");
+	}
 
 	/* Overrun */
 	if (flags & TRACE_GRAPH_PRINT_OVERRUN)
-- 
2.20.1


^ permalink raw reply related

* [PATCH] scripts/sphinx-pre-install: fix script for RHEL/CentOS
From: Mauro Carvalho Chehab @ 2019-07-13 11:53 UTC (permalink / raw)
  To: Linux Doc Mailing List
  Cc: Mauro Carvalho Chehab, Mauro Carvalho Chehab, linux-kernel,
	Jonathan Corbet, Greg Kroah-Hartman, Kate Stewart, Mike Rapoport,
	Bjorn Helgaas, Thomas Gleixner

There's a missing parenthesis at the script, with causes it to
fail to detect non-Fedora releases (e. g. RHEL/CentOS).

Tested with Centos 7.6.1810.

Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
---
 scripts/sphinx-pre-install | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/sphinx-pre-install b/scripts/sphinx-pre-install
index cbe3d2bb659d..c3e36b867be7 100755
--- a/scripts/sphinx-pre-install
+++ b/scripts/sphinx-pre-install
@@ -376,7 +376,7 @@ sub give_redhat_hints()
 	#
 	# Checks valid for RHEL/CentOS version 7.x.
 	#
-	if (! $system_release =~ /Fedora/) {
+	if (!($system_release =~ /Fedora/)) {
 		$map{"virtualenv"} = "python-virtualenv";
 	}
 
-- 
2.21.0


^ permalink raw reply related

* Re: [PATCH v2 3/9] rcu/sync: Remove custom check for reader-section
From: Paul E. McKenney @ 2019-07-13  8:21 UTC (permalink / raw)
  To: Joel Fernandes
  Cc: linux-kernel, Oleg Nesterov, Alexey Kuznetsov, Bjorn Helgaas,
	Borislav Petkov, c0d1n61at3, David S. Miller, edumazet,
	Greg Kroah-Hartman, Hideaki YOSHIFUJI, H. Peter Anvin,
	Ingo Molnar, Jonathan Corbet, Josh Triplett, keescook,
	kernel-hardening, kernel-team, Lai Jiangshan, Len Brown,
	linux-acpi, linux-doc, linux-pci, linux-pm, Mathieu Desnoyers,
	neilb, netdev, Pavel Machek, peterz, Rafael J. Wysocki,
	Rasmus Villemoes, rcu, Steven Rostedt, Tejun Heo, Thomas Gleixner,
	will, maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)
In-Reply-To: <20190713031008.GA248225@google.com>

On Fri, Jul 12, 2019 at 11:10:08PM -0400, Joel Fernandes wrote:
> On Fri, Jul 12, 2019 at 11:01:50PM -0400, Joel Fernandes wrote:
> > On Fri, Jul 12, 2019 at 04:32:06PM -0700, Paul E. McKenney wrote:
> > > On Fri, Jul 12, 2019 at 05:35:59PM -0400, Joel Fernandes wrote:
> > > > On Fri, Jul 12, 2019 at 01:00:18PM -0400, Joel Fernandes (Google) wrote:
> > > > > The rcu/sync code was doing its own check whether we are in a reader
> > > > > section. With RCU consolidating flavors and the generic helper added in
> > > > > this series, this is no longer need. We can just use the generic helper
> > > > > and it results in a nice cleanup.
> > > > > 
> > > > > Cc: Oleg Nesterov <oleg@redhat.com>
> > > > > Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
> > > > 
> > > > Hi Oleg,
> > > > Slightly unrelated to the patch,
> > > > I tried hard to understand this comment below in percpu_down_read() but no dice.
> > > > 
> > > > I do understand how rcu sync and percpu rwsem works, however the comment
> > > > below didn't make much sense to me. For one, there's no readers_fast anymore
> > > > so I did not follow what readers_fast means. Could the comment be updated to
> > > > reflect latest changes?
> > > > Also could you help understand how is a writer not able to change
> > > > sem->state and count the per-cpu read counters at the same time as the
> > > > comment tries to say?
> > > > 
> > > > 	/*
> > > > 	 * We are in an RCU-sched read-side critical section, so the writer
> > > > 	 * cannot both change sem->state from readers_fast and start checking
> > > > 	 * counters while we are here. So if we see !sem->state, we know that
> > > > 	 * the writer won't be checking until we're past the preempt_enable()
> > > > 	 * and that once the synchronize_rcu() is done, the writer will see
> > > > 	 * anything we did within this RCU-sched read-size critical section.
> > > > 	 */
> > > > 
> > > > Also,
> > > > I guess we could get rid of all of the gp_ops struct stuff now that since all
> > > > the callbacks are the same now. I will post that as a follow-up patch to this
> > > > series.
> > > 
> > > Hello, Joel,
> > > 
> > > Oleg has a set of patches updating this code that just hit mainline
> > > this week.  These patches get rid of the code that previously handled
> > > RCU's multiple flavors.  Or are you looking at current mainline and
> > > me just missing your point?
> > > 
> > 
> > Hi Paul,
> > You are right on point. I have a bad habit of not rebasing my trees. In this
> > case the feature branch of mine in concern was based on v5.1. Needless to
> > say, I need to rebase my tree.
> > 
> > Yes, this sync clean up patch does conflict when I rebase, but other patches
> > rebase just fine.
> > 
> > The 2 options I see are:
> > 1. Let us drop this patch for now and I resend it later.
> > 2. I resend all patches based on Linus's master branch.
> 
> Below is the updated patch based on Linus master branch:
> 
> ---8<-----------------------
> 
> >From 5f40c9a07fcf3d6dafc2189599d0ba9443097d0f Mon Sep 17 00:00:00 2001
> From: "Joel Fernandes (Google)" <joel@joelfernandes.org>
> Date: Fri, 12 Jul 2019 12:13:27 -0400
> Subject: [PATCH v2.1 3/9] rcu/sync: Remove custom check for reader-section
> 
> The rcu/sync code was doing its own check whether we are in a reader
> section. With RCU consolidating flavors and the generic helper added in
> this series, this is no longer need. We can just use the generic helper
> and it results in a nice cleanup.
> 
> Cc: Oleg Nesterov <oleg@redhat.com>
> Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
> ---
>  include/linux/rcu_sync.h | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/include/linux/rcu_sync.h b/include/linux/rcu_sync.h
> index 9b83865d24f9..0027d4c8087c 100644
> --- a/include/linux/rcu_sync.h
> +++ b/include/linux/rcu_sync.h
> @@ -31,9 +31,7 @@ struct rcu_sync {
>   */
>  static inline bool rcu_sync_is_idle(struct rcu_sync *rsp)
>  {
> -	RCU_LOCKDEP_WARN(!rcu_read_lock_held() &&
> -			 !rcu_read_lock_bh_held() &&
> -			 !rcu_read_lock_sched_held(),
> +	RCU_LOCKDEP_WARN(!rcu_read_lock_any_held(),

I believe that replacing rcu_read_lock_sched_held() with preemptible()
in a CONFIG_PREEMPT=n kernel will give you false-positive splats here.
If you have not already done so, could you please give it a try?

							Thanx, Paul

>  			 "suspicious rcu_sync_is_idle() usage");
>  	return !READ_ONCE(rsp->gp_state); /* GP_IDLE */
>  }
> -- 
> 2.22.0.510.g264f2c817a-goog
> 

^ permalink raw reply

* Re: [PATCH v3] kbuild: get rid of misleading $(AS) from documents
From: Masahiro Yamada @ 2019-07-13  3:42 UTC (permalink / raw)
  To: Linux Kbuild mailing list
  Cc: Jonathan Corbet, Michal Marek, open list:DOCUMENTATION,
	Linux Kernel Mailing List
In-Reply-To: <20190713024558.27214-1-yamada.masahiro@socionext.com>

On Sat, Jul 13, 2019 at 11:46 AM Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
>
> The assembler files in the kernel are *.S instead of *.s, so they must
> be preprocessed. Since 'as' of GNU binutils is not able to preprocess,
> we always use $(CC) as an assembler driver.
>
> $(AS) is almost unused in Kbuild. As of v5.2, there is just one place
> that directly invokes $(AS).
>
>   $ git grep -e '$(AS)' -e '${AS}' -e '$AS' -e '$(AS:' -e '${AS:' -- :^Documentation
>   drivers/net/wan/Makefile:  AS68K = $(AS)
>
> The documentation about *_AFLAGS* sounds like the flags were passed
> to $(AS). This is somewhat misleading.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>
> ---

Applied to linux-kbuild.



> Changes in v3:
>   - Rebase
>
> Changes in v2:
>   - Rephrase without using "assembling"
>
>  Documentation/kbuild/kbuild.rst    |  5 ++---
>  Documentation/kbuild/makefiles.rst | 12 ++++++------
>  2 files changed, 8 insertions(+), 9 deletions(-)
>
> diff --git a/Documentation/kbuild/kbuild.rst b/Documentation/kbuild/kbuild.rst
> index b25548963d70..727520b3d7b1 100644
> --- a/Documentation/kbuild/kbuild.rst
> +++ b/Documentation/kbuild/kbuild.rst
> @@ -38,12 +38,11 @@ Additional options to the assembler (for built-in and modules).
>
>  AFLAGS_MODULE
>  -------------
> -Additional module specific options to use for $(AS).
> +Additional assembler options for modules.
>
>  AFLAGS_KERNEL
>  -------------
> -Additional options for $(AS) when used for assembler
> -code for code that is compiled as built-in.
> +Additional assembler options for built-in.
>
>  KCFLAGS
>  -------
> diff --git a/Documentation/kbuild/makefiles.rst b/Documentation/kbuild/makefiles.rst
> index 093f2d79ab95..67e47589d9d2 100644
> --- a/Documentation/kbuild/makefiles.rst
> +++ b/Documentation/kbuild/makefiles.rst
> @@ -328,7 +328,7 @@ more details, with real examples.
>         variable $(KBUILD_CFLAGS) and uses it for compilation flags for the
>         entire tree.
>
> -       asflags-y specifies options for assembling with $(AS).
> +       asflags-y specifies assembler options.
>
>         Example::
>
> @@ -489,7 +489,7 @@ more details, with real examples.
>         as-instr checks if the assembler reports a specific instruction
>         and then outputs either option1 or option2
>         C escapes are supported in the test instruction
> -       Note: as-instr-option uses KBUILD_AFLAGS for $(AS) options
> +       Note: as-instr-option uses KBUILD_AFLAGS for assembler options
>
>      cc-option
>         cc-option is used to check if $(CC) supports a given option, and if
> @@ -905,7 +905,7 @@ When kbuild executes, the following steps are followed (roughly):
>         vmlinux. The usage of $(call if_changed,xxx) will be described later.
>
>      KBUILD_AFLAGS
> -       $(AS) assembler flags
> +       Assembler flags
>
>         Default value - see top level Makefile
>         Append or modify as required per architecture.
> @@ -948,16 +948,16 @@ When kbuild executes, the following steps are followed (roughly):
>         to 'y' when selected.
>
>      KBUILD_AFLAGS_KERNEL
> -       $(AS) options specific for built-in
> +       Assembler options specific for built-in
>
>         $(KBUILD_AFLAGS_KERNEL) contains extra C compiler flags used to compile
>         resident kernel code.
>
>      KBUILD_AFLAGS_MODULE
> -       Options for $(AS) when building modules
> +       Assembler options specific for modules
>
>         $(KBUILD_AFLAGS_MODULE) is used to add arch-specific options that
> -       are used for $(AS).
> +       are used for assembler.
>
>         From commandline AFLAGS_MODULE shall be used (see kbuild.txt).
>
> --
> 2.17.1
>


-- 
Best Regards
Masahiro Yamada

^ permalink raw reply

* Re: [PATCH 0/5] PDF output fixes
From: Mauro Carvalho Chehab @ 2019-07-13  3:41 UTC (permalink / raw)
  To: Jonathan Corbet
  Cc: Heiko Carstens, Hannes Reinecke, linux-kbuild, Jens Axboe,
	Andy Shevchenko, Christian Borntraeger, Dan Williams,
	Michal Marek, Masahiro Yamada, Vasily Gorbik, linux-s390,
	linux-doc, Omar Sandoval
In-Reply-To: <20190712192705.71b97717@coco.lan>

Em Fri, 12 Jul 2019 19:27:05 -0300
Mauro Carvalho Chehab <mchehab+samsung@kernel.org> escreveu:

> Em Fri, 12 Jul 2019 14:19:21 -0600
> Jonathan Corbet <corbet@lwn.net> escreveu:
> 
> > On Tue,  9 Jul 2019 15:33:18 -0300
> > Mauro Carvalho Chehab <mchehab+samsung@kernel.org> wrote:
> >   
> > > In order to be able to build all PDF books, besides the two patches I
> > > already sent:
> > > 
> > >     docs: pdf: add all Documentation/*/index.rst to PDF output
> > >     docs: automarkup.py: ignore exceptions when seeking for xrefs
> > > 
> > > A few others are needed:
> > > 
> > > - patch 1 removes nested tables for a few files I converted, as 
> > >   Sphinx LaTeX builder doesn't support it;
> > > - Patches 2 to 4 addresses some minor issues on some books,
> > >   usually requiring some blank lines, extra whitespaces or some
> > >   tag replacement;
> > > - Patch 5 is required in order to be able to build the translations
> > >   PDF book, as it adds Asian fonts support to XeLaTeX.    
> > 
> > So, modulo my one comment on the last patch the series seems OK, though I
> > don't like having to work around limitations in PDF generation this way.  
> 
> Yeah, I feel the pain. I didn't like writing those patches either.
> 
> > Can't you just make rst2pdf work instead? :)  
> 
> Well, we can try. Last time we tried, there were lots of things missing
> for it to work. 
> 
> Based on:
> 
> 	https://pypi.org/project/rst2pdf/
> 
> The project got stuck back on Dec, 2012, up to the beginning of this
> year. Maybe the issues we had in the past got fixed.
> 
> Also, right now, rst2pdf only supports Python 2.7.
> 
> I would wait for it to support Python 3.x before doing the actual
> migration.
> 
> -
> 
> I don't remember the exact details, but when rst2pdf were
> used with some documents, it didn't produce a valid PDF file, but perhaps
> someone fixed it.
> 
> Assuming that rst2pdf works now, there are still several LaTeX specific
> hacks on some files:
> 
> 	$ git grep -l latex|grep rst|wc -l
> 	     28  
> 
> I guess almost all of them are related to the lack of proper support
> at ReST markup language that would allow adjusting the output to fit
> within the page limits. 
> 
> On media, we need two types of additional features:
> 
> 1) Sphinx pdf builder uses two different LaTeX ways of producing a table.
> (tabular x tabularx x longtable - don't remember anymore the exact
> two dialects it uses).
> 
> One of the dialect allows a table to be on multiple pages; the other
> one doesn't. The builder decides the dialect if a table has more than 30
> columns, but there's a way to force longtable (by changing the css style).
> 
> Not sure if rst2pdf honors the same css style.
> 
> 2) Table scaling / font resize. The Sphinx PDF builder doesn't scale
> the fonts inside a table, nor the ReST dialect allows changing the font
> size. So, we had to use raw LaTeX dialects for it to work.
> 
> Anyway, I'm enclosing an experimental patch that would enable rst2pdf.
> It is not at production level.
> 
> It also shows some crashes like this one:
> 
> [ERROR] pdfbuilder.py:133 format not resolved, probably missing URL scheme or undefined destination target for 'Callback%20Registry'
> Traceback (most recent call last):
>   File "/devel/v4l/docs/sphinx_1.7.9/lib/python2.7/site-packages/rst2pdf/pdfbuilder.py", line 130, in write
>     docwriter.write(doctree, destination)
>   File "/devel/v4l/docs/sphinx_1.7.9/lib/python2.7/site-packages/docutils/writers/__init__.py", line 80, in write
>     self.translate()
>   File "/devel/v4l/docs/sphinx_1.7.9/lib/python2.7/site-packages/rst2pdf/pdfbuilder.py", line 633, in translate
>     compressed=self.compressed)
>   File "/devel/v4l/docs/sphinx_1.7.9/lib/python2.7/site-packages/rst2pdf/createpdf.py", line 664, in createPdf
>     pdfdoc.multiBuild(elements)
>   File "/devel/v4l/docs/sphinx_1.7.9/lib/python2.7/site-packages/reportlab/platypus/doctemplate.py", line 1154, in multiBuild
>     self.canv.save()
>   File "/devel/v4l/docs/sphinx_1.7.9/lib/python2.7/site-packages/reportlab/pdfgen/canvas.py", line 1242, in save
>     self._doc.SaveToFile(self._filename, self)
>   File "/devel/v4l/docs/sphinx_1.7.9/lib/python2.7/site-packages/reportlab/pdfbase/pdfdoc.py", line 215, in SaveToFile
>     data = self.GetPDFData(canvas)
>   File "/devel/v4l/docs/sphinx_1.7.9/lib/python2.7/site-packages/reportlab/pdfbase/pdfdoc.py", line 241, in GetPDFData
>     return self.format()
>   File "/devel/v4l/docs/sphinx_1.7.9/lib/python2.7/site-packages/reportlab/pdfbase/pdfdoc.py", line 423, in format
>     IOf = IO.format(self)
>   File "/devel/v4l/docs/sphinx_1.7.9/lib/python2.7/site-packages/reportlab/pdfbase/pdfdoc.py", line 871, in format
>     fcontent = format(self.content, document, toplevel=1)   # yes this is at top level
>   File "/devel/v4l/docs/sphinx_1.7.9/lib/python2.7/site-packages/reportlab/pdfbase/pdfdoc.py", line 80, in format
>     f = element.format(document)
>   File "/devel/v4l/docs/sphinx_1.7.9/lib/python2.7/site-packages/reportlab/pdfbase/pdfdoc.py", line 1608, in format
>     return D.format(document)
>   File "/devel/v4l/docs/sphinx_1.7.9/lib/python2.7/site-packages/reportlab/pdfbase/pdfdoc.py", line 679, in format
>     L = [(format(PDFName(k),document)+b" "+format(dict[k],document)) for k in keys]
>   File "/devel/v4l/docs/sphinx_1.7.9/lib/python2.7/site-packages/reportlab/pdfbase/pdfdoc.py", line 80, in format
>     f = element.format(document)
>   File "/devel/v4l/docs/sphinx_1.7.9/lib/python2.7/site-packages/reportlab/pdfbase/pdfdoc.py", line 1773, in format
>     if f is None: raise ValueError("format not resolved, probably missing URL scheme or undefined destination target for '%s'" % self.name)
> ValueError: format not resolved, probably missing URL scheme or undefined destination target for 'Callback%20Registry'
> FAILED
> 
> 
> > 
> > I guess it makes sense for these to go with the big band-aid-removal patch
> > set.  
> 
> OK, I'll send it together with the big patchset.
> 
> > 
> > Thanks,
> > 
> > jon
> > 
> > P.S. it seems that rst2pdf is actually being developed again:
> > https://akrabat.com/rst2pdf-back-from-the-dead/ .  I wonder how far
> > they'll get with it.  
> 
> Thanks,
> Mauro
> 
> [PATCH RFC] docs: experimental: build PDF with rst2pdf

There are a few issues with the quick hack to use rst2pdf. The
enclosed one works better.

Yet, from the existing 70 books (on my experimental tree), rst2pdf breaks 
with 14 ones.

It seems that the problem is related to broken cross-references. The html
builder handles it nicely, but rst2pdf seems to crash when it can't solve
all references. That sounds somewhat similar to the bug we solved with the
automarkup extension.


Thanks,
Mauro



[PATCH] docs: experimental: build PDF with rst2pdf

Change the logic to use rst2pdf instead of LaTeX.

Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>

diff --git a/Documentation/Makefile b/Documentation/Makefile
index e145e4db508b..a298bbcf6fa7 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -89,18 +89,22 @@ latexdocs:
 
 ifeq ($(HAVE_PDFLATEX),0)
 
-pdfdocs:
+oldpdfdocs:
 	$(warning The '$(PDFLATEX)' command was not found. Make sure you have it installed and in PATH to produce PDF output.)
 	@echo "  SKIP    Sphinx $@ target."
 
 else # HAVE_PDFLATEX
 
-pdfdocs: latexdocs
+oldpdfdocs: latexdocs
 	@$(srctree)/scripts/sphinx-pre-install --version-check
 	$(foreach var,$(SPHINXDIRS), $(MAKE) PDFLATEX="$(PDFLATEX)" LATEXOPTS="$(LATEXOPTS)" -C $(BUILDDIR)/$(var)/latex || exit;)
 
 endif # HAVE_PDFLATEX
 
+pdfdocs:
+	@$(srctree)/scripts/sphinx-pre-install --version-check
+	@+$(foreach var,$(SPHINXDIRS),$(call loop_cmd,sphinx,pdf,$(var),pdf,$(var)))
+
 epubdocs:
 	@$(srctree)/scripts/sphinx-pre-install --version-check
 	@+$(foreach var,$(SPHINXDIRS),$(call loop_cmd,sphinx,epub,$(var),epub,$(var)))
@@ -126,6 +130,7 @@ dochelp:
 	@echo  '  htmldocs        - HTML'
 	@echo  '  latexdocs       - LaTeX'
 	@echo  '  oldpdfdocs      - PDF'
+	@echo  '  pdfdocs         - PDF, using experimental rst2pdf support'
 	@echo  '  epubdocs        - EPUB'
 	@echo  '  xmldocs         - XML'
 	@echo  '  linkcheckdocs   - check for broken external links (will connect to external hosts)'
diff --git a/Documentation/conf.py b/Documentation/conf.py
index fa0a42b47e62..f53ea57aebe3 100644
--- a/Documentation/conf.py
+++ b/Documentation/conf.py
@@ -43,6 +43,8 @@ if (major == 1 and minor > 3) or (major > 1):
 else:
     extensions.append("sphinx.ext.pngmath")
 
+extensions.append("rst2pdf.pdfbuilder")
+
 # Add any paths that contain templates here, relative to this directory.
 templates_path = ['_templates']
 
@@ -564,12 +566,15 @@ epub_exclude_files = ['search.html']
 #
 # See the Sphinx chapter of http://ralsina.me/static/manual.pdf
 #
-# FIXME: Do not add the index file here; the result will be too big. Adding
-# multiple PDF files here actually tries to get the cross-referencing right
-# *between* PDF files.
-pdf_documents = [
-    ('kernel-documentation', u'Kernel', u'Kernel', u'J. Random Bozo'),
-]
+
+# Add all LaTeX files to PDF documents as well
+pdf_documents = []
+for l in latex_documents:
+    doc = l[0]
+    fn = l[1].replace(".tex", "")
+    name = l[2]
+    authors = l[3]
+    pdf_documents.append((doc, fn, name, authors))
 
 # kernel-doc extension configuration for running Sphinx directly (e.g. by Read
 # the Docs). In a normal build, these are supplied from the Makefile via command
diff --git a/Documentation/media/Makefile b/Documentation/media/Makefile
index d75d70f191bc..3050136ed489 100644
--- a/Documentation/media/Makefile
+++ b/Documentation/media/Makefile
@@ -53,12 +53,13 @@ $(BUILDDIR)/lirc.h.rst: ${UAPI}/lirc.h ${PARSER} $(SRC_DIR)/lirc.h.rst.exception
 
 # Media build rules
 
-.PHONY: all html epub xml latex
+.PHONY: all html epub xml latex pdf
 
 all: $(IMGDOT) $(BUILDDIR) ${TARGETS}
 html: all
 epub: all
 xml: all
+pdf: all
 latex: $(IMGPDF) all
 linkcheck:
 
diff --git a/Makefile b/Makefile
index 5bbaf5ee6f5b..99ae519fccf9 100644
--- a/Makefile
+++ b/Makefile
@@ -1562,7 +1562,7 @@ $(help-board-dirs): help-%:
 # Documentation targets
 # ---------------------------------------------------------------------------
 DOC_TARGETS := xmldocs latexdocs pdfdocs htmldocs epubdocs cleandocs \
-	       linkcheckdocs dochelp refcheckdocs
+	       linkcheckdocs dochelp refcheckdocs oldpdfdocs
 PHONY += $(DOC_TARGETS)
 $(DOC_TARGETS): scripts_basic FORCE
 	$(Q)$(MAKE) $(build)=Documentation $@


^ permalink raw reply related

* Re: [PATCH v2 3/9] rcu/sync: Remove custom check for reader-section
From: Joel Fernandes @ 2019-07-13  3:10 UTC (permalink / raw)
  To: Paul E. McKenney
  Cc: linux-kernel, Oleg Nesterov, Alexey Kuznetsov, Bjorn Helgaas,
	Borislav Petkov, c0d1n61at3, David S. Miller, edumazet,
	Greg Kroah-Hartman, Hideaki YOSHIFUJI, H. Peter Anvin,
	Ingo Molnar, Jonathan Corbet, Josh Triplett, keescook,
	kernel-hardening, kernel-team, Lai Jiangshan, Len Brown,
	linux-acpi, linux-doc, linux-pci, linux-pm, Mathieu Desnoyers,
	neilb, netdev, Pavel Machek, peterz, Rafael J. Wysocki,
	Rasmus Villemoes, rcu, Steven Rostedt, Tejun Heo, Thomas Gleixner,
	will, maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)
In-Reply-To: <20190713030150.GA246587@google.com>

On Fri, Jul 12, 2019 at 11:01:50PM -0400, Joel Fernandes wrote:
> On Fri, Jul 12, 2019 at 04:32:06PM -0700, Paul E. McKenney wrote:
> > On Fri, Jul 12, 2019 at 05:35:59PM -0400, Joel Fernandes wrote:
> > > On Fri, Jul 12, 2019 at 01:00:18PM -0400, Joel Fernandes (Google) wrote:
> > > > The rcu/sync code was doing its own check whether we are in a reader
> > > > section. With RCU consolidating flavors and the generic helper added in
> > > > this series, this is no longer need. We can just use the generic helper
> > > > and it results in a nice cleanup.
> > > > 
> > > > Cc: Oleg Nesterov <oleg@redhat.com>
> > > > Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
> > > 
> > > Hi Oleg,
> > > Slightly unrelated to the patch,
> > > I tried hard to understand this comment below in percpu_down_read() but no dice.
> > > 
> > > I do understand how rcu sync and percpu rwsem works, however the comment
> > > below didn't make much sense to me. For one, there's no readers_fast anymore
> > > so I did not follow what readers_fast means. Could the comment be updated to
> > > reflect latest changes?
> > > Also could you help understand how is a writer not able to change
> > > sem->state and count the per-cpu read counters at the same time as the
> > > comment tries to say?
> > > 
> > > 	/*
> > > 	 * We are in an RCU-sched read-side critical section, so the writer
> > > 	 * cannot both change sem->state from readers_fast and start checking
> > > 	 * counters while we are here. So if we see !sem->state, we know that
> > > 	 * the writer won't be checking until we're past the preempt_enable()
> > > 	 * and that once the synchronize_rcu() is done, the writer will see
> > > 	 * anything we did within this RCU-sched read-size critical section.
> > > 	 */
> > > 
> > > Also,
> > > I guess we could get rid of all of the gp_ops struct stuff now that since all
> > > the callbacks are the same now. I will post that as a follow-up patch to this
> > > series.
> > 
> > Hello, Joel,
> > 
> > Oleg has a set of patches updating this code that just hit mainline
> > this week.  These patches get rid of the code that previously handled
> > RCU's multiple flavors.  Or are you looking at current mainline and
> > me just missing your point?
> > 
> 
> Hi Paul,
> You are right on point. I have a bad habit of not rebasing my trees. In this
> case the feature branch of mine in concern was based on v5.1. Needless to
> say, I need to rebase my tree.
> 
> Yes, this sync clean up patch does conflict when I rebase, but other patches
> rebase just fine.
> 
> The 2 options I see are:
> 1. Let us drop this patch for now and I resend it later.
> 2. I resend all patches based on Linus's master branch.

Below is the updated patch based on Linus master branch:

---8<-----------------------

From 5f40c9a07fcf3d6dafc2189599d0ba9443097d0f Mon Sep 17 00:00:00 2001
From: "Joel Fernandes (Google)" <joel@joelfernandes.org>
Date: Fri, 12 Jul 2019 12:13:27 -0400
Subject: [PATCH v2.1 3/9] rcu/sync: Remove custom check for reader-section

The rcu/sync code was doing its own check whether we are in a reader
section. With RCU consolidating flavors and the generic helper added in
this series, this is no longer need. We can just use the generic helper
and it results in a nice cleanup.

Cc: Oleg Nesterov <oleg@redhat.com>
Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
---
 include/linux/rcu_sync.h | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/include/linux/rcu_sync.h b/include/linux/rcu_sync.h
index 9b83865d24f9..0027d4c8087c 100644
--- a/include/linux/rcu_sync.h
+++ b/include/linux/rcu_sync.h
@@ -31,9 +31,7 @@ struct rcu_sync {
  */
 static inline bool rcu_sync_is_idle(struct rcu_sync *rsp)
 {
-	RCU_LOCKDEP_WARN(!rcu_read_lock_held() &&
-			 !rcu_read_lock_bh_held() &&
-			 !rcu_read_lock_sched_held(),
+	RCU_LOCKDEP_WARN(!rcu_read_lock_any_held(),
 			 "suspicious rcu_sync_is_idle() usage");
 	return !READ_ONCE(rsp->gp_state); /* GP_IDLE */
 }
-- 
2.22.0.510.g264f2c817a-goog


^ permalink raw reply related

* Re: [PATCH v2 3/9] rcu/sync: Remove custom check for reader-section
From: Joel Fernandes @ 2019-07-13  3:01 UTC (permalink / raw)
  To: Paul E. McKenney
  Cc: linux-kernel, Oleg Nesterov, Alexey Kuznetsov, Bjorn Helgaas,
	Borislav Petkov, c0d1n61at3, David S. Miller, edumazet,
	Greg Kroah-Hartman, Hideaki YOSHIFUJI, H. Peter Anvin,
	Ingo Molnar, Jonathan Corbet, Josh Triplett, keescook,
	kernel-hardening, kernel-team, Lai Jiangshan, Len Brown,
	linux-acpi, linux-doc, linux-pci, linux-pm, Mathieu Desnoyers,
	neilb, netdev, Pavel Machek, peterz, Rafael J. Wysocki,
	Rasmus Villemoes, rcu, Steven Rostedt, Tejun Heo, Thomas Gleixner,
	will, maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)
In-Reply-To: <20190712233206.GZ26519@linux.ibm.com>

On Fri, Jul 12, 2019 at 04:32:06PM -0700, Paul E. McKenney wrote:
> On Fri, Jul 12, 2019 at 05:35:59PM -0400, Joel Fernandes wrote:
> > On Fri, Jul 12, 2019 at 01:00:18PM -0400, Joel Fernandes (Google) wrote:
> > > The rcu/sync code was doing its own check whether we are in a reader
> > > section. With RCU consolidating flavors and the generic helper added in
> > > this series, this is no longer need. We can just use the generic helper
> > > and it results in a nice cleanup.
> > > 
> > > Cc: Oleg Nesterov <oleg@redhat.com>
> > > Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
> > 
> > Hi Oleg,
> > Slightly unrelated to the patch,
> > I tried hard to understand this comment below in percpu_down_read() but no dice.
> > 
> > I do understand how rcu sync and percpu rwsem works, however the comment
> > below didn't make much sense to me. For one, there's no readers_fast anymore
> > so I did not follow what readers_fast means. Could the comment be updated to
> > reflect latest changes?
> > Also could you help understand how is a writer not able to change
> > sem->state and count the per-cpu read counters at the same time as the
> > comment tries to say?
> > 
> > 	/*
> > 	 * We are in an RCU-sched read-side critical section, so the writer
> > 	 * cannot both change sem->state from readers_fast and start checking
> > 	 * counters while we are here. So if we see !sem->state, we know that
> > 	 * the writer won't be checking until we're past the preempt_enable()
> > 	 * and that once the synchronize_rcu() is done, the writer will see
> > 	 * anything we did within this RCU-sched read-size critical section.
> > 	 */
> > 
> > Also,
> > I guess we could get rid of all of the gp_ops struct stuff now that since all
> > the callbacks are the same now. I will post that as a follow-up patch to this
> > series.
> 
> Hello, Joel,
> 
> Oleg has a set of patches updating this code that just hit mainline
> this week.  These patches get rid of the code that previously handled
> RCU's multiple flavors.  Or are you looking at current mainline and
> me just missing your point?
> 

Hi Paul,
You are right on point. I have a bad habit of not rebasing my trees. In this
case the feature branch of mine in concern was based on v5.1. Needless to
say, I need to rebase my tree.

Yes, this sync clean up patch does conflict when I rebase, but other patches
rebase just fine.

The 2 options I see are:
1. Let us drop this patch for now and I resend it later.
2. I resend all patches based on Linus's master branch.

thanks,

- Joel


^ permalink raw reply

* [PATCH v3] kbuild: get rid of misleading $(AS) from documents
From: Masahiro Yamada @ 2019-07-13  2:45 UTC (permalink / raw)
  To: linux-kbuild
  Cc: Masahiro Yamada, Jonathan Corbet, Michal Marek, linux-doc,
	linux-kernel

The assembler files in the kernel are *.S instead of *.s, so they must
be preprocessed. Since 'as' of GNU binutils is not able to preprocess,
we always use $(CC) as an assembler driver.

$(AS) is almost unused in Kbuild. As of v5.2, there is just one place
that directly invokes $(AS).

  $ git grep -e '$(AS)' -e '${AS}' -e '$AS' -e '$(AS:' -e '${AS:' -- :^Documentation
  drivers/net/wan/Makefile:  AS68K = $(AS)

The documentation about *_AFLAGS* sounds like the flags were passed
to $(AS). This is somewhat misleading.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>
---

Changes in v3:
  - Rebase

Changes in v2:
  - Rephrase without using "assembling"

 Documentation/kbuild/kbuild.rst    |  5 ++---
 Documentation/kbuild/makefiles.rst | 12 ++++++------
 2 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/Documentation/kbuild/kbuild.rst b/Documentation/kbuild/kbuild.rst
index b25548963d70..727520b3d7b1 100644
--- a/Documentation/kbuild/kbuild.rst
+++ b/Documentation/kbuild/kbuild.rst
@@ -38,12 +38,11 @@ Additional options to the assembler (for built-in and modules).
 
 AFLAGS_MODULE
 -------------
-Additional module specific options to use for $(AS).
+Additional assembler options for modules.
 
 AFLAGS_KERNEL
 -------------
-Additional options for $(AS) when used for assembler
-code for code that is compiled as built-in.
+Additional assembler options for built-in.
 
 KCFLAGS
 -------
diff --git a/Documentation/kbuild/makefiles.rst b/Documentation/kbuild/makefiles.rst
index 093f2d79ab95..67e47589d9d2 100644
--- a/Documentation/kbuild/makefiles.rst
+++ b/Documentation/kbuild/makefiles.rst
@@ -328,7 +328,7 @@ more details, with real examples.
 	variable $(KBUILD_CFLAGS) and uses it for compilation flags for the
 	entire tree.
 
-	asflags-y specifies options for assembling with $(AS).
+	asflags-y specifies assembler options.
 
 	Example::
 
@@ -489,7 +489,7 @@ more details, with real examples.
 	as-instr checks if the assembler reports a specific instruction
 	and then outputs either option1 or option2
 	C escapes are supported in the test instruction
-	Note: as-instr-option uses KBUILD_AFLAGS for $(AS) options
+	Note: as-instr-option uses KBUILD_AFLAGS for assembler options
 
     cc-option
 	cc-option is used to check if $(CC) supports a given option, and if
@@ -905,7 +905,7 @@ When kbuild executes, the following steps are followed (roughly):
 	vmlinux. The usage of $(call if_changed,xxx) will be described later.
 
     KBUILD_AFLAGS
-	$(AS) assembler flags
+	Assembler flags
 
 	Default value - see top level Makefile
 	Append or modify as required per architecture.
@@ -948,16 +948,16 @@ When kbuild executes, the following steps are followed (roughly):
 	to 'y' when selected.
 
     KBUILD_AFLAGS_KERNEL
-	$(AS) options specific for built-in
+	Assembler options specific for built-in
 
 	$(KBUILD_AFLAGS_KERNEL) contains extra C compiler flags used to compile
 	resident kernel code.
 
     KBUILD_AFLAGS_MODULE
-	Options for $(AS) when building modules
+	Assembler options specific for modules
 
 	$(KBUILD_AFLAGS_MODULE) is used to add arch-specific options that
-	are used for $(AS).
+	are used for assembler.
 
 	From commandline AFLAGS_MODULE shall be used (see kbuild.txt).
 
-- 
2.17.1


^ permalink raw reply related

* [PATCH v5 02/11] of/platform: Add functional dependency link from DT bindings
From: Saravana Kannan @ 2019-07-12 23:52 UTC (permalink / raw)
  To: Rob Herring, Mark Rutland, Greg Kroah-Hartman, Rafael J. Wysocki,
	Frank Rowand, Jonathan Corbet
  Cc: Saravana Kannan, devicetree, linux-kernel, David Collins,
	kernel-team, linux-doc
In-Reply-To: <20190712235245.202558-1-saravanak@google.com>

Add device-links after the devices are created (but before they are
probed) by looking at common DT bindings like clocks and
interconnects.

Automatically adding device-links for functional dependencies at the
framework level provides the following benefits:

- Optimizes device probe order and avoids the useless work of
  attempting probes of devices that will not probe successfully
  (because their suppliers aren't present or haven't probed yet).

  For example, in a commonly available mobile SoC, registering just
  one consumer device's driver at an initcall level earlier than the
  supplier device's driver causes 11 failed probe attempts before the
  consumer device probes successfully. This was with a kernel with all
  the drivers statically compiled in. This problem gets a lot worse if
  all the drivers are loaded as modules without direct symbol
  dependencies.

- Supplier devices like clock providers, interconnect providers, etc
  need to keep the resources they provide active and at a particular
  state(s) during boot up even if their current set of consumers don't
  request the resource to be active. This is because the rest of the
  consumers might not have probed yet and turning off the resource
  before all the consumers have probed could lead to a hang or
  undesired user experience.

  Some frameworks (Eg: regulator) handle this today by turning off
  "unused" resources at late_initcall_sync and hoping all the devices
  have probed by then. This is not a valid assumption for systems with
  loadable modules. Other frameworks (Eg: clock) just don't handle
  this due to the lack of a clear signal for when they can turn off
  resources. This leads to downstream hacks to handle cases like this
  that can easily be solved in the upstream kernel.

  By linking devices before they are probed, we give suppliers a clear
  count of the number of dependent consumers. Once all of the
  consumers are active, the suppliers can turn off the unused
  resources without making assumptions about the number of consumers.

By default we just add device-links to track "driver presence" (probe
succeeded) of the supplier device. If any other functionality provided
by device-links are needed, it is left to the consumer/supplier
devices to change the link when they probe.

Signed-off-by: Saravana Kannan <saravanak@google.com>
---
 .../admin-guide/kernel-parameters.txt         |  5 ++
 drivers/of/platform.c                         | 57 +++++++++++++++++++
 2 files changed, 62 insertions(+)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 138f6664b2e2..109b4310844f 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -3141,6 +3141,11 @@
 			This can be set from sysctl after boot.
 			See Documentation/sysctl/vm.txt for details.
 
+	of_devlink	[KNL] Make device links from common DT bindings. Useful
+			for optimizing probe order and making sure resources
+			aren't turned off before the consumer devices have
+			probed.
+
 	ohci1394_dma=early	[HW] enable debugging via the ohci1394 driver.
 			See Documentation/debugging-via-ohci1394.txt for more
 			info.
diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index 04ad312fd85b..0930f9f89571 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -509,6 +509,62 @@ int of_platform_default_populate(struct device_node *root,
 }
 EXPORT_SYMBOL_GPL(of_platform_default_populate);
 
+static int of_link_binding(struct device *dev,
+			   const char *binding, const char *cell)
+{
+	struct of_phandle_args sup_args;
+	struct platform_device *sup_dev;
+	unsigned int i = 0, links = 0;
+	u32 dl_flags = DL_FLAG_AUTOPROBE_CONSUMER;
+
+	while (!of_parse_phandle_with_args(dev->of_node, binding, cell, i,
+					   &sup_args)) {
+		i++;
+		sup_dev = of_find_device_by_node(sup_args.np);
+		of_node_put(sup_args.np);
+		if (!sup_dev)
+			continue;
+		if (device_link_add(dev, &sup_dev->dev, dl_flags))
+			links++;
+		put_device(&sup_dev->dev);
+	}
+	if (links < i)
+		return -ENODEV;
+	return 0;
+}
+
+static bool of_devlink;
+core_param(of_devlink, of_devlink, bool, 0);
+
+/*
+ * List of bindings and their cell names (use NULL if no cell names) from which
+ * device links need to be created.
+ */
+static const char * const link_bindings[] = {
+	"clocks", "#clock-cells",
+	"interconnects", "#interconnect-cells",
+};
+
+static int of_link_to_suppliers(struct device *dev)
+{
+	unsigned int i = 0;
+	bool done = true;
+
+	if (!of_devlink)
+		return 0;
+	if (unlikely(!dev->of_node))
+		return 0;
+
+	for (i = 0; i < ARRAY_SIZE(link_bindings) / 2; i++)
+		if (of_link_binding(dev, link_bindings[i * 2],
+					link_bindings[i * 2 + 1]))
+			done = false;
+
+	if (!done)
+		return -ENODEV;
+	return 0;
+}
+
 #ifndef CONFIG_PPC
 static const struct of_device_id reserved_mem_matches[] = {
 	{ .compatible = "qcom,rmtfs-mem" },
@@ -524,6 +580,7 @@ static int __init of_platform_default_populate_init(void)
 	if (!of_have_populated_dt())
 		return -ENODEV;
 
+	platform_bus_type.add_links = of_link_to_suppliers;
 	/*
 	 * Handle certain compatibles explicitly, since we don't want to create
 	 * platform_devices for every node in /reserved-memory with a
-- 
2.22.0.510.g264f2c817a-goog


^ permalink raw reply related

* Re: [PATCH v2 3/9] rcu/sync: Remove custom check for reader-section
From: Paul E. McKenney @ 2019-07-12 23:32 UTC (permalink / raw)
  To: Joel Fernandes
  Cc: linux-kernel, Oleg Nesterov, Alexey Kuznetsov, Bjorn Helgaas,
	Borislav Petkov, c0d1n61at3, David S. Miller, edumazet,
	Greg Kroah-Hartman, Hideaki YOSHIFUJI, H. Peter Anvin,
	Ingo Molnar, Jonathan Corbet, Josh Triplett, keescook,
	kernel-hardening, kernel-team, Lai Jiangshan, Len Brown,
	linux-acpi, linux-doc, linux-pci, linux-pm, Mathieu Desnoyers,
	neilb, netdev, Pavel Machek, peterz, Rafael J. Wysocki,
	Rasmus Villemoes, rcu, Steven Rostedt, Tejun Heo, Thomas Gleixner,
	will, maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)
In-Reply-To: <20190712213559.GA175138@google.com>

On Fri, Jul 12, 2019 at 05:35:59PM -0400, Joel Fernandes wrote:
> On Fri, Jul 12, 2019 at 01:00:18PM -0400, Joel Fernandes (Google) wrote:
> > The rcu/sync code was doing its own check whether we are in a reader
> > section. With RCU consolidating flavors and the generic helper added in
> > this series, this is no longer need. We can just use the generic helper
> > and it results in a nice cleanup.
> > 
> > Cc: Oleg Nesterov <oleg@redhat.com>
> > Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
> 
> Hi Oleg,
> Slightly unrelated to the patch,
> I tried hard to understand this comment below in percpu_down_read() but no dice.
> 
> I do understand how rcu sync and percpu rwsem works, however the comment
> below didn't make much sense to me. For one, there's no readers_fast anymore
> so I did not follow what readers_fast means. Could the comment be updated to
> reflect latest changes?
> Also could you help understand how is a writer not able to change
> sem->state and count the per-cpu read counters at the same time as the
> comment tries to say?
> 
> 	/*
> 	 * We are in an RCU-sched read-side critical section, so the writer
> 	 * cannot both change sem->state from readers_fast and start checking
> 	 * counters while we are here. So if we see !sem->state, we know that
> 	 * the writer won't be checking until we're past the preempt_enable()
> 	 * and that once the synchronize_rcu() is done, the writer will see
> 	 * anything we did within this RCU-sched read-size critical section.
> 	 */
> 
> Also,
> I guess we could get rid of all of the gp_ops struct stuff now that since all
> the callbacks are the same now. I will post that as a follow-up patch to this
> series.

Hello, Joel,

Oleg has a set of patches updating this code that just hit mainline
this week.  These patches get rid of the code that previously handled
RCU's multiple flavors.  Or are you looking at current mainline and
me just missing your point?

							Thanx, Paul

> thanks!
> 
>  - Joel
> 
> 
> > ---
> > Please note: Only build and boot tested this particular patch so far.
> > 
> >  include/linux/rcu_sync.h |  5 ++---
> >  kernel/rcu/sync.c        | 22 ----------------------
> >  2 files changed, 2 insertions(+), 25 deletions(-)
> > 
> > diff --git a/include/linux/rcu_sync.h b/include/linux/rcu_sync.h
> > index 6fc53a1345b3..c954f1efc919 100644
> > --- a/include/linux/rcu_sync.h
> > +++ b/include/linux/rcu_sync.h
> > @@ -39,9 +39,8 @@ extern void rcu_sync_lockdep_assert(struct rcu_sync *);
> >   */
> >  static inline bool rcu_sync_is_idle(struct rcu_sync *rsp)
> >  {
> > -#ifdef CONFIG_PROVE_RCU
> > -	rcu_sync_lockdep_assert(rsp);
> > -#endif
> > +	RCU_LOCKDEP_WARN(!rcu_read_lock_any_held(),
> > +			 "suspicious rcu_sync_is_idle() usage");
> >  	return !rsp->gp_state; /* GP_IDLE */
> >  }
> >  
> > diff --git a/kernel/rcu/sync.c b/kernel/rcu/sync.c
> > index a8304d90573f..535e02601f56 100644
> > --- a/kernel/rcu/sync.c
> > +++ b/kernel/rcu/sync.c
> > @@ -10,37 +10,25 @@
> >  #include <linux/rcu_sync.h>
> >  #include <linux/sched.h>
> >  
> > -#ifdef CONFIG_PROVE_RCU
> > -#define __INIT_HELD(func)	.held = func,
> > -#else
> > -#define __INIT_HELD(func)
> > -#endif
> > -
> >  static const struct {
> >  	void (*sync)(void);
> >  	void (*call)(struct rcu_head *, void (*)(struct rcu_head *));
> >  	void (*wait)(void);
> > -#ifdef CONFIG_PROVE_RCU
> > -	int  (*held)(void);
> > -#endif
> >  } gp_ops[] = {
> >  	[RCU_SYNC] = {
> >  		.sync = synchronize_rcu,
> >  		.call = call_rcu,
> >  		.wait = rcu_barrier,
> > -		__INIT_HELD(rcu_read_lock_held)
> >  	},
> >  	[RCU_SCHED_SYNC] = {
> >  		.sync = synchronize_rcu,
> >  		.call = call_rcu,
> >  		.wait = rcu_barrier,
> > -		__INIT_HELD(rcu_read_lock_sched_held)
> >  	},
> >  	[RCU_BH_SYNC] = {
> >  		.sync = synchronize_rcu,
> >  		.call = call_rcu,
> >  		.wait = rcu_barrier,
> > -		__INIT_HELD(rcu_read_lock_bh_held)
> >  	},
> >  };
> >  
> > @@ -49,16 +37,6 @@ enum { CB_IDLE = 0, CB_PENDING, CB_REPLAY };
> >  
> >  #define	rss_lock	gp_wait.lock
> >  
> > -#ifdef CONFIG_PROVE_RCU
> > -void rcu_sync_lockdep_assert(struct rcu_sync *rsp)
> > -{
> > -	RCU_LOCKDEP_WARN(!gp_ops[rsp->gp_type].held(),
> > -			 "suspicious rcu_sync_is_idle() usage");
> > -}
> > -
> > -EXPORT_SYMBOL_GPL(rcu_sync_lockdep_assert);
> > -#endif
> > -
> >  /**
> >   * rcu_sync_init() - Initialize an rcu_sync structure
> >   * @rsp: Pointer to rcu_sync structure to be initialized
> > -- 
> > 2.22.0.510.g264f2c817a-goog
> > 
> 

^ permalink raw reply

* Re: [PATCH 0/5] PDF output fixes
From: Mauro Carvalho Chehab @ 2019-07-12 22:27 UTC (permalink / raw)
  To: Jonathan Corbet
  Cc: Heiko Carstens, Hannes Reinecke, linux-kbuild, Jens Axboe,
	Andy Shevchenko, Christian Borntraeger, Dan Williams,
	Michal Marek, Masahiro Yamada, Vasily Gorbik, linux-s390,
	linux-doc, Omar Sandoval
In-Reply-To: <20190712141921.7f8a1d02@lwn.net>

Em Fri, 12 Jul 2019 14:19:21 -0600
Jonathan Corbet <corbet@lwn.net> escreveu:

> On Tue,  9 Jul 2019 15:33:18 -0300
> Mauro Carvalho Chehab <mchehab+samsung@kernel.org> wrote:
> 
> > In order to be able to build all PDF books, besides the two patches I
> > already sent:
> > 
> >     docs: pdf: add all Documentation/*/index.rst to PDF output
> >     docs: automarkup.py: ignore exceptions when seeking for xrefs
> > 
> > A few others are needed:
> > 
> > - patch 1 removes nested tables for a few files I converted, as 
> >   Sphinx LaTeX builder doesn't support it;
> > - Patches 2 to 4 addresses some minor issues on some books,
> >   usually requiring some blank lines, extra whitespaces or some
> >   tag replacement;
> > - Patch 5 is required in order to be able to build the translations
> >   PDF book, as it adds Asian fonts support to XeLaTeX.  
> 
> So, modulo my one comment on the last patch the series seems OK, though I
> don't like having to work around limitations in PDF generation this way.

Yeah, I feel the pain. I didn't like writing those patches either.

> Can't you just make rst2pdf work instead? :)

Well, we can try. Last time we tried, there were lots of things missing
for it to work. 

Based on:

	https://pypi.org/project/rst2pdf/

The project got stuck back on Dec, 2012, up to the beginning of this
year. Maybe the issues we had in the past got fixed.

Also, right now, rst2pdf only supports Python 2.7.

I would wait for it to support Python 3.x before doing the actual
migration.

-

I don't remember the exact details, but when rst2pdf were
used with some documents, it didn't produce a valid PDF file, but perhaps
someone fixed it.

Assuming that rst2pdf works now, there are still several LaTeX specific
hacks on some files:

	$ git grep -l latex|grep rst|wc -l
	     28  

I guess almost all of them are related to the lack of proper support
at ReST markup language that would allow adjusting the output to fit
within the page limits. 

On media, we need two types of additional features:

1) Sphinx pdf builder uses two different LaTeX ways of producing a table.
(tabular x tabularx x longtable - don't remember anymore the exact
two dialects it uses).

One of the dialect allows a table to be on multiple pages; the other
one doesn't. The builder decides the dialect if a table has more than 30
columns, but there's a way to force longtable (by changing the css style).

Not sure if rst2pdf honors the same css style.

2) Table scaling / font resize. The Sphinx PDF builder doesn't scale
the fonts inside a table, nor the ReST dialect allows changing the font
size. So, we had to use raw LaTeX dialects for it to work.

Anyway, I'm enclosing an experimental patch that would enable rst2pdf.
It is not at production level.

It also shows some crashes like this one:

[ERROR] pdfbuilder.py:133 format not resolved, probably missing URL scheme or undefined destination target for 'Callback%20Registry'
Traceback (most recent call last):
  File "/devel/v4l/docs/sphinx_1.7.9/lib/python2.7/site-packages/rst2pdf/pdfbuilder.py", line 130, in write
    docwriter.write(doctree, destination)
  File "/devel/v4l/docs/sphinx_1.7.9/lib/python2.7/site-packages/docutils/writers/__init__.py", line 80, in write
    self.translate()
  File "/devel/v4l/docs/sphinx_1.7.9/lib/python2.7/site-packages/rst2pdf/pdfbuilder.py", line 633, in translate
    compressed=self.compressed)
  File "/devel/v4l/docs/sphinx_1.7.9/lib/python2.7/site-packages/rst2pdf/createpdf.py", line 664, in createPdf
    pdfdoc.multiBuild(elements)
  File "/devel/v4l/docs/sphinx_1.7.9/lib/python2.7/site-packages/reportlab/platypus/doctemplate.py", line 1154, in multiBuild
    self.canv.save()
  File "/devel/v4l/docs/sphinx_1.7.9/lib/python2.7/site-packages/reportlab/pdfgen/canvas.py", line 1242, in save
    self._doc.SaveToFile(self._filename, self)
  File "/devel/v4l/docs/sphinx_1.7.9/lib/python2.7/site-packages/reportlab/pdfbase/pdfdoc.py", line 215, in SaveToFile
    data = self.GetPDFData(canvas)
  File "/devel/v4l/docs/sphinx_1.7.9/lib/python2.7/site-packages/reportlab/pdfbase/pdfdoc.py", line 241, in GetPDFData
    return self.format()
  File "/devel/v4l/docs/sphinx_1.7.9/lib/python2.7/site-packages/reportlab/pdfbase/pdfdoc.py", line 423, in format
    IOf = IO.format(self)
  File "/devel/v4l/docs/sphinx_1.7.9/lib/python2.7/site-packages/reportlab/pdfbase/pdfdoc.py", line 871, in format
    fcontent = format(self.content, document, toplevel=1)   # yes this is at top level
  File "/devel/v4l/docs/sphinx_1.7.9/lib/python2.7/site-packages/reportlab/pdfbase/pdfdoc.py", line 80, in format
    f = element.format(document)
  File "/devel/v4l/docs/sphinx_1.7.9/lib/python2.7/site-packages/reportlab/pdfbase/pdfdoc.py", line 1608, in format
    return D.format(document)
  File "/devel/v4l/docs/sphinx_1.7.9/lib/python2.7/site-packages/reportlab/pdfbase/pdfdoc.py", line 679, in format
    L = [(format(PDFName(k),document)+b" "+format(dict[k],document)) for k in keys]
  File "/devel/v4l/docs/sphinx_1.7.9/lib/python2.7/site-packages/reportlab/pdfbase/pdfdoc.py", line 80, in format
    f = element.format(document)
  File "/devel/v4l/docs/sphinx_1.7.9/lib/python2.7/site-packages/reportlab/pdfbase/pdfdoc.py", line 1773, in format
    if f is None: raise ValueError("format not resolved, probably missing URL scheme or undefined destination target for '%s'" % self.name)
ValueError: format not resolved, probably missing URL scheme or undefined destination target for 'Callback%20Registry'
FAILED


> 
> I guess it makes sense for these to go with the big band-aid-removal patch
> set.

OK, I'll send it together with the big patchset.

> 
> Thanks,
> 
> jon
> 
> P.S. it seems that rst2pdf is actually being developed again:
> https://akrabat.com/rst2pdf-back-from-the-dead/ .  I wonder how far
> they'll get with it.

Thanks,
Mauro

[PATCH RFC] docs: experimental: build PDF with rst2pdf

Change the logic to use rst2pdf instead of LaTeX.

Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>

diff --git a/Documentation/Makefile b/Documentation/Makefile
index e145e4db508b..cc913f96a66d 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -95,9 +95,10 @@ pdfdocs:
 
 else # HAVE_PDFLATEX
 
-pdfdocs: latexdocs
+pdfdocs:
 	@$(srctree)/scripts/sphinx-pre-install --version-check
-	$(foreach var,$(SPHINXDIRS), $(MAKE) PDFLATEX="$(PDFLATEX)" LATEXOPTS="$(LATEXOPTS)" -C $(BUILDDIR)/$(var)/latex || exit;)
+	mkdir -p $(BUILDDIR)/pdf
+	@+$(foreach var,$(SPHINXDIRS),$(call loop_cmd,sphinx,pdf,$(var),pdf,$(var)))
 
 endif # HAVE_PDFLATEX
 
diff --git a/Documentation/conf.py b/Documentation/conf.py
index fa0a42b47e62..4db8a62d7c6a 100644
--- a/Documentation/conf.py
+++ b/Documentation/conf.py
@@ -35,13 +35,14 @@ needs_sphinx = '1.3'
 # extensions coming with Sphinx (named 'sphinx.ext.*') or your custom
 # ones.
 extensions = ['kerneldoc', 'rstFlatTable', 'kernel_include', 'cdomain',
-              'kfigure', 'sphinx.ext.ifconfig', 'automarkup']
+              'kfigure', 'sphinx.ext.ifconfig', 'automarkup', 'rst2pdf.pdfbuilder']
 
-# The name of the math extension changed on Sphinx 1.4
-if (major == 1 and minor > 3) or (major > 1):
-    extensions.append("sphinx.ext.imgmath")
-else:
-    extensions.append("sphinx.ext.pngmath")
+# FIXME: when rst2pdf.pdfbuilder is added, the code below causes an error
+## The name of the math extension changed on Sphinx 1.4
+#if (major == 1 and minor > 3) or (major > 1):
+#    extensions.append("sphinx.ext.imgmath")
+#else:
+#    extensions.append("sphinx.ext.pngmath")
 
 # Add any paths that contain templates here, relative to this directory.
 templates_path = ['_templates']
@@ -564,12 +565,15 @@ epub_exclude_files = ['search.html']
 #
 # See the Sphinx chapter of http://ralsina.me/static/manual.pdf
 #
-# FIXME: Do not add the index file here; the result will be too big. Adding
-# multiple PDF files here actually tries to get the cross-referencing right
-# *between* PDF files.
-pdf_documents = [
-    ('kernel-documentation', u'Kernel', u'Kernel', u'J. Random Bozo'),
-]
+
+# Add all LaTeX files to PDF documents as well
+pdf_documents = []
+for l in latex_documents:
+    doc = l[0]
+    fn = l[1].replace("tex", "pdf")
+    name = l[2]
+    authors = l[3]
+    pdf_documents.append((doc, fn, name, authors))
 
 # kernel-doc extension configuration for running Sphinx directly (e.g. by Read
 # the Docs). In a normal build, these are supplied from the Makefile via command
diff --git a/Documentation/media/Makefile b/Documentation/media/Makefile
index d75d70f191bc..3050136ed489 100644
--- a/Documentation/media/Makefile
+++ b/Documentation/media/Makefile
@@ -53,12 +53,13 @@ $(BUILDDIR)/lirc.h.rst: ${UAPI}/lirc.h ${PARSER} $(SRC_DIR)/lirc.h.rst.exception
 
 # Media build rules
 
-.PHONY: all html epub xml latex
+.PHONY: all html epub xml latex pdf
 
 all: $(IMGDOT) $(BUILDDIR) ${TARGETS}
 html: all
 epub: all
 xml: all
+pdf: all
 latex: $(IMGPDF) all
 linkcheck:
 


^ permalink raw reply related

* Re: [PATCH v5 1/1] sched/fair: Fix low cpu usage with high throttling by removing expiration of cpu-local slices
From: bsegall @ 2019-07-12 22:09 UTC (permalink / raw)
  To: Dave Chiluk
  Cc: Peter Zijlstra, Pqhil Auld, Peter Oskolkov, Ingo Molnar, cgroups,
	Linux Kernel Mailing List, Brendan Gregg, Kyle Anderson,
	Gabriel Munos, John Hammond, Cong Wang, Jonathan Corbet,
	linux-doc, Paul Turner
In-Reply-To: <CAC=E7cV4sO50NpYOZ06n_BkZTcBqf1KQp83prc+oave3ircBrw@mail.gmail.com>

Dave Chiluk <chiluk+linux@indeed.com> writes:

> So I spent some more time testing this new patch as is *(interrupts disabled).  I know I probably should have fixed the patch, but it's hard to get time on big test hardware sometimes, and I was already well along my way with testing.
>
> In regards to the quota usage overage I was seeing earlier: I have a theory as to what might be happening here, and I'm pretty sure it's related to the IRQs being disabled during the rq->lock walk.  I think that the main fast thread was able to use an excess amount
> of quota because the timer interrupt meant to stop it wasn't being handled timely due to the interrupts being disabled.  On my 8 core machine this resulted in a what looked like simply improved usage of the quota, but when I ran the test on an 80 core machine I
> saw a massive overage of cpu usage when running fibtest.  Specifically when running fibtest for 5 seconds with 50ms quota/100ms period expecting ~2500ms of quota usage; I got 3731 ms of cpu usage which was an unexpected overage of 1231ms. Is that a
> reasonable theory?

I think I've figured out what's going on here (and a related issue
that gave me some inconsistency when trying to debug it): other "slow"
threads can wake up while the slack timer is in distribute and
double-spend some runtime. Since we lsub_positive rather than allow
cfs_b->runtime to be negative this double-spending is permanent, and can
go on indefinitely.

In addition, if things fall out in a slightly different way, all the
"slow" threads can wind up getting on cpu and claiming slices of runtime
before the "fast" thread, and then it just has to wait another slack
period to hope that the ordering winds up better that time. This just
depends on things like IPI latency and maybe what order things happened
to happen at the start of the period.

Ugh. Maybe we /do/ just give up and say that most people don't seem to
be using cfs_b in a way that expiration of the leftover 1ms matters.

^ permalink raw reply

* Re: [PATCH v2 3/9] rcu/sync: Remove custom check for reader-section
From: Joel Fernandes @ 2019-07-12 21:35 UTC (permalink / raw)
  To: linux-kernel
  Cc: Oleg Nesterov, Alexey Kuznetsov, Bjorn Helgaas, Borislav Petkov,
	c0d1n61at3, David S. Miller, edumazet, Greg Kroah-Hartman,
	Hideaki YOSHIFUJI, H. Peter Anvin, Ingo Molnar, Jonathan Corbet,
	Josh Triplett, keescook, kernel-hardening, kernel-team,
	Lai Jiangshan, Len Brown, linux-acpi, linux-doc, linux-pci,
	linux-pm, Mathieu Desnoyers, neilb, netdev, Paul E. McKenney,
	Pavel Machek, peterz, Rafael J. Wysocki, Rasmus Villemoes, rcu,
	Steven Rostedt, Tejun Heo, Thomas Gleixner, will,
	maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)
In-Reply-To: <20190712170024.111093-4-joel@joelfernandes.org>

On Fri, Jul 12, 2019 at 01:00:18PM -0400, Joel Fernandes (Google) wrote:
> The rcu/sync code was doing its own check whether we are in a reader
> section. With RCU consolidating flavors and the generic helper added in
> this series, this is no longer need. We can just use the generic helper
> and it results in a nice cleanup.
> 
> Cc: Oleg Nesterov <oleg@redhat.com>
> Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>

Hi Oleg,
Slightly unrelated to the patch,
I tried hard to understand this comment below in percpu_down_read() but no dice.

I do understand how rcu sync and percpu rwsem works, however the comment
below didn't make much sense to me. For one, there's no readers_fast anymore
so I did not follow what readers_fast means. Could the comment be updated to
reflect latest changes?
Also could you help understand how is a writer not able to change
sem->state and count the per-cpu read counters at the same time as the
comment tries to say?

	/*
	 * We are in an RCU-sched read-side critical section, so the writer
	 * cannot both change sem->state from readers_fast and start checking
	 * counters while we are here. So if we see !sem->state, we know that
	 * the writer won't be checking until we're past the preempt_enable()
	 * and that once the synchronize_rcu() is done, the writer will see
	 * anything we did within this RCU-sched read-size critical section.
	 */

Also,
I guess we could get rid of all of the gp_ops struct stuff now that since all
the callbacks are the same now. I will post that as a follow-up patch to this
series.

thanks!

 - Joel


> ---
> Please note: Only build and boot tested this particular patch so far.
> 
>  include/linux/rcu_sync.h |  5 ++---
>  kernel/rcu/sync.c        | 22 ----------------------
>  2 files changed, 2 insertions(+), 25 deletions(-)
> 
> diff --git a/include/linux/rcu_sync.h b/include/linux/rcu_sync.h
> index 6fc53a1345b3..c954f1efc919 100644
> --- a/include/linux/rcu_sync.h
> +++ b/include/linux/rcu_sync.h
> @@ -39,9 +39,8 @@ extern void rcu_sync_lockdep_assert(struct rcu_sync *);
>   */
>  static inline bool rcu_sync_is_idle(struct rcu_sync *rsp)
>  {
> -#ifdef CONFIG_PROVE_RCU
> -	rcu_sync_lockdep_assert(rsp);
> -#endif
> +	RCU_LOCKDEP_WARN(!rcu_read_lock_any_held(),
> +			 "suspicious rcu_sync_is_idle() usage");
>  	return !rsp->gp_state; /* GP_IDLE */
>  }
>  
> diff --git a/kernel/rcu/sync.c b/kernel/rcu/sync.c
> index a8304d90573f..535e02601f56 100644
> --- a/kernel/rcu/sync.c
> +++ b/kernel/rcu/sync.c
> @@ -10,37 +10,25 @@
>  #include <linux/rcu_sync.h>
>  #include <linux/sched.h>
>  
> -#ifdef CONFIG_PROVE_RCU
> -#define __INIT_HELD(func)	.held = func,
> -#else
> -#define __INIT_HELD(func)
> -#endif
> -
>  static const struct {
>  	void (*sync)(void);
>  	void (*call)(struct rcu_head *, void (*)(struct rcu_head *));
>  	void (*wait)(void);
> -#ifdef CONFIG_PROVE_RCU
> -	int  (*held)(void);
> -#endif
>  } gp_ops[] = {
>  	[RCU_SYNC] = {
>  		.sync = synchronize_rcu,
>  		.call = call_rcu,
>  		.wait = rcu_barrier,
> -		__INIT_HELD(rcu_read_lock_held)
>  	},
>  	[RCU_SCHED_SYNC] = {
>  		.sync = synchronize_rcu,
>  		.call = call_rcu,
>  		.wait = rcu_barrier,
> -		__INIT_HELD(rcu_read_lock_sched_held)
>  	},
>  	[RCU_BH_SYNC] = {
>  		.sync = synchronize_rcu,
>  		.call = call_rcu,
>  		.wait = rcu_barrier,
> -		__INIT_HELD(rcu_read_lock_bh_held)
>  	},
>  };
>  
> @@ -49,16 +37,6 @@ enum { CB_IDLE = 0, CB_PENDING, CB_REPLAY };
>  
>  #define	rss_lock	gp_wait.lock
>  
> -#ifdef CONFIG_PROVE_RCU
> -void rcu_sync_lockdep_assert(struct rcu_sync *rsp)
> -{
> -	RCU_LOCKDEP_WARN(!gp_ops[rsp->gp_type].held(),
> -			 "suspicious rcu_sync_is_idle() usage");
> -}
> -
> -EXPORT_SYMBOL_GPL(rcu_sync_lockdep_assert);
> -#endif
> -
>  /**
>   * rcu_sync_init() - Initialize an rcu_sync structure
>   * @rsp: Pointer to rcu_sync structure to be initialized
> -- 
> 2.22.0.510.g264f2c817a-goog
> 

^ permalink raw reply

* Re: [PATCH 5/5] docs: conf.py: add CJK package needed by translations
From: Mauro Carvalho Chehab @ 2019-07-12 21:16 UTC (permalink / raw)
  To: Jonathan Corbet; +Cc: linux-doc
In-Reply-To: <20190712141240.2acc23db@lwn.net>

Em Fri, 12 Jul 2019 14:12:40 -0600
Jonathan Corbet <corbet@lwn.net> escreveu:

> On Tue,  9 Jul 2019 15:33:23 -0300
> Mauro Carvalho Chehab <mchehab+samsung@kernel.org> wrote:
> 
> > In order to be able to output Asian symbols with XeLaTeX, we
> > need the xeCJK package, and a default font for CJK symbols.
> > 
> > Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
> > ---
> >  Documentation/conf.py | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/Documentation/conf.py b/Documentation/conf.py
> > index 191bd380c523..e0e1f087d351 100644
> > --- a/Documentation/conf.py
> > +++ b/Documentation/conf.py
> > @@ -277,6 +277,10 @@ latex_elements = {
> >          \\setromanfont{DejaVu Serif}
> >          \\setmonofont{DejaVu Sans Mono}
> >  
> > +	% This is needed for translations
> > +        \\usepackage{xeCJK}
> > +        \\setCJKmainfont{Noto Sans CJK SC}
> > +
> >       '''
> >  }  
> 
> Should there be a change to sphinx-pre-install as well so we don't send
> people down the "which of 5,000 packages do I need now?" rathole?

Makes sense. I'll try to add those, probably as a separate patch.

Adding dependencies for the xeCJK package should likely be easy,
but the requirements for the Noto font is trickier, as it seems
that it requires both a texlive package and a distro-specific one
(at least on Fedora - but I suspect the same will likely be true
on other distros as well). 

I'll need to run it though all VMs I used when I created 
sphinx-pre-install (or recreate them - I haven't touched them for
a while).

Thanks,
Mauro

^ permalink raw reply

* Re: [PATCH 0/5] PDF output fixes
From: Jonathan Corbet @ 2019-07-12 20:19 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Heiko Carstens, Hannes Reinecke, linux-kbuild, Jens Axboe,
	Andy Shevchenko, Christian Borntraeger, Dan Williams,
	Michal Marek, Masahiro Yamada, Vasily Gorbik, linux-s390,
	linux-doc, Omar Sandoval
In-Reply-To: <cover.1562696797.git.mchehab+samsung@kernel.org>

On Tue,  9 Jul 2019 15:33:18 -0300
Mauro Carvalho Chehab <mchehab+samsung@kernel.org> wrote:

> In order to be able to build all PDF books, besides the two patches I
> already sent:
> 
>     docs: pdf: add all Documentation/*/index.rst to PDF output
>     docs: automarkup.py: ignore exceptions when seeking for xrefs
> 
> A few others are needed:
> 
> - patch 1 removes nested tables for a few files I converted, as 
>   Sphinx LaTeX builder doesn't support it;
> - Patches 2 to 4 addresses some minor issues on some books,
>   usually requiring some blank lines, extra whitespaces or some
>   tag replacement;
> - Patch 5 is required in order to be able to build the translations
>   PDF book, as it adds Asian fonts support to XeLaTeX.

So, modulo my one comment on the last patch the series seems OK, though I
don't like having to work around limitations in PDF generation this way.
Can't you just make rst2pdf work instead? :)

I guess it makes sense for these to go with the big band-aid-removal patch
set.

Thanks,

jon

P.S. it seems that rst2pdf is actually being developed again:
https://akrabat.com/rst2pdf-back-from-the-dead/ .  I wonder how far
they'll get with it.

^ permalink raw reply

* Re: [PATCH 5/5] docs: conf.py: add CJK package needed by translations
From: Jonathan Corbet @ 2019-07-12 20:12 UTC (permalink / raw)
  To: Mauro Carvalho Chehab; +Cc: linux-doc
In-Reply-To: <545902a588b76f18cba035700fc0835067e9a1f8.1562696797.git.mchehab+samsung@kernel.org>

On Tue,  9 Jul 2019 15:33:23 -0300
Mauro Carvalho Chehab <mchehab+samsung@kernel.org> wrote:

> In order to be able to output Asian symbols with XeLaTeX, we
> need the xeCJK package, and a default font for CJK symbols.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
> ---
>  Documentation/conf.py | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/Documentation/conf.py b/Documentation/conf.py
> index 191bd380c523..e0e1f087d351 100644
> --- a/Documentation/conf.py
> +++ b/Documentation/conf.py
> @@ -277,6 +277,10 @@ latex_elements = {
>          \\setromanfont{DejaVu Serif}
>          \\setmonofont{DejaVu Sans Mono}
>  
> +	% This is needed for translations
> +        \\usepackage{xeCJK}
> +        \\setCJKmainfont{Noto Sans CJK SC}
> +
>       '''
>  }

Should there be a change to sphinx-pre-install as well so we don't send
people down the "which of 5,000 packages do I need now?" rathole?

jon

^ permalink raw reply

* Re: [PATCH v5 1/1] sched/fair: Fix low cpu usage with high throttling by removing expiration of cpu-local slices
From: bsegall @ 2019-07-12 18:01 UTC (permalink / raw)
  To: Dave Chiluk
  Cc: Peter Zijlstra, Pqhil Auld, Peter Oskolkov, Ingo Molnar, cgroups,
	Linux Kernel Mailing List, Brendan Gregg, Kyle Anderson,
	Gabriel Munos, John Hammond, Cong Wang, Jonathan Corbet,
	linux-doc, Paul Turner
In-Reply-To: <CAC=E7cV4sO50NpYOZ06n_BkZTcBqf1KQp83prc+oave3ircBrw@mail.gmail.com>

Dave Chiluk <chiluk+linux@indeed.com> writes:

> So I spent some more time testing this new patch as is *(interrupts disabled).  I know I probably should have fixed the patch, but it's hard to get time on big test hardware sometimes, and I was already well along my way with testing.
>
> In regards to the quota usage overage I was seeing earlier: I have a
> theory as to what might be happening here, and I'm pretty sure it's
> related to the IRQs being disabled during the rq->lock walk. I think
> that the main fast thread was able to use an excess amount of quota
> because the timer interrupt meant to stop it wasn't being handled
> timely due to the interrupts being disabled. On my 8 core machine this
> resulted in a what looked like simply improved usage of the quota, but
> when I ran the test on an 80 core machine I saw a massive overage of
> cpu usage when running fibtest. Specifically when running fibtest for
> 5 seconds with 50ms quota/100ms period expecting ~2500ms of quota
> usage; I got 3731 ms of cpu usage which was an unexpected overage of
> 1231ms. Is that a reasonable theory?

Tht doesn't seem likely - taking 1ms would be way longer than I'd expect
to begin with, and runtime_remaining can go negative for that sort of
reason anyways assuming the irq time is even counted towards the task.
Also I don't that the enable-irqs version will help for the scheduler
tick at least without rt patchsets.

That is still also too much for what I was thinking of though. I'll have
to look into this more.

>
> I'll try to get some time again tomorrow to test with IRQs disabled before the walk.  Ben if you have a chance to fix and resend the patch that'd help.
>
> I'm really starting to think that simply removing the quota expiration
> may be the best solution here.  Mathmatically it works out, it makes
> the code simpler, it doesn't have any of the lock walk issues, it
> doesn't add extra latency or overhead due to the slack timer,

It works out _for the job that is supposed to be throttled_. If the job
then gets a burst of actually-expensive work on many threads it can then
use NCPUs extra ms, adding latency to any other job on the system. Given
that it's still only 1ms on each runqueue, maybe this isn't the end of
the world, but the fail case does exist.

(We have to do exactly the same locking stuff on distribute, both more
rarely on the period timer, and on the currently existing slack timer)

> and that behavior is exactly what the kernel was doing for 5 years with few complaints about overage afaik.
>
> Either way, I'm very glad that we are getting to the end of this one, and all solutions appear to solve the core of the problem.  I thank you all the work you guys have put into this.
>
> On Thu, Jul 11, 2019 at 12:46 PM <bsegall@google.com> wrote:
>
>  Peter Zijlstra <peterz@infradead.org> writes:
>
>  > FWIW, good to see progress, still waiting for you guys to agree :-)
>  >
>  > On Mon, Jul 01, 2019 at 01:15:44PM -0700, bsegall@google.com wrote:
>  >
>  >> - Taking up-to-every rq->lock is bad and expensive and 5ms may be too
>  >>   short a delay for this. I haven't tried microbenchmarks on the cost of
>  >>   this vs min_cfs_rq_runtime = 0 vs baseline.
>  >
>  > Yes, that's tricky, SGI/HPE have definite ideas about that.
>  >
>  >> @@ -4781,12 +4790,41 @@ static __always_inline void return_cfs_rq_runtime(struct cfs_rq *cfs_rq)
>  >>   */
>  >>  static void do_sched_cfs_slack_timer(struct cfs_bandwidth *cfs_b)
>  >>  {
>  >> -    u64 runtime = 0, slice = sched_cfs_bandwidth_slice();
>  >> +    u64 runtime = 0;
>  >>      unsigned long flags;
>  >>      u64 expires;
>  >> +    struct cfs_rq *cfs_rq, *temp;
>  >> +    LIST_HEAD(temp_head);
>  >> +
>  >> +    local_irq_save(flags);
>  >> +
>  >> +    raw_spin_lock(&cfs_b->lock);
>  >> +    cfs_b->slack_started = false;
>  >> +    list_splice_init(&cfs_b->slack_cfs_rq, &temp_head);
>  >> +    raw_spin_unlock(&cfs_b->lock);
>  >> +
>  >> +
>  >> +    /* Gather all left over runtime from all rqs */
>  >> +    list_for_each_entry_safe(cfs_rq, temp, &temp_head, slack_list) {
>  >> +            struct rq *rq = rq_of(cfs_rq);
>  >> +            struct rq_flags rf;
>  >> +
>  >> +            rq_lock(rq, &rf);
>  >> +
>  >> +            raw_spin_lock(&cfs_b->lock);
>  >> +            list_del_init(&cfs_rq->slack_list);
>  >> +            if (!cfs_rq->nr_running && cfs_rq->runtime_remaining > 0 &&
>  >> +                cfs_rq->runtime_expires == cfs_b->runtime_expires) {
>  >> +                    cfs_b->runtime += cfs_rq->runtime_remaining;
>  >> +                    cfs_rq->runtime_remaining = 0;
>  >> +            }
>  >> +            raw_spin_unlock(&cfs_b->lock);
>  >> +
>  >> +            rq_unlock(rq, &rf);
>  >> +    }
>  >
>  > But worse still, you take possibly every rq->lock without ever
>  > re-enabling IRQs.
>  >
>
>  Yeah, I'm not sure why I did that, it isn't correctness.

^ permalink raw reply

* Re: [PATCH 01/12] Documentation: move architectures together
From: Jonathan Corbet @ 2019-07-12 17:34 UTC (permalink / raw)
  To: Alex Shi
  Cc: linux-doc, linux-kernel, linux-stm32, linux-arm-kernel,
	linuxppc-dev, linux-riscv, linux-omap, linux-fbdev,
	linux-samsung-soc, linux-ia64, linux-mips, linux-parisc,
	linux-scsi, linux-s390, kvm, linux-sh, linux-doc
In-Reply-To: <20190712022018.27989-1-alex.shi@linux.alibaba.com>

On Fri, 12 Jul 2019 10:20:07 +0800
Alex Shi <alex.shi@linux.alibaba.com> wrote:

> There are many different archs in Documentation/ dir, it's better to
> move them together in 'Documentation/arch' which follows from kernel source.

So this seems certain to collide badly with Mauro's RST-conversion monster
patch set.

More to the point, though...if we are going to thrash up things this
badly, we want to be sure that we're doing it right so we don't end up
renaming everything again.  Grouping stuff into a new arch/ subdirectory
adds a bit of order, but it doesn't do much toward trying to organize our
documentation for its readers, and it doesn't help us to modernize the
docs and get rid of the old, useless stuff.  A quick check shows that many
of these files have seen no changes other than typo fixes since the
beginning of the Git era.

So, in my mind, this needs some thought.  Maybe we want a
Documentation/arch in the end, but I'm not convinced that we should just
create it and fill it with a snow shovel.  This might be a good thing to
discuss at the kernel summit in September.

Thanks,

jon

^ permalink raw reply

* [PATCH v2 1/9] rcu/update: Remove useless check for debug_locks
From: Joel Fernandes (Google) @ 2019-07-12 17:00 UTC (permalink / raw)
  To: linux-kernel
  Cc: Joel Fernandes (Google), Alexey Kuznetsov, Bjorn Helgaas,
	Borislav Petkov, c0d1n61at3, David S. Miller, edumazet,
	Greg Kroah-Hartman, Hideaki YOSHIFUJI, H. Peter Anvin,
	Ingo Molnar, Jonathan Corbet, Josh Triplett, keescook,
	kernel-hardening, kernel-team, Lai Jiangshan, Len Brown,
	linux-acpi, linux-doc, linux-pci, linux-pm, Mathieu Desnoyers,
	neilb, netdev, Oleg Nesterov, Paul E. McKenney, Pavel Machek,
	peterz, Rafael J. Wysocki, Rasmus Villemoes, rcu, Steven Rostedt,
	Tejun Heo, Thomas Gleixner, will,
	maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)
In-Reply-To: <20190712170024.111093-1-joel@joelfernandes.org>

In rcu_read_lock_sched_held(), debug_locks can never be true at the
point we check it because we already check debug_locks in
debug_lockdep_rcu_enabled() in the beginning. Remove the check.

Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
---
 kernel/rcu/update.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/kernel/rcu/update.c b/kernel/rcu/update.c
index c3bf44ba42e5..bb961cd89e76 100644
--- a/kernel/rcu/update.c
+++ b/kernel/rcu/update.c
@@ -93,17 +93,13 @@ module_param(rcu_normal_after_boot, int, 0);
  */
 int rcu_read_lock_sched_held(void)
 {
-	int lockdep_opinion = 0;
-
 	if (!debug_lockdep_rcu_enabled())
 		return 1;
 	if (!rcu_is_watching())
 		return 0;
 	if (!rcu_lockdep_current_cpu_online())
 		return 0;
-	if (debug_locks)
-		lockdep_opinion = lock_is_held(&rcu_sched_lock_map);
-	return lockdep_opinion || !preemptible();
+	return lock_is_held(&rcu_sched_lock_map) || !preemptible();
 }
 EXPORT_SYMBOL(rcu_read_lock_sched_held);
 #endif
-- 
2.22.0.510.g264f2c817a-goog


^ permalink raw reply related

* [PATCH v2 3/9] rcu/sync: Remove custom check for reader-section
From: Joel Fernandes (Google) @ 2019-07-12 17:00 UTC (permalink / raw)
  To: linux-kernel
  Cc: Joel Fernandes (Google), Oleg Nesterov, Alexey Kuznetsov,
	Bjorn Helgaas, Borislav Petkov, c0d1n61at3, David S. Miller,
	edumazet, Greg Kroah-Hartman, Hideaki YOSHIFUJI, H. Peter Anvin,
	Ingo Molnar, Jonathan Corbet, Josh Triplett, keescook,
	kernel-hardening, kernel-team, Lai Jiangshan, Len Brown,
	linux-acpi, linux-doc, linux-pci, linux-pm, Mathieu Desnoyers,
	neilb, netdev, Paul E. McKenney, Pavel Machek, peterz,
	Rafael J. Wysocki, Rasmus Villemoes, rcu, Steven Rostedt,
	Tejun Heo, Thomas Gleixner, will,
	maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)
In-Reply-To: <20190712170024.111093-1-joel@joelfernandes.org>

The rcu/sync code was doing its own check whether we are in a reader
section. With RCU consolidating flavors and the generic helper added in
this series, this is no longer need. We can just use the generic helper
and it results in a nice cleanup.

Cc: Oleg Nesterov <oleg@redhat.com>
Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
---
Please note: Only build and boot tested this particular patch so far.

 include/linux/rcu_sync.h |  5 ++---
 kernel/rcu/sync.c        | 22 ----------------------
 2 files changed, 2 insertions(+), 25 deletions(-)

diff --git a/include/linux/rcu_sync.h b/include/linux/rcu_sync.h
index 6fc53a1345b3..c954f1efc919 100644
--- a/include/linux/rcu_sync.h
+++ b/include/linux/rcu_sync.h
@@ -39,9 +39,8 @@ extern void rcu_sync_lockdep_assert(struct rcu_sync *);
  */
 static inline bool rcu_sync_is_idle(struct rcu_sync *rsp)
 {
-#ifdef CONFIG_PROVE_RCU
-	rcu_sync_lockdep_assert(rsp);
-#endif
+	RCU_LOCKDEP_WARN(!rcu_read_lock_any_held(),
+			 "suspicious rcu_sync_is_idle() usage");
 	return !rsp->gp_state; /* GP_IDLE */
 }
 
diff --git a/kernel/rcu/sync.c b/kernel/rcu/sync.c
index a8304d90573f..535e02601f56 100644
--- a/kernel/rcu/sync.c
+++ b/kernel/rcu/sync.c
@@ -10,37 +10,25 @@
 #include <linux/rcu_sync.h>
 #include <linux/sched.h>
 
-#ifdef CONFIG_PROVE_RCU
-#define __INIT_HELD(func)	.held = func,
-#else
-#define __INIT_HELD(func)
-#endif
-
 static const struct {
 	void (*sync)(void);
 	void (*call)(struct rcu_head *, void (*)(struct rcu_head *));
 	void (*wait)(void);
-#ifdef CONFIG_PROVE_RCU
-	int  (*held)(void);
-#endif
 } gp_ops[] = {
 	[RCU_SYNC] = {
 		.sync = synchronize_rcu,
 		.call = call_rcu,
 		.wait = rcu_barrier,
-		__INIT_HELD(rcu_read_lock_held)
 	},
 	[RCU_SCHED_SYNC] = {
 		.sync = synchronize_rcu,
 		.call = call_rcu,
 		.wait = rcu_barrier,
-		__INIT_HELD(rcu_read_lock_sched_held)
 	},
 	[RCU_BH_SYNC] = {
 		.sync = synchronize_rcu,
 		.call = call_rcu,
 		.wait = rcu_barrier,
-		__INIT_HELD(rcu_read_lock_bh_held)
 	},
 };
 
@@ -49,16 +37,6 @@ enum { CB_IDLE = 0, CB_PENDING, CB_REPLAY };
 
 #define	rss_lock	gp_wait.lock
 
-#ifdef CONFIG_PROVE_RCU
-void rcu_sync_lockdep_assert(struct rcu_sync *rsp)
-{
-	RCU_LOCKDEP_WARN(!gp_ops[rsp->gp_type].held(),
-			 "suspicious rcu_sync_is_idle() usage");
-}
-
-EXPORT_SYMBOL_GPL(rcu_sync_lockdep_assert);
-#endif
-
 /**
  * rcu_sync_init() - Initialize an rcu_sync structure
  * @rsp: Pointer to rcu_sync structure to be initialized
-- 
2.22.0.510.g264f2c817a-goog


^ permalink raw reply related

* [PATCH v2 4/9] ipv4: add lockdep condition to fix for_each_entry
From: Joel Fernandes (Google) @ 2019-07-12 17:00 UTC (permalink / raw)
  To: linux-kernel
  Cc: Joel Fernandes (Google), Alexey Kuznetsov, Bjorn Helgaas,
	Borislav Petkov, c0d1n61at3, David S. Miller, edumazet,
	Greg Kroah-Hartman, Hideaki YOSHIFUJI, H. Peter Anvin,
	Ingo Molnar, Jonathan Corbet, Josh Triplett, keescook,
	kernel-hardening, kernel-team, Lai Jiangshan, Len Brown,
	linux-acpi, linux-doc, linux-pci, linux-pm, Mathieu Desnoyers,
	neilb, netdev, Oleg Nesterov, Paul E. McKenney, Pavel Machek,
	peterz, Rafael J. Wysocki, Rasmus Villemoes, rcu, Steven Rostedt,
	Tejun Heo, Thomas Gleixner, will,
	maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)
In-Reply-To: <20190712170024.111093-1-joel@joelfernandes.org>

Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
---
 net/ipv4/fib_frontend.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/ipv4/fib_frontend.c b/net/ipv4/fib_frontend.c
index b298255f6fdb..ef7c9f8e8682 100644
--- a/net/ipv4/fib_frontend.c
+++ b/net/ipv4/fib_frontend.c
@@ -127,7 +127,8 @@ struct fib_table *fib_get_table(struct net *net, u32 id)
 	h = id & (FIB_TABLE_HASHSZ - 1);
 
 	head = &net->ipv4.fib_table_hash[h];
-	hlist_for_each_entry_rcu(tb, head, tb_hlist) {
+	hlist_for_each_entry_rcu(tb, head, tb_hlist,
+				 lockdep_rtnl_is_held()) {
 		if (tb->tb_id == id)
 			return tb;
 	}
-- 
2.22.0.510.g264f2c817a-goog


^ permalink raw reply related

* [PATCH v2 5/9] driver/core: Convert to use built-in RCU list checking
From: Joel Fernandes (Google) @ 2019-07-12 17:00 UTC (permalink / raw)
  To: linux-kernel
  Cc: Joel Fernandes (Google), Greg Kroah-Hartman, Alexey Kuznetsov,
	Bjorn Helgaas, Borislav Petkov, c0d1n61at3, David S. Miller,
	edumazet, Hideaki YOSHIFUJI, H. Peter Anvin, Ingo Molnar,
	Jonathan Corbet, Josh Triplett, keescook, kernel-hardening,
	kernel-team, Lai Jiangshan, Len Brown, linux-acpi, linux-doc,
	linux-pci, linux-pm, Mathieu Desnoyers, neilb, netdev,
	Oleg Nesterov, Paul E. McKenney, Pavel Machek, peterz,
	Rafael J. Wysocki, Rasmus Villemoes, rcu, Steven Rostedt,
	Tejun Heo, Thomas Gleixner, will,
	maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)
In-Reply-To: <20190712170024.111093-1-joel@joelfernandes.org>

list_for_each_entry_rcu has built-in RCU and lock checking. Make use of
it in driver core.

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
---
 drivers/base/base.h          |  1 +
 drivers/base/core.c          | 10 ++++++++++
 drivers/base/power/runtime.c | 15 ++++++++++-----
 3 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/drivers/base/base.h b/drivers/base/base.h
index b405436ee28e..0d32544b6f91 100644
--- a/drivers/base/base.h
+++ b/drivers/base/base.h
@@ -165,6 +165,7 @@ static inline int devtmpfs_init(void) { return 0; }
 /* Device links support */
 extern int device_links_read_lock(void);
 extern void device_links_read_unlock(int idx);
+extern int device_links_read_lock_held(void);
 extern int device_links_check_suppliers(struct device *dev);
 extern void device_links_driver_bound(struct device *dev);
 extern void device_links_driver_cleanup(struct device *dev);
diff --git a/drivers/base/core.c b/drivers/base/core.c
index fd7511e04e62..6c5ca9685647 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -68,6 +68,11 @@ void device_links_read_unlock(int idx)
 {
 	srcu_read_unlock(&device_links_srcu, idx);
 }
+
+int device_links_read_lock_held(void)
+{
+	return srcu_read_lock_held(&device_links_srcu);
+}
 #else /* !CONFIG_SRCU */
 static DECLARE_RWSEM(device_links_lock);
 
@@ -91,6 +96,11 @@ void device_links_read_unlock(int not_used)
 {
 	up_read(&device_links_lock);
 }
+
+int device_links_read_lock_held(void)
+{
+	return lock_is_held(&device_links_lock);
+}
 #endif /* !CONFIG_SRCU */
 
 /**
diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c
index 952a1e7057c7..7a10e8379a70 100644
--- a/drivers/base/power/runtime.c
+++ b/drivers/base/power/runtime.c
@@ -287,7 +287,8 @@ static int rpm_get_suppliers(struct device *dev)
 {
 	struct device_link *link;
 
-	list_for_each_entry_rcu(link, &dev->links.suppliers, c_node) {
+	list_for_each_entry_rcu(link, &dev->links.suppliers, c_node,
+				device_links_read_lock_held()) {
 		int retval;
 
 		if (!(link->flags & DL_FLAG_PM_RUNTIME) ||
@@ -309,7 +310,8 @@ static void rpm_put_suppliers(struct device *dev)
 {
 	struct device_link *link;
 
-	list_for_each_entry_rcu(link, &dev->links.suppliers, c_node) {
+	list_for_each_entry_rcu(link, &dev->links.suppliers, c_node,
+				device_links_read_lock_held()) {
 		if (READ_ONCE(link->status) == DL_STATE_SUPPLIER_UNBIND)
 			continue;
 
@@ -1640,7 +1642,8 @@ void pm_runtime_clean_up_links(struct device *dev)
 
 	idx = device_links_read_lock();
 
-	list_for_each_entry_rcu(link, &dev->links.consumers, s_node) {
+	list_for_each_entry_rcu(link, &dev->links.consumers, s_node,
+				device_links_read_lock_held()) {
 		if (link->flags & DL_FLAG_STATELESS)
 			continue;
 
@@ -1662,7 +1665,8 @@ void pm_runtime_get_suppliers(struct device *dev)
 
 	idx = device_links_read_lock();
 
-	list_for_each_entry_rcu(link, &dev->links.suppliers, c_node)
+	list_for_each_entry_rcu(link, &dev->links.suppliers, c_node,
+				device_links_read_lock_held())
 		if (link->flags & DL_FLAG_PM_RUNTIME) {
 			link->supplier_preactivated = true;
 			refcount_inc(&link->rpm_active);
@@ -1683,7 +1687,8 @@ void pm_runtime_put_suppliers(struct device *dev)
 
 	idx = device_links_read_lock();
 
-	list_for_each_entry_rcu(link, &dev->links.suppliers, c_node)
+	list_for_each_entry_rcu(link, &dev->links.suppliers, c_node,
+				device_links_read_lock_held())
 		if (link->supplier_preactivated) {
 			link->supplier_preactivated = false;
 			if (refcount_dec_not_one(&link->rpm_active))
-- 
2.22.0.510.g264f2c817a-goog


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox