Re: inotify back end for tail -f on linux

2009-06-25 Thread Pádraig Brady
Jim Meyering wrote:

 diff --git a/NEWS b/NEWS
 index d7695e4..754f9e2 100644
 --- a/NEWS
 +++ b/NEWS
 @@ -28,6 +28,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.
 +
 

That's a little terse. How about:

tail --follow uses inotify when possible to be more responsive
to file changes and also be more scalable when many files are monitored.

 diff --git a/src/tail.c b/src/tail.c
 index 9d416e1..059ee82 100644
 --- a/src/tail.c
 +++ b/src/tail.c
 @@ -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));

Do we need to warn here since the fallback is functionally equivalent?
It's OK if the error is rare, but I'm worried that errors might be issued
from /sys or /proc or if the user boots an old kernel, or whatever.
Well not that worried TBH, just mentioning it :)

cheers,
Pádraig.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: inotify back end for tail -f on linux

2009-06-25 Thread Jim Meyering
Pádraig Brady wrote:
 Jim Meyering wrote:
 diff --git a/NEWS b/NEWS
 index d7695e4..754f9e2 100644
 --- a/NEWS
 +++ b/NEWS
 @@ -28,6 +28,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.
 +


 That's a little terse. How about:

 tail --follow uses inotify when possible to be more responsive
 to file changes and also be more scalable when many files are monitored.

Sure.  Mentioning improved responsiveness is better.
What do you mean by more scalable?
It's currently still subject to the limit on number
of open file descriptors, which is tied to the number
of tailed files.

...
 +  if (wd  0)
 +error (0, errno, _(inotify cannot be used, reverting to 
 polling));

 Do we need to warn here since the fallback is functionally equivalent?
 It's OK if the error is rare, but I'm worried that errors might be issued
 from /sys or /proc or if the user boots an old kernel, or whatever.
 Well not that worried TBH, just mentioning it :)

If I'm using tail -f on a system that *should* support the inotify-enabled
code, I want to know right away (and why, hence errno) if that fails.

Otherwise, we'd have to wait for someone to notice the degraded
performance and to debug it to determine why it reverted to the old method.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: inotify back end for tail -f on linux

2009-06-25 Thread Pádraig Brady
Jim Meyering wrote:
 Pádraig Brady wrote:
 Jim Meyering wrote:
 diff --git a/NEWS b/NEWS
 index d7695e4..754f9e2 100644
 --- a/NEWS
 +++ b/NEWS
 @@ -28,6 +28,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.
 +

 That's a little terse. How about:

 tail --follow uses inotify when possible to be more responsive
 to file changes and also be more scalable when many files are monitored.
 
 Sure.  Mentioning improved responsiveness is better.
 What do you mean by more scalable?
 It's currently still subject to the limit on number
 of open file descriptors, which is tied to the number
 of tailed files.

Well I was thinking it would be more efficient than polling
as the number of files monitored was increased. How about:

tail --follow now uses inotify when possible, to be more responsive
to file changes and also be more efficient when many files are monitored.

 +  if (wd  0)
 +error (0, errno, _(inotify cannot be used, reverting to 
 polling));
 Do we need to warn here since the fallback is functionally equivalent?
 It's OK if the error is rare, but I'm worried that errors might be issued
 from /sys or /proc or if the user boots an old kernel, or whatever.
 Well not that worried TBH, just mentioning it :)
 
 If I'm using tail -f on a system that *should* support the inotify-enabled
 code, I want to know right away (and why, hence errno) if that fails.
 
 Otherwise, we'd have to wait for someone to notice the degraded
 performance and to debug it to determine why it reverted to the old method.

Cool. Just ensuring it was considered.

cheers,
Pádraig.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: inotify back end for tail -f on linux

2009-06-25 Thread Jim Meyering
Pádraig Brady wrote:
...
 Well I was thinking it would be more efficient than polling
 as the number of files monitored was increased. How about:

 tail --follow now uses inotify when possible, to be more responsive
 to file changes and also be more efficient when many files are monitored.

Sounds good.
s/also be// and here's a tweak to use active voice:

tail --follow now uses inotify when possible, to be more responsive
to file changes and more efficient when monitoring many files.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: inotify back end for tail -f on linux

2009-06-15 Thread Jim Meyering
Giuseppe Scrivano wrote:
...
 This is the new version:

 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.

Thanks for persevering.
I've made a couple of tiny changes:
  - log: first line: no capital.  no trailing period
  - one of tests: use TAB-space, not space-TAB in [...]

and finally have pushed the result.

From ae494d4be80df2aff50a2ac46fdc0c23de109ea3 Mon Sep 17 00:00:00 2001
From: Giuseppe Scrivano gscriv...@gnu.org
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 d7695e4..754f9e2 100644
--- a/NEWS
+++ b/NEWS
@@ -28,6 +28,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 p...@ocf.berkeley.edu.
Extensions by David MacKenzie d...@gnu.ai.mit.edu.
-   tail -f for multiple files by Ian Lance Taylor i...@airs.com.  */
+   tail -f for multiple files by Ian Lance Taylor i...@airs.com.
+   inotify back-end by Giuseppe Scrivano gscriv...@gnu.org.  */

 #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 

Re: inotify back end for tail -f on linux

2009-06-14 Thread Giuseppe Scrivano
Jim Meyering j...@meyering.net 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 gscriv...@gnu.org
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 p...@ocf.berkeley.edu.
Extensions by David MacKenzie d...@gnu.ai.mit.edu.
-   tail -f for multiple files by Ian Lance Taylor i...@airs.com.  */
+   tail -f for multiple files by Ian Lance Taylor i...@airs.com.
+   inotify back-end by Giuseppe Scrivano gscriv...@gnu.org.  */
 
 #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 

Re: inotify back end for tail -f on linux

2009-06-13 Thread Jim Meyering
Giuseppe Scrivano wrote:

 Jim, thank for the review.

 Jim Meyering j...@meyering.net writes:

 Perhaps prev_wd would be more accurate?

 I fixed it.  The same name is used in `tail_forever', that is why I used
 it, should it be changed in `tail_forever' too?


 Another regression:

 touch k; chmod 0 k; tail -F k

 fails to open k (as it must), but even
 when I run chmod u+r k in another window,
 the inotify-based version of tail fails to open it.

 I fixed it.

 This version includes, among other things, a refactoring of the new
 tests as previously reported.

Thanks again.
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.

For example:

echok; ./tail -F k  sleep .5; mv k l; sleep .5; touch k; mv k l; \
  sleep .5; echo NO  l
./tail: `k' has become inaccessible: No such file or directory
./tail: `k' has appeared;  following end of new file
NO

Contrast with this:

  echok; /usr/bin/tail -s.1 -F k  sleep .5; mv k l; sleep .5; touch k; mv k 
