Re: inotify back end for tail -f on linux

[Giuseppe Scrivano]

Jim Meyering <address@hidden> writes:

> I found an undesired change in functionality.
> inotify-enabled tail -F sometimes follows the file descriptor of a
> renamed file, which is contrary to the semantics of --follow=name.
> The non-inotify version follows the name, not the file descriptor in
> that case.

Oops, I was using a different events mask for re-added files.  I added
your test case to the tests suite now.

Thanks,
Giuseppe



This is the new version:

>From 106b1d32a6956f902109bffdf87e7897210c0e04 Mon Sep 17 00:00:00 2001
From: Giuseppe Scrivano <address@hidden>
Date: Tue, 2 Jun 2009 08:28:23 +0200
Subject: [PATCH] tail: Use inotify if it is available.

* NEWS: Document the new feature.
* m4/jm-macros.m4: Check if inotify is present.
* src/tail.c (tail_forever_inotify): New function.
(main): Use the inotify-based function, if possible.
* tests/Makefile.am: Add new tests for tail.
* tests/test-lib.sh (require_proc_pid_status_, get_process_status_):
New functions.
* tests/tail-2/pid: New file.
* tests/tail-2/wait: New file.
* tests/tail-2/tail-n0f: Refactor code into the test-lib.sh
require_proc_pid_status_ function.
---
 NEWS                  |    2 +
 m4/jm-macros.m4       |    5 +
 src/tail.c            |  275 +++++++++++++++++++++++++++++++++++++++++++++++--
 tests/Makefile.am     |    2 +
 tests/tail-2/pid      |   68 ++++++++++++
 tests/tail-2/tail-n0f |    9 +--
 tests/tail-2/wait     |  131 +++++++++++++++++++++++
 tests/test-lib.sh     |   16 +++
 8 files changed, 493 insertions(+), 15 deletions(-)
 create mode 100755 tests/tail-2/pid
 create mode 100755 tests/tail-2/wait

diff --git a/NEWS b/NEWS
index 29b09a0..cc61bdc 100644
--- a/NEWS
+++ b/NEWS
@@ -14,6 +14,8 @@ GNU coreutils NEWS                                    -*- 
outline -*-
   sort accepts a new option, --human-numeric-sort (-h): sort numbers
   while honoring human readable suffixes like KiB and MB etc.
 
+  tail uses inotify when possible.
+
 
 * Noteworthy changes in release 7.4 (2009-05-07) [stable]
 
diff --git a/m4/jm-macros.m4 b/m4/jm-macros.m4
index 90c55bf..f14d6a3 100644
--- a/m4/jm-macros.m4
+++ b/m4/jm-macros.m4
@@ -52,6 +52,11 @@ AC_DEFUN([coreutils_MACROS],
   # Used by sort.c.
   AC_CHECK_FUNCS_ONCE([nl_langinfo])
 
+  # Used by tail.c.
+  AC_CHECK_FUNCS([inotify_init],
+    [AC_DEFINE([HAVE_INOTIFY], [1],
+     [Define to 1 if you have usable inotify support.])])
+
   AC_CHECK_FUNCS_ONCE( \
     endgrent \
     endpwent \
diff --git a/src/tail.c b/src/tail.c
index 9d416e1..059ee82 100644
--- a/src/tail.c
+++ b/src/tail.c
@@ -21,7 +21,8 @@
 
    Original version by Paul Rubin <address@hidden>.
    Extensions by David MacKenzie <address@hidden>.
-   tail -f for multiple files by Ian Lance Taylor <address@hidden>.  */
+   tail -f for multiple files by Ian Lance Taylor <address@hidden>.
+   inotify back-end by Giuseppe Scrivano <address@hidden>.  */
 
 #include <config.h>
 
@@ -46,6 +47,11 @@
 #include "xstrtol.h"
 #include "xstrtod.h"
 
+#if HAVE_INOTIFY
+# include "hash.h"
+# include <sys/inotify.h>
+#endif
+
 /* The official name of this program (e.g., no `g' prefix).  */
 #define PROGRAM_NAME "tail"
 
@@ -125,8 +131,26 @@ struct File_spec
   /* The value of errno seen last time we checked this file.  */
   int errnum;
 
+#if HAVE_INOTIFY
+  /* The watch descriptor used by inotify.  */
+  int wd;
+
+  /* The parent directory watch descriptor.  It is used only
+   * when Follow_name is used.  */
+  int parent_wd;
+
+  /* Offset in NAME of the basename part.  */
+  size_t basename_start;
+#endif
 };
 
+#if HAVE_INOTIFY
+/* The events mask used with inotify on files.  This mask is not used on
+   directories.  */
+const uint32_t inotify_wd_mask = (IN_MODIFY | IN_ATTRIB | IN_DELETE_SELF
+                                  | IN_MOVE_SELF);
+#endif
+
 /* Keep trying to open a file even if it is inaccessible when tail starts
    or if it becomes inaccessible later -- useful only with -f.  */
 static bool reopen_inaccessible_files;
@@ -964,7 +988,7 @@ any_live_files (const struct File_spec *f, int n_files)
   return false;
 }
 
-/* Tail NFILES files forever, or until killed.
+/* Tail N_FILES files forever, or until killed.
    The pertinent information for each file is stored in an entry of F.
    Loop over each of them, doing an fstat to see if they have changed size,
    and an occasional open/fstat to see if any dev/ino pair has changed.
@@ -972,22 +996,22 @@ any_live_files (const struct File_spec *f, int n_files)
    while and try again.  Continue until the user interrupts us.  */
 
 static void
-tail_forever (struct File_spec *f, int nfiles, double sleep_interval)
+tail_forever (struct File_spec *f, int n_files, double sleep_interval)
 {
   /* Use blocking I/O as an optimization, when it's easy.  */
   bool blocking = (pid == 0 && follow_mode == Follow_descriptor
-                  && nfiles == 1 && ! S_ISREG (f[0].mode));
+                  && n_files == 1 && ! S_ISREG (f[0].mode));
   int last;
   bool writer_is_dead = false;
 
-  last = nfiles - 1;
+  last = n_files - 1;
 
   while (1)
     {
       int i;
       bool any_input = false;
 
-      for (i = 0; i < nfiles; i++)
+      for (i = 0; i < n_files; i++)
        {
          int fd;
          char const *name;
@@ -1087,7 +1111,7 @@ tail_forever (struct File_spec *f, int nfiles, double 
sleep_interval)
          f[i].size += bytes_read;
        }
 
-      if (! any_live_files (f, nfiles) && ! reopen_inaccessible_files)
+      if (! any_live_files (f, n_files) && ! reopen_inaccessible_files)
        {
          error (0, 0, _("no files remaining"));
          break;
@@ -1117,6 +1141,224 @@ tail_forever (struct File_spec *f, int nfiles, double 
sleep_interval)
     }
 }
 
+#if HAVE_INOTIFY
+
+static size_t
+wd_hasher (const void *entry, size_t tabsize)
+{
+  const struct File_spec *spec = entry;
+  return spec->wd % tabsize;
+}
+
+static bool
+wd_comparator (const void *e1, const void *e2)
+{
+  const struct File_spec *spec1 = e1;
+  const struct File_spec *spec2 = e2;
+  return spec1->wd == spec2->wd;
+}
+
+/* Tail N_FILES files forever, or until killed.
+   Check modifications using the inotify events system.  */
+
+static void
+tail_forever_inotify (int wd, struct File_spec *f, int n_files)
+{
+  unsigned int i;
+  unsigned int max_realloc = 3;
+  Hash_table *wd_table;
+
+  bool found_watchable = false;
+  size_t prev_wd;
+  size_t evlen = 0;
+  char *evbuf;
+  size_t evbuf_off = 0;
+  ssize_t len = 0;
+
+  wd_table = hash_initialize (n_files, NULL, wd_hasher, wd_comparator, NULL);
+  if (! wd_table)
+    xalloc_die ();
+
+  /* Add an inotify watch for each watched file.  If -F is specified then watch
+     its parent directory too, in this way when they re-appear we can add them
+     again to the watch list.  */
+  for (i = 0; i < n_files; i++)
+    {
+      if (!f[i].ignore)
+        {
+          size_t fnlen = strlen (f[i].name);
+          if (evlen < fnlen)
+            evlen = fnlen;
+
+          f[i].wd = 0;
+
+          if (follow_mode == Follow_name)
+            {
+              size_t dirlen = dir_len (f[i].name);
+              char prev = f[i].name[dirlen];
+              f[i].basename_start = last_component (f[i].name) - f[i].name;
+
+              f[i].name[dirlen] = '\0';
+
+               /* It's fine to add the same directory more than once.
+                  In that case the same watch descriptor is returned.  */
+              f[i].parent_wd = inotify_add_watch (wd, dirlen ? f[i].name : ".",
+                                                  (IN_CREATE | IN_MOVED_TO
+                                                   | IN_ATTRIB));
+
+              f[i].name[dirlen] = prev;
+
+              if (f[i].parent_wd < 0)
+                {
+                  error (0, errno, _("cannot watch parent directory of %s"),
+                         quote (f[i].name));
+                  continue;
+                }
+            }
+
+          f[i].wd = inotify_add_watch (wd, f[i].name, inotify_wd_mask);
+
+          if (f[i].wd < 0)
+            {
+              if (errno != f[i].errnum)
+                error (0, errno, _("cannot watch %s"), quote (f[i].name));
+              continue;
+            }
+
+          if (hash_insert (wd_table, &(f[i])) == NULL)
+            xalloc_die ();
+
+          if (follow_mode == Follow_name || f[i].wd)
+            found_watchable = true;
+        }
+    }
+
+  if (follow_mode == Follow_descriptor && !found_watchable)
+    return;
+
+  prev_wd = f[n_files - 1].wd;
+
+  evlen += sizeof (struct inotify_event) + 1;
+  evbuf = xmalloc (evlen);
+
+  /* Wait for inotify events and handle them.  Events on directories make sure
+     that watched files can be re-added when -F is used.
+     This loop sleeps on the `safe_read' call until a new event is notified.  
*/
+  while (1)
+    {
+      char const *name;
+      struct File_spec *fspec;
+      uintmax_t bytes_read;
+      struct stat stats;
+
+      struct inotify_event *ev;
+
+      if (len <= evbuf_off)
+        {
+          len = safe_read (wd, evbuf, evlen);
+          evbuf_off = 0;
+
+          if (len == SAFE_READ_ERROR && errno == EINVAL && max_realloc--)
+            {
+              len = 0;
+              evlen *= 2;
+              evbuf = xrealloc (evbuf, evlen);
+              continue;
+            }
+
+          if (len == SAFE_READ_ERROR)
+            error (EXIT_FAILURE, errno, _("error reading inotify event"));
+        }
+
+      ev = (struct inotify_event *) (evbuf + evbuf_off);
+      evbuf_off += sizeof (*ev) + ev->len;
+
+      if (ev->len)
+        {
+          for (i = 0; i < n_files; i++)
+            {
+              if (f[i].parent_wd == ev->wd &&
+                  STREQ (ev->name, f[i].name + f[i].basename_start))
+                break;
+            }
+
+          /* It is not a watched file.  */
+          if (i == n_files)
+            continue;
+
+          f[i].wd = inotify_add_watch (wd, f[i].name, inotify_wd_mask);
+
+          if (f[i].wd < 0)
+            {
+              error (0, errno, _("cannot watch %s"), quote (f[i].name));
+              continue;
+            }
+
+          fspec = &(f[i]);
+          if (hash_insert (wd_table, fspec) == NULL)
+            xalloc_die ();
+
+          if (follow_mode == Follow_name)
+            recheck (&(f[i]), false);
+        }
+      else
+        {
+          struct File_spec key;
+          key.wd = ev->wd;
+          fspec = hash_lookup (wd_table, &key);
+        }
+
+      if (! fspec)
+        continue;
+
+      if (ev->mask & (IN_ATTRIB | IN_DELETE_SELF | IN_MOVE_SELF))
+        {
+          if (ev->mask & (IN_DELETE_SELF | IN_MOVE_SELF))
+            {
+              inotify_rm_watch (wd, f[i].wd);
+              hash_delete (wd_table, &(f[i]));
+            }
+          if (follow_mode == Follow_name)
+            recheck (fspec, false);
+
+          continue;
+        }
+
+      name = pretty_name (fspec);
+
+      if (fstat (fspec->fd, &stats) != 0)
+        {
+          close_fd (fspec->fd, name);
+          fspec->fd = -1;
+          fspec->errnum = errno;
+          continue;
+        }
+
+      if (S_ISREG (fspec->mode) && stats.st_size < fspec->size)
+        {
+          error (0, 0, _("%s: file truncated"), name);
+          prev_wd = ev->wd;
+          xlseek (fspec->fd, stats.st_size, SEEK_SET, name);
+          fspec->size = stats.st_size;
+        }
+
+      if (ev->wd != prev_wd)
+        {
+          if (print_headers)
+            write_header (name);
+          prev_wd = ev->wd;
+        }
+
+      bytes_read = dump_remainder (name, fspec->fd, COPY_TO_EOF);
+      fspec->size += bytes_read;
+
+      if (fflush (stdout) != 0)
+        error (EXIT_FAILURE, errno, _("write error"));
+    }
+
+}
+#endif
+
 /* Output the last N_BYTES bytes of file FILENAME open for reading in FD.
    Return true if successful.  */
 
@@ -1691,7 +1933,24 @@ main (int argc, char **argv)
     ok &= tail_file (&F[i], n_units);
 
   if (forever)
-    tail_forever (F, n_files, sleep_interval);
+    {
+#if HAVE_INOTIFY
+      if (pid == 0)
+        {
+          int wd = inotify_init ();
+          if (wd < 0)
+            error (0, errno, _("inotify cannot be used, reverting to 
polling"));
+          else
+            {
+              tail_forever_inotify (wd, F, n_files);
+
+              /* The only way the above returns is upon failure.  */
+              exit (EXIT_FAILURE);
+            }
+        }
+#endif
+      tail_forever (F, n_files, sleep_interval);
+    }
 
   if (have_read_stdin && close (STDIN_FILENO) < 0)
     error (EXIT_FAILURE, errno, "-");
diff --git a/tests/Makefile.am b/tests/Makefile.am
index a0ed986..8d7a3b1 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -137,6 +137,7 @@ TESTS =                                             \
   misc/date-next-dow                           \
   misc/ptx-overrun                             \
   misc/xstrtol                                 \
+  tail-2/pid                                   \
   misc/od                                      \
   misc/mktemp                                  \
   misc/arch                                    \
@@ -243,6 +244,7 @@ TESTS =                                             \
   misc/unexpand                                        \
   misc/uniq                                    \
   misc/xattr                                   \
+  tail-2/wait                                  \
   chmod/c-option                               \
   chmod/equal-x                                        \
   chmod/equals                                 \
diff --git a/tests/tail-2/pid b/tests/tail-2/pid
new file mode 100755
index 0000000..ff2f099
--- /dev/null
+++ b/tests/tail-2/pid
@@ -0,0 +1,68 @@
+#!/bin/sh
+# Test the --pid option of tail.
+
+# Copyright (C) 2003, 2006-2009 Free Software Foundation, Inc.
+
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+if test "$VERBOSE" = yes; then
+  set -x
+  tail --version
+fi
+
+. $srcdir/test-lib.sh
+
+require_proc_pid_status_
+
+touch here || framework_failure
+
+
+fail=0
+
+# Use tail itself to create a background process.
+
+tail -f here &
+bg_pid=$!
+
+tail -s0.1 -f here --pid=$bg_pid &
+
+pid=$!
+
+sleep 0.5
+
+state=$(get_process_status_ $pid)
+
+if test -n "$state"; then
+  case $state in
+    S*) ;;
+    *) echo $0: process dead 1>&2; fail=1 ;;
+  esac
+  kill $pid
+fi
+
+kill $bg_pid
+
+sleep 0.5
+
+state=$(get_process_status_ $pid)
+
+if test -n "$state"; then
+  case $state in
+    S*) echo $0: process still active 1>&2; fail=1 ;;
+    *) ;;
+  esac
+  kill $pid
+fi
+
+Exit $fail
diff --git a/tests/tail-2/tail-n0f b/tests/tail-2/tail-n0f
index bf19cf5..322fddc 100755
--- a/tests/tail-2/tail-n0f
+++ b/tests/tail-2/tail-n0f
@@ -2,7 +2,7 @@
 # Make sure that `tail -n0 -f' and `tail -c0 -f' sleep
 # rather than doing what amounted to a busy-wait.
 
-# Copyright (C) 2003, 2006-2008 Free Software Foundation, Inc.
+# Copyright (C) 2003, 2006-2009 Free Software Foundation, Inc.
 
 # This program is free software: you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -28,12 +28,7 @@ fi
 
 . $srcdir/test-lib.sh
 
-sleep 2 &
-pid=$!
-sleep .5
-grep '^State:[  ]*[S]' /proc/$pid/status > /dev/null 2>&1 ||
-  skip_test_ "/proc/$pid/status: missing or 'different'"
-kill $pid
+require_proc_pid_status_
 
 touch empty || framework_failure
 echo anything > nonempty || framework_failure
diff --git a/tests/tail-2/wait b/tests/tail-2/wait
new file mode 100755
index 0000000..7eee8b1
--- /dev/null
+++ b/tests/tail-2/wait
@@ -0,0 +1,131 @@
+#!/bin/sh
+# Make sure that `tail -f' returns immediately if a file doesn't exist
+# while `tail -F' waits for it to appear.
+
+# Copyright (C) 2003, 2006-2009 Free Software Foundation, Inc.
+
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+if test "$VERBOSE" = yes; then
+  set -x
+  tail --version
+fi
+
+. $srcdir/test-lib.sh
+
+require_proc_pid_status_
+
+touch here || framework_failure
+touch k || framework_failure
+(touch not_accessible && chmod 0 not_accessible) || framework_failure
+
+fail=0
+
+tail -s0.1 -f not_here &
+pid=$!
+sleep .5
+state=$(get_process_status_ $pid)
+
+if test -n "$state"; then
+  case $state in
+    S*) echo $0: process still active 1>&2; fail=1 ;;
+    *) ;;
+  esac
+  kill $pid
+fi
+
+tail -s0.1 -f not_accessible &
+pid=$!
+sleep .5
+state=$(get_process_status_ $pid)
+
+if test -n "$state"; then
+  case $state in
+    S*) echo $0: process still active 1>&2; fail=1 ;;
+    *) ;;
+  esac
+  kill $pid
+fi
+
+(tail -s0.1 -f here 2>tail.err) &
+pid=$!
+sleep .5
+state=$(get_process_status_ $pid)
+
+if test -n "$state"; then
+  case $state in
+    S*) ;;
+    *)  echo $0: process died 1>&2; fail=1 ;;
+  esac
+  kill $pid
+fi
+
+
+# `tail -F' must wait in any case.
+
+(tail -s0.1 -F here 2>>tail.err) &
+pid=$!
+sleep .5
+state=$(get_process_status_ $pid)
+
+if test -n "$state"; then
+  case $state in
+    S*) ;;
+    *) echo $0: process died 1>&2; fail=1 ;;
+  esac
+  kill $pid
+fi
+
+tail -s0.1 -F not_accessible &
+pid=$!
+sleep .5
+state=$(get_process_status_ $pid)
+
+if test -n "$state"; then
+  case $state in
+    S*) ;;
+    *) echo $0: process died 1>&2; fail=1 ;;
+  esac
+  kill $pid
+fi
+
+tail -s0.1 -F not_here &
+pid=$!
+sleep .5
+state=$(get_process_status_ $pid)
+
+if test -n "$state"; then
+  case $state in
+    S*) ;;
+    *) echo $0: process died 1>&2; fail=1 ;;
+  esac
+  kill $pid
+fi
+
+test -s tail.err && fail=1
+
+tail -s.1 -F k > tail.out &
+pid=$!
+sleep .5
+mv k l
+sleep .5
+touch k
+mv k l
+sleep .5
+echo NO >> l
+sleep .5
+kill $pid
+test -s tail.out && fail=1
+
+Exit $fail
diff --git a/tests/test-lib.sh b/tests/test-lib.sh
index a765bd6..26c442e 100644
--- a/tests/test-lib.sh
+++ b/tests/test-lib.sh
@@ -122,6 +122,11 @@ uid_is_privileged_()
   esac
 }
 
+get_process_status_()
+{
+  sed -n '/^State:[    ]*\([[:alpha:]]\).*/s//\1/p' /proc/$1/status
+}
+
 # Convert an ls-style permission string, like drwxr----x and -rw-r-x-wx
 # to the equivalent chmod --mode (-m) argument, (=,u=rwx,g=r,o=x and
 # =,u=rw,g=rx,o=wx).  Ignore ACLs.
@@ -234,6 +239,17 @@ of group names or numbers.  E.g.,
   esac
 }
 
+# Is /proc/$PID/status supported?
+require_proc_pid_status_()
+{
+    sleep 2 &
+    local pid=$!
+    sleep .5
+    grep '^State:[      ]*[S]' /proc/$pid/status > /dev/null 2>&1 ||
+    skip_test_ "/proc/$pid/status: missing or 'different'"
+    kill $pid
+}
+
 # Does the current (working-dir) file system support sparse files?
 require_sparse_support_()
 {
-- 
1.6.3.1