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

Proposed by Laurynas Biveinis
Status: Merged
Approved by: Laurynas Biveinis
Approved revision: no longer in the source branch.
Merged at revision: 734
Proposed branch: lp:~laurynas-biveinis/percona-server/bug1417953
Merge into: lp:percona-server/5.6
Diff against target: 191 lines (+14/-25)
8 files modified
storage/innobase/buf/buf0buf.cc (+2/-10)
storage/innobase/buf/buf0flu.cc (+4/-2)
storage/innobase/buf/buf0lru.cc (+1/-1)
storage/innobase/buf/buf0rea.cc (+5/-3)
storage/innobase/handler/ha_innodb.cc (+0/-1)
storage/innobase/handler/i_s.cc (+0/-5)
storage/innobase/include/buf0flu.h (+2/-2)
storage/innobase/include/sync0sync.h (+0/-1)
To merge this branch: bzr merge lp:~laurynas-biveinis/percona-server/bug1417953
Reviewer Review Type Date Requested Status
Laurynas Biveinis (community) Approve
Review via email: mp+248530@code.launchpad.net

Description of the change

Fix bug 1417953 (buf_read_ahead_linear dereferences buffer page
pointer without protection).

Fix by protecting the bpage dereference with page hash S latch.

At the same time fix some other issues discovered while porting to
5.7:
- remove unused buf_pool_mutex_key PFS instrumentation variable;
- remove zip mutex locking from buf_pool_watch_set and
  buf_pool_watch_remove, as the watch pages are already protected by
  page hash X latch;
- fix some comments;
- fix some bool variables getting assigned TRUE instead of true.

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

