Merge lp:~laurynas-biveinis/percona-server/bp-split-5.6 into lp:percona-server/5.6

[Laurynas Biveinis]

Repushed and resubmitted. The only change from the 3rd push is atomic ops split off to be handled later.

http://jenkins.percona.com/job/percona-server-5.6-param/259/

No BT or ST, but 5.6 GA prerequisite.

[Alexey Kopytov (akopytov)]

Hi Laurynas,

This is not a complete review, I'm only about 10% into the patch, but I'm posting the comments I have so far to parallelize things a bit:

  - the code in btr_blob_free() can be simplified. just initialize
    ‘freed’ with ‘false’, then assign to it the result of
    buf_LRU_free_page() whenever it is called, and then do this at the
    end:

    if (!freed) {
     mutex_exit(&buf_pool->LRU_list_mutex);
    }

    Would result in less hairy code.

  - wrong comments for buf_LRU_free_page(): s/block
    mutex/buf_page_get_mutex() mutex/

  - the comments for buf_LRU_free_page() say that both LRU_list_mutex
    and block_mutex may be released temporarily if ‘true’ is
    returned. But:
    1) even if ‘false’ is returned, block_mutex may also be released
       temporarily
    2) the comments don’t mention that if ‘true’ is returned,
       LRU_list_mutex is always released upon return, but block_mutex is
       always locked. And callers of buf_LRU_free_page() rely on that.

  - the following code in buf_LRU_free_page() is missing a
    buf_page_free_descriptor() call if b != NULL. Which is a potential
    memory leak.

  if (!buf_LRU_block_remove_hashed(bpage, zip)) {
+
+ mutex_exit(&buf_pool->LRU_list_mutex);
+
+ mutex_enter(block_mutex);
+
   return(true);
  }

  - the patch removes buf_pool_mutex_enter_all() from
    btr_search_validate_one_table(), but then does a number of dirty
    reads from ‘block’ before it locks block->mutex. Any reasons to not
    lock block->mutex earlier?

  - the following checks for mutex != NULL in buf_buddy_relocate() seem
    to be redundant, since they are made after mutex_enter(mutex), so we
    are guaranteed mutex != NULL if we reach that code:

@@ -584,7 +604,11 @@ buf_buddy_relocate(

  mutex_enter(mutex);

- if (buf_page_can_relocate(bpage)) {
+ rw_lock_s_unlock(hash_lock);
+
+ mutex_enter(&buf_pool->zip_free_mutex);
+
+ if (mutex && buf_page_can_relocate(bpage)) {

and

- mutex_exit(mutex);
+ if (mutex)
+ mutex_exit(mutex);
+
+ ut_ad(mutex_own(&buf_pool->zip_free_mutex));

    and the last hunk is also missing braces (in case you decide to keep
    it).

  - asserting that zip_free_mutex is locked also looks redundant to me,
    because it is locked just a few lines above, and there’s nothing in
    the code path that could release it.

  - os_atomic_load_ulint() / os_atomic_store_ulint()... I don’t think we
    need that stuff. Their names are misleading as they don’t enforce
    any atomicity. They should be named os_ordered_load_ulint() /
    os_ordered_store_ulint(), but... what specific order are you trying
    to enforce with those constructs?

  - I don’t see a point in maintaining multiple list nodes in buf_page_t
    (i.e. ‘free’, ‘flush_list’ and ‘zip_list’). As I understand, each
    page may only be in a single list at any point in time, so splitting
    the list node is purely cosmetic.

    On the other hand, we are looking at a non-trivial buf_page_t size
    increase (112 bytes before the patch, 144 bytes after). Leaving all
    cache and memory locality questions aside, that’s 64 MB of memory
    just for list node pointers on a system with a 32 GB buffer pool....

review: Needs Fixing

[Laurynas Biveinis (laurynas-biveinis)]

Alexey -

> - the code in btr_blob_free() can be simplified

Simplified.

> - wrong comments for buf_LRU_free_page(): s/block
> mutex/buf_page_get_mutex() mutex/

Edited. But there are numerous other places in this patch (and upstream) that would need this editing too, and "block mutex" is already an established shorthand for "really a block mutex or buf_pool->zip_mutex". Not to mention pointer to mutex variables named block_mutex.

Do you want me to edit the other places too?

> - the comments for buf_LRU_free_page() say that both LRU_list_mutex
> and block_mutex may be released temporarily if ‘true’ is
> returned. But:
> 1) even if ‘false’ is returned, block_mutex may also be released
> temporarily
> 2) the comments don’t mention that if ‘true’ is returned,
> LRU_list_mutex is always released upon return, but block_mutex is
> always locked. And callers of buf_LRU_free_page() rely on that.

Indeed callers rely on the current, arguably messy, buf_LRU_free_page() locking. This is how I edited the header comment for this and the previous review comment:

/******************************************************************//**
Try to free a block. If bpage is a descriptor of a compressed-only
page, the descriptor object will be freed as well.

NOTE: If this function returns true, it will release the LRU list mutex,
and temporarily release and relock the buf_page_get_mutex() mutex.
Furthermore, the page frame will no longer be accessible via bpage. If this
function returns false, the buf_page_get_get_mutex() might be temporarily
released and relocked too.

The caller must hold the LRU list and buf_page_get_mutex() mutexes.

@return true if freed, false otherwise. */

> - the following code in buf_LRU_free_page() is missing a
> buf_page_free_descriptor() call if b != NULL. Which is a potential
> memory leak.

Fixed.

> - the patch removes buf_pool_mutex_enter_all() from
> btr_search_validate_one_table(), but then does a number of dirty
> reads from ‘block’ before it locks block->mutex. Any reasons to not
> lock block->mutex earlier?

I *think* there were actual reasons, but I cannot remember them, due to the number of things going on with this patch. And I don't see why it locking block->mutex earlier is not possible now. I will look further.

> - the following checks for mutex != NULL in buf_buddy_relocate() seem
> to be redundant, since they are made after mutex_enter(mutex), so we
> are guaranteed mutex != NULL if we reach that code:

Fixed. This looks like a missed cleanup after removing 5.5's buf_page_get_mutex_enter().

> - asserting that zip_free_mutex is locked also looks redundant to me,
> because it is locked just a few lines above, and there’s nothing in
> the code path that could release it.

Removed. Added a comment to the function header "The caller must hold zip_free_mutex, and this
function will release and lock it again." instead.

> - os_atomic_load_ulint() / os_atomic_store_ulint()... I don’t think we
> need that stuff. Their names are misleading as they don’t enforce
> any atomicity.

The ops being load and store, their atom...

[Alexey Kopytov (akopytov)]

Hi Laurynas,

On Wed, 11 Sep 2013 09:45:13 -0000, Laurynas Biveinis wrote:
> Alexey -
>
>> - the code in btr_blob_free() can be simplified
>
> Simplified.
>
>> - wrong comments for buf_LRU_free_page(): s/block
>> mutex/buf_page_get_mutex() mutex/
>
> Edited. But there are numerous other places in this patch (and upstream) that would need this editing too, and "block mutex" is already an established shorthand for "really a block mutex or buf_pool->zip_mutex". Not to mention pointer to mutex variables named block_mutex.
>

I think editing existing comments is worth the efforts (and potential
extra maintenance cost in the future). I would be OK if this specific
comment was left intact too. It only caught my eye because the comment
was edited and I spent some time verifying it.

> Do you want me to edit the other places too?
>
>> - the comments for buf_LRU_free_page() say that both LRU_list_mutex
>> and block_mutex may be released temporarily if ‘true’ is
>> returned. But:
>> 1) even if ‘false’ is returned, block_mutex may also be released
>> temporarily
>> 2) the comments don’t mention that if ‘true’ is returned,
>> LRU_list_mutex is always released upon return, but block_mutex is
>> always locked. And callers of buf_LRU_free_page() rely on that.
>
> Indeed callers rely on the current, arguably messy, buf_LRU_free_page() locking. This is how I edited the header comment for this and the previous review comment:
>
> /******************************************************************//**
> Try to free a block. If bpage is a descriptor of a compressed-only
> page, the descriptor object will be freed as well.
>
> NOTE: If this function returns true, it will release the LRU list mutex,
> and temporarily release and relock the buf_page_get_mutex() mutex.
> Furthermore, the page frame will no longer be accessible via bpage. If this
> function returns false, the buf_page_get_get_mutex() might be temporarily
> released and relocked too.
>
> The caller must hold the LRU list and buf_page_get_mutex() mutexes.
>
> @return true if freed, false otherwise. */
>
>

Looks good.

>> - the patch removes buf_pool_mutex_enter_all() from
>> btr_search_validate_one_table(), but then does a number of dirty
>> reads from ‘block’ before it locks block->mutex. Any reasons to not
>> lock block->mutex earlier?
>
> I *think* there were actual reasons, but I cannot remember them, due to the number of things going on with this patch. And I don't see why it locking block->mutex earlier is not possible now. I will look further.
>

OK.

>> - os_atomic_load_ulint() / os_atomic_store_ulint()... I don’t think we
>> need that stuff. Their names are misleading as they don’t enforce
>> any atomicity.
>
> The ops being load and store, their atomicity is enforced by the data type width.
>

Right, the atomicity is enforced by the data type width on those
architectures that provide it. And even those that do provide it have a
number of prerequisites. Neither of those 2 facts is taken care of in
os_atomic_load_ulint() / os_atomic_store_ulint(). So they are not any
different with respect to atomi...

[Alexey Kopytov (akopytov)]

On Wed, 11 Sep 2013 16:06:21 +0400, Alexey Kopytov wrote:
> I think editing existing comments is worth the efforts (and potential
> extra maintenance cost in the future). I would be OK if this specific

Grr, that was supposed to be "NOT worth the efforts" of course.

[Laurynas Biveinis (laurynas-biveinis)]

> > - the patch removes buf_pool_mutex_enter_all() from
> > btr_search_validate_one_table(), but then does a number of dirty
> > reads from ‘block’ before it locks block->mutex. Any reasons to not
> > lock block->mutex earlier?
>
> I *think* there were actual reasons, but I cannot remember them, due to the
> number of things going on with this patch. And I don't see why it locking
> block->mutex earlier is not possible now. I will look further.

A test run helped to recover my memories. So the problem is buf_block_hash_get() call, which locks page_hash, which violates locking order. Keeping the initial reads is OK, because 1) block pointer will not be invalidated because we are reading AHI-indexed pages only, which can be only BUF_BLOCK_FILE_PAGE or BUF_BLOCK_REMOVE_HASH (which is a kind of BUF_BLOCK_FILE_PAGE for our purposes), 2) block space and page_id cannot change neither while we hold a corresponding AHI X latch. Thus the initial dirty reads are not actually dirty.

[Alexey Kopytov (akopytov)]

Hi Laurynas,

On Wed, 11 Sep 2013 12:26:57 -0000, Laurynas Biveinis wrote:
>>> - the patch removes buf_pool_mutex_enter_all() from
>>> btr_search_validate_one_table(), but then does a number of dirty
>>> reads from ‘block’ before it locks block->mutex. Any reasons to not
>>> lock block->mutex earlier?
>>
>> I *think* there were actual reasons, but I cannot remember them, due to the
>> number of things going on with this patch. And I don't see why it locking
>> block->mutex earlier is not possible now. I will look further.
>
> A test run helped to recover my memories. So the problem is buf_block_hash_get() call, which locks page_hash, which violates locking order. Keeping the initial reads is OK, because 1) block pointer will not be invalidated because we are reading AHI-indexed pages only, which can be only BUF_BLOCK_FILE_PAGE or BUF_BLOCK_REMOVE_HASH (which is a kind of BUF_BLOCK_FILE_PAGE for our purposes), 2) block space and page_id cannot change neither while we hold a corresponding AHI X latch. Thus the initial dirty reads are not actually dirty.
>

Right, makes sense.

[Laurynas Biveinis (laurynas-biveinis)]

> >> - wrong comments for buf_LRU_free_page(): s/block
> >> mutex/buf_page_get_mutex() mutex/
> >
> > Edited. But there are numerous other places in this patch (and upstream)
> that would need this editing too, and "block mutex" is already an established
> shorthand for "really a block mutex or buf_pool->zip_mutex". Not to mention
> pointer to mutex variables named block_mutex.
> >
>
> I think editing existing comments is not worth the efforts (and potential
> extra maintenance cost in the future). I would be OK if this specific
> comment was left intact too. It only caught my eye because the comment
> was edited and I spent some time verifying it.

Fair enough. I applied same edit through the other comments changed by this patch.

> >> - os_atomic_load_ulint() / os_atomic_store_ulint()... I don’t think we
> >> need that stuff. Their names are misleading as they don’t enforce
> >> any atomicity.
> >
> > The ops being load and store, their atomicity is enforced by the data type
> width.
> >
>
> Right, the atomicity is enforced by the data type width on those
> architectures that provide it.

I forgot to mention that they also have to be not misaligned so that one access does not translate to two accesses.

> And even those that do provide it have a
> number of prerequisites. Neither of those 2 facts is taken care of in
> os_atomic_load_ulint() / os_atomic_store_ulint(). So they are not any
> different with respect to atomicity as plain load/store of a ulint and
> thus, have misleading names.
>
> So to justify "atomic" in their names those functions should:
>
> - (if we want to be portable) protect those load/stores with a mutex

Why? I guess this question boils down to, what would the mutex implementation code additionally ensure here, let's say, on x86_64? Or is this referring to the 5.6 mutex fallbacks when no atomic ops are implemented for a platform?

> - (if we only care about x86/x86_64) make sure that values being
> loaded/stored do not cross cache lines or page boundaries. Which is of
> course impossible to guarantee in a generic function.

Why? We are talking about ulints here only, and I was not able to find such requirements in the x86_64 memory model descriptions. There is a requirement to be aligned, and misaligned stores/loads might indeed cross cache line or page boundaries, and anything that crosses them is indeed non-atomic. But alignment is possible to guarantee in a generic function (which doesn't even has to be generic: the x86_64 implementation is for x86_64 only, obviously).

Intel® 64 and IA-32 Architectures Software Developer's Manual
Volume 3A: System Programming Guide, Part 1, section 8.1.1, http://download.intel.com/products/processor/manual/253668.pdf:

"The Intel486 processor (and newer processors since) guarantees that the following basic memory operations will
always be carried out atomically:
(...)
• Reading or writing a doubleword aligned on a 32-bit boundary

The Pentium processor (...):
• Reading or writing a quadword aligned on a 64-bit boundary
"

My understanding of the above is that os_atomic_load_ulint()/os_atomic_store_ulint() fit the above description, modulo alignment ...

[Alexey Kopytov (akopytov)]

Hi Laurynas,

On Wed, 11 Sep 2013 15:29:33 -0000, Laurynas Biveinis wrote:
>>>> - os_atomic_load_ulint() / os_atomic_store_ulint()... I don’t think we
>>>> need that stuff. Their names are misleading as they don’t enforce
>>>> any atomicity.
>>>
>>> The ops being load and store, their atomicity is enforced by the data type
>> width.
>>>
>>
>> Right, the atomicity is enforced by the data type width on those
>> architectures that provide it.
>
>
> I forgot to mention that they also have to be not misaligned so that one access does not translate to two accesses.
>

Yes, but alignment does not guarantee atomicity, see below.

>
>> And even those that do provide it have a
>> number of prerequisites. Neither of those 2 facts is taken care of in
>> os_atomic_load_ulint() / os_atomic_store_ulint(). So they are not any
>> different with respect to atomicity as plain load/store of a ulint and
>> thus, have misleading names.
>>
>> So to justify "atomic" in their names those functions should:
>>
>> - (if we want to be portable) protect those load/stores with a mutex
>
>
> Why? I guess this question boils down to, what would the mutex implementation code additionally ensure here, let's say, on x86_64? Or is this referring to the 5.6 mutex fallbacks when no atomic ops are implemented for a platform?
>

mutex the only portable way to ensure atomicity. You can use atomic
primitives provided by specific architectures, but then you either limit
support for those architectures or yes, provide a mutex fallback.

>
>> - (if we only care about x86/x86_64) make sure that values being
>> loaded/stored do not cross cache lines or page boundaries. Which is of
>> course impossible to guarantee in a generic function.
>
>
> Why? We are talking about ulints here only, and I was not able to find such requirements in the x86_64 memory model descriptions. There is a requirement to be aligned, and misaligned stores/loads might indeed cross cache line or page boundaries, and anything that crosses them is indeed non-atomic. But alignment is possible to guarantee in a generic function (which doesn't even has to be generic: the x86_64 implementation is for x86_64 only, obviously).
>
> Intel® 64 and IA-32 Architectures Software Developer's Manual
> Volume 3A: System Programming Guide, Part 1, section 8.1.1, http://download.intel.com/products/processor/manual/253668.pdf:
>
> "The Intel486 processor (and newer processors since) guarantees that the following basic memory operations will
> always be carried out atomically:
> (...)
> • Reading or writing a doubleword aligned on a 32-bit boundary
>
> The Pentium processor (...):
> • Reading or writing a quadword aligned on a 64-bit boundary
> "

Why didn't you quote it further?

"
Accesses to cacheable memory that are split across cache lines and page
boundaries are not guaranteed to be atomic by <all processors>. <all
processors> provide bus control signals that permit external memory
subsystems to make split accesses atomic;
"

Which means even aligned accesses are not guaranteed to be atomic and
it's up to the implementation of "external memory subsystems" (that
probably means chipsets, motherboards, NUMA archi...

[Laurynas Biveinis (laurynas-biveinis)]

Alexey -

> >> - (if we only care about x86/x86_64) make sure that values being
> >> loaded/stored do not cross cache lines or page boundaries. Which is of
> >> course impossible to guarantee in a generic function.
> >
> >
> > Why? We are talking about ulints here only, and I was not able to find such
> requirements in the x86_64 memory model descriptions. There is a requirement
> to be aligned, and misaligned stores/loads might indeed cross cache line or
> page boundaries, and anything that crosses them is indeed non-atomic. But
> alignment is possible to guarantee in a generic function (which doesn't even
> has to be generic: the x86_64 implementation is for x86_64 only, obviously).
> >
> > Intel® 64 and IA-32 Architectures Software Developer's Manual
> > Volume 3A: System Programming Guide, Part 1, section 8.1.1,
> http://download.intel.com/products/processor/manual/253668.pdf:
> >
> > "The Intel486 processor (and newer processors since) guarantees that the
> following basic memory operations will
> > always be carried out atomically:
> > (...)
> > • Reading or writing a doubleword aligned on a 32-bit boundary
> >
> > The Pentium processor (...):
> > • Reading or writing a quadword aligned on a 64-bit boundary
> > "
>
> Why didn't you quote it further?
>
> "
> Accesses to cacheable memory that are split across cache lines and page
> boundaries are not guaranteed to be atomic by <all processors>. <all
> processors> provide bus control signals that permit external memory
> subsystems to make split accesses atomic;
> "
>
> Which means even aligned accesses are not guaranteed to be atomic and
> it's up to the implementation of "external memory subsystems" (that
> probably means chipsets, motherboards, NUMA architectures and the like).

I didn't quote because we both have already acknowledged that cache line- or page boundary-crossing accesses are non-atomic, and because I don't see how it's relevant here. I don't see how a properly-aligned ulint can possibly cross a cache line boundary, when cache lines are 64-byte wide and 64-byte aligned. Or even 32 for older architectures.

> > My understanding of the above is that
> os_atomic_load_ulint()/os_atomic_store_ulint() fit the above description,
> modulo alignment issues, if any. These are easy to ensure by ut_ad().
> >
>
> Modulo alignment, cache line boundary and page boundary issues.

Alignment only unless my reasoning above is wrong.

> I don't see how ut_ad() is going to help here. So a buf_pool_stat_t
> structure happens to be allocated in memory so that n_pages_written
> happens to be misaligned, or cross a cache line or a page boundary. How
> exactly ut_ad() is going to ensure that never happens at runtime?

A debug build would hit this assert and we'd fix the structure layout/allocation. Unless I'm mistaken, to get a misaligned ulint, we'd have to ask for this explicitly, by packing a struct, fetching a pointer to it from a byte array, etc. Thus ut_ad() seems reasonable to me.

> >>>> They should be named os_ordered_load_ulint() /
> >>>> os_ordered_store_ulint(),
> >>>
> >>> That's an option, but I needed atomicity, visibility, and ordering, and
> >> chose atomic for ...

[Laurynas Biveinis (laurynas-biveinis)]

buf_LRU_remove_all_pages() was ported incorrectly from 5.5, dropping the space id and I/O-fix re-checks after the block mutex acquisition. This has been caught by Roel as bug 1224282.

review: Needs Fixing

[Alexey Kopytov (akopytov)]

Hi Laurynas,

On Thu, 12 Sep 2013 06:16:52 -0000, Laurynas Biveinis wrote:
> Alexey -
>
>
>>>> - (if we only care about x86/x86_64) make sure that values being
>>>> loaded/stored do not cross cache lines or page boundaries. Which is of
>>>> course impossible to guarantee in a generic function.
>>>
>>>
>>> Why? We are talking about ulints here only, and I was not able to find such
>> requirements in the x86_64 memory model descriptions. There is a requirement
>> to be aligned, and misaligned stores/loads might indeed cross cache line or
>> page boundaries, and anything that crosses them is indeed non-atomic. But
>> alignment is possible to guarantee in a generic function (which doesn't even
>> has to be generic: the x86_64 implementation is for x86_64 only, obviously).
>>>
>>> Intel® 64 and IA-32 Architectures Software Developer's Manual
>>> Volume 3A: System Programming Guide, Part 1, section 8.1.1,
>> http://download.intel.com/products/processor/manual/253668.pdf:
>>>
>>> "The Intel486 processor (and newer processors since) guarantees that the
>> following basic memory operations will
>>> always be carried out atomically:
>>> (...)
>>> • Reading or writing a doubleword aligned on a 32-bit boundary
>>>
>>> The Pentium processor (...):
>>> • Reading or writing a quadword aligned on a 64-bit boundary
>>> "
>>
>> Why didn't you quote it further?
>>
>> "
>> Accesses to cacheable memory that are split across cache lines and page
>> boundaries are not guaranteed to be atomic by <all processors>. <all
>> processors> provide bus control signals that permit external memory
>> subsystems to make split accesses atomic;
>> "
>>
>> Which means even aligned accesses are not guaranteed to be atomic and
>> it's up to the implementation of "external memory subsystems" (that
>> probably means chipsets, motherboards, NUMA architectures and the like).
>
>
> I didn't quote because we both have already acknowledged that cache line- or page boundary-crossing accesses are non-atomic, and because I don't see how it's relevant here. I don't see how a properly-aligned ulint can possibly cross a cache line boundary, when cache lines are 64-byte wide and 64-byte aligned. Or even 32 for older architectures.
>

The array of buffer pool descriptors is allocated as follows:

 buf_pool_ptr = (buf_pool_t*) mem_zalloc(
  n_instances * sizeof *buf_pool_ptr);

so individual buf_pool_t instances are not guaranteed to have any
specific alignment, neither to cache line nor to page boundaries, right?

Now, the 'stat' member of buf_pool_t has the offset of 736 bytes into
buf_pool_t so nothing prevents it from crossing a cache line or a page
boundary?

Now, offsets of the buf_pool_stat_t members vary from 0 to 88. Again,
nothing prevents them from crossing a cache line or a page boundary, right?

>
>>> My understanding of the above is that
>> os_atomic_load_ulint()/os_atomic_store_ulint() fit the above description,
>> modulo alignment issues, if any. These are easy to ensure by ut_ad().
>>>
>>
>> Modulo alignment, cache line boundary and page boundary issues.
>
>
> Alignment only unless my reasoning above is wrong.
>

Yes.

>
>> I don't see how ut_ad() is going to help here. So a ...

[Laurynas Biveinis (laurynas-biveinis)]

Repushed branch with the 1st partial review comments. Not a resubmission due to partial review and ongoing discussions.

    Changes from the 1st MP.
    - Simplified btr_blob_free().
    - Added a note about mutexes to the header comment of
      buf_buddy_relocate().
    - Removed redundant mutex == NULL checks and mutex own assertions
      from buf_buddy_relocate().
    - Fixed locking notes in buf_LRU_free_page() header comment.
    - Removed a memory leak in one of the early exits in
      buf_LRU_free_page().
    - Clarified locking in a comment for buf_page_t::zip.
    - Added debug build checks to os_atomic_load_ulint() and
      os_atomic_store_ulint() x86_64 implementation that the accessed
      variable is properly aligned.
    - Added dropped by mistake buffer page space id and I/O fix 2nd
      checks after the buf_page_get_mutex() has been locked in
      buf_LRU_remove_all_pages().

Please ignore the "Added debug build checks to os_atomic_load_ulint() and os_atomic_store_ulint() x86_64 implementation that the accessed variable is properly aligned." bit for now.

[Laurynas Biveinis (laurynas-biveinis)]

A Jenkins run of the latest branch turned up bug 1224432. Logged a separate bug because I am not sure I'll manage to debug it during this MP cycle.

"...
2013-09-12 12:15:38 7ff450082700 InnoDB: Assertion failure in thread 140687291459328 in file buf0buf.cc line 3694
InnoDB: Failing assertion: buf_fix_count > 0
...

Which appears to be a race condition on buf_fix_count on a page that is a sentinel for buffer pool watch. How exactly this can happen is not clear to me currently. All the watch sentinel buf_fix_count changes happen under zip_mutex and a corresponding page_hash X lock. Further, buf_page_init_for_read() asserts buf_fix_count > 0 through buf_pool_watch_is_sentinel() at line 3647, thus this should have changed between 3647 and 3694, but the hash is X-latched throughout, even though the zip mutex is only acquired at 3670."

review: Needs Fixing

[Laurynas Biveinis (laurynas-biveinis)]

Alexey -

> I don't see how a properly-aligned ulint can possibly
> cross a cache line boundary, when cache lines are 64-byte wide and 64-byte
> aligned. Or even 32 for older architectures.
> >
>
> The array of buffer pool descriptors is allocated as follows:

You are right, I failed to consider the base addresses returned by dynamic memory allocation. I also failed to notice your hint to that direction in one of the previous mails.

> buf_pool_ptr = (buf_pool_t*) mem_zalloc(
> n_instances * sizeof *buf_pool_ptr);
>
> so individual buf_pool_t instances are not guaranteed to have any
> specific alignment, neither to cache line nor to page boundaries, right?

Right.

> Now, the 'stat' member of buf_pool_t has the offset of 736 bytes into
> buf_pool_t so nothing prevents it from crossing a cache line or a page
> boundary?

Right.

> Now, offsets of the buf_pool_stat_t members vary from 0 to 88. Again,
> nothing prevents them from crossing a cache line or a page boundary, right?

Right, nothing prevents an object of buf_pool_stat_t from crossing it. But that's OK. We only need the individual fields not to cross it.

> >> I don't see how ut_ad() is going to help here. So a buf_pool_stat_t
> >> structure happens to be allocated in memory so that n_pages_written
> >> happens to be misaligned, or cross a cache line or a page boundary. How
> >> exactly ut_ad() is going to ensure that never happens at runtime?
> >
> >
> > A debug build would hit this assert and we'd fix the structure
> layout/allocation. Unless I'm mistaken, to get a misaligned ulint, we'd have
> to ask for this explicitly, by packing a struct, fetching a pointer to it from
> a byte array, etc. Thus ut_ad() seems reasonable to me.
> >
>
> The only thing you can assume about dynamically allocated objects is
> that their addresses (and thus, the first member of a structure, if an
> object is a structure) is aligned to machine word size. Which is always
> lower than the cache line size. There are no guarantees on alignment of
> other structure members, no matter what compiler hints were used (those
> only matter for statically allocated objects).

Right. So, to conclude... no individual ulint is going to cross a cache line or a page boundary and we are good? We start with a machine-word aligned address returned from heap, and add a multiple of machine-word width to arrive at the address of an individual field, which is machine-word aligned and thus the individual field cannot cross anything?

> >>>>>> They should be named os_ordered_load_ulint() /
> >>>>>> os_ordered_store_ulint(),
> >>>>>
> >>>>> That's an option, but I needed atomicity, visibility, and ordering, and
> >>>> chose atomic for function name to match the existing CAS and atomic add
> >>>> operations, which also need all three.
> >>>>>
> >>>>
> >>>> I'm not sure you need all of those 3 in every occurrence of those
> >>>> functions, but see below.
> >>>
> >>>
> >>> That's right. And ordering probably is not needed anywhere, sorry about
> >> that, my understanding of atomics is far from fluent. But visibility
> should
> >> be needed in every occurrence if this is ever ported to a...

[Alexey Kopytov (akopytov)]

Hi Laurynas,

You are right that that os_atomic_{load,store}_ulint() will work as the
name suggests (i.e. be atomic) as long as:

1. it is used to access machine word sized members of structures
2. we are on x86/x86_64

However, the patch implements them as a generic primitives that do
nothing to enforce those restrictions, and that's why their names are
misleading. This is where this discussion has started, and it is a
design flaw. I don't see any arguments in this discussion that would
dispel those concerns. You also acknowledged them in one of the comments.

In contrast, other atomic primitives in existing code keep up to their
promise of being atomic, i.e. do not enforce any implicit requirements.
But they also have mutex-guarded fall back implementations for those
architectures that do not provide atomics.

I also agree that this discussion may be endless and time is precious.
So I think we should implement whatever we both agree does work. That
is: instead of implementing generic atomic primitives that are only
atomic under implicit requirements that are not enforceable at compile
time, it must either use separate mutex(es) to protect them, or use true
atomic primitives provided by the existing code if they are available on
the target architecture and fall back to mutex-guarded access. The
latter is how it is implemented in the rest of InnoDB.

Thanks.

[Laurynas Biveinis (laurynas-biveinis)]

Alexey -

> Hi Laurynas,
>
> You are right that that os_atomic_{load,store}_ulint() will work as the
> name suggests (i.e. be atomic) as long as:
>
> 1. it is used to access machine word sized members of structures
> 2. we are on x86/x86_64

Right.

> However, the patch implements them as a generic primitives that do
> nothing to enforce those restrictions, and that's why their names are
> misleading.

1. is enforced through "ulint" in the name and args. ulint is commented in univ.i as "unsigned long integer which should be equal to the word size of the machine".
2. is enforced by platform #ifdefs not providing any other implementation except one for x86/x86_64 with GCC or a GCC-like compiler.

Thus I provide generic primitives, whose current implementations will work as designed. However the 1. above also seems to be missing "properly-aligned" and that's where the design is debatable. On one hand it is possible to implement misaligned access atomically by LOCK MOV, and document that the primitives may be used with args of any alignment. But a better alternative to me seems to accept that misaligned accesses are bugs and document/allow aligned accesses only. Even though that's enforceable in debug builds only, so that's not ideally perfect, but IMHO acceptable.

> This is where this discussion has started, and it is a
> design flaw. I don't see any arguments in this discussion that would
> dispel those concerns. You also acknowledged them in one of the comments.

Addressed above.

> I also agree that this discussion may be endless and time is precious.

But it also cannot end prematurely.

> So I think we should implement whatever we both agree does work.

I suggest that the above either is working already or requires some improvements re. alignment only.

> That
> is: instead of implementing generic atomic primitives that are only
> atomic under implicit requirements that are not enforceable at compile
> time, it must either use separate mutex(es) to protect them, or use true
> atomic primitives provided by the existing code if they are available on
> the target architecture

If you either show that how I address 1. and 2. above is incorrect, either show that the alignment issue is major and unsurmountable, then I'll implement load as inc by zero, return old value, and store as dirty read + CAS in a loop using the existing primitives.

Thanks,
Laurynas

[Alexey Kopytov (akopytov)]

Hi Laurynas,

On Fri, 13 Sep 2013 07:40:10 -0000, Laurynas Biveinis wrote:
> Alexey -
>
>
>> Hi Laurynas,
>>
>> You are right that that os_atomic_{load,store}_ulint() will work as the
>> name suggests (i.e. be atomic) as long as:
>>
>> 1. it is used to access machine word sized members of structures
>> 2. we are on x86/x86_64
>
>
> Right.
>
>
>> However, the patch implements them as a generic primitives that do
>> nothing to enforce those restrictions, and that's why their names are
>> misleading.
>
>
> 1. is enforced through "ulint" in the name and args. ulint is commented in univ.i as "unsigned long integer which should be equal to the word size of the machine".

It is not enforced, because nothing prevents me from passing a
misaligned address to those functions and expect them to be atomic as
the name implies.

For example, os_atomic_inc_ulint() is guaranteed to be atomic for any
arguments on any platform. But os_atomic_load_ulint() is not. That is
the problem.

> 2. is enforced by platform #ifdefs not providing any other implementation except one for x86/x86_64 with GCC or a GCC-like compiler.
>

That's correct. I only mentioned #2 for completeness.

> Thus I provide generic primitives, whose current implementations will work as designed. However the 1. above also seems to be missing "properly-aligned" and that's where the design is debatable. On one hand it is possible to implement misaligned access atomically by LOCK MOV, and document that the primitives may be used with args of any alignment. But a better alternative to me seems to accept that misaligned accesses are bugs and document/allow aligned accesses only. Even though that's enforceable in debug builds only, so that's not ideally perfect, but IMHO acceptable.
>

You don't.

>
> If you either show that how I address 1. and 2. above is incorrect, either show that the alignment issue is major and unsurmountable, then I'll implement load as inc by zero, return old value, and store as dirty read + CAS in a loop using the existing primitives.
>

Yes, please do.

[Laurynas Biveinis (laurynas-biveinis)]

Alexey -

> >> You are right that that os_atomic_{load,store}_ulint() will work as the
> >> name suggests (i.e. be atomic) as long as:
> >>
> >> 1. it is used to access machine word sized members of structures
> >> 2. we are on x86/x86_64
> >
> >
> > Right.
> >
> >
> >> However, the patch implements them as a generic primitives that do
> >> nothing to enforce those restrictions, and that's why their names are
> >> misleading.
> >
> >
> > 1. is enforced through "ulint" in the name and args. ulint is commented in
> univ.i as "unsigned long integer which should be equal to the word size of the
> machine".
>
> It is not enforced, because nothing prevents me from passing a
> misaligned address to those functions and expect them to be atomic as
> the name implies.

This is exactly what I discussed below.

> For example, os_atomic_inc_ulint() is guaranteed to be atomic for any
> arguments on any platform. But os_atomic_load_ulint() is not. That is
> the problem.

