| Message ID | 1345615545-26133-4-git-send-email-wenqing.lz@taobao.com |
|---|---|
| State | Accepted, archived |
| Headers | show |
On Wed, Aug 22, 2012 at 02:05:40PM +0800, Zheng Liu wrote: > From: Zheng Liu <wenqing.lz@taobao.com> > > Let ext4 initialize extent status tree of an inode. > > Signed-off-by: Yongqiang Yang <xiaoqiangnk@gmail.com> > Signed-off-by: Allison Henderson <achender@linux.vnet.ibm.com> > Signed-off-by: Zheng Liu <wenqing.lz@taobao.com> One general comment --- "Signed-off-by" should only be used when someone actually handles a patch; that is, you add the Signed-off-by for yourself if you originally wrote the patch, or if you started with someone else's patch and modified it. If you merely reviewed the patch, you can request that the patch author add a "Reviewed-by: " header. The other footer that you will sometimes see is "Acked-by: ". This gets used by subsystem maintainers who approve of a patch, but instead of their handling the patch, they are going to let the patch flow into the upstream via some other tree. So for example, Lukas's invalidate_page_range patch set will require an "Acked-by: " footer from the XFS, OCFS2, and mm maintainers for those commits which touch their subsystems. Once we've gotten an OK from those folks, then when I accept those patches into the ext4 tree, I will add my "Signed-off-by: " header since the patches flowed through my tree. These are minor points, but Linus will sometimes complain if these footers aren't used correctly, since Signed-off-by: does have a very specific legal meaning. See: http://elinux.org/Developer_Certificate_Of_Origin In any case, I've changed the Signed-off-by footers for Yongqiang Yang and Allison Heanderson to be "Reviewed-by: ", in the commit descriptions, since I believe that should be the more appropriate footer to be using in this case. (If you did actually use some code from Yongqiang or Allison from patches which they had previously sent with a DCO, then Signed-off-by would be correct. Let me know if that's the case....) Regards, - 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 Wed, Aug 22, 2012 at 02:05:40PM +0800, Zheng Liu wrote: > diff --git a/fs/ext4/super.c b/fs/ext4/super.c > index 3e0851e..353b1fd 100644 > --- a/fs/ext4/super.c > +++ b/fs/ext4/super.c > @@ -944,6 +944,7 @@ static struct inode *ext4_alloc_inode(struct super_block *sb) > memset(&ei->i_cached_extent, 0, sizeof(struct ext4_ext_cache)); > INIT_LIST_HEAD(&ei->i_prealloc_list); > spin_lock_init(&ei->i_prealloc_lock); > + ext4_es_init_tree(&ei->i_es_tree); > ei->i_reserved_data_blocks = 0; > ei->i_reserved_meta_blocks = 0; > ei->i_allocated_meta_blocks = 0; This patch hunk immediately me ask, "so when does the extent_status tree get freed?" And I believe the answer is that currently, since it only tracks delayed extents (and we're not using it for locking purposes), by the time we have evicted the inode and are ready to call ext4_clear_inode(), we should have released all of the nodes in the ext4_es_tree. Is that correct? If so, we might want to think about adding a sanity check to make sure that by the time we are done with the inode in ext4_evict_inode() (after we have forced writeback), the ext4_es_tree is empty. Agreed? - 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 Wed, Sep 19, 2012 at 03:05:41PM -0400, Theodore Ts'o wrote: > On Wed, Aug 22, 2012 at 02:05:40PM +0800, Zheng Liu wrote: > > diff --git a/fs/ext4/super.c b/fs/ext4/super.c > > index 3e0851e..353b1fd 100644 > > --- a/fs/ext4/super.c > > +++ b/fs/ext4/super.c > > @@ -944,6 +944,7 @@ static struct inode *ext4_alloc_inode(struct super_block *sb) > > memset(&ei->i_cached_extent, 0, sizeof(struct ext4_ext_cache)); > > INIT_LIST_HEAD(&ei->i_prealloc_list); > > spin_lock_init(&ei->i_prealloc_lock); > > + ext4_es_init_tree(&ei->i_es_tree); > > ei->i_reserved_data_blocks = 0; > > ei->i_reserved_meta_blocks = 0; > > ei->i_allocated_meta_blocks = 0; > > This patch hunk immediately me ask, "so when does the extent_status > tree get freed?" And I believe the answer is that currently, since it > only tracks delayed extents (and we're not using it for locking > purposes), by the time we have evicted the inode and are ready to call > ext4_clear_inode(), we should have released all of the nodes in the > ext4_es_tree. Is that correct? > > If so, we might want to think about adding a sanity check to make sure > that by the time we are done with the inode in ext4_evict_inode() > (after we have forced writeback), the ext4_es_tree is empty. Agreed? Yes, I agree with you. I will add a sanity check. :-) 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 Wed, Sep 19, 2012 at 03:05:41PM -0400, Theodore Ts'o wrote: > On Wed, Aug 22, 2012 at 02:05:40PM +0800, Zheng Liu wrote: > > diff --git a/fs/ext4/super.c b/fs/ext4/super.c > > index 3e0851e..353b1fd 100644 > > --- a/fs/ext4/super.c > > +++ b/fs/ext4/super.c > > @@ -944,6 +944,7 @@ static struct inode *ext4_alloc_inode(struct super_block *sb) > > memset(&ei->i_cached_extent, 0, sizeof(struct ext4_ext_cache)); > > INIT_LIST_HEAD(&ei->i_prealloc_list); > > spin_lock_init(&ei->i_prealloc_lock); > > + ext4_es_init_tree(&ei->i_es_tree); > > ei->i_reserved_data_blocks = 0; > > ei->i_reserved_meta_blocks = 0; > > ei->i_allocated_meta_blocks = 0; > > This patch hunk immediately me ask, "so when does the extent_status > tree get freed?" And I believe the answer is that currently, since it > only tracks delayed extents (and we're not using it for locking > purposes), by the time we have evicted the inode and are ready to call > ext4_clear_inode(), we should have released all of the nodes in the > ext4_es_tree. Is that correct? > > If so, we might want to think about adding a sanity check to make sure > that by the time we are done with the inode in ext4_evict_inode() > (after we have forced writeback), the ext4_es_tree is empty. Agreed? Hi Ted, Today I revise this patch again, and I find extent_status_tree is freed in ext4_clear_inode(). So maybe I don't think that we need to check this tree to be freed in ext4_evict_inode(). This change is in this patch '[RFC][PATCH 4/8 v2] ext4: let ext4 maintain extent status tree'. What's your opinion? 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 Tue, Sep 25, 2012 at 08:42:52PM +0800, Zheng Liu wrote: > > If so, we might want to think about adding a sanity check to make sure > > that by the time we are done with the inode in ext4_evict_inode() > > (after we have forced writeback), the ext4_es_tree is empty. Agreed? > > Today I revise this patch again, and I find extent_status_tree is freed > in ext4_clear_inode(). So maybe I don't think that we need to check > this tree to be freed in ext4_evict_inode(). This change is in this > patch '[RFC][PATCH 4/8 v2] ext4: let ext4 maintain extent status tree'. > What's your opinion? When you say "revise this patch again", does that mean that you would like to submit a new set of patch series with changes? Or just that you are looking at this patch set again? It's certainly true that ext4_evict_inode() will call ext4_clear_inode(), so it's not a question of worrying about a memory leak. I was thinking more about doing this as a cheap sanity check for the data structure. By the time we call ext4_evict_inode(), the mm layer all writeback should be complete. Hence, all of the entries to the tree _should_ have been removed by the time we call ext4_evict_inode(). I don't know if this is going to change as you start using this data structure for other purposes (such as locking a range of pages), but if I understand how things are currently working, it _should_ be the case that when ext4_evict_inode() calls ext4_clear_inode(), the call to ext4_es_remove_extent() should be a no-op, since all of the nodes in the extent status tree should have been released by then. If it isn't, then either I'm not understanding the code, or there's a bug in the code. - 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 Tue, Sep 25, 2012 at 04:59:21PM -0400, Theodore Ts'o wrote: > On Tue, Sep 25, 2012 at 08:42:52PM +0800, Zheng Liu wrote: > > > If so, we might want to think about adding a sanity check to make sure > > > that by the time we are done with the inode in ext4_evict_inode() > > > (after we have forced writeback), the ext4_es_tree is empty. Agreed? > > > > Today I revise this patch again, and I find extent_status_tree is freed > > in ext4_clear_inode(). So maybe I don't think that we need to check > > this tree to be freed in ext4_evict_inode(). This change is in this > > patch '[RFC][PATCH 4/8 v2] ext4: let ext4 maintain extent status tree'. > > What's your opinion? > > When you say "revise this patch again", does that mean that you would > like to submit a new set of patch series with changes? Or just that > you are looking at this patch set again? Yes, I prepare to submit a new patch set. > > It's certainly true that ext4_evict_inode() will call > ext4_clear_inode(), so it's not a question of worrying about a memory > leak. I was thinking more about doing this as a cheap sanity check > for the data structure. By the time we call ext4_evict_inode(), the > mm layer all writeback should be complete. Hence, all of the entries > to the tree _should_ have been removed by the time we call > ext4_evict_inode(). > > I don't know if this is going to change as you start using this data > structure for other purposes (such as locking a range of pages), but > if I understand how things are currently working, it _should_ be the > case that when ext4_evict_inode() calls ext4_clear_inode(), the call > to ext4_es_remove_extent() should be a no-op, since all of the nodes > in the extent status tree should have been released by then. If it > isn't, then either I'm not understanding the code, or there's a bug in > the code. Yes, you are right. In currently implementation, extent status tree only maintains the status of delay extents. So in ext4_evict_inode() extent status tree should be an empty tree. I will add a sanity check to ensure it. Thanks for your explanation. 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 Wed, Sep 26, 2012 at 10:09:55AM +0800, Zheng Liu wrote: > On Tue, Sep 25, 2012 at 04:59:21PM -0400, Theodore Ts'o wrote: > > On Tue, Sep 25, 2012 at 08:42:52PM +0800, Zheng Liu wrote: > > > > If so, we might want to think about adding a sanity check to make sure > > > > that by the time we are done with the inode in ext4_evict_inode() > > > > (after we have forced writeback), the ext4_es_tree is empty. Agreed? > > > > > > Today I revise this patch again, and I find extent_status_tree is freed > > > in ext4_clear_inode(). So maybe I don't think that we need to check > > > this tree to be freed in ext4_evict_inode(). This change is in this > > > patch '[RFC][PATCH 4/8 v2] ext4: let ext4 maintain extent status tree'. > > > What's your opinion? > > > > When you say "revise this patch again", does that mean that you would > > like to submit a new set of patch series with changes? Or just that > > you are looking at this patch set again? > > Yes, I prepare to submit a new patch set. Well, note that the merge window is opening *soon*. I haven't yet moved the master branch, so I can update the patch set, but I'm going to need it soon. Can you let me know what changes you need to make? If it is to add new features or new sanity checks, does it make sense to simply make it as new commits to existing patch set? Or are there fundamental problems with the current set, that would be better to fix in the current set of commits? (Or is it just minor stylistic/spelling fixes?) 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 Tue, Sep 25, 2012 at 10:47:45PM -0400, Theodore Ts'o wrote: > On Wed, Sep 26, 2012 at 10:09:55AM +0800, Zheng Liu wrote: > > On Tue, Sep 25, 2012 at 04:59:21PM -0400, Theodore Ts'o wrote: > > > On Tue, Sep 25, 2012 at 08:42:52PM +0800, Zheng Liu wrote: > > > > > If so, we might want to think about adding a sanity check to make sure > > > > > that by the time we are done with the inode in ext4_evict_inode() > > > > > (after we have forced writeback), the ext4_es_tree is empty. Agreed? > > > > > > > > Today I revise this patch again, and I find extent_status_tree is freed > > > > in ext4_clear_inode(). So maybe I don't think that we need to check > > > > this tree to be freed in ext4_evict_inode(). This change is in this > > > > patch '[RFC][PATCH 4/8 v2] ext4: let ext4 maintain extent status tree'. > > > > What's your opinion? > > > > > > When you say "revise this patch again", does that mean that you would > > > like to submit a new set of patch series with changes? Or just that > > > you are looking at this patch set again? > > > > Yes, I prepare to submit a new patch set. > > Well, note that the merge window is opening *soon*. I haven't yet > moved the master branch, so I can update the patch set, but I'm going > to need it soon. > > Can you let me know what changes you need to make? If it is to add > new features or new sanity checks, does it make sense to simply make > it as new commits to existing patch set? Or are there fundamental > problems with the current set, that would be better to fix in the > current set of commits? (Or is it just minor stylistic/spelling > fixes?) > > Thanks!! In new patch set, there is three changes as beblow: 1. add a sanity check in ext4_evict_inode() 2. fix a bug in ext4_find_delalloc_range(). This bug is reported by xfstest #230 when we enable bigalloc feature. 3. Add a new rwlock to protect extent status tree. So I think that we can only add a sanity check and fix the bigalloc bug, and then apply this patch set because the changes are minor. For adding a new lock to protect extent status tree, we can add this feature in a new patch. If you think it is OK, I can generate a new patch set, do some tests using xfstest, and submit it as soon as possible. What's your opinion? 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 Wed, Sep 26, 2012 at 11:24:26AM +0800, Zheng Liu wrote: > > Can you let me know what changes you need to make? If it is to add > > new features or new sanity checks, does it make sense to simply make > > it as new commits to existing patch set? Or are there fundamental > > problems with the current set, that would be better to fix in the > > current set of commits? (Or is it just minor stylistic/spelling > > fixes?) > > > > Thanks!! > > In new patch set, there is three changes as beblow: > > 1. add a sanity check in ext4_evict_inode() > 2. fix a bug in ext4_find_delalloc_range(). This bug is reported by > xfstest #230 when we enable bigalloc feature. > 3. Add a new rwlock to protect extent status tree. > > So I think that we can only add a sanity check and fix the bigalloc bug, > and then apply this patch set because the changes are minor. For adding > a new lock to protect extent status tree, we can add this feature in a > new patch. If you think it is OK, I can generate a new patch set, do > some tests using xfstest, and submit it as soon as possible. What's > your opinion? Do you think you can get me the patches by the end of the week? If so, that should work. 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 Wed, Sep 26, 2012 at 11:54:48AM +0800, Zheng Liu wrote: > > Before this weekend, I can finish a new patch set with (1) adding a sanity > check and (2) fixing bigalloc bug. For adding a new lock, it is > unstable now. So this lock will not be included in this patch set. That sounds good to me. Adding a rw lock is something we can do in a separate patch (which might not make the merge window for 3.7, depending on when Linus decides to release 3.6, but that's OK). - 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 Tue, Sep 25, 2012 at 11:37:40PM -0400, Theodore Ts'o wrote: > On Wed, Sep 26, 2012 at 11:24:26AM +0800, Zheng Liu wrote: > > > Can you let me know what changes you need to make? If it is to add > > > new features or new sanity checks, does it make sense to simply make > > > it as new commits to existing patch set? Or are there fundamental > > > problems with the current set, that would be better to fix in the > > > current set of commits? (Or is it just minor stylistic/spelling > > > fixes?) > > > > > > Thanks!! > > > > In new patch set, there is three changes as beblow: > > > > 1. add a sanity check in ext4_evict_inode() > > 2. fix a bug in ext4_find_delalloc_range(). This bug is reported by > > xfstest #230 when we enable bigalloc feature. > > 3. Add a new rwlock to protect extent status tree. > > > > So I think that we can only add a sanity check and fix the bigalloc bug, > > and then apply this patch set because the changes are minor. For adding > > a new lock to protect extent status tree, we can add this feature in a > > new patch. If you think it is OK, I can generate a new patch set, do > > some tests using xfstest, and submit it as soon as possible. What's > > your opinion? > > Do you think you can get me the patches by the end of the week? If > so, that should work. Before this weekend, I can finish a new patch set with (1) adding a sanity check and (2) fixing bigalloc bug. For adding a new lock, it is unstable now. So this lock will not be included in this patch set. 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 Tue, Sep 25, 2012 at 11:46:37PM -0400, Theodore Ts'o wrote: > On Wed, Sep 26, 2012 at 11:54:48AM +0800, Zheng Liu wrote: > > > > Before this weekend, I can finish a new patch set with (1) adding a sanity > > check and (2) fixing bigalloc bug. For adding a new lock, it is > > unstable now. So this lock will not be included in this patch set. > > That sounds good to me. Adding a rw lock is something we can do in a > separate patch (which might not make the merge window for 3.7, > depending on when Linus decides to release 3.6, but that's OK). OK, I will submit a new patch set ASAP. 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 Tue, Sep 25, 2012 at 04:59:21PM -0400, Theodore Ts'o wrote: > On Tue, Sep 25, 2012 at 08:42:52PM +0800, Zheng Liu wrote: > > > If so, we might want to think about adding a sanity check to make sure > > > that by the time we are done with the inode in ext4_evict_inode() > > > (after we have forced writeback), the ext4_es_tree is empty. Agreed? > > > > Today I revise this patch again, and I find extent_status_tree is freed > > in ext4_clear_inode(). So maybe I don't think that we need to check > > this tree to be freed in ext4_evict_inode(). This change is in this > > patch '[RFC][PATCH 4/8 v2] ext4: let ext4 maintain extent status tree'. > > What's your opinion? > > When you say "revise this patch again", does that mean that you would > like to submit a new set of patch series with changes? Or just that > you are looking at this patch set again? > > It's certainly true that ext4_evict_inode() will call > ext4_clear_inode(), so it's not a question of worrying about a memory > leak. I was thinking more about doing this as a cheap sanity check > for the data structure. By the time we call ext4_evict_inode(), the > mm layer all writeback should be complete. Hence, all of the entries > to the tree _should_ have been removed by the time we call > ext4_evict_inode(). > > I don't know if this is going to change as you start using this data > structure for other purposes (such as locking a range of pages), but > if I understand how things are currently working, it _should_ be the > case that when ext4_evict_inode() calls ext4_clear_inode(), the call > to ext4_es_remove_extent() should be a no-op, since all of the nodes > in the extent status tree should have been released by then. If it > isn't, then either I'm not understanding the code, or there's a bug in > the code. Hi Ted, When I try to add a BUG_ON in ext4_evict_inode() to ensure that extent status tree is empty, I will get an error, which reports that the tree is not empty. I find that there still has some dirty pages when we call ext4_evict_inode() because vfs doesn't write all dirty pages back. In iput_final(), we firstly call ext4_drop_inode(). If the return value is true, we won't call write_inode_now() to do a writeback. It means that we write some data to a file and remove it immediately, and then all dirty pages that aren't during writeback progress won't be written. These pages will be truncated in ext4_evict_inode(). Thus, the extent status tree is possible not to be empty when we call ext4_evict_inode(). So we cannot add a sanity check in this function. Am I missing something? 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 Tue, Sep 25, 2012 at 11:37:40PM -0400, Theodore Ts'o wrote: > On Wed, Sep 26, 2012 at 11:24:26AM +0800, Zheng Liu wrote: > > > Can you let me know what changes you need to make? If it is to add > > > new features or new sanity checks, does it make sense to simply make > > > it as new commits to existing patch set? Or are there fundamental > > > problems with the current set, that would be better to fix in the > > > current set of commits? (Or is it just minor stylistic/spelling > > > fixes?) > > > > > > Thanks!! > > > > In new patch set, there is three changes as beblow: > > > > 1. add a sanity check in ext4_evict_inode() > > 2. fix a bug in ext4_find_delalloc_range(). This bug is reported by > > xfstest #230 when we enable bigalloc feature. > > 3. Add a new rwlock to protect extent status tree. > > > > So I think that we can only add a sanity check and fix the bigalloc bug, > > and then apply this patch set because the changes are minor. For adding > > a new lock to protect extent status tree, we can add this feature in a > > new patch. If you think it is OK, I can generate a new patch set, do > > some tests using xfstest, and submit it as soon as possible. What's > > your opinion? > > Do you think you can get me the patches by the end of the week? If > so, that should work. Hi Ted, Until now, I have fixed the bigalloc bug that is reported by xfstest #230, and merged Hugh's patch. But I do really think that this patch set couldn't be applied at this merge window because the change is not *minor*, and it still needs to do more tests. That would be great if you can keep this patch set in dev branch at this merge window. Thanks! 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, Sep 28, 2012 at 03:27:14PM +0800, Zheng Liu wrote: > > Until now, I have fixed the bigalloc bug that is reported by xfstest > #230, and merged Hugh's patch. But I do really think that this patch > set couldn't be applied at this merge window because the change is not > *minor*, and it still needs to do more tests. That would be great if > you can keep this patch set in dev branch at this merge window. Thanks! The dev branch is the set of patches that are planned to go to Linus during the next merge window, so if we drop it from the merge window, I would drop it from the dev branch and put it in the "unstable" portion of the patch series. It would be a shame to drop it since this provides the SEEK_HOLE capability, though. Can you say more about which change is not minor? The change to fix the bigalloc bug? Or the whole patch series? - 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 Sat, Sep 29, 2012 at 08:54:47AM +0800, Zheng Liu wrote: > > When I try to fix the bigalloc bug, some code that operates on extent > status tree and maintains its status are changed when I do some changes in > ext4_find_delalloc_range(). So that quite has a lot of changes I thought > in the whole patch series. So I think that we'd better drop this patch set > from dev branch. Thanks. OK, dropped. - 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
[Sorry, I got an delivery error when the original mail is sent to the mailing list. Cc' to them] On Saturday, September 29, 2012, Theodore Ts'o wrote: > > On Fri, Sep 28, 2012 at 03:27:14PM +0800, Zheng Liu wrote: > > > > Until now, I have fixed the bigalloc bug that is reported by xfstest > > #230, and merged Hugh's patch. But I do really think that this patch > > set couldn't be applied at this merge window because the change is not > > *minor*, and it still needs to do more tests. That would be great if > > you can keep this patch set in dev branch at this merge window. Thanks! > > The dev branch is the set of patches that are planned to go to Linus > during the next merge window, so if we drop it from the merge window, > I would drop it from the dev branch and put it in the "unstable" > portion of the patch series. > > It would be a shame to drop it since this provides the SEEK_HOLE > capability, though. Can you say more about which change is not minor? > The change to fix the bigalloc bug? Or the whole patch series? > > Hi Ted, When I try to fix the bigalloc bug, some code that operates on extent status tree and maintains its status are changed when I do some changes in ext4_find_delalloc_range(). So that quite has a lot of changes I thought in the whole patch series. So I think that we'd better drop this patch set from dev branch. Thanks. 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 Sat, Sep 29, 2012 at 11:07 AM, Theodore Ts'o <tytso@mit.edu> wrote: > On Sat, Sep 29, 2012 at 08:54:47AM +0800, Zheng Liu wrote: >> >> When I try to fix the bigalloc bug, some code that operates on extent >> status tree and maintains its status are changed when I do some changes in >> ext4_find_delalloc_range(). So that quite has a lot of changes I thought >> in the whole patch series. So I think that we'd better drop this patch set >> from dev branch. Thanks. > > OK, dropped. Thank you very much. I will add rwlock to protect this tree, and do more tests for this patch set. 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, 28 Sep 2012 23:07:59 -0400, Theodore Ts'o <tytso@mit.edu> wrote: > On Sat, Sep 29, 2012 at 08:54:47AM +0800, Zheng Liu wrote: > > > > When I try to fix the bigalloc bug, some code that operates on extent > > status tree and maintains its status are changed when I do some changes in > > ext4_find_delalloc_range(). So that quite has a lot of changes I thought > > in the whole patch series. So I think that we'd better drop this patch set > > from dev branch. Thanks. > > OK, dropped. After patch-set was dropped, 269'th succeed. Both warnings: ext4_convert_unwritten_extent which fail with ENOSPC and WARNING: at fs/ext4/inode.c:362 ext4_da_update_reserve_space are gone away. > > - Ted > -- > 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 -- 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/super.c b/fs/ext4/super.c index 3e0851e..353b1fd 100644 --- a/fs/ext4/super.c +++ b/fs/ext4/super.c @@ -944,6 +944,7 @@ static struct inode *ext4_alloc_inode(struct super_block *sb) memset(&ei->i_cached_extent, 0, sizeof(struct ext4_ext_cache)); INIT_LIST_HEAD(&ei->i_prealloc_list); spin_lock_init(&ei->i_prealloc_lock); + ext4_es_init_tree(&ei->i_es_tree); ei->i_reserved_data_blocks = 0; ei->i_reserved_meta_blocks = 0; ei->i_allocated_meta_blocks = 0;