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);
>                 block = buf_LRU_get_free_block(buf_pool);
>                 ut_a(block);


Reverted too.


>   - 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?


I don't see buf_page_get_mutex() being released directly in
buf_mark_space_corrupt()?   buf_LRU_free_one_page() will release hash
lock and buf_page_get_mutex(), thus no bug.  Should
buf_LRU_free_one_page() locking be documented better in its header
comment?


>   - 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?


It's pushed down to buf_flush_write_complete().  The reason is that
it must happen under flush_state_mutex in order to observe consistent
buffer pool in buf_pool_validate_instance().


>   - more spurious changes:

> -               UT_LIST_INSERT_AFTER(list, buf_pool->flush_list,
> -                                    prev_b, &block->page);
> +               UT_LIST_INSERT_AFTER(list, buf_pool->flush_list, prev_b,
> +                                    &block->page);
> -Remove a block from the flush list of modified blocks. */
> +Remove a block from the flush list of modified blocks.  */
> -       ulint           count   = 0;
> +       ulint   count   = 0;
> -       os_event_wait(buf_pool->no_flush[type]);
> +               os_event_wait(buf_pool->no_flush[type]);
> -                  || buf_pool->init_flush[BUF_FLUSH_LRU]) {
> +                   || buf_pool->init_flush[BUF_FLUSH_LRU]) {
> -       ibool           freed;
> +       ibool           freed;


All reverted.


>   - in the following hunk there’s no really need for the “else” block,
>     and a call to mutex_exit() can be moved out of the “if” statement:
> 
> @@ -1316,14 +1317,14 @@ buf_flush_try_neighbors(


Yes, fixed.


>   - typo in the comments:
> 
> +       /* The following call will release the buf_pgae_get_mutex() mutex. */
> ...
> +       As we are not holding LRU list or buf_pgae_get_mutex() mutex therefore


Oh. Fixed.