* [PATCH 1/6] xfs: remove if_real_bytes
2018-07-10 6:05 reduce lookups in the COW extent tree Christoph Hellwig
@ 2018-07-10 6:05 ` Christoph Hellwig
2018-07-11 16:22 ` Darrick J. Wong
2018-07-10 6:05 ` [PATCH 2/6] xfs: simplify xfs_idata_realloc Christoph Hellwig
` (4 subsequent siblings)
5 siblings, 1 reply; 17+ messages in thread
From: Christoph Hellwig @ 2018-07-10 6:05 UTC (permalink / raw)
To: linux-xfs
The field is only used for asserts, and to track if we really need to do
realloc when growing the inode fork data. But the krealloc function
already performs this check internally, so there is no need to keep track
of the real allocation size.
This will free space in the inode fork for keeping a sequence counter of
changes to the extent list.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/xfs/libxfs/xfs_inode_fork.c | 19 ++++---------------
fs/xfs/libxfs/xfs_inode_fork.h | 1 -
fs/xfs/xfs_inode.c | 3 +--
fs/xfs/xfs_inode_item.c | 4 ----
4 files changed, 5 insertions(+), 22 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_inode_fork.c b/fs/xfs/libxfs/xfs_inode_fork.c
index 183ec0cb8921..dee85b0f8846 100644
--- a/fs/xfs/libxfs/xfs_inode_fork.c
+++ b/fs/xfs/libxfs/xfs_inode_fork.c
@@ -158,7 +158,6 @@ xfs_init_local_fork(
}
ifp->if_bytes = size;
- ifp->if_real_bytes = real_size;
ifp->if_flags &= ~(XFS_IFEXTENTS | XFS_IFBROOT);
ifp->if_flags |= XFS_IFINLINE;
}
@@ -226,7 +225,6 @@ xfs_iformat_extents(
return -EFSCORRUPTED;
}
- ifp->if_real_bytes = 0;
ifp->if_bytes = 0;
ifp->if_u1.if_root = NULL;
ifp->if_height = 0;
@@ -317,7 +315,6 @@ xfs_iformat_btree(
ifp->if_flags &= ~XFS_IFEXTENTS;
ifp->if_flags |= XFS_IFBROOT;
- ifp->if_real_bytes = 0;
ifp->if_bytes = 0;
ifp->if_u1.if_root = NULL;
ifp->if_height = 0;
@@ -501,7 +498,6 @@ xfs_idata_realloc(
*/
real_size = roundup(new_size, 4);
if (ifp->if_u1.if_data == NULL) {
- ASSERT(ifp->if_real_bytes == 0);
ifp->if_u1.if_data = kmem_alloc(real_size,
KM_SLEEP | KM_NOFS);
} else {
@@ -509,15 +505,12 @@ xfs_idata_realloc(
* Only do the realloc if the underlying size
* is really changing.
*/
- if (ifp->if_real_bytes != real_size) {
- ifp->if_u1.if_data =
- kmem_realloc(ifp->if_u1.if_data,
- real_size,
- KM_SLEEP | KM_NOFS);
- }
+ ifp->if_u1.if_data =
+ kmem_realloc(ifp->if_u1.if_data,
+ real_size,
+ KM_SLEEP | KM_NOFS);
}
}
- ifp->if_real_bytes = real_size;
ifp->if_bytes = new_size;
ASSERT(ifp->if_bytes <= XFS_IFORK_SIZE(ip, whichfork));
}
@@ -543,17 +536,13 @@ xfs_idestroy_fork(
*/
if (XFS_IFORK_FORMAT(ip, whichfork) == XFS_DINODE_FMT_LOCAL) {
if (ifp->if_u1.if_data != NULL) {
- ASSERT(ifp->if_real_bytes != 0);
kmem_free(ifp->if_u1.if_data);
ifp->if_u1.if_data = NULL;
- ifp->if_real_bytes = 0;
}
} else if ((ifp->if_flags & XFS_IFEXTENTS) && ifp->if_height) {
xfs_iext_destroy(ifp);
}
- ASSERT(ifp->if_real_bytes == 0);
-
if (whichfork == XFS_ATTR_FORK) {
kmem_zone_free(xfs_ifork_zone, ip->i_afp);
ip->i_afp = NULL;
diff --git a/fs/xfs/libxfs/xfs_inode_fork.h b/fs/xfs/libxfs/xfs_inode_fork.h
index 781b1603df5e..46242052aad0 100644
--- a/fs/xfs/libxfs/xfs_inode_fork.h
+++ b/fs/xfs/libxfs/xfs_inode_fork.h
@@ -14,7 +14,6 @@ struct xfs_dinode;
*/
typedef struct xfs_ifork {
int if_bytes; /* bytes in if_u1 */
- int if_real_bytes; /* bytes allocated in if_u1 */
struct xfs_btree_block *if_broot; /* file's incore btree root */
short if_broot_bytes; /* bytes allocated for root */
unsigned char if_flags; /* per-fork flags */
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index 5df4de666cc1..b6da446ae946 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -927,7 +927,7 @@ xfs_ialloc(
case S_IFLNK:
ip->i_d.di_format = XFS_DINODE_FMT_EXTENTS;
ip->i_df.if_flags = XFS_IFEXTENTS;
- ip->i_df.if_bytes = ip->i_df.if_real_bytes = 0;
+ ip->i_df.if_bytes = 0;
ip->i_df.if_u1.if_root = NULL;
break;
default:
@@ -1877,7 +1877,6 @@ xfs_inactive(
* to clean up here.
*/
if (VFS_I(ip)->i_mode == 0) {
- ASSERT(ip->i_df.if_real_bytes == 0);
ASSERT(ip->i_df.if_broot_bytes == 0);
return;
}
diff --git a/fs/xfs/xfs_inode_item.c b/fs/xfs/xfs_inode_item.c
index 2389c34c172d..fa1c4fe2ffbf 100644
--- a/fs/xfs/xfs_inode_item.c
+++ b/fs/xfs/xfs_inode_item.c
@@ -194,8 +194,6 @@ xfs_inode_item_format_data_fork(
* to be there by xfs_idata_realloc().
*/
data_bytes = roundup(ip->i_df.if_bytes, 4);
- ASSERT(ip->i_df.if_real_bytes == 0 ||
- ip->i_df.if_real_bytes >= data_bytes);
ASSERT(ip->i_df.if_u1.if_data != NULL);
ASSERT(ip->i_d.di_size > 0);
xlog_copy_iovec(lv, vecp, XLOG_REG_TYPE_ILOCAL,
@@ -280,8 +278,6 @@ xfs_inode_item_format_attr_fork(
* to be there by xfs_idata_realloc().
*/
data_bytes = roundup(ip->i_afp->if_bytes, 4);
- ASSERT(ip->i_afp->if_real_bytes == 0 ||
- ip->i_afp->if_real_bytes >= data_bytes);
ASSERT(ip->i_afp->if_u1.if_data != NULL);
xlog_copy_iovec(lv, vecp, XLOG_REG_TYPE_IATTR_LOCAL,
ip->i_afp->if_u1.if_data,
--
2.18.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 1/6] xfs: remove if_real_bytes
2018-07-10 6:05 ` [PATCH 1/6] xfs: remove if_real_bytes Christoph Hellwig
@ 2018-07-11 16:22 ` Darrick J. Wong
0 siblings, 0 replies; 17+ messages in thread
From: Darrick J. Wong @ 2018-07-11 16:22 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs
On Tue, Jul 10, 2018 at 08:05:23AM +0200, Christoph Hellwig wrote:
> The field is only used for asserts, and to track if we really need to do
> realloc when growing the inode fork data. But the krealloc function
> already performs this check internally, so there is no need to keep track
> of the real allocation size.
>
> This will free space in the inode fork for keeping a sequence counter of
> changes to the extent list.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
Looks ok,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
--D
> ---
> fs/xfs/libxfs/xfs_inode_fork.c | 19 ++++---------------
> fs/xfs/libxfs/xfs_inode_fork.h | 1 -
> fs/xfs/xfs_inode.c | 3 +--
> fs/xfs/xfs_inode_item.c | 4 ----
> 4 files changed, 5 insertions(+), 22 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_inode_fork.c b/fs/xfs/libxfs/xfs_inode_fork.c
> index 183ec0cb8921..dee85b0f8846 100644
> --- a/fs/xfs/libxfs/xfs_inode_fork.c
> +++ b/fs/xfs/libxfs/xfs_inode_fork.c
> @@ -158,7 +158,6 @@ xfs_init_local_fork(
> }
>
> ifp->if_bytes = size;
> - ifp->if_real_bytes = real_size;
> ifp->if_flags &= ~(XFS_IFEXTENTS | XFS_IFBROOT);
> ifp->if_flags |= XFS_IFINLINE;
> }
> @@ -226,7 +225,6 @@ xfs_iformat_extents(
> return -EFSCORRUPTED;
> }
>
> - ifp->if_real_bytes = 0;
> ifp->if_bytes = 0;
> ifp->if_u1.if_root = NULL;
> ifp->if_height = 0;
> @@ -317,7 +315,6 @@ xfs_iformat_btree(
> ifp->if_flags &= ~XFS_IFEXTENTS;
> ifp->if_flags |= XFS_IFBROOT;
>
> - ifp->if_real_bytes = 0;
> ifp->if_bytes = 0;
> ifp->if_u1.if_root = NULL;
> ifp->if_height = 0;
> @@ -501,7 +498,6 @@ xfs_idata_realloc(
> */
> real_size = roundup(new_size, 4);
> if (ifp->if_u1.if_data == NULL) {
> - ASSERT(ifp->if_real_bytes == 0);
> ifp->if_u1.if_data = kmem_alloc(real_size,
> KM_SLEEP | KM_NOFS);
> } else {
> @@ -509,15 +505,12 @@ xfs_idata_realloc(
> * Only do the realloc if the underlying size
> * is really changing.
> */
> - if (ifp->if_real_bytes != real_size) {
> - ifp->if_u1.if_data =
> - kmem_realloc(ifp->if_u1.if_data,
> - real_size,
> - KM_SLEEP | KM_NOFS);
> - }
> + ifp->if_u1.if_data =
> + kmem_realloc(ifp->if_u1.if_data,
> + real_size,
> + KM_SLEEP | KM_NOFS);
> }
> }
> - ifp->if_real_bytes = real_size;
> ifp->if_bytes = new_size;
> ASSERT(ifp->if_bytes <= XFS_IFORK_SIZE(ip, whichfork));
> }
> @@ -543,17 +536,13 @@ xfs_idestroy_fork(
> */
> if (XFS_IFORK_FORMAT(ip, whichfork) == XFS_DINODE_FMT_LOCAL) {
> if (ifp->if_u1.if_data != NULL) {
> - ASSERT(ifp->if_real_bytes != 0);
> kmem_free(ifp->if_u1.if_data);
> ifp->if_u1.if_data = NULL;
> - ifp->if_real_bytes = 0;
> }
> } else if ((ifp->if_flags & XFS_IFEXTENTS) && ifp->if_height) {
> xfs_iext_destroy(ifp);
> }
>
> - ASSERT(ifp->if_real_bytes == 0);
> -
> if (whichfork == XFS_ATTR_FORK) {
> kmem_zone_free(xfs_ifork_zone, ip->i_afp);
> ip->i_afp = NULL;
> diff --git a/fs/xfs/libxfs/xfs_inode_fork.h b/fs/xfs/libxfs/xfs_inode_fork.h
> index 781b1603df5e..46242052aad0 100644
> --- a/fs/xfs/libxfs/xfs_inode_fork.h
> +++ b/fs/xfs/libxfs/xfs_inode_fork.h
> @@ -14,7 +14,6 @@ struct xfs_dinode;
> */
> typedef struct xfs_ifork {
> int if_bytes; /* bytes in if_u1 */
> - int if_real_bytes; /* bytes allocated in if_u1 */
> struct xfs_btree_block *if_broot; /* file's incore btree root */
> short if_broot_bytes; /* bytes allocated for root */
> unsigned char if_flags; /* per-fork flags */
> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> index 5df4de666cc1..b6da446ae946 100644
> --- a/fs/xfs/xfs_inode.c
> +++ b/fs/xfs/xfs_inode.c
> @@ -927,7 +927,7 @@ xfs_ialloc(
> case S_IFLNK:
> ip->i_d.di_format = XFS_DINODE_FMT_EXTENTS;
> ip->i_df.if_flags = XFS_IFEXTENTS;
> - ip->i_df.if_bytes = ip->i_df.if_real_bytes = 0;
> + ip->i_df.if_bytes = 0;
> ip->i_df.if_u1.if_root = NULL;
> break;
> default:
> @@ -1877,7 +1877,6 @@ xfs_inactive(
> * to clean up here.
> */
> if (VFS_I(ip)->i_mode == 0) {
> - ASSERT(ip->i_df.if_real_bytes == 0);
> ASSERT(ip->i_df.if_broot_bytes == 0);
> return;
> }
> diff --git a/fs/xfs/xfs_inode_item.c b/fs/xfs/xfs_inode_item.c
> index 2389c34c172d..fa1c4fe2ffbf 100644
> --- a/fs/xfs/xfs_inode_item.c
> +++ b/fs/xfs/xfs_inode_item.c
> @@ -194,8 +194,6 @@ xfs_inode_item_format_data_fork(
> * to be there by xfs_idata_realloc().
> */
> data_bytes = roundup(ip->i_df.if_bytes, 4);
> - ASSERT(ip->i_df.if_real_bytes == 0 ||
> - ip->i_df.if_real_bytes >= data_bytes);
> ASSERT(ip->i_df.if_u1.if_data != NULL);
> ASSERT(ip->i_d.di_size > 0);
> xlog_copy_iovec(lv, vecp, XLOG_REG_TYPE_ILOCAL,
> @@ -280,8 +278,6 @@ xfs_inode_item_format_attr_fork(
> * to be there by xfs_idata_realloc().
> */
> data_bytes = roundup(ip->i_afp->if_bytes, 4);
> - ASSERT(ip->i_afp->if_real_bytes == 0 ||
> - ip->i_afp->if_real_bytes >= data_bytes);
> ASSERT(ip->i_afp->if_u1.if_data != NULL);
> xlog_copy_iovec(lv, vecp, XLOG_REG_TYPE_IATTR_LOCAL,
> ip->i_afp->if_u1.if_data,
> --
> 2.18.0
>
> --
> 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] 17+ messages in thread
* [PATCH 2/6] xfs: simplify xfs_idata_realloc
2018-07-10 6:05 reduce lookups in the COW extent tree Christoph Hellwig
2018-07-10 6:05 ` [PATCH 1/6] xfs: remove if_real_bytes Christoph Hellwig
@ 2018-07-10 6:05 ` Christoph Hellwig
2018-07-11 16:23 ` Darrick J. Wong
2018-07-10 6:05 ` [PATCH 3/6] xfs: remove the xfs_ifork_t typedef Christoph Hellwig
` (3 subsequent siblings)
5 siblings, 1 reply; 17+ messages in thread
From: Christoph Hellwig @ 2018-07-10 6:05 UTC (permalink / raw)
To: linux-xfs
Streamline the code and take advantage of the fact that kmem_realloc
through krealloc will be have like a normal allocation if passing in a
NULL old pointer.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/xfs/libxfs/xfs_inode_fork.c | 55 ++++++++++++----------------------
1 file changed, 19 insertions(+), 36 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_inode_fork.c b/fs/xfs/libxfs/xfs_inode_fork.c
index dee85b0f8846..a0e3fb804605 100644
--- a/fs/xfs/libxfs/xfs_inode_fork.c
+++ b/fs/xfs/libxfs/xfs_inode_fork.c
@@ -468,51 +468,34 @@ xfs_iroot_realloc(
*/
void
xfs_idata_realloc(
- xfs_inode_t *ip,
- int byte_diff,
- int whichfork)
+ struct xfs_inode *ip,
+ int byte_diff,
+ int whichfork)
{
- xfs_ifork_t *ifp;
- int new_size;
- int real_size;
-
- if (byte_diff == 0) {
- return;
- }
+ struct xfs_ifork *ifp = XFS_IFORK_PTR(ip, whichfork);
+ int new_size = (int)ifp->if_bytes + byte_diff;
- ifp = XFS_IFORK_PTR(ip, whichfork);
- new_size = (int)ifp->if_bytes + byte_diff;
ASSERT(new_size >= 0);
+ ASSERT(new_size <= XFS_IFORK_SIZE(ip, whichfork));
+
+ if (byte_diff == 0)
+ return;
if (new_size == 0) {
kmem_free(ifp->if_u1.if_data);
ifp->if_u1.if_data = NULL;
- real_size = 0;
- } else {
- /*
- * Stuck with malloc/realloc.
- * For inline data, the underlying buffer must be
- * a multiple of 4 bytes in size so that it can be
- * logged and stay on word boundaries. We enforce
- * that here.
- */
- real_size = roundup(new_size, 4);
- if (ifp->if_u1.if_data == NULL) {
- ifp->if_u1.if_data = kmem_alloc(real_size,
- KM_SLEEP | KM_NOFS);
- } else {
- /*
- * Only do the realloc if the underlying size
- * is really changing.
- */
- ifp->if_u1.if_data =
- kmem_realloc(ifp->if_u1.if_data,
- real_size,
- KM_SLEEP | KM_NOFS);
- }
+ ifp->if_bytes = 0;
+ return;
}
+
+ /*
+ * For inline data, the underlying buffer must be a multiple of 4 bytes
+ * in size so that it can be logged and stay on word boundaries.
+ * We enforce that here.
+ */
+ ifp->if_u1.if_data = kmem_realloc(ifp->if_u1.if_data,
+ roundup(new_size, 4), KM_SLEEP | KM_NOFS);
ifp->if_bytes = new_size;
- ASSERT(ifp->if_bytes <= XFS_IFORK_SIZE(ip, whichfork));
}
void
--
2.18.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 2/6] xfs: simplify xfs_idata_realloc
2018-07-10 6:05 ` [PATCH 2/6] xfs: simplify xfs_idata_realloc Christoph Hellwig
@ 2018-07-11 16:23 ` Darrick J. Wong
0 siblings, 0 replies; 17+ messages in thread
From: Darrick J. Wong @ 2018-07-11 16:23 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs
On Tue, Jul 10, 2018 at 08:05:24AM +0200, Christoph Hellwig wrote:
> Streamline the code and take advantage of the fact that kmem_realloc
> through krealloc will be have like a normal allocation if passing in a
> NULL old pointer.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
Looks good to me,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
--D
> ---
> fs/xfs/libxfs/xfs_inode_fork.c | 55 ++++++++++++----------------------
> 1 file changed, 19 insertions(+), 36 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_inode_fork.c b/fs/xfs/libxfs/xfs_inode_fork.c
> index dee85b0f8846..a0e3fb804605 100644
> --- a/fs/xfs/libxfs/xfs_inode_fork.c
> +++ b/fs/xfs/libxfs/xfs_inode_fork.c
> @@ -468,51 +468,34 @@ xfs_iroot_realloc(
> */
> void
> xfs_idata_realloc(
> - xfs_inode_t *ip,
> - int byte_diff,
> - int whichfork)
> + struct xfs_inode *ip,
> + int byte_diff,
> + int whichfork)
> {
> - xfs_ifork_t *ifp;
> - int new_size;
> - int real_size;
> -
> - if (byte_diff == 0) {
> - return;
> - }
> + struct xfs_ifork *ifp = XFS_IFORK_PTR(ip, whichfork);
> + int new_size = (int)ifp->if_bytes + byte_diff;
>
> - ifp = XFS_IFORK_PTR(ip, whichfork);
> - new_size = (int)ifp->if_bytes + byte_diff;
> ASSERT(new_size >= 0);
> + ASSERT(new_size <= XFS_IFORK_SIZE(ip, whichfork));
> +
> + if (byte_diff == 0)
> + return;
>
> if (new_size == 0) {
> kmem_free(ifp->if_u1.if_data);
> ifp->if_u1.if_data = NULL;
> - real_size = 0;
> - } else {
> - /*
> - * Stuck with malloc/realloc.
> - * For inline data, the underlying buffer must be
> - * a multiple of 4 bytes in size so that it can be
> - * logged and stay on word boundaries. We enforce
> - * that here.
> - */
> - real_size = roundup(new_size, 4);
> - if (ifp->if_u1.if_data == NULL) {
> - ifp->if_u1.if_data = kmem_alloc(real_size,
> - KM_SLEEP | KM_NOFS);
> - } else {
> - /*
> - * Only do the realloc if the underlying size
> - * is really changing.
> - */
> - ifp->if_u1.if_data =
> - kmem_realloc(ifp->if_u1.if_data,
> - real_size,
> - KM_SLEEP | KM_NOFS);
> - }
> + ifp->if_bytes = 0;
> + return;
> }
> +
> + /*
> + * For inline data, the underlying buffer must be a multiple of 4 bytes
> + * in size so that it can be logged and stay on word boundaries.
> + * We enforce that here.
> + */
> + ifp->if_u1.if_data = kmem_realloc(ifp->if_u1.if_data,
> + roundup(new_size, 4), KM_SLEEP | KM_NOFS);
> ifp->if_bytes = new_size;
> - ASSERT(ifp->if_bytes <= XFS_IFORK_SIZE(ip, whichfork));
> }
>
> void
> --
> 2.18.0
>
> --
> 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] 17+ messages in thread
* [PATCH 3/6] xfs: remove the xfs_ifork_t typedef
2018-07-10 6:05 reduce lookups in the COW extent tree Christoph Hellwig
2018-07-10 6:05 ` [PATCH 1/6] xfs: remove if_real_bytes Christoph Hellwig
2018-07-10 6:05 ` [PATCH 2/6] xfs: simplify xfs_idata_realloc Christoph Hellwig
@ 2018-07-10 6:05 ` Christoph Hellwig
2018-07-11 16:23 ` Darrick J. Wong
2018-07-10 6:05 ` [PATCH 4/6] xfs: introduce a new xfs_inode_has_cow_data helper Christoph Hellwig
` (2 subsequent siblings)
5 siblings, 1 reply; 17+ messages in thread
From: Christoph Hellwig @ 2018-07-10 6:05 UTC (permalink / raw)
To: linux-xfs
We only have a few more callers left, so seize the opportunity and kill
it off.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/xfs/libxfs/xfs_attr_leaf.c | 8 ++++----
fs/xfs/libxfs/xfs_bmap.c | 20 ++++++++++----------
fs/xfs/libxfs/xfs_inode_fork.c | 8 ++++----
fs/xfs/libxfs/xfs_inode_fork.h | 4 ++--
fs/xfs/xfs_icache.c | 2 +-
fs/xfs/xfs_inode.h | 6 +++---
fs/xfs/xfs_super.c | 2 +-
7 files changed, 25 insertions(+), 25 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
index 76e90046731c..3030fba4785c 100644
--- a/fs/xfs/libxfs/xfs_attr_leaf.c
+++ b/fs/xfs/libxfs/xfs_attr_leaf.c
@@ -506,7 +506,7 @@ xfs_attr_shortform_create(xfs_da_args_t *args)
{
xfs_attr_sf_hdr_t *hdr;
xfs_inode_t *dp;
- xfs_ifork_t *ifp;
+ struct xfs_ifork *ifp;
trace_xfs_attr_sf_create(args);
@@ -541,7 +541,7 @@ xfs_attr_shortform_add(xfs_da_args_t *args, int forkoff)
int i, offset, size;
xfs_mount_t *mp;
xfs_inode_t *dp;
- xfs_ifork_t *ifp;
+ struct xfs_ifork *ifp;
trace_xfs_attr_sf_add(args);
@@ -682,7 +682,7 @@ xfs_attr_shortform_lookup(xfs_da_args_t *args)
xfs_attr_shortform_t *sf;
xfs_attr_sf_entry_t *sfe;
int i;
- xfs_ifork_t *ifp;
+ struct xfs_ifork *ifp;
trace_xfs_attr_sf_lookup(args);
@@ -758,7 +758,7 @@ xfs_attr_shortform_to_leaf(
int error, i, size;
xfs_dablk_t blkno;
struct xfs_buf *bp;
- xfs_ifork_t *ifp;
+ struct xfs_ifork *ifp;
trace_xfs_attr_sf_to_leaf(args);
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 68ea1f4b9c3f..7ac8f90bd218 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -326,7 +326,7 @@ xfs_bmap_check_leaf_extents(
xfs_buf_t *bp; /* buffer for "block" */
int error; /* error return value */
xfs_extnum_t i=0, j; /* index into the extents list */
- xfs_ifork_t *ifp; /* fork structure */
+ struct xfs_ifork *ifp; /* fork structure */
int level; /* btree level, for checking */
xfs_mount_t *mp; /* file system mount structure */
__be64 *pp; /* pointer to block address */
@@ -594,7 +594,7 @@ xfs_bmap_btree_to_extents(
xfs_fsblock_t cbno; /* child block number */
xfs_buf_t *cbp; /* child block's buffer */
int error; /* error return value */
- xfs_ifork_t *ifp; /* inode fork data */
+ struct xfs_ifork *ifp; /* inode fork data */
xfs_mount_t *mp; /* mount point structure */
__be64 *pp; /* ptr to block address */
struct xfs_btree_block *rblock;/* root btree block */
@@ -660,7 +660,7 @@ xfs_bmap_extents_to_btree(
struct xfs_btree_block *block; /* btree root block */
xfs_btree_cur_t *cur; /* bmap btree cursor */
int error; /* error return value */
- xfs_ifork_t *ifp; /* inode fork pointer */
+ struct xfs_ifork *ifp; /* inode fork pointer */
xfs_bmbt_key_t *kp; /* root block key pointer */
xfs_mount_t *mp; /* mount structure */
xfs_bmbt_ptr_t *pp; /* root block address pointer */
@@ -823,7 +823,7 @@ xfs_bmap_local_to_extents(
{
int error = 0;
int flags; /* logging flags returned */
- xfs_ifork_t *ifp; /* inode fork pointer */
+ struct xfs_ifork *ifp; /* inode fork pointer */
xfs_alloc_arg_t args; /* allocation arguments */
xfs_buf_t *bp; /* buffer for extent block */
struct xfs_bmbt_irec rec;
@@ -1501,7 +1501,7 @@ xfs_bmap_one_block(
xfs_inode_t *ip, /* incore inode */
int whichfork) /* data or attr fork */
{
- xfs_ifork_t *ifp; /* inode fork pointer */
+ struct xfs_ifork *ifp; /* inode fork pointer */
int rval; /* return value */
xfs_bmbt_irec_t s; /* internal version of extent */
struct xfs_iext_cursor icur;
@@ -1539,7 +1539,7 @@ xfs_bmap_add_extent_delay_real(
struct xfs_bmbt_irec *new = &bma->got;
int error; /* error return value */
int i; /* temp state */
- xfs_ifork_t *ifp; /* inode fork pointer */
+ struct xfs_ifork *ifp; /* inode fork pointer */
xfs_fileoff_t new_endoff; /* end offset of new entry */
xfs_bmbt_irec_t r[3]; /* neighbor extent entries */
/* left is 0, right is 1, prev is 2 */
@@ -2053,7 +2053,7 @@ xfs_bmap_add_extent_unwritten_real(
xfs_btree_cur_t *cur; /* btree cursor */
int error; /* error return value */
int i; /* temp state */
- xfs_ifork_t *ifp; /* inode fork pointer */
+ struct xfs_ifork *ifp; /* inode fork pointer */
xfs_fileoff_t new_endoff; /* end offset of new entry */
xfs_bmbt_irec_t r[3]; /* neighbor extent entries */
/* left is 0, right is 1, prev is 2 */
@@ -2520,7 +2520,7 @@ xfs_bmap_add_extent_hole_delay(
struct xfs_iext_cursor *icur,
xfs_bmbt_irec_t *new) /* new data to add to file extents */
{
- xfs_ifork_t *ifp; /* inode fork pointer */
+ struct xfs_ifork *ifp; /* inode fork pointer */
xfs_bmbt_irec_t left; /* left neighbor extent entry */
xfs_filblks_t newlen=0; /* new indirect size */
xfs_filblks_t oldlen=0; /* old indirect size */
@@ -4911,7 +4911,7 @@ xfs_bmap_del_extent_real(
struct xfs_bmbt_irec got; /* current extent entry */
xfs_fileoff_t got_endoff; /* first offset past got */
int i; /* temp state */
- xfs_ifork_t *ifp; /* inode fork pointer */
+ struct xfs_ifork *ifp; /* inode fork pointer */
xfs_mount_t *mp; /* mount structure */
xfs_filblks_t nblks; /* quota/sb block count */
xfs_bmbt_irec_t new; /* new record to be inserted */
@@ -5161,7 +5161,7 @@ __xfs_bunmapi(
int error; /* error return value */
xfs_extnum_t extno; /* extent number in list */
xfs_bmbt_irec_t got; /* current extent record */
- xfs_ifork_t *ifp; /* inode fork pointer */
+ struct xfs_ifork *ifp; /* inode fork pointer */
int isrt; /* freeing in rt area */
int logflags; /* transaction logging flags */
xfs_extlen_t mod; /* rt extent offset */
diff --git a/fs/xfs/libxfs/xfs_inode_fork.c b/fs/xfs/libxfs/xfs_inode_fork.c
index a0e3fb804605..f9acf1d436f6 100644
--- a/fs/xfs/libxfs/xfs_inode_fork.c
+++ b/fs/xfs/libxfs/xfs_inode_fork.c
@@ -269,7 +269,7 @@ xfs_iformat_btree(
{
struct xfs_mount *mp = ip->i_mount;
xfs_bmdr_block_t *dfp;
- xfs_ifork_t *ifp;
+ struct xfs_ifork *ifp;
/* REFERENCED */
int nrecs;
int size;
@@ -347,7 +347,7 @@ xfs_iroot_realloc(
{
struct xfs_mount *mp = ip->i_mount;
int cur_max;
- xfs_ifork_t *ifp;
+ struct xfs_ifork *ifp;
struct xfs_btree_block *new_broot;
int new_max;
size_t new_size;
@@ -503,7 +503,7 @@ xfs_idestroy_fork(
xfs_inode_t *ip,
int whichfork)
{
- xfs_ifork_t *ifp;
+ struct xfs_ifork *ifp;
ifp = XFS_IFORK_PTR(ip, whichfork);
if (ifp->if_broot != NULL) {
@@ -592,7 +592,7 @@ xfs_iflush_fork(
int whichfork)
{
char *cp;
- xfs_ifork_t *ifp;
+ struct xfs_ifork *ifp;
xfs_mount_t *mp;
static const short brootflag[2] =
{ XFS_ILOG_DBROOT, XFS_ILOG_ABROOT };
diff --git a/fs/xfs/libxfs/xfs_inode_fork.h b/fs/xfs/libxfs/xfs_inode_fork.h
index 46242052aad0..1492143371f3 100644
--- a/fs/xfs/libxfs/xfs_inode_fork.h
+++ b/fs/xfs/libxfs/xfs_inode_fork.h
@@ -12,7 +12,7 @@ struct xfs_dinode;
/*
* File incore extent information, present for each of data & attr forks.
*/
-typedef struct xfs_ifork {
+struct xfs_ifork {
int if_bytes; /* bytes in if_u1 */
struct xfs_btree_block *if_broot; /* file's incore btree root */
short if_broot_bytes; /* bytes allocated for root */
@@ -22,7 +22,7 @@ typedef struct xfs_ifork {
void *if_root; /* extent tree root */
char *if_data; /* inline file data */
} if_u1;
-} xfs_ifork_t;
+};
/*
* Per-fork incore inode flags.
diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c
index 47f417d20a30..79f344fa8b14 100644
--- a/fs/xfs/xfs_icache.c
+++ b/fs/xfs/xfs_icache.c
@@ -66,7 +66,7 @@ xfs_inode_alloc(
ip->i_cowfp = NULL;
ip->i_cnextents = 0;
ip->i_cformat = XFS_DINODE_FMT_EXTENTS;
- memset(&ip->i_df, 0, sizeof(xfs_ifork_t));
+ memset(&ip->i_df, 0, sizeof(ip->i_df));
ip->i_flags = 0;
ip->i_delayed_blks = 0;
memset(&ip->i_d, 0, sizeof(ip->i_d));
diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h
index 2ed63a49e890..1f910d2ae73a 100644
--- a/fs/xfs/xfs_inode.h
+++ b/fs/xfs/xfs_inode.h
@@ -34,9 +34,9 @@ typedef struct xfs_inode {
struct xfs_imap i_imap; /* location for xfs_imap() */
/* Extent information. */
- xfs_ifork_t *i_afp; /* attribute fork pointer */
- xfs_ifork_t *i_cowfp; /* copy on write extents */
- xfs_ifork_t i_df; /* data fork */
+ struct xfs_ifork *i_afp; /* attribute fork pointer */
+ struct xfs_ifork *i_cowfp; /* copy on write extents */
+ struct xfs_ifork i_df; /* data fork */
/* operations vectors */
const struct xfs_dir_ops *d_ops; /* directory ops vector */
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index f9f8dc490d3d..62312b4cef64 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -1886,7 +1886,7 @@ xfs_init_zones(void)
if (!xfs_da_state_zone)
goto out_destroy_btree_cur_zone;
- xfs_ifork_zone = kmem_zone_init(sizeof(xfs_ifork_t), "xfs_ifork");
+ xfs_ifork_zone = kmem_zone_init(sizeof(struct xfs_ifork), "xfs_ifork");
if (!xfs_ifork_zone)
goto out_destroy_da_state_zone;
--
2.18.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 3/6] xfs: remove the xfs_ifork_t typedef
2018-07-10 6:05 ` [PATCH 3/6] xfs: remove the xfs_ifork_t typedef Christoph Hellwig
@ 2018-07-11 16:23 ` Darrick J. Wong
0 siblings, 0 replies; 17+ messages in thread
From: Darrick J. Wong @ 2018-07-11 16:23 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs
On Tue, Jul 10, 2018 at 08:05:25AM +0200, Christoph Hellwig wrote:
> We only have a few more callers left, so seize the opportunity and kill
> it off.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
Wooooooo!
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
--D
> ---
> fs/xfs/libxfs/xfs_attr_leaf.c | 8 ++++----
> fs/xfs/libxfs/xfs_bmap.c | 20 ++++++++++----------
> fs/xfs/libxfs/xfs_inode_fork.c | 8 ++++----
> fs/xfs/libxfs/xfs_inode_fork.h | 4 ++--
> fs/xfs/xfs_icache.c | 2 +-
> fs/xfs/xfs_inode.h | 6 +++---
> fs/xfs/xfs_super.c | 2 +-
> 7 files changed, 25 insertions(+), 25 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
> index 76e90046731c..3030fba4785c 100644
> --- a/fs/xfs/libxfs/xfs_attr_leaf.c
> +++ b/fs/xfs/libxfs/xfs_attr_leaf.c
> @@ -506,7 +506,7 @@ xfs_attr_shortform_create(xfs_da_args_t *args)
> {
> xfs_attr_sf_hdr_t *hdr;
> xfs_inode_t *dp;
> - xfs_ifork_t *ifp;
> + struct xfs_ifork *ifp;
>
> trace_xfs_attr_sf_create(args);
>
> @@ -541,7 +541,7 @@ xfs_attr_shortform_add(xfs_da_args_t *args, int forkoff)
> int i, offset, size;
> xfs_mount_t *mp;
> xfs_inode_t *dp;
> - xfs_ifork_t *ifp;
> + struct xfs_ifork *ifp;
>
> trace_xfs_attr_sf_add(args);
>
> @@ -682,7 +682,7 @@ xfs_attr_shortform_lookup(xfs_da_args_t *args)
> xfs_attr_shortform_t *sf;
> xfs_attr_sf_entry_t *sfe;
> int i;
> - xfs_ifork_t *ifp;
> + struct xfs_ifork *ifp;
>
> trace_xfs_attr_sf_lookup(args);
>
> @@ -758,7 +758,7 @@ xfs_attr_shortform_to_leaf(
> int error, i, size;
> xfs_dablk_t blkno;
> struct xfs_buf *bp;
> - xfs_ifork_t *ifp;
> + struct xfs_ifork *ifp;
>
> trace_xfs_attr_sf_to_leaf(args);
>
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index 68ea1f4b9c3f..7ac8f90bd218 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -326,7 +326,7 @@ xfs_bmap_check_leaf_extents(
> xfs_buf_t *bp; /* buffer for "block" */
> int error; /* error return value */
> xfs_extnum_t i=0, j; /* index into the extents list */
> - xfs_ifork_t *ifp; /* fork structure */
> + struct xfs_ifork *ifp; /* fork structure */
> int level; /* btree level, for checking */
> xfs_mount_t *mp; /* file system mount structure */
> __be64 *pp; /* pointer to block address */
> @@ -594,7 +594,7 @@ xfs_bmap_btree_to_extents(
> xfs_fsblock_t cbno; /* child block number */
> xfs_buf_t *cbp; /* child block's buffer */
> int error; /* error return value */
> - xfs_ifork_t *ifp; /* inode fork data */
> + struct xfs_ifork *ifp; /* inode fork data */
> xfs_mount_t *mp; /* mount point structure */
> __be64 *pp; /* ptr to block address */
> struct xfs_btree_block *rblock;/* root btree block */
> @@ -660,7 +660,7 @@ xfs_bmap_extents_to_btree(
> struct xfs_btree_block *block; /* btree root block */
> xfs_btree_cur_t *cur; /* bmap btree cursor */
> int error; /* error return value */
> - xfs_ifork_t *ifp; /* inode fork pointer */
> + struct xfs_ifork *ifp; /* inode fork pointer */
> xfs_bmbt_key_t *kp; /* root block key pointer */
> xfs_mount_t *mp; /* mount structure */
> xfs_bmbt_ptr_t *pp; /* root block address pointer */
> @@ -823,7 +823,7 @@ xfs_bmap_local_to_extents(
> {
> int error = 0;
> int flags; /* logging flags returned */
> - xfs_ifork_t *ifp; /* inode fork pointer */
> + struct xfs_ifork *ifp; /* inode fork pointer */
> xfs_alloc_arg_t args; /* allocation arguments */
> xfs_buf_t *bp; /* buffer for extent block */
> struct xfs_bmbt_irec rec;
> @@ -1501,7 +1501,7 @@ xfs_bmap_one_block(
> xfs_inode_t *ip, /* incore inode */
> int whichfork) /* data or attr fork */
> {
> - xfs_ifork_t *ifp; /* inode fork pointer */
> + struct xfs_ifork *ifp; /* inode fork pointer */
> int rval; /* return value */
> xfs_bmbt_irec_t s; /* internal version of extent */
> struct xfs_iext_cursor icur;
> @@ -1539,7 +1539,7 @@ xfs_bmap_add_extent_delay_real(
> struct xfs_bmbt_irec *new = &bma->got;
> int error; /* error return value */
> int i; /* temp state */
> - xfs_ifork_t *ifp; /* inode fork pointer */
> + struct xfs_ifork *ifp; /* inode fork pointer */
> xfs_fileoff_t new_endoff; /* end offset of new entry */
> xfs_bmbt_irec_t r[3]; /* neighbor extent entries */
> /* left is 0, right is 1, prev is 2 */
> @@ -2053,7 +2053,7 @@ xfs_bmap_add_extent_unwritten_real(
> xfs_btree_cur_t *cur; /* btree cursor */
> int error; /* error return value */
> int i; /* temp state */
> - xfs_ifork_t *ifp; /* inode fork pointer */
> + struct xfs_ifork *ifp; /* inode fork pointer */
> xfs_fileoff_t new_endoff; /* end offset of new entry */
> xfs_bmbt_irec_t r[3]; /* neighbor extent entries */
> /* left is 0, right is 1, prev is 2 */
> @@ -2520,7 +2520,7 @@ xfs_bmap_add_extent_hole_delay(
> struct xfs_iext_cursor *icur,
> xfs_bmbt_irec_t *new) /* new data to add to file extents */
> {
> - xfs_ifork_t *ifp; /* inode fork pointer */
> + struct xfs_ifork *ifp; /* inode fork pointer */
> xfs_bmbt_irec_t left; /* left neighbor extent entry */
> xfs_filblks_t newlen=0; /* new indirect size */
> xfs_filblks_t oldlen=0; /* old indirect size */
> @@ -4911,7 +4911,7 @@ xfs_bmap_del_extent_real(
> struct xfs_bmbt_irec got; /* current extent entry */
> xfs_fileoff_t got_endoff; /* first offset past got */
> int i; /* temp state */
> - xfs_ifork_t *ifp; /* inode fork pointer */
> + struct xfs_ifork *ifp; /* inode fork pointer */
> xfs_mount_t *mp; /* mount structure */
> xfs_filblks_t nblks; /* quota/sb block count */
> xfs_bmbt_irec_t new; /* new record to be inserted */
> @@ -5161,7 +5161,7 @@ __xfs_bunmapi(
> int error; /* error return value */
> xfs_extnum_t extno; /* extent number in list */
> xfs_bmbt_irec_t got; /* current extent record */
> - xfs_ifork_t *ifp; /* inode fork pointer */
> + struct xfs_ifork *ifp; /* inode fork pointer */
> int isrt; /* freeing in rt area */
> int logflags; /* transaction logging flags */
> xfs_extlen_t mod; /* rt extent offset */
> diff --git a/fs/xfs/libxfs/xfs_inode_fork.c b/fs/xfs/libxfs/xfs_inode_fork.c
> index a0e3fb804605..f9acf1d436f6 100644
> --- a/fs/xfs/libxfs/xfs_inode_fork.c
> +++ b/fs/xfs/libxfs/xfs_inode_fork.c
> @@ -269,7 +269,7 @@ xfs_iformat_btree(
> {
> struct xfs_mount *mp = ip->i_mount;
> xfs_bmdr_block_t *dfp;
> - xfs_ifork_t *ifp;
> + struct xfs_ifork *ifp;
> /* REFERENCED */
> int nrecs;
> int size;
> @@ -347,7 +347,7 @@ xfs_iroot_realloc(
> {
> struct xfs_mount *mp = ip->i_mount;
> int cur_max;
> - xfs_ifork_t *ifp;
> + struct xfs_ifork *ifp;
> struct xfs_btree_block *new_broot;
> int new_max;
> size_t new_size;
> @@ -503,7 +503,7 @@ xfs_idestroy_fork(
> xfs_inode_t *ip,
> int whichfork)
> {
> - xfs_ifork_t *ifp;
> + struct xfs_ifork *ifp;
>
> ifp = XFS_IFORK_PTR(ip, whichfork);
> if (ifp->if_broot != NULL) {
> @@ -592,7 +592,7 @@ xfs_iflush_fork(
> int whichfork)
> {
> char *cp;
> - xfs_ifork_t *ifp;
> + struct xfs_ifork *ifp;
> xfs_mount_t *mp;
> static const short brootflag[2] =
> { XFS_ILOG_DBROOT, XFS_ILOG_ABROOT };
> diff --git a/fs/xfs/libxfs/xfs_inode_fork.h b/fs/xfs/libxfs/xfs_inode_fork.h
> index 46242052aad0..1492143371f3 100644
> --- a/fs/xfs/libxfs/xfs_inode_fork.h
> +++ b/fs/xfs/libxfs/xfs_inode_fork.h
> @@ -12,7 +12,7 @@ struct xfs_dinode;
> /*
> * File incore extent information, present for each of data & attr forks.
> */
> -typedef struct xfs_ifork {
> +struct xfs_ifork {
> int if_bytes; /* bytes in if_u1 */
> struct xfs_btree_block *if_broot; /* file's incore btree root */
> short if_broot_bytes; /* bytes allocated for root */
> @@ -22,7 +22,7 @@ typedef struct xfs_ifork {
> void *if_root; /* extent tree root */
> char *if_data; /* inline file data */
> } if_u1;
> -} xfs_ifork_t;
> +};
>
> /*
> * Per-fork incore inode flags.
> diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c
> index 47f417d20a30..79f344fa8b14 100644
> --- a/fs/xfs/xfs_icache.c
> +++ b/fs/xfs/xfs_icache.c
> @@ -66,7 +66,7 @@ xfs_inode_alloc(
> ip->i_cowfp = NULL;
> ip->i_cnextents = 0;
> ip->i_cformat = XFS_DINODE_FMT_EXTENTS;
> - memset(&ip->i_df, 0, sizeof(xfs_ifork_t));
> + memset(&ip->i_df, 0, sizeof(ip->i_df));
> ip->i_flags = 0;
> ip->i_delayed_blks = 0;
> memset(&ip->i_d, 0, sizeof(ip->i_d));
> diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h
> index 2ed63a49e890..1f910d2ae73a 100644
> --- a/fs/xfs/xfs_inode.h
> +++ b/fs/xfs/xfs_inode.h
> @@ -34,9 +34,9 @@ typedef struct xfs_inode {
> struct xfs_imap i_imap; /* location for xfs_imap() */
>
> /* Extent information. */
> - xfs_ifork_t *i_afp; /* attribute fork pointer */
> - xfs_ifork_t *i_cowfp; /* copy on write extents */
> - xfs_ifork_t i_df; /* data fork */
> + struct xfs_ifork *i_afp; /* attribute fork pointer */
> + struct xfs_ifork *i_cowfp; /* copy on write extents */
> + struct xfs_ifork i_df; /* data fork */
>
> /* operations vectors */
> const struct xfs_dir_ops *d_ops; /* directory ops vector */
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index f9f8dc490d3d..62312b4cef64 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -1886,7 +1886,7 @@ xfs_init_zones(void)
> if (!xfs_da_state_zone)
> goto out_destroy_btree_cur_zone;
>
> - xfs_ifork_zone = kmem_zone_init(sizeof(xfs_ifork_t), "xfs_ifork");
> + xfs_ifork_zone = kmem_zone_init(sizeof(struct xfs_ifork), "xfs_ifork");
> if (!xfs_ifork_zone)
> goto out_destroy_da_state_zone;
>
> --
> 2.18.0
>
> --
> 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] 17+ messages in thread
* [PATCH 4/6] xfs: introduce a new xfs_inode_has_cow_data helper
2018-07-10 6:05 reduce lookups in the COW extent tree Christoph Hellwig
` (2 preceding siblings ...)
2018-07-10 6:05 ` [PATCH 3/6] xfs: remove the xfs_ifork_t typedef Christoph Hellwig
@ 2018-07-10 6:05 ` Christoph Hellwig
2018-07-11 16:24 ` Darrick J. Wong
2018-07-10 6:05 ` [PATCH 5/6] xfs: maintain a sequence count for inode fork manipulations Christoph Hellwig
2018-07-10 6:05 ` [PATCH 6/6] xfs: avoid COW fork extent lookups in writeback if the fork didn't change Christoph Hellwig
5 siblings, 1 reply; 17+ messages in thread
From: Christoph Hellwig @ 2018-07-10 6:05 UTC (permalink / raw)
To: linux-xfs
We have a few places that already check if an inode has actual data in
the COW fork to avoid work on reflink inodes that do not actually have
outstanding COW blocks. There are a few more places that can avoid
working if doing the same check, so add a documented helper for this
condition and use it in all places where it makes sense.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/xfs/xfs_aops.c | 4 ++--
fs/xfs/xfs_bmap_util.c | 2 +-
fs/xfs/xfs_icache.c | 10 ++++------
fs/xfs/xfs_inode.c | 3 +--
fs/xfs/xfs_inode.h | 9 +++++++++
fs/xfs/xfs_reflink.c | 2 +-
6 files changed, 18 insertions(+), 12 deletions(-)
diff --git a/fs/xfs/xfs_aops.c b/fs/xfs/xfs_aops.c
index f4d3252236c1..814100d27343 100644
--- a/fs/xfs/xfs_aops.c
+++ b/fs/xfs/xfs_aops.c
@@ -338,7 +338,7 @@ xfs_map_blocks(
imap_valid = offset_fsb >= wpc->imap.br_startoff &&
offset_fsb < wpc->imap.br_startoff + wpc->imap.br_blockcount;
if (imap_valid &&
- (!xfs_is_reflink_inode(ip) || wpc->io_type == XFS_IO_COW))
+ (!xfs_inode_has_cow_data(ip) || wpc->io_type == XFS_IO_COW))
return 0;
if (XFS_FORCED_SHUTDOWN(mp))
@@ -363,7 +363,7 @@ xfs_map_blocks(
* Check if this is offset is covered by a COW extents, and if yes use
* it directly instead of looking up anything in the data fork.
*/
- if (xfs_is_reflink_inode(ip) &&
+ if (xfs_inode_has_cow_data(ip) &&
xfs_iext_lookup_extent(ip, ip->i_cowfp, offset_fsb, &icur, &imap) &&
imap.br_startoff <= offset_fsb) {
xfs_iunlock(ip, XFS_ILOCK_SHARED);
diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
index da561882c349..d78f8300607f 100644
--- a/fs/xfs/xfs_bmap_util.c
+++ b/fs/xfs/xfs_bmap_util.c
@@ -1280,7 +1280,7 @@ xfs_prepare_shift(
* we've flushed all the dirty data out to disk to avoid having
* CoW extents at the wrong offsets.
*/
- if (xfs_is_reflink_inode(ip)) {
+ if (xfs_inode_has_cow_data(ip)) {
error = xfs_reflink_cancel_cow_range(ip, offset, NULLFILEOFF,
true);
if (error)
diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c
index 79f344fa8b14..fdae4c2d461e 100644
--- a/fs/xfs/xfs_icache.c
+++ b/fs/xfs/xfs_icache.c
@@ -1697,14 +1697,13 @@ xfs_inode_clear_eofblocks_tag(
*/
static bool
xfs_prep_free_cowblocks(
- struct xfs_inode *ip,
- struct xfs_ifork *ifp)
+ struct xfs_inode *ip)
{
/*
* Just clear the tag if we have an empty cow fork or none at all. It's
* possible the inode was fully unshared since it was originally tagged.
*/
- if (!xfs_is_reflink_inode(ip) || !ifp->if_bytes) {
+ if (!xfs_inode_has_cow_data(ip)) {
trace_xfs_inode_free_cowblocks_invalid(ip);
xfs_inode_clear_cowblocks_tag(ip);
return false;
@@ -1742,11 +1741,10 @@ xfs_inode_free_cowblocks(
void *args)
{
struct xfs_eofblocks *eofb = args;
- struct xfs_ifork *ifp = XFS_IFORK_PTR(ip, XFS_COW_FORK);
int match;
int ret = 0;
- if (!xfs_prep_free_cowblocks(ip, ifp))
+ if (!xfs_prep_free_cowblocks(ip))
return 0;
if (eofb) {
@@ -1771,7 +1769,7 @@ xfs_inode_free_cowblocks(
* Check again, nobody else should be able to dirty blocks or change
* the reflink iflag now that we have the first two locks held.
*/
- if (xfs_prep_free_cowblocks(ip, ifp))
+ if (xfs_prep_free_cowblocks(ip))
ret = xfs_reflink_cancel_cow_range(ip, 0, NULLFILEOFF, false);
xfs_iunlock(ip, XFS_MMAPLOCK_EXCL);
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index b6da446ae946..2036e49f7e15 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -1868,7 +1868,6 @@ xfs_inactive(
xfs_inode_t *ip)
{
struct xfs_mount *mp;
- struct xfs_ifork *cow_ifp = XFS_IFORK_PTR(ip, XFS_COW_FORK);
int error;
int truncate = 0;
@@ -1889,7 +1888,7 @@ xfs_inactive(
return;
/* Try to clean out the cow blocks if there are any. */
- if (xfs_is_reflink_inode(ip) && cow_ifp->if_bytes > 0)
+ if (xfs_inode_has_cow_data(ip))
xfs_reflink_cancel_cow_range(ip, 0, NULLFILEOFF, true);
if (VFS_I(ip)->i_nlink != 0) {
diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h
index 1f910d2ae73a..82d27a295336 100644
--- a/fs/xfs/xfs_inode.h
+++ b/fs/xfs/xfs_inode.h
@@ -198,6 +198,15 @@ static inline bool xfs_is_reflink_inode(struct xfs_inode *ip)
return ip->i_d.di_flags2 & XFS_DIFLAG2_REFLINK;
}
+/*
+ * Check if an inode has any data in the COW fork. This might be often false
+ * even for inodes with the reflink flag when there is no pending COW operation.
+ */
+static inline bool xfs_inode_has_cow_data(struct xfs_inode *ip)
+{
+ return ip->i_cowfp && ip->i_cowfp->if_bytes;
+}
+
/*
* In-core inode flags.
*/
diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
index 49e4913fa779..789310c547e6 100644
--- a/fs/xfs/xfs_reflink.c
+++ b/fs/xfs/xfs_reflink.c
@@ -494,7 +494,7 @@ xfs_reflink_cancel_cow_blocks(
struct xfs_defer_ops dfops;
int error = 0;
- if (!xfs_is_reflink_inode(ip))
+ if (!xfs_inode_has_cow_data(ip))
return 0;
if (!xfs_iext_lookup_extent_before(ip, ifp, &end_fsb, &icur, &got))
return 0;
--
2.18.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 4/6] xfs: introduce a new xfs_inode_has_cow_data helper
2018-07-10 6:05 ` [PATCH 4/6] xfs: introduce a new xfs_inode_has_cow_data helper Christoph Hellwig
@ 2018-07-11 16:24 ` Darrick J. Wong
0 siblings, 0 replies; 17+ messages in thread
From: Darrick J. Wong @ 2018-07-11 16:24 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs
On Tue, Jul 10, 2018 at 08:05:26AM +0200, Christoph Hellwig wrote:
> We have a few places that already check if an inode has actual data in
> the COW fork to avoid work on reflink inodes that do not actually have
> outstanding COW blocks. There are a few more places that can avoid
> working if doing the same check, so add a documented helper for this
> condition and use it in all places where it makes sense.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
Looks ok, I think.
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
--D
> ---
> fs/xfs/xfs_aops.c | 4 ++--
> fs/xfs/xfs_bmap_util.c | 2 +-
> fs/xfs/xfs_icache.c | 10 ++++------
> fs/xfs/xfs_inode.c | 3 +--
> fs/xfs/xfs_inode.h | 9 +++++++++
> fs/xfs/xfs_reflink.c | 2 +-
> 6 files changed, 18 insertions(+), 12 deletions(-)
>
> diff --git a/fs/xfs/xfs_aops.c b/fs/xfs/xfs_aops.c
> index f4d3252236c1..814100d27343 100644
> --- a/fs/xfs/xfs_aops.c
> +++ b/fs/xfs/xfs_aops.c
> @@ -338,7 +338,7 @@ xfs_map_blocks(
> imap_valid = offset_fsb >= wpc->imap.br_startoff &&
> offset_fsb < wpc->imap.br_startoff + wpc->imap.br_blockcount;
> if (imap_valid &&
> - (!xfs_is_reflink_inode(ip) || wpc->io_type == XFS_IO_COW))
> + (!xfs_inode_has_cow_data(ip) || wpc->io_type == XFS_IO_COW))
> return 0;
>
> if (XFS_FORCED_SHUTDOWN(mp))
> @@ -363,7 +363,7 @@ xfs_map_blocks(
> * Check if this is offset is covered by a COW extents, and if yes use
> * it directly instead of looking up anything in the data fork.
> */
> - if (xfs_is_reflink_inode(ip) &&
> + if (xfs_inode_has_cow_data(ip) &&
> xfs_iext_lookup_extent(ip, ip->i_cowfp, offset_fsb, &icur, &imap) &&
> imap.br_startoff <= offset_fsb) {
> xfs_iunlock(ip, XFS_ILOCK_SHARED);
> diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
> index da561882c349..d78f8300607f 100644
> --- a/fs/xfs/xfs_bmap_util.c
> +++ b/fs/xfs/xfs_bmap_util.c
> @@ -1280,7 +1280,7 @@ xfs_prepare_shift(
> * we've flushed all the dirty data out to disk to avoid having
> * CoW extents at the wrong offsets.
> */
> - if (xfs_is_reflink_inode(ip)) {
> + if (xfs_inode_has_cow_data(ip)) {
> error = xfs_reflink_cancel_cow_range(ip, offset, NULLFILEOFF,
> true);
> if (error)
> diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c
> index 79f344fa8b14..fdae4c2d461e 100644
> --- a/fs/xfs/xfs_icache.c
> +++ b/fs/xfs/xfs_icache.c
> @@ -1697,14 +1697,13 @@ xfs_inode_clear_eofblocks_tag(
> */
> static bool
> xfs_prep_free_cowblocks(
> - struct xfs_inode *ip,
> - struct xfs_ifork *ifp)
> + struct xfs_inode *ip)
> {
> /*
> * Just clear the tag if we have an empty cow fork or none at all. It's
> * possible the inode was fully unshared since it was originally tagged.
> */
> - if (!xfs_is_reflink_inode(ip) || !ifp->if_bytes) {
> + if (!xfs_inode_has_cow_data(ip)) {
> trace_xfs_inode_free_cowblocks_invalid(ip);
> xfs_inode_clear_cowblocks_tag(ip);
> return false;
> @@ -1742,11 +1741,10 @@ xfs_inode_free_cowblocks(
> void *args)
> {
> struct xfs_eofblocks *eofb = args;
> - struct xfs_ifork *ifp = XFS_IFORK_PTR(ip, XFS_COW_FORK);
> int match;
> int ret = 0;
>
> - if (!xfs_prep_free_cowblocks(ip, ifp))
> + if (!xfs_prep_free_cowblocks(ip))
> return 0;
>
> if (eofb) {
> @@ -1771,7 +1769,7 @@ xfs_inode_free_cowblocks(
> * Check again, nobody else should be able to dirty blocks or change
> * the reflink iflag now that we have the first two locks held.
> */
> - if (xfs_prep_free_cowblocks(ip, ifp))
> + if (xfs_prep_free_cowblocks(ip))
> ret = xfs_reflink_cancel_cow_range(ip, 0, NULLFILEOFF, false);
>
> xfs_iunlock(ip, XFS_MMAPLOCK_EXCL);
> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> index b6da446ae946..2036e49f7e15 100644
> --- a/fs/xfs/xfs_inode.c
> +++ b/fs/xfs/xfs_inode.c
> @@ -1868,7 +1868,6 @@ xfs_inactive(
> xfs_inode_t *ip)
> {
> struct xfs_mount *mp;
> - struct xfs_ifork *cow_ifp = XFS_IFORK_PTR(ip, XFS_COW_FORK);
> int error;
> int truncate = 0;
>
> @@ -1889,7 +1888,7 @@ xfs_inactive(
> return;
>
> /* Try to clean out the cow blocks if there are any. */
> - if (xfs_is_reflink_inode(ip) && cow_ifp->if_bytes > 0)
> + if (xfs_inode_has_cow_data(ip))
> xfs_reflink_cancel_cow_range(ip, 0, NULLFILEOFF, true);
>
> if (VFS_I(ip)->i_nlink != 0) {
> diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h
> index 1f910d2ae73a..82d27a295336 100644
> --- a/fs/xfs/xfs_inode.h
> +++ b/fs/xfs/xfs_inode.h
> @@ -198,6 +198,15 @@ static inline bool xfs_is_reflink_inode(struct xfs_inode *ip)
> return ip->i_d.di_flags2 & XFS_DIFLAG2_REFLINK;
> }
>
> +/*
> + * Check if an inode has any data in the COW fork. This might be often false
> + * even for inodes with the reflink flag when there is no pending COW operation.
> + */
> +static inline bool xfs_inode_has_cow_data(struct xfs_inode *ip)
> +{
> + return ip->i_cowfp && ip->i_cowfp->if_bytes;
> +}
> +
> /*
> * In-core inode flags.
> */
> diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
> index 49e4913fa779..789310c547e6 100644
> --- a/fs/xfs/xfs_reflink.c
> +++ b/fs/xfs/xfs_reflink.c
> @@ -494,7 +494,7 @@ xfs_reflink_cancel_cow_blocks(
> struct xfs_defer_ops dfops;
> int error = 0;
>
> - if (!xfs_is_reflink_inode(ip))
> + if (!xfs_inode_has_cow_data(ip))
> return 0;
> if (!xfs_iext_lookup_extent_before(ip, ifp, &end_fsb, &icur, &got))
> return 0;
> --
> 2.18.0
>
> --
> 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] 17+ messages in thread
* [PATCH 5/6] xfs: maintain a sequence count for inode fork manipulations
2018-07-10 6:05 reduce lookups in the COW extent tree Christoph Hellwig
` (3 preceding siblings ...)
2018-07-10 6:05 ` [PATCH 4/6] xfs: introduce a new xfs_inode_has_cow_data helper Christoph Hellwig
@ 2018-07-10 6:05 ` Christoph Hellwig
2018-07-10 6:05 ` [PATCH 6/6] xfs: avoid COW fork extent lookups in writeback if the fork didn't change Christoph Hellwig
5 siblings, 0 replies; 17+ messages in thread
From: Christoph Hellwig @ 2018-07-10 6:05 UTC (permalink / raw)
To: linux-xfs
Add a simple 32-bit unsigned integer as the sequence count for
modifications to the extent list in the inode fork. This will be
used to optimize away extent list lookups in the writeback code.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/xfs/libxfs/xfs_iext_tree.c | 4 ++++
fs/xfs/libxfs/xfs_inode_fork.h | 1 +
2 files changed, 5 insertions(+)
diff --git a/fs/xfs/libxfs/xfs_iext_tree.c b/fs/xfs/libxfs/xfs_iext_tree.c
index b80c63faace2..cce7e8024f46 100644
--- a/fs/xfs/libxfs/xfs_iext_tree.c
+++ b/fs/xfs/libxfs/xfs_iext_tree.c
@@ -650,6 +650,7 @@ xfs_iext_insert(
cur->leaf->recs[i] = cur->leaf->recs[i - 1];
xfs_iext_set(cur_rec(cur), irec);
ifp->if_bytes += sizeof(struct xfs_iext_rec);
+ ifp->if_seq++;
trace_xfs_iext_insert(ip, cur, state, _RET_IP_);
@@ -869,6 +870,7 @@ xfs_iext_remove(
leaf->recs[i] = leaf->recs[i + 1];
xfs_iext_rec_clear(&leaf->recs[nr_entries]);
ifp->if_bytes -= sizeof(struct xfs_iext_rec);
+ ifp->if_seq++;
if (cur->pos == 0 && nr_entries > 0) {
xfs_iext_update_node(ifp, offset, xfs_iext_leaf_key(leaf, 0), 1,
@@ -983,6 +985,8 @@ xfs_iext_update_extent(
trace_xfs_bmap_pre_update(ip, cur, state, _RET_IP_);
xfs_iext_set(cur_rec(cur), new);
trace_xfs_bmap_post_update(ip, cur, state, _RET_IP_);
+
+ ifp->if_seq++;
}
/*
diff --git a/fs/xfs/libxfs/xfs_inode_fork.h b/fs/xfs/libxfs/xfs_inode_fork.h
index 1492143371f3..f20b2468ca35 100644
--- a/fs/xfs/libxfs/xfs_inode_fork.h
+++ b/fs/xfs/libxfs/xfs_inode_fork.h
@@ -14,6 +14,7 @@ struct xfs_dinode;
*/
struct xfs_ifork {
int if_bytes; /* bytes in if_u1 */
+ unsigned int if_seq;
struct xfs_btree_block *if_broot; /* file's incore btree root */
short if_broot_bytes; /* bytes allocated for root */
unsigned char if_flags; /* per-fork flags */
--
2.18.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 6/6] xfs: avoid COW fork extent lookups in writeback if the fork didn't change
2018-07-10 6:05 reduce lookups in the COW extent tree Christoph Hellwig
` (4 preceding siblings ...)
2018-07-10 6:05 ` [PATCH 5/6] xfs: maintain a sequence count for inode fork manipulations Christoph Hellwig
@ 2018-07-10 6:05 ` Christoph Hellwig
2018-07-11 17:15 ` Darrick J. Wong
5 siblings, 1 reply; 17+ messages in thread
From: Christoph Hellwig @ 2018-07-10 6:05 UTC (permalink / raw)
To: linux-xfs
Used the per-fork sequence counter to avoid lookups in the writeback code
unless the COW fork actually changed.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/xfs/xfs_aops.c | 28 +++++++++++++++++++++++-----
fs/xfs/xfs_iomap.c | 4 +++-
fs/xfs/xfs_iomap.h | 2 +-
3 files changed, 27 insertions(+), 7 deletions(-)
diff --git a/fs/xfs/xfs_aops.c b/fs/xfs/xfs_aops.c
index 814100d27343..495d5e74fd00 100644
--- a/fs/xfs/xfs_aops.c
+++ b/fs/xfs/xfs_aops.c
@@ -29,6 +29,7 @@
struct xfs_writepage_ctx {
struct xfs_bmbt_irec imap;
unsigned int io_type;
+ unsigned int cow_seq;
struct xfs_ioend *ioend;
};
@@ -310,6 +311,7 @@ xfs_map_blocks(
struct xfs_mount *mp = ip->i_mount;
ssize_t count = i_blocksize(inode);
xfs_fileoff_t offset_fsb = XFS_B_TO_FSBT(mp, offset), end_fsb;
+ xfs_fileoff_t cow_fsb = NULLFILEOFF;
struct xfs_bmbt_irec imap;
int whichfork = XFS_DATA_FORK;
struct xfs_iext_cursor icur;
@@ -333,12 +335,15 @@ xfs_map_blocks(
* COW fork blocks can overlap data fork blocks even if the blocks
* aren't shared. COW I/O always takes precedent, so we must always
* check for overlap on reflink inodes unless the mapping is already a
- * COW one.
+ * COW one, or the COW fork hasn't changed from the last time we looked
+ * at it.
*/
imap_valid = offset_fsb >= wpc->imap.br_startoff &&
offset_fsb < wpc->imap.br_startoff + wpc->imap.br_blockcount;
if (imap_valid &&
- (!xfs_inode_has_cow_data(ip) || wpc->io_type == XFS_IO_COW))
+ (!xfs_inode_has_cow_data(ip) ||
+ wpc->io_type == XFS_IO_COW ||
+ wpc->cow_seq == ip->i_cowfp->if_seq))
return 0;
if (XFS_FORCED_SHUTDOWN(mp))
@@ -364,8 +369,10 @@ xfs_map_blocks(
* it directly instead of looking up anything in the data fork.
*/
if (xfs_inode_has_cow_data(ip) &&
- xfs_iext_lookup_extent(ip, ip->i_cowfp, offset_fsb, &icur, &imap) &&
- imap.br_startoff <= offset_fsb) {
+ xfs_iext_lookup_extent(ip, ip->i_cowfp, offset_fsb, &icur, &imap))
+ cow_fsb = imap.br_startoff;
+ if (cow_fsb != NULLFILEOFF && cow_fsb <= offset_fsb) {
+ wpc->cow_seq = ip->i_cowfp->if_seq;
xfs_iunlock(ip, XFS_ILOCK_SHARED);
/*
* Truncate can race with writeback since writeback doesn't
@@ -411,6 +418,14 @@ xfs_map_blocks(
imap.br_startblock = HOLESTARTBLOCK;
wpc->io_type = XFS_IO_HOLE;
} else {
+ /*
+ * Truncate to the next COW extent if there is one. We'll
+ * need to treat the COW range separately.
+ */
+ if (cow_fsb != NULLFILEOFF &&
+ cow_fsb < imap.br_startoff + imap.br_blockcount)
+ imap.br_blockcount = cow_fsb - imap.br_startoff;
+
if (isnullstartblock(imap.br_startblock)) {
/* got a delalloc extent */
wpc->io_type = XFS_IO_DELALLOC;
@@ -427,9 +442,12 @@ xfs_map_blocks(
trace_xfs_map_blocks_found(ip, offset, count, wpc->io_type, &imap);
return 0;
allocate_blocks:
- error = xfs_iomap_write_allocate(ip, whichfork, offset, &imap);
+ error = xfs_iomap_write_allocate(ip, whichfork, offset, &imap,
+ &wpc->cow_seq);
if (error)
return error;
+ ASSERT(whichfork == XFS_COW_FORK || cow_fsb == NULLFILEOFF ||
+ imap.br_startoff + imap.br_blockcount <= cow_fsb);
wpc->imap = imap;
trace_xfs_map_blocks_alloc(ip, offset, count, wpc->io_type, &imap);
return 0;
diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
index fb9746cc7338..a843766bf7c5 100644
--- a/fs/xfs/xfs_iomap.c
+++ b/fs/xfs/xfs_iomap.c
@@ -660,7 +660,8 @@ xfs_iomap_write_allocate(
xfs_inode_t *ip,
int whichfork,
xfs_off_t offset,
- xfs_bmbt_irec_t *imap)
+ xfs_bmbt_irec_t *imap,
+ unsigned int *cow_seq)
{
xfs_mount_t *mp = ip->i_mount;
xfs_fileoff_t offset_fsb, last_block;
@@ -784,6 +785,7 @@ xfs_iomap_write_allocate(
if (error)
goto error0;
+ *cow_seq = XFS_IFORK_PTR(ip, whichfork)->if_seq;
xfs_iunlock(ip, XFS_ILOCK_EXCL);
}
diff --git a/fs/xfs/xfs_iomap.h b/fs/xfs/xfs_iomap.h
index 83474c9cede9..c6170548831b 100644
--- a/fs/xfs/xfs_iomap.h
+++ b/fs/xfs/xfs_iomap.h
@@ -14,7 +14,7 @@ struct xfs_bmbt_irec;
int xfs_iomap_write_direct(struct xfs_inode *, xfs_off_t, size_t,
struct xfs_bmbt_irec *, int);
int xfs_iomap_write_allocate(struct xfs_inode *, int, xfs_off_t,
- struct xfs_bmbt_irec *);
+ struct xfs_bmbt_irec *, unsigned int *);
int xfs_iomap_write_unwritten(struct xfs_inode *, xfs_off_t, xfs_off_t, bool);
void xfs_bmbt_to_iomap(struct xfs_inode *, struct iomap *,
--
2.18.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 6/6] xfs: avoid COW fork extent lookups in writeback if the fork didn't change
2018-07-10 6:05 ` [PATCH 6/6] xfs: avoid COW fork extent lookups in writeback if the fork didn't change Christoph Hellwig
@ 2018-07-11 17:15 ` Darrick J. Wong
2018-07-11 17:20 ` Christoph Hellwig
0 siblings, 1 reply; 17+ messages in thread
From: Darrick J. Wong @ 2018-07-11 17:15 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs
On Tue, Jul 10, 2018 at 08:05:28AM +0200, Christoph Hellwig wrote:
> Used the per-fork sequence counter to avoid lookups in the writeback code
> unless the COW fork actually changed.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> fs/xfs/xfs_aops.c | 28 +++++++++++++++++++++++-----
> fs/xfs/xfs_iomap.c | 4 +++-
> fs/xfs/xfs_iomap.h | 2 +-
> 3 files changed, 27 insertions(+), 7 deletions(-)
>
> diff --git a/fs/xfs/xfs_aops.c b/fs/xfs/xfs_aops.c
> index 814100d27343..495d5e74fd00 100644
> --- a/fs/xfs/xfs_aops.c
> +++ b/fs/xfs/xfs_aops.c
> @@ -29,6 +29,7 @@
> struct xfs_writepage_ctx {
> struct xfs_bmbt_irec imap;
> unsigned int io_type;
> + unsigned int cow_seq;
> struct xfs_ioend *ioend;
> };
>
> @@ -310,6 +311,7 @@ xfs_map_blocks(
> struct xfs_mount *mp = ip->i_mount;
> ssize_t count = i_blocksize(inode);
> xfs_fileoff_t offset_fsb = XFS_B_TO_FSBT(mp, offset), end_fsb;
> + xfs_fileoff_t cow_fsb = NULLFILEOFF;
> struct xfs_bmbt_irec imap;
> int whichfork = XFS_DATA_FORK;
> struct xfs_iext_cursor icur;
> @@ -333,12 +335,15 @@ xfs_map_blocks(
> * COW fork blocks can overlap data fork blocks even if the blocks
> * aren't shared. COW I/O always takes precedent, so we must always
> * check for overlap on reflink inodes unless the mapping is already a
> - * COW one.
> + * COW one, or the COW fork hasn't changed from the last time we looked
> + * at it.
> */
> imap_valid = offset_fsb >= wpc->imap.br_startoff &&
> offset_fsb < wpc->imap.br_startoff + wpc->imap.br_blockcount;
> if (imap_valid &&
> - (!xfs_inode_has_cow_data(ip) || wpc->io_type == XFS_IO_COW))
> + (!xfs_inode_has_cow_data(ip) ||
> + wpc->io_type == XFS_IO_COW ||
> + wpc->cow_seq == ip->i_cowfp->if_seq))
> return 0;
>
> if (XFS_FORCED_SHUTDOWN(mp))
> @@ -364,8 +369,10 @@ xfs_map_blocks(
> * it directly instead of looking up anything in the data fork.
> */
> if (xfs_inode_has_cow_data(ip) &&
> - xfs_iext_lookup_extent(ip, ip->i_cowfp, offset_fsb, &icur, &imap) &&
> - imap.br_startoff <= offset_fsb) {
> + xfs_iext_lookup_extent(ip, ip->i_cowfp, offset_fsb, &icur, &imap))
> + cow_fsb = imap.br_startoff;
> + if (cow_fsb != NULLFILEOFF && cow_fsb <= offset_fsb) {
> + wpc->cow_seq = ip->i_cowfp->if_seq;
> xfs_iunlock(ip, XFS_ILOCK_SHARED);
> /*
> * Truncate can race with writeback since writeback doesn't
> @@ -411,6 +418,14 @@ xfs_map_blocks(
> imap.br_startblock = HOLESTARTBLOCK;
> wpc->io_type = XFS_IO_HOLE;
> } else {
> + /*
> + * Truncate to the next COW extent if there is one. We'll
> + * need to treat the COW range separately.
> + */
> + if (cow_fsb != NULLFILEOFF &&
> + cow_fsb < imap.br_startoff + imap.br_blockcount)
> + imap.br_blockcount = cow_fsb - imap.br_startoff;
Hmm, this troubles me slightly -- a short time ago you removed the "trim
this data fork mapping to the first overlap with a cow fork mapping"
(i.e. xfs_reflink_trim_irec_to_next_cow) behavior on the grounds that
_writepage_map calls _map_blocks for every block in the page and so it
was unnecessary. But this seems to put that back. Why is that?
--D
> +
> if (isnullstartblock(imap.br_startblock)) {
> /* got a delalloc extent */
> wpc->io_type = XFS_IO_DELALLOC;
> @@ -427,9 +442,12 @@ xfs_map_blocks(
> trace_xfs_map_blocks_found(ip, offset, count, wpc->io_type, &imap);
> return 0;
> allocate_blocks:
> - error = xfs_iomap_write_allocate(ip, whichfork, offset, &imap);
> + error = xfs_iomap_write_allocate(ip, whichfork, offset, &imap,
> + &wpc->cow_seq);
> if (error)
> return error;
> + ASSERT(whichfork == XFS_COW_FORK || cow_fsb == NULLFILEOFF ||
> + imap.br_startoff + imap.br_blockcount <= cow_fsb);
> wpc->imap = imap;
> trace_xfs_map_blocks_alloc(ip, offset, count, wpc->io_type, &imap);
> return 0;
> diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
> index fb9746cc7338..a843766bf7c5 100644
> --- a/fs/xfs/xfs_iomap.c
> +++ b/fs/xfs/xfs_iomap.c
> @@ -660,7 +660,8 @@ xfs_iomap_write_allocate(
> xfs_inode_t *ip,
> int whichfork,
> xfs_off_t offset,
> - xfs_bmbt_irec_t *imap)
> + xfs_bmbt_irec_t *imap,
> + unsigned int *cow_seq)
> {
> xfs_mount_t *mp = ip->i_mount;
> xfs_fileoff_t offset_fsb, last_block;
> @@ -784,6 +785,7 @@ xfs_iomap_write_allocate(
> if (error)
> goto error0;
>
> + *cow_seq = XFS_IFORK_PTR(ip, whichfork)->if_seq;
> xfs_iunlock(ip, XFS_ILOCK_EXCL);
> }
>
> diff --git a/fs/xfs/xfs_iomap.h b/fs/xfs/xfs_iomap.h
> index 83474c9cede9..c6170548831b 100644
> --- a/fs/xfs/xfs_iomap.h
> +++ b/fs/xfs/xfs_iomap.h
> @@ -14,7 +14,7 @@ struct xfs_bmbt_irec;
> int xfs_iomap_write_direct(struct xfs_inode *, xfs_off_t, size_t,
> struct xfs_bmbt_irec *, int);
> int xfs_iomap_write_allocate(struct xfs_inode *, int, xfs_off_t,
> - struct xfs_bmbt_irec *);
> + struct xfs_bmbt_irec *, unsigned int *);
> int xfs_iomap_write_unwritten(struct xfs_inode *, xfs_off_t, xfs_off_t, bool);
>
> void xfs_bmbt_to_iomap(struct xfs_inode *, struct iomap *,
> --
> 2.18.0
>
> --
> 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] 17+ messages in thread* Re: [PATCH 6/6] xfs: avoid COW fork extent lookups in writeback if the fork didn't change
2018-07-11 17:15 ` Darrick J. Wong
@ 2018-07-11 17:20 ` Christoph Hellwig
2018-07-11 17:31 ` Darrick J. Wong
0 siblings, 1 reply; 17+ messages in thread
From: Christoph Hellwig @ 2018-07-11 17:20 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: Christoph Hellwig, linux-xfs
On Wed, Jul 11, 2018 at 10:15:51AM -0700, Darrick J. Wong wrote:
> Hmm, this troubles me slightly -- a short time ago you removed the "trim
> this data fork mapping to the first overlap with a cow fork mapping"
> (i.e. xfs_reflink_trim_irec_to_next_cow) behavior on the grounds that
> _writepage_map calls _map_blocks for every block in the page and so it
> was unnecessary. But this seems to put that back. Why is that?
Because back then map_blocks did a lookup in the COW fork everytime
(unless we are already on a COW mapping), and this patch wants to
only look it up when the COW fork actually changed, so the trim is
required now.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 6/6] xfs: avoid COW fork extent lookups in writeback if the fork didn't change
2018-07-11 17:20 ` Christoph Hellwig
@ 2018-07-11 17:31 ` Darrick J. Wong
2018-07-11 17:35 ` Christoph Hellwig
0 siblings, 1 reply; 17+ messages in thread
From: Darrick J. Wong @ 2018-07-11 17:31 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs
On Wed, Jul 11, 2018 at 07:20:32PM +0200, Christoph Hellwig wrote:
> On Wed, Jul 11, 2018 at 10:15:51AM -0700, Darrick J. Wong wrote:
> > Hmm, this troubles me slightly -- a short time ago you removed the "trim
> > this data fork mapping to the first overlap with a cow fork mapping"
> > (i.e. xfs_reflink_trim_irec_to_next_cow) behavior on the grounds that
> > _writepage_map calls _map_blocks for every block in the page and so it
> > was unnecessary. But this seems to put that back. Why is that?
>
> Because back then map_blocks did a lookup in the COW fork everytime
> (unless we are already on a COW mapping), and this patch wants to
> only look it up when the COW fork actually changed, so the trim is
> required now.
Ah, ok. Mind if I change the comment to:
/*
* Truncate to the next COW extent if there is one. This is the only
* opportunity to do this because we can skip COW fork lookups for the
* subsequent blocks in the mapping; however, the requirement to treat
* the COW range separately remains.
*/
I also wonder why we don't need to do the same for holes, but I think
the answer is that any dirty page with a cow fork mapping must also have
a data fork mapping (even if it's merely a delalloc reservation) and so
a hole will never overlap with a cow fork mapping.
--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] 17+ messages in thread
* Re: [PATCH 6/6] xfs: avoid COW fork extent lookups in writeback if the fork didn't change
2018-07-11 17:31 ` Darrick J. Wong
@ 2018-07-11 17:35 ` Christoph Hellwig
0 siblings, 0 replies; 17+ messages in thread
From: Christoph Hellwig @ 2018-07-11 17:35 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: Christoph Hellwig, linux-xfs
On Wed, Jul 11, 2018 at 10:31:52AM -0700, Darrick J. Wong wrote:
> On Wed, Jul 11, 2018 at 07:20:32PM +0200, Christoph Hellwig wrote:
> > On Wed, Jul 11, 2018 at 10:15:51AM -0700, Darrick J. Wong wrote:
> > > Hmm, this troubles me slightly -- a short time ago you removed the "trim
> > > this data fork mapping to the first overlap with a cow fork mapping"
> > > (i.e. xfs_reflink_trim_irec_to_next_cow) behavior on the grounds that
> > > _writepage_map calls _map_blocks for every block in the page and so it
> > > was unnecessary. But this seems to put that back. Why is that?
> >
> > Because back then map_blocks did a lookup in the COW fork everytime
> > (unless we are already on a COW mapping), and this patch wants to
> > only look it up when the COW fork actually changed, so the trim is
> > required now.
>
> Ah, ok. Mind if I change the comment to:
>
> /*
> * Truncate to the next COW extent if there is one. This is the only
> * opportunity to do this because we can skip COW fork lookups for the
> * subsequent blocks in the mapping; however, the requirement to treat
> * the COW range separately remains.
> */
>
> I also wonder why we don't need to do the same for holes, but I think
> the answer is that any dirty page with a cow fork mapping must also have
> a data fork mapping (even if it's merely a delalloc reservation) and so
> a hole will never overlap with a cow fork mapping.
I'll update the comment. I'll need to resend against the 4.19-merge
tree anyway, and I also found a little buglet in this patch (it updates
wpc->cow_seq for all allocations and not just COW ones, with that fixed
we should do even less lookups)
^ permalink raw reply [flat|nested] 17+ messages in thread