List:Commits« Previous MessageNext Message »
From:marko.makela Date:December 1 2010 1:21pm
Subject:bzr push into mysql-5.5-innodb branch (marko.makela:3249 to 3250) Bug#58212
View as plain text  
 3250 Marko Mäkelä	2010-12-01
      Bug#58212 Possible deadlock in change buffer in debug builds
      
      ibuf_page(): Renamed to ibuf_page_low(). Add the parameters file, line
      so that the latch diagnostics will be more meaningful.
      In debug builds, add the parameter ibool x_latch. When x_latch=FALSE,
      do not x-latch the page, but only buffer-fix it for reading the bit.
      In UNIV_SYNC_DEBUG, display a message if an insert buffer bitmap page
      was already latched. (The message should be displayed in those cases
      where the code would have previously failed.)
      
      ibuf_page(): A wrapper macro for ibuf_page_low(). Pass x_latch=TRUE.
      
      ibuf_bitmap_page_get_bits(): Renamed to ibuf_bitmap_page_get_bits_low().
      In UNIV_DEBUG, add the parameter latch_mode.
      Remove the parameter mtr unless UNIV_DEBUG is defined.
      
      ibuf_bitmap_page_get_bits(): A wrapper macro for
      ibuf_bitmap_page_get_bits_low(). Pass latch_type=MTR_MEMO_PAGE_X_FIX.
      
      buf_page_get_gen(): Use ibuf_page_low(x_latch=FALSE) in the debug assertion.
      This avoids the possible deadlock.

    modified:
      storage/innobase/buf/buf0buf.c
      storage/innobase/ibuf/ibuf0ibuf.c
      storage/innobase/include/ibuf0ibuf.h
 3249 Marko Mäkelä	2010-12-01
      Bug#58226 Some InnoDB debug checks consume too much CPU time
      
      Do not disable InnoDB inlining when UNIV_DEBUG is defined. The
      inlining is now solely controlled by the preprocessor symbol
      UNIV_MUST_NOT_INLINE and by any compiler options.
      
      mtr_memo_contains(): Add an explicit type conversion from void*, so
      that the function can be compiled by a C++ compiler. Previously, this
      function was never seen by the C++ compiler, because it is only
      present in UNIV_DEBUG builds and InnoDB inlining used to be disabled.
      
      buf_flush_validate_skip(): A wrapper that skips most calls of
      buf_flush_validate_low(). Invoked by debug assertions in
      buf_flush_insert_into_flush_list() and buf_flush_remove().
      
      fil_validate_skip(): A wrapper that skips most calls of
      fil_validate(). Invoked by debug assertions in fil_io() and fil_io_wait().
      
      os_aio_validate_skip(): A wrapper that skips most calls of
      os_aio_validate(). Invoked by debug assertions in
      os_aio_func(), os_aio_windows_handle() and os_aio_simulated_handle.
      
      os_get_os_version(): Only include this function if __WIN__ is defined.
      
      sync_array_deadlock_step(): Slight optimizations. This function is a
      major CPU consumer in UNIV_SYNC_DEBUG builds.

    modified:
      storage/innobase/buf/buf0flu.c
      storage/innobase/fil/fil0fil.c
      storage/innobase/include/mtr0mtr.ic
      storage/innobase/include/os0file.h
      storage/innobase/include/univ.i
      storage/innobase/os/os0file.c
      storage/innobase/sync/sync0arr.c
