[OPW,kernel,v4,2/2] reiserfs: Migrate j_trans_start_time to jiffies
diff mbox

Message ID fa3b834518472a8ed057700ee907dd890887d464.1414718402.git.ruchandani.tina@gmail.com
State New, archived
Headers show

Commit Message

Tina Ruchandani Oct. 31, 2014, 3:02 a.m. UTC
Field j_trans_start_time inside struct reiserfs_journal in journal.c uses
time_t. On 32-bit systems, time_t will overflow in year 2038 and beyond.
This patches changes the field to use jiffies instead. jiffies are chosen
for performance reasons over alternatives (i.e. timespec64).Using jiffies
provides monotonic time, which is good here, since we only care about elapsed
time.

Transaction time is measured as (now - j_trans_start_time) and often compared
against j_max_trans_age/j_max_commit_age. These quantities are therefore also
changed to a jiffies representation internally. They continue to be printed/
written to disk in seconds.

Old way of doing things:
  - j_trans_start_time in seconds.
    (now - j_trans_start_time) compared with j_max_trans_age to know if a
    transaction is too old.
  - j_max_trans_age, j_max_commit_age, commit_max_age stored in seconds.
New way:
  - j_trans_start_time, j_max_commit_age and j_max_trans_age in jiffies.
    commit_max_age continues to be in seconds.
  - Initialization done using (seconds value) * HZ.
    e.g j_max_commit_age = JOURNAL_MAX_COMMIT_AGE * HZ;
  - External interfaces (i.e. print logs and #define-ed constants for these
    quantities) continue to be in seconds.

journal_read now displays the time taken for transaction replay in milliseconds.
This helps the cases where the transactions are few and time is reported as 0
seconds.

Signed-off-by: Tina Ruchandani <ruchandani.tina@gmail.com>
Suggested-by: Arnd Bergmann <arnd@arndb.de>
---
Changes in v4:
  - Add comment in reiserfs.h mentioning j_trans_start_time is
    now in jiffies.
  - Fix printing of journal->j_trans_start_time -  it should be printed
    in seconds.
  - Make it clear that commit_max_age stays in seconds.
Changes in v3:
  - v2 used jiffies with u64 data-type for j_trans_start_time.
  - v3 uses 'unsigned long' and that requires checking for overflow
    conditions. j_trans_start_time is compared against
    j_max_trans_age and j_max_commit_age. Therefore these too are
    converted to jiffies representation.
Changes in v2:
  - v1 used timespec64 as a replacement for time_t
    which is not acceptable for performance.

Signed-off-by: Tina Ruchandani <ruchandani.tina@gmail.com>
---
 fs/reiserfs/journal.c  | 55 +++++++++++++++++++++++++++++++++-----------------
 fs/reiserfs/procfs.c   |  4 ++--
 fs/reiserfs/reiserfs.h | 16 ++++++++-------
 fs/reiserfs/super.c    |  8 ++++----
 4 files changed, 52 insertions(+), 31 deletions(-)

Comments

Arnd Bergmann Oct. 31, 2014, 8:44 p.m. UTC | #1
On Thursday 30 October 2014 20:02:49 Tina Ruchandani wrote:
> @@ -367,7 +367,7 @@ static int show_journal(struct seq_file *m, void *unused)
>                    JF(j_bcount),
>                    JF(j_first_unflushed_offset),
>                    JF(j_last_flush_trans_id),
> -                  JF(j_trans_start_time),
> +                  JF(j_trans_start_time) / HZ,
>                    JF(j_list_bitmap_index),
>                    JF(j_must_wait),
>                    JF(j_next_full_flush),
> 

This is not a correct conversion: The jiffies value starts at boot time and
overflows after a month to a year depending on the value of HZ. You have
to convert it back into "real time", e.g. through reading the current seconds
value and subtracting the jiffies that have passed since j_trans_start_time.

Everything else in the patch looks ok now.

	Arnd

Patch
diff mbox