To post a comment you must log in.
Revision history for this message
Laurynas Biveinis (laurynas-biveinis) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'storage/innobase/buf/buf0buf.cc'
--- storage/innobase/buf/buf0buf.cc 2014-12-30 14:14:11 +0000
+++ storage/innobase/buf/buf0buf.cc 2015-02-04 11:23:44 +0000
@@ -294,7 +294,6 @@
294294
295#ifdef UNIV_PFS_MUTEX295#ifdef UNIV_PFS_MUTEX
296UNIV_INTERN mysql_pfs_key_t buffer_block_mutex_key;296UNIV_INTERN mysql_pfs_key_t buffer_block_mutex_key;
297UNIV_INTERN mysql_pfs_key_t buf_pool_mutex_key;
298UNIV_INTERN mysql_pfs_key_t buf_pool_zip_mutex_key;297UNIV_INTERN mysql_pfs_key_t buf_pool_zip_mutex_key;
299UNIV_INTERN mysql_pfs_key_t buf_pool_flush_state_mutex_key;298UNIV_INTERN mysql_pfs_key_t buf_pool_flush_state_mutex_key;
300UNIV_INTERN mysql_pfs_key_t buf_pool_LRU_list_mutex_key;299UNIV_INTERN mysql_pfs_key_t buf_pool_LRU_list_mutex_key;
@@ -1735,16 +1734,12 @@
1735 ut_ad(!bpage->in_page_hash);1734 ut_ad(!bpage->in_page_hash);
1736 ut_ad(bpage->buf_fix_count == 0);1735 ut_ad(bpage->buf_fix_count == 0);
17371736
1738 mutex_enter(&buf_pool->zip_mutex);
1739
1740 bpage->state = BUF_BLOCK_ZIP_PAGE;1737 bpage->state = BUF_BLOCK_ZIP_PAGE;
1741 bpage->space = static_cast<ib_uint32_t>(space);1738 bpage->space = static_cast<ib_uint32_t>(space);
1742 bpage->offset = static_cast<ib_uint32_t>(offset);1739 bpage->offset = static_cast<ib_uint32_t>(offset);
1743 bpage->buf_fix_count = 1;1740 bpage->buf_fix_count = 1;
1744 bpage->buf_pool_index = buf_pool_index(buf_pool);1741 bpage->buf_pool_index = buf_pool_index(buf_pool);
17451742
1746 mutex_exit(&buf_pool->zip_mutex);
1747
1748 ut_d(bpage->in_page_hash = TRUE);1743 ut_d(bpage->in_page_hash = TRUE);
1749 HASH_INSERT(buf_page_t, hash, buf_pool->page_hash,1744 HASH_INSERT(buf_page_t, hash, buf_pool->page_hash,
1750 fold, bpage);1745 fold, bpage);
@@ -1796,7 +1791,6 @@
1796#endif /* UNIV_SYNC_DEBUG */1791#endif /* UNIV_SYNC_DEBUG */
17971792
1798 ut_ad(buf_page_get_state(watch) == BUF_BLOCK_ZIP_PAGE);1793 ut_ad(buf_page_get_state(watch) == BUF_BLOCK_ZIP_PAGE);
1799 ut_ad(buf_own_zip_mutex_for_page(watch));
18001794
1801 HASH_DELETE(buf_page_t, hash, buf_pool->page_hash, fold, watch);1795 HASH_DELETE(buf_page_t, hash, buf_pool->page_hash, fold, watch);
1802 ut_d(watch->in_page_hash = FALSE);1796 ut_d(watch->in_page_hash = FALSE);
@@ -1839,9 +1833,7 @@
1839#endif /* PAGE_ATOMIC_REF_COUNT */1833#endif /* PAGE_ATOMIC_REF_COUNT */
18401834
1841 if (bpage->buf_fix_count == 0) {1835 if (bpage->buf_fix_count == 0) {
1842 mutex_enter(&buf_pool->zip_mutex);
1843 buf_pool_watch_remove(buf_pool, fold, bpage);1836 buf_pool_watch_remove(buf_pool, fold, bpage);
1844 mutex_exit(&buf_pool->zip_mutex);
1845 }1837 }
1846 }1838 }
18471839
@@ -4421,7 +4413,7 @@
4421#endif /* UNIV_DEBUG || UNIV_BUF_DEBUG */4413#endif /* UNIV_DEBUG || UNIV_BUF_DEBUG */
4422 buf_page_get_flush_type(bpage) == BUF_FLUSH_LRU)) {4414 buf_page_get_flush_type(bpage) == BUF_FLUSH_LRU)) {
44234415
4424 have_LRU_mutex = TRUE; /* optimistic */4416 have_LRU_mutex = true; /* optimistic */
4425 }4417 }
4426retry_mutex:4418retry_mutex:
4427 if (have_LRU_mutex) {4419 if (have_LRU_mutex) {
@@ -4441,7 +4433,7 @@
4441 && !have_LRU_mutex)) {4433 && !have_LRU_mutex)) {
44424434
4443 mutex_exit(block_mutex);4435 mutex_exit(block_mutex);
4444 have_LRU_mutex = TRUE;4436 have_LRU_mutex = true;
4445 goto retry_mutex;4437 goto retry_mutex;
4446 }4438 }
44474439
44484440
=== modified file 'storage/innobase/buf/buf0flu.cc'
--- storage/innobase/buf/buf0flu.cc 2014-12-16 06:00:17 +0000
+++ storage/innobase/buf/buf0flu.cc 2015-02-04 11:23:44 +0000
@@ -1098,8 +1098,8 @@
1098# if defined UNIV_DEBUG || defined UNIV_IBUF_DEBUG1098# if defined UNIV_DEBUG || defined UNIV_IBUF_DEBUG
1099/********************************************************************//**1099/********************************************************************//**
1100Writes a flushable page asynchronously from the buffer pool to a file.1100Writes a flushable page asynchronously from the buffer pool to a file.
1101NOTE: block->mutex must be held upon entering this function, and it will be1101NOTE: block and LRU list mutexes must be held upon entering this function, and
1102released by this function after flushing. This is loosely based on1102they will be released by this function after flushing. This is loosely based on
1103buf_flush_batch() and buf_flush_page().1103buf_flush_batch() and buf_flush_page().
1104@return TRUE if the page was flushed and the mutexes released */1104@return TRUE if the page was flushed and the mutexes released */
1105UNIV_INTERN1105UNIV_INTERN
@@ -1653,6 +1653,8 @@
1653 flush_counters_t* n) /*!< out: flushed/evicted page1653 flush_counters_t* n) /*!< out: flushed/evicted page
1654 counts */1654 counts */
1655{1655{
1656 ut_ad(mutex_own(&buf_pool->LRU_list_mutex));
1657
1656 if (buf_LRU_evict_from_unzip_LRU(buf_pool)) {1658 if (buf_LRU_evict_from_unzip_LRU(buf_pool)) {
1657 n->unzip_LRU_evicted1659 n->unzip_LRU_evicted
1658 = buf_free_from_unzip_LRU_list_batch(buf_pool, max);1660 = buf_free_from_unzip_LRU_list_batch(buf_pool, max);
16591661
=== modified file 'storage/innobase/buf/buf0lru.cc'
--- storage/innobase/buf/buf0lru.cc 2014-12-01 07:53:48 +0000
+++ storage/innobase/buf/buf0lru.cc 2015-02-04 11:23:44 +0000
@@ -526,7 +526,7 @@
526526
527 mutex_exit(block_mutex);527 mutex_exit(block_mutex);
528528
529 *must_restart = TRUE;529 *must_restart = true;
530 processed = false;530 processed = false;
531531
532 } else if (!flush) {532 } else if (!flush) {
533533
=== modified file 'storage/innobase/buf/buf0rea.cc'
--- storage/innobase/buf/buf0rea.cc 2014-12-01 07:53:48 +0000
+++ storage/innobase/buf/buf0rea.cc 2015-02-04 11:23:44 +0000
@@ -640,10 +640,10 @@
640640
641 fail_count = 0;641 fail_count = 0;
642642
643 prio_rw_lock_t* hash_lock;
644
643 for (i = low; i < high; i++) {645 for (i = low; i < high; i++) {
644646
645 prio_rw_lock_t* hash_lock;
646
647 bpage = buf_page_hash_get_s_locked(buf_pool, space, i,647 bpage = buf_page_hash_get_s_locked(buf_pool, space, i,
648 &hash_lock);648 &hash_lock);
649649
@@ -691,7 +691,7 @@
691 /* If we got this far, we know that enough pages in the area have691 /* If we got this far, we know that enough pages in the area have
692 been accessed in the right order: linear read-ahead can be sensible */692 been accessed in the right order: linear read-ahead can be sensible */
693693
694 bpage = buf_page_hash_get(buf_pool, space, offset);694 bpage = buf_page_hash_get_s_locked(buf_pool, space, offset, &hash_lock);
695695
696 if (bpage == NULL) {696 if (bpage == NULL) {
697697
@@ -719,6 +719,8 @@
719 pred_offset = fil_page_get_prev(frame);719 pred_offset = fil_page_get_prev(frame);
720 succ_offset = fil_page_get_next(frame);720 succ_offset = fil_page_get_next(frame);
721721
722 rw_lock_s_unlock(hash_lock);
723
722 if ((offset == low) && (succ_offset == offset + 1)) {724 if ((offset == low) && (succ_offset == offset + 1)) {
723725
724 /* This is ok, we can continue */726 /* This is ok, we can continue */
725727
=== modified file 'storage/innobase/handler/ha_innodb.cc'
--- storage/innobase/handler/ha_innodb.cc 2014-12-16 06:00:17 +0000
+++ storage/innobase/handler/ha_innodb.cc 2015-02-04 11:23:44 +0000
@@ -340,7 +340,6 @@
340# ifndef PFS_SKIP_BUFFER_MUTEX_RWLOCK340# ifndef PFS_SKIP_BUFFER_MUTEX_RWLOCK
341 {&buffer_block_mutex_key, "buffer_block_mutex", 0},341 {&buffer_block_mutex_key, "buffer_block_mutex", 0},
342# endif /* !PFS_SKIP_BUFFER_MUTEX_RWLOCK */342# endif /* !PFS_SKIP_BUFFER_MUTEX_RWLOCK */
343 {&buf_pool_mutex_key, "buf_pool_mutex", 0},
344 {&buf_pool_zip_mutex_key, "buf_pool_zip_mutex", 0},343 {&buf_pool_zip_mutex_key, "buf_pool_zip_mutex", 0},
345 {&buf_pool_LRU_list_mutex_key, "buf_pool_LRU_list_mutex", 0},344 {&buf_pool_LRU_list_mutex_key, "buf_pool_LRU_list_mutex", 0},
346 {&buf_pool_free_list_mutex_key, "buf_pool_free_list_mutex", 0},345 {&buf_pool_free_list_mutex_key, "buf_pool_free_list_mutex", 0},
347346
=== modified file 'storage/innobase/handler/i_s.cc'
--- storage/innobase/handler/i_s.cc 2014-12-01 07:53:48 +0000
+++ storage/innobase/handler/i_s.cc 2015-02-04 11:23:44 +0000
@@ -5279,11 +5279,6 @@
5279 info_buffer = (buf_page_info_t*) mem_heap_zalloc(5279 info_buffer = (buf_page_info_t*) mem_heap_zalloc(
5280 heap, mem_size);5280 heap, mem_size);
52815281
5282 /* Obtain appropriate mutexes. Since this is diagnostic
5283 buffer pool info printout, we are not required to
5284 preserve the overall consistency, so we can
5285 release mutex periodically */
5286
5287 /* GO through each block in the chunk */5282 /* GO through each block in the chunk */
5288 for (n_blocks = num_to_process; n_blocks--; block++) {5283 for (n_blocks = num_to_process; n_blocks--; block++) {
5289 i_s_innodb_buffer_page_get_info(5284 i_s_innodb_buffer_page_get_info(
52905285
=== modified file 'storage/innobase/include/buf0flu.h'
--- storage/innobase/include/buf0flu.h 2014-12-01 07:53:48 +0000
+++ storage/innobase/include/buf0flu.h 2015-02-04 11:23:44 +0000
@@ -78,8 +78,8 @@
78# if defined UNIV_DEBUG || defined UNIV_IBUF_DEBUG78# if defined UNIV_DEBUG || defined UNIV_IBUF_DEBUG
79/********************************************************************//**79/********************************************************************//**
80Writes a flushable page asynchronously from the buffer pool to a file.80Writes a flushable page asynchronously from the buffer pool to a file.
81NOTE: block->mutex must be held upon entering this function, and they will be81NOTE: block and LRU list mutexes must be held upon entering this function, and
82released by this function after flushing. This is loosely based on82they will be released by this function after flushing. This is loosely based on
83buf_flush_batch() and buf_flush_page().83buf_flush_batch() and buf_flush_page().
84@return TRUE if the page was flushed and the mutexes released */84@return TRUE if the page was flushed and the mutexes released */
85UNIV_INTERN85UNIV_INTERN
8686
=== modified file 'storage/innobase/include/sync0sync.h'
--- storage/innobase/include/sync0sync.h 2014-12-01 07:53:48 +0000
+++ storage/innobase/include/sync0sync.h 2015-02-04 11:23:44 +0000
@@ -71,7 +71,6 @@
71/* Key defines to register InnoDB mutexes with performance schema */71/* Key defines to register InnoDB mutexes with performance schema */
72extern mysql_pfs_key_t autoinc_mutex_key;72extern mysql_pfs_key_t autoinc_mutex_key;
73extern mysql_pfs_key_t buffer_block_mutex_key;73extern mysql_pfs_key_t buffer_block_mutex_key;
74extern mysql_pfs_key_t buf_pool_mutex_key;
75extern mysql_pfs_key_t buf_pool_zip_mutex_key;74extern mysql_pfs_key_t buf_pool_zip_mutex_key;
76extern mysql_pfs_key_t buf_pool_LRU_list_mutex_key;75extern mysql_pfs_key_t buf_pool_LRU_list_mutex_key;
77extern mysql_pfs_key_t buf_pool_free_list_mutex_key;76extern mysql_pfs_key_t buf_pool_free_list_mutex_key;

Subscribers

People subscribed via source and target branches