Re: Re: question about striped_read

[majianpeng <majianpeng@gmail.com>]

>On Thu, 25 Jul 2013, Yan, Zheng wrote:
>> On Thu, Jul 25, 2013 at 2:55 PM, majianpeng <majianpeng@gmail.com> 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?
And can the short_read operation handle this situation?

>sage
>
>> 
>> regards
>> yan, zheng
>> 
>> 
>> >>> >               read = pos - off;
>> >>>
>> >>> At first , pos = off and off don't modify.
>> >>> Why does it judge 'read < pos -off ' ?
>> >>> Because the read = pos -off, so the read must equal pos -off.
>> >>
>> >>This block triggers if we hit a stripe.  off is always the original
>> >>starting offset.  pos is the current position that we just tried to read
>> >>from.  We might:
>> >>
>> >> - try to read a big extent from off == pos
>> >> - readpages truncates this to the end of the object
>> >> - we get a short read (not a complete object)
>> > What's the mean of not a complete object?
>> > FYI,a short read maybe  met the boundry of stripe(object) or the EOF of file.
>> > For the hole of file, i don't know how to handle.
>> >> - read is some partial amount
>> >> - pos is off + read + a gap
>> > From the code : read = pos - off; after that, the pos don't change.
>> > So the a gap where is from?
>> >
>> > Thanks!
>> > Jianpeng Ma
>> >> - we try to read from the next object and there is data
>> >>   => there was a hole and we need to zero the gap
>> >>
>> >>If, on the other hand, there isn't data in the next object, then we don't
>> >>want to zero the hole.. and least not until we decide where EOF is.
>> >>That's why this block is where it is, and never triggers on the first
>> >>iteration.
>> >>
>> >>Does that make sense?
>> >>sage
>> 
>>