Re: [PATCH for 3.2] fs/direct-io.c: Calculate fs_count correctly in get_more_blocks.

[Jeff Moyer <jmoyer@redhat.com>]

Tao Ma <tm@tao.ma> writes:

> From: Tao Ma <boyu.mt@taobao.com>
>
> 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 <jmoyer@redhat.com>

Cheers,
Jeff