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

[Laurynas Biveinis]

Fix bug 934377, bug 1110102, bug 1111211, bug 1116447.
No BT but is part of 16274 QA.
http://jenkins.percona.com/job/percona-server-5.5-param/657/

[Stewart Smith (stewart)]

review: Approve

=== modified file 'Percona-Server/storage/innobase/buf/buf0buf.c'
--- Percona-Server/storage/innobase/buf/buf0buf.c	2013-01-18 12:02:28 +0000
+++ Percona-Server/storage/innobase/buf/buf0buf.c	2013-02-06 13:11:25 +0000
@@ -3938,7 +3938,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) {

=== modified file 'Percona-Server/storage/innobase/buf/buf0flu.c'
--- Percona-Server/storage/innobase/buf/buf0flu.c	2013-01-16 16:12:15 +0000
+++ Percona-Server/storage/innobase/buf/buf0flu.c	2013-02-06 13:11:25 +0000
@@ -915,7 +915,7 @@
 				"InnoDB: Page buf fix count %lu,"
 				" io fix %lu, state %lu\n",
 				(ulong)block->page.buf_fix_count,
-				(ulong)buf_block_get_io_fix(block),
+				(ulong)buf_block_get_io_fix_unlocked(block),
 				(ulong)buf_block_get_state(block));
 		}
 
@@ -1115,7 +1115,7 @@
 	ut_ad(!mutex_own(&buf_pool->LRU_list_mutex));
 	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

=== modified file 'Percona-Server/storage/innobase/buf/buf0lru.c'
--- Percona-Server/storage/innobase/buf/buf0lru.c	2013-01-16 13:31:48 +0000
+++ Percona-Server/storage/innobase/buf/buf0lru.c	2013-02-06 13:11:25 +0000
@@ -393,18 +393,18 @@
 {
 	mutex_t*	block_mutex;
 
+	block_mutex = buf_page_get_mutex(bpage);
+
+	ut_ad(mutex_own(block_mutex));
 	ut_ad(mutex_own(&buf_pool->LRU_list_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. */
+	/* Now it is safe to release the LRU list mutex. */
 	mutex_exit(&buf_pool->LRU_list_mutex);
 
 	mutex_exit(block_mutex);
@@ -415,7 +415,7 @@
 
 	mutex_enter(block_mutex);
 	/* "Unfix" the block now that we have both the
-	buffer pool and block mutex again. */
+	LRU list and block mutex again. */
 	buf_page_unset_sticky(bpage);
 	mutex_exit(block_mutex);
 }
@@ -431,7 +431,9 @@
 /*================*/
 	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 */
+	ibool*		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
@@ -441,10 +443,40 @@
 
 	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) {
+
+		mutex_t*	block_mutex;
 
 		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.  */
+
+		block_mutex = buf_page_get_mutex_enter(bpage);
+		if (UNIV_UNLIKELY(block_mutex == NULL)) {
+
+			buf_flush_list_mutex_enter(buf_pool);
+
+			*must_restart = TRUE;
+			return FALSE;
+		}
+
+		/* 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);
+			buf_flush_list_mutex_enter(buf_pool);
+
+			*must_restart = TRUE;
+			return FALSE;
+		}
+
+		*must_restart = FALSE;
+
 		/* Release the LRU list and block mutex
 		to give the other threads a go. */
 
@@ -473,7 +505,9 @@
 buf_flush_or_remove_page(
 /*=====================*/
 	buf_pool_t*	buf_pool,	/*!< in/out: buffer pool instance */
-	buf_page_t*	bpage)		/*!< in/out: bpage to remove */
+	buf_page_t*	bpage,		/*!< in/out: bpage to remove */
+	ibool*		must_restart)	/*!< in/out: if TRUE, must restart the
+					flush list scan */
 {
 	mutex_t*	block_mutex;
 	ibool		processed = FALSE;
@@ -487,7 +521,8 @@
 	buf_pool->mutex and block_mutex. It is safe to check
 	them while holding buf_pool->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
@@ -496,21 +531,38 @@
 	} else {
 
 		/* 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 because buf_flush_remove()
-		needs buf_pool->mutex as well (for the non-flush case). */
+		latching order. We are not however guaranteed that the page
+		will stay in the flush_list. */
 
 		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 (bpage->buf_fix_count == 0) {
-
-			buf_flush_remove(bpage);
-
-			processed = TRUE;
+		/* Recheck the page I/O fix and the flush list presence now
+		thatwe 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. */
+			*must_restart = TRUE;
+
+		} else {
+
+			ut_ad(bpage->oldest_modification != 0);
+
+			if (bpage->buf_fix_count == 0) {
+
+				buf_flush_remove(bpage);
+
+				processed = TRUE;
+			}
 		}
 
 		mutex_exit(block_mutex);
@@ -541,11 +593,12 @@
 	buf_page_t*	bpage;
 	ulint		processed = 0;
 	ibool		all_freed = TRUE;
+	ibool		must_restart = FALSE;
 
 	buf_flush_list_mutex_enter(buf_pool);
 
 	for (bpage = UT_LIST_GET_LAST(buf_pool->flush_list);
-	     bpage != NULL;
+	     !must_restart && bpage != NULL;
 	     bpage = prev) {
 
 		ut_a(buf_page_in_file(bpage));
@@ -561,22 +614,31 @@
 			/* Skip this block, as it does not belong to
 			the target space. */
 
-		} else if (!buf_flush_or_remove_page(buf_pool, bpage)) {
+		} else if (!buf_flush_or_remove_page(buf_pool, bpage,
+						     &must_restart)) {
 
 			/* Remove was unsuccessful, we have to try again
 			by scanning the entire list from the end. */
 
 			all_freed = FALSE;
 		}
+		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;
+		} else if (UNIV_UNLIKELY(must_restart)) {
+			all_freed = FALSE;
 		}
 
 	}
@@ -641,41 +703,39 @@
 	     /* No op */) {
 
 		buf_page_t*	prev_bpage;
-		mutex_t*	block_mutex = NULL;
+		mutex_t*	block_mutex;
 
 		ut_a(buf_page_in_file(bpage));
 		ut_ad(bpage->in_LRU_list);
 
 		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. */
+		block_mutex = buf_page_get_mutex_enter(bpage);
+
+		if (!block_mutex) {
+			/* It may be impossible case...
+			   Something wrong, so will be scan_again */
+
+			all_freed = FALSE;
+			goto next_page;
+		}
 
 		if (buf_page_get_space(bpage) != id) {
 			/* Skip this block, as it does not belong to
 			the space that is being invalidated. */
+
+			mutex_exit(block_mutex);
 			goto next_page;
 		} else if (buf_page_get_io_fix(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 */
 
+			mutex_exit(block_mutex);
 			all_freed = FALSE;
 			goto next_page;
 		} else {
 
-			block_mutex = buf_page_get_mutex_enter(bpage);
-
-			if (!block_mutex) {
-				/* It may be impossible case...
-				   Something wrong, so will be scan_again */
-
-				all_freed = FALSE;
-				goto next_page;
-			}
-
-
 			if (bpage->buf_fix_count > 0) {
 
 				mutex_exit(block_mutex);

=== modified file 'Percona-Server/storage/innobase/fil/fil0fil.c'
--- Percona-Server/storage/innobase/fil/fil0fil.c	2013-01-09 23:45:25 +0000
+++ Percona-Server/storage/innobase/fil/fil0fil.c	2013-02-06 13:11:25 +0000
@@ -5575,7 +5575,7 @@
 	    && ((buf_page_t*)message)->space_was_being_deleted) {
 
 		/* intended not to be uncompress read page */
-		ut_a(buf_page_get_io_fix(message) == BUF_IO_WRITE
+		ut_a(buf_page_get_io_fix_unlocked(message) == BUF_IO_WRITE
 		     || !buf_page_get_zip_size(message)
 		     || buf_page_get_state(message) != BUF_BLOCK_FILE_PAGE);
 

=== modified file 'Percona-Server/storage/innobase/ibuf/ibuf0ibuf.c'
--- Percona-Server/storage/innobase/ibuf/ibuf0ibuf.c	2013-01-18 03:34:53 +0000
+++ Percona-Server/storage/innobase/ibuf/ibuf0ibuf.c	2013-02-06 13:11:25 +0000
@@ -4478,7 +4478,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/buf0buf.h'
--- Percona-Server/storage/innobase/include/buf0buf.h	2013-01-18 12:02:28 +0000
+++ Percona-Server/storage/innobase/include/buf0buf.h	2013-02-06 13:11:25 +0000
@@ -958,7 +958,7 @@
 	ulint			space,	/*!< in: tablespace id */
 	ulint			page_no);/*!< in: page number */
 /*********************************************************************//**
-Gets the io_fix state of a block.
+Gets the io_fix state of a block.  Requires that the block mutex is held.
 @return	io_fix state */
 UNIV_INLINE
 enum buf_io_fix
@@ -967,7 +967,17 @@
 	const buf_page_t*	bpage)	/*!< in: pointer to the control block */
 	__attribute__((pure));
 /*********************************************************************//**
-Gets the io_fix state of a block.
+Gets the io_fix state of a block.  Does not assert that the block 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));
+/*********************************************************************//**
+Gets the io_fix state of a block.  Requires that the block mutex is held.
 @return	io_fix state */
 UNIV_INLINE
 enum buf_io_fix
@@ -976,6 +986,16 @@
 	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 block 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 */
+	__attribute__((pure));
+/*********************************************************************//**
 Sets the io_fix state of a block. */
 UNIV_INLINE
 void

=== modified file 'Percona-Server/storage/innobase/include/buf0buf.ic'
--- Percona-Server/storage/innobase/include/buf0buf.ic	2013-01-17 22:50:22 +0000
+++ Percona-Server/storage/innobase/include/buf0buf.ic	2013-02-06 13:11:25 +0000
@@ -434,7 +434,7 @@
 }
 
 /*********************************************************************//**
-Gets the io_fix state of a block.
+Gets the io_fix state of a block.  Requires that the block mutex is held.
 @return	io_fix state */
 UNIV_INLINE
 enum buf_io_fix
@@ -442,6 +442,20 @@
 /*================*/
 	const buf_page_t*	bpage)	/*!< in: pointer to the control block */
 {
+	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 block 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) {
@@ -457,7 +471,7 @@
 }
 
 /*********************************************************************//**
-Gets the io_fix state of a block.
+Gets the io_fix state of a block.  Requires that the block mutex is held.
 @return	io_fix state */
 UNIV_INLINE
 enum buf_io_fix
@@ -469,6 +483,19 @@
 }
 
 /*********************************************************************//**
+Gets the io_fix state of a block.  Does not assert that the block 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