List:Commits« Previous MessageNext Message »
From:marko.makela Date:November 30 2010 2:09pm
Subject:bzr commit into mysql-5.5-innodb branch (marko.makela:3243) Bug#58212
View as plain text  
#At file:///home/marko/innobase/dev/mysql2a/5.5-innodb/ based on revid:sunny.bains@strippedp06vpp49ny2

 3243 Marko Mäkelä	2010-11-30
      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
=== modified file 'storage/innobase/buf/buf0buf.c'
--- a/storage/innobase/buf/buf0buf.c	revid:sunny.bains@stripped0-52jzzp06vpp49ny2
+++ b/storage/innobase/buf/buf0buf.c	revid:marko.makela@strippedjzc
@@ -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:sunny.bains@stripped
+++ b/storage/innobase/ibuf/ibuf0ibuf.c	revid:marko.makela@oracle.com-20101130140910-o3myzg74fy85hjzc
@@ -656,22 +656,39 @@ ibuf_parse_bitmap_init(
 	return(ptr);
 }
 #ifndef UNIV_HOTBACKUP
+# ifdef UNIV_DEBUG
+/** Gets the desired bits for a given page from a 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.
+@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,
-				IBUF_BITMAP_BUFFERED, ... */
-	mtr_t*		mtr __attribute__((unused)))
-				/*!< in: mtr containing an
+#ifdef UNIV_DEBUG
+	ulint		latch_type,
+				/*!< in: MTR_MEMO_PAGE_X_FIX,
+				MTR_MEMO_BUF_FIX, ... */
+	mtr_t*		mtr,	/*!< in: mtr containing an
 				x-latch to the bitmap page */
+#endif /* UNIV_DEBUG */
+	ulint		bit)	/*!< in: IBUF_BITMAP_FREE,
+				IBUF_BITMAP_BUFFERED, ... */
 {
 	ulint	byte_offset;
 	ulint	bit_offset;
@@ -683,7 +700,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 +1126,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 +1160,50 @@ 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
+		void* latch = sync_thread_levels_contains(SYNC_IBUF_BITMAP);
+		if (latch) {
+			fprintf(stderr, "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:sunny.bains@stripped
+++ b/storage/innobase/include/ibuf0ibuf.h	revid:marko.makela@oracle.com-20101130140910-o3myzg74fy85hjzc
@@ -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-20101130140910-o3myzg74fy85hjzc.bundle
Thread
bzr commit into mysql-5.5-innodb branch (marko.makela:3243) Bug#58212marko.makela30 Nov