>On Fri, Jul 26, 2013 at 9:38 AM, majianpeng <majianpeng@xxxxxxxxx> wrote:
>>>On Fri, Jul 26, 2013 at 9:22 AM, majianpeng <majianpeng@xxxxxxxxx> wrote:
>>>>>On Fri, Jul 26, 2013 at 8:48 AM, majianpeng <majianpeng@xxxxxxxxx> wrote:
>>>>>>>On Thu, 25 Jul 2013, Yan, Zheng wrote:
>>>>>>>> On Thu, Jul 25, 2013 at 2:55 PM, majianpeng <majianpeng@xxxxxxxxx> wrote:
>>>>>>>> >>On Thu, 25 Jul 2013, majianpeng wrote:
>>>>>>>> >>> Hi all,
>>>>>>>> >>> I met a problem and ask somebody could help me.
>>>>>>>> >>> In func striped_read()
>>>>>>>> >>> > 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);
>>>>>>>> >>> > }
>>>>>>>> >>> > pos += ret;
>>>>>>>>
>>>>>>>> I think you are right. probably above line should be 'pos += this_len'
>>>>>>>
>>>>>>>It should be easy to construct a simple test for this. E.g., something
>>>>>>>like
>>>>>>>
>>>>>>> pwrite(fd, buf, 0, 3000000);
>>>>>>> pwrite(fd, buf, 4194304, 1000);
>>>>>>> pread(fd, buf, 0, 6000000);
>>>>>>> ...
>>>>>>>
>>>>>>>and whatever else to verify that pos was in fact advanced properly?
>>>>>>>
>>>>>> The following is my test code:
>>>>>> void hole_test()
>>>>>> {
>>>>>> char buf[4194304];
>>>>>> ssize_t ret;
>>>>>> int fd = open("/media/ceph/test", O_RDWR|O_CREAT|O_DIRECT|O_TRUNC);
>>>>>> if (fd < 0) {
>>>>>> printf("open error %s\n", strerror(errno));
>>>>>> return;
>>>>>> }
>>>>>>
>>>>>> ret = pwrite(fd, buf, 0, 3000000);
>>>>>> ret = pwrite(fd, buf, 4194304, 1000);
>>>>>> ret = pread(fd, buf , 0, 6000000);
>>>>>> close(fd);
>>>>>> }
>>>>>>
>>>>>> The debug message from striped_read are:
>>>>>> [ 267.530266] ceph: file.c:356 : striped_read 6000000~0 (read 0) got 0
>>>>>> [ 267.530270] ceph: file.c:396 : striped_read returns 0
>>>>>>
>>>>>> The result isn't what's your said.
>>>>>> Am i missing something?
>>>>>>
>>>>>> BTW, i think Yan is ok,
>>>>>> The code
>>>>>> pos += ret; should pos += this_len.
>>>>>> Because this_len can larger than ret.
>>>>>> But the question is what's condition can cause this?
>>>>>
>>>>>by default, ceph strips file to 4M objects. In above example, the
>>>>>first object only has
>>>>>3M data, so 'ret = 3M' and 'this_len = 4M'
>>>> actually, in func calc_layout: the read length is the smaller between object and left.
>>>>>
>>>>>> And can the short_read operation handle this situation?
>>>>>
>>>>>I don't think it's good idea to short read unless we really reach EOF.
>>>> Can you explain in detail?
>>>>
>>>
>>>because some user program interpret short read as EOF reached. If we
>>>return short
>>>read they get confused.
>> The reason cause short-read is only EOF? or other reason?
>>
>
>I think yes, (at least for reading from FS and read size is not very large)
we can remove those code:
diff --git a/fs/ceph/file.c b/fs/ceph/file.c
index 2ddf061..94fa378 100644
--- a/fs/ceph/file.c
+++ b/fs/ceph/file.c
@@ -352,11 +352,6 @@ more:
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);
- }
pos += ret;
read = pos - off;
left -= ret;
Becase in fun striped_read, the short-read have two reasons:eof or met a hole.
In either case ,the later was_short handler can handle.
Is it ok?
Thanks
Jianpeng Ma
?韬{.n?????%??檩??w?{.n????u朕?Ф?塄}?财??j:+v?? ? ???2??璀??摺?囤??z夸z罐?+?????w棹f
