Introduce SEEK_DATA/SEEK_HOLE support V8
PATCH 1/1 Introduce SEEK_DATA/SEEK_HOLE support V8
Hello,
This is the revised patch according to Dave's comments for V7.
Changes to V8:
--------------
1. If there is an internal error raised at extent reading routine, just return it rather than ENXIO.
2. Add the commit message.
3. Remove the for(;;) loop since there is no continuous holes shown even if create a Petabyte sparse file with hole extent length longer than 32-bit. Thanks Dave for helping verify that!
4. In xfs_seek_data(), s/len/end/, looks 'end' is more meaningful here to indicate the range of extents mapped.
5. Remove BUG() from xfs_seek_data() since xfs_bmapi_read() have found any corruption during the lookup, it should not occurred at all.
Any comments are appreciated!
Thanks,
-Jeff
---
This patch adds lseek(2) SEEK_DATA/SEEK_HOLE functionality.
fs/xfs/xfs_file.c | 135 ++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 files changed, 134 insertions(+), 1 deletions(-)
fs/xfs/xfs_file.c +134 -1
@@ -1141,8 +1141,141 @@ xfs_vm_page_mkwrite(return block_page_mkwrite (vma, vmf, xfs_get_blocks); } +STATIC loff_t +xfs_seek_data ( +struct file *file, +loff_t start, +u32 type) +{ +struct inode *inode = file->f_mapping->host; +struct xfs_inode *ip =XFS_I (inode); +struct xfs_mount *mp = ip->i_mount; +struct xfs_bmbt_irec map[2 ]; +int nmap =2 ; +loff_t uninitialized_var (offset ); +xfs_fsize_t isize; +xfs_fileoff_t fsbno; +xfs_filblks_t end; +uint lock; +int error; + + lock =xfs_ilock_map_shared (ip); + + isize =i_size_read (inode); +if (start >= isize) { + error =ENXIO ; + goto out_unlock; + } + + fsbno =XFS_B_TO_FSBT (mp, start); + +/* +* Try to read extents from the first block indicated +* by fsbno to the end block of the file. +*/ + end =XFS_B_TO_FSB (mp, isize); + + error =xfs_bmapi_read (ip, fsbno, end - fsbno, map, &nmap, +XFS_BMAPI_ENTIRE ); +if (error) + goto out_unlock; + +/* +* Treat unwritten extent as data extent since it might +* contains dirty data in page cache. +*/ +if (map[0 ].br_startblock !=HOLESTARTBLOCK ) { + offset =max_t (loff_t, start, +XFS_FSB_TO_B (mp, map[0 ].br_startoff)); + }else { +if (nmap ==1 ) { + error =ENXIO ; + goto out_unlock; + } + + offset =max_t (loff_t, start, +XFS_FSB_TO_B (mp, map[1 ].br_startoff)); + } + +if (offset != file->f_pos) + file->f_pos = offset; + +out_unlock: +xfs_iunlock_map_shared (ip, lock); + +if (error) +return -error; +return offset; +} + +STATIC loff_t +xfs_seek_hole ( +struct file *file, +loff_t start, +u32 type) +{ +struct inode *inode = file->f_mapping->host; +struct xfs_inode *ip =XFS_I (inode); +struct xfs_mount *mp = ip->i_mount; +loff_t uninitialized_var (offset ); +loff_t holeoff; +xfs_fsize_t isize; +xfs_fileoff_t fsbno; +uint lock; +int error; + + lock =xfs_ilock_map_shared (ip); + + isize =i_size_read (inode); +if (start >= isize) { + error =ENXIO ; + goto out_unlock; + } + + fsbno =XFS_B_TO_FSBT (mp, start); + error =xfs_bmap_first_unused (NULL , ip,1 , &fsbno,XFS_DATA_FORK ); +if (error) + goto out_unlock; + + holeoff =XFS_FSB_TO_B (mp, fsbno); +if (holeoff <= start) + offset = start; +else + offset =min_t (loff_t, holeoff, isize); + +if (offset != file->f_pos) + file->f_pos = offset; + +out_unlock: +xfs_iunlock_map_shared (ip, lock); + +if (error) +return -error; +return offset; +} + +STATIC loff_t +xfs_file_llseek ( +struct file *file, +loff_t offset, +int origin) +{ +switch (origin) { +case SEEK_END : +case SEEK_CUR : +case SEEK_SET : +return generic_file_llseek (file, offset, origin); +case SEEK_DATA : +return xfs_seek_data (file, offset, origin); +case SEEK_HOLE : +return xfs_seek_hole (file, offset, origin); +default : +return -EINVAL ; + } +} +const struct file_operations xfs_file_operations = { - .llseek = generic_file_llseek, + .llseek = xfs_file_llseek, .read = do_sync_read, .write = do_sync_write, .aio_read = xfs_file_aio_read,
Signature · 6 lines
1.7.9 _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs
Quoted 24 lines · Jeff Liu
Hello,
This is the revised patch according to Dave's comments for V7.
Changes to V8:
--------------
1. If there is an internal error raised at extent reading routine, just return it rather than ENXIO.
2. Add the commit message.
3. Remove the for(;;) loop since there is no continuous holes shown even if create a Petabyte sparse file with hole extent length longer than 32-bit. Thanks Dave for helping verify that!
4. Inxfs_seek_data(), s/len/end/, looks 'end' is more meaningful here to indicate the range of extents mapped.
5. RemoveBUG()fromxfs_seek_data()sincexfs_bmapi_read()have found any corruption during the lookup, it should not occurred at all.Any comments are appreciated!
Thanks,
-JeffSigned-off-by Jie Liu<[email protected]>
...
Quoted 2 lines
fs/xfs/xfs_file.c +2
-0+STATIC loff_t +xfs_seek_hole(
...
fs/xfs/xfs_file.c xfs_vm_page_mkwrite( · line 1245
Quoted 12 lines
fs/xfs/xfs_file.c +12
-0+ + fsbno =XFS_B_TO_FSBT (mp, start); + error =xfs_bmap_first_unused (NULL , ip,1 ,&fsbno,XFS_DATA_FORK ); +if (error) + goto out_unlock; + + holeoff =XFS_FSB_TO_B (mp, fsbno); +if (holeoff<= start) + offset = start; +else + offset =min_t (loff_t, holeoff, isize); +
...
Very Nice. Much more concise.
Can xfs_bmap_first_unused() return something larger than the end of file?
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
Quoted 46 lines · Mark Tinguely
Quoted 24 lines · Jeff Liu
Hello,
This is the revised patch according to Dave's comments for V7.
Changes to V8:
--------------
1. If there is an internal error raised at extent reading routine, just return it rather than ENXIO.
2. Add the commit message.
3. Remove the for(;;) loop since there is no continuous holes shown even if create a Petabyte sparse file with hole extent length longer than 32-bit. Thanks Dave for helping verify that!
4. Inxfs_seek_data(), s/len/end/, looks 'end' is more meaningful here to indicate the range of extents mapped.
5. RemoveBUG()fromxfs_seek_data()sincexfs_bmapi_read()have found any corruption during the lookup, it should not occurred at all.Any comments are appreciated!
Thanks,
-JeffSigned-off-by Jie Liu<[email protected]>...
Quoted 2 lines
fs/xfs/xfs_file.c +2
-0+STATIC loff_t +xfs_seek_hole(...
Quoted 12 lines
fs/xfs/xfs_file.c +12
-0+ + fsbno =XFS_B_TO_FSBT (mp, start); + error =xfs_bmap_first_unused (NULL , ip,1 ,&fsbno,XFS_DATA_FORK ); +if (error) + goto out_unlock; + + holeoff =XFS_FSB_TO_B (mp, fsbno); +if (holeoff<= start) + offset = start; +else + offset =min_t (loff_t, holeoff, isize); +...
Quoted 3 lines
Very Nice. Much more concise.
Can
xfs_bmap_first_unused()return something larger than the end of file?
I think it could be happen if the file has no holes past the given offset. In this case, it will return the first block past the end of file. That is why "min_t()" is used to determine the final value.
Thanks,
-Jeff
Quoted 2 lines
Reviewed-by Mark Tinguely <[email protected]>
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
Quoted 50 lines · Jeff Liu
Quoted 48 lines · Mark Tinguely
Quoted 24 lines · Jeff Liu
Hello,
This is the revised patch according to Dave's comments for V7.
Changes to V8:
--------------
1. If there is an internal error raised at extent reading routine, just return it rather than ENXIO.
2. Add the commit message.
3. Remove the for(;;) loop since there is no continuous holes shown even if create a Petabyte sparse file with hole extent length longer than 32-bit. Thanks Dave for helping verify that!
4. Inxfs_seek_data(), s/len/end/, looks 'end' is more meaningful here to indicate the range of extents mapped.
5. RemoveBUG()fromxfs_seek_data()sincexfs_bmapi_read()have found any corruption during the lookup, it should not occurred at all.Any comments are appreciated!
Thanks,
-JeffSigned-off-by Jie Liu<[email protected]>...
Quoted 2 lines
fs/xfs/xfs_file.c +2
-0+STATIC loff_t +xfs_seek_hole(...
fs/xfs/xfs_file.c xfs_vm_page_mkwrite( · line 1245
Quoted 12 lines
fs/xfs/xfs_file.c +12
-0+ + fsbno =XFS_B_TO_FSBT (mp, start); + error =xfs_bmap_first_unused (NULL , ip,1 ,&fsbno,XFS_DATA_FORK ); +if (error) + goto out_unlock; + + holeoff =XFS_FSB_TO_B (mp, fsbno); +if (holeoff<= start) + offset = start; +else + offset =min_t (loff_t, holeoff, isize); +...
Very Nice. Much more concise.
Quoted 13 lines
Quoted 1 line
Can
xfs_bmap_first_unused()return something larger than the end of file?I think it could be happen if the file has no holes past the given offset. In this case, it will return the first block past the end of file. That is why "
min_t()" is used to determine the final value.Thanks,
-JeffQuoted 2 lines
Reviewed-by Mark Tinguely<[email protected]>
Okay. Looks good.
--Mark Tinguely.
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
Quoted 62 lines · Jeff Liu
Hello,
This is the revised patch according to Dave's comments for V7.
Changes to V8:
--------------
1. If there is an internal error raised at extent reading routine, just return it rather than ENXIO.
2. Add the commit message.
3. Remove the for(;;) loop since there is no continuous holes shown even if create a Petabyte sparse file with hole extent length longer than 32-bit. Thanks Dave for helping verify that!
4. Inxfs_seek_data(), s/len/end/, looks 'end' is more meaningful here to indicate the range of extents mapped.
5. RemoveBUG()fromxfs_seek_data()sincexfs_bmapi_read()have found any corruption during the lookup, it should not occurred at all.Any comments are appreciated!
Thanks,
-JeffSigned-off-by Jie Liu<[email protected]>---
This patch adds lseek(2) SEEK_DATA/SEEK_HOLE functionality.fs/xfs/xfs_file.c +35
-0+STATIC loff_t +xfs_seek_hole ( +struct file *file, +loff_t start, +u32 type) +{ +struct inode *inode = file->f_mapping->host; +struct xfs_inode *ip =XFS_I (inode); +struct xfs_mount *mp = ip->i_mount; +loff_t uninitialized_var (offset ); +loff_t holeoff; +xfs_fsize_t isize; +xfs_fileoff_t fsbno; +uint lock; +int error; + + lock =xfs_ilock_map_shared (ip); + + isize =i_size_read (inode); +if (start>= isize) { + error =ENXIO ; + goto out_unlock; + } + + fsbno =XFS_B_TO_FSBT (mp, start); + error =xfs_bmap_first_unused (NULL , ip,1 ,&fsbno,XFS_DATA_FORK ); +if (error) + goto out_unlock; + + holeoff =XFS_FSB_TO_B (mp, fsbno); +if (holeoff<= start) + offset = start; +else + offset =min_t (loff_t, holeoff, isize); +
fs/xfs/xfs_file.c xfs_vm_page_mkwrite( · line 1255
Quoted 10 lines
fs/xfs/xfs_file.c +10
-0+if (offset != file->f_pos) + file->f_pos = offset; + +out_unlock: +xfs_iunlock_map_shared (ip, lock); + +if (error) +return -error; +return offset; +}
Sorry this got set aside. This is being dusted off for inclusion.
Refresh my memory, do we need a XFS_FORCED_SHUTDOWN test in xfs_seek_hole()? In the earlier versions it was covered in the xfs_bmapi_read().
A comment on the min_t() that the xfs_bmap_first_unused() could return a value bigger than the isize if there are no more holes may help the forgetful such as myself.
Otherwise it looks good.
Thank-you.
Mark Tinguely <[email protected]>
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
Quoted 67 lines · Mark Tinguely
Quoted 66 lines · Jeff Liu
Hello,
This is the revised patch according to Dave's comments for V7.
Changes to V8:
--------------
1. If there is an internal error raised at extent reading routine, just return it rather than ENXIO.
2. Add the commit message.
3. Remove the for(;;) loop since there is no continuous holes shown even if create a Petabyte sparse file with hole extent length longer than 32-bit. Thanks Dave for helping verify that!
4. Inxfs_seek_data(), s/len/end/, looks 'end' is more meaningful here to indicate the range of extents mapped.
5. RemoveBUG()fromxfs_seek_data()sincexfs_bmapi_read()have found any corruption during the lookup, it should not occurred at all.Any comments are appreciated!
Thanks,
-JeffSigned-off-by Jie Liu<[email protected]>---
This patch adds lseek(2) SEEK_DATA/SEEK_HOLE functionality.fs/xfs/xfs_file.c +39
-0+STATIC loff_t +xfs_seek_hole ( +struct file *file, +loff_t start, +u32 type) +{ +struct inode *inode = file->f_mapping->host; +struct xfs_inode *ip =XFS_I (inode); +struct xfs_mount *mp = ip->i_mount; +loff_t uninitialized_var (offset ); +loff_t holeoff; +xfs_fsize_t isize; +xfs_fileoff_t fsbno; +uint lock; +int error; + + lock =xfs_ilock_map_shared (ip); + + isize =i_size_read (inode); +if (start>= isize) { + error =ENXIO ; + goto out_unlock; + } + + fsbno =XFS_B_TO_FSBT (mp, start); + error =xfs_bmap_first_unused (NULL , ip,1 ,&fsbno,XFS_DATA_FORK ); +if (error) + goto out_unlock; + + holeoff =XFS_FSB_TO_B (mp, fsbno); +if (holeoff<= start) + offset = start; +else + offset =min_t (loff_t, holeoff, isize); + +if (offset != file->f_pos) + file->f_pos = offset; + +out_unlock:
Quoted 8 lines
fs/xfs/xfs_file.c xfs_vm_page_mkwrite( · line 1209
Quoted 6 lines
fs/xfs/xfs_file.c +6
-0+xfs_iunlock_map_shared (ip, lock); + +if (error) +return -error; +return offset; +}Sorry this got set aside. This is being dusted off for inclusion.
Thanks for picking this up!
Quoted 4 lines
Refresh my memory, do we need a
XFS_FORCED_SHUTDOWNtest inxfs_seek_hole()? In the earlier versions it was covered in thexfs_bmapi_read().
Yes, definitely.
Quoted 4 lines
A comment on the
min_t()that thexfs_bmap_first_unused()could return a value bigger than the isize if there are no more holes may help the forgetful such as myself.
I'll leave a comment for it, it would be helpful to recall the case for a long period.
Thanks,
-Jeff
Quoted 11 lines
Otherwise it looks good.
Thank-you.
Mark Tinguely <[email protected]>
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs