Re: [PATCH RFC tip/core/rcu] Make SRCU be once again optional
From: Arnd Bergmann
Date: Tue Jan 16 2018 - 17:55:16 EST
On Tue, Jan 16, 2018 at 11:34 PM, Paul E. McKenney
<paulmck@xxxxxxxxxxxxxxxxxx> wrote:
> 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@xxxxxxxxxxxxxxxxxx> 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.)
This is using today's linux-next, but I got the same thing pretty much for
every step of the bisection since May.
My configuration has
CONFIG_TINY_RCU=y
# CONFIG_RCU_EXPERT is not set
# CONFIG_TASKS_RCU is not set
# CONFIG_RCU_STALL_COMMON is not set
# CONFIG_RCU_NEED_SEGCBLIST is not set
# CONFIG_PHY_LANTIQ_RCU_USB2 is not set
# CONFIG_PROVE_RCU is not set
# CONFIG_RCU_PERF_TEST is not set
# CONFIG_RCU_TORTURE_TEST is not set
# CONFIG_RCU_TRACE is not set
# CONFIG_RCU_EQS_DEBUG is not set
# CONFIG_PROVE_RCU is not set
CONFIG_DEBUG_LOCK_ALLOC=y
> 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 */
That fixed the first warning for me, doing the same thing for all three
fixed the rest:
diff --git a/include/linux/srcu.h b/include/linux/srcu.h
index 62be8966e837..1bab741b384b 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 */
@@ -157,7 +159,9 @@ static inline int srcu_read_lock(struct
srcu_struct *sp) __acquires(sp)
int retval;
retval = __srcu_read_lock(sp);
+#ifdef CONFIG_PROVE_RCU
rcu_lock_acquire(&(sp)->dep_map);
+#endif
return retval;
}
@@ -171,7 +175,9 @@ static inline int srcu_read_lock(struct
srcu_struct *sp) __acquires(sp)
static inline void srcu_read_unlock(struct srcu_struct *sp, int idx)
__releases(sp)
{
+#ifdef CONFIG_PROVE_RCU
rcu_lock_release(&(sp)->dep_map);
+#endif
__srcu_read_unlock(sp, idx);
}