On 11/01/2011 02:12 AM, Jeff Moyer wrote: > Tao Ma <tm@xxxxxx> writes: > >> From: Tao Ma <boyu.mt@xxxxxxxxxx> >> >> 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 <jmoyer@xxxxxxxxxx> > > Cheers, > Jeff > -- > To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
