Skip to contentSkip to conversation

Introduce SEEK_DATA/SEEK_HOLE support to XFS V6

PATCH In review Jeff Liu 2 participants 3 messages Jan 29, 2012 – Feb 2, 2012 XFS filesystem

lore
Share & tools
lore

Jeff Liu

author
#0 lore raw

PATCH 1/1 Introduce SEEK_DATA/SEEK_HOLE support to XFS V6

Hello,

Sorry for the delay!! I just got back from vacation.

This is the V6 to introduce SEEK_DATA/SEEK_HOLE support to XFS.
As we have discussed previously, I have removed the dirty data probing stuff and just treating unwritten extents as data in this post.

Changes to V6:
--------------
* remove xfs_has_unwritten_buffer() for now.
* xfs_bmapi_read() returns the br_state == XFS_EXT_NORM for a hole, so we need to check its startblock is not a "nullstartblock" in this case.
* call i_size_read() after taking the ilock shared, otherwise, isize could be stale.
* remove "ifp = XFS_IFORK_PTR(ip, XFS_DATA_FORK)" from xfs_seek_data() since it will not used.
* in xfs_file_llseek(), return -EINVAL rather than -EOPNOTSUPP if whence is not valid according to http://linux.die.net/man/2/lseek.
* s/int lock/uint lock/ in both xfs_seek_data() and xfs_seek_hole().
* s/out_lock/out_unlock/ in both functions too.

Tests:
------
* seek_sanity_tester:
http://permalink.gmane.org/gmane.comp.file-systems.xfs.general/42514

*seek_copy_tester:
http://permalink.gmane.org/gmane.comp.file-systems.xfs.general/42522

Thank you!
-Jeff

---
 fs/xfs/xfs_file.c |  168 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 167 insertions(+), 1 deletions(-)
fs/xfs/xfs_file.c +167 -1
index 753ed9b..41a045f 100644
@@ -1141,8 +1141,174 @@ 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;
+	loff_t			uninitialized_var(offset);
+	xfs_fsize_t		isize;
+	xfs_fileoff_t		fsbno;
+	xfs_filblks_t		len;
+	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);
+	len = XFS_B_TO_FSB(mp, isize);
+	for (;;) {
+		struct xfs_bmbt_irec	map[2];
+		int			nmap = 2;
+		loff_t			seekoff;
+
+		error = xfs_bmapi_read(ip, fsbno, len - fsbno, map, &nmap,
+				       XFS_BMAPI_ENTIRE);
+		if (error)
+			goto out_unlock;
+
+		/* No extents at given offset, must be beyond EOF */
+		if (nmap == 0) {
+			error = ENXIO;
+			goto out_unlock;
+		}
+
+		seekoff = XFS_FSB_TO_B(mp, fsbno);
+
+		if ((map[0].br_state == XFS_EXT_NORM &&
+		     !isnullstartblock(map[0].br_startblock)) ||
+		    map[0].br_startblock == DELAYSTARTBLOCK) {
+			offset = max_t(loff_t, seekoff,
+				       XFS_FSB_TO_B(mp, map[0].br_startoff));
+			break;
+		} else if (map[0].br_state == XFS_EXT_UNWRITTEN) {
+			offset = max_t(loff_t, seekoff,
+				       XFS_FSB_TO_B(mp, map[0].br_startoff));
+			break;
+		} else if (map[0].br_startblock == HOLESTARTBLOCK) {
+			if (nmap == 1) {
+				error = ENXIO;
+				goto out_unlock;
+			}
+
+			if ((map[1].br_state == XFS_EXT_NORM &&
+			     !isnullstartblock(map[1].br_startblock)) ||
+			    map[1].br_startblock == DELAYSTARTBLOCK) {
+				offset = max_t(loff_t, seekoff,
+					XFS_FSB_TO_B(mp, map[1].br_startoff));
+				break;
+			} else if (map[1].br_state == XFS_EXT_UNWRITTEN) {
+				offset = max_t(loff_t, seekoff,
+					XFS_FSB_TO_B(mp, map[1].br_startoff));
+				break;
+			} else if (map[1].br_startblock == HOLESTARTBLOCK) {
+				fsbno = map[1].br_startoff +
+					map[1].br_blockcount;
+			} else {
+				BUG();
+			}
+		} else {
+			BUG();
+		}
+
+		if (XFS_FSB_TO_B(mp, fsbno) > isize) {
+			error = ENXIO;
+			goto out_unlock;
+		}
+	}
+
+	if (offset < start)
+		offset = start;
+
+	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.4.1

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
#1 lore raw
Quoted 40 lines · Jeff Liu

Hello,

Sorry for the delay!! I just got back from vacation.

This is the V6 to introduce SEEK_DATA/SEEK_HOLE support to XFS.
As we have discussed previously, I have removed the dirty data probing stuff and just treating unwritten extents as data in this post.

Changes to V6:
--------------
* remove xfs_has_unwritten_buffer() for now.
* xfs_bmapi_read() returns the br_state == XFS_EXT_NORM for a hole, so we need to check its startblock is not a "nullstartblock" in this case.
* call i_size_read() after taking the ilock shared, otherwise, isize could be stale.
* remove "ifp = XFS_IFORK_PTR(ip, XFS_DATA_FORK)" from xfs_seek_data() since it will not used.
* in xfs_file_llseek(), return -EINVAL rather than -EOPNOTSUPP if whence is not valid according to http://linux.die.net/man/2/lseek.
* s/int lock/uint lock/ in both xfs_seek_data() and xfs_seek_hole().
* s/out_lock/out_unlock/ in both functions too.

