keycache: restructure functions that are controlled by arguements

finish_resize_simple_key_cache removed argument acquire for acquiring
locks.

resize_simple_key_cache enforces assertion that the cache is inited.

read_block was really two functions, primary and secondary so separated
as such. Make the callers of read_block explictly use the required function.
Signed-off-by: Daniel Black <daniel@linux.vnet.ibm.com>

mysys/mf_keycache.c

@@ -749,7 +749,6 @@ int prepare_resize_simple_key_cache(SIMPLE_KEY_CACHE_CB *keycache,
   SYNOPSIS
     finish_resize_simple_key_cache()
     keycache                pointer to the control block of a simple key cache		
-    acquire_lock            <=> acquire the key cache lock at start
 
   DESCRIPTION
     This function performs finalizing actions for the operation of 
@@ -757,8 +756,6 @@ int prepare_resize_simple_key_cache(SIMPLE_KEY_CACHE_CB *keycache,
     keycache as a pointer to the control block structure of the type
     SIMPLE_KEY_CACHE_CB for this key cache. The function sets the flag
     in_resize in this structure to FALSE.
-    The parameter acquire_lock says whether the key cache lock must be
-    acquired at the start of the function.
 
   RETURN VALUE
     none
@@ -770,14 +767,10 @@ int prepare_resize_simple_key_cache(SIMPLE_KEY_CACHE_CB *keycache,
 */
 
 static 
-void finish_resize_simple_key_cache(SIMPLE_KEY_CACHE_CB *keycache,
-                                    my_bool acquire_lock)
+void finish_resize_simple_key_cache(SIMPLE_KEY_CACHE_CB *keycache)
 {
   DBUG_ENTER("finish_resize_simple_key_cache");
 
-  if (acquire_lock)
-    keycache_pthread_mutex_lock(&keycache->cache_lock); 
-  
   mysql_mutex_assert_owner(&keycache->cache_lock);
 			   
   /*
@@ -849,8 +842,7 @@ int resize_simple_key_cache(SIMPLE_KEY_CACHE_CB *keycache,
   int blocks= 0;
   DBUG_ENTER("resize_simple_key_cache");
 
-  if (!keycache->key_cache_inited)
-    DBUG_RETURN(blocks);
+  DBUG_ASSERT(keycache->key_cache_inited);
 
   /*
     Note that the cache_lock mutex and the resize_queue are left untouched.
@@ -866,7 +858,7 @@ int resize_simple_key_cache(SIMPLE_KEY_CACHE_CB *keycache,
                                 changed_blocks_hash_size);
 
 finish:
-  finish_resize_simple_key_cache(keycache, 0);
+  finish_resize_simple_key_cache(keycache);
 
   DBUG_RETURN(blocks);
 }
@@ -2611,12 +2603,11 @@ static BLOCK_LINK *find_key_block(SIMPLE_KEY_CACHE_CB *keycache,
 
   SYNOPSIS
 
-    read_block()
+    read_block_{primary|secondary}()
       keycache            pointer to a key cache data structure
       block               block to which buffer the data is to be read
       read_length         size of data to be read
       min_length          at least so much data must be read
-      primary             <-> the current thread will read the data
 
   RETURN VALUE
     None
@@ -2630,90 +2621,100 @@ static BLOCK_LINK *find_key_block(SIMPLE_KEY_CACHE_CB *keycache,
     portion is less than read_length, but not less than min_length.
 */
 
-static void read_block(SIMPLE_KEY_CACHE_CB *keycache,
-                       BLOCK_LINK *block, uint read_length,
-                       uint min_length, my_bool primary)
+static void read_block_primary(SIMPLE_KEY_CACHE_CB *keycache,
+                               BLOCK_LINK *block, uint read_length,
+                               uint min_length)
 {
   size_t got_length;
 
   /* On entry cache_lock is locked */
 
-  KEYCACHE_THREAD_TRACE("read_block");
-  if (primary)
-  {
-    /*
-      This code is executed only by threads that submitted primary
-      requests. Until block->status contains BLOCK_READ, all other
-      request for the block become secondary requests. For a primary
-      request the block must be properly initialized.
-    */
-    DBUG_ASSERT(((block->status & ~BLOCK_FOR_UPDATE) == BLOCK_IN_USE) ||
-                fail_block(block));
-    DBUG_ASSERT((block->length == 0) || fail_block(block));
-    DBUG_ASSERT((block->offset == keycache->key_cache_block_size) ||
-                fail_block(block));
-    DBUG_ASSERT((block->requests > 0) || fail_block(block));
-
-    KEYCACHE_DBUG_PRINT("read_block",
-                        ("page to be read by primary request"));
-
-    keycache->global_cache_read++;
-    /* Page is not in buffer yet, is to be read from disk */
-    keycache_pthread_mutex_unlock(&keycache->cache_lock);
-    /*
-      Here other threads may step in and register as secondary readers.
-      They will register in block->wqueue[COND_FOR_REQUESTED].
-    */
-    got_length= my_pread(block->hash_link->file, block->buffer,
-                         read_length, block->hash_link->diskpos, MYF(0));
-    keycache_pthread_mutex_lock(&keycache->cache_lock);
-    /*
-      The block can now have been marked for free (in case of
-      FLUSH_RELEASE). Otherwise the state must be unchanged.
-    */
-    DBUG_ASSERT(((block->status & ~(BLOCK_REASSIGNED |
-                                    BLOCK_FOR_UPDATE)) == BLOCK_IN_USE) ||
-                fail_block(block));
-    DBUG_ASSERT((block->length == 0) || fail_block(block));
-    DBUG_ASSERT((block->offset == keycache->key_cache_block_size) ||
-                fail_block(block));
-    DBUG_ASSERT((block->requests > 0) || fail_block(block));
-
-    if (got_length < min_length)
-      block->status|= BLOCK_ERROR;
-    else
-    {
-      block->status|= BLOCK_READ;
-      block->length= got_length;
-      /*
-        Do not set block->offset here. If this block is marked
-        BLOCK_CHANGED later, we want to flush only the modified part. So
-        only a writer may set block->offset down from
-        keycache->key_cache_block_size.
-      */
-    }
-    KEYCACHE_DBUG_PRINT("read_block",
-                        ("primary request: new page in cache"));
-    /* Signal that all pending requests for this page now can be processed */
-    release_whole_queue(&block->wqueue[COND_FOR_REQUESTED]);
-  }
+  KEYCACHE_THREAD_TRACE("read_block_primary");
+
+  /*
+    This code is executed only by threads that submitted primary
+    requests. Until block->status contains BLOCK_READ, all other
+    request for the block become secondary requests. For a primary
+    request the block must be properly initialized.
+  */
+  DBUG_ASSERT(((block->status & ~BLOCK_FOR_UPDATE) == BLOCK_IN_USE) ||
+              fail_block(block));
+  DBUG_ASSERT((block->length == 0) || fail_block(block));
+  DBUG_ASSERT((block->offset == keycache->key_cache_block_size) ||
+              fail_block(block));
+  DBUG_ASSERT((block->requests > 0) || fail_block(block));
+
+  KEYCACHE_DBUG_PRINT("read_block_primary",
+                      ("page to be read by primary request"));
+
+  keycache->global_cache_read++;
+  /* Page is not in buffer yet, is to be read from disk */
+  keycache_pthread_mutex_unlock(&keycache->cache_lock);
+  /*
+    Here other threads may step in and register as secondary readers.
+    They will register in block->wqueue[COND_FOR_REQUESTED].
+  */
+  got_length= my_pread(block->hash_link->file, block->buffer,
+                       read_length, block->hash_link->diskpos, MYF(0));
+  keycache_pthread_mutex_lock(&keycache->cache_lock);
+  /*
+    The block can now have been marked for free (in case of
+    FLUSH_RELEASE). Otherwise the state must be unchanged.
+  */
+  DBUG_ASSERT(((block->status & ~(BLOCK_REASSIGNED |
+                                  BLOCK_FOR_UPDATE)) == BLOCK_IN_USE) ||
+              fail_block(block));
+  DBUG_ASSERT((block->length == 0) || fail_block(block));
+  DBUG_ASSERT((block->offset == keycache->key_cache_block_size) ||
+              fail_block(block));
+  DBUG_ASSERT((block->requests > 0) || fail_block(block));
+
+  if (got_length < min_length)
+    block->status|= BLOCK_ERROR;
   else
   {
+    block->status|= BLOCK_READ;
+    block->length= got_length;
     /*
-      This code is executed only by threads that submitted secondary
-      requests. At this point it could happen that the cache block is
-      not yet assigned to the hash_link for the requested file block.
-      But at awake from the wait this should be the case. Unfortunately
-      we cannot assert this here because we do not know the hash_link
-      for the requested file block nor the file and position. So we have
-      to assert this in the caller.
+      Do not set block->offset here. If this block is marked
+      BLOCK_CHANGED later, we want to flush only the modified part. So
+      only a writer may set block->offset down from
+      keycache->key_cache_block_size.
     */
-    KEYCACHE_DBUG_PRINT("read_block",
-                      ("secondary request waiting for new page to be read"));
-    wait_on_queue(&block->wqueue[COND_FOR_REQUESTED], &keycache->cache_lock);
-    KEYCACHE_DBUG_PRINT("read_block",
-                        ("secondary request: new page in cache"));
   }
+  KEYCACHE_DBUG_PRINT("read_block_primary",
+                      ("primary request: new page in cache"));
+  /* Signal that all pending requests for this page now can be processed */
+  release_whole_queue(&block->wqueue[COND_FOR_REQUESTED]);
+
+  DBUG_ASSERT(keycache->can_be_used);
+}
+
+
+static void read_block_secondary(SIMPLE_KEY_CACHE_CB *keycache,
+                                 BLOCK_LINK *block)
+{
+  KEYCACHE_THREAD_TRACE("read_block_secondary");
+
+  /*
+    This code is executed only by threads that submitted secondary
+    requests. At this point it could happen that the cache block is
+    not yet assigned to the hash_link for the requested file block.
+    But at awake from the wait this should be the case. Unfortunately
+    we cannot assert this here because we do not know the hash_link
+    for the requested file block nor the file and position. So we have
+    to assert this in the caller.
+  */
+  KEYCACHE_DBUG_PRINT("read_block_secondary",
+                      ("secondary request waiting for new page to be read"));
+
+  wait_on_queue(&block->wqueue[COND_FOR_REQUESTED], &keycache->cache_lock);
+
+  KEYCACHE_DBUG_PRINT("read_block_secondary",
+                      ("secondary request: new page in cache"));
+
+  DBUG_ASSERT(keycache->can_be_used);
+  DBUG_ASSERT(block->status & (BLOCK_READ | BLOCK_IN_USE));
 }
 
 
@@ -2858,22 +2859,24 @@ uchar *simple_key_cache_read(SIMPLE_KEY_CACHE_CB *keycache,
       }
       if (!(block->status & BLOCK_ERROR))
       {
-        if (page_st != PAGE_READ)
+        if (page_st == PAGE_TO_BE_READ)
+        {
+          MYSQL_KEYCACHE_READ_MISS();
+          read_block_primary(keycache, block,
+                     keycache->key_cache_block_size, read_length+offset);
+        }
+        else if (page_st == PAGE_WAIT_TO_BE_READ)
         {
           MYSQL_KEYCACHE_READ_MISS();
           /* The requested page is to be read into the block buffer */
-          read_block(keycache, block,
-                     keycache->key_cache_block_size, read_length+offset,
-                     (my_bool)(page_st == PAGE_TO_BE_READ));
+          read_block_secondary(keycache, block);
+
           /*
             A secondary request must now have the block assigned to the
-            requested file block. It does not hurt to check it for
-            primary requests too.
+            requested file block.
           */
-          DBUG_ASSERT(keycache->can_be_used);
           DBUG_ASSERT(block->hash_link->file == file);
           DBUG_ASSERT(block->hash_link->diskpos == filepos);
-          DBUG_ASSERT(block->status & (BLOCK_READ | BLOCK_IN_USE));
         }
         else if (block->length < read_length + offset)
         {
@@ -3077,23 +3080,30 @@ int simple_key_cache_insert(SIMPLE_KEY_CACHE_CB *keycache,
       }
       if (!(block->status & BLOCK_ERROR))
       {
-        if ((page_st == PAGE_WAIT_TO_BE_READ) ||
-            ((page_st == PAGE_TO_BE_READ) &&
-             (offset || (read_length < keycache->key_cache_block_size))))
+        if (page_st == PAGE_WAIT_TO_BE_READ)
         {
           /*
-            Either
-
             this is a secondary request for a block to be read into the
             cache. The block is in eviction. It is not yet assigned to
             the requested file block (It does not point to the right
             hash_link). So we cannot call remove_reader() on the block.
             And we cannot access the hash_link directly here. We need to
-            wait until the assignment is complete. read_block() executes
-            the correct wait when called with primary == FALSE.
-
-            Or
+            wait until the assignment is complete. read_block_secondary()
+            executes the correct wait.
+          */
+          read_block_secondary(keycache, block);
 
+          /*
+            A secondary request must now have the block assigned to the
+            requested file block.
+          */
+          DBUG_ASSERT(block->hash_link->file == file);
+          DBUG_ASSERT(block->hash_link->diskpos == filepos);
+        }
+        else if (page_st == PAGE_TO_BE_READ &&
+                 (offset || (read_length < keycache->key_cache_block_size)))
+        {
+          /*
             this is a primary request for a block to be read into the
             cache and the supplied data does not fill the whole block.
 
@@ -3108,17 +3118,8 @@ int simple_key_cache_insert(SIMPLE_KEY_CACHE_CB *keycache,
             Though reading again what the caller did read already is an
             expensive operation, we need to do this for correctness.
           */
-          read_block(keycache, block, keycache->key_cache_block_size,
-                     read_length + offset, (page_st == PAGE_TO_BE_READ));
-          /*
-            A secondary request must now have the block assigned to the
-            requested file block. It does not hurt to check it for
-            primary requests too.
-          */
-          DBUG_ASSERT(keycache->can_be_used);
-          DBUG_ASSERT(block->hash_link->file == file);
-          DBUG_ASSERT(block->hash_link->diskpos == filepos);
-          DBUG_ASSERT(block->status & (BLOCK_READ | BLOCK_IN_USE));
+          read_block_primary(keycache, block, keycache->key_cache_block_size,
+                             read_length + offset);
         }
         else if (page_st == PAGE_TO_BE_READ)
         {
@@ -3413,25 +3414,29 @@ int simple_key_cache_write(SIMPLE_KEY_CACHE_CB *keycache,
         reading the file block. If the read completes after us, it
         overwrites our new contents with the old contents. So we have to
         wait for the other thread to complete the read of this block.
-        read_block() takes care for the wait.
+        read_block_primary|secondary() takes care for the wait.
       */
-      if (!(block->status & BLOCK_ERROR) &&
-          ((page_st == PAGE_TO_BE_READ &&
-            (offset || read_length < keycache->key_cache_block_size)) ||
-           (page_st == PAGE_WAIT_TO_BE_READ)))
+      if (!(block->status & BLOCK_ERROR))
       {
-        read_block(keycache, block,
-                   offset + read_length >= keycache->key_cache_block_size?
-                   offset : keycache->key_cache_block_size,
-                   offset, (page_st == PAGE_TO_BE_READ));
-        DBUG_ASSERT(keycache->can_be_used);
-        DBUG_ASSERT(block->status & (BLOCK_READ | BLOCK_IN_USE));
-        /*
-          Prevent block from flushing and from being selected for to be
-          freed. This must be set when we release the cache_lock.
-          Here we set it in case we could not set it above.
-        */
-        block->status|= BLOCK_FOR_UPDATE;
+        if (page_st == PAGE_TO_BE_READ &&
+            (offset || read_length < keycache->key_cache_block_size))
+        {
+          read_block_primary(keycache, block,
+                             offset + read_length >= keycache->key_cache_block_size?
+                             offset : keycache->key_cache_block_size,
+                             offset);
+          /*
+            Prevent block from flushing and from being selected for to be
+            freed. This must be set when we release the cache_lock.
+            Here we set it in case we could not set it above.
+          */
+          block->status|= BLOCK_FOR_UPDATE;
+        }
+        else if (page_st == PAGE_WAIT_TO_BE_READ)
+        {
+          read_block_secondary(keycache, block);
+          block->status|= BLOCK_FOR_UPDATE;
+        }
       }
       /*
         The block should always be assigned to the requested file block
@@ -5279,7 +5284,8 @@ int resize_partitioned_key_cache(PARTITIONED_KEY_CACHE_CB *keycache,
   {
     for (i= 0; i < partitions; i++)
     {
-      finish_resize_simple_key_cache(keycache->partition_array[i], 1);
+      keycache_pthread_mutex_lock(&keycache->partition_array[i]->cache_lock);
+      finish_resize_simple_key_cache(keycache->partition_array[i]);
     }
   }
   DBUG_RETURN(blocks);