More updates from Bernhard Fischer

- flag rrd_resize's old file with RRD_COPY
- cleanup error-handling pathes in rrd_update and fix a few typos in
  comments
- rrd_close(): implement printing mincore results for the rrd if
  DEBUG=2 was defined
- rrd_open(): madvise start addresses need to be page-aligned; implement
  alternative path to the fine-grained (i.e. exact) madvise by flagging
  just the first two pages as needed (see TWO_PAGES). Implement
  alternative path that records the last madvise()ed area to avoid
  redundant calls to madvise() on identical areas (due to
  page-alignment constraints) -- see CHECK_MADVISE_OVERLAPS. Implement
  path for USE_DIRECT_IO.
- configure: add check for O_DIRECT flag to open(2). Add option
  --enable-direct-io. Add _GNU_SOURCE to CFLAGS to silence warnings
  about chroot which is marked LEGACY since SUSv2 and is a non POSIX
  extension. Make checks for posix_fadvise() dependant on
  --disable-mmap, since we do not need fadvise for the mmap case.

git-svn-id: svn://svn.oetiker.ch/rrdtool/trunk/program@1088 a5681a0c-68f1-0310-ab6d-d61299d08faa

diff --git a/configure.ac b/configure.ac
index 8fa3c0075bcb9d23a0f212061aa1dcddb5ad1e91..dc197e1f9c05484d87d3d3f5587992b597d13036 100644 (file)
--- a/configure.ac
+++ b/configure.ac
@@ -101,8 +101,7 @@ AH_BOTTOM([
 # include <sys/stat.h>
 #endif
 
-/* enable posix_fadvise on linux */
-#if defined(HAVE_POSIX_FADVISE) && defined(HAVE_FCNTL_H)
+#ifdef HAVE_FCNTL_H
 #include <fcntl.h>
 #endif
 
@@ -299,10 +298,12 @@ AC_ARG_ENABLE([mmap],
 [  --disable-mmap          disable mmap in rrd_update, use seek+write instead],
 [],
 [enable_mmap=yes])
+
+dnl will most likely not work on compressed filesystems, i think.. *shrug*
 AC_ARG_ENABLE([direct-io],
-[  --enable-direct-io      enable O_DIRECT],
-[],
-[enable_direct_io=yes])
+[  --enable-direct-io      enable O_DIRECT if available],
+[enable_direct_io=yes],
+[])
 
  AC_ARG_ENABLE(pthread,[  --disable-pthread       disable multithread support],
 [],[enable_pthread=yes])
@@ -317,6 +318,9 @@ AC_PROG_CC
 AC_PROG_CPP
 AC_PROG_LIBTOOL
 
+dnl Try to detect/use GNU features
+CFLAGS="$CFLAGS -D_GNU_SOURCE"
+
 dnl which flags does the compiler support?
 if test "x$GCC" = "xyes"; then
   for flag in -fno-strict-aliasing -Wall -std=c99 -pedantic -Wundef -Wshadow -Wpointer-arith -Wcast-align -Wmissing-prototypes -Wmissing-declarations -Wnested-externs -Winline -W; do
@@ -342,6 +346,26 @@ AC_HEADER_STDC
 AC_HEADER_DIRENT
 AC_CHECK_HEADERS(features.h sys/stat.h sys/types.h fcntl.h locale.h fp_class.h malloc.h unistd.h ieeefp.h math.h sys/times.h sys/param.h sys/resource.h signal.h float.h stdio.h stdlib.h errno.h string.h ctype.h)
 
+if test "x$enable_direct_io" = "xyes"; then
+AC_CACHE_CHECK([for O_DIRECT flag to open(2)],rrd_cv_HAVE_OPEN_O_DIRECT,[
+AC_TRY_COMPILE([
+#ifdef HAVE_UNISTD_H
+#include <unistd.h>
+#endif
+#ifdef HAVE_FCNTL_H
+#include <fcntl.h>
+#endif],
+[int fd = open("/dev/null", O_DIRECT);],
+rrd_cv_HAVE_OPEN_O_DIRECT=yes,rrd_cv_HAVE_OPEN_O_DIRECT=no)])
+  if test "x$rrd_cv_HAVE_OPEN_O_DIRECT" = "xyes"; then
+    AC_DEFINE(USE_DIRECT_IO,1,[Whether the open(2) accepts O_DIRECT])
+  else
+    enable_direct_io="no"
+  fi 
+else
+  enable_direct_io="no"
+fi
+
 dnl Checks for typedefs, structures, and compiler characteristics.
 AC_C_CONST
 AC_HEADER_TIME
@@ -365,12 +389,10 @@ AC_C_BIGENDIAN
 dnl for each function found we get a definition in config.h 
 dnl of the form HAVE_FUNCTION
 
-AC_CHECK_FUNCS(tzset mbstowcs opendir readdir chdir chroot getuid setlocale strerror strerror_r snprintf vsnprintf fpclass class fp_class isnan memmove strchr mktime getrusage gettimeofday posix_fadvise)
+AC_CHECK_FUNCS(tzset mbstowcs opendir readdir chdir chroot getuid setlocale strerror strerror_r snprintf vsnprintf fpclass class fp_class isnan memmove strchr mktime getrusage gettimeofday)
 
 dnl Could use these to know if we need to provide a prototype
 dnl AC_CHECK_DECLS(fdatasync, [], [], [#include <unistd.h>])
-dnl AC_CHECK_DECLS(posix_fadvise, [], [], [#define _XOPEN_SOURCE 600
-dnl #include <fcntl.h>])
 
 dnl XXX: dunno about windows.. add AC_CHECK_FUNCS(munmap) there too?
 if test "x$enable_mmap" = "xyes"; then
@@ -403,14 +425,17 @@ if test "x$enable_mmap" = "xyes"; then
   if test "x$ac_cv_func_mmap" != "xyes";
   then
     AC_MSG_ERROR([--enable-mmap requested but mmap() was not detected])
+dnl enable_mmap="no"
   fi
 fi
 
+dnl use FD based I/O ?
+if test "x$enable_mmap" = "xno";then
+    AC_CHECK_DECLS(posix_fadvise, [], [], [#define _XOPEN_SOURCE 600
+#include <fcntl.h>])
+    AC_CHECK_FUNCS(posix_fadvise)
+fi
 
-dnl if test "x$enable_direct_io" = "xyes"; then
-dnl check for working O_DIRECT
-dnl fi
-AC_SUBST([USE_DIRECT_IO])
 
 CONFIGURE_PART(IEEE Math Checks)
  
@@ -826,7 +851,8 @@ echo
 echo "----------------------------------------------------------------"
 echo "Config is DONE!"
 echo
-echo "          With MMAP IO: $ac_cv_func_mmap_fixed_mapped"
+echo "          With MMAP IO: $enable_mmap"
+echo "          Use O_DIRECT: $enable_direct_io"
 echo "          Perl Modules: $COMP_PERL"
 echo "           Perl Binary: $PERL"
 echo "          Perl Version: $PERL_VERSION"

diff --git a/examples/perftest.pl.in b/examples/perftest.pl.in
index 312ca5781795096fac0ef329fff5db65f9152530..e1e9a0420ed26e9b9a6851658981dc06b0f25496 100755 (executable)
--- a/examples/perftest.pl.in
+++ b/examples/perftest.pl.in
@@ -189,163 +189,4 @@ sub main (){
 }
 
 main;
-use strict;
-use Time::HiRes qw(time);
-use RRDs;
-use IO::File;
-use Time::HiRes qw( usleep );
-
-sub create($$){
-  my $file = shift;
-  my $time = shift;
-  my $start = time; #since we loaded HiRes
-  RRDs::create  ( $file.".rrd", "-b$time", qw(
-                       -s300                        
-                       DS:in:GAUGE:400:U:U
-                       DS:out:GAUGE:400:U:U
-                       RRA:AVERAGE:0.5:1:600
-                       RRA:AVERAGE:0.5:6:600
-                       RRA:MAX:0.5:6:600
-                       RRA:AVERAGE:0.5:24:600
-                       RRA:MAX:0.5:24:600
-                       RRA:AVERAGE:0.5:144:600
-                       RRA:MAX:0.5:144:600
-               ));
-   my $total = time - $start;
-   my $error =  RRDs::error;
-   die $error if $error;
-   return $total;
-}
-
-sub update($$){
-  my $file = shift;
-  my $time = shift;
-  my $in = rand(1000);
-  my $out = rand(1000);
-  my $start = time;
-  my $ret = RRDs::updatev($file.".rrd", $time.":$in:$out");
-#  print join("",map {"  $_ " . $ret->{$_}."\n" } grep /AVERAGE.\[1\]/, sort keys %$ret)."\n** $time\n\n";
-  # sync updates to disk immediately  
-#  usleep(1) if (rand(3) <1 );
-  my $total = time - $start;
-  my $error =  RRDs::error;
-  die $error if $error;
-  return $total;
-}
-
-sub tune($){
-  my $file = shift;
-  my $start = time;
-  RRDs::tune ($file.".rrd", "-a","in:U","-a","out:U","-d","in:GAUGE","-d","out:GAUGE");
-  my $total = time - $start;
-  my $error =  RRDs::error;
-  die $error if $error;
-  return $total;
-}
-
-sub infofetch($){
-  my $file = shift;
-  my $start = time;
-  my $info = RRDs::info ($file.".rrd");
-  my $error =  RRDs::error;
-  die $error if $error;
-  my $lasttime =  $info->{last_update} - $info->{last_update} % $info->{step};           
-  my $fetch = RRDs::fetch ($file.".rrd",'AVERAGE','-s',$lasttime-1,'-e',$lasttime);
-  my $total = time - $start;
-  my $error =  RRDs::error;
-  die $error if $error;
-  return $total;
-}
-
-sub stddev ($$$){ #http://en.wikipedia.org/wiki/Standard_deviation
-  my $sum = shift;
-  my $squaresum = shift;
-  my $count = shift;
-  return sqrt( 1 / $count * ( $squaresum - $sum*$sum / $count ))
-}
-
-sub makerrds($$$$){
-    my $count = shift;
-    my $total = shift;
-    my $list = shift;
-    my $time = shift;
-    my @files;
-    for (1..$count){
-        my $id = sprintf ("%07d",$total);
-        $id =~ s/^(.)(.)(.)(.)(.)//;
-        push @$list, "$1/$2/$3/$4/$5/$id";    
-        -d "$1" or mkdir "$1";
-        -d "$1/$2" or mkdir "$1/$2";
-        -d "$1/$2/$3" or mkdir "$1/$2/$3";
-        -d "$1/$2/$3/$4" or mkdir "$1/$2/$3/$4";
-        -d "$1/$2/$3/$4/$5" or mkdir "$1/$2/$3/$4/$5";
-       push @files, $list->[$total];
-        create $list->[$total++],$time-2;
-       print STDERR ".";
-    }
-   for (@files){ 
-       my $fd = new IO::File("$_.rrd","r");
-       if (defined $fd) {
-           $fd->sync;
-           $fd->close;
-        } else {
-            warn "failed to sync $_\n";
-        }        
-    }
-    return $count;
-}
-    
-    
-sub main (){
-    mkdir "db-$$" or die $!;
-    chdir "db-$$";
-
-    my $step = 200000; # number of rrds to creat for every round
-    
-    my @path;
-    my $time=int(time);
 
-    my $tracksize = 0;
-    my $uppntr = 0;
-
-    
-    my %squaresum = ( cr => 0, up => 0 );
-    my %sum = ( cr => 0, up => 0 );
-    my %count =( cr => 0, up => 0 );
-
-    my $printtime = time;
-    while (1) {
-        # enhance the track
-           $time += 300;
-        $tracksize += makerrds $step,$tracksize,\@path,$time;            
-        # run benchmark
-        for (0..10){
-           $time += 300;
-            my $count = 0;
-            my $sum = 0;
-            my $squaresum = 0;
-            for (my $i = 0; $i<$tracksize;$i ++){
-               my $elapsed = update($path[$i],$time); 
-               $sum += $elapsed;
-               $squaresum += $elapsed**2;
-               $count++;
-            };
-#            for (my $i = 0; $i<$tracksize;$i ++){
-#             my $fh = new IO::File "$path[$i].rrd","r";
-#             if (defined $fh) {
-#                 $fh->sync;
-#                 $fh->close;
-#                } else {
-#                 warn "failed to sync $path[$i]\n";
-#              }       
-#            }
-            my $ups = $count/$sum;
-            my $sdv = stddev($sum,$squaresum,$count);
-            printf STDERR "%4d %6.0f Up/s (%6.5f sdv)\n",$count,$ups,$sdv;
-        }
-       print STDERR "\n";
-       exit ;
-    }
-}
-
-main;

diff --git a/src/rrd_hw.c b/src/rrd_hw.c
index e8494161913a3bec26cfe182a0244bbf7a28de0c..d9247c9ecafb7c07271c712978217906be19ce55 100644 (file)
--- a/src/rrd_hw.c
+++ b/src/rrd_hw.c
@@ -818,7 +818,6 @@ void reset_aberrant_coefficients(
         != (ssize_t) (sizeof(cdp_prep_t) * (rrd->stat_head->rra_cnt) *
                       (rrd->stat_head->ds_cnt))) {
         rrd_set_error("reset_aberrant_coefficients: cdp_prep write failed");
-        return;         /*XXX: delme */
     }
 }
 

diff --git a/src/rrd_last.c b/src/rrd_last.c
index 99ff12517f83d142523c5ed86e8bc0f9d65c7d3f..42a807597de58d435a35948b1450fbc1a0d993aa 100644 (file)
--- a/src/rrd_last.c
+++ b/src/rrd_last.c
@@ -35,6 +35,7 @@ time_t rrd_last_r(
 
     lastup = rrd.live_head->last_up;
     rrd_free(&rrd);
+    close(rrd_file->fd);
     rrd_close(rrd_file);
     return (lastup);
 }

diff --git a/src/rrd_lastupdate.c b/src/rrd_lastupdate.c
index 47ecec75be57f50e01016d738c7e0c00069955fe..79274b55c929c727e27cd5d381b507498618fcd9 100644 (file)
--- a/src/rrd_lastupdate.c
+++ b/src/rrd_lastupdate.c
@@ -54,6 +54,7 @@ int rrd_lastupdate(
     }
 
     rrd_free(&rrd);
+    close(rrd_file->fd);
     rrd_close(rrd_file);
     return (0);
 }

diff --git a/src/rrd_open.c b/src/rrd_open.c
index baeb00355308de85f3a83e6a54c25947ff08fde9..c986f750beca7bec9fb3f0cba5c5a488cfda17d2 100644 (file)
--- a/src/rrd_open.c
+++ b/src/rrd_open.c
@@ -66,6 +66,15 @@
 #include "unused.h"
 #define MEMBLK 8192
 
+/* DEBUG 2 prints information obtained via mincore(2) */
+// #define DEBUG 2
+/* do not calculate exact madvise hints but assume 2 pages for headers and
+ * set DONTNEED for the rest, which is assumed to be data */
+//#define TWO_PAGES 1
+/* Avoid calling madvise on areas that were already hinted. May be benefical if
+ * your syscalls are very slow */
+//#define CHECK_MADVISE_OVERLAPS 1
+
 #ifdef HAVE_MMAP
 #define __rrd_read(dst, dst_t, cnt) \
        (dst) = (dst_t*) (data + offset); \
@@ -79,6 +88,35 @@
        offset += read (rrd_file->fd, dst, sizeof(dst_t)*(cnt))
 #endif
 
+/* next page-aligned (i.e. page-align up) */
+#ifndef PAGE_ALIGN
+#define PAGE_ALIGN(addr) (((addr)+_page_size-1)&(~(_page_size-1)))
+#endif
+/* previous page-aligned (i.e. page-align down) */
+#ifndef PAGE_ALIGN_DOWN
+#define PAGE_ALIGN_DOWN(addr) (((addr)+_page_size-1)&(~(_page_size-1)))
+#endif
+
+#ifdef HAVE_MMAP
+/* vector of last madvise hint */
+typedef struct _madvise_vec_t {
+    void     *start;
+    ssize_t   length;
+} _madvise_vec_t;
+_madvise_vec_t _madv_vec = { NULL, 0 };
+#endif
+
+#if defined CHECK_MADVISE_OVERLAPS
+#define _madvise(_start, _off, _hint) \
+    if ((_start) != _madv_vec.start && (ssize_t)(_off) != _madv_vec.length) { \
+        _madv_vec.start = (_start) ; _madv_vec.length = (_off); \
+        madvise((_start), (_off), (_hint)); \
+    }
+#else
+#define _madvise(_start, _off, _hint) \
+    madvise((_start), (_off), (_hint))
+#endif
+
 /* open a database file, return its header and an open filehandle */
 /* positioned to the first cdp in the first rra */
 
@@ -92,6 +130,7 @@ rrd_file_t *rrd_open(
     int       version;
 
 #ifdef HAVE_MMAP
+    ssize_t   _page_size = sysconf(_SC_PAGESIZE);
     int       mm_prot = PROT_READ, mm_flags = 0;
     char     *data;
 #endif
@@ -118,9 +157,8 @@ rrd_file_t *rrd_open(
 #ifdef HAVE_MMAP
         mm_flags = MAP_PRIVATE;
 # ifdef MAP_NORESERVE
-        mm_flags |= MAP_NORESERVE;
+        mm_flags |= MAP_NORESERVE;  /* readonly, so no swap backing needed */
 # endif
-        mm_flags |= MAP_PRIVATE;
 #endif
     } else {
         if (rdwr & RRD_READWRITE) {
@@ -137,19 +175,19 @@ rrd_file_t *rrd_open(
     }
     if (rdwr & RRD_READAHEAD) {
 #ifdef MAP_POPULATE
-        mm_flags |= MAP_POPULATE;
+        mm_flags |= MAP_POPULATE;   /* populate ptes and data */
 #endif
-#if defined MAP_NONBLOCK && !defined USE_DIRECT_IO
-        mm_flags |= MAP_NONBLOCK;   /* just populage ptes */
+#if defined MAP_NONBLOCK
+        mm_flags |= MAP_NONBLOCK;   /* just populate ptes */
 #endif
-    } else {
 #ifdef USE_DIRECT_IO
+    } else {
         flags |= O_DIRECT;
-#endif
-#if 0                   //def O_NONBLOCK
-        flags |= O_NONBLOCK;
 #endif
     }
+#ifdef O_NONBLOCK
+    flags |= O_NONBLOCK;
+#endif
 
     if ((rrd_file->fd = open(file_name, flags, mode)) < 0) {
         rrd_set_error("opening '%s': %s", file_name, rrd_strerror(errno));
@@ -196,21 +234,32 @@ rrd_file_t *rrd_open(
         goto out_close;
     }
     rrd_file->file_start = data;
-#else
 #endif
 #ifdef USE_MADVISE
-    if (rdwr & RRD_COPY) {  /*XXX: currently not used! */
+    if (rdwr & RRD_COPY) {
         /* We will read everything in a moment (copying) */
-        madvise(data, rrd_file->file_len, MADV_WILLNEED | MADV_SEQUENTIAL);
+        _madvise(data, rrd_file->file_len, MADV_WILLNEED | MADV_SEQUENTIAL);
         goto out_done;
     }
     /* We do not need to read anything in for the moment */
-    madvise(data, rrd_file->file_len, MADV_DONTNEED);
+#ifndef TWO_PAGES
+    _madvise(data, rrd_file->file_len, MADV_DONTNEED);
+//    _madvise(data, rrd_file->file_len, MADV_RANDOM);
+#else
+/* alternatively: keep 2 pages worth of data, likely headers,
+ * don't need the rest.  */
+    _madvise(data, _page_size * 2, MADV_WILLNEED | MADV_SEQUENTIAL);
+    _madvise(data + _page_size * 2, (rrd_file->file_len >= _page_size * 2)
+             ? rrd_file->file_len - _page_size * 2 : 0, MADV_DONTNEED);
+#endif
 #endif
 
-#ifdef USE_MADVISE
+#if defined USE_MADVISE && !defined TWO_PAGES
     /* the stat_head will be needed soonish, so hint accordingly */
-    madvise(data + offset, sizeof(stat_head_t), MADV_WILLNEED);
+// too finegrained to calc the individual sizes, just keep 2 pages worth of hdr
+    _madvise(data + PAGE_ALIGN_DOWN(offset), PAGE_ALIGN(sizeof(stat_head_t)),
+             MADV_WILLNEED);
+
 #endif
 
     __rrd_read(rrd->stat_head, stat_head_t,
@@ -234,18 +283,20 @@ rrd_file_t *rrd_open(
                       rrd->stat_head->version);
         goto out_nullify_head;
     }
-#ifdef USE_MADVISE
+#if defined USE_MADVISE && !defined TWO_PAGES
     /* the ds_def will be needed soonish, so hint accordingly */
-    madvise(data + offset, sizeof(ds_def_t) * rrd->stat_head->ds_cnt,
-            MADV_WILLNEED);
+    _madvise(data + PAGE_ALIGN_DOWN(offset),
+             PAGE_ALIGN(sizeof(ds_def_t) * rrd->stat_head->ds_cnt),
+             MADV_WILLNEED);
 #endif
     __rrd_read(rrd->ds_def, ds_def_t,
                rrd->stat_head->ds_cnt);
 
-#ifdef USE_MADVISE
+#if defined USE_MADVISE && !defined TWO_PAGES
     /* the rra_def will be needed soonish, so hint accordingly */
-    madvise(data + offset, sizeof(rra_def_t) * rrd->stat_head->rra_cnt,
-            MADV_WILLNEED);
+    _madvise(data + PAGE_ALIGN_DOWN(offset),
+             PAGE_ALIGN(sizeof(rra_def_t) * rrd->stat_head->rra_cnt),
+             MADV_WILLNEED);
 #endif
     __rrd_read(rrd->rra_def, rra_def_t,
                rrd->stat_head->rra_cnt);
@@ -265,9 +316,10 @@ rrd_file_t *rrd_open(
 #endif
         rrd->live_head->last_up_usec = 0;
     } else {
-#ifdef USE_MADVISE
+#if defined USE_MADVISE && !defined TWO_PAGES
         /* the live_head will be needed soonish, so hint accordingly */
-        madvise(data + offset, sizeof(live_head_t), MADV_WILLNEED);
+        _madvise(data + PAGE_ALIGN_DOWN(offset),
+                 PAGE_ALIGN(sizeof(live_head_t)), MADV_WILLNEED);
 #endif
         __rrd_read(rrd->live_head, live_head_t,
                    1);
@@ -289,7 +341,7 @@ rrd_file_t *rrd_open(
 #endif
     rrd_file->header_len = offset;
     rrd_file->pos = offset;
-/* we could close(rrd_file->fd); here, the mapping is still valid anyway */
+
     return (rrd_file);
   out_nullify_head:
     rrd->stat_head = NULL;
@@ -298,26 +350,86 @@ rrd_file_t *rrd_open(
     return NULL;
 }
 
+
 /* Close a reference to an rrd_file.  */
+
 int rrd_close(
     rrd_file_t *rrd_file)
 {
     int       ret;
 
+#if defined HAVE_MMAP
+    ssize_t   _page_size = sysconf(_SC_PAGESIZE);
+#endif
+#if defined DEBUG && DEBUG > 1
+    /* pretty print blocks in core */
+    off_t     off;
+    unsigned char *vec;
+
+    off =
+        rrd_file->file_len +
+        ((rrd_file->file_len + sysconf(_SC_PAGESIZE) -
+          1) / sysconf(_SC_PAGESIZE));
+    vec = malloc(off);
+    if (vec != NULL) {
+        memset(vec, 0, off);
+        if (mincore(rrd_file->file_start, rrd_file->file_len, vec) == 0) {
+            int       prev;
+            unsigned  is_in = 0, was_in = 0;
+
+            for (off = 0, prev = 0; off < rrd_file->file_len; ++off) {
+                is_in = vec[off] & 1;   /* if lsb set then is core resident */
+                if (off == 0)
+                    was_in = is_in;
+                if (was_in != is_in) {
+                    fprintf(stderr, "%sin core: %p len %ld\n",
+                            was_in ? "" : "not ", vec + prev, off - prev);
+                    was_in = is_in;
+                    prev = off;
+                }
+            }
+            fprintf(stderr,
+                    "%sin core: %p len %ld\n",
+                    was_in ? "" : "not ", vec + prev, off - prev);
+        } else
+            fprintf(stderr, "mincore: %s", rrd_strerror(errno));
+    }
+#endif                          /* DEBUG */
+
+#ifdef USE_MADVISE
+#ifdef TWO_PAGES
+//XXX: ?
+    /* Keep 2 pages worth of headers around, round up to next page boundary.  */
+    ret =
+        PAGE_ALIGN(rrd_file->header_len % _page_size + rrd_file->header_len);
+    if (rrd_file->file_len > ret)
+        _madvise(rrd_file->file_start + ret,
+                 rrd_file->file_len - ret, MADV_DONTNEED);
+#else
+    /* ignoring errors from RRDs that are smaller then the file_len+rounding */
+    _madvise(rrd_file->file_start + PAGE_ALIGN_DOWN(rrd_file->header_len),
+             rrd_file->file_len - PAGE_ALIGN(rrd_file->header_len),
+             MADV_DONTNEED);
+#endif
+#endif
 #ifdef HAVE_MMAP
     ret = munmap(rrd_file->file_start, rrd_file->file_len);
     if (ret != 0)
         rrd_set_error("munmap rrd_file: %s", rrd_strerror(errno));
+#else
+    ret = 0;
 #endif
-    ret = close(rrd_file->fd);
-    if (ret != 0)
-        rrd_set_error("closing file: %s", rrd_strerror(errno));
+//    ret = close(rrd_file->fd);
+//    if (ret != 0)
+//        rrd_set_error("closing file: %s", rrd_strerror(errno));
     free(rrd_file);
     rrd_file = NULL;
     return ret;
 }
 
+
 /* Set position of rrd_file.  */
+
 off_t rrd_seek(
     rrd_file_t *rrd_file,
     off_t off,
@@ -342,24 +454,25 @@ off_t rrd_seek(
     return ret == -1;   //XXX: or just ret to mimic lseek
 }
 
+
 /* Get current position in rrd_file.  */
-inline off_t rrd_tell(
-    rrd_file_t *rrd_file)
+
+inline off_t rrd_tell(rrd_file_t *rrd_file)
 {
     return rrd_file->pos;
 }
 
+
 /* read count bytes into buffer buf, starting at rrd_file->pos.
  * Returns the number of bytes read.  */
+
 ssize_t rrd_read(
     rrd_file_t *rrd_file,
     void *buf,
     size_t count)
 {
 #ifdef HAVE_MMAP
-    char     *pos = rrd_file->file_start + rrd_file->pos;
-
-    buf = memmove(buf, pos, count);
+    buf = memmove(buf, rrd_file->file_start + rrd_file->pos, count);
     rrd_file->pos += count; /* mimmic read() semantics */
     return count;
 #else
@@ -372,9 +485,11 @@ ssize_t rrd_read(
 #endif
 }
 
+
 /* write count bytes from buffer buf to the current position
  * rrd_file->pos of rrd_file->fd.
  * Returns the number of bytes written.  */
+
 ssize_t rrd_write(
     rrd_file_t *rrd_file,
     const void *buf,
@@ -388,7 +503,9 @@ ssize_t rrd_write(
 #endif
 }
 
+
 /* flush all data pending to be written to FD.  */
+
 inline void rrd_flush(
     rrd_file_t *rrd_file)
 {
@@ -398,6 +515,9 @@ inline void rrd_flush(
     }
 }
 
+
+/* Initialize RRD header.  */
+
 void rrd_init(
     rrd_t *rrd)
 {
@@ -411,6 +531,9 @@ void rrd_init(
     rrd->rrd_value = NULL;
 }
 
+
+/* free RRD header data.  */
+
 void rrd_free(
     rrd_t UNUSED(*rrd))
 {
@@ -424,19 +547,24 @@ void rrd_free(
     free(rrd->pdp_prep);
     free(rrd->cdp_prep);
     free(rrd->rrd_value);
-//XXX: ? rrd_init(rrd);
 #endif
 }
 
+
 /* routine used by external libraries to free memory allocated by
  * rrd library */
+
 void rrd_freemem(
     void *mem)
 {
     free(mem);
 }
 
-int readfile(
+
+/* XXX: FIXME: missing documentation.  */
+/*XXX: FIXME should be renamed to rrd_readfile or _rrd_readfile */
+
+int /*_rrd_*/ readfile(
     const char *file_name,
     char **buffer,
     int skipfirst)

diff --git a/src/rrd_resize.c b/src/rrd_resize.c
index f64615cf62bfb39e0927de16e8e23cd35528701b..11efcdaac7617f3fffa4cc768c48be29dfb73b60 100644 (file)
--- a/src/rrd_resize.c
+++ b/src/rrd_resize.c
@@ -55,7 +55,7 @@ int rrd_resize(
         modify = -modify;
 
 
-    rrd_file = rrd_open(infilename, &rrdold, RRD_READWRITE);
+    rrd_file = rrd_open(infilename, &rrdold, RRD_READWRITE | RRD_COPY);
     if (rrd_file == NULL) {
         rrd_set_error("could not open RRD");
         return (-1);
@@ -111,15 +111,14 @@ int rrd_resize(
     case 1:
         rrdold.stat_head->version[3] = '3';
         break;
-    default:{
+    default:
         rrd_set_error("Do not know how to handle RRD version %s",
                       rrdold.stat_head->version);
+        rrd_close(rrd_file);
         rrd_free(&rrdold);
-        close(rrd_file->fd);
         return (-1);
+        break;
     }
-    }
-
 
 /* XXX: Error checking? */
     rrd_write(rrd_out_file, rrdnew.stat_head, sizeof(stat_head_t) * 1);
@@ -231,8 +230,13 @@ int rrd_resize(
     rrd_write(rrd_out_file, rrdnew.rra_ptr,
               sizeof(rra_ptr_t) * rrdnew.stat_head->rra_cnt);
 
-    close(rrd_out_file->fd);
     rrd_free(&rrdold);
     close(rrd_file->fd);
+    rrd_close(rrd_file);
+
+    rrd_free(&rrdnew);
+    close(rrd_out_file->fd);
+    rrd_close(rrd_out_file);
+
     return (0);
 }

diff --git a/src/rrd_tool.c b/src/rrd_tool.c
index ba561ce52cabbf062589b7facec130b41c461d18..ee561d668293956d20eeb85a710fb8e2111b9d23 100644 (file)
--- a/src/rrd_tool.c
+++ b/src/rrd_tool.c
@@ -432,8 +432,6 @@ int main(
 
                 free(myargv);
                 if (ret == 0) {
-
-
 #if HAVE_GETRUSAGE
                     getrusage(RUSAGE_SELF, &myusage);
                     gettimeofday(&currenttime, NULL);

diff --git a/src/rrd_tune.c b/src/rrd_tune.c
index df22968378fa15da2aa7abd6142efcb0616a715d..680e5587e7d75e27ff631f795f2dc8c34bb708f6 100644 (file)
--- a/src/rrd_tune.c
+++ b/src/rrd_tune.c
@@ -289,7 +289,6 @@ int rrd_tune(
         }
     }
     if (optcnt > 0) {
-
         rrd_seek(rrd_file, 0, SEEK_SET);
         rrd_write(rrd_file, rrd.stat_head, sizeof(stat_head_t) * 1);
         rrd_write(rrd_file, rrd.ds_def,

diff --git a/src/rrd_update.c b/src/rrd_update.c
index eb08f78af02231c4512eb756ee3d9a6aab931625..6ef5d3fe4ed96728e8a9da76001015de6f2942ef 100644 (file)
--- a/src/rrd_update.c
+++ b/src/rrd_update.c
@@ -56,10 +56,10 @@ static int gettimeofday(
 
 #endif
 /*
- * normilize time as returned by gettimeofday. usec part must
+ * normalize time as returned by gettimeofday. usec part must
  * be always >= 0
  */
-static void normalize_time(
+static inline void normalize_time(
     struct timeval *t)
 {
     if (t->tv_usec < 0) {
@@ -271,7 +271,7 @@ int _rrd_update(
     rrd_value_t *pdp_new;   /* prepare the incoming data
                              * to be added the the
                              * existing entry */
-    rrd_value_t *pdp_temp;  /* prepare the pdp values 
+    rrd_value_t *pdp_temp;  /* prepare the pdp values
                              * to be added the the
                              * cdp values */
 
@@ -318,12 +318,12 @@ int _rrd_update(
     /* need at least 1 arguments: data. */
     if (argc < 1) {
         rrd_set_error("Not enough arguments");
-        return -1;
+        goto err_out;
     }
 
     rrd_file = rrd_open(filename, &rrd, RRD_READWRITE);
     if (rrd_file == NULL) {
-        return -1;
+        goto err_out;
     }
 
     /* initialize time */
@@ -362,36 +362,25 @@ int _rrd_update(
      */
     if (LockRRD(rrd_file->fd) != 0) {
         rrd_set_error("could not lock RRD");
-        rrd_free(&rrd);
-        close(rrd_file->fd);
-        return (-1);
+        goto err_close;
     }
 
     if ((updvals =
          malloc(sizeof(char *) * (rrd.stat_head->ds_cnt + 1))) == NULL) {
         rrd_set_error("allocating updvals pointer array");
-        rrd_free(&rrd);
-        close(rrd_file->fd);
-        return (-1);
+        goto err_close;
     }
 
     if ((pdp_temp = malloc(sizeof(rrd_value_t)
                            * rrd.stat_head->ds_cnt)) == NULL) {
         rrd_set_error("allocating pdp_temp ...");
-        free(updvals);
-        rrd_free(&rrd);
-        close(rrd_file->fd);
-        return (-1);
+        goto err_free_updvals;
     }
 
     if ((tmpl_idx = malloc(sizeof(unsigned long)
                            * (rrd.stat_head->ds_cnt + 1))) == NULL) {
         rrd_set_error("allocating tmpl_idx ...");
-        free(pdp_temp);
-        free(updvals);
-        rrd_free(&rrd);
-        close(rrd_file->fd);
-        return (-1);
+        goto err_free_pdp_temp;
     }
     /* initialize tmplt redirector */
     /* default config example (assume DS 1 is a CDEF DS)
@@ -421,22 +410,11 @@ int _rrd_update(
                 if (tmpl_cnt > rrd.stat_head->ds_cnt) {
                     rrd_set_error
                         ("tmplt contains more DS definitions than RRD");
-                    free(updvals);
-                    free(pdp_temp);
-                    free(tmpl_idx);
-                    rrd_free(&rrd);
-                    close(rrd_file->fd);
-                    return (-1);
+                    goto err_free_tmpl_idx;
                 }
                 if ((tmpl_idx[tmpl_cnt++] = ds_match(&rrd, dsname)) == -1) {
                     rrd_set_error("unknown DS name '%s'", dsname);
-                    free(updvals);
-                    free(pdp_temp);
-                    free(tmplt_copy);
-                    free(tmpl_idx);
-                    rrd_free(&rrd);
-                    close(rrd_file->fd);
-                    return (-1);
+                    goto err_free_tmpl_idx;
                 } else {
                     /* the first element is always the time */
                     tmpl_idx[tmpl_cnt - 1]++;
@@ -455,12 +433,7 @@ int _rrd_update(
     if ((pdp_new = malloc(sizeof(rrd_value_t)
                           * rrd.stat_head->ds_cnt)) == NULL) {
         rrd_set_error("allocating pdp_new ...");
-        free(updvals);
-        free(pdp_temp);
-        free(tmpl_idx);
-        rrd_free(&rrd);
-        close(rrd_file->fd);
-        return (-1);
+        goto err_free_tmpl_idx;
     }
 #if 0                   //def HAVE_MMAP
     rrd_mmaped_file = mmap(0,
@@ -494,20 +467,11 @@ int _rrd_update(
         if (stepper == NULL) {
             rrd_set_error("failed duplication argv entry");
             free(step_start);
-            free(updvals);
-            free(pdp_temp);
-            free(tmpl_idx);
-            rrd_free(&rrd);
-#ifdef HAVE_MMAP
-            rrd_close(rrd_file);
-#endif
-            close(rrd_file->fd);
-            return (-1);
+            goto err_free_pdp_new;
         }
         /* initialize all ds input to unknown except the first one
            which has always got to be set */
-        for (ii = 1; ii <= rrd.stat_head->ds_cnt; ii++)
-            updvals[ii] = "U";
+        memset(updvals + 1, 'U', rrd.stat_head->ds_cnt);
         updvals[0] = stepper;
         /* separate all ds elements; first must be examined separately
            due to alternate time syntax */
@@ -660,7 +624,7 @@ int _rrd_update(
 
             /* NOTE: DST_CDEF should never enter this if block, because
              * updvals[i+1][0] is initialized to 'U'; unless the caller
-             * accidently specified a value for the DST_CDEF. To handle 
+             * accidently specified a value for the DST_CDEF. To handle
              * this case, an extra check is required. */
 
             if ((updvals[i + 1][0] != 'U') &&
@@ -672,8 +636,6 @@ int _rrd_update(
                 /* pdp_new contains rate * time ... eg the bytes
                  * transferred during the interval. Doing it this way saves
                  * a lot of math operations */
-
-
                 switch (dst_idx) {
                 case DST_COUNTER:
                 case DST_DERIVE:
@@ -715,7 +677,7 @@ int _rrd_update(
                     errno = 0;
                     pdp_new[i] = strtod(updvals[i + 1], &endptr);
                     if (errno > 0) {
-                        rrd_set_error("converting  '%s' to float: %s",
+                        rrd_set_error("converting '%s' to float: %s",
                                       updvals[i + 1], rrd_strerror(errno));
                         break;
                     };
@@ -731,7 +693,7 @@ int _rrd_update(
                     errno = 0;
                     pdp_new[i] = strtod(updvals[i + 1], &endptr) * interval;
                     if (errno > 0) {
-                        rrd_set_error("converting  '%s' to float: %s",
+                        rrd_set_error("converting '%s' to float: %s",
                                       updvals[i + 1], rrd_strerror(errno));
                         break;
                     };
@@ -817,21 +779,21 @@ int _rrd_update(
         } else {
             /* an pdp_st has occurred. */
 
-            /* in pdp_prep[].scratch[PDP_val].u_val we have collected rate*seconds which 
-             * occurred up to the last run.        
-             pdp_new[] contains rate*seconds from the latest run.
-             pdp_temp[] will contain the rate for cdp */
+            /* in pdp_prep[].scratch[PDP_val].u_val we have collected
+               rate*seconds which occurred up to the last run.
+               pdp_new[] contains rate*seconds from the latest run.
+               pdp_temp[] will contain the rate for cdp */
 
             for (i = 0; i < rrd.stat_head->ds_cnt; i++) {
                 /* update pdp_prep to the current pdp_st. */
                 double    pre_unknown = 0.0;
 
-                if (isnan(pdp_new[i]))
+                if (isnan(pdp_new[i])) {
                     /* a final bit of unkonwn to be added bevore calculation
-                     * we use a tempaorary variable for this so that we 
-                     * don't have to turn integer lines before using the value */
+                       we use a temporary variable for this so that we
+                       don't have to turn integer lines before using the value */
                     pre_unknown = pre_int;
-                else {
+                } else {
                     if (isnan(rrd.pdp_prep[i].scratch[PDP_val].u_val)) {
                         rrd.pdp_prep[i].scratch[PDP_val].u_val =
                             pdp_new[i] / interval * pre_int;
@@ -844,9 +806,9 @@ int _rrd_update(
 
                 /* if too much of the pdp_prep is unknown we dump it */
                 if (
-                       /* removed because this does not agree with the definition
-                          a heart beat can be unknown */
-                       /* (rrd.pdp_prep[i].scratch[PDP_unkn_sec_cnt].u_cnt 
+                       /* removed because this does not agree with the
+                          definition that a heartbeat can be unknown */
+                       /* (rrd.pdp_prep[i].scratch[PDP_unkn_sec_cnt].u_cnt
                           > rrd.ds_def[i].par[DS_mrhb_cnt].u_cnt) || */
                        /* if the interval is larger thatn mrhb we get NAN */
                        (interval > rrd.ds_def[i].par[DS_mrhb_cnt].u_cnt) ||
@@ -944,16 +906,17 @@ int _rrd_update(
                 }
 
                 if (current_cf == CF_SEASONAL || current_cf == CF_DEVSEASONAL) {
-                    /* If this is a bulk update, we need to skip ahead in the seasonal
-                     * arrays so that they will be correct for the next observed value;
-                     * note that for the bulk update itself, no update will occur to
-                     * DEVSEASONAL or SEASONAL; futhermore, HWPREDICT and DEVPREDICT will
-                     * be set to DNAN. */
+                    /* If this is a bulk update, we need to skip ahead in
+                       the seasonal arrays so that they will be correct for
+                       the next observed value;
+                       note that for the bulk update itself, no update will
+                       occur to DEVSEASONAL or SEASONAL; futhermore, HWPREDICT
+                       and DEVPREDICT will be set to DNAN. */
                     if (rra_step_cnt[i] > 2) {
                         /* skip update by resetting rra_step_cnt[i],
-                         * note that this is not data source specific; this is due
-                         * to the bulk update, not a DNAN value for the specific data
-                         * source. */
+                           note that this is not data source specific; this is
+                           due to the bulk update, not a DNAN value for the
+                           specific data source. */
                         rra_step_cnt[i] = 0;
                         lookup_seasonal(&rrd, i, rra_start, rrd_file,
                                         elapsed_pdp_st, &last_seasonal_coef);
@@ -1463,18 +1426,12 @@ int _rrd_update(
         rrd_set_error("error writing(unmapping) file: %s", filename);
     }
 #else
-    rrd_flush(rrd_file);    //XXX: really needed?
+    //rrd_flush(rrd_file);    //XXX: really needed?
 #endif
     /* if we got here and if there is an error and if the file has not been
      * written to, then close things up and return. */
     if (rrd_test_error()) {
-        free(updvals);
-        free(tmpl_idx);
-        rrd_free(&rrd);
-        free(pdp_temp);
-        free(pdp_new);
-        close(rrd_file->fd);
-        return (-1);
+        goto err_free_pdp_new;
     }
 
     /* aargh ... that was tough ... so many loops ... anyway, its done.
@@ -1485,38 +1442,20 @@ int _rrd_update(
                             + sizeof(rra_def_t) * rrd.stat_head->rra_cnt),
                  SEEK_SET) != 0) {
         rrd_set_error("seek rrd for live header writeback");
-        free(updvals);
-        free(tmpl_idx);
-        rrd_free(&rrd);
-        free(pdp_temp);
-        free(pdp_new);
-        close(rrd_file->fd);
-        return (-1);
+        goto err_free_pdp_new;
     }
 
     if (version >= 3) {
         if (rrd_write(rrd_file, rrd.live_head,
                       sizeof(live_head_t) * 1) != sizeof(live_head_t) * 1) {
             rrd_set_error("rrd_write live_head to rrd");
-            free(updvals);
-            rrd_free(&rrd);
-            free(tmpl_idx);
-            free(pdp_temp);
-            free(pdp_new);
-            close(rrd_file->fd);
-            return (-1);
+            goto err_free_pdp_new;
         }
     } else {
         if (rrd_write(rrd_file, &rrd.live_head->last_up,
                       sizeof(time_t) * 1) != sizeof(time_t) * 1) {
             rrd_set_error("rrd_write live_head to rrd");
-            free(updvals);
-            rrd_free(&rrd);
-            free(tmpl_idx);
-            free(pdp_temp);
-            free(pdp_new);
-            close(rrd_file->fd);
-            return (-1);
+            goto err_free_pdp_new;
         }
     }
 
@@ -1525,13 +1464,7 @@ int _rrd_update(
                   sizeof(pdp_prep_t) * rrd.stat_head->ds_cnt)
         != (ssize_t) (sizeof(pdp_prep_t) * rrd.stat_head->ds_cnt)) {
         rrd_set_error("rrd_write pdp_prep to rrd");
-        free(updvals);
-        rrd_free(&rrd);
-        free(tmpl_idx);
-        free(pdp_temp);
-        free(pdp_new);
-        close(rrd_file->fd);
-        return (-1);
+        goto err_free_pdp_new;
     }
 
     if (rrd_write(rrd_file, rrd.cdp_prep,
@@ -1541,26 +1474,14 @@ int _rrd_update(
                       rrd.stat_head->ds_cnt)) {
 
         rrd_set_error("rrd_write cdp_prep to rrd");
-        free(updvals);
-        free(tmpl_idx);
-        rrd_free(&rrd);
-        free(pdp_temp);
-        free(pdp_new);
-        close(rrd_file->fd);
-        return (-1);
+        goto err_free_pdp_new;
     }
 
     if (rrd_write(rrd_file, rrd.rra_ptr,
                   sizeof(rra_ptr_t) * rrd.stat_head->rra_cnt)
         != (ssize_t) (sizeof(rra_ptr_t) * rrd.stat_head->rra_cnt)) {
         rrd_set_error("rrd_write rra_ptr to rrd");
-        free(updvals);
-        free(tmpl_idx);
-        rrd_free(&rrd);
-        free(pdp_temp);
-        free(pdp_new);
-        close(rrd_file->fd);
-        return (-1);
+        goto err_free_pdp_new;
     }
 #ifdef HAVE_POSIX_FADVISExxx
 
@@ -1572,11 +1493,10 @@ int _rrd_update(
     if (0 != posix_fadvise(rrd_file->fd, rra_begin, 0, POSIX_FADV_DONTNEED)) {
         rrd_set_error("setting POSIX_FADV_DONTNEED on '%s': %s", filename,
                       rrd_strerror(errno));
-        close(rrd_file->fd);
-        return (-1);
+        goto err_free_pdp_new;
     }
 #endif
-    /*XXX: ? */ rrd_flush(rrd_file);
+    /* rrd_flush(rrd_file); */
 
     /* calling the smoothing code here guarantees at most
      * one smoothing operation per rrd_update call. Unfortunately,
@@ -1584,8 +1504,6 @@ int _rrd_update(
      * for smoothing to occur off-schedule. This really isn't
      * critical except during the burning cycles. */
     if (schedule_smooth) {
-//    in_file = fopen(filename,"rb+");
-
 
         rra_start = rra_begin;
         for (i = 0; i < rrd.stat_head->rra_cnt; ++i) {
@@ -1607,19 +1525,35 @@ int _rrd_update(
             posix_fadvise(rrd_file->fd, rra_begin, 0, POSIX_FADV_DONTNEED)) {
             rrd_set_error("setting POSIX_FADV_DONTNEED on '%s': %s", filename,
                           rrd_strerror(errno));
-            close(rrd_file->fd);
-            return (-1);
+            goto err_free_pdp_new;
         }
 #endif
     }
 
-    rrd_close(rrd_file);
     rrd_free(&rrd);
-    free(updvals);
-    free(tmpl_idx);
+    close(rrd_file->fd);
+    rrd_close(rrd_file);
+
     free(pdp_new);
+    free(tmpl_idx);
     free(pdp_temp);
+    free(updvals);
     return (0);
+
+  err_free_pdp_new:
+    free(pdp_new);
+  err_free_tmpl_idx:
+    free(tmpl_idx);
+  err_free_pdp_temp:
+    free(pdp_temp);
+  err_free_updvals:
+    free(updvals);
+  err_close:
+    rrd_free(&rrd);
+    close(rrd_file->fd);
+    rrd_close(rrd_file);
+  err_out:
+    return (-1);
 }
 
 /*