diff mbox

Btrfs: fix truncate down when no_holes feature is enabled

Message ID 1478903265-24273-1-git-send-email-bo.li.liu@oracle.com (mailing list archive)
State Superseded
Headers show

Commit Message

Liu Bo Nov. 11, 2016, 10:27 p.m. UTC
For such a file mapping,

[0-4k][hole][8k-12k]

In NO_HOLES mode, we don't have the [hole] extent any more.
Commit c1aa45759e90 ("Btrfs: fix shrinking truncate when the no_holes feature is enabled")
 fixed disk isize not being updated in NO_HOLES mode when data is not flushed.

However, even if data has been flushed, we can still have trouble
in updating disk isize since we updated disk isize to 'start' of
the last evicted extent.

Also add a ASSERT for better catching (for developers only).

Signed-off-by: Liu Bo <bo.li.liu@oracle.com>
---
 fs/btrfs/inode.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

Comments

Chris Mason Nov. 22, 2016, 7:13 p.m. UTC | #1
On 11/11/2016 05:27 PM, Liu Bo wrote:
> For such a file mapping,
>
> [0-4k][hole][8k-12k]
>
> In NO_HOLES mode, we don't have the [hole] extent any more.
> Commit c1aa45759e90 ("Btrfs: fix shrinking truncate when the no_holes feature is enabled")
>  fixed disk isize not being updated in NO_HOLES mode when data is not flushed.
>
> However, even if data has been flushed, we can still have trouble
> in updating disk isize since we updated disk isize to 'start' of
> the last evicted extent.
>
> Also add a ASSERT for better catching (for developers only).
>
> Signed-off-by: Liu Bo <bo.li.liu@oracle.com>

Thanks Liu!

Reviewed-by: Chris Mason <clm@fb.com>
--
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
Liu Bo Nov. 22, 2016, 8:08 p.m. UTC | #2
On Tue, Nov 22, 2016 at 02:13:21PM -0500, Chris Mason wrote:
> On 11/11/2016 05:27 PM, Liu Bo wrote:
> > For such a file mapping,
> > 
> > [0-4k][hole][8k-12k]
> > 
> > In NO_HOLES mode, we don't have the [hole] extent any more.
> > Commit c1aa45759e90 ("Btrfs: fix shrinking truncate when the no_holes feature is enabled")
> >  fixed disk isize not being updated in NO_HOLES mode when data is not flushed.
> > 
> > However, even if data has been flushed, we can still have trouble
> > in updating disk isize since we updated disk isize to 'start' of
> > the last evicted extent.
> > 
> > Also add a ASSERT for better catching (for developers only).
> > 
> > Signed-off-by: Liu Bo <bo.li.liu@oracle.com>
> 
> Thanks Liu!
> 
> Reviewed-by: Chris Mason <clm@fb.com>

The added ASSERT() can cause generic/068 to crash because I didn't
filter out the 'log tree' case, I'm testing a V2 patch.

Thanks,

-liubo
--
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
David Sterba Nov. 23, 2016, 4:55 p.m. UTC | #3
On Tue, Nov 22, 2016 at 12:08:47PM -0800, Liu Bo wrote:
> On Tue, Nov 22, 2016 at 02:13:21PM -0500, Chris Mason wrote:
> > On 11/11/2016 05:27 PM, Liu Bo wrote:
> > > For such a file mapping,
> > > 
> > > [0-4k][hole][8k-12k]
> > > 
> > > In NO_HOLES mode, we don't have the [hole] extent any more.
> > > Commit c1aa45759e90 ("Btrfs: fix shrinking truncate when the no_holes feature is enabled")
> > >  fixed disk isize not being updated in NO_HOLES mode when data is not flushed.
> > > 
> > > However, even if data has been flushed, we can still have trouble
> > > in updating disk isize since we updated disk isize to 'start' of
> > > the last evicted extent.
> > > 
> > > Also add a ASSERT for better catching (for developers only).
> > > 
> > > Signed-off-by: Liu Bo <bo.li.liu@oracle.com>
> > 
> > Thanks Liu!
> > 
> > Reviewed-by: Chris Mason <clm@fb.com>
> 
> The added ASSERT() can cause generic/068 to crash because I didn't
> filter out the 'log tree' case, I'm testing a V2 patch.

I've hit the assert with btrfs/003, once and not deterministically in a VM.
--
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 mbox

Patch

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 2b790bd..603dd492 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -4483,8 +4483,19 @@  int btrfs_truncate_inode_items(struct btrfs_trans_handle *trans,
 		if (found_type > min_type) {
 			del_item = 1;
 		} else {
-			if (item_end < new_size)
+			if (item_end < new_size) {
+				/*
+				 * With NO_HOLES mode, for the following mapping
+				 *
+				 * [0-4k][hole][8k-12k]
+				 *
+				 * if truncating isize down to 6k, it ends up
+				 * isize being 8k.
+				 */
+				if (btrfs_fs_incompat(root->fs_info, NO_HOLES))
+					last_size = new_size;
 				break;
+			}
 			if (found_key.offset >= new_size)
 				del_item = 1;
 			else
@@ -4666,6 +4677,9 @@  int btrfs_truncate_inode_items(struct btrfs_trans_handle *trans,
 			btrfs_abort_transaction(trans, ret);
 	}
 error:
+	if (err == 0)
+		ASSERT(last_size == new_size);
+
 	if (root->root_key.objectid != BTRFS_TREE_LOG_OBJECTID)
 		btrfs_ordered_update_i_size(inode, last_size, NULL);