| Message ID | 5125ED66.4060405@cn.fujitsu.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Feb 21, 2013 at 02:48:22AM -0700, Miao Xie wrote: > When running the 083th case of xfstests on the filesystem with > "compress-force=lzo", the following WARNINGs were triggered. > WARNING: at fs/btrfs/inode.c:7908 > WARNING: at fs/btrfs/inode.c:7909 > WARNING: at fs/btrfs/inode.c:7911 > WARNING: at fs/btrfs/extent-tree.c:4510 > WARNING: at fs/btrfs/extent-tree.c:4511 > > This problem was introduced by the patch "Btrfs: fix deadlock due > to unsubmitted". In this patch, there are two bugs which caused > the above problem. I saw this as well on test 132 last night. My plan was to track it down this morning, so discovering it already fixed while I slept was wonderful. Thanks Miao. Josef I've got this one and Miao's defrag unmount patch queued up. -chris -- 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
On Thu, Feb 21, 2013 at 7:26 AM, Chris Mason <chris.mason@fusionio.com> wrote: > On Thu, Feb 21, 2013 at 02:48:22AM -0700, Miao Xie wrote: >> When running the 083th case of xfstests on the filesystem with >> "compress-force=lzo", the following WARNINGs were triggered. >> WARNING: at fs/btrfs/inode.c:7908 >> WARNING: at fs/btrfs/inode.c:7909 >> WARNING: at fs/btrfs/inode.c:7911 >> WARNING: at fs/btrfs/extent-tree.c:4510 >> WARNING: at fs/btrfs/extent-tree.c:4511 >> >> This problem was introduced by the patch "Btrfs: fix deadlock due >> to unsubmitted". In this patch, there are two bugs which caused >> the above problem. > > I saw this as well on test 132 last night. My plan was to track it down > this morning, so discovering it already fixed while I slept was > wonderful. > > Thanks Miao. Josef I've got this one and Miao's defrag unmount patch > queued up. > Thanks, I've also tested this patch, and it cleared the error I was receiving. -- 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
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c index b009fb5..9a1cc04 100644 --- a/fs/btrfs/inode.c +++ b/fs/btrfs/inode.c @@ -6067,12 +6067,9 @@ static int btrfs_get_blocks_direct(struct inode *inode, sector_t iblock, int unlock_bits = EXTENT_LOCKED; int ret = 0; - if (create) { - spin_lock(&BTRFS_I(inode)->lock); - BTRFS_I(inode)->outstanding_extents++; - spin_unlock(&BTRFS_I(inode)->lock); + if (create) unlock_bits |= EXTENT_DELALLOC | EXTENT_DIRTY; - } else + else len = min_t(u64, len, root->sectorsize); lockstart = start; @@ -6214,6 +6211,10 @@ unlock: if (start + len > i_size_read(inode)) i_size_write(inode, start + len); + spin_lock(&BTRFS_I(inode)->lock); + BTRFS_I(inode)->outstanding_extents++; + spin_unlock(&BTRFS_I(inode)->lock); + ret = set_extent_bit(&BTRFS_I(inode)->io_tree, lockstart, lockstart + len - 1, EXTENT_DELALLOC, NULL, &cached_state, GFP_NOFS); @@ -6716,14 +6717,11 @@ static ssize_t btrfs_direct_IO(int rw, struct kiocb *iocb, if (rw & WRITE) { if (ret < 0 && ret != -EIOCBQUEUED) btrfs_delalloc_release_space(inode, count); - else if (ret > 0 && (size_t)ret < count) { - spin_lock(&BTRFS_I(inode)->lock); - BTRFS_I(inode)->outstanding_extents++; - spin_unlock(&BTRFS_I(inode)->lock); + else if (ret >= 0 && (size_t)ret < count) btrfs_delalloc_release_space(inode, count - (size_t)ret); - } - btrfs_delalloc_release_metadata(inode, 0); + else + btrfs_delalloc_release_metadata(inode, 0); } out: if (wakeup)
When running the 083th case of xfstests on the filesystem with "compress-force=lzo", the following WARNINGs were triggered. WARNING: at fs/btrfs/inode.c:7908 WARNING: at fs/btrfs/inode.c:7909 WARNING: at fs/btrfs/inode.c:7911 WARNING: at fs/btrfs/extent-tree.c:4510 WARNING: at fs/btrfs/extent-tree.c:4511 This problem was introduced by the patch "Btrfs: fix deadlock due to unsubmitted". In this patch, there are two bugs which caused the above problem. The 1st one is a off-by-one bug, if the DIO write return 0, it is also a short write, we need release the reserved space for it. But we didn't do it in that patch. Fix it by change "ret > 0" to "ret >= 0". The 2nd one is ->outstanding_extents was increased twice when a short write happened. As we know, ->outstanding_extents is a counter to keep track of the number of extent items we may use duo to delalloc, when we reserve the free space for a delalloc write, we assume that the write will introduce just one extent item, so we increase ->outstanding_extents by 1 at that time. And then we will increase it every time we split the write, it is done at the beginning of btrfs_get_blocks_direct(). So when a short write happens, we needn't increase ->outstanding_extents again. But this patch done. In order to fix the 2nd problem, I re-write the logic for ->outstanding_extents operation. We don't increase it at the beginning of btrfs_get_blocks_direct(), instead, we just increase it when the split actually happens. Reported-by: Mitch Harder <mitch.harder@sabayonlinux.org> Signed-off-by: Miao Xie <miaox@cn.fujitsu.com> --- fs/btrfs/inode.c | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-)