* [PATCH] cifs: truncate the inode and mapping when we simulate fcollapse
@ 2022-02-22 5:35 Ronnie Sahlberg
2022-02-22 12:08 ` kernel test robot
0 siblings, 1 reply; 4+ messages in thread
From: Ronnie Sahlberg @ 2022-02-22 5:35 UTC (permalink / raw)
To: linux-cifs; +Cc: Steve French
RHBZ:1997367
When we collapse a range in smb3_collapse_range() we must make sure
we update the inode size and pagecache accordingly.
If not, both inode size and pagecahce may be stale until it is refreshed.
This can be demonstrated for the inode size by running :
xfs_io -i -f -c "truncate 320k" -c "fcollapse 64k 128k" -c "fiemap -v" \
/mnt/testfile
where we can see the result of stale data in the fiemap output.
The third line of the output is wrong, all this data should be truncated.
EXT: FILE-OFFSET BLOCK-RANGE TOTAL FLAGS
0: [0..127]: hole 128
1: [128..383]: 128..383 256 0x1
2: [384..639]: hole 256
And the correct output, when the inode size has been updated correctly should
look like this:
EXT: FILE-OFFSET BLOCK-RANGE TOTAL FLAGS
0: [0..127]: hole 128
1: [128..383]: 128..383 256 0x1
Reported-by: Xiaoli Feng <xifeng@redhat.com>
Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com>
---
fs/cifs/smb2ops.c | 19 +++++++++++++++----
1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
index af5d0830bc8a..5cd3247e71b9 100644
--- a/fs/cifs/smb2ops.c
+++ b/fs/cifs/smb2ops.c
@@ -25,6 +25,7 @@
#include "smb2glob.h"
#include "cifs_ioctl.h"
#include "smbdirect.h"
+#include "fscache.h"
#include "fs_context.h"
/* Change credits for different ops and return the total number of credits */
@@ -3887,29 +3888,39 @@ static long smb3_collapse_range(struct file *file, struct cifs_tcon *tcon,
{
int rc;
unsigned int xid;
+ struct inode *inode;
struct cifsFileInfo *cfile = file->private_data;
+ struct cifsInodeInfo *cifsi;
__le64 eof;
xid = get_xid();
- if (off >= i_size_read(file->f_inode) ||
- off + len >= i_size_read(file->f_inode)) {
+ inode = d_inode(cfile->dentry);
+ cifsi = CIFS_I(inode);
+
+ if (off >= i_size_read(inode) ||
+ off + len >= i_size_read(inode)) {
rc = -EINVAL;
goto out;
}
rc = smb2_copychunk_range(xid, cfile, cfile, off + len,
- i_size_read(file->f_inode) - off - len, off);
+ i_size_read(inode) - off - len, off);
if (rc < 0)
goto out;
- eof = cpu_to_le64(i_size_read(file->f_inode) - len);
+ eof = cpu_to_le64(i_size_read(inode) - len);
rc = SMB2_set_eof(xid, tcon, cfile->fid.persistent_fid,
cfile->fid.volatile_fid, cfile->pid, &eof);
if (rc < 0)
goto out;
rc = 0;
+
+ eof = i_size_read(inode) - len;
+ cifsi->server_eof = eof;
+ truncate_setsize(inode, eof);
+ fscache_resize_cookie(cifs_inode_cookie(inode), eof);
out:
free_xid(xid);
return rc;
--
2.30.2
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] cifs: truncate the inode and mapping when we simulate fcollapse
2022-02-22 5:35 [PATCH] cifs: truncate the inode and mapping when we simulate fcollapse Ronnie Sahlberg
@ 2022-02-22 12:08 ` kernel test robot
0 siblings, 0 replies; 4+ messages in thread
From: kernel test robot @ 2022-02-22 12:08 UTC (permalink / raw)
To: Ronnie Sahlberg, linux-cifs; +Cc: kbuild-all, Steve French
Hi Ronnie,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on cifs/for-next]
[also build test WARNING on v5.17-rc5 next-20220217]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Ronnie-Sahlberg/cifs-truncate-the-inode-and-mapping-when-we-simulate-fcollapse/20220222-133724
base: git://git.samba.org/sfrench/cifs-2.6.git for-next
config: i386-randconfig-s001-20220221 (https://download.01.org/0day-ci/archive/20220222/202202221941.P3ofWs7j-lkp@intel.com/config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce:
# apt-get install sparse
# sparse version: v0.6.4-dirty
# https://github.com/0day-ci/linux/commit/a29e6f21f30f32afbc6cf397a34ca2904038d759
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Ronnie-Sahlberg/cifs-truncate-the-inode-and-mapping-when-we-simulate-fcollapse/20220222-133724
git checkout a29e6f21f30f32afbc6cf397a34ca2904038d759
# save the config file to linux build tree
mkdir build_dir
make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=i386 SHELL=/bin/bash
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
sparse warnings: (new ones prefixed by >>)
>> fs/cifs/smb2ops.c:3920:13: sparse: sparse: incorrect type in assignment (different base types) @@ expected restricted __le64 [addressable] [assigned] [usertype] eof @@ got long long @@
fs/cifs/smb2ops.c:3920:13: sparse: expected restricted __le64 [addressable] [assigned] [usertype] eof
fs/cifs/smb2ops.c:3920:13: sparse: got long long
>> fs/cifs/smb2ops.c:3921:27: sparse: sparse: incorrect type in assignment (different base types) @@ expected unsigned long long [usertype] server_eof @@ got restricted __le64 [addressable] [assigned] [usertype] eof @@
fs/cifs/smb2ops.c:3921:27: sparse: expected unsigned long long [usertype] server_eof
fs/cifs/smb2ops.c:3921:27: sparse: got restricted __le64 [addressable] [assigned] [usertype] eof
>> fs/cifs/smb2ops.c:3922:33: sparse: sparse: incorrect type in argument 2 (different base types) @@ expected long long [usertype] newsize @@ got restricted __le64 [addressable] [assigned] [usertype] eof @@
fs/cifs/smb2ops.c:3922:33: sparse: expected long long [usertype] newsize
fs/cifs/smb2ops.c:3922:33: sparse: got restricted __le64 [addressable] [assigned] [usertype] eof
>> fs/cifs/smb2ops.c:3923:57: sparse: sparse: incorrect type in argument 2 (different base types) @@ expected long long [usertype] new_size @@ got restricted __le64 [addressable] [assigned] [usertype] eof @@
fs/cifs/smb2ops.c:3923:57: sparse: expected long long [usertype] new_size
fs/cifs/smb2ops.c:3923:57: sparse: got restricted __le64 [addressable] [assigned] [usertype] eof
vim +3920 fs/cifs/smb2ops.c
3885
3886 static long smb3_collapse_range(struct file *file, struct cifs_tcon *tcon,
3887 loff_t off, loff_t len)
3888 {
3889 int rc;
3890 unsigned int xid;
3891 struct inode *inode;
3892 struct cifsFileInfo *cfile = file->private_data;
3893 struct cifsInodeInfo *cifsi;
3894 __le64 eof;
3895
3896 xid = get_xid();
3897
3898 inode = d_inode(cfile->dentry);
3899 cifsi = CIFS_I(inode);
3900
3901 if (off >= i_size_read(inode) ||
3902 off + len >= i_size_read(inode)) {
3903 rc = -EINVAL;
3904 goto out;
3905 }
3906
3907 rc = smb2_copychunk_range(xid, cfile, cfile, off + len,
3908 i_size_read(inode) - off - len, off);
3909 if (rc < 0)
3910 goto out;
3911
3912 eof = cpu_to_le64(i_size_read(inode) - len);
3913 rc = SMB2_set_eof(xid, tcon, cfile->fid.persistent_fid,
3914 cfile->fid.volatile_fid, cfile->pid, &eof);
3915 if (rc < 0)
3916 goto out;
3917
3918 rc = 0;
3919
> 3920 eof = i_size_read(inode) - len;
> 3921 cifsi->server_eof = eof;
> 3922 truncate_setsize(inode, eof);
> 3923 fscache_resize_cookie(cifs_inode_cookie(inode), eof);
3924 out:
3925 free_xid(xid);
3926 return rc;
3927 }
3928
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH] cifs: truncate the inode and mapping when we simulate fcollapse
@ 2022-02-23 1:14 Ronnie Sahlberg
2022-02-23 2:44 ` Steve French
0 siblings, 1 reply; 4+ messages in thread
From: Ronnie Sahlberg @ 2022-02-23 1:14 UTC (permalink / raw)
To: linux-cifs; +Cc: Steve French
RHBZ:1997367
When we collapse a range in smb3_collapse_range() we must make sure
we update the inode size and pagecache accordingly.
If not, both inode size and pagecahce may be stale until it is refreshed.
This can be demonstrated for the inode size by running :
xfs_io -i -f -c "truncate 320k" -c "fcollapse 64k 128k" -c "fiemap -v" \
/mnt/testfile
where we can see the result of stale data in the fiemap output.
The third line of the output is wrong, all this data should be truncated.
EXT: FILE-OFFSET BLOCK-RANGE TOTAL FLAGS
0: [0..127]: hole 128
1: [128..383]: 128..383 256 0x1
2: [384..639]: hole 256
And the correct output, when the inode size has been updated correctly should
look like this:
EXT: FILE-OFFSET BLOCK-RANGE TOTAL FLAGS
0: [0..127]: hole 128
1: [128..383]: 128..383 256 0x1
Reported-by: Xiaoli Feng <xifeng@redhat.com>
Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com>
---
fs/cifs/smb2ops.c | 18 ++++++++++++++----
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
index af5d0830bc8a..891b11576e55 100644
--- a/fs/cifs/smb2ops.c
+++ b/fs/cifs/smb2ops.c
@@ -25,6 +25,7 @@
#include "smb2glob.h"
#include "cifs_ioctl.h"
#include "smbdirect.h"
+#include "fscache.h"
#include "fs_context.h"
/* Change credits for different ops and return the total number of credits */
@@ -3887,29 +3888,38 @@ static long smb3_collapse_range(struct file *file, struct cifs_tcon *tcon,
{
int rc;
unsigned int xid;
+ struct inode *inode;
struct cifsFileInfo *cfile = file->private_data;
+ struct cifsInodeInfo *cifsi;
__le64 eof;
xid = get_xid();
- if (off >= i_size_read(file->f_inode) ||
- off + len >= i_size_read(file->f_inode)) {
+ inode = d_inode(cfile->dentry);
+ cifsi = CIFS_I(inode);
+
+ if (off >= i_size_read(inode) ||
+ off + len >= i_size_read(inode)) {
rc = -EINVAL;
goto out;
}
rc = smb2_copychunk_range(xid, cfile, cfile, off + len,
- i_size_read(file->f_inode) - off - len, off);
+ i_size_read(inode) - off - len, off);
if (rc < 0)
goto out;
- eof = cpu_to_le64(i_size_read(file->f_inode) - len);
+ eof = cpu_to_le64(i_size_read(inode) - len);
rc = SMB2_set_eof(xid, tcon, cfile->fid.persistent_fid,
cfile->fid.volatile_fid, cfile->pid, &eof);
if (rc < 0)
goto out;
rc = 0;
+
+ cifsi->server_eof = i_size_read(inode) - len;
+ truncate_setsize(inode, cifsi->server_eof);
+ fscache_resize_cookie(cifs_inode_cookie(inode), cifsi->server_eof);
out:
free_xid(xid);
return rc;
--
2.30.2
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] cifs: truncate the inode and mapping when we simulate fcollapse
2022-02-23 1:14 Ronnie Sahlberg
@ 2022-02-23 2:44 ` Steve French
0 siblings, 0 replies; 4+ messages in thread
From: Steve French @ 2022-02-23 2:44 UTC (permalink / raw)
To: Ronnie Sahlberg; +Cc: linux-cifs
Tentatively merged into cifs-2.6.git for-next pending review and testing
On Tue, Feb 22, 2022 at 7:14 PM Ronnie Sahlberg <lsahlber@redhat.com> wrote:
>
> RHBZ:1997367
>
> When we collapse a range in smb3_collapse_range() we must make sure
> we update the inode size and pagecache accordingly.
>
> If not, both inode size and pagecahce may be stale until it is refreshed.
>
> This can be demonstrated for the inode size by running :
>
> xfs_io -i -f -c "truncate 320k" -c "fcollapse 64k 128k" -c "fiemap -v" \
> /mnt/testfile
>
> where we can see the result of stale data in the fiemap output.
> The third line of the output is wrong, all this data should be truncated.
>
> EXT: FILE-OFFSET BLOCK-RANGE TOTAL FLAGS
> 0: [0..127]: hole 128
> 1: [128..383]: 128..383 256 0x1
> 2: [384..639]: hole 256
>
> And the correct output, when the inode size has been updated correctly should
> look like this:
>
> EXT: FILE-OFFSET BLOCK-RANGE TOTAL FLAGS
> 0: [0..127]: hole 128
> 1: [128..383]: 128..383 256 0x1
>
> Reported-by: Xiaoli Feng <xifeng@redhat.com>
> Reported-by: kernel test robot <lkp@intel.com>
> Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com>
> ---
> fs/cifs/smb2ops.c | 18 ++++++++++++++----
> 1 file changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
> index af5d0830bc8a..891b11576e55 100644
> --- a/fs/cifs/smb2ops.c
> +++ b/fs/cifs/smb2ops.c
> @@ -25,6 +25,7 @@
> #include "smb2glob.h"
> #include "cifs_ioctl.h"
> #include "smbdirect.h"
> +#include "fscache.h"
> #include "fs_context.h"
>
> /* Change credits for different ops and return the total number of credits */
> @@ -3887,29 +3888,38 @@ static long smb3_collapse_range(struct file *file, struct cifs_tcon *tcon,
> {
> int rc;
> unsigned int xid;
> + struct inode *inode;
> struct cifsFileInfo *cfile = file->private_data;
> + struct cifsInodeInfo *cifsi;
> __le64 eof;
>
> xid = get_xid();
>
> - if (off >= i_size_read(file->f_inode) ||
> - off + len >= i_size_read(file->f_inode)) {
> + inode = d_inode(cfile->dentry);
> + cifsi = CIFS_I(inode);
> +
> + if (off >= i_size_read(inode) ||
> + off + len >= i_size_read(inode)) {
> rc = -EINVAL;
> goto out;
> }
>
> rc = smb2_copychunk_range(xid, cfile, cfile, off + len,
> - i_size_read(file->f_inode) - off - len, off);
> + i_size_read(inode) - off - len, off);
> if (rc < 0)
> goto out;
>
> - eof = cpu_to_le64(i_size_read(file->f_inode) - len);
> + eof = cpu_to_le64(i_size_read(inode) - len);
> rc = SMB2_set_eof(xid, tcon, cfile->fid.persistent_fid,
> cfile->fid.volatile_fid, cfile->pid, &eof);
> if (rc < 0)
> goto out;
>
> rc = 0;
> +
> + cifsi->server_eof = i_size_read(inode) - len;
> + truncate_setsize(inode, cifsi->server_eof);
> + fscache_resize_cookie(cifs_inode_cookie(inode), cifsi->server_eof);
> out:
> free_xid(xid);
> return rc;
> --
> 2.30.2
>
--
Thanks,
Steve
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-02-23 2:46 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-02-22 5:35 [PATCH] cifs: truncate the inode and mapping when we simulate fcollapse Ronnie Sahlberg
2022-02-22 12:08 ` kernel test robot
-- strict thread matches above, loose matches on Subject: below --
2022-02-23 1:14 Ronnie Sahlberg
2022-02-23 2:44 ` Steve French
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox