I didn't see your message until now. Sorry. On 1/24/24 9:13 AM, Luis Henriques wrote:
Bellow, I'm inlining a patch that started as debug patch that I've used to try to understand what was going on. It seems to workaround that bug, but I know it's not a real fix -- I don't yet understand what's going on.
Thanks for this. I'm not sure if you meant to say you think it works around the present issue. I just tested it, and it does not. In case you missed the start of the thread, here is the test I gave for triggering the issue:
$ rm -f test-file; dd if=/dev/zero of=test-file bs=64 count=3 status=none; lsattr test-file
Instead of writing the file all at once, it splits it into 3 writes, where the first is small enough to make the file inline, and then it becomes non-inline. Ideally, the output should be
--------------e------- test-file but delayed allocation means it instead shows ------------------N--- test-file until sync. I also gave this code for testing SEEK_HOLE: https://gist.github.com/ddawson/22cfd4cac32916f6f1dcc86f90eed21a
Regarding your specific usecase, I can reproduce it and, unfortunately, I don't thing Ted's suggestion will fix it as I don't even see ext4_iomap_begin_report() being executed at all.
To be clear, that function is called in a few specific circumstances, such as when lseek() is called with SEEK_HOLE or SEEK_DATA, or with FIEMAP. When I traced the kernel myself, I did see it being executed from the lseek() call. The changes are to address the file not yet being converted from inline, where the contents are still written where the map would otherwise be. If you treat it as the map, you get nonsense. Something else needs to be done.
I'm not clear on whether his proposed changes would then allow an application to function properly under such a condition, but it should at least *not* give ENOENT.
After testing what I think are the changes he proposed, I find it doesn't work. If I remove the "&& iomap->type == IOMAP_HOLE", lseek() no longer gives an error, but instead returns 0, which I'm pretty sure won't work for the affected use case. Either way, I'm not sure I interpreted his description of the changes correctly.
-- PGP fingerprint: 5BBD5080FEB0EF7F142F8173D572B791F7B4422A