l;\
sleep .5; echo NO  l
  /usr/bin/tail: `k' has become inaccessible: No such file or directory


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: inotify back end for tail -f on linux

2009-06-11 Thread Jim Meyering
Giuseppe Scrivano wrote:
 The new version includes all these changes.

...
 +/* 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 last;

Looking through this again, I wonder what last means.
So I inspect the uses...

 +  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 ();
 +
 +  for (i = 0; i  n_files; i++)
 +{
 +  if (!f[i].ignore)
 +{
...
 +  f[i].wd = inotify_add_watch (wd, f[i].name,
 +   (IN_MODIFY | IN_ATTRIB
 +| IN_DELETE_SELF | IN_MOVE_SELF));
 +
 +  if (last == old_wd)
 +last = f[i].wd;

This comparison looks like it must use last uninitialized.

 +
...
 +}
 +}
 +
 +  if (follow_mode == Follow_descriptor  !found_watchable)
 +return;
 +
 +  last = f[n_files - 1].wd;

...
[more below]

Perhaps prev_wd would be more accurate?
last can mean final or preceding, so is rarely a good variable name,
due to that ambiguity.  It'd be good to write a comment describing
it, too.

Speaking of which, please add a comment saying what
each of the two main loops in that function does.

Another regression:

touch k; chmod 0 k; tail -F k

fails to open k (as it must), but even
when I run chmod u+r k in another window,
the inotify-based version of tail fails to open it.

When I repeat the experiment with non-inotify-based tail, it does
what I want:

$ touch k; chmod 0 k; tail -F k
tail: cannot open `k' for reading: Permission denied
tail: `k' has become accessible


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: inotify back end for tail -f on linux

2009-06-11 Thread Giuseppe Scrivano
Jim, thank for the review.

Jim Meyering j...@meyering.net writes:

 Perhaps prev_wd would be more accurate?

I fixed it.  The same name is used in `tail_forever', that is why I used
it, should it be changed in `tail_forever' too?


 Another regression:

 touch k; chmod 0 k; tail -F k

 fails to open k (as it must), but even
 when I run chmod u+r k in another window,
 the inotify-based version of tail fails to open it.

I fixed it.

This version includes, among other things, a refactoring of the new
tests as previously reported.

Cheers,
Giuseppe


From 28cc1f8e2582adb269babd5a6371fc5797f9ba52 Mon Sep 17 00:00:00 2001
From: Giuseppe Scrivano gscriv...@gnu.org
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|  268 +++--
 tests/Makefile.am |2 +
 tests/tail-2/pid  |   68 +
 tests/tail-2/tail-n0f |9 +--
 tests/tail-2/wait |  118 ++
 tests/test-lib.sh |   16 +++
 8 files changed, 473 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..460e255 100644
--- a/src/tail.c
+++ b/src/tail.c
@@ -21,7 +21,8 @@
 
Original version by Paul Rubin p...@ocf.berkeley.edu.
Extensions by David MacKenzie d...@gnu.ai.mit.edu.
-   tail -f for multiple files by Ian Lance Taylor i...@airs.com.  */
+   tail -f for multiple files by Ian Lance Taylor i...@airs.com.
+   inotify back-end by Giuseppe Scrivano gscriv...@gnu.org.  */
 
 #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,6 +131,17 @@ 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
 };
 
 /* Keep trying to open a file even if it is inaccessible when tail starts
@@ -964,7 +981,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 +989,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 +1104,7 @@ 

Re: inotify back end for tail -f on linux

2009-06-11 Thread Jim Meyering
Giuseppe Scrivano wrote:

 Jim, thank for the review.

 Jim Meyering j...@meyering.net writes:

 Perhaps prev_wd would be more accurate?

 I fixed it.  The same name is used in `tail_forever', that is why I used
 it, should it be changed in `tail_forever' too?


 Another regression:

 touch k; chmod 0 k; tail -F k

 fails to open k (as it must), but even
 when I run chmod u+r k in another window,
 the inotify-based version of tail fails to open it.

 I fixed it.

 This version includes, among other things, a refactoring of the new
 tests as previously reported.
...
Thanks for yet another round.
I'll look more closely tomorrow, but here are things I noticed right away:

Comment style.  You added comments like this.  Thanks!

   /* 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.  */

Please remove the leading '*'s:

   /* 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.  */


 diff --git a/tests/test-lib.sh b/tests/test-lib.sh
 index a765bd6..a9684fb 100644
 --- a/tests/test-lib.sh
 +++ b/tests/test-lib.sh
 @@ -122,6 +122,11 @@ uid_is_privileged_()
esac
  }

 +get_process_status_()
 +{
 +  echo $(sed -n '/^State:[ \t]*\([[:alpha:]]\)\(.*\)/s//\1/p' 
 /proc/$1/status)
 +}

No need for echo and the subshell.
Sadly, \t is not portable
No need for 2nd \(...\) group

get_process_status_()
{
  sed -n '/^State:[  ]*\([[:alpha:]]\).*/s//\1/p' /proc/$1/status
}

  # Convert an ls-style permission string, like drwxrx 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 21 ||
 +skip_test_ /proc/$pid/status: missing or 'different'

Indent a statement like that to make it easier to see that
it is conditional:

   grep '^State:[]*[S]' /proc/$pid/status  /dev/null 21 ||
   skip_test_ /proc/$pid/status: missing or 'different'

 +kill $pid
 +}
 +
  # Does the current (working-dir) file system support sparse files?
  require_sparse_support_()
  {


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: inotify back end for tail -f on linux

2009-06-07 Thread Jim Meyering
Giuseppe Scrivano wrote:
 Jim Meyering j...@meyering.net writes:

 Actually, there may be a nice way to allow --pid=PID to
 integrate seamlessly with inotify support.

 I think you can add an IN_DELETE_SELF inotify watch on the /proc/PID
 directory.  I suspect that every system with inotify support also
 has usable /proc/PID directories.  The only requirement seems to be
 that the desired directory be readable.

 I tried to watch /proc/$PID using inotify-tools and a few lines program
 that I wrote ad-hoc and it seems I can't monitor /proc correctly.
 I didn't receive IN_DELETE_SELF or any other message when the $PID
 process was killed.

 It seems I can't use inotify in this direct way to monitor processes
 status.

Rats.
After you mentioned that, I tried

  inotifywatch /proc/$pid
and
  inotifywatch /proc/$pid/mem

and confirmed your finding.  They don't work.
Then I read on lkml that it appears the linux kernel deliberately
does not enable inotify for /proc.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: inotify back end for tail -f on linux

2009-06-07 Thread Giuseppe Scrivano
Hello,

Thanks for your fast review.

Jim Meyering j...@meyering.net writes:

 Please rename to n_files, to be consistent with other variables
 in this file.  Besides, it's slightly more readable.

The same name is used in the tail_forever function, to keep consistency
between tail_forever and tail_forever_inotify, I changed nfiles in
tail_forever too.


 Typically we abbreviate with buf, not buff.
 How about evbuf and evbuf_off?

Ok, done.


 Whoops.
 You forgot to detect insertion failure.
 I'm adding __attribute__ ((__warn_unused_result__))
 to that function and a few others in gnulib's hash.h,
 so the compiler will detect this if it happens again.

Added.


 I have a slight preference for the shorter #if ...:

   #if HAVE_INOTIFY

I replaced all #ifdef HAVE_INOTIFY with the shorter form.


 For readability, I usually prefer not to use a single-line else block
 unless the then-block is also a one-liner.  Please do this instead:

if (wd  0)
  error (0, errno, _(inotify cannot be used, reverting to 
 polling));
else
  {
...
  }

Done.


 How about just this (like below)?

 touch here || framework_failure

 Each test is guaranteed to start in an empty directory.


I removed all the unuseful additional checks to don't modify existing
files, I didn't know that tests start in a clean temporary directory.


 Bonus points if you can test effectively (i.e., still avoid race conditions)
 without sleeping for long.  I consider 4-5 seconds long.

I couldn't remove `sleep's completely but I reduced their time.  I also
fixed some mistakes I previously did.


 No need to remove files.
 The entire temporary directory is removed automatically.
 Same below.

As above, I didn't know it was a temporary directory.  I removed these
`rm's too.


 With these two new uses, the above test has been repeated enough times
 now that it deserves to be factored into a require_proc_pid_status_
 function in test-lib.sh.
 []
 The above 3 lines are repeated far too many times.
 Please define a function in test-lib.sh, and then do e.g.,

I refactored the code as you suggested, also I changed tail-2/tail-n0f
to use this new function.


 - insufficient quoting; that will evoke a syntax error if $state is empty
 (btw, below, no quotes in case $state in... is fine)
 - use -n in place of ! -z;
 - I prefer to use test in place of [...]

I changed it.


 +test  $(wc -w  tail.err) eq 0  || fail=1

 This can be written more simply:

   test -s tail.err  fail=1

Done.


The new version includes all these changes.

Cheers,
Giuseppe




From 53bf397a3d34ccd2186197926c63b089477e5a62 Mon Sep 17 00:00:00 2001
From: Giuseppe Scrivano gscriv...@gnu.org
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: Added new tests for tail.
* tests/test-lib.sh: The require_proc_pid_status_ and
get_process_status_ functions were added.
* tests/tail-2/pid: New file.
* tests/tail-2/wait: New file.
* tests/tail-2/tail-n0f: Refactored code into the
test-lib.sh require_proc_pid_status_ function.
---
 NEWS  |2 +
 m4/jm-macros.m4   |5 +
 src/tail.c|  265 +++--
 tests/Makefile.am |2 +
 tests/tail-2/pid  |   68 +
 tests/tail-2/tail-n0f |9 +--
 tests/tail-2/wait |  118 ++
 tests/test-lib.sh |   18 
 8 files changed, 472 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..1de76b1 100644
--- a/src/tail.c
+++ b/src/tail.c
@@ -21,7 +21,8 @@
 
Original version by Paul Rubin p...@ocf.berkeley.edu.
Extensions by David MacKenzie d...@gnu.ai.mit.edu.
-   tail -f for multiple files by Ian Lance Taylor i...@airs.com.  */
+   tail -f for multiple files by Ian Lance Taylor 

Re: inotify back end for tail -f on linux

2009-06-07 Thread Jim Meyering
Giuseppe Scrivano wrote:
 The new version includes all these changes.

From 53bf397a3d34ccd2186197926c63b089477e5a62 Mon Sep 17 00:00:00 2001
 From: Giuseppe Scrivano gscriv...@gnu.org
 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: Added new tests for tail.
 * tests/test-lib.sh: The require_proc_pid_status_ and
 get_process_status_ functions were added.
 * tests/tail-2/pid: New file.
 * tests/tail-2/wait: New file.
 * tests/tail-2/tail-n0f: Refactored code into the
 test-lib.sh require_proc_pid_status_ function.

Thanks.

As mentioned in HACKING,
http://git.savannah.gnu.org/cgit/coreutils.git/plain/HACKING:

When writing prose (documentation, comments, log entries), use an
active voice, not a passive one.  I.e., say print the frobnozzle,
not the frobnozzle will be printed.

i.e., s/Added/Add/, etc. :

* 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.

I'll comment on the code Monday.
Quick hint: test-lib.sh functions (and functions in general) should
avoid modifying variables like $pid and $* that may be in use by a caller.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: inotify back end for tail -f on linux

2009-06-07 Thread Jim Meyering
Giuseppe Scrivano wrote:
 The new version includes all these changes.

Please merge this into your changes:

diff --git a/tests/Makefile.am b/tests/Makefile.am
index c108356..e4c7f22 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/wait  \
   misc/od  \
   misc/mktemp  \
   misc/arch\
@@ -243,6 +244,7 @@ TESTS = \
   misc/unexpand\
   misc/uniq\
   misc/xattr   \
+  tail-2/pid   \
   chmod/c-option   \
   chmod/equal-x\
   chmod/equals \
@@ -419,8 +421,6 @@ TESTS = \
   tail-2/big-4gb   \
   tail-2/proc-ksyms\
   tail-2/start-middle  \
-  tail-2/pid   \
-  tail-2/wait  \
   touch/dangling-symlink   \
   touch/dir-1  \
   touch/fail-diag  \

Why?
Because the two new tests were started late enough
that they could make a parallel make check take longer.
Here's a comment from tests/Makefile.am:

  # Put tests that sleep early, but not all together, so in parallel builds
  # they share time with tests that burn CPU, not with others that sleep.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: inotify back end for tail -f on linux

2009-06-06 Thread Jim Meyering
Giuseppe Scrivano wrote:
 Jim Meyering j...@meyering.net writes:

 No test is too small or too obvious to add ;-)

 Adding this simple test would be great.
 Thanks!

Hi Giuseppe,

 I found another problem.  The inotify version can't be used when --pid
 is specified.  There are other inotify events that I want to investigate
 better if they can be used with --pid and in this case we can add them
 later when the inotify back end is stable enough for me to break it
 again :)  What do you think?

Good catch.

 This is the updated version, I integrated your test-case in the
 tests/tail-2/wait file and I added a new file (tests/tail-2/pid) with
 additional tests for the --pid option.

Thanks!

From f0824b8b79c1fb22e9a48cbf831a87433fc4d3e8 Mon Sep 17 00:00:00 2001
 From: Giuseppe Scrivano gscriv...@gnu.org
 Date: Tue, 2 Jun 2009 08:28:23 +0200
 Subject: [PATCH] tail: Use inotify if it is available.

...
 +  tail uses inotify if it is present.

We can be slightly more precise:

 tail uses inotify when possible

  * Noteworthy changes in release 7.4 (2009-05-07) [stable]

 diff --git a/configure.ac b/configure.ac
 index 4eb640e..a63ac0c 100644
 --- a/configure.ac
 +++ b/configure.ac
 @@ -410,6 +410,8 @@ AM_GNU_GETTEXT_VERSION([0.15])
  # For a test of uniq: it uses the $LOCALE_FR envvar.
  gt_LOCALE_FR

 +AC_CHECK_FUNCS([inotify_init], [AC_DEFINE([HAVE_INOTIFY], [1], [Check for 
 libinotify])])
 +

We like to avoid adding lines that occupy more than 80 columns.
How about something like this instead?
The comment is the one that ends up in lib/config.h.
This is not checking for a library.

  AC_CHECK_FUNCS([inotify_init],
[AC_DEFINE([HAVE_INOTIFY], [1],
  [Define to 1 if you have usable inotify support.])])

Oh, and it'd be better to put that test in m4/jm-macros.m4,
not in configure.ac.

  AC_CONFIG_FILES(
Makefile
doc/Makefile
...
 +  if (len = evbuff_off)
 +{
 +  len = read (wd, evbuff, evlen);

Please use safe_read here.
Then you can drop the EINTR test/block below.

 +  evbuff_off = 0;
 +
 +  if (len = 0  errno == EINVAL  max_realloc--)
 +{
 +  len = 0;
 +  evlen *= 2;
 +  evbuff = xrealloc (evbuff, evlen);
 +  continue;
 +}
 +
 +  if (len = 0  errno == EINTR)
 +{
 +  len = 0;
 +  continue;
 +}
 +
 +  if (len = 0)
 +error (EXIT_FAILURE, errno, _(error reading inotify event));
 +}
...

if (forever)
 -tail_forever (F, n_files, sleep_interval);
 +{
 +#ifdef HAVE_INOTIFY
 +  int wd = inotify_init ();
 +  if (wd  0  pid == 0)
 +tail_forever_inotify (wd, F, n_files);
 +#endif
 +  tail_forever (F, n_files, sleep_interval);
 +}

if (have_read_stdin  close (STDIN_FILENO)  0)
  error (EXIT_FAILURE, errno, -);

Thanks.
I had to make some small changes:

  - chmod a+x tests/tail-2/{pid,wait}
 That works around this make check failure:

--- t1  2009-06-06 09:50:14.0 +0200
+++ t2  2009-06-06 09:50:17.0 +0200
@@ -359,11 +359,9 @@
 tail-2/assert-2
 tail-2/big-4gb
 tail-2/infloop-1
-tail-2/pid
 tail-2/proc-ksyms
 tail-2/start-middle
 tail-2/tail-n0f
-tail-2/wait
 touch/dangling-symlink
 touch/dir-1
 touch/empty-file
make[2]: *** [vc_exe_in_TESTS] Error 1
make[2]: *** Waiting for unfinished jobs

--
Notice how when tail_forever_inotify returns,
execution continues into tail_forever.  It shouldn't.

--
Notice how this waits, as it should:

$ touch k; chmod 0 k; ./tail --pid $$ -F k
./tail: cannot open `k' for reading: Permission denied

