-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
syzbot: fs coverage dropped in v6.9-rc1 #4611
Comments
I've reverted |
But the problem is definitely there and is also well reproducible locally: two syzkaller instances running side by side for 1 hour: |
Manual bisection pointed to this series: https://lore.kernel.org/all/20240123-vfs-bdev-file-v2-0-adbd023e19cc@kernel.org/ |
And the commit that has actually changed the situation for us
|
Looks like we can no longer reuse the same loop device when some filesystem is still holding a reference to it. As I understand, we do try to unmount mounted filesystems here syzkaller/executor/common_linux.h Line 4531 in 2b6d9d2
But I don't know if there's any way to explicitly wait until the unmount process is complete. Or maybe we miss some somehow. Maybe it's now easier to move to loopfs? @dvyukov do you remember why don't we use it? Cc @jankara |
remove_dir() should return before the next test is started. If the fs is still mounted when umount returns, it looks like kernel bug. Any script that unmounts and mounts something may fail, if it's the case.
IIRC It did not exist at the time. A cleaner way may be to create a mount namespace per test, then we don't need remove_dir. Say, if we mount tmpfs as a working dir and then drop it all altogether with the process. |
So remove_dir() actually using umount2(filename, MNT_DETACH | UMOUNT_NOFOLLOW) which means it explicitely asks the kernel to just detach the existing mount from the directory hierarchy but leave the fs mounted until all references to it are dropped. So the device is very likely still mounted when this call returns and this is what kernel has been asked to do. Sadly I don't think there's an easy generic way to wait for a detached filesystem to actually get cleaned up - about the best I can think of is to mount it again (the mount will reuse the same superblock is it still exists) and then unmount it without MNT_DETACH. |
You are right. I forgot about MNT_DETACH.
If we want to wait synchronously, then we can just remove MNT_DETACH, right? I guess mounting a loopfs per test process will be good regardless, it will also eliminate more interference between parallel test processes. |
I see why -- this patch series was never merged to the mainline.
though it's not 100% clear what free means. If free means that no mounted filesystem is currently using the block device, we can try to use UPD: If a loop device stops being free only once mount is done, |
Maybe we could just use I'll try and see what it results in. |
My bet is that MNT_FORCE will hang due to reference leaks. We will also probably not detect it well, e.g. executor will be killed on timeout, but the device is still not free, so fuzzing coverage will again drop. |
Re LOOP_CTL_GET_FREE: IIRC it has own accounting of free/busy devices, which has nothing to do with actual use of devices. And also any code is free to just use /dev/loopN explicitly and that will conflict with LOOP_CTL_GET_FREE (it can hand out the same N). |
LOOP_CTL_GET_FREE will simply return loop device that is currently not attached to any backing file. There can be time-to-check-time-to-use races but it's a good tip what loop device to try. Also binding loop device to a file requires O_EXCL open these days (it grabs one within ioctl temporarily if open was without O_EXCL) so that is the moment you can rely on - if that succeeds you're guaranteed nobody else has the device mounted. If that fails, you need to go back to LOOP_CTL_GET_FREE and try again. |
For the record: after #4668 was deployed and some of the noisy v6.9 bugs were fixed, the fs coverage has gone somewhat up: from ~85k to ~135k coverage callbacks. Not the 176k that we had before March 2024, but already much better. |
After the first v6.9 commits have reached
torvalds
, we have seen a big drop in the number of covered trace points infs/
.This is best seen on our
ci2-upstream-fs
instance: we went from 178k to ~100k.Sample coverage report before: https://storage.googleapis.com/syzbot-assets/5e2921d5b3d3/ci2-upstream-fs-09e5c48f.html
After: https://storage.googleapis.com/syzbot-assets/be3e453a28d3/ci2-upstream-fs-fe46a7dd.html
As can be seen, it's not related to any specific filesystem -- no fs/X's coverage has dropped to 0, everything just decreased 1.5-2x.
I've looked at the git logs, and so far I have only identified this kernel commit that could have led to this change:
It may be related to how we operate with
/dev/loop
, but needs further investigation to confirm.The text was updated successfully, but these errors were encountered: