On Fri, Aug 02, 2013 at 06:08:01PM +0800, Miao Xie wrote: > Hi, Josef > > On Tue, 30 Jul 2013 14:27:40 +0800, Miao Xie wrote: > >> extent_clear_unlock_delalloc(inode, start, end, NULL, > >> - EXTENT_DIRTY | > >> - EXTENT_DELALLOC, > >> + EXTENT_DELALLOC | > >> + EXTENT_DO_ACCOUNTING | > >> + EXTENT_DEFRAG, > >> PAGE_UNLOCK | > >> PAGE_CLEAR_DIRTY | > >> PAGE_SET_WRITEBACK | > > > > I found we released the reserved space in cow_file_range_inline(), but at that time, > > we didn't drop the outstanding_extents counter by the number of the delalloc extents, > > so it might leave some reserved space which was not released. So I think we should remove > > the release function in cow_file_range_inline(). > > > > (This bug is not introduced by your patch. but if we don't fix the above problem before > > applying your patch, the double release would happen because we have released the space > > in cow_file_range_inline()) > > I'm sorry that I made a mistake, the problem I said above doesn't exist because the block > size is equal to the page size, and we can only inline the file extent which is <= the page size, > so it is unlikely that one inline extent has two extent state objects. > > But the double release of the reserved space exists actually. We can fix by removing the release > function in cow_file_range_inline() and passing EXTENT_DO_ACCOUNTING to the clear function. > Sorry I meant to send a v2, but what I've committed isn't actually this. We need DO_ACCOUNTING if ret < 0 but not if ret == 0, so I've changed this to take that into account since cow_file_range_inline does do the accounting as you say. Thanks, Josef -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
