Merge lp:~laurynas-biveinis/percona-server/bug1122462 into lp:percona-server/5.5

Proposed by Laurynas Biveinis on 2013-02-12
Status: Work in progress
Proposed branch: lp:~laurynas-biveinis/percona-server/bug1122462
Merge into: lp:percona-server/5.5
Prerequisite: lp:~laurynas-biveinis/percona-server/bug1086680-5.5
Diff against target: 107 lines (+20/-13)
1 file modified
Percona-Server/storage/innobase/buf/buf0lru.c (+20/-13)
To merge this branch: bzr merge lp:~laurynas-biveinis/percona-server/bug1122462
Reviewer Review Type Date Requested Status
Laurynas Biveinis (community) Needs Fixing on 2013-05-16
George Ormond Lorch III g2 2013-02-12 Approve on 2013-02-25
Review via email: mp+147863@code.launchpad.net

Description of the change

Fix bug 1122462.
http://jenkins.percona.com/job/percona-server-5.5-param/667/

Fix bug 1122462 (InnoDB: Failing assertion: bpage->in_flush_list in
file buf0lru.c line 605).

The issue is that, even after fixing bug 934377 and friends, there is
not enough protection for the "prev" pointer in the
buf_flush_or_remove_pages() loop to start pointing to a flushed page
or become dangling due to a compressed page flush list relocation.

The fix is two part. First is acquiring the compressed page flush
mutex in buf_flush_dirty_pages() (and releasing it in
buf_flush_yield()). This ensures that the prev pointer always points
to a valid page.

The second part is to fix that, while prev cannot become dangling
anymore, it still may get removed from the flush list when the flush
list mutex is released. This is handled by replacing the assertions
at the top of the buf_flush_or_remove_pages() loop with a block state
check.

Remove some comments that do not apply for XtraDB.

To post a comment you must log in.
review: Approve (g2)

I will redo this after the new bug 1086680 fix.

review: Needs Fixing

Unmerged revisions

441. By Laurynas Biveinis on 2013-02-12

Fix bug 1122462 (InnoDB: Failing assertion: bpage->in_flush_list in
file buf0lru.c line 605).

The issue is that, even after fixing bug 934377 and friends, there is
not enough protection for the "prev" pointer in the
buf_flush_or_remove_pages() loop to start pointing to a flushed page
or become dangling due to a compressed page flush list relocation.

The fix is two part. First is acquiring the compressed page flush
mutex in buf_flush_dirty_pages() (and releasing it in
buf_flush_yield()). This ensures that the prev pointer always points
to a valid page.

The second part is to fix that, while prev cannot become dangling
anymore, it still may get removed from the flush list when the flush
list mutex is released. This is handled by replacing the assertions
at the top of the buf_flush_or_remove_pages() loop with a block state
check.

Remove some comments that do not apply for XtraDB.

440. By Laurynas Biveinis on 2013-02-12

Automerge prerequisite branch

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'Percona-Server/storage/innobase/buf/buf0lru.c'
2--- Percona-Server/storage/innobase/buf/buf0lru.c 2013-02-06 11:01:22 +0000
3+++ Percona-Server/storage/innobase/buf/buf0lru.c 2013-02-12 09:23:34 +0000
4@@ -397,6 +397,7 @@
5
6 ut_ad(mutex_own(block_mutex));
7 ut_ad(mutex_own(&buf_pool->LRU_list_mutex));
8+ ut_ad(mutex_own(&buf_pool->zip_dirty_flush_mutex));
9 ut_ad(buf_page_in_file(bpage));
10
11 /* "Fix" the block so that the position cannot be
12@@ -406,11 +407,13 @@
13
14 /* Now it is safe to release the LRU list mutex. */
15 mutex_exit(&buf_pool->LRU_list_mutex);
16+ mutex_exit(&buf_pool->zip_dirty_flush_mutex);
17
18 mutex_exit(block_mutex);
19 /* Try and force a context switch. */
20 os_thread_yield();
21
22+ mutex_enter(&buf_pool->zip_dirty_flush_mutex);
23 mutex_enter(&buf_pool->LRU_list_mutex);
24
25 mutex_enter(block_mutex);
26@@ -435,6 +438,8 @@
27 ibool* must_restart) /*!< in/out: if TRUE, we have to
28 restart the flush list scan */
29 {
30+ ut_ad(mutex_own(&buf_pool->zip_dirty_flush_mutex));
31+
32 /* Every BUF_LRU_DROP_SEARCH_SIZE iterations in the
33 loop we release buf_pool->mutex to let other threads
34 do their job but only if the block is not IO fixed. This
35@@ -449,11 +454,6 @@
36
37 buf_flush_list_mutex_exit(buf_pool);
38
39- /* We don't have to worry about bpage becoming a dangling
40- pointer by a compressed page flush list relocation because
41- buf_page_get_gen() won't be called for pages from this
42- tablespace. */
43-
44 block_mutex = buf_page_get_mutex_enter(bpage);
45 if (UNIV_UNLIKELY(block_mutex == NULL)) {
46
47@@ -517,10 +517,6 @@
48
49 block_mutex = buf_page_get_mutex(bpage);
50
51- /* bpage->space and bpage->io_fix are protected by
52- buf_pool->mutex and block_mutex. It is safe to check
53- them while holding buf_pool->mutex only. */
54-
55 if (UNIV_UNLIKELY(buf_page_get_io_fix_unlocked(bpage)
56 != BUF_IO_NONE)) {
57
58@@ -543,9 +539,10 @@
59
60 mutex_enter(block_mutex);
61
62- /* Recheck the page I/O fix and the flush list presence now
63- thatwe hold the right mutex. */
64- if (UNIV_UNLIKELY(buf_page_get_io_fix(bpage) != BUF_IO_NONE
65+ /* Recheck the page type, I/O fix and the flush list presence
66+ now that we hold the right mutex. */
67+ if (UNIV_UNLIKELY(!buf_page_in_file(bpage)
68+ || buf_page_get_io_fix(bpage) != BUF_IO_NONE
69 || bpage->oldest_modification == 0)) {
70
71 /* The page became I/O-fixed or is not on the flush
72@@ -595,13 +592,21 @@
73 ibool all_freed = TRUE;
74 ibool must_restart = FALSE;
75
76+ ut_ad(mutex_own(&buf_pool->zip_dirty_flush_mutex));
77+
78 buf_flush_list_mutex_enter(buf_pool);
79
80 for (bpage = UT_LIST_GET_LAST(buf_pool->flush_list);
81 !must_restart && bpage != NULL;
82 bpage = prev) {
83
84- ut_a(buf_page_in_file(bpage));
85+ if (!buf_page_in_file(bpage)
86+ || bpage->oldest_modification == 0) {
87+
88+ all_freed = FALSE;
89+ break;
90+ }
91+
92 ut_ad(bpage->in_flush_list);
93
94 /* Save the previous link because once we free the
95@@ -663,11 +668,13 @@
96 ibool all_freed;
97
98 do {
99+ mutex_enter(&buf_pool->zip_dirty_flush_mutex);
100 mutex_enter(&buf_pool->LRU_list_mutex);
101
102 all_freed = buf_flush_or_remove_pages(buf_pool, id);
103
104 mutex_exit(&buf_pool->LRU_list_mutex);
105+ mutex_exit(&buf_pool->zip_dirty_flush_mutex);
106
107 ut_ad(buf_flush_validate(buf_pool));
108

Subscribers

People subscribed via source and target branches