[PATCH] xfs: fix unaligned access in xfs_btree_visit_blocks

[PATCH] xfs: fix unaligned access in xfs_btree_visit_blocks

[Eric Sandeen]

This was throwing unaligned access warnings on sparc64:

Kernel unaligned access at TPC[1043c088] xfs_btree_visit_blocks+0x88/0xe0 [xfs]

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

diff --git a/fs/xfs/libxfs/xfs_btree.c b/fs/xfs/libxfs/xfs_btree.c
index 5392674..84089f8 100644
--- a/fs/xfs/libxfs/xfs_btree.c
+++ b/fs/xfs/libxfs/xfs_btree.c
@@ -4395,7 +4395,7 @@ struct xfs_btree_split_args {
 			xfs_btree_readahead_ptr(cur, ptr, 1);
 
 			/* save for the next iteration of the loop */
-			lptr = *ptr;
+			lptr = get_unaligned(ptr);
 		}
 
 		/* for each buffer in the level */

Re: [PATCH] xfs: fix unaligned access in xfs_btree_visit_blocks

[Eric Sandeen]

On 5/22/17 10:48 AM, Eric Sandeen wrote:
> This was throwing unaligned access warnings on sparc64:
> 
> Kernel unaligned access at TPC[1043c088] xfs_btree_visit_blocks+0x88/0xe0 [xfs]
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---

(FYI lightly tested only - did a mount & fs traversal of the fs in
question with this patch)

-Eric
 
> diff --git a/fs/xfs/libxfs/xfs_btree.c b/fs/xfs/libxfs/xfs_btree.c
> index 5392674..84089f8 100644
> --- a/fs/xfs/libxfs/xfs_btree.c
> +++ b/fs/xfs/libxfs/xfs_btree.c
> @@ -4395,7 +4395,7 @@ struct xfs_btree_split_args {
>  			xfs_btree_readahead_ptr(cur, ptr, 1);
>  
>  			/* save for the next iteration of the loop */
> -			lptr = *ptr;
> +			lptr = get_unaligned(ptr);
>  		}
>  
>  		/* for each buffer in the level */
> --
> 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
> 

Re: [PATCH] xfs: fix unaligned access in xfs_btree_visit_blocks

[Christoph Hellwig]

On Mon, May 22, 2017 at 10:48:14AM -0500, Eric Sandeen wrote:
> This was throwing unaligned access warnings on sparc64:
> 
> Kernel unaligned access at TPC[1043c088] xfs_btree_visit_blocks+0x88/0xe0 [xfs]
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
> 
> diff --git a/fs/xfs/libxfs/xfs_btree.c b/fs/xfs/libxfs/xfs_btree.c
> index 5392674..84089f8 100644
> --- a/fs/xfs/libxfs/xfs_btree.c
> +++ b/fs/xfs/libxfs/xfs_btree.c
> @@ -4395,7 +4395,7 @@ struct xfs_btree_split_args {
>  			xfs_btree_readahead_ptr(cur, ptr, 1);
>  
>  			/* save for the next iteration of the loop */
> -			lptr = *ptr;
> +			lptr = get_unaligned(ptr);

Hm..  We don't really do this for the return value of of xfs_btree_ptr_addr
anywhere else.  So if it really returns something not suitably enough
aligned we're in much deeper trouble. 

Re: [PATCH] xfs: fix unaligned access in xfs_btree_visit_blocks

[Eric Sandeen]

On 5/22/17 2:07 PM, Christoph Hellwig wrote:
> On Mon, May 22, 2017 at 10:48:14AM -0500, Eric Sandeen wrote:
>> This was throwing unaligned access warnings on sparc64:
>>
>> Kernel unaligned access at TPC[1043c088] xfs_btree_visit_blocks+0x88/0xe0 [xfs]
>>
>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
>> ---
>>
>> diff --git a/fs/xfs/libxfs/xfs_btree.c b/fs/xfs/libxfs/xfs_btree.c
>> index 5392674..84089f8 100644
>> --- a/fs/xfs/libxfs/xfs_btree.c
>> +++ b/fs/xfs/libxfs/xfs_btree.c
>> @@ -4395,7 +4395,7 @@ struct xfs_btree_split_args {
>>  			xfs_btree_readahead_ptr(cur, ptr, 1);
>>  
>>  			/* save for the next iteration of the loop */
>> -			lptr = *ptr;
>> +			lptr = get_unaligned(ptr);
> 
> Hm..  We don't really do this for the return value of of xfs_btree_ptr_addr
> anywhere else.  So if it really returns something not suitably enough
> aligned we're in much deeper trouble. 

Yeah, I took a quick look, probably too quick.  But I think the problem
here is that it's a structure copy, which is unusual compared to most of
the other callers:


        union xfs_btree_ptr             lptr;

                        lptr = *ptr;

and is probably what's generating the warning, right?  I guess a better
comment and/or commit log would be in order.

I haven't looked at every other caller but several end up calling 
xfs_btree_copy_ptrs which does a memcpy.

Hm, ok, maybe we should just be using that helper...

xfs_btree_copy_ptrs(cur, &lptr, ptr, 1);

?

-Eric

[PATCH V2] xfs: fix unaligned access in xfs_btree_visit_blocks

[Eric Sandeen]

This structure copy was throwing unaligned access warnings on sparc64:

Kernel unaligned access at TPC[1043c088] xfs_btree_visit_blocks+0x88/0xe0 [xfs]

xfs_btree_copy_ptrs does a memcpy, which avoids it.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

diff --git a/fs/xfs/libxfs/xfs_btree.c b/fs/xfs/libxfs/xfs_btree.c
index 5392674..3a673ba 100644
--- a/fs/xfs/libxfs/xfs_btree.c
+++ b/fs/xfs/libxfs/xfs_btree.c
@@ -4395,7 +4395,7 @@ struct xfs_btree_split_args {
 			xfs_btree_readahead_ptr(cur, ptr, 1);
 
 			/* save for the next iteration of the loop */
-			lptr = *ptr;
+			xfs_btree_copy_ptrs(cur, &lptr, ptr, 1);
 		}
 
 		/* for each buffer in the level */

Re: [PATCH V2] xfs: fix unaligned access in xfs_btree_visit_blocks

[Darrick J. Wong]

On Mon, May 22, 2017 at 03:06:40PM -0500, Eric Sandeen wrote:
> This structure copy was throwing unaligned access warnings on sparc64:
> 
> Kernel unaligned access at TPC[1043c088] xfs_btree_visit_blocks+0x88/0xe0 [xfs]
> 
> xfs_btree_copy_ptrs does a memcpy, which avoids it.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>

Heh. :(

Looks ok to me...
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

--D

> ---
> 
> diff --git a/fs/xfs/libxfs/xfs_btree.c b/fs/xfs/libxfs/xfs_btree.c
> index 5392674..3a673ba 100644
> --- a/fs/xfs/libxfs/xfs_btree.c
> +++ b/fs/xfs/libxfs/xfs_btree.c
> @@ -4395,7 +4395,7 @@ struct xfs_btree_split_args {
>  			xfs_btree_readahead_ptr(cur, ptr, 1);
>  
>  			/* save for the next iteration of the loop */
> -			lptr = *ptr;
> +			xfs_btree_copy_ptrs(cur, &lptr, ptr, 1);
>  		}
>  
>  		/* for each buffer in the level */
> 
> --
> 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