[PATCH] blkparse: check smallest sequence read

This allows check_sequence() to know when a skip happened and
when it is ok to proceed. Should be the final fix, now we now
when a sequence is missing and we nicely limit the memory
consumed in presence of large traces and missing entries.
This change brings down memory consumption of a skippy 1.3
million trace here down from 130MiB to 15MiB.

diff --git a/TODO b/TODO
index 10017837c191f5727816b304d95863a087cb459a..70a5d2d7ba40d395b0225ac0e8e0188b9089b6f7 100644 (file)
--- a/TODO
+++ b/TODO
@@ -1,9 +1,4 @@
 TODO before relasing 1.0
 ------------------------
 
-- Need better logic in show_entries_rb() for knowing when it's ok
-  to have sequence out of order. For SMP traces, we can currently
-  get a huge backlog of entries in the rb tree. This eats memory
-  and slows things down.
-
 - Add full description of the output of blkparse

diff --git a/blkparse.c b/blkparse.c
index b0f45f0fde54fb2e944a9b26d890b5eee80c7e92..779b1b965163040f7afefce429e9de8056c3587d 100644 (file)
--- a/blkparse.c
+++ b/blkparse.c
@@ -158,7 +158,6 @@ struct trace {
        struct blk_io_trace *bit;
        struct rb_node rb_node;
        struct trace *next;
-       unsigned int skipped;
 };
 
 static struct rb_root rb_sort_root;
@@ -196,6 +195,7 @@ static char *output_name;
 
 static unsigned long long genesis_time;
 static unsigned long long last_allowed_time;
+static unsigned int smallest_seq_read;
 static unsigned long long stopwatch_start;     /* start from zero by default */
 static unsigned long long stopwatch_end = ULONG_LONG_MAX;      /* "infinity" */
 
@@ -203,9 +203,6 @@ static int per_process_stats;
 static int track_ios;
 static int ppi_hash_by_pid = 1;
 
-static unsigned long last_skipped;
-static unsigned int skip_runs;
-
 static unsigned int t_alloc_cache;
 static unsigned int bit_alloc_cache;
 
@@ -1285,6 +1282,9 @@ static int sort_entries(unsigned long long *youngest)
 
                if (trace_rb_insert_sort(t))
                        return -1;
+
+               if (bit->sequence < smallest_seq_read)
+                       smallest_seq_read = bit->sequence;
        }
 
        return 0;
@@ -1314,11 +1314,11 @@ static void put_trace(struct per_dev_info *pdi, struct trace *t)
        }
 }
 
-static int check_sequence(struct per_dev_info *pdi, struct trace *t, int force)
+static int check_sequence(struct per_dev_info *pdi, struct blk_io_trace *bit,
+                         int force)
 {
        unsigned long expected_sequence = pdi->last_sequence + 1;
-       struct blk_io_trace *bit = t->bit;
-       struct trace *__t;
+       struct trace *t;
        
        /*
         * first entry, always ok
@@ -1329,44 +1329,26 @@ static int check_sequence(struct per_dev_info *pdi, struct trace *t, int force)
        if (bit->sequence == expected_sequence)
                return 0;
 
-       if (bit->sequence < expected_sequence &&
-           bit->time > pdi->last_reported_time)
-               return 0;
-
        /*
-        * the wanted sequence is really there, continue
-        * because this means that the log time is earlier
-        * on the trace we have now
+        * we may not have seen that sequence yet. if we are not doing
+        * the final run, break and wait for more entries.
         */
-       __t = trace_rb_find_sort(pdi->dev, expected_sequence);
-       if (__t)
-               return 0;
+       if (expected_sequence < smallest_seq_read) {
+               t = trace_rb_find_last(pdi, expected_sequence);
+               if (!t)
+                       goto skip;
 
-       /*
-        * we already displayed this entry, it's ok to continue.
-        * kill old entry now
-        */
-       __t = trace_rb_find_last(pdi, expected_sequence);
-       if (__t) {
-               __put_trace_last(pdi, __t);
                return 0;
-       }
-
-       if (last_skipped == expected_sequence)
-               t->skipped++;
-
-       last_skipped = expected_sequence;
-
-       /*
-        * unless this is the last run, break and wait for more entries
-        */
-       if (!force && t->skipped <= skip_runs)
+       } else if (!force)
                return 1;
-
-       fprintf(stderr, "(%d,%d): skipping %lu -> %u\n", MAJOR(pdi->dev),
-                       MINOR(pdi->dev), pdi->last_sequence, bit->sequence);
-       pdi->skips++;
-       return 0;
+       else {
+skip:
+               fprintf(stderr, "(%d,%d): skipping %lu -> %u\n",
+                       MAJOR(pdi->dev), MINOR(pdi->dev),
+                       pdi->last_sequence, bit->sequence);
+               pdi->skips++;
+               return 0;
+       }
 }
 
 static void show_entries_rb(int force)
@@ -1399,7 +1381,7 @@ static void show_entries_rb(int force)
                        break;
                }
 
-               if (check_sequence(pdi, t, force))
+               if (check_sequence(pdi, bit, force))
                        break;
 
                if (!force && bit->time > last_allowed_time)
@@ -1514,8 +1496,6 @@ static int do_file(void)
        struct per_dev_info *pdi;
        int i, j, events, events_added;
 
-       skip_runs = 4;
-
        /*
         * first prepare all files for reading
         */
@@ -1556,6 +1536,7 @@ static int do_file(void)
 
                events_added = 0;
                last_allowed_time = -1ULL;
+               smallest_seq_read = -1U;
 
                for (i = 0; i < ndevices; i++) {
                        pdi = &devices[i];
@@ -1602,7 +1583,6 @@ static int do_stdin(void)
        unsigned long long youngest;
        int fd;
 
-       skip_runs = -1U;
        last_allowed_time = -1ULL;
        fd = dup(STDIN_FILENO);
        do {