Message ID | 1358493381-20150-2-git-send-email-wenqing.lz@taobao.com |
---|---|
State | Accepted, archived |
Headers | show |
On Fri, Jan 18, 2013 at 03:16:21PM +0800, Zheng Liu wrote: > From: Zheng Liu <wenqing.lz@taobao.com> > > After adding indirect punching hole feature, we need to enable it in fallocate. > For this purpose, some sanity checks need to be adjusted. Currently we need to > check FALLOC_FL_PUNCH_HOLE flag before other sanity checks. I've folded these two patches into one since the first patch in this series since there won't be any way to exercise the new code paths until we enable this in fallocate(), and it doesn't really change existing code paths. If it did make huge changes in the normal code path, it might be useful to keep the two commits separate, but that's not the case here. Anyway, they look good so I've checked them into my tree. I'm currently kicking off a test run to make sure there aren't any problems except for the test 255 failure when testing w/o extents, but I don't really anticipate any issues. Thanks!! - Ted -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Jan 24, 2013 at 10:32:01PM -0500, Theodore Ts'o wrote: > On Fri, Jan 18, 2013 at 03:16:21PM +0800, Zheng Liu wrote: > > From: Zheng Liu <wenqing.lz@taobao.com> > > > > After adding indirect punching hole feature, we need to enable it in fallocate. > > For this purpose, some sanity checks need to be adjusted. Currently we need to > > check FALLOC_FL_PUNCH_HOLE flag before other sanity checks. > > I've folded these two patches into one since the first patch in this > series since there won't be any way to exercise the new code paths > until we enable this in fallocate(), and it doesn't really change > existing code paths. If it did make huge changes in the normal code > path, it might be useful to keep the two commits separate, but that's > not the case here. Thanks for pointing out. :-) > > Anyway, they look good so I've checked them into my tree. I'm > currently kicking off a test run to make sure there aren't any > problems except for the test 255 failure when testing w/o extents, but > I don't really anticipate any issues. I wonder that maybe we need to submit a patch to let xfstest understand that a filesystem supports extents or not because after applied this patch indirect-based file in ext4 has supported seek_data/hole and hole punching. I usually run xfstest automatically, and every time I need to check the result of #255 and #285 manually. That is annoying for me. Regards, - Zheng -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Jan 25, 2013 at 11:59:12AM +0800, Zheng Liu wrote: > > I wonder that maybe we need to submit a patch to let xfstest understand > that a filesystem supports extents or not because after applied this > patch indirect-based file in ext4 has supported seek_data/hole and hole > punching. I usually run xfstest automatically, and every time I need > to check the result of #255 and #285 manually. That is annoying for me. I would think the right thing to do is to have xfstests make sure it understands that fallocate working with FALLOC_FL_PUNCH_HOLE does not imply that fallocate without the FALLOC_FL_PUNCH_HOLE flag OR'ed in will work. It should test for support for preallocation and hole punching separately, and do tests accordingly. That way we don't have to add explicit ext4 knowledge/logic to xfstests. (Maybe in the future there will be some other file system which supports punch hole but not preallocate, and it might not be based on whether or not the file is using ext4 extents or not.) Cheers, - Ted -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[cc xfs@oss.sgi.com] On Thu, Jan 24, 2013 at 11:21:22PM -0500, Theodore Ts'o wrote: > On Fri, Jan 25, 2013 at 11:59:12AM +0800, Zheng Liu wrote: > > > > I wonder that maybe we need to submit a patch to let xfstest understand > > that a filesystem supports extents or not because after applied this > > patch indirect-based file in ext4 has supported seek_data/hole and hole > > punching. I usually run xfstest automatically, and every time I need > > to check the result of #255 and #285 manually. That is annoying for me. > > I would think the right thing to do is to have xfstests make sure it > understands that fallocate working with FALLOC_FL_PUNCH_HOLE does not > imply that fallocate without the FALLOC_FL_PUNCH_HOLE flag OR'ed in > will work. We already have this capabiity in xfstests via _require_xfs_io_falloc_punch and _require_xfs_io_falloc. That, however, doesn't mean the tests that use these calls do the correct requirement checks. That's the problem with 255 - it doesn't call _require_xfs_io_falloc. As to 285, the seek_sanity_test does it's own check for seek hole/data support, and error out if it fails. This needs to be turned into an equivalent _require_seek_hole_data (e.g. by running "seek_sanity_test -t" to test for support) and 285 needs to the call the _require_seek_hole_data before running the test proper. Please send patches to xfs@oss.sgi.com.... Cheers, Dave.
On Mon, Jan 28, 2013 at 01:04:42PM +0800, Zheng Liu wrote: > > Sorry, my apology. I found a bug in indirect-based hole punching patch > when I tried to create a new test case in xfstest. A patch has been > sent out. Please look at it. Ah yes, this must be the patch found at: http://patchwork.ozlabs.org/patch/216105/ It would it make sense for me to fold this into the punch hole patch, yes? (Since I haven't pushed out the patch yet, I might as well just fold in the bugfix into the feature patch.) - Ted -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Jan 24, 2013 at 10:32:01PM -0500, Theodore Ts'o wrote: > On Fri, Jan 18, 2013 at 03:16:21PM +0800, Zheng Liu wrote: > > From: Zheng Liu <wenqing.lz@taobao.com> > > > > After adding indirect punching hole feature, we need to enable it in fallocate. > > For this purpose, some sanity checks need to be adjusted. Currently we need to > > check FALLOC_FL_PUNCH_HOLE flag before other sanity checks. > > I've folded these two patches into one since the first patch in this > series since there won't be any way to exercise the new code paths > until we enable this in fallocate(), and it doesn't really change > existing code paths. If it did make huge changes in the normal code > path, it might be useful to keep the two commits separate, but that's > not the case here. > > Anyway, they look good so I've checked them into my tree. I'm > currently kicking off a test run to make sure there aren't any > problems except for the test 255 failure when testing w/o extents, but > I don't really anticipate any issues. Hi Ted, Sorry, my apology. I found a bug in indirect-based hole punching patch when I tried to create a new test case in xfstest. A patch has been sent out. Please look at it. Thanks, - Zheng -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sun, Jan 27, 2013 at 11:57:13PM -0500, Theodore Ts'o wrote: > On Mon, Jan 28, 2013 at 01:04:42PM +0800, Zheng Liu wrote: > > > > Sorry, my apology. I found a bug in indirect-based hole punching patch > > when I tried to create a new test case in xfstest. A patch has been > > sent out. Please look at it. > > Ah yes, this must be the patch found at: > > http://patchwork.ozlabs.org/patch/216105/ Yes, it is. > > It would it make sense for me to fold this into the punch hole patch, > yes? (Since I haven't pushed out the patch yet, I might as well just > fold in the bugfix into the feature patch.) No problem. Thanks, - Zheng -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c index 5ae1674..76643fd 100644 --- a/fs/ext4/extents.c +++ b/fs/ext4/extents.c @@ -4397,13 +4397,6 @@ long ext4_fallocate(struct file *file, int mode, loff_t offset, loff_t len) struct ext4_map_blocks map; unsigned int credits, blkbits = inode->i_blkbits; - /* - * currently supporting (pre)allocate mode for extent-based - * files _only_ - */ - if (!(ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS))) - return -EOPNOTSUPP; - /* Return error if mode is not supported */ if (mode & ~(FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE)) return -EOPNOTSUPP; @@ -4415,6 +4408,13 @@ long ext4_fallocate(struct file *file, int mode, loff_t offset, loff_t len) if (ret) return ret; + /* + * currently supporting (pre)allocate mode for extent-based + * files _only_ + */ + if (!(ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS))) + return -EOPNOTSUPP; + trace_ext4_fallocate_enter(inode, offset, len, mode); map.m_lblk = offset >> blkbits; /*