On Tue, Jul 30, 2013 at 7:01 PM, majianpeng <majianpeng@xxxxxxxxx> wrote:
>>On Mon, Jul 29, 2013 at 11:00 AM, majianpeng <majianpeng@xxxxxxxxx> wrote:
>>>
>>> [snip]
>>> >I don't think the later was_short can handle the hole case. For the hole case,
>>> >we should try reading next strip object instead of return. how about
>>> >below patch.
>>> >
>>> Hi Yan,
>>> i uesed this demo to test hole case.
>>> dd if=/dev/urandom bs=4096 count=2 of=file_with_holes
>>> dd if=/dev/urandom bs=4096 seek=7 count=2 of=file_with_holes
>>>
>>> dd if=file_with_holes of=/dev/null bs=16k count=1 iflag=direct
>>> Using the dynamic_debug in striped_read, the message are:
>>> >[ 8743.663499] ceph: file.c:350 : striped_read 0~16384 (read 0) got 16384
>>> >[ 8743.663502] ceph: file.c:390 : striped_read returns 16384
>>> From the messages, we can see it can't hit the short-read.
>>> For the ceph-file-hole, how does the ceph handle?
>>> Or am i missing something?
>>
>>the default strip size is 4M, all data are written to the first object
>>in your test case.
>>could you try something like below.
>>
>
>>dd if=/dev/urandom bs=1M count=2 of=file_with_holes
>>dd if=/dev/urandom bs=1M count=2 seek=4 of=file_with_holes conv=notrunc
>>dd if=file_with_holes bs=8M >/dev/null
>>
> diff --git a/fs/ceph/file.c b/fs/ceph/file.c
> index 2ddf061..22a98e5 100644
> --- a/fs/ceph/file.c
> +++ b/fs/ceph/file.c
> @@ -349,17 +349,17 @@ more:
> dout("striped_read %llu~%u (read %u) got %d%s%s\n", pos, left, read,
> ret, hit_stripe ? " HITSTRIPE" : "", was_short ? " SHORT" : "");
>
> - if (ret > 0) {
> - int didpages = (page_align + ret) >> PAGE_CACHE_SHIFT;
> + if (ret >= 0) {
> + int didpages = (page_align + this_len) >> 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);
> + if (was_short) {
> + dout(" zero gap %llu to %llu\n", pos + ret, pos + this_len);
> + ceph_zero_page_vector_range(page_align + ret,
> + this_len - ret, pages);
> }
> - pos += ret;
> + pos += this_len;
> read = pos - off;
> - left -= ret;
> + left -= this_len;
> page_pos += didpages;
> pages_left -= didpages;
>
> This patch can do those case. It only add ret== 0 in judgement 'ret > 0".
maybe we should add a i_size check. stop reading next strip object
when 'pos > i_size'
> But i think i will add a parameter about hit_hole. It will make the code easy to understand.
>
i think 'was_short' is equal to 'hit_hole'
Regards
Yan, Zheng
>
>
>>Regards
>>Yan, Zheng
>>
>>
>>>
>>>
>>> Thanks!
>>> Jianpeng Ma
>>>
>>> >Regards
>>> >Yan, Zheng
>>> >---
>>> >diff --git a/fs/ceph/file.c b/fs/ceph/file.c
>>> >index 271a346..6ca2921 100644
>>> >--- a/fs/ceph/file.c
>>> >+++ b/fs/ceph/file.c
>>> >@@ -350,16 +350,17 @@ more:
>>> > ret, hit_stripe ? " HITSTRIPE" : "", was_short ? " SHORT" : "");
>>> >
>>> > if (ret > 0) {
>>> >- int didpages = (page_align + ret) >> PAGE_CACHE_SHIFT;
>>> >+ int didpages = (page_align + this_len) >> 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);
>>> >+ if (was_short) {
>>> >+ dout(" zero gap %llu to %llu\n",
>>> >+ pos + ret, pos + this_len);
>>> >+ ceph_zero_page_vector_range(page_align + ret,
>>> >+ this_len - ret, page_pos);
>>> > }
>>> >- pos += ret;
>>> >+ pos += this_len;
>>> > read = pos - off;
>>> >- left -= ret;
>>> >+ left -= this_len;
>>> > page_pos += didpages;
>>> > pages_left -= didpages;
>>> >
> Thanks!
> Jianpeng Ma
>>On Mon, Jul 29, 2013 at 11:00 AM, majianpeng <majianpeng@xxxxxxxxx> wrote:
>>>
>>> [snip]
>>> >I don't think the later was_short can handle the hole case. For the hole case,
>>> >we should try reading next strip object instead of return. how about
>>> >below patch.
>>> >
>>> Hi Yan,
>>> i uesed this demo to test hole case.
>>> dd if=/dev/urandom bs=4096 count=2 of=file_with_holes
>>> dd if=/dev/urandom bs=4096 seek=7 count=2 of=file_with_holes
>>>
>>> dd if=file_with_holes of=/dev/null bs=16k count=1 iflag=direct
>>> Using the dynamic_debug in striped_read, the message are:
>>> >[ 8743.663499] ceph: file.c:350 : striped_read 0~16384 (read 0) got 16384
>>> >[ 8743.663502] ceph: file.c:390 : striped_read returns 16384
>>> From the messages, we can see it can't hit the short-read.
>>> For the ceph-file-hole, how does the ceph handle?
>>> Or am i missing something?
>>
>>the default strip size is 4M, all data are written to the first object
>>in your test case.
>>could you try something like below.
>>
>>dd if=/dev/urandom bs=1M count=2 of=file_with_holes
>>dd if=/dev/urandom bs=1M count=2 seek=4 of=file_with_holes conv=notrunc
>>dd if=file_with_holes bs=8M >/dev/null
>>
>>Regards
>>Yan, Zheng
>>
>>
>>>
>>>
>>> Thanks!
>>> Jianpeng Ma
>>>
>>> >Regards
>>> >Yan, Zheng
>>> >---
>>> >diff --git a/fs/ceph/file.c b/fs/ceph/file.c
>>> >index 271a346..6ca2921 100644
>>> >--- a/fs/ceph/file.c
>>> >+++ b/fs/ceph/file.c
>>> >@@ -350,16 +350,17 @@ more:
>>> > ret, hit_stripe ? " HITSTRIPE" : "", was_short ? " SHORT" : "");
>>> >
>>> > if (ret > 0) {
>>> >- int didpages = (page_align + ret) >> PAGE_CACHE_SHIFT;
>>> >+ int didpages = (page_align + this_len) >> 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);
>>> >+ if (was_short) {
>>> >+ dout(" zero gap %llu to %llu\n",
>>> >+ pos + ret, pos + this_len);
>>> >+ ceph_zero_page_vector_range(page_align + ret,
>>> >+ this_len - ret, page_pos);
>>> > }
>>> >- pos += ret;
>>> >+ pos += this_len;
>>> > read = pos - off;
>>> >- left -= ret;
>>> >+ left -= this_len;
>>> > page_pos += didpages;
>>> > pages_left -= didpages;
>>> >
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
