From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38637) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YHDtc-0004Ar-JM for qemu-devel@nongnu.org; Fri, 30 Jan 2015 10:54:13 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YHDtX-0008NJ-IN for qemu-devel@nongnu.org; Fri, 30 Jan 2015 10:54:12 -0500 Received: from mx2.parallels.com ([199.115.105.18]:47362) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YHDtX-0008N9-Cb for qemu-devel@nongnu.org; Fri, 30 Jan 2015 10:54:07 -0500 Message-ID: <54CBA915.9080305@openvz.org> Date: Fri, 30 Jan 2015 18:53:57 +0300 From: "Denis V. Lunev" MIME-Version: 1.0 References: <1422607337-25335-1-git-send-email-den@openvz.org> <1422607337-25335-6-git-send-email-den@openvz.org> <54CB9C12.9040406@redhat.com> <54CBA621.9080807@openvz.org> <54CBA683.4070809@redhat.com> In-Reply-To: <54CBA683.4070809@redhat.com> Content-Type: text/plain; charset="iso-8859-15"; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 5/7] block/raw-posix: call plain fallocate in handle_aiocb_write_zeroes List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Max Reitz Cc: Kevin Wolf , Fam Zheng , Peter Lieven , qemu-devel@nongnu.org, Stefan Hajnoczi On 30/01/15 18:42, Max Reitz wrote: > On 2015-01-30 at 10:41, Denis V. Lunev wrote: >> On 30/01/15 17:58, Max Reitz wrote: >>> On 2015-01-30 at 03:42, Denis V. Lunev wrote: >>>> There is a possibility that we are extending our image and thus >>>> writing >>>> zeroes beyond the end of the file. In this case we do not need to care >>>> about the hole to make sure that there is no data in the file under >>>> this offset (pre-condition to fallocate(0) to work). We could >>>> simply call >>>> fallocate(0). >>>> >>>> This improves the performance of writing zeroes even on really old >>>> platforms which do not have even FALLOC_FL_PUNCH_HOLE. >>>> >>>> Before the patch do_fallocate was used when either >>>> CONFIG_FALLOCATE_PUNCH_HOLE or CONFIG_FALLOCATE_ZERO_RANGE are >>>> defined. >>>> Now the story is different. CONFIG_FALLOCATE is defined when Linux >>>> fallocate is defined, posix_fallocate is completely different story >>>> (CONFIG_POSIX_FALLOCATE). CONFIG_FALLOCATE is mandatory prerequite >>>> for both CONFIG_FALLOCATE_PUNCH_HOLE and CONFIG_FALLOCATE_ZERO_RANGE >>>> thus we are on the safe side. >>>> >>>> Signed-off-by: Denis V. Lunev >>>> CC: Max Reitz >>>> CC: Kevin Wolf >>>> CC: Stefan Hajnoczi >>>> CC: Peter Lieven >>>> CC: Fam Zheng >>>> --- >>>> block/raw-posix.c | 14 +++++++++++++- >>>> 1 file changed, 13 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/block/raw-posix.c b/block/raw-posix.c >>>> index 5a777e7..1c88ad8 100644 >>>> --- a/block/raw-posix.c >>>> +++ b/block/raw-posix.c >>>> @@ -147,6 +147,7 @@ typedef struct BDRVRawState { >>>> bool has_discard:1; >>>> bool has_write_zeroes:1; >>>> bool discard_zeroes:1; >>>> + bool has_fallocate; >>>> bool needs_alignment; >>>> } BDRVRawState; >>>> @@ -452,6 +453,7 @@ static int raw_open_common(BlockDriverState >>>> *bs, QDict *options, >>>> } >>>> if (S_ISREG(st.st_mode)) { >>>> s->discard_zeroes = true; >>>> + s->has_fallocate = true; >>> >>> This could be moved upwards where has_discard and has_write_zeroes >>> are initialized; but it won't matter in practice, I hope. Thus: >>> >>> Reviewed-by: Max Reitz >> >> This does matter as has_discard and has_write_zeroes are bit fields >> thus I can not insert something useful into the middle of those >> fields. > > Right, but I did not mean the placement inside of the structure but > the placement of the initialization statement (s->has_fallocate = > true) in raw_open_common(). > > Max hmm, you are right. This is possible but I don't want to have this bit set for block/character etc devices even if they are not using this bit/code. With my approach the assignment is made in a way to indicate application area. Thank you for a review :) It is somewhat difficult to obtain feedback here in comparison with Linux kernel.