yet this exits immediately:
[also, it'd be nice not to give two diagnostics about the same problem]

$ touch k; chmod 0 k; ./tail -F k
./tail: cannot open `k' for reading: Permission denied
./tail: cannot watch `k': Permission denied
[Exit 1]
--

I haven't fixed that last problem.
Please fold the following patch into yours for the next iteration.


From 2a200db8a5f3b36d23b179bc4343c647957d6d13 Mon Sep 17 00:00:00 2001
From: Jim Meyering meyer...@redhat.com
Date: Sat, 6 Jun 2009 10:12:02 +0200
Subject: [PATCH] tail: minor changes

* src/tail.c (main): Don't call inotify_init when waiting for a PID.
[HAVE_INOTIFY]: Exit nonzero whenever tail_forever_inotify returns.
Change wd  0 to 0 = wd, since 0 is a valid file descriptor.
---
 src/tail.c |   12 +---
 1 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/src/tail.c b/src/tail.c
index 6d5f870..4d9270f 100644
--- a/src/tail.c
+++ b/src/tail.c
@@ -1929,9 +1929,15 @@ main (int argc, char **argv)
   if (forever)
 {
 #ifdef HAVE_INOTIFY
-  int wd = inotify_init ();
-  if (wd  0  pid == 0)
-  

Re: inotify back end for tail -f on linux

2009-06-06 Thread Giuseppe Scrivano
Hi Jim,

Jim Meyering j...@meyering.net writes:

 Oh, and it'd be better to put that test in m4/jm-macros.m4,
 not in configure.ac.

Ok, I'll move it.


  AC_CONFIG_FILES(
Makefile
doc/Makefile
 ...
 +  if (len = evbuff_off)
 +{
 +  len = read (wd, evbuff, evlen);

 Please use safe_read here.
 Then you can drop the EINTR test/block below.

Ok.

 --
 Notice how this waits, as it should:

 $ touch k; chmod 0 k; ./tail --pid $$ -F k
 ./tail: cannot open `k' for reading: Permission denied

 yet this exits immediately:
 [also, it'd be nice not to give two diagnostics about the same problem]

 $ touch k; chmod 0 k; ./tail -F k
 ./tail: cannot open `k' for reading: Permission denied
 ./tail: cannot watch `k': Permission denied
 [Exit 1]
 --

 I haven't fixed that last problem.

Thanks, I'll take a look at it.


 Please fold the following patch into yours for the next iteration.


 From 2a200db8a5f3b36d23b179bc4343c647957d6d13 Mon Sep 17 00:00:00 2001
 From: Jim Meyering meyer...@redhat.com
 Date: Sat, 6 Jun 2009 10:12:02 +0200
 Subject: [PATCH] tail: minor changes

 * src/tail.c (main): Don't call inotify_init when waiting for a PID.
 [HAVE_INOTIFY]: Exit nonzero whenever tail_forever_inotify returns.
 Change wd  0 to 0 = wd, since 0 is a valid file descriptor.
 ---
  src/tail.c |   12 +---
  1 files changed, 9 insertions(+), 3 deletions(-)

 diff --git a/src/tail.c b/src/tail.c
 index 6d5f870..4d9270f 100644
 --- a/src/tail.c
 +++ b/src/tail.c
 @@ -1929,9 +1929,15 @@ main (int argc, char **argv)
if (forever)
  {
  #ifdef HAVE_INOTIFY
 -  int wd = inotify_init ();
 -  if (wd  0  pid == 0)
 -tail_forever_inotify (wd, F, n_files);
 +  if (pid == 0)
 +{
 +  int wd = inotify_init ();
 +  if (0 = wd)
 +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);
  }

I think the exit (EXIT_FAILURE) should be included in the if:

if (0 = wd)
  {
tail_forever_inotify (wd, F, n_files);
exit (EXIT_FAILURE);
  }

The inotify_init () call can fail if the inotify support is not present
at runtime, and in this case it is a good idea to use polling instead of
exit immediately.  Do you agree?

Regards,
Giuseppe


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: inotify back end for tail -f on linux

2009-06-06 Thread Jim Meyering
Giuseppe Scrivano wrote:
...
 I think the exit (EXIT_FAILURE) should be included in the if:

 if (0 = wd)
   {
 tail_forever_inotify (wd, F, n_files);
 exit (EXIT_FAILURE);
   }

 The inotify_init () call can fail if the inotify support is not present
 at runtime, and in this case it is a good idea to use polling instead of
 exit immediately.  Do you agree?

Yes.  Thanks.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: inotify back end for tail -f on linux

2009-06-06 Thread Jim Meyering
Giuseppe Scrivano wrote:
...
 The inotify_init () call can fail if the inotify support is not present
 at runtime, and in this case it is a good idea to use polling instead of
 exit immediately.

When inotify_init fails, I'd like to know that,
and that tail is reverting to the old polling-based code.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: inotify back end for tail -f on linux

2009-06-06 Thread Jim Meyering
Giuseppe Scrivano wrote:
 I found another problem.  The inotify version can't be used when --pid
 is specified.  There are other inotify events that I want to investigate
 better if they can be used with --pid and in this case we can add them
 later when the inotify back end is stable enough for me to break it
 again :)  What do you think?

Actually, there may be a nice way to allow --pid=PID to
integrate seamlessly with inotify support.

I think you can add an IN_DELETE_SELF inotify watch on the /proc/PID
directory.  I suspect that every system with inotify support also
has usable /proc/PID directories.  The only requirement seems to be
that the desired directory be readable.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: inotify back end for tail -f on linux

2009-06-06 Thread Giuseppe Scrivano
Jim Meyering j...@meyering.net writes:

 Actually, there may be a nice way to allow --pid=PID to
 integrate seamlessly with inotify support.

 I think you can add an IN_DELETE_SELF inotify watch on the /proc/PID
 directory.  I suspect that every system with inotify support also
 has usable /proc/PID directories.  The only requirement seems to be
 that the desired directory be readable.

I tried to watch /proc/$PID using inotify-tools and a few lines program
that I wrote ad-hoc and it seems I can't monitor /proc correctly.
I didn't receive IN_DELETE_SELF or any other message when the $PID
process was killed.

It seems I can't use inotify in this direct way to monitor processes
status.

Cheers,
Giuseppe


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: inotify back end for tail -f on linux

2009-06-06 Thread Giuseppe Scrivano
Hi Jim,

Jim Meyering j...@meyering.net writes:

 I haven't fixed that last problem.
 Please fold the following patch into yours for the next iteration.

in this version I included all the problems you reported me, the touch
k; chmod 0 k; ./tail -F k test case is included in the tests suite
now.

Regards,
Giuseppe



From 529e1c2ba2a74168995de9ae7f8b9efa0d2d71c4 Mon Sep 17 00:00:00 2001
From: Giuseppe Scrivano gscriv...@gnu.org
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: Added new tests for tail.
* tests/tail-2/pid: New file.
* tests/tail-2/wait: New file.
---
 NEWS  |2 +
 m4/jm-macros.m4   |5 +
 src/tail.c|  248 -
 tests/Makefile.am |2 +
 tests/tail-2/pid  |   81 +
 tests/tail-2/wait |  146 +++
 6 files changed, 483 insertions(+), 1 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..3a7b022 100644
--- a/src/tail.c
+++ b/src/tail.c
@@ -46,6 +46,11 @@
 #include xstrtol.h
 #include xstrtod.h
 
+#ifdef 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,6 +130,17 @@ struct File_spec
   /* The value of errno seen last time we checked this file.  */
   int errnum;
 
+#ifdef 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
 };
 
 /* Keep trying to open a file even if it is inaccessible when tail starts
@@ -1117,6 +1133,219 @@ tail_forever (struct File_spec *f, int nfiles, double 
sleep_interval)
 }
 }
 
+#ifdef 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 NFILES files forever, or until killed.
+   Check modifications using the inotify events system.  */
+static void
+tail_forever_inotify (int wd, struct File_spec *f, int nfiles)
+{
+  unsigned int i;
+  unsigned int max_realloc = 3;
+  Hash_table *wd_table;
+
+  bool found_watchable = false;
+  size_t last;
+  size_t evlen = 0;
+  char *evbuff;
+  size_t evbuff_off = 0;
+  ssize_t len = 0;
+
+  wd_table = hash_initialize (nfiles, NULL, wd_hasher, wd_comparator, NULL);
+  if (! wd_table)
+xalloc_die ();
+
+  for (i = 0; i  nfiles; i++)
+{
+  if (!f[i].ignore)
+{
+  int old_wd = f[i].wd;
+  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);
+
+  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;
+}
+}
+
+  old_wd = f[i].wd;
+  f[i].wd 

Re: inotify back end for tail -f on linux

2009-06-06 Thread Jim Meyering
Giuseppe Scrivano wrote:
 Jim Meyering j...@meyering.net writes:

 I haven't fixed that last problem.
 Please fold the following patch into yours for the next iteration.

 in this version I included all the problems you reported me, the touch
 k; chmod 0 k; ./tail -F k test case is included in the tests suite
 now.
...
 +static void
 +tail_forever_inotify (int wd, struct File_spec *f, int nfiles)

Please rename to n_files, to be consistent with other variables
in this file.  Besides, it's slightly more readable.

 +{
 +  unsigned int i;
 +  unsigned int max_realloc = 3;
 +  Hash_table *wd_table;
 +
 +  bool found_watchable = false;
 +  size_t last;
 +  size_t evlen = 0;
 +  char *evbuff;
 +  size_t evbuff_off = 0;

Typically we abbreviate with buf, not buff.
How about evbuf and evbuf_off?

...
 +  hash_insert (wd_table, fspec);

Whoops.
You forgot to detect insertion failure.
I'm adding __attribute__ ((__warn_unused_result__))
to that function and a few others in gnulib's hash.h,
so the compiler will detect this if it happens again.

 +  if (follow_mode == Follow_name)
 +recheck ((f[i]), false);
 +}
...

if (forever)
 -tail_forever (F, n_files, sleep_interval);
 +{
 +#ifdef HAVE_INOTIFY

I have a slight preference for the shorter #if ...:

  #if HAVE_INOTIFY

 +  if (pid == 0)
 +{
 +  int wd = inotify_init ();
 +  if (0 = wd)
 +{
 +  tail_forever_inotify (wd, F, n_files);
 +
 +  /* The only way the above returns is upon failure.  */
 +  exit (EXIT_FAILURE);
 +}
 +  else
 +error (0, errno, _(inotify cannot be used, reverting to 
 polling));

For readability, I usually prefer not to use a single-line else block
unless the then-block is also a one-liner.  Please do this instead:

   if (wd  0)
 error (0, errno, _(inotify cannot be used, reverting to 
polling));
   else
 {
   ...
 }

...
 +sleep 2 
 +pid=$!
 +sleep .5
 +grep '^State:[]*[S]' /proc/$pid/status  /dev/null 21 ||
 +  skip_test_ /proc/$pid/status: missing or 'different'
 +kill $pid
 +
 +if [ ! -e here ]; then
 +  touch here || framework_failure
 +fi

How about just this (like below)?

touch here || framework_failure

Each test is guaranteed to start in an empty directory.

 +fail=0
 +
 +
 +# Use tail itself to create a background process.
 +
 +tail -f here 
 +bg_pid=$!
 +
 +tail -f here --pid=$bg_pid 
 +
 +pid=$!
 +
 +sleep 2

Bonus points if you can test effectively (i.e., still avoid race conditions)
without sleeping for long.  I consider 4-5 seconds long.

 +set _ `sed -n '/^State:[  ]*\([^  ]\)/s//\1/p' /proc/$pid/status`
 +shift # Remove the leading `_'.
 +state=$1
 +
 +if [ ! -z $state ]; then
 +  case $state in
 +S*) ;;
 +*) echo $0: process dead 12; fail=1 ;;
 +  esac
 +  kill $pid
 +fi
 +
 +sleep 2
 +
 +set _ `sed -n '/^State:[  ]*\([^  ]\)/s//\1/p' /proc/$pid/status`
 +shift
 +state=$1
 +
 +if [ ! -z $state ]; then
 +  case $state in
 +*) ;;
 +S*) echo $0: process still active 12; fail=1 ;;
 +  esac
 +  kill $pid
 +fi
 +
 +
 +kill $bg_pid
 +rm here

No need to remove files.
The entire temporary directory is removed automatically.
Same below.

 diff --git a/tests/tail-2/wait b/tests/tail-2/wait
...
 +if test $VERBOSE = yes; then
 +  set -x
 +  tail --version
 +fi
 +
 +. $srcdir/test-lib.sh
 +
 +sleep 2 
 +pid=$!
 +sleep .5
 +grep '^State:[]*[S]' /proc/$pid/status  /dev/null 21 ||
 +  skip_test_ /proc/$pid/status: missing or 'different'

With these two new uses, the above test has been repeated enough times
now that it deserves to be factored into a require_proc_pid_status_
function in test-lib.sh.

 +kill $pid
 +
 +touch here || framework_failure
 +
 +(touch not_accessible  chmod 0 not_accessible) || framework_failure
 +
 +if [ -e not_here ]; then
 +  rm -f not_here || framework_failure
 +fi

No need for the -e not_here test,
since it starts in an empty directory.

 +fail=0
 +
 +rm -f  tail.err
 +
 +(tail -f not_here 2 tail.err) 
 +pid=$!
 +sleep .5
 +set _ `sed -n '/^State:[  ]*\([^  ]\)/s//\1/p' /proc/$pid/status`
 +shift # Remove the leading `_'.
 +state=$1

The above 3 lines are repeated far too many times.
Please define a function in test-lib.sh, and then do e.g.,

state=$(get_process_state_ $pid)

 +if [ ! -z $state ]; then

- insufficient quoting; that will evoke a syntax error if $state is empty
(btw, below, no quotes in case $state in... is fine)
- use -n in place of ! -z;
- I prefer to use test in place of [...]

   if test -n $state; then

 +  case $state in
 +*) ;;
 +S*) echo $0: process still active 12; fail=1 ;;
 +  esac
 +  kill $pid
 +fi
...

 +test  $(wc -w  tail.err) eq 0  || fail=1

This can be written more simply:

  test -s tail.err  fail=1

 +rm -f here
 +rm -f tail.err
 +rm -f not_accessible
 +
 +
 +Exit $fail



Re: inotify back end for tail -f on linux

2009-06-04 Thread Giuseppe Scrivano
Hi Jim,


Jim Meyering j...@meyering.net writes:

 No test is too small or too obvious to add ;-)

 Adding this simple test would be great.
 Thanks!

I found another problem.  The inotify version can't be used when --pid
is specified.  There are other inotify events that I want to investigate
better if they can be used with --pid and in this case we can add them
later when the inotify back end is stable enough for me to break it
again :)  What do you think?

This is the updated version, I integrated your test-case in the
tests/tail-2/wait file and I added a new file (tests/tail-2/pid) with
additional tests for the --pid option.

Regards,
Giuseppe




From f0824b8b79c1fb22e9a48cbf831a87433fc4d3e8 Mon Sep 17 00:00:00 2001
From: Giuseppe Scrivano gscriv...@gnu.org
Date: Tue, 2 Jun 2009 08:28:23 +0200
Subject: [PATCH] tail: Use inotify if it is available.

* NEWS: Document the new feature.
* configure.ac: Check if inotify is present.
* src/tail.c (tail_forever_inotify): New function.
(main): Use the inotify-based function, if possible.
* tests/Makefile.am: Added new tests for tail.
* tests/tail-2/pid: New file.
* tests/tail-2/wait: New file.
---
 NEWS  |2 +
 configure.ac  |2 +
 src/tail.c|  245 -
 tests/Makefile.am |2 +
 tests/tail-2/pid  |   81 ++
 tests/tail-2/wait |  114 +
 6 files changed, 445 insertions(+), 1 deletions(-)
 create mode 100644 tests/tail-2/pid
 create mode 100644 tests/tail-2/wait

diff --git a/NEWS b/NEWS
index 29b09a0..c5a2ef5 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 if it is present.
+
 
 * Noteworthy changes in release 7.4 (2009-05-07) [stable]
 
diff --git a/configure.ac b/configure.ac
index 4eb640e..a63ac0c 100644
--- a/configure.ac
+++ b/configure.ac
@@ -410,6 +410,8 @@ AM_GNU_GETTEXT_VERSION([0.15])
 # For a test of uniq: it uses the $LOCALE_FR envvar.
 gt_LOCALE_FR
 
+AC_CHECK_FUNCS([inotify_init], [AC_DEFINE([HAVE_INOTIFY], [1], [Check for 
libinotify])])
+
 AC_CONFIG_FILES(
   Makefile
   doc/Makefile
diff --git a/src/tail.c b/src/tail.c
index 9d416e1..6d5f870 100644
--- a/src/tail.c
+++ b/src/tail.c
@@ -46,6 +46,11 @@
 #include xstrtol.h
 #include xstrtod.h
 
+#ifdef 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,6 +130,17 @@ struct File_spec
   /* The value of errno seen last time we checked this file.  */
   int errnum;
 
+#ifdef 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
 };
 
 /* Keep trying to open a file even if it is inaccessible when tail starts
@@ -1117,6 +1133,226 @@ tail_forever (struct File_spec *f, int nfiles, double 
sleep_interval)
 }
 }
 
+#ifdef 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 NFILES files forever, or until killed.
+   Check modifications using the inotify events system.  */
+static void
+tail_forever_inotify (int wd, struct File_spec *f, int nfiles)
+{
+  unsigned int i;
+  unsigned int max_realloc = 3;
+  Hash_table *wd_table;
+
+  bool found_watchable = false;
+  size_t last;
+  size_t evlen = 0;
+  char *evbuff;
+  size_t evbuff_off = 0;
+  ssize_t len = 0;
+
+  wd_table = hash_initialize (nfiles, NULL, wd_hasher, wd_comparator, NULL);
+  if (! wd_table)
+xalloc_die ();
+
+  for (i = 0; i  nfiles; 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);
+
+  f[i].name[dirlen] = prev;
+
+  if (f[i].parent_wd  0)
+{
+

Re: inotify back end for tail -f on linux

2009-06-04 Thread Jim Meyering
Giuseppe Scrivano wrote:
 Jim Meyering j...@meyering.net writes:
 However, my first manual test provoked this surprising diagnostic:

   $ touch k  src/tail -F k 
   src/tail: cannot watch parent directory of `k': No such file or directory

 The following patch fixes that.
 Go ahead and fold it into yours.

 Thank you.  Do you want me to write tests for such simple use cases?
 It may seem not very helpful but in future it can help to find
 regressions as in this case.

No test is too small or too obvious to add ;-)

Adding this simple test would be great.
Thanks!


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: inotify back end for tail -f on linux

2009-06-03 Thread Jim Meyering
Giuseppe Scrivano wrote:
 Thank you for all your suggestions, I'll keep them in mind.  I used your
 modified patch as my starting point.  I fixed the segfault problem and I
 added a new test case.

Thanks.  That's a definite improvement ;-)

However, my first manual test provoked this surprising diagnostic:

  $ touch k  src/tail -F k 
  src/tail: cannot watch parent directory of `k': No such file or directory

The following patch fixes that.
Go ahead and fold it into yours.

diff --git a/src/tail.c b/src/tail.c
index 4b2b59d..e71b080 100644
--- a/src/tail.c
+++ b/src/tail.c
@@ -1185,12 +1185,11 @@ tail_forever_inotify (int wd, struct File_spec *f, int 
nfiles)
   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';

   /* Do not care if the directory was already added, in this
  case the same watch descriptor will be returned.  */
-  f[i].parent_wd = inotify_add_watch (wd, f[i].name,
+  f[i].parent_wd = inotify_add_watch (wd, dirlen ? f[i].name : .,
   IN_CREATE | IN_MOVED_TO);

   f[i].name[dirlen] = prev;

Also, please adjust that comment:

   /* It's fine to add the same directory more than once.
  In that case the same watch descriptor is returned.  */


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: inotify back end for tail -f on linux

2009-06-02 Thread Jim Meyering
Giuseppe Scrivano wrote:

 Jim Meyering j...@meyering.net writes:

 If we add inotify support, I'd like it to work for
 both tail-by-FD and tail-by-name.

 You can watch the parent directory for e.g., rename and creation events.
 Log rotation is so common, that it'd be a shame not to be able to take
 advantage of inotify when using tail -F 

 For starters, you needn't worry about recovering from parent-dir-removal.
 I think that parent removal is rare enough in practice that we won't
 need to deal with it for some time.

 Eventually, the code could monitor the longest parent prefix that *does* 
 exist.
 When the missing subdir reappears, monitor $parent/$subdir, etc.
 However, there's no need to do that now.

 Considering only the parent directory makes things easier but I don't
 think that in future it will be difficult to remove completely this
 limit.

 The version I attached is a bit more complex than the last one.  Since
 the watch descriptors are added at any time, I can't do any consideration
 on their range and I am using a hash table to access the files
 information in a costant time.

 I need to stress my patch better but it seems already to cover all cases
 you showed me.


 I haven't looked closely at the code below or even tried it,
 but here are a few superficial comments:

 Thank you.  I included them except this one:

 +  if (!len  errno == EINTR)
 +continue;
 +
 +  if (!len)
 +error (EXIT_FAILURE, errno, _(error reading inotify event));

 This function must not exit upon a failure that is specific
 to a single file.  Just diagnose it and continue with any other files.
 Of course, if there are no other usable file descriptors, *then*
 you will need to exit the loop -- since this is currently used only
 when tailing-by-FD.

 You would not exit a similar loop when tailing by name.

 I added some code to recover from errors when possible.  I don't think
 there are other recoverable cases but if we want to force it to re-try
 read then we will need to avoid an end-less loop when there is an
 unrecoverable situation.  What do you think?

Thanks for persevering.
A few suggestions:
  - configure with --enable-gcc-warnings.
  That exposed two unused-variables in your patch.
  - always run make syntax-check

That exposed two easily-fixed problems:
  - use of !strcmp that should be a use of STREQ
  - unnecessary inclusion of dirname.h

src/tail.c:51:# include dirname.h
maint.mk: the above are already included via system.h

src/tail.c:1276: !strcmp (ev-name, f[i].name + f[i].basename_start))
maint.mk: use STREQ in place of the above uses of strcmp
make: *** [sc_prohibit_strcmp] Error 1

Note this is the second time that my attempt to apply
your patch with git am FILE failed.
That seems to be due to your using an out of date version
of tail.c, since your base version has only 2008 listed on
the Copyright line, while the latest has 2008-2009.

I've also adjusted indentation, spacing-around-operators, added curly
braces around a single-stmt (but three-line) loop body, and
changed a variable name (s/watched/found_watchable/) and types.
In general: avoid int. i.e., prefer unsigned types for counters,
bool for boolean.

Please base your future changes on the patch below.
FYI, while we wait for savannah.gnu.org to recover from
its disk failures, you can use this up to date mirror:

  http://repo.or.cz/w/coreutils.git

First, save the patch below in FILE, and change the first line
so it starts with From , not From .
Then get a clean, up to date clone:

git clone git://repo.or.cz/coreutils.git
cd coreutils

Create a private branch

git checkout -b tail-inotify

And apply your modified patch:

git am FILE

Finally, please think about adding some tests.
This change is invasive enough that we'll want
to exercise as much of it as possible before imposing
it on everyone.

I'll actually run/test your modified version of
tail later today or tomorrow.

Oops.  I see that this change induces at least one make check
test failure:

FAIL: tail-2/tail-n0f

+ pid=9087
+ sleep .5
+ tail --sleep=4 -n 0 -f empty
./tail-2/tail-n0f: line 44:  9087 Segmentation fault  tail --sleep=4 
-${c_or_n} 0 -f $file


Here are the changes I've made to your patch,
and which are included in the amended patch below.
(i.e., do *not* apply this patch.  rather just save the complete
patch below and apply *it* with git am FILE)

diff --git a/src/tail.c b/src/tail.c
index b64b069..0acec2c 100644
--- a/src/tail.c
+++ b/src/tail.c
@@ -48,7 +48,6 @@

 #ifdef HAVE_INOTIFY
 # include hash.h
-# include dirname.h
 # include sys/inotify.h
 #endif

@@ -1156,23 +1155,18 @@ wd_comparator (const void *e1, const void *e2)
 static void
 tail_forever_inotify (int wd, struct File_spec *f, int nfiles)
 {
-  /* Create an index to access File_spec by its watch descriptor
-   * in costant time.  */
-  struct 

Re: inotify back end for tail -f on linux

2009-06-02 Thread Giuseppe Scrivano
Jim Meyering j...@meyering.net writes:

 A few suggestions:

Thank you for all your suggestions, I'll keep them in mind.  I used your
modified patch as my starting point.  I fixed the segfault problem and I
added a new test case.

Regards,
Giuseppe


From 159dad5939b3853789d9c3c5f30ed92d74812d10 Mon Sep 17 00:00:00 2001
From: Giuseppe Scrivano gscriv...@gnu.org
Date: Tue, 2 Jun 2009 08:28:23 +0200
Subject: [PATCH] tail: Use inotify if it is available.

* NEWS: Document the new feature.
* configure.ac: Check if inotify is present.
* src/tail.c (tail_forever_inotify): New function.
(main): Use the inotify-based function, if possible.
* tests/Makefile.am: Added new test for tail.
* tests/tail-2/wait: New file.
---
 NEWS  |2 +
 configure.ac  |2 +
 src/tail.c|  245 -
 tests/Makefile.am |1 +
 tests/tail-2/wait |   72 
 5 files changed, 321 insertions(+), 1 deletions(-)
 create mode 100644 tests/tail-2/wait

diff --git a/NEWS b/NEWS
index 29b09a0..c5a2ef5 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 if it is present.
+
 
 * Noteworthy changes in release 7.4 (2009-05-07) [stable]
 
diff --git a/configure.ac b/configure.ac
index 4eb640e..a63ac0c 100644
--- a/configure.ac
+++ b/configure.ac
@@ -410,6 +410,8 @@ AM_GNU_GETTEXT_VERSION([0.15])
 # For a test of uniq: it uses the $LOCALE_FR envvar.
 gt_LOCALE_FR
 
+AC_CHECK_FUNCS([inotify_init], [AC_DEFINE([HAVE_INOTIFY], [1], [Check for 
libinotify])])
+
 AC_CONFIG_FILES(
   Makefile
   doc/Makefile
diff --git a/src/tail.c b/src/tail.c
index 9d416e1..4b2b59d 100644
--- a/src/tail.c
+++ b/src/tail.c
@@ -46,6 +46,11 @@
 #include xstrtol.h
 #include xstrtod.h
 
+#ifdef 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,6 +130,17 @@ struct File_spec
   /* The value of errno seen last time we checked this file.  */
   int errnum;
 
+#ifdef 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
 };
 
 /* Keep trying to open a file even if it is inaccessible when tail starts
@@ -1117,6 +1133,226 @@ tail_forever (struct File_spec *f, int nfiles, double 
sleep_interval)
 }
 }
 
+#ifdef 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 NFILES files forever, or until killed.
+   Check modifications using the inotify events system.  */
+static void
+tail_forever_inotify (int wd, struct File_spec *f, int nfiles)
+{
+  unsigned int i;
+  unsigned int max_realloc = 3;
+  Hash_table *wd_table;
+
+  bool found_watchable = false;
+  size_t last;
+  size_t evlen = 0;
+  char *evbuff;
+  size_t evbuff_off = 0;
+  ssize_t len = 0;
+
+  wd_table = hash_initialize (nfiles, NULL, wd_hasher, wd_comparator, NULL);
+  if (! wd_table)
+xalloc_die ();
+
+  for (i = 0; i  nfiles; 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';
+
+  /* Do not care if the directory was already added, in this
+ case the same watch descriptor will be returned.  */
+  f[i].parent_wd = inotify_add_watch (wd, f[i].name,
+  IN_CREATE | IN_MOVED_TO);
+
+  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;
+}
+}
+
+  if (f[i].errnum != ENOENT)
+{
+  int old_wd = f[i].wd;
+  f[i].wd = inotify_add_watch (wd, f[i].name,
+   (IN_MODIFY | IN_ATTRIB
+| IN_DELETE_SELF | IN_MOVE_SELF));
+
+  if (last == old_wd)
+last = 

Re: inotify back end for tail -f on linux

2009-05-31 Thread Jim Meyering
Giuseppe Scrivano wrote:
 Jim Meyering j...@meyering.net writes:

 Note what happens whey you truncate a tracked file.
 With your version, nothing.

 With the original, it reports this:

   tail: F: file truncated

 Likewise, when tailing with e.g., tail -F FILE 
 if you unlink FILE, you get this:

   tail: `FILE' has become inaccessible: No such file or directory

 and if you then recreate it:

   tail: `FILE' has appeared;  following end of new file

 You get none of that with your patch.

 Thank you for the quick review.  I added some messages that are possible
 in `Follow_descriptor' mode but I had to rollback to the old polling
 mechanism when `Follow_name' is used.  It is not straightforward to
 emulate it using inotify because it is not possible to watch a file that
 doesn't exist yet or a file that is removed and re-created.
 It is possible to watch the parent directory if new files are created
 but then the problem is moved on the parent directory that can be
 removed too and so on (or am I misunderstanding something with
 inotify?).

If we add inotify support, I'd like it to work for
both tail-by-FD and tail-by-name.

You can watch the parent directory for e.g., rename and creation events.
Log rotation is so common, that it'd be a shame not to be able to take
advantage of inotify when using tail -F 

For starters, you needn't worry about recovering from parent-dir-removal.
I think that parent removal is rare enough in practice that we won't
need to deal with it for some time.

 I tried `inotify_add_watch' with a file that doesn't exist and a file
 that exists, removed and re-created, registering them with
 IN_ALL_EVENTS.  In the former case I get an error, in the latter I don't
 get any event when it is re-created as it is silently dropped from the
 watch list when the file is removed.

 Is there a way to monitor a path that doesn't exist yet and be alerted
 when it is created without polling?

Eventually, the code could monitor the longest parent prefix that *does* exist.
When the missing subdir reappears, monitor $parent/$subdir, etc.
However, there's no need to do that now.

This added complexity, and the additional features
we could gain via inotify, are why I was encouraging people
to add the feature via a new program.  And one not written
(at least not initially) in C.

I haven't looked closely at the code below or even tried it,
but here are a few superficial comments:

 @@ -45,6 +45,10 @@
  #include xstrtol.h
  #include xstrtod.h

 +#ifdef HAVE_INOTIFY
 +#include sys/inotify.h
 +#endif

Indent cpp directives to reflect nesting:

  #ifdef HAVE_INOTIFY
  # include sys/inotify.h
  #endif

  /* The official name of this program (e.g., no `g' prefix).  */
  #define PROGRAM_NAME tail

 @@ -1116,6 +1120,114 @@ tail_forever (struct File_spec *f, int nfiles, double 
 sleep_interval)
  }
  }

 +#ifdef HAVE_INOTIFY
 +/* Tail NFILES files forever, or until killed.
 +   Check modifications using the inotify events system.  */
 +static void
 +tail_forever_inotify (int wd, struct File_spec *f, int nfiles)
 +{
 +  /* Create an index to access File_spec by its watch descriptor
 +   * in costant time.  */
 +  struct File_spec **wd_index;
 +  int i;
 +  int *watch_fd = xmalloc (sizeof (int) * nfiles);
 +  int min_wd = -1;
 +  int max_wd = -1;
 +  int watched = 0;
 +  size_t last;
 +
 +  for (i = 0; i  nfiles; i++)
 +{
 +  watch_fd[i] = -1;
 +
 +  if (!f[i].ignore)
 +{
 +  watch_fd[i] = inotify_add_watch (wd, f[i].name, 
 IN_MODIFY|IN_ATTRIB);
 +
 +  if (watch_fd[i]  0)
 +{
 +  error (0, errno, _(cannot watch %s), quote (f[i].name));
 +  continue;
 +}
 +
 +  watched++;
 +
 +  if (watch_fd[i]  max_wd || max_wd  0)
 +  max_wd = watch_fd[i];
 +
 +  if (watch_fd[i]  min_wd || min_wd  0)
 +  min_wd = watch_fd[i];
 +}
 +}
 +
 +  if (!watched)
 +return;
 +
 +  wd_index = xmalloc (sizeof (struct File_spec**) * (max_wd - min_wd + 1));

Use sizeof VAR, not sizeof TYPE.

 wd_index = xmalloc (sizeof *wd_index * (max_wd - min_wd + 1));

 +  for (i = 0; i  nfiles; i++)
 +if (watch_fd[i]  0)

Oops.  Off-by-one.  Don't skip a file descriptor of 0, however unlikely.
Also, remember I prefer  and = over  and =:

   if (0 = watch_fd[i])

 +  wd_index[watch_fd[i] - min_wd] = (f[i]);
 +
 +  free (watch_fd);
 +
 +  last = max_wd + 1;
 +
 +  while (1)
 +{
 +  size_t len;
 +  struct inotify_event ev;
 +  char const *name;
 +  struct File_spec *fspec;
 +  uintmax_t bytes_read;
 +  struct stat stats;
 +
 +  len = read (wd, ev, sizeof (struct inotify_event));

use sizeof VAR here, too

 +  if (!len  errno == EINTR)
 +continue;
 +
 +  if (!len)
 +error (EXIT_FAILURE, errno, _(error reading inotify event));

This function must not exit upon a failure that is specific
to a single file.  Just diagnose it and 

Re: inotify back end for tail -f on linux

2009-05-30 Thread Giuseppe Scrivano
Hi Jim,

Jim Meyering j...@meyering.net writes:

 Thanks for the suggestion.
 Yes, this has been discussed, e.g., in

 http://thread.gmane.org/gmane.comp.gnu.coreutils.bugs/15031/focus=15052

 I like the idea and outlined how I'd like to see this functionality
 make it's way into the coreutils.

I read this discussion but looking at the tail source code I didn't see
any difficulty to use inotify.

These are some tests results:

test -e /tmp/files  || mkdir /tmp/files; 
 for i in $(seq 2000); do touch /tmp/files/$i; done

my version:
./timeout -sINT 30s env time ./tail -f /tmp/files/*
0.02user 0.09system 0:30.00elapsed 0%CPU (0avgtext+0avgdata 0maxresident)k
0inputs+0outputs (0major+232minor)pagefaults 0swaps

system version:
./timeout -sINT 30s env time tail -f /tmp/files/*
0.05user 0.16system 0:29.99elapsed 0%CPU (0avgtext+0avgdata 0maxresident)k
0inputs+0outputs (0major+236minor)pagefaults 0swaps


I am sure running the test for a longer time would give better
results.  The initial time using inotify is higher as there is need to
create watch descriptors and an index I use for access to the File_spec
in costant time.

What do you think of the attached patch?

Cheers,
Giuseppe


From aa815abdd3cbf2ad15d769a1f4cb6100e3a975e5 Mon Sep 17 00:00:00 2001
From: Giuseppe Scrivano gscriv...@gnu.org
Date: Sat, 30 May 2009 13:31:58 +0200
Subject: [PATCH] tail: Use inotify if it is available.

* NEWS: Document the new feature
* configure.ac: Check if inotify is present.
* src/tail.c (main): Use the tail_forever inotify version if it
is possible.
(tail_forever_inotify): Added new function.
---
 NEWS |2 +
 configure.ac |2 +
 src/tail.c   |  103 -
 3 files changed, 105 insertions(+), 2 deletions(-)

diff --git a/NEWS b/NEWS
index 29b09a0..c5a2ef5 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 if it is present.
+
 
 * Noteworthy changes in release 7.4 (2009-05-07) [stable]
 
diff --git a/configure.ac b/configure.ac
index 4eb640e..a63ac0c 100644
--- a/configure.ac
+++ b/configure.ac
@@ -410,6 +410,8 @@ AM_GNU_GETTEXT_VERSION([0.15])
 # For a test of uniq: it uses the $LOCALE_FR envvar.
 gt_LOCALE_FR
 
+AC_CHECK_FUNCS([inotify_init], [AC_DEFINE([HAVE_INOTIFY], [1], [Check for 
libinotify])])
+
 AC_CONFIG_FILES(
   Makefile
   doc/Makefile
diff --git a/src/tail.c b/src/tail.c
index fe34600..bb0950e 100644
--- a/src/tail.c
+++ b/src/tail.c
@@ -1,5 +1,5 @@
 /* tail -- output the last part of file(s)
-   Copyright (C) 1989, 90, 91, 1995-2006, 2008 Free Software Foundation, Inc.
+   Copyright (C) 1989, 90, 91, 1995-2006, 2008, 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
@@ -45,6 +45,10 @@
 #include xstrtol.h
 #include xstrtod.h
 
+#ifdef HAVE_INOTIFY
+#include sys/inotify.h
+#endif
+
 /* The official name of this program (e.g., no `g' prefix).  */
 #define PROGRAM_NAME tail
 
@@ -1116,6 +1120,93 @@ tail_forever (struct File_spec *f, int nfiles, double 
sleep_interval)
 }
 }
 
+#ifdef HAVE_INOTIFY
+/* Tail NFILES files forever, or until killed.
+   Check modifications using the inotify events system.  */
+static void
+tail_forever_inotify (int wd, struct File_spec *f, int nfiles)
+{
+  /* Create an index to access File_spec by its watch descriptor
+   * in costant time.  */
+  struct File_spec **wd_index;
+  int i;
+  int *watch_fd = xmalloc (sizeof (int) * nfiles);
+  int min_wd = -1;
+  int max_wd = -1;
+  int watched = 0;
+  size_t last;
+
+  for (i = 0; i  nfiles; i++)
+{
+  watch_fd[i] = -1;
+
+  if (!f[i].ignore)
+{
+  watch_fd[i] = inotify_add_watch (wd, f[i].name,
+   IN_MODIFY);
+
+  if (watch_fd[i]  0)
+{
+  error (0, errno, _(cannot watch %s), quote (f[i].name));
+  continue;
+}
+
+  watched++;
+
+  if (watch_fd[i]  max_wd || max_wd  0)
+  max_wd = watch_fd[i];
+
+  if (watch_fd[i]  min_wd || min_wd  0)
+  min_wd = watch_fd[i];
+}
+}
+
+  if (!watched)
+return;
+
+  wd_index = xmalloc (sizeof (struct File_spec**) * (max_wd - min_wd + 1));
+
+  for (i = 0; i  nfiles; i++)
+if (watch_fd[i]  0)
+  wd_index[watch_fd[i] - min_wd] = (f[i]);
+
+  free (watch_fd);
+
+  last = max_wd + 1;
+
+  while (1)
+{
+  size_t len;
+  struct inotify_event ev;
+  char const *name;
+  struct File_spec *fspec;
+  uintmax_t bytes_read;
+
+  len = read (wd, ev, sizeof (struct inotify_event));
+  if (!len  errno == EINTR)
+continue;
+
+  if 

Re: inotify back end for tail -f on linux

2009-05-30 Thread Jim Meyering
Giuseppe Scrivano wrote:
From aa815abdd3cbf2ad15d769a1f4cb6100e3a975e5 Mon Sep 17 00:00:00 2001
 From: Giuseppe Scrivano gscriv...@gnu.org
 Date: Sat, 30 May 2009 13:31:58 +0200
 Subject: [PATCH] tail: Use inotify if it is available.

 * NEWS: Document the new feature
 * configure.ac: Check if inotify is present.
 * src/tail.c (main): Use the tail_forever inotify version if it
 is possible.
 (tail_forever_inotify): Added new function.

Thank you for writing that.  Seems like a fine start.

Note what happens whey you truncate a tracked file.
With your version, nothing.

With the original, it reports this:

  tail: F: file truncated

Likewise, when tailing with e.g., tail -F FILE 
if you unlink FILE, you get this:

  tail: `FILE' has become inaccessible: No such file or directory

and if you then recreate it:

  tail: `FILE' has appeared;  following end of new file

You get none of that with your patch.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils


Re: inotify back end for tail -f on linux

2009-05-30 Thread Giuseppe Scrivano
Jim Meyering j...@meyering.net writes:

 Note what happens whey you truncate a tracked file.
 With your version, nothing.

 With the original, it reports this:

   tail: F: file truncated

 Likewise, when tailing with e.g., tail -F FILE 
 if you unlink FILE, you get this:

   tail: `FILE' has become inaccessible: No such file or directory

 and if you then recreate it:

   tail: `FILE' has appeared;  following end of new file

 You get none of that with your patch.

Thank you for the quick review.  I added some messages that are possible
in `Follow_descriptor' mode but I had to rollback to the old polling
mechanism when `Follow_name' is used.  It is not straightforward to
emulate it using inotify because it is not possible to watch a file that
doesn't exist yet or a file that is removed and re-created.
It is possible to watch the parent directory if new files are created
but then the problem is moved on the parent directory that can be
removed too and so on (or am I misunderstanding something with
inotify?).
I tried `inotify_add_watch' with a file that doesn't exist and a file
that exists, removed and re-created, registering them with
IN_ALL_EVENTS.  In the former case I get an error, in the latter I don't
get any event when it is re-created as it is silently dropped from the
watch list when the file is removed.

Is there a way to monitor a path that doesn't exist yet and be alerted
when it is created without polling?

Giuseppe


From b5cad2d1e51781b18e53eac7b102922319909f5b Mon Sep 17 00:00:00 2001
From: Giuseppe Scrivano gscriv...@gnu.org
Date: Sat, 30 May 2009 13:31:58 +0200
Subject: [PATCH] tail: Use inotify if it is available.

* NEWS: Document the new feature
* configure.ac: Check if inotify is present.
* src/tail.c (main): Use the tail_forever inotify version if it
is possible.
(tail_forever_inotify): Added new function.
---
 NEWS |2 +
 configure.ac |2 +
 src/tail.c   |  126 +-
 3 files changed, 128 insertions(+), 2 deletions(-)

diff --git a/NEWS b/NEWS
index 29b09a0..c5a2ef5 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 if it is present.
+
 
 * Noteworthy changes in release 7.4 (2009-05-07) [stable]
 
diff --git a/configure.ac b/configure.ac
index 4eb640e..a63ac0c 100644
--- a/configure.ac
+++ b/configure.ac
@@ -410,6 +410,8 @@ AM_GNU_GETTEXT_VERSION([0.15])
 # For a test of uniq: it uses the $LOCALE_FR envvar.
 gt_LOCALE_FR
 
+AC_CHECK_FUNCS([inotify_init], [AC_DEFINE([HAVE_INOTIFY], [1], [Check for 
libinotify])])
+
 AC_CONFIG_FILES(
   Makefile
   doc/Makefile
diff --git a/src/tail.c b/src/tail.c
index fe34600..3f77ba9 100644
--- a/src/tail.c
+++ b/src/tail.c
@@ -1,5 +1,5 @@
 /* tail -- output the last part of file(s)
-   Copyright (C) 1989, 90, 91, 1995-2006, 2008 Free Software Foundation, Inc.
+   Copyright (C) 1989, 90, 91, 1995-2006, 2008, 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
@@ -45,6 +45,10 @@
 #include xstrtol.h
 #include xstrtod.h
 
+#ifdef HAVE_INOTIFY
+#include sys/inotify.h
+#endif
+
 /* The official name of this program (e.g., no `g' prefix).  */
 #define PROGRAM_NAME tail
 
@@ -1116,6 +1120,114 @@ tail_forever (struct File_spec *f, int nfiles, double 
sleep_interval)
 }
 }
 
+#ifdef HAVE_INOTIFY
+/* Tail NFILES files forever, or until killed.
+   Check modifications using the inotify events system.  */
+static void
+tail_forever_inotify (int wd, struct File_spec *f, int nfiles)
+{
+  /* Create an index to access File_spec by its watch descriptor
+   * in costant time.  */
+  struct File_spec **wd_index;
+  int i;
+  int *watch_fd = xmalloc (sizeof (int) * nfiles);
+  int min_wd = -1;
+  int max_wd = -1;
+  int watched = 0;
+  size_t last;
+
+  for (i = 0; i  nfiles; i++)
+{
+  watch_fd[i] = -1;
+
+  if (!f[i].ignore)
+{
+  watch_fd[i] = inotify_add_watch (wd, f[i].name, IN_MODIFY|IN_ATTRIB);
+
+  if (watch_fd[i]  0)
+{
+  error (0, errno, _(cannot watch %s), quote (f[i].name));
+  continue;
+}
+
+  watched++;
+
+  if (watch_fd[i]  max_wd || max_wd  0)
+  max_wd = watch_fd[i];
+
+  if (watch_fd[i]  min_wd || min_wd  0)
+  min_wd = watch_fd[i];
+}
+}
+
+  if (!watched)
+return;
+
+  wd_index = xmalloc (sizeof (struct File_spec**) * (max_wd - min_wd + 1));
+
+  for (i = 0; i  nfiles; i++)
+if (watch_fd[i]  0)
+  wd_index[watch_fd[i] - min_wd] = (f[i]);
+
+  free (watch_fd);
+
+  last = max_wd + 1;
+
+  while (1)
+{
+  size_t len;
+  struct inotify_event 

Re: inotify back end for tail -f on linux

2009-04-13 Thread Jim Meyering
shawn wrote:
 Perhaps this has already been discussed

Thanks for the suggestion.
Yes, this has been discussed, e.g., in

http://thread.gmane.org/gmane.comp.gnu.coreutils.bugs/15031/focus=15052

I like the idea and outlined how I'd like to see this functionality
make it's way into the coreutils.

In the message above, I suggested this:

  Prototype a new tail-like tool that requires Linux inotify support.
  I'd do it in perl, maybe using the Linux::Inotify2 module.  There may
  be better; I haven't looked in a long time.  It's far easier to prototype
  something like that in a scripting language than in C.  To be useful,
  it'll need at least some of the options iwatch has, e.g., for filtering.

  Then, once everyone thinks the feature set is complete,
  rewrite it in C.


 tail -f polls and is highly inefficient

Can you describe a situation in which it is highly inefficient?
afaik, it becomes inefficient only when there are many files
or when you choose a polling interval that is much smaller
than the default.

 when there exists event
 notification such as inotify. It would be nice if this could be added in
 a way that does not break any systems, even if compiled for Linux and
 the Linux kernel it runs on does not have inotify. If inotify does not
 work it would fall back to polling.

 http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=423886


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils