| Message ID | 20231013094224.7060-3-jgross@suse.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xen/spinlock: make recursive spinlocks a dedicated type | expand |
On 13.10.2023 11:42, Juergen Gross wrote: > With some small adjustments to the LOCK_PROFILE_* macros some #ifdefs > can be dropped from spinlock.c. > > Signed-off-by: Juergen Gross <jgross@suse.com> > --- > V2: > - new patch > --- > xen/common/spinlock.c | 45 ++++++++++++++++++------------------------- > 1 file changed, 19 insertions(+), 26 deletions(-) > > diff --git a/xen/common/spinlock.c b/xen/common/spinlock.c > index 202c707540..4878a01302 100644 > --- a/xen/common/spinlock.c > +++ b/xen/common/spinlock.c > @@ -267,25 +267,28 @@ void spin_debug_disable(void) > lock->profile->time_hold += NOW() - lock->profile->time_locked; \ > lock->profile->lock_cnt++; \ > } > -#define LOCK_PROFILE_VAR s_time_t block = 0 > -#define LOCK_PROFILE_BLOCK block = block ? : NOW(); > -#define LOCK_PROFILE_GOT \ > +#define LOCK_PROFILE_VAR(val) s_time_t block = (val) This macro, and then at least for consistency also ... > +#define LOCK_PROFILE_BLOCK block = block ? : NOW() ... this one should imo take the variable name as an argument. Otherwise situations like ... > void _spin_barrier(spinlock_t *lock) > { > spinlock_tickets_t sample; > -#ifdef CONFIG_DEBUG_LOCK_PROFILE > - s_time_t block = NOW(); > -#endif > + LOCK_PROFILE_VAR(NOW()); > > check_barrier(&lock->debug); > smp_mb(); > @@ -432,13 +431,7 @@ void _spin_barrier(spinlock_t *lock) > { > while ( observe_head(&lock->tickets) == sample.head ) > arch_lock_relax(); > -#ifdef CONFIG_DEBUG_LOCK_PROFILE > - if ( lock->profile ) > - { > - lock->profile->time_block += NOW() - block; > - lock->profile->block_cnt++; > - } > -#endif > + LOCK_PROFILE_BLKACC(lock->profile, block); > } > smp_mb(); > } ... this arise where there's no visible declaration of "block", but a use. (Originally I was meaning to ask how this function would build, when the declaration is dropped.) Jan
On 18.10.23 17:57, Jan Beulich wrote: > On 13.10.2023 11:42, Juergen Gross wrote: >> With some small adjustments to the LOCK_PROFILE_* macros some #ifdefs >> can be dropped from spinlock.c. >> >> Signed-off-by: Juergen Gross <jgross@suse.com> >> --- >> V2: >> - new patch >> --- >> xen/common/spinlock.c | 45 ++++++++++++++++++------------------------- >> 1 file changed, 19 insertions(+), 26 deletions(-) >> >> diff --git a/xen/common/spinlock.c b/xen/common/spinlock.c >> index 202c707540..4878a01302 100644 >> --- a/xen/common/spinlock.c >> +++ b/xen/common/spinlock.c >> @@ -267,25 +267,28 @@ void spin_debug_disable(void) >> lock->profile->time_hold += NOW() - lock->profile->time_locked; \ >> lock->profile->lock_cnt++; \ >> } >> -#define LOCK_PROFILE_VAR s_time_t block = 0 >> -#define LOCK_PROFILE_BLOCK block = block ? : NOW(); >> -#define LOCK_PROFILE_GOT \ >> +#define LOCK_PROFILE_VAR(val) s_time_t block = (val) > > This macro, and then at least for consistency also ... > >> +#define LOCK_PROFILE_BLOCK block = block ? : NOW() > > ... this one should imo take the variable name as an argument. Otherwise > situations like ... > >> void _spin_barrier(spinlock_t *lock) >> { >> spinlock_tickets_t sample; >> -#ifdef CONFIG_DEBUG_LOCK_PROFILE >> - s_time_t block = NOW(); >> -#endif >> + LOCK_PROFILE_VAR(NOW()); >> >> check_barrier(&lock->debug); >> smp_mb(); >> @@ -432,13 +431,7 @@ void _spin_barrier(spinlock_t *lock) >> { >> while ( observe_head(&lock->tickets) == sample.head ) >> arch_lock_relax(); >> -#ifdef CONFIG_DEBUG_LOCK_PROFILE >> - if ( lock->profile ) >> - { >> - lock->profile->time_block += NOW() - block; >> - lock->profile->block_cnt++; >> - } >> -#endif >> + LOCK_PROFILE_BLKACC(lock->profile, block); >> } >> smp_mb(); >> } > > ... this arise where there's no visible declaration of "block", but a > use. (Originally I was meaning to ask how this function would build, > when the declaration is dropped.) Okay. Juergen
diff --git a/xen/common/spinlock.c b/xen/common/spinlock.c index 202c707540..4878a01302 100644 --- a/xen/common/spinlock.c +++ b/xen/common/spinlock.c @@ -267,25 +267,28 @@ void spin_debug_disable(void) lock->profile->time_hold += NOW() - lock->profile->time_locked; \ lock->profile->lock_cnt++; \ } -#define LOCK_PROFILE_VAR s_time_t block = 0 -#define LOCK_PROFILE_BLOCK block = block ? : NOW(); -#define LOCK_PROFILE_GOT \ +#define LOCK_PROFILE_VAR(val) s_time_t block = (val) +#define LOCK_PROFILE_BLOCK block = block ? : NOW() +#define LOCK_PROFILE_BLKACC(tst, val) \ + if ( tst ) \ + { \ + lock->profile->time_block += lock->profile->time_locked - (val); \ + lock->profile->block_cnt++; \ + } +#define LOCK_PROFILE_GOT(val) \ if ( lock->profile ) \ { \ lock->profile->time_locked = NOW(); \ - if ( block ) \ - { \ - lock->profile->time_block += lock->profile->time_locked - block; \ - lock->profile->block_cnt++; \ - } \ + LOCK_PROFILE_BLKACC(val, val); \ } #else #define LOCK_PROFILE_REL -#define LOCK_PROFILE_VAR +#define LOCK_PROFILE_VAR(val) #define LOCK_PROFILE_BLOCK -#define LOCK_PROFILE_GOT +#define LOCK_PROFILE_BLKACC(tst, val) +#define LOCK_PROFILE_GOT(val) #endif @@ -308,7 +311,7 @@ static void always_inline spin_lock_common(spinlock_t *lock, void (*cb)(void *), void *data) { spinlock_tickets_t tickets = SPINLOCK_TICKET_INC; - LOCK_PROFILE_VAR; + LOCK_PROFILE_VAR(0); check_lock(&lock->debug, false); preempt_disable(); @@ -323,7 +326,7 @@ static void always_inline spin_lock_common(spinlock_t *lock, } arch_lock_acquire_barrier(); got_lock(&lock->debug); - LOCK_PROFILE_GOT; + LOCK_PROFILE_GOT(block); } void _spin_lock(spinlock_t *lock) @@ -411,19 +414,15 @@ int _spin_trylock(spinlock_t *lock) * arch_lock_acquire_barrier(). */ got_lock(&lock->debug); -#ifdef CONFIG_DEBUG_LOCK_PROFILE - if ( lock->profile ) - lock->profile->time_locked = NOW(); -#endif + LOCK_PROFILE_GOT(0); + return 1; } void _spin_barrier(spinlock_t *lock) { spinlock_tickets_t sample; -#ifdef CONFIG_DEBUG_LOCK_PROFILE - s_time_t block = NOW(); -#endif + LOCK_PROFILE_VAR(NOW()); check_barrier(&lock->debug); smp_mb(); @@ -432,13 +431,7 @@ void _spin_barrier(spinlock_t *lock) { while ( observe_head(&lock->tickets) == sample.head ) arch_lock_relax(); -#ifdef CONFIG_DEBUG_LOCK_PROFILE - if ( lock->profile ) - { - lock->profile->time_block += NOW() - block; - lock->profile->block_cnt++; - } -#endif + LOCK_PROFILE_BLKACC(lock->profile, block); } smp_mb(); }
With some small adjustments to the LOCK_PROFILE_* macros some #ifdefs can be dropped from spinlock.c. Signed-off-by: Juergen Gross <jgross@suse.com> --- V2: - new patch --- xen/common/spinlock.c | 45 ++++++++++++++++++------------------------- 1 file changed, 19 insertions(+), 26 deletions(-)