From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tao Ma Subject: Re: [PATCH for 3.2] fs/direct-io.c: Calculate fs_count correctly in get_more_blocks. Date: Tue, 01 Nov 2011 11:31:14 +0800 Message-ID: <4EAF6802.2060005@tao.ma> References: <20111029105856.GA6479@infradead.org> <1320045873-3956-1-git-send-email-tm@tao.ma> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Cc: linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org, Christoph Hellwig , Al Viro , Andrew Morton To: Jeff Moyer Return-path: In-Reply-To: Sender: linux-kernel-owner@vger.kernel.org List-Id: linux-fsdevel.vger.kernel.org On 11/01/2011 02:12 AM, Jeff Moyer wrote: > 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? Actually it is there a long time ago. And the good thing is that it isn't a bug, only some performance overhead. > > How did you notice this? Was there any evidence of a problem, such as > performance overhead or less than ideal file layout? I found it when I dig into some ext4 issues. The ext4 can't create the whole 8K(in the above case) and ext4 has to create the blocks 2 times for just one direct i/o write. In some of our test, it costs. > > 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: So how about the following commit log(please feel free to modify it if I still don't describe it correctly). In get_more_blocks, we use dio_count to calculate fs_count to let the file system map(maybe also create) blocks. And some tricky things are done 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). 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). So the underlying file system is called twice and leads to some performance overhead. So fix it by calculating fs_count correctly and let the file system knows what we really want to write. Thanks Tao > > Acked-by: Jeff Moyer > > Cheers, > Jeff > -- > To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html