=== modified file 'storage/innobase/buf/buf0buf.c'
--- a/storage/innobase/buf/buf0buf.c	revid:marko.makela@strippedkrewqblfopg7
+++ b/storage/innobase/buf/buf0buf.c	revid:marko.makela@stripped
@@ -2745,7 +2745,8 @@ buf_page_get_gen(
 	ut_ad(zip_size == fil_space_get_zip_size(space));
 	ut_ad(ut_is_2pow(zip_size));
 #ifndef UNIV_LOG_DEBUG
-	ut_ad(!ibuf_inside() || ibuf_page(space, zip_size, offset, NULL));
+	ut_ad(!ibuf_inside() || ibuf_page_low(space, zip_size, offset,
+					      FALSE, file, line, NULL));
 #endif
 	buf_pool->stat.n_page_gets++;
 	fold = buf_page_address_fold(space, offset);

=== modified file 'storage/innobase/ibuf/ibuf0ibuf.c'
--- a/storage/innobase/ibuf/ibuf0ibuf.c	revid:marko.makela@stripped
+++ b/storage/innobase/ibuf/ibuf0ibuf.c	revid:marko.makela@oracle.com-20101201130902-q0g54ufii06or4oe
@@ -656,22 +656,49 @@ ibuf_parse_bitmap_init(
 	return(ptr);
 }
 #ifndef UNIV_HOTBACKUP
+# ifdef UNIV_DEBUG
+/** Gets the desired bits for a given page from a bitmap page.
+@param page	in: bitmap page
+@param offset	in: page whose bits to get
+@param zs	in: compressed page size in bytes; 0 for uncompressed pages
+@param bit	in: IBUF_BITMAP_FREE, IBUF_BITMAP_BUFFERED, ...
+@param mtr	in: mini-transaction holding an x-latch on the bitmap page
+@return	value of bits */
+#  define ibuf_bitmap_page_get_bits(page, offset, zs, bit, mtr)	\
+	ibuf_bitmap_page_get_bits_low(page, offset, zs,	\
+				      MTR_MEMO_PAGE_X_FIX, mtr, bit)
+# else /* UNIV_DEBUG */
+/** Gets the desired bits for a given page from a bitmap page.
+@param page	in: bitmap page
+@param offset	in: page whose bits to get
+@param zs	in: compressed page size in bytes; 0 for uncompressed pages
+@param bit	in: IBUF_BITMAP_FREE, IBUF_BITMAP_BUFFERED, ...
+@param mtr	in: mini-transaction holding an x-latch on the bitmap page
+@return	value of bits */
+#  define ibuf_bitmap_page_get_bits(page, offset, zs, bit, mtr)		\
+	ibuf_bitmap_page_get_bits_low(page, offset, zs, bit)
+# endif /* UNIV_DEBUG */
+
 /********************************************************************//**
 Gets the desired bits for a given page from a bitmap page.
 @return	value of bits */
 UNIV_INLINE
 ulint
-ibuf_bitmap_page_get_bits(
-/*======================*/
+ibuf_bitmap_page_get_bits_low(
+/*==========================*/
 	const page_t*	page,	/*!< in: bitmap page */
 	ulint		page_no,/*!< in: page whose bits to get */
 	ulint		zip_size,/*!< in: compressed page size in bytes;
 				0 for uncompressed pages */
-	ulint		bit,	/*!< in: IBUF_BITMAP_FREE,
+#ifdef UNIV_DEBUG
+	ulint		latch_type,
+				/*!< in: MTR_MEMO_PAGE_X_FIX,
+				MTR_MEMO_BUF_FIX, ... */
+	mtr_t*		mtr,	/*!< in: mini-transaction holding latch_type
+				on the bitmap page */
+#endif /* UNIV_DEBUG */
+	ulint		bit)	/*!< in: IBUF_BITMAP_FREE,
 				IBUF_BITMAP_BUFFERED, ... */
-	mtr_t*		mtr __attribute__((unused)))
-				/*!< in: mtr containing an
-				x-latch to the bitmap page */
 {
 	ulint	byte_offset;
 	ulint	bit_offset;
@@ -683,7 +710,7 @@ ibuf_bitmap_page_get_bits(
 # error "IBUF_BITS_PER_PAGE % 2 != 0"
 #endif
 	ut_ad(ut_is_2pow(zip_size));
-	ut_ad(mtr_memo_contains_page(mtr, page, MTR_MEMO_PAGE_X_FIX));
+	ut_ad(mtr_memo_contains_page(mtr, page, latch_type));
 
 	if (!zip_size) {
 		bit_offset = (page_no % UNIV_PAGE_SIZE) * IBUF_BITS_PER_PAGE
@@ -1109,21 +1136,29 @@ Must not be called when recv_no_ibuf_ope
 @return	TRUE if level 2 or level 3 page */
 UNIV_INTERN
 ibool
-ibuf_page(
-/*======*/
-	ulint	space,	/*!< in: space id */
-	ulint	zip_size,/*!< in: compressed page size in bytes, or 0 */
-	ulint	page_no,/*!< in: page number */
-	mtr_t*	mtr)	/*!< in: mtr which will contain an x-latch to the
-			bitmap page if the page is not one of the fixed
-			address ibuf pages, or NULL, in which case a new
-			transaction is created. */
+ibuf_page_low(
+/*==========*/
+	ulint		space,	/*!< in: space id */
+	ulint		zip_size,/*!< in: compressed page size in bytes, or 0 */
+	ulint		page_no,/*!< in: page number */
+#ifdef UNIV_DEBUG
+	ibool		x_latch,/*!< in: FALSE if relaxed check
+				(avoid latching the bitmap page) */
+#endif /* UNIV_DEBUG */
+	const char*	file,	/*!< in: file name */
+	ulint		line,	/*!< in: line where called */
+	mtr_t*		mtr)	/*!< in: mtr which will contain an
+				x-latch to the bitmap page if the page
+				is not one of the fixed address ibuf
+				pages, or NULL, in which case a new
+				transaction is created. */
 {
 	ibool	ret;
 	mtr_t	local_mtr;
 	page_t*	bitmap_page;
 
 	ut_ad(!recv_no_ibuf_operations);
+	ut_ad(x_latch || mtr == NULL);
 
 	if (ibuf_fixed_addr_page(space, zip_size, page_no)) {
 
@@ -1135,12 +1170,55 @@ ibuf_page(
 
 	ut_ad(fil_space_get_type(IBUF_SPACE_ID) == FIL_TABLESPACE);
 
+#ifdef UNIV_DEBUG
+	if (!x_latch) {
+		mtr_start(&local_mtr);
+
+		/* Get the bitmap page without a page latch, so that
+		we will not be violating the latching order when
+		another bitmap page has already been latched by this
+		thread. The page will be buffer-fixed, and thus it
+		cannot be removed or relocated while we are looking at
+		it. The contents of the page could change, but the
+		IBUF_BITMAP_IBUF bit that we are interested in should
+		not be modified by any other thread. Nobody should be
+		calling ibuf_add_free_page() or ibuf_remove_free_page()
+		while the page is linked to the insert buffer b-tree. */
+
+		bitmap_page = buf_block_get_frame(
+			buf_page_get_gen(
+				space, zip_size,
+				ibuf_bitmap_page_no_calc(zip_size, page_no),
+				RW_NO_LATCH, NULL, BUF_GET_NO_LATCH,
+				file, line, &local_mtr));
+# ifdef UNIV_SYNC_DEBUG
+		/* This is for tracking Bug #58212. This check and message can
+		be removed once it has been established that our assumptions
+		about this condition are correct. The bug was only a one-time
+		occurrence, unable to repeat since then. */
+		void* latch = sync_thread_levels_contains(SYNC_IBUF_BITMAP);
+		if (latch) {
+			fprintf(stderr, "Bug#58212 UNIV_SYNC_DEBUG"
+				" levels %p (%u,%u)\n",
+				latch, (unsigned) space, (unsigned) page_no);
+		}
+# endif /* UNIV_SYNC_DEBUG */
+		ret = ibuf_bitmap_page_get_bits_low(
+			bitmap_page, page_no, zip_size,
+			MTR_MEMO_BUF_FIX, &local_mtr, IBUF_BITMAP_IBUF);
+
+		mtr_commit(&local_mtr);
+		return(ret);
+	}
+#endif /* UNIV_DEBUG */
+
 	if (mtr == NULL) {
 		mtr = &local_mtr;
 		mtr_start(mtr);
 	}
 
-	bitmap_page = ibuf_bitmap_get_map_page(space, page_no, zip_size, mtr);
+	bitmap_page = ibuf_bitmap_get_map_page_func(space, page_no, zip_size,
+						    file, line, mtr);
 
 	ret = ibuf_bitmap_page_get_bits(bitmap_page, page_no, zip_size,
 					IBUF_BITMAP_IBUF, mtr);

=== modified file 'storage/innobase/include/ibuf0ibuf.h'
--- a/storage/innobase/include/ibuf0ibuf.h	revid:marko.makela@stripped01084333-c5ilkrewqblfopg7
+++ b/storage/innobase/include/ibuf0ibuf.h	revid:marko.makela@strippedq0g54ufii06or4oe
@@ -244,15 +244,44 @@ Must not be called when recv_no_ibuf_ope
 @return	TRUE if level 2 or level 3 page */
 UNIV_INTERN
 ibool
-ibuf_page(
-/*======*/
-	ulint	space,	/*!< in: space id */
-	ulint	zip_size,/*!< in: compressed page size in bytes, or 0 */
-	ulint	page_no,/*!< in: page number */
-	mtr_t*	mtr);	/*!< in: mtr which will contain an x-latch to the
-			bitmap page if the page is not one of the fixed
-			address ibuf pages, or NULL, in which case a new
-			transaction is created. */
+ibuf_page_low(
+/*==========*/
+	ulint		space,	/*!< in: space id */
+	ulint		zip_size,/*!< in: compressed page size in bytes, or 0 */
+	ulint		page_no,/*!< in: page number */
+#ifdef UNIV_DEBUG
+	ibool		x_latch,/*!< in: FALSE if relaxed check
+				(avoid latching the bitmap page) */
+#endif /* UNIV_DEBUG */
+	const char*	file,	/*!< in: file name */
+	ulint		line,	/*!< in: line where called */
+	mtr_t*		mtr)	/*!< in: mtr which will contain an
+				x-latch to the bitmap page if the page
+				is not one of the fixed address ibuf
+				pages, or NULL, in which case a new
+				transaction is created. */
+	__attribute__((warn_unused_result));
+#ifdef UNIV_DEBUG
+/** Checks if a page is a level 2 or 3 page in the ibuf hierarchy of
+pages.  Must not be called when recv_no_ibuf_operations==TRUE.
+@param space	tablespace identifier
+@param zip_size	compressed page size in bytes, or 0
+@param page_no	page number
+@param mtr	mini-transaction or NULL
+@return TRUE if level 2 or level 3 page */
+# define ibuf_page(space, zip_size, page_no, mtr)			\
+	ibuf_page_low(space, zip_size, page_no, TRUE, __FILE__, __LINE__, mtr)
+#else /* UVIV_DEBUG */
+/** Checks if a page is a level 2 or 3 page in the ibuf hierarchy of
+pages.  Must not be called when recv_no_ibuf_operations==TRUE.
+@param space	tablespace identifier
+@param zip_size	compressed page size in bytes, or 0
+@param page_no	page number
+@param mtr	mini-transaction or NULL
+@return TRUE if level 2 or level 3 page */
+# define ibuf_page(space, zip_size, page_no, mtr)			\
+	ibuf_page_low(space, zip_size, page_no, __FILE__, __LINE__, mtr)
+#endif /* UVIV_DEBUG */
 /***********************************************************************//**
 Frees excess pages from the ibuf free list. This function is called when an OS
 thread calls fsp services to allocate a new file segment, or a new page to a

Attachment: [text/bzr-bundle] bzr/marko.makela@oracle.com-20101201130902-q0g54ufii06or4oe.bundle
Thread
bzr push into mysql-5.5-innodb branch (marko.makela:3249 to 3250) Bug#58212marko.makela1 Dec