Message-ID: <CAK8P3a0STk8VU1H-JOGnwiFCGnu5Rqsw8_sddaYLgZCbXShb0Q@mail.gmail.com>
Date:   Tue, 16 Jan 2018 23:55:10 +0100
From:   Arnd Bergmann <arnd@...db.de>
To:     "Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>
Cc:     Nicolas Pitre <nicolas.pitre@...aro.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Josh Triplett <josh@...htriplett.org>
Subject: Re: [PATCH RFC tip/core/rcu] Make SRCU be once again optional

On Tue, Jan 16, 2018 at 11:34 PM, Paul E. McKenney
<paulmck@...ux.vnet.ibm.com> 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@...ux.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.)

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);
 }

Powered by blists - more mailing lists