more reflink fixes & debug
Hi all,
this series starts adds more reflink fixes on top of the series from two days ago, plus two new debug patches to dump leftover delalloc extents and a debug-only always_cow mode.
PATCH 1/5 xfs: cancel COW blocks before swapext
We need to make sure we have no outstanding COW blocks before we swap extents, as there is nothing preventing us from having COW preallocations on an inode that swapext is called on. That case can easily be reproduced by the upcoming always_cow mode.
--- fs/xfs/xfs_bmap_util.c | 6 ++++++ 1 file changed, 6 insertions(+)
fs/xfs/xfs_bmap_util.c +6 -0
@@ -1515,6 +1515,12 @@ xfs_swap_extent_flush(return error;truncate_pagecache_range (VFS_I (ip),0 ,-1 ); +if (xfs_inode_has_cow_data (ip)) { + error =xfs_reflink_cancel_cow_range (ip,0 ,NULLFILEOFF , true); +if (error) +return error; + } +/* Verify O_DIRECT for ftmp */ if (VFS_I (ip)->i_mapping->nrpages)return -EINVAL ;
Signature · 1 line
2.18.0
Re: [PATCH 1/5] xfs: cancel COW blocks before swapext
Quoted 6 lines · Christoph Hellwig
We need to make sure we have no outstanding COW blocks before we swap extents, as there is nothing preventing us from having COW preallocations on an inode that swapext is called on. That case can easily be reproduced by the upcoming
always_cowmode.Signed-off-by Christoph Hellwig <[email protected]>
Seems reasonable to me,
--D
Quoted 24 lines
--- fs/xfs/xfs_bmap_util.c | 6 ++++++ 1 file changed, 6 insertions(+)fs/xfs/xfs_bmap_util.c +6
-0@@ -1515,6 +1515,12 @@ xfs_swap_extent_flush(return error;truncate_pagecache_range (VFS_I (ip),0 ,-1 ); +if (xfs_inode_has_cow_data (ip)) { + error =xfs_reflink_cancel_cow_range (ip,0 ,NULLFILEOFF , true); +if (error) +return error; + } +/* Verify O_DIRECT for ftmp */ if (VFS_I (ip)->i_mapping->nrpages)return -EINVAL ;Signature · 1 line
2.18.0
Re: [PATCH 1/5] xfs: cancel COW blocks before swapext
In reply to Christoph HellwigQuoted 5 lines · Christoph Hellwig
We need to make sure we have no outstanding COW blocks before we swap extents, as there is nothing preventing us from having COW preallocations on an inode that swapext is called on. That case can easily be reproduced by the upcoming
always_cowmode.
Why? Is the current code to swap the cow forks not sufficient or problematic in some way?
Quoted 10 lines
Signed-off-by Christoph Hellwig <[email protected]>--- fs/xfs/xfs_bmap_util.c | 6 ++++++ 1 file changed, 6 insertions(+)fs/xfs/xfs_bmap_util.c +0
-0
fs/xfs/xfs_bmap_util.c xfs_swap_extent_flush( · line 1523
Quoted 9 lines
fs/xfs/xfs_bmap_util.c +6
-0@@ -1515,6 +1515,12 @@ xfs_swap_extent_flush(return error;truncate_pagecache_range (VFS_I (ip),0 ,-1 ); +if (xfs_inode_has_cow_data (ip)) { + error =xfs_reflink_cancel_cow_range (ip,0 ,NULLFILEOFF , true); +if (error) +return error; + } +
The change seems fine, but doesn't this make the follow on code to swap the cow blocks tags spurious..?
Brian
Quoted 6 lines
fs/xfs/xfs_bmap_util.c +0
-0/* Verify O_DIRECT for ftmp */ if (VFS_I (ip)->i_mapping->nrpages)return -EINVAL ;Signature · 1 line
2.18.0
Re: [PATCH 1/5] xfs: cancel COW blocks before swapext
Quoted 9 lines · Brian Foster
Quoted 5 lines · Christoph Hellwig
We need to make sure we have no outstanding COW blocks before we swap extents, as there is nothing preventing us from having COW preallocations on an inode that swapext is called on. That case can easily be reproduced by the upcoming
always_cowmode.Why? Is the current code to swap the cow forks not sufficient or problematic in some way?
The code didn't seem happy with non-delalloc COW fork extents sticking around over swapext, but I'd have to go back and look up why.
It seemed easiest to just cancel them as there is no reason to keep these alive over a swapext.
Re: [PATCH 1/5] xfs: cancel COW blocks before swapext
Quoted 8 lines · Christoph Hellwig
Quoted 7 lines · Brian Foster
Quoted 5 lines · Christoph Hellwig
We need to make sure we have no outstanding COW blocks before we swap extents, as there is nothing preventing us from having COW preallocations on an inode that swapext is called on. That case can easily be reproduced by the upcoming
always_cowmode.
Quoted 6 lines
Quoted 2 lines
Why? Is the current code to swap the cow forks not sufficient or problematic in some way?
The code didn't seem happy with non-delalloc COW fork extents sticking around over swapext, but I'd have to go back and look up why.
It would be nice to have at least a high level note in the commit log for historical purposes, so we have a reference on what needs fixing if we revisit it in the future.
Quoted 2 lines
It seemed easiest to just cancel them as there is no reason to keep these alive over a swapext.
Seems reasonable to me. I'd just perhaps replace the cowblocks flag swap code with asserts or something to be consistent, because it seems that should never be set on either inode with this change in place.
Brian
PATCH 2/5 xfs: skip delalloc COW blocks in xfs_reflink_end_cow
In reply to Christoph HellwigThe iomap direct I/O code issues a single ->end_io call for the whole I/O request, and if some of the extents cowered needed a COW operation it will call xfs_reflink_end_cow over the whole range.
When we do AIO writes we drop the iolock after doing the initial setup, but before the I/O completion. Between dropping the lock and completing the I/O we can have a racing buffered write create new delalloc COW fork extents in the region covered by the outstanding direct I/O write, and thus see delalloc COW fork extents in xfs_reflink_end_cow. As concurrent writes are fundamentally racy and no guarantees are given we can simply skip those.
This can be easily reproduced with xfstests generic/208 in always_cow mode.
--- fs/xfs/xfs_reflink.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-)
fs/xfs/xfs_reflink.c +4 -6
@@ -686,14 +686,12 @@ xfs_reflink_end_cow(if (!del.br_blockcount) goto prev_extent; -ASSERT (!isnullstartblock (got.br_startblock)); -/* -* Don't remap unwritten extents; these are -* speculatively preallocated CoW extents that have been -* allocated but have not yet been involved in a write. +* Only remap real extent that contain data. With AIO +* speculatively preallocations can leak into the range we +* are called upon, and we need to skip them. */ -if (got.br_state ==XFS_EXT_UNWRITTEN ) +if (!xfs_bmap_is_real_extent (&got)) goto prev_extent;/* Unmap the old blocks in the data fork. */
Signature · 1 line
2.18.0
Re: [PATCH 2/5] xfs: skip delalloc COW blocks in xfs_reflink_end_cow
Quoted 12 lines · Christoph Hellwig
The iomap direct I/O code issues a single ->end_io call for the whole I/O request, and if some of the extents cowered needed a COW operation it will call
xfs_reflink_end_cowover the whole range.When we do AIO writes we drop the iolock after doing the initial setup, but before the I/O completion. Between dropping the lock and completing the I/O we can have a racing buffered write create new delalloc COW fork extents in the region covered by the outstanding direct I/O write, and thus see delalloc COW fork extents in
xfs_reflink_end_cow. As concurrent writes are fundamentally racy and no guarantees are given we can simply skip those.
Quoted 4 lines
This can be easily reproduced with xfstests generic/208 in
always_cowmode.Signed-off-by Christoph Hellwig <[email protected]>
Looks ok to me...
--D
Quoted 30 lines
--- fs/xfs/xfs_reflink.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-)fs/xfs/xfs_reflink.c +4
-6@@ -686,14 +686,12 @@ xfs_reflink_end_cow(if (!del.br_blockcount) goto prev_extent; -ASSERT (!isnullstartblock (got.br_startblock)); -/* -* Don't remap unwritten extents; these are -* speculatively preallocated CoW extents that have been -* allocated but have not yet been involved in a write. +* Only remap real extent that contain data. With AIO +* speculatively preallocations can leak into the range we +* are called upon, and we need to skip them. */ -if (got.br_state ==XFS_EXT_UNWRITTEN ) +if (!xfs_bmap_is_real_extent (&got)) goto prev_extent;/* Unmap the old blocks in the data fork. */ Signature · 1 line
2.18.0
PATCH 3/5 xfs: fix fork selection in xfs_find_trim_cow_extent
In reply to Christoph HellwigWe should want to write directly into the data fork for blocks that don't have an extent in the COW fork covering them yet.
--- fs/xfs/xfs_reflink.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
fs/xfs/xfs_reflink.c +6 -2
@@ -369,9 +369,13 @@ xfs_find_trim_cow_extent(* If we don't find an overlapping extent, trim the range we need to * allocate to fit the hole we found. */ -if (!xfs_iext_lookup_extent (ip, ip->i_cowfp, offset_fsb, &icur, &got) || - got.br_startoff > offset_fsb) +if (!xfs_iext_lookup_extent (ip, ip->i_cowfp, offset_fsb, &icur, &got)) + got.br_startoff = offset_fsb + count_fsb; +if (got.br_startoff > offset_fsb) { +xfs_trim_extent (imap, imap->br_startoff, + got.br_startoff - imap->br_startoff);return xfs_reflink_trim_around_shared (ip, imap, shared); + } *shared = true;if (isnullstartblock (got.br_startblock)) {
Signature · 1 line
2.18.0
PATCH 4/5 xfs: print dangling delalloc extents
In reply to Christoph HellwigInstead of just asserting that we have no delalloc space dangling in an inode that gets freed print the actual offenders for debug mode.
--- fs/xfs/xfs_super.c | 32 +++++++++++++++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-)
fs/xfs/xfs_super.c +31 -1
@@ -933,6 +933,31 @@ xfs_fs_alloc_inode(return NULL ; } +#ifdef DEBUG +static void +xfs_check_delalloc ( +struct xfs_inode *ip, +int whichfork) +{ +struct xfs_ifork *ifp =XFS_IFORK_PTR (ip, whichfork); +struct xfs_bmbt_irec got; +struct xfs_iext_cursor icur; + +if (!ifp || !xfs_iext_lookup_extent (ip, ifp,0 , &icur, &got)) +return ; +do { +if (isnullstartblock (got.br_startblock)) { +xfs_warn (ip->i_mount, +"%s fork has delalloc extent at [0x%llx:0x%llx]" , + whichfork ==XFS_DATA_FORK ?"data" :"cow" , + got.br_startoff, got.br_blockcount); + } + }while (xfs_iext_next_extent (ifp, &icur, &got)); +} +#else +#define xfs_check_delalloc (ip, whichfork) do { } while (0) +#endif +/* * Now that the generic code is guaranteed not to be accessing * the linux inode, we can inactivate and reclaim the inode.
@@ -951,7 +976,12 @@ xfs_fs_destroy_inode(xfs_inactive (ip); -ASSERT (XFS_FORCED_SHUTDOWN (ip->i_mount) || ip->i_delayed_blks ==0 ); +if (!XFS_FORCED_SHUTDOWN (ip->i_mount) && ip->i_delayed_blks) { +xfs_check_delalloc (ip,XFS_DATA_FORK ); +xfs_check_delalloc (ip,XFS_COW_FORK ); +ASSERT (0 ); + } +XFS_STATS_INC (ip->i_mount, vn_reclaim);/*
Signature · 1 line
2.18.0
Re: [PATCH 4/5] xfs: print dangling delalloc extents
Quoted 5 lines · Christoph Hellwig
Instead of just asserting that we have no delalloc space dangling in an inode that gets freed print the actual offenders for debug mode.
Signed-off-by Christoph Hellwig <[email protected]>
Looks ok,
--D
Quoted 57 lines
--- fs/xfs/xfs_super.c | 32 +++++++++++++++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-)fs/xfs/xfs_super.c +31
-1@@ -933,6 +933,31 @@ xfs_fs_alloc_inode(return NULL ; } +#ifdef DEBUG +static void +xfs_check_delalloc ( +struct xfs_inode *ip, +int whichfork) +{ +struct xfs_ifork *ifp =XFS_IFORK_PTR (ip, whichfork); +struct xfs_bmbt_irec got; +struct xfs_iext_cursor icur; + +if (!ifp || !xfs_iext_lookup_extent (ip, ifp,0 , &icur, &got)) +return ; +do { +if (isnullstartblock (got.br_startblock)) { +xfs_warn (ip->i_mount, +"%s fork has delalloc extent at [0x%llx:0x%llx]" , + whichfork ==XFS_DATA_FORK ?"data" :"cow" , + got.br_startoff, got.br_blockcount); + } + }while (xfs_iext_next_extent (ifp, &icur, &got)); +} +#else +#define xfs_check_delalloc (ip, whichfork) do { } while (0) +#endif +/* * Now that the generic code is guaranteed not to be accessing * the linux inode, we can inactivate and reclaim the inode. @@ -951,7 +976,12 @@ xfs_fs_destroy_inode(xfs_inactive (ip); -ASSERT (XFS_FORCED_SHUTDOWN (ip->i_mount) || ip->i_delayed_blks ==0 ); +if (!XFS_FORCED_SHUTDOWN (ip->i_mount) && ip->i_delayed_blks) { +xfs_check_delalloc (ip,XFS_DATA_FORK ); +xfs_check_delalloc (ip,XFS_COW_FORK ); +ASSERT (0 ); + } +XFS_STATS_INC (ip->i_mount, vn_reclaim);/* Signature · 1 line
2.18.0
Re: [PATCH 4/5] xfs: print dangling delalloc extents
In reply to Christoph HellwigQuoted 25 lines · Christoph Hellwig
Instead of just asserting that we have no delalloc space dangling in an inode that gets freed print the actual offenders for debug mode.
Signed-off-by Christoph Hellwig <[email protected]>--- fs/xfs/xfs_super.c | 32 +++++++++++++++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-)fs/xfs/xfs_super.c +8
-0@@ -933,6 +933,31 @@ xfs_fs_alloc_inode(return NULL ; } +#ifdef DEBUG +static void +xfs_check_delalloc ( +struct xfs_inode *ip, +int whichfork) +{ +struct xfs_ifork *ifp =XFS_IFORK_PTR (ip, whichfork); +struct xfs_bmbt_irec got;
fs/xfs/xfs_super.c xfs_fs_alloc_inode( · line 953
Quoted 10 lines
fs/xfs/xfs_super.c +10
-0+struct xfs_iext_cursor icur; + +if (!ifp || !xfs_iext_lookup_extent (ip, ifp,0 , &icur, &got)) +return ; + do { + if (isnullstartblock(got.br_startblock)) { + xfs_warn(ip->i_mount, + "%s fork has delalloc extent at [0x%llx:0x%llx]", + whichfork == XFS_DATA_FORK ? "data" : "cow", + got.br_startoff, got.br_blockcount);
If we're going to print warnings, can we include inode-identifying info as well?
Brian
Quoted 27 lines
fs/xfs/xfs_super.c +13
-1+ } + }while (xfs_iext_next_extent (ifp, &icur, &got)); +} +#else +#define xfs_check_delalloc (ip, whichfork) do { } while (0) +#endif +/* * Now that the generic code is guaranteed not to be accessing * the linux inode, we can inactivate and reclaim the inode. @@ -951,7 +976,12 @@ xfs_fs_destroy_inode(xfs_inactive (ip); -ASSERT (XFS_FORCED_SHUTDOWN (ip->i_mount) || ip->i_delayed_blks ==0 ); +if (!XFS_FORCED_SHUTDOWN (ip->i_mount) && ip->i_delayed_blks) { +xfs_check_delalloc (ip,XFS_DATA_FORK ); +xfs_check_delalloc (ip,XFS_COW_FORK ); +ASSERT (0 ); + } +XFS_STATS_INC (ip->i_mount, vn_reclaim);/* Signature · 1 line
2.18.0
PATCH 5/5 xfs: introduce an always_cow mode
In reply to Christoph HellwigWhen the (debug-only) always_cow module parameter is set, we will always write out place, even if the file is not reflinked. In addition to being a useful debug aid this prepares for modes where we must always write out of place. An example for that is the upcoming support for atomic overwrites.
--- fs/xfs/xfs_aops.c | 2 +- fs/xfs/xfs_file.c | 2 +- fs/xfs/xfs_iomap.c | 8 +++---- fs/xfs/xfs_reflink.c | 50 ++++++++++++++++++++++++++++++++------------ fs/xfs/xfs_reflink.h | 13 ++++++++++++ 5 files changed, 56 insertions(+), 19 deletions(-)
fs/xfs/xfs_aops.c +1 -1
@@ -980,7 +980,7 @@ xfs_vm_bmap(* Since we don't pass back blockdev info, we can't return bmap * information for rt files either. */ -if (xfs_is_reflink_inode (ip) ||XFS_IS_REALTIME_INODE (ip)) +if (xfs_is_cow_inode (ip) ||XFS_IS_REALTIME_INODE (ip))return 0 ;return iomap_bmap (mapping, block, &xfs_iomap_ops); }
fs/xfs/xfs_file.c +1 -1
@@ -507,7 +507,7 @@ xfs_file_dio_aio_write(* We can't properly handle unaligned direct I/O to reflink * files yet, as we can't unshare a partial block. */ -if (xfs_is_reflink_inode (ip)) { +if (xfs_is_cow_inode (ip)) {trace_xfs_reflink_bounce_dio_write (ip, iocb->ki_pos, count);return -EREMCHG ; }
fs/xfs/xfs_iomap.c +4 -4
@@ -570,7 +570,7 @@ xfs_delalloc_iomap_begin(if (eof) got.br_startoff = maxbytes_fsb;if (got.br_startoff <= offset_fsb) { -if (xfs_is_reflink_inode (ip) && +if (xfs_is_cow_inode (ip) && ((flags &IOMAP_WRITE ) || got.br_state !=XFS_EXT_UNWRITTEN )) { end_fsb = min(XFS_B_TO_FSB(mp, offset + count),
@@ -1048,7 +1048,7 @@ xfs_ilock_for_iomap(* COW writes may allocate delalloc space or convert unwritten COW * extents, so we need to make sure to take the lock exclusively here. */ -if (xfs_is_reflink_inode (ip) && is_write) { +if (xfs_is_cow_inode (ip) && is_write) {/* * FIXME: It could still overwrite on unshared extents and not * need allocation.
@@ -1082,7 +1082,7 @@ xfs_ilock_for_iomap(* check, so if we got ILOCK_SHARED for a write and but we're now a * reflink inode we have to switch to ILOCK_EXCL and relock. */ -if (mode ==XFS_ILOCK_SHARED && is_write &&xfs_is_reflink_inode (ip)) { +if (mode ==XFS_ILOCK_SHARED && is_write &&xfs_is_cow_inode (ip)) {xfs_iunlock (ip, mode); mode =XFS_ILOCK_EXCL ; goto relock;
@@ -1151,7 +1151,7 @@ xfs_file_iomap_begin(* Break shared extents if necessary. Checks for non-blocking IO have * been done up front, so we don't need to do them here. */ -if (xfs_is_reflink_inode (ip)) { +if (xfs_is_cow_inode (ip)) { error =xfs_reflink_allocate_cow (ip, &imap, &shared, &lockmode);if (error) goto out_unlock;
fs/xfs/xfs_reflink.c +37 -13
112 lines not on this page · raw message
fs/xfs/xfs_reflink.h +13 -0
@@ -6,6 +6,19 @@#ifndef __XFS_REFLINK_H#define __XFS_REFLINK_H 1 +#ifdef DEBUG +extern bool xfs_always_cow; +#else +#define xfs_always_cow false +#endif + +static inline bool xfs_is_cow_inode (struct xfs_inode *ip) +{ +return xfs_is_reflink_inode (ip) || + (xfs_always_cow && +xfs_sb_version_hasreflink (&ip->i_mount->m_sb)); +} +extern int xfs_reflink_find_shared (struct xfs_mount *mp,struct xfs_trans *tp,xfs_agnumber_t agno,xfs_agblock_t agbno,xfs_extlen_t aglen,xfs_agblock_t *fbno,xfs_extlen_t *flen,bool find_maximal);
Signature · 1 line
2.18.0
Re: [PATCH 5/5] xfs: introduce an always_cow mode
Quoted 86 lines · Christoph Hellwig
When the (debug-only)
always_cowmodule parameter is set, we will always write out place, even if the file is not reflinked. In addition to being a useful debug aid this prepares for modes where we must always write out of place. An example for that is the upcoming support for atomic overwrites.Signed-off-by Christoph Hellwig <[email protected]>--- fs/xfs/xfs_aops.c | 2 +- fs/xfs/xfs_file.c | 2 +- fs/xfs/xfs_iomap.c | 8 +++---- fs/xfs/xfs_reflink.c | 50 ++++++++++++++++++++++++++++++++------------ fs/xfs/xfs_reflink.h | 13 ++++++++++++ 5 files changed, 56 insertions(+), 19 deletions(-)fs/xfs/xfs_aops.c +1
-1@@ -980,7 +980,7 @@ xfs_vm_bmap(* Since we don't pass back blockdev info, we can't return bmap * information for rt files either. */ -if (xfs_is_reflink_inode (ip) ||XFS_IS_REALTIME_INODE (ip)) +if (xfs_is_cow_inode (ip) ||XFS_IS_REALTIME_INODE (ip))return 0 ;return iomap_bmap (mapping, block, &xfs_iomap_ops); }fs/xfs/xfs_file.c +1
-1@@ -507,7 +507,7 @@ xfs_file_dio_aio_write(* We can't properly handle unaligned direct I/O to reflink * files yet, as we can't unshare a partial block. */ -if (xfs_is_reflink_inode (ip)) { +if (xfs_is_cow_inode (ip)) {trace_xfs_reflink_bounce_dio_write (ip, iocb->ki_pos, count);return -EREMCHG ; }fs/xfs/xfs_iomap.c +4
-4@@ -570,7 +570,7 @@ xfs_delalloc_iomap_begin(if (eof) got.br_startoff = maxbytes_fsb;if (got.br_startoff <= offset_fsb) { -if (xfs_is_reflink_inode (ip) && +if (xfs_is_cow_inode (ip) && ((flags &IOMAP_WRITE ) || got.br_state !=XFS_EXT_UNWRITTEN )) { end_fsb = min(XFS_B_TO_FSB(mp, offset + count),@@ -1048,7 +1048,7 @@ xfs_ilock_for_iomap(* COW writes may allocate delalloc space or convert unwritten COW * extents, so we need to make sure to take the lock exclusively here. */ -if (xfs_is_reflink_inode (ip) && is_write) { +if (xfs_is_cow_inode (ip) && is_write) {/* * FIXME: It could still overwrite on unshared extents and not * need allocation. @@ -1082,7 +1082,7 @@ xfs_ilock_for_iomap(* check, so if we got ILOCK_SHARED for a write and but we're now a * reflink inode we have to switch to ILOCK_EXCL and relock. */ -if (mode ==XFS_ILOCK_SHARED && is_write &&xfs_is_reflink_inode (ip)) { +if (mode ==XFS_ILOCK_SHARED && is_write &&xfs_is_cow_inode (ip)) {xfs_iunlock (ip, mode); mode =XFS_ILOCK_EXCL ; goto relock;@@ -1151,7 +1151,7 @@ xfs_file_iomap_begin(* Break shared extents if necessary. Checks for non-blocking IO have * been done up front, so we don't need to do them here. */ -if (xfs_is_reflink_inode (ip)) { +if (xfs_is_cow_inode (ip)) { error =xfs_reflink_allocate_cow (ip, &imap, &shared, &lockmode);if (error) goto out_unlock;fs/xfs/xfs_reflink.c +0
-0
fs/xfs/xfs_reflink.c line 135
Quoted 7 lines
fs/xfs/xfs_reflink.c +4
-0@@ -129,6 +129,11 @@* ioend, the better. */ +#ifdef DEBUG +bool xfs_always_cow; +module_param_named (always_cow, xfs_always_cow, bool,0644 ); +#endif
I would have expected this to end up in /proc/sys/fs/xfs/always_cow along with all the other behavior knobs, but I guess this is a debugging thing...?
Quoted 14 lines
fs/xfs/xfs_reflink.c +2
-1+/* * Given an AG extent, find the lowest-numbered run of shared blocks * within that range and return the range in fbno/flen. If @@ -192,7 +197,7 @@ xfs_reflink_trim_around_shared(int error =0 ;/* Holes, unwritten, and delalloc extents cannot be shared */ -if (!xfs_is_reflink_inode (ip) || !xfs_bmap_is_real_extent (irec)) { +if (!xfs_is_cow_inode (ip) || !xfs_bmap_is_real_extent (irec)) { *shared = false;return 0 ; }
fs/xfs/xfs_reflink.c xfs_reflink_trim_around_shared( · line 248
Quoted 10 lines
fs/xfs/xfs_reflink.c +7
-0@@ -234,6 +239,22 @@ xfs_reflink_trim_around_shared( } } +static bool +xfs_inode_need_cow ( +struct xfs_inode *ip, +struct xfs_bmbt_irec *imap, +bool *shared) +{ +/* We can't update any real extents in always COW mode. */
/me struggled with this for a minute before figuring out what the comment was trying to tell me. Is the following accurate?
"In always_cow mode, behave as if the incoming data fork imap is completely shared." ?
--D
Quoted 116 lines
fs/xfs/xfs_reflink.c +24
-12+if (xfs_always_cow && !isnullstartblock (imap->br_startblock)) { + *shared = true; +return 0 ; + } + +/* Trim the mapping to the nearest shared extent boundary. */ +return xfs_reflink_trim_around_shared (ip, imap, shared); +} +/* * Trim the passed in imap to the next shared/unshared extent boundary, and * if imap->br_startoff points to a shared extent reserve space for it in the @@ -248,13 +269,17 @@ xfs_reflink_reserve_cow(struct xfs_inode *ip, struct xfs_bmbt_irec *imap) { - struct xfs_ifork *ifp = XFS_IFORK_PTR(ip, XFS_COW_FORK); struct xfs_bmbt_irec got;int error =0 ;bool eof = false;struct xfs_iext_cursor icur;bool shared; +if (!ip->i_cowfp) { +ASSERT (!xfs_is_reflink_inode (ip)); +xfs_ifork_init_cow (ip); + } +/* * Search the COW fork extent list first. This serves two purposes: * first this implement the speculative preallocation using cowextisze, @@ -263,8 +288,8 @@ xfs_reflink_reserve_cow(* extent list is generally faster than going out to the shared extent * tree. */ - -if (!xfs_iext_lookup_extent (ip, ifp, imap->br_startoff, &icur, &got)) +if (!xfs_iext_lookup_extent (ip, ip->i_cowfp, imap->br_startoff, &icur, + &got)) eof = true;if (!eof && got.br_startoff <= imap->br_startoff) {trace_xfs_reflink_cow_found (ip, imap);@@ -273,14 +298,10 @@ xfs_reflink_reserve_cow( }/* Trim the mapping to the nearest shared extent boundary. */ - error =xfs_reflink_trim_around_shared (ip, imap, &shared); -if (error) + error =xfs_inode_need_cow (ip, imap, &shared); +if (error || !shared)return error; -/* Not shared? Just report the (potentially capped) extent. */ -if (!shared) -return 0 ; -/* * Fork all the shared blocks from our write offset until the end of * the extent. @@ -374,7 +395,7 @@ xfs_find_trim_cow_extent(if (got.br_startoff > offset_fsb) {xfs_trim_extent (imap, imap->br_startoff, got.br_startoff - imap->br_startoff); -return xfs_reflink_trim_around_shared (ip, imap, shared); +return xfs_inode_need_cow (ip, imap, shared); } *shared = true;@@ -408,7 +429,10 @@ xfs_reflink_allocate_cow(xfs_extlen_t resblks =0 ;ASSERT (xfs_isilocked (ip,XFS_ILOCK_EXCL )); -ASSERT (xfs_is_reflink_inode (ip)); +if (!ip->i_cowfp) { +ASSERT (!xfs_is_reflink_inode (ip)); +xfs_ifork_init_cow (ip); + } error =xfs_find_trim_cow_extent (ip, imap, shared, &found);if (error || !*shared)@@ -585,7 +609,7 @@ xfs_reflink_cancel_cow_range(int error;trace_xfs_reflink_cancel_cow_range (ip, offset, count); -ASSERT (xfs_is_reflink_inode (ip)); +ASSERT (ip->i_cowfp); offset_fsb =XFS_B_TO_FSBT (ip->i_mount, offset);if (count ==NULLFILEOFF )fs/xfs/xfs_reflink.h +13
-0@@ -6,6 +6,19 @@#ifndef __XFS_REFLINK_H#define __XFS_REFLINK_H 1 +#ifdef DEBUG +extern bool xfs_always_cow; +#else +#define xfs_always_cow false +#endif + +static inline bool xfs_is_cow_inode (struct xfs_inode *ip) +{ +return xfs_is_reflink_inode (ip) || + (xfs_always_cow && +xfs_sb_version_hasreflink (&ip->i_mount->m_sb)); +} +extern int xfs_reflink_find_shared (struct xfs_mount *mp,struct xfs_trans *tp,xfs_agnumber_t agno,xfs_agblock_t agbno,xfs_extlen_t aglen,xfs_agblock_t *fbno,xfs_extlen_t *flen,bool find_maximal);Signature · 1 line
2.18.0
Re: [PATCH 5/5] xfs: introduce an always_cow mode
Quoted 6 lines · Darrick J. Wong
Quoted 5 lines · Christoph Hellwig
When the (debug-only)
always_cowmodule parameter is set, we will always write out place, even if the file is not reflinked. In addition to being a useful debug aid this prepares for modes where we must always write out of place. An example for that is the upcoming support for atomic overwrites.
....
Quoted 8 lines
Quoted 8 lines
fs/xfs/xfs_reflink.c +0
-0@@ -129,6 +129,11 @@* ioend, the better. */
Quoted 8 lines
fs/xfs/xfs_reflink.c line 135
Quoted 4 lines
fs/xfs/xfs_reflink.c +4
-0+#ifdef DEBUG +bool xfs_always_cow; +module_param_named (always_cow, xfs_always_cow, bool,0644 ); +#endif I would have expected this to end up in
/proc/sys/fs/xfs/always_cowalong with all the other behavior knobs, but I guess this is a debugging thing...?
sys/fs/xfs/debug/always_cow is the place for this if it is truly a global debug option. However, that's probably not the best way to enable this sort of debug for people with XFS root filesystems trying to test XFS filesystems. I suspect it should be a per-mount configuration (i.e. /sys/fs/xfs/<dev>/debug/always_cow)
However, this mode of operation seems like it is going to form the basis of other per-filesystem features, so it would be good to know how/if those features make use of this always_cow mode or are controlled by some other set of knobs first.
Cheers,
Dave.
Signature · 2 lines
Dave Chinner [email protected]
Re: [PATCH 5/5] xfs: introduce an always_cow mode
Quoted 5 lines · Dave Chinner
sys/fs/xfs/debug/always_cow is the place for this if it is truly a global debug option. However, that's probably not the best way to enable this sort of debug for people with XFS root filesystems trying to test XFS filesystems. I suspect it should be a per-mount configuration (i.e. /sys/fs/xfs/<dev>/debug/always_cow)
True.
Quoted 4 lines
However, this mode of operation seems like it is going to form the basis of other per-filesystem features, so it would be good to know how/if those features make use of this
always_cowmode or are controlled by some other set of knobs first.
The current atomic write support is a per-inode fcntl based flag. (used to be an O_ATOMIC per open file flag, but that had all kinds of issues).
The zoned device support doesn't really existing for a bit of prototype write path code, but the current assumption is that all files in ѕequential write only zones have the RT bit set, and will get this behavior on that an a per-subperblock flag.
In other words - the know here is supposed to remain a debug build only debug nob.