Right. os_atomic_load_ulint() has additional restrictions on its arg over os_atomic_inc_ulint(). I believe that these restrictions are reasonable. It is a performance bug in any case to perform misaligned atomic ops even with those ops that make it technically possible. I have added ut_ad()s to catch this. I can rename os_atomic_ prefix to os_atomic_aligned_ prefix too, although that one looks like an overkill to me.

> > 2. is enforced by platform #ifdefs not providing any other implementation
> except one for x86/x86_64 with GCC or a GCC-like compiler.
> >
>
> That's correct. I only mentioned #2 for completeness.

OK, but I am not sure what does the #2 complete then.

> > Thus I provide generic primitives, whose current implementations will work
> as designed. However the 1. above also seems to be missing "properly-aligned"
> and that's where the design is debatable. On one hand it is possible to
> implement misaligned access atomically by LOCK MOV, and document that the
> primitives may be used with args of any alignment. But a better alternative
> to me seems to accept that misaligned accesses are bugs and document/allow
> aligned accesses only. Even though that's enforceable in debug builds only,
> so that's not ideally perfect, but IMHO acceptable.
> >
>
> You don't.

Will you reply to the rest of that paragraph too please? I am acknowledging that alignment is an issue, so let's see how to resolve it.

[Alexey Kopytov (akopytov)]

Hi Laurynas,

On Wed, 11 Sep 2013 15:29:33 -0000, Laurynas Biveinis wrote:
>>>> - the following hunk simply removes reference to buf_pool->mutex. As I
>>>> understand, it should just replace buf_pool->mutex with
>> zip_free_mutex?
>>>> + page_zip_des_t zip; /*!< compressed page; state
>>>> + == BUF_BLOCK_ZIP_PAGE and zip.data
>>>> + == NULL means an active
>>>
>>> Hm, it looked to me that it's protected not with zip_free_mutex but with
>> zip_mutex in its page mutex capacity. I will check.
>>>
>>
>> There was a place in the code that asserted zip_free_mutex locked when
>> bpage->zip.data is modified. But I'm not sure if that is correct.
>
>
> I have checked, I believe it's indeed zip_mutex. Re. zip_free_mutex, you must be referring to this bit in buf_buddy_relocate():
>
> mutex_enter(&buf_pool->zip_free_mutex);
>
> if (buf_page_can_relocate(bpage)) {
> ...
> bpage->zip.data = (page_zip_t*) dst;
> mutex_exit(mutex);
>
> buf_buddy_stat_t* buddy_stat = &buf_pool->buddy_stat[i];
> buddy_stat->relocated++;
> buddy_stat->relocated_usec += ut_time_us(NULL) - usec;
> ...
>
> Here zip_free_mutex happens to protect buddy_stat and pushing it down to if clause would require an else clause to appear that locks the same mutex.
>

No, I was referring to buf_pool_contains_zip(). It traverses the buffer
pool and examines (but not modifies) block->page.zip.data for each
block. However, the patch changes the assertion in
buf_pool_contains_zip() to make sure that zip_free_mutex is locked,
rather than zip_mutex. In fact, in one of the code paths calling
buf_pool_contains_zip() we assert that zip_mutex is NOT locked. Don't we
have a bug here?

[Alexey Kopytov (akopytov)]

On Fri, 13 Sep 2013 11:10:36 -0000, Laurynas Biveinis wrote:
>
>
> Right. os_atomic_load_ulint() has additional restrictions on its arg over os_atomic_inc_ulint(). I believe that these restrictions are reasonable. It is a performance bug in any case to perform misaligned atomic ops even with those ops that make it technically possible. I have added ut_ad()s to catch this. I can rename os_atomic_ prefix to os_atomic_aligned_ prefix too, although that one looks like an overkill to me.
>

The same restrictions would apply even if os_atomic_load_ulint() didn't
exist, right? I.e. the same restrictions would apply if we simply
accessed those variables without any helper functions?

Let me ask you a few simple questions and this time around I demand
"yes/no" answers.

- Do you agree that os_atomic_load_ulint() / os_atomic_store_ulint() do
not do what they promise to do?

- Do you agree that naming them os_ordered_load_ulint() /
os_ordered_store_ulint() would better reflect what they do?

- Do you agree that naming them that way also makes it obvious that
using them in most places is simply unnecessary (e.g. in
buf_get_total_stat(), buf_mark_space_corrupt(), buf_print_instance(),
buf_get_n_pending_read_ios(), etc.)?

>
>>> 2. is enforced by platform #ifdefs not providing any other implementation
>> except one for x86/x86_64 with GCC or a GCC-like compiler.
>>>
>>
>> That's correct. I only mentioned #2 for completeness.
>
>
> OK, but I am not sure what does the #2 complete then.
>
>
>>> Thus I provide generic primitives, whose current implementations will work
>> as designed. However the 1. above also seems to be missing "properly-aligned"
>> and that's where the design is debatable. On one hand it is possible to
>> implement misaligned access atomically by LOCK MOV, and document that the
>> primitives may be used with args of any alignment. But a better alternative
>> to me seems to accept that misaligned accesses are bugs and document/allow
>> aligned accesses only. Even though that's enforceable in debug builds only,
>> so that's not ideally perfect, but IMHO acceptable.
>>>
>>
>> You don't.
>
>
> Will you reply to the rest of that paragraph too please? I am acknowledging that alignment is an issue, so let's see how to resolve it.
>

I don't think enforcing requirements in debug builds only is acceptable.
It must be a compile-time assertion, not a run-time one.

[Laurynas Biveinis (laurynas-biveinis)]

> > Right. os_atomic_load_ulint() has additional restrictions on its arg over
> os_atomic_inc_ulint(). I believe that these restrictions are reasonable. It
> is a performance bug in any case to perform misaligned atomic ops even with
> those ops that make it technically possible. I have added ut_ad()s to catch
> this. I can rename os_atomic_ prefix to os_atomic_aligned_ prefix too,
> although that one looks like an overkill to me.
> >
>
> The same restrictions would apply even if os_atomic_load_ulint() didn't
> exist, right? I.e. the same restrictions would apply if we simply
> accessed those variables without any helper functions?

What would be the desired access semantics in that case? If "anything goes", then no restrictions would apply.

> Let me ask you a few simple questions and this time around I demand
> "yes/no" answers.
>
> - Do you agree that os_atomic_load_ulint() / os_atomic_store_ulint() do
> not do what they promise to do?

Yes.

> - Do you agree that naming them os_ordered_load_ulint() /
> os_ordered_store_ulint() would better reflect what they do?

No.

> - Do you agree that naming them that way also makes it obvious that
> using them in most places is simply unnecessary (e.g. in
> buf_get_total_stat(), buf_mark_space_corrupt(), buf_print_instance(),
> buf_get_n_pending_read_ios(), etc.)?

No.

The first answer would be No if not the alignment issue.

> >>> Thus I provide generic primitives, whose current implementations will work
> >> as designed. However the 1. above also seems to be missing "properly-
> aligned"
> >> and that's where the design is debatable. On one hand it is possible to
> >> implement misaligned access atomically by LOCK MOV, and document that the
> >> primitives may be used with args of any alignment. But a better
> alternative
> >> to me seems to accept that misaligned accesses are bugs and document/allow
> >> aligned accesses only. Even though that's enforceable in debug builds
> only,
> >> so that's not ideally perfect, but IMHO acceptable.
> >>>
> >>
> >> You don't.
> >
> >
> > Will you reply to the rest of that paragraph too please? I am acknowledging
> that alignment is an issue, so let's see how to resolve it.
> >
>
> I don't think enforcing requirements in debug builds only is acceptable.
> It must be a compile-time assertion, not a run-time one.

And as we both know, this is not enforceable at compile time. I think that requesting extra protections on the top of already provided ones, when the only way to get a misaligned ulint is to ask for one explicitly, is an overkill. But that's my hand-waving against your hand-waving. Thus let's say that yes, "the alignment issue is major and unsurmountable", and I proceed to do what was offered previously: implement load as atomic add zero and return value, and store as dirty read and CAS until success. The reason I didn't like these implementations is that they are pessimized. But that's OK.

[Alexey Kopytov (akopytov)]

On Fri, 13 Sep 2013 12:54:44 -0000, Laurynas Biveinis wrote:
>>> Right. os_atomic_load_ulint() has additional restrictions on its arg over
>> os_atomic_inc_ulint(). I believe that these restrictions are reasonable. It
>> is a performance bug in any case to perform misaligned atomic ops even with
>> those ops that make it technically possible. I have added ut_ad()s to catch
>> this. I can rename os_atomic_ prefix to os_atomic_aligned_ prefix too,
>> although that one looks like an overkill to me.
>>>
>>
>> The same restrictions would apply even if os_atomic_load_ulint() didn't
>> exist, right? I.e. the same restrictions would apply if we simply
>> accessed those variables without any helper functions?
>
>
> What would be the desired access semantics in that case? If "anything goes", then no restrictions would apply.
>
>
>> Let me ask you a few simple questions and this time around I demand
>> "yes/no" answers.
>>
>> - Do you agree that os_atomic_load_ulint() / os_atomic_store_ulint() do
>> not do what they promise to do?
>
>
> Yes.

OK, so we agree that naming is unfortunate.

>
>
>> - Do you agree that naming them os_ordered_load_ulint() /
>> os_ordered_store_ulint() would better reflect what they do?
>
>
> No.

What would be a better naming then?

>
>
>> - Do you agree that naming them that way also makes it obvious that
>> using them in most places is simply unnecessary (e.g. in
>> buf_get_total_stat(), buf_mark_space_corrupt(), buf_print_instance(),
>> buf_get_n_pending_read_ios(), etc.)?
>
>
> No.

OK, then 2 followup questions:

1. Why do we need os_atomic_load_ulint() in buf_get_total_stat(), for
example? Here's an example of a valid answer: "Because that will result
in incorrect values being used in case ...". And some examples of
invalid answers: "non-cache-coherent architectures, visibility, memory
model, sunspots, crop circles, global warming, ...".

2. Why do only 2 out of 9 values are being loaded with
os_atomic_load_ulint() in buf_get_total_stat()? Why don't the remaining
ones need them?

[Laurynas Biveinis (laurynas-biveinis)]

Alexey -

> >> - Do you agree that os_atomic_load_ulint() / os_atomic_store_ulint() do
> >> not do what they promise to do?
> >
> >
> > Yes.
>
> OK, so we agree that naming is unfortunate.

Vanishingly slightly so, due to the alignment issue, which I believe is mostly theoretical, but nevertheless am ready to address now.

> >> - Do you agree that naming them os_ordered_load_ulint() /
> >> os_ordered_store_ulint() would better reflect what they do?
> >
> >
> > No.
>
> What would be a better naming then?

os_atomic_load_aligned_ulint().

> OK, then 2 followup questions:
>
> 1. Why do we need os_atomic_load_ulint() in buf_get_total_stat(), for
> example? Here's an example of a valid answer: "Because that will result
> in incorrect values being used in case ...". And some examples of
> invalid answers: "non-cache-coherent architectures, visibility, memory
> model, sunspots, crop circles, global warming, ...".

We have gone through this with a disassembly example already, haven't we? We need os_atomic_load_ulint() because we don't want a dirty read. We may well decide that a dirty read there is fine and then replace it. But that's orthogonal to what is this primitive and why.

Are you also objecting to mutex protection here? If not, why? Note that the three n_flush values here are completely independent.

  mutex_enter(&buf_pool->flush_state_mutex);

  pending_io += buf_pool->n_flush[BUF_FLUSH_LRU];
  pending_io += buf_pool->n_flush[BUF_FLUSH_SINGLE_PAGE];
  pending_io += buf_pool->n_flush[BUF_FLUSH_LIST];

  mutex_exit(&buf_pool->flush_state_mutex);

And please don't group some of the valid answers with looney stuff.

> 2. Why do only 2 out of 9 values are being loaded with
> os_atomic_load_ulint() in buf_get_total_stat()? Why don't the remaining
> ones need them?

Upstream reads all 9 dirtily. I replaced two of them to be clean instead. Maybe I need to replace all 9. Maybe 0. But that's again orthogonal.

[Laurynas Biveinis (laurynas-biveinis)]

Alexey -

> No, I was referring to buf_pool_contains_zip(). It traverses the buffer
> pool and examines (but not modifies) block->page.zip.data for each
> block. However, the patch changes the assertion in
> buf_pool_contains_zip() to make sure that zip_free_mutex is locked,
> rather than zip_mutex.

Thanks for the pointer, I have reviewed buf_pool_contains_zip() further. Here, as you say, we iterate through the buffer pool uncompressed pages trying to find the uncompressed page of a given compressed page, in order to assert that no uncompressed page points to it, which can be either just taken from the buddy allocator, either be just removed from the buffer pool. In both cases it's an invalid pointer value and it seems to me that we could iterate through the buffer pool without any locking at all. I have tried to think of possible race conditions in the case of no locking, i.e. can the pointer become valid again somehow, by some uncompressed page starting pointing to this zip page. And the transition of the given zip page pointer unallocated -> allocated is protected by zip_free_mutex. Thus, as long as all its callers hold zip_free_mutex and it's only called with invalid pointer values to assert that FALSE is returned, buf_pool_contains_zip() itself does not need any locking. If my reasoning is correct, I can remove the assert from this function. But maybe this should be documented better somehow?

> In fact, in one of the code paths calling
> buf_pool_contains_zip() we assert that zip_mutex is NOT locked. Don't we
> have a bug here?

buf_buddy_free_low(), right? This function is called for a compressed page that is already removed from the buffer pool, that is, no pointer to it should be live in any other thread (and in the buffer pool, as it's asserted by !buf_pool_contains_zip()). Thus it's ok not hold zip_mutex.

[Alexey Kopytov (akopytov)]

Hi Laurynas,

On Fri, 13 Sep 2013 14:21:58 -0000, Laurynas Biveinis wrote:
> Alexey -
>
>>>> - Do you agree that os_atomic_load_ulint() / os_atomic_store_ulint() do
>>>> not do what they promise to do?
>>>
>>>
>>> Yes.
>>
>> OK, so we agree that naming is unfortunate.
>
>
> Vanishingly slightly so, due to the alignment issue, which I believe is mostly theoretical, but nevertheless am ready to address now.
>

It doesn't do anything to enforce atomicity, does it? I.e. the following
implementation would be equally "atomic":

ulint
os_atomic_load_ulint(ulint *ptr)
{
 printf("Hello, world!\n");
 return(*ptr);
}

>
>>>> - Do you agree that naming them os_ordered_load_ulint() /
>>>> os_ordered_store_ulint() would better reflect what they do?
>>>
>>>
>>> No.
>>
>> What would be a better naming then?
>
>
> os_atomic_load_aligned_ulint().
>

No, it doesn't do anything to enforce atomicity. That is a caller's
responsibility.

>
>> OK, then 2 followup questions:
>>
>> 1. Why do we need os_atomic_load_ulint() in buf_get_total_stat(), for
>> example? Here's an example of a valid answer: "Because that will result
>> in incorrect values being used in case ...". And some examples of
>> invalid answers: "non-cache-coherent architectures, visibility, memory
>> model, sunspots, crop circles, global warming, ...".
>
>
> We have gone through this with a disassembly example already, haven't we? We need os_atomic_load_ulint() because we don't want a dirty read. We may well decide that a dirty read there is fine and then replace it. But that's orthogonal to what is this primitive and why.
>

We need an atomic read rather than os_atomic_load_ulint() for the above
reasons. An it will be atomic without using any helper functions. Since
I see no answer in the form "Because that will result in incorrect
values being used in case ...", I assume you don't have an answer to
that question.

> Are you also objecting to mutex protection here? If not, why? Note that the three n_flush values here are completely independent.
>
> mutex_enter(&buf_pool->flush_state_mutex);
>
> pending_io += buf_pool->n_flush[BUF_FLUSH_LRU];
> pending_io += buf_pool->n_flush[BUF_FLUSH_SINGLE_PAGE];
> pending_io += buf_pool->n_flush[BUF_FLUSH_LIST];
>
> mutex_exit(&buf_pool->flush_state_mutex);
>

I'm not objecting to mutex protection in that code. Why would I?

> And please don't group some of the valid answers with looney stuff.
>
>
>> 2. Why do only 2 out of 9 values are being loaded with
>> os_atomic_load_ulint() in buf_get_total_stat()? Why don't the remaining
>> ones need them?
>
>
> Upstream reads all 9 dirtily. I replaced two of them to be clean instead. Maybe I need to replace all 9. Maybe 0. But that's again orthogonal.
>

All 9 reads are atomic. But 7 of them don't use compiler barriers
because they don't need it. Neither do the remaining 2, but you are
quite creative to avoid accepting this simple fact.

[Alexey Kopytov (akopytov)]

On Sun, 15 Sep 2013 07:47:59 -0000, Laurynas Biveinis wrote:
> Alexey -
>
>
>> No, I was referring to buf_pool_contains_zip(). It traverses the buffer
>> pool and examines (but not modifies) block->page.zip.data for each
>> block. However, the patch changes the assertion in
>> buf_pool_contains_zip() to make sure that zip_free_mutex is locked,
>> rather than zip_mutex.
>
>
> Thanks for the pointer, I have reviewed buf_pool_contains_zip() further. Here, as you say, we iterate through the buffer pool uncompressed pages trying to find the uncompressed page of a given compressed page, in order to assert that no uncompressed page points to it, which can be either just taken from the buddy allocator, either be just removed from the buffer pool. In both cases it's an invalid pointer value and it seems to me that we could iterate through the buffer pool without any locking at all. I have tried to think of possible race conditions in the case of no locking, i.e. can the pointer become valid again somehow, by some uncompressed page starting pointing to this zip page. And the transition of the given zip page pointer unallocated -> allocated is protected by zip_free_mutex. Thus, as long as all its callers hold zip_free_mutex and it's only called with invalid pointer values to assert that FALSE is returned, buf_pool_contains_zip() itself does not need
 any locki
ng. If my reasoning is correct, I can remove the assert from this function. But maybe this should be documented better somehow?
>

Looks correct to me. Let's remove the zip_free_mutex assertion then?

>
>> In fact, in one of the code paths calling
>> buf_pool_contains_zip() we assert that zip_mutex is NOT locked. Don't we
>> have a bug here?
>
>
> buf_buddy_free_low(), right? This function is called for a compressed page that is already removed from the buffer pool, that is, no pointer to it should be live in any other thread (and in the buffer pool, as it's asserted by !buf_pool_contains_zip()). Thus it's ok not hold zip_mutex.
>

OK, thanks for clarification.

[Laurynas Biveinis (laurynas-biveinis)]

Alexey -

> > I have reviewed buf_pool_contains_zip() further.
...
> Looks correct to me. Let's remove the zip_free_mutex assertion then?

Yes.

[Laurynas Biveinis (laurynas-biveinis)]

Alexey -

> >>>> - Do you agree that os_atomic_load_ulint() / os_atomic_store_ulint() do
> >>>> not do what they promise to do?
> >>>
> >>>
> >>> Yes.
> >>
> >> OK, so we agree that naming is unfortunate.
> >
> >
> > Vanishingly slightly so, due to the alignment issue, which I believe is
> mostly theoretical, but nevertheless am ready to address now.
> >
>
> It doesn't do anything to enforce atomicity, does it? I.e. the following
> implementation would be equally "atomic":
>
> ulint
> os_atomic_load_ulint(ulint *ptr)
> {
> printf("Hello, world!\n");
> return(*ptr);
> }

Yes, it would be equally atomic (ignoring visibility and ordering) on x86_64 as long as pointer is aligned.

> >>>> - Do you agree that naming them os_ordered_load_ulint() /
> >>>> os_ordered_store_ulint() would better reflect what they do?
> >>>
> >>>
> >>> No.
> >>
> >> What would be a better naming then?
> >
> >
> > os_atomic_load_aligned_ulint().
> >
>
> No, it doesn't do anything to enforce atomicity. That is a caller's
> responsibility.

As in, don't pass misaligned values? In that case, yes, it is a caller's responsibility not to pass misaligned values. But where would InnoDB get a misaligned pointer to ulint from that we'd wish to access atomically? Hence enforcing alignment on debug build only seemed like a reasonable compromise, but OK, that's debatable.

> >> 1. Why do we need os_atomic_load_ulint() in buf_get_total_stat(), for
> >> example? Here's an example of a valid answer: "Because that will result
> >> in incorrect values being used in case ...". And some examples of
> >> invalid answers: "non-cache-coherent architectures, visibility, memory
> >> model, sunspots, crop circles, global warming, ...".
> >
> >
> > We have gone through this with a disassembly example already, haven't we?
> We need os_atomic_load_ulint() because we don't want a dirty read. We may
> well decide that a dirty read there is fine and then replace it. But that's
> orthogonal to what is this primitive and why.
> >
>
> We need an atomic read rather than os_atomic_load_ulint() for the above
> reasons. An it will be atomic without using any helper functions.

OK, so is the problem that I wanted to introduce the primitives for such access, that would also document how is the variable accessed, and that they don't have to do much besides a compiler barrier on x86_64?

> Since
> I see no answer in the form "Because that will result in incorrect
> values being used in case ...", I assume you don't have an answer to
> that question.

I know that they are not resulting in incorrect values currently, and that the worst can happen in x86_64 with the most of possible future code changes is that the value loads could be moved earlier, resulting in more out-of-date values used. This and the fact that accessing the variable through the primitive serves as self-documentation seem good enough reasons to me.

> > Are you also objecting to mutex protection here? If not, why? Note that the
> three n_flush values here are completely independent.
> >
> > mutex_enter(&buf_pool->flush_state_mutex);
> >
> > pending_io += buf_pool->n_flush[BUF_FLUSH_...

[Alexey Kopytov (akopytov)]

On Mon, 16 Sep 2013 09:05:39 -0000, Laurynas Biveinis wrote:
> Alexey -
>
>
>>>>>> - Do you agree that os_atomic_load_ulint() / os_atomic_store_ulint() do
>>>>>> not do what they promise to do?
>>>>>
>>>>>
>>>>> Yes.
>>>>
>>>> OK, so we agree that naming is unfortunate.
>>>
>>>
>>> Vanishingly slightly so, due to the alignment issue, which I believe is
>> mostly theoretical, but nevertheless am ready to address now.
>>>
>>
>> It doesn't do anything to enforce atomicity, does it? I.e. the following
>> implementation would be equally "atomic":
>>
>> ulint
>> os_atomic_load_ulint(ulint *ptr)
>> {
>> printf("Hello, world!\n");
>> return(*ptr);
>> }
>
>
> Yes, it would be equally atomic (ignoring visibility and ordering) on x86_64 as long as pointer is aligned.
>

So... we don't need it after all?

>
>>>>>> - Do you agree that naming them os_ordered_load_ulint() /
>>>>>> os_ordered_store_ulint() would better reflect what they do?
>>>>>
>>>>>
>>>>> No.
>>>>
>>>> What would be a better naming then?
>>>
>>>
>>> os_atomic_load_aligned_ulint().
>>>
>>
>> No, it doesn't do anything to enforce atomicity. That is a caller's
>> responsibility.
>
>
> As in, don't pass misaligned values? In that case, yes, it is a caller's responsibility not to pass misaligned values. But where would InnoDB get a misaligned pointer to ulint from that we'd wish to access atomically? Hence enforcing alignment on debug build only seemed like a reasonable compromise, but OK, that's debatable.
>
>
>>>> 1. Why do we need os_atomic_load_ulint() in buf_get_total_stat(), for
>>>> example? Here's an example of a valid answer: "Because that will result
>>>> in incorrect values being used in case ...". And some examples of
>>>> invalid answers: "non-cache-coherent architectures, visibility, memory
>>>> model, sunspots, crop circles, global warming, ...".
>>>
>>>
>>> We have gone through this with a disassembly example already, haven't we?
>> We need os_atomic_load_ulint() because we don't want a dirty read. We may
>> well decide that a dirty read there is fine and then replace it. But that's
>> orthogonal to what is this primitive and why.
>>>
>>
>> We need an atomic read rather than os_atomic_load_ulint() for the above
>> reasons. An it will be atomic without using any helper functions.
>
>
> OK, so is the problem that I wanted to introduce the primitives for such access, that would also document how is the variable accessed, and that they don't have to do much besides a compiler barrier on x86_64?
>

Yes, the problem is that you introduce primitives that basically do
nothing, and then use those primitives unnecessarily and inconsistently.
Which in turn leads to blowing up the patch size, increased maintenance
burden, and opens the door for wrong assumptions made when reading the
existing code and implementing new code.

>
>> Since
>> I see no answer in the form "Because that will result in incorrect
>> values being used in case ...", I assume you don't have an answer to
>> that question.
>
>
> I know that they are not resulting in incorrect values currently, and that the worst can happen in x86_64 with the most of possible future code changes is that the va...

[Alexey Kopytov (akopytov)]

More comments on the patch:

  - typo (double “get”) in the updated buf_LRU_free_page() comments:
   “function returns false, the buf_page_get_get_mutex() might be temporarily”

  - what’s the reason for changing buf_page_t::in_LRU_list to be present
    in release builds? Unless I’m missing something in the current code,
    it is only assigned, but never read in release builds?

  - spurious blank line changes in buf_buddy_relocate(),
    buf_buddy_free_low(), buf_pool_watch_set(), buf_page_get_gen(),
    buf_page_init_for_read(), buf_pool_validate_instance(),
    buf_flush_check_neighbor(), buf_flush_LRU_list_batch(), buf_LRU_drop_page_hash_for_tablespace()
    and innodb_buffer_pool_evict_uncompressed().

  - the following change in buf_block_try_discard_uncompressed() does
    not release block_mutex if buf_LRU_free_page() returns false.

  bpage = buf_page_hash_get(buf_pool, space, offset);

  if (bpage) {
- buf_LRU_free_page(bpage, false);
+
+ ib_mutex_t* block_mutex = buf_page_get_mutex(bpage);
+
+ mutex_enter(block_mutex);
+
+ if (buf_LRU_free_page(bpage, false)) {
+
+ mutex_exit(block_mutex);
+ return;
+ }
  }

  - do you really need this change?

@@ -2114,8 +2098,8 @@ buf_page_get_zip(
   break;
  case BUF_BLOCK_ZIP_PAGE:
  case BUF_BLOCK_ZIP_DIRTY:
+ buf_enter_zip_mutex_for_page(bpage);
   block_mutex = &buf_pool->zip_mutex;
- mutex_enter(block_mutex);
   bpage->buf_fix_count++;
   goto got_block;
  case BUF_BLOCK_FILE_PAGE:

  - in the following change:

@@ -2721,13 +2707,14 @@ buf_page_get_gen(
   }

   bpage = &block->page;
+ ut_ad(buf_own_zip_mutex_for_page(bpage));

   if (bpage->buf_fix_count
       || buf_page_get_io_fix(bpage) != BUF_IO_NONE) {
    /* This condition often occurs when the buffer
    is not buffer-fixed, but I/O-fixed by
    buf_page_init_for_read(). */
- mutex_exit(block_mutex);
+ mutex_exit(&buf_pool->zip_mutex);
 wait_until_unfixed:
    /* The block is buffer-fixed or I/O-fixed.
    Try again later. */

     is there a reason for replacing block_mutex with zip_mutex? you
     could just assert that block_mutex is zip_mutex next to, or even
     instead of the buf_own_zip_mutex_for_page() call.

  - same comments for this change:

@@ -2737,11 +2724,11 @@ buf_page_get_gen(
   }

   /* Allocate an uncompressed page. */
- mutex_exit(block_mutex);
+ mutex_exit(&buf_pool->zip_mutex);
   block = buf_LRU_get_free_block(buf_pool);
   ut_a(block);

- buf_pool_mutex_enter(buf_pool);
+ mutex_enter(&buf_pool->LRU_list_mutex);

   /* As we have released the page_hash lock and the
   block_mutex to allocate an uncompressed page it is

  - in buf_mark_space_corrupt() I see LRU_list_mutex, hash_lock and
    buf_page_get_mutex() being acquired, but only LRU_list_mutex and
    buf_page_get_mutex() being released, i.e. it returns with hash_lock
    acquired?

  - it looks like with the changes in buf_page_io_complete() for io_type
    == BUF_IO_WRITE we don’t set io_fix to BUF_IO_NONE, though we did
    before the changes?

  - more spurious changes:

@@ -475,8 +473,8 @@ buf_flush_insert_sorted_into_flush_list(
  if (prev_b == NULL) {
   UT_LIST_ADD_FIRST(list, buf_pool->flush_list, &block->page);
...

review: Needs Fixing

[Laurynas Biveinis (laurynas-biveinis)]

Alexey -

Replying to this bit separately as it may need further discussion while I am addressing the rest of comments.

> - what’s the reason for changing buf_page_t::in_LRU_list to be present
> in release builds? Unless I’m missing something in the current code,
> it is only assigned, but never read in release builds?

This is another "automerge" from 5.5 and indeed serves no release
build purpose in the current 5.6 code. But it points out a
non-trivial thing. In 5.5 it is used as follows:
-) for checking whether a given page is still on the LRU list if both
   block and LRU mutexes were temporarily released:
   buf_page_get_zip(), buf_LRU_free_block() (both are different code
   from 5.6).
-) for iterating through the LRU list without holding the LRU list
   mutex at all: buf_LRU_free_from_common_LRU_list(),
   buf_LRU_free_block(),
   buf_flush_LRU_recommendation()/buf_flush_ready_for_replace(). I
   think this is unsafe and a bug in 5.5 due to page relocations
   potentially resulting in wild pointers, even if it does wonders for
   the LRU list contention. 5.6 holds the LRU list mutex in the corresponding code.
-) redundant checks, ie. on LRU list iteration where the mutex is not
   released: buf_LRU_insert_zip_clean(),
   buf_LRU_free_from_unzip_LRU_list().

Thus I think 1) in_LRU_list changes should be reverted now. 2) 5.5
might need fixing. 3) The LRU list mutex is hot in 5.6. If there is
a safe way not to hold it in 5.6 (for example, for
BUF_BLOCK_FILE_PAGE, but hard to tell the page type without
dereferencing page pointer - maybe by comparing the page address
against buffer pool chunk address range?), then it's worth looking
into it.

[Laurynas Biveinis (laurynas-biveinis)]

Alexey -

> - typo (double “get”) in the updated buf_LRU_free_page() comments:
> “function returns false, the buf_page_get_get_mutex() might be temporarily”

Fixed.

> - what’s the reason for changing buf_page_t::in_LRU_list to be present
> in release builds? Unless I’m missing something in the current code,
> it is only assigned, but never read in release builds?

in_LRU_list changes have been reverted with the exception of an extra
assert in buf_page_set_sticky(). I also found that in_unzip_LRU_list
was converted to a release build flag but no uses were converted.
Reverted that too.

> - spurious blank line changes in buf_buddy_relocate(),
> buf_buddy_free_low(), buf_pool_watch_set(),

Fixed.

> buf_page_get_gen(),

I didn't find this one. Will re-check before the final push.

> buf_page_init_for_read(), buf_pool_validate_instance(),
> buf_flush_check_neighbor(), buf_flush_LRU_list_batch(),
> buf_LRU_drop_page_hash_for_tablespace()
> and innodb_buffer_pool_evict_uncompressed().

Fixed. Also removed a diagnostic printf from
buf_pool_validate_instance().

> - the following change in buf_block_try_discard_uncompressed() does
> not release block_mutex if buf_LRU_free_page() returns false.

Fixed.

> - do you really need this change?
>
> @@ -2114,8 +2098,8 @@ buf_page_get_zip(
> break;
> case BUF_BLOCK_ZIP_PAGE:
> case BUF_BLOCK_ZIP_DIRTY:
> + buf_enter_zip_mutex_for_page(bpage);
> block_mutex = &buf_pool->zip_mutex;
> - mutex_enter(block_mutex);
> bpage->buf_fix_count++;
> goto got_block;
> case BUF_BLOCK_FILE_PAGE:

No, I don't. A debugging leftover, reverted.

> - in the following change:
>
> @@ -2721,13 +2707,14 @@ buf_page_get_gen(
> }
>
> bpage = &block->page;
> + ut_ad(buf_own_zip_mutex_for_page(bpage));
>
> if (bpage->buf_fix_count
> || buf_page_get_io_fix(bpage) != BUF_IO_NONE) {
> /* This condition often occurs when the buffer
> is not buffer-fixed, but I/O-fixed by
> buf_page_init_for_read(). */
> - mutex_exit(block_mutex);
> + mutex_exit(&buf_pool->zip_mutex);
> wait_until_unfixed:
> /* The block is buffer-fixed or I/O-fixed.
> Try again later. */
>
> is there a reason for replacing block_mutex with zip_mutex? you
> could just assert that block_mutex is zip_mutex next to, or even
> instead of the buf_own_zip_mutex_for_page() call.

Yes. Replaced buf_own_zip_mutex_for_page() with ut_ad(block_mutex ==
&buf_pool->zip_mutex), which also happens to be symmetric with "!="
assert above for BUF_BLOCK_FILE_PAGE. Reverted the mutex_exit change
too.

> - same comments for this change:
>
> @@ -2737,11 +2724,11 @@ buf_page_get_gen(
> }
>
> /* Allocate an uncompressed page. */
> - mutex_exit(block_mutex);
> + mutex_exit(&buf_pool->zip_mutex);...

[Laurynas Biveinis (laurynas-biveinis)]

buf_enter_zip_mutex_for_page(buf_page_t *) is a bad idea, unless a buffer pool pointer is passed too, which then makes it an overkill, because it dereferences an unprotected bpage pointer, resulting in the below. Will remove it and replace back with mutex_enter(&buf_pool->zip_mutex).

http://jenkins.percona.com/job/percona-server-5.6-param/273/BUILD_TYPE=debug,Host=debian-wheezy-x64/testReport/junit/%28root%29/innodb/innodb_wl5522_zip/

Thread 1 (Thread 0x7f3acebfe700 (LWP 321)):
#0 __pthread_kill (threadid=<optimized out>, signo=<optimized out>) at ../nptl/sysdeps/unix/sysv/linux/pthread_kill.c:63
#1 0x0000000000ae2f45 in my_write_core (sig=6) at /mnt/workspace/percona-server-5.6-param/BUILD_TYPE/debug/Host/debian-wheezy-x64/Percona-Server/mysys/stacktrace.c:422
#2 0x000000000075eaa9 in handle_fatal_signal (sig=6) at /mnt/workspace/percona-server-5.6-param/BUILD_TYPE/debug/Host/debian-wheezy-x64/Percona-Server/sql/signal_handler.cc:251
#3 <signal handler called>
#4 0x00007f3ae835c475 in *__GI_raise (sig=<optimized out>) at ../nptl/sysdeps/unix/sysv/linux/raise.c:64
#5 0x00007f3ae835f6f0 in *__GI_abort () at abort.c:92
#6 0x0000000000d9b44c in buf_pool_from_bpage (bpage=0x33912c0) at /mnt/workspace/percona-server-5.6-param/BUILD_TYPE/debug/Host/debian-wheezy-x64/Percona-Server/storage/innobase/include/buf0buf.ic:88
#7 0x0000000000d9cb3c in buf_enter_zip_mutex_for_page (bpage=0x33912c0) at /mnt/workspace/percona-server-5.6-param/BUILD_TYPE/debug/Host/debian-wheezy-x64/Percona-Server/storage/innobase/include/buf0buf.ic:1412
#8 0x0000000000da2e5b in buf_page_get_gen (space=431, zip_size=8192, offset=9, rw_latch=1, guess=0x0, mode=10, file=0x1109668 "/mnt/workspace/percona-server-5.6-param/BUILD_TYPE/debug/Host/debian-wheezy-x64/Percona-Server/storage/innobase/btr/btr0pcur.cc", line=438, mtr=0x7f3acebfd5e0) at /mnt/workspace/percona-server-5.6-param/BUILD_TYPE/debug/Host/debian-wheezy-x64/Percona-Server/storage/innobase/buf/buf0buf.cc:2743
#9 0x0000000000d8c45a in btr_block_get_func (space=431, zip_size=8192, page_no=9, mode=1, file=0x1109668 "/mnt/workspace/percona-server-5.6-param/BUILD_TYPE/debug/Host/debian-wheezy-x64/Percona-Server/storage/innobase/btr/btr0pcur.cc", line=438, index=0x3393d18, mtr=0x7f3acebfd5e0) at /mnt/workspace/percona-server-5.6-param/BUILD_TYPE/debug/Host/debian-wheezy-x64/Percona-Server/storage/innobase/include/btr0btr.ic:60
#10 0x0000000000d8df82 in btr_pcur_move_to_next_page (cursor=0x7f3acebfd420, mtr=0x7f3acebfd5e0) at /mnt/workspace/percona-server-5.6-param/BUILD_TYPE/debug/Host/debian-wheezy-x64/Percona-Server/storage/innobase/btr/btr0pcur.cc:438
#11 0x0000000000df3a47 in btr_pcur_move_to_next_user_rec (cursor=0x7f3acebfd420, mtr=0x7f3acebfd5e0) at /mnt/workspace/percona-server-5.6-param/BUILD_TYPE/debug/Host/debian-wheezy-x64/Percona-Server/storage/innobase/include/btr0pcur.ic:323
#12 0x0000000000df5a6e in dict_stats_analyze_index_level (index=0x3393d18, level=0, n_diff=0x3496418, total_recs=0x7f3acebfdac0, total_pages=0x7f3acebfdab8, n_diff_boundaries=0x0, mtr=0x7f3acebfd5e0) at /mnt/workspace/percona-server-5.6-param/BUILD_TYPE/debug/Host/debian-wheezy-x64/Percona-Server/storage/innobase/dict/dict0s...

review: Needs Fixing

[Laurynas Biveinis (laurynas-biveinis)]

Alexey -

> >>>>>> - Do you agree that os_atomic_load_ulint() / os_atomic_store_ulint() do
> >>>>>> not do what they promise to do?
> >>>>>
> >>>>>
> >>>>> Yes.
> >>>>
> >>>> OK, so we agree that naming is unfortunate.
> >>>
> >>>
> >>> Vanishingly slightly so, due to the alignment issue, which I believe is
> >> mostly theoretical, but nevertheless am ready to address now.
> >>>
> >>
> >> It doesn't do anything to enforce atomicity, does it? I.e. the following
> >> implementation would be equally "atomic":
> >>
> >> ulint
> >> os_atomic_load_ulint(ulint *ptr)
> >> {
> >> printf("Hello, world!\n");
> >> return(*ptr);
> >> }
> >
> >
> > Yes, it would be equally atomic (ignoring visibility and ordering) on x86_64
> as long as pointer is aligned.
> >
>
> So... we don't need it after all?

But we don't want to ignore visibility and ordering.

> >>>> 1. Why do we need os_atomic_load_ulint() in buf_get_total_stat(), for
> >>>> example? Here's an example of a valid answer: "Because that will result
> >>>> in incorrect values being used in case ...". And some examples of
> >>>> invalid answers: "non-cache-coherent architectures, visibility, memory
> >>>> model, sunspots, crop circles, global warming, ...".
> >>>
> >>>
> >>> We have gone through this with a disassembly example already, haven't we?
> >> We need os_atomic_load_ulint() because we don't want a dirty read. We may
> >> well decide that a dirty read there is fine and then replace it. But
> that's
> >> orthogonal to what is this primitive and why.
> >>>
> >>
> >> We need an atomic read rather than os_atomic_load_ulint() for the above
> >> reasons. An it will be atomic without using any helper functions.
> >
> >
> > OK, so is the problem that I wanted to introduce the primitives for such
> access, that would also document how is the variable accessed, and that they
> don't have to do much besides a compiler barrier on x86_64?
> >
>
> Yes, the problem is that you introduce primitives that basically do
> nothing, and then use those primitives unnecessarily and inconsistently.
> Which in turn leads to blowing up the patch size, increased maintenance
> burden, and opens the door for wrong assumptions made when reading the
> existing code and implementing new code.

OK, now I see your concerns, and understand a big part of them but not all of them. What wrong assumptions are encouraged by the primitives?

> >> Since
> >> I see no answer in the form "Because that will result in incorrect
> >> values being used in case ...", I assume you don't have an answer to
> >> that question.
> >
> >
> > I know that they are not resulting in incorrect values currently, and that
> the worst can happen in x86_64 with the most of possible future code changes
> is that the value loads could be moved earlier, resulting in more out-of-date
> values used. This and the fact that accessing the variable through the
> primitive serves as self-documentation seem good enough reasons to me.
> >
>
> Whether they are more "out-of-date" or less "out-of-date" depends on the
> definition of "date". By defining "date" as the "point in time when the
> function is called", "more out-of-date" can be easily...

[Alexey Kopytov (akopytov)]

Hi Laurynas,

On Tue, 17 Sep 2013 12:41:18 -0000, Laurynas Biveinis wrote:
> Alexey -
>
>
>>>>>>>> - Do you agree that os_atomic_load_ulint() / os_atomic_store_ulint() do
>>>>>>>> not do what they promise to do?
>>>>>>>
>>>>>>>
>>>>>>> Yes.
>>>>>>
>>>>>> OK, so we agree that naming is unfortunate.
>>>>>
>>>>>
>>>>> Vanishingly slightly so, due to the alignment issue, which I believe is
>>>> mostly theoretical, but nevertheless am ready to address now.
>>>>>
>>>>
>>>> It doesn't do anything to enforce atomicity, does it? I.e. the following
>>>> implementation would be equally "atomic":
>>>>
>>>> ulint
>>>> os_atomic_load_ulint(ulint *ptr)
>>>> {
>>>> printf("Hello, world!\n");
>>>> return(*ptr);
>>>> }
>>>
>>>
>>> Yes, it would be equally atomic (ignoring visibility and ordering) on x86_64
>> as long as pointer is aligned.
>>>
>>
>> So... we don't need it after all?
>
>
> But we don't want to ignore visibility and ordering.
>

Yeah, you forgot non-cache-coherent architectures.

This discussion has been running in circles for almost a week now, and I
have a feeling you deliberately keep it this way. Since I have not been
presented any technical arguments for keeping that code, and have better
things to do, I'm going to wrap up the democrazy and stop it forcefully.

I will not approve this MP with "atomic" primitives present in the code.
The discussion is over.

[Laurynas Biveinis (laurynas-biveinis)]

Alexey -

> This discussion has been running in circles for almost a week now, and I
> have a feeling you deliberately keep it this way. Since I have not been
> presented any technical arguments for keeping that code, and have better
> things to do, I'm going to wrap up the democrazy and stop it forcefully.
>
> I will not approve this MP with "atomic" primitives present in the code.
> The discussion is over.

I did not do anything to deserve this kind of treatment. Your feeling on me keeping this deliberately is plain wrong (What do I have to gain? Minus one week of my copious time? Smug feeling of being right?). I have addressed every single your comment without hesitation and coming from an assumption that you are right in this MP and tens of MPs before.

I have tried my best to explain why the code is correct. You seem to disagree but I have trouble understanding why. I am well within in my rights to ask you to explain further and the burden is on you to show why the code is wrong. Hence your refusal to continue the review is stalling the review right now. Please continue the technical discussion.

[Laurynas Biveinis (laurynas-biveinis)]

Repushed. Changes from the 2nd push. Not a resubmission yet.

    - Removed ut_ad(mutex_own(&buf_pool->zip_free_mutex)) from
      buf_pool_contains_zip().
    - Fixed comment typos.
    - Reverted in_LRU_list and in_unzip_LRU_list changes, with the
      exception of an extra assert in buf_page_set_sticky().
    - Reverted spurious whitespace changes.
    - Removed spurious diagnostic printf from
      buf_pool_validate_instance().
    - Fixed locking in buf_block_try_discard_uncompressed().
    - Reverted redundant locking changes in buf_page_get_zip() and
      buf_page_get_gen().
    - Removed buf_enter_zip_mutex_for_page().
    - Removed os_atomic_load_ulint() uses from from
      buf_get_total_stat(), buf_print_instance(),
      buf_stat_get_pool_info(), buf_LRU_evict_from_unzip_LRU(), the
      last instance in buf_read_recv_pages(), and.
      srv_mon_process_existing_counter().

[Alexey Kopytov (akopytov)]

review: Approve

=== modified file 'Percona-Server/storage/innobase/btr/btr0cur.cc'
--- Percona-Server/storage/innobase/btr/btr0cur.cc	2013-09-06 13:40:39 +0000
+++ Percona-Server/storage/innobase/btr/btr0cur.cc	2013-09-20 05:29:11 +0000
@@ -4503,12 +4503,14 @@
 	buf_pool_t*	buf_pool = buf_pool_from_block(block);
 	ulint		space	= buf_block_get_space(block);
 	ulint		page_no	= buf_block_get_page_no(block);
+	bool		freed	= false;
 
 	ut_ad(mtr_memo_contains(mtr, block, MTR_MEMO_PAGE_X_FIX));
 
 	mtr_commit(mtr);
 
-	buf_pool_mutex_enter(buf_pool);
+	mutex_enter(&buf_pool->LRU_list_mutex);
+	mutex_enter(&block->mutex);
 
 	/* Only free the block if it is still allocated to
 	the same file page. */
@@ -4518,16 +4520,26 @@
 	    && buf_block_get_space(block) == space
 	    && buf_block_get_page_no(block) == page_no) {
 
-		if (!buf_LRU_free_page(&block->page, all)
-		    && all && block->page.zip.data) {
+		freed = buf_LRU_free_page(&block->page, all);
+
+		if (!freed && all && block->page.zip.data
+		    /* Now, buf_LRU_free_page() may release mutexes
+		    temporarily */
+		    && buf_block_get_state(block) == BUF_BLOCK_FILE_PAGE
+		    && buf_block_get_space(block) == space
+		    && buf_block_get_page_no(block) == page_no) {
+
 			/* Attempt to deallocate the uncompressed page
 			if the whole block cannot be deallocted. */
-
-			buf_LRU_free_page(&block->page, false);
+			freed = buf_LRU_free_page(&block->page, false);
 		}
 	}
 
-	buf_pool_mutex_exit(buf_pool);
+	if (!freed) {
+		mutex_exit(&buf_pool->LRU_list_mutex);
+	}
+
+	mutex_exit(&block->mutex);
 }
 
 /*******************************************************************//**

=== modified file 'Percona-Server/storage/innobase/btr/btr0sea.cc'
--- Percona-Server/storage/innobase/btr/btr0sea.cc	2013-09-06 13:40:39 +0000
+++ Percona-Server/storage/innobase/btr/btr0sea.cc	2013-09-20 05:29:11 +0000
@@ -1922,19 +1922,15 @@
 
 	rec_offs_init(offsets_);
 
-	buf_pool_mutex_enter_all();
-
 	cell_count = hash_get_n_cells(btr_search_sys->hash_tables[t]);
 
 	for (i = 0; i < cell_count; i++) {
 		/* We release btr_search_latch every once in a while to
 		give other queries a chance to run. */
 		if ((i != 0) && ((i % chunk_size) == 0)) {
-			buf_pool_mutex_exit_all();
 			btr_search_x_unlock_all();
 			os_thread_yield();
 			btr_search_x_lock_all();
-			buf_pool_mutex_enter_all();
 		}
 
 		node = (ha_node_t*)
@@ -1942,7 +1938,7 @@
 					  i)->node;
 
 		for (; node != NULL; node = node->next) {
-			const buf_block_t*	block
+			buf_block_t*	block
 				= buf_block_align((byte*) node->data);
 			const buf_block_t*	hash_block;
 			buf_pool_t*		buf_pool;
@@ -1983,6 +1979,8 @@
 				     == BUF_BLOCK_REMOVE_HASH);
 			}
 
+			mutex_enter(&block->mutex);
+
 			ut_a(!dict_index_is_ibuf(block->index));
 
 			page_index_id = btr_page_get_index_id(block->frame);
@@ -2038,6 +2036,8 @@
 					n_page_dumps++;
 				}
 			}
+
+			mutex_exit(&block->mutex);
 		}
 	}
 
@@ -2047,11 +2047,9 @@
 		/* We release btr_search_latch every once in a while to
 		give other queries a chance to run. */
 		if (i != 0) {
-			buf_pool_mutex_exit_all();
 			btr_search_x_unlock_all();
 			os_thread_yield();
 			btr_search_x_lock_all();
-			buf_pool_mutex_enter_all();
 		}
 
 		if (!ha_validate(btr_search_sys->hash_tables[t], i,
@@ -2060,7 +2058,6 @@
 		}
 	}
 
-	buf_pool_mutex_exit_all();
 	if (UNIV_LIKELY_NULL(heap)) {
 		mem_heap_free(heap);
 	}

=== modified file 'Percona-Server/storage/innobase/buf/buf0buddy.cc'
--- Percona-Server/storage/innobase/buf/buf0buddy.cc	2013-08-06 15:16:34 +0000
+++ Percona-Server/storage/innobase/buf/buf0buddy.cc	2013-09-20 05:29:11 +0000
@@ -205,7 +205,7 @@
 {
 	const ulint	size	= BUF_BUDDY_LOW << i;
 
-	ut_ad(buf_pool_mutex_own(buf_pool));
+	ut_ad(mutex_own(&buf_pool->zip_free_mutex));
 	ut_ad(!ut_align_offset(buf, size));
 	ut_ad(i >= buf_buddy_get_slot(UNIV_ZIP_SIZE_MIN));
 
@@ -278,7 +278,7 @@
 	ulint			i)		/*!< in: index of
 						buf_pool->zip_free[] */
 {
-	ut_ad(buf_pool_mutex_own(buf_pool));
+	ut_ad(mutex_own(&buf_pool->zip_free_mutex));
 	ut_ad(buf_pool->zip_free[i].start != buf);
 
 	buf_buddy_stamp_free(buf, i);
@@ -297,7 +297,7 @@
 	ulint			i)		/*!< in: index of
 						buf_pool->zip_free[] */
 {
-	ut_ad(buf_pool_mutex_own(buf_pool));
+	ut_ad(mutex_own(&buf_pool->zip_free_mutex));
 	ut_ad(buf_buddy_check_free(buf_pool, buf, i));
 
 	UT_LIST_REMOVE(list, buf_pool->zip_free[i], buf);
@@ -316,7 +316,7 @@
 {
 	buf_buddy_free_t*	buf;
 
-	ut_ad(buf_pool_mutex_own(buf_pool));
+	ut_ad(mutex_own(&buf_pool->zip_free_mutex));
 	ut_a(i < BUF_BUDDY_SIZES);
 	ut_a(i >= buf_buddy_get_slot(UNIV_ZIP_SIZE_MIN));
 
@@ -369,10 +369,11 @@
 	buf_page_t*	bpage;
 	buf_block_t*	block;
 
-	ut_ad(buf_pool_mutex_own(buf_pool));
 	ut_ad(!mutex_own(&buf_pool->zip_mutex));
 	ut_a(!ut_align_offset(buf, UNIV_PAGE_SIZE));
 
+	mutex_enter(&buf_pool->zip_hash_mutex);
+
 	HASH_SEARCH(hash, buf_pool->zip_hash, fold, buf_page_t*, bpage,
 		    ut_ad(buf_page_get_state(bpage) == BUF_BLOCK_MEMORY
 			  && bpage->in_zip_hash && !bpage->in_page_hash),
@@ -384,6 +385,8 @@
 	ut_d(bpage->in_zip_hash = FALSE);
 	HASH_DELETE(buf_page_t, hash, buf_pool->zip_hash, fold, bpage);
 
+	mutex_exit(&buf_pool->zip_hash_mutex);
+
 	ut_d(memset(buf, 0, UNIV_PAGE_SIZE));
 	UNIV_MEM_INVALID(buf, UNIV_PAGE_SIZE);
 
@@ -406,7 +409,6 @@
 {
 	buf_pool_t*	buf_pool = buf_pool_from_block(block);
 	const ulint	fold = BUF_POOL_ZIP_FOLD(block);
-	ut_ad(buf_pool_mutex_own(buf_pool));
 	ut_ad(!mutex_own(&buf_pool->zip_mutex));
 	ut_ad(buf_block_get_state(block) == BUF_BLOCK_READY_FOR_USE);
 
@@ -418,7 +420,10 @@
 	ut_ad(!block->page.in_page_hash);
 	ut_ad(!block->page.in_zip_hash);
 	ut_d(block->page.in_zip_hash = TRUE);
+
+	mutex_enter(&buf_pool->zip_hash_mutex);
 	HASH_INSERT(buf_page_t, hash, buf_pool->zip_hash, fold, &block->page);
+	mutex_exit(&buf_pool->zip_hash_mutex);
 
 	ut_d(buf_pool->buddy_n_frames++);
 }
@@ -438,6 +443,7 @@
 					of buf_pool->zip_free[] */
 {
 	ulint	offs	= BUF_BUDDY_LOW << j;
+	ut_ad(mutex_own(&buf_pool->zip_free_mutex));
 	ut_ad(j <= BUF_BUDDY_SIZES);
 	ut_ad(i >= buf_buddy_get_slot(UNIV_ZIP_SIZE_MIN));
 	ut_ad(j >= i);
@@ -461,8 +467,8 @@
 
 /**********************************************************************//**
 Allocate a block.  The thread calling this function must hold
-buf_pool->mutex and must not hold buf_pool->zip_mutex or any block->mutex.
-The buf_pool_mutex may be released and reacquired.
+buf_pool->LRU_list_mutex and must not hold buf_pool->zip_mutex or any
+block->mutex.  The buf_pool->LRU_list_mutex may be released and reacquired.
 @return	allocated block, never NULL */
 UNIV_INTERN
 void*
@@ -474,23 +480,25 @@
 	ibool*		lru)		/*!< in: pointer to a variable that
 					will be assigned TRUE if storage was
 					allocated from the LRU list and
-					buf_pool->mutex was temporarily
-					released */
+					buf_pool->LRU_list_mutex was
+					temporarily released */
 {
 	buf_block_t*	block;
 
 	ut_ad(lru);
-	ut_ad(buf_pool_mutex_own(buf_pool));
+	ut_ad(mutex_own(&buf_pool->LRU_list_mutex));
 	ut_ad(!mutex_own(&buf_pool->zip_mutex));
 	ut_ad(i >= buf_buddy_get_slot(UNIV_ZIP_SIZE_MIN));
 
 	if (i < BUF_BUDDY_SIZES) {
 		/* Try to allocate from the buddy system. */
+		mutex_enter(&buf_pool->zip_free_mutex);
 		block = (buf_block_t*) buf_buddy_alloc_zip(buf_pool, i);
 
 		if (block) {
 			goto func_exit;
 		}
+		mutex_exit(&buf_pool->zip_free_mutex);
 	}
 
 	/* Try allocating from the buf_pool->free list. */
@@ -502,24 +510,28 @@
 	}
 
 	/* Try replacing an uncompressed page in the buffer pool. */
-	buf_pool_mutex_exit(buf_pool);
+	mutex_exit(&buf_pool->LRU_list_mutex);
 	block = buf_LRU_get_free_block(buf_pool);
 	*lru = TRUE;
-	buf_pool_mutex_enter(buf_pool);
+	mutex_enter(&buf_pool->LRU_list_mutex);
 
 alloc_big:
 	buf_buddy_block_register(block);
 
+	mutex_enter(&buf_pool->zip_free_mutex);
 	block = (buf_block_t*) buf_buddy_alloc_from(
 		buf_pool, block->frame, i, BUF_BUDDY_SIZES);
 
 func_exit:
 	buf_pool->buddy_stat[i].used++;
+	mutex_exit(&buf_pool->zip_free_mutex);
+
 	return(block);
 }
 
 /**********************************************************************//**
-Try to relocate a block.
+Try to relocate a block.  The caller must hold zip_free_mutex, and this
+function will release and lock it again.
 @return	true if relocated */
 static
 bool
@@ -536,8 +548,9 @@
 	ib_mutex_t*	mutex;
 	ulint		space;
 	ulint		offset;
+	rw_lock_t*	hash_lock;
 
-	ut_ad(buf_pool_mutex_own(buf_pool));
+	ut_ad(mutex_own(&buf_pool->zip_free_mutex));
 	ut_ad(!mutex_own(&buf_pool->zip_mutex));
 	ut_ad(!ut_align_offset(src, size));
 	ut_ad(!ut_align_offset(dst, size));
@@ -556,7 +569,9 @@
 
 	ut_ad(space != BUF_BUDDY_STAMP_FREE);
 
-	bpage = buf_page_hash_get(buf_pool, space, offset);
+	mutex_exit(&buf_pool->zip_free_mutex);
+	/* Lock page hash to prevent a relocation for the target page */
+	bpage = buf_page_hash_get_s_locked(buf_pool, space, offset, &hash_lock);
 
 	if (!bpage || bpage->zip.data != src) {
 		/* The block has probably been freshly
@@ -564,6 +579,10 @@
 		added to buf_pool->page_hash yet.  Obviously,
 		it cannot be relocated. */
 
+		if (bpage) {
+			rw_lock_s_unlock(hash_lock);
+		}
+		mutex_enter(&buf_pool->zip_free_mutex);
 		return(false);
 	}
 
@@ -573,6 +592,8 @@
 		For the sake of simplicity, give up. */
 		ut_ad(page_zip_get_size(&bpage->zip) < size);
 
+		rw_lock_s_unlock(hash_lock);
+		mutex_enter(&buf_pool->zip_free_mutex);
 		return(false);
 	}
 
@@ -584,6 +605,10 @@
 
 	mutex_enter(mutex);
 
+	rw_lock_s_unlock(hash_lock);
+
+	mutex_enter(&buf_pool->zip_free_mutex);
+
 	if (buf_page_can_relocate(bpage)) {
 		/* Relocate the compressed page. */
 		ullint	usec	= ut_time_us(NULL);
@@ -618,17 +643,19 @@
 {
 	buf_buddy_free_t*	buddy;
 
-	ut_ad(buf_pool_mutex_own(buf_pool));
 	ut_ad(!mutex_own(&buf_pool->zip_mutex));
 	ut_ad(i <= BUF_BUDDY_SIZES);
 	ut_ad(i >= buf_buddy_get_slot(UNIV_ZIP_SIZE_MIN));
+
+	mutex_enter(&buf_pool->zip_free_mutex);
+
 	ut_ad(buf_pool->buddy_stat[i].used > 0);
-
 	buf_pool->buddy_stat[i].used--;
 recombine:
 	UNIV_MEM_ASSERT_AND_ALLOC(buf, BUF_BUDDY_LOW << i);
 
 	if (i == BUF_BUDDY_SIZES) {
+		mutex_exit(&buf_pool->zip_free_mutex);
 		buf_buddy_block_free(buf_pool, buf);
 		return;
 	}
@@ -695,4 +722,5 @@
 	buf_buddy_add_to_free(buf_pool,
 			      reinterpret_cast<buf_buddy_free_t*>(buf),
 			      i);
+	mutex_exit(&buf_pool->zip_free_mutex);
 }

=== modified file 'Percona-Server/storage/innobase/buf/buf0buf.cc'
--- Percona-Server/storage/innobase/buf/buf0buf.cc	2013-09-02 10:01:38 +0000
+++ Percona-Server/storage/innobase/buf/buf0buf.cc	2013-09-20 05:29:11 +0000
@@ -119,24 +119,9 @@
 
 		Buffer pool struct
 		------------------
-The buffer buf_pool contains a single mutex which protects all the
+The buffer buf_pool contains several mutexes which protect all the
 control data structures of the buf_pool. The content of a buffer frame is
 protected by a separate read-write lock in its control block, though.
-These locks can be locked and unlocked without owning the buf_pool->mutex.
-The OS events in the buf_pool struct can be waited for without owning the
-buf_pool->mutex.
-
-The buf_pool->mutex is a hot-spot in main memory, causing a lot of
-memory bus traffic on multiprocessor systems when processors
-alternately access the mutex. On our Pentium, the mutex is accessed
-maybe every 10 microseconds. We gave up the solution to have mutexes
-for each control block, for instance, because it seemed to be
-complicated.
-
-A solution to reduce mutex contention of the buf_pool->mutex is to
-create a separate mutex for the page hash table. On Pentium,
-accessing the hash table takes 2 microseconds, about half
-of the total buf_pool->mutex hold time.
 
 		Control blocks
 		--------------
@@ -311,6 +296,11 @@
 UNIV_INTERN mysql_pfs_key_t	buffer_block_mutex_key;
 UNIV_INTERN mysql_pfs_key_t	buf_pool_mutex_key;
 UNIV_INTERN mysql_pfs_key_t	buf_pool_zip_mutex_key;
+UNIV_INTERN mysql_pfs_key_t	buf_pool_flush_state_mutex_key;
+UNIV_INTERN mysql_pfs_key_t	buf_pool_LRU_list_mutex_key;
+UNIV_INTERN mysql_pfs_key_t	buf_pool_free_list_mutex_key;
+UNIV_INTERN mysql_pfs_key_t	buf_pool_zip_free_mutex_key;
+UNIV_INTERN mysql_pfs_key_t	buf_pool_zip_hash_mutex_key;
 UNIV_INTERN mysql_pfs_key_t	flush_list_mutex_key;
 #endif /* UNIV_PFS_MUTEX */
 
@@ -1186,7 +1176,6 @@
 	buf_chunk_t*	chunk = buf_pool->chunks;
 
 	ut_ad(buf_pool);
-	ut_ad(buf_pool_mutex_own(buf_pool));
 	for (n = buf_pool->n_chunks; n--; chunk++) {
 
 		buf_block_t* block = buf_chunk_contains_zip(chunk, data);
@@ -1265,8 +1254,6 @@
 	ulint	i;
 	ulint	curr_size = 0;
 
-	buf_pool_mutex_enter_all();
-
 	for (i = 0; i < srv_buf_pool_instances; i++) {
 		buf_pool_t*	buf_pool;
 
@@ -1276,8 +1263,6 @@
 
 	srv_buf_pool_curr_size = curr_size;
 	srv_buf_pool_old_size = srv_buf_pool_size;
-
-	buf_pool_mutex_exit_all();
 }
 
 /********************************************************************//**
@@ -1297,12 +1282,18 @@
 
 	/* 1. Initialize general fields
 	------------------------------- */
-	mutex_create(buf_pool_mutex_key,
-		     &buf_pool->mutex, SYNC_BUF_POOL);
+	mutex_create(buf_pool_LRU_list_mutex_key,
+		     &buf_pool->LRU_list_mutex, SYNC_BUF_LRU_LIST);
+	mutex_create(buf_pool_free_list_mutex_key,
+		     &buf_pool->free_list_mutex, SYNC_BUF_FREE_LIST);
+	mutex_create(buf_pool_zip_free_mutex_key,
+		     &buf_pool->zip_free_mutex, SYNC_BUF_ZIP_FREE);
+	mutex_create(buf_pool_zip_hash_mutex_key,
+		     &buf_pool->zip_hash_mutex, SYNC_BUF_ZIP_HASH);
 	mutex_create(buf_pool_zip_mutex_key,
 		     &buf_pool->zip_mutex, SYNC_BUF_BLOCK);
-
-	buf_pool_mutex_enter(buf_pool);
+	mutex_create(buf_pool_flush_state_mutex_key,
+		     &buf_pool->flush_state_mutex, SYNC_BUF_FLUSH_STATE);
 
 	if (buf_pool_size > 0) {
 		buf_pool->n_chunks = 1;
@@ -1316,8 +1307,6 @@
 			mem_free(chunk);
 			mem_free(buf_pool);
 
-			buf_pool_mutex_exit(buf_pool);
-
 			return(DB_ERROR);
 		}
 
@@ -1361,8 +1350,6 @@
 
 	buf_pool->try_LRU_scan = TRUE;
 
-	buf_pool_mutex_exit(buf_pool);
-
 	return(DB_SUCCESS);
 }
 
@@ -1537,7 +1524,7 @@
 
 	fold = buf_page_address_fold(bpage->space, bpage->offset);
 
-	ut_ad(buf_pool_mutex_own(buf_pool));
+	ut_ad(mutex_own(&buf_pool->LRU_list_mutex));
 	ut_ad(buf_page_hash_lock_held_x(buf_pool, bpage));
 	ut_ad(mutex_own(buf_page_get_mutex(bpage)));
 	ut_a(buf_page_get_io_fix(bpage) == BUF_IO_NONE);
@@ -1670,14 +1657,14 @@
 			return(bpage);
 		}
 		/* Add to an existing watch. */
+		mutex_enter(&buf_pool->zip_mutex);
 		bpage->buf_fix_count++;
+		mutex_exit(&buf_pool->zip_mutex);
 		return(NULL);
 	}
 
 	/* From this point this function becomes fairly heavy in terms
-	of latching. We acquire the buf_pool mutex as well as all the
-	hash_locks. buf_pool mutex is needed because any changes to
-	the page_hash must be covered by it and hash_locks are needed
+	of latching. We acquire all the hash_locks. They are needed
 	because we don't want to read any stale information in
 	buf_pool->watch[]. However, it is not in the critical code path
 	as this function will be called only by the purge thread. */
@@ -1686,18 +1673,16 @@
 	/* To obey latching order first release the hash_lock. */
 	rw_lock_x_unlock(hash_lock);
 
-	buf_pool_mutex_enter(buf_pool);
 	hash_lock_x_all(buf_pool->page_hash);
 
 	/* We have to recheck that the page
 	was not loaded or a watch set by some other
 	purge thread. This is because of the small
 	time window between when we release the
-	hash_lock to acquire buf_pool mutex above. */
+	hash_lock to acquire all the hash locks above. */
 
 	bpage = buf_page_hash_get_low(buf_pool, space, offset, fold);
 	if (UNIV_LIKELY_NULL(bpage)) {
-		buf_pool_mutex_exit(buf_pool);
 		hash_unlock_x_all_but(buf_pool->page_hash, hash_lock);
 		goto page_found;
 	}
@@ -1716,21 +1701,19 @@
 			ut_ad(!bpage->in_page_hash);
 			ut_ad(bpage->buf_fix_count == 0);
 
-			/* bpage is pointing to buf_pool->watch[],
-			which is protected by buf_pool->mutex.
-			Normally, buf_page_t objects are protected by
-			buf_block_t::mutex or buf_pool->zip_mutex or both. */
+			mutex_enter(&buf_pool->zip_mutex);
 
 			bpage->state = BUF_BLOCK_ZIP_PAGE;
 			bpage->space = space;
 			bpage->offset = offset;
 			bpage->buf_fix_count = 1;
 
+			mutex_exit(&buf_pool->zip_mutex);
+
 			ut_d(bpage->in_page_hash = TRUE);
 			HASH_INSERT(buf_page_t, hash, buf_pool->page_hash,
 				    fold, bpage);
 
-			buf_pool_mutex_exit(buf_pool);
 			/* Once the sentinel is in the page_hash we can
 			safely release all locks except just the
 			relevant hash_lock */
@@ -1777,7 +1760,8 @@
 	ut_ad(rw_lock_own(hash_lock, RW_LOCK_EX));
 #endif /* UNIV_SYNC_DEBUG */
 
-	ut_ad(buf_pool_mutex_own(buf_pool));
+	ut_ad(buf_page_get_state(watch) == BUF_BLOCK_ZIP_PAGE);
+	ut_ad(buf_own_zip_mutex_for_page(watch));
 
 	HASH_DELETE(buf_page_t, hash, buf_pool->page_hash, fold, watch);
 	ut_d(watch->in_page_hash = FALSE);
@@ -1801,13 +1785,6 @@
 	rw_lock_t*	hash_lock = buf_page_hash_lock_get(buf_pool,
 							     fold);
 
-	/* We only need to have buf_pool mutex in case where we end
-	up calling buf_pool_watch_remove but to obey latching order
-	we acquire it here before acquiring hash_lock. This should
-	not cause too much grief as this function is only ever
-	called from the purge thread. */
-	buf_pool_mutex_enter(buf_pool);
-
 	rw_lock_x_lock(hash_lock);
 
 	bpage = buf_page_hash_get_low(buf_pool, space, offset, fold);
@@ -1825,12 +1802,13 @@
 	} else {
 		ut_a(bpage->buf_fix_count > 0);
 
+		mutex_enter(&buf_pool->zip_mutex);
 		if (UNIV_LIKELY(!--bpage->buf_fix_count)) {
 			buf_pool_watch_remove(buf_pool, fold, bpage);
 		}
+		mutex_exit(&buf_pool->zip_mutex);
 	}
 
-	buf_pool_mutex_exit(buf_pool);
 	rw_lock_x_unlock(hash_lock);
 }
 
@@ -1877,13 +1855,13 @@
 {
 	buf_pool_t*	buf_pool = buf_pool_from_bpage(bpage);
 
-	buf_pool_mutex_enter(buf_pool);
+	mutex_enter(&buf_pool->LRU_list_mutex);
 
 	ut_a(buf_page_in_file(bpage));
 
 	buf_LRU_make_block_young(bpage);
 
-	buf_pool_mutex_exit(buf_pool);
+	mutex_exit(&buf_pool->LRU_list_mutex);
 }
 
 /********************************************************************//**
@@ -1897,10 +1875,6 @@
 	buf_page_t*	bpage)		/*!< in/out: buffer block of a
 					file page */
 {
-#ifdef UNIV_DEBUG
-	buf_pool_t*	buf_pool = buf_pool_from_bpage(bpage);
-	ut_ad(!buf_pool_mutex_own(buf_pool));
-#endif /* UNIV_DEBUG */
 	ut_a(buf_page_in_file(bpage));
 
 	if (buf_page_peek_if_too_old(bpage)) {
@@ -1921,16 +1895,12 @@
 	buf_block_t*	block;
 	buf_pool_t*	buf_pool = buf_pool_get(space, offset);
 
-	buf_pool_mutex_enter(buf_pool);
-
 	block = (buf_block_t*) buf_page_hash_get(buf_pool, space, offset);
 
 	if (block && buf_block_get_state(block) == BUF_BLOCK_FILE_PAGE) {
 		ut_ad(!buf_pool_watch_is_sentinel(buf_pool, &block->page));
 		block->check_index_page_at_flush = FALSE;
 	}
-
-	buf_pool_mutex_exit(buf_pool);
 }
 
 #if defined UNIV_DEBUG_FILE_ACCESSES || defined UNIV_DEBUG
@@ -2014,21 +1984,32 @@
 	buf_page_t*	bpage;
 	buf_pool_t*	buf_pool = buf_pool_get(space, offset);
 
-	/* Since we need to acquire buf_pool mutex to discard
-	the uncompressed frame and because page_hash mutex resides
-	below buf_pool mutex in sync ordering therefore we must
-	first release the page_hash mutex. This means that the
-	block in question can move out of page_hash. Therefore
-	we need to check again if the block is still in page_hash. */
-	buf_pool_mutex_enter(buf_pool);
+	/* Since we need to acquire buf_pool->LRU_list_mutex to discard
+	the uncompressed frame and because page_hash mutex resides below
+	buf_pool->LRU_list_mutex in sync ordering therefore we must first
+	release the page_hash mutex. This means that the block in question
+	can move out of page_hash. Therefore we need to check again if the
+	block is still in page_hash. */
+
+	mutex_enter(&buf_pool->LRU_list_mutex);
 
 	bpage = buf_page_hash_get(buf_pool, space, offset);
 
 	if (bpage) {
-		buf_LRU_free_page(bpage, false);
+
+		ib_mutex_t* block_mutex = buf_page_get_mutex(bpage);
+
+		mutex_enter(block_mutex);
+
+		if (buf_LRU_free_page(bpage, false)) {
+
+			mutex_exit(block_mutex);
+			return;
+		}
+		mutex_exit(block_mutex);
 	}
 
-	buf_pool_mutex_exit(buf_pool);
+	mutex_exit(&buf_pool->LRU_list_mutex);
 }
 
 /********************************************************************//**
@@ -2324,7 +2305,8 @@
 			ut_ad(block->frame == page_align(ptr));
 #ifdef UNIV_DEBUG
 			/* A thread that updates these fields must
-			hold buf_pool->mutex and block->mutex.  Acquire
+			hold one of the buf_pool mutexes, depending on the
+			page state, and block->mutex.  Acquire
 			only the latter. */
 			mutex_enter(&block->mutex);
 
@@ -2708,6 +2690,7 @@
 		buf_page_t*	bpage;
 
 	case BUF_BLOCK_FILE_PAGE:
+		ut_ad(block_mutex != &buf_pool->zip_mutex);
 		break;
 
 	case BUF_BLOCK_ZIP_PAGE:
@@ -2721,13 +2704,14 @@
 		}
 
 		bpage = &block->page;
+		ut_ad(block_mutex == &buf_pool->zip_mutex);
 
 		if (bpage->buf_fix_count
 		    || buf_page_get_io_fix(bpage) != BUF_IO_NONE) {
 			/* This condition often occurs when the buffer
 			is not buffer-fixed, but I/O-fixed by
 			buf_page_init_for_read(). */
-			mutex_exit(block_mutex);
+			mutex_exit(&buf_pool->zip_mutex);
 wait_until_unfixed:
 			/* The block is buffer-fixed or I/O-fixed.
 			Try again later. */
@@ -2737,11 +2721,11 @@
 		}
 
 		/* Allocate an uncompressed page. */
-		mutex_exit(block_mutex);
+		mutex_exit(&buf_pool->zip_mutex);
 		block = buf_LRU_get_free_block(buf_pool);
 		ut_a(block);
 
-		buf_pool_mutex_enter(buf_pool);
+		mutex_enter(&buf_pool->LRU_list_mutex);
 
 		/* As we have released the page_hash lock and the
 		block_mutex to allocate an uncompressed page it is
@@ -2754,22 +2738,25 @@
 						   offset, fold);
 
 		mutex_enter(&block->mutex);
+		mutex_enter(&buf_pool->zip_mutex);
+
 		if (bpage != hash_bpage
 		    || bpage->buf_fix_count
 		    || buf_page_get_io_fix(bpage) != BUF_IO_NONE) {
 			buf_LRU_block_free_non_file_page(block);
-			buf_pool_mutex_exit(buf_pool);
+			mutex_exit(&buf_pool->LRU_list_mutex);
+			mutex_exit(&buf_pool->zip_mutex);
 			rw_lock_x_unlock(hash_lock);
 			mutex_exit(&block->mutex);
 
 			if (bpage != hash_bpage) {
 				/* The buf_pool->page_hash was modified
-				while buf_pool->mutex was not held
+				while buf_pool->LRU_list_mutex was not held
 				by this thread. */
 				goto loop;
 			} else {
 				/* The block was buffer-fixed or
-				I/O-fixed while buf_pool->mutex was
+				I/O-fixed while buf_pool->LRU_list_mutex was
 				not held by this thread. */
 				goto wait_until_unfixed;
 			}
@@ -2778,8 +2765,6 @@
 		/* Move the compressed page from bpage to block,
 		and uncompress it. */
 
-		mutex_enter(&buf_pool->zip_mutex);
-
 		buf_relocate(bpage, &block->page);
 		buf_block_init_low(block);
 		block->lock_hash_val = lock_rec_hash(space, offset);
@@ -2808,6 +2793,8 @@
 		/* Insert at the front of unzip_LRU list */
 		buf_unzip_LRU_add_block(block, FALSE);
 
+		mutex_exit(&buf_pool->LRU_list_mutex);
+
 		block->page.buf_fix_count = 1;
 		buf_block_set_io_fix(block, BUF_IO_READ);
 		rw_lock_x_lock_inline(&block->lock, 0, file, line);
@@ -2816,8 +2803,7 @@
 
 		rw_lock_x_unlock(hash_lock);
 
-		buf_pool->n_pend_unzip++;
-		buf_pool_mutex_exit(buf_pool);
+		os_atomic_increment_ulint(&buf_pool->n_pend_unzip, 1);
 
 		access_time = buf_page_is_accessed(&block->page);
 		mutex_exit(&block->mutex);
@@ -2826,7 +2812,7 @@
 		buf_page_free_descriptor(bpage);
 
 		/* Decompress the page while not holding
-		buf_pool->mutex or block->mutex. */
+		any buf_pool or block->mutex. */
 
 		/* Page checksum verification is already done when
 		the page is read from disk. Hence page checksum
@@ -2845,12 +2831,10 @@
 		}
 
 		/* Unfix and unlatch the block. */
-		buf_pool_mutex_enter(buf_pool);
 		mutex_enter(&block->mutex);
 		block->page.buf_fix_count--;
 		buf_block_set_io_fix(block, BUF_IO_NONE);
-		buf_pool->n_pend_unzip--;
-		buf_pool_mutex_exit(buf_pool);
+		os_atomic_decrement_ulint(&buf_pool->n_pend_unzip, 1);
 		rw_lock_x_unlock(&block->lock);
 
 		break;
@@ -2885,23 +2869,18 @@
 		insert buffer (change buffer) as much as possible. */
 
 		/* To obey the latching order, release the
-		block->mutex before acquiring buf_pool->mutex. Protect
+		block->mutex before acquiring buf_pool->LRU_list_mutex. Protect
 		the block from changes by temporarily buffer-fixing it
 		for the time we are not holding block->mutex. */
+
 		buf_block_buf_fix_inc(block, file, line);
 		mutex_exit(&block->mutex);
-		buf_pool_mutex_enter(buf_pool);
+		mutex_enter(&buf_pool->LRU_list_mutex);
 		mutex_enter(&block->mutex);
 		buf_block_buf_fix_dec(block);
-		mutex_exit(&block->mutex);
-
-		/* Now we are only holding the buf_pool->mutex,
-		not block->mutex or hash_lock. Blocks cannot be
-		relocated or enter or exit the buf_pool while we
-		are holding the buf_pool->mutex. */
 
 		if (buf_LRU_free_page(&block->page, true)) {
-			buf_pool_mutex_exit(buf_pool);
+			mutex_exit(&block->mutex);
 			rw_lock_x_lock(hash_lock);
 
 			if (mode == BUF_GET_IF_IN_POOL_OR_WATCH) {
@@ -2931,10 +2910,11 @@
 				"innodb_change_buffering_debug evict %u %u\n",
 				(unsigned) space, (unsigned) offset);
 			return(NULL);
+		} else {
+
+			mutex_exit(&buf_pool->LRU_list_mutex);
 		}
 
-		mutex_enter(&block->mutex);
-
 		if (buf_flush_page_try(buf_pool, block)) {
 			fprintf(stderr,
 				"innodb_change_buffering_debug flush %u %u\n",
@@ -2944,8 +2924,6 @@
 		}
 
 		/* Failed to evict the page; change it directly */
-
-		buf_pool_mutex_exit(buf_pool);
 	}
 #endif /* UNIV_DEBUG || UNIV_IBUF_DEBUG */
 
@@ -3420,7 +3398,6 @@
 	buf_page_t*	hash_page;
 
 	ut_ad(buf_pool == buf_pool_get(space, offset));
-	ut_ad(buf_pool_mutex_own(buf_pool));
 
 	ut_ad(mutex_own(&(block->mutex)));
 	ut_a(buf_block_get_state(block) != BUF_BLOCK_FILE_PAGE);
@@ -3455,11 +3432,17 @@
 	if (UNIV_LIKELY(!hash_page)) {
 	} else if (buf_pool_watch_is_sentinel(buf_pool, hash_page)) {
 		/* Preserve the reference count. */
+
+		mutex_enter(&buf_pool->zip_mutex);
+
 		ulint	buf_fix_count = hash_page->buf_fix_count;
 
 		ut_a(buf_fix_count > 0);
 		block->page.buf_fix_count += buf_fix_count;
 		buf_pool_watch_remove(buf_pool, fold, hash_page);
+
+		mutex_exit(&buf_pool->zip_mutex);
+
 	} else {
 		fprintf(stderr,
 			"InnoDB: Error: page %lu %lu already found"
@@ -3469,7 +3452,6 @@
 			(const void*) hash_page, (const void*) block);
 #if defined UNIV_DEBUG || defined UNIV_BUF_DEBUG
 		mutex_exit(&block->mutex);
-		buf_pool_mutex_exit(buf_pool);
 		buf_print();
 		buf_LRU_print();
 		buf_validate();
@@ -3556,7 +3538,7 @@
 	fold = buf_page_address_fold(space, offset);
 	hash_lock = buf_page_hash_lock_get(buf_pool, fold);
 
-	buf_pool_mutex_enter(buf_pool);
+	mutex_enter(&buf_pool->LRU_list_mutex);
 	rw_lock_x_lock(hash_lock);
 
 	watch_page = buf_page_hash_get_low(buf_pool, space, offset, fold);
@@ -3564,6 +3546,7 @@
 		/* The page is already in the buffer pool. */
 		watch_page = NULL;
 err_exit:
+		mutex_exit(&buf_pool->LRU_list_mutex);
 		rw_lock_x_unlock(hash_lock);
 		if (block) {
 			mutex_enter(&block->mutex);
@@ -3596,6 +3579,7 @@
 
 		/* The block must be put to the LRU list, to the old blocks */
 		buf_LRU_add_block(bpage, TRUE/* to old blocks */);
+		mutex_exit(&buf_pool->LRU_list_mutex);
 
 		/* We set a pass-type x-lock on the frame because then
 		the same thread which called for the read operation
@@ -3610,15 +3594,16 @@
 		buf_page_set_io_fix(bpage, BUF_IO_READ);
 
 		if (zip_size) {
-			/* buf_pool->mutex may be released and
+			/* buf_pool->LRU_list_mutex may be released and
 			reacquired by buf_buddy_alloc().  Thus, we
 			must release block->mutex in order not to
 			break the latching order in the reacquisition
-			of buf_pool->mutex.  We also must defer this
+			of buf_pool->LRU_list_mutex.  We also must defer this
 			operation until after the block descriptor has
 			been added to buf_pool->LRU and
 			buf_pool->page_hash. */
 			mutex_exit(&block->mutex);
+			mutex_enter(&buf_pool->LRU_list_mutex);
 			data = buf_buddy_alloc(buf_pool, zip_size, &lru);
 			mutex_enter(&block->mutex);
 			block->page.zip.data = (page_zip_t*) data;
@@ -3630,6 +3615,7 @@
 			after block->page.zip.data is set. */
 			ut_ad(buf_page_belongs_to_unzip_LRU(&block->page));
 			buf_unzip_LRU_add_block(block, TRUE);
+			mutex_exit(&buf_pool->LRU_list_mutex);
 		}
 
 		mutex_exit(&block->mutex);
@@ -3645,8 +3631,9 @@
 		rw_lock_x_lock(hash_lock);
 
 		/* If buf_buddy_alloc() allocated storage from the LRU list,
-		it released and reacquired buf_pool->mutex.  Thus, we must
-		check the page_hash again, as it may have been modified. */
+		it released and reacquired buf_pool->LRU_list_mutex.  Thus, we
+		must check the page_hash again, as it may have been
+		modified. */
 		if (UNIV_UNLIKELY(lru)) {
 
 			watch_page = buf_page_hash_get_low(
@@ -3657,6 +3644,7 @@
 							   watch_page))) {
 
 				/* The block was added by some other thread. */
+				mutex_exit(&buf_pool->LRU_list_mutex);
 				rw_lock_x_unlock(hash_lock);
 				watch_page = NULL;
 				buf_buddy_free(buf_pool, data, zip_size);
@@ -3700,6 +3688,7 @@
 			/* Preserve the reference count. */
 			ulint	buf_fix_count = watch_page->buf_fix_count;
 			ut_a(buf_fix_count > 0);
+			ut_ad(buf_own_zip_mutex_for_page(bpage));
 			bpage->buf_fix_count += buf_fix_count;
 			ut_ad(buf_pool_watch_is_sentinel(buf_pool, watch_page));
 			buf_pool_watch_remove(buf_pool, fold, watch_page);
@@ -3716,15 +3705,15 @@
 #if defined UNIV_DEBUG || defined UNIV_BUF_DEBUG
 		buf_LRU_insert_zip_clean(bpage);
 #endif /* UNIV_DEBUG || UNIV_BUF_DEBUG */
+		mutex_exit(&buf_pool->LRU_list_mutex);
 
 		buf_page_set_io_fix(bpage, BUF_IO_READ);
 
 		mutex_exit(&buf_pool->zip_mutex);
 	}
 
-	buf_pool->n_pend_reads++;
+	os_atomic_increment_ulint(&buf_pool->n_pend_reads, 1);
 func_exit:
-	buf_pool_mutex_exit(buf_pool);
 
 	if (mode == BUF_READ_IBUF_PAGES_ONLY) {
 
@@ -3773,7 +3762,7 @@
 	fold = buf_page_address_fold(space, offset);
 	hash_lock = buf_page_hash_lock_get(buf_pool, fold);
 
-	buf_pool_mutex_enter(buf_pool);
+	mutex_enter(&buf_pool->LRU_list_mutex);
 	rw_lock_x_lock(hash_lock);
 
 	block = (buf_block_t*) buf_page_hash_get_low(
@@ -3790,8 +3779,8 @@
 #endif /* UNIV_DEBUG_FILE_ACCESSES || UNIV_DEBUG */
 
 		/* Page can be found in buf_pool */
-		buf_pool_mutex_exit(buf_pool);
 		rw_lock_x_unlock(hash_lock);
+		mutex_exit(&buf_pool->LRU_list_mutex);
 
 		buf_block_free(free_block);
 
@@ -3827,17 +3816,17 @@
 		ibool	lru;
 
 		/* Prevent race conditions during buf_buddy_alloc(),
-		which may release and reacquire buf_pool->mutex,
+		which may release and reacquire buf_pool->LRU_list_mutex,
 		by IO-fixing and X-latching the block. */
 
 		buf_page_set_io_fix(&block->page, BUF_IO_READ);
 		rw_lock_x_lock(&block->lock);
 
 		mutex_exit(&block->mutex);
-		/* buf_pool->mutex may be released and reacquired by
+		/* buf_pool->LRU_list_mutex may be released and reacquired by
 		buf_buddy_alloc().  Thus, we must release block->mutex
 		in order not to break the latching order in
-		the reacquisition of buf_pool->mutex.  We also must
+		the reacquisition of buf_pool->LRU_list_mutex.  We also must
 		defer this operation until after the block descriptor
 		has been added to buf_pool->LRU and buf_pool->page_hash. */
 		data = buf_buddy_alloc(buf_pool, zip_size, &lru);
@@ -3856,7 +3845,7 @@
 		rw_lock_x_unlock(&block->lock);
 	}
 
-	buf_pool_mutex_exit(buf_pool);
+	mutex_exit(&buf_pool->LRU_list_mutex);
 
 	mtr_memo_push(mtr, block, MTR_MEMO_BUF_FIX);
 
@@ -3907,6 +3896,8 @@
 	const byte*	frame;
 	monitor_id_t	counter;
 
+	ut_ad(mutex_own(buf_page_get_mutex(bpage)));
+
 	/* If the counter module is not turned on, just return */
 	if (!MONITOR_IS_ON(MONITOR_MODULE_BUF_PAGE)) {
 		return;
@@ -4014,9 +4005,13 @@
 					== BUF_BLOCK_FILE_PAGE);
 	ulint		space = bpage->space;
 	ibool		ret = TRUE;
+	const ulint	fold = buf_page_address_fold(bpage->space,
+						     bpage->offset);
+	rw_lock_t*	hash_lock = buf_page_hash_lock_get(buf_pool, fold);
 
 	/* First unfix and release lock on the bpage */
-	buf_pool_mutex_enter(buf_pool);
+	mutex_enter(&buf_pool->LRU_list_mutex);
+	rw_lock_x_lock(hash_lock);
 	mutex_enter(buf_page_get_mutex(bpage));
 	ut_ad(buf_page_get_io_fix(bpage) == BUF_IO_READ);
 	ut_ad(bpage->buf_fix_count == 0);
@@ -4030,19 +4025,18 @@
 			BUF_IO_READ);
 	}
 
-	mutex_exit(buf_page_get_mutex(bpage));
-
 	/* Find the table with specified space id, and mark it corrupted */
 	if (dict_set_corrupted_by_space(space)) {
 		buf_LRU_free_one_page(bpage);
 	} else {
+		mutex_exit(buf_page_get_mutex(bpage));
 		ret = FALSE;
 	}
 
+	mutex_exit(&buf_pool->LRU_list_mutex);
+
 	ut_ad(buf_pool->n_pend_reads > 0);
-	buf_pool->n_pend_reads--;
-
-	buf_pool_mutex_exit(buf_pool);
+	os_atomic_decrement_ulint(&buf_pool->n_pend_reads, 1);
 
 	return(ret);
 }
@@ -4061,6 +4055,7 @@
 	buf_pool_t*	buf_pool = buf_pool_from_bpage(bpage);
 	const ibool	uncompressed = (buf_page_get_state(bpage)
 					== BUF_BLOCK_FILE_PAGE);
+	bool		have_LRU_mutex = false;
 
 	ut_a(buf_page_in_file(bpage));
 
@@ -4070,7 +4065,7 @@
 	ensures that this is the only thread that handles the i/o for this
 	block. */
 
-	io_type = buf_page_get_io_fix(bpage);
+	io_type = buf_page_get_io_fix_unlocked(bpage);
 	ut_ad(io_type == BUF_IO_READ || io_type == BUF_IO_WRITE);
 
 	if (io_type == BUF_IO_READ) {
@@ -4080,15 +4075,16 @@
 
 		if (buf_page_get_zip_size(bpage)) {
 			frame = bpage->zip.data;
-			buf_pool->n_pend_unzip++;
+			os_atomic_increment_ulint(&buf_pool->n_pend_unzip, 1);
 			if (uncompressed
 			    && !buf_zip_decompress((buf_block_t*) bpage,
 						   FALSE)) {
 
-				buf_pool->n_pend_unzip--;
+				os_atomic_decrement_ulint(
+					&buf_pool->n_pend_unzip, 1);
 				goto corrupt;
 			}
-			buf_pool->n_pend_unzip--;
+			os_atomic_decrement_ulint(&buf_pool->n_pend_unzip, 1);
 		} else {
 			ut_a(uncompressed);
 			frame = ((buf_block_t*) bpage)->frame;
@@ -4255,8 +4251,37 @@
 		}
 	}
 
-	buf_pool_mutex_enter(buf_pool);
-	mutex_enter(buf_page_get_mutex(bpage));
+	if (io_type == BUF_IO_WRITE
+	    && (
+#if defined UNIV_DEBUG || defined UNIV_BUF_DEBUG
+		/* to keep consistency at buf_LRU_insert_zip_clean() */
+		buf_page_get_state(bpage) == BUF_BLOCK_ZIP_DIRTY ||
+#endif /* UNIV_DEBUG || UNIV_BUF_DEBUG */
+		buf_page_get_flush_type(bpage) == BUF_FLUSH_LRU)) {
+
+		have_LRU_mutex = TRUE; /* optimistic */
+	}
+retry_mutex:
+	if (have_LRU_mutex) {
+		mutex_enter(&buf_pool->LRU_list_mutex);
+	}
+
+	ib_mutex_t*	block_mutex = buf_page_get_mutex(bpage);
+	mutex_enter(block_mutex);
+
+	if (UNIV_UNLIKELY(io_type == BUF_IO_WRITE
+			  && (
+#if defined UNIV_DEBUG || defined UNIV_BUF_DEBUG
+			      buf_page_get_state(bpage) == BUF_BLOCK_ZIP_DIRTY
+			      ||
+#endif
+			      buf_page_get_flush_type(bpage) == BUF_FLUSH_LRU)
+			  && !have_LRU_mutex)) {
+
+		mutex_exit(block_mutex);
+		have_LRU_mutex = TRUE;
+		goto retry_mutex;
+	}
 
 #ifdef UNIV_IBUF_COUNT_DEBUG
 	if (io_type == BUF_IO_WRITE || uncompressed) {
@@ -4271,17 +4296,20 @@
 	removes the newest lock debug record, without checking the thread
 	id. */
 
-	buf_page_set_io_fix(bpage, BUF_IO_NONE);
-
 	switch (io_type) {
 	case BUF_IO_READ:
+
+		buf_page_set_io_fix(bpage, BUF_IO_NONE);
+
 		/* NOTE that the call to ibuf may have moved the ownership of
 		the x-latch to this OS thread: do not let this confuse you in
 		debugging! */
 
 		ut_ad(buf_pool->n_pend_reads > 0);
-		buf_pool->n_pend_reads--;
-		buf_pool->stat.n_pages_read++;
+		os_atomic_decrement_ulint(&buf_pool->n_pend_reads, 1);
+		os_atomic_increment_ulint(&buf_pool->stat.n_pages_read, 1);
+
+		ut_ad(!have_LRU_mutex);
 
 		if (uncompressed) {
 			rw_lock_x_unlock_gen(&((buf_block_t*) bpage)->lock,
@@ -4296,13 +4324,17 @@
 
 		buf_flush_write_complete(bpage);
 
+		os_atomic_increment_ulint(&buf_pool->stat.n_pages_written, 1);
+
+		if (have_LRU_mutex) {
+			mutex_exit(&buf_pool->LRU_list_mutex);
+		}
+
 		if (uncompressed) {
 			rw_lock_s_unlock_gen(&((buf_block_t*) bpage)->lock,
 					     BUF_IO_WRITE);
 		}
 
-		buf_pool->stat.n_pages_written++;
-
 		break;
 
 	default:
@@ -4320,8 +4352,7 @@
 	}
 #endif /* UNIV_DEBUG */
 
-	mutex_exit(buf_page_get_mutex(bpage));
-	buf_pool_mutex_exit(buf_pool);
+	mutex_exit(block_mutex);
 
 	return(true);
 }
@@ -4340,14 +4371,16 @@
 
 	ut_ad(buf_pool);
 
-	buf_pool_mutex_enter(buf_pool);
-
 	chunk = buf_pool->chunks;
 
 	for (i = buf_pool->n_chunks; i--; chunk++) {
 
+		mutex_enter(&buf_pool->LRU_list_mutex);
+
 		const buf_block_t* block = buf_chunk_not_freed(chunk);
 
+		mutex_exit(&buf_pool->LRU_list_mutex);
+
 		if (UNIV_LIKELY_NULL(block)) {
 			fprintf(stderr,
 				"Page %lu %lu still fixed or dirty\n",
@@ -4357,8 +4390,6 @@
 		}
 	}
 
-	buf_pool_mutex_exit(buf_pool);
-
 	return(TRUE);
 }
 
@@ -4372,7 +4403,9 @@
 {
 	ulint		i;
 
-	buf_pool_mutex_enter(buf_pool);
+	ut_ad(!mutex_own(&buf_pool->LRU_list_mutex));
+
+	mutex_enter(&buf_pool->flush_state_mutex);
 
 	for (i = BUF_FLUSH_LRU; i < BUF_FLUSH_N_TYPES; i++) {
 
@@ -4390,21 +4423,20 @@
 		if (buf_pool->n_flush[i] > 0) {
 			buf_flush_t	type = static_cast<buf_flush_t>(i);
 
-			buf_pool_mutex_exit(buf_pool);
+			mutex_exit(&buf_pool->flush_state_mutex);
 			buf_flush_wait_batch_end(buf_pool, type);
-			buf_pool_mutex_enter(buf_pool);
+			mutex_enter(&buf_pool->flush_state_mutex);
 		}
 	}
-
-	buf_pool_mutex_exit(buf_pool);
+	mutex_exit(&buf_pool->flush_state_mutex);
 
 	ut_ad(buf_all_freed_instance(buf_pool));
 
-	buf_pool_mutex_enter(buf_pool);
-
 	while (buf_LRU_scan_and_free_block(buf_pool, TRUE)) {
 	}
 
+	mutex_enter(&buf_pool->LRU_list_mutex);
+
 	ut_ad(UT_LIST_GET_LEN(buf_pool->LRU) == 0);
 	ut_ad(UT_LIST_GET_LEN(buf_pool->unzip_LRU) == 0);
 
@@ -4412,10 +4444,10 @@
 	buf_pool->LRU_old = NULL;
 	buf_pool->LRU_old_len = 0;
 
+	mutex_exit(&buf_pool->LRU_list_mutex);
+
 	memset(&buf_pool->stat, 0x00, sizeof(buf_pool->stat));
 	buf_refresh_io_stats(buf_pool);
-
-	buf_pool_mutex_exit(buf_pool);
 }
 
 /*********************************************************************//**
@@ -4460,8 +4492,11 @@
 
 	ut_ad(buf_pool);
 
-	buf_pool_mutex_enter(buf_pool);
+	mutex_enter(&buf_pool->LRU_list_mutex);
 	hash_lock_x_all(buf_pool->page_hash);
+	mutex_enter(&buf_pool->zip_mutex);
+	mutex_enter(&buf_pool->free_list_mutex);
+	mutex_enter(&buf_pool->flush_state_mutex);
 
 	chunk = buf_pool->chunks;
 
@@ -4474,8 +4509,6 @@
 
 		for (j = chunk->size; j--; block++) {
 
-			mutex_enter(&block->mutex);
-
 			switch (buf_block_get_state(block)) {
 			case BUF_BLOCK_POOL_WATCH:
 			case BUF_BLOCK_ZIP_PAGE:
@@ -4486,6 +4519,7 @@
 				break;
 
 			case BUF_BLOCK_FILE_PAGE:
+
 				space = buf_block_get_space(block);
 				offset = buf_block_get_page_no(block);
 				fold = buf_page_address_fold(space, offset);
@@ -4496,14 +4530,15 @@
 				     == &block->page);
 
 #ifdef UNIV_IBUF_COUNT_DEBUG
-				ut_a(buf_page_get_io_fix(&block->page)
+				ut_a(buf_page_get_io_fix_unlocked(&block->page)
 				     == BUF_IO_READ
 				     || !ibuf_count_get(buf_block_get_space(
 								block),
 							buf_block_get_page_no(
 								block)));
 #endif
-				switch (buf_page_get_io_fix(&block->page)) {
+				switch (buf_page_get_io_fix_unlocked(
+						&block->page)) {
 				case BUF_IO_NONE:
 					break;
 
@@ -4511,17 +4546,8 @@
 					switch (buf_page_get_flush_type(
 							&block->page)) {
 					case BUF_FLUSH_LRU:
-						n_lru_flush++;
-						goto assert_s_latched;
 					case BUF_FLUSH_SINGLE_PAGE:
-						n_page_flush++;
-assert_s_latched:
-						ut_a(rw_lock_is_locked(
-							     &block->lock,
-								     RW_LOCK_SHARED));
-						break;
 					case BUF_FLUSH_LIST:
-						n_list_flush++;
 						break;
 					default:
 						ut_error;
@@ -4552,13 +4578,9 @@
 				/* do nothing */
 				break;
 			}
-
-			mutex_exit(&block->mutex);
 		}
 	}
 
-	mutex_enter(&buf_pool->zip_mutex);
-
 	/* Check clean compressed-only blocks. */
 
 	for (b = UT_LIST_GET_FIRST(buf_pool->zip_clean); b;
@@ -4604,7 +4626,9 @@
 		case BUF_BLOCK_ZIP_DIRTY:
 			n_lru++;
 			n_zip++;
-			switch (buf_page_get_io_fix(b)) {
+			/* fallthrough */
+		case BUF_BLOCK_FILE_PAGE:
+			switch (buf_page_get_io_fix_unlocked(b)) {
 			case BUF_IO_NONE:
 			case BUF_IO_READ:
 			case BUF_IO_PIN:
@@ -4624,11 +4648,10 @@
 					ut_error;
 				}
 				break;
+			default:
+				ut_error;
 			}
 			break;
-		case BUF_BLOCK_FILE_PAGE:
-			/* uncompressed page */
-			break;
 		case BUF_BLOCK_POOL_WATCH:
 		case BUF_BLOCK_ZIP_PAGE:
 		case BUF_BLOCK_NOT_USED:
@@ -4658,6 +4681,9 @@
 	}
 
 	ut_a(UT_LIST_GET_LEN(buf_pool->LRU) == n_lru);
+
+	mutex_exit(&buf_pool->LRU_list_mutex);
+
 	if (UT_LIST_GET_LEN(buf_pool->free) != n_free) {
 		fprintf(stderr, "Free list len %lu, free blocks %lu\n",
 			(ulong) UT_LIST_GET_LEN(buf_pool->free),
@@ -4665,11 +4691,13 @@
 		ut_error;
 	}
 
+	mutex_exit(&buf_pool->free_list_mutex);
+
 	ut_a(buf_pool->n_flush[BUF_FLUSH_LIST] == n_list_flush);
 	ut_a(buf_pool->n_flush[BUF_FLUSH_LRU] == n_lru_flush);
 	ut_a(buf_pool->n_flush[BUF_FLUSH_SINGLE_PAGE] == n_page_flush);
 
-	buf_pool_mutex_exit(buf_pool);
+	mutex_exit(&buf_pool->flush_state_mutex);
 
 	ut_a(buf_LRU_validate());
 	ut_a(buf_flush_validate(buf_pool));
@@ -4727,8 +4755,7 @@
 
 	counts = static_cast<ulint*>(mem_alloc(sizeof(ulint) * size));
 
-	buf_pool_mutex_enter(buf_pool);
-	buf_flush_list_mutex_enter(buf_pool);
+	/* Dirty reads below */
 
 	fprintf(stderr,
 		"buf_pool size %lu\n"
@@ -4755,12 +4782,12 @@
 		(ulong) buf_pool->stat.n_pages_created,
 		(ulong) buf_pool->stat.n_pages_written);
 
-	buf_flush_list_mutex_exit(buf_pool);
-
 	/* Count the number of blocks belonging to each index in the buffer */
 
 	n_found = 0;
 
+	mutex_enter(&buf_pool->LRU_list_mutex);
+
 	chunk = buf_pool->chunks;
 
 	for (i = buf_pool->n_chunks; i--; chunk++) {
@@ -4796,7 +4823,7 @@
 		}
 	}
 
-	buf_pool_mutex_exit(buf_pool);
+	mutex_exit(&buf_pool->LRU_list_mutex);
 
 	for (i = 0; i < n_found; i++) {
 		index = dict_index_get_if_in_cache(index_ids[i]);
@@ -4853,7 +4880,8 @@
 	buf_chunk_t*	chunk;
 	ulint		fixed_pages_number = 0;
 
-	buf_pool_mutex_enter(buf_pool);
+	/* The LRU list mutex is enough to protect the required fields below */
+	mutex_enter(&buf_pool->LRU_list_mutex);
 
 	chunk = buf_pool->chunks;
 
@@ -4870,18 +4898,17 @@
 				continue;
 			}
 
-			mutex_enter(&block->mutex);
-
 			if (block->page.buf_fix_count != 0
-			    || buf_page_get_io_fix(&block->page)
+			    || buf_page_get_io_fix_unlocked(&block->page)
 			    != BUF_IO_NONE) {
 				fixed_pages_number++;
 			}
 
-			mutex_exit(&block->mutex);
 		}
 	}
 
+	mutex_exit(&buf_pool->LRU_list_mutex);
+
 	mutex_enter(&buf_pool->zip_mutex);
 
 	/* Traverse the lists of clean and dirty compressed-only blocks. */
@@ -4925,7 +4952,6 @@
 
 	buf_flush_list_mutex_exit(buf_pool);
 	mutex_exit(&buf_pool->zip_mutex);
-	buf_pool_mutex_exit(buf_pool);
 
 	return(fixed_pages_number);
 }
@@ -5073,9 +5099,6 @@
 	/* Find appropriate pool_info to store stats for this buffer pool */
 	pool_info = &all_pool_info[pool_id];
 
-	buf_pool_mutex_enter(buf_pool);
-	buf_flush_list_mutex_enter(buf_pool);
-
 	pool_info->pool_unique_id = pool_id;
 
 	pool_info->pool_size = buf_pool->curr_size;
@@ -5094,6 +5117,8 @@
 
 	pool_info->n_pend_reads = buf_pool->n_pend_reads;
 
+	mutex_enter(&buf_pool->flush_state_mutex);
+
 	pool_info->n_pending_flush_lru =
 		 (buf_pool->n_flush[BUF_FLUSH_LRU]
 		  + buf_pool->init_flush[BUF_FLUSH_LRU]);
@@ -5106,7 +5131,7 @@
 		 (buf_pool->n_flush[BUF_FLUSH_SINGLE_PAGE]
 		  + buf_pool->init_flush[BUF_FLUSH_SINGLE_PAGE]);
 
-	buf_flush_list_mutex_exit(buf_pool);
+	mutex_exit(&buf_pool->flush_state_mutex);
 
 	current_time = time(NULL);
 	time_elapsed = 0.001 + difftime(current_time,
@@ -5189,7 +5214,6 @@
 	pool_info->unzip_cur = buf_LRU_stat_cur.unzip;
 
 	buf_refresh_io_stats(buf_pool);
-	buf_pool_mutex_exit(buf_pool);
 }
 
 /*********************************************************************//**
@@ -5398,22 +5422,22 @@
 	ulint		i;
 	ulint		pending_io = 0;
 
-	buf_pool_mutex_enter_all();
-
 	for (i = 0; i < srv_buf_pool_instances; i++) {
-		const buf_pool_t*	buf_pool;
+		buf_pool_t*	buf_pool;
 
 		buf_pool = buf_pool_from_array(i);
 
-		pending_io += buf_pool->n_pend_reads
-			      + buf_pool->n_flush[BUF_FLUSH_LRU]
-			      + buf_pool->n_flush[BUF_FLUSH_SINGLE_PAGE]
-			      + buf_pool->n_flush[BUF_FLUSH_LIST];
-
+		pending_io += buf_pool->n_pend_reads;
+
+		mutex_enter(&buf_pool->flush_state_mutex);
+
+		pending_io += buf_pool->n_flush[BUF_FLUSH_LRU];
+		pending_io += buf_pool->n_flush[BUF_FLUSH_SINGLE_PAGE];
+		pending_io += buf_pool->n_flush[BUF_FLUSH_LIST];
+
+		mutex_exit(&buf_pool->flush_state_mutex);
 	}
 
-	buf_pool_mutex_exit_all();
-
 	return(pending_io);
 }
 
@@ -5429,11 +5453,11 @@
 {
 	ulint	len;
 
-	buf_pool_mutex_enter(buf_pool);
+	mutex_enter(&buf_pool->free_list_mutex);
 
 	len = UT_LIST_GET_LEN(buf_pool->free);
 
-	buf_pool_mutex_exit(buf_pool);
+	mutex_exit(&buf_pool->free_list_mutex);
 
 	return(len);
 }

=== modified file 'Percona-Server/storage/innobase/buf/buf0dblwr.cc'
--- Percona-Server/storage/innobase/buf/buf0dblwr.cc	2013-06-20 15:16:00 +0000
+++ Percona-Server/storage/innobase/buf/buf0dblwr.cc	2013-09-20 05:29:11 +0000
@@ -936,6 +936,7 @@
 	ulint	zip_size;
 
 	ut_a(buf_page_in_file(bpage));
+	ut_ad(!mutex_own(&buf_pool_from_bpage(bpage)->LRU_list_mutex));
 
 try_again:
 	mutex_enter(&buf_dblwr->mutex);

=== modified file 'Percona-Server/storage/innobase/buf/buf0dump.cc'
--- Percona-Server/storage/innobase/buf/buf0dump.cc	2012-12-04 08:24:59 +0000
+++ Percona-Server/storage/innobase/buf/buf0dump.cc	2013-09-20 05:29:11 +0000
@@ -28,7 +28,7 @@
 #include <stdarg.h> /* va_* */
 #include <string.h> /* strerror() */
 
-#include "buf0buf.h" /* buf_pool_mutex_enter(), srv_buf_pool_instances */
+#include "buf0buf.h" /* srv_buf_pool_instances */
 #include "buf0dump.h"
 #include "db0err.h"
 #include "dict0dict.h" /* dict_operation_lock */
@@ -58,8 +58,8 @@
 static ibool	buf_load_abort_flag = FALSE;
 
 /* Used to temporary store dump info in order to avoid IO while holding
-buffer pool mutex during dump and also to sort the contents of the dump
-before reading the pages from disk during load.
+buffer pool LRU list mutex during dump and also to sort the contents of the
+dump before reading the pages from disk during load.
 We store the space id in the high 32 bits and page no in low 32 bits. */
 typedef ib_uint64_t	buf_dump_t;
 
@@ -218,15 +218,15 @@
 
 		buf_pool = buf_pool_from_array(i);
 
-		/* obtain buf_pool mutex before allocate, since
+		/* obtain buf_pool LRU list mutex before allocate, since
 		UT_LIST_GET_LEN(buf_pool->LRU) could change */
-		buf_pool_mutex_enter(buf_pool);
+		mutex_enter(&buf_pool->LRU_list_mutex);
 
 		n_pages = UT_LIST_GET_LEN(buf_pool->LRU);
 
 		/* skip empty buffer pools */
 		if (n_pages == 0) {
-			buf_pool_mutex_exit(buf_pool);
+			mutex_exit(&buf_pool->LRU_list_mutex);
 			continue;
 		}
 
@@ -234,7 +234,7 @@
 			ut_malloc(n_pages * sizeof(*dump))) ;
 
 		if (dump == NULL) {
-			buf_pool_mutex_exit(buf_pool);
+			mutex_exit(&buf_pool->LRU_list_mutex);
 			fclose(f);
 			buf_dump_status(STATUS_ERR,
 					"Cannot allocate " ULINTPF " bytes: %s",
@@ -256,7 +256,7 @@
 
 		ut_a(j == n_pages);
 
-		buf_pool_mutex_exit(buf_pool);
+		mutex_exit(&buf_pool->LRU_list_mutex);
 
 		for (j = 0; j < n_pages && !SHOULD_QUIT(); j++) {
 			ret = fprintf(f, ULINTPF "," ULINTPF "\n",

=== modified file 'Percona-Server/storage/innobase/buf/buf0flu.cc'
--- Percona-Server/storage/innobase/buf/buf0flu.cc	2013-08-16 09:11:51 +0000
+++ Percona-Server/storage/innobase/buf/buf0flu.cc	2013-09-20 05:29:11 +0000
@@ -350,7 +350,6 @@
 	buf_block_t*	block,		/*!< in/out: block which is modified */
 	lsn_t		lsn)		/*!< in: oldest modification */
 {
-	ut_ad(!buf_pool_mutex_own(buf_pool));
 	ut_ad(log_flush_order_mutex_own());
 	ut_ad(mutex_own(&block->mutex));
 
@@ -409,15 +408,14 @@
 	buf_page_t*	prev_b;
 	buf_page_t*	b;
 
-	ut_ad(!buf_pool_mutex_own(buf_pool));
 	ut_ad(log_flush_order_mutex_own());
 	ut_ad(mutex_own(&block->mutex));
 	ut_ad(buf_block_get_state(block) == BUF_BLOCK_FILE_PAGE);
 
 	buf_flush_list_mutex_enter(buf_pool);
 
-	/* The field in_LRU_list is protected by buf_pool->mutex, which
-	we are not holding.  However, while a block is in the flush
+	/* The field in_LRU_list is protected by buf_pool->LRU_list_mutex,
+	which we are not holding.  However, while a block is in the flush
 	list, it is dirty and cannot be discarded, not from the
 	page_hash or from the LRU list.  At most, the uncompressed
 	page frame of a compressed block may be discarded or created
@@ -501,7 +499,7 @@
 {
 #ifdef UNIV_DEBUG
 	buf_pool_t*	buf_pool = buf_pool_from_bpage(bpage);
-	ut_ad(buf_pool_mutex_own(buf_pool));
+	ut_ad(mutex_own(&buf_pool->LRU_list_mutex));
 #endif
 	ut_ad(mutex_own(buf_page_get_mutex(bpage)));
 	ut_ad(bpage->in_LRU_list);
@@ -535,17 +533,13 @@
 				buf_page_in_file(bpage) */
 	buf_flush_t	flush_type)/*!< in: type of flush */
 {
-#ifdef UNIV_DEBUG
-	buf_pool_t*	buf_pool = buf_pool_from_bpage(bpage);
-	ut_ad(buf_pool_mutex_own(buf_pool));
-#endif /* UNIV_DEBUG */
-
+	ut_ad(flush_type < BUF_FLUSH_N_TYPES);
+	ut_ad(mutex_own(buf_page_get_mutex(bpage))
+	      || flush_type == BUF_FLUSH_LIST);
 	ut_a(buf_page_in_file(bpage));
-	ut_ad(mutex_own(buf_page_get_mutex(bpage)));
-	ut_ad(flush_type < BUF_FLUSH_N_TYPES);
 
 	if (bpage->oldest_modification == 0
-	    || buf_page_get_io_fix(bpage) != BUF_IO_NONE) {
+	    || buf_page_get_io_fix_unlocked(bpage) != BUF_IO_NONE) {
 		return(false);
 	}
 
@@ -583,8 +577,11 @@
 	buf_pool_t*	buf_pool = buf_pool_from_bpage(bpage);
 	ulint		zip_size;
 
-	ut_ad(buf_pool_mutex_own(buf_pool));
 	ut_ad(mutex_own(buf_page_get_mutex(bpage)));
+#if defined UNIV_DEBUG || defined UNIV_BUF_DEBUG
+	ut_ad(buf_page_get_state(bpage) != BUF_BLOCK_ZIP_DIRTY
+	      || mutex_own(&buf_pool->LRU_list_mutex));
+#endif
 	ut_ad(bpage->in_flush_list);
 
 	buf_flush_list_mutex_enter(buf_pool);
@@ -655,7 +652,6 @@
 	buf_page_t*	prev_b = NULL;
 	buf_pool_t*	buf_pool = buf_pool_from_bpage(bpage);
 
-	ut_ad(buf_pool_mutex_own(buf_pool));
 	/* Must reside in the same buffer pool. */
 	ut_ad(buf_pool == buf_pool_from_bpage(dpage));
 
@@ -663,13 +659,6 @@
 
 	buf_flush_list_mutex_enter(buf_pool);
 
-	/* FIXME: At this point we have both buf_pool and flush_list
-	mutexes. Theoretically removal of a block from flush list is
-	only covered by flush_list mutex but currently we do
-	have buf_pool mutex in buf_flush_remove() therefore this block
-	is guaranteed to be in the flush list. We need to check if
-	this will work without the assumption of block removing code
-	having the buf_pool mutex. */
 	ut_ad(bpage->in_flush_list);
 	ut_ad(dpage->in_flush_list);
 
@@ -720,14 +709,15 @@
 /*=====================*/
 	buf_page_t*	bpage)	/*!< in: pointer to the block in question */
 {
-	buf_flush_t	flush_type;
+	buf_flush_t	flush_type = buf_page_get_flush_type(bpage);
 	buf_pool_t*	buf_pool = buf_pool_from_bpage(bpage);
 
-	ut_ad(bpage);
+	mutex_enter(&buf_pool->flush_state_mutex);
 
 	buf_flush_remove(bpage);
 
-	flush_type = buf_page_get_flush_type(bpage);
+	buf_page_set_io_fix(bpage, BUF_IO_NONE);
+
 	buf_pool->n_flush[flush_type]--;
 
 	/* fprintf(stderr, "n pending flush %lu\n",
@@ -742,6 +732,8 @@
 	}
 
 	buf_dblwr_update(bpage, flush_type);
+
+	mutex_exit(&buf_pool->flush_state_mutex);
 }
 #endif /* !UNIV_HOTBACKUP */
 
@@ -890,7 +882,7 @@
 
 #ifdef UNIV_DEBUG
 	buf_pool_t*	buf_pool = buf_pool_from_bpage(bpage);
-	ut_ad(!buf_pool_mutex_own(buf_pool));
+	ut_ad(!mutex_own(&buf_pool->LRU_list_mutex));
 #endif
 
 #ifdef UNIV_LOG_DEBUG
@@ -899,15 +891,14 @@
 
 	ut_ad(buf_page_in_file(bpage));
 
-	/* We are not holding buf_pool->mutex or block_mutex here.
+	/* We are not holding block_mutex here.
 	Nevertheless, it is safe to access bpage, because it is
 	io_fixed and oldest_modification != 0.  Thus, it cannot be
 	relocated in the buffer pool or removed from flush_list or
 	LRU_list. */
-	ut_ad(!buf_pool_mutex_own(buf_pool));
 	ut_ad(!buf_flush_list_mutex_own(buf_pool));
 	ut_ad(!mutex_own(buf_page_get_mutex(bpage)));
-	ut_ad(buf_page_get_io_fix(bpage) == BUF_IO_WRITE);
+	ut_ad(buf_page_get_io_fix_unlocked(bpage) == BUF_IO_WRITE);
 	ut_ad(bpage->oldest_modification != 0);
 
 #ifdef UNIV_IBUF_COUNT_DEBUG
@@ -989,9 +980,8 @@
 Writes a flushable page asynchronously from the buffer pool to a file.
 NOTE: in simulated aio we must call
 os_aio_simulated_wake_handler_threads after we have posted a batch of
-writes! NOTE: buf_pool->mutex and buf_page_get_mutex(bpage) must be
-held upon entering this function, and they will be released by this
-function. */
+writes! NOTE: buf_page_get_mutex(bpage) must be held upon entering this
+function, and it will be released by this function. */
 UNIV_INTERN
 void
 buf_flush_page(
@@ -1005,7 +995,7 @@
 	ibool		is_uncompressed;
 
 	ut_ad(flush_type < BUF_FLUSH_N_TYPES);
-	ut_ad(buf_pool_mutex_own(buf_pool));
+	ut_ad(!mutex_own(&buf_pool->LRU_list_mutex));
 	ut_ad(buf_page_in_file(bpage));
 	ut_ad(!sync || flush_type == BUF_FLUSH_SINGLE_PAGE);
 
@@ -1014,6 +1004,8 @@
 
 	ut_ad(buf_flush_ready_for_flush(bpage, flush_type));
 
+	mutex_enter(&buf_pool->flush_state_mutex);
+
 	buf_page_set_io_fix(bpage, BUF_IO_WRITE);
 
 	buf_page_set_flush_type(bpage, flush_type);
@@ -1025,6 +1017,8 @@
 
 	buf_pool->n_flush[flush_type]++;
 
+	mutex_exit(&buf_pool->flush_state_mutex);
+
 	is_uncompressed = (buf_page_get_state(bpage) == BUF_BLOCK_FILE_PAGE);
 	ut_ad(is_uncompressed == (block_mutex != &buf_pool->zip_mutex));
 
@@ -1042,7 +1036,6 @@
 		}
 
 		mutex_exit(block_mutex);
-		buf_pool_mutex_exit(buf_pool);
 
 		/* Even though bpage is not protected by any mutex at
 		this point, it is safe to access bpage, because it is
@@ -1080,11 +1073,10 @@
 		}
 
 		/* Note that the s-latch is acquired before releasing the
-		buf_pool mutex: this ensures that the latch is acquired
-		immediately. */
+		buf_page_get_mutex() mutex: this ensures that the latch is
+		acquired immediately. */
 
 		mutex_exit(block_mutex);
-		buf_pool_mutex_exit(buf_pool);
 		break;
 
 	default:
@@ -1109,9 +1101,9 @@
 # if defined UNIV_DEBUG || defined UNIV_IBUF_DEBUG
 /********************************************************************//**
 Writes a flushable page asynchronously from the buffer pool to a file.
-NOTE: buf_pool->mutex and block->mutex must be held upon entering this
-function, and they will be released by this function after flushing.
-This is loosely based on buf_flush_batch() and buf_flush_page().
+NOTE: block->mutex must be held upon entering this function, and it will be
+released by this function after flushing.  This is loosely based on
+buf_flush_batch() and buf_flush_page().
 @return TRUE if the page was flushed and the mutexes released */
 UNIV_INTERN
 ibool
@@ -1120,7 +1112,6 @@
 	buf_pool_t*	buf_pool,	/*!< in/out: buffer pool instance */
 	buf_block_t*	block)		/*!< in/out: buffer control block */
 {
-	ut_ad(buf_pool_mutex_own(buf_pool));
 	ut_ad(buf_block_get_state(block) == BUF_BLOCK_FILE_PAGE);
 	ut_ad(mutex_own(&block->mutex));
 
@@ -1149,21 +1140,27 @@
 	buf_page_t*	bpage;
 	buf_pool_t*	buf_pool = buf_pool_get(space, offset);
 	bool		ret;
+	rw_lock_t*	hash_lock;
+	ib_mutex_t*	block_mutex;
 
 	ut_ad(flush_type == BUF_FLUSH_LRU
 	      || flush_type == BUF_FLUSH_LIST);
 
-	buf_pool_mutex_enter(buf_pool);
-
 	/* We only want to flush pages from this buffer pool. */
-	bpage = buf_page_hash_get(buf_pool, space, offset);
+	bpage = buf_page_hash_get_s_locked(buf_pool, space, offset,
+					   &hash_lock);
 
 	if (!bpage) {
 
-		buf_pool_mutex_exit(buf_pool);
 		return(false);
 	}
 
+	block_mutex = buf_page_get_mutex(bpage);
+
+	mutex_enter(block_mutex);
+
+	rw_lock_s_unlock(hash_lock);
+
 	ut_a(buf_page_in_file(bpage));
 
 	/* We avoid flushing 'non-old' blocks in an LRU flush,
@@ -1171,15 +1168,13 @@
 
 	ret = false;
 	if (flush_type != BUF_FLUSH_LRU || buf_page_is_old(bpage)) {
-		ib_mutex_t* block_mutex = buf_page_get_mutex(bpage);
 
-		mutex_enter(block_mutex);
 		if (buf_flush_ready_for_flush(bpage, flush_type)) {
 			ret = true;
 		}
-		mutex_exit(block_mutex);
 	}
-	buf_pool_mutex_exit(buf_pool);
+
+	mutex_exit(block_mutex);
 
 	return(ret);
 }
@@ -1207,6 +1202,8 @@
 	buf_pool_t*	buf_pool = buf_pool_get(space, offset);
 
 	ut_ad(flush_type == BUF_FLUSH_LRU || flush_type == BUF_FLUSH_LIST);
+	ut_ad(!mutex_own(&buf_pool->LRU_list_mutex));
+	ut_ad(!buf_flush_list_mutex_own(buf_pool));
 
 	if (UT_LIST_GET_LEN(buf_pool->LRU) < BUF_LRU_OLD_MIN_LEN
 	    || srv_flush_neighbors == 0) {
@@ -1262,6 +1259,8 @@
 	for (i = low; i < high; i++) {
 
 		buf_page_t*	bpage;
+		rw_lock_t*	hash_lock;
+		ib_mutex_t*	block_mutex;
 
 		if ((count + n_flushed) >= n_to_flush) {
 
@@ -1280,17 +1279,21 @@
 
 		buf_pool = buf_pool_get(space, i);
 
-		buf_pool_mutex_enter(buf_pool);
-
 		/* We only want to flush pages from this buffer pool. */
-		bpage = buf_page_hash_get(buf_pool, space, i);
+		bpage = buf_page_hash_get_s_locked(buf_pool, space, i,
+						   &hash_lock);
 
 		if (!bpage) {
 
-			buf_pool_mutex_exit(buf_pool);
 			continue;
 		}
 
+		block_mutex = buf_page_get_mutex(bpage);
+
+		mutex_enter(block_mutex);
+
+		rw_lock_s_unlock(hash_lock);
+
 		ut_a(buf_page_in_file(bpage));
 
 		/* We avoid flushing 'non-old' blocks in an LRU flush,
@@ -1299,9 +1302,6 @@
 		if (flush_type != BUF_FLUSH_LRU
 		    || i == offset
 		    || buf_page_is_old(bpage)) {
-			ib_mutex_t* block_mutex = buf_page_get_mutex(bpage);
-
-			mutex_enter(block_mutex);
 
 			if (buf_flush_ready_for_flush(bpage, flush_type)
 			    && (i == offset || !bpage->buf_fix_count)) {
@@ -1316,14 +1316,12 @@
 
 				buf_flush_page(buf_pool, bpage, flush_type, false);
 				ut_ad(!mutex_own(block_mutex));
-				ut_ad(!buf_pool_mutex_own(buf_pool));
 				count++;
 				continue;
-			} else {
-				mutex_exit(block_mutex);
 			}
 		}
-		buf_pool_mutex_exit(buf_pool);
+
+		mutex_exit(block_mutex);
 	}
 
 	if (count > 0) {
@@ -1341,8 +1339,9 @@
 Check if the block is modified and ready for flushing. If the the block
 is ready to flush then flush the page and try o flush its neighbors.
 
-@return	TRUE if buf_pool mutex was released during this function.
-This does not guarantee that some pages were written as well.
+@return	TRUE if, depending on the flush type, either LRU or flush list
+mutex was released during this function.  This does not guarantee that some
+pages were written as well.
 Number of pages written are incremented to the count. */
 static
 ibool
@@ -1358,16 +1357,21 @@
 	ulint*		count)		/*!< in/out: number of pages
 					flushed */
 {
-	ib_mutex_t*	block_mutex;
+	ib_mutex_t*	block_mutex = NULL;
 	ibool		flushed = FALSE;
 #ifdef UNIV_DEBUG
 	buf_pool_t*	buf_pool = buf_pool_from_bpage(bpage);
 #endif /* UNIV_DEBUG */
 
-	ut_ad(buf_pool_mutex_own(buf_pool));
+	ut_ad((flush_type == BUF_FLUSH_LRU
+	       && mutex_own(&buf_pool->LRU_list_mutex))
+	      || (flush_type == BUF_FLUSH_LIST
+		  && buf_flush_list_mutex_own(buf_pool)));
 
-	block_mutex = buf_page_get_mutex(bpage);
-	mutex_enter(block_mutex);
+	if (flush_type == BUF_FLUSH_LRU) {
+		block_mutex = buf_page_get_mutex(bpage);
+		mutex_enter(block_mutex);
+	}
 
 	ut_a(buf_page_in_file(bpage));
 
@@ -1378,14 +1382,20 @@
 
 		buf_pool = buf_pool_from_bpage(bpage);
 
-		buf_pool_mutex_exit(buf_pool);
+		if (flush_type == BUF_FLUSH_LRU) {
+			mutex_exit(&buf_pool->LRU_list_mutex);
+		}
 
-		/* These fields are protected by both the
-		buffer pool mutex and block mutex. */
+		/* These fields are protected by the buf_page_get_mutex()
+		mutex. */
 		space = buf_page_get_space(bpage);
 		offset = buf_page_get_page_no(bpage);
 
-		mutex_exit(block_mutex);
+		if (flush_type == BUF_FLUSH_LRU) {
+			mutex_exit(block_mutex);
+		} else {
+			buf_flush_list_mutex_exit(buf_pool);
+		}
 
 		/* Try to flush also all the neighbors */
 		*count += buf_flush_try_neighbors(space,
@@ -1394,13 +1404,20 @@
 						  *count,
 						  n_to_flush);
 
-		buf_pool_mutex_enter(buf_pool);
+		if (flush_type == BUF_FLUSH_LRU) {
+			mutex_enter(&buf_pool->LRU_list_mutex);
+		} else {
+			buf_flush_list_mutex_enter(buf_pool);
+		}
 		flushed = TRUE;
-	} else {
+	} else if (flush_type == BUF_FLUSH_LRU) {
 		mutex_exit(block_mutex);
 	}
 
-	ut_ad(buf_pool_mutex_own(buf_pool));
+	ut_ad((flush_type == BUF_FLUSH_LRU
+	       && mutex_own(&buf_pool->LRU_list_mutex))
+	      || (flush_type == BUF_FLUSH_LIST
+		  && buf_flush_list_mutex_own(buf_pool)));
 
 	return(flushed);
 }
@@ -1428,22 +1445,31 @@
 	ulint		free_len = UT_LIST_GET_LEN(buf_pool->free);
 	ulint		lru_len = UT_LIST_GET_LEN(buf_pool->unzip_LRU);
 
-	ut_ad(buf_pool_mutex_own(buf_pool));
+	ut_ad(mutex_own(&buf_pool->LRU_list_mutex));
 
 	block = UT_LIST_GET_LAST(buf_pool->unzip_LRU);
 	while (block != NULL && count < max
 	       && free_len < srv_LRU_scan_depth
 	       && lru_len > UT_LIST_GET_LEN(buf_pool->LRU) / 10) {
 
+		ib_mutex_t*	block_mutex = buf_page_get_mutex(&block->page);
+
 		++scanned;
+
+		mutex_enter(block_mutex);
+
 		if (buf_LRU_free_page(&block->page, false)) {
-			/* Block was freed. buf_pool->mutex potentially
+
+			mutex_exit(block_mutex);
+			/* Block was freed. LRU list mutex potentially
 			released and reacquired */
 			++count;
+			mutex_enter(&buf_pool->LRU_list_mutex);
 			block = UT_LIST_GET_LAST(buf_pool->unzip_LRU);
 
 		} else {
 
+			mutex_exit(block_mutex);
 			block = UT_LIST_GET_PREV(unzip_LRU, block);
 		}
 
@@ -1451,7 +1477,7 @@
 		lru_len = UT_LIST_GET_LEN(buf_pool->unzip_LRU);
 	}
 
-	ut_ad(buf_pool_mutex_own(buf_pool));
+	ut_ad(mutex_own(&buf_pool->LRU_list_mutex));
 
 	if (scanned) {
 		MONITOR_INC_VALUE_CUMULATIVE(
@@ -1485,7 +1511,7 @@
 	ulint		free_len = UT_LIST_GET_LEN(buf_pool->free);
 	ulint		lru_len = UT_LIST_GET_LEN(buf_pool->LRU);
 
-	ut_ad(buf_pool_mutex_own(buf_pool));
+	ut_ad(mutex_own(&buf_pool->LRU_list_mutex));
 
 	bpage = UT_LIST_GET_LAST(buf_pool->LRU);
 	while (bpage != NULL && count < max
@@ -1494,13 +1520,20 @@
 
 		ib_mutex_t* block_mutex = buf_page_get_mutex(bpage);
 		ibool	 evict;
-
-		mutex_enter(block_mutex);
-		evict = buf_flush_ready_for_replace(bpage);
-		mutex_exit(block_mutex);
+		ulint	failed_acquire;
 
 		++scanned;
 
+		failed_acquire = mutex_enter_nowait(block_mutex);
+
+		evict = UNIV_LIKELY(!failed_acquire)
+			&& buf_flush_ready_for_replace(bpage);
+
+		if (UNIV_LIKELY(!failed_acquire) && !evict) {
+
+			mutex_exit(block_mutex);
+		}
+
 		/* If the block is ready to be replaced we try to
 		free it i.e.: put it on the free list.
 		Otherwise we try to flush the block and its
@@ -1514,28 +1547,35 @@
 		O(n*n). */
 		if (evict) {
 			if (buf_LRU_free_page(bpage, true)) {
-				/* buf_pool->mutex was potentially
-				released and reacquired. */
+
+				mutex_exit(block_mutex);
+				mutex_enter(&buf_pool->LRU_list_mutex);
 				bpage = UT_LIST_GET_LAST(buf_pool->LRU);
 			} else {
+
 				bpage = UT_LIST_GET_PREV(LRU, bpage);
+				mutex_exit(block_mutex);
 			}
-		} else if (buf_flush_page_and_try_neighbors(
+		} else if (UNIV_LIKELY(!failed_acquire)) {
+
+			if (buf_flush_page_and_try_neighbors(
 				bpage,
 				BUF_FLUSH_LRU, max, &count)) {
 
-			/* buf_pool->mutex was released.
-			Restart the scan. */
-			bpage = UT_LIST_GET_LAST(buf_pool->LRU);
-		} else {
-			bpage = UT_LIST_GET_PREV(LRU, bpage);
+				/* LRU list mutex was released.
+				Restart the scan. */
+				bpage = UT_LIST_GET_LAST(buf_pool->LRU);
+			} else {
+
+				bpage = UT_LIST_GET_PREV(LRU, bpage);
+			}
 		}
 
 		free_len = UT_LIST_GET_LEN(buf_pool->free);
 		lru_len = UT_LIST_GET_LEN(buf_pool->LRU);
 	}
 
-	ut_ad(buf_pool_mutex_own(buf_pool));
+	ut_ad(mutex_own(&buf_pool->LRU_list_mutex));
 
 	/* We keep track of all flushes happening as part of LRU
 	flush. When estimating the desired rate at which flush_list
@@ -1604,8 +1644,6 @@
 	ulint		count = 0;
 	ulint		scanned = 0;
 
-	ut_ad(buf_pool_mutex_own(buf_pool));
-
 	/* Start from the end of the list looking for a suitable
 	block to be flushed. */
 	buf_flush_list_mutex_enter(buf_pool);
@@ -1629,16 +1667,12 @@
 		prev = UT_LIST_GET_PREV(list, bpage);
 		buf_flush_set_hp(buf_pool, prev);
 
-		buf_flush_list_mutex_exit(buf_pool);
-
 #ifdef UNIV_DEBUG
 		bool flushed =
 #endif /* UNIV_DEBUG */
 		buf_flush_page_and_try_neighbors(
 			bpage, BUF_FLUSH_LIST, min_n, &count);
 
-		buf_flush_list_mutex_enter(buf_pool);
-
 		ut_ad(flushed || buf_flush_is_hp(buf_pool, prev));
 
 		if (!buf_flush_is_hp(buf_pool, prev)) {
@@ -1663,8 +1697,6 @@
 				     MONITOR_FLUSH_BATCH_SCANNED_PER_CALL,
 				     scanned);
 
-	ut_ad(buf_pool_mutex_own(buf_pool));
-
 	return(count);
 }
 
@@ -1701,13 +1733,13 @@
 	      || sync_thread_levels_empty_except_dict());
 #endif /* UNIV_SYNC_DEBUG */
 
-	buf_pool_mutex_enter(buf_pool);
-
-	/* Note: The buffer pool mutex is released and reacquired within
+	/* Note: The buffer pool mutexes are released and reacquired within
 	the flush functions. */
 	switch (flush_type) {
 	case BUF_FLUSH_LRU:
+		mutex_enter(&buf_pool->LRU_list_mutex);
 		count = buf_do_LRU_batch(buf_pool, min_n);
+		mutex_exit(&buf_pool->LRU_list_mutex);
 		break;
 	case BUF_FLUSH_LIST:
 		count = buf_do_flush_list_batch(buf_pool, min_n, lsn_limit);
@@ -1716,8 +1748,6 @@
 		ut_error;
 	}
 
-	buf_pool_mutex_exit(buf_pool);
-
 #ifdef UNIV_DEBUG
 	if (buf_debug_prints && count > 0) {
 		fprintf(stderr, flush_type == BUF_FLUSH_LRU
@@ -1765,21 +1795,21 @@
 	buf_flush_t	flush_type)	/*!< in: BUF_FLUSH_LRU
 					or BUF_FLUSH_LIST */
 {
-	buf_pool_mutex_enter(buf_pool);
+	mutex_enter(&buf_pool->flush_state_mutex);
 
 	if (buf_pool->n_flush[flush_type] > 0
-	   || buf_pool->init_flush[flush_type] == TRUE) {
+	    || buf_pool->init_flush[flush_type] == TRUE) {
 
 		/* There is already a flush batch of the same type running */
 
-		buf_pool_mutex_exit(buf_pool);
+		mutex_exit(&buf_pool->flush_state_mutex);
 
 		return(FALSE);
 	}
 
 	buf_pool->init_flush[flush_type] = TRUE;
 
-	buf_pool_mutex_exit(buf_pool);
+	mutex_exit(&buf_pool->flush_state_mutex);
 
 	return(TRUE);
 }
@@ -1794,7 +1824,7 @@
 	buf_flush_t	flush_type)	/*!< in: BUF_FLUSH_LRU
 					or BUF_FLUSH_LIST */
 {
-	buf_pool_mutex_enter(buf_pool);
+	mutex_enter(&buf_pool->flush_state_mutex);
 
 	buf_pool->init_flush[flush_type] = FALSE;
 
@@ -1807,7 +1837,7 @@
 		os_event_set(buf_pool->no_flush[flush_type]);
 	}
 
-	buf_pool_mutex_exit(buf_pool);
+	mutex_exit(&buf_pool->flush_state_mutex);
 }
 
 /******************************************************************//**
@@ -1989,7 +2019,7 @@
 	ibool		freed;
 	bool		evict_zip;
 
-	buf_pool_mutex_enter(buf_pool);
+	mutex_enter(&buf_pool->LRU_list_mutex);
 
 	for (bpage = UT_LIST_GET_LAST(buf_pool->LRU), scanned = 1;
 	     bpage != NULL;
@@ -2006,6 +2036,8 @@
 		mutex_exit(block_mutex);
 	}
 
+	mutex_exit(&buf_pool->LRU_list_mutex);
+
 	MONITOR_INC_VALUE_CUMULATIVE(
 		MONITOR_LRU_SINGLE_FLUSH_SCANNED,
 		MONITOR_LRU_SINGLE_FLUSH_SCANNED_NUM_CALL,
@@ -2014,22 +2046,20 @@
 
 	if (!bpage) {
 		/* Can't find a single flushable page. */
-		buf_pool_mutex_exit(buf_pool);
 		return(FALSE);
 	}
 
-	/* The following call will release the buffer pool and
-	block mutex. */
+	/* The following call will release the buf_page_get_mutex() mutex. */
 	buf_flush_page(buf_pool, bpage, BUF_FLUSH_SINGLE_PAGE, true);
 
 	/* At this point the page has been written to the disk.
-	As we are not holding buffer pool or block mutex therefore
+	As we are not holding LRU list or buf_page_get_mutex() mutex therefore
 	we cannot use the bpage safely. It may have been plucked out
 	of the LRU list by some other thread or it may even have
 	relocated in case of a compressed page. We need to start
 	the scan of LRU list again to remove the block from the LRU
 	list and put it on the free list. */
-	buf_pool_mutex_enter(buf_pool);
+	mutex_enter(&buf_pool->LRU_list_mutex);
 
 	for (bpage = UT_LIST_GET_LAST(buf_pool->LRU);
 	     bpage != NULL;
@@ -2040,23 +2070,25 @@
 		block_mutex = buf_page_get_mutex(bpage);
 		mutex_enter(block_mutex);
 		ready = buf_flush_ready_for_replace(bpage);
-		mutex_exit(block_mutex);
 		if (ready) {
 			break;
 		}
+		mutex_exit(block_mutex);
 
 	}
 
 	if (!bpage) {
 		/* Can't find a single replaceable page. */
-		buf_pool_mutex_exit(buf_pool);
+		mutex_exit(&buf_pool->LRU_list_mutex);
 		return(FALSE);
 	}
 
 	evict_zip = !buf_LRU_evict_from_unzip_LRU(buf_pool);;
 
 	freed = buf_LRU_free_page(bpage, evict_zip);
-	buf_pool_mutex_exit(buf_pool);
+	if (!freed)
+		mutex_exit(&buf_pool->LRU_list_mutex);
+	mutex_exit(block_mutex);
 
 	return(freed);
 }
@@ -2082,9 +2114,7 @@
 
 		/* srv_LRU_scan_depth can be arbitrarily large value.
 		We cap it with current LRU size. */
-		buf_pool_mutex_enter(buf_pool);
 		scan_depth = UT_LIST_GET_LEN(buf_pool->LRU);
-		buf_pool_mutex_exit(buf_pool);
 
 		scan_depth = ut_min(srv_LRU_scan_depth, scan_depth);
 
@@ -2143,15 +2173,15 @@
 
 		buf_pool = buf_pool_from_array(i);
 
-		buf_pool_mutex_enter(buf_pool);
+		mutex_enter(&buf_pool->flush_state_mutex);
 
 		if (buf_pool->n_flush[BUF_FLUSH_LRU] > 0
 		   || buf_pool->init_flush[BUF_FLUSH_LRU]) {
 
-			buf_pool_mutex_exit(buf_pool);
+			mutex_exit(&buf_pool->flush_state_mutex);
 			buf_flush_wait_batch_end(buf_pool, BUF_FLUSH_LRU);
 		} else {
-			buf_pool_mutex_exit(buf_pool);
+			mutex_exit(&buf_pool->flush_state_mutex);
 		}
 	}
 }
@@ -2629,7 +2659,6 @@
 {
 	ulint		count = 0;
 
-	buf_pool_mutex_enter(buf_pool);
 	buf_flush_list_mutex_enter(buf_pool);
 
 	buf_page_t*	bpage;
@@ -2648,7 +2677,6 @@
 	}
 
 	buf_flush_list_mutex_exit(buf_pool);
-	buf_pool_mutex_exit(buf_pool);
 
 	return(count);
 }

=== modified file 'Percona-Server/storage/innobase/buf/buf0lru.cc'
--- Percona-Server/storage/innobase/buf/buf0lru.cc	2013-08-16 09:11:51 +0000
+++ Percona-Server/storage/innobase/buf/buf0lru.cc	2013-09-20 05:29:11 +0000
@@ -75,7 +75,7 @@
 /** When dropping the search hash index entries before deleting an ibd
 file, we build a local array of pages belonging to that tablespace
 in the buffer pool. Following is the size of that array.
-We also release buf_pool->mutex after scanning this many pages of the
+We also release buf_pool->LRU_list_mutex after scanning this many pages of the
 flush_list when dropping a table. This is to ensure that other threads
 are not blocked for extended period of time when using very large
 buffer pools. */
@@ -133,7 +133,7 @@
 If the block is compressed-only (BUF_BLOCK_ZIP_PAGE),
 the object will be freed.
 
-The caller must hold buf_pool->mutex, the buf_page_get_mutex() mutex
+The caller must hold buf_pool->LRU_list_mutex, the buf_page_get_mutex() mutex
 and the appropriate hash_lock. This function will release the
 buf_page_get_mutex() and the hash_lock.
 
@@ -170,7 +170,7 @@
 	buf_page_t*	bpage,		/*!< in: control block */
 	buf_pool_t*	buf_pool)	/*!< in: buffer pool instance */
 {
-	ut_ad(buf_pool_mutex_own(buf_pool));
+	ut_ad(mutex_own(&buf_pool->LRU_list_mutex));
 	ulint zip_size = page_zip_get_size(&bpage->zip);
 	buf_pool->stat.LRU_bytes += zip_size ? zip_size : UNIV_PAGE_SIZE;
 	ut_ad(buf_pool->stat.LRU_bytes <= buf_pool->curr_pool_size);
@@ -189,7 +189,7 @@
 	ulint	io_avg;
 	ulint	unzip_avg;
 
-	ut_ad(buf_pool_mutex_own(buf_pool));
+	ut_ad(mutex_own(&buf_pool->LRU_list_mutex));
 
 	/* If the unzip_LRU list is empty, we can only use the LRU. */
 	if (UT_LIST_GET_LEN(buf_pool->unzip_LRU) == 0) {
@@ -276,7 +276,7 @@
 	page_arr = static_cast<ulint*>(ut_malloc(
 		sizeof(ulint) * BUF_LRU_DROP_SEARCH_SIZE));
 
-	buf_pool_mutex_enter(buf_pool);
+	mutex_enter(&buf_pool->LRU_list_mutex);
 	num_entries = 0;
 
 scan_again:
@@ -285,6 +285,7 @@
 	while (bpage != NULL) {
 		buf_page_t*	prev_bpage;
 		ibool		is_fixed;
+		ib_mutex_t*	block_mutex = buf_page_get_mutex(bpage);
 
 		prev_bpage = UT_LIST_GET_PREV(LRU, bpage);
 
@@ -301,10 +302,10 @@
 			continue;
 		}
 
-		mutex_enter(&((buf_block_t*) bpage)->mutex);
+		mutex_enter(block_mutex);
 		is_fixed = bpage->buf_fix_count > 0
 			|| !((buf_block_t*) bpage)->index;
-		mutex_exit(&((buf_block_t*) bpage)->mutex);
+		mutex_exit(block_mutex);
 
 		if (is_fixed) {
 			goto next_page;
@@ -320,18 +321,18 @@
 			goto next_page;
 		}
 
-		/* Array full. We release the buf_pool->mutex to obey
+		/* Array full. We release the buf_pool->LRU_list_mutex to obey
 		the latching order. */
-		buf_pool_mutex_exit(buf_pool);
+		mutex_exit(&buf_pool->LRU_list_mutex);
 
 		buf_LRU_drop_page_hash_batch(
 			id, zip_size, page_arr, num_entries);
 
 		num_entries = 0;
 
-		buf_pool_mutex_enter(buf_pool);
+		mutex_enter(&buf_pool->LRU_list_mutex);
 
-		/* Note that we released the buf_pool mutex above
+		/* Note that we released the buf_pool->LRU_list_mutex above
 		after reading the prev_bpage during processing of a
 		page_hash_batch (i.e.: when the array was full).
 		Because prev_bpage could belong to a compressed-only
@@ -345,15 +346,15 @@
 		guarantee that ALL such entries will be dropped. */
 
 		/* If, however, bpage has been removed from LRU list
-		to the free list then we should restart the scan.
-		bpage->state is protected by buf_pool mutex. */
+		to the free list then we should restart the scan.  */
+
 		if (bpage
 		    && buf_page_get_state(bpage) != BUF_BLOCK_FILE_PAGE) {
 			goto scan_again;
 		}
 	}
 
-	buf_pool_mutex_exit(buf_pool);
+	mutex_exit(&buf_pool->LRU_list_mutex);
 
 	/* Drop any remaining batch of search hashed pages. */
 	buf_LRU_drop_page_hash_batch(id, zip_size, page_arr, num_entries);
@@ -373,27 +374,25 @@
 	buf_pool_t*	buf_pool,	/*!< in/out: buffer pool instance */
 	buf_page_t*	bpage)		/*!< in/out: current page */
 {
-	ib_mutex_t*	block_mutex;
+	ib_mutex_t*	block_mutex	= buf_page_get_mutex(bpage);
 
-	ut_ad(buf_pool_mutex_own(buf_pool));
+	ut_ad(mutex_own(&buf_pool->LRU_list_mutex));
+	ut_ad(mutex_own(block_mutex));
 	ut_ad(buf_page_in_file(bpage));
 
-	block_mutex = buf_page_get_mutex(bpage);
-
-	mutex_enter(block_mutex);
 	/* "Fix" the block so that the position cannot be
 	changed after we release the buffer pool and
 	block mutexes. */
 	buf_page_set_sticky(bpage);
 
-	/* Now it is safe to release the buf_pool->mutex. */
-	buf_pool_mutex_exit(buf_pool);
+	/* Now it is safe to release the LRU list mutex */
+	mutex_exit(&buf_pool->LRU_list_mutex);
 
 	mutex_exit(block_mutex);
 	/* Try and force a context switch. */
 	os_thread_yield();
 
-	buf_pool_mutex_enter(buf_pool);
+	mutex_enter(&buf_pool->LRU_list_mutex);
 
 	mutex_enter(block_mutex);
 	/* "Unfix" the block now that we have both the
@@ -413,21 +412,47 @@
 /*================*/
 	buf_pool_t*	buf_pool,	/*!< in/out: buffer pool instance */
 	buf_page_t*	bpage,		/*!< in/out: bpage to remove */
-	ulint		processed)	/*!< in: number of pages processed */
+	ulint		processed,	/*!< in: number of pages processed */
+	bool*		must_restart)	/*!< in/out: if true, we have to
+					restart the flush list scan */
 {
 	/* Every BUF_LRU_DROP_SEARCH_SIZE iterations in the
-	loop we release buf_pool->mutex to let other threads
+	loop we release buf_pool->LRU_list_mutex to let other threads
 	do their job but only if the block is not IO fixed. This
 	ensures that the block stays in its position in the
 	flush_list. */
 
 	if (bpage != NULL
 	    && processed >= BUF_LRU_DROP_SEARCH_SIZE
-	    && buf_page_get_io_fix(bpage) == BUF_IO_NONE) {
+	    && buf_page_get_io_fix_unlocked(bpage) == BUF_IO_NONE) {
+
+		ib_mutex_t*	block_mutex = buf_page_get_mutex(bpage);
 
 		buf_flush_list_mutex_exit(buf_pool);
 
-		/* Release the buffer pool and block mutex
+		/* We don't have to worry about bpage becoming a dangling
+		pointer by a compressed page flush list relocation because
+		buf_page_get_gen() won't be called for pages from this
+		tablespace.  */
+
+		mutex_enter(block_mutex);
+		/* Recheck the I/O fix and the flush list presence now that we
+		hold the right mutex */
+		if (UNIV_UNLIKELY(buf_page_get_io_fix(bpage) != BUF_IO_NONE
+				  || bpage->oldest_modification == 0)) {
+
+			mutex_exit(block_mutex);
+
+			*must_restart = true;
+
+			buf_flush_list_mutex_enter(buf_pool);
+
+			return false;
+		}
+
+		*must_restart = false;
+
+		/* Release the LRU list and buf_page_get_mutex() mutex
 		to give the other threads a go. */
 
 		buf_flush_yield(buf_pool, bpage);
@@ -456,18 +481,22 @@
 /*=====================*/
 	buf_pool_t*	buf_pool,	/*!< in/out: buffer pool instance */
 	buf_page_t*	bpage,		/*!< in/out: bpage to remove */
-	bool		flush)		/*!< in: flush to disk if true but
+	bool		flush,		/*!< in: flush to disk if true but
 					don't remove else remove without
 					flushing to disk */
+	bool*		must_restart)	/*!< in/out: if true, must restart the
+					flush list scan */
 {
-	ut_ad(buf_pool_mutex_own(buf_pool));
+	ib_mutex_t*	block_mutex	= buf_page_get_mutex(bpage);
+
+	ut_ad(mutex_own(&buf_pool->LRU_list_mutex));
 	ut_ad(buf_flush_list_mutex_own(buf_pool));
 
-	/* bpage->space and bpage->io_fix are protected by
-	buf_pool->mutex and block_mutex. It is safe to check
-	them while holding buf_pool->mutex only. */
+	/* It is safe to check bpage->space and bpage->io_fix while holding
+	buf_pool->LRU_list_mutex only. */
 
-	if (buf_page_get_io_fix(bpage) != BUF_IO_NONE) {
+	if (UNIV_UNLIKELY(buf_page_get_io_fix_unlocked(bpage)
+			  != BUF_IO_NONE)) {
 
 		/* We cannot remove this page during this scan
 		yet; maybe the system is currently reading it
@@ -476,22 +505,31 @@
 
 	}
 
-	ib_mutex_t*	block_mutex = buf_page_get_mutex(bpage);
 	bool		processed = false;
 
-	/* We have to release the flush_list_mutex to obey the
-	latching order. We are however guaranteed that the page
-	will stay in the flush_list and won't be relocated because
-	buf_flush_remove() and buf_flush_relocate_on_flush_list()
-	need buf_pool->mutex as well. */
-
 	buf_flush_list_mutex_exit(buf_pool);
 
+	/* We don't have to worry about bpage becoming a dangling
+	pointer by a compressed page flush list relocation because
+	buf_page_get_gen() won't be called for pages from this
+	tablespace.  */
+
 	mutex_enter(block_mutex);
 
-	ut_ad(bpage->oldest_modification != 0);
-
-	if (!flush) {
+	/* Recheck the page I/O fix and the flush list presence now
+	that we hold the right mutex. */
+	if (UNIV_UNLIKELY(buf_page_get_io_fix(bpage) != BUF_IO_NONE
+			  || bpage->oldest_modification == 0)) {
+
+		/* The page became I/O-fixed or is not on the flush
+		list anymore, this invalidates any flush-list-page
+		pointers we have. */
+
+		mutex_exit(block_mutex);
+
+		*must_restart = TRUE;
+
+	} else if (!flush) {
 
 		buf_flush_remove(bpage);
 
@@ -502,8 +540,10 @@
 	} else if (buf_flush_ready_for_flush(bpage,
 					     BUF_FLUSH_SINGLE_PAGE)) {
 
-		/* The following call will release the buffer pool
-		and block mutex. */
+		mutex_exit(&buf_pool->LRU_list_mutex);
+
+		/* The following call will release the buf_page_get_mutex()
+		mutex. */
 		buf_flush_page(buf_pool, bpage, BUF_FLUSH_SINGLE_PAGE, false);
 		ut_ad(!mutex_own(block_mutex));
 
@@ -511,7 +551,7 @@
 		post the writes to the operating system */
 		os_aio_simulated_wake_handler_threads();
 
-		buf_pool_mutex_enter(buf_pool);
+		mutex_enter(&buf_pool->LRU_list_mutex);
 
 		processed = true;
 	} else {
@@ -525,7 +565,7 @@
 	buf_flush_list_mutex_enter(buf_pool);
 
 	ut_ad(!mutex_own(block_mutex));
-	ut_ad(buf_pool_mutex_own(buf_pool));
+	ut_ad(mutex_own(&buf_pool->LRU_list_mutex));
 
 	return(processed);
 }
@@ -558,6 +598,7 @@
 	buf_flush_list_mutex_enter(buf_pool);
 
 rescan:
+	bool	must_restart = false;
 	bool	all_freed = true;
 
 	for (bpage = UT_LIST_GET_LAST(buf_pool->flush_list);
@@ -576,15 +617,16 @@
 			/* Skip this block, as it does not belong to
 			the target space. */
 
-		} else if (!buf_flush_or_remove_page(buf_pool, bpage, flush)) {
+		} else if (!buf_flush_or_remove_page(buf_pool, bpage, flush,
+						     &must_restart)) {
 
 			/* Remove was unsuccessful, we have to try again
 			by scanning the entire list from the end.
 			This also means that we never released the
-			buf_pool mutex. Therefore we can trust the prev
+			flush list mutex. Therefore we can trust the prev
 			pointer.
 			buf_flush_or_remove_page() released the
-			flush list mutex but not the buf_pool mutex.
+			flush list mutex but not the LRU list mutex.
 			Therefore it is possible that a new page was
 			added to the flush list. For example, in case
 			where we are at the head of the flush list and
@@ -602,17 +644,23 @@
 		} else if (flush) {
 
 			/* The processing was successful. And during the
-			processing we have released the buf_pool mutex
+			processing we have released all the buf_pool mutexes
 			when calling buf_page_flush(). We cannot trust
 			prev pointer. */
 			goto rescan;
+		} else if (UNIV_UNLIKELY(must_restart)) {
+
+			ut_ad(!all_freed);
+			break;
 		}
 
 		++processed;
 
 		/* Yield if we have hogged the CPU and mutexes for too long. */
-		if (buf_flush_try_yield(buf_pool, prev, processed)) {
+		if (buf_flush_try_yield(buf_pool, prev, processed,
+					&must_restart)) {
 
+			ut_ad(!must_restart);
 			/* Reset the batch size counter if we had to yield. */
 
 			processed = 0;
@@ -658,11 +706,11 @@
 	dberr_t		err;
 
 	do {
-		buf_pool_mutex_enter(buf_pool);
+		mutex_enter(&buf_pool->LRU_list_mutex);
 
 		err = buf_flush_or_remove_pages(buf_pool, id, flush, trx);
 
-		buf_pool_mutex_exit(buf_pool);
+		mutex_exit(&buf_pool->LRU_list_mutex);
 
 		ut_ad(buf_flush_validate(buf_pool));
 
@@ -695,7 +743,7 @@
 	ibool		all_freed;
 
 scan_again:
-	buf_pool_mutex_enter(buf_pool);
+	mutex_enter(&buf_pool->LRU_list_mutex);
 
 	all_freed = TRUE;
 
@@ -712,15 +760,16 @@
 
 		prev_bpage = UT_LIST_GET_PREV(LRU, bpage);
 
-		/* bpage->space and bpage->io_fix are protected by
-		buf_pool->mutex and the block_mutex. It is safe to check
-		them while holding buf_pool->mutex only. */
+		/* It is safe to check bpage->space and bpage->io_fix while
+		holding buf_pool->LRU_list_mutex only and later recheck
+		while holding the buf_page_get_mutex() mutex.  */
 
 		if (buf_page_get_space(bpage) != id) {
 			/* Skip this block, as it does not belong to
 			the space that is being invalidated. */
 			goto next_page;
-		} else if (buf_page_get_io_fix(bpage) != BUF_IO_NONE) {
+		} else if (UNIV_UNLIKELY(buf_page_get_io_fix_unlocked(bpage)
+					 != BUF_IO_NONE)) {
 			/* We cannot remove this page during this scan
 			yet; maybe the system is currently reading it
 			in, or flushing the modifications to the file */
@@ -738,7 +787,11 @@
 			block_mutex = buf_page_get_mutex(bpage);
 			mutex_enter(block_mutex);
 
-			if (bpage->buf_fix_count > 0) {
+			if (UNIV_UNLIKELY(
+				    buf_page_get_space(bpage) != id
+				    || bpage->buf_fix_count > 0
+				    || (buf_page_get_io_fix(bpage)
+					!= BUF_IO_NONE))) {
 
 				mutex_exit(block_mutex);
 
@@ -772,15 +825,15 @@
 			ulint	page_no;
 			ulint	zip_size;
 
-			buf_pool_mutex_exit(buf_pool);
+			mutex_exit(&buf_pool->LRU_list_mutex);
 
 			zip_size = buf_page_get_zip_size(bpage);
 			page_no = buf_page_get_page_no(bpage);
 
+			mutex_exit(block_mutex);
+
 			rw_lock_x_unlock(hash_lock);
 
-			mutex_exit(block_mutex);
-
 			/* Note that the following call will acquire
 			and release block->lock X-latch. */
 
@@ -800,7 +853,10 @@
 		/* Remove from the LRU list. */
 
 		if (buf_LRU_block_remove_hashed(bpage, true)) {
+
+			mutex_enter(block_mutex);
 			buf_LRU_block_free_hashed_page((buf_block_t*) bpage);
+			mutex_exit(block_mutex);
 		} else {
 			ut_ad(block_mutex == &buf_pool->zip_mutex);
 		}
@@ -817,7 +873,7 @@
 		bpage = prev_bpage;
 	}
 
-	buf_pool_mutex_exit(buf_pool);
+	mutex_exit(&buf_pool->LRU_list_mutex);
 
 	if (!all_freed) {
 		os_thread_sleep(20000);
@@ -921,7 +977,8 @@
 	buf_page_t*	b;
 	buf_pool_t*	buf_pool = buf_pool_from_bpage(bpage);
 
-	ut_ad(buf_pool_mutex_own(buf_pool));
+	ut_ad(mutex_own(&buf_pool->LRU_list_mutex));
+	ut_ad(mutex_own(&buf_pool->zip_mutex));
 	ut_ad(buf_page_get_state(bpage) == BUF_BLOCK_ZIP_PAGE);
 
 	/* Find the first successor of bpage in the LRU list
@@ -961,7 +1018,7 @@
 	ibool 		freed;
 	ulint		scanned;
 
-	ut_ad(buf_pool_mutex_own(buf_pool));
+	ut_ad(mutex_own(&buf_pool->LRU_list_mutex));
 
 	if (!buf_LRU_evict_from_unzip_LRU(buf_pool)) {
 		return(FALSE);
@@ -976,12 +1033,16 @@
 		buf_block_t*	prev_block = UT_LIST_GET_PREV(unzip_LRU,
 						block);
 
+		mutex_enter(&block->mutex);
+
 		ut_ad(buf_block_get_state(block) == BUF_BLOCK_FILE_PAGE);
 		ut_ad(block->in_unzip_LRU_list);
 		ut_ad(block->page.in_LRU_list);
 
 		freed = buf_LRU_free_page(&block->page, false);
 
+		mutex_exit(&block->mutex);
+
 		block = prev_block;
 	}
 
@@ -1009,7 +1070,7 @@
 	ibool		freed;
 	ulint		scanned;
 
-	ut_ad(buf_pool_mutex_own(buf_pool));
+	ut_ad(mutex_own(&buf_pool->LRU_list_mutex));
 
 	for (bpage = UT_LIST_GET_LAST(buf_pool->LRU),
 	     scanned = 1, freed = FALSE;
@@ -1020,12 +1081,19 @@
 		unsigned	accessed;
 		buf_page_t*	prev_bpage = UT_LIST_GET_PREV(LRU,
 						bpage);
+		ib_mutex_t*	block_mutex = buf_page_get_mutex(bpage);
 
 		ut_ad(buf_page_in_file(bpage));
 		ut_ad(bpage->in_LRU_list);
 
 		accessed = buf_page_is_accessed(bpage);
+
+		mutex_enter(block_mutex);
+
 		freed = buf_LRU_free_page(bpage, true);
+
+		mutex_exit(block_mutex);
+
 		if (freed && !accessed) {
 			/* Keep track of pages that are evicted without
 			ever being accessed. This gives us a measure of
@@ -1057,11 +1125,24 @@
 					if TRUE, otherwise scan only
 					'old' blocks. */
 {
-	ut_ad(buf_pool_mutex_own(buf_pool));
-
-	return(buf_LRU_free_from_unzip_LRU_list(buf_pool, scan_all)
-	       || buf_LRU_free_from_common_LRU_list(
-			buf_pool, scan_all));
+	ibool	freed = FALSE;
+	bool	use_unzip_list = UT_LIST_GET_LEN(buf_pool->unzip_LRU) > 0;
+
+	mutex_enter(&buf_pool->LRU_list_mutex);
+
+	if (use_unzip_list) {
+		freed = buf_LRU_free_from_unzip_LRU_list(buf_pool, scan_all);
+	}
+
+	if (!freed) {
+		freed = buf_LRU_free_from_common_LRU_list(buf_pool, scan_all);
+	}
+
+	if (!freed) {
+		mutex_exit(&buf_pool->LRU_list_mutex);
+	}
+
+	return(freed);
 }
 
 /******************************************************************//**
@@ -1082,8 +1163,6 @@
 
 		buf_pool = buf_pool_from_array(i);
 
-		buf_pool_mutex_enter(buf_pool);
-
 		if (!recv_recovery_on
 		    && UT_LIST_GET_LEN(buf_pool->free)
 		       + UT_LIST_GET_LEN(buf_pool->LRU)
@@ -1091,8 +1170,6 @@
 
 			ret = TRUE;
 		}
-
-		buf_pool_mutex_exit(buf_pool);
 	}
 
 	return(ret);
@@ -1110,9 +1187,9 @@
 {
 	buf_block_t*	block;
 
-	ut_ad(buf_pool_mutex_own(buf_pool));
+	mutex_enter(&buf_pool->free_list_mutex);
 
-	block = (buf_block_t*) UT_LIST_GET_FIRST(buf_pool->free);
+	block = (buf_block_t*) UT_LIST_GET_LAST(buf_pool->free);
 
 	if (block) {
 
@@ -1122,18 +1199,23 @@
 		ut_ad(!block->page.in_LRU_list);
 		ut_a(!buf_page_in_file(&block->page));
 		UT_LIST_REMOVE(list, buf_pool->free, (&block->page));
-
-		mutex_enter(&block->mutex);
-
 		buf_block_set_state(block, BUF_BLOCK_READY_FOR_USE);
+
+		mutex_exit(&buf_pool->free_list_mutex);
+
+		mutex_enter(&block->mutex);
+
 		UNIV_MEM_ALLOC(block->frame, UNIV_PAGE_SIZE);
 
 		ut_ad(buf_pool_from_block(block) == buf_pool);
 
 		mutex_exit(&block->mutex);
+		return(block);
 	}
 
-	return(block);
+	mutex_exit(&buf_pool->free_list_mutex);
+
+	return(NULL);
 }
 
 /******************************************************************//**
@@ -1147,8 +1229,6 @@
 /*===================================*/
 	const buf_pool_t*	buf_pool)	/*!< in: buffer pool instance */
 {
-	ut_ad(buf_pool_mutex_own(buf_pool));
-
 	if (!recv_recovery_on && UT_LIST_GET_LEN(buf_pool->free)
 	    + UT_LIST_GET_LEN(buf_pool->LRU) < buf_pool->curr_size / 20) {
 		ut_print_timestamp(stderr);
@@ -1253,10 +1333,10 @@
 	ibool		mon_value_was	= FALSE;
 	ibool		started_monitor	= FALSE;
 
+	ut_ad(!mutex_own(&buf_pool->LRU_list_mutex));
+
 	MONITOR_INC(MONITOR_LRU_GET_FREE_SEARCH);
 loop:
-	buf_pool_mutex_enter(buf_pool);
-
 	buf_LRU_check_size_of_non_data_objects(buf_pool);
 
 	/* If there is a block in the free list, take it */
@@ -1264,7 +1344,6 @@
 
 	if (block) {
 
-		buf_pool_mutex_exit(buf_pool);
 		ut_ad(buf_pool_from_block(block) == buf_pool);
 		memset(&block->page.zip, 0, sizeof block->page.zip);
 
@@ -1275,22 +1354,28 @@
 		return(block);
 	}
 
+	mutex_enter(&buf_pool->flush_state_mutex);
+
 	if (buf_pool->init_flush[BUF_FLUSH_LRU]
 	    && srv_use_doublewrite_buf
 	    && buf_dblwr != NULL) {
 
+		mutex_exit(&buf_pool->flush_state_mutex);
+
 		/* If there is an LRU flush happening in the background
 		then we wait for it to end instead of trying a single
 		page flush. If, however, we are not using doublewrite
 		buffer then it is better to do our own single page
 		flush instead of waiting for LRU flush to end. */
-		buf_pool_mutex_exit(buf_pool);
 		buf_flush_wait_batch_end(buf_pool, BUF_FLUSH_LRU);
 		goto loop;
 	}
 
+	mutex_exit(&buf_pool->flush_state_mutex);
+
 	freed = FALSE;
 	if (buf_pool->try_LRU_scan || n_iterations > 0) {
+
 		/* If no block was in the free list, search from the
 		end of the LRU list and try to free a block there.
 		If we are doing for the first time we'll scan only
@@ -1308,8 +1393,6 @@
 		}
 	}
 
-	buf_pool_mutex_exit(buf_pool);
-
 	if (freed) {
 		goto loop;
 
@@ -1395,7 +1478,7 @@
 	ulint	new_len;
 
 	ut_a(buf_pool->LRU_old);
-	ut_ad(buf_pool_mutex_own(buf_pool));
+	ut_ad(mutex_own(&buf_pool->LRU_list_mutex));
 	ut_ad(buf_pool->LRU_old_ratio >= BUF_LRU_OLD_RATIO_MIN);
 	ut_ad(buf_pool->LRU_old_ratio <= BUF_LRU_OLD_RATIO_MAX);
 #if BUF_LRU_OLD_RATIO_MIN * BUF_LRU_OLD_MIN_LEN <= BUF_LRU_OLD_RATIO_DIV * (BUF_LRU_OLD_TOLERANCE + 5)
@@ -1461,7 +1544,7 @@
 {
 	buf_page_t*	bpage;
 
-	ut_ad(buf_pool_mutex_own(buf_pool));
+	ut_ad(mutex_own(&buf_pool->LRU_list_mutex));
 	ut_a(UT_LIST_GET_LEN(buf_pool->LRU) == BUF_LRU_OLD_MIN_LEN);
 
 	/* We first initialize all blocks in the LRU list as old and then use
@@ -1496,7 +1579,7 @@
 	ut_ad(buf_pool);
 	ut_ad(bpage);
 	ut_ad(buf_page_in_file(bpage));
-	ut_ad(buf_pool_mutex_own(buf_pool));
+	ut_ad(mutex_own(&buf_pool->LRU_list_mutex));
 
 	if (buf_page_belongs_to_unzip_LRU(bpage)) {
 		buf_block_t*	block = (buf_block_t*) bpage;
@@ -1521,7 +1604,7 @@
 
 	ut_ad(buf_pool);
 	ut_ad(bpage);
-	ut_ad(buf_pool_mutex_own(buf_pool));
+	ut_ad(mutex_own(&buf_pool->LRU_list_mutex));
 
 	ut_a(buf_page_in_file(bpage));
 
@@ -1601,7 +1684,7 @@
 
 	ut_ad(buf_pool);
 	ut_ad(block);
-	ut_ad(buf_pool_mutex_own(buf_pool));
+	ut_ad(mutex_own(&buf_pool->LRU_list_mutex));
 
 	ut_a(buf_page_belongs_to_unzip_LRU(&block->page));
 
@@ -1630,7 +1713,7 @@
 
 	ut_ad(buf_pool);
 	ut_ad(bpage);
-	ut_ad(buf_pool_mutex_own(buf_pool));
+	ut_ad(mutex_own(&buf_pool->LRU_list_mutex));
 
 	ut_a(buf_page_in_file(bpage));
 
@@ -1686,7 +1769,7 @@
 
 	ut_ad(buf_pool);
 	ut_ad(bpage);
-	ut_ad(buf_pool_mutex_own(buf_pool));
+	ut_ad(mutex_own(&buf_pool->LRU_list_mutex));
 
 	ut_a(buf_page_in_file(bpage));
 	ut_ad(!bpage->in_LRU_list);
@@ -1770,7 +1853,7 @@
 {
 	buf_pool_t*	buf_pool = buf_pool_from_bpage(bpage);
 
-	ut_ad(buf_pool_mutex_own(buf_pool));
+	ut_ad(mutex_own(&buf_pool->LRU_list_mutex));
 
 	if (bpage->old) {
 		buf_pool->stat.n_pages_made_young++;
@@ -1796,12 +1879,14 @@
 Try to free a block.  If bpage is a descriptor of a compressed-only
 page, the descriptor object will be freed as well.
 
-NOTE: If this function returns true, it will temporarily
-release buf_pool->mutex.  Furthermore, the page frame will no longer be
-accessible via bpage.
-
-The caller must hold buf_pool->mutex and must not hold any
-buf_page_get_mutex() when calling this function.
+NOTE: If this function returns true, it will release the LRU list mutex,
+and temporarily release and relock the buf_page_get_mutex() mutex.
+Furthermore, the page frame will no longer be accessible via bpage.  If this
+function returns false, the buf_page_get_mutex() might be temporarily released
+and relocked too.
+
+The caller must hold the LRU list and buf_page_get_mutex() mutexes.
+
 @return true if freed, false otherwise. */
 UNIV_INTERN
 bool
@@ -1819,13 +1904,11 @@
 
 	ib_mutex_t*	block_mutex = buf_page_get_mutex(bpage);
 
-	ut_ad(buf_pool_mutex_own(buf_pool));
+	ut_ad(mutex_own(&buf_pool->LRU_list_mutex));
+	ut_ad(mutex_own(block_mutex));
 	ut_ad(buf_page_in_file(bpage));
 	ut_ad(bpage->in_LRU_list);
 
-	rw_lock_x_lock(hash_lock);
-	mutex_enter(block_mutex);
-
 #if UNIV_WORD_SIZE == 4
 	/* On 32-bit systems, there is no padding in buf_page_t.  On
 	other systems, Valgrind could complain about uninitialized pad
@@ -1836,7 +1919,7 @@
 	if (!buf_page_can_relocate(bpage)) {
 
 		/* Do not free buffer-fixed or I/O-fixed blocks. */
-		goto func_exit;
+		return(false);
 	}
 
 #ifdef UNIV_IBUF_COUNT_DEBUG
@@ -1848,7 +1931,7 @@
 		/* Do not completely free dirty blocks. */
 
 		if (bpage->oldest_modification) {
-			goto func_exit;
+			return(false);
 		}
 	} else if ((bpage->oldest_modification)
 		   && (buf_page_get_state(bpage)
@@ -1857,18 +1940,13 @@
 		ut_ad(buf_page_get_state(bpage)
 		      == BUF_BLOCK_ZIP_DIRTY);
 
-func_exit:
-		rw_lock_x_unlock(hash_lock);
-		mutex_exit(block_mutex);
 		return(false);
 
 	} else if (buf_page_get_state(bpage) == BUF_BLOCK_FILE_PAGE) {
 		b = buf_page_alloc_descriptor();
 		ut_a(b);
-		memcpy(b, bpage, sizeof *b);
 	}
 
-	ut_ad(buf_pool_mutex_own(buf_pool));
 	ut_ad(buf_page_in_file(bpage));
 	ut_ad(bpage->in_LRU_list);
 	ut_ad(!bpage->in_flush_list == !bpage->oldest_modification);
@@ -1887,12 +1965,45 @@
 	}
 #endif /* UNIV_DEBUG */
 
-#ifdef UNIV_SYNC_DEBUG
-        ut_ad(rw_lock_own(hash_lock, RW_LOCK_EX));
-#endif /* UNIV_SYNC_DEBUG */
-	ut_ad(buf_page_can_relocate(bpage));
+	mutex_exit(block_mutex);
+
+	rw_lock_x_lock(hash_lock);
+	mutex_enter(block_mutex);
+
+	if (UNIV_UNLIKELY(!buf_page_can_relocate(bpage)
+			  || ((zip || !bpage->zip.data)
+			      && bpage->oldest_modification))) {
+
+not_freed:
+		rw_lock_x_unlock(hash_lock);
+		if (b) {
+			buf_page_free_descriptor(b);
+		}
+
+		return(false);
+	} else if (UNIV_UNLIKELY(bpage->oldest_modification
+				 && (buf_page_get_state(bpage)
+				     != BUF_BLOCK_FILE_PAGE))) {
+
+		ut_ad(buf_page_get_state(bpage)
+		      == BUF_BLOCK_ZIP_DIRTY);
+		goto not_freed;
+	}
+
+	if (b) {
+		memcpy(b, bpage, sizeof *b);
+	}
 
 	if (!buf_LRU_block_remove_hashed(bpage, zip)) {
+
+		mutex_exit(&buf_pool->LRU_list_mutex);
+
+		if (b) {
+			buf_page_free_descriptor(b);
+		}
+
+		mutex_enter(block_mutex);
+
 		return(true);
 	}
 
@@ -1997,6 +2108,8 @@
 			buf_LRU_add_block_low(b, buf_page_is_old(b));
 		}
 
+		mutex_enter(&buf_pool->zip_mutex);
+		rw_lock_x_unlock(hash_lock);
 		if (b->state == BUF_BLOCK_ZIP_PAGE) {
 #if defined UNIV_DEBUG || defined UNIV_BUF_DEBUG
 			buf_LRU_insert_zip_clean(b);
@@ -2008,35 +2121,16 @@
 
 		bpage->zip.data = NULL;
 		page_zip_set_size(&bpage->zip, 0);
-		mutex_exit(block_mutex);
 
 		/* Prevent buf_page_get_gen() from
-		decompressing the block while we release
-		buf_pool->mutex and block_mutex. */
-		block_mutex = buf_page_get_mutex(b);
-		mutex_enter(block_mutex);
+		decompressing the block while we release block_mutex. */
 		buf_page_set_sticky(b);
-		mutex_exit(block_mutex);
-
-		rw_lock_x_unlock(hash_lock);
-
-	} else {
-
-		/* There can be multiple threads doing an LRU scan to
-		free a block. The page_cleaner thread can be doing an
-		LRU batch whereas user threads can potentially be doing
-		multiple single page flushes. As we release
-		buf_pool->mutex below we need to make sure that no one
-		else considers this block as a victim for page
-		replacement. This block is already out of page_hash
-		and we are about to remove it from the LRU list and put
-		it on the free list. */
-		mutex_enter(block_mutex);
-		buf_page_set_sticky(bpage);
-		mutex_exit(block_mutex);
+		mutex_exit(&buf_pool->zip_mutex);
+		mutex_exit(block_mutex);
+
 	}
 
-	buf_pool_mutex_exit(buf_pool);
+	mutex_exit(&buf_pool->LRU_list_mutex);
 
 	/* Remove possible adaptive hash index on the page.
 	The page was declared uninitialized by
@@ -2069,13 +2163,17 @@
 				checksum);
 	}
 
-	buf_pool_mutex_enter(buf_pool);
-
 	mutex_enter(block_mutex);
-	buf_page_unset_sticky(b != NULL ? b : bpage);
-	mutex_exit(block_mutex);
+
+	if (b) {
+		mutex_enter(&buf_pool->zip_mutex);
+		buf_page_unset_sticky(b);
+		mutex_exit(&buf_pool->zip_mutex);
+	}
 
 	buf_LRU_block_free_hashed_page((buf_block_t*) bpage);
+	ut_ad(mutex_own(block_mutex));
+	ut_ad(!mutex_own(&buf_pool->LRU_list_mutex));
 	return(true);
 }
 
@@ -2091,7 +2189,6 @@
 	buf_pool_t*	buf_pool = buf_pool_from_block(block);
 
 	ut_ad(block);
-	ut_ad(buf_pool_mutex_own(buf_pool));
 	ut_ad(mutex_own(&block->mutex));
 
 	switch (buf_block_get_state(block)) {
@@ -2109,8 +2206,6 @@
 	ut_ad(!block->page.in_flush_list);
 	ut_ad(!block->page.in_LRU_list);
 
-	buf_block_set_state(block, BUF_BLOCK_NOT_USED);
-
 	UNIV_MEM_ALLOC(block->frame, UNIV_PAGE_SIZE);
 #ifdef UNIV_DEBUG
 	/* Wipe contents of page to reveal possible stale pointers to it */
@@ -2125,18 +2220,19 @@
 	if (data) {
 		block->page.zip.data = NULL;
 		mutex_exit(&block->mutex);
-		buf_pool_mutex_exit_forbid(buf_pool);
 
 		buf_buddy_free(
 			buf_pool, data, page_zip_get_size(&block->page.zip));
 
-		buf_pool_mutex_exit_allow(buf_pool);
 		mutex_enter(&block->mutex);
 		page_zip_set_size(&block->page.zip, 0);
 	}
 
+	mutex_enter(&buf_pool->free_list_mutex);
+	buf_block_set_state(block, BUF_BLOCK_NOT_USED);
 	UT_LIST_ADD_FIRST(list, buf_pool->free, (&block->page));
 	ut_d(block->page.in_free_list = TRUE);
+	mutex_exit(&buf_pool->free_list_mutex);
 
 	UNIV_MEM_ASSERT_AND_FREE(block->frame, UNIV_PAGE_SIZE);
 }
@@ -2146,7 +2242,7 @@
 If the block is compressed-only (BUF_BLOCK_ZIP_PAGE),
 the object will be freed.
 
-The caller must hold buf_pool->mutex, the buf_page_get_mutex() mutex
+The caller must hold buf_pool->LRU_list_mutex, the buf_page_get_mutex() mutex
 and the appropriate hash_lock. This function will release the
 buf_page_get_mutex() and the hash_lock.
 
@@ -2171,7 +2267,7 @@
 	rw_lock_t*		hash_lock;
 
 	ut_ad(bpage);
-	ut_ad(buf_pool_mutex_own(buf_pool));
+	ut_ad(mutex_own(&buf_pool->LRU_list_mutex));
 	ut_ad(mutex_own(buf_page_get_mutex(bpage)));
 
 	fold = buf_page_address_fold(bpage->space, bpage->offset);
@@ -2287,7 +2383,7 @@
 #if defined UNIV_DEBUG || defined UNIV_BUF_DEBUG
 		mutex_exit(buf_page_get_mutex(bpage));
 		rw_lock_x_unlock(hash_lock);
-		buf_pool_mutex_exit(buf_pool);
+		mutex_exit(&buf_pool->LRU_list_mutex);
 		buf_print();
 		buf_LRU_print();
 		buf_validate();
@@ -2314,13 +2410,11 @@
 
 		mutex_exit(&buf_pool->zip_mutex);
 		rw_lock_x_unlock(hash_lock);
-		buf_pool_mutex_exit_forbid(buf_pool);
 
 		buf_buddy_free(
 			buf_pool, bpage->zip.data,
 			page_zip_get_size(&bpage->zip));
 
-		buf_pool_mutex_exit_allow(buf_pool);
 		buf_page_free_descriptor(bpage);
 		return(false);
 
@@ -2344,14 +2438,15 @@
 		page_hash. Only possibility is when while invalidating
 		a tablespace we buffer fix the prev_page in LRU to
 		avoid relocation during the scan. But that is not
-		possible because we are holding buf_pool mutex.
+		possible because we are holding LRU list mutex.
 
 		2) Not possible because in buf_page_init_for_read()
-		we do a look up of page_hash while holding buf_pool
-		mutex and since we are holding buf_pool mutex here
+		we do a look up of page_hash while holding LRU list
+		mutex and since we are holding LRU list mutex here
 		and by the time we'll release it in the caller we'd
 		have inserted the compressed only descriptor in the
 		page_hash. */
+		ut_ad(mutex_own(&buf_pool->LRU_list_mutex));
 		rw_lock_x_unlock(hash_lock);
 		mutex_exit(&((buf_block_t*) bpage)->mutex);
 
@@ -2363,13 +2458,11 @@
 			ut_ad(!bpage->in_free_list);
 			ut_ad(!bpage->in_flush_list);
 			ut_ad(!bpage->in_LRU_list);
-			buf_pool_mutex_exit_forbid(buf_pool);
 
 			buf_buddy_free(
 				buf_pool, data,
 				page_zip_get_size(&bpage->zip));
 
-			buf_pool_mutex_exit_allow(buf_pool);
 			page_zip_set_size(&bpage->zip, 0);
 		}
 
@@ -2397,16 +2490,11 @@
 	buf_block_t*	block)	/*!< in: block, must contain a file page and
 				be in a state where it can be freed */
 {
-#ifdef UNIV_DEBUG
-	buf_pool_t*	buf_pool = buf_pool_from_block(block);
-	ut_ad(buf_pool_mutex_own(buf_pool));
-#endif
+	ut_ad(mutex_own(&block->mutex));
 
-	mutex_enter(&block->mutex);
 	buf_block_set_state(block, BUF_BLOCK_MEMORY);
 
 	buf_LRU_block_free_non_file_page(block);
-	mutex_exit(&block->mutex);
 }
 
 /******************************************************************//**
@@ -2419,19 +2507,26 @@
 				be in a state where it can be freed; there
 				may or may not be a hash index to the page */
 {
+#if defined(UNIV_DEBUG) || defined(UNIV_SYNC_DEBUG)
 	buf_pool_t*	buf_pool = buf_pool_from_bpage(bpage);
+#endif
+#ifdef UNIV_SYNC_DEBUG
 	const ulint	fold = buf_page_address_fold(bpage->space,
 						     bpage->offset);
 	rw_lock_t*	hash_lock = buf_page_hash_lock_get(buf_pool, fold);
+#endif
 	ib_mutex_t*	block_mutex = buf_page_get_mutex(bpage);
 
-	ut_ad(buf_pool_mutex_own(buf_pool));
-
-	rw_lock_x_lock(hash_lock);
-	mutex_enter(block_mutex);
+	ut_ad(mutex_own(&buf_pool->LRU_list_mutex));
+	ut_ad(mutex_own(block_mutex));
+#ifdef UNIV_SYNC_DEBUG
+	ut_ad(rw_lock_own(hash_lock, RW_LOCK_EX));
+#endif
 
 	if (buf_LRU_block_remove_hashed(bpage, true)) {
+		mutex_enter(block_mutex);
 		buf_LRU_block_free_hashed_page((buf_block_t*) bpage);
+		mutex_exit(block_mutex);
 	}
 
 	/* buf_LRU_block_remove_hashed() releases hash_lock and block_mutex */
@@ -2466,7 +2561,7 @@
 	}
 
 	if (adjust) {
-		buf_pool_mutex_enter(buf_pool);
+		mutex_enter(&buf_pool->LRU_list_mutex);
 
 		if (ratio != buf_pool->LRU_old_ratio) {
 			buf_pool->LRU_old_ratio = ratio;
@@ -2478,7 +2573,7 @@
 			}
 		}
 
-		buf_pool_mutex_exit(buf_pool);
+		mutex_exit(&buf_pool->LRU_list_mutex);
 	} else {
 		buf_pool->LRU_old_ratio = ratio;
 	}
@@ -2583,7 +2678,7 @@
 	ulint		new_len;
 
 	ut_ad(buf_pool);
-	buf_pool_mutex_enter(buf_pool);
+	mutex_enter(&buf_pool->LRU_list_mutex);
 
 	if (UT_LIST_GET_LEN(buf_pool->LRU) >= BUF_LRU_OLD_MIN_LEN) {
 
@@ -2641,6 +2736,10 @@
 
 	ut_a(buf_pool->LRU_old_len == old_len);
 
+	mutex_exit(&buf_pool->LRU_list_mutex);
+
+	mutex_enter(&buf_pool->free_list_mutex);
+
 	UT_LIST_VALIDATE(list, buf_page_t, buf_pool->free, CheckInFreeList());
 
 	for (bpage = UT_LIST_GET_FIRST(buf_pool->free);
@@ -2650,6 +2749,10 @@
 		ut_a(buf_page_get_state(bpage) == BUF_BLOCK_NOT_USED);
 	}
 
+	mutex_exit(&buf_pool->free_list_mutex);
+
+	mutex_enter(&buf_pool->LRU_list_mutex);
+
 	UT_LIST_VALIDATE(
                 unzip_LRU, buf_block_t, buf_pool->unzip_LRU,
                 CheckUnzipLRUAndLRUList());
@@ -2663,7 +2766,7 @@
 		ut_a(buf_page_belongs_to_unzip_LRU(&block->page));
 	}
 
-	buf_pool_mutex_exit(buf_pool);
+	mutex_exit(&buf_pool->LRU_list_mutex);
 }
 
 /**********************************************************************//**
@@ -2699,7 +2802,7 @@
 	const buf_page_t*	bpage;
 
 	ut_ad(buf_pool);
-	buf_pool_mutex_enter(buf_pool);
+	mutex_enter(&buf_pool->LRU_list_mutex);
 
 	bpage = UT_LIST_GET_FIRST(buf_pool->LRU);
 
@@ -2756,7 +2859,7 @@
 		bpage = UT_LIST_GET_NEXT(LRU, bpage);
 	}
 
-	buf_pool_mutex_exit(buf_pool);
+	mutex_exit(&buf_pool->LRU_list_mutex);
 }
 
 /**********************************************************************//**

=== modified file 'Percona-Server/storage/innobase/buf/buf0rea.cc'
--- Percona-Server/storage/innobase/buf/buf0rea.cc	2013-08-06 15:16:34 +0000
+++ Percona-Server/storage/innobase/buf/buf0rea.cc	2013-09-20 05:29:11 +0000
@@ -63,10 +63,15 @@
 	buf_pool_t*	buf_pool = buf_pool_from_bpage(bpage);
 	const bool	uncompressed = (buf_page_get_state(bpage)
 					== BUF_BLOCK_FILE_PAGE);
+	const ulint	fold = buf_page_address_fold(bpage->space,
+						     bpage->offset);
+	rw_lock_t*	hash_lock = buf_page_hash_lock_get(buf_pool, fold);
+
+	mutex_enter(&buf_pool->LRU_list_mutex);
+	rw_lock_x_lock(hash_lock);
+	mutex_enter(buf_page_get_mutex(bpage));
 
 	/* First unfix and release lock on the bpage */
-	buf_pool_mutex_enter(buf_pool);
-	mutex_enter(buf_page_get_mutex(bpage));
 	ut_ad(buf_page_get_io_fix(bpage) == BUF_IO_READ);
 	ut_ad(bpage->buf_fix_count == 0);
 
@@ -79,15 +84,13 @@
 			BUF_IO_READ);
 	}
 
-	mutex_exit(buf_page_get_mutex(bpage));
-
 	/* remove the block from LRU list */
 	buf_LRU_free_one_page(bpage);
 
+	mutex_exit(&buf_pool->LRU_list_mutex);
+
 	ut_ad(buf_pool->n_pend_reads > 0);
-	buf_pool->n_pend_reads--;
-
-	buf_pool_mutex_exit(buf_pool);
+	os_atomic_decrement_ulint(&buf_pool->n_pend_reads, 1);
 }
 
 /********************************************************************//**
@@ -216,6 +219,7 @@
 #endif
 
 	ut_ad(buf_page_in_file(bpage));
+	ut_ad(!mutex_own(&buf_pool_from_bpage(bpage)->LRU_list_mutex));
 
 	if (sync) {
 		thd_wait_begin(NULL, THD_WAIT_DISKIO);
@@ -332,11 +336,8 @@
 		high = fil_space_get_size(space);
 	}
 
-	buf_pool_mutex_enter(buf_pool);
-
 	if (buf_pool->n_pend_reads
 	    > buf_pool->curr_size / BUF_READ_AHEAD_PEND_LIMIT) {
-		buf_pool_mutex_exit(buf_pool);
 
 		return(0);
 	}
@@ -345,8 +346,12 @@
 	that is, reside near the start of the LRU list. */
 
 	for (i = low; i < high; i++) {
+
+		rw_lock_t*	hash_lock;
+
 		const buf_page_t* bpage =
-			buf_page_hash_get(buf_pool, space, i);
+			buf_page_hash_get_s_locked(buf_pool, space, i,
+						   &hash_lock);
 
 		if (bpage
 		    && buf_page_is_accessed(bpage)
@@ -357,13 +362,16 @@
 			if (recent_blocks
 			    >= BUF_READ_AHEAD_RANDOM_THRESHOLD(buf_pool)) {
 
-				buf_pool_mutex_exit(buf_pool);
+				rw_lock_s_unlock(hash_lock);
 				goto read_ahead;
 			}
 		}
+
+		if (bpage) {
+			rw_lock_s_unlock(hash_lock);
+		}
 	}
 
