Re: [PATCH v3 06/20] seqlock: Extend seqcount API with associated locks

Peter Zijlstra
2020-07-08T15:35:36Z

On Wed, Jul 08, 2020 at 05:09:30PM +0200, Ahmed S. Darwish wrote:
> On Wed, Jul 08, 2020 at 02:29:38PM +0200, Peter Zijlstra wrote:

> > How about something disguisting like this then?
> >
> ...
> > #define __SEQ_RT IS_BUILTIN(CONFIG_PREEMPT_RT)
> >
> > SEQCOUNT_LOCKTYPE(raw_spinlock, raw_spinlock_t, false, lock)
> > SEQCOUNT_LOCKTYPE(spinlock, spinlock_t, __SEQ_RT, lock)
> > SEQCOUNT_LOCKTYPE(rwlock, rwlock_t, __SEQ_RT, lock)
> > SEQCOUNT_LOCKTYPE(mutex, struct mutex, true, lock)
> > SEQCOUNT_LOCKTYPE(ww_mutex, struct ww_mutex, true, lock->base)
> >
> > #if (defined(CONFIG_CC_IS_GCC) && CONFIG_GCC_VERSION < 40900) || defined(__CHECKER__)
> >
> > #define __seqprop_pick(const_expr, s, locktype, prop, otherwise) \
> > __builtin_choose_expr(const_expr, \
> > __seqprop_##locktype##_##prop((void *)(s)), \
> > otherwise)
> >
> > extern void __seqprop_invalid(void);
> >
> > #define __seqprop(s, prop) \
> > __seqprop_pick(__same_type(*(s), seqcount_t), (s), seqcount, prop, \
> > __seqprop_pick(__same_type(*(s), seqcount_raw_spinlock_t), (s), raw_spinlock, prop, \
> > __seqprop_pick(__same_type(*(s), seqcount_spinlock_t), (s), spinlock, prop, \
> > __seqprop_pick(__same_type(*(s), seqcount_rwlock_t), (s), rwlock, prop, \
> > __seqprop_pick(__same_type(*(s), seqcount_mutex_t), (s), mutex, prop, \
> > __seqprop_pick(__same_type(*(s), seqcount_ww_mutex_t), (s), ww_mutex, prop, \
> > __seqprop_invalid()))))))
> >
> > #else
> >
> > #define __seqprop_case(s, locktype, prop) \
> > seqcount_##locktype##_t: __seqprop_##locktype##_##prop((void *)s)
> >
> > #define __seqprop(s, prop) \
> > _Generic(*(s), \
> > seqcount_t: __seqprop_seqcount_##prop((void*)s),\
> > __seqprop_case((s), raw_spinlock, prop), \
> > __seqprop_case((s), spinlock, prop), \
> > __seqprop_case((s), rwlock, prop), \
> > __seqprop_case((s), mutex, prop), \
> > __seqprop_case((s), ww_mutex, prop))
> >
> > #endif
> >
> > #define __to_seqcount_t(s) __seqprop(s, ptr)
> > #define __associated_lock_is_preemptible(s) __seqprop(s, preempt)
> > #define __assert_associated_lock_held(s) __seqprop(s, assert)
>
> Hmm, I'll prototype the whole thing (along with PREEMPT_RT associated
> lock()/unlock() as you've mentioned in the other e-mail), and come back.
>
> Honestly, I have a first impression that this is heading into too much
> complexity and compaction, but let's finish the whole thing first.

So the thing I pasted compiles kernel/sched/cputime.o, but that only
uses the old seqcount_t thing, not any of the fancy new stuff, still the
compiler groks it all.

And while the gcc-4.8 code is horrendous crap, the rest should be pretty
straight forward and concentrates on the pieces where there are
differences.

I even considered:

#define __SEQPROP(name, prop, expr) \
static __always_inline __seqprop_##prop##_t \
__seqprop##name##_##prop(seqcount##name##_t *s) \
{ \
expr; \
}

Such that we could write:

__SEQPROP(, ptr, return s)
__SEQPROP(, preempt, return false)
__SEQPROP(, assert, )

__SEQPROP(_##locktype, ptr, return &s->seqcount) \
__SEQPROP(_##locktype, preempt, return preempt) \
__SEQPROP(_##locktype, assert, __SEQCOUNT_LOCKDEP(lockdep_assert_held(s->lockmember))) \

But I figured _that_ might've been one step too far ;-)