Hi,
> +/*
> + * The following two macros are aimed to simplify buffer state modification
> + * in CAS loop. It's assumed that variable "uint32 state" is defined outside
> + * of this loop. It should be used as following:
> + *
> + * BEGIN_BUFSTATE_CAS_LOOP(bufHdr);
> + * modifications of state variable;
> + * END_BUFSTATE_CAS_LOOP(bufHdr);
> + *
> + * For local buffers, these macros shouldn't be used.. Since there is
> + * no cuncurrency, local buffer state could be chaged directly by atomic
> + * read/write operations.
> + */
> +#define BEGIN_BUFSTATE_CAS_LOOP(bufHdr) \
> + do { \
> + SpinDelayStatus delayStatus =
> init_spin_delay((Pointer)(bufHdr)); \
> + uint32 oldstate; \
> + oldstate = pg_atomic_read_u32(&(bufHdr)->state); \
> + for (;;) { \
> + while (oldstate & BM_LOCKED) \
> + { \
> + make_spin_delay(&delayStatus); \
> + oldstate =
> pg_atomic_read_u32(&(bufHdr)->state); \
> + } \
> + state = oldstate
> +
> +#define END_BUFSTATE_CAS_LOOP(bufHdr) \
> + if (pg_atomic_compare_exchange_u32(&bufHdr->state,
> &oldstate, state)) \
> + break; \
> + } \
> + finish_spin_delay(&delayStatus); \
> + } while (0)
Hm. Not sure if that's not too much magic. Will think about it.
> /*
> + * Lock buffer header - set BM_LOCKED in buffer state.
> + */
> + uint32
> + LockBufHdr(BufferDesc *desc)
> + {
> + uint32 state;
> +
> + BEGIN_BUFSTATE_CAS_LOOP(desc);
> + state |= BM_LOCKED;
> + END_BUFSTATE_CAS_LOOP(desc);
> +
> + return state;
> + }
> +
Hm. It seems a bit over the top to do the full round here. How about
uint32 oldstate;
while (true)
{
oldstate = pg_atomic_fetch_or(..., BM_LOCKED);
if (!(oldstate & BM_LOCKED)
break;
perform_spin_delay();
}
return oldstate | BM_LOCKED;
Especially if we implement atomic xor on x86, that'd likely be a good
bit more efficient.
> typedef struct BufferDesc
> {
> BufferTag tag; /* ID of page contained in
> buffer */
> - BufFlags flags; /* see bit definitions above */
> - uint8 usage_count; /* usage counter for clock sweep code */
> - slock_t buf_hdr_lock; /* protects a subset of fields, see
> above */
> - unsigned refcount; /* # of backends holding pins
> on buffer */
> - int wait_backend_pid; /* backend PID
> of pin-count waiter */
>
> + /* state of the tag, containing flags, refcount and usagecount */
> + pg_atomic_uint32 state;
> +
> + int wait_backend_pid; /* backend PID
> of pin-count waiter */
> int buf_id; /* buffer's index
> number (from 0) */
> int freeNext; /* link in freelist
> chain */
Hm. Won't matter on most platforms, but it seems like a good idea to
move to an 8 byte aligned boundary. Move buf_id up?
> +/*
> + * Support for spin delay which could be useful in other places where
> + * spinlock-like procedures take place.
> + */
> +typedef struct
> +{
> + int spins;
> + int delays;
> + int cur_delay;
> + Pointer ptr;
> + const char *file;
> + int line;
> +} SpinDelayStatus;
> +
> +#define init_spin_delay(ptr) {0, 0, 0, (ptr), __FILE__, __LINE__}
> +
> +void make_spin_delay(SpinDelayStatus *status);
> +void finish_spin_delay(SpinDelayStatus *status);
> +
> #endif /* S_LOCK_H */
s/make_spin_delay/perform_spin_delay/? The former sounds like it's
allocating something or such.
Regards,
Andres
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