-	buf_pool_mutex_exit(buf_pool);
 	/* Do nothing */
 	return(0);
 
@@ -551,6 +559,7 @@
 	buf_page_t*	bpage;
 	buf_frame_t*	frame;
 	buf_page_t*	pred_bpage	= NULL;
+	unsigned	pred_bpage_is_accessed = 0;
 	ulint		pred_offset;
 	ulint		succ_offset;
 	ulint		count;
@@ -602,10 +611,7 @@
 
 	tablespace_version = fil_space_get_version(space);
 
-	buf_pool_mutex_enter(buf_pool);
-
 	if (high > fil_space_get_size(space)) {
-		buf_pool_mutex_exit(buf_pool);
 		/* The area is not whole, return */
 
 		return(0);
@@ -613,7 +619,6 @@
 
 	if (buf_pool->n_pend_reads
 	    > buf_pool->curr_size / BUF_READ_AHEAD_PEND_LIMIT) {
-		buf_pool_mutex_exit(buf_pool);
 
 		return(0);
 	}
@@ -636,7 +641,11 @@
 	fail_count = 0;
 
 	for (i = low; i < high; i++) {
-		bpage = buf_page_hash_get(buf_pool, space, i);
+
+		rw_lock_t*	hash_lock;
+
+		bpage = buf_page_hash_get_s_locked(buf_pool, space, i,
+						   &hash_lock);
 
 		if (bpage == NULL || !buf_page_is_accessed(bpage)) {
 			/* Not accessed */
@@ -653,7 +662,7 @@
 			a little against this. */
 			int res = ut_ulint_cmp(
 				buf_page_is_accessed(bpage),
-				buf_page_is_accessed(pred_bpage));
+				pred_bpage_is_accessed);
 			/* Accesses not in the right order */
 			if (res != 0 && res != asc_or_desc) {
 				fail_count++;
@@ -662,12 +671,20 @@
 
 		if (fail_count > threshold) {
 			/* Too many failures: return */
-			buf_pool_mutex_exit(buf_pool);
+			if (bpage) {
+				rw_lock_s_unlock(hash_lock);
+			}
 			return(0);
 		}
 
-		if (bpage && buf_page_is_accessed(bpage)) {
-			pred_bpage = bpage;
+		if (bpage) {
+			if (buf_page_is_accessed(bpage)) {
+				pred_bpage = bpage;
+				pred_bpage_is_accessed
+					= buf_page_is_accessed(bpage);
+			}
+
+			rw_lock_s_unlock(hash_lock);
 		}
 	}
 
@@ -677,7 +694,6 @@
 	bpage = buf_page_hash_get(buf_pool, space, offset);
 
 	if (bpage == NULL) {
-		buf_pool_mutex_exit(buf_pool);
 
 		return(0);
 	}
@@ -703,8 +719,6 @@
 	pred_offset = fil_page_get_prev(frame);
 	succ_offset = fil_page_get_next(frame);
 
-	buf_pool_mutex_exit(buf_pool);
-
 	if ((offset == low) && (succ_offset == offset + 1)) {
 
 		/* This is ok, we can continue */
@@ -961,7 +975,8 @@
 
 		os_aio_print_debug = FALSE;
 		buf_pool = buf_pool_get(space, page_nos[i]);
-		while (buf_pool->n_pend_reads >= recv_n_pool_free_frames / 2) {
+		while (buf_pool->n_pend_reads
+		       >= recv_n_pool_free_frames / 2) {
 
 			os_aio_simulated_wake_handler_threads();
 			os_thread_sleep(10000);

=== modified file 'Percona-Server/storage/innobase/fsp/fsp0fsp.cc'
--- Percona-Server/storage/innobase/fsp/fsp0fsp.cc	2013-05-30 12:47:19 +0000
+++ Percona-Server/storage/innobase/fsp/fsp0fsp.cc	2013-09-20 05:29:11 +0000
@@ -2870,7 +2870,7 @@
 
 	/* The convoluted mutex acquire is to overcome latching order
 	issues: The problem is that the fil_mutex is at a lower level
-	than the tablespace latch and the buffer pool mutex. We have to
+	than the tablespace latch and the buffer pool mutexes. We have to
 	first prevent any operations on the file system by acquiring the
 	dictionary mutex. Then acquire the tablespace latch to obey the
 	latching order and then release the dictionary mutex. That way we

=== modified file 'Percona-Server/storage/innobase/handler/ha_innodb.cc'
--- Percona-Server/storage/innobase/handler/ha_innodb.cc	2013-08-30 13:23:53 +0000
+++ Percona-Server/storage/innobase/handler/ha_innodb.cc	2013-09-20 05:29:11 +0000
@@ -294,6 +294,11 @@
 #  endif /* !PFS_SKIP_BUFFER_MUTEX_RWLOCK */
 	{&buf_pool_mutex_key, "buf_pool_mutex", 0},
 	{&buf_pool_zip_mutex_key, "buf_pool_zip_mutex", 0},
+	{&buf_pool_LRU_list_mutex_key, "buf_pool_LRU_list_mutex", 0},
+	{&buf_pool_free_list_mutex_key, "buf_pool_free_list_mutex", 0},
+	{&buf_pool_zip_free_mutex_key, "buf_pool_zip_free_mutex", 0},
+	{&buf_pool_zip_hash_mutex_key, "buf_pool_zip_hash_mutex", 0},
+	{&buf_pool_flush_state_mutex_key, "buf_pool_flush_state_mutex", 0},
 	{&cache_last_read_mutex_key, "cache_last_read_mutex", 0},
 	{&dict_foreign_err_mutex_key, "dict_foreign_err_mutex", 0},
 	{&dict_sys_mutex_key, "dict_sys_mutex", 0},
@@ -15485,7 +15490,7 @@
 	for (ulint i = 0; i < srv_buf_pool_instances; i++) {
 		buf_pool_t*	buf_pool = &buf_pool_ptr[i];
 
-		buf_pool_mutex_enter(buf_pool);
+		mutex_enter(&buf_pool->LRU_list_mutex);
 
 		for (buf_block_t* block = UT_LIST_GET_LAST(
 			     buf_pool->unzip_LRU);
@@ -15497,14 +15502,19 @@
 			ut_ad(block->in_unzip_LRU_list);
 			ut_ad(block->page.in_LRU_list);
 
+			mutex_enter(&block->mutex);
 			if (!buf_LRU_free_page(&block->page, false)) {
+				mutex_exit(&block->mutex);
 				all_evicted = false;
+			} else {
+				mutex_exit(&block->mutex);
+				mutex_enter(&buf_pool->LRU_list_mutex);
 			}
 
 			block = prev_block;
 		}
 
-		buf_pool_mutex_exit(buf_pool);
+		mutex_exit(&buf_pool->LRU_list_mutex);
 	}
 
 	return(all_evicted);

=== modified file 'Percona-Server/storage/innobase/handler/i_s.cc'
--- Percona-Server/storage/innobase/handler/i_s.cc	2013-08-14 03:57:21 +0000
+++ Percona-Server/storage/innobase/handler/i_s.cc	2013-09-20 05:29:11 +0000
@@ -2103,7 +2103,7 @@
 
 		buf_pool = buf_pool_from_array(i);
 
-		buf_pool_mutex_enter(buf_pool);
+		mutex_enter(&buf_pool->zip_free_mutex);
 
 		for (uint x = 0; x <= BUF_BUDDY_SIZES; x++) {
 			buf_buddy_stat_t*	buddy_stat;
@@ -2122,7 +2122,8 @@
 				(ulong) (buddy_stat->relocated_usec / 1000000));
 
 			if (reset) {
-				/* This is protected by buf_pool->mutex. */
+				/* This is protected by
+				buf_pool->zip_free_mutex. */
 				buddy_stat->relocated = 0;
 				buddy_stat->relocated_usec = 0;
 			}
@@ -2133,7 +2134,7 @@
 			}
 		}
 
-		buf_pool_mutex_exit(buf_pool);
+		mutex_exit(&buf_pool->zip_free_mutex);
 
 		if (status) {
 			break;
@@ -4954,12 +4955,16 @@
 					out: structure filled with scanned
 					info */
 {
+	ib_mutex_t*	mutex = buf_page_get_mutex(bpage);
+
 	ut_ad(pool_id < MAX_BUFFER_POOLS);
 
 	page_info->pool_id = pool_id;
 
 	page_info->block_id = pos;
 
+	mutex_enter(mutex);
+
 	page_info->page_state = buf_page_get_state(bpage);
 
 	/* Only fetch information for buffers that map to a tablespace,
@@ -4998,6 +5003,7 @@
 			break;
 		case BUF_IO_READ:
 			page_info->page_type = I_S_PAGE_TYPE_UNKNOWN;
+			mutex_exit(mutex);
 			return;
 		}
 
@@ -5018,6 +5024,8 @@
 	} else {
 		page_info->page_type = I_S_PAGE_TYPE_UNKNOWN;
 	}
+
+	mutex_exit(mutex);
 }
 
 /*******************************************************************//**
@@ -5075,7 +5083,6 @@
 			buffer pool info printout, we are not required to
 			preserve the overall consistency, so we can
 			release mutex periodically */
-			buf_pool_mutex_enter(buf_pool);
 
 			/* GO through each block in the chunk */
 			for (n_blocks = num_to_process; n_blocks--; block++) {
@@ -5086,8 +5093,6 @@
 				num_page++;
 			}
 
-			buf_pool_mutex_exit(buf_pool);
-
 			/* Fill in information schema table with information
 			just collected from the buffer chunk scan */
 			status = i_s_innodb_buffer_page_fill(
@@ -5609,9 +5614,9 @@
 	DBUG_ENTER("i_s_innodb_fill_buffer_lru");
 	RETURN_IF_INNODB_NOT_STARTED(tables->schema_table_name);
 
-	/* Obtain buf_pool mutex before allocate info_buffer, since
+	/* Obtain buf_pool->LRU_list_mutex before allocate info_buffer, since
 	UT_LIST_GET_LEN(buf_pool->LRU) could change */
-	buf_pool_mutex_enter(buf_pool);
+	mutex_enter(&buf_pool->LRU_list_mutex);
 
 	lru_len = UT_LIST_GET_LEN(buf_pool->LRU);
 
@@ -5645,7 +5650,7 @@
 	ut_ad(lru_pos == UT_LIST_GET_LEN(buf_pool->LRU));
 
 exit:
-	buf_pool_mutex_exit(buf_pool);
+	mutex_exit(&buf_pool->LRU_list_mutex);
 
 	if (info_buffer) {
 		status = i_s_innodb_buf_page_lru_fill(

=== modified file 'Percona-Server/storage/innobase/ibuf/ibuf0ibuf.cc'
--- Percona-Server/storage/innobase/ibuf/ibuf0ibuf.cc	2013-09-02 10:01:38 +0000
+++ Percona-Server/storage/innobase/ibuf/ibuf0ibuf.cc	2013-09-20 05:29:11 +0000
@@ -4611,7 +4611,7 @@
 	ut_ad(!block || buf_block_get_space(block) == space);
 	ut_ad(!block || buf_block_get_page_no(block) == page_no);
 	ut_ad(!block || buf_block_get_zip_size(block) == zip_size);
-	ut_ad(!block || buf_block_get_io_fix(block) == BUF_IO_READ);
+	ut_ad(!block || buf_block_get_io_fix_unlocked(block) == BUF_IO_READ);
 
 	if (srv_force_recovery >= SRV_FORCE_NO_IBUF_MERGE
 	    || trx_sys_hdr_page(space, page_no)) {

=== modified file 'Percona-Server/storage/innobase/include/buf0buddy.h'
--- Percona-Server/storage/innobase/include/buf0buddy.h	2013-08-06 15:16:34 +0000
+++ Percona-Server/storage/innobase/include/buf0buddy.h	2013-09-20 05:29:11 +0000
@@ -36,8 +36,8 @@
 
 /**********************************************************************//**
 Allocate a block.  The thread calling this function must hold
-buf_pool->mutex and must not hold buf_pool->zip_mutex or any
-block->mutex.  The buf_pool->mutex may be released and reacquired.
+buf_pool->LRU_list_mutex and must not hold buf_pool->zip_mutex or any
+block->mutex.  The buf_pool->LRU_list_mutex may be released and reacquired.
 This function should only be used for allocating compressed page frames.
 @return	allocated block, never NULL */
 UNIV_INLINE
@@ -52,8 +52,8 @@
 	ibool*		lru)		/*!< in: pointer to a variable
 					that will be assigned TRUE if
 				       	storage was allocated from the
-				       	LRU list and buf_pool->mutex was
-				       	temporarily released */
+					LRU list and buf_pool->LRU_list_mutex
+					was temporarily released */
 	__attribute__((malloc, nonnull));
 
 /**********************************************************************//**

=== modified file 'Percona-Server/storage/innobase/include/buf0buddy.ic'
--- Percona-Server/storage/innobase/include/buf0buddy.ic	2013-08-06 15:16:34 +0000
+++ Percona-Server/storage/innobase/include/buf0buddy.ic	2013-09-20 05:29:11 +0000
@@ -35,8 +35,8 @@
 
 /**********************************************************************//**
 Allocate a block.  The thread calling this function must hold
-buf_pool->mutex and must not hold buf_pool->zip_mutex or any block->mutex.
-The buf_pool_mutex may be released and reacquired.
+buf_pool->LRU_list_mutex and must not hold buf_pool->zip_mutex or any
+block->mutex.  The buf_pool->LRU_list_mutex may be released and reacquired.
 @return	allocated block, never NULL */
 UNIV_INTERN
 void*
@@ -48,8 +48,8 @@
 	ibool*		lru)		/*!< in: pointer to a variable that
 					will be assigned TRUE if storage was
 					allocated from the LRU list and
-					buf_pool->mutex was temporarily
-					released */
+					buf_pool->LRU_list_mutex was
+					temporarily released */
 	__attribute__((malloc, nonnull));
 
 /**********************************************************************//**
@@ -88,8 +88,8 @@
 
 /**********************************************************************//**
 Allocate a block.  The thread calling this function must hold
-buf_pool->mutex and must not hold buf_pool->zip_mutex or any
-block->mutex.  The buf_pool->mutex may be released and reacquired.
+buf_pool->LRU_list_mutex and must not hold buf_pool->zip_mutex or any
+block->mutex.  The buf_pool->LRU_list_mutex may be released and reacquired.
 This function should only be used for allocating compressed page frames.
 @return	allocated block, never NULL */
 UNIV_INLINE
@@ -104,10 +104,10 @@
 	ibool*		lru)		/*!< in: pointer to a variable
 					that will be assigned TRUE if
 				       	storage was allocated from the
-				       	LRU list and buf_pool->mutex was
-				       	temporarily released */
+					LRU list and buf_pool->LRU_list_mutex
+					was temporarily released */
 {
-	ut_ad(buf_pool_mutex_own(buf_pool));
+	ut_ad(mutex_own(&buf_pool->LRU_list_mutex));
 	ut_ad(ut_is_2pow(size));
 	ut_ad(size >= UNIV_ZIP_SIZE_MIN);
 	ut_ad(size <= UNIV_PAGE_SIZE);
@@ -129,7 +129,6 @@
 	ulint		size)		/*!< in: block size,
 					up to UNIV_PAGE_SIZE */
 {
-	ut_ad(buf_pool_mutex_own(buf_pool));
 	ut_ad(ut_is_2pow(size));
 	ut_ad(size >= UNIV_ZIP_SIZE_MIN);
 	ut_ad(size <= UNIV_PAGE_SIZE);

=== modified file 'Percona-Server/storage/innobase/include/buf0buf.h'
--- Percona-Server/storage/innobase/include/buf0buf.h	2013-09-02 10:01:38 +0000
+++ Percona-Server/storage/innobase/include/buf0buf.h	2013-09-20 05:29:11 +0000
@@ -208,19 +208,6 @@
 };
 
 #ifndef UNIV_HOTBACKUP
-/********************************************************************//**
-Acquire mutex on all buffer pool instances */
-UNIV_INLINE
-void
-buf_pool_mutex_enter_all(void);
-/*===========================*/
-
-/********************************************************************//**
-Release mutex on all buffer pool instances */
-UNIV_INLINE
-void
-buf_pool_mutex_exit_all(void);
-/*==========================*/
 
 /********************************************************************//**
 Creates the buffer pool.
@@ -581,7 +568,7 @@
 					page frame */
 /********************************************************************//**
 Increments the modify clock of a frame by 1. The caller must (1) own the
-buf_pool->mutex and block bufferfix count has to be zero, (2) or own an x-lock
+LRU list mutex and block bufferfix count has to be zero, (2) or own an x-lock
 on the block. */
 UNIV_INLINE
 void
@@ -938,6 +925,17 @@
 	const buf_block_t*	block)	/*!< in: pointer to the control block */
 	__attribute__((pure));
 /*********************************************************************//**
+Gets the io_fix state of a block.  Does not assert that the
+buf_page_get_mutex() mutex is held, to be used in the cases where it is safe
+not to hold it.
+@return	io_fix state */
+UNIV_INLINE
+enum buf_io_fix
+buf_page_get_io_fix_unlocked(
+/*=========================*/
+	const buf_page_t*	bpage)	/*!< in: pointer to the control block */
+	__attribute__((pure));
+/*********************************************************************//**
 Sets the io_fix state of a block. */
 UNIV_INLINE
 void
@@ -955,7 +953,7 @@
 	enum buf_io_fix	io_fix);/*!< in: io_fix state */
 /*********************************************************************//**
 Makes a block sticky. A sticky block implies that even after we release
-the buf_pool->mutex and the block->mutex:
+the buf_pool->LRU_list_mutex and the block->mutex:
 * it cannot be removed from the flush_list
 * the block descriptor cannot be relocated
 * it cannot be removed from the LRU list
@@ -1410,6 +1408,19 @@
 
 #endif /* !UNIV_HOTBACKUP */
 
+#ifdef UNIV_DEBUG
+/********************************************************************//**
+Checks if buf_pool->zip_mutex is owned and is serving for a given page as its
+block mutex.
+@return true if buf_pool->zip_mutex is owned. */
+UNIV_INLINE
+bool
+buf_own_zip_mutex_for_page(
+/*=======================*/
+	const buf_page_t*	bpage)
+	__attribute__((nonnull,warn_unused_result));
+#endif /* UNIV_DEBUG */
+
 /** The common buffer control block structure
 for compressed and uncompressed frames */
 
@@ -1421,18 +1432,14 @@
 	None of these bit-fields must be modified without holding
 	buf_page_get_mutex() [buf_block_t::mutex or
 	buf_pool->zip_mutex], since they can be stored in the same
-	machine word.  Some of these fields are additionally protected
-	by buf_pool->mutex. */
+	machine word.  */
 	/* @{ */
 
-	unsigned	space:32;	/*!< tablespace id; also protected
-					by buf_pool->mutex. */
-	unsigned	offset:32;	/*!< page number; also protected
-					by buf_pool->mutex. */
+	unsigned	space:32;	/*!< tablespace id. */
+	unsigned	offset:32;	/*!< page number. */
 
 	unsigned	state:BUF_PAGE_STATE_BITS;
-					/*!< state of the control block; also
-					protected by buf_pool->mutex.
+					/*!< state of the control block.
 					State transitions from
 					BUF_BLOCK_READY_FOR_USE to
 					BUF_BLOCK_MEMORY need not be
@@ -1450,11 +1457,21 @@
 #ifndef UNIV_HOTBACKUP
 	unsigned	flush_type:2;	/*!< if this block is currently being
 					flushed to disk, this tells the
-					flush_type.
+					flush_type.  Writes during flushing
+					protected by buf_page_get_mutex_enter()
+					mutex and the corresponding flush state
+					mutex.
 					@see buf_flush_t */
-	unsigned	io_fix:2;	/*!< type of pending I/O operation;
-					also protected by buf_pool->mutex
-					@see enum buf_io_fix */
+	unsigned	io_fix:2;	/*!< type of pending I/O operation.
+					Transitions from BUF_IO_NONE to
+					BUF_IO_WRITE and back are protected by
+					the buf_page_get_mutex() mutex and the
+					corresponding flush state mutex.  The
+					flush state mutex protection for io_fix
+					and flush_type is not strictly
+					required, but it ensures consistent
+					buffer pool instance state snapshots in
+					buf_pool_validate_instance(). */
 	unsigned	buf_fix_count:19;/*!< count of how manyfold this block
 					is currently bufferfixed */
 	unsigned	buf_pool_index:6;/*!< index number of the buffer pool
@@ -1466,7 +1483,7 @@
 #endif /* !UNIV_HOTBACKUP */
 	page_zip_des_t	zip;		/*!< compressed page; zip.data
 					(but not the data it points to) is
-					also protected by buf_pool->mutex;
+					protected by buf_pool->zip_mutex;
 					state == BUF_BLOCK_ZIP_PAGE and
 					zip.data == NULL means an active
 					buf_pool->watch */
@@ -1479,15 +1496,13 @@
 	ibool		in_zip_hash;	/*!< TRUE if in buf_pool->zip_hash */
 #endif /* UNIV_DEBUG */
 
-	/** @name Page flushing fields
-	All these are protected by buf_pool->mutex. */
+	/** @name Page flushing fields */
 	/* @{ */
 
 	UT_LIST_NODE_T(buf_page_t) list;
 					/*!< based on state, this is a
 					list node, protected either by
-					buf_pool->mutex or by
-					buf_pool->flush_list_mutex,
+					a corresponding list mutex,
 					in one of the following lists in
 					buf_pool:
 
@@ -1500,7 +1515,8 @@
 					then the node pointers are
 					covered by buf_pool->flush_list_mutex.
 					Otherwise these pointers are
-					protected by buf_pool->mutex.
+					protected by a corresponding list
+					mutex.
 
 					The contents of the list node
 					is undefined if !in_flush_list
@@ -1523,8 +1539,8 @@
 					reads can happen while holding
 					any one of the two mutexes */
 	ibool		in_free_list;	/*!< TRUE if in buf_pool->free; when
-					buf_pool->mutex is free, the following
-					should hold: in_free_list
+					buf_pool->free_list_mutex is free, the
+					following should hold: in_free_list
 					== (state == BUF_BLOCK_NOT_USED) */
 #endif /* UNIV_DEBUG */
 	lsn_t		newest_modification;
@@ -1547,9 +1563,7 @@
 					reads can happen while holding
 					any one of the two mutexes */
 	/* @} */
-	/** @name LRU replacement algorithm fields
-	These fields are protected by buf_pool->mutex only (not
-	buf_pool->zip_mutex or buf_block_t::mutex). */
+	/** @name LRU replacement algorithm fields */
 	/* @{ */
 
 	UT_LIST_NODE_T(buf_page_t) LRU;
@@ -1560,7 +1574,10 @@
 					debugging */
 #endif /* UNIV_DEBUG */
 	unsigned	old:1;		/*!< TRUE if the block is in the old
-					blocks in buf_pool->LRU_old */
+					blocks in buf_pool->LRU_old.  Protected
+					by the LRU list mutex.  May be read for
+					heuristics purposes under the block
+					mutex instead. */
 	unsigned	freed_page_clock:31;/*!< the value of
 					buf_pool->freed_page_clock
 					when this block was the last
@@ -1612,8 +1629,7 @@
 					used in debugging */
 #endif /* UNIV_DEBUG */
 	ib_mutex_t		mutex;		/*!< mutex protecting this block:
-					state (also protected by the buffer
-					pool mutex), io_fix, buf_fix_count,
+					state, io_fix, buf_fix_count,
 					and accessed; we introduce this new
 					mutex in InnoDB-5.1 to relieve
 					contention on the buffer pool mutex */
@@ -1622,8 +1638,8 @@
 	unsigned	lock_hash_val:32;/*!< hashed value of the page address
 					in the record lock hash table;
 					protected by buf_block_t::lock
-					(or buf_block_t::mutex, buf_pool->mutex
-				        in buf_page_get_gen(),
+					(or buf_block_t::mutex in
+					buf_page_get_gen(),
 					buf_page_init_for_read()
 					and buf_page_create()) */
 	ibool		check_index_page_at_flush;
@@ -1646,8 +1662,8 @@
 					positioning: if the modify clock has
 					not changed, we know that the pointer
 					is still valid; this field may be
-					changed if the thread (1) owns the
-					pool mutex and the page is not
+					changed if the thread (1) owns the LRU
+					list mutex and the page is not
 					bufferfixed, or (2) the thread has an
 					x-latch on the block */
 	/* @} */
@@ -1754,11 +1770,11 @@
 	ulint	n_page_gets;	/*!< number of page gets performed;
 				also successful searches through
 				the adaptive hash index are
-				counted as page gets; this field
-				is NOT protected by the buffer
-				pool mutex */
-	ulint	n_pages_read;	/*!< number read operations */
-	ulint	n_pages_written;/*!< number write operations */
+				counted as page gets. */
+	ulint	n_pages_read;	/*!< number read operations.  Accessed
+				atomically. */
+	ulint	n_pages_written;/*!< number write operations.  Accessed
+				atomically.*/
 	ulint	n_pages_created;/*!< number of pages created
 				in the pool with no read */
 	ulint	n_ra_pages_read_rnd;/*!< number of pages read in
@@ -1798,12 +1814,16 @@
 
 	/** @name General fields */
 	/* @{ */
-	ib_mutex_t		mutex;		/*!< Buffer pool mutex of this
-					instance */
 	ib_mutex_t		zip_mutex;	/*!< Zip mutex of this buffer
 					pool instance, protects compressed
 					only pages (of type buf_page_t, not
 					buf_block_t */
+	ib_mutex_t	LRU_list_mutex;
+	ib_mutex_t	free_list_mutex;
+	ib_mutex_t	zip_free_mutex;
+	ib_mutex_t	zip_hash_mutex;
+	ib_mutex_t	flush_state_mutex;	/*!< Flush state protection
+					mutex */
 	ulint		instance_no;	/*!< Array index of this buffer
 					pool instance */
 	ulint		old_pool_size;  /*!< Old pool size in bytes */
@@ -1814,9 +1834,6 @@
 	ulint		buddy_n_frames; /*!< Number of frames allocated from
 					the buffer pool to the buddy system */
 #endif
-#if defined UNIV_DEBUG || defined UNIV_BUF_DEBUG
-	ulint		mutex_exit_forbidden; /*!< Forbid release mutex */
-#endif
 	ulint		n_chunks;	/*!< number of buffer pool chunks */
 	buf_chunk_t*	chunks;		/*!< buffer pool chunks */
 	ulint		curr_size;	/*!< current pool size in pages */
@@ -1828,26 +1845,23 @@
 					buf_page_in_file() == TRUE,
 					indexed by (space_id, offset).
 					page_hash is protected by an
-					array of mutexes.
-					Changes in page_hash are protected
-					by buf_pool->mutex and the relevant
-					page_hash mutex. Lookups can happen
-					while holding the buf_pool->mutex or
-					the relevant page_hash mutex. */
+					array of mutexes. */
 	hash_table_t*	zip_hash;	/*!< hash table of buf_block_t blocks
 					whose frames are allocated to the
 					zip buddy system,
 					indexed by block->frame */
 	ulint		n_pend_reads;	/*!< number of pending read
-					operations */
-	ulint		n_pend_unzip;	/*!< number of pending decompressions */
+					operations.  Accessed atomically */
+	ulint		n_pend_unzip;	/*!< number of pending decompressions.
+					Accesssed atomically */
 
 	time_t		last_printout_time;
 					/*!< when buf_print_io was last time
-					called */
+					called.  Accesses not protected */
 	buf_buddy_stat_t buddy_stat[BUF_BUDDY_SIZES_MAX + 1];
 					/*!< Statistics of buddy system,
-					indexed by block size */
+					indexed by block size.  Protected by
+					zip_free_mutex. */
 	buf_pool_stat_t	stat;		/*!< current statistics */
 	buf_pool_stat_t	old_stat;	/*!< old statistics */
 
@@ -1874,10 +1888,12 @@
 					list */
 	ibool		init_flush[BUF_FLUSH_N_TYPES];
 					/*!< this is TRUE when a flush of the
-					given type is being initialized */
+					given type is being initialized.
+					Protected by flush_state_mutex.  */
 	ulint		n_flush[BUF_FLUSH_N_TYPES];
 					/*!< this is the number of pending
-					writes in the given flush type */
+					writes in the given flush type.
+					Protected by flush_state_mutex.  */
 	os_event_t	no_flush[BUF_FLUSH_N_TYPES];
 					/*!< this is in the set state
 					when there is no flush batch
@@ -1904,7 +1920,8 @@
 					billion! A thread is allowed
 					to read this for heuristic
 					purposes without holding any
-					mutex or latch */
+					mutex or latch.  For non-heuristic
+					purposes protected by LRU_list_mutex */
 	ibool		try_LRU_scan;	/*!< Set to FALSE when an LRU
 					scan for free block fails. This
 					flag is used to avoid repeated
@@ -1913,8 +1930,7 @@
 					available in the scan depth for
 					eviction. Set to TRUE whenever
 					we flush a batch from the
-					buffer pool. Protected by the
-					buf_pool->mutex */
+					buffer pool. Accessed atomically. */
 	/* @} */
 
 	/** @name LRU replacement algorithm fields */
@@ -1942,7 +1958,8 @@
 
 	UT_LIST_BASE_NODE_T(buf_block_t) unzip_LRU;
 					/*!< base node of the
-					unzip_LRU list */
+					unzip_LRU list.  The list is protected
+					by LRU list mutex. */
 
 	/* @} */
 	/** @name Buddy allocator fields
@@ -1959,8 +1976,7 @@
 
 	buf_page_t*			watch;
 					/*!< Sentinel records for buffer
-					pool watches. Protected by
-					buf_pool->mutex. */
+					pool watches.  */
 
 #if BUF_BUDDY_LOW > UNIV_ZIP_SIZE_MIN
 # error "BUF_BUDDY_LOW > UNIV_ZIP_SIZE_MIN"
@@ -1968,18 +1984,10 @@
 	/* @} */
 };
 
-/** @name Accessors for buf_pool->mutex.
-Use these instead of accessing buf_pool->mutex directly. */
+/** @name Accessors for buffer pool mutexes
+Use these instead of accessing buffer pool mutexes directly. */
 /* @{ */
 
-/** Test if a buffer pool mutex is owned. */
-#define buf_pool_mutex_own(b) mutex_own(&b->mutex)
-/** Acquire a buffer pool mutex. */
-#define buf_pool_mutex_enter(b) do {		\
-	ut_ad(!mutex_own(&b->zip_mutex));	\
-	mutex_enter(&b->mutex);		\
-} while (0)
-
 /** Test if flush list mutex is owned. */
 #define buf_flush_list_mutex_own(b) mutex_own(&b->flush_list_mutex)
 
@@ -2035,31 +2043,6 @@
 # define buf_block_hash_lock_held_s_or_x(b, p)	(TRUE)
 #endif /* UNIV_SYNC_DEBUG */
 
-#if defined UNIV_DEBUG || defined UNIV_BUF_DEBUG
-/** Forbid the release of the buffer pool mutex. */
-# define buf_pool_mutex_exit_forbid(b) do {	\
-	ut_ad(buf_pool_mutex_own(b));		\
-	b->mutex_exit_forbidden++;		\
-} while (0)
-/** Allow the release of the buffer pool mutex. */
-# define buf_pool_mutex_exit_allow(b) do {	\
-	ut_ad(buf_pool_mutex_own(b));		\
-	ut_a(b->mutex_exit_forbidden);	\
-	b->mutex_exit_forbidden--;		\
-} while (0)
-/** Release the buffer pool mutex. */
-# define buf_pool_mutex_exit(b) do {		\
-	ut_a(!b->mutex_exit_forbidden);		\
-	mutex_exit(&b->mutex);			\
-} while (0)
-#else
-/** Forbid the release of the buffer pool mutex. */
-# define buf_pool_mutex_exit_forbid(b) ((void) 0)
-/** Allow the release of the buffer pool mutex. */
-# define buf_pool_mutex_exit_allow(b) ((void) 0)
-/** Release the buffer pool mutex. */
-# define buf_pool_mutex_exit(b) mutex_exit(&b->mutex)
-#endif
 #endif /* !UNIV_HOTBACKUP */
 /* @} */
 

