[vdr] cUnbufferedFile and NFS mounted video dir

Artur Skawina art_k at o2.pl
Sat Jan 28 23:50:42 CET 2006


Klaus Schmidinger wrote:
> Artur Skawina wrote:
>> However i consider the whole patch a bugfix :) Now that the fsyncs are 
>> disabled it probably isn't that critical though.
> 
> So does this mean that without the fdatasync() calls it works ok as
> it is in plain vanilla VDR (without any patch)?

As I haven't run vdr w/o this patch for a long time, i'm not sure. Apparently
things are not ok, as this thread shows, but other vdr-1.3.38+ users with a 2.6
kernel could answer the above question better...
For me it was the sync calls that made me look at this code; w/o them things
weren't as bad, IIRC. I ended up disabling the fdatasyncs completely even when
using fadvise.

> What is the exact meaning of the "WriteStrategy" option?
> Does it simply turn the fadvise stuff on/off, or is there more
> magic behind it? I would prefer a version that works without this
> option, because this is nothing a normal user would easily know
> how to set.

it started as a debugging knob, so i could test the impact of the sync calls,
compare cutting speed w/ fadvise on/off etc.
(Setup.WriteStrategy==1) meant no POSIX_FADV_DONTNEED calls while accessing a
file (the calls were always made when closing a file).

> So, if you can provide a patch against the latest VDR version that
> still uses "#ifdef USE_FADVISE" to completely turn the fadvise
> stuff off, and does _not_ introduce another setup option, I might
> consider including it for the final version 1.4.

i did the minimal fix to the existing vdr code -- note it's far from optimal, 
and i suspect there are cases where the fadvise-enabled version isn't 
necessarily an improvement. An option which basically turns USE_FADVISE into a 
runtime switch would be useful.
As i haven't needed the option myself lately, in fact didn't even remember what
exactly it did and had to check the code, i just killed it, patch attached.
It's vs 1.3.39 because the UI/channel-switch posts scared me away from upgrading
vdr, still remembering the broken menu key experience... ;)
cutter.c part is entirely optional (affects only cutting speed).

artur
-------------- next part --------------
--- vdr-1.3.39.org/cutter.c	2005-10-31 12:26:44.000000000 +0000
+++ vdr-1.3.39/cutter.c	2006-01-28 21:33:39.000000000 +0000
@@ -66,6 +66,8 @@ void cCuttingThread::Action(void)
      toFile = toFileName->Open();
      if (!fromFile || !toFile)
         return;
+     fromFile->SetReadAhead(MEGABYTE(20));
+     toFile->SetReadAhead(MEGABYTE(20));
      int Index = Mark->position;
      Mark = fromMarks.Next(Mark);
      int FileSize = 0;
@@ -90,6 +92,7 @@ void cCuttingThread::Action(void)
            if (fromIndex->Get(Index++, &FileNumber, &FileOffset, &PictureType, &Length)) {
               if (FileNumber != CurrentFileNumber) {
                  fromFile = fromFileName->SetOffset(FileNumber, FileOffset);
+                 fromFile->SetReadAhead(MEGABYTE(20));
                  CurrentFileNumber = FileNumber;
                  }
               if (fromFile) {
@@ -118,10 +121,11 @@ void cCuttingThread::Action(void)
                  break;
               if (FileSize > MEGABYTE(Setup.MaxVideoFileSize)) {
                  toFile = toFileName->NextFile();
-                 if (toFile < 0) {
+                 if (!toFile) {
                     error = "toFile 1";
                     break;
                     }
+                 toFile->SetReadAhead(MEGABYTE(20));
                  FileSize = 0;
                  }
               LastIFrame = 0;
@@ -158,10 +162,11 @@ void cCuttingThread::Action(void)
                  cutIn = true;
                  if (Setup.SplitEditedFiles) {
                     toFile = toFileName->NextFile();
-                    if (toFile < 0) {
+                    if (!toFile) {
                        error = "toFile 2";
                        break;
                        }
+                    toFile->SetReadAhead(MEGABYTE(20));
                     FileSize = 0;
                     }
                  }
--- vdr-1.3.39.org/tools.c	2006-01-15 14:31:45.000000000 +0000
+++ vdr-1.3.39/tools.c	2006-01-28 21:48:22.000000000 +0000
@@ -1040,10 +1040,9 @@ bool cSafeFile::Close(void)
 
 // --- cUnbufferedFile -------------------------------------------------------
 
-//#define USE_FADVISE
+#define USE_FADVISE
 
-#define READ_AHEAD MEGABYTE(2)
-#define WRITE_BUFFER MEGABYTE(10)
+#define WRITE_BUFFER KILOBYTE(800)
 
 cUnbufferedFile::cUnbufferedFile(void)
 {
@@ -1059,8 +1058,17 @@ int cUnbufferedFile::Open(const char *Fi
 {
   Close();
   fd = open(FileName, Flags, Mode);
-  begin = end = ahead = -1;
+  curpos = 0;
+  begin = lastpos = ahead = 0;
+  readahead = 128*1024;
+  pendingreadahead = 0;
   written = 0;
+  totwritten = 0;
+  if (fd >= 0) {
+     // we could use POSIX_FADV_SEQUENTIAL, but we do our own readahead
+     // disabling the kernel one.
+     posix_fadvise(fd, 0, 0, POSIX_FADV_RANDOM);
+  }
   return fd;
 }
 
@@ -1068,15 +1076,10 @@ int cUnbufferedFile::Close(void)
 {
 #ifdef USE_FADVISE
   if (fd >= 0) {
-     if (ahead > end)
-        end = ahead;
-     if (begin >= 0 && end > begin) {
-        //dsyslog("close buffer: %d (flush: %d bytes, %ld-%ld)", fd, written, begin, end);
-        if (written)
-           fdatasync(fd);
-        posix_fadvise(fd, begin, end - begin, POSIX_FADV_DONTNEED);
-        }
-     begin = end = ahead = -1;
+     if (totwritten)    // if we wrote anything make sure the data has hit the disk before
+        fdatasync(fd);  // calling fadvise, as this is our last chance to un-cache it.
+     posix_fadvise(fd, 0, 0, POSIX_FADV_DONTNEED);
+     begin = lastpos = ahead = 0;
      written = 0;
      }
 #endif
@@ -1085,45 +1088,95 @@ int cUnbufferedFile::Close(void)
   return close(OldFd);
 }
 
-off_t cUnbufferedFile::Seek(off_t Offset, int Whence)
-{
-  if (fd >= 0)
-     return lseek(fd, Offset, Whence);
-  return -1;
-}
-
+// When replaying and going eg FF->PLAY the position jumps back 2..8M
+// hence we might not want to drop that data at once. 
+// Ignoring this for now to avoid making this even more complex,
+// but we could at least try to handle the common cases.
+// (PLAY->FF->PLAY, small jumps, moving editing marks etc)
+
+#define KREADAHEAD MEGABYTE(4) // amount (guess) of kernel/fs prefetch that
+                               // could happen in addition to our own.
+#define FADVGRAN   4096        // AKA fadvise-chunk-size; PAGE_SIZE or
+                               // getpagesize(2) would also work.
+                               
 ssize_t cUnbufferedFile::Read(void *Data, size_t Size)
 {
   if (fd >= 0) {
 #ifdef USE_FADVISE
-     off_t pos = lseek(fd, 0, SEEK_CUR);
-     // jump forward - adjust end position
-     if (pos > end)
-        end = pos;
-     // after adjusting end - don't clear more than previously requested
-     if (end > ahead)
-        end = ahead;
-     // jump backward - drop read ahead of previous run
-     if (pos < begin)
-        end = ahead;
-     if (begin >= 0 && end > begin)
-        posix_fadvise(fd, begin - KILOBYTE(200), end - begin + KILOBYTE(200), POSIX_FADV_DONTNEED);//XXX macros/parameters???
-     begin = pos;
+     off_t jumped = curpos-lastpos; // nonzero means we're not at the last offset
+     if (jumped) {                  // ie some kind of jump happened.
+        pendingreadahead += ahead-lastpos+KREADAHEAD;
+        // jumped forward? - treat as if we did read all the way to current pos.
+        if (jumped >= 0) {
+           lastpos = curpos;
+           // but clamp at ahead so we don't clear more than previously requested.
+           // (would be mostly harmless anyway, unless we got more than one reader of this file)
+           if (lastpos > (ahead+KREADAHEAD))
+              lastpos = ahead+KREADAHEAD;
+        }
+        // jumped backward? - drop both last read _and_ read-ahead
+        else
+          if (curpos < begin)
+              lastpos = ahead+KREADAHEAD;
+           // jumped backward, but still inside prev read window? - pretend we read less.
+           else /* if (curpos >= begin) */
+              lastpos = curpos;
+        }
+        
 #endif
      ssize_t bytesRead = safe_read(fd, Data, Size);
 #ifdef USE_FADVISE
+     // Now drop data accessed during _previous_ Read(). (ie. begin...lastpos)
+     // fadvise() internally has page granularity; it drops only whole pages
+     // from the range we specify (since it cannot know if we will be accessing
+     // the rest). This is why we drop only the previously read data, and do it
+     // _after_ the current read() call, while rounding up the window to make
+     // sure that even not PAGE_SIZE-aligned data gets freed.
+     // We're also trying merge the fadvise calls a bit in order to reduce overhead
+     // (to avoid a fadvise() call here for every read() above).
+     if (begin >= 0 && lastpos > begin)
+        if (jumped || (size_t)(lastpos-begin) > readahead) {
+           //dsyslog("taildrop:    %ld..%ld size %ld", begin, lastpos, lastpos-begin);
+           posix_fadvise(fd, begin-(FADVGRAN-1), lastpos-begin+(FADVGRAN-1)*2, POSIX_FADV_DONTNEED);
+           begin = curpos;
+           }
+        
      if (bytesRead > 0) {
-        pos += bytesRead;
-        end = pos;
-        // this seems to trigger a non blocking read - this
-        // may or may not have been finished when we will be called next time.
-        // If it is not finished we can't release the not yet filled buffers.
-        // So this is commented out till we find a better solution.
-        //posix_fadvise(fd, pos, READ_AHEAD, POSIX_FADV_WILLNEED);
-        ahead = pos + READ_AHEAD;
+        curpos += bytesRead;
+        //dsyslog("jump: %06ld ra: %06ld size: %ld", jumped, (long)readahead, (long)Size);
+
+        // no jump? (allow small forward jump still inside readahead window).
+        if (jumped>=0 && jumped<=(off_t)readahead) {
+           // Trigger the readahead IO, but only if we've used at least
+           // 1/4 of the previously requested area. This avoids calling
+           // fadvise() after every read() call.
+           if ((ahead-curpos)<(off_t)(readahead-readahead/4)) {
+              posix_fadvise(fd, curpos, readahead, POSIX_FADV_WILLNEED);
+              ahead = curpos + readahead;
+              }
+           if ( readahead < Size*64 ) { // automagically tune readahead size.
+              readahead = Size*64;
+              //dsyslog("Readahead for fd: %d increased to %ld", fd, (long)readahead);
+              }
+           }
+        else {
+           // jumped - we really don't want any readahead now. otherwise
+           // eg fast-rewind gets in trouble.
+           ahead = curpos;
+
+           // Every now and then, flush all cached data from this file; mostly
+           // to get rid of nonflushed readahead coming from _previous_ jumps
+           // (if the readahead I/O hasn't finished by the time we called
+           // fadvice() to undo it, the data could still be cached).
+           // The accounting does not have to be 100% accurate, as long as
+           // this triggers after _some_ jumps we should be ok.
+           if (pendingreadahead>MEGABYTE(40)) {
+              pendingreadahead = 0;
+              posix_fadvise(fd, 0, 0, POSIX_FADV_DONTNEED);
+              }
+           }
         }
-     else
-        end = pos;
+     lastpos = curpos;
 #endif
      return bytesRead;
      }
@@ -1132,29 +1185,37 @@ ssize_t cUnbufferedFile::Read(void *Data
 
 ssize_t cUnbufferedFile::Write(const void *Data, size_t Size)
 {
+  //dsyslog("Unbuffered:Write: fd: %d  %8ld..%8ld  size: %5ld", fd, curpos, curpos+Size, (long)Size);
   if (fd >=0) {
-#ifdef USE_FADVISE
-     off_t pos = lseek(fd, 0, SEEK_CUR);
-#endif
      ssize_t bytesWritten = safe_write(fd, Data, Size);
 #ifdef USE_FADVISE
-     if (bytesWritten >= 0) {
+     if (bytesWritten > 0) {
+        begin = min(begin, curpos);
+        curpos += bytesWritten;
         written += bytesWritten;
-        if (begin >= 0) {
-           if (pos < begin)
-              begin = pos;
-           }
-        else
-           begin = pos;
-        if (pos + bytesWritten > end)
-           end = pos + bytesWritten;
+        lastpos = max(lastpos, curpos);
         if (written > WRITE_BUFFER) {
-           //dsyslog("flush buffer: %d (%d bytes, %ld-%ld)", fd, written, begin, end);
-           fdatasync(fd);
-           if (begin >= 0 && end > begin)
-              posix_fadvise(fd, begin, end - begin, POSIX_FADV_DONTNEED);
-           begin = end = -1;
+           //dsyslog("flush buffer: %d (%d bytes, %ld-%ld)", fd, written, begin, lastpos);
+           if (lastpos>begin) {
+              off_t headdrop = min(begin, WRITE_BUFFER*2L);
+              posix_fadvise(fd, begin-headdrop, lastpos-begin+headdrop, POSIX_FADV_DONTNEED);
+              }
+           begin = lastpos = max(0L, curpos-4095);
+           totwritten += written;
            written = 0;
+           // The above fadvise() works when writing slowly (recording), but could
+           // leave cached data around when writing at a high rate (cutting).
+           // Also, it seems in some setups, the above does not trigger any I/O and
+           // the fdatasync() call below has to do all the work (reiserfs with some
+           // kind of write gathering enabled).
+           // We add 'readahead' to the threshold in an attempt to increase cutting
+           // speed; it's a tradeoff -- speed vs RAM-used.
+           if (totwritten>(MEGABYTE(32)+readahead)) {
+              //fdatasync(fd);
+              off_t headdrop = min(curpos-totwritten, totwritten*2L);
+              posix_fadvise(fd, curpos-totwritten-headdrop, totwritten+headdrop, POSIX_FADV_DONTNEED);
+              totwritten = 0;
+              }
            }
         }
 #endif
--- vdr-1.3.39.org/tools.h	2006-01-08 11:40:37.000000000 +0000
+++ vdr-1.3.39/tools.h	2006-01-28 21:32:26.000000000 +0000
@@ -237,22 +237,40 @@ public:
 /// cUnbufferedFile is used for large files that are mainly written or read
 /// in a streaming manner, and thus should not be cached.
 
+#include <sys/types.h>
+#include <sys/mman.h>
+#include <unistd.h>
+
 class cUnbufferedFile {
 private:
   int fd;
+  off_t curpos;
   off_t begin;
-  off_t end;
+  off_t lastpos;
   off_t ahead;
-  ssize_t written;
+  size_t pendingreadahead;
+  size_t readahead;
+  size_t written;
+  size_t totwritten;
 public:
   cUnbufferedFile(void);
   ~cUnbufferedFile();
   int Open(const char *FileName, int Flags, mode_t Mode = DEFFILEMODE);
   int Close(void);
-  off_t Seek(off_t Offset, int Whence);
+  off_t Seek(off_t Offset, int Whence)
+  {
+    //dsyslog("Seek: fd: %d offs: %ld whence: %d diff: %ld", fd, (long)Offset, Whence, Offset-curpos);
+    if (Whence == SEEK_SET && Offset == curpos)
+       return curpos;
+
+    curpos = lseek(fd, Offset, Whence);
+    return curpos;
+  }
+
   ssize_t Read(void *Data, size_t Size);
   ssize_t Write(const void *Data, size_t Size);
   static cUnbufferedFile *Create(const char *FileName, int Flags, mode_t Mode = DEFFILEMODE);
+  void SetReadAhead(size_t ra) { readahead = ra; };
   };
 
 class cLockFile {



More information about the vdr mailing list