Re: [fuse-devel] PATCH: Unmount on inactivity (2)

[Csaba H. <csa...@cr...>]

On 2006-06-05, Remy Blank <rem...@po...> wrote:
> Here's an updated patch taking into account Csaba's feedback. It applies
> cleanly against CVS HEAD.

As of the time of trying it, it doesn't. I send hereby an updated
version (where I make no change in functionality, just adopt your patch
to current HEAD).

> It has been tested on linux kernel 2.6.15 with sshfs. I would appreciate
> if somebody could test it on a BSD platform.

I tested it and it works fine, with minor issues. These are:

 - Don't make mountpoint mandatory. On FreeBSD a Fuse fs doesn't
   necessarily knows about the mount point (it can be mounted
   externally); still it can unmoint itself, using the filehandle
   to the device. (And on Linux the mountpoint is alreadly made
   mandatory elsewhere.)

 - Having a separate control (or other purpose) thread can interfere
   with signal delivery, as Miklos has spotted it in case of sshfs (which
   has a separate thead to talk with the server). (I think this actually
   happens only on FreeBSD.) Cf:
   http://mercurial.creo.hu/repos/sshfs-hg/?cs=d4e34dc605ba

 - You don't call pthread_detach(3). According to sshfs and the manpage,
   it seems that it should indeed be called after the pthread_create()
   call.

I send another patch which handles these problems. The second 
patch applies to a tree which has already been patched by the
first patch.

(Note: patches are not sent as mime attachmets [due to the limitation of
my newsreader], they are delimited by lines consisting of "%" signs.
If someone requires that, I take the effort to resend them mime attached.)

Regards,
Csaba

[updated Remy patch]
%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
diff -r 8ba8f17842cd lib/fuse.c
--- a/lib/fuse.c	Tue Jun 06 10:16:37 2006 +0000
+++ b/lib/fuse.c	Wed Jun 07 03:15:57 2006 +0200
@@ -27,6 +27,7 @@
 #include <assert.h>
 #include <pthread.h>
 #include <sys/param.h>
+#include <sys/time.h>
 #include <sys/uio.h>
 
 #define FUSE_MAX_PATH 4096
@@ -41,6 +42,7 @@ struct fuse_config {
     double negative_timeout;
     double attr_timeout;
     double ac_attr_timeout;
+    unsigned int idle_timeout;
     int ac_attr_timeout_set;
     int debug;
     int hard_remove;
@@ -52,6 +54,15 @@ struct fuse_config {
     int direct_io;
     int kernel_cache;
     int auto_cache;
+};
+
+struct fuse_idle_monitor {
+    time_t last_activity; 
+    int running; 
+    pthread_t thread; 
+    pthread_mutex_t lock; 
+    pthread_cond_t wakeup_cond; 
+    char *mountpoint;
 };
 
 struct fuse {
@@ -67,6 +78,7 @@ struct fuse {
     unsigned int hidectr;
     pthread_mutex_t lock;
     pthread_rwlock_t tree_lock;
+    struct fuse_idle_monitor idle_monitor;
     void *user_data;
     struct fuse_config conf;
 };
@@ -594,6 +606,9 @@ static struct fuse *req_fuse_prepare(fus
     c->gid = ctx->gid;
     c->pid = ctx->pid;
     c->private_data = c->fuse->user_data;
+
+    if (c->fuse->conf.idle_timeout > 0) 
+        c->fuse->idle_monitor.last_activity = time(0);
 
     return c->fuse;
 }
@@ -1964,6 +1979,7 @@ static const struct fuse_opt fuse_lib_op
     FUSE_LIB_OPT("ac_attr_timeout=%lf",   ac_attr_timeout, 0),
     FUSE_LIB_OPT("ac_attr_timeout=",      ac_attr_timeout_set, 1),
     FUSE_LIB_OPT("negative_timeout=%lf",  negative_timeout, 0),
+    FUSE_LIB_OPT("idle_timeout=%d",       idle_timeout, 0),
     FUSE_OPT_END
 };
 
@@ -1983,6 +1999,7 @@ static void fuse_lib_help(void)
 "    -o negative_timeout=T  cache timeout for deleted names (0.0s)\n"
 "    -o attr_timeout=T      cache timeout for attributes (1.0s)\n"
 "    -o ac_attr_timeout=T   auto cache timeout for attributes (attr_timeout)\n"
+"    -o idle_timeout=T      timeout for automatic unmount when idle (none)\n"
 "\n");
 }
 
@@ -2076,6 +2093,8 @@ struct fuse *fuse_new_common(struct fuse
     pthread_rwlock_init(&f->tree_lock, NULL);
     memcpy(&f->op, op, op_size);
     f->compat = compat;
+    mutex_init(&f->idle_monitor.lock); 
+    pthread_cond_init(&f->idle_monitor.wakeup_cond, 0);
 
     root = (struct node *) calloc(1, sizeof(struct node));
     if (root == NULL) {
@@ -2148,8 +2167,99 @@ void fuse_destroy(struct fuse *f)
     free(f->name_table);
     pthread_mutex_destroy(&f->lock);
     pthread_rwlock_destroy(&f->tree_lock);
+    pthread_mutex_destroy(&f->idle_monitor.lock);
+    pthread_cond_destroy(&f->idle_monitor.wakeup_cond);
     fuse_session_destroy(f->se);
     free(f);
+}
+
+#define MIN_IDLE_INTERVAL 2
+
+static void *idle_monitor(void *arg)
+{
+    struct fuse *f = (struct fuse *)arg;
+    struct timeval now;
+    struct timespec wakeup_time;
+    long idle_time;
+    long next_check;
+
+    pthread_mutex_lock(&f->idle_monitor.lock);
+    
+    while(f->idle_monitor.running) {
+        gettimeofday(&now, 0);
+
+        idle_time = now.tv_sec - f->idle_monitor.last_activity;
+        next_check = MIN_IDLE_INTERVAL;
+        
+        if (f->conf.debug) {
+            printf("Idle monitor: idle for %ld seconds\n", (long)idle_time);
+            fflush(stdout);
+        }
+        
+        if (idle_time >= (long)f->conf.idle_timeout) {
+            if (f->conf.debug) {
+                printf("Idle timeout reached, trying to unmount %s\n", f->idle_monitor.mountpoint);
+                fflush(stdout);
+            }
+            fuse_try_unmount_nonlazy(f->idle_monitor.mountpoint, fuse_session_next_chan(f->se, NULL));
+            next_check = f->conf.idle_timeout;
+        } else {
+            next_check = MAX(f->conf.idle_timeout - idle_time, MIN_IDLE_INTERVAL);
+        }
+        
+        wakeup_time.tv_sec = now.tv_sec + next_check;
+        wakeup_time.tv_nsec = now.tv_usec * 1000;
+        pthread_cond_timedwait(&f->idle_monitor.wakeup_cond, &f->idle_monitor.lock, &wakeup_time);
+    }
+    
+    pthread_mutex_unlock(&f->idle_monitor.lock);
+    return 0;
+}
+
+int fuse_start_idle_monitor_thread(struct fuse *f, char *mountpoint)
+{
+    int res;
+    
+    if (f->conf.idle_timeout <= 0)
+        return 0;
+    
+    if (mountpoint == NULL) {
+        fprintf(stderr, "fuse: mountpoint not available for idle monitor\n");
+        return -1;
+    }
+
+    if (f->conf.debug) {
+        printf("Starting idle monitor thread, timeout=%ds\n", f->conf.idle_timeout);
+        fflush(stdout);
+    }
+
+    f->idle_monitor.mountpoint = mountpoint;
+    f->idle_monitor.last_activity = time(0);
+    f->idle_monitor.running = 1;
+    res = pthread_create(&f->idle_monitor.thread, 0, idle_monitor, (void *)f);
+    if (res != 0) {
+        fprintf(stderr, "fuse: unable to create idle monitor thread\n");
+        return -1;
+    }
+    
+    return 0;
+}
+
+void fuse_stop_idle_monitor_thread(struct fuse *f)
+{
+    if (f->conf.idle_timeout <= 0)
+        return;
+
+    if (f->conf.debug) {
+        printf("Stopping idle monitor thread\n");
+        fflush(stdout);
+    }
+    
+    f->idle_monitor.running = 0;
+    pthread_mutex_lock(&f->idle_monitor.lock);
+    pthread_cond_signal(&f->idle_monitor.wakeup_cond);
+    pthread_mutex_unlock(&f->idle_monitor.lock);
+    pthread_join(f->idle_monitor.thread, 0);    
 }
 
 #include "fuse_compat.h"
diff -r 8ba8f17842cd lib/fuse_i.h
--- a/lib/fuse_i.h	Tue Jun 06 10:16:37 2006 +0000
+++ b/lib/fuse_i.h	Wed Jun 07 03:15:57 2006 +0200
@@ -22,6 +22,9 @@ struct fuse *fuse_new_common(struct fuse
                              const struct fuse_operations *op,
                              size_t op_size, void *user_data, int compat);
 
+int fuse_start_idle_monitor_thread(struct fuse *f, char *mountpoint);
+void fuse_stop_idle_monitor_thread(struct fuse *f);
+
 int fuse_sync_compat_args(struct fuse_args *args);
 
 struct fuse_chan *fuse_kern_chan_new(int fd);
@@ -31,5 +34,6 @@ struct fuse_session *fuse_lowlevel_new_c
                                        size_t op_size, void *userdata);
 
 void fuse_kern_unmount_compat22(const char *mountpoint);
-void fuse_kern_unmount(const char *mountpoint, int fd);
+void fuse_kern_unmount(const char *mountpoint, int fd, int lazy);
 int fuse_kern_mount(const char *mountpoint, struct fuse_args *args);
+void fuse_try_unmount_nonlazy(const char *mountpoint, struct fuse_chan *ch);
diff -r 8ba8f17842cd lib/helper.c
--- a/lib/helper.c	Tue Jun 06 10:16:37 2006 +0000
+++ b/lib/helper.c	Wed Jun 07 03:15:57 2006 +0200
@@ -201,7 +201,7 @@ static struct fuse_chan *fuse_mount_comm
 
     ch = fuse_kern_chan_new(fd);
     if (!ch)
-        fuse_kern_unmount(mountpoint, fd);
+        fuse_kern_unmount(mountpoint, fd, 1);
 
     return ch;
 }
@@ -214,13 +214,19 @@ static void fuse_unmount_common(const ch
 static void fuse_unmount_common(const char *mountpoint, struct fuse_chan *ch)
 {
     int fd = ch ? fuse_chan_fd(ch) : -1;
-    fuse_kern_unmount(mountpoint, fd);
+    fuse_kern_unmount(mountpoint, fd, 1);
     fuse_chan_destroy(ch);
 }
 
 void fuse_unmount(const char *mountpoint, struct fuse_chan *ch)
 {
     fuse_unmount_common(mountpoint, ch);
+}
+
+void fuse_try_unmount_nonlazy(const char *mountpoint, struct fuse_chan *ch)
+{
+    int fd = ch ? fuse_chan_fd(ch) : -1;
+    fuse_kern_unmount(mountpoint, fd, 0);
 }
 
 static struct fuse *fuse_setup_common(int argc, char *argv[],
@@ -261,6 +267,10 @@ static struct fuse *fuse_setup_common(in
     if (res == -1)
         goto err_destroy;
 
+    res = fuse_start_idle_monitor_thread(fuse, *mountpoint);
+    if (res == -1)
+        goto err_destroy;
+
     if (fd)
         *fd = fuse_chan_fd(ch);
 
@@ -287,6 +297,7 @@ static void fuse_teardown_common(struct 
 {
     struct fuse_session *se = fuse_get_session(fuse);
     struct fuse_chan *ch = fuse_session_next_chan(se, NULL);
+    fuse_stop_idle_monitor_thread(fuse);
     fuse_remove_signal_handlers(se);
     fuse_unmount_common(mountpoint, ch);
     fuse_destroy(fuse);
diff -r 8ba8f17842cd lib/mount.c
--- a/lib/mount.c	Tue Jun 06 10:16:37 2006 +0000
+++ b/lib/mount.c	Wed Jun 07 03:15:57 2006 +0200
@@ -183,7 +183,7 @@ static int receive_fd(int fd)
     return *(int*)CMSG_DATA(cmsg);
 }
 
-void fuse_kern_unmount(const char *mountpoint, int fd)
+void fuse_kern_unmount(const char *mountpoint, int fd, int lazy)
 {
     int pid;
 
@@ -208,10 +208,15 @@ void fuse_kern_unmount(const char *mount
         return;
 
     if(pid == 0) {
-        const char *argv[] =
-            { FUSERMOUNT_PROG, "-u", "-q", "-z", "--", mountpoint, NULL };
-
-        exec_fusermount(argv);
+        if(lazy) {
+            const char *argv[] =
+                { FUSERMOUNT_PROG, "-u", "-q", "-z", "--", mountpoint, NULL };
+            exec_fusermount(argv);
+        } else {
+            const char *argv[] =
+                { FUSERMOUNT_PROG, "-u", "-q", "--", mountpoint, NULL };
+            exec_fusermount(argv);
+        }
         _exit(1);
     }
     waitpid(pid, NULL, 0);
@@ -219,7 +224,7 @@ void fuse_kern_unmount(const char *mount
 
 void fuse_unmount_compat22(const char *mountpoint)
 {
-    fuse_kern_unmount(mountpoint, -1);
+    fuse_kern_unmount(mountpoint, -1, 1);
 }
 
 int fuse_mount_compat22(const char *mountpoint, const char *opts)
diff -r 8ba8f17842cd lib/mount_bsd.c
--- a/lib/mount_bsd.c	Tue Jun 06 10:16:37 2006 +0000
+++ b/lib/mount_bsd.c	Wed Jun 07 03:15:57 2006 +0200
@@ -203,7 +203,7 @@ void fuse_unmount_compat22(const char *m
     system(umount_cmd);
 }
 
-void fuse_kern_unmount(const char *mountpoint, int fd)
+void fuse_kern_unmount(const char *mountpoint, int fd, int lazy)
 {
     char *ep, *umount_cmd, dev[128];
     struct stat sbuf;
%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%


[my modifications]
%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
diff -r 03fef9398ce7 lib/fuse.c
--- a/lib/fuse.c	Wed Jun 07 10:31:52 2006 +0200
+++ b/lib/fuse.c	Wed Jun 07 11:12:05 2006 +0200
@@ -26,6 +26,7 @@
 #include <errno.h>
 #include <assert.h>
 #include <pthread.h>
+#include <signal.h>
 #include <sys/param.h>
 #include <sys/time.h>
 #include <sys/uio.h>
@@ -2198,7 +2199,8 @@ static void *idle_monitor(void *arg)
         
         if (idle_time >= (long)f->conf.idle_timeout) {
             if (f->conf.debug) {
-                printf("Idle timeout reached, trying to unmount %s\n", f->idle_monitor.mountpoint);
+                printf("Idle timeout reached, trying to unmount %s\n",
+                       f->idle_monitor.mountpoint ?: "??");
                 fflush(stdout);
             }
             fuse_try_unmount_nonlazy(f->idle_monitor.mountpoint, fuse_session_next_chan(f->se, NULL));
@@ -2219,15 +2221,12 @@ int fuse_start_idle_monitor_thread(struc
 int fuse_start_idle_monitor_thread(struct fuse *f, char *mountpoint)
 {
     int res;
+    sigset_t oldset;
+    sigset_t newset;
     
     if (f->conf.idle_timeout <= 0)
         return 0;
     
-    if (mountpoint == NULL) {
-        fprintf(stderr, "fuse: mountpoint not available for idle monitor\n");
-        return -1;
-    }
-
     if (f->conf.debug) {
         printf("Starting idle monitor thread, timeout=%ds\n", f->conf.idle_timeout);
         fflush(stdout);
@@ -2236,11 +2235,20 @@ int fuse_start_idle_monitor_thread(struc
     f->idle_monitor.mountpoint = mountpoint;
     f->idle_monitor.last_activity = time(0);
     f->idle_monitor.running = 1;
+
+    sigemptyset(&newset);
+    sigaddset(&newset, SIGTERM);
+    sigaddset(&newset, SIGINT);
+    sigaddset(&newset, SIGHUP);
+    sigaddset(&newset, SIGQUIT);
+    pthread_sigmask(SIG_BLOCK, &newset, &oldset);
     res = pthread_create(&f->idle_monitor.thread, 0, idle_monitor, (void *)f);
     if (res != 0) {
         fprintf(stderr, "fuse: unable to create idle monitor thread\n");
         return -1;
     }
+    pthread_detach(f->idle_monitor.thread);
+    pthread_sigmask(SIG_SETMASK, &oldset, NULL);
     
     return 0;
 }
%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%