| Message ID | 55EF9184.5090007@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 2015/9/9 9:55, jiangyiwen wrote: > A simplified test case is (this case from Ryan): > 1) dd if=/dev/zero of=/mnt/hello bs=512 count=1 oflag=direct; > 2) truncate /mnt/hello -s 2097152 > file 'hello' is not exist before test. After this command, > file 'hello' should be all zero. But 512~4096 is some random data. > > Setting bh state to new when get a new block, if so, > direct_io_worker()->dio_zero_block() will fill-in the unused portion > of the block with zero. > > Signed-off-by: Yiwen Jiang <jiangyiwen@huawei.com> Reviewed-by: Joseph Qi <joseph.qi@huawei.com> > --- > fs/ocfs2/aops.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/fs/ocfs2/aops.c b/fs/ocfs2/aops.c > index 1a35c61..bd106b9 100644 > --- a/fs/ocfs2/aops.c > +++ b/fs/ocfs2/aops.c > @@ -581,6 +581,7 @@ static int ocfs2_direct_IO_get_blocks(struct inode *inode, sector_t iblock, > ret = -EIO; > goto bail; > } > + set_buffer_new(bh_result); > } > > /* >
Hi Yiwen, I try to reproduce this case, but it didn't act like you describe. What I did: === laptop:/mnt/shared # mount | grep ocfs2 ocfs2_dlmfs on /dlm type ocfs2_dlmfs (rw,relatime) /dev/sda3 on /mnt/shared type ocfs2 (rw,relatime,_netdev,heartbeat=local,nointr,data=ordered,errors=remount-ro,atime_quantum=60,coherency=full,user_xattr,acl) laptop:/mnt/shared # dd if=/dev/zero of=/mnt/shared/hello bs=512 count=1 oflag=direct 1+0 records in 1+0 records out 512 bytes (512 B) copied, 0.000684382 s, 748 kB/s laptop:/mnt/shared # truncate hello -s 2097152 laptop:/mnt/shared # cat hello ======> nothing laptop:/mnt/shared # uname -r 3.16.7-21-desktop === Did I do something wrong? If I misunderstand, please correct me. Thanks. >>> > A simplified test case is (this case from Ryan): > 1) dd if=/dev/zero of=/mnt/hello bs=512 count=1 oflag=direct; > 2) truncate /mnt/hello -s 2097152 > file 'hello' is not exist before test. After this command, > file 'hello' should be all zero. But 512~4096 is some random data. > > Setting bh state to new when get a new block, if so, > direct_io_worker()->dio_zero_block() will fill-in the unused portion > of the block with zero. > > Signed-off-by: Yiwen Jiang <jiangyiwen@huawei.com> > --- > fs/ocfs2/aops.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/fs/ocfs2/aops.c b/fs/ocfs2/aops.c > index 1a35c61..bd106b9 100644 > --- a/fs/ocfs2/aops.c > +++ b/fs/ocfs2/aops.c > @@ -581,6 +581,7 @@ static int ocfs2_direct_IO_get_blocks(struct inode > *inode, sector_t iblock, > ret = -EIO; > goto bail; > } > + set_buffer_new(bh_result); > } > > /* -- Eric, Ren
Hi Zhen, On 2015/9/9 13:16, Zhen Ren wrote: > Hi Yiwen, > > I try to reproduce this case, but it didn't act like you describe. > What I did: > === > laptop:/mnt/shared # mount | grep ocfs2 > ocfs2_dlmfs on /dlm type ocfs2_dlmfs (rw,relatime) > /dev/sda3 on /mnt/shared type ocfs2 (rw,relatime,_netdev,heartbeat=local,nointr,data=ordered,errors=remount-ro,atime_quantum=60,coherency=full,user_xattr,acl) > laptop:/mnt/shared # dd if=/dev/zero of=/mnt/shared/hello bs=512 count=1 oflag=direct > 1+0 records in > 1+0 records out > 512 bytes (512 B) copied, 0.000684382 s, 748 kB/s > laptop:/mnt/shared # truncate hello -s 2097152 > laptop:/mnt/shared # cat hello ======> nothing I don't think 'cat' works here. Could you please use hexdump? > laptop:/mnt/shared # uname -r > 3.16.7-21-desktop > === > > Did I do something wrong? If I misunderstand, please correct me. > Thanks. > > >>> >> A simplified test case is (this case from Ryan): >> 1) dd if=/dev/zero of=/mnt/hello bs=512 count=1 oflag=direct; >> 2) truncate /mnt/hello -s 2097152 >> file 'hello' is not exist before test. After this command, >> file 'hello' should be all zero. But 512~4096 is some random data. >> >> Setting bh state to new when get a new block, if so, >> direct_io_worker()->dio_zero_block() will fill-in the unused portion >> of the block with zero. >> >> Signed-off-by: Yiwen Jiang <jiangyiwen@huawei.com> >> --- >> fs/ocfs2/aops.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/fs/ocfs2/aops.c b/fs/ocfs2/aops.c >> index 1a35c61..bd106b9 100644 >> --- a/fs/ocfs2/aops.c >> +++ b/fs/ocfs2/aops.c >> @@ -581,6 +581,7 @@ static int ocfs2_direct_IO_get_blocks(struct inode >> *inode, sector_t iblock, >> ret = -EIO; >> goto bail; >> } >> + set_buffer_new(bh_result); >> } >> >> /* > > > > > -- > Eric, Ren > > > > > _______________________________________________ > Ocfs2-devel mailing list > Ocfs2-devel@oss.oracle.com > https://oss.oracle.com/mailman/listinfo/ocfs2-devel > >
On 2015/9/9 13:16, Zhen Ren wrote: > Hi Yiwen, > > I try to reproduce this case, but it didn't act like you describe. > What I did: > === > laptop:/mnt/shared # mount | grep ocfs2 > ocfs2_dlmfs on /dlm type ocfs2_dlmfs (rw,relatime) > /dev/sda3 on /mnt/shared type ocfs2 (rw,relatime,_netdev,heartbeat=local,nointr,data=ordered,errors=remount-ro,atime_quantum=60,coherency=full,user_xattr,acl) > laptop:/mnt/shared # dd if=/dev/zero of=/mnt/shared/hello bs=512 count=1 oflag=direct > 1+0 records in > 1+0 records out > 512 bytes (512 B) copied, 0.000684382 s, 748 kB/s > laptop:/mnt/shared # truncate hello -s 2097152 > laptop:/mnt/shared # cat hello ======> nothing Please use this command to read binary file "hexdump -C /mnt/shared/hello", you will see the result like this: linux-ZnXfWq:/mnt/ocfs2 # hexdump -C hello 00000000 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................| * 00200000 it means in file area 512~4096 is zero not random data. > laptop:/mnt/shared # uname -r > 3.16.7-21-desktop > === > > Did I do something wrong? If I misunderstand, please correct me. > Thanks. > > >>> >> A simplified test case is (this case from Ryan): >> 1) dd if=/dev/zero of=/mnt/hello bs=512 count=1 oflag=direct; >> 2) truncate /mnt/hello -s 2097152 >> file 'hello' is not exist before test. After this command, >> file 'hello' should be all zero. But 512~4096 is some random data. >> >> Setting bh state to new when get a new block, if so, >> direct_io_worker()->dio_zero_block() will fill-in the unused portion >> of the block with zero. >> >> Signed-off-by: Yiwen Jiang <jiangyiwen@huawei.com> >> --- >> fs/ocfs2/aops.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/fs/ocfs2/aops.c b/fs/ocfs2/aops.c >> index 1a35c61..bd106b9 100644 >> --- a/fs/ocfs2/aops.c >> +++ b/fs/ocfs2/aops.c >> @@ -581,6 +581,7 @@ static int ocfs2_direct_IO_get_blocks(struct inode >> *inode, sector_t iblock, >> ret = -EIO; >> goto bail; >> } >> + set_buffer_new(bh_result); >> } >> >> /* > > > > > -- > Eric, Ren > > > > > . >
Hi Joseph and Yiwen, Sorry for that misoperation. This time, I did the same cmds using "hexdump" on ocfs2 and btrfs. The testing results look the same: == laptop:/mnt/shared # dd if=/dev/zero of=hello bs=512 count=1 oflag=direct ===> on ocfs2 1+0 records in 1+0 records out 512 bytes (512 B) copied, 0.000834866 s, 613 kB/s laptop:/mnt/shared # hexdump hello -C 00000000 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................| * 00000200 laptop:/mnt/shared # truncate hello -s 2097152 laptop:/mnt/shared # truncate hello -s 2097152 laptop:/mnt/shared # hexdump hello -C 00000000 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................| * 00200000 laptop:~ # dd if=/dev/zero of=hello bs=512 count=1 oflag=direct ===> on btrfs 1+0 records in 1+0 records out 512 bytes (512 B) copied, 0.000201759 s, 2.5 MB/s laptop:~ # hexdump hello -C 00000000 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................| * 00000200 laptop:~ # truncate hello -s 2097152 laptop:~ # hexdump hello -C 00000000 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................| * 00200000 == BTW, Yiwen, I'm confused by your comment and commit message. 1. >> file 'hello' should be all zero. But 512~4096 is some random data. 2. it means in file area 512~4096 is zero not random data. Anyway, I didn't look into this case, but just try to understand it. So, if there's any problem, let it go ;-) Thanks. -- Eric Ren >>> Joseph Qi <joseph.qi@huawei.com> 09/09/15 6:04 PM >>> Hi Zhen, On 2015/9/9 13:16, Zhen Ren wrote: > Hi Yiwen, > > I try to reproduce this case, but it didn't act like you describe. > What I did: > === > laptop:/mnt/shared # mount | grep ocfs2 > ocfs2_dlmfs on /dlm type ocfs2_dlmfs (rw,relatime) > /dev/sda3 on /mnt/shared type ocfs2 (rw,relatime,_netdev,heartbeat=local,nointr,data=ordered,errors=remount-ro,atime_quantum=60,coherency=full,user_xattr,acl) > laptop:/mnt/shared # dd if=/dev/zero of=/mnt/shared/hello bs=512 count=1 oflag=direct > 1+0 records in > 1+0 records out > 512 bytes (512 B) copied, 0.000684382 s, 748 kB/s > laptop:/mnt/shared # truncate hello -s 2097152 > laptop:/mnt/shared # cat hello ======> nothing I don't think 'cat' works here. Could you please use hexdump? > laptop:/mnt/shared # uname -r > 3.16.7-21-desktop > === > > Did I do something wrong? If I misunderstand, please correct me. > Thanks. > > >>> >> A simplified test case is (this case from Ryan): >> 1) dd if=/dev/zero of=/mnt/hello bs=512 count=1 oflag=direct; >> 2) truncate /mnt/hello -s 2097152 >> file 'hello' is not exist before test. After this command, >> file 'hello' should be all zero. But 512~4096 is some random data. >> >> Setting bh state to new when get a new block, if so, >> direct_io_worker()->dio_zero_block() will fill-in the unused portion >> of the block with zero. >> >> Signed-off-by: Yiwen Jiang <jiangyiwen@huawei.com> >> --- >> fs/ocfs2/aops.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/fs/ocfs2/aops.c b/fs/ocfs2/aops.c >> index 1a35c61..bd106b9 100644 >> --- a/fs/ocfs2/aops.c >> +++ b/fs/ocfs2/aops.c >> @@ -581,6 +581,7 @@ static int ocfs2_direct_IO_get_blocks(struct inode >> *inode, sector_t iblock, >> ret = -EIO; >> goto bail; >> } >> + set_buffer_new(bh_result); >> } >> >> /* > > > > > -- > Eric, Ren > > > > > _______________________________________________ > Ocfs2-devel mailing list > Ocfs2-devel@oss.oracle.com > https://oss.oracle.com/mailman/listinfo/ocfs2-devel > >
On 2015/9/9 18:42, Zhen Ren wrote: > Hi Joseph and Yiwen, > > Sorry for that misoperation. > > This time, I did the same cmds using "hexdump" on ocfs2 and btrfs. > The testing results look the same: > == > laptop:/mnt/shared # dd if=/dev/zero of=hello bs=512 count=1 oflag=direct ===> on ocfs2 > 1+0 records in > 1+0 records out > 512 bytes (512 B) copied, 0.000834866 s, 613 kB/s > laptop:/mnt/shared # hexdump hello -C > 00000000 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................| > * > 00000200 > laptop:/mnt/shared # truncate hello -s 2097152 > laptop:/mnt/shared # truncate hello -s 2097152 > laptop:/mnt/shared # hexdump hello -C > 00000000 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................| > * > 00200000 > > laptop:~ # dd if=/dev/zero of=hello bs=512 count=1 oflag=direct ===> on btrfs > 1+0 records in > 1+0 records out > 512 bytes (512 B) copied, 0.000201759 s, 2.5 MB/s > laptop:~ # hexdump hello -C > 00000000 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................| > * > 00000200 > laptop:~ # truncate hello -s 2097152 > laptop:~ # hexdump hello -C > 00000000 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................| > * > 00200000 > == > > BTW, Yiwen, I'm confused by your comment and commit message. > 1. >> file 'hello' should be all zero. But 512~4096 is some random data. > 2. it means in file area 512~4096 is zero not random data. > > Anyway, I didn't look into this case, but just try to understand it. > So, if there's any problem, let it go ;-) sorry, my description is wrong, 512~4096 is zero. Thanks, Yiwen Jiang > > Thanks. > -- > Eric Ren > >>>> Joseph Qi <joseph.qi@huawei.com> 09/09/15 6:04 PM >>> > Hi Zhen, > > On 2015/9/9 13:16, Zhen Ren wrote: >> Hi Yiwen, >> >> I try to reproduce this case, but it didn't act like you describe. >> What I did: >> === >> laptop:/mnt/shared # mount | grep ocfs2 >> ocfs2_dlmfs on /dlm type ocfs2_dlmfs (rw,relatime) >> /dev/sda3 on /mnt/shared type ocfs2 (rw,relatime,_netdev,heartbeat=local,nointr,data=ordered,errors=remount-ro,atime_quantum=60,coherency=full,user_xattr,acl) >> laptop:/mnt/shared # dd if=/dev/zero of=/mnt/shared/hello bs=512 count=1 oflag=direct >> 1+0 records in >> 1+0 records out >> 512 bytes (512 B) copied, 0.000684382 s, 748 kB/s >> laptop:/mnt/shared # truncate hello -s 2097152 >> laptop:/mnt/shared # cat hello ======> nothing > I don't think 'cat' works here. > Could you please use hexdump? > >> laptop:/mnt/shared # uname -r >> 3.16.7-21-desktop >> === >> >> Did I do something wrong? If I misunderstand, please correct me. >> Thanks. >> >> >>> >>> A simplified test case is (this case from Ryan): >>> 1) dd if=/dev/zero of=/mnt/hello bs=512 count=1 oflag=direct; >>> 2) truncate /mnt/hello -s 2097152 >>> file 'hello' is not exist before test. After this command, >>> file 'hello' should be all zero. But 512~4096 is some random data. >>> >>> Setting bh state to new when get a new block, if so, >>> direct_io_worker()->dio_zero_block() will fill-in the unused portion >>> of the block with zero. >>> >>> Signed-off-by: Yiwen Jiang <jiangyiwen@huawei.com> >>> --- >>> fs/ocfs2/aops.c | 1 + >>> 1 file changed, 1 insertion(+) >>> >>> diff --git a/fs/ocfs2/aops.c b/fs/ocfs2/aops.c >>> index 1a35c61..bd106b9 100644 >>> --- a/fs/ocfs2/aops.c >>> +++ b/fs/ocfs2/aops.c >>> @@ -581,6 +581,7 @@ static int ocfs2_direct_IO_get_blocks(struct inode >>> *inode, sector_t iblock, >>> ret = -EIO; >>> goto bail; >>> } >>> + set_buffer_new(bh_result); >>> } >>> >>> /* >> >> >> >> >> -- >> Eric, Ren >> >> >> >> >> _______________________________________________ >> Ocfs2-devel mailing list >> Ocfs2-devel@oss.oracle.com >> https://oss.oracle.com/mailman/listinfo/ocfs2-devel >> >> > > > > > > . >
Hi Yiwen, I'm working on this issue. The patch will be send out soon. And the issue that do not support file hole will be fixed too. I have proved it can pass all ltp-aiodio test cases, and has better performance. ;-) Thanks, Ryan On 09/09/2015 09:55 AM, jiangyiwen wrote: > A simplified test case is (this case from Ryan): > 1) dd if=/dev/zero of=/mnt/hello bs=512 count=1 oflag=direct; > 2) truncate /mnt/hello -s 2097152 > file 'hello' is not exist before test. After this command, > file 'hello' should be all zero. But 512~4096 is some random data. > > Setting bh state to new when get a new block, if so, > direct_io_worker()->dio_zero_block() will fill-in the unused portion > of the block with zero. > > Signed-off-by: Yiwen Jiang <jiangyiwen@huawei.com> > --- > fs/ocfs2/aops.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/fs/ocfs2/aops.c b/fs/ocfs2/aops.c > index 1a35c61..bd106b9 100644 > --- a/fs/ocfs2/aops.c > +++ b/fs/ocfs2/aops.c > @@ -581,6 +581,7 @@ static int ocfs2_direct_IO_get_blocks(struct inode *inode, sector_t iblock, > ret = -EIO; > goto bail; > } > + set_buffer_new(bh_result); > } > > /*
On Thu, 10 Sep 2015 09:53:11 +0800 ryding <ryan.ding@oracle.com> wrote: > Hi Yiwen, > > I'm working on this issue. The patch will be send out soon. And the > issue that do not support file hole will be fixed too. I have proved it > can pass all ltp-aiodio test cases, and has better performance. ;-) Yes, your "ocfs2: fill in the unused portion of the block with zeros by dio_zero_block()" conflicts with this patch a little. Does your patchset need alteration to fix this bug?
On 09/15/2015 06:22 AM, Andrew Morton wrote: > On Thu, 10 Sep 2015 09:53:11 +0800 ryding <ryan.ding@oracle.com> wrote: > >> Hi Yiwen, >> >> I'm working on this issue. The patch will be send out soon. And the >> issue that do not support file hole will be fixed too. I have proved it >> can pass all ltp-aiodio test cases, and has better performance. ;-) > Yes, your "ocfs2: fill in the unused portion of the block with zeros by > dio_zero_block()" conflicts with this patch a little. > > Does your patchset need alteration to fix this bug? My patchset "ocfs2: fix ocfs2 direct io code patch to support sparse file and data ordering semantics" has already include the fix to this bug. Yiwen's patch is no longer needed if my patchset is accepted.
diff --git a/fs/ocfs2/aops.c b/fs/ocfs2/aops.c index 1a35c61..bd106b9 100644 --- a/fs/ocfs2/aops.c +++ b/fs/ocfs2/aops.c @@ -581,6 +581,7 @@ static int ocfs2_direct_IO_get_blocks(struct inode *inode, sector_t iblock, ret = -EIO; goto bail; } + set_buffer_new(bh_result); } /*
A simplified test case is (this case from Ryan): 1) dd if=/dev/zero of=/mnt/hello bs=512 count=1 oflag=direct; 2) truncate /mnt/hello -s 2097152 file 'hello' is not exist before test. After this command, file 'hello' should be all zero. But 512~4096 is some random data. Setting bh state to new when get a new block, if so, direct_io_worker()->dio_zero_block() will fill-in the unused portion of the block with zero. Signed-off-by: Yiwen Jiang <jiangyiwen@huawei.com> --- fs/ocfs2/aops.c | 1 + 1 file changed, 1 insertion(+)