From fb5fdb17dbe5a689d8d277e2eb4802c0553789a8 Mon Sep 17 00:00:00 2001 From: counterpoint Date: Fri, 8 Jan 2016 12:19:29 +0000 Subject: [PATCH] Switch bitmask_clear to be locking and offer bitmask_clear_without_spinlock for non-locking version, in response to review comments. Revert poll.c to use bitmask_clear (with locking) and amend dcb.c to use the non-locking version and to take advantage of the return of an indication of whether the whole bitmask is then clearn. --- server/core/dcb.c | 6 ++---- server/core/gwbitmask.c | 10 +++++----- server/core/poll.c | 2 +- server/include/gwbitmask.h | 2 +- 4 files changed, 9 insertions(+), 11 deletions(-) diff --git a/server/core/dcb.c b/server/core/dcb.c index 322a27163..ced6bc894 100644 --- a/server/core/dcb.c +++ b/server/core/dcb.c @@ -489,9 +489,7 @@ dcb_process_zombies(int threadid) else { - bitmask_clear(&zombiedcb->memdata.bitmask, threadid); - - if (bitmask_isallclear(&zombiedcb->memdata.bitmask)) + if (bitmask_clear_without_spinlock(&zombiedcb->memdata.bitmask)) { /** * Remove the DCB from the zombie queue @@ -1736,7 +1734,7 @@ dcb_drain_writeq_SSL(DCB *dcb) return n; } -/** +/** * Removes dcb from poll set, and adds it to zombies list. As a consequence, * dcb first moves to DCB_STATE_NOPOLLING, and then to DCB_STATE_ZOMBIE state. * At the end of the function state may not be DCB_STATE_ZOMBIE because once diff --git a/server/core/gwbitmask.c b/server/core/gwbitmask.c index 9ba51d347..ce49c8bd6 100644 --- a/server/core/gwbitmask.c +++ b/server/core/gwbitmask.c @@ -140,14 +140,14 @@ bitmask_set(GWBITMASK *bitmask, int bit) * Note that this function does not lock the bitmask, but assumes that * it is under the exclusive control of the caller. If you want to use the * bitmask spinlock to protect access while clearing the bit, then call - * the alternative bitmask_clear_with_lock. + * the alternative bitmask_clear. * * @param bitmask Pointer the bitmask * @param bit Bit to clear * @return int 1 if the bitmask is all clear after the operation, else 0. */ int -bitmask_clear(GWBITMASK *bitmask, int bit) +bitmask_clear_without_spinlock(GWBITMASK *bitmask, int bit) { unsigned char *ptr = bitmask->bits; int i; @@ -174,19 +174,19 @@ bitmask_clear(GWBITMASK *bitmask, int bit) /** * Clear the bit at the specified bit position in the bitmask using a spinlock. - * See bitmask_clear for more details + * See bitmask_clear_without_spinlock for more details * * @param bitmask Pointer the bitmask * @param bit Bit to clear * @return int 1 if the bitmask is all clear after the operation, else 0 */ int -bitmask_clear_with_lock(GWBITMASK *bitmask, int bit) +bitmask_clear(GWBITMASK *bitmask, int bit) { int result; spinlock_acquire(&bitmask->lock); - result = bitmask_clear(bitmask, bit); + result = bitmask_clear_without_spinlock(bitmask, bit); spinlock_release(&bitmask->lock); return result; } diff --git a/server/core/poll.c b/server/core/poll.c index 569ee6139..4155bc641 100644 --- a/server/core/poll.c +++ b/server/core/poll.c @@ -725,7 +725,7 @@ poll_waitevents(void *arg) { thread_data[thread_id].state = THREAD_STOPPED; } - bitmask_clear_with_lock(&poll_mask, thread_id); + bitmask_clear(&poll_mask, thread_id); /** Release mysql thread context */ mysql_thread_end(); return; diff --git a/server/include/gwbitmask.h b/server/include/gwbitmask.h index 71620f5fd..5df50114f 100644 --- a/server/include/gwbitmask.h +++ b/server/include/gwbitmask.h @@ -52,7 +52,7 @@ extern void bitmask_init(GWBITMASK *); extern void bitmask_free(GWBITMASK *); extern void bitmask_set(GWBITMASK *, int); extern int bitmask_clear(GWBITMASK *, int); -extern int bitmask_clear_with_lock(GWBITMASK *, int); +extern int bitmask_clear_without_spinlock(GWBITMASK *, int); extern int bitmask_isset(GWBITMASK *, int); extern int bitmask_isallclear(GWBITMASK *); extern void bitmask_copy(GWBITMASK *, GWBITMASK *);