Merge pull request #277 from soby-mathew/sm/coh_lock_opt
Optimize the bakery lock implementation
diff --git a/include/lib/bakery_lock.h b/include/lib/bakery_lock.h
index 9736f85..2e1afa2 100644
--- a/include/lib/bakery_lock.h
+++ b/include/lib/bakery_lock.h
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2013-2014, ARM Limited and Contributors. All rights reserved.
+ * Copyright (c) 2013-2015, ARM Limited and Contributors. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions are met:
@@ -38,16 +38,35 @@
#ifndef __ASSEMBLY__
#include <stdint.h>
+/*****************************************************************************
+ * Internal helper macros used by the bakery lock implementation.
+ ****************************************************************************/
+/* Convert a ticket to priority */
+#define PRIORITY(t, pos) (((t) << 8) | (pos))
+
+#define CHOOSING_TICKET 0x1
+#define CHOSEN_TICKET 0x0
+
+#define bakery_is_choosing(info) (info & 0x1)
+#define bakery_ticket_number(info) ((info >> 1) & 0x7FFF)
+#define make_bakery_data(choosing, number) \
+ (((choosing & 0x1) | (number << 1)) & 0xFFFF)
+
+/*****************************************************************************
+ * External bakery lock interface.
+ ****************************************************************************/
#if USE_COHERENT_MEM
typedef struct bakery_lock {
- int owner;
- volatile char entering[BAKERY_LOCK_MAX_CPUS];
- volatile unsigned number[BAKERY_LOCK_MAX_CPUS];
+ /*
+ * The lock_data is a bit-field of 2 members:
+ * Bit[0] : choosing. This field is set when the CPU is
+ * choosing its bakery number.
+ * Bits[1 - 15] : number. This is the bakery number allocated.
+ */
+ volatile uint16_t lock_data[BAKERY_LOCK_MAX_CPUS];
} bakery_lock_t;
-#define NO_OWNER (-1)
-
void bakery_lock_init(bakery_lock_t *bakery);
void bakery_lock_get(bakery_lock_t *bakery);
void bakery_lock_release(bakery_lock_t *bakery);
diff --git a/lib/locks/bakery/bakery_lock_coherent.c b/lib/locks/bakery/bakery_lock_coherent.c
index 5d538ce..fd87105 100644
--- a/lib/locks/bakery/bakery_lock_coherent.c
+++ b/lib/locks/bakery/bakery_lock_coherent.c
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2013-2014, ARM Limited and Contributors. All rights reserved.
+ * Copyright (c) 2013-2015, ARM Limited and Contributors. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions are met:
@@ -63,18 +63,13 @@
assert(entry < BAKERY_LOCK_MAX_CPUS); \
} while (0)
-/* Convert a ticket to priority */
-#define PRIORITY(t, pos) (((t) << 8) | (pos))
-
-
-/* Initialize Bakery Lock to reset ownership and all ticket values */
+/* Initialize Bakery Lock to reset all ticket values */
void bakery_lock_init(bakery_lock_t *bakery)
{
assert(bakery);
/* All ticket values need to be 0 */
memset(bakery, 0, sizeof(*bakery));
- bakery->owner = NO_OWNER;
}
@@ -84,6 +79,9 @@
unsigned int my_ticket, their_ticket;
unsigned int they;
+ /* Prevent recursive acquisition */
+ assert(!bakery_ticket_number(bakery->lock_data[me]));
+
/*
* Flag that we're busy getting our ticket. All CPUs are iterated in the
* order of their ordinal position to decide the maximum ticket value
@@ -95,9 +93,9 @@
* value, not the ticket value alone.
*/
my_ticket = 0;
- bakery->entering[me] = 1;
+ bakery->lock_data[me] = make_bakery_data(CHOOSING_TICKET, my_ticket);
for (they = 0; they < BAKERY_LOCK_MAX_CPUS; they++) {
- their_ticket = bakery->number[they];
+ their_ticket = bakery_ticket_number(bakery->lock_data[they]);
if (their_ticket > my_ticket)
my_ticket = their_ticket;
}
@@ -107,8 +105,7 @@
* finish calculating our ticket value that we're done
*/
++my_ticket;
- bakery->number[me] = my_ticket;
- bakery->entering[me] = 0;
+ bakery->lock_data[me] = make_bakery_data(CHOSEN_TICKET, my_ticket);
return my_ticket;
}
@@ -129,14 +126,12 @@
{
unsigned int they, me;
unsigned int my_ticket, my_prio, their_ticket;
+ unsigned int their_bakery_data;
me = platform_get_core_pos(read_mpidr_el1());
assert_bakery_entry_valid(me, bakery);
- /* Prevent recursive acquisition */
- assert(bakery->owner != me);
-
/* Get a ticket */
my_ticket = bakery_get_ticket(bakery, me);
@@ -150,14 +145,15 @@
continue;
/* Wait for the contender to get their ticket */
- while (bakery->entering[they])
- ;
+ do {
+ their_bakery_data = bakery->lock_data[they];
+ } while (bakery_is_choosing(their_bakery_data));
/*
* If the other party is a contender, they'll have non-zero
* (valid) ticket value. If they do, compare priorities
*/
- their_ticket = bakery->number[they];
+ their_ticket = bakery_ticket_number(their_bakery_data);
if (their_ticket && (PRIORITY(their_ticket, they) < my_prio)) {
/*
* They have higher priority (lower value). Wait for
@@ -167,12 +163,11 @@
*/
do {
wfe();
- } while (their_ticket == bakery->number[they]);
+ } while (their_ticket ==
+ bakery_ticket_number(bakery->lock_data[they]));
}
}
-
/* Lock acquired */
- bakery->owner = me;
}
@@ -182,14 +177,13 @@
unsigned int me = platform_get_core_pos(read_mpidr_el1());
assert_bakery_entry_valid(me, bakery);
- assert(bakery->owner == me);
+ assert(bakery_ticket_number(bakery->lock_data[me]));
/*
- * Release lock by resetting ownership and ticket. Then signal other
+ * Release lock by resetting ticket. Then signal other
* waiting contenders
*/
- bakery->owner = NO_OWNER;
- bakery->number[me] = 0;
+ bakery->lock_data[me] = 0;
dsb();
sev();
}
diff --git a/lib/locks/bakery/bakery_lock_normal.c b/lib/locks/bakery/bakery_lock_normal.c
index a325fd4..5439271 100644
--- a/lib/locks/bakery/bakery_lock_normal.c
+++ b/lib/locks/bakery/bakery_lock_normal.c
@@ -56,17 +56,6 @@
* accesses regardless of status of address translation.
*/
-/* Convert a ticket to priority */
-#define PRIORITY(t, pos) (((t) << 8) | (pos))
-
-#define CHOOSING_TICKET 0x1
-#define CHOOSING_DONE 0x0
-
-#define bakery_is_choosing(info) (info & 0x1)
-#define bakery_ticket_number(info) ((info >> 1) & 0x7FFF)
-#define make_bakery_data(choosing, number) \
- (((choosing & 0x1) | (number << 1)) & 0xFFFF)
-
/* This macro assumes that the bakery_info array is located at the offset specified */
#define get_my_bakery_info(offset, id) \
(((bakery_info_t *) (((uint8_t *)_cpu_data()) + offset)) + id)
@@ -99,6 +88,13 @@
assert(my_bakery_info);
/*
+ * Prevent recursive acquisition.
+ * Since lock data is written to and cleaned by the owning cpu, it
+ * doesn't require any cache operations prior to reading the lock data.
+ */
+ assert(!bakery_ticket_number(my_bakery_info->lock_data));
+
+ /*
* Tell other contenders that we are through the bakery doorway i.e.
* going to allocate a ticket for this cpu.
*/
@@ -138,7 +134,7 @@
* finish calculating our ticket value that we're done
*/
++my_ticket;
- my_bakery_info->lock_data = make_bakery_data(CHOOSING_DONE, my_ticket);
+ my_bakery_info->lock_data = make_bakery_data(CHOSEN_TICKET, my_ticket);
write_cache_op(my_bakery_info, is_cached);
@@ -150,7 +146,7 @@
unsigned int they, me, is_cached;
unsigned int my_ticket, my_prio, their_ticket;
bakery_info_t *their_bakery_info;
- uint16_t their_bakery_data;
+ unsigned int their_bakery_data;
me = platform_get_core_pos(read_mpidr_el1());
@@ -174,15 +170,12 @@
*/
their_bakery_info = get_bakery_info_by_index(offset, id, they);
assert(their_bakery_info);
- read_cache_op(their_bakery_info, is_cached);
-
- their_bakery_data = their_bakery_info->lock_data;
/* Wait for the contender to get their ticket */
- while (bakery_is_choosing(their_bakery_data)) {
+ do {
read_cache_op(their_bakery_info, is_cached);
their_bakery_data = their_bakery_info->lock_data;
- }
+ } while (bakery_is_choosing(their_bakery_data));
/*
* If the other party is a contender, they'll have non-zero
@@ -203,6 +196,7 @@
== bakery_ticket_number(their_bakery_info->lock_data));
}
}
+ /* Lock acquired */
}
void bakery_lock_release(unsigned int id, unsigned int offset)
@@ -211,6 +205,8 @@
unsigned int is_cached = read_sctlr_el3() & SCTLR_C_BIT;
my_bakery_info = get_my_bakery_info(offset, id);
+ assert(bakery_ticket_number(my_bakery_info->lock_data));
+
my_bakery_info->lock_data = 0;
write_cache_op(my_bakery_info, is_cached);
sev();