Percona Server with XtraDB

Redundant wakeups in priority rw_lock_s_unlock_func()

Reported by Laurynas Biveinis on 2013-10-15
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Percona Server
Status tracked in 5.6
5.1
Undecided
Unassigned
5.5
Undecided
Unassigned
5.6
Medium
Laurynas Biveinis

Bug Description

The current priority rw_lock_s_unlock_func() implementation has some design misunderstanding in it:

 if (lock_word == 0) {

  /* A waiting next-writer exists, either high priority or
  regular. Wake up the first waiter in this order: 1) high
  priority next-writer; 2) high priority X waiters; 3) high
  priority S waiters; 4) regular priority next-waiter. This
  allows high priority requests to overtake an already-waiting
  regular priority next-waiter. */
  if (lock->high_priority_wait_ex_waiter) {

   lock->high_priority_wait_ex_waiter = 0;
   /* Note that we do not have a separate high priority
   next-waiter event. There can be only one such waiter,
   here we already know it's high priority, no
   regular-priority wakeup may happen. */
   os_event_set(lock->base_lock.wait_ex_event);
  } else if (lock->high_priority_x_waiters) {

   os_event_set(lock->high_priority_x_event);
  } else if (lock->high_priority_s_waiters) {

   os_event_set(lock->high_priority_s_event);
  } else {

   os_event_set(lock->base_lock.wait_ex_event);
  }
  sync_array_object_signalled();

It is impossible with the current implementation, contrary to what the 1st comment says, for a high-priority X or S waiter, to overtake a low-priority next-writer, as the current S unlock already results in X-locked lock word for the next-waiter, whatever its priority is. Thus the high_priority_x_event and high_priority_s_event wakeups are redundant, although benign.

Related branches

lp:~laurynas-biveinis/percona-server/bug1236696
Merged into lp:percona-server at revision 483
Alexey Kopytov: Approve on 2013-10-22
Vlad Lesin: Pending (g2) requested 2013-10-15
Laurynas Biveinis: Pending requested 2013-10-15
tags: added: xtradb
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers