On 02/07/15 03:38, Pádraig Brady wrote: > On 02/07/15 02:55, Assaf Gordon wrote: >> Hello, >> >> few results for coreutils-8.23.245-8bf2af :
>> On OpenSUSE 13.2 x86_64, "tests/tail-2/assert" fail, log attached. > > Interesting. Reproduced 100% of the time here too. > BTW that VM gives a different version? > $ head -n1 /etc/issue > Welcome to openSUSE 12.3 ... It's not 100% reproducible actually, though once it starts happening it continues to, no matter how many times the file is replaced and removed. What seems to be happening is that the IN_ATTRIB event is delivered for the file on unlink() (the link count decreasing is an attribute change). However the subsequent open() in recheck() succeeds! I.E. the kernel dentry (cache) is still accessible through that name. Adding a xnanosleep(.1) before the open() is enough to make the test succeed. We could avoid this issue by adding IN_DELETE on the parent directory, though I'm loath to change this at the moment. cheers, Pádraig.
From 850915551f89099b1774fd4531f730d0cfff7498 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A1draig=20Brady?= <[email protected]> Date: Thu, 2 Jul 2015 08:41:25 +0100 Subject: [PATCH] tail: avoid kernel dentry unlink race seen on 3.7.10 * src/tail.c (tail_forever_inotify): Also monitor IN_DELETE events on the directory, to avoid a dentry unlink()..open() race seen on OpenSUSE 13.2, where the open() on the deleted file was seen to succeed after an unlink() and a subsequent IN_ATTRIB was sent to tail. Note the IN_ATTRIB is sent on the monitored file to indicate the change in number of links. --- src/tail.c | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/src/tail.c b/src/tail.c index c062d40..a9cfe5d 100644 --- a/src/tail.c +++ b/src/tail.c @@ -1429,8 +1429,8 @@ tail_forever_inotify (int wd, struct File_spec *f, size_t n_files, /* 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)); + (IN_CREATE | IN_DELETE + | IN_MOVED_TO | IN_ATTRIB)); f[i].name[dirlen] = prev; @@ -1619,9 +1619,16 @@ tail_forever_inotify (int wd, struct File_spec *f, size_t n_files, fspec = &(f[j]); - /* Adding the same inode again will look up any existing wd. */ - int new_wd = inotify_add_watch (wd, f[j].name, inotify_wd_mask); - if (new_wd < 0) + int new_wd = -1; + bool deleting = !! (ev->mask & IN_DELETE); + + if (! deleting) + { + /* Adding the same inode again will look up any existing wd. */ + new_wd = inotify_add_watch (wd, f[j].name, inotify_wd_mask); + } + + if (! deleting && new_wd < 0) { if (errno == ENOSPC || errno == ENOMEM) { @@ -1683,7 +1690,7 @@ tail_forever_inotify (int wd, struct File_spec *f, size_t n_files, if (! fspec) continue; - if (ev->mask & (IN_ATTRIB | IN_DELETE_SELF | IN_MOVE_SELF)) + if (ev->mask & (IN_ATTRIB | IN_DELETE | IN_DELETE_SELF | IN_MOVE_SELF)) { /* Note for IN_MOVE_SELF (the file we're watching has been clobbered via a rename) we leave the watch -- 2.4.1
