Messages in this thread | | | Date | Fri, 30 May 2014 14:23:05 +0530 | From | Raghavendra K T <> | Subject | Re: [RFC] Implement Batched (group) ticket lock |
| |
On 05/30/2014 04:15 AM, Waiman Long wrote: > On 05/28/2014 08:16 AM, Raghavendra K T wrote: >> - we need an intelligent way to nullify the effect of batching for >> baremetal >> (because extra cmpxchg is not required). > > To do this, you will need to have 2 slightly different algorithms > depending on the paravirt_ticketlocks_enabled jump label.
Thanks for the hint Waiman.
[...] >> +spin: >> + for (;;) { >> + inc.head = ACCESS_ONCE(lock->tickets.head); >> + if (!(inc.head& TICKET_LOCK_HEAD_INC)) { >> + new.head = inc.head | TICKET_LOCK_HEAD_INC; >> + if (cmpxchg(&lock->tickets.head, inc.head, new.head) >> + == inc.head) >> + goto out; >> + } >> + cpu_relax(); >> + } >> + > > It had taken me some time to figure out the the LSB of inc.head is used > as a bit lock for the contending tasks in the spin loop. I would suggest > adding some comment here to make it easier to look at.
Agree. 'll add a comment.
[...] >> +#define TICKET_BATCH 0x4 /* 4 waiters can contend simultaneously */ >> +#define TICKET_LOCK_BATCH_MASK >> (~(TICKET_BATCH<<TICKET_LOCK_INC_SHIFT) + \ >> + TICKET_LOCK_TAIL_INC - 1) > > I don't think TAIL_INC has anything to do with setting the BATCH_MASK. > It works here because TAIL_INC is 2. I think it is clearer to define it > as either "(~(TICKET_BATCH<<TICKET_LOCK_INC_SHIFT) + 1)" or > (~((TICKET_BATCH<<TICKET_LOCK_INC_SHIFT) - 1)).
You are right. Thanks for pointing out. Your expression is simple and clearer. 'll use one of them.
| |