Fam Zheng <f...@redhat.com> writes: > On Wed, 11/12 20:27, Markus Armbruster wrote: >> Commit 5500316 (May 2012) implemented raw_co_is_allocated() as >> follows: >> >> 1. If defined(CONFIG_FIEMAP), use the FS_IOC_FIEMAP ioctl >> >> 2. Else if defined(SEEK_HOLE) && defined(SEEK_DATA), use lseek() >> >> 3. Else pretend there are no holes >> >> Later on, raw_co_is_allocated() was generalized to >> raw_co_get_block_status(). >> >> Commit 4f11aa8 (May 2014) changed it to try the three methods in order >> until success, because "there may be implementations which support >> [SEEK_HOLE/SEEK_DATA] but not [FIEMAP] (e.g., NFSv4.2) as well as vice >> versa." >> >> Unfortunately, we used FIEMAP incorrectly: we lacked FIEMAP_FLAG_SYNC. >> Commit 38c4d0a (Sep 2014) added it. Because that's a significant >> speed hit, the next commit 38c4d0a put SEEK_HOLE/SEEK_DATA first. > > s/38c4d0a/7c159037/
Fixing... >> As you see, the obvious use of FIEMAP is wrong, and the correct use is >> slow. I guess this puts it somewhere between -7 "The obvious use is >> wrong" and -10 "It's impossible to get right" on Rusty Russel's Hard >> to Misuse scale[*]. > > Nice reading :) Adapted from a comment Paolo made in a discussion preceding this patch :) [...] > Other than the wrong commit id in message, > > Reviewed-by: Fam Zheng <f...@redhat.com> Thanks!
