On 2020-05-25 09:01:39 [+0200], Ingo Molnar wrote: > > * Sebastian Andrzej Siewior <[email protected]> wrote: > > > From: Thomas Gleixner <[email protected]> > > > > To address this PREEMPT_RT introduced the concept of local_locks which are > > strictly per CPU. > > > +++ b/include/linux/locallock_internal.h > > @@ -0,0 +1,90 @@ > > +/* SPDX-License-Identifier: GPL-2.0 */ > > +#ifndef _LINUX_LOCALLOCK_H > > +# error "Do not include directly, include linux/locallock.h" > > +#endif > > + > > +#include <linux/percpu-defs.h> > > +#include <linux/lockdep.h> > > + > > +struct local_lock { > > +#ifdef CONFIG_DEBUG_LOCK_ALLOC > > + struct lockdep_map dep_map; > > + struct task_struct *owner; > > +#endif > > +}; > > This this looks very nice to me, there's a minor data structure > nomenclature related comment I have: > > So local locks were supposed to be a look-alike to all the other > locking constructs we have, spinlock_t in particular. Why isn't there > a local_lock_t, instead of requiring 'struct local_lock'?
|git grep "struct \<spinlock\>" and I did convert them spinlock_t and got even asked why https://lore.kernel.org/driverdev-devel/[email protected]/ but yes. I can stick to local_lock_t instead. > This abbreviation signals that these are 'small' data structures on > mainline kernels (zero size in fact), but the other advantage is that > the shorter name would prevent bloating of previously compact > structure definitions, such as: > > > struct squashfs_stream { > > - void *stream; > > + void *stream; > > + struct local_lock lock; > > }; > > This would become: > > > struct squashfs_stream { > > void *stream; > > + locallock_t lock; > > }; Wasn't aware as this is considered bloating. > ( The other departure from spinlocks is that the 'spinlock_t' name, > without underscores, while making the API names such as spin_lock() > with an underscore, was a conscious didactic choice. Applying that > principle to local locks gives us the spinlock_t-equivalent name of > 'locallock_t' - but the double 'l' reads a bit weirdly in this > context. So I think using 'local_lock_t' as the data structure is > probably the better approach. ) Okay, okay, I'm all yours. > Thanks, > > Ingo Sebastian
