Remove the `owner` field in bakery_lock_t data structure
authorSoby Mathew <soby.mathew@arm.com>
Fri, 20 Feb 2015 16:04:17 +0000 (16:04 +0000)
committerSoby Mathew <soby.mathew@arm.com>
Fri, 27 Mar 2015 10:20:32 +0000 (10:20 +0000)
This patch removes the `owner` field in bakery_lock_t structure which
is the data structure used in the bakery lock implementation that uses
coherent memory. The assertions to protect against recursive lock
acquisition were based on the 'owner' field. They are now done based
on the bakery lock ticket number. These assertions are also added
to the bakery lock implementation that uses normal memory as well.

Change-Id: If4850a00dffd3977e218c0f0a8d145808f36b470

include/lib/bakery_lock.h
lib/locks/bakery/bakery_lock_coherent.c
lib/locks/bakery/bakery_lock_normal.c

index eaefecd9e2334b0c56460c8de269a10194b2bea2..2e1afa21f3b694c1b217ecd9697e6aa7eddb540f 100644 (file)
@@ -58,7 +58,6 @@
 #if USE_COHERENT_MEM
 
 typedef struct bakery_lock {
-       int owner;
        /*
         * The lock_data is a bit-field of 2 members:
         * Bit[0]       : choosing. This field is set when the CPU is
@@ -68,8 +67,6 @@ typedef struct bakery_lock {
        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);
index 02d31d6463999a0fd6be6e63d7d22d07200e5828..fd871053a7a077227ad01de717c9a81ca3320229 100644 (file)
        assert(entry < BAKERY_LOCK_MAX_CPUS);           \
 } while (0)
 
-/* 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;
 }
 
 
@@ -80,6 +79,9 @@ static unsigned int bakery_get_ticket(bakery_lock_t *bakery, unsigned int me)
        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
@@ -130,9 +132,6 @@ void bakery_lock_get(bakery_lock_t *bakery)
 
        assert_bakery_entry_valid(me, bakery);
 
-       /* Prevent recursive acquisition */
-       assert(bakery->owner != me);
-
        /* Get a ticket */
        my_ticket = bakery_get_ticket(bakery, me);
 
@@ -168,9 +167,7 @@ void bakery_lock_get(bakery_lock_t *bakery)
                                bakery_ticket_number(bakery->lock_data[they]));
                }
        }
-
        /* Lock acquired */
-       bakery->owner = me;
 }
 
 
@@ -180,13 +177,12 @@ void bakery_lock_release(bakery_lock_t *bakery)
        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->lock_data[me] = 0;
        dsb();
        sev();
index 4503ae09740c2d93bdfa0561a8ea4175a5bb4ece..5439271e97e9279de0d3732296741d84be96cde7 100644 (file)
@@ -87,6 +87,13 @@ static unsigned int bakery_get_ticket(int id, unsigned int offset,
        my_bakery_info = get_my_bakery_info(offset, id);
        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.
@@ -189,6 +196,7 @@ void bakery_lock_get(unsigned int id, unsigned int offset)
                                == bakery_ticket_number(their_bakery_info->lock_data));
                }
        }
+       /* Lock acquired */
 }
 
 void bakery_lock_release(unsigned int id, unsigned int offset)
@@ -197,6 +205,8 @@ void bakery_lock_release(unsigned int id, unsigned int offset)
        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();