Re: [dm-devel] [PATCH 2/2] realtime: avoid BUG when the list is not locked

Re: [dm-devel] [PATCH 2/2] realtime: avoid BUG when the list is not locked

[Daniel Wagner]

[cc: linux-user-rt, Thomas and Sebastian]

On Mon, Nov 11, 2019 at 08:59:44AM -0500, Mikulas Patocka wrote:
> list_bl.h would crash with BUG() if we used it without locking. 
> dm-snapshot uses its own locking on readltime kernels, so to avoid this 
> BUG, we must set LIST_BL_LOCKMASK = 0.
> 
> Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
> 
> Index: linux-rt-devel/include/linux/list_bl.h
> ===================================================================
> --- linux-rt-devel.orig/include/linux/list_bl.h	2019-11-07 14:01:51.000000000 +0100
> +++ linux-rt-devel/include/linux/list_bl.h	2019-11-08 10:12:49.000000000 +0100
> @@ -19,7 +19,7 @@
>   * some fast and compact auxiliary data.
>   */
>  
> -#if defined(CONFIG_SMP) || defined(CONFIG_DEBUG_SPINLOCK)
> +#if (defined(CONFIG_SMP) || defined(CONFIG_DEBUG_SPINLOCK)) && !defined(CONFIG_PREEMPT_RT_BASE)

CONFIG_PREEMPT_RT_BASE is only available in the out of tree -rt patch
set, therefore I assume it's not for mainline, right?

>  #define LIST_BL_LOCKMASK	1UL
>  #else
>  #define LIST_BL_LOCKMASK	0UL
> @@ -161,9 +161,6 @@ static inline void hlist_bl_lock(struct
>  	bit_spin_lock(0, (unsigned long *)b);
>  #else
>  	raw_spin_lock(&b->lock);
> -#if defined(CONFIG_SMP) || defined(CONFIG_DEBUG_SPINLOCK)
> -	__set_bit(0, (unsigned long *)b);
> -#endif
>  #endif
>  }
>  
> @@ -172,9 +169,6 @@ static inline void hlist_bl_unlock(struc
>  #ifndef CONFIG_PREEMPT_RT_BASE
>  	__bit_spin_unlock(0, (unsigned long *)b);
>  #else
> -#if defined(CONFIG_SMP) || defined(CONFIG_DEBUG_SPINLOCK)
> -	__clear_bit(0, (unsigned long *)b);
> -#endif
>  	raw_spin_unlock(&b->lock);
>  #endif
>  }
> 
> --
> dm-devel mailing list
> dm-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/dm-devel
> 

Thanks,
Daniel

Re: [dm-devel] [PATCH 2/2] realtime: avoid BUG when the list is not locked

[Mikulas Patocka]

On Tue, 12 Nov 2019, Daniel Wagner wrote:

> [cc: linux-user-rt, Thomas and Sebastian]
> 
> On Mon, Nov 11, 2019 at 08:59:44AM -0500, Mikulas Patocka wrote:
> > list_bl.h would crash with BUG() if we used it without locking. 
> > dm-snapshot uses its own locking on readltime kernels, so to avoid this 
> > BUG, we must set LIST_BL_LOCKMASK = 0.
> > 
> > Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
> > 
> > Index: linux-rt-devel/include/linux/list_bl.h
> > ===================================================================
> > --- linux-rt-devel.orig/include/linux/list_bl.h	2019-11-07 14:01:51.000000000 +0100
> > +++ linux-rt-devel/include/linux/list_bl.h	2019-11-08 10:12:49.000000000 +0100
> > @@ -19,7 +19,7 @@
> >   * some fast and compact auxiliary data.
> >   */
> >  
> > -#if defined(CONFIG_SMP) || defined(CONFIG_DEBUG_SPINLOCK)
> > +#if (defined(CONFIG_SMP) || defined(CONFIG_DEBUG_SPINLOCK)) && !defined(CONFIG_PREEMPT_RT_BASE)
> 
> CONFIG_PREEMPT_RT_BASE is only available in the out of tree -rt patch
> set, therefore I assume it's not for mainline, right?

Yes - this is intended to be applied to the realtime patchset.

Mikulas

> >  #define LIST_BL_LOCKMASK	1UL
> >  #else
> >  #define LIST_BL_LOCKMASK	0UL
> > @@ -161,9 +161,6 @@ static inline void hlist_bl_lock(struct
> >  	bit_spin_lock(0, (unsigned long *)b);
> >  #else
> >  	raw_spin_lock(&b->lock);
> > -#if defined(CONFIG_SMP) || defined(CONFIG_DEBUG_SPINLOCK)
> > -	__set_bit(0, (unsigned long *)b);
> > -#endif
> >  #endif
> >  }
> >  
> > @@ -172,9 +169,6 @@ static inline void hlist_bl_unlock(struc
> >  #ifndef CONFIG_PREEMPT_RT_BASE
> >  	__bit_spin_unlock(0, (unsigned long *)b);
> >  #else
> > -#if defined(CONFIG_SMP) || defined(CONFIG_DEBUG_SPINLOCK)
> > -	__clear_bit(0, (unsigned long *)b);
> > -#endif
> >  	raw_spin_unlock(&b->lock);
> >  #endif
> >  }
> > 
> > --
> > dm-devel mailing list
> > dm-devel@redhat.com
> > https://www.redhat.com/mailman/listinfo/dm-devel
> > 
> 
> Thanks,
> Daniel
> 

Re: [dm-devel] [PATCH 2/2] realtime: avoid BUG when the list is not locked

[Sebastian Andrzej Siewior]

On 2019-11-12 06:21:57 [-0500], Mikulas Patocka wrote:
> > CONFIG_PREEMPT_RT_BASE is only available in the out of tree -rt patch
> > set, therefore I assume it's not for mainline, right?
> 
> Yes - this is intended to be applied to the realtime patchset.

In that case please repost it and Cc tglx and myself and add the RT tag
after PATCH.
Please make sure it applies and fixes a problem in the latest available
RT kernel. Please state the problem on RT and how it is solved.

> Mikulas

Sebastian