Merge lp:~laurynas-biveinis/percona-server/bpage-io-fix into lp:percona-server/5.5

Proposed by Laurynas Biveinis
Status: Merged
Approved by: Stewart Smith
Approved revision: 418
Merged at revision: 428
Proposed branch: lp:~laurynas-biveinis/percona-server/bpage-io-fix
Merge into: lp:percona-server/5.5
Diff against target: 434 lines (+153/-46)
7 files modified
Percona-Server/storage/innobase/buf/buf0buf.c (+1/-1)
Percona-Server/storage/innobase/buf/buf0flu.c (+2/-2)
Percona-Server/storage/innobase/buf/buf0lru.c (+97/-37)
Percona-Server/storage/innobase/fil/fil0fil.c (+1/-1)
Percona-Server/storage/innobase/ibuf/ibuf0ibuf.c (+1/-1)
Percona-Server/storage/innobase/include/buf0buf.h (+22/-2)
Percona-Server/storage/innobase/include/buf0buf.ic (+29/-2)
To merge this branch: bzr merge lp:~laurynas-biveinis/percona-server/bpage-io-fix
Reviewer Review Type Date Requested Status
Stewart Smith (community) Approve
Review via email: mp+146852@code.launchpad.net

Description of the change

To post a comment you must log in.
Revision history for this message
Stewart Smith (stewart) :
review: Approve

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/buf0buf.c'
2--- Percona-Server/storage/innobase/buf/buf0buf.c 2013-01-18 12:02:28 +0000
3+++ Percona-Server/storage/innobase/buf/buf0buf.c 2013-02-06 13:11:25 +0000
4@@ -3938,7 +3938,7 @@
5 ensures that this is the only thread that handles the i/o for this
6 block. */
7
8- io_type = buf_page_get_io_fix(bpage);
9+ io_type = buf_page_get_io_fix_unlocked(bpage);
10 ut_ad(io_type == BUF_IO_READ || io_type == BUF_IO_WRITE);
11
12 if (io_type == BUF_IO_READ) {
13
14=== modified file 'Percona-Server/storage/innobase/buf/buf0flu.c'
15--- Percona-Server/storage/innobase/buf/buf0flu.c 2013-01-16 16:12:15 +0000
16+++ Percona-Server/storage/innobase/buf/buf0flu.c 2013-02-06 13:11:25 +0000
17@@ -915,7 +915,7 @@
18 "InnoDB: Page buf fix count %lu,"
19 " io fix %lu, state %lu\n",
20 (ulong)block->page.buf_fix_count,
21- (ulong)buf_block_get_io_fix(block),
22+ (ulong)buf_block_get_io_fix_unlocked(block),
23 (ulong)buf_block_get_state(block));
24 }
25
26@@ -1115,7 +1115,7 @@
27 ut_ad(!mutex_own(&buf_pool->LRU_list_mutex));
28 ut_ad(!buf_flush_list_mutex_own(buf_pool));
29 ut_ad(!mutex_own(buf_page_get_mutex(bpage)));
30- ut_ad(buf_page_get_io_fix(bpage) == BUF_IO_WRITE);
31+ ut_ad(buf_page_get_io_fix_unlocked(bpage) == BUF_IO_WRITE);
32 ut_ad(bpage->oldest_modification != 0);
33
34 #ifdef UNIV_IBUF_COUNT_DEBUG
35
36=== modified file 'Percona-Server/storage/innobase/buf/buf0lru.c'
37--- Percona-Server/storage/innobase/buf/buf0lru.c 2013-01-16 13:31:48 +0000
38+++ Percona-Server/storage/innobase/buf/buf0lru.c 2013-02-06 13:11:25 +0000
39@@ -393,18 +393,18 @@
40 {
41 mutex_t* block_mutex;
42
43+ block_mutex = buf_page_get_mutex(bpage);
44+
45+ ut_ad(mutex_own(block_mutex));
46 ut_ad(mutex_own(&buf_pool->LRU_list_mutex));
47 ut_ad(buf_page_in_file(bpage));
48
49- block_mutex = buf_page_get_mutex(bpage);
50-
51- mutex_enter(block_mutex);
52 /* "Fix" the block so that the position cannot be
53 changed after we release the buffer pool and
54 block mutexes. */
55 buf_page_set_sticky(bpage);
56
57- /* Now it is safe to release the buf_pool->mutex. */
58+ /* Now it is safe to release the LRU list mutex. */
59 mutex_exit(&buf_pool->LRU_list_mutex);
60
61 mutex_exit(block_mutex);
62@@ -415,7 +415,7 @@
63
64 mutex_enter(block_mutex);
65 /* "Unfix" the block now that we have both the
66- buffer pool and block mutex again. */
67+ LRU list and block mutex again. */
68 buf_page_unset_sticky(bpage);
69 mutex_exit(block_mutex);
70 }
71@@ -431,7 +431,9 @@
72 /*================*/
73 buf_pool_t* buf_pool, /*!< in/out: buffer pool instance */
74 buf_page_t* bpage, /*!< in/out: bpage to remove */
75- ulint processed) /*!< in: number of pages processed */
76+ ulint processed, /*!< in: number of pages processed */
77+ ibool* must_restart) /*!< in/out: if TRUE, we have to
78+ restart the flush list scan */
79 {
80 /* Every BUF_LRU_DROP_SEARCH_SIZE iterations in the
81 loop we release buf_pool->mutex to let other threads
82@@ -441,10 +443,40 @@
83
84 if (bpage != NULL
85 && processed >= BUF_LRU_DROP_SEARCH_SIZE
86- && buf_page_get_io_fix(bpage) == BUF_IO_NONE) {
87+ && buf_page_get_io_fix_unlocked(bpage) == BUF_IO_NONE) {
88+
89+ mutex_t* block_mutex;
90
91 buf_flush_list_mutex_exit(buf_pool);
92
93+ /* We don't have to worry about bpage becoming a dangling
94+ pointer by a compressed page flush list relocation because
95+ buf_page_get_gen() won't be called for pages from this
96+ tablespace. */
97+
98+ block_mutex = buf_page_get_mutex_enter(bpage);
99+ if (UNIV_UNLIKELY(block_mutex == NULL)) {
100+
101+ buf_flush_list_mutex_enter(buf_pool);
102+
103+ *must_restart = TRUE;
104+ return FALSE;
105+ }
106+
107+ /* Recheck the I/O fix and the flush list presence now that we
108+ hold the right mutex */
109+ if (UNIV_UNLIKELY(buf_page_get_io_fix(bpage) != BUF_IO_NONE
110+ || bpage->oldest_modification == 0)) {
111+
112+ mutex_exit(block_mutex);
113+ buf_flush_list_mutex_enter(buf_pool);
114+
115+ *must_restart = TRUE;
116+ return FALSE;
117+ }
118+
119+ *must_restart = FALSE;
120+
121 /* Release the LRU list and block mutex
122 to give the other threads a go. */
123
124@@ -473,7 +505,9 @@
125 buf_flush_or_remove_page(
126 /*=====================*/
127 buf_pool_t* buf_pool, /*!< in/out: buffer pool instance */
128- buf_page_t* bpage) /*!< in/out: bpage to remove */
129+ buf_page_t* bpage, /*!< in/out: bpage to remove */
130+ ibool* must_restart) /*!< in/out: if TRUE, must restart the
131+ flush list scan */
132 {
133 mutex_t* block_mutex;
134 ibool processed = FALSE;
135@@ -487,7 +521,8 @@
136 buf_pool->mutex and block_mutex. It is safe to check
137 them while holding buf_pool->mutex only. */
138
139- if (buf_page_get_io_fix(bpage) != BUF_IO_NONE) {
140+ if (UNIV_UNLIKELY(buf_page_get_io_fix_unlocked(bpage)
141+ != BUF_IO_NONE)) {
142
143 /* We cannot remove this page during this scan
144 yet; maybe the system is currently reading it
145@@ -496,21 +531,38 @@
146 } else {
147
148 /* We have to release the flush_list_mutex to obey the
149- latching order. We are however guaranteed that the page
150- will stay in the flush_list because buf_flush_remove()
151- needs buf_pool->mutex as well (for the non-flush case). */
152+ latching order. We are not however guaranteed that the page
153+ will stay in the flush_list. */
154
155 buf_flush_list_mutex_exit(buf_pool);
156
157+ /* We don't have to worry about bpage becoming a dangling
158+ pointer by a compressed page flush list relocation because
159+ buf_page_get_gen() won't be called for pages from this
160+ tablespace. */
161+
162 mutex_enter(block_mutex);
163
164- ut_ad(bpage->oldest_modification != 0);
165-
166- if (bpage->buf_fix_count == 0) {
167-
168- buf_flush_remove(bpage);
169-
170- processed = TRUE;
171+ /* Recheck the page I/O fix and the flush list presence now
172+ thatwe hold the right mutex. */
173+ if (UNIV_UNLIKELY(buf_page_get_io_fix(bpage) != BUF_IO_NONE
174+ || bpage->oldest_modification == 0)) {
175+
176+ /* The page became I/O-fixed or is not on the flush
177+ list anymore, this invalidates any flush-list-page
178+ pointers we have. */
179+ *must_restart = TRUE;
180+
181+ } else {
182+
183+ ut_ad(bpage->oldest_modification != 0);
184+
185+ if (bpage->buf_fix_count == 0) {
186+
187+ buf_flush_remove(bpage);
188+
189+ processed = TRUE;
190+ }
191 }
192
193 mutex_exit(block_mutex);
194@@ -541,11 +593,12 @@
195 buf_page_t* bpage;
196 ulint processed = 0;
197 ibool all_freed = TRUE;
198+ ibool must_restart = FALSE;
199
200 buf_flush_list_mutex_enter(buf_pool);
201
202 for (bpage = UT_LIST_GET_LAST(buf_pool->flush_list);
203- bpage != NULL;
204+ !must_restart && bpage != NULL;
205 bpage = prev) {
206
207 ut_a(buf_page_in_file(bpage));
208@@ -561,22 +614,31 @@
209 /* Skip this block, as it does not belong to
210 the target space. */
211
212- } else if (!buf_flush_or_remove_page(buf_pool, bpage)) {
213+ } else if (!buf_flush_or_remove_page(buf_pool, bpage,
214+ &must_restart)) {
215
216 /* Remove was unsuccessful, we have to try again
217 by scanning the entire list from the end. */
218
219 all_freed = FALSE;
220 }
221+ if (UNIV_UNLIKELY(must_restart)) {
222+ ut_ad(!all_freed);
223+ break;
224+ }
225
226 ++processed;
227
228 /* Yield if we have hogged the CPU and mutexes for too long. */
229- if (buf_flush_try_yield(buf_pool, prev, processed)) {
230+ if (buf_flush_try_yield(buf_pool, prev, processed,
231+ &must_restart)) {
232
233+ ut_ad(!must_restart);
234 /* Reset the batch size counter if we had to yield. */
235
236 processed = 0;
237+ } else if (UNIV_UNLIKELY(must_restart)) {
238+ all_freed = FALSE;
239 }
240
241 }
242@@ -641,41 +703,39 @@
243 /* No op */) {
244
245 buf_page_t* prev_bpage;
246- mutex_t* block_mutex = NULL;
247+ mutex_t* block_mutex;
248
249 ut_a(buf_page_in_file(bpage));
250 ut_ad(bpage->in_LRU_list);
251
252 prev_bpage = UT_LIST_GET_PREV(LRU, bpage);
253
254- /* bpage->space and bpage->io_fix are protected by
255- buf_pool->mutex and the block_mutex. It is safe to check
256- them while holding buf_pool->mutex only. */
257+ block_mutex = buf_page_get_mutex_enter(bpage);
258+
259+ if (!block_mutex) {
260+ /* It may be impossible case...
261+ Something wrong, so will be scan_again */
262+
263+ all_freed = FALSE;
264+ goto next_page;
265+ }
266
267 if (buf_page_get_space(bpage) != id) {
268 /* Skip this block, as it does not belong to
269 the space that is being invalidated. */
270+
271+ mutex_exit(block_mutex);
272 goto next_page;
273 } else if (buf_page_get_io_fix(bpage) != BUF_IO_NONE) {
274 /* We cannot remove this page during this scan
275 yet; maybe the system is currently reading it
276 in, or flushing the modifications to the file */
277
278+ mutex_exit(block_mutex);
279 all_freed = FALSE;
280 goto next_page;
281 } else {
282
283- block_mutex = buf_page_get_mutex_enter(bpage);
284-
285- if (!block_mutex) {
286- /* It may be impossible case...
287- Something wrong, so will be scan_again */
288-
289- all_freed = FALSE;
290- goto next_page;
291- }
292-
293-
294 if (bpage->buf_fix_count > 0) {
295
296 mutex_exit(block_mutex);
297
298=== modified file 'Percona-Server/storage/innobase/fil/fil0fil.c'
299--- Percona-Server/storage/innobase/fil/fil0fil.c 2013-01-09 23:45:25 +0000
300+++ Percona-Server/storage/innobase/fil/fil0fil.c 2013-02-06 13:11:25 +0000
301@@ -5575,7 +5575,7 @@
302 && ((buf_page_t*)message)->space_was_being_deleted) {
303
304 /* intended not to be uncompress read page */
305- ut_a(buf_page_get_io_fix(message) == BUF_IO_WRITE
306+ ut_a(buf_page_get_io_fix_unlocked(message) == BUF_IO_WRITE
307 || !buf_page_get_zip_size(message)
308 || buf_page_get_state(message) != BUF_BLOCK_FILE_PAGE);
309
310
311=== modified file 'Percona-Server/storage/innobase/ibuf/ibuf0ibuf.c'
312--- Percona-Server/storage/innobase/ibuf/ibuf0ibuf.c 2013-01-18 03:34:53 +0000
313+++ Percona-Server/storage/innobase/ibuf/ibuf0ibuf.c 2013-02-06 13:11:25 +0000
314@@ -4478,7 +4478,7 @@
315 ut_ad(!block || buf_block_get_space(block) == space);
316 ut_ad(!block || buf_block_get_page_no(block) == page_no);
317 ut_ad(!block || buf_block_get_zip_size(block) == zip_size);
318- ut_ad(!block || buf_block_get_io_fix(block) == BUF_IO_READ);
319+ ut_ad(!block || buf_block_get_io_fix_unlocked(block) == BUF_IO_READ);
320
321 if (srv_force_recovery >= SRV_FORCE_NO_IBUF_MERGE
322 || trx_sys_hdr_page(space, page_no)) {
323
324=== modified file 'Percona-Server/storage/innobase/include/buf0buf.h'
325--- Percona-Server/storage/innobase/include/buf0buf.h 2013-01-18 12:02:28 +0000
326+++ Percona-Server/storage/innobase/include/buf0buf.h 2013-02-06 13:11:25 +0000
327@@ -958,7 +958,7 @@
328 ulint space, /*!< in: tablespace id */
329 ulint page_no);/*!< in: page number */
330 /*********************************************************************//**
331-Gets the io_fix state of a block.
332+Gets the io_fix state of a block. Requires that the block mutex is held.
333 @return io_fix state */
334 UNIV_INLINE
335 enum buf_io_fix
336@@ -967,7 +967,17 @@
337 const buf_page_t* bpage) /*!< in: pointer to the control block */
338 __attribute__((pure));
339 /*********************************************************************//**
340-Gets the io_fix state of a block.
341+Gets the io_fix state of a block. Does not assert that the block mutex is
342+held, to be used in the cases where it is safe not to hold it.
343+@return io_fix state */
344+UNIV_INLINE
345+enum buf_io_fix
346+buf_page_get_io_fix_unlocked(
347+/*=========================*/
348+ const buf_page_t* bpage) /*!< in: pointer to the control block */
349+ __attribute__((pure));
350+/*********************************************************************//**
351+Gets the io_fix state of a block. Requires that the block mutex is held.
352 @return io_fix state */
353 UNIV_INLINE
354 enum buf_io_fix
355@@ -976,6 +986,16 @@
356 const buf_block_t* block) /*!< in: pointer to the control block */
357 __attribute__((pure));
358 /*********************************************************************//**
359+Gets the io_fix state of a block. Does not assert that the block mutex is
360+held, to be used in the cases where it is safe not to hold it.
361+@return io_fix state */
362+UNIV_INLINE
363+enum buf_io_fix
364+buf_block_get_io_fix_unlocked(
365+/*==========================*/
366+ const buf_block_t* block) /*!< in: pointer to the control block */
367+ __attribute__((pure));
368+/*********************************************************************//**
369 Sets the io_fix state of a block. */
370 UNIV_INLINE
371 void
372
373=== modified file 'Percona-Server/storage/innobase/include/buf0buf.ic'
374--- Percona-Server/storage/innobase/include/buf0buf.ic 2013-01-17 22:50:22 +0000
375+++ Percona-Server/storage/innobase/include/buf0buf.ic 2013-02-06 13:11:25 +0000
376@@ -434,7 +434,7 @@
377 }
378
379 /*********************************************************************//**
380-Gets the io_fix state of a block.
381+Gets the io_fix state of a block. Requires that the block mutex is held.
382 @return io_fix state */
383 UNIV_INLINE
384 enum buf_io_fix
385@@ -442,6 +442,20 @@
386 /*================*/
387 const buf_page_t* bpage) /*!< in: pointer to the control block */
388 {
389+ ut_ad(mutex_own(buf_page_get_mutex(bpage)));
390+ return buf_page_get_io_fix_unlocked(bpage);
391+}
392+
393+/*********************************************************************//**
394+Gets the io_fix state of a block. Does not assert that the block mutex is
395+held, to be used in the cases where it is safe not to hold it.
396+@return io_fix state */
397+UNIV_INLINE
398+enum buf_io_fix
399+buf_page_get_io_fix_unlocked(
400+/*=========================*/
401+ const buf_page_t* bpage) /*!< in: pointer to the control block */
402+{
403 enum buf_io_fix io_fix = (enum buf_io_fix) bpage->io_fix;
404 #ifdef UNIV_DEBUG
405 switch (io_fix) {
406@@ -457,7 +471,7 @@
407 }
408
409 /*********************************************************************//**
410-Gets the io_fix state of a block.
411+Gets the io_fix state of a block. Requires that the block mutex is held.
412 @return io_fix state */
413 UNIV_INLINE
414 enum buf_io_fix
415@@ -469,6 +483,19 @@
416 }
417
418 /*********************************************************************//**
419+Gets the io_fix state of a block. Does not assert that the block mutex is
420+held, to be used in the cases where it is safe not to hold it.
421+@return io_fix state */
422+UNIV_INLINE
423+enum buf_io_fix
424+buf_block_get_io_fix_unlocked(
425+/*==========================*/
426+ const buf_block_t* block) /*!< in: pointer to the control block */
427+{
428+ return(buf_page_get_io_fix_unlocked(&block->page));
429+}
430+
431+/*********************************************************************//**
432 Sets the io_fix state of a block. */
433 UNIV_INLINE
434 void

Subscribers

People subscribed via source and target branches