diff --git a/fs/reiserfs/journal.c b/fs/reiserfs/journal.c
index 2bcd6ab..16e881a 100644
--- a/fs/reiserfs/journal.c
+++ b/fs/reiserfs/journal.c
@@ -2377,7 +2377,8 @@  static int journal_read(struct super_block *sb)
 	struct reiserfs_journal_desc *desc;
 	unsigned int oldest_trans_id = 0;
 	unsigned int oldest_invalid_trans_id = 0;
-	time_t start;
+	unsigned long start;
+	unsigned long stop;
 	unsigned long oldest_start = 0;
 	unsigned long cur_dblock = 0;
 	unsigned long newest_mount_id = 9;
@@ -2392,7 +2393,7 @@  static int journal_read(struct super_block *sb)
 	cur_dblock = SB_ONDISK_JOURNAL_1st_BLOCK(sb);
 	reiserfs_info(sb, "checking transaction log (%s)\n",
 		      bdevname(journal->j_dev_bd, b));
-	start = get_seconds();
+	start = jiffies;
 
 	/*
 	 * step 1, read in the journal header block.  Check the transaction
@@ -2550,10 +2551,11 @@  start_log_replay:
 	reiserfs_debug(sb, REISERFS_DEBUG_CODE, "journal-1299: Setting "
 		       "newest_mount_id to %lu", journal->j_mount_id);
 	journal->j_first_unflushed_offset = journal->j_start;
+	stop = jiffies;
 	if (replay_count > 0) {
 		reiserfs_info(sb,
-			      "replayed %d transactions in %lu seconds\n",
-			      replay_count, get_seconds() - start);
+			      "replayed %d transactions in %lu milliseconds\n",
+			      replay_count, jiffies_to_msecs(stop-start));
 	}
 	/* needed to satisfy the locking in _update_journal_header_block */
 	reiserfs_write_lock(sb);
@@ -2709,7 +2711,7 @@  static int check_advise_trans_params(struct super_block *sb,
 		}
 		journal->j_trans_max = JOURNAL_TRANS_MAX_DEFAULT;
 		journal->j_max_batch = JOURNAL_MAX_BATCH_DEFAULT;
-		journal->j_max_commit_age = JOURNAL_MAX_COMMIT_AGE;
+		journal->j_max_commit_age = JOURNAL_MAX_COMMIT_AGE * HZ;
 	}
 	return 0;
 }
