-
Notifications
You must be signed in to change notification settings - Fork 144
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
f3read crash #211
Comments
What's your operating system, @axet? If I write a pull request for you to test, would you know how to apply the pull request to your local copy of F3, compile it, and test it? |
Debian 12. I can test and apply patches. But it is hard to reproduce. It only happens on bad SD cards, and I already dispose all of them. |
Errors coming from fdatasync() are unexpected, but according to issue #211 they happen. This patch just reports the error and general guidance to users.
I've added code to the repository to report an error message when this problem comes up. It's an expected error, so I'll watch for what the error messages look like to find a possible root cause and eventually improve the error message. |
What is the purpose to call fdatasync on readonly file descriptor? Can this call just be removed? |
One could remove In my experience, removing those functions likely won't avoid the underlying problem, but just going to force the kernel to raise the error at another system call. |
Dirty pages from man page are pages can only occurs when writing file, you working it read only. You wrote the file, closed it. No unwritten cache left. Then open it read-only. No dirty pages are possible here. But I think you right - it will crash later on probably due to posix_fadvise() call. Yet I think if we can continue, we shall continue testing next files, do not exit(saved_errno) on first error. At this point we know flash drive has corrupted metadata (few sectors at start of the drive with filesystem metadata) and probably files list is partially corrupted. But we still have 99% surface to check. |
Most users run
Continuing assumes that something else will work, but errors can compound. Especially in cases like this one in which there shouldn't be an error to start with. I expect that we'll learn enough about this error with time, so we'll find the root cause. The overheating error that
If you want to investigate your fake drive, take a look at |
fclose man page explicitly saying: "Any unwritten buffered data are flushed to the OS. Any unread buffered data are discarded. ". You do not even need to call fsync before fclose for that reason, for data get written to the drive. It is safe to call fclose on file descriptor to make all data written. Even if you crash your OS immediately after fclose, all data should be fine (but not guaranteed). fdatasync working with specific fd, freshly opened, I do not think kernel allow to work / flush cache pages from different application / file descriptor, even if here some dirty pages left. Most likely it will do nothing. I think what causing the crash is last access attribute, which have been written during fdatasync call (I know it not suppose to do that). So it is not important, and we shall continue. I'm using f3brew at first all the time. Question is, is the f3read suppose to handle errors better, assuming we are working with broken / bad hardware or not. I think it should assume bad hardware and not stop on simple errors. I would add more error handling like skipping files and showing read errors. Instead of stopping the app. EDIT: note about fdatasync attributes, and f3read error stability |
Your hypothesis makes sense, but I'm not changing the code without further evidence that it would be okay to remove
All code in F3 does a good job handling errors. I'm reluctant to quickly generalize how to handle new errors because fake drives and dying drives do behave in non-logical ways. As evidence accumulates on a given error, the path forward becomes clear. |
The text was updated successfully, but these errors were encountered: