* [PATCH 01/15] xfs: add a xfs_bmap_fork_to_state helper
2017-10-19 6:59 more extent mapping cleanups Christoph Hellwig
@ 2017-10-19 6:59 ` Christoph Hellwig
2017-10-19 22:48 ` Darrick J. Wong
2017-10-19 6:59 ` [PATCH 02/15] xfs: make better use of the 'state' variable in xfs_bmap_del_extent_real Christoph Hellwig
` (14 subsequent siblings)
15 siblings, 1 reply; 36+ messages in thread
From: Christoph Hellwig @ 2017-10-19 6:59 UTC (permalink / raw)
To: linux-xfs
This creates the right initial bmap state from the passed in inode
fork enum.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/xfs/libxfs/xfs_bmap.c | 45 +++++++++------------------------------------
fs/xfs/libxfs/xfs_bmap.h | 12 ++++++++++++
2 files changed, 21 insertions(+), 36 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 6c2ae3011964..676fc8aed67f 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -499,12 +499,7 @@ xfs_bmap_trace_exlist(
{
xfs_extnum_t idx; /* extent record index */
xfs_ifork_t *ifp; /* inode fork pointer */
- int state = 0;
-
- if (whichfork == XFS_ATTR_FORK)
- state |= BMAP_ATTRFORK;
- else if (whichfork == XFS_COW_FORK)
- state |= BMAP_COWFORK;
+ int state = xfs_bmap_fork_to_state(whichfork);
ifp = XFS_IFORK_PTR(ip, whichfork);
ASSERT(cnt == xfs_iext_count(ifp));
@@ -925,8 +920,7 @@ xfs_bmap_local_to_extents(
rec.br_state = XFS_EXT_NORM;
xfs_iext_insert(ip, 0, 1, &rec, 0);
- trace_xfs_bmap_post_update(ip, 0,
- whichfork == XFS_ATTR_FORK ? BMAP_ATTRFORK : 0,
+ trace_xfs_bmap_post_update(ip, 0, xfs_bmap_fork_to_state(whichfork),
_THIS_IP_);
XFS_IFORK_NEXT_SET(ip, whichfork, 1);
ip->i_d.di_nblocks = 1;
@@ -1571,7 +1565,7 @@ xfs_bmap_add_extent_delay_real(
xfs_bmbt_irec_t r[3]; /* neighbor extent entries */
/* left is 0, right is 1, prev is 2 */
int rval=0; /* return value (logging flags) */
- int state = 0;/* state bits, accessed thru macros */
+ int state = xfs_bmap_fork_to_state(whichfork);
xfs_filblks_t da_new; /* new count del alloc blocks used */
xfs_filblks_t da_old; /* old count del alloc blocks used */
xfs_filblks_t temp=0; /* value for da_new calculations */
@@ -1598,9 +1592,6 @@ xfs_bmap_add_extent_delay_real(
#define RIGHT r[1]
#define PREV r[2]
- if (whichfork == XFS_COW_FORK)
- state |= BMAP_COWFORK;
-
/*
* Set up a bunch of variables to make the tests simpler.
*/
@@ -2108,7 +2099,7 @@ xfs_bmap_add_extent_unwritten_real(
xfs_bmbt_irec_t r[3]; /* neighbor extent entries */
/* left is 0, right is 1, prev is 2 */
int rval=0; /* return value (logging flags) */
- int state = 0;/* state bits, accessed thru macros */
+ int state = xfs_bmap_fork_to_state(whichfork);
struct xfs_mount *mp = ip->i_mount;
struct xfs_bmbt_irec old;
@@ -2116,8 +2107,6 @@ xfs_bmap_add_extent_unwritten_real(
cur = *curp;
ifp = XFS_IFORK_PTR(ip, whichfork);
- if (whichfork == XFS_COW_FORK)
- state |= BMAP_COWFORK;
ASSERT(*idx >= 0);
ASSERT(*idx <= xfs_iext_count(ifp));
@@ -2601,13 +2590,10 @@ xfs_bmap_add_extent_hole_delay(
xfs_filblks_t newlen=0; /* new indirect size */
xfs_filblks_t oldlen=0; /* old indirect size */
xfs_bmbt_irec_t right; /* right neighbor extent entry */
- int state; /* state bits, accessed thru macros */
+ int state = xfs_bmap_fork_to_state(whichfork);
xfs_filblks_t temp; /* temp for indirect calculations */
ifp = XFS_IFORK_PTR(ip, whichfork);
- state = 0;
- if (whichfork == XFS_COW_FORK)
- state |= BMAP_COWFORK;
ASSERT(isnullstartblock(new->br_startblock));
/*
@@ -2760,7 +2746,7 @@ xfs_bmap_add_extent_hole_real(
xfs_bmbt_irec_t left; /* left neighbor extent entry */
xfs_bmbt_irec_t right; /* right neighbor extent entry */
int rval=0; /* return value (logging flags) */
- int state; /* state bits, accessed thru macros */
+ int state = xfs_bmap_fork_to_state(whichfork);
struct xfs_bmbt_irec old;
ASSERT(*idx >= 0);
@@ -2770,12 +2756,6 @@ xfs_bmap_add_extent_hole_real(
XFS_STATS_INC(mp, xs_add_exlist);
- state = 0;
- if (whichfork == XFS_ATTR_FORK)
- state |= BMAP_ATTRFORK;
- if (whichfork == XFS_COW_FORK)
- state |= BMAP_COWFORK;
-
/*
* Check and set flags if this segment has a left neighbor.
*/
@@ -4737,7 +4717,8 @@ xfs_bmap_del_extent_delay(
int64_t da_old, da_new, da_diff = 0;
xfs_fileoff_t del_endoff, got_endoff;
xfs_filblks_t got_indlen, new_indlen, stolen;
- int error = 0, state = 0;
+ int state = xfs_bmap_fork_to_state(whichfork);
+ int error = 0;
bool isrt;
XFS_STATS_INC(mp, xs_del_exlist);
@@ -4773,9 +4754,6 @@ xfs_bmap_del_extent_delay(
return error;
ip->i_delayed_blks -= del->br_blockcount;
- if (whichfork == XFS_COW_FORK)
- state |= BMAP_COWFORK;
-
if (got->br_startoff == del->br_startoff)
state |= BMAP_LEFT_FILLING;
if (got_endoff == del_endoff)
@@ -4969,17 +4947,12 @@ xfs_bmap_del_extent_real(
xfs_bmbt_irec_t new; /* new record to be inserted */
/* REFERENCED */
uint qfield; /* quota field to update */
- int state = 0;
+ int state = xfs_bmap_fork_to_state(whichfork);
struct xfs_bmbt_irec old;
mp = ip->i_mount;
XFS_STATS_INC(mp, xs_del_exlist);
- if (whichfork == XFS_ATTR_FORK)
- state |= BMAP_ATTRFORK;
- else if (whichfork == XFS_COW_FORK)
- state |= BMAP_COWFORK;
-
ifp = XFS_IFORK_PTR(ip, whichfork);
ASSERT((*idx >= 0) && (*idx < xfs_iext_count(ifp)));
ASSERT(del->br_blockcount > 0);
diff --git a/fs/xfs/libxfs/xfs_bmap.h b/fs/xfs/libxfs/xfs_bmap.h
index 851982a5dfbc..a61c5480b6ad 100644
--- a/fs/xfs/libxfs/xfs_bmap.h
+++ b/fs/xfs/libxfs/xfs_bmap.h
@@ -277,4 +277,16 @@ int xfs_bmap_map_extent(struct xfs_mount *mp, struct xfs_defer_ops *dfops,
int xfs_bmap_unmap_extent(struct xfs_mount *mp, struct xfs_defer_ops *dfops,
struct xfs_inode *ip, struct xfs_bmbt_irec *imap);
+static inline int xfs_bmap_fork_to_state(int whichfork)
+{
+ switch (whichfork) {
+ case XFS_ATTR_FORK:
+ return BMAP_ATTRFORK;
+ case XFS_COW_FORK:
+ return BMAP_COWFORK;
+ default:
+ return 0;
+ }
+}
+
#endif /* __XFS_BMAP_H__ */
--
2.14.1
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [PATCH 01/15] xfs: add a xfs_bmap_fork_to_state helper
2017-10-19 6:59 ` [PATCH 01/15] xfs: add a xfs_bmap_fork_to_state helper Christoph Hellwig
@ 2017-10-19 22:48 ` Darrick J. Wong
0 siblings, 0 replies; 36+ messages in thread
From: Darrick J. Wong @ 2017-10-19 22:48 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs
On Thu, Oct 19, 2017 at 08:59:28AM +0200, Christoph Hellwig wrote:
> This creates the right initial bmap state from the passed in inode
> fork enum.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
Looks ok,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
> fs/xfs/libxfs/xfs_bmap.c | 45 +++++++++------------------------------------
> fs/xfs/libxfs/xfs_bmap.h | 12 ++++++++++++
> 2 files changed, 21 insertions(+), 36 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index 6c2ae3011964..676fc8aed67f 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -499,12 +499,7 @@ xfs_bmap_trace_exlist(
> {
> xfs_extnum_t idx; /* extent record index */
> xfs_ifork_t *ifp; /* inode fork pointer */
> - int state = 0;
> -
> - if (whichfork == XFS_ATTR_FORK)
> - state |= BMAP_ATTRFORK;
> - else if (whichfork == XFS_COW_FORK)
> - state |= BMAP_COWFORK;
> + int state = xfs_bmap_fork_to_state(whichfork);
>
> ifp = XFS_IFORK_PTR(ip, whichfork);
> ASSERT(cnt == xfs_iext_count(ifp));
> @@ -925,8 +920,7 @@ xfs_bmap_local_to_extents(
> rec.br_state = XFS_EXT_NORM;
> xfs_iext_insert(ip, 0, 1, &rec, 0);
>
> - trace_xfs_bmap_post_update(ip, 0,
> - whichfork == XFS_ATTR_FORK ? BMAP_ATTRFORK : 0,
> + trace_xfs_bmap_post_update(ip, 0, xfs_bmap_fork_to_state(whichfork),
> _THIS_IP_);
> XFS_IFORK_NEXT_SET(ip, whichfork, 1);
> ip->i_d.di_nblocks = 1;
> @@ -1571,7 +1565,7 @@ xfs_bmap_add_extent_delay_real(
> xfs_bmbt_irec_t r[3]; /* neighbor extent entries */
> /* left is 0, right is 1, prev is 2 */
> int rval=0; /* return value (logging flags) */
> - int state = 0;/* state bits, accessed thru macros */
> + int state = xfs_bmap_fork_to_state(whichfork);
> xfs_filblks_t da_new; /* new count del alloc blocks used */
> xfs_filblks_t da_old; /* old count del alloc blocks used */
> xfs_filblks_t temp=0; /* value for da_new calculations */
> @@ -1598,9 +1592,6 @@ xfs_bmap_add_extent_delay_real(
> #define RIGHT r[1]
> #define PREV r[2]
>
> - if (whichfork == XFS_COW_FORK)
> - state |= BMAP_COWFORK;
> -
> /*
> * Set up a bunch of variables to make the tests simpler.
> */
> @@ -2108,7 +2099,7 @@ xfs_bmap_add_extent_unwritten_real(
> xfs_bmbt_irec_t r[3]; /* neighbor extent entries */
> /* left is 0, right is 1, prev is 2 */
> int rval=0; /* return value (logging flags) */
> - int state = 0;/* state bits, accessed thru macros */
> + int state = xfs_bmap_fork_to_state(whichfork);
> struct xfs_mount *mp = ip->i_mount;
> struct xfs_bmbt_irec old;
>
> @@ -2116,8 +2107,6 @@ xfs_bmap_add_extent_unwritten_real(
>
> cur = *curp;
> ifp = XFS_IFORK_PTR(ip, whichfork);
> - if (whichfork == XFS_COW_FORK)
> - state |= BMAP_COWFORK;
>
> ASSERT(*idx >= 0);
> ASSERT(*idx <= xfs_iext_count(ifp));
> @@ -2601,13 +2590,10 @@ xfs_bmap_add_extent_hole_delay(
> xfs_filblks_t newlen=0; /* new indirect size */
> xfs_filblks_t oldlen=0; /* old indirect size */
> xfs_bmbt_irec_t right; /* right neighbor extent entry */
> - int state; /* state bits, accessed thru macros */
> + int state = xfs_bmap_fork_to_state(whichfork);
> xfs_filblks_t temp; /* temp for indirect calculations */
>
> ifp = XFS_IFORK_PTR(ip, whichfork);
> - state = 0;
> - if (whichfork == XFS_COW_FORK)
> - state |= BMAP_COWFORK;
> ASSERT(isnullstartblock(new->br_startblock));
>
> /*
> @@ -2760,7 +2746,7 @@ xfs_bmap_add_extent_hole_real(
> xfs_bmbt_irec_t left; /* left neighbor extent entry */
> xfs_bmbt_irec_t right; /* right neighbor extent entry */
> int rval=0; /* return value (logging flags) */
> - int state; /* state bits, accessed thru macros */
> + int state = xfs_bmap_fork_to_state(whichfork);
> struct xfs_bmbt_irec old;
>
> ASSERT(*idx >= 0);
> @@ -2770,12 +2756,6 @@ xfs_bmap_add_extent_hole_real(
>
> XFS_STATS_INC(mp, xs_add_exlist);
>
> - state = 0;
> - if (whichfork == XFS_ATTR_FORK)
> - state |= BMAP_ATTRFORK;
> - if (whichfork == XFS_COW_FORK)
> - state |= BMAP_COWFORK;
> -
> /*
> * Check and set flags if this segment has a left neighbor.
> */
> @@ -4737,7 +4717,8 @@ xfs_bmap_del_extent_delay(
> int64_t da_old, da_new, da_diff = 0;
> xfs_fileoff_t del_endoff, got_endoff;
> xfs_filblks_t got_indlen, new_indlen, stolen;
> - int error = 0, state = 0;
> + int state = xfs_bmap_fork_to_state(whichfork);
> + int error = 0;
> bool isrt;
>
> XFS_STATS_INC(mp, xs_del_exlist);
> @@ -4773,9 +4754,6 @@ xfs_bmap_del_extent_delay(
> return error;
> ip->i_delayed_blks -= del->br_blockcount;
>
> - if (whichfork == XFS_COW_FORK)
> - state |= BMAP_COWFORK;
> -
> if (got->br_startoff == del->br_startoff)
> state |= BMAP_LEFT_FILLING;
> if (got_endoff == del_endoff)
> @@ -4969,17 +4947,12 @@ xfs_bmap_del_extent_real(
> xfs_bmbt_irec_t new; /* new record to be inserted */
> /* REFERENCED */
> uint qfield; /* quota field to update */
> - int state = 0;
> + int state = xfs_bmap_fork_to_state(whichfork);
> struct xfs_bmbt_irec old;
>
> mp = ip->i_mount;
> XFS_STATS_INC(mp, xs_del_exlist);
>
> - if (whichfork == XFS_ATTR_FORK)
> - state |= BMAP_ATTRFORK;
> - else if (whichfork == XFS_COW_FORK)
> - state |= BMAP_COWFORK;
> -
> ifp = XFS_IFORK_PTR(ip, whichfork);
> ASSERT((*idx >= 0) && (*idx < xfs_iext_count(ifp)));
> ASSERT(del->br_blockcount > 0);
> diff --git a/fs/xfs/libxfs/xfs_bmap.h b/fs/xfs/libxfs/xfs_bmap.h
> index 851982a5dfbc..a61c5480b6ad 100644
> --- a/fs/xfs/libxfs/xfs_bmap.h
> +++ b/fs/xfs/libxfs/xfs_bmap.h
> @@ -277,4 +277,16 @@ int xfs_bmap_map_extent(struct xfs_mount *mp, struct xfs_defer_ops *dfops,
> int xfs_bmap_unmap_extent(struct xfs_mount *mp, struct xfs_defer_ops *dfops,
> struct xfs_inode *ip, struct xfs_bmbt_irec *imap);
>
> +static inline int xfs_bmap_fork_to_state(int whichfork)
> +{
> + switch (whichfork) {
> + case XFS_ATTR_FORK:
> + return BMAP_ATTRFORK;
> + case XFS_COW_FORK:
> + return BMAP_COWFORK;
> + default:
> + return 0;
> + }
> +}
> +
> #endif /* __XFS_BMAP_H__ */
> --
> 2.14.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH 02/15] xfs: make better use of the 'state' variable in xfs_bmap_del_extent_real
2017-10-19 6:59 more extent mapping cleanups Christoph Hellwig
2017-10-19 6:59 ` [PATCH 01/15] xfs: add a xfs_bmap_fork_to_state helper Christoph Hellwig
@ 2017-10-19 6:59 ` Christoph Hellwig
2017-10-19 22:49 ` Darrick J. Wong
2017-10-19 6:59 ` [PATCH 03/15] xfs: remove post-bmap tracing in xfs_bmap_local_to_extents Christoph Hellwig
` (13 subsequent siblings)
15 siblings, 1 reply; 36+ messages in thread
From: Christoph Hellwig @ 2017-10-19 6:59 UTC (permalink / raw)
To: linux-xfs
We already have all the information about the fork aѕ well as additional
tracing information, so pass that to xfs_iext_remove().
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/xfs/libxfs/xfs_bmap.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 676fc8aed67f..06a9ac7dcdab 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -5022,8 +5022,7 @@ xfs_bmap_del_extent_real(
* Matches the whole extent. Delete the entry.
*/
trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_);
- xfs_iext_remove(ip, *idx, 1,
- whichfork == XFS_ATTR_FORK ? BMAP_ATTRFORK : 0);
+ xfs_iext_remove(ip, *idx, 1, state);
--*idx;
XFS_IFORK_NEXT_SET(ip, whichfork,
--
2.14.1
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [PATCH 02/15] xfs: make better use of the 'state' variable in xfs_bmap_del_extent_real
2017-10-19 6:59 ` [PATCH 02/15] xfs: make better use of the 'state' variable in xfs_bmap_del_extent_real Christoph Hellwig
@ 2017-10-19 22:49 ` Darrick J. Wong
0 siblings, 0 replies; 36+ messages in thread
From: Darrick J. Wong @ 2017-10-19 22:49 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs
On Thu, Oct 19, 2017 at 08:59:29AM +0200, Christoph Hellwig wrote:
> We already have all the information about the fork aѕ well as additional
> tracing information, so pass that to xfs_iext_remove().
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
Looks ok,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
> fs/xfs/libxfs/xfs_bmap.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index 676fc8aed67f..06a9ac7dcdab 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -5022,8 +5022,7 @@ xfs_bmap_del_extent_real(
> * Matches the whole extent. Delete the entry.
> */
> trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_);
> - xfs_iext_remove(ip, *idx, 1,
> - whichfork == XFS_ATTR_FORK ? BMAP_ATTRFORK : 0);
> + xfs_iext_remove(ip, *idx, 1, state);
> --*idx;
>
> XFS_IFORK_NEXT_SET(ip, whichfork,
> --
> 2.14.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH 03/15] xfs: remove post-bmap tracing in xfs_bmap_local_to_extents
2017-10-19 6:59 more extent mapping cleanups Christoph Hellwig
2017-10-19 6:59 ` [PATCH 01/15] xfs: add a xfs_bmap_fork_to_state helper Christoph Hellwig
2017-10-19 6:59 ` [PATCH 02/15] xfs: make better use of the 'state' variable in xfs_bmap_del_extent_real Christoph Hellwig
@ 2017-10-19 6:59 ` Christoph Hellwig
2017-10-19 22:49 ` Darrick J. Wong
2017-10-19 6:59 ` [PATCH 04/15] xfs: move pre/post-bmap tracing into xfs_iext_update_extent Christoph Hellwig
` (12 subsequent siblings)
15 siblings, 1 reply; 36+ messages in thread
From: Christoph Hellwig @ 2017-10-19 6:59 UTC (permalink / raw)
To: linux-xfs
Now that we use xfs_iext_insert this is already covered by the tracing
in that function.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/xfs/libxfs/xfs_bmap.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 06a9ac7dcdab..8c2e5d6bced2 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -920,8 +920,6 @@ xfs_bmap_local_to_extents(
rec.br_state = XFS_EXT_NORM;
xfs_iext_insert(ip, 0, 1, &rec, 0);
- trace_xfs_bmap_post_update(ip, 0, xfs_bmap_fork_to_state(whichfork),
- _THIS_IP_);
XFS_IFORK_NEXT_SET(ip, whichfork, 1);
ip->i_d.di_nblocks = 1;
xfs_trans_mod_dquot_byino(tp, ip,
--
2.14.1
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [PATCH 03/15] xfs: remove post-bmap tracing in xfs_bmap_local_to_extents
2017-10-19 6:59 ` [PATCH 03/15] xfs: remove post-bmap tracing in xfs_bmap_local_to_extents Christoph Hellwig
@ 2017-10-19 22:49 ` Darrick J. Wong
0 siblings, 0 replies; 36+ messages in thread
From: Darrick J. Wong @ 2017-10-19 22:49 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs
On Thu, Oct 19, 2017 at 08:59:30AM +0200, Christoph Hellwig wrote:
> Now that we use xfs_iext_insert this is already covered by the tracing
> in that function.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
Looks ok,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
> fs/xfs/libxfs/xfs_bmap.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index 06a9ac7dcdab..8c2e5d6bced2 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -920,8 +920,6 @@ xfs_bmap_local_to_extents(
> rec.br_state = XFS_EXT_NORM;
> xfs_iext_insert(ip, 0, 1, &rec, 0);
>
> - trace_xfs_bmap_post_update(ip, 0, xfs_bmap_fork_to_state(whichfork),
> - _THIS_IP_);
> XFS_IFORK_NEXT_SET(ip, whichfork, 1);
> ip->i_d.di_nblocks = 1;
> xfs_trans_mod_dquot_byino(tp, ip,
> --
> 2.14.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH 04/15] xfs: move pre/post-bmap tracing into xfs_iext_update_extent
2017-10-19 6:59 more extent mapping cleanups Christoph Hellwig
` (2 preceding siblings ...)
2017-10-19 6:59 ` [PATCH 03/15] xfs: remove post-bmap tracing in xfs_bmap_local_to_extents Christoph Hellwig
@ 2017-10-19 6:59 ` Christoph Hellwig
2017-10-19 22:50 ` Darrick J. Wong
2017-10-19 6:59 ` [PATCH 05/15] xfs: remove XFS_BMAP_TRACE_EXLIST Christoph Hellwig
` (11 subsequent siblings)
15 siblings, 1 reply; 36+ messages in thread
From: Christoph Hellwig @ 2017-10-19 6:59 UTC (permalink / raw)
To: linux-xfs
xfs_iext_update_extent already has basically all the information needed
to centralize the bmap pre/post tracing. We just need to pass inode +
bmap state instead of the inode fork pointer to get all trace annotations.
In addition to covering all the existing trace points this gives us
tracing coverage for the extent shifting operations for free.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/xfs/libxfs/xfs_bmap.c | 167 ++++++++++++-----------------------------
fs/xfs/libxfs/xfs_inode_fork.c | 7 +-
fs/xfs/libxfs/xfs_inode_fork.h | 4 +-
3 files changed, 55 insertions(+), 123 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 8c2e5d6bced2..ff9a8e1d4a96 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -1669,10 +1669,8 @@ xfs_bmap_add_extent_delay_real(
* The left and right neighbors are both contiguous with new.
*/
bma->idx--;
- trace_xfs_bmap_pre_update(bma->ip, bma->idx, state, _THIS_IP_);
LEFT.br_blockcount += PREV.br_blockcount + RIGHT.br_blockcount;
- xfs_iext_update_extent(ifp, bma->idx, &LEFT);
- trace_xfs_bmap_post_update(bma->ip, bma->idx, state, _THIS_IP_);
+ xfs_iext_update_extent(bma->ip, state, bma->idx, &LEFT);
xfs_iext_remove(bma->ip, bma->idx + 1, 2, state);
(*nextents)--;
@@ -1706,10 +1704,8 @@ xfs_bmap_add_extent_delay_real(
bma->idx--;
old = LEFT;
- trace_xfs_bmap_pre_update(bma->ip, bma->idx, state, _THIS_IP_);
LEFT.br_blockcount += PREV.br_blockcount;
- xfs_iext_update_extent(ifp, bma->idx, &LEFT);
- trace_xfs_bmap_post_update(bma->ip, bma->idx, state, _THIS_IP_);
+ xfs_iext_update_extent(bma->ip, state, bma->idx, &LEFT);
xfs_iext_remove(bma->ip, bma->idx + 1, 1, state);
if (bma->cur == NULL)
@@ -1731,11 +1727,9 @@ xfs_bmap_add_extent_delay_real(
* Filling in all of a previously delayed allocation extent.
* The right neighbor is contiguous, the left is not.
*/
- trace_xfs_bmap_pre_update(bma->ip, bma->idx, state, _THIS_IP_);
PREV.br_startblock = new->br_startblock;
PREV.br_blockcount += RIGHT.br_blockcount;
- xfs_iext_update_extent(ifp, bma->idx, &PREV);
- trace_xfs_bmap_post_update(bma->ip, bma->idx, state, _THIS_IP_);
+ xfs_iext_update_extent(bma->ip, state, bma->idx, &PREV);
xfs_iext_remove(bma->ip, bma->idx + 1, 1, state);
if (bma->cur == NULL)
@@ -1758,11 +1752,9 @@ xfs_bmap_add_extent_delay_real(
* Neither the left nor right neighbors are contiguous with
* the new one.
*/
- trace_xfs_bmap_pre_update(bma->ip, bma->idx, state, _THIS_IP_);
PREV.br_startblock = new->br_startblock;
PREV.br_state = new->br_state;
- xfs_iext_update_extent(ifp, bma->idx, &PREV);
- trace_xfs_bmap_post_update(bma->ip, bma->idx, state, _THIS_IP_);
+ xfs_iext_update_extent(bma->ip, state, bma->idx, &PREV);
(*nextents)++;
if (bma->cur == NULL)
@@ -1790,17 +1782,13 @@ xfs_bmap_add_extent_delay_real(
da_new = XFS_FILBLKS_MIN(xfs_bmap_worst_indlen(bma->ip, temp),
startblockval(PREV.br_startblock));
- trace_xfs_bmap_pre_update(bma->ip, bma->idx - 1, state, _THIS_IP_);
LEFT.br_blockcount += new->br_blockcount;
- xfs_iext_update_extent(ifp, bma->idx - 1, &LEFT);
- trace_xfs_bmap_post_update(bma->ip, bma->idx - 1, state, _THIS_IP_);
+ xfs_iext_update_extent(bma->ip, state, bma->idx - 1, &LEFT);
- trace_xfs_bmap_pre_update(bma->ip, bma->idx, state, _THIS_IP_);
PREV.br_blockcount = temp = PREV.br_blockcount - new->br_blockcount;
PREV.br_startoff += new->br_blockcount;
PREV.br_startblock = nullstartblock(da_new);
- xfs_iext_update_extent(ifp, bma->idx, &PREV);
- trace_xfs_bmap_post_update(bma->ip, bma->idx, state, _THIS_IP_);
+ xfs_iext_update_extent(bma->ip, state, bma->idx, &PREV);
if (bma->cur == NULL)
rval = XFS_ILOG_DEXT;
@@ -1853,13 +1841,10 @@ xfs_bmap_add_extent_delay_real(
startblockval(PREV.br_startblock) -
(bma->cur ? bma->cur->bc_private.b.allocated : 0));
- trace_xfs_bmap_pre_update(bma->ip, bma->idx + 1, state, _THIS_IP_);
PREV.br_startoff = new_endoff;
PREV.br_blockcount = temp;
PREV.br_startblock = nullstartblock(da_new);
- xfs_iext_update_extent(ifp, bma->idx + 1, &PREV);
- trace_xfs_bmap_post_update(bma->ip, bma->idx + 1, state, _THIS_IP_);
-
+ xfs_iext_update_extent(bma->ip, state, bma->idx + 1, &PREV);
break;
case BMAP_RIGHT_FILLING | BMAP_RIGHT_CONTIG:
@@ -1868,12 +1853,10 @@ xfs_bmap_add_extent_delay_real(
* The right neighbor is contiguous with the new allocation.
*/
old = RIGHT;
- trace_xfs_bmap_pre_update(bma->ip, bma->idx + 1, state, _THIS_IP_);
RIGHT.br_startoff = new->br_startoff;
RIGHT.br_startblock = new->br_startblock;
RIGHT.br_blockcount += new->br_blockcount;
- xfs_iext_update_extent(ifp, bma->idx + 1, &RIGHT);
- trace_xfs_bmap_post_update(bma->ip, bma->idx + 1, state, _THIS_IP_);
+ xfs_iext_update_extent(bma->ip, state, bma->idx + 1, &RIGHT);
if (bma->cur == NULL)
rval = XFS_ILOG_DEXT;
@@ -1892,11 +1875,9 @@ xfs_bmap_add_extent_delay_real(
da_new = XFS_FILBLKS_MIN(xfs_bmap_worst_indlen(bma->ip, temp),
startblockval(PREV.br_startblock));
- trace_xfs_bmap_pre_update(bma->ip, bma->idx, state, _THIS_IP_);
PREV.br_blockcount = temp;
PREV.br_startblock = nullstartblock(da_new);
- xfs_iext_update_extent(ifp, bma->idx, &PREV);
- trace_xfs_bmap_post_update(bma->ip, bma->idx, state, _THIS_IP_);
+ xfs_iext_update_extent(bma->ip, state, bma->idx, &PREV);
bma->idx++;
break;
@@ -1936,11 +1917,9 @@ xfs_bmap_add_extent_delay_real(
startblockval(PREV.br_startblock) -
(bma->cur ? bma->cur->bc_private.b.allocated : 0));
- trace_xfs_bmap_pre_update(bma->ip, bma->idx, state, _THIS_IP_);
PREV.br_startblock = nullstartblock(da_new);
PREV.br_blockcount = temp;
- xfs_iext_update_extent(ifp, bma->idx, &PREV);
- trace_xfs_bmap_post_update(bma->ip, bma->idx, state, _THIS_IP_);
+ xfs_iext_update_extent(bma->ip, state, bma->idx, &PREV);
bma->idx++;
break;
@@ -1981,13 +1960,11 @@ xfs_bmap_add_extent_delay_real(
RIGHT.br_blockcount));
/* truncate PREV */
- trace_xfs_bmap_pre_update(bma->ip, bma->idx, 0, _THIS_IP_);
PREV.br_blockcount = new->br_startoff - PREV.br_startoff;
PREV.br_startblock =
nullstartblock(xfs_bmap_worst_indlen(bma->ip,
PREV.br_blockcount));
- xfs_iext_update_extent(ifp, bma->idx, &PREV);
- trace_xfs_bmap_post_update(bma->ip, bma->idx, state, _THIS_IP_);
+ xfs_iext_update_extent(bma->ip, state, bma->idx, &PREV);
/* insert LEFT (r[0]) and RIGHT (r[1]) at the same time */
xfs_iext_insert(bma->ip, bma->idx + 1, 2, &LEFT, state);
@@ -2192,10 +2169,8 @@ xfs_bmap_add_extent_unwritten_real(
*/
--*idx;
- trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_);
LEFT.br_blockcount += PREV.br_blockcount + RIGHT.br_blockcount;
- xfs_iext_update_extent(ifp, *idx, &LEFT);
- trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_);
+ xfs_iext_update_extent(ip, state, *idx, &LEFT);
xfs_iext_remove(ip, *idx + 1, 2, state);
XFS_IFORK_NEXT_SET(ip, whichfork,
@@ -2233,10 +2208,8 @@ xfs_bmap_add_extent_unwritten_real(
*/
--*idx;
- trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_);
LEFT.br_blockcount += PREV.br_blockcount;
- xfs_iext_update_extent(ifp, *idx, &LEFT);
- trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_);
+ xfs_iext_update_extent(ip, state, *idx, &LEFT);
xfs_iext_remove(ip, *idx + 1, 1, state);
XFS_IFORK_NEXT_SET(ip, whichfork,
@@ -2266,11 +2239,9 @@ xfs_bmap_add_extent_unwritten_real(
* Setting all of a previous oldext extent to newext.
* The right neighbor is contiguous, the left is not.
*/
- trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_);
PREV.br_blockcount += RIGHT.br_blockcount;
PREV.br_state = new->br_state;
- xfs_iext_update_extent(ifp, *idx, &PREV);
- trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_);
+ xfs_iext_update_extent(ip, state, *idx, &PREV);
xfs_iext_remove(ip, *idx + 1, 1, state);
XFS_IFORK_NEXT_SET(ip, whichfork,
@@ -2301,10 +2272,8 @@ xfs_bmap_add_extent_unwritten_real(
* Neither the left nor right neighbors are contiguous with
* the new one.
*/
- trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_);
PREV.br_state = new->br_state;
- xfs_iext_update_extent(ifp, *idx, &PREV);
- trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_);
+ xfs_iext_update_extent(ip, state, *idx, &PREV);
if (cur == NULL)
rval = XFS_ILOG_DEXT;
@@ -2325,18 +2294,14 @@ xfs_bmap_add_extent_unwritten_real(
* Setting the first part of a previous oldext extent to newext.
* The left neighbor is contiguous.
*/
- trace_xfs_bmap_pre_update(ip, *idx - 1, state, _THIS_IP_);
LEFT.br_blockcount += new->br_blockcount;
- xfs_iext_update_extent(ifp, *idx - 1, &LEFT);
- trace_xfs_bmap_post_update(ip, *idx - 1, state, _THIS_IP_);
+ xfs_iext_update_extent(ip, state, *idx - 1, &LEFT);
old = PREV;
- trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_);
PREV.br_startoff += new->br_blockcount;
PREV.br_startblock += new->br_blockcount;
PREV.br_blockcount -= new->br_blockcount;
- xfs_iext_update_extent(ifp, *idx, &PREV);
- trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_);
+ xfs_iext_update_extent(ip, state, *idx, &PREV);
--*idx;
@@ -2366,12 +2331,10 @@ xfs_bmap_add_extent_unwritten_real(
* The left neighbor is not contiguous.
*/
old = PREV;
- trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_);
PREV.br_startoff += new->br_blockcount;
PREV.br_startblock += new->br_blockcount;
PREV.br_blockcount -= new->br_blockcount;
- xfs_iext_update_extent(ifp, *idx, &PREV);
- trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_);
+ xfs_iext_update_extent(ip, state, *idx, &PREV);
xfs_iext_insert(ip, *idx, 1, new, state);
XFS_IFORK_NEXT_SET(ip, whichfork,
@@ -2400,19 +2363,15 @@ xfs_bmap_add_extent_unwritten_real(
* The right neighbor is contiguous with the new allocation.
*/
old = PREV;
- trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_);
PREV.br_blockcount -= new->br_blockcount;
- xfs_iext_update_extent(ifp, *idx, &PREV);
- trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_);
+ xfs_iext_update_extent(ip, state, *idx, &PREV);
++*idx;
- trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_);
RIGHT.br_startoff = new->br_startoff;
RIGHT.br_startblock = new->br_startblock;
RIGHT.br_blockcount += new->br_blockcount;
- xfs_iext_update_extent(ifp, *idx, &RIGHT);
- trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_);
+ xfs_iext_update_extent(ip, state, *idx, &RIGHT);
if (cur == NULL)
rval = XFS_ILOG_DEXT;
@@ -2440,10 +2399,8 @@ xfs_bmap_add_extent_unwritten_real(
* The right neighbor is not contiguous.
*/
old = PREV;
- trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_);
PREV.br_blockcount -= new->br_blockcount;
- xfs_iext_update_extent(ifp, *idx, &PREV);
- trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_);
+ xfs_iext_update_extent(ip, state, *idx, &PREV);
++*idx;
xfs_iext_insert(ip, *idx, 1, new, state);
@@ -2478,10 +2435,8 @@ xfs_bmap_add_extent_unwritten_real(
* One extent becomes three extents.
*/
old = PREV;
- trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_);
PREV.br_blockcount = new->br_startoff - PREV.br_startoff;
- xfs_iext_update_extent(ifp, *idx, &PREV);
- trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_);
+ xfs_iext_update_extent(ip, state, *idx, &PREV);
r[0] = *new;
r[1].br_startoff = new_endoff;
@@ -2648,7 +2603,6 @@ xfs_bmap_add_extent_hole_delay(
temp = left.br_blockcount + new->br_blockcount +
right.br_blockcount;
- trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_);
oldlen = startblockval(left.br_startblock) +
startblockval(new->br_startblock) +
startblockval(right.br_startblock);
@@ -2656,8 +2610,7 @@ xfs_bmap_add_extent_hole_delay(
oldlen);
left.br_startblock = nullstartblock(newlen);
left.br_blockcount = temp;
- xfs_iext_update_extent(ifp, *idx, &left);
- trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_);
+ xfs_iext_update_extent(ip, state, *idx, &left);
xfs_iext_remove(ip, *idx + 1, 1, state);
break;
@@ -2671,15 +2624,13 @@ xfs_bmap_add_extent_hole_delay(
--*idx;
temp = left.br_blockcount + new->br_blockcount;
- trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_);
oldlen = startblockval(left.br_startblock) +
startblockval(new->br_startblock);
newlen = XFS_FILBLKS_MIN(xfs_bmap_worst_indlen(ip, temp),
oldlen);
left.br_blockcount = temp;
left.br_startblock = nullstartblock(newlen);
- xfs_iext_update_extent(ifp, *idx, &left);
- trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_);
+ xfs_iext_update_extent(ip, state, *idx, &left);
break;
case BMAP_RIGHT_CONTIG:
@@ -2688,7 +2639,6 @@ xfs_bmap_add_extent_hole_delay(
* on the right.
* Merge the new allocation with the right neighbor.
*/
- trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_);
temp = new->br_blockcount + right.br_blockcount;
oldlen = startblockval(new->br_startblock) +
startblockval(right.br_startblock);
@@ -2697,8 +2647,7 @@ xfs_bmap_add_extent_hole_delay(
right.br_startoff = new->br_startoff;
right.br_startblock = nullstartblock(newlen);
right.br_blockcount = temp;
- xfs_iext_update_extent(ifp, *idx, &right);
- trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_);
+ xfs_iext_update_extent(ip, state, *idx, &right);
break;
case 0:
@@ -2808,10 +2757,8 @@ xfs_bmap_add_extent_hole_real(
* Merge all three into a single extent record.
*/
--*idx;
- trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_);
left.br_blockcount += new->br_blockcount + right.br_blockcount;
- xfs_iext_update_extent(ifp, *idx, &left);
- trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_);
+ xfs_iext_update_extent(ip, state, *idx, &left);
xfs_iext_remove(ip, *idx + 1, 1, state);
@@ -2847,10 +2794,9 @@ xfs_bmap_add_extent_hole_real(
*/
--*idx;
old = left;
- trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_);
+
left.br_blockcount += new->br_blockcount;
- xfs_iext_update_extent(ifp, *idx, &left);
- trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_);
+ xfs_iext_update_extent(ip, state, *idx, &left);
if (cur == NULL) {
rval = xfs_ilog_fext(whichfork);
@@ -2873,12 +2819,11 @@ xfs_bmap_add_extent_hole_real(
* Merge the new allocation with the right neighbor.
*/
old = right;
- trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_);
+
right.br_startoff = new->br_startoff;
right.br_startblock = new->br_startblock;
right.br_blockcount += new->br_blockcount;
- xfs_iext_update_extent(ifp, *idx, &right);
- trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_);
+ xfs_iext_update_extent(ip, state, *idx, &right);
if (cur == NULL) {
rval = xfs_ilog_fext(whichfork);
@@ -4769,26 +4714,22 @@ xfs_bmap_del_extent_delay(
/*
* Deleting the first part of the extent.
*/
- trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_);
got->br_startoff = del_endoff;
got->br_blockcount -= del->br_blockcount;
da_new = XFS_FILBLKS_MIN(xfs_bmap_worst_indlen(ip,
got->br_blockcount), da_old);
got->br_startblock = nullstartblock((int)da_new);
- xfs_iext_update_extent(ifp, *idx, got);
- trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_);
+ xfs_iext_update_extent(ip, state, *idx, got);
break;
case BMAP_RIGHT_FILLING:
/*
* Deleting the last part of the extent.
*/
- trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_);
got->br_blockcount = got->br_blockcount - del->br_blockcount;
da_new = XFS_FILBLKS_MIN(xfs_bmap_worst_indlen(ip,
got->br_blockcount), da_old);
got->br_startblock = nullstartblock((int)da_new);
- xfs_iext_update_extent(ifp, *idx, got);
- trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_);
+ xfs_iext_update_extent(ip, state, *idx, got);
break;
case 0:
/*
@@ -4800,8 +4741,6 @@ xfs_bmap_del_extent_delay(
* Warn if either of the new indlen reservations is zero as this
* can lead to delalloc problems.
*/
- trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_);
-
got->br_blockcount = del->br_startoff - got->br_startoff;
got_indlen = xfs_bmap_worst_indlen(ip, got->br_blockcount);
@@ -4813,8 +4752,7 @@ xfs_bmap_del_extent_delay(
del->br_blockcount);
got->br_startblock = nullstartblock((int)got_indlen);
- xfs_iext_update_extent(ifp, *idx, got);
- trace_xfs_bmap_post_update(ip, *idx, 0, _THIS_IP_);
+ xfs_iext_update_extent(ip, state, *idx, got);
new.br_startoff = del_endoff;
new.br_state = got->br_state;
@@ -4879,30 +4817,24 @@ xfs_bmap_del_extent_cow(
/*
* Deleting the first part of the extent.
*/
- trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_);
got->br_startoff = del_endoff;
got->br_blockcount -= del->br_blockcount;
got->br_startblock = del->br_startblock + del->br_blockcount;
- xfs_iext_update_extent(ifp, *idx, got);
- trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_);
+ xfs_iext_update_extent(ip, state, *idx, got);
break;
case BMAP_RIGHT_FILLING:
/*
* Deleting the last part of the extent.
*/
- trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_);
got->br_blockcount -= del->br_blockcount;
- xfs_iext_update_extent(ifp, *idx, got);
- trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_);
+ xfs_iext_update_extent(ip, state, *idx, got);
break;
case 0:
/*
* Deleting the middle of the extent.
*/
- trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_);
got->br_blockcount = del->br_startoff - got->br_startoff;
- xfs_iext_update_extent(ifp, *idx, got);
- trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_);
+ xfs_iext_update_extent(ip, state, *idx, got);
new.br_startoff = del_endoff;
new.br_blockcount = got_endoff - del_endoff;
@@ -5019,7 +4951,6 @@ xfs_bmap_del_extent_real(
/*
* Matches the whole extent. Delete the entry.
*/
- trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_);
xfs_iext_remove(ip, *idx, 1, state);
--*idx;
@@ -5038,12 +4969,10 @@ xfs_bmap_del_extent_real(
/*
* Deleting the first part of the extent.
*/
- trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_);
got.br_startoff = del_endoff;
got.br_startblock = del_endblock;
got.br_blockcount -= del->br_blockcount;
- xfs_iext_update_extent(ifp, *idx, &got);
- trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_);
+ xfs_iext_update_extent(ip, state, *idx, &got);
if (!cur) {
flags |= xfs_ilog_fext(whichfork);
break;
@@ -5056,10 +4985,8 @@ xfs_bmap_del_extent_real(
/*
* Deleting the last part of the extent.
*/
- trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_);
got.br_blockcount -= del->br_blockcount;
- xfs_iext_update_extent(ifp, *idx, &got);
- trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_);
+ xfs_iext_update_extent(ip, state, *idx, &got);
if (!cur) {
flags |= xfs_ilog_fext(whichfork);
break;
@@ -5072,11 +4999,10 @@ xfs_bmap_del_extent_real(
/*
* Deleting the middle of the extent.
*/
- trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_);
-
old = got;
+
got.br_blockcount = del->br_startoff - got.br_startoff;
- xfs_iext_update_extent(ifp, *idx, &got);
+ xfs_iext_update_extent(ip, state, *idx, &got);
new.br_startoff = del_endoff;
new.br_blockcount = got_endoff - del_endoff;
@@ -5120,7 +5046,7 @@ xfs_bmap_del_extent_real(
* Reset the extent record back
* to the original value.
*/
- xfs_iext_update_extent(ifp, *idx, &old);
+ xfs_iext_update_extent(ip, state, *idx, &old);
flags = 0;
error = -ENOSPC;
goto done;
@@ -5130,7 +5056,6 @@ xfs_bmap_del_extent_real(
flags |= xfs_ilog_fext(whichfork);
XFS_IFORK_NEXT_SET(ip, whichfork,
XFS_IFORK_NEXTENTS(ip, whichfork) + 1);
- trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_);
xfs_iext_insert(ip, *idx + 1, 1, &new, state);
++*idx;
break;
@@ -5605,7 +5530,6 @@ xfs_bmse_merge(
int *logflags, /* output */
struct xfs_defer_ops *dfops)
{
- struct xfs_ifork *ifp = XFS_IFORK_PTR(ip, whichfork);
struct xfs_bmbt_irec new;
xfs_filblks_t blockcount;
int error, i;
@@ -5654,7 +5578,8 @@ xfs_bmse_merge(
return error;
done:
- xfs_iext_update_extent(ifp, current_ext - 1, &new);
+ xfs_iext_update_extent(ip, xfs_bmap_fork_to_state(whichfork),
+ current_ext - 1, &new);
xfs_iext_remove(ip, current_ext, 1, 0);
/* update reverse mapping. rmap functions merge the rmaps for us */
@@ -5772,7 +5697,8 @@ xfs_bmse_shift_one(
*logflags |= XFS_ILOG_DEXT;
}
- xfs_iext_update_extent(ifp, *current_ext, &new);
+ xfs_iext_update_extent(ip, xfs_bmap_fork_to_state(whichfork),
+ *current_ext, &new);
if (direction == SHIFT_LEFT)
(*current_ext)++;
@@ -6019,7 +5945,8 @@ xfs_bmap_split_extent_at(
}
got.br_blockcount = gotblkcnt;
- xfs_iext_update_extent(ifp, current_ext, &got);
+ xfs_iext_update_extent(ip, xfs_bmap_fork_to_state(whichfork),
+ current_ext, &got);
logflags = XFS_ILOG_CORE;
if (cur) {
diff --git a/fs/xfs/libxfs/xfs_inode_fork.c b/fs/xfs/libxfs/xfs_inode_fork.c
index 31840ca24018..7f40f53e6c43 100644
--- a/fs/xfs/libxfs/xfs_inode_fork.c
+++ b/fs/xfs/libxfs/xfs_inode_fork.c
@@ -2023,12 +2023,17 @@ xfs_iext_get_extent(
void
xfs_iext_update_extent(
- struct xfs_ifork *ifp,
+ struct xfs_inode *ip,
+ int state,
xfs_extnum_t idx,
struct xfs_bmbt_irec *gotp)
{
+ struct xfs_ifork *ifp = xfs_iext_state_to_fork(ip, state);
+
ASSERT(idx >= 0);
ASSERT(idx < xfs_iext_count(ifp));
+ trace_xfs_bmap_pre_update(ip, idx, state, _RET_IP_);
xfs_bmbt_set_all(xfs_iext_get_ext(ifp, idx), gotp);
+ trace_xfs_bmap_post_update(ip, idx, state, _RET_IP_);
}
diff --git a/fs/xfs/libxfs/xfs_inode_fork.h b/fs/xfs/libxfs/xfs_inode_fork.h
index 11af705219f6..6750f0462d21 100644
--- a/fs/xfs/libxfs/xfs_inode_fork.h
+++ b/fs/xfs/libxfs/xfs_inode_fork.h
@@ -187,8 +187,8 @@ bool xfs_iext_lookup_extent(struct xfs_inode *ip,
xfs_extnum_t *idxp, struct xfs_bmbt_irec *gotp);
bool xfs_iext_get_extent(struct xfs_ifork *ifp, xfs_extnum_t idx,
struct xfs_bmbt_irec *gotp);
-void xfs_iext_update_extent(struct xfs_ifork *ifp, xfs_extnum_t idx,
- struct xfs_bmbt_irec *gotp);
+void xfs_iext_update_extent(struct xfs_inode *ip, int state,
+ xfs_extnum_t idx, struct xfs_bmbt_irec *gotp);
extern struct kmem_zone *xfs_ifork_zone;
--
2.14.1
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [PATCH 04/15] xfs: move pre/post-bmap tracing into xfs_iext_update_extent
2017-10-19 6:59 ` [PATCH 04/15] xfs: move pre/post-bmap tracing into xfs_iext_update_extent Christoph Hellwig
@ 2017-10-19 22:50 ` Darrick J. Wong
0 siblings, 0 replies; 36+ messages in thread
From: Darrick J. Wong @ 2017-10-19 22:50 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs
On Thu, Oct 19, 2017 at 08:59:31AM +0200, Christoph Hellwig wrote:
> xfs_iext_update_extent already has basically all the information needed
> to centralize the bmap pre/post tracing. We just need to pass inode +
> bmap state instead of the inode fork pointer to get all trace annotations.
>
> In addition to covering all the existing trace points this gives us
> tracing coverage for the extent shifting operations for free.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
Looks ok,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
> fs/xfs/libxfs/xfs_bmap.c | 167 ++++++++++++-----------------------------
> fs/xfs/libxfs/xfs_inode_fork.c | 7 +-
> fs/xfs/libxfs/xfs_inode_fork.h | 4 +-
> 3 files changed, 55 insertions(+), 123 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index 8c2e5d6bced2..ff9a8e1d4a96 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -1669,10 +1669,8 @@ xfs_bmap_add_extent_delay_real(
> * The left and right neighbors are both contiguous with new.
> */
> bma->idx--;
> - trace_xfs_bmap_pre_update(bma->ip, bma->idx, state, _THIS_IP_);
> LEFT.br_blockcount += PREV.br_blockcount + RIGHT.br_blockcount;
> - xfs_iext_update_extent(ifp, bma->idx, &LEFT);
> - trace_xfs_bmap_post_update(bma->ip, bma->idx, state, _THIS_IP_);
> + xfs_iext_update_extent(bma->ip, state, bma->idx, &LEFT);
>
> xfs_iext_remove(bma->ip, bma->idx + 1, 2, state);
> (*nextents)--;
> @@ -1706,10 +1704,8 @@ xfs_bmap_add_extent_delay_real(
> bma->idx--;
>
> old = LEFT;
> - trace_xfs_bmap_pre_update(bma->ip, bma->idx, state, _THIS_IP_);
> LEFT.br_blockcount += PREV.br_blockcount;
> - xfs_iext_update_extent(ifp, bma->idx, &LEFT);
> - trace_xfs_bmap_post_update(bma->ip, bma->idx, state, _THIS_IP_);
> + xfs_iext_update_extent(bma->ip, state, bma->idx, &LEFT);
>
> xfs_iext_remove(bma->ip, bma->idx + 1, 1, state);
> if (bma->cur == NULL)
> @@ -1731,11 +1727,9 @@ xfs_bmap_add_extent_delay_real(
> * Filling in all of a previously delayed allocation extent.
> * The right neighbor is contiguous, the left is not.
> */
> - trace_xfs_bmap_pre_update(bma->ip, bma->idx, state, _THIS_IP_);
> PREV.br_startblock = new->br_startblock;
> PREV.br_blockcount += RIGHT.br_blockcount;
> - xfs_iext_update_extent(ifp, bma->idx, &PREV);
> - trace_xfs_bmap_post_update(bma->ip, bma->idx, state, _THIS_IP_);
> + xfs_iext_update_extent(bma->ip, state, bma->idx, &PREV);
>
> xfs_iext_remove(bma->ip, bma->idx + 1, 1, state);
> if (bma->cur == NULL)
> @@ -1758,11 +1752,9 @@ xfs_bmap_add_extent_delay_real(
> * Neither the left nor right neighbors are contiguous with
> * the new one.
> */
> - trace_xfs_bmap_pre_update(bma->ip, bma->idx, state, _THIS_IP_);
> PREV.br_startblock = new->br_startblock;
> PREV.br_state = new->br_state;
> - xfs_iext_update_extent(ifp, bma->idx, &PREV);
> - trace_xfs_bmap_post_update(bma->ip, bma->idx, state, _THIS_IP_);
> + xfs_iext_update_extent(bma->ip, state, bma->idx, &PREV);
>
> (*nextents)++;
> if (bma->cur == NULL)
> @@ -1790,17 +1782,13 @@ xfs_bmap_add_extent_delay_real(
> da_new = XFS_FILBLKS_MIN(xfs_bmap_worst_indlen(bma->ip, temp),
> startblockval(PREV.br_startblock));
>
> - trace_xfs_bmap_pre_update(bma->ip, bma->idx - 1, state, _THIS_IP_);
> LEFT.br_blockcount += new->br_blockcount;
> - xfs_iext_update_extent(ifp, bma->idx - 1, &LEFT);
> - trace_xfs_bmap_post_update(bma->ip, bma->idx - 1, state, _THIS_IP_);
> + xfs_iext_update_extent(bma->ip, state, bma->idx - 1, &LEFT);
>
> - trace_xfs_bmap_pre_update(bma->ip, bma->idx, state, _THIS_IP_);
> PREV.br_blockcount = temp = PREV.br_blockcount - new->br_blockcount;
> PREV.br_startoff += new->br_blockcount;
> PREV.br_startblock = nullstartblock(da_new);
> - xfs_iext_update_extent(ifp, bma->idx, &PREV);
> - trace_xfs_bmap_post_update(bma->ip, bma->idx, state, _THIS_IP_);
> + xfs_iext_update_extent(bma->ip, state, bma->idx, &PREV);
>
> if (bma->cur == NULL)
> rval = XFS_ILOG_DEXT;
> @@ -1853,13 +1841,10 @@ xfs_bmap_add_extent_delay_real(
> startblockval(PREV.br_startblock) -
> (bma->cur ? bma->cur->bc_private.b.allocated : 0));
>
> - trace_xfs_bmap_pre_update(bma->ip, bma->idx + 1, state, _THIS_IP_);
> PREV.br_startoff = new_endoff;
> PREV.br_blockcount = temp;
> PREV.br_startblock = nullstartblock(da_new);
> - xfs_iext_update_extent(ifp, bma->idx + 1, &PREV);
> - trace_xfs_bmap_post_update(bma->ip, bma->idx + 1, state, _THIS_IP_);
> -
> + xfs_iext_update_extent(bma->ip, state, bma->idx + 1, &PREV);
> break;
>
> case BMAP_RIGHT_FILLING | BMAP_RIGHT_CONTIG:
> @@ -1868,12 +1853,10 @@ xfs_bmap_add_extent_delay_real(
> * The right neighbor is contiguous with the new allocation.
> */
> old = RIGHT;
> - trace_xfs_bmap_pre_update(bma->ip, bma->idx + 1, state, _THIS_IP_);
> RIGHT.br_startoff = new->br_startoff;
> RIGHT.br_startblock = new->br_startblock;
> RIGHT.br_blockcount += new->br_blockcount;
> - xfs_iext_update_extent(ifp, bma->idx + 1, &RIGHT);
> - trace_xfs_bmap_post_update(bma->ip, bma->idx + 1, state, _THIS_IP_);
> + xfs_iext_update_extent(bma->ip, state, bma->idx + 1, &RIGHT);
>
> if (bma->cur == NULL)
> rval = XFS_ILOG_DEXT;
> @@ -1892,11 +1875,9 @@ xfs_bmap_add_extent_delay_real(
> da_new = XFS_FILBLKS_MIN(xfs_bmap_worst_indlen(bma->ip, temp),
> startblockval(PREV.br_startblock));
>
> - trace_xfs_bmap_pre_update(bma->ip, bma->idx, state, _THIS_IP_);
> PREV.br_blockcount = temp;
> PREV.br_startblock = nullstartblock(da_new);
> - xfs_iext_update_extent(ifp, bma->idx, &PREV);
> - trace_xfs_bmap_post_update(bma->ip, bma->idx, state, _THIS_IP_);
> + xfs_iext_update_extent(bma->ip, state, bma->idx, &PREV);
>
> bma->idx++;
> break;
> @@ -1936,11 +1917,9 @@ xfs_bmap_add_extent_delay_real(
> startblockval(PREV.br_startblock) -
> (bma->cur ? bma->cur->bc_private.b.allocated : 0));
>
> - trace_xfs_bmap_pre_update(bma->ip, bma->idx, state, _THIS_IP_);
> PREV.br_startblock = nullstartblock(da_new);
> PREV.br_blockcount = temp;
> - xfs_iext_update_extent(ifp, bma->idx, &PREV);
> - trace_xfs_bmap_post_update(bma->ip, bma->idx, state, _THIS_IP_);
> + xfs_iext_update_extent(bma->ip, state, bma->idx, &PREV);
>
> bma->idx++;
> break;
> @@ -1981,13 +1960,11 @@ xfs_bmap_add_extent_delay_real(
> RIGHT.br_blockcount));
>
> /* truncate PREV */
> - trace_xfs_bmap_pre_update(bma->ip, bma->idx, 0, _THIS_IP_);
> PREV.br_blockcount = new->br_startoff - PREV.br_startoff;
> PREV.br_startblock =
> nullstartblock(xfs_bmap_worst_indlen(bma->ip,
> PREV.br_blockcount));
> - xfs_iext_update_extent(ifp, bma->idx, &PREV);
> - trace_xfs_bmap_post_update(bma->ip, bma->idx, state, _THIS_IP_);
> + xfs_iext_update_extent(bma->ip, state, bma->idx, &PREV);
>
> /* insert LEFT (r[0]) and RIGHT (r[1]) at the same time */
> xfs_iext_insert(bma->ip, bma->idx + 1, 2, &LEFT, state);
> @@ -2192,10 +2169,8 @@ xfs_bmap_add_extent_unwritten_real(
> */
> --*idx;
>
> - trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_);
> LEFT.br_blockcount += PREV.br_blockcount + RIGHT.br_blockcount;
> - xfs_iext_update_extent(ifp, *idx, &LEFT);
> - trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_);
> + xfs_iext_update_extent(ip, state, *idx, &LEFT);
>
> xfs_iext_remove(ip, *idx + 1, 2, state);
> XFS_IFORK_NEXT_SET(ip, whichfork,
> @@ -2233,10 +2208,8 @@ xfs_bmap_add_extent_unwritten_real(
> */
> --*idx;
>
> - trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_);
> LEFT.br_blockcount += PREV.br_blockcount;
> - xfs_iext_update_extent(ifp, *idx, &LEFT);
> - trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_);
> + xfs_iext_update_extent(ip, state, *idx, &LEFT);
>
> xfs_iext_remove(ip, *idx + 1, 1, state);
> XFS_IFORK_NEXT_SET(ip, whichfork,
> @@ -2266,11 +2239,9 @@ xfs_bmap_add_extent_unwritten_real(
> * Setting all of a previous oldext extent to newext.
> * The right neighbor is contiguous, the left is not.
> */
> - trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_);
> PREV.br_blockcount += RIGHT.br_blockcount;
> PREV.br_state = new->br_state;
> - xfs_iext_update_extent(ifp, *idx, &PREV);
> - trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_);
> + xfs_iext_update_extent(ip, state, *idx, &PREV);
>
> xfs_iext_remove(ip, *idx + 1, 1, state);
> XFS_IFORK_NEXT_SET(ip, whichfork,
> @@ -2301,10 +2272,8 @@ xfs_bmap_add_extent_unwritten_real(
> * Neither the left nor right neighbors are contiguous with
> * the new one.
> */
> - trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_);
> PREV.br_state = new->br_state;
> - xfs_iext_update_extent(ifp, *idx, &PREV);
> - trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_);
> + xfs_iext_update_extent(ip, state, *idx, &PREV);
>
> if (cur == NULL)
> rval = XFS_ILOG_DEXT;
> @@ -2325,18 +2294,14 @@ xfs_bmap_add_extent_unwritten_real(
> * Setting the first part of a previous oldext extent to newext.
> * The left neighbor is contiguous.
> */
> - trace_xfs_bmap_pre_update(ip, *idx - 1, state, _THIS_IP_);
> LEFT.br_blockcount += new->br_blockcount;
> - xfs_iext_update_extent(ifp, *idx - 1, &LEFT);
> - trace_xfs_bmap_post_update(ip, *idx - 1, state, _THIS_IP_);
> + xfs_iext_update_extent(ip, state, *idx - 1, &LEFT);
>
> old = PREV;
> - trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_);
> PREV.br_startoff += new->br_blockcount;
> PREV.br_startblock += new->br_blockcount;
> PREV.br_blockcount -= new->br_blockcount;
> - xfs_iext_update_extent(ifp, *idx, &PREV);
> - trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_);
> + xfs_iext_update_extent(ip, state, *idx, &PREV);
>
> --*idx;
>
> @@ -2366,12 +2331,10 @@ xfs_bmap_add_extent_unwritten_real(
> * The left neighbor is not contiguous.
> */
> old = PREV;
> - trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_);
> PREV.br_startoff += new->br_blockcount;
> PREV.br_startblock += new->br_blockcount;
> PREV.br_blockcount -= new->br_blockcount;
> - xfs_iext_update_extent(ifp, *idx, &PREV);
> - trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_);
> + xfs_iext_update_extent(ip, state, *idx, &PREV);
>
> xfs_iext_insert(ip, *idx, 1, new, state);
> XFS_IFORK_NEXT_SET(ip, whichfork,
> @@ -2400,19 +2363,15 @@ xfs_bmap_add_extent_unwritten_real(
> * The right neighbor is contiguous with the new allocation.
> */
> old = PREV;
> - trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_);
> PREV.br_blockcount -= new->br_blockcount;
> - xfs_iext_update_extent(ifp, *idx, &PREV);
> - trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_);
> + xfs_iext_update_extent(ip, state, *idx, &PREV);
>
> ++*idx;
>
> - trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_);
> RIGHT.br_startoff = new->br_startoff;
> RIGHT.br_startblock = new->br_startblock;
> RIGHT.br_blockcount += new->br_blockcount;
> - xfs_iext_update_extent(ifp, *idx, &RIGHT);
> - trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_);
> + xfs_iext_update_extent(ip, state, *idx, &RIGHT);
>
> if (cur == NULL)
> rval = XFS_ILOG_DEXT;
> @@ -2440,10 +2399,8 @@ xfs_bmap_add_extent_unwritten_real(
> * The right neighbor is not contiguous.
> */
> old = PREV;
> - trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_);
> PREV.br_blockcount -= new->br_blockcount;
> - xfs_iext_update_extent(ifp, *idx, &PREV);
> - trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_);
> + xfs_iext_update_extent(ip, state, *idx, &PREV);
>
> ++*idx;
> xfs_iext_insert(ip, *idx, 1, new, state);
> @@ -2478,10 +2435,8 @@ xfs_bmap_add_extent_unwritten_real(
> * One extent becomes three extents.
> */
> old = PREV;
> - trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_);
> PREV.br_blockcount = new->br_startoff - PREV.br_startoff;
> - xfs_iext_update_extent(ifp, *idx, &PREV);
> - trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_);
> + xfs_iext_update_extent(ip, state, *idx, &PREV);
>
> r[0] = *new;
> r[1].br_startoff = new_endoff;
> @@ -2648,7 +2603,6 @@ xfs_bmap_add_extent_hole_delay(
> temp = left.br_blockcount + new->br_blockcount +
> right.br_blockcount;
>
> - trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_);
> oldlen = startblockval(left.br_startblock) +
> startblockval(new->br_startblock) +
> startblockval(right.br_startblock);
> @@ -2656,8 +2610,7 @@ xfs_bmap_add_extent_hole_delay(
> oldlen);
> left.br_startblock = nullstartblock(newlen);
> left.br_blockcount = temp;
> - xfs_iext_update_extent(ifp, *idx, &left);
> - trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_);
> + xfs_iext_update_extent(ip, state, *idx, &left);
>
> xfs_iext_remove(ip, *idx + 1, 1, state);
> break;
> @@ -2671,15 +2624,13 @@ xfs_bmap_add_extent_hole_delay(
> --*idx;
> temp = left.br_blockcount + new->br_blockcount;
>
> - trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_);
> oldlen = startblockval(left.br_startblock) +
> startblockval(new->br_startblock);
> newlen = XFS_FILBLKS_MIN(xfs_bmap_worst_indlen(ip, temp),
> oldlen);
> left.br_blockcount = temp;
> left.br_startblock = nullstartblock(newlen);
> - xfs_iext_update_extent(ifp, *idx, &left);
> - trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_);
> + xfs_iext_update_extent(ip, state, *idx, &left);
> break;
>
> case BMAP_RIGHT_CONTIG:
> @@ -2688,7 +2639,6 @@ xfs_bmap_add_extent_hole_delay(
> * on the right.
> * Merge the new allocation with the right neighbor.
> */
> - trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_);
> temp = new->br_blockcount + right.br_blockcount;
> oldlen = startblockval(new->br_startblock) +
> startblockval(right.br_startblock);
> @@ -2697,8 +2647,7 @@ xfs_bmap_add_extent_hole_delay(
> right.br_startoff = new->br_startoff;
> right.br_startblock = nullstartblock(newlen);
> right.br_blockcount = temp;
> - xfs_iext_update_extent(ifp, *idx, &right);
> - trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_);
> + xfs_iext_update_extent(ip, state, *idx, &right);
> break;
>
> case 0:
> @@ -2808,10 +2757,8 @@ xfs_bmap_add_extent_hole_real(
> * Merge all three into a single extent record.
> */
> --*idx;
> - trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_);
> left.br_blockcount += new->br_blockcount + right.br_blockcount;
> - xfs_iext_update_extent(ifp, *idx, &left);
> - trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_);
> + xfs_iext_update_extent(ip, state, *idx, &left);
>
> xfs_iext_remove(ip, *idx + 1, 1, state);
>
> @@ -2847,10 +2794,9 @@ xfs_bmap_add_extent_hole_real(
> */
> --*idx;
> old = left;
> - trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_);
> +
> left.br_blockcount += new->br_blockcount;
> - xfs_iext_update_extent(ifp, *idx, &left);
> - trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_);
> + xfs_iext_update_extent(ip, state, *idx, &left);
>
> if (cur == NULL) {
> rval = xfs_ilog_fext(whichfork);
> @@ -2873,12 +2819,11 @@ xfs_bmap_add_extent_hole_real(
> * Merge the new allocation with the right neighbor.
> */
> old = right;
> - trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_);
> +
> right.br_startoff = new->br_startoff;
> right.br_startblock = new->br_startblock;
> right.br_blockcount += new->br_blockcount;
> - xfs_iext_update_extent(ifp, *idx, &right);
> - trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_);
> + xfs_iext_update_extent(ip, state, *idx, &right);
>
> if (cur == NULL) {
> rval = xfs_ilog_fext(whichfork);
> @@ -4769,26 +4714,22 @@ xfs_bmap_del_extent_delay(
> /*
> * Deleting the first part of the extent.
> */
> - trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_);
> got->br_startoff = del_endoff;
> got->br_blockcount -= del->br_blockcount;
> da_new = XFS_FILBLKS_MIN(xfs_bmap_worst_indlen(ip,
> got->br_blockcount), da_old);
> got->br_startblock = nullstartblock((int)da_new);
> - xfs_iext_update_extent(ifp, *idx, got);
> - trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_);
> + xfs_iext_update_extent(ip, state, *idx, got);
> break;
> case BMAP_RIGHT_FILLING:
> /*
> * Deleting the last part of the extent.
> */
> - trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_);
> got->br_blockcount = got->br_blockcount - del->br_blockcount;
> da_new = XFS_FILBLKS_MIN(xfs_bmap_worst_indlen(ip,
> got->br_blockcount), da_old);
> got->br_startblock = nullstartblock((int)da_new);
> - xfs_iext_update_extent(ifp, *idx, got);
> - trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_);
> + xfs_iext_update_extent(ip, state, *idx, got);
> break;
> case 0:
> /*
> @@ -4800,8 +4741,6 @@ xfs_bmap_del_extent_delay(
> * Warn if either of the new indlen reservations is zero as this
> * can lead to delalloc problems.
> */
> - trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_);
> -
> got->br_blockcount = del->br_startoff - got->br_startoff;
> got_indlen = xfs_bmap_worst_indlen(ip, got->br_blockcount);
>
> @@ -4813,8 +4752,7 @@ xfs_bmap_del_extent_delay(
> del->br_blockcount);
>
> got->br_startblock = nullstartblock((int)got_indlen);
> - xfs_iext_update_extent(ifp, *idx, got);
> - trace_xfs_bmap_post_update(ip, *idx, 0, _THIS_IP_);
> + xfs_iext_update_extent(ip, state, *idx, got);
>
> new.br_startoff = del_endoff;
> new.br_state = got->br_state;
> @@ -4879,30 +4817,24 @@ xfs_bmap_del_extent_cow(
> /*
> * Deleting the first part of the extent.
> */
> - trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_);
> got->br_startoff = del_endoff;
> got->br_blockcount -= del->br_blockcount;
> got->br_startblock = del->br_startblock + del->br_blockcount;
> - xfs_iext_update_extent(ifp, *idx, got);
> - trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_);
> + xfs_iext_update_extent(ip, state, *idx, got);
> break;
> case BMAP_RIGHT_FILLING:
> /*
> * Deleting the last part of the extent.
> */
> - trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_);
> got->br_blockcount -= del->br_blockcount;
> - xfs_iext_update_extent(ifp, *idx, got);
> - trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_);
> + xfs_iext_update_extent(ip, state, *idx, got);
> break;
> case 0:
> /*
> * Deleting the middle of the extent.
> */
> - trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_);
> got->br_blockcount = del->br_startoff - got->br_startoff;
> - xfs_iext_update_extent(ifp, *idx, got);
> - trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_);
> + xfs_iext_update_extent(ip, state, *idx, got);
>
> new.br_startoff = del_endoff;
> new.br_blockcount = got_endoff - del_endoff;
> @@ -5019,7 +4951,6 @@ xfs_bmap_del_extent_real(
> /*
> * Matches the whole extent. Delete the entry.
> */
> - trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_);
> xfs_iext_remove(ip, *idx, 1, state);
> --*idx;
>
> @@ -5038,12 +4969,10 @@ xfs_bmap_del_extent_real(
> /*
> * Deleting the first part of the extent.
> */
> - trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_);
> got.br_startoff = del_endoff;
> got.br_startblock = del_endblock;
> got.br_blockcount -= del->br_blockcount;
> - xfs_iext_update_extent(ifp, *idx, &got);
> - trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_);
> + xfs_iext_update_extent(ip, state, *idx, &got);
> if (!cur) {
> flags |= xfs_ilog_fext(whichfork);
> break;
> @@ -5056,10 +4985,8 @@ xfs_bmap_del_extent_real(
> /*
> * Deleting the last part of the extent.
> */
> - trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_);
> got.br_blockcount -= del->br_blockcount;
> - xfs_iext_update_extent(ifp, *idx, &got);
> - trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_);
> + xfs_iext_update_extent(ip, state, *idx, &got);
> if (!cur) {
> flags |= xfs_ilog_fext(whichfork);
> break;
> @@ -5072,11 +4999,10 @@ xfs_bmap_del_extent_real(
> /*
> * Deleting the middle of the extent.
> */
> - trace_xfs_bmap_pre_update(ip, *idx, state, _THIS_IP_);
> -
> old = got;
> +
> got.br_blockcount = del->br_startoff - got.br_startoff;
> - xfs_iext_update_extent(ifp, *idx, &got);
> + xfs_iext_update_extent(ip, state, *idx, &got);
>
> new.br_startoff = del_endoff;
> new.br_blockcount = got_endoff - del_endoff;
> @@ -5120,7 +5046,7 @@ xfs_bmap_del_extent_real(
> * Reset the extent record back
> * to the original value.
> */
> - xfs_iext_update_extent(ifp, *idx, &old);
> + xfs_iext_update_extent(ip, state, *idx, &old);
> flags = 0;
> error = -ENOSPC;
> goto done;
> @@ -5130,7 +5056,6 @@ xfs_bmap_del_extent_real(
> flags |= xfs_ilog_fext(whichfork);
> XFS_IFORK_NEXT_SET(ip, whichfork,
> XFS_IFORK_NEXTENTS(ip, whichfork) + 1);
> - trace_xfs_bmap_post_update(ip, *idx, state, _THIS_IP_);
> xfs_iext_insert(ip, *idx + 1, 1, &new, state);
> ++*idx;
> break;
> @@ -5605,7 +5530,6 @@ xfs_bmse_merge(
> int *logflags, /* output */
> struct xfs_defer_ops *dfops)
> {
> - struct xfs_ifork *ifp = XFS_IFORK_PTR(ip, whichfork);
> struct xfs_bmbt_irec new;
> xfs_filblks_t blockcount;
> int error, i;
> @@ -5654,7 +5578,8 @@ xfs_bmse_merge(
> return error;
>
> done:
> - xfs_iext_update_extent(ifp, current_ext - 1, &new);
> + xfs_iext_update_extent(ip, xfs_bmap_fork_to_state(whichfork),
> + current_ext - 1, &new);
> xfs_iext_remove(ip, current_ext, 1, 0);
>
> /* update reverse mapping. rmap functions merge the rmaps for us */
> @@ -5772,7 +5697,8 @@ xfs_bmse_shift_one(
> *logflags |= XFS_ILOG_DEXT;
> }
>
> - xfs_iext_update_extent(ifp, *current_ext, &new);
> + xfs_iext_update_extent(ip, xfs_bmap_fork_to_state(whichfork),
> + *current_ext, &new);
>
> if (direction == SHIFT_LEFT)
> (*current_ext)++;
> @@ -6019,7 +5945,8 @@ xfs_bmap_split_extent_at(
> }
>
> got.br_blockcount = gotblkcnt;
> - xfs_iext_update_extent(ifp, current_ext, &got);
> + xfs_iext_update_extent(ip, xfs_bmap_fork_to_state(whichfork),
> + current_ext, &got);
>
> logflags = XFS_ILOG_CORE;
> if (cur) {
> diff --git a/fs/xfs/libxfs/xfs_inode_fork.c b/fs/xfs/libxfs/xfs_inode_fork.c
> index 31840ca24018..7f40f53e6c43 100644
> --- a/fs/xfs/libxfs/xfs_inode_fork.c
> +++ b/fs/xfs/libxfs/xfs_inode_fork.c
> @@ -2023,12 +2023,17 @@ xfs_iext_get_extent(
>
> void
> xfs_iext_update_extent(
> - struct xfs_ifork *ifp,
> + struct xfs_inode *ip,
> + int state,
> xfs_extnum_t idx,
> struct xfs_bmbt_irec *gotp)
> {
> + struct xfs_ifork *ifp = xfs_iext_state_to_fork(ip, state);
> +
> ASSERT(idx >= 0);
> ASSERT(idx < xfs_iext_count(ifp));
>
> + trace_xfs_bmap_pre_update(ip, idx, state, _RET_IP_);
> xfs_bmbt_set_all(xfs_iext_get_ext(ifp, idx), gotp);
> + trace_xfs_bmap_post_update(ip, idx, state, _RET_IP_);
> }
> diff --git a/fs/xfs/libxfs/xfs_inode_fork.h b/fs/xfs/libxfs/xfs_inode_fork.h
> index 11af705219f6..6750f0462d21 100644
> --- a/fs/xfs/libxfs/xfs_inode_fork.h
> +++ b/fs/xfs/libxfs/xfs_inode_fork.h
> @@ -187,8 +187,8 @@ bool xfs_iext_lookup_extent(struct xfs_inode *ip,
> xfs_extnum_t *idxp, struct xfs_bmbt_irec *gotp);
> bool xfs_iext_get_extent(struct xfs_ifork *ifp, xfs_extnum_t idx,
> struct xfs_bmbt_irec *gotp);
> -void xfs_iext_update_extent(struct xfs_ifork *ifp, xfs_extnum_t idx,
> - struct xfs_bmbt_irec *gotp);
> +void xfs_iext_update_extent(struct xfs_inode *ip, int state,
> + xfs_extnum_t idx, struct xfs_bmbt_irec *gotp);
>
> extern struct kmem_zone *xfs_ifork_zone;
>
> --
> 2.14.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH 05/15] xfs: remove XFS_BMAP_TRACE_EXLIST
2017-10-19 6:59 more extent mapping cleanups Christoph Hellwig
` (3 preceding siblings ...)
2017-10-19 6:59 ` [PATCH 04/15] xfs: move pre/post-bmap tracing into xfs_iext_update_extent Christoph Hellwig
@ 2017-10-19 6:59 ` Christoph Hellwig
2017-10-19 22:50 ` Darrick J. Wong
2017-10-19 6:59 ` [PATCH 06/15] xfs: remove the never fully implemented UUID fork format Christoph Hellwig
` (10 subsequent siblings)
15 siblings, 1 reply; 36+ messages in thread
From: Christoph Hellwig @ 2017-10-19 6:59 UTC (permalink / raw)
To: linux-xfs
Instead of looping over all extents in some debug-only helper just
insert trace points into the loops that already exist in the calling
functions.
Also split the xfs_extlist trace point into one each for reading and
writing extents from disk.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/xfs/libxfs/xfs_bmap.c | 23 ++---------------------
fs/xfs/libxfs/xfs_bmap.h | 9 ---------
fs/xfs/libxfs/xfs_inode_fork.c | 8 ++++++--
fs/xfs/xfs_trace.h | 3 ++-
4 files changed, 10 insertions(+), 33 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index ff9a8e1d4a96..6431386f4676 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -487,26 +487,6 @@ xfs_bmap_check_leaf_extents(
return;
}
-/*
- * Add bmap trace insert entries for all the contents of the extent records.
- */
-void
-xfs_bmap_trace_exlist(
- xfs_inode_t *ip, /* incore inode pointer */
- xfs_extnum_t cnt, /* count of entries in the list */
- int whichfork, /* data or attr or cow fork */
- unsigned long caller_ip)
-{
- xfs_extnum_t idx; /* extent record index */
- xfs_ifork_t *ifp; /* inode fork pointer */
- int state = xfs_bmap_fork_to_state(whichfork);
-
- ifp = XFS_IFORK_PTR(ip, whichfork);
- ASSERT(cnt == xfs_iext_count(ifp));
- for (idx = 0; idx < cnt; idx++)
- trace_xfs_extlist(ip, idx, state, caller_ip);
-}
-
/*
* Validate that the bmbt_irecs being returned from bmapi are valid
* given the caller's original parameters. Specifically check the
@@ -1210,6 +1190,7 @@ xfs_bmap_read_extents(
__be64 *pp; /* pointer to block address */
/* REFERENCED */
xfs_extnum_t room; /* number of entries there's room for */
+ int state = xfs_bmap_fork_to_state(whichfork);
mp = ip->i_mount;
ifp = XFS_IFORK_PTR(ip, whichfork);
@@ -1283,6 +1264,7 @@ xfs_bmap_read_extents(
XFS_ERRLEVEL_LOW, mp);
goto error0;
}
+ trace_xfs_read_extent(ip, i, state, _THIS_IP_);
}
xfs_trans_brelse(tp, bp);
bno = nextbno;
@@ -1300,7 +1282,6 @@ xfs_bmap_read_extents(
if (i != XFS_IFORK_NEXTENTS(ip, whichfork))
return -EFSCORRUPTED;
ASSERT(i == xfs_iext_count(ifp));
- XFS_BMAP_TRACE_EXLIST(ip, i, whichfork);
return 0;
error0:
xfs_trans_brelse(tp, bp);
diff --git a/fs/xfs/libxfs/xfs_bmap.h b/fs/xfs/libxfs/xfs_bmap.h
index a61c5480b6ad..c837e88ba19a 100644
--- a/fs/xfs/libxfs/xfs_bmap.h
+++ b/fs/xfs/libxfs/xfs_bmap.h
@@ -197,15 +197,6 @@ enum shift_direction {
SHIFT_RIGHT,
};
-#ifdef DEBUG
-void xfs_bmap_trace_exlist(struct xfs_inode *ip, xfs_extnum_t cnt,
- int whichfork, unsigned long caller_ip);
-#define XFS_BMAP_TRACE_EXLIST(ip,c,w) \
- xfs_bmap_trace_exlist(ip,c,w, _THIS_IP_)
-#else
-#define XFS_BMAP_TRACE_EXLIST(ip,c,w)
-#endif
-
void xfs_trim_extent(struct xfs_bmbt_irec *irec, xfs_fileoff_t bno,
xfs_filblks_t len);
int xfs_bmap_add_attrfork(struct xfs_inode *ip, int size, int rsvd);
diff --git a/fs/xfs/libxfs/xfs_inode_fork.c b/fs/xfs/libxfs/xfs_inode_fork.c
index 7f40f53e6c43..31786bad9738 100644
--- a/fs/xfs/libxfs/xfs_inode_fork.c
+++ b/fs/xfs/libxfs/xfs_inode_fork.c
@@ -336,6 +336,7 @@ xfs_iformat_extents(
{
struct xfs_mount *mp = ip->i_mount;
struct xfs_ifork *ifp = XFS_IFORK_PTR(ip, whichfork);
+ int state = xfs_bmap_fork_to_state(whichfork);
int nex = XFS_DFORK_NEXTENTS(dip, whichfork);
int size = nex * sizeof(xfs_bmbt_rec_t);
struct xfs_bmbt_rec *dp;
@@ -373,8 +374,8 @@ xfs_iformat_extents(
XFS_ERRLEVEL_LOW, mp);
return -EFSCORRUPTED;
}
+ trace_xfs_read_extent(ip, i, state, _THIS_IP_);
}
- XFS_BMAP_TRACE_EXLIST(ip, nex, whichfork);
}
ifp->if_flags |= XFS_IFEXTENTS;
return 0;
@@ -772,6 +773,7 @@ xfs_iextents_copy(
xfs_bmbt_rec_t *dp,
int whichfork)
{
+ int state = xfs_bmap_fork_to_state(whichfork);
int copied;
int i;
xfs_ifork_t *ifp;
@@ -783,7 +785,6 @@ xfs_iextents_copy(
ASSERT(ifp->if_bytes > 0);
nrecs = xfs_iext_count(ifp);
- XFS_BMAP_TRACE_EXLIST(ip, nrecs, whichfork);
ASSERT(nrecs > 0);
/*
@@ -806,9 +807,12 @@ xfs_iextents_copy(
continue;
}
+ trace_xfs_write_extent(ip, i, state, _RET_IP_);
+
/* Translate to on disk format */
put_unaligned_be64(ep->l0, &dp->l0);
put_unaligned_be64(ep->l1, &dp->l1);
+
dp++;
copied++;
}
diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
index 0a8999a310b9..665ef6cca90c 100644
--- a/fs/xfs/xfs_trace.h
+++ b/fs/xfs/xfs_trace.h
@@ -309,7 +309,8 @@ DEFINE_EVENT(xfs_bmap_class, name, \
DEFINE_BMAP_EVENT(xfs_iext_remove);
DEFINE_BMAP_EVENT(xfs_bmap_pre_update);
DEFINE_BMAP_EVENT(xfs_bmap_post_update);
-DEFINE_BMAP_EVENT(xfs_extlist);
+DEFINE_BMAP_EVENT(xfs_read_extent);
+DEFINE_BMAP_EVENT(xfs_write_extent);
DECLARE_EVENT_CLASS(xfs_buf_class,
TP_PROTO(struct xfs_buf *bp, unsigned long caller_ip),
--
2.14.1
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [PATCH 05/15] xfs: remove XFS_BMAP_TRACE_EXLIST
2017-10-19 6:59 ` [PATCH 05/15] xfs: remove XFS_BMAP_TRACE_EXLIST Christoph Hellwig
@ 2017-10-19 22:50 ` Darrick J. Wong
0 siblings, 0 replies; 36+ messages in thread
From: Darrick J. Wong @ 2017-10-19 22:50 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs
On Thu, Oct 19, 2017 at 08:59:32AM +0200, Christoph Hellwig wrote:
> Instead of looping over all extents in some debug-only helper just
> insert trace points into the loops that already exist in the calling
> functions.
>
> Also split the xfs_extlist trace point into one each for reading and
> writing extents from disk.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
Looks ok,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
> fs/xfs/libxfs/xfs_bmap.c | 23 ++---------------------
> fs/xfs/libxfs/xfs_bmap.h | 9 ---------
> fs/xfs/libxfs/xfs_inode_fork.c | 8 ++++++--
> fs/xfs/xfs_trace.h | 3 ++-
> 4 files changed, 10 insertions(+), 33 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index ff9a8e1d4a96..6431386f4676 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -487,26 +487,6 @@ xfs_bmap_check_leaf_extents(
> return;
> }
>
> -/*
> - * Add bmap trace insert entries for all the contents of the extent records.
> - */
> -void
> -xfs_bmap_trace_exlist(
> - xfs_inode_t *ip, /* incore inode pointer */
> - xfs_extnum_t cnt, /* count of entries in the list */
> - int whichfork, /* data or attr or cow fork */
> - unsigned long caller_ip)
> -{
> - xfs_extnum_t idx; /* extent record index */
> - xfs_ifork_t *ifp; /* inode fork pointer */
> - int state = xfs_bmap_fork_to_state(whichfork);
> -
> - ifp = XFS_IFORK_PTR(ip, whichfork);
> - ASSERT(cnt == xfs_iext_count(ifp));
> - for (idx = 0; idx < cnt; idx++)
> - trace_xfs_extlist(ip, idx, state, caller_ip);
> -}
> -
> /*
> * Validate that the bmbt_irecs being returned from bmapi are valid
> * given the caller's original parameters. Specifically check the
> @@ -1210,6 +1190,7 @@ xfs_bmap_read_extents(
> __be64 *pp; /* pointer to block address */
> /* REFERENCED */
> xfs_extnum_t room; /* number of entries there's room for */
> + int state = xfs_bmap_fork_to_state(whichfork);
>
> mp = ip->i_mount;
> ifp = XFS_IFORK_PTR(ip, whichfork);
> @@ -1283,6 +1264,7 @@ xfs_bmap_read_extents(
> XFS_ERRLEVEL_LOW, mp);
> goto error0;
> }
> + trace_xfs_read_extent(ip, i, state, _THIS_IP_);
> }
> xfs_trans_brelse(tp, bp);
> bno = nextbno;
> @@ -1300,7 +1282,6 @@ xfs_bmap_read_extents(
> if (i != XFS_IFORK_NEXTENTS(ip, whichfork))
> return -EFSCORRUPTED;
> ASSERT(i == xfs_iext_count(ifp));
> - XFS_BMAP_TRACE_EXLIST(ip, i, whichfork);
> return 0;
> error0:
> xfs_trans_brelse(tp, bp);
> diff --git a/fs/xfs/libxfs/xfs_bmap.h b/fs/xfs/libxfs/xfs_bmap.h
> index a61c5480b6ad..c837e88ba19a 100644
> --- a/fs/xfs/libxfs/xfs_bmap.h
> +++ b/fs/xfs/libxfs/xfs_bmap.h
> @@ -197,15 +197,6 @@ enum shift_direction {
> SHIFT_RIGHT,
> };
>
> -#ifdef DEBUG
> -void xfs_bmap_trace_exlist(struct xfs_inode *ip, xfs_extnum_t cnt,
> - int whichfork, unsigned long caller_ip);
> -#define XFS_BMAP_TRACE_EXLIST(ip,c,w) \
> - xfs_bmap_trace_exlist(ip,c,w, _THIS_IP_)
> -#else
> -#define XFS_BMAP_TRACE_EXLIST(ip,c,w)
> -#endif
> -
> void xfs_trim_extent(struct xfs_bmbt_irec *irec, xfs_fileoff_t bno,
> xfs_filblks_t len);
> int xfs_bmap_add_attrfork(struct xfs_inode *ip, int size, int rsvd);
> diff --git a/fs/xfs/libxfs/xfs_inode_fork.c b/fs/xfs/libxfs/xfs_inode_fork.c
> index 7f40f53e6c43..31786bad9738 100644
> --- a/fs/xfs/libxfs/xfs_inode_fork.c
> +++ b/fs/xfs/libxfs/xfs_inode_fork.c
> @@ -336,6 +336,7 @@ xfs_iformat_extents(
> {
> struct xfs_mount *mp = ip->i_mount;
> struct xfs_ifork *ifp = XFS_IFORK_PTR(ip, whichfork);
> + int state = xfs_bmap_fork_to_state(whichfork);
> int nex = XFS_DFORK_NEXTENTS(dip, whichfork);
> int size = nex * sizeof(xfs_bmbt_rec_t);
> struct xfs_bmbt_rec *dp;
> @@ -373,8 +374,8 @@ xfs_iformat_extents(
> XFS_ERRLEVEL_LOW, mp);
> return -EFSCORRUPTED;
> }
> + trace_xfs_read_extent(ip, i, state, _THIS_IP_);
> }
> - XFS_BMAP_TRACE_EXLIST(ip, nex, whichfork);
> }
> ifp->if_flags |= XFS_IFEXTENTS;
> return 0;
> @@ -772,6 +773,7 @@ xfs_iextents_copy(
> xfs_bmbt_rec_t *dp,
> int whichfork)
> {
> + int state = xfs_bmap_fork_to_state(whichfork);
> int copied;
> int i;
> xfs_ifork_t *ifp;
> @@ -783,7 +785,6 @@ xfs_iextents_copy(
> ASSERT(ifp->if_bytes > 0);
>
> nrecs = xfs_iext_count(ifp);
> - XFS_BMAP_TRACE_EXLIST(ip, nrecs, whichfork);
> ASSERT(nrecs > 0);
>
> /*
> @@ -806,9 +807,12 @@ xfs_iextents_copy(
> continue;
> }
>
> + trace_xfs_write_extent(ip, i, state, _RET_IP_);
> +
> /* Translate to on disk format */
> put_unaligned_be64(ep->l0, &dp->l0);
> put_unaligned_be64(ep->l1, &dp->l1);
> +
> dp++;
> copied++;
> }
> diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
> index 0a8999a310b9..665ef6cca90c 100644
> --- a/fs/xfs/xfs_trace.h
> +++ b/fs/xfs/xfs_trace.h
> @@ -309,7 +309,8 @@ DEFINE_EVENT(xfs_bmap_class, name, \
> DEFINE_BMAP_EVENT(xfs_iext_remove);
> DEFINE_BMAP_EVENT(xfs_bmap_pre_update);
> DEFINE_BMAP_EVENT(xfs_bmap_post_update);
> -DEFINE_BMAP_EVENT(xfs_extlist);
> +DEFINE_BMAP_EVENT(xfs_read_extent);
> +DEFINE_BMAP_EVENT(xfs_write_extent);
>
> DECLARE_EVENT_CLASS(xfs_buf_class,
> TP_PROTO(struct xfs_buf *bp, unsigned long caller_ip),
> --
> 2.14.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH 06/15] xfs: remove the never fully implemented UUID fork format
2017-10-19 6:59 more extent mapping cleanups Christoph Hellwig
` (4 preceding siblings ...)
2017-10-19 6:59 ` [PATCH 05/15] xfs: remove XFS_BMAP_TRACE_EXLIST Christoph Hellwig
@ 2017-10-19 6:59 ` Christoph Hellwig
2017-10-19 22:48 ` Darrick J. Wong
2017-10-19 6:59 ` [PATCH 07/15] xfs: remove if_rdev Christoph Hellwig
` (9 subsequent siblings)
15 siblings, 1 reply; 36+ messages in thread
From: Christoph Hellwig @ 2017-10-19 6:59 UTC (permalink / raw)
To: linux-xfs
Remove the dead code dealing with the UUID fork format that was never
implemented in Linux (and neither in IRIX as far as I know).
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/xfs/libxfs/xfs_attr_leaf.c | 6 +-----
fs/xfs/libxfs/xfs_bmap.c | 4 ----
fs/xfs/libxfs/xfs_format.h | 2 +-
fs/xfs/libxfs/xfs_inode_fork.c | 9 ---------
fs/xfs/libxfs/xfs_inode_fork.h | 1 -
fs/xfs/libxfs/xfs_log_format.h | 20 ++++++++++----------
fs/xfs/xfs_inode_item.c | 25 ++++++-------------------
fs/xfs/xfs_itable.c | 1 -
fs/xfs/xfs_log_recover.c | 10 +---------
9 files changed, 19 insertions(+), 59 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
index 5c16db86b38f..53cc8b986eac 100644
--- a/fs/xfs/libxfs/xfs_attr_leaf.c
+++ b/fs/xfs/libxfs/xfs_attr_leaf.c
@@ -397,13 +397,9 @@ xfs_attr_shortform_bytesfit(xfs_inode_t *dp, int bytes)
/* rounded down */
offset = (XFS_LITINO(mp, dp->i_d.di_version) - bytes) >> 3;
- switch (dp->i_d.di_format) {
- case XFS_DINODE_FMT_DEV:
+ if (dp->i_d.di_format == XFS_DINODE_FMT_DEV) {
minforkoff = roundup(sizeof(xfs_dev_t), 8) >> 3;
return (offset >= minforkoff) ? minforkoff : 0;
- case XFS_DINODE_FMT_UUID:
- minforkoff = roundup(sizeof(uuid_t), 8) >> 3;
- return (offset >= minforkoff) ? minforkoff : 0;
}
/*
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 6431386f4676..22e7578e5696 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -231,7 +231,6 @@ xfs_bmap_forkoff_reset(
{
if (whichfork == XFS_ATTR_FORK &&
ip->i_d.di_format != XFS_DINODE_FMT_DEV &&
- ip->i_d.di_format != XFS_DINODE_FMT_UUID &&
ip->i_d.di_format != XFS_DINODE_FMT_BTREE) {
uint dfl_forkoff = xfs_default_attroffset(ip) >> 3;
@@ -1086,9 +1085,6 @@ xfs_bmap_add_attrfork(
case XFS_DINODE_FMT_DEV:
ip->i_d.di_forkoff = roundup(sizeof(xfs_dev_t), 8) >> 3;
break;
- case XFS_DINODE_FMT_UUID:
- ip->i_d.di_forkoff = roundup(sizeof(uuid_t), 8) >> 3;
- break;
case XFS_DINODE_FMT_LOCAL:
case XFS_DINODE_FMT_EXTENTS:
case XFS_DINODE_FMT_BTREE:
diff --git a/fs/xfs/libxfs/xfs_format.h b/fs/xfs/libxfs/xfs_format.h
index 23229f0c5b15..66f9b9f61d92 100644
--- a/fs/xfs/libxfs/xfs_format.h
+++ b/fs/xfs/libxfs/xfs_format.h
@@ -941,7 +941,7 @@ typedef enum xfs_dinode_fmt {
XFS_DINODE_FMT_LOCAL, /* bulk data */
XFS_DINODE_FMT_EXTENTS, /* struct xfs_bmbt_rec */
XFS_DINODE_FMT_BTREE, /* struct xfs_bmdr_block */
- XFS_DINODE_FMT_UUID /* uuid_t */
+ XFS_DINODE_FMT_UUID /* added long ago, but never used */
} xfs_dinode_fmt_t;
/*
diff --git a/fs/xfs/libxfs/xfs_inode_fork.c b/fs/xfs/libxfs/xfs_inode_fork.c
index 31786bad9738..1d003ca21562 100644
--- a/fs/xfs/libxfs/xfs_inode_fork.c
+++ b/fs/xfs/libxfs/xfs_inode_fork.c
@@ -902,15 +902,6 @@ xfs_iflush_fork(
}
break;
- case XFS_DINODE_FMT_UUID:
- if (iip->ili_fields & XFS_ILOG_UUID) {
- ASSERT(whichfork == XFS_DATA_FORK);
- memcpy(XFS_DFORK_DPTR(dip),
- &ip->i_df.if_u2.if_uuid,
- sizeof(uuid_t));
- }
- break;
-
default:
ASSERT(0);
break;
diff --git a/fs/xfs/libxfs/xfs_inode_fork.h b/fs/xfs/libxfs/xfs_inode_fork.h
index 6750f0462d21..064babdc373c 100644
--- a/fs/xfs/libxfs/xfs_inode_fork.h
+++ b/fs/xfs/libxfs/xfs_inode_fork.h
@@ -70,7 +70,6 @@ typedef struct xfs_ifork {
char if_inline_data[XFS_INLINE_DATA];
/* very small file data */
xfs_dev_t if_rdev; /* dev number if special */
- uuid_t if_uuid; /* mount point value */
} if_u2;
} xfs_ifork_t;
diff --git a/fs/xfs/libxfs/xfs_log_format.h b/fs/xfs/libxfs/xfs_log_format.h
index 71de185735e0..a7ab6adae7f6 100644
--- a/fs/xfs/libxfs/xfs_log_format.h
+++ b/fs/xfs/libxfs/xfs_log_format.h
@@ -274,7 +274,7 @@ typedef struct xfs_inode_log_format {
uint64_t ilf_ino; /* inode number */
union {
uint32_t ilfu_rdev; /* rdev value for dev inode*/
- uuid_t ilfu_uuid; /* mount point value */
+ u8 __pad[16]; /* unused */
} ilf_u;
int64_t ilf_blkno; /* blkno of inode buffer */
int32_t ilf_len; /* len of inode buffer */
@@ -295,7 +295,7 @@ struct xfs_inode_log_format_32 {
uint64_t ilf_ino; /* inode number */
union {
uint32_t ilfu_rdev; /* rdev value for dev inode*/
- uuid_t ilfu_uuid; /* mount point value */
+ u8 __pad[16]; /* unused */
} ilf_u;
int64_t ilf_blkno; /* blkno of inode buffer */
int32_t ilf_len; /* len of inode buffer */
@@ -311,7 +311,7 @@ struct xfs_inode_log_format_32 {
#define XFS_ILOG_DEXT 0x004 /* log i_df.if_extents */
#define XFS_ILOG_DBROOT 0x008 /* log i_df.i_broot */
#define XFS_ILOG_DEV 0x010 /* log the dev field */
-#define XFS_ILOG_UUID 0x020 /* log the uuid field */
+#define XFS_ILOG_UUID 0x020 /* added long ago, but never used */
#define XFS_ILOG_ADATA 0x040 /* log i_af.if_data */
#define XFS_ILOG_AEXT 0x080 /* log i_af.if_extents */
#define XFS_ILOG_ABROOT 0x100 /* log i_af.i_broot */
@@ -329,9 +329,9 @@ struct xfs_inode_log_format_32 {
#define XFS_ILOG_NONCORE (XFS_ILOG_DDATA | XFS_ILOG_DEXT | \
XFS_ILOG_DBROOT | XFS_ILOG_DEV | \
- XFS_ILOG_UUID | XFS_ILOG_ADATA | \
- XFS_ILOG_AEXT | XFS_ILOG_ABROOT | \
- XFS_ILOG_DOWNER | XFS_ILOG_AOWNER)
+ XFS_ILOG_ADATA | XFS_ILOG_AEXT | \
+ XFS_ILOG_ABROOT | XFS_ILOG_DOWNER | \
+ XFS_ILOG_AOWNER)
#define XFS_ILOG_DFORK (XFS_ILOG_DDATA | XFS_ILOG_DEXT | \
XFS_ILOG_DBROOT)
@@ -341,10 +341,10 @@ struct xfs_inode_log_format_32 {
#define XFS_ILOG_ALL (XFS_ILOG_CORE | XFS_ILOG_DDATA | \
XFS_ILOG_DEXT | XFS_ILOG_DBROOT | \
- XFS_ILOG_DEV | XFS_ILOG_UUID | \
- XFS_ILOG_ADATA | XFS_ILOG_AEXT | \
- XFS_ILOG_ABROOT | XFS_ILOG_TIMESTAMP | \
- XFS_ILOG_DOWNER | XFS_ILOG_AOWNER)
+ XFS_ILOG_DEV | XFS_ILOG_ADATA | \
+ XFS_ILOG_AEXT | XFS_ILOG_ABROOT | \
+ XFS_ILOG_TIMESTAMP | XFS_ILOG_DOWNER | \
+ XFS_ILOG_AOWNER)
static inline int xfs_ilog_fbroot(int w)
{
diff --git a/fs/xfs/xfs_inode_item.c b/fs/xfs/xfs_inode_item.c
index 9bbc2d7cc8cb..bd60ad313173 100644
--- a/fs/xfs/xfs_inode_item.c
+++ b/fs/xfs/xfs_inode_item.c
@@ -72,7 +72,6 @@ xfs_inode_item_data_fork_size(
break;
case XFS_DINODE_FMT_DEV:
- case XFS_DINODE_FMT_UUID:
break;
default:
ASSERT(0);
@@ -156,8 +155,7 @@ xfs_inode_item_format_data_fork(
switch (ip->i_d.di_format) {
case XFS_DINODE_FMT_EXTENTS:
iip->ili_fields &=
- ~(XFS_ILOG_DDATA | XFS_ILOG_DBROOT |
- XFS_ILOG_DEV | XFS_ILOG_UUID);
+ ~(XFS_ILOG_DDATA | XFS_ILOG_DBROOT | XFS_ILOG_DEV);
if ((iip->ili_fields & XFS_ILOG_DEXT) &&
ip->i_d.di_nextents > 0 &&
@@ -181,8 +179,7 @@ xfs_inode_item_format_data_fork(
break;
case XFS_DINODE_FMT_BTREE:
iip->ili_fields &=
- ~(XFS_ILOG_DDATA | XFS_ILOG_DEXT |
- XFS_ILOG_DEV | XFS_ILOG_UUID);
+ ~(XFS_ILOG_DDATA | XFS_ILOG_DEXT | XFS_ILOG_DEV);
if ((iip->ili_fields & XFS_ILOG_DBROOT) &&
ip->i_df.if_broot_bytes > 0) {
@@ -200,8 +197,7 @@ xfs_inode_item_format_data_fork(
break;
case XFS_DINODE_FMT_LOCAL:
iip->ili_fields &=
- ~(XFS_ILOG_DEXT | XFS_ILOG_DBROOT |
- XFS_ILOG_DEV | XFS_ILOG_UUID);
+ ~(XFS_ILOG_DEXT | XFS_ILOG_DBROOT | XFS_ILOG_DEV);
if ((iip->ili_fields & XFS_ILOG_DDATA) &&
ip->i_df.if_bytes > 0) {
/*
@@ -224,18 +220,10 @@ xfs_inode_item_format_data_fork(
break;
case XFS_DINODE_FMT_DEV:
iip->ili_fields &=
- ~(XFS_ILOG_DDATA | XFS_ILOG_DBROOT |
- XFS_ILOG_DEXT | XFS_ILOG_UUID);
+ ~(XFS_ILOG_DDATA | XFS_ILOG_DBROOT | XFS_ILOG_DEXT);
if (iip->ili_fields & XFS_ILOG_DEV)
ilf->ilf_u.ilfu_rdev = ip->i_df.if_u2.if_rdev;
break;
- case XFS_DINODE_FMT_UUID:
- iip->ili_fields &=
- ~(XFS_ILOG_DDATA | XFS_ILOG_DBROOT |
- XFS_ILOG_DEXT | XFS_ILOG_DEV);
- if (iip->ili_fields & XFS_ILOG_UUID)
- ilf->ilf_u.ilfu_uuid = ip->i_df.if_u2.if_uuid;
- break;
default:
ASSERT(0);
break;
@@ -441,7 +429,7 @@ xfs_inode_item_format(
ilf->ilf_dsize = 0;
ilf->ilf_asize = 0;
ilf->ilf_pad = 0;
- uuid_copy(&ilf->ilf_u.ilfu_uuid, &uuid_null);
+ memset(&ilf->ilf_u, 0, sizeof(ilf->ilf_u));
xlog_finish_iovec(lv, vecp, sizeof(*ilf));
@@ -892,8 +880,7 @@ xfs_inode_item_format_convert(
in_f->ilf_asize = in_f32->ilf_asize;
in_f->ilf_dsize = in_f32->ilf_dsize;
in_f->ilf_ino = in_f32->ilf_ino;
- /* copy biggest field of ilf_u */
- uuid_copy(&in_f->ilf_u.ilfu_uuid, &in_f32->ilf_u.ilfu_uuid);
+ memcpy(&in_f->ilf_u, &in_f32->ilf_u, sizeof(in_f->ilf_u));
in_f->ilf_blkno = in_f32->ilf_blkno;
in_f->ilf_len = in_f32->ilf_len;
in_f->ilf_boffset = in_f32->ilf_boffset;
diff --git a/fs/xfs/xfs_itable.c b/fs/xfs/xfs_itable.c
index c393a2f6d8c3..23ba69fcc516 100644
--- a/fs/xfs/xfs_itable.c
+++ b/fs/xfs/xfs_itable.c
@@ -124,7 +124,6 @@ xfs_bulkstat_one_int(
buf->bs_blocks = 0;
break;
case XFS_DINODE_FMT_LOCAL:
- case XFS_DINODE_FMT_UUID:
buf->bs_rdev = 0;
buf->bs_blksize = mp->m_sb.sb_blocksize;
buf->bs_blocks = 0;
diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
index ee34899396b2..4e48e0534345 100644
--- a/fs/xfs/xfs_log_recover.c
+++ b/fs/xfs/xfs_log_recover.c
@@ -3163,16 +3163,8 @@ xlog_recover_inode_pass2(
}
fields = in_f->ilf_fields;
- switch (fields & (XFS_ILOG_DEV | XFS_ILOG_UUID)) {
- case XFS_ILOG_DEV:
+ if (fields & XFS_ILOG_DEV)
xfs_dinode_put_rdev(dip, in_f->ilf_u.ilfu_rdev);
- break;
- case XFS_ILOG_UUID:
- memcpy(XFS_DFORK_DPTR(dip),
- &in_f->ilf_u.ilfu_uuid,
- sizeof(uuid_t));
- break;
- }
if (in_f->ilf_size == 2)
goto out_owner_change;
--
2.14.1
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [PATCH 06/15] xfs: remove the never fully implemented UUID fork format
2017-10-19 6:59 ` [PATCH 06/15] xfs: remove the never fully implemented UUID fork format Christoph Hellwig
@ 2017-10-19 22:48 ` Darrick J. Wong
2017-10-20 7:02 ` Christoph Hellwig
0 siblings, 1 reply; 36+ messages in thread
From: Darrick J. Wong @ 2017-10-19 22:48 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs
On Thu, Oct 19, 2017 at 08:59:33AM +0200, Christoph Hellwig wrote:
> Remove the dead code dealing with the UUID fork format that was never
> implemented in Linux (and neither in IRIX as far as I know).
I think I need a little more context on this -- for what purpose was the
UUID format created, and has it ever been used anywhere? When I was
working on the ifork verifiers/scrub I noticed that we'd never actually
accept anything with that format.
(I'm ok with removing it, I just want to know what I'm killing before I
commit to it.)
--D
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> fs/xfs/libxfs/xfs_attr_leaf.c | 6 +-----
> fs/xfs/libxfs/xfs_bmap.c | 4 ----
> fs/xfs/libxfs/xfs_format.h | 2 +-
> fs/xfs/libxfs/xfs_inode_fork.c | 9 ---------
> fs/xfs/libxfs/xfs_inode_fork.h | 1 -
> fs/xfs/libxfs/xfs_log_format.h | 20 ++++++++++----------
> fs/xfs/xfs_inode_item.c | 25 ++++++-------------------
> fs/xfs/xfs_itable.c | 1 -
> fs/xfs/xfs_log_recover.c | 10 +---------
> 9 files changed, 19 insertions(+), 59 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
> index 5c16db86b38f..53cc8b986eac 100644
> --- a/fs/xfs/libxfs/xfs_attr_leaf.c
> +++ b/fs/xfs/libxfs/xfs_attr_leaf.c
> @@ -397,13 +397,9 @@ xfs_attr_shortform_bytesfit(xfs_inode_t *dp, int bytes)
> /* rounded down */
> offset = (XFS_LITINO(mp, dp->i_d.di_version) - bytes) >> 3;
>
> - switch (dp->i_d.di_format) {
> - case XFS_DINODE_FMT_DEV:
> + if (dp->i_d.di_format == XFS_DINODE_FMT_DEV) {
> minforkoff = roundup(sizeof(xfs_dev_t), 8) >> 3;
> return (offset >= minforkoff) ? minforkoff : 0;
> - case XFS_DINODE_FMT_UUID:
> - minforkoff = roundup(sizeof(uuid_t), 8) >> 3;
> - return (offset >= minforkoff) ? minforkoff : 0;
> }
>
> /*
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index 6431386f4676..22e7578e5696 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -231,7 +231,6 @@ xfs_bmap_forkoff_reset(
> {
> if (whichfork == XFS_ATTR_FORK &&
> ip->i_d.di_format != XFS_DINODE_FMT_DEV &&
> - ip->i_d.di_format != XFS_DINODE_FMT_UUID &&
> ip->i_d.di_format != XFS_DINODE_FMT_BTREE) {
> uint dfl_forkoff = xfs_default_attroffset(ip) >> 3;
>
> @@ -1086,9 +1085,6 @@ xfs_bmap_add_attrfork(
> case XFS_DINODE_FMT_DEV:
> ip->i_d.di_forkoff = roundup(sizeof(xfs_dev_t), 8) >> 3;
> break;
> - case XFS_DINODE_FMT_UUID:
> - ip->i_d.di_forkoff = roundup(sizeof(uuid_t), 8) >> 3;
> - break;
> case XFS_DINODE_FMT_LOCAL:
> case XFS_DINODE_FMT_EXTENTS:
> case XFS_DINODE_FMT_BTREE:
> diff --git a/fs/xfs/libxfs/xfs_format.h b/fs/xfs/libxfs/xfs_format.h
> index 23229f0c5b15..66f9b9f61d92 100644
> --- a/fs/xfs/libxfs/xfs_format.h
> +++ b/fs/xfs/libxfs/xfs_format.h
> @@ -941,7 +941,7 @@ typedef enum xfs_dinode_fmt {
> XFS_DINODE_FMT_LOCAL, /* bulk data */
> XFS_DINODE_FMT_EXTENTS, /* struct xfs_bmbt_rec */
> XFS_DINODE_FMT_BTREE, /* struct xfs_bmdr_block */
> - XFS_DINODE_FMT_UUID /* uuid_t */
> + XFS_DINODE_FMT_UUID /* added long ago, but never used */
> } xfs_dinode_fmt_t;
>
> /*
> diff --git a/fs/xfs/libxfs/xfs_inode_fork.c b/fs/xfs/libxfs/xfs_inode_fork.c
> index 31786bad9738..1d003ca21562 100644
> --- a/fs/xfs/libxfs/xfs_inode_fork.c
> +++ b/fs/xfs/libxfs/xfs_inode_fork.c
> @@ -902,15 +902,6 @@ xfs_iflush_fork(
> }
> break;
>
> - case XFS_DINODE_FMT_UUID:
> - if (iip->ili_fields & XFS_ILOG_UUID) {
> - ASSERT(whichfork == XFS_DATA_FORK);
> - memcpy(XFS_DFORK_DPTR(dip),
> - &ip->i_df.if_u2.if_uuid,
> - sizeof(uuid_t));
> - }
> - break;
> -
> default:
> ASSERT(0);
> break;
> diff --git a/fs/xfs/libxfs/xfs_inode_fork.h b/fs/xfs/libxfs/xfs_inode_fork.h
> index 6750f0462d21..064babdc373c 100644
> --- a/fs/xfs/libxfs/xfs_inode_fork.h
> +++ b/fs/xfs/libxfs/xfs_inode_fork.h
> @@ -70,7 +70,6 @@ typedef struct xfs_ifork {
> char if_inline_data[XFS_INLINE_DATA];
> /* very small file data */
> xfs_dev_t if_rdev; /* dev number if special */
> - uuid_t if_uuid; /* mount point value */
> } if_u2;
> } xfs_ifork_t;
>
> diff --git a/fs/xfs/libxfs/xfs_log_format.h b/fs/xfs/libxfs/xfs_log_format.h
> index 71de185735e0..a7ab6adae7f6 100644
> --- a/fs/xfs/libxfs/xfs_log_format.h
> +++ b/fs/xfs/libxfs/xfs_log_format.h
> @@ -274,7 +274,7 @@ typedef struct xfs_inode_log_format {
> uint64_t ilf_ino; /* inode number */
> union {
> uint32_t ilfu_rdev; /* rdev value for dev inode*/
> - uuid_t ilfu_uuid; /* mount point value */
> + u8 __pad[16]; /* unused */
> } ilf_u;
> int64_t ilf_blkno; /* blkno of inode buffer */
> int32_t ilf_len; /* len of inode buffer */
> @@ -295,7 +295,7 @@ struct xfs_inode_log_format_32 {
> uint64_t ilf_ino; /* inode number */
> union {
> uint32_t ilfu_rdev; /* rdev value for dev inode*/
> - uuid_t ilfu_uuid; /* mount point value */
> + u8 __pad[16]; /* unused */
> } ilf_u;
> int64_t ilf_blkno; /* blkno of inode buffer */
> int32_t ilf_len; /* len of inode buffer */
> @@ -311,7 +311,7 @@ struct xfs_inode_log_format_32 {
> #define XFS_ILOG_DEXT 0x004 /* log i_df.if_extents */
> #define XFS_ILOG_DBROOT 0x008 /* log i_df.i_broot */
> #define XFS_ILOG_DEV 0x010 /* log the dev field */
> -#define XFS_ILOG_UUID 0x020 /* log the uuid field */
> +#define XFS_ILOG_UUID 0x020 /* added long ago, but never used */
> #define XFS_ILOG_ADATA 0x040 /* log i_af.if_data */
> #define XFS_ILOG_AEXT 0x080 /* log i_af.if_extents */
> #define XFS_ILOG_ABROOT 0x100 /* log i_af.i_broot */
> @@ -329,9 +329,9 @@ struct xfs_inode_log_format_32 {
>
> #define XFS_ILOG_NONCORE (XFS_ILOG_DDATA | XFS_ILOG_DEXT | \
> XFS_ILOG_DBROOT | XFS_ILOG_DEV | \
> - XFS_ILOG_UUID | XFS_ILOG_ADATA | \
> - XFS_ILOG_AEXT | XFS_ILOG_ABROOT | \
> - XFS_ILOG_DOWNER | XFS_ILOG_AOWNER)
> + XFS_ILOG_ADATA | XFS_ILOG_AEXT | \
> + XFS_ILOG_ABROOT | XFS_ILOG_DOWNER | \
> + XFS_ILOG_AOWNER)
>
> #define XFS_ILOG_DFORK (XFS_ILOG_DDATA | XFS_ILOG_DEXT | \
> XFS_ILOG_DBROOT)
> @@ -341,10 +341,10 @@ struct xfs_inode_log_format_32 {
>
> #define XFS_ILOG_ALL (XFS_ILOG_CORE | XFS_ILOG_DDATA | \
> XFS_ILOG_DEXT | XFS_ILOG_DBROOT | \
> - XFS_ILOG_DEV | XFS_ILOG_UUID | \
> - XFS_ILOG_ADATA | XFS_ILOG_AEXT | \
> - XFS_ILOG_ABROOT | XFS_ILOG_TIMESTAMP | \
> - XFS_ILOG_DOWNER | XFS_ILOG_AOWNER)
> + XFS_ILOG_DEV | XFS_ILOG_ADATA | \
> + XFS_ILOG_AEXT | XFS_ILOG_ABROOT | \
> + XFS_ILOG_TIMESTAMP | XFS_ILOG_DOWNER | \
> + XFS_ILOG_AOWNER)
>
> static inline int xfs_ilog_fbroot(int w)
> {
> diff --git a/fs/xfs/xfs_inode_item.c b/fs/xfs/xfs_inode_item.c
> index 9bbc2d7cc8cb..bd60ad313173 100644
> --- a/fs/xfs/xfs_inode_item.c
> +++ b/fs/xfs/xfs_inode_item.c
> @@ -72,7 +72,6 @@ xfs_inode_item_data_fork_size(
> break;
>
> case XFS_DINODE_FMT_DEV:
> - case XFS_DINODE_FMT_UUID:
> break;
> default:
> ASSERT(0);
> @@ -156,8 +155,7 @@ xfs_inode_item_format_data_fork(
> switch (ip->i_d.di_format) {
> case XFS_DINODE_FMT_EXTENTS:
> iip->ili_fields &=
> - ~(XFS_ILOG_DDATA | XFS_ILOG_DBROOT |
> - XFS_ILOG_DEV | XFS_ILOG_UUID);
> + ~(XFS_ILOG_DDATA | XFS_ILOG_DBROOT | XFS_ILOG_DEV);
>
> if ((iip->ili_fields & XFS_ILOG_DEXT) &&
> ip->i_d.di_nextents > 0 &&
> @@ -181,8 +179,7 @@ xfs_inode_item_format_data_fork(
> break;
> case XFS_DINODE_FMT_BTREE:
> iip->ili_fields &=
> - ~(XFS_ILOG_DDATA | XFS_ILOG_DEXT |
> - XFS_ILOG_DEV | XFS_ILOG_UUID);
> + ~(XFS_ILOG_DDATA | XFS_ILOG_DEXT | XFS_ILOG_DEV);
>
> if ((iip->ili_fields & XFS_ILOG_DBROOT) &&
> ip->i_df.if_broot_bytes > 0) {
> @@ -200,8 +197,7 @@ xfs_inode_item_format_data_fork(
> break;
> case XFS_DINODE_FMT_LOCAL:
> iip->ili_fields &=
> - ~(XFS_ILOG_DEXT | XFS_ILOG_DBROOT |
> - XFS_ILOG_DEV | XFS_ILOG_UUID);
> + ~(XFS_ILOG_DEXT | XFS_ILOG_DBROOT | XFS_ILOG_DEV);
> if ((iip->ili_fields & XFS_ILOG_DDATA) &&
> ip->i_df.if_bytes > 0) {
> /*
> @@ -224,18 +220,10 @@ xfs_inode_item_format_data_fork(
> break;
> case XFS_DINODE_FMT_DEV:
> iip->ili_fields &=
> - ~(XFS_ILOG_DDATA | XFS_ILOG_DBROOT |
> - XFS_ILOG_DEXT | XFS_ILOG_UUID);
> + ~(XFS_ILOG_DDATA | XFS_ILOG_DBROOT | XFS_ILOG_DEXT);
> if (iip->ili_fields & XFS_ILOG_DEV)
> ilf->ilf_u.ilfu_rdev = ip->i_df.if_u2.if_rdev;
> break;
> - case XFS_DINODE_FMT_UUID:
> - iip->ili_fields &=
> - ~(XFS_ILOG_DDATA | XFS_ILOG_DBROOT |
> - XFS_ILOG_DEXT | XFS_ILOG_DEV);
> - if (iip->ili_fields & XFS_ILOG_UUID)
> - ilf->ilf_u.ilfu_uuid = ip->i_df.if_u2.if_uuid;
> - break;
> default:
> ASSERT(0);
> break;
> @@ -441,7 +429,7 @@ xfs_inode_item_format(
> ilf->ilf_dsize = 0;
> ilf->ilf_asize = 0;
> ilf->ilf_pad = 0;
> - uuid_copy(&ilf->ilf_u.ilfu_uuid, &uuid_null);
> + memset(&ilf->ilf_u, 0, sizeof(ilf->ilf_u));
>
> xlog_finish_iovec(lv, vecp, sizeof(*ilf));
>
> @@ -892,8 +880,7 @@ xfs_inode_item_format_convert(
> in_f->ilf_asize = in_f32->ilf_asize;
> in_f->ilf_dsize = in_f32->ilf_dsize;
> in_f->ilf_ino = in_f32->ilf_ino;
> - /* copy biggest field of ilf_u */
> - uuid_copy(&in_f->ilf_u.ilfu_uuid, &in_f32->ilf_u.ilfu_uuid);
> + memcpy(&in_f->ilf_u, &in_f32->ilf_u, sizeof(in_f->ilf_u));
> in_f->ilf_blkno = in_f32->ilf_blkno;
> in_f->ilf_len = in_f32->ilf_len;
> in_f->ilf_boffset = in_f32->ilf_boffset;
> diff --git a/fs/xfs/xfs_itable.c b/fs/xfs/xfs_itable.c
> index c393a2f6d8c3..23ba69fcc516 100644
> --- a/fs/xfs/xfs_itable.c
> +++ b/fs/xfs/xfs_itable.c
> @@ -124,7 +124,6 @@ xfs_bulkstat_one_int(
> buf->bs_blocks = 0;
> break;
> case XFS_DINODE_FMT_LOCAL:
> - case XFS_DINODE_FMT_UUID:
> buf->bs_rdev = 0;
> buf->bs_blksize = mp->m_sb.sb_blocksize;
> buf->bs_blocks = 0;
> diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
> index ee34899396b2..4e48e0534345 100644
> --- a/fs/xfs/xfs_log_recover.c
> +++ b/fs/xfs/xfs_log_recover.c
> @@ -3163,16 +3163,8 @@ xlog_recover_inode_pass2(
> }
>
> fields = in_f->ilf_fields;
> - switch (fields & (XFS_ILOG_DEV | XFS_ILOG_UUID)) {
> - case XFS_ILOG_DEV:
> + if (fields & XFS_ILOG_DEV)
> xfs_dinode_put_rdev(dip, in_f->ilf_u.ilfu_rdev);
> - break;
> - case XFS_ILOG_UUID:
> - memcpy(XFS_DFORK_DPTR(dip),
> - &in_f->ilf_u.ilfu_uuid,
> - sizeof(uuid_t));
> - break;
> - }
>
> if (in_f->ilf_size == 2)
> goto out_owner_change;
> --
> 2.14.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [PATCH 06/15] xfs: remove the never fully implemented UUID fork format
2017-10-19 22:48 ` Darrick J. Wong
@ 2017-10-20 7:02 ` Christoph Hellwig
2017-10-20 16:52 ` Darrick J. Wong
0 siblings, 1 reply; 36+ messages in thread
From: Christoph Hellwig @ 2017-10-20 7:02 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: Christoph Hellwig, linux-xfs
On Thu, Oct 19, 2017 at 03:48:13PM -0700, Darrick J. Wong wrote:
> On Thu, Oct 19, 2017 at 08:59:33AM +0200, Christoph Hellwig wrote:
> > Remove the dead code dealing with the UUID fork format that was never
> > implemented in Linux (and neither in IRIX as far as I know).
>
> I think I need a little more context on this -- for what purpose was the
> UUID format created, and has it ever been used anywhere? When I was
> working on the ifork verifiers/scrub I noticed that we'd never actually
> accept anything with that format.
>
> (I'm ok with removing it, I just want to know what I'm killing before I
> commit to it.)
For that you'd need to find people that were around at SGI in the 90s..
Note that we already removed it briefly in 2008, but unlike my patch
that didn't keep the padding so broken log recovery.
relevant xfs-import commits:
Commit that adds the di_format values:
c049a3e8d776a76a1ba3adaf168a406a8ee52172
then various commits in early development add UUID inode format
bits, but none seems to actually ever generate it.
1st removal with minor comments on the history:
415c805b441845dfc2be7d887f995ea8875358e9
Revert of the above:
4ddd5953fad77a14f665fefa06899ef9f388b134
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 06/15] xfs: remove the never fully implemented UUID fork format
2017-10-20 7:02 ` Christoph Hellwig
@ 2017-10-20 16:52 ` Darrick J. Wong
0 siblings, 0 replies; 36+ messages in thread
From: Darrick J. Wong @ 2017-10-20 16:52 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs
On Fri, Oct 20, 2017 at 09:02:35AM +0200, Christoph Hellwig wrote:
> On Thu, Oct 19, 2017 at 03:48:13PM -0700, Darrick J. Wong wrote:
> > On Thu, Oct 19, 2017 at 08:59:33AM +0200, Christoph Hellwig wrote:
> > > Remove the dead code dealing with the UUID fork format that was never
> > > implemented in Linux (and neither in IRIX as far as I know).
> >
> > I think I need a little more context on this -- for what purpose was the
> > UUID format created, and has it ever been used anywhere? When I was
> > working on the ifork verifiers/scrub I noticed that we'd never actually
> > accept anything with that format.
> >
> > (I'm ok with removing it, I just want to know what I'm killing before I
> > commit to it.)
>
> For that you'd need to find people that were around at SGI in the 90s..
>
> Note that we already removed it briefly in 2008, but unlike my patch
> that didn't keep the padding so broken log recovery.
>
> relevant xfs-import commits:
>
> Commit that adds the di_format values:
> c049a3e8d776a76a1ba3adaf168a406a8ee52172
>
> then various commits in early development add UUID inode format
> bits, but none seems to actually ever generate it.
>
> 1st removal with minor comments on the history:
> 415c805b441845dfc2be7d887f995ea8875358e9
Yummmmmmmmmy!
Looks ok,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
--D
>
> Revert of the above:
> 4ddd5953fad77a14f665fefa06899ef9f388b134
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH 07/15] xfs: remove if_rdev
2017-10-19 6:59 more extent mapping cleanups Christoph Hellwig
` (5 preceding siblings ...)
2017-10-19 6:59 ` [PATCH 06/15] xfs: remove the never fully implemented UUID fork format Christoph Hellwig
@ 2017-10-19 6:59 ` Christoph Hellwig
2017-10-19 22:52 ` Darrick J. Wong
2017-10-19 6:59 ` [PATCH 08/15] xfs: inline xfs_shift_file_space into callers Christoph Hellwig
` (8 subsequent siblings)
15 siblings, 1 reply; 36+ messages in thread
From: Christoph Hellwig @ 2017-10-19 6:59 UTC (permalink / raw)
To: linux-xfs
We can simply use the i_rdev field in the Linux inode and just convert
to and from the XFS dev_t when reading or logging/writing the inode.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/xfs/libxfs/xfs_inode_fork.c | 38 +++++++++++++++++++++-----------------
fs/xfs/libxfs/xfs_inode_fork.h | 1 -
fs/xfs/xfs_inode.c | 9 ++++-----
fs/xfs/xfs_inode.h | 4 ++--
fs/xfs/xfs_inode_item.c | 2 +-
fs/xfs/xfs_iops.c | 16 +---------------
fs/xfs/xfs_itable.c | 2 +-
7 files changed, 30 insertions(+), 42 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_inode_fork.c b/fs/xfs/libxfs/xfs_inode_fork.c
index 1d003ca21562..b1e69734c450 100644
--- a/fs/xfs/libxfs/xfs_inode_fork.c
+++ b/fs/xfs/libxfs/xfs_inode_fork.c
@@ -42,21 +42,27 @@ STATIC int xfs_iformat_local(xfs_inode_t *, xfs_dinode_t *, int, int);
STATIC int xfs_iformat_extents(xfs_inode_t *, xfs_dinode_t *, int);
STATIC int xfs_iformat_btree(xfs_inode_t *, xfs_dinode_t *, int);
+static inline dev_t xfs_to_linux_dev_t(xfs_dev_t dev)
+{
+ return MKDEV(sysv_major(dev) & 0x1ff, sysv_minor(dev));
+}
+
/*
- * Move inode type and inode format specific information from the
- * on-disk inode to the in-core inode. For fifos, devs, and sockets
- * this means set if_rdev to the proper value. For files, directories,
- * and symlinks this means to bring in the in-line data or extent
- * pointers. For a file in B-tree format, only the root is immediately
- * brought in-core. The rest will be in-lined in if_extents when it
- * is first referenced (see xfs_iread_extents()).
+ * Copy inode type and data and attr format specific information from the
+ * on-disk inode to the in-core inode and fork structures. For fifos, devices,
+ * and sockets this means set i_rdev to the proper value. For files,
+ * directories, and symlinks this means to bring in the in-line data or extent
+ * pointers as well as the attribute fork. For a fork in B-tree format, only
+ * the root is immediately brought in-core. The rest will be read in later when
+ * first referenced (see xfs_iread_extents()).
*/
int
xfs_iformat_fork(
- xfs_inode_t *ip,
- xfs_dinode_t *dip)
+ struct xfs_inode *ip,
+ struct xfs_dinode *dip)
{
- xfs_attr_shortform_t *atp;
+ struct inode *inode = VFS_I(ip);
+ struct xfs_attr_shortform *atp;
int size;
int error = 0;
xfs_fsize_t di_size;
@@ -95,8 +101,7 @@ xfs_iformat_fork(
return -EFSCORRUPTED;
}
- if (unlikely(xfs_is_reflink_inode(ip) &&
- (VFS_I(ip)->i_mode & S_IFMT) != S_IFREG)) {
+ if (unlikely(xfs_is_reflink_inode(ip) && !S_ISREG(inode->i_mode))) {
xfs_warn(ip->i_mount,
"corrupt dinode %llu, wrong file type for reflink.",
ip->i_ino);
@@ -115,7 +120,7 @@ xfs_iformat_fork(
return -EFSCORRUPTED;
}
- switch (VFS_I(ip)->i_mode & S_IFMT) {
+ switch (inode->i_mode & S_IFMT) {
case S_IFIFO:
case S_IFCHR:
case S_IFBLK:
@@ -126,7 +131,7 @@ xfs_iformat_fork(
return -EFSCORRUPTED;
}
ip->i_d.di_size = 0;
- ip->i_df.if_u2.if_rdev = xfs_dinode_get_rdev(dip);
+ inode->i_rdev = xfs_to_linux_dev_t(xfs_dinode_get_rdev(dip));
break;
case S_IFREG:
@@ -184,8 +189,7 @@ xfs_iformat_fork(
return error;
/* Check inline dir contents. */
- if (S_ISDIR(VFS_I(ip)->i_mode) &&
- dip->di_format == XFS_DINODE_FMT_LOCAL) {
+ if (S_ISDIR(inode->i_mode) && dip->di_format == XFS_DINODE_FMT_LOCAL) {
error = xfs_dir2_sf_verify(ip);
if (error) {
xfs_idestroy_fork(ip, XFS_DATA_FORK);
@@ -898,7 +902,7 @@ xfs_iflush_fork(
case XFS_DINODE_FMT_DEV:
if (iip->ili_fields & XFS_ILOG_DEV) {
ASSERT(whichfork == XFS_DATA_FORK);
- xfs_dinode_put_rdev(dip, ip->i_df.if_u2.if_rdev);
+ xfs_dinode_put_rdev(dip, sysv_encode_dev(VFS_I(ip)->i_rdev));
}
break;
diff --git a/fs/xfs/libxfs/xfs_inode_fork.h b/fs/xfs/libxfs/xfs_inode_fork.h
index 064babdc373c..e0c42ea9b8d0 100644
--- a/fs/xfs/libxfs/xfs_inode_fork.h
+++ b/fs/xfs/libxfs/xfs_inode_fork.h
@@ -69,7 +69,6 @@ typedef struct xfs_ifork {
/* very small file extents */
char if_inline_data[XFS_INLINE_DATA];
/* very small file data */
- xfs_dev_t if_rdev; /* dev number if special */
} if_u2;
} xfs_ifork_t;
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index 4ec5b7f45401..a929ca72fa8e 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -767,7 +767,7 @@ xfs_ialloc(
xfs_inode_t *pip,
umode_t mode,
xfs_nlink_t nlink,
- xfs_dev_t rdev,
+ dev_t rdev,
prid_t prid,
int okalloc,
xfs_buf_t **ialloc_context,
@@ -819,6 +819,7 @@ xfs_ialloc(
set_nlink(inode, nlink);
ip->i_d.di_uid = xfs_kuid_to_uid(current_fsuid());
ip->i_d.di_gid = xfs_kgid_to_gid(current_fsgid());
+ inode->i_rdev = rdev;
xfs_set_projid(ip, prid);
if (pip && XFS_INHERIT_GID(pip)) {
@@ -867,7 +868,6 @@ xfs_ialloc(
case S_IFBLK:
case S_IFSOCK:
ip->i_d.di_format = XFS_DINODE_FMT_DEV;
- ip->i_df.if_u2.if_rdev = rdev;
ip->i_df.if_flags = 0;
flags |= XFS_ILOG_DEV;
break;
@@ -975,7 +975,7 @@ xfs_dir_ialloc(
the inode. */
umode_t mode,
xfs_nlink_t nlink,
- xfs_dev_t rdev,
+ dev_t rdev,
prid_t prid, /* project id */
int okalloc, /* ok to allocate new space */
xfs_inode_t **ipp, /* pointer to inode; it will be
@@ -1147,7 +1147,7 @@ xfs_create(
xfs_inode_t *dp,
struct xfs_name *name,
umode_t mode,
- xfs_dev_t rdev,
+ dev_t rdev,
xfs_inode_t **ipp)
{
int is_dir = S_ISDIR(mode);
@@ -1183,7 +1183,6 @@ xfs_create(
return error;
if (is_dir) {
- rdev = 0;
resblks = XFS_MKDIR_SPACE_RES(mp, name->len);
tres = &M_RES(mp)->tr_mkdir;
} else {
diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h
index 0ee453de239a..cc13c3763721 100644
--- a/fs/xfs/xfs_inode.h
+++ b/fs/xfs/xfs_inode.h
@@ -391,7 +391,7 @@ void xfs_inactive(struct xfs_inode *ip);
int xfs_lookup(struct xfs_inode *dp, struct xfs_name *name,
struct xfs_inode **ipp, struct xfs_name *ci_name);
int xfs_create(struct xfs_inode *dp, struct xfs_name *name,
- umode_t mode, xfs_dev_t rdev, struct xfs_inode **ipp);
+ umode_t mode, dev_t rdev, struct xfs_inode **ipp);
int xfs_create_tmpfile(struct xfs_inode *dp, struct dentry *dentry,
umode_t mode, struct xfs_inode **ipp);
int xfs_remove(struct xfs_inode *dp, struct xfs_name *name,
@@ -428,7 +428,7 @@ xfs_extlen_t xfs_get_extsz_hint(struct xfs_inode *ip);
xfs_extlen_t xfs_get_cowextsz_hint(struct xfs_inode *ip);
int xfs_dir_ialloc(struct xfs_trans **, struct xfs_inode *, umode_t,
- xfs_nlink_t, xfs_dev_t, prid_t, int,
+ xfs_nlink_t, dev_t, prid_t, int,
struct xfs_inode **, int *);
/* from xfs_file.c */
diff --git a/fs/xfs/xfs_inode_item.c b/fs/xfs/xfs_inode_item.c
index bd60ad313173..eb6f4f7c9520 100644
--- a/fs/xfs/xfs_inode_item.c
+++ b/fs/xfs/xfs_inode_item.c
@@ -222,7 +222,7 @@ xfs_inode_item_format_data_fork(
iip->ili_fields &=
~(XFS_ILOG_DDATA | XFS_ILOG_DBROOT | XFS_ILOG_DEXT);
if (iip->ili_fields & XFS_ILOG_DEV)
- ilf->ilf_u.ilfu_rdev = ip->i_df.if_u2.if_rdev;
+ ilf->ilf_u.ilfu_rdev = sysv_encode_dev(VFS_I(ip)->i_rdev);
break;
default:
ASSERT(0);
diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
index 17081c77ef86..8b5676d244ca 100644
--- a/fs/xfs/xfs_iops.c
+++ b/fs/xfs/xfs_iops.c
@@ -160,7 +160,6 @@ xfs_generic_create(
if (S_ISCHR(mode) || S_ISBLK(mode)) {
if (unlikely(!sysv_valid_dev(rdev) || MAJOR(rdev) & ~0x1ff))
return -EINVAL;
- rdev = sysv_encode_dev(rdev);
} else {
rdev = 0;
}
@@ -535,8 +534,7 @@ xfs_vn_getattr(
case S_IFBLK:
case S_IFCHR:
stat->blksize = BLKDEV_IOSIZE;
- stat->rdev = MKDEV(sysv_major(ip->i_df.if_u2.if_rdev) & 0x1ff,
- sysv_minor(ip->i_df.if_u2.if_rdev));
+ stat->rdev = inode->i_rdev;
break;
default:
if (XFS_IS_REALTIME_INODE(ip)) {
@@ -1231,18 +1229,6 @@ xfs_setup_inode(
inode->i_uid = xfs_uid_to_kuid(ip->i_d.di_uid);
inode->i_gid = xfs_gid_to_kgid(ip->i_d.di_gid);
- switch (inode->i_mode & S_IFMT) {
- case S_IFBLK:
- case S_IFCHR:
- inode->i_rdev =
- MKDEV(sysv_major(ip->i_df.if_u2.if_rdev) & 0x1ff,
- sysv_minor(ip->i_df.if_u2.if_rdev));
- break;
- default:
- inode->i_rdev = 0;
- break;
- }
-
i_size_write(inode, ip->i_d.di_size);
xfs_diflags_to_iflags(inode, ip);
diff --git a/fs/xfs/xfs_itable.c b/fs/xfs/xfs_itable.c
index 23ba69fcc516..7595baee0c7c 100644
--- a/fs/xfs/xfs_itable.c
+++ b/fs/xfs/xfs_itable.c
@@ -119,7 +119,7 @@ xfs_bulkstat_one_int(
switch (dic->di_format) {
case XFS_DINODE_FMT_DEV:
- buf->bs_rdev = ip->i_df.if_u2.if_rdev;
+ buf->bs_rdev = sysv_encode_dev(inode->i_rdev);
buf->bs_blksize = BLKDEV_IOSIZE;
buf->bs_blocks = 0;
break;
--
2.14.1
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [PATCH 07/15] xfs: remove if_rdev
2017-10-19 6:59 ` [PATCH 07/15] xfs: remove if_rdev Christoph Hellwig
@ 2017-10-19 22:52 ` Darrick J. Wong
0 siblings, 0 replies; 36+ messages in thread
From: Darrick J. Wong @ 2017-10-19 22:52 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs
On Thu, Oct 19, 2017 at 08:59:34AM +0200, Christoph Hellwig wrote:
> We can simply use the i_rdev field in the Linux inode and just convert
> to and from the XFS dev_t when reading or logging/writing the inode.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
Looks ok,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
> fs/xfs/libxfs/xfs_inode_fork.c | 38 +++++++++++++++++++++-----------------
> fs/xfs/libxfs/xfs_inode_fork.h | 1 -
> fs/xfs/xfs_inode.c | 9 ++++-----
> fs/xfs/xfs_inode.h | 4 ++--
> fs/xfs/xfs_inode_item.c | 2 +-
> fs/xfs/xfs_iops.c | 16 +---------------
> fs/xfs/xfs_itable.c | 2 +-
> 7 files changed, 30 insertions(+), 42 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_inode_fork.c b/fs/xfs/libxfs/xfs_inode_fork.c
> index 1d003ca21562..b1e69734c450 100644
> --- a/fs/xfs/libxfs/xfs_inode_fork.c
> +++ b/fs/xfs/libxfs/xfs_inode_fork.c
> @@ -42,21 +42,27 @@ STATIC int xfs_iformat_local(xfs_inode_t *, xfs_dinode_t *, int, int);
> STATIC int xfs_iformat_extents(xfs_inode_t *, xfs_dinode_t *, int);
> STATIC int xfs_iformat_btree(xfs_inode_t *, xfs_dinode_t *, int);
>
> +static inline dev_t xfs_to_linux_dev_t(xfs_dev_t dev)
> +{
> + return MKDEV(sysv_major(dev) & 0x1ff, sysv_minor(dev));
> +}
> +
> /*
> - * Move inode type and inode format specific information from the
> - * on-disk inode to the in-core inode. For fifos, devs, and sockets
> - * this means set if_rdev to the proper value. For files, directories,
> - * and symlinks this means to bring in the in-line data or extent
> - * pointers. For a file in B-tree format, only the root is immediately
> - * brought in-core. The rest will be in-lined in if_extents when it
> - * is first referenced (see xfs_iread_extents()).
> + * Copy inode type and data and attr format specific information from the
> + * on-disk inode to the in-core inode and fork structures. For fifos, devices,
> + * and sockets this means set i_rdev to the proper value. For files,
> + * directories, and symlinks this means to bring in the in-line data or extent
> + * pointers as well as the attribute fork. For a fork in B-tree format, only
> + * the root is immediately brought in-core. The rest will be read in later when
> + * first referenced (see xfs_iread_extents()).
> */
> int
> xfs_iformat_fork(
> - xfs_inode_t *ip,
> - xfs_dinode_t *dip)
> + struct xfs_inode *ip,
> + struct xfs_dinode *dip)
> {
> - xfs_attr_shortform_t *atp;
> + struct inode *inode = VFS_I(ip);
> + struct xfs_attr_shortform *atp;
> int size;
> int error = 0;
> xfs_fsize_t di_size;
> @@ -95,8 +101,7 @@ xfs_iformat_fork(
> return -EFSCORRUPTED;
> }
>
> - if (unlikely(xfs_is_reflink_inode(ip) &&
> - (VFS_I(ip)->i_mode & S_IFMT) != S_IFREG)) {
> + if (unlikely(xfs_is_reflink_inode(ip) && !S_ISREG(inode->i_mode))) {
> xfs_warn(ip->i_mount,
> "corrupt dinode %llu, wrong file type for reflink.",
> ip->i_ino);
> @@ -115,7 +120,7 @@ xfs_iformat_fork(
> return -EFSCORRUPTED;
> }
>
> - switch (VFS_I(ip)->i_mode & S_IFMT) {
> + switch (inode->i_mode & S_IFMT) {
> case S_IFIFO:
> case S_IFCHR:
> case S_IFBLK:
> @@ -126,7 +131,7 @@ xfs_iformat_fork(
> return -EFSCORRUPTED;
> }
> ip->i_d.di_size = 0;
> - ip->i_df.if_u2.if_rdev = xfs_dinode_get_rdev(dip);
> + inode->i_rdev = xfs_to_linux_dev_t(xfs_dinode_get_rdev(dip));
> break;
>
> case S_IFREG:
> @@ -184,8 +189,7 @@ xfs_iformat_fork(
> return error;
>
> /* Check inline dir contents. */
> - if (S_ISDIR(VFS_I(ip)->i_mode) &&
> - dip->di_format == XFS_DINODE_FMT_LOCAL) {
> + if (S_ISDIR(inode->i_mode) && dip->di_format == XFS_DINODE_FMT_LOCAL) {
> error = xfs_dir2_sf_verify(ip);
> if (error) {
> xfs_idestroy_fork(ip, XFS_DATA_FORK);
> @@ -898,7 +902,7 @@ xfs_iflush_fork(
> case XFS_DINODE_FMT_DEV:
> if (iip->ili_fields & XFS_ILOG_DEV) {
> ASSERT(whichfork == XFS_DATA_FORK);
> - xfs_dinode_put_rdev(dip, ip->i_df.if_u2.if_rdev);
> + xfs_dinode_put_rdev(dip, sysv_encode_dev(VFS_I(ip)->i_rdev));
> }
> break;
>
> diff --git a/fs/xfs/libxfs/xfs_inode_fork.h b/fs/xfs/libxfs/xfs_inode_fork.h
> index 064babdc373c..e0c42ea9b8d0 100644
> --- a/fs/xfs/libxfs/xfs_inode_fork.h
> +++ b/fs/xfs/libxfs/xfs_inode_fork.h
> @@ -69,7 +69,6 @@ typedef struct xfs_ifork {
> /* very small file extents */
> char if_inline_data[XFS_INLINE_DATA];
> /* very small file data */
> - xfs_dev_t if_rdev; /* dev number if special */
> } if_u2;
> } xfs_ifork_t;
>
> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> index 4ec5b7f45401..a929ca72fa8e 100644
> --- a/fs/xfs/xfs_inode.c
> +++ b/fs/xfs/xfs_inode.c
> @@ -767,7 +767,7 @@ xfs_ialloc(
> xfs_inode_t *pip,
> umode_t mode,
> xfs_nlink_t nlink,
> - xfs_dev_t rdev,
> + dev_t rdev,
> prid_t prid,
> int okalloc,
> xfs_buf_t **ialloc_context,
> @@ -819,6 +819,7 @@ xfs_ialloc(
> set_nlink(inode, nlink);
> ip->i_d.di_uid = xfs_kuid_to_uid(current_fsuid());
> ip->i_d.di_gid = xfs_kgid_to_gid(current_fsgid());
> + inode->i_rdev = rdev;
> xfs_set_projid(ip, prid);
>
> if (pip && XFS_INHERIT_GID(pip)) {
> @@ -867,7 +868,6 @@ xfs_ialloc(
> case S_IFBLK:
> case S_IFSOCK:
> ip->i_d.di_format = XFS_DINODE_FMT_DEV;
> - ip->i_df.if_u2.if_rdev = rdev;
> ip->i_df.if_flags = 0;
> flags |= XFS_ILOG_DEV;
> break;
> @@ -975,7 +975,7 @@ xfs_dir_ialloc(
> the inode. */
> umode_t mode,
> xfs_nlink_t nlink,
> - xfs_dev_t rdev,
> + dev_t rdev,
> prid_t prid, /* project id */
> int okalloc, /* ok to allocate new space */
> xfs_inode_t **ipp, /* pointer to inode; it will be
> @@ -1147,7 +1147,7 @@ xfs_create(
> xfs_inode_t *dp,
> struct xfs_name *name,
> umode_t mode,
> - xfs_dev_t rdev,
> + dev_t rdev,
> xfs_inode_t **ipp)
> {
> int is_dir = S_ISDIR(mode);
> @@ -1183,7 +1183,6 @@ xfs_create(
> return error;
>
> if (is_dir) {
> - rdev = 0;
> resblks = XFS_MKDIR_SPACE_RES(mp, name->len);
> tres = &M_RES(mp)->tr_mkdir;
> } else {
> diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h
> index 0ee453de239a..cc13c3763721 100644
> --- a/fs/xfs/xfs_inode.h
> +++ b/fs/xfs/xfs_inode.h
> @@ -391,7 +391,7 @@ void xfs_inactive(struct xfs_inode *ip);
> int xfs_lookup(struct xfs_inode *dp, struct xfs_name *name,
> struct xfs_inode **ipp, struct xfs_name *ci_name);
> int xfs_create(struct xfs_inode *dp, struct xfs_name *name,
> - umode_t mode, xfs_dev_t rdev, struct xfs_inode **ipp);
> + umode_t mode, dev_t rdev, struct xfs_inode **ipp);
> int xfs_create_tmpfile(struct xfs_inode *dp, struct dentry *dentry,
> umode_t mode, struct xfs_inode **ipp);
> int xfs_remove(struct xfs_inode *dp, struct xfs_name *name,
> @@ -428,7 +428,7 @@ xfs_extlen_t xfs_get_extsz_hint(struct xfs_inode *ip);
> xfs_extlen_t xfs_get_cowextsz_hint(struct xfs_inode *ip);
>
> int xfs_dir_ialloc(struct xfs_trans **, struct xfs_inode *, umode_t,
> - xfs_nlink_t, xfs_dev_t, prid_t, int,
> + xfs_nlink_t, dev_t, prid_t, int,
> struct xfs_inode **, int *);
>
> /* from xfs_file.c */
> diff --git a/fs/xfs/xfs_inode_item.c b/fs/xfs/xfs_inode_item.c
> index bd60ad313173..eb6f4f7c9520 100644
> --- a/fs/xfs/xfs_inode_item.c
> +++ b/fs/xfs/xfs_inode_item.c
> @@ -222,7 +222,7 @@ xfs_inode_item_format_data_fork(
> iip->ili_fields &=
> ~(XFS_ILOG_DDATA | XFS_ILOG_DBROOT | XFS_ILOG_DEXT);
> if (iip->ili_fields & XFS_ILOG_DEV)
> - ilf->ilf_u.ilfu_rdev = ip->i_df.if_u2.if_rdev;
> + ilf->ilf_u.ilfu_rdev = sysv_encode_dev(VFS_I(ip)->i_rdev);
> break;
> default:
> ASSERT(0);
> diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
> index 17081c77ef86..8b5676d244ca 100644
> --- a/fs/xfs/xfs_iops.c
> +++ b/fs/xfs/xfs_iops.c
> @@ -160,7 +160,6 @@ xfs_generic_create(
> if (S_ISCHR(mode) || S_ISBLK(mode)) {
> if (unlikely(!sysv_valid_dev(rdev) || MAJOR(rdev) & ~0x1ff))
> return -EINVAL;
> - rdev = sysv_encode_dev(rdev);
> } else {
> rdev = 0;
> }
> @@ -535,8 +534,7 @@ xfs_vn_getattr(
> case S_IFBLK:
> case S_IFCHR:
> stat->blksize = BLKDEV_IOSIZE;
> - stat->rdev = MKDEV(sysv_major(ip->i_df.if_u2.if_rdev) & 0x1ff,
> - sysv_minor(ip->i_df.if_u2.if_rdev));
> + stat->rdev = inode->i_rdev;
> break;
> default:
> if (XFS_IS_REALTIME_INODE(ip)) {
> @@ -1231,18 +1229,6 @@ xfs_setup_inode(
> inode->i_uid = xfs_uid_to_kuid(ip->i_d.di_uid);
> inode->i_gid = xfs_gid_to_kgid(ip->i_d.di_gid);
>
> - switch (inode->i_mode & S_IFMT) {
> - case S_IFBLK:
> - case S_IFCHR:
> - inode->i_rdev =
> - MKDEV(sysv_major(ip->i_df.if_u2.if_rdev) & 0x1ff,
> - sysv_minor(ip->i_df.if_u2.if_rdev));
> - break;
> - default:
> - inode->i_rdev = 0;
> - break;
> - }
> -
> i_size_write(inode, ip->i_d.di_size);
> xfs_diflags_to_iflags(inode, ip);
>
> diff --git a/fs/xfs/xfs_itable.c b/fs/xfs/xfs_itable.c
> index 23ba69fcc516..7595baee0c7c 100644
> --- a/fs/xfs/xfs_itable.c
> +++ b/fs/xfs/xfs_itable.c
> @@ -119,7 +119,7 @@ xfs_bulkstat_one_int(
>
> switch (dic->di_format) {
> case XFS_DINODE_FMT_DEV:
> - buf->bs_rdev = ip->i_df.if_u2.if_rdev;
> + buf->bs_rdev = sysv_encode_dev(inode->i_rdev);
> buf->bs_blksize = BLKDEV_IOSIZE;
> buf->bs_blocks = 0;
> break;
> --
> 2.14.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH 08/15] xfs: inline xfs_shift_file_space into callers
2017-10-19 6:59 more extent mapping cleanups Christoph Hellwig
` (6 preceding siblings ...)
2017-10-19 6:59 ` [PATCH 07/15] xfs: remove if_rdev Christoph Hellwig
@ 2017-10-19 6:59 ` Christoph Hellwig
2017-10-21 0:07 ` Darrick J. Wong
2017-10-19 6:59 ` [PATCH 09/15] xfs: remove XFS_BMAP_MAX_SHIFT_EXTENTS Christoph Hellwig
` (7 subsequent siblings)
15 siblings, 1 reply; 36+ messages in thread
From: Christoph Hellwig @ 2017-10-19 6:59 UTC (permalink / raw)
To: linux-xfs
The code is sufficiently different for the insert vs collapse cases both
in xfs_shift_file_space itself and the callers that untangling them will
make life a lot easier down the road.
We still keep a common helper for flushing all data and COW state to get
the inode into the right shape for shifting the extents around.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/xfs/xfs_bmap_util.c | 192 ++++++++++++++++++++++++++-----------------------
1 file changed, 102 insertions(+), 90 deletions(-)
diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
index 0543423651ff..47b53c88de7c 100644
--- a/fs/xfs/xfs_bmap_util.c
+++ b/fs/xfs/xfs_bmap_util.c
@@ -1260,53 +1260,12 @@ xfs_zero_file_space(
}
-/*
- * @next_fsb will keep track of the extent currently undergoing shift.
- * @stop_fsb will keep track of the extent at which we have to stop.
- * If we are shifting left, we will start with block (offset + len) and
- * shift each extent till last extent.
- * If we are shifting right, we will start with last extent inside file space
- * and continue until we reach the block corresponding to offset.
- */
static int
-xfs_shift_file_space(
- struct xfs_inode *ip,
- xfs_off_t offset,
- xfs_off_t len,
- enum shift_direction direction)
+xfs_prepare_shift(
+ struct xfs_inode *ip,
+ loff_t offset)
{
- int done = 0;
- struct xfs_mount *mp = ip->i_mount;
- struct xfs_trans *tp;
int error;
- struct xfs_defer_ops dfops;
- xfs_fsblock_t first_block;
- xfs_fileoff_t stop_fsb;
- xfs_fileoff_t next_fsb;
- xfs_fileoff_t shift_fsb;
- uint resblks;
-
- ASSERT(direction == SHIFT_LEFT || direction == SHIFT_RIGHT);
-
- if (direction == SHIFT_LEFT) {
- /*
- * Reserve blocks to cover potential extent merges after left
- * shift operations.
- */
- resblks = XFS_DIOSTRAT_SPACE_RES(mp, 0);
- next_fsb = XFS_B_TO_FSB(mp, offset + len);
- stop_fsb = XFS_B_TO_FSB(mp, VFS_I(ip)->i_size);
- } else {
- /*
- * If right shift, delegate the work of initialization of
- * next_fsb to xfs_bmap_shift_extent as it has ilock held.
- */
- resblks = 0;
- next_fsb = NULLFSBLOCK;
- stop_fsb = XFS_B_TO_FSB(mp, offset);
- }
-
- shift_fsb = XFS_B_TO_FSB(mp, len);
/*
* Trim eofblocks to avoid shifting uninitialized post-eof preallocation
@@ -1322,8 +1281,7 @@ xfs_shift_file_space(
* Writeback and invalidate cache for the remainder of the file as we're
* about to shift down every extent from offset to EOF.
*/
- error = filemap_write_and_wait_range(VFS_I(ip)->i_mapping,
- offset, -1);
+ error = filemap_write_and_wait_range(VFS_I(ip)->i_mapping, offset, -1);
if (error)
return error;
error = invalidate_inode_pages2_range(VFS_I(ip)->i_mapping,
@@ -1343,16 +1301,48 @@ xfs_shift_file_space(
return error;
}
- /*
- * The extent shifting code works on extent granularity. So, if
- * stop_fsb is not the starting block of extent, we need to split
- * the extent at stop_fsb.
- */
- if (direction == SHIFT_RIGHT) {
- error = xfs_bmap_split_extent(ip, stop_fsb);
- if (error)
- return error;
- }
+ return 0;
+}
+
+/*
+ * xfs_collapse_file_space()
+ * This routine frees disk space and shift extent for the given file.
+ * The first thing we do is to free data blocks in the specified range
+ * by calling xfs_free_file_space(). It would also sync dirty data
+ * and invalidate page cache over the region on which collapse range
+ * is working. And Shift extent records to the left to cover a hole.
+ * RETURNS:
+ * 0 on success
+ * errno on error
+ *
+ */
+int
+xfs_collapse_file_space(
+ struct xfs_inode *ip,
+ xfs_off_t offset,
+ xfs_off_t len)
+{
+ int done = 0;
+ struct xfs_mount *mp = ip->i_mount;
+ struct xfs_trans *tp;
+ int error;
+ struct xfs_defer_ops dfops;
+ xfs_fsblock_t first_block;
+ xfs_fileoff_t stop_fsb = XFS_B_TO_FSB(mp, VFS_I(ip)->i_size);
+ xfs_fileoff_t next_fsb = XFS_B_TO_FSB(mp, offset + len);
+ xfs_fileoff_t shift_fsb = XFS_B_TO_FSB(mp, len);
+ uint resblks = XFS_DIOSTRAT_SPACE_RES(mp, 0);
+
+ ASSERT(xfs_isilocked(ip, XFS_IOLOCK_EXCL));
+ trace_xfs_collapse_file_space(ip);
+
+ error = xfs_free_file_space(ip, offset, len);
+ if (error)
+ return error;
+
+ error = xfs_prepare_shift(ip, offset);
+ if (error)
+ return error;
while (!error && !done) {
error = xfs_trans_alloc(mp, &M_RES(mp)->tr_write, resblks, 0, 0,
@@ -1366,7 +1356,6 @@ xfs_shift_file_space(
XFS_QMOPT_RES_REGBLKS);
if (error)
goto out_trans_cancel;
-
xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL);
xfs_defer_init(&dfops, &first_block);
@@ -1377,14 +1366,13 @@ xfs_shift_file_space(
*/
error = xfs_bmap_shift_extents(tp, ip, &next_fsb, shift_fsb,
&done, stop_fsb, &first_block, &dfops,
- direction, XFS_BMAP_MAX_SHIFT_EXTENTS);
+ SHIFT_LEFT, XFS_BMAP_MAX_SHIFT_EXTENTS);
if (error)
goto out_bmap_cancel;
error = xfs_defer_finish(&tp, &dfops);
if (error)
goto out_bmap_cancel;
-
error = xfs_trans_commit(tp);
}
@@ -1397,36 +1385,6 @@ xfs_shift_file_space(
return error;
}
-/*
- * xfs_collapse_file_space()
- * This routine frees disk space and shift extent for the given file.
- * The first thing we do is to free data blocks in the specified range
- * by calling xfs_free_file_space(). It would also sync dirty data
- * and invalidate page cache over the region on which collapse range
- * is working. And Shift extent records to the left to cover a hole.
- * RETURNS:
- * 0 on success
- * errno on error
- *
- */
-int
-xfs_collapse_file_space(
- struct xfs_inode *ip,
- xfs_off_t offset,
- xfs_off_t len)
-{
- int error;
-
- ASSERT(xfs_isilocked(ip, XFS_IOLOCK_EXCL));
- trace_xfs_collapse_file_space(ip);
-
- error = xfs_free_file_space(ip, offset, len);
- if (error)
- return error;
-
- return xfs_shift_file_space(ip, offset, len, SHIFT_LEFT);
-}
-
/*
* xfs_insert_file_space()
* This routine create hole space by shifting extents for the given file.
@@ -1445,10 +1403,64 @@ xfs_insert_file_space(
loff_t offset,
loff_t len)
{
+ struct xfs_mount *mp = ip->i_mount;
+ struct xfs_trans *tp;
+ int error;
+ struct xfs_defer_ops dfops;
+ xfs_fsblock_t first_block;
+ xfs_fileoff_t stop_fsb = XFS_B_TO_FSB(mp, offset);
+ xfs_fileoff_t next_fsb = NULLFSBLOCK;
+ xfs_fileoff_t shift_fsb = XFS_B_TO_FSB(mp, len);
+ int done = 0;
+
ASSERT(xfs_isilocked(ip, XFS_IOLOCK_EXCL));
trace_xfs_insert_file_space(ip);
- return xfs_shift_file_space(ip, offset, len, SHIFT_RIGHT);
+ error = xfs_prepare_shift(ip, offset);
+ if (error)
+ return error;
+
+ /*
+ * The extent shifting code works on extent granularity. So, if stop_fsb
+ * is not the starting block of extent, we need to split the extent at
+ * stop_fsb.
+ */
+ error = xfs_bmap_split_extent(ip, stop_fsb);
+ if (error)
+ return error;
+
+ while (!error && !done) {
+ error = xfs_trans_alloc(mp, &M_RES(mp)->tr_write, 0, 0, 0,
+ &tp);
+ if (error)
+ break;
+
+ xfs_ilock(ip, XFS_ILOCK_EXCL);
+ xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL);
+ xfs_defer_init(&dfops, &first_block);
+
+ /*
+ * We are using the write transaction in which max 2 bmbt
+ * updates are allowed
+ */
+ error = xfs_bmap_shift_extents(tp, ip, &next_fsb, shift_fsb,
+ &done, stop_fsb, &first_block, &dfops,
+ SHIFT_RIGHT, XFS_BMAP_MAX_SHIFT_EXTENTS);
+ if (error)
+ goto out_bmap_cancel;
+
+ error = xfs_defer_finish(&tp, &dfops);
+ if (error)
+ goto out_bmap_cancel;
+ error = xfs_trans_commit(tp);
+ }
+
+ return error;
+
+out_bmap_cancel:
+ xfs_defer_cancel(&dfops);
+ xfs_trans_cancel(tp);
+ return error;
}
/*
--
2.14.1
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [PATCH 08/15] xfs: inline xfs_shift_file_space into callers
2017-10-19 6:59 ` [PATCH 08/15] xfs: inline xfs_shift_file_space into callers Christoph Hellwig
@ 2017-10-21 0:07 ` Darrick J. Wong
2017-10-21 8:13 ` Christoph Hellwig
0 siblings, 1 reply; 36+ messages in thread
From: Darrick J. Wong @ 2017-10-21 0:07 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs
On Thu, Oct 19, 2017 at 08:59:35AM +0200, Christoph Hellwig wrote:
> The code is sufficiently different for the insert vs collapse cases both
> in xfs_shift_file_space itself and the callers that untangling them will
> make life a lot easier down the road.
>
> We still keep a common helper for flushing all data and COW state to get
> the inode into the right shape for shifting the extents around.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> fs/xfs/xfs_bmap_util.c | 192 ++++++++++++++++++++++++++-----------------------
> 1 file changed, 102 insertions(+), 90 deletions(-)
>
> diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
> index 0543423651ff..47b53c88de7c 100644
> --- a/fs/xfs/xfs_bmap_util.c
> +++ b/fs/xfs/xfs_bmap_util.c
> @@ -1260,53 +1260,12 @@ xfs_zero_file_space(
>
> }
>
> -/*
> - * @next_fsb will keep track of the extent currently undergoing shift.
> - * @stop_fsb will keep track of the extent at which we have to stop.
> - * If we are shifting left, we will start with block (offset + len) and
> - * shift each extent till last extent.
> - * If we are shifting right, we will start with last extent inside file space
> - * and continue until we reach the block corresponding to offset.
> - */
> static int
> -xfs_shift_file_space(
> - struct xfs_inode *ip,
> - xfs_off_t offset,
> - xfs_off_t len,
> - enum shift_direction direction)
> +xfs_prepare_shift(
> + struct xfs_inode *ip,
> + loff_t offset)
> {
> - int done = 0;
> - struct xfs_mount *mp = ip->i_mount;
> - struct xfs_trans *tp;
> int error;
> - struct xfs_defer_ops dfops;
> - xfs_fsblock_t first_block;
> - xfs_fileoff_t stop_fsb;
> - xfs_fileoff_t next_fsb;
> - xfs_fileoff_t shift_fsb;
> - uint resblks;
> -
> - ASSERT(direction == SHIFT_LEFT || direction == SHIFT_RIGHT);
> -
> - if (direction == SHIFT_LEFT) {
> - /*
> - * Reserve blocks to cover potential extent merges after left
> - * shift operations.
> - */
> - resblks = XFS_DIOSTRAT_SPACE_RES(mp, 0);
> - next_fsb = XFS_B_TO_FSB(mp, offset + len);
> - stop_fsb = XFS_B_TO_FSB(mp, VFS_I(ip)->i_size);
> - } else {
> - /*
> - * If right shift, delegate the work of initialization of
> - * next_fsb to xfs_bmap_shift_extent as it has ilock held.
> - */
> - resblks = 0;
> - next_fsb = NULLFSBLOCK;
> - stop_fsb = XFS_B_TO_FSB(mp, offset);
> - }
> -
> - shift_fsb = XFS_B_TO_FSB(mp, len);
>
> /*
> * Trim eofblocks to avoid shifting uninitialized post-eof preallocation
> @@ -1322,8 +1281,7 @@ xfs_shift_file_space(
> * Writeback and invalidate cache for the remainder of the file as we're
> * about to shift down every extent from offset to EOF.
> */
> - error = filemap_write_and_wait_range(VFS_I(ip)->i_mapping,
> - offset, -1);
> + error = filemap_write_and_wait_range(VFS_I(ip)->i_mapping, offset, -1);
> if (error)
> return error;
> error = invalidate_inode_pages2_range(VFS_I(ip)->i_mapping,
> @@ -1343,16 +1301,48 @@ xfs_shift_file_space(
> return error;
> }
>
> - /*
> - * The extent shifting code works on extent granularity. So, if
> - * stop_fsb is not the starting block of extent, we need to split
> - * the extent at stop_fsb.
> - */
> - if (direction == SHIFT_RIGHT) {
> - error = xfs_bmap_split_extent(ip, stop_fsb);
> - if (error)
> - return error;
> - }
> + return 0;
> +}
> +
> +/*
> + * xfs_collapse_file_space()
> + * This routine frees disk space and shift extent for the given file.
> + * The first thing we do is to free data blocks in the specified range
> + * by calling xfs_free_file_space(). It would also sync dirty data
> + * and invalidate page cache over the region on which collapse range
> + * is working. And Shift extent records to the left to cover a hole.
> + * RETURNS:
> + * 0 on success
> + * errno on error
> + *
> + */
> +int
> +xfs_collapse_file_space(
> + struct xfs_inode *ip,
> + xfs_off_t offset,
> + xfs_off_t len)
> +{
> + int done = 0;
> + struct xfs_mount *mp = ip->i_mount;
> + struct xfs_trans *tp;
> + int error;
> + struct xfs_defer_ops dfops;
> + xfs_fsblock_t first_block;
> + xfs_fileoff_t stop_fsb = XFS_B_TO_FSB(mp, VFS_I(ip)->i_size);
> + xfs_fileoff_t next_fsb = XFS_B_TO_FSB(mp, offset + len);
> + xfs_fileoff_t shift_fsb = XFS_B_TO_FSB(mp, len);
> + uint resblks = XFS_DIOSTRAT_SPACE_RES(mp, 0);
> +
> + ASSERT(xfs_isilocked(ip, XFS_IOLOCK_EXCL));
So it took me a while of wondering "don't we have to have the
MMAPLOCK_EXCL too?" before realizing that yes, the caller actually does
grab that too. I wonder if it's worth checking here, since you're
asserting the lock status at all?
Aside from that,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
> + trace_xfs_collapse_file_space(ip);
> +
> + error = xfs_free_file_space(ip, offset, len);
> + if (error)
> + return error;
> +
> + error = xfs_prepare_shift(ip, offset);
> + if (error)
> + return error;
>
> while (!error && !done) {
> error = xfs_trans_alloc(mp, &M_RES(mp)->tr_write, resblks, 0, 0,
> @@ -1366,7 +1356,6 @@ xfs_shift_file_space(
> XFS_QMOPT_RES_REGBLKS);
> if (error)
> goto out_trans_cancel;
> -
> xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL);
>
> xfs_defer_init(&dfops, &first_block);
> @@ -1377,14 +1366,13 @@ xfs_shift_file_space(
> */
> error = xfs_bmap_shift_extents(tp, ip, &next_fsb, shift_fsb,
> &done, stop_fsb, &first_block, &dfops,
> - direction, XFS_BMAP_MAX_SHIFT_EXTENTS);
> + SHIFT_LEFT, XFS_BMAP_MAX_SHIFT_EXTENTS);
> if (error)
> goto out_bmap_cancel;
>
> error = xfs_defer_finish(&tp, &dfops);
> if (error)
> goto out_bmap_cancel;
> -
> error = xfs_trans_commit(tp);
> }
>
> @@ -1397,36 +1385,6 @@ xfs_shift_file_space(
> return error;
> }
>
> -/*
> - * xfs_collapse_file_space()
> - * This routine frees disk space and shift extent for the given file.
> - * The first thing we do is to free data blocks in the specified range
> - * by calling xfs_free_file_space(). It would also sync dirty data
> - * and invalidate page cache over the region on which collapse range
> - * is working. And Shift extent records to the left to cover a hole.
> - * RETURNS:
> - * 0 on success
> - * errno on error
> - *
> - */
> -int
> -xfs_collapse_file_space(
> - struct xfs_inode *ip,
> - xfs_off_t offset,
> - xfs_off_t len)
> -{
> - int error;
> -
> - ASSERT(xfs_isilocked(ip, XFS_IOLOCK_EXCL));
> - trace_xfs_collapse_file_space(ip);
> -
> - error = xfs_free_file_space(ip, offset, len);
> - if (error)
> - return error;
> -
> - return xfs_shift_file_space(ip, offset, len, SHIFT_LEFT);
> -}
> -
> /*
> * xfs_insert_file_space()
> * This routine create hole space by shifting extents for the given file.
> @@ -1445,10 +1403,64 @@ xfs_insert_file_space(
> loff_t offset,
> loff_t len)
> {
> + struct xfs_mount *mp = ip->i_mount;
> + struct xfs_trans *tp;
> + int error;
> + struct xfs_defer_ops dfops;
> + xfs_fsblock_t first_block;
> + xfs_fileoff_t stop_fsb = XFS_B_TO_FSB(mp, offset);
> + xfs_fileoff_t next_fsb = NULLFSBLOCK;
> + xfs_fileoff_t shift_fsb = XFS_B_TO_FSB(mp, len);
> + int done = 0;
> +
> ASSERT(xfs_isilocked(ip, XFS_IOLOCK_EXCL));
> trace_xfs_insert_file_space(ip);
>
> - return xfs_shift_file_space(ip, offset, len, SHIFT_RIGHT);
> + error = xfs_prepare_shift(ip, offset);
> + if (error)
> + return error;
> +
> + /*
> + * The extent shifting code works on extent granularity. So, if stop_fsb
> + * is not the starting block of extent, we need to split the extent at
> + * stop_fsb.
> + */
> + error = xfs_bmap_split_extent(ip, stop_fsb);
> + if (error)
> + return error;
> +
> + while (!error && !done) {
> + error = xfs_trans_alloc(mp, &M_RES(mp)->tr_write, 0, 0, 0,
> + &tp);
> + if (error)
> + break;
> +
> + xfs_ilock(ip, XFS_ILOCK_EXCL);
> + xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL);
> + xfs_defer_init(&dfops, &first_block);
> +
> + /*
> + * We are using the write transaction in which max 2 bmbt
> + * updates are allowed
> + */
> + error = xfs_bmap_shift_extents(tp, ip, &next_fsb, shift_fsb,
> + &done, stop_fsb, &first_block, &dfops,
> + SHIFT_RIGHT, XFS_BMAP_MAX_SHIFT_EXTENTS);
> + if (error)
> + goto out_bmap_cancel;
> +
> + error = xfs_defer_finish(&tp, &dfops);
> + if (error)
> + goto out_bmap_cancel;
> + error = xfs_trans_commit(tp);
> + }
> +
> + return error;
> +
> +out_bmap_cancel:
> + xfs_defer_cancel(&dfops);
> + xfs_trans_cancel(tp);
> + return error;
> }
>
> /*
> --
> 2.14.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [PATCH 08/15] xfs: inline xfs_shift_file_space into callers
2017-10-21 0:07 ` Darrick J. Wong
@ 2017-10-21 8:13 ` Christoph Hellwig
2017-10-21 18:06 ` Darrick J. Wong
0 siblings, 1 reply; 36+ messages in thread
From: Christoph Hellwig @ 2017-10-21 8:13 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: Christoph Hellwig, linux-xfs
> > + ASSERT(xfs_isilocked(ip, XFS_IOLOCK_EXCL));
>
> So it took me a while of wondering "don't we have to have the
> MMAPLOCK_EXCL too?" before realizing that yes, the caller actually does
> grab that too. I wonder if it's worth checking here, since you're
> asserting the lock status at all?
This just moves the previous code around.. I can send an incremental
patch if you want, though.
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 08/15] xfs: inline xfs_shift_file_space into callers
2017-10-21 8:13 ` Christoph Hellwig
@ 2017-10-21 18:06 ` Darrick J. Wong
0 siblings, 0 replies; 36+ messages in thread
From: Darrick J. Wong @ 2017-10-21 18:06 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs
On Sat, Oct 21, 2017 at 10:13:14AM +0200, Christoph Hellwig wrote:
> > > + ASSERT(xfs_isilocked(ip, XFS_IOLOCK_EXCL));
> >
> > So it took me a while of wondering "don't we have to have the
> > MMAPLOCK_EXCL too?" before realizing that yes, the caller actually does
> > grab that too. I wonder if it's worth checking here, since you're
> > asserting the lock status at all?
>
> This just moves the previous code around.. I can send an incremental
> patch if you want, though.
Sure, either resending this patch or tacking a new one on the end is
fine, I don't mind either way.
--D
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH 09/15] xfs: remove XFS_BMAP_MAX_SHIFT_EXTENTS
2017-10-19 6:59 more extent mapping cleanups Christoph Hellwig
` (7 preceding siblings ...)
2017-10-19 6:59 ` [PATCH 08/15] xfs: inline xfs_shift_file_space into callers Christoph Hellwig
@ 2017-10-19 6:59 ` Christoph Hellwig
2017-10-21 0:10 ` Darrick J. Wong
2017-10-19 6:59 ` [PATCH 10/15] xfs: split xfs_bmap_shift_extents Christoph Hellwig
` (6 subsequent siblings)
15 siblings, 1 reply; 36+ messages in thread
From: Christoph Hellwig @ 2017-10-19 6:59 UTC (permalink / raw)
To: linux-xfs
The define was always set to 1, which means looping until we reach is
was dead code from the start.
Also remove an initialization of next_fsb for the done case that doesn't
fit the new code flow - it was never checked by the caller in the done
case to start with.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/xfs/libxfs/xfs_bmap.c | 47 ++++++++++++++++++++---------------------------
fs/xfs/libxfs/xfs_bmap.h | 12 +-----------
fs/xfs/xfs_bmap_util.c | 14 ++------------
3 files changed, 23 insertions(+), 50 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 22e7578e5696..89ea8a5235c0 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -5692,8 +5692,7 @@ xfs_bmse_shift_one(
/*
* Shift extent records to the left/right to cover/create a hole.
*
- * The maximum number of extents to be shifted in a single operation is
- * @num_exts. @stop_fsb specifies the file offset at which to stop shift and the
+ * @stop_fsb specifies the file offset at which to stop shift and the
* file offset where we've left off is returned in @next_fsb. @offset_shift_fsb
* is the length by which each extent is shifted. If there is no hole to shift
* the extents into, this will be considered invalid operation and we abort
@@ -5709,14 +5708,12 @@ xfs_bmap_shift_extents(
xfs_fileoff_t stop_fsb,
xfs_fsblock_t *firstblock,
struct xfs_defer_ops *dfops,
- enum shift_direction direction,
- int num_exts)
+ enum shift_direction direction)
{
struct xfs_btree_cur *cur = NULL;
struct xfs_bmbt_irec got;
struct xfs_mount *mp = ip->i_mount;
struct xfs_ifork *ifp;
- xfs_extnum_t nexts = 0;
xfs_extnum_t current_ext;
xfs_extnum_t total_extents;
xfs_extnum_t stop_extent;
@@ -5814,31 +5811,27 @@ xfs_bmap_shift_extents(
}
}
- while (nexts++ < num_exts) {
- error = xfs_bmse_shift_one(ip, whichfork, offset_shift_fsb,
- ¤t_ext, &got, cur, &logflags,
- direction, dfops);
- if (error)
- goto del_cursor;
- /*
- * If there was an extent merge during the shift, the extent
- * count can change. Update the total and grade the next record.
- */
- if (direction == SHIFT_LEFT) {
- total_extents = xfs_iext_count(ifp);
- stop_extent = total_extents;
- }
+ error = xfs_bmse_shift_one(ip, whichfork, offset_shift_fsb,
+ ¤t_ext, &got, cur, &logflags,
+ direction, dfops);
+ if (error)
+ goto del_cursor;
+ /*
+ * If there was an extent merge during the shift, the extent
+ * count can change. Update the total and grade the next record.
+ */
+ if (direction == SHIFT_LEFT) {
+ total_extents = xfs_iext_count(ifp);
+ stop_extent = total_extents;
+ }
- if (current_ext == stop_extent) {
- *done = 1;
- *next_fsb = NULLFSBLOCK;
- break;
- }
- xfs_iext_get_extent(ifp, current_ext, &got);
+ if (current_ext == stop_extent) {
+ *done = 1;
+ goto del_cursor;
}
+ xfs_iext_get_extent(ifp, current_ext, &got);
- if (!*done)
- *next_fsb = got.br_startoff;
+ *next_fsb = got.br_startoff;
del_cursor:
if (cur)
diff --git a/fs/xfs/libxfs/xfs_bmap.h b/fs/xfs/libxfs/xfs_bmap.h
index c837e88ba19a..985b1c26566a 100644
--- a/fs/xfs/libxfs/xfs_bmap.h
+++ b/fs/xfs/libxfs/xfs_bmap.h
@@ -183,15 +183,6 @@ static inline bool xfs_bmap_is_real_extent(struct xfs_bmbt_irec *irec)
!isnullstartblock(irec->br_startblock);
}
-/*
- * This macro is used to determine how many extents will be shifted
- * in one write transaction. We could require two splits,
- * an extent move on the first and an extent merge on the second,
- * So it is proper that one extent is shifted inside write transaction
- * at a time.
- */
-#define XFS_BMAP_MAX_SHIFT_EXTENTS 1
-
enum shift_direction {
SHIFT_LEFT = 0,
SHIFT_RIGHT,
@@ -239,8 +230,7 @@ uint xfs_default_attroffset(struct xfs_inode *ip);
int xfs_bmap_shift_extents(struct xfs_trans *tp, struct xfs_inode *ip,
xfs_fileoff_t *next_fsb, xfs_fileoff_t offset_shift_fsb,
int *done, xfs_fileoff_t stop_fsb, xfs_fsblock_t *firstblock,
- struct xfs_defer_ops *dfops, enum shift_direction direction,
- int num_exts);
+ struct xfs_defer_ops *dfops, enum shift_direction direction);
int xfs_bmap_split_extent(struct xfs_inode *ip, xfs_fileoff_t split_offset);
int xfs_bmapi_reserve_delalloc(struct xfs_inode *ip, int whichfork,
xfs_fileoff_t off, xfs_filblks_t len, xfs_filblks_t prealloc,
diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
index 47b53c88de7c..3273f083c496 100644
--- a/fs/xfs/xfs_bmap_util.c
+++ b/fs/xfs/xfs_bmap_util.c
@@ -1359,14 +1359,9 @@ xfs_collapse_file_space(
xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL);
xfs_defer_init(&dfops, &first_block);
-
- /*
- * We are using the write transaction in which max 2 bmbt
- * updates are allowed
- */
error = xfs_bmap_shift_extents(tp, ip, &next_fsb, shift_fsb,
&done, stop_fsb, &first_block, &dfops,
- SHIFT_LEFT, XFS_BMAP_MAX_SHIFT_EXTENTS);
+ SHIFT_LEFT);
if (error)
goto out_bmap_cancel;
@@ -1438,14 +1433,9 @@ xfs_insert_file_space(
xfs_ilock(ip, XFS_ILOCK_EXCL);
xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL);
xfs_defer_init(&dfops, &first_block);
-
- /*
- * We are using the write transaction in which max 2 bmbt
- * updates are allowed
- */
error = xfs_bmap_shift_extents(tp, ip, &next_fsb, shift_fsb,
&done, stop_fsb, &first_block, &dfops,
- SHIFT_RIGHT, XFS_BMAP_MAX_SHIFT_EXTENTS);
+ SHIFT_RIGHT);
if (error)
goto out_bmap_cancel;
--
2.14.1
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [PATCH 09/15] xfs: remove XFS_BMAP_MAX_SHIFT_EXTENTS
2017-10-19 6:59 ` [PATCH 09/15] xfs: remove XFS_BMAP_MAX_SHIFT_EXTENTS Christoph Hellwig
@ 2017-10-21 0:10 ` Darrick J. Wong
0 siblings, 0 replies; 36+ messages in thread
From: Darrick J. Wong @ 2017-10-21 0:10 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs
On Thu, Oct 19, 2017 at 08:59:36AM +0200, Christoph Hellwig wrote:
> The define was always set to 1, which means looping until we reach is
> was dead code from the start.
>
> Also remove an initialization of next_fsb for the done case that doesn't
> fit the new code flow - it was never checked by the caller in the done
> case to start with.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> fs/xfs/libxfs/xfs_bmap.c | 47 ++++++++++++++++++++---------------------------
> fs/xfs/libxfs/xfs_bmap.h | 12 +-----------
> fs/xfs/xfs_bmap_util.c | 14 ++------------
> 3 files changed, 23 insertions(+), 50 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index 22e7578e5696..89ea8a5235c0 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -5692,8 +5692,7 @@ xfs_bmse_shift_one(
> /*
> * Shift extent records to the left/right to cover/create a hole.
> *
> - * The maximum number of extents to be shifted in a single operation is
> - * @num_exts. @stop_fsb specifies the file offset at which to stop shift and the
> + * @stop_fsb specifies the file offset at which to stop shift and the
> * file offset where we've left off is returned in @next_fsb. @offset_shift_fsb
> * is the length by which each extent is shifted. If there is no hole to shift
> * the extents into, this will be considered invalid operation and we abort
> @@ -5709,14 +5708,12 @@ xfs_bmap_shift_extents(
> xfs_fileoff_t stop_fsb,
> xfs_fsblock_t *firstblock,
> struct xfs_defer_ops *dfops,
> - enum shift_direction direction,
> - int num_exts)
> + enum shift_direction direction)
> {
> struct xfs_btree_cur *cur = NULL;
> struct xfs_bmbt_irec got;
> struct xfs_mount *mp = ip->i_mount;
> struct xfs_ifork *ifp;
> - xfs_extnum_t nexts = 0;
> xfs_extnum_t current_ext;
> xfs_extnum_t total_extents;
> xfs_extnum_t stop_extent;
> @@ -5814,31 +5811,27 @@ xfs_bmap_shift_extents(
> }
> }
>
> - while (nexts++ < num_exts) {
> - error = xfs_bmse_shift_one(ip, whichfork, offset_shift_fsb,
> - ¤t_ext, &got, cur, &logflags,
> - direction, dfops);
> - if (error)
> - goto del_cursor;
> - /*
> - * If there was an extent merge during the shift, the extent
> - * count can change. Update the total and grade the next record.
> - */
> - if (direction == SHIFT_LEFT) {
> - total_extents = xfs_iext_count(ifp);
> - stop_extent = total_extents;
> - }
> + error = xfs_bmse_shift_one(ip, whichfork, offset_shift_fsb,
> + ¤t_ext, &got, cur, &logflags,
> + direction, dfops);
> + if (error)
> + goto del_cursor;
> + /*
> + * If there was an extent merge during the shift, the extent
> + * count can change. Update the total and grade the next record.
/me wonders what "grade the next record" means, but afaict this comment
disappears anyway...
--D
> + */
> + if (direction == SHIFT_LEFT) {
> + total_extents = xfs_iext_count(ifp);
> + stop_extent = total_extents;
> + }
>
> - if (current_ext == stop_extent) {
> - *done = 1;
> - *next_fsb = NULLFSBLOCK;
> - break;
> - }
> - xfs_iext_get_extent(ifp, current_ext, &got);
> + if (current_ext == stop_extent) {
> + *done = 1;
> + goto del_cursor;
> }
> + xfs_iext_get_extent(ifp, current_ext, &got);
>
> - if (!*done)
> - *next_fsb = got.br_startoff;
> + *next_fsb = got.br_startoff;
>
> del_cursor:
> if (cur)
> diff --git a/fs/xfs/libxfs/xfs_bmap.h b/fs/xfs/libxfs/xfs_bmap.h
> index c837e88ba19a..985b1c26566a 100644
> --- a/fs/xfs/libxfs/xfs_bmap.h
> +++ b/fs/xfs/libxfs/xfs_bmap.h
> @@ -183,15 +183,6 @@ static inline bool xfs_bmap_is_real_extent(struct xfs_bmbt_irec *irec)
> !isnullstartblock(irec->br_startblock);
> }
>
> -/*
> - * This macro is used to determine how many extents will be shifted
> - * in one write transaction. We could require two splits,
> - * an extent move on the first and an extent merge on the second,
> - * So it is proper that one extent is shifted inside write transaction
> - * at a time.
> - */
> -#define XFS_BMAP_MAX_SHIFT_EXTENTS 1
> -
> enum shift_direction {
> SHIFT_LEFT = 0,
> SHIFT_RIGHT,
> @@ -239,8 +230,7 @@ uint xfs_default_attroffset(struct xfs_inode *ip);
> int xfs_bmap_shift_extents(struct xfs_trans *tp, struct xfs_inode *ip,
> xfs_fileoff_t *next_fsb, xfs_fileoff_t offset_shift_fsb,
> int *done, xfs_fileoff_t stop_fsb, xfs_fsblock_t *firstblock,
> - struct xfs_defer_ops *dfops, enum shift_direction direction,
> - int num_exts);
> + struct xfs_defer_ops *dfops, enum shift_direction direction);
> int xfs_bmap_split_extent(struct xfs_inode *ip, xfs_fileoff_t split_offset);
> int xfs_bmapi_reserve_delalloc(struct xfs_inode *ip, int whichfork,
> xfs_fileoff_t off, xfs_filblks_t len, xfs_filblks_t prealloc,
> diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
> index 47b53c88de7c..3273f083c496 100644
> --- a/fs/xfs/xfs_bmap_util.c
> +++ b/fs/xfs/xfs_bmap_util.c
> @@ -1359,14 +1359,9 @@ xfs_collapse_file_space(
> xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL);
>
> xfs_defer_init(&dfops, &first_block);
> -
> - /*
> - * We are using the write transaction in which max 2 bmbt
> - * updates are allowed
> - */
> error = xfs_bmap_shift_extents(tp, ip, &next_fsb, shift_fsb,
> &done, stop_fsb, &first_block, &dfops,
> - SHIFT_LEFT, XFS_BMAP_MAX_SHIFT_EXTENTS);
> + SHIFT_LEFT);
> if (error)
> goto out_bmap_cancel;
>
> @@ -1438,14 +1433,9 @@ xfs_insert_file_space(
> xfs_ilock(ip, XFS_ILOCK_EXCL);
> xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL);
> xfs_defer_init(&dfops, &first_block);
> -
> - /*
> - * We are using the write transaction in which max 2 bmbt
> - * updates are allowed
> - */
> error = xfs_bmap_shift_extents(tp, ip, &next_fsb, shift_fsb,
> &done, stop_fsb, &first_block, &dfops,
> - SHIFT_RIGHT, XFS_BMAP_MAX_SHIFT_EXTENTS);
> + SHIFT_RIGHT);
> if (error)
> goto out_bmap_cancel;
>
> --
> 2.14.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH 10/15] xfs: split xfs_bmap_shift_extents
2017-10-19 6:59 more extent mapping cleanups Christoph Hellwig
` (8 preceding siblings ...)
2017-10-19 6:59 ` [PATCH 09/15] xfs: remove XFS_BMAP_MAX_SHIFT_EXTENTS Christoph Hellwig
@ 2017-10-19 6:59 ` Christoph Hellwig
2017-10-21 0:22 ` Darrick J. Wong
2017-10-19 6:59 ` [PATCH 11/15] xfs: remove xfs_bmse_shift_one Christoph Hellwig
` (5 subsequent siblings)
15 siblings, 1 reply; 36+ messages in thread
From: Christoph Hellwig @ 2017-10-19 6:59 UTC (permalink / raw)
To: linux-xfs
Have a separate helper for insert vs collapse, as this prepares us for
simplifying the code in the next patches.
Also changed the done output argument to a bool intead of int for both
new functions.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/xfs/libxfs/xfs_bmap.c | 197 ++++++++++++++++++++++++++++++++---------------
fs/xfs/libxfs/xfs_bmap.h | 10 ++-
fs/xfs/xfs_bmap_util.c | 14 ++--
3 files changed, 148 insertions(+), 73 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 89ea8a5235c0..7d3a38e69d28 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -5689,57 +5689,151 @@ xfs_bmse_shift_one(
return xfs_rmap_map_extent(mp, dfops, ip, whichfork, &new);
}
-/*
- * Shift extent records to the left/right to cover/create a hole.
- *
- * @stop_fsb specifies the file offset at which to stop shift and the
- * file offset where we've left off is returned in @next_fsb. @offset_shift_fsb
- * is the length by which each extent is shifted. If there is no hole to shift
- * the extents into, this will be considered invalid operation and we abort
- * immediately.
- */
int
-xfs_bmap_shift_extents(
+xfs_bmap_collapse_extents(
struct xfs_trans *tp,
struct xfs_inode *ip,
xfs_fileoff_t *next_fsb,
xfs_fileoff_t offset_shift_fsb,
- int *done,
+ bool *done,
xfs_fileoff_t stop_fsb,
xfs_fsblock_t *firstblock,
- struct xfs_defer_ops *dfops,
- enum shift_direction direction)
+ struct xfs_defer_ops *dfops)
{
- struct xfs_btree_cur *cur = NULL;
- struct xfs_bmbt_irec got;
- struct xfs_mount *mp = ip->i_mount;
- struct xfs_ifork *ifp;
- xfs_extnum_t current_ext;
- xfs_extnum_t total_extents;
- xfs_extnum_t stop_extent;
- int error = 0;
- int whichfork = XFS_DATA_FORK;
- int logflags = 0;
+ int whichfork = XFS_DATA_FORK;
+ struct xfs_mount *mp = ip->i_mount;
+ struct xfs_ifork *ifp = XFS_IFORK_PTR(ip, whichfork);
+ struct xfs_btree_cur *cur = NULL;
+ struct xfs_bmbt_irec got;
+ xfs_extnum_t current_ext;
+ xfs_extnum_t total_extents;
+ xfs_extnum_t stop_extent;
+ int error = 0;
+ int logflags = 0;
if (unlikely(XFS_TEST_ERROR(
(XFS_IFORK_FORMAT(ip, whichfork) != XFS_DINODE_FMT_EXTENTS &&
XFS_IFORK_FORMAT(ip, whichfork) != XFS_DINODE_FMT_BTREE),
mp, XFS_ERRTAG_BMAPIFORMAT))) {
- XFS_ERROR_REPORT("xfs_bmap_shift_extents",
- XFS_ERRLEVEL_LOW, mp);
+ XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW, mp);
return -EFSCORRUPTED;
}
if (XFS_FORCED_SHUTDOWN(mp))
return -EIO;
- ASSERT(xfs_isilocked(ip, XFS_IOLOCK_EXCL));
- ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
- ASSERT(direction == SHIFT_LEFT || direction == SHIFT_RIGHT);
+ ASSERT(xfs_isilocked(ip, XFS_IOLOCK_EXCL | XFS_ILOCK_EXCL));
+
+ if (!(ifp->if_flags & XFS_IFEXTENTS)) {
+ error = xfs_iread_extents(tp, ip, whichfork);
+ if (error)
+ return error;
+ }
+
+ if (ifp->if_flags & XFS_IFBROOT) {
+ cur = xfs_bmbt_init_cursor(mp, tp, ip, whichfork);
+ cur->bc_private.b.firstblock = *firstblock;
+ cur->bc_private.b.dfops = dfops;
+ cur->bc_private.b.flags = 0;
+ }
+
+ /*
+ * There may be delalloc extents in the data fork before the range we
+ * are collapsing out, so we cannot use the count of real extents here.
+ * Instead we have to calculate it from the incore fork.
+ */
+ total_extents = xfs_iext_count(ifp);
+ if (total_extents == 0) {
+ *done = true;
+ goto del_cursor;
+ }
+
+ /*
+ * Look up the extent index for the fsb where we start shifting. We can
+ * henceforth iterate with current_ext as extent list changes are locked
+ * out via ilock.
+ *
+ * If next_fsb lies in a hole beyond which there are no extents we are
+ * done.
+ */
+ if (!xfs_iext_lookup_extent(ip, ifp, *next_fsb, ¤t_ext, &got)) {
+ *done = true;
+ goto del_cursor;
+ }
+
+ stop_extent = total_extents;
+ if (current_ext >= stop_extent) {
+ error = -EIO;
+ goto del_cursor;
+ }
+
+ error = xfs_bmse_shift_one(ip, whichfork, offset_shift_fsb,
+ ¤t_ext, &got, cur, &logflags,
+ SHIFT_LEFT, dfops);
+ if (error)
+ goto del_cursor;
+ /*
+ * If there was an extent merge during the shift, the extent
+ * count can change. Update the total and grade the next record.
+ */
+ total_extents = xfs_iext_count(ifp);
+ stop_extent = total_extents;
+ if (current_ext == stop_extent) {
+ *done = true;
+ goto del_cursor;
+ }
+ xfs_iext_get_extent(ifp, current_ext, &got);
+
+ if (!*done)
+ *next_fsb = got.br_startoff;
+
+del_cursor:
+ if (cur)
+ xfs_btree_del_cursor(cur,
+ error ? XFS_BTREE_ERROR : XFS_BTREE_NOERROR);
+
+ if (logflags)
+ xfs_trans_log_inode(tp, ip, logflags);
+
+ return error;
+}
+
+int
+xfs_bmap_insert_extents(
+ struct xfs_trans *tp,
+ struct xfs_inode *ip,
+ xfs_fileoff_t *next_fsb,
+ xfs_fileoff_t offset_shift_fsb,
+ bool *done,
+ xfs_fileoff_t stop_fsb,
+ xfs_fsblock_t *firstblock,
+ struct xfs_defer_ops *dfops)
+{
+ int whichfork = XFS_DATA_FORK;
+ struct xfs_mount *mp = ip->i_mount;
+ struct xfs_ifork *ifp = XFS_IFORK_PTR(ip, whichfork);
+ struct xfs_btree_cur *cur = NULL;
+ struct xfs_bmbt_irec got, s;
+ xfs_extnum_t current_ext;
+ xfs_extnum_t total_extents;
+ xfs_extnum_t stop_extent;
+ int error = 0;
+ int logflags = 0;
+
+ if (unlikely(XFS_TEST_ERROR(
+ (XFS_IFORK_FORMAT(ip, whichfork) != XFS_DINODE_FMT_EXTENTS &&
+ XFS_IFORK_FORMAT(ip, whichfork) != XFS_DINODE_FMT_BTREE),
+ mp, XFS_ERRTAG_BMAPIFORMAT))) {
+ XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW, mp);
+ return -EFSCORRUPTED;
+ }
+
+ if (XFS_FORCED_SHUTDOWN(mp))
+ return -EIO;
+
+ ASSERT(xfs_isilocked(ip, XFS_IOLOCK_EXCL | XFS_ILOCK_EXCL));
- ifp = XFS_IFORK_PTR(ip, whichfork);
if (!(ifp->if_flags & XFS_IFEXTENTS)) {
- /* Read in all the extents */
error = xfs_iread_extents(tp, ip, whichfork);
if (error)
return error;
@@ -5759,7 +5853,7 @@ xfs_bmap_shift_extents(
*/
total_extents = xfs_iext_count(ifp);
if (total_extents == 0) {
- *done = 1;
+ *done = true;
goto del_cursor;
}
@@ -5767,12 +5861,10 @@ xfs_bmap_shift_extents(
* In case of first right shift, we need to initialize next_fsb
*/
if (*next_fsb == NULLFSBLOCK) {
- ASSERT(direction == SHIFT_RIGHT);
-
current_ext = total_extents - 1;
xfs_iext_get_extent(ifp, current_ext, &got);
if (stop_fsb > got.br_startoff) {
- *done = 1;
+ *done = true;
goto del_cursor;
}
*next_fsb = got.br_startoff;
@@ -5787,46 +5879,27 @@ xfs_bmap_shift_extents(
*/
if (!xfs_iext_lookup_extent(ip, ifp, *next_fsb, ¤t_ext,
&got)) {
- *done = 1;
+ *done = true;
goto del_cursor;
}
}
/* Lookup the extent index at which we have to stop */
- if (direction == SHIFT_RIGHT) {
- struct xfs_bmbt_irec s;
-
- xfs_iext_lookup_extent(ip, ifp, stop_fsb, &stop_extent, &s);
- /* Make stop_extent exclusive of shift range */
- stop_extent--;
- if (current_ext <= stop_extent) {
- error = -EIO;
- goto del_cursor;
- }
- } else {
- stop_extent = total_extents;
- if (current_ext >= stop_extent) {
- error = -EIO;
- goto del_cursor;
- }
+ xfs_iext_lookup_extent(ip, ifp, stop_fsb, &stop_extent, &s);
+ /* Make stop_extent exclusive of shift range */
+ stop_extent--;
+ if (current_ext <= stop_extent) {
+ error = -EIO;
+ goto del_cursor;
}
error = xfs_bmse_shift_one(ip, whichfork, offset_shift_fsb,
¤t_ext, &got, cur, &logflags,
- direction, dfops);
+ SHIFT_RIGHT, dfops);
if (error)
goto del_cursor;
- /*
- * If there was an extent merge during the shift, the extent
- * count can change. Update the total and grade the next record.
- */
- if (direction == SHIFT_LEFT) {
- total_extents = xfs_iext_count(ifp);
- stop_extent = total_extents;
- }
-
if (current_ext == stop_extent) {
- *done = 1;
+ *done = true;
goto del_cursor;
}
xfs_iext_get_extent(ifp, current_ext, &got);
diff --git a/fs/xfs/libxfs/xfs_bmap.h b/fs/xfs/libxfs/xfs_bmap.h
index 985b1c26566a..f7ccf2de1a8c 100644
--- a/fs/xfs/libxfs/xfs_bmap.h
+++ b/fs/xfs/libxfs/xfs_bmap.h
@@ -227,10 +227,14 @@ int xfs_bmap_del_extent_delay(struct xfs_inode *ip, int whichfork,
void xfs_bmap_del_extent_cow(struct xfs_inode *ip, xfs_extnum_t *idx,
struct xfs_bmbt_irec *got, struct xfs_bmbt_irec *del);
uint xfs_default_attroffset(struct xfs_inode *ip);
-int xfs_bmap_shift_extents(struct xfs_trans *tp, struct xfs_inode *ip,
+int xfs_bmap_collapse_extents(struct xfs_trans *tp, struct xfs_inode *ip,
xfs_fileoff_t *next_fsb, xfs_fileoff_t offset_shift_fsb,
- int *done, xfs_fileoff_t stop_fsb, xfs_fsblock_t *firstblock,
- struct xfs_defer_ops *dfops, enum shift_direction direction);
+ bool *done, xfs_fileoff_t stop_fsb, xfs_fsblock_t *firstblock,
+ struct xfs_defer_ops *dfops);
+int xfs_bmap_insert_extents(struct xfs_trans *tp, struct xfs_inode *ip,
+ xfs_fileoff_t *next_fsb, xfs_fileoff_t offset_shift_fsb,
+ bool *done, xfs_fileoff_t stop_fsb, xfs_fsblock_t *firstblock,
+ struct xfs_defer_ops *dfops);
int xfs_bmap_split_extent(struct xfs_inode *ip, xfs_fileoff_t split_offset);
int xfs_bmapi_reserve_delalloc(struct xfs_inode *ip, int whichfork,
xfs_fileoff_t off, xfs_filblks_t len, xfs_filblks_t prealloc,
diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
index 3273f083c496..034f3429ca8c 100644
--- a/fs/xfs/xfs_bmap_util.c
+++ b/fs/xfs/xfs_bmap_util.c
@@ -1322,7 +1322,6 @@ xfs_collapse_file_space(
xfs_off_t offset,
xfs_off_t len)
{
- int done = 0;
struct xfs_mount *mp = ip->i_mount;
struct xfs_trans *tp;
int error;
@@ -1332,6 +1331,7 @@ xfs_collapse_file_space(
xfs_fileoff_t next_fsb = XFS_B_TO_FSB(mp, offset + len);
xfs_fileoff_t shift_fsb = XFS_B_TO_FSB(mp, len);
uint resblks = XFS_DIOSTRAT_SPACE_RES(mp, 0);
+ bool done = false;
ASSERT(xfs_isilocked(ip, XFS_IOLOCK_EXCL));
trace_xfs_collapse_file_space(ip);
@@ -1359,9 +1359,8 @@ xfs_collapse_file_space(
xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL);
xfs_defer_init(&dfops, &first_block);
- error = xfs_bmap_shift_extents(tp, ip, &next_fsb, shift_fsb,
- &done, stop_fsb, &first_block, &dfops,
- SHIFT_LEFT);
+ error = xfs_bmap_collapse_extents(tp, ip, &next_fsb, shift_fsb,
+ &done, stop_fsb, &first_block, &dfops);
if (error)
goto out_bmap_cancel;
@@ -1406,7 +1405,7 @@ xfs_insert_file_space(
xfs_fileoff_t stop_fsb = XFS_B_TO_FSB(mp, offset);
xfs_fileoff_t next_fsb = NULLFSBLOCK;
xfs_fileoff_t shift_fsb = XFS_B_TO_FSB(mp, len);
- int done = 0;
+ bool done = false;
ASSERT(xfs_isilocked(ip, XFS_IOLOCK_EXCL));
trace_xfs_insert_file_space(ip);
@@ -1433,9 +1432,8 @@ xfs_insert_file_space(
xfs_ilock(ip, XFS_ILOCK_EXCL);
xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL);
xfs_defer_init(&dfops, &first_block);
- error = xfs_bmap_shift_extents(tp, ip, &next_fsb, shift_fsb,
- &done, stop_fsb, &first_block, &dfops,
- SHIFT_RIGHT);
+ error = xfs_bmap_insert_extents(tp, ip, &next_fsb, shift_fsb,
+ &done, stop_fsb, &first_block, &dfops);
if (error)
goto out_bmap_cancel;
--
2.14.1
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [PATCH 10/15] xfs: split xfs_bmap_shift_extents
2017-10-19 6:59 ` [PATCH 10/15] xfs: split xfs_bmap_shift_extents Christoph Hellwig
@ 2017-10-21 0:22 ` Darrick J. Wong
0 siblings, 0 replies; 36+ messages in thread
From: Darrick J. Wong @ 2017-10-21 0:22 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs
On Thu, Oct 19, 2017 at 08:59:37AM +0200, Christoph Hellwig wrote:
> Have a separate helper for insert vs collapse, as this prepares us for
> simplifying the code in the next patches.
>
> Also changed the done output argument to a bool intead of int for both
> new functions.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
Looks ok,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
> fs/xfs/libxfs/xfs_bmap.c | 197 ++++++++++++++++++++++++++++++++---------------
> fs/xfs/libxfs/xfs_bmap.h | 10 ++-
> fs/xfs/xfs_bmap_util.c | 14 ++--
> 3 files changed, 148 insertions(+), 73 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index 89ea8a5235c0..7d3a38e69d28 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -5689,57 +5689,151 @@ xfs_bmse_shift_one(
> return xfs_rmap_map_extent(mp, dfops, ip, whichfork, &new);
> }
>
> -/*
> - * Shift extent records to the left/right to cover/create a hole.
> - *
> - * @stop_fsb specifies the file offset at which to stop shift and the
> - * file offset where we've left off is returned in @next_fsb. @offset_shift_fsb
> - * is the length by which each extent is shifted. If there is no hole to shift
> - * the extents into, this will be considered invalid operation and we abort
> - * immediately.
> - */
> int
> -xfs_bmap_shift_extents(
> +xfs_bmap_collapse_extents(
> struct xfs_trans *tp,
> struct xfs_inode *ip,
> xfs_fileoff_t *next_fsb,
> xfs_fileoff_t offset_shift_fsb,
> - int *done,
> + bool *done,
> xfs_fileoff_t stop_fsb,
> xfs_fsblock_t *firstblock,
> - struct xfs_defer_ops *dfops,
> - enum shift_direction direction)
> + struct xfs_defer_ops *dfops)
> {
> - struct xfs_btree_cur *cur = NULL;
> - struct xfs_bmbt_irec got;
> - struct xfs_mount *mp = ip->i_mount;
> - struct xfs_ifork *ifp;
> - xfs_extnum_t current_ext;
> - xfs_extnum_t total_extents;
> - xfs_extnum_t stop_extent;
> - int error = 0;
> - int whichfork = XFS_DATA_FORK;
> - int logflags = 0;
> + int whichfork = XFS_DATA_FORK;
> + struct xfs_mount *mp = ip->i_mount;
> + struct xfs_ifork *ifp = XFS_IFORK_PTR(ip, whichfork);
> + struct xfs_btree_cur *cur = NULL;
> + struct xfs_bmbt_irec got;
> + xfs_extnum_t current_ext;
> + xfs_extnum_t total_extents;
> + xfs_extnum_t stop_extent;
> + int error = 0;
> + int logflags = 0;
>
> if (unlikely(XFS_TEST_ERROR(
> (XFS_IFORK_FORMAT(ip, whichfork) != XFS_DINODE_FMT_EXTENTS &&
> XFS_IFORK_FORMAT(ip, whichfork) != XFS_DINODE_FMT_BTREE),
> mp, XFS_ERRTAG_BMAPIFORMAT))) {
> - XFS_ERROR_REPORT("xfs_bmap_shift_extents",
> - XFS_ERRLEVEL_LOW, mp);
> + XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW, mp);
> return -EFSCORRUPTED;
> }
>
> if (XFS_FORCED_SHUTDOWN(mp))
> return -EIO;
>
> - ASSERT(xfs_isilocked(ip, XFS_IOLOCK_EXCL));
> - ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
> - ASSERT(direction == SHIFT_LEFT || direction == SHIFT_RIGHT);
> + ASSERT(xfs_isilocked(ip, XFS_IOLOCK_EXCL | XFS_ILOCK_EXCL));
> +
> + if (!(ifp->if_flags & XFS_IFEXTENTS)) {
> + error = xfs_iread_extents(tp, ip, whichfork);
> + if (error)
> + return error;
> + }
> +
> + if (ifp->if_flags & XFS_IFBROOT) {
> + cur = xfs_bmbt_init_cursor(mp, tp, ip, whichfork);
> + cur->bc_private.b.firstblock = *firstblock;
> + cur->bc_private.b.dfops = dfops;
> + cur->bc_private.b.flags = 0;
> + }
> +
> + /*
> + * There may be delalloc extents in the data fork before the range we
> + * are collapsing out, so we cannot use the count of real extents here.
> + * Instead we have to calculate it from the incore fork.
> + */
> + total_extents = xfs_iext_count(ifp);
> + if (total_extents == 0) {
> + *done = true;
> + goto del_cursor;
> + }
> +
> + /*
> + * Look up the extent index for the fsb where we start shifting. We can
> + * henceforth iterate with current_ext as extent list changes are locked
> + * out via ilock.
> + *
> + * If next_fsb lies in a hole beyond which there are no extents we are
> + * done.
> + */
> + if (!xfs_iext_lookup_extent(ip, ifp, *next_fsb, ¤t_ext, &got)) {
> + *done = true;
> + goto del_cursor;
> + }
> +
> + stop_extent = total_extents;
> + if (current_ext >= stop_extent) {
> + error = -EIO;
> + goto del_cursor;
> + }
> +
> + error = xfs_bmse_shift_one(ip, whichfork, offset_shift_fsb,
> + ¤t_ext, &got, cur, &logflags,
> + SHIFT_LEFT, dfops);
> + if (error)
> + goto del_cursor;
> + /*
> + * If there was an extent merge during the shift, the extent
> + * count can change. Update the total and grade the next record.
> + */
> + total_extents = xfs_iext_count(ifp);
> + stop_extent = total_extents;
> + if (current_ext == stop_extent) {
> + *done = true;
> + goto del_cursor;
> + }
> + xfs_iext_get_extent(ifp, current_ext, &got);
> +
> + if (!*done)
> + *next_fsb = got.br_startoff;
> +
> +del_cursor:
> + if (cur)
> + xfs_btree_del_cursor(cur,
> + error ? XFS_BTREE_ERROR : XFS_BTREE_NOERROR);
> +
> + if (logflags)
> + xfs_trans_log_inode(tp, ip, logflags);
> +
> + return error;
> +}
> +
> +int
> +xfs_bmap_insert_extents(
> + struct xfs_trans *tp,
> + struct xfs_inode *ip,
> + xfs_fileoff_t *next_fsb,
> + xfs_fileoff_t offset_shift_fsb,
> + bool *done,
> + xfs_fileoff_t stop_fsb,
> + xfs_fsblock_t *firstblock,
> + struct xfs_defer_ops *dfops)
> +{
> + int whichfork = XFS_DATA_FORK;
> + struct xfs_mount *mp = ip->i_mount;
> + struct xfs_ifork *ifp = XFS_IFORK_PTR(ip, whichfork);
> + struct xfs_btree_cur *cur = NULL;
> + struct xfs_bmbt_irec got, s;
> + xfs_extnum_t current_ext;
> + xfs_extnum_t total_extents;
> + xfs_extnum_t stop_extent;
> + int error = 0;
> + int logflags = 0;
> +
> + if (unlikely(XFS_TEST_ERROR(
> + (XFS_IFORK_FORMAT(ip, whichfork) != XFS_DINODE_FMT_EXTENTS &&
> + XFS_IFORK_FORMAT(ip, whichfork) != XFS_DINODE_FMT_BTREE),
> + mp, XFS_ERRTAG_BMAPIFORMAT))) {
> + XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW, mp);
> + return -EFSCORRUPTED;
> + }
> +
> + if (XFS_FORCED_SHUTDOWN(mp))
> + return -EIO;
> +
> + ASSERT(xfs_isilocked(ip, XFS_IOLOCK_EXCL | XFS_ILOCK_EXCL));
>
> - ifp = XFS_IFORK_PTR(ip, whichfork);
> if (!(ifp->if_flags & XFS_IFEXTENTS)) {
> - /* Read in all the extents */
> error = xfs_iread_extents(tp, ip, whichfork);
> if (error)
> return error;
> @@ -5759,7 +5853,7 @@ xfs_bmap_shift_extents(
> */
> total_extents = xfs_iext_count(ifp);
> if (total_extents == 0) {
> - *done = 1;
> + *done = true;
> goto del_cursor;
> }
>
> @@ -5767,12 +5861,10 @@ xfs_bmap_shift_extents(
> * In case of first right shift, we need to initialize next_fsb
> */
> if (*next_fsb == NULLFSBLOCK) {
> - ASSERT(direction == SHIFT_RIGHT);
> -
> current_ext = total_extents - 1;
> xfs_iext_get_extent(ifp, current_ext, &got);
> if (stop_fsb > got.br_startoff) {
> - *done = 1;
> + *done = true;
> goto del_cursor;
> }
> *next_fsb = got.br_startoff;
> @@ -5787,46 +5879,27 @@ xfs_bmap_shift_extents(
> */
> if (!xfs_iext_lookup_extent(ip, ifp, *next_fsb, ¤t_ext,
> &got)) {
> - *done = 1;
> + *done = true;
> goto del_cursor;
> }
> }
>
> /* Lookup the extent index at which we have to stop */
> - if (direction == SHIFT_RIGHT) {
> - struct xfs_bmbt_irec s;
> -
> - xfs_iext_lookup_extent(ip, ifp, stop_fsb, &stop_extent, &s);
> - /* Make stop_extent exclusive of shift range */
> - stop_extent--;
> - if (current_ext <= stop_extent) {
> - error = -EIO;
> - goto del_cursor;
> - }
> - } else {
> - stop_extent = total_extents;
> - if (current_ext >= stop_extent) {
> - error = -EIO;
> - goto del_cursor;
> - }
> + xfs_iext_lookup_extent(ip, ifp, stop_fsb, &stop_extent, &s);
> + /* Make stop_extent exclusive of shift range */
> + stop_extent--;
> + if (current_ext <= stop_extent) {
> + error = -EIO;
> + goto del_cursor;
> }
>
> error = xfs_bmse_shift_one(ip, whichfork, offset_shift_fsb,
> ¤t_ext, &got, cur, &logflags,
> - direction, dfops);
> + SHIFT_RIGHT, dfops);
> if (error)
> goto del_cursor;
> - /*
> - * If there was an extent merge during the shift, the extent
> - * count can change. Update the total and grade the next record.
> - */
> - if (direction == SHIFT_LEFT) {
> - total_extents = xfs_iext_count(ifp);
> - stop_extent = total_extents;
> - }
> -
> if (current_ext == stop_extent) {
> - *done = 1;
> + *done = true;
> goto del_cursor;
> }
> xfs_iext_get_extent(ifp, current_ext, &got);
> diff --git a/fs/xfs/libxfs/xfs_bmap.h b/fs/xfs/libxfs/xfs_bmap.h
> index 985b1c26566a..f7ccf2de1a8c 100644
> --- a/fs/xfs/libxfs/xfs_bmap.h
> +++ b/fs/xfs/libxfs/xfs_bmap.h
> @@ -227,10 +227,14 @@ int xfs_bmap_del_extent_delay(struct xfs_inode *ip, int whichfork,
> void xfs_bmap_del_extent_cow(struct xfs_inode *ip, xfs_extnum_t *idx,
> struct xfs_bmbt_irec *got, struct xfs_bmbt_irec *del);
> uint xfs_default_attroffset(struct xfs_inode *ip);
> -int xfs_bmap_shift_extents(struct xfs_trans *tp, struct xfs_inode *ip,
> +int xfs_bmap_collapse_extents(struct xfs_trans *tp, struct xfs_inode *ip,
> xfs_fileoff_t *next_fsb, xfs_fileoff_t offset_shift_fsb,
> - int *done, xfs_fileoff_t stop_fsb, xfs_fsblock_t *firstblock,
> - struct xfs_defer_ops *dfops, enum shift_direction direction);
> + bool *done, xfs_fileoff_t stop_fsb, xfs_fsblock_t *firstblock,
> + struct xfs_defer_ops *dfops);
> +int xfs_bmap_insert_extents(struct xfs_trans *tp, struct xfs_inode *ip,
> + xfs_fileoff_t *next_fsb, xfs_fileoff_t offset_shift_fsb,
> + bool *done, xfs_fileoff_t stop_fsb, xfs_fsblock_t *firstblock,
> + struct xfs_defer_ops *dfops);
> int xfs_bmap_split_extent(struct xfs_inode *ip, xfs_fileoff_t split_offset);
> int xfs_bmapi_reserve_delalloc(struct xfs_inode *ip, int whichfork,
> xfs_fileoff_t off, xfs_filblks_t len, xfs_filblks_t prealloc,
> diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
> index 3273f083c496..034f3429ca8c 100644
> --- a/fs/xfs/xfs_bmap_util.c
> +++ b/fs/xfs/xfs_bmap_util.c
> @@ -1322,7 +1322,6 @@ xfs_collapse_file_space(
> xfs_off_t offset,
> xfs_off_t len)
> {
> - int done = 0;
> struct xfs_mount *mp = ip->i_mount;
> struct xfs_trans *tp;
> int error;
> @@ -1332,6 +1331,7 @@ xfs_collapse_file_space(
> xfs_fileoff_t next_fsb = XFS_B_TO_FSB(mp, offset + len);
> xfs_fileoff_t shift_fsb = XFS_B_TO_FSB(mp, len);
> uint resblks = XFS_DIOSTRAT_SPACE_RES(mp, 0);
> + bool done = false;
>
> ASSERT(xfs_isilocked(ip, XFS_IOLOCK_EXCL));
> trace_xfs_collapse_file_space(ip);
> @@ -1359,9 +1359,8 @@ xfs_collapse_file_space(
> xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL);
>
> xfs_defer_init(&dfops, &first_block);
> - error = xfs_bmap_shift_extents(tp, ip, &next_fsb, shift_fsb,
> - &done, stop_fsb, &first_block, &dfops,
> - SHIFT_LEFT);
> + error = xfs_bmap_collapse_extents(tp, ip, &next_fsb, shift_fsb,
> + &done, stop_fsb, &first_block, &dfops);
> if (error)
> goto out_bmap_cancel;
>
> @@ -1406,7 +1405,7 @@ xfs_insert_file_space(
> xfs_fileoff_t stop_fsb = XFS_B_TO_FSB(mp, offset);
> xfs_fileoff_t next_fsb = NULLFSBLOCK;
> xfs_fileoff_t shift_fsb = XFS_B_TO_FSB(mp, len);
> - int done = 0;
> + bool done = false;
>
> ASSERT(xfs_isilocked(ip, XFS_IOLOCK_EXCL));
> trace_xfs_insert_file_space(ip);
> @@ -1433,9 +1432,8 @@ xfs_insert_file_space(
> xfs_ilock(ip, XFS_ILOCK_EXCL);
> xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL);
> xfs_defer_init(&dfops, &first_block);
> - error = xfs_bmap_shift_extents(tp, ip, &next_fsb, shift_fsb,
> - &done, stop_fsb, &first_block, &dfops,
> - SHIFT_RIGHT);
> + error = xfs_bmap_insert_extents(tp, ip, &next_fsb, shift_fsb,
> + &done, stop_fsb, &first_block, &dfops);
> if (error)
> goto out_bmap_cancel;
>
> --
> 2.14.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH 11/15] xfs: remove xfs_bmse_shift_one
2017-10-19 6:59 more extent mapping cleanups Christoph Hellwig
` (9 preceding siblings ...)
2017-10-19 6:59 ` [PATCH 10/15] xfs: split xfs_bmap_shift_extents Christoph Hellwig
@ 2017-10-19 6:59 ` Christoph Hellwig
2017-10-21 0:25 ` Darrick J. Wong
2017-10-19 6:59 ` [PATCH 12/15] xfs: update got in xfs_bmap_shift_update_extent Christoph Hellwig
` (4 subsequent siblings)
15 siblings, 1 reply; 36+ messages in thread
From: Christoph Hellwig @ 2017-10-19 6:59 UTC (permalink / raw)
To: linux-xfs
Instead do the actual left and right shift work in the callers, and just
keep a helper to update the bmap and rmap btrees as well as the in-core
extent list.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/xfs/libxfs/xfs_bmap.c | 181 +++++++++++++++++++----------------------------
fs/xfs/libxfs/xfs_bmap.h | 5 --
2 files changed, 71 insertions(+), 115 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 7d3a38e69d28..703596caf255 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -5568,94 +5568,21 @@ xfs_bmse_merge(
return xfs_rmap_map_extent(mp, dfops, ip, whichfork, &new);
}
-/*
- * Shift a single extent.
- */
-STATIC int
-xfs_bmse_shift_one(
- struct xfs_inode *ip,
- int whichfork,
- xfs_fileoff_t offset_shift_fsb,
- int *current_ext,
- struct xfs_bmbt_irec *got,
- struct xfs_btree_cur *cur,
- int *logflags,
- enum shift_direction direction,
- struct xfs_defer_ops *dfops)
+static int
+xfs_bmap_shift_update_extent(
+ struct xfs_inode *ip,
+ int whichfork,
+ xfs_extnum_t idx,
+ struct xfs_bmbt_irec *got,
+ struct xfs_btree_cur *cur,
+ int *logflags,
+ struct xfs_defer_ops *dfops,
+ xfs_fileoff_t startoff)
{
- struct xfs_ifork *ifp;
- struct xfs_mount *mp;
- xfs_fileoff_t startoff;
- struct xfs_bmbt_irec adj_irec, new;
- int error;
- int i;
- int total_extents;
-
- mp = ip->i_mount;
- ifp = XFS_IFORK_PTR(ip, whichfork);
- total_extents = xfs_iext_count(ifp);
-
- /* delalloc extents should be prevented by caller */
- XFS_WANT_CORRUPTED_RETURN(mp, !isnullstartblock(got->br_startblock));
-
- if (direction == SHIFT_LEFT) {
- startoff = got->br_startoff - offset_shift_fsb;
-
- /*
- * Check for merge if we've got an extent to the left,
- * otherwise make sure there's enough room at the start
- * of the file for the shift.
- */
- if (!*current_ext) {
- if (got->br_startoff < offset_shift_fsb)
- return -EINVAL;
- goto update_current_ext;
- }
-
- /*
- * grab the left extent and check for a large enough hole.
- */
- xfs_iext_get_extent(ifp, *current_ext - 1, &adj_irec);
- if (startoff < adj_irec.br_startoff + adj_irec.br_blockcount)
- return -EINVAL;
-
- /* check whether to merge the extent or shift it down */
- if (xfs_bmse_can_merge(&adj_irec, got, offset_shift_fsb)) {
- return xfs_bmse_merge(ip, whichfork, offset_shift_fsb,
- *current_ext, got, &adj_irec,
- cur, logflags, dfops);
- }
- } else {
- startoff = got->br_startoff + offset_shift_fsb;
- /* nothing to move if this is the last extent */
- if (*current_ext >= (total_extents - 1))
- goto update_current_ext;
-
- /*
- * If this is not the last extent in the file, make sure there
- * is enough room between current extent and next extent for
- * accommodating the shift.
- */
- xfs_iext_get_extent(ifp, *current_ext + 1, &adj_irec);
- if (startoff + got->br_blockcount > adj_irec.br_startoff)
- return -EINVAL;
-
- /*
- * Unlike a left shift (which involves a hole punch),
- * a right shift does not modify extent neighbors
- * in any way. We should never find mergeable extents
- * in this scenario. Check anyways and warn if we
- * encounter two extents that could be one.
- */
- if (xfs_bmse_can_merge(got, &adj_irec, offset_shift_fsb))
- WARN_ON_ONCE(1);
- }
+ struct xfs_mount *mp = ip->i_mount;
+ struct xfs_bmbt_irec new;
+ int error, i;
- /*
- * Increment the extent index for the next iteration, update the start
- * offset of the in-core extent and update the btree if applicable.
- */
-update_current_ext:
*logflags |= XFS_ILOG_CORE;
new = *got;
@@ -5674,13 +5601,8 @@ xfs_bmse_shift_one(
*logflags |= XFS_ILOG_DEXT;
}
- xfs_iext_update_extent(ip, xfs_bmap_fork_to_state(whichfork),
- *current_ext, &new);
-
- if (direction == SHIFT_LEFT)
- (*current_ext)++;
- else
- (*current_ext)--;
+ xfs_iext_update_extent(ip, xfs_bmap_fork_to_state(whichfork), idx,
+ &new);
/* update reverse mapping */
error = xfs_rmap_unmap_extent(mp, dfops, ip, whichfork, got);
@@ -5704,10 +5626,11 @@ xfs_bmap_collapse_extents(
struct xfs_mount *mp = ip->i_mount;
struct xfs_ifork *ifp = XFS_IFORK_PTR(ip, whichfork);
struct xfs_btree_cur *cur = NULL;
- struct xfs_bmbt_irec got;
+ struct xfs_bmbt_irec got, prev;
xfs_extnum_t current_ext;
xfs_extnum_t total_extents;
xfs_extnum_t stop_extent;
+ xfs_fileoff_t new_startoff;
int error = 0;
int logflags = 0;
@@ -5760,6 +5683,7 @@ xfs_bmap_collapse_extents(
*done = true;
goto del_cursor;
}
+ XFS_WANT_CORRUPTED_RETURN(mp, !isnullstartblock(got.br_startblock));
stop_extent = total_extents;
if (current_ext >= stop_extent) {
@@ -5767,11 +5691,36 @@ xfs_bmap_collapse_extents(
goto del_cursor;
}
- error = xfs_bmse_shift_one(ip, whichfork, offset_shift_fsb,
- ¤t_ext, &got, cur, &logflags,
- SHIFT_LEFT, dfops);
+ new_startoff = got.br_startoff - offset_shift_fsb;
+ if (current_ext) {
+ xfs_iext_get_extent(ifp, current_ext - 1, &prev);
+ if (new_startoff < prev.br_startoff + prev.br_blockcount) {
+ error = -EINVAL;
+ goto del_cursor;
+ }
+
+ /* check whether to merge the extent or shift it down */
+ if (xfs_bmse_can_merge(&prev, &got, offset_shift_fsb)) {
+ error = xfs_bmse_merge(ip, whichfork, offset_shift_fsb,
+ current_ext, &got, &prev, cur,
+ &logflags, dfops);
+ if (error)
+ goto del_cursor;
+ goto done;
+ }
+ } else {
+ if (got.br_startoff < offset_shift_fsb) {
+ error = -EINVAL;
+ goto del_cursor;
+ }
+ }
+
+ error = xfs_bmap_shift_update_extent(ip, whichfork, current_ext, &got,
+ cur, &logflags, dfops, new_startoff);
if (error)
goto del_cursor;
+ current_ext++;
+done:
/*
* If there was an extent merge during the shift, the extent
* count can change. Update the total and grade the next record.
@@ -5784,17 +5733,13 @@ xfs_bmap_collapse_extents(
}
xfs_iext_get_extent(ifp, current_ext, &got);
- if (!*done)
- *next_fsb = got.br_startoff;
-
+ *next_fsb = got.br_startoff;
del_cursor:
if (cur)
xfs_btree_del_cursor(cur,
error ? XFS_BTREE_ERROR : XFS_BTREE_NOERROR);
-
if (logflags)
xfs_trans_log_inode(tp, ip, logflags);
-
return error;
}
@@ -5813,10 +5758,11 @@ xfs_bmap_insert_extents(
struct xfs_mount *mp = ip->i_mount;
struct xfs_ifork *ifp = XFS_IFORK_PTR(ip, whichfork);
struct xfs_btree_cur *cur = NULL;
- struct xfs_bmbt_irec got, s;
+ struct xfs_bmbt_irec got, next, s;
xfs_extnum_t current_ext;
xfs_extnum_t total_extents;
xfs_extnum_t stop_extent;
+ xfs_fileoff_t new_startoff;
int error = 0;
int logflags = 0;
@@ -5883,6 +5829,7 @@ xfs_bmap_insert_extents(
goto del_cursor;
}
}
+ XFS_WANT_CORRUPTED_RETURN(mp, !isnullstartblock(got.br_startblock));
/* Lookup the extent index at which we have to stop */
xfs_iext_lookup_extent(ip, ifp, stop_fsb, &stop_extent, &s);
@@ -5893,27 +5840,41 @@ xfs_bmap_insert_extents(
goto del_cursor;
}
- error = xfs_bmse_shift_one(ip, whichfork, offset_shift_fsb,
- ¤t_ext, &got, cur, &logflags,
- SHIFT_RIGHT, dfops);
+ new_startoff = got.br_startoff + offset_shift_fsb;
+ if (current_ext < total_extents - 1) {
+ xfs_iext_get_extent(ifp, current_ext + 1, &next);
+ if (new_startoff + got.br_blockcount > next.br_startoff) {
+ error = -EINVAL;
+ goto del_cursor;
+ }
+
+ /*
+ * Unlike a left shift (which involves a hole punch), a right
+ * shift does not modify extent neighbors in any way. We should
+ * never find mergeable extents in this scenario. Check anyways
+ * and warn if we encounter two extents that could be one.
+ */
+ if (xfs_bmse_can_merge(&got, &next, offset_shift_fsb))
+ WARN_ON_ONCE(1);
+ }
+
+ error = xfs_bmap_shift_update_extent(ip, whichfork, current_ext, &got,
+ cur, &logflags, dfops, new_startoff);
if (error)
goto del_cursor;
- if (current_ext == stop_extent) {
+ if (--current_ext == stop_extent) {
*done = true;
goto del_cursor;
}
xfs_iext_get_extent(ifp, current_ext, &got);
*next_fsb = got.br_startoff;
-
del_cursor:
if (cur)
xfs_btree_del_cursor(cur,
error ? XFS_BTREE_ERROR : XFS_BTREE_NOERROR);
-
if (logflags)
xfs_trans_log_inode(tp, ip, logflags);
-
return error;
}
diff --git a/fs/xfs/libxfs/xfs_bmap.h b/fs/xfs/libxfs/xfs_bmap.h
index f7ccf2de1a8c..6c426cdfb758 100644
--- a/fs/xfs/libxfs/xfs_bmap.h
+++ b/fs/xfs/libxfs/xfs_bmap.h
@@ -183,11 +183,6 @@ static inline bool xfs_bmap_is_real_extent(struct xfs_bmbt_irec *irec)
!isnullstartblock(irec->br_startblock);
}
-enum shift_direction {
- SHIFT_LEFT = 0,
- SHIFT_RIGHT,
-};
-
void xfs_trim_extent(struct xfs_bmbt_irec *irec, xfs_fileoff_t bno,
xfs_filblks_t len);
int xfs_bmap_add_attrfork(struct xfs_inode *ip, int size, int rsvd);
--
2.14.1
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [PATCH 11/15] xfs: remove xfs_bmse_shift_one
2017-10-19 6:59 ` [PATCH 11/15] xfs: remove xfs_bmse_shift_one Christoph Hellwig
@ 2017-10-21 0:25 ` Darrick J. Wong
0 siblings, 0 replies; 36+ messages in thread
From: Darrick J. Wong @ 2017-10-21 0:25 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs
On Thu, Oct 19, 2017 at 08:59:38AM +0200, Christoph Hellwig wrote:
> Instead do the actual left and right shift work in the callers, and just
> keep a helper to update the bmap and rmap btrees as well as the in-core
> extent list.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
Looks ok,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
> fs/xfs/libxfs/xfs_bmap.c | 181 +++++++++++++++++++----------------------------
> fs/xfs/libxfs/xfs_bmap.h | 5 --
> 2 files changed, 71 insertions(+), 115 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index 7d3a38e69d28..703596caf255 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -5568,94 +5568,21 @@ xfs_bmse_merge(
> return xfs_rmap_map_extent(mp, dfops, ip, whichfork, &new);
> }
>
> -/*
> - * Shift a single extent.
> - */
> -STATIC int
> -xfs_bmse_shift_one(
> - struct xfs_inode *ip,
> - int whichfork,
> - xfs_fileoff_t offset_shift_fsb,
> - int *current_ext,
> - struct xfs_bmbt_irec *got,
> - struct xfs_btree_cur *cur,
> - int *logflags,
> - enum shift_direction direction,
> - struct xfs_defer_ops *dfops)
> +static int
> +xfs_bmap_shift_update_extent(
> + struct xfs_inode *ip,
> + int whichfork,
> + xfs_extnum_t idx,
> + struct xfs_bmbt_irec *got,
> + struct xfs_btree_cur *cur,
> + int *logflags,
> + struct xfs_defer_ops *dfops,
> + xfs_fileoff_t startoff)
> {
> - struct xfs_ifork *ifp;
> - struct xfs_mount *mp;
> - xfs_fileoff_t startoff;
> - struct xfs_bmbt_irec adj_irec, new;
> - int error;
> - int i;
> - int total_extents;
> -
> - mp = ip->i_mount;
> - ifp = XFS_IFORK_PTR(ip, whichfork);
> - total_extents = xfs_iext_count(ifp);
> -
> - /* delalloc extents should be prevented by caller */
> - XFS_WANT_CORRUPTED_RETURN(mp, !isnullstartblock(got->br_startblock));
> -
> - if (direction == SHIFT_LEFT) {
> - startoff = got->br_startoff - offset_shift_fsb;
> -
> - /*
> - * Check for merge if we've got an extent to the left,
> - * otherwise make sure there's enough room at the start
> - * of the file for the shift.
> - */
> - if (!*current_ext) {
> - if (got->br_startoff < offset_shift_fsb)
> - return -EINVAL;
> - goto update_current_ext;
> - }
> -
> - /*
> - * grab the left extent and check for a large enough hole.
> - */
> - xfs_iext_get_extent(ifp, *current_ext - 1, &adj_irec);
> - if (startoff < adj_irec.br_startoff + adj_irec.br_blockcount)
> - return -EINVAL;
> -
> - /* check whether to merge the extent or shift it down */
> - if (xfs_bmse_can_merge(&adj_irec, got, offset_shift_fsb)) {
> - return xfs_bmse_merge(ip, whichfork, offset_shift_fsb,
> - *current_ext, got, &adj_irec,
> - cur, logflags, dfops);
> - }
> - } else {
> - startoff = got->br_startoff + offset_shift_fsb;
> - /* nothing to move if this is the last extent */
> - if (*current_ext >= (total_extents - 1))
> - goto update_current_ext;
> -
> - /*
> - * If this is not the last extent in the file, make sure there
> - * is enough room between current extent and next extent for
> - * accommodating the shift.
> - */
> - xfs_iext_get_extent(ifp, *current_ext + 1, &adj_irec);
> - if (startoff + got->br_blockcount > adj_irec.br_startoff)
> - return -EINVAL;
> -
> - /*
> - * Unlike a left shift (which involves a hole punch),
> - * a right shift does not modify extent neighbors
> - * in any way. We should never find mergeable extents
> - * in this scenario. Check anyways and warn if we
> - * encounter two extents that could be one.
> - */
> - if (xfs_bmse_can_merge(got, &adj_irec, offset_shift_fsb))
> - WARN_ON_ONCE(1);
> - }
> + struct xfs_mount *mp = ip->i_mount;
> + struct xfs_bmbt_irec new;
> + int error, i;
>
> - /*
> - * Increment the extent index for the next iteration, update the start
> - * offset of the in-core extent and update the btree if applicable.
> - */
> -update_current_ext:
> *logflags |= XFS_ILOG_CORE;
>
> new = *got;
> @@ -5674,13 +5601,8 @@ xfs_bmse_shift_one(
> *logflags |= XFS_ILOG_DEXT;
> }
>
> - xfs_iext_update_extent(ip, xfs_bmap_fork_to_state(whichfork),
> - *current_ext, &new);
> -
> - if (direction == SHIFT_LEFT)
> - (*current_ext)++;
> - else
> - (*current_ext)--;
> + xfs_iext_update_extent(ip, xfs_bmap_fork_to_state(whichfork), idx,
> + &new);
>
> /* update reverse mapping */
> error = xfs_rmap_unmap_extent(mp, dfops, ip, whichfork, got);
> @@ -5704,10 +5626,11 @@ xfs_bmap_collapse_extents(
> struct xfs_mount *mp = ip->i_mount;
> struct xfs_ifork *ifp = XFS_IFORK_PTR(ip, whichfork);
> struct xfs_btree_cur *cur = NULL;
> - struct xfs_bmbt_irec got;
> + struct xfs_bmbt_irec got, prev;
> xfs_extnum_t current_ext;
> xfs_extnum_t total_extents;
> xfs_extnum_t stop_extent;
> + xfs_fileoff_t new_startoff;
> int error = 0;
> int logflags = 0;
>
> @@ -5760,6 +5683,7 @@ xfs_bmap_collapse_extents(
> *done = true;
> goto del_cursor;
> }
> + XFS_WANT_CORRUPTED_RETURN(mp, !isnullstartblock(got.br_startblock));
>
> stop_extent = total_extents;
> if (current_ext >= stop_extent) {
> @@ -5767,11 +5691,36 @@ xfs_bmap_collapse_extents(
> goto del_cursor;
> }
>
> - error = xfs_bmse_shift_one(ip, whichfork, offset_shift_fsb,
> - ¤t_ext, &got, cur, &logflags,
> - SHIFT_LEFT, dfops);
> + new_startoff = got.br_startoff - offset_shift_fsb;
> + if (current_ext) {
> + xfs_iext_get_extent(ifp, current_ext - 1, &prev);
> + if (new_startoff < prev.br_startoff + prev.br_blockcount) {
> + error = -EINVAL;
> + goto del_cursor;
> + }
> +
> + /* check whether to merge the extent or shift it down */
> + if (xfs_bmse_can_merge(&prev, &got, offset_shift_fsb)) {
> + error = xfs_bmse_merge(ip, whichfork, offset_shift_fsb,
> + current_ext, &got, &prev, cur,
> + &logflags, dfops);
> + if (error)
> + goto del_cursor;
> + goto done;
> + }
> + } else {
> + if (got.br_startoff < offset_shift_fsb) {
> + error = -EINVAL;
> + goto del_cursor;
> + }
> + }
> +
> + error = xfs_bmap_shift_update_extent(ip, whichfork, current_ext, &got,
> + cur, &logflags, dfops, new_startoff);
> if (error)
> goto del_cursor;
> + current_ext++;
> +done:
> /*
> * If there was an extent merge during the shift, the extent
> * count can change. Update the total and grade the next record.
> @@ -5784,17 +5733,13 @@ xfs_bmap_collapse_extents(
> }
> xfs_iext_get_extent(ifp, current_ext, &got);
>
> - if (!*done)
> - *next_fsb = got.br_startoff;
> -
> + *next_fsb = got.br_startoff;
> del_cursor:
> if (cur)
> xfs_btree_del_cursor(cur,
> error ? XFS_BTREE_ERROR : XFS_BTREE_NOERROR);
> -
> if (logflags)
> xfs_trans_log_inode(tp, ip, logflags);
> -
> return error;
> }
>
> @@ -5813,10 +5758,11 @@ xfs_bmap_insert_extents(
> struct xfs_mount *mp = ip->i_mount;
> struct xfs_ifork *ifp = XFS_IFORK_PTR(ip, whichfork);
> struct xfs_btree_cur *cur = NULL;
> - struct xfs_bmbt_irec got, s;
> + struct xfs_bmbt_irec got, next, s;
> xfs_extnum_t current_ext;
> xfs_extnum_t total_extents;
> xfs_extnum_t stop_extent;
> + xfs_fileoff_t new_startoff;
> int error = 0;
> int logflags = 0;
>
> @@ -5883,6 +5829,7 @@ xfs_bmap_insert_extents(
> goto del_cursor;
> }
> }
> + XFS_WANT_CORRUPTED_RETURN(mp, !isnullstartblock(got.br_startblock));
>
> /* Lookup the extent index at which we have to stop */
> xfs_iext_lookup_extent(ip, ifp, stop_fsb, &stop_extent, &s);
> @@ -5893,27 +5840,41 @@ xfs_bmap_insert_extents(
> goto del_cursor;
> }
>
> - error = xfs_bmse_shift_one(ip, whichfork, offset_shift_fsb,
> - ¤t_ext, &got, cur, &logflags,
> - SHIFT_RIGHT, dfops);
> + new_startoff = got.br_startoff + offset_shift_fsb;
> + if (current_ext < total_extents - 1) {
> + xfs_iext_get_extent(ifp, current_ext + 1, &next);
> + if (new_startoff + got.br_blockcount > next.br_startoff) {
> + error = -EINVAL;
> + goto del_cursor;
> + }
> +
> + /*
> + * Unlike a left shift (which involves a hole punch), a right
> + * shift does not modify extent neighbors in any way. We should
> + * never find mergeable extents in this scenario. Check anyways
> + * and warn if we encounter two extents that could be one.
> + */
> + if (xfs_bmse_can_merge(&got, &next, offset_shift_fsb))
> + WARN_ON_ONCE(1);
> + }
> +
> + error = xfs_bmap_shift_update_extent(ip, whichfork, current_ext, &got,
> + cur, &logflags, dfops, new_startoff);
> if (error)
> goto del_cursor;
> - if (current_ext == stop_extent) {
> + if (--current_ext == stop_extent) {
> *done = true;
> goto del_cursor;
> }
> xfs_iext_get_extent(ifp, current_ext, &got);
>
> *next_fsb = got.br_startoff;
> -
> del_cursor:
> if (cur)
> xfs_btree_del_cursor(cur,
> error ? XFS_BTREE_ERROR : XFS_BTREE_NOERROR);
> -
> if (logflags)
> xfs_trans_log_inode(tp, ip, logflags);
> -
> return error;
> }
>
> diff --git a/fs/xfs/libxfs/xfs_bmap.h b/fs/xfs/libxfs/xfs_bmap.h
> index f7ccf2de1a8c..6c426cdfb758 100644
> --- a/fs/xfs/libxfs/xfs_bmap.h
> +++ b/fs/xfs/libxfs/xfs_bmap.h
> @@ -183,11 +183,6 @@ static inline bool xfs_bmap_is_real_extent(struct xfs_bmbt_irec *irec)
> !isnullstartblock(irec->br_startblock);
> }
>
> -enum shift_direction {
> - SHIFT_LEFT = 0,
> - SHIFT_RIGHT,
> -};
> -
> void xfs_trim_extent(struct xfs_bmbt_irec *irec, xfs_fileoff_t bno,
> xfs_filblks_t len);
> int xfs_bmap_add_attrfork(struct xfs_inode *ip, int size, int rsvd);
> --
> 2.14.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH 12/15] xfs: update got in xfs_bmap_shift_update_extent
2017-10-19 6:59 more extent mapping cleanups Christoph Hellwig
` (10 preceding siblings ...)
2017-10-19 6:59 ` [PATCH 11/15] xfs: remove xfs_bmse_shift_one Christoph Hellwig
@ 2017-10-19 6:59 ` Christoph Hellwig
2017-10-21 0:25 ` Darrick J. Wong
2017-10-19 6:59 ` [PATCH 13/15] xfs: don't rely on extent indices in xfs_bmap_collapse_extents Christoph Hellwig
` (3 subsequent siblings)
15 siblings, 1 reply; 36+ messages in thread
From: Christoph Hellwig @ 2017-10-19 6:59 UTC (permalink / raw)
To: linux-xfs
This way the caller gets the proper updated extent returned in got.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/xfs/libxfs/xfs_bmap.c | 16 +++++++---------
1 file changed, 7 insertions(+), 9 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 703596caf255..2de9458903b6 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -5580,35 +5580,33 @@ xfs_bmap_shift_update_extent(
xfs_fileoff_t startoff)
{
struct xfs_mount *mp = ip->i_mount;
- struct xfs_bmbt_irec new;
+ struct xfs_bmbt_irec prev = *got;
int error, i;
*logflags |= XFS_ILOG_CORE;
- new = *got;
- new.br_startoff = startoff;
+ got->br_startoff = startoff;
if (cur) {
- error = xfs_bmbt_lookup_eq(cur, got, &i);
+ error = xfs_bmbt_lookup_eq(cur, &prev, &i);
if (error)
return error;
XFS_WANT_CORRUPTED_RETURN(mp, i == 1);
- error = xfs_bmbt_update(cur, &new);
+ error = xfs_bmbt_update(cur, got);
if (error)
return error;
} else {
*logflags |= XFS_ILOG_DEXT;
}
- xfs_iext_update_extent(ip, xfs_bmap_fork_to_state(whichfork), idx,
- &new);
+ xfs_iext_update_extent(ip, xfs_bmap_fork_to_state(whichfork), idx, got);
/* update reverse mapping */
- error = xfs_rmap_unmap_extent(mp, dfops, ip, whichfork, got);
+ error = xfs_rmap_unmap_extent(mp, dfops, ip, whichfork, &prev);
if (error)
return error;
- return xfs_rmap_map_extent(mp, dfops, ip, whichfork, &new);
+ return xfs_rmap_map_extent(mp, dfops, ip, whichfork, got);
}
int
--
2.14.1
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [PATCH 12/15] xfs: update got in xfs_bmap_shift_update_extent
2017-10-19 6:59 ` [PATCH 12/15] xfs: update got in xfs_bmap_shift_update_extent Christoph Hellwig
@ 2017-10-21 0:25 ` Darrick J. Wong
0 siblings, 0 replies; 36+ messages in thread
From: Darrick J. Wong @ 2017-10-21 0:25 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs
On Thu, Oct 19, 2017 at 08:59:39AM +0200, Christoph Hellwig wrote:
> This way the caller gets the proper updated extent returned in got.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
Looks ok,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
> fs/xfs/libxfs/xfs_bmap.c | 16 +++++++---------
> 1 file changed, 7 insertions(+), 9 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index 703596caf255..2de9458903b6 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -5580,35 +5580,33 @@ xfs_bmap_shift_update_extent(
> xfs_fileoff_t startoff)
> {
> struct xfs_mount *mp = ip->i_mount;
> - struct xfs_bmbt_irec new;
> + struct xfs_bmbt_irec prev = *got;
> int error, i;
>
> *logflags |= XFS_ILOG_CORE;
>
> - new = *got;
> - new.br_startoff = startoff;
> + got->br_startoff = startoff;
>
> if (cur) {
> - error = xfs_bmbt_lookup_eq(cur, got, &i);
> + error = xfs_bmbt_lookup_eq(cur, &prev, &i);
> if (error)
> return error;
> XFS_WANT_CORRUPTED_RETURN(mp, i == 1);
>
> - error = xfs_bmbt_update(cur, &new);
> + error = xfs_bmbt_update(cur, got);
> if (error)
> return error;
> } else {
> *logflags |= XFS_ILOG_DEXT;
> }
>
> - xfs_iext_update_extent(ip, xfs_bmap_fork_to_state(whichfork), idx,
> - &new);
> + xfs_iext_update_extent(ip, xfs_bmap_fork_to_state(whichfork), idx, got);
>
> /* update reverse mapping */
> - error = xfs_rmap_unmap_extent(mp, dfops, ip, whichfork, got);
> + error = xfs_rmap_unmap_extent(mp, dfops, ip, whichfork, &prev);
> if (error)
> return error;
> - return xfs_rmap_map_extent(mp, dfops, ip, whichfork, &new);
> + return xfs_rmap_map_extent(mp, dfops, ip, whichfork, got);
> }
>
> int
> --
> 2.14.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH 13/15] xfs: don't rely on extent indices in xfs_bmap_collapse_extents
2017-10-19 6:59 more extent mapping cleanups Christoph Hellwig
` (11 preceding siblings ...)
2017-10-19 6:59 ` [PATCH 12/15] xfs: update got in xfs_bmap_shift_update_extent Christoph Hellwig
@ 2017-10-19 6:59 ` Christoph Hellwig
2017-10-21 0:26 ` Darrick J. Wong
2017-10-19 6:59 ` [PATCH 14/15] xfs: don't rely on extent indices in xfs_bmap_insert_extents Christoph Hellwig
` (2 subsequent siblings)
15 siblings, 1 reply; 36+ messages in thread
From: Christoph Hellwig @ 2017-10-19 6:59 UTC (permalink / raw)
To: linux-xfs
Rewrite xfs_bmap_collapse_extents so that we don't rely on extent indices
except for iterating over them. Not being able to iterate to the next
extent is a sufficient exit condition, and we don't need to do any extent
count games given that:
a) we already flushed all delalloc extents past our start offset
before doing the operation
b) xfs_iext_count() includes delalloc extents anyway
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/xfs/libxfs/xfs_bmap.c | 54 +++++++++++-------------------------------------
1 file changed, 12 insertions(+), 42 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 2de9458903b6..4c89fdacda90 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -5626,8 +5626,6 @@ xfs_bmap_collapse_extents(
struct xfs_btree_cur *cur = NULL;
struct xfs_bmbt_irec got, prev;
xfs_extnum_t current_ext;
- xfs_extnum_t total_extents;
- xfs_extnum_t stop_extent;
xfs_fileoff_t new_startoff;
int error = 0;
int logflags = 0;
@@ -5658,52 +5656,31 @@ xfs_bmap_collapse_extents(
cur->bc_private.b.flags = 0;
}
- /*
- * There may be delalloc extents in the data fork before the range we
- * are collapsing out, so we cannot use the count of real extents here.
- * Instead we have to calculate it from the incore fork.
- */
- total_extents = xfs_iext_count(ifp);
- if (total_extents == 0) {
- *done = true;
- goto del_cursor;
- }
-
- /*
- * Look up the extent index for the fsb where we start shifting. We can
- * henceforth iterate with current_ext as extent list changes are locked
- * out via ilock.
- *
- * If next_fsb lies in a hole beyond which there are no extents we are
- * done.
- */
if (!xfs_iext_lookup_extent(ip, ifp, *next_fsb, ¤t_ext, &got)) {
*done = true;
goto del_cursor;
}
XFS_WANT_CORRUPTED_RETURN(mp, !isnullstartblock(got.br_startblock));
- stop_extent = total_extents;
- if (current_ext >= stop_extent) {
- error = -EIO;
- goto del_cursor;
- }
-
new_startoff = got.br_startoff - offset_shift_fsb;
- if (current_ext) {
- xfs_iext_get_extent(ifp, current_ext - 1, &prev);
+ if (xfs_iext_get_extent(ifp, current_ext - 1, &prev)) {
if (new_startoff < prev.br_startoff + prev.br_blockcount) {
error = -EINVAL;
goto del_cursor;
}
- /* check whether to merge the extent or shift it down */
if (xfs_bmse_can_merge(&prev, &got, offset_shift_fsb)) {
error = xfs_bmse_merge(ip, whichfork, offset_shift_fsb,
current_ext, &got, &prev, cur,
&logflags, dfops);
if (error)
goto del_cursor;
+
+ /* update got after merge */
+ if (!xfs_iext_get_extent(ifp, current_ext, &got)) {
+ *done = true;
+ goto del_cursor;
+ }
goto done;
}
} else {
@@ -5717,20 +5694,13 @@ xfs_bmap_collapse_extents(
cur, &logflags, dfops, new_startoff);
if (error)
goto del_cursor;
- current_ext++;
-done:
- /*
- * If there was an extent merge during the shift, the extent
- * count can change. Update the total and grade the next record.
- */
- total_extents = xfs_iext_count(ifp);
- stop_extent = total_extents;
- if (current_ext == stop_extent) {
- *done = true;
- goto del_cursor;
+
+ if (!xfs_iext_get_extent(ifp, ++current_ext, &got)) {
+ *done = true;
+ goto del_cursor;
}
- xfs_iext_get_extent(ifp, current_ext, &got);
+done:
*next_fsb = got.br_startoff;
del_cursor:
if (cur)
--
2.14.1
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [PATCH 13/15] xfs: don't rely on extent indices in xfs_bmap_collapse_extents
2017-10-19 6:59 ` [PATCH 13/15] xfs: don't rely on extent indices in xfs_bmap_collapse_extents Christoph Hellwig
@ 2017-10-21 0:26 ` Darrick J. Wong
0 siblings, 0 replies; 36+ messages in thread
From: Darrick J. Wong @ 2017-10-21 0:26 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs
On Thu, Oct 19, 2017 at 08:59:40AM +0200, Christoph Hellwig wrote:
> Rewrite xfs_bmap_collapse_extents so that we don't rely on extent indices
> except for iterating over them. Not being able to iterate to the next
> extent is a sufficient exit condition, and we don't need to do any extent
> count games given that:
>
> a) we already flushed all delalloc extents past our start offset
> before doing the operation
> b) xfs_iext_count() includes delalloc extents anyway
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
Looks ok,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
> fs/xfs/libxfs/xfs_bmap.c | 54 +++++++++++-------------------------------------
> 1 file changed, 12 insertions(+), 42 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index 2de9458903b6..4c89fdacda90 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -5626,8 +5626,6 @@ xfs_bmap_collapse_extents(
> struct xfs_btree_cur *cur = NULL;
> struct xfs_bmbt_irec got, prev;
> xfs_extnum_t current_ext;
> - xfs_extnum_t total_extents;
> - xfs_extnum_t stop_extent;
> xfs_fileoff_t new_startoff;
> int error = 0;
> int logflags = 0;
> @@ -5658,52 +5656,31 @@ xfs_bmap_collapse_extents(
> cur->bc_private.b.flags = 0;
> }
>
> - /*
> - * There may be delalloc extents in the data fork before the range we
> - * are collapsing out, so we cannot use the count of real extents here.
> - * Instead we have to calculate it from the incore fork.
> - */
> - total_extents = xfs_iext_count(ifp);
> - if (total_extents == 0) {
> - *done = true;
> - goto del_cursor;
> - }
> -
> - /*
> - * Look up the extent index for the fsb where we start shifting. We can
> - * henceforth iterate with current_ext as extent list changes are locked
> - * out via ilock.
> - *
> - * If next_fsb lies in a hole beyond which there are no extents we are
> - * done.
> - */
> if (!xfs_iext_lookup_extent(ip, ifp, *next_fsb, ¤t_ext, &got)) {
> *done = true;
> goto del_cursor;
> }
> XFS_WANT_CORRUPTED_RETURN(mp, !isnullstartblock(got.br_startblock));
>
> - stop_extent = total_extents;
> - if (current_ext >= stop_extent) {
> - error = -EIO;
> - goto del_cursor;
> - }
> -
> new_startoff = got.br_startoff - offset_shift_fsb;
> - if (current_ext) {
> - xfs_iext_get_extent(ifp, current_ext - 1, &prev);
> + if (xfs_iext_get_extent(ifp, current_ext - 1, &prev)) {
> if (new_startoff < prev.br_startoff + prev.br_blockcount) {
> error = -EINVAL;
> goto del_cursor;
> }
>
> - /* check whether to merge the extent or shift it down */
> if (xfs_bmse_can_merge(&prev, &got, offset_shift_fsb)) {
> error = xfs_bmse_merge(ip, whichfork, offset_shift_fsb,
> current_ext, &got, &prev, cur,
> &logflags, dfops);
> if (error)
> goto del_cursor;
> +
> + /* update got after merge */
> + if (!xfs_iext_get_extent(ifp, current_ext, &got)) {
> + *done = true;
> + goto del_cursor;
> + }
> goto done;
> }
> } else {
> @@ -5717,20 +5694,13 @@ xfs_bmap_collapse_extents(
> cur, &logflags, dfops, new_startoff);
> if (error)
> goto del_cursor;
> - current_ext++;
> -done:
> - /*
> - * If there was an extent merge during the shift, the extent
> - * count can change. Update the total and grade the next record.
> - */
> - total_extents = xfs_iext_count(ifp);
> - stop_extent = total_extents;
> - if (current_ext == stop_extent) {
> - *done = true;
> - goto del_cursor;
> +
> + if (!xfs_iext_get_extent(ifp, ++current_ext, &got)) {
> + *done = true;
> + goto del_cursor;
> }
> - xfs_iext_get_extent(ifp, current_ext, &got);
>
> +done:
> *next_fsb = got.br_startoff;
> del_cursor:
> if (cur)
> --
> 2.14.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH 14/15] xfs: don't rely on extent indices in xfs_bmap_insert_extents
2017-10-19 6:59 more extent mapping cleanups Christoph Hellwig
` (12 preceding siblings ...)
2017-10-19 6:59 ` [PATCH 13/15] xfs: don't rely on extent indices in xfs_bmap_collapse_extents Christoph Hellwig
@ 2017-10-19 6:59 ` Christoph Hellwig
2017-10-21 0:27 ` Darrick J. Wong
2017-10-19 6:59 ` [PATCH 15/15] xfs: rewrite xfs_bmap_first_unused to make better use of xfs_iext_get_extent Christoph Hellwig
2017-10-19 20:04 ` more extent mapping cleanups Darrick J. Wong
15 siblings, 1 reply; 36+ messages in thread
From: Christoph Hellwig @ 2017-10-19 6:59 UTC (permalink / raw)
To: linux-xfs
Rewrite xfs_bmap_insert_extents so that we don't rely on extent indices
except for iterating over them. Not being able to iterate to the previous
extent or finding the extent that stop_fsb is in are sufficient exit
conditions, and we don't need to do any extent count games given that:
a) we already flushed all delalloc extents past our start offset
before doing the operation
b) xfs_iext_count() includes delalloc extents anyway
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/xfs/libxfs/xfs_bmap.c | 47 +++++++++--------------------------------------
1 file changed, 9 insertions(+), 38 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 4c89fdacda90..9b638cc49f5c 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -5726,10 +5726,8 @@ xfs_bmap_insert_extents(
struct xfs_mount *mp = ip->i_mount;
struct xfs_ifork *ifp = XFS_IFORK_PTR(ip, whichfork);
struct xfs_btree_cur *cur = NULL;
- struct xfs_bmbt_irec got, next, s;
+ struct xfs_bmbt_irec got, next;
xfs_extnum_t current_ext;
- xfs_extnum_t total_extents;
- xfs_extnum_t stop_extent;
xfs_fileoff_t new_startoff;
int error = 0;
int logflags = 0;
@@ -5760,37 +5758,14 @@ xfs_bmap_insert_extents(
cur->bc_private.b.flags = 0;
}
- /*
- * There may be delalloc extents in the data fork before the range we
- * are collapsing out, so we cannot use the count of real extents here.
- * Instead we have to calculate it from the incore fork.
- */
- total_extents = xfs_iext_count(ifp);
- if (total_extents == 0) {
- *done = true;
- goto del_cursor;
- }
-
- /*
- * In case of first right shift, we need to initialize next_fsb
- */
if (*next_fsb == NULLFSBLOCK) {
- current_ext = total_extents - 1;
- xfs_iext_get_extent(ifp, current_ext, &got);
- if (stop_fsb > got.br_startoff) {
+ current_ext = xfs_iext_count(ifp) - 1;
+ if (!xfs_iext_get_extent(ifp, current_ext, &got) ||
+ stop_fsb > got.br_startoff) {
*done = true;
goto del_cursor;
}
- *next_fsb = got.br_startoff;
} else {
- /*
- * Look up the extent index for the fsb where we start shifting. We can
- * henceforth iterate with current_ext as extent list changes are locked
- * out via ilock.
- *
- * If next_fsb lies in a hole beyond which there are no extents we are
- * done.
- */
if (!xfs_iext_lookup_extent(ip, ifp, *next_fsb, ¤t_ext,
&got)) {
*done = true;
@@ -5799,18 +5774,13 @@ xfs_bmap_insert_extents(
}
XFS_WANT_CORRUPTED_RETURN(mp, !isnullstartblock(got.br_startblock));
- /* Lookup the extent index at which we have to stop */
- xfs_iext_lookup_extent(ip, ifp, stop_fsb, &stop_extent, &s);
- /* Make stop_extent exclusive of shift range */
- stop_extent--;
- if (current_ext <= stop_extent) {
+ if (stop_fsb >= got.br_startoff + got.br_blockcount) {
error = -EIO;
goto del_cursor;
}
new_startoff = got.br_startoff + offset_shift_fsb;
- if (current_ext < total_extents - 1) {
- xfs_iext_get_extent(ifp, current_ext + 1, &next);
+ if (xfs_iext_get_extent(ifp, current_ext + 1, &next)) {
if (new_startoff + got.br_blockcount > next.br_startoff) {
error = -EINVAL;
goto del_cursor;
@@ -5830,11 +5800,12 @@ xfs_bmap_insert_extents(
cur, &logflags, dfops, new_startoff);
if (error)
goto del_cursor;
- if (--current_ext == stop_extent) {
+
+ if (!xfs_iext_get_extent(ifp, --current_ext, &got) ||
+ stop_fsb >= got.br_startoff + got.br_blockcount) {
*done = true;
goto del_cursor;
}
- xfs_iext_get_extent(ifp, current_ext, &got);
*next_fsb = got.br_startoff;
del_cursor:
--
2.14.1
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [PATCH 14/15] xfs: don't rely on extent indices in xfs_bmap_insert_extents
2017-10-19 6:59 ` [PATCH 14/15] xfs: don't rely on extent indices in xfs_bmap_insert_extents Christoph Hellwig
@ 2017-10-21 0:27 ` Darrick J. Wong
0 siblings, 0 replies; 36+ messages in thread
From: Darrick J. Wong @ 2017-10-21 0:27 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs
On Thu, Oct 19, 2017 at 08:59:41AM +0200, Christoph Hellwig wrote:
> Rewrite xfs_bmap_insert_extents so that we don't rely on extent indices
> except for iterating over them. Not being able to iterate to the previous
> extent or finding the extent that stop_fsb is in are sufficient exit
> conditions, and we don't need to do any extent count games given that:
>
> a) we already flushed all delalloc extents past our start offset
> before doing the operation
> b) xfs_iext_count() includes delalloc extents anyway
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
Looks ok,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
> fs/xfs/libxfs/xfs_bmap.c | 47 +++++++++--------------------------------------
> 1 file changed, 9 insertions(+), 38 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index 4c89fdacda90..9b638cc49f5c 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -5726,10 +5726,8 @@ xfs_bmap_insert_extents(
> struct xfs_mount *mp = ip->i_mount;
> struct xfs_ifork *ifp = XFS_IFORK_PTR(ip, whichfork);
> struct xfs_btree_cur *cur = NULL;
> - struct xfs_bmbt_irec got, next, s;
> + struct xfs_bmbt_irec got, next;
> xfs_extnum_t current_ext;
> - xfs_extnum_t total_extents;
> - xfs_extnum_t stop_extent;
> xfs_fileoff_t new_startoff;
> int error = 0;
> int logflags = 0;
> @@ -5760,37 +5758,14 @@ xfs_bmap_insert_extents(
> cur->bc_private.b.flags = 0;
> }
>
> - /*
> - * There may be delalloc extents in the data fork before the range we
> - * are collapsing out, so we cannot use the count of real extents here.
> - * Instead we have to calculate it from the incore fork.
> - */
> - total_extents = xfs_iext_count(ifp);
> - if (total_extents == 0) {
> - *done = true;
> - goto del_cursor;
> - }
> -
> - /*
> - * In case of first right shift, we need to initialize next_fsb
> - */
> if (*next_fsb == NULLFSBLOCK) {
> - current_ext = total_extents - 1;
> - xfs_iext_get_extent(ifp, current_ext, &got);
> - if (stop_fsb > got.br_startoff) {
> + current_ext = xfs_iext_count(ifp) - 1;
> + if (!xfs_iext_get_extent(ifp, current_ext, &got) ||
> + stop_fsb > got.br_startoff) {
> *done = true;
> goto del_cursor;
> }
> - *next_fsb = got.br_startoff;
> } else {
> - /*
> - * Look up the extent index for the fsb where we start shifting. We can
> - * henceforth iterate with current_ext as extent list changes are locked
> - * out via ilock.
> - *
> - * If next_fsb lies in a hole beyond which there are no extents we are
> - * done.
> - */
> if (!xfs_iext_lookup_extent(ip, ifp, *next_fsb, ¤t_ext,
> &got)) {
> *done = true;
> @@ -5799,18 +5774,13 @@ xfs_bmap_insert_extents(
> }
> XFS_WANT_CORRUPTED_RETURN(mp, !isnullstartblock(got.br_startblock));
>
> - /* Lookup the extent index at which we have to stop */
> - xfs_iext_lookup_extent(ip, ifp, stop_fsb, &stop_extent, &s);
> - /* Make stop_extent exclusive of shift range */
> - stop_extent--;
> - if (current_ext <= stop_extent) {
> + if (stop_fsb >= got.br_startoff + got.br_blockcount) {
> error = -EIO;
> goto del_cursor;
> }
>
> new_startoff = got.br_startoff + offset_shift_fsb;
> - if (current_ext < total_extents - 1) {
> - xfs_iext_get_extent(ifp, current_ext + 1, &next);
> + if (xfs_iext_get_extent(ifp, current_ext + 1, &next)) {
> if (new_startoff + got.br_blockcount > next.br_startoff) {
> error = -EINVAL;
> goto del_cursor;
> @@ -5830,11 +5800,12 @@ xfs_bmap_insert_extents(
> cur, &logflags, dfops, new_startoff);
> if (error)
> goto del_cursor;
> - if (--current_ext == stop_extent) {
> +
> + if (!xfs_iext_get_extent(ifp, --current_ext, &got) ||
> + stop_fsb >= got.br_startoff + got.br_blockcount) {
> *done = true;
> goto del_cursor;
> }
> - xfs_iext_get_extent(ifp, current_ext, &got);
>
> *next_fsb = got.br_startoff;
> del_cursor:
> --
> 2.14.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH 15/15] xfs: rewrite xfs_bmap_first_unused to make better use of xfs_iext_get_extent
2017-10-19 6:59 more extent mapping cleanups Christoph Hellwig
` (13 preceding siblings ...)
2017-10-19 6:59 ` [PATCH 14/15] xfs: don't rely on extent indices in xfs_bmap_insert_extents Christoph Hellwig
@ 2017-10-19 6:59 ` Christoph Hellwig
2017-10-21 0:27 ` Darrick J. Wong
2017-10-19 20:04 ` more extent mapping cleanups Darrick J. Wong
15 siblings, 1 reply; 36+ messages in thread
From: Christoph Hellwig @ 2017-10-19 6:59 UTC (permalink / raw)
To: linux-xfs
Look at the return value of xfs_iext_get_extent instead of figuring out
the extent count first and looping up to it.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/xfs/libxfs/xfs_bmap.c | 56 ++++++++++++++++++++++--------------------------
1 file changed, 26 insertions(+), 30 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 9b638cc49f5c..46813b71dd74 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -1285,57 +1285,53 @@ xfs_bmap_read_extents(
}
/*
- * Returns the file-relative block number of the first unused block(s)
- * in the file with at least "len" logically contiguous blocks free.
- * This is the lowest-address hole if the file has holes, else the first block
- * past the end of file.
- * Return 0 if the file is currently local (in-inode).
+ * Returns the relative block number of the first unused block(s) in the given
+ * fork with at least "len" logically contiguous blocks free. This is the
+ * lowest-address hole if the fork has holes, else the first block past the end
+ * of fork. Return 0 if the fork is currently local (in-inode).
*/
int /* error */
xfs_bmap_first_unused(
- xfs_trans_t *tp, /* transaction pointer */
- xfs_inode_t *ip, /* incore inode */
- xfs_extlen_t len, /* size of hole to find */
- xfs_fileoff_t *first_unused, /* unused block */
- int whichfork) /* data or attr fork */
+ struct xfs_trans *tp, /* transaction pointer */
+ struct xfs_inode *ip, /* incore inode */
+ xfs_extlen_t len, /* size of hole to find */
+ xfs_fileoff_t *first_unused, /* unused block */
+ int whichfork) /* data or attr fork */
{
- int error; /* error return value */
- int idx; /* extent record index */
- xfs_ifork_t *ifp; /* inode fork pointer */
- xfs_fileoff_t lastaddr; /* last block number seen */
- xfs_fileoff_t lowest; /* lowest useful block */
- xfs_fileoff_t max; /* starting useful block */
- xfs_extnum_t nextents; /* number of extent entries */
+ struct xfs_ifork *ifp = XFS_IFORK_PTR(ip, whichfork);
+ struct xfs_bmbt_irec got;
+ xfs_extnum_t idx = 0;
+ xfs_fileoff_t lastaddr = 0;
+ xfs_fileoff_t lowest, max;
+ int error;
ASSERT(XFS_IFORK_FORMAT(ip, whichfork) == XFS_DINODE_FMT_BTREE ||
XFS_IFORK_FORMAT(ip, whichfork) == XFS_DINODE_FMT_EXTENTS ||
XFS_IFORK_FORMAT(ip, whichfork) == XFS_DINODE_FMT_LOCAL);
+
if (XFS_IFORK_FORMAT(ip, whichfork) == XFS_DINODE_FMT_LOCAL) {
*first_unused = 0;
return 0;
}
- ifp = XFS_IFORK_PTR(ip, whichfork);
- if (!(ifp->if_flags & XFS_IFEXTENTS) &&
- (error = xfs_iread_extents(tp, ip, whichfork)))
- return error;
- lowest = *first_unused;
- nextents = xfs_iext_count(ifp);
- for (idx = 0, lastaddr = 0, max = lowest; idx < nextents; idx++) {
- struct xfs_bmbt_irec got;
- xfs_iext_get_extent(ifp, idx, &got);
+ if (!(ifp->if_flags & XFS_IFEXTENTS)) {
+ error = xfs_iread_extents(tp, ip, whichfork);
+ if (error)
+ return error;
+ }
+ lowest = max = *first_unused;
+ while (xfs_iext_get_extent(ifp, idx++, &got)) {
/*
* See if the hole before this extent will work.
*/
if (got.br_startoff >= lowest + len &&
- got.br_startoff - max >= len) {
- *first_unused = max;
- return 0;
- }
+ got.br_startoff - max >= len)
+ break;
lastaddr = got.br_startoff + got.br_blockcount;
max = XFS_FILEOFF_MAX(lastaddr, lowest);
}
+
*first_unused = max;
return 0;
}
--
2.14.1
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [PATCH 15/15] xfs: rewrite xfs_bmap_first_unused to make better use of xfs_iext_get_extent
2017-10-19 6:59 ` [PATCH 15/15] xfs: rewrite xfs_bmap_first_unused to make better use of xfs_iext_get_extent Christoph Hellwig
@ 2017-10-21 0:27 ` Darrick J. Wong
0 siblings, 0 replies; 36+ messages in thread
From: Darrick J. Wong @ 2017-10-21 0:27 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs
On Thu, Oct 19, 2017 at 08:59:42AM +0200, Christoph Hellwig wrote:
> Look at the return value of xfs_iext_get_extent instead of figuring out
> the extent count first and looping up to it.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
Looks ok,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
> fs/xfs/libxfs/xfs_bmap.c | 56 ++++++++++++++++++++++--------------------------
> 1 file changed, 26 insertions(+), 30 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index 9b638cc49f5c..46813b71dd74 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -1285,57 +1285,53 @@ xfs_bmap_read_extents(
> }
>
> /*
> - * Returns the file-relative block number of the first unused block(s)
> - * in the file with at least "len" logically contiguous blocks free.
> - * This is the lowest-address hole if the file has holes, else the first block
> - * past the end of file.
> - * Return 0 if the file is currently local (in-inode).
> + * Returns the relative block number of the first unused block(s) in the given
> + * fork with at least "len" logically contiguous blocks free. This is the
> + * lowest-address hole if the fork has holes, else the first block past the end
> + * of fork. Return 0 if the fork is currently local (in-inode).
> */
> int /* error */
> xfs_bmap_first_unused(
> - xfs_trans_t *tp, /* transaction pointer */
> - xfs_inode_t *ip, /* incore inode */
> - xfs_extlen_t len, /* size of hole to find */
> - xfs_fileoff_t *first_unused, /* unused block */
> - int whichfork) /* data or attr fork */
> + struct xfs_trans *tp, /* transaction pointer */
> + struct xfs_inode *ip, /* incore inode */
> + xfs_extlen_t len, /* size of hole to find */
> + xfs_fileoff_t *first_unused, /* unused block */
> + int whichfork) /* data or attr fork */
> {
> - int error; /* error return value */
> - int idx; /* extent record index */
> - xfs_ifork_t *ifp; /* inode fork pointer */
> - xfs_fileoff_t lastaddr; /* last block number seen */
> - xfs_fileoff_t lowest; /* lowest useful block */
> - xfs_fileoff_t max; /* starting useful block */
> - xfs_extnum_t nextents; /* number of extent entries */
> + struct xfs_ifork *ifp = XFS_IFORK_PTR(ip, whichfork);
> + struct xfs_bmbt_irec got;
> + xfs_extnum_t idx = 0;
> + xfs_fileoff_t lastaddr = 0;
> + xfs_fileoff_t lowest, max;
> + int error;
>
> ASSERT(XFS_IFORK_FORMAT(ip, whichfork) == XFS_DINODE_FMT_BTREE ||
> XFS_IFORK_FORMAT(ip, whichfork) == XFS_DINODE_FMT_EXTENTS ||
> XFS_IFORK_FORMAT(ip, whichfork) == XFS_DINODE_FMT_LOCAL);
> +
> if (XFS_IFORK_FORMAT(ip, whichfork) == XFS_DINODE_FMT_LOCAL) {
> *first_unused = 0;
> return 0;
> }
> - ifp = XFS_IFORK_PTR(ip, whichfork);
> - if (!(ifp->if_flags & XFS_IFEXTENTS) &&
> - (error = xfs_iread_extents(tp, ip, whichfork)))
> - return error;
> - lowest = *first_unused;
> - nextents = xfs_iext_count(ifp);
> - for (idx = 0, lastaddr = 0, max = lowest; idx < nextents; idx++) {
> - struct xfs_bmbt_irec got;
>
> - xfs_iext_get_extent(ifp, idx, &got);
> + if (!(ifp->if_flags & XFS_IFEXTENTS)) {
> + error = xfs_iread_extents(tp, ip, whichfork);
> + if (error)
> + return error;
> + }
>
> + lowest = max = *first_unused;
> + while (xfs_iext_get_extent(ifp, idx++, &got)) {
> /*
> * See if the hole before this extent will work.
> */
> if (got.br_startoff >= lowest + len &&
> - got.br_startoff - max >= len) {
> - *first_unused = max;
> - return 0;
> - }
> + got.br_startoff - max >= len)
> + break;
> lastaddr = got.br_startoff + got.br_blockcount;
> max = XFS_FILEOFF_MAX(lastaddr, lowest);
> }
> +
> *first_unused = max;
> return 0;
> }
> --
> 2.14.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: more extent mapping cleanups
2017-10-19 6:59 more extent mapping cleanups Christoph Hellwig
` (14 preceding siblings ...)
2017-10-19 6:59 ` [PATCH 15/15] xfs: rewrite xfs_bmap_first_unused to make better use of xfs_iext_get_extent Christoph Hellwig
@ 2017-10-19 20:04 ` Darrick J. Wong
15 siblings, 0 replies; 36+ messages in thread
From: Darrick J. Wong @ 2017-10-19 20:04 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs
On Thu, Oct 19, 2017 at 08:59:27AM +0200, Christoph Hellwig wrote:
> Hi all,
>
> This series contains another round of extent map related cleanups
> to prepare for the new incore extent list.
>
> Because it is based on a huge number of outstanding (and reviewed)
> patches it might be a good idea to pull in the git tree for testing
> or detailed review:
Looks ok on first glance, will feed it to the testing cloud while I take
a closer look this afternoon.
--D
>
> git://git.infradead.org/users/hch/xfs.git xfs-bmbt-cleanup.2
>
> Gitweb:
>
> http://git.infradead.org/users/hch/xfs.git/shortlog/refs/heads/xfs-bmbt-cleanup.2
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 36+ messages in thread