xfs: lseek SEEK_DATA/SEEK_HOLE consolidation
PATCH 1/1 xfs: lseek SEEK_DATA/SEEK_HOLE consolidation
From: Jie Liu <[email protected]>
Consolidate lseek(2) SEEK_DATA/SEEK_HOLE according to the implementation of VFS lseek_execute():
- if end up with a negative offset, return EINVAL if file is not huge.
- if end up with an offset larger than s_maxbytes, return EINVAL as well.
- reset file version to 0 if end up with an offset that is not equal to the current file offset.
--- fs/xfs/xfs_file.c | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-)
fs/xfs/xfs_file.c +24 -2
@@ -1270,8 +1270,19 @@ xfs_seek_data( } out: -if (offset != file->f_pos) +if (offset <0 && !(file->f_mode &FMODE_UNSIGNED_OFFSET )) { + error =EINVAL ; + goto out_unlock; + } +if (offset > inode->i_sb->s_maxbytes) { + error =EINVAL ; + goto out_unlock; + } + +if (offset != file->f_pos) { file->f_pos = offset; + file->f_version =0 ; + } out_unlock:xfs_iunlock_map_shared (ip, lock);
@@ -1372,6 +1383,15 @@ xfs_seek_hole( } out: +if (offset <0 && !(file->f_mode &FMODE_UNSIGNED_OFFSET )) { + error =EINVAL ; + goto out_unlock; + } +if (offset > inode->i_sb->s_maxbytes) { + error =EINVAL ; + goto out_unlock; + } +/* * At this point, we must have found a hole. However, the returned * offset may be bigger than the file size as it may be aligned to
@@ -1379,8 +1399,10 @@ out:* situation in particular. */ offset =min_t (loff_t, offset, isize); -if (offset != file->f_pos) +if (offset != file->f_pos) { file->f_pos = offset; + file->f_version =0 ; + } out_unlock:xfs_iunlock_map_shared (ip, lock);
Signature · 6 lines
1.7.9.5 _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs
Quoted 29 lines · Jeff Liu
From: Jie Liu <[email protected]>
Consolidate lseek(2) SEEK_DATA/SEEK_HOLE according to the implementation of VFS
lseek_execute():
- if end up with a negative offset, return EINVAL if file is not huge.
- if end up with an offset larger thans_maxbytes, return EINVAL as well.
- reset file version to 0 if end up with an offset that is not equal to the current file offset.Signed-off-by Jie Liu <[email protected]>--- fs/xfs/xfs_file.c | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-)fs/xfs/xfs_file.c +3
-1@@ -1270,8 +1270,19 @@ xfs_seek_data( } out: -if (offset != file->f_pos) +if (offset <0 && !(file->f_mode &FMODE_UNSIGNED_OFFSET )) { + error =EINVAL ; + goto out_unlock;
fs/xfs/xfs_file.c xfs_seek_data( · line 1285
Quoted 10 lines
fs/xfs/xfs_file.c +9
-0+ } +if (offset > inode->i_sb->s_maxbytes) { + error =EINVAL ; + goto out_unlock; + } + +if (offset != file->f_pos) { file->f_pos = offset; + file->f_version =0 ; + }
Hi Jeff, why are you copy-n-pasting this code from lseek_execute() rather than making lseek_execute() an exported function and calling that directly?
Quoted 5 lines
fs/xfs/xfs_file.c +0
-0out_unlock:xfs_iunlock_map_shared (ip, lock);@@ -1372,6 +1383,15 @@ xfs_seek_hole( }
fs/xfs/xfs_file.c xfs_seek_hole( · line 1393
Quoted 10 lines
fs/xfs/xfs_file.c +8
-0out: +if (offset <0 && !(file->f_mode &FMODE_UNSIGNED_OFFSET )) { + error =EINVAL ; + goto out_unlock; + } +if (offset > inode->i_sb->s_maxbytes) { + error =EINVAL ; + goto out_unlock; + }
These checks belong after we truncated offset to isize, don't they? And that would make both of these functions simply require a call to lseek_execute(), yes?
Cheers,
Dave.
Signature · 7 lines
Dave Chinner [email protected] _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs
Hi Dave,
Quoted 36 lines · Dave Chinner
Quoted 35 lines · Jeff Liu
From: Jie Liu <[email protected]>
Consolidate lseek(2) SEEK_DATA/SEEK_HOLE according to the implementation of VFS
lseek_execute():
- if end up with a negative offset, return EINVAL if file is not huge.
- if end up with an offset larger thans_maxbytes, return EINVAL as well.
- reset file version to 0 if end up with an offset that is not equal to the current file offset.Signed-off-by Jie Liu <[email protected]>--- fs/xfs/xfs_file.c | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-)fs/xfs/xfs_file.c +9
-1@@ -1270,8 +1270,19 @@ xfs_seek_data( } out: -if (offset != file->f_pos) +if (offset <0 && !(file->f_mode &FMODE_UNSIGNED_OFFSET )) { + error =EINVAL ; + goto out_unlock; + } +if (offset > inode->i_sb->s_maxbytes) { + error =EINVAL ; + goto out_unlock; + } +
Quoted 8 lines
fs/xfs/xfs_file.c xfs_seek_data( · line 1285
Quoted 4 lines
fs/xfs/xfs_file.c +3
-0+if (offset != file->f_pos) { file->f_pos = offset; + file->f_version =0 ; + }Hi Jeff, why are you copy-n-pasting this code from
lseek_execute()rather than makinglseek_execute()an exported function and calling that directly?
I found other file systems are implemented in this way. But I should consider how to make it better rather than following others in this situation.
Quoted 7 lines
Quoted 7 lines
fs/xfs/xfs_file.c +0
-0out_unlock:xfs_iunlock_map_shared (ip, lock);@@ -1372,6 +1383,15 @@ xfs_seek_hole( }
Quoted 11 lines
fs/xfs/xfs_file.c xfs_seek_data( · line 1280
Quoted 9 lines
fs/xfs/xfs_file.c +8
-0out: +if (offset <0 && !(file->f_mode &FMODE_UNSIGNED_OFFSET )) { + error =EINVAL ; + goto out_unlock; + } +if (offset > inode->i_sb->s_maxbytes) { + error =EINVAL ; + goto out_unlock; + }These checks belong after we truncated offset to isize, don't they?
They do.
Quoted 2 lines
And that would make both of these functions simply require a call to
lseek_execute(), yes?
Yep, I'll post a patch set to export this call and propagate it to other file systems as well.
Thanks,
-Jeff
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs