Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Wed, 8 Jul 2020 17:35:22 +0200
From:   Peter Zijlstra <peterz@infradead.org>
To:     "Ahmed S. Darwish" <a.darwish@linutronix.de>
Cc:     Ingo Molnar <mingo@redhat.com>, Will Deacon <will@kernel.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        "Paul E. McKenney" <paulmck@kernel.org>,
        "Sebastian A. Siewior" <bigeasy@linutronix.de>,
        Steven Rostedt <rostedt@goodmis.org>,
        LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v3 06/20] seqlock: Extend seqcount API with associated
 locks
Message-ID: <20200708153522.GR4800@hirez.programming.kicks-ass.net>
References: <20200630054452.3675847-1-a.darwish@linutronix.de>
 <20200630054452.3675847-7-a.darwish@linutronix.de>
 <20200706212148.GE5523@worktop.programming.kicks-ass.net>
 <20200707084024.GA4097637@debian-buster-darwi.lab.linutronix.de>
 <20200707130410.GO4800@hirez.programming.kicks-ass.net>
 <20200707143726.GO117543@hirez.programming.kicks-ass.net>
 <20200708103314.GB4151780@debian-buster-darwi.lab.linutronix.de>
 <20200708122938.GQ4800@hirez.programming.kicks-ass.net>
 <20200708150930.GA147323@debian-buster-darwi.lab.linutronix.de>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20200708150930.GA147323@debian-buster-darwi.lab.linutronix.de>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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