Date: Tue, 16 Jan 2018 14:34:31 -0800
From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
To: Arnd Bergmann <arnd@arndb.de>
Cc: Nicolas Pitre <nicolas.pitre@linaro.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Josh Triplett <josh@joshtriplett.org>
Subject: Re: [PATCH RFC tip/core/rcu] Make SRCU be once again optional
Reply-To: paulmck@linux.vnet.ibm.com
References: <20170428211546.GA23590@linux.vnet.ibm.com>
 <alpine.LFD.2.20.1704281744220.21943@knanqh.ubzr>
 <20170429001040.GH3956@linux.vnet.ibm.com>
 <20170512184155.GA9482@linux.vnet.ibm.com>
 <alpine.LFD.2.20.1705121444260.3061@knanqh.ubzr>
 <20170512191005.GE3956@linux.vnet.ibm.com>
 <20170603035915.GA23375@linux.vnet.ibm.com>
 <alpine.LFD.2.20.1706030102570.17440@knanqh.ubzr>
 <20170603203620.GL3721@linux.vnet.ibm.com>
 <CAK8P3a1AKdSiansYoztQ1GqOUvL8YPAP8HnSGe_gv+wwHk4f4w@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CAK8P3a1AKdSiansYoztQ1GqOUvL8YPAP8HnSGe_gv+wwHk4f4w@mail.gmail.com>
User-Agent: Mutt/1.5.21 (2010-09-15)
Message-Id: <20180116223431.GA9671@linux.vnet.ibm.com>
Sender: linux-kernel-owner@vger.kernel.org

On Tue, Jan 16, 2018 at 10:02:10PM +0100, Arnd Bergmann wrote:
> On Sat, Jun 3, 2017 at 10:36 PM, Paul E. McKenney
> <paulmck@linux.vnet.ibm.com> wrote:
> > On Sat, Jun 03, 2017 at 01:18:43AM -0400, Nicolas Pitre wrote:
> >> On Fri, 2 Jun 2017, Paul E. McKenney wrote:
> >>
> >> > On Fri, May 12, 2017 at 12:10:05PM -0700, Paul E. McKenney wrote:
> >> > > On Fri, May 12, 2017 at 02:59:48PM -0400, Nicolas Pitre wrote:
> >> > > > On Fri, 12 May 2017, Paul E. McKenney wrote:
> >> >
> >> > [ . . . ]
> >> >
> >> > > > No.  "Available in mainline" is the name of the game for all I do. If it
> >> > > > can't be made acceptable for mainline then it basically has no chance of
> >> > > > gaining traction and becoming generally useful. My approach is therefore
> >> > > > to always find solutions that can be maintained upstream and contributed
> >> > > > to with minimal fuss by anyone.
> >> > >
> >> > > OK, then wish me luck.  ;-)
> >> >
> >> > And still quite a bit of back and forth.  How are things with tty?
> >> >
> >> > One question that came up -- what sort of SoCs are you targeting?
> >> > A number of people are insisting that smartphone SoCs with 256M DRAM
> >> > are the minimal systems of the future.  This seems unlikely to me,
> >> > given the potential for extremely cheap SoCs with EDRAM or some such,
> >> > but figured I should ask what you are targeting.
> >>
> >> I'm targetting 256 *kilobytes* of RAM. Most likely SRAM. That's not for
> >> smart phones but really cheap IoT devices. That's the next area for
> >> (trimmed down) Linux to conquer. Example targets are STM32 chips.
> >>
> >> Please see the following for the rationale and how to get there:
> >>
> >> https://lwn.net/Articles/721074/
> >>
> >> http://www.mail-archive.com/search?l=mid&q=alpine.LFD.2.20.1703241215540.2304%40knanqh.ubzr
> >
> > Ah, thank you for the reminder.  I did read that article, but somehow
> > got a few megabytes stuck in my head instead of the correct quarter meg.
> >
> > Anyway, don't look now, but Tiny {S,}RCU just might live on, for a bit
> > longer, anyway.
> 
> It took me around 200000 randconfig builds since May, but I eventually
> ran into the regression caused by this patch, building an ARM kernel
> with the defconfig from https://pastebin.com/TiTWHP8t as input results
> in this build failure:

Yow!!!  I am impressed!

>   CC      arch/arm/kernel/asm-offsets.s
> In file included from ./include/linux/notifier.h:16:0,
>                  from ./include/linux/memory_hotplug.h:7,
>                  from ./include/linux/mmzone.h:775,
>                  from ./include/linux/gfp.h:6,
>                  from ./include/linux/mm.h:10,
>                  from arch/arm/kernel/asm-offsets.c:15:
> ./include/linux/srcu.h: In function 'srcu_read_lock_held':
> ./include/linux/srcu.h:99:25: error: 'struct srcu_struct' has no
> member named 'dep_map'
>   return lock_is_held(&sp->dep_map);
>                          ^~

This one I get -- I messed up and let the compiler evaluate ->dep_map
even for !CONFIG_DEBUG_LOCK_ALLOC.  Does the patch below help?

> ./include/linux/srcu.h: In function 'srcu_read_lock':
> ./include/linux/srcu.h:160:24: error: 'struct srcu_struct' has no
> member named 'dep_map'
>   rcu_lock_acquire(&(sp)->dep_map);
>                         ^~
> ./include/linux/srcu.h: In function 'srcu_read_unlock':
> ./include/linux/srcu.h:174:24: error: 'struct srcu_struct' has no
> member named 'dep_map'
>   rcu_lock_release(&(sp)->dep_map);
>                         ^~

These two I don't get given the definitions for !CONFIG_DEBUG_LOCK_ALLOC:

# define rcu_lock_acquire(a)		do { } while (0)
# define rcu_lock_release(a)		do { } while (0)

Is your build somehow picking up a different definition?  Or are you
using an older kernel (if so, please let me know the version.)

> I think what happened here is that randconfig builds basically never hit the
> CONFIG_SRCU=n case since lots of other things 'select SRCU', directly
> or indirectly. Until commit c9afbec27089 ("debugfs: purge obsolete SRCU
> based removal protection"), SRCU was selected by debugfs, which is
> practically always on, now it has become much easier to disable it,
> but it's still fairly unlikely.

It has been getting harder.

Another option would be simply conditionally compile out all or most
of include/linux/srcu.h for !CONFIG_SRCU builds.

Thoughts?

							Thanx, Paul

------------------------------------------------------------------------

diff --git a/include/linux/srcu.h b/include/linux/srcu.h
index 62be8966e837..b4fd484ad6cb 100644
--- a/include/linux/srcu.h
+++ b/include/linux/srcu.h
@@ -94,9 +94,11 @@ void synchronize_srcu(struct srcu_struct *sp);
  */
 static inline int srcu_read_lock_held(struct srcu_struct *sp)
 {
-	if (!debug_lockdep_rcu_enabled())
-		return 1;
+#ifdef CONFIG_PROVE_RCU
 	return lock_is_held(&sp->dep_map);
+#else /* #ifdef CONFIG_PROVE_RCU */
+	return 1;
+#endif /* #else #ifdef CONFIG_PROVE_RCU */
 }
 
 #else /* #ifdef CONFIG_DEBUG_LOCK_ALLOC */