=== modified file 'Percona-Server/storage/innobase/include/buf0buf.ic'
--- Percona-Server/storage/innobase/include/buf0buf.ic	2013-06-25 13:13:06 +0000
+++ Percona-Server/storage/innobase/include/buf0buf.ic	2013-09-20 05:29:11 +0000
@@ -121,7 +121,7 @@
 /*==========================*/
 	const buf_page_t*	bpage)	/*!< in: block */
 {
-	/* This is sometimes read without holding buf_pool->mutex. */
+	/* This is sometimes read without holding any buffer pool mutex. */
 	return(bpage->freed_page_clock);
 }
 
@@ -420,8 +420,21 @@
 /*================*/
 	const buf_page_t*	bpage)	/*!< in: pointer to the control block */
 {
-	ut_ad(bpage != NULL);
+	ut_ad(mutex_own(buf_page_get_mutex(bpage)));
+	return buf_page_get_io_fix_unlocked(bpage);
+}
 
+/*********************************************************************//**
+Gets the io_fix state of a block.  Does not assert that the
+buf_page_get_mutex() mutex is held, to be used in the cases where it is safe
+not to hold it.
+@return	io_fix state */
+UNIV_INLINE
+enum buf_io_fix
+buf_page_get_io_fix_unlocked(
+/*=========================*/
+	const buf_page_t*	bpage)	/*!< in: pointer to the control block */
+{
 	enum buf_io_fix	io_fix = (enum buf_io_fix) bpage->io_fix;
 #ifdef UNIV_DEBUG
 	switch (io_fix) {
@@ -449,6 +462,21 @@
 }
 
 /*********************************************************************//**
+Gets the io_fix state of a block.  Does not assert that the
+buf_page_get_mutex() mutex is held, to be used in the cases where it is safe
+not to hold it.
+@return	io_fix state */
+UNIV_INLINE
+enum buf_io_fix
+buf_block_get_io_fix_unlocked(
+/*==========================*/
+	const buf_block_t*	block)	/*!< in: pointer to the control block */
+{
+	return(buf_page_get_io_fix_unlocked(&block->page));
+}
+
+
+/*********************************************************************//**
 Sets the io_fix state of a block. */
 UNIV_INLINE
 void
@@ -457,10 +485,6 @@
 	buf_page_t*	bpage,	/*!< in/out: control block */
 	enum buf_io_fix	io_fix)	/*!< in: io_fix state */
 {
-#ifdef UNIV_DEBUG
-	buf_pool_t*	buf_pool = buf_pool_from_bpage(bpage);
-	ut_ad(buf_pool_mutex_own(buf_pool));
-#endif
 	ut_ad(mutex_own(buf_page_get_mutex(bpage)));
 
 	bpage->io_fix = io_fix;
@@ -481,7 +505,7 @@
 
 /*********************************************************************//**
 Makes a block sticky. A sticky block implies that even after we release
-the buf_pool->mutex and the block->mutex:
+the buf_pool->LRU_list_mutex and the block->mutex:
 * it cannot be removed from the flush_list
 * the block descriptor cannot be relocated
 * it cannot be removed from the LRU list
@@ -496,10 +520,11 @@
 {
 #ifdef UNIV_DEBUG
 	buf_pool_t*	buf_pool = buf_pool_from_bpage(bpage);
-	ut_ad(buf_pool_mutex_own(buf_pool));
+	ut_ad(mutex_own(&buf_pool->LRU_list_mutex));
 #endif
 	ut_ad(mutex_own(buf_page_get_mutex(bpage)));
 	ut_ad(buf_page_get_io_fix(bpage) == BUF_IO_NONE);
+	ut_ad(bpage->in_LRU_list);
 
 	bpage->io_fix = BUF_IO_PIN;
 }
@@ -512,10 +537,6 @@
 /*==================*/
 	buf_page_t*	bpage)	/*!< in/out: control block */
 {
-#ifdef UNIV_DEBUG
-	buf_pool_t*	buf_pool = buf_pool_from_bpage(bpage);
-	ut_ad(buf_pool_mutex_own(buf_pool));
-#endif
 	ut_ad(mutex_own(buf_page_get_mutex(bpage)));
 	ut_ad(buf_page_get_io_fix(bpage) == BUF_IO_PIN);
 
@@ -531,10 +552,6 @@
 /*==================*/
 	const buf_page_t*	bpage)	/*!< control block being relocated */
 {
-#ifdef UNIV_DEBUG
-	buf_pool_t*	buf_pool = buf_pool_from_bpage(bpage);
-	ut_ad(buf_pool_mutex_own(buf_pool));
-#endif
 	ut_ad(mutex_own(buf_page_get_mutex(bpage)));
 	ut_ad(buf_page_in_file(bpage));
 	ut_ad(bpage->in_LRU_list);
@@ -554,8 +571,12 @@
 {
 #ifdef UNIV_DEBUG
 	buf_pool_t*	buf_pool = buf_pool_from_bpage(bpage);
-	ut_ad(buf_pool_mutex_own(buf_pool));
 #endif
+	/* Buffer page mutex is not strictly required here for heuristic
+	purposes even if LRU mutex is not being held.  Keep the assertion
+	for now since all the callers hold it.  */
+	ut_ad(mutex_own(buf_page_get_mutex(bpage))
+	      || mutex_own(&buf_pool->LRU_list_mutex));
 	ut_ad(buf_page_in_file(bpage));
 
 	return(bpage->old);
@@ -574,7 +595,7 @@
 	buf_pool_t*	buf_pool = buf_pool_from_bpage(bpage);
 #endif /* UNIV_DEBUG */
 	ut_a(buf_page_in_file(bpage));
-	ut_ad(buf_pool_mutex_own(buf_pool));
+	ut_ad(mutex_own(&buf_pool->LRU_list_mutex));
 	ut_ad(bpage->in_LRU_list);
 
 #ifdef UNIV_LRU_DEBUG
@@ -619,11 +640,7 @@
 /*==================*/
 	buf_page_t*	bpage)		/*!< in/out: control block */
 {
-#ifdef UNIV_DEBUG
-	buf_pool_t*	buf_pool = buf_pool_from_bpage(bpage);
-	ut_ad(!buf_pool_mutex_own(buf_pool));
 	ut_ad(mutex_own(buf_page_get_mutex(bpage)));
-#endif
 	ut_a(buf_page_in_file(bpage));
 
 	if (!bpage->access_time) {
@@ -885,10 +902,6 @@
 /*===========*/
 	buf_block_t*	block)	/*!< in, own: block to be freed */
 {
-	buf_pool_t*	buf_pool = buf_pool_from_bpage((buf_page_t*) block);
-
-	buf_pool_mutex_enter(buf_pool);
-
 	mutex_enter(&block->mutex);
 
 	ut_a(buf_block_get_state(block) != BUF_BLOCK_FILE_PAGE);
@@ -896,8 +909,6 @@
 	buf_LRU_block_free_non_file_page(block);
 
 	mutex_exit(&block->mutex);
-
-	buf_pool_mutex_exit(buf_pool);
 }
 #endif /* !UNIV_HOTBACKUP */
 
@@ -962,7 +973,7 @@
 
 /********************************************************************//**
 Increments the modify clock of a frame by 1. The caller must (1) own the
-buf_pool mutex and block bufferfix count has to be zero, (2) or own an x-lock
+LRU list mutex and block bufferfix count has to be zero, (2) or own an x-lock
 on the block. */
 UNIV_INLINE
 void
@@ -973,7 +984,7 @@
 #ifdef UNIV_SYNC_DEBUG
 	buf_pool_t*	buf_pool = buf_pool_from_bpage((buf_page_t*) block);
 
-	ut_ad((buf_pool_mutex_own(buf_pool)
+	ut_ad((mutex_own(&buf_pool->LRU_list_mutex)
 	       && (block->page.buf_fix_count == 0))
 	      || rw_lock_own(&(block->lock), RW_LOCK_EXCLUSIVE));
 #endif /* UNIV_SYNC_DEBUG */
@@ -1371,39 +1382,6 @@
 	sync_thread_add_level(&block->lock, level, FALSE);
 }
 #endif /* UNIV_SYNC_DEBUG */
-/********************************************************************//**
-Acquire mutex on all buffer pool instances. */
-UNIV_INLINE
-void
-buf_pool_mutex_enter_all(void)
-/*==========================*/
-{
-	ulint   i;
-
-	for (i = 0; i < srv_buf_pool_instances; i++) {
-		buf_pool_t*	buf_pool;
-
-		buf_pool = buf_pool_from_array(i);
-		buf_pool_mutex_enter(buf_pool);
-	}
-}
-
-/********************************************************************//**
-Release mutex on all buffer pool instances. */
-UNIV_INLINE
-void
-buf_pool_mutex_exit_all(void)
-/*=========================*/
-{
-	ulint   i;
-
-	for (i = 0; i < srv_buf_pool_instances; i++) {
-		buf_pool_t*	buf_pool;
-
-		buf_pool = buf_pool_from_array(i);
-		buf_pool_mutex_exit(buf_pool);
-	}
-}
 /*********************************************************************//**
 Get the nth chunk's buffer block in the specified buffer pool.
 @return the nth chunk's buffer block. */
@@ -1421,4 +1399,26 @@
 	*chunk_size = chunk->size;
 	return(chunk->blocks);
 }
+
+#ifdef UNIV_DEBUG
+/********************************************************************//**
+Checks if buf_pool->zip_mutex is owned and is serving for a given page as its
+block mutex.
+@return true if buf_pool->zip_mutex is owned. */
+UNIV_INLINE
+bool
+buf_own_zip_mutex_for_page(
+/*=======================*/
+	const buf_page_t*	bpage)
+{
+	buf_pool_t*	buf_pool	= buf_pool_from_bpage(bpage);
+
+	ut_ad(buf_page_get_state(bpage) == BUF_BLOCK_ZIP_PAGE
+	      || buf_page_get_state(bpage) == BUF_BLOCK_ZIP_DIRTY);
+	ut_ad(buf_page_get_mutex(bpage) == &buf_pool->zip_mutex);
+
+	return(mutex_own(&buf_pool->zip_mutex));
+}
+#endif /* UNIV_DEBUG */
+
 #endif /* !UNIV_HOTBACKUP */

=== modified file 'Percona-Server/storage/innobase/include/buf0flu.h'
--- Percona-Server/storage/innobase/include/buf0flu.h	2013-08-16 09:11:51 +0000
+++ Percona-Server/storage/innobase/include/buf0flu.h	2013-09-20 05:29:11 +0000
@@ -37,7 +37,7 @@
 extern ibool buf_page_cleaner_is_active;
 
 /********************************************************************//**
-Remove a block from the flush list of modified blocks. */
+Remove a block from the flush list of modified blocks.  */
 UNIV_INTERN
 void
 buf_flush_remove(
@@ -75,9 +75,9 @@
 # if defined UNIV_DEBUG || defined UNIV_IBUF_DEBUG
 /********************************************************************//**
 Writes a flushable page asynchronously from the buffer pool to a file.
-NOTE: buf_pool->mutex and block->mutex must be held upon entering this
-function, and they will be released by this function after flushing.
-This is loosely based on buf_flush_batch() and buf_flush_page().
+NOTE: block->mutex must be held upon entering this function, and they will be
+released by this function after flushing.  This is loosely based on
+buf_flush_batch() and buf_flush_page().
 @return TRUE if the page was flushed and the mutexes released */
 UNIV_INTERN
 ibool
@@ -232,9 +232,8 @@
 Writes a flushable page asynchronously from the buffer pool to a file.
 NOTE: in simulated aio we must call
 os_aio_simulated_wake_handler_threads after we have posted a batch of
-writes! NOTE: buf_pool->mutex and buf_page_get_mutex(bpage) must be
-held upon entering this function, and they will be released by this
-function. */
+writes! NOTE: buf_page_get_mutex(bpage) must be held upon entering this
+function, and they will be released by this function. */
 UNIV_INTERN
 void
 buf_flush_page(

=== modified file 'Percona-Server/storage/innobase/include/buf0flu.ic'
--- Percona-Server/storage/innobase/include/buf0flu.ic	2013-08-06 15:16:34 +0000
+++ Percona-Server/storage/innobase/include/buf0flu.ic	2013-09-20 05:29:11 +0000
@@ -69,7 +69,6 @@
 	ut_ad(rw_lock_own(&(block->lock), RW_LOCK_EX));
 #endif /* UNIV_SYNC_DEBUG */
 
-	ut_ad(!buf_pool_mutex_own(buf_pool));
 	ut_ad(!buf_flush_list_mutex_own(buf_pool));
 	ut_ad(!mtr->made_dirty || log_flush_order_mutex_own());
 
@@ -116,7 +115,6 @@
 	ut_ad(rw_lock_own(&(block->lock), RW_LOCK_EX));
 #endif /* UNIV_SYNC_DEBUG */
 
-	ut_ad(!buf_pool_mutex_own(buf_pool));
 	ut_ad(!buf_flush_list_mutex_own(buf_pool));
 	ut_ad(log_flush_order_mutex_own());
 

=== modified file 'Percona-Server/storage/innobase/include/buf0lru.h'
--- Percona-Server/storage/innobase/include/buf0lru.h	2013-08-16 09:11:51 +0000
+++ Percona-Server/storage/innobase/include/buf0lru.h	2013-09-20 05:29:11 +0000
@@ -79,12 +79,14 @@
 Try to free a block.  If bpage is a descriptor of a compressed-only
 page, the descriptor object will be freed as well.
 
-NOTE: If this function returns true, it will temporarily
-release buf_pool->mutex.  Furthermore, the page frame will no longer be
-accessible via bpage.
-
-The caller must hold buf_pool->mutex and must not hold any
-buf_page_get_mutex() when calling this function.
+NOTE: If this function returns true, it will release the LRU list mutex,
+and temporarily release and relock the buf_page_get_mutex() mutex.
+Furthermore, the page frame will no longer be accessible via bpage.  If this
+function returns false, the buf_page_get_mutex() might be temporarily released
+and relocked too.
+
+The caller must hold the LRU list and buf_page_get_mutex() mutexes.
+
 @return true if freed, false otherwise. */
 UNIV_INTERN
 bool
@@ -291,7 +293,7 @@
 extern buf_LRU_stat_t	buf_LRU_stat_cur;
 
 /** Running sum of past values of buf_LRU_stat_cur.
-Updated by buf_LRU_stat_update().  Protected by buf_pool->mutex. */
+Updated by buf_LRU_stat_update().  */
 extern buf_LRU_stat_t	buf_LRU_stat_sum;
 
 /********************************************************************//**

=== modified file 'Percona-Server/storage/innobase/include/sync0sync.h'
--- Percona-Server/storage/innobase/include/sync0sync.h	2013-08-06 15:16:34 +0000
+++ Percona-Server/storage/innobase/include/sync0sync.h	2013-09-20 05:29:11 +0000
@@ -71,6 +71,11 @@
 extern mysql_pfs_key_t	buffer_block_mutex_key;
 extern mysql_pfs_key_t	buf_pool_mutex_key;
 extern mysql_pfs_key_t	buf_pool_zip_mutex_key;
+extern mysql_pfs_key_t	buf_pool_LRU_list_mutex_key;
+extern mysql_pfs_key_t	buf_pool_free_list_mutex_key;
+extern mysql_pfs_key_t	buf_pool_zip_free_mutex_key;
+extern mysql_pfs_key_t	buf_pool_zip_hash_mutex_key;
+extern mysql_pfs_key_t	buf_pool_flush_state_mutex_key;
 extern mysql_pfs_key_t	cache_last_read_mutex_key;
 extern mysql_pfs_key_t	dict_foreign_err_mutex_key;
 extern mysql_pfs_key_t	dict_sys_mutex_key;
@@ -632,7 +637,7 @@
 Search system mutex
 |
 V
-Buffer pool mutex
+Buffer pool mutexes
 |
 V
 Log mutex
@@ -723,11 +728,15 @@
 					SYNC_SEARCH_SYS, as memory allocation
 					can call routines there! Otherwise
 					the level is SYNC_MEM_HASH. */
-#define	SYNC_BUF_POOL		150	/* Buffer pool mutex */
+#define	SYNC_BUF_LRU_LIST	151
 #define	SYNC_BUF_PAGE_HASH	149	/* buf_pool->page_hash rw_lock */
 #define	SYNC_BUF_BLOCK		146	/* Block mutex */
-#define	SYNC_BUF_FLUSH_LIST	145	/* Buffer flush list mutex */
-#define SYNC_DOUBLEWRITE	140
+#define	SYNC_BUF_FREE_LIST	145
+#define	SYNC_BUF_ZIP_FREE	144
+#define	SYNC_BUF_ZIP_HASH	143
+#define	SYNC_BUF_FLUSH_STATE	142
+#define	SYNC_BUF_FLUSH_LIST	141	/* Buffer flush list mutex */
+#define	SYNC_DOUBLEWRITE	139
 #define	SYNC_ANY_LATCH		135
 #define	SYNC_MEM_HASH		131
 #define	SYNC_MEM_POOL		130

=== modified file 'Percona-Server/storage/innobase/sync/sync0sync.cc'
--- Percona-Server/storage/innobase/sync/sync0sync.cc	2013-09-02 10:01:38 +0000
+++ Percona-Server/storage/innobase/sync/sync0sync.cc	2013-09-20 05:29:11 +0000
@@ -1201,7 +1201,11 @@
 		/* fallthrough */
 	}
 	case SYNC_BUF_FLUSH_LIST:
-	case SYNC_BUF_POOL:
+	case SYNC_BUF_LRU_LIST:
+	case SYNC_BUF_FREE_LIST:
+	case SYNC_BUF_ZIP_FREE:
+	case SYNC_BUF_ZIP_HASH:
+	case SYNC_BUF_FLUSH_STATE:
 		/* We can have multiple mutexes of this type therefore we
 		can only check whether the greater than condition holds. */
 		if (!sync_thread_levels_g(array, level-1, TRUE)) {
@@ -1215,17 +1219,12 @@
 
 	case SYNC_BUF_PAGE_HASH:
 		/* Multiple page_hash locks are only allowed during
-		buf_validate and that is where buf_pool mutex is already
-		held. */
+		buf_validate. */
 		/* Fall through */
 
 	case SYNC_BUF_BLOCK:
-		/* Either the thread must own the buffer pool mutex
-		(buf_pool->mutex), or it is allowed to latch only ONE
-		buffer block (block->mutex or buf_pool->zip_mutex). */
 		if (!sync_thread_levels_g(array, level, FALSE)) {
 			ut_a(sync_thread_levels_g(array, level - 1, TRUE));
-			ut_a(sync_thread_levels_contain(array, SYNC_BUF_POOL));
 		}
 		break;
 	case SYNC_REC_LOCK: