On Wed, Jul 31, 2013 at 3:32 PM, majianpeng <[email protected]> wrote:
>
> [snip]
> Test case
> A: touch file
> dd if=file of=/dev/null bs=5M count=1 iflag=direct
> B: [data(2M)|hole(2m)][data(2M)]
> dd if=file of=/dev/null bs=8M count=1 iflag=direct
> C: [data(4M)[hole(4M)][hole(4M)][data(2M)]
> dd if=file of=/dev/null bs=16M count=1 iflag=direct
> D: touch file;truncate -s 5M file
> dd if=file of=/dev/null bs=8M count=1 iflag=direct
>
> Those cases can work.
> Now i make different processing for short-read between 'ret > 0' and "ret
> =0".
> For the short-read which ret > 0, it don't do read-page rather than zero the
> left area.
> This means reduce one meaningless read operation.
>
This patch looks good. But I still hope not to duplicate code.
how about change
"hit_stripe = this_len < left;"
to
"hit_stripe = this_len < left && (ret == this_len || pos + this_len <
inode->i_size);"
> diff --git a/fs/ceph/file.c b/fs/ceph/file.c
> index 2ddf061..f643ec8 100644
> --- a/fs/ceph/file.c
> +++ b/fs/ceph/file.c
> @@ -350,13 +350,17 @@ more:
> ret, hit_stripe ? " HITSTRIPE" : "", was_short ? " SHORT" : "");
>
> if (ret > 0) {
changes this to "(ret >= 0)"
> - int didpages = (page_align + ret) >> PAGE_CACHE_SHIFT;
> -
> - if (read < pos - off) {
> diff --git a/fs/ceph/file.c b/fs/ceph/file.c
> index 2ddf061..f643ec8 100644
> --- a/fs/ceph/file.c
> +++ b/fs/ceph/file.c
> @@ -350,13 +350,17 @@ more:
> ret, hit_stripe ? " HITSTRIPE" : "", was_short ? " SHORT" : "");
>
> if (ret > 0) {
> - int didpages = (page_align + ret) >> PAGE_CACHE_SHIFT;
> -
> - if (read < pos - off) {
> - dout(" zero gap %llu to %llu\n", off + read, pos);
> - ceph_zero_page_vector_range(page_align + read,
> - pos - off - read, pages);
> + int didpages;
> +
> + if (was_short && pos + ret < inode->i_size) {
change this to "(was_short && hit_stripe)"
by this way, we can avoid modifying the "zero trailing bytes" code.
Regards,
Yan, Zheng
> + u64 tmp = min(this_len - ret,
> + inode->i_size - pos - ret);
> + dout(" zero gap %llu to %llu\n", pos + ret, pos +
> ret + tmp);
> + ceph_zero_page_vector_range(page_align + read + ret,
> + tmp, pages);
> + ret += tmp;
> }
> + didpages = (page_align + ret) >> PAGE_CACHE_SHIFT;
> pos += ret;
> read = pos - off;
> left -= ret;
> @@ -375,13 +379,25 @@ more:
>
> /* zero trailing bytes (inside i_size) */
> if (left > 0 && pos < inode->i_size) {
> + int didpages;
> if (pos + left > inode->i_size)
> left = inode->i_size - pos;
>
> - dout("zero tail %d\n", left);
> - ceph_zero_page_vector_range(page_align + read, left,
> + ret = min(left, this_len);
> + dout("zero tail %d\n", ret);
> + ceph_zero_page_vector_range(page_align + read, ret,
> pages);
> - read += left;
> + didpages = (page_align + ret) >> PAGE_CACHE_SHIFT;
> +
> + pos += ret;
> + read = pos - off;
> + left -= ret;
> + page_pos += didpages;
> + pages_left -= didpages;
> +
> + /* hit stripe? */
> + if (left && hit_stripe)
> + goto more;
> }
> }
>
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
