On 06.03.25 13:42, Brendan Jackman wrote:
On Thu, Mar 06, 2025 at 10:28:09AM +0100, David Hildenbrand wrote:
On 28.02.25 17:54, Brendan Jackman wrote:
Some filesystems don't support funtract()ing unlinked files. They return ENOENT. In that case, skip the test.
That's not documented in the man page, so is this a bug of these filesystems?
Note that I meant that ftruncate doesn't mention this in the man page. The only occurrence is
"ENOENT The named file does not exist.", and that only applies to truncate, not ftruncate.
Um...
unlink(2) does say:
If the name was the last link to a file but any processes still have the file open, the file will remain in existence until the last file descriptor referring to it is closed.
And POSIX says
If one or more processes have the file open when the last link is removed, the link shall be removed before unlink() returns, but the removal of the file contents shall be postponed until all references to the file are closed
Right, it's supposed to just stay around, it simply cannot be looked up anymore.
I didn't call it a bug in the commit message because my impression was always that filesystem semantics are broadly determined by vibes. But looking at the above I do feel more confident that the "unlink isn't delete" thing is actually a pretty solid expectation.
I have a faint recollection that 9pfs is problematic with unlink ... and indeed:
https://gitlab.com/qemu-project/qemu/-/issues/103
I'm not sure at this point what's expected to work and what not with 9pfs at this point.
What are examples for these weird filesystems?
My experience of the issue is with 9pfs. broonie reported on #mm that NFS can display similar issues but I haven't hit it myself.
As we have the fstype available, we could instead simply reject more
filesystems earlier. See fs_is_unknown().
Oh. I didn't know this was so easy, I thought that checking the filesystem type would require some awful walk to find the mountpoint and join it against the mount list. (Now I think about it, I should have recorded this rationale in the commit message, so you could easily see my bogus reasoning).
If there's a syscall to just say "what FS is this file on please?" we should just do that and explicitly denylist the systems that are known to have issues. I will just do 9pfs for now. Maybe we can log warning if the error shows up on systems that aren't listed, then if someone does run into it on NFS they should get a strong clue about what the problem is.
Yes, just skip 9pfs early, and mention in the commit message that 9pfs has a history of being probematic with "use-after-unlink", maybe mentioning the discussion I linked above.
Maybe something like this would work?
diff --git a/tools/testing/selftests/mm/gup_longterm.c b/tools/testing/selftests/mm/gup_longterm.c index 9423ad439a614..349e40d3979f2 100644 --- a/tools/testing/selftests/mm/gup_longterm.c +++ b/tools/testing/selftests/mm/gup_longterm.c @@ -47,6 +47,16 @@ static __fsword_t get_fs_type(int fd) return ret ? 0 : fs.f_type; }
+static bool fs_is_problematic(__fsword_t fs_type) +{ + switch (fs_type) { + case V9FS_MAGIC: + return false; + default: + return true; + } +} + static bool fs_is_unknown(__fsword_t fs_type) { /* @@ -95,6 +105,11 @@ static void do_test(int fd, size_t size, enum test_type type, bool shared) char *mem; int ret;
+ if (fs_is_problematic(fs_type)) { + ksft_test_result_skip("problematic fs\n"); + return; + } + if (ftruncate(fd, size)) { ksft_test_result_fail("ftruncate() failed\n"); return;
I am not 100% sure if V9FS_MAGIC is what we should be using? "man fstatfs" lists most magic values.