some observations (and some questions) about the logistics of
finally removing the remnants of the SPIN_LOCK_UNLOCKED macro (all of
which should apply equally well to RW_LOCK_UNLOCKED, so i'll deal with
only the first case here).
as you can read in Documentation/spinlocks.txt:
"SPIN_LOCK_UNLOCKED and RW_LOCK_UNLOCKED defeat lockdep state tracking
and are hence deprecated.
Please use DEFINE_SPINLOCK()/DEFINE_RWLOCK() or
__SPIN_LOCK_UNLOCKED()/__RW_LOCK_UNLOCKED() as appropriate for static
initialization."
fair enough, but what possibilities are there to consider?
*** One ***
first, and trivially, anything that looks like this (from
arch/arm/kernel/irq.c):
static struct irq_desc bad_irq_desc = {
.handle_irq = handle_bad_irq,
.lock = SPIN_LOCK_UNLOCKED
};
should be replaceable with:
static struct irq_desc bad_irq_desc = {
.handle_irq = handle_bad_irq,
.lock = __SPIN_LOCK_UNLOCKED(bad_irq_desc.lock)
};
it really should be that simple, correct? since all you're doing is
supplying a name for debugging purposes. in fact, at the moment, you
can see in include/linux/spinlock_types.h:
#define SPIN_LOCK_UNLOCKED __SPIN_LOCK_UNLOCKED(old_style_spin_init)
#define RW_LOCK_UNLOCKED __RW_LOCK_UNLOCKED(old_style_rw_init)
in other words, the deprecated macro is defined to simply use the
newer macro with an uninformative name, so the simple change above
should be all that's required in those circumstances, yes? moving on.
*** Two ***
in some cases, you can't make that trivial replacement since you're
dealing with an *array* of locks, as in (from
arch/sparc/lib/atomic32.c):
spinlock_t __atomic_hash[ATOMIC_HASH_SIZE] = {
[0 ... (ATOMIC_HASH_SIZE-1)] = SPIN_LOCK_UNLOCKED
};
to the best of my knowledge, there's no way to give each of those
locks a unique name for debugging, but there are two other
possibilities:
1) use a __RAW_SPIN_LOCK_UNLOCKED for all of those locks, which will
work but won't support any debugging, or
2) let those locks all have the same name for debugging purposes:
spinlock_t __atomic_hash[ATOMIC_HASH_SIZE] = {
[0 ... (ATOMIC_HASH_SIZE-1)] = __SPIN_LOCK_UNLOCKED(__atomic_hash.lock)
};
AFAIK, this second alternative is legal, right? since the lock
name is strictly for informational purposes, you can use any name
you want. the above will simply not be as specific as you want,
but it will still work, yes? or am i misunderstanding something
important here?
so ... do both of the above techniques work? and which one would
be preferred?
*** Three ***
next, i'm not sure what to do with locks defined in the context
of PER_CPU structures, like this from arch/arm/kernel/smp.c:
static DEFINE_PER_CPU(struct ipi_data, ipi_data) = {
.lock = SPIN_LOCK_UNLOCKED,
};
is it still valid to just change that to:
static DEFINE_PER_CPU(struct ipi_data, ipi_data) = {
.lock = __SPIN_LOCK_UNLOCKED(ipi_data.lock),
};
*** Four ***
in a number of rwsem.h header files, you have this macro definition:
#define __RWSEM_INITIALIZER(name) \
{ RWSEM_UNLOCKED_VALUE, SPIN_LOCK_UNLOCKED, \
LIST_HEAD_INIT((name).wait_list) \
__RWSEM_DEP_MAP_INIT(name) }
superficially, it looks like you can replace that with:
#define __RWSEM_INITIALIZER(name) \
{ RWSEM_UNLOCKED_VALUE, __SPIN_LOCK_UNLOCKED((name).lock), \
LIST_HEAD_INIT((name).wait_list) \
__RWSEM_DEP_MAP_INIT(name) }
does that look reasonable? it seems to.
*** Five ***
and, finally, take advantage of the newer macro:
#define DEFINE_SPINLOCK(x) spinlock_t x = __SPIN_LOCK_UNLOCKED(x)
to replace code like:
spinlock_t init_lock = SPIN_LOCK_UNLOCKED;
with the improved:
DEFINE_SPINLOCK(init_lock);
have i missed any possible transformations, or misunderstood any of
them? thanks.
rday
--
========================================================================
Robert P. J. Day
Linux Consulting, Training and Annoying Kernel Pedantry:
Have classroom, will lecture.
http://crashcourse.ca Waterloo, Ontario, CANADA
========================================================================
--
To unsubscribe from this list: send an email with
"unsubscribe kernelnewbies" to ecartis@xxxxxxxxxxxx
Please read the FAQ at http://kernelnewbies.org/FAQ
