* [PATCH 5/5] generic block based fiemap implementation
@ 2008-05-25 0:02 Mark Fasheh
2008-05-25 8:00 ` Andreas Dilger
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Mark Fasheh @ 2008-05-25 0:02 UTC (permalink / raw)
To: linux-fsdevel; +Cc: Josef Bacik
From: Josef Bacik <jbacik@redhat.com>
Here is my updated generic block based fiemap implementation with the fixes that
Andreas suggested. Basically any block based fs (this patch includes ext3) just
has to declare its own fiemap() function and then call this generic function
with its own get_block_t. This works well for block based filesystems that will
map multiple contiguous blocks at one time, but will work for filesystems that
only map one block at a time, you will just end up with an "extent" for each
block. One gotcha is this will not play nicely where there is hole+data after
the EOF. This function will assume its hit the end of the data as soon as it
hits a hole after the EOF, so if there is any data past that it will not pick
that up. AFAIK no block based fs does this anyway, but its in the comments of
the function anyway just in case. Thanks much,
Signed-off-by: Josef Bacik <jbacik@redhat.com>
---
fs/ext3/file.c | 1 +
fs/ext3/inode.c | 8 +++
fs/ioctl.c | 128 +++++++++++++++++++++++++++++++++++++++++++++++
include/linux/ext3_fs.h | 2 +
include/linux/fs.h | 3 +
5 files changed, 142 insertions(+), 0 deletions(-)
diff --git a/fs/ext3/file.c b/fs/ext3/file.c
index acc4913..3be1e06 100644
--- a/fs/ext3/file.c
+++ b/fs/ext3/file.c
@@ -134,5 +134,6 @@ const struct inode_operations ext3_file_inode_operations = {
.removexattr = generic_removexattr,
#endif
.permission = ext3_permission,
+ .fiemap = ext3_fiemap,
};
diff --git a/fs/ext3/inode.c b/fs/ext3/inode.c
index 6ae4ecf..878b6da 100644
--- a/fs/ext3/inode.c
+++ b/fs/ext3/inode.c
@@ -36,6 +36,7 @@
#include <linux/mpage.h>
#include <linux/uio.h>
#include <linux/bio.h>
+#include <linux/fiemap.h>
#include "xattr.h"
#include "acl.h"
@@ -981,6 +982,13 @@ out:
return ret;
}
+int ext3_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
+ u64 start, u64 len)
+{
+ return generic_block_fiemap(inode, fieinfo, start, len,
+ ext3_get_block);
+}
+
/*
* `handle' can be NULL if create is zero
*/
diff --git a/fs/ioctl.c b/fs/ioctl.c
index 405bbcb..dbefd3b 100644
--- a/fs/ioctl.c
+++ b/fs/ioctl.c
@@ -13,6 +13,8 @@
#include <linux/security.h>
#include <linux/module.h>
#include <linux/uaccess.h>
+#include <linux/writeback.h>
+#include <linux/buffer_head.h>
#include <asm/ioctls.h>
@@ -209,6 +211,132 @@ out_bad_flags:
return -EBADR;
}
+#define blk_to_logical(inode, blk) (blk << (inode)->i_blkbits)
+#define logical_to_blk(inode, offset) (offset >> (inode)->i_blkbits);
+
+/*
+ * @inode - the inode to map
+ * @arg - the pointer to userspace where we copy everything to
+ * @get_block - the fs's get_block function
+ *
+ * This does FIEMAP for block based inodes. Basically it will just loop
+ * through get_block until we hit the number of extents we want to map, or we
+ * go past the end of the file and hit a hole.
+ *
+ * If it is possible to have holes and data past EOF then please don't use this
+ * function, it will do wrong things.
+ */
+int generic_block_fiemap(struct inode *inode,
+ struct fiemap_extent_info *fieinfo, u64 start,
+ u64 len, get_block_t *get_block)
+{
+ struct buffer_head tmp;
+ unsigned int start_blk;
+ long long length = 0, map_len = 0;
+ u64 logical = 0, phys = 0, size = 0;
+ u32 flags = 0;
+ int ret = 0;
+ int just_count = fieinfo->fi_flags & FIEMAP_FLAG_NUM_EXTENTS;
+
+ start_blk = logical_to_blk(inode, start);
+
+ /* guard against change */
+ mutex_lock(&inode->i_mutex);
+
+ length = (long long)min_t(u64, len, i_size_read(inode));
+ map_len = length;
+
+ do {
+ /*
+ * we set b_size to the total size we want so it will map as
+ * many contiguous blocks as possible at once
+ */
+ memset(&tmp, 0, sizeof(struct buffer_head));
+ tmp.b_size = map_len;
+
+ ret = get_block(inode, start_blk, &tmp, 0);
+ if (ret)
+ break;
+
+ /* HOLE */
+ if (!tmp.b_blocknr) {
+ /*
+ * first hole after going past the EOF, this is our
+ * last extent
+ */
+ if (length <= 0) {
+ if (just_count) {
+ fieinfo->fi_extents_mapped++;
+ break;
+ }
+
+ flags = FIEMAP_EXTENT_LAST;
+ ret = fiemap_fill_next_extent(fieinfo, logical,
+ phys, size,
+ flags, 0);
+ break;
+ }
+
+ length -= blk_to_logical(inode, 1);
+
+ /* if we have holes up to/past EOF then we're done */
+ if (length <= 0)
+ break;
+
+ start_blk++;
+ } else {
+ if (length <= 0 && size && !just_count) {
+ ret = fiemap_fill_next_extent(fieinfo, logical,
+ phys, size,
+ flags, 0);
+ if (ret)
+ break;
+ }
+
+ logical = blk_to_logical(inode, start_blk);
+ phys = blk_to_logical(inode, tmp.b_blocknr);
+ size = tmp.b_size;
+ flags = 0;
+
+ length -= tmp.b_size;
+ start_blk += logical_to_blk(inode, size);
+
+ if (just_count) {
+ fieinfo->fi_extents_mapped++;
+ if (fieinfo->fi_extents_mapped ==
+ fieinfo->fi_extents_max)
+ break;
+ continue;
+ }
+
+ /*
+ * if we are past the EOF we need to loop again to see
+ * if there is a hole so we can mark this extent as the
+ * last one, and if not keep mapping things until we
+ * find a hole, or we run out of slots in the extent
+ * array
+ */
+ if (length <= 0)
+ continue;
+
+ ret = fiemap_fill_next_extent(fieinfo, logical, phys,
+ size, flags, 0);
+ if (ret)
+ break;
+ }
+ cond_resched();
+ } while (1);
+
+ mutex_unlock(&inode->i_mutex);
+
+ /* if ret is 1 then we just hit the end of the extent array */
+ if (ret == 1)
+ ret = 0;
+
+ return ret;
+}
+EXPORT_SYMBOL(generic_block_fiemap);
+
static int file_ioctl(struct file *filp, unsigned int cmd,
unsigned long arg)
{
diff --git a/include/linux/ext3_fs.h b/include/linux/ext3_fs.h
index 36c5403..2dc0dbb 100644
--- a/include/linux/ext3_fs.h
+++ b/include/linux/ext3_fs.h
@@ -836,6 +836,8 @@ extern void ext3_truncate (struct inode *);
extern void ext3_set_inode_flags(struct inode *);
extern void ext3_get_inode_flags(struct ext3_inode_info *);
extern void ext3_set_aops(struct inode *inode);
+extern int ext3_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
+ u64 start, u64 len);
/* ioctl.c */
extern int ext3_ioctl (struct inode *, struct file *, unsigned int,
diff --git a/include/linux/fs.h b/include/linux/fs.h
index ed90393..5945136 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1976,6 +1976,9 @@ extern int vfs_fstat(unsigned int, struct kstat *);
extern int do_vfs_ioctl(struct file *filp, unsigned int fd, unsigned int cmd,
unsigned long arg);
+extern int generic_block_fiemap(struct inode *inode,
+ struct fiemap_extent_info *fieinfo, u64 start,
+ u64 len, get_block_t *get_block);
extern void get_filesystem(struct file_system_type *fs);
extern void put_filesystem(struct file_system_type *fs);
--
1.5.4.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 5/5] generic block based fiemap implementation 2008-05-25 0:02 [PATCH 5/5] generic block based fiemap implementation Mark Fasheh @ 2008-05-25 8:00 ` Andreas Dilger 2008-05-27 17:51 ` Chris Mason 2008-05-28 15:31 ` Josef Bacik 2 siblings, 0 replies; 6+ messages in thread From: Andreas Dilger @ 2008-05-25 8:00 UTC (permalink / raw) To: Mark Fasheh; +Cc: linux-fsdevel, Josef Bacik On May 24, 2008 17:02 -0700, Mark Fasheh wrote: > +int ext3_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo, > + u64 start, u64 len) > +{ > + return generic_block_fiemap(inode, fieinfo, start, len, > + ext3_get_block); > +} We can wire up ext2 in the same way, and likely most of the other filesystems. #define GENERIC_FIEMAP_FLAG_SUPP (FIEMAP_FLAG_NUM_EXTENTS | FIEMAP_FLAG_SYNC | FIEMAP_FLAG_HSM_READ | FIEMAP_FLAG_LUN_ORDER) > +int generic_block_fiemap(struct inode *inode, > + struct fiemap_extent_info *fieinfo, u64 start, > + u64 len, get_block_t *get_block) > +{ > + int just_count = fieinfo->fi_flags & FIEMAP_FLAG_NUM_EXTENTS; Don't need this anymore. if (fieinfo->fi_flags & ~GENERIC_FIEMAP_FLAG_SUPP) { fieinfo->fi_flags &= ~GENERIC_FIEMAP_FLAG_SUPP; return -EBADF; } > + /* HOLE */ > + if (!tmp.b_blocknr) { > + if (length <= 0) { > + if (just_count) { > + fieinfo->fi_extents_mapped++; > + break; > + } > + flags = FIEMAP_EXTENT_LAST; > + ret = fiemap_fill_next_extent(fieinfo, logical, > + phys, size, > + flags, 0); > + break; Can remove the just_count check, fiemap_fill_next_extent() will handle it. > + } else { > + if (length <= 0 && size && !just_count) { > + ret = fiemap_fill_next_extent(fieinfo, logical, > + phys, size, > + flags, 0); > + if (ret) > + break; > + } > + > + logical = blk_to_logical(inode, start_blk); > + phys = blk_to_logical(inode, tmp.b_blocknr); > + size = tmp.b_size; > + flags = 0; > + > + length -= tmp.b_size; > + start_blk += logical_to_blk(inode, size); > + > + if (just_count) { > + fieinfo->fi_extents_mapped++; > + if (fieinfo->fi_extents_mapped == > + fieinfo->fi_extents_max) > + break; > + continue; > + } Can also remove this just_count check, which is incorrect in any case. The fi_extents_max is only when storing extents in the array, it isn't meaningful when only counting extents. More important is the extent merging I proposed for fiemap_fill_next_extent() for filesystems that only return single blocks, so we want to always call it from the generic_block_fiemap() code. Cheers, Andreas -- Andreas Dilger Sr. Staff Engineer, Lustre Group Sun Microsystems of Canada, Inc. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 5/5] generic block based fiemap implementation 2008-05-25 0:02 [PATCH 5/5] generic block based fiemap implementation Mark Fasheh 2008-05-25 8:00 ` Andreas Dilger @ 2008-05-27 17:51 ` Chris Mason 2008-05-28 15:31 ` Josef Bacik 2 siblings, 0 replies; 6+ messages in thread From: Chris Mason @ 2008-05-27 17:51 UTC (permalink / raw) To: Mark Fasheh; +Cc: linux-fsdevel, Josef Bacik On Saturday 24 May 2008, Mark Fasheh wrote: > From: Josef Bacik <jbacik@redhat.com> > > Here is my updated generic block based fiemap implementation with the fixes > that Andreas suggested. Basically any block based fs (this patch includes > ext3) just has to declare its own fiemap() function and then call this > generic function with its own get_block_t. > > +int generic_block_fiemap(struct inode *inode, > + struct fiemap_extent_info *fieinfo, u64 start, > + u64 len, get_block_t *get_block) > +{ > + struct buffer_head tmp; [ ... ] > + do { > + /* > + * we set b_size to the total size we want so it will map as > + * many contiguous blocks as possible at once > + */ > + memset(&tmp, 0, sizeof(struct buffer_head)); > + tmp.b_size = map_len; > + > + ret = get_block(inode, start_blk, &tmp, 0); > + if (ret) > + break; > + > + /* HOLE */ > + if (!tmp.b_blocknr) { This should be if (!buffer_mapped(&tmp)). You can't be sure that filesystems returning a hole will use a b_blocknr of zero, and you can't be sure that a block number of zero is a hole. -chris -chris ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 5/5] generic block based fiemap implementation 2008-05-25 0:02 [PATCH 5/5] generic block based fiemap implementation Mark Fasheh 2008-05-25 8:00 ` Andreas Dilger 2008-05-27 17:51 ` Chris Mason @ 2008-05-28 15:31 ` Josef Bacik 2008-05-29 23:22 ` Andreas Dilger 2008-05-29 23:24 ` Andreas Dilger 2 siblings, 2 replies; 6+ messages in thread From: Josef Bacik @ 2008-05-28 15:31 UTC (permalink / raw) To: Mark Fasheh; +Cc: linux-fsdevel, Josef Bacik On Sat, May 24, 2008 at 05:02:35PM -0700, Mark Fasheh wrote: > From: Josef Bacik <jbacik@redhat.com> > > Here is my updated generic block based fiemap implementation with the fixes that > Andreas suggested. Basically any block based fs (this patch includes ext3) just > has to declare its own fiemap() function and then call this generic function > with its own get_block_t. This works well for block based filesystems that will > map multiple contiguous blocks at one time, but will work for filesystems that > only map one block at a time, you will just end up with an "extent" for each > block. One gotcha is this will not play nicely where there is hole+data after > the EOF. This function will assume its hit the end of the data as soon as it > hits a hole after the EOF, so if there is any data past that it will not pick > that up. AFAIK no block based fs does this anyway, but its in the comments of > the function anyway just in case. Thanks much, > Here is my updated patch that should address Andreas's and Chris's concerns. I added ext2 support as well. Thank you, Signed-off-by: Josef Bacik <jbacik@redhat.com> Index: linux-2.6/fs/ext3/file.c =================================================================== --- linux-2.6.orig/fs/ext3/file.c +++ linux-2.6/fs/ext3/file.c @@ -134,5 +134,6 @@ const struct inode_operations ext3_file_ .removexattr = generic_removexattr, #endif .permission = ext3_permission, + .fiemap = ext3_fiemap, }; Index: linux-2.6/fs/ext3/inode.c =================================================================== --- linux-2.6.orig/fs/ext3/inode.c +++ linux-2.6/fs/ext3/inode.c @@ -36,6 +36,7 @@ #include <linux/mpage.h> #include <linux/uio.h> #include <linux/bio.h> +#include <linux/fiemap.h> #include "xattr.h" #include "acl.h" @@ -981,6 +982,13 @@ out: return ret; } +int ext3_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo, + u64 start, u64 len) +{ + return generic_block_fiemap(inode, fieinfo, start, len, + ext3_get_block); +} + /* * `handle' can be NULL if create is zero */ Index: linux-2.6/include/linux/ext3_fs.h =================================================================== --- linux-2.6.orig/include/linux/ext3_fs.h +++ linux-2.6/include/linux/ext3_fs.h @@ -836,6 +836,8 @@ extern void ext3_truncate (struct inode extern void ext3_set_inode_flags(struct inode *); extern void ext3_get_inode_flags(struct ext3_inode_info *); extern void ext3_set_aops(struct inode *inode); +extern int ext3_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo, + u64 start, u64 len); /* ioctl.c */ extern int ext3_ioctl (struct inode *, struct file *, unsigned int, Index: linux-2.6/fs/ioctl.c =================================================================== --- linux-2.6.orig/fs/ioctl.c +++ linux-2.6/fs/ioctl.c @@ -13,6 +13,8 @@ #include <linux/security.h> #include <linux/module.h> #include <linux/uaccess.h> +#include <linux/writeback.h> +#include <linux/buffer_head.h> #include <asm/ioctls.h> @@ -209,6 +211,118 @@ out_bad_flags: return -EBADR; } +#define blk_to_logical(inode, blk) (blk << (inode)->i_blkbits) +#define logical_to_blk(inode, offset) (offset >> (inode)->i_blkbits); + +/* + * @inode - the inode to map + * @arg - the pointer to userspace where we copy everything to + * @get_block - the fs's get_block function + * + * This does FIEMAP for block based inodes. Basically it will just loop + * through get_block until we hit the number of extents we want to map, or we + * go past the end of the file and hit a hole. + * + * If it is possible to have holes and data past EOF then please don't use this + * function, it will do wrong things. + */ +int generic_block_fiemap(struct inode *inode, + struct fiemap_extent_info *fieinfo, u64 start, + u64 len, get_block_t *get_block) +{ + struct buffer_head tmp; + unsigned int start_blk; + long long length = 0, map_len = 0; + u64 logical = 0, phys = 0, size = 0; + u32 flags = 0; + int ret = 0; + + start_blk = logical_to_blk(inode, start); + + /* guard against change */ + mutex_lock(&inode->i_mutex); + + length = (long long)min_t(u64, len, i_size_read(inode)); + map_len = length; + + do { + /* + * we set b_size to the total size we want so it will map as + * many contiguous blocks as possible at once + */ + memset(&tmp, 0, sizeof(struct buffer_head)); + tmp.b_size = map_len; + + ret = get_block(inode, start_blk, &tmp, 0); + if (ret) + break; + + /* HOLE */ + if (!buffer_mapped(&tmp)) { + /* + * first hole after going past the EOF, this is our + * last extent + */ + if (length <= 0) { + flags = FIEMAP_EXTENT_LAST; + ret = fiemap_fill_next_extent(fieinfo, logical, + phys, size, + flags, 0); + break; + } + + length -= blk_to_logical(inode, 1); + + /* if we have holes up to/past EOF then we're done */ + if (length <= 0) + break; + + start_blk++; + } else { + if (length <= 0 && size) { + ret = fiemap_fill_next_extent(fieinfo, logical, + phys, size, + flags, 0); + if (ret) + break; + } + + logical = blk_to_logical(inode, start_blk); + phys = blk_to_logical(inode, tmp.b_blocknr); + size = tmp.b_size; + flags = 0; + + length -= tmp.b_size; + start_blk += logical_to_blk(inode, size); + + /* + * if we are past the EOF we need to loop again to see + * if there is a hole so we can mark this extent as the + * last one, and if not keep mapping things until we + * find a hole, or we run out of slots in the extent + * array + */ + if (length <= 0) + continue; + + ret = fiemap_fill_next_extent(fieinfo, logical, phys, + size, flags, 0); + if (ret) + break; + } + cond_resched(); + } while (1); + + mutex_unlock(&inode->i_mutex); + + /* if ret is 1 then we just hit the end of the extent array */ + if (ret == 1) + ret = 0; + + return ret; +} +EXPORT_SYMBOL(generic_block_fiemap); + static int file_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) { Index: linux-2.6/include/linux/fs.h =================================================================== --- linux-2.6.orig/include/linux/fs.h +++ linux-2.6/include/linux/fs.h @@ -1976,6 +1976,9 @@ extern int vfs_fstat(unsigned int, struc extern int do_vfs_ioctl(struct file *filp, unsigned int fd, unsigned int cmd, unsigned long arg); +extern int generic_block_fiemap(struct inode *inode, + struct fiemap_extent_info *fieinfo, u64 start, + u64 len, get_block_t *get_block); extern void get_filesystem(struct file_system_type *fs); extern void put_filesystem(struct file_system_type *fs); Index: linux-2.6/fs/ext2/ext2.h =================================================================== --- linux-2.6.orig/fs/ext2/ext2.h +++ linux-2.6/fs/ext2/ext2.h @@ -133,6 +133,8 @@ extern void ext2_truncate (struct inode extern int ext2_setattr (struct dentry *, struct iattr *); extern void ext2_set_inode_flags(struct inode *inode); extern void ext2_get_inode_flags(struct ext2_inode_info *); +extern int ext2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo, + u64 start, u64 len); int __ext2_write_begin(struct file *file, struct address_space *mapping, loff_t pos, unsigned len, unsigned flags, struct page **pagep, void **fsdata); Index: linux-2.6/fs/ext2/file.c =================================================================== --- linux-2.6.orig/fs/ext2/file.c +++ linux-2.6/fs/ext2/file.c @@ -86,4 +86,5 @@ const struct inode_operations ext2_file_ #endif .setattr = ext2_setattr, .permission = ext2_permission, + .fiemap = ext2_fiemap, }; Index: linux-2.6/fs/ext2/inode.c =================================================================== --- linux-2.6.orig/fs/ext2/inode.c +++ linux-2.6/fs/ext2/inode.c @@ -31,6 +31,7 @@ #include <linux/writeback.h> #include <linux/buffer_head.h> #include <linux/mpage.h> +#include <linux/fiemap.h> #include "ext2.h" #include "acl.h" #include "xip.h" @@ -704,6 +705,13 @@ int ext2_get_block(struct inode *inode, } +int ext2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo, + u64 start, u64 len) +{ + return generic_block_fiemap(inode, fieinfo, start, len, + ext2_get_block); +} + static int ext2_writepage(struct page *page, struct writeback_control *wbc) { return block_write_full_page(page, ext2_get_block, wbc); ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 5/5] generic block based fiemap implementation 2008-05-28 15:31 ` Josef Bacik @ 2008-05-29 23:22 ` Andreas Dilger 2008-05-29 23:24 ` Andreas Dilger 1 sibling, 0 replies; 6+ messages in thread From: Andreas Dilger @ 2008-05-29 23:22 UTC (permalink / raw) To: Josef Bacik; +Cc: Mark Fasheh, linux-fsdevel On May 28, 2008 11:31 -0400, Josef Bacik wrote: > Signed-off-by: Josef Bacik <jbacik@redhat.com> > +/* > + * @inode - the inode to map > + * @arg - the pointer to userspace where we copy everything to > + * @get_block - the fs's get_block function > + * > + * This does FIEMAP for block based inodes. Basically it will just loop > + * through get_block until we hit the number of extents we want to map, or we > + * go past the end of the file and hit a hole. > + * > + * If it is possible to have holes and data past EOF then please don't use > + * this function, it will do wrong things. > + */ I think it would be clearer to write at the end "If it is possible to have data blocks beyond a hole past @inode->i_size, then please do not use this function, it will stop at the first unmapped block beyond i_size." > +int generic_block_fiemap(struct inode *inode, > + struct fiemap_extent_info *fieinfo, u64 start, > + u64 len, get_block_t *get_block) > +{ > + struct buffer_head tmp; > + unsigned int start_blk; > + long long length = 0, map_len = 0; > + u64 logical = 0, phys = 0, size = 0; > + u32 flags = 0; > + int ret = 0; > + > + start_blk = logical_to_blk(inode, start); > + > + /* guard against change */ > + mutex_lock(&inode->i_mutex); > + > + length = (long long)min_t(u64, len, i_size_read(inode)); > + map_len = length; > + > + do { > + /* > + * we set b_size to the total size we want so it will map as > + * many contiguous blocks as possible at once > + */ > + memset(&tmp, 0, sizeof(struct buffer_head)); > + tmp.b_size = map_len; > + > + ret = get_block(inode, start_blk, &tmp, 0); > + if (ret) > + break; Shouldn't map_len be reduced as the file is traversed? Otherwise the filesystem may be doing a lot more work than it needs to for block mapping if the requested length is not the whole file. If, for example, the first 1GB of the file is requested (map_len = 2 << 30) and 1022MB are mapped, the last 1MB get_block() request will still request 1GB mapping. It isn't a major problem (only a corner case really), and the map_len can't be exactly the same as "length", because it still needs to be positive after EOF so that blocks will continue to be mapped. Cheers, Andreas -- Andreas Dilger Sr. Staff Engineer, Lustre Group Sun Microsystems of Canada, Inc. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 5/5] generic block based fiemap implementation 2008-05-28 15:31 ` Josef Bacik 2008-05-29 23:22 ` Andreas Dilger @ 2008-05-29 23:24 ` Andreas Dilger 1 sibling, 0 replies; 6+ messages in thread From: Andreas Dilger @ 2008-05-29 23:24 UTC (permalink / raw) To: Josef Bacik; +Cc: Mark Fasheh, linux-fsdevel On May 28, 2008 11:31 -0400, Josef Bacik wrote: > +int generic_block_fiemap(struct inode *inode, > + struct fiemap_extent_info *fieinfo, u64 start, > + u64 len, get_block_t *get_block) > +{ This still doesn't do any validity checking on fieinfo->fi_flags. You can't assume that the check in ioctl_fiemap() is correct for this routine. As soon as some new flag is added (e.g. FIEMAP_FLAG_XATTR) this code would do the wrong thing. Cheers, Andreas -- Andreas Dilger Sr. Staff Engineer, Lustre Group Sun Microsystems of Canada, Inc. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2008-05-29 23:24 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2008-05-25 0:02 [PATCH 5/5] generic block based fiemap implementation Mark Fasheh 2008-05-25 8:00 ` Andreas Dilger 2008-05-27 17:51 ` Chris Mason 2008-05-28 15:31 ` Josef Bacik 2008-05-29 23:22 ` Andreas Dilger 2008-05-29 23:24 ` Andreas Dilger
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).