From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jeff Moyer Subject: Re: [PATCH for 3.2] fs/direct-io.c: Calculate fs_count correctly in get_more_blocks. Date: Mon, 31 Oct 2011 14:12:09 -0400 Message-ID: References: <20111029105856.GA6479@infradead.org> <1320045873-3956-1-git-send-email-tm@tao.ma> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: linux-fsdevel@vger.kernel.org, linux-devel@vger.kernel.org, Christoph Hellwig , Al Viro , Andrew Morton To: Tao Ma Return-path: Received: from mx1.redhat.com ([209.132.183.28]:45875 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932287Ab1JaSMQ (ORCPT ); Mon, 31 Oct 2011 14:12:16 -0400 In-Reply-To: <1320045873-3956-1-git-send-email-tm@tao.ma> (Tao Ma's message of "Mon, 31 Oct 2011 15:24:33 +0800") Sender: linux-fsdevel-owner@vger.kernel.org List-ID: Tao Ma writes: > From: Tao Ma > > In get_more_blocks, we use dio_count to calculate fs_count and do some > tricky things to increase fs_count if dio_count isn't aligned. But > actually it still has some cornor case that can't be coverd. See the > following example: > ./dio_write foo -s 1024 -w 4096(direct write 4096 bytes at offset 1024). > The same goes if the offset isn't aligned to fs_blocksize. > > In this case, the old calculation counts fs_count to be 1, but actually > we will write into 2 different blocks(if fs_blocksize=4096). The old code > just works, since it will call get_block twice(and may have to allocate > and create extent twice for file systems like ext4). So we'd better call > get_block just once with the proper fs_count. This description was *really* hard for me to understand. It seems to me that right now there's an inefficiency in the code. It's not clear whether you're claiming that it was introduced recently, though. Was it, or has this problem been around for a while? How did you notice this? Was there any evidence of a problem, such as performance overhead or less than ideal file layout? Anyway, I agree that the code does not correctly calculate the number of file system blocks in a request. I also agree that your patch fixes that issue. Please ammend the description and then you can add my: Acked-by: Jeff Moyer Cheers, Jeff