[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 2/2] raw-posix: SEEK_HOLE suffices, get rid of F
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH 2/2] raw-posix: SEEK_HOLE suffices, get rid of FIEMAP |
Date: |
Thu, 13 Nov 2014 09:26:01 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) |
Fam Zheng <address@hidden> 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 <address@hidden>
Thanks!
- [Qemu-devel] [PATCH 0/2] raw-posix: Get rid of FIEMAP, Markus Armbruster, 2014/11/12
- [Qemu-devel] [PATCH 2/2] raw-posix: SEEK_HOLE suffices, get rid of FIEMAP, Markus Armbruster, 2014/11/12
- Re: [Qemu-devel] [PATCH 2/2] raw-posix: SEEK_HOLE suffices, get rid of FIEMAP, Fam Zheng, 2014/11/12
- Re: [Qemu-devel] [PATCH 2/2] raw-posix: SEEK_HOLE suffices, get rid of FIEMAP,
Markus Armbruster <=
- Re: [Qemu-devel] [PATCH 2/2] raw-posix: SEEK_HOLE suffices, get rid of FIEMAP, Eric Blake, 2014/11/12
- Re: [Qemu-devel] [PATCH 2/2] raw-posix: SEEK_HOLE suffices, get rid of FIEMAP, Markus Armbruster, 2014/11/13
- Re: [Qemu-devel] [PATCH 2/2] raw-posix: SEEK_HOLE suffices, get rid of FIEMAP, Kevin Wolf, 2014/11/13
- Re: [Qemu-devel] [PATCH 2/2] raw-posix: SEEK_HOLE suffices, get rid of FIEMAP, Max Reitz, 2014/11/13
- Re: [Qemu-devel] [PATCH 2/2] raw-posix: SEEK_HOLE suffices, get rid of FIEMAP, Kevin Wolf, 2014/11/13
- Re: [Qemu-devel] [PATCH 2/2] raw-posix: SEEK_HOLE suffices, get rid of FIEMAP, Max Reitz, 2014/11/13
- Re: [Qemu-devel] [PATCH 2/2] raw-posix: SEEK_HOLE suffices, get rid of FIEMAP, Markus Armbruster, 2014/11/13
- Re: [Qemu-devel] [PATCH 2/2] raw-posix: SEEK_HOLE suffices, get rid of FIEMAP, Kevin Wolf, 2014/11/13
Re: [Qemu-devel] [PATCH 2/2] raw-posix: SEEK_HOLE suffices, get rid of FIEMAP, Max Reitz, 2014/11/13