pndnotifyd: rework event wait

Block indefinitely. Main motivation is to get rid of idle wakeups.
Also attempts to deal with multiple inotify events better.

diff --git a/apps/pndnotifyd.c b/apps/pndnotifyd.c
index 218608c..5f7f231 100644 (file)
--- a/apps/pndnotifyd.c
+++ b/apps/pndnotifyd.c
@@ -15,6 +15,7 @@
 #include <sys/stat.h>  // for umask
 #include <dirent.h>    // for opendir()
 #include <signal.h>    // for sigaction
 #include <sys/stat.h>  // for umask
 #include <dirent.h>    // for opendir()
 #include <signal.h>    // for sigaction

+#include <errno.h>     // for errno

 
 #include "pnd_conf.h"
 #include "pnd_container.h"
 
 #include "pnd_conf.h"
 #include "pnd_container.h"


@@ -263,18 +264,94 @@ int main ( int argc, char *argv[] ) {
 
   /* daemon main loop
    */
 
   /* daemon main loop
    */

-  unsigned char watch_inotify, watch_dbus;
+  unsigned char watch_inotify = 0, watch_dbus = 0;
+  unsigned char idle = 0;

   while ( 1 ) {
   while ( 1 ) {

+    int retcode;
+
+    // collect all events to avoid multiple PND rescans
+    do {
+      struct timeval timeout_1s = { 1, 0};
+      int fd_inotify = -1, fd_dbus = -1;
+      struct timeval *timeout = NULL;
+      fd_set rfds, efds;
+      int fd_max = -1;
+
+      if ( dbh )
+        fd_dbus = pnd_dbusnotify_fd ( dbh );
+      if ( nh )
+        fd_inotify = pnd_notify_fd ( nh );
+
+      FD_ZERO ( &rfds );
+      FD_ZERO ( &efds );
+
+      if ( fd_dbus != -1 ) {
+        FD_SET ( fd_dbus, &rfds );
+        FD_SET ( fd_dbus, &efds );
+        if ( fd_dbus > fd_max )
+          fd_max = fd_dbus;
+      }
+
+      if ( fd_inotify != -1 ) {
+        FD_SET ( fd_inotify, &rfds );
+        FD_SET ( fd_inotify, &efds );
+        if ( fd_inotify > fd_max )
+          fd_max = fd_inotify;
+      }
+
+      if ( fd_max == -1 ) {
+        pnd_log ( pndn_error, "ERROR: notify: nothing to watch\n" );
+        retcode = -1;
+        break;
+      }

 
 

-    watch_dbus = 0;
-    watch_inotify = 0;
+      // the idle timeout is needed for inotify as some file updates produce several events
+      // with a delay in between (length depends on things like file size and system load), like
+      // - IN_CREATE, IN_CLOSE_WRITE (cp, mv from other fs, etc)
+      // - IN_MOVED_TO, IN_CLOSE_WRITE (PNDManager)
+      // - multiple file copies, deletes, etc
+      timeout = idle ? NULL : &timeout_1s;

 
 

-    if ( dbh ) {
-      watch_dbus = pnd_dbusnotify_rediscover_p ( dbh );
+      retcode = select ( fd_max + 1, &rfds, NULL, &efds, timeout );
+
+      if ( retcode < 0 ) {
+        pnd_log ( pndn_error, "ERROR: notify: select failed: %d\n", errno );
+        if ( errno == EINTR )
+          retcode = 0;
+
+        break;
+      }
+      idle = ( retcode == 0 );
+
+      if ( dbh && FD_ISSET ( fd_dbus, &efds ) ) {
+        pnd_log ( pndn_error, "ERROR: notify: dbus fd error\n" );
+        pnd_dbusnotify_shutdown ( dbh );
+        dbh = NULL;
+      }
+
+      if ( nh && FD_ISSET ( fd_inotify, &efds ) ) {
+        pnd_log ( pndn_error, "ERROR: notify: inotify fd error\n" );
+        pnd_notify_shutdown ( nh );
+        nh = NULL;
+      }
+
+      if ( dbh && FD_ISSET ( fd_dbus, &rfds ) ) {
+        watch_dbus |= pnd_dbusnotify_rediscover_p ( dbh );
+        if ( watch_dbus && nh ) {
+          // will restart inotify (see below), drop all events
+          pnd_notify_shutdown ( nh );
+          nh = NULL;
+        }
+      }
+
+      if ( nh && FD_ISSET ( fd_inotify, &rfds ) ) {
+        watch_inotify |= pnd_notify_rediscover_p ( nh );
+      }

     }
     }

+    while ( !idle );

 
 

-    if ( ! watch_dbus && nh ) {
-      watch_inotify = pnd_notify_rediscover_p ( nh );
+    if ( retcode < 0 ) {
+      break;

     }
 
     // need to rediscover?
     }
 
     // need to rediscover?


@@ -299,13 +376,12 @@ int main ( int argc, char *argv[] ) {
 
       if ( watch_dbus ) {
        pnd_log ( pndn_rem, "dbusnotify detected an event\n" );
 
       if ( watch_dbus ) {
        pnd_log ( pndn_rem, "dbusnotify detected an event\n" );

-       pnd_notify_shutdown ( nh );
-       nh = 0;

       }
 
       }
 

-      if ( time ( NULL ) - createtime <= 2 ) {
-       pnd_log ( pndn_rem, "Rediscovery request comes to soon after previous discovery; skipping.\n" );
+      if ( time ( NULL ) - createtime < interval_secs ) {
+       pnd_log ( pndn_rem, "Rediscovery request comes to soon after previous discovery; delaying.\n" );

        sleep ( interval_secs );
        sleep ( interval_secs );

+       idle = 0;

        continue;
       }
 
        continue;
       }
 


@@ -352,16 +428,9 @@ int main ( int argc, char *argv[] ) {
        setup_notifications();
       }
 
        setup_notifications();
       }
 

-    } // need to rediscover?
+      watch_inotify = watch_dbus = 0;

 
 

-    // lets not eat up all the CPU
-    // should use an alarm or select() or something -- I mean really, why aren't I putting interval_secs into
-    // the select() call above in pnd_notify_whatsitcalled()? -- but lets not break this right before release shall we
-    // NOTE: Oh right, I remember now -- inotify will spam when a card is inserted, and it will not be instantaneoous..
-    // the events will dribble in over a second. So this sleep is a lame trick that generally works. I really should
-    // do select(), and then when it returns just spin for a couple seconds slurping up events until no more and a thresh-hold
-    // time is hit, but this will do for now. I suck.
-    sleep ( interval_secs );
+    } // need to rediscover?

 
   } // while
 
 
   } // while
 

diff --git a/include/pnd_dbusnotify.h b/include/pnd_dbusnotify.h
index 00f8581..590cecf 100644 (file)
--- a/include/pnd_dbusnotify.h
+++ b/include/pnd_dbusnotify.h
@@ -32,6 +32,8 @@ void pnd_dbusnotify_shutdown ( pnd_dbusnotify_handle h );
  */
 unsigned char pnd_dbusnotify_rediscover_p ( pnd_dbusnotify_handle h );
 
  */
 unsigned char pnd_dbusnotify_rediscover_p ( pnd_dbusnotify_handle h );
 

+int pnd_dbusnotify_fd ( pnd_dbusnotify_handle h );
+

 #ifdef __cplusplus
 } /* "C" */
 #endif
 #ifdef __cplusplus
 } /* "C" */
 #endif

diff --git a/include/pnd_notify.h b/include/pnd_notify.h
index f4bf594..fc31289 100644 (file)
--- a/include/pnd_notify.h
+++ b/include/pnd_notify.h
@@ -40,6 +40,8 @@ unsigned char pnd_notify_rediscover_p ( pnd_notify_handle h );
  */
 unsigned char pnd_notify_wait_until_ready ( unsigned int secs_timeout );
 
  */
 unsigned char pnd_notify_wait_until_ready ( unsigned int secs_timeout );
 

+int pnd_notify_fd ( pnd_notify_handle h );
+

 #ifdef __cplusplus
 } /* "C" */
 #endif
 #ifdef __cplusplus
 } /* "C" */
 #endif

diff --git a/lib/pnd_dbusnotify.c b/lib/pnd_dbusnotify.c
index dfa200c..b2ff461 100644 (file)
--- a/lib/pnd_dbusnotify.c
+++ b/lib/pnd_dbusnotify.c
@@ -88,6 +88,16 @@ void pnd_dbusnotify_shutdown ( pnd_dbusnotify_handle h ) {
   return;
 }
 
   return;
 }
 

+int pnd_dbusnotify_fd ( pnd_dbusnotify_handle h ) {
+  pnd_dbusnotify_t *p = (pnd_dbusnotify_t*) h;
+  int r;
+
+  if ( p )
+    r = p -> fd [ 0 ]; // read side of pipe
+
+  return r;
+}
+

 unsigned char pnd_dbusnotify_rediscover_p ( pnd_dbusnotify_handle h ) {
   pnd_dbusnotify_t *p = (pnd_dbusnotify_t*) h;
   int r;
 unsigned char pnd_dbusnotify_rediscover_p ( pnd_dbusnotify_handle h ) {
   pnd_dbusnotify_t *p = (pnd_dbusnotify_t*) h;
   int r;

diff --git a/lib/pnd_notify.c b/lib/pnd_notify.c
index 33d9714..efcadcb 100644 (file)
--- a/lib/pnd_notify.c
+++ b/lib/pnd_notify.c
@@ -254,3 +254,13 @@ unsigned char pnd_notify_wait_until_ready ( unsigned int secs_timeout ) {
 
   return ( 0 ); // fail
 }
 
   return ( 0 ); // fail
 }

+
+int pnd_notify_fd ( pnd_notify_handle h ) {
+  pnd_notify_t *p = (pnd_notify_t*) h;
+  int r = -1;
+
+  if ( p )
+    r = p->fd;
+
+  return r;
+}