Test inotify watch after drop caches #246

amir73il · 2018-01-14T14:59:46Z

A test for a problem reported by Niklas Cassel for a setup of inotify watch on overlayfs:
https://marc.info/?l=linux-unionfs&m=151581833424827&w=2

Starts with a face lift to existing inotify tests

metan-ucw · 2018-01-24T16:25:35Z

The first two patches looks good to me.

I do have some questions for the new testcase though.

Is there a kernel commit that fixes the bug? We usually add the commit hash to the top level comment of the test so that everyone can check if the tested kernel should contain the fix or not.

Also is there a pointer to original reproducer? We tend to include link to it as well.

metan-ucw · 2018-01-24T16:28:00Z

testcases/kernel/syscalls/inotify/inotify07.c

+		tst_brk(TBROK, "Failed to open drop_caches");
+	}
+}
+


We do have SAFE_FILE_PRINTF() exactly for this purpose, the whole function can be replaced with:
SAFE_FILE_PRINTF("proc/sys/vm/drop_caches", "2");

metan-ucw · 2018-01-24T16:29:38Z

testcases/kernel/syscalls/inotify/inotify07.c

+{
+	struct stat buf;
+
+	fd_notify = tst_syscall(__NR_inotify_init1, O_NONBLOCK);


This calls for addition of myinotify_init1() to the inotify.h.

amir73il · 2018-01-24T17:09:22Z

@metan-ucw the fix commit is not yet in upstream. it is on Miklos's overlayfs-next branch,
which is queued for the upcoming v4.16 merge window
31747eda41ef - ovl: hash directory inodes for fsnotify
I will add this commit to the test description once fix commit is merged upstream.

I did not get a reproducer. The reproducer was described in the post from Niklas that I referred to in the RP.

metan-ucw · 2018-01-25T14:14:22Z

Then something is not right, the test is passing for me on vanilla 4.14.11. Do I get it right that we should stop getting updates after we drop the dentry cache?

amir73il · 2018-01-25T14:28:54Z

That's because the bug is an overlay bug, not an inotify bug.
I ran the test with TMPDIR=<path/to/overlay/mount>.
Sorry for failing to mention that.
Example for setting up overlay mount:
mkdir upper work lower ovl
mount -t overlay overlay ovl -oupperdir=upper,lowerdir=lower,workdir=work

metan-ucw · 2018-01-25T14:30:39Z

Then the test has to mount the overlay in the test setup().

amir73il · 2018-01-25T14:35:49Z

OK. will do that.

amir73il · 2018-01-26T13:48:34Z

Please note that the commit in the test description is in linux-next tree, but not yet upstream

amir73il · 2018-01-28T14:38:36Z

Rebased to master.
Added directory copy-up before drop caches to cover more cases.
Exit with TCONF status if overlayfs is not available in kernel.
Not sure what these CI build failures are about?

amir73il · 2018-02-04T14:32:11Z

Added another test for another issue I found.
I will update the fix commit once it gets merged upstream.

amir73il · 2018-02-14T11:34:22Z

@metan-ucw commit 31747eda41ef that fixes test case inotify07 in now merged to kernel v4.16-rc1.
The test case inotify08 is still failing in upstream kernel.
Would you like me to split test inotify08 to a new pull request? or just modify test description to not mention a fix commit?

metan-ucw · 2018-02-27T13:10:06Z

If there is a real bug we can always push the test even if there is no upstream commit, we just have to add that information to the test top level comment and then replace it with kernel commit id once the fix is released.

amir73il · 2018-02-27T13:25:13Z

@metan-ucw I updated fix commit of inotify08. It's not upstream yet, but on overlayfs-next branch, so will get upstream soon.

metan-ucw · 2018-03-02T15:52:51Z

I've pushed the first two patches along with seven more fixes for the original inotify tests on the top of that. Some of the problems I've fixed are in the new test you wrote as well. Can you please rebase the pull request and also update the two new test cases?

amir73il · 2018-03-14T20:57:55Z

FYI, commit 764baba80168 from inotify08 was merged to v4.16-rc5

amir73il · 2018-03-22T09:39:23Z

@metan-ucw ping

amir73il · 2018-04-05T10:27:47Z

@metan-ucw rebased and fixed problem with inotify07 -i 2

metan-ucw · 2018-04-12T12:49:22Z

testcases/kernel/syscalls/inotify/inotify07.c

+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of version 2 of the GNU General Public License as
+ * published by the Free Software Foundation.
+ *


LTP default license is GPLv2+ (the any later one) but that is very minor.

metan-ucw · 2018-04-12T12:53:35Z

testcases/kernel/syscalls/inotify/inotify07.c

+		tst_brk(TBROK | TERRNO,
+			"read(%d, buf, %zu) failed",
+			fd_notify, EVENT_BUF_LEN);
+


The only nit for this version is that when executed on broken kernel the failure message produced is:

BROK: read(3, buf, 32768) failed: EAGAIN/EWOULDBLOCK

Which looks much more like a broken test rather than anything else. Can we please check for the EAGAIN errno here and print a bit more user friendly TFAIL message in this case?

The same applies to the next test and apart from that the code looks good to me.

ok. there are already meaningful TFAIL messages further down, so I only skipped TBROK for EAGAIN case

metan-ucw · 2018-04-12T14:45:40Z

testcases/kernel/syscalls/inotify/inotify08.c

+	SAFE_STAT(FILE_PATH, &buf);
+	tst_res(TINFO, FILE_PATH " ino=%lu, dev=%u:%u", buf.st_ino,
+			major(buf.st_dev), minor(buf.st_dev));
+


I've also got a header deprecation warnings on newer systems for the minor() and major():

inotify08.c:211:13: warning: In the GNU C Library, "minor" is defined by <sys/sysmacros.h>. For historical compatibility, it is currently defined by <sys/types.h> as well, but we plan to remove this soon. To use "minor", include <sys/sysmacros.h> directly. If you did not intend to use a system-defined macro "minor", you should undefine it after including <sys/types.h>.

So we should add #include <sys/sysmacros.h> to the includes here.

Add watch on an overlayfs lower directory, then copy up the directory and drop dentry and inode caches. An inotify watch pins the directory inode in cache, but not the dentry. Execute operations on directory and child and expect events to be reported on directory watch. This will fail if file system does not obtain the pinned inode to the new allocated dentry after copy up and drop caches. This is a regression test for commit 31747eda41ef ("ovl: hash directory inodes for fsnotify"). Signed-off-by: Amir Goldstein <amir73il@gmail.com>

Add watch on an overlayfs lower file, then copy up the file and drop dentry and inode caches. An inotify watch pins the file inode in cache, but not the dentry. Execute operations on file and expect events to be reported on file watch. This will fail if file system does not obtain the pinned inode to the new allocated dentry after copy up and drop caches. This is a regression test for commit 764baba80168 ("ovl: hash non-dir by lower inode for fsnotify"). Signed-off-by: Amir Goldstein <amir73il@gmail.com>

metan-ucw · 2018-04-19T12:37:20Z

Applied, thanks!

metan-ucw reviewed Jan 24, 2018

View reviewed changes

amir73il force-pushed the inotify-drop-caches branch from fab253c to c5a5dd9 Compare January 26, 2018 13:43

amir73il force-pushed the inotify-drop-caches branch 3 times, most recently from 3b4f443 to a3183bb Compare January 28, 2018 14:01

amir73il force-pushed the inotify-drop-caches branch 2 times, most recently from cb34158 to d2445a2 Compare February 4, 2018 14:30

amir73il force-pushed the inotify-drop-caches branch from d2445a2 to 4df8894 Compare February 27, 2018 13:23

amir73il force-pushed the inotify-drop-caches branch from 4df8894 to 2139add Compare March 3, 2018 13:51

amir73il force-pushed the inotify-drop-caches branch 2 times, most recently from 2171cff to 1b26582 Compare April 5, 2018 10:24

metan-ucw reviewed Apr 12, 2018

View reviewed changes

amir73il force-pushed the inotify-drop-caches branch 2 times, most recently from 291c24d to c6b6a5b Compare April 12, 2018 14:51

metan-ucw closed this Apr 19, 2018

amir73il deleted the inotify-drop-caches branch November 7, 2020 15:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Test inotify watch after drop caches #246

Test inotify watch after drop caches #246

amir73il commented Jan 14, 2018

metan-ucw commented Jan 24, 2018

metan-ucw Jan 24, 2018

metan-ucw Jan 24, 2018

amir73il commented Jan 24, 2018

metan-ucw commented Jan 25, 2018

amir73il commented Jan 25, 2018 •

edited

metan-ucw commented Jan 25, 2018

amir73il commented Jan 25, 2018

amir73il commented Jan 26, 2018

amir73il commented Jan 28, 2018

amir73il commented Feb 4, 2018

amir73il commented Feb 14, 2018

metan-ucw commented Feb 27, 2018

amir73il commented Feb 27, 2018

metan-ucw commented Mar 2, 2018

amir73il commented Mar 14, 2018

amir73il commented Mar 22, 2018

amir73il commented Apr 5, 2018

metan-ucw Apr 12, 2018

metan-ucw Apr 12, 2018

amir73il Apr 12, 2018

metan-ucw Apr 12, 2018

metan-ucw commented Apr 19, 2018

Test inotify watch after drop caches #246

Test inotify watch after drop caches #246

Conversation

amir73il commented Jan 14, 2018

metan-ucw commented Jan 24, 2018

metan-ucw Jan 24, 2018

Choose a reason for hiding this comment

metan-ucw Jan 24, 2018

Choose a reason for hiding this comment

amir73il commented Jan 24, 2018

metan-ucw commented Jan 25, 2018

amir73il commented Jan 25, 2018 • edited

metan-ucw commented Jan 25, 2018

amir73il commented Jan 25, 2018

amir73il commented Jan 26, 2018

amir73il commented Jan 28, 2018

amir73il commented Feb 4, 2018

amir73il commented Feb 14, 2018

metan-ucw commented Feb 27, 2018

amir73il commented Feb 27, 2018

metan-ucw commented Mar 2, 2018

amir73il commented Mar 14, 2018

amir73il commented Mar 22, 2018

amir73il commented Apr 5, 2018

metan-ucw Apr 12, 2018

Choose a reason for hiding this comment

metan-ucw Apr 12, 2018

Choose a reason for hiding this comment

amir73il Apr 12, 2018

Choose a reason for hiding this comment

metan-ucw Apr 12, 2018

Choose a reason for hiding this comment

metan-ucw commented Apr 19, 2018

amir73il commented Jan 25, 2018 •

edited