Merge lp:~stewart/percona-server/5.5.27-plus-bugfixes into lp:percona-server/5.5

[Stewart Smith]

Include critical bugfixes based on Alexey's feedback to initial 5.5.27 branch. This merge req has that branch as a prerequisite.

http://jenkins.percona.com/job/percona-server-5.5-param/461/

[Laurynas Biveinis (laurynas-biveinis)]

Does this apply to 5.1? (If it does, should be fixed there first and merged up...)

Why did you add the "impossible case" if (!block_mutex) ... ?

review: Needs Information

[Stewart Smith (stewart)]

Laurynas Biveinis <email address hidden> writes:

> Review: Needs Information
>
> Does this apply to 5.1? (If it does, should be fixed there first and
> merged up...)

Haven't checked to be honest :)

>
> Why did you add the "impossible case" if (!block_mutex) ... ?

It was there in original code that was missed during merge.... it is a
bit odd though.

--
Stewart Smith

[Laurynas Biveinis (laurynas-biveinis)]

> > Does this apply to 5.1? (If it does, should be fixed there first and
> > merged up...)
>
> Haven't checked to be honest :)

OK if the fixes do not apply for 5.1. If they do, then 5.1 has to be fixed and upmerged.

[Stewart Smith (stewart)]

> > > Does this apply to 5.1? (If it does, should be fixed there first and
> > > merged up...)
> >
> > Haven't checked to be honest :)
>
> OK if the fixes do not apply for 5.1. If they do, then 5.1 has to be fixed and
> upmerged.

FYI they don't seem to apply to 5.1 from code inspection.

[Alexey Kopytov (akopytov)]

I don't the "It may be impossible case" code in the original fix. The upstream code is:

---
  } else {

   block_mutex = buf_page_get_mutex(bpage);
   mutex_enter(block_mutex);

   if (bpage->buf_fix_count > 0) {

    mutex_exit(block_mutex);

    /* We cannot remove this page during
    this scan yet; maybe the system is
    currently reading it in, or flushing
    the modifications to the file */

    all_freed = FALSE;

    goto next_page;
   }
  }

---

Which appears to be rather different than what is now in Percona Server. Why?

review: Needs Information

Unmerged revisions

=== modified file 'Percona-Server/storage/innobase/buf/buf0lru.c'
--- Percona-Server/storage/innobase/buf/buf0lru.c	2012-08-10 06:51:31 +0000
+++ Percona-Server/storage/innobase/buf/buf0lru.c	2012-08-10 06:51:33 +0000
@@ -472,7 +472,7 @@
 	mutex_t*	block_mutex;
 	ibool		processed = FALSE;
 
-	ut_ad(buf_pool_mutex_own(buf_pool));
+	ut_ad(mutex_own(&buf_pool->LRU_list_mutex));
 	ut_ad(buf_flush_list_mutex_own(buf_pool));
 
 	block_mutex = buf_page_get_mutex(bpage);
@@ -595,11 +595,11 @@
 	ibool	all_freed;
 
 	do {
-		buf_pool_mutex_enter(buf_pool);
+		mutex_enter(&buf_pool->LRU_list_mutex);
 
 		all_freed = buf_flush_or_remove_pages(buf_pool, id);
 
-		buf_pool_mutex_exit(buf_pool);
+		mutex_exit(&buf_pool->LRU_list_mutex);
 
 		ut_ad(buf_flush_validate(buf_pool));
 
@@ -659,8 +659,16 @@
 			goto next_page;
 		} else {
 
-			block_mutex = buf_page_get_mutex(bpage);
-			mutex_enter(block_mutex);
+			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) {
 
@@ -694,7 +702,8 @@
 			ulint	page_no;
 			ulint	zip_size;
 
-			buf_pool_mutex_exit(buf_pool);
+			mutex_exit(&buf_pool->LRU_list_mutex);
+			rw_lock_x_unlock(&buf_pool->page_hash_latch);
 
 			zip_size = buf_page_get_zip_size(bpage);
 			page_no = buf_page_get_page_no(bpage);