diff mbox

[1/3] xfs: refactor extended attribute list operation

Message ID 150905817055.20365.4909470444810149179.stgit@magnolia (mailing list archive)
State Accepted
Headers show

Commit Message

Darrick J. Wong Oct. 26, 2017, 10:49 p.m. UTC 
From: Darrick J. Wong <darrick.wong@oracle.com>

When we're iterating the attribute list and we can't find our previous
location based off the attribute cursor, we'll instead walk down the
attribute btree from the root trying to find where we left off.  Move
this code into a separate function for later cleanups.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/xfs_attr_list.c |  130 +++++++++++++++++++++++++++++-------------------
 1 file changed, 78 insertions(+), 52 deletions(-)



--
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

Comments

Brian Foster Oct. 27, 2017, 10:51 a.m. UTC | #1 
On Thu, Oct 26, 2017 at 03:49:30PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> When we're iterating the attribute list and we can't find our previous
> location based off the attribute cursor, we'll instead walk down the
> attribute btree from the root trying to find where we left off.  Move
> this code into a separate function for later cleanups.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  fs/xfs/xfs_attr_list.c |  130 +++++++++++++++++++++++++++++-------------------
>  1 file changed, 78 insertions(+), 52 deletions(-)
> 
> 
> diff --git a/fs/xfs/xfs_attr_list.c b/fs/xfs/xfs_attr_list.c
> index 5816786..021ec5a 100644
> --- a/fs/xfs/xfs_attr_list.c
> +++ b/fs/xfs/xfs_attr_list.c
> @@ -204,19 +204,83 @@ xfs_attr_shortform_list(xfs_attr_list_context_t *context)
>  	return 0;
>  }
>  
> +/*
> + * We didn't find the block & hash mentioned in the cursor state, so
> + * walk down the attr btree looking for the hash.
> + */
>  STATIC int
> -xfs_attr_node_list(xfs_attr_list_context_t *context)
> +xfs_attr_node_list_lookup(
> +	struct xfs_attr_list_context	*context,
> +	struct attrlist_cursor_kern	*cursor,
> +	struct xfs_buf			**pbp)
>  {
> -	attrlist_cursor_kern_t *cursor;
> -	xfs_attr_leafblock_t *leaf;
> -	xfs_da_intnode_t *node;
> -	struct xfs_attr3_icleaf_hdr leafhdr;
> -	struct xfs_da3_icnode_hdr nodehdr;
> -	struct xfs_da_node_entry *btree;
> -	int error, i;
> -	struct xfs_buf *bp;
> -	struct xfs_inode	*dp = context->dp;
> -	struct xfs_mount	*mp = dp->i_mount;
> +	struct xfs_da3_icnode_hdr	nodehdr;
> +	struct xfs_da_intnode		*node;
> +	struct xfs_da_node_entry	*btree;
> +	struct xfs_inode		*dp = context->dp;
> +	struct xfs_mount		*mp = dp->i_mount;
> +	struct xfs_trans		*tp = context->tp;
> +	struct xfs_buf			*bp;
> +	int				i;
> +	int				error = 0;
> +	uint16_t			magic;
> +
> +	ASSERT(*pbp == NULL);
> +	cursor->blkno = 0;
> +	for (;;) {
> +		error = xfs_da3_node_read(tp, dp, cursor->blkno, -1, &bp,
> +				XFS_ATTR_FORK);
> +		if (error)
> +			return error;
> +		node = bp->b_addr;
> +		magic = be16_to_cpu(node->hdr.info.magic);
> +		if (magic == XFS_ATTR_LEAF_MAGIC ||
> +		    magic == XFS_ATTR3_LEAF_MAGIC)
> +			break;
> +		if (magic != XFS_DA_NODE_MAGIC &&
> +		    magic != XFS_DA3_NODE_MAGIC) {
> +			XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp,
> +					node);
> +			goto out_corruptbuf;
> +		}
> +
> +		dp->d_ops->node_hdr_from_disk(&nodehdr, node);
> +
> +		btree = dp->d_ops->node_tree_p(node);
> +		for (i = 0; i < nodehdr.count; btree++, i++) {
> +			if (cursor->hashval <= be32_to_cpu(btree->hashval)) {
> +				cursor->blkno = be32_to_cpu(btree->before);
> +				trace_xfs_attr_list_node_descend(context,
> +						btree);
> +				break;
> +			}
> +		}
> +		xfs_trans_brelse(tp, bp);
> +
> +		if (i == nodehdr.count)
> +			return 0;
> +	}
> +
> +	*pbp = bp;
> +	return 0;
> +
> +out_corruptbuf:
> +	xfs_trans_brelse(tp, bp);
> +	return -EFSCORRUPTED;
> +}
> +
> +STATIC int
> +xfs_attr_node_list(
> +	struct xfs_attr_list_context	*context)
> +{
> +	struct xfs_attr3_icleaf_hdr	leafhdr;
> +	struct attrlist_cursor_kern	*cursor;
> +	struct xfs_attr_leafblock	*leaf;
> +	struct xfs_da_intnode		*node;
> +	struct xfs_buf			*bp;
> +	struct xfs_inode		*dp = context->dp;
> +	struct xfs_mount		*mp = dp->i_mount;
> +	int				error;
>  
>  	trace_xfs_attr_node_list(context);
>  
> @@ -277,47 +341,9 @@ xfs_attr_node_list(xfs_attr_list_context_t *context)
>  	 * Note that start of node block is same as start of leaf block.
>  	 */
>  	if (bp == NULL) {
> -		cursor->blkno = 0;
> -		for (;;) {
> -			uint16_t magic;
> -
> -			error = xfs_da3_node_read(context->tp, dp,
> -						      cursor->blkno, -1, &bp,
> -						      XFS_ATTR_FORK);
> -			if (error)
> -				return error;
> -			node = bp->b_addr;
> -			magic = be16_to_cpu(node->hdr.info.magic);
> -			if (magic == XFS_ATTR_LEAF_MAGIC ||
> -			    magic == XFS_ATTR3_LEAF_MAGIC)
> -				break;
> -			if (magic != XFS_DA_NODE_MAGIC &&
> -			    magic != XFS_DA3_NODE_MAGIC) {
> -				XFS_CORRUPTION_ERROR("xfs_attr_node_list(3)",
> -						     XFS_ERRLEVEL_LOW,
> -						     context->dp->i_mount,
> -						     node);
> -				xfs_trans_brelse(context->tp, bp);
> -				return -EFSCORRUPTED;
> -			}
> -
> -			dp->d_ops->node_hdr_from_disk(&nodehdr, node);
> -			btree = dp->d_ops->node_tree_p(node);
> -			for (i = 0; i < nodehdr.count; btree++, i++) {
> -				if (cursor->hashval
> -						<= be32_to_cpu(btree->hashval)) {
> -					cursor->blkno = be32_to_cpu(btree->before);
> -					trace_xfs_attr_list_node_descend(context,
> -									 btree);
> -					break;
> -				}
> -			}
> -			if (i == nodehdr.count) {
> -				xfs_trans_brelse(context->tp, bp);
> -				return 0;
> -			}
> -			xfs_trans_brelse(context->tp, bp);
> -		}
> +		error = xfs_attr_node_list_lookup(context, cursor, &bp);
> +		if (error || !bp)
> +			return error;
>  	}
>  	ASSERT(bp != NULL);
>  
> 
> --
> 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
--
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
diff mbox

Patch

diff --git a/fs/xfs/xfs_attr_list.c b/fs/xfs/xfs_attr_list.c
index 5816786..021ec5a 100644
--- a/fs/xfs/xfs_attr_list.c
+++ b/fs/xfs/xfs_attr_list.c
@@ -204,19 +204,83 @@  xfs_attr_shortform_list(xfs_attr_list_context_t *context)
 	return 0;
 }
 