Tests:
------
* seek_sanity_tester:
http://permalink.gmane.org/gmane.comp.file-systems.xfs.general/42514

*seek_copy_tester:
http://permalink.gmane.org/gmane.comp.file-systems.xfs.general/42522

Thank you!
-Jeff

---
  fs/xfs/xfs_file.c |  168 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
  1 files changed, 167 insertions(+), 1 deletions(-)
fs/xfs/xfs_file.c +0 -0
index 753ed9b..41a045f 100644

fs/xfs/xfs_file.c xfs_vm_page_mkwrite( · line 1145

Quoted 2 lines
fs/xfs/xfs_file.c +2 -0
+STATIC loff_t
+xfs_seek_data(

<deleted>

fs/xfs/xfs_file.c xfs_vm_page_mkwrite( · line 1245

Quoted 8 lines
fs/xfs/xfs_file.c +6 -0
+		error = xfs_bmapi_read(ip, fsbno, len - fsbno, map,&nmap,
+				       XFS_BMAPI_ENTIRE);
+		if (error)
+			goto out_unlock;
 
 
+STATIC loff_t
+xfs_seek_hole(

<deleted>

fs/xfs/xfs_file.c xfs_vm_page_mkwrite( · line 1178

Quoted 3 lines
fs/xfs/xfs_file.c +3 -0
+	error = xfs_bmap_first_unused(NULL, ip, 1,&fsbno, XFS_DATA_FORK);
+	if (error)
+		goto out_unlock;

The code looks good for the reduced problem. It test correctly. I am still finding holes only if they start on a 16KB boundary which we discussed before. I mention it in case more advanced test cases are generated.

Question:
If the routines that are looking for extents/hole return an error (I see EFSCORRUPTED, EAGAIN, ENOMEM, EIO as possible errors in these routines), should you convert them to an error such as EIO? There is no specific error mention in the lseek manual page.

--Mark Tinguely.

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

author
#2 lore raw
Quoted 69 lines · Mark Tinguely
Quoted 46 lines · Jeff Liu

Hello,

Sorry for the delay!! I just got back from vacation.

This is the V6 to introduce SEEK_DATA/SEEK_HOLE support to XFS. As we have discussed previously, I have removed the dirty data probing stuff and just treating unwritten extents as data in this post.

Changes to V6:
--------------
* remove xfs_has_unwritten_buffer() for now.
* xfs_bmapi_read() returns the br_state == XFS_EXT_NORM for a hole, so we need to check its startblock is not a "nullstartblock" in this case.
* call i_size_read() after taking the ilock shared, otherwise, isize could be stale.
* remove "ifp = XFS_IFORK_PTR(ip, XFS_DATA_FORK)" from xfs_seek_data() since it will not used.
* in xfs_file_llseek(), return -EINVAL rather than -EOPNOTSUPP if whence is not valid according to http://linux.die.net/man/2/lseek.
* s/int lock/uint lock/ in both xfs_seek_data() and xfs_seek_hole().
* s/out_lock/out_unlock/ in both functions too.

Tests:
------
* seek_sanity_tester:
http://permalink.gmane.org/gmane.comp.file-systems.xfs.general/42514

*seek_copy_tester:
http://permalink.gmane.org/gmane.comp.file-systems.xfs.general/42522

Thank you!
-Jeff

---
  fs/xfs/xfs_file.c |  168
++++++++++++++++++++++++++++++++++++++++++++++++++++-
  1 files changed, 167 insertions(+), 1 deletions(-)
fs/xfs/xfs_file.c +0 -0
index 753ed9b..41a045f 100644

fs/xfs/xfs_file.c xfs_vm_page_mkwrite( · line 1145

Quoted 2 lines
fs/xfs/xfs_file.c +2 -0
+STATIC loff_t
+xfs_seek_data(

<deleted>

fs/xfs/xfs_file.c xfs_vm_page_mkwrite( · line 1245

Quoted 8 lines
fs/xfs/xfs_file.c +6 -0
+        error = xfs_bmapi_read(ip, fsbno, len - fsbno, map,&nmap,
+                       XFS_BMAPI_ENTIRE);
+        if (error)
+            goto out_unlock;
 
 
+STATIC loff_t
+xfs_seek_hole(

<deleted>

fs/xfs/xfs_file.c xfs_vm_page_mkwrite( · line 1178

Quoted 3 lines
fs/xfs/xfs_file.c +3 -0
+    error = xfs_bmap_first_unused(NULL, ip, 1,&fsbno, XFS_DATA_FORK);
+    if (error)
+        goto out_unlock;

The code looks good for the reduced problem. It test correctly. I am still finding holes only if they start on a 16KB boundary which we discussed before. I mention it in case more advanced test cases are generated.

Quoted 5 lines

Question:
If the routines that are looking for extents/hole return an error (I see EFSCORRUPTED, EAGAIN, ENOMEM, EIO as possible errors in these routines), should you convert them to an error such as EIO? There is no specific error mention in the lseek manual page.

Thanks for your review!
The 'errno' should be convert to -ENXIO in those cases(this is the way I observed from Btrfs and OCFS2), I'll take care of it in next post.

Thanks,
-Jeff

Quoted 7 lines

--Mark Tinguely.

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs