* [PATCH 1/4] fs: add SEEK_HOLE and SEEK_DATA flags
@ 2011-06-27 18:02 Josef Bacik
2011-06-27 18:02 ` [PATCH 2/4] Btrfs: implement our own ->llseek Josef Bacik
` (4 more replies)
0 siblings, 5 replies; 37+ messages in thread
From: Josef Bacik @ 2011-06-27 18:02 UTC (permalink / raw)
To: linux-fsdevel, linux-btrfs, linux-kernel, viro
This just gets us ready to support the SEEK_HOLE and SEEK_DATA flags. Turns out
using fiemap in things like cp cause more problems than it solves, so lets try
and give userspace an interface that doesn't suck. We need to match solaris
here, and the definitions are
*o* If /whence/ is SEEK_HOLE, the offset of the start of the
next hole greater than or equal to the supplied offset
is returned. The definition of a hole is provided near
the end of the DESCRIPTION.
*o* If /whence/ is SEEK_DATA, the file pointer is set to the
start of the next non-hole file region greater than or
equal to the supplied offset.
So in the generic case the entire file is data and there is a virtual hole at
the end. That means we will just return i_size for SEEK_HOLE and will return
the same offset for SEEK_DATA. This is how Solaris does it so we have to do it
the same way.
Thanks,
Signed-off-by: Josef Bacik <josef@redhat.com>
---
fs/read_write.c | 37 +++++++++++++++++++++++++++++++++++++
include/linux/fs.h | 4 +++-
2 files changed, 40 insertions(+), 1 deletions(-)
diff --git a/fs/read_write.c b/fs/read_write.c
index 5520f8a..d58551e 100644
--- a/fs/read_write.c
+++ b/fs/read_write.c
@@ -64,6 +64,23 @@ generic_file_llseek_unlocked(struct file *file, loff_t offset, int origin)
return file->f_pos;
offset += file->f_pos;
break;
+ case SEEK_DATA:
+ /*
+ * In the generic case the entire file is data, so as long as
+ * offset isn't at the end of the file then the offset is data.
+ */
+ if (offset >= inode->i_size)
+ return -ENXIO;
+ break;
+ case SEEK_HOLE:
+ /*
+ * There is a virtual hole at the end of the file, so as long as
+ * offset isn't i_size or larger, return i_size.
+ */
+ if (offset >= inode->i_size)
+ return -ENXIO;
+ offset = inode->i_size;
+ break;
}
if (offset < 0 && !unsigned_offsets(file))
@@ -141,6 +158,26 @@ loff_t default_llseek(struct file *file, loff_t offset, int origin)
goto out;
}
offset += file->f_pos;
+ break;
+ case SEEK_DATA:
+ /*
+ * In the generic case the entire file is data, so as
+ * long as offset isn't at the end of the file then the
+ * offset is data.
+ */
+ if (offset >= inode->i_size)
+ return -ENXIO;
+ break;
+ case SEEK_HOLE:
+ /*
+ * There is a virtual hole at the end of the file, so
+ * as long as offset isn't i_size or larger, return
+ * i_size.
+ */
+ if (offset >= inode->i_size)
+ return -ENXIO;
+ offset = inode->i_size;
+ break;
}
retval = -EINVAL;
if (offset >= 0 || unsigned_offsets(file)) {
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 6e73e2e..552cdfe 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -32,7 +32,9 @@
#define SEEK_SET 0 /* seek relative to beginning of file */
#define SEEK_CUR 1 /* seek relative to current file position */
#define SEEK_END 2 /* seek relative to end of file */
-#define SEEK_MAX SEEK_END
+#define SEEK_DATA 3 /* seek to the next data */
+#define SEEK_HOLE 4 /* seek to the next hole */
+#define SEEK_MAX SEEK_HOLE
struct fstrim_range {
__u64 start;
--
1.7.5.2
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH 2/4] Btrfs: implement our own ->llseek
2011-06-27 18:02 [PATCH 1/4] fs: add SEEK_HOLE and SEEK_DATA flags Josef Bacik
@ 2011-06-27 18:02 ` Josef Bacik
2011-06-27 18:02 ` [PATCH 3/4] Ext4: handle SEEK_HOLE/SEEK_DATA generically Josef Bacik
` (3 subsequent siblings)
4 siblings, 0 replies; 37+ messages in thread
From: Josef Bacik @ 2011-06-27 18:02 UTC (permalink / raw)
To: linux-fsdevel, linux-btrfs, linux-kernel, viro
In order to handle SEEK_HOLE/SEEK_DATA we need to implement our own llseek.
Basically for the normal SEEK_*'s we will just defer to the generic helper, and
for SEEK_HOLE/SEEK_DATA we will use our fiemap helper to figure out the nearest
hole or data. Currently this helper doesn't check for delalloc bytes for
prealloc space, so for now treat prealloc as data until that is fixed. Thanks,
Signed-off-by: Josef Bacik <josef@redhat.com>
---
fs/btrfs/ctree.h | 3 +
fs/btrfs/file.c | 148 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 150 insertions(+), 1 deletions(-)
diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 3006287..8040f4b 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -2506,6 +2506,9 @@ int btrfs_csum_truncate(struct btrfs_trans_handle *trans,
int btrfs_lookup_csums_range(struct btrfs_root *root, u64 start, u64 end,
struct list_head *list, int search_commit);
/* inode.c */
+struct extent_map *btrfs_get_extent_fiemap(struct inode *inode, struct page *page,
+ size_t pg_offset, u64 start, u64 len,
+ int create);
/* RHEL and EL kernels have a patch that renames PG_checked to FsMisc */
#if defined(ClearPageFsMisc) && !defined(ClearPageChecked)
diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
index fa4ef18..bd4d061 100644
--- a/fs/btrfs/file.c
+++ b/fs/btrfs/file.c
@@ -1664,8 +1664,154 @@ out:
return ret;
}
+static int find_desired_extent(struct inode *inode, loff_t *offset, int origin)
+{
+ struct btrfs_root *root = BTRFS_I(inode)->root;
+ struct extent_map *em;
+ struct extent_state *cached_state = NULL;
+ u64 lockstart = *offset;
+ u64 lockend = i_size_read(inode);
+ u64 start = *offset;
+ u64 orig_start = *offset;
+ u64 len = i_size_read(inode);
+ u64 last_end = 0;
+ int ret = 0;
+
+ lockend = max_t(u64, root->sectorsize, lockend);
+ if (lockend <= lockstart)
+ lockend = lockstart + root->sectorsize;
+
+ len = lockend - lockstart + 1;
+
+ len = max_t(u64, len, root->sectorsize);
+ if (inode->i_size == 0)
+ return -ENXIO;
+
+ lock_extent_bits(&BTRFS_I(inode)->io_tree, lockstart, lockend, 0,
+ &cached_state, GFP_NOFS);
+
+ /*
+ * Delalloc is such a pain. If we have a hole and we have pending
+ * delalloc for a portion of the hole we will get back a hole that
+ * exists for the entire range since it hasn't been actually written
+ * yet. So to take care of this case we need to look for an extent just
+ * before the position we want in case there is outstanding delalloc
+ * going on here.
+ */
+ if (origin == SEEK_HOLE && start != 0) {
+ if (start <= root->sectorsize)
+ em = btrfs_get_extent_fiemap(inode, NULL, 0, 0,
+ root->sectorsize, 0);
+ else
+ em = btrfs_get_extent_fiemap(inode, NULL, 0,
+ start - root->sectorsize,
+ root->sectorsize, 0);
+ if (IS_ERR(em)) {
+ ret = -ENXIO;
+ goto out;
+ }
+ last_end = em->start + em->len;
+ if (em->block_start == EXTENT_MAP_DELALLOC)
+ last_end = min_t(u64, last_end, inode->i_size);
+ free_extent_map(em);
+ }
+
+ while (1) {
+ em = btrfs_get_extent_fiemap(inode, NULL, 0, start, len, 0);
+ if (IS_ERR(em)) {
+ ret = -ENXIO;
+ break;
+ }
+
+ if (em->block_start == EXTENT_MAP_HOLE) {
+ if (test_bit(EXTENT_FLAG_VACANCY, &em->flags)) {
+ if (last_end <= orig_start) {
+ free_extent_map(em);
+ ret = -ENXIO;
+ break;
+ }
+ }
+
+ if (origin == SEEK_HOLE) {
+ *offset = start;
+ free_extent_map(em);
+ break;
+ }
+ } else {
+ if (origin == SEEK_DATA) {
+ if (em->block_start == EXTENT_MAP_DELALLOC) {
+ if (start >= inode->i_size) {
+ free_extent_map(em);
+ ret = -ENXIO;
+ break;
+ }
+ }
+
+ *offset = start;
+ free_extent_map(em);
+ break;
+ }
+ }
+
+ start = em->start + em->len;
+ last_end = em->start + em->len;
+
+ if (em->block_start == EXTENT_MAP_DELALLOC)
+ last_end = min_t(u64, last_end, inode->i_size);
+
+ if (test_bit(EXTENT_FLAG_VACANCY, &em->flags)) {
+ free_extent_map(em);
+ ret = -ENXIO;
+ break;
+ }
+ free_extent_map(em);
+ cond_resched();
+ }
+ if (!ret)
+ *offset = min(*offset, inode->i_size);
+out:
+ unlock_extent_cached(&BTRFS_I(inode)->io_tree, lockstart, lockend,
+ &cached_state, GFP_NOFS);
+ return ret;
+}
+
+static loff_t btrfs_file_llseek(struct file *file, loff_t offset, int origin)
+{
+ struct inode *inode = file->f_mapping->host;
+ int ret;
+
+ mutex_lock(&inode->i_mutex);
+ switch (origin) {
+ case SEEK_END:
+ case SEEK_CUR:
+ offset = generic_file_llseek_unlocked(file, offset, origin);
+ goto out;
+ case SEEK_DATA:
+ case SEEK_HOLE:
+ ret = find_desired_extent(inode, &offset, origin);
+ if (ret) {
+ mutex_unlock(&inode->i_mutex);
+ return ret;
+ }
+ }
+
+ if (offset < 0 && !(file->f_mode & FMODE_UNSIGNED_OFFSET))
+ return -EINVAL;
+ if (offset > inode->i_sb->s_maxbytes)
+ return -EINVAL;
+
+ /* Special lock needed here? */
+ if (offset != file->f_pos) {
+ file->f_pos = offset;
+ file->f_version = 0;
+ }
+out:
+ mutex_unlock(&inode->i_mutex);
+ return offset;
+}
+
const struct file_operations btrfs_file_operations = {
- .llseek = generic_file_llseek,
+ .llseek = btrfs_file_llseek,
.read = do_sync_read,
.write = do_sync_write,
.aio_read = generic_file_aio_read,
--
1.7.5.2
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH 3/4] Ext4: handle SEEK_HOLE/SEEK_DATA generically
2011-06-27 18:02 [PATCH 1/4] fs: add SEEK_HOLE and SEEK_DATA flags Josef Bacik
2011-06-27 18:02 ` [PATCH 2/4] Btrfs: implement our own ->llseek Josef Bacik
@ 2011-06-27 18:02 ` Josef Bacik
2011-06-27 18:02 ` [PATCH 4/4] fs: handle SEEK_HOLE/SEEK_DATA properly in all fs's that define their own llseek Josef Bacik
` (2 subsequent siblings)
4 siblings, 0 replies; 37+ messages in thread
From: Josef Bacik @ 2011-06-27 18:02 UTC (permalink / raw)
To: linux-fsdevel, linux-btrfs, linux-kernel, viro
Since Ext4 has its own lseek we need to make sure it handles
SEEK_HOLE/SEEK_DATA. For now just do the same thing that is done in the generic
case, somebody else can come along and make it do fancy things later. Thanks,
Signed-off-by: Josef Bacik <josef@redhat.com>
---
fs/ext4/file.c | 21 +++++++++++++++++++++
1 files changed, 21 insertions(+), 0 deletions(-)
diff --git a/fs/ext4/file.c b/fs/ext4/file.c
index 2c09723..ce766f9 100644
--- a/fs/ext4/file.c
+++ b/fs/ext4/file.c
@@ -236,6 +236,27 @@ loff_t ext4_llseek(struct file *file, loff_t offset, int origin)
}
offset += file->f_pos;
break;
+ case SEEK_DATA:
+ /*
+ * In the generic case the entire file is data, so as long as
+ * offset isn't at the end of the file then the offset is data.
+ */
+ if (offset >= inode->i_size) {
+ mutex_unlock(&inode->i_mutex);
+ return -ENXIO;
+ }
+ break;
+ case SEEK_HOLE:
+ /*
+ * There is a virtual hole at the end of the file, so as long as
+ * offset isn't i_size or larger, return i_size.
+ */
+ if (offset >= inode->i_size) {
+ mutex_unlock(&inode->i_mutex);
+ return -ENXIO;
+ }
+ offset = inode->i_size;
+ break;
}
if (offset < 0 || offset > maxbytes) {
--
1.7.5.2
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH 4/4] fs: handle SEEK_HOLE/SEEK_DATA properly in all fs's that define their own llseek
2011-06-27 18:02 [PATCH 1/4] fs: add SEEK_HOLE and SEEK_DATA flags Josef Bacik
2011-06-27 18:02 ` [PATCH 2/4] Btrfs: implement our own ->llseek Josef Bacik
2011-06-27 18:02 ` [PATCH 3/4] Ext4: handle SEEK_HOLE/SEEK_DATA generically Josef Bacik
@ 2011-06-27 18:02 ` Josef Bacik
2011-06-27 18:02 ` [PATCH] xfstests 255: add a seek_data/seek_hole tester Josef Bacik
2011-06-27 21:04 ` [PATCH 1/4] fs: add SEEK_HOLE and SEEK_DATA flags Josef Bacik
4 siblings, 0 replies; 37+ messages in thread
From: Josef Bacik @ 2011-06-27 18:02 UTC (permalink / raw)
To: linux-fsdevel, linux-btrfs, linux-kernel, viro
This converts everybody to handle SEEK_HOLE/SEEK_DATA properly. In some cases
we just return -EINVAL, in others we do the normal generic thing, and in others
we're simply making sure that the properly due-dilligence is done. For example
in NFS/CIFS we need to make sure the file size is update properly for the
SEEK_HOLE and SEEK_DATA case, but since it calls the generic llseek stuff itself
that is all we have to do. Thanks,
Signed-off-by: Josef Bacik <josef@redhat.com>
---
fs/block_dev.c | 11 ++++++++---
fs/ceph/dir.c | 8 +++++++-
fs/ceph/file.c | 20 ++++++++++++++++++--
fs/cifs/cifsfs.c | 7 +++++--
fs/fuse/file.c | 21 +++++++++++++++++++--
fs/hpfs/dir.c | 4 ++++
fs/nfs/file.c | 7 +++++--
7 files changed, 66 insertions(+), 12 deletions(-)
diff --git a/fs/block_dev.c b/fs/block_dev.c
index 1a2421f..29fde2c 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -355,20 +355,25 @@ static loff_t block_llseek(struct file *file, loff_t offset, int origin)
mutex_lock(&bd_inode->i_mutex);
size = i_size_read(bd_inode);
+ retval = -EINVAL;
switch (origin) {
- case 2:
+ case SEEK_END:
offset += size;
break;
- case 1:
+ case SEEK_CUR:
offset += file->f_pos;
+ case SEEK_SET:
+ break;
+ default:
+ goto out;
}
- retval = -EINVAL;
if (offset >= 0 && offset <= size) {
if (offset != file->f_pos) {
file->f_pos = offset;
}
retval = offset;
}
+out:
mutex_unlock(&bd_inode->i_mutex);
return retval;
}
diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c
index ef8f08c..79cd77c 100644
--- a/fs/ceph/dir.c
+++ b/fs/ceph/dir.c
@@ -446,14 +446,19 @@ static loff_t ceph_dir_llseek(struct file *file, loff_t offset, int origin)
loff_t retval;
mutex_lock(&inode->i_mutex);
+ retval = -EINVAL;
switch (origin) {
case SEEK_END:
offset += inode->i_size + 2; /* FIXME */
break;
case SEEK_CUR:
offset += file->f_pos;
+ case SEEK_SET:
+ break;
+ default:
+ goto out;
}
- retval = -EINVAL;
+
if (offset >= 0 && offset <= inode->i_sb->s_maxbytes) {
if (offset != file->f_pos) {
file->f_pos = offset;
@@ -477,6 +482,7 @@ static loff_t ceph_dir_llseek(struct file *file, loff_t offset, int origin)
if (offset > old_offset)
fi->dir_release_count--;
}
+out:
mutex_unlock(&inode->i_mutex);
return retval;
}
diff --git a/fs/ceph/file.c b/fs/ceph/file.c
index 9542f07..774feb1 100644
--- a/fs/ceph/file.c
+++ b/fs/ceph/file.c
@@ -770,13 +770,16 @@ static loff_t ceph_llseek(struct file *file, loff_t offset, int origin)
mutex_lock(&inode->i_mutex);
__ceph_do_pending_vmtruncate(inode);
- switch (origin) {
- case SEEK_END:
+ if (origin != SEEK_CUR || origin != SEEK_SET) {
ret = ceph_do_getattr(inode, CEPH_STAT_CAP_SIZE);
if (ret < 0) {
offset = ret;
goto out;
}
+ }
+
+ switch (origin) {
+ case SEEK_END:
offset += inode->i_size;
break;
case SEEK_CUR:
@@ -792,6 +795,19 @@ static loff_t ceph_llseek(struct file *file, loff_t offset, int origin)
}
offset += file->f_pos;
break;
+ case SEEK_DATA:
+ if (offset >= inode->i_size) {
+ ret = -ENXIO;
+ goto out;
+ }
+ break;
+ case SEEK_HOLE:
+ if (offset >= inode->i_size) {
+ ret = -ENXIO;
+ goto out;
+ }
+ offset = inode->i_size;
+ break;
}
if (offset < 0 || offset > inode->i_sb->s_maxbytes) {
diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index 2f0c586..acaaf62 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -772,8 +772,11 @@ static ssize_t cifs_file_aio_write(struct kiocb *iocb, const struct iovec *iov,
static loff_t cifs_llseek(struct file *file, loff_t offset, int origin)
{
- /* origin == SEEK_END => we must revalidate the cached file length */
- if (origin == SEEK_END) {
+ /*
+ * origin == SEEK_END || SEEK_DATA || SEEK_HOLE => we must revalidate
+ * the cached file length
+ */
+ if (origin != SEEK_SET || origin != SEEK_CUR) {
int rc;
struct inode *inode = file->f_path.dentry->d_inode;
diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index 82a6646..73b89df 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -1600,15 +1600,32 @@ static loff_t fuse_file_llseek(struct file *file, loff_t offset, int origin)
struct inode *inode = file->f_path.dentry->d_inode;
mutex_lock(&inode->i_mutex);
- switch (origin) {
- case SEEK_END:
+ if (origin != SEEK_CUR || origin != SEEK_SET) {
retval = fuse_update_attributes(inode, NULL, file, NULL);
if (retval)
goto exit;
+ }
+
+ switch (origin) {
+ case SEEK_END:
offset += i_size_read(inode);
break;
case SEEK_CUR:
offset += file->f_pos;
+ break;
+ case SEEK_DATA:
+ if (offset >= i_size_read(inode)) {
+ retval = -ENXIO;
+ goto exit;
+ }
+ break;
+ case SEEK_HOLE:
+ if (offset >= i_size_read(inode)) {
+ retval = -ENXIO;
+ goto exit;
+ }
+ offset = i_size_read(inode);
+ break;
}
retval = -EINVAL;
if (offset >= 0 && offset <= inode->i_sb->s_maxbytes) {
diff --git a/fs/hpfs/dir.c b/fs/hpfs/dir.c
index f46ae02..96a8ed9 100644
--- a/fs/hpfs/dir.c
+++ b/fs/hpfs/dir.c
@@ -29,6 +29,10 @@ static loff_t hpfs_dir_lseek(struct file *filp, loff_t off, int whence)
struct hpfs_inode_info *hpfs_inode = hpfs_i(i);
struct super_block *s = i->i_sb;
+ /* Somebody else will have to figure out what to do here */
+ if (whence == SEEK_DATA || whence == SEEK_HOLE)
+ return -EINVAL;
+
hpfs_lock(s);
/*printk("dir lseek\n");*/
diff --git a/fs/nfs/file.c b/fs/nfs/file.c
index 2f093ed..2c1705b 100644
--- a/fs/nfs/file.c
+++ b/fs/nfs/file.c
@@ -187,8 +187,11 @@ static loff_t nfs_file_llseek(struct file *filp, loff_t offset, int origin)
filp->f_path.dentry->d_name.name,
offset, origin);
- /* origin == SEEK_END => we must revalidate the cached file length */
- if (origin == SEEK_END) {
+ /*
+ * origin == SEEK_END || SEEK_DATA || SEEK_HOLE => we must revalidate
+ * the cached file length
+ */
+ if (origin != SEEK_SET || origin != SEEK_CUR) {
struct inode *inode = filp->f_mapping->host;
int retval = nfs_revalidate_file_size(inode, filp);
--
1.7.5.2
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH] xfstests 255: add a seek_data/seek_hole tester
2011-06-27 18:02 [PATCH 1/4] fs: add SEEK_HOLE and SEEK_DATA flags Josef Bacik
` (2 preceding siblings ...)
2011-06-27 18:02 ` [PATCH 4/4] fs: handle SEEK_HOLE/SEEK_DATA properly in all fs's that define their own llseek Josef Bacik
@ 2011-06-27 18:02 ` Josef Bacik
2011-06-27 18:32 ` Andreas Dilger
2011-06-27 21:04 ` [PATCH 1/4] fs: add SEEK_HOLE and SEEK_DATA flags Josef Bacik
4 siblings, 1 reply; 37+ messages in thread
From: Josef Bacik @ 2011-06-27 18:02 UTC (permalink / raw)
To: linux-fsdevel, linux-btrfs, linux-kernel, viro
This is a test to make sure seek_data/seek_hole is acting like it does on
Solaris. It will check to see if the fs supports finding a hole or not and will
adjust as necessary.
Cc: xfs@oss.sgi.com
Signed-off-by: Josef Bacik <josef@redhat.com>
---
255 | 71 ++++++++
255.out | 2 +
src/seek-tester.c | 473 +++++++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 546 insertions(+), 0 deletions(-)
create mode 100755 255
create mode 100644 255.out
create mode 100644 src/seek-tester.c
diff --git a/255 b/255
new file mode 100755
index 0000000..4bb4d0b
--- /dev/null
+++ b/255
@@ -0,0 +1,71 @@
+#! /bin/bash
+# FS QA Test No. 255
+#
+# Test SEEK_DATA and SEEK_HOLE
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2011 Red Hat. All Rights Reserved.
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it would be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write the Free Software Foundation,
+# Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+#
+#-----------------------------------------------------------------------
+#
+# creator
+owner=josef@redhat.com
+
+seq=`basename $0`
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1 # failure is the default!
+
+_cleanup()
+{
+ rm -f $tmp.*
+}
+
+trap "_cleanup ; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+
+# real QA test starts here
+_supported_fs generic
+_supported_os Linux
+
+testfile=$TEST_DIR/seek_test.$$
+logfile=$TEST_DIR/seek_test.$$.log
+
+[ -x $here/src/seek-tester ] || _notrun "seek-tester not built"
+
+_cleanup()
+{
+ rm -f $testfile
+ rm -f $logfile
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+echo "Silence is golden"
+$here/src/seek-tester -q $testfile 2>&1 | tee -a $logfile
+
+if grep -q "SEEK_HOLE is not supported" $logfile; then
+ _notrun "SEEK_HOLE/SEEK_DATA not supported by this kernel"
+fi
+
+rm -f $logfile
+rm -f $testfile
+
+status=0 ; exit
diff --git a/255.out b/255.out
new file mode 100644
index 0000000..7eefb82
--- /dev/null
+++ b/255.out
@@ -0,0 +1,2 @@
+QA output created by 255
+Silence is golden
diff --git a/src/seek-tester.c b/src/seek-tester.c
new file mode 100644
index 0000000..2b8c957
--- /dev/null
+++ b/src/seek-tester.c
@@ -0,0 +1,473 @@
+/*
+ * Copyright (C) 2011 Oracle. All rights reserved.
+ * Copyright (C) 2011 Red Hat. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public
+ * License v2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public
+ * License along with this program; if not, write to the
+ * Free Software Foundation, Inc., 59 Temple Place - Suite 330,
+ * Boston, MA 021110-1307, USA.
+ */
+
+#define _XOPEN_SOURCE 500
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <errno.h>
+#include <fcntl.h>
+#include <stdio.h>
+#include <string.h>
+#include <unistd.h>
+#include <stdlib.h>
+
+#define SEEK_DATA 3
+#define SEEK_HOLE 4
+
+#define FS_NO_HOLES (1 << 0)
+#define QUIET (1 << 1)
+
+static blksize_t alloc_size;
+static unsigned flags = 0;
+
+static int get_io_sizes(int fd)
+{
+ struct stat buf;
+ int ret;
+
+ ret = fstat(fd, &buf);
+ if (ret)
+ fprintf(stderr, " ERROR %d: Failed to find io blocksize\n",
+ errno);
+
+ /* st_blksize is typically also the allocation size */
+ alloc_size = buf.st_blksize;
+
+ if (!(flags & QUIET))
+ printf("Allocation size: %ld\n", alloc_size);
+
+ return ret;
+}
+
+#define do_free(x) do { if(x) free(x); } while(0);
+
+static void *do_malloc(size_t size)
+{
+ void *buf;
+
+ buf = malloc(size);
+ if (!buf)
+ fprintf(stderr, " ERROR: Unable to allocate %ld bytes\n",
+ (long)size);
+
+ return buf;
+}
+
+static int do_truncate(int fd, off_t length)
+{
+ int ret;
+
+ ret = ftruncate(fd, length);
+ if (ret)
+ fprintf(stderr, " ERROR %d: Failed to extend file "
+ "to %ld bytes\n", errno, (long)length);
+ return ret;
+}
+
+static ssize_t do_pwrite(int fd, const void *buf, size_t count, off_t offset)
+{
+ ssize_t ret, written = 0;
+
+ while (count > written) {
+ ret = pwrite(fd, buf + written, count - written, offset + written);
+ if (ret < 0) {
+ fprintf(stderr, " ERROR %d: Failed to write %ld "
+ "bytes\n", errno, (long)count);
+ return ret;
+ }
+ written += ret;
+ }
+
+ return 0;
+}
+
+static int do_lseek(int testnum, int subtest, int fd, int origin, off_t set,
+ off_t exp)
+{
+ off_t pos;
+ int ret = -1;
+
+ pos = lseek(fd, set, origin);
+
+ if (pos != exp) {
+ fprintf(stderr, " ERROR in Test %d.%d: POS expected %ld, "
+ "got %ld\n", testnum, subtest, (long)exp, (long)pos);
+ goto out;
+ }
+
+ if (pos == -1 && errno != ENXIO) {
+ fprintf(stderr, " ERROR in Test %d.%d: ERRNO expected %d, "
+ "got %d\n", testnum, subtest, ENXIO, errno);
+ goto out;
+ }
+
+ ret = 0;
+
+out:
+ return ret;
+}
+
+static int get_flags(int fd)
+{
+ const char *buf = "ABCDEF";
+ ssize_t written;
+ off_t pos;
+ int ret;
+
+ ret = do_truncate(fd, alloc_size * 2);
+ if (ret)
+ return ret;
+
+ written = do_pwrite(fd, buf, strlen(buf), 0);
+ if (written)
+ return -1;
+
+ pos = lseek(fd, 0, SEEK_HOLE);
+ if (pos == alloc_size * 2) {
+ if (!(flags & QUIET))
+ printf("File system does not recognize holes, the only "
+ "hole found will be at the end.\n");
+ flags |= FS_NO_HOLES;
+ } else if (pos == (off_t)-1) {
+ fprintf(stderr, "SEEK_HOLE is not supported\n");
+ return -1;
+ }
+
+ return 0;
+}
+
+/* test hole data hole data */
+static int test06(int fd, int testnum)
+{
+ int ret = 0;
+ char *buf = NULL;
+ int bufsz = alloc_size;
+ int filsz = bufsz * 4;
+ int off;
+
+ if (flags & FS_NO_HOLES)
+ return 1;
+
+ /* HOLE - DATA - HOLE - DATA */
+ /* Each unit is bufsz */
+
+ buf = do_malloc(bufsz);
+ if (!buf)
+ goto out;
+ memset(buf, 'a', bufsz);
+
+ ret = do_pwrite(fd, buf, bufsz, bufsz);
+ if (!ret)
+ do_pwrite(fd, buf, bufsz, bufsz * 3);
+ if (ret)
+ goto out;
+
+ /* offset at the beginning */
+ ret += do_lseek(testnum, 1, fd, SEEK_HOLE, 0, 0);
+ ret += do_lseek(testnum, 2, fd, SEEK_HOLE, 1, 1);
+ ret += do_lseek(testnum, 3, fd, SEEK_DATA, 0, bufsz);
+ ret += do_lseek(testnum, 4, fd, SEEK_DATA, 1, bufsz);
+
+ /* offset around first hole-data boundary */
+ off = bufsz;
+ ret += do_lseek(testnum, 5, fd, SEEK_HOLE, off - 1, off - 1);
+ ret += do_lseek(testnum, 6, fd, SEEK_DATA, off - 1, off);
+ ret += do_lseek(testnum, 7, fd, SEEK_HOLE, off, bufsz * 2);
+ ret += do_lseek(testnum, 8, fd, SEEK_DATA, off, off);
+ ret += do_lseek(testnum, 9, fd, SEEK_HOLE, off + 1, bufsz * 2);
+ ret += do_lseek(testnum, 10, fd, SEEK_DATA, off + 1, off + 1);
+
+ /* offset around data-hole boundary */
+ off = bufsz * 2;
+ ret += do_lseek(testnum, 11, fd, SEEK_HOLE, off - 1, off);
+ ret += do_lseek(testnum, 12, fd, SEEK_DATA, off - 1, off - 1);
+ ret += do_lseek(testnum, 13, fd, SEEK_HOLE, off, off);
+ ret += do_lseek(testnum, 14, fd, SEEK_DATA, off, bufsz * 3);
+ ret += do_lseek(testnum, 15, fd, SEEK_HOLE, off + 1, off + 1);
+ ret += do_lseek(testnum, 16, fd, SEEK_DATA, off + 1, bufsz * 3);
+
+ /* offset around second hole-data boundary */
+ off = bufsz * 3;
+ ret += do_lseek(testnum, 17, fd, SEEK_HOLE, off - 1, off - 1);
+ ret += do_lseek(testnum, 18, fd, SEEK_DATA, off - 1, off);
+ ret += do_lseek(testnum, 19, fd, SEEK_HOLE, off, filsz);
+ ret += do_lseek(testnum, 20, fd, SEEK_DATA, off, off);
+ ret += do_lseek(testnum, 21, fd, SEEK_HOLE, off + 1, filsz);
+ ret += do_lseek(testnum, 22, fd, SEEK_DATA, off + 1, off + 1);
+
+ /* offset around the end of file */
+ off = filsz;
+ ret += do_lseek(testnum, 23, fd, SEEK_HOLE, off - 1, filsz);
+ ret += do_lseek(testnum, 24, fd, SEEK_DATA, off - 1, filsz - 1);
+ ret += do_lseek(testnum, 25, fd, SEEK_HOLE, off, -1);
+ ret += do_lseek(testnum, 26, fd, SEEK_DATA, off, -1);
+ ret += do_lseek(testnum, 27, fd, SEEK_HOLE, off + 1, -1);
+ ret += do_lseek(testnum, 28, fd, SEEK_DATA, off + 1, -1);
+
+out:
+ do_free(buf);
+ return ret;
+}
+
+/* test file with data at the beginning and a hole at the end */
+static int test05(int fd, int testnum)
+{
+ int ret = -1;
+ char *buf = NULL;
+ int bufsz = alloc_size;
+ int filsz = bufsz * 4;
+
+ if (flags & FS_NO_HOLES)
+ return 1;
+
+ /* DATA - HOLE */
+ /* Each unit is bufsz */
+
+ buf = do_malloc(bufsz);
+ if (!buf)
+ goto out;
+ memset(buf, 'a', bufsz);
+
+ ret = do_truncate(fd, filsz);
+ if (!ret)
+ ret = do_pwrite(fd, buf, bufsz, 0);
+ if (ret)
+ goto out;
+
+ /* offset at the beginning */
+ ret += do_lseek(testnum, 1, fd, SEEK_HOLE, 0, bufsz);
+ ret += do_lseek(testnum, 2, fd, SEEK_HOLE, 1, bufsz);
+ ret += do_lseek(testnum, 3, fd, SEEK_DATA, 0, 0);
+ ret += do_lseek(testnum, 4, fd, SEEK_DATA, 1, 1);
+
+ /* offset around data-hole boundary */
+ ret += do_lseek(testnum, 5, fd, SEEK_HOLE, bufsz - 1, bufsz);
+ ret += do_lseek(testnum, 6, fd, SEEK_DATA, bufsz - 1, bufsz - 1);
+ ret += do_lseek(testnum, 7, fd, SEEK_HOLE, bufsz, bufsz);
+ ret += do_lseek(testnum, 8, fd, SEEK_DATA, bufsz, -1);
+ ret += do_lseek(testnum, 9, fd, SEEK_HOLE, bufsz + 1, bufsz + 1);
+ ret += do_lseek(testnum, 10, fd, SEEK_DATA, bufsz + 1, -1);
+
+ /* offset around eof */
+ ret += do_lseek(testnum, 11, fd, SEEK_HOLE, filsz - 1, filsz - 1);
+ ret += do_lseek(testnum, 12, fd, SEEK_DATA, filsz - 1, -1);
+ ret += do_lseek(testnum, 13, fd, SEEK_HOLE, filsz, -1);
+ ret += do_lseek(testnum, 14, fd, SEEK_DATA, filsz, -1);
+ ret += do_lseek(testnum, 15, fd, SEEK_HOLE, filsz + 1, -1);
+ ret += do_lseek(testnum, 16, fd, SEEK_DATA, filsz + 1, -1);
+
+out:
+ do_free(buf);
+ return ret;
+}
+
+/* test hole begin and data end */
+static int test04(int fd, int testnum)
+{
+ int ret;
+ char *buf = "ABCDEFGH";
+ int bufsz = sizeof(buf);
+ int holsz = alloc_size * 2;
+ int filsz = holsz + bufsz;
+
+ if (flags & FS_NO_HOLES)
+ return 1;
+
+ /* HOLE - DATA */
+
+ ret = do_pwrite(fd, buf, bufsz, holsz);
+ if (ret)
+ goto out;
+
+ /* offset at the beginning */
+ ret += do_lseek(testnum, 1, fd, SEEK_HOLE, 0, 0);
+ ret += do_lseek(testnum, 2, fd, SEEK_HOLE, 1, 1);
+ ret += do_lseek(testnum, 3, fd, SEEK_DATA, 0, holsz);
+ ret += do_lseek(testnum, 4, fd, SEEK_DATA, 1, holsz);
+
+ /* offset around hole-data boundary */
+ ret += do_lseek(testnum, 5, fd, SEEK_HOLE, holsz - 1, holsz - 1);
+ ret += do_lseek(testnum, 6, fd, SEEK_DATA, holsz - 1, holsz);
+ ret += do_lseek(testnum, 7, fd, SEEK_HOLE, holsz, filsz);
+ ret += do_lseek(testnum, 8, fd, SEEK_DATA, holsz, holsz);
+ ret += do_lseek(testnum, 9, fd, SEEK_HOLE, holsz + 1, filsz);
+ ret += do_lseek(testnum, 10, fd, SEEK_DATA, holsz + 1, holsz + 1);
+
+ /* offset around eof */
+ ret += do_lseek(testnum, 11, fd, SEEK_HOLE, filsz - 1, filsz);
+ ret += do_lseek(testnum, 12, fd, SEEK_DATA, filsz - 1, filsz - 1);
+ ret += do_lseek(testnum, 13, fd, SEEK_HOLE, filsz, -1);
+ ret += do_lseek(testnum, 14, fd, SEEK_DATA, filsz, -1);
+ ret += do_lseek(testnum, 15, fd, SEEK_HOLE, filsz + 1, -1);
+ ret += do_lseek(testnum, 16, fd, SEEK_DATA, filsz + 1, -1);
+out:
+ return ret;
+}
+
+/* test full file */
+static int test03(int fd, int testnum)
+{
+ char *buf = NULL;
+ int bufsz = alloc_size + 100;
+ int ret = -1;
+
+ buf = do_malloc(bufsz);
+ if (!buf)
+ goto out;
+ memset(buf, 'a', bufsz);
+
+ ret = do_pwrite(fd, buf, bufsz, 0);
+ if (ret)
+ goto out;
+
+ /* offset at the beginning */
+ ret += do_lseek(testnum, 1, fd, SEEK_HOLE, 0, bufsz);
+ ret += do_lseek(testnum, 2, fd, SEEK_HOLE, 1, bufsz);
+ ret += do_lseek(testnum, 3, fd, SEEK_DATA, 0, 0);
+ ret += do_lseek(testnum, 4, fd, SEEK_DATA, 1, 1);
+
+ /* offset around eof */
+ ret += do_lseek(testnum, 5, fd, SEEK_HOLE, bufsz - 1, bufsz);
+ ret += do_lseek(testnum, 6, fd, SEEK_DATA, bufsz - 1, bufsz - 1);
+ ret += do_lseek(testnum, 7, fd, SEEK_HOLE, bufsz, -1);
+ ret += do_lseek(testnum, 8, fd, SEEK_DATA, bufsz, -1);
+ ret += do_lseek(testnum, 9, fd, SEEK_HOLE, bufsz + 1, -1);
+ ret += do_lseek(testnum, 10, fd, SEEK_DATA, bufsz + 1, -1);
+
+out:
+ do_free(buf);
+ return ret;
+}
+
+/* test empty file */
+static int test02(int fd, int testnum)
+{
+ int ret = 0;
+
+ ret += do_lseek(testnum, 1, fd, SEEK_DATA, 0, -1);
+ ret += do_lseek(testnum, 2, fd, SEEK_HOLE, 0, -1);
+ ret += do_lseek(testnum, 3, fd, SEEK_HOLE, 1, -1);
+
+ return ret;
+}
+
+/* test feature support */
+static int test01(int fd, int testnum)
+{
+ int ret;
+ char buf[] = "ABCDEFGH";
+ int bufsz = sizeof(buf);
+
+ ret = do_pwrite(fd, buf, bufsz, 0);
+ if (ret)
+ goto out;
+
+ ret += do_lseek(testnum, 1, fd, SEEK_DATA, 0, 0);
+ ret += do_lseek(testnum, 2, fd, SEEK_HOLE, 0, bufsz);
+
+out:
+ return ret;
+}
+
+struct testrec {
+ int test_num;
+ int (*test_func)(int fd, int testnum);
+ char *test_desc;
+};
+
+struct testrec seek_tests[] = {
+ { 1, test01, "Test basic support" },
+ { 2, test02, "Test an empty file" },
+ { 3, test03, "Test a full file" },
+ { 4, test04, "Test file hole at beg, data at end" },
+ { 5, test05, "Test file data at beg, hole at end" },
+ { 6, test06, "Test file hole data hole data" },
+};
+
+static int run_test(int fd, struct testrec *tr)
+{
+ int ret;
+
+ ret = tr->test_func(fd, tr->test_num);
+ if (!(flags & QUIET))
+ printf("%02d. %-50s\t%s\n", tr->test_num, tr->test_desc,
+ ret < 0 ? "FAIL" : (ret == 0 ? "SUCC" : "NOT RUN"));
+ return ret;
+}
+
+void print_help()
+{
+ printf("seek-test [-h] [-q] filename\n");
+ printf("\t-h - this message\n");
+ printf("\t-q - quiet, no output\n");
+ printf("\tfilename - file to use for the test\n");
+}
+
+int main(int argc, char **argv)
+{
+ int ret = -1;
+ int i, fd = -1;
+ int c;
+ int numtests = sizeof(seek_tests) / sizeof(struct testrec);
+
+ while ((c = getopt(argc, argv, "qh")) != -1) {
+ switch (c) {
+ case 'q':
+ flags |= QUIET;
+ break;
+ case 'h':
+ print_help();
+ exit(0);
+ default:
+ print_help();
+ exit(1);
+ }
+ }
+
+ if (optind >= argc) {
+ print_help();
+ exit(1);
+ }
+
+ fd = open(argv[optind], O_RDWR|O_CREAT|O_TRUNC, 0644);
+ if (fd < 0) {
+ fprintf(stderr, "Failed to open testfile: %d\n", errno);
+ goto out;
+ }
+
+ ret = get_io_sizes(fd);
+ if (ret)
+ goto out;
+
+ ret = get_flags(fd);
+ if (ret)
+ goto out;
+
+ for (i = 0; i < numtests; ++i) {
+ ret = do_truncate(fd, 0);
+ if (ret)
+ goto out;
+ run_test(fd, &seek_tests[i]);
+ }
+
+out:
+ if (fd > -1)
+ close(fd);
+ return ret;
+}
--
1.7.5.2
^ permalink raw reply related [flat|nested] 37+ messages in thread
* Re: [PATCH] xfstests 255: add a seek_data/seek_hole tester
2011-06-27 18:02 ` [PATCH] xfstests 255: add a seek_data/seek_hole tester Josef Bacik
@ 2011-06-27 18:32 ` Andreas Dilger
2011-06-27 18:47 ` Josef Bacik
0 siblings, 1 reply; 37+ messages in thread
From: Andreas Dilger @ 2011-06-27 18:32 UTC (permalink / raw)
To: Josef Bacik; +Cc: linux-fsdevel, linux-btrfs, linux-kernel, viro
On 2011-06-27, at 12:02 PM, Josef Bacik wrote:
> This is a test to make sure seek_data/seek_hole is acting like it does on
> Solaris. It will check to see if the fs supports finding a hole or not and will
> adjust as necessary.
>
> diff --git a/src/seek-tester.c b/src/seek-tester.c
> new file mode 100644
> index 0000000..2b8c957
> --- /dev/null
> +++ b/src/seek-tester.c
> @@ -0,0 +1,473 @@
> +/*
> + * Copyright (C) 2011 Oracle. All rights reserved.
> + * Copyright (C) 2011 Red Hat. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public
> + * License v2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public
> + * License along with this program; if not, write to the
> + * Free Software Foundation, Inc., 59 Temple Place - Suite 330,
> + * Boston, MA 021110-1307, USA.
> + */
> +
> +#define _XOPEN_SOURCE 500
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +#include <errno.h>
> +#include <fcntl.h>
> +#include <stdio.h>
> +#include <string.h>
> +#include <unistd.h>
> +#include <stdlib.h>
> +
> +#define SEEK_DATA 3
> +#define SEEK_HOLE 4
These should probably be "#ifndef SEEK_DATA" so that gcc doesn't complain
in the future when these are added to a standard header.
> +#define FS_NO_HOLES (1 << 0)
> +#define QUIET (1 << 1)
> +
> +static blksize_t alloc_size;
> +static unsigned flags = 0;
> +
> +static int get_io_sizes(int fd)
> +{
> + struct stat buf;
> + int ret;
> +
> + ret = fstat(fd, &buf);
> + if (ret)
> + fprintf(stderr, " ERROR %d: Failed to find io blocksize\n",
> + errno);
> +
> + /* st_blksize is typically also the allocation size */
> + alloc_size = buf.st_blksize;
> +
> + if (!(flags & QUIET))
> + printf("Allocation size: %ld\n", alloc_size);
> +
> + return ret;
> +}
> +
> +#define do_free(x) do { if(x) free(x); } while(0);
> +
> +static void *do_malloc(size_t size)
> +{
> + void *buf;
> +
> + buf = malloc(size);
> + if (!buf)
> + fprintf(stderr, " ERROR: Unable to allocate %ld bytes\n",
> + (long)size);
> +
> + return buf;
> +}
> +
> +static int do_truncate(int fd, off_t length)
> +{
> + int ret;
> +
> + ret = ftruncate(fd, length);
> + if (ret)
> + fprintf(stderr, " ERROR %d: Failed to extend file "
> + "to %ld bytes\n", errno, (long)length);
> + return ret;
> +}
> +
> +static ssize_t do_pwrite(int fd, const void *buf, size_t count, off_t offset)
> +{
> + ssize_t ret, written = 0;
> +
> + while (count > written) {
> + ret = pwrite(fd, buf + written, count - written, offset + written);
> + if (ret < 0) {
> + fprintf(stderr, " ERROR %d: Failed to write %ld "
> + "bytes\n", errno, (long)count);
> + return ret;
> + }
> + written += ret;
> + }
> +
> + return 0;
> +}
> +
> +static int do_lseek(int testnum, int subtest, int fd, int origin, off_t set,
> + off_t exp)
> +{
> + off_t pos;
> + int ret = -1;
> +
> + pos = lseek(fd, set, origin);
> +
> + if (pos != exp) {
> + fprintf(stderr, " ERROR in Test %d.%d: POS expected %ld, "
> + "got %ld\n", testnum, subtest, (long)exp, (long)pos);
> + goto out;
> + }
> +
> + if (pos == -1 && errno != ENXIO) {
> + fprintf(stderr, " ERROR in Test %d.%d: ERRNO expected %d, "
> + "got %d\n", testnum, subtest, ENXIO, errno);
> + goto out;
> + }
> +
> + ret = 0;
> +
> +out:
> + return ret;
> +}
> +
> +static int get_flags(int fd)
> +{
> + const char *buf = "ABCDEF";
> + ssize_t written;
> + off_t pos;
> + int ret;
> +
> + ret = do_truncate(fd, alloc_size * 2);
> + if (ret)
> + return ret;
> +
> + written = do_pwrite(fd, buf, strlen(buf), 0);
> + if (written)
> + return -1;
> +
> + pos = lseek(fd, 0, SEEK_HOLE);
> + if (pos == alloc_size * 2) {
> + if (!(flags & QUIET))
> + printf("File system does not recognize holes, the only "
> + "hole found will be at the end.\n");
> + flags |= FS_NO_HOLES;
This is a question that I've also had about compatibility with older
(well, every) Linux kernel that does not support SEEK_{HOLE,DATA}
today.
My reading of the existing generic_file_llseek() and default_llseek()
code, along with most filesystem-specific llseek() implementations is
that they will happily ignore the @whence parameter if it is not
known, and pretend like it is 0 (SEEK_SET), so they will just set the
position to the @offset parameter and return this value. In that
case, the above "SEEK_HOLE" test would incorrectly fail on every
Linux kernel in existence today because the returned pos == 0.
Should applications call both SEEK_HOLE and SEEK_DATA with @offset=0,
and if they return the same values (which is normally impossible,
decide that the kernel does not support this SEEK_* functionality?
> + } else if (pos == (off_t)-1) {
> + fprintf(stderr, "SEEK_HOLE is not supported\n");
> + return -1;
> + }
> +
> + return 0;
> +}
> +
> +/* test hole data hole data */
> +static int test06(int fd, int testnum)
> +{
> + int ret = 0;
> + char *buf = NULL;
> + int bufsz = alloc_size;
> + int filsz = bufsz * 4;
> + int off;
> +
> + if (flags & FS_NO_HOLES)
> + return 1;
> +
> + /* HOLE - DATA - HOLE - DATA */
> + /* Each unit is bufsz */
> +
> + buf = do_malloc(bufsz);
> + if (!buf)
> + goto out;
> + memset(buf, 'a', bufsz);
> +
> + ret = do_pwrite(fd, buf, bufsz, bufsz);
> + if (!ret)
> + do_pwrite(fd, buf, bufsz, bufsz * 3);
> + if (ret)
> + goto out;
> +
> + /* offset at the beginning */
> + ret += do_lseek(testnum, 1, fd, SEEK_HOLE, 0, 0);
> + ret += do_lseek(testnum, 2, fd, SEEK_HOLE, 1, 1);
> + ret += do_lseek(testnum, 3, fd, SEEK_DATA, 0, bufsz);
> + ret += do_lseek(testnum, 4, fd, SEEK_DATA, 1, bufsz);
> +
> + /* offset around first hole-data boundary */
> + off = bufsz;
> + ret += do_lseek(testnum, 5, fd, SEEK_HOLE, off - 1, off - 1);
> + ret += do_lseek(testnum, 6, fd, SEEK_DATA, off - 1, off);
> + ret += do_lseek(testnum, 7, fd, SEEK_HOLE, off, bufsz * 2);
> + ret += do_lseek(testnum, 8, fd, SEEK_DATA, off, off);
> + ret += do_lseek(testnum, 9, fd, SEEK_HOLE, off + 1, bufsz * 2);
> + ret += do_lseek(testnum, 10, fd, SEEK_DATA, off + 1, off + 1);
> +
> + /* offset around data-hole boundary */
> + off = bufsz * 2;
> + ret += do_lseek(testnum, 11, fd, SEEK_HOLE, off - 1, off);
> + ret += do_lseek(testnum, 12, fd, SEEK_DATA, off - 1, off - 1);
> + ret += do_lseek(testnum, 13, fd, SEEK_HOLE, off, off);
> + ret += do_lseek(testnum, 14, fd, SEEK_DATA, off, bufsz * 3);
> + ret += do_lseek(testnum, 15, fd, SEEK_HOLE, off + 1, off + 1);
> + ret += do_lseek(testnum, 16, fd, SEEK_DATA, off + 1, bufsz * 3);
> +
> + /* offset around second hole-data boundary */
> + off = bufsz * 3;
> + ret += do_lseek(testnum, 17, fd, SEEK_HOLE, off - 1, off - 1);
> + ret += do_lseek(testnum, 18, fd, SEEK_DATA, off - 1, off);
> + ret += do_lseek(testnum, 19, fd, SEEK_HOLE, off, filsz);
> + ret += do_lseek(testnum, 20, fd, SEEK_DATA, off, off);
> + ret += do_lseek(testnum, 21, fd, SEEK_HOLE, off + 1, filsz);
> + ret += do_lseek(testnum, 22, fd, SEEK_DATA, off + 1, off + 1);
> +
> + /* offset around the end of file */
> + off = filsz;
> + ret += do_lseek(testnum, 23, fd, SEEK_HOLE, off - 1, filsz);
> + ret += do_lseek(testnum, 24, fd, SEEK_DATA, off - 1, filsz - 1);
> + ret += do_lseek(testnum, 25, fd, SEEK_HOLE, off, -1);
> + ret += do_lseek(testnum, 26, fd, SEEK_DATA, off, -1);
> + ret += do_lseek(testnum, 27, fd, SEEK_HOLE, off + 1, -1);
> + ret += do_lseek(testnum, 28, fd, SEEK_DATA, off + 1, -1);
> +
> +out:
> + do_free(buf);
> + return ret;
> +}
> +
> +/* test file with data at the beginning and a hole at the end */
> +static int test05(int fd, int testnum)
> +{
> + int ret = -1;
> + char *buf = NULL;
> + int bufsz = alloc_size;
> + int filsz = bufsz * 4;
> +
> + if (flags & FS_NO_HOLES)
> + return 1;
> +
> + /* DATA - HOLE */
> + /* Each unit is bufsz */
> +
> + buf = do_malloc(bufsz);
> + if (!buf)
> + goto out;
> + memset(buf, 'a', bufsz);
> +
> + ret = do_truncate(fd, filsz);
> + if (!ret)
> + ret = do_pwrite(fd, buf, bufsz, 0);
> + if (ret)
> + goto out;
> +
> + /* offset at the beginning */
> + ret += do_lseek(testnum, 1, fd, SEEK_HOLE, 0, bufsz);
> + ret += do_lseek(testnum, 2, fd, SEEK_HOLE, 1, bufsz);
> + ret += do_lseek(testnum, 3, fd, SEEK_DATA, 0, 0);
> + ret += do_lseek(testnum, 4, fd, SEEK_DATA, 1, 1);
> +
> + /* offset around data-hole boundary */
> + ret += do_lseek(testnum, 5, fd, SEEK_HOLE, bufsz - 1, bufsz);
> + ret += do_lseek(testnum, 6, fd, SEEK_DATA, bufsz - 1, bufsz - 1);
> + ret += do_lseek(testnum, 7, fd, SEEK_HOLE, bufsz, bufsz);
> + ret += do_lseek(testnum, 8, fd, SEEK_DATA, bufsz, -1);
> + ret += do_lseek(testnum, 9, fd, SEEK_HOLE, bufsz + 1, bufsz + 1);
> + ret += do_lseek(testnum, 10, fd, SEEK_DATA, bufsz + 1, -1);
> +
> + /* offset around eof */
> + ret += do_lseek(testnum, 11, fd, SEEK_HOLE, filsz - 1, filsz - 1);
> + ret += do_lseek(testnum, 12, fd, SEEK_DATA, filsz - 1, -1);
> + ret += do_lseek(testnum, 13, fd, SEEK_HOLE, filsz, -1);
> + ret += do_lseek(testnum, 14, fd, SEEK_DATA, filsz, -1);
> + ret += do_lseek(testnum, 15, fd, SEEK_HOLE, filsz + 1, -1);
> + ret += do_lseek(testnum, 16, fd, SEEK_DATA, filsz + 1, -1);
> +
> +out:
> + do_free(buf);
> + return ret;
> +}
> +
> +/* test hole begin and data end */
> +static int test04(int fd, int testnum)
> +{
> + int ret;
> + char *buf = "ABCDEFGH";
> + int bufsz = sizeof(buf);
> + int holsz = alloc_size * 2;
> + int filsz = holsz + bufsz;
> +
> + if (flags & FS_NO_HOLES)
> + return 1;
> +
> + /* HOLE - DATA */
> +
> + ret = do_pwrite(fd, buf, bufsz, holsz);
> + if (ret)
> + goto out;
> +
> + /* offset at the beginning */
> + ret += do_lseek(testnum, 1, fd, SEEK_HOLE, 0, 0);
> + ret += do_lseek(testnum, 2, fd, SEEK_HOLE, 1, 1);
> + ret += do_lseek(testnum, 3, fd, SEEK_DATA, 0, holsz);
> + ret += do_lseek(testnum, 4, fd, SEEK_DATA, 1, holsz);
> +
> + /* offset around hole-data boundary */
> + ret += do_lseek(testnum, 5, fd, SEEK_HOLE, holsz - 1, holsz - 1);
> + ret += do_lseek(testnum, 6, fd, SEEK_DATA, holsz - 1, holsz);
> + ret += do_lseek(testnum, 7, fd, SEEK_HOLE, holsz, filsz);
> + ret += do_lseek(testnum, 8, fd, SEEK_DATA, holsz, holsz);
> + ret += do_lseek(testnum, 9, fd, SEEK_HOLE, holsz + 1, filsz);
> + ret += do_lseek(testnum, 10, fd, SEEK_DATA, holsz + 1, holsz + 1);
> +
> + /* offset around eof */
> + ret += do_lseek(testnum, 11, fd, SEEK_HOLE, filsz - 1, filsz);
> + ret += do_lseek(testnum, 12, fd, SEEK_DATA, filsz - 1, filsz - 1);
> + ret += do_lseek(testnum, 13, fd, SEEK_HOLE, filsz, -1);
> + ret += do_lseek(testnum, 14, fd, SEEK_DATA, filsz, -1);
> + ret += do_lseek(testnum, 15, fd, SEEK_HOLE, filsz + 1, -1);
> + ret += do_lseek(testnum, 16, fd, SEEK_DATA, filsz + 1, -1);
> +out:
> + return ret;
> +}
> +
> +/* test full file */
> +static int test03(int fd, int testnum)
> +{
> + char *buf = NULL;
> + int bufsz = alloc_size + 100;
> + int ret = -1;
> +
> + buf = do_malloc(bufsz);
> + if (!buf)
> + goto out;
> + memset(buf, 'a', bufsz);
> +
> + ret = do_pwrite(fd, buf, bufsz, 0);
> + if (ret)
> + goto out;
> +
> + /* offset at the beginning */
> + ret += do_lseek(testnum, 1, fd, SEEK_HOLE, 0, bufsz);
> + ret += do_lseek(testnum, 2, fd, SEEK_HOLE, 1, bufsz);
> + ret += do_lseek(testnum, 3, fd, SEEK_DATA, 0, 0);
> + ret += do_lseek(testnum, 4, fd, SEEK_DATA, 1, 1);
> +
> + /* offset around eof */
> + ret += do_lseek(testnum, 5, fd, SEEK_HOLE, bufsz - 1, bufsz);
> + ret += do_lseek(testnum, 6, fd, SEEK_DATA, bufsz - 1, bufsz - 1);
> + ret += do_lseek(testnum, 7, fd, SEEK_HOLE, bufsz, -1);
> + ret += do_lseek(testnum, 8, fd, SEEK_DATA, bufsz, -1);
> + ret += do_lseek(testnum, 9, fd, SEEK_HOLE, bufsz + 1, -1);
> + ret += do_lseek(testnum, 10, fd, SEEK_DATA, bufsz + 1, -1);
> +
> +out:
> + do_free(buf);
> + return ret;
> +}
> +
> +/* test empty file */
> +static int test02(int fd, int testnum)
> +{
> + int ret = 0;
> +
> + ret += do_lseek(testnum, 1, fd, SEEK_DATA, 0, -1);
> + ret += do_lseek(testnum, 2, fd, SEEK_HOLE, 0, -1);
> + ret += do_lseek(testnum, 3, fd, SEEK_HOLE, 1, -1);
> +
> + return ret;
> +}
> +
> +/* test feature support */
> +static int test01(int fd, int testnum)
> +{
> + int ret;
> + char buf[] = "ABCDEFGH";
> + int bufsz = sizeof(buf);
> +
> + ret = do_pwrite(fd, buf, bufsz, 0);
> + if (ret)
> + goto out;
> +
> + ret += do_lseek(testnum, 1, fd, SEEK_DATA, 0, 0);
> + ret += do_lseek(testnum, 2, fd, SEEK_HOLE, 0, bufsz);
> +
> +out:
> + return ret;
> +}
> +
> +struct testrec {
> + int test_num;
> + int (*test_func)(int fd, int testnum);
> + char *test_desc;
> +};
> +
> +struct testrec seek_tests[] = {
> + { 1, test01, "Test basic support" },
> + { 2, test02, "Test an empty file" },
> + { 3, test03, "Test a full file" },
> + { 4, test04, "Test file hole at beg, data at end" },
> + { 5, test05, "Test file data at beg, hole at end" },
> + { 6, test06, "Test file hole data hole data" },
> +};
> +
> +static int run_test(int fd, struct testrec *tr)
> +{
> + int ret;
> +
> + ret = tr->test_func(fd, tr->test_num);
> + if (!(flags & QUIET))
> + printf("%02d. %-50s\t%s\n", tr->test_num, tr->test_desc,
> + ret < 0 ? "FAIL" : (ret == 0 ? "SUCC" : "NOT RUN"));
> + return ret;
> +}
> +
> +void print_help()
> +{
> + printf("seek-test [-h] [-q] filename\n");
> + printf("\t-h - this message\n");
> + printf("\t-q - quiet, no output\n");
> + printf("\tfilename - file to use for the test\n");
> +}
> +
> +int main(int argc, char **argv)
> +{
> + int ret = -1;
> + int i, fd = -1;
> + int c;
> + int numtests = sizeof(seek_tests) / sizeof(struct testrec);
> +
> + while ((c = getopt(argc, argv, "qh")) != -1) {
> + switch (c) {
> + case 'q':
> + flags |= QUIET;
> + break;
> + case 'h':
> + print_help();
> + exit(0);
> + default:
> + print_help();
> + exit(1);
> + }
> + }
> +
> + if (optind >= argc) {
> + print_help();
> + exit(1);
> + }
> +
> + fd = open(argv[optind], O_RDWR|O_CREAT|O_TRUNC, 0644);
> + if (fd < 0) {
> + fprintf(stderr, "Failed to open testfile: %d\n", errno);
> + goto out;
> + }
> +
> + ret = get_io_sizes(fd);
> + if (ret)
> + goto out;
> +
> + ret = get_flags(fd);
> + if (ret)
> + goto out;
> +
> + for (i = 0; i < numtests; ++i) {
> + ret = do_truncate(fd, 0);
> + if (ret)
> + goto out;
> + run_test(fd, &seek_tests[i]);
> + }
> +
> +out:
> + if (fd > -1)
> + close(fd);
> + return ret;
> +}
> --
> 1.7.5.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Cheers, Andreas
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH] xfstests 255: add a seek_data/seek_hole tester
2011-06-27 18:32 ` Andreas Dilger
@ 2011-06-27 18:47 ` Josef Bacik
0 siblings, 0 replies; 37+ messages in thread
From: Josef Bacik @ 2011-06-27 18:47 UTC (permalink / raw)
To: Andreas Dilger; +Cc: linux-fsdevel, linux-btrfs, linux-kernel, viro
On 06/27/2011 02:32 PM, Andreas Dilger wrote:
> On 2011-06-27, at 12:02 PM, Josef Bacik wrote:
<snip>
>> +
>> +#define SEEK_DATA 3
>> +#define SEEK_HOLE 4
>
> These should probably be "#ifndef SEEK_DATA" so that gcc doesn't complain
> in the future when these are added to a standard header.
>
Good point, I will fix that, thanks.
<snip>
>> +
>> + pos = lseek(fd, 0, SEEK_HOLE);
>> + if (pos == alloc_size * 2) {
>> + if (!(flags & QUIET))
>> + printf("File system does not recognize holes, the only "
>> + "hole found will be at the end.\n");
>> + flags |= FS_NO_HOLES;
>
> This is a question that I've also had about compatibility with older
> (well, every) Linux kernel that does not support SEEK_{HOLE,DATA}
> today.
>
If you look at the xfstest part of it, I grep for a complaint about not
support SEEK_HOLE and I just say _notrun. I ran this test on an old
kernel and it just skipped the test.
> My reading of the existing generic_file_llseek() and default_llseek()
> code, along with most filesystem-specific llseek() implementations is
> that they will happily ignore the @whence parameter if it is not
> known, and pretend like it is 0 (SEEK_SET), so they will just set the
> position to the @offset parameter and return this value. In that
> case, the above "SEEK_HOLE" test would incorrectly fail on every
> Linux kernel in existence today because the returned pos == 0.
>
First, older kernels will check for whence > SEEK_MAX and automatically
return -EINVAL, so we are ok there.
Second, I looked at everybody in fs/ and changed anybody that did what
you suggest. Anybody that I didn't change will return -EINVAL properly
so I didn't touch them. I also looked at the drivers that didn't use
default_llseek/seq_lseek or whatever and they all seem to handle things
properly, though I'm sure I missed somebody.
> Should applications call both SEEK_HOLE and SEEK_DATA with @offset=0,
> and if they return the same values (which is normally impossible,
> decide that the kernel does not support this SEEK_* functionality?
Yeah if you want to be super careful. I mean for all file systems we
should be ok with my patches, but if you hit some weird proc file that
has it's llseek thing tied to something specific in the driver you may
run into trouble, and by trouble you will just get weird return's for
your seek. Thanks,
Josef
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 1/4] fs: add SEEK_HOLE and SEEK_DATA flags
2011-06-27 18:02 [PATCH 1/4] fs: add SEEK_HOLE and SEEK_DATA flags Josef Bacik
` (3 preceding siblings ...)
2011-06-27 18:02 ` [PATCH] xfstests 255: add a seek_data/seek_hole tester Josef Bacik
@ 2011-06-27 21:04 ` Josef Bacik
4 siblings, 0 replies; 37+ messages in thread
From: Josef Bacik @ 2011-06-27 21:04 UTC (permalink / raw)
To: linux-fsdevel, linux-btrfs, linux-kernel, viro
On 06/27/2011 02:02 PM, Josef Bacik wrote:
> This just gets us ready to support the SEEK_HOLE and SEEK_DATA flags. Turns out
> using fiemap in things like cp cause more problems than it solves, so lets try
> and give userspace an interface that doesn't suck. We need to match solaris
> here, and the definitions are
>
> *o* If /whence/ is SEEK_HOLE, the offset of the start of the
> next hole greater than or equal to the supplied offset
> is returned. The definition of a hole is provided near
> the end of the DESCRIPTION.
>
> *o* If /whence/ is SEEK_DATA, the file pointer is set to the
> start of the next non-hole file region greater than or
> equal to the supplied offset.
>
> So in the generic case the entire file is data and there is a virtual hole at
> the end. That means we will just return i_size for SEEK_HOLE and will return
> the same offset for SEEK_DATA. This is how Solaris does it so we have to do it
> the same way.
>
> Thanks,
>
> Signed-off-by: Josef Bacik <josef@redhat.com>
> ---
> fs/read_write.c | 37 +++++++++++++++++++++++++++++++++++++
> include/linux/fs.h | 4 +++-
> 2 files changed, 40 insertions(+), 1 deletions(-)
>
> diff --git a/fs/read_write.c b/fs/read_write.c
> index 5520f8a..d58551e 100644
> --- a/fs/read_write.c
> +++ b/fs/read_write.c
> @@ -64,6 +64,23 @@ generic_file_llseek_unlocked(struct file *file, loff_t offset, int origin)
> return file->f_pos;
> offset += file->f_pos;
> break;
> + case SEEK_DATA:
> + /*
> + * In the generic case the entire file is data, so as long as
> + * offset isn't at the end of the file then the offset is data.
> + */
> + if (offset >= inode->i_size)
> + return -ENXIO;
> + break;
> + case SEEK_HOLE:
> + /*
> + * There is a virtual hole at the end of the file, so as long as
> + * offset isn't i_size or larger, return i_size.
> + */
> + if (offset >= inode->i_size)
> + return -ENXIO;
> + offset = inode->i_size;
> + break;
> }
>
> if (offset < 0 && !unsigned_offsets(file))
> @@ -141,6 +158,26 @@ loff_t default_llseek(struct file *file, loff_t offset, int origin)
> goto out;
> }
> offset += file->f_pos;
> + break;
> + case SEEK_DATA:
> + /*
> + * In the generic case the entire file is data, so as
> + * long as offset isn't at the end of the file then the
> + * offset is data.
> + */
> + if (offset >= inode->i_size)
> + return -ENXIO;
> + break;
> + case SEEK_HOLE:
> + /*
> + * There is a virtual hole at the end of the file, so
> + * as long as offset isn't i_size or larger, return
> + * i_size.
> + */
> + if (offset >= inode->i_size)
> + return -ENXIO;
> + offset = inode->i_size;
> + break;
Hrm sorry I just noticed this doesn't compile. I'll fix it up and
resend along with the xfstest update that Andreas pointed out. Thanks,
Josef
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 1/4] fs: add SEEK_HOLE and SEEK_DATA flags
2011-08-22 17:56 ` Marco Stornelli
@ 2011-08-31 0:35 ` Dave Chinner
-1 siblings, 0 replies; 37+ messages in thread
From: Dave Chinner @ 2011-08-31 0:35 UTC (permalink / raw)
To: Marco Stornelli
Cc: Sunil Mushran, Josef Bacik, linux-fsdevel, linux-kernel,
linux-btrfs, xfs, viro
On Mon, Aug 22, 2011 at 07:56:31PM +0200, Marco Stornelli wrote:
> Il 22/08/2011 17:57, Sunil Mushran ha scritto:
> >>>Any proposal that differentiates between holes is wrong. It should not
> >>>matter where the hole is.
> >>>
> >>>Think of it from the usage-pov.
> >>>
> >>>doff = 0;
> >>>while ((doff = lseek(SEEK_DATA, doff)) != -ENXIO) {
> >>>hoff = lseek(SEEK_HOLE, doff);
> >>>read_offset = doff;
> >>>read_len = hoff -doff;
> >>>process();
> >>>doff = hoff;
> >>>}
> >>>
> >>>The goal is to make this as efficient as follows. Treating the last
> >>>hole differently adds more code for no benefit.
> >>>
> >>Mmmm.....It seems that Josef has to be clear in this point. However I
> >>looked for the seek hole test in xfs test suite, but I didn't find
> >>anything. Btrfs guys, how have you got tested the implementation? What
> >>do you think about this corner case? Al, what do you think about it?
> >
> >
> >The following test was used to test the early implementations.
> >http://oss.oracle.com/~smushran/seek_data/
> >
>
> Thank you very much!! I found another point. Your test fails with my
> implementation because here
> (http://www.austingroupbugs.net/view.php?id=415) says: "If whence is
> SEEK_DATA, the file offset shall be set to the smallest location of
> a byte not within a hole and not less than offset. It shall be an
> error if no such byte exists." So in this case I return ENXIO but
> the test expects another value. I have to say that there is a bit of
> confusion about the real behavior of this new feature :)
Which is exactly why I'm trying to get the definitions clarified
first, then the behaviour codified in a single test suite we can
call the 'authoritive test'.
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 1/4] fs: add SEEK_HOLE and SEEK_DATA flags
@ 2011-08-31 0:35 ` Dave Chinner
0 siblings, 0 replies; 37+ messages in thread
From: Dave Chinner @ 2011-08-31 0:35 UTC (permalink / raw)
To: Marco Stornelli
Cc: Sunil Mushran, linux-kernel, xfs, Josef Bacik, linux-fsdevel,
linux-btrfs, viro
On Mon, Aug 22, 2011 at 07:56:31PM +0200, Marco Stornelli wrote:
> Il 22/08/2011 17:57, Sunil Mushran ha scritto:
> >>>Any proposal that differentiates between holes is wrong. It should not
> >>>matter where the hole is.
> >>>
> >>>Think of it from the usage-pov.
> >>>
> >>>doff = 0;
> >>>while ((doff = lseek(SEEK_DATA, doff)) != -ENXIO) {
> >>>hoff = lseek(SEEK_HOLE, doff);
> >>>read_offset = doff;
> >>>read_len = hoff -doff;
> >>>process();
> >>>doff = hoff;
> >>>}
> >>>
> >>>The goal is to make this as efficient as follows. Treating the last
> >>>hole differently adds more code for no benefit.
> >>>
> >>Mmmm.....It seems that Josef has to be clear in this point. However I
> >>looked for the seek hole test in xfs test suite, but I didn't find
> >>anything. Btrfs guys, how have you got tested the implementation? What
> >>do you think about this corner case? Al, what do you think about it?
> >
> >
> >The following test was used to test the early implementations.
> >http://oss.oracle.com/~smushran/seek_data/
> >
>
> Thank you very much!! I found another point. Your test fails with my
> implementation because here
> (http://www.austingroupbugs.net/view.php?id=415) says: "If whence is
> SEEK_DATA, the file offset shall be set to the smallest location of
> a byte not within a hole and not less than offset. It shall be an
> error if no such byte exists." So in this case I return ENXIO but
> the test expects another value. I have to say that there is a bit of
> confusion about the real behavior of this new feature :)
Which is exactly why I'm trying to get the definitions clarified
first, then the behaviour codified in a single test suite we can
call the 'authoritive test'.
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 1/4] fs: add SEEK_HOLE and SEEK_DATA flags
2011-08-22 21:22 ` Sunil Mushran
@ 2011-08-23 17:44 ` Marco Stornelli
-1 siblings, 0 replies; 37+ messages in thread
From: Marco Stornelli @ 2011-08-23 17:44 UTC (permalink / raw)
To: Sunil Mushran
Cc: Josef Bacik, linux-fsdevel, linux-kernel, linux-btrfs, xfs, viro
Il 22/08/2011 23:22, Sunil Mushran ha scritto:
> On 08/22/2011 10:56 AM, Marco Stornelli wrote:
>> Il 22/08/2011 17:57, Sunil Mushran ha scritto:
>>>
>>> The following test was used to test the early implementations.
>>> http://oss.oracle.com/~smushran/seek_data/
>>>
>>
>> Thank you very much!! I found another point. Your test fails with my
>> implementation because here
>> (http://www.austingroupbugs.net/view.php?id=415) says: "If whence is
>> SEEK_DATA, the file offset shall be set to the smallest location of a
>> byte not within a hole and not less than offset. It shall be an error
>> if no such byte exists." So in this case I return ENXIO but the test
>> expects another value. I have to say that there is a bit of confusion
>> about the real behavior of this new feature :)
>>
>
> That's test 5.10, 5.12, 5.14. And it expects -ENXIO.
>
> Which test is failing for you?
>
Sorry, I was reading the results in a wrong way.
Marco
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 1/4] fs: add SEEK_HOLE and SEEK_DATA flags
@ 2011-08-23 17:44 ` Marco Stornelli
0 siblings, 0 replies; 37+ messages in thread
From: Marco Stornelli @ 2011-08-23 17:44 UTC (permalink / raw)
To: Sunil Mushran
Cc: linux-kernel, Josef Bacik, xfs, viro, linux-fsdevel, linux-btrfs
Il 22/08/2011 23:22, Sunil Mushran ha scritto:
> On 08/22/2011 10:56 AM, Marco Stornelli wrote:
>> Il 22/08/2011 17:57, Sunil Mushran ha scritto:
>>>
>>> The following test was used to test the early implementations.
>>> http://oss.oracle.com/~smushran/seek_data/
>>>
>>
>> Thank you very much!! I found another point. Your test fails with my
>> implementation because here
>> (http://www.austingroupbugs.net/view.php?id=415) says: "If whence is
>> SEEK_DATA, the file offset shall be set to the smallest location of a
>> byte not within a hole and not less than offset. It shall be an error
>> if no such byte exists." So in this case I return ENXIO but the test
>> expects another value. I have to say that there is a bit of confusion
>> about the real behavior of this new feature :)
>>
>
> That's test 5.10, 5.12, 5.14. And it expects -ENXIO.
>
> Which test is failing for you?
>
Sorry, I was reading the results in a wrong way.
Marco
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 1/4] fs: add SEEK_HOLE and SEEK_DATA flags
2011-08-22 17:56 ` Marco Stornelli
@ 2011-08-22 21:22 ` Sunil Mushran
-1 siblings, 0 replies; 37+ messages in thread
From: Sunil Mushran @ 2011-08-22 21:22 UTC (permalink / raw)
To: Marco Stornelli
Cc: Josef Bacik, linux-fsdevel, linux-kernel, linux-btrfs, xfs, viro
On 08/22/2011 10:56 AM, Marco Stornelli wrote:
> Il 22/08/2011 17:57, Sunil Mushran ha scritto:
>>
>> The following test was used to test the early implementations.
>> http://oss.oracle.com/~smushran/seek_data/
>>
>
> Thank you very much!! I found another point. Your test fails with my
> implementation because here
> (http://www.austingroupbugs.net/view.php?id=415) says: "If whence is
> SEEK_DATA, the file offset shall be set to the smallest location of a
> byte not within a hole and not less than offset. It shall be an error
> if no such byte exists." So in this case I return ENXIO but the test
> expects another value. I have to say that there is a bit of confusion
> about the real behavior of this new feature :)
>
That's test 5.10, 5.12, 5.14. And it expects -ENXIO.
Which test is failing for you?
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 1/4] fs: add SEEK_HOLE and SEEK_DATA flags
@ 2011-08-22 21:22 ` Sunil Mushran
0 siblings, 0 replies; 37+ messages in thread
From: Sunil Mushran @ 2011-08-22 21:22 UTC (permalink / raw)
To: Marco Stornelli
Cc: linux-kernel, Josef Bacik, xfs, viro, linux-fsdevel, linux-btrfs
On 08/22/2011 10:56 AM, Marco Stornelli wrote:
> Il 22/08/2011 17:57, Sunil Mushran ha scritto:
>>
>> The following test was used to test the early implementations.
>> http://oss.oracle.com/~smushran/seek_data/
>>
>
> Thank you very much!! I found another point. Your test fails with my
> implementation because here
> (http://www.austingroupbugs.net/view.php?id=415) says: "If whence is
> SEEK_DATA, the file offset shall be set to the smallest location of a
> byte not within a hole and not less than offset. It shall be an error
> if no such byte exists." So in this case I return ENXIO but the test
> expects another value. I have to say that there is a bit of confusion
> about the real behavior of this new feature :)
>
That's test 5.10, 5.12, 5.14. And it expects -ENXIO.
Which test is failing for you?
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 1/4] fs: add SEEK_HOLE and SEEK_DATA flags
2011-08-22 15:57 ` Sunil Mushran
@ 2011-08-22 17:56 ` Marco Stornelli
-1 siblings, 0 replies; 37+ messages in thread
From: Marco Stornelli @ 2011-08-22 17:56 UTC (permalink / raw)
To: Sunil Mushran
Cc: Josef Bacik, linux-fsdevel, linux-kernel, linux-btrfs, xfs, viro
Il 22/08/2011 17:57, Sunil Mushran ha scritto:
>>> Any proposal that differentiates between holes is wrong. It should not
>>> matter where the hole is.
>>>
>>> Think of it from the usage-pov.
>>>
>>> doff = 0;
>>> while ((doff = lseek(SEEK_DATA, doff)) != -ENXIO) {
>>> hoff = lseek(SEEK_HOLE, doff);
>>> read_offset = doff;
>>> read_len = hoff -doff;
>>> process();
>>> doff = hoff;
>>> }
>>>
>>> The goal is to make this as efficient as follows. Treating the last
>>> hole differently adds more code for no benefit.
>>>
>> Mmmm.....It seems that Josef has to be clear in this point. However I
>> looked for the seek hole test in xfs test suite, but I didn't find
>> anything. Btrfs guys, how have you got tested the implementation? What
>> do you think about this corner case? Al, what do you think about it?
>
>
> The following test was used to test the early implementations.
> http://oss.oracle.com/~smushran/seek_data/
>
Thank you very much!! I found another point. Your test fails with my
implementation because here
(http://www.austingroupbugs.net/view.php?id=415) says: "If whence is
SEEK_DATA, the file offset shall be set to the smallest location of a
byte not within a hole and not less than offset. It shall be an error if
no such byte exists." So in this case I return ENXIO but the test
expects another value. I have to say that there is a bit of confusion
about the real behavior of this new feature :)
Marco
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 1/4] fs: add SEEK_HOLE and SEEK_DATA flags
@ 2011-08-22 17:56 ` Marco Stornelli
0 siblings, 0 replies; 37+ messages in thread
From: Marco Stornelli @ 2011-08-22 17:56 UTC (permalink / raw)
To: Sunil Mushran
Cc: linux-kernel, Josef Bacik, xfs, viro, linux-fsdevel, linux-btrfs
Il 22/08/2011 17:57, Sunil Mushran ha scritto:
>>> Any proposal that differentiates between holes is wrong. It should not
>>> matter where the hole is.
>>>
>>> Think of it from the usage-pov.
>>>
>>> doff = 0;
>>> while ((doff = lseek(SEEK_DATA, doff)) != -ENXIO) {
>>> hoff = lseek(SEEK_HOLE, doff);
>>> read_offset = doff;
>>> read_len = hoff -doff;
>>> process();
>>> doff = hoff;
>>> }
>>>
>>> The goal is to make this as efficient as follows. Treating the last
>>> hole differently adds more code for no benefit.
>>>
>> Mmmm.....It seems that Josef has to be clear in this point. However I
>> looked for the seek hole test in xfs test suite, but I didn't find
>> anything. Btrfs guys, how have you got tested the implementation? What
>> do you think about this corner case? Al, what do you think about it?
>
>
> The following test was used to test the early implementations.
> http://oss.oracle.com/~smushran/seek_data/
>
Thank you very much!! I found another point. Your test fails with my
implementation because here
(http://www.austingroupbugs.net/view.php?id=415) says: "If whence is
SEEK_DATA, the file offset shall be set to the smallest location of a
byte not within a hole and not less than offset. It shall be an error if
no such byte exists." So in this case I return ENXIO but the test
expects another value. I have to say that there is a bit of confusion
about the real behavior of this new feature :)
Marco
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 1/4] fs: add SEEK_HOLE and SEEK_DATA flags
2011-08-22 10:56 ` Marco Stornelli
@ 2011-08-22 15:57 ` Sunil Mushran
-1 siblings, 0 replies; 37+ messages in thread
From: Sunil Mushran @ 2011-08-22 15:57 UTC (permalink / raw)
To: Marco Stornelli
Cc: Josef Bacik, linux-fsdevel, linux-kernel, linux-btrfs, xfs, viro
On 08/22/2011 03:56 AM, Marco Stornelli wrote:
> 2011/8/22 Sunil Mushran<sunil.mushran@oracle.com>:
>> On 08/20/2011 09:32 AM, Marco Stornelli wrote:
>>> Thank. Yes the word "next" is not very clear. I re-read the proposal for
>>> the standard, actually it's seems to me that if we are in the last hole we
>>> should return the file size, if we are not in the last hole than it's ok the
>>> same offset - "....except that
>>> if offset falls beyond the last byte not within a hole, then the file
>>> offset may be set to the file size instead".
>> Any proposal that differentiates between holes is wrong. It should not
>> matter where the hole is.
>>
>> Think of it from the usage-pov.
>>
>> doff = 0;
>> while ((doff = lseek(SEEK_DATA, doff)) != -ENXIO) {
>> hoff = lseek(SEEK_HOLE, doff);
>> read_offset = doff;
>> read_len = hoff -doff;
>> process();
>> doff = hoff;
>> }
>>
>> The goal is to make this as efficient as follows. Treating the last
>> hole differently adds more code for no benefit.
>>
> Mmmm.....It seems that Josef has to be clear in this point. However I
> looked for the seek hole test in xfs test suite, but I didn't find
> anything. Btrfs guys, how have you got tested the implementation? What
> do you think about this corner case? Al, what do you think about it?
The following test was used to test the early implementations.
http://oss.oracle.com/~smushran/seek_data/
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 1/4] fs: add SEEK_HOLE and SEEK_DATA flags
@ 2011-08-22 15:57 ` Sunil Mushran
0 siblings, 0 replies; 37+ messages in thread
From: Sunil Mushran @ 2011-08-22 15:57 UTC (permalink / raw)
To: Marco Stornelli
Cc: linux-kernel, Josef Bacik, xfs, viro, linux-fsdevel, linux-btrfs
On 08/22/2011 03:56 AM, Marco Stornelli wrote:
> 2011/8/22 Sunil Mushran<sunil.mushran@oracle.com>:
>> On 08/20/2011 09:32 AM, Marco Stornelli wrote:
>>> Thank. Yes the word "next" is not very clear. I re-read the proposal for
>>> the standard, actually it's seems to me that if we are in the last hole we
>>> should return the file size, if we are not in the last hole than it's ok the
>>> same offset - "....except that
>>> if offset falls beyond the last byte not within a hole, then the file
>>> offset may be set to the file size instead".
>> Any proposal that differentiates between holes is wrong. It should not
>> matter where the hole is.
>>
>> Think of it from the usage-pov.
>>
>> doff = 0;
>> while ((doff = lseek(SEEK_DATA, doff)) != -ENXIO) {
>> hoff = lseek(SEEK_HOLE, doff);
>> read_offset = doff;
>> read_len = hoff -doff;
>> process();
>> doff = hoff;
>> }
>>
>> The goal is to make this as efficient as follows. Treating the last
>> hole differently adds more code for no benefit.
>>
> Mmmm.....It seems that Josef has to be clear in this point. However I
> looked for the seek hole test in xfs test suite, but I didn't find
> anything. Btrfs guys, how have you got tested the implementation? What
> do you think about this corner case? Al, what do you think about it?
The following test was used to test the early implementations.
http://oss.oracle.com/~smushran/seek_data/
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 1/4] fs: add SEEK_HOLE and SEEK_DATA flags
2011-08-22 6:08 ` Sunil Mushran
(?)
(?)
@ 2011-08-22 10:56 ` Marco Stornelli
-1 siblings, 0 replies; 37+ messages in thread
From: Marco Stornelli @ 2011-08-22 10:56 UTC (permalink / raw)
To: Sunil Mushran
Cc: Josef Bacik, linux-fsdevel, linux-kernel, linux-btrfs, xfs, viro
2011/8/22 Sunil Mushran <sunil.mushran@oracle.com>:
> On 08/20/2011 09:32 AM, Marco Stornelli wrote:
>>
>> Il 20/08/2011 17:36, Sunil Mushran ha scritto:
>>>
>>> On 08/20/2011 03:03 AM, Marco Stornelli wrote:
>>>>
>>>> Il 20/08/2011 11:41, Marco Stornelli ha scritto:
>>>>>
>>>>> Hi,
>>>>>
>>>>> Il 28/06/2011 17:33, Josef Bacik ha scritto:
>>>>>>
>>>>>> This just gets us ready to support the SEEK_HOLE and SEEK_DATA f=
lags.
>>>>>> Turns out
>>>>>> using fiemap in things like cp cause more problems than it solve=
s, so
>>>>>> lets try
>>>>>> and give userspace an interface that doesn't suck. We need to ma=
tch
>>>>>> solaris
>>>>>> here, and the definitions are
>>>>>>
>>>>>> *o* If /whence/ is SEEK_HOLE, the offset of the start of the
>>>>>> next hole greater than or equal to the supplied offset
>>>>>> is returned. The definition of a hole is provided near
>>>>>> the end of the DESCRIPTION.
>>>>>>
>>>>>> *o* If /whence/ is SEEK_DATA, the file pointer is set to the
>>>>>> start of the next non-hole file region greater than or
>>>>>> equal to the supplied offset.
>>>>>>
>>>>>
>>>>> I'm implementing the SEEK_DATA/SEEK_HOLE management for pramfs an=
d I've
>>>>> got some doubts about the right behavior:
>>>>>
>>>>> 1) when we use SEEK_DATA/SEEK_HOLE, the offset used in lseek mean=
s
>>>>> always the offset from the start of the file, right?
>>>>>
>>>>> 2) in case of a file with hole at the beginning and data at the e=
nd, if
>>>>> I do lseek(fd, 0, SEEK_HOLE) I should receive the end of the file
>>>>> because the idea is to search the *next* hole and we have always =
a
>>>>> virtual hole at the end of the file, right?
>>>>
>>>> Just to be precise about this question: the alternative here, it's=
to
>>>> return the same position because we are already in a hole.
>>>
>>> Yes, the offset is from the start of the file.
>>>
>>> And yes, same offset is ok. I think the word next should be
>>> dropped from the definition. It is misleading.
>>>
>>
>> Thank. Yes the word "next" is not very clear. I re-read the proposal=
for
>> the standard, actually it's seems to me that if we are in the last h=
ole we
>> should return the file size, if we are not in the last hole than it'=
s ok the
>> same offset - "....except that
>> if offset falls beyond the last byte not within a hole, then the fil=
e
>> offset may be set to the file size instead".
>
> Any proposal that differentiates between holes is wrong. It should no=
t
> matter where the hole is.
>
> Think of it from the usage-pov.
>
> doff =3D 0;
> while ((doff =3D lseek(SEEK_DATA, doff)) !=3D -ENXIO) {
> =A0 =A0hoff =3D lseek(SEEK_HOLE, doff);
> =A0 =A0read_offset =3D doff;
> =A0 =A0read_len =3D hoff -doff;
> =A0 =A0process();
> =A0 =A0doff =3D hoff;
> }
>
> The goal is to make this as efficient as follows. Treating the last
> hole differently adds more code for no benefit.
>
>
Mmmm.....It seems that Josef has to be clear in this point. However I
looked for the seek hole test in xfs test suite, but I didn't find
anything. Btrfs guys, how have you got tested the implementation? What
do you think about this corner case? Al, what do you think about it?
Marco
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" =
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 1/4] fs: add SEEK_HOLE and SEEK_DATA flags
@ 2011-08-22 10:56 ` Marco Stornelli
0 siblings, 0 replies; 37+ messages in thread
From: Marco Stornelli @ 2011-08-22 10:56 UTC (permalink / raw)
To: Sunil Mushran
Cc: Josef Bacik, linux-fsdevel, linux-kernel, linux-btrfs, xfs, viro
2011/8/22 Sunil Mushran <sunil.mushran@oracle.com>:
> On 08/20/2011 09:32 AM, Marco Stornelli wrote:
>>
>> Il 20/08/2011 17:36, Sunil Mushran ha scritto:
>>>
>>> On 08/20/2011 03:03 AM, Marco Stornelli wrote:
>>>>
>>>> Il 20/08/2011 11:41, Marco Stornelli ha scritto:
>>>>>
>>>>> Hi,
>>>>>
>>>>> Il 28/06/2011 17:33, Josef Bacik ha scritto:
>>>>>>
>>>>>> This just gets us ready to support the SEEK_HOLE and SEEK_DATA flags.
>>>>>> Turns out
>>>>>> using fiemap in things like cp cause more problems than it solves, so
>>>>>> lets try
>>>>>> and give userspace an interface that doesn't suck. We need to match
>>>>>> solaris
>>>>>> here, and the definitions are
>>>>>>
>>>>>> *o* If /whence/ is SEEK_HOLE, the offset of the start of the
>>>>>> next hole greater than or equal to the supplied offset
>>>>>> is returned. The definition of a hole is provided near
>>>>>> the end of the DESCRIPTION.
>>>>>>
>>>>>> *o* If /whence/ is SEEK_DATA, the file pointer is set to the
>>>>>> start of the next non-hole file region greater than or
>>>>>> equal to the supplied offset.
>>>>>>
>>>>>
>>>>> I'm implementing the SEEK_DATA/SEEK_HOLE management for pramfs and I've
>>>>> got some doubts about the right behavior:
>>>>>
>>>>> 1) when we use SEEK_DATA/SEEK_HOLE, the offset used in lseek means
>>>>> always the offset from the start of the file, right?
>>>>>
>>>>> 2) in case of a file with hole at the beginning and data at the end, if
>>>>> I do lseek(fd, 0, SEEK_HOLE) I should receive the end of the file
>>>>> because the idea is to search the *next* hole and we have always a
>>>>> virtual hole at the end of the file, right?
>>>>
>>>> Just to be precise about this question: the alternative here, it's to
>>>> return the same position because we are already in a hole.
>>>
>>> Yes, the offset is from the start of the file.
>>>
>>> And yes, same offset is ok. I think the word next should be
>>> dropped from the definition. It is misleading.
>>>
>>
>> Thank. Yes the word "next" is not very clear. I re-read the proposal for
>> the standard, actually it's seems to me that if we are in the last hole we
>> should return the file size, if we are not in the last hole than it's ok the
>> same offset - "....except that
>> if offset falls beyond the last byte not within a hole, then the file
>> offset may be set to the file size instead".
>
> Any proposal that differentiates between holes is wrong. It should not
> matter where the hole is.
>
> Think of it from the usage-pov.
>
> doff = 0;
> while ((doff = lseek(SEEK_DATA, doff)) != -ENXIO) {
> hoff = lseek(SEEK_HOLE, doff);
> read_offset = doff;
> read_len = hoff -doff;
> process();
> doff = hoff;
> }
>
> The goal is to make this as efficient as follows. Treating the last
> hole differently adds more code for no benefit.
>
>
Mmmm.....It seems that Josef has to be clear in this point. However I
looked for the seek hole test in xfs test suite, but I didn't find
anything. Btrfs guys, how have you got tested the implementation? What
do you think about this corner case? Al, what do you think about it?
Marco
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 1/4] fs: add SEEK_HOLE and SEEK_DATA flags
@ 2011-08-22 10:56 ` Marco Stornelli
0 siblings, 0 replies; 37+ messages in thread
From: Marco Stornelli @ 2011-08-22 10:56 UTC (permalink / raw)
To: Sunil Mushran
Cc: Josef Bacik, linux-fsdevel, linux-kernel, linux-btrfs, xfs, viro
2011/8/22 Sunil Mushran <sunil.mushran@oracle.com>:
> On 08/20/2011 09:32 AM, Marco Stornelli wrote:
>>
>> Il 20/08/2011 17:36, Sunil Mushran ha scritto:
>>>
>>> On 08/20/2011 03:03 AM, Marco Stornelli wrote:
>>>>
>>>> Il 20/08/2011 11:41, Marco Stornelli ha scritto:
>>>>>
>>>>> Hi,
>>>>>
>>>>> Il 28/06/2011 17:33, Josef Bacik ha scritto:
>>>>>>
>>>>>> This just gets us ready to support the SEEK_HOLE and SEEK_DATA flags.
>>>>>> Turns out
>>>>>> using fiemap in things like cp cause more problems than it solves, so
>>>>>> lets try
>>>>>> and give userspace an interface that doesn't suck. We need to match
>>>>>> solaris
>>>>>> here, and the definitions are
>>>>>>
>>>>>> *o* If /whence/ is SEEK_HOLE, the offset of the start of the
>>>>>> next hole greater than or equal to the supplied offset
>>>>>> is returned. The definition of a hole is provided near
>>>>>> the end of the DESCRIPTION.
>>>>>>
>>>>>> *o* If /whence/ is SEEK_DATA, the file pointer is set to the
>>>>>> start of the next non-hole file region greater than or
>>>>>> equal to the supplied offset.
>>>>>>
>>>>>
>>>>> I'm implementing the SEEK_DATA/SEEK_HOLE management for pramfs and I've
>>>>> got some doubts about the right behavior:
>>>>>
>>>>> 1) when we use SEEK_DATA/SEEK_HOLE, the offset used in lseek means
>>>>> always the offset from the start of the file, right?
>>>>>
>>>>> 2) in case of a file with hole at the beginning and data at the end, if
>>>>> I do lseek(fd, 0, SEEK_HOLE) I should receive the end of the file
>>>>> because the idea is to search the *next* hole and we have always a
>>>>> virtual hole at the end of the file, right?
>>>>
>>>> Just to be precise about this question: the alternative here, it's to
>>>> return the same position because we are already in a hole.
>>>
>>> Yes, the offset is from the start of the file.
>>>
>>> And yes, same offset is ok. I think the word next should be
>>> dropped from the definition. It is misleading.
>>>
>>
>> Thank. Yes the word "next" is not very clear. I re-read the proposal for
>> the standard, actually it's seems to me that if we are in the last hole we
>> should return the file size, if we are not in the last hole than it's ok the
>> same offset - "....except that
>> if offset falls beyond the last byte not within a hole, then the file
>> offset may be set to the file size instead".
>
> Any proposal that differentiates between holes is wrong. It should not
> matter where the hole is.
>
> Think of it from the usage-pov.
>
> doff = 0;
> while ((doff = lseek(SEEK_DATA, doff)) != -ENXIO) {
> hoff = lseek(SEEK_HOLE, doff);
> read_offset = doff;
> read_len = hoff -doff;
> process();
> doff = hoff;
> }
>
> The goal is to make this as efficient as follows. Treating the last
> hole differently adds more code for no benefit.
>
>
Mmmm.....It seems that Josef has to be clear in this point. However I
looked for the seek hole test in xfs test suite, but I didn't find
anything. Btrfs guys, how have you got tested the implementation? What
do you think about this corner case? Al, what do you think about it?
Marco
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 1/4] fs: add SEEK_HOLE and SEEK_DATA flags
@ 2011-08-22 10:56 ` Marco Stornelli
0 siblings, 0 replies; 37+ messages in thread
From: Marco Stornelli @ 2011-08-22 10:56 UTC (permalink / raw)
To: Sunil Mushran
Cc: linux-kernel, Josef Bacik, xfs, viro, linux-fsdevel, linux-btrfs
2011/8/22 Sunil Mushran <sunil.mushran@oracle.com>:
> On 08/20/2011 09:32 AM, Marco Stornelli wrote:
>>
>> Il 20/08/2011 17:36, Sunil Mushran ha scritto:
>>>
>>> On 08/20/2011 03:03 AM, Marco Stornelli wrote:
>>>>
>>>> Il 20/08/2011 11:41, Marco Stornelli ha scritto:
>>>>>
>>>>> Hi,
>>>>>
>>>>> Il 28/06/2011 17:33, Josef Bacik ha scritto:
>>>>>>
>>>>>> This just gets us ready to support the SEEK_HOLE and SEEK_DATA flags.
>>>>>> Turns out
>>>>>> using fiemap in things like cp cause more problems than it solves, so
>>>>>> lets try
>>>>>> and give userspace an interface that doesn't suck. We need to match
>>>>>> solaris
>>>>>> here, and the definitions are
>>>>>>
>>>>>> *o* If /whence/ is SEEK_HOLE, the offset of the start of the
>>>>>> next hole greater than or equal to the supplied offset
>>>>>> is returned. The definition of a hole is provided near
>>>>>> the end of the DESCRIPTION.
>>>>>>
>>>>>> *o* If /whence/ is SEEK_DATA, the file pointer is set to the
>>>>>> start of the next non-hole file region greater than or
>>>>>> equal to the supplied offset.
>>>>>>
>>>>>
>>>>> I'm implementing the SEEK_DATA/SEEK_HOLE management for pramfs and I've
>>>>> got some doubts about the right behavior:
>>>>>
>>>>> 1) when we use SEEK_DATA/SEEK_HOLE, the offset used in lseek means
>>>>> always the offset from the start of the file, right?
>>>>>
>>>>> 2) in case of a file with hole at the beginning and data at the end, if
>>>>> I do lseek(fd, 0, SEEK_HOLE) I should receive the end of the file
>>>>> because the idea is to search the *next* hole and we have always a
>>>>> virtual hole at the end of the file, right?
>>>>
>>>> Just to be precise about this question: the alternative here, it's to
>>>> return the same position because we are already in a hole.
>>>
>>> Yes, the offset is from the start of the file.
>>>
>>> And yes, same offset is ok. I think the word next should be
>>> dropped from the definition. It is misleading.
>>>
>>
>> Thank. Yes the word "next" is not very clear. I re-read the proposal for
>> the standard, actually it's seems to me that if we are in the last hole we
>> should return the file size, if we are not in the last hole than it's ok the
>> same offset - "....except that
>> if offset falls beyond the last byte not within a hole, then the file
>> offset may be set to the file size instead".
>
> Any proposal that differentiates between holes is wrong. It should not
> matter where the hole is.
>
> Think of it from the usage-pov.
>
> doff = 0;
> while ((doff = lseek(SEEK_DATA, doff)) != -ENXIO) {
> hoff = lseek(SEEK_HOLE, doff);
> read_offset = doff;
> read_len = hoff -doff;
> process();
> doff = hoff;
> }
>
> The goal is to make this as efficient as follows. Treating the last
> hole differently adds more code for no benefit.
>
>
Mmmm.....It seems that Josef has to be clear in this point. However I
looked for the seek hole test in xfs test suite, but I didn't find
anything. Btrfs guys, how have you got tested the implementation? What
do you think about this corner case? Al, what do you think about it?
Marco
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 1/4] fs: add SEEK_HOLE and SEEK_DATA flags
2011-08-20 16:32 ` Marco Stornelli
@ 2011-08-22 6:08 ` Sunil Mushran
-1 siblings, 0 replies; 37+ messages in thread
From: Sunil Mushran @ 2011-08-22 6:08 UTC (permalink / raw)
To: Marco Stornelli
Cc: Josef Bacik, linux-fsdevel, linux-kernel, linux-btrfs, xfs, viro
On 08/20/2011 09:32 AM, Marco Stornelli wrote:
> Il 20/08/2011 17:36, Sunil Mushran ha scritto:
>> On 08/20/2011 03:03 AM, Marco Stornelli wrote:
>>> Il 20/08/2011 11:41, Marco Stornelli ha scritto:
>>>> Hi,
>>>>
>>>> Il 28/06/2011 17:33, Josef Bacik ha scritto:
>>>>> This just gets us ready to support the SEEK_HOLE and SEEK_DATA flags.
>>>>> Turns out
>>>>> using fiemap in things like cp cause more problems than it solves, so
>>>>> lets try
>>>>> and give userspace an interface that doesn't suck. We need to match
>>>>> solaris
>>>>> here, and the definitions are
>>>>>
>>>>> *o* If /whence/ is SEEK_HOLE, the offset of the start of the
>>>>> next hole greater than or equal to the supplied offset
>>>>> is returned. The definition of a hole is provided near
>>>>> the end of the DESCRIPTION.
>>>>>
>>>>> *o* If /whence/ is SEEK_DATA, the file pointer is set to the
>>>>> start of the next non-hole file region greater than or
>>>>> equal to the supplied offset.
>>>>>
>>>>
>>>> I'm implementing the SEEK_DATA/SEEK_HOLE management for pramfs and
>>>> I've
>>>> got some doubts about the right behavior:
>>>>
>>>> 1) when we use SEEK_DATA/SEEK_HOLE, the offset used in lseek means
>>>> always the offset from the start of the file, right?
>>>>
>>>> 2) in case of a file with hole at the beginning and data at the
>>>> end, if
>>>> I do lseek(fd, 0, SEEK_HOLE) I should receive the end of the file
>>>> because the idea is to search the *next* hole and we have always a
>>>> virtual hole at the end of the file, right?
>>>
>>> Just to be precise about this question: the alternative here, it's to
>>> return the same position because we are already in a hole.
>>
>> Yes, the offset is from the start of the file.
>>
>> And yes, same offset is ok. I think the word next should be
>> dropped from the definition. It is misleading.
>>
>
> Thank. Yes the word "next" is not very clear. I re-read the proposal
> for the standard, actually it's seems to me that if we are in the last
> hole we should return the file size, if we are not in the last hole
> than it's ok the same offset - "....except that
> if offset falls beyond the last byte not within a hole, then the file
> offset may be set to the file size instead".
Any proposal that differentiates between holes is wrong. It should not
matter where the hole is.
Think of it from the usage-pov.
doff = 0;
while ((doff = lseek(SEEK_DATA, doff)) != -ENXIO) {
hoff = lseek(SEEK_HOLE, doff);
read_offset = doff;
read_len = hoff -doff;
process();
doff = hoff;
}
The goal is to make this as efficient as follows. Treating the last
hole differently adds more code for no benefit.
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 1/4] fs: add SEEK_HOLE and SEEK_DATA flags
@ 2011-08-22 6:08 ` Sunil Mushran
0 siblings, 0 replies; 37+ messages in thread
From: Sunil Mushran @ 2011-08-22 6:08 UTC (permalink / raw)
To: Marco Stornelli
Cc: linux-kernel, Josef Bacik, xfs, viro, linux-fsdevel, linux-btrfs
On 08/20/2011 09:32 AM, Marco Stornelli wrote:
> Il 20/08/2011 17:36, Sunil Mushran ha scritto:
>> On 08/20/2011 03:03 AM, Marco Stornelli wrote:
>>> Il 20/08/2011 11:41, Marco Stornelli ha scritto:
>>>> Hi,
>>>>
>>>> Il 28/06/2011 17:33, Josef Bacik ha scritto:
>>>>> This just gets us ready to support the SEEK_HOLE and SEEK_DATA flags.
>>>>> Turns out
>>>>> using fiemap in things like cp cause more problems than it solves, so
>>>>> lets try
>>>>> and give userspace an interface that doesn't suck. We need to match
>>>>> solaris
>>>>> here, and the definitions are
>>>>>
>>>>> *o* If /whence/ is SEEK_HOLE, the offset of the start of the
>>>>> next hole greater than or equal to the supplied offset
>>>>> is returned. The definition of a hole is provided near
>>>>> the end of the DESCRIPTION.
>>>>>
>>>>> *o* If /whence/ is SEEK_DATA, the file pointer is set to the
>>>>> start of the next non-hole file region greater than or
>>>>> equal to the supplied offset.
>>>>>
>>>>
>>>> I'm implementing the SEEK_DATA/SEEK_HOLE management for pramfs and
>>>> I've
>>>> got some doubts about the right behavior:
>>>>
>>>> 1) when we use SEEK_DATA/SEEK_HOLE, the offset used in lseek means
>>>> always the offset from the start of the file, right?
>>>>
>>>> 2) in case of a file with hole at the beginning and data at the
>>>> end, if
>>>> I do lseek(fd, 0, SEEK_HOLE) I should receive the end of the file
>>>> because the idea is to search the *next* hole and we have always a
>>>> virtual hole at the end of the file, right?
>>>
>>> Just to be precise about this question: the alternative here, it's to
>>> return the same position because we are already in a hole.
>>
>> Yes, the offset is from the start of the file.
>>
>> And yes, same offset is ok. I think the word next should be
>> dropped from the definition. It is misleading.
>>
>
> Thank. Yes the word "next" is not very clear. I re-read the proposal
> for the standard, actually it's seems to me that if we are in the last
> hole we should return the file size, if we are not in the last hole
> than it's ok the same offset - "....except that
> if offset falls beyond the last byte not within a hole, then the file
> offset may be set to the file size instead".
Any proposal that differentiates between holes is wrong. It should not
matter where the hole is.
Think of it from the usage-pov.
doff = 0;
while ((doff = lseek(SEEK_DATA, doff)) != -ENXIO) {
hoff = lseek(SEEK_HOLE, doff);
read_offset = doff;
read_len = hoff -doff;
process();
doff = hoff;
}
The goal is to make this as efficient as follows. Treating the last
hole differently adds more code for no benefit.
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 1/4] fs: add SEEK_HOLE and SEEK_DATA flags
[not found] ` <CAGpXXZ+xjhadprkc_LiP3qUypLLkCxdeEmo8+K+6mOnBuNhmLg@mail.gmail.com>
@ 2011-08-20 17:18 ` Greg Freemyer
0 siblings, 0 replies; 37+ messages in thread
From: Greg Freemyer @ 2011-08-20 17:18 UTC (permalink / raw)
To: Marco Stornelli
Cc: linux-btrfs, linux-kernel, xfs, viro, linux-fsdevel, Josef Bacik
[-- Attachment #1.1: Type: text/plain, Size: 1645 bytes --]
On Aug 20, 2011 5:52 AM, "Marco Stornelli" <marco.stornelli@gmail.com>
wrote:
>
> Hi,
>
>
> Il 28/06/2011 17:33, Josef Bacik ha scritto:
>>
>> This just gets us ready to support the SEEK_HOLE and SEEK_DATA flags.
Turns out
>> using fiemap in things like cp cause more problems than it solves, so
lets try
>> and give userspace an interface that doesn't suck. We need to match
solaris
>> here, and the definitions are
>>
>> *o* If /whence/ is SEEK_HOLE, the offset of the start of the
>> next hole greater than or equal to the supplied offset
>> is returned. The definition of a hole is provided near
>> the end of the DESCRIPTION.
>>
>> *o* If /whence/ is SEEK_DATA, the file pointer is set to the
>> start of the next non-hole file region greater than or
>> equal to the supplied offset.
>>
>
> I'm implementing the SEEK_DATA/SEEK_HOLE management for pramfs and I've
got some doubts about the right behavior:
>
> 1) when we use SEEK_DATA/SEEK_HOLE, the offset used in lseek means always
the offset from the start of the file, right?
>
> 2) in case of a file with hole at the beginning and data at the end, if I
do lseek(fd, 0, SEEK_HOLE) I should receive the end of the file because the
idea is to search the *next* hole and we have always a virtual hole at the
end of the file, right?
>
> 3) about the last sentence of point 2), is it always true even if we have
a case of block allocation beyond the end of file (fallocate with keep size
option)?
>
Marco,
You may want to enable the xfstests test(s) for SEEK_HOLE and SEEK_DATA for
pramfs. That should give you some confidence your implementing the api like
other filesystems are.
Greg
[-- Attachment #1.2: Type: text/html, Size: 2034 bytes --]
[-- Attachment #2: Type: text/plain, Size: 121 bytes --]
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 1/4] fs: add SEEK_HOLE and SEEK_DATA flags
2011-08-20 15:36 ` Sunil Mushran
@ 2011-08-20 16:32 ` Marco Stornelli
-1 siblings, 0 replies; 37+ messages in thread
From: Marco Stornelli @ 2011-08-20 16:32 UTC (permalink / raw)
To: Sunil Mushran
Cc: Josef Bacik, linux-fsdevel, linux-kernel, linux-btrfs, xfs, viro
Il 20/08/2011 17:36, Sunil Mushran ha scritto:
> On 08/20/2011 03:03 AM, Marco Stornelli wrote:
>> Il 20/08/2011 11:41, Marco Stornelli ha scritto:
>>> Hi,
>>>
>>> Il 28/06/2011 17:33, Josef Bacik ha scritto:
>>>> This just gets us ready to support the SEEK_HOLE and SEEK_DATA flags.
>>>> Turns out
>>>> using fiemap in things like cp cause more problems than it solves, so
>>>> lets try
>>>> and give userspace an interface that doesn't suck. We need to match
>>>> solaris
>>>> here, and the definitions are
>>>>
>>>> *o* If /whence/ is SEEK_HOLE, the offset of the start of the
>>>> next hole greater than or equal to the supplied offset
>>>> is returned. The definition of a hole is provided near
>>>> the end of the DESCRIPTION.
>>>>
>>>> *o* If /whence/ is SEEK_DATA, the file pointer is set to the
>>>> start of the next non-hole file region greater than or
>>>> equal to the supplied offset.
>>>>
>>>
>>> I'm implementing the SEEK_DATA/SEEK_HOLE management for pramfs and I've
>>> got some doubts about the right behavior:
>>>
>>> 1) when we use SEEK_DATA/SEEK_HOLE, the offset used in lseek means
>>> always the offset from the start of the file, right?
>>>
>>> 2) in case of a file with hole at the beginning and data at the end, if
>>> I do lseek(fd, 0, SEEK_HOLE) I should receive the end of the file
>>> because the idea is to search the *next* hole and we have always a
>>> virtual hole at the end of the file, right?
>>
>> Just to be precise about this question: the alternative here, it's to
>> return the same position because we are already in a hole.
>
> Yes, the offset is from the start of the file.
>
> And yes, same offset is ok. I think the word next should be
> dropped from the definition. It is misleading.
>
Thank. Yes the word "next" is not very clear. I re-read the proposal for
the standard, actually it's seems to me that if we are in the last hole
we should return the file size, if we are not in the last hole than it's
ok the same offset - "....except that
if offset falls beyond the last byte not within a hole, then the file
offset may be set to the file size instead".
Marco
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 1/4] fs: add SEEK_HOLE and SEEK_DATA flags
@ 2011-08-20 16:32 ` Marco Stornelli
0 siblings, 0 replies; 37+ messages in thread
From: Marco Stornelli @ 2011-08-20 16:32 UTC (permalink / raw)
To: Sunil Mushran
Cc: linux-kernel, Josef Bacik, xfs, viro, linux-fsdevel, linux-btrfs
Il 20/08/2011 17:36, Sunil Mushran ha scritto:
> On 08/20/2011 03:03 AM, Marco Stornelli wrote:
>> Il 20/08/2011 11:41, Marco Stornelli ha scritto:
>>> Hi,
>>>
>>> Il 28/06/2011 17:33, Josef Bacik ha scritto:
>>>> This just gets us ready to support the SEEK_HOLE and SEEK_DATA flags.
>>>> Turns out
>>>> using fiemap in things like cp cause more problems than it solves, so
>>>> lets try
>>>> and give userspace an interface that doesn't suck. We need to match
>>>> solaris
>>>> here, and the definitions are
>>>>
>>>> *o* If /whence/ is SEEK_HOLE, the offset of the start of the
>>>> next hole greater than or equal to the supplied offset
>>>> is returned. The definition of a hole is provided near
>>>> the end of the DESCRIPTION.
>>>>
>>>> *o* If /whence/ is SEEK_DATA, the file pointer is set to the
>>>> start of the next non-hole file region greater than or
>>>> equal to the supplied offset.
>>>>
>>>
>>> I'm implementing the SEEK_DATA/SEEK_HOLE management for pramfs and I've
>>> got some doubts about the right behavior:
>>>
>>> 1) when we use SEEK_DATA/SEEK_HOLE, the offset used in lseek means
>>> always the offset from the start of the file, right?
>>>
>>> 2) in case of a file with hole at the beginning and data at the end, if
>>> I do lseek(fd, 0, SEEK_HOLE) I should receive the end of the file
>>> because the idea is to search the *next* hole and we have always a
>>> virtual hole at the end of the file, right?
>>
>> Just to be precise about this question: the alternative here, it's to
>> return the same position because we are already in a hole.
>
> Yes, the offset is from the start of the file.
>
> And yes, same offset is ok. I think the word next should be
> dropped from the definition. It is misleading.
>
Thank. Yes the word "next" is not very clear. I re-read the proposal for
the standard, actually it's seems to me that if we are in the last hole
we should return the file size, if we are not in the last hole than it's
ok the same offset - "....except that
if offset falls beyond the last byte not within a hole, then the file
offset may be set to the file size instead".
Marco
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 1/4] fs: add SEEK_HOLE and SEEK_DATA flags
2011-08-20 10:03 ` Marco Stornelli
@ 2011-08-20 15:36 ` Sunil Mushran
-1 siblings, 0 replies; 37+ messages in thread
From: Sunil Mushran @ 2011-08-20 15:36 UTC (permalink / raw)
To: Marco Stornelli
Cc: Josef Bacik, linux-fsdevel, linux-kernel, linux-btrfs, xfs, viro
On 08/20/2011 03:03 AM, Marco Stornelli wrote:
> Il 20/08/2011 11:41, Marco Stornelli ha scritto:
>> Hi,
>>
>> Il 28/06/2011 17:33, Josef Bacik ha scritto:
>>> This just gets us ready to support the SEEK_HOLE and SEEK_DATA flags.
>>> Turns out
>>> using fiemap in things like cp cause more problems than it solves, so
>>> lets try
>>> and give userspace an interface that doesn't suck. We need to match
>>> solaris
>>> here, and the definitions are
>>>
>>> *o* If /whence/ is SEEK_HOLE, the offset of the start of the
>>> next hole greater than or equal to the supplied offset
>>> is returned. The definition of a hole is provided near
>>> the end of the DESCRIPTION.
>>>
>>> *o* If /whence/ is SEEK_DATA, the file pointer is set to the
>>> start of the next non-hole file region greater than or
>>> equal to the supplied offset.
>>>
>>
>> I'm implementing the SEEK_DATA/SEEK_HOLE management for pramfs and I've
>> got some doubts about the right behavior:
>>
>> 1) when we use SEEK_DATA/SEEK_HOLE, the offset used in lseek means
>> always the offset from the start of the file, right?
>>
>> 2) in case of a file with hole at the beginning and data at the end, if
>> I do lseek(fd, 0, SEEK_HOLE) I should receive the end of the file
>> because the idea is to search the *next* hole and we have always a
>> virtual hole at the end of the file, right?
>
> Just to be precise about this question: the alternative here, it's to
> return the same position because we are already in a hole.
Yes, the offset is from the start of the file.
And yes, same offset is ok. I think the word next should be
dropped from the definition. It is misleading.
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 1/4] fs: add SEEK_HOLE and SEEK_DATA flags
@ 2011-08-20 15:36 ` Sunil Mushran
0 siblings, 0 replies; 37+ messages in thread
From: Sunil Mushran @ 2011-08-20 15:36 UTC (permalink / raw)
To: Marco Stornelli
Cc: linux-kernel, Josef Bacik, xfs, viro, linux-fsdevel, linux-btrfs
On 08/20/2011 03:03 AM, Marco Stornelli wrote:
> Il 20/08/2011 11:41, Marco Stornelli ha scritto:
>> Hi,
>>
>> Il 28/06/2011 17:33, Josef Bacik ha scritto:
>>> This just gets us ready to support the SEEK_HOLE and SEEK_DATA flags.
>>> Turns out
>>> using fiemap in things like cp cause more problems than it solves, so
>>> lets try
>>> and give userspace an interface that doesn't suck. We need to match
>>> solaris
>>> here, and the definitions are
>>>
>>> *o* If /whence/ is SEEK_HOLE, the offset of the start of the
>>> next hole greater than or equal to the supplied offset
>>> is returned. The definition of a hole is provided near
>>> the end of the DESCRIPTION.
>>>
>>> *o* If /whence/ is SEEK_DATA, the file pointer is set to the
>>> start of the next non-hole file region greater than or
>>> equal to the supplied offset.
>>>
>>
>> I'm implementing the SEEK_DATA/SEEK_HOLE management for pramfs and I've
>> got some doubts about the right behavior:
>>
>> 1) when we use SEEK_DATA/SEEK_HOLE, the offset used in lseek means
>> always the offset from the start of the file, right?
>>
>> 2) in case of a file with hole at the beginning and data at the end, if
>> I do lseek(fd, 0, SEEK_HOLE) I should receive the end of the file
>> because the idea is to search the *next* hole and we have always a
>> virtual hole at the end of the file, right?
>
> Just to be precise about this question: the alternative here, it's to
> return the same position because we are already in a hole.
Yes, the offset is from the start of the file.
And yes, same offset is ok. I think the word next should be
dropped from the definition. It is misleading.
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 1/4] fs: add SEEK_HOLE and SEEK_DATA flags
2011-08-20 9:41 ` Marco Stornelli
@ 2011-08-20 10:03 ` Marco Stornelli
-1 siblings, 0 replies; 37+ messages in thread
From: Marco Stornelli @ 2011-08-20 10:03 UTC (permalink / raw)
To: Josef Bacik; +Cc: linux-fsdevel, linux-kernel, linux-btrfs, xfs, viro
Il 20/08/2011 11:41, Marco Stornelli ha scritto:
> Hi,
>
> Il 28/06/2011 17:33, Josef Bacik ha scritto:
>> This just gets us ready to support the SEEK_HOLE and SEEK_DATA flags.
>> Turns out
>> using fiemap in things like cp cause more problems than it solves, so
>> lets try
>> and give userspace an interface that doesn't suck. We need to match
>> solaris
>> here, and the definitions are
>>
>> *o* If /whence/ is SEEK_HOLE, the offset of the start of the
>> next hole greater than or equal to the supplied offset
>> is returned. The definition of a hole is provided near
>> the end of the DESCRIPTION.
>>
>> *o* If /whence/ is SEEK_DATA, the file pointer is set to the
>> start of the next non-hole file region greater than or
>> equal to the supplied offset.
>>
>
> I'm implementing the SEEK_DATA/SEEK_HOLE management for pramfs and I've
> got some doubts about the right behavior:
>
> 1) when we use SEEK_DATA/SEEK_HOLE, the offset used in lseek means
> always the offset from the start of the file, right?
>
> 2) in case of a file with hole at the beginning and data at the end, if
> I do lseek(fd, 0, SEEK_HOLE) I should receive the end of the file
> because the idea is to search the *next* hole and we have always a
> virtual hole at the end of the file, right?
Just to be precise about this question: the alternative here, it's to
return the same position because we are already in a hole.
Marco
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 1/4] fs: add SEEK_HOLE and SEEK_DATA flags
@ 2011-08-20 10:03 ` Marco Stornelli
0 siblings, 0 replies; 37+ messages in thread
From: Marco Stornelli @ 2011-08-20 10:03 UTC (permalink / raw)
To: Josef Bacik; +Cc: linux-fsdevel, viro, linux-kernel, linux-btrfs, xfs
Il 20/08/2011 11:41, Marco Stornelli ha scritto:
> Hi,
>
> Il 28/06/2011 17:33, Josef Bacik ha scritto:
>> This just gets us ready to support the SEEK_HOLE and SEEK_DATA flags.
>> Turns out
>> using fiemap in things like cp cause more problems than it solves, so
>> lets try
>> and give userspace an interface that doesn't suck. We need to match
>> solaris
>> here, and the definitions are
>>
>> *o* If /whence/ is SEEK_HOLE, the offset of the start of the
>> next hole greater than or equal to the supplied offset
>> is returned. The definition of a hole is provided near
>> the end of the DESCRIPTION.
>>
>> *o* If /whence/ is SEEK_DATA, the file pointer is set to the
>> start of the next non-hole file region greater than or
>> equal to the supplied offset.
>>
>
> I'm implementing the SEEK_DATA/SEEK_HOLE management for pramfs and I've
> got some doubts about the right behavior:
>
> 1) when we use SEEK_DATA/SEEK_HOLE, the offset used in lseek means
> always the offset from the start of the file, right?
>
> 2) in case of a file with hole at the beginning and data at the end, if
> I do lseek(fd, 0, SEEK_HOLE) I should receive the end of the file
> because the idea is to search the *next* hole and we have always a
> virtual hole at the end of the file, right?
Just to be precise about this question: the alternative here, it's to
return the same position because we are already in a hole.
Marco
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 1/4] fs: add SEEK_HOLE and SEEK_DATA flags
2011-06-28 15:33 ` Josef Bacik
@ 2011-08-20 9:41 ` Marco Stornelli
-1 siblings, 0 replies; 37+ messages in thread
From: Marco Stornelli @ 2011-08-20 9:41 UTC (permalink / raw)
To: Josef Bacik; +Cc: linux-fsdevel, linux-kernel, linux-btrfs, xfs, viro
Hi,
Il 28/06/2011 17:33, Josef Bacik ha scritto:
> This just gets us ready to support the SEEK_HOLE and SEEK_DATA flags. Turns out
> using fiemap in things like cp cause more problems than it solves, so lets try
> and give userspace an interface that doesn't suck. We need to match solaris
> here, and the definitions are
>
> *o* If /whence/ is SEEK_HOLE, the offset of the start of the
> next hole greater than or equal to the supplied offset
> is returned. The definition of a hole is provided near
> the end of the DESCRIPTION.
>
> *o* If /whence/ is SEEK_DATA, the file pointer is set to the
> start of the next non-hole file region greater than or
> equal to the supplied offset.
>
I'm implementing the SEEK_DATA/SEEK_HOLE management for pramfs and I've
got some doubts about the right behavior:
1) when we use SEEK_DATA/SEEK_HOLE, the offset used in lseek means
always the offset from the start of the file, right?
2) in case of a file with hole at the beginning and data at the end, if
I do lseek(fd, 0, SEEK_HOLE) I should receive the end of the file
because the idea is to search the *next* hole and we have always a
virtual hole at the end of the file, right?
3) about the last sentence of point 2), is it always true even if we
have a case of block allocation beyond the end of file (fallocate with
keep size option)?
Thanks.
Regards,
Marco
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 1/4] fs: add SEEK_HOLE and SEEK_DATA flags
@ 2011-08-20 9:41 ` Marco Stornelli
0 siblings, 0 replies; 37+ messages in thread
From: Marco Stornelli @ 2011-08-20 9:41 UTC (permalink / raw)
To: Josef Bacik; +Cc: linux-fsdevel, viro, linux-kernel, linux-btrfs, xfs
Hi,
Il 28/06/2011 17:33, Josef Bacik ha scritto:
> This just gets us ready to support the SEEK_HOLE and SEEK_DATA flags. Turns out
> using fiemap in things like cp cause more problems than it solves, so lets try
> and give userspace an interface that doesn't suck. We need to match solaris
> here, and the definitions are
>
> *o* If /whence/ is SEEK_HOLE, the offset of the start of the
> next hole greater than or equal to the supplied offset
> is returned. The definition of a hole is provided near
> the end of the DESCRIPTION.
>
> *o* If /whence/ is SEEK_DATA, the file pointer is set to the
> start of the next non-hole file region greater than or
> equal to the supplied offset.
>
I'm implementing the SEEK_DATA/SEEK_HOLE management for pramfs and I've
got some doubts about the right behavior:
1) when we use SEEK_DATA/SEEK_HOLE, the offset used in lseek means
always the offset from the start of the file, right?
2) in case of a file with hole at the beginning and data at the end, if
I do lseek(fd, 0, SEEK_HOLE) I should receive the end of the file
because the idea is to search the *next* hole and we have always a
virtual hole at the end of the file, right?
3) about the last sentence of point 2), is it always true even if we
have a case of block allocation beyond the end of file (fallocate with
keep size option)?
Thanks.
Regards,
Marco
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 1/4] fs: add SEEK_HOLE and SEEK_DATA flags
2011-06-28 15:33 ` Josef Bacik
@ 2011-07-29 9:58 ` Marco Stornelli
-1 siblings, 0 replies; 37+ messages in thread
From: Marco Stornelli @ 2011-07-29 9:58 UTC (permalink / raw)
To: Josef Bacik; +Cc: linux-fsdevel, linux-kernel, linux-btrfs, xfs, viro
Sorry maybe I'm a bit late? :)
Il 28/06/2011 17:33, Josef Bacik ha scritto:
>
> loff_t default_llseek(struct file *file, loff_t offset, int origin)
> {
> + struct inode *inode = file->f_path.dentry->d_inode;
> loff_t retval;
>
> - mutex_lock(&file->f_dentry->d_inode->i_mutex);
> + mutex_lock(&inode->i_mutex);
> switch (origin) {
> case SEEK_END:
> - offset += i_size_read(file->f_path.dentry->d_inode);
> + offset += i_size_read(inode);
Here we are under mutex, so I think we can use directly i_size without
i_size_read.
Marco
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH 1/4] fs: add SEEK_HOLE and SEEK_DATA flags
@ 2011-07-29 9:58 ` Marco Stornelli
0 siblings, 0 replies; 37+ messages in thread
From: Marco Stornelli @ 2011-07-29 9:58 UTC (permalink / raw)
To: Josef Bacik; +Cc: linux-fsdevel, viro, linux-kernel, linux-btrfs, xfs
Sorry maybe I'm a bit late? :)
Il 28/06/2011 17:33, Josef Bacik ha scritto:
>
> loff_t default_llseek(struct file *file, loff_t offset, int origin)
> {
> + struct inode *inode = file->f_path.dentry->d_inode;
> loff_t retval;
>
> - mutex_lock(&file->f_dentry->d_inode->i_mutex);
> + mutex_lock(&inode->i_mutex);
> switch (origin) {
> case SEEK_END:
> - offset += i_size_read(file->f_path.dentry->d_inode);
> + offset += i_size_read(inode);
Here we are under mutex, so I think we can use directly i_size without
i_size_read.
Marco
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH 1/4] fs: add SEEK_HOLE and SEEK_DATA flags
@ 2011-06-28 15:33 ` Josef Bacik
0 siblings, 0 replies; 37+ messages in thread
From: Josef Bacik @ 2011-06-28 15:33 UTC (permalink / raw)
To: linux-fsdevel, linux-kernel, linux-btrfs, xfs, viro
This just gets us ready to support the SEEK_HOLE and SEEK_DATA flags. Turns out
using fiemap in things like cp cause more problems than it solves, so lets try
and give userspace an interface that doesn't suck. We need to match solaris
here, and the definitions are
*o* If /whence/ is SEEK_HOLE, the offset of the start of the
next hole greater than or equal to the supplied offset
is returned. The definition of a hole is provided near
the end of the DESCRIPTION.
*o* If /whence/ is SEEK_DATA, the file pointer is set to the
start of the next non-hole file region greater than or
equal to the supplied offset.
So in the generic case the entire file is data and there is a virtual hole at
the end. That means we will just return i_size for SEEK_HOLE and will return
the same offset for SEEK_DATA. This is how Solaris does it so we have to do it
the same way.
Thanks,
Signed-off-by: Josef Bacik <josef@redhat.com>
---
fs/read_write.c | 44 +++++++++++++++++++++++++++++++++++++++++---
include/linux/fs.h | 4 +++-
2 files changed, 44 insertions(+), 4 deletions(-)
diff --git a/fs/read_write.c b/fs/read_write.c
index 5520f8a..5907b49 100644
--- a/fs/read_write.c
+++ b/fs/read_write.c
@@ -64,6 +64,23 @@ generic_file_llseek_unlocked(struct file *file, loff_t offset, int origin)
return file->f_pos;
offset += file->f_pos;
break;
+ case SEEK_DATA:
+ /*
+ * In the generic case the entire file is data, so as long as
+ * offset isn't at the end of the file then the offset is data.
+ */
+ if (offset >= inode->i_size)
+ return -ENXIO;
+ break;
+ case SEEK_HOLE:
+ /*
+ * There is a virtual hole at the end of the file, so as long as
+ * offset isn't i_size or larger, return i_size.
+ */
+ if (offset >= inode->i_size)
+ return -ENXIO;
+ offset = inode->i_size;
+ break;
}
if (offset < 0 && !unsigned_offsets(file))
@@ -128,12 +145,13 @@ EXPORT_SYMBOL(no_llseek);
loff_t default_llseek(struct file *file, loff_t offset, int origin)
{
+ struct inode *inode = file->f_path.dentry->d_inode;
loff_t retval;
- mutex_lock(&file->f_dentry->d_inode->i_mutex);
+ mutex_lock(&inode->i_mutex);
switch (origin) {
case SEEK_END:
- offset += i_size_read(file->f_path.dentry->d_inode);
+ offset += i_size_read(inode);
break;
case SEEK_CUR:
if (offset == 0) {
@@ -141,6 +159,26 @@ loff_t default_llseek(struct file *file, loff_t offset, int origin)
goto out;
}
offset += file->f_pos;
+ break;
+ case SEEK_DATA:
+ /*
+ * In the generic case the entire file is data, so as
+ * long as offset isn't at the end of the file then the
+ * offset is data.
+ */
+ if (offset >= inode->i_size)
+ return -ENXIO;
+ break;
+ case SEEK_HOLE:
+ /*
+ * There is a virtual hole at the end of the file, so
+ * as long as offset isn't i_size or larger, return
+ * i_size.
+ */
+ if (offset >= inode->i_size)
+ return -ENXIO;
+ offset = inode->i_size;
+ break;
}
retval = -EINVAL;
if (offset >= 0 || unsigned_offsets(file)) {
@@ -151,7 +189,7 @@ loff_t default_llseek(struct file *file, loff_t offset, int origin)
retval = offset;
}
out:
- mutex_unlock(&file->f_dentry->d_inode->i_mutex);
+ mutex_unlock(&inode->i_mutex);
return retval;
}
EXPORT_SYMBOL(default_llseek);
diff --git a/include/linux/fs.h b/include/linux/fs.h
index b5b9792..c9156f3 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -32,7 +32,9 @@
#define SEEK_SET 0 /* seek relative to beginning of file */
#define SEEK_CUR 1 /* seek relative to current file position */
#define SEEK_END 2 /* seek relative to end of file */
-#define SEEK_MAX SEEK_END
+#define SEEK_DATA 3 /* seek to the next data */
+#define SEEK_HOLE 4 /* seek to the next hole */
+#define SEEK_MAX SEEK_HOLE
struct fstrim_range {
__u64 start;
--
1.7.5.2
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH 1/4] fs: add SEEK_HOLE and SEEK_DATA flags
@ 2011-06-28 15:33 ` Josef Bacik
0 siblings, 0 replies; 37+ messages in thread
From: Josef Bacik @ 2011-06-28 15:33 UTC (permalink / raw)
To: linux-fsdevel, linux-kernel, linux-btrfs, xfs, viro
This just gets us ready to support the SEEK_HOLE and SEEK_DATA flags. Turns out
using fiemap in things like cp cause more problems than it solves, so lets try
and give userspace an interface that doesn't suck. We need to match solaris
here, and the definitions are
*o* If /whence/ is SEEK_HOLE, the offset of the start of the
next hole greater than or equal to the supplied offset
is returned. The definition of a hole is provided near
the end of the DESCRIPTION.
*o* If /whence/ is SEEK_DATA, the file pointer is set to the
start of the next non-hole file region greater than or
equal to the supplied offset.
So in the generic case the entire file is data and there is a virtual hole at
the end. That means we will just return i_size for SEEK_HOLE and will return
the same offset for SEEK_DATA. This is how Solaris does it so we have to do it
the same way.
Thanks,
Signed-off-by: Josef Bacik <josef@redhat.com>
---
fs/read_write.c | 44 +++++++++++++++++++++++++++++++++++++++++---
include/linux/fs.h | 4 +++-
2 files changed, 44 insertions(+), 4 deletions(-)
diff --git a/fs/read_write.c b/fs/read_write.c
index 5520f8a..5907b49 100644
--- a/fs/read_write.c
+++ b/fs/read_write.c
@@ -64,6 +64,23 @@ generic_file_llseek_unlocked(struct file *file, loff_t offset, int origin)
return file->f_pos;
offset += file->f_pos;
break;
+ case SEEK_DATA:
+ /*
+ * In the generic case the entire file is data, so as long as
+ * offset isn't at the end of the file then the offset is data.
+ */
+ if (offset >= inode->i_size)
+ return -ENXIO;
+ break;
+ case SEEK_HOLE:
+ /*
+ * There is a virtual hole at the end of the file, so as long as
+ * offset isn't i_size or larger, return i_size.
+ */
+ if (offset >= inode->i_size)
+ return -ENXIO;
+ offset = inode->i_size;
+ break;
}
if (offset < 0 && !unsigned_offsets(file))
@@ -128,12 +145,13 @@ EXPORT_SYMBOL(no_llseek);
loff_t default_llseek(struct file *file, loff_t offset, int origin)
{
+ struct inode *inode = file->f_path.dentry->d_inode;
loff_t retval;
- mutex_lock(&file->f_dentry->d_inode->i_mutex);
+ mutex_lock(&inode->i_mutex);
switch (origin) {
case SEEK_END:
- offset += i_size_read(file->f_path.dentry->d_inode);
+ offset += i_size_read(inode);
break;
case SEEK_CUR:
if (offset == 0) {
@@ -141,6 +159,26 @@ loff_t default_llseek(struct file *file, loff_t offset, int origin)
goto out;
}
offset += file->f_pos;
+ break;
+ case SEEK_DATA:
+ /*
+ * In the generic case the entire file is data, so as
+ * long as offset isn't at the end of the file then the
+ * offset is data.
+ */
+ if (offset >= inode->i_size)
+ return -ENXIO;
+ break;
+ case SEEK_HOLE:
+ /*
+ * There is a virtual hole at the end of the file, so
+ * as long as offset isn't i_size or larger, return
+ * i_size.
+ */
+ if (offset >= inode->i_size)
+ return -ENXIO;
+ offset = inode->i_size;
+ break;
}
retval = -EINVAL;
if (offset >= 0 || unsigned_offsets(file)) {
@@ -151,7 +189,7 @@ loff_t default_llseek(struct file *file, loff_t offset, int origin)
retval = offset;
}
out:
- mutex_unlock(&file->f_dentry->d_inode->i_mutex);
+ mutex_unlock(&inode->i_mutex);
return retval;
}
EXPORT_SYMBOL(default_llseek);
diff --git a/include/linux/fs.h b/include/linux/fs.h
index b5b9792..c9156f3 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -32,7 +32,9 @@
#define SEEK_SET 0 /* seek relative to beginning of file */
#define SEEK_CUR 1 /* seek relative to current file position */
#define SEEK_END 2 /* seek relative to end of file */
-#define SEEK_MAX SEEK_END
+#define SEEK_DATA 3 /* seek to the next data */
+#define SEEK_HOLE 4 /* seek to the next hole */
+#define SEEK_MAX SEEK_HOLE
struct fstrim_range {
__u64 start;
--
1.7.5.2
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply related [flat|nested] 37+ messages in thread
end of thread, other threads:[~2011-08-31 0:35 UTC | newest]
Thread overview: 37+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-06-27 18:02 [PATCH 1/4] fs: add SEEK_HOLE and SEEK_DATA flags Josef Bacik
2011-06-27 18:02 ` [PATCH 2/4] Btrfs: implement our own ->llseek Josef Bacik
2011-06-27 18:02 ` [PATCH 3/4] Ext4: handle SEEK_HOLE/SEEK_DATA generically Josef Bacik
2011-06-27 18:02 ` [PATCH 4/4] fs: handle SEEK_HOLE/SEEK_DATA properly in all fs's that define their own llseek Josef Bacik
2011-06-27 18:02 ` [PATCH] xfstests 255: add a seek_data/seek_hole tester Josef Bacik
2011-06-27 18:32 ` Andreas Dilger
2011-06-27 18:47 ` Josef Bacik
2011-06-27 21:04 ` [PATCH 1/4] fs: add SEEK_HOLE and SEEK_DATA flags Josef Bacik
2011-06-28 15:33 Josef Bacik
2011-06-28 15:33 ` Josef Bacik
2011-07-29 9:58 ` Marco Stornelli
2011-07-29 9:58 ` Marco Stornelli
2011-08-20 9:41 ` Marco Stornelli
2011-08-20 9:41 ` Marco Stornelli
2011-08-20 10:03 ` Marco Stornelli
2011-08-20 10:03 ` Marco Stornelli
2011-08-20 15:36 ` Sunil Mushran
2011-08-20 15:36 ` Sunil Mushran
2011-08-20 16:32 ` Marco Stornelli
2011-08-20 16:32 ` Marco Stornelli
2011-08-22 6:08 ` Sunil Mushran
2011-08-22 6:08 ` Sunil Mushran
2011-08-22 10:56 ` Marco Stornelli
2011-08-22 10:56 ` Marco Stornelli
2011-08-22 10:56 ` Marco Stornelli
2011-08-22 10:56 ` Marco Stornelli
2011-08-22 15:57 ` Sunil Mushran
2011-08-22 15:57 ` Sunil Mushran
2011-08-22 17:56 ` Marco Stornelli
2011-08-22 17:56 ` Marco Stornelli
2011-08-22 21:22 ` Sunil Mushran
2011-08-22 21:22 ` Sunil Mushran
2011-08-23 17:44 ` Marco Stornelli
2011-08-23 17:44 ` Marco Stornelli
2011-08-31 0:35 ` Dave Chinner
2011-08-31 0:35 ` Dave Chinner
[not found] ` <CAGpXXZ+xjhadprkc_LiP3qUypLLkCxdeEmo8+K+6mOnBuNhmLg@mail.gmail.com>
2011-08-20 17:18 ` Greg Freemyer
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.