@@ -2807,7 +2809,12 @@  int journal_init(struct super_block *sb, const char *j_dev_name,
 	journal->j_max_batch = le32_to_cpu(jh->jh_journal.jp_journal_max_batch);
 	journal->j_max_commit_age =
 	    le32_to_cpu(jh->jh_journal.jp_journal_max_commit_age);
-	journal->j_max_trans_age = JOURNAL_MAX_TRANS_AGE;
+	if (journal->j_max_commit_age > ULONG_MAX / HZ)
+		journal->j_max_commit_age = ULONG_MAX;
+	else
+		journal->j_max_commit_age *= HZ;
+
+	journal->j_max_trans_age = JOURNAL_MAX_TRANS_AGE * HZ;
 
 	if (check_advise_trans_params(sb, journal) != 0)
 	        goto free_and_return;
@@ -2818,6 +2825,16 @@  int journal_init(struct super_block *sb, const char *j_dev_name,
 		journal->j_max_trans_age = commit_max_age;
 	}
 
+	/* Convert journal->j_max_commit_age and
+	 * journal->j_max_trans_age to jiffies now */
+	if (commit_max_age > ULONG_MAX / HZ) {
+		journal->j_max_commit_age = ULONG_MAX;
+		journal->j_max_trans_age = ULONG_MAX;
+	} else {
+		journal->j_max_commit_age *= HZ;
+		journal->j_max_trans_age *= HZ;
+	}
+
 	reiserfs_info(sb, "journal params: device %s, size %u, "
 		      "journal first block %u, max trans len %u, max batch %u, "
 		      "max commit age %u, max trans age %u\n",
@@ -2826,7 +2843,8 @@  int journal_init(struct super_block *sb, const char *j_dev_name,
 		      SB_ONDISK_JOURNAL_1st_BLOCK(sb),
 		      journal->j_trans_max,
 		      journal->j_max_batch,
-		      journal->j_max_commit_age, journal->j_max_trans_age);
+		      journal->j_max_commit_age / HZ,
+		      journal->j_max_trans_age / HZ);
 
 	brelse(bhjh);
 
@@ -2912,7 +2930,9 @@  int journal_transaction_should_end(struct reiserfs_transaction_handle *th,
 				   int new_alloc)
 {
 	struct reiserfs_journal *journal = SB_JOURNAL(th->t_super);
-	time_t now = get_seconds();
+	unsigned long now;
+
+	now = jiffies;
 	/* cannot restart while nested */
 	BUG_ON(!th->t_trans_id);
 	if (th->t_refcount > 1)
@@ -3021,7 +3041,7 @@  static int do_journal_begin_r(struct reiserfs_transaction_handle *th,
 			      struct super_block *sb, unsigned long nblocks,
 			      int join)
 {
-	time_t now = get_seconds();
+	unsigned long now;
 	unsigned int old_trans_id;
 	struct reiserfs_journal *journal = SB_JOURNAL(sb);
 	struct reiserfs_transaction_handle myth;
@@ -3054,7 +3074,7 @@  relock:
 		PROC_INFO_INC(sb, journal.journal_relock_writers);
 		goto relock;
 	}
-	now = get_seconds();
+	now = jiffies;
 
 	/*
 	 * if there is no room in the journal OR
@@ -3062,7 +3082,6 @@  relock:
 	 * wait for it to finish before beginning we don't sleep if there
 	 * aren't other writers
 	 */
-
 	if ((!join && journal->j_must_wait > 0) ||
 	    (!join
 	     && (journal->j_len_alloc + nblocks + 2) >= journal->j_max_batch)
@@ -3116,9 +3135,9 @@  relock:
 		goto relock;
 	}
 	/* we are the first writer, set trans_id */
-	if (journal->j_trans_start_time == 0) {
-		journal->j_trans_start_time = get_seconds();
-	}
+	if (journal->j_trans_start_time == 0)
+		journal->j_trans_start_time = jiffies;
+
 	atomic_inc(&journal->j_wcount);
 	journal->j_len_alloc += nblocks;
 	th->t_blocks_logged = 0;
@@ -3557,11 +3576,11 @@  static void flush_async_commits(struct work_struct *work)
  */
 void reiserfs_flush_old_commits(struct super_block *sb)
 {
-	time_t now;
+	unsigned long now;
 	struct reiserfs_transaction_handle th;
 	struct reiserfs_journal *journal = SB_JOURNAL(sb);
 
-	now = get_seconds();
+	now = jiffies;
 	/*
 	 * safety check so we don't flush while we are replaying the log during
 	 * mount
@@ -3611,7 +3630,7 @@  void reiserfs_flush_old_commits(struct super_block *sb)
 static int check_journal_end(struct reiserfs_transaction_handle *th, int flags)
 {
 
-	time_t now;
+	unsigned long now;
 	int flush = flags & FLUSH_ALL;
 	int commit_now = flags & COMMIT_NOW;
 	int wait_on_commit = flags & WAIT;
@@ -3692,7 +3711,7 @@  static int check_journal_end(struct reiserfs_transaction_handle *th, int flags)
 	}
 
 	/* deal with old transactions where we are the last writers */
-	now = get_seconds();
+	now = jiffies;
 	if ((now - journal->j_trans_start_time) > journal->j_max_trans_age) {
 		commit_now = 1;
 		journal->j_next_async_flush = 1;
diff --git a/fs/reiserfs/procfs.c b/fs/reiserfs/procfs.c
index 621b9f3..6f18e0a 100644
--- a/fs/reiserfs/procfs.c
+++ b/fs/reiserfs/procfs.c
@@ -354,7 +354,7 @@  static int show_journal(struct seq_file *m, void *unused)
 		   DJP(jp_journal_trans_max),
 		   DJP(jp_journal_magic),
 		   DJP(jp_journal_max_batch),
-		   SB_JOURNAL(sb)->j_max_commit_age,
+		   SB_JOURNAL(sb)->j_max_commit_age / HZ,
 		   DJP(jp_journal_max_trans_age),
 		   JF(j_1st_reserved_block),
 		   JF(j_state),
@@ -367,7 +367,7 @@  static int show_journal(struct seq_file *m, void *unused)
 		   JF(j_bcount),
 		   JF(j_first_unflushed_offset),
 		   JF(j_last_flush_trans_id),
-		   JF(j_trans_start_time),
+		   JF(j_trans_start_time) / HZ,
 		   JF(j_list_bitmap_index),
 		   JF(j_must_wait),
 		   JF(j_next_full_flush),
diff --git a/fs/reiserfs/reiserfs.h b/fs/reiserfs/reiserfs.h
index 8243af9..f4f5d4b 100644
--- a/fs/reiserfs/reiserfs.h
+++ b/fs/reiserfs/reiserfs.h
@@ -325,7 +325,9 @@  struct reiserfs_journal {
 
 	struct buffer_head *j_header_bh;
 
-	time_t j_trans_start_time;	/* time this transaction started */
+	/* in jiffies, time this transaction started */
+	unsigned long j_trans_start_time;
+
 	struct mutex j_mutex;
 	struct mutex j_flush_mutex;
 
@@ -353,14 +355,14 @@  struct reiserfs_journal {
 	/* max number of blocks to batch into a trans */
 	unsigned int j_max_batch;
 
-	/* in seconds, how old can an async commit be */
-	unsigned int j_max_commit_age;
+	/* in jiffies, how old can an async commit be */
+	unsigned long j_max_commit_age;
 
-	/* in seconds, how old can a transaction be */
-	unsigned int j_max_trans_age;
+	/* in jiffies, how old can a transaction be */
+	unsigned long j_max_trans_age;
 
-	/* the default for the max commit age */
-	unsigned int j_default_max_commit_age;
+	/* in jiffies, the default for the max commit age */
+	unsigned long j_default_max_commit_age;
 
 	struct reiserfs_journal_cnode *j_cnode_free_list;
 
diff --git a/fs/reiserfs/super.c b/fs/reiserfs/super.c
index f1376c9..b6161d1 100644
--- a/fs/reiserfs/super.c
+++ b/fs/reiserfs/super.c
@@ -713,7 +713,7 @@  static int reiserfs_show_options(struct seq_file *seq, struct dentry *root)
 		seq_printf(seq, ",jdev=%s", REISERFS_SB(s)->s_jdev);
 
 	if (journal->j_max_commit_age != journal->j_default_max_commit_age)
-		seq_printf(seq, ",commit=%d", journal->j_max_commit_age);
+		seq_printf(seq, ",commit=%d", journal->j_max_commit_age / HZ);
 
 #ifdef CONFIG_QUOTA
 	if (REISERFS_SB(s)->s_qf_names[USRQUOTA])
@@ -1440,12 +1440,12 @@  static int reiserfs_remount(struct super_block *s, int *mount_flags, char *arg)
 	     s_mount_opt & ~safe_mask) | (mount_options & safe_mask);
 
 	if (commit_max_age != 0 && commit_max_age != (unsigned int)-1) {
-		journal->j_max_commit_age = commit_max_age;
-		journal->j_max_trans_age = commit_max_age;
+		journal->j_max_commit_age = commit_max_age * HZ;
+		journal->j_max_trans_age = commit_max_age * HZ;
 	} else if (commit_max_age == 0) {
 		/* 0 means restore defaults. */
 		journal->j_max_commit_age = journal->j_default_max_commit_age;
-		journal->j_max_trans_age = JOURNAL_MAX_TRANS_AGE;
+		journal->j_max_trans_age = JOURNAL_MAX_TRANS_AGE * HZ;
 	}
 
 	if (blocks) {