| Message ID | 20200620025033.4180077-1-zhangxiaoxu5@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | cifs/smb3: Fix data inconsistent when zero file range | expand |
пт, 19 июн. 2020 г. в 22:04, Zhang Xiaoxu <zhangxiaoxu5@huawei.com>: > > CIFS implements the fallocate(FALLOC_FL_ZERO_RANGE) with send SMB > ioctl(FSCTL_SET_ZERO_DATA) to server. It just set the range of the > remote file to zero, but local page cache not update, then the data > inconsistent with server, which leads the xfstest generic/008 failed. > > So we need to remove the local page caches before send SMB > ioctl(FSCTL_SET_ZERO_DATA) to server. After next read, it will > re-cache it. > > Reported-by: Hulk Robot <hulkci@huawei.com> > Signed-off-by: Zhang Xiaoxu <zhangxiaoxu5@huawei.com> > --- > fs/cifs/smb2ops.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c > index 736d86b8a910..250b51aca514 100644 > --- a/fs/cifs/smb2ops.c > +++ b/fs/cifs/smb2ops.c > @@ -3187,6 +3187,11 @@ static long smb3_zero_range(struct file *file, struct cifs_tcon *tcon, > trace_smb3_zero_enter(xid, cfile->fid.persistent_fid, tcon->tid, > ses->Suid, offset, len); > > + /* > + * We zero the range through ioctl, so we need remove the page caches > + * first, otherwise the data may be inconsistent with the server. > + */ > + truncate_pagecache_range(inode, offset, offset + len - 1); > > /* if file not oplocked can't be sure whether asking to extend size */ > if (!CIFS_CACHE_READ(cifsi)) > -- > 2.25.4 > Looks good! Reviewed-by: Pavel Shilovsky <pshilov@microsoft.com> Don't we need to do the same for smb3_punch_hole()? -- Best regards, Pavel Shilovsky
在 2020/6/23 1:46, Pavel Shilovsky 写道: > пт, 19 июн. 2020 г. в 22:04, Zhang Xiaoxu <zhangxiaoxu5@huawei.com>: >> >> CIFS implements the fallocate(FALLOC_FL_ZERO_RANGE) with send SMB >> ioctl(FSCTL_SET_ZERO_DATA) to server. It just set the range of the >> remote file to zero, but local page cache not update, then the data >> inconsistent with server, which leads the xfstest generic/008 failed. >> >> So we need to remove the local page caches before send SMB >> ioctl(FSCTL_SET_ZERO_DATA) to server. After next read, it will >> re-cache it. >> >> Reported-by: Hulk Robot <hulkci@huawei.com> >> Signed-off-by: Zhang Xiaoxu <zhangxiaoxu5@huawei.com> >> --- >> fs/cifs/smb2ops.c | 5 +++++ >> 1 file changed, 5 insertions(+) >> >> diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c >> index 736d86b8a910..250b51aca514 100644 >> --- a/fs/cifs/smb2ops.c >> +++ b/fs/cifs/smb2ops.c >> @@ -3187,6 +3187,11 @@ static long smb3_zero_range(struct file *file, struct cifs_tcon *tcon, >> trace_smb3_zero_enter(xid, cfile->fid.persistent_fid, tcon->tid, >> ses->Suid, offset, len); >> >> + /* >> + * We zero the range through ioctl, so we need remove the page caches >> + * first, otherwise the data may be inconsistent with the server. >> + */ >> + truncate_pagecache_range(inode, offset, offset + len - 1); >> >> /* if file not oplocked can't be sure whether asking to extend size */ >> if (!CIFS_CACHE_READ(cifsi)) >> -- >> 2.25.4 >> > > Looks good! > > Reviewed-by: Pavel Shilovsky <pshilov@microsoft.com> > Thanks. > Don't we need to do the same for smb3_punch_hole()? The problem also exists when punch hole. # dmesg > dmesg # strace -e trace=pread64,fallocate xfs_io -f -c "pread 20 40" \ -c "fpunch 20 40" \ -c"pread 20 40" dmesg pread64(3, " VFS: \\\\192.168.26.62 Cancelling"..., 40, 20) = 40 fallocate(3, FALLOC_FL_KEEP_SIZE|FALLOC_FL_PUNCH_HOLE, 20, 40) = 0 pread64(3, " VFS: \\\\192.168.26.62 Cancelling"..., 40, 20) = 40 When punch hole success, we also can read old data from file. I will send a new patch to fix it. > > -- > Best regards, > Pavel Shilovsky >
diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c index 736d86b8a910..250b51aca514 100644 --- a/fs/cifs/smb2ops.c +++ b/fs/cifs/smb2ops.c @@ -3187,6 +3187,11 @@ static long smb3_zero_range(struct file *file, struct cifs_tcon *tcon, trace_smb3_zero_enter(xid, cfile->fid.persistent_fid, tcon->tid, ses->Suid, offset, len); + /* + * We zero the range through ioctl, so we need remove the page caches + * first, otherwise the data may be inconsistent with the server. + */ + truncate_pagecache_range(inode, offset, offset + len - 1); /* if file not oplocked can't be sure whether asking to extend size */ if (!CIFS_CACHE_READ(cifsi))
CIFS implements the fallocate(FALLOC_FL_ZERO_RANGE) with send SMB ioctl(FSCTL_SET_ZERO_DATA) to server. It just set the range of the remote file to zero, but local page cache not update, then the data inconsistent with server, which leads the xfstest generic/008 failed. So we need to remove the local page caches before send SMB ioctl(FSCTL_SET_ZERO_DATA) to server. After next read, it will re-cache it. Reported-by: Hulk Robot <hulkci@huawei.com> Signed-off-by: Zhang Xiaoxu <zhangxiaoxu5@huawei.com> --- fs/cifs/smb2ops.c | 5 +++++ 1 file changed, 5 insertions(+)