[Ocfs2-commits] mfasheh commits r1326 - trunk/src

svn-commits at oss.oracle.com svn-commits at oss.oracle.com
Tue Aug 3 20:53:15 CDT 2004


Author: mfasheh
Date: 2004-08-03 19:53:13 -0500 (Tue, 03 Aug 2004)
New Revision: 1326

Modified:
   trunk/src/alloc.c
   trunk/src/file.c
   trunk/src/inode.c
   trunk/src/journal.c
   trunk/src/namei.c
   trunk/src/ocfs_journal.h
Log:
* default transactions to no syncing and no checkpointing instead of
  the latter. Lets us remove a bunch of lines of code :)

* remove even more code by deleting all the transaction checkpointing
  code (other than what the commit thread needs). We don't ever want to
  do this now, even in abort. We leave in sync as that may be used later
  (and is really trivial).

* now that we're past this hurdle, eventually (likely post 1.0) we'll
  want to do no more aborting of transactions. Provide a new flag
  "OCFS_HANDLE_ALWAYS_COMMITS" to help us along. It skips the normal
  copyout stuff which costs us performance on the condition that you
  don't abort that transaction.

* Make the oin map stuff in the open path always commit (trivial).

* Have ocfs_double_lock always put locks on the handle as it's only
  got one caller anyways which always passes a handle...



Modified: trunk/src/alloc.c
===================================================================
--- trunk/src/alloc.c	2004-08-03 18:55:49 UTC (rev 1325)
+++ trunk/src/alloc.c	2004-08-04 00:53:13 UTC (rev 1326)
@@ -346,8 +346,6 @@
 		LOG_ERROR_STATUS(status);
 		goto finally;
 	}
-	ocfs_handle_set_checkpoint(handle, 0);
-	ocfs_handle_set_sync(handle, 0);
 
 	/* Get all the locks we need. do global bitmap last to
 	 * preserve lock ordering with extend/create */

Modified: trunk/src/file.c
===================================================================
--- trunk/src/file.c	2004-08-03 18:55:49 UTC (rev 1325)
+++ trunk/src/file.c	2004-08-04 00:53:13 UTC (rev 1326)
@@ -131,9 +131,7 @@
 			LOG_ERROR_STATUS(status = -ENOMEM);
 			goto leave;
 		}
-		/* we want this to be a really fast transaction. */
-		ocfs_handle_set_sync(handle, 0);
-		ocfs_handle_set_checkpoint(handle, 0);
+		ocfs_handle_set_always_commits(handle, 1);
 	}
 
 	/* this will update the lock info from disk while also
@@ -158,12 +156,8 @@
 		LOG_ERROR_STATUS (status);
 
 leave:
-	if (local_handle && handle) {
-		if (status < 0)
-			ocfs_abort_trans(handle);
-		else
-			ocfs_commit_trans(handle);
-	}
+	if (local_handle && handle)
+		ocfs_commit_trans(handle);
 
 	LOG_EXIT_STATUS (status);
 	return status;
@@ -551,9 +545,6 @@
 		LOG_ERROR_STATUS(status);
 		goto leave;
 	}
-	/* we want this transaction to return quickly. */
-	ocfs_handle_set_checkpoint(handle, 0);
-	ocfs_handle_set_sync(handle, 0);
 
 	status = ocfs_acquire_lock (osb, OCFS_LKM_EXMODE, lock_flags,
 				    &bh, inode);
@@ -931,9 +922,6 @@
 		goto leave;
 	}
 
-	ocfs_handle_set_checkpoint(handle, 0);
-	ocfs_handle_set_sync(handle, 0);
-
 	status = ocfs_acquire_lock (osb, OCFS_LKM_EXMODE, 
 				    FLAG_FILE_TRUNCATE|FLAG_FILE_UPDATE_OIN,
 				    &bh, inode);
@@ -1120,10 +1108,6 @@
 			goto leave;
 		}
 
-		/* we don't want to checkpoint extends. */
-		ocfs_handle_set_checkpoint(handle, 0);
-		ocfs_handle_set_sync(handle, 0);
-
 		status = ocfs_acquire_lock (osb, OCFS_LKM_EXMODE, 
 					    FLAG_FILE_EXTEND, &bh, inode);
 		if (status < 0) {

Modified: trunk/src/inode.c
===================================================================
--- trunk/src/inode.c	2004-08-03 18:55:49 UTC (rev 1325)
+++ trunk/src/inode.c	2004-08-04 00:53:13 UTC (rev 1326)
@@ -641,8 +641,6 @@
 		LOG_ERROR_STATUS(-ENOMEM);
 		goto clear_inode;
 	}
-	ocfs_handle_set_checkpoint(handle, 0);
-	ocfs_handle_set_sync(handle, 0);
 
 	ocfs_handle_add_inode(handle, orphan_dir_inode);
 

Modified: trunk/src/journal.c
===================================================================
--- trunk/src/journal.c	2004-08-03 18:55:49 UTC (rev 1325)
+++ trunk/src/journal.c	2004-08-04 00:53:13 UTC (rev 1326)
@@ -57,7 +57,6 @@
 spinlock_t trans_inc_lock = SPIN_LOCK_UNLOCKED;
 
 static int ocfs_reset_publish (ocfs_super * osb, __u64 node_num);
-static int ocfs_handle_release_locks(ocfs_journal_handle *handle);
 static int ocfs_force_read_journal(ocfs_super *osb, __u64 size, 
 				   struct inode *inode);
 static int ocfs_recover_node(struct _ocfs_super *osb, int node_num);
@@ -470,10 +469,6 @@
 
 	atomic_inc(&(osb->journal->num_trans));
 
-	/* default handle flags! */
-	ocfs_handle_set_sync(retval, 1);
-	ocfs_handle_set_checkpoint(retval, 1);
-
 	LOG_EXIT_PTR(retval);
 	return(retval);
 
@@ -545,59 +540,6 @@
 	return;
 }
 
-/* 
- * Called from commit / abort only for the checkpointing case. When we
- * stop doing that there, we can kill this function. 
- */
-static int ocfs_handle_release_locks(ocfs_journal_handle *handle)
-{
-	ocfs_super *osb;
-	ocfs_journal_lock *lock;
-	int status = 0;
-	int tmpstat;
-	struct list_head *p, *n;
-	ocfs_journal *journal = handle->journal;
-
-	LOG_ENTRY();
-
-	osb = handle->osb;
-
-	LOG_TRACE_ARGS("num_locks = %d\n", handle->num_locks);
-
-	list_for_each_safe(p, n, &(handle->locks)) {
-		lock = list_entry(p, ocfs_journal_lock, lock_list);
-
-		if (!lock->inode)
-			BUG();
-
-		/* The file may have been deleted before we got to
-		 * this lock release. If so, just skip it.  */
-		if (!INODE_DELETED(lock->inode)) {
-
-			tmpstat = ocfs_release_lock(osb, 
-						    lock->type, 
-						    lock->flags, 
-						    lock->inode);
-			if (tmpstat < 0) {
-				LOG_ERROR_ARGS("Could not release lock: "
-					       "%llu\n", 
-					       OCFS_I(lock->inode)->ip_blkno << lock->inode->i_sb->s_blocksize_bits);
-				LOG_ERROR_STATUS(tmpstat);
-				status = tmpstat;
-			}
-		}
-
-		iput(lock->inode);
-		list_del(&(lock->lock_list));
-		handle->num_locks--;
-		atomic_dec(&journal->num_cmt_locks);
-		kmem_cache_free(OcfsGlobalCtxt.lock_cache, lock);
-	}
-
-	LOG_EXIT_STATUS(status);
-	return(status);
-}
-
 /* This for loop is for debug purposes. Basically we want to check the
  * BH_JBD bit on our buffers. If the handle was checkpointed, then
  * none of them should have that bit set after the revoke
@@ -621,11 +563,9 @@
 	ocfs_super *osb;
 	handle_t *kern_handle;
 	transaction_t *kern_trans;
-	int retval, i;
-	int checkpoint, sync;
+	int retval, i, sync;
 	ocfs_journal *journal;
 	ocfs_bitmap_free_head *commit_head;
-
 	LOG_ENTRY();
 
 	if (!handle)
@@ -635,14 +575,9 @@
 	kern_handle = handle->k_handle;
 	kern_trans = kern_handle->h_transaction;
 	journal = osb->journal;
-	checkpoint = handle->flags & OCFS_HANDLE_CHECKPOINT;
-	/* if you want to checkpoint, you MUST also sync. */
-	if (checkpoint)
-		ocfs_handle_set_sync(handle, 1);
 	sync = handle->flags & OCFS_HANDLE_SYNC;
 
-	LOG_TRACE_ARGS("Commiting %d blocks, checkpoint = %s, sync = %s\n", 
-		       handle->num_buffs, checkpoint ? "true" : "false",
+	LOG_TRACE_ARGS("Commiting %d blocks, sync = %s\n", handle->num_buffs, 
 		       sync ? "true" : "false");
 
 	if (sync)
@@ -662,30 +597,15 @@
 		BUG();
 	}
 
-	/* in the checkpoint case we num_trans as there's nothing for
-	 * the commit thread to do on our behalf. */
-	if (checkpoint)
-		atomic_dec(&(osb->journal->num_trans));
-	else {
-		ocfs_handle_move_locks(osb->journal, handle);
-		spin_lock(&osb->journal->cmt_lock);
-		osb->needs_flush = 1;
-		spin_unlock(&osb->journal->cmt_lock);
-	}
+	ocfs_handle_move_locks(osb->journal, handle);
+	spin_lock(&osb->journal->cmt_lock);
+	osb->needs_flush = 1;
+	spin_unlock(&osb->journal->cmt_lock);
+
 	up_read(&journal->trans_barrier);
 
 	handle->k_handle = NULL; /* it's been free'd in journal_stop */
 
-	/* In the future we'll try to queue up as many
-	 * buffer_heads as possible so that a single callback call
-	 * will checkpoint and revoke everything from that
-	 * transaction. */
-	if (checkpoint) {
-		retval = ocfs_journal_flush(journal);
-		if (retval < 0)
-			LOG_ERROR_STATUS(retval);
-	}
-
 	for(i = 0; i < handle->num_buffs; i++) {
 		brelse(handle->buffs[i]);
 		handle->buffs[i] = NULL;
@@ -703,12 +623,6 @@
 	handle->commit_bits = NULL;
 
 /* done: */
-	if (checkpoint) {
-		/* Release locks associated with this handle. */
-		retval = ocfs_handle_release_locks(handle);
-		if (retval < 0)
-			LOG_ERROR_STATUS(retval);
-	}
 
 	if (commit_head && (retval == 0))
 		ocfs_process_bitmap_free_head(osb, commit_head);
@@ -738,6 +652,9 @@
 
 	LOG_ENTRY();
 
+	OCFS_ASSERT(handle);
+	OCFS_ASSERT(!(handle->flags & OCFS_HANDLE_ALWAYS_COMMITS));
+
 	osb = handle->osb;
 	journal = osb->journal;
 
@@ -812,45 +729,27 @@
 	/* release inode semaphores we took during this transaction */
 	ocfs_handle_unlock_inodes(handle);
 
-	/* done copying them, free it now. */
-	ocfs_handle_free_all_copyout(handle);
-
-	/* want to force our handle to disk in abort case. */
-	handle->k_handle->h_sync = 1;
-
 	retval = journal_stop(handle->k_handle);
 	if (retval < 0) {
 		LOG_ERROR_STR("Could not commit aborted transaction!");
 		LOG_ERROR_STATUS(retval);
 	}
-	atomic_dec(&(osb->journal->num_trans));
 
+	ocfs_handle_move_locks(osb->journal, handle);
+	spin_lock(&osb->journal->cmt_lock);
+	osb->needs_flush = 1;
+	spin_unlock(&osb->journal->cmt_lock);
+
 	up_read(&journal->trans_barrier);
 
 	handle->k_handle = NULL;
 
-
 /* done: */
-	if (handle->num_buffs) {
-		/* Ok, we now want to fill our buffers with the older (but
-		 * valid) data, instead of leaving them with the aborted
-		 * data. To do so we want to first checkpoint the valid
-		 * transactions in the journal so that we know that disk
-		 * reflects the latest correct blocks. After that, we just
-		 * repopulate the buffers from disk. */
-		retval = ocfs_journal_flush(journal);
-		if (retval < 0)
-			LOG_ERROR_STATUS(retval);
-	}
+	ocfs_handle_free_all_copyout(handle);
 
 	for(i = 0; i < handle->num_buffs; i++)
 		brelse(handle->buffs[i]);
 
-	/* drop locks associated with the handle here. */
-	retval = ocfs_handle_release_locks(handle);
-	if (retval < 0)
-		LOG_ERROR_STATUS(retval);
-
 	/* Should only be processed in commit. */
 	ocfs_free_bitmap_free_head(handle->commit_bits);
 
@@ -886,6 +785,10 @@
 		BUG();
 	}
 
+	/* the copyout junk is only for abort... */
+	if (handle->flags & OCFS_HANDLE_ALWAYS_COMMITS)
+		goto skip_copyout;
+
 	/* search for this buffer in our copyout list. If it's already
 	 * there, we need to do nothing. Otherwise, add it to the
 	 * handle. 
@@ -946,6 +849,7 @@
 		}
 	}
 
+skip_copyout:
 	switch (type) {
 	case OCFS_JOURNAL_ACCESS_CREATE:
 	case OCFS_JOURNAL_ACCESS_WRITE:

Modified: trunk/src/namei.c
===================================================================
--- trunk/src/namei.c	2004-08-03 18:55:49 UTC (rev 1325)
+++ trunk/src/namei.c	2004-08-04 00:53:13 UTC (rev 1326)
@@ -229,9 +229,6 @@
 		goto leave;
 	}
 
-	ocfs_handle_set_checkpoint(handle, 0);
-	ocfs_handle_set_sync(handle, 0);
-
 	/* lock the parent directory */
 	status = ocfs_acquire_lock (osb, OCFS_LKM_EXMODE,
 				    FLAG_FILE_CREATE | FLAG_DIR, 
@@ -573,9 +570,6 @@
 		goto bail;
 	}
 
-	ocfs_handle_set_checkpoint(handle, 0);
-	ocfs_handle_set_sync(handle, 0);
-
 	down_write(&OCFS_I(dir)->ip_io_sem);
 
 	/* lock the parent directory */
@@ -702,8 +696,6 @@
 		LOG_ERROR_STATUS (status = -ENOMEM);
 		goto leave;
 	}
-	ocfs_handle_set_checkpoint(handle, 0);
-	ocfs_handle_set_sync(handle, 0);
 
 	/* lock parent directory, yes we use FLAG_FILE_CREATE even
 	 * though we're deleting ;) */
@@ -861,7 +853,6 @@
 			    struct inode *inode2)
 {
 	int status = 0;
-	int id2_locked = 0;
 	__u64 tmpid, id1, id2;
 	__u32 tmptype, tmpflags;
 	struct buffer_head **tmpbh;
@@ -871,6 +862,8 @@
 		       OCFS_I(inode1)->ip_blkno,
 		       OCFS_I(inode2)->ip_blkno);
 
+	OCFS_ASSERT(handle);
+
 	id1 = OCFS_I(inode1)->ip_blkno << inode1->i_sb->s_blocksize_bits;
 	id2 = OCFS_I(inode2)->ip_blkno << inode2->i_sb->s_blocksize_bits;
 
@@ -910,7 +903,7 @@
 			LOG_ERROR_STATUS (status);
 			goto bail;
 		}
-		id2_locked = 1;
+		ocfs_handle_add_lock(handle, type2, flags2, inode2);
 	}
 	/* lock id1 */
 	status = ocfs_acquire_lock(osb, type1, flags1, 
@@ -919,21 +912,9 @@
 		LOG_ERROR_STATUS (status);
 		goto bail;
 	}
+	ocfs_handle_add_lock(handle, type1, flags1, inode1);
 
 bail:
-	if (status < 0) {
-		if (id2_locked) {
-			status = ocfs_release_lock(osb, type2, flags2, inode2);
-			if (bh2) {
-				brelse(*bh2);
-				*bh2 = NULL;
-			}
-		}
-	} else if (handle) {
-		if (id2_locked)
-			ocfs_handle_add_lock(handle, type2, flags2, inode2);
-		ocfs_handle_add_lock(handle, type1, flags1, inode1);
-	}
 
 	LOG_EXIT_STATUS(status);
 	return(status);
@@ -1032,8 +1013,7 @@
 		LOG_ERROR_STATUS(status = -ENOMEM);
 		goto bail;
 	}
-	ocfs_handle_set_checkpoint(handle, 0);
-	ocfs_handle_set_sync(handle, 0);
+
 	/* if old and new are the same, this'll just do one lock. */
 	status = ocfs_double_lock(osb, handle, 
 				  OCFS_LKM_EXMODE, 
@@ -1480,9 +1460,6 @@
 		goto bail;
 	}
 
-	ocfs_handle_set_checkpoint(handle, 0);
-	ocfs_handle_set_sync(handle, 0);
-
 	/* lock the parent directory */
 	status = ocfs_acquire_lock(osb, OCFS_LKM_EXMODE,
 				   FLAG_FILE_CREATE | FLAG_DIR, 

Modified: trunk/src/ocfs_journal.h
===================================================================
--- trunk/src/ocfs_journal.h	2004-08-03 18:55:49 UTC (rev 1325)
+++ trunk/src/ocfs_journal.h	2004-08-04 00:53:13 UTC (rev 1326)
@@ -205,10 +205,13 @@
 	struct list_head     inode_list;
 };
 
-/* should we checkpoint this handle on commit? */
-#define OCFS_HANDLE_CHECKPOINT			1
 /* should we sync-commit this handle? */
-#define OCFS_HANDLE_SYNC			2
+#define OCFS_HANDLE_SYNC			1
+/* This is really the right way to do things, but until we fix all the
+ * code, it's a performance improvement for a handle which never
+ * aborts. Should be set before passing any buffers to
+ * journal_access! */
+#define OCFS_HANDLE_ALWAYS_COMMITS		2
 
 static inline void ocfs_handle_free_all_copyout(ocfs_journal_handle *handle)
 {
@@ -221,16 +224,6 @@
 	handle->co_buffs = NULL;
 }
 
-
-static inline void ocfs_handle_set_checkpoint(ocfs_journal_handle *handle,
-					      int checkpoint)
-{
-	if (checkpoint)
-		handle->flags |= OCFS_HANDLE_CHECKPOINT;
-	else
-		handle->flags &= ~OCFS_HANDLE_CHECKPOINT;
-}
-
 static inline void ocfs_handle_set_sync(ocfs_journal_handle *handle, int sync)
 {
 	if (sync)
@@ -239,6 +232,15 @@
 		handle->flags &= ~OCFS_HANDLE_SYNC;
 }
 
+static inline void ocfs_handle_set_always_commits(ocfs_journal_handle *handle, 
+						  int always)
+{
+	if (always)
+		handle->flags |= OCFS_HANDLE_ALWAYS_COMMITS;
+	else
+		handle->flags &= ~OCFS_HANDLE_ALWAYS_COMMITS;
+}
+
 static inline int ocfs_handle_add_commit_bits(ocfs_journal_handle *handle,
 					      __u32 len, __u32 fileoff,
 					      __u32 nodenum, __u32 type)



More information about the Ocfs2-commits mailing list