+/*
+ * We didn't find the block & hash mentioned in the cursor state, so
+ * walk down the attr btree looking for the hash.
+ */
 STATIC int
-xfs_attr_node_list(xfs_attr_list_context_t *context)
+xfs_attr_node_list_lookup(
+	struct xfs_attr_list_context	*context,
+	struct attrlist_cursor_kern	*cursor,
+	struct xfs_buf			**pbp)
 {
-	attrlist_cursor_kern_t *cursor;
-	xfs_attr_leafblock_t *leaf;
-	xfs_da_intnode_t *node;
-	struct xfs_attr3_icleaf_hdr leafhdr;
-	struct xfs_da3_icnode_hdr nodehdr;
-	struct xfs_da_node_entry *btree;
-	int error, i;
-	struct xfs_buf *bp;
-	struct xfs_inode	*dp = context->dp;
-	struct xfs_mount	*mp = dp->i_mount;
+	struct xfs_da3_icnode_hdr	nodehdr;
+	struct xfs_da_intnode		*node;
+	struct xfs_da_node_entry	*btree;
+	struct xfs_inode		*dp = context->dp;
+	struct xfs_mount		*mp = dp->i_mount;
+	struct xfs_trans		*tp = context->tp;
+	struct xfs_buf			*bp;
+	int				i;
+	int				error = 0;
+	uint16_t			magic;
+
+	ASSERT(*pbp == NULL);
+	cursor->blkno = 0;
+	for (;;) {
+		error = xfs_da3_node_read(tp, dp, cursor->blkno, -1, &bp,
+				XFS_ATTR_FORK);
+		if (error)
+			return error;
+		node = bp->b_addr;
+		magic = be16_to_cpu(node->hdr.info.magic);
+		if (magic == XFS_ATTR_LEAF_MAGIC ||
+		    magic == XFS_ATTR3_LEAF_MAGIC)
+			break;
+		if (magic != XFS_DA_NODE_MAGIC &&
+		    magic != XFS_DA3_NODE_MAGIC) {
+			XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp,
+					node);
+			goto out_corruptbuf;
+		}
+
+		dp->d_ops->node_hdr_from_disk(&nodehdr, node);
+
+		btree = dp->d_ops->node_tree_p(node);
+		for (i = 0; i < nodehdr.count; btree++, i++) {
+			if (cursor->hashval <= be32_to_cpu(btree->hashval)) {
+				cursor->blkno = be32_to_cpu(btree->before);
+				trace_xfs_attr_list_node_descend(context,
+						btree);
+				break;
+			}
+		}
+		xfs_trans_brelse(tp, bp);
+
+		if (i == nodehdr.count)
+			return 0;
+	}
+
+	*pbp = bp;
+	return 0;
+
+out_corruptbuf:
+	xfs_trans_brelse(tp, bp);
+	return -EFSCORRUPTED;
+}
+
+STATIC int
+xfs_attr_node_list(
+	struct xfs_attr_list_context	*context)
+{
+	struct xfs_attr3_icleaf_hdr	leafhdr;
+	struct attrlist_cursor_kern	*cursor;
+	struct xfs_attr_leafblock	*leaf;
+	struct xfs_da_intnode		*node;
+	struct xfs_buf			*bp;
+	struct xfs_inode		*dp = context->dp;
+	struct xfs_mount		*mp = dp->i_mount;
+	int				error;
 
 	trace_xfs_attr_node_list(context);
 
@@ -277,47 +341,9 @@  xfs_attr_node_list(xfs_attr_list_context_t *context)
 	 * Note that start of node block is same as start of leaf block.
 	 */
 	if (bp == NULL) {
-		cursor->blkno = 0;
-		for (;;) {
-			uint16_t magic;
-
-			error = xfs_da3_node_read(context->tp, dp,
-						      cursor->blkno, -1, &bp,
-						      XFS_ATTR_FORK);
-			if (error)
-				return error;
-			node = bp->b_addr;
-			magic = be16_to_cpu(node->hdr.info.magic);
-			if (magic == XFS_ATTR_LEAF_MAGIC ||
-			    magic == XFS_ATTR3_LEAF_MAGIC)
-				break;
-			if (magic != XFS_DA_NODE_MAGIC &&
-			    magic != XFS_DA3_NODE_MAGIC) {
-				XFS_CORRUPTION_ERROR("xfs_attr_node_list(3)",
-						     XFS_ERRLEVEL_LOW,
-						     context->dp->i_mount,
-						     node);
-				xfs_trans_brelse(context->tp, bp);
-				return -EFSCORRUPTED;
-			}
-
-			dp->d_ops->node_hdr_from_disk(&nodehdr, node);
-			btree = dp->d_ops->node_tree_p(node);
-			for (i = 0; i < nodehdr.count; btree++, i++) {
-				if (cursor->hashval
-						<= be32_to_cpu(btree->hashval)) {
-					cursor->blkno = be32_to_cpu(btree->before);
-					trace_xfs_attr_list_node_descend(context,
-									 btree);
-					break;
-				}
-			}
-			if (i == nodehdr.count) {
-				xfs_trans_brelse(context->tp, bp);
-				return 0;
-			}
-			xfs_trans_brelse(context->tp, bp);
-		}
+		error = xfs_attr_node_list_lookup(context, cursor, &bp);
+		if (error || !bp)
+			return error;
 	}
 	ASSERT(bp != NULL);